feat(home/chart): repoint per-service range bars to potential savings + move to Potential section#782
Merged
cristim merged 2 commits intoMay 28, 2026
Conversation
… + move to Potential section - Move #savings-by-service-section below #savings-chart-section so both live in the Potential Savings area of the Home page. Previously the range bars sat in the current/historical savings group despite showing forward-looking data. - Re-data the chart: renderSavingsByService now accepts LocalRecommendation[] instead of SavingsDataPoint[]. A new computeServiceStatsFromRecs helper groups recommendation rows by service.service and computes min/max of rec.savings within each group. Floor = min potential savings for that service; upside = max - min. Wired in loadDashboard alongside renderSavingsChart (both have recs in scope). Removed the now-redundant calls from loadSavingsTrendChart. - Update copy: heading -> "Potential savings range per service", dataset labels -> "Min potential"/"Upside", tooltip lines use "Min potential / Max potential / Options / Min option / Max option". Empty state -> "No recommendations available yet." - Tests: replace SavingsDataPoint fixtures with Recommendation fixtures throughout the renderSavingsByService DOM suite. Add computeServiceStatsFromRecs unit tests (empty, single, multi-rec, minLabel/maxLabel tracking, samples). Add integration test asserting loadDashboard wires the bar chart to recs not analytics data. Remove obsolete loadSavingsTrendChart re-render tests. - Drop medianOf helper (no longer called after tooltip update). NOTE: current recommendations carry a single savings field per row, not per-payment-option variants. Min/max within a service therefore reflects the range across different recommendation rows (e.g. different regions or resource types). Per-variant breakdowns (1yr/3yr x no-upfront/all-upfront) will further widen the range automatically once the backend ships them. Closes #769
cristim
added
priority/p2
|
Warning Review limit reached
More reviews will be available in 16 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe dashboard's per-service savings range bar chart is refactored to compute min/max savings directly from recommendation objects instead of historical analytics data points. The data contract is extended, a new computation function is introduced, chart rendering is rewritten, dashboard orchestration is updated, and comprehensive tests validate the new recommendation-driven pipeline. ChangesPer-Service Savings Chart Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
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)
Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@frontend/src/__tests__/dashboard.test.ts`:
- Around line 1434-1438: The test fixture passed to (api.getDashboardSummary as
jest.Mock).mockResolvedValue uses snake_case keys that don't match the
DashboardSummary contract; update the object to use the exact DashboardSummary
field names (e.g., potentialMonthlySavings, currentMonthlySavings,
totalRecommendations, activeReservations, targetCoverage, ytdSavings, byService)
so the mocked response matches the typed contract used by the code under test
(adjust any nested keys similarly).
In `@frontend/src/dashboard.ts`:
- Around line 712-713: The label construction uses rec.payment which is optional
and can produce "undefined" in labels; update the code that builds label (the
const label = ... line) to guard against undefined by using a default or
conditional—e.g., use rec.payment ?? '' or build the string only when
rec.payment exists (e.g., rec.payment ? `${rec.term}yr ${rec.payment}` :
`${rec.term}yr`) so labels never contain "undefined"; locate this change around
the label variable near stats and svc references.
In `@frontend/src/index.html`:
- Line 103: The empty-state text for the element with id
"savings-by-service-empty" and classes "empty hidden" is misleading because it
displays when recommendations exist but none have positive savings; update the
text content to accurately reflect that case (for example, "No recommendations
with positive savings." or similar) so users understand recommendations exist
but no positive-savings items are available; modify the inner text of the <p
id="savings-by-service-empty"> element accordingly and keep existing
classes/visibility logic intact.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d46d3b7-950f-4836-bd5b-d87a16b2253f
📒 Files selected for processing (3)
frontend/src/__tests__/dashboard.test.tsfrontend/src/dashboard.tsfrontend/src/index.html
… copy + update test fixture
Guard rec.payment in computeServiceStatsFromRecs: when the field is absent or
whitespace-only the tooltip label previously rendered as "1yr undefined"; it now
falls back to "1yr unspecified". A new unit test asserts the fallback for both
undefined and empty-string payment values, and verifies the string "undefined"
never appears in the label.
Correct the savings-by-service empty-state paragraph: the previous text ("No
recommendations available yet.") was misleading because the element is also shown
when recommendations exist but none carry positive potential savings; the updated
copy ("No positive potential savings found for current recommendations.") matches
the actual emit condition.
Align the getDashboardSummary mock fixture in dashboard.test.ts with the current
DashboardSummary contract: drop the removed fields current_monthly_savings and
active_reservations, and add the current fields active_commitments and
committed_monthly.
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
1 participant
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
Prior state confusion:
#savings-by-service-section(the range bar chart from Add per-service savings-range bar chart on Home page (solid floor + lighter range, surface ByService data already on the wire) #765) was placed between the "Savings over time" trend line and "Potential Savings by Service" pie/bar chart. Its data source was historicaldata_points[].by_servicevalues from/api/history/analytics, so the bars reflected realized past savings -- not potential future ones. The heading said "Savings range by service" with no temporal framing, making it easy to mistake for forward-looking data.Move: The section now sits below
#savings-chart-sectionso both potential-savings charts are adjacent and share the "Potential Savings" area of the Home page.Re-data:
renderSavingsByServicenow acceptsLocalRecommendation[](already fetched byloadDashboardviagetRecommendations). A newcomputeServiceStatsFromRecshelper groups rows byrec.serviceand computes min/max ofrec.savingswithin each group. Floor = min potential savings across recommendation rows for that service; upside = max - min. The call was moved out ofloadSavingsTrendChartand intoloadDashboardwhererecsis already in scope, eliminating the dependency on the analytics endpoint.Design
Layout choice: stacked (range bars below the existing
#savings-chart-section). Both sections convey different views of potential savings -- the existing chart shows current vs potential totals per service; the range bars show the spread of individual recommendation options. Keeping both provides complementary detail.Floor/range derivation: per the user's clarification,
min = min(rec.savings)andmax = max(rec.savings)across all recommendation rows for a service. Each row is treated as one "option." When the backend ships per-variant rows (1yr/3yr x payment option), the range will widen automatically without a code change. The TODO comment incomputeServiceStatsFromRecsreferences #769.Tooltip enrichment: the new tooltip surfaces
Min optionandMax optionlabels (e.g. "1yr no_upfront", "3yr all_upfront") so users can identify which term/payment combination drives the floor and ceiling.Test plan
computeServiceStatsFromRecsunit tests: empty input, single rec (zero upside), two recs same service (min/max/count), minLabel/maxLabel tracking, multi-service independence, samples accumulation.renderSavingsByServiceDOM tests updated to useRecommendationfixtures (notSavingsDataPoint).loadDashboardwires range bars to recs, not analytics data (recs present + analytics empty -> chart renders).loadSavingsTrendChartre-render tests (that coupling no longer exists).npm run typecheck: exit 0, no errors.jest --testPathPattern=dashboard.test: 69 pass, 0 fail.Summary by CodeRabbit