Skip to content

feat(skippy-cache): per-segment codec contract, CPU golden CacheGen, CubeCL spike (#1652) - #1752

Closed
i386 wants to merge 12 commits into
scama/skippy-l3-streaming-restorefrom
jy/skippy-codec-capability-contract
Closed

i386 wants to merge 12 commits into
scama/skippy-l3-streaming-restorefrom
jy/skippy-codec-capability-contract

Conversation

@i386

@i386 i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

First reviewable follow-on slice of #1652, stacked on #1750 (head a0876ee0) over #1736 (head e63f86ca6). Synced onto main d4ffbbacd 2026-09-10 by ordered merge-forward — reviewed heads 7247a397a / 5effeffb1 / 34dd77e9c all preserved as ancestors; the only manual conflict resolutions were two Cargo.toml dep blocks (main's v0.76.0 version bump vs the stack's added deps).

  • 29d87ef3cmanifest v4: per-segment codec capability contract. Every v4 segment carries {codec, version, exact/lossy class, decoded_len, calibration_digest}. Stripped-field v4 rejects at load, commit, and assembly (no payload-codec/raw fallback); v2/v3 read-compat on-disk; unsupported segment codecs refused naming the segment index. CodecClass separates exact (payload-digest verified) from lossy (calibration-namespaced, never satisfies an exact lookup).
  • 23814d0adpure-Rust CPU golden CacheGen. Static-CDF byte-rANS ported from public-domain ryg_rans (the coder LMCache ships), per-segment min/max affine 4-bit quantization, mod-16 token-axis delta, CGv1 container carrying calibration + 16-entry CDF histogram. Calibration digest (BLAKE3 over bit-exact calibration + shape) namespaces lossy entries. Test proves a real CacheGen segment is refused at commit while raw is the only supported class. skippy-protocol's f16 RNE conversion is now shared, not duplicated.
  • 106c31752CubeCL feasibility spike + backend plan. cubecl 0.10.0 pinned behind the optional cachegen-spike feature; nothing in the library links CubeCL (now pinned =0.10.0).
  • e00a539f4 + 1d3bbd672first-round review fixes: CGv1 u32 histogram + header validation, synchronized spike timings, committed golden fixture; backend plan re-measured with the synchronized harness.
  • 03d1fbd2asecond-round review fixes: container boundary validation (NaN/inf calibration, negative scale — flat-tile scale 0 stays legal — overflowed shapes, with attack-reproduction regressions); the golden fixture now pins the encoder too (byte-for-byte vs the frozen 129-byte stream, provenance upstream c9d162d9 → independent C repro → fixture); honest spike accounting (all four live buffers, H2D includes the 8-byte calibration, H2D/D2H timed to completion).
  • 34dd77e9cthird-round review fix: MAX_DECODED_VALUES bound to a decode working set, not a token count — 2^302^24 (32 MiB decoded f16; worst-case decode working set ~112 MiB vs the ~7 GiB a 92-byte header could previously command), enforced symmetrically through one shared admission check (checked_tile_len) routed through both parse and encode, encode guard firing before the f32 materialization. Boundary regressions at both edges (65536×256 admitted; 16777217 = 97×172961 refused everywhere); Rust 1.98 clippy fixed; console-print allowlist regenerated for the stack's drift.

Spike results (M2 Max, 4096x128 tile, release build, synchronized stages, bitwise equality)

Metric cubecl-cpu wgpu (Metal)
Cold JIT (quantize / undelta) ~63 / ~25 ms ~33 / ~10 ms
Warm dispatch (avg) ~4.1 / ~1.5 ms ~7.7 / ~5.9 ms
H2D bytes (f32 tile + 8 B calibration, to completion) 2,097,160 2,097,160
D2H bytes (symbols + rebuilt, to completion) 4,194,304 4,194,304
Live device buffer peak (tile + calibration + both outputs) 6,291,464 6,291,712
Encoded-size ratio (rANS / raw) 0.075 (13.3x) 0.075 (13.3x)
Equality vs CPU reference (bitwise) exact, 0/524,288 mismatches exact, 0/524,288 mismatches

Every timed stage ends with client.sync() inside the timer; H2D/D2H are timed to real completion. Honest shape: quantization is embarrassingly parallel; the token-axis delta is a per-column sequential scan so output is bit-exact by construction (parallel scan = follow-up). rANS stays CPU in this reference, measured as ratio only. Copy path is the honest one (create_from_slice/read_one, no zero-copy into the store's packed segments) — per the agreed stop rule, evidence is back before CubeCL becomes a committed dependency.

Backends: CPU and Metal are the only implemented ones (real-hardware verified). CUDA and HIP/ROCm are compile-only gates until they run on real hardware — docs/skippy/CACHEGEN_BACKEND_PLAN.md (relabeled: quantize+delta kernel spike verified; full backend unimplemented).

Bounds held: no request-path wiring, no Candle/Burn/Python/PyTorch, native runtime-format passthrough remains the exact control arm, lossy entries can never satisfy an exact lookup.

Status

  • Code review cleared at 34dd77e9c; post-sync review cleared at aa533c0d4 (merge chain, ancestor preservation, conflict resolutions, both CI catalog blobs identical to main).
  • Post-sync GitHub runs are terminal but planning-only (pr-draft profile, required_slices=[]; execution lanes skipped — correct for a draft). The full execution gate is the outstanding item.
  • Validation on the sync head aa533c0d45e1b8af025ee9af9887de68a7f02baf, tree clean: skippy-cache 165 passed / 2 ignored, skippy-protocol 72 passed, fmt clean, clippy -D warnings --all-targets with and without cachegen-spike, no-console-print + repo-consistency pass, release spike PASS CPU+Metal.
  • Pending owner decision: mark ready for the pr-ready matrix, or trigger the CI Manual Full workflow (workflow_dispatch) while staying draft. Draft state unchanged; CacheGen stop rule intact.

i386 and others added 6 commits September 10, 2026 15:21
…#1652)

First slice of codec versioning: record an explicit codec name+version in the
L3 handoff manifest so payload representations are namespaced rather than
guessed, and refuse a payload this build cannot decode before assembly.

- Add PayloadCodec { name, version } and CODEC_RAW / CODEC_RAW_VERSION. raw is
  the only implemented representation; the segment bytes are unchanged.
- HandoffManifest gains a #[serde(default)] codec field, defaulting to raw.
  A manifest written before codec identity existed has no field and reads as
  raw — backward compatible, and MANIFEST_VERSION is unchanged so existing
  on-disk entries stay valid.
- reject_unsupported_codec gates both try_commit (nothing unassemblable is
  ever persisted) and assemble (an unknown codec/version is a miss, never a
  silent misinterpretation of the bytes).

Tests (skippy-cache, l3): explicit-raw round trip, legacy manifest without a
codec field reads/assembles as raw, unknown codec refused at commit (no
manifest left) and before assembly, unknown raw version refused on both paths,
and a supported-codec payload whose bytes are corrupted still fails digest
verification. No Q8/Q4 data path, request-path, or #1650/#1651 changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d paths (#1652 review)

Addresses scama's two compatibility blockers on the raw codec slice.

1. Codec identity was downgradeable. With a serde-default codec at a fixed
   manifest version, the field could be stripped to force a raw reinterpretation
   of possibly non-raw bytes (the whole-payload digest still matches those
   bytes, bypassing the gate). Fix: bump MANIFEST_VERSION to 3, which *requires*
   an explicit codec, and add LEGACY_MANIFEST_VERSION (2) decoded as raw through
   an explicit legacy path. `codec` is now `Option<PayloadCodec>` so a stripped
   v3 codec is detectable and rejected instead of defaulted.

2. Unsupported codecs were locatable "hits". decode_manifest now centrally
   rejects an unsupported/absent codec, so every load path enforces it: startup
   reconciliation quarantines the manifest, manifest_for_prefix prunes the bad
   link and locate_longest falls back to a shorter supported prefix, and a
   direct load_manifest fails — all before segments are read or the LRU heats.

Tests: downgrade (strip codec from a current-version manifest rejects; a
genuine v2 legacy manifest still reads/assembles as raw), on-disk direct-load
rejection, startup reconciliation quarantine, and locate_longest
unsupported-longest/supported-shorter fallback — in addition to the existing
round-trip, unknown-codec/version, and corruption-guard cases.

Scope unchanged: payload-level raw-only; per-segment/mixed codec identity is
later #1652 work.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…commit/assemble (#1652 review)

try_commit and assemble checked codec support but not manifest.version, so a
raw-codec manifest carrying an unknown future version could be persisted (and
its segments read) even though decode_manifest/load_manifest reject it on the
next read. Centralize both checks in validate_manifest_compatibility (supported
current/legacy version + supported codec) and call it before commit and before
any segment access.

Regressions: a future-version raw manifest is refused by commit (leaving no
persisted manifest) and before assembly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bumps the L3 manifest format to v4 and moves codec identity down to the
segment level, the contract #1652 needs before any non-raw codec exists:
each segment names its codec, representation version, exact/lossy class,
decoded length, and (lossy only) calibration digest.

- MANIFEST_VERSION is now 4 and requires explicit identity on every
  segment; v3 (#1750, payload-level codec) and v2 (pre-codec) become
  explicit legacy read paths, never written.
- The per-segment identity is Option only so a v4 manifest with the field
  stripped is detectable and rejected — it never falls back to the
  payload codec or to raw. The check runs in decode_manifest (load),
  try_commit (persist), and assemble (read), so nothing unloadable is
  ever persisted or partially served.
- CodecClass distinguishes exact entries (payload-digest verified) from
  lossy ones (calibration-namespaced, never satisfy an exact lookup).
  A lossy identity must carry a calibration digest; an exact identity
  must decode to its own stored bytes.
- Writers stamp SegmentCodecIdentity::raw on every segment; capability
  negotiation names the offending segment index on refusal.

Regressions: v4 per-segment stamping round trip, v3 read compatibility,
v4 stripped-segment rejection at load/commit/assemble (single and all
segments), unsupported segment codec naming the segment, and decoded_len
mismatch refusal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the deterministic CacheGen reference the later GPU work must match
bit-for-bit: per-segment min/max affine 4-bit quantization, token-axis
delta decorrelation, and static-CDF byte-rANS, wrapped in a versioned
CGv1 container that carries its calibration and 16-entry symbol
histogram (the CDF metadata) so decoding is self-contained.

- reference.rs is the math oracle: every op is single-precision IEEE-754
  or integer, so any backend performing the same ops in the same order
  reproduces it exactly. The calibration digest is BLAKE3 over the
  bit-exact calibration values plus tile shape — lossy lookups match
  only identically-calibrated entries.
- rans.rs ports the canonical public-domain ryg_rans byte coder (the
  same one LMCache uses), deliberately in divide/mod reference form:
  this crate is the correctness oracle, not the performance path.
- container.rs defines the wire format, the lossy SegmentCodecIdentity
  a CacheGen segment carries, and the store-contract test proving a
  real CacheGen segment is refused at commit while raw is the only
  supported class.

Deliberate simplification, documented in reference.rs: calibration is
per-segment min/max affine rather than the paper's per-model K/M-mixed
calibration; the container is shape-versioned so both are follow-up
experiments behind the same wire contract.

skippy-protocol: the f16 RNE conversion moves to a pub(crate) module
re-exported from binary, so the reference reuses the exact conversion
instead of duplicating a subtle bit-exact routine.

Regressions: rANS round trips (skewed, single-symbol, table rejection),
quantization error bounds, flat tiles, delta wrap-around, deterministic
histogram derivation, container corruption rejection (magic, reserved
bytes, length lies, histogram tampering), noise non-expansion,
per-segment identity namespacing, and the end-to-end store refusal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1652)

Slice 3 and 4 of the #1652 follow-on, both evidence-only:

- cubecl 0.10.0 is pinned behind the optional `cachegen-spike` feature;
  nothing in the library links CubeCL. The example proves bit-exact
  CPU+Metal parity of the quantize+delta and undelta+dequantize kernels
  against the CPU golden reference, and reports the six agreed metrics:
  cold JIT, warm dispatch, H2D/D2H bytes, peak temporary memory,
  encoded-size ratio (0.075 = 13.3x on the smooth 4096x128 tile), and
  equality (symbols and values exact on both cubecl-cpu and wgpu/Metal).
- Honest kernel shape: quantization is embarrassingly parallel; the
  token-axis delta is a per-column sequential scan, so the result is
  bit-exact by construction. A parallel scan is the follow-up; rANS
  stays CPU in this reference and is measured as ratio only.
- The stop rule is respected: the spike measured the honest copy path
  (create_from_slice/read_one, no zero-copy interop with the store's
  packed segments); the evidence goes back before CubeCL becomes a
  committed dependency.
- docs/skippy/CACHEGEN_BACKEND_PLAN.md: Metal and CPU are the only
  implemented backends (real-hardware verified); CUDA and HIP/ROCm are
  compile-only gates until they run on real hardware, with explicit
  capability failure through the v4 per-segment identity gate and no
  hidden fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@i386 i386 added the skippy-kv Work coordinated in Buzz #skippy-kv label Sep 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2d9db079-835e-4d8a-8191-0198a703d67d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Exact-head review at 106c317524abebe40da0283096c347317a13ba7b: the manifest-v4 compatibility gate itself looks sound, and the exact-head package suites pass (skippy-cache 157 passed / 1 ignored; skippy-protocol 72 passed). The CacheGen/spike portion has five blockers before this should leave draft.

  1. The CGv1 container cannot encode the tile used as the spike evidence. container.rs:35-37 says the tile is capped at 64 Ki symbols, but encode_f16_segment never enforces or chunks that cap; it serializes each symbol count as u16 at lines 117-120. On this exact head, both a flat and the PR's smooth 4096x128 f16 tile return Err(symbol count exceeds u16 histogram). The spike bypasses the container and rANS-encodes device symbols directly, so its 0.075 ratio is not evidence for a working CGv1 container at the stated shape. Use a wide enough count field or explicit deterministic tiling, then run the end-to-end container at the same measured shape.

  2. Malformed histogram totals can make decode spend pathological time in normalization. parse_container accepts histogram counts without checking that their sum equals rows * dims, then histogram_to_freqs repairs the resulting frequency sum one decrement at a time. A parsed total of 1 with [65535; 16] did not return within 200 ms and has billions of decrements to perform. Validate the histogram sum and all checked shape/length arithmetic before normalization; also reject non-finite/negative calibration fields at the header boundary.

  3. The reported CubeCL timings do not measure completed kernel work. timed_stage deliberately leaves _client unused and calls launch() without a device synchronization; read_one happens only after the timed loop. The 5/4 us figures are enqueue costs, while the actual work is paid later. The cold figure is also explicitly from a warm compilation cache, so the required cold JIT number is still absent. Synchronize after the cold launch and each measured warm launch (or use backend timestamps), report transfer time separately, and rerun from a cleared compile cache. The equality gate must compare to_bits() or final f16 bytes: the current < 1e-6 tolerance does not support the PR's bit-exact claim.

  4. There is no independent golden fixture. The rANS tests round-trip this encoder through this decoder, and the GPU spike compares against the same Rust implementation. No committed expected byte stream or decode fixture comes from ryg_rans, LMCache, or the paper. The implementation also deliberately substitutes per-segment min/max quantization for CacheGen's calibrated K/M path. Add an externally derived fixed fixture that pins symbols, normalized frequencies/CDF, rANS bytes, and decoded output. If the simplified algorithm is intentionally a prototype rather than CacheGen-compatible, name and document it that way instead of calling it the CacheGen golden oracle.

  5. CubeCL is not exact-pinned. version = "0.10.0" is Cargo caret syntax and accepts later 0.10.x releases. Use =0.10.0 or an immutable git revision as required by the spike gate.

Keep the stop rule active. The backend-plan table should describe Metal as a verified quantize/delta kernel spike until the full codec path and honest synchronized measurements pass; it currently overstates that backend as implemented.

jian yang and others added 2 commits September 11, 2026 08:00
…ke timings, golden fixture (#1652)

Address the five PR #1752 review blockers on top of 106c317:

- CGv1 histogram entries widen from u16 to u32 counts. A 4096x128 tile
  is 524,288 symbols and cannot be encoded under the old cap; CGv1 now
  carries every tile the codec accepts. Regression: end-to-end
  encode->container->decode at exactly the measured 4096x128 shape.
- The container parser validates sum(histogram) == rows*dims before any
  table is built, and histogram_to_freqs enforces the same caller
  contract, so a forged or corrupt header can no longer drive
  unbounded normalization repair; the convergence loop carries an
  explicit iteration bound. Regressions: corrupt + off-by-one
  histogram-total rejection at both layers.
- Spike timings are now synchronized: every timed stage ends with
  client.sync() inside the timer (warm dispatch is total/iterations,
  not unsynchronized enqueue), the client parameter is used, cold JIT
  is the true first synchronized launch of the kernel specialization,
  and equality is bitwise (to_bits) with mismatch counts reported.
  Release-run numbers on M2 Max, 4096x128: cold 38/8 ms (CPU/Metal
  quantize+delta), warm 0.58/4.6 ms; ratio 0.075; 0/524288 mismatches
  on both backends.
- An independent golden rANS stream is committed
  (src/cachegen/fixtures/ryg_rans_golden.bin) with a deterministic
  generator test (--ignored) and a decoder test that reproduces the
  declared symbol sequence from the frozen artifact.
- cubecl is pinned to exactly =0.10.0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…zed re-run (#1652)

The backend plan's six-metric table now reflects the PR #1752 fix head:
synchronized stages (client.sync() inside the timer), true first-launch
cold JIT, bitwise equality with mismatch counts, and the convergence
sweep across iteration counts. Notes why per-process first launch is
the true cold path in cubecl 0.10.0 (the only on-disk kernel cache,
SPIR-V, is Vulkan-only and not enabled here).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Fix head for all five review blockers is pushed:

  • e00a539f4CGv1 u32 histogram + header validation, honest spike timings, golden fixture.
  • 1d3bbd672backend plan re-measured with the synchronized harness (replaces the enqueue-only table).

1. u16 histogram cap (blocker 1): CGv1 histogram entries are now u32 counts; the header grows 32→64 bytes and the container carries every tile the codec accepts. New regression the_measured_4096x128_tile_round_trips_through_the_container runs encode → CGv1 container → decode at exactly 4096×128 — the shape the spike reports.

2. Unbounded normalization (blocker 2): the parser now rejects any container where sum(histogram) != rows*dims before a table is built (u64 total, no overflow), and histogram_to_freqs enforces the same caller contract plus an explicit convergence bound on the increment loop. Regressions cover corrupt and off-by-one totals at both layers.

3. Timings (blocker 3): timed_stage now synchronizes via client.sync() inside the timer (warm = total/iterations over synchronized launches; _client is used); cold JIT is the true first synchronized launch of the kernel specialization in-process (cubecl 0.10.0's only on-disk kernel cache is the Vulkan-only SPIR-V cache, not enabled here, so per-process first launch is the genuine cold path for CPU and Metal); equality is bitwise (to_bits) with mismatch counts printed, and an alphabet range-check guards the u32→u8 cast. Release re-run on M2 Max, 4096×128: cold JIT 38/13 ms (CPU) and 8/4 ms (Metal) per kernel; warm 578/577 µs (CPU) and 4590/4593 µs (Metal); convergence verified across 5/50/200/500 iterations; ratio 0.075; 0/524,288 symbol and value mismatches on both backends.

4. Golden fixture (blocker 4): src/cachegen/fixtures/ryg_rans_golden.bin is committed: a 1000-symbol skewed stream through the canonical ryg_rans construction (50/50 split CDF, long runs, multi-byte renormalize paths), with meta header (count + stream length). The generator is a deterministic --ignored test (provenance is re-checkable); the decoder test reproduces the declared symbol sequence from the frozen artifact.

5. Exact pin (blocker 5): cubecl = "=0.10.0".

PR stays draft; the copy-path stop rule is untouched. Validation on the fix head: skippy-cache 160 passed / 2 ignored (including the fixture generator), skippy-protocol 72 passed, clippy -D warnings clean with and without cachegen-spike, fmt clean, spike PASS on cubecl-cpu + wgpu(Metal).

@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Exact-head re-review at 1d3bbd6728f53cea55f119a1043d2dd6512f1ba2 clears the u16-cap failure, the pathological histogram-normalization loop, the unsynchronized parity measurement, and the CubeCL version pin. The 4096x128 container regression passes, and my release rerun produced 0/524,288 symbol and value mismatches on both CPU and Metal with ratio 0.075.

Three blockers remain before this leaves draft.

  1. The container boundary still accepts invalid calibration and unbounded decoded shapes. parse_container reads min_bits and scale_bits at container.rs:187-188 and returns them at :216-224 without validating either float. I mutated a valid one-value container: min = NaN was accepted by both container_calibration and decode_f16_segment (the latter returned an f16 NaN), and scale = -1.0 was also accepted and decoded. The earlier review explicitly required finite calibration and non-negative scale at the header boundary. The shape arithmetic is still not bounded either: a 92-byte CGv1 payload with dims=16, rows=u32::MAX, all 16 histogram counts u32::MAX, and a four-byte stream is accepted by decoded_value_count as 68,719,476,720 values. decode_f16_segment then uses unchecked rows * dims, allocates Vec::with_capacity(count) at :137, and later count * 2 at :146. Keep scale == 0 for valid flat tiles, but require finite min/scale, scale >= 0, checked shape/byte arithmetic, and a decoded-size/expected-length bound before allocation.

  2. The frozen rANS fixture only gates the decoder, not the encoder. The normal test at rans.rs:198-220 decodes the file, while the ignored “generator” at :166-189 calls the same RansEncoder under test and overwrites the fixture. That does not detect encoder drift and cannot establish provenance. I independently compiled canonical upstream ryg_rans rans_byte.h at commit c9d162d996fd600315af9ae8eb89d832576cb32d; its 129-byte stream matches the committed fixture byte-for-byte (SHA-256 7887612d9c251e19cfa54558718d3388dd7a5b5dfd7f8d0f419681eaddc62a32). The bytes are good. Add a normal assertion that the Rust encoder produces those frozen 129 bytes for the pinned symbols/CDF, and record that upstream commit in the fixture provenance. Do not make the same-implementation overwriter the provenance check.

  3. The spike still understates memory and does not report the requested transfer time. At cachegen_cubecl_spike.rs:164-173, values_handle, symbols_handle, rebuilt_handle, and calib_handle are all live, but peak_temporary_bytes sums only symbols + rebuilt. The harness peak is 6,291,464 bytes, not 4,194,304; H2D also includes the eight-byte calibration buffer. create_from_slice and both read_one calls remain untimed, despite the prior request to report transfer time separately. Measure H2D/D2H durations (or mark them explicitly unmeasured), report the actual live allocation peak, and change CACHEGEN_BACKEND_PLAN.md:11-12 from “Real, verified” to the precise state: CPU/Metal quantize+delta kernel spike verified; full CacheGen backend/container path remains unimplemented and rANS is still CPU.

Validation on this exact head:

  • cargo test -p skippy-cache --lib: 160 passed, 2 ignored.
  • cargo test -p skippy-protocol --lib: 72 passed.
  • cargo fmt --all -- --check: clean.
  • cargo clippy -p skippy-cache --all-targets -- -D warnings: clean.
  • The same clippy command with --features cachegen-spike: clean.
  • Release spike, M2 Max, 4096x128, 20 iterations: CPU cold 62/17 ms and warm 828/839 us; Metal cold 28/21 ms and warm 4096/4748 us; both bitwise exact, ratio 0.075.

The red GitHub checks are indeed planner-catalog drift rather than a code/test failure: this head and base 7247a397a contain ci/slices.yml blob 203189ab..., while current main contains 7d1cceaf... from 80ffde505; the failing job reports exactly that comparison. Landing #1736 alone will not change this source head. Avoid an isolated rebase now: fix these blockers first, then sync the dependent stack once. If #1736 lands meanwhile, rebase/retarget the surviving #1750/#1752 descendants onto current main in order so the source catalog matches the protected catalog.

Keep #1752 draft and keep the copy-path stop rule active.

… honest spike accounting (#1652)

Address the three re-review blockers on top of 1d3bbd6:

- Container boundary: parse_container now rejects non-finite
  calibration (NaN/inf min or scale), negative scale (flat-tile
  scale == 0 stays legal), checked-overflow tile shapes, and any
  rows*dims above a format ceiling (MAX_DECODED_VALUES = 2^30). The
  validated count travels in the header; decoded_value_count,
  container_calibration, and decode_f16_segment all refuse, and
  decode sizes (count*2) are checked before allocation. Regressions
  reproduce the review's exact attacks: NaN min, inf min, NaN scale,
  scale = -1 (all three consumers), and the 92-byte rows=u32::MAX
  container that used to report 68,719,476,720 values.
- Golden fixture now pins the encoder too: a normal test asserts the
  Rust encoder reproduces the frozen 129-byte stream byte-for-byte
  for the pinned symbols/CDF (upstream ryg_rans c9d162d9, stream SHA-256
  7887612d...), so encoder drift fails CI instead of being overwritten
  by the same-implementation generator.
- Spike accounting: live device buffer peak now sums all four buffers
  the harness holds (tile + calibration + symbols + rebuilt =
  6,291,464 bytes on CPU, 6,291,712 on Metal), H2D includes the
  8-byte calibration upload, and H2D/D2H are timed to completion
  (CPU: 2,097,160 B up / 4,194,304 B down; Metal: ~0.9-1.4 ms up,
  ~3.0-4.1 ms down). Backend plan relabels CPU/Metal as quantize+delta
  kernel-spike verified, full CacheGen backend unimplemented, rANS
  still CPU.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Fix head for the three re-review blockers is pushed: 03d1fbd2a55503deba46e47b45637bc86f20d9e9 (single commit on top of 1d3bbd672).

1. Container boundary (blocker 1): parse_container now rejects non-finite calibration (NaN/inf min or scale), negative scale — with scale == 0 still legal for flat tiles — checked-overflow tile shapes, and any rows*dims above a new format ceiling (MAX_DECODED_VALUES = 2^30; raising it is a format-version decision). The validated count travels in ContainerHeader, all three consumers (container_calibration, decoded_value_count, decode_f16_segment) refuse hostile inputs, and the decoded byte size is checked before allocation. Regressions reproduce your exact attacks: NaN min, inf min, NaN scale, scale = −1.0 (asserted at all three consumers, not just decode), and the 92-byte dims=16, rows=u32::MAX, histogram=16×u32::MAX, 4-byte-stream container that previously reported 68,719,476,720 values — now rejected at parse.

2. Encoder pin (blocker 2): new normal (non-ignored) test ryg_rans_golden_fixture_pins_the_encoder asserts the Rust encoder reproduces the frozen 129-byte stream byte-for-byte for the pinned symbols/CDF. The provenance chain is now: canonical upstream ryg_rans c9d162d9 → your independent C reproduction (stream SHA-256 7887612d…, which I confirmed hashes the stream bytes of the committed file) → frozen fixture → both decoder and encoder gates in the normal suite. The --ignored generator remains only as a regeneration tool and is no longer part of any provenance claim.

3. Spike accounting (blocker 3): the peak now sums all four live buffers — tile + calibration + symbols + rebuilt = 6,291,464 bytes on CPU, 6,291,712 on Metal (allocator rounding difference visible and reported) — H2D includes the 8-byte calibration upload (2,097,160 bytes total), and H2D/D2H are timed to completion: CPU 2,097,160 B up in ~12–18 ms / 4,194,304 B down in ~36–44 µs; Metal up in ~0.9–1.4 ms, down in ~3.0–4.1 ms. CACHEGEN_BACKEND_PLAN.md now labels CPU and Metal as "quantize+delta kernel spike verified — full CacheGen backend/container path unimplemented; rANS stays CPU" rather than "Real, verified", and the measurement table carries the transfer timings and live peak.

Stack handling per your instruction: no rebase yet — this head still carries catalog blob 203189ab, so the planner stays red by design until the one-time sync after these blockers clear (or a #1736 landing triggers the ordered rebase/retarget).

Validation on 03d1fbd2a: skippy-cache 163 passed / 2 ignored, skippy-protocol 72 passed, fmt clean, clippy -D warnings clean with and without cachegen-spike, release spike PASS on cubecl-cpu + wgpu(Metal) with 0/524,288 mismatches and ratio 0.075. PR remains draft; stop rule untouched.

@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Exact-head re-review at 03d1fbd2a55503deba46e47b45637bc86f20d9e9 clears the rANS fixture and spike-accounting blockers. The container boundary is improved, but one allocation-safety blocker remains, and the new test does not pass the repository's clippy gate on the current toolchain.

Blocking: the inclusive format ceiling still permits a multi-GiB allocation bomb

MAX_DECODED_VALUES is 1 << 30. I built a 92-byte, otherwise well-formed container with dims = 16, rows = 67,108,864, histogram[0] = 1,073,741,824, and a four-byte rANS state. decoded_value_count accepts it and reports 1,073,741,824 values.

decode_f16_segment then reserves roughly 1 GiB for symbols, materializes roughly 4 GiB of f32 values in reference::dequantize, and reserves roughly 2 GiB for the f16 output. The checked decoded_byte_len call happens only after the f32 allocation. That leaves a tiny input able to request about 7 GiB of live host storage, even though the source comment says this ceiling is far above any real KV segment and the L3 path is designed around bounded segments. This does not satisfy #1652's “Bound CacheGen encode/decode buffers” requirement.

Please bind the decoder to the segment's expected decoded_len or a realistic configured segment/working-set limit before any allocation. Apply the same limit in encode_f16_segment before collecting the f16 input into Vec<f32>; today the encoder can emit a container its own parser refuses. Add a regression at the largest accepted boundary, not only a shape above it.

Blocking validation failure

Both clippy configurations fail under the unpinned current Rust 1.98.0 toolchain at container.rs:438:

error: using `chunks_exact_mut` with a constant chunk size
help: consider using `as_chunks_mut::<4>().0`

This is confined to the new hostile-header regression and should be mechanical to fix, but -D warnings is not clean at this head.

Cleared from the prior review

  • The normal suite now pins both directions of the frozen rANS artifact: the Rust encoder reproduces the upstream-derived 129-byte stream byte-for-byte, and the decoder reconstructs the declared sequence. I retain the independent upstream C reproduction at canonical ryg_rans commit c9d162d996fd600315af9ae8eb89d832576cb32d.
  • Calibration rejects NaN/inf and negative finite scales; checked shape arithmetic and histogram-total validation happen in the capability probes as claimed.
  • The spike now counts all four live device buffers, includes the 8-byte calibration upload, times H2D/D2H to completion, and labels CPU/Metal as kernel-spike evidence rather than full backends.

Exact-head verification:

  • cargo test -p skippy-cache --lib: 163 passed, 2 ignored
  • cargo test -p skippy-protocol --lib: 72 passed
  • cargo fmt --all -- --check: passed
  • cargo clippy -p skippy-cache --all-targets -- -D warnings: failed as above
  • cargo clippy -p skippy-cache --all-targets --features cachegen-spike -- -D warnings: failed as above
  • release 4096x128 spike, 20 iterations: PASS on cubecl-cpu and wgpu/Metal, 0/524,288 symbol mismatches, 0/524,288 value mismatches, ratio 0.075; reported live peaks 6,291,464 B CPU / 6,291,712 B Metal; H2D 2,097,160 B and D2H 4,194,304 B

Keep the PR draft and leave the stack unsynced for now. Once this boundary is made operationally safe and clippy is green, sync the dependent stack once against current main to clear the known slice-catalog planner failure.

…orced symmetrically (#1652)

Third re-review blocker plus the Rust 1.98 clippy failure:

- MAX_DECODED_VALUES drops from 2^30 to 2^24 (16,777,216 values =
  32 MiB decoded f16). Decode working set is ~7 bytes per value
  (symbols + f32 materialization + f16 output), so the worst case a
  header can command falls from ~7 GiB to ~112 MiB; the measured
  4096x128 tile sits 32x under the ceiling. Raising it is a
  format-version decision, not a per-parse judgment call.
- The ceiling is now one shared admission check (checked_tile_len)
  routed through both parse_container and encode_f16_segment, so
  encode and decode are symmetric by construction. The encode guard
  runs before the f32 materialization: an over-ceiling input is
  refused without doing the work it names.
- Boundary regressions: a crafted 65536x256 header is admitted at
  exactly MAX_DECODED_VALUES; 16777217 == 97 * 172961 (a real shape,
  not a truncation artifact) is refused one value later by
  decoded_value_count, container_calibration, and decode_f16_segment,
  naming the ceiling. The encode guard is probed at MAX and MAX + 1
  without allocating the tile, plus an end-to-end refusal of a real
  33.5 MB input.
- Clippy on unpinned Rust 1.98.0: the bomb test's chunks_exact_mut
  loop becomes a byte fill (LE u32::MAX is 0xFF bytes) and the new
  helper writes its histogram the way the encoder does.
- Console-print ratchet regenerated: allowlist entries had drifted
  under the stack (l3/tests.rs print moved 605 -> 608 in the
  manifest-v4 commit; the cachegen_cubecl_spike example landed in
  round 1 without a ratchet run). Mechanical only - the print moved,
  and the example's existing prints were never allowlisted.

Validation on this head: fmt clean; skippy-cache 165 passed /
2 ignored; skippy-protocol 72 passed; clippy -D warnings
--all-targets with and without cachegen-spike; release spike PASS on
CPU and Metal (0/524288 mismatches, ratio 0.075, live peak
6291464/6291712 bytes); no-console-print and repo-consistency pass.
@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Fix head for the remaining allocation-safety blocker and the Rust 1.98 clippy failure. Exact head: 34dd77e9c2378a654e5a3f936330bf3cd8aa9756 (one commit on 03d1fbd2a). Stack still untouched — no rebase, no main sync.

1. Working-set ceiling, not a token-count ceilingMAX_DECODED_VALUES drops from 2^30 to 2^24 (16,777,216 values = 32 MiB decoded f16). Decode holds ~7 bytes per value (symbols + f32 materialization + f16 output), so the worst case a header can command falls from ~7 GiB to ~112 MiB. The measured 4096×128 tile sits 32× under the ceiling; raising it is a format-version decision, not a per-parse judgment call. Documented that a later slice additionally binds decode to the manifest's per-segment decoded_len — your stronger option, not silently dropped.

2. Symmetric by construction — the ceiling is one shared admission check, checked_tile_len, routed through both parse_container and encode_f16_segment. The encode guard runs before the f32 materialization, so an over-ceiling input is refused without doing the work it names.

3. Boundary tested at both edges — a crafted header at exactly 65,536×256 is admitted (decoded_value_count == 16,777,216); 16,777,217 = 97 × 172,961 (a real shape, not a truncation artifact) is refused one value later by all three consumers, naming the ceiling. The encode guard is probed directly at MAX and MAX + 1 without allocating the tile, plus an end-to-end refusal of a real 33.5 MB input.

4. Clippy 1.98 — the bomb test's chunks_exact_mut loop is now a byte fill (LE u32::MAX is all-0xFF), and the new header helper writes its histogram the way the encoder does; no fixed-size-chunk patterns left in the file.

5. One honest find on top — running the repo's full pre-push ritual for the first time in this stack surfaced console-print ratchet drift: the l3/tests.rs print moved 605→608 in the manifest-v4 commit, and the round-1 spike example's prints were never allowlisted. Regenerated mechanically (a line move plus entries for prints that already existed in 106c31752); no count increases, no new print sites.

Validation on 34dd77e9c, tree clean at HEAD: fmt clean; skippy-cache 165 passed / 2 ignored; skippy-protocol 72 passed; clippy -D warnings --all-targets with and without cachegen-spike; no-console-print + repo-consistency pass; release spike PASS CPU+Metal (0/524,288 mismatches, ratio 0.075, live peaks 6,291,464 / 6,291,712 B). Planner stays red on the known catalog drift until the one-time post-merge sync, per your sequencing.

@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Exact-head re-review of 34dd77e9c2378a654e5a3f936330bf3cd8aa9756 is complete.

The remaining allocation-safety blocker is cleared. checked_tile_len is now the single 2^24-value admission check used by both encode_f16_segment and parse_container; decode is bounded to about 112 MiB of symbols + f32 materialization + f16 output, and encode rejects an over-ceiling input before its f32 allocation. The boundary tests admit exactly 65,536x256 values, reject exactly one more through all public probes, and exercise the real 33.5 MB encode refusal. The Rust 1.98 chunks_exact_mut lint is gone.

I independently reproduced on the exact head:

  • cargo test -p skippy-cache --lib: 165 passed, 2 ignored
  • cargo test -p skippy-protocol --lib: 72 passed
  • cargo fmt --all --check: clean
  • Clippy -D warnings --all-targets for skippy-cache, with and without cachegen-spike: clean
  • just no-console-print: pass; the allowlist diff is limited to the nine pre-existing spike output sites and the moved L3 test print
  • release 4096x128 spike on CPU + Metal: PASS, ratio 0.075, 0/524,288 symbol and value mismatches, live peaks 6,291,464 / 6,291,712 bytes

No further code blocker found in this fix. The PR should remain draft for the one-time dependent-stack sync: current exact-head checks still fail only at planning with source slice catalog differs from the protected planner catalog (SOURCE_SHA=34dd77e9c, BASE_SHA=7247a397a). #1736 is still open, so sync #1736, #1750, and #1752 in dependency order against current main, then let the complete CI matrix run before advancing the draft.

@i386

i386 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Post-sync review of aa533c0d45e1b8af025ee9af9887de68a7f02baf is complete.

The sync itself is sound:

  • Remote heads match the reported e63f86ca6 / a0876ee06 / aa533c0d4 chain.
  • 7247a397a, 5effeffb1, and reviewed CacheGen head 34dd77e9c remain ancestors of their corresponding synced heads.
  • The only manual conflict resolution is in the feat(skippy): add durable local KV cache and restart restore #1736 merge. It correctly keeps the stack-added fs2, libc, windows-sys, and host-runtime skippy-cache dependency while taking main's 0.76.0 dependency versions.
  • ci/slices.yml and ci/ownership.yml at aa533c0d4 are byte-identical to current main d4ffbbacd.
  • Independent exact-head validation: cargo metadata --no-deps, 165/165 skippy-cache tests (2 ignored), 72/72 skippy-protocol tests, fmt, both skippy-cache Clippy configurations with -D warnings, and just no-console-print all pass. The tree is clean at the exact head.

The current GitHub result is not yet the full post-sync matrix. All five workflow runs are terminal and their planners pass, but the PR is still draft, so the plan selected profile=pr-draft with required_slices=[]. The Quality, Linux, macOS, Windows, and Website execution jobs were all skipped; only their stable summaries are green.

Therefore the merge-sync review is cleared, but CI execution evidence is still outstanding. Marking the PR ready should trigger the pr-ready matrix; alternatively an explicitly authorized manual-full run can supply that evidence while the PR remains draft. Do not describe the current planning-only result as a green full matrix.

@i386
i386 marked this pull request as ready for review September 11, 2026 03:23
@github-actions
github-actions Bot requested a review from ndizazzo September 11, 2026 03:23
@i386
i386 added this pull request to stack #1790 September 11, 2026 05:52
@i386

i386 commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #1816: #1816

This PR's head is an ancestor of the consolidated branch, so its commits and behavior remain in the combined review. Please continue review on #1816.

@i386 i386 closed this Sep 12, 2026
i386 pushed a commit that referenced this pull request Sep 12, 2026
…s and fixture (#1652)

Address the five PR #1752 review blockers on top of 106c317:

- CGv1 histogram entries widen from u16 to u32 counts. A 4096x128 tile
  is 524,288 symbols and cannot be encoded under the old cap; CGv1 now
  carries every tile the codec accepts. Regression: end-to-end
  encode->container->decode at exactly the measured 4096x128 shape.
- The container parser validates sum(histogram) == rows*dims before any
  table is built, and histogram_to_freqs enforces the same caller
  contract, so a forged or corrupt header can no longer drive
  unbounded normalization repair; the convergence loop carries an
  explicit iteration bound. Regressions: corrupt + off-by-one
  histogram-total rejection at both layers.
- Spike timings are now synchronized: every timed stage ends with
  client.sync() inside the timer (warm dispatch is total/iterations,
  not unsynchronized enqueue), the client parameter is used, cold JIT
  is the true first synchronized launch of the kernel specialization,
  and equality is bitwise (to_bits) with mismatch counts reported.
  Release-run numbers on M2 Max, 4096x128: cold 38/8 ms (CPU/Metal
  quantize+delta), warm 0.58/4.6 ms; ratio 0.075; 0/524288 mismatches
  on both backends.
- An independent golden rANS stream is committed
  (src/cachegen/fixtures/ryg_rans_golden.bin) with a deterministic
  generator test (--ignored) and a decoder test that reproduces the
  declared symbol sequence from the frozen artifact.
- cubecl is pinned to exactly =0.10.0.
i386 pushed a commit that referenced this pull request Sep 12, 2026
…zed re-run (#1652)

The backend plan's six-metric table now reflects the PR #1752 fix head:
synchronized stages (client.sync() inside the timer), true first-launch
cold JIT, bitwise equality with mismatch counts, and the convergence
sweep across iteration counts. Notes why per-process first launch is
the true cold path in cubecl 0.10.0 (the only on-disk kernel cache,
SPIR-V, is Vulkan-only and not enabled here).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skippy-kv Work coordinated in Buzz #skippy-kv

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants