feat(cli): end-of-run summary of dropped recommendations (closes #361)

cmd/helpers.go

@@ -123,6 +123,12 @@ func CalculateTotalInstances(recs []common.Recommendation) int {
 // on-demand ratio) and stays unscaled. Pre-sizing values can still be
 // recovered: RecommendedCount holds AWS's pre-sized count for RIs.
 func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Recommendation {
+	return applyCoverage(recs, coverage, nil)
+}
+
+// applyCoverage applies legacy percentage sizing and optionally records
+// recommendations whose discrete RI count is reduced to zero.
+func applyCoverage(recs []common.Recommendation, coverage float64, drops *common.DropSummary) []common.Recommendation {
 	if coverage >= 100 {
 		return recs
 	}
@@ -170,6 +176,8 @@ func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Reco
 			adjusted = common.ScaleRecommendationCosts(adjusted, sizedRatio)
 			adjusted.Count = newCount
 			result = append(result, adjusted)
+		} else if drops != nil {
+			drops.Add(common.DropTargetSizedToZero, 1)
 		}
 	}
 	return result
@@ -241,7 +249,10 @@ func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Reco
 //
 // Recs of any other CommitmentType are passed through unmodified (warned
 // once per type per run).
-func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []common.Recommendation {
+// ApplyTargetCoverage applies the target coverage percentage to a slice of
+// recommendations. drops accumulates per-reason drop counts for the
+// end-of-run summary; pass nil to skip tracking.
+func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64, drops *common.DropSummary) []common.Recommendation {
 	if targetPct <= 0 || targetPct > 100 {
 		// Validation ensures we never get here in production, but be defensive
 		// so a buggy caller doesn't divide by zero.
@@ -253,14 +264,15 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm
 	var skipped int
 	unsupportedSeen := make(map[common.CommitmentType]bool)
 
-	for _rvc := range recs {
-		rec := recs[_rvc]
-		adjusted, kept, missingSignal := applyTargetCoverageOne(rec, targetPct, unsupportedSeen)
+	for i := range recs {
+		adjusted, kept, missingSignal, dropReason := applyTargetCoverageOne(recs[i], targetPct, unsupportedSeen)
 		if missingSignal {
 			skipped++
 		}
 		if kept {
 			result = append(result, adjusted)
+		} else if dropReason != "" {
+			drops.Add(dropReason, 1)
 		}
 	}
 
@@ -273,51 +285,53 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm
 }
 
 // applyTargetCoverageOne dispatches a single recommendation through the
-// appropriate branch. Returns (rec, kept, missingSignal):
+// appropriate branch. Returns (rec, kept, missingSignal, dropReason):
 //   - kept=true → caller appends `rec` (the adjusted or pass-through value).
 //   - kept=false → caller drops the rec (only the RI "target unreachable"
-//     branch returns this; an INFO log already fired).
+//     branches return this; an INFO log already fired).
 //   - missingSignal=true → counted toward the end-of-run skip summary.
+//   - dropReason is non-empty when kept=false and the drop has a named category.
 //
 // Split out of ApplyTargetCoverage to keep that function under gocyclo's
 // complexity threshold.
-func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (common.Recommendation, bool, bool) {
+func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (result common.Recommendation, kept, missingSignal bool, drop string) {
 	switch {
 	case common.IsSavingsPlan(rec.Service):
 		adjusted, ok := applyTargetCoverageSP(rec, targetPct)
 		if !ok {
 			// SP no-signal: pass through unchanged.
-			return rec, true, true
+			return rec, true, true, ""
 		}
-		return adjusted, true, false
+		return adjusted, true, false, ""
 	case rec.CommitmentType == common.CommitmentReservedInstance:
-		adjusted, ok := applyTargetCoverageRI(rec, targetPct)
+		adjusted, ok, dropReason := applyTargetCoverageRI(rec, targetPct)
 		if !ok {
 			// Distinguish "no signal" (pass through, count in summary) from
 			// "target unreachable" (drop with already-fired INFO log).
 			if rec.AverageInstancesUsedPerHour <= 0 {
-				return rec, true, true
+				return rec, true, true, ""
 			}
-			return rec, false, false
+			return rec, false, false, dropReason
 		}
-		return adjusted, true, false
+		return adjusted, true, false, ""
 	default:
 		if !unsupportedSeen[rec.CommitmentType] {
 			AppLogger.Printf("WARNING: --target-coverage not supported for CommitmentType=%q; passing recommendations through unchanged\n", rec.CommitmentType)
 			unsupportedSeen[rec.CommitmentType] = true
 		}
-		return rec, true, false
+		return rec, true, false, ""
 	}
 }
 
 // applyTargetCoverageRI is the RI branch of ApplyTargetCoverage. Returns
-// (adjusted, true) on success, (rec, false) when the rec should be passed
-// through unscaled (no signal) or dropped (target unreachable). Caller
-// distinguishes the two via rec.AverageInstancesUsedPerHour.
-func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common.Recommendation, bool) {
+// (adjusted, true, "") on success, (rec, false, dropReason) when the rec
+// should be passed through unscaled (no signal) or dropped (target
+// unreachable). Caller distinguishes no-signal from drop via
+// rec.AverageInstancesUsedPerHour and uses dropReason for the summary.
+func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (result common.Recommendation, ok bool, drop string) {
 	if rec.AverageInstancesUsedPerHour <= 0 {
 		// No signal — caller will pass through and count in the summary.
-		return rec, false
+		return rec, false, ""
 	}
 
 	avg := rec.AverageInstancesUsedPerHour
@@ -346,7 +360,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
 		// pass through".
 		AppLogger.Printf("INFO: --target-coverage=%.1f%% already met by existing coverage %.1f%% for %s/%s/%s; dropped recommendation\n",
 			targetPct, rec.ExistingCoveragePct, rec.Service, rec.Region, rec.ResourceType)
-		return rec, false
+		return rec, false, common.DropTargetAlreadyMet
 	}
 	// Floor so we never over-shoot the target on integer-arithmetic edges.
 	// Strict-target semantics: 80% means "at most 80% coverage", not "at
@@ -366,7 +380,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
 		// Returning (_, false) with avg > 0 signals "drop, don't pass through".
 		// applyTargetCoverageRI's caller branches on
 		// rec.AverageInstancesUsedPerHour to distinguish drop vs no-signal.
-		return rec, false
+		return rec, false, common.DropTargetSizedToZero
 	}
 
 	// Cost-bearing fields scale by the ratio of sized-to-original count, so the
@@ -402,7 +416,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
 	}
 	adjusted.ProjectedUtilization = projUtil
 	adjusted.ProjectedCoverage = projCov
-	return adjusted, true
+	return adjusted, true, ""
 }
 
 // applyTargetCoverageSP is the SP branch of ApplyTargetCoverage. Returns
@@ -463,11 +477,14 @@ func applyTargetCoverageSP(rec common.Recommendation, targetPct float64) (common
 // (the main path passes cfg.Coverage; the CSV path passes csvModeCoverage,
 // which substitutes the default 80% with 100% so CSV-driven counts aren't
 // silently dropped).
-func applySizing(recs []common.Recommendation, cfg Config, coverage float64) []common.Recommendation {
+//
+// drops accumulates per-reason drop counts for the end-of-run summary.
+// Pass nil to skip tracking.
+func applySizing(recs []common.Recommendation, cfg Config, coverage float64, drops *common.DropSummary) []common.Recommendation {
 	if cfg.TargetCoverage > 0 {
-		return ApplyTargetCoverage(recs, cfg.TargetCoverage)
+		return ApplyTargetCoverage(recs, cfg.TargetCoverage, drops)
 	}
-	return ApplyCoverage(recs, coverage)
+	return applyCoverage(recs, coverage, drops)
 }
 
 // ApplyCountOverride overrides the count for all recommendations.

cmd/helpers_test.go

@@ -383,6 +383,21 @@ func TestApplyCoverage_RICostScaling(t *testing.T) {
 	assert.Equal(t, 60.0, monthly)
 }
 
+func TestApplySizing_LegacyCoverageRecordsCountsFlooredToZero(t *testing.T) {
+	recs := []common.Recommendation{
+		{Service: common.ServiceEC2, Count: 1},
+		{Service: common.ServiceRDS, Count: 10},
+	}
+	drops := common.NewDropSummary()
+
+	out := applySizing(recs, Config{}, 10, drops)
+
+	require.Len(t, out, 1)
+	assert.Equal(t, 1, out[0].Count)
+	assert.Equal(t, 1, drops.Total())
+	assert.Contains(t, drops.FormatOneLine(), common.DropTargetSizedToZero)
+}
+
 func TestAdjustRecommendationsForExisting(t *testing.T) {
 	ctx := context.Background()
 
@@ -1028,7 +1043,7 @@ func TestApplyTargetCoverage_RI(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			recs := []common.Recommendation{tt.rec}
-			out := ApplyTargetCoverage(recs, tt.target)
+			out := ApplyTargetCoverage(recs, tt.target, nil)
 			if tt.wantDropped {
 				if len(out) != 0 {
 					t.Fatalf("expected drop; got %d recs", len(out))
@@ -1079,7 +1094,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
 	// n = floor(8 * 80/100) = 6. Ratio = 6/10 = 0.6 (cost scaling still
 	// uses rec.Count to convert AWS's quoted cost-for-rec.Count into
 	// cost-for-nTarget).
-	out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
+	out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
 	require.Len(t, out, 1)
 	assert.Equal(t, 6, out[0].Count)
 	assert.InDelta(t, 600.0, out[0].CommitmentCost, 0.001, "CommitmentCost scales by nTarget/rec.Count")
@@ -1095,7 +1110,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
 		monthly := 50.0
 		recWithMonthly := rec
 		recWithMonthly.RecurringMonthlyCost = &monthly
-		out := ApplyTargetCoverage([]common.Recommendation{recWithMonthly}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{recWithMonthly}, 80, nil)
 		require.Len(t, out, 1)
 		require.NotNil(t, out[0].RecurringMonthlyCost, "scaled pointer should be non-nil")
 		assert.InDelta(t, 30.0, *out[0].RecurringMonthlyCost, 0.001, "monthly cost scales by 6/10")
@@ -1107,7 +1122,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
 		// AWS API didn't return RecurringStandardMonthlyCost (all-upfront,
 		// or field missing). The sized rec should also have nil so
 		// downstream renders "unknown" rather than zero.
-		out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
 		require.Len(t, out, 1)
 		assert.Nil(t, out[0].RecurringMonthlyCost, "nil input → nil output")
 	})
@@ -1198,7 +1213,7 @@ func TestApplyTargetCoverage_RI_ExistingCoverage(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, tt.target)
+			out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, tt.target, nil)
 			if tt.wantDropped {
 				assert.Len(t, out, 0, "expected drop")
 				return
@@ -1233,7 +1248,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
 		// flag's intent is "leave 20% headroom", so the commitment shrinks
 		// to 80% of AWS rec. All cost-bearing fields scale by 0.8.
 		// Projected util = 95/0.80 = 118.75 clamped to 100.
-		out := ApplyTargetCoverage([]common.Recommendation{mkSP(95)}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{mkSP(95)}, 80, nil)
 		require.Len(t, out, 1)
 		assert.InDelta(t, 1.6, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment, 0.001)
 		assert.InDelta(t, 800.0, out[0].CommitmentCost, 0.001, "CommitmentCost scales by target/100")
@@ -1247,7 +1262,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
 	t.Run("AWS below target — scale down by target (under-buy)", func(t *testing.T) {
 		// RecUtil=50, target=80. All cost-bearing fields shrink to 80%.
 		// Projected util = 50/0.80 = 62.5 (no clamp needed).
-		out := ApplyTargetCoverage([]common.Recommendation{mkSP(50)}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{mkSP(50)}, 80, nil)
 		require.Len(t, out, 1)
 		details := out[0].Details.(*common.SavingsPlanDetails)
 		assert.InDelta(t, 1.6, details.HourlyCommitment, 0.001)
@@ -1260,7 +1275,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
 	})
 
 	t.Run("no signal → passed through unchanged", func(t *testing.T) {
-		out := ApplyTargetCoverage([]common.Recommendation{mkSP(0)}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{mkSP(0)}, 80, nil)
 		require.Len(t, out, 1)
 		// Original recommendation values intact.
 		assert.Equal(t, 2.0, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment)
@@ -1281,7 +1296,7 @@ func TestApplySizing(t *testing.T) {
 
 	t.Run("TargetCoverage > 0 → ApplyTargetCoverage", func(t *testing.T) {
 		cfg := Config{TargetCoverage: 80, Coverage: 100}
-		out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage)
+		out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage, nil)
 		require.Len(t, out, 1)
 		// avg=8, target=80%, existing=0%. gap=80.
 		// n = floor(8 * 80/100) = floor(6.4) = 6. ProjUtil = 8/6 = 133% → 100.
@@ -1291,7 +1306,7 @@ func TestApplySizing(t *testing.T) {
 
 	t.Run("TargetCoverage == 0 → ApplyCoverage", func(t *testing.T) {
 		cfg := Config{TargetCoverage: 0, Coverage: 50}
-		out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage)
+		out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage, nil)
 		require.Len(t, out, 1)
 		// ApplyCoverage(50) on count=10 → 5. ProjectedUtilization NOT set
 		// (zero) because we took the coverage branch.
@@ -1336,7 +1351,7 @@ func TestApplyTargetCoverage_RI_Target100(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, 100)
+			out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, 100, nil)
 			if tt.wantDropped {
 				assert.Len(t, out, 0, "expected drop at target=100 for avg=%.3f", tt.rec.AverageInstancesUsedPerHour)
 				return
@@ -1360,7 +1375,7 @@ func TestApplyTargetCoverage_SP_NoSignalGuards(t *testing.T) {
 			RecommendedUtilization: 50,
 			Details:                &common.SavingsPlanDetails{HourlyCommitment: 0},
 		}
-		out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
 		require.Len(t, out, 1, "$0 SP rec should still be in output (pass-through)")
 		// Pass-through — projection fields must NOT be set, savings unchanged.
 		assert.Equal(t, 0.0, out[0].ProjectedUtilization, "ProjectedUtilization must NOT be set for $0-commitment pass-through")
@@ -1381,9 +1396,61 @@ func TestApplyTargetCoverage_SP_NoSignalGuards(t *testing.T) {
 			RecommendedUtilization: 50,
 			Details:                common.ComputeDetails{Platform: "Linux/UNIX"}, // wrong type
 		}
-		out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
+		out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
 		require.Len(t, out, 1)
 		assert.Equal(t, 0.0, out[0].ProjectedUtilization, "must NOT set projection when scaling failed")
 		assert.Equal(t, 1500.0, out[0].EstimatedSavings, "EstimatedSavings must remain unscaled when scaling failed")
 	})
 }
+
+// TestApplyTargetCoverage_DropTargetAlreadyMet verifies that when existing
+// coverage already meets or exceeds the target, the recommendation is
+// dropped and the drop is recorded in a non-nil DropSummary under the
+// DropTargetAlreadyMet category. If the drops.Add call for that branch
+// were removed, d.Total() would stay at 0 and the assertion below would fail.
+func TestApplyTargetCoverage_DropTargetAlreadyMet(t *testing.T) {
+	// ExistingCoveragePct=90 >= target=80: gapPct=80-90=-10 <= 0 -> drop.
+	rec := common.Recommendation{
+		Service:                     common.ServiceEC2,
+		Region:                      "us-east-1",
+		ResourceType:                "t3.medium",
+		Count:                       5,
+		CommitmentType:              common.CommitmentReservedInstance,
+		AverageInstancesUsedPerHour: 8.0,
+		ExistingCoveragePct:         90.0,
+	}
+
+	d := common.NewDropSummary()
+	out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, d)
+
+	assert.Empty(t, out, "already-covered rec should be dropped")
+	assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop")
+	assert.Contains(t, d.FormatOneLine(), common.DropTargetAlreadyMet,
+		"drop summary should name the target-already-met category")
+}
+
+// TestApplyTargetCoverage_DropTargetSizedToZero verifies that when the
+// floor(avg * gapPct / 100) formula produces 0, the recommendation is
+// dropped and the drop is recorded in a non-nil DropSummary under the
+// DropTargetSizedToZero category. If the drops.Add call for that branch
+// were removed, d.Total() would stay at 0 and the assertion below would fail.
+func TestApplyTargetCoverage_DropTargetSizedToZero(t *testing.T) {
+	// avg=0.4, target=80%, existing=0%: floor(0.4 * 80/100) = floor(0.32) = 0 -> drop.
+	rec := common.Recommendation{
+		Service:                     common.ServiceEC2,
+		Region:                      "us-east-1",
+		ResourceType:                "t3.micro",
+		Count:                       1,
+		CommitmentType:              common.CommitmentReservedInstance,
+		AverageInstancesUsedPerHour: 0.4,
+		ExistingCoveragePct:         0.0,
+	}
+
+	d := common.NewDropSummary()
+	out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, d)
+
+	assert.Empty(t, out, "floor-to-zero rec should be dropped")
+	assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop")
+	assert.Contains(t, d.FormatOneLine(), common.DropTargetSizedToZero,
+		"drop summary should name the target-sized-to-zero category")
+}