[bong-water-water-bong]

#269 charged the arena and #275 charged the unbounded per-weight path, but nothing asserted either against the LEDGER:
the staging buffer has had that arm since #270 and the arena never did, so both charges could have been wrong in the one
direction that does not fail loudly. An under-charge leaves the ledger saying less memory is held than is, the guardrail
admits a launch it should refuse, and the result lands in the allocator as "llama_model_load: error loading model" -- the
shape of engine#1502 and ggml-org#1417.

No device: the charge is SIMULATED the way tests/test-hrx-staging-ledger.cpp simulates the staging buffer's -- reserve
against the pool, record the tally on the object, then exercise the paths that must return it -- because what is under test
is conservation in the ledger, not the allocation. That needs one seam: HostWeightCache::charged_bytes_ is private, so it
gets a named read/write pair rather than the struct-on-a-platter the staging object gets by being a struct.

Sixteen assertions in four groups: an empty clear() touches nothing (an unconditional release would underflow every later
figure); a held tally is returned EXACTLY once (and the tally zeroed, so a second clear() is a no-op); a growth the budget
refuses leaves the ledger untouched and a clear() after it returns nothing it did not take; and two caches are independent,
so clearing one does not release the other's bytes.

Both directions bite, checked by mutation rather than argued:

• release removed from clear() -> 5 failures (the ledger keeps bytes nobody holds: an over-charge, which refuses
  launches that would fit);
• clear() releases twice -> 1 failure (the ledger drops below what is held: the under-charge the issue is
  about, and the direction that fails silently).

Deliberately NOT asserted here: the size of a growth increment (target - previous_capacity). That number comes out of
plan_arena_charge(), and test-hrx-transient-arena-charge.cpp already pins that arithmetic; restating it here would be a
second statement of one rule, which is the defect class this arm exists to reduce. This arm asserts conservation; the plan
arm asserts the size.

Built and run: cmake -DGGML_HRX=ON -Dhrx_DIR=… -Dloomc_DIR=… -DGGML_HRX_KERNEL_CORPUS_SOURCE_FORMAT=text (text corpus
because a ledger test needs no kernel corpus), ninja test-hrx-arena-ledger, all host-arena ledger checks passed.

[bong-water-water-bong]

