feat(serve): OpenAI client parity on the multi-slot path — tools, reasoning_effort, sampling, usage, tool-iteration reuse - #609
Open
alpineQ wants to merge 3 commits into
Conversation
…i_slot_prefill_chunk
…nk prefill scratch, apply device visibility
…soning_effort, sampling, usage, tool-iteration reuse
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.
Summary
Fixes #603. Makes the multi-slot path serve real OpenAI agent clients. Verified end-to-end against a production agent gateway (~10k-token system prompt, 31 tool schemas, per-chat title generation): tool calls round-trip (including two parallel calls in one turn), tool iterations reuse the session KV, interleaved title calls no longer evict conversations, and clients finally see real usage numbers.
JinjaChatFrame::render_messageswith the model's chat template and the request'stools— the same rendering the daemon path uses; the hand-rolled ChatML frame stays as the no-template fallback. TypedMessage/ToolCallmapping from OpenAI messages (assistanttool_callsecho,toolrole,tool_call_id).Qwen35Emit(begin/observe/finish) — the same EosFilter + think/tool-routing + post-hoc-grammar aggregate the daemon drives, so both backends classify output identically. Tool grammar is gated byqwen35_grammar_onexactly like the daemon (withheld tools ⇒ matcher inactive; the Hermes-JSON envelope matcher otherwise rejects qwen3.6's native XML at the second body token). Slots-specific config: nostopsequences yet, andmax_think = 0— a think force-close needstake_forcedtoken injection into the slot's KV, which the engine does not support. Both are listed as known gaps rather than silently ignored.SessionTable::find_reentrymatches an EQUAL convo key (same user turns — a tool round), and the client sends acontinuation_suffix_tool_resultssuffix rendered in the official template's<tool_response>shape. Same strict-KV-extension construction ascontinuation_suffix: nothing is re-rendered, so the DeltaNet state stays valid. Without it, every agent iteration cold-prefilled the conversation, becausefind_continuationrequires the key to be one user turn ahead.SubmitRequestcarriestemperature/top_p/top_k/seed; admit installs them into the slot'sSlotSampleParams— per SP4's own session-state design. Absent fields mean greedy, the previous behaviour.Event::Accepted { reused, prefill }/Event::Done { generated }; the completion'sdoneenvelope carriescached_tokens/prefill_tokens/tokens, whichcompletion_usagealready projects. Closes the "usage.completion_tokens is always 0" gap from feat(serve): multi-slot batched attention — 3–4 concurrent agents on one GPU (SP1–SP7) #573's self-review and makes prefix-cache hits client-visible viaprompt_tokens_details.cached_tokens.none/reasoning_budget_tokens: 0/max_think_tokens: 1→enable_thinking = falserendering, matching the daemon's contract. Before this, areasoning_effort: "none",max_tokens: 128structured call burned its whole budget inside<think>and returned empty content.Deliberately not included:
response_format(#604) andGET /props(#605) — both are RFCs awaiting direction.Stacked on #607 and #608 — review those first; this branch contains their commits.
Which crate(s) does this touch?
crates/hipfire-runtime(serve protocol event fields,continuation_suffix_tool_results,SessionTable::find_reentry)crates/hipfire-arch-qwen35(serve_engine: counter emission, per-request sample params, reentry match)hipfire-cliserve slots path)Test plan
cargo build --release --workspace --features deltanetcleancargo test --lib --workspace --features deltanetpasses (incl. newfind_reentryand serve-protocol tests)finish_reason = tool_callswith correct arguments, a tool iteration reuses the session — engine tracecontinuation HIT — reused 375 of 423 tokens, responseusage.prompt_tokens_details.cached_tokens: 375; chat switches and interleaved title calls reuse 10k+ contexts with 30–50-token prefillsspeed.arch-fastpasses in the gate belowscripts/serve_concurrency_gate.shnot run as acceptance evidence here. Its speedup floor (1.30×) was locked on a 128 GB gfx1151 box; on this 24 GB host the gate cannot produce a comparison, because the beta side it would be compared against is exactly what #602 fixes — beta's engine OOMs before the server accepts a request. Worth a re-measure on hardware where both arms can run.change_gate telemetry
unit.env-docs— 173 findings, every one in a crate this diff does not touch (quantize, gemma4, deepseek4, redline, ...). No finding names a file in this PR.unit.no-gpu-control—hipfire-tui'sbounded_run_stdout_fill_is_not_falsely_timed_outfails identically on pristine beta (timing-sensitive dashboard test).serve.agentic.a3b-fast—zero tokens emittedon pristine beta too.serve.battery.qwen35-4b/-9b— the route argv asks--thinking medwithmax_tokens180/300, whichserve_harness.pyrejects before touching the GPU (the harness defect PR fix(serve): emit Qwen reasoning content #572 documented).