fix: replace bare asserts with ValueError in _normalize_response (tool response parsing)#716
Open
DhruvTilva wants to merge 1 commit into
Open
Conversation
_normalize_response() used bare �ssert statements to validate that an MCP tool response contains exactly one text content block before converting it to structuredContent. This has two problems: 1. Python removes �ssert statements when running with -O / PYTHONOPTIMIZE, so the guard disappears entirely in optimized production deployments. 2. When the assert fires, it raises AssertionError with no message, giving the caller no information about which tool, which response, or what the content actually was. MCP tools routinely return multiple content blocks or non-text types (image, blob), both of which are valid per the MCP spec. Replace both asserts with explicit ValueError that include the unexpected content in the message so failures are immediately actionable. Add _manager_test.py with tests for the normal path and both error cases, which also serve as the first test coverage for this module.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This has two concrete problems:
1. The guards disappear in optimized deployments.
Python silently removes all
assertstatements when running with-OorPYTHONOPTIMIZE=1. This means the validation is completely absent in anyproduction environment using optimized Python, allowing invalid tool
responses to corrupt
structuredContentwith no error.2. The error gives no useful context.
When the assert does fire (in non-optimized mode), the caller sees:
No information about which tool, how many content blocks were returned, or
what the types were. Debugging requires a full stack trace and a debugger.
Root cause
_normalize_responsewas written assuming MCP tools always return a singletext content block. This is a common case but not a contract — the MCP spec
allows tools to return multiple content blocks and non-text types (image,
blob). Both are valid and observable in real MCP server implementations.
Fix
Replace both
assertstatements with explicitValueErrorthat include theactual content in the message:
ValueErroris the correct exception for runtime data validation. UnlikeAssertionError, it is never optimized away, and the message immediatelyidentifies what the tool actually returned.
Testing
Added
gemma/gm/tools/_manager_test.py— the first test file for thismodule — with four tests:
test_normalize_response_passthrough_when_structured_content_presenttest_normalize_response_success_single_text_blocktest_normalize_response_error_single_text_blocktest_normalize_response_raises_for_multiple_content_blocks← catches the bugtest_normalize_response_raises_for_non_text_content_type← catches the bugThe last two tests would have failed against the original code (wrong
exception type) and pass after the fix.