Skip to content

refactor(csheap)!: ASAPv1 only — drop the portable format, add a golden - #169

Closed
GordonYuanyc wants to merge 2 commits into
mainfrom
asapv1-only/csheap
Closed

GordonYuanyc wants to merge 2 commits into
mainfrom
asapv1-only/csheap

Conversation

@GordonYuanyc

Copy link
Copy Markdown
Collaborator

Part of the ASAPv1-only series: one PR per sketch group, each off main. This one removes the portable Count-Sketch-with-heap format; ASAPv1 (0x0a 0x00) is the only CSHeap format left, and it gets its first golden.

Removed

  • message_pack_format::portable::countsketch_topk: CountSketchWithHeap, CsHeapItem, WireHeapItem, SketchlibCSHeap, the sketchlib_cs_heap_* / new_sketchlib_cs_heap / matrix_from_sketchlib_cs_heap / heap_to_wire helpers, CountSketchInnerWire, CountSketchWithHeapWire; crate-root re-exports CountSketchWithHeap, CsHeapItem.
  • CSHeap had no proto message and no native shim (native/countsketch_topk.rs is CountL2HH's and goes with that PR).

Added

  • Golden csheap_i64_regular_2x4_strkeys (pushed to sketchlib-golden-bytes; submodule bumped): the Count Sketch golden's signed 2x4 matrix plus a k=5 string-keyed heap {alpha: 2^32, beta: 127, delta: 127, gamma: -33}, set directly, never hashed. It pins uint64 and negative heap counts and the tie-by-key order. The test checks both directions and that a plain Count decoder refuses it.
  • The ASAPv1 docs no longer state a fixture count; they defer to the golden README's Coverage list (avoids conflicts between the sibling PRs).

Downstream note

  • ASAPQuery-backend reads the collector's CS-with-heap msgpack frame through CountMinSketchWithHeap::from_msgpack (the two portable wire shapes were identical). Until the backend moves to ASAPv1, that frame is read with Count-Min semantics (min over signed rows) rather than refused.
  • The CS-with-heap heap-delta frame has no ASAPv1 counterpart; no delta codec is removed here.

Verification (rustc 1.99)

cargo fmt --check; clippy -D warnings for all four CI feature sets; RUSTDOCFLAGS="-D warnings" cargo doc; gen-proto drift clean; filtered tests (asapv1_golden, --lib -- countsketch_topk, e2e -- composition wire topk, pbt -- topk_wrappers). The reviewer hand-decoded the new golden byte by byte against spec §3.10. Implemented and reviewed by separate agents; review findings are addressed in the second commit.

🤖 Generated with Claude Code

GordonYuanyc and others added 2 commits October 2, 2026 00:50
CSHeap serializes through ASAPv1 only. Removes CountSketchWithHeap,
CsHeapItem and the rest of message_pack_format::portable::countsketch_topk,
with the e2e test and docs rows that covered them, and adds the CSHeap
golden fixture csheap_i64_regular_2x4_strkeys (kind_id 0x0a 0x00).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@GordonYuanyc

Copy link
Copy Markdown
Collaborator Author

Outdated. ASAPv1 encodings and goldens landed in #232 and #235 against the current spec; removing the legacy formats is redone on current main in #206 and #207.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant