Skip to content

Conversation

@Jopo-JP
Copy link

@Jopo-JP Jopo-JP commented Jul 13, 2025

Related GitHub Issue

Closes: #5621

Description

This PR introduces support for the new Gemini embedding model, gemini-embedding-exp-03-07, and its flexible output dimensions.

The key changes include:

  • Updating embeddingModels.ts to include the new model.
  • Enhancing OpenAICompatibleEmbedder to accept and process a dimension parameter in API requests.
  • Modifying GeminiEmbedder to pass the model and dimension options to the underlying embedder.
  • Adding a null-safety check in webviewMessageHandler.ts to prevent potential TypeError exceptions if codeIndexManager is not initialized.

Test Procedure

  • Unit tests for GeminiEmbedder in gemini.spec.ts have been updated to verify that the model and dimension parameters are correctly passed down.
  • Existing tests were adapted to the new method signatures.
  • All related tests were confirmed to pass after the changes.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required at this time. User-facing configuration for embedding dimensions isn`t implemented.

Additional Notes

This implementation provides the core backend support for variable embedding dimensions. The UI components and VS Code settings for users to select a dimension are not included in this PR and might be included in another PR.

The default dimension is currently hardcoded to 768.

image

Help to get the configuration site working is welcome.


Important

Introduces Gemini embedding model with flexible dimensions, updates GeminiEmbedder and OpenAICompatibleEmbedder to handle dimensions, and adds null-safety check in webviewMessageHandler.ts.

  • Behavior:
    • Adds support for gemini-embedding-exp-03-07 model with flexible dimensions in embeddingModels.ts.
    • Updates GeminiEmbedder in gemini.ts to handle dimension parameter and pass it to OpenAICompatibleEmbedder.
    • Adds null-safety check for codeIndexManager in webviewMessageHandler.ts.
  • Tests:
    • Updates gemini.spec.ts to test model and dimension parameters in createEmbeddings().
  • Misc:
    • Updates IEmbedder interface in embedder.ts to include dimension option in createEmbeddings().

This description was created by Ellipsis for 9e9ab05. You can customize this summary. It will automatically update as commits are pushed.

/**
* Creates embeddings for the given texts using Gemini's embedding API
* @param texts Array of text strings to embed
* @param model Optional model identifier (ignored - always uses text-embedding-004)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the JSDoc for the 'model' parameter – it now honors a provided value (using model || GEMINI_MODEL) instead of always ignoring it.

Suggested change
* @param model Optional model identifier (ignored - always uses text-embedding-004)
* @param model Optional model identifier (uses provided value or defaults to text-embedding-004)

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jul 13, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 13, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 13, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 13, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Jopo-JP for the contribution!

I left a couple of suggestions worth taking a look at. Let me know if you have any questions!

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 14, 2025
@Jopo-JP
Copy link
Author

Jopo-JP commented Jul 15, 2025

I noticed a UI Bug and will be fixing it.
New GIF

@Jopo-JP
Copy link
Author

Jopo-JP commented Jul 15, 2025

Hi @daniel-lxs,

Thank you for your valuable feedback! I've incorporated the changes you suggested.
Google deprecated gemini-embedding-exp-03-07 and released the non-experimental gemini-embedding-001. I saw that you already did the groundwork for this in PR #5698 by including the new model, which was a great help.
Building on that, I've now implemented a slider in the UI to allow users to dynamically select the embedding dimension, which addresses your point about the hardcoded values.

gemini-embedding-001 does support flexible dimension between 128–3072 as Qdran does.

The unrelated null-safety check has also been removed as requested.

Let me know what you think of the new changes!

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 15, 2025
Replaces the dimension slider with a text input for better precision, addressing reviewer feedback. Adds validation for the dimension range and corrects all related UI text and translations.
@Jopo-JP
Copy link
Author

Jopo-JP commented Jul 15, 2025

Hi @daniel-lxs,
Thank you for the suggestion! You were right, the slider was not ideal for selecting precise dimension values.

My original intention with the slider was to ensure users could only select a value within the valid range. However, I agree that a text input is more practical and consistent with the rest of the UI.

I've now replaced the slider with a standard text input field. I've also added validation to display an error message if the entered dimension is outside the model's supported range, as you can see in the screenshot below.

Translation in other langauge is missing for the new strings.

At the moment, tha minDimension and maxDimension are displayed in the UI, as you can see in the screenshot. So the user knows the range.

image image

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 27, 2025
@daniel-lxs
Copy link
Member

Hey @Jopo-JP, thank you for addressing the review suggestions!

I noticed some unrelated changes to .github/workflows/code-qa.yml. This is a critical CI file, so it would be best to keep them as it was.

Also, I saw a few missing translations. Let me know if you need help with those. Thank you!

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 28, 2025
I removed it as i run github runners locally
@Jopo-JP
Copy link
Author

Jopo-JP commented Jul 28, 2025

Hey @Jopo-JP, thank you for addressing the review suggestions!

I noticed some unrelated changes to .github/workflows/code-qa.yml. This is a critical CI file, so it would be best to keep them as it was.

Also, I saw a few missing translations. Let me know if you need help with those. Thank you!

@daniel-lxs Thanks for the response.

Yeah, i changed the workflow so i can run it local as i use self-hosted runners.

i reverte the file to the original state.

Sorry for that.

Regarding, translation, where are the translation from?
I can for sure do the german one, but i don's speak spanish and so one.

Have a nice day!
Nikolas

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 29, 2025
@daniel-lxs
Copy link
Member

Hey @Jopo-JP I think is ready to go, can you resolve the conflicts?

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 29, 2025
@Jopo-JP
Copy link
Author

Jopo-JP commented Jul 30, 2025

Hi @daniel-lxs,
i resolved the merge conflict and the test passed, except the translation one.

Have a nice evening.
Nikolas

@daniel-lxs
Copy link
Member

Hey @Jopo-JP I am unable to submit changes to your branch to help you create the missing translations

@Jopo-JP
Copy link
Author

Jopo-JP commented Jul 31, 2025

Hey @Jopo-JP I am unable to submit changes to your branch to help you create the missing translations

Thanks for the response @daniel-lxs

is there something need to be configured on my side?

Nikolas

@Jopo-JP Jopo-JP closed this Aug 21, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 21, 2025
@github-project-automation github-project-automation bot moved this from PR [Changes Requested] to Done in Roo Code Roadmap Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request PR - Changes Requested size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

feat: Add support for Google Gemini embedding model

3 participants