Feat/gemma 4 models#1507
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a ChangesGemma-4 Model Integration
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/models/gemini.ts`:
- Around line 26-38: The gemmaModel function currently sets classification
without 'vision' while it uses geminiCapabilities that indicate supportsVision:
true; update gemmaModel (the function named gemmaModel) so its classification
array includes 'vision' (e.g., ['chat','reasoning','coding','vision']) or
otherwise split/adjust the capability object (geminiCapabilities) if you intend
to disable vision for certain gemma-4-* models; ensure the change is made where
defineModel is invoked so model metadata and geminiCapabilities remain
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8f65351-2087-4c9a-9048-17c44713757f
📒 Files selected for processing (3)
src/integrations/brands/gemini.tssrc/integrations/models/gemini.tssrc/integrations/vendors/gemini.ts
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Use the documented Gemma API model IDs in the Gemini catalog
src/integrations/vendors/gemini.ts:39
The catalog now exposesgemma-4-e2b,gemma-4-e4b,gemma-4-12b,gemma-4-26b-a4b, andgemma-4-31bas GeminiapiNames, but the Gemini API docs show hosted Gemma calls using instruction-tuned IDs such asgemma-4-26b-a4b-it, and Google's launch material only points Gemini API / AI Studio access at the hosted 26B MoE and 31B models. Since this route sends the catalogapiNameas the model name, selecting these new rows would call non-documented model IDs likegemma-4-31band also advertise variants that are not shown as hosted Gemini API choices. Please update the catalog/model/brand IDs to the actual Gemini API model names and only expose variants that this route can call successfully. -
[P2] Complete CodeRabbit's request to keep Gemma vision metadata consistent
src/integrations/models/gemini.ts:32
CodeRabbit's inline review request is still valid:gemmaModelomitsvisionfromclassificationwhile reusinggeminiCapabilities, which setssupportsVision: true. Google's Gemma 4 model card lists image input/image understanding for the family, so the model metadata currently disagrees with both the capability flags and the documented model behavior. Please complete that review request by adding thevisionclassification for supported Gemma entries, or split the capabilities if any entry should truly be text-only.
|
@jatmn Thanks for the feedback! Both issues have been addressed: [P1]: I updated the catalog, brand, and model definitions to only include the hosted models and appended the correct -it suffix (gemma-4-26b-a4b-it and gemma-4-31b-it). The smaller edge variants (E2B, E4B, 12B) have been removed since they aren't accessible via this route. |
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the changed paths and found one small drift item that still needs to be addressed.
Findings
- [P3] Update the PR description to match the hosted Gemma entries
PR description
The implementation now only exposesgemma-4-26b-a4b-itandgemma-4-31b-it, which addresses the earlier request to avoid advertising unsupported Gemma variants through the Gemini route. However, the PR description still says this addsgemma-4-e2b,gemma-4-e4b,gemma-4-12b,gemma-4-26b-a4b, andgemma-4-31b, mentions both 128K and 256K architectures, and lists E2B/E4B as possible follow-up tuning targets. Please update the PR body so the public summary and release-note context do not still advertise the removed, unsupported model variants.
|
@jatmn Good catch! I've updated the PR description to remove all references to the unsupported edge models and the 128K context window. It now accurately reflects that only the hosted gemma-4-26b-a4b-it and gemma-4-31b-it models (with 256K context) are included in this PR. |
|
@coderabbitai please do full review and approve |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== src/integrations/brands/gemini.ts ==="
cat src/integrations/brands/gemini.ts
echo ""
echo "=== src/integrations/models/gemini.ts ==="
cat src/integrations/models/gemini.ts
echo ""
echo "=== src/integrations/vendors/gemini.ts ==="
cat src/integrations/vendors/gemini.tsLength of output: 4214
Minor cosmetic note (non-blocking): Everything looks solid. Approving. ✅ Action performedFull review finished. |
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
Summary
Impact
Testing
Notes
Summary by CodeRabbit