fix(plans): allocate marked ramps by exact step count

[cristim]

What

Allocate marked (managed) ramps by exact step count: size count targets and Savings Plans shares from exact ratios, keep legacy percent sizing for unmarked ramps, and reject cent loss at serialization.

Why

M2 managed-ramp chain for #631: float cumulative percent sizing drifts for uneven step counts (e.g. 3 steps of a base total), so marked ramps need exact rational allocation.

Local verification

• go build ./... and go test -race ./internal/config/... ./internal/purchase/... on this tip (macOS).
• Fixture-backed: regressions call the real Resolve with fake recommendation reads and native PostgreSQL; provider completion simulated. Parent-fail and deletion probes were run in scratch copies. Not a real-account scenario.

Stack (M2 chain, merge in order)

1. scheduling PR: fix(plans): preserve managed next-root dates on completion #906
2. this PR (fix(plans): allocate marked ramps by exact step count #907, base scheduling branch)
3. claim PR: fix(purchase): claim the exact managed schedule root #908
4. target-met PR: fix(plans): lock the plan before completing a target-met step #909

Refs #631

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 7 billable files and costs up to $1.75.}

• 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 22 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 63 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-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: b3868991-7be0-410c-8464-d96cf01ae197

📥 Commits

Reviewing files that changed from the base of the PR and between 56829d8 and cf12d41.

📒 Files selected for processing: 7

• internal/config/types.go
• internal/config/validation.go
• internal/planstep/allocation.go
• internal/planstep/allocation_test.go
• internal/planstep/precheck.go
• internal/planstep/resolve.go
• internal/purchase/plan_step_allocation_integration_test.go

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

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

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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]

Independent review of 03144c3: no actionable findings

Base used: 36d385e, which is the current head of #906 (fix/631-m2-scheduling). Diff reviewed: 36d385eb...03144c39 (345 additions, 35 deletions, 7 files). Stack is consistent.

Findings (all NON-BLOCKING)

1. Nit, internal/planstep/allocation_test.go: the step > TotalSteps clamp in equalCountShare (min(step, r.count), allocation.go:38) has no test. A mutation probe replacing it with step passes the whole planstep suite. Production code is correct (the clamp keeps the target at 100% of base if TotalSteps is lowered mid-ramp). Suggest one resolver case with step 5, count 3, expecting target-met or no over-buy.
2. Nit, marked Savings Plans do not telescope: each step is sized as floor-to-cent(hourly x coverage / (100 x count)) from the share at step 1 (resolve.go equalCountShare(coverage, 1)), so over count manual steps the cents lost to each floor add up (hourly 1.234, count 3: 3 x 0.41 = 1.23 vs 1.234; the loss is bounded by about one cent per step). This matches the plan's stated contract ("manual count3/97 exact cent floor") and automatic multi-step SP stays refused, so it is not a defect. It is worth saying in the PR body that "exact" applies to counts, and that SP shares are exact per step with a cent floor.

What I attacked

• Count arithmetic: target = floor(base x coverage x min(step,count) / (100 x count)) in big.Rat (allocation.go:35-39, resolve.go target). The cumulative floor telescopes, so the last step equals floor(base x coverage/100) exactly. Verified by test: base 97, count 3 gives deltas [32,32,33]; count 97 gives 1 per step; base 100, percent 33 legacy still gives 99 (unchanged).
• Decimals: decimalRatio parses the shortest round-trip text, so 33.3 is exactly 333/10, not the float expansion. NaN, Inf and negative are rejected. Exponent forms (1e20) parse. floorAllocation rejects values beyond int range.
• Zero or one step: ValidateAllocationMode requires 1..MaxTotalSteps, and immediate requires exactly 1. Resolve calls Precheck (resolve.go:95) before any use, so big.NewRat(.., 100*count) cannot see count 0 (no division panic). Step < 1 is rejected already. Immediate with count 1 and stored percent 33 or 0 buys the full 100%.
• Mid-ramp count change: baseline is frozen and the own-ledger committed count is subtracted, so lowering or raising TotalSteps after steps ran can only produce gap <= 0 (no buy) or a smaller catch-up, never a double buy. Partly executed ramp: the test with committed 17 and shown 2 clamps to shown; committed 32 at step 2 gives ErrTargetMet. Negative baseline, shown or committed counts error out in marked mode.
• Legacy path: empty mode keeps math.Floor(base*(coverage*cum)/10000) and the old validateInputs bounds; immediate PercentPerStep != 100 is still refused when mode is empty.
• SP representation: exactSPHourly floors exact cents and checks both the shortest-JSON text and %.2f equal those cents, else errors (no silent wrap).
• Tests at this SHA: go build ./... ok; go test -race ./internal/planstep/... ./internal/config/... 872 passed; go test -tags integration ./internal/purchase -run TestEqualCountAttach passed against native Postgres (24.8s).
• Fail-before: copied allocation_test.go onto the parent tip; TestEqualCountResolve, TestEqualCountSavingsPlan (all 6 cases), TestEqualCountImmediateOmittedPercent, TestEqualCountRejectsBeforeReads and TestEqualCountInvalidQuantities all fail there.
• Mutation probes (4, each reverted): float-cumulative target instead of rational (killed by 3 tests); ceil instead of floor (killed); SP share at step=count instead of 1 (killed); remove min(step,count) clamp (SURVIVED, finding 1).
• Not tested by me: the 97-step native attach covers only the first step (the 97 steps all run through the resolver unit test); provider completion is simulated, as the PR states.

Gate status (origin/main Review gate)

• Independent review: this comment, at the SHA above.
• Findings resolved: no actionable findings; two nits above.
• CI on SHA: not applicable until retarget (base is a child branch, not main); statusCheckRollup has 1 entry only.
• CodeRabbit: not applicable until retarget; no review threads exist.
• Local real-path evidence: PR body states fixture-backed Resolve plus native Postgres, provider completion simulated, not a real-account scenario. I reproduced the planstep and native attach runs above.
• Labels: match #631 (effort/xl, impact/many, priority/p1, severity/high, triaged, type/feat, urgency/this-sprint). The PR has no closing issue (Refs #631).
• Size: 345 additions, under the 400 cap for additions; resolve.go and allocation.go stay under 500 lines.

[cristim]

Rebased onto main after #906 merged (squash 4e1fb1b); retargeted to main. Pure rebase, git range-diff 36d385eb..03144c39 origin/main..cf12d41a:

1:  03144c39 = 1:  cf12d41a fix(plans): allocate marked ramps by exact step count

Empty range-diff, so the independent verdict on 03144c3 carries to cf12d41 (owner #631 rule). Gate still needs CI green on cf12d41.

[cristim]

Gate record for cf12d41 (origin/main CLAUDE.md "Review gate"):

• Independent review of 03144c3: no actionable findings. Two non-blocking nits: an untested over-step clamp, and SP per-step cent flooring, which is a doc-wording issue.
• cf12d41 is a pure rebase of 03144c3 onto main after fix(plans): preserve managed next-root dates on completion #906. git range-diff is empty (recorded above), so the verdict carries under the owner feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631 rule.
• CI on cf12d41: all checks pass, including CI Success and Run pre-commit hooks.
• Local verification (macOS):
  • go test -race ./internal/planstep ./internal/config passes, as does the integration test TestEqualCountAttach on native Postgres.
  • The new allocation tests fail on the parent tip.
  • Probes: the float target, ceil and wrong-step SP share probes are all killed.
  • Fixture-backed; provider completion simulated. No real purchase.
• CR skipped: quota-limited; covered by the independent review plus local verification.
• Labels mirrored from feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631.

Merging at cf12d41 with --match-head-commit. #908 will be retargeted to main and rebased next.