iframe-proxy

cristim · 2026-05-27T22:45:23Z

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 historical data_points[].by_service values 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-section so both potential-savings charts are adjacent and share the "Potential Savings" area of the Home page.
Re-data: renderSavingsByService now accepts LocalRecommendation[] (already fetched by loadDashboard via getRecommendations). A new computeServiceStatsFromRecs helper groups rows by rec.service and computes min/max of rec.savings within each group. Floor = min potential savings across recommendation rows for that service; upside = max - min. The call was moved out of loadSavingsTrendChart and into loadDashboard where recs is 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) and max = 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 in computeServiceStatsFromRecs references #769.

Tooltip enrichment: the new tooltip surfaces Min option and Max option labels (e.g. "1yr no_upfront", "3yr all_upfront") so users can identify which term/payment combination drives the floor and ceiling.

Test plan

computeServiceStatsFromRecs unit tests: empty input, single rec (zero upside), two recs same service (min/max/count), minLabel/maxLabel tracking, multi-service independence, samples accumulation.
renderSavingsByService DOM tests updated to use Recommendation fixtures (not SavingsDataPoint).
Dataset label assertions updated: "Floor" -> "Min potential", "Range" -> "Upside".
Heading reset assertion updated to new copy.
Integration test: loadDashboard wires range bars to recs, not analytics data (recs present + analytics empty -> chart renders).
Removed obsolete loadSavingsTrendChart re-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

Feature Changes
- Updated "Potential Savings by Service" chart to display min and max potential savings across recommendation options
- Enhanced chart tooltips with min/max potential values and associated option labels
- Reordered chart sections on the dashboard for improved visual layout
- Refined empty-state messaging and chart labels for clarity

… + 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

coderabbitai · 2026-05-27T22:45:31Z

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: db710084-f202-4c3c-ae89-ef61c5b467da

📥 Commits

Reviewing files that changed from the base of the PR and between 0c3729b and 54b932b.

📒 Files selected for processing (3)

frontend/src/__tests__/dashboard.test.ts
frontend/src/dashboard.ts
frontend/src/index.html

📝 Walkthrough

Walkthrough

The 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.

Changes

Per-Service Savings Chart Refactoring

Layer / File(s)	Summary
Data Contract: ServiceSavingsStats Extension `frontend/src/dashboard.ts`	`ServiceSavingsStats` is extended with optional `minLabel` and `maxLabel` fields to track which recommendation option produced min/max bounds. New exported function `computeServiceStatsFromRecs()` derives per-service min/max potentials and option labels from `LocalRecommendation[]`.
Chart Rendering from Recommendations `frontend/src/dashboard.ts`	`renderSavingsByService()` is rewritten to accept recommendations instead of analytics data points and compute stats via the new helper. Heading, empty-state text, dataset labels ("Min potential" and "Upside"), and tooltip content are updated to reflect recommendation-driven semantics with option labels and counts.
Dashboard Integration and Trend Chart Cleanup `frontend/src/dashboard.ts`	`loadDashboard()` now calls `renderSavingsByService(recs)` immediately after rendering the summary, decoupling the chart from analytics `data_points`. `loadSavingsTrendChart()` no-data and error handlers are adjusted to avoid triggering `renderSavingsByService([])`, allowing the chart to be driven independently.
HTML Markup Reordering and Accessibility Updates `frontend/src/index.html`	Home tab savings chart sections are reordered to place the savings-by-service range card after the savings trend card. The card heading, canvas `aria-label`, and empty-state message are updated to reflect min/max potential and recommendation semantics while preserving element IDs.
Test Suite: Imports, Fixtures, and Comprehensive Coverage `frontend/src/__tests__/dashboard.test.ts`	Test imports are extended to include `computeServiceStatsFromRecs`; a minimal recommendation fixture builder `rec()` is added. Comprehensive unit tests validate the new computation function for empty/single/multi-service accumulation, min/max derivation, and minLabel/maxLabel generation. DOM and integration tests are updated to use recommendation fixtures and verify chart behavior when driven from recommendations and `loadDashboard()` orchestration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

LeanerCloud/CUDly#765: Directly addresses the same per-service savings-range bar chart refactoring to compute min/max bars from recommendation options rather than analytics data points.

Possibly related PRs

LeanerCloud/CUDly#766: Introduces the per-service savings-range bar chart that this PR refactors to be recommendations-driven.
LeanerCloud/CUDly#295: Also refactors dashboard savings UI in renderDashboardSummary to derive from getRecommendations and LocalRecommendation[], applying the same recommendations pipeline pattern to a different chart.
LeanerCloud/CUDly#311: Adds defensive guards around recommendation iteration in dashboard.ts, providing safe patterns for the same recommendation-array consumption that this PR extends to the per-service chart computation.

Suggested labels

effort/m, type/feat

Poem

🐰 A chart that once counted analytics past,
Now counts recommendation dreams vast,
With min-max potentials aglow,
Per service savings on show,
The dashboard's bright future is cast! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: repointing the per-service range bars chart from historical analytics data to recommendation-based potential savings, and relocating the chart section within the UI layout.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/769-potential-range-bars

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

cristim · 2026-05-27T22:45:34Z

@coderabbitai review

coderabbitai · 2026-05-27T22:45:41Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

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

📥 Commits

Reviewing files that changed from the base of the PR and between d986b4d and 0c3729b.

📒 Files selected for processing (3)

frontend/src/__tests__/dashboard.test.ts
frontend/src/dashboard.ts
frontend/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.

cristim · 2026-05-28T12:00:09Z

@coderabbitai review

coderabbitai · 2026-05-28T12:00:16Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-05-28T12:39:43Z

@coderabbitai review

coderabbitai · 2026-05-28T12:39:49Z

cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint impact/all-users Affects every user enhancement New feature or request triaged Item has been triaged labels May 27, 2026

coderabbitai Bot reviewed May 27, 2026

View reviewed changes

Comment thread frontend/src/__tests__/dashboard.test.ts

Comment thread frontend/src/dashboard.ts Outdated

Comment thread frontend/src/index.html Outdated

cristim merged commit c72601e into feat/multicloud-web-frontend May 28, 2026
6 checks passed

This was referenced May 28, 2026

Add per-service savings-range bar chart on Home page (solid floor + lighter range, surface ByService data already on the wire) #765

Closed

'Plan from X selected' button should disable on heterogeneous selection (QA 6.2) #769

Closed

coderabbitai Bot mentioned this pull request Jun 3, 2026

feat(home): 2x height + stack Current under Potential on savings-by-service chart #925

Merged

3 tasks

cristim deleted the feat/769-potential-range-bars branch June 3, 2026 21:53

coderabbitai Bot mentioned this pull request Jun 5, 2026

feat(home/chart): stack Current/Committed band on per-service potential-range bars #968

Merged

5 tasks done

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

cristim commented May 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

cristim commented May 27, 2026

Uh oh!

coderabbitai Bot commented May 27, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cristim commented May 28, 2026

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

Uh oh!

cristim commented May 28, 2026

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented May 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 27, 2026 •

edited

Loading