serve: multi-slot thinking-on — read the assistant framing from the Jinja render by ghazni101 · Pull Request #811 · warpfront/hipfire

ghazni101 · 2026-10-03T11:50:13Z

Summary

Every thinking-on request on the multi-slot serve route failed closed. Two
gates in crates/hipfire-daemon/src/slots.rs both assumed the request's
enable_thinking must equal the prompt's assistant framing. They are
different quantities: enable_thinking is a Jinja input.

The standard Qwen3-family template (verified against the shipped
chat_template of MiMo-V2.6-Distill-Qwen-9B-MQ4) emits no opener when
thinking is enabled and lets the model generate <think> itself; only
enable_thinking=false emits a closed <think>\n\n</think>\n\n block:

{%- if add_generation_prompt -%}
    {{- '<|im_start|>assistant\n' -}}
    {%- if enable_thinking is false -%}
        {{- '<think>\n\n</think>\n\n' -}}
    {%- endif -%}
{%- endif -%}

So a thinking-on render ends in a bare assistant\n.

Failure 1 — `reasoning prefix mismatch: expected OpenThink got ClosedThink`

The route derived the continuation/reasoning framing from has_think, which
is the tokenizer's inventory of a <think> special token — not the prompt.
Thinking-on gives started_in_think=false and has_think=true, fabricating
ClosedThink, which then failed the equality check against the
request-derived OpenThink (400, validation).

Failure 2 — `think_mode mismatch with enable_thinking`

Reached once the first gate is fixed: think_mode was derived from the prompt
framing and then failed closed unless it agreed with enable_thinking. With
thinking on, started_in_think is false (correct — the model opens the span),
so this rejected the request too, typed internal (500).

Reproduction (before)

Against a serve.multi_slot container on this tree:

request	before
`{"messages":[{"role":"user","content":"hi"}]}` (auto)	400 `reasoning prefix mismatch`
`... ,"enable_thinking":true`	400 `reasoning prefix mismatch`
`... ,"enable_thinking":false`	200 OK

Fix

New hipfire_engine::emit::render_assistant_prefix: sibling of the existing
render_tail_opens_think, mapping the live generation turn (the segment
after the last <|im_start|>) to ClosedThink / OpenThink / Plain by
think-marker pairs. The Jinja branch reads the framing back from what the
template actually emitted; the VL and ChatFrame builders frame the turn
themselves from expected_prefix and keep returning it. Marker pairs rather
than an ends_with suffix match so that:
- a template that seeds reasoning text after the opener
  (<think>Let me…) frames OpenThink — a suffix read misclassified it
  Plain, starting the parser outside a span the prompt had already opened
  and routing that reasoning to the visible channel;
- a re-opened span in the same turn (<think>a</think><think>) frames
  OpenThink;
- think blocks in history turns and literal think markup in user
  content stay out of the classification (they live before the last
  <|im_start|>);
- every guard-block spelling (<think>\n\n</think>\n\n and
  <think></think>) still frames ClosedThink.
  A render with no <|im_start|> (non-ChatML template) classifies the whole
  trimmed render.
started_in_think is now simply prefix == OpenThink, so
emit_generation_start, the engine request and think_mode all agree with
the prompt by construction.
Both fail-closed equality checks are removed: each compared two independent
derivations of the same quantity, and per the existing contract in emit.rs
("Jinja owns the generation suffix, so the request's assistant_prefix is
not authoritative once rendering succeeds") the render is the authoritative
one.
Dropped the now-stale "think" case from the post-start error-release test
seam (the remaining three still cover the internal class).

Verification

surface	after
`cargo test -p hipfire-daemon`	36/36
`render_tail_think_tests` (incl. `rendered_framing_is_read_from_the_suffix` + `seeded_and_reopened_openers_frame_open_think`)	6/6
`cargo test -p hipfire-isa -p peacemaker-ir -p hipfire-quantize`	green (LLVM gates skip where `/opt/rocm/core-10.0` is absent)
`cargo clippy --workspace --all-targets`	error-clean
`scripts/leanup-ratchets.sh`	OK — 0 violations (`daemon_lines` 5372 ≤ 5377, paid down — see below)
`scripts/ratchet-diff.sh` vs `beta` (label-required mode)	OK — no expectation weakened
API auto	200, `finish:stop`, reasoning + content
API `enable_thinking:true`	200, `finish:stop`, reasoning + content
API `enable_thinking:false`	200, `finish:stop`, 0 reasoning + content
embedded `/ui` (`--ui`), `thinking=auto` and `thinking=on`	both complete, no error card, reasoning separated

Confirmed live in a serve.multi_slot container (2 slots × 81920 ctx, paged KV

radix prefix cache) with the rebuilt image.

Which surface(s) does this touch?

serve — crates/hipfire-daemon slots (multi-slot route)
beta-CI-green carry — this branch cherry-picks the three fix(ci): make beta CI green (layering, clippy deny, pinned-LLVM gates) #804 fixes
(fbcf592d8 quantize vacuous-assert, abf20d57b pinned-LLVM gate skips,
2d9042df2 xdna mut_from_ref allow) plus the /ui iterations, so the PR
can go green against a still-red beta; they drop out as duplicates when
fix(ci): make beta CI green (layering, clippy deny, pinned-LLVM gates) #804 / serve: opt-in embedded chat UI at /ui #808 merge first. One more LLVM gate needed the same skip:
iu4_v2b_a4 postdates fix(ci): make beta CI green (layering, clippy deny, pinned-LLVM gates) #804's sweep and panicked on the no-GPU runner.
fmt — one commit normalizing PR-touched files to the clippy-1.99
rustfmt style edition (formatting only).
ratchet paydown — land/041t + land/041u grew daemon main.rs to
5392 against the 5377 ceiling, leaving beta's own gates job red for every
branch based on its head. A raise needs the maintainer-only
ratchet-raise label, so this pays the debt instead:
no_model_loaded_suffix + its test move to request_guards (whose
charter is exactly "small load/generate message helpers kept out of
main.rs"), semantics untouched — main.rs lands at 5372. Stale crate maps
for engine/generate/quantize/cli/config/daemon are refreshed alongside.

Notes

Also present on master (slots.rs:1168), not beta-only.
Not a regression from any open branch — reproduced on beta itself.
Rebased on beta @ e268a0798 (the branch's current head).
Divergence with fix(daemon): multi-slot thinking-on rejected for implicit-<think> templates (+ embedded /ui chat) #801 (master), maintainer call needed: fix(daemon): multi-slot thinking-on rejected for implicit-<think> templates (+ embedded /ui chat) #801 fixes the
same false rejections with classify_render_tail + a narrowed
contradiction gate (thinking_tail_contradicts_policy: thinking-on +
closed tail, or thinking-off + open tail, still 400); this PR removes the
gate entirely. The classifications now agree on every shape fix(daemon): multi-slot thinking-on rejected for implicit-<think> templates (+ embedded /ui chat) #801 enumerates
(the marker-pair read above mirrors classify_render_tail, including the
seeded-text case), so the remaining difference is policy: silently
honouring a contradicting template (this PR) vs failing it closed (fix(daemon): multi-slot thinking-on rejected for implicit-<think> templates (+ embedded /ui chat) #801).
Whichever lands second needs to reconcile with the other before the next
master↔beta cross-merge.
CI correction vs an earlier revision of this description: the clippy
failure on the original head was absurd_extreme_comparisons at
crates/hipfire-quantize/src/quant_mq.rs:3540 (vacuous <= 3 assert), NOT
mut_from_ref/hipfire-xdna — that lint is also real and both are
inherited from beta and fixed by the fix(ci): make beta CI green (layering, clippy deny, pinned-LLVM gates) #804 carry above. The unit-test
failure was the pinned llvm-mc (/opt/rocm/core-10.0) missing on the
no-GPU runner, likewise gated by the carry.

…E_UI) Port of feat/serve-chat-ui-v040 onto the 0.4.x beta line (the v040 branch is authored against master; serve/complete.rs, serve/mod.rs and serve/http.rs have since been refactored on beta, so the Rust hunks were re-applied against the beta shapes rather than rebased). What ships: - Zero-dependency chat frontend: five static assets embedded via include_str! (index/app/db/render/style), served by a new serve/ui.rs matcher that shares the listener and the existing /v1/chat/completions path. IndexedDB history, branching, markdown/highlighting, effort picker, live TTFT/tok-s strip, mobile layout, ember palette. - Enablement in precedence order: --ui, --open (implies --ui, launches the browser), serve.ui = true, HIPFIRE_SERVE_UI=1. Off is byte-for-byte the old behavior: / and /ui both 404. Non-loopback binds warn on stderr; detached children get --ui. - Security headers on every UI route: strict CSP (default-src 'none', frame-ancestors 'none', form-action 'none'), X-Frame-Options: DENY, nosniff, no-cache — the page is an unauthenticated control surface and is not frameable. - /health and /v1/models publish the resident model's reasoning contract and effort rungs (from meta.loaded — no runtime lock), and capabilities gains chat_ui. - Carries the two gateway fixes the UI surfaced: mid-generation daemon failures without attested rollback poison the session so the next request cold-resets (poisons_session_state), and completion_timings no longer reports wall tok/s as decode tok/s. (slots.rs unchanged: beta already clamps max_tokens_fit via hipfire_generate::common::fit_max_tokens_if.) Verified on gfx1101 against a fresh --ui --no-prewarm serve: / -> 302 /ui; /ui 200 with the full header set; all four assets 200 with correct content types; non-UI paths still 404; /health reports chat_ui: true and the reasoning fields. cargo test -p hipfire-cli 312/312; crate maps regenerated.

The module-level use sat unused on beta; the ported test now resolves through it instead of shadowing with its own import (clears the unused-import advisory).

`(b >> 2j) & 0x3 <= 3` is always true; clippy's deny-level bad_bit_mask stopped `cargo clippy --all-targets` at hipfire-quantize. Pin the block geometry instead.

The no-GPU CI runner has no /opt/rocm/core-10.0, so peacemaker-ir's codec gates panicked in the lib-test step and 14 hipfire-isa integration tests in the GPU-free step. Follow the existing table-gate convention: skip with a log line when llvm-mc is absent; the gates still run where it exists.

clippy's deny-by-default mut_from_ref stopped `cargo clippy --all-targets` at hipfire-xdna once hipfire-quantize compiled. Bo is a cloneable handle to one shared mapping, so &mut self would not prevent aliasing; exclusivity is the documented caller contract. Same fix as a1ac7f4 on feat/qwen38-flash-next.

The UI held a single global in-flight guard (state.stream), serializing every chat through one request at a time even when serve.multi_slot runs the daemon's concurrent slot engine. Streams are now keyed by conversation id (state.streams: Map), with curStream()/anyStreaming() helpers. - send/regenerate/edit/tool-reply/variant-switch block only on the same conversation's stream; other chats generate in parallel - stop button and Esc abort the open conversation's stream only - deleteConv/clearAll/bc del+reload abort the matching streams - sidebar conv-streaming dot shows for every in-flight chat - per-conversation debounced saves (a shared timer dropped the second stream's writes) - view updates/scroll only for the open conversation; streaming view is nulled when switching away and rebound on return - doc title shows ● Generating… while any stream runs hidden - busy toast rewritten: 'keep working in another chat' Verified against a stubbed /v1/chat/completions SSE backend driving the real page in headless Chromium over CDP: two conversations streamed concurrently (streams.size=2, both lens growing), same-chat send refused mid-stream, stop aborted only the open chat, deleteConv aborted its stream, sidebar dots tracked in-flight convs.

…lot thinking-on) Every thinking-on (or thinking-auto) request on the multi-slot route failed closed with 'reasoning prefix mismatch: expected OpenThink got ClosedThink'. The route derived the continuation/reasoning framing as OpenThink / ClosedThink / Plain from `has_think`, which is the *tokenizer's* inventory of a `<think>` special token — not the prompt. Standard Qwen3-family templates emit no opener when thinking is enabled and let the model generate `<think>` itself (only enable_thinking=false emits a closed `<think>\n\n</think>\n\n` block), so a thinking-on render ends in a bare `assistant\n`: started_in_think=false and has_think=true fabricated ClosedThink, which then failed the equality check against the request-derived expected OpenThink. The Jinja render owns the generation suffix, so the framing is read back from what the template actually emitted: new render_assistant_prefix() (sibling of render_tail_opens_think, tail-only for the same reason) maps a closed `</think>` tail to ClosedThink, an open `<think>` tail to OpenThink, and anything else to Plain. The VL and ChatFrame builders frame the assistant turn themselves from expected_prefix and keep returning it. The fail-closed equality check is removed: it compared two independent derivations of the same quantity, and the render is the authoritative one. started_in_think (what emit_generation_start latches) is now simply prefix == OpenThink. Reproduced against the MiMo-V2.6-Distill-Qwen-9B container on this branch: 'hi' with no thinking field, enable_thinking=true both returned the mismatch error; enable_thinking=false worked. Also present on upstream/beta and upstream/master (slots.rs:1168/1172) — not a regression from this branch. Test: render_tail_think_tests::rendered_framing_is_read_from_the_suffix.

…hinking Second gate on the same thinking-on path, reached once the prefix check was fixed: 'think_mode mismatch with enable_thinking' (typed internal, so a 500). The route derived think_mode from the prompt framing (started_in_think) and then failed closed unless it agreed with the request's enable_thinking. Those are different quantities: enable_thinking is a Jinja *input*, and a thinking-on Qwen3-family template frames no opener — the model emits <think> itself, so started_in_think is false while enable_thinking is true. The parser must start outside the reasoning span and open it on that token, which is what the framing-derived value already says. Drop the equality check and keep think_mode = framing. Also drop the now-stale 'think' case from the post-start error-release seam list (the remaining three still cover the internal class).

- Deleted-chat toast no longer covers the composer: #toasts anchors below the topbar (top: calc(var(--topbar-h) + 12px)) instead of the bottom edge, and the enter/leave keyframes drop from above to match. - Each sidebar row gains a visible delete button beside the three-dot menu, wired to the same deleteConv path (undo toast included). It shares the three-dot button's reveal: hidden until row hover/active/focus, always visible on touch/narrow. Hover tints it with the error colour. - Drop the topbar chat-title button (redundant with the sidebar row): the Rename action now targets the sidebar row, opening the sidebar first when it is collapsed. Removes .conv-title/.conv-title-input and the responsive hide. The delete entry stays in the three-dot menu as before.

The changed-file rustfmt gate runs under the 1.99 toolchain, whose style edition reflows the dense one-line isa/peacemaker tests and reorders imports. Normalize every file this PR already touches so the gate sees a clean tree; formatting only, no semantic changes.

A template that seeds reasoning text after the opener (<think>Let me…) frames OpenThink, but the suffix read classified it Plain — generation would begin with the parser outside a span the prompt had already opened, routing that reasoning to the visible channel. Classify the live generation turn (after the last <|im_start|>) by marker pairs: seeded and re-opened openers frame OpenThink, history think blocks and user-literal markup stay out of the classification, and every guard-block spelling still closes. Mirrors classify_render_tail in warpfront#801 so the beta and master fixes converge on the same shapes.

…ilure suffix helper out of main.rs land/041t + land/041u grew daemon main.rs to 5392 against the 5377 ceiling, leaving beta's own gates job red for every branch based on its head. Raising the ceiling needs the maintainer-only 'ratchet-raise' label, so pay the debt instead: no_model_loaded_suffix and its test move to request_guards (the module whose charter is exactly 'small load/generate message helpers kept out of main.rs'), semantics untouched — main.rs lands at 5372. Also refreshes the stale crate maps (engine/generate/quantize/cli/config/daemon).

The slot route refused the effort dial outright: validate_generate_caps rejected any reasoning_effort but 'none', the capability advert listed it in refused_request_fields, and the render hardcoded JinjaChatFrame::reasoning_effort to None while the picker was hidden. A model whose template consumes effort therefore could not be steered at all on multi-slot, while the sequential and batch routes honoured the same field. - slots.rs: read reasoning_effort (with the project-custom thinking_mode alias) and hand it to the template. The suppression rule copies qwen_jinja_reasoning exactly — thinking_enabled=false, a disable rung (none/off/chat), or the immediate-close sentinel (1) leave the variable undefined so the template's own default applies. It is deliberately NOT gated on the route's `enable_thinking`, which describes the <think>-token ChatML framing; templates that open reasoning through a different token (e.g. <ifm|think>) keep has_think false and would otherwise lose the dial. - slots.rs: the authority fallback now defers to qwen_jinja_reasoning, so the disable rungs behave as they do on the sequential route (previously a bare reasoning_effort='none' still enabled thinking). - slots.rs: drop the reasoning_effort refusal; an unsupported rung now fails the render (validation) instead of being refused at the door. - serve/mod.rs: remove reasoning_effort from the multi-slot refused list, so the capability advert matches what the route accepts (no silent inflation and no false refusal). - app.js: gate the effort picker on reasoning_effort_native — the daemon's render probe, true only when setting the variable CHANGES the prompt. A template can accept a rung and ignore its value; advertising rungs alone offered a control that did nothing. Tests: slots validator accepts high/xhigh, gateway advert list asserts the field is absent.

The rule is subtle enough that getting it wrong is silent (keying it on the route's <think>-token framing made the dial a no-op for templates that open reasoning with another token), so it gets a name and a test rather than living inline: slot_jinja_effort() documents the suppression cases and pins them (pass-through verbatim, auto/absent undefined, disabled by thinking_enabled=false / none / off / chat / max_think_tokens=1, but not by a finite budget).

…ed ROCm toolchain Same table-gate convention as abf20d5 (warpfront#804 carry): the A4-fusion assembly test spawns /opt/rocm/core-10.0's llvm-mc directly and panicked on the no-GPU runner, where every other LLVM gate already skips. The file postdates warpfront#804's sweep.

A second refusal lived in the multi-slot request guard, so the daemon-side plumbing never got a chance: any effort but 'none' was rejected with 'reasoning_effort is not supported' before projection ran. apply_http_reasoning_request already owns the field end-to-end — it validates the shape, normalises qwen_jinja's vocabulary (low|medium|xhigh, with the OpenAI spellings mapped onto it), drops the dial with a warning when the loaded template does not natively honour effort, and drops it when thinking is off. Refusing it in the guard rejected requests the sequential route accepts while pre-empting that logic. Verified live (MiMo 9B, multi-slot): reasoning_effort='high' is now accepted, and 'none' reaches the reasoning derivation and renders thinking off (prompt_tokens 8 -> 10 with the closed think block, reasoning_content empty).

… work

UI hunk of 0fb8d4c ("serve: honour reasoning_effort on the multi-slot route", #811); the daemon/gateway half of that commit is on land/pr811. (cherry picked from commit 0fb8d4c (app.js only))

# Conflicts: # crates/hipfire-cli/map.md # crates/hipfire-generate/map.md # docs/env-vars.md

fivetide · 2026-10-08T06:11:04Z

Thanks @ghazni101 for the multi-slot thinking-on fix!

Shipped in v0.4.1 (CHANGELOG § Serve & CLI), landed together with #807 and #808. It went in through the maintainer land branches (rebased/cherry-picked commits and land: merges) rather than a plain merge of this PR, which is why GitHub doesn't show it as merged — sorry about that.

Commits on master and beta:

8c6daf0 serve: read the assistant framing back from the Jinja render (multi-slot thinking-on)
419bc8d serve: the slot parser's think state follows the prompt, not enable_thinking
9c9d250 serve: classify the rendered assistant turn by think markers, not suffix
0d8be43 serve: honour reasoning_effort on the multi-slot route
d1cc961 serve: lift the slot effort-suppression rule into a tested helper
66987a7 serve: stop refusing reasoning_effort in the multi-slot projection
42e9205 regenerate crate maps
4c2c73e CHANGELOG entry
810ff32 land merge of the serve stack

Closing as ingested. Thanks again!

ghazni101 requested a review from Kaden-Schutt as a code owner October 3, 2026 11:50

ghazni101 force-pushed the fix/slots-thinking-on-framing branch from e1cdf1a to db637b7 Compare October 3, 2026 11:52

ghazni101 marked this pull request as draft October 3, 2026 11:55

ghazni101 force-pushed the fix/slots-thinking-on-framing branch from db637b7 to 3327bee Compare October 3, 2026 12:31

ghazni101 marked this pull request as ready for review October 3, 2026 12:32

ghazni101 and others added 11 commits October 3, 2026 16:34

serve/ui port: use the tests module's ClientError import

7555fac

The module-level use sat unused on beta; the ported test now resolves through it instead of shadowing with its own import (clears the unused-import advisory).

fix(quantize): replace the vacuous 2-bit range assert

8e01829

`(b >> 2j) & 0x3 <= 3` is always true; clippy's deny-level bad_bit_mask stopped `cargo clippy --all-targets` at hipfire-quantize. Pin the block geometry instead.

ghazni101 force-pushed the fix/slots-thinking-on-framing branch from 3327bee to 28d27d4 Compare October 3, 2026 12:37

ghazni101 force-pushed the fix/slots-thinking-on-framing branch from 28d27d4 to 4a5a109 Compare October 3, 2026 12:48

ghazni101 added 5 commits October 3, 2026 16:51

chore: refresh cli/daemon crate maps after the reasoning_effort slots…

35e8cd7

… work

ghazni101 mentioned this pull request Oct 3, 2026

fix(daemon): multi-slot thinking-on rejected for implicit-<think> templates (+ embedded /ui chat) #801

Closed

Kaden-Schutt added a commit that referenced this pull request Oct 6, 2026

CHANGELOG: v0.4.1 entry for #811

4c2c73e

Kaden-Schutt added a commit that referenced this pull request Oct 6, 2026

land: merge serve stack #811 + #807 + #808 by @ghazni101 (f914781)

810ff32

# Conflicts: # crates/hipfire-cli/map.md # crates/hipfire-generate/map.md # docs/env-vars.md

Kaden-Schutt mentioned this pull request Oct 6, 2026

Tracking: 0.4.1 (Tue 10/06) — Flash-Next prefill, bugfixes, in-flight work #799

Open

fivetide mentioned this pull request Oct 8, 2026

serve: completion timings no longer report wall tok/s as decode tok/s #807

Closed

3 tasks done

fivetide mentioned this pull request Oct 8, 2026

serve: opt-in embedded chat UI at /ui #808

Closed

6 of 8 tasks

fivetide closed this Oct 8, 2026

ghazni101 deleted the fix/slots-thinking-on-framing branch October 9, 2026 01:32

Navigation Menu

Repository navigation

serve: multi-slot thinking-on — read the assistant framing from the Jinja render - #811

serve: multi-slot thinking-on — read the assistant framing from the Jinja render#811
ghazni101 wants to merge 17 commits into
warpfront:betafrom
ghazni101:fix/slots-thinking-on-framing

ghazni101 commented Oct 3, 2026 •

edited

Loading

Uh oh!

fivetide commented Oct 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ghazni101 commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Failure 1 — reasoning prefix mismatch: expected OpenThink got ClosedThink

Failure 2 — think_mode mismatch with enable_thinking

Reproduction (before)

Fix

Verification

Which surface(s) does this touch?

Notes

Uh oh!

fivetide commented Oct 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ghazni101 commented Oct 3, 2026 •

edited

Loading

Failure 1 — `reasoning prefix mismatch: expected OpenThink got ClosedThink`

Failure 2 — `think_mode mismatch with enable_thinking`