fix(filters): --regions also narrows EC2 Instance Savings Plan recs

[cristim]

Summary

• --regions now also filters EC2 Instance Savings Plan recommendations by their own Details.Region (owner decision, item 76). Compute, SageMaker and Database plans stay unfiltered.
• ANDed with --include-regions / --exclude-regions; empty --regions is unchanged; an EC2 Instance SP with no known region is dropped when --regions is set. Applies to --csv-input runs too.
• Docs and CHANGELOG updated. Follows fix(sp): stop filtering Savings Plan recommendations on the endpoint region #2182.

Verification (unit-level, fixture based)

• New table test TestApplyFilters_SavingsPlansRegionsFlag incl. the eu-west-2 scenario. With the new branch removed, 4 cases fail.
• Mutation probes on the new branch: removed, inverted, empty-guard weakened, SP gate removed: all caught.
• go build, go vet, go test ./..., golangci-lint (0 issues).

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 6 billable files and costs up to $1.50.}

• 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 11 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 62 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: e64b268e-dfb6-442d-932d-5b858deb4457

📥 Commits

Reviewing files that changed from the base of the PR and between 43f1d3b and d61c1ac.

📒 Files selected for processing: 6

• CHANGELOG.md
• cmd/multi_service_filters.go
• cmd/multi_service_filters_test.go
• cmd/multi_service_helpers.go
• docs/cli/README.md
• docs/cli/filtering.md

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

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

[cristim]

Gate verdict: CHANGES REQUESTED at c4f7a9d

Blocker: --csv-input path drops every Savings Plan under --regions, including Compute and in-scope EC2 Instance plans.

parseCSVRecord (cmd/multi_service_csv.go:164-203) never sets Provider or CommitmentType. With Provider == "", awsprovider.EffectiveRegion returns rec.Region and ignores Details.Region (providers/aws service_client.go:156-163 at the pinned 9b35837e78d1), and IsRegionAgnostic returns false (service_client.go:203-209). The writer emits rec.Region, which is empty for SP rows (cmd/multi_service_csv.go:325; SP region lives in DetailsRegion). So the new branch at cmd/multi_service_filters.go:139 sees region "" for every CSV SP row and drops it.

Reproduced (scratch test via loadRecommendationsFromCSV + applyFilters, Config{Regions: ["eu-west-2"]}), rows: savings-plans-compute (PlanType Compute), savings-plans-ec2instance DetailsRegion eu-west-2, savings-plans-ec2instance DetailsRegion us-east-1:

• origin/main: kept 3 of 3
• this head: kept 0 of 3 (expected per decision 76 A: Compute + eu-west-2 EC2 SP kept, 2 of 3)

This contradicts the CHANGELOG ("Compute, SageMaker and Database plans stay unfiltered. Applies to --csv-input runs too") and docs/cli/filtering.md. Direction is fail-closed (nothing bought out of scope), but it silently empties the SP set for CSV users of --regions. The new tests only build recs with Provider: ProviderAWS, so they miss the CSV shape. Suggested fix: stamp Provider = ProviderAWS and CommitmentType = CommitmentSavingsPlan for SP rows in the CSV loader (or classify by service in the filter), plus a regression test that goes through loadRecommendationsFromCSV. Note --include-regions has the same pre-existing CSV gap.

Otherwise verified:

• Only callers of the filter: cmd/multi_service_helpers.go:671 (API path, SP recs carry ProviderAWS) and cmd/multi_service.go:922 (CSV). Non-SP recs unaffected (branch gated on common.IsSavingsPlan, pkg common/types.go:159).
• isAccountLevelSPPlanType (service_client.go:226-233): Compute/SageMaker/Database exempt, EC2Instance and unknown types region-scoped. Matches decision 76 A for API recs.
• Exact-match semantics match IncludesRegion (recfilter/filters.go:40-52); --regions is not trimmed/lowercased anywhere, mismatches fail closed (pre-existing, non-blocking).
• Fails-before: removing the new branch fails 3 subtests of TestApplyFilters_SavingsPlansRegionsFlag.
• Local, from git archive at head, GOTOOLCHAIN=go1.26.9: go test ./... exit 0, go vet ./... clean, golangci-lint run ./... no issues.

[cristim]

Gate verdict: APPROVED at d61c1ac

Delta since c4f7a9d: the PR's own change adds cfg.CSVInput == "" to the --regions Savings Plan check (cmd/multi_service_filters.go), updates the CHANGELOG and docs to say "CSV mode unchanged", and adds TestApplyFilters_SavingsPlansRegionsFlagCSVModeUnchanged. The rest comes from a normal merge of origin/main (account-filter change 43f1d3b), and the PR diff against main touches only the 6 files above.

Evidence (git archive at head, GOTOOLCHAIN=go1.26.9):

• Earlier CSV finding is resolved: loadRecommendationsFromCSV + applyFilters with --regions eu-west-2 on the Compute, EC2 Instance eu-west-2 and EC2 Instance us-east-1 rows keeps 3 of 3, same as main (the new test covers exactly this shape).
• Removing the CSV gate makes the new test fail ("should have 3 item(s), but has 0").
• API path still filters: disabling the --regions SP branch fails 3 subtests of TestApplyFilters_SavingsPlansRegionsFlag (EC2 SP in another region, unknown region, eu-west-2 scenario).
• go test ./... exit 0 (1438 tests), go vet ./... clean, golangci-lint run ./... no issues.
• CI: all required checks SUCCESS at this SHA, mergeStateStatus CLEAN.

Note for the owner: decision 76 (A) said --regions should also apply in CSV mode. This PR keeps CSV mode the same as main instead, because CSV rows carry no Provider or CommitmentType. That is no regression from main. Applying the filter to CSV runs needs a follow-up that sets those fields in the CSV loader.