Skip to content

feat(server): serve Florence-2 through a seq2seq worker loop - #1083

Merged
inureyes merged 5 commits into
mainfrom
feature/issue-1073-florence2-seq2seq-server
Aug 7, 2026
Merged

feat(server): serve Florence-2 through a seq2seq worker loop#1083
inureyes merged 5 commits into
mainfrom
feature/issue-1073-florence2-seq2seq-server

Conversation

@inureyes

@inureyes inureyes commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Gives mlxcel-server a 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.content carries the same human-readable text the CLI prints, and the parsed coordinates additionally arrive as JSON in the mlxcel-specific message.florence2_result extension field, following the existing reasoning_content optional-field convention.

What changed

  • src/server/florence2_worker.rs (new): batch-1 serving loop off the shared mpsc request channel, following the DiffusionGemma / LLaDA-2 precedent in diffusion_worker.rs. Each request runs parse_task_prompt (the same parser the CLI -p flag uses), boundary validation for the 7 input-taking task modes, image decode under the configured ImageInputLimits, then Florence2VlmModel::run_task_with_cancel.
  • 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 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 server message.content is byte-identical to 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) in original-image pixels.
  • src/server/types/response.rs, src/server/routes/chat.rs, src/server/model_provider.rs: GenerationResult.structured_output transports the parsed result from the worker, and ChatMessage.florence2_result (skip_serializing_if optional) 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 typed text parts; 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 generic User: 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 on Florence2RunOutput for 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: docs/supported-models.md states the family is servable and documents the serving semantics; docs/responses-api.md gains an "mlxcel extension fields" section documenting florence2_result.

Security requirements from #855, honored

  • Image decode routes through 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).
  • Task-prompt input is validated at the request boundary for all 7 input-taking modes: at most 2048 bytes, no control characters, the 4 region tasks require exactly <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 COCO val2017/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):

cat: [17.0, 54.0, 318.4, 471.6]
cat: [346.6, 25.7, 639.0, 372.7]
couch: [0.3, 0.2, 639.0, 479.3]
remote: [40.6, 73.7, 175.7, 116.9]
remote: [333.1, 76.6, 370.2, 186.5]

Server <OD> response (bf16): content is byte-identical to the CLI block above, and florence2_result carries {"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: server content matches 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-4bit served through the same path: <CAPTION> returns Two 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:

  • 20000x20000 PNG (380 KB payload, 400M decoded pixels): 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 single delta.content chunk followed by the finish_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_cancel and dropped when it returns, so per-request isolation is structural. The model-level test sequential_requests_reuse_no_encoder_state pins 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.md states 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

  • The issue's "worker variant that runs the encoder pass once per request, caches the encoder output, and drives the cross-attention decode loop" already existed as Florence2Model::generate_greedy from 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.
  • Greedy decode means sampling parameters are accepted and ignored; documented in docs/supported-models.md.
  • The structured field rides the non-streaming chat completions surface; streaming and /v1/responses return the rendered text only, documented in docs/responses-api.md.

Test plan

  • cargo clippy --profile test-fast --features metal,accelerate --all-targets -- -D warnings (clean)
  • cargo fmt --check
  • cargo 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)
  • Real-checkpoint HTTP validation as above (bf16 and 4-bit, CAPTION / OD / grounding parity, five rejection paths, streaming, repeat-request isolation)

Closes #1073

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
@inureyes inureyes added type:enhancement New features, capabilities, or significant additions priority:medium Medium priority area:models Model architectures, weights, loading, metadata area:inference Generation, sampling, decoding (incl. speculative, DRY) status:review Under review labels Aug 7, 2026
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
@inureyes

inureyes commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Implementation Review Summary

Intent

Make Florence-2 reachable over OpenAI-compatible HTTP by giving mlxcel-server a seq2seq worker path, honoring the #855 security handoffs, and removing the #856 startup refusal.

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 chat_template in tokenizer_config.json, no chat_template.jinja, no chat_template.json, and model_type is exactly florence2), decode_request_images does apply current_image_input_limits() before any pixel work, the renderer moved without a behavior change (diffed the removed CLI copy against render.rs; the only difference is the dropped _ => String::new() arm, which is an improvement since #[non_exhaustive] no longer forces a wildcard in-crate), and every test count in the test plan reproduces.

Two integration questions worth recording because they are not obvious from the diff:

  • No spawn path can hand Florence-2 to a decoder-only loop. Tensor-parallel and pipeline-parallel requests route through the same spawn_model_worker_with_batch_config, so they hit the new branch. The XLA worker never builds a LoadedModel; it loads through XlaBatchEngine, which cannot load this architecture and fails at load. The acceptance criterion holds.
  • The two-phase generation timeout is safe here. Phase 1 blocks indefinitely by design, and this worker emits nothing until generation completes, so the whole run sits in Phase 1 and the bounded decode-hang window never applies.

Findings Addressed

Fixed in cf4faa86:

  • Cancelled requests paid a full encoder pass before the first cancellation poll (MEDIUM). generate_greedy_with_cancel polls once per decode step, but that poll only begins after the encoder. With serial serving, a request queued behind another whose client had already disconnected still paid for the DaViT tower and the bidirectional BART pass over the fused sequence, which is the expensive half. handle_florence2_request now checks the flag first.
  • The request-to-response mapping had no unit coverage (MEDIUM). Issue feat(server): serve Florence-2 through a seq2seq worker loop #1073 asks for it explicitly. FLORENCE2_MEDIA_UNSUPPORTED_MSG and FLORENCE2_IMAGE_REQUIRED_MSG were pub(crate) with no test referencing them, and the "length" versus "stop" choice was untested. Extracted reject_media, reject_image_count, and florence2_finish_reason as pure functions with tests, matching the reject_audio_video / diffusion_finish_reason_str shape diffusion_worker.rs already uses. Worker tests 12 to 15.
  • MAX_TASK_INPUT_BYTES documented a property the server path does not have (LOW). The comment said the bound refuses a hostile payload "before tokenization", but the dispatch thread pre-tokenizes the rendered prompt (issue perf(server): incremental detokenization and per-token streaming overhead reduction #633) before the worker sees it. The comment now states what the bound actually does, and notes that the encoder's max_position_embeddings check can reject a shorter input because the fused sequence carries the image's projected tokens ahead of the prompt.
  • response_format accepted and silently ignored (MEDIUM, documented not fixed). options.structured is consumed only in batch/scheduler.rs, so no single-stream worker applies it. docs/supported-models.md now says so for this family.
  • Stale tensor-parallel placeholder comment (LOW) still described Florence-2 as CLI-only.

Remaining Items

These 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.

  • --max-queue-depth is not honored (MEDIUM). can_accept_request() reads batch_metrics.queue_depth(), which only BatchScheduler updates. The legacy worker passes usize::MAX for this explicitly, so the behavior is at least deliberate there.
  • Usage prompt_tokens excludes the 577 projected image tokens (LOW). It is documented on the field; reporting the fused length means plumbing it out of Florence2Model::encode.
  • Declared versus resolved image cardinality is not cross-checked (LOW). A request declaring two images where one fails resolution is accepted as one image. This matches the convention MediaRequestMetadata's own doc records for MLX and diffusion workers.

One item from an automated first pass did not reproduce: malformed client requests do not return HTTP 500. ErrorResponse::new hard-codes StatusCode::BAD_REQUEST; only the error.type string reads server_error, which is pre-existing across the whole server.

Verification

  • All stated requirements implemented (all seven acceptance criteria; cross_cache_is_one_shot_and_self_cache_grows already covered the encoder-cache-reuse criterion)
  • No placeholder/mock code remaining
  • Integrated into project code flow (both MLX spawn paths, chat route, response type, chat template, startup)
  • Project conventions followed (single-stream worker precedent, reasoning_content extension-field convention, // Used by: comments on the shared renderer, upstream URLs not local references/ paths)
  • Existing modules reused where applicable (model-owned generate_greedy pipeline rather than a second encoder-cache layer; decode_request_images; parse_task_prompt shared with the CLI)
  • No unintended structural changes (the renderer relocation is the only move, and it is what makes CLI/HTTP byte-identity structural)
  • Tests pass: clippy clean, cargo fmt --check clean, --lib florence2 186 passed, --lib server:: 1655 passed

Technical report committed in e660c1fd (TECHNICAL_REPORTS/1083-florence2-seq2seq-server-20260808.{en,ko}.md).

@inureyes

inureyes commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Security and performance review

Scope: the seq2seq worker and everything it newly exposes to the network. No CRITICAL or HIGH findings, so no fix commits were pushed. Verified with cargo clippy --profile test-fast --features metal,accelerate --lib --tests -- -D warnings (clean) and cargo test ... --lib server::florence2_worker (15/15).

Verified sound

  • parse_region_bins against pathological input. No panic path. Every slice index lands on a char boundary (<loc_ and > are ASCII, so find('>') returns a boundary offset and end + 1 steps past a one-byte char). The digit run is capped at 3 ASCII digits before parse::<u16>(), so no overflow; the loop is fixed at four iterations; rest.is_empty() rejects trailing junk.
  • No panic reachable from hostile input. This matters because the loop runs inside run_core_thread_or_abort("model-worker"), which aborts the process. Audited parse_task_prompt, validate_task_input, scan.rs (leading_phrase, location_bins, strip_bare_loc_prefix, box_bins), parse.rs, postprocess.rs, coords.rs, render.rs. All slicing is boundary-safe, location_bins always advances so it cannot spin, parse_bin uses unwrap_or(i32::MAX), box_bins uses chunks_exact(4), and parse_ocr checks group presence and length before copy_from_slice. image 0.25.10 does carry an assert!(width != 0 || height == 0) in the resize path, but the build enables only png, jpeg, webp, none of which can express a zero dimension, so it is unreachable.
  • The 2048-byte input bound cannot produce an out-of-range position gather. Florence2Model::encode_fused rejects seq > max_position_embeddings with an Err before Florence2Encoder::forward builds arange(POSITION_OFFSET, POSITION_OFFSET + seq) against the learned table. The worker turns that into a GenerateEvent::Error and keeps serving. The doc comment on MAX_TASK_INPUT_BYTES describes this accurately.
  • Decode compute is bounded independently of max_tokens. generate_greedy_with_cancel breaks on cache.offset() >= max_position_embeddings, so a "max_tokens": 100000000 request still costs at most max_position_embeddings decode steps.
  • The angle-bracket rejection is a complete defense against added-token smuggling on this checkpoint. All 1029 added tokens in Florence-2-base-ft-bf16/tokenizer.json are angle-bracketed, and no non-angle-bracketed vocab entry is marked special. Florence2Task::expand uses a single str::replace("{input}", text), so {input} inside the caller's own text is not re-expanded.
  • Integer and JSON handling. A non-finite coordinate serializes as null (serde_json maps Number::from_f64(NaN) == None to Value::Null) rather than panicking, and a pathological <loc_...> digit run saturates at i32::MAX in parse_bin, so florence2_result stays finite and well formed. boxes and labels are built index-aligned in every post_process arm, so render_task_result's zip cannot silently drop instances that the JSON still reports.
  • Per-request memory lifecycle. clear_memory_cache() covers both the success and the generation-error arms, and is correctly omitted on the early-return arms that do no MLX work. The Err arm covers every failure that allocates (preprocess, the encoder length rejection, argmax). Encoder output, seq2seq cache, and pixel tensor are all locals of run_task_with_cancel and drop on return.
  • Worker routing. All three spawn paths are covered: spawn_model_worker_with_batch_config and spawn_legacy_model_worker both branch before any scheduler setup (including before the speculative dispatch is consumed), and spawn_xla_model_worker never receives a LoadedModel. startup.model_path is not mutated between the is_florence2 computation and the warmup gate, so the flag cannot go stale.
  • Template reachability, one direction. "florence2" is the only key selecting FLORENCE2_CHAT_TEMPLATE, and it is the same key detection.rs maps to ModelType::Florence2VLM, so no other family can reach it.
  • Prompt cache. PromptCacheRequestContext is plain data with no reservation, and the seq2seq worker ignores options.prompt_cache_ctx, so there is no KV cross-contamination and nothing leaks when the context goes unused.

MEDIUM (reported, not fixed)

M1. builtin_chat_template reads model_type raw while get_model_type normalizes it. src/server/chat_template.rs matches the literal "florence2" on a plain serde_json::from_str of config.json, but src/models/detection.rs::get_model_type runs sanitize_config_json and to_ascii_lowercase first. A checkpoint that routes to ModelType::Florence2VLM through the normalized path but misses the raw literal (upstream casing such as "Florence2", or a config carrying a bare NaN / Infinity literal) loads, starts, and reaches the seq2seq worker, but falls back to the generic User: / Assistant: template, which makes parse_task_prompt reject 100% of requests. All five local checkpoints are lowercase and strict JSON, so this is latent rather than live. The same divergence already exists for jvlm / jina_vlm, but the blast radius is larger here: a wrong template degrades quality for Jina-VLM and produces total unavailability for Florence-2. Suggested fix: resolve the type through crate::models::get_model_type(model_path) in builtin_chat_template, or apply the same sanitize plus lowercase.

M2. mlxcel chat still hands a Florence-2 checkpoint to the decoder-only REPL. src/commands/chat.rs rejects DiffusionGemma and Llada2Moe from the interactive loop but not Florence2VLM, so mlxcel chat -m models/Florence-2-base-ft-bf16 drives the trait-completeness forward and prints garbage. This predates the PR (it arrived with #856 and is not in this diff), but the PR's edit to src/loaded_model.rs now asserts that "the CLI routes it to the Florence-2 task pipeline before the autoregressive loop", which holds for generate and not for chat. Either add Florence2VLM(_) to that match arm or narrow the doc claim to generate.

LOW (reported, not fixed)

L1. parse_task_prompt echoes the whole prompt into its error messages. {trimmed:?} on the not-a-task-marker and unclosed-marker arms is now network reachable. It is bounded by axum's default 2 MB body limit on /v1/chat/completions (the 25 MB DefaultBodyLimit applies only to the audio sub-router), but Debug escaping of a control-character-heavy prompt inflates roughly sixfold, so a 2 MB prompt builds and returns a ~12 MB error string. Truncating the echoed prompt to its first ~120 characters would close it. Note validate_task_input already gets this right: only the region arm echoes text, and only after the 2048-byte bound has passed.

L2. clear_memory_cache() on every request. Correct and consistent with the diffusion_worker precedent, but on a serial batch-1 worker with no other MLX consumer it drops the encoder workspace the next request immediately re-allocates. Per the project's performance-issue rules this needs a measurement at the real call site before changing, so it belongs in a follow-up rather than here.

L3. Prompt-cache context is built and discarded for Florence-2 requests. When --prompt-cache is installed, build_prompt_cache_request_context still hashes the full image payload into a MultimodalDigest for a key the seq2seq worker never reads. Harmless, but it is a per-request hash over the image bytes that is pure waste on this path.

Out of scope by prior agreement and not re-litigated: serial batch-1 serving, the --max-queue-depth gap, usage.prompt_tokens excluding the 577 image tokens, declared-vs-resolved image cardinality, and the content-plus-florence2_result response shape.

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
@inureyes inureyes added status:done Completed and removed status:review Under review labels Aug 7, 2026
@inureyes
inureyes merged commit 608c7a6 into main Aug 7, 2026
8 checks passed
@inureyes
inureyes deleted the feature/issue-1073-florence2-seq2seq-server branch August 7, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) area:models Model architectures, weights, loading, metadata priority:medium Medium priority status:done Completed type:enhancement New features, capabilities, or significant additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(server): serve Florence-2 through a seq2seq worker loop

1 participant