Skip to content

feat(embedding): add native Google Gemini embedding driver#1052

Closed
minhnvc wants to merge 1 commit intoRightNow-AI:mainfrom
minhnvc:fix/gemini-embedding-cleanup
Closed

feat(embedding): add native Google Gemini embedding driver#1052
minhnvc wants to merge 1 commit intoRightNow-AI:mainfrom
minhnvc:fix/gemini-embedding-cleanup

Conversation

@minhnvc
Copy link
Copy Markdown

@minhnvc minhnvc commented Apr 14, 2026

Summary

  • Adds GeminiEmbeddingDriver using the v1beta batchEmbedContents API (not OpenAI-compatible — uses x-goog-api-key header)
  • Adds gemini-embedding-001 (3072 dims) and text-embedding-004 (768 dims) to infer_dimensions
  • Extracts warn_external_api() helper to deduplicate the locality-check warning that was copy-pasted between the Gemini and OpenAI-compat branches of create_embedding_driver
  • Promotes batch size to module-level GEMINI_BATCH_LIMIT const with doc comment
  • Hoists URL construction outside the chunk loop to avoid redundant allocations

Config

[memory]
embedding_provider = "gemini"
embedding_model = "gemini-embedding-001"
embedding_api_key_env = "GEMINI_API_KEY"

Test plan

  • cargo build --workspace --lib — clean compile, 0 new warnings
  • test_create_embedding_driver_gemini_requires_api_key — missing key returns MissingApiKey
  • Live test: start daemon with GEMINI_API_KEY set, send agent message, verify no embedding warning in logs

🤖 Generated with Claude Code

@jaberjaber23
Copy link
Copy Markdown
Member

Thanks @minhnvc — the Gemini embedding driver is something we want, but after comparing with #997 (which landed slightly earlier as a parallel effort) we're going to land #997 as the canonical implementation because it adds:

  • batch → single embedContent fallback (Gemini's batchEmbedContents isn't available in every region/plan)
  • GEMINI_API_KEY / GOOGLE_API_KEY env both accepted
  • google alias alongside gemini in create_embedding_driver

Your implementation is cleaner and smaller, and the GEMINI_BATCH_LIMIT = 100 constant + x-goog-api-key header + warn_external_api() helper are all nice touches. If you'd be willing to submit a follow-up PR adding those specific pieces on top of #997 (once it lands), that would be great.

Closing in favor of #997 — not because your code is wrong, just to avoid duplicate review rounds on two nearly-identical drivers. Really appreciate the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants