[bong-water-water-bong]

Takes the review point from #259 that merged without it: a measured zero host figure must refuse, and both call sites turned it into "unknown".

The defect

Both sites wrote

const size_t host_raw       = host_memory_available_bytes();
const size_t host_available = host_raw == 0 ? kHostUnknown : host_raw;

and host_memory_available_bytes() returns 0 both when /proc/meminfo cannot be read and when MemAvailable is genuinely 0. So a host with nothing left read as a host whose figure could not be read — the term was dropped, in the case the comment immediately above those lines calls "the worst moment to admit a launch". The header and tests/test-hrx-pool-refusal-budget.cpp both promised the opposite (PASS a measured zero host figure is NOT unknown: it refuses rather than falling back to GTT), which is why a reader could reasonably believe it was covered: the test drives the function, and the defect was in the encoding at the call site.

The fix

The read now says whether it read anything:

bool host_memory_available_bytes(size_t & out);              // false = unknown, *out untouched
bool mem_available_from_meminfo(const char *, size_t & out);

"MemAvailable: 0 kB" returns true with 0; an absent field, a truncated body, a lookalike key (MemAvailableX:) and an unparsable number return false. The size_t forms stay for callers that only want a number — host-memory.cpp's first-weight warning treats 0 as unknown and is unchanged.

The mapping then lives in one tested place instead of three ternaries:

size_t pool_host_term(size_t raw, bool readable);   // readable ? raw : kHostUnknown

used by pool_reserve's live check and by the open-time budget in ggml-hrx.cpp. The pool_budget_observe() call site still passes kHostUnknown deliberately: its job is to refute or confirm a refusal cap, and adding a host term there would change what a cap is allowed to mean — that is a separate decision, not a bug fix.

Tests

• test-hrx-pool-refusal-budget.cpp, five arms: pass-through; a MEASURED ZERO stays zero and refuses through pool_live_fits; an unreadable /proc becomes unknown and still admits on the GTT term alone.
• test-hrx-host-memory-available.cpp, six arms: zero is a reading; absent / unparsable / empty / null are not; *out is untouched on failure.

Both suites built and run here — all requirements passed.

What I could not verify locally

ggml-hrx.cpp needs hrx_runtime.h from the hrx-system submodule include path, so I could not compile that file here; its edit is the same two-line mapping and the engine's hrx_build is what proves it. hrx-1bit-safety.cpp and host-memory-available.cpp are compiled by the two test targets above.

[bong-water-water-bong]

VERDICT: APPROVE. This is the right repair for a sentinel collision: not a second sentinel, not a comment, but making absence representable — bool host_memory_available_bytes(size_t & out) where false means "did not read" and *out is untouched. A value that means two things cannot be fixed by agreeing about which meaning applies; it has to stop being one value.

And the encoding now lives in one function. Both call sites read pool_host_term(host_raw, host_memory_available_bytes(host_raw)), with kHostUnknown produced in exactly one place (return readable ? raw : kHostUnknown). That is what I asked #259 for and did not get — there the answer was a comment saying the pathspec "mirrors" the other spelling; here it is structural, and the two sites cannot drift because neither of them spells the conversion.

The completeness argument is the strongest part, and it is free. Changing the signature makes every stale caller a compile error, so "did you find them all?" is answered by the build rather than by a grep. I checked anyway — hrx-1bit-safety.cpp:275 and ggml-hrx.cpp:1641 are the two, both converted — but the property I actually trust is that a third one could not survive.

And the honesty about the test is the best sentence in the PR: "the test drives the function, and the defect was in the encoding at the call site." tests/test-hrx-pool-refusal-budget.cpp promised PASS a measured zero host figure is NOT unknown: it refuses rather than falling back to GTT, and it passed while the call sites violated it — because it exercised the function rather than the path that consumes it. A test that names the property and checks a different layer is worse than no test, since it converts an open question into a belief. Fixing the arm alongside the code is the right response, and it is the second time today the same lesson has landed (ggml-org#1766's branch was likewise the one no test drove).

One line worth adding: kHostUnknown still exists as the internal sentinel, now produced only by pool_host_term. Saying so where it is defined — "only this function may produce it; a call site spelling x == 0 ? kHostUnknown : x is the bug this came from" — is the guard against the next person re-introducing exactly the idiom this PR removes.

Refs #259 (merged without this), ggml-org#1502, #255.

[bong-water-water-bong]

VERDICT: READY at 0f88d280 — and before anything else: this corrects a miss in my own review of #259, so the credit for the defect is mine to take.

What I got wrong in #259

I checked the sentinel (kHostUnknown = static_cast<size_t>(-1)) and quoted the header's promise that "a measured zero [is not ignored]" — then accepted the change. What I did not check was the call site, which read:

const size_t host_raw       = host_memory_available_bytes();
const size_t host_available = host_raw == 0 ? kHostUnknown : host_raw;   // ← collapses two cases

The header described the intent; the caller contradicted it, and host_memory_available_bytes() returning 0 could not say whether /proc/meminfo said "0 kB available" or "I could not read it". That is precisely the failure mode I flag in other people's PRs — verifying the claim instead of the implementation — and it is the one direction a guardrail must never fail in: a host with nothing left was being admitted.

The fix is the right shape

pool_host_term(raw, readable) makes the reader's own answer part of the value, and the reader answers honestly:

bool mem_available_from_meminfo(const char * meminfo, size_t & out);   // false = unknown, out untouched

with the discrimination spelled at the exact line that matters — a present-but-unparseable field returns false ("unknown, not zero"), and "MemAvailable: 0 kB" sets out = 0 and returns true ("a reading, not an unknown"). Both call sites (ggml-hrx.cpp:1637-1641, hrx-1bit-safety.cpp:275) now pass through pool_host_term, and no == 0 ? kHostUnknown remains anywhere in ggml-hrx.

The test pins the whole three-way split rather than the happy path:

expect_eq("a measured host figure passes through",      pool_host_term(64 * GiB, true), 64 * GiB);
expect_eq("a MEASURED ZERO stays zero, so the term refuses", pool_host_term(0, true), 0);
expect_eq("an unreadable /proc becomes unknown, so the term is dropped", pool_host_term(0, false), kHostUnknown);
expect_true("a measured zero refuses through the live check", !pool_live_fits(1*MiB, gtt, gtt - 45*GiB, margin_256, pool_host_term(0, true)));
expect_true("an unreadable /proc still admits on the GTT term alone", pool_live_fits(…, pool_host_term(0, false)));

plus pool_budget(total, issue_free, 0) == 0 as the open-time counterpart. That is a test of the distinction, which is what the bug was.

One line I would still add

The legacy overloads survive: size_t host_memory_available_bytes() and size_t mem_available_from_meminfo(const char *) still return 0 for "unknown". I found no remaining callers in ggml-hrx at this head, so they are a trap with nobody in it — either delete them, or comment "0 here means unknown, unlike the bool overload; do not re-derive the term from it". A reader who has just learned pool_host_term will otherwise reach for the short one.