fix(workflows): avoid cloning payloads for graph updates

[ashkanvg]

Summary

Companion patch for bastani-inc/atomic#2138.

PR bastani-inc#2138 makes large graph painting topology-bounded by clipping cards/edges to the viewport, retaining layout identity, and skipping idle animation frames. While testing that branch with payload-heavy nested workflows, I found a separate bottleneck before rendering: every store mutation still built a full JSON.stringify / JSON.parse snapshot, so a small question/status update traversed complete workflow inputs, results, child outputs, and tool bodies before bastani-inc#2138's optimized renderer ran.

This patch keeps bastani-inc#2138's viewport, topology, and animation implementation intact and makes its store observation payload-independent.

What changed

• Added an optional synchronous invalidation-only store channel that does not construct a full snapshot.
• Added one immutable, memoized graph projection per store version.
• Excluded unbounded graph-irrelevant payloads:
  • run inputs;
  • authored stage result bodies;
  • child output values;
  • tool input/output bodies and unbounded tool history;
  • model-attempt and MCP-scope details.
• Preserved topology, status, timing, prompts, attachment state, notices, bounded returned-status fields, durable tool summaries, and child output counts.
• Migrated graph/overlay/attach/stage-chat/widget, lifecycle/HIL notifications, resume picker, send admission, and graph-only control helpers to the compact channel.
• Kept the existing Store.snapshot() / Store.subscribe(snapshot) contract for compatibility. The opt-in full status-file writer intentionally remains on that path because its external JSON schema is unchanged.
• Corrected fix(workflows): keep overlay graph TUI responsive for large stage graphs bastani-inc/atomic#2138's animation integration fixture to include a running stage, so its visible/hidden/dispose tick assertions exercise the new idle-skip contract rather than expecting animation from an empty graph.

Before / after

Exact bastani-inc#2138 head versus this stacked commit, using the same dependencies and a real GraphView with 480 nodes plus a 100 MiB workflow input:

Branch	Store mutation median	Viewport render median
bastani-inc#2138 (9505c1ba)	147.71 ms	3.58 ms
bastani-inc#2138 + this patch	2.85 ms	4.46 ms

bastani-inc#2138 already fixes topology-scaled rendering; this patch removes the remaining payload-scaled mutation cost.

Tests

• npm run check — Biome, TypeScript, and shrinkwrap pass.
• npm run test:unit — 613 files, 5,719 passed, 2 skipped.
• npm run test:integration — 35 files passed, 1 skipped; 484 passed, 1 skipped.
• npm run test:ci-contracts — 5 files, 38 passed.
• Focused combined regression gate:
  • payload-free synchronous observation;
  • legacy full-snapshot compatibility;
  • real large-payload graph click and question interaction;
  • fix(workflows): keep overlay graph TUI responsive for large stage graphs bastani-inc/atomic#2138 viewport-bounded card/edge rendering;
  • fix(workflows): keep overlay graph TUI responsive for large stage graphs bastani-inc/atomic#2138 animation eligibility.

Notes

• No dependency or lockfile changes.
• No DBOS, checkpoint, persistence, or workflow authoring schema changes.
• Intended to be merged or cherry-picked into fix/2100-overlay-graph-tui-perf before fix(workflows): keep overlay graph TUI responsive for large stage graphs bastani-inc/atomic#2138 lands.

[flora131]

Thanks @ashkanvg — this is a good catch, and it's the right complement to @Shreyasd10's work. bastani-inc#2138 made painting topology-bounded; you found that the store was still payload-bound before the renderer ever ran, which is the half that a 100 MiB workflow input actually hits.

Heads-up on where this is landing. Both changes are now combined on a branch in the upstream repository, bastani-inc/atomic#2143, as two commits:

8a45311 fix(workflows): avoid cloning payloads for graph updates   ← you
608309f fix(workflows): keep overlay graph TUI responsive …        ← @Shreyasd10

Both keep their original authors, so GitHub attributes each commit to whoever wrote it, and bastani-inc#2143 credits you both in the description.

Your commit needed rebasing onto current main, which has moved since you branched. Two conflicts, both caused by bastani-inc#2140 reshaping run cards after you wrote this. I resolved them as follows — please sanity-check the second one, since it changes your patch:

node-card.ts — mechanical. bastani-inc#2140 moved the child run id onto its own wrapped row, so the meta line no longer carries run <shortRunId> · . I kept main's shorter line and applied your completed.outputCount ?? Object.keys(completed.outputs).length.

session-overlays.ts — needs your eyes. Main had started caching a snapshot from the store.subscribe callback to feed the resume-candidate lookup:

let currentSnapshot = store.snapshot();
…
unsubscribe = store.subscribe((snapshot) => { currentSnapshot = snapshot; … });

Swapping in subscribeStoreInvalidation alone would have left currentSnapshot frozen at its initial value, because the invalidation channel carries no snapshot — newly started runs would never appear in the picker. I kept your channel and made the picker read one memoized projection per render instead:

const selectRows = () => {
  const snapshot = readGraphStoreSnapshot(store);
  const resumeCandidateLookup = intent === "resume" ? resumeCandidateCache(snapshot) : undefined;
  return selectRunsForPicker(snapshot.runs, state.query, state.includeAll, Date.now(), intent, resumeCandidateLookup);
};
unsubscribe = subscribeStoreInvalidation(store, () => tui.requestRender?.());

