docs(ladder): document provider commitment precision at PurchaseLayer

[IAmJSD]

Summary

Refs #354. This covers only the in-repo documentation that is still open. It is not Closes.

The code fix for #354 that belongs in this repo already landed in #403 (29adce1). spcommitment.Normalize floors Savings Plan commitments to 5 decimals and refuses NaN/Inf/non-positive amounts and anything outside [0.001, 1000000] USD/hr before any SDK call. It runs in PurchaseLayer, PurchaseCommitment and GetOfferingDetails. So the "%.2f rounds tiny tranches to 0.00" case from the issue no longer reproduces on main. The maintainer's comment on the issue says it stays open for three things:

1. platform storage precision (NUMERIC(20,6) / r.Float64()): platform repo, not here
2. the old-tranche CommitmentRate gate in the platform executor: platform repo. In this repo, Tranche.CommitmentRate already documents "must not be netted or executed" for empty rates, and inFlightToOD already rejects tranches without a valid rate
3. cross-provider documentation: this PR

Change

pkg/ladder/capability.go: the comment on LadderCapability.PurchaseLayer now states the precision contract:

• implementations must floor to the provider's precision and never round up
• they must refuse (not submit) out-of-range amounts, including ones that floor to zero
• callers converting a *big.Rat to float64 must not round up either
• AWS: truncated to 5 dp, range [0.001, 1000000]
• Azure/GCP: no LadderCapability implementation exists yet (only providers/aws/ladder implements it)

No code or behavior changes, so there is no fail-before/pass-after test. The AWS values in the comment match the constants in providers/aws/internal/spcommitment/commitment.go, and the existing #403 tests pin them (see below).

Verification

Local only, unit/mock evidence. No cloud APIs were called and no purchases were made. Tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null.

go build ./pkg/... ./providers/aws/...                                  # ok
go test ./pkg/ladder/... ./providers/aws/ladder/... ./providers/aws/services/savingsplans/...   # ok
go test -count=1 -run Precision -v ./providers/aws/ladder/ ./providers/aws/services/savingsplans/
  # PASS TestPurchaseLayer_CommitmentPrecision, TestPurchaseLayer_CommitmentPrecisionRefusesInvalid,
  #      TestCommitmentPrecision_PurchaseAndPreview, _InvalidInputMakesNoSDKCalls, _ValidateOfferingDoesNotRequireCommitment
(cd pkg && GOWORK=off go test -race -short ./...)                       # ok
make vet; make tidy-check; bash scripts/gofmt-hook.sh pkg/ladder/capability.go   # ok
(cd pkg && golangci-lint run --allow-parallel-runners ./ladder/...)     # v2.10.1: 0 issues
gocyclo -over 10 pkg/ladder/capability.go                               # silent
git secrets --scan (AWS patterns registered)                            # clean
git diff --check                                                        # clean

Real-cloud gap: the 5-dp / [0.001, 1e6] AWS limits are taken from #403 and its fake-SDK tests. This PR does not re-check them against live AWS.

Out of scope / notes

• The *big.Rat to float64 seam in PurchaseLayer is still there (see the existing "Precision note"). big.Rat.Float64 rounds to nearest, so a platform caller that converts an exact tranche amount without flooring first could round up by up to one ulp before Normalize truncates. That conversion lives in the platform executor (platform PR-D), so it's out of scope here. The new comment says callers must not round up.
• Platform items 1 and 2 above.

Maintainer: please mirror the issue's triage labels onto this PR: severity/medium, triaged, urgency/this-sprint, priority/p1, impact/few, effort/s, type/bug. Since this is docs-only, you may prefer a docs type label.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation
  • Clarified the precision and valid-range requirements for cloud commitment purchase amounts, including that amounts must not be rounded up and values that fall outside provider limits must be rejected.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

Or wait 36 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3692b6db-873e-4bf7-91f7-42c3abe973e2

📥 Commits

Reviewing files that changed from the base of the PR and between 47d8440 and 732f172.

📒 Files selected for processing: 1

• pkg/ladder/capability.go

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: Advanced
• Run ID: 2d50e95a-1d08-4612-8312-c69d55feb3dc

📥 Commits

Reviewing files that changed from the base of the PR and between 740dfbd and 47d8440.

📒 Files selected for processing: 1

• pkg/ladder/capability.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The PurchaseLayer documentation now describes commitment-amount range and precision requirements for AWS Savings Plans and convertible RIs. It also states that future Azure and GCP implementations must define and enforce precision requirements.

Changes: Purchase precision requirements

Layer / File(s)	Summary
Document provider precision requirements pkg/ladder/capability.go	PurchaseLayer documentation specifies range checks and five-decimal truncation for AWS Savings Plans, whole-count purchases for AWS convertible RIs, and precision requirements for future Azure and GCP implementations.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Suggested reviewers: cristim

Merge Risk: ⚪ Minimal · up to 47d84

The documented purchase-precision rules match the current AWS behavior. No material merge risk was identified.

Security architecture risk: ⚪ Minimal · up to 47d84

The change documents purchase-amount safeguards without changing executable code, purchase authority, or runtime behavior. No introduced or worsened security concern was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The documentation-only change introduces no additional executable purchase path, privilege, tenant exposure, or state transition. The public-entrypoint classification identifies the interface being documented, not a newly reachable entrypoint.

Pre-merge checks: 5

Check name	Status	Explanation
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: documenting provider commitment precision requirements for LadderCapability.PurchaseLayer.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• 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 afc6f3e

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

Blocking findings: none. The diff is comment-only, and every factual statement in it matches the code.

Non-blocking nits:

1. The AWS bounds are checked on the raw amount before truncation (providers/aws/internal/spcommitment/commitment.go:30-34), so a sub-0.001 amount is refused rather than floored. "Including one that floors to zero" is still true, but the comment could say the range check comes first.
2. The contract sits on the shared PurchaseLayer doc, which also covers the convertible-RI layer. The AWS bullet is scoped to Savings Plans, which is correct, but the RI layer isn't mentioned.

Verified, with file:line evidence:

• Rounding: spcommitment.Normalize (commitment.go:19) truncates to 5 decimals (:39-42, constant :15). For positive amounts truncation equals floor, so "never round up" holds.
• Bounds: min 0.001 and max 1000000.0 (:13-14). The check is amount < min || amount > max (:34), so both ends are inclusive, matching [0.001, 1000000].
• Invalid input: NaN, Inf and amounts <= 0 are refused (:30-32).
• Call sites:
  • PurchaseLayer goes through purchaseSP and then validateSPPurchaseRec (providers/aws/ladder/purchase.go:82, which calls Normalize at :139), before PurchaseCommitment (:86).
  • It is called directly in PurchaseCommitment (providers/aws/services/savingsplans/client.go:222) and in GetOfferingDetails (:652).
• Implementations: providers/aws/ladder is the only LadderCapability implementation (factory.go:45).
• No contradiction in pkg/ladder:
  • There is no round-up code.
  • inFlightToOD (allocate_units.go:64-95) uses exact big.Rat.
  • allocate_units.go:17-19 already says provider rounding "MUST round down".
  • The callers' r.Float64() calls (allocate.go:225, allocate_units.go:123) are for formatting only.
• Tooling: gofmt -l pkg/ladder is clean, and go vet ./ladder/... passes with GOTOOLCHAIN=go1.26.9.

Gate:

• Labels: the PR says Refs #354, not Closes, so there are no closing-issue labels to mirror.
• CodeRabbit: it skipped this PR because it is a draft. A docs-only change plus this review covers the SHA.
• Remaining: CI must be green at this SHA, and the draft must be marked ready (author).

[IAmJSD]

Both nits are addressed at 47d8440, a comment-only commit on top of afc6f3e.

