Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion agent/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ type Response struct {
// ID identifies this response.
ID string `json:",omitzero"`

// ModelID is the identifier of the model that produced this response, when
// the provider supplies it. It is empty otherwise.
ModelID string `json:",omitzero"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR adds ModelID to agent.Response/agent.ResponseUpdate (the Go equivalent of upstream AgentResponse/AgentResponseUpdate, not ChatResponse/ChatResponseUpdate). Upstream, ModelId/model exists only on the lower-level ChatResponse/ChatResponseUpdate types:

  • .NET: Microsoft.Agents.AI.Abstractions/AgentResponse.cs and AgentResponseUpdate.cs have no ModelId property (properties are AgentId, ResponseId, ContinuationToken, CreatedAt, FinishReason, Usage, RawRepresentation, AdditionalProperties). ModelId is folded only in AIAgentChatClient.CloneWithConversationId on ChatResponse (dotnet/src/Microsoft.Agents.AI/ChatClient/AIAgentChatClient.cs:368), and referenced in AgentResponseUpdateTests.ConstructorWithChatResponseUpdateRoundtrips (dotnet/tests/Microsoft.Agents.AI.Abstractions.UnitTests/AgentResponseUpdateTests.cs:42) as a ChatResponseUpdate field that is not asserted to roundtrip onto AgentResponseUpdate.
  • Python: AgentResponse.__init__/AgentResponseUpdate.__init__ (python/packages/core/agent_framework/_types.py:2848 and :3134) take no model/model_id parameter, while the sibling ChatResponse.__init__/ChatResponseUpdate.__init__ (same file, :2449 and :2735) do.

So both upstream implementations deliberately keep model identity at the chat-client layer and omit it from the agent-level response contract. Adding ModelID unconditionally to Go's agent.Response/ResponseUpdate (the AgentResponse analog) is a divergence from that design, not merely an idiomatic Go difference — it's a new agent-level field neither upstream implementation exposes.

Suggested resolution: either (a) confirm with maintainers this is an intentional Go-specific enhancement over the agent-level contract (and document that divergence), or (b) if a ModelID surface is wanted, scope it to a chat-level type analogous to ChatResponse/ChatResponseUpdate if/when Go introduces one, rather than agent.Response.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Parity concern (unresolved from prior review): ModelID added at the agent-response layer, not the chat-response layer

Upstream .NET and Python deliberately keep model identity on the lower-level chat-response contract and omit it from the agent-level response contract that Go's agent.Response/ResponseUpdate mirror:

  • .NET: Microsoft.Agents.AI.Abstractions/AgentResponse.cs and AgentResponseUpdate.cs have no ModelId property (members are AgentId, ResponseId, ContinuationToken, CreatedAt, FinishReason, Usage, RawRepresentation, AdditionalProperties). ModelId is folded only onto ChatResponse in AIAgentChatClient.CloneWithConversationId (dotnet/src/Microsoft.Agents.AI/ChatClient/AIAgentChatClient.cs:368). AgentResponseUpdateTests.ConstructorWithChatResponseUpdateRoundtrips (dotnet/tests/Microsoft.Agents.AI.Abstractions.UnitTests/AgentResponseUpdateTests.cs:42) exercises ModelId as a ChatResponseUpdate fixture field but never asserts it round-trips onto AgentResponseUpdate.
  • Python: AgentResponse.__init__/AgentResponseUpdate.__init__ (python/packages/core/agent_framework/_types.py:2848, :3134) take no model/model_id parameter, while sibling ChatResponse.__init__/ChatResponseUpdate.__init__ (same file, :2449, :2735) do.

Go has no chat-level response type distinct from agent.Response, so this PR's choice to add ModelID at the agent layer is a real API-shape divergence from both upstream implementations, not just an idiomatic naming difference. Suggest either explicitly confirming with maintainers that this is an intentional Go-specific widening of the agent-level contract (and documenting the divergence in the doc comment), or deferring the field to a future chat-level type if/when Go introduces one.


// ConversationID identifies conversation history retained after this response
// by the service or per-service-call history persistence. When nil, the response
// does not claim that its messages can be recovered from a retained conversation.
Expand Down Expand Up @@ -157,6 +161,7 @@ func (resp *Response) ToUpdates() []*ResponseUpdate {
AgentID: resp.AgentID,
MessageID: msg.ID,
ResponseID: resp.ID,
ModelID: resp.ModelID,
ConversationID: resp.ConversationID,
FinishReason: resp.FinishReason,
AuthorName: msg.AuthorName,
Expand All @@ -166,11 +171,12 @@ func (resp *Response) ToUpdates() []*ResponseUpdate {
})
}

if hasAdditionalProperties || resp.ContinuationToken != "" {
if hasAdditionalProperties || resp.ContinuationToken != "" || resp.ModelID != "" {
extra := &ResponseUpdate{
AdditionalProperties: resp.AdditionalProperties,
AgentID: resp.AgentID,
ResponseID: resp.ID,
ModelID: resp.ModelID,
ConversationID: resp.ConversationID,
ContinuationToken: resp.ContinuationToken,
CreatedAt: resp.CreatedAt,
Expand Down Expand Up @@ -213,6 +219,7 @@ func (resp *Response) Update(update *ResponseUpdate) {
// clear values already received for the response.
resp.AgentID = cmp.Or(update.AgentID, resp.AgentID)
resp.ID = cmp.Or(update.ResponseID, resp.ID)
resp.ModelID = cmp.Or(update.ModelID, resp.ModelID)
if update.ConversationID != nil {
resp.ConversationID = update.ConversationID
}
Expand Down Expand Up @@ -310,6 +317,11 @@ type ResponseUpdate struct {
// ResponseID identifies the response of which this update is a part.
ResponseID string

// ModelID is the identifier of the model that produced this update, when the
// provider supplies it. It is typically set on updates that carry provider
// response metadata.
ModelID string `json:",omitzero"`

// ConversationID identifies history retained by the service or per-service-call
// history persistence. Providers set it only when later requests can refer to that
// history instead of resending the messages. Nil means this update does not
Expand Down
20 changes: 20 additions & 0 deletions agent/response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,26 @@ func TestResponse_CreatedAt(t *testing.T) {
}
}

// ModelID folds onto the response from later updates and round-trips through
// ToUpdates, matching how ResponseID/FinishReason are handled.
func TestResponse_Update_ModelID(t *testing.T) {
resp := &agent.Response{}
resp.Update(&agent.ResponseUpdate{MessageID: "m1", Contents: message.Contents{&message.TextContent{Text: "hi"}}})
resp.Update(&agent.ResponseUpdate{MessageID: "m1", ModelID: "gpt-4o-mini-2024-07-18"})
if resp.ModelID != "gpt-4o-mini-2024-07-18" {
t.Fatalf("ModelID = %q, want gpt-4o-mini-2024-07-18", resp.ModelID)
}

// Round-trip: ToUpdates carries ModelID, and re-collecting preserves it.
var collected agent.Response
for _, u := range resp.ToUpdates() {
collected.Update(u)
}
if collected.ModelID != resp.ModelID {
t.Errorf("round-tripped ModelID = %q, want %q", collected.ModelID, resp.ModelID)
}
}

func TestResponse_Update_AdditionalProperties(t *testing.T) {
resp := &agent.Response{}

Expand Down
1 change: 1 addition & 0 deletions provider/anthropicprovider/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
Role: message.RoleAssistant,
MessageID: resp.ID,
ResponseID: resp.ID,
ModelID: string(resp.Model),
CreatedAt: time.Now(),
FinishReason: mapStopReason(resp.StopReason),
RawRepresentation: resp,
Expand Down
1 change: 1 addition & 0 deletions provider/geminiprovider/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
yield(&agent.ResponseUpdate{
Contents: responseContents,
Role: message.RoleAssistant,
ModelID: resp.ModelVersion,
FinishReason: finishReason,
CreatedAt: time.Now(),
RawRepresentation: resp,
Expand Down
2 changes: 2 additions & 0 deletions provider/openaiprovider/chat.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func (a *chatClient) run(ctx context.Context, messages []*message.Message, optio
Role: message.RoleAssistant,
ResponseID: resp.ID,
MessageID: resp.ID,
ModelID: resp.Model,
FinishReason: finishReason,
CreatedAt: time.Unix(resp.Created, 0),
RawRepresentation: resp,
Expand Down Expand Up @@ -216,6 +217,7 @@ func (a *chatClient) run(ctx context.Context, messages []*message.Message, optio
Role: role,
ResponseID: chunk.ID,
MessageID: chunk.ID,
ModelID: chunk.Model,
FinishReason: finishReason,
CreatedAt: time.Unix(chunk.Created, 0),
RawRepresentation: chunk,
Expand Down
17 changes: 17 additions & 0 deletions provider/openaiprovider/chat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,23 @@ func TestChatLegacyFunctionCallFinishReasonNormalized_Streaming(t *testing.T) {
}
}

// The model that produced the response must be surfaced on Response.ModelID.
func TestChatModelIDSurfaced(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, `{"id":"chatcmpl-m","object":"chat.completion","created":1727888631,"model":"gpt-4o-mini-2024-07-18","choices":[{"index":0,"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`)
}))
defer server.Close()

resp, err := newTestClient(server).RunText(t.Context(), "hi").Collect()
if err != nil {
t.Fatalf("error = %v", err)
}
if resp.ModelID != "gpt-4o-mini-2024-07-18" {
t.Errorf("ModelID = %q, want gpt-4o-mini-2024-07-18", resp.ModelID)
}
}

