fix(azure): derive covered monthly cost instead of hardcoding 0 (GUI showed on-demand)

providers/azure/internal/recommendations/converter.go

@@ -49,11 +49,22 @@ type ExtractedFields struct {
 	// subscription scoped recommendations are ever requested. See finding M1/M2
 	// in docs/code-review/09-provider-azure.md.
 	Scope string
-	// RecurringMonthlyCost is the monthly recurring charge for this
-	// commitment. Azure Reservation recommendations are all all-upfront
-	// (a single payment, no recurring monthly charge), so this is always
-	// a pointer to 0.0 — meaning "no recurring charge" rather than
-	// "data not available" (which would be nil).
+	// RecurringMonthlyCost is the covered/effective recurring cost for this
+	// commitment, i.e. what the customer pays WITH the reservation in place.
+	// The frontend renders this column as the "covered" spend; leaving it 0
+	// makes the GUI fall back to displaying OnDemandCost (the spend WITHOUT
+	// any reservation), which is the opposite of the intended figure.
+	//
+	// The value is sourced from TotalCostWithReservedInstances (preferred);
+	// if that field is absent it is reconstructed as OnDemandCost - NetSavings
+	// (covered = on-demand minus net savings). Like OnDemandCost and
+	// EstimatedSavings, the figure is over Azure's lookback period and is
+	// treated downstream as a monthly run-rate.
+	//
+	// nil means the provider returned neither a total-with-RI nor an
+	// (on-demand, net-savings) pair to reconstruct it from. nil renders as
+	// "—" (data not available); it is NEVER set to a fabricated 0 (which
+	// would falsely claim "free recurring charge").
 	RecurringMonthlyCost *float64
 }
 
@@ -63,6 +74,35 @@ func float64Ptr(v float64) *float64 {
 	return &v
 }
 
+// deriveCoveredMonthlyCost computes the covered/effective recurring cost
+// (what the customer pays WITH the reservation) used to populate
+// ExtractedFields.RecurringMonthlyCost. Inputs are the already-normalised
+// pointers from either response shape:
+//
+//   - totalWithRI: TotalCostWithReservedInstances (preferred, authoritative).
+//   - onDemand / netSavings: used to reconstruct covered = on-demand - net
+//     savings only when totalWithRI is absent.
+//
+// Returns nil (NOT 0) when neither source is available, and logs a warning
+// so the gap surfaces instead of silently shipping a fabricated figure. The
+// returned value is over Azure's lookback period and treated as a monthly
+// run-rate downstream, consistent with OnDemandCost/EstimatedSavings.
+func deriveCoveredMonthlyCost(resourceType string, totalWithRI, onDemand, netSavings *float64) *float64 {
+	if totalWithRI != nil {
+		return float64Ptr(*totalWithRI)
+	}
+	if onDemand != nil && netSavings != nil {
+		return float64Ptr(*onDemand - *netSavings)
+	}
+	logging.Warnf(
+		"azure recommendations: covered monthly cost unavailable for %q "+
+			"(no TotalCostWithReservedInstances and no on-demand/net-savings pair); "+
+			"leaving RecurringMonthlyCost nil",
+		resourceType,
+	)
+	return nil
+}
+
 // Extract reads the Azure reservation recommendation payload into
 // *ExtractedFields, normalising the Legacy/Modern shape difference.
 // Returns nil if the input is:
@@ -126,10 +166,15 @@ func extractLegacy(rec *armconsumption.LegacyReservationRecommendation) *Extract
 		// depends on (issue #215 audit).
 		out.EstimatedSavings = *props.NetSavings
 	}
-	// Azure Reservation recommendations are always all-upfront (single payment,
-	// no monthly recurring charge). Set to 0 (not nil) so the frontend renders
-	// "$0" rather than "—" (which would imply "data not available").
-	out.RecurringMonthlyCost = float64Ptr(0)
+	// Covered/effective recurring cost = what the customer pays WITH the
+	// reservation. Prefer the provider-reported total-with-RI; fall back to
+	// on-demand minus net savings; nil (never 0) when neither is available.
+	out.RecurringMonthlyCost = deriveCoveredMonthlyCost(
+		out.ResourceType,
+		props.TotalCostWithReservedInstances,
+		props.CostWithNoReservedInstances,
+		props.NetSavings,
+	)
 	return out
 }
 
