Skip to content

[Test] e2e hybrid-Mamba prefix-cache corruption regression tests (#43559) - #48970

Closed
puririshi98 wants to merge 6 commits into
vllm-project:mainfrom
puririshi98:hybrid-mamba-43559-e2e-tests
Closed

[Test] e2e hybrid-Mamba prefix-cache corruption regression tests (#43559)#48970
puririshi98 wants to merge 6 commits into
vllm-project:mainfrom
puririshi98:hybrid-mamba-43559-e2e-tests

Conversation

@puririshi98

@puririshi98 puririshi98 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Purpose

End-to-end output-correctness regression tests for #43559 — hybrid-Mamba
prefix caching silently corrupting output under mamba_cache_mode="align"

Two graded arms, each compared against an enable_prefix_caching=False
control using deterministic greedy needle recall:

  • cold-race (write-side): concurrent cold prefills fragment a prefill
    chunk mid Mamba block; the chunk-end state can be cached as the boundary
    snapshot.
  • multi-turn (read-side): reuse of blocks written during (speculative)
    decode, where the eagle-lookahead lookup can let the Mamba hit length
    overrun the attention-verified hit.

Parametrized on Nemotron-Super-120B-A12B-BF16 (TP4) and
Qwen3.6-27B-FP8. Corruption raises CorruptionDetected; geometry /
control-quality / cache-liveness problems hard-fail with dedicated
exceptions (never pytest.skip, since the fork runner reports in-body
skips as PASS). An anti-vacuity liveness gate hard-fails a "clean" run
whose prefix cache is not demonstrably live (and now also hard-fails if the
≥3-block liveness probe cannot fit max_model_len, rather than silently
disabling itself). Registered as a non-gating optional Buildkite step.

What these tests actually guard (scope)

These are in-process (LLM) tests; they exercise the
HybridKVCacheCoordinator read path and the align-mode scheduler, not
the Mooncake KV-connector. Concretely:

Dependency: requires #48668 to pass. It does not exercise
#48361's Mooncake connector path (_find_hit_blocks) — #48361 is covered
by its own unit test, not here. (Kept in the #43559 stack for context;
merge order relative to #48361 does not matter for these tests.)

Why not a duplicate

Supersedes #48198 (test-only, draft). The production fixes live in #48361
(Mooncake connector) and #48668 (stats); this PR is the in-process e2e
regression coverage for #46384/#47782 + the #48668 liveness gate.

Test Plan / Test Result

pytest -v -s -rA tests/v1/e2e/test_hybrid_mamba_prefix_cache_correctness.py

Validated on GB200 (Nemotron-Super-120B-A12B-BF16, TP4, current main +
#48668; #48361 present but not exercised):

Arm Result Engagement
cold-race 1 passed liveness_probe_hits=4224, no corruption
multi-turn 1 passed wave-2 hit ratio 0.477 (67584 hits), no corruption

RED baseline (pre-#46384/#47782 main) reproduced corruption on both arms
(documented in issue #43559).

AI assistance disclosure

Developed with AI assistance (Claude Code), including GB200 validation and
an adversarial self-review that corrected the dependency scope above;
every line reviewed by the human submitter.

@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.

@puririshi98

Copy link
Copy Markdown
Contributor Author

@TomerBN-Nvidia @benchislett

@mgoin mgoin 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.

Please reduce the amount of comment slop in this PR. There must be close to 200 lines of comments here with dense blocks. This is simply not needed to this degree. Reduce to match the style of the codebase and long-lived concerns/information

@puririshi98 puririshi98 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks — fair, and done in 050adedf.

Prose is down from 286 lines to 137 (38% of the file to 23%): 252 deletions, 101 insertions. Specifically:

  • module docstring 64 lines -> 23
  • the three exception-class docstrings (~10 lines each) -> 1-4 lines
  • dropped a 5-line paragraph on which exception type means what that was duplicated verbatim in both test docstrings; it is stated once at module level now
  • ~10 inline blocks explaining calibration numbers, metric-flush mechanics and re-derivations of already-stated rationale
  • the buildkite step comment 5 lines -> 1 (it was the only commented step in that file, which rather made your point)

What I kept is the non-obvious, long-lived rationale: why grading is relative to an APC-off control rather than zero-tolerance (vLLM forces mamba_cache_mode="none" when prefix caching is off, so the arms differ numerically by construction), why these paths hard-fail instead of calling pytest.skip (the fork-based per-test runner reports an in-body skip as PASS and would silently green the step), and why ControlQualityFailure is a plain Exception rather than pytest.fail (Failed derives from BaseException and escapes the fork wrapper). Happy to trim further if any of those still read as slop.

Executable code is unchanged — verified by comparing the ASTs with all docstrings stripped — and pytest --collect-only still collects all 3 parametrizations on a GB200 node.

Comment thread .buildkite/test_areas/engine.yaml Outdated
Comment on lines +170 to +173
timeout_in_minutes: 240
device: h200
num_devices: 4
optional: true

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.

This is an extremely expensive test to run, can we make this more efficient? I think we should be able to run this on a single B200, which would be preferred due to the amount we have

Comment on lines +42 to +57
# Hybrid (full-attention + Mamba/GDN) models with MTP weights. #43559 reports
# Qwen3.6-35B-A3B; the 27B-FP8 sibling keeps this affordable on one GPU.
_QWEN_PARAM = pytest.param(
"Qwen/Qwen3.6-27B-FP8",
1,
marks=[large_gpu_mark(min_gb=80)],
id="qwen3.6-27b-fp8",
)
_NEMOTRON_PARAM = pytest.param(
"nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-BF16",
4,
# min_gb=140 separates H200 (~150 decimal GB) from H100-80GB, which OOMs
# at boot under gpu_memory_utilization=0.8.
marks=[large_gpu_mark(min_gb=140)] + multi_gpu_marks(num_gpus=4),
id="nemotron-super-120b-bf16",
)

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.

Is there a reason why you can't use smaller models, or more heavily quantized versions? For instance why not Qwen3.5 4B and nemotron 3 nano, and NVFP4 versions?

@puririshi98

Copy link
Copy Markdown
Contributor Author

Thanks — both points were right, and chasing the second one turned up a defect in my own test.

Cost. 240 min was a guess. Measured on this branch: the Qwen arm is ~130 s and the Nemotron arm ~330 s. I've split the step by model — the Qwen param was already tp=1, so it now runs on a single B200 at a 45-min budget; only the 120B TP4 param needs 4 GPUs, kept optional at 90. The remaining budget is headroom for weight download, not runtime.

Nemotron 3 Nano. No variant ships an MTP head — Nano-30B-A3B BF16/FP8, 4B and Nano-Omni have no num_nextn_predict_layers, and the 30B safetensors index has 0 of 6243 tensors matching mtp|nextn — so method="mtp" raises NotImplementedError at construction. Worth flagging against my own case: this PR's paths gate on use_eagle(), which covers eagle3 as well as mtp, and NemotronH implements SupportsEagle3 — so an eagle3 draft on Nano would exercise the same read-side hit-length drop and write-side boundary backoff. I haven't validated any Nano eagle3 head, so I can't claim that configuration works, only that "Nano is impossible here" would be wrong.

NVFP4. nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-NVFP4 does retain a real MTP module (1040 mtp.* tensors, left unquantized in BF16) and the drafter genuinely engages — I measured 88 drafts / 172 accepted at tp=1. What blocks it is geometry, not the model: at tp=1 the resolved Mamba block jumps to 8400 while the weights cap gpu_memory_utilization near 0.59, leaving ~30 GiB of KV. Every prompt in this test is sized in blocks, so the working set grows as the cache shrinks and the arms fail. It's a different test point rather than the same test on cheaper hardware, so I'd rather not fold it in here.

I also removed the cold-prefill arm, because your push on cost made me run the checked-in config end to end and it does not grade its own trigger.

It never armed. Under align mode with EAGLE/MTP the last cacheable boundary is pulled back one block (scheduler._mamba_block_aligned_split), so the sub-2-block prompt it builds commits no boundary snapshot at all — measured zero prefix-cache hits across the whole arm. Its cache-liveness probe then asserted on vllm:prefix_cache_hits, which excludes preempted and unadmitted requests (kv_cache_manager.record_prefix_cache_statspreempted_*, never exported); I instrumented it and the probe's own query delta was 0, so that counter never observed the request. The assertion "no corruption + zero probe hits ⇒ cache is dead" was therefore unsound as written.

I rebuilt it at a >2-block geometry as a same-engine cached-vs-uncached A/B — same engine, same config, single-request batches, so cache state is the only variable. That version did fire when I reverted the #45477/#47861 boundary backoff. But on a fixed tree it reported 12/40 recall regressions versus 3/40 with the fix reverted: anti-correlated, because resuming from a Mamba prefix cache legitimately differs from full recomputation by considerably more than the defect does. An 8-probe variant detected the reverted fix but flaked red 1-in-3 on correct code, with signal and noise both at ~1 probe. Rather than ship something that flags correct code, I've dropped the arm — #45477/#47861 cover that mechanism at the scheduler level, and the module docstring now says so.

The multi-turn arm is unaffected and passes on both models (hit ratios 0.444 and 0.477, 4147 and 3993 MTP drafts).

@puririshi98

Copy link
Copy Markdown
Contributor Author

@mgoin let me know if this alleviates your concerns or if you would me to make any further changes

@mergify

mergify Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @puririshi98.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Aug 19, 2026
puririshi98 and others added 5 commits August 19, 2026 14:29
…m-project#43559)

End-to-end output-correctness regression tests for vllm-project#43559 (hybrid-Mamba
prefix caching under MTP/EAGLE spec decode). Two graded arms vs an
APC-off control: a cold-concurrent-prefill (write-side) arm and a
multi-turn decode-written-block (read-side) arm, parametrized on
Nemotron-Super-120B-A12B-BF16 (TP4) and Qwen3.6-27B-FP8. Corruption
raises CorruptionDetected; an anti-vacuous liveness gate fails a "clean"
run whose prefix cache is not demonstrably live, so a silently-disabled
cache cannot pass vacuously. Registered as a non-gating optional
Buildkite step (4xH200, 240-min).

Supersedes the test-only vllm-project#48198. Requires both the corruption fix
(vllm-project#48361, Mooncake connector eagle-peek gate) and the prefix-cache stats
fix (vllm-project#48668) to pass — the cold-race liveness gate reads
vllm:prefix_cache_hits, which under-reports to 0 without vllm-project#48668.
Validated green on GB200 with both applied: cold-race
liveness_probe_hits=4224 + no corruption, multi-turn wave-2 hit ratio
0.477 + no corruption.

Signed-off-by: Rishi Puri <riship@nvidia.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… dep

Address adversarial self-review findings on the cold-race arm:

- The anti-vacuity liveness gate silently self-disabled when the >=3-block
  probe did not fit max_model_len (probe skipped, liveness_hits stayed
  None, require_hits=False -> a silently-disabled cache could pass
  vacuously). Now hard-fail with GeometryUnsupported instead of skipping,
  so the anti-vacuity guarantee cannot evaporate.
- Add vllm/v1/engine/llm_engine.py to the Buildkite step's
  source_file_dependencies: the cold-race liveness gate reads
  vllm:prefix_cache_hits, which depends on the offline stats path fixed in
  that file (vllm-project#48668).

Re-validated on GB200 (Nemotron-Super-120B-A12B-BF16 TP4): both arms still
pass (cold-race liveness_probe_hits=4224, multi-turn 67584 hits) with only
vllm-project#48668 applied (no vllm-project#48361), confirming these tests exercise the in-process
path, not vllm-project#48361's Mooncake connector.

Signed-off-by: Rishi Puri <riship@nvidia.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Align the calibration comment with the description/GB200 result: the
Nemotron-Super-120B-A12B-BF16 TP4 wave-2 hit ratio is 0.477 (measured on
GB200), not the stale 0.46. Comment-only.

Signed-off-by: Rishi Puri <riship@nvidia.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review: cut the module docstring, exception docstrings, and inline
blocks to the codebase's style, keeping only the non-obvious, long-lived
rationale (control-relative grading, why these paths hard-fail instead of
skipping, the fork-wrapper exception constraint). Executable code is
unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
…I step

The cold-prefill arm cannot grade its own trigger. Under align mode with
EAGLE/MTP the last cacheable boundary is pulled back one block, so the
sub-2-block prompt it builds commits no boundary snapshot and never arms
(measured: zero prefix-cache hits). At a >2-block geometry, where caching
is active, a cached-vs-uncached comparison is dominated by the recall
difference between resuming and full recomputation: on a fixed tree it
reported 12/40 regressions versus 3/40 with the fix reverted, i.e. the
measurement is anti-correlated with the defect. Remove the arm; the
mechanism is covered by the scheduler unit tests in vllm-project#45477 / vllm-project#47861.

Split the CI step by model. The Qwen param is already tp=1, so it runs on
a single B200; only the 120B TP4 param needs 4 GPUs. Timeouts are measured
(~130s and ~330s) rather than guessed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Rishi Puri <riship@nvidia.com>
@puririshi98
puririshi98 force-pushed the hybrid-mamba-43559-e2e-tests branch from 1b57572 to 91fee4c Compare August 19, 2026 21:36
@mgoin

mgoin commented Aug 22, 2026

Copy link
Copy Markdown
Member

Replaced by #53189

@mgoin mgoin closed this Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants