feat(kv-events): publish the batch envelope, and honour kv_cache_report_mode (#352) - #421
Merged
Conversation
KV-EVENTS W1 landed the whole producer side of the KV-cache event stream and
left the consumer side empty. Nothing outside a unit test holding a BlockPool
can observe an event today: the Scheduler hard-codes
/*enable_kv_cache_events=*/false into its KVCacheManager
(src/vllm/v1/core/sched/scheduler.cpp:159), owns no publisher, and never calls
take_events(); and BlockPool::emit_cached_block_events — ported and gated in W1
— has no caller at all, because upstream fires it only under
request.kv_cache_report_mode == "full" and our Request has no such field.
This commits the W3 spec BEFORE the implementation. It re-reads the wiring
against the pin rather than trusting W1's carried line numbers (which came from
an older revision) and records:
- upstream anchors scheduler.py:86,116-119,155-158 (config -> enable flag +
publisher), :1901-1915 (drain + publish inside update_from_output),
:2456-2459 (shutdown), request.py:116-127 (report mode from extra_args),
kv_cache_manager.py:262-280 (the report_mode=="full" reuse emission);
- the design, including why the batch is assembled in update_from_output and
not schedule(), why ts is a wall clock, and why the gate injects a
CollectingEventPublisher instead of inventing a `publisher="collecting"`
config value vLLM does not have;
- that upstream has NO test for either half (grep over the pin returns three
source sites and no test), so the W3 cases are written from the source
anchors and recorded as written-from-scratch;
- three honest deferrals: the connector leg of the drain (our
OffloadingEvent is a different type owned by KV-OFFLOAD), vllm_xargs (so
report_mode is reachable from the C++ API but not yet over HTTP), and the
live ZMQ transport, which needs a third-party socket library that does not
exist under third_party/ — a dependency decision, not an implementation one.
scripts/check-gate-commands.py is re-pinned in the same change because the
checker requires it: the new Gates section names the CPU test binaries and the
ctest sweep, so KV-EVENTS gains a command that can fail and joins the runnable
population (29 -> 30). That is growth, not a lost gate. Evidence that the pin is
load-bearing rather than decorative: with the spec edit and without the re-pin,
tests/scripts/test_check_gate_commands.py fails
test_the_baseline_matches_the_shipped_record with "Items in the first set but
not the second: 'KV-EVENTS'"; with the re-pin it is OK.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
…rt_mode (#352) KV-EVENTS W1 built the whole producer side of the KV-cache event stream and nothing that could consume it. The events existed; they were unreachable. The Scheduler hard-coded /*enable_kv_cache_events=*/false into its KVCacheManager, owned no publisher and never called take_events(), so no KVEventBatch was ever constructed outside a unit test holding a BlockPool. And BlockPool::emit_cached_block_events — ported, tested, correct — had no caller at all, because upstream fires it only under request.kv_cache_report_mode == "full" and our Request had no such field. The header said so in as many words. This wires the CPU-reachable half. The envelope (scheduler.py:86,116-119,155-158, :1901-1915, :2456-2459). The Scheduler ctor takes an optional KVEventsConfig and a data-parallel rank, derives enable_kv_cache_events from it, and builds its publisher through EventPublisherFactory. update_from_output drains kv_cache_manager->take_events() and, only if the step raised any, publishes one KVEventBatch stamped with a WALL-CLOCK ts (time.time(), not a steady clock — a consumer correlates it against its own wall clock). The drain sits at the tail of update_from_output, where upstream puts it, because eviction events raised by allocate_slots during schedule() and store events raised by cache_blocks at the end of the step have to land in the SAME batch. shutdown() reaches the publisher. The report mode (request.py:116-127, kv_cache_manager.py:262-280). SamplingParams gains extra_args; Request derives kv_cache_report_mode from it with upstream's exact .get(key, "incremental") / None -> "incremental" fallback; KVCacheManager retains enable_kv_cache_events (it previously only forwarded it) and, under report_mode == "full", emits the reused prefix blocks per group after find_longest_cache_hit. A prefix-cache HIT is now reportable, which is the case a prefix-cache-aware router actually consumes. The default path is untouched, and that is gated rather than asserted. A null KVEventsConfig — what every existing call site passes — resolves the flag to the same false that was hard-coded, the factory returns a NullEventPublisher whose publish is an empty body, and report mode is "incremental" on every request that does not ask otherwise. The inertness case is green in BOTH red stages and after. RED came in two stages, because a C++ port of a missing API cannot fail an assertion before it fails to compile, and only the second stage shows the tests discriminate. Stage 1, API absent: 14 compile errors naming extra_args, kv_cache_report_mode, set_kv_event_publisher and shutdown. Stage 2, API present and the four behaviours absent: 5 of 12 cases, 5 assertions, each pointing at one missing behaviour. GREEN 12/12, 105 assertions. Upstream has no test for either half — grep over the pin returns three source sites and no test, and the event coverage in tests/v1/core/test_prefix_caching.py drives the pool directly rather than stepping a scheduler — so the new cases are written from the source anchors and recorded as written-from-scratch. One of my own expectations was wrong on the first green run and the correction is the most useful thing the gate produced: a "full" hit publishes TWO BlockStored in one batch, the reuse report and then the ordinary store of the single block the request still had to compute, parented on the last reused block. The test now asserts that whole shape, including the parent linkage that lets a consumer chain the prefix. The "incremental" control publishes only the second. Gates (CPU Release, -DVLLM_CPP_CUDA/VULKAN/METAL=OFF): test_kv_events 12/105. Regression test_scheduler 36/423, test_llm_engine 23/450, test_block_pool 14/132, test_kv_cache_manager 10/74, test_prefix_cache_stats 12/36, test_kv_cache_coordinator 16/106, test_kv_cache_utils 29/253, test_async_scheduler 7/63, test_scheduler_lpm 6/47. Full ctest 368/369 at -j 6; the single failure, test_openai_conformance, is the documented load-starver and is 23/23 · 252 re-run serially, so 369/369. Clean build, 0 warnings under -Werror. Deferred, each recorded at its call site rather than papered over. The live ZMQ transport still throws: it needs a third-party socket library that does not exist under third_party/, which is a dependency decision and not mine to make. The connector leg of the drain (scheduler.py:1903-1910) is not ported because our OffloadingEvent is a different type owned by KV-OFFLOAD, and coercing one into the other here would hide a real gap behind a plausible-looking merge. vllm_xargs stays deferred, so the report mode is reachable from the C++ API but not over HTTP. Nothing calls Scheduler::shutdown() in production yet — upstream calls it from EngineCore.shutdown and our EngineCore has no shutdown path — which is harmless while both shipped publishers are a no-op and a flag. Found while doing this and filed rather than silently fixed: #353, KVEventsConfig has no PostInit, so {enable_kv_cache_events: true} with an unset publisher throws "unknown event publisher ''" where kv_events.py:50-52 resolves it to zmq — which here should surface as the deliberately loud zmq deferral. The W3 tests set publisher = "null" explicitly and point at that issue. Row stays ACTIVE; the ZMQ transport is the named residual. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
Main moved 46 commits while KV-EVENTS W3 was being gated. Merged rather than rebased: main is never force-pushed, so the branch has to absorb main, not rewrite over it. Gates re-run on the merged tree. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
Main moved 158 commits since the last merge. Merged, not rebased: main is never force-pushed, so the branch absorbs main rather than rewriting over it. Three files conflicted and two keyed records auto-merged; none of the five was left to git's three-way result (AGENTS.md "Records"). Each was reset to `origin/main` wholesale and the branch's scoped edit reapplied against an anchor asserted to match exactly once: - `.agents/roadmap_v1.md` (CONFLICT) — main re-sorted the intake table by row ID; the two `KV-EVENTS` issue rows (#352, #353) are reinserted in the sorted position ahead of `KV-EXTERNAL-CACHE`, not where the old unsorted table had them. - `docs/USAGE.md` (CONFLICT) — both sides appended after the logprobs prose. Kept BOTH: main's `logprob_token_ids` paragraph, then this branch's "KV-cache events, and `kv_cache_report_mode`" section. - `scripts/check-gate-commands.py` (CONFLICT) — both sides grew `RUNNABLE_BASELINE` at the same anchor. Took main's set entire (which is four entries wider and drops the three DONE rows #374 closed) and reinserted only the `KV-EVENTS` entry with its re-pin rationale. - `.agents/engine-matrix.md` (auto-merged, REDONE) — main's `KV-EVENTS` row is byte-identical to the merge base's, so the row replacement reapplies cleanly. Lifecycle state is `ACTIVE` on both sides, so no matrix count moves and none of the branch's totals are carried. - `.agents/coordination.md` (auto-merged, REDONE) — reverted to `origin/main` byte-for-byte. `CLAIM-KV-EVENTS-W3` moves to `.agents/claims/CLAIM-KV-EVENTS-W3.md`, the per-claim surface #364 added while this branch was parked; its README says new claims go there, and it is what made this row conflict in the first place. `check-agent-record.py` reads both. `scripts/check-public-doc-tables.py` is taken from main unchanged: the `STATUS_RATCHET["chars"]` pin was retired by #364/#368 and is not reintroduced. `docs/BENCHMARKS.md` is untouched — this row has no measurement, so nothing is charged against its 45,000-char cap. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
The W3 evidence was captured against `main` at `7020de93`. After merging `origin/main` `157080c8` (158 commits), the gate is re-run on the merged tree and the numbers recorded where the row's evidence lives — an implementer's earlier report is not a gate result for a tree it never saw. Re-gate, CPU-only Release `build-cpu` (`-DVLLM_CPP_CUDA=OFF -DVLLM_CPP_VULKAN=OFF -DVLLM_CPP_METAL=OFF`), foreground, unbounded: build 0 warnings; `test_kv_events` 12/12 · 105; `test_scheduler` 36/36 · 423; `test_llm_engine` 24/24 · 493; `ctest --test-dir build-cpu -j 6` **385/385, 0 failed** in 28.31 s. That sweep also closes the one open question in the W3 evidence. `test_openai_conformance` was the single pre-merge failure and was recorded as a suspected load-starver; it now PASSES in-sweep at 0.35 s on a box at load 16, and 0.59 s standalone at load 2.12, against failures at load 121+. Starvation, not a defect — so the qualifier comes off the record. `CLAIM-KV-EVENTS-W3` also notes the forward merge. The row stays `ACTIVE`; the live ZMQ transport is still the named residual and `EventPublisherFactory` still throws on `"zmq"` rather than silently downgrading. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
Main advanced one commit (#416, `ENG-TRAILER-MERGE-ARTIFACTS` closed DONE) between the gate and the push. Merged forward rather than pushed over. It touches both keyed records this branch also edits, and git auto-merged both. Neither auto-merge was kept (AGENTS.md "Records"): each file was reset to `origin/main` wholesale and the branch's one scoped edit reapplied against an anchor asserted to match exactly once. - `.agents/engine-matrix.md` — #416 rewrote the `ENG-TRAILER-MERGE-ARTIFACTS` row and its lifecycle counts. The `KV-EVENTS` row is untouched by it and stays `ACTIVE`, so no count moves and none of the branch's totals are carried. - `scripts/check-gate-commands.py` — #416 removed `ENG-TRAILER-MERGE-ARTIFACTS` from `RUNNABLE_BASELINE` (DONE is closed evidence, not live gated work). Took that set entire and reinserted only the `KV-EVENTS` entry. No product source is touched by this merge, so the 385/385 CPU `ctest` recorded one commit earlier still describes this tree's product code; the record gates are re-run below it. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
Main advanced again (#418/#419, the forge's `Co-authored-by` is attribution and not authorship) while this branch was being pushed. Merged forward, never rebased. Three keyed records auto-merged and none of the three auto-merges was kept (AGENTS.md "Records"). Each file reset to `origin/main` wholesale and the branch's one scoped edit reapplied against an anchor asserted to match exactly once: - `.agents/engine-matrix.md` — the `KV-EVENTS` row replacement. #419 edits `ENG-TRAILER-MERGE-ARTIFACTS`, not this row, and `KV-EVENTS` stays `ACTIVE`, so no lifecycle count moves. - `.agents/roadmap_v1.md` — the #352 and #353 intake rows, reinserted in the sorted position ahead of `KV-EXTERNAL-CACHE`. - `scripts/check-gate-commands.py` — main's `RUNNABLE_BASELINE` taken entire, with only the `KV-EVENTS` entry and its re-pin rationale reinserted. No product source moves, so the CPU `ctest` recorded on this branch still describes this tree's product code; it and the record gates are re-run on the merged result. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
Main advanced again (#274/#397, a main baseline CI lane that can finish) between the gate and the push. Merged forward, never rebased. One keyed record auto-merged and the auto-merge was not kept (AGENTS.md "Records"): `.agents/roadmap_v1.md` was reset to `origin/main` wholesale and the #352/#353 intake rows reapplied ahead of `KV-EXTERNAL-CACHE`, against an anchor asserted to match exactly once. Every other file in the incoming range is new to this branch, so nothing else needed reapplying. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Row
KV-EVENTS, W3. Issue #352. Spec.agents/specs/kv-events.md, committed before the implementation (3e379400). ClaimCLAIM-KV-EVENTS-W3.What landed
W1 built the whole producer side of the KV-cache event stream and nothing that could consume it. The
Schedulerhard-coded/*enable_kv_cache_events=*/falseinto itsKVCacheManager, owned no publisher and never calledtake_events(), so noKVEventBatchwas ever constructed outside a unit test holding aBlockPool. AndBlockPool::emit_cached_block_events— ported, tested, correct — had no caller at all, because upstream fires it only underrequest.kv_cache_report_mode == "full"and ourRequesthad no such field.This wires the CPU-reachable half, through the existing
CollectingEventPublisherseam.The envelope (
scheduler.py:86,116-119,155-158,:1901-1915,:2456-2459). TheSchedulerctor takes an optionalKVEventsConfigand a data-parallel rank, derivesenable_kv_cache_eventsfrom it, and builds its publisher throughEventPublisherFactory.update_from_outputdrainskv_cache_manager->take_events()and, only if the step raised any, publishes oneKVEventBatchstamped with a wall-clockts(time.time(), not a steady clock — a consumer correlates it against its own wall clock). The drain sits at the tail ofupdate_from_output, where upstream puts it, because eviction events raised byallocate_slotsduringschedule()and store events raised bycache_blocksat the end of the step have to land in the SAME batch.shutdown()reaches the publisher.The report mode (
request.py:116-127,kv_cache_manager.py:262-280).SamplingParamsgainsextra_args;Requestderiveskv_cache_report_modefrom it with upstream's exact.get(key, "incremental")/None -> "incremental"fallback;KVCacheManagerretainsenable_kv_cache_eventsand, underreport_mode == "full", emits the reused prefix blocks per group afterfind_longest_cache_hit. A prefix-cache HIT is now reportable, which is the case a prefix-cache-aware router actually consumes.The default path is untouched, and that is gated rather than asserted: a null
KVEventsConfig— what every existing call site passes — resolves the flag to the samefalsethat was hard-coded, the factory returns aNullEventPublisherwhosepublishis an empty body, and report mode is"incremental"on every request that does not ask otherwise. The inertness case is green in BOTH red stages and after.Deliberately excluded
The live ZMQ transport is NOT ported, and that exclusion still holds on this head.
EventPublisherFactory::createthrows loudly onpublisher == "zmq"rather than silently downgrading to a null publisher:tests/vllm/v1/test_kv_events.cpp:234-238pins that throw. Why it is excluded rather than implemented: the PUB/ROUTER sockets, replay buffer, publisher thread and DP port offset need a third-party socket library that does not exist underthird_party/. Vendoring one is a dependency decision, not an implementation one, so it is not a call this row gets to make silently. It is the row's named residual, which is whyKV-EVENTSstaysACTIVEand does not move toDONE.Also deferred, each recorded at its call site: the connector leg of the drain (
scheduler.py:1903-1910— ourOffloadingEventis a different type owned byKV-OFFLOAD),vllm_xargs(soreport_modeis reachable from the C++ engine API but not over HTTP), and DP aggregation.Found and filed rather than silently fixed: #353 —
KVEventsConfighas noPostInit, so an enabled config with an unset publisher throwsunknown event publisher ''where upstream (kv_events.py:50-52) resolves it to zmq. W3's tests setpublisher = "null"explicitly.RED-first
RED came in two stages, because a C++ port of a missing API cannot fail an assertion before it fails to compile, and only the second stage shows the tests discriminate.
extra_args,kv_cache_report_mode,set_kv_event_publisherandshutdown.Upstream has no test for either half — a grep over the pin returns three source sites and no test, and the event coverage in
tests/v1/core/test_prefix_caching.pydrives the pool directly rather than stepping a scheduler — so the new cases are written from the source anchors and recorded as written-from-scratch.One of the author's own expectations was wrong on the first green run, and the correction is the most useful thing the gate produced: a
"full"hit publishes twoBlockStoredin one batch — the reuse report, then the ordinary store of the single block the request still had to compute, parented on the last reused block. The test now asserts that whole shape, including the parent linkage that lets a consumer chain the prefix. The"incremental"control publishes only the second.Gate numbers
Re-run on the merged head
cbabbf88, CPU-only Releasebuild-cpu(-DVLLM_CPP_CUDA=OFF -DVLLM_CPP_VULKAN=OFF -DVLLM_CPP_METAL=OFF), foreground, unbounded:-Werror)./build-cpu/tests/test_kv_events./build-cpu/tests/test_scheduler./build-cpu/tests/test_llm_enginectest --test-dir build-cpu -j 6scripts/agent-preflight.shtest_openai_conformancewas starvation, not a defect. It was the single failure in the pre-merge sweep. It passes in-sweep at 0.35-0.96 s on this head, and 0.59 s standalone on a quiet box at load 2.12, against failures recorded at load 121+. It is the documented parallel-ctest load-starver, and this sweep settles it — the qualifier is removed from the row's evidence rather than carried forward.No GPU, SACRED or CUDA gate is implicated: no kernel, no vt op, no ABI, no CMake, no model file is touched.
Merging main, and how the keyed records were resolved
The branch was interrupted before it was ever pushed, so it absorbed 160 commits of main across four forward merges (
157080c8,211ff73a,902b0e39,50374199). Merged, never rebased — main is never force-pushed, so the branch absorbs main rather than rewriting over it.No keyed record was left at git's three-way result, even where it merged cleanly (AGENTS.md §Records). Every one was reset to
origin/mainwholesale and this branch's single scoped edit reapplied against an anchor asserted in a script to match exactly once:.agents/roadmap_v1.md(conflicted, then auto-merged twice) — main re-sorted the intake table by row ID, so the KV-EVENTS: the batch envelope is never published and kv_cache_report_mode does not exist — events can only be observed by reaching into the BlockPool #352/KVEventsConfig has no __post_init__: an enabled config with an unset publisher reports "unknown event publisher ''" instead of resolving to zmq #353 rows are reinserted in the sorted position ahead ofKV-EXTERNAL-CACHE, not where the old unsorted table had them..agents/engine-matrix.md(auto-merged, redone) — main'sKV-EVENTSrow was byte-identical to the merge base's, so the row replacement reapplies cleanly. Lifecycle isACTIVEon both sides, so no matrix count moves and none of the branch's totals are carried..agents/coordination.md(auto-merged, redone) — reverted toorigin/mainbyte-for-byte.CLAIM-KV-EVENTS-W3moved to.agents/claims/CLAIM-KV-EVENTS-W3.md, the per-claim surface Shared record surfaces are a lock: 16/29 open PRs conflict, 13 of them in bookkeeping only #364 added while this branch was parked; its README says new claims go there, and the insert-at-one-anchor claims table is exactly the conflict source that made this row collide.check-agent-record.pyreads both.scripts/check-gate-commands.py(conflicted, then auto-merged twice) — both sides grewRUNNABLE_BASELINEat the same anchor. Took main's set entire, each time, and reinserted only theKV-EVENTSentry with its re-pin rationale.docs/USAGE.md(conflicted) — both sides appended after the logprobs prose. Kept BOTH: main'slogprob_token_idsparagraph, then this branch's "KV-cache events, andkv_cache_report_mode" section..agents/benchmark-record.mdneeded no union: this branch never wrote it.scripts/check-public-doc-tables.pyis taken from main unchanged — theSTATUS_RATCHET["chars"]pin retired by #364/#368 is not reintroduced.docs/BENCHMARKS.mdis untouched: this row carries no measurement, so nothing is charged against its 45,000-char cap.Found on main while merging
#420, filed not fixed.
211ff73a(#416) movesENG-TRAILER-MERGE-ARTIFACTSACTIVE -> DONEin the engine matrix and touches no public document;python3 scripts/check-doc-checkpoint.py --commit 211ff73aerrors. CI misses it becauseci.yml:240runs that checker over a--base/--headrange rather than per commit. It is another row's DONE record anddocs/BENCHMARKS.mdis cap-constrained, so repairing it inside aKV-EVENTSPR would surprise a reviewer. This PR's own range is clean:check-doc-checkpoint.py --base origin/main --head HEADreports OK.Records
Issue #352 appears in the three places AGENTS.md requires them to agree: the intake table in
.agents/roadmap_v1.md, the spec's header and## Evidence, and this body.Landing note: a GitHub squash merge duplicates the trailers and adds a
Co-authored-by(#406). A localgit merge --no-ffpreserves them.