fix(aws): credit own replacement RIs in RDS family expiry sizing

[cristim]

What

ApplyFamilyNUSizingRDSWithExpiry (#341) re-buys replacement RIs the account already bought, when the inventory is partial (the CLI inventory is single-account, cli#2131). Cost Explorer coverage is a lookback average, so RIs started inside the lookback are not in it yet, and the C-E branch of max(C-E, min(C, A-E)) ignores them. Found in the cli#2175 gate (F1): lagged coverage 82.5%, 4 large expiring, 4 replacements bought 2 days ago, 12 other-account large not in the inventory. True gap is 0, the CLI dry run buys 3 x db.r6g.large.

Adds ApplyFamilyNUSizingRDSWithExpiryOptions with FamilyExpiryOptions{WindowDays, CoverageLookbackDays}. R = NU of visible active, non-expiring RIs in the family that started inside CoverageLookbackDays. Post-expiry covered:

max(0, max(min(C, C - E + R), min(C, A - E)))

FamilyExpiryReport.ReplacementNU records R. Existing exports are unchanged; ApplyFamilyNUSizingRDSWithExpiry delegates with lookback 0, so R = 0 and it is byte-for-byte the merged behaviour. The CLI should pass the lookbackDays it gives GetRICoverageMap.

Proof sketch

• R >= 0, so min(C, C-E+R) >= C-E: covered is never below the merged formula, so the purchase never grows.
• covered <= C, and C - covered <= E: the deduction never exceeds expiring NU.
• Replacements inside the lookback are in lagged C only in part (fraction f of them), so crediting them in full over-credits by at most f*R: the error is toward buying less.
• Unsaturated model (usage above every RI; C = all old RIs + f*R; others' RIs old and invisible): if R <= E then C-E+R = L+Oth+(1+f)R >= L+Oth+R >= A_all-E_all; if R > E the first term equals C >= the full-inventory value. So covered >= the full-inventory covered, i.e. purchase <= the full-inventory purchase.
• Known limit, unchanged by this PR: in a saturated family with unseen other-account RIs the C-E branch can still over-buy against the full inventory (cli#2131).

Tests

• Exact repro: current function buys 3, new buys 0.
• Full inventory buys 0; go#341 F1 case (64 NU covered, 16 expiring, 48 unlisted) still buys 2 x xlarge.
• Replacements outside the lookback, themselves expiring, inactive, or in another family: not counted. Mixed sizes summed in NU. Lookback 0 credits nothing.
• 3000-seed golden test: lookback 0 equals the merged formula.
• 3000-seed property test: never below the R=0 covered, never above C, deduction <= expiring NU, covered and purchase vs a full-inventory oracle.
• Mutation (3 manual): dropping +rep, flipping the start-date comparison, removing the min(C, .) cap each fail tests.

GOWORK=off go test ./... in providers/aws, golangci-lint (go1.26.9) and git-secrets scan clean.

Refs LeanerCloud/cloud-commitments-cli#2132, LeanerCloud/cloud-commitments-cli#2175. Follow-up for the EC2 per-pool path: #346.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 3 billable files and costs up to $0.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 28 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 60 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-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 909e2370-9ad9-4b25-b907-198c88ba7951

---

📥 Commits

Reviewing files that changed from the base of the PR and between 4178c85 and 0384b87.

---

📒 Files selected for processing (3)

• CHANGELOG.md
• providers/aws/recommendations/family_nu.go
• providers/aws/recommendations/family_nu_expiry_replacements_test.go

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Head moved only by a merge of origin/main (no formula or test changes). Conflict file: CHANGELOG.md (both ### Added entries kept). Re-ran providers/aws tests, golangci-lint (go1.26.9) and git-secrets scan: clean.

[cristim]

Independent merge-gate review (Opus 5.5, claude-opus-5-5): APPROVE

Reviewed SHA: 0384b87d53bc272857d8bf052defc2530e1c72cf. The code was reviewed at 49ad67eb. b4de8f82 and 0384b87d are merges of main only (#345, #344), and git diff origin/main 0384b87d outside CHANGELOG.md is byte-identical to git diff 4c75a84 49ad67eb. The CHANGELOG resolution keeps both Added entries.

What I checked

• Formula safety: I wrote my own randomized property test (150k worlds, not committed). It uses an hourly lagged-coverage simulation with random families, mixed sizes, start dates (old, inside the lookback, exactly at the boundary, future payment-pending), expiries (inside the window, long-lived, retired inside the lookback), RIs that are both recent and expiring, invisible other-account RIs, saturated and unsaturated usage, lookbacks 7/14/30/60 and windows 7/30/60.
  • Invariants held in every world: new covered >= the feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 covered, new covered <= C, C - covered <= expiring NU, and covered >= 0.
  • The new purchase was never larger than the feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 purchase: 0 of 150k worlds.
  • In the PR's stated model (unsaturated, other-account RIs old, own expiry present): 0 over-buys against the lag-capped full-inventory oracle min(C, true post-expiry), where feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 over-bought in 410 worlds. Against the PR's own oracle (the feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 function with full inventory) the worst excess was 0.
  • Outside that model, over-buys against the true inventory still happen, but feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 has every one of them too. They come from saturated families with unseen RIs, other-account RIs bought inside the lookback, and recent own buys beyond E (covered is capped at C).
• Double counting: a replacement already partly in lagged C is credited in full. That over-credits by f·R, so the error only goes toward buying less. It corrects itself once the RI ages out of the lookback. The result is capped at C and floored at 0, so a family at or over target is dropped as AlreadyAtTarget and no negative buy is possible.
• Lookback semantics: StartDate.After(now - L days) compares instants, so timezones do not matter. The boundary is exclusive. A zero StartDate is not credited.
  • Golden comparison against the verbatim feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 adjustFamilyCoverageForExpiry: 150k random inputs (multiple families and engines, unparsable classes, unknown families, active/payment-pending/retired, windows -5..55, lookback <= 0) gave exact == CoveredNU, identical Unusable/ExpiringNU, and ReplacementNU 0.
  • The CoverageLookbackDays > 0 guard is required. Without it, future-start payment-pending RIs get credited at lookback 0 (270 golden diffs). The FamilyExpiryOptions godoc ties CoverageLookbackDays to GetRICoverageMap's lookbackDays.
• API: the change is additive (new function, new options struct, new ReplacementNU field). The godoc and CHANGELOG formulas match the code.
• Tests: the repro asserts feat(aws): add ApplyFamilyNUSizingRDSWithExpiry for rebuy-window RDS sizing #341 buys 3 and the new function buys 0. The full-inventory case buys 0, the 64-NU case still buys 2 x xlarge, and outside-lookback, expiring, inactive and other-family replacements are not counted.
• Mutation (17 mutants on changed lines, one run): 12 killed. Survivors, all non-blocking:
  • M5 drops the outer max(0,…). It is equivalent, because A-E >= 0.
  • M6 changes A-E to A-E+R, which only buys less.
  • M8 makes the boundary inclusive.
  • M9/M10 weaken the CoverageLookbackDays > 0 guard. The PR's tests do not cover it; my golden test does.
  • M16 changes WindowDays <= 0 to < 0, a pre-existing line.
• Follow-up and gates: EC2 per-pool expiry path re-buys replacement RIs bought inside the coverage lookback #346 (EC2 per-pool path) is filed and labelled. Labels match. CI is green on 0384b87, and the state is CLEAN. The earlier Lint Code (pkg) failure on 49ad67e was the golangci-lint schema-fetch timeout flake; it was green on the reruns. Locally with go1.26.9 and GOWORK=off, all 5 modules pass and git secrets --scan is clean under BSD and GNU grep.

Non-blocking follow-ups

1. Add a test that pins the lookback-0 guard using a future-start payment-pending RI, plus one for the boundary.
2. The godoc and CHANGELOG do not state the remaining partial-inventory limits: saturated families, other-account RIs bought inside the lookback, and buys beyond E capped at C. The PR body covers the first and states the model assumption for the second.
3. Optionally credit (1-f)·R instead of R. f is known from StartDate, so this would remove the transient under-buy.