Skip to content

Use index for filename listing - #127

Merged
Shengyu Fu (shengyfu) merged 5 commits into
mainfrom
shengyfu-indexed-filename-search
Sep 2, 2026
Merged

Shengyu Fu (shengyfu) merged 5 commits into
mainfrom
shengyfu-indexed-filename-search

Conversation

@shengyfu

Copy link
Copy Markdown
Member

Summary

  • route --files through the live server or local index instead of walking the repository
  • add a compact files-extra.bin sidecar for binary and other listable paths absent from the content index
  • preserve legacy/corrupt-index and traversal-option filesystem fallbacks
  • keep filename state current through watcher, reconciliation, and publication flows

Testing

  • cargo test --workspace --quiet
  • cargo clippy --workspace --benches -- -D warnings
  • cargo fmt --all -- --check

Closes #123

Copilot AI balanced review requested due to automatic review settings August 30, 2026 19:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: 1 High severity · 2 Medium severity

New issues introduced by this change (3)
Severity Finding
High severity tgrep-cli/​src/​serve.rs — The new files RPC reads this in-memory delta, but handle_reload only replaces state.index…
Medium severity tgrep-cli/​src/​search.rs — --ignore-file-case-insensitive also changes traversal membership, but this compatibility gate…
Medium severity 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.bin sidecar.
  • 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_files without incrementing skipped_error. The server treats skipped_error == 0 as proof that this walk is authoritative, which can publish a stale/uninspectable path in files-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.

Comment thread tgrep-cli/src/serve.rs
Comment thread tgrep-cli/src/search.rs
Comment thread tgrep-core/src/walker.rs
Copilot AI review requested due to automatic review settings August 31, 2026 04:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity 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
Medium severity tgrep-core/​src/​walker.rs — This similarly converts a metadata error into too_large == false, so a raced-away or… View resolved comment
Medium severity tgrep-cli/​src/​search.rs — --ignore-file-case-insensitive also changes traversal membership, but this compatibility gate… View resolved comment
High severity 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_paths has 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 --files can 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);

Comment thread tgrep-cli/src/serve.rs Outdated
Copilot AI review requested due to automatic review settings September 1, 2026 03:44
@shengyfu
Shengyu Fu (shengyfu) force-pushed the shengyfu-indexed-filename-search branch from b07a5ab to ac7c2cd Compare September 1, 2026 03:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: 1 High severity

New issues introduced by this change (1)
Severity Finding
High severity 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
Medium severity tgrep-cli/​src/​serve.rs — This update is not idempotent: LiveIndex::delete_file increments dirty_count on every call,… View resolved comment

Comment thread tgrep-cli/src/serve.rs Outdated
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
Copilot AI review requested due to automatic review settings September 2, 2026 01:47
@shengyfu
Shengyu Fu (shengyfu) force-pushed the shengyfu-indexed-filename-search branch from ac7c2cd to d3769b3 Compare September 2, 2026 01:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Medium severity

New issues introduced by this change (2)
Severity Finding
Medium severity tgrep-cli/​src/​serve.rs — This metadata walk can return an incomplete listed_files set while reporting skipped_error (for…
Medium severity 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
High severity 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

Comment thread tgrep-cli/src/serve.rs Outdated
Comment thread tgrep-core/src/hybrid.rs Outdated
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
Copilot AI review requested due to automatic review settings September 2, 2026 02:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Medium severity tgrep-core/​src/​hybrid.rs — IndexReader already maintains a sorted path order and exposes contains_path, but this… View resolved comment
Medium severity 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 --hidden makes a plain tgrep --files return hidden paths, while one built with a smaller --max-filesize silently 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 {

@shengyfu
Shengyu Fu (shengyfu) merged commit ab7765a into main Sep 2, 2026
10 checks passed
@shengyfu
Shengyu Fu (shengyfu) deleted the shengyfu-indexed-filename-search branch September 2, 2026 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Indexed filename search

2 participants