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', () => {