refactor(aws): move the capped Cost Explorer loops onto the shared pagination helper

[cristim]

Summary

This is PR 2 of 2 for #35. It is stacked on #462 (PR 1), so merge #462 first and then retarget this PR to main. CI only runs on PRs to main.

It moves the five Cost Explorer loops that already had a cap onto the shared paginateCE helper from #462:

File	Cap constant
client.go	maxRecommendationPages
sp_coverage.go	maxSPCoveragePages
sp_coverage_series.go	maxSPCoveragePages
ondemand_series.go	maxOnDemandSeriesPages
sp_commitment_rate.go	(existing cap)

Every cap stays at 20. Each callback keeps its existing accumulation and its fetch-error wraps, including sp commitment rate: %w, GetOnDemandSeries: %w, sp coverage: %w and sp covered series: %w. The "recommendation absent on page %d" index in sp_commitment_rate comes from a closure counter and keeps the same 0-based numbering.

Wording changes, listed in the commit body:

• A canceled ctx is now returned raw. Some sites used to wrap it ("pagination canceled", "context canceled during pagination"). errors.Is still matches.
• The cap message is unified to <what>: pagination cap reached after N pages, for example "RI EC2: pagination cap reached after 20 pages".
• No consumer depends on the old wording. I grepped cloud-commitments-cli, -mcp and -platform and found no matches. The existing tests only pin the "pagination cap" substring.
• sp_commitment_rate now checks ctx before each page. Before, it made one call even when ctx was already canceled.

