mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 22:19:42 +02:00
fix(web): render one Claude response-viewer message per model message
The Claude reader concatenated every assistant row between two human prompts
into one card, fusing up to 74 distinct model messages into a single card, and
it never read the attachment rows that hold a prompt typed while the agent was
working. Measured over 57 real transcripts on 2026-09-01, the viewer shows
1,806 messages instead of 356 and 353 user cards instead of 178, with the
assistant text sequence unchanged row for row and the response without
?context=full byte-identical on all 57 files.
One assistant row IS one whole model message: in that corpus no assistant row
carries more than one content block and no message id carries more than one
text block, so there was nothing to reassemble. Each row becomes its own
message carrying an additive {kind, label, turn}, and the frontend renders a
same-role run inside one turn as badge-less continuation segments — which is
what keeps a p90 of 11 messages per turn from reading as card spam. A numeric
turn gates that rendering, so Codex, the external-CLI pane parser and an older
server keep one badge per card.
A prompt typed while Claude is working is recorded ONLY as an
attachment/queued_command row. Taking it when origin.kind is 'human' and
commandMode is 'prompt' recovers 162 user cards from 163 such rows — one is a
verbatim repeat inside an unanswered user run and is collapsed by the existing
dedup guard — and restores the turn boundary whose absence let the assistant
runs fuse. The CLI's own queue entries are cleanly separable: of 322
queued_command rows, 159 are commandMode 'task-notification' and not one of
them carries an origin key.
This narrows #169 rather than reverting it: sidechain exclusion, the
restored-<uuid8> rebind, replayed-snapshot dedup and synthetic-row filtering
are all unchanged and still asserted.
This commit is contained in:
@@ -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';");
|
||||
});
|
||||
|
||||
@@ -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 = '';
|
||||
});
|
||||
});
|
||||
@@ -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],
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user