Skip to content

Commit 692af95

Browse files
aperepellizzij
authored andcommitted
refactor: address bot review suggestions
- Simplify tracing.py by removing isinstance check (always enum now) - Refactor test assertions to use dictionary mapping instead of if/elif - Reduce code duplication and improve readability Addresses Gemini Code Assist bot suggestions: - tracing.py: Direct .name access since finish_reason is always enum - test_litellm.py: Dictionary mapping for cleaner test assertions
1 parent a1c0938 commit 692af95

2 files changed

Lines changed: 11 additions & 15 deletions

File tree

src/google/adk/telemetry/tracing.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,8 @@ def trace_call_llm(
303303
llm_response.usage_metadata.candidates_token_count,
304304
)
305305
if llm_response.finish_reason:
306-
if isinstance(llm_response.finish_reason, types.FinishReason):
307-
finish_reason_str = llm_response.finish_reason.name.lower()
308-
else:
309-
# Defensive fallback for string values (should never occur - all values mapped to enum)
310-
finish_reason_str = str(llm_response.finish_reason).lower()
306+
# finish_reason is always FinishReason enum
307+
finish_reason_str = llm_response.finish_reason.name.lower()
311308
span.set_attribute(
312309
'gen_ai.response.finish_reasons',
313310
[finish_reason_str],

tests/unittests/models/test_litellm.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,17 +1967,16 @@ async def test_finish_reason_propagation(
19671967

19681968
async for response in lite_llm_instance.generate_content_async(llm_request):
19691969
assert response.content.role == "model"
1970-
# Verify finish_reason is mapped to FinishReason enum, not raw string
1970+
# Verify finish_reason is mapped to FinishReason enum
19711971
assert isinstance(response.finish_reason, types.FinishReason)
1972-
# Verify correct enum mapping
1973-
if finish_reason == "length":
1974-
assert response.finish_reason == types.FinishReason.MAX_TOKENS
1975-
elif finish_reason == "stop":
1976-
assert response.finish_reason == types.FinishReason.STOP
1977-
elif finish_reason == "tool_calls":
1978-
assert response.finish_reason == types.FinishReason.STOP
1979-
elif finish_reason == "content_filter":
1980-
assert response.finish_reason == types.FinishReason.SAFETY
1972+
# Verify correct enum mapping using dictionary
1973+
expected_mapping = {
1974+
"length": types.FinishReason.MAX_TOKENS,
1975+
"stop": types.FinishReason.STOP,
1976+
"tool_calls": types.FinishReason.STOP,
1977+
"content_filter": types.FinishReason.SAFETY,
1978+
}
1979+
assert response.finish_reason == expected_mapping[finish_reason]
19811980
if expected_content:
19821981
assert response.content.parts[0].text == expected_content
19831982
if has_tool_calls:

0 commit comments

Comments
 (0)