Skip to content

fix(ladder)!: every ramp step incl. AfterDays==0 becomes a Tranche, drop BuyNow - #351

Merged
cristim merged 1 commit into
mainfrom
fix/ladder-buynow-netting-36
Oct 12, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/ladder-buynow-netting-36

Conversation

@cristim

@cristim cristim commented Oct 12, 2026

Copy link
Copy Markdown
Member

Every ramp step, including AfterDays == 0, is now a scheduled Tranche (immediate step: FireAfter == Now), and TrancheResult.BuyNow is removed. The immediate step used to be a BuyNow action that nothing persisted, so in-flight netting never saw it.

Breaking: the only consumer of BuyNow is the platform's internal/server/handler_ladder.go (cli and mcp checked); it needs a pin bump plus the platform B1 change (persist immediate rows, display-only plan JSON).

Tests: issue-input regression (gap 10, ramp [0:.5, 30:.5] gives two 5/hr tranches, step 0 at Now; fails before the change), nil InFlightUSDPerHour rejection. Units documented on Tranche.AmountUSDPerHour and InFlightUSDPerHour (#349).

Refs #36 (closes after the platform PRs).

🤖 Generated with Claude Code

…drop TrancheResult.BuyNow

The immediate step was returned as a BuyNow action the platform never
persisted, so in-flight netting could not see it. Every non-zero step is now a
scheduled Tranche (immediate: FireAfter == Now). Adds the issue-input
regression test and the nil-InFlight validation test, and documents the
amount unit on Tranche and InFlightUSDPerHour.

Refs #36

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged urgency/this-quarter Within the quarter impact/many Affects most users effort/m Days type/bug Defect labels Oct 12, 2026
@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 6 billable files and costs up to $1.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 58 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 60 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: ecb72a19-bf8e-424e-b5cd-941857fa3c04

📥 Commits

Reviewing files that changed from the base of the PR and between 5b9155e and 829de8e.


📒 Files selected for processing (6)
  • CHANGELOG.md
  • pkg/ladder/allocate.go
  • pkg/ladder/allocate_test.go
  • pkg/ladder/ladder.go
  • pkg/ladder/ladder_test.go
  • pkg/ladder/store.go

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

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

@cristim

cristim commented Oct 12, 2026

Copy link
Copy Markdown
Member Author

Independent merge-gate review (Opus, claude-opus-5-5): APPROVE at 829de8e

Base: main 5b9155e (head not behind). Local runs from a fresh clone at the exact head, GOWORK=off, GOTOOLCHAIN=go1.26.9, GOMAXPROCS=2, -p 2.

Evidence

  1. appendStep/buildTranche: every non-zero step becomes exactly one Tranche, in step order. Randomized golden + property harness (20,000 seeded cases: random valid ramps of 1-5 steps incl. fully-delayed, plus fixed sets {0.5,0.5}, epsilon overshoot {0.5,0.5000000008,1e-10}, sums just under 1 {0.4999999995 x2}, thirds, {0.004,0.996}; 0-3 allocations with random big.Rat gaps): per allocation the tranche amounts sum to the gap exactly (big.Rat), none dropped or duplicated, StepIndex strictly increasing, FireAfter == Now.AddDate(0,0,AfterDays) by == (AfterDays 0 gives FireAfter == Now, same instant and location, no epsilon). Golden vs base: base BuyNow (layer, amount, term, payment) is byte-identical to head's FireAfter==Now tranches, and all future tranches are identical apart from IDs (17,922 immediate items, 162k-line dump, cmp identical). No behaviour change for non-immediate steps. allocate.go change is comment-only, so Allocate output is unchanged.
  2. go#36 scenario: gap 10, ramp [0:.5, 30:.5] gives two tranches 5 + 5, step 0 at Now. Fails before: head tests run against base code fail TestBuildTranches_ImmediateStepIsTranche ("Tranches count = 1, want 2") plus 6 updated step tests.
  3. Deleted tests (rationale contents, data sources, small-fraction rationale): they asserted only the buy-now PlannedAction's step-rationale text and DataSources. The platform never reads TrancheResult.BuyNow (git grep BuyNow on platform origin/main: test comments only, handler_ladder_test.go:519,728). UI rationale and data_sources come from plan actions built from Allocation.Rationale/DataSources in assembleLadderPlan (handler_ladder.go:566-567), read by frontend ladder-timeline.ts:71,90 and ladder-timeline-editor.ts:17. What is lost: the per-step "ramp step i/n (x%): $a of $g total" string, never persisted or shown. Acceptable.
  4. Consumers: cli and mcp origin/main have no BuyNow/BuildTranches/TrancheResult. No other go package references them. No stale BuyNow in README/docs.
  5. Nil InFlight: TestAllocate_NilInFlightRejected passes on base and head (it pins existing behaviour; error contains "InFlightUSDPerHour must not be nil").
  6. Units docs on Tranche.AmountUSDPerHour and InFlightUSDPerHour match plan amendment 6 (OD-equivalent $/hr until go#349, same unit as the gap).
  7. CHANGELOG flags the break.
  8. Tests: pkg, providers/aws, providers/azure, providers/gcp, ci_cd_sanity_tests go test ./... all exit 0; go vet ./ladder/ clean. Optional mutants (4, changed lines): drop the immediate step, FireAfter +1ns, duplicate the immediate tranche, FireAfter +1 day: all killed (7 tests fail each). git ls-files -z | xargs -0 git secrets --scan with the AWS patterns registered: exit 0 under both BSD and GNU grep. CI all green, mergeStateStatus CLEAN, labels match go#36, body says Refs ladder: buy-now ramp steps escape netting + follow-up hardening for L5 convergence #36.

Non-blocking follow-ups

  • Stale doc comments: ladder.go:10-11 ("immediate purchase actions"), :19 and :24 ("buy-now purchase action and future tranche"); test comments ladder_test.go:149, 211, 797; TestBuildTranches_EmptyAllocations now checks len(result.Tranches) != 0 twice.
  • CHANGELOG wording: the platform calls BuildTranches but never reads BuyNow, so it compiles unchanged on the bump. The behaviour change on bump is that immediate rows get persisted with FireAfter == run start, and that is B1's concern (grace bound vs cancelOverdue, #544).
  • Plan T5/H5 randomized property test was not added in this PR. The harness above shows the property holds; adding it to the suite is a coverage follow-up.

@cristim
cristim merged commit ad67030 into main Oct 12, 2026
15 checks passed
@cristim
cristim deleted the fix/ladder-buynow-netting-36 branch October 17, 2026 13:23
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/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant