Skip to content

feat(server): OpenAI protocol coverage -- sampling extras, n, echo/suffix, usage details, tokenize/detokenize/metrics - #393

Closed
gdevenyi wants to merge 3 commits into
FlashML-org:mainfrom
gdevenyi:feat/openai-protocol-coverage
Closed

gdevenyi wants to merge 3 commits into
FlashML-org:mainfrom
gdevenyi:feat/openai-protocol-coverage

Conversation

@gdevenyi

@gdevenyi gdevenyi commented Sep 5, 2026

Copy link
Copy Markdown

Summary

Closes the most common gaps between FreeToken's OpenAI-compatible surface and vLLM / SGLang / llama.cpp's server. docs/openai_api.md has the four-way table of endpoints, request parameters and response fields (honoured / accepted / 400), and lists what is deliberately not implemented.

Sampling extras (/v1/chat/completions and /v1/completions): min_p, presence_penalty, frequency_penalty (over generated tokens), repetition_penalty (prompt + generated, HF semantics), logit_bias (token id → bias, clamped to [-100, 100]), min_tokens (no EOS / stop token before N generated tokens), stop_token_ids, include_stop_str_in_output, skip_special_tokens (per request; default off here because the reasoning and tool parsers read the tags). Out-of-range values answer 400 with the field named.

Implemented as logits processors (engine/sample.py): Sampler.prepare builds a per-batch LogitsPlan on the host from the requests' SamplingParams and token histories, only for the rows that asked; apply_logits_processors applies it to a float32 copy of the logits before the sampling kernel (repetition → presence → frequency → logit_bias → min_tokens mask → min_p, vLLM's order). A batch without them takes exactly the old path, and sampling already runs outside the CUDA graph, so no capture changes. Cost measured on an Ada card with a 3k-token history: 0.97 ms per step for a batch that uses them (0.32 ms on the device), 0.08 ms on the plain path.

n (1..16) on both routes: one generation per choice submitted together (the prefix cache serves the shared prompt), results gathered; streams interleaved into one SSE response with the choice index, one usage chunk (prompt counted once, completions summed) and one [DONE]; a client disconnect aborts every uid of the fan-out.

Completions: echo (the prompt leads the text, or the first chunk), suffix as a fill-in-the-middle prompt on models whose vocabulary has the FIM tokens (Qwen family; 400 otherwise).

Chat: continue_final_message (the final assistant message is continued, no generation prompt), request_id echoed as the response id, seed / user accepted, system_fingerprint: null in responses and chunks.

Usage: completion_tokens_details.reasoning_tokens — the detokenizer counts the tokens up to and including the reasoning end tag and reports it on the finished reply.

