Skip to content

fix(inference): five issues where the agent reported something untrue about local inference (#263, #225, #70, #35, #203, #69) - #773

Merged
gen16k merged 5 commits into
mainfrom
fix/inference-honest-reporting
Aug 13, 2026
Merged

fix(inference): five issues where the agent reported something untrue about local inference (#263, #225, #70, #35, #203, #69)#773
gen16k merged 5 commits into
mainfrom
fix/inference-honest-reporting

Conversation

@gen16k

@gen16k gen16k commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Five inference issues, one per commit so a revert can be surgical. They are bundled rather than split because every same-repo PR here runs the full 3-OS install test and CI is concurrency-limited — splitting them would have cost five times the CI for the same diff.

Every issue's premise was re-checked against origin/main before implementing, and four of the five had moved. Three would have produced the wrong change if implemented as written. Each section says what was stale.

#263 — the hf CLI guarantee was nominal

The only issue whose description was still accurate.

huggingface_hub[cli] was pinned with a comment saying the extra made the console-script entry point "a hard guarantee rather than a transitive accident". huggingface_hub 1.x removed that extra; uv warns and carries on:

warning: The package `huggingface-hub==1.25.1` does not have an extra named `cli`

So it resolved to plain huggingface_hub. The venv still worked — 1.x ships the scripts in the base package — which means it was working for exactly the reason the pin existed to stop relying on. ResolveHFCLI shells out to that binary for safetensors downloads, so losing it breaks vLLM model pulls.

The request now states the real requirement (huggingface_hub>=1.0), and the verify stage asserts the binary is actually there, looking for the same two names in the same order resolveVenvHFCLI will later resolve. That assertion is the part that cannot go stale: it holds whether the extra exists, comes back, or the scripts move again. Before it, a venv with no downloader passed verify clean and failed at the first pull.

The ninja pin's comment referred back to huggingface_hub's justification, so it is restated rather than left pointing at withdrawn reasoning.

Test gap this exposed: the happy-path assertion never named huggingface_hub, which is why nothing noticed the extra had stopped existing. It does now, and rejects any huggingface_hub[...] form. ResolveHFCLI's PATH lookup order was also untested — the one existing case is the override, which returns before any lookup — so the hf → huggingface-cli chain is now a table test.

#225 — stale premise: the PATH probe was already fixed

The issue says the field is a LookPath("vllm") probe that cannot see the venv. #238 already fixed that by injecting engineVersionOnHost into the daemon's profiler. The proposed call signature does not exist either (engineInstalledOnHost takes three arguments and no cfg), and the cited exemplar is unrelated code.

What is still real, and why it is still worth fixing:

So the arm gets the seam ollama has had since #188, with the profile as the nil fallback. Symmetry is the fix — the defect was two engines answering one question by different means. Full premise correction is on the issue.

Coverage gap: engineInstalledOnHost's positive vLLM direction was unasserted anywhere. The portable test pins only the absent case, for a good reason (the installer is stubbed on windows/darwin) — but that left the direction that matters with no bar. It now exists in the linux-tagged file, with sealPATH so a PATH-shaped answer cannot pass.

#70 + #35 — one defect, and #35's stated fix would have changed nothing

resolveBackendWithProbe returned immediately unless plan.Probes(), so only the two-step plans ever verified that a model reached VRAM. Every single-step GPU host — cuda, vulkan, metal, Strix-Halo-Windows — skipped it, and a detected GPU that failed to engage kept reporting its GPU backend while inference ran on the CPU.

#35 is the same defect on darwin. Its proposed fix — gate Accelerators{Metal:true} behind a probe — would have changed nothing: Accelerators has no production reader at all, and the metal plan comes from GPUs[0].Vendor == "apple". The reachable half is the mislabel, so both issues close through one mechanism and one test table.

Two deliberate restraints:

  • No forced model load on a single-step plan. The verdict there can only relabel, a cold load costs up to probeLoadTimeout, and the engine is known to restart under a running screen on its own. Multi-step plans still load, since an unverified plan there would leave a working GPU path untried.
  • A plan that already says cpu is not probed. No claim to check, nothing below it to fall to.

Removing the caller's Probes() gate made an unguarded Preferred() reachable — it indexes Steps[0], and !Probes() had been shielding the zero-value plan. An empty plan now declines with "", and the caller leaves the boot seed rather than clearing it.

#35's own remaining half is in detectApple: a system_profiler failure produced a device named "Apple GPU" and no error, which is the ABSENT/UNKNOWN conflation VendorDetector forbids and docs/decisions/20260728/0250 ruled on. Reporting the device on architecture alone stays correct (arm64 is the driver-level fact, and Metal:true is unchanged), but an unreadable name is now a warning joined into Profile.Errors alongside the device. That decision moved into an untagged appleGPUModel so it is table-testable from any host — detectApple had no test on any platform before this.

Correction to an earlier claim: no test is inverted here

I said this PR would invert TestResolveBackendWithProbe_SingleStepNoProbe. It does not. That test used an unreachable URL, so it still passes — but for a different reason: "we never looked" became "we looked, it was inconclusive, so the backend stands". It is renamed to say that, and real coverage added: the relabel on cuda and metal, an engaged host keeping its backend, no load forced where only a label can change, multi-step still spending one, the cpu plan skipped, and the empty plan.

#203 — two of three proposals were already implemented

Proposal 1 (split the classes) and the "indefinite de-rating" half of proposal 3 are done and pinned by tests that cite the issue. Capacity=1 is no longer a silent de-rate either — it is unmeasuredCapacity, with the wire's 0=UNLIMITED reason written beside it (#738).

Proposal 2 is not done, and is broken on two surfaces:

  • The wizard's benchmark row flattened every failure to SetupErrorInternal while SetupErrorEngineNotReady sat unused. An operator whose engine had not finished installing was shown an internal error. Every other place in this path draws the line — the readiness gate, the 425 door, runBenchmarkJob refusing to record it — only the projection did not.
  • The boot benchmark reached no surface at all. It warn-logs and returns: no record, no status, nothing in SetupProgress. The failure this issue actually reported (dial tcp 127.0.0.1:9475: refused, from a failed engine install) was visible only in the daemon log.

The second is why the error-code fix alone was not enough: the benchmark row only exists on a host the control plane asked for a generation, which a boot run is not. That was established by the #753/#756 session while reviewing this — snapshot() gates on d.benchmarkGen > 0.

So BenchmarkStatus now reports the boot result when there is nothing else to report, and never persists it. A gen-0 boot failure written to catalog.State would overwrite a good higher-generation record and — because a gen-0 write keeps the stored generation — show that generation as failed.

Two endings are excluded, and that exclusion is load-bearing: a skipped run is a deliberate Capacity 0, and an engine-not-ready run is what a fresh install looks like while init is still installing. Reporting either would turn a normal first boot into a visible failure — this issue's own mistake, arriving from the other direction.

Not doable here: making the de-rating visible on the node view is the admin UI in the private repo; waired#1202 holds the adjacent question.

#69 — the reader, paying the debt #772 declared

#772 landed the contract inert. This fills it in.

memory.free is appended to the nvidia-smi query so every existing field index is untouched and the basic retry keeps its shape. On Windows nothing new is asked for at all — nvmlDeviceGetMemoryInfo already returned {total, free, used} and the free member was being discarded.

The reading is frozen after the first one, per device. This is the load-bearing part: the profile is re-sampled on a TTL, including after our own engine has loaded weights, and a free reading taken then excludes those weights — the budget shrinks, the next re-tune sizes against the smaller budget, and the reading after that is smaller again. RAMAvailableGB names this hazard in the same words (#568). Frozen values are keyed by UUID (enumeration order is not stable), and an unreadable figure is not remembered, since 0 means "unknown" downstream and freezing it would make a host permanently unmeasured.

Stated consequence: a machine that frees VRAM later does not get the larger budget until restart. Same trade #568 accepted, and the stale figure is the pessimistic one.

vram-free-v1 is declared unconditionally beside the other build-level capabilities — it says this build understands the field, which is true of a host whose driver reports nothing.

The ordering proto → agent → CP is a safety requirement here, not a convention: a poller receiving a field it does not know drops it on canonical re-marshal and fails signature verification. The CP half — stripping vram_free_mb for undeclared pollers — lands with the tag bump in the private repo.

Docs updated rather than declined

docs-surface-guard does not fire for this diff (the change is in internal/hardware/, not a listed surface), but its own reasoning applies: "a machine that used to be handed a 22.6 GB model and is now handed a smaller one saw a change no printed string announced." choose-a-model gains the explanation on the English page and the ja mirror, using the graphics-memory term TRANSLATION.md pins, with the sourceHash recorded via i18n:accept (31 pairs in sync).

Debt entries deleted

All three declared by #772 — notPublishedByAgent and two protoconsumer entries — are gone, which is how the guard says the debt is paid.

Verification

Full local gate, all green:

  • scripts/dev/ci-lint-local.sh — 20 of 20
  • protoconsumer — OK (292 exported proto fields, 230 with a producer, 62 declared) — 3 more producers, 3 fewer declarations than before
  • gofmt -l clean · go vet ./... clean · golangci-lint run (v2.12.2, after cache clean) → 0 issues
  • go test ./... -timeout 10m — pass · go build -tags prod ./... · make verify-cross — exit 0
  • GOOS=darwin go vet over the touched packages, since gpu_apple_darwin.go is invisible to a linux vet
  • node scripts/i18n-sync.mjs --check — 31 page pairs, all in sync

Coordination

No docs-not-needed declaration is made here: docs-site is updated for #69, and the other four change no user-visible surface.

Fixes #263
Fixes #225
Fixes #70
Fixes #35
Fixes #69
Refs #203

gen16k added 4 commits August 13, 2026 09:37
@gen16k
fix(vllm): make the downloader guarantee real instead of declared (#263)
e299d69 
The install pinned `huggingface_hub[cli]`, and the comment beside it
said why: the extra was supposed to make the console-script entry point
a hard guarantee rather than a transitive accident, so a resolver change
could not leave the venv without a downloader.

huggingface_hub 1.x removed that extra. uv warns and carries on —

  warning: The package `huggingface-hub==1.25.1` does not have an extra
  named `cli`

— so the request resolved to plain huggingface_hub. The venv still had
`hf` and `huggingface-cli`, because 1.x ships the console scripts in the
BASE package, which means it was working for precisely the reason the
pin was written to stop relying on. ResolveHFCLI shells out to that
binary for the safetensors download, so losing it breaks vLLM model
pulls rather than a convenience.

Two changes, of different kinds. The request now states the real
requirement, `huggingface_hub>=1.0` — the version from which the scripts
are in the base package — with the upper end left to uv so it still
resolves what vllm pins. And the verify stage, which already exists,
now asserts a console script is actually present beside the venv's
interpreter, looking for the same two names in the same order as
resolveVenvHFCLI so that what verify accepts is what the daemon later
resolves.

The assertion is the part that cannot go stale. It holds whether the
extra exists, whether it comes back, and whether the scripts move again
— the reverse failure the issue also names. Before it, a venv with no
downloader passed verify clean and failed at the first safetensors pull,
several minutes and one engine start later.

The ninja pin's comment referred back to huggingface_hub's justification
for its own, so it is restated rather than left pointing at withdrawn
reasoning, and says why it gets no matching assertion: it is not a
console script resolved by name, and flashinfer's import fails loudly at
engine start.

Tests: the happy-path pip assertion never named huggingface_hub at all,
which is why nothing noticed the extra had stopped existing — it now
does, and rejects any `huggingface_hub[...]` form. ResolveHFCLI's
PATH lookup order was likewise untested (the one existing case is the
override, which returns before any lookup), so the `hf`-then-
`huggingface-cli` chain is now a table test over both names present,
each alone, and neither.

Refs #255

Signed-off-by: gen16k <gen16k@users.noreply.github.com>
@gen16k
fix(agent): ask the one engine-presence rule about vllm too (#225)
1cfcb16 
hasUsableEngine's vllm arm read hardware.Profile.Engines.VLLM.Installed
directly, which made it the last engine-presence site not routed through
engineInstalledOnHost. engineViable and setupEngineState both ask that
way; this one did not, so a vllm-only host could answer the no_engine
question from a different rule than the rest of the daemon uses.

The issue's stated cause has since been fixed and is worth correcting:
that profile field is no longer a LookPath("vllm") probe. #238 injects
engineVersionOnHost into the daemon's profiler, so it does resolve the
venv under the state dir. What remains is the divergence itself and one
concrete consequence — the profile is TTL-cached for 30 s, and
engine_resolve.go already says in as many words why that is not good
enough for this question: still LATE for a fresh install, which is
exactly what the wizard could not tolerate (#179). A venv that appeared
during setup could be reported as no_engine for half a minute after the
host could serve.

So the arm gets the same seam ollama has had since #188: a vllmUsable
func on the provider, wired to engineInstalledOnHost(GOOS, stateDir,
RuntimeVLLM), with the profile kept as the nil fallback so unit fixtures
that construct the provider directly keep working. Symmetry is the
point — the defect was that the two engines answered the same question
by different means.

Tests. TestHasUsableEngine gains the resolver column and three rows, the
first of which is the case the old shape could not express: a venv the
daemon can resolve right now on a host whose cached profile has not
caught up. And engineInstalledOnHost's positive vLLM direction was never
asserted at all — the portable test pins only the absent case, because
the installer is stubbed on windows and darwin — so the bar now exists
in the linux-tagged file where fakeVLLMVenv already lives, with an
emptied PATH so a PATH-shaped answer cannot pass it.

Refs #179, #205, #238

Signed-off-by: gen16k <gen16k@users.noreply.github.com>
@gen16k
fix(inference): verify every backend plan engaged the GPU, not only t…
640e0ad 
…he ones with a fallback (#70, #35)

resolveBackendWithProbe returned immediately unless plan.Probes(), so
only the two-step plans — Strix Halo Linux and discrete AMD with ROCm —
ever checked that a model actually landed in VRAM. Every single-step GPU
host (cuda, vulkan, metal, and Strix Halo on Windows) skipped the check
entirely. A DETECTED GPU that then fails to ENGAGE — a broken driver
runtime, VRAM already spoken for — kept reporting its GPU backend while
inference ran on the CPU, which is exactly the silent fallback
entry.Backend's own comment promises to make visible.

The probe now runs for every plan and decides for itself what a verdict
may change: a restart where there is a fallback, the label alone where
there is not. A single-step host has no better backend to try, so the
correction is honest reporting rather than recovery.

#35 folds in here rather than needing its own mechanism. Its stated fix
— gate Accelerators{Metal:true} behind a usability probe — would have
changed nothing: Accelerators has no production reader at all, and the
metal plan is chosen from GPUs[0].Vendor == "apple". The reachable half
of that issue is a Metal backend that reports itself while the engine
runs on the CPU, and that is this same defect on darwin. It is now
covered by the same probe and the same test table.

Two things this deliberately does not do. It does not force a model load
on a single-step plan: the verdict there can only relabel, a cold load
costs up to probeLoadTimeout, and the engine is known to restart under a
running screen on its own — pushing a load into that window is the "make
it worse" the file header forbids. Multi-step plans keep loading exactly
as before, since an unverified plan there would leave a working GPU path
untried. And a plan that already says cpu is not probed at all: no claim
to check, nothing below it to fall to.

Removing the caller's Probes() gate made an unguarded Preferred() call
reachable — it indexes Steps[0], and !Probes() had been shielding the
zero-value plan a provider built without a boot plan carries. An empty
plan now declines with "", which is what ResolvedBackend already means
by "not decided", and the caller leaves the boot seed in place rather
than clearing it.

#35's own remaining half is in detectApple: a system_profiler failure,
or an answer naming no GPU, produced a device called "Apple GPU" and NO
error. That is the ABSENT/UNKNOWN conflation VendorDetector's contract
forbids and docs/decisions/20260728/0250 ruled on. Reporting the device
on architecture alone stays correct — every Apple Silicon part has an
integrated GPU, so arm64 IS the driver-level fact, and Metal:true is
unchanged — but an unreadable name is now a warning joined into
Profile.Errors, alongside the device, which is the documented shape for
a real device whose details could not be read.

That decision moved into an untagged appleGPUModel so it can be table
-tested from any host: detectApple is darwin-only and had no test on any
platform before this.

Tests. TestResolveBackendWithProbe_SingleStepNoProbe asserted the right
outcome for a reason that no longer holds — it passed because nothing
was ever checked — so it is renamed to say what it now pins: an
unreachable engine is inconclusive, and inconclusive keeps the backend.
New cases cover the relabel on cuda and metal, an engaged single-step
host keeping its backend, that no load is forced where only a label can
change, that multi-step still spends one, the cpu plan being skipped,
and the empty plan.

Refs #290, #67

Signed-off-by: gen16k <gen16k@users.noreply.github.com>
@gen16k
fix(agent): say which ending a benchmark had, and stop losing the boo…
7ae5d97 
…t one (#203)

Two of this issue's three proposals are already implemented and pinned
by contract tests — the readiness gate that keeps "the engine is not up"
out of failBench, and the live Capacity getter that ended the indefinite
de-rating. Capacity=1 is no longer a silent de-rate either: it is
unmeasuredCapacity, with the wire's 0=UNLIMITED reason written out
beside it (#738). What is left is proposal 2, surfacing the result, and
it is broken on two different surfaces.

The wizard's benchmark row flattened EVERY failure to
SetupErrorInternal, while signer.SetupErrorEngineNotReady sat unused in
the enum. So an operator whose engine had not finished installing — the
ordinary shape of a first run, and the exact case this issue reports —
was shown an internal error. Every other place in this path already
draws the line: RunBootBenchmark gates before it dials, the management
API answers 425 rather than 503, and runBenchmarkJob refuses to record
that ending at all. Only the projection did not, for want of the outcome
reaching it. BenchResult.Outcome was purely internal, so it is now
carried on catalog.BenchmarkRecord and BenchmarkStatusResponse, both
additive and both `omitempty` — an older record reads as "unknown
ending" and keeps the old flattening rather than being guessed at.

The second surface is that the boot benchmark reached NO surface. It
warn-logs and returns: no record persisted, BenchmarkStatus untouched,
nothing in SetupProgress, and BenchResult.Err never read again. The
failure this issue reported — `dial tcp 127.0.0.1:9475: refused`, caused
by a failed engine install — was observable only by reading the daemon
log. That is also why the error-code fix alone would not have covered
it: the benchmark row only exists on a host the control plane asked for
a generation, which a boot run is not.

So BenchmarkStatus now reports the boot result when there is nothing
else to report. Reported, never persisted, and that is deliberate: a
gen-0 boot failure written into catalog.State would overwrite a good
higher-generation record, and because a gen-0 write keeps the stored
generation it would then show THAT generation as failed — the shape
runBenchmarkJob's own comment warns about. Filling the gap only while
the status is otherwise idle touches exactly the case that was silent.

Two endings are excluded from that, and the exclusion is the load-
bearing half. A skipped run is a deliberate Capacity 0 for a routing-only
node or an external endpoint, which three separate places document must
not read as a fault. An engine-not-ready run did not reach the engine,
which is what a fresh install looks like while init is still installing —
it self-heals minutes later. Reporting either would turn a normal first
boot into a visible failure, which is this issue's own mistake arriving
from the other direction.

Not done here, and not doable here: making the de-rating visible on the
node view. That is the admin UI in the private repo, and waired#1202
already holds the adjacent question of what a served Capacity of 0 means.

Tests live in a new file rather than the existing setup_* ones, which an
in-flight PR is already editing.

Refs #738, #307, waired-ai/waired#1202

Signed-off-by: gen16k <gen16k@users.noreply.github.com>
@gen16k gen16k mentioned this pull request Aug 13, 2026
Closed
@github-actions

github-actions Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown

📘 Docs preview — the preview channel for this PR has been deleted now that it is closed.

@gen16k
feat(hardware): read free VRAM and size the ollama budget on it (#69)
49ca569 
Pays the producer debt the proto PR declared. The contract landed inert
because nothing filled it in; this is the reader, the two adapters, and
the capability declaration that lets the control plane serve the field.

nvidia-smi gains memory.free, APPENDED to the query rather than placed
beside memory.total so every field index the parser already uses stays
where it was — the basic retry keeps its 3-field shape, and a driver
that rejects memory.free falls back to it and reports the device with
the free figure simply unknown. On Windows nothing had to be asked for
at all: nvmlDeviceGetMemoryInfo already returned {total, free, used} in
one call and the free member was being discarded.

The reading is frozen after the first one, per device, and that is the
load-bearing part rather than an optimisation. This profile is
re-sampled on a TTL, including long after the agent's own engine has
loaded weights, and a free reading taken then EXCLUDES those weights: the
budget would shrink, the next re-tune would size against the smaller
budget, and the reading after that would be smaller again — a spiral
driven by the host's own success at serving. signer.HardwareSummary's
RAMAvailableGB names this hazard in the same words and answers it the
same way (#568). Frozen values are keyed by UUID because enumeration
order is not guaranteed stable and replaying one card's figure onto
another would be worse than having none, and an UNREADABLE figure is not
remembered: 0 means "unknown" everywhere downstream, so freezing it
would make a host permanently unmeasured rather than temporarily.

The stated consequence is that a machine which frees VRAM later does not
get the larger budget until the agent restarts. That is the same trade
#568 accepted and it errs in the safe direction — the stale figure is
the pessimistic one.

vram-free-v1 is declared unconditionally beside the other two build-level
capabilities: it says this build UNDERSTANDS the field on peer entries,
which is true of a host whose driver reports nothing. Until the control
plane strips the field for pollers that have not declared it, the
ordering proto → agent → CP is a safety requirement here rather than a
convention: a poller that receives a field it does not know drops it on
canonical re-marshal and fails signature verification.

Docs are updated rather than declined. docs-surface-guard does not fire
for this diff — the change is in internal/hardware, not one of the listed
surfaces — but the guard's own reasoning applies: a machine that used to
be handed a model sized for its whole card is now handed one sized for
what is free, and no printed string announces that. choose-a-model gains
the explanation on both the English page and the ja mirror, using the
graphics-memory term TRANSLATION.md pins, with the sourceHash recorded.

Refs #568, #264

Signed-off-by: gen16k <gen16k@users.noreply.github.com>
@gen16k
gen16k force-pushed the fix/inference-honest-reporting branch from 114b216 to 49ca569 Compare August 13, 2026 00:53
@gen16k
gen16k merged commit 1630795 into main Aug 13, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment