keyviz admin: apply default rows budget when omitted (PR #660 follow-up) (#672)

## Summary

Follow-up to PR #660, which was merged at the round-2 commit before this
Codex P1 fix propagated. The orphaned change:

**Codex round-3 P1**: `?rows=` was optional and parsed to `0` when
omitted; `applyKeyVizRowBudget` then treats `budget <= 0` as "no cap"
and returns every tracked route in one payload — defeating the 1024-row
resource guard the cap was supposed to provide. A normal SPA poll
without `?rows=` would fall into this trap.

- `parseKeyVizParams` now pre-seeds `rows = keyVizRowBudgetCap` so an
omitted query parameter inherits the cap.
- `setKeyVizRowsParam` collapses an explicit `rows=0` to the cap so
callers cannot disable the budget by passing 0.
- Godoc updated: omitted / 0 / negative all yield the cap; explicit
values above the cap are silently clamped down.

## Test plan

- [x] `TestKeyVizHandlerOmittedRowsAppliesDefaultCap` — stages
`keyVizRowBudgetCap+5` routes and confirms both the omitted (`""`) and
`?rows=0` forms truncate down to `keyVizRowBudgetCap`.
- [x] Existing `TestKeyVizHandlerHonorsRowsBudget` and
`TestKeyVizHandlerClampsRowsBudgetToCap` still pass — the change is
additive at the default end.
- [x] `go test -race -count=1 ./internal/admin/...` clean.
- [x] `golangci-lint run ./internal/admin/...` clean.

Author: bootjp

internal/admin/keyviz_handler.go

@@ -88,7 +88,9 @@ type KeyVizRow struct {
 //	from_unix_ms - lower bound in unix ms; 0 or omitted means unbounded
 //	               on that side (NOT the Unix epoch)
 //	to_unix_ms   - upper bound in unix ms; same 0 = unbounded contract
-//	rows         - row budget; 0 means no cap, capped at 1024 (default: 0)
+//	rows         - row budget; default and maximum is 1024 (design §4.1).
+//	               Omitted / 0 / negative all yield the cap; explicit
+//	               values above the cap are silently clamped down.
 //
 // Returns 503 codes.Unavailable when no sampler is configured so the
 // SPA can distinguish "keyviz disabled" from "no data yet" (the
@@ -166,7 +168,12 @@ type keyVizParams struct {
 }
 
 func parseKeyVizParams(r *http.Request) (keyVizParams, error) {
-	p := keyVizParams{series: keyVizDefaultSeries}
+	// rows defaults to the cap so a normal SPA poll without the
+	// query parameter still respects the budget — leaving it at the
+	// zero value would let applyKeyVizRowBudget fall through to "no
+	// cap" and return every tracked route in one payload (Codex
+	// round-3 P1 on PR #660).
+	p := keyVizParams{series: keyVizDefaultSeries, rows: keyVizRowBudgetCap}
 	q := r.URL.Query()
 	if err := setKeyVizSeriesParam(&p, q.Get("series")); err != nil {
 		return keyVizParams{}, err
@@ -209,13 +216,17 @@ func setKeyVizTimeParam(dst *time.Time, name, raw string) error {
 
 func setKeyVizRowsParam(dst *int, raw string) error {
 	if raw == "" {
+		// Caller pre-set dst to the default cap; preserve it.
 		return nil
 	}
 	n, err := strconv.Atoi(raw)
-	if err != nil || n < 0 {
-		return errors.New("rows must be a non-negative integer")
+	if err != nil {
+		return errors.New("rows must be an integer")
 	}
-	if n > keyVizRowBudgetCap {
+	if n <= 0 || n > keyVizRowBudgetCap {
+		// Explicit 0 / negative / above-cap all collapse to the cap
+		// (same as omitting the param) so callers can't disable the
+		// budget by passing pathological values.
 		n = keyVizRowBudgetCap
 	}
 	*dst = n

internal/admin/keyviz_handler_test.go

@@ -291,6 +291,69 @@ func TestKeyVizHandlerRowsBudgetTieBreakDeterministic(t *testing.T) {
 	}
 }
 
+// TestKeyVizHandlerOmittedRowsAppliesDefaultCap pins Codex round-3 P1
+// on PR #660: when the SPA polls without ?rows=, the handler must
+// still apply the keyVizRowBudgetCap default — leaving p.rows at
+// zero would let applyKeyVizRowBudget fall through to "no cap" and
+// return every tracked route in one payload.
+func TestKeyVizHandlerOmittedRowsAppliesDefaultCap(t *testing.T) {
+	t.Parallel()
+	srv := newKeyVizTestServer(t, &fakeKeyVizSource{cols: []keyviz.MatrixColumn{
+		{At: time.Unix(1_700_000_000, 0), Rows: stagedRowsForBudgetTest()},
+	}})
+	defer srv.Close()
+
+	for _, query := range []string{"", "?rows=0", "?rows=-1"} {
+		resp := keyVizGet(t, srv.URL+query)
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		var matrix KeyVizMatrix
+		require.NoError(t, json.NewDecoder(resp.Body).Decode(&matrix))
+		require.NoError(t, resp.Body.Close())
+		require.Len(t, matrix.Rows, keyVizRowBudgetCap,
+			"omitted/0/negative rows must apply the default cap (query=%q)", query)
+	}
+}
+
+// TestKeyVizHandlerClampsRowsBudgetToCap pins the above-cap branch of
+// setKeyVizRowsParam: an explicit rows= value greater than
+// keyVizRowBudgetCap must be silently clamped down so callers cannot
+// bypass the resource guard by asking for more rows than the cap.
+func TestKeyVizHandlerClampsRowsBudgetToCap(t *testing.T) {
+	t.Parallel()
+	srv := newKeyVizTestServer(t, &fakeKeyVizSource{cols: []keyviz.MatrixColumn{
+		{At: time.Unix(1_700_000_000, 0), Rows: stagedRowsForBudgetTest()},
+	}})
+	defer srv.Close()
+
+	resp := keyVizGet(t, srv.URL+"?rows=9999")
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	var matrix KeyVizMatrix
+	require.NoError(t, json.NewDecoder(resp.Body).Decode(&matrix))
+	require.NoError(t, resp.Body.Close())
+	require.Len(t, matrix.Rows, keyVizRowBudgetCap,
+		"rows=9999 must clamp down to keyVizRowBudgetCap")
+}
+
+// stagedRowsForBudgetTest builds keyVizRowBudgetCap+5 distinct rows so
+// any test that exercises the budget cap can confirm truncation
+// occurred. The loop counter is uint64 to avoid an int→uint64
+// conversion that would need a gosec suppression; Start / End encode
+// the index as a 2-byte big-endian key.
+func stagedRowsForBudgetTest() []keyviz.MatrixRow {
+	const total uint64 = keyVizRowBudgetCap + 5
+	rows := make([]keyviz.MatrixRow, total)
+	for i := uint64(0); i < total; i++ {
+		n := i + 1
+		rows[i] = keyviz.MatrixRow{
+			RouteID: n,
+			Start:   []byte{byte(i >> 8), byte(i)},
+			End:     []byte{byte(n >> 8), byte(n)},
+			Writes:  n,
+		}
+	}
+	return rows
+}
+
 // TestKeyVizHandlerTimeBoundsParam exercises the from_unix_ms /
 // to_unix_ms query parameters: a non-zero pair filters columns to the
 // requested half-open window, while 0 means "unbounded on that side"