Python: fix Gemini finish reason fallback and usage-attach cascade - #7837
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the Gemini Python chat client where unmapped provider finish reasons were silently dropped to None. #7105 added a raw-string fallback to the other chat clients (ag, bedrock, claude, core, github_copilot, ollama, openai) but missed gemini. Because streamed-chunk usage is attached only when finish_reason is truthy, an unmapped reason (e.g. TOO_MANY_TOOL_CALLS) dropped both the finish reason and the turn's token/billing accounting. The fix mirrors the established ollama/openai fallback pattern, wrapping the raw reason as FinishReason(reason) so it type-checks under this package's strict Pyright config.
Changes:
_map_finish_reasonnow falls back toFinishReason(reason)for values absent from_FINISH_REASON_MAP, whileFINISH_REASON_UNSPECIFIEDand absent reasons still map toNone; return type widened toFinishReasonLiteral | FinishReason | None.- Added the
FinishReasonimport and updated the docstring to explain the fallback behavior. - Expanded finish-reason mapping tests and added a regression test confirming usage is still attached to the final streamed chunk under an unmapped reason.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
python/packages/gemini/agent_framework_gemini/_chat_client.py |
Adds raw-string fallback in _map_finish_reason, imports FinishReason, widens the return type, and updates the docstring. |
python/packages/gemini/tests/test_gemini_client.py |
Expands parametrized mapping cases (mapped, unmapped-passthrough, None/UNSPECIFIED) and adds a streamed-chunk usage-cascade regression test. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
microsoft#7105 added a fallback so unmapped provider finish reasons pass through as the raw string instead of being dropped to None. It covered ag, bedrock, claude, core, github_copilot, ollama, and openai, but not gemini. _FINISH_REASON_MAP covers 13 of the 18 members of google.genai.types.FinishReason in the package's pinned dependency range. OTHER, TOO_MANY_TOOL_CALLS, NO_IMAGE, and IMAGE_OTHER fell through to None. FINISH_REASON_UNSPECIFIED still correctly maps to None. _process_chunk only attaches usage to a streamed chunk when finish_reason is truthy, so an unmapped reason silently dropped both the finish reason and the whole turn's usage/token accounting. Mirrors the ollama/openai pattern of wrapping the raw fallback in FinishReason(...) rather than bedrock's plain str return, since bedrock's file carries a blanket type: ignore that gemini's does not. Fixes microsoft#7836
8a273b5 to
b0e2f6c
Compare
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
Updated [Microsoft.Agents.AI](https://github.com/microsoft/agent-framework) from 1.19.0 to 1.20.0. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Agents.AI's releases](https://github.com/microsoft/agent-framework/releases)._ ## 1.20.0 ## What's Changed * .NET: Bump AWSSDK.Extensions.Bedrock.MEAI from 4.0.6.10 to 4.0.101.8 by @dependabot[bot] in microsoft/agent-framework#7829 * .NET: Stabilize Foundry recovery tests by @rogerbarreto in microsoft/agent-framework#7817 * .NET: fix: preserve Responses logprobs field by @he-yufeng in microsoft/agent-framework#5860 * .NET: Honor cancellation for Foundry-hosted workflow responses by @rogerbarreto with @Copilot in microsoft/agent-framework#7842 * .NET: Use Responses API for hosted web search in AG-UI by @rogerbarreto with @Copilot in microsoft/agent-framework#7843 * .NET: Bump Aspire.Hosting from 13.1.0 to 13.5.2 by @dependabot[bot] in microsoft/agent-framework#7826 * .NET: Suppress false positive Zip Slip alert by @SergeyMenshykh in microsoft/agent-framework#7858 * .NET: added Mem0Sharp integration for in-memory storage in agent samples. by @jihadkhawaja in microsoft/agent-framework#7792 * .NET: Annotate DevUI aggregator static-analysis false positives by @SergeyMenshykh in microsoft/agent-framework#7864 * .NET: Rename CommunityToolkit.VectorData.CosmosNoSql to AzureCosmosDB by @adamsitnik in microsoft/agent-framework#7878 * .NET: chore: upgrades aspnet openapi dependency by @baywet in microsoft/agent-framework#7870 * .NET: Simplify A2A function tool samples by @SergeyMenshykh in microsoft/agent-framework#7861 * .NET: Bump Azure.AI.AgentServer.Invocations from 1.0.0-beta.5 to 1.0.0-beta.6 by @dependabot[bot] in microsoft/agent-framework#7886 * .NET: docs: updates the contributing information for CFS users by @baywet in microsoft/agent-framework#7869 * Bump CommunityToolkit.VectorData.InMemory from 1.0.0 to 1.0.1 by @dependabot[bot] in microsoft/agent-framework#7888 * .NET: Remove retired OpenAI Assistants integration tests by @rogerbarreto in microsoft/agent-framework#7896 * .NET: Simplify A2A client-server sample by @SergeyMenshykh in microsoft/agent-framework#7891 * Bump Dapr.AI.Microsoft.Extensions from 1.18.4 to 1.18.5 by @dependabot[bot] in microsoft/agent-framework#7889 * .NET: docs/workflow fileinput sample dotnet by @baywet in microsoft/agent-framework#7913 * .NET: Add timeout for wait-for-first-completion by @westey-m in microsoft/agent-framework#7911 * .NET: Fix duplicate Foundry AgentHost port binding by @rogerbarreto in microsoft/agent-framework#7932 * .NET: tests: removes dependency on fluent assersion because of licensing concerns by @baywet in microsoft/agent-framework#7938 * .NET: docs(decisions): resolve duplicate ADR sequence numbers (0016, 0021, 0024) by @jluocsa in microsoft/agent-framework#6046 * .NET: Bump Azure.Core from 1.61.0 to 1.62.0 by @dependabot[bot] in microsoft/agent-framework#7954 * .NET: Improve Cosmos DB Emulator startup reliability by @TheovanKraay in microsoft/agent-framework#3932 * .NET: add public API analyzers by @baywet in microsoft/agent-framework#7935 * .NET: Update version for 1.20.0 release by @SergeyMenshykh in microsoft/agent-framework#7972 ## New Contributors * @madanmishra1223 made their first contribution in microsoft/agent-framework#7705 * @YashvantHange made their first contribution in microsoft/agent-framework#7850 * @jihadkhawaja made their first contribution in microsoft/agent-framework#7792 * @adamsitnik made their first contribution in microsoft/agent-framework#7878 * @baywet made their first contribution in microsoft/agent-framework#7870 * @Namraa310806 made their first contribution in microsoft/agent-framework#7901 * @Sweetteabittersugar made their first contribution in microsoft/agent-framework#7903 * @shoemoney made their first contribution in microsoft/agent-framework#7837 * @jluocsa made their first contribution in microsoft/agent-framework#6046 **Full Changelog**: microsoft/agent-framework@dotnet-1.19.0...dotnet-1.20.0 Commits viewable in [compare view](microsoft/agent-framework@dotnet-1.19.0...dotnet-1.20.0). </details> Updated [Microsoft.Agents.AI.OpenAI](https://github.com/microsoft/agent-framework) from 1.19.0 to 1.20.0. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Agents.AI.OpenAI's releases](https://github.com/microsoft/agent-framework/releases)._ ## 1.20.0 ## What's Changed * .NET: Bump AWSSDK.Extensions.Bedrock.MEAI from 4.0.6.10 to 4.0.101.8 by @dependabot[bot] in microsoft/agent-framework#7829 * .NET: Stabilize Foundry recovery tests by @rogerbarreto in microsoft/agent-framework#7817 * .NET: fix: preserve Responses logprobs field by @he-yufeng in microsoft/agent-framework#5860 * .NET: Honor cancellation for Foundry-hosted workflow responses by @rogerbarreto with @Copilot in microsoft/agent-framework#7842 * .NET: Use Responses API for hosted web search in AG-UI by @rogerbarreto with @Copilot in microsoft/agent-framework#7843 * .NET: Bump Aspire.Hosting from 13.1.0 to 13.5.2 by @dependabot[bot] in microsoft/agent-framework#7826 * .NET: Suppress false positive Zip Slip alert by @SergeyMenshykh in microsoft/agent-framework#7858 * .NET: added Mem0Sharp integration for in-memory storage in agent samples. by @jihadkhawaja in microsoft/agent-framework#7792 * .NET: Annotate DevUI aggregator static-analysis false positives by @SergeyMenshykh in microsoft/agent-framework#7864 * .NET: Rename CommunityToolkit.VectorData.CosmosNoSql to AzureCosmosDB by @adamsitnik in microsoft/agent-framework#7878 * .NET: chore: upgrades aspnet openapi dependency by @baywet in microsoft/agent-framework#7870 * .NET: Simplify A2A function tool samples by @SergeyMenshykh in microsoft/agent-framework#7861 * .NET: Bump Azure.AI.AgentServer.Invocations from 1.0.0-beta.5 to 1.0.0-beta.6 by @dependabot[bot] in microsoft/agent-framework#7886 * .NET: docs: updates the contributing information for CFS users by @baywet in microsoft/agent-framework#7869 * Bump CommunityToolkit.VectorData.InMemory from 1.0.0 to 1.0.1 by @dependabot[bot] in microsoft/agent-framework#7888 * .NET: Remove retired OpenAI Assistants integration tests by @rogerbarreto in microsoft/agent-framework#7896 * .NET: Simplify A2A client-server sample by @SergeyMenshykh in microsoft/agent-framework#7891 * Bump Dapr.AI.Microsoft.Extensions from 1.18.4 to 1.18.5 by @dependabot[bot] in microsoft/agent-framework#7889 * .NET: docs/workflow fileinput sample dotnet by @baywet in microsoft/agent-framework#7913 * .NET: Add timeout for wait-for-first-completion by @westey-m in microsoft/agent-framework#7911 * .NET: Fix duplicate Foundry AgentHost port binding by @rogerbarreto in microsoft/agent-framework#7932 * .NET: tests: removes dependency on fluent assersion because of licensing concerns by @baywet in microsoft/agent-framework#7938 * .NET: docs(decisions): resolve duplicate ADR sequence numbers (0016, 0021, 0024) by @jluocsa in microsoft/agent-framework#6046 * .NET: Bump Azure.Core from 1.61.0 to 1.62.0 by @dependabot[bot] in microsoft/agent-framework#7954 * .NET: Improve Cosmos DB Emulator startup reliability by @TheovanKraay in microsoft/agent-framework#3932 * .NET: add public API analyzers by @baywet in microsoft/agent-framework#7935 * .NET: Update version for 1.20.0 release by @SergeyMenshykh in microsoft/agent-framework#7972 ## New Contributors * @madanmishra1223 made their first contribution in microsoft/agent-framework#7705 * @YashvantHange made their first contribution in microsoft/agent-framework#7850 * @jihadkhawaja made their first contribution in microsoft/agent-framework#7792 * @adamsitnik made their first contribution in microsoft/agent-framework#7878 * @baywet made their first contribution in microsoft/agent-framework#7870 * @Namraa310806 made their first contribution in microsoft/agent-framework#7901 * @Sweetteabittersugar made their first contribution in microsoft/agent-framework#7903 * @shoemoney made their first contribution in microsoft/agent-framework#7837 * @jluocsa made their first contribution in microsoft/agent-framework#6046 **Full Changelog**: microsoft/agent-framework@dotnet-1.19.0...dotnet-1.20.0 Commits viewable in [compare view](microsoft/agent-framework@dotnet-1.19.0...dotnet-1.20.0). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Motivation & Context
#7105 gave chat clients a fallback so an unmapped provider finish reason value passes through as the raw string instead of vanishing to
None. It touchedag,bedrock,claude,core,github_copilot,ollama, andopenai. It did not touchgemini.Bedrock's version of the same method after #7105:
Gemini's still has no fallback:
_FINISH_REASON_MAPcovers 13 of the 18 members ofgoogle.genai.types.FinishReasonin the package's own pinned dependency range (google-genai>=1.69.0,<3.0.0). Missing:OTHER,TOO_MANY_TOOL_CALLS,NO_IMAGE,IMAGE_OTHER(plusFINISH_REASON_UNSPECIFIED, which is a legitimate absent case and correctly staysNone).The bug cascades. In
_process_chunk:Usage is attached to a streamed chunk only when
finish_reasonis truthy, so an unmapped reason drops the finish reason and the whole turn's token/billing accounting.ChatTelemetryLayerinobservability.pyskips recording the terminal span state the same way.Concrete scenario: a Gemini call inside an agentic tool loop trips Gemini's own tool-call-count guardrail and returns
finish_reason=TOO_MANY_TOOL_CALLS. The caller seesfinish_reason=None, no usage, and no way to tell the run stopped abnormally instead of completing normally.Description & Review Guide
_map_finish_reasonnow falls back to the raw reason string, wrapped asFinishReason(reason), instead ofNone.FINISH_REASON_UNSPECIFIEDand an absent reason still map toNone.FinishReasonLiteral | NonetoFinishReasonLiteral | FinishReason | None. This is not the same pattern bedrock uses (str | Nonewith no wrapping) — bedrock's file carries a blanket# type: ignoreat the top of the module, gemini's does not, so a barestrfails this package's strict Pyright config.FinishReason(reason)is the same constructollamaandopenaiused for the same fallback in Python: Normalize chat finish reasons #7105.OTHER,TOO_MANY_TOOL_CALLS,NO_IMAGE,IMAGE_OTHER) is now surfaced to the caller instead of silently disappearing, and the final streamed chunk's usage/token accounting is no longer dropped alongside it.FINISH_REASON_UNSPECIFIEDis the only value that should keep mapping toNone, or whether other values should be excluded from the fallback too.Related Issue
Fixes #7836
Testing
Ran the gemini package's unit test suite only (not the full monorepo suite):
GREEN (with the fix): 157 passed, 8 deselected (integration tests, no credentials configured).
Reverted only the source change (kept the new/updated tests) to confirm RED:
Restored the fix, reran, back to GREEN (157 passed).
Also ran
pyrightandruff check/ruff format --checkagainst the changed files only; all clean. Did not run the .NET suite, the full Python monorepo test suite, or integration tests (no Gemini/Vertex credentials in this environment).Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.