refactor: Find pipeline entities inside nested subgraphs by camielvs · Pull Request #2685 · TangleML/tangle-ui

camielvs · 2026-08-31T23:47:36Z

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

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.

github-actions · 2026-08-31T23:47:48Z

🎩 Preview

A preview build has been created at: 08-31-feat_locate_entities_across_subgraphs/c58b371

camielvs · 2026-09-01T00:06:59Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

camielvs

Reviewed as the bottom of the stack. The findTaskById refactor is behaviour-preserving — the new path can return early with a non-task kind where the old recursion would have kept descending, but that only diverges under duplicate $ids across levels, and I traced every id source looking for one: generateUniqueId's module-level counter is shared even across fresh IncrementingIdGenerator instances (so promoteInlineSubgraph's per-call generator is still unique), ReplayIdGenerator replays a stack collected from a live spec and falls back to unique generation when exhausted, and createSubgraph / unpackSubgraph detach originals before rebuilding with fresh ids. No collision path exists.

Also verified: no null deref (inputs/outputs/tasks/bindings are all mobx-keystone props with () => [] defaults, so the four new .find() calls are safe); parentContext's nested.parentContext ?? {...} correctly keeps the deepest parent rather than overwriting with the root, and it matches navigationStore.ParentContext field-for-field; and no cycle guard is needed here (unlike collectIssues' visitedSpecIds) because subgraphSpec is a child prop rather than a ref, making a spec-as-own-descendant structurally impossible. tsc --noEmit and eslint clean, full src/models/componentSpec suite green (238 tests including the 9 new ones).

One finding inline. I reviewed the three PRs above this one first to avoid raising anything they already fix — this one isn't fixed up the stack, it compounds.

Minor, not worth a thread: locatedEntityName is exported but only used by tests until the stacked consumers land, and locateEntity never matches a spec's own $id — fine today, but relevant if a consumer ever passes a navigationPath.specId.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

morgan-wowk

🤖 Automated review

Approving. Verified this is a behavior-preserving refactor: findTaskById now delegates to locateEntity and filters to kind === "task"; for globally-unique ids the DFS returns the same task the old pre-order search did, and the repointed callers don't expose the ordering difference. parentContext correctly reports the immediate owning subgraph task, not the root (tested for a deep entity).

Same non-blocking recursion note as #2655 — first-match-wins correctness rests on id uniqueness, so a one-line assertion/comment at the recursion site would help future readers (see the related SHOULD-FIX on #2687).

camielvs · 2026-09-04T00:51:04Z

Merge activity

Sep 4, 12:51 AM UTC: A user started a stack merge that includes this pull request via Graphite.
Sep 4, 12:57 AM UTC: Graphite rebased this pull request as part of a merge.
Sep 4, 1:01 AM UTC: @camielvs merged this pull request with Graphite.

`locateEntity` answers "which spec in this pipeline owns this $id" — returning the owning spec, the chain of subgraph task names leading to it, and the immediate parent subgraph task. Entity $ids are unique across the whole document, so a match at any depth is unambiguous. `findTaskById` now delegates to it instead of running its own recursion. Additive and behaviour-preserving on its own: the consumers land in the PRs above this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`EntityLocation.subgraphPath` reused the name and type of `ComponentValidationIssue.subgraphPath` while omitting the `"root"` prefix that every consumer of the latter assumes, so routing one through the other compiles clean and silently no-ops. Naming it `subgraphTaskNames` makes the difference visible at each call site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

## 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. ![image.png](https://app.graphite.com/user-attachments/assets/77d77d64-1b0a-4921-aae6-1f4a5f4ea2d2.png) ## 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.

camielvs requested a review from a team as a code owner August 31, 2026 23:47

This was referenced Aug 31, 2026

fix: Apply AI edits in the subgraph the entity lives in #2683

Merged

feat: Let the AI add steps and ports inside a subgraph #2684

Merged

fix: Misc Bugfixes #2646

Merged

fix: Give AI full visibility into subgraphs #2655

Merged

camielvs mentioned this pull request Sep 1, 2026

fix: Make AI chat chips work for steps inside subgraphs #2686

Merged

3 tasks

camielvs force-pushed the 08-31-feat_locate_entities_across_subgraphs branch from 40017e2 to 6aa6cc2 Compare September 1, 2026 00:34

camielvs commented Sep 1, 2026

View reviewed changes

Comment thread src/models/componentSpec/queries/locateEntity.ts Outdated

camielvs mentioned this pull request Sep 1, 2026

refactor: Resolve AI mutation targets and explain refusals #2687

Merged

3 tasks

morgan-wowk approved these changes Sep 3, 2026

View reviewed changes

camielvs force-pushed the 08-20-fix_give_ai_full_visibility_into_subgraphs branch from 7982f96 to e58f2e5 Compare September 4, 2026 00:10

camielvs force-pushed the 08-31-feat_locate_entities_across_subgraphs branch from ea1b8b1 to 93a41cf Compare September 4, 2026 00:10

camielvs changed the base branch from 08-20-fix_give_ai_full_visibility_into_subgraphs to graphite-base/2685 September 4, 2026 00:51

camielvs changed the base branch from graphite-base/2685 to master September 4, 2026 00:55

camielvs and others added 2 commits September 4, 2026 00:56

camielvs force-pushed the 08-31-feat_locate_entities_across_subgraphs branch from 93a41cf to c58b371 Compare September 4, 2026 00:56

camielvs merged commit ba9a830 into master Sep 4, 2026
17 checks passed

camielvs deleted the 08-31-feat_locate_entities_across_subgraphs branch September 4, 2026 01:01

Navigation Menu

Uh oh!

refactor: Find pipeline entities inside nested subgraphs - #2685

refactor: Find pipeline entities inside nested subgraphs#2685
camielvs merged 2 commits into
masterfrom
08-31-feat_locate_entities_across_subgraphs

camielvs commented Aug 31, 2026

Uh oh!

github-actions Bot commented Aug 31, 2026 •

edited

Loading

Uh oh!

camielvs commented Sep 1, 2026 •

edited

Loading

Uh oh!

camielvs left a comment •

edited

Loading

Uh oh!

Uh oh!

morgan-wowk left a comment

Uh oh!

camielvs commented Sep 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

camielvs commented Aug 31, 2026

Description

Related Issue and Pull requests

Type of Change

Checklist

Test Instructions

Additional Comments

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 Preview

Uh oh!

camielvs commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camielvs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

morgan-wowk left a comment

Choose a reason for hiding this comment

Uh oh!

camielvs commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Aug 31, 2026 •

edited

Loading

camielvs commented Sep 1, 2026 •

edited

Loading

camielvs left a comment •

edited

Loading

camielvs commented Sep 4, 2026 •

edited

Loading