collection_sp.go is still excluded on purpose, because it merges partial pages. ratelimiter.go (#432) and parser_ri.go (#452) are untouched.

Closes #35 (with #462).

Verification (macOS, Go 1.26.9)

• (cd providers/aws && GOWORK=off go test -race -short ./...) passes, and no existing test changed. make build and make vet pass. golangci-lint v2.10.1 (the CI pin) reports 0 issues. gofmt is clean.
• New tests:
  • TestGetSPCoveredSeries_CapsEndlessTokens, TestGetOnDemandSeries_CapsEndlessTokens, TestGetSPCommitmentRate_CapsEndlessTokens. Each asserts the literal cap of 20 and the call count, using the bounded mock.
  • TestGetSPCommitmentRate_PreCanceledContext.
  • Three pinning tests for behaviour this refactor must keep: AbsentRecommendationReportsPageIndex ("page 1"), SPCommitmentRate_FetchErrorKeepsPrefix and GetOnDemandSeries_FetchErrorKeepsPrefix.
• Fail-before: the pre-canceled test fails on PR 1's code, which made one call and returned a wrapped error.
• The independent reviewer ran mutation probes in scratch copies. Each of these fails exactly its own test:
  • page counter += 2
  • dropping either error wrap
  • raising a cap to 21
• Review: two diff passes by the same non-author reviewer. Pass 1 found that the three behaviours above were unpinned, and the tests were added. No actionable findings remain on 8e33717.

Fakes only; no AWS call.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of multi-page AWS cost recommendations, including Savings Plans coverage, commitment rates, Reserved Instance recommendations, and on-demand series.
  • Pagination limits and API errors are now handled consistently across these results.

[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: 94aec3fd-15ec-4d34-92ad-ee32bd2d99be

📥 Commits

Reviewing files that changed from the base of the PR and between 01f1c76 and 1e1a41e.

📒 Files selected for processing: 6

• providers/aws/recommendations/ce_paginate_test.go
• providers/aws/recommendations/client.go
• providers/aws/recommendations/ondemand_series.go
• providers/aws/recommendations/sp_commitment_rate.go
• providers/aws/recommendations/sp_coverage.go
• providers/aws/recommendations/sp_coverage_series.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

Five AWS recommendation retrieval paths now use paginateCE for Cost Explorer pagination. New tests cover page limits, canceled contexts, missing recommendations on later pages, and fetch-error handling.

Changes: Cost Explorer pagination

Layer / File(s)	Summary
Delegate recommendation pagination providers/aws/recommendations/client.go, providers/aws/recommendations/ondemand_series.go, providers/aws/recommendations/sp_commitment_rate.go, providers/aws/recommendations/sp_coverage.go, providers/aws/recommendations/sp_coverage_series.go, providers/aws/recommendations/ce_paginate_test.go	fetchRIAllPages, getOnDemandSeries, sumSPRateDetails, GetSPCoverageSummary, and fetchSPCoveredByDate delegate page iteration to paginateCE. Tests check page limits, cancellation, missing recommendations, and preserved error prefixes.

Priority: ➖ Normal

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to 1e1a4

No merge-blocking issue is identified; the change is ready for normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 6 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 describes the main change: moving capped AWS Cost Explorer pagination loops to the shared helper.
Linked Issues check		Issue #35 requires shared Cost Explorer pagination and page-safety behavior. paginateCE is present with context checks, a 20-page cap at the callers, token forwarding, and standardized errors. The f…
Out of Scope Changes check		The production changes are limited to the five Cost Explorer pagination paths described for issue #35. The test changes verify caps and preserved behavior for those paths. The excluded partial-page me…

Full details: Linked Issues check

Explanation

Issue #35 requires shared Cost Explorer pagination and page-safety behavior. paginateCE is present with context checks, a 20-page cap at the callers, token forwarding, and standardized errors. The five loops in this PR use the helper and retain accumulation and fetch-error handling. The other audit paths, including fetchCoveragePaged, GetRIUtilization, and fetchDailyCoverage, already use the helper at the reviewed head. The PR adds tests for caps, pre-canceled context, page indexing, and error prefixes.

Full details: Out of Scope Changes check

Explanation

The production changes are limited to the five Cost Explorer pagination paths described for issue #35. The test changes verify caps and preserved behavior for those paths. The excluded partial-page merge path and the unrelated ratelimiter.go and parser_ri.go paths remain unchanged.

• 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 verdict: CLEAN at 8e33717

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Scope: git diff d0744592 8e33717a (stacked on #462, with merge-base d074459).

Blocking findings: none. All five loops errored at the cap before and still do, and none truncates. The purchase-recommendation loop is unchanged in behaviour.

Loop	Token field (costexplorer v1.61.0)	Cap	Pages
client.go:190 (RI recommendations)	NextPageToken	20, unchanged	appended in order, none dropped
ondemand_series.go:138	NextPageToken (GetCostAndUsage)	20	same accumulators and wraps
sp_commitment_rate.go:84	NextPageToken	20	0-based page index kept
sp_coverage.go:354	NextToken (GetSavingsPlansCoverage)	20	same
sp_coverage_series.go:93	NextToken	20	same

Token threading: every site sets the input token from the callback argument, and the helper ends the loop on a nil or empty token. The helper takes the next token from the callback's return value, so a site cannot forget to thread it.

Note (non-blocking):

• ctx errors are now returned raw at sites that used to wrap them.
• sp_commitment_rate now checks ctx before page 1.
• The cap error wording is now the same at every site.

What was attacked and how (providers/aws, GOWORK=off, no AWS calls):

• go vet ./... is clean. go test -race -count=1 ./... passes 2220 in 17 packages at this head, and the recommendations package passes 851 at d074459.
• Mutation probes, both killed:
  • Token not threaded in client.go: TestGetRecommendations_RI_Paginates fails.
  • Truncate at the cap: 8 tests fail, including TestGetRecommendations_RI_PaginationCapError and the *_CapsEndlessTokens tests.

Gate: merge after #462. Then retarget this PR to main and rebase it. A range-diff must show the commit unchanged, then CI must be green.

[cristim]

Retargeted to main after #462 squash-merged, then rebased with git rebase --onto origin/main d0744592. git range-diff of 8e33717 against new head 0ab32e0 shows the commit identical (=), so the CLEAN verdict carries. (claude-cc-reviewer)

[cristim]

CI is green on head 0ab32e0b after the retarget to main: pre-commit 38967140950 and Build & Test 38967140893.

[cristim]

Pure rebase onto main after go#467 merged. git range-diff of 0ab32e0 against new head b81b120 shows the commit identical (=), so the CLEAN verdict carries. (claude-cc-reviewer)

[cristim]

Pure rebase onto main after go#468 merged. git range-diff of b81b120 against new head d33dc8f shows the commit identical (=), so the CLEAN verdict carries. (claude-cc-reviewer)

[cristim]

Pure rebase onto main after go#469 merged. git range-diff of d33dc8f against new head 1e1a41e shows the commit identical (=), so the CLEAN verdict carries. (claude-cc-reviewer)