Repository navigation
Use index for filename listing - #127
Conversation
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
tgrep-cli/src/serve.rs — The new files RPC reads this in-memory delta, but handle_reload only replaces state.index… |
|
tgrep-cli/src/search.rs — --ignore-file-case-insensitive also changes traversal membership, but this compatibility gate… |
|
tgrep-core/src/walker.rs — This similarly converts a metadata error into too_large == false, so a raced-away or… |
What changed in this PR
Routes --files through indexed filename data while preserving filesystem fallbacks.
Changes:
- Adds and maintains the
files-extra.binsidecar. - Adds server and local-index filename listing.
- Adds watcher synchronization, tests, and documentation.
| File | Description |
|---|---|
tgrep-core/src/walker.rs |
Collects listable paths. |
tgrep-core/src/path_index.rs |
Implements the filename sidecar. |
tgrep-core/src/ondisk.rs |
Documents sidecar integration. |
tgrep-core/src/lib.rs |
Exports the sidecar module. |
tgrep-core/src/hybrid.rs |
Exposes active indexed paths. |
tgrep-core/src/builder.rs |
Builds filename sidecars. |
tgrep-cli/src/search.rs |
Uses server/local filename indexes. |
tgrep-cli/src/serve.rs |
Maintains and serves filename state. |
tgrep-cli/src/main.rs |
Passes index and size-policy options. |
tgrep-cli/tests/indexed_files.rs |
Tests indexed filename listing. |
README.md |
Documents indexed --files. |
Suppressed comments (1)
tgrep-core/src/walker.rs:582
- Metadata failures are treated as “not too large” here, so a binary-extension entry whose metadata cannot be read is added to
listed_fileswithout incrementingskipped_error. The server treatsskipped_error == 0as proof that this walk is authoritative, which can publish a stale/uninspectable path infiles-extra.bin. Read metadata with the same error handling used for normal files before adding either kind of path.
let too_large = max_file_size
.is_some_and(|limit| entry.metadata().is_ok_and(|meta| meta.len() > limit));
if !too_large {
listed_files.lock().unwrap().push(rel_path);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
tgrep-cli/src/serve.rs — This update is not idempotent: LiveIndex::delete_file increments dirty_count on every call,… |
Issues resolved since last review (3)
| Severity | Finding |
|---|---|
tgrep-core/src/walker.rs — This similarly converts a metadata error into too_large == false, so a raced-away or… View resolved comment |
|
tgrep-cli/src/search.rs — --ignore-file-case-insensitive also changes traversal membership, but this compatibility gate… View resolved comment |
|
tgrep-cli/src/serve.rs — The new files RPC reads this in-memory delta, but handle_reload only replaces state.index… View resolved comment |
Suppressed comments (1)
tgrep-cli/src/serve.rs:2175
- The publication result is discarded after
replace_filename_extra_pathshas already updated memory and marked the index ready. If the rename fails, an older valid sidecar remains on disk; subsequent refreshes see no in-memory change and never retry, so standalone indexed--filescan later trust stale paths. Preserve a pending/dirty state until publication succeeds (and ensure a stale sidecar cannot be treated as current).
if replace_filename_extra_paths(state, listed_files) {
persist_filename_extra_paths(state, index_dir);
b07a5ab to
ac7c2cd
Compare
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
tgrep-cli/src/serve.rs — This swap is atomic for RPC readers, but watcher events can still be lost: handle_reload rebuilds… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
tgrep-cli/src/serve.rs — This update is not idempotent: LiveIndex::delete_file increments dirty_count on every call,… View resolved comment |
Add a compact filename-only sidecar and route --files through live or local indexes while preserving filesystem fallback semantics. Co-authored-by: Copilot App <[email protected]>
Keep reload filename state synchronized, fall back for case-insensitive ignore traversal, and reject capped-walk metadata failures. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 010dddbc-d242-4b2b-bbb7-9d3d66136013
Co-authored-by: Copilot App <[email protected]> Copilot-Session: 010dddbc-d242-4b2b-bbb7-9d3d66136013
Persist sidecar-only changes, preserve content/filename membership invariants, and replay binary filename changes across reloads and subtree recovery. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 010dddbc-d242-4b2b-bbb7-9d3d66136013
ac7c2cd to
d3769b3
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The background build can publish incomplete filename state after metadata errors, and active-path checks introduce avoidable linear scans.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
tgrep-cli/src/serve.rs — This metadata walk can return an incomplete listed_files set while reporting skipped_error (for… |
|
tgrep-core/src/hybrid.rs — IndexReader already maintains a sorted path order and exposes contains_path, but this… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
tgrep-cli/src/serve.rs — This swap is atomic for RPC readers, but watcher events can still be lost: handle_reload rebuilds… View resolved comment |
Preserve an authoritative filename snapshot when the final metadata walk is incomplete, and use the reader's indexed path lookup for active-path checks. Co-authored-by: Copilot App <[email protected]> Copilot-Session: 010dddbc-d242-4b2b-bbb7-9d3d66136013
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Indexed listing does not verify that build-time traversal settings match the current query.
Review tier: Balanced
Findings: None
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
tgrep-core/src/hybrid.rs — IndexReader already maintains a sorted path order and exposes contains_path, but this… View resolved comment |
|
tgrep-cli/src/serve.rs — This metadata walk can return an incomplete listed_files set while reporting skipped_error (for… View resolved comment |
Suppressed comments (1)
tgrep-cli/src/search.rs:497
- This checks only the query flags, not the traversal policy used to build the index/server snapshot. For example, an index built with
--hiddenmakes a plaintgrep --filesreturn hidden paths, while one built with a smaller--max-filesizesilently omits files the default query admits. Persist the build-time membership settings (hidden/ignore/excludes/size cap) and require compatibility before using either local or server filename data; legacy indexes without that metadata should walk.
fn filename_index_compatible(opts: &SearchOptions) -> bool {


Summary
--filesthrough the live server or local index instead of walking the repositoryfiles-extra.binsidecar for binary and other listable paths absent from the content indexTesting
cargo test --workspace --quietcargo clippy --workspace --benches -- -D warningscargo fmt --all -- --checkCloses #123