Repository navigation
fix(cli): --input-csv enforces neither --min-count nor savings-ordered capping, silently #1741
Description
The --input-csv purchase path enforces neither --min-count nor savings-ordered selection. Both are enforced on the default path.
Found while reviewing PR #1725, which cites the CSV path as the reference for correct global cap semantics. That citation is wrong in a way worth correcting: the CSV path is global, but it is global in load order.
Two distinct defects
1. --min-count is silently unenforced in CSV mode.
There is no scorer and no --min-count gate anywhere on the CSV path. Confirmed by exhaustive grep, not inference: scoreLimitAndDisplay is called only at cmd/multi_service.go:140 (the default path), and cfg.MinCount appears only at :217 and in the new floor-drop code #1725 added to the default path.
Consequence: a recommendation truncated by --max-instances to below the --min-count floor is purchased short in CSV mode. On the default path #1725 now drops it. An operator setting --min-count 5 and getting a 2-instance commitment is the exact defect #1608/#1725 fixed for the default path, still live here.
2. The CSV cap selects in load order, not savings order.
filterAndAdjustRecommendations applies ApplyInstanceLimit at cmd/multi_service.go:610, before any scoring. ApplyInstanceLimit consumes its input in slice order and drops the tail, so whichever rows appear first in the file consume the budget.
That is precisely the selection defect PR #1725 established as the important correctness property on the default path — where the cap now runs after scorer.Score so the survivors are the highest-savings recommendations run-wide. In CSV mode the survivors are whatever the file happened to list first.
Why this is not simply "the CSV path is different"
--max-instances and --min-count are documented in docs/cli/filtering.md as flags of the tool, not of a mode. An operator who reads that page and passes --input-csv gets neither behaviour, with nothing indicating the flags were ignored. Silent partial enforcement of a spend guard is worse than not offering it.
Suggested approach
Decide first, then implement — the two ends are genuinely different products:
- If CSV is meant to be "purchase exactly what I listed", then
--min-countand savings-ordered capping arguably should not apply at all — but the tool must then refuse those flags in CSV mode with a clear error rather than accepting and ignoring them. - If CSV is meant to be "the same pipeline, different input", route it through the same scoring and floor gates the default path uses, and the cap moves after scoring for the same reason it did in fix(cli): apply --max-instances once run-wide, not per service and region #1725.
Do not split the difference by applying one and not the other.
Tests must cover the real scenario either way: a CSV whose best-value rows are not first, a cap that binds, and a --min-count that a truncation would breach. A test asserting only the total passes today with both defects present — see #1725, where exactly that gap let a pre-scoring cap look correct.
Related
- fix(cli): apply --max-instances once run-wide, not per service and region #1725 / fix(cli): --max-instances is applied per service and region, not as the documented global total cap #1608 — the same two properties, fixed on the default path.
- fix(cli): the --input-csv purchase path writes no audit records and skips the writability pre-check #1609 — the CSV path writes no audit record.
- fix(cli): CSV-mode purchase confirmation is per-region and declining does not abort the run #1610 — CSV-mode confirmation is per-region and a decline does not abort.
Note #1609, #1610 and this issue all live in filterAndAdjustRecommendations / runToolFromCSV, and #1719 records that both of those functions sit at exactly the gocyclo ceiling of 10. Whoever takes any of the three inherits that constraint.
Activity
fix(cli): enforce --min-count and savings-ordered capping on --input-csv
fix(cli): rank --input-csv rows by savings per instance, refuse unran…
fix(cli): enforce --min-count and savings-ordered capping on --input-…
fix(cli): enforce --min-count and savings-ordered capping on --input-…