refactor: Resolve AI mutation targets and explain refusals - #2687
Merged
camielvs merged 1 commit intoSep 4, 2026
Merged
Conversation
3 tasks
🎩 PreviewA preview build has been created at: |
This was referenced Sep 1, 2026
Merged
Collaborator
Author
morgan-wowk
approved these changes
Sep 3, 2026
morgan-wowk
left a comment
morgan-wowk
left a comment
There was a problem hiding this comment.
🤖 Automated review
Approving — the resolver is fail-closed and total: resolveTarget / resolveConnectable / resolveArgumentValue each return a typed { ok: true } or { ok: false, error } with enumerated refusal reasons (not-found, wrong-kind, cross-graph argument refs, existing binding), and the 21 tests cover the not-found / wrong-kind / name-collision / cross-subgraph cases.
SHOULD-FIX (defense-in-depth, not blocking): the one safety-critical assumption — that $ids are globally unique — is the one thing the resolver doesn't defend. locateEntity (src/models/componentSpec/queries/locateEntity.ts) is first-match-wins DFS, justified only by its docstring invariant. If a duplicate $id ever appears across subgraphs (a future generator change, a hand-authored/imported spec, an upstream bug), resolveTarget silently picks the first match and the edit lands in the wrong subgraph — the exact fail-open outcome this stack exists to prevent, with no refusal surfaced. Since this PR's whole job is to explain refusals, detecting a second match at a different location → a typed ambiguous refusal (plus a duplicate-id test, currently the one uncovered case) would close the loop. Safe by invariant today; just un-defended and unverified.
Minor: the description says "3 new files" — it's 2 new + 1 modified (locateEntity.ts).
Mbeaulne
approved these changes
Sep 3, 2026
camielvs
force-pushed
the
08-31-fix_entity_chips_for_nested_entities
branch
from
September 4, 2026 00:10
da70de1 to
cd23d48
Compare
camielvs
force-pushed
the
09-01-refactor_resolve_ai_mutation_targets_and_explain_refusals
branch
from
September 4, 2026 00:10
42cf28b to
44220cc
Compare
Collaborator
Author
camielvs
changed the base branch from
08-31-fix_entity_chips_for_nested_entities
to
graphite-base/2687
September 4, 2026 01:02
Extracted from the PR above, which rewires the agent's edit tools to act on whichever graph owns the entity they name. This is the layer that answers "which spec, and is it the kind I expected" and turns every failure into a sentence naming the entity and its subgraph. Nothing consumes it yet; the PR above swaps its handlers onto it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
camielvs
force-pushed
the
09-01-refactor_resolve_ai_mutation_targets_and_explain_refusals
branch
from
September 4, 2026 01:08
44220cc to
4d3efdb
Compare
camielvs
deleted the
09-01-refactor_resolve_ai_mutation_targets_and_explain_refusals
branch
September 4, 2026 01:14
camielvs
added a commit
that referenced
this pull request
Sep 4, 2026
## Description When you asked the AI assistant to change something inside a subgraph, it applied the change to the top-level pipeline instead — whatever depth the thing you named actually lived at. One cause, four ways of showing up: - **Nothing happened.** The edit went looking at the top level, found nothing, and quietly did nothing — or worse, hit an unrelated top-level step that happened to match. - **Connections could corrupt the pipeline.** Asked to wire something across a subgraph boundary, it wrote a connection pointing at a step that isn't in that graph. The pipeline format has no such connection, so the result was broken. - **Deleting a connection lied.** It reported success while deleting nothing. - **Nine tools couldn't explain a failure.** They could only answer "that didn't work", with no way to say whether the thing didn't exist or was a different kind of thing than expected — so the assistant guessed, and often told you something untrue. Each edit now works out which graph owns the thing you named and changes that graph. This turned out to be mostly plumbing that was already in place: every editing operation already took the graph to act on as an argument, and the editor itself has always used that to let you edit inside a subgraph. The assistant was the only caller that always handed over the top-level pipeline. The layer that answers "which graph, and is this the kind of thing you asked for" — along with the wording of every refusal — now lives in #2687, directly below. This PR is the rewiring: swapping each handler onto it, and the knock-on fixes below. Also fixed here, because they're the same underlying thing: - **Renaming or deleting a subgraph's input no longer breaks the wiring above it.** The matching port on the subgraph step is renamed or removed with it, so the connection feeding it in the parent survives. - **Validation reports where issues are.** Issues found inside subgraphs now carry the name and the trail to the subgraph they're in, so the assistant can act on them instead of reporting a problem it can't locate. - **Setting a value can't reach across a subgraph boundary either.** Same corruption as the connection case above, through a different door: asked to set a step's input to a value coming from another graph, it used to write it and produce a pipeline that no longer loads. It's refused now, with the way round it. - **Grouping needs at least two steps.** Asked to make a subgraph out of one step — or the same step named twice — it used to do it, despite saying elsewhere that it wouldn't. Two refusals remain, and both are about structure rather than depth, so they're correct rather than missing. Both now say what to do instead: - A connection can't cross a subgraph boundary. - A new subgraph can't be made from steps that live at different levels. Where a refusal has a knowable cause, it now names it — a rename that collides with an existing name, or being asked to unpack something that isn't a subgraph. The vague "couldn't do that" is what's left when there's genuinely nothing to say, rather than the standard answer. **The canvas doesn't follow the assistant into a subgraph.** A single request can produce several edits across several subgraphs, and yanking the view around for each one would lose wherever you were. The assistant is told to name the subgraph it changed, so you're told where to look rather than being taken there. ## Related Issue and Pull requests Stacked on #2687 → #2686 → #2685 → #2655. This PR and #2684 were re-cut. Previously the two split this fix down the middle: the lower one described the failures and refused them, the upper one rewrote it to actually fix them. Since all four symptoms above share one cause and one fix, that seam meant ~635 lines were written twice and reviewed twice to net zero. The stack is now split by concern instead: shared resolver (#2685), an unrelated chip bug (#2686), this fix, then the new capability on top (#2684). ## Type of Change - [x] Bug fix ## Checklist - [ ] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Test Instructions 1. Open a pipeline with a subgraph in it (nested subgraphs are a better test) and ask the AI chat for edits inside it: "rename the second step in \<subgraph name>", "delete the connection between X and Y in \<subgraph name>", "set the input file on \<nested step>". Each should apply, and the reply should say which subgraph it changed. 2. Open the subgraph afterwards and confirm the change is there and looks right. 3. Undo (Cmd/Ctrl+Z) — a nested edit should undo like any other, and the pipeline should still save normally. 4. Rename an input inside a subgraph that the parent feeds a value to, then check the parent: the port should be renamed and still connected, not orphaned. 5. Ask for something genuinely impossible — "connect \<top-level step> straight to \<a step inside a subgraph>" — and it should explain that a connection can't cross a subgraph boundary and offer the way round it. Same for asking it to group a top-level step together with a nested one. 6. Ask it to change something that doesn't exist, and something of the wrong kind ("delete the connection \<id of a step>"). It should say what's actually wrong rather than inventing a reason. Same for renaming a step to a name already taken in that graph, and for "unpack \<a step that isn't a subgraph>". 7. Ask it to set an input on a nested step to a value that comes from the top-level pipeline — it should refuse and explain, not write it. Then ask it to group a single step into a subgraph, which it should also refuse. 8. Ask it to validate a pipeline that has problems inside a subgraph — it should report them and say where they are. 9. Sanity check that ordinary top-level editing through the chat is unchanged. ## Additional Comments Nothing new was needed for undo or saving: both already watch the whole pipeline including everything nested, which the editor has always relied on. One pre-existing quirk left alone: renaming a subgraph step doesn't update the name stored on the subgraph's own contents. Nothing depends on it, and the same thing happens when you rename a subgraph step by hand in the editor, so it's not new here — but it's worth a separate look at some point.
camielvs
added a commit
that referenced
this pull request
Sep 4, 2026
## Description The PR below this one made the assistant change existing things wherever they live. Creating something new still only worked at the top level, which left one job impossible: getting a value into or out of a subgraph. That needs a new port on the subgraph itself, and there was no way to ask for one. Adding a step, an input, or an output can now target a subgraph. Ask for something without saying where and nothing changes — it goes to the top level as before. Name a subgraph, or open one and say "add a filter step here", and it goes inside that one instead. An input added inside a subgraph shows up as a new input port on that subgraph step, which is how you feed a value into it. If you name something that isn't a subgraph, it says so and tells you how to add at the top level instead, rather than failing quietly. **Getting a value in or out now finishes the job.** A port on the boundary is only a third of the work — it also has to be wired to the step inside the subgraph that uses the value, and to whatever feeds it in the parent. Asked for this, the assistant used to create the port and stop, leaving a value that reaches nothing and two fresh validation errors where there had been none. It now does all three, so "feed this into \<subgraph>" is a single request that ends with a working pipeline. Two smaller corrections in the same area. Asked to fix a problem while you happen to be looking at a subgraph, it no longer drops the fix into that subgraph — repairs land on whatever they're repairing, wherever that lives. And it now gets the subgraph you're viewing handed to it directly rather than matching on its name, which was ambiguous whenever two subgraphs at different levels shared a name.  ## Related Issue and Pull requests Stacked on #2683 → #2687 → #2686 → #2685 → #2655. Re-cut alongside #2683 — see that PR for why. This one is now purely the new capability: it adds to the PR below rather than rewriting it. ## Type of Change - [x] New feature ## Checklist - [ ] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Test Instructions 1. Open a pipeline with a subgraph. Ask the chat: "add a \<component> inside \<subgraph name>". Open the subgraph and confirm it's there, and that it did not also appear at the top level. 2. Open a subgraph, then say "add an input called threshold here" — it should go into the subgraph you're looking at, not the top level. 3. Go back up: that subgraph step should now have a `threshold` input port on it. 4. Ask for the whole route in one go — "feed \<some top-level value> into \<subgraph name> so \<inner step> can use it". It should end with the port wired on both sides: connected inside the subgraph to the step that uses it, and connected in the parent to the value you named. Ask it to validate afterwards — there should be no new errors, and in particular no unconnected port left behind. 5. Same again with an output — it should appear as an output port on the subgraph step, and the route out should get wired end to end the same way. 6. Ask for something with no location at all ("add a \<component>") while looking at the top level — it should go to the top level, as before. 7. Ask it to add something "inside" a step that isn't a subgraph — it should explain that step isn't a subgraph rather than silently doing something else. 8. Open a pipeline with a validation problem on a top-level step, then open a subgraph before asking the chat to fix it. The fix should land on the top-level step, not inside the subgraph you happen to be looking at. 9. Undo (Cmd/Ctrl+Z) after a nested add, and confirm the pipeline still saves. ## Additional Comments The earlier version of this PR also returned the subgraph trail on every single edit so the assistant could quote it back. That's been dropped: the assistant already knows which subgraph it targeted, because it passed the id, so the extra field was duplicating information it had. It's still told to name the subgraph it changed in its reply — the canvas doesn't move, so you need to be told where to look.
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.
Description
Split out of the PR above it.
That PR reroutes the AI assistant's edit tools so they act on the graph that
actually owns the thing you named, instead of always the top level. Underneath
that sits a layer answering a narrower question: given the pipeline and an
entity's id, which graph owns it, is it the kind of thing the caller expected,
and — when the answer is no — what do you tell the user?
That layer is this PR. It's three things:
graph that owns it. Connection endpoints resolve slightly differently, since
they can be a step or a port but not an existing connection.
DropNullsinsidesubgraph
Preprocess" — so every failure can say where it was looking.a rename onto a name already taken in that graph, unpacking something that
isn't a subgraph, and a value referencing something that lives in a different
graph (which can't be written down in the pipeline format at all).
Nothing consumes it yet — the PR above swaps its handlers onto it. Same shape as
the resolver PR further down the stack, and split for the same reason: it can be
read on its own in one sitting.
Related Issue and Pull requests
Stacked on #2686 → #2685 → #2655.
Carved out of #2683 after review, which is down from +999 to +744 as a result.
Type of Change
Checklist
Test Instructions
Nothing to click through — no caller reaches this code until the PR above.
21 unit tests cover it: resolving at the top level and inside a nested subgraph,
each failure message, and the three explained refusals.
Additional Comments
The two type helpers added to
locateEntityare here rather than in #2685because they exist to narrow a location to an expected kind, which is this
layer's job and has no consumer in #2685.
The ratio of message text to logic is high on purpose — explaining refusals in
words the assistant can pass on is the point of the work, not a side effect of
it.