Skip to content

Fix disagg overlap scheduler grammar sync for spec_v2 + guided decoding - #25361

Closed
nvyutwu wants to merge 2 commits into
sgl-project:mainfrom
nvyutwu:fix/disagg-spec-v2-grammar-sync
Closed

nvyutwu wants to merge 2 commits into
sgl-project:mainfrom
nvyutwu:fix/disagg-spec-v2-grammar-sync

Conversation

@nvyutwu

@nvyutwu nvyutwu commented May 15, 2026

Copy link
Copy Markdown

Summary

Mirror the normal overlap scheduler's early-commit guard in event_loop_overlap_disagg_decode.

When is_disable_overlap_for_batch() returns True (spec_v2 + grammar + decode + pending result), commit the previous batch's result before launching the next batch so req.output_ids and req.grammar are up-to-date before the next MTP verify step prepares grammar masks.

Root Cause

Call trace — before the fix

# Entering iter N:
#   grammar state committed through B_{N-2}
#   result_queue = [(B_{N-1}.copy(), result_{N-1})]   ← B_{N-1} pending
#   is_disable_overlap_for_batch(B_N) = True           ← spec_v2 + grammar + result pending

# BUG: no early commit
run_batch(B_N)               # MTP verify built from B_{N-2} grammar state  ← STALE
result_queue.append(…)       # queue grows to 2 items

if last_batch:               # True (= B_{N-1})
    if not disable_overlap:  # False → SKIPPED
                             # process_batch_result never called while flag stays True
                             # result_queue grows unboundedly; grammar never advances
                             # → guided-decoding output corrupted

Call trace — after the fix

# Same state entering iter N

if disable_overlap and last_batch:
    process_batch_result(B_{N-1}.copy())  # grammar committed through B_{N-1} ✓

run_batch(B_N)               # MTP verify sees up-to-date grammar ✓
result_queue.append(…)       # back to 1 item

if last_batch:
    if not disable_overlap:  # False → SKIPPED (already committed above) ✓

Reproducer

Requires disagg setup (prefill + decode servers) + a model with EAGLE weights

Launch prefill and decode servers with --speculative-algorithm EAGLE --speculative-draft-model-path <eagle-weights> in disagg mode, then:

import json
import openai

client = openai.OpenAI(base_url="http://<decode-server>:30001/v1", api_key="x")
schema = {
    "type": "object",
    "properties": {"answer": {"type": "string"}},
    "required": ["answer"],
}

for i in range(20):
    resp = client.chat.completions.create(
        model="<model>",
        messages=[{"role": "user", "content": "Respond in JSON with key 'answer'"}],
        extra_body={"json_schema": schema},
        max_tokens=64,
    )
    text = resp.choices[0].message.content
    try:
        json.loads(text)
    except json.JSONDecodeError as e:
        print(f"iter {i}: CORRUPT OUTPUT — {e}\n{text}")
        break
else:
    print("All 20 passed")

Before fix: grammar-constrained output corrupted after a few requests (invalid JSON, schema violation).
After fix: all requests return valid JSON.

Tests

  • New unit test test/registered/unit/managers/test_overlap_disagg_decode_grammar_sync.py:
    • test_grammar_sync_commits_before_run_batch: asserts process_batch_result fires before run_batch when disable_overlap_for_batch=True — catches the exact regression
    • test_no_double_pop_when_disable_overlap_then_overlap: normal overlap path processes at bottom, no skipped pops
    • test_no_pop_on_first_iter_with_disable_overlap: no pop attempt when last_batch=None (empty queue guard)
  • python3 -m py_compile python/sglang/srt/disaggregation/decode.py — clean

CI States

Latest PR Test (Base): ❌ Run #26934738739
Latest PR Test (Extra): ❌ Run #26934738649

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@nvyutwu nvyutwu changed the title Fix disagg spec decoding grammar sync Fix disagg overlap scheduler grammar sync for spec_v2 + guided decoding May 15, 2026
@junliu-mde

junliu-mde commented May 20, 2026

Copy link
Copy Markdown

Related issue #25388
Although IDK whether this is the correct way out.

@nvyutwu
nvyutwu force-pushed the fix/disagg-spec-v2-grammar-sync branch from 25646e5 to e2ab513 Compare June 4, 2026 06:22
@DarkraiHL

Copy link
Copy Markdown
Contributor

We hit the same bug on a GLM-5 PD deployment (NEXTN spec + xgrammar, triggered by forced tool_choice): every such request aborted with Grammar accept_token failed ... Tokens not accepted.

We instrumented the grammar object (accept_token / rollback / fill_vocab_mask) and confirmed the root cause matches this PR exactly — event_loop_overlap_disagg_decode generates the vocab mask for the next verify step before the
previous step's tokens are committed into the grammar, so the mask is always one verify step stale. It stays harmless during the unconstrained reasoning phase, then breaks right after </think>: the stale mask permits grammar-invalid tokens,
which later fail accept_token at commit time → FINISH_ABORT. This also explains why the standard event_loop_overlap is unaffected — it already has the need_grammar_sync guard from #13425; this loop just never received it.

After applying the same guard as this PR, forced tool_choice works reliably and non-grammar requests are unaffected.

+1, hope this lands soon.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks @nvyutwu. Closing this because it is still a draft and has not been updated in 94 days.

Reopen it if the work is still relevant.

Some directories moved recently, so an older branch may need retargeting:
sgl-kernel/ -> python/sglang/kernels/aot/, python/sglang/jit_kernel/
-> python/sglang/kernels/jit/, docs/ -> docs/docs/ (.mdx),
bench_serving.py -> benchmark/serving.py, test/srt/ -> test/registered/.

@github-actions github-actions Bot closed this Sep 15, 2026
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.

3 participants