From 056269629fbe13000319dd23f4e63fdd0494835a Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 12:29:36 +0900 Subject: [PATCH 1/8] =?UTF-8?q?docs(keyviz):=20revise=20=C2=A73.2=20?= =?UTF-8?q?=E2=80=94=20sub-divide=20unbounded-End=20routes=20over=20[subSt?= =?UTF-8?q?art,=20MaxUint64]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses the first-round Open-Q1 decision ("single row until split"). That kept an open-End route at one row, which made the feature inert for the single-route cluster and every cluster's tail route — exactly where hot-key visibility is most wanted (observed on a live 5-node deploy: one route route:1 = [nil,nil), so no sub-ranges ever appeared). The original blocker was an overflow bug in the draft fallback (1<<(8*W) wraps uint64 to 0 -> divide-by-zero). Using MaxUint64 as the effective top makes the span a valid uint64, so the open end can be divided into K by the leading W-byte window; the last sub-bucket keeps the unbounded End. subBucketIndex skips the upper-edge clamp for the unbounded sentinel (subHi == nil). Caveat documented: prefixLen 0 means leading-byte-coarse resolution; tight route bounds still give finer intra-prefix detail. Doc-first; implementation + deploy wiring follow. --- ...05_25_proposed_keyviz_subrange_sampling.md | 55 ++++++++++++------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md index ec0fbcd52..47c9f12b8 100644 --- a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md +++ b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md @@ -152,20 +152,32 @@ lives in the bytes *after* that prefix. So: - **`Start == nil`** (first route / open low end): treat as all-`0x00`, i.e. `subStart = 0`. Natural — `0x00…` is the true low end of the byte-order key space. -- **`End == nil`** (last route / open high end): the route has **no - finite span to divide**, so it is kept as a **single sub-bucket** - (`K_effective = 1`) until a route split gives it a finite `End`. - This is the decision adopted from review (Open Q1): the earlier - "high-order bits over a `W`-byte window" fallback (a) had no finite - `subSpan` to divide by — `1 << (8*W)` with `W = 8` overflows - `uint64` to `0` and would divide-by-zero on the hot path — and - (b) introduced an observable discontinuity (the tail's sub-ranges - would shift the moment its heuristic span changed). A split is a - discrete event operators already understand; the hotspot-shard-split - machinery (`docs/design/2026_02_18_partial_hotspot_shard_split.md`) - is exactly what gives a hot tail a finite `End`, at which point the - route re-registers (§4.2) and gains real sub-buckets. No overflow, - no heuristic, no surprise. +- **`End == nil`** (last route / open high end): **sub-divide over + `[subStart, MaxUint64]`** (revised — see the note below). There is no + `End` window, so set `subPrefixLen = 0`, `subStart = + windowUint64(start)`, `subEnd = math.MaxUint64`, `subSpan = + MaxUint64 - subStart`, and divide that finite span into `K`. Keys + bucket by their leading `W`-byte window across the open high end; the + **last** sub-bucket keeps the route's unbounded `End` (`nil`). The + `subBucketIndex` upper edge clamp is skipped for this slot (its + `subHi` snapshot is `nil` — the unbounded sentinel), relying on the + window math + the `w >= subEnd` guard instead. + + *Why this reverses Open Q1.* The first review chose "single row until + split" because the original fallback used `subSpan = 1 << (8*W)`, + which with `W = 8` overflows `uint64` to `0` and divides by zero. + Using `MaxUint64` (= `2^64 − 1`) as the effective top makes the span + a valid `uint64` with no overflow, so the original objection no + longer applies. The practical driver: the **single-route cluster and + every cluster's tail route are unbounded**, and that is exactly where + an operator most wants hot-key visibility — keeping them at one row + made the feature inert for the most common topology. The "split + changes the heuristic" discontinuity is accepted: a split is a + discrete event that already reshuffles routes. **Caveat:** with + `subPrefixLen = 0` the resolution is leading-`W`-byte-coarse, so keys + sharing a long prefix (e.g. `user:0001…`/`user:0002…`) still collapse + into one bucket — finite, tight route bounds remain the way to get + fine intra-prefix resolution. - **`subEnd <= subStart`** (the `W`-byte window captures no difference — e.g. `Start`/`End` differ only beyond `subPrefixLen + W`): the route is likewise a **single bucket** (`K_effective = 1`, `subSpan` left 0 @@ -490,11 +502,16 @@ The four questions opened in the first draft converged in review (Codex, Gemini, Claude) and are now decided; recorded here so the implementation has no ambiguity. -1. **Unbounded-tail bucketing (§3.2): single row until split.** The - last route's open `End` stays one row until a route split gives it a - finite `End`. This is both honest (no heuristic discontinuity when - the tail's effective span changes) and what kills the - `1 << (8*W)` overflow / divide-by-zero entirely. Adopted. +1. **Unbounded-tail bucketing (§3.2): ~~single row until split~~ → + sub-divide over `[subStart, MaxUint64]` (revised post-merge).** The + first-round decision kept an open-`End` route at one row. That made + the feature inert for the single-route cluster and every cluster's + tail route — the most common place hot-key visibility is wanted. The + original blocker (`1 << (8*W)` overflow) is avoided by using + `MaxUint64` as the effective top, so the open end now sub-divides by + the leading-`W`-byte window (§3.2). The last sub-bucket keeps the + unbounded `End`. Resolution is leading-byte-coarse for `prefixLen 0`; + tight route bounds still give finer intra-prefix detail. 2. **Mixed-`K` fan-out (§7): coexist, don't merge.** Rows keyed by `bucketID` (now embedding `#subIdx`) mean a `K = 1` row and the `K = 16` sub-rows appear side by side rather than merging. Tolerated From 88fa3b94cdca643d3bf30c4ccc881c5a1f1f9018 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 12:34:54 +0900 Subject: [PATCH 2/8] feat(keyviz): sub-divide unbounded-End routes (fix single-route heatmap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deployed 5-node cluster showed a single route route:1 = [nil,nil) (unbounded both ends) and the heatmap stayed one row even though KeyViz was sampling real traffic — because computeSubLayout collapsed any unbounded-End route to a single bucket (the first-round §3.2 decision). That made the feature inert for the single-route cluster and every cluster's tail route, exactly where hot-key visibility is wanted. Now an unbounded-End route sub-divides over [subStart, MaxUint64]: bucket the leading W-byte window of the key across that span (MaxUint64, not 1<<64, so subSpan is a valid uint64 — the overflow that sank the original fallback). The last sub-bucket keeps the unbounded End (nil). subBucketIndex skips the upper-edge clamp for the unbounded sentinel (subHi == nil). K=1 / aggregate / empty-range still collapse to one bucket; fully-bounded routes are unchanged. Resolution caveat (documented): prefixLen is 0 for unbounded routes, so a whole-keyspace route is leading-byte-coarse — keys sharing a long prefix still share a bucket. Tighter route bounds give finer detail. Tests: computeSubLayout unbounded case; subBucketIndex unbounded ([nil,nil) + bounded-low tail); e2e unbounded route emits >1 sub-range row with the last End nil; updated the prior single-bucket assertions. rapid monotonicity property now also covers unbounded routes. Design §3.2 + §9 amended (commit 05626962). keyviz race-clean; lint 0. --- keyviz/sampler.go | 25 +++++++++++--- keyviz/sampler_subrange_test.go | 60 +++++++++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/keyviz/sampler.go b/keyviz/sampler.go index cdb03ed22..160cc9aeb 100644 --- a/keyviz/sampler.go +++ b/keyviz/sampler.go @@ -565,7 +565,11 @@ func (slot *routeSlot) subBucketIndex(key []byte) int { if bytes.Compare(key, slot.subLo) <= 0 { return 0 } - if bytes.Compare(key, slot.subHi) >= 0 { + // subHi == nil is the unbounded-End sentinel (open high end, §3.2): + // there is no finite upper bound to clamp against, so rely on the + // window math + the w >= subEnd (== MaxUint64) guard below. A finite + // route always has a non-nil subHi. + if slot.subHi != nil && bytes.Compare(key, slot.subHi) >= 0 { return k - 1 } w := windowUint64(key, slot.subPrefixLen) @@ -664,11 +668,24 @@ func (s *MemSampler) initSubLayout(slot *routeSlot, start, end []byte) { // subSpan 0) when the route cannot be sub-divided, otherwise k. See // design §3.1–§3.2. func computeSubLayout(start, end []byte, k int) (prefixLen int, subStart, subEnd, subSpan uint64, effK int) { - if k <= 1 || len(end) == 0 { - // K == 1, or an unbounded high end (no finite span to divide): - // a single bucket until a split gives the route a finite End. + if k <= 1 { return 0, 0, 0, 0, 1 } + if len(end) == 0 { + // Unbounded high end (single-route cluster, or any cluster's + // tail route). There is no End window to share a prefix with, so + // bucket the leading W-byte window of the key across + // [subStart, MaxUint64]. MaxUint64 (not 1<<64) keeps subSpan a + // valid uint64 — the overflow that sank the original fallback. + // See design §3.2. + subStart = windowUint64(start, 0) + if subStart == math.MaxUint64 { + // start's window is already at the top of the space (all-0xFF + // leading bytes) — nothing left to divide. + return 0, subStart, subStart, 0, 1 + } + return 0, subStart, math.MaxUint64, math.MaxUint64 - subStart, k + } prefixLen = commonPrefixLen(start, end) subStart = windowUint64(start, prefixLen) subEnd = windowUint64(end, prefixLen) diff --git a/keyviz/sampler_subrange_test.go b/keyviz/sampler_subrange_test.go index 0facda851..831c1fcce 100644 --- a/keyviz/sampler_subrange_test.go +++ b/keyviz/sampler_subrange_test.go @@ -32,7 +32,12 @@ func TestComputeSubLayout(t *testing.T) { checkStartEndSet bool }{ {name: "k=1 single bucket", start: []byte("a"), end: []byte("z"), k: 1, wantEffK: 1}, - {name: "unbounded end single bucket", start: []byte("a"), end: nil, k: 8, wantEffK: 1}, + { + // Unbounded high end now sub-divides over [subStart, MaxUint64] + // (§3.2 revised) rather than collapsing to one bucket. + name: "unbounded end subdivides", start: []byte("a"), end: nil, k: 8, + wantEffK: 8, wantSpanNonZero: true, + }, { name: "normal range subdivides", start: []byte("a"), end: []byte("b"), k: 4, wantEffK: 4, wantSpanNonZero: true, wantPrefixLen: 0, checkStartEndSet: true, @@ -98,8 +103,7 @@ func TestSubBucketIndexSingleBucketAlwaysZero(t *testing.T) { t.Parallel() for _, slot := range []*routeSlot{ layoutSlot([]byte("a"), []byte("z"), 1), // K=1 - layoutSlot([]byte("a"), nil, 8), // unbounded tail - layoutSlot([]byte{0x00}, []byte{0x00}, 8), // degenerate + layoutSlot([]byte{0x00}, []byte{0x00}, 8), // degenerate (empty range) } { require.Len(t, slot.subBuckets, 1) for _, key := range [][]byte{nil, {0x00}, {0x80}, {0xFF, 0xFF}} { @@ -108,6 +112,56 @@ func TestSubBucketIndexSingleBucketAlwaysZero(t *testing.T) { } } +// TestSubBucketIndexUnboundedEnd pins the §3.2 revision: an open high +// end ([start, nil)) — the single-route cluster and every cluster's +// tail route — now sub-divides over [subStart, MaxUint64] instead of +// collapsing to one bucket. The upper-edge clamp is skipped (subHi nil +// sentinel), so distinct leading bytes land in distinct buckets. +func TestSubBucketIndexUnboundedEnd(t *testing.T) { + t.Parallel() + // The single-route cluster the user hit: route owns the whole space. + slot := layoutSlot(nil, nil, 4) + require.Len(t, slot.subBuckets, 4, "[nil,nil) must sub-divide, not collapse to 1") + require.Nil(t, slot.subHi, "unbounded end => subHi nil sentinel") + + // Leading byte spreads keys across the [0, MaxUint64] window: + // 0x00.. -> low bucket, 0xFF.. -> top bucket, and order preserved. + require.Equal(t, 0, slot.subBucketIndex([]byte{0x00})) + require.Equal(t, 3, slot.subBucketIndex([]byte{0xFF})) + i40 := slot.subBucketIndex([]byte{0x40}) + i80 := slot.subBucketIndex([]byte{0x80}) + iC0 := slot.subBucketIndex([]byte{0xC0}) + require.LessOrEqual(t, i40, i80) + require.LessOrEqual(t, i80, iC0) + + // A bounded-low / unbounded-high tail route ([0x80, nil)). + tail := layoutSlot([]byte{0x80}, nil, 4) + require.Len(t, tail.subBuckets, 4) + require.Equal(t, 0, tail.subBucketIndex([]byte{0x00}), "key below tail Start pins to bucket 0") + require.Equal(t, 3, tail.subBucketIndex([]byte{0xFF})) +} + +// TestSamplerUnboundedRouteEmitsSubRanges is the end-to-end fix for the +// deployed single-route cluster: with K>1, an unbounded [nil,nil) route +// now produces multiple sub-range rows (previously exactly one). The +// last sub-bucket keeps the unbounded End (nil). +func TestSamplerUnboundedRouteEmitsSubRanges(t *testing.T) { + t.Parallel() + s := NewMemSampler(MemSamplerOptions{Step: time.Second, HistoryColumns: 4, KeyBucketsPerRoute: 4}) + require.True(t, s.RegisterRoute(1, nil, nil, 0)) // whole keyspace, unbounded both ends + for _, key := range [][]byte{{0x10}, {0x50}, {0x90}, {0xD0}} { + s.Observe(1, key, OpWrite, 0) + } + s.Flush() + rows := s.Snapshot(time.Time{}, time.Time{})[0].Rows + require.Greater(t, len(rows), 1, "unbounded route must emit >1 sub-range row") + sort.Slice(rows, func(i, j int) bool { return bytes.Compare(rows[i].Start, rows[j].Start) < 0 }) + require.Nil(t, rows[len(rows)-1].End, "last sub-bucket keeps the unbounded End (nil)") + for _, r := range rows { + require.Equal(t, 4, r.SubBucketCount) + } +} + // TestSubBucketIndexMonotone is the rapid property: key1 <= key2 implies // idx1 <= idx2 for any [start, end) and K (order preservation). func TestSubBucketIndexMonotone(t *testing.T) { From 42bb65309e4c108c91603a8a4cb67550e17d60f1 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 15:22:07 +0900 Subject: [PATCH 3/8] feat(keyviz): address PR #841 review (stale comments, strict test asserts) No P0/P1/high/critical/major; Claude bot "must-fix before merge" items: - Stale comments: the routeSlot.subSpan comment and initSubLayout docstring still listed "unbounded-tail route (End == nil)" as a single-bucket case. After this PR an unbounded-End route with K>1 sub-divides over [subStart, MaxUint64]; corrected both so only K==1 / aggregate / degenerate-window remain in the subSpan==0 list. - Test: TestSubBucketIndexUnboundedEnd used LessOrEqual for 0x40/0x80/ 0xC0 which land in strictly-increasing buckets 1/2/3 -> tightened to Less so a bucket-collapse regression fails. - Added a deterministic TestComputeSubLayout case for the all-0xFF start guard (subStart == MaxUint64 -> single bucket), previously only covered probabilistically by the rapid property. Deferred (Gemini medium / Claude "enhancement, not a blocker"): counting leading-0xFF bytes as prefixLen for unbounded routes to preserve resolution for 0xFF-heavy system keyspaces. Orthogonal to the single-route [nil,nil) fix this PR targets (start=nil -> prefixLen 0 either way); tracked as a follow-up. Doc/test-only; no logic change -> no caller audit. keyviz green; lint 0. --- keyviz/sampler.go | 17 ++++++++++------- keyviz/sampler_subrange_test.go | 13 +++++++++++-- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/keyviz/sampler.go b/keyviz/sampler.go index 160cc9aeb..e25b33860 100644 --- a/keyviz/sampler.go +++ b/keyviz/sampler.go @@ -310,10 +310,11 @@ type routeSlot struct { // doc §4.2. // // subSpan == 0 (and len(subBuckets) == 1) marks a slot that is not - // sub-divided: K==1, a virtual aggregate, a degenerate window - // (subEnd <= subStart), or an unbounded-tail route (End == nil). - // subBucketIndex then hard-wires the index to 0, collapsing to - // today's route-level behaviour. + // sub-divided: K==1, a virtual aggregate, or a degenerate window + // (subEnd <= subStart). An unbounded-tail route (End == nil) with + // K > 1 DOES sub-divide — over [subStart, MaxUint64] (§3.2) — so it + // is not in this list. subBucketIndex hard-wires the index to 0 for + // the subSpan == 0 slots, collapsing to today's route-level behaviour. subPrefixLen int subStart uint64 subEnd uint64 @@ -646,9 +647,11 @@ func windowUint64(b []byte, off int) uint64 { // initSubLayout computes the immutable sub-range layout for an // individual route spanning [start, end) and allocates its subBuckets. // Called once at slot creation (off the hot path). A route that cannot -// be sub-divided — K == 1, an unbounded `end`, or a window that -// captures no difference — gets a single bucket (subSpan 0), which -// makes subBucketIndex hard-wire to 0 and reproduces today's behaviour. +// be sub-divided — K == 1, or a window that captures no difference — +// gets a single bucket (subSpan 0), which makes subBucketIndex +// hard-wire to 0 and reproduces today's behaviour. An unbounded `end` +// (End == nil) with K > 1 sub-divides over [subStart, MaxUint64] (§3.2), +// not a single bucket. func (s *MemSampler) initSubLayout(slot *routeSlot, start, end []byte) { prefixLen, subStart, subEnd, subSpan, effK := computeSubLayout(start, end, s.opts.KeyBucketsPerRoute) slot.subPrefixLen = prefixLen diff --git a/keyviz/sampler_subrange_test.go b/keyviz/sampler_subrange_test.go index 831c1fcce..cc0cad2a9 100644 --- a/keyviz/sampler_subrange_test.go +++ b/keyviz/sampler_subrange_test.go @@ -59,6 +59,13 @@ func TestComputeSubLayout(t *testing.T) { k: 16, wantEffK: 1, wantPrefixLen: 3, }, {name: "nil start treated as zero", start: nil, end: []byte{0x10}, k: 4, wantEffK: 4, wantSpanNonZero: true}, + { + // Unbounded end whose start's leading 8 bytes are all 0xFF: + // subStart == MaxUint64, nothing left above it to divide, so + // the subStart == MaxUint64 guard collapses to one bucket. + name: "all-0xFF start unbounded end single bucket", + start: bytes.Repeat([]byte{0xFF}, 8), end: nil, k: 8, wantEffK: 1, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -128,11 +135,13 @@ func TestSubBucketIndexUnboundedEnd(t *testing.T) { // 0x00.. -> low bucket, 0xFF.. -> top bucket, and order preserved. require.Equal(t, 0, slot.subBucketIndex([]byte{0x00})) require.Equal(t, 3, slot.subBucketIndex([]byte{0xFF})) + // 0x40/0x80/0xC0 fall in quarters of [0, MaxUint64] -> buckets 1/2/3, + // strictly increasing (Less, not LessOrEqual, so a collapse regresses). i40 := slot.subBucketIndex([]byte{0x40}) i80 := slot.subBucketIndex([]byte{0x80}) iC0 := slot.subBucketIndex([]byte{0xC0}) - require.LessOrEqual(t, i40, i80) - require.LessOrEqual(t, i80, iC0) + require.Less(t, i40, i80) + require.Less(t, i80, iC0) // A bounded-low / unbounded-high tail route ([0x80, nil)). tail := layoutSlot([]byte{0x80}, nil, 4) From 1878ef449b0ddac83070966d75bd7c90b2024ce0 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 15:34:41 +0900 Subject: [PATCH 4/8] feat(keyviz): cap effK at span + clear stale unbounded-tail doc (PR #841 round-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 (invalid emitted bounds): for a high `start` whose 8-byte window is near MaxUint64 AND carries suffix bytes past the window, subSpan is tiny (< k), so boundaryAt rounded interior boundaries back to subStart; the reconstructed 8-byte boundary lacks the start's suffix, so it sorted BEFORE routeStart -> bucket 0 emitted Start > End (invalid/overlapping ranges on tail routes). Fix: effKForSpan caps the effective bucket count at the span (effK = min(k, subSpan)); a span < k cannot be divided into k order-distinct sub-ranges. This only triggers for unbounded high-start routes — bounded routes always get subSpan >= 1<<56 because commonPrefixLen lands on the first differing (top window) byte, so their behaviour is unchanged. Pinned by a computeSubLayout case (effK capped) and an e2e test asserting no emitted sub-row has Start > End. Claude bot must-fix (doc): three sections still listed "unbounded-tail" as a single-bucket / SubBucketCount==1 case, contradicting the revised §3.2/§9 in the same file — corrected the struct-field comment, the §4.2 paragraph, and the §4.3 / §5.2 SubBucketCount/BucketID lists. Also synced SubBucket/SubBucketCount to `int` in the doc (matches the code). effK is internal to computeSubLayout (one caller, initSubLayout, which sizes subBuckets + sets subLo/subHi) — not a fail-closed / return-meaning / error-handling change, so no caller audit. keyviz race-clean; lint 0. --- ...05_25_proposed_keyviz_subrange_sampling.md | 25 ++++++++------- keyviz/sampler.go | 26 +++++++++++++-- keyviz/sampler_subrange_test.go | 32 +++++++++++++++++++ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md index 47c9f12b8..1d2dd2e2e 100644 --- a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md +++ b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md @@ -233,14 +233,15 @@ subPrefixLen int subStart uint64 subEnd uint64 // subStart + subSpan; the §3.1 guard reads it subSpan uint64 // 0 ⇒ single-bucket slot (§3.2) -subBuckets []subCounter // len == K (1 for aggregate / degenerate / unbounded-tail) +subBuckets []subCounter // len == effK (1 for aggregate / degenerate) ``` `subCounter` holds the same four `atomic.Uint64` the slot holds today. -For `K = 1` (flag default) and for aggregate / degenerate / -unbounded-tail slots, `subBuckets` has length 1 and `subBucketIndex` -short-circuits to 0, so the code path collapses to exactly today's -behaviour with one extra indirection. The `subShift` field from the +For `K = 1` (flag default) and for aggregate / degenerate slots, +`subBuckets` has length 1 and `subBucketIndex` short-circuits to 0, so +the code path collapses to exactly today's behaviour with one extra +indirection. (An unbounded-`End` route with `K > 1` does sub-divide — +§3.2 — over `[subStart, MaxUint64]`, capped at the span.) The `subShift` field from the first draft is gone — `math/bits` (§3.1) makes the multiply/divide exact without it. @@ -326,11 +327,12 @@ sub-range emits one row, not `K`. `MatrixRow` gains **two** fields, not one: -- `SubBucket uint32` — this row's index within its route, `0` for the +- `SubBucket int` — this row's index within its route, `0` for the first (or only) sub-bucket. -- `SubBucketCount uint32` — how many sub-buckets the parent route is - divided into: `1` for a `K = 1` / aggregate / degenerate / - unbounded-tail slot, `> 1` for a genuinely sub-bucketed route. +- `SubBucketCount int` — how many sub-buckets the parent route is + divided into: `1` for a `K = 1` / aggregate / degenerate slot, `> 1` + for a genuinely sub-bucketed route (including an unbounded-`End` route + with `K > 1`). `SubBucketCount` is the disambiguator `bucketIDFor` needs (Claude bot gap): a `K = 1` slot's single row and a `K > 1` slot's first sub-row @@ -375,9 +377,10 @@ have narrower bounds. To keep `BucketID` unique per row within a column (the fan-out merge and the SPA both key on it), `bucketIDFor` gains a sub-range discriminator keyed on `SubBucketCount` (§4.3): -- sub-bucketed route (`SubBucketCount > 1`): `route:#` +- sub-bucketed route (`SubBucketCount > 1`, including an unbounded-`End` + route with `K > 1`): `route:#` - whole route (`SubBucketCount == 1`, the `K = 1` / aggregate / - degenerate / unbounded-tail case): `route:` (unchanged) + degenerate case): `route:` (unchanged) - virtual bucket: `virtual:` (unchanged) ### 5.3 SPA diff --git a/keyviz/sampler.go b/keyviz/sampler.go index e25b33860..0f11046fd 100644 --- a/keyviz/sampler.go +++ b/keyviz/sampler.go @@ -668,7 +668,9 @@ func (s *MemSampler) initSubLayout(slot *routeSlot, start, end []byte) { // computeSubLayout derives the sub-range layout for [start, end) divided // into k buckets. effK is the effective bucket count: 1 (single bucket, -// subSpan 0) when the route cannot be sub-divided, otherwise k. See +// subSpan 0) when the route cannot be sub-divided, otherwise +// min(k, subSpan) — capping at the span keeps every reconstructed +// boundary valid when the span is narrower than k (effKForSpan). See // design §3.1–§3.2. func computeSubLayout(start, end []byte, k int) (prefixLen int, subStart, subEnd, subSpan uint64, effK int) { if k <= 1 { @@ -687,7 +689,8 @@ func computeSubLayout(start, end []byte, k int) (prefixLen int, subStart, subEnd // leading bytes) — nothing left to divide. return 0, subStart, subStart, 0, 1 } - return 0, subStart, math.MaxUint64, math.MaxUint64 - subStart, k + subSpan = math.MaxUint64 - subStart + return 0, subStart, math.MaxUint64, subSpan, effKForSpan(subSpan, k) } prefixLen = commonPrefixLen(start, end) subStart = windowUint64(start, prefixLen) @@ -697,7 +700,24 @@ func computeSubLayout(start, end []byte, k int) (prefixLen int, subStart, subEnd // only past prefixLen + W): single bucket. return prefixLen, subStart, subEnd, 0, 1 } - return prefixLen, subStart, subEnd, subEnd - subStart, k + subSpan = subEnd - subStart + return prefixLen, subStart, subEnd, subSpan, effKForSpan(subSpan, k) +} + +// effKForSpan caps the effective bucket count at the window span. A span +// smaller than k cannot be divided into k order-distinct sub-ranges: +// boundaryAt would round several interior boundaries back to subStart, +// and since a reconstructed boundary carries no bytes past the window it +// can sort BEFORE a route start that has suffix bytes — emitting a row +// with Start > End (Codex P2). Capping at subSpan keeps every emitted +// boundary strictly increasing in the window, so bounds stay valid and +// non-overlapping. subSpan is > 0 here (the degenerate subEnd<=subStart +// and all-0xFF cases return earlier), so effK >= 1. +func effKForSpan(subSpan uint64, k int) int { + if subSpan < u64NonNeg(k) { + return intFromUint64(subSpan) + } + return k } // commonPrefixLen returns the number of leading bytes a and b share. diff --git a/keyviz/sampler_subrange_test.go b/keyviz/sampler_subrange_test.go index cc0cad2a9..2f482e321 100644 --- a/keyviz/sampler_subrange_test.go +++ b/keyviz/sampler_subrange_test.go @@ -66,6 +66,14 @@ func TestComputeSubLayout(t *testing.T) { name: "all-0xFF start unbounded end single bucket", start: bytes.Repeat([]byte{0xFF}, 8), end: nil, k: 8, wantEffK: 1, }, + { + // Unbounded high start: window 0xFF..FD leaves a span of 2 ( End. Capping effK at the +// span (effKForSpan) keeps reconstructed boundaries strictly above the +// route start. Without the cap, bucket 0 would emit Start > End here. +func TestSamplerUnboundedHighStartValidBounds(t *testing.T) { + t.Parallel() + s := NewMemSampler(MemSamplerOptions{Step: time.Second, HistoryColumns: 4, KeyBucketsPerRoute: 4}) + start := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFD, 0xFF} // window 0xFF..FD + suffix + require.True(t, s.RegisterRoute(1, start, nil, 0)) + for _, key := range [][]byte{start, {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE}} { + s.Observe(1, key, OpWrite, 0) + } + s.Flush() + rows := s.Snapshot(time.Time{}, time.Time{})[0].Rows + require.NotEmpty(t, rows) + for _, r := range rows { + if r.End != nil { // the last sub-bucket keeps the unbounded End (nil) + require.Less(t, bytes.Compare(r.Start, r.End), 0, + "sub-row must have Start < End (no invalid/overlapping bounds): %x..%x", r.Start, r.End) + } + } +} + // TestSubBucketIndexMonotone is the rapid property: key1 <= key2 implies // idx1 <= idx2 for any [start, end) and K (order preservation). func TestSubBucketIndexMonotone(t *testing.T) { From 1176aabc847c02d3218c092b0f7ab4632780343d Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 15:45:09 +0900 Subject: [PATCH 5/8] feat(keyviz): ceil boundary reconstruction for bucket/range consistency (PR #841 round-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 (boundary/bucket consistency): boundaryAt used floor while subBucketIndex uses floor on the forward map, so a key landing exactly on an interior boundary value (e.g. 0x3fff... for [nil,nil) k=4) was counted in the lower bucket yet excluded from that bucket's half-open [Start,End) label and appeared to belong to the next row. Fix: reconstruct boundaries with ceil(i*subSpan/effK) (new fracMulCeil), which is exactly the forward map's lower edge of bucket i — so the displayed range of a key's bucket always contains that key. For spans divisible by effK (the common case) ceil == floor, so no change; only off-boundary cases shift. Pinned by TestSubBucketBoundsContainCountedKey (sweeps keys incl. exact boundaries for bounded + unbounded, asserts the counted bucket's range contains the key; in-range keys only — the out-of-range upper clamp is exempt). Claude bot minor doc gaps: §3.2 now states effK = min(K, subSpan) and the ceil reconstruction; §3.1's bits.Div64 safety argument updated from "K capped well below subSpan" to "hi < K <= subSpan" (effKForSpan can make effK == subSpan). fracMulCeil/boundaryAt are internal to the Flush reconstruction path (no new callers; not a fail-closed/return-meaning/error change) -> no caller audit. keyviz race-clean; lint 0. --- ...05_25_proposed_keyviz_subrange_sampling.md | 29 +++++++---- keyviz/sampler.go | 35 +++++++++---- keyviz/sampler_subrange_test.go | 49 +++++++++++++++++++ 3 files changed, 94 insertions(+), 19 deletions(-) diff --git a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md index 1d2dd2e2e..1cb109907 100644 --- a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md +++ b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md @@ -131,10 +131,11 @@ lives in the bytes *after* that prefix. So: (`uint64(K)` cast shown explicitly — `K` is an `int`/`uint32` config value while `bits.Mul64` takes `uint64`). `bits.Div64` panics only when `hi >= subSpan`; the interior guard - guarantees `k - subStart < subSpan`, so `hi < K`, and `K` is - capped well below `subSpan` whenever `subSpan > 0` (the degenerate - `subSpan == 0` route is forced to `K=1` at register — §3.2 — and - never reaches this branch). With the `k >= subEnd` guard already + guarantees `k - subStart < subSpan`, so `hi < K`, and the effective + `K` is **at most `subSpan`** (`effKForSpan` caps it — §3.2), so + `hi < K ≤ subSpan` and the divide is safe. The degenerate + `subSpan == 0` route is forced to `effK = 1` at register (§3.2) and + never reaches this branch. With the `k >= subEnd` guard already handling the top edge, the interior case (`k <= subEnd - 1`) yields at most `K-1` exactly, so a final clamp to `[0, K-1]` can **never actually fire** — it is kept only as defensive @@ -156,12 +157,20 @@ lives in the bytes *after* that prefix. So: `[subStart, MaxUint64]`** (revised — see the note below). There is no `End` window, so set `subPrefixLen = 0`, `subStart = windowUint64(start)`, `subEnd = math.MaxUint64`, `subSpan = - MaxUint64 - subStart`, and divide that finite span into `K`. Keys - bucket by their leading `W`-byte window across the open high end; the - **last** sub-bucket keeps the route's unbounded `End` (`nil`). The - `subBucketIndex` upper edge clamp is skipped for this slot (its - `subHi` snapshot is `nil` — the unbounded sentinel), relying on the - window math + the `w >= subEnd` guard instead. + MaxUint64 - subStart`, and divide that finite span into + `effK = min(K, subSpan)` buckets. For a typical unbounded route + `subSpan ≫ K`, so `effK = K`; the cap (`effKForSpan`) only bites for a + near-`MaxUint64` start, where a span `< K` could otherwise make + `boundaryAt` emit bounds that sort before the route start (an invalid + `Start > End` row). Keys bucket by their leading `W`-byte window across + the open high end; the **last** sub-bucket keeps the route's unbounded + `End` (`nil`). The `subBucketIndex` upper edge clamp is skipped for + this slot (its `subHi` snapshot is `nil` — the unbounded sentinel), + relying on the window math + the `w >= subEnd` guard instead. + Interior boundaries are reconstructed with **`ceil(i·subSpan/effK)`** + (`fracMulCeil`), exactly the forward map's lower edge of bucket `i`, so + a key landing on a boundary value is displayed in the same bucket it is + counted in. *Why this reverses Open Q1.* The first review chose "single row until split" because the original fallback used `subSpan = 1 << (8*W)`, diff --git a/keyviz/sampler.go b/keyviz/sampler.go index 0f11046fd..22c47918b 100644 --- a/keyviz/sampler.go +++ b/keyviz/sampler.go @@ -616,15 +616,31 @@ func intFromUint64(v uint64) int { // fracMul returns floor(a*b/c) with an exact 128-bit intermediate via // math/bits, so the multiply cannot overflow uint64. Callers guarantee // c > 0 and the quotient < 2^64 (both hold for sub-bucket math: the -// interior guard bounds a*b below c*2^64 because k is capped well below -// 2^64). Shared by the forward index path and the Flush bound -// reconstruction so the two can never diverge. +// interior guard bounds a*b below c*2^64 because effK <= subSpan). +// Used by the forward index path (subBucketIndex). func fracMul(a, b, c uint64) uint64 { hi, lo := bits.Mul64(a, b) q, _ := bits.Div64(hi, lo, c) return q } +// fracMulCeil returns ceil(a*b/c) with the same exact 128-bit +// intermediate. boundaryAt uses ceil (not floor) so an emitted interior +// boundary is exactly the forward map's lower edge of bucket i: +// subBucketIndex puts w in bucket i iff w-subStart in +// [ceil(i*subSpan/k), ceil((i+1)*subSpan/k)). With floor boundaries a +// key landing exactly on a boundary value would be counted in the lower +// bucket yet excluded from its half-open [Start, End) label (Codex P2); +// ceil keeps the displayed ranges consistent with where keys are counted. +func fracMulCeil(a, b, c uint64) uint64 { + hi, lo := bits.Mul64(a, b) + q, r := bits.Div64(hi, lo, c) + if r != 0 { + q++ + } + return q +} + // windowUint64 reads up to subWindowBytes of b starting at off, // big-endian, zero-padding past the end of b. A short/absent window is // the natural low end (0x00…) of the key space. @@ -756,13 +772,14 @@ func (slot *routeSlot) subBucketBounds(i int, routeStart, routeEnd []byte) (star return start, end } -// boundaryAt reconstructs the key at fraction i/k of the route's window: -// subStart + floor(i*subSpan/k), written big-endian into the W-byte -// window at subPrefixLen, behind the route's shared prefix. Uses the -// same math/bits kernel as subBucketIndex so the forward and inverse -// directions can never disagree. +// boundaryAt reconstructs bucket i's lower edge in key space: +// subStart + ceil(i*subSpan/k), written big-endian into the W-byte +// window at subPrefixLen, behind the route's shared prefix. ceil (via +// fracMulCeil) makes this exactly the forward map's lower edge of bucket +// i, so the emitted half-open [Start, End) ranges agree with where +// subBucketIndex counts boundary-value keys (Codex P2). func (slot *routeSlot) boundaryAt(i int, routeStart []byte) []byte { - off := slot.subStart + fracMul(u64NonNeg(i), slot.subSpan, u64NonNeg(len(slot.subBuckets))) + off := slot.subStart + fracMulCeil(u64NonNeg(i), slot.subSpan, u64NonNeg(len(slot.subBuckets))) out := make([]byte, slot.subPrefixLen+subWindowBytes) copy(out, routeStart[:min(slot.subPrefixLen, len(routeStart))]) binary.BigEndian.PutUint64(out[slot.subPrefixLen:], off) diff --git a/keyviz/sampler_subrange_test.go b/keyviz/sampler_subrange_test.go index 2f482e321..a698bfeb1 100644 --- a/keyviz/sampler_subrange_test.go +++ b/keyviz/sampler_subrange_test.go @@ -203,6 +203,55 @@ func TestSamplerUnboundedHighStartValidBounds(t *testing.T) { } } +// TestSubBucketBoundsContainCountedKey pins Codex P2 (boundary/bucket +// consistency): the emitted [Start, End) of the bucket subBucketIndex +// assigns a key to must actually CONTAIN that key — including keys that +// land exactly on an interior boundary value. ceil-based boundaryAt +// guarantees this; floor-based would mis-shelve boundary-value keys. +func TestSubBucketBoundsContainCountedKey(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + start, end []byte + k int + }{ + {"unbounded whole space", nil, nil, 4}, + {"unbounded k not dividing span", nil, nil, 7}, + {"bounded", []byte{0x00}, []byte{0x40}, 4}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + slot := layoutSlot(tc.start, tc.end, tc.k) + // Sweep keys, plus the exact reconstructed interior boundaries + // (the values most likely to expose a floor/ceil mismatch). + keys := [][]byte{{0x00}, {0x01}, {0x3F}, {0x40}, {0x7F}, {0x80}, {0xC0}, {0xFE}, {0xFF}} + for i := 1; i < len(slot.subBuckets); i++ { + keys = append(keys, slot.boundaryAt(i, cloneBytes(tc.start))) + } + for _, key := range keys { + // Only in-range keys (start <= key < end) are ever Observed + // against this route; the out-of-range clamp (key >= End -> + // last bucket) intentionally does not place such keys inside + // a half-open range. + if bytes.Compare(key, tc.start) < 0 { + continue + } + if tc.end != nil && bytes.Compare(key, tc.end) >= 0 { + continue + } + idx := slot.subBucketIndex(key) + lo, hi := slot.subBucketBounds(idx, cloneBytes(tc.start), cloneBytes(tc.end)) + require.LessOrEqual(t, bytes.Compare(lo, key), 0, + "%s: key %x counted in bucket %d but Start %x is above it", tc.name, key, idx, lo) + if hi != nil { + require.Less(t, bytes.Compare(key, hi), 0, + "%s: key %x counted in bucket %d but End %x excludes it", tc.name, key, idx, hi) + } + } + }) + } +} + // TestSubBucketIndexMonotone is the rapid property: key1 <= key2 implies // idx1 <= idx2 for any [start, end) and K (order preservation). func TestSubBucketIndexMonotone(t *testing.T) { From 64df29048bcce42ba30e8eb09e60d46fc5733128 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 15:51:43 +0900 Subject: [PATCH 6/8] feat(keyviz): fix stale fracMul comment + bounded non-divisible test (PR #841 round-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude bot (ready-to-merge once addressed): - subBucketBounds comment said interiors "use the same fracMul kernel as the forward path", but after the ceil fix boundaryAt uses fracMulCeil — intentionally the ceil-dual of the forward floor fracMul. Updated the comment so it no longer implies the two paths are identical. - Added a "bounded k not dividing span" case (k=7, 2^62/7 non-integer -> ceil != floor) to TestSubBucketBoundsContainCountedKey for deterministic coverage of the ceil reconstruction on bounded routes (the prior bounded case had a divisible span where ceil == floor). Comment/test-only; no logic change -> no caller audit. keyviz green; lint 0. --- keyviz/sampler.go | 10 ++++++---- keyviz/sampler_subrange_test.go | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/keyviz/sampler.go b/keyviz/sampler.go index 22c47918b..1927e6e52 100644 --- a/keyviz/sampler.go +++ b/keyviz/sampler.go @@ -753,10 +753,12 @@ func commonPrefixLen(a, b []byte) int { // i for a slot divided into len(subBuckets) buckets. Bucket 0 pins to // the route's actual start and the last bucket pins to its actual end // (the window arithmetic drops bytes past prefixLen + W, so the -// reconstructed extremes would otherwise fall short); interiors use the -// same fracMul kernel as the forward path. Together this tiles -// [routeStart, routeEnd) exactly — each bucket's start equals the -// previous bucket's end. Caller guarantees len(subBuckets) > 1. +// reconstructed extremes would otherwise fall short); interiors use +// boundaryAt's fracMulCeil — the ceil-dual of the forward path's floor +// fracMul, deliberately different so a boundary-value key lands in the +// same bucket it is counted in. Together this tiles [routeStart, +// routeEnd) exactly — each bucket's start equals the previous bucket's +// end. Caller guarantees len(subBuckets) > 1. func (slot *routeSlot) subBucketBounds(i int, routeStart, routeEnd []byte) (start, end []byte) { last := len(slot.subBuckets) - 1 if i == 0 { diff --git a/keyviz/sampler_subrange_test.go b/keyviz/sampler_subrange_test.go index a698bfeb1..67a10328f 100644 --- a/keyviz/sampler_subrange_test.go +++ b/keyviz/sampler_subrange_test.go @@ -217,7 +217,10 @@ func TestSubBucketBoundsContainCountedKey(t *testing.T) { }{ {"unbounded whole space", nil, nil, 4}, {"unbounded k not dividing span", nil, nil, 7}, - {"bounded", []byte{0x00}, []byte{0x40}, 4}, + {"bounded divisible span", []byte{0x00}, []byte{0x40}, 4}, + // 2^62 / 7 is not integer, so ceil != floor for these boundaries — + // deterministic coverage of the ceil reconstruction for bounded. + {"bounded k not dividing span", []byte{0x00}, []byte{0x40}, 7}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() From c08eecaf6e663eb5d3afd4bb9179900c01626588 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 16:00:48 +0900 Subject: [PATCH 7/8] =?UTF-8?q?docs(keyviz):=20=C2=A74.3=20reconstruction?= =?UTF-8?q?=20uses=20fracMulCeil=20(match=20=C2=A73.2)=20(PR=20#841=20roun?= =?UTF-8?q?d-5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude bot: §4.3 still showed the floor-based reconstruction snippet and said the boundary "must use the same path as the forward computation", contradicting §3.2 (already updated in 1176aab) which documents fracMulCeil. After the ceil fix, boundaryAt deliberately uses ceil — the DUAL of the forward floor fracMul, not the same function (using floor is what mis-shelves boundary-value keys, the Codex P2 this closes). Updated §4.3 to reference fracMulCeil / ceil(i*subSpan/effK) and keep the 128-bit overflow-safety note. Doc-only; no code change -> no caller audit. --- ...05_25_proposed_keyviz_subrange_sampling.md | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md index 1cb109907..95cbc2c8c 100644 --- a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md +++ b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md @@ -307,18 +307,20 @@ target `K ≤ 16`. `appendDrainedRow` becomes a loop over the slot's `subBuckets`: for each sub-bucket with any non-zero counter, emit one `MatrixRow` whose `Start`/`End` are the **sub-bucket bounds**, reconstructed by inverting -the §3.1 interpolation: bucket `i` spans -`[subStart + i*subSpan/K, subStart + (i+1)*subSpan/K)` mapped back into -the key byte space at offset `subPrefixLen`. The interior boundary -`i*subSpan/K` **must use the same 128-bit `math/bits` path as the -forward computation** — `hi, lo := bits.Mul64(uint64(i), subSpan); -bound, _ := bits.Div64(hi, lo, uint64(K))` — not plain `uint64` -arithmetic: `i*subSpan` overflows a `uint64` once `subSpan > 2^63` and -`i ≥ 2` (e.g. `K=3, i=2, subSpan=2^63` → `2^64` wraps to `0`), -producing a nonsensical interior bound and a visible gap in the -heatmap. To guarantee the sub-rows **tile the parent route exactly** -(the "contiguous" property the heatmap advertises) despite integer -truncation in the inverse: +the §3.1 interpolation: bucket `i`'s lower edge is +`subStart + ceil(i*subSpan/effK)`, mapped back into the key byte space +at offset `subPrefixLen`. The interior boundary uses **`fracMulCeil`** — +`hi, lo := bits.Mul64(uint64(i), subSpan); q, r := bits.Div64(hi, lo, +uint64(effK)); if r != 0 { q++ }` — i.e. **`ceil`, deliberately the +*dual* of the forward path's `floor` `fracMul`**, not the same function. +Using `floor` here (the forward kernel) is exactly what mis-shelves a +boundary-value key (§3.2, Codex P2): `ceil` makes the emitted lower edge +equal the forward map's bucket-`i` lower edge, so a key on a boundary is +displayed in the bucket it is counted in. The 128-bit `math/bits` form +is still required for overflow safety — `i*subSpan` overflows `uint64` +once `subSpan > 2^63` and `i ≥ 2`. To guarantee the sub-rows **tile the +parent route exactly** (the "contiguous" property the heatmap advertises) +despite integer truncation in the inverse: - sub-bucket `0`'s `Start` is pinned to the route's actual `Start` (not the reconstructed `subStart` window value, which dropped the From 075b305938596cb500fcab4203438956e1fe10e0 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Tue, 26 May 2026 16:06:04 +0900 Subject: [PATCH 8/8] docs(keyviz): clear remaining stale single-bucket mentions (PR #841 round-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude bot: §10 item 5's test-coverage list still said "End==nil→single-bucket", contradicting this PR's core reversal (End==nil with K>1 now sub-divides). Swept the whole doc and also fixed §3.2's "Both single-bucket cases" wrap-up, which assumed the old two-case structure. Now both describe the actual single-bucket fall-backs (K=1, degenerate window, all-0xFF start) and note that an unbounded End with K>1 sub-divides. Doc-only; no code change -> no caller audit. --- .../2026_05_25_proposed_keyviz_subrange_sampling.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md index 95cbc2c8c..c4ec828a6 100644 --- a/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md +++ b/docs/design/2026_05_25_proposed_keyviz_subrange_sampling.md @@ -195,8 +195,11 @@ lives in the bytes *after* that prefix. So: wider `W` or a route split resolves. Surfaced via a debug metric so we can tell whether `W = 8` is too small in practice. -Both single-bucket cases reduce that slot to today's exact route-level -behaviour, so they are always safe fall-backs rather than error paths. +The single-bucket fall-backs (`K = 1`, a degenerate window, and the +all-`0xFF` start of an unbounded route — §3.1) reduce that slot to +today's exact route-level behaviour, so they are always safe fall-backs +rather than error paths. An unbounded `End` with `K > 1` and a +non-`MaxUint64` start does sub-divide. ## 4. Sampler changes (`keyviz/`) @@ -579,8 +582,8 @@ implementation has no ambiguity. unchanged. 5. **Test coverage** — new unit tests: `subBucketIndex` order-preservation + clamping + underflow guard (table-driven, incl. - `Start==nil`, `End==nil`→single-bucket, degenerate window, - `k == subEnd` edge); a `rapid` property test that `key1 <= key2 ⇒ + `Start==nil`, `End==nil`(K>1)→sub-divides, all-0xFF-start→single-bucket, + degenerate window, `k == subEnd` edge); a `rapid` property test that `key1 <= key2 ⇒ idx1 <= idx2` over random `[Start,End)` and keys; `Flush` emits one row per non-empty sub-bucket with bounds that **tile `[Start,End)` exactly** (no gap/overlap; bucket-0 `Start == route Start`,