keyviz admin: doc + aggregate fallback + time-bounds tests

Round-2 review fixes for PR #660 (Claude bot):

- buildAPIMux layout comment was missing the keyviz route entry —
  add it next to the s3 routes so the documented surface matches
  the actual mux dispatch.
- KeyVizHandler godoc now spells out that from_unix_ms / to_unix_ms
  treat 0 (or omitted) as "unbounded on that side" — NOT the Unix
  epoch. The behaviour was internally consistent with
  ringBuffer.Range but a caller could reasonably read 0 as 1970.
- newKeyVizRowFrom now has a defensive fallback for an aggregate
  row with MemberRoutesTotal == 0: render route_count =
  len(MemberRoutes) instead of 0, so a virtual bucket never
  reports "0 routes" to the SPA. Should never happen with the
  current sampler, but the guard costs nothing.

Tests:
- TestKeyVizHandlerTimeBoundsParam — exercises a real time pair
  (parsed and forwarded to Snapshot via a capturingKeyVizSource),
  the 0 → unbounded contract, and the non-numeric → 400
  invalid_query branch.
- TestKeyVizHandlerAggregateFallbackWhenTotalZero — pins the
  aggregate-row defensive fallback.

Author: bootjp

internal/admin/keyviz_handler.go

@@ -85,8 +85,9 @@ type KeyVizRow struct {
 // Query parameters (all optional):
 //
 //	series      - reads | writes | read_bytes | write_bytes (default: writes)
-//	from_unix_ms - lower bound in unix ms (default: unbounded)
-//	to_unix_ms   - upper bound in unix ms (default: unbounded)
+//	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)
 //
 // Returns 503 codes.Unavailable when no sampler is configured so the
@@ -298,8 +299,19 @@ func keyVizSeriesPicker(series KeyVizSeries) func(keyviz.MatrixRow) uint64 {
 
 func newKeyVizRowFrom(mr keyviz.MatrixRow, numCols int) *KeyVizRow {
 	total := mr.MemberRoutesTotal
-	if !mr.Aggregate && total == 0 {
+	switch {
+	case !mr.Aggregate && total == 0:
+		// Individual slots with the field zero-initialised — every
+		// real route contributes exactly one member to itself.
 		total = 1
+	case mr.Aggregate && total == 0:
+		// Defensive fallback: a virtual bucket should always carry a
+		// non-zero MemberRoutesTotal once foldIntoBucket has run, but
+		// if a sampler ever serialises a just-coalesced bucket before
+		// the count is set the SPA would render "0 routes" — which is
+		// nonsense for an aggregate row. Fall back to the visible
+		// MemberRoutes length so route_count stays meaningful.
+		total = uint64(len(mr.MemberRoutes))
 	}
 	row := &KeyVizRow{
 		BucketID:          bucketIDFor(mr),

internal/admin/keyviz_handler_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"testing"
 	"time"
 
@@ -156,6 +157,8 @@ func TestKeyVizHandlerSeriesParam(t *testing.T) {
 		{"write_bytes", 4444},
 	} {
 		t.Run(tc.series, func(t *testing.T) {
+			// Sequential: the parent's `defer srv.Close()` would fire
+			// before parallel subtests get to dial.
 			resp := keyVizGet(t, srv.URL+"?series="+tc.series)
 			defer resp.Body.Close()
 			require.Equal(t, http.StatusOK, resp.StatusCode)
@@ -287,3 +290,100 @@ func TestKeyVizHandlerRowsBudgetTieBreakDeterministic(t *testing.T) {
 		require.Equal(t, "route:2", matrix.Rows[1].BucketID, "iteration %d", i)
 	}
 }
+
+// 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"
+// (NOT the Unix epoch). The fakeKeyVizSource here does not actually
+// honour the bounds (its Snapshot ignores them) — what we're pinning
+// is the parse/dispatch contract: a parsable pair must yield 200,
+// 0 must reach the source as the zero Time, and a non-numeric value
+// must surface as 400 invalid_query.
+func TestKeyVizHandlerTimeBoundsParam(t *testing.T) {
+	t.Parallel()
+	captured := &capturingKeyVizSource{}
+	h := NewKeyVizHandler(captured).WithClock(func() time.Time {
+		return time.Unix(1_700_000_000, 0).UTC()
+	})
+	srv := httptest.NewServer(h)
+	defer srv.Close()
+
+	from := time.Unix(1_699_000_000, 0)
+	to := time.Unix(1_700_500_000, 0)
+	u := srv.URL + "?from_unix_ms=" + strconv.FormatInt(from.UnixMilli(), 10) +
+		"&to_unix_ms=" + strconv.FormatInt(to.UnixMilli(), 10)
+	resp := keyVizGet(t, u)
+	resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	require.True(t, captured.from.Equal(from.UTC()), "from = %v, want %v", captured.from, from.UTC())
+	require.True(t, captured.to.Equal(to.UTC()), "to = %v, want %v", captured.to, to.UTC())
+
+	// 0 → unbounded (zero Time), not Unix epoch.
+	captured.reset()
+	resp = keyVizGet(t, srv.URL+"?from_unix_ms=0&to_unix_ms=0")
+	resp.Body.Close()
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	require.True(t, captured.from.IsZero(), "from = %v, want zero", captured.from)
+	require.True(t, captured.to.IsZero(), "to = %v, want zero", captured.to)
+
+	// Non-numeric → 400 invalid_query.
+	resp = keyVizGet(t, srv.URL+"?from_unix_ms=notanumber")
+	defer resp.Body.Close()
+	require.Equal(t, http.StatusBadRequest, resp.StatusCode)
+	var body map[string]string
+	require.NoError(t, json.NewDecoder(resp.Body).Decode(&body))
+	require.Equal(t, "invalid_query", body["error"])
+}
+
+// capturingKeyVizSource records the from/to bounds the handler
+// forwarded so tests can assert on the parse step without rendering
+// any matrix data.
+type capturingKeyVizSource struct {
+	from time.Time
+	to   time.Time
+}
+
+func (c *capturingKeyVizSource) Snapshot(from, to time.Time) []keyviz.MatrixColumn {
+	c.from = from
+	c.to = to
+	return nil
+}
+
+func (c *capturingKeyVizSource) reset() {
+	c.from = time.Time{}
+	c.to = time.Time{}
+}
+
+// TestKeyVizHandlerAggregateFallbackWhenTotalZero pins the defensive
+// fallback for the unlikely case where the sampler emits an aggregate
+// row with MemberRoutesTotal == 0: the handler must not serialise
+// route_count = 0 (which the SPA would render as "0 routes" — nonsense
+// for a virtual bucket). Instead it falls back to len(MemberRoutes).
+func TestKeyVizHandlerAggregateFallbackWhenTotalZero(t *testing.T) {
+	t.Parallel()
+	srv := newKeyVizTestServer(t, &fakeKeyVizSource{cols: []keyviz.MatrixColumn{
+		{
+			At: time.Unix(1_700_000_000, 0),
+			Rows: []keyviz.MatrixRow{
+				{
+					RouteID:           ^uint64(0),
+					Start:             []byte("c"),
+					End:               []byte("d"),
+					Aggregate:         true,
+					MemberRoutes:      []uint64{2, 3},
+					MemberRoutesTotal: 0, // pathological zero
+					Writes:            5,
+				},
+			},
+		},
+	}})
+	defer srv.Close()
+
+	resp := keyVizGet(t, srv.URL)
+	defer resp.Body.Close()
+	var matrix KeyVizMatrix
+	require.NoError(t, json.NewDecoder(resp.Body).Decode(&matrix))
+	require.Len(t, matrix.Rows, 1)
+	require.True(t, matrix.Rows[0].Aggregate)
+	require.Equal(t, uint64(2), matrix.Rows[0].RouteCount, "fallback to len(MemberRoutes)")
+}

internal/admin/server.go

@@ -208,6 +208,7 @@ func (s *Server) APIHandler() http.Handler {
 //	DELETE /admin/api/v1/dynamo/tables/{name}       (auth required, full role)
 //	GET    /admin/api/v1/s3/buckets                 (auth required)
 //	GET    /admin/api/v1/s3/buckets/{name}          (auth required)
+//	GET    /admin/api/v1/keyviz/matrix              (auth required)
 //
 // Body limit applies uniformly. CSRF and Audit middleware apply to
 // write-capable protected endpoints; login and logout carry their own