fix(recommendations/aws): flush stale on_demand_cost cache + parser visibility (closes #321)

internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.down.sql

@@ -0,0 +1,5 @@
+-- No rollback for the payload rewrite: the original AWS rows lacking
+-- on_demand_cost were themselves stale (pre-PR-#312 data). Reverting
+-- monthly_cost from null back to those stale values would re-introduce
+-- the very bug this migration fixes. The correct values will be written
+-- on the next scheduled collection.

internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.up.sql

@@ -0,0 +1,30 @@
+-- Rewrite stale pre-PR-#312 AWS recommendation payloads that lack
+-- on_demand_cost in their JSONB payload.
+--
+-- PR #312 added on_demand_cost population for AWS Savings Plans rows
+-- (via CurrentAverageHourlyOnDemandSpend × 730). PR #277 added it for
+-- Azure rows. Neither shipped a cache-invalidation migration for the
+-- existing AWS rows already in the DB.
+--
+-- Pre-#312 AWS rows have no on_demand_cost key, so the frontend falls
+-- back to reconstructing the on-demand denominator from
+-- monthly_cost + savings + amortized — a formula that double-counts
+-- amortization vs. how AWS computes its savings amounts, producing
+-- implausibly high Effective % values.
+--
+-- Setting monthly_cost to null here causes the frontend to render "—"
+-- instead, which is accurate ("data not yet collected") until the next
+-- scheduler tick re-collects with correct on_demand_cost values.
+--
+-- NOTE: We scope this strictly to AWS rows that lack on_demand_cost.
+-- Azure rows were handled by migration 000046. GCP rows are not affected.
+--
+-- NOTE: This UPDATE is idempotent — once a row has on_demand_cost
+-- (written by the next scheduler tick), the NOT (payload ? 'on_demand_cost')
+-- condition no longer matches it, so re-running this migration is safe.
+--
+-- See GitHub issue #321 for the full root-cause investigation.
+UPDATE recommendations
+   SET payload = jsonb_set(payload, '{monthly_cost}', 'null'::jsonb)
+ WHERE payload->>'provider' = 'aws'
+   AND NOT (payload ? 'on_demand_cost');

providers/aws/recommendations/parser_ri.go

@@ -2,6 +2,7 @@ package recommendations
 
 import (
 	"fmt"
+	"log"
 	"math"
 	"strconv"
 	"time"
@@ -127,6 +128,12 @@ func (c *Client) parseAWSCostDetails(rec *common.Recommendation, details *types.
 		if onDemand, err := strconv.ParseFloat(*details.EstimatedMonthlyOnDemandCost, 64); err == nil {
 			rec.OnDemandCost = onDemand
 		}
+	} else {
+		// EstimatedMonthlyOnDemandCost absent from AWS CE response — OnDemandCost
+		// will be 0 and the scheduler's nonZeroPtr will store nil, causing the
+		// frontend to fall back to the reconstruction formula. Log so operators
+		// can detect when the API field is missing. See #321.
+		log.Printf("WARNING: EstimatedMonthlyOnDemandCost is nil for RI recommendation (service=%s, account=%s) — Effective %% will use reconstruction fallback", rec.Service, rec.Account)
 	}
 	// RecurringStandardMonthlyCost is the recurring charge per month for this RI.
 	// It is distinct from CommitmentCost (upfront) and EstimatedMonthlySavingsAmount.

providers/aws/recommendations/parser_sp.go

@@ -147,6 +147,13 @@ func (c *Client) parseSavingsPlanDetail(
 	// where monthly_cost reflects only the no-upfront recurring charge, not
 	// the full on-demand baseline). See #303.
 	onDemandCost := parseOptionalFloat("CurrentAverageHourlyOnDemandSpend", detail.CurrentAverageHourlyOnDemandSpend) * hoursPerMonth
+	if detail.CurrentAverageHourlyOnDemandSpend == nil {
+		// CurrentAverageHourlyOnDemandSpend absent from AWS CE response — onDemandCost
+		// will be 0 and the scheduler's nonZeroPtr will store nil, causing the
+		// frontend to fall back to the reconstruction formula. Log so operators
+		// can detect when the API field is missing. See #321.
+		log.Printf("WARNING: CurrentAverageHourlyOnDemandSpend is nil for SP recommendation (planType=%s, account=%s) — Effective %% will use reconstruction fallback", planType, aws.ToString(detail.AccountId))
+	}
 
 	planTypeStr := string(planType)
 	switch planType {