-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: Add support for Google Gemini embedding model gemini-embedding-exp-03-07 #7808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…exp-03-07 - Add gemini-embedding-exp-03-07 model variants with flexible dimensions (3072, 1536, 768) - Update GeminiEmbedder to support outputDimension parameter - Update OpenAICompatibleEmbedder to handle outputDimension in API calls - Set gemini-embedding-exp-03-07-3072 as new default model for Gemini provider - Preserve backward compatibility with existing text-embedding-004 model - Update tests to cover new model and outputDimension functionality Fixes #5621
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| * Supported models: | ||
| * - text-embedding-004 (dimension: 768) | ||
| * - gemini-embedding-001 (dimension: 2048) | ||
| * - gemini-embedding-001 (dimension: 3072) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? The comment says gemini-embedding-001 (dimension: 3072) but on line 44 we pass GEMINI_MAX_ITEM_TOKENS (2048) to the constructor. These seem to represent different concepts (embedding dimension vs max tokens), but it might be worth clarifying in a comment to avoid confusion.
| * @param outputDimension Optional output dimension for flexible models | ||
| */ | ||
| constructor(apiKey: string, modelId?: string) { | ||
| constructor(apiKey: string, modelId?: string, outputDimension?: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we validate that the outputDimension parameter matches one of the supported dimensions for the selected model? For example, if someone uses gemini-embedding-exp-03-07-3072 with outputDimension: 768, this could lead to unexpected behavior.
| constructor(apiKey: string, modelId?: string, outputDimension?: number) { | |
| constructor(apiKey: string, modelId?: string, outputDimension?: number) { | |
| if (!apiKey) { | |
| throw new Error(t("embeddings:validation.apiKeyRequired")) | |
| } | |
| // Use provided model or default | |
| this.modelId = modelId || GeminiEmbedder.DEFAULT_MODEL | |
| // Validate outputDimension if provided | |
| if (outputDimension && this.modelId.includes('gemini-embedding-exp-03-07')) { | |
| const validDimensions = [3072, 1536, 768] | |
| if (!validDimensions.includes(outputDimension)) { | |
| throw new Error(`Invalid outputDimension ${outputDimension}. Must be one of: ${validDimensions.join(', ')}`) | |
| } | |
| } |
| ) | ||
| }) | ||
|
|
||
| it("should create an instance with specified model and outputDimension", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test case for dimension mismatch scenarios? For example, what happens when someone specifies gemini-embedding-exp-03-07-3072 as the model but provides outputDimension: 768? This would help ensure the behavior is well-defined.
| gemini: { | ||
| "text-embedding-004": { dimension: 768 }, | ||
| "gemini-embedding-001": { dimension: 3072, scoreThreshold: 0.4 }, | ||
| "gemini-embedding-exp-03-07-3072": { dimension: 3072, scoreThreshold: 0.4 }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new models follow a pattern where the dimension is part of the model name (e.g., gemini-embedding-exp-03-07-3072). Should we consider extracting and validating this dimension from the model name to ensure consistency with any provided outputDimension parameter?
This PR attempts to address Issue #5621 by implementing support for Google's latest model with flexible output dimensions.
Changes Made
Acceptance Criteria Verification
Given a user wants to configure codebase indexing with the latest Google model
When they navigate to the "Roo-Code: Codebase Indexing Settings"
And select "Gemini" as the Embedder Provider
Then the "Select a model" dropdown should list with options for 3072, 1536, and 768 dimensions.
And the user can select any of these options and save the settings without errors.
But the existing model must remain available and fully functional.
Technical Implementation
Files Modified
Feedback and guidance are welcome!
Important
Adds support for Google Gemini model
gemini-embedding-exp-03-07with flexible dimensions and updates related classes and tests.gemini-embedding-exp-03-07with dimensions 3072, 1536, and 768 ingemini.ts.GeminiEmbedderconstructor to acceptoutputDimensionparameter.OpenAICompatibleEmbedderto handleoutputDimensionin API calls.gemini-embedding-exp-03-07-3072.gemini.spec.tsto cover new model andoutputDimensionfunctionality.embeddingModels.tsforgemini-embedding-exp-03-07-3072,gemini-embedding-exp-03-07-1536, andgemini-embedding-exp-03-07-768.This description was created by
for c0f88e0. You can customize this summary. It will automatically update as commits are pushed.