fix(EVO-2181): judge the inline mime exactly as ADK does - #45
Merged
Conversation
Review follow-up to EVO-2181. _inline_skip_reason normalized the content type before checking it (lowercase, parameters dropped) while the Blob kept it verbatim, so the guard and ADK were answering different questions. _get_content matches the raw value with a case-sensitive startswith and an exact-match set, which means "IMAGE/PNG" and "application/pdf; charset=binary" cleared the guard and then raised ValueError inside ADK — the 500 that costs the customer the whole turn, caption included, and precisely what the guard was added to prevent. Checking the verbatim value closes the gap without changing any accepted case: "audio/webm;codecs=opus" still matches the audio/ prefix with its parameter attached. The new tests assert both halves — that those types are skipped, and that ADK really does raise on them — so an ADK bump that widens what it accepts fails here instead of quietly dropping readable media. Also adds .github/workflows/ci.yml: nothing ran pytest on a PR, including the PR this fixes. tests/unit/test_exception_handlers.py is excluded there with the reason written down — it fails at collection because generic_exception_handler no longer exists, which is a separate regression.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @gomessguii, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
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
1 participant
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.
Review follow-up to EVO-2181 (PR #44, already merged). Finding from the EVO-2178 review of the merged media path.
The guard judged the mime differently from ADK
_inline_skip_reasonnormalized the content type before checking it (lowercase, parameters dropped) while theBlobkept it verbatim, so the guard and ADK were answering different questions._get_content(google-adk 1.19) matches the raw value with a case-sensitivestartswithand an exact-match set, which meansIMAGE/PNGandapplication/pdf; charset=binarycleared the guard and then raisedValueErrorinside ADK — the500that costs the customer the whole turn, caption included, and precisely what the guard was added to prevent.Checking the verbatim value closes the gap without changing any accepted case:
audio/webm;codecs=opusstill matches theaudio/prefix with its parameter attached.The new tests assert both halves — that those types are skipped, and that ADK really does raise on them — so an ADK bump that widens what it accepts fails here instead of quietly dropping readable media.
Also in this PR
.github/workflows/ci.yml. Nothing ran pytest on a PR, including the PR this fixes.tests/unit/test_exception_handlers.pyis excluded there with the reason written down: it fails at collection becausegeneric_exception_handlerno longer exists insrc/core/exception_handlers.py(removed ine02a229) and is no longer registered insrc/main.py. That is a separate regression (EVO-972 kept internal class names out of500bodies) and needs its own card — drop the exclusion when it is restored.Verification
pytest tests/unit/test_media_file_parts.py→ 22 passed. Fullpytest tests/unit(with the documented--ignore) → 244 passed. Run inside the processor image (google-adk==1.19.0).Part of EVO-2178 · follow-up to EVO-2181.
🤖 Generated with Claude Code