Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fc16ffc38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fc16ffc38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
da57fd3 to
fd85b20
Compare
|
Included in #454, merged as b6f4e75. Reviewed head |
Why
Literal
--limitcounted matching lines before grouping files, so repeated hits in one file could hide another file. Filename-only searches also built previews for matches that could never appear in the output. The ASCII trigram filter could exclude files whose Unicode lowercase form matched the query.What changed
Rank and limit literal results by file, using exact matching-line counts and deduplicating overlapping workspaces. Bounded searches return up to three previews per file and report omitted matches through
more_hits_in_fileand text output.--no-limitreturns every matching line, ranked by file.If a selected file changes between counting and preview generation, reconsider candidates rather than returning an empty or stale result. Each retained file's count and previews come from the same content read.
For filename-only output, stream file contents and retain only the selected paths. When one candidate file is sufficient, stop after the first match and the binary-sniff prefix. Candidate filtering avoids ASCII trigrams that are unsafe under the existing Unicode lowercase rules. Queries without a safe trigram walk the workspace instead.
To reproduce the file-limit and Unicode bugs:
The first query returns both
b.txtanda.txt. The second returnsunicode.txt.Validation
cargo fmt -- --check,cargo clippy --locked --all-targets -- -D warnings,cargo test --locked --test literal_cli --test cli_snapshot --test daemon_ipc --test mcp_e2e,cargo test --locked --no-default-features --test literal_cli, focused streaming, ranking, and file-change library tests, and./test.sh --quickran on macOS. A release binary from the initial revision completedscripts/e2e_all.sh --binary <release-binary>, including daemon equivalence and headless Web UI acceptance.A new client stopped an incompatible version 9 daemon and returned results locally with autospawn disabled. A version 9 client searched through the new daemon without restarting it. Linux CI on the initial revision exercised the
RLIMIT_DATAmemory regression. The full./test.shsuite and neural E2E were not rerun.A temporary worktree combined the code changes from the three PR branches. It ran
cargo test --locked --test skip_gitignore --test literal_cli, both E2E infrastructure unit-test modules, andscripts/e2e_all.sh --binary <debug-binary>with an overlongTMPDIR. The browser used one headless worker. Markdownlint checked the later architecture note.The dependency audit fails on the unchanged main-branch lockfile because of existing
rustlsandzipadvisories.A daemon mutation soak on this head exceeded its anonymous RSS growth budget: 35.6 MiB against 32 MiB. An earlier run of the byte-identical binary and a rerun passed. The cause of the observed growth is not established. The soak sends hybrid search requests rather than literal file requests.
Evidence
On an Apple M5 Max with 64 GiB of memory, the filename-only reproduction used about 309 MiB peak RSS on the base release and 17 MiB on this release. Each measurement used the same indexed 700,000-byte file with 20,000 matching lines. The JSON outputs were identical across repeated runs and a follow-up measurement after the review fixes. Normal bounded output used about 17 MiB and reported 20,000 matches, three previews, and 19,997 omitted matches.
To reproduce the macOS memory measurement, set
BASE_IGandPR_IGto release binaries built from the base and this branch:On the same macOS 26.7.1 host, a warm-cache filename-only benchmark compared the base and this branch with ripgrep 15.2.0. The corpus was the
sharkdp/batarchive ataf1f53d9a977. It contained 909 source-tree files. ivygrep indexed 829 files andrg --filesreported 850 under its filters. A UTF-16 file was excluded from both searches. The index was already built, query caching was disabled, four threads were used, and each tool ran 15 times in rotating order. Each query returned the same filename set from all tools.HighlightingAssetserrorkeykeyfalls back to a workspace walk to preserve Unicode matches. One outlier dominated theHighlightingAssetsp95 for this branch. A later 50-invocation comparison for that query measured 12.93 ms median and 16.69 ms p95 for this branch, with a 19.35 ms maximum. This small warm benchmark does not measure index build time or cold-cache performance, and the tools have different file filters.The file-change check recounts selected files before returning previews. On the same 700,000-byte file, bounded output with
-n 1 -C 100had a 24.69 ms median before the review fix and 26.15 ms after it, with identical JSON across 15 rotating invocations.To repeat the comparison, extract the archive, initialize a Git directory, and index it with a release binary. Use an isolated
IVYGREP_HOME,IVYGREP_NO_AUTOSPAWN=1,IVYGREP_DISABLE_QUERY_CACHE=1,RAYON_NUM_THREADS=4, andTOKIO_WORKER_THREADS=2. From the corpus directory, run the following commands for each query, normalize the paths, verify the filename sets, and rotate the tools across 15 warm invocations:Run the ivygrep command with
BASE_IGas well. The absent marker was__ivygrep_absent_marker_9b65__.Risk
The daemon protocol changes from version 9 to 10. Older version 8 and 9 clients retain legacy hit-limit semantics against the new daemon. A new client restarts an older incompatible daemon. No index format migration is required. Revert the change and restart the daemon to roll back.
Downsides
Bounded literal output can now omit previews that previously appeared in a single-file result. The exact matching-line count and omitted count remain visible.
--no-limitrestores all previews, but can use much more memory. A short query without a safe trigram can require a full file walk. An edit to a selected file during a search can cause another candidate scan. The search does not provide an atomic snapshot across the workspace. The matcher preserves existing lowercase and sigma handling, not full Unicode case folding such asßmatchingss.Checklist
./test.sh --quickpassed during iteration../test.shpasses, or Validation explains narrower proportional checks.