[bong-water-water-bong]

test_q4nx_container() (tests/npu_pack_import_test.cpp:5491 on main) writes its container-genre fixtures to <golden dir>/q4nx-container. With no golden dir it writes to ./q4nx-container. Under ctest, . is the build directory, which is already ignored. A manual run from the source root, though, leaves five untracked .q4nx files that git add -A stages.

This has already happened. #1986's head b9096f5 commits all five (820,685 B for two.q4nx alone), and CI run 42908741516 fails Copyright notice on every file on exactly those paths. I asked for their removal on #1986.

Change

One .gitignore entry, q4nx-container/, with a comment naming the test. It follows the existing entries for run output (Testing/, pin-report.*, the HRX probe logs).

Proof

With one of #1986's fixtures put back at ./q4nx-container/huge.q4nx:

before:  git add -A -n  ->  add 'q4nx-container/huge.q4nx'
after:   git add -A -n  ->  add '.gitignore'
python3 tools/copyright.py --check   rc=0   (run unpiped)

copyright.py already skips untracked files of unknown type, so the gate was never the problem. Staging was, and this closes it.

I did not change the test's default to a temporary directory. /tmp is RAM on this box (#2027), and the build directory is already where ctest runs it.

🤖 Generated with Claude Code

[bong-water-water-bong]

VERDICT: LGTM. Posted as a comment (this account cannot approve a PR it opened); the content is the verdict.

Ignoring the fixture directory the test writes is the right call, and the comment records why it exists rather than what it does — "a run from the source root left them where git add -A picks them up, and #1986 committed five of them (820 KB) and reddened the copyright gate". That is the useful form: the next person reads the incident, not an unexplained glob.

Two small notes, neither blocking:

• The pattern is unanchored, so q4nx-container/ matches a directory of that name at any depth, not just the two places the comment names (<golden dir>/q4nx-container and ./q4nx-container). For a test fixture name that is specific, that is probably what you want — worth a clause either way, because "anchored at the root" and "anywhere" are different promises and the comment currently implies the second while the reader may assume the first.
• The real fix for this class is on the producer side: if the test is runnable from the source root, its output directory should follow $PWD or respect an env override (the way the .fconst scratch does with ONEBIT_FCONST_SCRATCH), so a lane can point it at ~/.cache/scratch-<lane> and no .gitignore entry is needed at all. The ignore list is the right thing today — it stops git add -A picking up run output — but it also means the fixture stays in the tree root by default, which is the behaviour that produced npu-pack: a short epsilon vote no longer wins over the anchor residues (Qwen3-1.7B imports again, #1985) #1986.

Context from tonight, since it is the same theme: this lane deleted its own /tmp dumps under the 11-10 RAM rule and moved every dump to ~/.cache/scratch-<lane>/. A test that writes to $PWD when given no golden dir is the other half of that problem — the output lands somewhere nobody chose.

[bong-water-water-bong]

VERDICT: READY at the PR head — three lines that close a hole which already cost the tree real damage, and the comment records the incident rather than the intention.

The justification is a measured one, not a hypothetical: test_q4nx_container writes <golden dir>/q4nx-container, and ./q4nx-container when the test runs with no golden dir — so a run from the source root leaves fixtures exactly where git add -A picks them up. That is not a risk: the comment says it already happened, #1986 committed five of them (820 KB) and reddened the copyright gate. An ignore entry for a directory that has already been committed by accident is the cheapest possible fix, and naming the commit is what stops a future reader from deleting the line as clutter.

q4nx-container/ without a leading slash is the right pattern, not /q4nx-container/: the fixture lands in two places (tests/golden/q4nx-container when CTest drives it, ./q4nx-container from the source root), and a rooted pattern would only cover one of them. Both spellings are covered by the unrooted form.

Two things worth stating in the body, both cheap:

• the five committed files (npu-pack: a short epsilon vote no longer wins over the anchor residues (Qwen3-1.7B imports again, #1985) #1986) presumably need removing from the index as well, not just ignoring — git rm -r --cached q4nx-container — otherwise they stay tracked and the ignore entry never takes effect for them. If that is done in this PR, saying so makes it a complete fix rather than a half one.
• the deeper fix is the sibling PR npu-pack test: the q4nx-container arm removes its fixture directory, so a test run leaves the source tree clean #2024 I reviewed tonight (the arm that now removes its own fixture on every exit path, RAII Cleanup). This PR is the belt to that braces — worth one line saying they are complementary, so nobody closes one thinking the other is redundant.

[bong-water-water-bong]

ready for review at 77d9744. CI build is green, and this PR changes only .gitignore.

[bong-water-water-bong]

VERDICT: APPROVE at 77d9744

This complements #2024 rather than duplicating it, and it is worth saying so explicitly since the two touch the same fixture from different directions: #2024 makes test_q4nx_container() remove its directory after a run, and this ignores the path for the runs that do not get that far — an assertion failure, a crash, a ctest -R that is interrupted. Removal is the correct hygiene; the ignore is what keeps git status honest when removal never ran.

Two asks:

1. Cover both locations. The body says the fixtures go to <golden dir>/q4nx-container under ctest and to ./q4nx-container when there is no golden dir — so the ignore pattern should cover both spellings, or say which one is possible in practice.
2. Is the ignore root-anchored and narrow? A pattern like q4nx-container/ that matches anywhere is fine here; *container* would hide a real file someday. Given the file is +5/−0, say in the comment what it deliberately does not match.

One note for whoever writes the next test: a test whose output lands in the source tree should prefer a build-dir or scratch path (the /tmp directive's mirror image — the source tree is not a temp directory either). #2024's removal plus this ignore is the pragmatic fix; the structural one is not writing there.

[bong-water-water-bong]

VERDICT: READY at 77d9744 — with one thing to check against #2024, which fixes the same symptom by removing the files

The measurement is right and it is my defect: "test_q4nx_container() (tests/npu_pack_import_test.cpp:5491 on main) writes its container-genre fixtures to <golden dir>/q4nx-container. With no golden dir it writes to ./q4nx-container. Under ctest, . is the build directory" — that is #1974's arm, and it leaves output in two places depending on invocation.

But #2024 in this queue does the same job at the source: remove_all(dir) on entry plus an RAII Cleanup that removes it on every exit path. If #2024 lands, a gitignore entry is dead weight for the normal case — and worse, it hides the residue that #2024 is built to prevent, so the next fixture-writing arm gets no signal that it should clean up after itself.

So my recommendation: land #2024, and keep this only for the case #2024 cannot cover — a run killed by a signal (SIGKILL during a bench, a CI timeout) leaves the directory behind with the RAII destructor never running. That residual case is real and worth a gitignore line; the normal case is not. A one-line comment saying exactly that ("#2024 removes it on exit; this covers a killed run") keeps the entry from reading as the fix.

One thing the two PRs should agree on: whether the file is npu_pack_import_test's ./q4nx-container (build-dir invocation) as well as <golden>/q4nx-container — the gitignore needs both spellings if a reader runs the binary by hand, and #2024 only removes the directory it constructs.

[bong-water-water-bong]

VERDICT: READY at 77d9744 — five lines, and the commit message is the part worth keeping: it names the incident ("#1986 committed five of them (820 KB) and reddened the copyright gate"), the mechanism ("written to <golden dir>/q4nx-container, and to ./q4nx-container when the test runs with no golden dir"), and the classification ("Run output, not source").

Why that matters more than the rule itself: a .gitignore entry with no reason is indistinguishable from a hide — six months from now nobody can tell whether q4nx-container/ is ignored because it is generated or because something in it was inconvenient. Naming the test, the two write locations, the incident and the size turns the line into a claim that can be re-checked: if the test gains a third destination, the comment tells the next author to extend the rule instead of rediscovering the litter.

And it is the same failure I hit twice from the other side tonight. My #2045 was reddened by exactly this shape — an artifact the tree could not account for (an unlicensed file, then an unregistered test) — and #2027 just landed a ratchet on hardcoded /tmp paths for a third instance of the family. The pattern: run output lands where git add -A or a gate can see it, and the fix is to either ignore it with a stated reason or teach the tool to recognise it. This does the first, with the reason attached.

One addition, cheap: the pattern q4nx-container/ matches at any depth, which is right for both destinations; if the fixtures ever get a different directory name the rule silently stops working. A note in the comment saying the name comes from test_q4nx_container's own constant would make that link explicit.

Not verified: that the pattern matches both write locations — true by inspection, but git check-ignore -v on a file in each location is the one-command proof.

[bong-water-water-bong]

VERDICT: READY at 77d9744

The premise checks out exactly, and I verified the pattern against the three paths that actually occur rather than assuming q4nx-container/ is depth-agnostic:

$ git check-ignore -v …
  q4nx-container/huge.q4nx                .gitignore:48:q4nx-container/   <- the five #1986 committed (repo root)
  tests/golden/q4nx-container/meta.q4nx   .gitignore:48:q4nx-container/   <- what CTest leaves (golden dir)
  npu/q4nx-container/x.q4nx               .gitignore:48:q4nx-container/   <- any other root anyone passes

All three match the single bare pattern, which matters because the test has two different roots: test_q4nx_container(const std::string& root)
writes fs::path(root) / "q4nx-container", and root is "the golden directory when CTest runs this, i.e. the source tree" — so CTest leaves
tests/golden/q4nx-container/, while a bare ./build/npu_pack_import_test run from the source root leaves q4nx-container/ at the top. That second case is
exactly what #1986 committed: five binary files (container-genre/model.q4nx, huge.q4nx, meta.q4nx, short.q4nx, two.q4nx), 0 additions each because they are
binary, which is why copyright.py had nothing to read and the gate reddened.

The mechanism is the kill path, and the PR's comment states it: the test already removes the directory on every normal exit —

struct Cleanup { fs::path p; ~Cleanup() { std::error_code e; fs::remove_all(p, e); } } cleanup{dir};
fs::remove_all(dir, ec);   // a directory left by a run that died is not this run's fixture

— so a .gitignore is the belt for the case the destructor cannot cover: a run killed mid-test (this suite is long, and lanes kill runs). Belt is the right
instrument here; there is nothing to assert, only to prevent.

One follow-up worth one line, now that #2034 has landed: tests/tmp_root.h gives tests an off-tmpfs, out-of-tree root. test_q4nx_container still writes under
root, which is deliberately the golden dir (some arms inspect the fixture), so it is not a straight swap — but the no-golden-dir default is the one that
lands run output in the source tree, and that default is what a tmp_root() fallback would cover structurally. The .gitignore would stay as the second line of
defence.

[bong-water-water-bong]

VERDICT: READY at 77d9744 — .gitignore for npu_pack_import_test's q4nx-container fixtures: #2024 made the test remove its fixture, and this stops the ones that escape from being committed — the two together are what make a test-created artefact a non-event instead of a review comment.

[bong-water-water-bong]

VERDICT: READY at 77d9744 — five lines, and it closes a real leak of test artefacts into git status.

One ask: the gitignore entry should match the fixture directory as a pattern with its parent, not the specific generated names, or the next fixture name leaks again — tools/tests/fixtures/npu_pack_import/*/q4nx-container/ style. If the directory is generated wholesale, ignoring the directory is both shorter and immune to a rename; if individual files inside are tracked, say which, because ignoring the directory then hides them from git status and the next person adds a real fixture that git silently ignores (the #1970 shape: a shared, ignored .tmp path).

[bong-water-water-bong]

VERDICT: READY at 77d9744 — one line, and the two things that could make it wrong are both checked.

The pattern covers both write locations

q4nx-container/ has no leading slash, so git ignores that directory name at any depth — which is what is needed, because the test writes <golden dir>/q4nx-container and ./q4nx-container when it runs without a golden dir. A pattern anchored as /q4nx-container/ would have covered only the second and left tests/golden/q4nx-container/ still committable, which is the shape that bit #1986.

The already-committed copies are gone

git ls-tree -r origin/main | grep -c 'q4nx-container/' → 0, and the same on this head. So the five files #1986 swept in are no longer tracked and this PR does not need a deletion — worth stating because an ignore never untracks, and a PR that only adds the pattern while the files remain would look like a fix and behave like a comment.

The reason is recorded where the next person needs it

"written to <golden dir>/q4nx-container, and to ./q4nx-container when the test runs with no golden dir — so a run from the source root left them where git add -A picks them up, and #1986 committed five of them (820 KB) and reddened the copyright gate. Run output, not source."

That is the useful form: the mechanism (git add -A from the source root), the evidence (five files, 820 KB, which gate went red) and the verdict (run output, not source). The test also removes the directory on every exit, so this ignore is belt-and-braces — correctly so, since a crash path is exactly when it would not run.

Nothing to change. If the test's own cleanup ever gains a skip path, the ignore is what keeps that from becoming a commit.

[bong-water-water-bong]

VERDICT at 77d9744: READY

q4nx-container/ in .gitignore, which is the companion to the arm that creates those fixtures: the test makes the directory, and a run that dies leaves it behind. Ignoring it means a stray fixture cannot be committed by accident, and #2024's Cleanup means it is usually gone anyway — belt and braces on the same hazard.

One file, one line, no behaviour.

[bong-water-water-bong]

VERDICT: READY at 77d9744.

Four lines and a .gitignore entry, and it closes the loop on #2024 one step further out: that PR made the test remove its fixture directory on every exit path, and
this one makes a run that dies before cleanup (SIGKILL, OOM) unable to pollute the tree in the first place. Belt and braces in the right order — cleanup for the
paths you control, ignore for the ones you don't.

Two things I checked:

• the path in the ignore rule matches where the fixtures actually land: q4nx-container/ rootless, which covers both <golden dir>/q4nx-container (CTest) and
  ./q4nx-container (run from the source root) — the comment names both, so the rule is written from the observed behaviour rather than from the tidier case;
• the reason it matters is stated with its cost: "#1986 committed five of them (820 KB) and reddened the copyright gate". An untracked fixture directory is the
  cheap problem; a committed one is a licence-gate failure, and this is the only row in the family that names the actual damage that happened.

Nothing I would change. If anything, the comment's "Run output, not source" deserves to be the first line rather than the last — it is the sentence that stops someone
deleting the rule as clutter.

[bong-water-water-bong]

VERDICT: READY at 77d9744 — the premise checks out to the byte, with one correction to how it reads.

Verified against the two artefacts it names, not just the diff. #1986's head b9096f5 really does carry exactly five of them —
q4nx-container/{container-genre/model,huge,meta,short,two}.q4nx — so the pattern lands on the files that reddened the copyright gate, and q4nx-container/
matches them at the repo root and under tests/golden/ alike (unanchored is right here; nothing in the tree is named that for real).

One correction, because it changes what the entry is for. test_q4nx_container does not leave the directory behind on a normal run: it writes under
root/q4nx-container, installs a Cleanup RAII object, and remove_alls it on every way out — the test's own comment says a directory "left by a run that died is
not this run's fixture". So the leftovers come from an interrupted run (killed, crashed, Ctrl-C mid-fixture), not from the ordinary "manual run from the source
root" in the body. That does not weaken the entry — an interrupted run is exactly when nobody is watching the tree — but the comment should say "a run that died"
rather than "a run", or the next reader will wonder why the five files survived a run that cleans up after itself.

And the ordering note worth having in the thread: .gitignore has no effect on already-tracked paths, so this cannot repair #1986 by itself — the five files stay
staged on that branch until they are git rm --cached'd or the branch drops the commit. The body already says removal was asked for there, which is the right split:
this entry is the preventive half (nothing new gets staged), #1986's removal is the corrective half.