Skip to content

fix(embedding): raise Gemini max_embedding_tokens cap from 2048 to 8192#925

Open
Stromweld wants to merge 4 commits into
plastic-labs:mainfrom
Stromweld:fix/gemini-embedding-token-limit
Open

fix(embedding): raise Gemini max_embedding_tokens cap from 2048 to 8192#925
Stromweld wants to merge 4 commits into
plastic-labs:mainfrom
Stromweld:fix/gemini-embedding-token-limit

Conversation

@Stromweld

@Stromweld Stromweld commented Jul 22, 2026

Copy link
Copy Markdown

Summary

Gemini embedding models support up to 8192 input tokens, not 2048. The code had an artificial min(max_input_tokens, 2048) cap that unnecessarily truncated/chunked text for Gemini-backed embedding configs even when the caller-configured max_input_tokens was higher.

Fix

Changed the Gemini-specific cap in _EmbeddingClient.__init__ from 2048 to 8192, matching Gemini's actual documented per-request token limit. This is a provider-specific fix (only affects the `gemini" transport branch); the OpenAI branch is untouched.

Tests

Added test_gemini_embedding_client_honors_8192_token_cap in tests/llm/test_embedding_client.py, verifying:

  • max_input_tokens above the provider cap is clamped to 8192 (previously would have been clamped to 2048)
  • max_input_tokens below the cap passes through unchanged

Full targeted test run (all passing):

17 passed, 17 warnings in 8.95s

Created with AI assistance — Hermes Agent v0.17.0, model Claude Sonnet 4.6.

Summary by CodeRabbit

  • Bug Fixes

    • Updated Gemini embedding token-limit handling to use model-specific input caps: 8,192 tokens for gemini-embedding-2, 2,048 tokens for gemini-embedding-001, with a conservative 2,048-token fallback for unknown model IDs.
    • This changes which inputs pass token-limit validation during embedding.
  • Tests

    • Added Gemini-specific tests verifying per-model clamping, unknown-model fallback behavior, and correct handling to avoid applying the higher cap to near-miss model IDs.

Gemini's embedding models actually support up to 8192 input tokens,
not 2048. The artificial 2048 cap was truncating/chunking text more
aggressively than necessary for Gemini-backed embedding configs.

Added a unit test asserting the client honors the 8192 cap (and still
respects a lower max_input_tokens when configured below the cap).

Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3f201400-e811-48dd-9ced-020e3b9fd321

📥 Commits

Reviewing files that changed from the base of the PR and between ef3a50b and fda1156.

📒 Files selected for processing (2)
  • src/embedding_client.py
  • tests/llm/test_embedding_client.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/embedding_client.py
  • tests/llm/test_embedding_client.py

Walkthrough

Gemini embedding token limits are selected by model name: 8192 for gemini-embedding-2, and 2048 for gemini-embedding-001 or unknown models. Tests cover clamping, pass-through, canonical names, and near-miss IDs.

Changes

Gemini embedding token cap

Layer / File(s) Summary
Token cap selection and validation
src/embedding_client.py, tests/llm/test_embedding_client.py
Selects model-specific Gemini token caps and tests supported models, unknown models, exact matching, clamping, and below-cap inputs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a bunny with tokens, hopping high,
Model caps now match the sky.
Eight thousand here, two thousand there,
Tests keep every limit fair.
Fluffy embeddings bloom with care!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: increasing the Gemini embedding token cap from 2048 to 8192.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@src/embedding_client.py`:
- Around line 194-195: Update the EmbeddingClient initialization around
self.max_embedding_tokens to select the Gemini input-token cap by self.model:
use 2,048 for gemini-embedding-001 and 8,192 for gemini-embedding-2, while still
respecting max_input_tokens. In tests/llm/test_embedding_client.py lines 95-132,
use gemini-embedding-2 for the 8,192-token case or change the
gemini-embedding-001 expectation to 2,048.
🪄 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: 05cb4c26-1660-45e4-b0ae-43df406e44f6

📥 Commits

Reviewing files that changed from the base of the PR and between 4f9a413 and 6620a3c.

📒 Files selected for processing (2)
  • src/embedding_client.py
  • tests/llm/test_embedding_client.py

Comment thread src/embedding_client.py Outdated
…001: 2048, gemini-embedding-2: 8192)

Signed-off-by: Corey Hemminger <hemminger@hotmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/embedding_client.py (1)

199-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add an explicit type hint for the new cap variable.

Annotate gemini_model_token_cap as int to follow the repository’s Python typing requirement.

As per coding guidelines, Python code should use explicit type hints, including SQLAlchemy mapped_column annotations.

🤖 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/embedding_client.py` around lines 199 - 204, Annotate the
gemini_model_token_cap variable with an explicit int type in the model-capacity
conditional, preserving the existing values and branching behavior.

Source: Coding guidelines

🤖 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/embedding_client.py`:
- Around line 199-204: Update the model selection logic in the embedding
token-capacity path to match the known Gemini model ID exactly, rather than
using a substring check that also accepts names such as gemini-embedding-20.
Preserve the 8192-token cap only for gemini-embedding-2 and keep the
conservative 2048-token fallback for all other models, including
gemini-embedding-001.

---

Nitpick comments:
In `@src/embedding_client.py`:
- Around line 199-204: Annotate the gemini_model_token_cap variable with an
explicit int type in the model-capacity conditional, preserving the existing
values and branching behavior.
🪄 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: 23837dce-a6c0-4c6b-a8aa-560ef2830fe3

📥 Commits

Reviewing files that changed from the base of the PR and between 6620a3c and 0eec91f.

📒 Files selected for processing (2)
  • src/embedding_client.py
  • tests/llm/test_embedding_client.py

Comment thread src/embedding_client.py Outdated
…l cap

Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
Signed-off-by: Corey Hemminger <hemminger@hotmail.com>
@karaokedjodua

Copy link
Copy Markdown

Finn-loop review of fda1156

CI: not configured (no required checks reported)
Mergeability: clean (mergeable=MERGEABLE, mergeStateStatus=BLOCKED due to stale branch protection)

Review

Summary: Replaces the hardcoded 2048-token cap for Gemini embedding models with a model-specific cap: 8192 for gemini-embedding-2, 2048 for gemini-embedding-001 and unknown models. Five new tests cover all clamping paths including models/ prefix handling and near-miss model IDs.

No Paperclip issue linked — reviewed against PR body as contract.

1. Must fix before merge

  • [CI] mergeStateStatus: BLOCKED with mergeable: MERGEABLE and gh pr checks --required reporting "no required checks reported" — this is a stale branch-protection rule. The rule references a required check that no longer exists or doesn't match any running check. CodeRabbit passes but is not a required check. An admin must update or remove the stale branch-protection rule before this PR can be merged.

2. Should fix soon

None.

3. Safe to merge

No — blocked by the stale branch-protection rule above. Code changes themselves are correct and well-tested; the block is purely a repo-configuration issue.

Labels to apply

loop-changes-requested, needs-human-review

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