Enhance targeted node editing functionality in Copilot#2132
Enhance targeted node editing functionality in Copilot#2132
Conversation
- Introduced NodeEditContext to manage state for targeted node editing, capturing relevant information for user-selected nodes. - Implemented useNodeEditContext hook to synchronize selected node context with the Copilot store. - Added NodeEditBanner component to display editing context and options for modifying or extending nodes. - Updated various components to utilize the new node editing context, ensuring consistent use of optional chaining and nullish coalescing. - Enhanced workflow patch operations to support targeted editing, allowing for incremental updates to workflow plans. This update improves user experience by providing clear context and options for editing nodes directly within the Copilot interface.
📝 WalkthroughWalkthroughThis PR introduces targeted node editing capability for the Copilot by adding NodeEditContext propagation through the request pipeline, implementing incremental patch operations for workflow plans, and creating UI components for selecting edit modes (modify or extend). Changes span backend services, frontend components, state management, and type definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Copilot UI
participant Store as Copilot Store
participant Backend as Backend API
participant Agent as AI Agent
participant Canvas as Canvas Component
User->>UI: Select single skillResponse node
UI->>Store: useNodeEditContext builds context<br/>(nodeId, taskId, graphContext)
Store->>Store: Store nodeEditContext by canvasId
UI->>UI: NodeEditBanner renders with<br/>modify/extend buttons
User->>UI: Select edit mode (modify/extend)
Store->>Store: setNodeEditMode updates editMode
UI->>UI: ChatBox captures nodeEditContext<br/>from store
User->>UI: Send message to Copilot
UI->>Backend: InvokeSkillRequest with<br/>nodeEditContext
Backend->>Agent: SkillInvokerService passes<br/>nodeEditContext in config
Agent->>Agent: buildWorkflowCopilotPrompt<br/>injects context + mode instructions
Agent->>Agent: Generate patch operations<br/>(patch_workflow)
Agent->>Backend: Return patch operations
Backend->>Canvas: WorkflowPlanService parses<br/>patchOperations
Canvas->>Canvas: copilot-message detects<br/>isPatchOperation
Canvas->>Canvas: applyIncrementalChangesToCanvas<br/>applies targeted edits
Canvas->>Canvas: Update nodes/edges/variables
Canvas->>UI: Render patched canvas
UI->>Store: Clear nodeEditContext on complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/agent-tools/src/copilot/index.ts (1)
168-182: Use validated parsed data instead of unchecked input with type assertions.After validating with
parseWorkflowPlanPatchat line 125, use the parsed result (parsed.data?.operations) instead of the originalinput.operations. This ensures you're always working with validated data and removes the unnecessary type assertion. The nullish coalescing provides defensive safety.🛠️ Suggested change
const result = await reflyService.patchWorkflowPlan(user, { planId: planId!, - operations: input.operations as WorkflowPatchOperation[], + operations: parsed.data?.operations ?? [], resultId, resultVersion, }); return { status: 'success', data: { planId: result.planId, version: result.version, }, - summary: `Successfully patched workflow plan with ID: ${result.planId} and created new version: ${result.version}. Applied ${input.operations.length} operation(s).`, + summary: `Successfully patched workflow plan with ID: ${result.planId} and created new version: ${result.version}. Applied ${(parsed.data?.operations ?? []).length} operation(s).`, };Per coding guidelines, prefer validated parsed data and nullish coalescing over direct access with type assertions.
packages/ai-workspace-common/src/components/canvas/copilot/chat-box.tsx (1)
43-54: Use optional chaining and nullish coalescing for safe property access on map lookups.Line 53 should use optional chaining when accessing the
nodeEditContextmap and normalize tonullfor consistency with how the value is used throughout the component.🛠️ Suggested change
- nodeEditContext: state.nodeEditContext[canvasId], + nodeEditContext: state.nodeEditContext?.[canvasId] ?? null,This follows the coding guidelines for optional chaining and ensures stable values when working with map-like store properties.
🤖 Fix all issues with AI agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx`:
- Around line 174-195: The code extracts variableOperations from
applyIncrementalChangesToCanvas but then ignores them and blindly calls
setVariables(finalPlan.variables ?? []), which can clear variables; instead,
when variableOperations?.length exists you must apply those operations to the
current variables and pass the resulting list to setVariables. Locate where
variable state is stored (e.g., currentVariables from state or a variablesRef),
implement or call a utility to apply create/update/delete operations (e.g.,
applyVariableOperations(currentVariables, variableOperations)), compute
newVariables, then call setVariables(newVariables, { archiveOldFiles: false })
rather than using finalPlan.variables; keep existing behavior when no
variableOperations are present.
In
`@packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx`:
- Around line 18-46: Replace the fallback using logical OR with nullish
coalescing for the node title: update the displayTitle assignment to use
currentState.title ?? t('copilot.nodeEdit.untitled') (referencing displayTitle
and currentState.title) so empty strings are preserved; do not add any extra
guards around canvasId from useCanvasContext() because that hook already
guarantees a string.
In `@packages/canvas-common/src/workflow-plan.ts`:
- Around line 433-458: planVariableToWorkflowVariable currently casts incoming
strings to union types allowing invalid values through; update it to
clamp/normalize incoming enums by explicitly mapping/checking
planVariable.variableType to one of 'string'|'option'|'resource' (default
'string' if not valid), map each value.type to 'text'|'resource' (default
'text'), and map resource.fileType to 'document'|'image'|'video'|'audio'
(default or omit if invalid). Apply the same normalization logic in the
corresponding updateVariable conversion (lines ~911-931) so both creation and
updates only produce allowed enum values in the WorkflowVariable payloads.
In `@packages/openapi-schema/schema.yml`:
- Around line 12873-12917: WorkflowPatchData currently lacks support for
option-type variables so patch requests cannot change option variables; update
the WorkflowPatchData schema (the object under WorkflowPatchData in the diff) to
include "option" in the variableType enum (matching
WorkflowVariable.variableType), and add properties to carry option configuration
such as an options array (items referencing the existing or new
VariableOption/VariableValue schema) and a selectionMode field (enum like
"single"/"multiple" or referencing VariableSelectionMode) so patch operations
can update option lists and selection behavior; ensure property names match the
runtime model (e.g., variableType, options, selectionMode) and reference
existing component schemas where appropriate.
In `@packages/openapi-schema/src/types.gen.ts`:
- Around line 8078-8134: The WorkflowPatchData type's variableType union is
missing the 'option' member, causing patches to reject valid updates; update the
OpenAPI schema definition that describes WorkflowPatchData (and/or the
referenced WorkflowVariable.variableType enum) to include 'option' in the
allowed values, then regenerate the TypeScript types so
WorkflowPatchData.variableType becomes 'string' | 'resource' | 'option' and
consumers can PATCH option-typed variables without casting.
In `@packages/request/src/requests/types.gen.ts`:
- Around line 8089-8110: The WorkflowPatchData type's variableType union is
missing 'option' causing inconsistency with WorkflowVariable.variableType;
update the schema so WorkflowPatchData.variableType includes 'option' (i.e.,
change the union for variableType in WorkflowPatchData to 'string' | 'option' |
'resource') and then regenerate the OpenAPI-generated types so the updated union
appears in packages/request/src/requests/types.gen.ts; target the
WorkflowPatchData and ensure it matches WorkflowVariable.variableType.
In `@packages/stores/src/stores/copilot.ts`:
- Around line 139-160: The setNodeEditMode updater should use optional chaining
when reading state.nodeEditContext[canvasId] to match the pattern used
elsewhere; in the setNodeEditMode function replace the direct lookup into
state.nodeEditContext with an optional-chained access (e.g., const
currentContext = state.nodeEditContext?.[canvasId]) and keep the early return if
no context, while leaving setNodeEditContext's spread logic as-is since
state.nodeEditContext is always an object.
🧹 Nitpick comments (14)
packages/stores/src/stores/copilot.ts (2)
1-1: Use type-only import for OpenAPI types.Line 1 only brings in types, so switch to
import type(and keep type imports in their own group) to avoid runtime dependencies.♻️ Suggested change
-import { CopilotSession, CanvasNodeType } from '@refly/openapi-schema'; +import type { CopilotSession, CanvasNodeType } from '@refly/openapi-schema';As per coding guidelines, use type-only imports and separate type import groups.
6-33: Avoid duplicating NodeEditContext; reuse the shared OpenAPI type.The OpenAPI schema already defines
NodeEditContext(with optionalcurrentState/graphContext). Duplicating it here risks drift and forces stricter requirements than the API contract. Consider aliasing or extending the shared type instead.♻️ Suggested change
-import type { CopilotSession, CanvasNodeType } from '@refly/openapi-schema'; +import type { + CopilotSession, + NodeEditContext as OpenApiNodeEditContext, +} from '@refly/openapi-schema'; -export interface NodeEditContext { - /** The internal node ID from ReactFlow */ - nodeId: string; - ... - /** The edit mode: modify the current node or extend from it */ - editMode: 'modify' | 'extend'; -} +export type NodeEditContext = OpenApiNodeEditContext;As per coding guidelines, prefer extending existing types over creating new ones.
packages/agent-tools/src/copilot/index.ts (1)
11-11: Use type-only import for OpenAPI types.Line 11 imports types only; switch to
import typeand keep type imports grouped separately.♻️ Suggested change
-import { User, WorkflowPlanRecord, WorkflowPatchOperation } from '@refly/openapi-schema'; +import type { User, WorkflowPlanRecord, WorkflowPatchOperation } from '@refly/openapi-schema';As per coding guidelines, use type-only imports and separate type import groups.
packages/ai-workspace-common/src/hooks/canvas/use-invoke-action.ts (1)
5-20: Preferimport typefor schema-only types.This avoids runtime imports and aligns with TS type-only import guidance. If any of these are runtime values, keep them as value imports.
♻️ Suggested change
-import { +import type { ActionMessage, ActionResult, ActionStatus, ActionStep, ActionStepMeta, AgentMode, Artifact, CodeArtifactType, Entity, GenericToolset, InvokeSkillRequest, ModelInfo, NodeEditContext, SkillEvent, } from '@refly/openapi-schema';packages/skill-template/src/prompts/copilot-agent.ts (1)
1-1: Guard nodeEditContext field access and avoid magic numbers in the prompt.Direct dereferences can throw if the payload is partial at runtime. Also, hoist the preview length to a named constant for clarity and reuse.
♻️ Suggested change
// Build node edit context section if provided let nodeEditContextSection = ''; if (params.nodeEditContext) { + const QUERY_PREVIEW_LENGTH = 100; + const nodeEditContext = params.nodeEditContext; + const taskId = nodeEditContext?.taskId ?? ''; + const currentTitle = nodeEditContext?.currentState?.title ?? ''; + const currentQuery = nodeEditContext?.currentState?.query ?? ''; + const currentToolsets = nodeEditContext?.currentState?.toolsets ?? []; + const upstreamTasks = nodeEditContext?.graphContext?.upstreamTaskIds ?? []; + nodeEditContextSection = ` ## Targeted Node Editing Mode @@ -| **modify** | \`patch_workflow\` | \`updateTask\` | Update the selected task (taskId: "${params.nodeEditContext.taskId}"); only change what user requests; preserve other fields | -| **extend** | \`patch_workflow\` | \`createTask\` | Create new task with \`dependentTasks: ["${params.nodeEditContext.taskId}"]\` to extend from selected node | +| **modify** | \`patch_workflow\` | \`updateTask\` | Update the selected task (taskId: "${taskId}"); only change what user requests; preserve other fields | +| **extend** | \`patch_workflow\` | \`createTask\` | Create new task with \`dependentTasks: ["${taskId}"]\` to extend from selected node | @@ -- Current state: ${params.nodeEditContext.currentState?.title ? `"${params.nodeEditContext.currentState.title}"` : 'Untitled'} -- Current query/prompt: ${params.nodeEditContext.currentState?.query ? `"${params.nodeEditContext.currentState.query.substring(0, 100)}..."` : 'None'} -- Current toolsets: ${params.nodeEditContext.currentState?.toolsets?.length ? params.nodeEditContext.currentState.toolsets.join(', ') : 'None'} +- Current state: ${currentTitle ? `"${currentTitle}"` : 'Untitled'} +- Current query/prompt: ${currentQuery ? `"${currentQuery.substring(0, QUERY_PREVIEW_LENGTH)}..."` : 'None'} +- Current toolsets: ${currentToolsets.length ? currentToolsets.join(', ') : 'None'} @@ -- Upstream context available: ${params.nodeEditContext.graphContext?.upstreamTaskIds?.join(', ') || 'None (this is a root task)'} +- Upstream context available: ${upstreamTasks.length ? upstreamTasks.join(', ') : 'None (this is a root task)'}Also applies to: 15-18, 25-66
packages/ai-workspace-common/src/hooks/canvas/use-node-edit-context.ts (2)
1-16: Useimport typefor type-only imports and add an explicit hook return type.This aligns with TS guidelines and avoids runtime imports for types.
♻️ Suggested change
-import { CanvasNode, SkillNodeMeta } from '@refly/canvas-common'; -import { useCopilotStoreShallow, NodeEditContext } from '@refly/stores'; +import type { CanvasNode, SkillNodeMeta } from '@refly/canvas-common'; +import { useCopilotStoreShallow } from '@refly/stores'; +import type { NodeEditContext } from '@refly/stores'; @@ -export const useNodeEditContext = () => { +export const useNodeEditContext = (): void => {
22-25: Guard the nodes array before calling.filter.This prevents a potential crash if ReactFlow’s store ever returns
undefinedand satisfies the array-existence guideline.♻️ Suggested change
- const selectedNodes = useStore( - useShallow((state) => state.nodes.filter((node) => node.selected)), - ); + const selectedNodes = useStore( + useShallow((state) => state.nodes?.filter?.((node) => node.selected) ?? []), + );apps/api/src/modules/workflow/workflow-plan.service.ts (1)
4-10: Preferimport typefor schema-only workflow plan types.This keeps runtime imports lean and adheres to TS type-only import guidance. If any of these are runtime values, keep them as value imports.
♻️ Suggested change
-import { +import type { GetWorkflowPlanDetailData, WorkflowPlan, WorkflowPlanRecord, WorkflowPatchOperation, User, } from '@refly/openapi-schema';packages/openapi-schema/src/schemas.gen.ts (1)
11319-11352: Consider op-specific schema variants to enforce required fields.Right now only
opis required, so invalid combinations (e.g.,updateTaskwithouttaskId/data) still validate. Consider aoneOfwithopconst + required fields per operation to tighten the API contract (update inschema.ymland regenerate).packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx (2)
77-104: Avoid inline onClick lambdas for mode buttonsInline lambdas create new handlers each render. Memoize the two button handlers to keep referential stability and align with the performance guidelines. As per coding guidelines, avoid inline function creation in render and use
useCallbackfor function props.♻️ Memoized handlers
+ const handleModifyClick = useCallback(() => { + handleModeChange('modify'); + }, [handleModeChange]); + + const handleExtendClick = useCallback(() => { + handleModeChange('extend'); + }, [handleModeChange]); ... - onClick={() => handleModeChange('modify')} + onClick={handleModifyClick} ... - onClick={() => handleModeChange('extend')} + onClick={handleExtendClick}
17-17: Add JSDoc + explicit return type for the exported componentThe component is exported and should carry a JSDoc comment and an explicit return type (
JSX.Element | null) to align with the TypeScript/JSX guidelines. As per coding guidelines, please add JSDoc for functions and explicit return types.packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx (1)
21-21: Use type-only import forCanvasNode/CanvasEdgeThese are used as types only; switch to
import typeto keep runtime bundles leaner and satisfy TS import guidelines. As per coding guidelines, always import types explicitly usingimport type.♻️ Suggested change
-import { CanvasNode, CanvasEdge } from '@refly/openapi-schema'; +import type { CanvasNode, CanvasEdge } from '@refly/openapi-schema';packages/canvas-common/src/workflow-plan.ts (2)
1-12: Useimport typefor OpenAPI-only symbolsThese are type-only imports, so
import typekeeps the emitted JS clean and follows the project’s TS conventions.♻️ Suggested update
-import { +import type { GenericToolset, RawCanvasData, CanvasNode, WorkflowVariable, WorkflowTask, WorkflowPlan, ModelInfo, VariableValue, WorkflowPatchOperation as OpenApiWorkflowPatchOperation, } from '@refly/openapi-schema';As per coding guidelines.
688-695: Remove unnecessaryanycasts; properties are already typedLines 688–695 and 773–777 use
anyto access properties that are already explicitly typed inCanvasNodeData:
node.datais typed asCanvasNodeData<T>, which definesentityId: string(required) andmetadata?: T(optional)- Direct access without
anycasts preserves type safetySimilarly, the
nodesandedgesparameters already satisfyAddNodeParam's expected types (Node[]andEdge[]) sinceCanvasNodeextendsNode, making theas any[]casts unnecessary.Suggested refactor
- const metadata = (node.data as any)?.metadata; + const metadata = node.data.metadata; const taskId = metadata?.taskId;- taskIdToEntityId.set(taskId, (node.data as any)?.entityId); + taskIdToEntityId.set(taskId, node.data.entityId);- nodes: nodes as any[], - edges: edges as any[], + nodes, + edges,
| // For patch operations with patchOperations available, use incremental changes | ||
| if (isPatchOperation && finalPlan.patchOperations?.length) { | ||
| const currentEdges = getEdges() as CanvasEdge[]; | ||
|
|
||
| const { nodes, edges, affectedNodeIds, variableOperations } = applyIncrementalChangesToCanvas( | ||
| finalPlan.patchOperations, | ||
| currentNodes, | ||
| currentEdges, | ||
| tools?.data ?? [], | ||
| { defaultModel: defaultAgentModel }, | ||
| ); | ||
|
|
||
| setNodes(nodes); | ||
| setEdges(edges); | ||
|
|
||
| // Handle variable operations | ||
| if (variableOperations?.length) { | ||
| // For now, we'll handle this by setting all variables including updates | ||
| // The setVariables function will merge with existing variables | ||
| const existingVariables = finalPlan.variables ?? []; | ||
| setVariables(existingVariables, { archiveOldFiles: false }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "applyIncrementalChangesToCanvas" -g '*.ts' -g '*.tsx' -A 3 -B 1Repository: refly-ai/refly
Length of output: 172
🏁 Script executed:
rg -n -C5 "variableOperations" -g '*.ts' -g '*.tsx'Repository: refly-ai/refly
Length of output: 7454
🏁 Script executed:
rg -n "setVariables" -g '*.ts' -g '*.tsx' -A 2 -B 1Repository: refly-ai/refly
Length of output: 8753
Apply variableOperations instead of ignoring them
The extracted variableOperations are never applied. When variableOperations exist, passing only finalPlan.variables ?? [] bypasses the operations and can lose data. If patch_workflow returns operations with no full variable list, calling setVariables([], {archiveOldFiles: false}) silently clears variables. Apply the operations (create, update, delete) to the current variables or restructure to fetch the complete plan before calling setVariables.
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx`
around lines 174 - 195, The code extracts variableOperations from
applyIncrementalChangesToCanvas but then ignores them and blindly calls
setVariables(finalPlan.variables ?? []), which can clear variables; instead,
when variableOperations?.length exists you must apply those operations to the
current variables and pass the resulting list to setVariables. Locate where
variable state is stored (e.g., currentVariables from state or a variablesRef),
implement or call a utility to apply create/update/delete operations (e.g.,
applyVariableOperations(currentVariables, variableOperations)), compute
newVariables, then call setVariables(newVariables, { archiveOldFiles: false })
rather than using finalPlan.variables; keep existing behavior when no
variableOperations are present.
| const { t } = useTranslation(); | ||
| const { canvasId } = useCanvasContext(); | ||
|
|
||
| const { nodeEditContext, setNodeEditContext, setNodeEditMode } = useCopilotStoreShallow( | ||
| (state) => ({ | ||
| nodeEditContext: state.nodeEditContext[canvasId], | ||
| setNodeEditContext: state.setNodeEditContext, | ||
| setNodeEditMode: state.setNodeEditMode, | ||
| }), | ||
| ); | ||
|
|
||
| const handleClear = useCallback(() => { | ||
| setNodeEditContext(canvasId, null); | ||
| }, [canvasId, setNodeEditContext]); | ||
|
|
||
| const handleModeChange = useCallback( | ||
| (mode: 'modify' | 'extend') => { | ||
| setNodeEditMode(canvasId, mode); | ||
| }, | ||
| [canvasId, setNodeEditMode], | ||
| ); | ||
|
|
||
| if (!nodeEditContext) { | ||
| return null; | ||
| } | ||
|
|
||
| const { currentState, editMode } = nodeEditContext; | ||
| const displayTitle = currentState.title || t('copilot.nodeEdit.untitled'); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the complete file to understand the context
cat -n packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsxRepository: refly-ai/refly
Length of output: 4430
🏁 Script executed:
# Search for the useCanvasContext hook definition
fd -t f "useCanvasContext" packages/ai-workspace-common/src --exec grep -l "export.*useCanvasContext"Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Search for the useCopilotStoreShallow hook definition
fd -t f "copilot" packages/ai-workspace-common/src --type f -name "*store*" | head -10Repository: refly-ai/refly
Length of output: 288
🏁 Script executed:
# Search for useCanvasContext definition
rg -t ts -t tsx "export.*useCanvasContext" --max-count 5Repository: refly-ai/refly
Length of output: 85
🏁 Script executed:
# Search for copilot store files
fd copilot packages/ai-workspace-common/src -type f | head -20Repository: refly-ai/refly
Length of output: 228
🏁 Script executed:
# Also search in the stores package
fd -t f "store" packages | grep -i copilot | head -10Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Look for the stores package
fd -t f "packages/stores" --max-depth 1Repository: refly-ai/refly
Length of output: 423
🏁 Script executed:
# Search for useCanvasContext definition with proper syntax
rg "export.*useCanvasContext" --max-count 5Repository: refly-ai/refly
Length of output: 165
🏁 Script executed:
# Find copilot store files
find packages -name "*copilot*store*" -o -name "*store*copilot*" | head -20Repository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Look for stores directory and files
find packages -type d -name "stores" | head -5Repository: refly-ai/refly
Length of output: 100
🏁 Script executed:
# Search for nodeEditContext in the codebase
rg "nodeEditContext" --max-count 10 -A 2Repository: refly-ai/refly
Length of output: 8727
🏁 Script executed:
# Check useCanvasContext implementation
cat packages/ai-workspace-common/src/context/canvas.tsxRepository: refly-ai/refly
Length of output: 29557
🏁 Script executed:
# Check copilot store implementation
cat packages/stores/src/stores/copilot.tsRepository: refly-ai/refly
Length of output: 6267
🏁 Script executed:
# Search for NodeEditContext type definition
rg "type NodeEditContext|interface NodeEditContext" -A 10Repository: refly-ai/refly
Length of output: 2202
🏁 Script executed:
# Verify if currentState is guaranteed to exist after the guard
grep -A 5 "if (!nodeEditContext)" packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsxRepository: refly-ai/refly
Length of output: 236
🏁 Script executed:
# Check other files using the same pattern to confirm best practices
rg "nodeEditContext\[canvasId\]" --context 5 | head -40Repository: refly-ai/refly
Length of output: 3360
Use nullish coalescing for optional field access
currentState.title is an optional field, so use ?? instead of || to align with coding guidelines. However, canvasId from useCanvasContext() is guaranteed to be a string—the hook throws an error if no context is available, so guards for canvasId are unnecessary.
- const displayTitle = currentState.title || t('copilot.nodeEdit.untitled');
+ const displayTitle = currentState.title ?? t('copilot.nodeEdit.untitled');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { t } = useTranslation(); | |
| const { canvasId } = useCanvasContext(); | |
| const { nodeEditContext, setNodeEditContext, setNodeEditMode } = useCopilotStoreShallow( | |
| (state) => ({ | |
| nodeEditContext: state.nodeEditContext[canvasId], | |
| setNodeEditContext: state.setNodeEditContext, | |
| setNodeEditMode: state.setNodeEditMode, | |
| }), | |
| ); | |
| const handleClear = useCallback(() => { | |
| setNodeEditContext(canvasId, null); | |
| }, [canvasId, setNodeEditContext]); | |
| const handleModeChange = useCallback( | |
| (mode: 'modify' | 'extend') => { | |
| setNodeEditMode(canvasId, mode); | |
| }, | |
| [canvasId, setNodeEditMode], | |
| ); | |
| if (!nodeEditContext) { | |
| return null; | |
| } | |
| const { currentState, editMode } = nodeEditContext; | |
| const displayTitle = currentState.title || t('copilot.nodeEdit.untitled'); | |
| const { t } = useTranslation(); | |
| const { canvasId } = useCanvasContext(); | |
| const { nodeEditContext, setNodeEditContext, setNodeEditMode } = useCopilotStoreShallow( | |
| (state) => ({ | |
| nodeEditContext: state.nodeEditContext[canvasId], | |
| setNodeEditContext: state.setNodeEditContext, | |
| setNodeEditMode: state.setNodeEditMode, | |
| }), | |
| ); | |
| const handleClear = useCallback(() => { | |
| setNodeEditContext(canvasId, null); | |
| }, [canvasId, setNodeEditContext]); | |
| const handleModeChange = useCallback( | |
| (mode: 'modify' | 'extend') => { | |
| setNodeEditMode(canvasId, mode); | |
| }, | |
| [canvasId, setNodeEditMode], | |
| ); | |
| if (!nodeEditContext) { | |
| return null; | |
| } | |
| const { currentState, editMode } = nodeEditContext; | |
| const displayTitle = currentState.title ?? t('copilot.nodeEdit.untitled'); |
🤖 Prompt for AI Agents
In
`@packages/ai-workspace-common/src/components/canvas/copilot/node-edit-banner.tsx`
around lines 18 - 46, Replace the fallback using logical OR with nullish
coalescing for the node title: update the displayTitle assignment to use
currentState.title ?? t('copilot.nodeEdit.untitled') (referencing displayTitle
and currentState.title) so empty strings are preserved; do not add any extra
guards around canvasId from useCanvasContext() because that hook already
guarantees a string.
| export const planVariableToWorkflowVariable = ( | ||
| planVariable: WorkflowVariable, | ||
| ): WorkflowVariable => { | ||
| planVariable: PlanVariableInput, | ||
| ): WorkflowVariable | null => { | ||
| // Ensure required fields are present | ||
| if (!planVariable.variableId || !planVariable.name) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| variableId: planVariable.variableId, | ||
| variableType: planVariable.variableType as 'string' | 'option' | 'resource', | ||
| variableType: (planVariable.variableType as 'string' | 'option' | 'resource') ?? 'string', | ||
| name: planVariable.name, | ||
| value: planVariable.value?.map((value) => ({ | ||
| type: value?.type as 'text' | 'resource', | ||
| text: value?.text, | ||
| // Only include resource if it has the required fields | ||
| ...(value?.resource?.name && value?.resource?.fileType | ||
| ? { | ||
| resource: { | ||
| name: value.resource.name, | ||
| fileType: value.resource.fileType, | ||
| }, | ||
| } | ||
| : {}), | ||
| })), | ||
| description: planVariable.description, | ||
| value: | ||
| planVariable.value?.map((value) => ({ | ||
| type: (value?.type as 'text' | 'resource') ?? 'text', | ||
| text: value?.text, | ||
| // Only include resource if it has the required fields | ||
| ...(value?.resource?.name && value?.resource?.fileType | ||
| ? { | ||
| resource: { | ||
| name: value.resource.name, | ||
| fileType: value.resource.fileType as 'document' | 'image' | 'video' | 'audio', | ||
| }, | ||
| } | ||
| : {}), | ||
| })) ?? [], |
There was a problem hiding this comment.
Clamp loose variable/value types to allowed enums
The current casts let arbitrary strings through (e.g., variableType = 'foo'), which can leak invalid values into WorkflowVariable and patch updates. Normalize to allowed enums before defaulting.
✅ Suggested normalization (apply similarly in updateVariable conversion)
- return {
- variableId: planVariable.variableId,
- variableType: (planVariable.variableType as 'string' | 'option' | 'resource') ?? 'string',
+ const normalizedVariableType =
+ planVariable.variableType === 'string' ||
+ planVariable.variableType === 'option' ||
+ planVariable.variableType === 'resource'
+ ? planVariable.variableType
+ : 'string';
+ return {
+ variableId: planVariable.variableId,
+ variableType: normalizedVariableType,
name: planVariable.name,
value:
planVariable.value?.map((value) => ({
- type: (value?.type as 'text' | 'resource') ?? 'text',
+ type: value?.type === 'resource' ? 'resource' : 'text',
text: value?.text,Also applies to: 911-931
🤖 Prompt for AI Agents
In `@packages/canvas-common/src/workflow-plan.ts` around lines 433 - 458,
planVariableToWorkflowVariable currently casts incoming strings to union types
allowing invalid values through; update it to clamp/normalize incoming enums by
explicitly mapping/checking planVariable.variableType to one of
'string'|'option'|'resource' (default 'string' if not valid), map each
value.type to 'text'|'resource' (default 'text'), and map resource.fileType to
'document'|'image'|'video'|'audio' (default or omit if invalid). Apply the same
normalization logic in the corresponding updateVariable conversion (lines
~911-931) so both creation and updates only produce allowed enum values in the
WorkflowVariable payloads.
| type: object | ||
| description: Data for updating a task or variable | ||
| properties: | ||
| title: | ||
| type: string | ||
| description: New display title for the task | ||
| prompt: | ||
| type: string | ||
| description: New prompt or instruction for this task | ||
| dependentTasks: | ||
| type: array | ||
| items: | ||
| type: string | ||
| description: New list of task IDs that must execute before this task | ||
| toolsets: | ||
| type: array | ||
| items: | ||
| type: string | ||
| description: New list of toolset IDs for this task | ||
| variableType: | ||
| type: string | ||
| enum: | ||
| - string | ||
| - resource | ||
| description: New variable type | ||
| name: | ||
| type: string | ||
| description: New variable name | ||
| description: | ||
| type: string | ||
| description: New variable description | ||
| required: | ||
| type: boolean | ||
| description: Whether this variable is required | ||
| resourceTypes: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/VariableResourceType' | ||
| description: New accepted resource types | ||
| value: | ||
| type: array | ||
| items: | ||
| $ref: '#/components/schemas/VariableValue' | ||
| description: New variable values | ||
| WorkflowPatchOperation: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the schema file to see WorkflowVariable definition and confirm current WorkflowPatchData state
fd -t f "schema.yml" packages/openapi-schema/ --exec cat -n {} \; | head -100Repository: refly-ai/refly
Length of output: 4058
🏁 Script executed:
# Get file size and structure
wc -l packages/openapi-schema/schema.ymlRepository: refly-ai/refly
Length of output: 98
🏁 Script executed:
# Search for WorkflowVariable and WorkflowPatchData definitions
rg -n "WorkflowVariable:|WorkflowPatchData:" packages/openapi-schema/schema.yml -A 20Repository: refly-ai/refly
Length of output: 1515
🏁 Script executed:
# Search for variableType enum definitions in the schema
rg -n "variableType:" packages/openapi-schema/schema.yml -B 2 -A 8Repository: refly-ai/refly
Length of output: 1348
🏁 Script executed:
# Search TypeScript implementation for patch operations and option handling
rg -n "WorkflowPatchData|WorkflowPatchOperation" --type=ts -B 3 -A 5Repository: refly-ai/refly
Length of output: 24932
🏁 Script executed:
# Search for option variable handling in codebase
rg -n "variableType.*option|option.*variable" --type=ts -B 2 -A 2Repository: refly-ai/refly
Length of output: 50370
🏁 Script executed:
# Search for isSingle property references
rg -n "isSingle" --type=ts packagesRepository: refly-ai/refly
Length of output: 4746
Add support for option-type variables in WorkflowPatchData.
WorkflowVariable.variableType supports option, but WorkflowPatchData.variableType does not. This prevents patch operations from updating option variables and their configuration (option lists and selection mode).
🛠️ Suggested schema extension
variableType:
type: string
enum:
- string
+ - option
- resource
description: New variable type
+ options:
+ type: array
+ items:
+ type: string
+ description: Option values for option-type variables
+ isSingle:
+ type: boolean
+ description: Whether the option variable is single-select or multi-select📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type: object | |
| description: Data for updating a task or variable | |
| properties: | |
| title: | |
| type: string | |
| description: New display title for the task | |
| prompt: | |
| type: string | |
| description: New prompt or instruction for this task | |
| dependentTasks: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of task IDs that must execute before this task | |
| toolsets: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of toolset IDs for this task | |
| variableType: | |
| type: string | |
| enum: | |
| - string | |
| - resource | |
| description: New variable type | |
| name: | |
| type: string | |
| description: New variable name | |
| description: | |
| type: string | |
| description: New variable description | |
| required: | |
| type: boolean | |
| description: Whether this variable is required | |
| resourceTypes: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableResourceType' | |
| description: New accepted resource types | |
| value: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableValue' | |
| description: New variable values | |
| WorkflowPatchOperation: | |
| type: object | |
| description: Data for updating a task or variable | |
| properties: | |
| title: | |
| type: string | |
| description: New display title for the task | |
| prompt: | |
| type: string | |
| description: New prompt or instruction for this task | |
| dependentTasks: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of task IDs that must execute before this task | |
| toolsets: | |
| type: array | |
| items: | |
| type: string | |
| description: New list of toolset IDs for this task | |
| variableType: | |
| type: string | |
| enum: | |
| - string | |
| - option | |
| - resource | |
| description: New variable type | |
| options: | |
| type: array | |
| items: | |
| type: string | |
| description: Option values for option-type variables | |
| isSingle: | |
| type: boolean | |
| description: Whether the option variable is single-select or multi-select | |
| name: | |
| type: string | |
| description: New variable name | |
| description: | |
| type: string | |
| description: New variable description | |
| required: | |
| type: boolean | |
| description: Whether this variable is required | |
| resourceTypes: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableResourceType' | |
| description: New accepted resource types | |
| value: | |
| type: array | |
| items: | |
| $ref: '#/components/schemas/VariableValue' | |
| description: New variable values | |
| WorkflowPatchOperation: |
🤖 Prompt for AI Agents
In `@packages/openapi-schema/schema.yml` around lines 12873 - 12917,
WorkflowPatchData currently lacks support for option-type variables so patch
requests cannot change option variables; update the WorkflowPatchData schema
(the object under WorkflowPatchData in the diff) to include "option" in the
variableType enum (matching WorkflowVariable.variableType), and add properties
to carry option configuration such as an options array (items referencing the
existing or new VariableOption/VariableValue schema) and a selectionMode field
(enum like "single"/"multiple" or referencing VariableSelectionMode) so patch
operations can update option lists and selection behavior; ensure property names
match the runtime model (e.g., variableType, options, selectionMode) and
reference existing component schemas where appropriate.
| /** | ||
| * Type of patch operation to apply to a workflow plan | ||
| */ | ||
| export type WorkflowPatchOp = | ||
| | 'updateTitle' | ||
| | 'createTask' | ||
| | 'updateTask' | ||
| | 'deleteTask' | ||
| | 'createVariable' | ||
| | 'updateVariable' | ||
| | 'deleteVariable'; | ||
|
|
||
| /** | ||
| * Data for updating a task or variable | ||
| */ | ||
| export type WorkflowPatchData = { | ||
| /** | ||
| * New display title for the task | ||
| */ | ||
| title?: string; | ||
| /** | ||
| * New prompt or instruction for this task | ||
| */ | ||
| prompt?: string; | ||
| /** | ||
| * New list of task IDs that must execute before this task | ||
| */ | ||
| dependentTasks?: Array<string>; | ||
| /** | ||
| * New list of toolset IDs for this task | ||
| */ | ||
| toolsets?: Array<string>; | ||
| /** | ||
| * New variable type | ||
| */ | ||
| variableType?: 'string' | 'resource'; | ||
| /** | ||
| * New variable name | ||
| */ | ||
| name?: string; | ||
| /** | ||
| * New variable description | ||
| */ | ||
| description?: string; | ||
| /** | ||
| * Whether this variable is required | ||
| */ | ||
| required?: boolean; | ||
| /** | ||
| * New accepted resource types | ||
| */ | ||
| resourceTypes?: Array<VariableResourceType>; | ||
| /** | ||
| * New variable values | ||
| */ | ||
| value?: Array<VariableValue>; | ||
| }; |
There was a problem hiding this comment.
WorkflowPatchData.variableType blocks 'option' updates
WorkflowVariable.variableType now supports 'option', but patch updates still only accept 'string' | 'resource'. That prevents valid updates and forces downstream casts. Please update the OpenAPI schema and regenerate types so patch updates can set 'option'.
🐛 Suggested schema outcome (regenerate after updating schema)
- variableType?: 'string' | 'resource';
+ variableType?: 'string' | 'resource' | 'option';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Type of patch operation to apply to a workflow plan | |
| */ | |
| export type WorkflowPatchOp = | |
| | 'updateTitle' | |
| | 'createTask' | |
| | 'updateTask' | |
| | 'deleteTask' | |
| | 'createVariable' | |
| | 'updateVariable' | |
| | 'deleteVariable'; | |
| /** | |
| * Data for updating a task or variable | |
| */ | |
| export type WorkflowPatchData = { | |
| /** | |
| * New display title for the task | |
| */ | |
| title?: string; | |
| /** | |
| * New prompt or instruction for this task | |
| */ | |
| prompt?: string; | |
| /** | |
| * New list of task IDs that must execute before this task | |
| */ | |
| dependentTasks?: Array<string>; | |
| /** | |
| * New list of toolset IDs for this task | |
| */ | |
| toolsets?: Array<string>; | |
| /** | |
| * New variable type | |
| */ | |
| variableType?: 'string' | 'resource'; | |
| /** | |
| * New variable name | |
| */ | |
| name?: string; | |
| /** | |
| * New variable description | |
| */ | |
| description?: string; | |
| /** | |
| * Whether this variable is required | |
| */ | |
| required?: boolean; | |
| /** | |
| * New accepted resource types | |
| */ | |
| resourceTypes?: Array<VariableResourceType>; | |
| /** | |
| * New variable values | |
| */ | |
| value?: Array<VariableValue>; | |
| }; | |
| /** | |
| * Type of patch operation to apply to a workflow plan | |
| */ | |
| export type WorkflowPatchOp = | |
| | 'updateTitle' | |
| | 'createTask' | |
| | 'updateTask' | |
| | 'deleteTask' | |
| | 'createVariable' | |
| | 'updateVariable' | |
| | 'deleteVariable'; | |
| /** | |
| * Data for updating a task or variable | |
| */ | |
| export type WorkflowPatchData = { | |
| /** | |
| * New display title for the task | |
| */ | |
| title?: string; | |
| /** | |
| * New prompt or instruction for this task | |
| */ | |
| prompt?: string; | |
| /** | |
| * New list of task IDs that must execute before this task | |
| */ | |
| dependentTasks?: Array<string>; | |
| /** | |
| * New list of toolset IDs for this task | |
| */ | |
| toolsets?: Array<string>; | |
| /** | |
| * New variable type | |
| */ | |
| variableType?: 'string' | 'resource' | 'option'; | |
| /** | |
| * New variable name | |
| */ | |
| name?: string; | |
| /** | |
| * New variable description | |
| */ | |
| description?: string; | |
| /** | |
| * Whether this variable is required | |
| */ | |
| required?: boolean; | |
| /** | |
| * New accepted resource types | |
| */ | |
| resourceTypes?: Array<VariableResourceType>; | |
| /** | |
| * New variable values | |
| */ | |
| value?: Array<VariableValue>; | |
| }; |
🤖 Prompt for AI Agents
In `@packages/openapi-schema/src/types.gen.ts` around lines 8078 - 8134, The
WorkflowPatchData type's variableType union is missing the 'option' member,
causing patches to reject valid updates; update the OpenAPI schema definition
that describes WorkflowPatchData (and/or the referenced
WorkflowVariable.variableType enum) to include 'option' in the allowed values,
then regenerate the TypeScript types so WorkflowPatchData.variableType becomes
'string' | 'resource' | 'option' and consumers can PATCH option-typed variables
without casting.
| export type WorkflowPatchData = { | ||
| /** | ||
| * New display title for the task | ||
| */ | ||
| title?: string; | ||
| /** | ||
| * New prompt or instruction for this task | ||
| */ | ||
| prompt?: string; | ||
| /** | ||
| * New list of task IDs that must execute before this task | ||
| */ | ||
| dependentTasks?: Array<string>; | ||
| /** | ||
| * New list of toolset IDs for this task | ||
| */ | ||
| toolsets?: Array<string>; | ||
| /** | ||
| * New variable type | ||
| */ | ||
| variableType?: 'string' | 'resource'; | ||
| /** |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the structure of WorkflowPatchData around the given lines
sed -n '8089,8130p' packages/request/src/requests/types.gen.tsRepository: refly-ai/refly
Length of output: 853
🏁 Script executed:
# Search for WorkflowVariable type definition in the same file
grep -n "type WorkflowVariable" packages/request/src/requests/types.gen.ts | head -20Repository: refly-ai/refly
Length of output: 95
🏁 Script executed:
# Search for all variableType definitions/references in the file
grep -n "variableType" packages/request/src/requests/types.gen.ts | head -30Repository: refly-ai/refly
Length of output: 280
🏁 Script executed:
# Check if there's an OpenAPI schema file
find packages -name "*.openapi.*" -o -name "*openapi*" -o -name "*.yaml" -o -name "*.yml" | grep -i workflowRepository: refly-ai/refly
Length of output: 40
🏁 Script executed:
# Read WorkflowVariable definition starting at line 8421
sed -n '8421,8475p' packages/request/src/requests/types.gen.tsRepository: refly-ai/refly
Length of output: 1167
🏁 Script executed:
# Get context around line 8449 to see what type it belongs to
sed -n '8435,8460p' packages/request/src/requests/types.gen.tsRepository: refly-ai/refly
Length of output: 574
Update WorkflowPatchData.variableType to include 'option'
WorkflowPatchData.variableType supports only 'string' | 'resource' (line 8109), but WorkflowVariable.variableType includes 'string' | 'option' | 'resource' (line 8449). WorkflowVariable explicitly documents options field as "only valid when variable type is option". If patch operations cannot set or update option-type variables, this breaks consistency. Update the OpenAPI schema to include 'option' in the patch operation's variableType field and regenerate this file.
🤖 Prompt for AI Agents
In `@packages/request/src/requests/types.gen.ts` around lines 8089 - 8110, The
WorkflowPatchData type's variableType union is missing 'option' causing
inconsistency with WorkflowVariable.variableType; update the schema so
WorkflowPatchData.variableType includes 'option' (i.e., change the union for
variableType in WorkflowPatchData to 'string' | 'option' | 'resource') and then
regenerate the OpenAPI-generated types so the updated union appears in
packages/request/src/requests/types.gen.ts; target the WorkflowPatchData and
ensure it matches WorkflowVariable.variableType.
| setNodeEditContext: (canvasId: string, context: NodeEditContext | null) => | ||
| set((state) => ({ | ||
| nodeEditContext: { | ||
| ...state.nodeEditContext, | ||
| [canvasId]: context, | ||
| }, | ||
| })), | ||
|
|
||
| setNodeEditMode: (canvasId: string, editMode: 'modify' | 'extend') => | ||
| set((state) => { | ||
| const currentContext = state.nodeEditContext[canvasId]; | ||
| if (!currentContext) return state; | ||
| return { | ||
| nodeEditContext: { | ||
| ...state.nodeEditContext, | ||
| [canvasId]: { | ||
| ...currentContext, | ||
| editMode, | ||
| }, | ||
| }, | ||
| }; | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/stores/src/stores/copilot.ts | sed -n '130,170p'Repository: refly-ai/refly
Length of output: 172
🏁 Script executed:
head -n 200 packages/stores/src/stores/copilot.tsRepository: refly-ai/refly
Length of output: 6267
Use optional chaining for consistency when accessing nodeEditContext values.
Line 149 should use optional chaining to align with the pattern used elsewhere in the file (e.g., removeHistoryTemplateSession). However, the spread in setNodeEditContext does not need nullish coalescing since state.nodeEditContext is always initialized as an object.
🛠️ Suggested change
setNodeEditMode: (canvasId: string, editMode: 'modify' | 'extend') =>
set((state) => {
- const currentContext = state.nodeEditContext[canvasId];
+ const currentContext = state.nodeEditContext?.[canvasId] ?? null;
if (!currentContext) return state;🤖 Prompt for AI Agents
In `@packages/stores/src/stores/copilot.ts` around lines 139 - 160, The
setNodeEditMode updater should use optional chaining when reading
state.nodeEditContext[canvasId] to match the pattern used elsewhere; in the
setNodeEditMode function replace the direct lookup into state.nodeEditContext
with an optional-chained access (e.g., const currentContext =
state.nodeEditContext?.[canvasId]) and keep the early return if no context,
while leaving setNodeEditContext's spread logic as-is since
state.nodeEditContext is always an object.
- Introduced comprehensive documentation for the Node Edit Context feature, detailing its architecture, functionality, and core capabilities. - Included flowcharts and sequence diagrams to illustrate the user interaction and backend processing for targeted node editing. - Documented the core implementation details, including file changes, type definitions, and key design decisions. - Enhanced clarity on the incremental update mechanism and the distinction between modify and extend modes for user actions. This documentation serves to improve understanding and facilitate future development related to the Node Edit Context feature.
1 participant
This update improves user experience by providing clear context and options for editing nodes directly within the Copilot interface.
Summary by CodeRabbit
Release Notes