fix(local-llm): managed Qwen 3.5 stalls in an unterminated tool-call grammar - #48
Open
sachin-detrax wants to merge 2 commits into
Open
Conversation
…d grammar
Managed mode shipped a chat-template override for qwen-3.5-4b (the default
managed model) and qwen-3.5-35b, passed to llama-server as
`--chat-template-file assets/ai-models/qwen3.5-chat-template.jinja`. That
file is a Qwen2.5-style ChatML template with no `<think>` or
`enable_thinking` markers.
llama-server echoes the override back at `/props.chat_template`, which is
the only signal `detectModelProfile` has. Without the markers,
`looksLikeQwenThinkModel` fails and the profile is demoted to
`plain-instruct`. `buildGrammar` then early-returns before adding the
reasoning prelude, leaving `root ::= tool-call-array` — so a thinking model
is forbidden from thinking and the sampler's only legal opening token is
`[`. It emits the forced `[`, lands in the unbounded `ws ::= [ \t\n\r]*`,
and since GBNF hard-masks EOG until the whole root is satisfied
(llama-grammar.cpp), it has an infinite legal move: newlines until
`n_predict` (8192). The stream parser sits in its `json_tool` state matching
`"tool":` against whitespace, so zero StepEvents reach the UI. GPU and CPU
pegged, nothing rendered, no completion.
External mode never passes `--chat-template-file`, so `/props` reports the
GGUF's real template, the profile resolves to `qwen-think`, and the same
model on the same hardware works — which is exactly the reported split.
Fixes, smallest first:
- Drop `chatTemplateAsset` from qwen-3.5-4b / qwen-3.5-35b and delete the
asset. The GGUF ships the correct template; managed mode is now
byte-identical to the working external setup. The `chatTemplateAsset`
mechanism stays as an escape hatch, with the invariant documented: an
override MUST preserve the reasoning markers profile detection keys on.
- Bound `ws` to `( [ \t\n\r] ){0,64}` in tool-call.gbnf. Defence in depth:
any future profile/grammar mismatch now fails fast instead of burning
8192 tokens in silence. 64 is far past any realistic indent run, and the
reasoning-seam whitespace rule was already bounded this way.
- Stop retrying our own `requestTimeoutMs` abort. It surfaces as
`status === null`, structurally identical to a dropped socket, so
`isRetryableLlamaError` replayed it up to `completionRetries` times —
3 x 300s of silent GPU churn before anything surfaced. `LlamaServerError`
now carries `timedOut` and short-circuits like a 4xx, with a message that
names the knob to raise.
Tests: no catalog model ships a template override; any declared override
must resolve on disk; `ws` is bounded for all three profiles; a timeout
aborts once while genuine transport failures still retry.
…ot count Two follow-ups from the managed-mode hang investigation. Neither was the trigger, both made it worse and both misbehave on their own. Context floor. `MIN_AUTO_CONTEXT` was 8192 — exactly equal to the default `localModels.completionMaxTokens`, and smaller than the agent's fixed prompt (~5.2k measured) plus that generation budget. Any model >= ~14 GB on a 16 GB card lands on this floor, so it had ~2-3k tokens of real room: not enough to close a reasoning block and a tool-call array, so llama.cpp's context ceiling fired and every step returned `truncated: true`. Raised to 16384, which is derived rather than picked — `minUsableContextWindow()` now names the requirement (fixed prompt + generation budget + boundary margin = 14704 on defaults) and a test asserts the floor clears it, so the two constants cannot drift into crossing again. Bootstrap also warns when the *reported* contextWindow is under that figure, managed or external, naming the knob to raise; previously the budget code detected the impossibility, silently floored the conversation to 512, and said nothing. Slot count. `SlotManager` defaulted to 4 slots. Managed mode always defers the boot health check (the daemon may not be up yet), so `resolveModelProfile` short-circuits and returns `totalSlots: null` — meaning the default outlived the probe and sessions were handed ids 1-3 against a `--parallel 2` daemon. llama.cpp wraps out-of-range ids (`id_slot % n_slots`, server-context.cpp) rather than erroring, so nothing surfaced: the ids silently collided with another session's slot, evicting its KV cache and forcing a full ~5k-token prompt reprocess on every rotation. - Default is now 1. Slot 0 is the only id every llama-server is guaranteed to have, so the pre-probe pool can no longer overrun the server. - `SlotManager.resize()` widens it once the real count is known. It drops stale assignments (a session's slot may not exist, and the server-side KV is not where we think either way — one honest cold rebuild beats pointing at a stranger's cache) and re-takes an out-of-range reflection reservation. - `ModelProfileManager` already re-probes `/props` at turn start; it now reports `total_slots` through an `onTotalSlots` hook, which bootstrap wires to `resize()`. Discovery runs before the profile/grammar work so a grammar rebuild failure cannot swallow it. - `extractTotalSlots` moved from bootstrap to model-profile.ts so both the boot probe and the refresh path share one parser. Tests: floor clears the agent requirement and stays under the ceiling; default pool never emits an id above 0; resize widens, no-ops on an unchanged count, drops stale assignments, keeps in-range reservations, releases out-of-range ones, and hands the sole slot back when shrinking to one; `onTotalSlots` fires on a successful probe, stays silent when the field is absent, and still fires when profile detection finds nothing.
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.
Closes #47.
Managed mode never returned a response on
qwen-3.5-4b(the default managed model) while the same model in external mode worked fine. That split is what isolated it: the difference is the managed daemon's launch flags, not the model or the hardware.Verified on the reporter's machine (RTX 5070 Ti 16 GB / Ryzen 9 9900X / 32 GB) — managed mode now streams tokens and completes turns.
The bug
models-catalog.tsattached a bundled chat template toqwen-3.5-4bandqwen-3.5-35b, forwarded bystartDaemonas--chat-template-file. That file is a Qwen2.5-style ChatML template with no thinking markers:llama-serverechoes the override back at/props.chat_template, which is the only signaldetectModelProfilehas. Without<think>/enable_thinking,looksLikeQwenThinkModelfails and the profile is demoted toplain-instruct. A/B with the same model and alias, only the template differing:buildGrammarearly-returns for areasoningStyle: "none"profile, so the reasoning prelude leaves the GBNF root. A reasoning model is then forbidden from reasoning: it emits the forced[, lands in the unboundedws ::= [ \t\n\r]*, and since llama.cpp hard-masks EOG until a grammar stack empties, whitespace is an infinite legal move.It is silent because
assistant_deltais only emitted once the stream parser reaches{"tool":"reply","args":{"text":". Having consumed the[, the parser sits injson_toolmatching"tool":against whitespace:Changes
1. Drop the Qwen 3.5 chat-template override (
models-catalog.ts, asset deleted). The GGUF ships the correct template; managed mode is now byte-identical to the working external setup. ThechatTemplateAssetmechanism stays as an escape hatch with the invariant documented: an override MUST preserve every reasoning marker profile detection keys on.2. Bound
wsto( [ \t\n\r] ){0,64}(grammars/tool-call.gbnf). Defence in depth — any future profile/grammar mismatch now fails fast instead of burning 8192 tokens in silence. Same grouped{0,n}shape already proven by the reasoning-seam whitespace rule. 64 is far past any realistic newline+indent run; past that the mask simply forces the next real token.3. Stop retrying our own
requestTimeoutMsabort (llama-server-client.ts). It surfaces asstatus === null, structurally identical to a dropped socket, soisRetryableLlamaErrorreplayed it up tocompletionRetriestimes — 3 × 300 s of silent GPU churn before anything surfaced.LlamaServerErrornow carriestimedOutand short-circuits like a 4xx, with a message naming the knob to raise. Genuine transport failures still retry.4. Raise
MIN_AUTO_CONTEXT8192 → 16384 (context-size.ts). The old value was exactly equal to the defaultcompletionMaxTokensand smaller than the agent's fixed prompt (~5.2k measured) plus that budget, so any model ≥ ~14 GB on a 16 GB card had ~2–3k tokens of real room and returnedtruncated: trueon every step. The new value is derived rather than picked:minUsableContextWindow()names the requirement (fixed prompt + generation budget + boundary margin = 14704 on defaults) and a test asserts the floor clears it, so the two constants cannot drift into crossing again. Bootstrap now also warns when the reportedcontextWindowis under that figure, managed or external, naming the knob to raise — previously the budget code detected the impossibility, silently floored the conversation to 512, and said nothing.5. Stop guessing the slot count (
slot-manager.ts,model-profile-manager.ts,bootstrap.ts).SlotManagerdefaulted to 4 slots, and because managed mode always defers the boot health check that default outlived the probe — sessions were handed ids 1–3 against a--parallel 2daemon. llama.cpp wraps out-of-range ids (id_slot % n_slots) rather than erroring, so nothing surfaced: the ids silently collided with another session's slot, evicting its KV cache and forcing a full ~5k-token prompt reprocess on every rotation.llama-serveris guaranteed to have, so the pre-probe pool cannot overrun the server.SlotManager.resize()widens it once the real count is known. It drops stale assignments (a session's slot may no longer exist, and the server-side KV is not where we think either way — one honest cold rebuild beats pointing at a stranger's cache) and re-takes an out-of-range reflection reservation.ModelProfileManageralready re-probes/propsat turn start; it now reportstotal_slotsthrough anonTotalSlotshook that bootstrap wires toresize(). Discovery runs before the profile/grammar work so a grammar rebuild failure cannot swallow it.extractTotalSlotsmoved frombootstrap.tstomodel-profile.tsso the boot probe and the refresh path share one parser.Tests
wsis bounded for all three profiles.resizewidens, no-ops on an unchanged count, drops stale assignments, keeps in-range reservations, releases out-of-range ones, and hands the sole slot back when shrinking to one.onTotalSlotsfires on a successful probe, stays silent when the field is absent, and still fires when profile detection finds nothing.npm run lintandnpm run buildclean. Full suite: 3343 passed.Pre-existing failures, untouched by this branch (confirmed by stashing these changes and re-running on a clean tree — identical set):
tui-app,chat-log,splash-banner,llm-panel-selectors,persist-embedding-hybrid-recall,send-message-concurrency,fs-glob-real. Separately,tui/llm-health/llm-health-polleris a real-timer test (~450 ms/case) that flakes under full-suite load — it passed in isolation and on a re-run, and this branch does not touch it.Upgrade note
--chat-template-fileand--ctx-sizeare baked into the running daemon, so a rebuild alone changes nothing —models stop && models startis required. Verify with: