fix(csv): honor an explicit --coverage 80 on --input-csv runs

cmd/csv_coverage_flag_test.go

@@ -0,0 +1,89 @@
+package main
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	csvCoverageArgsEnv = "CUDLY_CSV_COVERAGE_TEST_ARGS"
+	csvCoverageWantEnv = "CUDLY_CSV_COVERAGE_TEST_WANT"
+)
+
+// TestCSVCoverageFlagThroughRootCommand reproduces #2236 through the real
+// command: cobra flag parsing, validateFlags, runTool and the --input-csv
+// dry run. Before the fix an explicit --coverage 80 equaled the flag's default,
+// so determineCSVCoverage treated it as unset and every row was bought at its
+// full Count. Each case runs in a child process so the flag state cobra keeps
+// on rootCmd (values and Changed bits) never leaks between cases or tests.
+func TestCSVCoverageFlagThroughRootCommand(t *testing.T) {
+	if args, ok := os.LookupEnv(csvCoverageArgsEnv); ok {
+		runCSVCoverageChild(t, args, os.Getenv(csvCoverageWantEnv))
+		return
+	}
+	tests := []struct {
+		name string
+		args []string
+		want string
+	}{
+		{"no flag buys each row's full count", nil, "10,20"},
+		{"explicit 80 is honored", []string{"--coverage", "80"}, "8,16"},
+		{"explicit 80 via shorthand is honored", []string{"-c", "80"}, "8,16"},
+		{"explicit 80 with equals is honored", []string{"--coverage=80"}, "8,16"},
+		{"explicit 100 buys each row's full count", []string{"--coverage", "100"}, "10,20"},
+		{"explicit 50 halves each row", []string{"--coverage", "50"}, "5,10"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+			child := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestCSVCoverageFlagThroughRootCommand$", "-test.v")
+			child.Env = []string{
+				csvCoverageArgsEnv + "=" + strings.Join(tt.args, "\x1f"),
+				csvCoverageWantEnv + "=" + tt.want,
+			}
+			output, err := child.CombinedOutput()
+			require.NoError(t, err, "%s", output)
+			assert.Contains(t, string(output), "--- PASS: TestCSVCoverageFlagThroughRootCommand")
+		})
+	}
+}
+
+// runCSVCoverageChild executes rootCmd with the case's flags on a dry-run
+// --input-csv invocation and checks the per-row purchase counts in the report.
+func runCSVCoverageChild(t *testing.T, joinedArgs, want string) {
+	awsHits := isolateAWSEnv(t)
+	dir := t.TempDir()
+	csvPath := writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Count,Term,PaymentOption,Account,EstimatedSavings
+rds,us-east-1,db.t3.small,postgres,10,1yr,all-upfront,111111111111,100
+rds,us-east-1,db.t3.medium,postgres,20,1yr,all-upfront,111111111111,200
+`)
+	reportPath := filepath.Join(dir, "report.csv")
+	args := []string{"--input-csv", csvPath, "--output", reportPath, "--audit-log", filepath.Join(dir, "audit.jsonl")}
+	if joinedArgs != "" {
+		args = append(args, strings.Split(joinedArgs, "\x1f")...)
+	}
+	rootCmd.SetArgs(args)
+	require.NoError(t, rootCmd.Execute())
+
+	header, rows := readPurchaseReport(t, reportPath)
+	byType := map[string]string{}
+	types := reportColumn(t, header, rows, "ResourceType")
+	for i, count := range reportColumn(t, header, rows, "Count") {
+		byType[types[i]] = count
+	}
+	wantCounts := strings.Split(want, ",")
+	assert.Equal(t, map[string]string{"db.t3.small": wantCounts[0], "db.t3.medium": wantCounts[1]}, byType)
+	for _, dryRun := range reportColumn(t, header, rows, "Success") {
+		assert.Equal(t, "true", dryRun)
+	}
+	assert.Positive(t, awsHits.Load(), "the local AWS endpoint must have served the duplicate checks")
+}

cmd/main.go

@@ -77,6 +77,12 @@ type Config struct {
 	IncludeExtendedSupport bool
 	AllServices            bool
 	ActualPurchase         bool
+	// CoverageFromDefault is true when --coverage was not passed on the
+	// command line, so Coverage holds the flag's default. runTool sets it
+	// from cobra's Changed(); --input-csv runs use it to buy each row's full
+	// Count when no coverage was asked for (determineCSVCoverage). A Config
+	// built in code leaves it false, so its Coverage is always honored.
+	CoverageFromDefault bool
 	// RecLookbackPeriod controls the LookbackPeriodInDays passed to
 	// GetReservationPurchaseRecommendation. Valid values: "7d", "30d", "60d"
 	// (recommendations.DefaultRecLookbackPeriod is the shared default).
@@ -391,8 +397,11 @@ func runTool(cmd *cobra.Command, args []string) {
 	ctx, stop := invocationContext()
 	defer stop()
 
+	cfg := toolCfg
+	cfg.CoverageFromDefault = cmd == nil || !cmd.Flags().Changed("coverage")
+
 	// Always use the multi-service implementation
-	runToolMultiService(ctx, toolCfg)
+	runToolMultiService(ctx, cfg)
 }
 
 // invocationContext returns the context for one CLI invocation, canceled by

cmd/multi_service_csv.go

@@ -20,11 +20,12 @@ import (
 )
 
 // determineCSVCoverage determines the coverage percentage to use for CSV mode.
+// Without --coverage on the command line a CSV run buys each row's full Count
+// (100%). An explicit --coverage is always honored, including a value equal to
+// the flag's default (80): the check uses CoverageFromDefault, which runTool
+// sets from cobra's Changed(), never a comparison against the default value.
 func determineCSVCoverage(cfg Config) float64 {
-	// When using CSV input, default to 100% coverage (use exact numbers from CSV)
-	// unless user explicitly provided a different coverage value
-	if cfg.Coverage == 80.0 {
-		// User didn't override the default, so use 100% for CSV mode
+	if cfg.CoverageFromDefault {
 		return 100.0
 	}
 	return cfg.Coverage

cmd/multi_service_csv_test.go

@@ -21,12 +21,21 @@ func TestDetermineCSVCoverage(t *testing.T) {
 		expected float64
 	}{
 		{
-			name: "Default coverage (80) changed to 100 for CSV",
+			name: "No --coverage flag buys 100% for CSV",
 			cfg: Config{
-				Coverage: 80.0,
+				Coverage:            80.0,
+				CoverageFromDefault: true,
 			},
 			expected: 100.0,
 		},
+		{
+			// #2236: an explicit value equal to the flag's default is honored.
+			name: "Explicit 80% coverage preserved",
+			cfg: Config{
+				Coverage: 80.0,
+			},
+			expected: 80.0,
+		},
 		{
 			name: "User-specified coverage preserved",
 			cfg: Config{

docs/cli/README.md

@@ -30,7 +30,7 @@ All flags belong to the root command unless noted otherwise.
 
 | Flag | Short | Default | Description |
 |------|-------|---------|-------------|
-| `--coverage` | `-c` | `80` | Percentage (0-100) of each recommendation's instance count to purchase (`rec.Count * coverage/100`). Ignored when `--target-coverage` is also set. |
+| `--coverage` | `-c` | `80` | Percentage (0-100) of each recommendation's instance count to purchase (`rec.Count * coverage/100`). Ignored when `--target-coverage` is also set. With `--input-csv`, omitting the flag buys each row's full `Count` (100%); any explicitly passed value, including `80`, is applied. |
 | `--target-coverage` | `-u` | `0` (disabled) | Target percentage (0-100) of historical average hourly usage to cover with commitments. Sizes each recommendation to `floor(avg * target/100)`, leaving the remainder on-demand. Overrides `--coverage` when non-zero. Pairs with `--rebuy-window-days` and `--min-pool-size` (see [filtering.md](filtering.md)). |
 | `--coverage-lookback-days` | | `30` | Calendar days of historical demand fed to `GetReservationCoverage` when computing the existing-RI coverage map for `--target-coverage` sizing. Match this to your AWS console coverage report window to reconcile cudly's `ExistingCoverage` column against the console export. Only affects `--target-coverage`. |
 | `--override-count` | | `0` (disabled) | Replace every recommendation's count with this fixed number, rescaling that row's costs and savings by the same ratio so they describe the quantity that will actually be bought. Savings Plans are exempt and pass through unchanged: an SP commitment is priced in dollars per hour, not in instances. Rows with a non-positive count are exempt too, having no quantity to rescale from. Overriding above the quantity the provider's figures cover keeps the costs accurate but extrapolates the savings past the observed demand; all three cases are named on stdout. Applied before `--max-instances`. Useful when testing a specific purchase size. |