Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Sep 5, 2025

Related GitHub Issue

No linked issue; internal bug fix. Please link the appropriate tracking issue.

Roo Code Task Context (Optional)

No Roo Code task context for this PR

Description

This PR fixes a regression in the OpenAI Responses API provider that caused conversation context to be lost after a continuity failure.

What failed:

  • When previous_response_id is present (stateful mode), only the latest user message is sent.
  • When no previous_response_id is present (stateless mode), the full conversation history is sent.
  • On a 400 error due to an invalid/expired previous_response_id (e.g., “Invalid previous_response_id: …”, “Previous response … not found”), our retry removed the ID but did not re-prepare the input, so the second attempt still sent only the latest message. This dropped prior context and degraded answer quality.

What this fixes:

Why it matters:

  • The correct recovery from an invalid/expired previous_response_id is to switch to stateless mode and include the complete conversation. Without this, the model answers using only the last turn, leading to context loss and incorrect responses.

Key changes:

  • src/api/providers/openai-native.ts
    • Robust detection of previous_response_id-related 400s (matches “Previous response”, “not found”, and “previous_response_id”)
    • Retry logic now re-prepares the full conversation on both SDK and SSE paths
  • src/api/providers/tests/openai-native.spec.ts
    • New regression test “should retry with full conversation when previous_response_id fails” validates the fix end-to-end

Test Procedure

Automated:

  • Run: cd src && npx vitest run api/providers/tests/openai-native.spec.ts
  • Expected: All tests pass (35/35)
  • New coverage explicitly verifies:
    • First attempt (stateful) sends only the latest message with previous_response_id
    • On 400 invalid previous_response_id, retry (stateless) sends full conversation and succeeds

Manual (optional):

  1. Start a conversation, capture a valid previous_response_id.
  2. Force a turn with an invalid/expired ID.
  3. Confirm the provider retries without the ID and includes full conversation, preserving response quality.

Pre-Submission Checklist

  • Issue Linked: No public issue reference included; please link the appropriate internal/external issue.
  • Scope: Changes limited to OpenAI Responses provider and tests.
  • Self-Review: Verified minimal, targeted changes; no unrelated refactors.
  • Testing: Added regression test; full suite passes locally.
  • Documentation Impact: None.
  • Contribution Guidelines: Followed.

Screenshots / Videos

No UI changes in this PR

Documentation Updates

  • No documentation updates are required.

Additional Notes

  • Backward compatible: Only affects retry behavior after specific 400 continuity errors.
  • Maintains existing verbosity/reasoning gating and cost tracking.

Get in Touch

@your-discord-handle


Important

Fixes context loss in OpenAI Responses API by retrying with full conversation on invalid previous_response_id.

  • Behavior:
    • Fixes context loss in OpenAiNativeHandler by retrying in stateless mode when previous_response_id is invalid.
    • Clears lastResponseId and re-prepares input with full conversation on 400 errors related to previous_response_id.
    • Implemented in executeRequest() and makeGpt5ResponsesAPIRequest() in openai-native.ts.
  • Tests:
    • Adds test "should retry with full conversation when previous_response_id fails" in openai-native.spec.ts to validate retry logic.
    • Ensures retry sends full conversation and succeeds after initial failure due to invalid previous_response_id.

This description was created by Ellipsis for 8093615. You can customize this summary. It will automatically update as commits are pushed.

Copilot AI review requested due to automatic review settings September 5, 2025 15:53
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Sep 5, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes a regression in the OpenAI Responses API where conversation context was lost when retrying after an invalid previous_response_id error. The bug occurred because retries removed the ID but didn't re-prepare the input to include the full conversation, sending only the latest message instead.

  • Enhanced error detection to catch all previous_response_id related 400 errors
  • Modified retry logic to re-prepare the full conversation when falling back from stateful to stateless mode
  • Added comprehensive test coverage to verify the fix works on both SDK and SSE code paths

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/api/providers/openai-native.ts Enhanced retry logic to preserve context by re-preparing full conversation on previous_response_id failures
src/api/providers/tests/openai-native.spec.ts Added regression test validating proper context preservation during retry scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

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
Copy link

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding an else clause to handle the case where systemPrompt or messages are undefined. Without proper fallback handling, the retry might still send incomplete data if these parameters are missing.

Suggested change
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 uses AI. Check for mistakes.
Comment on lines +500 to +507
if (systemPrompt && messages) {
// Re-prepare input without previous_response_id (will send full conversation)
// Note: We pass undefined metadata to prepareStructuredInput to ensure it doesn't use previousResponseId
const { formattedInput } = this.prepareStructuredInput(systemPrompt, messages, undefined)

// Update the input in the retry request body to include full conversation
retryRequestBody.input = formattedInput
}
Copy link

Copilot AI Sep 5, 2025

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! I've reviewed the changes and found that this fix effectively addresses the regression where conversation context was lost after continuity failures. The solution is well-implemented with good test coverage. I have some suggestions for improvement below.

// 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

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
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

role: "user",
content: [{ type: "input_text", text: "Latest message" }],
},
])
Copy link
Contributor

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:

  • What happens if the retry also fails with a different error?
  • Behavior when systemPrompt or messages are undefined during retry

This would ensure the fix is robust in all scenarios.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 5, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 5, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 5, 2025
@daniel-lxs
Copy link
Member

Closing this PR to implement a cleaner solution that sends the full conversation on retry when previous_response_id fails.

@daniel-lxs daniel-lxs closed this Sep 5, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Sep 5, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Needs Preliminary Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants