fix(crawl): --grep no longer scans gitignored files the indexer skips — the leak was in the crawl, not the verb - #87
Merged
Conversation
… — the unsupported-ext class consults the ignore verdict before it records a row --grep/--regex additionally scan the crawl's unsupported-ext, text-looking population (CrawlSkips::unsupported) and print those hits in the trailing <unindexed> block. That population was recorded before the ignore set was consulted: collectSources classified the extension first (recordPreSizeDrop) and asked git only about files that survived, so a file that was BOTH gitignored AND of an unindexed extension was rowed as unsupported-ext and grep read it. Measured 2026-09-09: --regex='^#include' --grep-in=any served four hits from a .cpp.bak the repository's own .gitignore names (rg does not open it), and unindexed_files_scanned= / unsupported_ext= both counted it. The fix keeps the ordering contract (the ignore lookup stringifies the path, so it is still not paid for a binary asset or an --exclude'd file) and adds ONE consult for ONE class: the unsupported-ext branch asks a lazy `ignored` predicate — the same predicate the indexable-file test already uses, now defined once — before it records a row, and records nothing when the answer is yes. The file is then in NO class, exactly as an --exclude'd unsupported-ext file already was: not unsupported-ext (grep serves that list), and not ignored= (that counter describes only what would otherwise have been indexed, the number the header's accounting invariant carries). --no-ignore makes the predicate false, so the escape hatch restores the row and both counts with it. Gate first: test/grepignorecheck.sh (written RED — arms A B C D G failed on the pre-fix binary, every mutation control fired) pins the served set against an independent oracle (`git ls-files -co --exclude-standard` piped to grep -l, plus rg where present), on both --grep and the report's own --regex='^#include'; that unindexed_files_scanned= and the skipped verb's unsupported_ext= describe what was actually scanned; that a tracked file matching the pattern and an untracked-but-unignored file are still served (git's rule, not rg's textual one); the --no-ignore and non-git escape hatches; the MCP grep twin; determinism; xmllint. Absorbed into regression.sh's loop; the eight stated gate counts move 563 -> 564. Also: the skipped legend's definition of unsupported_ext= now names both exclusions; ARCHITECTURE.md still said ".gitignore is not consulted", which has been false since the N6-C lane — corrected. Replayed on the reporting corpus (read-only, --no-cache): default run 0 hits in the .bak, unindexed_files_scanned 409 -> 52 (+140 skipped = unsupported_ext 192, reconciled); --no-ignore still serves it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…eader — ripwirepubliccheck arm 1 The header's measured-defect paragraph named the private validation corpus and its directory; CI run 34359885383 (Release clang shard 3/4) failed ripwirepubliccheck.sh arm 1 on it, the one gate that sweeps every tracked file for exactly this. Fixed forward, not rewritten: the name stays in this branch's history at 4c249fd, which makes the branch a squash-merge candidate at landing (the owner's call, as for the tgrep and codeburn lanes this round). Wording only; the gate's arms and its fixture are untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…569 -> 570 at the eight stated sites The loop in test/regression.sh names 570 with grepignorecheck absorbed; the number is derived from the loop on this tip, never carried from an earlier base (this lane and the tgrep lane both derived 568 against the same 567 main, each correct in isolation, and identical text at all eight sites merges cleanly and silently). Sites: README.md x2, docs/EVALS.md x3 (incl. the §8 prose line), present/deck5_ripwire_build.js x3. test/manifestcheck.sh green at 570. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📝 SummarySummary by CodeRabbit
WalkthroughThe crawl now applies Gitignore results before recording unsupported-extension drops. A new regression gate verifies CLI and MCP grep behavior, no-ignore behavior, non-Git roots, reporting, and deterministic output. Documentation and presentation references update the gate count to 570. ChangesGitignore-aware grep scanning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Grep
participant Crawl
participant Git
participant Report
Grep->>Crawl: request source scan
Crawl->>Git: evaluate Gitignore status
Git-->>Crawl: ignored or not ignored
Crawl->>Report: record eligible unsupported files
Report-->>Grep: return files and scan counters
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
--grepwas scanning files the crawl had already decided not to index.The leak was upstream of the verb, in the crawl:
recordPreSizeDropandcollectSourcesinsrc/ingest_crawl.hadmitted gitignored files whose extension the indexer does not handle, so--grepwalked and read them.grepCollectAuxitself was never wrong — it was being handed the wrong file set.docs/ARCHITECTURE.md§1 said ".gitignore is not consulted", which had been stale since N6-C; that paragraph is corrected, and the documented directory denylist was cross-checked entry-for-entry againstkCrawlSkipDirs(22 names, includingcmake-build-*and theCMakeCache.txtsentinel).test/grepignorecheck.shis the gate. Gate count 569 → 570.Scope, honestly reported: §2–§5 of
docs/ARCHITECTURE.md(data model, determinism, honesty, the two build flavours) were not audited and are unreviewed rather than clean.A gate-header prose leak, and the vector is worth naming. This lane's first push put the private validation corpus's name into
test/grepignorecheck.sh's own header — the paragraph describing the defect the gate measures cited the corpus and its directory. Not run output, not a path: documentation. It was scrubbed forward at3d9ea048, and it is squash-merged here because4c249fd0still carries the name in the branch's history.Three of nine lanes in this round leaked that string, by three different routes — benchmark run output, a commit's absolute paths, and this one. The general rule the round settled on: a gate header may name what it measured and how, never where — unless the corpus is public, in which case name it.
The rebase found a trap that has been recorded. Rebasing onto a main that had moved twice, git auto-merged README and EVALS (this branch's 570 survived) and conflicted only on the deck — because whether a count site conflicts depends on the surrounding lines, not the number. A uniform "take upstream, then bump" therefore reset only the deck to 569 while README and EVALS already read 570, and the bump script aborted on its own precondition without the shell stopping. Caught from
git show HEAD:present/deck5_ripwire_build.jsrather than from the worktree. The recipe fix: reset every count file explicitly fromorigin/mainbefore bumping.Also from this lane, now a durable note: the "900 s macOS timeout" that the h2h-graft round recorded without a name is runner starvation. Job wall 3,070 s against 1,474 s / 1,903 s on green main runs;
ripwirepubliccheck607 s on macOS against 79 s on ubuntu in the same run; the timing-out gate costs ~54 s locally. Compare the job's wall before debugging anrc=124, and never raise the gate's budget.Verified before landing: tip contains current main; CI run 34410923221 26/26 green on the exact tip; gate count re-derived on the tip and all eight published sites confirmed at 570 from the commit (
git show HEAD:), not the worktree;manifestcheckre-derived both sides independently and agreed;qschemetripcheckunchanged; leak sweep clean.🤖 Generated with Claude Code