Skip to content

fix(models): raise ValueError instead of assert in Gemma model check - #7356

Open
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix/gemma-model-check-valueerror
Open

chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix/gemma-model-check-valueerror

Conversation

@chelsealong

Copy link
Copy Markdown
Contributor

Fixes #7355

Gemma.generate_content_async() validated the model family with assert, which is stripped under python -O. It now raises ValueError (matching the repo's "No Asserts in Production Code" guidance). The existing test_not_gemma_model is updated to expect ValueError.

Testing:

  • Without the fix: 1 failed, 25 passed (test_not_gemma_model hit the AssertionError at gemma_llm.py:255).
  • With the fix: 26 passed (pytest tests/unittests/models/test_gemma_llm.py).

AI assistance: drafted with Claude Code; reviewed and tested by the author.

assert is stripped under python -O, so the non-Gemma model guard vanished.

@MateehUllah MateehUllah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this against #7355. Replacing the runtime assert with an explicit ValueError addresses the reported python -O / -OO behavior, since the model-family guard is no longer stripped by optimization.

The updated negative-model test also correctly checks the new exception type. I don't see a correctness issue with the change.

One optional improvement would be adding an optimization-mode regression test, but the implementation itself directly fixes the underlying issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gemma model-family validation is skipped under python -O

3 participants