fix(google): map Gemini finish_reason via enum value, not str()#2595
Conversation
Signed-off-by: WatchTree-19 <119982314+WatchTree-19@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request fixes an issue in the Google Chat translator where the candidate.finish_reason enum was not being correctly mapped because its string representation included the enum class name (e.g., FinishReason.SAFETY). The fix extracts the underlying string value using getattr before performing the lookup. Additionally, regression tests have been added to verify that finish reasons like MAX_TOKENS and SAFETY map correctly. I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…message Follow-up on the finish_reason enum fix from code review: - Extract the finish reason via an explicit ``FinishReason.value`` ternary instead of ``str(getattr(candidate.finish_reason, "value", ...))``. The SDK is an optional dependency imported by the caller, so ``FinishReason`` is resolved with a lazy local import (never at module load). This drops the redundant ``str()`` on the enum path while staying type-clean. - Remove the dead ``"SAFETY": "content_filter"`` map entry: it is unconditionally overridden to ``"refusal"`` by the REFUSAL_REASONS merge. - Add a regression test covering the ``finish_message`` branch of the refusal text (previously only the reason-code fallback was exercised). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E5chPh9nQey5A8ztMeqjQf
kevinmessiaen
left a comment
There was a problem hiding this comment.
Thanks for the fix @WatchTree-19
problem
in the Gemini -> giskard translator (
llm/translators/google_chat.py), the finish reason is looked up withstr(candidate.finish_reason):but
candidate.finish_reasonis agoogle.genaiFinishReasonenum, andstr(FinishReason.MAX_TOKENS)is"FinishReason.MAX_TOKENS", not"MAX_TOKENS".FINISH_REASON_MAPis keyed on the bare names ("MAX_TOKENS","SAFETY", ...), so every non-STOP reason misses the map and falls back to"stop".STOPonly "works" by accident because the fallback is also"stop".net effect: Gemini truncations are reported as normal completions (
lengthlost) and, more importantly for a safety tool, SAFETY blocks / refusals are never surfaced as refusals.repro (real google-genai SDK)
fix
look up via the enum's
.value(getattr(finish_reason, "value", finish_reason), robust to both the real enum and the plain-string form used in dict-validated inputs). also fixed therefusal_outline so refusal text is a clean reason string rather than"FinishReason.SAFETY".test
added
test_from_google_max_tokens_maps_to_lengthandtest_from_google_safety_maps_to_refusalmirroring the file's existing tests. before:2 failed(assert 'stop' == 'length',assert 'stop' == 'refusal'); after: pass. the existing translator tests only coveredSTOP, which is indistinguishable from the fallback, so the bug went unnoticed. ruff clean.found by reading the translator.