feat: many search quality improvements#15
Conversation
📝 WalkthroughWalkthroughSwitches ChangesCore embedding, search, and wiring
Eval config pruning, new dual-vector configs, and experiment reports
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over SearchTool,Qdrant: Query-time embedding and retrieval
end
participant SearchTool
participant embed_query
participant TextEmbedding_text as TextEmbedding (text)
participant TextEmbedding_code as TextEmbedding (code)
participant BM25Embedder
participant search
participant Qdrant
participant TextRerank
SearchTool->>embed_query: query string
embed_query->>TextEmbedding_text: lock + encode "query: ..."
TextEmbedding_text-->>embed_query: dense Vec<f32>
embed_query->>TextEmbedding_code: lock + encode "query: ..."
TextEmbedding_code-->>embed_query: code Vec<f32>
embed_query->>BM25Embedder: sparse encode
BM25Embedder-->>embed_query: SparseVector
embed_query-->>SearchTool: (dense, code, sparse)
SearchTool->>search: original_query, dense, code, sparse, is_code
search->>search: compute branch weights from is_code
search->>Qdrant: 3-branch prefetch (sparse/dense/code) + RRF fusion
Qdrant-->>search: candidate points
search->>TextRerank: original_query + candidate points
TextRerank->>Qdrant: scroll payloads for candidate docs
Qdrant-->>TextRerank: payloads
TextRerank->>TextRerank: rerank + group by file
TextRerank-->>search: top files × top chunks
search-->>SearchTool: SearchResults.processed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
evals/experiment-baseline/report.json (1)
21-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStrip workspace-specific absolute paths from all checked-in reports.
The shared root cause is that these JSON artifacts serialize canonical
/Users/clee/...paths intoretrieved_chunks/missed_chunks, which leaks local machine details and makes the reports non-portable.
evals/experiment-baseline/report.json#L21-L34: emit repo-relative paths before saving.evals/experiment-jina-split-embeddings-more-code-weight/report.json#L23-L34: emit repo-relative paths before saving.evals/experiment-jina-split-embeddings-with-reranking-top-2-files/report.json#L23-L34: emit repo-relative paths before saving.evals/experiment-jina-split-embeddings-with-reranking/report.json#L23-L34: emit repo-relative paths before saving.evals/experiment-jina-split-embeddings/report.json#L23-L34: emit repo-relative paths before saving.evals/experiment-overlap-nodes-20/report.json#L23-L34: emit repo-relative paths before saving.🤖 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 `@evals/experiment-baseline/report.json` around lines 21 - 34, The issue is that absolute workspace-specific paths (like /Users/clee/...) are being serialized into the retrieved_chunks and missed_chunks arrays across multiple report files, which makes the reports non-portable and machine-specific. Find the code that generates and saves these JSON report files and modify it to convert absolute paths to repo-relative paths before serialization. This change must be applied consistently across all affected report files: evals/experiment-baseline/report.json (lines 21-34), evals/experiment-jina-split-embeddings-more-code-weight/report.json (lines 23-34), evals/experiment-jina-split-embeddings-with-reranking-top-2-files/report.json (lines 23-34), evals/experiment-jina-split-embeddings-with-reranking/report.json (lines 23-34), evals/experiment-jina-split-embeddings/report.json (lines 23-34), and evals/experiment-overlap-nodes-20/report.json (lines 23-34). The fix should normalize all path serialization to use relative paths from the repository root rather than absolute local filesystem paths.
🧹 Nitpick comments (2)
microagents-cli/src/processing.rs (1)
173-196: ⚖️ Poor tradeoffHolding two mutex locks simultaneously in
embed()could cause lock contention.Both
text_embedding_model()andcode_embedding_model()locks are held for the entire duration of processing all chunks. If another thread needs either model, it will block until the entire batch completes. Consider embedding text and code in separate passes to reduce contention, or use a batch-level approach where locks are released between chunks.🤖 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 `@microagents-cli/src/processing.rs` around lines 173 - 196, The embed() function holds both the embedder_mu and code_embedder_mu mutex locks simultaneously throughout the entire loop over all chunks, causing lock contention for other threads. Refactor the function to use two separate passes over the chunks: first acquire embedder_mu, iterate through all chunks to compute and store dense embeddings, then release that lock; next acquire code_embedder_mu, iterate through all chunks again to compute and store code embeddings, then release that lock. The bm25 embedder (which is not mutex-protected) can remain in the original loop or be handled separately. This approach reduces contention by never holding both locks at the same time and by releasing locks between batch operations.evals/configs/experiment-jina-split-embeddings.json (1)
4-27: 💤 Low valueVector ordering inconsistency across jina configs.
This config defines vectors in the order
"code"then"text", whileexperiment-jina-split-embeddings-more-code-weight.jsonandexperiment-jina-split-embeddings-with-reranking.jsonuse"text"then"code". While JSON object ordering doesn't affect deserialization, this inconsistency suggests potential copy-paste variance and makes the configs harder to compare.🔄 Suggested standardization
"vectors": { - "code": { + "text": { "size": 768, "distance": "Cosine", "on_disk": true, "quantization_config": { "turbo": { "always_ram": true, "bits": "bits4" } } }, - "text": { + "code": { "size": 768, "distance": "Cosine", "on_disk": true, "quantization_config": { "turbo": { "always_ram": true, "bits": "bits4" } } } },🤖 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 `@evals/configs/experiment-jina-split-embeddings.json` around lines 4 - 27, The vectors in the "vectors" section are ordered with "code" first and "text" second, which is inconsistent with the ordering used in the other similar Jina configuration files that place "text" first and "code" second. Reorder the vector definitions in the "vectors" object so that the "text" vector definition appears before the "code" vector definition to standardize the structure and make the configs easier to compare.
🤖 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 `@evals/configs/experiment-jina-split-embeddings-more-code-weight.json`:
- Around line 62-64: The experiment config file is named with "more-code-weight"
intent but the code_weight value is 0.5, which is identical to the base variant.
Increase the code_weight value from 0.5 to a higher value (such as 0.6 or 0.7)
to differentiate this variant as intended, and correspondingly adjust the
dense_weight and sparse_weight values downward so that all three weights in
lines 62-64 maintain a reasonable total distribution.
In `@evals/configs/experiment-jina-split-embeddings.json`:
- Around line 49-50: There is a typo in the field names across three
jina-split-embeddings configuration files where "embedding" is misspelled as
"embeddng" (missing 'i'). This mismatch could break deserialization if the
parsing struct expects correctly spelled field names. In
evals/configs/experiment-jina-split-embeddings.json at lines 49-50, change
text_embeddng_model_name to text_embedding_model_name and
code_embeddng_model_name to code_embedding_model_name. Apply the same
corrections in
evals/configs/experiment-jina-split-embeddings-more-code-weight.json at lines
49-50 and in evals/configs/experiment-jina-split-embeddings-with-reranking.json
at lines 49-50.
In `@microagents-cli/src/processing.rs`:
- Around line 180-182: Fix the typo in the error message strings where "Mutext"
should be "Mutex". In the code_embedder_mu.lock().expect() call around line 180,
change the error message text from "Mutext on code embedding model has been
poisoned" to "Mutex on code embedding model has been poisoned". Apply the same
fix to the second occurrence of this typo in the similar mutex lock operation
around lines 205-207.
- Around line 188-190: The expect call on the code_embedder.embed method
contains an empty string message, which provides no useful diagnostic
information if the embedding fails. Replace the empty string in the expect("")
call with a descriptive error message that explains what operation failed
(embedding code content) and provides context about what code is being embedded
to aid debugging when the operation panics.
- Around line 50-57: The `fastembed_cache_dir()` function uses `.expect()` on
the `Option` returned by `dirs::home_dir()`, which will panic on systems where
the home directory cannot be determined. Instead of panicking, implement a
fallback strategy by checking if `dirs::home_dir()` returns `Some` and providing
an alternative path (such as the current working directory via
`std::env::current_dir()` or a temporary directory via `std::env::temp_dir()`)
when the home directory is unavailable. This ensures the function gracefully
handles headless or containerized environments without causing runtime panics.
In `@microagents-cli/src/search.rs`:
- Around line 236-241: In the Fusion::Rrf configuration where k is currently set
to top_k, replace the k parameter with a fixed smoothing constant instead. The k
parameter in RRF's scoring formula (score = sum of 1/(k + rank)) should be a
small constant like 60 for standard RRF or 2 for Qdrant defaults, not the top_k
result limit. Change k: top_k to k: 60 (or k: 2 if preferring Qdrant defaults)
to ensure correct fusion scoring behavior.
In `@search-evals/src/ingest.rs`:
- Around line 32-33: Fix the spelling errors in the JSON keys where the letter
'i' is missing from "embedding". Replace "text_embeddng_model_name" with
"text_embedding_model_name" and replace "code_embeddng_model_name" with
"code_embedding_model_name" to ensure the configuration keys are spelled
correctly and match what downstream code expects.
In `@search-evals/src/search.rs`:
- Line 13: The search function call has is_code hardcoded to true, which applies
code-boosted fusion weights to all queries regardless of their actual type. The
Question struct lacks an is_code field to specify query types, causing
inconsistency with SearchTool's default of false. Fix this by either adding an
is_code field to the Question struct and populating it from the evaluation
dataset, then using that value in the search call instead of the hardcoded true,
or by simply changing the hardcoded is_code parameter from true to false to
align with SearchTool's default behavior. Choose the first approach if the
evaluation dataset distinguishes between code and natural-language queries,
otherwise use the second approach for consistency.
---
Outside diff comments:
In `@evals/experiment-baseline/report.json`:
- Around line 21-34: The issue is that absolute workspace-specific paths (like
/Users/clee/...) are being serialized into the retrieved_chunks and
missed_chunks arrays across multiple report files, which makes the reports
non-portable and machine-specific. Find the code that generates and saves these
JSON report files and modify it to convert absolute paths to repo-relative paths
before serialization. This change must be applied consistently across all
affected report files: evals/experiment-baseline/report.json (lines 21-34),
evals/experiment-jina-split-embeddings-more-code-weight/report.json (lines
23-34),
evals/experiment-jina-split-embeddings-with-reranking-top-2-files/report.json
(lines 23-34), evals/experiment-jina-split-embeddings-with-reranking/report.json
(lines 23-34), evals/experiment-jina-split-embeddings/report.json (lines 23-34),
and evals/experiment-overlap-nodes-20/report.json (lines 23-34). The fix should
normalize all path serialization to use relative paths from the repository root
rather than absolute local filesystem paths.
---
Nitpick comments:
In `@evals/configs/experiment-jina-split-embeddings.json`:
- Around line 4-27: The vectors in the "vectors" section are ordered with "code"
first and "text" second, which is inconsistent with the ordering used in the
other similar Jina configuration files that place "text" first and "code"
second. Reorder the vector definitions in the "vectors" object so that the
"text" vector definition appears before the "code" vector definition to
standardize the structure and make the configs easier to compare.
In `@microagents-cli/src/processing.rs`:
- Around line 173-196: The embed() function holds both the embedder_mu and
code_embedder_mu mutex locks simultaneously throughout the entire loop over all
chunks, causing lock contention for other threads. Refactor the function to use
two separate passes over the chunks: first acquire embedder_mu, iterate through
all chunks to compute and store dense embeddings, then release that lock; next
acquire code_embedder_mu, iterate through all chunks again to compute and store
code embeddings, then release that lock. The bm25 embedder (which is not
mutex-protected) can remain in the original loop or be handled separately. This
approach reduces contention by never holding both locks at the same time and by
releasing locks between batch operations.
🪄 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 Plus
Run ID: a2a8846f-aca7-4a97-9c3d-79e41d86fff7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
evals/configs/experiment-baseline.jsonevals/configs/experiment-bm25-only-1000-chunks.jsonevals/configs/experiment-bm25-only-2000-chunks.jsonevals/configs/experiment-bm25-only-300-chunks.jsonevals/configs/experiment-bm25-only-rerank-10.jsonevals/configs/experiment-bm25-only.jsonevals/configs/experiment-hnsq-tuning.jsonevals/configs/experiment-hnsw-and-quantiz.jsonevals/configs/experiment-hnsw-config-full-hybrid-8MB.jsonevals/configs/experiment-hnsw-tuning.jsonevals/configs/experiment-hybrid-longer-bm25-1000.jsonevals/configs/experiment-hybrid-longer-bm25-normalize.jsonevals/configs/experiment-hybrid-longer-bm25.jsonevals/configs/experiment-jina-split-embeddings-more-code-weight.jsonevals/configs/experiment-jina-split-embeddings-with-reranking-top-2-files.jsonevals/configs/experiment-jina-split-embeddings-with-reranking.jsonevals/configs/experiment-jina-split-embeddings.jsonevals/configs/experiment-less-dense.jsonevals/configs/experiment-less-less-dense.jsonevals/configs/experiment-more-dense.jsonevals/configs/experiment-more-more-dense.jsonevals/configs/experiment-no-quantization-baseline-hybrid-8MB.jsonevals/configs/experiment-no-quantization-full-hybrid.jsonevals/configs/experiment-overlap-nodes-20.jsonevals/configs/experiment-overlap-nodes-30.jsonevals/configs/experiment-overlap-nodes-50.jsonevals/configs/experiment-prefetch-50.jsonevals/configs/experiment-product-quant-x4.jsonevals/configs/experiment-product-quant-x8.jsonevals/configs/experiment-real-bm25-only.jsonevals/configs/experiment-real-bm25-quarters.jsonevals/configs/experiment-tune-bm25-full-1000.jsonevals/configs/experiment-tune-bm25-full.jsonevals/configs/experiment-tune-bm25.jsonevals/experiment-baseline/report.jsonevals/experiment-jina-split-embeddings-more-code-weight/report.jsonevals/experiment-jina-split-embeddings-with-reranking-top-2-files/report.jsonevals/experiment-jina-split-embeddings-with-reranking/report.jsonevals/experiment-jina-split-embeddings/report.jsonevals/experiment-overlap-nodes-20/report.jsonevals/experiment-overlap-nodes-30/report.jsonevals/experiment-overlap-nodes-50/report.jsonevals/experiment-prefetch-50/report.jsonmicroagents-cli/Cargo.tomlmicroagents-cli/src/init_env.rsmicroagents-cli/src/processing.rsmicroagents-cli/src/search.rsmicroagents-cli/src/tools.rsmicroagents-cli/tests/integration_test.rssearch-evals/Cargo.tomlsearch-evals/src/ingest.rssearch-evals/src/search.rsv1/evals/experiment-baseline/report.jsonv1/evals/experiment-dense-only-1000-chunks/report.jsonv1/evals/experiment-dense-only-2000-chunks/report.jsonv1/evals/experiment-dense-only-300-chunks/report.jsonv1/evals/experiment-dense-only-rerank-10/report.jsonv1/evals/experiment-dense-only-smaller-chunks/report.jsonv1/evals/experiment-dense-only/report.jsonv1/evals/experiment-full-hybrid-1000-chunks/report.jsonv1/evals/experiment-hnsq-tuning/report.jsonv1/evals/experiment-hnsw-and-quantiz/report.jsonv1/evals/experiment-hnsw-config-full-hybrid-8MB/report.jsonv1/evals/experiment-hnsw-tuning-m-60/report.jsonv1/evals/experiment-hnsw-tuning/report.jsonv1/evals/experiment-hybrid-longer-bm25-1000/report.jsonv1/evals/experiment-hybrid-longer-bm25-normalize/report.jsonv1/evals/experiment-hybrid-longer-bm25/report.jsonv1/evals/experiment-less-dense/report.jsonv1/evals/experiment-less-less-dense/report.jsonv1/evals/experiment-more-dense/report.jsonv1/evals/experiment-more-more-dense/report.jsonv1/evals/experiment-no-quantization-baseline-hybrid-8MB/report.jsonv1/evals/experiment-no-quantization-full-hybrid-8MB/report.jsonv1/evals/experiment-no-quantization-full-hybrid/report.jsonv1/evals/experiment-product-quant-x4/report.jsonv1/evals/experiment-product-quant-x8/report.jsonv1/evals/experiment-real-bm25-only/report.jsonv1/evals/experiment-real-bm25-quarters/report.jsonv1/evals/experiment-tune-bm25-full-1000/report.jsonv1/evals/experiment-tune-bm25-full/report.jsonv1/evals/experiment-tune-bm25/report.json
💤 Files with no reviewable changes (25)
- evals/configs/experiment-tune-bm25-full-1000.json
- evals/configs/experiment-hnsq-tuning.json
- evals/configs/experiment-less-less-dense.json
- evals/configs/experiment-bm25-only.json
- evals/configs/experiment-hnsw-and-quantiz.json
- evals/configs/experiment-bm25-only-1000-chunks.json
- evals/configs/experiment-hybrid-longer-bm25.json
- evals/configs/experiment-tune-bm25.json
- evals/configs/experiment-hybrid-longer-bm25-normalize.json
- evals/configs/experiment-less-dense.json
- evals/configs/experiment-hnsw-config-full-hybrid-8MB.json
- evals/configs/experiment-more-dense.json
- evals/configs/experiment-hybrid-longer-bm25-1000.json
- evals/configs/experiment-hnsw-tuning.json
- evals/configs/experiment-tune-bm25-full.json
- evals/configs/experiment-no-quantization-baseline-hybrid-8MB.json
- evals/configs/experiment-product-quant-x4.json
- evals/configs/experiment-real-bm25-quarters.json
- evals/configs/experiment-bm25-only-rerank-10.json
- evals/configs/experiment-product-quant-x8.json
- evals/configs/experiment-real-bm25-only.json
- evals/configs/experiment-bm25-only-300-chunks.json
- evals/configs/experiment-bm25-only-2000-chunks.json
- evals/configs/experiment-no-quantization-full-hybrid.json
- evals/configs/experiment-more-more-dense.json
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
microagents-cli/src/init_env.rs (1)
526-529: 💤 Low valueVerbose log condition doesn't match the actual operation.
The condition checks
diff.deleted.is_empty()butdelete_filesis called withdiff.to_delete(), which includes both deleted and modified files. When files are modified (but none deleted), the deletion still occurs silently even withverbose=true.Consider using the same set for the check:
Suggested fix
- if verbose && !diff.deleted.is_empty() { + if verbose && !diff.to_delete().is_empty() { println!("Removing deleted files from vector index...") }🤖 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 `@microagents-cli/src/init_env.rs` around lines 526 - 529, The verbose condition at line 526-527 checks if diff.deleted is empty, but the delete_files function is called with diff.to_delete() which may include both deleted and modified files. Update the condition to check diff.to_delete().is_empty() instead of diff.deleted.is_empty() so the verbose log accurately reflects what files are actually being processed by the delete_files operation.
🤖 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 `@evals/configs/experiment-dbsf.json`:
- Around line 1-68: Add an explicit fusion strategy field (e.g., fusion: "dbsf")
to evals/configs/experiment-dbsf.json (lines 1-68) and ensure the search
execution code is wired to select DBSF instead of hardcoded RRF for this
experiment. In evals/configs/experiment-no-code-search.json (lines 61-65), add
an explicit mode flag enable_code_prefetch set to false and set code_weight to
zero to actually disable the code modality. In
evals/configs/experiment-top-10.json (lines 61-62), set rerank_top_n to 10 to
align the behavior with the experiment variant name.
In `@evals/experiment-correct-rrf/report.json`:
- Around line 25-35: The report files are serializing absolute file paths
starting with /Users/... instead of repo-relative paths, creating privacy and
reproducibility issues. In evals/experiment-correct-rrf/report.json at lines
25-35, evals/experiment-dbsf/report.json at lines 25-35, and
evals/experiment-no-code-search/report.json at lines 25-35, convert all chunk
paths in the retrieved_chunks and missed_chunks arrays from absolute paths
(e.g.,
/Users/clee/Desktop/code-projects/microagents/microagents-core/src/agent.rs:174-243)
to repo-relative paths (e.g., microagents-core/src/agent.rs:174-243). This fix
should be applied at the metrics generation source code level to ensure all
three report files emit consistent repo-relative paths instead of leaking local
machine paths.
In `@evals/experiment-top-10/report.json`:
- Around line 26-40: The report.json file contains absolute local filesystem
paths in the retrieved_chunks and missed_chunks arrays (starting with
/Users/clee/Desktop/...). These absolute paths need to be converted to
repository-relative paths to make the report portable and prevent leaking local
machine details. Regenerate the report using the proper evaluation process that
generates relative paths, or manually update all path entries in both the
retrieved_chunks and missed_chunks arrays to use paths relative to the
repository root (for example, changing
/Users/clee/Desktop/code-projects/microagents/microagents-core/src/agent.rs:174-243
to microagents-core/src/agent.rs:174-243 or similar repository-relative format).
---
Nitpick comments:
In `@microagents-cli/src/init_env.rs`:
- Around line 526-529: The verbose condition at line 526-527 checks if
diff.deleted is empty, but the delete_files function is called with
diff.to_delete() which may include both deleted and modified files. Update the
condition to check diff.to_delete().is_empty() instead of
diff.deleted.is_empty() so the verbose log accurately reflects what files are
actually being processed by the delete_files operation.
🪄 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 Plus
Run ID: f0a73e2e-7b72-4370-bf31-f08faf8c0962
⛔ Files ignored due to path filters (2)
.DS_Storeis excluded by!**/.DS_StoreCargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.gitignoreevals/configs/experiment-correct-rrf.jsonevals/configs/experiment-dbsf.jsonevals/configs/experiment-no-code-search.jsonevals/configs/experiment-top-10.jsonevals/configs/experiment-top-15.jsonevals/experiment-correct-rrf/report.jsonevals/experiment-dbsf/report.jsonevals/experiment-no-code-search/report.jsonevals/experiment-top-10/report.jsonevals/experiment-top-15/report.jsonmicroagents-cli/Cargo.tomlmicroagents-cli/src/init_env.rsmicroagents-cli/src/processing.rsmicroagents-cli/src/search.rssearch-evals/src/search.rs
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- evals/experiment-top-15/report.json
🚧 Files skipped from review as they are similar to previous changes (4)
- search-evals/src/search.rs
- microagents-cli/Cargo.toml
- microagents-cli/src/processing.rs
- microagents-cli/src/search.rs
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores