Skip to content

Commit 3fcf8c0

Browse files
committed
fix: prevent retry barrier flashing
1 parent cb9bdaf commit 3fcf8c0

File tree

6 files changed

+53
-13
lines changed

6 files changed

+53
-13
lines changed

docs/AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,5 @@ gh pr view <number> --json mergeable,mergeStateStatus | jq '.'
152152

153153
## Mode: Plan
154154

155+
- When Plan Mode is requested, assume the user wants the actual completed plan; do not merely describe how you would devise one.
155156
- Attach a net LoC estimate (product code only) to each recommended approach.

src/browser/components/ChatInput/index.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import { setTelemetryEnabled } from "@/common/telemetry";
6262
import { getTokenCountPromise } from "@/browser/utils/tokenizer/rendererClient";
6363
import { CreationCenterContent } from "./CreationCenterContent";
6464
import { CreationControls } from "./CreationControls";
65+
import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore";
6566
import { useCreationWorkspace } from "./useCreationWorkspace";
6667

6768
type TokenCountReader = () => number;
@@ -136,6 +137,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
136137
const [mode, setMode] = useMode();
137138
const { recentModels, addModel, evictModel } = useModelLRU();
138139
const commandListId = useId();
140+
const workspaceStore = useWorkspaceStoreRaw();
139141
const telemetry = useTelemetry();
140142
const [vimEnabled, setVimEnabled] = usePersistedState<boolean>(VIM_ENABLED_KEY, false, {
141143
listener: true,
@@ -443,6 +445,12 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
443445
// Workspace variant: full command handling + message send
444446
if (variant !== "workspace") return; // Type guard
445447

448+
const notifyPendingSendFailed = () => {
449+
setTimeout(() => {
450+
workspaceStore.markPendingStreamFailed(props.workspaceId);
451+
}, 0);
452+
};
453+
446454
try {
447455
// Parse command
448456
const parsed = parseCommand(messageText);
@@ -716,6 +724,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
716724
setToast(createErrorToast(result.error));
717725
// Restore input and images on error so user can try again
718726
setInput(messageText);
727+
notifyPendingSendFailed();
719728
setImageAttachments(previousImageAttachments);
720729
} else {
721730
// Track telemetry for successful message send
@@ -736,6 +745,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
736745
raw: error instanceof Error ? error.message : "Failed to send message",
737746
})
738747
);
748+
notifyPendingSendFailed();
739749
setInput(messageText);
740750
setImageAttachments(previousImageAttachments);
741751
} finally {

src/browser/stores/WorkspaceStore.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,20 @@ export class WorkspaceStore {
365365
return allStates;
366366
}
367367

368+
/**
369+
* Notify the aggregator that a pending stream never started (renderer send failure).
370+
*/
371+
markPendingStreamFailed(workspaceId: string): void {
372+
const aggregator = this.aggregators.get(workspaceId);
373+
if (!aggregator) {
374+
return;
375+
}
376+
377+
if (aggregator.markPendingStreamStartFailed()) {
378+
this.states.bump(workspaceId);
379+
}
380+
}
381+
368382
/**
369383
* Get recency timestamps for all workspaces (for sorting in command palette).
370384
* Derived on-demand from individual workspace states.

src/browser/utils/messages/StreamingMessageAggregator.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,18 @@ export class StreamingMessageAggregator {
267267
return this.pendingStreamStartTime;
268268
}
269269

270+
271+
/**
272+
* Clear pending stream-start tracking when send fails before we ever receive stream-start.
273+
* Returns true when the internal state changed so callers can trigger updates.
274+
*/
275+
markPendingStreamStartFailed(): boolean {
276+
if (this.pendingStreamStartTime === null) {
277+
return false;
278+
}
279+
this.pendingStreamStartTime = null;
280+
return true;
281+
}
270282
private setPendingStreamStartTime(time: number | null): void {
271283
this.pendingStreamStartTime = time;
272284
}

src/browser/utils/messages/retryEligibility.test.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
hasInterruptedStream,
44
isEligibleForAutoRetry,
55
isNonRetryableSendError,
6+
PENDING_STREAM_START_GRACE_PERIOD_MS,
67
} from "./retryEligibility";
78
import type { DisplayedMessage } from "@/common/types/message";
89
import type { SendMessageError } from "@/common/types/errors";
@@ -165,7 +166,7 @@ describe("hasInterruptedStream", () => {
165166
expect(hasInterruptedStream(messages, null)).toBe(true);
166167
});
167168

