fix(gemini): honor custom model ids instead of falling back to default (#227)#317
Conversation
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
|
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 (2)
📝 WalkthroughWalkthroughGeminiHandler.getModel() now branches: use known geminiModels entry, accept unlisted ids starting with "gemini-" by preserving the id and clearing pricing/tier fields, or fall back to geminiDefaultModelId. Two unit tests cover a custom id and a prototype-key id. ChangesCustom Gemini Model ID Handling
sequenceDiagram
participant Client as GeminiHandler
participant Registry as geminiModels
participant Default as geminiDefaultModelId
Client->>Registry: lookup(apiModelId)
alt registry has model
Registry-->>Client: modelInfo (used)
else apiModelId starts with "gemini-"
Client->>Default: copy structure (clear pricing/tiers)
Default-->>Client: modelInfo (pricing cleared / undefined)
else fallback
Default-->>Client: default modelInfo
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/api/providers/__tests__/gemini.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/api/providers/gemini.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/api/providers/__tests__/gemini.spec.ts (1)
190-193: ⚡ Quick winAlso assert cache pricing fields are undefined for custom models.
This test already validates unknown core pricing. Add
cacheReadsPriceandcacheWritesPriceassertions too, since the implementation explicitly clears both.Suggested patch
expect(modelInfo.info.inputPrice).toBeUndefined() expect(modelInfo.info.outputPrice).toBeUndefined() + expect(modelInfo.info.cacheReadsPrice).toBeUndefined() + expect(modelInfo.info.cacheWritesPrice).toBeUndefined() expect(modelInfo.info.tiers).toBeUndefined()🤖 Prompt for 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. In `@src/api/providers/__tests__/gemini.spec.ts` around lines 190 - 193, The test in gemini.spec.ts that asserts unknown core pricing currently checks inputPrice, outputPrice, and tiers but misses the cache pricing fields; update the same test block (where modelInfo.info is asserted) to also assert that modelInfo.info.cacheReadsPrice and modelInfo.info.cacheWritesPrice are undefined by adding expectations for both properties alongside the existing input/output/tiers assertions.
🤖 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/api/providers/gemini.ts`:
- Around line 496-499: The current check uses `modelId && modelId in
geminiModels`, which can match prototype properties and lead to invalid `info`
lookup; update the conditional to use an own-property check (e.g.
`Object.hasOwn(geminiModels, modelId)` or
`Object.prototype.hasOwnProperty.call(geminiModels, modelId)`) so that when
setting `id = modelId` and `info = geminiModels[modelId as GeminiModelId]` you
only match real keys on the `geminiModels` map and avoid prototype-key
collisions.
---
Nitpick comments:
In `@src/api/providers/__tests__/gemini.spec.ts`:
- Around line 190-193: The test in gemini.spec.ts that asserts unknown core
pricing currently checks inputPrice, outputPrice, and tiers but misses the cache
pricing fields; update the same test block (where modelInfo.info is asserted) to
also assert that modelInfo.info.cacheReadsPrice and
modelInfo.info.cacheWritesPrice are undefined by adding expectations for both
properties alongside the existing input/output/tiers assertions.
🪄 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: 7dbdc206-4e39-4ec7-9cd0-3963e97b5cca
📒 Files selected for processing (2)
src/api/providers/__tests__/gemini.spec.tssrc/api/providers/gemini.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Zoo-Code-Org#227) Selecting a custom Gemini model id not present in geminiModels (e.g. gemini-3.5-flash) silently invoked geminiDefaultModelId. The settings ModelPicker exposes a 'use custom model' option and useSelectedModel keeps the configured id, so the UI and the actual request disagreed. getModel() now honors a custom id when it looks like a Gemini model (gemini- prefix), using the default model's structural info as a baseline with pricing/tiers dropped so cost shows as unknown rather than the default model's rates. Ids that don't look like Gemini still fall back to default.
…#227) modelId in geminiModels also matches inherited Object.prototype keys (e.g. "toString"), resolving info to a function. Use Object.hasOwn so only real model keys match; prototype-key ids fall through to the default.
proyectoauraorg
force-pushed
the
fix/227-gemini-custom-model
branch
from
7669c0f to
eaf7417
Compare
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Thanks,
there are other providers with the same problem. I wonder if this is worth doing for others?
2 participants
Closes #227
Problem
Selecting a custom Gemini model id that isn't in the hardcoded
geminiModelsmap (e.g.gemini-3.5-flash) silently invokesgemini-3.1-pro-previewinstead. The settings ModelPicker exposes a "use custom model" option anduseSelectedModelkeeps the configured id, butGeminiHandler.getModel()discarded any unknown id and fell back togeminiDefaultModelId— so the UI and the actual request disagreed.Fix
getModel()now honors a custom id when it looks like a Gemini model (gemini-prefix, case-insensitive), using the default model's structural info as a baseline. Pricing fields (inputPrice/outputPrice/cache/tiers) are dropped for unknown models so cost reporting shows "unknown" rather than charging the default model's rates. Ids that don't look like Gemini models (e.g.invalid-model) still fall back to the default, preserving existing behavior.Tests
Added a
getModelcase asserting a customgemini-3.5-flashid is honored (not swapped) and that pricing is left undefined. The existinginvalid-model→ default test is unchanged.tsc,eslint,prettier, and the gemini suite (20 tests) pass locally.Summary by CodeRabbit