fix(frontend/recs): effectiveSavingsPct returns null when denominator is zero (closes #323)

[cristim]

Summary

• effectiveSavingsPct now returns null for AWS rows where on_demand_cost is absent or zero, so the UI renders — instead of a value computed from the inaccurate reconstruction formula (monthly_cost + savings + amortized_upfront)
• Azure rows retain the reconstruction fallback since older cached rows may legitimately omit on_demand_cost
• Three new test cases: AWS without on_demand_cost (null), AWS with on_demand_cost=0 (null), AWS with valid on_demand_cost (computes correctly)

Root cause

For AWS RI/SP recs, EstimatedMonthlyOnDemandCost from Cost Explorer is the canonical denominator. When it is absent from the CE response the backend logs a warning and stores zero, which the frontend converts to null via the nonZeroPtr convention. The reconstruction formula then picks monthly_cost + savings + amortized_upfront as the denominator, which diverges from the true on-demand baseline and can produce percentages well above realistic savings ceilings (100%+).

Test plan

• cd frontend && npx jest --no-coverage --testPathPattern=recommendations.test.ts -- 322 tests pass
• cd frontend && npx tsc --noEmit -- clean
• AWS rec row in the UI with no on_demand_cost renders — in the Effective % column instead of an inflated value
• AWS rec row with on_demand_cost populated continues to render the correct percentage

[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 59 minutes and 25 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: 42fef711-9726-4cfb-89b9-e9b0f437a425

📥 Commits

Reviewing files that changed from the base of the PR and between 4956d66 and 5866dc6.

📒 Files selected for processing (2)

• frontend/src/__tests__/recommendations.test.ts
• frontend/src/recommendations.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/323-wave10

---

_{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]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[cristim]

@coderabbitai review

(Re-pinging: the prior full-review request on 2026-05-30 was acknowledged but no review body was posted, likely because of the org-level usage-credit exhaustion shown in the earlier rate-limit warning. PR has been CLEAN with CI green for several days; please post any findings so they can be addressed before merge.)

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the PR diff
gh pr view 832 --repo LeanerCloud/CUDly --json title,body,files,additions,deletions,commits
echo "---DIFF---"
gh pr diff 832 --repo LeanerCloud/CUDly

Length of output: 9580

---

🏁 Script executed:

#!/bin/bash
# Check what provider values are used in the codebase
rg -n "provider.*['\"]" frontend/src/ --type ts | grep -v "test" | grep -v ".d.ts" | head -40
echo "---"
# Check LocalRecommendation type definition for provider field
rg -n "provider" frontend/src/recommendations.ts | head -30
echo "---"
# Check if there are any GCP or other providers
rg -n "'gcp'\|'google'\|'azure'\|'aws'" frontend/src/ --type ts | head -30

Length of output: 8143

---

---

@cristim: The implementation looks correct and the tests are thorough. Here are my findings:

---

frontend/src/recommendations.ts

The new guard is well-placed and the ordering is correct:

const hasOnDemand = r.on_demand_cost != null && r.on_demand_cost > 0;
if (r.monthly_cost == null && !hasOnDemand) return null;   // existing – all providers, both absent
if (r.provider === 'aws' && !hasOnDemand) return null;     // new – AWS, missing CE denominator

The on_demand_cost === 0 case is correctly caught by hasOnDemand (> 0), consistent with the backend nonZeroPtr convention. ✅

---

frontend/src/__tests__/recommendations.test.ts

• mk() default switch from 'aws' to 'azure' correctly preserves existing reconstruction-path test semantics. ✅
• Three new AWS tests are well-specified; math checks out:
  • Valid case: savings=100, upfront=0, term=1, on_demand_cost=300 → amortized=0, effectiveSavings=100, pct=33.33% ✅
  • All-upfront rendering: savings=50, upfront=600, term=1, on_demand_cost=100 → amortized=50, effectiveSavings=0, pct=0.0% ✅
  • Negative rendering: savings=10, upfront=1200, term=1, on_demand_cost=510 → amortized=100, effectiveSavings=-90, pct=−17.65% ✅

---

One question worth confirming

The codebase has a third provider: gcp (compute, sql, memorystore, storage — CUDs). With this PR, GCP rows silently fall through to the reconstruction formula (monthly_cost + savings + amortized) if on_demand_cost is absent. Is that intentional? If GCP's backend also uses a nonZeroPtr-like convention for its on-demand baseline, a missing on_demand_cost could produce the same inflated-percentage issue for GCP rows.

If GCP's reconstruction formula is known to be reliable (or GCP always populates on_demand_cost), this is fine as-is and worth a brief code comment to document the intent. If it's uncertain, a follow-up issue might be warranted.

---

No blocking issues. The fix is clean and well-tested for the stated scope.

---

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