[shaohuzhang1]

fix: [AI-Generated Earnings] The AI-generated earnings feature modifies earnings directly, without first presenting the proposed adjustments to the user for their review and confirmation.

[shaohuzhang1]

Review

Correctness / potential bugs

1. Early return may skip cleanup code in run()

   if (result.status === 'done') {
     status.value = 'done'
     if (!canvasChanged) return   // <-- leaves the function entirely
   }

   Anything that follows in run() (agent disposal, clearDebugLog, status reset, counters, finally-less cleanup, logging) will now be skipped on the "planning only" path. Prefer disabling only the block instead of returning:

   if (result.status === 'done') {
     status.value = 'done'
     if (canvasChanged) {
       await layoutCanvas(lf)
       lf.fitView()
       MsgSuccess('工作流已生成，请检查后保存')
     }
   }

   If you keep the return, make sure run() has no post-processing and ideally wrap the body in try/finally.

2. event.position.length can throw if position is optional
   !event.position.length blows up on undefined. Use !(event.position?.length) (or !event.position?.length) unless the type guarantees a non-nullable array. Also consider a small helper (isTopLevelEvent(event)) instead of the inline magic check, since the semantics ("depth 0 = top-level agent") is not obvious.

3. Concurrency / re-entrancy of canvasChanged
   It is a single component-scoped flag mutated from onEvent. If run() can be triggered again while a previous run is still in flight (double click, retry button, queued prompts), the second run resets the flag and the first run's result will read stale state. Either guard with an isRunning check / abort previous run, or make it a local value returned by the run session (e.g. pass a small session object into the agent, or track Set<toolName> in a per-run ref).

4. Flag is set on tool_start, not on success
   A failing add_node / delete_edge etc. still sets canvasChanged = true, so you call layoutCanvas + fitView and show "工作流已生成" even when nothing was actually mutated. Consider setting the flag on the corresponding successful tool_end event instead (or clear it when the tool errors).

5. Nested agents are ignored
   Because of !event.position.length, tool calls from sub-agents never set the flag. If a sub-agent can mutate the canvas, the success message/layout will be skipped incorrectly. Confirm this is intentional and document it.

6. Silent path for the user
   When !canvasChanged you return without any feedback. The comment explains why, but from the UI the run just "stops" with status = 'done'. Consider a neutral MsgInfo/inline hint (e.g. "方案已生成，等待确认") so the user knows the run finished.

7. provide placement and semantics

   • provide(VIEW_KEY, VIEW_TYPE.DEBUG) at the top of <script setup> is technically valid (synchronous, before any await), but it sits between the imports and defineOptions, which reads like a stray side effect. Move it next to the other setup logic (after defineOptions / useRoute / inject) or add a comment explaining the override.
   • It silently overrides VIEW_KEY for all descendants of this panel. If a child already provides/injects its own view type, this changes behavior implicitly. If it's only meant for a specific child, prefer a prop or provide at a narrower wrapper.
   • The provided value is a static constant; if anything downstream should react to it changing, wrap it in ref/computed.

8. Possible circular import
   @/conversation-panel/view is imported into a workflow panel. If that module ever imports from the workflow panel tree, you get a cycle at module-evaluation time. If it only exports the key/enum, consider moving them to a leaf constants/keys module.

Style / consistency

• import { computed, inject, nextTick, onBeforeUnmount, ref, useTemplateRef ,provide } from 'vue' — extra space before the comma, and provide breaks alphabetical ordering used elsewhere. Should be:

  import { computed, inject, nextTick, onBeforeUnmount, provide, ref, useTemplateRef } from 'vue'

• import { VIEW_KEY,VIEW_TYPE } — missing space after the comma (inconsistent with the rest of the file).
• CANVAS_MUTATING_TOOLS is defined inside <script setup>, so a new Set is allocated per component instance. Hoist it to a plain <script lang="ts"> block or a shared constants module (that also makes it unit-testable):

  // <script lang="ts"> (module scope)
  export const CANVAS_MUTATING_TOOLS: ReadonlySet<string> = new Set([...])

  Whether layout belongs in this list is worth double-checking — if the layout tool already lays out the canvas, the subsequent layoutCanvas(lf) is redundant.

Optimization / maintainability

• Prefer the if (canvasChanged) { ... } form (point 1) — clearer intent than an early return and future-proof against added code.
• Consider resetting the flag in a finally of run() so a rejected agent.run() (which currently leaves status = 'running') doesn't leave stale state behind.
• Add a short type for the tool names (e.g. type CanvasTool = 'add_node' | ...) so a typo in the string literal becomes a compile error rather than a silently missed flag.
• The comment on CANVAS_MUTATING_TOOLS is good; mirror it at the if (!canvasChanged) branch so future readers don't "fix" it into an unconditional layout.

[shaohuzhang1]

Here is the analysis of the provided code diff:

1. Irregularities & Potential Issues

• Removal of Key (Functional Regression): The most critical issue is the potential removal of a valid functionality. If submit_plan was actively used elsewhere in the codebase (e.g., in tool definitions, UI components, or execution logic), removing its label here does not remove the tool itself, but may result in missing UI text (fallback to raw key) or undefined behavior if strict typing relies on all keys being present.
• Unused Import/Definition Warning: If no code references TOOL_LABELS['submit_plan'] anymore, this change is correct cleanup. However, static analysis tools might flag the original definition as unused before this diff if it was already orphaned.

2. Optimization Suggestions

• No Performance Issue: Removing a single entry from a static Record<string, string> map has negligible impact on performance or bundle size.
• Recommendation: Ensure that the corresponding tool implementation for submit_plan is also removed or deprecated consistently to avoid mismatch between available tools and their labels/tooltips.