From e0788a467543c5e7c87761a5fe91b54eec86de85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Wed, 20 May 2026 11:03:15 +0300 Subject: [PATCH] fix(observer): infer blocked from unrecovered_error tail, not raw error/retry count (A-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: inferOutcome flagged `blocked` whenever errorCount > retryCount across the turn's events. But the parser emits an `error` event for ANY tool_result with is_error=true — including expected failures: TDD failing-test-first, grep returning nothing, git commands with intentional non-zero exit. On TDD-heavy turns (project's standard discipline) this systematically marked turns as blocked even when they ended on a successful tool_use. Fix: - Parser (extractProcessEvents): walk turn from end, find the LAST tool_result; if its is_error=true, emit a single `unrecovered_error` event. Distinguishes "turn ended on failure" from "errors recovered later". The original per-is_error `error` events remain (useful as raw factor signals). - Analyzer (inferOutcome): replace `errorCount > retryCount → blocked` with `events.some(kind === 'unrecovered_error') → blocked`. Same ordering preserved (interrupt > blocked > rework/success/unknown). Tests: - Parser: emits unrecovered_error when last tool_result is_error; does NOT emit when turn ended on a successful tool_result; does NOT emit for turns with no tool_results. - Analyzer: blocked iff unrecovered_error event present (not raw count); events=[error, error, retry] → success (no unrecovered_error). 142/142 vitest green (was 128). Co-Authored-By: Claude Opus 4.7 (1M context) --- tools/brain-retro-analyzer.mjs | 12 ++++++----- tools/brain-retro-analyzer.test.mjs | 12 +++++++++-- tools/observer-transcript-parser.mjs | 21 +++++++++++++++++++ tools/observer-transcript-parser.test.mjs | 25 +++++++++++++++++++++++ 4 files changed, 63 insertions(+), 7 deletions(-) diff --git a/tools/brain-retro-analyzer.mjs b/tools/brain-retro-analyzer.mjs index 35c4b60a..0f6da62a 100644 --- a/tools/brain-retro-analyzer.mjs +++ b/tools/brain-retro-analyzer.mjs @@ -32,11 +32,13 @@ export function inferOutcome(episode, nextEpisode) { if (events.some((e) => e.kind === 'interrupt')) { return 'partial'; } - // A turn that hit more tool errors than it retried away ended on an - // unrecovered failure — the work was blocked, not merely reworked later. - const errorCount = events.filter((e) => e.kind === 'error').length; - const retryCount = events.filter((e) => e.kind === 'retry').length; - if (errorCount > retryCount) { + // A turn is `blocked` only when it ENDED on an unrecovered tool failure — + // emitted by the parser as a single `unrecovered_error` event when the + // LAST tool_result of the turn was is_error=true. Raw error/retry counts + // do NOT imply blocked: a TDD red→green cycle or a grep that returns + // nothing both surface as `error` events but are intentional and + // recovered — counting them as blocked over-reports failures (A-1 fix). + if (events.some((e) => e.kind === 'unrecovered_error')) { return 'blocked'; } // 'failure' (work wrong AND never corrected) is a judgment, not diff --git a/tools/brain-retro-analyzer.test.mjs b/tools/brain-retro-analyzer.test.mjs index acdc8776..c2ab5259 100644 --- a/tools/brain-retro-analyzer.test.mjs +++ b/tools/brain-retro-analyzer.test.mjs @@ -53,10 +53,18 @@ describe('inferOutcome', () => { it('infers unknown when there is no next episode', () => { expect(inferOutcome(ep(), null)).toBe('unknown'); }); - it('infers blocked when the episode has more error than retry events', () => { - const blocked = ep({ events: [{ kind: 'error' }, { kind: 'error' }, { kind: 'retry' }] }); + it('infers blocked ONLY when an unrecovered_error event is present (turn ended on error)', () => { + const blocked = ep({ events: [{ kind: 'error' }, { kind: 'error' }, { kind: 'unrecovered_error' }] }); expect(inferOutcome(blocked, ep({ prompt_signal: 'approval' }))).toBe('blocked'); }); + it('does NOT infer blocked from raw error/retry count (TDD failing-test-first is not a block)', () => { + // A turn with N errors + N retries that ends on a successful tool_result — + // e.g., TDD red→green, or git command that legitimately fails then recovers — + // must NOT count as blocked. The parser emits unrecovered_error iff the LAST + // tool_result was is_error, which is absent here. + const recovered = ep({ events: [{ kind: 'error' }, { kind: 'error' }, { kind: 'retry' }] }); + expect(inferOutcome(recovered, ep({ prompt_signal: 'approval' }))).toBe('success'); + }); it('does not infer blocked when every error was retried', () => { const recovered = ep({ events: [{ kind: 'error' }, { kind: 'retry' }] }); expect(inferOutcome(recovered, ep({ prompt_signal: 'approval' }))).toBe('success'); diff --git a/tools/observer-transcript-parser.mjs b/tools/observer-transcript-parser.mjs index 6b03b08d..9d2681ba 100644 --- a/tools/observer-transcript-parser.mjs +++ b/tools/observer-transcript-parser.mjs @@ -325,6 +325,27 @@ export function extractProcessEvents(turn, broken, total, durationMs) { events.push({ kind: 'parse_gap', broken, total }); } + // unrecovered_error: emitted iff the LAST tool_result in the turn was + // is_error=true. Distinguishes "turn ended on failure" from "errors that + // were retried away" (e.g., TDD red→green, expected-fail commands). The + // analyzer uses this event to flag `blocked` instead of raw error/retry + // count — see brain-retro-analyzer.inferOutcome (A-1 fix). + let lastToolResultIsError = null; + outer: for (let i = turn.length - 1; i >= 0; i--) { + const content = + turn[i] && turn[i].message && Array.isArray(turn[i].message.content) ? turn[i].message.content : []; + for (let j = content.length - 1; j >= 0; j--) { + const b = content[j]; + if (b && b.type === 'tool_result') { + lastToolResultIsError = b.is_error === true; + break outer; + } + } + } + if (lastToolResultIsError === true) { + events.push({ kind: 'unrecovered_error' }); + } + return events; } diff --git a/tools/observer-transcript-parser.test.mjs b/tools/observer-transcript-parser.test.mjs index ccd0cfaa..7e14cb4f 100644 --- a/tools/observer-transcript-parser.test.mjs +++ b/tools/observer-transcript-parser.test.mjs @@ -382,6 +382,31 @@ describe('extractProcessEvents', () => { it('emits nothing for a clean empty turn', () => { expect(extractProcessEvents([], 0, 0, 0)).toEqual([]); }); + + it('emits unrecovered_error when the LAST tool_result in the turn is is_error', () => { + const turn = [ + { message: { role: 'assistant', content: [{ type: 'tool_use', id: 'u1', name: 'Bash', input: {} }] } }, + { message: { role: 'user', content: [{ type: 'tool_result', tool_use_id: 'u1', is_error: true }] } }, + ]; + expect(extractProcessEvents(turn, 0, 0, 0).filter((e) => e.kind === 'unrecovered_error')).toHaveLength(1); + }); + + it('does NOT emit unrecovered_error when the turn ends on a successful tool_result', () => { + const turn = [ + { message: { role: 'assistant', content: [{ type: 'tool_use', id: 'u1', name: 'Bash', input: {} }] } }, + { message: { role: 'user', content: [{ type: 'tool_result', tool_use_id: 'u1', is_error: true }] } }, + { message: { role: 'assistant', content: [{ type: 'tool_use', id: 'u2', name: 'Bash', input: {} }] } }, + { message: { role: 'user', content: [{ type: 'tool_result', tool_use_id: 'u2', is_error: false }] } }, + ]; + expect(extractProcessEvents(turn, 0, 0, 0).filter((e) => e.kind === 'unrecovered_error')).toHaveLength(0); + }); + + it('does NOT emit unrecovered_error for a turn with no tool_results at all', () => { + const turn = [ + { message: { role: 'assistant', content: [{ type: 'text', text: 'just talking' }] } }, + ]; + expect(extractProcessEvents(turn, 0, 0, 0).filter((e) => e.kind === 'unrecovered_error')).toHaveLength(0); + }); }); describe('parseRoutingTag', () => {