From 550e98459ee5f169194d2d2bfcf6d102421ec25a Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 22 May 2026 17:20:42 +0200
Subject: [PATCH 1/6] feat(providers/azure): expand reservation recs into
 all-upfront + no-upfront variants

Azure reservation recommendations previously emitted a single entry with
PaymentOption="upfront" (bare, non-canonical). Users could not see the
no-upfront (monthly billing) alternative even though Azure supports both
billing plans at the same total price.

Changes:
- Add ExpandPaymentVariants helper in providers/azure/internal/recommendations/
  (alongside the existing Extract helper). Given a base recommendation it returns
  two copies: one with PaymentOption="all-upfront" (RecurringMonthlyCost=0) and
  one with PaymentOption="no-upfront" (RecurringMonthlyCost=CommitmentCost/termMonths).
  EstimatedSavings and SavingsPercentage are identical across both variants since
  Azure charges the same total price for both billing plans.
- Update all five service converters (compute, cache, database, cosmosdb, search)
  and the Azure Advisor path to call ExpandPaymentVariants, producing two recs per
  API recommendation instead of one.
- Rename the import alias to azrecs in each service client to resolve the local-
  variable shadowing that the recommendations package name previously caused.
- Replace bare "upfront" with canonical "all-upfront" in all Azure code paths.
- Add focused unit tests for ExpandPaymentVariants covering: zero on-demand guard,
  zero commitment cost, 1yr vs 3yr term-month math, pointer independence between
  variants, savings equality across variants, and shared-field pass-through.
- Add TestComputeClient_GetRecommendations_EmitsBothPaymentVariants to exercise
  the end-to-end fan-out path via a mock pager.
- Update existing converter tests to expect "all-upfront" instead of "upfront".

Closes #679
---
 .../internal/recommendations/converter.go     |  61 ++++++++++
 .../recommendations/converter_test.go         | 112 ++++++++++++++++++
 providers/azure/recommendations.go            |   5 +-
 providers/azure/recommendations_test.go       |   2 +-
 providers/azure/services/cache/client.go      |   8 +-
 providers/azure/services/cache/client_test.go |   2 +-
 providers/azure/services/compute/client.go    |   8 +-
 .../azure/services/compute/client_test.go     |  56 ++++++++-
 providers/azure/services/cosmosdb/client.go   |   8 +-
 .../azure/services/cosmosdb/client_test.go    |   2 +-
 providers/azure/services/database/client.go   |   8 +-
 .../azure/services/database/client_test.go    |   2 +-
 providers/azure/services/search/client.go     |   5 +-
 .../azure/services/search/client_test.go      |   2 +-
 14 files changed, 255 insertions(+), 26 deletions(-)

diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go
index 565fcd812..fb69ce20e 100644
--- a/providers/azure/internal/recommendations/converter.go
+++ b/providers/azure/internal/recommendations/converter.go
@@ -25,6 +25,7 @@ package recommendations
 import (
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
 
+	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 )
 
@@ -246,3 +247,63 @@ func strDeref(s *string) string {
 	}
 	return *s
 }
+
+// termToMonths maps a normalised term string ("1yr", "3yr") to the number of
+// months it spans. Unknown terms default to 12 so the monthly-cost arithmetic
+// stays valid rather than dividing by zero.
+func termToMonths(term string) int {
+	switch term {
+	case "3yr":
+		return 36
+	default:
+		return 12
+	}
+}
+
+// ExpandPaymentVariants fans out a single Azure reservation recommendation
+// into two variants that differ only in payment schedule:
+//
+//   - "all-upfront"  — the full reservation cost is paid today; no monthly
+//     recurring charge (RecurringMonthlyCost = pointer to 0).
+//   - "no-upfront"   — nothing is paid today; the same total reservation cost
+//     is spread evenly across the term months (RecurringMonthlyCost =
+//     CommitmentCost / termMonths).
+//
+// Azure charges the same total reservation price for both billing plans
+// (unlike AWS, which prices partial-upfront separately), so EstimatedSavings
+// and SavingsPercentage vs on-demand are identical between the two variants;
+// only the cashflow split changes.
+//
+// The base recommendation must already have PaymentOption set to "all-upfront"
+// and a valid CommitmentCost (total reservation price) and OnDemandCost (total
+// on-demand cost over the same period). If OnDemandCost is zero the savings
+// fields are forced to zero to avoid a divide-by-zero; if CommitmentCost is
+// zero both variants are still emitted with zero costs (caller's responsibility
+// to validate upstream).
+func ExpandPaymentVariants(base common.Recommendation) []common.Recommendation {
+	totalReservation := base.CommitmentCost
+	totalOnDemand := base.OnDemandCost
+
+	var savingsPct float64
+	if totalOnDemand != 0 {
+		savingsPct = (totalOnDemand - totalReservation) / totalOnDemand * 100
+	}
+	savings := totalOnDemand - totalReservation
+
+	months := termToMonths(base.Term)
+	recurringMonthly := totalReservation / float64(months)
+
+	allUpfront := base
+	allUpfront.PaymentOption = "all-upfront"
+	allUpfront.EstimatedSavings = savings
+	allUpfront.SavingsPercentage = savingsPct
+	allUpfront.RecurringMonthlyCost = float64Ptr(0)
+
+	noUpfront := base
+	noUpfront.PaymentOption = "no-upfront"
+	noUpfront.EstimatedSavings = savings
+	noUpfront.SavingsPercentage = savingsPct
+	noUpfront.RecurringMonthlyCost = float64Ptr(recurringMonthly)
+
+	return []common.Recommendation{allUpfront, noUpfront}
+}
diff --git a/providers/azure/internal/recommendations/converter_test.go b/providers/azure/internal/recommendations/converter_test.go
index 20fdc0c03..04fa41784 100644
--- a/providers/azure/internal/recommendations/converter_test.go
+++ b/providers/azure/internal/recommendations/converter_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/providers/azure/mocks"
 )
 
@@ -239,3 +240,114 @@ func TestExtract_Modern_RecurringMonthlyCostIsZeroPointer(t *testing.T) {
 	require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure reservations")
 	assert.InDelta(t, 0.0, *f.RecurringMonthlyCost, 1e-9)
 }
+
+// --- ExpandPaymentVariants --------------------------------------------------
+
+func baseRec(service common.ServiceType, term string, onDemand, commitment float64) common.Recommendation {
+	return common.Recommendation{
+		Provider:       common.ProviderAzure,
+		Service:        service,
+		Region:         "eastus",
+		ResourceType:   "Standard_D2s_v3",
+		CommitmentType: common.CommitmentReservedInstance,
+		Term:           term,
+		PaymentOption:  "all-upfront",
+		OnDemandCost:   onDemand,
+		CommitmentCost: commitment,
+	}
+}
+
+func TestExpandPaymentVariants_ReturnsTwoVariants(t *testing.T) {
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70))
+	require.Len(t, variants, 2, "must return exactly two variants")
+}
+
+func TestExpandPaymentVariants_PaymentOptionValues(t *testing.T) {
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70))
+	assert.Equal(t, "all-upfront", variants[0].PaymentOption)
+	assert.Equal(t, "no-upfront", variants[1].PaymentOption)
+}
+
+func TestExpandPaymentVariants_AllUpfrontCashflow(t *testing.T) {
+	// all-upfront: RecurringMonthlyCost must be a non-nil pointer to 0.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70))
+	allUpfront := variants[0]
+	require.NotNil(t, allUpfront.RecurringMonthlyCost)
+	assert.InDelta(t, 0.0, *allUpfront.RecurringMonthlyCost, 1e-9)
+}
+
+func TestExpandPaymentVariants_NoUpfront1yrCashflow(t *testing.T) {
+	// no-upfront 1yr: RecurringMonthlyCost = CommitmentCost / 12.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 72))
+	noUpfront := variants[1]
+	require.NotNil(t, noUpfront.RecurringMonthlyCost)
+	assert.InDelta(t, 72.0/12.0, *noUpfront.RecurringMonthlyCost, 1e-9)
+}
+
+func TestExpandPaymentVariants_NoUpfront3yrCashflow(t *testing.T) {
+	// no-upfront 3yr: RecurringMonthlyCost = CommitmentCost / 36.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "3yr", 200, 120))
+	noUpfront := variants[1]
+	require.NotNil(t, noUpfront.RecurringMonthlyCost)
+	assert.InDelta(t, 120.0/36.0, *noUpfront.RecurringMonthlyCost, 1e-9)
+}
+
+func TestExpandPaymentVariants_SavingsIdenticalAcrossVariants(t *testing.T) {
+	// EstimatedSavings and SavingsPercentage must be the same for both
+	// variants — Azure's total reservation price is unchanged between billing
+	// plans; only cashflow splits.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70))
+	assert.InDelta(t, variants[0].EstimatedSavings, variants[1].EstimatedSavings, 1e-9)
+	assert.InDelta(t, variants[0].SavingsPercentage, variants[1].SavingsPercentage, 1e-9)
+}
+
+func TestExpandPaymentVariants_SavingsValues(t *testing.T) {
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70))
+	assert.InDelta(t, 30.0, variants[0].EstimatedSavings, 1e-9)
+	assert.InDelta(t, 30.0, variants[1].EstimatedSavings, 1e-9)
+	assert.InDelta(t, 30.0, variants[0].SavingsPercentage, 1e-9)
+	assert.InDelta(t, 30.0, variants[1].SavingsPercentage, 1e-9)
+}
+
+func TestExpandPaymentVariants_ZeroOnDemand_NoSavings(t *testing.T) {
+	// Guard: avoid divide-by-zero when OnDemandCost is 0.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 0, 0))
+	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))
+	require.Len(t, variants, 2)
+	require.NotNil(t, variants[1].RecurringMonthlyCost)
+	assert.InDelta(t, 0.0, *variants[1].RecurringMonthlyCost, 1e-9)
+}
+
+func TestExpandPaymentVariants_SharedFieldsCarriedThrough(t *testing.T) {
+	// Service, Region, ResourceType, CommitmentType, Term, Account must be
+	// identical between the two returned variants (only payment schedule changes).
+	base := baseRec(common.ServiceRelationalDB, "3yr", 200, 140)
+	base.Account = "sub-123"
+	variants := ExpandPaymentVariants(base)
+	for _, v := range variants {
+		assert.Equal(t, common.ServiceRelationalDB, v.Service)
+		assert.Equal(t, "eastus", v.Region)
+		assert.Equal(t, "Standard_D2s_v3", v.ResourceType)
+		assert.Equal(t, common.CommitmentReservedInstance, v.CommitmentType)
+		assert.Equal(t, "3yr", v.Term)
+		assert.Equal(t, "sub-123", v.Account)
+	}
+}
+
+func TestExpandPaymentVariants_RecurringMonthlyCostPointersAreIndependent(t *testing.T) {
+	// The two variants must hold independent pointer values — mutating one
+	// must not affect the other.
+	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 60))
+	require.NotNil(t, variants[0].RecurringMonthlyCost)
+	require.NotNil(t, variants[1].RecurringMonthlyCost)
+	assert.True(t, variants[0].RecurringMonthlyCost != variants[1].RecurringMonthlyCost,
+		"each variant must own an independent pointer to its RecurringMonthlyCost")
+}
diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go
index ea0c007a5..b34722185 100644
--- a/providers/azure/recommendations.go
+++ b/providers/azure/recommendations.go
@@ -14,6 +14,7 @@ import (
 	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/pkg/concurrency"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
+	azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/cache"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/compute"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/cosmosdb"
@@ -233,7 +234,7 @@ func (r *RecommendationsClientAdapter) getAdvisorRecommendations(ctx context.Con
 			// Convert Azure Advisor recommendation to our common format
 			rec := r.convertAdvisorRecommendation(advisorRec)
 			if rec != nil && shouldIncludeService(params, rec.Service) {
-				recommendations = append(recommendations, *rec)
+				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*rec)...)
 			}
 		}
 	}
@@ -277,7 +278,7 @@ func (r *RecommendationsClientAdapter) convertAdvisorRecommendation(advisorRec *
 		Account:        r.subscriptionID,
 		CommitmentType: common.CommitmentReservedInstance,
 		Term:           "1yr",
-		PaymentOption:  "upfront",
+		PaymentOption:  "all-upfront",
 	}
 
 	rec.Region = resolveAdvisorRegion(advisorRec)
diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go
index b2eb251db..1b79d5524 100644
--- a/providers/azure/recommendations_test.go
+++ b/providers/azure/recommendations_test.go
@@ -344,7 +344,7 @@ func TestConvertAdvisorRecommendation(t *testing.T) {
 	assert.Equal(t, "test-subscription", result.Account)
 	assert.Equal(t, common.CommitmentReservedInstance, result.CommitmentType)
 	assert.Equal(t, "1yr", result.Term)
-	assert.Equal(t, "upfront", result.PaymentOption)
+	assert.Equal(t, "all-upfront", result.PaymentOption)
 }
 
 func TestConvertAdvisorRecommendation_NilProperties(t *testing.T) {
diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go
index e7fd1921f..a0ff240fd 100644
--- a/providers/azure/services/cache/client.go
+++ b/providers/azure/services/cache/client.go
@@ -21,7 +21,7 @@ import (
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/pricing"
-	"github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
+	azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations"
 )
 
@@ -175,7 +175,7 @@ func (c *CacheClient) GetRecommendations(ctx context.Context, params common.Reco
 		for _, rec := range page.Value {
 			converted := c.convertAzureRedisRecommendation(ctx, rec)
 			if converted != nil {
-				recommendations = append(recommendations, *converted)
+				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...)
 			}
 		}
 	}
@@ -574,7 +574,7 @@ func extractRedisPricing(items []CacheRetailPriceItem, termYears int) (onDemand,
 // Premium-tier SKU; otherwise stays 0 (zero means "unknown", not
 // "definitely zero shards" — see the redisSKUEntry godoc).
 func (c *CacheClient) convertAzureRedisRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation {
-	f := recommendations.Extract(azureRec)
+	f := azrecs.Extract(azureRec)
 	if f == nil {
 		return nil
 	}
@@ -598,7 +598,7 @@ func (c *CacheClient) convertAzureRedisRecommendation(ctx context.Context, azure
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "upfront",
+		PaymentOption:        "all-upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go
index 930f39885..e89a54d27 100644
--- a/providers/azure/services/cache/client_test.go
+++ b/providers/azure/services/cache/client_test.go
@@ -787,7 +787,7 @@ func TestCacheClient_ConvertAzureRedisRecommendation_PopulatesAllFields(t *testi
 	assert.InDelta(t, 15.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "3yr", out.Term)
-	assert.Equal(t, "upfront", out.PaymentOption)
+	assert.Equal(t, "all-upfront", out.PaymentOption)
 
 	// Details carries Engine=redis + NodeType from the SKU string. Shards
 	// stays 0 when no cache instance matches in the subscription (the
diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go
index e016e963f..2181c0db0 100644
--- a/providers/azure/services/compute/client.go
+++ b/providers/azure/services/compute/client.go
@@ -22,7 +22,7 @@ import (
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/pricing"
-	"github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
+	azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations"
 )
 
@@ -194,7 +194,7 @@ func (c *ComputeClient) GetRecommendations(ctx context.Context, params common.Re
 		for _, rec := range page.Value {
 			converted := c.convertAzureVMRecommendation(ctx, rec)
 			if converted != nil {
-				recommendations = append(recommendations, *converted)
+				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...)
 			}
 		}
 	}
@@ -698,7 +698,7 @@ func extractVMPricing(items []AzureRetailPriceItem, termYears int) (onDemand, re
 // sources (consumption usage records, dedicated-host inventory) and
 // remain unpopulated — out of scope for this issue.
 func (c *ComputeClient) convertAzureVMRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation {
-	f := recommendations.Extract(azureRec)
+	f := azrecs.Extract(azureRec)
 	if f == nil {
 		return nil
 	}
@@ -726,7 +726,7 @@ func (c *ComputeClient) convertAzureVMRecommendation(ctx context.Context, azureR
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "upfront",
+		PaymentOption:        "all-upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/compute/client_test.go b/providers/azure/services/compute/client_test.go
index fb9b2076b..f8a4f74c9 100644
--- a/providers/azure/services/compute/client_test.go
+++ b/providers/azure/services/compute/client_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
@@ -218,6 +219,59 @@ func TestComputeClient_GetRecommendations_WithMock(t *testing.T) {
 	assert.Empty(t, recommendations)
 }
 
+// TestComputeClient_GetRecommendations_EmitsBothPaymentVariants asserts that
+// a single Azure reservation recommendation from the API is expanded into two
+// entries — "all-upfront" and "no-upfront" — with correct cashflow split and
+// identical savings figures.
+func TestComputeClient_GetRecommendations_EmitsBothPaymentVariants(t *testing.T) {
+	ctx := context.Background()
+	client := NewClient(nil, "test-subscription", "eastus")
+
+	// Inject a single recommendation via the mock pager.
+	apiRec := mocks.BuildLegacyReservationRecommendation(
+		mocks.WithRegion("eastus"),
+		mocks.WithScope("Shared"),
+		mocks.WithTerm("P1Y"),
+		mocks.WithQuantity(1),
+		mocks.WithNormalizedSize("Standard_D2s_v3"),
+		mocks.WithCosts(120, 84, 36), // onDemand=120, commitment=84, savings=36
+	)
+	mockPager := &mocks.MockRecommendationsPager{
+		Results: []armconsumption.ReservationRecommendationClassification{apiRec},
+		HasMore: true,
+	}
+	client.SetRecommendationsPager(mockPager)
+
+	recs, err := client.GetRecommendations(ctx, common.RecommendationParams{})
+	require.NoError(t, err)
+	require.Len(t, recs, 2, "one API rec must expand to two payment-variant entries")
+
+	payments := make(map[string]common.Recommendation)
+	for _, r := range recs {
+		payments[r.PaymentOption] = r
+	}
+	require.Contains(t, payments, "all-upfront", "all-upfront variant must be present")
+	require.Contains(t, payments, "no-upfront", "no-upfront variant must be present")
+
+	allUp := payments["all-upfront"]
+	noUp := payments["no-upfront"]
+
+	// Cashflow: all-upfront has zero recurring; no-upfront spreads over 12 months.
+	require.NotNil(t, allUp.RecurringMonthlyCost)
+	assert.InDelta(t, 0.0, *allUp.RecurringMonthlyCost, 1e-9)
+	require.NotNil(t, noUp.RecurringMonthlyCost)
+	assert.InDelta(t, 84.0/12.0, *noUp.RecurringMonthlyCost, 1e-9)
+
+	// Savings must be identical across variants.
+	assert.InDelta(t, allUp.EstimatedSavings, noUp.EstimatedSavings, 1e-9)
+	assert.InDelta(t, allUp.SavingsPercentage, noUp.SavingsPercentage, 1e-9)
+
+	// Shared fields.
+	assert.Equal(t, "test-subscription", allUp.Account)
+	assert.Equal(t, common.ServiceCompute, allUp.Service)
+	assert.Equal(t, "1yr", allUp.Term)
+}
+
 func TestComputeClient_GetExistingCommitments_WithMock(t *testing.T) {
 	ctx := context.Background()
 	client := NewClient(nil, "test-subscription", "eastus")
@@ -829,7 +883,7 @@ func TestComputeClient_ConvertAzureVMRecommendation_PopulatesAllFields(t *testin
 	assert.InDelta(t, 30.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "3yr", out.Term)
-	assert.Equal(t, "upfront", out.PaymentOption)
+	assert.Equal(t, "all-upfront", out.PaymentOption)
 
 	// Azure reservations are all-upfront: RecurringMonthlyCost must be a
 	// non-nil pointer to 0 so the frontend shows "$0" not "—" (unknown).
diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go
index b77ad88f7..28f623dca 100644
--- a/providers/azure/services/cosmosdb/client.go
+++ b/providers/azure/services/cosmosdb/client.go
@@ -22,7 +22,7 @@ import (
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/pricing"
-	"github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
+	azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations"
 )
 
@@ -169,7 +169,7 @@ func (c *CosmosDBClient) GetRecommendations(ctx context.Context, params common.R
 		for _, rec := range page.Value {
 			converted := c.convertAzureCosmosRecommendation(ctx, rec)
 			if converted != nil {
-				recommendations = append(recommendations, *converted)
+				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...)
 			}
 		}
 	}
@@ -577,7 +577,7 @@ func calculateCosmosSavingsPercentage(onDemandPrice, hoursInTerm, reservationPri
 // subscription has zero Cosmos accounts, multiple Cosmos accounts with
 // different API types (ambiguous), or the listing fails.
 func (c *CosmosDBClient) convertAzureCosmosRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation {
-	f := recommendations.Extract(azureRec)
+	f := azrecs.Extract(azureRec)
 	if f == nil {
 		return nil
 	}
@@ -598,7 +598,7 @@ func (c *CosmosDBClient) convertAzureCosmosRecommendation(ctx context.Context, a
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "upfront",
+		PaymentOption:        "all-upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/cosmosdb/client_test.go b/providers/azure/services/cosmosdb/client_test.go
index 7dd73df61..97bf385a2 100644
--- a/providers/azure/services/cosmosdb/client_test.go
+++ b/providers/azure/services/cosmosdb/client_test.go
@@ -1003,7 +1003,7 @@ func TestCosmosDBClient_ConvertAzureCosmosRecommendation_PopulatesAllFields(t *t
 	assert.InDelta(t, 90.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "1yr", out.Term)
-	assert.Equal(t, "upfront", out.PaymentOption)
+	assert.Equal(t, "all-upfront", out.PaymentOption)
 
 	// Details carries Engine="cosmos". For this fixture the SKU doesn't
 	// start with digits, so ThroughputUnits is 0 — which correctly
diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go
index f0238c00c..b41d37146 100644
--- a/providers/azure/services/database/client.go
+++ b/providers/azure/services/database/client.go
@@ -21,7 +21,7 @@ import (
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/pricing"
-	"github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
+	azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations"
 )
 
@@ -177,7 +177,7 @@ func (c *DatabaseClient) GetRecommendations(ctx context.Context, params common.R
 		for _, rec := range page.Value {
 			converted := c.convertAzureSQLRecommendation(ctx, rec)
 			if converted != nil {
-				recommendations = append(recommendations, *converted)
+				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...)
 			}
 		}
 	}
@@ -575,7 +575,7 @@ func extractSQLPricing(items []DatabaseRetailPriceItem, termYears int) (onDemand
 // otherwise stays empty. AZConfig/Deployment still need additional
 // signals (per-server config) and remain deferred.
 func (c *DatabaseClient) convertAzureSQLRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation {
-	f := recommendations.Extract(azureRec)
+	f := azrecs.Extract(azureRec)
 	if f == nil {
 		return nil
 	}
@@ -596,7 +596,7 @@ func (c *DatabaseClient) convertAzureSQLRecommendation(ctx context.Context, azur
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "upfront",
+		PaymentOption:        "all-upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/database/client_test.go b/providers/azure/services/database/client_test.go
index c26628466..bd3efcc6c 100644
--- a/providers/azure/services/database/client_test.go
+++ b/providers/azure/services/database/client_test.go
@@ -694,7 +694,7 @@ func TestDatabaseClient_ConvertAzureSQLRecommendation_PopulatesAllFields(t *test
 	assert.InDelta(t, 60.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "1yr", out.Term)
-	assert.Equal(t, "upfront", out.PaymentOption)
+	assert.Equal(t, "all-upfront", out.PaymentOption)
 
 	// Details carries Engine=sqlserver + InstanceClass from the SKU
 	// string. EngineVersion stays empty when the catalogue has no
diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go
index 7573577be..b1f1e453c 100644
--- a/providers/azure/services/search/client.go
+++ b/providers/azure/services/search/client.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
+	azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations"
 	"github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations"
 )
 
@@ -149,7 +150,7 @@ func (c *SearchClient) GetRecommendations(ctx context.Context, params common.Rec
 		for _, rec := range page.Value {
 			converted := c.convertAzureSearchRecommendation(ctx, rec)
 			if converted != nil {
-				recommendations = append(recommendations, *converted)
+				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...)
 			}
 		}
 	}
@@ -547,7 +548,7 @@ func (c *SearchClient) convertAzureSearchRecommendation(ctx context.Context, azu
 		CommitmentType: common.CommitmentReservedInstance,
 		Timestamp:      time.Now(),
 		Term:           "1yr",
-		PaymentOption:  "upfront",
+		PaymentOption:  "all-upfront",
 	}
 
 	return rec
diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go
index bfee43ce7..dc8d005ed 100644
--- a/providers/azure/services/search/client_test.go
+++ b/providers/azure/services/search/client_test.go
@@ -599,7 +599,7 @@ func TestSearchClient_ConvertAzureSearchRecommendation(t *testing.T) {
 	assert.Equal(t, "eastus", rec.Region)
 	assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType)
 	assert.Equal(t, "1yr", rec.Term)
-	assert.Equal(t, "upfront", rec.PaymentOption)
+	assert.Equal(t, "all-upfront", rec.PaymentOption)
 }
 
 // MockTokenCredential for testing PurchaseCommitment

From f99927434dc0c25562e6617878a3fc4feae29a80 Mon Sep 17 00:00:00 2001
From: "coderabbitai[bot]"
 <136622811+coderabbitai[bot]@users.noreply.github.com>
Date: Fri, 22 May 2026 16:09:07 +0000
Subject: [PATCH 2/6] fix: apply CodeRabbit auto-fixes

Fixed 2 file(s) based on 2 unresolved review comments.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
---
 providers/azure/recommendations.go        | 12 +++++++++++-
 providers/azure/services/search/client.go | 24 ++++++++++++++++++++---
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go
index b34722185..e83ab3544 100644
--- a/providers/azure/recommendations.go
+++ b/providers/azure/recommendations.go
@@ -234,7 +234,17 @@ func (r *RecommendationsClientAdapter) getAdvisorRecommendations(ctx context.Con
 			// Convert Azure Advisor recommendation to our common format
 			rec := r.convertAdvisorRecommendation(advisorRec)
 			if rec != nil && shouldIncludeService(params, rec.Service) {
-				recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*rec)...)
+				// Preserve Advisor-provided EstimatedSavings when both OnDemandCost
+				// and CommitmentCost are unset (zero), since ExpandPaymentVariants
+				// would otherwise overwrite it with zero (OnDemandCost - CommitmentCost).
+				advisorSavings := rec.EstimatedSavings
+				variants := azrecs.ExpandPaymentVariants(*rec)
+				if rec.OnDemandCost == 0 && rec.CommitmentCost == 0 && advisorSavings != 0 {
+					for i := range variants {
+						variants[i].EstimatedSavings = advisorSavings
+					}
+				}
+				recommendations = append(recommendations, variants...)
 			}
 		}
 	}
diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go
index b1f1e453c..c8311bf9e 100644
--- a/providers/azure/services/search/client.go
+++ b/providers/azure/services/search/client.go
@@ -540,15 +540,33 @@ func calculateSearchSavingsPercentage(onDemandPrice, hoursInTerm, reservationPri
 
 // convertAzureSearchRecommendation converts Azure Search reservation recommendation to common format
 func (c *SearchClient) convertAzureSearchRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation {
+	// Extract fields from Azure recommendation using the shared converter
+	extracted := azrecs.Extract(azureRec)
+	if extracted == nil {
+		return nil
+	}
+
 	rec := &common.Recommendation{
 		Provider:       common.ProviderAzure,
 		Service:        common.ServiceOther,
 		Account:        c.subscriptionID,
-		Region:         c.region,
 		CommitmentType: common.CommitmentReservedInstance,
 		Timestamp:      time.Now(),
-		Term:           "1yr",
-		PaymentOption:  "all-upfront",
+		// Populate fields from Azure API response
+		Region:               extracted.Region,
+		ResourceType:         extracted.ResourceType,
+		Count:                extracted.Count,
+		OnDemandCost:         extracted.OnDemandCost,
+		CommitmentCost:       extracted.CommitmentCost,
+		EstimatedSavings:     extracted.EstimatedSavings,
+		Term:                 extracted.Term,
+		RecurringMonthlyCost: extracted.RecurringMonthlyCost,
+		PaymentOption:        "all-upfront", // Default, will be expanded by ExpandPaymentVariants
+	}
+
+	// Override region with client region if extraction didn't find one
+	if rec.Region == "" {
+		rec.Region = c.region
 	}
 
 	return rec

From 3f34a9f9cc31d589c3e2b2bef8ff84f58366abea Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 22 May 2026 18:24:05 +0200
Subject: [PATCH 3/6] refactor(azure/recommendations): extract page-processing
 helper to fit gocyclo budget

Pull the per-page advisor-record loop body out of getAdvisorRecommendations
into appendAdvisorPageRecs, following the same pattern used for
resolveAdvisorRegion. getAdvisorRecommendations drops from cyclomatic 12
to 4; appendAdvisorPageRecs lands at 5. No behaviour change.
---
 providers/azure/recommendations.go | 48 ++++++++++++++++++------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go
index e83ab3544..c981ea2bc 100644
--- a/providers/azure/recommendations.go
+++ b/providers/azure/recommendations.go
@@ -225,31 +225,41 @@ func (r *RecommendationsClientAdapter) getAdvisorRecommendations(ctx context.Con
 			logging.Warnf("Azure Advisor pagination error (partial results may be returned): %v", err)
 			break
 		}
+		recommendations = r.appendAdvisorPageRecs(params, page, recommendations)
+	}
 
-		for _, advisorRec := range page.Value {
-			if advisorRec.Properties == nil {
-				continue
-			}
+	return recommendations, nil
+}
 
-			// Convert Azure Advisor recommendation to our common format
-			rec := r.convertAdvisorRecommendation(advisorRec)
-			if rec != nil && shouldIncludeService(params, rec.Service) {
-				// Preserve Advisor-provided EstimatedSavings when both OnDemandCost
-				// and CommitmentCost are unset (zero), since ExpandPaymentVariants
-				// would otherwise overwrite it with zero (OnDemandCost - CommitmentCost).
-				advisorSavings := rec.EstimatedSavings
-				variants := azrecs.ExpandPaymentVariants(*rec)
-				if rec.OnDemandCost == 0 && rec.CommitmentCost == 0 && advisorSavings != 0 {
-					for i := range variants {
-						variants[i].EstimatedSavings = advisorSavings
-					}
+// appendAdvisorPageRecs converts one Advisor pager page into common recommendations
+// and appends them to the provided slice. Pulled out of getAdvisorRecommendations
+// to keep that function under the cyclomatic limit.
+func (r *RecommendationsClientAdapter) appendAdvisorPageRecs(
+	params common.RecommendationParams,
+	page armadvisor.RecommendationsClientListResponse,
+	recommendations []common.Recommendation,
+) []common.Recommendation {
+	for _, advisorRec := range page.Value {
+		if advisorRec.Properties == nil {
+			continue
+		}
+		// Convert Azure Advisor recommendation to our common format
+		rec := r.convertAdvisorRecommendation(advisorRec)
+		if rec != nil && shouldIncludeService(params, rec.Service) {
+			// Preserve Advisor-provided EstimatedSavings when both OnDemandCost
+			// and CommitmentCost are unset (zero), since ExpandPaymentVariants
+			// would otherwise overwrite it with zero (OnDemandCost - CommitmentCost).
+			advisorSavings := rec.EstimatedSavings
+			variants := azrecs.ExpandPaymentVariants(*rec)
+			if rec.OnDemandCost == 0 && rec.CommitmentCost == 0 && advisorSavings != 0 {
+				for i := range variants {
+					variants[i].EstimatedSavings = advisorSavings
 				}
-				recommendations = append(recommendations, variants...)
 			}
+			recommendations = append(recommendations, variants...)
 		}
 	}
-
-	return recommendations, nil
+	return recommendations
 }
 
 // resolveAdvisorRegion picks the region for an Advisor recommendation.

From 3f590048191ee1441d71a470d38db57f5853cdfa Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 22 May 2026 18:40:48 +0200
Subject: [PATCH 4/6] test(azure/search): update converter test for nil-guard
 contract

After the azrecs.Extract refactor, convertAzureSearchRecommendation
returns nil on unusable SDK payloads (matching the contract shared
by cache, compute, cosmosdb, and database converters). Split the
single test into a nil-guard test and a field-population test that
uses BuildLegacyReservationRecommendation, consistent with the
other service converter test patterns.
---
 .../azure/services/search/client_test.go      | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go
index dc8d005ed..1638f966c 100644
--- a/providers/azure/services/search/client_test.go
+++ b/providers/azure/services/search/client_test.go
@@ -19,6 +19,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/LeanerCloud/CUDly/pkg/common"
+	"github.com/LeanerCloud/CUDly/providers/azure/mocks"
 )
 
 // MockRecommendationsPager mocks the RecommendationsPager interface
@@ -587,16 +588,34 @@ func TestSearchClient_GetCommonSKUs(t *testing.T) {
 	assert.Contains(t, skus, "storage_optimized_l2")
 }
 
-func TestSearchClient_ConvertAzureSearchRecommendation(t *testing.T) {
-	ctx := context.Background()
+// TestSearchClient_ConvertAzureSearchRecommendation_NilGuards pins the contract:
+// unusable SDK payloads (nil or nil Properties) produce a nil *Recommendation.
+func TestSearchClient_ConvertAzureSearchRecommendation_NilGuards(t *testing.T) {
 	client := NewClient(nil, "test-subscription", "eastus")
+	assert.Nil(t, client.convertAzureSearchRecommendation(context.Background(), nil))
+}
 
-	rec := client.convertAzureSearchRecommendation(ctx, nil)
+// TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields asserts
+// the converter forwards every helper-extracted field plus the Search-service
+// constants (Provider, Service, CommitmentType, PaymentOption).
+func TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields(t *testing.T) {
+	client := NewClient(nil, "test-subscription", "eastus")
+
+	azRec := mocks.BuildLegacyReservationRecommendation(
+		mocks.WithRegion("eastus"),
+		mocks.WithTerm("P1Y"),
+		mocks.WithQuantity(2),
+		mocks.WithNormalizedSize("standard2"),
+		mocks.WithCosts(120, 80, 40),
+	)
+	rec := client.convertAzureSearchRecommendation(context.Background(), azRec)
 	require.NotNil(t, rec)
 	assert.Equal(t, common.ProviderAzure, rec.Provider)
 	assert.Equal(t, common.ServiceOther, rec.Service)
 	assert.Equal(t, "test-subscription", rec.Account)
 	assert.Equal(t, "eastus", rec.Region)
+	assert.Equal(t, "standard2", rec.ResourceType)
+	assert.Equal(t, 2, rec.Count)
 	assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType)
 	assert.Equal(t, "1yr", rec.Term)
 	assert.Equal(t, "all-upfront", rec.PaymentOption)

From c6c2df9097cbcc99f3b54ba3b184d3a3c8866a6d Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 22 May 2026 21:19:58 +0200
Subject: [PATCH 5/6] refactor(azure): use Azure-canonical upfront/monthly
 payment terms in variant expansion

ExpandPaymentVariants was emitting "all-upfront"/"no-upfront" (AWS terms)
instead of the Azure-canonical "upfront"/"monthly". This made the emitted
values inconsistent with every other Azure code path and with the
AZURE_PAYMENTS constant in commitmentOptions.ts which defines "upfront" and
"monthly" as the canonical values.

Rename throughout:
- "all-upfront" -> "upfront" in ExpandPaymentVariants, all five service
  converter functions (compute/cache/database/cosmosdb/search), and the
  convertAdvisorRecommendation base rec.
- "no-upfront" -> "monthly" in ExpandPaymentVariants.
- Update all test assertions and fixtures to match.
- Update the ExpandPaymentVariants docstring precondition and value list.

The purchase-path switch cases in all Azure reservation clients already
accept both the canonical value and the former alias
(e.g. case "all-upfront", "upfront": and case "monthly", "no-upfront":),
so renaming the emitted values does not break the purchase path.

Add "upfront" -> "Upfront" to PAYMENT_DISPLAY_LABELS in recommendations.ts
so the Payment column in the recommendations table renders a human-readable
label for Azure "upfront" recs rather than falling back to the raw string.
"monthly" was already covered.
---
 frontend/src/recommendations.ts                    |  1 +
 .../azure/internal/recommendations/converter.go    | 10 +++++-----
 .../internal/recommendations/converter_test.go     |  6 +++---
 providers/azure/recommendations.go                 |  2 +-
 providers/azure/recommendations_test.go            |  2 +-
 providers/azure/services/cache/client.go           |  2 +-
 providers/azure/services/cache/client_test.go      |  2 +-
 providers/azure/services/compute/client.go         |  2 +-
 providers/azure/services/compute/client_test.go    | 14 +++++++-------
 providers/azure/services/cosmosdb/client.go        |  2 +-
 providers/azure/services/cosmosdb/client_test.go   |  2 +-
 providers/azure/services/database/client.go        |  2 +-
 providers/azure/services/database/client_test.go   |  2 +-
 providers/azure/services/search/client.go          |  2 +-
 providers/azure/services/search/client_test.go     |  2 +-
 15 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts
index b26750c9f..1ab3c5ae2 100644
--- a/frontend/src/recommendations.ts
+++ b/frontend/src/recommendations.ts
@@ -2425,6 +2425,7 @@ const PAYMENT_DISPLAY_LABELS: Record<string, string> = {
   'all-upfront':     'All Upfront',
   'partial-upfront': 'Partial Upfront',
   'no-upfront':      'No Upfront',
+  'upfront':         'Upfront',
   'monthly':         'Monthly',
 };
 
diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go
index fb69ce20e..7bea467ec 100644
--- a/providers/azure/internal/recommendations/converter.go
+++ b/providers/azure/internal/recommendations/converter.go
@@ -263,9 +263,9 @@ func termToMonths(term string) int {
 // ExpandPaymentVariants fans out a single Azure reservation recommendation
 // into two variants that differ only in payment schedule:
 //
-//   - "all-upfront"  — the full reservation cost is paid today; no monthly
+//   - "upfront"  — the full reservation cost is paid today; no monthly
 //     recurring charge (RecurringMonthlyCost = pointer to 0).
-//   - "no-upfront"   — nothing is paid today; the same total reservation cost
+//   - "monthly"  — nothing is paid today; the same total reservation cost
 //     is spread evenly across the term months (RecurringMonthlyCost =
 //     CommitmentCost / termMonths).
 //
@@ -274,7 +274,7 @@ func termToMonths(term string) int {
 // and SavingsPercentage vs on-demand are identical between the two variants;
 // only the cashflow split changes.
 //
-// The base recommendation must already have PaymentOption set to "all-upfront"
+// The base recommendation must already have PaymentOption set to "upfront"
 // and a valid CommitmentCost (total reservation price) and OnDemandCost (total
 // on-demand cost over the same period). If OnDemandCost is zero the savings
 // fields are forced to zero to avoid a divide-by-zero; if CommitmentCost is
@@ -294,13 +294,13 @@ func ExpandPaymentVariants(base common.Recommendation) []common.Recommendation {
 	recurringMonthly := totalReservation / float64(months)
 
 	allUpfront := base
-	allUpfront.PaymentOption = "all-upfront"
+	allUpfront.PaymentOption = "upfront"
 	allUpfront.EstimatedSavings = savings
 	allUpfront.SavingsPercentage = savingsPct
 	allUpfront.RecurringMonthlyCost = float64Ptr(0)
 
 	noUpfront := base
-	noUpfront.PaymentOption = "no-upfront"
+	noUpfront.PaymentOption = "monthly"
 	noUpfront.EstimatedSavings = savings
 	noUpfront.SavingsPercentage = savingsPct
 	noUpfront.RecurringMonthlyCost = float64Ptr(recurringMonthly)
diff --git a/providers/azure/internal/recommendations/converter_test.go b/providers/azure/internal/recommendations/converter_test.go
index 04fa41784..3868338c9 100644
--- a/providers/azure/internal/recommendations/converter_test.go
+++ b/providers/azure/internal/recommendations/converter_test.go
@@ -251,7 +251,7 @@ func baseRec(service common.ServiceType, term string, onDemand, commitment float
 		ResourceType:   "Standard_D2s_v3",
 		CommitmentType: common.CommitmentReservedInstance,
 		Term:           term,
-		PaymentOption:  "all-upfront",
+		PaymentOption:  "upfront",
 		OnDemandCost:   onDemand,
 		CommitmentCost: commitment,
 	}
@@ -264,8 +264,8 @@ func TestExpandPaymentVariants_ReturnsTwoVariants(t *testing.T) {
 
 func TestExpandPaymentVariants_PaymentOptionValues(t *testing.T) {
 	variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70))
-	assert.Equal(t, "all-upfront", variants[0].PaymentOption)
-	assert.Equal(t, "no-upfront", variants[1].PaymentOption)
+	assert.Equal(t, "upfront", variants[0].PaymentOption)
+	assert.Equal(t, "monthly", variants[1].PaymentOption)
 }
 
 func TestExpandPaymentVariants_AllUpfrontCashflow(t *testing.T) {
diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go
index c981ea2bc..4019c46ec 100644
--- a/providers/azure/recommendations.go
+++ b/providers/azure/recommendations.go
@@ -298,7 +298,7 @@ func (r *RecommendationsClientAdapter) convertAdvisorRecommendation(advisorRec *
 		Account:        r.subscriptionID,
 		CommitmentType: common.CommitmentReservedInstance,
 		Term:           "1yr",
-		PaymentOption:  "all-upfront",
+		PaymentOption:  "upfront",
 	}
 
 	rec.Region = resolveAdvisorRegion(advisorRec)
diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go
index 1b79d5524..b2eb251db 100644
--- a/providers/azure/recommendations_test.go
+++ b/providers/azure/recommendations_test.go
@@ -344,7 +344,7 @@ func TestConvertAdvisorRecommendation(t *testing.T) {
 	assert.Equal(t, "test-subscription", result.Account)
 	assert.Equal(t, common.CommitmentReservedInstance, result.CommitmentType)
 	assert.Equal(t, "1yr", result.Term)
-	assert.Equal(t, "all-upfront", result.PaymentOption)
+	assert.Equal(t, "upfront", result.PaymentOption)
 }
 
 func TestConvertAdvisorRecommendation_NilProperties(t *testing.T) {
diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go
index a0ff240fd..32d9b55cb 100644
--- a/providers/azure/services/cache/client.go
+++ b/providers/azure/services/cache/client.go
@@ -598,7 +598,7 @@ func (c *CacheClient) convertAzureRedisRecommendation(ctx context.Context, azure
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "all-upfront",
+		PaymentOption:        "upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go
index e89a54d27..930f39885 100644
--- a/providers/azure/services/cache/client_test.go
+++ b/providers/azure/services/cache/client_test.go
@@ -787,7 +787,7 @@ func TestCacheClient_ConvertAzureRedisRecommendation_PopulatesAllFields(t *testi
 	assert.InDelta(t, 15.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "3yr", out.Term)
-	assert.Equal(t, "all-upfront", out.PaymentOption)
+	assert.Equal(t, "upfront", out.PaymentOption)
 
 	// Details carries Engine=redis + NodeType from the SKU string. Shards
 	// stays 0 when no cache instance matches in the subscription (the
diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go
index 2181c0db0..c32be7761 100644
--- a/providers/azure/services/compute/client.go
+++ b/providers/azure/services/compute/client.go
@@ -726,7 +726,7 @@ func (c *ComputeClient) convertAzureVMRecommendation(ctx context.Context, azureR
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "all-upfront",
+		PaymentOption:        "upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/compute/client_test.go b/providers/azure/services/compute/client_test.go
index f8a4f74c9..740c384ba 100644
--- a/providers/azure/services/compute/client_test.go
+++ b/providers/azure/services/compute/client_test.go
@@ -221,7 +221,7 @@ func TestComputeClient_GetRecommendations_WithMock(t *testing.T) {
 
 // TestComputeClient_GetRecommendations_EmitsBothPaymentVariants asserts that
 // a single Azure reservation recommendation from the API is expanded into two
-// entries — "all-upfront" and "no-upfront" — with correct cashflow split and
+// entries — "upfront" and "monthly" — with correct cashflow split and
 // identical savings figures.
 func TestComputeClient_GetRecommendations_EmitsBothPaymentVariants(t *testing.T) {
 	ctx := context.Background()
@@ -250,13 +250,13 @@ func TestComputeClient_GetRecommendations_EmitsBothPaymentVariants(t *testing.T)
 	for _, r := range recs {
 		payments[r.PaymentOption] = r
 	}
-	require.Contains(t, payments, "all-upfront", "all-upfront variant must be present")
-	require.Contains(t, payments, "no-upfront", "no-upfront variant must be present")
+	require.Contains(t, payments, "upfront", "upfront variant must be present")
+	require.Contains(t, payments, "monthly", "monthly variant must be present")
 
-	allUp := payments["all-upfront"]
-	noUp := payments["no-upfront"]
+	allUp := payments["upfront"]
+	noUp := payments["monthly"]
 
-	// Cashflow: all-upfront has zero recurring; no-upfront spreads over 12 months.
+	// Cashflow: upfront has zero recurring; monthly spreads over 12 months.
 	require.NotNil(t, allUp.RecurringMonthlyCost)
 	assert.InDelta(t, 0.0, *allUp.RecurringMonthlyCost, 1e-9)
 	require.NotNil(t, noUp.RecurringMonthlyCost)
@@ -883,7 +883,7 @@ func TestComputeClient_ConvertAzureVMRecommendation_PopulatesAllFields(t *testin
 	assert.InDelta(t, 30.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "3yr", out.Term)
-	assert.Equal(t, "all-upfront", out.PaymentOption)
+	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).
diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go
index 28f623dca..c60d77ddc 100644
--- a/providers/azure/services/cosmosdb/client.go
+++ b/providers/azure/services/cosmosdb/client.go
@@ -598,7 +598,7 @@ func (c *CosmosDBClient) convertAzureCosmosRecommendation(ctx context.Context, a
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "all-upfront",
+		PaymentOption:        "upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/cosmosdb/client_test.go b/providers/azure/services/cosmosdb/client_test.go
index 97bf385a2..7dd73df61 100644
--- a/providers/azure/services/cosmosdb/client_test.go
+++ b/providers/azure/services/cosmosdb/client_test.go
@@ -1003,7 +1003,7 @@ func TestCosmosDBClient_ConvertAzureCosmosRecommendation_PopulatesAllFields(t *t
 	assert.InDelta(t, 90.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "1yr", out.Term)
-	assert.Equal(t, "all-upfront", out.PaymentOption)
+	assert.Equal(t, "upfront", out.PaymentOption)
 
 	// Details carries Engine="cosmos". For this fixture the SKU doesn't
 	// start with digits, so ThroughputUnits is 0 — which correctly
diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go
index b41d37146..fb564fdaa 100644
--- a/providers/azure/services/database/client.go
+++ b/providers/azure/services/database/client.go
@@ -596,7 +596,7 @@ func (c *DatabaseClient) convertAzureSQLRecommendation(ctx context.Context, azur
 		RecurringMonthlyCost: f.RecurringMonthlyCost,
 		CommitmentType:       common.CommitmentReservedInstance,
 		Term:                 f.Term,
-		PaymentOption:        "all-upfront",
+		PaymentOption:        "upfront",
 		Timestamp:            time.Now(),
 		Details:              details,
 	}
diff --git a/providers/azure/services/database/client_test.go b/providers/azure/services/database/client_test.go
index bd3efcc6c..c26628466 100644
--- a/providers/azure/services/database/client_test.go
+++ b/providers/azure/services/database/client_test.go
@@ -694,7 +694,7 @@ func TestDatabaseClient_ConvertAzureSQLRecommendation_PopulatesAllFields(t *test
 	assert.InDelta(t, 60.0, out.EstimatedSavings, 1e-9)
 	assert.Equal(t, common.CommitmentReservedInstance, out.CommitmentType)
 	assert.Equal(t, "1yr", out.Term)
-	assert.Equal(t, "all-upfront", out.PaymentOption)
+	assert.Equal(t, "upfront", out.PaymentOption)
 
 	// Details carries Engine=sqlserver + InstanceClass from the SKU
 	// string. EngineVersion stays empty when the catalogue has no
diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go
index c8311bf9e..372a6ac6f 100644
--- a/providers/azure/services/search/client.go
+++ b/providers/azure/services/search/client.go
@@ -561,7 +561,7 @@ func (c *SearchClient) convertAzureSearchRecommendation(ctx context.Context, azu
 		EstimatedSavings:     extracted.EstimatedSavings,
 		Term:                 extracted.Term,
 		RecurringMonthlyCost: extracted.RecurringMonthlyCost,
-		PaymentOption:        "all-upfront", // Default, will be expanded by ExpandPaymentVariants
+		PaymentOption:        "upfront", // Default, will be expanded by ExpandPaymentVariants
 	}
 
 	// Override region with client region if extraction didn't find one
diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go
index 1638f966c..58d686681 100644
--- a/providers/azure/services/search/client_test.go
+++ b/providers/azure/services/search/client_test.go
@@ -618,7 +618,7 @@ func TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields(t *tes
 	assert.Equal(t, 2, rec.Count)
 	assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType)
 	assert.Equal(t, "1yr", rec.Term)
-	assert.Equal(t, "all-upfront", rec.PaymentOption)
+	assert.Equal(t, "upfront", rec.PaymentOption)
 }
 
 // MockTokenCredential for testing PurchaseCommitment

From 4bc19df71aaef3f29dc4ddded256628699a5e87f Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 22 May 2026 22:41:31 +0200
Subject: [PATCH 6/6] 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.
---
 .../azure/internal/recommendations/converter.go      |  5 +++--
 .../azure/internal/recommendations/converter_test.go | 12 ++++++++++++
 providers/azure/services/search/client_test.go       |  3 +++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go
index 7bea467ec..69d52f8b7 100644
--- a/providers/azure/internal/recommendations/converter.go
+++ b/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)
diff --git a/providers/azure/internal/recommendations/converter_test.go b/providers/azure/internal/recommendations/converter_test.go
index 3868338c9..f0ebd162c 100644
--- a/providers/azure/internal/recommendations/converter_test.go
+++ b/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))
diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go
index 58d686681..6f2e91f12 100644
--- a/providers/azure/services/search/client_test.go
+++ b/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)