[PerryZhang01]

Brings DeepSeek-V4.1-Flash up on the vLLM plugin path. Text backbone only, no
KV offload — this is the half of #2357 that is about the model running under
vLLM at all, split out so it can land on its own. #2357 then becomes the
LMCache byte-offload work on top of it.

Based on main directly: it carries none of #2443's MP LMCache commits, and
none of the offload connector, STATE tier or prefix-caching work that depends
on them.

What it takes to run V4.1 under vLLM

V4.1 buys its cache in two currencies out of one contiguous allocation. PAGE
bytes scale with cached history; STATE bytes scale with in-flight requests —
each one owns a fixed-size entry holding its sliding-window ring, the
compressor's rings and its Engram cursor. vLLM can only size the first: it buys
num_blocks uniform blocks and hands every one to its BlockPool.

So the bridge declares a proxy attention layer whose page_size_bytes is
exactly one V4.1 PAGE, and then withholds the STATE tail's worth of block ids
from the scheduler while leaving the allocation's byte size alone
(deepseek_v41_state_reserve_patch). The profiled memory budget is unchanged;
what shrinks is how many of those blocks may hold history. At cache-bind time
the bridge adopts that slab as PagedAttentionCache's whole pool rather than
allocating a second one beside vLLM's accounting.

v41_proxy_geometry is the single authority for the shape: the proxy layer's
head_size, the reserve, and the bound cache all come from it, so the bytes
vLLM allocated and the bytes the kernels address cannot disagree.

Two defects in V4.1 itself

Neither is in the bridge.

V4.1 never received V4's non-immediate block reuse patch. The markers are
matched as substrings, and ".atom_deepseek_v4_proxy" is not one of
"...atom_deepseek_v41_proxy", so the model went without a patch that exists
for precisely its property: PAGE and STATE share one address space, a slot's
ring being addressed as an offset past the end of the paged region, so a block
id vLLM recycles immediately can expose live arena contents.

The deferred stale-slot probe reports false positives. prepare_state
ships the cursor rows asynchronously and judges them on the next step. Under
concurrency on this path it pairs a position with a cursor one step out of
register and, since the verdict is a hard refusal, kills the engine on state
that was never stale — always needs state at N, found N+1, off by exactly the
one token a decode step advances. The blocking path is now the default on the
plugin path only; the native engine keeps its ~58 µs/step saving, since that
is where the fault was never seen. ATOM_V41_BLOCKING_STATE_PROBE overrides
either way.

What changed in ATOM's shared code, and why

Five files outside atom/plugin/, 135 lines. The rule applied throughout: a
fact about one frontend does not live in shared code, even at the cost of a
copy. Where this path needs different behaviour it carries its own version in
atom/plugin/vllm/, and core keeps what its own engine ran.

File	Change	Why it is here and not in the plugin
model_ops/attentions/deepseek_v41/backend.py	_staging_advances(), and candidate_blocks passed to the scorer workspace	Both are defects that bite native too. The first: the cursor advance is owed exactly once, by the staging or by the caller, and the old condition (staged is None) assumed the two always travel together -- where they did not, a step advanced nothing, no error, the request one behind its own position for the rest of its life (needs state at 828, found 818). The second is below
model_ops/deepseek_v41/score_workspace.py	candidate_blocks added to the plane sizing	A layer reading an earlier layer's candidate list needs candidate_blocks × index_block_rows, which the checkpoint fixes and max_model_len does not bound -- so which of the two widths is larger flips with context length. Sizing from context alone fits every long-context configuration and overruns the short ones, and the error names the scorer rather than the budget it was sized from
model_ops/attentions/deepseek_v41/cache.py	_allocate_backing(size, device) seam, default torch.zeros; current_step held on the cache	vLLM already allocated the slab, and core has to be able to be handed storage rather than allocate a second pool beside vLLM's own accounting. current_step is a read point, not a policy: who reads it, and whether to prefer it over an argument, is the frontend's call -- getting that wrong in core is the regression described below
models/deepseek_v41/runtime.py	Two early-outs on staged_outside_forward, and the removal of @eager_break_during_capture with its import vllm	The flag names no frontend, and sits beside if not metadata.step.requests: return. The decorator's removal is a reduction in coupling: it only ever covered one frontend's PIECEWISE mode, and the work it guarded has moved out of the forward
models/deepseek_v41/config.py	One line deleted: ("plugin mode", config.plugin_config is not None)	It asked which frontend was driving, in a list where every other entry names a capability -- and no plugin backend other than vLLM registers V4.1, so it guarded nothing reachable

grep -r "from atom.plugin\|import vllm" under the V4.1 and Engram trees
returns 0.

What moved out, and what that cost

Four things started as seams in core and are now copies in the plugin:
Engram's staging boundaries, the pool geometry, Attention.forward, and
_prepare's dummy-batch cache. atom/models/deepseek_v41/attention.py,
model.py, engram/device/staging.py and engram/device/runtime.py are
byte-identical to main.

Two of those copies are procedural code main actively changes -- it added
queries to the forward and is_final_chunk/replaying to the batch
protocol in the same week -- and their drift is arithmetic rather than a
crash. Each is pinned by a digest over core's version
(tests/plugin/test_vllm_deepseek_v41_attention_copy.py), and the failure
says to port the change, because refreshing the digest is the one way to
defeat it. The geometry copy gets something stronger: a parity test that
compares every argument of both V41PoolGeometry(...) constructions and
names the field that diverged, because that pair must agree numerically --
two shapes means vLLM allocates N bytes while the kernels read M.

The one place a seam was got wrong

_attend_block read cache.current_step instead of its step argument --
a workaround for the capture freezing arguments, written into shared code.
Bounded replay re-enters attention with a tail Step while the cache still
holds the whole forward's, so a 1024-token prefill with a 128-token window
handed attention 128 rows under a Step describing 1024.
enable_decoder_swa_bounded_replay defaults to True, so that was the
ordinary native path. Caught in review; the substitution now lives in the
plugin's copy, and tests assert both directions.

What the plugin does, and why each piece exists

Module	What it does	Why
deepseek_v41_bridge.py	The adaptation: proxy layer/backend/builder, pool binding, per-step staging outside the forward, the DSpark tentative lifecycle	vLLM owns scheduling and the KV allocation; ATOM owns the CSA2 geometry and the step. This is where the two meet
deepseek_v41_state_reserve_patch.py	Withholds the STATE tail's block ids while leaving the allocation's byte size alone	vLLM can only buy the PAGE currency. The STATE region has to come out of the same contiguous slab because V41PoolGeometry.window is an offset past the absolute end of the paged region
v41_core_overrides.py	The four core classes as this frontend needs them: metadata builder, CSA2 cache, attention break + buffer, Engram staging	Each is an override on a subclass or an attribute set before any model is built. Nothing here changes native
v41_speculative.py	DSpark: the shared LM head, the scheduler guard, the tentative-step commit	None of the three exists when speculation is off, and all three failed quietly when wrong — an unshared head gives correct answers from noise drafts, visible only as acceptance length 1.04
state_slot_allocator.py	Stable request → STATE slot assignment	A model whose state is per request cannot take its slot from vLLM's block table. Shared by V4 and V4.1, so it is its own module
vllm_runtime.py	Which runner class this deployment selected; whether a capture may break; whether this batch is synthetic	Three questions about vLLM, asked by both V4 and V4.1. Each replaces an inference that read correctly right up to the deployment where it did not
req_id_passthrough_patch.py	Exposes batch-ordered req_ids, the InputBatch and the cudagraph mode	Removes the block-table D2H in slot assignment. Extended to the V2 runner, which has no _build_attention_metadata at all — the patch had been silently inert for every deployment selecting it, and inert reads exactly like applied in a log
platform.py	Startup refusals: vision, prefix caching, KV connectors, --hf-overrides, non-DSpark speculation	Refuse before a forward, not as wrong output. Called from two sites because vLLM may never activate ATOMPlatform

Why prefix caching is refused rather than fixed

vLLM's prefix cache is bookkeeping: registering a cached block is a claim that
the backend left recoverable state at that block's boundary. A V4.1 forward
leaves state only at the end of the query, never at an interior boundary.
Zeroing num_prefill_checkpoint_blocks does not opt out — align mode still
caches the last complete block's state. So this is a property of the contract,
not a flag.

Why --hf-overrides is refused

Measured through vLLM's own ModelConfig, which is what a worker gets: the
resolved config carries text_config=None and no vision_config, and ATOM's
normalize_hf_config is built from exactly those two. Normalizing vLLM's
object was attempted and reverted — it works through
AutoConfig.from_pretrained and dies in a worker. Refusing by name beats a
server that starts and answers as though the flag had not been passed.

