Skip to content

Commit 4b68fc2

Browse files
authored
Merge pull request #430 from Mng-dev-ai/fix/streaming-projection-truncation-and-queue-handoff
Fix streaming content truncation on unmount and queue handoff drop
2 parents f317e6b + cc8ed3e commit 4b68fc2

3 files changed

Lines changed: 103 additions & 42 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
- Never use `useEffect` to derive state from other state or props — if state is always computable from existing values, use inline computation or `useMemo`; `useEffect(() => setX(f(y)), [y])` causes an unnecessary extra render cycle
120120
- When state must reset on a prop/ID change, use a ref-based render check instead of `useEffect` — pattern: `const prevRef = useRef(prop); if (prevRef.current !== prop) { prevRef.current = prop; setState(initial); }` — this runs synchronously during render and avoids the effect-induced double render
121121
- Distinguish "derived state" from "form state initialization" — if local state is a copy of server data that the user then edits independently (e.g., secrets form, settings form), syncing it via `useEffect` on query data change is the correct pattern; do not convert these to inline derivation, as the local state intentionally diverges from the source
122+
- `useEffect` cleanup closures must not rely on hook-scoped utilities that close over the current entity ID (e.g., `updateMessageInCache` scoped to the rendered `chatId`) when the cleanup may serve sessions from a different entity — use refs or the underlying API directly (e.g., `queryClient.setQueryData` with `session.chatId`) to target the correct entity
122123

123124
### Component Variants
124125
- Create explicit variant components instead of one component with many boolean modes (e.g., `ThreadComposer`, `EditComposer` instead of `<Composer isThread isEditing />`)
@@ -145,6 +146,7 @@
145146
- Do not wrap trivial expressions in `useMemo` (e.g., `useMemo(() => x || [], [x])`) — use direct expressions (`x ?? []`)
146147
- Hoist regex patterns to module-level constants — never create RegExp inside loops or frequently-called functions
147148
- Prefer single-pass iteration (`.reduce()`) over chained `.filter().map()` in render paths
149+
- When reordering a function call to run earlier in a per-event hot path (e.g., stream envelope processing), gate the call with the cheapest possible condition check at the call site — avoid paying function-call overhead for the 99% of events that will just early-return
148150
- Keep `useEffect` for external system subscriptions and DOM side effects — keyboard shortcuts, resize observers, WebSocket lifecycle, scroll-into-view, and focus management require post-render timing and cleanup; do not convert these to ref-based render checks
149151

150152
### Async Patterns

frontend/src/hooks/useStreamCallbacks.ts

Lines changed: 95 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import type { StreamOptions } from '@/services/streamService';
2222
import { useChatSettingsStore } from '@/store/chatSettingsStore';
2323
import type { PaginatedMessages } from '@/types/api.types';
2424

25+
// Batching window for streaming content updates. Envelopes arrive at token-level
26+
// granularity (~10-50ms apart); flushing on every token would thrash React state
27+
// and the query cache. 130ms collects a visible chunk of text per paint cycle.
2528
const STREAM_FLUSH_INTERVAL_MS = 130;
2629

2730
function findMessageInCache(
@@ -59,46 +62,6 @@ function buildProjectionUpdate(
5962
});
6063
}
6164

62-
interface UseStreamCallbacksParams {
63-
messages: Message[];
64-
chatId: string | undefined;
65-
currentChat: Chat | undefined;
66-
queryClient: QueryClient;
67-
refetchFilesMetadata: () => Promise<unknown>;
68-
onContextUsageUpdate?: (data: ContextUsage, chatId?: string) => void;
69-
onPermissionRequest?: (request: PermissionRequest) => void;
70-
setMessages: Dispatch<SetStateAction<Message[]>>;
71-
setStreamState: Dispatch<SetStateAction<StreamState>>;
72-
setCurrentMessageId: Dispatch<SetStateAction<string | null>>;
73-
setError: Dispatch<SetStateAction<Error | null>>;
74-
pendingStopRef: React.MutableRefObject<Set<string>>;
75-
onPendingUserMessageIdChange?: (id: string | null) => void;
76-
}
77-
78-
interface UseStreamCallbacksResult {
79-
onEnvelope: (envelope: StreamEnvelope) => void;
80-
onComplete: (
81-
messageId?: string,
82-
streamId?: string,
83-
terminalKind?: 'complete' | 'cancelled',
84-
) => void;
85-
onError: (error: Error, messageId?: string, streamId?: string) => void;
86-
onQueueProcess: (data: QueueProcessingData) => void;
87-
startStream: (request: StreamOptions['request']) => Promise<string>;
88-
replayStream: (messageId: string, afterSeq?: number) => Promise<string>;
89-
stopStream: (messageId: string) => Promise<void>;
90-
updateMessageInCache: ReturnType<typeof useMessageCache>['updateMessageInCache'];
91-
addMessageToCache: ReturnType<typeof useMessageCache>['addMessageToCache'];
92-
removeMessagesFromCache: ReturnType<typeof useMessageCache>['removeMessagesFromCache'];
93-
setPendingUserMessageId: (id: string | null) => void;
94-
}
95-
96-
interface StreamSessionState {
97-
messageId: string;
98-
lastSeq: number;
99-
chatId: string;
100-
}
101-
10265
function envelopeToRenderEvent(envelope: StreamEnvelope): AssistantStreamEvent | null {
10366
const payload = envelope.payload as Record<string, unknown>;
10467

@@ -148,6 +111,46 @@ function envelopeToRenderEvent(envelope: StreamEnvelope): AssistantStreamEvent |
148111
}
149112
}
150113

114+
interface UseStreamCallbacksParams {
115+
messages: Message[];
116+
chatId: string | undefined;
117+
currentChat: Chat | undefined;
118+
queryClient: QueryClient;
119+
refetchFilesMetadata: () => Promise<unknown>;
120+
onContextUsageUpdate?: (data: ContextUsage, chatId?: string) => void;
121+
onPermissionRequest?: (request: PermissionRequest) => void;
122+
setMessages: Dispatch<SetStateAction<Message[]>>;
123+
setStreamState: Dispatch<SetStateAction<StreamState>>;
124+
setCurrentMessageId: Dispatch<SetStateAction<string | null>>;
125+
setError: Dispatch<SetStateAction<Error | null>>;
126+
pendingStopRef: React.MutableRefObject<Set<string>>;
127+
onPendingUserMessageIdChange?: (id: string | null) => void;
128+
}
129+
130+
interface UseStreamCallbacksResult {
131+
onEnvelope: (envelope: StreamEnvelope) => void;
132+
onComplete: (
133+
messageId?: string,
134+
streamId?: string,
135+
terminalKind?: 'complete' | 'cancelled',
136+
) => void;
137+
onError: (error: Error, messageId?: string, streamId?: string) => void;
138+
onQueueProcess: (data: QueueProcessingData) => void;
139+
startStream: (request: StreamOptions['request']) => Promise<string>;
140+
replayStream: (messageId: string, afterSeq?: number) => Promise<string>;
141+
stopStream: (messageId: string) => Promise<void>;
142+
updateMessageInCache: ReturnType<typeof useMessageCache>['updateMessageInCache'];
143+
addMessageToCache: ReturnType<typeof useMessageCache>['addMessageToCache'];
144+
removeMessagesFromCache: ReturnType<typeof useMessageCache>['removeMessagesFromCache'];
145+
setPendingUserMessageId: (id: string | null) => void;
146+
}
147+
148+
interface StreamSessionState {
149+
messageId: string;
150+
lastSeq: number;
151+
chatId: string;
152+
}
153+
151154
export function useStreamCallbacks({
152155
messages,
153156
chatId,
@@ -215,6 +218,10 @@ export function useStreamCallbacks({
215218
return undefined;
216219
}, []);
217220

221+
// Snapshot the accumulator's collected content into both React state (for the
222+
// visible chat) and the query cache (so navigating away and back shows progress).
223+
// Only updates React state when the session's chat is the one on screen — off-screen
224+
// streams still write to the cache so content isn't lost.
218225
const applyProjection = useCallback(
219226
(streamId: string, { writeToCache }: { writeToCache: boolean }) => {
220227
const accumulator = accumulatorsRef.current.get(streamId);
@@ -236,6 +243,8 @@ export function useStreamCallbacks({
236243
[setMessages, updateMessageInCache],
237244
);
238245

246+
// Coalescing timer: only one pending flush per stream. Multiple envelopes arriving
247+
// within the same 130ms window are batched into a single applyProjection call.
239248
const scheduleProjection = useCallback(
240249
(streamId: string) => {
241250
if (flushTimersRef.current.has(streamId)) {
@@ -252,6 +261,9 @@ export function useStreamCallbacks({
252261
[applyProjection],
253262
);
254263

264+
// Returns (or creates) the accumulator for a stream. On first call for a given
265+
// streamId, seeds the accumulator from the message's existing content so that
266+
// reconnections append to prior content rather than starting from blank.
255267
const ensureAccumulator = useCallback(
256268
(
257269
streamId: string,
@@ -270,6 +282,7 @@ export function useStreamCallbacks({
270282
return existing;
271283
}
272284

285+
// Seed from the on-screen message (fast path) or the query cache (off-screen chat).
273286
let seedEvents: AssistantStreamEvent[] = [];
274287
let seedText = '';
275288
const existingMessage =
@@ -308,7 +321,34 @@ export function useStreamCallbacks({
308321
timerIdsRef.current.forEach(clearTimeout);
309322
timerIdsRef.current = [];
310323

311-
flushTimers.forEach((timer) => clearTimeout(timer));
324+
// Flush any pending projections to the query cache before clearing,
325+
// so content already cursor-acked in chatStorage is not lost on unmount.
326+
// Write directly via queryClient using session.chatId — the hook-level
327+
// updateMessageInCache is scoped to the current chatId, which may differ
328+
// from the session's owning chat after cross-chat navigation.
329+
for (const [streamId, timer] of flushTimers.entries()) {
330+
clearTimeout(timer);
331+
const accumulator = accumulators.get(streamId);
332+
const session = streamSessions.get(streamId);
333+
if (accumulator && session) {
334+
const update = buildProjectionUpdate(streamId, accumulator, session);
335+
queryClient.setQueryData(
336+
queryKeys.messages(session.chatId),
337+
(oldData: { pages: PaginatedMessages[]; pageParams: unknown[] } | undefined) => {
338+
if (!oldData?.pages) return oldData;
339+
return {
340+
...oldData,
341+
pages: oldData.pages.map((page: PaginatedMessages) => ({
342+
...page,
343+
items: page.items.map((msg: Message) =>
344+
msg.id === session.messageId ? update(msg) : msg,
345+
),
346+
})),
347+
};
348+
},
349+
);
350+
}
351+
}
312352
flushTimers.clear();
313353

314354
accumulators.clear();
@@ -324,16 +364,23 @@ export function useStreamCallbacks({
324364
[onPendingUserMessageIdChange],
325365
);
326366

367+
// Central dispatch for every envelope arriving from the EventSource. Handles
368+
// side-effect-only events (permissions, system metadata, plan mode transitions)
369+
// inline, then delegates renderable content (text, thinking, tools) to the
370+
// accumulator → scheduleProjection pipeline for batched UI updates.
327371
const onEnvelope = useCallback(
328372
(envelope: StreamEnvelope) => {
329373
if (pendingStopRef.current.has(envelope.messageId)) {
330374
return;
331375
}
332376

377+
// First envelope for a message clears the optimistic "sending…" indicator.
333378
if (pendingUserMessageIdRef.current && chatId === chatIdRef.current) {
334379
setPendingUserMessageId(null);
335380
}
336381

382+
// Permission requests are dispatched directly to the modal system and
383+
// never accumulate into the message content — early return skips the pipeline.
337384
if (envelope.kind === 'permission_request' && onPermissionRequest) {
338385
const payload = envelope.payload as Record<string, unknown>;
339386
const request_id = typeof payload.request_id === 'string' ? payload.request_id : undefined;
@@ -497,6 +544,8 @@ export function useStreamCallbacks({
497544
timerIdsRef.current.forEach(clearTimeout);
498545
timerIdsRef.current = [];
499546

547+
// Delay usage refetch — the backend finalizes token counts asynchronously
548+
// after the stream ends, so an immediate query would return stale totals.
500549
timerIdsRef.current.push(
501550
setTimeout(() => {
502551
queryClient.invalidateQueries({ queryKey: [queryKeys.auth.usage] });
@@ -509,6 +558,8 @@ export function useStreamCallbacks({
509558
queryClient.invalidateQueries({ queryKey: [queryKeys.chats, 'infinite'] });
510559
queryClient.invalidateQueries({ queryKey: queryKeys.workspaces });
511560
}
561+
// Context usage aggregation runs as a background task after the stream
562+
// completes; 6s gives it time to finish before we refetch.
512563
timerIdsRef.current.push(
513564
setTimeout(() => {
514565
queryClient.invalidateQueries({ queryKey: queryKeys.contextUsage(chatId) });
@@ -648,6 +699,9 @@ export function useStreamCallbacks({
648699
],
649700
);
650701

702+
// Stash the latest callbacks in a ref so startStream/replayStream — which are
703+
// intentionally stable (empty deps) to avoid re-registering the EventSource —
704+
// always dispatch through the freshest closures.
651705
useEffect(() => {
652706
optionsRef.current = chatId
653707
? { chatId, onEnvelope, onComplete, onError, onQueueProcess }

frontend/src/services/streamService.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,18 @@ class StreamService {
170170
chatStorage.setEventId(chatId, String(seq));
171171
}
172172

173+
// Queue handoff events carry the old assistant message ID but must be processed
174+
// before the messageId filter — they update the stream's messageId for subsequent events.
175+
if (parsed.kind === 'queue_processing') {
176+
this.maybeHandleQueueEvent(parsed, chatId);
177+
}
178+
173179
const isForActiveMessage = parsed.messageId === currentStream.messageId;
174180
if (!isForActiveMessage) {
175181
return;
176182
}
177183

178184
currentStream.callbacks?.onEnvelope?.(parsed);
179-
this.maybeHandleQueueEvent(parsed, chatId);
180185

181186
if (parsed.kind === 'complete' || parsed.kind === 'cancelled') {
182187
const { callbacks } = currentStream;

0 commit comments

Comments
 (0)