Skip to content

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

Closed
ghazni101 wants to merge 9 commits into
warpfront:betafrom
ghazni101:fix/multislot-thinking-tail-state
Closed

fix(daemon): multi-slot thinking-on rejected for implicit-<think> templates (+ embedded /ui chat)#801
ghazni101 wants to merge 9 commits into
warpfront:betafrom
ghazni101:fix/multislot-thinking-tail-state

Conversation

@ghazni101

@ghazni101 ghazni101 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #800

The change

The slot engine conflated thinking policy (what the request asked for) with the rendered prompt's tail state (what bytes the template emitted). It derived started_in_think from ends_with("<think>"), re-derived prefix from the tokenizer's has_think property, and 400'd any request where they disagreed — so a template that emits a bare assistant\n tail with thinking ON (the model opens the span itself; MiMo's convention) was stamped ClosedThink and rejected with reasoning prefix mismatch: expected OpenThink got ClosedThink. Because the daemon's no-fields fallback enables thinking for any tokenizer with a <think> token, the default request failed too. A second think_mode-vs-enable_thinking check repeated the same conflation one layer down.

Now:

  • classify_render_tail classifies the rendered tail — the region after the last <|im_start|>, so closed spans in history and literal markup in user text are not tail state — into OpenThink (unclosed span, including seeded reasoning) / ClosedThink (guard block) / Plain (no think markup).
  • The gate (extracted into thinking_tail_contradicts_policy) fires only on genuine contradiction: thinking on + ClosedThink tail, or thinking off + OpenThink tail. Plain is legal under either policy.
  • prefix (continuation suffix, emit ctx) uses the observed tail; think_mode uses the request policy.
  • Truth-table test now drives the production predicate plus real render shapes (Qwen open-tail / guard-block / MiMo plain-tail) instead of a local re-implementation of the gate; stale think_mode mismatch case removed.

This branch also carries the embedded chat UI work it was authored on top of (/ui at serve, history/branching/markdown/themes, palette + mobile fixes — commits 8983808..d1bbf3b). The UI is compile-time-embedded assets behind an exact-match route table with a strict CSP (default-src 'none', no unsafe-inline); render.js builds every node with createElement/textContent and restricts link hrefs to https?:/mailto:, so model output cannot execute script. Path traversal is structurally impossible (probed live: /ui/../etc/passwd, ..%2f variants → 404).

Verification

GPU: RX 7700 XT (gfx1101), ROCm 7.2.4 containers built hermetically from Containerfile --target runtime for both this branch and master (daemon md5 0fe0a38b… vs f8b88467…). MiMo-V2.6-9B, serve.multi_slot=true, 2 slots:

request master this branch
no thinking fields (default) 400 expected OpenThink got ClosedThink 200, answer correct, reasoning separated
enable_thinking: true 400 same 200
enable_thinking: false 200 (guard-block path) 200
  • Reasoning separation on Plain tails: the emitter's ThinkOutputRouter flips into the reasoning channel on a model-generated <think> even when constructed from a Plain tail (emit_text.rs drain), so MiMo's self-opened span routes to reasoning_content — verified across default/on/thinking.type/streams/multi-turn/concurrency; no think text ever leaked into content.
  • Adversarial probes: literal <|im_start|>assistant\n<think> and stray </think>/<think> in user text — all 200, no misclassification (the last-<|im_start|> scoping neutralizes them in standard templates).
  • Parity: identical greedy request through slots and the sequential route produces byte-identical content and reasoning_content.
  • VALIDATION route (user-facing serve semantics): serve_harness.py battery (thinking low), chain (thinking low), battery (thinking off) against the containerized slots serve — every run turns=5 runaway=0 empty=0 attractor=0 retrieval_miss=0, decoded output eyeballed coherent. Chain shows turn-over-turn cache reuse (cached 276→343→464); session-continuation reuse cached_tokens=31 on turn 2. (Cross-request radix reuse needs paged-KV/VMM, unavailable with the fixed Q8 slot arena on this card — environment, not this change.)
  • Concurrency: two parallel streams, one thinking-on one thinking-off, complete on 2 slots with correct separation.
  • Unit: 34/34 daemon tests; qwen3.5-4b (open-tail convention) verified on/off.
  • master reproduces the original 400s with the exact error from multi-slot: every thinking request (incl. the no-fields default) 400s on templates that let the model open <think> itself #800 on the same fixtures (A/B above).

Known follow-ups (out of scope here)

  • unsafe multi_slot terminal: open_think (pre-existing guard, b053ad9): a thinking-on request whose budget is exhausted inside the think span 500s with rollback and no partial content (qwen3.5, max_tokens=80 → 500; 600 → 200). vLLM-style finish_reason=length would be friendlier; this change newly reaches that terminal for MiMo-style requests that previously died at the gate.
  • SpecEmitCtx.think_mode is never read by the qwen35 emitter — wire it or drop it; correctness rides on assistant_prefix (tail) + started_in_think.

ghazni101 added a commit to ghazni101/hipfire that referenced this pull request Oct 3, 2026
@ghazni101
serve: classify the rendered assistant turn by think markers, not suffix
36b99c4 
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.
@ghazni101 ghazni101 mentioned this pull request Oct 3, 2026
Closed
4 tasks done
ghazni101 added a commit to ghazni101/hipfire that referenced this pull request Oct 3, 2026
@ghazni101
serve: classify the rendered assistant turn by think markers, not suffix
1839f36 
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.
@ghazni101
ghazni101 changed the base branch from master to beta October 3, 2026 13:45
@ghazni101
serve: opt-in embedded chat UI at /ui (--ui / serve.ui / HIPFIRE_SERV…
fda7ac6 
…E_UI)

Adds a zero-dependency chat frontend to the OpenAI gateway, off by
default. No new crates: three static assets embedded via include_str!
and served from a new serve/ui.rs asset matcher, sharing the serve
listener and the existing /v1/chat/completions path.

Enablement, in precedence order:

    hipfire serve --ui            # flag
    hipfire serve --ui --open     # flag + xdg-open/open/start the URL
    hipfire config set serve.ui true
    HIPFIRE_SERVE_UI=1

Off is byte-for-byte the old behavior: / and /ui both 404. --open
implies --ui (the launch promise needs the route) and is forwarded to
detached children as --ui. Non-loopback binds with the UI on emit a
stderr warning — the gateway already answers cross-origin, but the UI
makes that reachable from any browser.

Routes (GET, gated on shared.ui_enabled):

    /        -> 302 /ui
    /ui      -> embedded index.html
    /ui/app.js, /ui/style.css

Strict CSP (script-src/style-src 'self', connect-src 'self',
img-src 'self' data:, no unsafe-inline) + nosniff; the API-wide CORS
wildcard is not sent on UI responses. The UI only ever writes
textContent, so untrusted model output can never become markup.

