fix(recs): stop double-subtracting amortized upfront in server pct

[cristim]

Problem

Closes #1148 (review finding COR-01, P1).

recEffectiveSavingsPct in internal/config/store_postgres_recommendations.go computed the server-side effective savings percentage as (savings - amortized_upfront) / on_demand, but all three providers (AWS, Azure, GCP) report savings as already-net monthly savings, with amortized upfront factored in. The frontend was fixed for this in #1103/#1104, but the backend mirror diverged: the min_savings_pct query-param floor in ListStoredRecommendations filtered on an understated, and for high-upfront RIs negative, percentage while the UI displayed the correct one. Recommendations whose true effective savings exceeded the floor were silently dropped from API results, penalizing all-upfront and partial-upfront RIs hardest.

Fix

Realign the backend with the frontend's displaySavingsPct / effectiveSavingsPct:

• Numerator is rec.Savings directly; amortized upfront remains only in the on-demand denominator reconstruction (monthly_cost + savings + amortized).
• Prefer the provider-authoritative rec.SavingsPercentage when non-nil and finite, mirroring displaySavingsPct.
• AWS rows without an explicit on-demand baseline return (0, false) instead of a reconstructed value that diverges from Cost Explorer's true baseline (the frontend's fix(frontend/recs): return null from effectiveSavingsPct on AWS reconstruction fallback (followup to #321) #323 guard). Uncomputable rows pass through the floor unchanged, as before.

A local providerAWS constant is used because internal/config must not import pkg/common (documented dependency direction).

Also fixed the unit tests in store_postgres_savings_filter_test.go that entrenched the double-subtract formula, and added regression tests for the high-upfront negative-pct shape, the savings_percentage preference, and the AWS no-baseline guard.

Test evidence

• go build ./... passes.
• go test ./internal/config/...: 585 tests pass.
• Pre-fix verification: stashing the fix and re-running makes all four new/updated subtests fail, including high-upfront RI is not driven negative (#1148 regression) where the old formula yields -25% for a 3yr all-upfront RI whose true effective savings are +25% (silently dropped by any positive min_savings_pct floor).
• go vet ./internal/config/: clean.

Summary by CodeRabbit

Bug Fixes

• Corrected the effective savings percentage calculation used for filtering and displaying recommendations
• AWS recommendations now appropriately indicate "not computable" when baseline cost data is unavailable, preventing potentially inaccurate estimates
• Fixed savings computation logic to eliminate double-subtraction of amortized upfront costs
• Improved alignment between server-side and client-side savings percentage semantics

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1bdc3204-1cf9-49e4-af91-383f88b9fb3f

📥 Commits

Reviewing files that changed from the base of the PR and between 006aba9 and 94d0d92.

📒 Files selected for processing (2)

• internal/config/store_postgres_recommendations.go
• internal/config/store_postgres_savings_filter_test.go

---

📝 Walkthrough

Walkthrough

The PR fixes server-side effective savings percentage computation in recEffectiveSavingsPct, which filters recommendations by MinSavingsPct. Changes prefer explicit finite SavingsPercentage, remove double-subtraction of amortized upfront costs, add AWS on-demand-cost absence guards, and expand test coverage with corrected assertions and regression cases.

Changes

Effective Savings Percentage Computation and Tests

Layer / File(s)	Summary
Effective savings percentage computation rewrite internal/config/store_postgres_recommendations.go	math import added; recEffectiveSavingsPct rewritten to prefer explicit finite SavingsPercentage, reconstruct on-demand baselines conditionally, use rec.Savings directly (no double-subtract of amortized upfront), and return "not computable" for AWS records lacking on-demand cost.
Test coverage for savings percentage filtering and edge cases internal/config/store_postgres_savings_filter_test.go	math import added; existing test case corrected to assert no double-subtraction from net savings; new subtests added for high-upfront RI floor constraints, provider SavingsPercentage preference, non-finite fallback, and AWS on-demand-cost absence guards.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• LeanerCloud/CUDly#1148: Directly addresses the server-side recEffectiveSavingsPct double-amortized numerator fix, provider SavingsPercentage preference, AWS no-on-demand guard, and semantic alignment with frontend logic.

Possibly related PRs

• LeanerCloud/CUDly#312: Adjusts Effective Savings % denominator around provider-supplied on-demand cost and SavingsPercentage semantics (plumbs OnDemandCost for AWS SP; main PR updates backend recomputation rules and AWS "not computable" behavior).
• LeanerCloud/CUDly#277: Updates effective savings % to prefer/require valid on-demand baselines and avoid reconstruction when missing (main PR adjusts server-side recEffectiveSavingsPct logic and tests; retrieved PR adjusts frontend effectiveSavingsPct denominator).
• LeanerCloud/CUDly#324: Targets AWS effective savings % path when on_demand_cost is missing (#324 stubs missing data in JSON; main PR adjusts recEffectiveSavingsPct reconstruction and "not computable" handling).

Poem

🐰 Hop, hop! The savings math now flows so clean,
No double-counts, no amortized cheat;
AWS knows when baseline's unseen,
And test-cases dance to the provider's beat! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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 fix: correcting the double-subtraction of amortized upfront costs in the server-side percentage calculation for recommendations.
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.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cor-01-effective-savings-pct

---

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

[cristim]

@coderabbitai review

[coderabbitai]

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