fix(sample): widen the logprobs=-1 sentinel at admission (#231)

[localai-bot]

Fixes #231. Spec:
.agents/specs/logprobs-all-sentinel.md.
Row SAMPLE-LOGPROBS stays DONE — this removes a deviation, it does not move a
lifecycle.

Found while gating #223; opened
as its own issue rather than folded in silently.

The bug

logprobs=-1 — a legal, validated value meaning "every vocab entry" — crashed
the engine. Any request carrying it died with SIGSEGV inside
LogprobsProcessor::UpdateSampleLogprobs.

The premise I got wrong first, and the one that is right

The obvious reading blames the sampler: sampler.cpp:343-352 has a
num_logprobs == -1 arm returning a raw-vocab LogprobsTensors with empty
ids and ranks, and logprobs.cpp:51-73 indexes all three arrays behind a guard
(width <= 0) that this shape passes. My first write-up of #231 said upstream has
no such arm and that our port invented it.

That was wrong, and it is corrected on the
issue
rather than quietly, because the fix reads differently once the premise is right.
The pinned vLLM has exactly that arm (sampler.py:120-131). Our port is faithful.

What upstream also has, and we did not, is gpu_input_batch.py:434-440:

if sampling_params.logprobs is not None:
    self.num_logprobs[req_id] = (
        self.vocab_size if sampling_params.logprobs == -1 else sampling_params.logprobs
    )

max_num_logprobs is fed from that map, so upstream's -1 arm is unreachable
on the V1 path — defensive code for a value the input batch cannot produce. A user
request gets the ordinary gathered shape, and one shape reaches every consumer.

We preserved the sentinel instead (input_batch.cpp:292-297, propagated by
max_num_logprobs()), written down in input_batch.h as a deliberate recorded
deviation on the reasoning that our Sampler consumes -1 directly. It does. What
the note missed is that consuming it yields a different tensor shape, and that
upstream can only afford that arm because nothing reaches it. We took the branch
without the widening that makes it dead.

The fix

Mirror the widening. One line at admission, and max_num_logprobs() loses its
sentinel case to become the plain max upstream's max(...) already was — "all" is
simply the largest count and wins on its own.

Nothing downstream changes: GatherLogprobs at k == vocab produces the ordinary
[n, vocab+1] shape, and AppendLogprobsForNextPosition already derives k from
the row width.

Rejected: teaching UpdateSampleLogprobs the second shape. It fixes the crash
and keeps the deviation, so our engine would carry two logprob shapes where
upstream carries one and every future consumer would need to know that. Widening
deletes the class of bug instead of the instance.

Kept deliberately: the sampler's -1 arm, because upstream keeps it and a
hand-built SamplingMetadata can still reach it. Its comment now records both
that the input batch cannot reach it and that its shape differs — the fact whose
absence caused this.

A replaced assertion, stated plainly

C7 wiring: -1 logprobs sentinel dominates max_num_logprobs asserted
max_num_logprobs == -1 — precisely the behaviour that crashes. It is replaced,
not relaxed, and the reason sits in the test beside it. Flagging it explicitly
because "a red gate turned green by changing an assertion" is exactly the shape a
reviewer should be suspicious of; the engine-level RED below is what shows the old
assertion was encoding the defect.

Evidence

RED — new cases against the unmodified engine (git stash push -- src include):

TEST CASE:  llm_engine: logprobs=-1 returns a full-vocab logprobs dict
FATAL ERROR: test case CRASHED: SIGSEGV - Segmentation violation signal

TEST CASE:  C7 wiring: a lone -1 logprobs request still carries vocab_size
ERROR: CHECK( batch.num_logprobs.at("only") == batch.vocab_size ) is NOT correct!
  values: CHECK( -1 == 1024 )

GREEN — focused:

test_llm_engine   13/13 test cases, 228 assertions
test_input_batch  26/26 test cases, 190 assertions

Full gate — clean CPU Release build, zero warnings under -Werror:

100% tests passed, 0 tests failed out of 360
Total Test time (real) = 729.31 sec

No flake, no serial re-run needed.

Scope note

The roadmap issue table is deliberately untouched. PR
#235 already registers #231 there,
and duplicating the row would guarantee a keyed-record conflict between two open
PRs of mine. Saying so here rather than leaving it as a silent omission.

Independent of #235 otherwise: different branch, based on current main
(58f43f66), no stacking. That row's -1 path was already correct — the runner
widens to vocab_size for prompt logprobs the way this change now does for
sampled ones.

🤖 Generated with Claude Code

[localai-bot]

Closing as OBSOLETE — superseded, not abandoned.

The content of this PR is already on main: another session landed it as fd9af7d9 via the branch row/SAMPLE-LOGPROBS-ALL-SENTINEL, merged as 723d96a8, while this PR stayed open. So the logprobs=-1 fix is shipped; only the PR is stale.

Verified before closing: main carries the widening, and this branch's head d75de383 adds nothing main does not already have.

Two follow-ups exist because of what the review of this PR found:

• #259 — the independent review returned FAIL on the RECORDS, not the code. My write-up attributed the SIGSEGV to UpdateSampleLogprobs, and the reviewer proved by instrumentation that that function is never entered: the fault is a null-pointer copy in LogprobsTensors::slice_request. That false attribution shipped to main with the fix, so fix(record): the logprobs=-1 SIGSEGV was in slice_request, not the processor (#231) #259 corrects it in the spec, the engine matrix, the test comments and docs/USAGE.md — which also carried a false claim that the HTTP logprobs field is range-limited. fix(record): the logprobs=-1 SIGSEGV was in slice_request, not the processor (#231) #259 additionally adds the chat top_logprobs=-1 test for a path this fix unblocked but left untested.
• #249 — GatherLogprobs never bounds k <= vocab and nothing clamps it from HTTP, so {"logprobs": 999999} crashes the server. Pre-existing and unrelated; filed separately rather than folded in.

Reason recorded per AGENTS.md: verified PRs merge in-session, obsolete ones close with the reason.