fix(vllm): measure FPM self-benchmark decode points on a steady-state

[liyuanzhe1991]

Overview:

Every FPM self-benchmark decode point admits its requests as brand-new, so the
measured step ships full token arrays through scheduled_new_reqs — 8.5 MB
vs 6.8 KB per step at B=513, plus first-step bookkeeping that production
decode traffic never pays after its prefill. The recorded rows are
systematically inflated (up to 6.4x: 171.62 ms recorded vs 25.36 ms
measured end-to-end at B=256 on MiniMax-M2.7-NVFP4, 4xB200), and the perf
model downstream is fitted to the bias. This is the mechanism behind the
"more self-benchmark points somehow degrade real-traffic prediction"
observation in the AIC forward-pass evaluation: the contaminated y-values,
not the point distribution.

This PR measures each decode point on the request's second step — a
production-shaped steady-state decode iteration.

Details:

• Two-step measurement: the batch is admitted one token short
  (max(1, ctx-1) per request) and one extra step is dispatched through
  scheduled_cached_reqs (_bench_make_steady_step, mirroring the reference
  scheduler's running-request branch: per-request delta payload,
  delay_cache_blocks like the admission injection, kv/ec connector metadata
  like every sibling path). Only the steady FPM is recorded; the admission
  FPM is scaffolding and is dropped at save time.
• Timing: the steady sample uses the inter-update period. Under async
  scheduling its schedule() runs while the admission step is still on the
  GPU, so the narrow schedule->output span would also count that wait
  (measured: B=1 read 11.29 ms ≈ 6.74 + 4.83 with the narrow window, 4.71 ms
  with the inter-update period vs 4.83 ms real). The gate fails closed to
  the narrow window if the two updates are ever not adjacent.
• Coordinate-honest validation: the admission output is validated against
  the actually injected lengths; the steady FPM against the coordinate the
  steady step actually reads. ctx=1 entries cannot give up a token, so
  clamped points are recorded at their measured coordinate with a
  context_clamped sample reason (this is also what makes the safety net
  work — an admission-only FPM can never masquerade as a valid measurement).
• No new skip paths / DP-safe by construction: a rank whose steady FPM
  never arrives sends its admission FPM through the existing
  collect_result barrier, where shape validation rejects it and every
  attention-DP rank skips the point together. Regression-tested with a
  two-rank inproc synchronizer pair (one rank short, no deadlock, identical
  group decision).
• Deliberately no per-step READY/GO round for the steady step: production
  decode steps have no per-step barrier either, and a ZMQ round between the
  two updates would be counted into the steady step's inter-update wall_time
  under synchronous scheduling. Group agreement is enforced at
  collect_result.
• Guards: decode/agg self-benchmarking now rejects pipeline parallelism
  (the steady step neither returns new_token_ids for non-last PP stages nor
  honors the pp_size decode cadence), and the feasibility bound is
  clamp-aware (max(ctx,2)+2 <= max_model_len), fixing an engine-killing
  assert at the degenerate max_model_len==3 configuration.
• Artifacts carry a measurement_policy block
  (decode: steady_state_second_step) so database provenance is explicit.

Validation

• Prototype A/B (MiniMax-M2.7-NVFP4, 4xB200, vLLM 0.25.1):

point	before	after	real (end-to-end)
B=256	171.62 ms	26.71 ms	25.36 ms
B=1	6.74 ms	4.71 ms	4.83 ms

• Full scheduler unit suite — 133 tests, incl. 10 new steady-state tests
  and the DP-barrier regression test — executed in-container against
  vLLM 0.25.1; vLLM 0.26.0 API compatibility (CachedRequestData fields,
  allocate_slots kwargs, num_output_placeholders) verified against the
  0.26.0 sources.
• Adversarial multi-agent review over the final diff (6 dimensions, each
  finding independently re-verified): all confirmed findings fixed in this
  commit; 9 of 14 initial claims were refuted with code evidence.

Composition note for #12176 (eager-warmup)

The two PRs are textually independent (verified: this diff applies to main
with zero overlap against #12176's hunks). Whichever merges second should
handle two semantic touch points: warmup replicas will also run two-step
(desired — identical execution keeps DP grids rank-identical), and a warmup
replica that fails steady validation must be discarded, not recorded in
skipped_points (evaluate EAGER_WARMUP_REASON before converting a
validation failure into a skip).

Where should the reviewer start?

• components/src/dynamo/vllm/instrumented_scheduler.py:
  • _bench_make_steady_step — the production-shaped step (docstring carries
    the payload rationale)
  • _bench_step_decode — ctx-1 injection, coordinate rewrite, steady
    dispatch, deadline flow
  • _bench_steady_fpm_expected + the timing branch in _update_from_output
  • _bench_output_validation_error / _bench_save_current_point — the
    admission-vs-steady expectations and the save-time trim
• components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py —
  the "Steady-state decode measurement" block, especially
  test_two_step_group_skip_traverses_barrier_without_deadlock

Related Issues

🚫 This PR is NOT linked to an issue:

• Confirmed — no related issue

---

Summary by CodeRabbit

• New Features

  • Decode benchmarks now measure steady-state performance using a production-shaped second step.
  • Benchmark results identify the measurement policy used for decode and prefill.

• Bug Fixes

  • Improved decode timing and token accounting for multi-step measurements.
  • Added safer handling for timeouts, exhausted KV capacity, and incomplete benchmark results.
  • Decode self-benchmarking now rejects unsupported pipeline-parallel configurations.

• Tests

  • Expanded coverage for steady-state decode scheduling, validation, fallbacks, and result saving.

[coderabbitai]

Walkthrough

Changes

Decode benchmark measurement

Layer / File(s)	Summary
Benchmark state and admission validation components/src/dynamo/vllm/instrumented_scheduler.py, components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py	Benchmark state tracks admission KV tokens and expected FPM counts; decode validation now targets the shortened admission step and rejects unsupported pipeline parallelism.
Admission and steady-step scheduling components/src/dynamo/vllm/instrumented_scheduler.py, components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py	Decode points use a shortened admission injection followed by a production-shaped steady scheduler output, with coverage for allocation, timeout, clamping, and dispatch behavior.
Steady-state timing and result persistence components/src/dynamo/vllm/instrumented_scheduler.py, components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py	Wall time, FPM classification, save filtering, barrier handling, and result metadata now distinguish steady-state decode measurements from admission measurements.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: moving decode FPM self-benchmarking to the steady-state step.
Description check	✅ Passed	The description follows the template headings and provides a detailed overview, implementation notes, reviewer start points, and related-issue status.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py (1)

2998-3000: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move import time to module scope. Same applies to the import time inside test_two_step_group_skip_traverses_barrier_without_deadlock (Line 3135); time is stdlib, so there's no collection-resilience reason to defer it.

As per coding guidelines: "Keep imports at the top of the file; always flag import statements inside function bodies, methods, or classes as they hide dependencies and make modules harder to understand".

♻️ Proposed change

 def test_steady_step_unavailable_waits_out_the_deadline():
-    import time
-
     stub = InstrumentedScheduler.__new__(InstrumentedScheduler)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py` around
lines 2998 - 3000, Move the time import from
test_steady_step_unavailable_waits_out_the_deadline and
test_two_step_group_skip_traverses_barrier_without_deadlock to the module-level
imports, removing both function-local imports while preserving the tests’
behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py`:
- Around line 3131-3177: Add a pytest timeout marker to
test_two_step_group_skip_traverses_barrier_without_deadlock, using a duration
that covers its existing join timeout and cleanup. Keep the test logic unchanged
while ensuring a stalled ZMQ barrier cannot hang the suite indefinitely.

---

Nitpick comments:
In `@components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py`:
- Around line 2998-3000: Move the time import from
test_steady_step_unavailable_waits_out_the_deadline and
test_two_step_group_skip_traverses_barrier_without_deadlock to the module-level
imports, removing both function-local imports while preserving the tests’
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: afcbb6af-fd3e-4045-9b30-0580c5a03e5e

📥 Commits

Reviewing files that changed from the base of the PR and between 29ef3b5 and 1241ba7.

📒 Files selected for processing (2)

• components/src/dynamo/vllm/instrumented_scheduler.py
• components/src/dynamo/vllm/tests/test_vllm_instrumented_scheduler.py

[devin-ai-integration]

Devin Review found 2 potential issues.

[tedzhouhk]

One blocking data-grid issue inline; the steady-state timing approach otherwise looks good.