Feat：Add AI assistant to canvas node#1800
Merged
chilingling merged 10 commits intoopentiny:developfrom Apr 30, 2026
Merged
Conversation
Contributor
|
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:
WalkthroughAdds a per-node AI assistant: typed AI status in canvas types, UI components (chat/loading/confirm), a composable implementing AI workflow and JSON-patch application, agent service endpoints, canvas API updates for schema initialization/update/insert, and pre-save validation to block unresolved AI modifications. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CanvasUI as Canvas UI (CanvasAction.vue)
participant AIChat as AI Workflow (useAIChat)
participant AIService as Agent Services
participant Schema as Page Schema Store
User->>CanvasUI: Open AI helper for node
CanvasUI->>AIChat: openNodeAIChat(nodeId)
CanvasUI->>CanvasUI: render Chat/Dialog components
User->>CanvasUI: Submit message
CanvasUI->>AIChat: buildAIChatRequest(nodeId, message)
AIChat->>AIService: search() / fetchAssets() (optional)
AIChat->>AIService: chat(request)
AIService-->>AIChat: AI response (JSON patches)
AIChat->>Schema: applyAIPatches(nodeId, patches)
Schema-->>AIChat: updated schema
AIChat-->>CanvasUI: show confirm dialog (aiModifiedNodeData)
alt User adopts
CanvasUI->>AIChat: adoptNodeAIModification(nodeId)
AIChat->>Schema: persist modified data and publish schemaChange
else User rejects
CanvasUI->>AIChat: rejectNodeAIModification(nodeId)
AIChat->>Schema: restore originalNodeData
end
User->>CanvasUI: Save
CanvasUI->>AIChat: hasAnyPendingAIModification()
alt Pending exist
CanvasUI->>User: Block save + warning
else No pending
CanvasUI->>Schema: proceed save
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
packages/toolbars/save/src/js/index.ts (1)
158-160: Reuse a single validation instance in this block.This is correct as-is, but calling
useSaveValidation()once here will reduce duplication and keep this branch easier to maintain.♻️ Small cleanup
- if (useSaveValidation().hasAnyPendingAIModification()) { - const pendingNodeIds = useSaveValidation().getAllNodesWithPendingAIModification() + const saveValidation = useSaveValidation() + if (saveValidation.hasAnyPendingAIModification()) { + const pendingNodeIds = saveValidation.getAllNodesWithPendingAIModification()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/toolbars/save/src/js/index.ts` around lines 158 - 160, Replace the two separate calls to useSaveValidation() with a single local variable: call useSaveValidation() once (e.g., const saveValidation = useSaveValidation()) and then use saveValidation.hasAnyPendingAIModification() and saveValidation.getAllNodesWithPendingAIModification() to compute the branch and pendingNodeIds; update references to useSaveValidation() in this block to use the new saveValidation variable.packages/canvas/container/src/components/AIConfirmDialog.vue (2)
28-28: Remove unusedclosesurface if not needed.
closeis declared inemitsand.close-iconstyles exist, but no close UI/action is implemented in this component.Also applies to: 87-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/components/AIConfirmDialog.vue` at line 28, The component declares an unused "close" event and has .close-icon styles without any close UI/action; either implement the close button and emit it or remove the unused surface. To fix, edit AIConfirmDialog.vue: remove 'close' from the emits array (emits: ['confirm','cancel','refresh']) and delete the .close-icon CSS and any unused close-related markup/refs, or alternatively add a close button element (e.g., a clickable .close-icon) wired to $emit('close') and a handler in the component if you prefer to keep the event.
11-11: Make refresh action keyboard-accessible.Using a clickable icon alone here limits keyboard interaction. Prefer a button wrapper (or add role/tabindex/key handlers) for accessibility parity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/components/AIConfirmDialog.vue` at line 11, The refresh SVG icon currently only has a click handler (svg-icon with class "refresh-icon" and `@click`="handleRefresh"), which is not keyboard-accessible; update the template so the icon is keyboard operable by either wrapping the svg-icon in a semantic button element (preferred) that forwards click and has accessible attributes (aria-label/title) or, if you must keep the element, add role="button", tabindex="0" and keydown handlers that call handleRefresh for Enter/Space; ensure focus styles and use the existing handleRefresh method name so behavior remains unchanged.packages/canvas/container/src/components/CanvasAIChat.vue (1)
28-41: Trim unused close API in this component.
closeis declared/emitted in logic, but there is no close interaction in the template. Consider removing it until needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/components/CanvasAIChat.vue` around lines 28 - 41, The component declares and emits a "close" event but the template never uses it; remove the unused API by deleting "close" from the emits array and removing the handleClose function (and any emit('close') calls) to keep the component surface minimal; specifically update the emits declaration (remove 'close') and delete the handleClose function and any references to emit('close') in setup.packages/canvas/container/src/components/AILoadingDialog.vue (1)
93-97: Drop unused.refresh-iconstyle block.No refresh icon exists in this component template, so this style section is dead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/components/AILoadingDialog.vue` around lines 93 - 97, Remove the dead CSS rule for .refresh-icon from the AILoadingDialog.vue component’s style block: delete the entire .refresh-icon rule set since no element uses that class in the template, and before committing run a quick search for ".refresh-icon" to confirm it's not referenced elsewhere; if there are references, update or remove them accordingly.packages/toolbars/save/src/Main.vue (1)
179-190: Centralize save-block policy to one place.This duplicates the pending-AI check already present in
openCommon. Keeping only one enforcement path avoids message/UX divergence later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/toolbars/save/src/Main.vue` around lines 179 - 190, The pending-AI check in Main.vue duplicates the enforcement already implemented in openCommon; remove this duplicate block (the calls to useSaveValidation().hasAnyPendingAIModification() and getAllNodesWithPendingAIModification() and the useNotify warning) and rely on openCommon's single save-block policy, or replace the block with a single call to the centralized openCommon validation/guard function so all save-block logic (and UX messaging) remains in one place.packages/canvas/DesignCanvas/src/api/useCanvas.ts (2)
438-451: RedundantuseAIChat()call inside the recursive function.
useAIChat()is already destructured at line 431 (useAIChat().initializeNodeAIStatus), but a new reference is obtained at line 440. SinceuseAIChatreturns the same singleton functions, this works but is redundant.♻️ Suggested refactor
// 初始化新节点的AI状态 if (newNodeData.id) { - useAIChat().initializeNodeAIStatus(newNodeData) + const { initializeNodeAIStatus } = useAIChat() + initializeNodeAIStatus(newNodeData) } // 6. 如果新节点有子节点，递归构建 nodeMap if (Array.isArray(newNodeData?.children) && newNodeData.children.length > 0) { const newNode = getNode(newNodeData.id) generateNodesMap(newNodeData.children, newNode) // 递归初始化所有子节点的AI状态 - const { initializeNodeAIStatus: initAIStatus } = useAIChat() const initChildrenAIStatus = (children: Node[]) => { children.forEach((child) => { if (child.id) { - initAIStatus(child) + initializeNodeAIStatus(child) } if (Array.isArray(child?.children) && child.children.length > 0) { initChildrenAIStatus(child.children) } }) } initChildrenAIStatus(newNodeData.children) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts` around lines 438 - 451, The recursive initializer creates a redundant call to useAIChat(); remove the extra const { initializeNodeAIStatus: initAIStatus } = useAIChat() inside initChildrenAIStatus and instead reuse the already available initializeNodeAIStatus (or its previously assigned alias) when calling initAIStatus for each child; keep the recursion logic and invocation on newNodeData.children unchanged.
241-247: Consider excluding the root wrapper node from AI status initialization.The
nodesMapincludes the root node withid: 0(the wrapper div). Initializing AI status for this synthetic node may be unnecessary and could cause confusion. The root node is not a user-created component.🔧 Suggested fix
// 为新增的节点初始化AI状态（已存在的不覆盖） const { initializeNodeAIStatus } = useAIChat() nodesMap.value.forEach(({ node }) => { - if (node.id && !pageState.nodesStatus[node.id]?.aiStatus) { + if (node.id && node.id !== 0 && !pageState.nodesStatus[node.id]?.aiStatus) { initializeNodeAIStatus(node) } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts` around lines 241 - 247, The current loop that calls initializeNodeAIStatus for each entry in nodesMap should skip the synthetic root wrapper node (id 0) so we don't initialize AI state for a non-user component; update the iteration inside useCanvas (the block referencing nodesMap.value.forEach, initializeNodeAIStatus, and pageState.nodesStatus) to add a guard that returns/continues when node.id === 0 (or another root-identifying flag if present) before checking/initializing aiStatus.packages/canvas/container/src/composables/useAIChat.ts (2)
474-480: Consider adding error handling for optional context fetching.The
search()andfetchAssets()calls are not wrapped in try-catch. If these optional services fail, the entire AI chat request will fail. Consider handling these errors gracefully with fallback to empty context.🛡️ Suggested defensive handling
if (getRobotServiceOptions()?.enableRagContext) { - referenceContext = await search(content) + try { + referenceContext = await search(content) + } catch (error) { + console.warn('Failed to fetch RAG context:', error) + referenceContext = '' + } } if (getRobotServiceOptions()?.enableResourceContext) { const appId = getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id - imageAssets = await fetchAssets(appId) + try { + imageAssets = await fetchAssets(appId) + } catch (error) { + console.warn('Failed to fetch assets:', error) + imageAssets = [] + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 474 - 480, When optional context fetches may fail, wrap the calls to search(...) and fetchAssets(...) in try/catch blocks so failures don't abort the whole AI chat flow: for the enableRagContext branch, call search(content) inside a try and on error set referenceContext = [] (or null/empty fallback used elsewhere) and log the error; for the enableResourceContext branch, call fetchAssets(appId) inside a try and on error set imageAssets = [] and log the error (use the same getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id to build appId), ensuring referenceContext and imageAssets have safe default values when the services fail.
79-96: Parameternodeshould be typed with a proper interface.The
nodeparameter is typed asobject, but the function accessesnode.id(line 83). This lacks type safety.🔧 Suggested fix
-const initializeNodeAIStatus = (node: object, initialStatus: Partial<NodeAIStatus> = {}) => { +const initializeNodeAIStatus = (node: { id: string | number; [key: string]: any }, initialStatus: Partial<NodeAIStatus> = {}) => { const { pageState } = useCanvas() if (!pageState.nodesStatus[node.id]) {Or import the
Nodetype from the types file if available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 79 - 96, The parameter node in initializeNodeAIStatus is currently typed as object but the function reads node.id; update the signature to use the proper Node interface (or import the existing Node type) so node.id and any other properties are type-checked; change the parameter from node: object to node: Node (or the correct named type) and adjust any imports to bring in Node from the types file to restore type safety for initializeNodeAIStatus and usages of node.id.packages/toolbars/save/src/js/aiSaveValidation.ts (1)
46-54: Inefficient:useCanvas()called inside map callback for each node.
useCanvas()is called repeatedly inside the.map()callback. Move it outside the callback.♻️ Suggested fix
const pendingNodeIds = getAllNodesWithPendingAIModification() + const { pageState } = useCanvas() const pendingNodes = pendingNodeIds.map((nodeId) => { - const { pageState } = useCanvas() const aiStatus = pageState.nodesStatus[nodeId]?.aiStatus || null return { nodeId, aiStatus } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/toolbars/save/src/js/aiSaveValidation.ts` around lines 46 - 54, The code calls useCanvas() inside the pendingNodes map causing repeated hook reads; move the call outside the map by calling const { pageState } = useCanvas() once before building pendingNodes (which is derived from pendingNodeIds from getAllNodesWithPendingAIModification()), then inside the map access pageState.nodesStatus[nodeId]?.aiStatus || null to populate aiStatus for each nodeId; update references to pendingNodeIds, pendingNodes, useCanvas, getAllNodesWithPendingAIModification and pageState accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/canvas/container/src/components/CanvasAction.vue`:
- Around line 806-819: The call to buildAIChatRequest can throw and is currently
outside the try block, leaving the node in loading state; move
buildAIChatRequest(chatContent) inside the existing try {} so both building the
params and awaiting chat(params) are protected, and in the catch block call
cancelNodeAILoading(nodeId) (as already done) and rethrow or handle/log the
error as appropriate; update references to params so its scope is inside the
try, and keep applyAIPatches(nodeId, response, chatContent) in the try after
chat(params) succeeds.
- Around line 747-761: handleAIChatComplete currently awaits buildAIChatRequest,
chat, and applyAIPatches without catching errors which can leave the node stuck
in loading; wrap the async flow in try/catch/finally inside
handleAIChatComplete, call startNodeAILoading(currentSchema.id, ...) before the
try, in try do params = await buildAIChatRequest(...); response = await
chat(params); applyAIPatches(...), in catch log/report the error and set an AI
error state on the node (or pass an error message to the node), and in finally
ensure you clear the loading state by calling the corresponding loader-clear
function (e.g., stopNodeAILoading or clearNodeAILoading) for currentSchema.id so
the node never remains loading indefinitely.
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 154-159: The call to deepClone(currentStatus.aiModifiedNodeData)
can pass undefined; update the code around updateNodeAIStatus so you only call
deepClone when currentStatus.aiModifiedNodeData is defined (e.g., compute const
original = currentStatus.aiModifiedNodeData ?
deepClone(currentStatus.aiModifiedNodeData) : undefined) and then pass original
into updateNodeAIStatus; reference the updateNodeAIStatus call and the
deepClone/currentStatus.aiModifiedNodeData symbols when making this change.
In `@packages/canvas/container/src/services/agentServices.ts`:
- Around line 46-50: The current chain uses group.resources directly which can
throw if a group has no resources; change the flattening to guard missing values
by using group?.resources or defaulting to an empty array. For example, replace
the .map((group)=>group.resources).flat().filter(...) logic with a guarded
flatten such as .flatMap((group: any) => (group?.resources ?? []).filter((item:
any) => item?.description)) so getMetaApi(META_SERVICE.Http).get(...) results
are normalized without throwing on missing resources or descriptions.
In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts`:
- Line 18: There is a circular dependency between useCanvas and useAIChat
because useCanvas imports useAIChat and useAIChat calls useCanvas(); break this
by extracting the shared AI state/initialization logic (the parts that access
pageState and initialize AI state) into a new module (e.g., aiStateManager or
useAIState) that exports the common state and helper functions; update useCanvas
to import only the new aiState module for AI initialization and update useAIChat
to import the same aiState module instead of calling useCanvas(), leaving
useCanvas and useAIChat decoupled and both referencing the shared ai state
helpers.
In `@packages/toolbars/save/src/js/aiSaveValidation.ts`:
- Around line 134-150: The function rejectAllPendingModifications is calling
rejectNodeAIModification from useCanvas but that symbol is exported from
useAIChat (same bug as adoptAllPendingModifications); update
rejectAllPendingModifications to obtain rejectNodeAIModification from useAIChat
(e.g., const { rejectNodeAIModification } = useAIChat()) instead of useCanvas(),
keep getAllNodesWithPendingAIModification usage and the loop logic as-is, and
ensure any other references (like in adoptAllPendingModifications) are corrected
to import/request rejectNodeAIModification from useAIChat so the correct hook
provides the function.
- Around line 112-128: The bug is that adoptAllPendingModifications destructures
adoptNodeAIModification from useCanvas (which doesn't export it) causing a
runtime error; fix it by obtaining adoptNodeAIModification from useAIChat
instead of useCanvas (e.g., replace const { adoptNodeAIModification } =
useCanvas() with const { adoptNodeAIModification } = useAIChat() or call
useAIChat() once before the forEach), keep the rest of
adoptAllPendingModifications unchanged (ensure the symbol names
adoptAllPendingModifications and adoptNodeAIModification are used exactly as in
the diff).
- Around line 6-9: Calling useMessage() at module scope can fail if the module
is imported before Vue is initialized; move the composable call inside the
function that needs it. Remove the top-level const { publish } = useMessage()
and instead call useMessage() inside showSaveValidationDialog (or any other
function that uses publish) to obtain publish locally before using it; update
references to publish in aiSaveValidation.ts accordingly so there are no
module-level composable calls.
---
Nitpick comments:
In `@packages/canvas/container/src/components/AIConfirmDialog.vue`:
- Line 28: The component declares an unused "close" event and has .close-icon
styles without any close UI/action; either implement the close button and emit
it or remove the unused surface. To fix, edit AIConfirmDialog.vue: remove
'close' from the emits array (emits: ['confirm','cancel','refresh']) and delete
the .close-icon CSS and any unused close-related markup/refs, or alternatively
add a close button element (e.g., a clickable .close-icon) wired to
$emit('close') and a handler in the component if you prefer to keep the event.
- Line 11: The refresh SVG icon currently only has a click handler (svg-icon
with class "refresh-icon" and `@click`="handleRefresh"), which is not
keyboard-accessible; update the template so the icon is keyboard operable by
either wrapping the svg-icon in a semantic button element (preferred) that
forwards click and has accessible attributes (aria-label/title) or, if you must
keep the element, add role="button", tabindex="0" and keydown handlers that call
handleRefresh for Enter/Space; ensure focus styles and use the existing
handleRefresh method name so behavior remains unchanged.
In `@packages/canvas/container/src/components/AILoadingDialog.vue`:
- Around line 93-97: Remove the dead CSS rule for .refresh-icon from the
AILoadingDialog.vue component’s style block: delete the entire .refresh-icon
rule set since no element uses that class in the template, and before committing
run a quick search for ".refresh-icon" to confirm it's not referenced elsewhere;
if there are references, update or remove them accordingly.
In `@packages/canvas/container/src/components/CanvasAIChat.vue`:
- Around line 28-41: The component declares and emits a "close" event but the
template never uses it; remove the unused API by deleting "close" from the emits
array and removing the handleClose function (and any emit('close') calls) to
keep the component surface minimal; specifically update the emits declaration
(remove 'close') and delete the handleClose function and any references to
emit('close') in setup.
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 474-480: When optional context fetches may fail, wrap the calls to
search(...) and fetchAssets(...) in try/catch blocks so failures don't abort the
whole AI chat flow: for the enableRagContext branch, call search(content) inside
a try and on error set referenceContext = [] (or null/empty fallback used
elsewhere) and log the error; for the enableResourceContext branch, call
fetchAssets(appId) inside a try and on error set imageAssets = [] and log the
error (use the same getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id to
build appId), ensuring referenceContext and imageAssets have safe default values
when the services fail.
- Around line 79-96: The parameter node in initializeNodeAIStatus is currently
typed as object but the function reads node.id; update the signature to use the
proper Node interface (or import the existing Node type) so node.id and any
other properties are type-checked; change the parameter from node: object to
node: Node (or the correct named type) and adjust any imports to bring in Node
from the types file to restore type safety for initializeNodeAIStatus and usages
of node.id.
In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts`:
- Around line 438-451: The recursive initializer creates a redundant call to
useAIChat(); remove the extra const { initializeNodeAIStatus: initAIStatus } =
useAIChat() inside initChildrenAIStatus and instead reuse the already available
initializeNodeAIStatus (or its previously assigned alias) when calling
initAIStatus for each child; keep the recursion logic and invocation on
newNodeData.children unchanged.
- Around line 241-247: The current loop that calls initializeNodeAIStatus for
each entry in nodesMap should skip the synthetic root wrapper node (id 0) so we
don't initialize AI state for a non-user component; update the iteration inside
useCanvas (the block referencing nodesMap.value.forEach, initializeNodeAIStatus,
and pageState.nodesStatus) to add a guard that returns/continues when node.id
=== 0 (or another root-identifying flag if present) before checking/initializing
aiStatus.
In `@packages/toolbars/save/src/js/aiSaveValidation.ts`:
- Around line 46-54: The code calls useCanvas() inside the pendingNodes map
causing repeated hook reads; move the call outside the map by calling const {
pageState } = useCanvas() once before building pendingNodes (which is derived
from pendingNodeIds from getAllNodesWithPendingAIModification()), then inside
the map access pageState.nodesStatus[nodeId]?.aiStatus || null to populate
aiStatus for each nodeId; update references to pendingNodeIds, pendingNodes,
useCanvas, getAllNodesWithPendingAIModification and pageState accordingly.
In `@packages/toolbars/save/src/js/index.ts`:
- Around line 158-160: Replace the two separate calls to useSaveValidation()
with a single local variable: call useSaveValidation() once (e.g., const
saveValidation = useSaveValidation()) and then use
saveValidation.hasAnyPendingAIModification() and
saveValidation.getAllNodesWithPendingAIModification() to compute the branch and
pendingNodeIds; update references to useSaveValidation() in this block to use
the new saveValidation variable.
In `@packages/toolbars/save/src/Main.vue`:
- Around line 179-190: The pending-AI check in Main.vue duplicates the
enforcement already implemented in openCommon; remove this duplicate block (the
calls to useSaveValidation().hasAnyPendingAIModification() and
getAllNodesWithPendingAIModification() and the useNotify warning) and rely on
openCommon's single save-block policy, or replace the block with a single call
to the centralized openCommon validation/guard function so all save-block logic
(and UX messaging) remains in one place.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e5d9b243-e326-4b7c-a75d-60c806fecc91
📒 Files selected for processing (14)
packages/canvas/DesignCanvas/src/api/types.tspackages/canvas/DesignCanvas/src/api/useCanvas.tspackages/canvas/container/assets/loading.webppackages/canvas/container/src/components/AIConfirmDialog.vuepackages/canvas/container/src/components/AILoadingDialog.vuepackages/canvas/container/src/components/CanvasAIChat.vuepackages/canvas/container/src/components/CanvasAction.vuepackages/canvas/container/src/composables/useAIChat.tspackages/canvas/container/src/services/agentServices.tspackages/canvas/package.jsonpackages/plugins/robot/src/composables/core/useConfig.tspackages/toolbars/save/src/Main.vuepackages/toolbars/save/src/js/aiSaveValidation.tspackages/toolbars/save/src/js/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/canvas/container/src/composables/useAIChat.ts (2)
155-158:⚠️ Potential issue | 🟠 MajorGuard
deepClonewhen there is no AI-modified snapshot.Both acceptance paths assume
aiModifiedNodeDatais present, butconfirmis just a state flag here. If that snapshot was never populated, these lines still calldeepClone(undefined)and can break the adopt/confirm flow. Bail out or fall back to the existingoriginalNodeDatawhen the modified snapshot is missing.Also applies to: 218-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 155 - 158, Guard the deepClone call so we don't clone undefined: in the updateNodeAIStatus calls (the block updating state to 'completed' and the similar block at the other location), check currentStatus.aiModifiedNodeData and if it's falsy use currentStatus.originalNodeData (or simply omit cloning and pass the existing originalNodeData) instead of calling deepClone(undefined); update the payload so originalNodeData is deepClone(currentStatus.aiModifiedNodeData ?? currentStatus.originalNodeData) (or equivalent guard) and ensure aiModifiedNodeData is set to undefined only when appropriate.
664-691:⚠️ Potential issue | 🟠 MajorValidate the AI response shape and resolved target path before mutating state.
This block still assumes
chatResponse.choices[0].message.contentexists, and it also assumesfindJsonPatchPath()always returns a string. If either is missing, patch application throws or builds invalid"null..."pointers, but the later state updates still move the node toconfirm, mark the page dirty, and add history as if the apply succeeded. Please fail fast unless bothcontentandparentPathare present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 664 - 691, Check that chatResponse.choices[0].message.content exists and that parseJsonPatches(content) returns a non-empty array, and also verify findJsonPatchPath(getPageSchema(), nodeId) returns a valid (non-null/non-empty) parentPath before calling applyPatchesToSchema; if either check fails, abort early (return false) and do not call applyPatchesToSchema, updatePageSchema, updateNodeAIStatus, setSaved, useHistory().addHistory, or completeNodeAILoading. Concretely, add guards around reading chatResponse.choices[0].message.content and around parentPath from findJsonPatchPath, only proceed to compute newSchema, call fixMethods/schemaAutoFix, and update state (updatePageSchema, updateNodeAIStatus, setSaved, useHistory().addHistory, completeNodeAILoading) when both content/patched JSON and parentPath are valid. Ensure modifiedNodeData determination (getNode / deepClone fallback) only runs after these validations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 172-179: The rollback uses Object.assign(node,
deepClone(currentStatus.originalNodeData)) which only overwrites/adds keys and
leaves any AI-added properties/children behind; to fully restore, replace the
node's shape in-place by removing keys not present in original and then
assigning the deep-cloned original (or clear all own properties and assign each
key from deepClone(currentStatus.originalNodeData)) so the reactive reference
returned by getNode(nodeId) is preserved; update the same pattern in the other
occurrence (the block around lines 239-245) and ensure you call publish({ topic:
'schemaChange', data: { nodeId } }) after the in-place restoration.
- Around line 711-717: Optional context lookups (search and fetchAssets) are
currently allowed to throw and abort buildAIChatRequest; wrap the await calls to
search(content) and fetchAssets(appId) in try/catch blocks inside useAIChat (the
block where getRobotServiceOptions()?.enableRagContext and enableResourceContext
are checked) so failures "fail open": on error set referenceContext to an empty
array or null and imageAssets to an empty array, log the error for diagnostics
(use existing logger or processLogger), and do not rethrow so buildAIChatRequest
can continue building the request even if optional lookups fail.
- Around line 538-539: The segment-walker getValueBySegments fails when an
intermediate lookup returns undefined because it only checks o !== null and the
next iteration tries to access o[key], causing an exception (which is swallowed
in applyPatchesToSchema) and aborts applying patches; update getValueBySegments
to guard against both null and undefined (e.g., use o != null or explicitly
check o !== null && o !== undefined) in the reducer so that when a segment is
missing it returns undefined safely and does not throw, ensuring
applyPatchesToSchema can continue applying add-patch operations.
- Around line 81-96: The function initializeNodeAIStatus uses a parameter typed
as object but accesses node.id, which fails strict TypeScript checks; import the
Node interface from `@opentiny/tiny-engine-canvas/types` and change the parameter
signature to initializeNodeAIStatus(node: Node, initialStatus:
Partial<NodeAIStatus> = {}), then ensure any downstream usages (e.g.,
deepClone(node), pageState.nodesStatus[node.id]) remain compatible with the Node
type so the compiler recognizes node.id correctly.
---
Duplicate comments:
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 155-158: Guard the deepClone call so we don't clone undefined: in
the updateNodeAIStatus calls (the block updating state to 'completed' and the
similar block at the other location), check currentStatus.aiModifiedNodeData and
if it's falsy use currentStatus.originalNodeData (or simply omit cloning and
pass the existing originalNodeData) instead of calling deepClone(undefined);
update the payload so originalNodeData is
deepClone(currentStatus.aiModifiedNodeData ?? currentStatus.originalNodeData)
(or equivalent guard) and ensure aiModifiedNodeData is set to undefined only
when appropriate.
- Around line 664-691: Check that chatResponse.choices[0].message.content exists
and that parseJsonPatches(content) returns a non-empty array, and also verify
findJsonPatchPath(getPageSchema(), nodeId) returns a valid (non-null/non-empty)
parentPath before calling applyPatchesToSchema; if either check fails, abort
early (return false) and do not call applyPatchesToSchema, updatePageSchema,
updateNodeAIStatus, setSaved, useHistory().addHistory, or completeNodeAILoading.
Concretely, add guards around reading chatResponse.choices[0].message.content
and around parentPath from findJsonPatchPath, only proceed to compute newSchema,
call fixMethods/schemaAutoFix, and update state (updatePageSchema,
updateNodeAIStatus, setSaved, useHistory().addHistory, completeNodeAILoading)
when both content/patched JSON and parentPath are valid. Ensure modifiedNodeData
determination (getNode / deepClone fallback) only runs after these validations.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 350766d7-cd4f-45a2-a05a-1cb8ce0897b8
📒 Files selected for processing (1)
packages/canvas/container/src/composables/useAIChat.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/canvas/container/src/components/CanvasAction.vue`:
- Around line 747-760: handleAILoadingCancel currently only updates local state
but doesn't stop the asynchronous chat() started by
handleAIChatComplete/handleAIRefresh, so late responses still call
applyAIPatches and mutate schemas; fix by attaching an AbortController or a
per-node request token when calling chat(params) inside handleAIChatComplete and
handleAIRefresh (created alongside startNodeAILoading), pass the signal/token
into chat, and have handleAILoadingCancel call abort() or mark the token
cancelled; before calling applyAIPatches, verify the node is still in loading
state and that the signal/token is not aborted (or matches the active token for
that node) to ignore late responses and prevent mutations when cancelled.
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 95-99: closeNodeAIHelper currently forces state: 'hidden' which
can hide nodes that still have pending AI modifications; change
closeNodeAIHelper to first call hasNodePendingAIModification(nodeId) and, if it
returns true, avoid setting state to 'hidden' (keep or set state to 'confirm')
so the save blocker and confirm flow remain active; otherwise proceed to set
state: 'hidden' and lastAIAction: 'close' when no pending changes. Ensure you
use updateNodeAIStatus(nodeId, {...}) and the existing
hasNodePendingAIModification(nodeId) check so the toolbar toggle cannot dismiss
pending AI changes.
In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts`:
- Around line 104-121: The current initializeAllNodesAIStatus only walks
pageState.pageSchema.children and misses nodes recorded in pageState.nodesMap
(created by generateNodesMap), so update initializeAllNodesAIStatus to also
iterate over Object.values(pageState.nodesMap) (or the generated nodesMap) and
call initializeNodeAIStatus(node) for any node with an id that lacks
pageState.nodesStatus[node.id]?.aiStatus; keep the existing recursive
traverseNodes for pageSchema children but dedupe by id to avoid
double-initializing, and apply the same change to the other initialization
helper that uses the same pageSchema-only traversal so nodes nested in
props/slots (e.g., TinyGrid slot content) get originalNodeData and AI state
initialized.
- Around line 272-282: After rebuilding nodesMap (the block that clears
nodesMap, sets root and calls generateNodesMap) prune orphaned entries from
pageState.nodesStatus: collect all current node ids from nodesMap.value (the
keys or node.id from each entry) and remove any keys in pageState.nodesStatus
that are not in that set so stale statuses (e.g. deleted nodes stuck in
"confirm") are dropped; do this before the loop that initializes AI state with
initializeNodeAIStatus and ensure you use the same identifiers (nodesMap,
pageState.nodesStatus, initializeNodeAIStatus,
getAllNodesWithPendingAIModification) so subsequent checks like
getAllNodesWithPendingAIModification only see existing nodes.
- Around line 88-101: Import the NodeAIStatus type from ./types and update the
initializeNodeAIStatus signature to use a properly typed node parameter (e.g.,
node: { id: string } or the existing Node interface if available) instead of
node: object so node.id is type-safe; change the parameter type to node: { id:
string } | Node and add the import statement for NodeAIStatus (and Node if you
use it) so Partial<NodeAIStatus> resolves and the code compiles.
- Around line 474-485: The recursive initializer calls a nonexistent
initAIStatus causing a runtime error; replace that call with the correct
function initializeNodeAIStatus so initChildrenAIStatus(children: Node[]) uses
initializeNodeAIStatus(child) when child.id exists; update the body of
initChildrenAIStatus (and any other references) to consistently call
initializeNodeAIStatus and then recurse over newNodeData.children as already
done.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6b86828-69f8-48b1-9aa6-13936e826d46
📒 Files selected for processing (4)
packages/canvas/DesignCanvas/src/api/useCanvas.tspackages/canvas/container/src/components/CanvasAction.vuepackages/canvas/container/src/composables/useAIChat.tspackages/toolbars/save/src/js/aiSaveValidation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/toolbars/save/src/js/aiSaveValidation.ts
hexqi
reviewed
Apr 29, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
packages/canvas/container/src/composables/useAIChat.ts (4)
674-680:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOptional RAG/resource lookups should fail open.
search()andfetchAssets()are enrichments, but either rejection currently abortsbuildAIChatRequest()and blocks the whole assistant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 674 - 680, The optional RAG/resource lookups (search and fetchAssets) must fail open so they don't abort buildAIChatRequest; wrap the calls to search(content) and fetchAssets(appId) in try/catch blocks (within useAIChat/buildAIChatRequest) and on error log the failure and continue by leaving referenceContext undefined/null and imageAssets as an empty array (or previous default) instead of throwing; reference the getRobotServiceOptions() checks, the search(content) call that assigns referenceContext, and the fetchAssets(appId) assignment to imageAssets (using getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id to compute appId) when applying the change.
619-628:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the AI response shape before reading
choices[0].A provider error payload or empty response will throw on
chatResponse.choices[0].message.content, and the caller currently has no recovery path from that exception.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 619 - 628, The function applyAIPatches currently reads chatResponse.choices[0].message.content without validating the response shape; add a defensive check before accessing choices to ensure chatResponse is an object, chatResponse.choices exists and is a non-empty array, and choices[0].message and choices[0].message.content are present (e.g. if (!chatResponse || !Array.isArray(chatResponse.choices) || chatResponse.choices.length === 0 || !chatResponse.choices[0].message || typeof chatResponse.choices[0].message.content !== 'string') return false), and only then call parseJsonPatches(content); this prevents throws on provider error payloads and gives the caller a consistent false/early-return recovery path in applyAIPatches.
501-502:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard both
nullandundefinedingetValueBySegments().Once a lookup returns
undefined, the next reduce step still satisfieso !== nulland throws ono[key], which makes add-patch application silently stop.Suggested fix
- return segments.reduce((o, key) => (o !== null ? o[key] : undefined), obj) + return segments.reduce((o, key) => (o != null ? o[key] : undefined), obj)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 501 - 502, getValueBySegments currently only checks for null which lets an undefined intermediate value cause a TypeError on the next property access; update the reducer in getValueBySegments to guard both null and undefined (e.g., check o != null or o !== null && o !== undefined) so that when an intermediate lookup yields undefined the reducer returns undefined rather than attempting o[key], ensuring safe traversal of segments in getValueBySegments.
95-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't let
closehide a node with pending AI changes.Forcing
state: 'hidden'here lets the toolbar dismiss aconfirmstate without accept/reject, so the save blocker stops seeing a schema that is still AI-modified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 95 - 99, closeNodeAIHelper currently forces state: 'hidden' which can dismiss a 'confirm' state with pending AI edits; instead, read the node's current AI status and only set state to 'hidden' when there are no pending AI changes (i.e., state is not 'confirm' or there is no outstanding AI modification), otherwise call updateNodeAIStatus(nodeId, { lastAIAction: 'close' }) but preserve the existing state so the save blocker still sees the AI-modified schema; update the logic inside closeNodeAIHelper to check the current status and conditionally include state: 'hidden' only when safe.packages/canvas/DesignCanvas/src/api/useCanvas.ts (3)
273-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrune orphaned
nodesStatusentries when replacing the schema.After
nodesMapis rebuilt, deleted node IDs stay inpageState.nodesStatus. A removed node left inconfirmwill still be reported as pending and can block save even though it no longer exists on the canvas.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts` around lines 273 - 283, When rebuilding nodesMap (nodesMap.value.clear(), nodesMap.value.set(...), generateNodesMap(...)) you must also prune pageState.nodesStatus of IDs that no longer exist to avoid orphaned statuses blocking save; after reconstructing nodesMap, compute the set of current node IDs from nodesMap.value (or from newPageSchema children via generateNodesMap) and remove any entries in pageState.nodesStatus whose keys are not in that set before initializing AI status with initializeNodeAIStatus; update the logic around nodesMap rebuild to perform this cleanup so only existing nodes remain in pageState.nodesStatus.
89-96:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winType
nodeasNodehere.
node.idis accessed repeatedly, butobjectdoes not expose that property under strict TypeScript, so this still won't type-check.Suggested fix
-const initializeNodeAIStatus = (node: object, initialStatus: Partial<NodeAIStatus> = {}) => { +const initializeNodeAIStatus = (node: Node, initialStatus: Partial<NodeAIStatus> = {}) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts` around lines 89 - 96, The function initializeNodeAIStatus currently types its parameter as object which prevents accessing node.id; change the parameter signature to (node: Node, initialStatus: Partial<NodeAIStatus> = {}) in initializeNodeAIStatus and import or reference the appropriate Node type so TypeScript recognizes node.id; update any related usages/calls if necessary to satisfy the new Node type.
105-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInitialize AI state from
nodesMap, not onlypageSchema.children.
generateNodesMap()also registers descendants stored under props trees, but this helper only walkspageState.pageSchema.children. Those nodes never getoriginalNodeData, so cancel/refresh cannot restore them after AI edits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts` around lines 105 - 122, The helper initializeAllNodesAIStatus currently only traverses pageState.pageSchema.children so it misses descendant nodes registered in the nodesMap (created by generateNodesMap) that live inside props trees; update initializeAllNodesAIStatus to iterate over pageState.nodesMap (or the generateNodesMap output) in addition to traversing pageSchema.children and call initializeNodeAIStatus for any node.id that lacks pageState.nodesStatus[node.id]?.aiStatus, ensuring nodes registered under props are also given originalNodeData and AI state for cancel/refresh to work; keep the existing traverseNodes logic but add a pass over nodesMap entries (or merge nodes from nodesMap) to cover all nodes.packages/canvas/container/src/components/CanvasAction.vue (1)
773-789:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNon-abort failures still leave the node stuck in
loading.In
handleAIChatComplete, non-abort chat errors fall through toapplyAIPatches(undefined, ...), and both handlers ignore afalsereturn fromapplyAIPatches(). Those paths never callcancelNodeAILoading(), so the loading dialog can remain indefinitely.Also applies to: 849-863
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/components/CanvasAction.vue` around lines 773 - 789, The code lets non-abort chat failures and failed applyAIPatches leave the node stuck in loading; update handleAIChatComplete to call cancelNodeAILoading() on any non-AbortError catch branch (when chat(...) throws) and then return, and after calling applyAIPatches(currentSchema.id, response, content) check its boolean result and call cancelNodeAILoading() and return if it returned false; use the existing symbols chat, aiChatAbortController, getNodeAIStatus, applyAIPatches, and cancelNodeAILoading to implement these early-exit cleanup paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/canvas/container/src/components/CanvasAction.vue`:
- Around line 252-253: Global aiChatAbortController causes cross-node
interference; replace it with a per-node controller (e.g., attach an
AbortController to the node object or store it in the node's ai state map keyed
by node id) and update all AI request start/cancel handlers to use that per-node
controller instead of the shared aiChatAbortController; when starting a request
create and save a new controller for that node, when cancelling use the node's
controller, and ensure response handlers verify the node's current controller or
node id before applying results so late responses for previous controllers are
ignored.
In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts`:
- Around line 463-486: The inserted root handling misses descendants embedded
under the node's props (e.g., slot schema) so nodes reachable via
newNodeData.props never get added to nodesMap or initialized; after
setNode(newNodeData, parentNode) and when newNodeData.id exists, traverse
newNodeData.props (and any known prop keys that can contain node trees, e.g.,
slots/schema) to collect descendant node objects, call generateNodesMap on those
prop-derived child arrays (using getNode(newNodeData.id) as parent where
appropriate), and run initializeNodeAIStatus for each discovered descendant
(recursively) in addition to the existing recursion over newNodeData.children so
that prop-based descendants are included in nodesMap and nodesStatus.
---
Duplicate comments:
In `@packages/canvas/container/src/components/CanvasAction.vue`:
- Around line 773-789: The code lets non-abort chat failures and failed
applyAIPatches leave the node stuck in loading; update handleAIChatComplete to
call cancelNodeAILoading() on any non-AbortError catch branch (when chat(...)
throws) and then return, and after calling applyAIPatches(currentSchema.id,
response, content) check its boolean result and call cancelNodeAILoading() and
return if it returned false; use the existing symbols chat,
aiChatAbortController, getNodeAIStatus, applyAIPatches, and cancelNodeAILoading
to implement these early-exit cleanup paths.
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 674-680: The optional RAG/resource lookups (search and
fetchAssets) must fail open so they don't abort buildAIChatRequest; wrap the
calls to search(content) and fetchAssets(appId) in try/catch blocks (within
useAIChat/buildAIChatRequest) and on error log the failure and continue by
leaving referenceContext undefined/null and imageAssets as an empty array (or
previous default) instead of throwing; reference the getRobotServiceOptions()
checks, the search(content) call that assigns referenceContext, and the
fetchAssets(appId) assignment to imageAssets (using
getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id to compute appId) when
applying the change.
- Around line 619-628: The function applyAIPatches currently reads
chatResponse.choices[0].message.content without validating the response shape;
add a defensive check before accessing choices to ensure chatResponse is an
object, chatResponse.choices exists and is a non-empty array, and
choices[0].message and choices[0].message.content are present (e.g. if
(!chatResponse || !Array.isArray(chatResponse.choices) ||
chatResponse.choices.length === 0 || !chatResponse.choices[0].message || typeof
chatResponse.choices[0].message.content !== 'string') return false), and only
then call parseJsonPatches(content); this prevents throws on provider error
payloads and gives the caller a consistent false/early-return recovery path in
applyAIPatches.
- Around line 501-502: getValueBySegments currently only checks for null which
lets an undefined intermediate value cause a TypeError on the next property
access; update the reducer in getValueBySegments to guard both null and
undefined (e.g., check o != null or o !== null && o !== undefined) so that when
an intermediate lookup yields undefined the reducer returns undefined rather
than attempting o[key], ensuring safe traversal of segments in
getValueBySegments.
- Around line 95-99: closeNodeAIHelper currently forces state: 'hidden' which
can dismiss a 'confirm' state with pending AI edits; instead, read the node's
current AI status and only set state to 'hidden' when there are no pending AI
changes (i.e., state is not 'confirm' or there is no outstanding AI
modification), otherwise call updateNodeAIStatus(nodeId, { lastAIAction: 'close'
}) but preserve the existing state so the save blocker still sees the
AI-modified schema; update the logic inside closeNodeAIHelper to check the
current status and conditionally include state: 'hidden' only when safe.
In `@packages/canvas/DesignCanvas/src/api/useCanvas.ts`:
- Around line 273-283: When rebuilding nodesMap (nodesMap.value.clear(),
nodesMap.value.set(...), generateNodesMap(...)) you must also prune
pageState.nodesStatus of IDs that no longer exist to avoid orphaned statuses
blocking save; after reconstructing nodesMap, compute the set of current node
IDs from nodesMap.value (or from newPageSchema children via generateNodesMap)
and remove any entries in pageState.nodesStatus whose keys are not in that set
before initializing AI status with initializeNodeAIStatus; update the logic
around nodesMap rebuild to perform this cleanup so only existing nodes remain in
pageState.nodesStatus.
- Around line 89-96: The function initializeNodeAIStatus currently types its
parameter as object which prevents accessing node.id; change the parameter
signature to (node: Node, initialStatus: Partial<NodeAIStatus> = {}) in
initializeNodeAIStatus and import or reference the appropriate Node type so
TypeScript recognizes node.id; update any related usages/calls if necessary to
satisfy the new Node type.
- Around line 105-122: The helper initializeAllNodesAIStatus currently only
traverses pageState.pageSchema.children so it misses descendant nodes registered
in the nodesMap (created by generateNodesMap) that live inside props trees;
update initializeAllNodesAIStatus to iterate over pageState.nodesMap (or the
generateNodesMap output) in addition to traversing pageSchema.children and call
initializeNodeAIStatus for any node.id that lacks
pageState.nodesStatus[node.id]?.aiStatus, ensuring nodes registered under props
are also given originalNodeData and AI state for cancel/refresh to work; keep
the existing traverseNodes logic but add a pass over nodesMap entries (or merge
nodes from nodesMap) to cover all nodes.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 118416dd-618a-4487-8a83-fefb9dfd4589
📒 Files selected for processing (5)
packages/canvas/DesignCanvas/src/api/useCanvas.tspackages/canvas/container/src/components/AILoadingDialog.vuepackages/canvas/container/src/components/CanvasAction.vuepackages/canvas/container/src/composables/useAIChat.tspackages/canvas/container/src/services/agentServices.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/canvas/container/src/components/AILoadingDialog.vue
- packages/canvas/container/src/services/agentServices.ts
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/canvas/container/src/composables/useAIChat.ts (4)
508-510:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard segment walker against
undefinedintermediates.The reducer only checks
null; once an intermediate lookup returnsundefined, the next access can throw and silently break add-patch handling.Suggested fix
const getValueBySegments = (obj: any, segments: string[]): any => { - return segments.reduce((o, key) => (o !== null ? o[key] : undefined), obj) + return segments.reduce((o, key) => (o != null ? o[key] : undefined), obj) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 508 - 510, The segment walker getValueBySegments currently only checks for null and will try to access properties on undefined intermediates; update the reducer in getValueBySegments to guard against both null and undefined (e.g., use a != null check or explicit typeof check) so each step returns undefined if the current object is nullish, preventing throws and preserving add-patch handling.
634-636:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate AI response shape before dereferencing content.
chatResponse.choices[0].message.contentcan throw when upstream response format changes or partial failures occur.Suggested fix
- const content = chatResponse.choices[0].message.content + const content = chatResponse?.choices?.[0]?.message?.content + if (typeof content !== 'string' || !content.trim()) { + return false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 634 - 636, The code dereferences chatResponse.choices[0].message.content without validating the response shape, which can throw on unexpected upstream changes; update the logic in useAIChat (around the variables chatResponse, content, and validJsonPatches) to first verify that chatResponse?.choices is an array with at least one element and that choices[0]?.message?.content exists and is a string (or otherwise handle non-string content) before assigning content or calling parseJsonPatches; if the shape is invalid, log a clear error/warning and return/abort gracefully instead of dereferencing, so parseJsonPatches is only invoked with a safe string value.
96-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep pending confirm state from being hidden by close action.
Closing currently forces
state: 'hidden', which can bypass pending-modification detection that only checksstate === 'confirm'.Suggested fix
const closeNodeAIHelper = (nodeId: string) => { + if (hasNodePendingAIModification(nodeId)) { + return + } + updateNodeAIStatus(nodeId, { state: 'hidden', lastAIAction: 'close' })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 96 - 100, closeNodeAIHelper currently force-sets state:'hidden', which hides nodes in a pending 'confirm' state; change closeNodeAIHelper to first read the node's current AI status (e.g., via getNodeAIStatus(nodeId) or the node AI store getter) and if current.state === 'confirm' only set lastAIAction:'close' while leaving state unchanged, otherwise set state:'hidden' and lastAIAction:'close'; if updateNodeAIStatus supports an updater callback, use that to atomically inspect previous state and return the appropriate patched object.
682-688:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOptional context enrichment should fail open.
Failures in
search()/fetchAssets()currently abort request construction, even though these are optional enrichments.Suggested fix
if (getRobotServiceOptions()?.enableRagContext) { - referenceContext = await search(content) + try { + referenceContext = await search(content) + } catch { + referenceContext = '' + } } if (getRobotServiceOptions()?.enableResourceContext) { const appId = getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id - imageAssets = await fetchAssets(appId) + try { + imageAssets = await fetchAssets(appId) + } catch { + imageAssets = [] + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/composables/useAIChat.ts` around lines 682 - 688, Wrap the optional enrichment calls so failures do not abort request construction: when getRobotServiceOptions()?.enableRagContext is true, call search(content) inside a try/catch and on error log (or swallow) and leave referenceContext as its default (e.g., empty array/null) instead of throwing; similarly, when getRobotServiceOptions()?.enableResourceContext is true, call fetchAssets(appId) inside a try/catch (appId from getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id) and on error log (or swallow) and keep imageAssets as its default. Ensure you reference the existing symbols referenceContext, imageAssets, search, fetchAssets, and getMetaApi(...) so the code continues building the request even if these optional calls fail.
🧹 Nitpick comments (1)
packages/canvas/container/src/components/CanvasAIChat.vue (1)
28-28: ⚡ Quick win
closeis declared but never emitted in this component.Either emit
closefrom a concrete UI action inCanvasAIChat.vue, or removeclosefrom this component’s event contract to avoid a dead listener path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/canvas/container/src/components/CanvasAIChat.vue` at line 28, The component declares emits: ['complete', 'close'] but never actually emits 'close'; either remove 'close' from the emits array in CanvasAIChat.vue to avoid a dead listener, or implement and call an emission where the UI closes (e.g., add a method like handleClose() or onClose() and call this.$emit('close') or emit('close') from the close-button click handler or modal close hook), ensuring the emit is tied to the concrete close UI action so the declared event is actually fired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 711-741: This change touches runtime TypeScript under the package
that exports the default function returning AI helpers (symbols include export
default function, buildAIChatRequest, applyAIPatches, etc.), but the PR lacks
package-level verification; before merging, run the affected package's local
test script (npm/pnpm run test if present) and run the package build step (pnpm
run build:plugin or the package's published-build script) to ensure no
regressions, and add these verification steps to the PR checklist or CI for
packages/** changes so future PRs include package-local test/build verification.
- Around line 640-642: findJsonPatchPath can return null so building patch paths
with parentPath may produce invalid paths; before calling applyPatchesToSchema
(where you call findJsonPatchPath(getPageSchema(), nodeId) and then
applyPatchesToSchema(validJsonPatches, getPageSchema(), parentPath)) check if
parentPath is null/undefined and handle it (e.g., log/warn and return the
original schema or throw a clear error, or skip/adjust validJsonPatches to have
safe paths) so you never concatenate patches with a null parentPath and corrupt
the schema; update the logic in the block that computes parentPath and calls
applyPatchesToSchema (references: findJsonPatchPath, applyPatchesToSchema,
getPageSchema, validJsonPatches, nodeId, parentPath).
---
Duplicate comments:
In `@packages/canvas/container/src/composables/useAIChat.ts`:
- Around line 508-510: The segment walker getValueBySegments currently only
checks for null and will try to access properties on undefined intermediates;
update the reducer in getValueBySegments to guard against both null and
undefined (e.g., use a != null check or explicit typeof check) so each step
returns undefined if the current object is nullish, preventing throws and
preserving add-patch handling.
- Around line 634-636: The code dereferences
chatResponse.choices[0].message.content without validating the response shape,
which can throw on unexpected upstream changes; update the logic in useAIChat
(around the variables chatResponse, content, and validJsonPatches) to first
verify that chatResponse?.choices is an array with at least one element and that
choices[0]?.message?.content exists and is a string (or otherwise handle
non-string content) before assigning content or calling parseJsonPatches; if the
shape is invalid, log a clear error/warning and return/abort gracefully instead
of dereferencing, so parseJsonPatches is only invoked with a safe string value.
- Around line 96-100: closeNodeAIHelper currently force-sets state:'hidden',
which hides nodes in a pending 'confirm' state; change closeNodeAIHelper to
first read the node's current AI status (e.g., via getNodeAIStatus(nodeId) or
the node AI store getter) and if current.state === 'confirm' only set
lastAIAction:'close' while leaving state unchanged, otherwise set state:'hidden'
and lastAIAction:'close'; if updateNodeAIStatus supports an updater callback,
use that to atomically inspect previous state and return the appropriate patched
object.
- Around line 682-688: Wrap the optional enrichment calls so failures do not
abort request construction: when getRobotServiceOptions()?.enableRagContext is
true, call search(content) inside a try/catch and on error log (or swallow) and
leave referenceContext as its default (e.g., empty array/null) instead of
throwing; similarly, when getRobotServiceOptions()?.enableResourceContext is
true, call fetchAssets(appId) inside a try/catch (appId from
getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id) and on error log (or
swallow) and keep imageAssets as its default. Ensure you reference the existing
symbols referenceContext, imageAssets, search, fetchAssets, and getMetaApi(...)
so the code continues building the request even if these optional calls fail.
---
Nitpick comments:
In `@packages/canvas/container/src/components/CanvasAIChat.vue`:
- Line 28: The component declares emits: ['complete', 'close'] but never
actually emits 'close'; either remove 'close' from the emits array in
CanvasAIChat.vue to avoid a dead listener, or implement and call an emission
where the UI closes (e.g., add a method like handleClose() or onClose() and call
this.$emit('close') or emit('close') from the close-button click handler or
modal close hook), ensuring the emit is tied to the concrete close UI action so
the declared event is actually fired.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7ec3df5e-aa71-4578-b2dd-094db2d8b3bd
📒 Files selected for processing (5)
packages/canvas/DesignCanvas/src/api/types.tspackages/canvas/container/src/components/AILoadingDialog.vuepackages/canvas/container/src/components/CanvasAIChat.vuepackages/canvas/container/src/components/CanvasAction.vuepackages/canvas/container/src/composables/useAIChat.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/canvas/container/src/components/AILoadingDialog.vue
- packages/canvas/container/src/components/CanvasAction.vue
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
reviewed
Apr 30, 2026
chilingling
approved these changes
Apr 30, 2026
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
3 participants
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.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Save / Validation
Bug Fixes / UX