What is supported, and what is refused

Capability	On the plugin path
Image requests	Refused — text only
DSpark speculative decoding	Supported — 5 draft tokens, fixed by the model's own config gate; the draft is the target checkpoint's mtp.* stages, so there is no separate draft model. The bridge drives the tentative Engram lifecycle and commits the accepted prefix
Prefix caching	Refused — a block-table hit restores the compressed pages but not the per-request window ring, compressor rings or Engram cursor that CSA2 attention reads alongside them
KV transfer / offload connectors	Refused — no connector on this path carries the STATE leg; a PAGE prefix restored without its STATE is a dead engine, not a degraded answer. That is what #2357 builds
CUDA graphs	Supported, opt-in — see below

The connector allow-list (_VLLM_PLUGIN_KV_CONNECTORS) is empty rather than
optimistic, and is where a connector that owns both legs gets named once one
exists. The plumbing that makes the gate reachable stays: ATOM's Config
carries vLLM's kv_transfer_config across, without which the dict is always
empty there and every connector is admitted because the question was never
asked.

CUDA graphs: opt-in, and what makes capture work

The stated reason V4.1 ran eager was that ATOM's plugin models are not fx-split,
so PIECEWISE had nothing to split on. VLLM_USE_BREAKABLE_CUDAGRAPH ends the
capture at runtime instead, so that reason does not survive it. Three further
things are each load-bearing:

• The step's host work moved out of the forward. The metadata builder
  stages it before the model is called, so nothing that must run per step sits
  inside a recorded region.
• Attention runs in an eager break, writing through a caller-owned buffer.
  The decorator requires in-place output — a fresh tensor hands back a new
  address every replay while the captured segments after it still point at the
  first. _attn_out_buffer allocates it in what stays inside the graph. This
  is what the earlier "the remedy does not apply as a decoration" was missing.
• Engram keeps its side stream, with the orderings in breaks. A fork held
  open across a segment boundary is refused (hipErrorStreamCaptureUnjoined),
  and so is a captured stream waiting on an event recorded off-capture. Under
  FULL the decorator returns the function unwrapped, so there is no break and
  the fork is joined immediately instead.

Still default-off, and not out of doubt: vLLM auto-enables
VLLM_USE_BREAKABLE_CUDAGRAPH for some architectures, so without
ATOM_V41_EXPERIMENTAL_CUDAGRAPH=1 on top of it, an upgrade that adds V4.1 to
that list would change how this path runs with nothing in the log. A test holds
that shut.

Accuracy, measured on this tree

Full GSM8K, 1,319 questions, TP4, gfx950, BF16 pool, --max-model-len 8192,
5-shot:

Arm	strict-match
Eager, no speculation	0.9280
Graphs on, no speculation	0.9280 FULL / 0.9325 PIECEWISE
Graphs on, DSpark 5 tokens	0.9303 ± 0.0070

Native TP4 reference on the same set is 0.9204; the number this work was
first written against (on #2357's base, a different tree) was 0.9242.

The speculative arm is the one the CI cell runs. Its graphs and drafts were
both confirmed active rather than silently skipped: 47 FULL and 110 PIECEWISE
graphs captured, mean acceptance length 3.47–3.57. Server identity and
max_model_len were checked against the served model before the eval, and the
source-tree fingerprint was re-checked afterwards.

Acceptance length is gated separately from accuracy in CI
(mtp_accept_threshold), because rejection sampling preserves the target
distribution: a draft that produces noise leaves accuracy intact and only
shows up as acceptance collapsing to ~1.

Testing

recipes/atom_vllm/DeepSeek-V4.1-Flash.md carries the operating instructions,
the pool-sizing arithmetic, the refusal table and the cudagraph record.

~130 new unit tests, no GPU. Failure attribution, since "all green" is not
available in a container whose aiter and xgrammar predate main:

suite	main	this branch
native (run_unit_tests.sh shape)	45 failed / 98 errors	43 failed / 98 errors
V4.1 attention + the three plugin test files	2 failed / 4 errors	2 failed / 4 errors, 216 passed

No failure appears on this branch that is not on main. The two that main
had and this run did not are in tests/model_ops/test_blockscale_packed.py,
which this branch does not touch — run-to-run variation, not a fix claimed
here. The 2 failures left in the V4.1 set come from an aiter too old for
main (make_fp4_mqa_plan), the 4 errors from an xgrammar/tvm-ffi mismatch
that breaks import vllm.v1.request; both reproduce on main unchanged.

Lint is at parity with main on every file touched.

One thing that is new here and not on #2357's measured tree

main moved under this work: PagedAttentionCache gained a workspace
argument and the pool geometry gained index_fp4. Both are threaded through
(the bridge now hands the builder's own score_workspace to the cache it
binds, as the native path does), and the bind asserts that the proxy geometry
and the runtime builder's geometry are equal before carving — a divergence is a
byte layout the kernels and the allocation disagree about, which nothing later
catches. That assertion is the one piece of logic here that #2357's hardware
runs did not cover; it is a startup-time check that names both geometries if it
ever fires.

🤖 Generated with Claude Code

[github-actions]

🏷️ CI Guide

Runs automatically on every eligible PR before approval:

• ✅ Pre Checkin: Black, Ruff, catalog schema validation, non-GPU unit tests

Heavy model tests:

• ✅ Run after the PR is approved and Pre Checkin passes
• ✅ Run immediately when an approval review is submitted
• ✅ Can be requested before approval with labels

Label	Tests
ci:full	Run all heavy PR model tests: native ATOM, vLLM, and SGLang
ci:atom	Run native ATOM model accuracy tests
ci:vllm	Run ATOM vLLM OOT model accuracy tests
ci:sglang	Run ATOM SGLang model accuracy tests

Heavy jobs are skipped when the PR is not approved and no matching ci:* label is present.
Add labels via the sidebar or gh pr edit 2494 --add-label <label>

[PerryZhang01]

Review summary

I recommend addressing the confirmed correctness issues before merging. The shared PAGE/STATE allocation and centralized proxy geometry are sensible directions; the main problems are at the boundaries between native execution, vLLM batch semantics, and upstream interfaces.

Revision scope: the original review covered 43c7cec9854a971915a373fe01693290ca19dc75. Before posting, I checked the intervening changes and revalidated the findings against da446baa07842b89cd6f9c34ecc4a3d565cc3d39. Links below are pinned to the latter revision unless explicitly marked historical. This is not a claim that the entire updated branch received another full review.

Confirmed issues

P1: The shared attention path overrides the native bounded-replay row view

Location: attention.py:426.

step = getattr(cache, "current_step", None) or _step replaces the caller's tail Step with the full-forward Step retained in the cache. The newly merged bounded-replay refactor still has this mismatch: the runtime compresses the full input, gathers tail rows, and enters rows_of, while the row-view context replaces metadata but not cache.current_step.

For an ordinary finishing, unpadded 1024-token prefill with a 128-token window, decoder attention receives 128 rows, but selects a Step describing 1024 rows. A concrete incompatible consumer is the KV reshape using step.width. The wrong full-forward compression plans can also be used earlier, so I am not claiming that this reshape must be the first observed failure.

This affects the native path with decoder bounded replay enabled by default; it does not require FULL graphs. The existing row-context unit test does not exercise this real attention/cache interaction.

Suggested change: preserve the caller's Step on native execution and select plugin live metadata at the adapter boundary. Updating every Step consumer within the scoped context is another possibility, but creates more shared mutable-state coupling. Add a native long-prefill test that reaches the real tail attention path.

P1: The globally installed sampled-output wrapper breaks the upstream calling convention

Location: v41_speculative.py:218–227.

Upstream postprocess_sampled takes sampled_tokens and defaults query_start_loc to None. The wrapper renames the former to sampled_token_ids and makes the latter required. The non-last pipeline rank calls this method with the dictionary returned by get_prev_sampled_outputs, which contains sampled_tokens and no query_start_loc.

The exact signatures and upstream keyword payload reproduce this failure in a CPU-only binding probe:

TypeError: postprocess_sampled() missing 2 required positional arguments:
'sampled_token_ids' and 'query_start_loc'

The patch is installed globally. Python argument binding fails before the V4.1-specific pending-state check executes, so other plugin models using the V2 runner with pipeline parallelism are affected too, even without speculation.

Suggested change: preserve upstream parameter names, defaults, and forwarding semantics. Add tests using the actual pipeline-parallel keyword payload and a call omitting the optional argument, with no V4.1 speculative step pending.

P1: A legal chunked-prefill tail is mistaken for speculative verification

Location: deepseek_v41_bridge.py:1538–1569.

_v41_prepare_kwargs infers draft presence from position and query length rather than actual scheduled draft metadata. This ordinary, non-synthetic batch contains no draft tokens:

speculative_tokens = 5
request A: position=4096, query_length=3, actual draft count=0
request B: position=0,    query_length=100, actual draft count=0

A 4099-token prompt can reach this state after its first 4096-token chunk, then share the next step with a new request. The short tail is classified as a draft block, and the new request triggers NotImplementedError for mixed prefill/verification. Clearing draft lists in the scheduler does not fix the predicate because positions and lengths are unchanged.

Suggested change: carry num_draft_tokens_per_req and prefill identity through the snapshot/batch adapter. Apply verification constraints only when actual drafts are scheduled; no-draft short tails should use ordinary prefill metadata. Add this mixed-prefill case as a regression test.

P2: Resolved Hugging Face configuration overrides are discarded

Locations: model_wrapper.py:457–469, configuration conversion, and bridge configuration loading.

The V4.1 wrapper keeps ATOM's freshly loaded checkpoint configuration instead of vLLM's resolved configuration, but the conversion forwards neither the resolved HF config nor its overrides. The bridge independently reloads and caches the checkpoint config by model path. For example, an override applied by vLLM is lost before ATOM constructs its RMSNorm:

--hf-overrides '{"text_config":{"rms_norm_eps":1e-5}}'

Suggested change: normalize a supported serialization/copy of vLLM's resolved configuration before ATOM's validation runs, then use that same normalized result for model construction and proxy geometry. Simply assigning the unnormalized root object back, or fixing only the model while leaving geometry on checkpoint defaults, is insufficient. Cache by resolved configuration identity rather than only model path.

This finding concerns HF fields consumed from the normalized config, not every CLI option; maximum length, dtype, and tensor parallelism have separate conversion paths.

P2: A waiting backlog unnecessarily disables draft scheduling on decode-only steps

Location: v41_speculative.py:135–170.

The wrapper treats a nonempty waiting queue as proof that the current step will mix prefill with drafts, then clears all running requests' speculative slots before calling the scheduler. Counterexample: every running request is decoding, the active-request cap is reached, waiting is nonempty, and KV capacity is sufficient with no preemption. Upstream cannot admit another request in that step, so there is no prefill to avoid.

Suggested change: base the restriction on the actual scheduled batch or actual prefill eligibility, including the active cap, token budget, and blocked/deferred state. Queue nonemptiness is not a substitute.

The confirmed effect is unnecessary clearing of speculative slots. Some slots can be asynchronous placeholders, not already-generated draft tokens. I did not measure a throughput or latency penalty.

P2: Reading a V1 block-table attribute from V2 InputBatch discards valid request IDs and forces a synchronous table copy

Location: deepseek_v41_bridge.py:901–930.

The code successfully reads input_batch.req_ids, then accesses the V1-only input_batch.block_table[0].block_table.np. The real V2 InputBatch has request IDs but no such block-table attribute. The shared exception handler discards the valid IDs too. The fallback copies block_table_tensor.cpu().numpy() before slicing the required columns and uses first-block identities instead.

For a nonempty real V2 batch with a device-resident table, each affected snapshot fallback therefore copies the entire supplied table, with size numel × element_size. This is not a claim that the snapshot is called exactly once per model step, that slot collisions necessarily occur, or that an end-to-end slowdown was measured.

Suggested change: separate request-ID acquisition from host-table acquisition. Preserve valid IDs independently and provide the V2 adapter with its separately supplied block tables or an appropriately ordered host staging view. An invalid V1 attribute lookup should not be the normal V2 path.

P2: Post-STATE-reservation capacity validation omits the permanent null block

Location: deepseek_v41_state_reserve_patch.py:94–130.

The new lower bound compares the remaining total block count with ceil(max_model_len / 256). vLLM's BlockPool permanently consumes one null block, so those totals are not all allocatable. For the plugin's 256-token PAGE:

Quantity	Value
Maximum model length	8192 tokens
Total block IDs after STATE reservation	32
Allocatable blocks after the null block	31
Blocks needed by a legal 8191-token prompt	32

The existing boundary test incorrectly accepts this case. The original upstream capacity check happens before the wrapper removes the STATE reserve, so it does not catch the resulting shortage.

For an actual reserve of R >= 1, a pre-reserve total of R + 32 can pass the original check but leave only 31 allocatable blocks after reservation. The test's reserve of 32 is a fixture, not a measured production reserve.

Suggested change: revalidate capacity after reservation using final block counts, the permanent null block, and actual per-group allocation requirements. Target and draft groups share the pool, so a complete fix needs group-aware accounting rather than assuming a fixed additional draft allocation. Correct the boundary test and add draft-group coverage.

Conditional graph concerns and design recommendations

The following are unverified hypotheses, not confirmed GPU failures or established root causes. They arose in the original review. The intervening changes do not provide an evident fix, but I have not re-proven them against the complete updated graph stack. Historical non-speculative FULL-graph accuracy results were good, and their relationship to these candidate edge cases remains unresolved.

• Compression-boundary capture: without speculation, the prepare helper returns no fixed query-width argument. If a ratio-2 compression owner is actually FULL-captured at position zero with a single token, a tight zero-capacity plan may omit compression/index-writing launches entirely. A later boundary-crossing step cannot add launches to that recorded graph. Please test the actual checkpoint's relevant owner across a compression boundary against eager execution. The actual owner/capture combination must be established before treating this as a defect.
• Inactive tails in padded FULL buckets: actual request counts and captured request capacities need distinct treatment. Updating only the live prefix can leave formerly valid plans in a bucket's inactive tail. A useful adversarial test is a large-to-small batch transition where a still-live last request moves toward the front. The non-speculative write plan already clears its full capacity, so this is not a blanket claim of stale non-speculative STATE writes; the remaining non-speculative concern is compression plans/PAGE writes, with additional write-plan/STATE concerns for speculative verification. Actual live-request corruption has not been demonstrated. This frozen-launch argument does not apply to PIECEWISE execution in the same way.

Recommended structure:

1. Keep the shared pool and centralized geometry, but derive model configuration, proxy geometry, and validation from the same resolved configuration.
2. Treat a Step as the semantic view for the current call. Do not substitute another view through hidden cache state.
3. Give capture descriptors ownership of fixed capacities and give runtime buffers ownership of actual positions, slots, and plans. Invalidate inactive entries across the complete captured capacity using sentinels the kernels actually recognize.
4. Use explicit native/V1/V2 adapters rather than interpreting absent attributes or tensor sizes as batch semantics.
5. Consider independently verifiable eager integration, DSpark, and FULL-graph changes. Test draft acceptance as well as target accuracy: rejection sampling can preserve target accuracy while an ineffective draft silently loses its acceleration.

Documentation, compatibility, and validation scope

Please reconcile the capability descriptions. The PR body still says DSpark is refused and graphs are forced off, while the recipe and implementation support DSpark and opt-in graph execution. The platform docstring still describes the experimental graph mode as "wrong on purpose", and the graph test module's opening description says PIECEWISE-only even though its tests preserve FULL-bearing modes. State defaults, opt-ins, supported combinations, and the revision associated with each measurement consistently.

Older vLLM compatibility is not a blocker in this review. The new _pool_bytes_per_block wrapper is incompatible with the older two-argument helper in vLLM 0.28, but both the original PR base and reviewed head pin vLLM 0.31.0 at db9527a46873454610df6dbedf79a36d6bf1a7f6, whose helper uses one argument. Unless older releases are part of the supported contract, this should be documented/version-gated rather than reported as a default-install regression.

Validation performed:

Check	Result and revision scope
Selected native contract/staging tests on the original immutable 43c7cec985 snapshot	75 passed, 4 skipped
Three plugin test modules on that snapshot	Collection failed because the review environment lacks vllm; not attributed to this PR
Isolated CPU probes during the original review	Reproduced short-tail rejection, unnecessary draft-clearing predicate, sampled-output binding failure, and resolved-config override loss
Syntax/static checks on the original diff	28 changed Python files parsed; shell syntax, JSON parsing, and git diff --check passed
Pre-publication update check at da446baa07	Read-only revalidation of reported paths; exact-signature CPU probe reproduced the sampled-output failure again; no full updated-branch test rerun
New GPU serving, accuracy, performance, or graph/eager comparisons	Not performed

The ordinary native unit-test script excludes tests/plugin, so adding tests there does not by itself establish coverage through that job. Please ensure the appropriate plugin job exercises the new contracts.

No source files or containers were modified during this review. The reported performance-related behaviors are not measured end-to-end performance claims.

🤖 Generated with Claude Code

[valarLip]

Reviewed at fc0b853 (after the latest push). Several points from an earlier pass are already fixed in this push: the compress-plan shape under FULL capture (_v41_prepare_shape), the fork join under FULL capture (capturing), the state slots (now V2's idx_mapping_np), _staging_advances, the core-copy digest test moving out of tests/plugin, and the staging test no longer patching the core class. They are not repeated here.

What is left, most severe first:

1. Every plugin model fails at startup on current vLLM (spec_decode_patch.py:519). The wrapper takes one argument; vLLM's _pool_bytes_per_block is (vllm_config, kv_cache_groups) and every call site passes two. The patch is installed for all plugin models, so this is not V4.1-only.
2. Requests that should be refused at startup are accepted. PP / DCP / PCP > 1 (the plugin Config never carries those sizes, so the remaining checks cannot fire), and KV connectors other than lmcache_mp (LMCacheMPConnector passes through the alias; other vLLM connector names fail with "unknown KV connector").
3. The engine can die mid-serving. A preempted request recomputing its output is not seen as prefill by the mixed-step guard; --max-model-len -1 always fails after vLLM's auto-fit; with DSpark the minimum-capacity check ignores the draft group.
4. Latent / masking issues: the proxy bind ignores the slice's storage_offset; the staged path skips the width check; the blocking state probe hides an unexplained cursor off-by-one.
5. Cleanups: eager deployments still swap in the copied _forward; patch-install results that report failure as success; docs and env vars out of sync with the code.

Details inline. Everything here is from reading the code at this commit against vLLM main; nothing was run.

[valarLip]

Thanks for the substantial work here. I went through HEAD 6746431fb and verified each point below against the pinned vLLM; 15 inline comments — the first 8 are correctness, the rest are about duplicated implementations of the same functionality.

Blocking: DSpark currently never verifies a draft — the prefill predicate in v41_speculative.py:177 is off by one token at schedule entry, so spec_token_ids is cleared every step (introduced in 061208745; the reported acceptance predates it, and the CI DSpark case gates on ≥ 0.35). The tests/plugin tests that should catch this and several other issues here are skipped by run_unit_tests.sh --ignore=tests/plugin.

Structural theme: a lot of the plugin is copies of core / V4 code (pool geometry, _prepare, Attention.forward, Engram staging, forward_hidden, proxy-layer helpers, capture probes, V4.1 identity checks, CI acceptance gate), held together by digest/AST/parity tests that can only alarm on drift. Small seams in core plus a few shared helpers would remove most of them.

Smaller items (not inline):

• Derived state stored twice: module-global _V41_PENDING_SPECULATIVE_STEP vs cache.pending; cache.current_step is added to core cache.py:361 but only the plugin reads it (and the insertion separates an existing comment from the code it describes).
• Three hand-built SimpleNamespace batches guarded by an AST test (test_vllm_deepseek_v41_batch_fields.py); seven test files each re-implement an "extract function via AST and exec" helper.
• Docs vs code: state_slot_allocator.py says V4.1 uses it (V4.1 uses V2's idx_mapping_np); model_wrapper.py:89 still says DSpark is refused; two docstrings in req_id_passthrough_patch disagree on whether V4.1 pays the D2H; _staging_advances() in the description is no longer in HEAD; --kv-cache-dtype nvfp4 + DSpark is rejected natively ("requires a BF16 KV cache"), contradicting the platform hint text and the recipe.
• Size: deepseek_v41_bridge.py is 1893 lines; _v41_stage_outside_forward and bind_deepseek_v41_proxy_cache are ~155 lines each.

[valarLip]

Follow-up: the review above was written against 6746431fb; fd6c354e1 landed while it was being posted. Checked against it:

• Addressed by fd6c354e1: PP/CP being admitted (deepseek_v41/config.py comment), the --max-model-len -1 auto-fit vs STATE tail (deepseek_v41_state_reserve_patch.py comment), and part (a) of the plugin/config.py:461 comment (V4.1 now refuses every connector).
• Still open from that plugin/config.py:461 comment: (b) and (c) — the passthrough still applies to every plugin model, _vllm_kv_transfer_config still yields a truthy dict with no connector configured (pydantic dataclass, no model_dump), so e.g. Qwen4Exp's if config.kv_transfer_config: raise now fires in plugin mode.
• All other inline comments are on files fd6c354e1 didn't touch and still apply, including the blocking DSpark predicate at v41_speculative.py:177.