keyviz adapter: clamp rows to design cap + writes default + doc divergence

Round-3 review polish for PR #646 (Claude bot):

- Default series swapped from Reads to Writes for KEYVIZ_SERIES_UNSPECIFIED
  to match design doc §4.1, which spec's writes as the default. The
  read-sampling path is intentionally Phase 2 (no reads will land in
  the matrix until then) so defaulting to a permanently-zero series
  was actively misleading.
- Add keyVizRowBudgetCap = 1024 + clampRowBudget. Design §4.1 caps
  rows at 1024 to bound server work; pathological clients asking for
  more get the cap, not an error. Regression test
  TestGetKeyVizMatrixClampsRowsBudgetToCap stages 1029 rows and
  verifies the response carries 1024.
- Document the design §5.5 divergence on applyKeyVizRowBudget:
  truncation is a Phase-1 simplification of the spec'd
  "lexicographic walk + greedy merge" so future contributors don't
  treat the truncation behavior as the contract.
- Update TestGetKeyVizMatrixSeriesSelection's UNSPECIFIED case to
  expect Writes instead of Reads.

Author: bootjp

adapter/admin_grpc.go

@@ -535,7 +535,24 @@ func (s *AdminServer) GetKeyVizMatrix(
 	to := unixMsToTime(req.GetToUnixMs())
 	cols := sampler.Snapshot(from, to)
 	pickValue := matrixSeriesPicker(req.GetSeries())
-	return matrixToProto(cols, pickValue, int(req.GetRows())), nil
+	return matrixToProto(cols, pickValue, clampRowBudget(int(req.GetRows()))), nil
+}
+
+// keyVizRowBudgetCap is the upper bound on the per-request rows
+// budget — design doc §4.1 caps rows at 1024 to bound server work
+// (sort + payload) for adversarial / over-large requests.
+const keyVizRowBudgetCap = 1024
+
+// clampRowBudget enforces design §4.1's upper bound. A request of 0
+// (or negative) means "no cap" and is preserved; anything past the
+// cap is silently clamped — clients asking for more rows than the
+// server is willing to render get the most rows the server will
+// render, not an error.
+func clampRowBudget(requested int) int {
+	if requested > keyVizRowBudgetCap {
+		return keyVizRowBudgetCap
+	}
+	return requested
 }
 
 // unixMsToTime converts a Unix-millisecond timestamp into a time.Time,
@@ -549,21 +566,22 @@ func unixMsToTime(ms int64) time.Time {
 }
 
 // matrixSeriesPicker returns a callback that extracts the requested
-// counter from a MatrixRow. KEYVIZ_SERIES_UNSPECIFIED (and READS)
-// fall through to Reads so a default-valued request still returns
-// something useful.
+// counter from a MatrixRow. KEYVIZ_SERIES_UNSPECIFIED falls through
+// to Writes per design doc §4.1 — write traffic is the primary
+// signal the heatmap is built around, and the read path is wired in
+// a follow-up phase.
 func matrixSeriesPicker(series pb.KeyVizSeries) func(keyviz.MatrixRow) uint64 {
 	switch series {
-	case pb.KeyVizSeries_KEYVIZ_SERIES_WRITES:
-		return func(r keyviz.MatrixRow) uint64 { return r.Writes }
+	case pb.KeyVizSeries_KEYVIZ_SERIES_READS:
+		return func(r keyviz.MatrixRow) uint64 { return r.Reads }
 	case pb.KeyVizSeries_KEYVIZ_SERIES_READ_BYTES:
 		return func(r keyviz.MatrixRow) uint64 { return r.ReadBytes }
 	case pb.KeyVizSeries_KEYVIZ_SERIES_WRITE_BYTES:
 		return func(r keyviz.MatrixRow) uint64 { return r.WriteBytes }
-	case pb.KeyVizSeries_KEYVIZ_SERIES_UNSPECIFIED, pb.KeyVizSeries_KEYVIZ_SERIES_READS:
-		return func(r keyviz.MatrixRow) uint64 { return r.Reads }
+	case pb.KeyVizSeries_KEYVIZ_SERIES_UNSPECIFIED, pb.KeyVizSeries_KEYVIZ_SERIES_WRITES:
+		return func(r keyviz.MatrixRow) uint64 { return r.Writes }
 	default:
-		return func(r keyviz.MatrixRow) uint64 { return r.Reads }
+		return func(r keyviz.MatrixRow) uint64 { return r.Writes }
 	}
 }
 
@@ -608,6 +626,14 @@ func matrixToProto(cols []keyviz.MatrixColumn, pick func(keyviz.MatrixRow) uint6
 // applyKeyVizRowBudget caps rows to budget by total activity per row
 // (sum of per-column values), preserving the top-N rows. budget <= 0
 // means "no cap."
+//
+// NOTE: design doc §5.5 specifies a "lexicographic walk + greedy
+// merge of low-activity adjacent ranges" algorithm — we simplify to
+// activity-descending truncation for Phase 1 because it covers the
+// common UI need (highlight hotspots) without needing the synthetic
+// virtual-bucket plumbing the merge requires. Phase 2 should swap
+// this for the spec'd merge so low-activity ranges become coarse
+// aggregates instead of being silently dropped.
 func applyKeyVizRowBudget(rows []*pb.KeyVizRow, budget int) []*pb.KeyVizRow {
 	if budget <= 0 || len(rows) <= budget {
 		return rows

adapter/admin_grpc_keyviz_test.go

@@ -126,7 +126,7 @@ func TestGetKeyVizMatrixSeriesSelection(t *testing.T) {
 		series pb.KeyVizSeries
 		want   uint64
 	}{
-		{"unspecified defaults to reads", pb.KeyVizSeries_KEYVIZ_SERIES_UNSPECIFIED, 11},
+		{"unspecified defaults to writes", pb.KeyVizSeries_KEYVIZ_SERIES_UNSPECIFIED, 22},
 		{"reads", pb.KeyVizSeries_KEYVIZ_SERIES_READS, 11},
 		{"writes", pb.KeyVizSeries_KEYVIZ_SERIES_WRITES, 22},
 		{"read_bytes", pb.KeyVizSeries_KEYVIZ_SERIES_READ_BYTES, 333},
@@ -214,6 +214,35 @@ func TestGetKeyVizMatrixSurfacesRouteCountTruncation(t *testing.T) {
 	require.Equal(t, []uint64{2, 3}, r.RouteIds)
 }
 
+// TestGetKeyVizMatrixClampsRowsBudgetToCap pins design §4.1's
+// upper-bound: rows requests above the keyVizRowBudgetCap are
+// silently clamped down to the cap so a pathological client cannot
+// force the server to materialise an unbounded payload.
+func TestGetKeyVizMatrixClampsRowsBudgetToCap(t *testing.T) {
+	t.Parallel()
+	rows := make([]keyviz.MatrixRow, keyVizRowBudgetCap+5)
+	for i := range rows {
+		idx := uint64(i + 1) //nolint:gosec // i is bounded by keyVizRowBudgetCap+5
+		rows[i] = keyviz.MatrixRow{
+			RouteID: idx,
+			Start:   []byte{byte(i / 256), byte(i % 256)},
+			End:     []byte{byte((i + 1) / 256), byte((i + 1) % 256)},
+			Writes:  idx,
+		}
+	}
+	srv := newAdminServerWithFakeSampler(t, []keyviz.MatrixColumn{{
+		At:   time.Unix(1_700_000_000, 0),
+		Rows: rows,
+	}})
+
+	resp, err := srv.GetKeyVizMatrix(context.Background(), &pb.GetKeyVizMatrixRequest{
+		Series: pb.KeyVizSeries_KEYVIZ_SERIES_WRITES,
+		Rows:   uint32(keyVizRowBudgetCap + 1000),
+	})
+	require.NoError(t, err)
+	require.Len(t, resp.Rows, keyVizRowBudgetCap, "rows must be clamped to keyVizRowBudgetCap")
+}
+
 // TestGetKeyVizMatrixHonorsRowsBudget pins Codex round-1 P1 on
 // PR #646: a request with rows=N must return at most N rows. We
 // stage 4 routes with distinct activity totals and request rows=2;