Skip to content

fix(cli): --input-csv silently ignores --min-savings-pct and --max-break-even-months #1819

Open

Description

@cristim

--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.

Why #1741's fix does not reach them

#1741 routes the CSV path through scorer.Score so --min-count and savings-ordered capping behave the same on both paths. It deliberately passes only MinCount:

scored := scorer.Score(recs, scorer.Config{MinCount: cfg.MinCount})

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.

Related

Activity

  1. cristim commented on Oct 16, 2026

    @cristim
    MemberAuthor

    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.

  2. IAmJSD commented on Oct 16, 2026

    @IAmJSD
    Collaborator

    Released: already queued by [claude-cli-impl] behind #2203 (see owner decision above)

  3. IAmJSD commented on Oct 16, 2026

    @IAmJSD
    Collaborator

    Released: I missed that your owner decision already has this queued for [claude-cli-impl], behind #2203. Leaving it to that worker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions