fix oai-compat embedding API#220
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughC++ now returns typed server task results and formats embedding outputs into OpenAI-compatible responses; TypeScript uses GlueMsgEmbeddingRes for embeddings; examples and tests read vectors from ChangesEmbedding Response Format Standardization
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@cpp/wllama-context.h`:
- Around line 183-189: In get_next_result(), avoid calling result->is_error()
after moving result; compute the boolean before the move (e.g., bool is_err =
result->is_error()) and then return {std::move(result), is_err}; reference the
server_task_result_ptr variable result and the is_error() call so the check
happens prior to std::move.
In `@src/wllama.test.ts`:
- Around line 183-185: The test accidentally reads the second embedding from the
first response; update the extraction to use res2 instead of res when building
embedding2 so the comparison uses two separate calls. Locate the second call to
wllama.createEmbedding (res2) and change the line that sets embedding2 to read
from res2.data[0].embedding (keeping the cast to number[]), ensuring the dot
product uses embedding and embedding2 as intended.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe04dd23-1921-45fe-ae66-01ff6bc653b4
⛔ Files ignored due to path filters (1)
src/wasm/wllama.wasmis excluded by!**/*.wasm
📒 Files selected for processing (5)
cpp/wllama-context.hexamples/basic/index.htmlexamples/embeddings/index.htmlsrc/wllama.test.tssrc/wllama.ts
Fix #219
Missing
format_embeddings_response_oaicompatto convert llama.cpp-specific response to OAI-compat responseSummary by CodeRabbit
Bug Fixes
Examples & Tests
Chores