feat(home): 2x height + stack Current under Potential on savings-by-service chart by cristim · Pull Request #925 · LeanerCloud/CUDly

cristim · 2026-06-02T18:49:31Z

Summary

Adds a scoped CSS rule #savings-by-service-chart { max-height: 600px; min-height: 560px; } immediately after the generic .chart-section canvas block -- other charts remain at 300/280 px.
Merges all three datasets (Current (committed), Min potential, Upside) into a single stack: 'savings' so the Current bar renders visually below the potential range (Chart.js renders bottom-to-top in dataset-array order).
Updates the tooltip discriminator from ctx.dataset.stack === 'current' to ctx.dataset.label === 'Current (committed)' since all datasets now share one stack name.
Two new tests: (a) stacked-axes + single-stack invariant, (b) dataset order Current first.

Dependency note

The Current (committed) bar still reads zeros until the parallel PR fix/current-savings-per-service (which populates ServiceSavings.CurrentSavings on the backend) lands. The UI change is self-contained and correct; stacked bars with a zero base segment are visually equivalent to the old grouped layout until real data flows in.

Test plan

cd frontend && npm test -- --testPathPattern=dashboard -- all 85 tests pass (2 new: stacking invariant + dataset order).
Visual inspection: chart is approximately 2x taller on the Home page; Current bar stacks below Min potential + Upside bars when data is present.
Confirm no other charts changed height (e.g. savings trend, savings history).

Canvas selector used

#savings-by-service-chart (confirmed from frontend/src/index.html:98 and frontend/src/dashboard.ts:739).

Summary by CodeRabbit

Bug Fixes
- Fixed the "savings by service" chart stacking behavior to properly display current and potential savings datasets together.
Style
- Enhanced chart readability by adjusting the vertical sizing constraints of the savings chart.
Tests
- Updated test suite to validate stacked bar chart configuration and dataset ordering.

Merge-order dependency

The headline visual change (Current committed savings stacked below Potential) only renders once the backend PR fix/current-savings-per-service is merged and deployed. Until that PR lands and populates ServiceSavings.CurrentSavings, the base segment of each stacked bar is zero and the chart looks identical to the previous grouped layout.

cristim · 2026-06-02T18:49:40Z

@coderabbitai review

coderabbitai · 2026-06-02T18:49:40Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c4556490-9bbe-4eda-a78b-f91a2e6880b7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR reconfigures the "savings-by-service" bar chart to stack "Current (committed)" at the base of a unified dataset stack with potential-range datasets, updates tooltip detection logic, and adds CSS sizing for the updated layout. Test assertions and new coverage validate the stacking behavior and dataset ordering.

Changes

Savings by Service Chart Stacking

Layer / File(s)	Summary
Chart dataset stacking and tooltip logic `frontend/src/dashboard.ts`	Reconfigures the datasets array so "Current (committed)", "Min potential", and "Upside" all use `stack: 'savings'` with "Current (committed)" positioned first (base). Updates `borderRadius` values for the stacked layout. Changes tooltip detection from `ctx.dataset.stack` to `ctx.dataset.label === 'Current (committed)'` to identify the current dataset.
Canvas sizing for stacked layout `frontend/src/styles/charts.css`	Adds a dedicated CSS rule for `#savings-by-service-chart` canvas to increase `max-height` and `min-height` for improved readability of the stacked bars.
Test updates and new coverage `frontend/src/__tests__/dashboard.test.ts`	Updates the "Current (committed)" dataset stack assertion to expect the shared "savings" stack. Adds new tests validating that all three datasets use a single shared stack with both axes marked stacked, and verifying dataset ordering with "Current (committed)" first, followed by "Min potential" and "Upside".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

LeanerCloud/CUDly#766: Introduced the per-service savings-range bar chart and renderSavingsByService test coverage that this PR refactors.
LeanerCloud/CUDly#782: Previously modified renderSavingsByService Chart.js stacked dataset configuration, directly related to this PR's dataset stacking and ordering changes.

Suggested labels

severity/low

Poem

🐰 Stacked bars now dance as one sweet line,
With "Current" first, all datasets align,
Labels detect what the tooltips shall show,
Canvas sizes large as the rankings grow,
Tests confirm the order flows just right!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the two main changes: increasing chart height by 2x and stacking the Current (committed) dataset under the Potential range datasets.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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/home-chart-stack-tall

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

coderabbitai · 2026-06-02T18:49:45Z

