From 7f2304725b4d86d7c8f9d204358151f174ff4e58 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 18 Oct 2026 13:37:31 +0200 Subject: [PATCH] fix(story-mode): a live drawer that holds still, and panels that keep their place through a fold From review round 6: - An open drawer rebuilt its body on every refresh, unchanged or not (11 rebuilds in 25 s, 7 identical), so a focused button inside it fell to the page body and a text selection was cleared. A refresh that changes nothing now keeps the nodes; a real change rebuilds and gives focus back to the control that does the same thing. Helper steps are filled after that decision, so a skipped rebuild costs no request and a helper still working still refreshes its steps. - When a fix folded into a loop under an open drawer, Escape left focus on the body: the opener (`diff@fix:1`) was gone. Its replacement is found by view and panel position (`diff@loop:1`). - Panels are matched by identity (kind and first event), not position: a fold into a montage near the start of a long live turn shifted every later index and rebuilt all eleven panels with their entrances. Only the montage is new now. - An edit with no lines (a script, or a command Claude Code says changed the file) says so: "What changed is not in the transcript: a script or another program wrote this file." Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/story-mode.js | 88 +++++++++++++++++++++++++++++------- test/story-mode-ui.test.ts | 79 +++++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 17 deletions(-) diff --git a/src/web/public/story-mode.js b/src/web/public/story-mode.js index 58870aba..1042e1a5 100644 --- a/src/web/public/story-mode.js +++ b/src/web/public/story-mode.js @@ -2160,28 +2160,40 @@ Object.assign(CodemanApp.prototype, { for (const rt of st.panels.splice(0)) this._storyDropPanel(rt); st.shownPageId = page?.id ?? null; } - want.forEach((panel, i) => { - let rt = st.panels[i]; - if (rt && rt.kind !== panel.kind) { - this._storyDropPanel(rt); - rt = null; - } - if (!rt) { + // A panel is the same panel while its kind and first event are: a fold into a montage near the + // start shifts every later index, and those panels must keep their state (no second entrance). + const identity = (panel) => `${panel.kind}|${page.events[panel.events[0]]?.id ?? ''}`; + const old = new Map(st.panels.map((rt) => [rt.identity, rt])); + const next = want.map((panel) => { + const id = identity(panel); + let rt = old.get(id); + if (rt) old.delete(id); + else { rt = this._storyCreatePanel(panel.kind, fresh); - const after = st.panels[i - 1]?.el; - if (after) after.after(rt.el); - else st.pageEl.prepend(rt.el); - st.panels[i] = rt; st.ro?.observe(rt.el); st.io?.observe(rt.el); } + rt.identity = id; + return rt; + }); + for (const rt of old.values()) this._storyDropPanel(rt); + st.panels = next; + // In page order, moving only what is out of place (moving a node would drop its focus). + next.forEach((rt, i) => { + const prev = next[i - 1]?.el; + if (prev ? prev.nextElementSibling !== rt.el : st.pageEl.firstElementChild !== rt.el) { + if (prev) prev.after(rt.el); + else st.pageEl.prepend(rt.el); + } + }); + want.forEach((panel, i) => { + const rt = st.panels[i]; rt.panel = panel; rt.el.style.setProperty('--story-span6', String(spans6[i])); rt.el.style.setProperty('--story-span2', String(spans2[i])); this._storyFillPanelText(rt, page, i); this._storyQueueDraw(rt); }); - for (const rt of st.panels.splice(want.length)) this._storyDropPanel(rt); // While the turn runs, a blue-pencil rough stands where the next panel will be inked. let pencils = st.pageEl.querySelector('.story-pencils'); @@ -2927,6 +2939,7 @@ Object.assign(CodemanApp.prototype, { if (!st.drawer) st.drawerReturn = trigger || null; st.drawer = source; st.drawerSeen = new Set(); + st.drawerSig = null; st.drawerEl.hidden = false; st.root.classList.add('has-drawer'); // On a phone the drawer is a sheet from the bottom: it starts below the bar, whatever its height. @@ -2947,7 +2960,14 @@ Object.assign(CodemanApp.prototype, { st.drawerReturn = null; // A control rebuilt while the drawer was open: its replacement opens the same thing. const key = back && !document.contains(back) ? back.dataset?.storyOpen : undefined; - if (key) back = [...st.root.querySelectorAll('[data-story-open]')].find((n) => n.dataset.storyOpen === key); + if (key) { + const opens = [...st.root.querySelectorAll('[data-story-open]')]; + // `diff@fix:1` opens the same view of the same panel as `diff@loop:1` once the fix became a loop. + const m = /^([^@]*)@[^:]*:(\d+)$/.exec(key); + back = + opens.find((n) => n.dataset.storyOpen === key) || + (m && opens.find((n) => new RegExp(`^${m[1]}@[^:]*:${m[2]}$`).test(n.dataset.storyOpen))); + } if (refocus && back && document.contains(back)) back.focus({ preventScroll: true }); }, @@ -2992,6 +3012,8 @@ Object.assign(CodemanApp.prototype, { let title = ''; let sub = ''; const out = []; + // Helper boxes, filled once the drawer knows which nodes stay: [box, agentId, working]. + const fills = []; let firstNew = null; const isNew = (id) => !first && !st.drawerSeen.has(id); @@ -3021,7 +3043,7 @@ Object.assign(CodemanApp.prototype, { if (ev.helper?.agentId) { const box = el('div', 'story-helper-steps'); out.push(el('h4', 'story-h', 'What the helper did'), box); - this._storyFillHelperSteps(box, ev.helper.agentId, false); + fills.push([box, ev.helper.agentId, false]); } } if (prompts.some((x) => x.ev.label.length >= 3999)) @@ -3099,7 +3121,7 @@ Object.assign(CodemanApp.prototype, { if (h.agentId) { const box = el('div', 'story-helper-steps'); out.push(el('h4', 'story-h', 'What the helper did'), box); - this._storyFillHelperSteps(box, h.agentId, working); + fills.push([box, h.agentId, working]); } } } else if (view === 'plan') { @@ -3143,8 +3165,33 @@ Object.assign(CodemanApp.prototype, { st.drawerKind.textContent = kind; st.drawerTitle.textContent = title; st.drawerSub.textContent = sub; + // A refresh that changes nothing keeps the nodes, so focus, a text selection and the scroll + // stay put; only a helper still working refreshes its steps. + const sig = [view, kind, title, sub, ...out.map((n) => n.outerHTML)].join('\u0000'); + if (!first && sig === st.drawerSig) { + for (const box of body.querySelectorAll('.story-helper-steps')) + if (box.dataset.working === '1') this._storyFillHelperSteps(box, box.dataset.agent, true); + return; + } + st.drawerSig = sig; + // A real change rebuilds: focus goes back to the control that does the same thing. + const active = document.activeElement; + const controls = (root) => [...root.querySelectorAll('button, a[href], [tabindex]')]; + const was = + active && body.contains(active) + ? { open: active.dataset?.storyOpen, text: active.textContent, at: controls(body).indexOf(active) } + : null; body.replaceChildren(...out); + for (const [box, agentId, working] of fills) this._storyFillHelperSteps(box, agentId, working); for (const { ev } of items) st.drawerSeen.add(ev.id); + if (was) { + const now = controls(body); + const back = + (was.open && now.find((n) => n.dataset?.storyOpen === was.open)) || + now.find((n) => n.textContent === was.text) || + now[Math.min(was.at, now.length - 1)]; + back?.focus({ preventScroll: true }); + } if (first) body.scrollTop = 0; else if (firstNew) body.scrollTop = Math.max(0, firstNew.offsetTop - 12); else body.scrollTop = prevScroll; @@ -3153,6 +3200,7 @@ Object.assign(CodemanApp.prototype, { /** A helper's own steps (GET …/story/helpers/:agentId), cached per helper and revalidated by ETag. */ async _storyFillHelperSteps(box, agentId, working = false) { const st = this._story; + box.dataset.agent = agentId; box.dataset.working = working ? '1' : ''; const cached = st.helperSteps?.get(agentId); if (cached) this._storyRenderHelperSteps(box, cached.data); @@ -3277,7 +3325,15 @@ Object.assign(CodemanApp.prototype, { } section.append(code); if (edit.truncated) - section.append(el('p', 'story-empty', 'Some lines of this edit were left out. Open the file to see all of it.')); + section.append( + el( + 'p', + 'story-empty', + edit.hunks?.length + ? 'Some lines of this edit were left out. Open the file to see all of it.' + : 'What changed is not in the transcript: a script or another program wrote this file. Open it to see it as it is now.' + ) + ); return section; }, diff --git a/test/story-mode-ui.test.ts b/test/story-mode-ui.test.ts index 813ba8fb..16b257bc 100644 --- a/test/story-mode-ui.test.ts +++ b/test/story-mode-ui.test.ts @@ -10,7 +10,7 @@ import { JSDOM } from 'jsdom'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { buildStoryPage, buildStoryTurns, createStoryReducer, summarizeTurns } from '../src/story-mode.js'; import type { StoryPage, StoryPanel } from '../src/types/story.js'; -import { bash, demoTurn, end, prompt, read, resetClock, result, said, tool } from './story-fixtures.js'; +import { bash, demoTurn, edit, end, prompt, read, resetClock, result, said, tool } from './story-fixtures.js'; const STORY_JS = readFileSync(new URL('../src/web/public/story-mode.js', import.meta.url), 'utf-8'); // The real path matcher (findFilePathLinks / resolveLinkedFilePath) the terminal uses. @@ -260,6 +260,83 @@ describe('story-mode.js overlay', () => { expect($('.story-pencils')).toBeNull(); }); + it('a fold near the start keeps every later panel as it was: no second entrance', async () => { + const data = storyOf(demoTurn()); + boot(data); + app.openStoryMode(SID); + await settle(); + sizeAndDraw(); + const before = new Map(app._story.panels.map((rt: { kind: string }) => [rt.kind, rt])); + const folded = JSON.parse(JSON.stringify(data)); + const ps = folded.page.panels; + ps.splice(1, 2, { ...ps[1], kind: 'montage', events: [...ps[1].events, ...ps[2].events] }); + responses = [{ status: 200, body: { success: true, data: folded } }]; + await app._storyRefresh(); + await settle(); + expect($$('.story-panel').map((p) => p.dataset.kind)).toEqual([ + 'mission', + 'montage', + 'backup', + 'loop', + 'save', + 'complete', + ]); + for (const kind of ['mission', 'backup', 'loop', 'save', 'complete']) { + const rt = app._story.panels.find((r: { kind: string }) => r.kind === kind); + expect(rt).toBe(before.get(kind)); + } + expect($('.story-panel[data-kind="montage"]')!.classList.contains('story-fresh')).toBe(true); + expect($('.story-panel[data-kind="loop"]')!.classList.contains('story-fresh')).toBe(false); + }); + + it('a refresh that changes nothing keeps the open drawer as it is, focus included', async () => { + const data = storyOf(demoTurn()); + boot(data); + app.openStoryMode(SID); + await settle(); + sizeAndDraw(); + $('.story-panel[data-kind="loop"] .story-hit')!.click(); + const btn = $('.story-drawer-body button')!; + btn.focus(); + responses = [{ status: 200, body: { success: true, data: JSON.parse(JSON.stringify(data)) } }]; + await app._storyRefresh(); + await settle(); + expect(win.document.contains(btn)).toBe(true); + expect(win.document.activeElement).toBe(btn); + }); + + it('a fix that folds into a loop under an open drawer: Escape gives focus to the loop', async () => { + const lines = [prompt('fix it'), ...edit('e1', '/work/demo-shop/a.py', ['+a = 1'])]; + boot(storyOf(lines)); + app.openStoryMode(SID); + await settle(); + sizeAndDraw(); + const hit = $$('.story-panel[data-kind="fix"] .story-hit').find((h) => h.dataset.storyOpen?.startsWith('diff@'))!; + hit.focus(); + hit.click(); + responses = [ + { + status: 200, + body: { + success: true, + data: storyOf([ + ...lines, + ...bash('b1', 'pytest -q', 'Run tests', '1 failed, 2 passed in 0.1s'), + ...edit('e2', '/work/demo-shop/a.py', ['-a = 1', '+a = 2']), + ...bash('b2', 'pytest -q', 'Run tests', '3 passed in 0.1s'), + ]), + }, + }, + ]; + await app._storyRefresh(); + await settle(); + sizeAndDraw(); + expect($('.story-panel[data-kind="loop"]')).toBeTruthy(); + press('Escape'); + expect($('.story-drawer')!.hidden).toBe(true); + expect((win.document.activeElement as HTMLElement).dataset.storyOpen).toMatch(/^diff@loop:/); + }); + it('activity for this session refreshes, activity for another does not', async () => { vi.useFakeTimers(); try {