Fix Gemini models list metadata#5000
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughPopulates Gemini ChangesGemini Model Metadata Population
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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
🤖 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 `@controller/model.go`:
- Around line 241-243: Replace the raw openAIModel.Id used for DisplayName with
the normalized ID so prefixed inputs are converted consistently: set DisplayName
to normalizeGeminiModelID(openAIModel.Id) (leave Name as
fmt.Sprintf("models/%s", normalizeGeminiModelID(openAIModel.Id))) so both fields
use the same normalized identifier; update the DisplayName assignment where the
struct is constructed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 213d9c7c-0394-42de-aea9-1c4babea008f
📒 Files selected for processing (3)
controller/model.gocontroller/model_list_test.godto/pricing.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/model.go (1)
248-257:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReverse the priority: check endpoint metadata first, use prefix-based detection as fallback.
The function checks model ID prefixes (lines 250–257) with early returns before consulting
SupportedEndpointTypes(lines 259–273). This contradicts the PR objective stating "prefer existing endpoint capability metadata where available; provide Gemini-native fallbacks."The metadata is computed from channel abilities and passed to the function, but prefix-based detection always takes priority, meaning metadata configurations are silently ignored. Reorder the checks to validate
SupportedEndpointTypesfirst, then fall back to prefix-based detection only if metadata is empty or absent.🤖 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 `@controller/model.go` around lines 248 - 257, The function getGeminiSupportedGenerationMethods currently prioritizes prefix-based detection (normalizeGeminiModelID + isGeminiVideoModel/isGeminiEmbeddingModel/isGeminiPredictModel) over endpoint metadata; change it to first inspect openAIModel.SupportedEndpointTypes (and use that to return the appropriate methods), and only if SupportedEndpointTypes is empty or nil fall back to the existing prefix checks via normalizeGeminiModelID and the isGemini* helpers; keep the same return values ("predictLongRunning", "embedContent", "batchEmbedContents", "predict") and ensure the function still returns nil when neither metadata nor prefix detection match.
🧹 Nitpick comments (2)
controller/model_list_test.go (2)
277-316: ⚡ Quick winConsider adding test coverage for SupportedEndpointTypes mapping.
This test validates the prefix-based detection path (embedding/predict models), but doesn't cover the
SupportedEndpointTypes→ generation methods mapping (lines 259-268 inmodel.go).Consider adding test cases for:
- A non-prefixed model with
SupportedEndpointTypes = [EndpointTypeGemini]should get["generateContent"]- A non-prefixed model with
SupportedEndpointTypes = [EndpointTypeEmbeddings]should get["embedContent", "batchEmbedContents"]- A non-prefixed model with empty endpoint types should get the fallback
["generateContent"]This would ensure the endpoint types logic is properly exercised.
🤖 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 `@controller/model_list_test.go` around lines 277 - 316, Add test cases exercising the SupportedEndpointTypes → generation methods mapping by extending TestListModelsGeminiIncludesSupportedGenerationMethods (or adding a new test) to create non-prefixed model entries (no "gemini-" prefix) and set their SupportedEndpointTypes on the model records: one with SupportedEndpointTypes = []string{model.EndpointTypeGemini} and assert geminiMethods(models["models/<name>"]) returns ["generateContent"], one with SupportedEndpointTypes = []string{model.EndpointTypeEmbeddings} and assert it returns ["embedContent","batchEmbedContents"], and one with SupportedEndpointTypes = []string{} (or nil) and assert the fallback ["generateContent"] is returned; use the same DB setup, model.InvalidatePricingCache/GetPricing flow and require.ElementsMatch assertions as in TestListModelsGeminiIncludesSupportedGenerationMethods to validate the mapping logic in model.go (SupportedEndpointTypes).
154-168: ⚡ Quick winConsider using
stringtype fordto.GeminiModel.Nameinstead ofinterface{}.The type assertion on line 163 (
name, ok := item.Name.(string)) confirms thatNameis currently aninterface{}field. Since model names from the Gemini API are always strings, changing this field tostringwould eliminate the runtime type assertion and provide compile-time type safety.🤖 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 `@controller/model_list_test.go` around lines 154 - 168, The test's runtime type assertion in decodeGeminiListModelsResponse shows dto.GeminiModel.Name is declared as interface{}; change the Gemini model struct so Name is a string (update dto.GeminiModel.Name from interface{} to string) to get compile-time safety, remove the type assertion in decodeGeminiListModelsResponse, and then update any call sites and JSON unmarshalling expectations that reference dto.GeminiModel.Name to accept a string.
🤖 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.
Outside diff comments:
In `@controller/model.go`:
- Around line 248-257: The function getGeminiSupportedGenerationMethods
currently prioritizes prefix-based detection (normalizeGeminiModelID +
isGeminiVideoModel/isGeminiEmbeddingModel/isGeminiPredictModel) over endpoint
metadata; change it to first inspect openAIModel.SupportedEndpointTypes (and use
that to return the appropriate methods), and only if SupportedEndpointTypes is
empty or nil fall back to the existing prefix checks via normalizeGeminiModelID
and the isGemini* helpers; keep the same return values ("predictLongRunning",
"embedContent", "batchEmbedContents", "predict") and ensure the function still
returns nil when neither metadata nor prefix detection match.
---
Nitpick comments:
In `@controller/model_list_test.go`:
- Around line 277-316: Add test cases exercising the SupportedEndpointTypes →
generation methods mapping by extending
TestListModelsGeminiIncludesSupportedGenerationMethods (or adding a new test) to
create non-prefixed model entries (no "gemini-" prefix) and set their
SupportedEndpointTypes on the model records: one with SupportedEndpointTypes =
[]string{model.EndpointTypeGemini} and assert
geminiMethods(models["models/<name>"]) returns ["generateContent"], one with
SupportedEndpointTypes = []string{model.EndpointTypeEmbeddings} and assert it
returns ["embedContent","batchEmbedContents"], and one with
SupportedEndpointTypes = []string{} (or nil) and assert the fallback
["generateContent"] is returned; use the same DB setup,
model.InvalidatePricingCache/GetPricing flow and require.ElementsMatch
assertions as in TestListModelsGeminiIncludesSupportedGenerationMethods to
validate the mapping logic in model.go (SupportedEndpointTypes).
- Around line 154-168: The test's runtime type assertion in
decodeGeminiListModelsResponse shows dto.GeminiModel.Name is declared as
interface{}; change the Gemini model struct so Name is a string (update
dto.GeminiModel.Name from interface{} to string) to get compile-time safety,
remove the type assertion in decodeGeminiListModelsResponse, and then update any
call sites and JSON unmarshalling expectations that reference
dto.GeminiModel.Name to accept a string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c8a0462-598d-475f-a3ce-927d2aa5ae70
📒 Files selected for processing (2)
controller/model.gocontroller/model_list_test.go
1 participant
Summary
GET /v1beta/modelsresponses with non-emptysupportedGenerationMethodsmodels/{id}shape while keepingdisplayNameas the plain model idFixes #3365.
Tests
~/go/bin/go1.25.1 test ./controller~/go/bin/go1.25.1 test ./routerSummary by CodeRabbit
New Features
Tests