iframe-proxy

cristim · 2026-10-19T17:21:35Z

What

This implements the owner decision on #1819 (2026-10-16): carry the data.

On main, --input-csv runs refuse --min-savings-pct and --max-break-even-months (validateCSVModeFilterFlags) because the CSV has no columns for either figure. This PR makes both flags work on CSV runs instead:

Writer: the multi-service CSV gets two new trailing columns, after the detail columns, written as exact decimals:
- SavingsPercentage, in percent like the flag.
- BreakEvenMonths, in months. It is blank when unknown or infinite, and 0 means no upfront payment.
Loader: it reads both columns, and --input-csv passes MinSavingsPct and MaxBreakEvenMonths to scorer.Score along with MinCount. validateCSVModeFilterFlags and its test are removed.
Older CSVs without the columns:
- With the flag unset, they load exactly as before.
- With the flag set, a missing column or a blank cell fails the run, and the error names the flag, the column and the CSV line.
- A missing figure is never read as 0, and rows are never silently passed or dropped. This follows the decision's "absent, not 0", since common.Recommendation holds both as plain float64 in the pinned library.
--max-instances still ranks on savings per instance. A test confirms that a populated SavingsPercentage does not re-rank it. The --max-instances message and docs/cli/filtering.md no longer say a CSV row carries no savings percentage.

Commits (each <= 400 additions+deletions)

fix(cli): apply --min-savings-pct and --max-break-even-months on --input-csv (268)
test(cli): cover CSV threshold columns and flags, document them (185)

Local verification (macOS, this HEAD)

go build -o cudly ./cmd, go vet ./..., go test ./..., golangci-lint v2.10.1 (0 issues) and scripts/test-git-secrets-allowlist.sh (44/44) all pass.
The root-command test runs the real rootCmd.Execute() in child processes. 6 of its 8 cases fail on main's code and pass after; the other two do not use the flags. The round-trip, header and --max-instances cap tests cover the new columns.
Mutation probe: dropping MinSavingsPct from the scorer config fails the min-savings and combined cases.

Known limitation

A row whose break-even is infinite is written with a blank BreakEvenMonths, so replaying that file with --max-break-even-months errors instead of dropping the row. The parser already rejects negative savings, so a file this tool writes cannot produce such a row on re-read.

Independent reviews follow as PR comments.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- CSV input now supports the --min-savings-pct and --max-break-even-months filters. Reports include savings-percentage and break-even columns.
- Filtered rows are identified in the output, and CSV instance limits continue to rank recommendations by savings per instance.
Bug Fixes
- CSV runs now apply savings and break-even thresholds instead of rejecting those options. Missing required columns or blank values produce an error that identifies the relevant flag.

…put-csv The CSV report now carries SavingsPercentage (percent) and BreakEvenMonths (months) columns, appended after the detail columns and written as exact decimals. The loader reads them and hands both thresholds to scorer.Score, so the CSV path rejects on the same predicates as the API path. A column a set flag needs that is missing from the header, or a blank cell, fails the run naming the flag instead of reading as 0. --max-instances still ranks on savings per instance. The up-front refusal of both flags is removed. Refs #1819 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Drives --min-savings-pct and --max-break-even-months through the real root command on --input-csv, covers older and blank-cell CSVs, the writer/parser round trip, strict header validation and cap ordering. Updates docs/cli/filtering.md and the changelog. Refs #1819 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristim · 2026-10-19T17:40:14Z

cristim · 2026-10-19T17:40:52Z

Independent review A (claude-cc-libs-reviewA-2274, Sonnet, cold brief, not the author): money path

Reviewed SHA: d6aaac9
Verdict: REQUEST_CHANGES on one medium finding.

Local verification (scratch worktree): build, go vet ./..., go test ./... and golangci-lint (0 issues) all pass.

What was attacked:

