Hub Quality & Risk banded dials — epic #500 child 1 (#525)

[richard-devbot]

Closes #525 · first implementation child of epic #500 (data-viz).

What

Each 0-100 score renders as a banded arc beside its number:

• The band is the server's. The client consumes state.qualityRisk and never re-derives a band.
• pathLength="100" makes the dash units the score itself (stroke-dasharray="62 38" for 62) — which also means the Hub client: full-DOM live repaints destroy node identity — swallowed clicks, dropped focus, broken selection #495 morph patching that one attribute on a stable node is the draw-in animation, at the --motion-draw tier, frozen by the pinned reduced-motion guard.
• Honest nulls stay distinguishable: unknown draws the empty track only (no zero-length arc may imply a measured zero), while a real 0 — contracts present, nothing flagged — draws its empty arc and reads 0, per the Quality & Risk Index: Aggregated Risk Score + Complexity Index from the 15 stages #453 contract.
• One band palette: --band-* tokens are read by both the tile border and the arc stroke, so the two can't drift.
• A11y: role="img" with "<label> <score> of 100, band <band>"; the arc is aria-hidden; the readable number the dial augments stays in the DOM.

The test that almost lied

The first draft of the browser tests passed on unfixed code — every canonical fixture leaves quality/risk null (no builder.json anywhere), so each assertion sailed through an honest-unknown branch and proved nothing. Fixed by adding the canonical fixtureScoredRun (real builder contracts, mixed severities incl. one mitigated, a guardrail block, files touched, a container-verified execution) and rewriting the tests to demand real arcs whose lengths equal the projection's own numbers. Only then did they go red.

What live verification then found — #527

Looking at the rendered card (screenshot captured via playwright-core), the chips read "0 files touched · 0 builder tasks" against a fixture authoring two contracts and five files. Probing full-parse vs index-served on one root:

FULL PARSE  : risk 50 band "high"  | complexity 6 (5 files, 2 tasks)
INDEX SERVED: risk 15 band "low"   | complexity 2 (0 files, 0 tasks)

The same run flips high → low purely because of a cache tier, while risk_count: [2,2] sits in the same snapshot. That is a false score with a reassuring green band — filed as #527 (out of scope here; the dials render the projection faithfully either way, and the fixture docstring records the trap).

Verification

Core 1899/1899 · browser 26/26 · lint 0 · typecheck 0 · validate 196 · security green · whitespace clean. Live-verified: dials match the projection exactly (arc "15 85" for score 15, band class low).

Merging after green checks + reviewer bodies read, per the current working protocol.

🤖 Generated with Claude Code

[strix-security]

Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here.

[coderabbitai]

Warning

Review limit reached

@richard-devbot, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5da5c735-0f9e-4a96-8a05-49750d622ec6

📥 Commits

Reviewing files that changed from the base of the PR and between 9f93cac and e0be981.

📒 Files selected for processing (5)

• src/observability/dashboard/ui/pages/command-center.js
• src/observability/dashboard/ui/styles.js
• tests/browser/dashboard-525-dials.test.js
• tests/helpers/dashboard-fixtures.js
• tests/hub-risk-dials-525.test.js

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Render banded Quality & Risk dial arcs from server-provided scores/bands

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add SVG dial arcs for 0–100 Quality/Risk scores, using server-provided band classes.
• Centralize band colors in shared CSS tokens for tile borders and dial strokes.
• Add scored fixture + browser/unit tests to validate arcs, a11y labels, and motion behavior.

Diagram

graph TD
A["Server projection"] --> B["Command Center page"] --> C["qrScoreHtml"] --> D["qrDialHtml"] --> E["SVG dial"]
C --> F["CSS tokens"] --> E
G["Test fixtures"] --> H["Browser/unit tests"] --> B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. CSS conic-gradient dial (no SVG)

• ➕ Less DOM/SVG markup; can be a single element
• ➕ Potentially simpler styling for arc fill
• ➖ Harder to make the arc length an exact score unit (vs pathLength=100)
• ➖ More finicky reduced-motion handling and cross-browser rendering consistency

2. Client-derived bands from score thresholds

• ➕ No dependency on server band field
• ➕ UI can evolve independently if server lags
• ➖ Violates the stated contract: band must be server-defined
• ➖ Risks drift and mismatches between server and UI

3. Adopt a small chart/dial library

• ➕ Prebuilt a11y and animation patterns
• ➕ Potential reuse for other dashboard charts in epic EPIC: Hub data-viz layer — spend, risk, stage health and activity visualized from real telemetry #500
• ➖ Adds dependency weight for a very small visualization
• ➖ Harder to guarantee the exact semantics around null vs 0 rendering

Recommendation: Keep the current SVG + pathLength=100 approach. It cleanly enforces “dash units == score”, allows server-provided band classes to drive color without re-derivation, and makes the draw-in animation a single attribute transition that can be globally frozen under reduced motion. The fixture/test additions are the right guardrail against vacuous passes when scores are null.

Files changed (5) +295 / -9

Enhancement (2) +58 / -9

command-center.jsAdd banded SVG dial markup for Quality/Risk score tiles +26/-3

Add banded SVG dial markup for Quality/Risk score tiles

• Introduces qrDialHtml() to render a track circle plus an optional arc when a score is known, using pathLength=100 and stroke-dasharray="score remainder". Updates qrScoreHtml() to include the dial, keep readable text in the DOM, and provide a role=img aria-label that includes label, score, and band while hiding decorative circles from the a11y tree.

src/observability/dashboard/ui/pages/command-center.js

styles.jsCentralize band palette tokens and style/animate dial arcs +32/-6

Centralize band palette tokens and style/animate dial arcs

• Adds --band-* CSS variables and rewires tile border colors to use them, ensuring border and arc share one palette source. Adds .qr-dial styles (rotation, sizing) and animates stroke-dasharray at the --motion-draw tier while mapping band classes to arc stroke color.

src/observability/dashboard/ui/styles.js

Tests (3) +237 / -0

dashboard-525-dials.test.jsBrowser tests: assert dial arcs match projection, color, and motion +90/-0

Browser tests: assert dial arcs match projection, color, and motion

• Adds Playwright-based tests that seed a scored fixture, then asserts both tiles draw arcs with valid 0–100 dash lengths and correct aria labels. Verifies arc length equals the projection value (no client re-derivation), checks CSS transition targets stroke-dasharray with the draw tier duration, and confirms reduced-motion collapses transitions.

tests/browser/dashboard-525-dials.test.js

dashboard-fixtures.jsAdd fixtureScoredRun to produce non-null Quality/Risk projections +69/-0

Add fixtureScoredRun to produce non-null Quality/Risk projections

• Introduces a new canonical fixture that writes builder.json contracts with mixed severities (including a mitigated risk), guardrail activity, files touched, and a container execution so projections emit real quality/risk and complexity values. Documents a known cache/index discrepancy (#527) so tests avoid relying on index-served state for severity counts.

tests/helpers/dashboard-fixtures.js

hub-risk-dials-525.test.jsUnit tests: emitted HTML includes correct dial semantics for null/zero +78/-0

Unit tests: emitted HTML includes correct dial semantics for null/zero

• Adds sandboxed renderer tests that validate the emitted HTML contains an SVG dial with stroke-dasharray derived directly from the score and band class from the server. Covers accessibility labelling, honest null behavior (no arc), and the distinct zero-score rendering (arc present with 0 100). Also asserts band palette tokens are shared between border and arc and that animation is tied to --motion-draw.

tests/hub-risk-dials-525.test.js

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

Context used

✅ Compliance rules (platform): 300 rules

✅ Skills: 17 invoked

  code-review-pr

  claude-api

  documentation-writing

  pptx

  docx

  performance-monitoring

  security-compliance

  cso

  plan-eng-review

  design-review

  prompt-engineering

  mcp-builder

  qa-testing

  code-patterns

  xlsx

  security-owasp

  testing-qa

1. Unvalidated dial score ✓ Resolved 🐞 Bug ≡ Correctness

Description

qrDialHtml() treats any non-null/undefined score as valid and directly emits
stroke-dasharray="<score> <100-score>", so non-finite or out-of-range values produce NaN/negative
dash segments and can break/misrender the dial. This is a new failure mode introduced by the SVG
renderer (esc() only HTML-escapes and does not enforce numeric semantics).

Code

src/observability/dashboard/ui/pages/command-center.js[R420-430]

+function qrDialHtml(label, score, band) {
+  var known = score !== null && score !== undefined;
+  var ariaLabel = known
+    ? label + ' ' + score + ' of 100, band ' + String(band || 'unknown').replace(/_/g, ' ')
+    : label + ' unknown';
+  return '<svg class="qr-dial" viewBox="0 0 40 40" role="img" aria-label="' + esc(ariaLabel) + '">' +
+    '<circle class="qr-dial-track" cx="20" cy="20" r="16" pathLength="100" aria-hidden="true"></circle>' +
+    (known
+      ? '<circle class="qr-dial-arc" cx="20" cy="20" r="16" pathLength="100" stroke-dasharray="' +
+        esc(score) + ' ' + esc(100 - score) + '" aria-hidden="true"></circle>'
+      : '') +

Relevance

●●● Strong

Team has accepted numeric-safety guards (Number.isFinite/Math.max clamping) in prior UI/state code.

PR-#508
PR-#11

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

qrDialHtml’s known check allows NaN/strings/out-of-range values through, and then uses `100 -
score and emits both values into stroke-dasharray`. esc() only escapes HTML metacharacters, so it
will happily emit NaN/-5 etc. The server computation clamps to 0–100, showing the intended
contract, but the renderer does not enforce it.

src/observability/dashboard/ui/pages/command-center.js[414-432]
src/observability/dashboard/ui/lib.js[312-318]
src/observability/dashboard/state/quality-risk.js[58-61]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`qrDialHtml(label, score, band)` only checks `score !== null && score !== undefined` and then uses it in arithmetic (`100 - score`) and in the emitted `stroke-dasharray`. If `score` is NaN, a string, or outside 0–100, the SVG gets invalid dash segments (NaN/negative), which can break or mislead the UI.

### Issue Context
The server-side projection intends scores to be numeric 0–100, but the renderer should be defensive: stale/malformed state, future refactors, or alternate callers can pass non-finite values.

### Fix
Coerce `score` to a number, require it to be finite, and clamp to [0, 100]. If not finite, treat as unknown (draw track only, no arc). Use the sanitized numeric score consistently for both aria-label and dasharray.

### Fix Focus Areas
- src/observability/dashboard/ui/pages/command-center.js[414-445]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Dial test can throw ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new browser test dereferences card.projection.risk and riskTile.dash without first asserting
card.projection/riskTile exist, so regressions can fail with TypeError instead of an actionable
assertion message. This reduces test diagnostics and can obscure the real cause of a failure.

Code

tests/browser/dashboard-525-dials.test.js[R32-62]

+      projection: qr ? { risk: qr.risk && qr.risk.score, riskBand: qr.risk && qr.risk.band } : null,
+      tiles: tiles.map((tile) => ({
+        band: [...tile.classList].filter((c) => c !== 'qr-score')[0] ?? null,
+        hasArc: Boolean(tile.querySelector('.qr-dial-arc')),
+        dash: (tile.querySelector('.qr-dial-arc') || {}).getAttribute
+          ? tile.querySelector('.qr-dial-arc').getAttribute('stroke-dasharray')
+          : null,
+        label: (tile.querySelector('.qr-dial') || {}).getAttribute
+          ? tile.querySelector('.qr-dial').getAttribute('aria-label')
+          : null,
+        text: tile.textContent.replace(/\s+/g, ' ').trim(),
+      })),
+    };
+  });
+  assert.equal(card.tiles.length, 2, 'both score tiles render');
+  assert.ok(Number.isFinite(card.projection.risk),
+    `the scored fixture must produce a real risk score, or this test proves nothing — got ${card.projection.risk}`);
+
+  const drawn = card.tiles.filter((tile) => tile.hasArc);
+  assert.equal(drawn.length, 2, 'both scored tiles draw an arc');
+  for (const tile of drawn) {
+    const score = Number(String(tile.dash).split(' ')[0]);
+    assert.ok(Number.isFinite(score) && score >= 0 && score <= 100, `arc length is a 0-100 score — got ${tile.dash}`);
+    assert.match(tile.label ?? '', /of 100, band /, 'the dial is labelled with score + band');
+    assert.ok(tile.text.includes(String(score)), 'and the readable number is still in the tile');
+  }
+  const riskTile = drawn.find((tile) => (tile.label ?? '').startsWith('Aggregated Risk'));
+  assert.equal(Number(String(riskTile.dash).split(' ')[0]), card.projection.risk,
+    'the arc length IS the projection score — the client never re-derives it');
+  assert.equal(riskTile.band, card.projection.riskBand,
+    'and the band class is the server band');

