mlx: run with MLX's graph-cache thrashing check off; keep the first panic

[glennneuber]

Implements the two fixes proposed in #211 (docs(mlx): the graph-cache thrashing check kills the runner and masks itself as cudaGraphAddDependencies).

What changes

1. MLX_ENABLE_CACHE_THRASHING_CHECK=0 for the MLX runner subprocess by default (x/mlxrunner/client.go, mlxRunnerEnvDefaults, applied next to the CUDA_PATH/CUDA_HOME setEnv calls). A non-empty value exported on the server is left alone, so an operator who wants MLX's advisory back sets MLX_ENABLE_CACHE_THRASHING_CHECK=1.
2. Keep the first panic (x/mlxrunner/unwind.go): TextGenerationPipeline's five deferred cleanups go through guardClose. If a cleanup panics while the request is already unwinding a panic, the cleanup's panic is logged and the original is re-raised (wrapped with the stack captured while its frames were still live), so recoverRequest / mlxthread report the real cause instead of whatever the last close() hit. On the normal path it is a plain call; a cleanup that is the first thing to fail still propagates.

Why

MLX's CUDA backend keys a graph cache by shape; the "thrashing check" (ml-explore/mlx ollama#2600) is a lifetime miss counter that throws once misses pass 2 × MLX_CUDA_GRAPH_CACHE_SIZE (default 400). The runner cannot catch-and-continue a throw out of graph commit: the request dies, the deferred prefix-cache close() fails on the poisoned encoder, and the log blames cudaGraphAddDependencies. A think-on decode crosses the threshold in ~700 distinct prefill lengths — on every image we ship, and not introduced by the v0.32.15 sync. With the check off the LRU still evicts and nothing measurable changes. Full mechanism and measurements: docs/maxusai/mlx-thrash-check-masks-as-cudagraph.md (#211).

Verification

• go test ./... clean (golang:1.26 container); new tests: env default + operator override; guardClose keeps the first panic and still runs every cleanup, plain on the normal path, lets a cleanup's own first panic through; runRequest reports the first cause.
• Go-only binary swapped into maxusai/ollama:sync-0.32.15 (native payload unchanged vs 76918a7), fresh containers, gemma4:12b-nvfp4, MLX_CUDA_GRAPH_CACHE_SIZE=8 to provoke the fuse fast:
  • runner subprocess environment (read from /proc/<pid>/environ): MLX_ENABLE_CACHE_THRASHING_CHECK=0 present with no thrash env on the container — the default reaches MLX.
  • fix path (nothing set on the container, 120 unique-length requests): 120/120 ok, 0 fail, 0.2–0.3 s/request after the cold start; server log: cudaGraph=0 thrashing=0 runner_starts=1. Yesterday's Phase A under identical conditions: 120/120 fail (see docs(mlx): the graph-cache thrashing check kills the runner and masks itself as cudaGraphAddDependencies #211).
  • control (operator override MLX_ENABLE_CACHE_THRASHING_CHECK=1, same cache 8, n=12): running — result appended below when it lands.
  • (first request of a fresh container is the usual ~10 min cold start; everything after is 0.2–0.3 s.)

Notes

• Campaign containers that set the env explicitly (vsuite) are unaffected; images built from this commit no longer need it.
• docs(mlx): the graph-cache thrashing check kills the runner and masks itself as cudaGraphAddDependencies #211 gets a one-line cross-reference to this PR.

[glennneuber]

Reviewing as consolidator. guardClose is the more important half of this PR, and it fixes a defect in code I wrote.

I added recoverRequest / fatalRunnerError so a panic in the pipeline reached the caller as a reason instead of a dead subprocess and a Go stack. It did that — and it faithfully reported the wrong panic, because mlxthread.run() keeps whatever recover() last saw and the deferred session.close() panics on the poisoned encoder after the real fault. Every campaign stack I looked at showed the double panic through cacheSession.close, and I read the teardown as implicated. It was the reporting path.

So the guarantee I thought I had shipped was "a panic reaches you as a reason". The guarantee that actually existed was "a panic reaches you". This closes that, and it is the reason the thrash throw never appeared in any log.

Two properties I checked because they are what make it safe rather than merely better:

• it still runs every cleanup. A guard that re-raises the first panic but skips the remaining defers would trade a diagnosis for a resource leak, and the tests pin that it does not.
• a cleanup that is the first thing to fail still propagates. Otherwise the guard would swallow genuine teardown bugs, which is the failure mode of every "log and continue" wrapper.

Wrapping with the stack captured while its frames were still live is the detail that makes the re-raise useful rather than a bare error string.

On the env default

Applying it in mlxRunnerEnvDefaults next to the CUDA_PATH/CUDA_HOME setEnv calls, and leaving a non-empty operator value alone, is consistent with how #180 and #186 settled the memory-limit knob — derived by default, overridable downward, never silently overwritten. Reading it back from /proc/<pid>/environ rather than asserting the code path is the right verification: that is the same class of check as reading the applied limit= line out of the runner log, and it is what distinguishes "we set it" from "it arrived".

120/120 ok against yesterday's 120/120 fail under identical conditions is about as clean as a before/after gets.

Two things worth stating in the code comment

1. The check was telling the truth. Disabling it trades a fatal abort for a performance question nobody is currently measuring — MLX's graph cache genuinely misses a lot on these shapes. Worth one line so a future reader does not take "check off" as "thrashing solved".
2. Cite ml-explore/mlx Refactor parsing model configuration ollama/ollama#4356. It is open and unmerged; when it lands, the encoder no longer poisons itself and this default can be revisited. Without the issue number in the source, the next pin bump has no prompt to re-check.

The control arm (MLX_ENABLE_CACHE_THRASHING_CHECK=1, cache 8, n=12) is still running per the description. I would want it appended before merge — not because I doubt the result, but because the override path is the thing an operator will reach for and it is currently unverified in the same run as the default.