168-
it("returns false when message was sent very recently (< 3s)", () => {
169+
it("returns false when message was sent very recently (within grace period)", () => {
169170
const messages: DisplayedMessage[] = [
170171
{
171172
type: "user",
@@ -194,8 +195,8 @@ describe("hasInterruptedStream", () => {
194195
historySequence: 3,
195196
},
196197
];
197-
// Message sent 1 second ago - still within 3s window
198-
const recentTimestamp = Date.now() - 1000;
198+
// Message sent 1 second ago - still within grace window
199+
const recentTimestamp = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS - 1000);
199200
expect(hasInterruptedStream(messages, recentTimestamp)).toBe(false);
200201
});
201202

@@ -212,7 +213,7 @@ describe("hasInterruptedStream", () => {
212213
expect(hasInterruptedStream(messages, null)).toBe(true);
213214
});
214215

215-
it("returns false when user message just sent (< 3s ago)", () => {
216+
it("returns false when user message just sent (within grace period)", () => {
216217
const messages: DisplayedMessage[] = [
217218
{
218219
type: "user",
@@ -222,11 +223,11 @@ describe("hasInterruptedStream", () => {
222223
historySequence: 1,
223224
},
224225
];
225-
const justSent = Date.now() - 500; // 0.5s ago
226+
const justSent = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS - 500);
226227
expect(hasInterruptedStream(messages, justSent)).toBe(false);
227228
});
228229

229-
it("returns true when message sent over 3s ago (stream likely hung)", () => {
230+
it("returns true when message sent beyond grace period (stream likely hung)", () => {
230231
const messages: DisplayedMessage[] = [
231232
{
232233
type: "user",
@@ -236,7 +237,7 @@ describe("hasInterruptedStream", () => {
236237
historySequence: 1,
237238
},
238239
];
239-
const longAgo = Date.now() - 4000; // 4s ago - past 3s threshold
240+
const longAgo = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS + 1000);
240241
expect(hasInterruptedStream(messages, longAgo)).toBe(true);
241242
});
242243

@@ -545,7 +546,7 @@ describe("isEligibleForAutoRetry", () => {
545546
expect(isEligibleForAutoRetry(messages, null)).toBe(true);
546547
});
547548

548-
it("returns false when user message sent very recently (< 3s)", () => {
549+
it("returns false when user message sent very recently (within grace period)", () => {
549550
const messages: DisplayedMessage[] = [
550551
{
551552
type: "user",
@@ -555,7 +556,7 @@ describe("isEligibleForAutoRetry", () => {
555556
historySequence: 1,
556557
},
557558
];
558-
const justSent = Date.now() - 500; // 0.5s ago
559+
const justSent = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS - 500);
559560
expect(isEligibleForAutoRetry(messages, justSent)).toBe(false);
560561
});
561562
});

src/browser/utils/messages/retryEligibility.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ declare global {
1818
}
1919
}
2020

21+
export const PENDING_STREAM_START_GRACE_PERIOD_MS = 15000; // 15 seconds
22+
2123
/**
2224
* Check if the debug flag to force all errors to be retryable is enabled
2325
*/
@@ -69,20 +71,20 @@ export function isNonRetryableSendError(error: SendMessageError): boolean {
6971
* 3. Last message is a user message (indicating we sent it but never got a response)
7072
* - This handles app restarts during slow model responses (models can take 30-60s to first token)
7173
* - User messages are only at the end when response hasn't started/completed
72-
* - EXCEPT: Not if recently sent (<3s ago) - prevents flash during normal send flow
74+
* - EXCEPT: Not if recently sent (within PENDING_STREAM_START_GRACE_PERIOD_MS) - prevents flash during normal send flow
7375
*/
7476
export function hasInterruptedStream(
7577
messages: DisplayedMessage[],
7678
pendingStreamStartTime: number | null = null
7779
): boolean {
7880
if (messages.length === 0) return false;
7981

80-
// Don't show retry barrier if user message was sent very recently (< 3s)
82+
// Don't show retry barrier if user message was sent very recently (within the grace period)
8183
// This prevents flash during normal send flow while stream-start event arrives
82-
// After 3s, we assume something is wrong and show the barrier
84+
// After the grace period, assume something is wrong and show the barrier
8385
if (pendingStreamStartTime !== null) {
8486
const elapsed = Date.now() - pendingStreamStartTime;
85-
if (elapsed < 3000) return false;
87+
if (elapsed < PENDING_STREAM_START_GRACE_PERIOD_MS) return false;
8688
}
8789

8890
const lastMessage = messages[messages.length - 1];

0 commit comments

Comments
 (0)