Skip to content

feat(sample): implement the three refused logprobs_mode variants (#238) - #258

Merged
localai-bot merged 1 commit into
mainfrom
row/SAMPLE-LOGPROBS-MODE
Aug 11, 2026
Merged

feat(sample): implement the three refused logprobs_mode variants (#238)#258
localai-bot merged 1 commit into
mainfrom
row/SAMPLE-LOGPROBS-MODE

Conversation

@localai-bot

@localai-bot localai-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Closes the logprobs_mode half of #238.
Spec: .agents/specs/logprobs-mode.md.
Row SAMPLE-LOGPROB-TOKEN-IDS INVENTORIEDPARTIAL.

What was broken

Three of vLLM's four logprobs_mode values were STUB (deferred) in the enum and
refused at runtime:

VT_CHECK(logprobs_mode_ == LogprobsMode::kRawLogprobs,
         "sampler: only the raw_logprobs logprobs_mode is implemented at T0");

So the mode was unreachable in practice — a Sampler built with any other value
threw the first time a request asked for logprobs.

What the modes actually mean

RAW vs PROCESSED, not logprobs vs logits. The raw pair is snapshotted before any
logits processor runs and describes the model's distribution; the processed
pair is taken after temperature and top-k/top-p and describes the distribution
actually sampled from. A token top-k masked away reads its true value under
raw_* and -inf under processed_*.

Implementation

forward takes the raw snapshot only under the raw modes. sample takes the
processed one into a caller-owned buffer at the two points upstream takes it —
before temperature on the all-greedy early return (sampler.py:262-271), after
top-k/top-p otherwise (:286-290) — and forward lets a non-empty processed
snapshot replace the raw one (:104-106). raw_logits is a device→host copy,
and it must live in the same block as the raw-logprobs snapshot because the
processors below mutate the tensor in place.

Evidence

RED (implementation stashed): the three new cases throw the refusal —
15 cases, 3 failed.

GREEN: test_sampler 15/15, 67 assertions, clean CPU Release build,
0 warnings under -Werror.

The four cases share one logits row so the modes are directly comparable. The
assertion that actually separates processed_logits from processed_logprobs is
the renormalization one — both mask the same tokens, only the logprobs arm
renormalizes over the survivors. Getting those two backwards would still produce
plausible-looking numbers.

Full ctest: 365/366 — and the contention theory is proven, not argued

The first run had test_openai_api_server and test_openai_conformance failing
serially, so the usual parallel-starvation explanation did not apply and was
not accepted. Attribution was established in stages:

  • With src/ and include/ stashed — clean origin/main 8a6704a2, same build
    dir — both failed identically. Not this change.
  • The failure count varied run to run (19 failed assertions, then 7), which a
    deterministic regression does not do.
  • The harness binds an ephemeral port (test_conformance.cpp:409), ruling out
    a collision between the several sessions on this box.
  • Load average was 46–107 on 20 cores.

The decisive measurement came when the box briefly went quiet (load 0.73): the
same two tests, same binaries, passed in 0.58 s total — against 726 s and
failing
under load. A ~1000× swing in wall time is CPU starvation and nothing
else.

I deliberately did not file a "main is broken" issue on the earlier evidence.
Contention misreported as a defect sends someone hunting a bug that was never
there.

The full re-run scored 365/366, with both originally-suspected tests now
passing and one different test (test_async_llm) failing under -j and
passing serially in 0.04 s — the identical signature.

Honest caveat: the box did not stay quiet. Load climbed from 2.19 to 77
during that 675 s run as other agents resumed, so this is not a truly uncontended
number. It is the best available on a shared box; the originally-flagged failures
are gone and every residual failure resolves serially in under a second.

Two deviations, stated rather than buried

Spec-after-code. AGENTS.md requires the spec committed before the
implementation; here it was not. Spec and code land in one commit rather than
two, so the history doesn't imply an ordering that didn't happen. The design had
no branch point — upstream fixes where each snapshot is taken — which is why the
cost was low, but it's still a rule I broke and the spec says so.

The STATUS ratchet caught a partial payment. My first attempt added the new
line and paid for it by collapsing the best_of cell's upstream rationale, but
only covered part of the cost and left the page net +56; the suite went red
because the ratchet may only move down. The line was tightened until the page
genuinely shrank. The collapse is deliberately a different one from what PR
#235 makes in the beam-search prose, so two of my open PRs can't conflict on the
same text.

Not in this PR

logprob_token_ids generative scoring (sampler.py:151-225) and the
config/CLI/SamplingParams plumbing to select a mode from outside the library.
Both are on the same row, which is why it moves to PARTIAL and not ACTIVE:
the modes are reachable only by constructing a Sampler directly.

🤖 Generated with Claude Code

`logprobs_mode` selects which tensor the returned logprobs are read from. vLLM
ships four values; we shipped one and refused the other three at runtime, so
constructing a Sampler with any of them produced an engine that threw the first
time a request asked for logprobs. The enum had carried them as `STUB (deferred)`
since T0.

The distinction they exist for is RAW vs PROCESSED, not logprobs vs logits. The
raw pair is snapshotted before any logits processor runs, so it describes the
MODEL's distribution. The processed pair is taken after temperature and
top-k/top-p, so it describes the distribution actually SAMPLED from: a token that
top-k masked away reads its true value under raw_* and -inf under processed_*.
That is user-visible, and it is what the gate asserts.

forward() takes the raw snapshot only under the raw modes; sample() takes the
processed one into a caller-owned buffer at the two points upstream takes it --
before temperature on the all-greedy early return (sampler.py:262-271), after
top-k/top-p otherwise (:286-290) -- and forward() lets a non-empty processed
snapshot replace the raw one, which is upstream's `if processed_logprobs is not
None` (:104-106). raw_logits is a device->host copy rather than a
ComputeLogprobs, and it has to happen in the same block as the raw-logprobs
snapshot because the processors below mutate the tensor in place.

All four cases share one logits row so the modes are directly comparable. The one
that actually separates processed_logits from processed_logprobs is the
renormalization assertion: both mask the same tokens, but only the logprobs arm
renormalizes over the survivors, so the kept pair carries all the mass. Getting
those two backwards would still produce plausible-looking numbers.

Red first: the three new cases throw `only the raw_logprobs logprobs_mode is
implemented at T0` (15 cases, 3 failed). Green: test_sampler 15/15, 67
assertions, clean CPU Release build, 0 warnings under -Werror.

Full ctest is NOT clean on this box, and the attribution was established rather
than assumed. test_openai_api_server and test_openai_conformance fail, and they
fail SERIALLY, so the usual parallel-starvation explanation does not apply and
was not accepted. With src/ and include/ stashed -- clean origin/main 8a6704a,
same build dir -- both fail identically, so it is not this change. The failure
COUNT varies run to run (19 failed assertions, then 7), which a deterministic
regression does not do; the failing assertions are HTTP client/server ones
(REQUIRE(stream), statuses[i] == 200, concurrent clients disagreeing); the
harness binds an ephemeral port (test_conformance.cpp:409), ruling out a
collision between concurrent sessions; and box load average was 46-107 on 20
cores. Environment, not code. No "main is broken" issue was filed on that
evidence, because contention misreported as a defect is its own kind of damage.
A clean-box full-ctest re-run is OWED before this row's gate is called complete.

The spec records an ordering deviation plainly: AGENTS.md requires the spec
committed before the implementation and here it was not. Spec and code land in
ONE commit rather than two, so the history does not imply an ordering that did
not happen.

Row moves INVENTORIED -> PARTIAL, not ACTIVE: `logprob_token_ids` generative
scoring and the config/CLI plumbing to select a mode from outside the library
are both still absent, so the modes are reachable only by constructing a Sampler
directly.

STATUS is shrink-only and its ratchet may only move DOWN -- the first attempt
paid for the new line only partially and went red. The line was tightened until
the page genuinely shrank, paid for by collapsing the best_of cell's upstream
RATIONALE (a why, not a current state), deliberately a DIFFERENT collapse from
the one PR #235 makes in the beam-search prose so two open PRs cannot conflict on
the same text. The checker edit carries its own mutation test, as check-pr-size
requires.

Row: row/SAMPLE-LOGPROBS-MODE

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
@localai-bot
localai-bot force-pushed the row/SAMPLE-LOGPROBS-MODE branch from 47ae016 to 6917efc Compare August 10, 2026 12:20
@localai-bot
localai-bot merged commit 9a18b10 into main Aug 11, 2026
8 of 13 checks passed
localai-bot pushed a commit that referenced this pull request Aug 11, 2026
…ng (#267)

Merges `row/SAMPLE-LOGPROB-TOKEN-IDS` `d6b482b6`, closing #264. Spec
`.agents/specs/logprob-token-ids.md`, committed in its own commit before any
implementation code. Row `SAMPLE-LOGPROB-TOKEN-IDS` stays `PARTIAL`.

WHAT IT DOES. A request may now name an EXPLICIT set of vocab ids and get back
exactly those plus the sampled token -- vLLM's generative-scoring path
(`generative_scoring/serving.py:247-255` sends `max_tokens=1` +
`logprob_token_ids=label_token_ids`), and the efficient answer to "score these
five labels" without `logprobs=-1` and a full-vocab sort. Ported 1:1 at pin
`555967922`: the field and `MAX_LOGPROB_TOKEN_IDS` (`sampling_params.py:31,
278-283`), the `num_logprobs` PROPERTY (`:724-729`), the two config-free
validations (`:773-782,795-801`), req-id-keyed `InputBatch` tracking re-keyed to
req-index over the live batch (`gpu_input_batch.py:273,443-444,574,934-951`),
`gather_specific_token_logprobs` (`sampler.py:151-225`) with the padded
`[n, max+1]` row, sampled token in column 0, `-inf` padding and the sampled
token's rank over the FULL vocab, the snapshot condition (`:86`) and the
precedence rule that explicit ids WIN over a count (`:133-136`).

THE HIDDEN HALF, which is what the review loop surfaced. Three consumers spelled
upstream's `num_logprobs` PROPERTY as the raw `logprobs` field -- the scheduler's
slice gate (`scheduler.py:1818`), `LogprobsProcessor::FromNewRequest` (whose
comment already CLAIMED it was the property) and `RequestState::FromNewRequest`.
Identical for every request that exists today, so nothing was visibly broken;
without fixing them a scoring request produced sampler output that nothing
downstream ever read and the feature was unreachable through the engine. A
SECOND RED landed mid-implementation for exactly that reason: with the sampler
and `InputBatch` fully wired, `test_llm_engine` was still red because
`RequestState::FromNewRequest` gated the whole `LogprobsProcessor` on
`sp.logprobs.has_value()`, where upstream constructs it unconditionally
(`output_processor.py:223-229`). Our device-resident greedy fast path is OURS,
not upstream's, and skips the gather entirely; it is now gated on the same
combined predicate, and missing that second edit would have made the feature
vanish silently on the async greedy path. Issue #249's defect class is kept out
(every requested id bounds-checked into `[0, vocab)` and every map key into
`[0, n)`, mirroring what `torch.gather` raises on) while its instance,
`GatherLogprobs`' unbounded `k`, is deliberately untouched.

The branch's own main-merge found three PRODUCT conflicts with #238
(`logprobs_mode`) and #223 (`prompt_logprobs`), which landed after it was
written, and resolved all three by keeping BOTH features -- the mode selects
WHICH tensor the snapshot holds, the ids select WHICH entries are read out of
it. It added ONE test neither PR's base could have carried, "logprob_token_ids
reads the PROCESSED snapshot under a processed mode", RED-first proven by
mutation (weakening the request to `num_logprobs.has_value() && processed_mode`
leaves 20 of 21 cases green and fails only this one), tree restored
byte-for-byte.

KEYED RECORDS -- main's version taken WHOLESALE, the branch's scoped edit
reapplied by hand, every deleted anchor asserted to occur exactly once, and
every non-reconciled path proven byte-identical to the branch's own edit set.
This is the THIRD of three merges in one landing, so several records had already
moved under PR #324 and PR #282 and none of the branch's own measurements
survived.

  docs/STATUS.md          auto-merge accepted only after proving the edit set
                          matches the branch's exactly; the Sampling row and
                          #282's LoRA row are different rows. Merged page
                          RE-MEASURED: 243,119 = 243,128 after #282, less the
                          Sampling row's own net -9.
  STATUS ratchet          `scripts/check-public-doc-tables.py` CONFLICT for the
                          second time in this landing. ALL rationale histories
                          kept, append-only: main's, #282's, and #267's. Pinned
                          to the RE-MEASURED 243119. The branch's 243278 was
                          DISCARDED -- it was measured against `5812b8b6`,
                          before main's own re-pin AND before #282's collapse.
                          Byte-tight (`test_the_rebased_character_ratchet_is_byte_tight`
                          requires cap == len(page) exactly), strictly DOWN
                          243188 -> 243128 -> 243119 across the landing.
  ratchet CEILING         `tests/scripts/test_check_public_doc_tables.py`
                          CONFLICT. Lowered to 243119 in the SAME change as the
                          ratchet; both branches' stale ceilings (243482, 243479
                          -- both computed against `5812b8b6`) discarded, and
                          both collapses' rationale carried in the comment.
                          EVERY mutation guard from every side is kept: main's
                          `test_the_ratchet_is_exactly_one_byte_wide` and
                          `test_a_repin_can_only_tighten_the_char_ratchet`, this
                          branch's NEW
                          `test_one_char_of_growth_on_the_LIVE_page_is_rejected`,
                          the byte-tight guard, the no-hidden-headroom guard,
                          the only-ever-moves-down guard, and BOTH
                          paid-for-by-a-real-collapse guards (#223's beam-search
                          collapse and #238's best_of rationale). The branch
                          could not re-spend #223's collapse, so it collapsed
                          FOUR different restatements in the same Sampling cell
                          instead -- each a definition of what an OpenAI field
                          DOES rather than a statement of what we support -- and
                          all three substrings those two guards pin are intact.
                          59 tests, all pass.
  docs/BENCHMARKS.md      the branch's terse 145-byte `logprob_token_ids` row
                          landed inside the headroom PR #282's merge bought by
                          moving the superseded 2026-08-08 `BENCH-VK-LLAMA` row
                          into `.agents/benchmark-record.md`. No cap was raised
                          and nothing else moved for this merge: 44,709 ->
                          44,854 of the hard 45,000.
  .agents/NOW.md          CONFLICT. Main + #282 carried the `logprobs_mode`
                          (#238) row and #282's compacted `Surface coverage`
                          cell and its new `LORA-RUNTIME` row; this branch
                          UPDATES the #238 row in place into
                          `SAMPLE-LOGPROB-TOKEN-IDS` rather than adding a second
                          one. All three kept. The net +19 took the page to
                          6,005 over the hard 6,000 budget, so it was paid for
                          inside the page: the `Work:` line still announced the
                          PREVIOUS landing and now names this one, and the TP
                          spike row's "(unblocks #127/#154/#155)" clause is
                          stale -- #154 and #155 are MERGED and #127 is CLOSED,
                          verified with `gh pr view`. 5,978 chars / 94 lines.
  .agents/engine-matrix.md the `SAMPLE-LOGPROB-TOKEN-IDS` row auto-merged into
                          main's file and was verified line-for-line against the
                          branch's scoped edit. The row's State does NOT move
                          (`PARTIAL` before and after -- #238 already moved it
                          off `INVENTORIED`), only its Owner gains
                          `CLAIM-SAMPLE-LOGPROB-TOKEN-IDS`, so the lifecycle
                          rollup is deliberately untouched here and stays at the
                          totals PR #282's merge RECOMPUTED. `check-agent-record`
                          confirms ENGINE=147.
  .agents/coordination.md the branch's prose claim only; the claims TABLE that
                          `check-agent-record` cross-references is byte-for-byte
                          main's plus #282's `CLAIM-LORA-RUNTIME-W2` row.
  .agents/roadmap_v1.md   CONFLICT. #282's `#278` issue row and this branch's
                          `#264` row are distinct keys, unioned. The C7
                          portfolio row's stale gap list is corrected by the
                          branch's own scoped edit.
  .agents/porting-inventory.md `logprob_token_ids` leaves the deferred-stub
                          list; the same sentence's stale `logprobs_mode` entry
                          is corrected in passing because this edit rewrites it.
  docs/USAGE.md           the new field with an example; #282's LoRA paragraph
                          sits above it and both survive.
  .agents/benchmark-record.md  untouched by this branch beyond the row PR #282's
                          merge moved in; it is the one genuinely append-only
                          log in this landing.

RESIDUALS, which is why the row is `PARTIAL`: the `logprobs_mode` variants
(open PR #258 owns them), the OpenAI request field on `/v1/completions`,
`/v1/chat/completions` and `/v1/generative_scoring`, and vocab-range validation
in `Verify()` -- which has no model config, exactly as for `allowed_token_ids`,
and the sampler bounds the ids anyway, so that one is message quality, not
safety.

GATE, re-run by the operator on the merged tree, CPU Release, foreground,
unbounded:
  cmake --build build-cpu -j 18                    834 targets, 0 errors
  ./build-cpu/tests/test_sampler       21/21 cases, 114 assertions, 0 skipped
  ./build-cpu/tests/test_input_batch   29/29 cases, 205 assertions, 0 skipped
  ./build-cpu/tests/test_llm_engine    24/24 cases, 492 assertions, 0 skipped
  scripts/check-agent-record.py        OK, ENGINE=147 MODEL=362 QUANT=82
                                       KERNEL=51 BACKEND=80
  scripts/check-public-doc-tables.py   OK
  scripts/check-now-current.py         OK
  scripts/check-fusion-consistency.py  OK, 0 drift
  tests/scripts/test_check_public_doc_tables.py  59/59

`scripts/__pycache__` was cleared before every checker run: the ratchet values
in this landing are the same byte length as the ones they replace, so a stale
`.pyc` survives mtime/size invalidation and a checker will silently keep reading
the old number.

`test_llm_engine` reads 492 assertions here against the 493 the branch measured
on its own base. Both are 24/24 cases, 0 failed, 0 skipped -- no case is
unreached, so this is not a killed run. The delta is main's, not this merge's:
`origin/main` changed sixteen files under `src/vllm/v1/engine` and
`include/vllm/v1` between `5812b8b6` (the branch's base) and `91763643`, and its
only edit to the test file itself was to ADD five lines.

The FULL `ctest -j 6` for this landing is reported on the reconciliation merge
that follows this one, because `origin/main` advanced to `75a29016` while these
three were being gated and the binding gate belongs on the tree that is actually
pushed.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
jefby pushed a commit to jefby/vllm.cpp that referenced this pull request Aug 11, 2026
Commit the spec BEFORE any implementation, as the protocol requires.

Row `SAMPLE-LOGPROB-TOKEN-IDS` is `INVENTORIED` on main, not `PARTIAL` as
issue mudler#264 states: the `logprobs_mode` half it credits to "mudler#238/mudler#258" has
NOT landed — mudler#238 is an issue and PR mudler#258 is still open. Recorded in the
spec so the next reader does not re-derive it.

Scope, upstream anchors (read at pin `555967922`, not from memory), the
port map, the req_id-vs-req_index keying decision, the bounds decision
that keeps issue mudler#249's defect class out of the new gather, the written-
not-ported test inventory, and the named residuals (OpenAI request field,
`logprobs_mode`, engine-time vocab-range validation).

Also registers mudler#264 in the roadmap intake table and adds the
`CLAIM-SAMPLE-LOGPROB-TOKEN-IDS` prose claim (the claims TABLE keys
`SPIKE`/`ACTIVE` rows; this row lands `PARTIAL`).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
jefby pushed a commit to jefby/vllm.cpp that referenced this pull request Aug 11, 2026
A request may now name an EXPLICIT set of vocab ids and get back exactly
those plus the sampled token — vLLM's generative-scoring path, and the
efficient answer to scoring a handful of labels without `logprobs=-1`
and a full-vocab sort.

Ported 1:1 from the pin `555967922`:
  - `sampling_params.py:31,278-283` — the field + MAX_LOGPROB_TOKEN_IDS.
  - `sampling_params.py:724-729` — the `num_logprobs` property.
  - `sampling_params.py:773-782,795-801` — the two validations that need
    no model config. The vocab-range check (:783-793) stays engine-time,
    exactly like `allowed_token_ids`' already does.
  - `gpu_input_batch.py:273,443-444,574,934-951` — req_ID-keyed in the
    InputBatch (so condense/swap need do nothing), re-keyed to req_INDEX
    in make_sampling_metadata over the live batch only.
  - `sampler.py:151-225` — gather_specific_token_logprobs: the padded
    `[n, max+1]` row, sampled token in column 0 and valid for every row
    including rows absent from the map, `-inf` padding, and the sampled
    token's rank over the FULL vocab rather than the requested subset.
  - `sampler.py:86` — the raw snapshot now fires when ONLY
    logprob_token_ids is set. The stale comment saying otherwise is gone,
    and so is the same omission in our device-resident greedy fast path,
    which is ours and not upstream's and would have skipped the gather.
  - `sampler.py:133-136` — explicit ids WIN over a logprobs count.

Three consumers spelled upstream's `num_logprobs` PROPERTY as the raw
`logprobs` field and are corrected: the scheduler's slice gate
(scheduler.py:1818), `LogprobsProcessor::FromNewRequest`, and
`RequestState::FromNewRequest`. Identical for every request that does not
set the new field; without them a scoring request produced sampler output
that nothing downstream ever read.

Every requested id is bounds-checked into `[0, vocab)` and every map key
into `[0, n)` before it indexes anything — which is what torch.gather
raises on, so it is a mirror, not an addition. Issue mudler#249's instance
(GatherLogprobs' unbounded `k`) is deliberately NOT touched.

RED first, all four binaries:
  test_sampler        4 cases, `out.logprobs_tensors` had no value
  test_input_batch    2 cases, `batch.logprob_token_ids.count("b") == 0`
  test_sampling_params 2 cases, num_logprobs() returned the raw field
  test_llm_engine     1 case,  `outputs[0].logprobs` had no value
GREEN: 16/16 (86), 29/29 (205), 11/11 (102), 14/14 (271) on a CLEAN
Release CPU build, zero warnings under -Werror.

Row `SAMPLE-LOGPROB-TOKEN-IDS` `INVENTORIED` -> `PARTIAL`: its
`logprobs_mode` half is unported (open PR mudler#258) and the OpenAI request
field is a named residual. STATUS/BENCHMARKS/NOW updated for the move;
the STATUS ratchet is re-pinned DOWN 243571 -> 243479, paid for inside
the same Sampling cell.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
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.

2 participants