Skip to content

fix(server): separate native completion and embedding routes from OpenAI routes #1441

Closed

Description

@inureyes

Parent

Problem

The native /completion response and streaming envelope expose only a subset of b10621 fields. More seriously, /completions is routed to an OpenAI-shaped handler in mlxcel while b10621 treats it as a native llama-server alias. /embeddings has the same native-versus-OpenAI collision.

Scope

  • Align native /completion request parsing, default resolution, cache controls, token-return options, stop metadata, timings, and streaming termination.
  • Route /completions to the native handler and retain OpenAI behavior exclusively under /v1/completions.
  • Route /embeddings according to the b10621 native schema and retain OpenAI behavior under /v1/embeddings.
  • Add compatible native fields including resolved generation settings, tokens/prompt when requested, cached/truncated state, stop type, stopping word, and progress reporting.
  • Align reverse-prompt stopping and special-token output controls, including their interaction with streaming and returned token IDs.
  • Match error status/envelopes and client-disconnect cancellation.
  • Publish a migration note because the current non-/v1 routes return different schemas.

Acceptance criteria

  • Route method/path tests prove native and OpenAI handlers are no longer conflated.
  • Non-streaming and streaming golden-schema tests cover normal length, EOS, stop-string, cancellation, cache hit, and truncation outcomes.
  • Reverse-prompt and special-token tests distinguish emitted text, stop metadata, raw token IDs, and detokenization behavior.
  • Unknown native request fields are rejected or reported consistently rather than silently ignored when they affect semantics.
  • /v1/completions and /v1/embeddings remain OpenAI compatible.
  • Existing non-/v1 behavior change is documented as breaking.

Validation

  • Compare the same prompt against b10621 and mlxcel using a real checkpoint, normalizing only backend-specific timing and model identifiers.

Activity

  1. added
    status:backlogIn the backlog, not yet ready
    type:bugBug fixes, error corrections, or issue resolutions
    impact:breakingBreaking change requiring migration
    area:architectureArchitecture and code structure changes
    area:inferenceGeneration, sampling, decoding (incl. speculative, DRY)
    on Aug 26, 2026
  2. added and removed
    status:backlogIn the backlog, not yet ready
    on Aug 27, 2026
  3. inureyes commented on Aug 27, 2026

    @inureyes
    MemberAuthor

    Partial implementation landed in #1467. This issue stays OPEN: closing it would strand manifest entries that are not honestly resolvable yet, and scripts/ci/check_llama_compat_manifest.py --check-issues-open fails the whole repository on a deferred entry whose issue is closed.

    Done in #1467: /completion and /completions both reach the native handler and answer b10621's full native object (index, content, tokens, id_slot, stop, model, tokens_predicted, tokens_evaluated, generation_settings, prompt, has_new_line, truncated, stop_type, stopping_word, tokens_cached, timings with cache_n leading); the streaming form ends with that same object and no [DONE] sentinel; /embedding is mounted for the first time and /embeddings answers upstream's bare array of {index, embedding} with the nested embedding; /v1/completions and /v1/embeddings are unchanged and stay OpenAI compatible; n_predict gains the max_tokens and max_completion_tokens aliases; and n_cmpl above 1, n_indent, t_max_predict_ms, return_progress, verbose and return_tokens are refused with a diagnostic naming the field instead of being silently ignored. The key sets, orders and the stop_type vocabulary were captured from the pinned b10621 binary serving real checkpoints.

    Remaining, in the order a follow-up should take them:

    1. String stop sequences are not enforced at all on the MLX serving path, which is filed as fix(server): honor string stop sequences on the MLX serving path #1466. They reach ServerGenerateOptions::stop_sequences and are never read; only the xla-iree-gated worker consumes them. Measured on models/mlx/qwen3-0.6b: /completion with and without "stop":["5"] returns byte-identical text. Until that lands, stop_type here can never report word and stopping_word is always empty, which is what keeps POST /completion and POST /completions deferred.
    2. generation_settings reports the settings mlxcel resolved rather than upstream's full task_params block, and id_slot is always -1 because the continuous-batching scheduler exposes no stable per-request slot number. Both are recorded as divergences on the two route entries.
    3. field:response_fields, field:stream_options and field:stream_options.include_usage are untouched and still deferred.
    4. The golden-schema coverage for cancellation, cache-hit and truncation outcomes is not written yet, and the reverse-prompt / raw-token-id criterion depends on both fix(server): honor string stop sequences on the MLX serving path #1466 and a return_tokens implementation.
    5. The four GET /health / GET /v1/health / GET /models / GET /v1/models entries are filed under this issue only because routes.json's owner set in pin.json is [1441, 1442, 1452]; their divergence text names feat(server): implement b10621 router mode and resolve models-dir semantics #1438 and feat(server): align props, slots, metrics, health, and slot persistence #1440 as the real owners. Whether to add those issues to that owner set and repoint the entries is a call for the epic owner, so feat(server): separate the native completion and embedding routes #1467 changed neither.
  4. added and removed on Aug 27, 2026
  5. inureyes commented on Aug 27, 2026

    @inureyes
    MemberAuthor

    PR #1476 closes the implementable remainder and this issue deliberately stays OPEN, for the same reason it stayed open after #1467: two manifest entries filed under it are still deferred, and the validator requires a deferred entry's issue to be OPEN.

    Flipped to supported in routes.json, each verified against the pinned b10621 binary serving a real checkpoint: field:response_fields, field:stream_options, field:stream_options.include_usage, field:timings_per_token and field:n_predict. That covers acceptance criterion "unknown native request fields are rejected or reported consistently" for the four fields that were simply unimplemented, and the n_predict: 0 prompt-only case is now measured rather than assumed: the binary emits exactly one token for it despite its schema description saying none, and mlxcel emits the same one token on the same prompt.

    Still deferred under this issue: POST /completion and POST /completions, with three divergences. generation_settings is mlxcel's resolved set rather than upstream's full task_params block. tokens_cached counts the prompt tokens mlxcel's prefix cache supplied while b10621 reports the slot's cache occupancy after the request (tokens_evaluated + tokens_predicted - 1), which is newly recorded by that PR and was not on the list before. And the six fields mlxcel refuses rather than honors, each of which has its own not_applicable entry and does not itself block this issue.

    The id_slot divergence line moved: exposing a stable per-request slot number is #1440's scope, so the line now names #1440 as its owner. The entries themselves stay filed here, because their other divergences are this issue's.

    The remaining generation_settings and tokens_cached work is response-body fidelity with no dependency on the field work, so it probably wants its own issue rather than keeping this one open indefinitely. That call belongs to the epic owner and no follow-up has been filed.

  6. inureyes commented on Aug 28, 2026

    @inureyes
    MemberAuthor

    Closing against what this issue delivered.

    PR #1467 moved /completions and /embeddings to the native handlers, mounted /embedding for the first time, gave the native completion object b10621's full sixteen-key set, and ended streaming on that object with no [DONE] sentinel. PR #1476 then added response_fields, stream_options and include_usage, corrected the timings derivations against the pinned binary, made n_predict: -1 work where it had been rejected with HTTP 422, and verified the n_predict: 0 prompt-only case. Five field entries moved to supported.

    What this issue did not finish now belongs to #1477, and the eight manifest entries that still described it were repointed there in #1478, so no deferred entry references this issue and the compatibility gate stays green. The substantive remainder is the six native request fields that are refused rather than honored, two of which (return_tokens and return_progress) this issue's own scope named; #1477 owns them with per-field acceptance criteria. id_slot stays recorded but names #1440, because exposing a stable per-request slot number is that issue's scope. generation_settings shrinks on its own as #1436 and #1433 land their options, so #1477 re-measures it rather than chasing it now.

  7. self-assigned this
    on Aug 31, 2026
  8. added a commit that references this issue on Sep 6, 2026
    1218bc4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:architectureArchitecture and code structure changesarea:inferenceGeneration, sampling, decoding (incl. speculative, DRY)impact:breakingBreaking change requiring migrationpriority:highHigh 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