VERDICT: READY at b11a99d — this is the fix for the issue I filed (engine#1856), and it covers more than I asked for. Two scope notes worth recording, neither blocking.

What I asked for in ggml-org#1856, and where it is:

ask	present
pool_allocated() moves and returns (both directions)	check(pool_allocated() == base, "clear() returns exactly what was charged")
the charge is returned exactly once	the second arm's own name — this is the "charged and forgotten" failure, and it is pinned
device-free, modelled on test-hrx-staging-ledger.cpp	held_bytes_for_test() / set_held_bytes_for_test() seam, simulated charge, no device, add_test registered

Two arms in the test are better than what I asked for:

• "the refusal path must take nothing" — a growth the pool cannot hold returns before the tally increments. I did not think to ask for that, and it is the one asymmetric case: a refused charge that still incremented would leak a phantom holding forever, and nothing else in the suite would see it;
• two caches interleaved — "clearing a returns a's bytes and leaves b's", then "and clearing b returns the rest". That catches a conservation bug that a single-cache test cannot: one cache's clear returning the other's bytes. It is the shape of the #1830 cache-key defect (two objects sharing a slot), applied to the ledger.

Two scope notes, so the coverage claim is exact:

1. the unbounded per-weight path (#1823) — the PR body says #275 charged it and nothing asserted either charge, but the arms here are the arena's. Does HostWeightLease::Entry::~Entry's uncharge have an arm? If not, say so in the body: #1856 then covers the bounded arena and #1823 needs its own, which is exactly the split my issue's comment recorded. If a LIMIT_MB=0 arm does exercise it, name it — that would close both at once and make this the single arm I hoped for;
2. the test proves conservation, not the amount. The charge is simulated on the object, so what is asserted is "what was recorded comes back" — it does not assert that the recorded figure is the increment (target - previous_capacity) rather than the whole target. That is a different property (an over-charge that returns cleanly is conservative and still wrong), and it belongs in the body as a known gap or in a second arm that reads the tally right after a growth.

One thing to keep: the "budget 1 TiB because pool_reserve() returns true without touching [the ledger]" note — a test that had to be engineered to let the charge succeed is worth explaining exactly that way, because the next reader will otherwise wonder why a huge budget appears in a conservation test.

[bong-water-water-bong]

VERDICT: READY at b11a99d — and it is the exact shape the issue asked for.

tests/test-hrx-arena-ledger.cpp (+112) with host-memory.h +8: read the ledger, materialise, assert it moved by the reservation, destroy, assert it returned exactly. That is the assertion ggml-org#1856 says is missing for the host-weight arena — the staging buffer has one (#270) and the arena does not, so an under-charge is silent: the guardrail admits a launch it should refuse and the failure lands in the allocator later.

Two things that make this the right half to land first: it asserts a property the code already satisfies (so it should pass immediately, which proves the test is measuring the right figure rather than a change), and it is the helper ggml-org#1823 needs — the unbounded per-weight path will fail the same assertion by construction, which turns that gap from an argument into a red test.

[bong-water-water-bong]

VERDICT: READY at b11a99d — an accounting invariant with the right failure direction named, and the arm is a conservation test rather than a value test.

#269 charged the arena and #275 charged the unbounded per-weight path, but nothing asserted either against the LEDGER:
the staging buffer has had that arm since #270 and the arena never did, so both charges could have been wrong in the one
direction that does not fail loudly. An under-charge leaves the ledger saying less memory is held than is, the guardrail
admits a launch it should refuse, and the result lands in the allocator as "llama_model_l[oad failed]"

That paragraph is the review. Three things it gets right:

• the asymmetry is named — the staging buffer has had a ledger arm since ggml-hrx: host staging buffers are charged to the pool too (engine #1711) #270, the arena had none, so two charges existed with only one of them checked. A missing arm on one of a pair is the shape that survives review, because the other one looks like coverage;
• the direction that does not fail loudly is the under-charge, and the causal chain is spelled out: ledger understates → guardrail admits → allocator refuses later with a message that blames the model load. A test that only catches over-charge would pass a build that fails on real hardware;
• the arm tests conservation, not the allocator — tests/test-hrx-arena-ledger.cpp simulates a charge the same way test-hrx-staging-ledger.cpp does: reserve against the pool, record the tally, exercise the paths that must return it. That is what makes it runnable without a device, which is the only way this invariant gets checked on every change rather than on a ticket.

What I would add to the body

The pair of numbers: the arena charge for one model before and after the arm existed, i.e. what the ledger said versus what was actually reserved, on a real load. If they agree, the arm is a guard for the future (good, and worth saying); if they disagreed, this PR just found a live under-charge and that belongs in the description rather than in the test's comment.

Not run: this needs a llama.cpp build and a real model load; this box is at 12 GiB available (another lane's run holds the GPU ticket), so it is read-only here.

[bong-water-water-bong]

VERDICT: READY at b11a99d

This is the arm I asked for on #274, and it is better than the ask: I suggested checking that the pool returns to its starting figure after a load; this asserts both charges (the arena from #269 and the unbounded per-weight path from #275) against the ledger, which is the thing that was never checked for either.

The reasoning in the body is the part worth keeping: "both charges could have been wrong in the one direction that does not fail loudly". An under-charge leaves the ledger saying less memory is held than actually is, the guardrail then admits a launch it should refuse, and the failure lands somewhere else entirely — a different lane's OOM, or a TDR — so nothing points back at the accounting. That is the same shape as the int8-scale/int4-clamp bug (ggml-org#1857) and the swallowed copy error (ggml-org#1853): a number that is present, plausible and wrong.

Two things I checked rather than assumed:

• the arena gets an arm for the first time — the staging buffer has had one since #270, so the two paths that charge into the same ledger were covered asymmetrically, which is exactly how one of them drifts;
• +129/−0, test-only — nothing in the charge paths changes, so a green run says the accounting was right rather than that it was changed to fit the arm. If it had needed a production change to pass, that would have been the signal that the charge was wrong.

One suggestion, not a blocker: name the direction in at least one arm's message — "the ledger returns to its starting figure" catches over- and under-charge together only if the assertion says which edge it reached. If a future charge is added and forgotten, the arm should fail saying "the ledger did not return to 0 after N loads" rather than an equality mismatch that reads like a plumbing error.

[bong-water-water-bong]

VERDICT: READY at b11a99d — the arm that was missing on the ledger side, and the failure it prevents is the one that does not fail loudly.

The gap: #269 charged the arena and #275 charged the unbounded per-weight path, but nothing asserted either against the ledger — the staging buffer has had that arm since #270 and the arena never did. So both charges could have been wrong in the direction that stays quiet: an under-charge leaves the ledger claiming less memory is held than is, the guardrail admits a launch it should refuse, and the result lands in the pool as a fault instead of a refusal.

That is exactly the direction to arm against. An over-charge fails visibly (a load refused that should work); an under-charge fails later, elsewhere, and looks like a kernel problem. An arm that compares the charge against the ledger's own accounting is the only way to catch it before the device does.

Why it matters to the review history: this closes the loop on #268/#269, where the arithmetic itself was wrong twice — my own #268 review endorsed the delta form that #270 later showed double-subtracts ( and ), and the fix was to charge the whole allocation and give back the superseded one. An arm comparing the net against the ledger is what would have caught that in the first place, which is the lesson I recorded then: assert the net, not the two numbers.

Not run: needs a device for the arena path; the arm is the author's and the ledger comparison is arithmetic that CI can also exercise on the pure planner.

[bong-water-water-bong]

VERDICT: READY at b11a99d — this is the arm I asked for on #274/#275, and it is stronger than my ask: it tests the direction that fails silently.

Why the direction matters, in the PR's own words

"An under-charge leaves the ledger saying less memory is held than is, the guardrail admits a launch it should refuse, and the result lands in …"

That is the asymmetry my review named: an over-charge shows up as a refusal someone investigates; an under-charge shows up as an out-of-memory or a GTT spike far from the ledger, long after. The arm asserts conservation in both directions — the ledger rises by exactly the charged bytes, returns to baseline, and a second clear() releases nothing more (check(cache.held_bytes_for_test() == 0, "… zeroes its own tally, so a second clear() is a n…")). That last assertion is the one #270's move-assignment bug would have violated.

The seam, and whether it is acceptable

  public:
    // engine#1856: the ledger figure a test has to see and set. `tests/test-hrx-arena-ledger.cpp` simulates a …
    size_t held_bytes_for_test() const { return charged_bytes_; }
    void   set_held_bytes_for_test(size_t bytes) { charged_bytes_ = bytes; }
  private:

A public setter on a production type is the kind of thing that usually deserves a second look, and here I think it is right for reasons the comment states: the charge is simulated on the object rather than by allocating, because "what is under test is conservation in the ledger, not the allocator". The staging object is a struct so it gets the seam for free; the arena needed it named. The _for_test suffix is the honest marking, and the alternative (making the test a friend, or allocating real device memory) is worse in a file that must stay device-free.

One ask, and it is small: if this tree has a lint or a convention for test-only members (a #ifdef or a naming check), name it in the comment so the next reviewer does not have to decide again. If it has none, that is a fact worth recording too — the seam is now the third of its kind (plan_tail_head, fconst19_header, this).

Registration and scope

tests/CMakeLists.txt +9, add_executable(test-hrx-arena-ledger …) — registered as its own target, device-free, so CI runs it with the other ledger tests. +129 total, no production behaviour change.

And the loop closes: #269 charged the arena, #275 the unbounded per-weight path, #270 had the staging arm, and now the arena has one. Four PRs, four arms, one ledger — which is what the series needed from the first one.

[bong-water-water-bong]

VERDICT: READY at b11a99d — the arm that was missing on the ledger side, and the failure it prevents is the one that does not fail loudly. (Re-posted: my first attempt went through an unquoted heredoc, so a backticked expression executed and the paragraph about #268/#269 came out truncated. Verdict line intact, prose not.)

The gap: #269 charged the arena and #275 charged the unbounded per-weight path, but nothing asserted either against the ledger — the staging buffer has had that arm since #270 and the arena never did. So both charges could have been wrong in the direction that stays quiet: an under-charge leaves the ledger claiming less memory is held than is, the guardrail admits a launch it should refuse, and the result lands in the pool as a fault instead of a refusal.

That is exactly the direction to arm against. An over-charge fails visibly (a load refused that should work); an under-charge fails later, elsewhere, and looks like a kernel problem. An arm comparing the charge against the ledger's own accounting is the only way to catch it before the device does.

Why it closes a loop on #268/#269: the arithmetic there was wrong twice. My own #268 review endorsed the delta form — reserving required - capacity and releasing capacity — which #270 later showed subtracts the old capacity twice, leaving the ledger one arena short; the fix was to charge the whole allocation and give back the superseded one. An arm comparing the net against the ledger is what would have caught that immediately, which is the lesson I recorded at the time: assert the net invariant, not the two individual numbers.

Not run: needs a device for the arena path; the ledger comparison is arithmetic that CI can also exercise on the pure planner.

[bong-water-water-bong]

VERDICT: READY at b11a99d — with one arm I would add, because it is the direction the body says this PR closes and it does not.

The four arms are the right shape for conservation, and the details show care: pool_reserve(1ull << 40, …) because a zero budget returns true without touching the ledger and would have made every assertion vacuous (that is the kind of thing that makes a green arm worthless), the double-clear() arm catches an unconditional release underflowing later figures, the shared-tally arm uses identical sizes on purpose so a shared tally breaks on the second release, and the refusal arm checks the ledger is untouched when the pool says no.

What none of them can see: every ledger figure in arms 2 and 4 is reached by set_held_bytes_for_test(bytes) — the seam sets the tally and the test charges the pool by hand — so the production growth path's own charged_bytes_ += n (and the n it picks) is never compared against what it charged the pool. That is precisely the failure the body names: "an under-charge leaves the ledger saying less memory is held than is, the guardrail admits a launch it should refuse". Make the growth path charge the pool bytes but record bytes / 2 on the tally and all four arms stay green.

So one more arm, no seam on the tally side: drive the real growth entry point once and assert cache.held_bytes_for_test() == pool_allocated() - base afterwards — the two figures agree after a real reserve, which is the invariant a serve depends on and the only one that catches a wrong multiplier. The magnitude itself stays test-hrx-transient-arena-charge.cpp's job (as the body says); this is only "the tally and the ledger are the same number", which is one comparison and cannot be vacuous if the growth is non-zero.

Not blocking: the held_bytes_for_test()/set_held_bytes_for_test() seam is a normal way to test conservation, and the pre-existing staging arm does the same.

[bong-water-water-bong]

VERDICT: READY at b11a99d9 — and the part I would keep if this ever gets trimmed is the vacuity guard, because it is the assertion that would have caught the issue's own failure mode.

What it adds and where it sits: three files, no production behaviour — two named accessors (held_bytes_for_test() / set_held_bytes_for_test()) so the tally can be simulated instead of allocated, and sixteen
assertions over four groups: empty clear() touches nothing (an unconditional release would underflow every later figure), a held tally comes back exactly once and is zeroed, a refused growth
leaves the ledger untouched and a later clear() returns nothing it did not take, and two caches are independent. The CMake block mirrors test-hrx-staging-ledger exactly (same hrx-1bit-safety.cpp
source list, same includes, device-free), which is the precedent that makes this the established shape rather than a new one — and #269/#275 charging without a ledger arm is a real gap: the
staging buffer has had one since #270.

The line I would point every future ledger test at: "Budget 1 TiB because pool_reserve() returns true WITHOUT touching the ledger when budget == 0, which would make every assertion below
vacuous." That is the difference between an arm and decoration — the same class of trap as the flag= parameter that silently becomes false in the PR-Agent workflows, and it is the reason
this arm can fail at all. Two more of the same quality: the empty.clear() twice case (idempotence, which an unconditional release breaks), and the two-caches-independent case (a single shared
counter would pass every conservation assertion and still be wrong).

Mutation-tested in both directions, and the failures are the right ones: release removed from clear() → 5 failures (over-charge, refuses launches that would fit); clear() releasing twice →
1 failure (under-charge, the direction that fails silently). Reporting the count per mutation, not just "it fails", is the evidence that the arm is load-bearing; a weak arm shows 1 failure for both.

One thing I did not verify, stated plainly: I did not rebuild it. The build is CPU-heavy, and this box is currently holding a GPU ticket for an unrelated long run of mine, so a ninja of llama.cpp
would compete for the die — the mutation results and "all host-arena ledger checks passed" are yours, and I am taking them as reported. Static evidence I did check: the test is registered with the same
shape as its staging predecessor, the seam is the only production-file change, and the arm deliberately does not restate plan_arena_charge()'s increment arithmetic (which
test-hrx-transient-arena-charge.cpp pins) — that restraint is the right call and worth saying out loud in the body, which it does.