@@ -162,10 +207,17 @@ func extractModern(rec *armconsumption.ModernReservationRecommendation) *Extract
 	out.OnDemandCost = amountValue(props.CostWithNoReservedInstances)
 	out.CommitmentCost = amountValue(props.TotalCostWithReservedInstances)
 	out.EstimatedSavings = amountValue(props.NetSavings)
-	// Azure Reservation recommendations are always all-upfront (single payment,
-	// no monthly recurring charge). Set to 0 (not nil) so the frontend renders
-	// "$0" rather than "—" (which would imply "data not available").
-	out.RecurringMonthlyCost = float64Ptr(0)
+	// Covered/effective recurring cost = what the customer pays WITH the
+	// reservation. Prefer the provider-reported total-with-RI; fall back to
+	// on-demand minus net savings; nil (never 0) when neither is available.
+	// amountValuePtr preserves the "field absent" signal (nil) so the
+	// fallback/nil logic matches the Legacy path's *float64 inputs.
+	out.RecurringMonthlyCost = deriveCoveredMonthlyCost(
+		out.ResourceType,
+		amountValuePtr(props.TotalCostWithReservedInstances),
+		amountValuePtr(props.CostWithNoReservedInstances),
+		amountValuePtr(props.NetSavings),
+	)
 
 	return out
 }
@@ -253,6 +305,17 @@ func amountValue(a *armconsumption.Amount) float64 {
 	return *a.Value
 }
 
+// amountValuePtr unwraps Modern's *Amount to a *float64, preserving the
+// "field absent" signal: it returns nil (not a pointer to 0) when the
+// *Amount or its Value is missing. Used where the caller must distinguish
+// "value absent" from "value is zero" (e.g. deriveCoveredMonthlyCost).
+func amountValuePtr(a *armconsumption.Amount) *float64 {
+	if a == nil || a.Value == nil {
+		return nil
+	}
+	return float64Ptr(*a.Value)
+}
+
 func strDeref(s *string) string {
 	if s == nil {
 		return ""

providers/azure/internal/recommendations/converter_test.go

@@ -139,19 +139,52 @@ func TestExtract_MissingCostsReadAsZero(t *testing.T) {
 	assert.InDelta(t, 0.0, f.EstimatedSavings, 1e-9)
 }
 
-func TestExtract_Legacy_RecurringMonthlyCostIsZeroPointer(t *testing.T) {
-	// Azure reservations are always all-upfront (single payment, no monthly
-	// recurring charge). RecurringMonthlyCost must be a non-nil pointer to 0
-	// so the frontend renders "$0" instead of "—" (which would mean unknown).
+func TestExtract_Legacy_RecurringMonthlyCostIsCoveredCost(t *testing.T) {
+	// RecurringMonthlyCost is the covered/effective cost (what you pay WITH
+	// the reservation) = TotalCostWithReservedInstances, NOT 0 (which would
+	// make the GUI fall back to displaying the on-demand spend).
 	rec := mocks.BuildLegacyReservationRecommendation(
 		mocks.WithRegion("eastus"),
 		mocks.WithNormalizedSize("Standard_D2s_v3"),
-		mocks.WithCosts(100, 70, 30),
+		mocks.WithCosts(100, 70, 30), // on-demand, total-with-RI, net savings
+	)
+	f := Extract(rec)
+	require.NotNil(t, f)
+	require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be populated from the covered cost")
+	assert.InDelta(t, 70.0, *f.RecurringMonthlyCost, 1e-9,
+		"covered cost must equal TotalCostWithReservedInstances, not 0")
+}
+
+func TestExtract_Legacy_RecurringMonthlyCostReconstructedWhenTotalAbsent(t *testing.T) {
+	// TotalCostWithReservedInstances absent → reconstruct covered cost as
+	// on-demand minus net savings (120 - 45 = 75), still never 0.
+	withOnDemandAndSavingsOnly := func(_ *armconsumption.LegacyReservationRecommendation, props *armconsumption.LegacyReservationRecommendationProperties) {
+		onDemand := 120.0
+		savings := 45.0
+		props.CostWithNoReservedInstances = &onDemand
+		props.NetSavings = &savings
+		props.TotalCostWithReservedInstances = nil
+	}
+	rec := mocks.BuildLegacyReservationRecommendation(
+		mocks.WithNormalizedSize("Standard_D2s_v3"),
+		withOnDemandAndSavingsOnly,
+	)
+	f := Extract(rec)
+	require.NotNil(t, f)
+	require.NotNil(t, f.RecurringMonthlyCost, "covered cost must be reconstructed when total-with-RI is absent")
+	assert.InDelta(t, 75.0, *f.RecurringMonthlyCost, 1e-9)
+}
+
+func TestExtract_Legacy_RecurringMonthlyCostNilWhenSourcesAbsent(t *testing.T) {
+	// Neither total-with-RI nor a complete (on-demand, net-savings) pair →
+	// RecurringMonthlyCost must be nil ("data not available"), NEVER 0.
+	rec := mocks.BuildLegacyReservationRecommendation(
+		mocks.WithNormalizedSize("Standard_D2s_v3"),
+		// no WithCosts → all three cost pointers nil
 	)
 	f := Extract(rec)
 	require.NotNil(t, f)
-	require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure reservations")
-	assert.InDelta(t, 0.0, *f.RecurringMonthlyCost, 1e-9)
+	assert.Nil(t, f.RecurringMonthlyCost, "covered cost must be nil (absent), not a fabricated 0")
 }
 
 // --- Modern (MCA billing account) ----------------------------------------
@@ -226,19 +259,52 @@ func TestExtract_Modern_MissingCostAmountsReadAsZero(t *testing.T) {
 	assert.InDelta(t, 0.0, f.EstimatedSavings, 1e-9)
 }
 
-func TestExtract_Modern_RecurringMonthlyCostIsZeroPointer(t *testing.T) {
-	// Azure Reservation recommendations are always all-upfront regardless
-	// of billing account type. RecurringMonthlyCost must be a non-nil pointer
-	// to 0 for both Legacy and Modern response shapes.
+func TestExtract_Modern_RecurringMonthlyCostIsCoveredCost(t *testing.T) {
+	// Same covered-cost semantics as Legacy: RecurringMonthlyCost equals
+	// TotalCostWithReservedInstances, not 0.
 	rec := mocks.BuildModernReservationRecommendation(
 		mocks.WithModernRegion("westeurope"),
 		mocks.WithModernSKUName("Standard_D4s_v5"),
-		mocks.WithModernCosts(400, 260, 140),
+		mocks.WithModernCosts(400, 260, 140), // on-demand, total-with-RI, net savings
+	)
+	f := Extract(rec)
+	require.NotNil(t, f)
+	require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be populated from the covered cost")
+	assert.InDelta(t, 260.0, *f.RecurringMonthlyCost, 1e-9,
+		"covered cost must equal TotalCostWithReservedInstances, not 0")
+}
+
+func TestExtract_Modern_RecurringMonthlyCostReconstructedWhenTotalAbsent(t *testing.T) {
+	// TotalCostWithReservedInstances absent → covered = on-demand - net
+	// savings (500 - 180 = 320). amountValuePtr must preserve the nil for
+	// the absent *Amount so the fallback engages.
+	currency := "USD"
+	withModernOnDemandAndSavingsOnly := func(_ *armconsumption.ModernReservationRecommendation, props *armconsumption.ModernReservationRecommendationProperties) {
+		onDemand := 500.0
+		savings := 180.0
+		props.CostWithNoReservedInstances = &armconsumption.Amount{Currency: &currency, Value: &onDemand}
+		props.NetSavings = &armconsumption.Amount{Currency: &currency, Value: &savings}
+		props.TotalCostWithReservedInstances = nil
+	}
+	rec := mocks.BuildModernReservationRecommendation(
+		mocks.WithModernSKUName("Standard_D4s_v5"),
+		withModernOnDemandAndSavingsOnly,
+	)
+	f := Extract(rec)
+	require.NotNil(t, f)
+	require.NotNil(t, f.RecurringMonthlyCost, "covered cost must be reconstructed when total-with-RI is absent")
+	assert.InDelta(t, 320.0, *f.RecurringMonthlyCost, 1e-9)
+}
+
+func TestExtract_Modern_RecurringMonthlyCostNilWhenSourcesAbsent(t *testing.T) {
+	// No cost *Amount fields at all → RecurringMonthlyCost nil, never 0.
+	rec := mocks.BuildModernReservationRecommendation(
+		mocks.WithModernSKUName("Standard_D4s_v5"),
+		// no WithModernCosts → all three *Amount fields nil
 	)
 	f := Extract(rec)
 	require.NotNil(t, f)
-	require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure reservations")
-	assert.InDelta(t, 0.0, *f.RecurringMonthlyCost, 1e-9)
+	assert.Nil(t, f.RecurringMonthlyCost, "covered cost must be nil (absent), not a fabricated 0")
 }
 
 // --- ExpandPaymentVariants --------------------------------------------------

providers/azure/services/compute/client_test.go

@@ -954,10 +954,10 @@ func TestComputeClient_ConvertAzureVMRecommendation_PopulatesAllFields(t *testin
 	assert.Equal(t, "3yr", out.Term)
 	assert.Equal(t, "upfront", out.PaymentOption)
 
-	// Azure reservations are all-upfront: RecurringMonthlyCost must be a
-	// non-nil pointer to 0 so the frontend shows "$0" not "—" (unknown).
-	require.NotNil(t, out.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure compute recs")
-	assert.InDelta(t, 0.0, *out.RecurringMonthlyCost, 1e-9)
+	// RecurringMonthlyCost is the covered/effective cost (paid WITH the
+	// reservation) = TotalCostWithReservedInstances, not 0.
+	require.NotNil(t, out.RecurringMonthlyCost, "RecurringMonthlyCost must be populated for Azure compute recs")
+	assert.InDelta(t, 70.0, *out.RecurringMonthlyCost, 1e-9)
 
 	// Details is populated from the payload's ResourceType (InstanceType
 	// only — Platform/Tenancy/Scope are deferred to batched enrichment).

providers/azure/services/managedredis/client_test.go

@@ -778,8 +778,9 @@ func TestConvertRecommendation_legacy(t *testing.T) {
 	assert.InDelta(t, 1000.0, rec.OnDemandCost, 0.01)
 	assert.InDelta(t, 700.0, rec.CommitmentCost, 0.01)
 	assert.InDelta(t, 300.0, rec.EstimatedSavings, 0.01)
+	// Covered/effective cost (paid WITH the reservation) = CommitmentCost.
 	require.NotNil(t, rec.RecurringMonthlyCost)
-	assert.Equal(t, 0.0, *rec.RecurringMonthlyCost)
+	assert.InDelta(t, 700.0, *rec.RecurringMonthlyCost, 0.01)
 }
 
 // -- RedisPricing struct --

providers/azure/services/synapse/client_test.go

@@ -206,8 +206,9 @@ func TestGetRecommendations_singlePage(t *testing.T) {
 	assert.InDelta(t, 5000.0, r.OnDemandCost, 0.01)
 	assert.InDelta(t, 3500.0, r.CommitmentCost, 0.01)
 	assert.InDelta(t, 1500.0, r.EstimatedSavings, 0.01)
+	// Covered/effective cost (paid WITH the reservation) = CommitmentCost.
 	require.NotNil(t, r.RecurringMonthlyCost)
-	assert.Equal(t, 0.0, *r.RecurringMonthlyCost)
+	assert.InDelta(t, 3500.0, *r.RecurringMonthlyCost, 0.01)
 
 	details, ok := r.Details.(common.DataWarehouseDetails)
 	require.True(t, ok, "Details should be DataWarehouseDetails")