Repository navigation
chore: remove a scratch test transcript that reached main in #1951 - #1975
Merged
Merged
Conversation
`.pris_v4.log` is 1364 lines of `cargo test` output. It is mine and it should never have left my worktree. How it got in: I amended a commit with `git add -A` in a worktree that had a teed test log sitting in it. The file was named outside every pattern in .gitignore, so nothing stopped it, and I did not read the commit's file list before pushing. I caught it in the same check that should have run first -- `git diff --name-only origin/main...HEAD` -- but by then auto-merge had already taken the earlier head. No behavioural change; the file is inert. The .gitignore line is the same belt-and-braces reasoning the file already documents for CM*.txt / PR*.md / .issue-*.md, which exist because a stray commit-message body reached a PR once before. This is the same failure with a different extension, so it belongs in the same list. Scoped to `/*.log` -- root only -- so a crate or fixture that legitimately tracks a .log is unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
enabled auto-merge (squash)
August 24, 2026 11:01
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1975 +/- ##
==========================================
+ Coverage 80.21% 80.40% +0.18%
==========================================
Files 421 423 +2
Lines 203480 207429 +3949
Branches 203480 207429 +3949
==========================================
+ Hits 163231 166773 +3542
- Misses 34675 35016 +341
- Partials 5574 5640 +66
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
## What `.body.md` — the PR body for #2026 — is tracked on `main` right now. This removes it and adds a `Diff guard` job that makes the root an enumerated set rather than a pattern-matching problem. ## Why not another .gitignore pattern It is the **fourth** file of this class, and it landed *after* the two most recent fixes for it: | file | arrived | removed by | |---|---|---| | `.commitmsg/` | — | `490b846c3` | | `.commitmsg` | #1881 | #1999 | | `.pris_v4.log` (1364-line cargo transcript) | #1951 | #1975 | | **`.body.md`** | **#2026, today** | **this PR** | Every repair added one more pattern. The block's own comments predict the failure twice — *"the name matching none of the patterns above and nothing stopped the next one"* and *"the file was named outside every pattern above"* — and then it happened again, hours later, to a file whose intended name (`.pr-body.md`) **is** already listed. Someone typed a shorter one. Three non-converging iterations is enough evidence. A pattern list has to predict the next filename; an allowlist does not. ## The check Asserts the repository root is exactly `.github/root-file-allowlist.txt`. It lives beside `deletion-ratio` because it guards the same blind spot from the other end: that job catches a change too large for anyone to read, this one catches a single added line invisible in a diff of hundreds. Both are cases where *the shape of the change*, not its content, is the signal. Adding a legitimate root file = add a line to the list in the same PR. That is deliberately not a label: a label is ephemeral, a line in a tracked file is a permanent record of the decision, which is the property the existing `.gitignore` comments were reaching for. ## Validation Six arms, driving the script **extracted from the workflow YAML** rather than a copy — a seam that reimplements the check passes whether or not CI runs the same thing (per #1897): | arm | expect | result | |---|---|---| | clean root | accept | `rc=0`, "Root is exactly the 16 allowlisted file(s)" | | stray root file (`.body.md`) | reject | `rc=1`, names the file | | **nested** `crates/…/.body.md` | accept | `rc=0` — not over-broad | | stale allowlist entry | reject | `rc=1` — the list cannot rot into a rubber stamp | | allowlist file **missing** | reject | `rc=1` — fails closed, no vacuous pass | | new root file, allowlisted in-PR | accept | `rc=0` — escape hatch works | Two of those are the ones I would have skipped if I were not being careful. **Missing-allowlist** matters because the natural implementation returns success when it cannot find its own input — the guard would go permanently green the moment someone moved the file. **Stale-entry** matters because an entry that outlives its file silently pre-approves that exact filename, so a rotting list is worse than none. `.gitignore` also gets `/.body.md`, verified both directions (root ignored, nested not, shadows zero tracked files). That line is explicitly the *trailing* half of the repair — it is the layer that has failed four times, kept because it stops the local `git add -A`, not because it is the fix. ## Not claimed This does not stop a stray file in a **subdirectory**; the root is where the evidence is (4/4), and a repo-wide version would need a scratch-file heuristic, which is the guessing game this PR is trying to end. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 25, 2026
…2052) Follow-up to #2036, merged earlier today. **The guard it shipped cannot see a stray *directory* at the root — and two of the seven historical instances are directories, one of them cited in #2036's own header.** ## The gap #2036 compares tracked files whose path contains no separator: ```bash git ls-files | grep -v '/' ``` A stray directory has no such path. `.commitmsg/m.txt` contains a separator, so it is discarded as "not at the root" — and the thing that *is* at the root, `.commitmsg`, never appears in `ls-files` output at all. The guard doesn't fail to complain; it affirmatively reports a clean root. Not hypothetical: `.commitmsg/` reached `main` as `faedea4d1`, removed three minutes later by `490b846c3` *"Remove accidentally tracked PR artifacts"*. #2036's header lists it, with the trailing slash. **I enumerated the instance and then validated against six arms that all used a file.** Naming an instance is not testing its shape. ## The fix Compare **first path segments** — a root file contributes itself, a nested file contributes its top-level directory: ```bash git -c core.quotePath=false ls-files | sed 's#/.*##' | LC_ALL=C sort -u ``` The allowlist gains the 20 tracked root directories (36 entries), and unlisted entries are labelled `(directory)` or `(symlink)` where they are one, because the remedy differs. ## The inventory, corrected by review I claimed six instances "found by walking every root path ever added, not by collecting what people reported". **The walk had the same blind spot as the guard** — it filtered to paths without a separator, so it could not see a stray directory either. Redone on first segments: | entry | added → removed | on `main` | |---|---|---| | `.msg.txt` | `39675330b` → `bbc193117` | 1.6h | | **`.commitmsg/`** | `faedea4d1` → `490b846c3` | 3m | | **`.goldens/`** | `faedea4d1` → `490b846c3` | 3m | | `.wa64.log` | `83a51bfa6` → `c07acaa78` | 17.2h | | `.commitmsg` | `e42fa9470` (#1881) → `398cff8e5` (#1999) | 19.8h | | `.pris_v4.log` | `589d48ffd` (#1951) → `7a6482c83` (#1975) | 1.3h | | `.body.md` | `79196f89d` (#2026) → `54625db9d` (#2036) | 2.7h | **Seven entries, six incidents** — `.commitmsg/` and `.goldens/` arrived and left together. `.goldens/` is the most on-point instance available (a directory removed as a "PR artifact") and my method could not see it. Excluded, with reasons recorded in the file rather than silently: `site` (moved to onnx-genai-wiki, #1488), `third_party` (oneDNN removal), and `abresults` — 131h on `main`, the longest of any, but added by a `docs(benchmarks): record the … result` commit that says it is recording a result. Intentional-when-added is the line; `.wa64.log` rode in on a `test(cpu):` commit that never mentions it. `.wa64.log` still matters beyond the count: it predates `.pris_v4.log` by two days, so `/*.log` in #1975 was reactive to the *second* log incident. No authorship attributed — squash-merge rewrites `%an` to the merging account, so it reads identically for all seven and says nothing about who staged the file. ## Validation — 15/15 Driving the script **extracted from the workflow YAML**, never a copy. | arm | rc | |---|---| | clean root | 0 — `Root is exactly the 36 allowlisted entr(ies).` | | **stray root directory → new guard** | **1**, labelled `(directory)` | | **same stray → #2036 guard + #2036 allowlist** | **0** — `Root is exactly the 16 allowlisted file(s).` | | `.goldens/`, the second directory instance | 1 | | duplicate allowlist entry → not reported stale | 0, warning names it | | root symlink | 1, labelled `(symlink)` | | stray root file / stale entry / comments-only / missing list | 1 / 1 / 1 / 1 | | nested file under an allowlisted dir | 0 | | non-ASCII root file, allowlisted | 0 | | new root directory allowlisted in-PR / not | 0 / 1 | | CRLF allowlist | 0 | Row 3 is the control and must pair **both** of #2036's halves. My first attempt paired the old guard with the *new* allowlist: it returned 1 and looked like coverage, but the 1 came from 20 directory entries reading as stale. ## Two instrument bugs in my own battery 1. **Wrong control**, above — a control that changes two things measures neither. 2. **The staging check had the defect the guard was fixed for.** Each arm asserts its input reached the index before believing the output, but that check used bare `git diff --cached --name-only`, which renders non-ASCII as `"caf\303\251.txt"` while the guard uses `core.quotePath=false`. It reported the non-ASCII arm VACUOUS against a setup that had worked. Opus caught exactly this in #2036's guard; it reappeared in the thing measuring the guard. ## Also fixed, from review - An entry listed twice left one copy unpaired in `comm` and was reported as "not present at the root" for a name that is. Deduped both sides; duplicates now raise a `::warning::` naming them. - `[ -d ]` follows symlinks, so a root symlink to a directory was labelled a directory and advised `git rm -r --cached`. `-L` tested first. - Recorded the cost of first-segment comparison: the guard sees **root children only**. Scratch under an already-blessed directory is invisible to it. ## Scope CI-config only — `.github/workflows/diff-guard.yml` and `.github/root-file-allowlist.txt`. No Rust, no runtime behaviour, no test changes. Adding a root entry, file or directory, means adding it to the allowlist in the same PR; the error message says so. --------- Co-authored-by: holden <holden@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
What this removes
.pris_v4.log— 1364 lines ofcargo testtranscript — reachedmainin #1951 (589d48ffd). It is mine. It is inert, but it is junk in the tree and it should come out before anyone branches off it.How it happened, stated plainly
I amended a commit with
git add -Ain a worktree that had a teed test log sitting in it, and I pushed without reading the commit's file list. The file was named outside every pattern in.gitignore, so nothing stopped it.I did catch it — running
git diff --name-only origin/main...HEADshowed exactly three paths where there should have been two, and I amended it out. By then auto-merge had already taken the earlier head, which is correct behaviour on its part: the required checks were green and nothing in this repo's CI has an opinion about a stray file. The check that would have caught it is the one I ran second instead of first.The generalisable bit, since it is not specific to me:
git add -Ain a worktree you have been running builds in stages whatever is lying around, and the only thing between that andmainis a file-list check you have to remember to run. Readinggit show --statbefore pushing costs nothing. It is the same class as the fixture blast-radius traps @resch and @Gaff hit today — the command's scope was wider than the change I had in mind, and nothing about the command said so.Why the
.gitignoreline.gitignorealready carries a block for precisely this:That block exists because a stray commit-message body reached a PR once. This is the same failure with a different extension, so it belongs in the same list rather than in a habit I promise to keep.
Scoped to
/*.log— root only — so a crate or fixture that legitimately tracks a.logis unaffected. Falsified in both directions rather than assumed:The third line is the one that matters: a rule that ignored every
.loganywhere would hide a tracked fixture log fromgit status, which is a worse failure than the one it prevents.Validation
No behavioural change — the deletion is an untracked-by-anything text file and the
.gitignoreedit cannot affect a build. No test, lint, or build result is a function of either change, so I am not going to dress this up with a green matrix that would be true of an empty commit.Normal auto-merge, required checks only. No admin bypass.
Refs #1951.