feat(server): serve Florence-2 through a seq2seq worker loop - #1083
Conversation
Florence-2 worked end to end through the CLI since issue #856, but mlxcel-server refused the checkpoint at startup because its BART-style encoder-decoder generation (one encoder pass per request, then a cross-attention greedy decode against the cached encoder output) cannot run on the decoder-only worker loops. This gives the server a dedicated single-stream seq2seq worker following the DiffusionGemma / LLaDA-2 precedent, removes the startup refusal, and makes the family reachable over OpenAI-compatible HTTP. Changes: - src/server/florence2_worker.rs (new): batch-1 serving loop off the shared mpsc request channel. Each request is parsed with the same parse_task_prompt the CLI uses, validated at the boundary (7 input-taking modes: 2048-byte bound, control-character rejection, strict <loc_a><loc_b><loc_c><loc_d> quadruple for the 4 region tasks, angle-bracket rejection for the 3 free-text tasks), image-decoded under the configured ImageInputLimits (decompression-bomb defense carried forward from issue #855), and run through Florence2VlmModel::run_task_with_cancel. Requests are served serially by design; the batched admission cost model for the encoder pass is unmeasured and documented as such. - src/server/model_worker.rs: both the batched and the legacy spawn paths branch LoadedModel::Florence2VLM into the new loop before any scheduler starts, so no code path can hand the checkpoint to a decoder-only loop. - src/models/florence2/render.rs (new): the CLI's answer renderer moved into the lib so server message.content is byte-identical to the CLI output, plus structured_task_json, which serializes the parsed result with upstream mlx-vlm / HuggingFace key names (bboxes, quad_boxes, polygons, labels, bboxes_labels, polygons_labels). - src/server/types/response.rs + src/server/routes/chat.rs + src/server/model_provider.rs: per the decision recorded on issue #1073, the non-streaming chat response carries the human-readable text in message.content and the parsed coordinates side by side in the mlxcel-specific message.florence2_result extension field, following the reasoning_content optional-field convention; the field is absent for every other family. - src/server/chat_template.rs: a built-in Florence-2 template renders the request messages' text verbatim (no role prefixes, no generation prompt) so the worker receives the task prompt exactly as the CLI -p flag would. - src/models/florence2/{model,runtime}.rs: cooperative per-decode-step cancellation and an encoder prompt-token count for the usage block; encoder output and decode cache remain call-local, so per-request isolation is structural. - src/server/startup.rs: refusal removed; the text-only warmup is skipped for this image-task family with an info log. - Tests: task-input validation and region-quadruple parsing (florence2_worker_tests.rs), renderer and JSON mapping (florence2_render_tests.rs), template verbatim rendering, response-field serialization, and a model-level sequential-request isolation test that proves a leaked encoder cache would change the answer. Validated on real checkpoints: for Florence-2-base-ft-bf16 and Florence-2-base-ft-4bit, HTTP answers for <CAPTION>, <OD>, and <CAPTION_TO_PHRASE_GROUNDING> on COCO 000000039769 are byte-identical to the CLI answers for the same model, image, and task; a 20000x20000 PNG decompression bomb, malformed and oversized task input, and angle-bracket smuggling are all rejected with named errors; streaming returns the full rendered answer as a single delta. Refs #1073
Review follow-up on the #1073 serving path. Three request-boundary gaps, none of which changes a served answer. Cancelled requests now return before the encoder pass. Serving is serial by design, so a request can sit in the channel while its client disconnects, and the only cancellation poll lived inside `generate_greedy_with_cancel`, which runs after the encoder. An abandoned queued request therefore still paid for the DaViT tower and the bidirectional BART pass over the fused sequence. `handle_florence2_request` now checks the flag first and answers `FLORENCE2_CANCELLED_BEFORE_START_MSG`. The media, image-cardinality, and finish-reason decisions moved into pure functions (`reject_media`, `reject_image_count`, `florence2_finish_reason`) with unit tests, following the `reject_audio_video` / `diffusion_finish_reason_str` shape the sibling single-stream worker already uses. Issue #1073 asks for request-to-response mapping coverage; before this the two message constants had no test referencing them and the "length" vs "stop" choice was untested. Two documentation corrections. The `MAX_TASK_INPUT_BYTES` comment claimed the bound refuses a payload "before tokenization", which is not true on the server path: the dispatch thread pre-tokenizes the rendered prompt (issue #633) before the worker sees it. The comment now says what the bound actually does and notes that the encoder's `max_position_embeddings` check can reject a shorter input, since the fused sequence carries the image's projected tokens ahead of the prompt. `docs/supported-models.md` now also states that a `response_format` structured-output constraint is accepted and ignored on this path, which was true but undocumented; the single-stream workers do not read `options.structured`, that constraint is applied by the batch scheduler. The stale tensor-parallel placeholder comment for `ModelType::Florence2VLM` still said the family is CLI-only. Validated with `cargo clippy --profile test-fast --features metal,accelerate --lib --tests` (clean), `cargo fmt --check`, `cargo test --lib server::` (1655 passed), and `cargo test --lib florence2` (186 passed). Refs #1073
Implementation Review SummaryIntent
The implementation matches the issue and the response-shape decision recorded on it. I re-verified the load-bearing claims rather than taking the PR body's word: the built-in chat template is genuinely reached (all five local Florence-2 conversions ship no Two integration questions worth recording because they are not obvious from the diff:
Findings AddressedFixed in
Remaining ItemsThese are properties of the single-stream worker class, not of this PR. DiffusionGemma and LLaDA-2 have the first, second, and fourth identically, so fixing them for one family would leave the tree inconsistent. They belong in one issue covering all three workers, and are written up in section 8 of the technical report.
One item from an automated first pass did not reproduce: malformed client requests do not return HTTP 500. Verification
Technical report committed in |
Security and performance reviewScope: the seq2seq worker and everything it newly exposes to the network. No CRITICAL or HIGH findings, so no fix commits were pushed. Verified with Verified sound
MEDIUM (reported, not fixed)M1. M2. LOW (reported, not fixed)L1. L2. L3. Prompt-cache context is built and discarded for Florence-2 requests. When Out of scope by prior agreement and not re-litigated: serial batch-1 serving, the |
Security-review follow-up on the #1073 serving path, two MEDIUM findings. builtin_chat_template matched the raw `model_type` string while `get_model_type` sanitizes the JSON and lowercases the value first, so a checkpoint that routes to Florence-2 (or Jina VLM) through the normalized path but misses the raw literal would silently fall back to the generic `User:`/`Assistant:` template; for Florence-2 that means the task parser rejects every request, total unavailability rather than degraded quality. Template selection now goes through `get_model_type` itself, so the same normalization that picks the model's worker picks its template, with a regression test using a mixed-case `model_type`. The interactive REPL (`mlxcel chat`) rejected DiffusionGemma and LLaDA-2 but still admitted Florence-2 into its autoregressive loop, whose `LanguageModel` forward exists for trait completeness only and would re-encode every step. The REPL now refuses the family with a pointer to the task pipeline, which also makes the `LoadedModel` doc claim (the CLI routes Florence-2 away from the autoregressive loop) true for both CLI surfaces. Validated with `cargo clippy --profile test-fast --features metal,accelerate --lib --tests -- -D warnings` (clean), the same for `--bins`, and `cargo test --profile test-fast --features metal,accelerate --lib chat_template::tests::` (69 passed). Refs #1073
…eport The bilingual report predated commit 94d5774; the findings table now carries the two MEDIUM fixes from the security pass (template selection normalized through get_model_type, REPL rejection of Florence-2) and the properties the pass verified without changes (panic-free region parser, bounded position gather, decode bound, angle-bracket coverage of the tokenizer's special tokens). Refs #1073
Summary
Gives
mlxcel-servera dedicated single-stream seq2seq worker for Florence-2 and removes the #856-era startup refusal, so the family is now reachable over OpenAI-compatible HTTP with answers byte-identical to the CLI. Implements the response shape recorded on issue #1073:message.contentcarries the same human-readable text the CLI prints, and the parsed coordinates additionally arrive as JSON in the mlxcel-specificmessage.florence2_resultextension field, following the existingreasoning_contentoptional-field convention.What changed
src/server/florence2_worker.rs(new): batch-1 serving loop off the sharedmpscrequest channel, following the DiffusionGemma / LLaDA-2 precedent indiffusion_worker.rs. Each request runsparse_task_prompt(the same parser the CLI-pflag uses), boundary validation for the 7 input-taking task modes, image decode under the configuredImageInputLimits, thenFlorence2VlmModel::run_task_with_cancel.src/server/model_worker.rs: both the batched and the legacy spawn paths branchLoadedModel::Florence2VLMinto the new loop before any scheduler starts, so no code path can hand a Florence-2 checkpoint to a decoder-only worker loop.src/models/florence2/render.rs(new): the CLI answer renderer moved into the lib so servermessage.contentis byte-identical to CLI output, plusstructured_task_json, which serializes the parsed result with upstream mlx-vlm / HuggingFace key names (bboxes,quad_boxes,polygons,labels,bboxes_labels,polygons_labels) in original-image pixels.src/server/types/response.rs,src/server/routes/chat.rs,src/server/model_provider.rs:GenerationResult.structured_outputtransports the parsed result from the worker, andChatMessage.florence2_result(skip_serializing_ifoptional) surfaces it on the non-streaming chat response; absent for every other family so the wire shape is unchanged.src/server/chat_template.rs: built-in Florence-2 template that renders the request messages' text verbatim (string content or typedtextparts; no role prefixes, no generation prompt), so the worker receives the task prompt exactly as the CLI would. Florence-2 checkpoints ship no chat template, and the genericUser:fallback would break the task-marker parse.src/models/florence2/model.rs,runtime.rs: cooperative per-decode-step cancellation (generate_greedy_with_cancel/run_task_with_cancel, polled once per step so a disconnected client aborts within one step) and an encoder prompt-token count onFlorence2RunOutputfor the usage block.src/server/startup.rs: startup refusal removed; the text-only"Hello"warmup is skipped for this image-task family with an info log instead of failing noisily.src/commands/generate_florence2.rs: now imports the shared renderer from the lib; behavior unchanged.docs/supported-models.mdstates the family is servable and documents the serving semantics;docs/responses-api.mdgains an "mlxcel extension fields" section documentingflorence2_result.Security requirements from #855, honored
decode_request_images/ImageInputLimits: an oversized or decompression-bomb payload is rejected before any pixel work (verified below with a 20000x20000 PNG that is only 380 KB on the wire).<loc_a><loc_b><loc_c><loc_d>with bins in 0..=999, and the 3 free-text tasks reject</>so location, sequence, or task markers cannot be smuggled into the encoder prompt.Validation on real checkpoints
CLI references and HTTP answers were produced with the same binaries at this commit, model
Florence-2-base-ft-bf16(and-4bit), image COCOval2017/000000039769.jpg(640x480, two cats on a pink blanket with two TV remotes).CLI
<CAPTION>(bf16):Two cats are sleeping on a pink blanket.Server request:
{"model": "florence2", "messages": [{"role": "user", "content": [{"type": "text", "text": "<CAPTION>"}, {"type": "image_url", "image_url": {"url": "data:image/jpeg;base64,..."}}]}], "max_tokens": 256}Server response (bf16, non-streaming, abbreviated to the choice):
{"message": {"role": "assistant", "content": "Two cats are sleeping on a pink blanket.", "florence2_result": {"task": "<CAPTION>", "kind": "text", "text": "Two cats are sleeping on a pink blanket."}}, "finish_reason": "stop"}CLI
<OD>(bf16):Server
<OD>response (bf16):contentis byte-identical to the CLI block above, andflorence2_resultcarries{"task": "<OD>", "kind": "bboxes", "bboxes": [[16.96, 54.0, 318.4, 471.6], [346.56, 25.68, 639.04, 372.72], [0.32, 0.24, 639.04, 479.28], [40.64, 73.68, 175.68, 116.88], [333.12, 76.56, 370.24, 186.48]], "labels": ["cat", "cat", "couch", "remote", "remote"]}(full f32 precision; content prints one decimal, matching the CLI renderer).Input-taking mode
<CAPTION_TO_PHRASE_GROUNDING> two cats on a couch: servercontentmatches the CLI line for line (two cats: [0.3, 0.2, 639.0, 479.3]and four more instances).Quantized checkpoint
Florence-2-base-ft-4bitserved through the same path:<CAPTION>returnsTwo cats laying on a pink blanket next to remotes.and<OD>returns the same five labels with slightly shifted boxes, both byte-identical to the 4-bit CLI answers for the same inputs.Rejection paths, all HTTP 400 with named errors and the worker kept serving afterward:
Image decode rejected by configured limits: Image size exceeds limit, before decode.<REGION_TO_CATEGORY> not-a-region:takes a region as exactly four location tokens <loc_a><loc_b><loc_c><loc_d> ... got "not-a-region".<CAPTION_TO_PHRASE_GROUNDING>with a 2799-byte input:the server accepts at most 2048 bytes.<CAPTION_TO_PHRASE_GROUNDING> a </s> cat:must be plain text without '<' or '>'.<NOT_A_TASK>:unknown Florence-2 task marker <NOT_A_TASK>; valid markers: ....Streaming (
stream: true): the whole rendered answer arrives as a singledelta.contentchunk followed by thefinish_reason: "stop"chunk, by design (post-processing needs the complete decode; incremental raw<loc_*>tokens would not match the non-streaming content). Re-running<CAPTION>after<OD>, grounding, and the rejection requests returned the identical caption, confirming request isolation end to end.Encoder cache isolation
The encoder output and the seq2seq decode cache are created inside
run_task_with_canceland dropped when it returns, so per-request isolation is structural. The model-level testsequential_requests_reuse_no_encoder_statepins it: request B served after request A must equal request B served fresh, and the test also proves the two requests' encoder outputs produce different decode logits, so a leaked or shared encoder cache would change the answer and fail the equality assertion rather than pass coincidentally.Batching semantics
Serving is one request at a time by design in this first landing: requests queue on the worker channel and are answered serially, exactly like the DiffusionGemma and LLaDA-2 loops. The issue allows this if documented;
docs/supported-models.mdstates the limitation and that no concurrent-throughput property is claimed, since the encoder pass has a different cost profile from the decode loop and no batched admission policy has been designed or measured.Notes relative to the issue text
Florence2Model::generate_greedyfrom feat(models): Florence-2 end-to-end integration and real-checkpoint validation (sub of #850) #856; the worker reuses that model-owned pipeline (now with a cancel hook) rather than duplicating an encoder-cache layer in the server, which is also what makes per-request isolation structural.docs/supported-models.md./v1/responsesreturn the rendered text only, documented indocs/responses-api.md.Test plan
cargo clippy --profile test-fast --features metal,accelerate --all-targets -- -D warnings(clean)cargo fmt --checkcargo test --profile test-fast --features metal,accelerate --lib florence2(183 passed)cargo test --profile test-fast --features metal,accelerate --lib server::florence2_worker::tests(12 passed)cargo test --profile test-fast --features metal,accelerate --lib server::types::response::tests(14 passed)cargo test --profile test-fast --features metal,accelerate --lib server::model_provider::tests(12 passed)cargo test --profile test-fast --features metal,accelerate --lib chat_template::tests::a_florence2(2 passed)Closes #1073