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.

Comment on lines +48 to +50

// 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 @@ -160,6 +164,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 @@ -169,11 +174,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 @@ -216,6 +222,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 @@ -313,6 +320,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
5 changes: 5 additions & 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 All @@ -135,6 +136,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
defer func() { _ = stream.Close() }()

var messageID string
var modelID string
var finishReason string
var usage message.UsageDetails
var accumulated anthropic.Message
Expand All @@ -150,6 +152,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
switch event := event.AsAny().(type) {
case anthropic.MessageStartEvent:
messageID = cmp.Or(messageID, event.Message.ID)
modelID = cmp.Or(modelID, string(event.Message.Model))
usage.Add(toUsageDetails(event.Message.Usage))
case anthropic.MessageDeltaEvent:
// Anthropic reports the final cumulative output token count on
Expand Down Expand Up @@ -213,6 +216,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
Role: message.RoleAssistant,
ResponseID: messageID,
MessageID: messageID,
ModelID: modelID,
CreatedAt: time.Now(),
RawRepresentation: event,
}, nil) {
Expand All @@ -227,6 +231,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
CreatedAt: time.Now(),
Role: message.RoleAssistant,
MessageID: messageID,
ModelID: modelID,
FinishReason: finishReason,
Contents: []message.Content{
&message.UsageContent{
Expand Down
19 changes: 19 additions & 0 deletions provider/anthropicprovider/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,25 @@ func TestStreamingUsage_DoesNotDoubleCountOutputTokens(t *testing.T) {

// minimalStreamingResponse returns an SSE stream that delivers payload as a
// single text delta.
// A streamed run collected via ResponseStream.Collect must surface the model
// that produced the response on Response.ModelID, matching the non-streaming
// path. The model arrives on the message_start event.
func TestStreamingModelIDSurfaced(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "text/event-stream")
_, _ = io.WriteString(w, minimalStreamingResponse("hi"))
}))
defer server.Close()

resp, err := newTestClient(t, server).RunText(t.Context(), "hello", agent.Stream(true)).Collect()
if err != nil {
t.Fatalf("error = %v", err)
}
if resp.ModelID != "claude-3-5-sonnet-20241022" {
t.Errorf("ModelID = %q, want claude-3-5-sonnet-20241022", resp.ModelID)
}
}

