(MOT-4053) feat(provider-llamacpp): native embeddings surface - #513
Conversation
provider::llamacpp::embed serves batch embeddings from the configured llama-server (--embeddings plus an embedding-capable model), deriving the sibling /v1/embeddings from the chat api_url so nonstandard ports and path prefixes keep working. Credential only when the server runs with --api-key, matching the chat path. One-vector-per-input enforced (count and index contract); upstream errors name the --embeddings requirement. Registered internal like the other provider functions and denied to agents; router::embed discovers it via the registry walk. Live-verified: llama-server with a nomic-embed GGUF on a custom port, direct provider call and router::embed provider=llamacpp both return correct 768-dim vectors.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 44 skipped (no docs/).
Four for four. Nicely done. |
|
Warning Review limit reached
Next review available in: 12 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 (3)
📝 WalkthroughWalkthroughAdds ChangesLlama.cpp embeddings provider
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RouterEmbed as router::embed
participant LlamaCppEmbed as provider::llamacpp::embed
participant LlamaServer as llama-server
RouterEmbed->>LlamaCppEmbed: Submit batch text
LlamaCppEmbed->>LlamaCppEmbed: Resolve config and validate inputs
LlamaCppEmbed->>LlamaServer: POST /v1/embeddings
LlamaServer-->>LlamaCppEmbed: Return indexed vectors
LlamaCppEmbed-->>RouterEmbed: Return vectors in input order
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
provider-llamacpp/src/embed.rs (2)
112-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConditionally append the configuration hint based on status.
Unconditionally appending the hint
(llama-server needs --embeddings and an embedding-capable model)to all non-success HTTP responses might mislead users when debugging unrelated issues, such as authentication errors (401/403) or payload size limits (413).Consider appending this helpful hint only when the upstream server returns a
404 Not Foundor501 Not Implemented.🛠️ Proposed fix
- return Err(Error::Handler(format!( - "provider/upstream_status: {status}: {excerpt} \ - (llama-server needs --embeddings and an embedding-capable model)" - ))); + let hint = if status == reqwest::StatusCode::NOT_FOUND || status == reqwest::StatusCode::NOT_IMPLEMENTED { + " (llama-server needs --embeddings and an embedding-capable model)" + } else { + "" + }; + return Err(Error::Handler(format!( + "provider/upstream_status: {status}: {excerpt}{hint}" + )));🤖 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 `@provider-llamacpp/src/embed.rs` around lines 112 - 115, Update the upstream error construction in the embedding request handler so the llama-server configuration hint is appended only for HTTP 404 Not Found or 501 Not Implemented responses. Keep other non-success statuses, including authentication and payload errors, limited to their status and response excerpt.
94-97: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid allocating a JSON AST for large arrays.
Using
serde_json::json!movesreq.inputinto an intermediate JSON AST representation (Vec<Value>). For large inputs (up to 512 texts), this introduces unnecessary allocations and performance overhead.Consider defining an inline serializable struct to stream the data directly to the request body.
⚡ Proposed fix using a dedicated struct
+ #[derive(Serialize)] + struct UpstreamRequest { + model: String, + input: Vec<String>, + } + let mut request = http .post(embed_url(&cfg.api_url)) .timeout(std::time::Duration::from_secs(EMBED_TIMEOUT_SECS)) - .json(&serde_json::json!({ "model": model, "input": req.input })); + .json(&UpstreamRequest { + model, + input: req.input, + });🤖 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 `@provider-llamacpp/src/embed.rs` around lines 94 - 97, Replace the serde_json::json! payload in the request construction with a small inline serializable struct containing model and input fields, so req.input is serialized directly without building an intermediate Vec<Value>. Preserve the existing embed_url, timeout, and request payload field names.
🤖 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.
Nitpick comments:
In `@provider-llamacpp/src/embed.rs`:
- Around line 112-115: Update the upstream error construction in the embedding
request handler so the llama-server configuration hint is appended only for HTTP
404 Not Found or 501 Not Implemented responses. Keep other non-success statuses,
including authentication and payload errors, limited to their status and
response excerpt.
- Around line 94-97: Replace the serde_json::json! payload in the request
construction with a small inline serializable struct containing model and input
fields, so req.input is serialized directly without building an intermediate
Vec<Value>. Preserve the existing embed_url, timeout, and request payload field
names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc72d348-3ce2-4985-a617-384e72cfc6bf
⛔ Files ignored due to path filters (1)
provider-llamacpp/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
provider-llamacpp/README.mdprovider-llamacpp/iii-permissions.yamlprovider-llamacpp/src/embed.rsprovider-llamacpp/src/lib.rsprovider-llamacpp/src/register.rsprovider-llamacpp/src/surface.rsprovider-llamacpp/tests/schemas.rs
Embed was registered before on_router_ready while the catalog listed it last, breaking the registration-order lockstep the schema test documents (nothing asserted the actual order, so CI stayed green). The registration now matches the catalog. The published request schema also gains the 1..=512 input bounds the handler already enforces.
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Adds
provider::llamacpp::embed: batch text embeddings from the configured llama-server, which exposes an OpenAI-compatible/v1/embeddingswhen started with--embeddingsand an embedding-capable model. The endpoint derives from the configured chatapi_url(sibling path), the credential is optional to match the chat path (local servers usually run without--api-key), and the one-vector-per-input contract is enforced with count and index validation. Behindrouter::embed's registry walk this gives the memory worker fully local semantic recall.Live-verified on a rig: llama-server with a nomic-embed GGUF on a custom port;
provider::llamacpp::embeddirectly androuter::embed { provider: llamacpp }both returned correct 768-dim vectors, provider available in the registry. Wire schema golden committed; 67 tests green. The 3 integration tests that need an isolated engine fail identically on clean main on this host (environment) and are untouched.Fixes MOT-4053
Summary by CodeRabbit
New Features
Documentation
Security