From 2fe9827f26f53290e5f17d4195addf8a65ff6f17 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 13:12:04 -0500 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=A4=96=20Fix=20retry=20barrier=20flas?= =?UTF-8?q?h=20on=20message=20send?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When sending a message, the retry barrier would briefly appear before the stream-start event arrived. This happened because: 1. User message added to history immediately 2. hasInterruptedStream() returns true for trailing user messages 3. canInterrupt is still false (no active stream yet) 4. Retry barrier renders briefly until stream starts Fixed by ignoring very recent user messages (<2s) in hasInterruptedStream(). This prevents the flash during normal send flow while still catching truly interrupted streams after app restarts or slow model responses. Updated tests to verify the time-based behavior. --- src/utils/messages/retryEligibility.test.ts | 57 ++++++++++++++++++++- src/utils/messages/retryEligibility.ts | 10 +++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/utils/messages/retryEligibility.test.ts b/src/utils/messages/retryEligibility.test.ts index 3760cebf2..75fb425a2 100644 --- a/src/utils/messages/retryEligibility.test.ts +++ b/src/utils/messages/retryEligibility.test.ts @@ -128,7 +128,8 @@ describe("hasInterruptedStream", () => { expect(hasInterruptedStream(messages)).toBe(false); }); - it("returns true when last message is user message (app restarted during slow model)", () => { + it("returns true when last message is user message sent >2s ago (app restarted during slow model)", () => { + const oldTimestamp = Date.now() - 5000; // 5 seconds ago const messages: DisplayedMessage[] = [ { type: "user", @@ -155,12 +156,14 @@ describe("hasInterruptedStream", () => { historyId: "user-2", content: "Another question", historySequence: 3, + timestamp: oldTimestamp, }, ]; expect(hasInterruptedStream(messages)).toBe(true); }); - it("returns true when user message has no response (slow model scenario)", () => { + it("returns false when last message is very recent user message (<2s)", () => { + const recentTimestamp = Date.now() - 500; // 500ms ago const messages: DisplayedMessage[] = [ { type: "user", @@ -169,7 +172,57 @@ describe("hasInterruptedStream", () => { content: "Hello", historySequence: 1, }, + { + type: "assistant", + id: "assistant-1", + historyId: "assistant-1", + content: "Complete response", + historySequence: 2, + streamSequence: 0, + isStreaming: false, + isPartial: false, + isLastPartOfMessage: true, + isCompacted: false, + }, + { + type: "user", + id: "user-2", + historyId: "user-2", + content: "Another question", + historySequence: 3, + timestamp: recentTimestamp, + }, + ]; + expect(hasInterruptedStream(messages)).toBe(false); + }); + + it("returns true when user message has no response sent >2s ago (slow model scenario)", () => { + const oldTimestamp = Date.now() - 3000; // 3 seconds ago + const messages: DisplayedMessage[] = [ + { + type: "user", + id: "user-1", + historyId: "user-1", + content: "Hello", + historySequence: 1, + timestamp: oldTimestamp, + }, ]; expect(hasInterruptedStream(messages)).toBe(true); }); + + it("returns false when user message was just sent (<2s ago)", () => { + const recentTimestamp = Date.now() - 1000; // 1 second ago + const messages: DisplayedMessage[] = [ + { + type: "user", + id: "user-1", + historyId: "user-1", + content: "Hello", + historySequence: 1, + timestamp: recentTimestamp, + }, + ]; + expect(hasInterruptedStream(messages)).toBe(false); + }); }); diff --git a/src/utils/messages/retryEligibility.ts b/src/utils/messages/retryEligibility.ts index c7da3cfea..1db8fd688 100644 --- a/src/utils/messages/retryEligibility.ts +++ b/src/utils/messages/retryEligibility.ts @@ -15,15 +15,23 @@ import type { DisplayedMessage } from "@/types/message"; * 3. Last message is a user message (indicating we sent it but never got a response) * - This handles app restarts during slow model responses (models can take 30-60s to first token) * - User messages are only at the end when response hasn't started/completed + * - EXCEPT: Ignore very recent user messages (< 2s) to prevent flash during normal send flow */ export function hasInterruptedStream(messages: DisplayedMessage[]): boolean { if (messages.length === 0) return false; const lastMessage = messages[messages.length - 1]; + // For user messages, check if enough time has passed to consider it truly interrupted + // This prevents the retry barrier from flashing briefly when sending a message + if (lastMessage.type === "user") { + const messageAge = Date.now() - (lastMessage.timestamp ?? 0); + const MIN_AGE_FOR_INTERRUPT = 2000; // 2 seconds + return messageAge >= MIN_AGE_FOR_INTERRUPT; + } + return ( lastMessage.type === "stream-error" || // Stream errored out - lastMessage.type === "user" || // No response received yet (e.g., app restarted during slow model) (lastMessage.type === "assistant" && lastMessage.isPartial === true) || (lastMessage.type === "tool" && lastMessage.isPartial === true) || (lastMessage.type === "reasoning" && lastMessage.isPartial === true) From 234231dd76a1daeb531cebb6d950070bd9d95d13 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 13:41:02 -0500 Subject: [PATCH 2/6] Add timer to force re-evaluation of retry barrier The initial fix prevented the flash but introduced a regression: if a send truly hangs (no stream-start event arrives), the retry barrier would never appear because React doesn't re-render just because time passes. Fixed by adding a timer that forces a re-render 2.1s after a user message is sent. This ensures the retry barrier appears even if the stream never starts, while still preventing the flash during normal operation. --- src/components/AIView.tsx | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/components/AIView.tsx b/src/components/AIView.tsx index 6151eec5d..8f7d5cc7c 100644 --- a/src/components/AIView.tsx +++ b/src/components/AIView.tsx @@ -52,6 +52,9 @@ const AIViewInner: React.FC = ({ const [activeTab, setActiveTab] = useState("costs"); const isReviewTabActive = activeTab === "review"; + // Force re-check state for retry barrier (incremented to trigger re-evaluation) + const [, setForceRecheck] = useState(0); + // Resizable sidebar for Review tab only // Hook encapsulates all drag logic, persistence, and constraints // Returns width to apply to RightSidebar and startResize for handle's onMouseDown @@ -269,6 +272,26 @@ const AIViewInner: React.FC = ({ // Uses same logic as useResumeManager for DRY const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages); + // Force re-evaluation after 2s when last message is a recent user message + // This ensures retry barrier appears even if stream-start never arrives + useEffect(() => { + if (messages.length === 0) return; + const lastMessage = messages[messages.length - 1]; + + if (lastMessage.type === "user" && !canInterrupt) { + const messageAge = Date.now() - (lastMessage.timestamp ?? 0); + const timeUntilCheck = Math.max(0, 2100 - messageAge); // 2.1s to ensure we're past threshold + + if (timeUntilCheck > 0) { + const timer = setTimeout(() => { + // Force re-render to re-evaluate showRetryBarrier + setForceRecheck((prev) => prev + 1); + }, timeUntilCheck); + return () => clearTimeout(timer); + } + } + }, [messages, canInterrupt]); + // Note: We intentionally do NOT reset autoRetry when streams start. // If user pressed Ctrl+C, autoRetry stays false until they manually retry. // This makes state transitions explicit and predictable. From f2234a38dfef436c6a88290c69622cd8b6286088 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 13:43:16 -0500 Subject: [PATCH 3/6] Move useEffect before early return to satisfy hooks rules React hooks must be called in the same order on every render, so they cannot be placed after conditional returns. Moved the timer useEffect before the early return and adjusted it to safely check workspaceState. --- src/components/AIView.tsx | 44 +++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/components/AIView.tsx b/src/components/AIView.tsx index 8f7d5cc7c..e0be42958 100644 --- a/src/components/AIView.tsx +++ b/src/components/AIView.tsx @@ -245,6 +245,30 @@ const AIViewInner: React.FC = ({ } }, [workspaceState, editingMessage]); + // Force re-evaluation after 2s when last message is a recent user message + // This ensures retry barrier appears even if stream-start never arrives + // Must be before early return to satisfy React Hooks rules + useEffect(() => { + if (!workspaceState) return; + const { messages, canInterrupt } = workspaceState; + + if (messages.length === 0) return; + const lastMessage = messages[messages.length - 1]; + + if (lastMessage.type === "user" && !canInterrupt) { + const messageAge = Date.now() - (lastMessage.timestamp ?? 0); + const timeUntilCheck = Math.max(0, 2100 - messageAge); // 2.1s to ensure we're past threshold + + if (timeUntilCheck > 0) { + const timer = setTimeout(() => { + // Force re-render to re-evaluate showRetryBarrier + setForceRecheck((prev) => prev + 1); + }, timeUntilCheck); + return () => clearTimeout(timer); + } + } + }, [workspaceState]); + // Return early if workspace state not loaded yet if (!workspaceState) { return ( @@ -272,26 +296,6 @@ const AIViewInner: React.FC = ({ // Uses same logic as useResumeManager for DRY const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages); - // Force re-evaluation after 2s when last message is a recent user message - // This ensures retry barrier appears even if stream-start never arrives - useEffect(() => { - if (messages.length === 0) return; - const lastMessage = messages[messages.length - 1]; - - if (lastMessage.type === "user" && !canInterrupt) { - const messageAge = Date.now() - (lastMessage.timestamp ?? 0); - const timeUntilCheck = Math.max(0, 2100 - messageAge); // 2.1s to ensure we're past threshold - - if (timeUntilCheck > 0) { - const timer = setTimeout(() => { - // Force re-render to re-evaluate showRetryBarrier - setForceRecheck((prev) => prev + 1); - }, timeUntilCheck); - return () => clearTimeout(timer); - } - } - }, [messages, canInterrupt]); - // Note: We intentionally do NOT reset autoRetry when streams start. // If user pressed Ctrl+C, autoRetry stays false until they manually retry. // This makes state transitions explicit and predictable. From 3e9eb8405b962105bad2a36851afcced929bb90d Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 14:04:05 -0500 Subject: [PATCH 4/6] =?UTF-8?q?=F0=9F=A4=96=20Replace=20time-based=20hack?= =?UTF-8?q?=20with=20explicit=20pendingStreamStart=20tracking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original fix used time-based hacks (checking message age, forced re-renders with timers) to prevent retry barrier flash. This was janky and semantically incorrect. New approach: - Track pendingStreamStart flag in StreamingMessageAggregator - Set true when user message is received - Set false when stream-start arrives - 30s timeout if stream never starts (safety net) - hasInterruptedStream() checks this flag instead of message age Benefits: - Semantically correct: explicitly tracks "waiting for stream-start" - No frontend timer hacks - state updates drive UI naturally - Cleaner: removed ~24 LoC of hacky code, added ~60 LoC of proper state - The 30s timeout is meaningful (truly stuck) vs arbitrary ("hide for 2s") Changes: - StreamingMessageAggregator: +25 LoC (flag, timer, handlers) - WorkspaceStore: +3 LoC (expose via state) - retryEligibility: -14 LoC (removed time checks, added flag param) - AIView: -27 LoC (removed timer hack) - Tests: Updated to use flag instead of timestamps Net: +40 LoC but much cleaner architecture. --- src/components/AIView.tsx | 35 +++--------------- src/hooks/useResumeManager.ts | 2 +- src/stores/WorkspaceStore.ts | 2 ++ .../messages/StreamingMessageAggregator.ts | 36 +++++++++++++++++++ src/utils/messages/retryEligibility.test.ts | 24 +++++-------- src/utils/messages/retryEligibility.ts | 20 +++++------ 6 files changed, 62 insertions(+), 57 deletions(-) diff --git a/src/components/AIView.tsx b/src/components/AIView.tsx index e0be42958..781632d0b 100644 --- a/src/components/AIView.tsx +++ b/src/components/AIView.tsx @@ -52,9 +52,6 @@ const AIViewInner: React.FC = ({ const [activeTab, setActiveTab] = useState("costs"); const isReviewTabActive = activeTab === "review"; - // Force re-check state for retry barrier (incremented to trigger re-evaluation) - const [, setForceRecheck] = useState(0); - // Resizable sidebar for Review tab only // Hook encapsulates all drag logic, persistence, and constraints // Returns width to apply to RightSidebar and startResize for handle's onMouseDown @@ -214,7 +211,8 @@ const AIViewInner: React.FC = ({ currentModel: workspaceState?.currentModel ?? null, canInterrupt: workspaceState?.canInterrupt ?? false, showRetryBarrier: workspaceState - ? !workspaceState.canInterrupt && hasInterruptedStream(workspaceState.messages) + ? !workspaceState.canInterrupt && + hasInterruptedStream(workspaceState.messages, workspaceState.pendingStreamStart) : false, currentWorkspaceThinking, setThinkingLevel, @@ -245,30 +243,6 @@ const AIViewInner: React.FC = ({ } }, [workspaceState, editingMessage]); - // Force re-evaluation after 2s when last message is a recent user message - // This ensures retry barrier appears even if stream-start never arrives - // Must be before early return to satisfy React Hooks rules - useEffect(() => { - if (!workspaceState) return; - const { messages, canInterrupt } = workspaceState; - - if (messages.length === 0) return; - const lastMessage = messages[messages.length - 1]; - - if (lastMessage.type === "user" && !canInterrupt) { - const messageAge = Date.now() - (lastMessage.timestamp ?? 0); - const timeUntilCheck = Math.max(0, 2100 - messageAge); // 2.1s to ensure we're past threshold - - if (timeUntilCheck > 0) { - const timer = setTimeout(() => { - // Force re-render to re-evaluate showRetryBarrier - setForceRecheck((prev) => prev + 1); - }, timeUntilCheck); - return () => clearTimeout(timer); - } - } - }, [workspaceState]); - // Return early if workspace state not loaded yet if (!workspaceState) { return ( @@ -287,14 +261,15 @@ const AIViewInner: React.FC = ({ } // Extract state from workspace state - const { messages, canInterrupt, isCompacting, loading, currentModel } = workspaceState; + const { messages, canInterrupt, isCompacting, loading, currentModel, pendingStreamStart } = + workspaceState; // Get active stream message ID for token counting const activeStreamMessageId = aggregator.getActiveStreamMessageId(); // Track if last message was interrupted or errored (for RetryBarrier) // Uses same logic as useResumeManager for DRY - const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages); + const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages, pendingStreamStart); // Note: We intentionally do NOT reset autoRetry when streams start. // If user pressed Ctrl+C, autoRetry stays false until they manually retry. diff --git a/src/hooks/useResumeManager.ts b/src/hooks/useResumeManager.ts index dbf4f38d0..c4dd137b2 100644 --- a/src/hooks/useResumeManager.ts +++ b/src/hooks/useResumeManager.ts @@ -95,7 +95,7 @@ export function useResumeManager() { // 1. Must have interrupted stream (not currently streaming) if (state.canInterrupt) return false; // Currently streaming - if (!hasInterruptedStream(state.messages)) { + if (!hasInterruptedStream(state.messages, state.pendingStreamStart)) { return false; } diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index 692de2c24..be280f3c7 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -29,6 +29,7 @@ export interface WorkspaceState { currentModel: string | null; recencyTimestamp: number | null; todos: TodoItem[]; + pendingStreamStart: boolean; } /** @@ -362,6 +363,7 @@ export class WorkspaceStore { currentModel: aggregator.getCurrentModel() ?? null, recencyTimestamp: aggregator.getRecencyTimestamp(), todos: aggregator.getCurrentTodos(), + pendingStreamStart: aggregator.isPendingStreamStart(), }; }); } diff --git a/src/utils/messages/StreamingMessageAggregator.ts b/src/utils/messages/StreamingMessageAggregator.ts index b71c09fe1..107498068 100644 --- a/src/utils/messages/StreamingMessageAggregator.ts +++ b/src/utils/messages/StreamingMessageAggregator.ts @@ -60,6 +60,11 @@ export class StreamingMessageAggregator { timestamp: number; } | null = null; + // Track when we're waiting for stream-start after user message + // Prevents retry barrier flash during normal send flow + private pendingStreamStart = false; + private pendingStreamStartTimer: ReturnType | null = null; + // Workspace creation timestamp (used for recency calculation) // REQUIRED: Backend guarantees every workspace has createdAt via config.ts private readonly createdAt: string; @@ -181,6 +186,29 @@ export class StreamingMessageAggregator { return this.messages.size > 0; } + isPendingStreamStart(): boolean { + return this.pendingStreamStart; + } + + private setPendingStreamStart(pending: boolean): void { + // Clear existing timer if any + if (this.pendingStreamStartTimer) { + clearTimeout(this.pendingStreamStartTimer); + this.pendingStreamStartTimer = null; + } + + this.pendingStreamStart = pending; + + if (pending) { + // Set 30s timeout - if stream hasn't started by then, something is wrong + this.pendingStreamStartTimer = setTimeout(() => { + this.pendingStreamStart = false; + this.pendingStreamStartTimer = null; + this.invalidateCache(); // Trigger re-render to show retry barrier + }, 30000); + } + } + getActiveStreams(): StreamingContext[] { return Array.from(this.activeStreams.values()); } @@ -251,6 +279,9 @@ export class StreamingMessageAggregator { // Unified event handlers that encapsulate all complex logic handleStreamStart(data: StreamStartEvent): void { + // Clear pending stream start flag - stream has started + this.setPendingStreamStart(false); + // Detect if this stream is compacting by checking if last user message is a compaction-request const messages = this.getAllMessages(); const lastUserMsg = [...messages].reverse().find((m) => m.role === "user"); @@ -571,6 +602,11 @@ export class StreamingMessageAggregator { // Now add the new message this.addMessage(incomingMessage); + + // If this is a user message, set pendingStreamStart flag and start timeout + if (incomingMessage.role === "user") { + this.setPendingStreamStart(true); + } } } diff --git a/src/utils/messages/retryEligibility.test.ts b/src/utils/messages/retryEligibility.test.ts index 75fb425a2..97d40f8ce 100644 --- a/src/utils/messages/retryEligibility.test.ts +++ b/src/utils/messages/retryEligibility.test.ts @@ -128,8 +128,7 @@ describe("hasInterruptedStream", () => { expect(hasInterruptedStream(messages)).toBe(false); }); - it("returns true when last message is user message sent >2s ago (app restarted during slow model)", () => { - const oldTimestamp = Date.now() - 5000; // 5 seconds ago + it("returns true when last message is user message (app restarted during slow model)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -156,14 +155,12 @@ describe("hasInterruptedStream", () => { historyId: "user-2", content: "Another question", historySequence: 3, - timestamp: oldTimestamp, }, ]; - expect(hasInterruptedStream(messages)).toBe(true); + expect(hasInterruptedStream(messages, false)).toBe(true); }); - it("returns false when last message is very recent user message (<2s)", () => { - const recentTimestamp = Date.now() - 500; // 500ms ago + it("returns false when pendingStreamStart is true", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -190,14 +187,12 @@ describe("hasInterruptedStream", () => { historyId: "user-2", content: "Another question", historySequence: 3, - timestamp: recentTimestamp, }, ]; - expect(hasInterruptedStream(messages)).toBe(false); + expect(hasInterruptedStream(messages, true)).toBe(false); }); - it("returns true when user message has no response sent >2s ago (slow model scenario)", () => { - const oldTimestamp = Date.now() - 3000; // 3 seconds ago + it("returns true when user message has no response (slow model scenario)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -205,14 +200,12 @@ describe("hasInterruptedStream", () => { historyId: "user-1", content: "Hello", historySequence: 1, - timestamp: oldTimestamp, }, ]; - expect(hasInterruptedStream(messages)).toBe(true); + expect(hasInterruptedStream(messages, false)).toBe(true); }); - it("returns false when user message was just sent (<2s ago)", () => { - const recentTimestamp = Date.now() - 1000; // 1 second ago + it("returns false when user message just sent and pendingStreamStart is true", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -220,9 +213,8 @@ describe("hasInterruptedStream", () => { historyId: "user-1", content: "Hello", historySequence: 1, - timestamp: recentTimestamp, }, ]; - expect(hasInterruptedStream(messages)).toBe(false); + expect(hasInterruptedStream(messages, true)).toBe(false); }); }); diff --git a/src/utils/messages/retryEligibility.ts b/src/utils/messages/retryEligibility.ts index 1db8fd688..f24d744db 100644 --- a/src/utils/messages/retryEligibility.ts +++ b/src/utils/messages/retryEligibility.ts @@ -15,23 +15,23 @@ import type { DisplayedMessage } from "@/types/message"; * 3. Last message is a user message (indicating we sent it but never got a response) * - This handles app restarts during slow model responses (models can take 30-60s to first token) * - User messages are only at the end when response hasn't started/completed - * - EXCEPT: Ignore very recent user messages (< 2s) to prevent flash during normal send flow + * - EXCEPT: Not if pendingStreamStart is true (waiting for stream-start event) */ -export function hasInterruptedStream(messages: DisplayedMessage[]): boolean { +export function hasInterruptedStream( + messages: DisplayedMessage[], + pendingStreamStart = false +): boolean { if (messages.length === 0) return false; - const lastMessage = messages[messages.length - 1]; + // Don't show retry barrier if we're waiting for stream-start + // This prevents flash during normal send flow + if (pendingStreamStart) return false; - // For user messages, check if enough time has passed to consider it truly interrupted - // This prevents the retry barrier from flashing briefly when sending a message - if (lastMessage.type === "user") { - const messageAge = Date.now() - (lastMessage.timestamp ?? 0); - const MIN_AGE_FOR_INTERRUPT = 2000; // 2 seconds - return messageAge >= MIN_AGE_FOR_INTERRUPT; - } + const lastMessage = messages[messages.length - 1]; return ( lastMessage.type === "stream-error" || // Stream errored out + lastMessage.type === "user" || // No response received yet (app restart during slow model) (lastMessage.type === "assistant" && lastMessage.isPartial === true) || (lastMessage.type === "tool" && lastMessage.isPartial === true) || (lastMessage.type === "reasoning" && lastMessage.isPartial === true) From 699a89311e4dd4cc1e4b5f4862d6a6b662bdf086 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 15:33:17 -0500 Subject: [PATCH 5/6] =?UTF-8?q?=F0=9F=A4=96=20Use=20timestamp=20instead=20?= =?UTF-8?q?of=20boolean=20for=20pendingStreamStart?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace boolean flag with timestamp to gracefully handle stale pending states. Instead of a 30s timeout with timer cleanup, check if timestamp is recent (<3s) when evaluating retry eligibility. Benefits: - No timer management/cleanup needed - UI naturally re-evaluates on re-render - More graceful - shows barrier after 3s if stream truly hung - Handles edge cases (app restart, etc.) better Changes: - StreamingMessageAggregator: Store timestamp instead of bool, remove timer - retryEligibility: Check if timestamp < 3s old to prevent flash - WorkspaceStore: Expose pendingStreamStartTime instead of bool - AIView/useResumeManager: Pass timestamp to hasInterruptedStream - Tests: Updated to verify timestamp-based logic (11 tests passing) --- src/components/AIView.tsx | 6 ++-- src/hooks/useResumeManager.ts | 2 +- src/stores/WorkspaceStore.ts | 4 +-- .../messages/StreamingMessageAggregator.ts | 35 ++++++------------- src/utils/messages/retryEligibility.test.ts | 29 +++++++++++---- src/utils/messages/retryEligibility.ts | 14 +++++--- 6 files changed, 48 insertions(+), 42 deletions(-) diff --git a/src/components/AIView.tsx b/src/components/AIView.tsx index 781632d0b..bfe343b3f 100644 --- a/src/components/AIView.tsx +++ b/src/components/AIView.tsx @@ -212,7 +212,7 @@ const AIViewInner: React.FC = ({ canInterrupt: workspaceState?.canInterrupt ?? false, showRetryBarrier: workspaceState ? !workspaceState.canInterrupt && - hasInterruptedStream(workspaceState.messages, workspaceState.pendingStreamStart) + hasInterruptedStream(workspaceState.messages, workspaceState.pendingStreamStartTime) : false, currentWorkspaceThinking, setThinkingLevel, @@ -261,7 +261,7 @@ const AIViewInner: React.FC = ({ } // Extract state from workspace state - const { messages, canInterrupt, isCompacting, loading, currentModel, pendingStreamStart } = + const { messages, canInterrupt, isCompacting, loading, currentModel, pendingStreamStartTime } = workspaceState; // Get active stream message ID for token counting @@ -269,7 +269,7 @@ const AIViewInner: React.FC = ({ // Track if last message was interrupted or errored (for RetryBarrier) // Uses same logic as useResumeManager for DRY - const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages, pendingStreamStart); + const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages, pendingStreamStartTime); // Note: We intentionally do NOT reset autoRetry when streams start. // If user pressed Ctrl+C, autoRetry stays false until they manually retry. diff --git a/src/hooks/useResumeManager.ts b/src/hooks/useResumeManager.ts index c4dd137b2..48f8cd1d3 100644 --- a/src/hooks/useResumeManager.ts +++ b/src/hooks/useResumeManager.ts @@ -95,7 +95,7 @@ export function useResumeManager() { // 1. Must have interrupted stream (not currently streaming) if (state.canInterrupt) return false; // Currently streaming - if (!hasInterruptedStream(state.messages, state.pendingStreamStart)) { + if (!hasInterruptedStream(state.messages, state.pendingStreamStartTime)) { return false; } diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index be280f3c7..609fbbe5f 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -29,7 +29,7 @@ export interface WorkspaceState { currentModel: string | null; recencyTimestamp: number | null; todos: TodoItem[]; - pendingStreamStart: boolean; + pendingStreamStartTime: number | null; } /** @@ -363,7 +363,7 @@ export class WorkspaceStore { currentModel: aggregator.getCurrentModel() ?? null, recencyTimestamp: aggregator.getRecencyTimestamp(), todos: aggregator.getCurrentTodos(), - pendingStreamStart: aggregator.isPendingStreamStart(), + pendingStreamStartTime: aggregator.getPendingStreamStartTime(), }; }); } diff --git a/src/utils/messages/StreamingMessageAggregator.ts b/src/utils/messages/StreamingMessageAggregator.ts index 107498068..5446b49e7 100644 --- a/src/utils/messages/StreamingMessageAggregator.ts +++ b/src/utils/messages/StreamingMessageAggregator.ts @@ -62,8 +62,8 @@ export class StreamingMessageAggregator { // Track when we're waiting for stream-start after user message // Prevents retry barrier flash during normal send flow - private pendingStreamStart = false; - private pendingStreamStartTimer: ReturnType | null = null; + // Stores timestamp of when user message was sent (null = no pending stream) + private pendingStreamStartTime: number | null = null; // Workspace creation timestamp (used for recency calculation) // REQUIRED: Backend guarantees every workspace has createdAt via config.ts @@ -186,27 +186,12 @@ export class StreamingMessageAggregator { return this.messages.size > 0; } - isPendingStreamStart(): boolean { - return this.pendingStreamStart; + getPendingStreamStartTime(): number | null { + return this.pendingStreamStartTime; } - private setPendingStreamStart(pending: boolean): void { - // Clear existing timer if any - if (this.pendingStreamStartTimer) { - clearTimeout(this.pendingStreamStartTimer); - this.pendingStreamStartTimer = null; - } - - this.pendingStreamStart = pending; - - if (pending) { - // Set 30s timeout - if stream hasn't started by then, something is wrong - this.pendingStreamStartTimer = setTimeout(() => { - this.pendingStreamStart = false; - this.pendingStreamStartTimer = null; - this.invalidateCache(); // Trigger re-render to show retry barrier - }, 30000); - } + private setPendingStreamStartTime(time: number | null): void { + this.pendingStreamStartTime = time; } getActiveStreams(): StreamingContext[] { @@ -279,8 +264,8 @@ export class StreamingMessageAggregator { // Unified event handlers that encapsulate all complex logic handleStreamStart(data: StreamStartEvent): void { - // Clear pending stream start flag - stream has started - this.setPendingStreamStart(false); + // Clear pending stream start timestamp - stream has started + this.setPendingStreamStartTime(null); // Detect if this stream is compacting by checking if last user message is a compaction-request const messages = this.getAllMessages(); @@ -603,9 +588,9 @@ export class StreamingMessageAggregator { // Now add the new message this.addMessage(incomingMessage); - // If this is a user message, set pendingStreamStart flag and start timeout + // If this is a user message, record timestamp for pending stream detection if (incomingMessage.role === "user") { - this.setPendingStreamStart(true); + this.setPendingStreamStartTime(Date.now()); } } } diff --git a/src/utils/messages/retryEligibility.test.ts b/src/utils/messages/retryEligibility.test.ts index 97d40f8ce..b716f6626 100644 --- a/src/utils/messages/retryEligibility.test.ts +++ b/src/utils/messages/retryEligibility.test.ts @@ -157,10 +157,10 @@ describe("hasInterruptedStream", () => { historySequence: 3, }, ]; - expect(hasInterruptedStream(messages, false)).toBe(true); + expect(hasInterruptedStream(messages, null)).toBe(true); }); - it("returns false when pendingStreamStart is true", () => { + it("returns false when message was sent very recently (< 3s)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -189,7 +189,9 @@ describe("hasInterruptedStream", () => { historySequence: 3, }, ]; - expect(hasInterruptedStream(messages, true)).toBe(false); + // Message sent 1 second ago - still within 3s window + const recentTimestamp = Date.now() - 1000; + expect(hasInterruptedStream(messages, recentTimestamp)).toBe(false); }); it("returns true when user message has no response (slow model scenario)", () => { @@ -202,10 +204,10 @@ describe("hasInterruptedStream", () => { historySequence: 1, }, ]; - expect(hasInterruptedStream(messages, false)).toBe(true); + expect(hasInterruptedStream(messages, null)).toBe(true); }); - it("returns false when user message just sent and pendingStreamStart is true", () => { + it("returns false when user message just sent (< 3s ago)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -215,6 +217,21 @@ describe("hasInterruptedStream", () => { historySequence: 1, }, ]; - expect(hasInterruptedStream(messages, true)).toBe(false); + const justSent = Date.now() - 500; // 0.5s ago + expect(hasInterruptedStream(messages, justSent)).toBe(false); + }); + + it("returns true when message sent over 3s ago (stream likely hung)", () => { + const messages: DisplayedMessage[] = [ + { + type: "user", + id: "user-1", + historyId: "user-1", + content: "Hello", + historySequence: 1, + }, + ]; + const longAgo = Date.now() - 4000; // 4s ago - past 3s threshold + expect(hasInterruptedStream(messages, longAgo)).toBe(true); }); }); diff --git a/src/utils/messages/retryEligibility.ts b/src/utils/messages/retryEligibility.ts index f24d744db..f222b52c1 100644 --- a/src/utils/messages/retryEligibility.ts +++ b/src/utils/messages/retryEligibility.ts @@ -15,17 +15,21 @@ import type { DisplayedMessage } from "@/types/message"; * 3. Last message is a user message (indicating we sent it but never got a response) * - This handles app restarts during slow model responses (models can take 30-60s to first token) * - User messages are only at the end when response hasn't started/completed - * - EXCEPT: Not if pendingStreamStart is true (waiting for stream-start event) + * - EXCEPT: Not if recently sent (<3s ago) - prevents flash during normal send flow */ export function hasInterruptedStream( messages: DisplayedMessage[], - pendingStreamStart = false + pendingStreamStartTime: number | null = null ): boolean { if (messages.length === 0) return false; - // Don't show retry barrier if we're waiting for stream-start - // This prevents flash during normal send flow - if (pendingStreamStart) return false; + // Don't show retry barrier if user message was sent very recently (< 3s) + // This prevents flash during normal send flow while stream-start event arrives + // After 3s, we assume something is wrong and show the barrier + if (pendingStreamStartTime !== null) { + const elapsed = Date.now() - pendingStreamStartTime; + if (elapsed < 3000) return false; + } const lastMessage = messages[messages.length - 1]; From bf93fa9e1ed22791af9a1182c128838cfe995fcf Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 15:39:06 -0500 Subject: [PATCH 6/6] Deduplicate showRetryBarrier computation in AIView Compute showRetryBarrier once before early return, use for both keybinds and UI rendering. Eliminates duplicate hasInterruptedStream call. --- src/components/AIView.tsx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/components/AIView.tsx b/src/components/AIView.tsx index bfe343b3f..a2a63e019 100644 --- a/src/components/AIView.tsx +++ b/src/components/AIView.tsx @@ -205,15 +205,20 @@ const AIViewInner: React.FC = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [workspaceId, workspaceState?.loading]); + // Compute showRetryBarrier once for both keybinds and UI + // Track if last message was interrupted or errored (for RetryBarrier) + // Uses same logic as useResumeManager for DRY + const showRetryBarrier = workspaceState + ? !workspaceState.canInterrupt && + hasInterruptedStream(workspaceState.messages, workspaceState.pendingStreamStartTime) + : false; + // Handle keyboard shortcuts (using optional refs that are safe even if not initialized) useAIViewKeybinds({ workspaceId, currentModel: workspaceState?.currentModel ?? null, canInterrupt: workspaceState?.canInterrupt ?? false, - showRetryBarrier: workspaceState - ? !workspaceState.canInterrupt && - hasInterruptedStream(workspaceState.messages, workspaceState.pendingStreamStartTime) - : false, + showRetryBarrier, currentWorkspaceThinking, setThinkingLevel, setAutoRetry, @@ -261,16 +266,11 @@ const AIViewInner: React.FC = ({ } // Extract state from workspace state - const { messages, canInterrupt, isCompacting, loading, currentModel, pendingStreamStartTime } = - workspaceState; + const { messages, canInterrupt, isCompacting, loading, currentModel } = workspaceState; // Get active stream message ID for token counting const activeStreamMessageId = aggregator.getActiveStreamMessageId(); - // Track if last message was interrupted or errored (for RetryBarrier) - // Uses same logic as useResumeManager for DRY - const showRetryBarrier = !canInterrupt && hasInterruptedStream(messages, pendingStreamStartTime); - // Note: We intentionally do NOT reset autoRetry when streams start. // If user pressed Ctrl+C, autoRetry stays false until they manually retry. // This makes state transitions explicit and predictable.