fix(recommendations): plumb provider on_demand_cost through to frontend (closes #274)

[cristim]

Summary

Closes #274. Fixes the implausibly-high "Effective %" values by
plumbing the cloud providers' canonical on-demand baseline through to
the frontend.

Before: the frontend reconstructed the on-demand denominator from
monthly_cost + savings + amortized_upfront. For Azure all-upfront
recs monthly_cost = $0, so the reconstruction collapses to
savings + amortized — much smaller than the real on-demand cost.
Result: the Standard_D11_v2 1y all-upfront row from the screenshot
read 85.9%.

After: when the provider populates on_demand_cost (Azure
CostWithNoReservedInstances, AWS EstimatedMonthlyOnDemandCost),
the frontend uses it directly. With the real on-demand of $122.64 for
the same row, the percentage drops to 21.9% — within realistic
1-year RI ceilings.

Plumbing path

provider SDK
  → pkg/common/types.go::Recommendation.OnDemandCost  (already populated)
  → internal/config/types.go::RecommendationRecord.OnDemandCost  ← NEW
  → JSONB `payload` column in recommendations table  (no DDL migration —
                                                      record persisted as
                                                      JSON via existing
                                                      payload column)
  → internal/api/types.go (uses config.RecommendationRecord directly — no API change)
  → frontend/src/api/types.ts::Recommendation.on_demand_cost  ← NEW
  → frontend/src/types.ts::LocalRecommendation.on_demand_cost  ← NEW
  → effectiveSavingsPct prefers it; falls back to reconstruction if null/0

Compatibility

• Older cached recs without on_demand_cost → fallback to the
  previous reconstruction formula (visually unchanged).
• Provider returns 0 → nonZeroPtr writes nil so the frontend
  falls back. Defensive: a literal $0 baseline is impossible (the
  resource would be free).
• monthly_cost === null + populated on_demand_cost → previously
  returned null (em-dash); now returns a real value. New rescue path.

Test plan

• TestScheduler_ConvertRecommendations_OnDemandCost — pins
  populated values round-trip and 0 → nil.
• 5 new frontend tests in effectiveSavingsPct's
  "on_demand_cost preference (bug(frontend/recs): Effective % shows implausible values (85.9% on Azure 1y RI, 54% on savings=$0 row) #274)" sub-describe:
  • live D11_v2 repro (86% reconstructed → 22% with baseline)
  • override behaviour (baseline wins over reconstruction)
  • null fallback (back-compat with older cached recs)
  • 0 fallback (defensive — provider returned 0)
  • rescue of monthly_cost === null when baseline is populated
• npm test -- --testPathPattern=recommendations — 145 / 145 pass.
• npm run typecheck — clean.
• go test ./internal/scheduler/... — pass.
• gocyclo -over 10 clean (helper extraction keeps
  convertRecommendations under the gate).

🤖 Generated with claude-flow

Summary by CodeRabbit

• New Features

  • Recommendations now utilize provider-supplied on-demand cost baselines for more accurate calculations.

• Bug Fixes

  • Fixed inflated effective savings percentage calculations for all-upfront pricing scenarios.
  • Improved fallback logic when on-demand cost baseline data is unavailable.

[cristim]

@coderabbitai review

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce an on_demand_cost field across frontend and backend types to represent the provider-supplied on-demand baseline, update the effective savings percentage formula to prefer this field when present, and modify the scheduler to persist it correctly, treating zero values as unpopulated (nil) to fix implausibly high percentages on Azure all-upfront recommendations.

Changes

On-Demand Baseline Integration

Layer / File(s)	Summary
Data Shape frontend/src/api/types.ts, frontend/src/types.ts, internal/config/types.go	Added on_demand_cost?: number | null (frontend) and OnDemandCost \*float64 (backend) fields to Recommendation, LocalRecommendation, and RecommendationRecord types with documentation clarifying it is the canonical provider-supplied baseline.
Core Calculation frontend/src/recommendations.ts	Updated effectiveSavingsPct to prefer on_demand_cost as the denominator when present and > 0, return null early when both monthly_cost and on_demand_cost are missing, and reconstruct denominator only as fallback using updated onDemand = monthly_cost + savings + amortized formula.
Persistence & Nullability internal/scheduler/scheduler.go	Added nonZeroPtr helper to convert provider OnDemandCost to nullable pointer, storing nil when value is 0 to prevent zero from being treated as a populated baseline.
Tests frontend/src/__tests__/recommendations.test.ts, internal/scheduler/scheduler_test.go	Added test suite for #274 covering on_demand_cost preference, fallback behavior, and zero-value handling; added TestScheduler_ConvertRecommendations_OnDemandCost to verify round-trip persistence of non-zero and zero baselines.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

• fix(frontend/recs): Monthly cost column always $0; Effective % shows 100% / negatives — scheduler hardcodes MonthlyCost=0 #252: The previous round of fixes (#254) addressed similar symptoms in the effective savings percentage formula; this PR completes the fix by sourcing the on-demand baseline directly from providers rather than reconstructing it, directly solving the "85.9% on Azure 1y RI" and "54% on zero-savings row" problems described.

Possibly related PRs

• LeanerCloud/CUDly#256: Both treat zero vs. null semantics specially to prevent zero values from being interpreted as populated baselines.
• LeanerCloud/CUDly#254: Both update the effectiveSavingsPct denominator logic to improve accuracy of the savings percentage calculation.
• LeanerCloud/CUDly#242: Introduces related changes to the same effectiveSavingsPct function and its test suite.

Poem

🐰 A cost baseline hops into view,
No more reconstructions (that math was askew),
Percentages plausible, zeros now null,
The effective savings is finally truthful! 📊

🚥 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 change: plumbing the provider's on_demand_cost field through to the frontend to fix implausible Effective % values (issue #274).
Linked Issues check	✅ Passed	The PR addresses all key coding objectives from #274: plumbing on_demand_cost through the stack, updating type definitions, implementing frontend fallback logic, and adding comprehensive tests for the new behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the on_demand_cost plumbing and associated fallback logic specified in #274; no unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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/recs-effective-pct-274

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

frontend/src/recommendations.ts (1)

414-431: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add the term-based sanity cap before returning effectiveSavingsPct.

Using on_demand_cost fixes the denominator, but this still returns arbitrarily large Effective % values when provider data is stale or scaled incorrectly. Issue #274 explicitly called for rendering implausible values as unknown, so without the guard the UI can still surface nonsense percentages.

Suggested guard

   const onDemand = hasOnDemand
     ? (r.on_demand_cost as number)
     : (r.monthly_cost as number) + r.savings + amortized;
   if (onDemand === 0) return null;
-  return (effectiveSavings / onDemand) * 100;
+  const pct = (effectiveSavings / onDemand) * 100;
+  const maxPct = r.term === 1 ? 60 : r.term === 3 ? 75 : null;
+  if (maxPct !== null && pct > maxPct) return null;
+  return pct;

🤖 Prompt for 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.

In `@frontend/src/recommendations.ts` around lines 414 - 431, The
effectiveSavingsPct function can still return implausibly large percentages; add
a term-based sanity cap before returning: define a constant
MAX_EFFECTIVE_SAVINGS_PCT (e.g. 1000) and compute a threshold scaled by term
(e.g. threshold = MAX_EFFECTIVE_SAVINGS_PCT / r.term), then after you compute
pct = (effectiveSavings / onDemand) * 100 check if Math.abs(pct) > threshold and
return null if so, otherwise return pct; update the effectiveSavingsPct function
and use symbols monthsInTerm / r.term, effectiveSavings, onDemand, and the new
MAX_EFFECTIVE_SAVINGS_PCT to locate and apply the guard.

🤖 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__/recommendations.test.ts`:
- Around line 2472-2476: The test currently expects an implausible high
reconstructed percentage from effectiveSavingsPct(mk(...)) but should validate
the "unknown + warning" guardrail; change the assertion for pctReconstructed
produced by effectiveSavingsPct (called with mk({ savings: 29, upfront_cost: 26,
monthly_cost: 0, term: 1 })) to assert it returns the unknown sentinel (e.g.,
undefined or null) rather than >80, and add an expectation that a warning was
emitted (use the existing warning/log spy in the test harness or spy on
console.warn) to confirm the guardrail path was taken.

---

Outside diff comments:
In `@frontend/src/recommendations.ts`:
- Around line 414-431: The effectiveSavingsPct function can still return
implausibly large percentages; add a term-based sanity cap before returning:
define a constant MAX_EFFECTIVE_SAVINGS_PCT (e.g. 1000) and compute a threshold
scaled by term (e.g. threshold = MAX_EFFECTIVE_SAVINGS_PCT / r.term), then after
you compute pct = (effectiveSavings / onDemand) * 100 check if Math.abs(pct) >
threshold and return null if so, otherwise return pct; update the
effectiveSavingsPct function and use symbols monthsInTerm / r.term,
effectiveSavings, onDemand, and the new MAX_EFFECTIVE_SAVINGS_PCT to locate and
apply the guard.

🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

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: 2ec56332-14be-474d-8d36-054d1f549c1e

📥 Commits

Reviewing files that changed from the base of the PR and between e010987 and a696a29.

📒 Files selected for processing (7)

• frontend/src/__tests__/recommendations.test.ts
• frontend/src/api/types.ts
• frontend/src/recommendations.ts
• frontend/src/types.ts
• internal/config/types.go
• internal/scheduler/scheduler.go
• internal/scheduler/scheduler_test.go

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found.