Studio: retry as one sequence when llama.cpp refuses a unified KV cache - #10371
Conversation
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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
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. |
|
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 The
Also marked the slot clamp with 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. |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
The bug
Studio appends
--kv-unifiedon its own whenever it asks for more than one slot, so llama.cpp does not split-cinto 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:Under
--kv-unifiedwith more than one sequence,n_streamcollapses to 1,n_psbecomes the slot count, and the assert aborts the child. Drop the flag andn_streamequals the sequence count,n_psis 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_ASSERTstringifies 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:
--fitrecovery rungs inside_spawn_and_waitstand 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 47with--fit off) would have taken thefully_gpu_offloadedarm 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 onthe user never asked for.Matching is on the message rather than the exit code, so the POSIX
SIGABRTand the MSVC CRT exit 3 both land here.Two details that follow the existing rungs:
_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-np8and--n-parallel 4are all handled.LLAMA_ARG_KV_UNIFIEDis dropped from the child environment, for the same reason_drop_env_flash_attnexists: 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, theGGML_ASSERTthe 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_fitandtest_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):
-cand--gpu-layerssurvive.Note on what this does not do
Dropping to one slot means concurrent requests queue, and the warning says so. Keeping
--paralleland dropping only--kv-unifiedwould satisfy the assert too and preserve concurrency, but it silently divides the requested context by the slot count, which is the behaviour--kv-unifiedwas added to prevent. Loading at the requested context is the better trade for a model that otherwise does not load at all.