Skip to content

Fix unnecessary server reindexing and Git-internal indexing - #165

Merged
Shengyu Fu (shengyfu) merged 5 commits into
mainfrom
shengyfu-tgrep-log-review
Sep 26, 2026
Merged

Shengyu Fu (shengyfu) merged 5 commits into
mainfrom
shengyfu-tgrep-log-review

Conversation

@shengyfu

Copy link
Copy Markdown
Member

Summary

Addresses server logs reporting source-file modifications during an otherwise unchanged initial build, processing .git/objects files, and reporting zero delta changes before saving live mutations.

  • Exclude .git directory subtrees from default index builds and server refreshes using the shared exclusion list. Preserve --no-ignore opt-in, normal filesystem-scan behavior, and Git metadata monitoring for ignore rules and tracked paths.
  • Preserve indexed content fingerprints when withholding startup metadata evidence. Deferred notifications still force content verification, but unchanged files no longer produce redundant overlay mutations or saves.
  • Distinguish filesystem notifications, binary classification, and content-index updates in trace messages. Report unique delta candidates alongside separate scan and live-overlay counts.
  • Document the behavior in README and the agent guide.

Regression coverage

Covers both index-build strategies, explicit opt-in, nested Git directories, binary filename membership, native watcher subscriptions, fresh and pre-existing indexes, .git/info/exclude updates, reload persistence, and unchanged versus changed startup replay (including directory notifications and preserved modification times).

Validation

Executed on Windows:

  • cargo test --workspace --lib --bins --locked --quiet — 524 passed, 1 ignored.
  • cargo test -p tgrep-cli --test indexed_hidden --test indexed_files --test watcher_dot_ignore --test warm_start_gitignore --test watcher_watch_registration --locked --quiet — 25 passed.
  • cargo clippy --workspace --benches --locked --quiet -- -D warnings — passed.
  • cargo fmt --all -- --check and git diff --check — passed.

Compatibility

--hidden no longer admits .git directories into the default indexed corpus. Use --no-ignore consistently on index and serve to include them, or --no-index --hidden for explicit filesystem inspection. Startup reconciliation removes Git-internal entries from older indexes.

Preserve indexed content fingerprints during deferred-event replay, exclude Git metadata directories from the default indexed corpus, and clarify watcher and save diagnostics. Add regression coverage and document the behavior.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings September 26, 2026 05:43

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

Linux and Android native watchers no longer observe ordinary .git/info/exclude updates.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Prevents redundant server reindexing and excludes Git internals from indexed corpora.

Changes:

  • Excludes .git subtrees by default while preserving --no-ignore.
  • Retains content fingerprints during deferred-event replay.
  • Expands regression tests and documentation.
File Description
tgrep-core/​src/​builder.rs Adds default Git-directory exclusion.
tgrep-cli/​src/​serve.rs Updates watcher, evidence, and trace handling.
tgrep-cli/​tests/​indexed_hidden.rs Adds Git-internal indexing regressions.
README.md Documents indexing and replay behavior.
AGENTS.md Updates agent-facing guidance.

💡 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
Keep native per-directory subscriptions for ordinary ignore sources such as .git/info/exclude, while retaining Git-internal indexing filters and in-root containment. Cover regular, missing, and symlinked sources.

Co-authored-by: Copilot App <[email protected]>
Copilot AI review requested due to automatic review settings September 26, 2026 05:54

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

Recreating a deleted .git/info/exclude can go unnoticed on non-recursive watcher backends.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread tgrep-cli/src/serve.rs
Resolve the candidate ignore-file location separately from existing sources, retain its contained parent watch, and recognize newly created exclude files during events and recovery. Cover absent startup and delete-refresh-recreate on native watchers.

Co-authored-by: Copilot App <[email protected]>
Copilot AI review requested due to automatic review settings September 26, 2026 07:02

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

Cross-platform watcher registration and concurrent startup evidence publication remain sensitive enough to warrant final human review.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Seed a historical exclude file so the exact corrective-pass assertion is not affected by the independent ignore-source recovery window.

Co-authored-by: Copilot App <[email protected]>
Copilot AI review requested due to automatic review settings September 26, 2026 15:10

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

A running server can miss the transition from a non-Git directory to a Git repository on per-directory watcher backends.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread tgrep-cli/src/serve.rs
refresh: RefreshControl::new(watch_mode, poll_interval, watch_budget),
watch_registry: Mutex::new(None),
exclude_dirs: exclude_dirs.to_vec(),
exclude_dirs: builder::index_exclude_dirs(exclude_dirs, no_ignore),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A narrow edge case, ignore for now.

Copilot AI review requested due to automatic review settings September 26, 2026 15:36

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

Case-insensitive Git directory names and live repository-boundary transitions can still leave indexing or ignore state incorrect.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Honor filesystem case semantics for implicit .git exclusion

tgrep-core/​src/​builder.rs:119

The implicit exclusion is matched through the existing case-sensitive directory-name checks (walker.rs:72 and serve.rs:3014). On a case-insensitive filesystem, Git accepts a repository metadata directory whose preserved spelling is .GIT, but DirEntry::file_name()/the watcher path retain that spelling, so the injected lowercase .git does not match and its objects are still indexed. Please make the implicit Git-directory check follow filesystem case semantics without changing explicit --exclude matching, and cover mixed-case metadata on Windows.

@shengyfu
Shengyu Fu (shengyfu) merged commit 7b70671 into main Sep 26, 2026
12 checks passed
@shengyfu
Shengyu Fu (shengyfu) deleted the shengyfu-tgrep-log-review branch September 26, 2026 15:44
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.

2 participants