Repository navigation
preflight: gate the M5 Neural Accelerators in all three places they can be lost - #341
Conversation
…an be lost #338 landed nax_probe.m as a tool. This makes it a gate, and adds the two halves a host probe cannot see. The tensor path is worth 2.14x prefill on the GGUF arm (275.2 vs 128.6 tok/s, docs/maxusai/m5-neural-accelerators.md) and every way of losing it is silent. There are three, which is why this is three checks: metal_tensor_host ggml compiles a dummy mpp::tensor_ops::matmul2d at runtime and disables the path if it fails. nax_probe asks its four questions in ~100 ms, no model load. metal_tensor_payload a llama.cpp older than b10864, or one built without the Metal 4 source, carries no tensor kernels at all. The built llama-server holds 28 GGML_METAL_HAS_TENSOR strings; such a payload holds none. metal_tensor_runtime ours, and the one nobody was looking for: when GPU discovery hits a Metal init error, discover/runner.go retries with GGML_METAL_TENSOR_DISABLE=1 and recordPersistentRunnerEnv pins that on every runner the process spawns. One WARN at startup, then half the prefill for the life of the server. It has never fired here -- 0 in 25 MB of serve.err.log over 69 restarts -- which is exactly why it needs a check. All three are gated on a profile's expect_metal_tensor_api and skip loudly without it. Only mlx-metal-0-34-0 declares it: every older profile's llama.cpp predates b10864 and has no tensor kernels to enable, so claiming true there would assert something that was never measured. Each refuses rather than reaches. The host probe runs where the HARNESS runs, so a remote server skips instead of getting a confident answer about the wrong GPU. The payload half inspects a binary -- which README.md rightly distrusts inside the images -- so it runs only when the payload is on this machine, and it names both the path and the route it took: the path the server printed when it spawned a runner, or failing that the one the listening executable resolves to by ollama's own rule (ml/path.go). An earlier draft would have read an in-container path off the harness's own filesystem and failed a healthy containerised deploy; the test that caught it is test_a_containerised_server_skips_rather_than_reading_a_local_path. Also fixes container_logs, which this found by reading a whole log for the first time: one undecodable byte 20 MB into serve.err.log made text=True raise UnicodeDecodeError, turning every log-derived check on that host into an ERROR about a perfectly healthy server. errors="replace" -- every consumer is a regex over ASCII log lines. Verified live against the production server on :11435, all three PASS, and each fails on its own counterfactual: GGML_METAL_TENSOR_DISABLE=1 in the environment, a real binary with no tensor kernels, and a log whose current process fell back. 140 offline tests pass, including in the suiteless tree CI builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… ERRORs check_metal_tensor_runtime reached container_logs with container=None and log_cmd=None, which put a None into the docker argv and came back ERROR -- failing the whole run on a server that is perfectly healthy. mlx_payload_pin has guarded this since it was written; this now guards it the same way and says the same thing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n the matrix
Three holes, all found after the first commit.
1. The host probe answered for the wrong environment. nax_probe reads
GGML_METAL_TENSOR_* from its own process, so a server started by launchd
with DISABLE=1 would be serving at half the prefill while a probe launched
from the operator's shell reported everything fine -- and, the other way
round, an operator who exported the variable to test something would have
every later run report a healthy server as degraded. It now reads the two
variables off the listening process (ps -wwE) and runs the probe under
those, deleting rather than inheriting the ones the server lacks. Verified
both ways against :11435. `ps -wwE` prints nothing for another user's
process, which would read as "the server has nothing set", so an unreadable
environment is None, never {}, and the result says which environment it
answered for.
2. A non-Metal profile was told to add a Metal field. cuda and rocm have no
tensor API to lose, so they now skip as "does not apply" -- the exact phrase
release_matrix.effective() maps to a neutral N/A rather than a coverage gap.
A Metal profile that declares nothing still gets the loud "add the field".
3. The gate rendered nowhere. A check absent from release_matrix.GROUPS is
simply not in the fold's headline artifact -- the quiet version of the false
green that column mapping already cost us once. The three checks are now one
"M5 tensor path" column, reported at its weakest, N/A where there is no
Metal. Every recorded run predates the checks and regenerates as "not run",
which is true; README.md's embedded table is the verbatim v0.34.1 output and
is NOT hand-edited to match a later generator -- the note above it says so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Three more holes found after the first commit, all closed in 1 — the host probe was reading the wrong environment.
2 — a native run without 3 — the gate rendered nowhere. A check absent from Every run recorded so far predates these checks and regenerates as README.md's embedded table is the verbatim v0.34.1 generator output and is not hand-edited to match a later generator — the note above it now says so, and the column will appear at the next fold's regeneration. 149 offline tests, in-tree and in the suiteless copy CI builds. |
…ured says no Four defects in one guard in two days, all of them false alarms on a healthy host and one of them also a false green on a broken one. That is a rule, not a run of bad luck: a guard exists to be believed, and the second false red is when the operator starts ignoring it. H20 keeps the three states apart in the code rather than in the operator's head, requires a check that reads an artefact to name the artefact and the route it took there, requires it to refuse when that route would answer about a different machine, binary or process than the one under test, and requires a new check to join a release_matrix column in the same change -- a check nothing renders is a check nobody reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same argument the payload half already makes, and the same assumption that produced its bug: a container published on a local port passes the localhost test while lsof finds the port forwarder and the probe describes the host kernel. No Metal profile runs in a container today -- which is exactly what was assumed about the payload half before it was caught reading a container path off the harness's own disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…versarial review
An Opus review of the branch went after false greens specifically. Six landed.
The worst one I had already seen and misread as confirmation.
1. THE PAYLOAD CHECK WAS READING A STALE BINARY AND CALLING IT THIS RUN'S.
`cat <serve log>` cannot substitute {since}, so container_logs returns the
whole file and launched_runner_paths was picking from it unfiltered. Measured
just now against production: 522 launch lines in the file, 0 in a real
five-second window. It now filters per line on each line's own slog
timestamp, exactly as mlx_build has since 2026-08-30 -- whose docstring
records this same trap costing a PASS from a month-old line. I read my own
warning and walked into it anyway: the earlier "both routes agree on the
binary" was luck, not corroboration.
2. server_env HARVESTED =-TOKENS FROM ARGV. `sh -c 'OLLAMA_HOST=... ollama
serve'` would have been parsed as environment -- and worse, would have made
an unreadable environment look readable, so the check would DELETE the two
variables it exists to honour. The environment is now the suffix after the
same process's plain `command=`. (The review's headline claim, that
`ps -wwE` never prints an environment on macOS, is wrong: it was tested with
/bin/sleep, a platform binary, the one class the kernel hides. An ollama
server prints all 13 of its variables, verified both ways.)
3. AN UNREADABLE ENVIRONMENT FELL BACK ON THE OPERATOR'S SHELL. The m5 doc's own
A/B has them export GGML_METAL_TENSOR_DISABLE=1; preflight from that shell
would have failed a healthy host, and an exported _ENABLE would have masked a
real loss. Both variables are cleared instead.
4. binary_marker_count TURNED "CANNOT READ" INTO "NO KERNELS". `grep -c` prints
0 on empty input and `|| true` swallowed the status, so a missing file or a
missing `strings` read as FAIL -- contradicting the host check, which
deliberately SKIPs when the toolchain is absent. It raises ProbeError now.
5. THE RUNTIME ANCHOR IGNORED THE PORT. Four ollama servers share this host.
Anchoring on the last "Listening on" of whatever log was named attributed
another server's discovery to the build under test, and -- being the LAST
one -- pushed a fallback logged by the server under test outside the window,
reading PASS. The anchor line carries the port; it is matched on now, and the
skip says which ports the log did contain.
6. lib_ollama_llama_server DIVERGED FROM ml/path.go TOWARD ANSWERING. It walked
past a candidate directory that path.go stops at, and searched beside a
symlink where path.go EvalSymlinks first -- which is the deploy flow's
archive-and-swap exactly. Both mirrored now.
Plus: a tunnelled port (ssh -L) made a remote server look local, so all three
now require the listening executable to be ollama; the payload FAIL diagnosis
was written for one direction only; and three comments claimed things the code
no longer did.
163 offline tests, 13 of them new and written first. All three checks still PASS
live against production on :11435, now through the fallback route rather than
the stale-log one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
I put an adversarial reviewer on this branch with one instruction: hunt false greens. It found six, fixed in 1. The payload check was inspecting a stale binary and labelling it "launched by this run"
It now filters per line on each line's own slog timestamp — exactly as 2.
|
The same defect, twice, from the same cause: a --log-cmd that cannot
substitute {since} returns the whole file, so the caller's window is inert.
2026-08-30 it was a month-old engine-init line passing a five-second window;
2026-09-20 it was 522 stale runner-launch lines, none of them in the window,
one of them labelled "launched by this run". The fix the first shipped lives
in mlx_build's docstring, three functions from the second, and was read before
the second was written -- so it goes in the spec.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…first' The kickstart window: the outgoing and incoming servers both hold the port for a moment, and lsof lists both. Taking pid[0] would let a dying process supply the binary and the environment for a verdict about the new one -- and the two lsof calls behind exe and env could disagree about which. Both now return None, which every caller already treats as a skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Merged as The log route, proven on a real runner launchA peer's The route flips at the window boundary, and both name the same binary. That agreement means something now — before The end-to-end runProduction's own binary served on The runtime check anchored on Production was healthy throughout and its resident model was not evicted; the I did not commit the run artifact, deliberately. |
Closes the follow-up #338 left open: "wire it into
preflight.pyas a check, with an expectations field so a host that should accelerate and doesn't fails the gate rather than merely printing. Pair it with astringsassertion that the builtllama-servercarriesGGML_METAL_HAS_TENSOR, so the build half is covered too."Both of those, plus a third half that only turned up while wiring the first two.
Three checks, because there are three ways to lose it
The tensor path is worth 2.14× prefill on the GGUF arm (275.2 vs 128.6 tok/s,
docs/maxusai/m5-neural-accelerators.md) and every way of losing it is silent — the only symptom is half the prefill.metal_tensor_hostnax_probe.mreplays ggml's own four-stephas_tensordecision — Metal 4 family,GGML_METAL_TENSOR_DISABLE, the M5/M6/A19/A20 name allowlist, a runtime compile of its dummymatmul2d— in ~100 ms with no model load, run under the serving process's own environment (ps -wwE) rather than the operator'smetal_tensor_payloadllama-serverthis server actually runs carriesGGML_METAL_HAS_TENSORmetal_tensor_runtimeGGML_METAL_TENSOR_DISABLE=1The third one is ours, and nobody was looking for it
discover/runner.go: when GPU discovery hits a Metal init error thatllm.ShouldRetryWithMetalTensorDisabledrecognises — including, pointedly,"input types must match cooperative tensor types"— it retries withGGML_METAL_TENSOR_DISABLE=1, andrecordPersistentRunnerEnvthen pins that variable on every runner the process spawns afterwards. OneWARNat startup, then half the prefill for the life of the server.It has never fired on this host: 0 occurrences in 25 MB of
serve.err.logover 69 restarts. That is the argument for checking it, not against — a fallback that never happens is invisible until the day it does, and nothing else about the server would look wrong.Live, against the production server on :11435
and each one red on its own counterfactual —
GGML_METAL_TENSOR_DISABLE=1in the environment, a real binary carrying no tensor kernels, and a log whose current process fell back:The driver wiring is proven live too, on a run that loads no model (
--arch nosuch, which errors at the arch gate after the early checks report).What it refuses to do
Each half declines rather than guesses, because a confident wrong answer here is worse than none:
GGML_METAL_TENSOR_*variables it honours are read off the listening process, not the shell — a server started by launchd withDISABLE=1is unaccelerated however capable the machine is, and an operator who exports the variable to test something must not thereby fail every later run. A probe that cannot be built (no clang) also skips — that is a gap in the harness, not a verdict about the host, and failing there would train us to ignore the check.README.mdrightly distrusts inside the images. So it runs only when the payload is on this machine, and it names both the path and the route it took to it: the path the server printed when it spawned a runner, or failing that the one the listening executable resolves to by ollama's own rule (ml/path.go). An earlier draft would have read an in-container path off the harness's own filesystem and failed a healthy containerised deploy;test_a_containerised_server_skips_rather_than_reading_a_local_pathis what caught it.expect_metal_tensor_apigates all three, and onlymlx-metal-0-34-0declares it. Every older profile's llama.cpp predates b10864 and has no tensor kernels to enable, so claimingtruethere would assert something that was never measured.A pre-existing bug this surfaced
container_logsdecoded withtext=True. One undecodable byte 20 MB intoserve.err.logmade that raiseUnicodeDecodeError— turning every log-derived check on this host into an ERROR about a perfectly healthy server. The windowed readers had been lucky; a reader of the whole file is not. Nowerrors="replace": every consumer is a regex over ASCII log lines.Tests
164 offline tests pass, in-tree and in the suiteless copy CI builds — 114 on
main, so 50 are new, each written before the code it tests. Two of them exist because the implementation went wrong first — the containerised-path false alarm above, and the host tests, which pinsys.platformso they answer the same on the ubuntu runner as on the Mac.Ten more defects were found after the first commit and are written up in the comments below — six of them by an adversarial review that went hunting for false greens, including a payload check that was reading a stale binary out of the log and calling it this run's.
release_matrix.pynow carries an M5 tensor path column, and SPEC H20 records the rule they share.One gap, stated plainly: the payload check runs after the arch loop, for the same reason
mlx_payload_pindoes — the authoritative path to the binary is the one the server prints when it spawns a runner, and none has spawned before then. That placement is covered by unit tests and by a direct live call, but not by a fullpreflight.pyrun, because every model inmlx-metal-0-34-0is MLX and a full run would put a 12B+ load on the shared GPU with no coordination channel open.🤖 Generated with Claude Code