fix: Give AI full visibility into subgraphs - #2655
Conversation
🎩 PreviewA preview build has been created at: |
309e910 to
f2dc13e
Compare
5132457 to
24082f8
Compare
24082f8 to
9b50a5c
Compare
f2dc13e to
1838665
Compare
1838665 to
4f27feb
Compare
Cross-cutting: the mutation surface isn't subgraph-aware, and this PR is what makes that reachablePosting this at the top level rather than inline because the code involved is outside this PR's diff — so it isn't a defect in the change. But this PR is the first thing that hands the model inner-subgraph
The new prompt sections mitigate this with "ask the user before editing," which is prompt-level guidance layered over a code-level footgun. Suggested fix: have the bridges distinguish "no such id" from "id exists, but inside subgraph X" and say so in the error. The shared recursive finder I suggested in my inline comment on Happy to open this as a follow-up issue instead if you'd rather keep it off this PR. |
4f27feb to
1b60fe2
Compare
|
^ I will split out into a separate PR, since making the AI edit inside subgraphs properly needs a wider look at subgraph editing capabilities rather than just better error messages on the current tools. Not a blocker for this PR, which stays read-only. |
1b60fe2 to
7982f96
Compare
morgan-wowk
left a comment
There was a problem hiding this comment.
🤖 Automated review
Approving. Verified the core claim: serializeSpecForAi still narrows subgraph tasks to their interface (isSubgraph: true, contents omitted — tested), and the new get_subgraph_state tool expands exactly one level on demand, so a deeply nested pipeline doesn't blow up the prompt. Inner $ids stay globally unique (module-level counter in generateUniqueId; deserializeSubgraphSpec's fresh generator doesn't restart the id space). promoteInlineSubgraph is now the single guard, so isSubgraph and subgraphSpec can no longer contradict each other.
Two non-blocking notes: (1) findTaskById.ts recurses over subgraphSpec with no explicit depth/visited guard — safe because the model is a single-parent tree, but a /* finite tree */ note or depth cap would make the guarantee explicit; (2) pre-existing and outside this diff: createSubgraph's snapshotTask drops a nested subgraphSpec when grouping (data loss, not a contradiction) — worth a follow-up ticket.
7982f96 to
e58f2e5
Compare
e58f2e5 to
067fa45
Compare
## Description Groundwork for the three PRs above this one. No behaviour change on its own. A pipeline can contain subgraphs, and those subgraphs can contain more subgraphs. Several parts of the app need to answer the same question about an entity — "which graph does this thing actually live in, and how do I get to it?" — and each was answering it separately, or not at all. This adds one shared helper that answers it: given the pipeline and an entity's id, it reports the graph that owns the entity, the trail of subgraph names leading down to it, and the subgraph step directly above it. Ids are unique across the whole pipeline, so there's no ambiguity about which match is meant. The existing "find a task by id" helper now uses it instead of walking the tree itself. Nothing consumes the parent-step part yet — the PRs above do. It's returned here because it falls out of the same walk, and it's covered by the tests. ## Related Issue and Pull requests Stacked on #2655. The rest of the stack builds on this. ## Type of Change - [x] Improvement ## Checklist - [ ] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Test Instructions Nothing to click through — this is internal plumbing with no user-visible effect. Unit tests cover finding entities at the top level, one level down, and several levels down, plus the parent-step reporting. ## Additional Comments Split out of the AI subgraph work so the shared piece can be reviewed on its own rather than buried in a larger diff.
## Description When the AI assistant mentions a step, input, or output in its reply, the UI turns it into a clickable chip that jumps you to it on the canvas. #2655 gave the assistant the ability to look inside subgraphs, so it started mentioning nested steps — and the chips broke for them. The chip only ever looked at the top level of the pipeline, so anything inside a subgraph rendered as a grey question mark and did nothing when clicked. Chips now find the entity wherever it lives and remember the route to it, so clicking one opens the subgraph it's in and selects it. A chip that can't take you anywhere — a connection, or something that no longer exists — is now visibly inert rather than looking clickable and doing nothing on click. **One limit in run view.** Chips there only jump within the graph you're already looking at. Run view keeps the canvas and the run's data in step by following one subgraph at a time, so a chip that jumped two levels down, or sideways into a different subgraph, would show you one subgraph's canvas alongside another subgraph's step statuses and artifacts. Those chips are inert for now rather than wrong; the fix belongs in the run view sync and is flagged as a follow-up. Chips in the editor are unrestricted. ## Related Issue and Pull requests Stacked on the resolver PR below, which is stacked on #2655. This was originally bundled into the AI editing PR. It's pulled out because it's a plain UI bug with nothing to do with editing, and can merge on its own. ## 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 that has a subgraph with a few steps inside it. 2. Ask the AI chat something that makes it mention a step inside that subgraph — e.g. "what's inside \<subgraph name>?" or "explain what \<nested step> does". 3. The chips in its reply should show the right icon (step / input / output), not a question mark. 4. Click one — it should open that subgraph and select the step. 5. Check chips for top-level steps still work exactly as before. 6. Ask something that makes it mention a connection — that chip should look plainly unclickable, and be skipped by tab rather than focusable. 7. Open a run of the same pipeline and ask the chat about a step inside a subgraph. From the top level, that chip should be inert. Open the subgraph from the canvas and ask again — chips for steps in that graph should work. ## Additional Comments Nested subgraphs work too — the chip carries the full route down, not just one level.
## 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: - **Resolve.** Find the entity, confirm it's the kind asked for, hand back the graph that owns it. Connection endpoints resolve slightly differently, since they can be a step or a port but not an existing connection. - **Describe.** Turn a location into a phrase — "step `DropNulls` inside subgraph `Preprocess`" — so every failure can say where it was looking. - **Explain.** Three cases where the reason for a refusal is knowable up front: 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 - [x] Improvement ## Checklist - [ ] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## 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 `locateEntity` are here rather than in #2685 because 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.
## 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.
## 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.

Description
The AI assistant could see that a subgraph existed, but not what was inside it. When
get_pipeline_statedescribes the pipeline, a subgraph task shows up as just a name andits input/output ports — the tasks running inside it are invisible. So if you asked
"why did this fail?" or "can I wire my new component into this?", the model was guessing
from the subgraph's name alone, and sometimes guessing wrong.
This adds a
get_subgraph_statetool that lets the assistant open up a single subgraphand see what's actually in it. It's on-demand rather than automatic: rather than stuffing
every nested subgraph into every request (which would make every message slower and more
expensive for pipelines that nest deeply), the model asks for the one subgraph it cares
about. Subgraphs inside subgraphs work the same way — it can keep going a level deeper as
needed.
The tool is read-only and available in both the editor and the run view, so the debug
assistant can now trace a failure into a nested task instead of stopping at the subgraph
boundary. Prompts for the architect, pipeline-repair, and debug specialists were updated
to tell them to look inside rather than assume.
Review turned up a related bug, now fixed here too: depending on how a subgraph got into
the pipeline, it wasn't always recorded internally as a subgraph. Pipelines loaded from
YAML were fine, but a subgraph component added straight from the library was stored
inconsistently — enough that the pipeline would describe it as a subgraph while the new
tool refused to open it, leaving the assistant contradicting itself. The rule that already
existed for this is now applied on every path a task can be created, so the two always
agree. A duplicated lookup helper was also folded into one shared copy along the way.
BEFORE
AFTER
Related Issue and Pull requests
Type of Change
Checklist
Test Instructions
"what happens inside the <subgraph name> step?" or "which component in
<subgraph name> reads the input file?"
subgraph's name or saying it can't see inside.
"why did this run fail?" — it should point at the specific inner task.
the assistant what's inside it. It should answer for a freshly added subgraph just as
it does for one that came from a saved pipeline.
normally, since the fix touches the shared path every task is created through.
Additional Comments
Worth knowing: the assistant can now see inside a subgraph, but it still can't edit
inside one — all edits apply to the top-level pipeline. The prompts tell it to ask the
user first in that situation, but the underlying tools don't yet give a clear error when
it tries, and deleting a connection inside a subgraph currently reports success without
doing anything.
That's being handled as a separate PR rather than here, since doing it properly means
looking at subgraph editing as a whole instead of just improving the error messages on
the current tools. This PR stays read-only.