fix(ladder): plan the gap under the per-config lock

[cristim]

Refs #118

Stacked on the PR 1a branch (feat/ladder-lock-store); retarget to main once 1a merges.

What

Switches executeLadderRun to PlanAndSaveLadderRun: Allocate, BuildTranches and plan assembly run in the locked transaction (ladderRunPlanner.plan), so two overlapping runs for one config cannot both plan the same gap. Provider reads (baseline, layer states) stay before the transaction; no lock is held across network I/O. Removes SaveLadderRunWithTranches and GetInFlightLadderCommitUSDHr from the interface and mocks (the handler had the only production caller; the concrete store methods remain for tests).

#842 and #843 are separate executor blockers and are not addressed here.

Verification (macOS, real Postgres 17 throwaway cluster)

• New TestLadderLock_ConcurrentRunsPlanTheGapOnce: goroutine 1 is held inside the locked builder, pg_locks shows goroutine 2 waiting on this config's key, then released; total live tranches = 10 (target). With the lock statement replaced by a no-op the same test fails: "second run planned while the first still held the lock" and total 20 vs 10.
• All 23 TestLadder* server integration tests pass; handler unit tests pass.

Summary by CodeRabbit

• Bug Fixes
  • Ladder runs for the same configuration are now coordinated, so overlapping runs account for the latest in-flight commitments instead of planning against the same available capacity.
  • Ladder plans and their scheduled tranches are saved together, keeping run records and tranche schedules consistent.

[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: 106bd0a2-92da-4fcf-b8f6-029517ec1bb1

📥 Commits

Reviewing files that changed from the base of the PR and between f606770 and 8e9c358.

📒 Files selected for processing (7)

• internal/analytics/collector_test.go
• internal/config/interfaces.go
• internal/mocks/stores.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

💤 Files with no reviewable changes (3)

• internal/analytics/collector_test.go
• internal/mocks/stores.go
• internal/server/test_helpers_test.go

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

📝 Walkthrough

Ladder planning and persistence now use one store operation. The server supplies a planner callback, which receives the in-flight commitment under the config lock. Updated tests cover the callback flow and concurrent runs.

Changes: Ladder planning and persistence

Layer / File(s)	Summary
Combined planning contract and mock updates internal/config/interfaces.go, internal/mocks/stores.go, internal/analytics/collector_test.go, internal/server/test_helpers_test.go	StoreInterface replaces the separate in-flight lookup and run-save methods with PlanAndSaveLadderRun. Related mocks remove the replaced methods.
Server planner and serialized-run tests internal/server/handler_ladder.go, internal/server/handler_ladder_test.go, internal/server/handler_ladder_lock_integration_test.go	executeLadderRun passes a planner callback to the combined store operation. The callback builds and validates run and tranche rows. Tests cover the updated adapters and verify that concurrent runs for one config serialize.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Execute as executeLadderRun
  participant Store as Config store
  participant Planner as ladderRunPlanner
  participant Database as PostgreSQL
  Execute->>Store: Call PlanAndSaveLadderRun
  Store->>Database: Acquire config lock and read in-flight commitment
  Store->>Planner: Call plan with in-flight amount
  Planner-->>Store: Return run and tranche rows
  Store->>Database: Persist run and tranches
  Store-->>Execute: Return persisted run

Loading

Merge Risk: ⚪ Minimal · up to 8e9c3

The planning and persistence path meets the intended serialization contract. No issue identified here needs to be resolved before merge.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 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: planning the ladder gap under the per-config lock.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• 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 independent adversarial review, money path): APPROVE at 8e9c3586b62759b7288669d7bbf6b9c8697d62b9

Head: author's merge of origin/main (#845 squash f606770). The +/- lines of git diff origin/main 8e9c3586 are identical to the reviewed git diff e64c030d dd870c02 (+222/-215, 7 files); nothing else changed.

Review:

• handler_ladder.go:149-165: executeLadderRun keeps GetUsageBaseline/GetLayerStates (provider I/O) before the tx, then calls only PlanAndSaveLadderRun(ctx, dbCfg.ID, now, in.plan); no other persist path remains.
• ladderRunPlanner.plan/buildRows (:187-366) is the old Allocate/BuildTranches/assemble/persist body moved verbatim: same Allocate inputs (Now, Baseline, LayerStates, SupportedLayers, engineCfg, DataSources, InFlight), same BuildTranches inputs, scope == engineCfg.Scope, same run row fields and TotalHourlyCommit. Pure compute (uuid, marshal); no I/O under the lock. Only difference: the impossible nil-inFlight branch is gone (store returns float64).
• SaveLadderRunWithTranches and GetInFlightLadderCommitUSDHr have no remaining non-test caller (grep); both stay only on *PostgresStore.

Local evidence (macOS, throwaway PG17 cluster, git archive at the exact head, GOTOOLCHAIN=go1.26.9):

• build + go vet -tags integration: ok; go test ./internal/...: ok
• go test -tags integration ./internal/config/: ok; ./internal/server/ -skip TestServerIntegration: ok (that test needs Docker; Linux CI covers it)
• TestLadderLock_ConcurrentRunsPlanTheGapOnce x5: PASS (pg_locks filtered by this key's classid/objid)
• Fails-before reproduced (on the same change merged over feat(ladder): per-config persist lock and PlanAndSaveLadderRun #845's head): lock statement replaced by a no-op -> second run planned while the first still held the lock and Max difference between 10 and 20; in-flight read skipped inside the tx -> SUM 20 vs 10.
• git-secrets --register-aws, 7 files: BSD rc=0, GNU grep 3.12 rc=0
• CI: all jobs green at this SHA, mergeStateStatus CLEAN

#842 and #843 remain executor blockers (out of scope).