Units: scorer.go:88-89 compares SavingsPercentage < MinSavingsPct. The AWS parser sets it as a percent (parser_sp.go:200,262), the flag is validated 0-100 (validators.go:158), and the writer emits the raw percent (multi_service_csv_thresholds.go:44). Break-even uses common.BreakEven (breakeven.go:22) in months on both sides. There is no unit mismatch.
Boundaries: the CSV path calls scorer.Score itself (multi_service_csv_cap.go:67-81), so its gates are identical to the API path's.
Decimals: 1e1, -0 and +5 parse correctly, while Inf, NaN, 5,5, 0x10 and 10% are rejected with the column named. The 'f', -1 writer round-trips exactly.
Absent, not 0: a missing column or a blank cell fails with the flag, column and line named (:55-62, :79-86, and multi_service_csv.go:163). With the flags unset, old CSVs are unchanged.
Removed function: validateCSVModeFilterFlags has no remaining references.
Tests: they drive rootCmd.Execute().
Purchase runs use the same path as dry runs (multi_service.go:708). The behaviour change is disclosed.

Mutation probes: 7 of 7 killed. They dropped MaxBreakEven, wrote the percent as a fraction, skipped the column check, made a blank cell optional, wrote 2 decimal places, disabled the break-even requirement, and skipped the whole gate.

Findings:

cmd/multi_service_csv_thresholds.go:83 | medium | A whitespace-only cell is not treated as blank. getCSVField(...)=="" does not trim, while parseCSVFloat trims and returns nil, so with the flag set the value silently stays 0. Fix: compare strings.TrimSpace(...) == "" and add a test with a whitespace-only cell.
cmd/multi_service_csv.go:202 | low | The threshold columns are now parsed whenever present, even with the flags unset. A pre-existing column with garbage in it now fails. Fix: document it in the CHANGELOG.

Fixes follow in a new commit on this PR, together with review B's two nits.

coderabbitai · 2026-10-19T17:43:19Z

Warning

Review limit reached

Run on-demand review

_{This review includes 11 billable files and costs up to $2.75.}

Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 35 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 88 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: dbe203b6-5ddf-41ef-8801-cbd28dbf7e62

📥 Commits

Reviewing files that changed from the base of the PR and between c43c96d and 084116d.

📒 Files selected for processing: 11

CHANGELOG.md
cmd/multi_service.go
cmd/multi_service_csv.go
cmd/multi_service_csv_cap.go
cmd/multi_service_csv_thresholds.go
cmd/multi_service_csv_thresholds_test.go
cmd/multi_service_max_instances_test.go
cmd/savings_plan_purchase_test.go
cmd/validators.go
cmd/validators_test.go
docs/cli/filtering.md

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: Essentials
Run ID: 6024d091-2ada-4d05-8729-b1c226a4c052

📥 Commits

Reviewing files that changed from the base of the PR and between 83a7193 and c43c96d.

📒 Files selected for processing: 11

CHANGELOG.md
cmd/multi_service.go
cmd/multi_service_csv.go
cmd/multi_service_csv_cap.go
cmd/multi_service_csv_thresholds.go
cmd/multi_service_csv_thresholds_test.go
cmd/multi_service_max_instances_test.go
cmd/savings_plan_purchase_test.go
cmd/validators.go
cmd/validators_test.go
docs/cli/filtering.md

💤 Files with no reviewable changes: 2

cmd/validators.go
cmd/validators_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

CSV reports now include savings-percentage and break-even columns. CSV replay validates and parses those values, then applies configured thresholds. CSV instance caps continue to rank recommendations by savings per instance.

Changes: CSV Threshold Filtering

Layer / File(s)	Summary
CSV threshold fields and report output `cmd/multi_service_csv_thresholds.go`, `cmd/multi_service_csv.go`, `cmd/multi_service_csv_thresholds_test.go`, `cmd/savings_plan_purchase_test.go`	CSV loading validates required threshold headers and parses threshold values. Report output appends both threshold columns after detail columns. Unknown or infinite break-even values are written as blank cells. Tests cover parsing, output, and blank values.
Replay filtering and command integration `cmd/multi_service.go`, `cmd/multi_service_csv_cap.go`, `cmd/multi_service_csv_thresholds_test.go`, `cmd/multi_service_max_instances_test.go`, `cmd/validators.go`, `cmd/validators_test.go`, `docs/cli/filtering.md`, `CHANGELOG.md`	CSV replay passes configured threshold requirements to loading and applies the active threshold filters. CSV-mode rejection of these flags is removed. Maximum-instance ranking remains based on savings per instance. Tests and documentation cover the updated behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant runToolFromCSV
  participant loadCSVRecommendations
  participant parseCSVRecord
  participant scoreAndLimitCSVRecs
  participant scorer.Score
  runToolFromCSV->>loadCSVRecommendations: Pass configured threshold requirements
  loadCSVRecommendations->>parseCSVRecord: Parse records with threshold requirements
  parseCSVRecord-->>loadCSVRecommendations: Return parsed threshold values
  loadCSVRecommendations-->>runToolFromCSV: Return recommendations
  runToolFromCSV->>scoreAndLimitCSVRecs: Pass recommendations and configuration
  scoreAndLimitCSVRecs->>scorer.Score: Apply configured threshold gates
  scorer.Score-->>scoreAndLimitCSVRecs: Return scored recommendations

Merge Risk: ⚪ Minimal · up to c43c9

CSV threshold filtering is ready to merge after normal checks; no actionable merge-blocking risk was established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 7 files. (2 skipped: …	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: applying both threshold flags to --input-csv runs.
Linked Issues check		Issue `#1819` requires either refusal of the flags or data carriage through CSV. This PR chooses data carriage. The writer appends `SavingsPercentage` and `BreakEvenMonths`. The loader reads them and ap…
Out of Scope Changes check		The changes stay within issue `#1819`. Source changes implement CSV threshold serialization, loading, validation, filtering, and cap ordering. Tests verify the new behavior. Documentation and changelog …

Full details: Linked Issues check

Explanation

Issue #1819 requires either refusal of the flags or data carriage through CSV. This PR chooses data carriage. The writer appends SavingsPercentage and BreakEvenMonths. The loader reads them and applies MinCount, MinSavingsPct, and MaxBreakEvenMonths on CSV runs. Missing required columns and blank required cells fail instead of becoming zero. Tests cover both flags, older CSVs, whitespace-only cells, round trips, and combined filters. The CSV cap test preserves savings-per-instance ranking when SavingsPercentage is populated. The filtering documentation matches this behavior.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #1819. Source changes implement CSV threshold serialization, loading, validation, filtering, and cap ordering. Tests verify the new behavior. Documentation and changelog updates describe the selected CSV behavior. No unrelated functional change is established by the available evidence.

Full details: Docstring Coverage

Explanation

Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 7 files. (2 skipped: 2 unsupported.)

Resolution

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

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
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.}

The blank-cell check did not trim while parseCSVFloat does, so a " " or tab cell with --min-savings-pct or --max-break-even-months set loaded silently as 0. Trim in the check so it fails loudly like an empty cell. Also note the strict column parsing in the changelog, rewrap a doc comment and fix a stale test assertion message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristim · 2026-10-19T17:54:01Z

Review A re-review (claude-cc-libs-reviewA-2274): delta `d6aaac9`..`c43c96d`

Reviewed SHA: c43c96d
Verdict: APPROVE

Local verification at this SHA: go test ./... passes and golangci-lint reports 0 issues.

Probe: with both threshold flags set, a whitespace-only cell (" ", "\t") now fails the load with CSV line 2: column "SavingsPercentage" is blank, so --min-savings-pct cannot be applied to this row. With the flags unset, the same file loads.

Untrimmed reads: cmd/multi_service_csv_thresholds.go:83 now uses strings.TrimSpace. Every other threshold read goes through parseCSVFloat (multi_service_csv.go:235,252), which already trims. This resolves review A's medium finding.

The CHANGELOG note and the comment rewrap are fine. No new findings.

cristim · 2026-10-19T17:55:22Z

Review B re-review (claude-cc-libs-reviewB-2274): delta `d6aaac9`..`c43c96d`

Reviewed SHA: c43c96d
Verdict: APPROVE. No findings.

Nits: both are fixed. The comment in cmd/multi_service_csv_cap.go is rewrapped, and the assertion message at cmd/multi_service_max_instances_test.go:469 is reworded.
Whitespace fix: parseThresholdCell trims before the blank check, and non-blank cells still go through parseCSVFloat.
New test: TestCSVThresholdWhitespaceCellIsBlank covers a space and a tab in both columns. With the flag set it asserts the error names the column, the flag and the line; with no flag set, the row loads. Reverting TrimSpace makes it fail.
CHANGELOG: the note is accurate.
Local verification at this SHA: go vet ./... is clean, go test ./... passes and golangci-lint reports 0 issues.

cristim · 2026-10-19T18:37:18Z

@coderabbitai review

coderabbitai · 2026-10-19T18:37:25Z

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Resolve the CHANGELOG.md conflict with #2273 by keeping both Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristim · 2026-10-19T19:48:36Z

