Skip to content

fix(spec decode): harden DSpark and DFlash edge paths - #105

Closed
voipmonitor wants to merge 11 commits into
dev/fathomless-firmamentfrom
codex/ff-dspark-core-canonical-20260717
Closed

fix(spec decode): harden DSpark and DFlash edge paths#105
voipmonitor wants to merge 11 commits into
dev/fathomless-firmamentfrom
codex/ff-dspark-core-canonical-20260717

Conversation

@voipmonitor

Copy link
Copy Markdown

Summary

Extract the remaining correctness fixes from the conflicting #88 onto current Fathomless Firmament, without its PCIe stack, deployment launcher, memory tuning, or capacity-controller experiment.

Fixes

  • Skip zero-width DSD CUDA-graph candidates instead of constructing an invalid query length.
  • Keep DSpark target metadata, request-slot recycling, and fallback sampling seeds deterministic across TP ranks.
  • Mark padded draft sampling rows inert and clear their query inputs.
  • Mask prefix-cache-restored tokens whose draft KV was never computed.
  • Make the overlapping in-place draft block-table shift race-free across Triton lanes.
  • Prefer FlashAttention where SM100f non-causal FlashInfer is unsupported and keep non-causal FlashInfer draft attention out of FULL graphs.
  • Preserve an explicit adaptive depth of zero instead of replacing it with the default K.
  • Retain DFlash backbone storage referenced by captured CUDA graphs.

Important review finding

The original #88 block-table shift had a real nondeterministic cross-lane read/write race. The long-row test reproduced corrupted block IDs. This version inserts a barrier between overlapping source loads and destination stores; 100 consecutive overlap iterations passed.

Validation

  • 30 passed across DSD graph dispatch, DSpark metadata, TP seed state, prefix-cache masking, adaptive-depth scheduling, and DFlash graph lifetime.
  • Additional 100-iteration SM120 overlap stress passed.
  • Ruff check and format pass on all changed Python files.
  • git diff --check passes.

Capacity-aware/load-aware DSpark verification is intentionally excluded and will be proposed separately.

EanWang211123 and others added 10 commits July 17, 2026 09:34
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
…context

DFlash/DSpark build the draft's context KV from target aux hidden states,
which only exist for tokens that flow through a target forward pass. Tokens
restored from the prefix cache (or a KV connector) at request (re)admission
never do, so their draft KV slots are never written — yet the draft attends
over the full sequence. With automatic prefix caching and a long shared
prefix, the draft reads thousands of uninitialized slots and acceptance
collapses to ~0.3% (position-0 only); the same workload with unique prompts
reaches ~20%. MTP is unaffected (no context KV), which hid the interaction.

Fix: track per request-slot how many tokens were restored at the last
(re)admission (RequestState.num_cached_tokens) and hide the restored whole
blocks from the draft's attention — the prep kernel shortens the draft
seq_lens and a new kernel left-shifts the draft block-table rows in place
(safe: input_block_tables are regathered every step, and the shift runs
after slot mappings are computed from the unshifted table). Draft KV stores
post-RoPE keys at absolute positions, so no position rewriting is needed.
Requests without cache hits and dense DFlash/DSpark setups are unaffected
(shift 0). Up to block_size - 1 restored slots stay visible when the
restored count is not block-aligned (e.g. full-prompt hits).

The draft loses the cached prefix from its context (bounded by its training
window anyway) in exchange for prefix caching and speculative decoding
composing at all. A durable alternative — letting the draft KV cache group
participate in prefix-cache block reuse — is left for a follow-up RFC.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: giorgiopiatti-dfinity <giorgio.piatti@dfinity.org>
…tention

Non-causal draft attention (DFlash/DSpark) skips trtllm-gen and runs the FlashInfer prefill wrapper, whose run() is not replay-safe once plan() changes; replaying a full CUDA graph then returns wrong output or an illegal memory access. Only claim UNIFORM_BATCH cudagraph support for causal attention, build draft attention metadata under the draft's attention config, and fall back to eager draft attention when full graphs are unsupported.

Signed-off-by: mgoin <mgoin64@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@voipmonitor, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a6d3eed0-0a81-4edd-9934-61bbaf37d9e9

📥 Commits

Reviewing files that changed from the base of the PR and between e4879e1 and caa795e.

📒 Files selected for processing (20)
  • tests/v1/attention/test_deepseek_v4_dspark_metadata.py
  • tests/v1/spec_decode/test_acceptance_length_controller.py
  • tests/v1/spec_decode/test_dflash_cudagraph_lifetime.py
  • tests/v1/spec_decode/test_dflash_prefix_cache_masking.py
  • tests/v1/spec_decode/test_dynamic_sd_cug.py
  • tests/v1/worker/test_gpu_sampling_states_seed.py
  • tools/pre_commit/generate_attention_backend_docs.py
  • vllm/platforms/cuda.py
  • vllm/v1/attention/backends/flashinfer.py
  • vllm/v1/attention/backends/mla/sparse_swa.py
  • vllm/v1/core/sched/async_scheduler.py
  • vllm/v1/core/sched/output.py
  • vllm/v1/core/sched/scheduler.py
  • vllm/v1/worker/gpu/cudagraph_utils.py
  • vllm/v1/worker/gpu/model_runner.py
  • vllm/v1/worker/gpu/sample/sampler.py
  • vllm/v1/worker/gpu/sample/states.py
  • vllm/v1/worker/gpu/spec_decode/dflash/speculator.py
  • vllm/v1/worker/gpu/spec_decode/speculator.py
  • vllm/v1/worker/gpu/states.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/ff-dspark-core-canonical-20260717

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@voipmonitor

Copy link
Copy Markdown
Author

Follow-up hardening added: block-unaligned restored prefixes now fail closed because a whole-block table shift cannot hide the residual partial block. A CPU mirror avoids GPU synchronization. Updated focused suite: 31 passed; Ruff and diff checks remain clean.

@voipmonitor

Copy link
Copy Markdown
Author

Superseded by #109. The final code was rebuilt directly on current dev/fathomless-firmament as three clean commits: correctness, optional FP8 draft head, and optional load-aware capacity. #109 preserves the validated behavior while removing the stacked merge history and excluding the unrelated #108 indexer cleanup.

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.

4 participants