Conversation
Bring the L3 exact-state store onto main with the accounting the disk-tier PRD requires, wire it into the kv_integration worker and the transactional restore path, bound the record queue in bytes, single-flight fills, and expose a status snapshot with the activity counters the status contract needs. Adds a kv-page-growth probe to skippy-correctness to measure write amplification before the on-disk format is fixed. Still env-gated (SKIPPY_L3_*) and off by default; the public configuration surface lands separately.
The runtime exports exact state layer-major: every layer's K rows, then every layer's V rows, each run one row per token. Adding tokens extends every run, so segments cut at fixed byte offsets land in a different place each turn and nothing dedupes. Measured on an M4 mini at 8x the newly committed bytes with 8 MiB segments, 2x at 1 MiB, against a 1.2x gate. Cut each run into fixed windows of token-rows instead, so a longer prefix reuses the segments of the shorter one it extends. The window depends only on the model's shape, never on the entry's token count, or boundaries move between turns. A geometry that does not describe the payload exactly is ignored rather than trusted, and counted, so a fallback is visible in the status rather than showing up as unexplained write amplification. The probe now spills through the real tier instead of simulating chunking, so the number it reports is the number that ships.
The 1.00x from the aligned probe was best case: turns landed exactly on window boundaries. With a 2000-token base and 300-token turns, measured on an M4 mini, 512-row windows give 1.92x over the soak and 2.55x at worst, missing the 1.2x gate. 128 rows gives 1.18x with no margin; 64 rows gives 1.07x, worst turn 1.17x. Cap the window at 64 rows. Smaller windows mean more segments, and eviction parses every manifest to build its reference map: 812 ms for 20 manifests of ~9.5k refs, which a full cache would pay on every commit. Evict to 85% of the budget instead of exactly to it, so that cost is amortised over the writes that refill the headroom, and serialize manifests compactly. Holding segments until their manifest commits is now explicit. Eviction under pressure was collecting segments a writer had already put but not yet referenced, failing the commit as "manifest references missing segment" — found by the low-water change making eviction more eager. put_segment returns a StoredSegment whose guard releases on drop, so an abandoned write frees its bytes instead of leaking them until restart.
Share one locked manager per cache root so stage-local handles use the same reservations, pins, lifecycle gate, fill claims, and activity counters. Reconcile interrupted and corrupt state before serving, and close the concurrent segment-hold race. Wire a placement-independent numerical state identity into the live disk path and expose read-only manifest/segment source contracts for a later verified network importer without allowing transport reads to bypass local admission.
Persist a placement-independent numerical model identity so model-scoped prune and clear are exact across split stages. Share record single-flight at the node manager and surface bounded effective-state transitions for low-space and storage failures. Reject incompatible or incomplete startup manifests, preserve typed admission refusals through the tier, and keep lifecycle operations behind the manager gate.
Expose a bounded runtime.kv_cache.disk contract with strict IEC sizes, field-level precedence and source reporting, legacy fallback warnings, and explicit node-manager injection across solo and split serving. Apply budget and reserve changes live with persistence rollback while staging mode and path changes for restart. Add versioned local status, exact-identity prune and clear routes, plus mesh-llm kv-cache commands; cache lifecycle work drains without blocking inference from cold fallback.
Carry status, prune, and clear over the authenticated owner-control protocol and expose bounded multi-node orchestration through the local management API and CLI. Return one result per requested endpoint, preserve input ordering under bounded concurrency, and keep exact numerical model identity filters end to end.
Extend captured agent replay with disk-off, empty-root, multi-turn, L1, restart-L3, concurrent fill/write, lifecycle-under-traffic, and forced-low-space phases. Gate exact output identity, 2x restart TTFT, one physical fill/write, 1.2x payload writes, and c64/c128/c256 p99 decode-event latency while retaining hashed evidence and reports.
# Conflicts: # crates/mesh-llm-commands/src/operational_logging.rs # crates/skippy-server/src/kv_integration/config.rs
Correctness and safety fixes from the PR 1632 review. Refusals and panics: bound manifest.kv_bytes before split_off, so a corrupt manifest is a recorded miss instead of a panic on a serving thread; fail the kv-page-growth command when the amplification gate fails, since certification automation reads the exit status rather than the printed verdict. Filesystem containment: FUSE_SUPER_MAGIC was wrong, so no FUSE mount was ever detected; statfs cannot name the subtype, so the whole class is refused. Symlink refusal now covers the components below the resolved root rather than the leaf alone, and the store canonicalises its root as the manager already did. The system prefix stays unpoliced: /var is a symlink on macOS, so refusing every symlinked ancestor would reject the default cache locations. Accounting: reserve() recomputed the managed total by stating every file under the root, on every segment put, under the admission lock. One spill of a 19K prefix is ~9.5k segments, so the write path was quadratic. The total is now kept incrementally on the put path and invalidated by every bulk mutation, and a refusal always rescans first so drift can never manufacture an InsufficientSpace. A new test asserts the cached figure against a full scan after each kind of mutation. Operations: prune with no target no longer clears the root when the budget is unset; model_identity is trimmed and validated in the HTTP handler, answering 400 rather than 500 and matching the owner-control contract; only an unknown command maps to ControlUnsupported, so a supported server's BadRequest is no longer reported as a missing capability. Also: drive-letter paths count as absolute only on Windows hosts; the manager retries the root lock while a previous in-process owner is still dropping; kv-page-growth sizes its context from its own workload; the low_space eval phase records activity_delta, which its gate reads; per-process test directories; l3.rs tests moved to l3/tests.rs, under the 2,000-line limit.
The review fixes added an eprintln! to the kv-page-growth probe and moved three approved prints to new lines, which failed the console-print drift check and cancelled the rest of CI. Drop the new print rather than approve it: the raised context is carried in the report struct and the existing verdict line, so sizing the context to the workload is still visible without adding console debt to a crate that has no event plumbing. Regenerate the ratchet for the three prints whose anchors moved: l3.rs's into l3/tests.rs after the module split, and two in kv_page_growth.rs. The legacy hit count is unchanged at 1062.
…ixes-v2 # Conflicts: # tools/xtask/data/console_print_allowlist.json
…ixes-v2 # Conflicts: # evals/README.md # evals/agentic-replay.py
…gate The 128 default (PR #564, no recorded rationale) diverged from llama.cpp's own LLAMA_SERVER_DEFAULT_N_UBATCH = 512 and missed the CUDA SSM SSD kernel gate (n_tok > SSM_SSD_MIN_TOKENS, 128, strict) by exactly one token on every default recurrent prefill, forcing the sequential-scan fallback. Measured on granite-4.0-h-1b (2026-09-08 competitive bench, same binary and protocol): TTFT p50 0.670 -> 0.415 s (C1) and 6.38 -> 3.97 s (C8), C8 decode 22.2 -> 39.4 tok/s. Dense negative control (Qwen3-1.7B) flat. Cost: +203 MiB CUDA compute buffer. Also aligns the gpu-tune planner copy, corrects the setting description (physical prefill chunk size, not decode micro-batch), and forwards the resolved n_ubatch / flash_attn llama_context lines into mesh.log so config landing is observable without buffer-size fingerprinting.
Frozen-conversation benchmark that measures serving latency across a full process restart: fill (cold server, growing multi-turn prefix), restore (SIGINT, fresh serve on the same state directory), and warm (repeat replay without restart). Server starts with production defaults; the only extra arguments are an explicit --serve-extra-args pass-through so a durable KV tier can be A/B-measured without touching the harness. Per-run provenance (source SHA, binary/model SHA-256, hardware fingerprint, manifest SHA-256) plus JSONL request rows and a Markdown report land in the output directory. Verified end to end on darwin/aarch64 (Apple M2, SmolLM2-135M-Instruct Q8_0): fill cache 61%, restore cohort captured across a measured 7s restart, warm cache 100%, zero failed requests.
- fill prefixes now end on the user turn being answered (CodeRabbit #454) - restore cohort records only the first post-restart replay; subsequent replays are resident-warm and recorded under the warm cohort (#499) - forbidden-startup-options check also rejects --opt=value forms (#166) - stream failures degrade to per-request errors instead of aborting (#348) - missing git degrades provenance instead of aborting (#413)
added 5 commits
September 10, 2026 11:31
Add explicit per-segment codec/version/calibration identity to manifests and compatibility checks. Raw is the only currently implemented representation; Q8/Q4 and lossy paths are reserved for future slices and must never alias the exact codec namespace. Changes: - Add SegmentCodec struct with name, version, and optional calibration_digest - Add codec field to HandoffSegmentRef with default to raw v1 - Validate codec identity at manifest load, commit, and assembly - Accept version 2 (pre-codec) manifests, defaulting codec to raw v1 - Reject unknown codecs, mismatched versions, and mixed-codec manifests - Add tests for: older manifests, unknown codec, mismatched version, mixed codec, corruption, restart, prune, and clear behavior - Bump MANIFEST_VERSION to 3 Test results: - cargo test -p skippy-cache: 129 passed - cargo test -p skippy-server: 707 passed - cargo clippy -p skippy-cache --all-targets -- -D warnings: clean - cargo fmt --all --check: clean
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
Collaborator
Author
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
2 participants
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.
This is the first reviewable slice for #1652, stacked on top of #1736 head
7247a397a.Summary
Add explicit per-segment codec/version/calibration identity to manifests and compatibility checks. Raw is the only currently implemented representation; Q8/Q4 and lossy paths are reserved for future slices and must never alias the exact codec namespace.
Changes
SegmentCodecstruct withname,version, and optionalcalibration_digestcodecfield toHandoffSegmentRefwith default to raw v1MANIFEST_VERSIONto 3Test Results
cargo test -p skippy-cache: 129 passedcargo test -p skippy-server: 707 passedcargo clippy -p skippy-cache --all-targets -- -D warnings: cleancargo fmt --all --check: cleanCompatibility Decisions for Review
Version 2 manifest support: The decoder now accepts version 2 manifests (written before codec identity existed) and defaults their codec to raw v1. This preserves backward compatibility for existing cache roots.
Strict codec validation: Unknown codecs, version mismatches, and mixed-codec manifests are hard-rejected at load time. This prevents silent numerical corruption.
Lossy/exact namespace separation: The
is_exact()method ensures lossy codecs (with calibration_digest) can never satisfy exact lookups. Future slices must respect this boundary.Follow-up (not in this slice)
Labels: skippy-kv
Related: #1652, #1736, #1732