Review B re-review (claude-cc-libs-reviewB-2274): merge of main at `084116d`

Reviewed SHA: 084116d
Verdict: APPROVE. No findings.

CHANGELOG.md: both fix(aws): adopt the go expiry lookback credit so --target-coverage stops re-buying replacement RIs #2235 entries and the fix(cli): --input-csv silently ignores --min-savings-pct and --max-break-even-months #1819 entry sit under "### Fixed", correctly formatted. There are no conflict markers here or in cmd/savings_plan_purchase_test.go, and markdownlint is clean.
Local verification: go vet ./cmd is clean, and go test -count=1 ./cmd -run 'CSV|SavingsPlan' passes, so the auto-merged test file compiles and passes alongside fix(aws): credit replacement RIs bought inside the coverage lookback #2273.

cristim · 2026-10-19T19:49:18Z

Review A re-review (claude-cc-libs-reviewA-2274): merge of main at `084116d`

Reviewed SHA: 084116d
Verdict: APPROVE

CHANGELOG: the delta is +7 lines. Both fix(aws): credit replacement RIs bought inside the coverage lookback #2273 Fixed entries are intact above the unchanged fix(cli): --input-csv silently ignores --min-savings-pct and --max-break-even-months #1819 entry, with no conflict markers.
Diff vs main: it covers the same 11 files as the approved head. go.mod is not touched, so the pinned pkg (scorer, BreakEven) is the one reviewed.
Combined behaviour: fix(aws): credit replacement RIs bought inside the coverage lookback #2273 changed cmd/savings_plan_purchase.go and the aws provider. This PR only tweaks a 3-line header assertion in savings_plan_purchase_test.go for the new columns, which is compatible with main. No combined-behaviour issue.
Local verification: go test -count=1 ./... passes, and golangci-lint reports 0 issues.

cristim · 2026-10-19T19:55:44Z

cristim and others added 2 commits October 19, 2026 19:19

cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-quarter Within the quarter impact/many Affects most users effort/m Days type/bug Defect labels Oct 19, 2026

Merge origin/main into fix/1819-csv-thresholds

084116d

Resolve the CHANGELOG.md conflict with #2273 by keeping both Fixed entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cristim merged commit 5a5eca0 into main Oct 19, 2026
12 checks passed

cristim deleted the fix/1819-csv-thresholds branch October 19, 2026 20:14

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

fix(cli): apply --min-savings-pct and --max-break-even-months on --input-csv - #2274

fix(cli): apply --min-savings-pct and --max-break-even-months on --input-csv#2274
cristim merged 4 commits into
mainfrom
fix/1819-csv-thresholds

cristim commented Oct 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Review limit reached

Sequence Diagram(s)

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

cristim commented Oct 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Commits (each <= 400 additions+deletions)

Local verification (macOS, this HEAD)

Known limitation

Summary by CodeRabbit

Uh oh!

cristim commented Oct 19, 2026

Independent review B (claude-cc-libs-reviewB-2274, Sonnet, cold brief, not the author, independent of review A): CSV format compatibility and test quality

Uh oh!

cristim commented Oct 19, 2026

Independent review A (claude-cc-libs-reviewA-2274, Sonnet, cold brief, not the author): money path

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Sequence Diagram(s)

Uh oh!

cristim commented Oct 19, 2026

Review A re-review (claude-cc-libs-reviewA-2274): delta d6aaac9..c43c96d

Uh oh!

cristim commented Oct 19, 2026

Review B re-review (claude-cc-libs-reviewB-2274): delta d6aaac9..c43c96d

Uh oh!

cristim commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Oct 19, 2026

Review B re-review (claude-cc-libs-reviewB-2274): merge of main at 084116d

Uh oh!

cristim commented Oct 19, 2026

Review A re-review (claude-cc-libs-reviewA-2274): merge of main at 084116d

Uh oh!

cristim commented Oct 19, 2026

Merge gate verdict (claude-cc-libs)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Oct 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Review A re-review (claude-cc-libs-reviewA-2274): delta `d6aaac9`..`c43c96d`

Review B re-review (claude-cc-libs-reviewB-2274): delta `d6aaac9`..`c43c96d`

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Review B re-review (claude-cc-libs-reviewB-2274): merge of main at `084116d`

Review A re-review (claude-cc-libs-reviewA-2274): merge of main at `084116d`