That keeps the payload-free path — graphSnapshot() is memoized per store version, and resumeCandidateCache already keys on snapshot.version — while dropping the staleness. npm run check passes on the combined branch. If you'd rather it read store.runs() as your original did, say so and I'll change it.

I'm reviewing your commit properly next; this comment is only about the merge mechanics. Please follow along on bastani-inc#2143.

[flora131]

@ashkanvg — your commit has been through the same adversarial review @Shreyasd10's got, run against your layer alone. Full report on #2144. It holds up: render output is byte-identical between the legacy snapshot and your compact projection across every migrated consumer, and memoization freshness held at 31 checkpoints across every mutating store method with zero stale reads.

One thing was falsified, and I think you'll find it interesting:

The projection was not actually payload-free. compactResultField bounded a result with value.slice(0, 1024). V8 backs a slice of a flat string of 13+ characters with a SlicedString that points at its parent, so the projected field reported length === 1024 while keeping the entire original alive.

That is worse here than at an ordinary truncation site, precisely because of the thing that makes your change fast: graphSnapshot() memoizes, so the store held the retention after the run left state.runs, and so did every long-lived projection holder. Your inputs: {} was correctly freed — the leak was specific to run.result. Net effect: a session with large workflow results grew the heap your change exists to reclaim.

Fixed in b7f077a — truncated fields now go through split("").join(""), which is code-unit exact and leaves no parent pointer. The regression test is calibrated rather than one-sided: its "payload deliberately held" and "no projection taken" arms pass under both the fixed and the broken source, so only the projection arm moves. I re-checked it myself by reverting just the flatten call — that arm alone fails, retained 33571744 bytes.

Related, not fixed, and not yours: summarizeToolResult in durable/tool-primitive.ts:615 builds ${serialized.slice(0, 237)}..., whose ConsString still points at the SlicedString and so at the whole JSON.stringify output — 8.4 MB pinned by a 240-char summary. It exists identically on main. Worth its own issue.

Two other things worth your attention before merge, both in the review:

• The resume intent in session-overlays.ts feeds the projection into workflowRunHasArtifactReference, which detects run-scoped artifacts by serialising { result, stages } and searching for /runs/<runId>/ — text living in exactly the fields you delete. So artifactsIntact degrades false → undefined and the resumability guard never fires. No shipped surface passes "resume" to openSessionPicker today, so it is latent, but it is a consequence of my rebase resolution putting the projection on that path. Your call on which way to fix it.
• The memoization contract — the entire point of your change — has no assertion. Removing the memo restores the per-mutation cost you removed and the full 5808-test project stays green.

And I confirmed your fixture change to overlay-entrypoints-animation.test.ts is a repair, not an accommodation: the base branch shipped that test red, because stages: [] leaves hasAnimatingStages === false so no tick ever fired. The gate is byte-identical on both branches and all three original assertions survive verbatim. Good catch on your part, and exactly the kind of thing that gets mistaken for weakening a test — so it is now on the record that it isn't.

Nice work. The SlicedString thing is genuinely subtle and I doubt many reviewers would have caught it by eye.

[flora131]

Shipped. @ashkanvg — your commit is on bastani-inc/atomic main as 0f11d37, with you as the author:

0f11d3799 Ashkan Vedadi Gargary <70602836+ashkanvg@users.noreply.github.com>
  fix(workflows): avoid cloning payloads for graph updates

It merged with a merge commit rather than a squash specifically so that stays true — git log --author finds it and git blame attributes to you. @Shreyasd10's bastani-inc#2138 landed directly below it as 532c4d0, same treatment.

Two things came out of your change that are worth knowing.

Your patch surfaced a real memory leak, and not the one you were fixing. compactResultField bounded a result with value.slice(0, 1024). V8 backs that with a SlicedString pointing at the parent, so the projected field reported length === 1024 while keeping the whole run result alive — and because graphSnapshot() memoizes, the store itself held that retention after the run left state.runs. Your inputs: {} was correctly freed; the leak was specific to run.result. Fixed on your branch before merge.

Chasing it turned up the same shape in two places on the durable path that predate all of this: summarizeToolResult and summarizeCompletedToolResult build `${serialized.slice(0, 237)}...`, pinning 8.4 MiB behind a 240-character summary written into durable checkpoints. That became its own PR, sharing a flattenTruncatedString helper extracted from your fix. There is now an open issue (#2151) to audit the rest of the codebase for the same pattern.

Your fixture change was a repair, not an accommodation, and it is now on the record as such. The edit to overlay-entrypoints-animation.test.ts looked exactly like weakening a test to make a change pass, so it got checked specifically: the base branch shipped that test red, because a run with stages: [] leaves hasAnimatingStages === false so no tick ever fired. The animation gate is byte-identical on both branches and all three original assertions survive verbatim. Good catch, and precisely the kind of thing that gets misread.

One note on attribution: that fixture repair had to move down into bastani-inc#2143, because the layer that breaks a test has to be the layer that fixes it or the lower PR is red on its own. It is unchanged and co-authored to you on that commit.

Thanks — the 147 ms → 2.85 ms measurement on a real 480-node graph with a 100 MiB input is what made the case, and the half you found was genuinely invisible from the rendering side.