feat(ladder): staggered tranche building from allocations - #1341
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a new ChangesRamp Tranche Building and Self-Describing Tranches
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant BuildTranches
participant processAllocation
participant computeStepAmount
participant appendStep
Caller->>BuildTranches: TrancheInput
BuildTranches->>processAllocation: allocation
processAllocation->>computeStepAmount: gap, priorSum, fraction
computeStepAmount-->>processAllocation: step amount
processAllocation->>appendStep: step amount, AfterDays
appendStep-->>BuildTranches: BuyNow action or Tranche
BuildTranches-->>Caller: TrancheResult
Related issues: None found in the provided context. Related PRs: None found in the provided context. Suggested labels: enhancement, needs-review Suggested reviewers: None identified from provided context. Poem 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/ladder/store.go (1)
226-238: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
AmountUSDPerHouraccepts non-canonical rationals.big.Rat.SetStringalso parses decimal/exponent forms, so the validator allows inputs beyond the documentedRatString()encoding. If canonicala/boraform matters for storage/equality, reject or normalize non-canonical inputs; otherwise update the docs to match.🤖 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/store.go` around lines 226 - 238, The AmountUSDPerHour validator in Tranche.validateAmountUSDPerHour currently accepts non-canonical rationals because big.Rat.SetString parses decimals and exponent forms too. Tighten the validation so only the intended canonical RatString format is accepted, or normalize the parsed value before storing it; if you choose to keep broader parsing, update the documented contract and error text around validateAmountUSDPerHour and AmountUSDPerHour to match the actual accepted formats.
🤖 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/ladder.go`:
- Around line 190-221: The ramp fraction handling in computeStepAmount can panic
because NaN/Inf values may slip past RampSchedule.Validate and then reach
big.Rat.SetFloat64. Add an explicit non-finite check either in
RampSchedule.Validate or right before the conversion in computeStepAmount, and
reject or clamp invalid fractions before calling SetFloat64. Use the
RampSchedule.Validate and computeStepAmount symbols to keep the fix aligned with
the existing validation flow.
---
Nitpick comments:
In `@pkg/ladder/store.go`:
- Around line 226-238: The AmountUSDPerHour validator in
Tranche.validateAmountUSDPerHour currently accepts non-canonical rationals
because big.Rat.SetString parses decimals and exponent forms too. Tighten the
validation so only the intended canonical RatString format is accepted, or
normalize the parsed value before storing it; if you choose to keep broader
parsing, update the documented contract and error text around
validateAmountUSDPerHour and AmountUSDPerHour to match the actual accepted
formats.
🪄 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: a6f593be-742d-42fa-9b21-ee9a300e7792
📒 Files selected for processing (4)
pkg/ladder/ladder.gopkg/ladder/ladder_test.gopkg/ladder/store.gopkg/ladder/store_test.go
2d7f6cb to
03fffc4
Compare
…dule Split each Allocation into a buy-now purchase for the immediate ramp step and future Tranche rows per delayed step, so commitment terms expire staggered. Step amounts use exact big.Rat arithmetic with a last-step remainder and an overshoot clamp, so totals always reconstruct the gap exactly. Tranche rows are fully self-describing (Layer, Term, PaymentOption, amount) and clock plus ID generation are injected. Part of #1334.
…ring Reject empty or repeated IDs from the injected NewID across all tranches of a run, since SaveTranches upserts by ID and a collision would silently drop a scheduled purchase. Render step fractions with %.4g so small percentages no longer show as 0% on positive purchases, require a non-empty rationale on every input allocation, and align the tranche amount validation docs with what big.Rat SetString actually enforces.
Switch TrancheInput and Tranche Term/PaymentOption fields from raw strings to the ladder.Term and ladder.PaymentOption enums introduced on feat/ladder-types, validate them via the enum Validate methods instead of non-empty checks, and move all test fixtures to the typed constants. Unknown enum values are now rejected at the tranche-building boundary in addition to empty ones.
6b4eb50 to
328743e
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
BuildTranches(pkg/ladder/ladder.go): splits eachAllocationfromAllocateinto (a) a buy-nowActionPurchasefor the ramp step withAfterDays == 0and (b) futureTrancherows for delayed steps (FireAfter = Now + AfterDays), so commitment terms expire staggered instead of bunching. A fully delayed ramp (no day-0 step) is valid and produces zero buy-now actions.big.Ratstep arithmetic: every step amount is clamped to the remaining unallocated gap and the last step receives the exact leftover, so per-allocation totals reconstruct the gap exactly for everyRampSchedule.Validate-passing schedule, including fraction sets whose exact rational sum slightly exceeds 1 within the validator epsilon (regression-tested with{0.5, 0.5000000008, 1e-10}).Trancherows are now fully self-describing: newAmountUSDPerHour(losslessbig.Rat.RatString()encoding for DB round-tripping),Layer,Term, andPaymentOptionfields withValidatecoverage, so an executor can fire a tranche without any RunRecord/PlanJSON lookup; two equal-gap allocations on different layers stay distinguishable.Now) and ID generator (NewID), notime.Now()or randomness inside; fail-loud validation naming each offending field.Stacked on #1340 (which is stacked on #1338); auto-retargets as parents merge. Part of #1334 (phase 1 of #1333) - completes the phase-1 engine core.
Test plan
pkg/ladder(go test ./ladder/ -count=1, exit 0), including: exact staggering over a 3-step ramp with FireAfter date assertions, fully delayed and single-step schedules, multi-allocation determinism (double-run comparison with length asserts), inexact-fraction and tiny-gap remainder exactness, epsilon-overshoot clamping, tranche self-description (Layer/Term/PaymentOption stamping and distinguishability), sequential injected IDs, and 11 fail-loud validation subcases.go build ./...,go vet ./ladder/,gofmt -l ladder/(empty),golangci-lint run --config ../.golangci.yml ./ladder/...,gocyclo -over 10on non-test files (nothing over threshold).PlannedActionandTranchepasses its ownValidate(asserted explicitly in a shared test helper).Summary by CodeRabbit
New Features
Bug Fixes
Tests