From e118846aa5bd25fc57e11f2460935033e987a895 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 16 Oct 2025 14:24:56 -0600 Subject: [PATCH] fix: Address PR review feedback - fix stale resume IDs, update model description, remove duplicate test, revert gitignore --- .gitignore | 1 - packages/types/src/providers/openai.ts | 2 +- src/api/providers/openai-native.ts | 22 ++++++++---- .../utils/__tests__/backgroundStatus.test.ts | 35 ------------------- 4 files changed, 17 insertions(+), 43 deletions(-) delete mode 100644 webview-ui/src/utils/__tests__/backgroundStatus.test.ts diff --git a/.gitignore b/.gitignore index b7bfacd920..ed8e397899 100644 --- a/.gitignore +++ b/.gitignore @@ -49,7 +49,6 @@ logs # Qdrant qdrant_storage/ - # Architect plans plans/ diff --git a/packages/types/src/providers/openai.ts b/packages/types/src/providers/openai.ts index 4bfefa5eb1..b05185402d 100644 --- a/packages/types/src/providers/openai.ts +++ b/packages/types/src/providers/openai.ts @@ -57,7 +57,7 @@ export const openAiNativeModels = { inputPrice: 15.0, outputPrice: 120.0, description: - "GPT-5 Pro: a slow, reasoning-focused model built to tackle tough problems. Requests can take several minutes to finish. Responses API only; no streaming, so it may appear stuck until the reply is ready.", + "GPT-5 Pro: A slow, reasoning-focused model for complex problems. Uses background mode with resilient streaming - requests may take several minutes with automatic recovery if connection drops.", supportsVerbosity: true, supportsTemperature: false, backgroundMode: true, diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index bd0be481c5..15e6255c18 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -58,6 +58,9 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio private currentRequestIsBackground?: boolean // Cutoff sequence for filtering stale events during resume private resumeCutoffSequence?: number + // Per-request tracking to prevent stale resume attempts + private currentRequestResponseId?: string + private currentRequestSequenceNumber?: number // Event types handled by the shared event processor to avoid duplication private readonly coreHandledEventTypes = new Set([ @@ -357,12 +360,15 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio // Annotate if this request uses background mode (used for status chunks) this.currentRequestIsBackground = !!requestBody?.background + // Reset per-request tracking to prevent stale values from previous requests + this.currentRequestResponseId = undefined + this.currentRequestSequenceNumber = undefined const canAttemptResume = () => this.currentRequestIsBackground && (this.options.openAiNativeBackgroundAutoResume ?? true) && - !!this.lastResponseId && - typeof this.lastSequenceNumber === "number" + !!this.currentRequestResponseId && + typeof this.currentRequestSequenceNumber === "number" try { // Use the official SDK @@ -395,8 +401,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio // Stream dropped mid-flight; attempt resume for background requests if (canAttemptResume()) { for await (const chunk of this.attemptResumeOrPoll( - this.lastResponseId!, - this.lastSequenceNumber!, + this.currentRequestResponseId!, + this.currentRequestSequenceNumber!, model, )) { yield chunk @@ -420,8 +426,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio } if (canAttemptResume()) { for await (const chunk of this.attemptResumeOrPoll( - this.lastResponseId!, - this.lastSequenceNumber!, + this.currentRequestResponseId!, + this.currentRequestSequenceNumber!, model, )) { yield chunk @@ -684,6 +690,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio // Record sequence number for cursor tracking if (typeof parsed?.sequence_number === "number") { this.lastSequenceNumber = parsed.sequence_number + // Also track for per-request resume capability + this.currentRequestSequenceNumber = parsed.sequence_number } // Capture resolved service tier if present @@ -1384,6 +1392,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio // Record sequence number for cursor tracking if (typeof event?.sequence_number === "number") { this.lastSequenceNumber = event.sequence_number + // Also track for per-request resume capability + this.currentRequestSequenceNumber = event.sequence_number } // Map lifecycle events to status chunks diff --git a/webview-ui/src/utils/__tests__/backgroundStatus.test.ts b/webview-ui/src/utils/__tests__/backgroundStatus.test.ts deleted file mode 100644 index aac4c73b3e..0000000000 --- a/webview-ui/src/utils/__tests__/backgroundStatus.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { labelForBackgroundStatus } from "@src/utils/backgroundStatus" - -describe("labelForBackgroundStatus()", () => { - it("maps queued", () => { - expect(labelForBackgroundStatus("queued")).toBe("API Request: background mode (queued)…") - }) - - it("maps in_progress", () => { - expect(labelForBackgroundStatus("in_progress")).toBe("API Request: background mode (in progress)…") - }) - - it("maps reconnecting", () => { - expect(labelForBackgroundStatus("reconnecting")).toBe("API Request: background mode (reconnecting…)") - }) - - it("maps polling", () => { - expect(labelForBackgroundStatus("polling")).toBe("API Request: background mode (polling…)") - }) - - it("maps completed", () => { - expect(labelForBackgroundStatus("completed")).toBe("API Request: background mode (completed)") - }) - - it("maps failed", () => { - expect(labelForBackgroundStatus("failed")).toBe("API Request: background mode (failed)") - }) - - it("maps canceled", () => { - expect(labelForBackgroundStatus("canceled")).toBe("API Request: background mode (canceled)") - }) - - it("maps undefined to generic label", () => { - expect(labelForBackgroundStatus(undefined)).toBe("API Request: background mode") - }) -})