test: fail on render-phase update warnings; smoke-render every registry metric

[kylebernhardy]

Summary

Test-infrastructure only — turns the two most valuable findings from the recent runtime-verification (Playwright) passes into permanent suite guards:

1. Render-phase update tripwire (src/testSetup/failOnRenderPhaseUpdate.ts, registered globally): any test during which React logs Cannot update a component while rendering a different component now fails, with the actual component names substituted into the failure message. This defect class has shipped silently twice (the router-rebuild Transitioner warning, and the analytics freshness watcher just fixed in fix(status): stop the freshness watcher updating TimeRangePicker during sibling renders #1510) — the suite stayed green both times while production consoles/RUM filled with warnings. The match is deliberately narrow (that one React message, not a blanket console.error ban), so intentional error-logging paths (PanelErrorBoundary, the query-cache toast handler) are unaffected. Zero offenders in the current suite (220 files / 1529 tests run clean), so there's no ordering dependency on other open PRs.

2. Registry-wide panel smoke test (registry-smoke.test.tsx): every specRegistry metric renders through the real MetricRenderer dispatch inside the real PanelErrorBoundary, fed synthetic records derived from each spec's own required fields (plus quantile columns, confidence-gate-clearing counts, and id-mirrored timestamps). Two tiers: no metric may trip the boundary or the render-failed fallback, and every line/stacked-area metric — derived from spec.primitive, not a hardcoded list, so new metrics are automatically held to it — must emit an actual chart, not just its empty state. This is the unit-test form of "do the metrics panels actually load?", the class the recent shim-deletion sweep had to browser-verify.

Where to focus

• The tripwire's failure mechanism is an afterEach throw. If a test both fails an assertion and trips the wire, vitest reports the hook failure — it can shadow the primary assertion in the output (both are real failures; you fix the same code). Accepted trade-off, flagging for awareness.
• The offenders accumulator is not safe under test.concurrent, which this repo doesn't use.
• The synthetic-record generator was validated in both directions during development: it initially rendered empty states for quantile specs (confidence gate) and timestamp: 'id' specs — the chart-tier assertions caught both, which is the behavior you want from it going forward.

Comment generated by kAIle (Claude Fable 5)

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	51.26%	5271 / 10282
🔵	Statements	51.83%	5646 / 10892
🔵	Functions	43.31%	1289 / 2976
🔵	Branches	45.06%	3562 / 7905

File Coverage

No changed files found.

Generated in workflow #1525 for commit a80dfec by the Vitest Coverage Report Action

[gemini-code-assist]

Code Review

This pull request introduces a registry-wide panel smoke test to verify that all registered metrics render correctly, along with a global Vitest setup to fail tests that trigger React render-phase cross-component updates. The reviewer provided valuable feedback, suggesting the use of optional chaining when reading metric specifications to avoid potential runtime errors, and recommending leveraging Vitest's TestContext in the afterEach hook to prevent render-phase update errors from masking primary test assertion failures.

[kriszyp]

Really like these two guards — the render-phase-update tripwire in particular is the right permanent net for the defect class that shipped green twice (the router Transitioner and the #1510 freshness watcher). Verified the mechanism: the afterEach throw genuinely fails the test, console.error is restored on every path (including the already-failed branch) to the true original captured in beforeEach, and the narrow 'Cannot update a component' .includes() match is robust to React's version-dependent Warning: prefix without becoming a blanket console.error ban. The smoke test's Tier-1 copy anchors ('is unavailable', 'Render failed') map exactly to the two failure surfaces, and auto-deriving MUST_RENDER_CHART from spec.primitive is the anti-staleness property that matters. All good to merge — none of the below blocks, purely follow-up material.

1. derivedRegistry isn't smoke-tested, but the framing says "every registry metric." Both tiers iterate Object.keys(specRegistry), yet MetricRenderer also dispatches derivedRegistry[metric] at the top of its body — so mqtt-traffic-sent, mqtt-traffic-received, request-rate, error-rate, and transaction-log-growth get zero coverage, and a newly added derived metric is silently uncovered. The branch name (kyle/verify-derived-test-hardening) suggests derived hardening was actually the focus, so this may be intentional scoping — if so, worth softening the header/PR wording to "every spec-driven metric." Otherwise, looping derivedRegistry too (synthetic records keyed off DERIVED_REQUIRED_FIELDS) would close the gap.

2. The chart-tier SVG selector is effectively dead. registry-smoke.test.tsx:124 — querySelector('.recharts-responsive-container, svg') returns the first match in document order, and the .recharts-responsive-container wrapper <div> is an ancestor of the <svg>, so it always matches first and the svg branch never evaluates. That reduces the assertion to "a ResponsiveContainer mounted." It's not vacuous overall — the analytics setup.ts forces 800×600 so recharts really does draw, and the companion not.toContain('No data in window') is the guard actually biting — but the selector as written doesn't verify the SVG. Collapsing to querySelector('svg') makes it bite (setup guarantees it will).

3. Neither guard directly re-proves #1510. The smoke test feeds records straight into MetricRenderer, bypassing useAnalyticsFreshness/the QueryCache subscription that produced the #1510 warning, so it doesn't exercise that path itself. The global tripwire would catch a reintroduced #1510 — but only via some other test that mounts the freshness-watcher subscription. That's the inherent limitation of console-spy tripwires (they catch what something renders); not a defect here, just worth knowing the tripwire and smoke test guard different things. A tiny targeted mount of the freshness-watcher component would close the loop.

Two minor nits if you're touching it anyway: original?.(...) on the already-failed path can just be console.error(...) (it's already restored two lines up), and MUST_RENDER_CHART vs syntheticRecords disagree on .spec vs ?.spec (both safe — spec is required — just inconsistent).

Nice work — this is exactly the kind of durable regression net worth having.

— reviewed with KrAIs (Claude Opus 4.8)

[dawsontoth]

Fresh pass on top of @kriszyp's review (which I agree with — the derived-registry gap, the dead .recharts-responsive-container, svg selector, and the "neither guard re-proves #1510 directly" point all hold). A few things I'd add that build on those, both filed as non-blocking follow-ups:

1. The tripwire is silently disabled in tests that mock console.error → #1520. It wraps console.error in beforeEach, but a test doing vi.spyOn(console, 'error').mockImplementation(() => {}) replaces that wrapper, so a render-phase warning in that test is never seen. Four files do this — and two are in the exact subsystems this guard exists for: OverviewTab.test.tsx:78 (analytics — where #1510 lived) and preloadEvictionRepro.test.ts (router — where the Transitioner warning shipped). This sharpens Kris's point #3: the tests that actually mount those render trees can't act as the tripwire's proxy, because they've silenced console.error. Cheapest fix: have the afterEach notice when console.error is no longer the wrapper it installed and surface that the net was disabled for that test.

2. The "actually renders" tier excludes the two most complex panels → #1521. MUST_RENDER_CHART is line/stacked-area only, so the heatmap (replication latency) and small-multiples (resource-usage) are no-crash-only — never checked for actually rendering data. Combined with the dead SVG selector Kris flagged, the effective guarantee is "line/stacked-area metrics don't render the LineChart empty state."

3. Minor (no issue): the tripwire depends on React's dev-mode warning. It does fire in this vitest env (confirmed — it caught the #1510 repro when I reverted that hook), but if the test runtime ever picks up a production React build the tripwire goes silently green on everything. A one-line comment noting the dependency would save a future head-scratch.

None of this blocks — agreeing with Kris that it's good-to-merge with follow-ups. Nice durable net.

[kylebernhardy]

Thanks @kriszyp and @dawsontoth for the thorough review — really appreciate the catches. Pushed a80dfec addressing every point:

registry-smoke.test.tsx

• Dead SVG selector (kriszyp). Great catch — .recharts-responsive-container is an ancestor of the <svg>, so .recharts-responsive-container, svg always matched the wrapper first and the svg branch never evaluated. Now asserts container.querySelector('svg') directly; the analytics __tests__/setup.ts forces an 800×600 viewport so recharts genuinely draws. Kept the companion not.toContain('No data in window').
• .spec vs ?.spec consistency (kriszyp). syntheticRecords now reads specRegistry[metric].spec non-optionally, matching MUST_RENDER_CHART — spec is required on SpecRegistryEntry, so both are safe.
• derivedRegistry coverage gap (kriszyp Project easy studio #1). This was the big one for the branch name. Added a derived no-crash tier iterating Object.keys(derivedRegistry) — MetricRenderer dispatches derivedRegistry[metric] before it ever consults specRegistry, so mqtt-traffic-sent/received, request-rate, error-rate, and transaction-log-growth had zero coverage. They're fed synthetic records carrying the raw source columns each recompute reads directly (path/count/period, path/count/total, type, database/transactionLog). Bar is no-crash (no is unavailable / Render failed) since the derived renderers' populated-vs-empty shapes aren't uniform — noted why in a comment.
• Heatmap + small-multiples real-render (dawson Registry smoke test doesn't verify heatmap / small-multiples panels actually render data #1521). Added dedicated cases rather than shoehorning into MUST_RENDER_CHART, since both need different synthetic data. replication-latency gets a 2×2 source/destination matrix (sources parsed from path, destinations from node, count=500 to clear the confidence gate) that clears the ≥2-rows/≥2-cols and ≤12-cells gates, then asserts a real [role="grid"] with gridcells. resource-usage asserts multiple small-multiple sub-charts. Both check real DOM, not the empty state.

failOnRenderPhaseUpdate.ts

• Tripwire disabled when a test mocks console.error (dawson Render-phase-update tripwire is silently disabled in tests that mock console.error #1520). Added a self-check: the wrapper installed in beforeEach is captured, and afterEach detects + defensively restores when a test replaced console.error and left it replaced. Severity is a loud console.warn naming the test (not a hard failure) so the tests that legitimately mock console.error keep passing. Refactored the core into exported installTripwire / detectDisabledNet with a new failOnRenderPhaseUpdate.test.ts proving the detection fires. One nice side effect: the self-check surfaced that queryClient.test.ts used mockReset() (which leaves a swallowing spy installed at teardown) — switched it to mockRestore() so the net is re-armed between tests.
• Dev-mode dependency (dawson Stage #3 / kriszyp Stage #3). Added a comment documenting that the tripwire relies on React's DEV-build warning and silently goes green under a production React build.
• original?.() nit (kriszyp). Simplified the already-failed path to call console.error(...) directly (it's restored two lines up).

Deferred: kriszyp #3's separate freshness-watcher mount to re-prove #1510 — that belongs in a freshness-specific test, out of scope for this hardening pass. The relevant part (making the tripwire observable in the subsystems it protects) is covered by the #1520 self-check.

Gates: tsc -b, dprint fmt, oxlint src, and the full vitest suite (221 files, 1539 passed / 11 skipped) all green, with zero net-disabled warnings.

Comment generated by kAIle (Claude Fable 5)

[kriszyp]

Approving — the derived-registry coverage, dead-selector, and non-cartesian-tier gaps from the last pass are all closed, CI is green, and this is test-infra-only with no product risk.

One follow-up worth keeping in mind (not blocking): the tripwire's own core "fires" logic still doesn't have a direct test — only the "installs" half is covered. Might be worth a fast follow-up so the detection behavior has its own assertion.

— Claude