Skip to content

ladder: buy-now ramp steps escape netting + follow-up hardening for L5 convergence #36

Closed

Description

@cristim

Adversarial review of LeanerCloud/cloud-commitments-cli#1397 (L5 tranche-netting convergence, verdict SHIP) surfaced follow-up items that are out of scope for that PR:

1. Buy-now ramp steps are invisible to netting (most important). Ramp steps with AfterDays==0 become buy-now PlannedActions, not ladder_tranches rows, so they are neither summed by GetInFlightLadderCommitUSDHr nor reflected in existing-commitment E until executed. In email_approval mode a config with an immediate ramp step can re-plan the same buy-now amount on consecutive daily runs while an approval is still pending. Not introduced by LeanerCloud/cloud-commitments-cli#1397 (plan-only phase; the daily cadence gate bounds re-plans to 1/day), but it belongs with the documented drift-down follow-up. Fix direction: either persist buy-now amounts as same-day scheduled tranche rows so netting sees them, or net pending (unapproved) buy-now PlannedActions from prior runs into the gap.

2. Untested fail-loud branches. ladderTestStore.inFlightErr/inFlightUSDHr knobs (handler_ladder_test.go:128-129) are never exercised, so handler_ladder.go:326-333 (in-flight fetch error + nil handling) and the engine's nil-InFlight validation error (allocate.go:203) have no direct test. Branches are correct but should get 2 small regression tests.

3. No serialization between the netting read and the append. GetInFlightLadderCommitUSDHr runs outside the persist transaction; two truly concurrent runs of the same config could both read a stale in-flight sum and jointly overshoot target-E. Theoretical today (single scheduled Lambda + advisory lock + cadence gate), but must be closed before execution/concurrency lands (e.g. read in-flight inside the persist tx, or an advisory lock keyed on config_id spanning read+append).

Source: independent Fable adversarial review on LeanerCloud/cloud-commitments-cli#1397. All three are design/hardening items, not regressions.

Activity

  1. cristim commented on Jul 28, 2026

    @cristim
    MemberAuthor

    Additional evidence from the 2026-07-28 money-core review (reviewed commit be11bdcb5, origin/main). Confirmed still live on origin/main; filing here rather than as a duplicate issue.

    Where: pkg/ladder/ladder.go:209-224 (appendStep routes AfterDays == 0 to result.BuyNow) vs internal/server/handler_ladder.go:437-465 (persistLadderRun persists only trancheResult.Tranches). git grep BuyNow over non-test code returns no consumer.

    What: the AfterDays == 0 share of every allocation is produced, validated and dropped. Meanwhile allocTotalHourlyCommit (handler_ladder.go:645-648) and assembleLadderPlan's ActionPurchase entries (handler_ladder.go:558-568) both report the full allocation gap, so the persisted plan and run row claim commitment that nothing will ever place.

    Failure scenario: ramp [{AfterDays:0,Fraction:0.5},{AfterDays:30,Fraction:0.5}], gap $10/hr. Run 1 writes total_hourly_commit=10 and a plan JSON saying "purchase $10.00/hr", but persists only a $5/hr tranche. GetInFlightLadderCommitUSDHr returns 5, so run 2 re-opens a $5 gap and again drops half of it. The operator-facing plan and the dashboard total permanently overstate what will be committed, and the intended immediate step never happens.

    Fix direction: persist buy-now steps as tranches with FireAfter = Now (or execute them inline) so they are both placed and counted by the netting query.

  2. cristim commented on Oct 16, 2026

    @cristim
    MemberAuthor

    Checked against main 9fe0d74: item 1 and the comment are fixed by #351 (ad67030). pkg/ladder/ladder.go:43-56,263 emits a Tranche for every ramp step, TrancheResult.BuyNow is gone, and the go half of item 2 is pinned by TestAllocate_NilInFlightRejected (pkg/ladder/allocate_test.go:1904). What remains is platform-side: the handler_ladder.go plan/total reporting, the untested inFlightErr/inFlightUSDHr branches and the read/append serialization (item 3), plus the platform#118 tranche lifecycle that the netting depends on. Marking needs-info: close here and track the rest in the platform repo, or keep open.

  3. cristim commented on Oct 20, 2026

    @cristim
    MemberAuthor

    Verified on current main: the buy-now step is now a Tranche (pkg/ladder/ladder.go:38-56, #351), a nil InFlight is rejected (pkg/ladder/allocate.go:232, TestAllocate_NilInFlightRejected), and the read/append serialization is fixed in platform by the per-config lock in PlanAndSaveLadderRun. The remaining platform work is tracked in LeanerCloud/cloud-commitments-platform#1180 (untested in-flight fetch error branch) and platform#118 (tranche lifecycle). [claude-cc-libs]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions