fix(analyst-loop): forward same-run findings - #658
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 5944cb23
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.
tangletools · auto-approval · reason: drewstone_author · 2026-07-29T20:08:17Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 102.0s (2 bridge agents) |
| Total | 102.0s |
💰 Value — sound
Forwards agent-eval's existing opt-in same-run finding-chain option through runAnalystLoop, which was previously swallowing it; clean, minimal, defaults to off, tested against the real registry.
- What it does: Exposes the
chainFindings?: booleanoption onRunAnalystLoopOptsand threads it into theregistryOptionsobject passed toregistry.run/registry.runStream. When on, the underlyingAnalystRegistrypopulates each later analyst'sctx.upstreamFindingswith findings emitted by earlier analysts in the same run (registration order = dependency order). When off (the default), behavior is unch - Goals it achieves: Close a real gap: the orchestrator was constructing the registry options object internally and silently dropping
chainFindings, so callers had no way to turn on same-run finding chaining throughrunAnalystLoopeven though the underlying registry fully supported it (agent-eval accumulates findings in registration order and populatesctx.upstreamFindingswhen the flag is set, with dedicated `r - Assessment: Good change. It is a minimal, correct forwarding that matches the exact established grain in the same object literal (
...(opts.x ? { x: opts.x } : {})), defaults to off to preserve existing behavior, and adds the option to the narrowed registry type shapes so the typed contract stays honest. The two new tests use the REALAnalystRegistry(not the stub) to prove both the on-path (second analyst - Better / existing approach: none — this is the right approach. Verified
chainFindings/upstreamFindingsis the real, mature capability in agent-eval (default-registry-Brxr728w.d.ts:230 RegistryRunOpts.chainFindings; default-registry-CHmdy2An.js:2088 populates ctx.upstreamFindings from an accumulating allFindings array; renderUpstreamFindings exists at d.ts:139). There is no pre-existing passthrough in runAnalystLoop — it - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
A minimal, opt-in pass-through that exposes agent-eval's native same-run finding chain through the orchestrator, tested against the real registry — coherent, non-competing, and reachable.
- Integration: Reachable and correctly wired. run-analyst-loop.ts:148 forwards opts.chainFindings into the curated registryOptions object alongside priorFindings/costLedger/signal, using the same conditional-spread idiom. The underlying agent-eval AnalystRegistry natively consumes chainFindings and populates ctx.upstreamFindings (confirmed in node_modules/@tangle-network/agent-eval/dist/default-registry-Brxr728w
- Fit with existing patterns: Fits the grain precisely. It mirrors the established forwarding pattern for the sibling priorFindings option and the other curated registry opts; the orchestrator deliberately curates which fields cross into the registry rather than blanket-spreading (since RunAnalystLoopOpts carries loop-only fields like findingsStore/baselineRunId that must not reach the registry), and this change respects that
- Real-world viability: Holds up because the orchestrator's role here is pure pass-through; the hard parts (sequential/dependency-ordered execution, populating upstreamFindings, handling a failed earlier analyst) are owned by the agent-eval registry, an established component. The flag defaults off, so every existing analyst suite keeps identical behavior — zero regression surface. The forwarding guard `opts.chainFindings
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
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 —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 3 non-blocking findings — 5944cb23
Full multi-shot audit completed 4/4 planned shots over 6 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-07-29T20:13:24Z · immutable trace
Summary
Verification