Skip to content

[Bugfix][MiniMax-M3] Guard sparse decode sentinel indices - #54597

Merged
benchislett merged 7 commits into
vllm-project:mainfrom
kyleliang-nv:agent/minimax-m3-sparse-sentinel-safety
Oct 8, 2026
Merged

benchislett merged 7 commits into
vllm-project:mainfrom
kyleliang-nv:agent/minimax-m3-sparse-sentinel-safety

Conversation

@kyleliang-nv

@kyleliang-nv kyleliang-nv commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Guard MiniMax-M3's Triton sparse-decode block-table dereference against negative sentinel and out-of-range logical block indices. The independent metadata-producer correction remains #54604.

Rebased on October 5, 2026 onto public main 54d93af9fb89d0bb8392bdb72db42bb6a17cb4ca; head 4a4e4ee62ddc232d930a2666403356cad436eefc. All four PR patches are unchanged according to git range-diff. Main still has the unguarded dereference. Ready for code review, with the remaining validation below explicit.

Claims

  • Invalid top-k entries cannot become block-table or KV-cache pointers.
  • Invalid entries leave the online-softmax state unchanged; the loop stays branch-free.
  • A standalone B200 CI job runs the existing complete MiniMax-M3 attention suite so failures in unrelated, earlier commands cannot suppress it. Existing reference-accuracy thresholds are unchanged.

Validation

  • October 5: pre-commit run --from-ref 54d93af9fb89d0bb8392bdb72db42bb6a17cb4ca --to-ref HEAD and git diff --check passed.
  • The guard and its 16-case regression are unchanged by this rebase; main's surrounding test updates are retained.
  • On the previous head 1e2fc48cc515, Buildkite #92185's standalone B200 job passed: 121 passed, 45 skipped. The overall build failed in the AMD sleep-mode job, which exited with code 139; the full build is not claimed as green.
  • Earlier head dafeddcc063f passed all 16 regression cases on each of B200, L4, and MI300 (48 total) in Buildkite #91309. Historical results validate those exact commits, not the October 5 head.
  • A fresh /ci run started Buildkite #92915 for the October 5 head 4a4e4ee62ddc232d930a2666403356cad436eefc. Current-head GPU results are pending.
  • No GPU tests ran locally during this refresh: the local environment has no CUDA/ROCm device. End-to-end serving reproduction, unfixed-kernel negative controls, and all-valid decode-latency measurements remain outstanding. No performance or model-quality result is claimed.

Reproduce the focused suite on a supported accelerator:

.venv/bin/python -m pytest tests/kernels/attention/test_minimax_m3.py \
  -k 'decode_sparse_attention or decode_wrong_layout' -v

Details

Triton's bt_row + blk uses raw pointer arithmetic. For blk == -1, the unsafe access reads immediately before the request's row, not its final element. The regression uses contiguous rows and runs request 1 with a poisoned final entry in row 0, making this address deterministic.

The kernel maps invalid logical blocks to safe block 0, masks their K/V and scale loads, and preserves softmax state for those entries. The upper bound is this query token's ceil(kv_len / 128): a block that becomes valid for a later speculative token contributes nothing to the earlier token because it was already fully causally masked. This does not redefine all-empty query output in the merge kernel.

The regression covers negative and large positive invalid indices, invalid-before/after-valid orderings, split-K and single-chunk execution, both KV layouts, eager execution, capture, and three graph replays. It requires finite output and exact parity with a valid-only run on identical inputs; independent FP32-reference tests and their tolerances are unchanged. Run unfixed-kernel controls in a dedicated process because an illegal access can poison the device context.

Open-PR searches were refreshed on October 5. Related #56932 adds specialized decode paths but does not replace the general-loop guard; #57420 concerns prefill, #49054 empty-row merge NaNs, and #52060 scalar KV scales. This PR does not duplicate those changes.

The checklist review found no new rebase-related code issue. The scope remains a local kernel safety fix plus regression/CI coverage; it adds no model-runner or scheduler policy.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

Testing and Validation remains unchecked because the GPU/end-to-end items above have not all been completed. OpenAI Codex assisted with implementation, review, rebases, and tests. The human submitter must review every changed line and run/confirm the relevant validation before merge.

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)

