Merge pull request #369 from shenlvkang-collab/pr/claude-response-viewer-per-message

fix(web): render one Claude response-viewer message per model message

test/frontend-public-tooling.test.ts

@@ -24,7 +24,15 @@ describe('frontend public asset tooling', () => {
     const appJs = readFileSync(resolve(repoRoot, 'src/web/public/app.js'), 'utf8');
 
     expect(appJs).toContain("body.appendChild(this._buildResponseViewerMessage(lastResponse, 'assistant'");
-    expect(appJs).toContain('body.appendChild(this._buildResponseViewerMessage(msg.text, msg.role, agentLabel));');
+    expect(appJs).toContain(
+      'body.appendChild(this._buildResponseViewerMessage(msg.text, msg.role, agentLabel, { ...msg, continuation }));'
+    );
+    // ⚠️ A numeric `turn` gates continuation rendering. Only the Claude reader
+    // emits turns; Codex, the external-CLI pane parser and an older server emit
+    // adjacent same-role messages with none, and must keep one badge per card.
+    expect(appJs).toContain(
+      "!!previous && previous.role === msg.role && typeof msg.turn === 'number' && previous.turn === msg.turn"
+    );
     expect(appJs).toContain("div.className = 'rv-message ' + (isUser ? 'rv-msg-user' : 'rv-msg-assistant');");
     expect(appJs).toContain("renderedText.className = 'rv-text';");
   });

test/response-viewer-turn-segments.test.ts

@@ -0,0 +1,149 @@
+/**
+ * @fileoverview Response-viewer turn segmentation (`CodemanApp._buildResponseViewerMessage`).
+ *
+ * The server now emits one message per model message instead of concatenating a
+ * human turn's replies into one card, so a long autonomous run arrives as tens
+ * of messages rather than one 12,000-character block. Rendered naively that is
+ * card spam — the measured distribution is p50 3 messages per turn, p90 11,
+ * max 51, with 58% of messages under 80 characters. So consecutive messages
+ * from one speaker inside one `turn` render as SEGMENTS of one card: no
+ * repeated role badge, a hairline seam.
+ *
+ * Pinned here because the badge suppression is the only thing standing between
+ * the server change and a wall of 51 "Claude" badges:
+ *
+ *  1. A continuation carries `rv-msg-cont` and has NO `.rv-role` child, while
+ *     keeping its role class so the CSS accent survives (the colour rules match
+ *     on both `:has(.rv-role-*)` and `.rv-msg-*` — only the class arm hits here).
+ *  2. A queued prompt is marked in the DOM, not in text, so the i18n
+ *     MutationObserver cannot rewrite the marker.
+ *  3. The 4th argument is genuinely optional: the brief view's 3-argument call
+ *     still renders a badge.
+ *
+ * Loaded via `vm` with a jsdom document injected (same technique as
+ * response-viewer-file-links.test.ts).
+ * Port: N/A
+ */
+import { readFileSync } from 'node:fs';
+import { performance } from 'node:perf_hooks';
+import { resolve } from 'node:path';
+import vm from 'node:vm';
+import { JSDOM } from 'jsdom';
+import { describe, expect, it, vi } from 'vitest';
+
+const dom = new JSDOM('<!DOCTYPE html><html><body></body></html>');
+const { document, NodeFilter } = dom.window;
+
+interface MessageBuilder {
+  _buildResponseViewerMessage(text: string, role: string, agentLabel: string, meta?: unknown): HTMLElement;
+  loadFullContext(): Promise<void>;
+  activeSessionId?: string;
+}
+
+function loadCodemanAppClass() {
+  const constants = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
+  const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
+  const context = vm.createContext({
+    console,
+    performance,
+    setInterval: vi.fn(),
+    clearInterval: vi.fn(),
+    setTimeout,
+    clearTimeout,
+    requestAnimationFrame: vi.fn(),
+    HTMLCanvasElement: class HTMLCanvasElement {},
+    fetch: vi.fn(),
+    document,
+    NodeFilter,
+    localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() },
+    window: { addEventListener: vi.fn(), removeEventListener: vi.fn() },
+    MobileDetection: {},
+  });
+  vm.runInContext(`${constants}\n${source}\nglobalThis.__CodemanApp = CodemanApp;`, context);
+  // The context is returned too: app.js closes over the context's own `fetch`, so a
+  // test that drives loadFullContext has to replace THAT binding, not globalThis'.
+  return { CodemanApp: (context as { __CodemanApp: { prototype: MessageBuilder } }).__CodemanApp, context };
+}
+
+const { CodemanApp, context: appContext } = loadCodemanAppClass();
+
+function build(text: string, role: string, meta?: unknown): HTMLElement {
+  const app = Object.create(CodemanApp.prototype) as MessageBuilder;
+  return app._buildResponseViewerMessage(text, role, 'Claude', meta);
+}
+
+describe('response viewer turn segmentation', () => {
+  it('renders a continuation without a repeated role badge but keeps its role class', () => {
+    const div = build('second half of the same turn', 'assistant', { continuation: true, kind: 'response', turn: 3 });
+
+    expect(div.classList.contains('rv-msg-cont')).toBe(true);
+    expect(div.classList.contains('rv-msg-assistant')).toBe(true);
+    expect(div.querySelector('.rv-role')).toBeNull();
+    expect(div.querySelector('.rv-text')).not.toBeNull();
+    expect(div.dataset.kind).toBe('response');
+  });
+
+  it('marks a prompt the user queued mid-turn in the DOM, not in the text', () => {
+    const div = build('actually use PowerShell', 'user', {
+      continuation: false,
+      kind: 'prompt',
+      queued: true,
+      turn: 2,
+    });
+
+    expect(div.dataset.queued).toBe('1');
+    expect(div.dataset.kind).toBe('prompt');
+    const badge = div.querySelector('.rv-role');
+    expect(badge).not.toBeNull();
+    expect(badge!.classList.contains('rv-role-user')).toBe(true);
+    // The marker is a CSS pseudo-element, so the badge text stays translatable.
+    expect(badge!.textContent).toBe('You');
+  });
+
+  it('still renders a badge for the brief view, which passes no meta', () => {
+    const div = build('the last response', 'assistant');
+
+    expect(div.classList.contains('rv-msg-cont')).toBe(false);
+    expect(div.querySelector('.rv-role')!.textContent).toBe('Claude');
+    expect(div.dataset.kind).toBeUndefined();
+    expect(div.dataset.queued).toBeUndefined();
+  });
+});
+
+/**
+ * The empty-state branch deliberately does NOT wipe the body — the brief view has
+ * a terminal-buffer fallback this endpoint does not — and deliberately leaves the
+ * More button live so a transcript that appears a moment later can still be
+ * loaded. Both together mean the notice must be idempotent: without that, every
+ * retry stacks another identical line. Upstream got this for free because it
+ * assigned `body.textContent`.
+ */
+describe('response viewer empty full-context state', () => {
+  it('reuses one notice across repeated More clicks and keeps the brief card', async () => {
+    const body = document.createElement('div');
+    body.id = 'responseViewerBody';
+    const title = document.createElement('div');
+    title.id = 'responseViewerTitle';
+    const more = document.createElement('button');
+    more.id = 'responseViewerMore';
+    document.body.append(body, title, more);
+    body.textContent = 'No response yet — send a message in this session first.';
+
+    const app = Object.create(CodemanApp.prototype) as MessageBuilder;
+    app.activeSessionId = 's1';
+    (appContext as { fetch: unknown }).fetch = vi.fn(async () => ({
+      json: async () => ({ data: { messages: [] } }),
+    }));
+
+    await app.loadFullContext();
+    await app.loadFullContext();
+    await app.loadFullContext();
+
+    expect(body.querySelectorAll('.rv-notice')).toHaveLength(1);
+    expect(body.textContent).toContain('No response yet');
+    // More stays clickable: it is the only retry path once a transcript lands.
+    expect(more.style.display).toBe('');
+
+    document.body.innerHTML = '';
+  });
+});

test/routes/session-routes-claude-last-response.test.ts

@@ -60,6 +60,24 @@ const assistantEntry = (text: string, timestamp: string) => ({
   message: { content: [{ type: 'text', text }] },
 });
 
+/**
+ * A prompt typed while Claude is working. Shape copied from a real CLI 2.1.251
+ * row: the CLI's own queue entries carry commandMode 'task-notification' and no
+ * `origin` key at all, which is what separates them from the human's.
+ */
+const queuedEntry = (prompt: string, timestamp: string, kind: 'human' | 'task-notification' = 'human') => ({
+  type: 'attachment',
+  timestamp,
+  attachment: {
+    type: 'queued_command',
+    prompt,
+    source_uuid: `src-${timestamp}`,
+    commandMode: kind === 'human' ? 'prompt' : 'task-notification',
+    ...(kind === 'human' ? { origin: { kind: 'human' } } : {}),
+    timestamp,
+  },
+});
+
 describe('GET /api/sessions/:id/last-response (claude)', () => {
   let harness: LocalHarness;
   let testHome: string;
@@ -93,7 +111,7 @@ describe('GET /api/sessions/:id/last-response (claude)', () => {
     return { response, body: JSON.parse(response.body) };
   }
 
-  it('recovers a placeholder tmux session by UUID prefix and groups JSONL fragments into turns', async () => {
+  it('recovers a placeholder tmux session by UUID prefix and renders one message per model message', async () => {
     const restoredId = 'restored-40568a29';
     const conversationId = '40568a29-d4eb-4eb6-b671-8401428e4f39';
     const session = harness.ctx._session as typeof harness.ctx._session & {
@@ -135,15 +153,60 @@ describe('GET /api/sessions/:id/last-response (claude)', () => {
     expect(full.body.data).toEqual({
       text: 'Second half.',
       timestamp: '2026-07-21T00:00:06Z',
+      // #169's guarantees all still hold and this array proves them: the replayed
+      // 'first prompt' row, the replayed 'Checking the files.' snapshot, the
+      // sidechain row and all five synthetic rows are absent. Only the GROUPING
+      // UNIT narrows, from one card per human turn to one card per model
+      // message, carried by `turn` instead of by a '\n\n' joiner.
       messages: [
-        { role: 'user', text: 'first prompt', timestamp: '2026-07-21T00:00:00Z' },
         {
+          kind: 'prompt',
+          label: 'Prompt',
+          role: 'user',
+          text: 'first prompt',
+          timestamp: '2026-07-21T00:00:00Z',
+          turn: 1,
+        },
+        {
+          kind: 'response',
+          label: 'Response',
           role: 'assistant',
-          text: 'Checking the files.\n\nThe first result is ready.',
+          text: 'Checking the files.',
+          timestamp: '2026-07-21T00:00:01Z',
+          turn: 1,
+        },
+        {
+          kind: 'response',
+          label: 'Response',
+          role: 'assistant',
+          text: 'The first result is ready.',
           timestamp: '2026-07-21T00:00:03Z',
+          turn: 1,
+        },
+        {
+          kind: 'prompt',
+          label: 'Prompt',
+          role: 'user',
+          text: 'second prompt',
+          timestamp: '2026-07-21T00:00:00Z',
+          turn: 2,
+        },
+        {
+          kind: 'response',
+          label: 'Response',
+          role: 'assistant',
+          text: 'First half.',
+          timestamp: '2026-07-21T00:00:04Z',
+          turn: 2,
+        },
+        {
+          kind: 'response',
+          label: 'Response',
+          role: 'assistant',
+          text: 'Second half.',
+          timestamp: '2026-07-21T00:00:06Z',
+          turn: 2,
         },
-        { role: 'user', text: 'second prompt', timestamp: '2026-07-21T00:00:00Z' },
-        { role: 'assistant', text: 'First half.\n\nSecond half.', timestamp: '2026-07-21T00:00:06Z' },
       ],
     });
     expect(session.adoptClaudeSessionId).toHaveBeenCalledWith(conversationId);
@@ -175,6 +238,137 @@ describe('GET /api/sessions/:id/last-response (claude)', () => {
       ['assistant', 'Second answer.'],
     ]);
   });
+
+  /**
+   * A prompt typed while Claude is working is absorbed mid-turn and recorded
+   * ONLY as an attachment row — 160 of the 347 user cards across a real
+   * ~/.claude/projects. Reading only `user` rows lost them outright AND lost the
+   * turn boundary they carry, which is what let an assistant run fuse.
+   */
+  it('surfaces a prompt the user queued while Claude was working', async () => {
+    const sessionId = harness.ctx._session.id;
+    const session = harness.ctx._session as typeof harness.ctx._session & {
+      claudeSessionId: string;
+      adoptClaudeSessionId: ReturnType<typeof vi.fn>;
+    };
+    session.claudeSessionId = sessionId;
+    session.adoptClaudeSessionId = vi.fn();
+    writeTranscript(sessionId, [
+      userEntry('start the job'),
+      assistantEntry('Working on it.', '2026-07-21T00:00:01Z'),
+      queuedEntry('actually use PowerShell', '2026-07-21T00:00:02Z'),
+      queuedEntry('background agent finished', '2026-07-21T00:00:03Z', 'task-notification'),
+      // The most common attachment subtype; it carries no prompt/origin at all.
+      { type: 'attachment', attachment: { type: 'total_tokens_reminder', tokens: 1 } },
+      assistantEntry('Switched to PowerShell.', '2026-07-21T00:00:04Z'),
+    ]);
+
+    const { body } = await getLastResponse(sessionId, true);
+    const messages = body.data.messages as Array<{ role: string; text: string; turn: number; queued?: boolean }>;
+    expect(messages.map((message) => [message.role, message.text, message.turn])).toEqual([
+      ['user', 'start the job', 1],
+      ['assistant', 'Working on it.', 1],
+      ['user', 'actually use PowerShell', 2],
+      ['assistant', 'Switched to PowerShell.', 2],
+    ]);
+    expect(messages[2].queued).toBe(true);
+    expect(messages[0].queued).toBeUndefined();
+  });
+
+  /**
+   * Mostly forward insurance. A queued prompt re-emitted as a `user` row AFTER
+   * its attachment row — the shape that would double-render — is not observed on
+   * CLI 2.1.220-2.1.251 (0 of 163 measured 2026-09-01). The only exact-text
+   * collisions are three occurrences of the same one-character nudge in a single
+   * transcript, and the guard fires on one of them, which is why 163 human
+   * queued rows yield 162 cards. The guard exists so a CLI that starts writing
+   * both rows does not double every absorbed prompt.
+   */
+  it('renders an absorbed prompt once when the CLI also writes it as a user row', async () => {
+    const sessionId = harness.ctx._session.id;
+    const session = harness.ctx._session as typeof harness.ctx._session & {
+      claudeSessionId: string;
+      adoptClaudeSessionId: ReturnType<typeof vi.fn>;
+    };
+    session.claudeSessionId = sessionId;
+    session.adoptClaudeSessionId = vi.fn();
+    writeTranscript(sessionId, [
+      userEntry('go'),
+      assistantEntry('OK.', '2026-07-21T00:00:01Z'),
+      queuedEntry('switch to PowerShell', '2026-07-21T00:00:02Z'),
+      userEntry('switch to PowerShell'),
+      assistantEntry('Done.', '2026-07-21T00:00:03Z'),
+    ]);
+
+    const { body } = await getLastResponse(sessionId, true);
+    const messages = body.data.messages as Array<{ role: string; text: string; queued?: boolean }>;
+    const absorbed = messages.filter((message) => message.role === 'user' && message.text === 'switch to PowerShell');
+    expect(absorbed).toHaveLength(1);
+    expect(absorbed[0].queued).toBe(true);
+  });
+
+  /**
+   * The brief response is what agent pollers hash (skills/codeman/preamble.sh
+   * last_text()). It must stay the last assistant row and must NEVER be derived
+   * from messages.at(-1), which can be the user's own queued prompt.
+   */
+  it('keeps the brief response on the last assistant row while a turn is in flight', async () => {
+    const sessionId = harness.ctx._session.id;
+    const session = harness.ctx._session as typeof harness.ctx._session & {
+      claudeSessionId: string;
+      adoptClaudeSessionId: ReturnType<typeof vi.fn>;
+    };
+    session.claudeSessionId = sessionId;
+    session.adoptClaudeSessionId = vi.fn();
+    writeTranscript(sessionId, [
+      userEntry('go'),
+      assistantEntry('Let me look.', '2026-07-21T00:00:01Z'),
+      { type: 'assistant', message: { content: [{ type: 'tool_use', id: 'x' }] } },
+      { type: 'user', message: { content: [{ type: 'tool_result', tool_use_id: 'x' }] } },
+    ]);
+
+    const brief = await getLastResponse(sessionId);
+    expect(brief.body.data).toEqual({ text: 'Let me look.', timestamp: '2026-07-21T00:00:01Z' });
+
+    const { body } = await getLastResponse(sessionId, true);
+    const messages = body.data.messages as Array<{ role: string; text: string }>;
+    expect(messages.at(-1)).toMatchObject({ role: 'assistant', text: 'Let me look.' });
+    expect(body.data.text).toBe('Let me look.');
+  });
+
+  /**
+   * A multi-line paste absorbed mid-turn arrives as N queued rows within a few
+   * hundred milliseconds (observed: 5 rows inside ~360ms). They are one turn, so
+   * the viewer renders them under one badge instead of N.
+   */
+  it('groups a burst of queued prompts into one turn', async () => {
+    const sessionId = harness.ctx._session.id;
+    const session = harness.ctx._session as typeof harness.ctx._session & {
+      claudeSessionId: string;
+      adoptClaudeSessionId: ReturnType<typeof vi.fn>;
+    };
+    session.claudeSessionId = sessionId;
+    session.adoptClaudeSessionId = vi.fn();
+    writeTranscript(sessionId, [
+      userEntry('go'),
+      assistantEntry('OK.', '2026-07-21T00:00:01Z'),
+      queuedEntry('one more thing', '2026-07-21T00:00:02.100Z'),
+      queuedEntry('and the requirements are', '2026-07-21T00:00:02.360Z'),
+      queuedEntry('finally, keep it fast', '2026-07-21T00:00:02.480Z'),
+      assistantEntry('Understood.', '2026-07-21T00:00:05Z'),
+    ]);
+
+    const { body } = await getLastResponse(sessionId, true);
+    const messages = body.data.messages as Array<{ role: string; turn: number }>;
+    expect(messages.map((message) => [message.role, message.turn])).toEqual([
+      ['user', 1],
+      ['assistant', 1],
+      ['user', 2],
+      ['user', 2],
+      ['user', 2],
+      ['assistant', 2],
+    ]);
+  });
 });
 
 /**