fix(rds): re-derive family ProjectedCoverage after cap, override and dedupe

CHANGELOG.md

@@ -19,6 +19,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
   `--min-count` floor no longer drives the tally negative (which panicked in
   `DropSummary.Add`), and removed zero-Count entries are now counted and named
   (#1742).
+- `--target-coverage` RDS recommendations now report a `ProjectedCoverage` that
+  matches the final count after `--max-instances`, `--count-override` or the
+  duplicate-RI check lowered it, instead of the pre-cap family value (#2214).
 - `--max-instances` and `--min-count` no longer apply to Savings Plans, whose
   Count of 1 is not an instance count: a Savings Plan neither uses up the
   instance cap nor is dropped by the floor (#1613).

cmd/helpers.go

@@ -308,7 +308,9 @@ func mapRecs(recs []common.Recommendation, fn func(common.Recommendation) common
 // usage signal (zero, negative, NaN or infinite avg) and rows without a positive
 // Count are returned unchanged.
 // RDS ProjectedCoverage is a family aggregate set by family-NU sizing, which a
-// per-row formula would corrupt, so only RDS ProjectedUtilization is refreshed.
+// per-row formula would corrupt, so only RDS ProjectedUtilization is refreshed;
+// the family coverage is re-derived once on the final set by
+// rederiveRDSFamilyCoverage (#2214).
 func rederiveProjections(rec common.Recommendation) common.Recommendation {
 	avg := rec.AverageInstancesUsedPerHour
 	if common.IsSavingsPlan(rec.Service) || rec.Count <= 0 || !(avg > 0) || math.IsInf(avg, 1) {

cmd/instance_limit_savings_plan_test.go

@@ -101,7 +101,7 @@ func assertSameRelativeOrder(t *testing.T, in, got []common.Recommendation) {
 // scoreLimitAndDisplay hands MinCount to scorer.Score, which must exempt
 // Savings Plans (Count 1 is a placeholder) at the pinned library version.
 func TestScoreLimitAndDisplay_MinCountKeepsSavingsPlan(t *testing.T) {
-	result := scoreLimitAndDisplay([]common.Recommendation{mkSP(), mkRI("a", 3)}, Config{MinCount: 2}, nil)
+	result := scoreLimitAndDisplay([]common.Recommendation{mkSP(), mkRI("a", 3)}, Config{MinCount: 2}, nil, nil)
 
 	types := make([]common.CommitmentType, 0, len(result.Passed))
 	for _, r := range result.Passed {

cmd/multi_service.go

@@ -194,7 +194,7 @@ func runToolMultiService(ctx context.Context, cfg Config) {
 	exitIfInterrupted(ctx, "fetching recommendations")
 
 	// Phase 2: score, enforce the run-wide instance cap, and display.
-	scoredResult := scoreLimitAndDisplay(allRecs, cfg, drops)
+	scoredResult := scoreLimitAndDisplay(allRecs, cfg, drops, coverageMap)
 	if len(scoredResult.Passed) == 0 {
 		printDropSummary(drops)
 		AppLogger.Printf("\nℹ️  No recommendations passed filters. Nothing to purchase.\n")
@@ -348,20 +348,38 @@ func loadAWSConfig(ctx context.Context, cfg Config) (aws.Config, error) {
 // prompt and the purchase loop all describe the same post-cap set, and so the
 // instances that survive are the highest-savings ones run-wide (scorer.Score
 // sorts Passed by savings percentage descending).
-func scoreLimitAndDisplay(recs []common.Recommendation, cfg Config, drops *common.DropSummary) scorer.ScoredResult {
+//
+// RDS family coverage is re-derived after the cap (see rederiveRDSFamilyCoverage),
+// because the cap, --count-override and the duplicate check all change Count
+// after family-NU sizing wrote ProjectedCoverage (#2214). coverageMap must be
+// the map the sizing stage used.
+func scoreLimitAndDisplay(recs []common.Recommendation, cfg Config, drops *common.DropSummary, coverageMap recommendations.PoolCoverageMap) scorer.ScoredResult {
 	scorerCfg := scorer.Config{
 		MinSavingsPct:      cfg.MinSavingsPct,
 		MaxBreakEvenMonths: cfg.MaxBreakEvenMonths,
 		MinCount:           cfg.MinCount,
 	}
 	result := scorer.Score(recs, scorerCfg)
 	result.Passed = applyGlobalInstanceLimit(result.Passed, cfg, rankBySavingsPercentage, drops)
+	result.Passed = rederiveRDSFamilyCoverage(result.Passed, cfg, coverageMap)
 	fmt.Print(reporter.RenderTable(result))
 	fmt.Print(reporter.RenderExcluded(result))
 	fmt.Print(reporter.RenderSummary(result))
 	return result
 }
 
+// rederiveRDSFamilyCoverage refreshes the family-wide RDS ProjectedCoverage
+// from the final counts. It only runs for --target-coverage, the sole path
+// where ApplyFamilyNUSizingRDS wrote that field; on the legacy --coverage path
+// the field is a per-pool value that RederiveFamilyCoverage would overwrite.
+// The CSV path has no coverage map and does not call it.
+func rederiveRDSFamilyCoverage(recs []common.Recommendation, cfg Config, coverageMap recommendations.PoolCoverageMap) []common.Recommendation {
+	if cfg.TargetCoverage <= 0 {
+		return recs
+	}
+	return recommendations.RederiveFamilyCoverage(recs, coverageMap)
+}
+
 // rankingRule names the ordering --max-instances consumes, so the cap can say
 // which rule decided what survived. The two paths rank on different keys
 // because they carry different data, and an operator reading the drop list has

cmd/multi_service_max_instances_test.go

@@ -103,7 +103,7 @@ func TestMaxInstancesCapsWholeRunAcrossServicesAndRegions(t *testing.T) {
 	require.Equal(t, naturalTotal, CalculateTotalInstances(allRecs),
 		"fetch stage must not apply the cap per region")
 
-	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops)
+	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops, nil)
 
 	total := CalculateTotalInstances(scored.Passed)
 	assert.LessOrEqual(t, total, maxInstances,
@@ -176,7 +176,7 @@ func TestMaxInstancesKeepsHighestSavingsNotFirstFetched(t *testing.T) {
 	awsStub.requireDuplicateChecks(t, len(maxInstancesFixtureServices)*len(maxInstancesFixtureRegions))
 	require.Len(t, allRecs, len(maxInstancesFixtureServices)*len(maxInstancesFixtureRegions))
 
-	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops)
+	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops, nil)
 
 	// The total is respected under either placement, so it proves nothing on
 	// its own here. It is asserted only to keep the fixture honest.
@@ -246,7 +246,7 @@ func TestMaxInstancesNeverPurchasesBelowMinCount(t *testing.T) {
 	awsStub.requireDuplicateChecks(t, len(maxInstancesFixtureServices)*len(maxInstancesFixtureRegions))
 	require.Len(t, allRecs, len(maxInstancesFixtureServices)*len(maxInstancesFixtureRegions))
 
-	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops)
+	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops, nil)
 
 	// The property under test. Every purchased recommendation honors the
 	// floor; none is bought at a truncated count below it.
@@ -333,7 +333,7 @@ func TestMaxInstancesNotAppliedWhenUnset(t *testing.T) {
 		maxInstancesFixtureServices, engineVersionData{}, toolCfg, nil)
 	awsStub.requireDuplicateChecks(t, len(maxInstancesFixtureServices)*len(maxInstancesFixtureRegions))
 
-	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops)
+	scored := scoreLimitAndDisplay(allRecs, toolCfg, drops, nil)
 
 	pairs := len(maxInstancesFixtureServices) * len(maxInstancesFixtureRegions)
 	assert.Len(t, scored.Passed, pairs)

cmd/multi_service_rederive_coverage_test.go

@@ -0,0 +1,66 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func rederiveRDSRec(count int) common.Recommendation {
+	return common.Recommendation{
+		Service: common.ServiceRDS, CommitmentType: common.CommitmentReservedInstance,
+		Region: "us-east-1", ResourceType: "db.r7g.large", Count: count,
+		CommitmentCost: float64(count) * 100, EstimatedSavings: 100,
+		AverageInstancesUsedPerHour: 3,
+		Details:                     &common.DatabaseDetails{Engine: "aurora-mysql", AZConfig: "single-az"},
+	}
+}
+
+var rederiveCoverage = recommendations.PoolCoverageMap{
+	"us-east-1:db.r7g.large:auroramysql:singleaz": {Pct: 20, AvgInstancesPerHour: 40},
+}
+
+// sizedRederiveRec returns one family-NU sized RDS rec (the --target-coverage
+// output) and its ProjectedCoverage.
+func sizedRederiveRec(t *testing.T) common.Recommendation {
+	t.Helper()
+	sized, _, _ := recommendations.ApplyFamilyNUSizingRDS([]common.Recommendation{rederiveRDSRec(30)}, rederiveCoverage, 90)
+	require.Len(t, sized, 1)
+	require.Greater(t, sized[0].Count, 5)
+	return sized[0]
+}
+
+func TestScoreLimitAndDisplay_TargetCoverageRederivesRDSCoverageAfterCap(t *testing.T) {
+	sized := sizedRederiveRec(t)
+	uncapped := sized.ProjectedCoverage
+
+	got := scoreLimitAndDisplay([]common.Recommendation{sized}, Config{TargetCoverage: 90, MaxInstances: 5}, nil, rederiveCoverage)
+
+	require.Len(t, got.Passed, 1)
+	assert.Equal(t, 5, got.Passed[0].Count)
+	// existing 20% + 5 instances of 40 = 12.5%
+	assert.InDelta(t, 32.5, got.Passed[0].ProjectedCoverage, 1e-6)
+	assert.Less(t, got.Passed[0].ProjectedCoverage, uncapped)
+}
+
+func TestScoreLimitAndDisplay_LegacyCoverageLeavesProjectedCoverage(t *testing.T) {
+	rec := rederiveRDSRec(30)
+	rec.ExistingCoverageKnown, rec.ExistingCoveragePct, rec.ProjectedCoverage = true, 20, 77
+
+	got := scoreLimitAndDisplay([]common.Recommendation{rec}, Config{Coverage: 80, MaxInstances: 5}, nil, rederiveCoverage)
+
+	require.Len(t, got.Passed, 1)
+	assert.Equal(t, 77.0, got.Passed[0].ProjectedCoverage)
+}
+
+func TestScoreLimitAndDisplay_CSVPathNilCoverageUnchanged(t *testing.T) {
+	sized := sizedRederiveRec(t)
+
+	got := scoreLimitAndDisplay([]common.Recommendation{sized}, Config{TargetCoverage: 90}, nil, nil)
+
+	require.Len(t, got.Passed, 1)
+	assert.Equal(t, sized.ProjectedCoverage, got.Passed[0].ProjectedCoverage)
+}