- 
                Notifications
    You must be signed in to change notification settings 
- Fork 841
Allow ChatOptions.ConversationId to be an OpenAI conversation ID with Responses #6960
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
Conversation
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.
Pull Request Overview
This PR allows ChatOptions.ConversationId to accept OpenAI conversation IDs (prefixed with "conv_") in addition to response IDs (prefixed with "resp_") when using the Responses API. The implementation differentiates between conversation IDs and response IDs based on the "conv_" prefix and routes them to the appropriate OpenAI API fields.
Key Changes:
- ChatOptions.ConversationIdcan now accept both "conv_" prefixed conversation IDs and "resp_" prefixed response IDs
- Conversation IDs are routed to the conversationfield in the OpenAI API request
- Response IDs are routed to the previous_response_idfield as before
- The ConversationIdis now tracked separately from the response ID to preserve the original value in responses
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description | 
|---|---|
| OpenAIResponsesChatClient.cs | Added logic to detect and handle conversation IDs vs response IDs, including JSON manipulation to set the conversation field when needed | 
| MicrosoftExtensionsAIResponsesExtensions.cs | Updated extension method signatures to pass null for conversationId parameter | 
| OpenAIResponseClientTests.cs | Added comprehensive test coverage for conversation ID scenarios in both streaming and non-streaming modes | 
        
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientTests.cs
          
            Show resolved
            Hide resolved
        
      | I understand that relying on the  What do you think of giving this new OpenAI  openAIResponseClient.AsIChatClient(useConversationIdAsPreviousResponse: false);Note An extra benefit of this approach is that all the internal  | 
| 
 It's possible we'll want to do something like that eventually, but it also complicates the model and puts this very front-and-center. I can also imagine other ways we'd want to expose this, ranging from an options bag that's passed to AsIChatClient to a delegate that maps from ChatOptions to the underlying raw options type (like RawRepresentationFactory but after configuration rather than before). RawRepresentationFactory is also a valid escape hatch already, as you can configure it however you like and this logic respects previously set values. 
 Not really. If it were just a flag check, the logic in this PR would be as simple. Most of the complication in this PR stems instead from wanting to respect anything set on ResponseCreationOptions in RawRepresentationFactory, combined with Conversation not being exposed yet on ResponseCreationOptions. | 
        
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientTests.cs
          
            Show resolved
            Hide resolved
        
      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.
@copilot address the unresolved feedback please.
        
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      |   What's "cross repository" about this PR? I was hoping it would work as it did here: modelcontextprotocol/csharp-sdk#892 (comment). | 
0d2a32c    to
    32875d8      
    Compare
  
    
Microsoft Reviewers: Open in CodeFlow