Repository navigation
Conversation
Per-request records report reused_input_tokens as the largest reuse across prefill admissions, which a readmission after preemption can raise with KV the request computed itself. The collector already tracks first-admission reuse for the summary ratio; expose it on PerRequestRecord as first_admission_reused_input_tokens, so paths without cache-tier attribution, including SGLang, report it as well. Re-pin the AgentX snapshot report digests, which hash per-request JSON. With the new field excluded from serialization, the previous digests still match. Signed-off-by: Rudy Pei <yanrpei@gmail.com> Co-authored-by: Isaac <no-reply@databricks.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description explains the behavior change, public and serialized contract impact, review map, tests, boundary cases, expected values, documentation changes, and CI status. Required sections are pre… |
| Title check | |
The title precisely states that the pull request reports first-admission cache reuse per request. It matches the main behavioral change and avoids vague wording. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Cross-Layer Contract | |
The new output field is traced through all affected layers. Rust stores the first-admission value once at on_admit and emits it in PerRequestRecord (`crates/core/src/replay/report.rs:1567-1596, 93… |
| Modeling And Data Evidence | |
The PR provides reproducible, machine-readable evidence for the changed reuse selection and ratios. The collector records the first admission value once and keeps the existing maximum-reuse field for … |
| Compatibility Boundaries | |
The change keeps the public surfaces synchronized. PerRequestRecord adds a serialized Rust field and populates it in per_request_records() (crates/core/src/replay/report.rs:933-1020, 2250-2298),… |
| Review Evidence | |
The PR description names relevant commands and results: cargo test -p aisimulate-core and the public API manifest test. It includes boundary validation for a single admission and a larger-reuse read… |
Full details: Description check
-
Explanation
The description explains the behavior change, public and serialized contract impact, review map, tests, boundary cases, expected values, documentation changes, and CI status. Required sections are present and sufficiently complete.
Full details: Cross-Layer Contract
-
Explanation
The new output field is traced through all affected layers. Rust stores the first-admission value once at
on_admitand emits it inPerRequestRecord(crates/core/src/replay/report.rs:1567-1596, 938-987, 2250-2295). The derivedSerializepath and canonical replay serializer include the field automatically (crates/core/src/replay/canonical.rs:90-120). The Python binding streams eachPerRequestRecordthrough generic JSON serialization (crates/core/src/python.rs:3005-3049), and the CLI forwards the nativeper_requestmappings torequests.jsonl(python/aisimulate/src/aisimulate/main.py:233-241). Documentation lists both reuse fields and their semantics (docs/replay/engine/kv-cache.md:78-93). Tests cover readmission preservation and both summary ratios (crates/core/src/replay/canonical.rs:382-439), single-admission reporting (crates/core/src/replay/report.rs:3198-3215), external public literals (crates/tests/public-api/src/lib.rs:828-855), streamed Python serialization equivalence (crates/core/src/python.rs:5905-5921), and updated output digest consumers (crates/core/tests/agentx_qualification.rs:649-676). The downstream Dynamo binding also treats per-request rows as generic dictionaries and uses generic Python/JSON conversion, so it has no stale field-specific schema (ai-dynamo/dynamo at commitd043ae04174571aeac28dccb156f52d59c3f77fd,lib/bindings/python/rust/llm/replay.rs:96-104,:873-891,components/src/dynamo/replay/report.py:49-70).
Full details: Modeling And Data Evidence
-
Explanation
The PR provides reproducible, machine-readable evidence for the changed reuse selection and ratios. The collector records the first admission value once and keeps the existing maximum-reuse field for later admissions (
crates/core/src/replay/report.rs:1587-1596). The new canonical test constructs admissions of 512 and 25,088 tokens, then verifies both JSON fields, admission history, counts, and both summary ratios (crates/core/src/replay/canonical.rs:384-445). The PR description explains the snapshot hash refresh: per-request JSON gains one key while summary semantics remain unchanged. The updated documentation also states the field provenance and readmission behavior (docs/replay/engine/kv-cache.md:78-94). This is not binary-only validation or an unexplained golden refresh.
Full details: Compatibility Boundaries
-
Explanation
The change keeps the public surfaces synchronized.
PerRequestRecordadds a serialized Rust field and populates it inper_request_records()(crates/core/src/replay/report.rs:933-1020, 2250-2298), while the external public-API fixture copies the field (crates/tests/public-api/src/lib.rs:790-852). The Python binding serializes records generically through Serde (crates/core/src/python.rs:3005-3048), and the CLI writes arbitrary record mappings without a second schema (python/aisimulate/src/aisimulate/output.py:81-87). The replay input schema version is unchanged; the change only extends report output and updates its documentation (docs/replay/engine/kv-cache.md:80-93). Python and Rust remain at version0.13.0(python/aisimulate/pyproject.toml:4-7,crates/core/Cargo.toml:4-8), matching the repository's cross-package version contract (python/aisimulate/tests/cross_package/test_core_version_contract.py:39-58). The diff changes no packaging or dependency manifests. Maturin still builds the wheel from the core crate (python/aisimulate/pyproject.toml:104-122), and the documented two-artifact contract remains unchanged (docs/ci/release.md:8-28). AISimulate's workspace and core dependency list contain no Dynamo dependency (Cargo.toml:4-8, 19-33;crates/core/Cargo.toml:19-47). The downstream Dynamo consumer re-exports the record and writes it through Serde rather than constructing literals (ai-dynamo/dynamo@d043ae04174571aeac28dccb156f52d59c3f77fd,lib/mocker/src/replay/mod.rs:19-28,lib/bindings/python/rust/llm/replay.rs:875).
Full details: Review Evidence
-
Explanation
The PR description names relevant commands and results:
cargo test -p aisimulate-coreand the public API manifest test. It includes boundary validation for a single admission and a larger-reuse readmission. It clearly labels the reported results as local, states that Fast CI and Full CI are pending, and does not cite inactive nested workflows as hosted CI evidence.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
Superseded by #583, which now contains this change together with the related SGLang cache lifecycle, eviction, reporting, and KV-event corrections. The consolidated branch has passed local core and public-API validation. |
Why and what changed
requests.jsonlreportsreused_input_tokensas the largest reuse across a request's prefill admissions. After a preemption, the readmission can reuse KV that the request itself computed, so that value can exceed what the cache actually provided when the request first arrived. The collector already tracks first-admission reuse and uses it for the summary'sfirst_admission_prefix_cache_reused_ratio, but per-request records only expose it through the tier-attributed G1/host fields, which SGLang and other paths without tier attribution leave empty.This adds
first_admission_reused_input_tokenstoPerRequestRecord: total prompt tokens reused from all tiers at the first admission, unchanged by later readmissions. Existing fields keep their meaning. The KV-cache docs table now lists both reuse fields and how they relate to the summary ratios.Review map
crates/core/src/replay/report.rs, then the new test incrates/core/src/replay/canonical.rs.PerRequestRecordgains a public field and per-request JSON gains a key;crates/tests/public-apiis updated for the new field.PerRequestRecordliterals must set the new field.Evidence
cargo test -p aisimulate-corepassed: 2,429 cases, zero failures, one ignored;cargo test --manifest-path crates/tests/public-api/Cargo.tomlpassed (external consumer compiled; zero runtime cases). The newcanonical_record_preserves_first_admission_reuse_after_readmissiontest admits a request twice (512 reused tokens, then 25,088 after readmission) and checks the per-request record, admission history and both summary ratios. An existing report test now also checks the new field on a single admission.Modeling or data provenance
N/A.
Tracking
None.
This pull request and its description were written by Isaac.