fix(workspace): thread document path through search results - #503
Conversation
Memory search results were showing chunk UUIDs instead of source file paths. Thread document_path through RankedResult, SearchResult, and the RRF fusion pipeline so handlers can display the actual file path. Fixes nearai#481 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where memory search results were linking to chunk UUIDs instead of human-readable source file paths. The changes involve extending core data structures to include document paths, updating database queries across different backends to retrieve these paths, and ensuring they are correctly propagated and displayed in both the web UI and tool outputs. This significantly improves the usability and clarity of search results by providing direct file path references. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces and propagates a document_path field throughout the memory search and retrieval system. The changes involve adding document_path to SearchResult and RankedResult structs, modifying SQL queries in libsql/workspace.rs and workspace/repository.rs to select the document path, and updating result parsing to populate this new field. The document_path is then utilized in the memory_search_handler, included in the JSON output of the MemorySearchTool, and integrated into the reciprocal_rank_fusion logic. The reviewer suggests performance optimizations by using .into_iter() instead of .iter() in src/channels/web/handlers/memory.rs and src/tools/builtin/memory.rs to avoid unnecessary cloning of document_path and content strings when the original results collection is consumed.
| .iter() | ||
| .map(|r| SearchHit { | ||
| path: r.document_id.to_string(), | ||
| path: r.document_path.clone(), | ||
| content: r.content.clone(), | ||
| score: r.score as f64, | ||
| }) |
There was a problem hiding this comment.
To improve performance, you can avoid cloning document_path and content. Since results is not used after this, you can use .into_iter() to consume it and move the string values into SearchHit.
| .iter() | |
| .map(|r| SearchHit { | |
| path: r.document_id.to_string(), | |
| path: r.document_path.clone(), | |
| content: r.content.clone(), | |
| score: r.score as f64, | |
| }) | |
| .into_iter() | |
| .map(|r| SearchHit { | |
| path: r.document_path, | |
| content: r.content, | |
| score: r.score as f64, | |
| }) |
References
- To improve performance, avoid unnecessary heap allocations by using iterators directly to consume collections and move values, rather than cloning.
| @@ -100,6 +100,7 @@ impl Tool for MemorySearchTool { | |||
| "results": results.iter().map(|r| serde_json::json!({ | |||
There was a problem hiding this comment.
Since results is not used after this, you can use .into_iter() to consume the vector and move the string values (content and path) into the json! macro. This avoids unnecessary cloning and improves performance.
| "results": results.iter().map(|r| serde_json::json!({ | |
| "results": results.into_iter().map(|r| serde_json::json!({ |
References
- To improve performance, avoid unnecessary heap allocations by using iterators directly to consume collections and move values, rather than cloning.
Address review feedback: consume results with into_iter() to move String fields directly instead of cloning them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…509) * test(workspace): add regression test for document_path propagation through RRF Verifies that search results carry the source document's file path through the RRF fusion pipeline, not the document UUID. Covers the bug fixed in PR #503 / issue #481. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update src/workspace/search.rs Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * chore: merge main and fix formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> [skip-regression-check] --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* fix(workspace): thread document path through search results Memory search results were showing chunk UUIDs instead of source file paths. Thread document_path through RankedResult, SearchResult, and the RRF fusion pipeline so handlers can display the actual file path. Fixes nearai#481 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: use into_iter to move values instead of cloning Address review feedback: consume results with into_iter() to move String fields directly instead of cloning them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…earai#509) * test(workspace): add regression test for document_path propagation through RRF Verifies that search results carry the source document's file path through the RRF fusion pipeline, not the document UUID. Covers the bug fixed in PR nearai#503 / issue nearai#481. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update src/workspace/search.rs Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * chore: merge main and fix formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> [skip-regression-check] --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…earai#509) * test(workspace): add regression test for document_path propagation through RRF Verifies that search results carry the source document's file path through the RRF fusion pipeline, not the document UUID. Covers the bug fixed in PR nearai#503 / issue nearai#481. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update src/workspace/search.rs Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * chore: merge main and fix formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> [skip-regression-check] --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Summary
Fixes #481 - Memory search results link to chunk UUID instead of source file path.
document_path: StringtoRankedResultandSearchResultinsearch.rsrepository.rsto SELECTd.path as document_path(JOIN already exists)document_paththroughChunkInfodb/libsql/workspace.rssimilarlyhandlers/memory.rsto user.document_path.clone()instead ofr.document_id.to_string()document_pathto memory tool search output inbuiltin/memory.rsTest plan
cargo test workspace::search::teststo confirm RRF unit tests pass--features libsql) to verify parityCo-Authored-By: Claude Opus 4.6 noreply@anthropic.com