✅ 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-06-03T20:53:08Z

@coderabbitai review

coderabbitai · 2026-06-03T20:53:14Z

✅ Action performed

Review finished.

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-06-04T13:51:56Z

@coderabbitai review

coderabbitai · 2026-06-04T13:52:02Z

✅ Action performed

Review finished.

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-06-05T11:44:13Z

@coderabbitai review

coderabbitai · 2026-06-05T11:44:19Z

✅ Action performed

Review finished.

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-06-05T13:46:16Z

Verification note: PR #968 restacks the same savings-by-service chart into the single stack:'savings' layout and is verified correct on top of the already-merged backend (#926). That makes this PR's grouped-bar approach stale. Recommend closing #925 as superseded by #968 (leaving open for a human to confirm).

…ervice chart - Add #savings-by-service-chart CSS rule (600/560 px) scoped to that canvas only; the generic .chart-section canvas rule (300/280 px) is untouched so other charts are unaffected. - Merge all three datasets into a single "savings" stack so Current (committed) renders visually below the potential range (bottom-to-top = dataset order in Chart.js stacked bars). - Update tooltip discriminator from stack === 'current' to label match since all datasets now share the same stack name. - Extend dashboard tests: assert stacked axes, single-stack invariant, and dataset order (Current first, then Min potential, then Upside).

cristim · 2026-06-05T17:03:57Z

Rebased on feat/multicloud-web-frontend. Resolved conflicts in frontend/src/dashboard.ts and frontend/src/tests/dashboard.test.ts: kept base dataset labels ('Current / Committed', 'Lowest option') aligned with #968, merged the tooltip improvement (dataset-specific single-line for the current layer), updated test label references to match. CR billing is blocked org-wide; no @coderabbitai review ping.

coderabbitai · 2026-06-05T17:04:17Z

Tip

For best results, initiate chat on the files or code changes.

@cristim Thanks for the update — noted. Conflict resolution looks consistent with what #968 landed: unified dataset labels ('Current / Committed', 'Lowest option') and the tooltip discriminator switched to ctx.dataset.label. The rebase keeps this branch current if a human reviewer does decide to merge it instead of closing as superseded.

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 277
File: frontend/src/__tests__/recommendations.test.ts:2472-2476
Timestamp: 2026-05-05T07:46:01.902Z
Learning: In the CUDly frontend (`frontend/src/recommendations.ts`), `effectiveSavingsPct` intentionally has NO plausibility guard (no null return or warning for reconstructed percentages above per-term ceilings). The maintainer explicitly rejected that approach in PR `#277`. The fix for inflated percentages (issue `#274`) is purely a data-path fix: plumbing the provider's canonical `on_demand_cost` through to the frontend so the denominator is not reconstructed from potentially misleading `monthly_cost + savings + amortized`. Tests that pin the reconstructed >80% behavior are intentional documentation of the pre-fix path, not bugs.

cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship urgency/eventually No deadline impact/internal Team-internal only effort/xs Trivial / one-liner type/feat New capability labels Jun 2, 2026

cristim force-pushed the feat/home-chart-stack-tall branch from d0687f7 to 4604f33 Compare June 5, 2026 17:03

cristim merged commit f7c0e87 into feat/multicloud-web-frontend Jun 6, 2026
6 checks passed

cristim mentioned this pull request Jun 26, 2026

fix(api): current_savings zero for services with no active commitments (closes #1031) #1041

Merged

7 tasks

cristim deleted the feat/home-chart-stack-tall branch July 27, 2026 11:09

Uh oh!

Conversation

cristim commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dependency note

Test plan

Canvas selector used

Summary by CodeRabbit

Merge-order dependency

Uh oh!

cristim commented Jun 2, 2026

Uh oh!

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

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Uh oh!

coderabbitai Bot commented Jun 2, 2026

Uh oh!

cristim commented Jun 3, 2026

Uh oh!

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

Uh oh!

Uh oh!

cristim commented Jun 4, 2026

Uh oh!

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

Uh oh!

Uh oh!

cristim commented Jun 5, 2026

Uh oh!

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

Uh oh!

Uh oh!

cristim commented Jun 5, 2026

Uh oh!

cristim commented Jun 5, 2026

Uh oh!

coderabbitai Bot commented Jun 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Jun 2, 2026 •

edited

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading

coderabbitai Bot commented Jun 4, 2026 •

edited

Loading

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading