fix(seedless): check command buffer status, poison the backend on GPU failure - #171
Merged
Merged
Conversation
… failure (#169) A failed Metal command buffer leaves its output buffers untouched. Shared buffers are zero-initialised, so reading them anyway yields an all-zero logit row, which argmax_rows tie-breaks to index 0 — token id 0, forever. Nothing in this codebase inspected cb.status before this commit (94 waitUntilCompleted call sites, zero error checks), so a GPU OOM surfaced as a plausible-looking `<word>` followed by an unbounded run of `!`. Measured on a 16GB-class config: bolt mixed C=128 fails at ~4.0-4.3K prompt tokens, bolt generic C=128 between 6.6K and 14.7K, strict passes 41K even at an 8GB budget. cb.error read `kIOGPUCommandBufferCallbackErrorOutOfMemory` at every failing point, while the hidden state and lm_head input were healthy — the CB simply never ran. - commitAndWaitChecked(cb, label) -> String?: nil on success, a fault description otherwise. A non-nil result means the caller must not read any output buffer of that command buffer. - GPUFaultState: per-instance, sticky, first-fault-wins. Deliberately NOT process-global — one failed request must not kill a long-running server. SeedlessFusedForward owns one. - Applied to the decode readback funnels: stepArgmax (.resident/.bolt) and chainedStepArgmax, plus an entry guard so a poisoned backend refuses rather than reading stale buffers. chain advances CPU-side KV length at encode time, so continuing after a failure is unsound, not merely wasteful. - Decode path selection is now logged unconditionally. The same logical op has three implementations (raw fused step, GPU chain, MLX-composed fallback) and which one runs is a runtime decision; without this line an instrumentation attempt lands on a path that never executes. - QWISP_LOGIT_DBG (default off): per-row logit/hidden stats and cb.status, the instrumentation that found this. Not covered here, deliberately: the strict path's split command buffers, stepArgmaxBatch, prefill CBs, a static gate against bare waitUntilCompleted, and an error channel on LLMBackend.generate's AsyncStream. Today the failure stops generation and reports on stderr; it does not yet reach the caller as a typed error. The OOM itself is a separate capacity bug and stays open in #169. RAWTESTS 99/99 (new locked test asserts the checked-commit contract and that the poison is per-instance, sticky, and does not leak between backends). Refs #169 Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #169. Stops the silent failure; does not fix the OOM itself.
What this changes
A failed Metal command buffer leaves its output buffers untouched. Shared buffers are zero-initialised, so reading them anyway yields an all-zero logit row, which
argmax_rowstie-breaks to index 0 — token id 0 (!), forever. Nothing inspectedcb.statusbefore this: 94waitUntilCompletedcall sites, zero error checks.commitAndWaitChecked(cb, label) -> String?— nil on success, a fault description otherwise. Non-nil means the caller must not read that command buffer's outputs.GPUFaultState— per-instance, sticky, first-fault-wins. Deliberately not process-global: one failed request must not kill a long-running server.SeedlessFusedForwardowns one.stepArgmax(.resident/.bolt) andchainedStepArgmax— plus an entry guard so a poisoned backend refuses rather than reading stale buffers. The chain advances CPU-side KV length at encode time, so continuing after a failure is unsound, not merely wasteful.!) output above ~2–6K prompt tokens #169.QWISP_LOGIT_DBG(default off): per-row logit/hidden stats andcb.status.Evidence
16GB-class config, one prose prompt truncated to a ladder,
cb.errorread at the failing commit:At every failure:
MTLCommandBufferErrorDomain Code=8 kIOGPUCommandBufferCallbackErrorOutOfMemory, logits all zero (nan=0), whilehBufandnormedheld healthy leftover values from prefill — the CB simply never ran.Not in this PR, deliberately
stepArgmaxBatch, prefill CBswaitUntilCompletedLLMBackend.generate'sAsyncStream— today the failure stops generation and reports on stderr, but does not reach the caller as a typed error, so the OpenAI-compatible surface cannot yet return a proper error response!) output above ~2–6K prompt tokens #169Gates
RAWTESTS 99/99. The new locked test asserts the checked-commit contract and that the poison is per-instance, sticky, and does not leak between backends. Known limitation: failure is injected via a test flag, so removing only thecb.statuscomparison while keeping the injection check would not be caught — the static gate in the follow-up should close that.🤖 Generated with Claude Code