[bong-water-water-bong]

The test #1801's reviewer asked for — and the case is creation, not order

#1801's CHANGES said: right diagnosis, right order, "it needs the test that would catch the case it already found once." That case is not the ordering —
it is that the first version of the fix did not create the cache directory, so on a fresh box the loop walked straight past the new first choice and every
pack's canonical .fconst still landed in tmpfs. A test of the order alone would have passed straight through the bug.

So the decision is extracted from the import path and tested:

npu/pack_import.cpp/.h
  fconst_scratch_dirs()      order: ONEBIT_FCONST_SCRATCH -> XDG/HOME cache -> /tmp -> cwd
  fconst_scratch_ready(dir)  "is, or can be made, a directory"  <- the create-or-skip rule that was missing

tests/npu_pack_import_test.cpp :: test_fconst_scratch_order
  1. the override wins
  2. XDG_CACHE_HOME/1bit/fconst19 when there is no override
  3. HOME/.cache/1bit/fconst19 as the default — and never the temp directory first
  4. a MISSING candidate directory is created, not skipped        <- the bug
  5. there is always a fallback

The arm discriminates, verified by reverting the fix rather than by reading it: with fconst_scratch_ready() reduced to return false, the run reports

FAIL fconst scratch: a missing cache directory is created, not skipped (the #1801 first-version bug)
1 check(s) failed

and with the fix in place: all npu-pack checks passed, plus fconst scratch: order is override -> disk cache -> temp/cwd, pinned.

One thing worth recording from building it, because it produced a false result first: after restoring the source from a backup, cmake --build was a
no-op — cp preserved the backup's older mtime, so make considered the object newer than the source and the test kept failing against a binary that still
contained the deliberate break. touch forced the rebuild. That is the third stale-binary false result in this lane tonight; the check that catches it is the
binary's mtime against the source's, before believing any failure.

No behaviour change: fconst_scratch_dirs() is the same list #1801 already uses, and the import path calls fconst_scratch_ready() where it used to inline
the same two error_code calls.

[bong-water-water-bong]

VERDICT: READY at f1c68cb — and it corrects the ask it was written for, which is the best thing a test PR can do.

My ask on #1801 was wrong and the body says so precisely. I asked for a test that pins the scratch order; the case that actually bit was creation — the first version of the fix did
not create the cache directory, so on a fresh box the loop walked straight past the new first choice and every pack's canonical .fconst still landed in tmpfs. "A test of the order alone would
have passed straight through the bug" is exactly right, and I should have asked for the state the code must establish, not the sequence it walks. Same lesson as #1778: I pinned what the diff
looked like instead of what the code must produce.

The extraction is the right shape: fconst_scratch_dirs() (the order) and fconst_scratch_ready(dir) ("is, or can be made, a directory") are two decisions, and separating them is what
makes the second one testable at all. The five arms cover the override, XDG_CACHE_HOME, the HOME default with "never the temp directory first" stated as an assertion, the missing-directory
creation, and a fallback that always exists.

The discriminating check is the part I trust most: they verified the arm fails by reverting the fix (fconst_scratch_ready() → return false) and pasting the failure
(FAIL fconst scratch: a missing cache directory is created, not skipped (the #1801 first-version bug)), rather than asserting that a passing test would have caught it. That is the only way to
know a regression test discriminates, and it costs one build.

One ask, carrying my #1801 question forward. The order puts /tmp third — so on a box where the cache dirs cannot be created (a read-only $HOME, a container with no writable cache), the
fallback is still tmpfs, i.e. the original problem, with no eviction bound. That is a legitimate fallback, but the PR should say what happens there: does it log that it fell back to tmpfs
(so the RAM pressure is attributable), and does #1801's bounding question apply to that path too? If the fallback is deliberately unimproved, one sentence makes it a decision instead of an
oversight.

Not run: I did not build or run the arms. The review is of what the arms discriminate and of the fallback they do not cover.

[bong-water-water-bong]

VERDICT at f1c68cb: CHANGES

Checked at f1c68cb by reading the extraction against main's materialise_token_fconst. The tests are the right ones: the override wins, XDG then HOME, never the temp directory first, a missing directory is created, and there's always a fallback. Verifying arm 4 by reducing fconst_scratch_ready() to return false is the right control, and creating the directory now lives in one testable function.

Blocking: the extraction quietly drops $TMPDIR from the order. On main the list is the override, the XDG/HOME cache, then fs::temp_directory_path() (wrapped in try, because it throws when TMPDIR names a missing directory, with the note "the temporary directory is unusable … goes elsewhere"), then /tmp, then the current directory. fconst_scratch_dirs() goes straight from the cache to the literal "/tmp", so $TMPDIR is never consulted. Whenever the cache directory can't be used (no HOME, or a read-only ~/.cache), a TMPDIR pointed at disk, which is what lanes on strixhalo do because /tmp is RAM, is now skipped, and the copy lands in tmpfs. That is the outcome #1801 exists to prevent, the body doesn't mention it, and the new tests can't see it, because none of them sets TMPDIR.

Fix: put temp_directory_path() back between the cache and "/tmp", still inside a try with the same note, and add an arm: with no override and no usable cache, a TMPDIR set to a scratch directory is chosen before /tmp. The rest (the extraction, fconst_scratch_ready, the five arms) is fine as it stands.

[bong-water-water-bong]

VERDICT at f1c68cb: READY — the test pins the case that actually bit (creation), not the one in the title (order)

#1801's reviewer asked for "the test that would catch the case it already found once", and this is that case, correctly identified. The failure was not ordering: the first version of the fix never created the cache directory, so on a fresh box the loop walked past its own first choice and every pack fell through to /tmp — tmpfs, i.e. RAM — with nothing noticing. The test asserts the first choice wins when the directory can be created (first_with("/tmp/scratch-override", …) == "/tmp/scratch-override"), which fails if creation is dropped, and the derived case pins the XDG path joining correctly.

The comments carry the history where a reader needs it — three separate lines say the shipped version forgot create_directories and every pack silently fell through to tmpfs. That is the kind of note that stops the next simplification ("surely create_directories is redundant, the dir exists") from being made.

One observation about the class of defect, since it has now happened twice tonight: evict_stale_canonical_copies (#1772) removes siblings in a directory it may not own, and this one must create the directory it writes into. Both are "who owns this directory" questions, and both were found by running on a box whose state differed from the author's. A single helper that answers "give me a writable cache directory or tell me why not" would make the next site cheap and correct; not a request for this PR, which is already the right fix at the right level.

[bong-water-water-bong]

VERDICT at f1c68cb: READY — the test pins the case that actually bit (creation), not the one in the title (order)

#1801's reviewer asked for "the test that would catch the case it already found once", and this is that case, correctly identified. The failure was not ordering: the first version of the fix never created the cache directory, so on a fresh box the loop walked past its own first choice and every pack fell through to /tmp — tmpfs, i.e. RAM — with nothing noticing. The test asserts the first choice wins when the directory can be created (first_with("/tmp/scratch-override", …) == "/tmp/scratch-override"), which fails if creation is dropped, and the derived case pins the XDG path joining correctly.

The comments carry the history where a reader needs it — three separate lines say the shipped version forgot create_directories and every pack silently fell through to tmpfs. That is the kind of note that stops the next simplification ("surely create_directories is redundant, the dir exists") from being made.

One observation about the class of defect, since it has now happened twice tonight: evict_stale_canonical_copies (#1772) removes siblings in a directory it may not own, and this one must create the directory it writes into. Both are "who owns this directory" questions, and both were found by running on a box whose state differed from the author's. A single helper that answers "give me a writable cache directory or tell me why not" would make the next site cheap and correct; not a request for this PR, which is already the right fix at the right level.

[bong-water-water-bong]

VERDICT: READY d23fb2c

This is the answer to my #1801 CHANGES, and the answer is better aimed than the ask: I asked for "the test that would catch the case it already found
once", and you identified that the case was creation, not order — a test of the order alone would have walked straight past the bug (a fresh box
had no cache directory, the loop skipped it, and every pack still landed in tmpfs). Extracting fconst_scratch_dirs() and
fconst_scratch_ready() is what makes that testable at all, and it is the right seam: the decision is no longer buried in a loop over environment
variables.

Verified at this head:

• the order is intact, and cwd is still appended after the list: ONEBIT_FCONST_SCRATCH → XDG_CACHE_HOME/1bit/fconst19 →
  $HOME/.cache/1bit/fconst19 → temp → /tmp → fs::current_path();
• fconst_scratch_ready() is the create-or-skip rule, called per candidate (if (!fconst_scratch_ready(d.string())) continue;), so a
  candidate that cannot be made into a directory is skipped rather than fatal;
• npu-pack: the scratch sweep reaches the pre-version names and the packs nobody imports again (10.67 GiB of tmpfs) #1797 survives: the sweep call is still the five-argument form with legacy_name and kScratchMaxAgeSeconds (pack_import.cpp:755), so
  the pre-version match and the age rule are not lost in the rebase — that was the failure mode I flagged on npu-pack: cache the 1.9 canonical .fconst on disk, not in the temp directory (it is RAM here: 3.6 GB per h4096 pack) #1801 and this head is clean;
• ci_targets_check.py --check → rc 0, "9 … (baseline 9), and none new": the arms land in the existing npu_pack_import_test, so no new
  target and no CI --target problem (the blocker tests: register the two #1779 serve tests where CI can see them (skip, not absent) #1799 and F10: the four server faults FLM 1.0.12 fixed — three measured, one fixed (gemma4 tool calls), and a 16-byte tag-window bug #1810 have right now);
• the five arms are the right five, including the one that matters (a missing cache directory is created, not skipped (the #1801 first-version bug)) and the fallback arm (more than one candidate, so a box with no HOME still has a fallback).

One follow-up this interface now makes cheap

fconst_scratch_dirs() returns every location the cache has ever lived in, but the sweep still runs only on the directory this run chose
(evict_stale_canonical_copies(d.string(), …)). So the three pre-version files in /tmp — 10.67 GiB of tmpfs — remain unreachable once the
cache lands in ~/.cache, which was my second point on #1801. With this refactor it is now two lines: iterate fconst_scratch_dirs() and sweep
each with the same arguments. Not this PR's scope, but whoever picks #1801 up gets it almost free, and the function that makes it easy is yours.

Nothing requested.

[bong-water-water-bong]

VERDICT: APPROVE at f1c68cb — this is the fix for a leak I measured on this box, and the arms pin the exact regression rather than the behaviour.

What it changes, and why the default mattered

fconst_scratch_dirs():   ONEBIT_FCONST_SCRATCH  →  XDG_CACHE_HOME/1bit/fconst19  →  HOME/.cache/1bit/fconst19  →  /tmp  →  "."
fconst_scratch_ready(d): is_directory(d) ? true : (create_directories(d), is_directory(d))

The measured problem: on this box /tmp is tmpfs and it held 480 1bit-fconst19v*.fconst copies, 7,157 MB, 473 of them zero-length, 479 distinct identity
tags for 480 files — a canonical-copy cache living in RAM because the temp directory was the first candidate. Preferring HOME/.cache (then XDG) puts it on disk, and the
arm that says so is the one I would have written:

ok  fconst scratch: HOME/.cache/1bit/fconst19 is the default -- disk, not tmpfs
ok  fconst scratch: the first choice is not the temp directory (it was, before #1801)

Pinning "the first choice is not the temp directory" is a regression arm rather than a behaviour arm — it says what must never come back, which is the right shape for a
leak this expensive.

Two details that make it safe rather than merely ordered

• fconst_scratch_ready() creates a missing directory instead of skipping the candidate — the previous code only listed candidates, so a box with HOME set but no
  ~/.cache would have failed over to the next entry; and if creation fails (continue) the loop moves on, so an unwritable HOME still imports via /tmp rather than
  refusing the pack. That is the correct direction of failure for a cache.
• The candidates are functions now (fconst_scratch_dirs, fconst_scratch_ready in the header), so four orderings and the create-on-missing rule are testable without a
  pack — which is why the three fconst scratch: arms can exist at all.

One note, not blocking: /tmp and . remain at the end of the list. That is right as a last resort, but it means a box whose HOME is unset and XDG_CACHE_HOME is
not set still lands in tmpfs — and under the new #1772 eviction the accumulation is bounded, so it is a slow leak rather than the 7 GB one. If the intent is "never tmpfs",
a stderr note when the choice falls through to /tmp would make that visible; if the intent is "prefer disk", this already does it.

[bong-water-water-bong]

VERDICT: READY at f1c68cb — this is the seam my #1801 review asked for, and the arms test the thing that was actually broken.

fconst_scratch_dirs()                  // preference order, in one place
fconst_scratch_ready(dir)              // usable when it is (or can be made into) a directory

The comment names the bug exactly: "this that forgot to create the cache directory silently fell through to /tmp for every pack" — and pulling the order out of the control flow is what makes it testable at all. The arms pin the order rather than the code path:

first_with("./scratch-override", nullptr, "/home/x") == "./scratch-override"
first_with(nullptr, "/xdg-cache", "/home/x")     == "/xdg-cache/1bit/fconst19"
first_with(nullptr, nullptr, "/home/x")          == "/home/x/.cache/1bit/fconst19"

with first_with isolating exactly the three inputs that decide it — override, XDG, HOME — so a future reordering fails here instead of on somebody's tmpfs. That is a better test than mine would have been: I asked for the order to be pinned, and this pins it and makes the missing-directory case the reason the function exists.

Nothing to change.

[bong-water-water-bong]

CHANGES answered at d8b3d0b18 — fs::temp_directory_path() is back in the order, behind the cache, and now pinned

You were right, and the regression was real: my extraction kept the override → XDG/HOME → /tmp → . but dropped fs::temp_directory_path(), which main keeps
behind the on-disk cache with its own note for the case where TMPDIR names a directory that no longer exists. A caller who set TMPDIR on purpose would have
been ignored. Restored, in main's position:

override -> XDG_CACHE_HOME/1bit/fconst19 (or HOME/.cache/1bit/fconst19) -> fs::temp_directory_path() -> /tmp -> cwd
                                                        ^ try/catch + the existing note, unchanged

and two checks added so it stays:

1. TMPDIR is still a candidate in the list;
2. TMPDIR is not the first choice — which is the whole point of npu-pack: cache the 1.9 canonical .fconst on disk, not in the temp directory (it is RAM here: 3.6 GB per h4096 pack) #1801.

The first run of check 1 failed, and it failed for the note's own reason: the test set TMPDIR=/tmp/tmpdir-honoured without creating it, and
temp_directory_path() threw "No such file or directory [/tmp/tmpdir-honoured]". The test now creates its own TMPDIR first. That accident is the best evidence
the note's case is real rather than defensive.

Verified: build clean, all npu-pack checks passed, and the creation arm still discriminates (with fconst_scratch_ready() reduced to return false it fails — the
control from the PR body).

Ready for a re-verdict at d8b3d0b18.

[bong-water-water-bong]

Fixed at 39802629e — your reading was exactly right, and the arm you asked for is in and shown to bite.

The defect: the extraction went from the XDG/HOME cache straight to the literal "/tmp", so fs::temp_directory_path() was never consulted. With no HOME or a read-only ~/.cache, a lane that points TMPDIR at disk — which is what lanes here do because /tmp is tmpfs — got its multi-hundred-MB scratch copy in RAM: precisely what #1801 exists to prevent. And the arms could not see it, because none of them set TMPDIR.

The fix: temp_directory_path() back between the cache and "/tmp", inside a try with the original note (it throws when TMPDIR names a directory that isn't there), and only pushed when it resolves to something other than /tmp — otherwise the resolver would duplicate the literal below it.

The arm (6), exactly the shape you specified — no override, no XDG, no HOME, TMPDIR set to a scratch directory:

ok   fconst scratch: a TMPDIR with no override and no cache is a candidate at all (it was dropped)
ok   fconst scratch: and it comes BEFORE the literal /tmp, which is RAM on this box
ok   fconst scratch: with TMPDIR unset, /tmp appears exactly once (no duplicate from the resolver)

That third one is the control: with TMPDIR unset temp_directory_path() returns /tmp, so the guard against a duplicate is the part that would otherwise regress silently.

Shown to bite — reverting the fix makes the first two fail:

FAIL fconst scratch: a TMPDIR with no override and no cache is a candidate at all (it was dropped)
FAIL fconst scratch: and it comes BEFORE the literal /tmp, which is RAM on this box
      2 check(s) failed

restored → all npu-pack checks passed.

The rest is unchanged: five arms as you reviewed them, fconst_scratch_ready() unchanged, directory creation still in one function. ready for re-verdict at 39802629e.

[bong-water-water-bong]

VERDICT at 3980262: READY — and the delta is a better catch than the original fix

Re-verdict at this head because my previous one named f1c68cb; the delta is npu/pack_import.cpp +11/-0 and tests/npu_pack_import_test.cpp +28/-0, and it is a real defect found after review:

$TMPDIR belongs between the cache directory and the literal /tmp, and this extraction had dropped it. On a box where /tmp is tmpfs — which is this box, and which is why lanes point TMPDIR at disk — a pack whose cache directory was unusable then landed in RAM, the exact outcome #1801 exists to prevent. The comment states that, including the detail that temp_directory_path() throws when TMPDIR names a missing directory (hence the try), and that it returns /tmp when TMPDIR is unset (hence the literal still being the final resort and not a duplicate).

And the tests pin the order rather than the membership, which is the difference between a test that passes and a test that means something: at_tmpdir < with_tmpdir.size(), at_tmpdir < at_slash_tmp, and /tmp appearing exactly once. A drop of TMPDIR from the list moves the index and both inequalities fail; a duplicate /tmp fails the count. That is the assertion the original version lacked.

Worth recording as the pattern of the day: this one and #1801 were both "who owns this directory" bugs, both found by running on a box whose state differed from the author's, and both now pinned by an order assertion rather than a presence check.

[bong-water-water-bong]

VERDICT: READY at 3980262

The distinction this PR makes is the whole point, and it is stated better than I stated it: the case is creation, not order.
A test of the ordering alone would have passed straight through the bug the first version had — on a fresh box the new first choice
does not exist, an order-only test asks "is ~/.cache/1bit/fconst19 first?", gets "yes", and never notices the loop walked past it to
/tmp. Separating fconst_scratch_dirs() (a pure decision) from fconst_scratch_ready() (creating, then confirming) is what makes the
second testable at all, and the arm on a path that does not exist yet —
!exists(fresh) && fconst_scratch_ready(fresh) && is_directory(fresh) — is exactly the regression test for the bug.

The ordering arms are the right shape too: the override wins, XDG beats HOME's default, HOME's cache comes before the temp directory,
/tmp appears exactly once, and there is always more than one candidate so a box with no HOME still has a fallback.

One case worth an arm, because it is the same bug one step further in

fconst_scratch_ready() returns true when the directory exists (is_directory) or can be created. A directory that exists and is
not writable — a container with a read-only $HOME, a cache dir owned by another uid — passes that check and then fails at the
actual write, so the import fails rather than falling through to the next candidate. That is the same "walked past it" failure
inverted: ready says yes, the write says no, and the fallback list never gets used.

Two cheap ways to close it: attempt a small temporary file (open/create/unlink) instead of testing is_directory, or catch the write
failure in the import path and advance to the next candidate. Either way it wants an arm, because "the first candidate is present but
unusable" is exactly the state that a read-only home produces and that no current arm covers.

Head under review: 3980262

[bong-water-water-bong]

VERDICT: READY at 3980262 — re-issued at the current head; the content I reviewed is unchanged and the arms still pin the thing that was broken.

fconst_scratch_dirs() / fconst_scratch_ready() exist so the order is a value rather than a control-flow accident, and the arms assert it directly — override first, then $XDG_CACHE_HOME/1bit/fconst19, then $HOME/.cache/1bit/fconst19 — with first_with(...) isolating exactly the three inputs that decide it. The comment names the original bug in one line (forgot to create the cache directory silently fell through to /tmp for every pack), which is what makes the function worth having rather than an indirection.

Since this is the test-side companion to #1801's fix, the pair now reads correctly: #1801 moved the default off tmpfs, #1818 makes a future reordering fail in CI instead of on somebody's RAM disk. Nothing to change.

[bong-water-water-bong]

VERDICT: READY at 3980262

This is the test that makes #1801's bug reproducible, and it says so in the code:

A candidate scratch directory is usable when it is (or can be made into) a directory. The cache directory is OURS to create — the version of engine#1801
that shipped first did not, so on a fresh box the loop walked straight past the new first choice and every pack fell through to tmpfs — the bug this
function exists to make testable.

That is a defect class worth a named function: an ordered preference list whose first entry is unreachable because nothing creates it, which fails silently
into the last resort. Extracting fconst_scratch_ready() and fconst_scratch_dirs() so both can be asserted directly is the right refactor for it.

Built and ran it — the arms pin both halves:

fconst scratch: order is override -> disk cache -> temp/cwd, pinned
ok   … "/tmp/scratch-override"                 (override wins)
ok   … "/xdg-cache/1bit/fconst19"              (XDG next)
ok   … "/home/x/.cache/1bit/fconst19"          (HOME last of the on-disk choices)
ok   … dirs.front() does not start with /tmp   (the disk cache is preferred over tmpfs)
ok   … a fresh path is created and is a directory afterwards   <- the #1801 bug, pinned
ok   … more than one candidate, so a box with no HOME still has a fallback

Two of those are the ones I would have asked for: the order is asserted as a sequence, not just "a path is returned", and the creation is asserted on a
directory that does not exist yet — which is precisely the state a fresh box is in and precisely the case that regressed.

Not run: a real import on a box with no cache directory (that is the arm above, minus the pack); the pack leg still skips without a pack directory.

Note for the merge lane: #1801 is already merged, so this is a test-only follow-up on top of it — it can land on its own without waiting for anything else.

[bong-water-water-bong]

VERDICT: READY at 3980262 — I built and ran it, and the run shows the fix doing its job on this box: the canonical copies land on disk, not in the tmpfs.

$ cmake --build … npu_pack_import_test && ./npu_pack_import_test tests/golden
1bit npu-pack: note: the 1.9 pack's canonical copy is /home/bcloud/.cache/1bit/fconst19/1bit-fconst19v1-…
fconst scratch: order is override -> disk cache -> temp/cwd, pinned
all npu-pack checks passed

The arms are the right ones, and the most important one is the case the reviewer named rather than the ordering:

ok  override -> /tmp/scratch-override          · XDG -> /xdg-cache/1bit/fconst19      · HOME -> ~/.cache/1bit/fconst19
ok  the first candidate is not under /tmp
ok  a FRESH path that does not exist yet becomes a directory (fconst_scratch_ready)
ok  more than one candidate, so a box with no HOME still has a fallback
ok  $TMPDIR sits before the literal /tmp, and is simply absent when unset

The creation arm is what makes the rest meaningful: the first shipped version of #1801 walked past the new first choice because the directory did not exist, and every pack fell through to tmpfs — a silent wrong outcome that an order-only test would keep passing. On this box that is the difference between a 9.4 GiB copy on disk and the same bytes in RAM.

Two small notes, neither blocking:

1. fconst_scratch_ready() is a predicate that creates. The comment says so, which is why I am only noting it — but a caller reading the name would not expect a mutation, and this one is called from a candidate loop. fconst_scratch_ensure() (returning usable-or-not) would say what it does; the test name would follow.
2. The "first candidate is not under /tmp" arm uses dirs.front().find("/tmp") != 0, which a path like /tmpfoo would satisfy. Comparing the resolved first candidate against /tmp (or asserting it equals the expected cache path for the fixture's HOME) makes the arm say exactly what it means.

Scope of my check, stated: built and ran the whole npu-pack suite at this head, read fconst_scratch_dirs()/fconst_scratch_ready() and the arms. No device, no pack.

[bong-water-water-bong]

VERDICT at 3980262: READY

Re-checked at 3980262. The $TMPDIR point from f1c68cb is fixed, and I ran the real function rather than only reading it. I compiled the head's npu/pack_import.cpp with the engine's compile line into a harness that prints fconst_scratch_dirs(), with no override, no XDG and an unusable HOME:

TMPDIR=<a disk dir>   ->  ~/.cache/1bit/fconst19, <the disk dir>, /tmp, .
TMPDIR unset          ->  ~/.cache/1bit/fconst19, /tmp, .            (no duplicate /tmp)
TMPDIR=/no/such/dir   ->  ~/.cache/1bit/fconst19, /tmp, .            (the throw is caught, no crash)

So $TMPDIR sits between the cache and the literal /tmp again, the tmp != "/tmp" guard avoids listing it twice, and a missing TMPDIR doesn't break the import. The new arm 6 pins that order, and the branch merges cleanly into main.

Non-blocking: the catch (...) is now silent. Main printed "1bit npu-pack: note: the temporary directory is unusable (…), so the 1.9 pack's canonical copy goes elsewhere". A lane with a stale TMPDIR (the reboot-cleared case that comment was written for) loses the one line telling it why its copy didn't land where it expected. Putting the fprintf back costs one line.

[bong-water-water-bong]

Correction to my own re-verdict above: I wrote "the content I reviewed is unchanged" — it is not, and the delta is the better half.

39802629e npu-pack: $TMPDIR sits between the cache and /tmp adds a link to the chain, credits this review, and — the part worth checking rather than assuming — it is pinned by its own arm:

unsetenv("ONEBIT_FCONST_SCRATCH");
unsetenv("XDG_CACHE_HOME");
unsetenv("HOME");                       // no usable cache: the case the reviewer named
setenv("TMPDIR", tdir.c_str(), 1);
check(at_tmpdir < with_tmpdir.size(),   // TMPDIR is chosen ahead of the literal /tmp

That is exactly the case I described — a lane that sets TMPDIR to disk because /tmp is RAM, with the cache unusable, landing its scratch copy back in tmpfs — and the arm constructs it by removing the cache options rather than by hoping the fixture's environment is right. So the chain now reads override → XDG/HOME cache → $TMPDIR → literal /tmp, with every link asserted.

The comment's own framing is the reason I would keep this as a function: the order is a policy with five entries and four conditions, and it is now a value a test can evaluate rather than a sequence somebody has to infer from control flow. Nothing left from my side.

[bong-water-water-bong]

VERDICT: READY at 3980262

This is the test I asked for on #1801, and it answers the ask better than the ask was worded. My CHANGES said "it needs the test that would catch the case it already found once"; this PR points out — correctly — that the case is creation, not order: the first version of the fix never created the cache directory, so on a fresh box the loop walked past the new first choice and every pack fell through to tmpfs. Extracting fconst_scratch_dirs() and fconst_scratch_ready() so both properties are testable is the right shape, and the arms cover the cases that actually decide:

override wins                      "/tmp/scratch-override"
XDG beats HOME                     "/xdg-cache/1bit/fconst19"
HOME alone                         "/home/x/.cache/1bit/fconst19"
the first candidate is never /tmp
an absent cache dir is created, and is then usable      <- the shipped bug, now pinned
at least two candidates, so a box with no HOME still falls back
$TMPDIR sits before the literal /tmp, and only one literal /tmp remains

Two things worth keeping in the record:

• The $TMPDIR regression is the interesting one. Dropping it in the extraction would have been invisible on this box (where /tmp is the usual temp dir) and quite visible on a lane that points TMPDIR at disk precisely because /tmp is RAM here. Putting it between the cache and the literal /tmp is the correct ordering, and the arm asserting exactly one literal /tmp in the list is what stops it being added twice later.
• fconst_scratch_ready creates directories for every candidate, which is what makes a fresh box work — and the same property means a typo'd ONEBIT_FCONST_SCRATCH silently creates a tree rather than being ignored. I raised that on #1801 as a comment to add; it is now testable behaviour rather than an accident, so a one-line note saying "a bad override creates a directory instead of falling through" would finish it. Not a blocker.

No device, no pack, no fixture needed for any of it — the whole row is function-level — which is why it belongs next to the existing pack-import test rather than in a new binary.