feat(graph): graduate the caller-brain seam to the production runGraph surface - #868
feat(graph): graduate the caller-brain seam to the production runGraph surface#868drewstone wants to merge 2 commits into
Conversation
…h surface RunGraphOptions.brain accepts a caller-owned ToolLoopChat on the /kernel entry (#694, option A). The router-brained default is unchanged when the option is omitted. runGraph refuses brain + driverBackend and refuses a brain on an external-harness root before any compute. /kernel re-exports ToolLoopChat and ToolLoopCallContext. The user-sim-conversation example and the p1-parity graph arm now enter through the production door.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — e5e58d9f
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-15T10:38:40Z
|
Full local suite (isolated run): 216 test files passed / 2 skipped, 2714 tests passed / 6 skipped, exit 0. An earlier concurrent run showed 1 transient failure while @tangletools review now |
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — e5e58d9f
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-15T10:39:39Z
|
@tangletools review now |
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — e5e58d9f
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-15T10:41:24Z
|
| State | Detail |
|---|---|
| Interrupted | webhook restarted |
No review verdict was produced for this run. Trigger a fresh review on the current PR head if the PR is still open.
tangletools · #868 · model: kimi-for-coding · updated 2026-08-15T11:23:29Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — e5e58d9f
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-08-15T12:07:20Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 4 (1 strong-concern, 3 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 400.2s (2 bridge agents) |
| Total | 400.2s |
💰 Value — sound-with-nits
Exposes the existing test-only ToolLoopChat seam as a production option on runGraph's root driver, reusing the entire shipped execution path with sharp fail-loud refusals — coherent and in-grain; only naming/transition nits remain.
- What it does: Adds
RunGraphOptions.brain?: ToolLoopChatto the production/kernelrunGraph(src/runtime/supervise/graph.ts:220-232, 550-553): a caller-owned brain makes every ROOT driver model call by routing to the pre-existingsuperviseWithTestBrain→superviseInternalchain (graph.ts:1011-1018, supervise.ts:1445-1452), where the root's profile contract switches from `routerSupervisorProfileMaterial - Goals it achieves: Lets production consumers own the root driver's inference — deterministic conversation drivers, persona loops with their own LLM calls, byte-compatible multishot→runGraph migrations — without importing a
/testingentry or faking an OpenAI wire transport. Everything around the seam (node pinning, directive delivery, edge ledger, journal twin) is reused unchanged, and the new ledger-parity test (t - Assessment: Good change, built in the codebase's grain. The doctrine it relaxes ('production model execution enters through supervise(AgentProfile)') is amended explicitly in the /kernel export comment (src/runtime/index.ts:1022-1024), not silently violated, and the exception is scoped tightly:
supervise()/supervisorAgentstill refuse direct brain injection (supervise.ts:1436-1438), andrunGraphis `@ex - Better / existing approach: none — this is the right approach. Searched for alternatives:
RouterTransportConfig.complete(src/runtime/router-client.ts:31-42, already production but transport-level with runtime-owned model identity and metering),driverBackend/SuperviseOptions.driverBackend(harness placement, not caller inference), and the/testingentry itself (superviseWithTestBrain/supervisorAgentWithTestBrain). G - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound-with-nits
The caller-brain graduation is a clean, in-grain promotion through the existing test-brain machinery with in-repo consumers today, but it forgot the repo's own export-boundary guard — scripts/verify-package-exports.mjs still demands a 'test-only' refusal from runGraph({brain}), so the release-ga
- Integration: The seam is fully wired through existing machinery, not forked: runGraph strips the brain (src/runtime/supervise/graph.ts:551) and routes to
superviseWithTestBrain(graph.ts:1016) →buildSupervisorAgent, where the caller brain replaces the router brain (testBrain ?? routerBrainFromProfile, src/runtime/supervise/supervisor-agent.ts:505) withexpectedModelcorrectly skipped (supervisor-agent - Fit with existing patterns: Fits the codebase grain well. It is a graduation, not a rewrite: the production path reuses the exact
superviseWithTestBraininternals the/testingentry uses,runGraphWithTestBrainbecomes a thin alias so no existing caller breaks, and the STABILITY.md graduation bar items are present (substantive tests in tests/kernel/graph.test.ts, curated-doc row in docs/canonical-api.md, CHANGELOG entry - Real-world viability: Error paths are handled before any compute: the brain+driverBackend mutual exclusion and the harness-root refusal throw synchronously (graph.ts:579-588) and are test-asserted; the router-brained default and its no-router refusal are unchanged and asserted. One soft edge:
brain+routertogether is silently accepted —routeris forwarded (graph.ts:998) and ignored when the brain wins (supervi - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🔴 Stale export-boundary probe rejects the new contract; the release-gate CI step is red on this PR [integration] ``
scripts/verify-package-exports.mjs:626-637 still asserts
kernel.runGraph({}, { brain })throws an error containing 'test-only'. After this change runGraph accepts the brain, so the probe instead surfacesrunGraph: graph.nodes must be a non-empty array(graph.ts:312), the guard at line 635 rethrows it, andpnpm run verify:packagefails — confirmed live: CI run 31880028087, step 'Verify packed package exports' = failure while Lint/Test/Build/Typecheck all passed (.github/workflows/ci.yml:65,
🟡 brain + router together is silently accepted [robustness] ``
graph.ts:998 forwards
opts.routerinto superviseOptions even when a caller brain is supplied, and supervisor-agent.ts:505 (testBrain ?? routerBrainFromProfile) ignores the router when the brain wins. This contradicts the PR's own fail-loud principle for configuration contradictions (it refuses brain+driverBackend and brain+harness-root at graph.ts:579-588). A caller leaving stale router config beside a brain gets no signal. Either refuse the pair or document thatrouteris brain-less only;
💰 Value Audit
🟡 ? [maintenance] ``
Production traffic now flows through functions named and documented as test-only: graph.ts:1016 calls
superviseWithTestBrain, documented 'Deterministic scripted-brain path for tests. Not exported from Runtime's main entry' (supervise.ts:1444), which callssupervisorAgentWithTestBrain. The names now lie about the path's role. Rename the internal chain (e.g.superviseWithBrain) or have graph.ts consume an unexported-semantic entry, in a follow-up.
🟡 ? [maintenance] ``
The migration is partial: examples best-of-n, collaborates-review-loop, analyst-agent-review, shot-loop, shot-loop-resumed, watchdog-steer plus tests/examples/graph-topologies.test.ts still use the
/testingdoor, examples/p1-parity/arms.ts:53 still typesToolLoopChatfrom../../src/testingdespite the PR's stated point being the/kernelre-export, andrunGraphWithTestBrain(graph.ts:557-563) is now a byte-identical alias ofrunGraphwhile tests/helpers/runtime-with-test-brain.ts:37-3
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
✅ No Blockers —
|
| opencode GLM 5.2 | opencode DeepSeek v4 Pro | opencode DeepSeek v4 Flash | aggregate | |
|---|---|---|---|---|
| Readiness | 77 | 86 | 71 | 71 |
| Confidence | 85 | 85 | 85 | 85 |
| Correctness | 77 | 86 | 71 | 71 |
| Security | 77 | 86 | 71 | 71 |
| Testing | 77 | 86 | 71 | 71 |
| Architecture | 77 | 86 | 71 | 71 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 5/5 planned shots over 11 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 11 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 11 changed files. Global verifier still owns final merge decision.
🟡 LOW Generator byte-identity not reproducible in this review environment — docs/api/runtime.md
Could not verify the committed docs are byte-identical to
pnpm run docs:apioutput: tsdown/rolldown panics in this sandbox with a rayon ThreadPoolBuildError (thread creation blocked — 'Resource temporarily unavailable'), an environment limit, not a PR defect. Risk is low because every changed doc statement was hand-verified against source and CI'sdocs:check(typedoc + gen-primitive-catalog.mjs +git diff --exit-code -- docs/api) fails on any drift. No action needed beyond normal CI.
🟡 LOW RunGraphTestOptions.brain repeats 'Omit = ... default' text though brain is required — docs/api/testing.md
The copied doc block for the REQUIRED brain property (line 528 shows
readonly brain:with no?) retains 'Omit = the router brain derived from the root profile — the unchanged default.' from the optional RunGraphOptions.brain doc. In RunGraphTestOptions (source src/runtime/supervise/graph.ts:279readonly brain: ToolLoopChat), omission is a type error, so the sentence describes an impossible state. The remaining copied clauses (mutually exclusive with driverBackend, external-harness refusal) DO hold on the wrapper path since runGraphWithTestBrain forwards into runGraphInternal which enforces both, so only the 'Omit =' sentence is wrong. Fix: drop the 'Omit =' clause
🟡 LOW runGraphWithTestBrain described as pure 'alias' but is a real thin wrapper — docs/api/testing.md
Summary says 'Alias for graph tests written before RunGraphOptions.brain was production ... this wrapper only keeps the /testing import path working.' It is behaviorally an alias (both it and runGraph call the same runGraphInternal), so this is accurate in substance, but it understates that the wrapper still exists as a real exported function preserving the brain-REQUIRED typing, and the primitive-catalog.md row truncates the same text mid-sentence ('The production'). Cosmetic; no correctness impact. Optional tightening: 'Thin forwarder for the /testing entry that keeps brain REQUIRED.'
🟡 LOW Comment says 'both arms' where it means both graph backend kinds — examples/p1-parity/arms.ts
The comment 'so both arms enter the same door' uses 'arm', which this file's header (lines 4-9) defines as the multishot-vs-graph A/B pair — but the multishot arm never calls runGraph. Inside runGraphArm 'both' actually means the seam and chat backend kinds. A wording nit in a file otherwise meticulous about its terminology; fix by writing 'both backend kinds enter the same door'. No behavior impact.
🟡 LOW ToolLoopChat type still imported from the /testing entry — examples/p1-parity/arms.ts
The seam arm now rides the production
RunGraphOptions.brain, but itsToolLoopChattype is still imported from../../src/testing(line 53). The type is now exported from the production kernel entry (src/runtime/index.ts:1025-1027), so the import can move to@tangle-network/agent-runtime/kernel, keeping the example fully off the test-only surface. Type-only import — no runtime coupling, purely cosmetic.
🟡 LOW Production path calls the 'TestBrain'-named constructor — src/runtime/supervise/graph.ts
With
brainnow a production option, the production graph path dispatches tosuperviseWithTestBrain(supervise.ts:1445, still commented 'Not exported from Runtime's main entry' and exported via/testing). The behavior is correct — it is a thin destructure intosuperviseInternal— but a production runtime path depending on a testing-named export is a naming/architecture smell that will confuse the next reader at exactly the seam this PR is about, and supervise.ts's 'test-only' doc onSuperviseTestOptions(supervise.ts:1429-1432) is now stale relative to this production caller. Fix in a follow-up: rename the internal constructor (e.g.superviseWithBrain) and keepsuperviseWithTestBrainas the/testingalias. supervise.ts is outside this shot's files, so no edit required here —
🟡 LOW brain + router is silently a no-op for the router — src/runtime/supervise/graph.ts
When both
brainandopts.routerare supplied,superviseOptionsstill forwardsrouter(line 998), andbuildSupervisorAgenteagerly callssnapshotRouterTransportConfig(supervisor-agent.ts:456) which validates and can throw on a malformed config — but then the brain arm is never used:brain = testBrain ?? routerBrainFromProfile(...)(supervisor-agent.ts:505) means a suppliedbrainalways wins and the router config is otherwise discarded. Thebraindoc comment covers driverBackend mutual-exclusion but is silent on this coexistence, so a caller passing both gets a router that is validated but ignored. Recommend either documenting it or mirroring
🟡 LOW brain + router is silently ignored instead of refused — src/runtime/supervise/graph.ts
opts.routeris forwarded into superviseOptions unconditionally (graph.ts:998), andsuperviseOptionsis built identically for both arms (graph.ts:1013-1019). Whenbrainis set, the caller brain REPLACES the router brain (brain = testBrain ?? routerBrainFromProfile(...), supervisor-agent.ts:505), so a caller supplying both arouterconfig and abrainhas the router config silently dropped — no error, no warning. This is the same class of 'two answers to WHO makes the root's calls' contradiction the PR deliberately fails loud on fordriverBackend(graph.ts:579) and for an external-harness root (graph.ts:584), butrouter+brainis not refused. Impact: a production caller can waste effort configuring a router substrate that is dead code, and debugging why the router is never co
🟡 LOW brain docstring omits profile axes that become refused under prompt-control-execution — src/runtime/supervise/graph.ts
The new
braindocstring (graph.ts:220-232) says the root profile 'keeps prompt control (prompt-control-executionmaterialization —systemPrompt/instructionsstill apply)', listing only what still applies. It does not state that under that contract the root profile'sdescription,version,tags,resources.instructions, andresourceFailOnErroraxes are now REFUSED: with a brain, supervise's root contract switches fromrouterSupervisorProfileMaterialization(supervise.ts:348, which carries those axes) topromptControlProfileMaterialization(supervise.ts:1672-1674; axes at profile-materialization.ts:161-174, which omits them), andassertProfileMaterializationthrows on any changed unsupported axis (profile-materialization.ts:252-256). This was pre-existing /testing behav
🟡 LOW router + brain is not refused; router config is validated then silently unused — src/runtime/supervise/graph.ts
The new guards refuse brain+driverBackend and brain+external-harness, but a caller passing both
opts.routerandopts.braingets no refusal: graph.ts:998 forwardsrouterinto superviseOptions, and in supervisor-agent.ts:505 the caller brain takes precedence sorouterBrainFromProfileis never called — the router transport is only snapshot-validated (supervisor-agent.ts:456-457) and then ignored.routeris documented in RunGraphOptions:218-219 as 'the driver brain's router substrate', so with a caller brain it is dead configuration a misconfigured caller will never learn about. Impact is mild (wasted config, no behavioral contradiction like driverBackend), which is why this is low. Fix: refusebrain && opts.routeralongside the driverBackend check at graph.ts:579, or note in the `
🟡 LOW runGraphWithTestBrain is now an untested back-compat alias — src/runtime/supervise/graph.ts
The wrapper is byte-identical to
runGraph(both destructurebrainand callrunGraphInternal). Its only remaining consumers are gone (examples migrated torunGraph), so no test exercises it; it is exported solely from /testing for back-compat. Harmless, but the alias andRunGraphTestOptionsnow exist only to keep external imports working and carry no coverage.
🟡 LOW Caller-brain seam's usage-metering divergence is unasserted — tests/kernel/graph.test.ts
The comparison test proves ledger/edge-journal/runId equality between the router arm and the caller-brain arm, but never asserts the one thing the production doc (src/runtime/supervise/graph.ts:226-229) says actually differs: per-turn usage reporting. The router arm's injected
completereturnsusage: { prompt_tokens: 7, completion_tokens: 3 }, while the caller arm'sscriptedBrain(driverDecisions)returns nousageat all. No assertion checks thatres.result.spent/token metering differs or that 'a brain that reports no usage meters nothing into the pool'. The seam is exercised for control-flow equivalence only; its metering contract is untested. Low severity because this is a coverage gap on a test-only shot, not a defect — but the test title/comment claims 'any ledger difference w
🟡 LOW Captured leaf profiles never asserted in the parity test — tests/kernel/graph.test.ts
runArm allocates
const received: AgentProfile[] = []and passes it to leafSeam(received) in both arms but never asserts on it, unlike every other describe block in the file that uses leafSeam's capture. Either drop the capture or add the cheap parity assertionexpect(receivedFromCaller).toEqual(receivedFromRouter)— it is the one worker-side observable the differential design could compare for free. Cosmetic; tests pass as-is.
🟡 LOW Equivalence test never asserts spentTotal — the one documented seam difference is unpinned — tests/kernel/graph.test.ts
The router arm's scriptedComplete reports
usage: { prompt_tokens: 7, completion_tokens: 3 }on every turn (metered into the conserved pool), while the caller arm's scriptedBrain(driverDecisions) reports no usage (meters nothing). The test compares ledger, exhaustedEdges, runId, result.out, and journal edgeEvents — but never spentTotal. The option doc (graph.ts:228-229) explicitly says a brain that reports no usage meters nothing, so this asymmetry is intended, but the 'row for row identical' claim would not catch a regression that accidentally stops metering the router arm. Add an assertion on both arms' result.spentTotal (they should differ today, or the caller brain should be given scripted usage to make them equal).
🟡 LOW No coverage of allowedModels/expectedModel interaction with a caller brain — tests/kernel/graph.test.ts
The RunGraphOptions.brain doc explicitly says model-selection moves to the caller and expectedModel cannot be enforced on a caller-placed call, but no test pins what happens when opts.allowedModels (forwarded at graph.ts:996-ish into superviseOptions) co-occurs with a brain — today it is presumably ignored by superviseWithTestBrain, and a future change that enforces or crashes there would land untested. One refusal-or-silent test in this block would close the gap. Not blocking: the four shipped tests cover the two documented refusals, the default, and the parity contract.
🟡 LOW Router arm has no call-count assertion on the injected transport — tests/kernel/graph.test.ts
scriptedComplete clamps
Math.min(turn, driverDecisions.length - 1), so any number of driver turns ≥3 returns 'done' with no tool calls. Both arms clamp identically, so the test proves the transport was consumed (otherwise the run would fail against http://injected.invalid) but not that it was called exactly the 3 scripted decisions. A driver-loop over-call would be masked symmetrically in both arms. Pin an invocation counter (expect the router transport to be called exactly 3 times) to make the equivalence stronger.
🟡 LOW runArm captures received but never asserts it — tests/kernel/graph.test.ts
The per-arm
received: AgentProfile[]from leafSeam is populated but never asserted, so a seam leak that changed the worker profile delivered to the leaf (vs. the pinned node profile) would go undetected unless it changed ledger bytes or workerId. Assert both arms' received arrays equal and match the pinned 'worker' profile (systemPrompt 'You build what the driver asks.' + the delegates/worker-brief/v1 directive) — the existing 2-node acceptance test at line 299-301 already shows the pattern.
tangletools · 2026-08-15T12:56:17Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 17 non-blocking findings — e5e58d9f
Full multi-shot audit completed 5/5 planned shots over 11 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 11 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 11 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-08-15T12:56:17Z · immutable trace
Summary
Executes the #694 option-A ruling: a caller-supplied root driver brain is expressible on the PRODUCTION
runGraphsurface.RunGraphOptions.brainaccepts a caller-ownedToolLoopChaton the/kernelentry.The root driver's inference becomes caller data, which is the seam the multishot→
runGraphconsumer migrations require (tax-agent and gtm-agent phase-4 reports, 2026-08-15): strict-alternation driver orchestration must be caller-owned for the recorded artifacts to stay byte-compatible.The graph machinery around the seam — node pinning, directive delivery, the edge ledger, the journal twin — is the same shipped path.
The contract
brain= the unchanged router-brained default. The root model call derives from the rootAgentProfileexactly as before. The refusal for a router-brained root with neither brain nor router config still fires (asserted).prompt-control-executionmaterialization —systemPrompt/instructionsstill apply). Model selection, provider-identity validation (expectedModel), and per-turn usage reporting move to the caller.brain+driverBackend(two answers to who makes the root's model calls), andbrainon a root whose profile declares an external harness (the harness IS that root's brain — a supplied one would have been silently ignored downstream).supervise/supervisorAgentkeep refusing direct brain injection. The graduated surface is the graph root only, per the ruling./kernelre-exportsToolLoopChat/ToolLoopCallContextso a consumer types its brain from the production entry./testingentry (runGraphWithTestBrain,RunGraphTestOptions) keeps working as an alias.The proof (STABILITY.md graduation bar)
tests/kernel/graph.test.ts, new describe "the caller-brain seam on the production surface (feat(kernel): agent graphs — profiles as nodes, optimizable directives as edges (subsumes the driver/loop/supervisor/multishot families) #694 option A)":winner, same output), and the edge ledger plus its journaledge-event twin are asserted DEEP-EQUAL row for row. The only differences are the brain source itself (who made the calls, usage metering).docs/canonical-api.md: new/kerneltable rowrunGraph(graph, { brain }).examples/graphs/user-sim-conversation.ts(the multishot-shaped persona⟷agent conversation) now runs on the production surface end-to-end, verified against the builtdist/:brain: ToolLoopChatis unchanged since before 0.128.0; only its placement (test-only → production) moves. refactor(runtime): enforce canonical model execution boundary #736 (2026-08-04) demoted the placement; this PR is the ruled reversal for the graph root.docs/apiregenerated (pnpm docs:api);docs:freshnesspasses.Checks
pnpm typecheck(src + examples): clean.pnpm vitest run tests/kernel/graph.test.ts: 50 passed.Refs #694 (option A ruling, comments 2026-08-15).