refactor: consolidate stat-card markup in static analytics template (#4923)

[frano-m]

Closes #4923

⚠️ Stacked on #4930 (fran/4922-access-request-service-cards). Until #4930 merges, this PR's diff also shows #4922's commits — only 63859e11 belongs to this PR. Merge #4930 first; this branch is rebased on it.

What changed

The stat-card markup in the shared static analytics template was hand-written in three places and had diverged (tooltip/change-row/grid-class combinations). This PR consolidates all three onto the existing statCard() helper, extended with an options object:

• statCard(value, label, { change, gridClass, tooltip }) — omitting change omits the change row; null renders "N/A vs prior month"; multi-line labels (\n) render as line breaks; gridClass is omitted when the container sizes cards itself (access-requests grid).
• Top stats grid (renderStats): output byte-identical (verified by string comparison across all change states: positive, negative, null, zero).
• Key metrics (renderEventCounts): inline copy deleted; now also reuses pctChange() instead of hand-rolling the same formula (?? 0 preserves its old "+0%" default exactly).
• Access-requests stats: inline copy deleted.
• pctChange() hardened with a Number.isFinite guard (a non-finite result now renders "N/A vs prior month" instead of "NaN%" — pre-existing, practically unreachable edge).

One deliberate visual delta: event-count and access-request labels now sit in the same stat-label-wrap wrapper as the top stats, which adds a few pixels of label height on those cards — that's the unification working as intended, flagged here for anyone pixel-comparing.

How verified

• Node equivalence tests: unified statCard() vs the legacy markup for all three shapes and every change-value path; pctChange() unit cases including the NaN guard.
• Playwright rendering tests: top stats (4 cards, tooltips work on hover), key metrics (6 cards, multi-line labels, change rows, fui-grid-item-6), access-request cards (no change rows), plus the full fix: generalize access-request service classification and stat cards in static analytics template #4922 regression suite (multi-service, single-service, malformed payloads) — zero JS errors.
• /code-review at high effort: no correctness bugs; the two cleanup findings it raised are applied in this PR, the label-wrap geometry note is the deliberate delta above.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This pull request refactors the shared static analytics template to consolidate duplicated “stat card” HTML into a single statCard() helper with an options object, while also tightening up change-percentage calculation and the access-requests stats/table rendering logic.

Changes:

• Expanded statCard(value, label, { change, gridClass, tooltip }) to support optional change rows, optional grid classes, tooltips, and multi-line labels.
• Updated the top stats grid (renderStats) and key metrics (renderEventCounts) to reuse statCard() and pctChange().
• Reworked access-requests rendering to accept pre-fetched JSON, compute per-service counts generically, and render per-service cards dynamically.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[frano-m]

Follow-up review pass (9e3df872) — fixed a real defect the consolidation exposed, plus the cleanups it invited.

The defect: pctChange() returned a toFixed(1) string, so statCard tested the sign after rounding. A tiny decline (e.g. current 9999 vs prior 10000 → -0.01% → "-0.0") coerces to -0, which passes >= 0, so the card rendered +-0.0% vs prior month in green. Reproduced before the fix; pctChange() now returns a number and statCard rounds at render time, so the sign comes from the true value.

Also in this commit:

• aria-label escaping — escapeHtml doesn't escape double quotes, and its output landed inside a double-quoted attribute; a label with a quote could inject attributes. Now quote-encoded (and newlines collapsed to spaces for multi-line labels).
• Shared changeClass() — the positive/negative ternary existed in four places (statCard + the three table renderers). Extracted and used at all four; valid for both the fractions tables carry and the percentages cards carry.
• Escape the label once instead of N+1 times per card, dropping the redundant String() coercion and the second escape call site.
• Flattened the paired nested ternaries (the sign test was evaluated three times) and single-sourced the event-label newline split, which happened up to four times per card.

One item deliberately not applied, flagged for a call: renderEventCounts still passes pctChange(...) ?? 0, so an event with no prior-month baseline shows +0.0% in green rather than the N/A vs prior month that statCard now supports (and that the traffic cards show for the same condition). Four review angles flagged it. It's base-parity behaviour, but changing it changes what published reports display — happy to switch it to N/A if that's the preferred semantics.

Verified: display parity against real AnVIL fixture data (−19.9% / −23.7% / −31.7% / +7.1% identical before and after), all three card families, table change cells still coloured, tooltips working, malformed-payload path intact, zero JS errors.

🤖 Generated with Claude Code