fix(frontend): backport SGLang chat logprobs to 1.4.0 - #12988
Conversation
github-actions
Bot
added
fix
frontend
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
| token_ids: list[int], | ||
| ) -> dict[str, Any] | None: | ||
| if len(log_probs) != len(token_ids): | ||
| return None |
There was a problem hiding this comment.
_build_openai_logprobs indexes top_logprobs[index] without validating that top_logprobs has the same length as token_ids, so a malformed or partially populated backend response can raise IndexError and abort the stream. Fix: validate top_logprobs length before indexing or treat mismatched top_logprobs as absent.
🤖 AI Fix
In components/src/dynamo/frontend/sglang_prepost.py, update SglangStreamingPostProcessor._build_openai_logprobs so it checks if top_logprobs is not None and len(top_logprobs) != len(token_ids): top_logprobs = None before the for index, ... loop.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| for index, (token_id, logprob) in enumerate(zip(token_ids, log_probs)): | ||
| context_token_ids = (self._logprob_context_ids + token_ids[:index])[-4:] | ||
| token = self._decode_logprob_token(token_id, None, context_token_ids) | ||
| candidates = top_logprobs[index] if top_logprobs else [] |
There was a problem hiding this comment.
🟡 Streaming reply can abort when the model returns fewer alternative-token lists than tokens
The per-token list of alternative candidates is looked up by position without checking that the list is long enough (top_logprobs[index] at components/src/dynamo/frontend/sglang_prepost.py:1074), so a short list ends the whole reply with an internal error instead of just omitting the extras.
Impact: A client asking for token probabilities can get its entire streamed answer replaced by an internal error.
Mechanism: unchecked positional indexing of top_logprobs vs. validated log_probs
_build_openai_logprobs validates only the chosen-token array (len(log_probs) != len(token_ids) at components/src/dynamo/frontend/sglang_prepost.py:1067) but then indexes top_logprobs positionally for every token. The backend builds top_logprobs by slicing meta_info["output_top_logprobs"] independently of output_token_logprobs (components/src/dynamo/common/backend/logprobs.py:319-343), so a shorter (but non-empty) output_top_logprobs slice yields fewer candidate rows than tokens. The EOS-trimming guard in process_output also only truncates top_logprobs when its length already equals the raw token count (components/src/dynamo/frontend/sglang_prepost.py:1233-1234), leaving short lists untouched. The resulting IndexError propagates out of post.process_output into _generate_and_stream's generic handler and is re-raised as Unknown, failing the request.
| candidates = top_logprobs[index] if top_logprobs else [] | |
| candidates = ( | |
| top_logprobs[index] | |
| if top_logprobs is not None and index < len(top_logprobs) | |
| else [] | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| log_probs = engine_response.get("log_probs") | ||
| top_logprobs = engine_response.get("top_logprobs") |
There was a problem hiding this comment.
🔍 Verify the SGLang backend actually returns logprobs for a plain logprobs: true request
This path only produces output when the engine chunks carry log_probs. The frontend maps a bare OpenAI logprobs: true (no top_logprobs) to output_options.logprobs = 1 (components/src/dynamo/frontend/sglang_processor.py:290-291), and the SGLang backend rejects logprobs >= 1 with a ValueError unless DYN_SGL_ALLOW_TOP_LOGPROBS=1 (components/src/dynamo/common/backend/logprobs.py:249-281). If that gate is still in place on the 1.4.0 release branch, the common logprobs: true request would error out rather than return the newly-built payload, so QA verification of the NVBug should explicitly cover that case (and not only logprobs: 0).
Was this helpful? React with 👍 or 👎 to provide feedback.
| @pytest.fixture(scope="module") | ||
| def byte_fallback_tokenizer(): | ||
| return get_tokenizer(BYTE_FALLBACK_MODEL) |
There was a problem hiding this comment.
🔍 New TinyLlama tokenizer fixture is not registered in the session predownload manifest
The module docstring/marker block notes that pytest.mark.model("Qwen/Qwen3-0.6B") exists specifically so the tokenizer stays fetchable after another test flips HF_HUB_OFFLINE (tests/conftest.py:527-560 builds the download list from those markers). The new byte_fallback_tokenizer fixture pulls TinyLlama/TinyLlama-1.1B-Chat-v1.0, which is not declared via a model marker, so test_byte_fallback_sequence_longer_than_six_tokens can fail to fetch in offline/parallel CI runs. Adding pytest.mark.model(BYTE_FALLBACK_MODEL) to pytestmark would keep the manifest complete.
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Overview
Backports the SGLang chat-completion logprobs fix from #12820 to
release/1.4.0so QA can re-verify NVBug 6556494.Details
log_probsandtop_logprobs, convert them to the OpenAI-compatible response contract, preserve alignment after EOS trimming, and retain probability data across buffered stream chunks.mainchanges are not required for this fix.Validation
-xcherry-pick provenance.Related Issues