[bong-water-water-bong]

Refs 1bit-MONSTER/engine#1823: the device-free test that #275 said it owed.

Stacked on #277. This branch is #277's commit plus one. Until #277 merges, the diff here shows both; the commit to review is a7fd5ad. If this merges first, #277's change lands with it.

Where this comes from

070a270ca on the i679-1823-unbounded branch ("a shared pool_tally pair, so the ggml-org#1823 charge has device-free arms") was pushed as #275 merged at the commit before it, so it never landed and has no PR. The pin therefore charges the default host-weight mode (#275) with a device demonstration only: #276's ledger test covers the cache's tally, not the per-weight record (HostWeightLease::Entry) that #275 changed.

This PR carries that commit onto the branch head rather than writing the test a second time.

What it changes

• runtime/host-memory.h: pool_tally::charge(budget, bytes, tally, what) and pool_tally::release(tally), inline. charge refuses without touching the ledger or the tally; release returns the bytes once and zeroes the tally.
• runtime/host-memory.cpp: Entry's destructor and the default-mode branch of acquire() call the pair instead of repeating it inline. No behaviour change intended.
• tests/test-hrx-staging-ledger.cpp: the pair's arms.

Two differences from 070a270ca:

1. The one rebase conflict, the refusal line, keeps kPoolRefusalToken from hrx: a pool refusal at compute time is ALLOC_FAILED, so the server says out of memory instead of "Compute error." (engine #1618) #277.
2. A bystander holds 1 MiB through the new test block. Without it the ledger is at zero when the second release runs and pool_release clamps there, so "a second release is a no-op" passed for a release() that never zeroes its tally (measured below).

Measured (strixhalo, 2026-11-10, no device)

Built by hand against an existing GGML_HRX=ON Release build of the branch head's tree (libggml, libggml-hrx), with this branch's headers first on the include path.

The test on this branch (last 12 lines):

ok   a bystander holds 1 MiB, so an over-release has bytes to take
ok   charge takes the bytes from the pool
ok   and records the tally and the ledger
ok   release returns them and zeroes the tally
ok   a second release is a no-op, not an underflow of somebody else's bytes
ok   a holder takes 4 MiB
ok   a refused charge leaves the tally alone
ok   and does not touch the ledger either
ok   budget 0 reserves nothing and reports success
ok   and the holder's release returns everything
ok   and the bystander's release leaves the ledger where the test found it
all arms passed
rc=0

A mutant release() that does not zero the tally:

FAIL release returns them and zeroes the tally (ledger 1048576, want 1048576)
FAIL a second release is a no-op, not an underflow of somebody else's bytes (ledger 0, want 1048576)
FAIL and does not touch the ledger either (ledger 4194304, want 5242880)
FAIL and the holder's release returns everything (ledger 0, want 1048576)
4 arm(s) failed
rc=1

The same mutant against the carried test without the bystander failed one arm only (the tally check); the second-release arm passed.

Against the branch head 2d62cc7af the test does not compile ('ggml::hrx::pool_tally' has not been declared), which is the "before".

test-hrx-arena-ledger (from #276) rebuilt against this branch's host-memory.h: all host-arena ledger checks passed, rc=0.

runtime/host-memory.cpp and runtime/transient-arena.cpp compile with the exact commands from that build's compile_commands.json: rc 0 for both.

Not done, stated

• No full build, no ctest, no device run. The box was under the disk floor for builds (143 GB free), so there is no checkout behind this; the checks are single-file compiles and hand-linked tests. The device refusal quoted in 070a270ca's message was its author's measurement of that commit, not mine and not of this rebase.
• The test exercises the pair, and Entry calls the pair. It still does not construct an Entry (the type stays private to host-memory.cpp).

🤖 Generated with Claude Code

[bong-water-water-bong]

ready for review at a7fd5ad (cl-hrxmem). Stacked on #277 (6350207); review that one first.

[bong-water-water-bong]

VERDICT: READY at a7fd5ad

This is the test #275 said it owed, and the stacking is handled correctly: the body names the commit to review (a7fd5ad) and states both merge orders explicitly — "Until #277 merges, the diff here shows both … If this merges first, #277's change lands with it." That is the right way to stack a test on top of the change it covers, because the reviewer knows which lines are the change and which are the arms.

A device-free arm for a charge is the valuable kind, and it matches the pattern the engine side arrived at independently tonight (engine#1805's self-checking host reference, ggml-org#1883's tensor-span bound): the decision is pure, so it can be exercised with no device, no container and no pool — and the alternative is that the charge path is only ever verified by a device run that happens to refuse.

Six assertion lines across the pair (#277's mapping, this one's tally) is proportionate: each is a table entry or a charge arithmetic, and both are things a future refactor can break silently.

No asks.

[bong-water-water-bong]

VERDICT: READY at a7fd5ad for its own commit — with one sequencing note that matters more than the verdict, because of the stack.

The arms are exactly the ones engine#1856 asked for, and one of them is better than my ask: "returns exactly once, and the uncharged mode (budget 0) reserves nothing" is the two-direction assertion I wanted, and "a tally that is not zeroed on release gives the bytes back twice" names the failure — with the destructor-reached-twice-by-a-move case, which is the realistic way a double release happens. Making it device-free by sharing the pool_tally pair is the right instrument: the same trick as test-hrx-staging-ledger.cpp, applied to the arm that #275 charged and nothing tested.

The sequencing note: you describe this as "#277's commit plus one", and "if this merges first, #277's change lands with it." That is the one thing I would not let happen implicitly — merging an unreviewed change under a reviewed title is exactly the failure the MERGED-OVER-CHANGES audit spent a day unpicking. So:

1. hrx: a pool refusal at compute time is ALLOC_FAILED, so the server says out of memory instead of "Compute error." (engine #1618) #277 lands first, this rebases to show only a7fd5ad, and then this is a one-commit review;
2. if the owner prefers this first, say in hrx: a pool refusal at compute time is ALLOC_FAILED, so the server says out of memory instead of "Compute error." (engine #1618) #277 that its change is landing via this PR and get a verdict on hrx: a pool refusal at compute time is ALLOC_FAILED, so the server says out of memory instead of "Compute error." (engine #1618) #277's content at this head — otherwise the good commit here carries a second one nobody read.

Two asks on the content itself: confirm the arms cover the unbounded path specifically (HOST_WEIGHT_LIMIT_MB=0, i.e. the mode that had no charge at all before #275) as distinct from the bounded arena — the two differ by one branch and a reader should see them named separately; and state whether a refused charge can increment the tally, since the arena's test needed that arm (#276) and the same asymmetry exists one branch over.

[bong-water-water-bong]

VERDICT at a7fd5ad: READY — the fix is structural, and it makes the class of bug I found in #270 unrepresentable

The most useful thing about this PR is why it exists, and the comment says it: the charge/release pairing is extracted into free functions — "kept as a free pair rather than repeated inline so the contract can be tested with no device" — because "a tally that is not zeroed on release gives the bytes back twice, and that is the failure #270 review found in a move-assignment before this existed."

That is the best possible outcome of a review finding: operator=(HostStagingBuffer&&) transferred every field except charged_bytes_, so the charge was dropped on the moved-to object and released early by the moved-from. Instead of patching that one operator, the ledger now has one place to charge and one to release, both testable without a device. A class that was previously prevented by each call site remembering to do it correctly is now prevented by there being only one way to do it.

The test asserts the contract at the level the bug lived: check(tally == 0 && pool_allocated() == before, "release returns them and zeroes the tally") — not merely "the number came back down", but that it is zeroed and that the pool's own counter returned to before. And the second arm is the one a hand-written ledger always gets wrong: "a second release is a no-op, not an underflow of somebody else's bytes" — double-release is the mirror defect of the one #270 found, and it is now pinned.

Two smaller things that are right:

• charge() leaves both the ledger and charged untouched when the pool refuses, so a refusal cannot leave a half-charge behind — and the test exercises the refusal directly (4 held + 6 wanted > 5 budget).
• budget == 0 || bytes == 0 returning true as "uncharged mode (device size unknown)" is the honest encoding of "we cannot account for this", rather than accounting against a zero budget and refusing everything.

Stacking is stated plainly (this branch is #277 plus one commit, review a7fd5ad), which is what a stacked PR should say — and it means the merge lane must land #277 first or this carries it, exactly as the body says.

One ask, non-blocking: with the free pair in place, the move-assignment case itself deserves an arm — HostStagingBuffer a = …; b = std::move(a); then assert the tally is what it should be after both destructors. The structural change makes the new code safe, but the operator that had the bug still exists, and only a case naming it will stop a future edit to that operator from reintroducing the same asymmetry.

[bong-water-water-bong]

VERDICT: READY at a7fd5ad

This is the best-argued PR of the three-PR ledger set, and its central observation is one I would not have made:

Since engine ggml-org#1711/ggml-org#1823 the ledger refuses first (the transient arena, a host staging buffer, a materialized host weight), and those refusals are this
backend's own prose: "refused: a 121.7 MiB host weight does not fit the pool" names neither RESOURCE_EXHAUSTED nor OUT_OF_RESOURCES — so the better
the accounting got, the more often an out-of-memory condition went back to being "Compute error." for the client.

That is a regression introduced by improving the thing it fixed: every new refusal site adds prose that the existing classifier cannot recognise, so the
classification silently gets worse exactly as the guardrail gets better. The fix names the third identifier (kPoolRefusalToken, the token this backend writes
itself) and appends it at each refusal site — which is the right shape, because the alternative (a growing needle list of English phrases) is unmatchable.

And the kAllocationHints list is a second, weaker net with its own stated limit: "'pool' is here for our own refusals: one written without
kPoolRefusalToken is then at least named in the log." A hint that logs without reclassifying is the honest way to handle a needle you are not sure about.

The pool_tally extraction is what makes the charge testable without a device, and the arms are the four states that matter:
clear() on an empty cache leaves the ledger alone (and twice does not either) · a charge lands (base + bytes) · clear() returns exactly what was
charged · a second clear() does not release it again · a growth past the budget is refused. The "clear twice" pair is the one that catches a double release,
which in this ledger would under-count and admit a later allocation the pool cannot serve.

Not built: no llama.cpp build tree here and the HRX configure needs the private runtime, so this verdict rests on the code and the arms' assertions, not on a
run. That is a real gap and I am labelling it rather than implying otherwise.

One note for whoever lands the set: #277 (compute-time refusal classification), #278 (this) and #279 (host term read before classifying) are three halves of
one story — how a resource failure reaches the client. They touch overlapping files (failure-status.{h,cpp}, hrx-1bit-safety.h), so landing them in
sequence is easier than in parallel.

[bong-water-water-bong]

VERDICT: READY at a7fd5ad — and it is the first PR this session that declares its stack up front:

Refs engine#1823: the device-free test that #275 said it owed.
**Stacked on #277.** This branch is #277's commit plus one. Until #277 merges, the diff here shows both

That sentence is worth more than it looks. Three duplicate clusters today (ggml-org#1885/ggml-org#1883, ggml-org#1884/ggml-org#1861/ggml-org#1873, ggml-org#1876/ggml-org#1878) each cost a reviewer a git merge-base to work out whether the diffs were competing or stacked; saying it in the body turns that into reading.

The substance

"the test that #275 said it owed" is the right framing for a follow-up: #275 landed a charge and promised its test, and this pays the debt rather than making a new claim. And a device-free test for a pool tally is the only version that runs on every change — pool_tally is arithmetic over accounting, so it can be pinned without a GPU. The same session has produced three other examples of that split: #276's ledger arm, ggml-org#1858's oracle registration, ggml-org#1919's state test.

Nothing to change. When #277 merges, amend the body so the stack note becomes history rather than an instruction.

[bong-water-water-bong]

VERDICT at a7fd5ad: READY

The pool refusal now reads as an allocation failure, and it is the superset of #277 — same five files plus host-memory.h and test-hrx-staging-ledger.cpp; 39 of #277's 41 added lines are here verbatim, plus 69 more.

The change itself is two needles and one token, which is the right size for the problem: a refusal that comes from the pool at compute time says nothing about "RESOURCE_EXHAUSTED" or "OUT_OF_RESOURCES", so it was being classified as a generic failure and the server answered something other than out-of-memory. Adding kPoolRefusalToken to kResourceNeedles and "pool" to kAllocationHints makes the classification reach it without widening the match so far that unrelated errors get called OOM — the hints still require a memory word.

The shared pool_tally pair is what makes the ggml-org#1823 charge observable from both sides, which is the part that cannot be faked: a test can now see the charge rather than infer it.

CI green on this head.

[bong-water-water-bong]

VERDICT: READY at a7fd5ad — reviewed as the stacked pair with #277, and the stack is declared, which is the part that makes it reviewable.

Stacked on #277. This branch is #277's commit plus one. Until #277 merges, the diff here shows both; the commit to review is a7fd5ad.
If this merges first, #277's change lands with it.

That sentence is what every stacked PR should carry: it names the base, names the commit to review, and states the merge-order consequence in both directions. I have spent parts of tonight closing or flagging pairs where exactly this was missing (ggml-org#1790⊂ggml-org#1794, ggml-org#1896/ggml-org#1897, ggml-org#1915/ggml-org#1917), and a two-line declaration would have prevented all of them.

The provenance is the interesting part of the content: 070a270ca was pushed as #275 merged at the commit before it, so it never landed and has no PR of its own. The result is that the pin charges the default host-weight mode with a device demonstration only — #276's ledger test covers the cache's tally, not the per-weight HostWeightLease::Entry that #275 changed. Carrying the commit onto the branch head rather than rewriting the test a second time is the right call: the test already exists and was written by whoever knew the charge.

That is the third arm of the same ledger story I have reviewed tonight, and the three together are now coherent:

PR	what it establishes
#274	the charge exists (and reserve-before-alloc, refund-what-was-stored)
#276	the ledger returns to its starting figure for both charges
#278	the per-weight record gets a device-free arm, not only a device demonstration

One thing to keep: the note that a device demonstration is not a test — it proves the number on one run on one box, and it cannot fail in CI. Asking for the device-free arm here is the difference between "it worked when we looked" and "it will keep working", which is the same distinction as ggml-org#1919 (a fix merged with no arm) and ggml-org#1915 (a sweep whose scope was never asserted).

No objection. If #277 merges first this becomes a clean one-commit review; if this merges first, #277's classification change rides along and should be noted at merge so the two don't get re-litigated separately.