From dc980d121c0c501c452a111dddd5664c3aa22b7a Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 18 Oct 2026 17:47:44 +0200 Subject: [PATCH] fix(story-mode): a new file counts what it ends with, a rewrite is a change, short endings are read (review round 11) From the fact-checker: - A script created and then edited in the same panel showed "+95 -3": a file created in the span counts what it ends with, never lines removed from its own draft (+92), on the badge, the panel and the page. - "Claude writes 4 files" for files an earlier panel wrote and this one rewrote with here-documents: writing a file the turn already touched is changing it. - "Claude says: Compacted": a local command's output is the CLI's own, a note, not Claude's words. - "The CHANGELOG helper is still running" was dropped under "Mission complete!": key sentences were read only in endings over 160 characters; a short ending is read too, and that one is a note. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/story-mode.ts | 70 ++++++++++++++----- src/web/public/story-mode.js | 8 ++- .../background-helpers-reports.expected.json | 4 +- ...ash-script-background-helper.expected.json | 6 +- .../failing-test-on-purpose.expected.json | 6 +- .../python-cli-fix-loop.expected.json | 6 +- .../question-tests-first-push.expected.json | 6 +- .../teammates-then-interrupt.expected.json | 2 +- test/story-mode.test.ts | 37 ++++++++++ 9 files changed, 109 insertions(+), 36 deletions(-) diff --git a/src/story-mode.ts b/src/story-mode.ts index adfdb857..4666cece 100644 --- a/src/story-mode.ts +++ b/src/story-mode.ts @@ -372,7 +372,7 @@ export function keyKindOf(point: string): StoryKeyKind | undefined { * follows it. */ function keyPointsOf(text: string): string[] { - if (text.length <= 160) return []; + if (text.length <= 40) return []; const spans: string[] = []; const plain = plainMarkdown(text.slice(0, 8000), spans); // Every sentence in order, with the line it came from (list items are lines). @@ -2409,6 +2409,31 @@ function newBranchAt(indices: number[], events: StoryEvent[]): string | undefine return undefined; } +/** + * Lines added and removed, per file summed: a file created in these edits counts what it ends with + * (its later edits in the same span are its own drafting), so a new file never shows removed lines. + */ +function netCounts(edits: StoryEdit[]): { added: number; removed: number } { + let added = 0; + let removed = 0; + for (const file of unique(edits.map((d) => d.file))) { + const mine = edits.filter((d) => d.file === file); + const a = mine.reduce((n, d) => n + d.added, 0); + const r = mine.reduce((n, d) => n + d.removed, 0); + if (mine.some((d) => d.created)) added += Math.max(0, a - r); + else { + added += a; + removed += r; + } + } + return { added, removed }; +} + +/** A file an earlier panel of the turn already wrote or changed: writing it again is changing it. */ +function touchedBefore(file: string, before: StoryEvent[]): boolean { + return before.some((e) => editsOf(e).some((d) => d.file === file)); +} + /** One or two files by name ("calc.py and test_calc.py"), more by count ("3 files"). */ const fileWords = (files: string[], noun = 'file'): string => files.length && files.length <= 2 ? files.map((f) => basename(f)).join(' and ') : plural(files.length, noun); @@ -2593,17 +2618,19 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde ...edits.map((d) => d.file), ]); panel.files = files.slice(0, 12); - panel.added = edits.reduce((n, e) => n + e.added, 0); - panel.removed = edits.reduce((n, e) => n + e.removed, 0); + ({ added: panel.added, removed: panel.removed } = netCounts(edits)); if (draft.kind === 'fix') { const created = unique(edits.filter((e) => e.created).map((e) => e.file)).length; // Some files written whole (new ones), others patched: "writes README.md and changes CLAUDE.md". - // A file created here is written, even when Claude then edited it in the same panel. + // A file created here is written, even when Claude then edited it in the same panel; one an + // earlier panel already wrote is changed, even when this panel rewrites it whole. + const before = events.slice(0, Math.min(...draft.events)); const wholeOf = (f: string) => { const mine = edits.filter((d) => d.file === f); return ( mine.some((d) => d.created) || - mine.every((d) => d.hunks.length > 0 && d.hunks.every((h) => !h.oldStart && !h.oldLines)) + (!touchedBefore(f, before) && + mine.every((d) => d.hunks.length > 0 && d.hunks.every((h) => !h.oldStart && !h.oldLines))) ); }; const wholeList = files.filter(wholeOf); @@ -2619,7 +2646,7 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde ? files.length <= 2 ? `Claude writes the new ${files.length === 1 ? 'file' : 'files'} ${fileWords(files)}.` : `Claude writes ${plural(created, 'new file')}.` - : whole && files.length + : whole && files.length && wholeList.length === files.length ? `Claude writes ${fileWords(files)}.` : mixed ? `Claude writes ${fileWords(mixed.whole)} and changes ${fileWords(mixed.patched)}.` @@ -2627,11 +2654,8 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde ? `Claude changes ${fileWords(files)}.` : 'Claude changes a file.'; const perFile = files.slice(0, 3).map((f) => { - const mine = edits.filter((e) => e.file === f); - return `${basename(f)} ${fmtCounts( - mine.reduce((n, e) => n + e.added, 0), - mine.reduce((n, e) => n + e.removed, 0) - )}`; + const mine = netCounts(edits.filter((e) => e.file === f)); + return `${basename(f)} ${fmtCounts(mine.added, mine.removed)}`; }); director = `${tools}${perFile.length ? ` · ${perFile.join(' · ')}` : ''}`; } else { @@ -2753,7 +2777,12 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde // Whole files are written; a file it patched is changed (a later panel often patches one written earlier). const isWhole = (d: StoryEdit) => d.created || (d.hunks.length > 0 && d.hunks.every((h) => !h.oldStart && !h.oldLines)); - const wholeFiles = written.filter((f) => edits.filter((d) => d.file === f).every(isWhole)); + const earlier = events.slice(0, Math.min(...draft.events)); + const wholeFiles = written.filter( + (f) => + edits.filter((d) => d.file === f).some((d) => d.created) || + (!touchedBefore(f, earlier) && edits.filter((d) => d.file === f).every(isWhole)) + ); const patched = written.filter((f) => !wholeFiles.includes(f)); const did = [ wholeFiles.length ? `writes ${fileWords(wholeFiles)}` : '', @@ -2890,10 +2919,13 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde : `Claude saves the work${inCommits}${said}.`; const wrote = unique(edits.map((d) => d.file)); const startsRepo = startsRepoAt(draft.events, events); - const wroteWhole = wrote.filter((f) => - edits - .filter((d) => d.file === f) - .every((d) => d.created || (d.hunks.length > 0 && d.hunks.every((h) => !h.oldStart && !h.oldLines))) + const wroteWhole = wrote.filter( + (f) => + edits.filter((d) => d.file === f).some((d) => d.created) || + (!touchedBefore(f, events.slice(0, Math.min(...draft.events))) && + edits + .filter((d) => d.file === f) + .every((d) => d.hunks.length > 0 && d.hunks.every((h) => !h.oldStart && !h.oldLines))) ); const wrotePatched = wrote.filter((f) => !wroteWhole.includes(f)); const lead = [ @@ -3045,7 +3077,8 @@ function completePanel(turn: StoryTurn, page: Omit, trailin // A local command answers itself: its own first line is the last word. const said = turn.endedBy === 'local command' ? turn.events[0]?.output?.split('\n')[0] : undefined; if (trailingSay) panel.balloon = { who: 'claude', text: trailingSay }; - else if (said) panel.balloon = { who: 'claude', text: clip(said, 160) }; + // A local command's output (/compact's "Compacted") is the CLI's own, not Claude's words: a note. + else if (said) panel.balloon = { who: 'note', text: clip(said, 160) }; if (page.filesChanged) { panel.added = page.added; panel.removed = page.removed; @@ -3083,8 +3116,7 @@ export function buildStoryPage(turn: StoryTurn): StoryPage { truncated, events, filesChanged: unique(edits.map((e) => e.file)).length, - added: edits.reduce((n, e) => n + e.added, 0), - removed: edits.reduce((n, e) => n + e.removed, 0), + ...netCounts(edits), // The advisor is a second opinion, not a helper that does work. helpers: events.filter((e) => e.helper && e.tool !== 'Advisor' && e.intent !== 'prompt').length, helpersRunning: events.filter((e) => e.helper?.running).length, diff --git a/src/web/public/story-mode.js b/src/web/public/story-mode.js index 230e8d8c..24594148 100644 --- a/src/web/public/story-mode.js +++ b/src/web/public/story-mode.js @@ -1063,8 +1063,12 @@ const StoryArt = (() => { // With several files the badge counts the card in front: a total beside one file's card read as // that file's own. The Director's cut has every file's count. const front = files.length > 1 && first ? edits.filter((d) => d.file === first.file) : null; - const added = front ? front.reduce((n, d) => n + d.added, 0) : p.panel.added; - const removed = front ? front.reduce((n, d) => n + d.removed, 0) : p.panel.removed; + // A file created here counts what it ends with, never lines it removed from its own draft. + const fa = front ? front.reduce((n, d) => n + d.added, 0) : 0; + const fr = front ? front.reduce((n, d) => n + d.removed, 0) : 0; + const fresh = !!front?.some((d) => d.created); + const added = front ? (fresh ? Math.max(0, fa - fr) : fa) : p.panel.added; + const removed = front ? (fresh ? 0 : fr) : p.panel.removed; if (added) sfx( ctx, diff --git a/test/fixtures/story-corpus/background-helpers-reports.expected.json b/test/fixtures/story-corpus/background-helpers-reports.expected.json index 77962270..e0d06774 100644 --- a/test/fixtures/story-corpus/background-helpers-reports.expected.json +++ b/test/fixtures/story-corpus/background-helpers-reports.expected.json @@ -282,7 +282,7 @@ "helpers": 0, "helpersRunning": 0, "finishedLater": 0, - "openNote": false, + "openNote": "note", "asks": false, "panels": [ { @@ -318,7 +318,7 @@ "story": "Done in 7 seconds.", "unaskedSave": false, "director": "turn_duration · 7 s · 1 tool call", - "balloon": "The background test run finished and all 23 tests passed, including the new --json test.", + "balloon": "The background test run finished and all 23 tests passed, including the new --json test. The CHANGELOG helper is still running; I'll report when it finishes.", "tests": null, "roundsOk": null, "printed": null, diff --git a/test/fixtures/story-corpus/bash-script-background-helper.expected.json b/test/fixtures/story-corpus/bash-script-background-helper.expected.json index 601f81d0..a5b26ca0 100644 --- a/test/fixtures/story-corpus/bash-script-background-helper.expected.json +++ b/test/fixtures/story-corpus/bash-script-background-helper.expected.json @@ -7,8 +7,8 @@ "durationMs": 121460, "toolCalls": 13, "filesChanged": 2, - "added": 96, - "removed": 3, + "added": 93, + "removed": 0, "helpers": 1, "helpersRunning": 0, "finishedLater": 1, @@ -68,7 +68,7 @@ "ok": null, "story": "Claude changes rename_by_date.sh and -dash.jpg.", "unaskedSave": false, - "director": "Write · Edit ×3 · Bash · rename_by_date.sh +95 −3 · -dash.jpg +1", + "director": "Write · Edit ×3 · Bash · rename_by_date.sh +92 · -dash.jpg +1", "balloon": "Now I'll create the samples and run the dry run.", "tests": null, "roundsOk": null, 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 6722616c..5dd220af 100644 --- a/test/fixtures/story-corpus/failing-test-on-purpose.expected.json +++ b/test/fixtures/story-corpus/failing-test-on-purpose.expected.json @@ -7,8 +7,8 @@ "durationMs": 83905, "toolCalls": 9, "filesChanged": 2, - "added": 35, - "removed": 1, + "added": 34, + "removed": 0, "helpers": 0, "helpersRunning": 0, "finishedLater": 0, @@ -44,7 +44,7 @@ "ok": true, "story": "The first run fails on purpose, as you asked; the second passes.", "unaskedSave": false, - "director": "2 checks · 3 edits · +35 −1 · last: ok · after 1 second opinion", + "director": "2 checks · 3 edits · +34 · last: ok · after 1 second opinion", "balloon": "All 4 pass.", "tests": { "passed": 4, 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 a7b73e8c..8f4597a2 100644 --- a/test/fixtures/story-corpus/python-cli-fix-loop.expected.json +++ b/test/fixtures/story-corpus/python-cli-fix-loop.expected.json @@ -7,8 +7,8 @@ "durationMs": 192853, "toolCalls": 11, "filesChanged": 3, - "added": 220, - "removed": 1, + "added": 219, + "removed": 0, "helpers": 0, "helpersRunning": 0, "finishedLater": 0, @@ -44,7 +44,7 @@ "ok": true, "story": "Fix and test, 2 rounds: a test fails and Claude fixes it, then a formatting check fails and Claude fixes that; everything passes. Claude starts a git repository and saves the work in 4 commits along the way.", "unaskedSave": true, - "director": "3 checks · 5 edits · +218 −1 · last: ok · after 1 second opinion", + "director": "3 checks · 5 edits · +217 · last: ok · after 1 second opinion", "balloon": "All 19 tests pass, ruff check is clean, and the output matches real wc column for column.", "tests": { "passed": 19, diff --git a/test/fixtures/story-corpus/question-tests-first-push.expected.json b/test/fixtures/story-corpus/question-tests-first-push.expected.json index 96da27d1..7ac8b0b6 100644 --- a/test/fixtures/story-corpus/question-tests-first-push.expected.json +++ b/test/fixtures/story-corpus/question-tests-first-push.expected.json @@ -7,8 +7,8 @@ "durationMs": 297209, "toolCalls": 18, "filesChanged": 6, - "added": 221, - "removed": 2, + "added": 220, + "removed": 1, "helpers": 0, "helpersRunning": 0, "finishedLater": 0, @@ -56,7 +56,7 @@ "ok": true, "story": "The first run fails on purpose, as you asked; the second passes.", "unaskedSave": false, - "director": "2 checks · 4 edits · +151 −1 · last: ok · after 1 second opinion", + "director": "2 checks · 4 edits · +150 · last: ok · after 1 second opinion", "balloon": "All 19 test cases pass (13 test functions), and ruff check and ruff format both come back clean.", "tests": { "passed": 19, diff --git a/test/fixtures/story-corpus/teammates-then-interrupt.expected.json b/test/fixtures/story-corpus/teammates-then-interrupt.expected.json index cf273a4b..3b661af0 100644 --- a/test/fixtures/story-corpus/teammates-then-interrupt.expected.json +++ b/test/fixtures/story-corpus/teammates-then-interrupt.expected.json @@ -297,7 +297,7 @@ { "kind": "check", "ok": true, - "story": "Claude writes 4 files; the tests pass.", + "story": "Claude changes 4 files; the tests pass.", "unaskedSave": false, "director": "npm test 2>&1 → ok · 8 passed · writes 4 files", "balloon": "Every @example output matches what the code actually returns, and 8/8 tests pass.", diff --git a/test/story-mode.test.ts b/test/story-mode.test.ts index 48d31ae6..051e3c7f 100644 --- a/test/story-mode.test.ts +++ b/test/story-mode.test.ts @@ -571,6 +571,8 @@ describe('buildStoryTurns', () => { expect(page.panels.map((p) => p.kind)).toEqual(['mission', 'compact', 'complete']); expect(page.panels[0].caption.story).toBe('You run /compact.'); expect(page.events[0]).toMatchObject({ tool: 'Command', output: 'Compacted' }); + // The CLI's own output is a note, not Claude's words. + expect(page.panels.at(-1)!.balloon).toEqual({ who: 'note', text: 'Compacted' }); // A local command like /model ends at its output too. const [, model] = buildStoryTurns( entriesOf([ @@ -1059,6 +1061,41 @@ describe('buildStoryPage', () => { expect(keyKindOf('Anything unknown is left out rather than guessed.')).toBeUndefined(); }); + it('a new file counts what it ends with; a file written earlier and rewritten is changed', () => { + const write = (id: string, file: string, content: string) => [ + tool(id, 'Write', { file_path: file, content }), + result(id, 'File created', { + toolUseResult: { type: 'create', filePath: file, content, structuredPatch: [] }, + }), + ]; + const page = pageOf([ + prompt('build it'), + ...write('w1', '/w/run.sh', 'a\nb\nc\nd\n'), + ...edit('e1', '/w/run.sh', ['-b', '+B']), + ...edit('e2', '/w/run.sh', ['-c']), + ...bash('b1', 'pytest -q', 'Test', '1 passed in 0.1s'), + ...bash('b2', "cat > /w/run.sh <<'EOF'\na\nB\nd\nEOF\npytest -q", 'Rewrite and test', '1 passed in 0.1s'), + end(1000), + ]); + // Written (4 lines), then 1 line changed and 1 removed in the same panel: +3, nothing removed. + expect(page.panels.find((p) => p.kind === 'fix')).toMatchObject({ added: 3, removed: 0 }); + expect(page.panels.find((p) => p.kind === 'fix')!.caption.story).toBe('Claude writes the new file run.sh.'); + // The here-document rewrites run.sh whole, but an earlier panel wrote it: that is a change. + expect(page.panels.map((p) => p.caption.story)).toContain('Claude changes run.sh; the tests pass.'); + }); + + it('a short ending keeps the sentence that leaves something open', () => { + const page = pageOf([ + prompt('run them'), + said('The background test run finished and all 23 tests passed. The CHANGELOG helper is still running.'), + end(1000), + ]); + expect(page.panels.at(-1)!.balloon!.text).toBe( + 'The background test run finished and all 23 tests passed. The CHANGELOG helper is still running.' + ); + expect(page.openNote).toBe('note'); + }); + it('the first save no prompt asked for is marked, once; a new branch is named', () => { const saves = (ask: string) => pageOf([