fix: preserve analyst outcomes and stop authority in Jev-compatible execution - #1283
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The branch's only CI failure is `biome check src tests examples` on tests/kernel/jev-composition.test.ts: one import list out of order and object literals the formatter would print differently. Applied with `biome check --write` rather than by hand, so the result is what CI computes rather than what I guessed it wants. biome check across all 729 files: clean. jev-composition.test.ts: 3/3.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 149c52e1
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-09-18T19:06:06Z
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 | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 195.0s (2 bridge agents) |
| Total | 195.0s |
💰 Value — sound-with-nits
Adds a passing deterministic regression suite plus a guidance doc proving 'Jev on existing seams' — registry analysts through the real graph path, beforeTurn→compaction→inference ordering, and caller-owned brain composition — with no runtime changes; good and in-grain, with one tautological test.
- What it does: Adds tests/kernel/jev-composition.test.ts (3 tests, all verified passing locally via
pnpm exec vitest run) and docs/jev.md. Test 1 registers a fixture analyst on a real agent-eval AnalystRegistry, adapts it via analystsFromRegistry (src/runtime/supervise-surface.ts:126), and runs it through the actual runGraphWithTestBrain analyzes edge of the shotLoop example, asserting the shared CostLedger/co - Goals it achieves: Makes the 'native decisions compose on existing seams, not a new runtime' policy executable and regression-guarded: the eval-registry→graph→ledger path, cost-ledger identity propagation, beforeTurn/compaction ordering, and cancellation/identity preservation through an explicit brain. It also documents the integration contract for the related agent-eval/router/dev-container changes (billing limits,
- Assessment: Good on its merits. I ran the suite: 3/3 pass against the installed @tangle-network/agent-eval 0.182.0 (which does export /analyst and carries costLedger/costPhase in its context types). The coverage fills real gaps, not duplicates: beforeTurn had zero test coverage anywhere (only production use at src/runtime/supervise/coordination-driver.ts:1253); the registry→graph composition is tested elsewhe
- Better / existing approach: none — this is the right approach. Searched for prior art: analystsFromRegistry consumers (only supervise-surface.test.ts lens-level tests and a comment in supervise-option-capture.test.ts), analyzes-edge coverage (graph.test.ts, graph-topologies.test.ts — inline registries only), beforeTurn usage (src-only), compaction tests (tool-loop-compaction.test.ts covers mechanics but not hook ordering). E
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error event without a message: {"type":"error","timestamp":1789761906509,"sessionID":"ses_f49e09a0bffe6BTdSUbRa3ltee","error":{"name":"UnknownError","data":{"message":"Unexpected server error. Check server logs for details.","ref":"err_c93cd92e"}}}
🎯 Usefulness — sound-with-nits
A passing, genuinely additive composition regression test that routes the real agent-eval AnalystRegistry through analystsFromRegistry into runGraph's analyzes edge with cost-ledger context assertions, plus first-ever beforeTurn→compaction ordering coverage; one of its three tests is self-referentia
- Integration: Verified reachable and real, not dead surface. Every import resolves to a live seam: AnalystRegistry/CostLedger from pinned @tangle-network/agent-eval@0.182.0 (package.json:134), analystsFromRegistry (src/runtime/supervise-surface.ts:126), runGraph's analysts option and analyzes edge ledger (src/runtime/supervise/graph.ts:442,150-152), runBrainLoop hooks.beforeTurn (src/runtime/tool-loop.ts:208),
- Fit with existing patterns: Follows the established tests/kernel pattern exactly (scripted brains, deterministic fixtures, ledger assertions — same style as tests/kernel/graph.test.ts:590-636 and tests/kernel/tool-loop-compaction.test.ts). It does not compete with or duplicate existing coverage: graph.test.ts tests analyzes edges with a hand-rolled lens (graph.test.ts:591-594), supervise-surface.test.ts tests the adapter in
- Real-world viability: Deterministic, no network, fast (34ms of test time), so it will hold in CI. The first test fails meaningfully if the adapter, edge naming, or context propagation regresses (analystsFromRegistry throws on unregistered kinds at supervise-surface.ts:137-141, so it can't silently pass). The second test pins real runBrainLoop ordering. The third does not: see finding. Error paths like abort mid-graph a
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 Third test is tautological — it exercises no runtime code [maintenance] ``
tests/kernel/jev-composition.test.ts:126-146 defines both
transportandbraininside the test and calls them directly;runBrainLoopis never invoked in that test, so any regression in src/runtime/tool-loop.ts leaves it green. It only demonstrates JS semantics (frozen-object passthrough, throwIfAborted after abort). It works as an executable mirror of the docs/jev.md:113-122 brain snippet, but as a 'composition proof' it pins nothing about the system. Either route the wrapper through a real
🎯 Usefulness Audit
🟡 Third test is self-referential: it invokes only its own closures, no runtime code [problem-fit] ``
tests/kernel/jev-composition.test.ts:126-146 defines
brainandtransportlocally and callsbrain([], [], context)directly; nothing from src/ executes except AbortController semantics. It asserts that the test's own closure forwards a frozen argument and that throwIfAborted throws after abort — true regardless of runtime behavior, so it passes even if the runtime regressed caller-owned cancellation. The real contract (frozen ToolLoopCallContext built at src/runtime/supervise/coordination-d
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.
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 149c52e1
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-09-18T20:27:13Z
…of real loop regressions
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — cb194c39
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-09-18T21:09:42Z
Changes
analystsFromRegistry→ graph/supervisor path. No new runtime, agent class, scheduler, or hook bus.Verification
Full CI passed for head
cb194c39511f3f14013bc8374c816f936c320da2:https://github.com/tangle-network/agent-runtime/actions/runs/35394548837
Passed: lint, full tests, build, source/examples typecheck, packed exports, docs freshness, exact first-party package cohort, official optimizer integrations, and agent-bench.
The first benchmark attempt failed a temporary-checkout fixture with
ENOENTunder/tmp/swe-cache-copy-*/destination/.git/objects. A rerun passed without weakening assertions or changing benchmark behavior. The intermittent filesystem cause remains unproven; this is not presented as a benchmark bug fix.Composition
Use
jevEvaluatoror an ordinary typed evaluator where a bounded decision is useful. Use Eval'sjevAnalyst/asAnalystfor graph analysis. Retain registry summaries through existing observation/persistence hooks for diagnostics and spend attribution; function-shaped analysts still need the shared paid-call account.Related: agent-eval #761; tangle-router #533 (native endpoint #531 is merged); agent-dev-container #7620. Production consumers need the released Eval adapter. No publish, merge, deployment, or funded live-model test performed.