Skip to content

Conversation

@stephentoub
Copy link
Member

@stephentoub stephentoub commented Oct 24, 2025

Microsoft Reviewers: Open in CodeFlow

@stephentoub stephentoub requested a review from a team as a code owner October 24, 2025 19:42
@stephentoub stephentoub added the area-ai Microsoft.Extensions.AI libraries label Oct 24, 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

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.ConversationId can now accept both "conv_" prefixed conversation IDs and "resp_" prefixed response IDs
  • Conversation IDs are routed to the conversation field in the OpenAI API request
  • Response IDs are routed to the previous_response_id field as before
  • The ConversationId is 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

@rogerbarreto
Copy link
Contributor

rogerbarreto commented Oct 28, 2025

I understand that relying on the conv_ and resp_ naming is enough for OpenAI pattern but this may be fragile if used in custom scenarios.

What do you think of giving this new OpenAI Conversation behavior decision responsibility to the caller, defaulting the Old pattern to false. e.g:

openAIResponseClient.AsIChatClient(useConversationIdAsPreviousResponse: false);

Note

An extra benefit of this approach is that all the internal conversation implementation logic is isolated with a simple flag check compared to actual logic with extra complexity checking the same field for different behaviors.

@stephentoub
Copy link
Member Author

stephentoub commented Oct 28, 2025

What do you think of giving this new OpenAI Conversation behavior decision responsibility to the caller, defaulting the Old pattern to false.

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.

An extra benefit of this approach is that all the internal conversation implementation logic is isolated with a simple flag check compared to actual logic with extra complexity checking the same field for different behaviors.

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.

@stephentoub stephentoub requested a review from jozkee October 29, 2025 15:24
Copy link
Member

@jozkee jozkee left a 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.

@jozkee
Copy link
Member

jozkee commented Oct 30, 2025

image

What's "cross repository" about this PR? I was hoping it would work as it did here: modelcontextprotocol/csharp-sdk#892 (comment).

@stephentoub stephentoub merged commit 44f6484 into dotnet:main Oct 30, 2025
6 checks passed
@stephentoub stephentoub deleted the conversationid branch October 30, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-ai Microsoft.Extensions.AI libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants