fix(ladder): bound amend raises by the config commitment ceiling (#842)

[cristim]

Closes #842

PR-B of the #842 plan (PR-A #849 settled_at, PR-C #850 skew margin are merged; #846 planner lock merged).

• AmendLadderTranche now runs inside withLadderConfigLock (advisory lock first, then FOR NO KEY UPDATE on config and run, tranche row after).
• A RAISE is refused unless in-flight (fired + scheduled, all runs, with the new amount) + completed tranches settled after ladderSettleAnchor(latest non-canceled run.started_at) <= C, where C = trunc(min(target_L, baseline_L * current target_coverage/100) - existing_L). Decrease, re-date-only and cancel skip the check. A NULL target/existing/baseline on the latest run refuses a raise. The original_total and per-run cap terms stay as defence in depth.
• Lock timeout (55P03) is now ErrLadderLockTimeout; the amend handler maps it to 503 with Retry-After: 5 (was a 500).
• TestLadderAmendmentAuditAndSiblingConcurrency fixture seeds target=10, existing=0, baseline=100.

Units blocker: today's target is on-demand-equivalent and E amortized commitment dollars. After go#349 tranche amounts are commitment dollars and target - E overstates the ceiling by 1/rate. Tracked for the commitment_rate PR (persist a unit-correct ceiling or refuse raises on rows with a commitment_rate); not fixed here. #843 (planner side) was the separate PR-C.

Verification (local PG17, the Docker-based setupLadderStore temporarily pointed at it; CI runs the normal path): 3-step replay (decrease A, planner tops up B, raise A) refused; completed tranche then raise refused; re-date-only with in-flight > C passes; coverage lowered after the run; gross existing; NULL snapshots; latest-run and canceled ordering; amend x planner with a barrier inside the build closure and pg_locks polling; skew test with literal offsets (30s counts, 2m does not). Mutations each fail a test: drop -old, count only scheduled, NULL snapshot allows a raise, ORDER BY ASC, remove the lock, drop the settled term, >=, margin 0, margin 1h. Removing the ceiling call (about main) fails 8 tests.

Also: the raise path first cancels scheduled rows past the 24h grace window (same sweep as the planner) so a later claim cannot fire a row the sum did not count; amounts above 1e12 USD/hr are refused (micro-dollar overflow guard). Accepted residual: the ceiling comes from the latest run's snapshot, so non-ladder purchases made after that run are not counted (same as drift). #118 item: the go#349 units conversion (commitment dollars vs on-demand target) must land before the executor.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Ladder tranche increases are now limited by the available commitment ceiling, helping prevent amendments that exceed the run’s supported capacity.
• Bug Fixes
  • Amendments now coordinate with ladder planning to avoid conflicting updates.
  • If planning is busy, the request returns a temporary service-unavailable response with guidance to retry after five seconds.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 1f6f03e6-e380-4ec1-acc2-df816ac07815

📥 Commits

Reviewing files that changed from the base of the PR and between b85908a and 2d7ed48.

📒 Files selected for processing (9)

• internal/api/handler_ladder_amend.go
• internal/api/handler_ladder_amend_test.go
• internal/config/interfaces.go
• internal/config/ladder_amend.go
• internal/config/store_postgres_ladder_amend.go
• internal/config/store_postgres_ladder_amend_ceiling.go
• internal/config/store_postgres_ladder_amend_ceiling_integration_test.go
• internal/config/store_postgres_ladder_amend_integration_test.go
• internal/config/store_postgres_ladder_lock.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

📝 Walkthrough

Ladder tranche amendments now acquire the per-config lock before row locks and validate increases against the config commitment ceiling. Lock timeouts return HTTP 503 with a retry message and header.

Changes: Ladder amendment validation

Layer / File(s)	Summary
Config locking and timeout handling internal/config/interfaces.go, internal/config/store_postgres_ladder_lock.go, internal/config/store_postgres_ladder_amend.go, internal/api/handler_ladder_amend.go, internal/api/handler_ladder_amend_test.go, internal/config/store_postgres_ladder_amend_ceiling_integration_test.go	Amendments resolve the matching config and acquire its lock before row locks. PostgreSQL lock timeouts return the typed ErrLadderLockTimeout; the API maps this error to HTTP 503 with Retry-After: 5.
Ceiling calculation and enforcement internal/config/ladder_amend.go, internal/config/store_postgres_ladder_amend.go, internal/config/store_postgres_ladder_amend_ceiling.go, internal/config/store_postgres_ladder_amend_ceiling_integration_test.go, internal/config/store_postgres_ladder_amend_integration_test.go	Raises are checked against a ceiling based on the latest non-cancelled run, current coverage, existing commitment, and in-flight and settled amounts. Integration tests cover ceiling accounting, snapshots, locking, overdue tranches, precision, and large amounts.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~40 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 2d7ed

The reviewed change has no established merge-blocking defect. The identified ceiling-unit concern does not apply to the current tranche amounts.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: enforcing the config commitment ceiling for ladder amendment raises.
Linked Issues check		Issue #842 requires a real-PostgreSQL regression and a config-wide amendment bound under the same per-config lock. The new integration tests include the three-step decrease, planner top-up, and raise …
Out of Scope Changes check		The changed files support the ladder-amendment bound or its operation. The lock-timeout sentinel and HTTP 503 mapping expose the lock behavior safely. The ceiling tests cover concurrency, snapshots, s…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

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

[cristim]

Gate verdict (Opus 5.5, independent adversarial review): REQUEST CHANGES

Reviewed SHA: 7d5c7bf1a24592e6683fe09865c00b54b56e43b3 (base main b85908ad, not behind). CI green and CLEAN at this SHA; not merged because of F1.

Blocking

F1. A raise can still end above C: the amend creates "counted by neither" rows that a later claim can fire. internal/config/store_postgres_ladder_amend_ceiling.go:58 sums in-flight at time.Now(), which drops scheduled rows past ladderGraceCutoff(now). Unlike PlanAndSaveLadderRun, the amend does not cancel those rows, so they are neither counted nor canceled. That breaks the invariant documented at store_postgres_ladder_lifecycle.go:16 ("every scheduled row is counted or canceled, never neither"). The planner closes this case with the overdue sweep in the same transaction (the ladder-lock A3 finding). The amend reopens it:

1. Run L (target 10, E 0) plans X=5 (date now-24h-30s, still in grace at L's asOf) and Y=5.
2. Amend Y 5 -> 10. X is past the grace cutoff at now, so it is excluded: committed 10 <= 10, allowed.
3. ClaimDueLadderTranches(asOf = now-1m) runs, for example from an executor tick whose now was captured before the lock wait or before earlier configs. It fires X.

Result: in-flight is 15 against C=10. I reproduced this on real PG17 with a throwaway test that uses only the store APIs (claimed 1 row(s); in-flight now 15.00 vs target 10). Rows sitting near grace expiry are expected under the b2 plan: an over-gap tranche "stays scheduled until grace expiry".
Today the exposure is latent, because ClaimDueLadderTranches has no production caller. Since this PR is a #118 blocker, the fix belongs here. Verified fix: on the raise path, capture now := time.Now() once, call cancelOverdueLadderTranchesTx(ctx, tx, &configID, now), then inFlightLadderCommit(ctx, tx, configID, now). With that change the probe passes and all TestLadderAmendCeiling* and TestLadderAmendmentAudit* tests still pass. Please add the probe as a regression test (sketch: plan Y=5 with target 10; insert a scheduled X=5 at now-LadderFireGrace-30s on the same run; raise Y to 10; claim with asOf=now-1m using newExecCreator; assert in-flight <= 10).

Non-blocking follow-ups

• F2 store_postgres_ladder_amend_ceiling_integration_test.go:206: release is not registered with t.Cleanup. When the assertion fails, the held connection makes the pool close hang, and the test fails by a 10-minute go test timeout instead of a clean failure (seen with mutant M14).
• F3 Mutant trunc -> round at :44 survives: no test pins the rounding direction. The rounding is floor for positive C. For negative C, trunc rounds toward zero, which is harmless because a raise always makes committed > 0.
• F4 :17 int64(math.Round(v*1e6)) overflows at amounts >= ~9.2e12 USD/hr (the amount pattern allows 14 integer digits). Only the original-total term guards against this today. Comparing in SQL numeric would remove the risk.
• F5 Units (go#349): this is tracked only in the PR body and as "units/cap checks" in b2 PR4b. Make it an explicit fix(ladder): tranche status is never updated, so in-flight netting grows and planning silently stops #118 item: persist a unit-correct ceiling, or refuse raises on rows that have a commitment_rate.
• OWNER: the ceiling is the run-L snapshot. Non-ladder purchases made after run L are not in E_L, so a raise in that window can push coverage above target until the next run. Same class as planner staleness; accept or decide.

Verified (all PASS except F1)

• (1) Amend runs in withLadderConfigLock: the advisory lock first, then config/run FOR NO KEY UPDATE, then the tranche FOR UPDATE. Tranche writers: insert (planner), overdue sweep, transition, claim, amend. Only amend changes amount or date. Immediate tranches are persisted rows (scheduled at run start), are counted, and cannot be amended.
• (2) Counter-examples checked and covered: multi-run, a tranche completed and then a sibling raised, coverage lowered after the run, coverage raised (target_L still binds), canceled latest run, ties (started_at, created_at, id DESC), failed/canceled tranches not counted, gross E (conservative), and NULL snapshots refusing a raise. The only path I found that ends above C is F1.
• (3) 55P03 -> ErrLadderLockTimeout -> 503 with Retry-After: 5. Store and handler tests exist (mutants M14 and M15 are killed).
• (4) Fails-before: the PR's tests run against main's code fail 8 top-level tests (3-step replay, T6, coverage, NULL snapshot, latest run, barrier: "expected 1 waiter", skew 30s). The fixture test passes on both. At head everything passes. The barrier blocks inside the build closure and polls pg_locks filtered by this lock's classid/objid. The skew test uses literal 30s/2m offsets.
• Mutations (16; every one fails a test except M12): drop -old, count only scheduled, NULL snapshot allows a raise, ORDER BY ASC, WithTx instead of the lock, drop the settled term, >=, margin 0, margin -1h, include canceled runs, re-date treated as a raise, drop the coverage term, drop the ceiling call, no 55P03 map, no 503 map, drop existing. Survivor: trunc -> round (F3).
• (5) The TestLadderAmendmentAuditAndSiblingConcurrency fixture adds only snapshot columns. Its original-total and sibling assertions are unchanged.
• (6) Lock order: advisory lock before rows everywhere. The reconciler and retry linker lock only tranche/execution rows and never the config/run rows. FK inserts take KEY SHARE, which is compatible with NO KEY UPDATE. No deadlock cycle.
• Local evidence (macOS, git archive at the exact SHA, GOTOOLCHAIN=go1.26.9, local PG17 standing in for the Docker helper, under the suite lock): build, vet, and unit tests for config/api/server exit 0. All Ladder integration tests: 128 PASS (1 pre-existing env-gated skip). New tests run 5x: ok. Linux CI Integration Tests ran and passed every new test. git-secrets (--register-aws) clean on BSD and GNU grep.

[cristim]

Re-gate verdict (Opus 5.5): APPROVE at 2d7ed48f951e4313c15bd64a71990dc2d2243790

The diff since 7d5c7bf1 touches only store_postgres_ladder_amend_ceiling.go and its integration test.

• F1 fixed. On the raise path, checkLadderConfigCeiling captures now once, then runs cancelOverdueLadderTranchesTx(ctx, tx, &configID, now) and then inFlightLadderCommit(..., now). Both run in the same transaction under withLadderConfigLock. A failed amend rolls the sweep back.
  • My repro passes on real PG17: plan Y=5 against target 10, add X=5 at now-24h-30s, raise Y to 10, then claim with asOf=now-1m. Result: claimed 0 row(s); in-flight 10.00.
  • The regression test TestLadderAmendCeiling_CancelsOverdueBeforeCounting fails when the sweep is removed (mutant killed). Moving the sweep after the sum is equivalent, because both use the same now and every row is still either counted or canceled.
• F2. The test now registers t.Cleanup(release), so a failed assertion no longer hangs on the held lock.
• F3. TestLadderAmendCeiling_CeilingTruncatesNotRounds kills the trunc -> round mutant.
• F4. Values above 1e12 USD/hr are refused before the micro-dollar conversion. TestLadderAmendCeiling_HugeAmountRefused kills the mutant that removes the guard. The largest possible sum is 3e18, which stays below the int64 limit.
• F5 and the owner note. The PR body now records the units item as a fix(ladder): tranche status is never updated, so in-flight netting grows and planning silently stops #118 blocker and the snapshot residual as accepted.

Local evidence:

• Setup: macOS, git archive at the exact SHA, GOTOOLCHAIN=go1.26.9, local PG17 standing in for the Docker helper, run under the suite lock.
• Build, vet and unit tests for config/api/server exit 0.
• Ladder integration tests: 131 PASS, plus 1 skip that was already env-gated before this PR.
• The amend, lock and plan tests pass 5 times in a row.
• git-secrets is clean with both BSD and GNU grep.

CI is fully green at this SHA, including Linux Integration Tests, which ran the 3 new tests. The PR is CLEAN.