fix(gemini): map candidate.finish_reason to the OpenAI vocabulary - #1202
Conversation
The Gemini converters discarded candidate.finish_reason: the non-streaming path hardcoded "tool_calls"/"stop" and the streaming path only mapped STOP. As a result LengthFinishReasonError and ContentFilterFinishReasonError never fired for Gemini, and truncated structured output surfaced as a misleading malformed-JSON ValidationError. Map MAX_TOKENS to "length" and the safety-related reasons (SAFETY, RECITATION, PROHIBITED_CONTENT, BLOCKLIST, SPII, IMAGE_*) to "content_filter" in both converters. Truncation and filtering take priority over "tool_calls", and a choice is emitted even when Gemini returns no content (e.g. a thinking model spent the whole max_output_tokens budget on reasoning), so callers see the terminal reason instead of empty choices. Unmapped reasons fall back to "stop" for complete responses and stay None for stream chunks. Fixes mozilla-ai#1196 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughGemini finish reasons are mapped to OpenAI-compatible values in both response converters. Terminal responses now preserve truncation and content-filter outcomes when content is absent. Unit and integration tests cover structured-output exceptions. ChangesGemini finish-reason handling
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/any_llm/providers/gemini/utils.py`:
- Around line 374-386: Update _FINISH_REASON_MAP and the choice-generation logic
in _convert_response_to_response_dict to treat MALFORMED_FUNCTION_CALL and
UNEXPECTED_TOOL_CALL as terminal error reasons, ensuring choices is populated
even when content and tool_calls are absent so existing error handling is
reached. Preserve STOP without content as returning no choices, and retain the
current length/content_filter behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ce28e44-b2ac-4c3a-ae2a-ca630a319297
📒 Files selected for processing (3)
src/any_llm/providers/gemini/utils.pytests/integration/test_finish_reason.pytests/unit/providers/test_gemini_provider.py
Two of the new test lines exceeded the 120 character limit, so ruff-format (pinned at v0.15.20 in .pre-commit-config.yaml) reformatted them and the run-linter job would have failed. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
... and 15 files with indirect coverage changes 🚀 New features to boost your workflow:
|
njbrake
left a comment
There was a problem hiding this comment.
Note: this review was drafted by Claude via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's.
Approving. Thanks for the thorough writeup, and for validating the integration test in both directions; that made this much faster to review.
I pushed one commit to your branch (87d18b5): two of the new test lines were over the 120 character limit, so ruff-format (pinned at v0.15.20) rewrapped them, and run-linter would have failed. It slipped through because CI on this PR was sitting at action_required awaiting approval for a first-time fork contributor, so nothing beyond the template check ever ran. I have approved the workflows and everything is green now.
What I verified beyond reading the diff:
- All ten mapped
FinishReasonmembers resolve on google-genai 2.14.0. Worth checking, since google-genai is unpinned in pyproject.toml. - Unknown wire values are safe. google-genai synthesizes a pseudo-member for a reason it does not recognize, so
_FINISH_REASON_MAP.get()returnsNoneand falls through to your"stop"/Nonefallbacks rather than raising. - A differential run of both converters, main versus this branch, over 6 content shapes by 18 finish reasons. Every divergence is intentional: mapped reasons now surface,
lengthandcontent_filteremit a choice where main returnedchoices == [], and the streaming path returns a chunk where main raisedAssertionError.STOPwith no content still returnschoices == [], and all six unmapped reasons (OTHER,LANGUAGE,NO_IMAGE,IMAGE_OTHER,MALFORMED_FUNCTION_CALL,UNEXPECTED_TOOL_CALL) behave identically to main. Nothing that previously worked got reclassified. - Clean merge against current main, with the unit suite and ruff both passing on the merged state. Of the commits main has gained since your merge-base, only #1198 and #1201 touch
any_llm.py, and neither goes near the finish-reason guard.
I agree with your response to the CodeRabbit finding on MALFORMED_FUNCTION_CALL / UNEXPECTED_TOOL_CALL. The matrix confirms those two produce identical output on main and on this branch, so it is pre-existing behavior, and a dedicated error contract is the right follow-up rather than folding it into #1196.
I have added the run-integration-tests label to get the Gemini suite running here with real keys, since this changes the Gemini response path for all callers and not only structured-output ones.
One follow-up worth doing later, not blocking this: google-genai is unpinned and _FINISH_REASON_MAP falls back to "stop" for anything new Google adds, so a future safety-flavored reason would quietly regress #1196. A test that iterates types.FinishReason and asserts each member is either in the map or in an explicit known-unmapped set would turn that into a loud dependabot failure. It can live entirely in the test file.
…mozilla-ai#1216) actions/checkout v7.0.0 began refusing to check out fork PR code from a pull_request_target workflow (actions/checkout#2454). tests-integration.yaml picked that up via the v6 to v7 dependabot bump in mozilla-ai#1157, so since 2026-07-14 labeling a fork PR failed at checkout in determine-jobs-to-run and both test jobs were skipped. Found on mozilla-ai#1202, the first fork PR labeled since the bump. Set allow-unsafe-pr-checkout: true on the three checkout steps that pass a fork head, with a note at the top of the file recording why the opt-in is acceptable and what it does not cover. Dropping the ref: inputs would also satisfy the action but would test main instead of the PR. Narrow permissions: pull-requests: write is granted only on remove-label, leaving the jobs that execute fork code with contents: read. Set persist-credentials: false on all three checkouts so the token is not left in .git/config for that code to read. The label gate is any member at triage or above, not write access; the workflow does not verify the labeler's permission level. Recorded in the file rather than enforced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
## Description Preserve provider-typed safety refusals through the existing OpenAI-compatible completion contract so downstream agent runtimes can terminate blocked work without parsing refusal prose. - Anthropic `stop_reason="refusal"` maps to `finish_reason="content_filter"` in non-streaming and streaming responses, with `message.refusal` / `delta.refusal` populated. - Anthropic `stop_details` remains available under existing `extra_content.anthropic.stop_details`; ordinary model-written refusals remain ordinary text. - Anthropic terminal reasons now come from `message_delta`, so the official `content_block_stop -> message_delta -> message_stop` sequence emits exactly one terminal marker. - Gemini candidate content-filter reasons now populate `message.refusal` / `delta.refusal` in addition to the already-normalized `content_filter` finish reason. - Gemini prompt blocks with no candidates now become a typed content-filter choice rather than an empty completion or streaming assertion. - Empty unblocked Gemini chunks remain nonterminal; ordinary answers, reasoning, and tool calls are unchanged. This incorporates the original commit from #1306 unchanged, preserving its author and co-author attribution, and completes its streaming path. It also builds on the merged Gemini candidate mapping from #1202. A deterministic live safety-filter test would require committing prohibited prompts and would be inherently unstable as provider classifiers change. The safety paths therefore use official Anthropic and Google SDK response objects with their documented enums. Real-key integration tests verify that normal responses still work through both changed providers. ## PR Type - 🐛 Bug Fix ## Relevant issues - Extends and supersedes #1306 while preserving its original commit attribution. - Complements the Gemini candidate finish-reason work in #1202. ## Checklist - [x] I understand the code I am submitting. - [x] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [x] Documentation was updated where necessary (no user-facing API changed; provider metadata uses existing fields) - [x] I have read and followed the [contribution guidelines](https://github.com/mozilla-ai/any-llm/blob/main/CONTRIBUTING.md) - [x] **AI Usage:** - [ ] No AI was used. - [ ] AI was used for drafting/refactoring. - [x] This is fully AI-generated. ## Verification - `uv run pre-commit run --all-files --verbose` - `uv run pytest -q --reruns 0 tests/unit`: 2,289 passed, 67 optional-provider skips - Changed provider suites: 292 passed - Real-key integration, `gemini-3-flash-preview` and `claude-haiku-4-5`: - async non-streaming completion - async streaming completion - 4 passed; normal responses returned one `stop` and no refusal - Cross-repository consumer smoke: Gemini prompt-block and Anthropic refusal fixtures crossed `AnyLLMModel` in streamed and non-streamed modes and became the consumer's typed terminal safety outcome. - Independent Standards re-review: no findings. - Independent Spec re-review against the official provider schemas: no findings. ## AI Usage Information - AI Model used: GPT-5 Codex - AI Developer Tool used: Codex - Any other info you'd like to share: The implementation was iterated under human direction, tested against official SDK response types, live normal provider calls, and independently reviewed along separate Standards and Spec axes. - [x] I am an AI Agent filling out this form (check box if true) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added clearer content-filter refusal details to Anthropic and Gemini responses. * Added support for Gemini prompt-block refusals, including streamed responses. * Preserved Anthropic stop details alongside thinking information. * Added consistent mapping of provider stop reasons to standard finish reasons. * **Bug Fixes** * Improved consistency of streaming finish reasons. * Prevented duplicate finish reasons in streamed Anthropic responses. * Ensured filtered responses are represented correctly when no candidate is available. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Tony Coder <407243179@qq.com>
…1358) ## Description The Bedrock provider maps only three of the Converse API's nine `stopReason` values, so a guardrail-blocked or context-overflowed response reaches the caller as an ordinary `finish_reason="stop"`. `src/any_llm/providers/bedrock/utils.py:517` (non-streaming): ```python finish_reason: Literal["stop", "length"] = "length" if stop_reason == "max_tokens" else "stop" ``` and `:611-617` (streaming) handles `max_tokens` and `tool_use` and sends everything else to `"stop"`. The visible failure is the structured-output guard in `src/any_llm/any_llm.py:798-806`. When a Bedrock Guardrail blocks a response, Converse returns `stopReason="guardrail_intervened"` with the guardrail's blocked-message text as content. Because that arrives as `finish_reason="stop"`, the `content_filter` branch never fires and the guardrail's prose is handed to `parse_json_content()` instead, so the caller gets a pydantic `ValidationError` from an unrelated layer rather than the `ContentFilterFinishReasonError` this library raises for every other provider. `content_filtered` behaves the same way, and `model_context_window_exceeded` reports as `"stop"` rather than `"length"`. This is the same fix already merged for gemini (#1202), zai (#1204), cohere (#1301) and anthropic (#1306/#1328). Bedrock was the last provider still hardcoding a partial mapping, so this follows `ANTHROPIC_STOP_REASON_TO_FINISH_REASON` in shape and naming: ```python BEDROCK_STOP_REASON_TO_FINISH_REASON: dict[str, _FinishReason] = { "end_turn": "stop", "max_tokens": "length", "model_context_window_exceeded": "length", "tool_use": "tool_calls", "content_filtered": "content_filter", "guardrail_intervened": "content_filter", } ``` `stop_sequence`, `malformed_model_output` and `malformed_tool_use` have no OpenAI counterpart and keep falling through to the `"stop"` default, as do any values a future service model adds. Both call sites now go through one `_map_stop_reason()` helper, which also lets the `cast` at the non-streaming call site go away. The nine-value enum is not taken from the AWS docs prose. It is read out of the installed botocore service model (`bedrock-runtime`, shape `StopReason`), which is the same source the SDK validates against: ```python >>> import botocore.session >>> botocore.session.Session().get_service_model("bedrock-runtime").shape_for("StopReason").enum ['end_turn', 'tool_use', 'max_tokens', 'stop_sequence', 'guardrail_intervened', 'content_filtered', 'malformed_model_output', 'malformed_tool_use', 'model_context_window_exceeded'] ``` The tests read the enum from that same service model and parametrize over it, so a botocore upgrade that adds a stop reason fails the suite instead of silently defaulting the new reason to `"stop"`. ## Reproduction ```python import asyncio from unittest.mock import Mock from pydantic import BaseModel from any_llm.exceptions import ContentFilterFinishReasonError from any_llm.providers.bedrock import BedrockProvider class City(BaseModel): name: str # What Converse returns when a Bedrock Guardrail blocks the response. blocked = { "output": {"message": {"content": [{"text": "Sorry, I cannot answer that."}]}}, "stopReason": "guardrail_intervened", } client = Mock() client.converse.return_value = blocked provider = BedrockProvider(client=client) print("finish_reason:", provider._convert_completion_response(blocked).choices[0].finish_reason) try: asyncio.run( provider.acompletion( model="us.anthropic.claude-sonnet-4-20250514-v1:0", messages=[{"role": "user", "content": "Hello"}], response_format=City, ) ) except Exception as exc: print("raised:", type(exc).__name__) ``` Before: ``` finish_reason: stop raised: ValidationError ``` After: ``` finish_reason: content_filter raised: ContentFilterFinishReasonError ``` ## Tests Added to `tests/unit/providers/test_aws_provider.py`: - `test_convert_response_maps_every_bedrock_stop_reason` and `test_streaming_chunk_maps_every_bedrock_stop_reason`, parametrized over the full botocore `StopReason` enum, asserting both paths agree. - `test_convert_response_without_stop_reason_finishes_as_stop`. - `test_guardrail_blocked_structured_output_raises_content_filter_error`, the end-to-end case above. On the unfixed tree these fail: ``` FAILED test_convert_response_maps_every_bedrock_stop_reason[tool_use] FAILED test_convert_response_maps_every_bedrock_stop_reason[guardrail_intervened] FAILED test_convert_response_maps_every_bedrock_stop_reason[content_filtered] FAILED test_convert_response_maps_every_bedrock_stop_reason[model_context_window_exceeded] FAILED test_streaming_chunk_maps_every_bedrock_stop_reason[guardrail_intervened] FAILED test_streaming_chunk_maps_every_bedrock_stop_reason[content_filtered] FAILED test_streaming_chunk_maps_every_bedrock_stop_reason[model_context_window_exceeded] FAILED test_guardrail_blocked_structured_output_raises_content_filter_error 8 failed, 12 passed ``` With the fix, `uv run pytest tests/unit/providers/test_aws_provider.py` → 116 passed. The `[tool_use]` case is the one behaviour change beyond the three broken reasons: a `stopReason="tool_use"` response that carries no `toolUse` block now reports `"tool_calls"` instead of `"stop"`. Both real `tool_use` shapes (a genuine tool call, and the synthetic `any_llm_structured_output` unwrap) return earlier in `_convert_response` and are untouched. ## PR Type - 🐛 Bug Fix ## Relevant issues No open issue. Same fix as #1202 / #1204 / #1301 / #1328, applied to the remaining provider. ## Checklist - [x] I understand the code I am submitting. - [x] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [x] Documentation was updated where necessary (no user-facing API changed) - [x] I have read and followed the [contribution guidelines](https://github.com/mozilla-ai/any-llm/blob/main/CONTRIBUTING.md) - [x] **AI Usage:** - [ ] No AI was used. - [ ] AI was used for drafting/refactoring. - [x] This is fully AI-generated. ## Verification - `uv run pre-commit run --all-files` (ruff, ruff-format, mypy, codespell) clean. - `uv run pytest tests/unit/providers/test_aws_provider.py` → 116 passed. - `uv run pytest tests/unit` → 2308 passed, 69 skipped, 16 failed. All 16 failures are in `test_anthropic_messages.py` / `test_anthropic_provider.py` and reproduce identically on an unmodified `main` in this environment: `TypeError: Invalid 'http_client' argument; Expected an instance of httpx2.AsyncClient but got <class 'httpx.AsyncClient'>`, from what a local `uv sync --all-extras -U` resolves. Nothing bedrock-related. - No integration run: I do not have AWS Bedrock credentials, and the two Bedrock paths this touches are pure response converters exercised by the unit tests above against service-model-sourced stop reasons. ## AI Usage Information - AI Model used: Claude Opus 5 - AI Developer Tool used: Claude Code - Any other info you'd like to share: The stop-reason enum was read from the installed botocore service model rather than the AWS docs, and the tests read it from the same place so the list cannot drift. - [x] I am an AI Agent filling out this form (check box if true) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Amazon Bedrock response handling for length limits, tool calls, and content-filter outcomes. * Standardised finish reasons across streaming and non-streaming responses. * Unknown or unsupported stop reasons now safely default to `stop`. * Structured-output requests blocked by a Bedrock guardrail now report a content-filter error correctly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Description
The Gemini converters discarded
candidate.finish_reason: the non-streaming path hardcoded"tool_calls"/"stop", and the streaming path only special-casedSTOP. As a result the #881 exceptions (LengthFinishReasonError,ContentFilterFinishReasonError) could never fire for Gemini, and truncated structured output surfaced as a misleading malformed-JSONValidationError(see #1196 for a production repro).Following the approach endorsed in the issue comments:
candidate.finish_reasonvia a shared_map_finish_reasonhelper:MAX_TOKENS -> "length";SAFETY,RECITATION,PROHIBITED_CONTENT,BLOCKLIST,SPII,IMAGE_SAFETY,IMAGE_PROHIBITED_CONTENT,IMAGE_RECITATION->"content_filter";STOP -> "stop". Unmapped reasons fall back to"stop"for complete responses and stayNonefor stream chunks, so non-terminal chunks are not forced to a terminal reason."tool_calls"in both paths (shared_resolve_finish_reasonhelper), so a response cut short mid tool call does not look like a completed tool-call round.max_output_tokensbudget on reasoning), so the feat: Raise exceptions on finish_reason='length' or 'content_filter' for ParsedChatCompletion #881 guard fires instead of returning emptychoices. The streaming converter previouslyasserted oncandidate.content, so the same empty-content terminal chunk would have crashed there; it now handles it too.vertexaiinherits both converters fromGoogleProvider, so it picks up the fix automatically.Tests:
acompletionwith a pydanticresponse_formatassertingLengthFinishReasonErrorandContentFilterFinishReasonError(mocked client,tests/unit)._map_finish_reason, representative rows through both converters, the tool-call priority rule, and the empty-content edge cases.tests/integration/test_finish_reason.py) reproducing the issue: smallmax_tokens+ pydanticresponse_formatmust raiseLengthFinishReasonError. Verified it fails onmain(malformed-JSONValidationError, 6/6 attempts) and passes with the fix. A liveContentFilterFinishReasonErrortest was deliberately left out: reliably tripping Gemini's safety filter requires keeping prohibited content in the repo and is inherently flaky as Google tunes the filters, so that path is covered at the unit level.Ran locally:
pre-commit run --all-filesclean,pytest tests/unitgreen (1603 passed), and the full Gemini integration suite with a real key (25 passed, 5 pre-existing "gemini does not support responses" skips).@njbrake pinging you as offered in the issue, review welcome!
PR Type
Relevant issues
Fixes #1196
Checklist
AI Usage Information
AI Model used: Claude Fable 5
AI Developer Tool used: Claude Code
Any other info you'd like to share: Implementation, tests, and this PR text were AI-generated under human direction and review; the integration test was validated against the real Gemini API in both directions (fails on main, passes with the fix).
I am an AI Agent filling out this form (check box if true)
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests