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

[IAmJSD]

Root cause

determineCSVCoverage (cmd/multi_service_csv.go) decided whether the user had asked for a coverage by comparing cfg.Coverage with the flag's default, 80.0. An explicit --coverage 80 matched that value, so it was treated as unset: the function returned 100, filterAndAdjustRecommendations skipped sizing (csvModeCoverage < 100 was false), and the run bought 100% of every CSV row's Count.

Fix

• runTool (cmd/main.go) copies toolCfg and sets a new field, Config.CoverageFromDefault, to !cmd.Flags().Changed("coverage"). This is the same Changed() check validateTargetCoverage in cmd/validators.go already uses.
• determineCSVCoverage returns 100 only when CoverageFromDefault is set. Otherwise it returns cfg.Coverage.
• The field's zero value means "explicit". A Config built in code (all callers outside runTool are tests) has its Coverage honored as given. If a future entry point forgets to set the field, the run sizes to the given coverage and does not silently buy 100%.
• docs/cli/README.md: the --coverage row now documents the CSV default.

There is no config file or environment variable for coverage. No viper is used, and the only os.Getenv calls are for Archera and DISABLE_PURCHASE_DELAY. So the precedence stays as before: an explicit flag wins, and with no flag a CSV run buys 100% and a non-CSV run uses the flag default of 80. determineCSVCoverage was the only code that relied on the 80 sentinel (grep 'Coverage == 80'). The non-CSV path never reads CoverageFromDefault.

Behavior matrix (--input-csv, dry run, rows with Count 10 and 20)

Invocation	Before	After
no --coverage	10, 20 (100%)	10, 20 (100%), unchanged
--coverage 80 / -c 80 / --coverage=80	10, 20 (100%)	8, 16 (80%)
--coverage 100	10, 20	10, 20
--coverage 50	5, 10	5, 10
--target-coverage N	target sizing	target sizing, unchanged
non-CSV runs	cfg.Coverage	cfg.Coverage, unchanged

Closes #2236

Verification

All of this ran locally on macOS against the worktree at 7579572b, with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null. No real AWS calls were made and nothing was purchased.

• New TestCSVCoverageFlagThroughRootCommand (cmd/csv_coverage_flag_test.go). Each case runs rootCmd.Execute() in a child test process, covering cobra flag parsing, validateFlags, runTool, runToolMultiService and the --input-csv dry run. It uses isolateAWSEnv's local 403 endpoint and asserts the per-row Count values in the written purchase report. Cases: no flag, --coverage 80, -c 80, --coverage=80, 100 and 50. A child process is used so the flag state cobra keeps on rootCmd cannot leak between cases.
• TestDetermineCSVCoverage now has two cases: defaulted 80 returns 100, and explicit 80 returns 80.
• Fail-before / mutation probes. Each mutant was applied in turn, then reverted:
  • Original Coverage == 80.0 sentinel: the three explicit-80 cases fail, plus the unit case. This is the fail-before.
  • runTool never sets the field: the no-flag case fails.
  • Inverted Changed(): the no-flag, explicit-80 and explicit-50 cases fail.
  • CSV 100% default removed: the no-flag case and the unit case fail.
• go build ./cmd, go vet ./... and go test ./cmd/ -count=1 all pass.
• golangci-lint v2.10.1 run --allow-parallel-runners ./... reports 0 issues.
• gocyclo -over 10 on cmd/main.go and cmd/multi_service_csv.go: clean.
• scripts/gofmt-hook.sh, make tidy-check and the trailing-whitespace/EOF checks on the changed files: clean.
• git-secrets --scan on a temp copy of the tree with an isolated HOME and --register-aws: clean.
• Not run locally: markdownlint and gosec (not installed here) and the repo-wide go test -short -race ./... (skipped for disk space). CI covers these.

Out of scope

• On a CSV run with no --coverage, the dry-run commitment IDs still carry the 80pct label (effectiveSizingPct reads cfg.Coverage), even though 100% is bought. This label-only issue predates this PR.
• This PR is coordinated with the open fix(cli): only print "CSV report written to" after a complete CSV write #2203, which touches cmd/multi_service.go, cmd/multi_service_csv.go and cmd/multi_service_csv_test.go. This PR does not change cmd/multi_service.go. The overlap in multi_service_csv.go is limited to determineCSVCoverage and one TestDetermineCSVCoverage case, so whichever lands second should rebase trivially.

Labels

Please mirror the labels from #2236 onto this PR: triaged, priority/p1, severity/high, urgency/this-sprint, impact/few, effort/s, type/bug. This PR was opened from a fork, so they were not applied here.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • CSV imports now use each row’s full count when --coverage is omitted. Explicit coverage values—including 80—are honored.
• Documentation
  • Clarified how --coverage affects CSV imports when omitted or explicitly set.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 690d232e-2a13-4339-bec7-4d1f84aeb04d

📥 Commits

Reviewing files that changed from the base of the PR and between b514b37 and 7579572.

📒 Files selected for processing: 5

• cmd/csv_coverage_flag_test.go
• cmd/main.go
• cmd/multi_service_csv.go
• cmd/multi_service_csv_test.go
• docs/cli/README.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

CSV runs now distinguish omitted --coverage from an explicitly supplied value. Omitted coverage uses 100%; explicit values, including 80%, are preserved. Unit and root-command tests cover the behavior.