@mergify mergify Bot added minimax bug Something isn't working labels Aug 31, 2026
@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from 78b6065 to 991029d Compare August 31, 2026 17:30
@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from 991029d to f0736aa Compare August 31, 2026 22:41
@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from f0736aa to b663838 Compare September 15, 2026 22:09

@benchislett benchislett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benchislett benchislett added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 21, 2026
@kyleliang-nv
kyleliang-nv marked this pull request as ready for review September 21, 2026 22:45

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90309 for commit 152f378b913d.

@benchislett
benchislett enabled auto-merge (squash) September 22, 2026 03:10
auto-merge was automatically disabled September 22, 2026 04:46

Head branch was pushed to by a user without write access

@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

❌ This PR is 15 commits behind upstream main. Your branch must contain every commit currently on upstream main. No new CI build was started. Merge or rebase onto the latest main, then rerun /ci run. To test this branch at your own risk, use /ci run --allow-stale.

@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from 9f43f7d to d3ad848 Compare September 22, 2026 15:26
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90441 for commit d3ad848781fe.

@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from d3ad848 to 1bcbaaa Compare September 22, 2026 16:37
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@mergify mergify Bot added the ci/build label Sep 22, 2026
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90452 for commit 1bcbaaa22f5a.

@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from 1bcbaaa to 90a990b Compare September 23, 2026 15:25
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #90694 for commit 90a990bc8ecf.

@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from 90a990b to dafeddc Compare September 25, 2026 20:47
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #91309 for commit dafeddcc063f.

@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from dafeddc to 22d85e2 Compare September 26, 2026 16:58
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #91417 for commit 22d85e287fe9.

@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch 2 times, most recently from ebf2b31 to 1e2fc48 Compare September 30, 2026 15:34
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92185 for commit 1e2fc48cc515.

@vllm-agent

vllm-agent commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

CI selector (shadow): 39 test steps (87 jobs) instead of 51 (87 jobs)

Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works.

Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.

steps (jobs) Today's rules Selector Would skip Would add
NVIDIA, CPU and others 51 (87) 39 (87) 20 (30) 8 (30)
AMD mirrors 46 (77) 30 (57) 19 (26) 3 (6)
Selector would run (39)
  • amd-fp8-moe-kernels-mi355
  • amd-kernels-mi355
  • amd-native-quantization-kernels-mi355
  • arm-cpu-test ×3
  • basic-models-tests-extra-initialization ×14
  • basic-models-tests-other
  • cpu-language-generation-and-pooling-model-tests ×3
  • cpu-multi-modal-model-tests-n ×4
  • cpu-qwen2-5-vl-multimodal-tests
  • deepseek-v4-kernel-test-b200
  • deepseek-v4-kernel-test-h100
  • distributed-model-tests-2-gpus ×3
  • kernels-attention-diffkv-test-h100
  • kernels-attention-test ×7
  • kernels-b200 ×3
  • kernels-core-operation-test ×3
  • kernels-deepgemm-test-h100
  • kernels-fla-ops-test-b200
  • kernels-flashmla-test-h100
  • kernels-fusedmoe-layer-test-2-b200s
  • kernels-fusedmoe-layer-test-2-h100s
  • kernels-helion-test ×5
  • kernels-mamba-test
  • kernels-mhc-test-b200
  • kernels-minimax-reduce-rms-test-2-gpus
  • kernels-moe-test ×5
  • kernels-quantization-test ×6
  • kernels-root-misc-test-b200
  • kimi-k3-prefix-cache-4xb200 ×2
  • language-models-tests-extra-standard ×2
  • language-models-tests-granite-l4-compatibility
  • language-models-tests-hybrid ×2
  • language-models-tests-standard
  • multi-modal-models-standard-1-qwen2
  • multi-modal-models-standard-2-qwen3-gemma
  • multi-modal-models-standard-3-llava-qwen2-vl
  • multi-modal-models-standard-4-other-whisper
  • quantized-models-test
  • vllm-ir-tests