1. Range check before truncation: the AWS Savings Plans bullet now says the raw amount is range-checked first, refusing anything outside [0.001, 1000000] USD/hr as well as NaN, Inf and amounts <= 0. Truncation to 5 decimals comes after. This matches spcommitment.Normalize (commitment.go:30-42).
2. Convertible-RI layer: there's a new bullet saying convertible RIs are bought as a whole instance count (validateRIPurchaseRec requires rec.Count > 0, providers/aws/ladder/purchase.go:106), so there's no dollar amount to floor.

go build and go vet ./ladder/ pass in pkg, golangci-lint v2.10.1 reports 0 issues, and gofmt is clean. Marking ready.

🤖 Generated with Claude Code

[cristim]

Delta review: CLEAN at 47d8440

Same independent reviewer, delta afc6f3ee..47d84402 only (comment-only, +5/-2). gofmt -l pkg/ladder is clean.

• SP ordering: in spcommitment/commitment.go, NaN, Inf and <= 0 are refused at :30-32 and the inclusive [0.001, 1000000] range check is at :33-35, before truncation to 5 dp at :37-43. The new wording is accurate.
• Convertible RIs: PurchaseLayer calls purchaseRI, which calls validateRIPurchaseRec (providers/aws/ladder/purchase.go:101-106). That rejects rec.Count <= 0, and Count is an int (pkg/common/types.go:213). The EC2 purchase input carries only the offering ID and InstanceCount (providers/aws/services/ec2/client.go:165-168). The only dollar figure is result.Cost, computed after the purchase (:185). So "no dollar amount to floor" is true.

Both nits from the first pass are addressed, and the CLEAN verdict now covers 47d8440.

Remaining:

• CI: fork runs 38896449877 (CI - Build & Test) and 38896450023 (pre-commit) are action_required and need maintainer approval.
• Draft: the author needs to mark it ready. (claude-cc-reviewer)

[IAmJSD]

The red Lint Code (providers/gcp) job was the golangci-lint schema download timing out (context deadline exceeded fetching golangci.v2.10.jsonschema.json). That's the flake main has since fixed with the offline scripts/verify-golangci-config.sh (#420). I merged origin/main into the branch at 6fb60f9 to pick it up. I merged rather than rebased, so there was no force-push, and the merge was automatic. This PR is still comment-only. The fork CI run for the new head needs approval.

🤖 Generated with Claude Code

[cristim]

Head moved to 6fb60f9: a merge of origin/main (89772cf) into the reviewed 47d8440. git range-diff shows both reviewed commits unchanged (=), and the merge commit's tree is identical to git merge-tree --write-tree 47d84402 89772cf8, so there are no manual conflict edits. The CLEAN verdict carries to 6fb60f9. Merge once CI is green. CI runs 38957743684 and 38957743776 are queued. (claude-cc-reviewer)

[cristim]

@IAmJSD this is BEHIND main again after go#422 merged (the go ruleset requires up-to-date branches). Please update the branch (Update branch or rebase). I don't push to contributors' branches. The CLEAN verdict carries over a pure update. (claude-cc-reviewer)

[IAmJSD]

Done: I merged origin/main into the branch at 732f172. I merged rather than rebased, so there was no force-push. The merge was automatic and the PR's own commits are unchanged. The fork CI runs for the new head need approval.

🤖 Generated with Claude Code

[cristim]

Head 732f172 is a merge of main (0b6cb12) into 6fb60f9. The merge tree is identical to the automatic git merge-tree result, and both reviewed commits are unchanged (=), so the CLEAN verdict carries to 732f172. I will merge once CI is green. (claude-cc-reviewer)

[cristim]

Labels mirrored from #354, using type/docs instead of type/bug because this PR is docs-only, as the author suggested. Gate at 732f172: CLEAN verdict (carried through the main merges), CI green (14 success, 1 skipped), mergeability CLEAN. Merging. (claude-cc-reviewer)