You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(cli): --input-csv silently ignores --min-savings-pct and --max-break-even-months #1819
--min-savings-pct and --max-break-even-months are accepted on --input-csv runs and have no effect. Nothing tells the operator they were ignored. This is the same silence defect as #1741, in the same function, on the two threshold flags #1741 did not cover.
#1741 routes the CSV path through scorer.Score so --min-count and savings-ordered capping behave the same on both paths. It deliberately passes onlyMinCount:
Passing the other two would be worse than passing nothing. The CSV format carries neither figure: writeMultiServiceCSVReport emits no savings-percentage and no break-even column, and parseCSVRecord reads neither, so SavingsPercentage and BreakEvenMonths both load as 0 on every row. scorer.filterReason gates with rec.SavingsPercentage < cfg.MinSavingsPct, so a non-zero --min-savings-pct against a uniformly-zero field rejects every row of every file. A silent no-op would become a silent empty run.
Note the ordering consequence, already relied on by #1741: because SavingsPercentage is uniformly zero on loaded rows, scorer.Score's sort resolves on its secondary key, EstimatedSavings descending. That is the savings signal a CSV does carry, and it is what makes savings-ordered capping meaningful on this path. Any fix that starts populating SavingsPercentage must keep that ordering correct rather than silently re-ranking on a newly-populated field.
The two ends
Same shape as #1741's framing, and the choice should be made before implementing:
Carry the data. Add savings-percentage and break-even columns to writeMultiServiceCSVReport, read them in parseCSVRecord, and hand all three thresholds to the scorer. Needs a decision on rows from older reports that lack the columns: a missing figure must be represented as absent, not coerced to 0, or the gates read "0% savings" and reject rows they were never meant to judge.
Refuse the flags. If CSV mode is not meant to re-litigate quality thresholds, error out when either flag is combined with --input-csv, rather than accepting and ignoring it.
Do not split the difference, and do not fix the silence with different silence.
Scope note
docs/cli/filtering.md documents both flags as flags of the tool, not of a mode, exactly as it does for the two flags #1741 covers. Whichever end is chosen, that page needs to match.
filterAndAdjustRecommendations sits under the gocyclo ceiling tracked in LeanerCloud/cloud-commitments-platform#163 (it is at 8 after #1741's extraction), so there is some headroom, but not much.
Owner decision (2026-10-16): carry the data. Add savings-percentage and break-even columns to the multi-service CSV writer, read them in the parser, represent a missing column in older reports as absent (not 0), and pass all three thresholds to the scorer while keeping the EstimatedSavings ordering intact. Queued by [claude-cli-impl] behind the open CSV-writer PR #2203 to avoid file overlap.
Taking over: #2203 has merged and the queued worker [claude-cli-impl] is no longer running; implementing the owner decision (carry the data) [claude-cc-libs]
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…
cristim commentedon Oct 16, 2026
Owner decision (2026-10-16): carry the data. Add savings-percentage and break-even columns to the multi-service CSV writer, read them in the parser, represent a missing column in older reports as absent (not 0), and pass all three thresholds to the scorer while keeping the EstimatedSavings ordering intact. Queued by [claude-cli-impl] behind the open CSV-writer PR #2203 to avoid file overlap.
IAmJSD commentedon Oct 16, 2026
Released: already queued by [claude-cli-impl] behind #2203 (see owner decision above)
IAmJSD commentedon Oct 16, 2026
Released: I missed that your owner decision already has this queued for [claude-cli-impl], behind #2203. Leaving it to that worker.
cristim commentedon Oct 19, 2026
Taking over: #2203 has merged and the queued worker [claude-cli-impl] is no longer running; implementing the owner decision (carry the data) [claude-cc-libs]
fix(cli): apply --min-savings-pct and --max-break-even-months on --in…