From 175017f604740f63c1def9009d11513195be270b Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 18 Oct 2026 17:22:11 +0200 Subject: [PATCH] fix(story-mode): the badge counts the card it sits on, opinions mid-step, a deviation is a note (review round 10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the fact-checker: - "+37" sat on mathx.py's card, which has 11 of those lines; with several files the badge now counts the card in front (the Director's cut keeps every file's count). - A second opinion asked in the middle of a step's calls read "after asking"; it now reads "along the way, asks a stronger AI model…". - "Two places where I didn't follow your wording exactly" under "Mission complete!": not following your words is a note ("Done, with a note"), and the sentence counts as a word on what you asked. - A test run that patched a file written a panel before said "Claude writes" it: whole files are written, patched ones changed ("Claude writes pyproject.toml and changes test_csvstat.py; …"). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/story-mode.ts | 38 +++++++++++++----- src/web/public/story-mode.js | 28 +++++++++---- .../compact-then-plan-mode.expected.json | 2 +- .../errors-push-append.expected.json | 2 +- .../long-turn-prompt-added.expected.json | 2 +- .../text-plan-dry-run-helper.expected.json | 2 +- test/story-corpus.test.ts | 9 ++++- test/story-mode-ui.test.ts | 39 +++++++++++++++++++ 8 files changed, 100 insertions(+), 22 deletions(-) diff --git a/src/story-mode.ts b/src/story-mode.ts index 7ae79ddb..4ecbb7ad 100644 --- a/src/story-mode.ts +++ b/src/story-mode.ts @@ -314,7 +314,7 @@ const KEY_KINDS: Array<[StoryKeyKind, RegExp]> = [ ['failed', /(?> = { const ANNOUNCES = /\b(I'll|I will|let me|let's|I'm going to|next,? I|now I)\b/i; /** What Claude's last words leave open: a catch (a problem), a note (something pending), a question. */ +/** Claude did something differently from what you asked: worth a note on the final panel. */ +const DEVIATED = + /\b(didn't|did not|couldn't|could not) (follow|use|do) (your|exactly what you)|\bnot exactly as you\b/i; + function closingKinds(points: string[] = []): { openNote?: 'catch' | 'note'; asks?: boolean } { const kinds = new Set(points.map(keyKindOf)); return { ...(kinds.has('problem') ? { openNote: 'catch' as const } - : kinds.has('pending') + : kinds.has('pending') || points.some((p) => DEVIATED.test(p)) ? { openNote: 'note' as const } : {}), ...(points.some((p) => keyKindOf(p) === 'offer' && ASKS_YOU.test(p)) ? { asks: true } : {}), @@ -2573,7 +2577,8 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde if (draft.kind === 'fix') { const created = unique(edits.filter((e) => e.created).map((e) => e.file)).length; const whole = - edits.length > 0 && edits.every((d) => d.created || d.hunks.every((h) => !h.oldStart && !h.oldLines)); + edits.length > 0 && + edits.every((d) => d.created || (d.hunks.length > 0 && d.hunks.every((h) => !h.oldStart && !h.oldLines))); story = created && created === files.length ? files.length <= 2 @@ -2708,7 +2713,16 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde `The $1 $2$3 in a fresh copy made from ${basename(cloned.replace(/\/+$/, ''))}` ); const written = unique(edits.map((d) => d.file)); - if (written.length) story = `Claude writes ${fileWords(written)}; ${story[0].toLowerCase()}${story.slice(1)}`; + // 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 patched = written.filter((f) => !wholeFiles.includes(f)); + const did = [ + wholeFiles.length ? `writes ${fileWords(wholeFiles)}` : '', + patched.length ? `changes ${fileWords(patched)}` : '', + ].filter(Boolean); + if (did.length) story = `Claude ${did.join(' and ')}; ${story[0].toLowerCase()}${story.slice(1)}`; const saved = checks.reduce((n, e) => n + (e.commits ?? 0), 0); if (saved) { director += ` · ${plural(saved, 'commit')}`; @@ -2875,11 +2889,17 @@ function decorate(draft: DraftPanel, events: StoryEvent[], turn: StoryTurn, inde break; } if (panel.secondOpinion) { - // Asked before the step, or (nothing following) after the last one. - const lastWork = Math.max(...draft.events.filter((i) => events[i].tool !== 'Advisor')); - const trailing = draft.events.some((i) => events[i].tool === 'Advisor' && i > lastWork); - const unanswered = draft.events.every((i) => events[i].tool !== 'Advisor' || events[i].ok === false); - director += ` · ${trailing ? 'then' : 'after'} ${plural(panel.secondOpinion, 'second opinion')}${unanswered ? ' (no answer)' : ''}`; + // Asked before the step, after it (nothing following), or in the middle of a step's calls. + const work = draft.events.filter((i) => events[i].tool !== 'Advisor'); + const asked = draft.events.filter((i) => events[i].tool === 'Advisor'); + const when = asked.every((i) => i < Math.min(...work)) + ? 'after' + : asked.every((i) => i > Math.max(...work)) + ? 'then' + : 'during'; + const unanswered = asked.every((i) => events[i].ok === false); + const count = plural(panel.secondOpinion, 'second opinion'); + director += ` · ${when === 'during' ? `${count} along the way` : `${when} ${count}`}${unanswered ? ' (no answer)' : ''}`; } panel.caption = { story, director }; return panel; diff --git a/src/web/public/story-mode.js b/src/web/public/story-mode.js index d86e9bd0..919f79d1 100644 --- a/src/web/public/story-mode.js +++ b/src/web/public/story-mode.js @@ -1060,10 +1060,15 @@ const StoryArt = (() => { const hp = [tip[0] + 20 + Math.sin(f / 2.6) * 3, tip[1] - 28 + Math.cos(f / 3.1) * 2.5]; const a = agent(ctx, ax, 248, { frame: f, mood: 'focus', l: [hp[0] - ax, hp[1] - 248], r: [30, -42] }); pencil(ctx, a.l, [tip[0] + Math.sin(f / 2.6) * 3, tip[1] + Math.cos(f / 3.1) * 2.5]); - if (p.panel.added) + // 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; + if (added) sfx( ctx, - `+${p.panel.added}`, + `+${added}`, x0 + pw * 0.32, y0 + ph + 2, 26, @@ -1071,10 +1076,10 @@ const StoryArt = (() => { -0.18, spring(f - 44, { damping: 9, stiffness: 170 }) ); - if (p.panel.removed) + if (removed) sfx( ctx, - `−${p.panel.removed}`, + `−${removed}`, x0 + pw * 0.74, y0 + ph + 6, 24, @@ -2494,15 +2499,22 @@ Object.assign(CodemanApp.prototype, { // Asked before this step, or after it when nothing followed. const work = p.events.filter((i) => page.events[i]?.tool !== 'Advisor'); const asked = p.events.filter((i) => page.events[i]?.tool === 'Advisor'); - const trailing = asked.some((i) => i > Math.max(...work)); + // Before the step's calls, after them, or in the middle of them. + const before = asked.every((i) => i < Math.min(...work)); + const trailing = !before && asked.every((i) => i > Math.max(...work)); // A call that came back with an error brought no opinion: say so, never that one was given. const unanswered = asked.length > 0 && asked.every((i) => page.events[i].ok === false); + const ask = unanswered ? 'tries to ask' : 'asks'; rt.capNote.hidden = !p.secondOpinion; rt.capNote.textContent = !p.secondOpinion ? '' - : unanswered - ? `${trailing ? 'then tries to ask' : 'after trying to ask'} a stronger AI model for a second opinion (no answer came)` - : `${trailing ? 'then asks' : 'after asking'} a stronger AI model for a second opinion`; + : `${ + before + ? `after ${unanswered ? 'trying to ask' : 'asking'}` + : trailing + ? `then ${ask}` + : `along the way, ${ask}` + } a stronger AI model for a second opinion${unanswered ? ' (no answer came)' : ''}`; } rt.el.classList.toggle('is-live', !!p.live); // Named by its place only: the caption inside is read as the panel's content, once. diff --git a/test/fixtures/story-corpus/compact-then-plan-mode.expected.json b/test/fixtures/story-corpus/compact-then-plan-mode.expected.json index f2967104..3c5e7f24 100644 --- a/test/fixtures/story-corpus/compact-then-plan-mode.expected.json +++ b/test/fixtures/story-corpus/compact-then-plan-mode.expected.json @@ -179,7 +179,7 @@ "kind": "plan", "ok": null, "story": "Claude writes a plan: 3 steps.", - "director": "EnterPlanMode · Write · plan file · 3 steps · after 1 second opinion", + "director": "EnterPlanMode · Write · plan file · 3 steps · 1 second opinion along the way", "balloon": null, "tests": null, "roundsOk": null, diff --git a/test/fixtures/story-corpus/errors-push-append.expected.json b/test/fixtures/story-corpus/errors-push-append.expected.json index e3c0d936..86674b6a 100644 --- a/test/fixtures/story-corpus/errors-push-append.expected.json +++ b/test/fixtures/story-corpus/errors-push-append.expected.json @@ -41,7 +41,7 @@ "kind": "run", "ok": null, "story": "Claude works in the terminal.", - "director": "Bash ×2 · python3 -c \"import calc_stats\" 2>&1 | tail -1; ls pytest.in… · after 1 second opinion", + "director": "Bash ×2 · python3 -c \"import calc_stats\" 2>&1 | tail -1; ls pytest.in… · 1 second opinion along the way", "balloon": null, "tests": null, "roundsOk": null, 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 998dba01..9b470e42 100644 --- a/test/fixtures/story-corpus/long-turn-prompt-added.expected.json +++ b/test/fixtures/story-corpus/long-turn-prompt-added.expected.json @@ -171,7 +171,7 @@ "helpers": 0, "helpersRunning": 0, "finishedLater": 0, - "openNote": false, + "openNote": "note", "asks": false, "panels": [ { diff --git a/test/fixtures/story-corpus/text-plan-dry-run-helper.expected.json b/test/fixtures/story-corpus/text-plan-dry-run-helper.expected.json index e8f62866..31bc3f35 100644 --- a/test/fixtures/story-corpus/text-plan-dry-run-helper.expected.json +++ b/test/fixtures/story-corpus/text-plan-dry-run-helper.expected.json @@ -62,7 +62,7 @@ { "kind": "check", "ok": true, - "story": "Claude writes test_csvstat.py and pyproject.toml; the tests pass, then Claude starts a git repository and saves the work.", + "story": "Claude writes pyproject.toml and changes test_csvstat.py; the tests pass, then Claude starts a git repository and saves the work.", "director": "python3 -m pytest -q 2>&1 → ok ×2 · 17 passed · writes 2 files · 1 commit", "balloon": "Code and tests are committed.", "tests": { diff --git a/test/story-corpus.test.ts b/test/story-corpus.test.ts index c8d6a00f..173807cb 100644 --- a/test/story-corpus.test.ts +++ b/test/story-corpus.test.ts @@ -107,7 +107,8 @@ describe('Story Mode on real sessions', () => { it('a commit at the end of a test one-liner is kept, and a background helper reports back in 9 s', () => { expect(panel('text-plan-dry-run-helper', 0, 'check')?.story).toBe( - 'Claude writes test_csvstat.py and pyproject.toml; the tests pass, then Claude starts a git repository and saves the work.' + // test_csvstat.py was written a panel before and only patched here. + 'Claude writes pyproject.toml and changes test_csvstat.py; the tests pass, then Claude starts a git repository and saves the work.' ); expect(panel('text-plan-dry-run-helper', 0, 'plan')?.story).toBe('Claude writes a plan: 4 steps.'); expect(pagesOf('text-plan-dry-run-helper')[1]).toMatchObject({ origin: 'task', durationMs: 9008 }); @@ -195,6 +196,12 @@ describe('Story Mode on real sessions', () => { expect(panel).toBeTruthy(); }); + it('an ending that says Claude did not follow your wording is a note, not a clean finish (round 10)', () => { + const [, exact] = pagesOf('long-turn-prompt-added'); + expect(exact.openNote).toBe('note'); + expect(exact.panels.at(-1)!.balloon).toMatch(/Steps 5, 10 and 15: I added a second -m/); + }); + it('a helper reporting back is heard in its own words, not only "finished" (round 9)', () => { const [, readme, extra] = pagesOf('background-helpers-reports'); expect(readme.panels[0].balloon).toBe('"Helper A: write README.md" reports: README.md'); diff --git a/test/story-mode-ui.test.ts b/test/story-mode-ui.test.ts index 39c9a735..7ae8f60b 100644 --- a/test/story-mode-ui.test.ts +++ b/test/story-mode-ui.test.ts @@ -36,10 +36,14 @@ function storyOf(lines: unknown[]) { }; } +/** Every text a scene drew, in order (the fake context records `fillText`). */ +const drawnTexts: string[] = []; + /** A 2D context that accepts every call; measureText answers a plausible width. */ function fakeContext() { const target: Record = { measureText: (text: string) => ({ width: String(text).length * 6 }), + fillText: (text: unknown) => void drawnTexts.push(String(text)), }; return new Proxy(target, { get(t, key) { @@ -886,6 +890,41 @@ describe('story-mode.js overlay', () => { expect(note.textContent).toBe('after asking a stronger AI model for a second opinion'); }); + it('a second opinion asked in the middle of a step says so, and the badge counts the card in front', async () => { + const advisor = { + type: 'assistant', + timestamp: '2026-10-18T09:41:20.000Z', + message: { + content: [ + { type: 'server_tool_use', id: 'srv1', name: 'advisor', input: {} }, + { type: 'advisor_tool_result', tool_use_id: 'srv1', content: { type: 'advisor_redacted_result' } }, + ], + }, + }; + boot( + storyOf([ + prompt('fix it'), + ...edit('e1', '/work/demo-shop/a.py', ['+a = 1', '+b = 2']), + advisor, + ...edit('e2', '/work/demo-shop/b.py', ['+c = 3', '+d = 4', '+e = 5']), + end(1000), + ]) + ); + drawnTexts.length = 0; + app.openStoryMode(SID); + await settle(); + expect($('.story-panel[data-kind="fix"] .story-cap-note')!.textContent).toBe( + 'along the way, asks a stronger AI model for a second opinion' + ); + // Two files: the badge counts a.py, the card in front (+2), not the panel's +5. (JSDOM lays out + // nothing, so the panel is given a size and drawn late in its animation, the badge showing.) + const rt = app._story.panels.find((p: { kind: string }) => p.kind === 'fix'); + Object.assign(rt, { w: 600, h: 270, dpr: 1, bgKey: '', clock: 200 }); + app._storyDrawPanel(rt); + expect(drawnTexts).toContain('+2'); + expect(drawnTexts).not.toContain('+5'); + }); + it('a second opinion that came back with an error says no answer came', async () => { const failed = { type: 'assistant',