func TestChatURLCitationAnnotations_NonStreaming(t *testing.T) {
const input = `
{
Expand Down
2 changes: 2 additions & 0 deletions provider/openaiprovider/responses.go
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,7 @@ func responsesProcessResponse(resp *responses.Response, seqNum int64, yield func

currentUpdate := &agent.ResponseUpdate{
ResponseID: resp.ID,
ModelID: resp.Model,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ModelID is only propagated on non-streaming responses across three providers

This PR populates ResponseUpdate.ModelID from the final/non-streaming response object in responsesProcessResponse (here), provider/anthropicprovider/agent.go (non-streaming branch, resp.Model), and provider/geminiprovider/agent.go (non-streaming branch, resp.ModelVersion). None of the corresponding streaming paths set it:

  • responsesProcessStreamingUpdate (provider/openaiprovider/responses.go) builds every streaming agent.ResponseUpdate via createUpdate, which never sets ModelID, even though ResponseCreatedEvent/ResponseCompletedEvent carry event.Response.Model.
  • The Anthropic streaming loop (provider/anthropicprovider/agent.go, around the stream.Next() loop) never reads accumulated.Model or event.Message.Model into the yielded updates.
  • The Gemini streaming loop (provider/geminiprovider/agent.go, around the per-chunk yield) never reads resp.ModelVersion into the yielded updates.

Upstream M.E.AI sets ChatResponseUpdate.ModelId consistently for both streaming and non-streaming chat responses (the same ModelId property backs both ChatResponse and ChatResponseUpdate, and provider adapters populate it per-chunk). As written, collecting a streaming run via ResponseStream.Collect on any of these three providers silently produces an empty Response.ModelID, even though the PR's non-streaming tests pass. Suggest populating ModelID in each streaming update from the per-chunk/per-event model field, matching the non-streaming branches added in this PR.

FinishReason: finishReason,
CreatedAt: time.Unix(int64(resp.CreatedAt), 0),
Role: message.RoleAssistant,
Expand All @@ -1141,6 +1142,7 @@ func responsesProcessResponse(resp *responses.Response, seqNum int64, yield func
}
currentUpdate.MessageID = out.ID
currentUpdate.ResponseID = resp.ID
currentUpdate.ModelID = resp.Model
currentUpdate.FinishReason = finishReason
// Only set ContinuationToken if it's not empty
if contToken != "" {
Expand Down
Loading