fix(azure): guard EstimatedSavings when OnDemandCost is zero (CR #682)

Three findings from CodeRabbit review 4348367061:

1. `converter.go:287-292` (recurring Major): the OnDemandCost-zero guard
   protected savingsPct (divide-by-zero) but the savings = totalOnDemand -
   totalReservation line ran unconditionally. Result: when OnDemandCost
   was zero and CommitmentCost was non-zero, EstimatedSavings was emitted
   as a negative number (-CommitmentCost), contradicting the function
   contract. Move savings inside the guard so both fields stay 0 when
   on-demand is zero.

2. `converter_test.go`: add
   TestExpandPaymentVariants_ZeroOnDemand_NonZeroCommitment regression
   asserting EstimatedSavings == 0 (not -CommitmentCost) when OnDemand=0,
   Commitment>0. The existing _ZeroOnDemand_NoSavings test only covered
   the 0/0 sub-case and would have missed the regression.

3. `client_test.go`: add cost-field assertions to
   TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields
   so the WithCosts(120, 80, 40) fixture is actually checked through to
   rec.OnDemandCost / rec.CommitmentCost / rec.EstimatedSavings. Locks
   in the converter behaviour this PR depends on.

Author: cristim

providers/azure/internal/recommendations/converter.go

@@ -285,10 +285,11 @@ func ExpandPaymentVariants(base common.Recommendation) []common.Recommendation {
 	totalOnDemand := base.OnDemandCost
 
 	var savingsPct float64
+	var savings float64
 	if totalOnDemand != 0 {
-		savingsPct = (totalOnDemand - totalReservation) / totalOnDemand * 100
+		savings = totalOnDemand - totalReservation
+		savingsPct = savings / totalOnDemand * 100
 	}
-	savings := totalOnDemand - totalReservation
 
 	months := termToMonths(base.Term)
 	recurringMonthly := totalReservation / float64(months)

providers/azure/internal/recommendations/converter_test.go

@@ -318,6 +318,18 @@ func TestExpandPaymentVariants_ZeroOnDemand_NoSavings(t *testing.T) {
 	}
 }
 
+func TestExpandPaymentVariants_ZeroOnDemand_NonZeroCommitment(t *testing.T) {
+	// Regression: when OnDemandCost == 0 but CommitmentCost > 0, the guard
+	// must also force EstimatedSavings to 0. Without it, savings would be
+	// computed as 0 - CommitmentCost and emit negative savings.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 0, 10))
+	require.Len(t, variants, 2)
+	for _, v := range variants {
+		assert.InDelta(t, 0.0, v.EstimatedSavings, 1e-9)
+		assert.InDelta(t, 0.0, v.SavingsPercentage, 1e-9)
+	}
+}
+
 func TestExpandPaymentVariants_ZeroCommitmentCost(t *testing.T) {
 	// Zero reservation total: both variants still emitted; no-upfront monthly = 0.
 	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 50, 0))

providers/azure/services/search/client_test.go

@@ -616,6 +616,9 @@ func TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields(t *tes
 	assert.Equal(t, "eastus", rec.Region)
 	assert.Equal(t, "standard2", rec.ResourceType)
 	assert.Equal(t, 2, rec.Count)
+	assert.InDelta(t, 120.0, rec.OnDemandCost, 1e-9)
+	assert.InDelta(t, 80.0, rec.CommitmentCost, 1e-9)
+	assert.InDelta(t, 40.0, rec.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType)
 	assert.Equal(t, "1yr", rec.Term)
 	assert.Equal(t, "upfront", rec.PaymentOption)