fix(common): treat a zero upfront on an upfront payment option as unknown

pkg/common/breakeven.go

@@ -1,12 +1,17 @@
 package common //nolint:revive // pkg/common is the module's core domain package (Recommendation, ProviderType, ...), imported by name (common.X) throughout this repo and by consumer-mcp; renaming would break every consumer import
 
-import "math"
+import (
+	"math"
+	"strings"
+)
 
 // BreakEven returns the number of months until the upfront payment of rec is
 // recovered by its recurring discount, and whether that is known.
 //
 //   - A positive rec.BreakEvenMonths set by a provider wins.
 //   - No upfront (CommitmentCost == 0): (0, true). There is nothing to recover.
+//     Exception: a zero upfront on a payment option that carries an upfront
+//     (see UpfrontPriceUnknown) is a missing price, not a known zero: (0, false).
 //   - Otherwise upfront / gross monthly savings, where gross monthly savings is
 //     EstimatedSavings plus the upfront amortized over the term. EstimatedSavings
 //     is the net monthly figure (already after the amortized upfront), so using
@@ -28,7 +33,7 @@ func BreakEven(rec Recommendation) (months float64, known bool) {
 		return 0, false
 	}
 	if upfront == 0 {
-		return 0, true
+		return 0, !UpfrontPriceUnknown(rec)
 	}
 	termMonths, ok := breakEvenTermMonths(rec.Term)
 	if !ok {
@@ -53,3 +58,21 @@ func breakEvenTermMonths(term string) (float64, bool) {
 }
 
 func isFinite(v float64) bool { return !math.IsNaN(v) && !math.IsInf(v, 0) }
+
+// UpfrontPriceUnknown reports whether rec's zero CommitmentCost is a missing
+// price rather than a known zero upfront: the payment option says an upfront
+// is charged (all, partial or Azure "upfront"), but the provider gave no
+// figure. Azure Advisor "upfront" variants and AWS recommendations with a nil
+// UpfrontCost have this shape. No-upfront, monthly and empty or unrecognized
+// payment options keep a zero as a known zero upfront.
+func UpfrontPriceUnknown(rec Recommendation) bool {
+	if rec.CommitmentCost != 0 {
+		return false
+	}
+	switch strings.NewReplacer("-", "", "_", "", " ", "").Replace(strings.ToLower(strings.TrimSpace(rec.PaymentOption))) {
+	case "allupfront", "upfront", "partialupfront", "partial":
+		return true
+	default:
+		return false
+	}
+}

pkg/common/breakeven_test.go

@@ -27,6 +27,13 @@ func TestBreakEven(t *testing.T) {
 		{"empty term", Recommendation{CommitmentCost: 100, EstimatedSavings: 10}, 0, false},
 		{"negative upfront", Recommendation{CommitmentCost: -1, EstimatedSavings: 10, Term: "1yr"}, 0, false},
 		{"nan savings", Recommendation{CommitmentCost: 10, EstimatedSavings: math.NaN(), Term: "1yr"}, 0, false},
+		// #392: a zero upfront on an upfront-bearing payment option is a missing price.
+		{"azure advisor upfront variant", Recommendation{PaymentOption: "upfront", EstimatedSavings: 50, Term: "1yr"}, 0, false},
+		{"all upfront without price", Recommendation{PaymentOption: "all-upfront", EstimatedSavings: 50, Term: "3yr"}, 0, false},
+		{"partial upfront without price", Recommendation{PaymentOption: "partial-upfront", EstimatedSavings: 50, Term: "1yr"}, 0, false},
+		{"no upfront option is known zero", Recommendation{PaymentOption: "no-upfront", EstimatedSavings: 50, Term: "1yr"}, 0, true},
+		{"azure monthly variant is known zero", Recommendation{PaymentOption: "monthly", EstimatedSavings: 50, Term: "1yr"}, 0, true},
+		{"provider value wins over missing upfront", Recommendation{BreakEvenMonths: 9, PaymentOption: "all-upfront", Term: "1yr"}, 9, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -45,3 +52,34 @@ func TestBreakEven_InvariantUnderScaling(t *testing.T) {
 	after, _ := BreakEven(ScaleRecommendationCosts(rec, 0.4))
 	assert.InDelta(t, before, after, 1e-9)
 }
+
+func TestUpfrontPriceUnknown(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		payment string
+		cost    float64
+		want    bool
+	}{
+		{"all-upfront", 0, true},
+		{"partial-upfront", 0, true},
+		{"upfront", 0, true},
+		{"partial", 0, true},
+		{"All Upfront", 0, true},
+		{" Partial Upfront ", 0, true},
+		{"ALL_UPFRONT", 0, true},
+		{"all-upfront", 720, false},
+		{"upfront", 0.01, false},
+		{"no-upfront", 0, false},
+		{"No Upfront", 0, false},
+		{"NO_UPFRONT", 0, false},
+		{"monthly", 0, false},
+		{"", 0, false},
+		{"weekly-bananas", 0, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.payment, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.want, UpfrontPriceUnknown(Recommendation{PaymentOption: tt.payment, CommitmentCost: tt.cost}))
+		})
+	}
+}

pkg/scorer/scorer.go

@@ -15,11 +15,16 @@ import (
 // Zero values mean "no filter" for numeric thresholds; empty slice means "all services".
 type Config struct {
 	MinSavingsPct      float64  // Minimum savings percentage. 0 = no filter.
-	MaxBreakEvenMonths int      // Maximum break-even months. 0 = no filter. No-upfront recs pass; recs whose break-even cannot be computed are filtered.
+	MaxBreakEvenMonths int      // Maximum break-even months. 0 = no filter. No-upfront recs pass; recs whose break-even cannot be computed are filtered (an upfront payment option with no upfront price as FilterReasonPriceUnknown).
 	MinCount           int      // Minimum count per recommendation. 0 = no filter.
 	EnabledServices    []string // Empty = all services. E.g. ["ec2", "rds"].
 }
 
+// FilterReasonPriceUnknown prefixes the FilterReason of a recommendation
+// dropped by MaxBreakEvenMonths because its upfront price is unknown
+// (common.UpfrontPriceUnknown).
+const FilterReasonPriceUnknown = "price-unknown"
+
 // FilteredRecommendation holds a recommendation that did not pass a filter, with the reason.
 type FilteredRecommendation struct {
 	Recommendation common.Recommendation
@@ -90,12 +95,8 @@ func filterReason(rec common.Recommendation, cfg Config, enabledServices map[str
 	}
 
 	if cfg.MaxBreakEvenMonths > 0 {
-		months, known := common.BreakEven(rec)
-		if !known {
-			return fmt.Sprintf("break-even cannot be computed (upfront %.2f, savings %.2f, term %q); --max-break-even-months %d cannot be enforced", rec.CommitmentCost, rec.EstimatedSavings, rec.Term, cfg.MaxBreakEvenMonths)
-		}
-		if months > float64(cfg.MaxBreakEvenMonths) {
-			return fmt.Sprintf("break-even %.1f months exceeds maximum %d months", months, cfg.MaxBreakEvenMonths)
+		if reason := breakEvenReason(rec, cfg.MaxBreakEvenMonths); reason != "" {
+			return reason
 		}
 	}
 
@@ -106,6 +107,22 @@ func filterReason(rec common.Recommendation, cfg Config, enabledServices map[str
 	return ""
 }
 
+// breakEvenReason returns why rec fails a positive maxMonths break-even
+// filter, or "" if it passes. Unknown break-even fails closed.
+func breakEvenReason(rec common.Recommendation, maxMonths int) string {
+	months, known := common.BreakEven(rec)
+	if !known {
+		if common.UpfrontPriceUnknown(rec) {
+			return fmt.Sprintf("%s: no upfront price for payment option %q; --max-break-even-months %d cannot be enforced", FilterReasonPriceUnknown, rec.PaymentOption, maxMonths)
+		}
+		return fmt.Sprintf("break-even cannot be computed (upfront %.2f, savings %.2f, term %q); --max-break-even-months %d cannot be enforced", rec.CommitmentCost, rec.EstimatedSavings, rec.Term, maxMonths)
+	}
+	if months > float64(maxMonths) {
+		return fmt.Sprintf("break-even %.1f months exceeds maximum %d months", months, maxMonths)
+	}
+	return ""
+}
+
 // buildServiceSet converts a slice of service names to a lowercase set for O(1) lookup.
 func buildServiceSet(services []string) map[string]struct{} {
 	if len(services) == 0 {

pkg/scorer/scorer_test.go

@@ -1,6 +1,7 @@
 package scorer
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
@@ -167,3 +168,46 @@ func TestScore_MaxBreakEvenMonths_ComputedFromCosts(t *testing.T) {
 	assert.Len(t, loose.Passed, 1)
 	assert.InDelta(t, 7.2, loose.Passed[0].BreakEvenMonths, 1e-9, "computed value is surfaced for reports")
 }
+
+// #392: Azure Advisor fans each recommendation out to an "upfront" and a
+// "monthly" variant, both with CommitmentCost 0 because Advisor reports no
+// price. The upfront variant's break-even is unknown, not 0 months.
+func TestScore_MaxBreakEvenMonths_PriceUnknown(t *testing.T) {
+	t.Parallel()
+	upfront := rec(common.ServiceCompute, "eastus", "Standard_D2s_v3", 0, 50, 0, 1, 0)
+	upfront.Term, upfront.PaymentOption = "1yr", "upfront"
+	monthly := upfront
+	monthly.PaymentOption = "monthly"
+	awsNoUpfront := rec(common.ServiceEC2, "us-east-1", "m5.large", 30, 40, 0, 1, 0)
+	awsNoUpfront.Term, awsNoUpfront.PaymentOption = "1yr", "no-upfront"
+	awsPartialNoPrice := awsNoUpfront
+	awsPartialNoPrice.ResourceType, awsPartialNoPrice.PaymentOption = "c5.large", "partial-upfront"
+	providerValue := awsPartialNoPrice
+	providerValue.ResourceType, providerValue.BreakEvenMonths = "r5.large", 6
+	recs := []common.Recommendation{upfront, monthly, awsNoUpfront, awsPartialNoPrice, providerValue}
+
+	result := Score(recs, Config{MaxBreakEvenMonths: 12})
+
+	passed := map[string]bool{}
+	for _, p := range result.Passed {
+		passed[p.ResourceType+"|"+p.PaymentOption] = true
+	}
+	assert.Equal(t, map[string]bool{
+		"Standard_D2s_v3|monthly":  true,
+		"m5.large|no-upfront":      true,
+		"r5.large|partial-upfront": true,
+	}, passed, "known zero upfront and a provider break-even still pass")
+	assert.Len(t, result.Filtered, 2)
+	for _, f := range result.Filtered {
+		assert.True(t, strings.HasPrefix(f.FilterReason, FilterReasonPriceUnknown+":"), f.FilterReason)
+		assert.Contains(t, f.FilterReason, f.Recommendation.PaymentOption)
+	}
+
+	unfiltered := Score(recs, Config{})
+	assert.Len(t, unfiltered.Passed, len(recs), "price-unknown only drops recs when break-even filtering is active")
+	for _, p := range unfiltered.Passed {
+		if p.ResourceType != "r5.large" {
+			assert.Zero(t, p.BreakEvenMonths, "unknown or zero break-even is never surfaced")
+		}
+	}
+}