func minimalStreamingResponse(payload string) string {
payloadJSON, _ := json.Marshal(payload)
return "" +
Expand Down
3 changes: 3 additions & 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 Expand Up @@ -187,6 +188,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
if !yield(&agent.ResponseUpdate{
Contents: streamContents,
Role: message.RoleAssistant,
ModelID: resp.ModelVersion,
FinishReason: finishReason,
CreatedAt: time.Now(),
RawRepresentation: resp,
Expand All @@ -202,6 +204,7 @@ func (a *client) run(ctx context.Context, messages []*message.Message, options .
yield(&agent.ResponseUpdate{
Contents: finalContents,
Role: message.RoleAssistant,
ModelID: latestMetadataResp.ModelVersion,
CreatedAt: time.Now(),
RawRepresentation: latestMetadataResp,
}, nil)
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 @@ -611,6 +611,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
20 changes: 20 additions & 0 deletions provider/openaiprovider/responses.go
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed — ModelID is now set on the streaming updates for all three providers: OpenAI Responses threads the model through responsesStreamState from the lifecycle events' Response.Model; Anthropic reads it from the message_start Message.Model; Gemini reads each chunk's ModelVersion (and the final usage update). Added TestStreamingModelIDSurfaced to lock in the Collect() round-trip.

FinishReason: finishReason,
CreatedAt: time.Unix(int64(resp.CreatedAt), 0),
Role: message.RoleAssistant,
Expand All @@ -1140,6 +1141,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 Expand Up @@ -1429,6 +1431,7 @@ func responsesErrorContent(msg string, code string, details string) *message.Err
type responsesStreamState struct {
mcpApprovalRequests map[string]*message.ToolApprovalRequestContent
messageID string
model string
role message.Role
anyFunctions bool
// imagePartialsSeen records image-generation item IDs that emitted a
Expand All @@ -1447,12 +1450,23 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
Role: role,
Contents: contents,
MessageID: state.messageID,
ModelID: state.model,
ResponseID: responseID,
RawRepresentation: update,
}
return u
}

// setModel records the response model from a lifecycle event so this and
// every subsequent streaming update carries it, matching the non-streaming
// path. The createUpdate above runs before this, so stamp u directly too.
setModel := func(u *agent.ResponseUpdate, model string) {
if model != "" {
state.model = model
u.ModelID = model
}
}

// Lifecycle metadata belongs to the response, not the active message.
// Omit its MessageID without resetting the state used by content updates.
var u *agent.ResponseUpdate
Expand All @@ -1462,6 +1476,7 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
u.MessageID = ""
u.CreatedAt = time.Unix(int64(event.Response.CreatedAt), 0)
u.ResponseID = event.Response.ID
setModel(u, event.Response.Model)
u.AdditionalProperties = responsesPopulateAdditionalProperties(&event.Response)
if contToken := createContinuationToken(event.Response.ID, event.SequenceNumber, event.Response.Status, isBackground); contToken != "" {
u.ContinuationToken = contToken
Expand All @@ -1472,6 +1487,7 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
u.MessageID = ""
u.CreatedAt = time.Unix(int64(event.Response.CreatedAt), 0)
u.ResponseID = event.Response.ID
setModel(u, event.Response.Model)
u.AdditionalProperties = responsesPopulateAdditionalProperties(&event.Response)
if contToken := createContinuationToken(event.Response.ID, event.SequenceNumber, event.Response.Status, isBackground); contToken != "" {
u.ContinuationToken = contToken
Expand All @@ -1482,6 +1498,7 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
u.MessageID = ""
u.CreatedAt = time.Unix(int64(event.Response.CreatedAt), 0)
u.ResponseID = event.Response.ID
setModel(u, event.Response.Model)
u.AdditionalProperties = responsesPopulateAdditionalProperties(&event.Response)
if contToken := createContinuationToken(event.Response.ID, event.SequenceNumber, event.Response.Status, isBackground); contToken != "" {
u.ContinuationToken = contToken
Expand Down Expand Up @@ -1509,6 +1526,7 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
u.MessageID = ""
u.CreatedAt = time.Unix(int64(event.Response.CreatedAt), 0)
u.ResponseID = event.Response.ID
setModel(u, event.Response.Model)
u.FinishReason = responsesFinishReason(&event.Response)
if state.anyFunctions && u.FinishReason == "stop" {
u.FinishReason = "tool_calls"
Expand All @@ -1524,6 +1542,7 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
u.MessageID = ""
u.CreatedAt = time.Unix(int64(event.Response.CreatedAt), 0)
u.ResponseID = event.Response.ID
setModel(u, event.Response.Model)
u.FinishReason = responsesFinishReason(&event.Response)
u.AdditionalProperties = responsesPopulateAdditionalProperties(&event.Response)
if contToken := createContinuationToken(event.Response.ID, event.SequenceNumber, event.Response.Status, isBackground); contToken != "" {
Expand All @@ -1535,6 +1554,7 @@ func responsesProcessStreamingUpdate(update responses.ResponseStreamEventUnion,
u.MessageID = ""
u.CreatedAt = time.Unix(int64(event.Response.CreatedAt), 0)
u.ResponseID = event.Response.ID
setModel(u, event.Response.Model)
u.AdditionalProperties = responsesPopulateAdditionalProperties(&event.Response)
if failure := responsesFailureContent(&event.Response); failure != nil {
u.Contents = []message.Content{failure}
Expand Down
Loading