feat(admin): expose keyviz heatmap via /admin/api/v1/keyviz/matrix#660
feat(admin): expose keyviz heatmap via /admin/api/v1/keyviz/matrix#660
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 49 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request introduces a new KeyViz HTTP API endpoint at Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant KeyVizHandler
participant KeyVizSource
participant JSONEncoder
Client->>KeyVizHandler: GET /admin/api/v1/keyviz/matrix?series=writes&rows=100
alt Source is nil
KeyVizHandler-->>Client: 503 {error: keyviz_disabled}
else Source configured
KeyVizHandler->>KeyVizHandler: Validate method, parse params
KeyVizHandler->>KeyVizSource: Snapshot(from, to)
KeyVizSource-->>KeyVizHandler: []MatrixColumn (column-major)
KeyVizHandler->>KeyVizHandler: Pivot to row-major format
KeyVizHandler->>KeyVizHandler: Sort & truncate rows by activity
KeyVizHandler->>KeyVizHandler: Construct BucketIDs & aggregate
KeyVizHandler->>JSONEncoder: KeyVizMatrix
JSONEncoder-->>KeyVizHandler: JSON bytes
KeyVizHandler-->>Client: 200 JSON response<br/>(Cache-Control: no-store)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the KeyVizHandler and wires it into the admin server to expose heatmap matrix data via a new API endpoint. The changes include logic for pivoting data, parsing query parameters, and applying row-based truncation. Review feedback recommends optimizing the row-budgeting logic by pre-calculating activity totals to improve sorting efficiency and adding a tie-breaker to ensure deterministic results.
| type KeyVizRow struct { | ||
| BucketID string `json:"bucket_id"` | ||
| Start []byte `json:"start"` | ||
| End []byte `json:"end"` | ||
| Aggregate bool `json:"aggregate"` | ||
| RouteIDs []uint64 `json:"route_ids,omitempty"` | ||
| RouteIDsTruncated bool `json:"route_ids_truncated,omitempty"` | ||
| RouteCount uint64 `json:"route_count"` | ||
| Values []uint64 `json:"values"` | ||
| } |
There was a problem hiding this comment.
The KeyVizRow struct is used for JSON serialization but also serves as the intermediate object for sorting and truncation. To improve the performance of applyKeyVizRowBudget, consider adding a Total field with a json:"-" tag. This allows pre-calculating the activity sum during the pivot phase, avoiding redundant O(M) iterations over the Values slice during the O(N log N) sort comparisons.
| type KeyVizRow struct { | |
| BucketID string `json:"bucket_id"` | |
| Start []byte `json:"start"` | |
| End []byte `json:"end"` | |
| Aggregate bool `json:"aggregate"` | |
| RouteIDs []uint64 `json:"route_ids,omitempty"` | |
| RouteIDsTruncated bool `json:"route_ids_truncated,omitempty"` | |
| RouteCount uint64 `json:"route_count"` | |
| Values []uint64 `json:"values"` | |
| } | |
| type KeyVizRow struct { | |
| BucketID string "json:\"bucket_id\"" | |
| Start []byte "json:\"start\"" | |
| End []byte "json:\"end\"" | |
| Aggregate bool "json:\"aggregate\"" | |
| RouteIDs []uint64 "json:\"route_ids,omitempty\"" | |
| RouteIDsTruncated bool "json:\"route_ids_truncated,omitempty\"" | |
| RouteCount uint64 "json:\"route_count\"" | |
| Values []uint64 "json:\"values\"" | |
| // Total is the sum of Values, used for activity-based truncation. | |
| Total uint64 "json:\"-\"" | |
| } |
| rowsByID[mr.RouteID] = row | ||
| order = append(order, mr.RouteID) | ||
| } | ||
| row.Values[j] = pick(mr) |
There was a problem hiding this comment.
Accumulate the total activity for the row here to avoid re-calculating it during the sorting phase.
val := pick(mr)
row.Values[j] = val
row.Total += val| sort.Slice(rows, func(i, j int) bool { | ||
| return rowActivityTotal(rows[i]) > rowActivityTotal(rows[j]) | ||
| }) |
There was a problem hiding this comment.
The current sorting logic in applyKeyVizRowBudget is inefficient because it calls rowActivityTotal (an O(M) operation) inside the sort comparator (O(N log N) calls). Additionally, it lacks a tie-breaker, which makes the truncation non-deterministic when multiple rows have the same total activity. This can lead to inconsistent UI states upon refresh. Using a pre-calculated Total field and adding BucketID as a tie-breaker ensures both performance and determinism.
sort.Slice(rows, func(i, j int) bool {
if rows[i].Total != rows[j].Total {
return rows[i].Total > rows[j].Total
}
return rows[i].BucketID < rows[j].BucketID
})References
- Avoid silently dropping entries or using non-deterministic normalization to ensure state consistency and prevent UI inconsistencies.
- Apply fixed bounds to collections that grow from external requests to prevent unbounded memory growth.
|
@claude review |
Round-1 review fixes for PR #660 (Gemini medium): The rows-budget sort previously called rowActivityTotal inside the comparator, paying O(M) per pair × O(N log N) comparisons — quadratic-ish in column count for the cost of one HTTP request. Worse, the lack of a tie-breaker made truncation non-deterministic: two refreshes against identical data could return different row sets, surfacing as flicker in the SPA heatmap. Accumulate a precomputed row.total during the pivot phase (json:"-" field, internal only) and sort on it in the budget step. Add BucketID as the tie-breaker so identical data → identical response. Test TestKeyVizHandlerRowsBudgetTieBreakDeterministic stages three routes with equal Writes, requests rows=2 three times in a row, and asserts the same two routes (lowest BucketID) survive every time.
bootjp
force-pushed
the
feat/keyviz-admin-binary-handler
branch
from
50d38d0 to
5237296
Compare
|
Claude finished @bootjp's task in 3m 2s —— View job Code Review
Overall the implementation is clean and well-structured. The pivot logic mirrors the gRPC adapter correctly, the nil-source → 503 path is load-bearing and handled properly, and the Gemini round-1 suggestions (pre-computed Medium priority1. Stale route layout comment in The layout comment documents all registered routes but is missing the new one: Minor doc drift, but this comment is the only place the full route table is described in one place. 2. Undocumented zero-value behavior of if n == 0 {
return time.Time{}, nil
}
3. total := mr.MemberRoutesTotal
if !mr.Aggregate && total == 0 {
total = 1 // correct: non-aggregate rows always have 1 route
}The non-aggregate guard is correct, but an aggregate row with Low priority4. No test coverage for All seven handler tests use zero-time bounds. The 5. The parent test is parallel but the table-driven inner loop isn't. Low impact but inconsistent with the rest of the file. Already addressed (Gemini round-1)The PR already implements what Gemini requested:
Positive observations
|
Bridges the in-process keyviz.MemSampler into the in-node admin HTTP listener (Layer C) so the SPA can render the heatmap without an extra hop through the gRPC Admin service. The SPA was merged in PR #649 but has no KeyViz page yet — this PR is the backend half; the SPA page is task B in the design's Phase 2 follow-up. - New KeyVizSource interface in internal/admin (Snapshot only) so *keyviz.MemSampler satisfies it without a wrapper. Tests pass an in-memory fake. - KeyVizHandler serves GET /admin/api/v1/keyviz/matrix. Query parameters: series (default writes per design §4.1), from_unix_ms, to_unix_ms, rows (capped at keyVizRowBudgetCap=1024 per design §4.1). - Returns 503 keyviz_disabled when the source is nil so the SPA renders an explicit "feature off" state instead of an empty matrix. - Pivots column-major MatrixColumn into row-major JSON, mirroring the adapter (gRPC) handler from PR #646. The duplication is intentional for now — extracting a shared pivot helper is a future cleanup. - ServerDeps.KeyViz field; buildAPIMux always registers the route (even when source is nil) so 503 is served instead of 404. - main.go threads *keyviz.MemSampler through startAdminFromFlags → startAdminServer → buildAdminHTTPServer; keyvizSourceFromSampler converts the typed-nil to interface-nil so the handler's "is keyviz disabled" check fires correctly. Tests: - TestKeyVizHandlerReturnsServiceUnavailableWhenNoSource - TestKeyVizHandlerRejectsNonGet - TestKeyVizHandlerPivotsMatrix (two-column, two-route fixture) - TestKeyVizHandlerSeriesParam (table-driven across all four enums) - TestKeyVizHandlerSeriesParamRejectsUnknown (400 invalid_query) - TestKeyVizHandlerHonorsRowsBudget - TestKeyVizHandlerEncodesAggregateBucket
Round-1 review fixes for PR #660 (Gemini medium): The rows-budget sort previously called rowActivityTotal inside the comparator, paying O(M) per pair × O(N log N) comparisons — quadratic-ish in column count for the cost of one HTTP request. Worse, the lack of a tie-breaker made truncation non-deterministic: two refreshes against identical data could return different row sets, surfacing as flicker in the SPA heatmap. Accumulate a precomputed row.total during the pivot phase (json:"-" field, internal only) and sort on it in the budget step. Add BucketID as the tie-breaker so identical data → identical response. Test TestKeyVizHandlerRowsBudgetTieBreakDeterministic stages three routes with equal Writes, requests rows=2 three times in a row, and asserts the same two routes (lowest BucketID) survive every time.
bootjp
force-pushed
the
feat/keyviz-admin-binary-handler
branch
from
5237296 to
25ac777
Compare
|
@claude review |
|
Claude finished @bootjp's task in 2m 16s —— View job Claude finished @bootjp's task — Code Review
The implementation is clean and well-structured. I've reviewed it against all five CLAUDE.md lenses. Summary upfront: no data-loss or correctness blockers; a few medium-priority issues and one new finding. Lens 1 — Data lossNo writes in this path. The handler is GET-only; the 405 guard ( No issues. Lens 2 — Concurrency / distributed failures
No issues. Lens 3 — Performance
No issues. Lens 4 — Data consistencyMedium: The layout comment documents all registered routes but is missing the new one. It's the only place the full route table is described in one place, so drift here misleads operators reading the source. Medium: if n == 0 {
return time.Time{}, nil // treated as "unbounded" — same as omitting the param
}
Medium: Aggregate row with total := mr.MemberRoutesTotal
if !mr.Aggregate && total == 0 {
total = 1 // correct for non-aggregate rows
}The non-aggregate guard is correct. But an aggregate row with if mr.Aggregate && total == 0 {
total = uint64(len(mr.MemberRoutes))
}Lens 5 — Test coverageLow: No test coverage for All seven handler tests use zero-time bounds; the time-range filtering code path in Low: The parent is Positive observations
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/admin/keyviz_handler.go (3)
261-274: Per-row metadata is frozen from the first column the route appears in.
Start,End,Aggregate,RouteCount, andRouteIDsTruncatedare populated bynewKeyVizRowFrom(mr, ...)only on the first sighting ofmr.RouteID. If the sameRouteIDappears in a later column with a changed range (e.g. a split/merge mid-window) or, for an aggregate bucket, a differentMemberRoutesTotal/MemberRoutes, those updates are silently dropped — onlyValues[j]andtotalcontinue to track. In practice route identity is stable within a sampling window so this is fine, but it's a subtle behavioral choice worth a short comment so a future "why is the matrix showing the old range?" debug session is shorter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/keyviz_handler.go` around lines 261 - 274, Per-row metadata (Start, End, Aggregate, RouteCount, RouteIDsTruncated) is only set when a route first appears by newKeyVizRowFrom and later sightings only update Values[j] and total, so add a short explanatory comment in the loop (near the rowsByID lookup inside the for j, col := range cols block) clarifying that route-level metadata is intentionally frozen on first sighting (or, if you prefer to keep metadata mutable, update the existing row's Start/End/Aggregate/RouteCount/RouteIDsTruncated when mr differs from the stored row), referencing newKeyVizRowFrom, rowsByID, pick, matrix.ColumnUnixMs and order so future readers know why only Values and total are updated.
284-297: Minor:defaultarm is unreachable.
pivotKeyVizColumnsis only ever called afterparseKeyVizSeriesvalidates the enum, so thedefaultcase is dead code. Consider folding it intokeyVizSeriesWritesor panicking with"unreachable: " + string(series)to make the invariant explicit and silence any future "I added a series and forgot to update the picker" drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/keyviz_handler.go` around lines 284 - 297, The default arm in keyVizSeriesPicker is unreachable because parseKeyVizSeries already validates KeyVizSeries; update keyVizSeriesPicker to make this invariant explicit by replacing the default return with a panic (e.g., panic(fmt.Sprintf("unreachable: %v", series))) or by removing the default and returning the keyVizSeriesWrites handler directly; ensure you reference the KeyVizSeries enum and keep the function signature of keyVizSeriesPicker unchanged so callers (e.g., pivotKeyVizColumns) are unaffected.
167-183: Optional: tightenfrom_unix_ms/to_unix_msvalidation.
strconv.ParseInt(raw, 10, 64)accepts negative values, so e.g.?from_unix_ms=-1is silently turned into a pre-epochtime.Timerather than a400 invalid_query. Likewise there is nofrom <= tocheck, so a reversed range just produces an empty matrix indistinguishable from "no activity." Both surface as confusing-looking200 OKresponses to a misbehaving client; rejecting them keeps the wire contract crisp.♻️ Suggested validation tweaks
func parseUnixMs(name, raw string) (time.Time, error) { n, err := strconv.ParseInt(raw, 10, 64) if err != nil { return time.Time{}, errors.New(name + " must be an integer (unix milliseconds)") } + if n < 0 { + return time.Time{}, errors.New(name + " must be non-negative") + } if n == 0 { return time.Time{}, nil } return time.UnixMilli(n).UTC(), nil }And after both bounds are parsed in
parseKeyVizParams:if err := setKeyVizTimeParam(&p.to, "to_unix_ms", q.Get("to_unix_ms")); err != nil { return keyVizParams{}, err } + if !p.from.IsZero() && !p.to.IsZero() && p.from.After(p.to) { + return keyVizParams{}, errors.New("from_unix_ms must be <= to_unix_ms") + }Also applies to: 233-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/keyviz_handler.go` around lines 167 - 183, Reject negative/unix-ms timestamps and enforce from<=to: modify setKeyVizTimeParam so it treats parsed int64 < 0 as an error (returning the same invalid_query-style error) instead of producing a pre-epoch time, and then in parseKeyVizParams (after calling setKeyVizTimeParam for p.from and p.to) add a check that p.from <= p.to and return an invalid_query error if not; apply the same negative-value and range check fixes to the other time-parsing block in this file that mirrors these calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/admin/keyviz_handler.go`:
- Around line 261-274: Per-row metadata (Start, End, Aggregate, RouteCount,
RouteIDsTruncated) is only set when a route first appears by newKeyVizRowFrom
and later sightings only update Values[j] and total, so add a short explanatory
comment in the loop (near the rowsByID lookup inside the for j, col := range
cols block) clarifying that route-level metadata is intentionally frozen on
first sighting (or, if you prefer to keep metadata mutable, update the existing
row's Start/End/Aggregate/RouteCount/RouteIDsTruncated when mr differs from the
stored row), referencing newKeyVizRowFrom, rowsByID, pick, matrix.ColumnUnixMs
and order so future readers know why only Values and total are updated.
- Around line 284-297: The default arm in keyVizSeriesPicker is unreachable
because parseKeyVizSeries already validates KeyVizSeries; update
keyVizSeriesPicker to make this invariant explicit by replacing the default
return with a panic (e.g., panic(fmt.Sprintf("unreachable: %v", series))) or by
removing the default and returning the keyVizSeriesWrites handler directly;
ensure you reference the KeyVizSeries enum and keep the function signature of
keyVizSeriesPicker unchanged so callers (e.g., pivotKeyVizColumns) are
unaffected.
- Around line 167-183: Reject negative/unix-ms timestamps and enforce from<=to:
modify setKeyVizTimeParam so it treats parsed int64 < 0 as an error (returning
the same invalid_query-style error) instead of producing a pre-epoch time, and
then in parseKeyVizParams (after calling setKeyVizTimeParam for p.from and p.to)
add a check that p.from <= p.to and return an invalid_query error if not; apply
the same negative-value and range check fixes to the other time-parsing block in
this file that mirrors these calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b6b82b9-10c3-4db3-ae35-5f39b9afad65
📒 Files selected for processing (6)
internal/admin/keyviz_handler.gointernal/admin/keyviz_handler_test.gointernal/admin/server.gomain.gomain_admin.gomain_admin_test.go
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.
|
@claude review |
|
Claude finished @bootjp's task in 2m 29s —— View job Code Review
All medium-priority issues from the two prior rounds have been addressed in commits Resolved since last round ✅
Still open (CodeRabbit nitpicks)1.
if n < 0 {
return time.Time{}, errors.New(name + " must be non-negative")
}And after parsing both bounds in if !p.from.IsZero() && !p.to.IsZero() && p.from.After(p.to) {
return keyVizParams{}, errors.New("from_unix_ms must be <= to_unix_ms")
}2.
default:
panic("unreachable: keyVizSeriesPicker called with invalid series: " + string(series))3. Per-row metadata silently frozen on first column sighting (
// Route-level metadata (Start/End/Aggregate/RouteCount) is frozen
// on first sighting; route identity is stable within a window.Lens summary
This is ready to merge. The three remaining items are all one-liners and can land in a follow-up if preferred. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7db3c8746c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if raw == "" { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
rows is absent
When the query omits rows, this branch leaves p.rows at its zero value, and later applyKeyVizRowBudget treats budget <= 0 as "no cap," so /admin/api/v1/keyviz/matrix returns every tracked row instead of respecting the intended 1024-row limit. In practice, a normal SPA poll without rows can trigger large sorts and oversized payloads (up to all tracked routes), which undermines the endpoint’s resource guard.
Useful? React with 👍 / 👎.
|
@claude review |
|
Claude finished @bootjp's task in 2m 19s —— View job Code Review
This is the third review round. Two previous rounds caught and confirmed fixes for the major issues. Reviewing the current merged state against all five CLAUDE.md lenses. What's already in good shape ✅
Still open — follow-up itemsP1: Default row budget bypasses This is the Codex P1 item. When The constant's own comment says "SPA cannot force unbounded payloads via the JSON path" and the PR description says "capped at 1024 per design §4.1." The current behavior contradicts both. The godoc for the Fix: in if p.rows == 0 {
p.rows = keyVizRowBudgetCap
}And update the godoc: Medium:
if n < 0 {
return time.Time{}, errors.New(name + " must be non-negative")
}Bonus: a Minor:
Replace with a panic to make the invariant explicit: default:
panic("unreachable: keyVizSeriesPicker called with invalid series: " + string(series))Minor: Per-row metadata freeze is undocumented (
// Route-level metadata (Start/End/Aggregate/RouteCount) is frozen on
// first sighting; route identity is stable within a sampling window.Lens summary
The P1 (default |
…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.
## Summary Phase 2-B of the Key Visualizer design (`docs/admin_ui_key_visualizer_design.md`): integrate the heatmap into the existing `web/admin/` SPA rather than building a parallel dashboard. Doc + implementation in one PR (doc commit first, per CLAUDE.md design-doc-first workflow). - New page at `/keyviz` — canvas heatmap polling `/admin/api/v1/keyviz/matrix` with series picker (writes / reads / write_bytes / read_bytes), row-budget input (clamped at 1024), and off / 5 s / 30 s auto-refresh. - Cold cells (value 0) render as the page background, not a faint blue — spotting actually-cold routes stays the dominant visual signal. - Row-detail flyout on hover: bucket_id, start, end, aggregate, route_count, route_ids (with truncation indicator). `start` / `end` decode through a printable-or-hex preview so binary keys do not render as mojibake. - Dependency-free: hand-rolled five-stop colour ramp in `lib/colorRamp.ts`. No d3, no ECharts. Bundle grew from ~155 kB to ~208 kB (raw), 64 kB gzipped. Backend is unchanged. The handler at `/admin/api/v1/keyviz/matrix` and its sampler wiring already shipped under Phase 2-A (PRs #639 / #645 / #646 / #647 / #651 / #660 / #661 / #672). ## Design `docs/design/2026_04_27_proposed_keyviz_spa_integration.md` — proposed status, lands with this PR. Parent design §12 phase table is split into 2-A (server, shipped) / 2-B (SPA, this PR) / 2-C (cluster fan-out, open). ## Five-lens self-review 1. **Data loss** — n/a; SPA is read-only against an existing handler. 2. **Concurrency / distributed** — n/a; single browser tab polling a single handler. Sampler concurrency was already covered by Phase 2-A tests. 3. **Performance** — Canvas + per-cell `fillRect` runs under §10 budget at 1024 × 500. Auto-refresh defaults off; 5-second cadence is the lower bound (sampler flush is 1 s). 4. **Data consistency** — SPA renders whatever the handler returns; consistency guarantees come from the existing leader-issued counters in the sampler. 5. **Test coverage** — `tsc -b --noEmit` clean; `vite build` clean; `go build ./internal/admin/...` clean (embed glob unaffected); `go test ./internal/admin/...` clean. Manual verification documented in the design doc §5. ## Test plan - [x] `npm run lint` (`tsc -b --noEmit`) — clean - [x] `npm run build` (Vite) — clean, output goes to `internal/admin/dist` - [x] `go build ./internal/admin/...` — clean - [x] `go test ./internal/admin/...` — clean - [ ] Manual: `make run` + `make client`, navigate to `/keyviz`, see hot routes light up red within ~5 s of write traffic - [ ] Manual: series picker swaps the displayed counter; row-budget input clamps at 1024; auto-refresh polls without flicker ## Out of scope - **Cluster fan-out** — handler is currently node-local. Phase 2-C will add a cross-node admin RPC; this PR will pick up the aggregate view automatically once that ships. - **Drill-down per-route sparkline** — Phase 3. - **Routes / Raft Groups correlation** — Phase 1 SPA pages not yet built; correlation lands when those pages do. - **`localStorage` for series / rows / refresh** — punt to follow-up.
1 participant
Summary
Bridges the in-process
keyviz.MemSamplerinto the in-node admin HTTP listener (Layer C,internal/admin) so the SPA can render the heatmap without an extra hop through the gRPC Admin service. The SPA was merged in #649 but has no KeyViz page yet — this PR is the backend half; the SPA page will follow as a separate slice.KeyVizSourceinterface ininternal/admin(Snapshot only) so*keyviz.MemSamplersatisfies it without a wrapper. Tests pass an in-memory fake.KeyVizHandlerservesGET /admin/api/v1/keyviz/matrix. Query parameters:series(defaultwritesper design §4.1),from_unix_ms,to_unix_ms,rows(capped atkeyVizRowBudgetCap=1024per design §4.1).503 keyviz_disabledwhen the source is nil so the SPA renders an explicit "feature off" state instead of an empty matrix.MatrixColumninto row-major JSON, mirroring the adapter (gRPC) handler from PR feat(adapter): expose keyviz heatmap via AdminServer.GetKeyVizMatrix #646. The duplication is intentional for now — extracting a shared pivot helper is a future cleanup.ServerDeps.KeyVizfield;buildAPIMuxalways registers the route (even when source is nil) so503is served instead of404.main.gothreads*keyviz.MemSamplerthroughstartAdminFromFlags→startAdminServer→buildAdminHTTPServer;keyvizSourceFromSamplerconverts the typed-nil to interface-nil so the handler's "is keyviz disabled" check fires correctly.Test plan
TestKeyVizHandlerReturnsServiceUnavailableWhenNoSource— 503 +keyviz_disabledcode.TestKeyVizHandlerRejectsNonGet— 405.TestKeyVizHandlerPivotsMatrix— two-column, two-route fixture; verifies missing-row-becomes-zero contract.TestKeyVizHandlerSeriesParam— table-driven across all four enum values.TestKeyVizHandlerSeriesParamRejectsUnknown— 400invalid_queryfor typo'd series.TestKeyVizHandlerHonorsRowsBudget—?rows=2returns top-2 by activity, sorted by Start.TestKeyVizHandlerEncodesAggregateBucket— virtual-bucket layout withroute_countfromMemberRoutesTotalandroute_ids_truncatedflag.go test ./internal/admin/... .clean.golangci-lint run . ./internal/admin/...clean.Summary by CodeRabbit
/admin/api/v1/keyviz/matrixAPI endpoint for retrieving key visualization matrix data