Skip to content

test(aws): preserve requested-window SP coverage rates - #168

Merged
cristim merged 3 commits into
mainfrom
test/51-sp-window-contract
Sep 30, 2026
Merged

cristim merged 3 commits into
mainfrom
test/51-sp-window-contract

Conversation

@cristim

@cristim cristim commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Savings Plans hourly rates normalize reported totals over the requested window. Counting only returned coverage blocks would inflate rates for valid sparse usage: 240USD across a 30-day window is 1/3USD/hour whether 20 inactive days are omitted or explicitly zero.

Add the sparse/explicit-zero/nil regression, including pagination, and clarify the exported rate and Days comments. No production calculation changes. Refs #51, specifically the independently reviewed A07-019 disposition. Reporting completeness cannot be inferred from this endpoint's missing days.

Issue #51 remains open while the separate RI fix in #167 and the reporting-completeness disposition are completed. The recorded semantic decision retains the full-window divisor. The fixture cannot establish whether an omitted live period means zero activity or delayed reporting; no new live incomplete-data defect is claimed.

Verification: recommendations race suite, AWS build, pinned lint2.10.1 and normal hooks pass. An intentional Days-based divisor mutation fails the new regression at 1 versus 1/3USD/hour. A real AWS SDK/local HTTP fixture confirms equal requested-window rates for sparse and explicit-zero responses; no live AWS calls.

Independent gpt-6-astra review approved final integrated commit cd4aa1286951dbf70e0b1ba413c7a61b0349291c: clean checkout and parent verification, full main-relative diff reread, GCP files identical to main 28efbcd, fresh AWS RI/SP and GCP consumer race tests, and real SDK/local HTTP experiment. Additive merges preserve the original SP commit 19dba433cc6db5b63978b2b14dfe5b2c81fcc96d; the SP contract files are unchanged. The reviewer independently confirmed that the Days-based mutation fails the intended rate assertions. Explicit normal hooks on the SP and incoming GCP files passed. This is the user-authorized independent review path; CodeRabbit is optional.

Verify sparse and explicit-zero days produce identical hourly rates,
including paginated and nil-coverage responses. Clarify that Days does
not certify reporting completeness or alter the requested-window divisor.

Refs #51
@cristim cristim added triaged Item has been triaged urgency/this-sprint Within the current sprint priority/p1 Next up; this sprint severity/high Significant harm impact/many Affects most users effort/m Days type/bug Defect labels Sep 30, 2026
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.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 10 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 70 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: ca5468d9-84c7-44ae-8d8a-554ab9f9416e

📥 Commits

Reviewing files that changed from the base of the PR and between 28efbcd and cd4aa12.

📒 Files selected for processing (2)
  • providers/aws/recommendations/sp_coverage.go
  • providers/aws/recommendations/sp_coverage_window_test.go

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

Preserve the reviewed Savings Plans contract regression while including
the merged RI coverage aggregation fix from main.
Include the merged storage pricing change without rewriting the reviewed
Savings Plans contract regression or published branch history.
@cristim
cristim merged commit f260c4c into main Sep 30, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/many Affects most users priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant