Conversation
🎩 PreviewA preview build has been created at: |
895d21c to
fc752ef
Compare
camielvs
left a comment
There was a problem hiding this comment.
Correctness pass over the branch (diff against ai-sticky-notes). Typecheck, eslint and all three touched test files pass; the findings below came from reading the new code against the surrounding canvas/registry machinery plus two throwaway probes.
Seven findings inline, ranked roughly by severity. Two things to flag up front:
The module-init cycle is the blocking one. The new editorRegistry import makes csomBridge.ts throw TypeError: NodeTypeRegistry is not a constructor when it is the first module imported — verified failing on this branch, passing on base. It is masked purely by import order in toolBridge/index.ts, so it will surface on an unrelated refactor or on any test that imports the bridge directly.
(0,0) means "unset" throughout the codebase, and two findings here are downstream of that (move_node to the origin, and unplaced nodes in the serializer). #2766 has a third. resolvePosition substitutes an index-based default for the origin, so the position the model reads and the position the user sees can disagree on any pipeline without a saved layout. Probably one shared decision rather than three local fixes: either report the resolved position to the model, or give the AI-side paths the same resolvePosition treatment the rest of the canvas uses.
fc752ef to
1f249d0
Compare
1f249d0 to
05e3796
Compare
05e3796 to
08ba50a
Compare
The assistant had no idea where anything was. Nodes carry a position on the canvas, and none of it reached the model, so it could neither describe the layout nor do anything about it. New steps were placed to the right of whatever already existed — fine for one, but ask for a five-stage pipeline and you got a straight horizontal line to rearrange by hand. Positions now appear alongside every task, input, output and sticky note. A node that has never been placed says nothing rather than claiming to be at the origin, so the assistant cannot "restore" something to coordinates it invented. Two new abilities. `move_node` nudges a single task, port or note — the targeted fix for one thing overlapping another. `auto_layout` arranges the whole graph along its connections, and it is the editor's own auto-layout rather than a second implementation, so you get exactly what View > Auto-layout gives you, right down to the choice of algorithm. That matters more than it sounds: the arrangement depends on the rendered size of each node, which only the live canvas knows. Both limits of auto-layout are in the prompts rather than left to be discovered. It only arranges the graph on screen, so after building inside a subgraph the assistant says the layout has not been applied there instead of claiming a tidy canvas. And it moves sticky notes along with everything else, so it is told to read them first and to mention that it rearranged them. Repair is told layout is not correctness and not to tidy the canvas as a flourish after a fix. The run view keeps its read-only stance: it gains the positions in what it reads and neither of the two writes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- csomBridge no longer imports the editor node barrel. That barrel pulls in the whole canvas and loops back through the node registries, so importing the bridge first threw `NodeTypeRegistry is not a constructor`. The registry lookup moves to `moveNodeToPosition` in task.actions, alongside the auto-layout one that already lives there. csomBridge.moduleInit.test.ts guards the import. - `move_node` to exactly (0,0) is honoured. `resolvePosition` decides "never placed" on the presence of the annotation rather than on the value being the origin, which the codec also returns as its default. This also stops `create_subgraph` relocating its top-left member. - auto_layout no longer relocates locked sticky notes; the guard is in `updateFlexNodePosition` so every path into it agrees. - get_pipeline_state reports resolved positions, so a YAML-imported pipeline no longer reads as unplaced when the user can see it laid out. architect.md updated to match. - auto_layout reports failure when the handler bailed out, via an onLaidOut param rather than the shortcut's return value, which already means "let the native key event through". - move_node lands as a named "Move node" undo step like every other AI mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
08ba50a to
11133e5
Compare
Description
The assistant had no idea where anything was. Nodes carry a position on the canvas, and
none of it reached the model, so it could neither describe the layout nor do anything
about it. New steps were placed to the right of whatever already existed — fine for one,
but ask for a five-stage pipeline and you got a straight horizontal line to rearrange by
hand.
Positions now appear alongside every task, input, output and sticky note. These are the
resolved positions — where the node is actually drawn — so a pipeline imported from YAML
with no saved layout reports the coordinates the user is looking at rather than nothing
at all.
Two new abilities.
move_nodenudges a single task, port or note — the targeted fix forone thing overlapping another.
auto_layoutarranges the whole graph along itsconnections, and it is the editor's own auto-layout rather than a second implementation,
so you get exactly what View > Auto-layout gives you, right down to the choice of
algorithm. That matters more than it sounds: the arrangement depends on the rendered size
of each node, which only the live canvas knows.
Both limits of auto-layout are in the prompts rather than left to be discovered. It only
arranges the graph on screen, so after building inside a subgraph the assistant says the
layout has not been applied there instead of claiming a tidy canvas. And it moves sticky
notes along with everything else, so it is told to read them first and to mention that it
rearranged them. Repair is told layout is not correctness and not to tidy the canvas as a
flourish after a fix.
The run view keeps its read-only stance: it gains the positions in what it reads and
neither of the two writes.
Related Issue and Pull requests
Second of five. Stacked on the sticky-notes PR below it, which is where the
stickyNotespayload thatauto_layoutis told to read comes from.Type of Change
Checklist
Test Instructions
relative positions, not guesses.
with
move_node, not rearrange the whole graph.The result should match what Cmd/Ctrl+Shift+L gives you.
algorithm through.
everything else, and it should say so in its reply.
inside a subgraph — it should say the subgraph was not laid out rather than implying it
was.
🤖 Generated with Claude Code