Would skip (today's rules run them) (20)
  • ascend-npu-test
  • basic-correctness ×2
  • basic-correctness-cpu-offload
  • basic-correctness-cumem
  • basic-correctness-prefetch-offload
  • basic-correctness-sleep-mode
  • basic-models-test-other-cpu
  • basic-models-tests-initialization
  • benchmarks-cli-test
  • entrypoints-integration-api-server ×4
  • entrypoints-integration-api-server-generate
  • entrypoints-integration-api-server-openai-chat_completion
  • entrypoints-integration-api-server-openai-completion
  • entrypoints-integration-llm
  • entrypoints-integration-multimodal
  • entrypoints-integration-pooling
  • entrypoints-integration-responses-api
  • entrypoints-integration-speech_to_text
  • multi-modal-processor ×4
  • multi-modal-processor-cpu ×4
Would add (today's rules do not run them) (8)
  • arm-cpu-test ×3 (code map)
  • basic-models-tests-extra-initialization ×14 (code map)
  • cpu-multi-modal-model-tests-n ×4 (code map)
  • cpu-qwen2-5-vl-multimodal-tests (code map)
  • distributed-model-tests-2-gpus ×3 (code map)
  • kimi-k3-prefix-cache-4xb200 ×2 (code map)
  • language-models-tests-extra-standard ×2 (code map)
  • quantized-models-test (code map)
AMD mirrors: would skip (19)
  • basic-correctness ×2
  • basic-correctness-cpu-offload
  • basic-correctness-cumem
  • basic-correctness-prefetch-offload
  • basic-correctness-sleep-mode
  • basic-models-tests-initialization
  • benchmarks-cli-test
  • entrypoints-integration-api-server ×4
  • entrypoints-integration-api-server-generate
  • entrypoints-integration-api-server-openai-chat_completion
  • entrypoints-integration-api-server-openai-completion
  • entrypoints-integration-llm
  • entrypoints-integration-multimodal
  • entrypoints-integration-pooling
  • entrypoints-integration-responses-api
  • entrypoints-integration-speech_to_text
  • multi-modal-processor ×4
  • platform-tests
  • pytorch-compilation-passes-unit-tests
AMD mirrors: would add (3)
  • distributed-model-tests-2-gpus ×3 (code map)
  • language-models-tests-extra-standard ×2 (code map)
  • quantized-models-test (code map)

3 changed files · base cbee82324c · head bb31488f3a · Python record: build 93039 at 1e5d0ea888 · kernel record: table 385d86a (build 93187), map 385d86a · not counted: 10 build steps, 5 A100 steps the generator no longer emits, 24 optional steps the selector would also run

kyleliang-nv and others added 4 commits October 5, 2026 09:56
Map invalid top-k entries to a safe logical block and preserve neutral softmax state without branching in the split-K loop.

Assisted-by: OpenAI Codex
Signed-off-by: Kyle Liang <kylliang@nvidia.com>
Exercise sentinels and positive out-of-range indices before and after valid entries with both split-K and single-chunk execution. Retain layout and graph coverage after rebasing onto current main.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Kyle Liang <kyleliang-nv@users.noreply.github.com>
Compare the poisoned invalid-entry path with a valid-only run on identical Q/K/V, using exact output equality. Preserve the full layout, split-K, and CUDA graph matrix and leave independent reference-accuracy tests unchanged.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Kyle Liang <kyleliang-nv@users.noreply.github.com>
Move the existing MiniMax-M3 attention suite out of the fail-fast mixed kernel shard so unrelated failures cannot prevent its regression coverage from running. Keep all other commands and the L4/AMD coverage unchanged.

Co-authored-by: OpenAI Codex <codex@openai.com>
Signed-off-by: Kyle Liang <kyleliang-nv@users.noreply.github.com>
@kyleliang-nv
kyleliang-nv force-pushed the agent/minimax-m3-sparse-sentinel-safety branch from 1e2fc48 to 4a4e4ee Compare October 5, 2026 16:59
@kyleliang-nv

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92915 for commit 4a4e4ee62ddc.

@benchislett
benchislett enabled auto-merge (squash) October 8, 2026 17:05
@benchislett

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93659 for commit 59a61f09f1b3.

@benchislett
benchislett merged commit 1664c4f into vllm-project:main Oct 8, 2026
164 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci/build minimax ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants