feat(studio): score-driven Safe Synthesizer report gauges [ASTD-310]#849
Open
marcusds wants to merge 3 commits into
Open
feat(studio): score-driven Safe Synthesizer report gauges [ASTD-310]#849marcusds wants to merge 3 commits into
marcusds wants to merge 3 commits into
Conversation
Replace the fixed blue/purple SQS/DPS dials with a score-driven ScoreGauge matching the redesigned evaluation report (NVIDIA-NeMo/Safe-Synthesizer#653): a red->green gradient ring with a score marker for large gauges and a single-tier ring for sub-metrics. Gauge color is now a function of the score tier, identical for both metrics in every view, so the summary/detail color mismatch (ASTD-310) can no longer occur. Migrate all four Dial consumers (report summary, detail panels, score table, jobs data view), drop the per-metric color props, and retire the now-unused Dial component. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
📝 WalkthroughWalkthroughChanges
Sequence Diagram(s)sequenceDiagram
participant ScorePanel
participant TitledDial
participant ScoreGauge
participant SVG
ScorePanel->>TitledDial: pass raw score
TitledDial->>ScoreGauge: render large gauge
ScoreGauge->>SVG: draw tier arcs and marker
SVG-->>ScoreGauge: render score or unavailable state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/packages/common/src/components/ScoreGauge/index.test.tsx (1)
45-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo component-level test for score clamping.
scoreTier/scoreColorare tested against negative/NaN inputs, butScoreGaugeitself is never rendered with a score >10 or negative to verify the clamp (Math.min(Math.max(score, 0), 10), Line 87 ofindex.tsx) actually caps the displayed/percentage output.✅ Suggested additional test
+ it('clamps an out-of-range score to 10', () => { + render(<ScoreGauge score={15} size="lg" />); + expect(screen.getByTestId('gauge-display')).toHaveTextContent('10.0'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/ScoreGauge/index.test.tsx` around lines 45 - 65, Extend the ScoreGauge tests to render scores above 10 and below 0, verifying the component clamps them to the 0–10 range in both displayed value and progress/percentage output. Add these cases alongside the existing tests in the ScoreGauge describe block, using the gauge-display and relevant progress or marker attributes to assert the clamped results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/TitledDial.tsx`:
- Line 6: Update the React import in TitledDial.tsx to use a type-only import
for FC, since FC is referenced only as a type.
---
Nitpick comments:
In `@web/packages/common/src/components/ScoreGauge/index.test.tsx`:
- Around line 45-65: Extend the ScoreGauge tests to render scores above 10 and
below 0, verifying the component clamps them to the 0–10 range in both displayed
value and progress/percentage output. Add these cases alongside the existing
tests in the ScoreGauge describe block, using the gauge-display and relevant
progress or marker attributes to assert the clamped results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 46b1ba93-74d7-4f09-9311-db7266cfb867
📒 Files selected for processing (14)
web/packages/common/src/components/Dial/index.test.tsxweb/packages/common/src/components/Dial/index.tsxweb/packages/common/src/components/ScoreGauge/index.test.tsxweb/packages/common/src/components/ScoreGauge/index.tsxweb/packages/common/src/components/index.tsweb/packages/studio/src/components/dataViews/SafeSynthesizerJobsDataView/index.tsxweb/packages/studio/src/routes/SafeSynthesizerJobDetailsRoute/components/ReportSummaryPanel.test.tsxweb/packages/studio/src/routes/SafeSynthesizerJobDetailsRoute/components/ReportSummaryPanel.tsxweb/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/DataPrivacyPanel.test.tsxweb/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/DataPrivacyPanel.tsxweb/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/ScoreTable.tsxweb/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/SyntheticQualityPanel.test.tsxweb/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/SyntheticQualityPanel.tsxweb/packages/studio/src/routes/SafeSynthesizerJobReportRoute/components/ScorePanels/TitledDial.tsx
💤 Files with no reviewable changes (2)
- web/packages/common/src/components/Dial/index.test.tsx
- web/packages/common/src/components/Dial/index.tsx
marcusds
marked this pull request as ready for review
July 23, 2026 16:17
- Use a type-only import for FC in TitledDial - Add ScoreGauge test covering out-of-range score clamping Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
astd-310-report-colors-quality-privacy-swap/mschwab
branch
from
July 23, 2026 16:20
7bb8baf to
649a61e
Compare
Contributor
|
…D-310] Move the score-tier constants and gauge geometry into consts.ts and the pure helper functions into utils.ts. index.tsx keeps only the component and re-exports both modules so existing import paths are unchanged. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/packages/common/src/components/ScoreGauge/utils.ts (1)
16-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer arrow functions for the short pure helpers.
Convert
scoreTier,scoreColor,point,arcPath, andinterpolateto exported arrow-function constants; none require hoisting.As per coding guidelines, “Prefer arrow functions for short, pure functions in TypeScript.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/ScoreGauge/utils.ts` around lines 16 - 53, Convert the exported pure helpers scoreTier, scoreColor, point, arcPath, and interpolate from function declarations to exported arrow-function constants, preserving their parameters, return types, and existing logic.Source: Coding guidelines
web/packages/common/src/components/ScoreGauge/index.tsx (1)
25-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate the public component return type.
ScoreGaugeis exported without an explicit return type. Add the repository’s standard React element return annotation.As per coding guidelines, “Use explicit return types for public APIs and complex functions in TypeScript.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/ScoreGauge/index.tsx` around lines 25 - 29, Update the exported ScoreGauge component’s signature to include the repository-standard explicit React element return type, while preserving its existing props, calculations, and rendering behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/packages/common/src/components/ScoreGauge/index.tsx`:
- Around line 25-29: Update the exported ScoreGauge component’s signature to
include the repository-standard explicit React element return type, while
preserving its existing props, calculations, and rendering behavior.
In `@web/packages/common/src/components/ScoreGauge/utils.ts`:
- Around line 16-53: Convert the exported pure helpers scoreTier, scoreColor,
point, arcPath, and interpolate from function declarations to exported
arrow-function constants, preserving their parameters, return types, and
existing logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 675ce020-a36a-4554-b293-2bf980a8cfed
📒 Files selected for processing (3)
web/packages/common/src/components/ScoreGauge/consts.tsweb/packages/common/src/components/ScoreGauge/index.tsxweb/packages/common/src/components/ScoreGauge/utils.ts
steramae-nvidia
approved these changes
Jul 23, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes ASTD-310 — SQS/DPS gauge colors swapped between the Safe Synthesizer report summary and detail views.
Rather than picking a canonical blue/purple mapping, this aligns Studio with the redesigned evaluation report (NVIDIA-NeMo/Safe-Synthesizer#653), which drives gauge color off the score value, not the metric. Because both SQS and DPS use identical score-tier coloring in every view, the summary/detail mismatch can no longer occur.
Changes
ScoreGauge(packages/common), replacingDial:lg: red→green gradient ring + white score marker +N / of 10center (report summary + detail main gauges).sm: single-tier ring for sub-metrics and the jobs table.scoreColor/scoreTier/SCORE_TIER_COLORS. Tiers: ≥8#22c55e, ≥6#84cc16, ≥4#eab308, ≥2#f97316, else#ef4444; 0/NaN → gray.Dialconsumers (ReportSummaryPanel,TitledDial,ScoreTable,SafeSynthesizerJobsDataView) and dropped every per-metriccolorprop.Dialcomponent (+ test + barrel export).Test plan
@nemo/commonsuite — 1319 passed (incl. newScoreGaugetests)dial-colorassertions)@nemo/commontypecheck + eslint cleanExperimentGroupDataView)Notes
getSyntheticQualityGradeLabel) still uses the old scale with a "Very Poor" bucket; PR feat(experiments): Update metadata shape and add filter support #653 removed it. Left the grade/pass-fail logic untouched here (out of scope for the gauge color fix) — can follow up if desired.Summary by CodeRabbit
Summary
New Features
Changes
Tests