Skip to content

fix: small decode-path defects found while mapping CLI vs server decode #2177

Closed

Description

@inureyes

Problem / Background

Mapping the CLI and server decode paths for epic #2166 turned up two small defects that do not need to wait for the epic. Do both in a single PR, run the local gate once at the end, and keep pushes to a minimum. The speculative burst stream's missing loop detection (src/server/batch/speculative_burst.rs never calls detect_repetition_loop) is not included here, because #2168 (epic Phase 1) fixes it as one of the decode sites it unifies.

1. The VLM stats path runs after_prefill before the prefill graph is evaluated

Current behavior. CxxGenerator::generate_streaming_with_embeddings (src/lib/mlxcel-core/src/generate.rs:2304-2311) calls ffi::eval(&logits) before model.after_prefill(). The comment there gives the reason: after_prefill may replace weight arrays that the lazy prefill graph still references. generate_with_stats_and_embeddings, the path mlxcel generate --image ... --profile takes, calls model.after_prefill() at generate.rs:2498 with no eval before it. The prefill logits are first evaluated through the sampled token at :2508.

Impact today. The defect is latent. Since #887 removed Phi4MM's override, no family overrides after_prefill: the only implementations are the default no-op (generate.rs:643) and the delegates in src/loaded_model.rs:538 and src/vision/mod.rs:367. The first family that overrides the hook would get different output from the profile and non-profile image paths.

Fix. Move the shared tail of both embedding variants (the prefill result, ffi::eval, then after_prefill) into one private helper in generate.rs, and call it from both functions, so the order cannot drift again. Keep the existing comment on the helper.

Acceptance criteria.

  • generate_with_stats_and_embeddings evaluates the prefill logits before after_prefill, through the same helper generate_streaming_with_embeddings uses
  • A unit test in src/lib/mlxcel-core/src/generate.rs's test module uses a recording mock model that overrides after_prefill. It asserts that both embedding variants call it exactly once, after the prefill forward and before the first decode forward. A failing-first test of the eval itself is not possible while no family overrides the hook. The helper makes the order structural instead
  • On a real VLM checkpoint (for example gemma-3-4b-it-4bit with tests/fixtures/test_image.png), mlxcel generate --image ... --temp 0 gives the same text with and without --profile

2. Comments that describe the CLI/server split incorrectly

Each of these says the server runs a path it does not run. The server decodes through BatchScheduler, --no-batch included (src/server/model_worker.rs:1103-1108). Its SpeculativeDispatch::Classic arm declines to plain decode (src/server/batch/speculative_burst.rs:924, :2376) and never constructs SpeculativeGenerator.

  • src/lib/mlxcel-core/src/generate.rs:1913: "Used by: CxxGenerator::generate, ModelProvider (server streaming)". The server does not call generate_streaming.
  • src/lib/mlxcel-core/src/generate.rs:2415: "Used by: CLI --image path, server VLM requests". The server builds VLM embeddings with prepare_request_vlm_embeddings and decodes through the scheduler.
  • src/server/config.rs:941: "constructing per-sequence CxxGenerator instances". The scheduler applies kv_cache_mode to its pool (src/server/model_worker.rs:753).
  • src/server/config.rs:781, src/server/config.rs:1156, src/server/cli_input.rs:125 and src/server/startup.rs:1694-1698 say the classic SpeculativeGenerator path runs on the server when no drafter kind applies.
  • src/server/speculative_dispatch.rs:46-51 and :105-111 describe Classic as falling back to the classic SpeculativeGenerator dispatch, which contradicts the same file's line 45.
  • docs/CONTINUOUS_BATCHING.md:62: "--no-batch (legacy sequential worker, no scheduler)". The legacy worker is the scheduler at max_batch_size=1.

Acceptance criteria.

  • Each comment above describes the actual path: the scheduler for the server, plain decode for Classic, and CxxGenerator only for the CLI
  • rg -n 'SpeculativeGenerator' src/server matches only text that says the server does not use it
  • Comment-only changes: no behavior change, and the gate is green

Verification

make verify                                   # macOS gate
make verify-fmt verify-test-cuda              # Linux/CUDA gate
cargo test --profile test-fast --features cuda -p mlxcel-core generate
mlxcel generate -m <gemma-3-4b-it-4bit path> --image tests/fixtures/test_image.png -p "Describe the image." --temp 0 -n 64
mlxcel generate -m <gemma-3-4b-it-4bit path> --image tests/fixtures/test_image.png -p "Describe the image." --temp 0 -n 64 --profile

A pass means the gates are green and the two generate runs print identical text.

References verified against main at fc21784 (2026-10-07).

Activity

  1. added
    type:bugBug fixes, error corrections, or issue resolutions
    area:inferenceGeneration, sampling, decoding (incl. speculative, DRY)
    area:cliCommand-line interface / CLI flags
    and removed on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:cliCommand-line interface / CLI flagsarea:inferenceGeneration, sampling, decoding (incl. speculative, DRY)priority:lowLow prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions