[cristim]

Summary

PR 2 of the commitment-laddering feature: the pure allocation algorithm in pkg/ladder/allocate.go. Given pre-fetched provider data, Allocate decides how much new hourly commitment to place on each layer.

Pipeline:

• Validate: config (LadderConfig.Validate), layer topology (exactly one flex role, at most one base role), and layer states (every layer needs non-nil Existing and Expiring $/hr; providers must pass explicit zeros - incomplete data is a hard error, never treated as 0).
• Baseline: nil low-water baseline is an in-band explainable no-op (Hold with rationale), not a silent fallback.
• Gap: gap = min(target - existing, low_water - existing) - the utilization-protection clamp never commits above the observed usage floor. Gaps at or below the named 1-cent threshold produce a Hold whose rationale embeds the concrete numbers.
• Split: buffer fraction first, then base up to the stable-usage headroom, remainder to flex. Handles the Azure merged topology where one layer carries both base and buffer roles (single combined allocation). Unknown stable usage or missing base layer routes core gap to flex with a rationale note naming the actual reason.
• Guardrails: proportional scaling of all allocations to max_hourly_commit_per_run (scaled sum equals the cap exactly); exceeding max_actions_per_run is an error, never a silent drop; a bufferless topology with a nonzero buffer fraction is an error (no purchase directive may target a nonexistent layer).
• Reshape detection: buffer layers with existing commitments and utilization below the new BufferUtilizationThresholdPct config field (validated (0, 100], default 90) emit ActionReshape; unknown utilization emits an informational Hold; empty buffer layers are skipped entirely (no noise on fresh accounts).

All money math is exact *big.Rat; float64 inputs are converted once at the boundary with NaN/Inf/negative rejection. No time.Now() inside - the clock is injected via AllocationInput.Now.

Stacked on #1338 (feat/ladder-types); will auto-retarget to main when that merges. Part of #1334 (phase 1 of LeanerCloud/cloud-commitments-go#27).

The package remains inert: no callers until PR 8 wires the engine.

Test plan

• 122 table-driven test cases in allocate_test.go (154 total in the package post-rebase), including fails-pre-fix regressions for the empty-buffer-layer noise and the bufferless-topology empty-layer allocation bug.
• Exact big.Rat expectations (no float tolerance) for the AWS 3-layer split, Azure merged split, cap scaling (sum == cap exactly), and utilization clamp.
• Rationale assertions verify the concrete numbers (baseline, target %, existing, gap) appear in every hold/allocation.
• All gates exit 0: go build ./..., go vet ./ladder/, gofmt -l ladder/ (empty), go test ./ladder/ -count=1, golangci-lint run ./ladder/....

Summary by CodeRabbit

• New Features

  • Added ladder allocation engine to compute per-layer commitment gaps, split allocations across base/flex/buffer roles, and recommend buffer reshapes when buffer utilization is low.
  • Introduced BufferUtilizationThresholdPct with a default value to control when reshapes are issued.

• Bug Fixes

  • Strengthened input and run-limit validation, including tighter handling of small commitment gaps and missing utilization data.
  • Improved result behavior to reduce unnecessary actions and enforce allocation caps.

• Tests

  • Expanded test coverage for allocation correctness, topology validation, scaling limits, and new buffer-threshold validation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new Allocate engine for ladder commitment sizing, with exact rational gap calculations, per-layer allocation splitting, cap scaling, and buffer utilization-based reshape handling. It also adds a buffer utilization config threshold and broad validation and coverage.

Changes

Ladder Allocation Engine and Config

Layer / File(s)	Summary
Buffer utilization config pkg/ladder/types.go, pkg/ladder/types_test.go	Adds the default threshold, new config field, range validation, and matching validation cases.
Allocation contracts and entry point pkg/ladder/allocate.go	Defines allocation input/output types, the no-op result check, the minimum gap threshold, and top-level Allocate validation and flow.
Net usage and hold results pkg/ladder/allocate.go	Computes rational net existing usage and baseline/target values, rejects inconsistent snapshots, and returns hold results for missing baseline or sub-threshold gaps.
Gap splitting and allocation assembly pkg/ladder/allocate.go	Splits the gap across roles, handles Azure merged layers, scales oversized plans, and emits reshape or hold actions from buffer utilization.
Allocation engine tests pkg/ladder/allocate_test.go	Adds coverage for validation, topology, allocation math, cap limits, reshape behavior, helper validation, Azure merging, and no-op semantics.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding the ladder allocation algorithm with split logic and guardrails.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/ladder-allocate

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

pkg/ladder/allocate_test.go (1)

456-475: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting a validConfigAzure helper.

The Azure test builds a full LadderConfig literal from scratch instead of reusing a shared helper like validConfigAWS. Minor duplication; not blocking.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/ladder/allocate_test.go` around lines 456 - 475, The Azure allocation
test is duplicating a full LadderConfig setup instead of reusing a shared valid
config helper. Extract or reuse a `validConfigAzure` helper alongside
`validConfigAWS`, and update `TestAllocate_AzureMergedBaseBuffer` to build from
that helper so the Azure-specific defaults stay centralized and the test only
overrides the fields it needs.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/ladder/allocate.go`:
- Around line 240-243: The allocation logic in allocate.go currently floors
negative net commitment to zero after subtracting expiring from existing, which
hides inconsistent provider snapshots. Update the calculation in the
net/existing/expiring path inside the allocation function so that if
ExpiringUSDPerHour exceeds ExistingUSDPerHour it fails loudly instead of
silently clamping, using the existing allocate flow and big.Rat comparison logic
to return an error or otherwise stop processing.
- Around line 176-192: validateLayers only checks flex/base counts, but splitGap
assumes stricter topology invariants. Update validateLayers in allocate.go to
reject duplicate LayerType values, enforce that only the base+buffer role merge
is allowed, and limit buffer roles to at most one; use the existing LayerSpec,
LayerType, RoleBase, RoleFlex, and any buffer role symbols to keep the
validation aligned with findLayerForRole’s first-match behavior.
- Around line 501-505: The Allocation rationale in allocate.go still describes
the pre-cap role gap after scaling, so update the rationale built in the
allocation scaling path to include the final capped amount. Use the existing
symbols Allocation, a.GapUSDPerHour, and the scaling logic in the loop to append
the post-cap GapUSDPerHour (or equivalent final layer amount) alongside the
current capped-by-max_hourly_commit_per_run text so approval/audit output
reflects the actual capped value.
- Around line 198-210: Validate UtilizationPct in validateLayerStates before it
is used by reshape logic. The current LayerState checks in validateLayerStates
only verify ExistingUSDPerHour and ExpiringUSDPerHour, so add a finite-range
check for st.UtilizationPct when it is non-nil, ensuring it is not NaN/Inf and
stays within [0,100]. Use the existing LayerState and validateLayerStates
symbols so the new validation catches bad utilization values before decisions
made later in the allocate flow.

---

Nitpick comments:
In `@pkg/ladder/allocate_test.go`:
- Around line 456-475: The Azure allocation test is duplicating a full
LadderConfig setup instead of reusing a shared valid config helper. Extract or
reuse a `validConfigAzure` helper alongside `validConfigAWS`, and update
`TestAllocate_AzureMergedBaseBuffer` to build from that helper so the
Azure-specific defaults stay centralized and the test only overrides the fields
it needs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f633a0a8-fc7b-4cc7-b57a-a206a69ab7f6

📥 Commits

Reviewing files that changed from the base of the PR and between 0754d39 and 19a03d0.

📒 Files selected for processing (4)

• pkg/ladder/allocate.go
• pkg/ladder/allocate_test.go
• pkg/ladder/types.go
• pkg/ladder/types_test.go

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.