Changes: CSV Coverage Flag Handling

Layer / File(s)	Summary
Track and resolve coverage defaults cmd/main.go, cmd/multi_service_csv.go, cmd/multi_service_csv_test.go, docs/cli/README.md	Config records whether coverage was omitted. CSV coverage resolution uses that state to choose 100% for the default and preserve explicit values. Unit tests and CLI documentation cover the distinction.
Verify coverage through the root command cmd/csv_coverage_flag_test.go	An integration test runs the root command in child processes and checks dry-run purchase counts for omitted coverage and explicit values of 80%, 100%, and 50%.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: cristim

Merge Risk: ⚪ Minimal · up to 75795

No actionable issue remains identified for this change. It is ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving an explicitly supplied --coverage 80 for --input-csv runs.
Linked Issues check		Issue #2236 requires explicit coverage detection with Cobra's Changed("coverage") and a dry-run regression test for explicit 80%. The diff shows runTool setting Config.CoverageFromDefault from cmd.Fla…
Out of Scope Changes check		The changed files are connected to #2236. They implement coverage-state propagation, CSV coverage resolution, focused unit and integration tests, and CLI documentation. No unrelated production behavio…

Full details: Linked Issues check

Explanation

Issue #2236 requires explicit coverage detection with Cobra's Changed("coverage") and a dry-run regression test for explicit 80%. The diff shows runTool setting Config.CoverageFromDefault from cmd.Flags().Changed("coverage"), determineCSVCoverage honoring explicit 80, and the root-command test verifying 8 and 16 counts for --coverage 80, -c 80, and --coverage=80. The test also preserves 100% sizing when the flag is omitted.

Full details: Out of Scope Changes check

Explanation

The changed files are connected to #2236. They implement coverage-state propagation, CSV coverage resolution, focused unit and integration tests, and CLI documentation. No unrelated production behavior or unrelated files appear in the reviewed diff.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review verdict: CLEAN at 7579572

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

--input-csv invocation	Sizing
no flag	100% (unchanged)
--coverage 80, -c 80, --coverage=80	80% (the fix)
--coverage 100	100%
--coverage 50	50%
0, negative, >100	rejected by validateCoverageAndSavingsPct (cmd/validators.go:151-157); traced, not run

Detection:

• cmd/main.go:401 uses cmd.Flags().Changed("coverage") on the root flag set (:116), the same pattern as validators.go:198.
• No viper, config-file or env path sets coverage.
• A nil cmd counts as default, and a hand-built Config fails toward sizing, not toward buying 100%.

Same sizing as the non-CSV path:

• Rounding: both paths call applyCoverage → recfilter.ApplyCoverage (pkg/recfilter/sizing.go:28), so rounding is identical. Coverage 0 drops every row.
• Order unchanged: coverage, then override-count, then scoreAndLimitCSVRecs (min-count, max-instances) (multi_service.go:931-944), then the replay dedupe. Coverage is applied once.
• Non-CSV path untouched: determineCSVCoverage is the only reader of the new field.

Notes (non-blocking):

1. docs/cli/README.md could warn that an explicit flag is applied on top of a CSV that was already coverage-adjusted when exported.
2. Already present before this PR: purchase-ID labels say 80pct on a no-flag CSV run that buys 100% (main.go:289). Filed as fix(csv): purchase IDs say 80pct on --input-csv runs that buy 100% #2248.

What was attacked and how (AWS_CONFIG_FILE=/dev/null, no purchases):

• go vet ./cmd/... is clean, and go test -race -count=1 ./cmd/... passes 1550.
• TestCSVCoverageFlagThroughRootCommand (6 subtests, run through the real root command) and TestDetermineCSVCoverage pass.
• Mutation probes, all killed:
  • The old Coverage==80 logic fails the 3 explicit-80 subtests (this is the fail-before).
  • Always applying coverage fails the no-flag case.
  • Never applying it fails the explicit-80 cases.

Gate:

• Labels are mirrored from fix(csv): explicit --coverage 80 is treated as unset on --input-csv runs and buys 100% #2236 (p1, severity/high).
• Needs the author to mark the draft ready, then green CI and the CodeRabbit gate.

[cristim]

Merge gate met at 7579572b5b2bd61317e02bee3ff7e116283ff3e5

• Independent adversarial review: CLEAN at this SHA (verdict above, non-author cold-brief reviewer). Fail-before is shown by mutation probes: the old Coverage==80 logic fails the 3 explicit-80 subtests.
• CI: green at this SHA.
• CodeRabbit: no actionable comments, and there are no inline threads.
• Labels: mirrored from fix(csv): explicit --coverage 80 is treated as unset on --input-csv runs and buys 100% #2236.
• Re-verified on current main: main has moved 10 commits since CI ran, some touching cmd/main.go and cmd/multi_service_csv.go, so I re-ran the checks locally on macOS against b0eeb466 merged with this PR. Auto-merge is clean.
  • go build ./cmd, go vet ./... and go test ./... -count=1 all pass.
  • TestCSVCoverageFlagThroughRootCommand and TestDetermineCSVCoverage pass.
  • This is test-based evidence through the real root command. No real purchase was made and --yes was not used.

Squash-merging with --match-head-commit 7579572b.