Skip to content

Studio: retry as one sequence when llama.cpp refuses a unified KV cache - #10371

Merged
danielhanchen merged 7 commits into
mainfrom
studio/retry-without-kv-unified
Sep 7, 2026
Merged

Studio: retry as one sequence when llama.cpp refuses a unified KV cache#10371
danielhanchen merged 7 commits into
mainfrom
studio/retry-without-kv-unified

Conversation

@danielhanchen

@danielhanchen danielhanchen commented Sep 6, 2026

Copy link
Copy Markdown
Member

The bug

Studio appends --kv-unified on its own whenever it asks for more than one slot, so llama.cpp does not split -c into per-slot windows (_ctx_integrity_flags: if n_parallel > 1 and caps.get("supports_kv_unified")). Some architectures need one sequence per stream and cannot build a context that way. In the GLM-5-Next port (ggml-org/llama.cpp#27754) the pooled indexer resolves its geometry like this:

const int64_t n_stream = cparams.kv_unified ? 1 : ubatch.n_seqs_unq;
const int64_t n_ps     = (int64_t) ubatch.n_seqs_unq/n_stream;
...
// one row per stream, so a shared cell has nowhere to put its second pool
GGML_ASSERT(n_ps == 1 && "the per-cell pool view needs one sequence per stream");

Under --kv-unified with more than one sequence, n_stream collapses to 1, n_ps becomes the slot count, and the assert aborts the child. Drop the flag and n_stream equals the sequence count, n_ps is 1, and the same slots load. So the flag is the cause, not the slot count and not the context.

Neither flag was asked for by the user, and neither is reachable from the UI, so the model simply cannot load and nothing they change helps. From a Strix Halo report: GLM-5.3-Flash UD-IQ4_XS was tried four times over two days after a ~146 GB download and never ran once. Each attempt spent ~56 s loading the model before the context check. The second attempt cut the context from 128000 to 8192, which could not have worked, because the context was never the problem.

The fix

Retry once with the geometry that would never have added the flag in the first place: one slot, no unified cache, the requested context untouched. This reverses Studio's own choice rather than the user's.

The match is on llama.cpp's own wording, not on an architecture name, so a second model with the same constraint is covered without a list to keep current. GGML_ASSERT stringifies the whole expression, so the message the binary prints carries the marker verbatim.

Because the refusal is an assert, the child aborts, and two things follow:

  • The rung sits ahead of the flash-attention retry. That one fires on any signal crash, so it would otherwise take this one first and spend the retry disabling a flag that was never the cause.
  • The three --fit recovery rungs inside _spawn_and_wait stand down for it, the same way they already do for the split-axis abort and the pre-b9455 quantized-KV refusal. They are gated on a bare startup crash, so the reported launch (--gpu-layers 47 with --fit off) would have taken the fully_gpu_offloaded arm first: a second full model load on a theory unrelated to the failure, after which the retry would have read a fit-rewritten argv and committed a --fit on the user never asked for.

Matching is on the message rather than the exit code, so the POSIX SIGABRT and the MSVC CRT exit 3 both land here.

Two details that follow the existing rungs:

  • Every occurrence and every alias of the slot count is rewritten, not just the one Studio emitted. Extras are appended after Unsloth's flags and llama.cpp is last-wins, so one surviving in the tail would put the rejected geometry straight back. The aliases come from _PARALLEL_FLAGS, the same frozenset the extra-args denylist and its [Bug] API feedback #9510 hint use, so the three cannot drift apart. --parallel=4, -np 4, the attached short -np8 and --n-parallel 4 are all handled.
  • LLAMA_ARG_KV_UNIFIED is dropped from the child environment, for the same reason _drop_env_flash_attn exists: llama.cpp applies its environment before parsing argv. It is dropped rather than negated with --no-kv-unified, because the drop needs nothing of the binary while emitting a flag needs every build that reaches here to know it.

Verification

studio/backend/tests/test_llama_cpp_single_sequence_retry.py, 13 cases, including the report's own argv, the GGML_ASSERT the binary actually prints, and the upstream minimax-m3 warning that must not trip it (that one degrades to dense attention and still loads, so retrying it would cost three slots for nothing).

608 tests pass across that file plus test_tp_vision_regression, test_llama_server_args, test_llama_cpp_effective_parallel_slots, test_llama_cpp_no_context_shift, test_llama_cpp_context_fit and test_llama_cpp_mmproj_fallback.

Beyond the unit tests, the rewrite was simulated against a llama-server stand-in that implements the three lines above and llama.cpp's own resolution order (environment first, then last-wins argv):

  • 22,051 generated command lines, exhaustive over slot spelling x count x unified-cache spelling x position plus 20,000 randomised ones, checking that no unrelated token is dropped, duplicated or reordered, that the result reads back as one slot and not unified through Studio's own parsers, that it is idempotent, and that -c and --gpu-layers survive.
  • 18 before/after loads, each run with the rung and without it. Before: the reported 4-slot launch never loads, at 128000 or at 8192. After: it loads on the second attempt at one slot with the context intact. Launches that already worked are never retried and keep their slots.
  • 720 combinations of [Windows, Linux, WSL, macOS] x [NVIDIA, AMD, CPU] x slots x context x fit x vision, checking that every platform and device flag comes back byte for byte and that each one loads.
  • The whole set re-run under Python 3.9, 3.10, 3.11, 3.12 and 3.13 in isolated venvs, identical results, for older installs.

Note on what this does not do

Dropping to one slot means concurrent requests queue, and the warning says so. Keeping --parallel and dropping only --kv-unified would satisfy the assert too and preserve concurrency, but it silently divides the requested context by the slot count, which is the behaviour --kv-unified was added to prevent. Loading at the requested context is the better trade for a model that otherwise does not load at all.

Studio appends --kv-unified whenever it asks for more than one slot, so
llama.cpp does not split -c into per-slot windows. Some architectures need one
sequence per stream and refuse to build a context that way:

  llama_init_from_model: failed to initialize the context: glm5next: the pooled
  indexer needs one sequence per stream, so a unified KV cache is only supported
  with a single sequence

The flag is Studio's, not the user's, so nothing they can change in the UI
reaches it. A Strix Halo report shows GLM-5.3-Flash tried four times over two
days after a 146 GB download and never once ran; the second attempt cut the
context 15x, which could not have helped because it was never the context.

Retry once with the geometry that would never have added the flag: one slot,
no unified cache, the requested context untouched. Matched on llama.cpp's own
wording rather than on an architecture name, so a second model with the same
constraint needs no list entry. The rung sits ahead of the flash-attention
retry, which only fires on a signal crash, and this is a clean refusal.

Every --parallel occurrence is rewritten, not just the emitted one: extras are
appended after Unsloth's flags and llama.cpp is last-wins. LLAMA_ARG_KV_UNIFIED
is dropped from the child environment for the same reason the flash-attn retry
drops LLAMA_ARG_FLASH_ATTN - llama.cpp applies the environment before argv, and
there is no negated flag to emit against it.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T10:31:56.814290Z 4e48341 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@danielhanchen

Copy link
Copy Markdown
Member Author

Confirmed this hits the path in studio/backend/core/inference/llama_cpp.py where Studio appends --kv-unified for more than one slot, so the load fails before the user can change anything, and the retry now commits the one-slot geometry it launched. Will get this reviewed.

@danielhanchen

Copy link
Copy Markdown
Member Author

Went back to llama.cpp to check what the binary actually does here, and three things came out of it.

The refusal is an abort, not a clean exit. The constraint lives in ggml-org/llama.cpp#27754 as GGML_ASSERT(n_ps == 1 && "the per-cell pool view needs one sequence per stream"), so the child prints the stringified expression and calls abort(). The matcher still catches it, because GGML_ASSERT puts the message in the output verbatim, but the ordering note I wrote was backwards: this is a signal crash, so the flash-attention rung would have taken it, and staying ahead of that rung is what makes the retry work rather than being incidental.

The --fit rungs inside _spawn_and_wait were taking it first. All three are gated on a bare startup crash, so the reported launch (--gpu-layers 47 with --fit off) hit the fully_gpu_offloaded arm before the ladder ever got there: a second full model load on a theory unrelated to the failure, and the argv the single-sequence rung then read would have been the fit-rewritten one, committing a --fit on the user never asked for. Worse, if the fit rewrite changed the failure mode the marker would be gone from the last attempt's output and the model would stay unloadable. They now stand down for this refusal the same way they already do for the split-axis abort and the pre-b9455 quantized-KV refusal, so the guard is _capability_crash with _tensor_capability_crash left as the tensor-only half the ROCm rung still gates on.

--n-parallel was not being rewritten. The extra-args denylist treats {-np, --parallel, --n-parallel} as one alias group, and the retry covered two of the three, so the alias the denylist knows about would have survived into the tail and, llama.cpp being last-wins, put the rejected geometry straight back. The set now lives in _PARALLEL_FLAGS in llama_server_args.py and is shared by the denylist, its #9510 hint and the retry. A valueless --parallel also used to swallow the token behind it, which would have deleted the --kv-unified the retry exists to remove.

Also marked the slot clamp with # allow-slot-clamp:, which is what was failing Lint CI and the repo CPU test.

Verified with a llama-server stand-in implementing those three lines and llama.cpp's own resolution order: 22,051 generated command lines for the rewrite invariants, 18 before/after loads (the reported launch never loads without the rung, at 128000 or at 8192, and loads on the second attempt with it, context intact), 720 combinations of [Windows, Linux, WSL, macOS] x [NVIDIA, AMD, CPU] x slots x context x fit x vision confirming every platform flag comes back byte for byte, and the whole set re-run under Python 3.9 through 3.13 in isolated venvs for older installs. 608 backend tests pass across the touched files.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 7, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 7, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 7, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex security review

danielhanchen added a commit to shimmyshimmer/unsloth-staging-4 that referenced this pull request Sep 7, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 7, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 7, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 4e48341b49

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 7, 2026
@danielhanchen
danielhanchen merged commit e37730d into main Sep 7, 2026
50 checks passed
@danielhanchen
danielhanchen deleted the studio/retry-without-kv-unified branch September 7, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant