Skip to content

Conversation

@DeagleGross
Copy link
Contributor

Adds the code to restore and save thread accross the agent invocation

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

@DeagleGross DeagleGross self-assigned this Nov 17, 2025
Copilot AI review requested due to automatic review settings November 17, 2025 20:55
Copilot finished reviewing on behalf of DeagleGross November 17, 2025 20:59
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 adds thread management support to the OpenAI Responses hosting layer, enabling conversation state to persist across agent invocations. The implementation mirrors the existing A2A hosting pattern by integrating AgentThreadStore to save and restore thread state based on conversation IDs.

  • Introduces IsNewConversation tracking in IdGenerator and AgentInvocationContext
  • Implements thread retrieval and persistence in HostedAgentResponseExecutor
  • Removes ConversationId from ChatOptions as thread management now handles conversation tracking

Reviewed Changes

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

File Description
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/HostedAgentResponseExecutor.cs Implements thread management by retrieving threads from AgentThreadStore for existing conversations and saving them after agent execution
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/AgentInvocationContext.cs Adds IsNewConversation property to expose new conversation detection from the underlying IdGenerator
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/IdGenerator.cs Adds IsNewConversation boolean property that tracks whether a conversation ID was provided or generated, enabling detection of new vs. existing conversations
Comments suppressed due to low confidence (1)

dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/HostedAgentResponseExecutor.cs:92

  • The ConversationId property has been removed from ChatOptions, but it's still being set in AIAgentResponseExecutor.cs (line 39). This creates an inconsistency between the two executor implementations.

If the removal is intentional because thread management now handles conversation tracking via the thread parameter, consider:

  1. Documenting why it was removed in a comment
  2. Ensuring AIAgentResponseExecutor follows the same pattern if applicable
  3. Verifying that downstream code doesn't rely on ChatOptions.ConversationId

If the removal is not intentional, it should be restored:

var chatOptions = new ChatOptions
{
    ConversationId = conversationId,
    Temperature = (float?)request.Temperature,
    // ...
};
        var chatOptions = new ChatOptions
        {
            Temperature = (float?)request.Temperature,
            TopP = (float?)request.TopP,
            MaxOutputTokens = request.MaxOutputTokens,
            Instructions = request.Instructions,
            ModelId = request.Model,
        };

Comment on lines +82 to +83
var agent = this._serviceProvider.GetRequiredKeyedService<AIAgent>(agentName);
var threadStore = this._serviceProvider.GetKeyedService<AgentThreadStore>(agent.Name);
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

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

The variable declaration uses var instead of an explicit type. According to the C# Sample Code Guidelines (CodingGuidelineID: 1000000), you should "Prefer defining variables using types rather than var, to help users understand the types involved."

Consider changing to:

AIAgent agent = this._serviceProvider.GetRequiredKeyedService<AIAgent>(agentName);
AgentThreadStore? threadStore = this._serviceProvider.GetKeyedService<AgentThreadStore>(agent.Name);

Copilot generated this review using guidance from repository custom instructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants