Repository navigation
refactor: route mlxcel run and chat through the in-process server engine - #2224
Merged
Merged
Conversation
mlxcel run (with and without -p) and the chat REPL drove CxxGenerator and kept their own copies of the chat template rendering, reasoning split and sampling defaults. They now start the model server in-process at one slot and submit requests through the /v1/chat/completions request path, the way llama-cli is a client of llama-server but without a loopback socket (ADR 0007, CLI front end). - server::in_process::InProcessServer builds what start_server builds minus the listener (config, prompt cache store, ModelProvider worker, tokenizer and template via the new startup::load_chat_front, AppState). The engine probe's ServerEngine now starts on it instead of its own copy. - routes::chat_generation holds the chat handler's validation prefix (admit_chat_request) and render/option build (prepare_chat_generation); the streaming and non-streaming handlers and InProcessServer::chat all call them. InProcessServer::chat consumes the worker stream through StreamFilter, parses tool calls, records the reasoning echo and submits the next-turn warm-up; complete() is the /v1/completions path for --no-chat-template. - cli::in_process_client maps every CLI sampling flag onto the matching mlxcel-server option, with ADR 0007 defaults (server windows for repeat_last_n and DRY, b10621 DRY breakers, greedy CLI base sampler). run gains the server-only sampling flags, --stop, --draft-model and --draft-kind. - The REPL sends the whole transcript with a per-conversation prompt_cache_key (/clear moves to a new one), supports /image, and Ctrl-C cancels the in-flight request through the worker's cancel flag. - mlxcel-engine-parity gains arm e:run, the run -p settings and request through the in-process server, compared with the dense and paged server arms. Refs #2173
inureyes
added
status:in-progress
The chat REPL and run -p now get the reasoning split from the server's StreamFilter, so the CLI's own splitter in src/reasoning_stream.rs had one remaining user, the one-shot generate display. reasoning_display keeps the three helpers the server chat routes also call (primed-prompt detection, its close marker, the reasoning-only predicate) and renders a full decoded reply through StreamFilter, so one splitter decides what is reasoning on every path. A one-shot generate reply now also loses the tool-call and turn markup StreamFilter strips. mlxcel-engine-parity's chat-request arms now compare like with like: e:run (the settings run -p --temp 0 builds) against f:server (the same chat request on the mlxcel-server default configuration). The a to d arms use the harness's CLI-style render, which on Qwen3 primes a different thinking prefix than the server's render, so comparing e:run with them measured the template, not the engine. On GB10 under MLXCEL_SDPA_DETERMINISTIC=1, f:server vs e:run is identical for Qwen3-1.7B 4-bit and Llama-3.2-1B 4-bit. A cancelled turn no longer prints the reasoning-only notice. Refs #2173
mlxcel generate dispatched --image, --audio and --video to the vision and audio towers through compute_vlm_embeddings, a CLI copy of the per-family dispatch the model worker runs in prepare_request_vlm_embeddings. server::local_media::prepare_local_vlm_embeddings reads the files named on the command line (videos by trusted path) and calls the worker's function, so the CLI copy and the roughly 1,800 lines of per-family helpers only it used are gone. The shared vlm_runtime entry points are renamed prepare_vlm_embeddings*, which leaves no compute_vlm_embeddings name in the tree. On GB10, generate on qwen2.5-vl-3b-instruct-4bit with an image and on gemma-4-e2b-it-4bit with an audio clip produces the same prompt ids and the same greedy token ids as the pre-change binary (MLXCEL_PRINT_TOKEN_IDS). run -p also prints the speculative acceptance counters when a drafter ran; --draft-model with qwen3.5-4b-dflash on qwen3.5-4b-4bit runs through DFlashTargetModel, and gemma-4-12b-it-assistant-4bit with --draft-kind mtp through the MTP burst. Refs #2173
docs/CONTINUOUS_BATCHING.md's token-exactness section replaces the refusal to promise CLI and server parity with the equivalence invariant (run -p equals the server for the same chat request under MLXCEL_SDPA_DETERMINISTIC=1, gated by engine-parity arms e:run and f:server) and lists the two measured divergences that remain (prompt-cache suffix forwarding, live batching). The README run section explains the in-process server, the flag mapping, --draft-model and the REPL commands. ADR 0007 records the two refinements Phase 5 made (prompt cache off for run -p, the -1 repeat window), and ADR 0009 notes that the CLI reaches Engine through the in-process scheduler rather than a CLI StepRowHooks. The run --help text describes the server mapping. Refs #2173
6 of 7 tasks
inureyes
added
status:review
inureyes
marked this pull request as ready for review
October 7, 2026 17:23
On main, mlxcel run -p --image ... --image-soft-tokens N reached the Gemma 4 vision processor through generate's compute_vlm_embeddings. After the move to the in-process server, run -p and the chat REPL build image_url parts without the budget, so the flag was parsed and then silently ignored. messages_json now writes the budget on every image_url part as the server's max_soft_tokens extension, which the request path already validates against the Gemma 4 ladder, and ChatOptions carries it for the REPL (from run and from generate without -p). A transcript test checks that the server parses the budget back from the parts. Refs #2173
Review fixes on PR #2224 for the server half of the in-process client. - InProcessServer::start now runs the startup warmup through the same helper as start_server (run_startup_warmup): VoiceChat and the image-task seq2seq families skip it, and a failed warmup is logged instead of aborting the CLI. - InProcessServer::chat records reasoning in the re-echo store only when the reasoning format emits reasoning_content, as the streaming handler does; under --reasoning-format none nothing is stored. - mlxcel generate keeps its Inkling media layouts: audio and video take the server's Inkling functions with Structured after the chat template and Plain under --no-chat-template. The server's Ordered audio path needs the ordered media markers only the chat route renders, so it failed on generate's prompts. - Unit tests that need no checkpoint for the in-process request bodies, the raw completion option build, the cancel and tool-call gates, delta delivery, local_media file and video resolution, the Inkling layout selection, and the warmup skip set. Validated with clippy -D warnings (lib, tests, bins) and the server::in_process, server::local_media, engine_probe, server::routes::chat and startup warmup lib tests on CUDA. Refs #2173
Review fixes on PR #2224 for the CLI half of the in-process client. - Ctrl-C outside a turn restores the default SIGINT disposition and re-raises it (_exit(130) as a fallback) instead of calling process::exit from the signal thread, so no atexit handlers or static destructors run while the worker may hold the GPU. - The REPL reads each image once, when --image or /image attaches it, and keeps the data URI in the transcript; a file deleted after its turn no longer breaks every later turn. run -p reads --image before the model loads. - run -p prints the same base-model notice as the REPL when the checkpoint has no chat template. - generate without -p passes a drafter to the in-process server only with an explicit --draft-kind dflash|mtp. --draft-model alone means a classic draft model there, which the server does not run, so the REPL ignores it with a notice, as it effectively did before. - run -p with --no-chat-template and --image stays on the generate flow, and the raw REPL refuses /image, because the raw completion path carries no media and would drop the image silently. - Short-flag detection requires a numeric attached value, so a prompt such as -tfoo no longer counts as -t; generate shares the check. Validated with clippy -D warnings (lib, tests, bins) and the commands:: bin tests and cli_input lib tests on CUDA. Refs #2173
The server (startup::load_chat_front) and mlxcel generate (load_cli_prompt) default enable_thinking=true when the tokenizer has a think-marker pair, Gemma 4 excepted. The engine-parity harness's own render and mlxcel-bench-decode's render do not, so on a thinking checkpoint the arms on the harness render see a different prompt than e:run and f:server. The token-exactness section now names this as a known divergence that Phase 6 (#2176) removes, and the render_chat_prompt doc comment no longer claims to match generate. Refs #2173
The base-model notice no longer reloads the tokenizer and re-parses the chat template from disk on every run or REPL start. The in-process server already holds both, so `InProcessServer::uses_generic_chat_template` answers from the loaded processor (`ChatTemplateProcessor::is_generic_default`), and the question is only asked when the notice can print. The REPL re-sends the whole transcript with its images every turn, so an image past the server's per-request cap made every later turn fail until /clear. `/image` and `--image` now refuse an attachment that would exceed the cap, which is read from `mlxcel::current_image_input_limits` rather than duplicated. A turn error in `run -p` and a line-editor failure in the REPL now join the model worker (`shutdown_then_fail`) before the error propagates, so the process does not exit with the worker still holding the model. A second Ctrl-C while the turn's cancel flag is already set ends the process the way the between-turns path does, instead of being swallowed when the cancel is not taking effect. Each streamed delta is written straight to the locked stdout with a flush, without allocating a String or format! per delta. Validation: `cargo test --features cuda --bin mlxcel -- commands::` (202 passed), `--lib` filters server::in_process, server::local_media, server::routes::chat, startup, cli_input and the new chat_template test (338 passed), all with --test-threads=1 under gpu-lock. The turn-error shutdown path needs a loaded checkpoint and has no unit test. Refs #2173
The `prepare_and_compute_vlm_embeddings` family was renamed to `prepare_vlm_embeddings` in this PR, so the OpenXLA multimodal design note now names the current function and its location. The `reasoning_display` module comment no longer points at the deleted `src/reasoning_stream.rs`. The README run section now says a second Ctrl-C quits and that `/image` and `--image` refuse past the server's image cap. The `cli_flag_was_set` doc comment named a flag spelling that does not exist (`--name`), which `tests/cli_help_consistency.rs` rejects for src/commands; it is reworded. Validation: `--test cli_help_consistency` (27 passed), `--test dead_doc_pointers`, `--test vlm_wrapper_capability_delegation`, `--test model_loader_stdout_guard`, `--test llama_model_source_cli`, `--lib prefill_span_coverage structure_tests`, `insert_apache_header.py --check`, `cargo fmt --check` and `cargo clippy --profile test-fast --features cuda --lib --tests --bins --examples -- -D warnings` all pass. Refs #2173
This was referenced Oct 8, 2026
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
1 participant
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
Epic #2166 Phase 5.
mlxcel run(with and without-p) and the chat REPL are in-process clients of the server engine: they start the server worker at one slot and send each turn through the/v1/chat/completionsrequest path, with no HTTP listener.server::in_process::InProcessServerbuilds whatstart_serverbuilds minus the listener (config, prompt cache store,ModelProvider, tokenizer and template viastartup::load_chat_front,AppState). The engine probe'sServerEnginenow starts on it.routes::chat_generationholds the chat handler's validation and render/option build; both handlers andInProcessServer::chatcall it.chatconsumes the worker stream throughStreamFilter, parses tool calls, records the reasoning echo and submits the next-turn warm-up.--no-chat-templateuses the/v1/completionspath.prompt_cache_key(/clearmoves to a new one),/image <path>, Ctrl-C cancels the request through the worker's cancel flag.src/reasoning_stream.rsis gone;reasoning_displayrenders throughStreamFilter.compute_vlm_embeddingsis gone;mlxcel generateprepares media throughprepare_request_vlm_embeddings(server::local_media), and the sharedvlm_runtimeentry points are renamedprepare_vlm_embeddings*.rungains--draft-model,--draft-kind,--draft-block-size, and prints speculative acceptance.Flag mapping (ADR 0007)
Every CLI sampling flag maps onto the
mlxcel-serveroption of the same meaning; an unset flag keeps the server default.--temp,--top-k,--top-p--temp/--top-k/--top-pwith the*_was_setrulegeneration_config.jsonstill wins--min-p,--typical-p,--top-n-sigma,--repetition-penalty,--dry-multiplier/-base/-allowed-length,--seed--repeat-last-n--repeat-last-n(-1maps to the widest window)-1to 64 when unset (inert without a penalty)--dry-penalty-last-n--dry-penalty-last-n-1to 64 when unset (inert without DRY)-n--n-predict-1= context window, one slot)--p-less, new--stopp_less,stop--frequency-penalty,--presence-penalty,--xtc-*,--mirostat*,--dynatemp-*,--dry-sequence-breaker--adapterkv_cache_mode, adapterAlso: loop detection follows the server's resolution; prefill chunk 2048; storage dense at one slot; prompt cache on for the REPL, off for
run -p. No flag was removed.Behaviour changes
runand the REPL render turns with the server's default chat format instead of the raw prompt. Both print a base-model notice;--no-chat-templatekeeps raw text.run -pno longer echoes the prompt before the reply. Onlygenerateprinted it, and no doc treats it as output contract.generatewithout-p(the REPL) uses a drafter only with--draft-kind dflash|mtp.--draft-modelalone is a classic draft model ongenerate, which the server does not run, so the REPL ignores it with a notice (before this PR the REPL ignored speculative flags).--no-chat-templatewith--image:run -pstays on thegenerateflow, and the raw REPL refuses/image, since the raw completion path carries no media.generaterenders prompts the way the server does (enable_thinking=truewith think markers, Gemma 4 excepted). The parity harness render andmlxcel-bench-decodedo not; that divergence is documented in docs/CONTINUOUS_BATCHING.md and goes away in Phase 6 (refactor: run mlxcel generate on the engine and retire CxxGenerator #2176).Changes during review
--image-soft-tokensrides onrun's image parts (ebd4dee).InProcessServer::startsharesstart_server's warmup decision: VoiceChat and image-task seq2seq skip it, failure is non-fatal (0bbc776).reasoning_content(0bbc776).generatekeeps its Inkling audio and video layouts (Structuredtemplated,Plainraw); the server'sOrderedaudio path needs markers only the chat route renders (0bbc776).server::in_processandserver::local_media(0bbc776).process::exiton the signal thread; REPL images are read once and kept as data URIs; short-flag detection ignores-p -tfoo(f4285ba)./imageand--imagerefuse attachments past the server's per-request image cap (16 by default) instead of failing every later turn; a turn error joins the worker before exit; a second Ctrl-C during a turn quits; deltas are written to stdout without a per-delta allocation.Test plan
-D warnings(lib, tests, bins); narrow CUDA lib/bin tests (commands::, chat routes,server::in_process,engine_probe,request_options,stream_filter,model_provider,batch::scheduler,reasoning_display,vlm_runtime)MLXCEL_SDPA_DETERMINISTIC=1 mlxcel-engine-parity:f:servervse:runidentical on Qwen3-1.7B 4-bit and Llama-3.2-1B 4-bit/clear, reasoning hidden and shown; image turn on Qwen2.5-VL-3Brun -p --draft-model: DFlash (qwen3.5-4b) and MTP (gemma-4-12b assistant)generateimage and audio: same prompt and greedy ids as the pre-change binary-D warnings(lib, tests, bins);server::in_process,server::local_media,engine_probe,server::routes::chat,cli_input, startup warmup lib tests andcommands::bin tests on CUDArun -pon Qwen3-1.7B and Llama-3.2-1B; REPL on Qwen2.5-VL-3B with/image, the file deleted before the follow-up turn (answered), then SIGINT at the prompt (process died by SIGINT). No Inkling checkpoint is local; its layout selection is unit-tested-D warnings(lib, tests, bins, examples), license headers;--test-threads=1modulesserver::in_process,server::local_media,server::routes::,server::tool_calls,server::request_options,startup,cli_input,engine_probe,reasoning_display,server::chat_template,server::model_worker,server::model_provider,server::batch::scheduler,multimodal::,prefill_span_coverage, bincommands::;--testvlm_wrapper_capability_delegation,dead_doc_pointers,model_loader_stdout_guard,cli_help_consistency,llama_model_source_cli-n 400, deterministic):f:servervse:runidentical on Qwen3-1.7B and Llama-3.2-1B; the other pairs match the Phase 0 baseline (only the documented prompt-cache rows diverge)run -pon Llama-3.2-1B; REPL on Qwen3-1.7B (recall across turns, Ctrl-C mid-essay prints[interrupted],/clearforgets the name); REPL on Qwen2.5-VL-3B (/imageturn names the red square, blue circle and green triangle, follow-up answers)Closes #2173