fix: resolve vec0 chunk excerpts via batch content lookup #192 - #193
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
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 `@src/search/unified.ts`:
- Around line 371-374: The vec0 excerpt formatting in unified search is
exceeding the 80-character contract because it slices to 80 and then appends an
ellipsis. Update the excerpt logic in the unified search path around the
chunk.content truncation so the final returned string never exceeds 80
characters, including any suffix, while preserving the existing excerpt
behavior.
- Around line 334-343: Cap the vec0 post-lookup in searchVec0 to avoid
generating an oversized WHERE c.id IN (...) clause from a caller-provided
options.limit. Update the chunkIds/chunkRows lookup logic in unified.ts to
either clamp the effective limit to a safe ceiling before building placeholders
or batch the chunkIds into multiple queries, so db.query does not hit SQLite
host-parameter limits and fall back to an empty result set.
🪄 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: b3ccdeb8-c96d-4352-ab8d-2b1c1a787698
📒 Files selected for processing (2)
package.jsonsrc/search/unified.ts
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/search/unified.ts">
<violation number="1" location="src/search/unified.ts:342">
P2: Vec0 content lookup still ignores search filters. Apply `mergedFilters` to the vec0 chunk resolution path (or pass filters into `searchVec0`) so filtered searches do not return unrelated chunks.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| c.chunk_index, c.kind, d.path AS source_path, d.title AS doc_title | ||
| FROM chunks c | ||
| JOIN documents d ON d.id = c.document_id | ||
| WHERE c.id IN (${placeholders})`, |
There was a problem hiding this comment.
P2: Vec0 content lookup still ignores search filters. Apply mergedFilters to the vec0 chunk resolution path (or pass filters into searchVec0) so filtered searches do not return unrelated chunks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/search/unified.ts, line 342:
<comment>Vec0 content lookup still ignores search filters. Apply `mergedFilters` to the vec0 chunk resolution path (or pass filters into `searchVec0`) so filtered searches do not return unrelated chunks.</comment>
<file context>
@@ -328,13 +328,67 @@ async function searchVec0(
+ c.chunk_index, c.kind, d.path AS source_path, d.title AS doc_title
+ FROM chunks c
+ JOIN documents d ON d.id = c.document_id
+ WHERE c.id IN (${placeholders})`,
+ )
+ .all(...chunkIds) as Array<{
</file context>
|
@coderabbitai Fixed both actionable comments in 550e96d:
Issue #1 (clamp): Fixed in 550e96d — added Issue #2 (filters): Won't fix in this PR. This is a pre-existing architectural gap — |
@four-bytes-robby Looks good on both fronts. Excerpt clamp — Batch ID clamp — Filters — Your reasoning makes sense. Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
|
Tip For best results, initiate chat on the files or code changes.
If you want, I can also open a follow-up GitHub issue for the deferred vec0 filter propagation work so it’s tracked separately. ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/search/unified.ts">
<violation number="1" location="src/search/unified.ts:342">
P2: Vec0 content lookup still ignores search filters. Apply `mergedFilters` to the vec0 chunk resolution path (or pass filters into `searchVec0`) so filtered searches do not return unrelated chunks.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@cubic-dev-ai Good catch. Fixed in a5faf9e: changed |
@four-bytes-robby Looks correct. Both the batch query and the iteration now share the same Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
Closes #192
Problem
searchVec0()insrc/search/unified.tsqueried onlychunks_vec(chunk_id + distance) and returned placeholder excerpts like[vec0 chunk] distance=13.3280with opaquevec0:<uuid>titles. Users got no meaningful content from vec0 search results.Fix
After the KNN query returns
chunk_id+distance, a batchSELECT ... FROM chunks JOIN documents WHERE id IN (...)resolves all chunk IDs in one query. The results are mapped to aMap<chunk_id, chunk>for efficient lookup.searchChunksFallback)symbol ??${doc_title}:${chunk_type}#${chunk_index}`` (same as fallback path)Version
1.8.1 → 1.8.2 (patch bump)
Testing
bun run buildpasses cleanlysearchChunksFallbackand FTS5 resultsSummary by cubic
Fixes vec0 search results by resolving chunk IDs to real content via a batched lookup with safe limits. Results now show real titles, concise excerpts, and full metadata to match fallback and FTS5 results.
Written for commit a5faf9e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores