From 4c92c34641212595d3753aedcf2b64563fee4f99 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 11:13:27 -0700 Subject: [PATCH] fix(native-chat): match glue against each pending send's own boundary Desktop glue matching filtered candidate transcript rows against the OLDEST still-open send's boundary and then matched the ENTIRE open queue against them. A prompt queued after a row landed could be consumed by that row and pruned, so a real queued prompt disappeared with no bubble and no delivery. Carry every send's own boundary into the match: `gluedCandidateRows` tags each candidate row with the pending indices it landed after, and the matcher ends the glue run at the first send the row predates (adjacency is what makes it glue, so an ineligible entry is a stop, not a skip). Sends with no recorded message boundary fall back to their send time, as the exact path already does. Splits the slash-command marker cache out to native-chat-command-marker.ts to keep native-chat-pending.ts under its line budget; no behavior change there. Fixes STA-4477 Refs #14663, #14262 --- .../NativeChatInteractiveCard.test.tsx | 2 +- .../components/native-chat/NativeChatView.tsx | 12 +- .../native-chat/native-chat-command-marker.ts | 111 +++++++++++++ .../native-chat-pending-occurrence.ts | 63 ++++--- .../native-chat/native-chat-pending.test.ts | 55 +++++- .../native-chat/native-chat-pending.ts | 156 ++++-------------- 6 files changed, 243 insertions(+), 156 deletions(-) create mode 100644 src/renderer/src/components/native-chat/native-chat-command-marker.ts diff --git a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx index 7f5c41034f98..cd08f779e712 100644 --- a/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatInteractiveCard.test.tsx @@ -5,7 +5,7 @@ import '@testing-library/jest-dom/vitest' import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NativeChatMessage } from '../../../../shared/native-chat-types' -import { applyCommandMarkerBoundaries } from './native-chat-pending' +import { applyCommandMarkerBoundaries } from './native-chat-command-marker' import type { NativeChatInteractiveSend } from './use-native-chat-interactive-send' const INITIAL_PROMPT = JSON.stringify({ diff --git a/src/renderer/src/components/native-chat/NativeChatView.tsx b/src/renderer/src/components/native-chat/NativeChatView.tsx index 4da40f7a1fc2..4fc6df0f13b3 100644 --- a/src/renderer/src/components/native-chat/NativeChatView.tsx +++ b/src/renderer/src/components/native-chat/NativeChatView.tsx @@ -18,21 +18,23 @@ import { shouldShowNativeChatWorking } from './native-chat-working-suppression' import { - applyCommandMarkerBoundaries, appendPendingSendCache, - commandMarkersAsMessages, - appendCommandMarkerCache, launchPromptAsMessage, pendingSendsAsMessages, nextNativeChatPendingSendId, prunePendingSends, - readCommandMarkerCache, readPendingSendCache, shouldPruneLaunchPrompt, writePendingSendCache, - type NativeChatCommandMarker, type NativeChatPendingSend } from './native-chat-pending' +import { + appendCommandMarkerCache, + applyCommandMarkerBoundaries, + commandMarkersAsMessages, + readCommandMarkerCache, + type NativeChatCommandMarker +} from './native-chat-command-marker' import { deriveNativeChatStreamingText, nativeChatStreamingMessage diff --git a/src/renderer/src/components/native-chat/native-chat-command-marker.ts b/src/renderer/src/components/native-chat/native-chat-command-marker.ts new file mode 100644 index 000000000000..94634256e80d --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-command-marker.ts @@ -0,0 +1,111 @@ +// Locally-recorded slash commands (e.g. `/clear`). They dispatch into the +// agent's TUI rather than the conversation, so native chat renders its own +// marker line and applies their transcript boundaries — kept out of the +// optimistic-send pruning in native-chat-pending.ts, which is a separate rule. + +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import { setBoundedScopeCacheEntry } from './native-chat-composer-scope-cache' + +/** A locally-recorded slash command (e.g. `/clear`). Slash commands dispatch to + * the agent's TUI and are not chat turns, so we surface a small system line as + * feedback that the command ran rather than echoing a user bubble. */ +export type NativeChatCommandMarker = { + id: string + /** The command as typed, e.g. `/clear`. */ + command: string + sentAt: number +} + +export type NativeChatCommandMarkerScope = { + paneKey: string + agent: string + sessionId: string | null +} + +const COMMAND_MARKER_LIMIT = 8 +const commandMarkerCache = new Map() +let commandMarkerCounter = 0 + +function commandMarkerScopeKey(scope: NativeChatCommandMarkerScope): string { + return `${scope.paneKey}\0${scope.agent}\0${scope.sessionId ?? ''}` +} + +export function readCommandMarkerCache( + scope: NativeChatCommandMarkerScope +): NativeChatCommandMarker[] { + return [...(commandMarkerCache.get(commandMarkerScopeKey(scope)) ?? [])] +} + +export function appendCommandMarkerCache( + scope: NativeChatCommandMarkerScope, + command: string, + sentAt = Date.now() +): NativeChatCommandMarker[] { + commandMarkerCounter += 1 + const key = commandMarkerScopeKey(scope) + // Why: native/TUI view switches remount the chat surface, but slash commands + // are not transcript turns, so their local feedback needs a pane-scoped cache. + const next = [ + ...(commandMarkerCache.get(key) ?? []), + { id: `${sentAt}-${commandMarkerCounter}`, command, sentAt } + ].slice(-COMMAND_MARKER_LIMIT) + // Why: the per-key array is capped at 8, but the KEY (paneKey\0agent\0sessionId, + // sessionId changes on every /clear) is ephemeral and was never evicted, so it + // grew one entry per (pane, session) for the renderer's whole life. LRU-bound + // the key count (mirrors the #7566 draft/attachment caches in this folder). + setBoundedScopeCacheEntry(commandMarkerCache, key, next) + return [...next] +} + +export function clearCommandMarkerCacheForTests(): void { + commandMarkerCache.clear() + commandMarkerCounter = 0 +} + +function isClearCommand(command: string): boolean { + return command.trim().toLowerCase().split(/\s+/)[0] === '/clear' +} + +function latestClearSentAt(markers: readonly NativeChatCommandMarker[]): number | null { + let latest: number | null = null + for (const marker of markers) { + if (isClearCommand(marker.command) && (latest === null || marker.sentAt > latest)) { + latest = marker.sentAt + } + } + return latest +} + +export function applyCommandMarkerBoundaries( + messages: readonly NativeChatMessage[], + markers: readonly NativeChatCommandMarker[] +): NativeChatMessage[] { + const clearSentAt = latestClearSentAt(markers) + if (clearSentAt === null) { + return messages as NativeChatMessage[] + } + // Why: `/clear` mutates the TUI/transcript asynchronously. Hide the current + // transcript immediately so native chat reflects the command before the agent + // writes a replacement session or truncates the file. + return messages.filter((message) => message.timestamp !== null && message.timestamp > clearSentAt) +} + +/** Render command markers as compact `system` messages. The `system` role draws + * as a muted aside (not a user bubble); the text avoids the harness noise + * prefixes so stripNoiseMessages keeps it. */ +export function commandMarkersAsMessages( + markers: readonly NativeChatCommandMarker[] +): NativeChatMessage[] { + return markers.map((marker) => ({ + id: `command:${marker.id}`, + role: 'system' as const, + blocks: [{ type: 'text' as const, text: `Ran ${marker.command}` }], + timestamp: marker.sentAt, + source: 'scrape' as const + })) +} + +/** True when a message id was minted for a slash-command marker. */ +export function isCommandMarkerId(id: string): boolean { + return id.startsWith('command:') +} diff --git a/src/renderer/src/components/native-chat/native-chat-pending-occurrence.ts b/src/renderer/src/components/native-chat/native-chat-pending-occurrence.ts index 1184affb3690..bdabded3051f 100644 --- a/src/renderer/src/components/native-chat/native-chat-pending-occurrence.ts +++ b/src/renderer/src/components/native-chat/native-chat-pending-occurrence.ts @@ -79,17 +79,21 @@ export function advancedNativeChatUserContentCounts( return advanced } -/** User texts that already have a later non-user turn (ready to prune echoes). */ -export function advancedNativeChatUserTexts( +/** A transcript user turn, kept identifiable so a caller can decide which + * pending sends it is allowed to represent. */ +export type NativeChatUserRow = { id: string; text: string } + +/** User rows that already have a later non-user turn (ready to prune echoes). */ +export function advancedNativeChatUserRows( messages: readonly NativeChatMessage[] -): readonly string[] { - const advanced: string[] = [] - const waiting: string[] = [] +): readonly NativeChatUserRow[] { + const advanced: NativeChatUserRow[] = [] + const waiting: NativeChatUserRow[] = [] for (const message of messages) { if (message.role === 'user') { const text = normalizedNativeChatUserMessageText(message) if (text) { - waiting.push(text) + waiting.push({ id: message.id, text }) } continue } @@ -99,18 +103,18 @@ export function advancedNativeChatUserTexts( return advanced } -/** All user texts (for hiding optimistic echoes once the turn exists). */ -export function matchingNativeChatUserTexts( +/** All user rows (for hiding optimistic echoes once the turn exists). */ +export function matchingNativeChatUserRows( messages: readonly NativeChatMessage[] -): readonly string[] { - const texts: string[] = [] +): readonly NativeChatUserRow[] { + const rows: NativeChatUserRow[] = [] for (const message of messages) { const text = normalizedNativeChatUserMessageText(message) if (text) { - texts.push(text) + rows.push({ id: message.id, text }) } } - return texts + return rows } /** @@ -149,32 +153,49 @@ export function countLeadingPendingTextsGluedToUserText( return 0 } +/** A transcript row a glue match may consume, carrying the send boundaries it + * satisfies — this matcher has no clock of its own. */ +export type NativeChatGluedUserRow = { + text: string + /** Indices into `pending` this row landed after. A row that already existed + * when a send was issued can never be that send's echo. */ + representablePendingIndices: ReadonlySet +} + /** * Mark pending entries represented only by multi-send glue (2+ consecutive * optimistic texts concatenated into one transcript user row). Exact single * matches stay in the content-key/occurrence path so repeated prompts and * send boundaries keep their existing semantics. - * - * `userTexts` must already be filtered to rows after the oldest entry's send - * boundary — this matcher has no clock of its own. */ -export function selectPendingIndicesRepresentedByUserTexts( +export function selectPendingIndicesRepresentedByUserRows( pending: readonly NativeChatPendingOccurrence[], - userTexts: readonly string[] + rows: readonly NativeChatGluedUserRow[] ): Set { const represented = new Set() - if (pending.length < 2 || userTexts.length === 0) { + if (pending.length < 2 || rows.length === 0) { return represented } const remaining = pending.map((entry, index) => ({ index, text: normalizeNativeChatPendingText(entry.text) })) - for (const userText of userTexts) { - const open = remaining.filter((entry) => !represented.has(entry.index) && entry.text.length > 0) + for (const row of rows) { + const open: typeof remaining = [] + for (const entry of remaining) { + if (represented.has(entry.index) || entry.text.length === 0) { + continue + } + // Glue consumes a leading run, so a send this row predates ends the run + // rather than being skipped over — adjacency is what makes it glue. + if (!row.representablePendingIndices.has(entry.index)) { + break + } + open.push(entry) + } const gluedCount = countLeadingPendingTextsGluedToUserText( open.map((entry) => entry.text), - userText + row.text ) // Why: gluedCount === 1 is an exact match — leave it to occurrence counting. if (gluedCount < 2) { diff --git a/src/renderer/src/components/native-chat/native-chat-pending.test.ts b/src/renderer/src/components/native-chat/native-chat-pending.test.ts index 29771fd9507b..67da71e11c01 100644 --- a/src/renderer/src/components/native-chat/native-chat-pending.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-pending.test.ts @@ -2,24 +2,26 @@ import { describe, expect, it } from 'vitest' import type { NativeChatMessage } from '../../../../shared/native-chat-types' import { appendPendingSendCache, - appendCommandMarkerCache, - applyCommandMarkerBoundaries, - clearCommandMarkerCacheForTests, clearPendingSendCacheForTests, - commandMarkersAsMessages, - isCommandMarkerId, isLaunchPromptMessageId, isPendingMessageId, launchPromptAsMessage, nextNativeChatPendingSendId, pendingSendsAsMessages, prunePendingSends, - readCommandMarkerCache, readPendingSendCache, shouldPruneLaunchPrompt, writePendingSendCache, type NativeChatPendingSend } from './native-chat-pending' +import { + appendCommandMarkerCache, + applyCommandMarkerBoundaries, + clearCommandMarkerCacheForTests, + commandMarkersAsMessages, + isCommandMarkerId, + readCommandMarkerCache +} from './native-chat-command-marker' import { stripNoiseMessages } from './native-chat-noise' function userMessage(id: string, text: string, timestamp = 1): NativeChatMessage { @@ -330,6 +332,47 @@ describe('glued rapid sends', () => { expect(prunePendingSends(pending, history)).toEqual(pending) expect(pendingSendsAsMessages(pending, history)).toHaveLength(2) }) + + // A row that already existed when a send was issued can never be that send's + // echo — for EVERY queued send, not just the oldest one the glue run starts at. + it('keeps a queued send whose own boundary is newer than the glued row (STA-4477)', () => { + const gluedRow = userMessage('glue-row', 'fix the bug', 6000) + const messages = [ + GLUE_BOUNDARY, + gluedRow, + assistantMessage('glue-answer', 'fixed', 6100), + userMessage('later-history', 'anything', 6200) + ] + // 'fix the' was queued before the row landed; 'bug' only after it. + const pending = [ + gluePending('p1', 'fix the'), + { + id: 'p2', + text: 'bug', + sentAt: 7000, + afterMessageId: 'glue-answer', + afterMessageTimestamp: 6100 + } + ] + + expect(prunePendingSends(pending, messages)).toEqual(pending) + expect(pendingSendsAsMessages(pending, messages)).toHaveLength(2) + }) + + it('still retires the leading run the glued row did land after (STA-4477)', () => { + const gluedRow = userMessage('glue-row', 'first second', 6000) + const messages = [GLUE_BOUNDARY, gluedRow, assistantMessage('glue-answer', 'done', 6100)] + const third = { + id: 'p3', + text: 'third', + sentAt: 7000, + afterMessageId: 'glue-row', + afterMessageTimestamp: gluedRow.timestamp + } + const pending = [gluePending('p1', 'first'), gluePending('p2', 'second'), third] + + expect(prunePendingSends(pending, messages)).toEqual([third]) + }) }) describe('pendingSendsAsMessages', () => { diff --git a/src/renderer/src/components/native-chat/native-chat-pending.ts b/src/renderer/src/components/native-chat/native-chat-pending.ts index c322656f1526..dcf47ea6f2fd 100644 --- a/src/renderer/src/components/native-chat/native-chat-pending.ts +++ b/src/renderer/src/components/native-chat/native-chat-pending.ts @@ -8,15 +8,17 @@ import { setBoundedScopeCacheEntry } from './native-chat-composer-scope-cache' import type { NativeChatLaunchPrompt } from '@/lib/native-chat-launch-prompt' import { advancedNativeChatUserContentCounts, - advancedNativeChatUserTexts, + advancedNativeChatUserRows, assignNativeChatPendingOccurrence, matchingNativeChatUserContentCounts, - matchingNativeChatUserTexts, + matchingNativeChatUserRows, nativeChatPendingContentKey, nativeChatPendingMatchKey, nativeChatPendingMatchingAfter, nativeChatPendingOccurrence, - selectPendingIndicesRepresentedByUserTexts + selectPendingIndicesRepresentedByUserRows, + type NativeChatGluedUserRow, + type NativeChatUserRow } from './native-chat-pending-occurrence' /** An optimistic, not-yet-confirmed composer send. */ @@ -126,23 +128,35 @@ function messageIsAfterPendingTimestamp( } /** - * Rows a glue match may consume. Glue always starts at the oldest still-open - * echo, so that echo's send boundary is the floor: unbounded, an older turn - * whose text happens to split across the queue ("fix the bug" vs "fix the" + - * "bug") would retire sends issued long after it. A missing message boundary - * falls back to send time — a fuzzy match must never reach further back than - * an exact one. + * Rows a glue match may consume, each tagged with the sends it landed after. + * Unbounded, an older turn whose text happens to split across the queue ("fix + * the bug" vs "fix the" + "bug") would retire sends issued long after it — so + * every send carries its OWN boundary into the match, not just the oldest one + * the run starts at (#14663 pruned newer queued prompts against older rows). + * A missing message boundary falls back to send time: a fuzzy match must never + * reach further back than an exact one. */ -function gluedCandidateMessages( +function gluedCandidateRows( messages: readonly NativeChatMessage[], - open: readonly NativeChatPendingSend[] -): readonly NativeChatMessage[] { - const oldest = open[0] + open: readonly NativeChatPendingSend[], + rowsOf: (messages: readonly NativeChatMessage[]) => readonly NativeChatUserRow[] +): readonly NativeChatGluedUserRow[] { + const anchored = open.map((entry) => + entry.afterMessageId === undefined ? { ...entry, afterMessageId: null } : entry + ) + const oldest = anchored[0] if (!oldest) { return [] } - const anchor = oldest.afterMessageId === undefined ? { ...oldest, afterMessageId: null } : oldest - return messagesAfterPendingBoundary(messages, anchor) + const rowIdsAfterOwnBoundary = anchored.map( + (entry) => new Set(rowsOf(messagesAfterPendingBoundary(messages, entry)).map((row) => row.id)) + ) + return rowsOf(messagesAfterPendingBoundary(messages, oldest)).map((row) => ({ + text: row.text, + representablePendingIndices: new Set( + rowIdsAfterOwnBoundary.flatMap((ids, index) => (ids.has(row.id) ? [index] : [])) + ) + })) } /** @@ -175,9 +189,9 @@ export function prunePendingSends( // transcript carries one row ("joke"+"continue"→"jokecontinue") that no exact // key matches. Drop those echoes once an assistant turn advances past it. const stillOpen = pending.filter((_, index) => exactKeep[index]) - const gluedRepresented = selectPendingIndicesRepresentedByUserTexts( + const gluedRepresented = selectPendingIndicesRepresentedByUserRows( stillOpen, - advancedNativeChatUserTexts(gluedCandidateMessages(messages, stillOpen)) + gluedCandidateRows(messages, stillOpen, advancedNativeChatUserRows) ) const next = pending.filter((entry, index) => { if (!exactKeep[index]) { @@ -218,9 +232,9 @@ export function pendingSendsAsMessages( // Hide optimistic echoes that were glued into a single transcript user row // even before the assistant reply lands (matching, not advanced). const stillVisible = pending.filter((_, index) => exactVisible[index]) - const gluedRepresented = selectPendingIndicesRepresentedByUserTexts( + const gluedRepresented = selectPendingIndicesRepresentedByUserRows( stillVisible, - matchingNativeChatUserTexts(gluedCandidateMessages(existingMessages, stillVisible)) + gluedCandidateRows(existingMessages, stillVisible, matchingNativeChatUserRows) ) return pending .filter((entry, index) => { @@ -299,107 +313,3 @@ export function nextNativeChatPendingSendId(now = Date.now()): string { export function isLaunchPromptMessageId(id: string): boolean { return id.startsWith('launch-pending:') } - -/** A locally-recorded slash command (e.g. `/clear`). Slash commands dispatch to - * the agent's TUI and are not chat turns, so we surface a small system line as - * feedback that the command ran rather than echoing a user bubble. */ -export type NativeChatCommandMarker = { - id: string - /** The command as typed, e.g. `/clear`. */ - command: string - sentAt: number -} - -export type NativeChatCommandMarkerScope = { - paneKey: string - agent: string - sessionId: string | null -} - -const COMMAND_MARKER_LIMIT = 8 -const commandMarkerCache = new Map() -let commandMarkerCounter = 0 - -function commandMarkerScopeKey(scope: NativeChatCommandMarkerScope): string { - return `${scope.paneKey}\0${scope.agent}\0${scope.sessionId ?? ''}` -} - -export function readCommandMarkerCache( - scope: NativeChatCommandMarkerScope -): NativeChatCommandMarker[] { - return [...(commandMarkerCache.get(commandMarkerScopeKey(scope)) ?? [])] -} - -export function appendCommandMarkerCache( - scope: NativeChatCommandMarkerScope, - command: string, - sentAt = Date.now() -): NativeChatCommandMarker[] { - commandMarkerCounter += 1 - const key = commandMarkerScopeKey(scope) - // Why: native/TUI view switches remount the chat surface, but slash commands - // are not transcript turns, so their local feedback needs a pane-scoped cache. - const next = [ - ...(commandMarkerCache.get(key) ?? []), - { id: `${sentAt}-${commandMarkerCounter}`, command, sentAt } - ].slice(-COMMAND_MARKER_LIMIT) - // Why: the per-key array is capped at 8, but the KEY (paneKey\0agent\0sessionId, - // sessionId changes on every /clear) is ephemeral and was never evicted, so it - // grew one entry per (pane, session) for the renderer's whole life. LRU-bound - // the key count (mirrors the #7566 draft/attachment caches in this folder). - setBoundedScopeCacheEntry(commandMarkerCache, key, next) - return [...next] -} - -export function clearCommandMarkerCacheForTests(): void { - commandMarkerCache.clear() - commandMarkerCounter = 0 -} - -function isClearCommand(command: string): boolean { - return command.trim().toLowerCase().split(/\s+/)[0] === '/clear' -} - -function latestClearSentAt(markers: readonly NativeChatCommandMarker[]): number | null { - let latest: number | null = null - for (const marker of markers) { - if (isClearCommand(marker.command) && (latest === null || marker.sentAt > latest)) { - latest = marker.sentAt - } - } - return latest -} - -export function applyCommandMarkerBoundaries( - messages: readonly NativeChatMessage[], - markers: readonly NativeChatCommandMarker[] -): NativeChatMessage[] { - const clearSentAt = latestClearSentAt(markers) - if (clearSentAt === null) { - return messages as NativeChatMessage[] - } - // Why: `/clear` mutates the TUI/transcript asynchronously. Hide the current - // transcript immediately so native chat reflects the command before the agent - // writes a replacement session or truncates the file. - return messages.filter((message) => message.timestamp !== null && message.timestamp > clearSentAt) -} - -/** Render command markers as compact `system` messages. The `system` role draws - * as a muted aside (not a user bubble); the text avoids the harness noise - * prefixes so stripNoiseMessages keeps it. */ -export function commandMarkersAsMessages( - markers: readonly NativeChatCommandMarker[] -): NativeChatMessage[] { - return markers.map((marker) => ({ - id: `command:${marker.id}`, - role: 'system' as const, - blocks: [{ type: 'text' as const, text: `Ran ${marker.command}` }], - timestamp: marker.sentAt, - source: 'scrape' as const - })) -} - -/** True when a message id was minted for a slash-command marker. */ -export function isCommandMarkerId(id: string): boolean { - return id.startsWith('command:') -}