-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: preserve context by retrying stateless on invalid previous_response_id (Responses API) #7709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -207,7 +207,7 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Make the request | ||||||||||||||||||||||||||||||||
| yield* this.executeRequest(requestBody, model, metadata) | ||||||||||||||||||||||||||||||||
| yield* this.executeRequest(requestBody, model, metadata, systemPrompt, messages) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private buildRequestBody( | ||||||||||||||||||||||||||||||||
|
|
@@ -276,6 +276,8 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||||||||
| requestBody: any, | ||||||||||||||||||||||||||||||||
| model: OpenAiNativeModel, | ||||||||||||||||||||||||||||||||
| metadata?: ApiHandlerCreateMessageMetadata, | ||||||||||||||||||||||||||||||||
| systemPrompt?: string, | ||||||||||||||||||||||||||||||||
| messages?: Anthropic.Messages.MessageParam[], | ||||||||||||||||||||||||||||||||
| ): ApiStream { | ||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||
| // Use the official SDK | ||||||||||||||||||||||||||||||||
|
|
@@ -297,17 +299,24 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio | |||||||||||||||||||||||||||||||
| const errorMessage = sdkErr?.message || sdkErr?.error?.message || "" | ||||||||||||||||||||||||||||||||
| const is400Error = sdkErr?.status === 400 || sdkErr?.response?.status === 400 | ||||||||||||||||||||||||||||||||
| const isPreviousResponseError = | ||||||||||||||||||||||||||||||||
| errorMessage.includes("Previous response") || errorMessage.includes("not found") | ||||||||||||||||||||||||||||||||
| errorMessage.includes("Previous response") || | ||||||||||||||||||||||||||||||||
| errorMessage.includes("not found") || | ||||||||||||||||||||||||||||||||
| errorMessage.includes("previous_response_id") | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if (is400Error && requestBody.previous_response_id && isPreviousResponseError) { | ||||||||||||||||||||||||||||||||
| // Log the error and retry without the previous_response_id | ||||||||||||||||||||||||||||||||
| // Clear the stored lastResponseId to prevent using it again | ||||||||||||||||||||||||||||||||
| this.lastResponseId = undefined | ||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding debug logging here when a retry occurs due to invalid previous_response_id. This would be helpful for monitoring and debugging in production environments. |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Remove the problematic previous_response_id and retry | ||||||||||||||||||||||||||||||||
| const retryRequestBody = { ...requestBody } | ||||||||||||||||||||||||||||||||
| // Re-prepare the request body with full conversation (no previous_response_id) | ||||||||||||||||||||||||||||||||
| let retryRequestBody = { ...requestBody } | ||||||||||||||||||||||||||||||||
| delete retryRequestBody.previous_response_id | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Clear the stored lastResponseId to prevent using it again | ||||||||||||||||||||||||||||||||
| this.lastResponseId = undefined | ||||||||||||||||||||||||||||||||
| // Re-prepare the input to send full conversation if we have the necessary data | ||||||||||||||||||||||||||||||||
| if (systemPrompt && messages) { | ||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notice there's duplicate retry logic here and in makeGpt5ResponsesAPIRequest() (lines 487-531). Could we extract this into a shared helper method to reduce duplication and ensure consistency? Something like: |
||||||||||||||||||||||||||||||||
| // Re-prepare input without previous_response_id (will send full conversation) | ||||||||||||||||||||||||||||||||
| const { formattedInput } = this.prepareStructuredInput(systemPrompt, messages, undefined) | ||||||||||||||||||||||||||||||||
| retryRequestBody.input = formattedInput | ||||||||||||||||||||||||||||||||
|
Comment on lines
+315
to
+318
|
||||||||||||||||||||||||||||||||
| if (systemPrompt && messages) { | |
| // Re-prepare input without previous_response_id (will send full conversation) | |
| const { formattedInput } = this.prepareStructuredInput(systemPrompt, messages, undefined) | |
| retryRequestBody.input = formattedInput | |
| retryRequestBody.input = formattedInput | |
| } else { | |
| // If we don't have the necessary data, fall back to SSE immediately | |
| yield* this.makeGpt5ResponsesAPIRequest( | |
| retryRequestBody, | |
| model, | |
| metadata, | |
| systemPrompt, | |
| messages, | |
| ) | |
| return |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is duplicated from lines 315-319. Consider extracting this retry preparation logic into a separate method to avoid code duplication and improve maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test coverage for the main scenario! Consider adding edge case tests for:
This would ensure the fix is robust in all scenarios.