Implement canvas drift detection in Copilot workflow#2265
Open
Implement canvas drift detection in Copilot workflow#2265
Conversation
- Added `computeCanvasDrift` function to analyze discrepancies between workflow plans and canvas nodes, identifying tasks missing from the canvas and nodes added outside the plan. - Integrated drift detection into the `get_workflow_summary` method, returning a summary of drift status and details when discrepancies are found. - Updated documentation for `get_workflow_summary` to include new canvas drift information and handling instructions for users. - Enhanced the tool selection guide to clarify when to use `get_canvas_snapshot` based on drift detection results. This update improves the accuracy of workflow management by providing users with insights into potential inconsistencies between their plans and the actual canvas state.
📝 WalkthroughWalkthroughThis PR introduces Canvas Drift Detection, a new feature that compares planned workflow tasks against canvas nodes to identify synchronization mismatches. Drift checks are integrated into patch and generate workflows, validating that targeted tasks still exist before applying changes. Drift metadata is surfaced in workflow summary responses, and prompt templates are updated to guide drift-aware handling. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Workflow
participant Canvas as Canvas Storage
participant DriftDetector as Drift Detector
participant Summary as Workflow Summary
Client->>Workflow: Request patch/generate with canvasId
Workflow->>Canvas: Fetch canvas data & skillResponse nodes
Canvas-->>Workflow: Return canvas nodes
Workflow->>DriftDetector: computeCanvasDrift(plan, canvasNodes)
DriftDetector->>DriftDetector: Compare tasks vs nodes
DriftDetector-->>Workflow: Return CanvasDrift{hasDrift, counts, items, summary}
alt Drift Detected
Workflow->>Workflow: Validate patch targets exist
alt Targets Missing
Workflow-->>Client: Error with drift details
else Targets OK
Workflow->>Summary: Apply patch/generate with canvasDrift
Summary-->>Client: Return summary + canvasDrift metadata
end
else No Drift
Workflow->>Summary: Apply patch/generate
Summary-->>Client: Return summary
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Deploying refly-branch-test with
|
| Latest commit: |
2208691
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://487f4a35.refly-branch-test.pages.dev |
| Branch Preview URL: | https://fix-cavans-draft.refly-branch-test.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
packages/agent-tools/src/copilot/index.ts (4)
44-52: Consider extracting taskId extraction to a helper.The pattern for extracting
taskIdfrom node metadata is repeated multiple times (lines 47, 60, 66, and 250-252). A small helper would improve readability and reduce duplication.♻️ Optional helper extraction
function getNodeTaskId(node: CanvasNode): string | undefined { return (node.data?.metadata as Record<string, unknown>)?.taskId as string | undefined; }Then usage becomes:
-const taskId = (node.data?.metadata as Record<string, unknown>)?.taskId as string | undefined; +const taskId = getNodeTaskId(node);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 44 - 52, Extract the repeated taskId extraction into a small helper (e.g., getNodeTaskId) and replace the inline casts in places that read taskId from a node (examples: the loop over canvasNodes that sets canvasTaskIds, other occurrences at the locations currently using the same pattern) with calls to this helper; ensure the helper signature returns string | undefined and internally does the safe optional chaining and cast ((node.data?.metadata as Record<string, unknown>)?.taskId as string | undefined) so callers like canvasTaskIds.set(taskId, node) and any conditional checks use the helper instead of duplicating the expression.
38-91: Add JSDoc documentation for the function.Per coding guidelines, functions should have JSDoc style comments. This is an internal utility but documenting its purpose and parameters improves maintainability.
📝 Suggested JSDoc
+/** + * Computes drift between a workflow plan and actual canvas nodes. + * Identifies tasks missing from canvas and nodes added outside the plan. + * `@param` plan - The workflow plan record to compare against + * `@param` canvasNodes - Array of canvas nodes to check + * `@returns` CanvasDrift object with drift status and details + */ function computeCanvasDrift(plan: WorkflowPlanRecord, canvasNodes: CanvasNode[]): CanvasDrift {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 38 - 91, Add a JSDoc comment for the computeCanvasDrift function describing its purpose, parameters, and return value: explain that computeCanvasDrift compares a WorkflowPlanRecord (plan) and an array of CanvasNode (canvasNodes) to detect drift, document the plan parameter and its tasks, the canvasNodes parameter and that only 'skillResponse' nodes are considered, and describe the CanvasDrift return shape (hasDrift, planTaskCount, canvasWorkflowNodeCount, missingFromCanvas, addedOnCanvas, summary). Place the JSDoc immediately above the computeCanvasDrift declaration and reference the WorkflowPlanRecord, CanvasNode and CanvasDrift types in the tags (`@param`, `@returns`) and include short notes about how taskId is extracted from node.data?.metadata.
271-273: Empty catch block silently swallows errors.While the comment indicates this is non-critical, silently swallowing errors can make debugging difficult. Consider adding minimal logging or at least a more descriptive comment about what errors are expected.
🛡️ Suggested improvement
} catch { - // Non-critical: proceed with patch even if drift check fails + // Non-critical: drift check is best-effort; proceed with patch if canvas + // data fetch fails (e.g., network issues, permissions). The patch operation + // itself will validate task existence on the server side. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 271 - 273, The empty catch following the drift check silently swallows errors; change it to catch the error (e.g., catch (err)) and emit a minimal log with context (use the existing logger such as processLogger or console.warn/console.error) like "Drift check failed, proceeding with patch" plus the error details, and update the comment to note which error types are expected to be ignored; update the catch block in the try/catch around the drift check in packages/agent-tools/src/copilot/index.ts accordingly.
262-270: Error message exceeds 100 character line length limit.Per coding guidelines, maximum line length is 100 characters. Consider breaking this into a multi-line template literal or constructing the message with a variable.
✏️ Suggested fix
if (staleTaskOps.length > 0) { + const missingIds = staleTaskOps.map((op) => op.taskId).join(', '); + const errorMsg = `Cannot patch: ${staleTaskOps.length} targeted task(s) no longer ` + + `exist on canvas. Missing task IDs: ${missingIds}. The canvas may have been ` + + 'manually modified. Call get_workflow_summary to see drift details, or use ' + + 'generate_workflow to recreate.'; return { status: 'error', data: { - error: `Cannot patch: ${staleTaskOps.length} targeted task(s) no longer exist on canvas. Missing task IDs: ${staleTaskOps.map((op) => op.taskId).join(', ')}. The canvas may have been manually modified. Call get_workflow_summary to see drift details, or use generate_workflow to recreate.`, + error: errorMsg, }, summary: 'Patch failed due to canvas drift', }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-tools/src/copilot/index.ts` around lines 262 - 270, The long error string in the return block that checks staleTaskOps exceeds the 100-char line length limit; refactor the message construction inside the same block (where staleTaskOps is referenced) by assigning the detailed message to a local variable or using a multi-line template literal (e.g., const errorMsg = `...`) and then return that variable in data.error, keeping each source code line under 100 characters; ensure you still include the staleTaskOps.map(...).join(', ') output and preserve the existing status and summary fields.packages/skill-template/src/prompts/templates/copilot-agent-system.md (1)
45-45: Consider varying sentence structure for better readability.The static analysis tool flagged three successive sentences beginning with "Use". Consider rephrasing to improve flow:
✏️ Suggested rewording
-**Default Preference**: Use `patch_workflow` when an existing workflow plan exists and user requests specific modifications. Use `generate_workflow` for new workflows or major restructuring. Use `get_workflow_summary` when you need to verify task/variable IDs before making changes. Use `get_canvas_snapshot` when `get_workflow_summary` returns no plan, when drift is detected (`canvasDrift.hasDrift` is true), or when you need to see the actual canvas state. +**Default Preference**: Prefer `patch_workflow` when an existing workflow plan exists and the user requests specific modifications. For new workflows or major restructuring, call `generate_workflow`. To verify task/variable IDs before making changes, invoke `get_workflow_summary`. Finally, call `get_canvas_snapshot` when `get_workflow_summary` returns no plan, when drift is detected (`canvasDrift.hasDrift` is true), or when you need to see the actual canvas state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skill-template/src/prompts/templates/copilot-agent-system.md` at line 45, Rewrite the "**Default Preference**" sentence to avoid three consecutive sentences starting with "Use": consolidate and vary sentence starters so it reads smoothly while keeping the same rules (prefer patch_workflow for edits to existing plans, generate_workflow for new or major restructures, get_workflow_summary to verify IDs before changes, and get_canvas_snapshot when get_workflow_summary returns no plan or canvasDrift.hasDrift is true). Update the text under the Default Preference heading in copilot-agent-system.md to merge some clauses and swap sentence openings (e.g., "Prefer", "Reserve", "Run", "Fallback to") so the guidance is clearer and the original intent and keywords (patch_workflow, generate_workflow, get_workflow_summary, get_canvas_snapshot, canvasDrift.hasDrift) remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/agent-tools/src/copilot/index.ts`:
- Around line 44-52: Extract the repeated taskId extraction into a small helper
(e.g., getNodeTaskId) and replace the inline casts in places that read taskId
from a node (examples: the loop over canvasNodes that sets canvasTaskIds, other
occurrences at the locations currently using the same pattern) with calls to
this helper; ensure the helper signature returns string | undefined and
internally does the safe optional chaining and cast ((node.data?.metadata as
Record<string, unknown>)?.taskId as string | undefined) so callers like
canvasTaskIds.set(taskId, node) and any conditional checks use the helper
instead of duplicating the expression.
- Around line 38-91: Add a JSDoc comment for the computeCanvasDrift function
describing its purpose, parameters, and return value: explain that
computeCanvasDrift compares a WorkflowPlanRecord (plan) and an array of
CanvasNode (canvasNodes) to detect drift, document the plan parameter and its
tasks, the canvasNodes parameter and that only 'skillResponse' nodes are
considered, and describe the CanvasDrift return shape (hasDrift, planTaskCount,
canvasWorkflowNodeCount, missingFromCanvas, addedOnCanvas, summary). Place the
JSDoc immediately above the computeCanvasDrift declaration and reference the
WorkflowPlanRecord, CanvasNode and CanvasDrift types in the tags (`@param`,
`@returns`) and include short notes about how taskId is extracted from
node.data?.metadata.
- Around line 271-273: The empty catch following the drift check silently
swallows errors; change it to catch the error (e.g., catch (err)) and emit a
minimal log with context (use the existing logger such as processLogger or
console.warn/console.error) like "Drift check failed, proceeding with patch"
plus the error details, and update the comment to note which error types are
expected to be ignored; update the catch block in the try/catch around the drift
check in packages/agent-tools/src/copilot/index.ts accordingly.
- Around line 262-270: The long error string in the return block that checks
staleTaskOps exceeds the 100-char line length limit; refactor the message
construction inside the same block (where staleTaskOps is referenced) by
assigning the detailed message to a local variable or using a multi-line
template literal (e.g., const errorMsg = `...`) and then return that variable in
data.error, keeping each source code line under 100 characters; ensure you still
include the staleTaskOps.map(...).join(', ') output and preserve the existing
status and summary fields.
In `@packages/skill-template/src/prompts/templates/copilot-agent-system.md`:
- Line 45: Rewrite the "**Default Preference**" sentence to avoid three
consecutive sentences starting with "Use": consolidate and vary sentence
starters so it reads smoothly while keeping the same rules (prefer
patch_workflow for edits to existing plans, generate_workflow for new or major
restructures, get_workflow_summary to verify IDs before changes, and
get_canvas_snapshot when get_workflow_summary returns no plan or
canvasDrift.hasDrift is true). Update the text under the Default Preference
heading in copilot-agent-system.md to merge some clauses and swap sentence
openings (e.g., "Prefer", "Reserve", "Run", "Fallback to") so the guidance is
clearer and the original intent and keywords (patch_workflow, generate_workflow,
get_workflow_summary, get_canvas_snapshot, canvasDrift.hasDrift) remain
unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/agent-tools/src/copilot/index.tspackages/skill-template/src/prompts/templates/copilot-agent-system.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
computeCanvasDriftfunction to analyze discrepancies between workflow plans and canvas nodes, identifying tasks missing from the canvas and nodes added outside the plan.get_workflow_summarymethod, returning a summary of drift status and details when discrepancies are found.get_workflow_summaryto include new canvas drift information and handling instructions for users.get_canvas_snapshotbased on drift detection results.This update improves the accuracy of workflow management by providing users with insights into potential inconsistencies between their plans and the actual canvas state.
Summary by CodeRabbit
New Features
Documentation