fix(api/dashboard): populate ServiceSavings.CurrentSavings from active commitments

[cristim]

Problem

ServiceSavings.CurrentSavings was declared in internal/api/types.go but never populated. The frontend at frontend/src/dashboard.ts reads byService[s]?.current_savings for the green bar dataset, so every service rendered as $0. summarizeRecommendationsWithCoverage only filled PotentialSavings; no code path ever aggregated active purchase history into the per-service map.

Changes

• aggregateActiveCommitmentsPerService (new): pure function that sums EstimatedSavings of active (non-expired) rows from purchase history, grouped by Service. This is the shared primitive for both the KPI total and the per-service chart so both paths apply the same isActiveCommitment gate.
• calculateCommitmentMetrics: now returns a fourth value -- savingsByService map[string]float64 -- and derives committedMonthly from the per-service map (previously summed inline). No change to the returned KPI values.
• getDashboardSummary: after the recommendations reducer runs, merges the per-service commitment map into byService, setting entry.CurrentSavings for each service.

Tests added

Three new tests, 9 new sub-cases total:

• TestAggregateActiveCommitmentsPerService: two active services accumulate independently; one expired + one active row for the same service; all-expired returns empty map.
• TestHandler_getDashboardSummary_CurrentSavingsPopulated: EC2 + RDS active commitments with an expired EC2 row that must not be counted; asserts both PotentialSavings and CurrentSavings are correct in the response struct.
• TestHandler_getDashboardSummary_CurrentSavingsJSON: asserts the current_savings field carries the active purchase's EstimatedSavings through the response.

All existing TestHandler_calculateCommitmentMetrics cases updated for the new 4-return-value signature.

Coordination with sibling PRs

If a fix/committed-monthly PR adds status filtering (e.g. only counting "succeeded" rows) or unit normalisation to calculateCommitmentMetrics, aggregateActiveCommitmentsPerService is the single place to update. Both the KPI total and the per-service breakdown flow through it. This PR should merge after any such sibling so the gating is consistent from day one -- or the sibling can build on top of this primitive.

Test run

go test ./internal/api/...: 1391 passed
frontend jest --testPathPattern=dashboard: 83 passed, 1 skipped (pre-existing)

[cristim]

@coderabbitai review

[coderabbitai]

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 13 minutes and 28 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: b5d0f4be-0e2f-47d0-8588-e2dbb2cf9d19

📥 Commits

Reviewing files that changed from the base of the PR and between 623758c and b0820dd.

📒 Files selected for processing (2)

• internal/api/handler_dashboard.go
• internal/api/handler_dashboard_test.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/current-savings-per-service

---

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

[coderabbitai]

✅ 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]

Rebased on feat/multicloud-web-frontend (now includes #927's committed_monthly tightening). Resolved conflicts in internal/api/handler_dashboard.go and internal/api/handler_dashboard_test.go.

The two PRs touched the same calculateCommitmentMetrics code path, so this was a semantic (not purely additive) merge. Reconciliation:

• Kept fix(api/dashboard): tighten committed_monthly aggregation (status + unit + account scope) #927's tightened isActiveCommitment (term-not-expired AND successful status) and its 3-way account-scope fetch switch (GetPurchaseHistoryFiltered / GetPurchaseHistory / GetAllPurchaseHistory).
• Layered fix(api/dashboard): populate ServiceSavings.CurrentSavings from active commitments #926's per-service CurrentSavings on top: calculateCommitmentMetrics gains a 4th return value (savingsByService), derived from the extracted aggregateActiveCommitmentsPerService primitive.
• committedMonthly is now summed from that same per-service map instead of inline in the loop. Because the map is built with the shared isActiveCommitment gate, it transparently inherits fix(api/dashboard): tighten committed_monthly aggregation (status + unit + account scope) #927's status filter, so the KPI total stays identical and there is no double counting (dropped fix(api/dashboard): tighten committed_monthly aggregation (status + unit + account scope) #927's inline committedMonthly += p.EstimatedSavings).
• Updated the two getDashboardSummary CurrentSavings tests to mock GetAllPurchaseHistory (the no-filter fetch path post-fix(api/dashboard): tighten committed_monthly aggregation (status + unit + account scope) #927) and added the 4th return-value discard to fix(api/dashboard): tighten committed_monthly aggregation (status + unit + account scope) #927's status-filter and UUID-filter unit tests.

Verified: gofmt clean, go vet clean, go build ./... ok, go test ./internal/api/... -count=1 -> 1412 passed.

@coderabbitai review

[coderabbitai]

@cristim: Thanks for the detailed rebase summary. The reconciliation approach looks solid — deriving committedMonthly from the savingsByService map rather than maintaining a separate inline accumulator is a clean way to ensure the KPI total and per-service breakdown stay in sync while transparently inheriting #927's isActiveCommitment gate. Triggering a fresh review of the rebased commits now.

---

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

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-25T21:45:57.471Z
Learning: In this repository (CUDly), GCP CUD commitments are inherently monthly-billed. The GCP CUD purchase API (`providers/gcp/services/computeengine/client.go:350-373`, `buildCommitmentRequests`) takes only a `Plan` field (TWELVE_MONTH / THIRTY_SIX_MONTH) and never reads a payment-option field. Therefore, the only valid payment option for GCP in `ValidPaymentOptionsByProvider` is `"monthly"`. The `NormalizePaymentOption` GCP branch should collapse any non-monthly token (including legacy `"upfront"`) to `"monthly"`, with a WARN log at the `scheduler.convertRecommendations` boundary.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 339
File: providers/aws/recommendations/coverage.go:177-205
Timestamp: 2026-05-13T21:30:43.093Z
Learning: In the CUDly project (LeanerCloud/CUDly), the CLI flag for target-based coverage sizing is `--target-coverage` (not `--target-utilization`). The PR renamed it during design iteration: "utilization" (how full each RI is) stays ~100% under the new sizing, while "coverage" (% of demand covered) is the user-facing knob. `ce:GetReservationCoverage` is required only when `--target-coverage` is set, gated by `cfg.TargetCoverage <= 0` in `cmd/multi_service.go`. The legacy `--coverage` path is unaffected and pays no CE cost or IAM change.

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