feat(ENG-EXPERT-STREAM-DEVICE): W1 — a device slot store, and the fill contract without which nothing could fill it (#1124)

[localai-bot]

--device cuda serves Qwen3.8-2.4T-A95B UD-Q1_0 today only where the platform's kernels can dereference host storage, because the only production ExpertSlotStore keeps its slots in a std::vector<uint8_t>. That is one part, a GB10. A discrete device cannot read that arena at all, so for it the slice has to live in device memory. DeviceExpertSlotStore is that store: one contiguous arena through vt::Backend::Alloc, plus one pinned host staging slot.

The fill contract is not a separate wave, and could not have been

The class could not be filled by ANY caller before this change. ExpertStreamer::EnsureFile hands SlotForWrite()'s pointer straight to ::pread, and vt::Backend::DeviceMemoryIsHostAddressable() is false for CUDA, so a device slot pointer is not a legal pread destination. Scheduling the fix as a wave after W1 would have scheduled a wave that deadlocks its predecessor. The spec says so under "Verdict on issue #1124's piece 3" and folds it in here.

So ExpertSlotStore gains CommitSlot(int32_t, size_t). SlotForWrite returns a host-writable destination, pread fills it exactly as it does today, and CommitSlot publishes it. On the host store that is a bounds-checked no-op over a SlotForWrite that still returns the slot itself, so the host path is byte-identical, which is this wave's stop condition. On the device store SlotForWrite returns the single pinned staging buffer and CommitSlot performs one contiguous H2D followed by a queue synchronize.

Three decisions inside that, recorded because the diff does not carry a reason.

CommitSlot is PURE, not a defaulted no-op. A default is correct for exactly one implementation, the host one, and silently wrong for every store whose slots the host cannot write — the entire population it was added for. Its failure presents as zeros in a slot rather than as a compile error, and that failure is literally the RED this change was gated on. The cost is two overrides: the host store's no-op and test_expert_streamer's RecordingStore, which now counts the calls.

The publish call sits INSIDE EnsureFile's existing try. Publishing is the last step of the fill and it can fail for the same class of reason the read can. A device copy that throws would otherwise leave the slot holding whatever it held before, under a cache entry that already claims the key is resident — the same silent, plausible and wrong outcome the loop is wrapped for. It takes the same Invalidate undo.

ONE staging slot, not one per slot. The filler is synchronous by design (expert_streamer.h: no async I/O, no prefetch, no read-ahead; overlap is ENG-EXPERT-STREAM W6 and is conditional on a measurement), so exactly one fill is ever in flight. A buffer per slot would double the arena's host cost — 18.55 GiB on the target checkpoint — to buffer a concurrency that does not exist. The single buffer is also what makes the write/commit pairing a contract: CommitSlot refuses a slot that is not the one SlotForWrite last handed out, because with one buffer that mismatch files one expert's weights under another expert's key and the cache then reports a HIT over the wrong bytes.

Why a staging bounce and not cuFile

A zero-copy filler (GPUDirect Storage / cuFile, or O_DIRECT DMA into a device BAR mapping) moves fewer bytes and needs a driver capability probe, a mount-level check, an aligned-I/O path and a fallback for each of those. The bounce costs one extra host-to-device copy of one slice per MISS on top of a disk read of the same size, lands in one wave, and keeps the zero-copy filler genuinely optional rather than load-bearing. It is chosen for that reason and not because it is faster. The measurement that would justify replacing it — a device-arm decode where the H2D leg is a measurable fraction of fill time — does not exist until W1 has run somewhere, and it is recorded under the spec's ## Owed.

G1, red-first

DeviceExpertSlotStore driven through ExpertStreamer::EnsureFile produces byte-identical slot contents to HostExpertSlotStore on the same input, on a CPU vt::Backend. No GPU was leased; the gate does not need one and no discrete NVIDIA GPU is reachable from this project, which is why G-DISCRETE stays owed. There is no upstream: pinned vLLM 555967922 has inference-time expert paging nowhere, so the host store is the oracle and the spec already says so.

RED, with everything present except the streamer's publish call — which is exactly #1124's third piece: test_device_expert_slot_store exit status 1, Status: FAILURE!, compile status 0, grep -c 'No space left on device' 0. All four slices failed both halves of the comparison, against the host store AND against the file, because the bytes sat in staging and never reached the device slot. Re-measured at the head of the second review's repair, where the suite is 10 cases / 112 assertions rather than the 9 / 97 the first offering had: 10 cases / 2 failed, 112 assertions / 18 failed.

GREEN, with the call: exit status 0, 10 cases / 10 passed, 112 assertions / 0 failed. test_host_expert_slot_store 9 cases / 203 assertions / 0 failed, unchanged in count from before the contract change. test_expert_streamer 12 cases / 269 assertions / 0 failed. test_expert_stream_wiring 4 cases / 882 assertions / 0 failed.

The comparison asserts equality with the FILE as well as with the host store, and its stated reason has now been wrong twice. The first draft said two empty arms would pass a bare memcmp; the first review caught that (F2) — the host arm is filled by its own streamer and is non-zero, so the host-versus-device comparison reds on its own, measured rather than conceded: with both file CHECKs deleted AND the H2D copy deleted the suite is still RED at 10 cases / 3 failed, 104 assertions / 10 failed, rc 1 (re-measured at this head). The replacement reason was "it makes the red DETERMINISTIC", and the second review showed that one wrong too (F5): the arena prefill described under F4 below now does that job, and determinism was never the stronger ground.

The ground that holds is the shared helper. Host-arm against device-arm is a comparison between two runs of the SAME ExpertStreamer, over the same descriptor, at the same file_offset. A streamer that read the wrong offset, read short, or read one slice twice makes both arms identically wrong and passes it. The bytes on disk are the only input neither arm computed, so the file CHECK is the only assertion in the case that can see that class of defect. It is kept, with the reason replaced rather than the check deleted.

The two structural claims a CPU tier could otherwise fake — that SlotForWrite hands out staging rather than the slot, and that SlotForRead hands out the slot rather than staging — are asserted as pointer relationships, because on a unified allocator both spellings produce the right bytes.

"Byte-identical" is over the filled prefix (F3). The host arena is zero-filled at construction and the device arena is not, so the two stores differ past the bytes a fill wrote. It does not reach G1, where every fill writes a whole slot, and no reader may look there — the streamer never hands out a slot it has not filled. Zeroing the arena would cost a full write of the whole budget at load, 18.55 GiB on the target checkpoint, to define bytes nobody reads.

The qualification reached the header, the gate case and the spec's ## Now, and not the two lines that DEFINE the gate six hundred lines away, which the second review found (F3 again). ## Gates G1 and the ## Tests to port row are what a W2 or a G-DISCRETE implementer reads to learn what PASS means, and both still made the unqualified claim; a gate written against that sentence would be gating a property the class does not have. Both now read "over the bytes a fill wrote", and the G1 entry says why in one sentence rather than pointing six hundred lines back.

Mutations

Seventeen, each applied alone in the tree, built, run, then restored and verified byte-identical by sha256 (RESTORE_BAD=[], four files, all True). Every row carries its compile status, its git diff --stat, and a non-zero case count, because a mutation that fails to build and a mutation that never applied both read as a passing test. M1 was rejected and rewritten for exactly that reason: deleting the copy left dst unused, -Werror=unused-variable failed the build at compile status 1, and the stale binary printed SUCCESS!. The row below is the rewritten one.

Every row is re-measured at the repaired head, not only the two the review named. The device suite is now 112 assertions where it was 97 and the streamer suite is 269 where it was 187, so every recorded denominator was stale and a stale count is a claim the tree does not support. No mutation was carried over from the previous table.

#	Mutation	compile	diff --stat	Suite	Result
M1	delete CommitSlot's H2D copy	0	1 file, 1+/1-	device	RED 10 cases/3 failed, 112 assertions/18 failed, rc 1
M2	SlotForWrite returns the slot instead of staging	0	1 file, 1+/1-	device	RED 10/2 failed, 112/17 failed, rc 1
M3	SlotForRead returns staging instead of the slot	0	1 file, 2+/1-	device	RED 10/3 failed, 112/15 failed, rc 1
M4	delete store_.CommitSlot(...) in EnsureFile	0	1 file, 0+/1-	device + streamer + wiring	RED device 10/2 failed, 112/18 failed, rc 1; streamer 12/2 failed, 255/16 failed, rc 1; test_expert_stream_wiring GREEN 4/882/0, rc 0 — the reachability control, see below
M5	delete CommitSlot's Synchronize	0	1 file, 0+/1-	device	RED 10/1 failed, 112/2 failed, rc 1
M6	delete the staged-slot identity refusal	0	1 file, 0+/10-	device	RED 10/1 failed, 112/6 failed, rc 1
M7	widen the arena overflow guard so it cannot fire	0	1 file, 1+/2-	device	RED 10/1 failed, 112/3 failed, rc 1
M8	move CreateQueue above the budget refusals	0	1 file, 1+/1-	device	RED 10/1 failed, 112/1 failed, rc 1
M9	publish on the HIT path too	0	1 file, 1+/0-	device + streamer	RED device 10/1 failed, rc 1 — and note the assertions line reads 108 / 0 failed, because the case THREW; streamer 12/1 failed, 272/5 failed, rc 1
M10	make the host CommitSlot skip its bounds check	0	1 file, 2+/9-	device + host	RED device 10/1 failed, 112/3 failed, rc 1; host suite green, correctly — the assertion lives beside the device oracle
M11	move store_.CommitSlot(...) out of EnsureFile's try	0	1 file, 1+/1-	device + streamer	RED streamer 12/1 failed, 264/6 failed, rc 1; device suite green, correctly — no store in THAT suite can fail a publish
M12	delete EnsureSpan's try/catch around WriteSlot	0	1 file, 1+/7-	streamer + device	RED streamer 12/1 failed, 235/8 failed, rc 1; device green, correctly — no store there fails a write
M13	delete Ensure's try/catch around WriteSlot	0	1 file, 1+/7-	streamer + device	RED streamer 12/1 failed, 266/8 failed, rc 1; device green, same reason
M14	drop the arena release from the constructor's catch	0	1 file, 0+/1-	device	RED 10/1 failed, 112/2 failed, rc 1
M15	drop the constructor's whole catch body	0	1 file, 1+/5-	device	RED 10/1 failed, 112/4 failed, rc 1
M16	make EnsureFile's pread ignore file_offset (the shared-helper defect, F5)	0	1 file, 1+/1-	device	RED 10/2 failed, 112/12 failed, rc 1 — and the host-against-device assertion at :359 stays GREEN through it
F2C	delete both file CHECKs AND the H2D copy	0	2 files, 2+/3-	device	RED 10/3 failed, 104/10 failed, rc 1

M12 and M13 are separate rows on purpose. They are what proves the two new wraps are independent: a try on one entry point leaves the other exactly as exposed as it was, so one mutation covering both would have measured a single guard twice.

M9's device row is the reason Status: is read and not only assertions:. 0 failed on the assertions line with a failing case is a thrown case, which is what a grep assertions: gate would have called a pass.

M11 was added by the fresh review's FAIL finding, and the finding was right. The first ten mutations did not include it. store_.CommitSlot(...) sits inside EnsureFile's try so a failed publish takes the same cache_.Invalidate a failed read takes; that placement was argued in the body, the commit, the spec and the source comment, and attacked nowhere. Moving the call to just after the catch left all three suites GREEN, because no store in the tree could fail a publish.

The scenario is real and arrives with W2: DeviceExpertSlotStore::CommitSlot calls vt::Backend::Copy and Synchronize, and a real CUDA backend throws out of both. Outside the try the throw escapes with the cache still claiming the key resident over a slot holding the previous expert's bytes; the next request is an ordinary HIT, no read is issued, and the GEMM multiplies the wrong expert with no symptom.

Repaired red-first. RecordingStore takes a throw_on_commit flag and test_expert_streamer gains "a PUBLISH that throws leaves nothing resident either", mirroring the existing case that covers the pread arm only. RED with the call moved out: 10 cases / 1 failed, 182 assertions / 6 failed, rc 1, compile status 0, no ENOSPC. The red IS the corruption rather than a proxy for it — cache.IsResident(key) stays TRUE and the retry comes back hit with filled false, which is the predicted "ordinary HIT over a slot nobody published", in the assertion output. GREEN with the call restored: 10 cases / 187 assertions / 0 failed, rc 0, expert_streamer.cpp byte-identical to git show HEAD: by sha256.

The second fresh review, and what it changed

F1 (blocking) — two more entry points had the identical corruption window

EnsureFile was wrapped and EnsureSpan and Ensure were not. Their store_.WriteSlot(...) calls are the same window one step earlier: Acquire has to run first because the write needs a slot, so by the time the write throws the cache already says the key is resident, over a slot still holding the expert the acquisition just evicted. The throw escapes, the next request for that key is an ordinary HIT, no bytes move because a hit moves none, and the GEMM multiplies the evicted expert.

This PR is what makes WriteSlot throwable. DeviceExpertSlotStore::WriteSlot calls vt::Backend::Copy and Synchronize, which route through Check() in the CUDA backend and raise std::runtime_error. Before this wave every store's WriteSlot was a memcpy.

EnsureSpan is a PRODUCTION call site, reached from Qwen3_5Model::Forward through Qwen35ExpertStream::Slice, and this PR's own device suite drives the device store through it. So this half is not the wait-for-W2 shape the publish arm has; it is in this wave's scope.

Both now take the same try / catch (...) { cache_.Invalidate(key); throw; } as EnsureFile. Gated red-first: RecordingStore takes a throw_on_write flag mirroring throw_on_commit, with one case per entry point, and each asserts CONSISTENCY rather than emptiness — the key is not resident, SlotOf is empty, resident() is 0, fills() and bytes_filled() are unmoved, the slot still holds the EVICTED expert's bytes, and the retry is a real MISS that refills.

RED with neither try present: 12 cases / 2 failed, 232 assertions / 16 failed, rc 1, compile status 0, no ENOSPC. The red is the corruption itself rather than a proxy — retry.hit is true and store.slot(retry.slot)[0] reads 4 where the test asked for expert 6. GREEN with both: 12 cases / 269 assertions / 0 failed, rc 0. Independence proven by M12 and M13 above.

F2 — the constructor leaked on the failure that happens and guarded one that cannot

It tested Alloc and AllocPinned for nullptr. No backend in this tree returns one: CpuBackend::Alloc refuses with VT_CHECK, CudaBackend::Alloc and AllocPinned refuse through Check(...), and the base Backend::AllocPinned forwards to Alloc. They all throw. So both guarded branches were unreachable, while a throw from Alloc stranded the CUDA stream and a throw from AllocPinned stranded the stream AND the whole device arena — 18.55 GiB on the target checkpoint — at the one moment the device has no memory left to lose. Out of memory is this class's headline failure; #1123 is literally vt cuda: cudaMalloc: out of memory.

The acquisitions now sit inside a try whose catch runs the destructor's body and rethrows unchanged, so the caller still sees the backend's own message. The nullptr branches are KEPT, deliberately: vt::Backend is an interface, and a nullptr-returning implementation would otherwise hand out slot pointers off a null arena instead of being refused. They now cost one branch and no cleanup code, because the catch owns the release. The header's claim that a failed allocation raises this constructor's own std::runtime_error is corrected to describe what actually happens, and the .cpp comment that said nothing is acquired above a possible throw now says which part of that is still true.

Gated by throw_on_alloc and throw_on_pinned_alloc on the suite's CountingBackend, with frees, pinned_frees and last_freed counters so the assertion is "the arena came back", not "no crash". RED at 10 cases / 1 failed, 112 assertions / 4 failed, rc 1, compile 0; GREEN at 112 / 0. Mutations M14 and M15 above.

F4 — two recorded mutation counts were allocator-dependent

The "each slot holds a DIFFERENT slice" check compared two device slots that a publish-suppressing mutation leaves UNWRITTEN, and vt::Backend::Alloc does not initialise them, so whether that assertion red was decided by std::aligned_alloc garbage. The reviewer measured M4 device at 112/18 against the recorded 17 and the F2 combination at 104/10 against the recorded 9; both deltas were that single assertion. That is the very allocator non-determinism the F2 correction invokes as its justification, appearing inside the gate's own assertions.

The gate now writes every device slot to a known byte before the fills, so an unmutated fill is the only thing that can make two slots differ, and the assertion measures the store. Proven, not asserted: under M4 the assertion at test_device_expert_slot_store.cpp:374 fails on 25 consecutive runs and the suite reads 112 assertions / 18 failed on all 25. The whole table above is re-measured at this head for the same reason.

F5 — the file CHECK's reason, restated rather than deleted, and then measured

Covered under G1 above: the check is load-bearing on the shared-helper ground, not on determinism, and it stays. Restating a reason in prose is the same shape as the reason it replaced, so M16 measures it. Making EnsureFile's pread ignore file_offset fills every key in BOTH arms from offset 0 — one shared helper, one defect, two identically wrong arms. The host-against-device assertion at test_device_expert_slot_store.cpp:359 stays GREEN through exactly that, as the argument predicts, and the only assertions that red are the two file CHECKs at :370 and :371 and the different-slice check at :375: 10 cases / 2 failed, 112 assertions / 12 failed, rc 1, compile 0, tree restored byte-identical by sha256.

That is the whole case for keeping a check whose stated reason was wrong twice: it is the only assertion in the case that can see a class of defect the rest of the case is structurally blind to.

Nothing lands dead: this lands UNREACHED, and here is what that means exactly

What is not reached: DeviceExpertSlotStore. No loader, no model, no example and no registered command constructs one. Qwen35ExpertStream::store_ is still a std::unique_ptr<HostExpertSlotStore> and Qwen35ExpertStream::Slice reads the concrete HostExpertSlotStore::Slot, so there is no seam a device store could be handed to yet.

The owning row: ENG-EXPERT-STREAM-DEVICE, wave W2 — the virtual SlotForRead, store_ becoming a std::unique_ptr<ExpertSlotStore>, and selecting the store from the platform.

The tracking issue: #1124, which stays OPEN. This is one wave of three.

The spec lists it under ## Owed, the landing commit body names it, and the class carries the statement in its own header comment. The spec's own recommendation is still to land W1 and W2 as one pull request; the split was a dispatch decision and this declaration is the cost it names.

Measured rather than asserted, and the earlier draft of this line overstated it. git grep -n DeviceExpertSlotStore -- src include examples benchmarks tests, excluding the class's own files and its own suite, returns five matches, every one of them inside a // comment: expert_streamer.h:18, expert_streamer.cpp:159, and test_expert_streamer.cpp:50, :84 and :396. Filtering the comment lines out leaves rc 1, no output — no construction, no declaration, no type use anywhere outside the class. Two of those five predate this review; the third source one is a comment F1's repair added. The positive control that makes this an absence rather than a wrong pattern is the same grep for HostExpertSlotStore, which returns qwen3_5.cpp:5912 and :5941 — a real production construction and a real production member.

The narrower half, stated so nobody reads it generously. The CommitSlot CALL is not in the same position as the class. It sits in ExpertStreamer::EnsureFile, which IS a production call site: qwen3_5.cpp's Qwen35ExpertStream::Slice reaches it from Qwen3_5Model::Forward, and the line executes on every real streamed fill today. What it does there is nothing, because the store production selects is the host one and its CommitSlot is a no-op. The reachability mutation says so in numbers: deleting that call reds the new suite (M4 above) and leaves test_expert_stream_wiring — the suite that enters through Qwen3_5Model::Forward — at 4 cases / 882 assertions / 0 failed, rc 0, compile status 0, unchanged from its baseline and re-measured at this head. A call count is not a capability. Both halves wait on W2.

One thing the second review's F1 does change here, and it is worth saying precisely. The EnsureSpan and Ensure wraps are not in the unreached half. Those entry points are production paths today — Qwen35ExpertStream::Slice calls EnsureSpan from Qwen3_5Model::Forward — so that repair is reached at its own merge commit, on the host store, on every streamed fill. What stays unreached is the CLASS, exactly as stated above.

Records

.agents/specs/expert-stream-device-slots.md: ## Now records the wave and the G1 result, the W1 work-breakdown section records what landed and the pure-vs-defaulted decision, ## Owed converts the conditional "W1 may land unreached" entry into the factual one, and the ## Tests to port table is corrected to the file that exists. .agents/issue-index.md is NOT touched: #1124 already has a row, and the index is append-only, so a second row would be the duplicate check-agent-record.py refuses.

The gate file is tests/vllm/model_executor/test_device_expert_slot_store.cpp, not the test_expert_slot_store.cpp the spec's table named when it was written. Named rather than done quietly: the header it gates is device_expert_slot_store.h and the suite sits beside test_host_expert_slot_store.cpp, which is its ORACLE, so a name that does not say which store it is about is the one thing a reader has to disambiguate every time. The spec table is corrected to match the tree.

No public document is owed. This edits src/, include/, tests/ and one spec; no row changes lifecycle state, no benchmark ID changes disposition, and no user-visible surface moves.

Not mine

check-agent-record and test_agent_record were RED on this branch and on origin/main because .agents/issue-index.md listed #1649 twice. That was repaired on main by #1742 while this branch was in review, and merging it in cleared the gate here. This branch never touched that file.

At the repair head scripts/agent-preflight.sh is rc 0 with no FAIL and no SKIP, trailer and commit-style gates included, and test_cpu_x86_llamacpp_floor (the #618 flake) passed on the first run with no discrimination needed. check-commit-style.py --range origin/main..HEAD and check-commit-trailers.py --range origin/main..HEAD are both rc 0.

CI at 7e0cf84ac (run 32612431657): 14 of 17 jobs green, and the three reds are each attributed by reading the log rather than assumed. Green includes build-test-cpu, build-test-cpu-arm64, build-test-vulkan, build-newest-gcc, cuda-fat-build, cuda-arch-features, both sanitize-cpu lanes, device-leakage, pr-size, commit-protocol-tag, documentation-checkpoint, last-gated-commit and vulkan-spirv-freshness.

• windows-msvc-cpu and windows-msvc-vulkan: #584, not this branch. Both fail at tests\Release\test_openai_api_server.exe exited with status -1073740791 (0xC0000409, the __fastfail status). Attributed by MATCHED ARM rather than by argument: the run on this PR's own previous head a3c89a609 (run 32608548556) fails both lanes with the byte-identical signature, before any of this repair existed. test_openai_api_server crashes on Windows with STATUS_STACK_BUFFER_OVERRUN (0xC0000409), unmasked by the #512 fix #584 already carries the diagnosis, owned by ENG-RELEASE-WINDOWS, so no new issue is filed.
• agent-record: #1722, not this branch. It errors in SiteGuardTests.test_rendered_benchmark_index_links_resolve_to_emitted_pages with FileNotFoundError: [Errno 2] No such file or directory: 'hugo' — the runner has no Hugo. Attributed by baseline: agent-record is failure on main's own runs at 6354755ba (runs 32611673963 and 32608320394) and at b508cbce6 (run 32613454280). This branch edits no docs-site input.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]