feat(embeddings): port provider implementations#58
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR expands embedding support with Cloud, Cohere, Ollama, Voyage, and no-op providers. It adds stable embedding identities, shared rate limiting and retry handling, stricter OpenAI-compatible validation, public exports, tests, and dependency updates. ChangesEmbedding provider expansion
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca47b5b20b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/harness/embeddings/cloud.rs (1)
71-78: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftReuse a shared client for cloud embedding requests.
CloudEmbeddingModel::embedrebuildsOpenAiEmbeddingModelon every call (src/harness/embeddings/cloud.rs:71-78), so the underlyingreqwest::Clientand its connection pool are discarded each time. Add a path that keeps one client onCloudEmbeddingModeland only swaps the bearer token per request.🤖 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/harness/embeddings/cloud.rs` around lines 71 - 78, Update CloudEmbeddingModel and its embed method to retain and reuse a single underlying reqwest client across requests, while applying the current bearer token for each call. Avoid constructing a new OpenAiEmbeddingModel/client in every embed invocation, and preserve the existing base URL, model, dimensions, and API-key behavior.
🤖 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/harness/embeddings/mod.rs`:
- Around line 281-282: Make the request-control modules declared privately in
the embeddings module, then explicitly re-export only the documented host-facing
functions and constants from rate_limit and retry_after. Avoid exposing helper
functions such as acquire or parsing internals through public module paths,
while preserving the intended external API.
In `@src/harness/embeddings/rate_limit.rs`:
- Around line 18-25: Update the nested condition around BUCKETS in the
rate-limit reset logic to use a let-chain combining previous != per_minute with
the optional registry check. Preserve the existing lock, poison recovery, and
clear behavior.
---
Nitpick comments:
In `@src/harness/embeddings/cloud.rs`:
- Around line 71-78: Update CloudEmbeddingModel and its embed method to retain
and reuse a single underlying reqwest client across requests, while applying the
current bearer token for each call. Avoid constructing a new
OpenAiEmbeddingModel/client in every embed invocation, and preserve the existing
base URL, model, dimensions, and API-key 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: 2112dbd5-5a87-419c-9e1f-a78fcf5cae4f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlexamples/local_model_probe.rssrc/harness/embeddings/cloud.rssrc/harness/embeddings/cohere.rssrc/harness/embeddings/mod.rssrc/harness/embeddings/noop.rssrc/harness/embeddings/ollama.rssrc/harness/embeddings/openai.rssrc/harness/embeddings/rate_limit.rssrc/harness/embeddings/retry_after.rssrc/harness/embeddings/test.rssrc/harness/embeddings/types.rssrc/harness/embeddings/voyage.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f60981bdce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 777cd7600c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f75355727
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if live.is_empty() { | ||
| return Ok(vec![Vec::new(); texts.len()]); |
There was a problem hiding this comment.
When every input is blank, live is empty and this returns one zero-dimensional vector per input even though OllamaEmbeddingModel::dimensions() is 1024. That still violates the EmbeddingModel contract in types.rs and surfaces as a vector-store dimension error for Retriever::retrieve(" ", ...) against a nonempty Ollama index instead of rejecting invalid provider input; mixed blank batches now error, so all-blank batches should too or return valid 1024-dimensional placeholders.
Useful? React with 👍 / 👎.
| .with_dimensions(dimensions) | ||
| .with_base_url(base_url) | ||
| .with_send_dimensions(false) |
There was a problem hiding this comment.
When callers use VoyageEmbeddingModel::with_options(key, "voyage-3-large", 512, ...), the wrapper records and later validates 512 dimensions, but this path sends no dimension request to Voyage. Voyage's API uses output_dimension for supported lower dimensions and otherwise returns the model default (1024 for voyage-3-large; see https://docs.voyageai.com/reference/embeddings-api), so every non-default dimension call fails with a dimension mismatch instead of producing the requested vector size.
Useful? React with 👍 / 👎.
| async fn embed(&self, texts: &[String]) -> Result<Vec<Vec<f32>>> { | ||
| self.inner.embed(texts).await |
There was a problem hiding this comment.
For Voyage retrievers, Retriever::retrieve now calls embed_query, but VoyageEmbeddingModel does not override it, so the default query path calls this same document embed method and the OpenAI-shaped body has neither input_type: "query" nor input_type: "document". Voyage recommends those values for search and prepends different retrieval prompts for each (https://docs.voyageai.com/reference/embeddings-api), so Voyage-backed retrieval loses the provider's query/document tuning even though the trait now supports asymmetric providers.
Useful? React with 👍 / 👎.
1 participant
Summary
Port the OpenHuman embedding provider implementations into tinyagents. This adds native Voyage, Cohere, Ollama, cloud, and no-op models, shared retry/rate-limit behavior, and stable provider/model/signature identity on
EmbeddingModel.API Or Behavior Changes
EmbeddingModelnow exposesname,model_id, and a stablesignature. OpenAI preserves blank batch positions and provider implementations validate configuration before network calls.Tests
cargo fmt --checkcargo clippy --all-targets -- -D warnings- not run locallycargo clippy --all-targets --all-features -- -D warnings- not run locallycargo build --all-targets- not run separatelycargo build --all-targets --all-features- not run separatelycargo test- focused suite run insteadcargo test --all-features- not run locallyFocused validation:
cargo test --lib harness::embeddings(35 passed).Documentation
Public APIs and provider behavior are documented inline; no separate docs page exists for this module.
Summary by CodeRabbit
New Features
Bug Fixes