Routes: POST /tokenize and /detokenize (also under /v1/; the vLLM / SGLang shape, messages render through the chat template so count equals a generation's prompt_tokens), GET /metrics (Prometheus text of /v1/stats), GET /version.

Not in this PR, on purpose: constrained decoding (response_format json / json_schema, grammars: needs a grammar engine on the sampling path), prompt logprobs and echo + logprobs (prefill logits), a per-request seed (one batched sampling kernel), embeddings / rerank / score / audio.

Test plan

  • tests/engine/test_logits_processors.py (pure torch on the CPU: each processor, the plan builder, the greedy path), tests/tokenizer/test_detokenize_extras.py (stop-string keep, per-request skip_special_tokens, reasoning token count), tests/server/test_openai_extras.py (n fan-out non-stream and stream, echo, suffix with and without FIM tokens, prompt-major choice order, the extras reaching SamplingParams, 400s, continue_final_message, request_id, usage details, tokenize / detokenize, metrics exposition)
  • tests/server, tests/tokenizer, tests/scheduler, tests/engine: 790 passed on the CPU
  • GPU micro-check of the processors and the sampler with the real vocabulary (248,320) on RTX 6000 Ada
  • Served Qwen3.8-Flash-Next (RadixArk NVFP4, offload backend, TP=1) from this branch and ran a live probe of every field and route: 24/24 (penalties change a repetitive output, logit_bias steers the answer, min_tokens forces 2 → 35 tokens, stop_token_ids on . stops at 18 tokens, include_stop_str_in_output keeps the stop word, min_p / seed / user accepted, n=2 non-stream and stream, echo, suffix as FIM (return a + b), continue_final_message, request_id as the response id, reasoning_tokens 39 of 44 completion tokens with thinking on, tokenize / detokenize round trip and a rendered-messages count equal to the generation's prompt_tokens, /metrics, /version, five 400s), 8-question quality probe unchanged (6/8), decode unchanged (61.7 tok/s single-stream / 166 at 8 concurrent, the same as main on that card)

🤖 Generated with Claude Code

https://claude.ai/code/session_0173pf9k9fSVtwbm3f898HDt

gdevenyi and others added 3 commits September 5, 2026 08:15
…ffix, usage details, tokenize/detokenize/metrics

Brings the OpenAI-compatible surface closer to vLLM / SGLang / llama.cpp
(docs/openai_api.md has the four-way table):

- sampling: min_p, presence_penalty, frequency_penalty (generated tokens),
  repetition_penalty (prompt + generated, HF semantics), logit_bias (token id ->
  bias, clamped to [-100, 100]), min_tokens (no EOS / stop token before N
  generated tokens), stop_token_ids, include_stop_str_in_output,
  skip_special_tokens (per request; default off, the parsers read the tags).
  Implemented as logits processors in engine/sample.py: a per-batch LogitsPlan
  built on the host from the requests' SamplingParams and token histories, applied
  to a float32 copy of the logits before the sampling kernel, only for the rows
  that asked. Nothing changes for a batch without them; sampling stays outside the
  CUDA graph, so no capture is affected.
- n (1..16) on chat and completions: one generation per choice submitted
  together, results gathered; streams interleaved into one SSE response with the
  choice index, one usage chunk (prompt counted once) and one [DONE]; a
  disconnect aborts every uid of the fan-out.
- completions: echo (prompt leads the text / the stream), suffix as a
  fill-in-the-middle prompt on models with the FIM tokens (Qwen family).
- chat: continue_final_message (no generation prompt; the final assistant
  message is continued), request_id echoed as the response id, seed / user
  accepted, system_fingerprint: null.
- usage.completion_tokens_details.reasoning_tokens: the detokenizer counts the
  tokens up to and including the reasoning end tag on the finished reply.
- routes: POST /tokenize and /detokenize (also under /v1, the vLLM / SGLang
  shape; messages render through the chat template), GET /metrics (Prometheus
  text of /v1/stats), GET /version.

Tests: tests/engine/test_logits_processors.py (pure torch on the CPU),
tests/tokenizer/test_detokenize_extras.py, tests/server/test_openai_extras.py.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0173pf9k9fSVtwbm3f898HDt
…he processor tests

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0173pf9k9fSVtwbm3f898HDt
…keys across the worker boundary

Found by serving the branch: the tokenizer worker died decoding a SamplingParams with a
dict[int, float] (ValueError: int is not allowed for map key when strict_map_key=True).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0173pf9k9fSVtwbm3f898HDt
@gdevenyi

Copy link
Copy Markdown
Author

Re-tested against current main (fb7f732, i.e. after the #418 / #427 / #426 quantization refactor) on 2 x RTX 6000 Ada, TP=2 box.

Method. This PR's head merged onto main, then the full pytest tests suite. The run is CUDA-hidden (CUDA_VISIBLE_DEVICES="") on purpose: this box is serving a model on both GPUs, and with them visible 65-95 GPU tests fail on main itself with AcceleratorError: out of memory, with the count swinging ~10 between identical runs. Hiding CUDA makes the result deterministic, so a failure-set difference against main means something. Baseline: main = 1205 passed, 350 skipped, 0 failed.

Result: 1235 passed, 350 skipped, no new failures.

The +30 over main are this PR's own tests, and they ran (not skipped).

🤖 Generated with Claude Code

https://claude.ai/code/session_0173pf9k9fSVtwbm3f898HDt

@gdevenyi

gdevenyi commented Sep 14, 2026

Copy link
Copy Markdown
Author

Closing this. It was written with heavy AI assistance, and the maintainers have indicated that do not want such contributions.

The description and the diff stay here for anyone who wants to pick the idea up.

@gdevenyi gdevenyi closed this Sep 14, 2026
gdevenyi added a commit to gdevenyi/FreeToken that referenced this pull request Sep 15, 2026
…verage

# Conflicts:
#	python/freetoken/tokenizer/tokenize.py
gdevenyi added a commit to gdevenyi/FreeToken that referenced this pull request Sep 15, 2026
MT-z added a commit to MT-z/FreeToken that referenced this pull request Sep 16, 2026
gdevenyi's feat/openai-protocol-coverage @ 1ca5b2e, on origin/main af71ba4.
Four files conflicted, eight hunks:

- engine/sample.py (4 hunks): FlashML-org#393's LogitsPlan supersedes this branch's presence
  penalty -- same application point (raw logits, before the temperature divide),
  wider coverage (presence + frequency + repetition + logit_bias + min_tokens).
  The only commits this branch had in that file were 1dcde81 and 156fd6e, both
  the presence work, so the file is taken from FlashML-org#393 whole.
- server/generation.py, server/openai_api.py (3 hunks): the same supersession at
  the call sites; FlashML-org#393's parameter list contains presence_penalty. Only those
  hunks are taken from FlashML-org#393 -- --template-kwarg, the vision work and the
  advertised-context fix in these files auto-merged and are untouched.
- scheduler/scheduler.py (1 hunk): both sides add an independent block just
  before prefill_manager.add_one_req(msg) -- this branch runs the vision tower
  for an online image request, FlashML-org#393 sets up min_tokens. Both are kept verbatim,
  image encode first because it can reject the request and return.

tests/engine/test_presence_penalty.py still pins the removed implementation and
is handled in a follow-up commit.

Assisted-by: Claude Opus 5
MT-z added a commit to MT-z/FreeToken that referenced this pull request Sep 16, 2026
…s penalty rewrite

The merge left two presence_penalty keywords in resolve_sampling -- a syntax error, and
the fix is not "drop one". FlashML-org#393 types the parameter `float = 0.0`, but this branch's
pick() reads a None sentinel:

    def pick(value, key, framework):
        return value if value is not None else model_sampling.get(key, framework)

so an unspecified request would arrive as 0.0, read as "the client asked for zero", and
never consult `--sampling-override presence_penalty=...`. A model card's anti-repetition
half (Ornith-1.5: temperature 1.0 + presence_penalty 1.5) would stop being served, with
no error anywhere. The parameter keeps `float | None = None`, the value is resolved once
before the range check, and the resolved value is what both the check and SamplingParams
see. This branch's own tests for that path were already there and pass unchanged.

tests/engine/test_presence_penalty.py: the nine tests that poked at args.penalties /
args.seen described an implementation FlashML-org#393 replaced, and its LogitsPlan is pinned by
tests/engine/test_logits_processors.py. They are dropped. Kept: the four protocol tests
above, plus the greedy one ported to the new plan builder -- temperature=0 takes the
argmax shortcut, and FlashML-org#393 does not assert that the penalty still changes that output.
The stub carries max_device_len/output_len because the plan builder recovers the prompt
length from those, not from prompt_len.

Full suite, one process: 13 failed, 1796 passed, 57 skipped. try/all at 3480fe0 fails the
same 13 with 1774 passed, so FlashML-org#393 adds 22 passing tests and breaks none.

Assisted-by: Claude Opus 5
MT-z added a commit to MT-z/FreeToken that referenced this pull request Sep 16, 2026
…with FlashML-org#393)

Artemowka22's FlashML-org#224, open since 2026-08-26. FlashML-org#393 declines logprobs on both routes and its
own compatibility table points here ("400 (see FlashML-org#224 for the sampled-token logprobs)"), so
the two are complementary by design -- and both rewrite the same sampling and message
plumbing, which is where the work was.

Before taking it, measured what this branch actually did with a logprobs request:

    /v1/completions   logprobs=5   400 "logprobs is not supported"        declared
    /v1/chat/...      logprobs=true, top_logprobs=5   200, no logprobs    silent

`ChatCompletionRequest` carries `extra="allow"` and declares neither field, so they were
swallowed before any validator saw them. FlashML-org#393's table says this route returns 400; it
returned 200 and dropped them. Same on pr393 alone, so it is upstream, not this stack.

Merge: 9 files, 20 conflict blocks, all from FlashML-org#393's `n` sampling (many uids per request)
meeting FlashML-org#224's single-uid shape. Resolved by keeping FlashML-org#393's structure and hanging the
logprobs off it -- the multi-uid `choices` loop, `_completion_chunk`, `_resolve_sampling`
(which also fills min_p, penalties, logit_bias, stop_token_ids) all stay. FlashML-org#224's rejections
of `echo`, `suffix` and `logit_bias` were dropped: FlashML-org#393 implements all three, and those
lines are older than it. Its `echo` + `logprobs` rejection is kept -- that one is real, the
prompt logits are not there.

Two failures had to be split apart before either could be fixed:

  - `tests/scheduler/test_abort_inflight_prefill.py`, 4 tests: **pr224 fails these on its
    own base too**, so it is the PR's regression, not the merge. `ForwardOutput` grows from
    3 fields to 6, `_process_last_data` switches to reading it by name, and the upstream
    test (there since 3af9d90) hands it a bare 3-tuple. Fixed by having the test build a
    real `ForwardOutput`.
  - `tests/engine/test_sample_logprobs.py`, 1 test: this one is the merge. FlashML-org#393 adds the
    `needs_logits_processing` property to SamplingParams and FlashML-org#224's new stub is a
    SimpleNamespace without it. Fixed in the stub, not by loosening `_plan` -- a getattr
    default there would hide a real type mismatch. Third instance of this pattern today
    (FlashML-org#354's `k_scale`, FlashML-org#198's direct `num_page_override`).

Verified on the wire, Ornith-1.5-35B-A3B-NVFP4:

    /v1/completions logprobs=3   ' Paris' -0.7205, top3 [' Paris' -0.72, ' a' -2.16,
                                 '\n' -2.60], text_offset [0,6,7,8]; streaming carries
                                 per-chunk logprobs
    /v1/chat/...    with the default qwen3 reasoning parser: 400 naming the reason
                    ("reasoning tokens are hidden from message content ... use
                    /v1/completions"); with --reasoning-parser off: full entries with
                    token/logprob/bytes/top_logprobs, streaming too

FlashML-org#393's side survives: `n=2` returns 2 choices with correct usage, `echo` echoes, `suffix`
and `logit_bias` are accepted. FlashML-org#224's own guards fire: `echo`+`logprobs` and `logprobs=9`
both 400 with their reasons.

Default path, no logprobs requested, 3 single-stream decodes of 399 tokens:

    try/all   91.7 / 95.2 / 94.1 tok/s   median 94.1
    try/224   92.5 / 95.3 / 94.4 tok/s   median 94.4

Full suite: 11 failed, 1849 passed, 60 skipped -- the same 11 as try/all, plus the 13
tests the PR adds.

Assisted-by: Claude Opus 5
MT-z added a commit to MT-z/FreeToken that referenced this pull request Sep 16, 2026
…he stubs rather than the code

Two stub-vs-contract mismatches surfaced by the FlashML-org#224 merge, both fixed on the test side so
the production code keeps saying what it means:

`ForwardOutput` grew from 3 fields to 6 and `_process_last_data` now reads it by name, but
`tests/scheduler/test_abort_inflight_prefill.py` (upstream since 3af9d90) handed it a bare
3-tuple. It builds a real `ForwardOutput` now. pr224 fails these four on its own base too,
so this is the PR's regression rather than the merge's.

`tests/engine/test_sample_logprobs.py` builds SamplingParams as a SimpleNamespace without
`needs_logits_processing`, the property FlashML-org#393 added and `_plan` reads. Added to the stub. A
getattr default in `_plan` would have hidden a real type mismatch instead.

Assisted-by: Claude Opus 5
MT-z added a commit to MT-z/FreeToken that referenced this pull request Sep 16, 2026
's temperature/top_p/top_k

Assisted-by: Claude Opus 5
MT-z added a commit to MT-z/FreeToken that referenced this pull request Sep 16, 2026
…oudly, measured on this box)

Artemowka22's FlashML-org#223, open since 2026-08-26. `resolve_sampling` forwarded arbitrary client
floats; the PR validates `temperature`, `top_p` and `top_k` after the checkpoint defaults
are filled, and bounds `CacheRebuildRequest.timeout` to (0, 3600].

gdevenyi probed the live server on his deploy branch and reported the 400s. What this box
adds is the before: the PR says invalid values "die (or misbehave) deep in the sampler",
and neither is what happens here.

    try/all b3e2b21, every case answered 200 and the server stayed up:
      temperature=-3, top_p=0, top_p=7, top_k=0, top_k=-5   200, ordinary-looking text
      temperature=NaN, temperature=inf                      200, output was '!!!'

Nothing dies. The two failure shapes are a client getting a normal-looking answer that was
not sampled the way it asked, and a client getting '!!!' -- both with no way to tell. That
is worse than the missing 400, and it is the same family as the rest of FINDINGS.

With the merge, all seven answer 400 naming the value (`temperature must be a finite
number >= 0, got nan`, `top_p must be in (0, 1], got 7.0`, `top_k must be -1 (disabled) or
>= 1, got 0`), and the four boundaries -- temperature=0, top_p=1, top_k=-1, top_k=1 --
still answer 200.

Conflicted with FlashML-org#393 in `generation.py`, and the resolution is the point: FlashML-org#393 already
validates min_p, presence/frequency_penalty, repetition_penalty, min_tokens, logit_bias
and stop_token_ids -- the parameters it added -- and FlashML-org#223 covers the three core knobs it
did not touch. Complementary, not competing, so both sets are kept and the constructor
takes FlashML-org#223's resolved values. Verified after the merge that FlashML-org#393's side still fires: all
seven of its checks answer 400 with their own messages.

Also committed conflict markers once by running `git add -A` on a tree where `tail -2`
had hidden the second CONFLICT line; the server then failed to start with a SyntaxError
and the whole "after" probe was void. Re-measured.

Full suite: 11 failed, 1858 passed, 60 skipped -- the same 11 (5 upstream's own, 6 from
FlashML-org#354), plus the 6 tests the PR adds.

Assisted-by: Claude Opus 5
gdevenyi added a commit to gdevenyi/FreeToken that referenced this pull request Sep 18, 2026
…verage

# Conflicts:
#	python/freetoken/tokenizer/tokenize.py
gdevenyi added a commit to gdevenyi/FreeToken that referenced this pull request Sep 18, 2026
…isconnect

Guards the FlashML-org#222 resolution against FlashML-org#393's fan-out: the disconnect watcher takes the
whole uid list, not the first sample's uid.

Assisted-by: Claude Fable 5.1
(cherry picked from commit 93ff96349235ea3ca1504733a2abbd6e6bcccf94)
gdevenyi added a commit to gdevenyi/FreeToken that referenced this pull request Sep 19, 2026
…verage

# Conflicts:
#	python/freetoken/tokenizer/tokenize.py

# Conflicts:
#	python/freetoken/engine/sample.py
#	python/freetoken/tokenizer/tokenize.py
gdevenyi added a commit to gdevenyi/FreeToken that referenced this pull request Sep 19, 2026
…isconnect

Guards the FlashML-org#222 resolution against FlashML-org#393's fan-out: the disconnect watcher takes the
whole uid list, not the first sample's uid.

Assisted-by: Claude Fable 5.1
(cherry picked from commit 93ff96349235ea3ca1504733a2abbd6e6bcccf94)
(cherry picked from commit 69a5efa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant