From 555efef53d157183b25231b0ec7949666a29174d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 18 Oct 2026 15:45:17 +0200 Subject: [PATCH] fix(story-mode): name the check that failed, say whose plan a failure was, and when background work finished MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review round 9: - "another check" was too vague to act on. A check other than the tests is named by its tool: a layout check (a formatter), a style check (a linter), a type check, the build. "Fix and test, 2 rounds: a test fails and Claude fixes it, then a layout check fails and Claude fixes that; everything passes." - A failure the prompt asked for reads "The first run fails on purpose, as you asked; the second passes." ("The tests don't even load, as you asked."); one Claude planned itself keeps "Test first: … as planned". - A test run sent to the background that reported in a later turn reads "Claude starts the tests in the background; later they pass.", not as if they passed in that turn. - The test board says "1 test" and, when nothing loaded, "1 error". Co-Authored-By: Claude Opus 5.5 (1M context) --- src/story-mode.ts | 68 +++++++++++++------ src/web/public/story-mode.js | 9 ++- .../background-helpers-reports.expected.json | 2 +- .../errors-push-append.expected.json | 2 +- .../failing-test-on-purpose.expected.json | 4 +- .../long-turn-prompt-added.expected.json | 2 +- .../python-cli-fix-loop.expected.json | 2 +- test/story-corpus.test.ts | 8 ++- test/story-mode.test.ts | 4 +- 9 files changed, 66 insertions(+), 35 deletions(-) diff --git a/src/story-mode.ts b/src/story-mode.ts index 48b2e880..0e251356 100644 --- a/src/story-mode.ts +++ b/src/story-mode.ts @@ -2259,14 +2259,26 @@ function printableLine(line: string): string | null { return clip(text.replace(/[\w.+-]+@[\w-]+(\.[\w-]+)+/g, '…@…').replace(/[A-Za-z0-9+/=_-]{32,}/g, '…'), 90); } +/** What a check other than the tests is, by its tool, for a reader: "a layout check", "a style check". */ +function checkName(e: StoryEvent | undefined): string { + const cmd = e?.label ?? ''; + if (/\b(ruff format|black|prettier|gofmt|rustfmt|isort|autopep8|biome format)\b/.test(cmd)) return 'a layout check'; + if (/\b(ruff|eslint|flake8|pylint|clippy|shellcheck|golangci-lint|stylelint|biome)\b/.test(cmd)) + return 'a style check'; + if (/\b(tsc|mypy|pyright|vue-tsc)\b/.test(cmd)) return 'a type check'; + if (/\b(build|make|cargo build|go build|webpack|vite build|esbuild)\b/.test(cmd)) return 'the build'; + return 'another check'; +} + /** You asked for a failing run first (test first), or Claude said the failure was expected. */ const PLANNED_IN_PROMPT = /\b(test[- ]first|tdd|fail(s|ing)? (first|on purpose)|failing tests? first|on purpose|red first|before (the )?(code|module|function|implementation) exists)\b/i; const EXPECTED_BY_CLAUDE = /\b(as expected|as planned|as intended|expected (failure|to fail|red)|deliberately fail\w*|on purpose|test[- ]first|the red state)\b/i; -function plannedFailure(turn: StoryTurn, events: StoryEvent[]): boolean { +function plannedFailure(turn: StoryTurn, events: StoryEvent[]): false | 'asked' | 'planned' { const prompt = events.find((e) => e.intent === 'prompt')?.label ?? turn.prompt; - return PLANNED_IN_PROMPT.test(prompt) || events.some((e) => e.intent === 'say' && EXPECTED_BY_CLAUDE.test(e.label)); + if (PLANNED_IN_PROMPT.test(prompt)) return 'asked'; + return events.some((e) => e.intent === 'say' && EXPECTED_BY_CLAUDE.test(e.label)) ? 'planned' : false; } function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, index: number): StoryPanel { @@ -2451,27 +2463,35 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde const n = panel.rounds; const testFails = fails - lintFails; // A failure you asked for (test first), or that Claude said it expected, is the plan working. - const planned = plannedFailure(turn, events) && passed(checks[0]) === false; + const planned = passed(checks[0]) === false ? plannedFailure(turn, events) : false; + // The check that failed while the tests passed, by name: "a layout check", not "another check". + const otherCheck = checkName( + failed.find((e) => !!e.tests && e.tests.passed > 0 && !e.tests.failed && !e.tests.errors) + ); story = panel.ok === true && !fails ? `Build and test, ${plural(n, 'round')}. Every check passes.` : panel.ok === true && fails === lintFails - ? `Build and test, ${plural(n, 'round')}. The tests pass every time; another check fails ${fails === 1 ? 'once' : `${fails} times`}, then passes.` - : panel.ok === true && planned && checks.length === 2 - ? 'Test first: the first run fails as planned, the second passes.' - : panel.ok === true && planned - ? `Test first, ${plural(n, 'round')}: the first run fails as planned, then it passes.` - : panel.ok === true && checks.length === 2 && fails === 1 - ? 'Fix and test: the first run fails, the second passes.' - : panel.ok === true && testFails && lintFails - ? `Fix and test, ${plural(n, 'round')}: ${testFails === 1 ? 'a test run fails' : `${testFails} test runs fail`}, then another check, then everything passes.` - : panel.ok === true - ? `Fix and test, ${plural(n, 'round')}: ${fails === 1 ? 'one run fails' : `${fails} runs fail`}, then it passes.` - : panel.ok === false && tests && tests.passed && !tests.failed && !tests.errors - ? `Fix and test, ${plural(n, 'round')}. The tests pass, but another check fails.` - : panel.ok === false - ? `Fix and test, ${plural(n, 'round')}. Still failing.` - : `Fix and test, ${plural(n, 'round')}.`; + ? `Build and test, ${plural(n, 'round')}. The tests pass every time; ${otherCheck} fails ${fails === 1 ? 'once' : `${fails} times`}, then passes.` + : panel.ok === true && planned === 'asked' + ? checks.length === 2 + ? 'The first run fails on purpose, as you asked; the second passes.' + : `${plural(n, 'round')}: the first run fails on purpose, as you asked, then it passes.` + : panel.ok === true && planned && checks.length === 2 + ? 'Test first: the first run fails as planned, the second passes.' + : panel.ok === true && planned + ? `Test first, ${plural(n, 'round')}: the first run fails as planned, then it passes.` + : panel.ok === true && checks.length === 2 && fails === 1 + ? 'Fix and test: the first run fails, the second passes.' + : panel.ok === true && testFails && lintFails + ? `Fix and test, ${plural(n, 'round')}: ${testFails === 1 ? 'a test fails' : `tests fail ${testFails} times`} and Claude fixes it, then ${otherCheck} fails and Claude fixes that; everything passes.` + : panel.ok === true + ? `Fix and test, ${plural(n, 'round')}: ${fails === 1 ? 'one run fails' : `${fails} runs fail`}, then it passes.` + : panel.ok === false && tests && tests.passed && !tests.failed && !tests.errors + ? `Fix and test, ${plural(n, 'round')}. The tests pass, but another check fails.` + : panel.ok === false + ? `Fix and test, ${plural(n, 'round')}. Still failing.` + : `Fix and test, ${plural(n, 'round')}.`; // Commits made inside the loop (a test one-liner that ends in one) are part of its story. const saved = evs.reduce((k, e) => k + (e.commits ?? 0), 0); if (saved) story += ` Claude saves the work${saved > 1 ? ` in ${saved} commits` : ''} along the way.`; @@ -2491,9 +2511,11 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde const failedFirst = checks.slice(0, -1).some((e) => passed(e) === false); if (lastCheck?.running) story = isTest ? 'Claude starts the tests in the background.' : 'Claude starts the checks in the background.'; - // Finished later, in the background: not tests that ran during this turn. + // Finished in the background: not tests that ran during this turn, and said when it was later. else if (lastCheck?.background && ok !== undefined) - story = `The ${isTest ? 'tests' : 'checks'} run in the background and ${ok ? 'pass' : 'fail'}.`; + story = lastCheck.finishedLater + ? `Claude starts the ${isTest ? 'tests' : 'checks'} in the background; later they ${ok ? 'pass' : 'fail'}.` + : `The ${isTest ? 'tests' : 'checks'} run in the background and ${ok ? 'pass' : 'fail'}.`; else if (ok === undefined) story = isTest ? 'Claude runs the tests.' : 'Claude runs the checks.'; else if (!ok && tests && !tests.passed && !tests.failed && tests.errors) story = "The tests don't even load."; else if (!ok && tests && tests.passed && !tests.failed && !tests.errors) @@ -2506,7 +2528,9 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde ? `${tests.failed} tests fail.` : 'A test fails.' : 'A check fails.'; - if (ok === false && plannedFailure(turn, events)) story = `${story.replace(/[.]$/, '')}, as planned.`; + const plannedCheck = ok === false ? plannedFailure(turn, events) : false; + if (plannedCheck) + story = `${story.replace(/[.]$/, '')}, ${plannedCheck === 'asked' ? 'as you asked' : 'as planned'}.`; // The same command with the same outcome twice in a row reads once, with a count. const runs: Array<{ text: string; n: number }> = []; for (const e of checks) { diff --git a/src/web/public/story-mode.js b/src/web/public/story-mode.js index 02201356..2be1df55 100644 --- a/src/web/public/story-mode.js +++ b/src/web/public/story-mode.js @@ -689,6 +689,11 @@ const StoryArt = (() => { return { w, h }; } /** A terminal card: the commands (`› …`), then, dimmer, the first lines one of them printed. */ + /** A test board's label: "1 test", "19 tests", or "1 error" when nothing even loaded. */ + function boardLabel(t, total) { + if (t && !t.passed && !t.failed && t.errors) return `${t.errors} error${t.errors === 1 ? '' : 's'}`; + return `${total} test${total === 1 ? '' : 's'}`; + } function terminal(ctx, x, y, w, h, lines, f, printed = []) { rr(ctx, x, y, w, h, 7); inked(ctx, C.line, 2.5); @@ -1094,7 +1099,7 @@ const StoryArt = (() => { const dots = Math.min(42, total); const fail = bad ? Math.max(1, Math.round((bad / total) * dots)) : 0; const fill = running ? (f * 0.6) % (dots + 6) : Math.min(dots, (f - 4) * 1.6); - b = board(ctx, 16, 106, `${total} tests`, dots, fail, fill); + b = board(ctx, 16, 106, boardLabel(t, total), dots, fail, fill); } else { b = verdict(ctx, 16, 106, 'checks', p.panel.ok, f); } @@ -1153,7 +1158,7 @@ const StoryArt = (() => { if (total) { const dots = Math.min(28, total); const fail = bad ? Math.max(1, Math.round((bad / total) * dots)) : 0; - b = board(ctx, bx, 116, `${total} tests`, dots, fail, Math.min(dots, (f - 4) * 1.6)); + b = board(ctx, bx, 116, boardLabel(t, total), dots, fail, Math.min(dots, (f - 4) * 1.6)); } else b = verdict(ctx, bx, 116, 'checks', p.panel.ok, f); hit('output', bx, 116, b.w, b.h, 'See the test output'); } diff --git a/test/fixtures/story-corpus/background-helpers-reports.expected.json b/test/fixtures/story-corpus/background-helpers-reports.expected.json index 9855594f..d7df322d 100644 --- a/test/fixtures/story-corpus/background-helpers-reports.expected.json +++ b/test/fixtures/story-corpus/background-helpers-reports.expected.json @@ -221,7 +221,7 @@ { "kind": "check", "ok": true, - "story": "The tests run in the background and pass.", + "story": "Claude starts the tests in the background; later they pass.", "director": "python3 -m pytest -q → ok", "balloon": null, "tests": null, diff --git a/test/fixtures/story-corpus/errors-push-append.expected.json b/test/fixtures/story-corpus/errors-push-append.expected.json index 5344956c..10416180 100644 --- a/test/fixtures/story-corpus/errors-push-append.expected.json +++ b/test/fixtures/story-corpus/errors-push-append.expected.json @@ -73,7 +73,7 @@ { "kind": "check", "ok": false, - "story": "The tests don't even load, as planned.", + "story": "The tests don't even load, as you asked.", "director": "python3 -m pytest -q → error · 0 passed, 1 error", "balloon": "Now the red run, once, with calc_stats.py confirmed absent:", "tests": { diff --git a/test/fixtures/story-corpus/failing-test-on-purpose.expected.json b/test/fixtures/story-corpus/failing-test-on-purpose.expected.json index 6c8c3dda..51836c16 100644 --- a/test/fixtures/story-corpus/failing-test-on-purpose.expected.json +++ b/test/fixtures/story-corpus/failing-test-on-purpose.expected.json @@ -51,7 +51,7 @@ { "kind": "loop", "ok": true, - "story": "Test first: the first run fails as planned, the second passes.", + "story": "The first run fails on purpose, as you asked; the second passes.", "director": "2 checks · 3 edits · +35 −1 · last: ok", "balloon": "All 4 pass.", "tests": { @@ -133,7 +133,7 @@ { "kind": "loop", "ok": true, - "story": "Test first: the first run fails as planned, the second passes.", + "story": "The first run fails on purpose, as you asked; the second passes.", "director": "2 checks · 2 edits · +5 −1 · last: ok", "balloon": "All 5 pass.", "tests": { diff --git a/test/fixtures/story-corpus/long-turn-prompt-added.expected.json b/test/fixtures/story-corpus/long-turn-prompt-added.expected.json index 98718526..b9d797a7 100644 --- a/test/fixtures/story-corpus/long-turn-prompt-added.expected.json +++ b/test/fixtures/story-corpus/long-turn-prompt-added.expected.json @@ -73,7 +73,7 @@ { "kind": "loop", "ok": true, - "story": "Build and test, 4 rounds. The tests pass every time; another check fails once, then passes.", + "story": "Build and test, 4 rounds. The tests pass every time; a layout check fails once, then passes.", "director": "5 checks · 10 edits · +694 · last: ok", "balloon": "All 4 modules are done: 63 tests pass and ruff is clean.", "tests": { diff --git a/test/fixtures/story-corpus/python-cli-fix-loop.expected.json b/test/fixtures/story-corpus/python-cli-fix-loop.expected.json index 33b198df..d07c1270 100644 --- a/test/fixtures/story-corpus/python-cli-fix-loop.expected.json +++ b/test/fixtures/story-corpus/python-cli-fix-loop.expected.json @@ -51,7 +51,7 @@ { "kind": "loop", "ok": true, - "story": "Fix and test, 2 rounds: a test run fails, then another check, then everything passes. Claude saves the work in 4 commits along the way.", + "story": "Fix and test, 2 rounds: a test fails and Claude fixes it, then a layout check fails and Claude fixes that; everything passes. Claude saves the work in 4 commits along the way.", "director": "3 checks · 5 edits · +218 −1 · last: ok", "balloon": "All 19 tests pass, ruff check is clean, and the output matches real wc column for column.", "tests": { diff --git a/test/story-corpus.test.ts b/test/story-corpus.test.ts index 4157ad30..78d96ace 100644 --- a/test/story-corpus.test.ts +++ b/test/story-corpus.test.ts @@ -77,7 +77,7 @@ describe('Story Mode on real sessions', () => { expect(panel('python-cli-fix-loop', 0, 'loop')).toMatchObject({ // The failing test was a real bug, the second failure only ruff: said apart, with the 4 commits made inside. story: - 'Fix and test, 2 rounds: a test run fails, then another check, then everything passes. Claude saves the work in 4 commits along the way.', + 'Fix and test, 2 rounds: a test fails and Claude fixes it, then a layout check fails and Claude fixes that; everything passes. Claude saves the work in 4 commits along the way.', roundsOk: [false, false, true], }); }); @@ -131,7 +131,7 @@ describe('Story Mode on real sessions', () => { it('tests that do not load, a commit then a push in its own command, a push with a failing step after it', () => { const [first, second, third] = pagesOf('errors-push-append'); - expect(first.panels.find((p) => p.kind === 'check')?.story).toBe("The tests don't even load, as planned."); + expect(first.panels.find((p) => p.kind === 'check')?.story).toBe("The tests don't even load, as you asked."); expect(second.panels.find((p) => p.kind === 'loop')).toMatchObject({ story: 'Test first: the first run fails as planned, the second passes.', roundsOk: [false, true], @@ -155,7 +155,9 @@ describe('Story Mode on real sessions', () => { expect(report.helpers).toBe(0); expect(report.panels[0].story).toBe('A background job reports back.'); } - expect(pages[3].panels.map((p) => p.story)).toContain('The tests run in the background and pass.'); + expect(pages[3].panels.map((p) => p.story)).toContain( + 'Claude starts the tests in the background; later they pass.' + ); // A python script that rewrote README.md counts as a change, beside the two files written. expect(pages[3].panels.map((p) => p.story)).toContain('Claude changes 3 files.'); }); diff --git a/test/story-mode.test.ts b/test/story-mode.test.ts index b8333835..8025c0b1 100644 --- a/test/story-mode.test.ts +++ b/test/story-mode.test.ts @@ -742,7 +742,7 @@ describe('buildStoryTurns', () => { ); // Not tests that ran during that turn: they ran later, in the background. expect(first.panels.find((p) => p.kind === 'check')!.caption.story).toBe( - 'The tests run in the background and pass.' + 'Claude starts the tests in the background; later they pass.' ); const second = buildStoryPage(turns[1]); expect(second.panels.find((p) => p.kind === 'check')).toMatchObject({ @@ -1194,7 +1194,7 @@ describe('buildStoryPage', () => { const loop = page.panels.find((p) => p.kind === 'loop')!; expect(loop.roundsOk).toEqual([true, false, true]); expect(loop.caption.story).toBe( - 'Build and test, 2 rounds. The tests pass every time; another check fails once, then passes.' + 'Build and test, 2 rounds. The tests pass every time; a layout check fails once, then passes.' ); });