feat(wire): ASAPv1 self-describing wire format (envelope + HLL + Count-Min)#65
Merged
Conversation
GordonYuanyc
added a commit
that referenced
this pull request
Jul 10, 2026
Review findings on PR #65: - CMS deserialize panicked on a crafted zero-dimension payload ([rows,0,[]]) via Vector2D::from_fn -> 0.ilog2(); now rejected with an Err before from_fn - portable HLL decode used 1<<precision (shift-overflow panic on crafted precision); now checked_shl -> Err - add negative tests: CMS zero-dim rejection, CMS unknown-key rejection, HLL precision cross-rejection (P12 bytes vs P14 decoder), HIP kind_id rejected by a Classic decoder Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GordonYuanyc
force-pushed
the
feat/asapv1-wire-envelope
branch
from
July 11, 2026 05:19
1e8de0b to
1e8a4ba
Compare
Three-layer self-describing sketch wire format: sketch-agnostic envelope (magic|version|kind_id|metadata_len|payload_len), a msgpack-map metadata descriptor (hash spec + structural params), and a per-sketch positional msgpack-array payload. Records the resolved design decisions (kind_id = algorithm identity, counter type/mode in metadata, seed_list optional, no payload version) and the byte-level encoding rules for Go parity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the sketch-agnostic envelope framing + hash-profile constants into message_pack_format::envelope (encode/split, with the new payload_len field). HLL serialization moves onto it: kind_id carries the variant, metadata is a msgpack map (precision as a structural param, seed_list omitted for the registered profile), and the payload is a positional msgpack array with registers as msgpack bin (serde_bytes) to match Go's []byte. The deprecated portable HllSketch codec reuses the same shared primitives so it stays byte-identical with the native path (guarded by a native<->portable test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Count-Min serializes to the shared ASAPv1 envelope: one kind_id (0x02 0x00), with counter type (i64/f64) and column-derivation mode (fast/regular) carried in the metadata via the CmsWireCounter / CmsWireMode marker traits. Payload is a positional [rows, cols, counts] msgpack array. Wire serialization is restricted to the canonical i64/f64 configs (the blanket serialize-anything impl is removed); exotic counters convert to a wire type first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- envelope: guard kind_id length (u8::try_from) and use checked arithmetic for the metadata/payload offsets so crafted lengths fail closed instead of panicking - metadata: #[serde(deny_unknown_fields)] on HllMetadata/CmsMetadata so an unexpected key (e.g. an inlined seed_list) is rejected, not silently dropped (v1 accepts only the standard registered profile); add a fail-closed test - portable HLL decode: enforce registers.len() == 2^precision - drop the unused Default bound from the CMS wire impls - doc: pin the msgpack integer family/width rule (non-negative -> uint minimal width) — the main cross-language trap before Go goldens — and record the deny_unknown_fields / registered-profile-only behavior Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Carry the full 20-seed list in HLL/CMS metadata (not just hash_profile_id), so a consumer can read the exact seeds and algorithm straight from the bytes with no registry. ~130 bytes/sketch; resolving seeds from the profile id alone is left as a v2 space optimization. Doc + fail-closed unknown-key test updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apply `cargo clippy --fix` for the uninlined_format_args lint across all
targets (lib + tests) so the CI clippy step passes with
`--all-targets --all-features -D warnings`. Purely mechanical
`format!("{}", x)` -> `format!("{x}")` rewrites; no logic changes.
Also update examples/serialize_deserialize.rs to use an i64 CMS counter,
since the CMS wire format now constrains counters to CmsWireCounter
(i64/f64); this restores compilation of the --all-targets build.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HLL and Count-Min `serialize_to_bytes` were generic over the hasher but hardcoded the standard ProjectASAP profile into the metadata, so a sketch built with a custom hasher would serialize metadata that lied about how it was hashed. Introduce a `HashProfile` trait alongside `SketchHasher` and derive the wire metadata from it: - New `HashProfile` trait in `common/hash.rs`, implemented for `DefaultXxHasher` with the exact standard-profile values (now the single source of truth; the envelope `HASH_*` string constants are removed). - HLL: `standard_hll_metadata` is now `hll_metadata::<DefaultXxHasher>`; the generic wire methods build/validate `hll_metadata::<H>` and are bounded on `H: HashProfile`. HIP stays on `DefaultXxHasher`. Portable path unchanged. - Count-Min: `cms_metadata::<H>` reads the profile (incl. `MATRIX_SEED_INDEX`); wire methods and the native `MessagePackCodec` impl gain `H: HashProfile`. Unprofiled hashers become impossible to serialize (compile error → fail closed). The standard-profile output is byte-identical: all existing round-trips, `native_and_portable_hll_bytes_match`, and the Go-parity goldens still pass. Adds custom-hasher (`AltHasher`) round-trip tests for HLL and CMS verifying distinct, self-describing bytes and that a standard-profile decode rejects custom-profile bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…handling The ASAPv1 hash-spec metadata is now derived from the hasher's HashProfile (hll_metadata::<H> / cms_metadata::<H>) rather than hardcoded. Document that the field values are sourced from the hasher (standard profile = DefaultXxHasher), that custom hash profiles are supported and self-describing, and that the format fails closed on both ends (serialize requires H: HashProfile; decode validates against the target type's profile). Add a Custom hash profiles subsection, clarify Section 2 validation, add a Q-PROFILE decision, and note Go-side handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nfigs) Add "Section 4 — Wire coverage" to docs/asapv1_wire_format.md making the coverage decision explicit: which in-memory HLL and Count-Min configs are wire-eligible, and the concrete function/trait to implement (or conversion to do) for the ones that are not. Includes an HLL coverage table (all variants, precisions, and any H: HashProfile), a Count-Min coverage table, an actionable "If you want X, do Y" table, and the rationale for keeping the wire a small fixed set. Renumber the former Section 4 (Wire encoding rules) to Section 5 and update the two cross-references in Decisions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ic_ids Align the Section 1 kind_id registry family bytes to the ids already committed in sketchlib-go's wire/asapmsgpack/magic_ids.go, correcting the earlier speculative KLL/DDSketch/KMV/CountSketch assignments that conflicted with Go. Extend the registry with new family bytes (0x0a+) for the remaining sketches inventoried in docs/apis.md, add a Status column, and add mapping notes for the CMSHeap/CSHeap, Hydra, and SetAggregator/DeltaResult cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI's stable clippy is newer than the local toolchain and flags `redundant reference in println! argument` at countsketch.rs:470 (`row_slice` already returns a slice). Remove the extra & (and inline the format arg). Was the only newer-clippy error under -D warnings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… notes - Mermaid erDiagram (Structure/entity view) contrasting ASAPv1's self-contained one-to-one shape with OTel-Arrow's normalized one-to-many fan-out - placeholder payload subsections for every registry kind_id (§3.3–§3.22), with per-sketch design notes - registry: KLL-dynamic row; 'kind_id = algorithm level' framing note Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review findings on PR #65: - CMS deserialize panicked on a crafted zero-dimension payload ([rows,0,[]]) via Vector2D::from_fn -> 0.ilog2(); now rejected with an Err before from_fn - portable HLL decode used 1<<precision (shift-overflow panic on crafted precision); now checked_shl -> Err - add negative tests: CMS zero-dim rejection, CMS unknown-key rejection, HLL precision cross-rejection (P12 bytes vs P14 decoder), HIP kind_id rejected by a Classic decoder Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add shared ASAPv1 golden byte-vectors that machine-prove the Rust and Go wire encodings are byte-identical, closing the "no cross-language goldens" gap both code reviews flagged. Each fixture is built from a fixed, KNOWN raw sketch state (register bytes / matrix values set directly, never hashed), so the golden tests the wire encoding in isolation from the hash functions. Fixtures cover the implemented wire configs: HLL Classic/Ertl-MLE/HIP at P12, Count-Min i64/RegularPath and f64/FastPath. The .hex goldens (asapv1_golden/) are authored by the Rust reference encoder and are checked in byte-identically to sketchlib-go/asapv1_golden/. tests/asapv1_golden.rs builds each fixture from known state, serializes, and asserts == golden, plus asserts deserialize(golden) round-trips. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…+ wire.rs (serialization)
Count-Min's matrix dimensions are configuration that shapes the payload (like HLL's precision), so per the spec's config→metadata rule they now live in the descriptor metadata instead of the payload. - CmsMetadata gains `rows: u32` / `cols: u32` as structural params. Canonical field order (the wire contract Go must mirror): metadata_version, hash_profile_id, hash_algorithm, seed_derivation, input_encoding, seed_list, matrix_seed_index, rows, cols, counter_type, mode. - cms_metadata::<H>(..) now takes rows/cols and populates them. - CmsPayload drops rows/cols; the payload is now a 1-element positional array `[counts]` (mirroring HLL Classic's `[registers]`). - decode reads rows/cols from the validated metadata; the zero-dimension guard and the counts.len() == rows*cols check are unchanged. - Regenerated the two CMS goldens (cms_i64_regular_2x3.hex, cms_f64_fast_2x3.hex). The three HLL goldens are unchanged. - Updated docs/asapv1_wire_format.md (§2, §3.2, §5, Decisions) and added a "Code organization" note documenting the mod/wire split. NOTE: the Go copies of the two CMS goldens now differ from Rust and will stay mismatched until Go PR #70 makes the matching rows/cols → metadata change. That is expected and a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- §2 Fields: add an overview table (metadata = Hash spec + Structural params, with each group's role and member fields) above the two detail tables - §2 Encoding: replace the loose 'optional / omit the key' wording (and the incorrect 'unknown keys are skippable', which contradicts deny_unknown_fields) with the accurate model: each sketch has its own fixed metadata schema; every field is required within it; missing/extra keys fail closed - mermaid METADATA entity: annotate the hash-spec vs structural-params grouping Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add §5 Implementation detail (move Validation + the exotic-sketch conversion recipe out of §2/§3 into it). Reflow the whole doc to one-sentence-per-line. Tighten Status to bullets, lead with Layering, drop the redundant three-way overview, trim the registry history, and fix a §3.2 inaccuracy (both FastPath/RegularPath serialize directly; a mode is only 'converted' to change it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add "What this is" intro and "Which parts to read" reading guide - Add a "Terms" section (envelope, metadata, payload, kind_id, hash profile, seed_list, wire-eligible) so terms are defined before use - Clarify the Layering "who authors each part" paragraph (envelope and metadata are authored once but shipped with every sketch) - Replace the structure diagram with an erDiagram: SerializedSketch as an ordered envelope/metadata/payload container, payload fanning out per kind - Redraw the layering text box in ASCII; drop redundant caption and the weak "by goal" reading list Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Replace all non-ASCII characters with ASCII equivalents (em/en dashes, section signs, arrows, checkmarks, sigma/superscript, <=, etc.) - Rephrase "rather than" and "X, not Y" constructions as positive statements - Drop "2-byte" from the kind_id prose and mark [family, variant] as today's layout, since kind_id is variable-length (kind_id_len is a u8) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…table - Cut redundant restatements of seed_list-inlined, fail-closed, and golden-vectors-lock across Section 2/4/5, Cross-language, and Decisions (keep one canonical statement of each) - Collapse the 20 not-yet-designed payload subsections (3.3-3.22) into a single reference table - Remove the deferred "Wire coverage" HTML-commented block Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GordonYuanyc
force-pushed
the
feat/asapv1-wire-envelope
branch
from
July 19, 2026 03:28
166e23a to
130e4e1
Compare
5 tasks
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
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.
What & why
Introduces the ASAPv1 self-describing wire format for serialized sketches in Rust: every serialized binary carries the hash metadata + structural params an out-of-process (and cross-language) consumer needs, so it is self-describing end to end.
Deliverables
1. Design doc:
docs/asapv1_wire_format.mdThe byte-level spec, written for a first-time reader. It opens with a "What this is" intro, a section-linked "Which parts to read" guide, and a Terms glossary (envelope / metadata / payload / kind / hash profile / seed_list / wire-eligible), then lays out the three layers:
magic | version | kind_id_len | kind_id | metadata_len | payload_len | metadata | payload.Structure is shown as an entity-relationship diagram (a
SerializedSketchis one ordered envelope, metadata, payload, with the payload fanning out per kind). Thekind_idregistry is the single source of truth, mirrored verbatim withsketchlib-go; sketches not yet designed are parked in a compact TBD table. Prose is plain ASCII throughout.Resolved decisions:
kind_id= algorithm identity (structural params, i.e. counter type / precision / mode, live in metadata and are read before the payload);seed_listis inlined so the bytes self-describe the hash with no registry; no payload version field (a new encoding gets a newkind_id, never recycled); byte-level int family/width rules pinned for Go parity.2. Shared envelope module + per-sketch
wire.rssplitmessage_pack_format::envelope(encode/splitframing): one sketch-agnostic module every sketch calls into, replacing the envelope logic that was previously duplicated per sketch.src/sketches/<sketch>.rs(algorithm) +src/sketches/<sketch>/wire.rs(serialization child submodule, reads the struct's private fields directly, no visibility widened). Done for HLL and Count-Min.3.
HashProfiletrait: metadata derived, not hardcodedsrc/common/hash.rs: the hash-spec metadata (hash_profile_id, algorithm, seeds, seed indices) is read off the hasher'sHashProfile, so the bytes truthfully describe how the sketch was hashed.serialize_to_bytesis bounded onH: HashProfile, so an unprofiled hasher cannot serialize (compile error, by design); a custom profile serializes truthfully and self-describes (inlinedseed_list). Decode validates the read metadata against the target type's profile, so profile-A bytes won't decode into a profile-B sketch.4. HLL serialization
0x01 0x01/ Ertl-MLE0x01 0x02/ HIP0x01 0x03), all precisions, anyH: HashProfile.seed_list+canonical_seed_index+precision); payload = positional array withregistersas msgpack bin (matches Go's[]byte); HIP adds three running f64.5. Count-Min serialization
kind_id(0x02 0x00); counter type (i64/f64), mode (fast/regular), and rows/cols all carried in metadata (via theCmsWireCounter/CmsWireModemarker traits + the config-to-metadata rule).[counts]. Wire-eligible configs are restricted toVector2D<i64|f64>with FastPath/RegularPath (the blanket serialize-anything impl is removed; exotic counters convert to a wire type first).6. Cross-language golden byte-vectors
asapv1_golden/*.hex(HLL Classic/ErtlMLE/HIP P12, CMS i64/regular, CMS f64/fast) +tests/asapv1_golden.rs. Byte-identical withsketchlib-go, so cross-language parity is machine-proven.Hardening (from adversarial self-review)
#[serde(deny_unknown_fields)]on both metadata structs, so an unexpected key is rejected instead of silently dropped (fail closed).kind_idlength guarded, metadata/payload offsets use checked arithmetic (no panic on crafted lengths).registers.len() == 2^precision.Testing
cargo test(lib + integration) green; CIcargo clippy --workspace --all-targets --all-features --locked -- -D warningsclean.Related / follow-ups
message_pack_format.md) with ASAPv1 (stacked on this branch, merge after).portable/nativeshims is a later cleanup; the goldens now supersede thenative == portabledrift guard.🤖 Generated with Claude Code