fix(playground): harden jsonPatch apply and component id assignment - #222
Open
yy-wow wants to merge 15 commits into
Open
fix(playground): harden jsonPatch apply and component id assignment#222yy-wow wants to merge 15 commits into
yy-wow wants to merge 15 commits into
Conversation
Prevent duplicate ids on insert, persist applyFailed for history cards, and stop swallowing format errors so failed patches fail closed without crashing debug UI. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe update tracks JSON patch application failures through streaming, persisted conversations, schema cards, and rendering. It also preserves schema IDs, separates version-history current marking, hardens patch formatting, and replaces the history-panel chevron asset. ChangesSchema state and rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant jsonPatchRenderer
participant applyJsonPatchOperations
participant ConversationState
participant TemplateSchemaMessageRenderer
jsonPatchRenderer->>applyJsonPatchOperations: apply JSON patch operations
applyJsonPatchOperations-->>jsonPatchRenderer: return patched schema or null
jsonPatchRenderer->>ConversationState: store applyFailed flag
ConversationState->>TemplateSchemaMessageRenderer: provide card state and errors
TemplateSchemaMessageRenderer-->>jsonPatchRenderer: render patch failure message
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sites/playground/web/src/components/genui-template/GenuiTemplateChat.vue`:
- Around line 297-301: Prevent failed schema patches from using stale preview
snapshots: in
sites/playground/web/src/components/genui-template/GenuiTemplateChat.vue lines
297-301, omit the schema field when the card has failed to apply; in
sites/playground/web/src/components/genui-template/template-chat-utils/conversation-schema.ts
lines 161-167, update the reconstruction logic to ignore persisted schema when
applyFailed is true, while preserving normal schema resolution for successful
cards.
In
`@sites/playground/web/src/components/genui-template/template-chat-utils/json-patch-format.ts`:
- Around line 82-85: Update the conversion flow around formatJsonPatch so it
detects any operation without a resolved idToPath and fails the entire patch
instead of filtering that operation out. Preserve conversion of all operations
only when every target resolves, and ensure the failure propagates so mixed
invalid patches cannot be persisted as applyFailed: false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a482abf7-e93e-4f71-88f9-d5749563247f
⛔ Files ignored due to path filters (1)
sites/playground/web/src/assets/images/chevron-down.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
sites/playground/web/src/components/genui-template/GenuiTemplateChat.vuesites/playground/web/src/components/genui-template/SchemaVersionCard.vuesites/playground/web/src/components/genui-template/SchemaVersionHistoryPanel.vuesites/playground/web/src/components/genui-template/TemplateSchemaMessageRenderer.vuesites/playground/web/src/components/genui-template/chat.types.tssites/playground/web/src/components/genui-template/composables/use-template-actions.tssites/playground/web/src/components/genui-template/composables/use-template-conversation.tssites/playground/web/src/components/genui-template/composables/use-template-schema.tssites/playground/web/src/components/genui-template/composables/use-template-stream-render.tssites/playground/web/src/components/genui-template/composables/use-template-version-control.tssites/playground/web/src/components/genui-template/template-chat-utils/conversation-schema.tssites/playground/web/src/components/genui-template/template-chat-utils/json-patch-format.tssites/playground/web/src/components/genui-template/template-chat-utils/schema-id-generator.tssites/playground/web/src/components/genui-template/template-chat-utils/schema-version-history.ts
Skip persisting/rebuilding schema for applyFailed cards, and fail the whole patch when any operation lacks a resolved target. Co-authored-by: Cursor <cursoragent@cursor.com>
Enhance the handling of applyFailed cards by returning a parsed schema from the previous state instead of null, ensuring better persistence of conversation state.
Refactor the ID merging process to improve the conditions under which IDs can be borrowed from previous nodes. This includes adding a new helper function to streamline the logic and ensuring that child nodes are only processed when their lengths match.
…andling Update the schema card finalization process to improve ID generation for components and enhance notification handling. The new implementation ensures that the current schema is set correctly based on the preview state, while also streamlining the logic for applying failed cards.
Enhance the error message handling in the schema version card and template message renderer components. Update the logic for applying failed JSON patches and streamline the error message propagation. Remove unnecessary flags and improve the clarity of the error state management.
…enderer Update the logic for setting the current preview schema to use the stream completion status instead of the successful parse flag, ensuring accurate schema representation during streaming operations.
…ne error handling Refactor the handling of error messages by removing the errorMessagesMap prop from several components. Update the logic to directly utilize the stream's errorMessagesMap, ensuring a more cohesive error management approach across the template components.
…actions Add the clearErrorMessages function from useTemplateStreamRender to the useTemplateActions composable. This enhancement ensures that error messages are cleared during the reset process, improving error management and user experience in the template components.
…on history handling Eliminate the isCurrent flag from the schema version history entry interface and related functions to streamline the versioning logic. Update the SchemaVersionHistoryPanel component to reflect these changes, enhancing clarity and maintainability of the code.
…ema card processing Refactor the error handling for JSON patches by introducing a new function to set the applyFailed flag directly on schema cards. This change simplifies the error management process across components and ensures accurate tracking of JSON patch application failures. Additionally, remove redundant error message handling logic from various components to improve code clarity and maintainability.
…dling Refactor the JSON patch error handling by introducing the resolveJsonPatchApplyFailed function, which centralizes the logic for determining the applyFailed state of schema cards. Update the jsonPatchRenderer to utilize the new function, ensuring accurate tracking of JSON patch application failures and enhancing the clarity of schema processing during streaming operations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
模板对话里，通过增量修改（json-patch）来改页面 schema。大模型输出的 op 有时会出现 id 转换路径后对应不上，导致解析失败。
旧逻辑容易出现如下问题：
问题分析
1. 解析失败的 patch 怎么处理
2. 失败信息记在哪
以前用组件外的
errorMessagesMap记失败。问题是：3. 组件 id 什么时候生成
流式每一帧都补 id，或者按旧树位置借 id，都会让预览不稳定。id 只用在下次大模型生成 op 时做定位用，应该在生成结束、确认没失败后再一次性补齐；已有且不重复的 id 保留，缺的或重复的再生成。
解决方案
1. 解析失败的 patch：全部 op 应用成功才算解析成功
2. 卡片新增标记字段：
applyFailedapplyFailed字段，流式过程中应用成功/失败，直接写到对应卡片。applyFailed显示失败，不再依赖 errorMessagesMap。3. 组件 id：生成结束后一次性补齐
其他修改