Frontend features, all backed by existing wire shapes:

    - streaming via fetch + hand-rolled SSE reader (EventSource can't
      POST); AbortController drives the existing AtomicBool cancel path,
      so Stop really does kill GPU work
    - thinking panel from delta.reasoning_content
    - thinking controls driven by the model's advertised contract:
      /health now reports reasoning_contract / reasoning_effort_native /
      reasoning_efforts (populated from ServeRuntime, probed from the
      chat template at load). qwen_jinja gets an effort dropdown over
      the advertised rungs + auto/off; gemma_boolean gets on/off;
      muse_glimmer and unsupported hide the controls
    - tools JSON editor + tool_choice; terminal delta.tool_calls render
      with a manual "reply as tool" round-trip (assistant history keeps
      tool_calls so the tool turn is well-formed); hidden when
      refused_request_fields includes tools (multi-slot)
    - image attach gated on capabilities.vision_sidecar; PNG/JPEG data
      URI, one image per request (matches the gateway's validation)
    - model picker from /v1/models with sidecar/multi-slot badges;
      path-loaded models are injected as options so `model` is always
      sent (the gateway 400s without it)
    - status bar polls /stats (queue depth, recent tok/s, loading state),
      paused while the tab is hidden; per-turn ttft/tok-s/tau/usage from
      the terminal chunk's timings + usage + hipfire.reasoning shows the
      *resolved* effort/mode and config warnings

Route capabilities advertise "chat_ui": bool so clients can discover it.

Known gap: deepseek4 is effort-native in the request resolver but the
daemon only probes reasoning_efforts for qwen_jinja, so DS4 models get
the toggle without an effort dropdown until that probe extends.

Verified live on qwen3.5-4b.mq4 (gfx1101): redirect, all assets, CSP
headers, SSE content/reasoning deltas, terminal timings + resolved
reasoning, usage chunk, disconnect -> daemon abort, and 404 on /ui
without the flag. cargo test -p hipfire-cli serve: 152 pass; the one
failure (multi_slot_request_supported max_think_tokens=0) is
pre-existing on HEAD (confirmed via stash).
@ghazni101
serve: overhaul embedded chat UI — history, branching, markdown, themes
4cc213f 
Rebuilds the /ui frontend end to end, still zero external deps (CSP
default-src 'none', textContent-only rendering).

- History: IndexedDB store (db.js), sidebar with pinned/date groups,
  full-text search with hit snippets, rename/pin/delete with Undo,
  export Markdown/JSON, import, storage meter, cross-tab sync via
  BroadcastChannel, per-chat #id routes and per-chat drafts.
- Branching: regenerate/edit fork messages into variants; each variant
  keeps its own tail, so ‹ › restores whole branches.
- Messages: live streaming with reasoning disclosure ('Thought for Xs'),
  markdown (tables, nested/task lists, links, TeX-unicode math), syntax
  highlighting for ~15 languages, copy/download per code block, tok/s
  stats chips, tool-call cards with inline tool-result replies.
- Composer: image attach/paste/drag-drop gated on the model's vision
  capability, think pill (auto/on/off + effort), ~token count, Esc stop.
- Settings drawer persists sampling params, system prompt, tools and
  response_format; theme switcher (system/light/dark via light-dark()),
  status pill with live tok/s and queue depth, shortcuts dialog (?),
  toasts, lightbox, TTS, mobile layout with scrim.
- Unmatched backtick runs now emit literally instead of letting inner
  ticks re-scan as shorter openers.

Chat history is browser-local (IndexedDB); serve gains no new write
surface. ui.rs serves db.js/render.js alongside the existing assets.
@ghazni101
ui: effort picker in composer, live TTFT/tok-s strip, mobile pass
2bc1421 
- Reasoning effort select moves from the settings drawer into the
  composer bar as a second pill next to the think toggle (model-advertised
  efforts drive its options; hidden when the route/model can't take it).
- New #live-stats strip above the composer shows TTFT + tok/s while
  streaming and pins the server's final timings afterwards; the topbar
  pill goes back to state-only (ready / generating / loading).
- Mobile: composer bar wraps, textarea bumped to 16px so iOS doesn't
  zoom on focus, per-row and per-message actions stay visible on coarse
  pointers, safe-area padding on composer + sidebar.
@ghazni101
ui: fix small-screen overlaps — model select, composer wrap, fab
1a70f05 
- #model had min-width:auto inside its inline-flex wrapper, so at narrow
  widths the transparent select painted over the status pill and settings
  button. Constrain it to the picker's box (width/min-width/overflow).
- Composer bar under 520px: icon-only think pill, 64px effort select,
  char-count hidden, nowrap — send stays on the row instead of wrapping.
- #scroll-bottom was fixed at bottom:128px and could overlap a grown
  composer; a ResizeObserver now pins it to composer height + 12px.
- Verified headless at 320/360/390/768px: zero horizontal overflow,
  zero element overlaps, single-row composer, select ends before pill.
@ghazni101
ui: retune palette to the README header art — deep black + ember
6617b1e 
Dark theme now matches the warpfront/hipfire README header image:
near-black surfaces (#08080a / #0e0f13 / #121318), the hot
#ff6a2a/#ff8a1f flame gradient as the primary accent, teal (#41c4dd)
links and violet (#a98ef0) thinking-block accents — the poster's
secondary colors. Light theme keeps its warm-paper base with a
deepened #e24a12 accent to match.
@ghazni101
fix(daemon): multi-slot thinking-on rejected for implicit-<think> tem…
5ec35b1 
…plates

The slot engine conflated thinking POLICY with the rendered prompt's tail
STATE. It derived `started_in_think` from `rendered.ends_with("<think>")`,
re-derived `prefix` as `OpenThink if started else ClosedThink if tokenizer
has <think>`, then 400'd any request where `prefix != expected_prefix`.
Two defects:

- `has_think` is a tokenizer property, not a render property: a template
  that emits a bare `assistant\n` tail with thinking ON (the model opens
  the span itself) was misclassified as ClosedThink and rejected with
  "reasoning prefix mismatch: expected OpenThink got ClosedThink".
  Verified against the baked templates of every local model: Qwen3.5 /
  Ornith / NuExtract3 emit `assistant\n<think>` when enabled; MiMo's
  `enable_thinking is false` branch only writes the closed guard block,
  so thinking-on renders Plain — every thinking request 400'd on slots.
- `think_mode` was derived from `started_in_think` and compared against
  `enable_thinking`, a second policy-vs-tail conflation behind the first.

The sequential route has no such gate: `spec_assistant_prefix` treats the
request prefix as non-authoritative once the Jinja render succeeds.

Now: `classify_render_tail` scans think markers in the region after the
last `<|im_start|>` (the live generation turn), so closed spans in history
or literal markup in user text are not tail state, and a span opened by the
template but seeded with reasoning text still counts as OpenThink. The
contradiction gate fires only on real disagreement — thinking=on with a
ClosedThink tail, or thinking=off with an OpenThink tail; a Plain tail is
legal under either policy. `prefix` (continuation suffix, emit ctx) uses
the observed tail; `think_mode` uses the request policy.

Verified on hipfire-ui (MiMo-V2.6-9B, 2 slots x 81920 ctx, paged KV +
radix prefix cache): enable_thinking true/false/auto all generate,
cached_tokens=256 reuse on repeat, 30 slot tests pass including two new
classifier/contradiction tables.
@ghazni101
test(daemon): gate thinking-tail contradiction through a tested predi…
d7464f8 
…cate

Adversarial-review follow-up to d5c7d68. The truth-table test asserted
a LOCAL closure that re-implemented the gate expression — it could not
fail if the production gate drifted, and nothing tested the wiring
between classify_render_tail and the rejection decision.

Extract the contradiction predicate into
thinking_tail_contradicts_policy(enable_thinking, tail), call it from
the handler gate, and drive the truth table plus real render shapes
(Qwen open-tail, guard-block, MiMo plain-tail) through the same function
the handler uses.

Also drop the stale "think_mode mismatch with enable_thinking" case from
qwen_ar_slot_post_start_errors_release_route_and_reuse_generation: the
production emitter of that message was removed in d5c7d68, so the case
label lied about what the table covers.

All 34 daemon tests pass.
@ghazni101
fix(xdna): allow the intentional mut_from_ref in Bo::as_mut_slice
2625bbe 
The advisory clippy job's only hard error on the PR: deny-by-default
clippy::mut_from_ref fires on as_mut_slice(&self) -> &mut [u8]. The shape
is deliberate — BO memory is device-shared with the Arc-owned lifetime
and a documented caller contract (crate docs: as_slice/as_mut_slice
require no other live slice; device writes happen only between submit
and wait). Mutation rights come from the submission contract, not Rust
borrow state, as in every device-mapped-buffer API. Scoped to the one
function with the rationale; everything else in the crate stays
default-denied.
@ghazni101
ci: refresh crate maps after the serve/ui rebase onto beta
bce9b05
@ghazni101
ghazni101 force-pushed the fix/multislot-thinking-tail-state branch from 452a6fa to bce9b05 Compare October 3, 2026 13:51
@ghazni101

ghazni101 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Retargeted and rebased onto beta — which surfaced that this PR is now fully superseded on beta: #808 carries the /ui work and #811 carries this same thinking-on fix (its 'classify the rendered assistant turn by think markers' commit explicitly mirrors classify_render_tail from this branch). No unique diff remains; closing.

@ghazni101 ghazni101 closed this Oct 3, 2026
Kaden-Schutt pushed a commit that referenced this pull request Oct 6, 2026
@ghazni101 @Kaden-Schutt
serve: classify the rendered assistant turn by think markers, not suffix
9c9d250 
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 #801
so the beta and master fixes converge on the same shapes.

(cherry picked from commit 1839f36)
@ghazni101
ghazni101 deleted the fix/multislot-thinking-tail-state branch October 9, 2026 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kaden-Schutt Kaden-Schutt Awaiting requested review from Kaden-Schutt Kaden-Schutt is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

multi-slot: every thinking request (incl. the no-fields default) 400s on templates that let the model open <think> itself

1 participant

@ghazni101