From 6c3a9e31bf08227cf7a1b0034bd9814f02bdf557 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 12:31:37 +0000 Subject: [PATCH] fix(bgz-tensor): reject HhtlF32Tensor palette sizes >= 256 (codex #184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per codex P1 comment on #184: HhtlF32Entry.twig is u8, so valid centroid IDs are 0..=255. Before this fix, encode() accepted any k and assign_nearest_f32 silently wrapped ci as u8 — passing k=300 (say) would assign centroid-300 as twig-44 and reconstruct the wrong row. This was actively dangerous because the next-session plan (PR #184 thread) explicitly proposed k=1024 or 2048 centroids as the quality fallback. Fix: - New `pub const MAX_PALETTE_K: usize = 256` with clear docstring - Both `encode` and `encode_with_leaf` now assert: k > 0 k <= MAX_PALETTE_K with explicit panic messages naming the u8 twig limit Larger palettes need a codec with a wider twig-index (u16 would lift the cap to 65536, but changes the wire format). That's a separate PR if/when the quality probe shows k=512+ earns its keep. Tests (4 new, all pass + 5 existing): encode_rejects_zero_k (#[should_panic = "k > 0"]) encode_rejects_k_above_256 (#[should_panic = "u8 twig limit"]) encode_with_leaf_rejects_k_above_256 (same) encode_accepts_k_at_max_palette (k=256 must still succeed) Refs: - PR #184 codex P1 comment ("Reject palette sizes that exceed 255 centroids") - Follow-up to merged PRs #180/#181/#182/#183/#184 https://claude.ai/code/session_01NYGrxVopyszZYgLBxe4hgj --- crates/bgz-tensor/src/hhtl_f32.rs | 59 +++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/crates/bgz-tensor/src/hhtl_f32.rs b/crates/bgz-tensor/src/hhtl_f32.rs index 1101d12b..01bf2cff 100644 --- a/crates/bgz-tensor/src/hhtl_f32.rs +++ b/crates/bgz-tensor/src/hhtl_f32.rs @@ -40,6 +40,15 @@ impl HhtlF32Entry { pub fn from_le_bytes(b: &[u8; 1]) -> Self { Self { twig: b[0] } } } +/// Maximum palette size supported by HhtlF32Tensor (constrained by u8 twig index). +/// +/// `HhtlF32Entry.twig` is u8, so palette centroid IDs above 255 would silently +/// wrap at encode time (e.g. 300 → 44) and map rows to the wrong centroid. +/// `encode` / `encode_with_leaf` enforce `0 < k <= MAX_PALETTE_K`; callers that +/// need a larger palette must use a codec with a wider twig index (future work, +/// separate wire format). +pub const MAX_PALETTE_K: usize = 256; + /// Reconstruction-grade HHTL-like tensor: f32 palette + SlotL residual. #[derive(Clone, Debug)] pub struct HhtlF32Tensor { @@ -120,7 +129,16 @@ fn assign_nearest_f32(rows: &[Vec], centroids: &[Vec]) -> Vec { impl HhtlF32Tensor { /// Encode without SlotL (argmax-regime path, 1 byte/row + palette). + /// + /// # Panics + /// `k` must satisfy `0 < k <= MAX_PALETTE_K` (256). Larger palettes need + /// a codec with a wider twig-index; using `k > 256` here would silently + /// wrap indices via `as u8` and corrupt row assignments. pub fn encode(role: &str, rows_f32: &[Vec], k: usize) -> Self { + assert!(k > 0, "HhtlF32Tensor::encode requires k > 0, got {}", k); + assert!(k <= MAX_PALETTE_K, + "HhtlF32Tensor::encode requires k <= {} (u8 twig limit), got {}", + MAX_PALETTE_K, k); let n_rows = rows_f32.len(); let n_cols = if n_rows > 0 { rows_f32[0].len() } else { 0 }; @@ -143,12 +161,19 @@ impl HhtlF32Tensor { /// Encode with SlotL leaf residual (index-regime path, 9 bytes/row). /// `svd_basis` should have `SLOT_L_LANES` components built from /// representative rows of the group (via `SvdBasis::build`). + /// + /// # Panics + /// Same `k` bounds as [`encode`]: `0 < k <= MAX_PALETTE_K` (256). pub fn encode_with_leaf( role: &str, rows_f32: &[Vec], k: usize, svd_basis: &SvdBasis, ) -> Self { + assert!(k > 0, "HhtlF32Tensor::encode_with_leaf requires k > 0, got {}", k); + assert!(k <= MAX_PALETTE_K, + "HhtlF32Tensor::encode_with_leaf requires k <= {} (u8 twig limit), got {}", + MAX_PALETTE_K, k); let mut t = Self::encode(role, rows_f32, k); if rows_f32.is_empty() { return t; } @@ -330,6 +355,40 @@ mod tests { assert_eq!(HhtlF32Entry::from_le_bytes(&b), e); } + #[test] + #[should_panic(expected = "k > 0")] + fn encode_rejects_zero_k() { + let rows = low_rank_rows(4, 8, 0); + let _ = HhtlF32Tensor::encode("zero_k", &rows, 0); + } + + #[test] + #[should_panic(expected = "u8 twig limit")] + fn encode_rejects_k_above_256() { + // Codex P1 regression: u8 twig index can only represent 0..=255, + // so k > 256 would silently wrap `ci as u8`. Must panic loudly instead. + // Use a bigger row set so clam_furthest_point_f32 doesn't cap k first. + let rows = low_rank_rows(512, 16, 0); + let _ = HhtlF32Tensor::encode("oversize", &rows, 300); + } + + #[test] + #[should_panic(expected = "u8 twig limit")] + fn encode_with_leaf_rejects_k_above_256() { + let rows = low_rank_rows(512, 16, 0); + let basis = SvdBasis::build("oversize", &rows, SLOT_L_LANES); + let _ = HhtlF32Tensor::encode_with_leaf("oversize_leaf", &rows, 300, &basis); + } + + #[test] + fn encode_accepts_k_at_max_palette() { + // k == MAX_PALETTE_K (256) is the largest legal value and must succeed. + let rows = low_rank_rows(300, 16, 0); // Need > 256 rows so all 256 centroid slots fill + let t = HhtlF32Tensor::encode("max_k", &rows, MAX_PALETTE_K); + assert!(t.palette_f32.len() <= MAX_PALETTE_K); + assert_eq!(t.entries.len(), 300); + } + #[test] fn storage_accounting_is_additive() { let rows = low_rank_rows(16, 64, 0xDEAD);