fix(cli): log dropped recs from --min-pool-size filter (closes #359)

cmd/multi_service_filters.go

@@ -1,24 +1,40 @@
 package main
 
 import (
+	"fmt"
+	"log"
 	"slices"
 	"strings"
 
 	"github.com/LeanerCloud/CUDly/pkg/common"
 )
 
 // applyFilters applies region, instance type, engine, and engine version filters to recommendations.
-// currentRegion is the region being processed in the current loop iteration - if non-empty, only recommendations for that region are included.
+// currentRegion is the region being processed in the current loop iteration; if non-empty, only
+// recommendations for that region are included.
 func applyFilters(recs []common.Recommendation, cfg *Config, instanceVersions map[string][]InstanceEngineVersion, versionInfo map[string]MajorEngineVersionInfo, currentRegion string) []common.Recommendation {
 	var filtered []common.Recommendation
+	var poolDropCount int
+	var poolDropInstances float64
 
 	for i := range recs {
+		if cfg.MinPoolSize > 0 && !shouldIncludePoolSize(&recs[i], cfg) {
+			poolDropInstances += recs[i].AverageInstancesUsedPerHour
+			label := fmt.Sprintf("%s/%s/%s", recs[i].Service, recs[i].Region, recs[i].ResourceType)
+			log.Printf("INFO: --min-pool-size=%.1f dropped %s (avg=%.2f < threshold)", cfg.MinPoolSize, label, recs[i].AverageInstancesUsedPerHour)
+			poolDropCount++
+			continue
+		}
 		adjusted, include := processRecommendation(&recs[i], cfg, instanceVersions, versionInfo, currentRegion)
 		if include {
 			filtered = append(filtered, adjusted)
 		}
 	}
 
+	if poolDropCount > 0 {
+		log.Printf("INFO: --min-pool-size dropped %d recommendation(s) (%.2f avg instances/hr total)", poolDropCount, poolDropInstances)
+	}
+
 	return filtered
 }
 
@@ -27,9 +43,9 @@ func applyFilters(recs []common.Recommendation, cfg *Config, instanceVersions ma
 // boolean-filter checks are delegated to passesDimensionFilters to keep
 // this function under gocyclo's complexity threshold.
 func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVersions map[string][]InstanceEngineVersion, versionInfo map[string]MajorEngineVersionInfo, currentRegion string) (common.Recommendation, bool) {
-	// Filter to only recommendations for the current region being processed
-	// This prevents duplicating recommendations across all regions
-	// Skip this filter for Savings Plans as they are account-level, not regional
+	// Filter to only recommendations for the current region being processed.
+	// This prevents duplicating recommendations across all regions.
+	// Skip this filter for Savings Plans as they are account-level, not regional.
 	if currentRegion != "" && rec.Region != currentRegion && !common.IsSavingsPlan(rec.Service) {
 		return *rec, false
 	}
@@ -38,10 +54,10 @@ func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVers
 		return *rec, false
 	}
 
-	// Apply engine version filters - adjust instance count by subtracting extended support versions
+	// Apply engine version filters - adjust instance count by subtracting extended support versions.
 	if !cfg.IncludeExtendedSupport {
 		adjusted := adjustRecommendationForExcludedVersions(*rec, instanceVersions, versionInfo)
-		// Skip if all instances were excluded (count reduced to 0)
+		// Skip if all instances were excluded (count reduced to 0).
 		if adjusted.Count <= 0 {
 			return adjusted, false
 		}
@@ -52,11 +68,11 @@ func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVers
 }
 
 // passesDimensionFilters runs the stateless include/exclude checks on
-// region, instance type, engine, account, and pool size. Returns false on
+// region, instance type, engine, and account. Returns false on
 // the first failing filter. Split out of processRecommendation to keep
 // each function's cyclomatic complexity under the gocyclo limit; the
 // dimension filters here are pure functions of rec + cfg with no side
-// effects.
+// effects. Pool-size filtering is handled with logging in applyFilters.
 func passesDimensionFilters(rec *common.Recommendation, cfg *Config) bool {
 	if !shouldIncludeRegion(rec.Region, cfg) {
 		return false
@@ -70,9 +86,6 @@ func passesDimensionFilters(rec *common.Recommendation, cfg *Config) bool {
 	if !shouldIncludeAccount(rec.AccountName, cfg) {
 		return false
 	}
-	if !shouldIncludePoolSize(rec, cfg) {
-		return false
-	}
 	return true
 }
 
@@ -84,7 +97,7 @@ func passesDimensionFilters(rec *common.Recommendation, cfg *Config) bool {
 // where target can be meaningfully approximated.
 //
 // Pass-through cases: filter disabled (MinPoolSize<=0), or rec has no
-// per-hour signal (avg<=0 - SPs and recs CE didn't return usage for).
+// per-hour signal (avg<=0 -- SPs and recs CE didn't return usage for).
 // Those pools aren't sized via the per-hour formula so the filter doesn't
 // apply to them.
 func shouldIncludePoolSize(rec *common.Recommendation, cfg *Config) bool {
@@ -99,12 +112,12 @@ func shouldIncludePoolSize(rec *common.Recommendation, cfg *Config) bool {
 
 // shouldIncludeRegion checks if a region should be included based on filters.
 func shouldIncludeRegion(region string, cfg *Config) bool {
-	// If include list is specified, region must be in it
+	// If include list is specified, region must be in it.
 	if len(cfg.IncludeRegions) > 0 && !slices.Contains(cfg.IncludeRegions, region) {
 		return false
 	}
 
-	// If exclude list is specified, region must not be in it
+	// If exclude list is specified, region must not be in it.
 	if slices.Contains(cfg.ExcludeRegions, region) {
 		return false
 	}
@@ -114,12 +127,12 @@ func shouldIncludeRegion(region string, cfg *Config) bool {
 
 // shouldIncludeInstanceType checks if an instance type should be included based on filters.
 func shouldIncludeInstanceType(instanceType string, cfg *Config) bool {
-	// If include list is specified, instance type must be in it
+	// If include list is specified, instance type must be in it.
 	if len(cfg.IncludeInstanceTypes) > 0 && !slices.Contains(cfg.IncludeInstanceTypes, instanceType) {
 		return false
 	}
 
-	// If exclude list is specified, instance type must not be in it
+	// If exclude list is specified, instance type must not be in it.
 	if slices.Contains(cfg.ExcludeInstanceTypes, instanceType) {
 		return false
 	}
@@ -129,17 +142,17 @@ func shouldIncludeInstanceType(instanceType string, cfg *Config) bool {
 
 // shouldIncludeEngine checks if a recommendation should be included based on engine filters.
 func shouldIncludeEngine(rec *common.Recommendation, cfg *Config) bool {
-	// Extract engine from recommendation
+	// Extract engine from recommendation.
 	engine := getEngineFromRecommendation(*rec)
 	if engine == "" {
-		// If no engine info, include by default unless there's an include list
+		// If no engine info, include by default unless there's an include list.
 		return len(cfg.IncludeEngines) == 0
 	}
 
-	// Normalize engine name to lowercase for comparison
+	// Normalize engine name to lowercase for comparison.
 	engine = strings.ToLower(engine)
 
-	// If include list is specified, engine must be in it
+	// If include list is specified, engine must be in it.
 	if len(cfg.IncludeEngines) > 0 {
 		found := false
 		for _, e := range cfg.IncludeEngines {
@@ -153,7 +166,7 @@ func shouldIncludeEngine(rec *common.Recommendation, cfg *Config) bool {
 		}
 	}
 
-	// If exclude list is specified, engine must not be in it
+	// If exclude list is specified, engine must not be in it.
 	if len(cfg.ExcludeEngines) > 0 {
 		for _, e := range cfg.ExcludeEngines {
 			if strings.EqualFold(e, engine) {
@@ -167,19 +180,19 @@ func shouldIncludeEngine(rec *common.Recommendation, cfg *Config) bool {
 
 // shouldIncludeAccount checks if an account should be included based on filters.
 func shouldIncludeAccount(accountName string, cfg *Config) bool {
-	// If account name is empty and there are filters, skip it (unless include list is empty)
+	// If account name is empty and there are filters, skip it (unless include list is empty).
 	if accountName == "" {
 		return len(cfg.IncludeAccounts) == 0 && len(cfg.ExcludeAccounts) == 0
 	}
 
 	accountLower := strings.ToLower(accountName)
 
-	// Check include list
+	// Check include list.
 	if !checkIncludeList(accountLower, cfg.IncludeAccounts) {
 		return false
 	}
 
-	// Check exclude list
+	// Check exclude list.
 	if checkExcludeList(accountLower, cfg.ExcludeAccounts) {
 		return false
 	}

cmd/multi_service_filters_test.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"bytes"
+	"log"
 	"testing"
 
 	"github.com/LeanerCloud/CUDly/pkg/common"
@@ -413,3 +415,62 @@ func TestShouldIncludeAccount(t *testing.T) {
 		})
 	}
 }
+
+func TestShouldIncludePoolSize(t *testing.T) {
+	tests := []struct {
+		name     string
+		avg      float64
+		minPool  float64
+		expected bool
+	}{
+		{"filter disabled (0)", 0.5, 0, true},
+		{"avg=0 passes through", 0, 2.0, true},
+		{"avg below threshold", 1.5, 2.0, false},
+		{"avg equal to threshold", 2.0, 2.0, true},
+		{"avg above threshold", 3.0, 2.0, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rec := common.Recommendation{AverageInstancesUsedPerHour: tt.avg}
+			cfg := Config{MinPoolSize: tt.minPool}
+			assert.Equal(t, tt.expected, shouldIncludePoolSize(&rec, &cfg))
+		})
+	}
+}
+
+// TestApplyFilters_PoolSizeLogs verifies that applyFilters emits per-rec and
+// summary log lines when --min-pool-size drops recommendations.
+func TestApplyFilters_PoolSizeLogs(t *testing.T) {
+	origCfg := toolCfg
+	defer func() { toolCfg = origCfg }()
+
+	// Capture log output.
+	var buf bytes.Buffer
+	origFlags := log.Flags()
+	origWriter := log.Writer()
+	log.SetOutput(&buf)
+	log.SetFlags(0) // strip timestamps so the assertions are stable
+	defer func() {
+		log.SetOutput(origWriter)
+		log.SetFlags(origFlags)
+	}()
+
+	recs := []common.Recommendation{
+		{Service: "rds", Region: "us-east-1", ResourceType: "db.t3.micro", AverageInstancesUsedPerHour: 0.8, Count: 1},
+		{Service: "rds", Region: "us-east-1", ResourceType: "db.r5.large", AverageInstancesUsedPerHour: 3.5, Count: 2},
+	}
+	cfg := Config{MinPoolSize: 2.0}
+
+	result := applyFilters(recs, &cfg, nil, nil, "")
+
+	// Only the rec above threshold passes through.
+	assert.Len(t, result, 1)
+	assert.Equal(t, "db.r5.large", result[0].ResourceType)
+
+	output := buf.String()
+	// Per-rec line present.
+	assert.Contains(t, output, "--min-pool-size=2.0 dropped rds/us-east-1/db.t3.micro (avg=0.80 < threshold)")
+	// Summary line present.
+	assert.Contains(t, output, "--min-pool-size dropped 1 recommendation(s)")
+}

cmd/validators.go

@@ -82,6 +82,22 @@ func validateNumericRanges(cmd *cobra.Command) error {
 		return fmt.Errorf("override-count (%d) exceeds reasonable limit of %d", toolCfg.OverrideCount, MaxReasonableInstances)
 	}
 
+	if err := validateMinPoolSize(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// validateMinPoolSize checks that --min-pool-size is a non-negative whole number.
+// Instance counts are integers, so a fractional threshold is nonsensical.
+func validateMinPoolSize() error {
+	if toolCfg.MinPoolSize < 0 {
+		return fmt.Errorf("min-pool-size must be 0 (disabled) or a positive whole number, got: %.2f", toolCfg.MinPoolSize)
+	}
+	if toolCfg.MinPoolSize > 0 && toolCfg.MinPoolSize != float64(int64(toolCfg.MinPoolSize)) {
+		return fmt.Errorf("min-pool-size must be a whole number (instance counts are integers), got: %.2f", toolCfg.MinPoolSize)
+	}
 	return nil
 }
 

cmd/validators_test.go

@@ -87,6 +87,52 @@ func TestValidateNumericRanges(t *testing.T) {
 			wantErr: true,
 			errMsg:  "override-count",
 		},
+		{
+			name: "min-pool-size negative",
+			setupFunc: func() {
+				toolCfg.Coverage = 80.0
+				toolCfg.MaxInstances = 0
+				toolCfg.OverrideCount = 0
+				toolCfg.CoverageLookbackDays = 30
+				toolCfg.MinPoolSize = -1.0
+			},
+			wantErr: true,
+			errMsg:  "min-pool-size must be 0 (disabled) or a positive whole number",
+		},
+		{
+			name: "min-pool-size fractional rejected",
+			setupFunc: func() {
+				toolCfg.Coverage = 80.0
+				toolCfg.MaxInstances = 0
+				toolCfg.OverrideCount = 0
+				toolCfg.CoverageLookbackDays = 30
+				toolCfg.MinPoolSize = 1.5
+			},
+			wantErr: true,
+			errMsg:  "min-pool-size must be a whole number",
+		},
+		{
+			name: "min-pool-size disabled (zero)",
+			setupFunc: func() {
+				toolCfg.Coverage = 80.0
+				toolCfg.MaxInstances = 0
+				toolCfg.OverrideCount = 0
+				toolCfg.CoverageLookbackDays = 30
+				toolCfg.MinPoolSize = 0
+			},
+			wantErr: false,
+		},
+		{
+			name: "min-pool-size whole number accepted",
+			setupFunc: func() {
+				toolCfg.Coverage = 80.0
+				toolCfg.MaxInstances = 0
+				toolCfg.OverrideCount = 0
+				toolCfg.CoverageLookbackDays = 30
+				toolCfg.MinPoolSize = 2.0
+			},
+			wantErr: false,
+		},
 	}
 
 	for _, tt := range tests {