[cristim]

Refs #367 (pkg-first step; the CLI picks it up via a go.mod bump).

Root cause

pkg/scorer/scorer.go:85 reads rec.BreakEvenMonths, which no parser assigns, so 0 always passed. Live: EC2 partial-upfront with the flag at 1 let 23 of 23 through; the table column was always -.

Change

• common.BreakEven(rec) (months, known): provider value wins; no upfront is (0, true); otherwise upfront / (net monthly savings + upfront/term months). Net savings alone would overstate it (an all-upfront 1yr RI with 40% net savings would read 18 months instead of 7.2). Never pays back is +Inf. Unknown (non 1yr/3yr term, negative or non-finite money) is (0, false), never reported as 0.
• Scorer: with a maximum set, unknown break-even is filtered with an explicit reason (fail closed on a wrong-money guard); no-upfront passes. Passed recs get the finite computed value so the table shows it.

Evidence

Fail before (new test against old scorer.go):

--- FAIL: TestScore_MaxBreakEvenMonths_ComputedFromCosts
    "[...3 recs]" should have 1 item(s), but has 3

After: go test ./... in pkg all ok; go vet clean. golangci-lint crashes locally in the toolchain; left to CI.

Doc change for the CLI

docs/cli/filtering.md (--max-break-even-months): says recs without a computable break-even pass through. Now: no-upfront passes (nothing to recover); a rec with upfront > 0 whose break-even cannot be computed (unrecognised term) is dropped with a reason. State the formula: upfront / (net monthly savings + upfront amortised over the term).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Break-even periods can now be calculated from upfront costs, estimated savings, and supported contract terms when a provider has not supplied a value.
  • Scored recommendations can include the calculated break-even period when it is available.

• Bug Fixes

  • The maximum break-even filter now evaluates calculated periods and excludes recommendations when the period exceeds the limit or cannot be determined. Recommendations with no upfront cost pass the filter.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: aeb04f48-7ce1-47fb-a35a-4607f4698815

📥 Commits

Reviewing files that changed from the base of the PR and between 4869d9b and 083a289.

📒 Files selected for processing: 4

• pkg/common/breakeven.go
• pkg/common/breakeven_test.go
• pkg/scorer/scorer.go
• pkg/scorer/scorer_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

The change adds a function to calculate break-even months from recommendation data. The scorer uses that calculation for maximum-break-even filtering and stores computed values on passing recommendations.

Changes: Break-even calculation and scoring

Layer / File(s)	Summary
Calculate break-even months pkg/common/breakeven.go, pkg/common/breakeven_test.go	Adds break-even calculation for supplied values, upfront costs, and supported terms. Tests cover known and unknown results, no-payback cases, and scaling invariance.
Apply break-even filtering and output pkg/scorer/scorer.go, pkg/scorer/scorer_test.go	The scorer computes break-even months for filtering. It rejects uncomputable values and values above the configured maximum. For passing recommendations, it stores computed values when they are known, positive, and finite. Tests cover computed filtering and output values.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 083a2

The break-even calculation and filtering change appears 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 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files.	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 identifies the main change: computing break-even values so the --max-break-even-months filter works.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

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

[cristim]

Independent review (claude-opus-5-5) of b75d4cd: APPROVE

No blocking findings. Formula, fail-loud behaviour and tests check out.

Formula

upfront / (net + upfront/termMonths) equals upfront / (OnDemand - recurring), the monthly cash saving that repays the upfront. It is correct when EstimatedSavings is net of amortized upfront:

• All-upfront 1yr: OD 100/mo, upfront 720, recurring 0. Net = 100 - 60 = 40, gross = 100, so 7.2 months. Net alone would give 18, which is wrong.
• Partial-upfront 1yr: OD 100, upfront 360, recurring 30. Net = 100 - 30 - 30 = 40, gross = 70 = OD - recurring, so 5.14 months.
• 3yr test case (360 / (30 + 10) = 9) also checks out.

Is savings net, per provider

• AWS RI/SP: CE EstimatedMonthlySavingsAmount ("how much AWS estimates this recommendation might save you in a month", SDK types.go) is measured against on-demand at the effective, amortized rate. The AWS docs never say "net" in words, though. The same response carries EstimatedBreakEvenInMonths, which providers/aws/recommendations/parser_ri.go:185-227 ignores. Non-blocking follow-up: parse it into BreakEvenMonths so the provider-wins branch covers AWS RIs and the derivation becomes a fallback only.
• Azure Consumption path (consumption.go:75): savings = OD - TotalCostWithRI, which is net. The upfront variant's CommitmentCost is the term total (consumption.go:274). Correct.
• GCP: sets BreakEvenMonths itself, so the provider value wins. That contradicts the body's "no parser assigns it". computeengine/client.go:1263, cloudsql:512 and memorystore:480 compute CommitmentPrice (a term total) / monthly savings. For a monthly-billed CUD that gives term*(1-p)/p, for example about 61 months on a 3yr CUD at 37%. Filter behaviour is the same as before this PR. File a follow-up.

Non-blocking

• breakeven.go:30: CommitmentCost == 0 counts as known "no upfront". Azure Advisor "upfront" variants (providers/azure/recommendations.go:404) and AWS RIs with a nil UpfrontCost therefore pass as 0. G4b/G6a-3 (CommitmentCostUnknown) is the planned fix, and this PR does not conflict with it. G4b should switch the reason to the price-unknown label.
• PR is BEHIND main. The merge with main and fix(recfilter): coverage drop reason, EC2 platform engine filter, money scaling on dedupe #379 builds, and pkg tests are green.

Evidence

• GOWORK=off go vet ./... && go test ./... in pkg: all ok.
• aws, azure, gcp and ci_cd_sanity_tests build with GOWORK=off and in the workspace.
• CLI go test ./... with pkg replaced by this head: all ok.
• Fail-before (new test, old scorer.go): should have 1 item(s), but has 3; "" does not contain "7.2 months exceeds maximum 1".
• Mutations, all killed:
  • drop the amortization term: TestBreakEven and the scorer test fail
  • unknown term -> (0, true): both fail
  • scorer lets unknown pass: scorer test fails
  • never-pays-back -> 0: TestBreakEven fails
  • no upfront -> unknown: both fail
• CI: 14/14 green on this SHA.

[cristim]

Independent review (claude-opus-5-5) of 083a289: APPROVE
Update-branch of the approved b75d4cd: only main's commits came in, as clean merges with no conflict resolution; the PR's own diff content is unchanged; GOWORK=off vet and tests pass in: pkg.