Skip to content

feat: Add Gemini 3 Pro support with thinking_level parameter#47

Merged
Kamilbenkirane merged 1 commit intomainfrom
feature/gemini-3-support
Nov 19, 2025
Merged

feat: Add Gemini 3 Pro support with thinking_level parameter#47
Kamilbenkirane merged 1 commit intomainfrom
feature/gemini-3-support

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

Changes

  • Add gemini-3-pro-preview model with 65,536 output token limit
  • Add thinking_level parameter (low/high) for Gemini 3 Pro
  • Update all Gemini 2.5 models to correct 65,536 output token limits
    • gemini-2.5-flash: 8,192 → 65,536
    • gemini-2.5-flash-lite: 8,192 → 65,536
    • gemini-2.5-pro: 8,192 → 65,536
  • Add ThinkingLevelMapper for Google provider
  • Bump version to 0.2.9

Testing

All pre-commit hooks passed:

  • ✅ Type checking with mypy
  • ✅ Security check with Bandit
  • ✅ Tests with coverage

References

- Add gemini-3-pro-preview model with 65,536 output token limit
- Add thinking_level parameter (low/high) for Gemini 3
- Update all Gemini 2.5 models to correct 65,536 output token limits
- Add ThinkingLevelMapper for Google provider
- Bump version to 0.2.9
@claude
Copy link
Copy Markdown

claude Bot commented Nov 19, 2025

Pull Request Review: Gemini 3 Pro Support

Summary

This PR adds support for Google's Gemini 3 Pro model with the thinking_level parameter and updates output token limits for Gemini 2.5 models. The implementation follows established patterns in the codebase and appears well-structured.

✅ Strengths

Code Quality

  • Consistent patterns: The ThinkingLevelMapper follows the same structure as ThinkingBudgetMapper and other parameter mappers
  • Clean implementation: The mapper correctly uses setdefault() for nested dictionary creation
  • Type safety: Proper use of Choice constraint with explicit options (["low", "high"])
  • Documentation: Good docstrings following the existing style

Architecture

  • Separation of concerns: Model definitions and parameter mapping are properly separated
  • Unified interface: The THINKING_LEVEL parameter is correctly added to the unified TextGenerationParameters class
  • Constraint validation: Proper use of the constraint system via _validate_value()

🔍 Observations & Suggestions

1. Missing Test Coverage ⚠️

Issue: No tests found for the new ThinkingLevelMapper or the Gemini 3 Pro model configuration.

Recommendation: Add unit/integration tests to cover:

  • Validation of valid thinking_level values ("low", "high")
  • Rejection of invalid values with appropriate error messages
  • Correct mapping to Google's API format
  • Integration test for Gemini 3 Pro generation

Example test structure (based on existing constraint tests):

def test_thinking_level_mapper_valid_values():
    mapper = ThinkingLevelMapper()
    model = # ... Gemini 3 Pro model
    
    result = mapper.map({}, "low", model)
    assert result["generationConfig"]["thinkingConfig"]["thinkingLevel"] == "low"

def test_thinking_level_mapper_rejects_invalid():
    mapper = ThinkingLevelMapper()
    model = # ... Gemini 3 Pro model
    
    with pytest.raises(ConstraintViolationError):
        mapper.map({}, "medium", model)

2. Output Token Limit Updates

Changes: Updated max tokens from 8,192 → 65,536 for:

  • gemini-2.5-flash
  • gemini-2.5-flash-lite
  • gemini-2.5-pro

Verification needed: Can you confirm these limits are from official Google documentation? The PR description links to docs, but explicit confirmation helps reviewers.

3. Gemini 3 Pro Constraints

Observation: Gemini 3 Pro uses thinking_level instead of thinking_budget (unlike Gemini 2.5 models).

Question: Is this an intentional API change by Google? Should we document this difference for users migrating between models?

4. Parameter Mapper Registration

Good: The mapper is correctly added to GOOGLE_PARAMETER_MAPPERS list (line 245 in parameters.py).

5. Type Annotation for thinking_level

Current: thinking_level: str | None in TextGenerationParameters

Consideration: While str | None is correct for the public API, consider whether a Literal["low", "high"] type hint would improve type safety at the call site. This is a minor point and the current approach is fine.

🔒 Security

✅ No security concerns identified:

  • No user input directly interpolated into requests
  • Constraint validation prevents invalid values
  • No credential handling changes

⚡ Performance

✅ No performance concerns:

  • Parameter mapping is lightweight
  • No blocking operations or heavy computation
  • Consistent with existing mapper implementations

🧪 Testing Status

According to the PR description, pre-commit hooks passed:

  • ✅ Type checking (mypy)
  • ✅ Security check (Bandit)
  • ✅ Tests with coverage

However: No new tests were added for the new functionality. This is the main gap.

📝 Documentation

Missing: User-facing documentation about:

  • When to use thinking_level vs thinking_budget
  • What "low" vs "high" thinking levels mean
  • Differences between Gemini 2.5 and 3.0 parameter support

Consider adding examples or updating API documentation.

🎯 Recommendations

Must Address:

  1. Add test coverage for ThinkingLevelMapper and Gemini 3 Pro model

Should Consider:

  1. Add inline documentation about thinking_level parameter behavior
  2. Verify and document the output token limit changes with official sources
  3. Consider adding migration notes for users switching between Gemini versions

Nice to Have:

  1. Add an integration test demonstrating Gemini 3 Pro with thinking_level
  2. Update README or docs with Gemini 3 Pro examples

📊 Code Changes Review

parameters.py:14,25 - Adding THINKING_LEVEL

✅ Correct addition to enum and dataclass

parameters.py:76-95 - ThinkingLevelMapper

✅ Well-implemented following established patterns
⚠️ Needs tests

models.py:52-63 - Gemini 3 Pro Model Definition

✅ Correct constraint configuration
✅ Proper use of Choice constraint
⚠️ Needs integration tests

models.py:16,29,44 - Output Token Updates

✅ Consistent updates across all Gemini 2.5 models
❓ Need verification against official docs

🎬 Conclusion

This is a solid implementation that follows the codebase patterns well. The main gap is missing test coverage for the new functionality. Once tests are added and the documentation considerations are addressed, this will be ready to merge.

Suggested Action: Add tests for the new ThinkingLevelMapper and consider adding a basic integration test for Gemini 3 Pro before merging.


Review Confidence: High - familiar with the codebase patterns and constraint system.

@Kamilbenkirane Kamilbenkirane merged commit cc9b712 into main Nov 19, 2025
11 checks passed
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.

1 participant