Skip to content

Normalize legacy function_call finish reason on the Chat path - #1103

Open
PratikDhanave (PratikDhanave) wants to merge 1 commit into
microsoft:mainfrom
PratikDhanaveFork:fix/openai-chat-function-call-finish-reason
Open

PratikDhanave (PratikDhanave) wants to merge 1 commit into
microsoft:mainfrom
PratikDhanaveFork:fix/openai-chat-function-call-finish-reason

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

Some OpenAI-compatible endpoints still emit the deprecated "function_call" finish reason. The Chat path assigned choice.FinishReason / chunk.Choices[0].FinishReason verbatim, so a caller hitting such an endpoint saw FinishReason == "function_call" where the Python and .NET clients report "tool_calls".

The Python client normalizes this on both parse paths: finish_reason = "tool_calls" if choice.finish_reason == "function_call" else choice.finish_reason.

Change

  • Add normalizeChatFinishReason and apply it at both the non-streaming and streaming finish-reason assignment sites. Only "function_call" is remapped; all other reasons pass through unchanged.

This is distinct from the already-implemented Responses-path tool_calls synthesis — this remaps a legacy finish-reason string on the Chat path.

Test

  • TestChatLegacyFunctionCallFinishReasonNormalized: a canned completion with "finish_reason":"function_call" now yields FinishReason == "tool_calls". Fails before the change ("function_call"), passes after.

Some OpenAI-compatible endpoints still emit the deprecated "function_call"
finish reason. The Chat path forwarded the raw string verbatim, so callers
saw "function_call" where the Python and .NET clients report "tool_calls".

Normalize "function_call" to "tool_calls" on both the streaming and
non-streaming paths, matching the Python client; other reasons pass through
unchanged.
Copilot AI lite review requested due to automatic review settings September 18, 2026 12:25
@github-actions github-actions Bot added area:provider Changes files in the provider area area:provider/openai Changes files in the provider / openai area size:medium At most 100 changed lines across at most 5 files labels Sep 18, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

Add regression coverage for the streaming Chat path.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Normalizes legacy Chat finish reasons from function_call to tool_calls for compatibility across response paths.

Changes:

  • Added shared finish-reason normalization.
  • Applied it to streaming and non-streaming Chat responses.
  • Added non-streaming regression coverage.
File summaries
File Summary
provider/openaiprovider/chat.go Normalizes legacy finish reasons on both Chat paths.
provider/openaiprovider/chat_test.go Tests non-streaming normalization; streaming coverage is still requested.

Review note (nit, 3 votes): Add streaming regression coverage for function_call normalization.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

var finishReason string
if len(chunk.Choices) > 0 {
finishReason = chunk.Choices[0].FinishReason
finishReason = normalizeChatFinishReason(chunk.Choices[0].FinishReason)
@github-actions github-actions Bot added kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure labels Sep 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Scope: user-visible behavior (internal implementation, no exported API change)
Changed Go contract: provider/openaiprovider/chat.go — unexported normalizeChatFinishReason helper now remaps the legacy "function_call" finish reason to "tool_calls" at both the non-streaming and streaming Chat-path assignment sites. No exported types, functions, or signatures changed.
Upstream evidence reviewed: python/packages/openai/agent_framework_openai/_chat_completion_client.py lines 873-877 and 908-920 — finish_reason = "tool_calls" if choice.finish_reason == "function_call" else choice.finish_reason, applied identically on both the non-streaming and streaming parse paths. No equivalent .NET code found in dotnet/src/ (the reference .NET OpenAI chat client relies on Microsoft.Extensions.AI, which is outside this upstream repo).
Result: aligned — the Go change is a faithful, one-to-one port of the Python normalization (same trigger value, same replacement value, same two call sites). No parity issues found.

Generated by Go API Consistency Review Agent · copilot · auto · 23.5 AIC · ⌖ 5.66 AIC · ⊞ 9.2K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:provider/openai Changes files in the provider / openai area area:provider Changes files in the provider area kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure size:medium At most 100 changed lines across at most 5 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants