fix(eval): make opencode NDJSON output parseable + gate only latest eval round

[JeiKeiLim]

Summary

Two bugs discovered while investigating a production run. Both produced the same symptom — a report-only parent stuck in blocked_on_finding with no path forward — and were misdiagnosed in the report as a broken auto-resume gate.

Bug 1: opencode critic verdicts were unparseable (root cause of most "auto-resume never fires")

OpenCodeAdapter returned the raw NDJSON event stream from opencode run --format json as AgentResponse.output. extractRubricJson cannot parse NDJSON — it tries JSON.parse on the whole stream and falls back to slicing between the first { and last } (which spans multiple events and is never valid JSON). Result: every opencode critic's verdict parsed as NULL, so the blocking-finding auto-resume gate correctly-but-permanently refused to unblock.

Verified against the production DB (1102 opencode eval jobs): 0/1102 parseable, vs claude-code 379/477 and codex 289/487 parseable-to-PASS. The 5 successful auto-resumes in the same DB all used claude-code.

Fix: the adapter now collapses the NDJSON stream into its text event parts (all assistant text events, including intermediate reasoning + the trailing verdict; note claude --print returns only the final message, so multi-turn sessions differ), with a raw-stdout fallback and truncated-tail tolerance. Verified against the stored run: 12/14 previously-unreadable verdicts now parse (the 2 remaining genuinely never emitted a rubric — visible for the first time via tenet_job_result).

Bug 2: stale critic rows from earlier eval rounds poisoned the gate

When a child dev job is retried, the orchestrator re-fires tenet_start_eval, which creates new critic rows but leaves prior rounds' rows in place. getEvalsForSource returns all rounds, and the gate required EVERY sibling to be completed + passed:true. A failed critic in round 1 poisons rounds 2 and 3 forever (e2e-1 in the run: round-1 interaction_e2e FAIL(false) blocked a fully-green round 3).

Fix: tenet_start_eval now stamps every critic in one dispatch with a shared eval_round id; a re-fire gets a fresh id. The resume gate keys on the newest complete round — every critic in a round evaluated the same source state, so requiring the whole round to pass avoids mixing verdicts across code revisions (the "green gate, still wrong code" failure). The gate runs whenever any stamped round exists; unstamped siblings (ad-hoc re-fires, legacy pre-stamp evals) become singleton rounds, so a NEWER unstamped critic forces the gate to wait for a fresh stamped round (fail-closed) instead of being invisible — a red ad-hoc re-fire cannot be ignored while the parent unblocks on an older round's stale green. Older rounds stay in the DB (history preserved) but no longer count.

Rubric extraction hardening (shared parser)

extractRubricJson moved to a shared module (src/core/rubric.ts) used by both the resume gate and tenet_get_status, so the two consumers of the same stored critic output can't drift apart. The parser is a single rightmost-object walk:

• walks { positions from the end (the preamble mandates the verdict at the END), parses each to its matching }, and returns the rightmost top-level object with a boolean passed key, preferring one that also carries a stage key (tool-result echoes rarely carry stage, so a failing critic that pastes a passing tool result after its verdict can't false-green the gate)
• rejects objects nested inside a valid JSON object or array (a finding or tool echo is never the verdict)
• recovers from unbalanced braces in prose (a stray { before the verdict no longer strands the parent)
• tenet_get_status uses the same parser, so latest_e2e_status survives prose braces
• validated against the production DB: a golden test of 12 real critic outputs (including a case the old parser got wrong — an unmatched quote in prose false-rejected a valid verdict)

Tests

• NDJSON collapse: verdict extraction, truncated tail, raw fallback, stream-order join
• Rubric parser: fenced-echo, echoed-verdict, unbalanced-brace, bracket-depth, stage-preference, custom-critic shape
• Round gate: C3 (stale failed critic doesn't block a green round), C4 (regression), C5 (round 2 missing a stage doesn't mix with round 1 — the discriminator), C6 (sequential chaining), C7 (job-level failed critic in newest round stays blocked), C8 (ad-hoc unstamped critic can't disable the gate), C9 (legacy unstamped critic can't unblock)
• Status surfacing: D4 (prose braces still surface layer2_status)
• typecheck, lint, and all 301 tests pass

[JeiKeiLim]

Full version of the follow-up review (head 5514dd9) — the Slack delivery of this got length-truncated, so posting the complete text here for the record.

---

I've read the final state at head SHA 5514dd9 (the branch-name raw fetch briefly served a stale copy of the adapter — the SHA-pinned fetches confirm the exported, guarded final version). No terminal in that review session, so I couldn't clone and run the 271 tests myself — I'll say that plainly and lean on the diffs, the fixture, and the test summaries I could pull.

---

PR #21 review — fix(eval): opencode NDJSON parseable + gate only latest eval round

Verdict: approve with nits. Both root causes are real, both fixes sit at the correct layer, and the tests target the actual failure modes. Also: 0/1102 parseable opencode verdicts is a beautiful number — by which I mean horrifying.

Bug 1 — extractTextPartsFromNdjson (src/adapters/opencode-adapter.ts)

• Normalizing at the adapter boundary to match the claude --print shape is the right call. Teaching extractRubricJson to speak NDJSON would have welded the parser to a second vendor's format.
• Like: schema-pin comment, null → raw-stdout fallback, truncated-tail tolerance, unknown casts + runtime guards. And the degradation direction is fail-safe: shape change → NULL verdict → gate blocks. It never wrongly unblocks.
• Design note: you join all text events across the whole session, not just the final message. The verdict survives via extractRubricJson's {-to-} slice, but output now carries mid-run chatter ("Running the zero-findings recheck...") into job_result/worker-facing surfaces. If that noise matters, collapse only the last message group (text events after the final step_start/before step_finish reason:"stop").
• Inherited fragility you're now amplifying: extractRubricJson slices first { → last }. More joined prose = higher odds an earlier text event contains braces (code snippets!), making that slice span invalid JSON → NULL verdict → same stall, new costume. Follow-up worth doing while you're in this seam: scan for the last balanced {...} object instead of first/last slice. Not a blocker — claude has the same hole — but this PR is standing right next to it.

Bug 2 — gate (src/core/job-manager.ts)

• Per-stage-newest fixes the poisoning correctly; resolveExpectedEvalStages reading the newest roster stamp is the right authoritative source. History preserved, gate semantics fixed. Good.
• The >= tie-break's correctness depends on evalSiblings iteration order. "Later-seen wins" only holds if getEvalsForSource returns rows deterministically — if unordered, equal-ms ties are a coin flip. Sort by createdAt ascending (secondary: id) before the loop, and the comment's promise becomes actually true.
• new Map() is Map<any, any> — still untyped at head. Map<string, Job> catches the '' bucket's mistakes at compile time.
• Speaking of the '' bucket: any sibling without eval_stage collapses into one anonymous stage, newest wins. Previously all counted. Verify nothing legitimately fires eval without eval_stage, or this silently weakens the gate for stage-less jobs.
• The design concern in the thread stands and is the one real hole: per-stage newest can mix verdicts across source revisions — round-3 code_critic PASS + round-2 test_critic PASS = a composite verdict no single round ever produced, unblocking the parent on code the round-2 critic never saw. Strictly better than forever-blocked, but the round-id stamp + "newest round with complete coverage" follow-up is the right shape. Don't let it rot on the board — this is exactly the "green gate, still wrong code" incident generator.

Tests

• C3 with the maxUses: 1 trick is exactly how you write that regression deterministically. Red-verified per description, green in CI.
• Stream-order join, truncated tail, non-event JSON lines, raw fallback — good collapse coverage. The fixture now has two text events, which closes the earlier single-event gap.
• Nit: the E2E rubric test mirrors extractRubricJson inline instead of importing it. Mirrors drift. Export the real function and import it, or lean on the C-series integration path that exercises the real parser end-to-end.
• Missing one cheap case: a text event after the verdict that contains braces → pins down that the {-} slice still lands on the verdict.

Couldn't verify: no terminal in that session — didn't run the 271 tests or clone the branch; relying on green CI + the documented C3 red/green run. Reviewed against head 5514dd9d1 via SHA-pinned raw fetches.

Ship it after the Map typing + tie-break sort. Put the round-id stamp on the follow-up board, and keep the brace-slice fragility on your radar. Nice root-cause work — this is what "blocked_on_finding with no path forward" actually was.

[JeiKeiLim]

Follow-up review (head 5514dd9) addressed in ffd4cdb:

Fixed:

1. ✅ Mirror drift — extractRubricJson is now exported from job-manager.ts and the end-to-end test imports the real parser instead of an inline mirror.
2. ✅ Brace-slice fragility test — new test: a text event after the verdict containing braces (src/foo.ts etc.) must not break the {-} slice; asserts the verdict still parses. Pinned as a regression test rather than just a radar note.

Verified against the code (no change needed):

1. Map typing — already new Map<string, Job>() at job-manager.ts:1046; the '' bucket is excluded by the expectedStages.has(stage) filter that runs before dedup.
2. Tie-break determinism — getEvalsForSource is ORDER BY created_at ASC (state-store.ts:903), so iteration order is deterministic and the >= tie-break's "later-seen wins" promise holds. No sort needed.

Agreed, tracked as follow-up (not in this PR):

• Round-id stamp so the gate can key on "newest round with complete coverage" instead of per-stage mixing — the one real design hole.
• The {-} slice fragility in extractRubricJson itself (last-balanced-object scan) — now covered by a regression test, but the deeper fix is a separate change.