Relevance

●● Moderate

No close historical pattern on guarding browser-test dereferences; team sometimes hardens tests,
sometimes rejects style changes.

PR-#514
PR-#524

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The evaluated object explicitly sets projection to null when qualityRisk is missing, but the
test unconditionally accesses card.projection.risk. Similarly, riskTile can be undefined if no
tile label matches, and the test dereferences it without a guard.

tests/browser/dashboard-525-dials.test.js[26-63]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
In `dashboard-525-dials.test.js`, the test builds `projection: ... ? {...} : null` but later unconditionally reads `card.projection.risk`. It also uses `riskTile.dash` after `find()` without asserting the tile exists.

### Issue Context
If the dashboard stops emitting `qualityRisk` or changes the tile label/class structure, this test will crash with a TypeError, producing less actionable failures than explicit assertions.

### Fix
- Add `assert.ok(card.projection, ...)` before reading `card.projection.risk`.
- Add `assert.ok(riskTile, ...)` before dereferencing `riskTile.dash`/`riskTile.band`.
- (Optional) If labels are expected to be stable, assert the label is present before comparing dash values.

### Fix Focus Areas
- tests/browser/dashboard-525-dials.test.js[26-63]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. .qr-dial-arc animates stroke-dasharray 📜 Skill insight ➹ Performance

Description

The new dial arc animation transitions stroke-dasharray, which violates the compliance requirement
that CSS animations/transitions be limited to transform and opacity only. This
non-compositor-friendly transition can cause performance/rendering issues and fails the project’s
animation compliance rule.

Code

src/observability/dashboard/ui/styles.js[R1592-1596]

+.qr-dial-arc {
+  stroke: var(--band-unknown);
+  stroke-linecap: round;
+  transition: stroke-dasharray var(--motion-draw) var(--motion-ease-emphatic);
+}

Relevance

● Weak

Similar “transform/opacity only” animation-compliance suggestions were repeatedly rejected; repo
keeps stroke-dasharray/background transitions.

PR-#514
PR-#520

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance IDs 1400011 and 1399699 both state that animations/transitions must target only
transform and opacity. In the new styling, the .qr-dial-arc rule explicitly uses a
transition on stroke-dasharray, which is neither transform nor opacity, directly
demonstrating the violation.

src/observability/dashboard/ui/styles.js[1592-1596]
Skill: performance-monitoring
Skill: design-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CSS for the new dial arc animates `stroke-dasharray` (via a `transition`), but the compliance rule requires animations/transitions to be limited to `transform` and `opacity` only.

## Issue Context
The dial/banded dial currently animates the SVG arc draw-in (arc length) by transitioning `stroke-dasharray` in the `.qr-dial-arc` styling.

## Fix Focus Areas
- src/observability/dashboard/ui/styles.js[1592-1596]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[richard-devbot]

Both Qodo findings addressed — both were real:

1. Unvalidated dial score: qrNumericScore is now the single guard for the arc AND the number. Non-numeric renders the honest unknown (no arc, em-dash); out-of-range clamps the arc. Worth noting the text path had the same weakness pre-existing — it would have printed NaN/100 — so I shared the guard rather than patching only the code I added. New test covers NaN/string/object plus 150 and -5 (verified failing first).
2. Test can throw: card.projection and riskTile are asserted before dereferencing, so a regression reports a diagnosis instead of a TypeError.

Suite note: one full-suite run showed dashboard-command-pages failing on fromIndex being undefined — verified NOT from this branch (passes in isolation with AND without my changes; the next full run was 1900/1900). It's a third sighting of the load-dependent async-write flake family — mechanism recorded on #517.

7/7 dial pins, 26/26 browser, lint/typecheck clean. — Claude Main