fix(ladder): net in-flight tranches per row basis in on-demand-equivalent dollars

[cristim]

Summary

Net in-flight ladder tranches per row in on-demand-equivalent (OD) dollars, using each row's stored amount_basis and commitment_rate, instead of one raw SUM(amount_usd_hr).

Why

cloud-commitments-go#353 (fixing go#349) changed ladder.Allocate to work in commitment dollars. Every go commit since then removes AllocationInput.InFlightUSDPerHour and adds a per-row InFlight plus CommitmentRates. The platform cannot bump its go pin, which #799 needs, until its readers understand mixed bases. This PR makes the readers basis-aware first. The writers keep writing OD until the bump PR, so main is consistent at every step.

Changes

• config.LadderInFlightRow{Layer, CommitmentUSDPerHour, Rate}. LadderPlanBuilder now receives these rows, built by ladderNettingRows:
  • commitment rows carry their stored rate;
  • legacy OD rows carry rate 1;
  • a NULL basis still fails loud, as before.
• The OD total is summed exactly in big.Rat. It is rounded up only when the float is below the exact value, matching the existing nettingFloat rule, so netting is never understated.
• The handler adapter computes the OD total from the rows, so behaviour is unchanged for today's legacy-only data. A golden test proves the total is bit-identical.
• The exported GetInFlightLadderCommitUSDHr is deleted. It was a raw mixed-basis SUM with only test callers, and those tests are rewritten to assert the OD netting instead.
• Fired and settled-after-anchor rows are still netted, which errs toward under-buying. This is a deliberate difference from go's InFlight doc and is recorded in a comment.

Verification

• go build ./..., go vet ./... and go vet -tags integration ./... are clean.
• Unit tests pass for internal/config, server, purchase, analytics and mocks.
• Ladder integration tests pass against a native local PostgreSQL 17 on loopback. The testcontainers helper was replaced by a go test -overlay stub that hands out a fresh database; the repo helper is unchanged. Before the split, an unfiltered integration run of the combined A1+A2 change passed for config, server, purchase and analytics.
• Regression tests:
  • golden: legacy-only data gives a bit-identical total;
  • a rate "1/3" commitment row is seeded directly and its OD is never below the exact rational;
  • NULL netting still fails loud.
• Mutation: dropping the per-row rate division in nettingOnDemand is killed by TestLadderNetting_ExactRatesAndBoundary.
• Local golangci-lint could not run (go1.26-built binary against go1.27), so CI lint is authoritative.
• No money path changes. ClaimDueLadderTranches, PurchaseLayer and ReshapeBuffer have no production caller, and the ladder executor is not wired.

Stack: this is the base. #969 (amend ceiling) is stacked on it, then the timeline/editor units PR, then the go bump plus commitment-dollar writers. Refs #799, refs LeanerCloud/cloud-commitments-go#349. Related: #946.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 21 billable files and costs up to $5.25.}

• 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 8 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 64 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: 82639ab0-bb90-448d-b3f4-f0c7b403c9f4

📥 Commits

Reviewing files that changed from the base of the PR and between 3b088a4 and f4a286a.

📒 Files selected for processing: 21

• internal/analytics/collector_test.go
• internal/config/ladder_provenance.go
• internal/config/ladder_provenance_test.go
• internal/config/store_postgres_ladder.go
• internal/config/store_postgres_ladder_amend_ceiling.go
• internal/config/store_postgres_ladder_amend_ceiling_integration_test.go
• internal/config/store_postgres_ladder_basis_integration_test.go
• internal/config/store_postgres_ladder_lifecycle_integration_test.go
• internal/config/store_postgres_ladder_lock.go
• internal/config/store_postgres_ladder_lock_integration_test.go
• internal/config/store_postgres_ladder_netting_integration_test.go
• internal/config/store_postgres_ladder_settled.go
• internal/config/store_postgres_ladder_settled_integration_test.go
• internal/config/store_postgres_ladder_settled_plan_integration_test.go
• internal/config/store_postgres_ladder_test.go
• internal/mocks/stores.go
• internal/purchase/ladder_reconcile_integration_test.go
• internal/server/handler_ladder.go
• internal/server/handler_ladder_lock_integration_test.go
• internal/server/handler_ladder_test.go
• internal/server/test_helpers_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]

Independent review verdict (ladder stack #968 → #969 → #972): CLEAN

• fix(ladder): net in-flight tranches per row basis in on-demand-equivalent dollars #968 CLEAN at 201b877
• fix(ladder): convert amend ceiling amounts by the amended tranche's basis #969 CLEAN at 2c390a1
• fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972 CLEAN at c0ae02f

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief, covering each PR's own diff and the whole stack against main. The PR bodies were treated as claims. Coordinator: claude-cc-reviewer.

Blocking findings: none.

Non-blocking notes (follow-ups before the go bump):

• NULL basis is handled inconsistently across the stack.
  • Netting (nettingOnDemand, internal/config/ladder_provenance.go) fails loud on a NULL basis.
  • The amend ceiling (assumeOnDemandIfUnknown) and the fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972 timeline (ladderTimelineODMicro) treat NULL as OD.
  • This is not an understatement path. The timeline is display-only, and an amend can only succeed on a config whose next planning run would fail loud. NULL basis with a non-NULL rate fails loud in all three.
• store_postgres_ladder_amend.go:155 checkLadderRunTotal sums native amount_usd_hr. That is correct only while a run holds a single basis, which one Allocate call guarantees today. After the bump, a mixed-basis run would be silently wrong.
• fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972: the scope total runs an unbounded jsonb_agg over every in-scope tranche on each page request. It is fine at current volume and worth watching for performance.
• fix(ladder): convert amend ceiling amounts by the amended tranche's basis #969 adds a known-issues note that a future executor could claim a legacy OD row, tracked on fix(ladder): existing Savings Plan coverage is commitment dollars but the target is on-demand-equivalent #946.

What was attacked and how (local macOS):

• OD direction: checked against go at the pinned module (pkg@v0.0.0-20261015143912-740dfbd72456). ladder/allocate_units.go:84 does od = c / r, and allocate.go:36-44 defines CommitmentRates as commitment dollars per OD dollar, in (0,1]. The platform's total.Quo(total, rate) uses the same direction.
• Rounding never understates:
  • fix(ladder): net in-flight tranches per row basis in on-demand-equivalent dollars #968: LadderInFlightRow rounds the commitment up and the rate down. The exact big.Rat sum is then rounded up via nettingFloat.
  • fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972: od_usd_hr is rounded up to micros per row, and the total is the sum of the rounded rows from the same SQL statement that pages them.
  • Legacy rate-1 rows are bit-identical (TestLadderNetting_LegacyOnlyRowsKeepTheExactTotalBitForBit).
• fix(ladder): convert amend ceiling amounts by the amended tranche's basis #969 units: the old and new amounts are converted with the amended row's own basis and rate, then compared exactly against the micros ceiling. The UPDATE still writes native amounts, so OD is never written into a commitment column.
• fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972 frontend: the chart, table, cumulative and selection total use OD, while the editor and amend use native amounts. The od_usd_hr fallback applies only when the field is absent (an older API, all OD), so deploy skew is safe. A malformed rate gives ErrLadderTimelineRate, which maps to a 500 with a fixed message and no fallback.
• Deleted GetInFlightLadderCommitUSDHr: it has no callers left in the repo, with all build tags checked.
• Tests:
  • At each head, go build ./... && go vet ./... && go test -count=1 ./internal/config/... ./internal/api/... ./internal/server/... ./internal/purchase/... passes.
  • Integration at fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972 (Docker Postgres): go test -tags integration -run Ladder -v ./internal/config/ gave 79 PASS, 0 FAIL. The one SKIP, TestPostgresStoreDB_LadderArming, is unrelated. The mixed-basis timeline and amend-ceiling integration tests ran and passed.
  • Frontend at fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972: npx jest ladder passed 3330 tests (1 skipped), and npx tsc --noEmit is clean.
• Mutation probes (all killed): Quo→Mul (fix(ladder): net in-flight tranches per row basis in on-demand-equivalent dollars #968), dropping the netting round-up (fix(ladder): net in-flight tranches per row basis in on-demand-equivalent dollars #968), dropping the timeline round-up (fix(ladder): show timeline amounts in on-demand-equivalent dollars per row basis #972), and converting the amend amount without the row's basis (fix(ladder): convert amend ceiling amounts by the amended tranche's basis #969).

Gate: labels are set on each PR (they reference #799, with no closing issue). CI must be green at each head. Merge order is #968, then #969 rebased onto main, then #972 rebased onto main. Each rebase needs an empty git range-diff of the PR's own commits to keep this verdict.

[cristim]

Independent review at 201b877

Verdict: changes requested (one lint blocker, two test-strength gaps; no correctness defects found in the netting logic).

Findings

1. Blocker (CI), internal/config/store_postgres_ladder_lock_integration_test.go:127: unparam reports result 1 of the ok builder as always nil. The //nolint:unparam was moved onto the closing } line (line 129), but unparam reports at the ok := func(...) line. Fix: put //nolint:unparam // matches LadderPlanBuilder on the ok := func(...) { line.
2. Low, test gap, ladder_provenance.go:~122 (newLadderInFlightRow): no test asserts the plan's "legacy OD rows carry Rate 1 and CommitmentUSDPerHour == amount". exactOD masks the float fields, so changing the legacy rate to 1/2 survives. PR B feeds these floats to Allocate, so assert Rate == 1 and CommitmentUSDPerHour == amount for a legacy row (in TestLadderNetting_LegacyOnlyRows...).
3. Low, test gap: rate round-down direction only exercised with "1/3" (nearest float is already below). Flipping the Cmp(rate) > 0 check survives. Add "1/10" (nearest float is above) to TestLadderInFlightOnDemand_RoundsAwayFromUnderstating. Also untested: stored rate "1" and a rate so small that Float64() is 0 (the SmallestNonzeroFloat64 clamp).
4. Info: parseStoredCommitmentRate uses big.Rat.SetString, which also accepts "0.5", "+1/2", "0x1/2". Go's library spells rates as RatString ("1/2"); all are in (0,1] after parsing so the unit is still right, and the writer canonicalises. No action needed beyond awareness.
5. Info: LadderInFlightRow.exactOD is stale if a consumer edits the exported floats on a copy. Fine for PR A; PR B should not mutate rows.

Attacked

• Missed readers: git grep -i amount_usd_hr|AmountUSDHr across internal/. Remaining raw readers are checkLadderRunTotal (same-run native total, intentional), the timeline SQL (PR C), claim/reconcile scans (no netting), and sumTrancheRowAmounts (writer). No missed netting reader.
• Rate parsing: "0", "1", "2/1", "1/0", negative, whitespace, "1e1000000000", huge numerators. All either rejected or in (0,1]; no panic.
• Rounding: rate down, commitment up, OD summed in big.Rat then rounded up only when below exact; float-only fallback never understates.
• Golden test: it reimplements the main algorithm (exact Rat sum, then nettingFloat, which already exists on main), so it is a valid reference. Mutant ignoring exactOD is killed.
• NULL basis: netting still fails loud (mutant that assumes OD in netting killed).
• Deleted GetInFlightLadderCommitUSDHr: its callers are rewritten (store_postgres_ladder_test, lifecycle, settled, purchase reconcile rig) to assert the OD total, not dropped. They run via the assume-OD path, so the fail-loud path is covered by the netting tests.
• Locking: rows are read inside the config lock in READ COMMITTED, in-flight first then settled, order preserved.
• gocyclo: max 10 (PlanAndSaveLadderRun, unchanged), none above. store_postgres_ladder.go is 515 lines (543 on main, shrank). No new file over 500.
• git merge-tree of this head against current origin/main (b49b494): clean, no conflicts; head's merge-base is origin/main.

Mutation probes (applied to the #969 tree, which contains this PR's code)

Killed: rate division to multiply; no round-up in nettingFloat; ignore exactOD; netting assumes NULL; commitment rounded to nearest; drop settled rows from plan netting. Survived: legacy rate 1 to 1/2 (finding 2); rate rounding direction flipped (finding 3); rate "1" rejected (finding 3); rate clamp removed (finding 3).

Local verification (native local PostgreSQL 17 integration with synthetic data)

Fresh scratch worktree at this SHA stacked tree (#969 head, which includes these changes): GOWORK=off go build ./... OK; go vet -tags integration ./... OK; unit tests for internal/config, server, mocks, analytics pass; -tags integration -run Ladder for internal/config, internal/purchase and internal/server pass against my own native PostgreSQL 17 cluster via a go test -overlay stub for the testcontainers helper. golangci-lint could not run locally (toolchain mismatch), so CI lint is authoritative. This is not real-account verification.

[cristim]

Rebase + delta: verdict carried to the new heads

The stack was rebased onto main (picks up #960) by claude-cc-impl.

Range-diffs against the reviewed heads:

#968: 1: 201b877d = 1: f838030c fix(ladder): net in-flight tranches per row basis ...
      -: -------- > 2: f4a286af style(ladder): keep the unparam suppression on the builder func line
#969: 1: 2c390a18 = 1: 6cc1b901
#972: 1: c0ae02f3 = 1: 57c3dd56

Delta review of f4a286a (same independent reviewer): CLEAN.

• The commit is +2/-2 in internal/config/store_postgres_ladder_lock_integration_test.go. It moves //nolint:unparam // matches LadderPlanBuilder onto the func-literal line, where unparam reports. That is justified, because the closure must keep the LadderPlanBuilder signature.
• git diff f838030c f4a286af shows nothing else changed.
• golangci-lint v2.10.1 (the CI pin) with --build-tags integration ./internal/config/... reports 0 issues.

The CLEAN stack verdict above therefore covers #968 f4a286a, #969 6cc1b90 and #972 57c3dd5. Merge order is #968, then #969, then #972, each when its CI is green. (claude-cc-reviewer)