fix(memory): clarify missing ollama embedding model#2533
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughOllama embedder now detects "model not found" 404 responses by inspecting response body text and emits a detailed error message with an ChangesOllama Embedder Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 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. 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 (1)
src/openhuman/memory/tree/score/embed/ollama.rs (1)
108-115: ⚡ Quick winConsider using the actual Ollama error JSON structure in tests.
The detection logic checks for "model" and "not found" as substrings, which is simple but effective. However, the test at line 323 uses a minimal JSON body
{"error":"model not found"}, while the actual Ollama response format (shown in context snippet fromsrc/openhuman/embeddings/ollama_tests.rs:330-357) is:{"error":"model \"bge-m3\" not found, try pulling it first"}Using the actual format in the test would better validate that the detection works with real-world responses.
🧪 Suggested test improvement
At line 323, use the actual Ollama response format:
- post(|| async { (StatusCode::NOT_FOUND, "{\"error\":\"model not found\"}") }), + post(|| async { (StatusCode::NOT_FOUND, r#"{"error":"model \"custom-embed\" not found, try pulling it first"}"#) }),🤖 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/openhuman/memory/tree/score/embed/ollama.rs` around lines 108 - 115, The detection function is_missing_model_response currently checks lowercase body substrings "model" and "not found"; update the corresponding test to use a realistic Ollama error JSON (e.g. {"error":"model \"bge-m3\" not found, try pulling it first"}) so the test exercises the real-world message format and verifies is_missing_model_response correctly detects missing-model responses from Ollama.
🤖 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 `@src/openhuman/memory/tree/score/embed/ollama.rs`:
- Around line 108-115: The detection function is_missing_model_response
currently checks lowercase body substrings "model" and "not found"; update the
corresponding test to use a realistic Ollama error JSON (e.g. {"error":"model
\"bge-m3\" not found, try pulling it first"}) so the test exercises the
real-world message format and verifies is_missing_model_response correctly
detects missing-model responses from Ollama.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ae5228d-b10b-42bd-84b7-d8fefe10fd08
📒 Files selected for processing (1)
src/openhuman/memory/tree/score/embed/ollama.rs
|
Current red check is CI infrastructure, not this Rust change.
|
|
@graycyrus @senamakel Required checks are green, the full check set is clean after the retry, and there are no unresolved review threads. Ready for review. |
2 participants
Summary
ollama pull <model>recovery command in the returned error.127.0.0.1:1always refuses connections.Problem
model not foundfor the embedding model.Solution
ollama pullcommand.Submission Checklist
## Related- N/A: no matrix feature ID.docs/RELEASE-MANUAL-SMOKE.md) - N/A: no release-cut surface.Closes #NNNin the## Relatedsection - N/A: partial diagnostic fix; issue remains broader.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/2474-ollama-embedding-guidanced6f7d64dValidation Run
pnpm --filter openhuman-app format:check- N/A: no frontend changes.pnpm typecheck- N/A: no TypeScript changes.GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml missing_model_404_mentions_pull_command --lib;GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml memory::tree::score::embed::ollama --libcargo fmt --manifest-path Cargo.toml --all --check;GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml;git diff --checkValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
status/bodyformat.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests