Skip to content

docs: align README with implementation and trim repetition - #164

Merged
Shengyu Fu (shengyfu) merged 2 commits into
mainfrom
shengyfu-readme-accuracy-review
Sep 24, 2026
Merged

Shengyu Fu (shengyfu) merged 2 commits into
mainfrom
shengyfu-readme-accuracy-review

Conversation

@shengyfu

Copy link
Copy Markdown
Member

Summary

  • Reduce README wording by roughly 33%, removing repeated benchmark narratives and marketing language while retaining the flag reference and operational caveats.
  • Correct server-only options, save timing, locking claims, posting format, encoding behavior, and index freshness guidance against the implementation.
  • Fix search examples, build instructions, and architecture-specific installation commands.

Validation

  • Cross-checked documentation against CLI parsing, search/index/server code, release workflows, and benchmark documentation.
  • Checked local links and heading anchors, documented CLI flags, code-fence balance, and git diff --check.
  • Documentation-only change; no application tests run.

Copilot AI balanced review requested due to automatic review settings September 24, 2026 05:17

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 critical search-flag behavior mismatch and several documentation inaccuracies remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity

Open (4)
What changed in this PR

This documentation-only PR condenses the README and aligns usage, indexing, server, search, and installation guidance with the implementation.

Changes:

  • Reduces repeated performance and architecture content.
  • Updates CLI, watcher, freshness, encoding, and index guidance.
  • Corrects build and installation examples.

Review findings:

  • Critical (README.md:531): Several flags do not automatically bypass indexed searches as documented and may be ignored without --no-index; update the bypass behavior and add tests.
  • Moderate (README.md:41): Scope parallel matching claims to server RPC searches.
  • Moderate (README.md:46): Describe 0.93x as a measured ratio, not a speedup.
  • Moderate (README.md:163): Document that --max-memory also applies to fresh index builds.
  • Moderate (README.md:164): Clarify that --index-threads does not control fresh bootstrap builds.
  • Moderate (README.md:184): Describe watcher fallback in terms of incomplete native coverage.
  • Moderate (README.md:512): Scope binary-file exceptions to searches, not indexing.
File Description
README.md Consolidates and corrects project documentation and examples.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Copilot AI review requested due to automatic review settings September 24, 2026 05: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

Multiple documentation claims remain inconsistent with the implementation.

Review effort: Balanced
Findings: None

Resolved since last review (4)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Scope parallelism claim to server-backed searches

README.md:43

This still overstates parallelism for non-server searches. Only server RPC matching uses Rayon (serve.rs:1932-1939); local-index and filesystem paths iterate candidates sequentially (search.rs:1331-1352, 1518-1539). Remove the general parallel claim or scope it explicitly to server-backed queries.

Medium severity Document sticky polling fallback for incomplete coverage

README.md:190

The watcher also switches to sticky polling when the coverage walk is incomplete, not only for a budget or native-registration failure: apply_watch_registrations_locked reports this case at tgrep-cli/src/serve.rs:3615-3623. Please retain that operational fallback caveat in the shortened description.

@shengyfu
Shengyu Fu (shengyfu) merged commit 69e4fd0 into main Sep 24, 2026
12 checks passed
@shengyfu
Shengyu Fu (shengyfu) deleted the shengyfu-readme-accuracy-review branch September 24, 2026 05: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