From 6156fd0783a31740ef7a48a538c65ec21c1539e6 Mon Sep 17 00:00:00 2001 From: Flood Sung Date: Thu, 14 May 2026 00:51:12 +0000 Subject: [PATCH] =?UTF-8?q?feat(ui):=20compact=20'Agent=20activity=20betwe?= =?UTF-8?q?en=20turns'=20card=20=E2=80=94=20drop=20tool=20noise,=20show=20?= =?UTF-8?q?only=20final=20result?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #268 hid the running tool list from the main card. The "Agent activity between turns" spontaneous card had the same problem one layer deeper β€” it accumulated every assistant message (including bare `tool_use` blocks that rendered as `πŸ”§ Bash`/`πŸ”§ Read` lines) into a numbered list, exactly the play-by-play we just hid from the main card. Apply the same UX bet here: - `extractSpontaneousSnippet`: tool_use-only assistant messages now return null. Text snippets (the agent's actual conclusion) still survive. Mixed messages still surface the text and ignore the adjacent tool_use, same as before. - `formatSpontaneousCardBody`: render only the LATEST snippet (the conclusion of the burst), not a numbered list of all snippets. When N>1 were coalesced, append a small `_(N events coalesced; showing latest)_` footer so the user knows there was more activity if they want to dig into pm2 logs or the web UI. Side effects: - A between-turn burst that produced ONLY tool calls (no agent text) no longer triggers a card at all β€” the buffer stays empty, the 30 s flush is a no-op. This is the correct outcome: nothing user-meaningful to report. - The continuation-turn render path (handleContinuationTurn) was already covered by PR #268 since it goes through the same card-builder.ts surface. No change needed there. Tests: - extractSpontaneousSnippet: assert tool_use-only returns null, text+tool_use still returns the text, multi-tool_use returns null. - formatSpontaneousCardBody: assert single-snippet renders without a numbered prefix; N>1 renders the latest snippet plus the coalesced-count footer, and the earlier snippets are NOT in the body. 290/290 vitest pass. Build + lint clean (2 pre-existing warnings). Co-Authored-By: Claude Opus 4.7 --- src/bridge/message-bridge.ts | 40 +++++++++++++++++--------- tests/message-bridge.test.ts | 55 ++++++++++++++++++++++++++++++------ 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/bridge/message-bridge.ts b/src/bridge/message-bridge.ts index ff05eaf8..7059d73c 100644 --- a/src/bridge/message-bridge.ts +++ b/src/bridge/message-bridge.ts @@ -72,11 +72,16 @@ export const SPONTANEOUS_CARD_HEADER = * Extract a one-line summary from an SDK stream message for the spontaneous * activity card. Returns null if the message has nothing user-readable. * - * Intentionally only handles `assistant` messages β€” `result` messages would - * always duplicate the last assistant text block (SDK's `result.result` is a - * verbatim echo), so including them caused the same content to show up twice - * in the card with two different prefixes. Don't add a `result` branch back - * without first verifying the SDK no longer echoes. + * Intentionally only handles `assistant` *text* blocks β€” same UX bet we made + * for the main card in PR #268: the user only cares about the agent's + * conclusion, not the per-tool play-by-play. `πŸ”§ ` lines used to + * be included for tool_use blocks but that's the exact intermediate noise + * we just hid from the live card; surfacing it on the spontaneous card + * would just re-introduce the same complaint between turns. + * + * Result-type messages are also ignored β€” SDK's `result.result` is a + * verbatim echo of the last assistant text block, so including them caused + * the same content to show up twice in the card. */ export function extractSpontaneousSnippet(msg: unknown): string | null { const m = msg as { type?: string; message?: { content?: Array<{ type?: string; text?: string; name?: string }> } }; @@ -85,20 +90,29 @@ export function extractSpontaneousSnippet(msg: unknown): string | null { if (blk.type === 'text' && blk.text) { const trimmed = String(blk.text).trim(); if (trimmed) return trimmed.slice(0, 400); - } else if (blk.type === 'tool_use' && blk.name) { - return `πŸ”§ ${blk.name}`; } + // tool_use blocks intentionally fall through β€” see docstring above. } return null; } -/** Build the markdown body of a spontaneous activity card from collected snippets. */ +/** + * Build the markdown body of a spontaneous activity card from collected + * snippets. Shows ONLY the latest snippet (the agent's conclusion of the + * burst); if multiple snippets were coalesced, a small footer notes the + * count so users know there was more activity if they want to dig into + * logs. Mirrors the "show only the final result, hide the play-by-play" + * pattern from PR #268's main-card tool indicator. + */ export function formatSpontaneousCardBody(snippets: string[]): string { - return [ - `_${SPONTANEOUS_CARD_HEADER}_`, - '', - ...snippets.map((s, i) => `**${i + 1}.** ${s}`), - ].join('\n'); + const lines: string[] = [`_${SPONTANEOUS_CARD_HEADER}_`, '']; + if (snippets.length === 0) return lines.join('\n'); + lines.push(snippets[snippets.length - 1]); + if (snippets.length > 1) { + lines.push(''); + lines.push(`_(${snippets.length} events coalesced; showing latest)_`); + } + return lines.join('\n'); } interface PendingBatch { diff --git a/tests/message-bridge.test.ts b/tests/message-bridge.test.ts index 172e1eb7..585d34c4 100644 --- a/tests/message-bridge.test.ts +++ b/tests/message-bridge.test.ts @@ -81,15 +81,20 @@ describe('extractSpontaneousSnippet', () => { expect(extractSpontaneousSnippet(msg)).toBe('Weather is sunny'); }); - it('returns πŸ”§ prefixed tool name for tool_use blocks', () => { + // Tool-use blocks used to render as `πŸ”§ ` lines in the + // spontaneous card. That's the exact intermediate noise we hid from the + // main card in PR #268 β€” surfacing it between turns would just put it + // right back. extractSpontaneousSnippet now drops tool_use blocks + // entirely; only text snippets (the agent's actual conclusion) survive. + it('returns null for tool_use-only assistant messages (intermediate noise dropped)', () => { const msg = { type: 'assistant', message: { content: [{ type: 'tool_use', name: 'Bash' }] }, }; - expect(extractSpontaneousSnippet(msg)).toBe('πŸ”§ Bash'); + expect(extractSpontaneousSnippet(msg)).toBeNull(); }); - it('prefers the first usable block (text wins over later tool_use)', () => { + it('returns text and ignores adjacent tool_use blocks', () => { const msg = { type: 'assistant', message: { content: [ @@ -100,6 +105,17 @@ describe('extractSpontaneousSnippet', () => { expect(extractSpontaneousSnippet(msg)).toBe('hello'); }); + it('returns null when only tool_use blocks are present (text-less burst)', () => { + const msg = { + type: 'assistant', + message: { content: [ + { type: 'tool_use', name: 'Read' }, + { type: 'tool_use', name: 'Bash' }, + ] }, + }; + expect(extractSpontaneousSnippet(msg)).toBeNull(); + }); + it('truncates very long text to 400 chars', () => { const long = 'x'.repeat(800); const out = extractSpontaneousSnippet({ @@ -124,7 +140,7 @@ describe('extractSpontaneousSnippet', () => { expect(extractSpontaneousSnippet({})).toBeNull(); }); - it('returns null for assistant messages with no text/tool_use content', () => { + it('returns null for assistant messages with no usable text content', () => { const msg = { type: 'assistant', message: { content: [{ type: 'thinking', text: 'silent' }, { type: 'image' }] }, @@ -142,11 +158,34 @@ describe('extractSpontaneousSnippet', () => { }); describe('formatSpontaneousCardBody', () => { - it('renders snippets as a numbered list under the header', () => { - const body = formatSpontaneousCardBody(['πŸ”§ Bash', 'Weather is sunny']); + // After the post-#268 simplification, the card shows ONLY the latest + // snippet (the agent's conclusion of the burst). Earlier snippets are + // hidden β€” same UX bet as the main card's single-line tool indicator, + // i.e. surface only the final result, not the play-by-play. If the user + // wants the intermediate steps, they can read pm2 logs or the web UI's + // expandable tool view. + it('renders only the latest snippet when a single snippet is present', () => { + const body = formatSpontaneousCardBody(['Weather is sunny']); + expect(body).toContain(SPONTANEOUS_CARD_HEADER); + expect(body).toContain('Weather is sunny'); + // No numbered prefix β€” single snippet doesn't need one. + expect(body).not.toMatch(/\*\*1\.\*\*/); + }); + + it('renders only the latest snippet + a coalesced-count footer when N>1', () => { + const body = formatSpontaneousCardBody([ + 'Looking at the PR comments…', + 'Found 3 things to address.', + 'Pushed commit abc1234 to the branch.', + ]); expect(body).toContain(SPONTANEOUS_CARD_HEADER); - expect(body).toMatch(/\*\*1\.\*\*\s+πŸ”§ Bash/); - expect(body).toMatch(/\*\*2\.\*\*\s+Weather is sunny/); + expect(body).toContain('Pushed commit abc1234 to the branch.'); + expect(body).toMatch(/3 events coalesced/); + // Earlier snippets must NOT appear in the body. + expect(body).not.toContain('Looking at the PR comments'); + expect(body).not.toContain('Found 3 things to address'); + // No numbered list prefixes either. + expect(body).not.toMatch(/\*\*1\.\*\*/); }); // Regression for Bug A (misleading title): the header used to say