fix(mlxrunner): admit against the overridden budget, without asking for eviction

[glennneuber]

Follow-up to #180, which is already on main. It let an operator lower the MLX ceiling but left the admission check reading the un-overridden available. The two were the same number before the override existed, so one check covered both — now a cap below the model size is admitted and handed straight to the subprocess.

Reproduced, not inferred

By executing Client.Load on the numbers from #180's own benchmark — gemma4:31b-nvfp4 at 18.29 GiB, 88 GiB free, OLLAMA_MLX_MEMORY_LIMIT=8589934592:

• Load returns nil even with requireFull=true
• the child receives a ceiling 10.3 GiB under the weights
• a cap of 1 byte behaves identically

Nothing downstream catches it: runner.go evaluates the weights before configureMemoryLimit runs, so the cap lands on an already-resident model, and that function only guards upward. The operator gets a runner death or a 500 at first inference instead of a refusal naming the variable they set.

The naive repair is a second bug

Changing the existing check to read vramBudget routes an operator shortfall into ErrLoadRequiredFull. sched.go turns that, under requireFull, into "evict a runner and retry". Eviction raises FreeMemory — but the operator's cap is a constant that free memory cannot move, so every retry recomputes the same budget and fails identically. That evicts every other model to satisfy a constraint eviction cannot satisfy.

So the two shortfalls stay distinct, and the ordering is load-bearing:

shortfall	check	result
physical	modelSize > available	evictable — stays ErrLoadRequiredFull
operator-set	modelSize > vramBudget	not evictable — plain error naming OLLAMA_MLX_MEMORY_LIMIT and both numbers

Tests exercise Client.Load, not the helper

The existing unit tests pass with the call site deleted — the feature could be entirely unwired and the suite would stay green. These drive Load itself.

Verified against both failure modes:

MUTANT 1 (revert to merged behaviour):
  FAIL TestLoadRefusesAnOverrideBelowTheModelSize
  FAIL TestLoadDoesNotAskForEvictionOverAnOperatorCap

MUTANT 2 (naive ErrLoadRequiredFull repair):
  FAIL TestLoadRefusesAnOverrideBelowTheModelSize  — "error must name the variable the operator has to change"
  FAIL TestLoadDoesNotAskForEvictionOverAnOperatorCap

TestLoadStillEvictsForAPhysicalShortfall is the ordering guard: a real out-of-memory must keep asking the scheduler to free something rather than reporting the operator's knob.

Not included

Other findings from the same review, left for their own change since #181 is still open and touches this area:

• a malformed OLLAMA_MLX_CACHE_LIMIT is discarded in total silence, indistinguishable from unset — the failure mode fix(mlxrunner): let an operator lower the MLX memory limit #180 was written to remove, reappearing in feat(mlxrunner): bound MLX's retained buffer cache instead of the whole card #181
• mlx.SetCacheLimit has no test at all; swapping its C call to mlx_set_memory_limit, the exact confusion its doc comment warns about, survives the whole suite
• a malformed OLLAMA_MLX_MEMORY_LIMIT is still silently discarded here too (minor, same theme)

🤖 Generated with Claude Code

[glennneuber]

This is a correct bug report against code I merged in #180, and the analysis is better than the analysis that produced the bug. Endorsing it.

I verified the test criticism rather than taking it on faith. Deleted the budgetWithOverride call site from Load, reran:

call site removed
go test -run TestBudgetOverride ./x/mlxrunner/   ok   EXIT=0

Green with the feature entirely unwired. My tests exercised a helper nobody was required to call — they pinned the arithmetic and asserted nothing about the behaviour. Driving Load itself is the right fix and I should have written it that way.

The eviction point is the part I would have got wrong. My instinct on reading "the admission check uses the wrong variable" was to point it at vramBudget and move on. That routes an operator shortfall into ErrLoadRequiredFull, and sched.go reads that as evict and retry — but the cap is a constant that FreeMemory cannot move, so every retry recomputes the same budget and fails identically, having evicted another model for nothing. Keeping the two shortfalls distinct, with the physical one still evictable, is the non-obvious half of this and it is the half that matters.

The ordering being load-bearing is worth the emphasis it gets: physical first (evictable), operator-set second (not). Reversed, a genuinely over-subscribed card would report an operator problem and never trigger the eviction that would fix it.

One small thing worth stating in the header, since #180's text does not: the failure this fixes is silent at load and loud later. Load returns nil, the model resolves, and the operator sees a runner death or a 500 at first inference — with nothing naming the variable they set. That is why a plain error naming OLLAMA_MLX_MEMORY_LIMIT and both numbers is the right shape, and why "cap of 1 byte behaves identically" is the detail that makes the severity obvious.