[IAmJSD]

Why

Two review follow-ups:

• The Go doc comment on analytics.MonthlySummary.TotalCommitment / TotalUsage described them as "committed spend and covered usage". The openapi total_commitment / total_usage descriptions say something different: upfront cost amortized per month, and recurring monthly fees, with committed spend as their sum (feat(frontend): Trends view consuming GET /api/analytics/trends #30).
• GetLadderConfigs is now capped at config.MaxListLimit (1000) (chore(config/db): medium/low findings from the 2026-07-28 full review #120). The scheduled ladder run still had a comment saying it reads all rows. If a run reads exactly the cap, the older configs past the cap are not run, and nothing logged that.

What

• internal/analytics/interfaces.go: the MonthlySummary comment now says TotalCommitment is the upfront cost amortized per month, TotalUsage is the recurring monthly fees, and committed spend is their sum. The existing notes on run-rate averaging and nil-means-unknown are kept.
• internal/server/handler_ladder.go: handleLadderRun logs a warning (ladder configs may be truncated) when GetLadderConfigs returns config.MaxListLimit rows or more. The comment now says the read is capped at MaxListLimit, newest first.
• internal/config/interfaces.go: the ConfigStore.GetLadderConfigs comment says the same thing.
• internal/server/handler_ladder_test.go: TestHandleLadderRun_WarnsWhenConfigsHitListCap uses a store fake. When it returns exactly MaxListLimit configs the run must warn; with one fewer it must not.

The GET ladder-config listing in internal/api/handler_ladder.go reads the same capped list. It is unchanged here.

Verification

Linux, Go from ~/.local/go, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true, on head b358cf41 (origin/main 76180820 plus one commit):

• Fail-before: with internal/server/handler_ladder.go from origin/main, the at the cap subtest fails (expected: true, actual: false). It passes with this change. The under the cap subtest passes on both.
• go build ./... passed.
• go vet passed on internal/server, internal/analytics and internal/config, with and without -tags integration.
• go test -count=1 passed on the same three packages.
• golangci-lint found 0 issues on those packages.
• gocyclo -over 10 reports nothing on the changed non-test files.

Combined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree: queue/platform @ 6144e322 = origin/main f50f1d3e plus these heads, merged in this order:

Branch	Tested head
fix/issue-712-gcp-azure	9569af9
fix/issue-134-mfa-token-lookup	7c78a0b
chore/review-followups-1028	b358cf4

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers.

• Passed: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks: golangci-lint v2.10.1 found 0 issues on every changed package, with and without --build-tags integration. gocyclo -over 10 is silent on the changed non-test files.
• Integration tests, every changed package that has them, one package at a time (-tags integration -count=1): internal/analytics, internal/auth, internal/config and internal/server all pass.
• Secrets: git-secrets on the changed files found nothing, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh passes.
• Terraform: terraform fmt -check and validate (init -backend=false) on the touched modules; .tftest.hcl files run in CI. Nothing was applied to any cloud.

Refs #30
Refs #120

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: f50f1d3
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3e037cfb-1dcf-4ea2-9084-6f329b602ef1

Commits

Reviewing files that changed from the base of the PR and between f50f1d3 and b358cf4.

Files selected for processing: 4

• internal/analytics/interfaces.go
• internal/config/interfaces.go
• internal/server/handler_ladder.go
• internal/server/handler_ladder_test.go

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE b358cf412df8c24b4dbe5ee08765121438a67b98

Blocking:

• none

Non-blocking:

• internal/server/handler_ladder.go:113: the warning only makes the truncation visible. With more than 1000 ladder_configs rows, the oldest configs past the cap (ORDER BY created_at DESC, id LIMIT 1000) still never run on the scheduled ladder. That is fine for a follow-up under "Refs chore(config/db): medium/low findings from the 2026-07-28 full review #120", but if anyone is likely to reach 1000 configs, the real fix is to page through all the rows in handleLadderRun. The warning also fires at exactly 1000 rows when nothing was dropped, which the "may be truncated" wording already allows for.

Attacked:

• I checked the new comments against the code. GetLadderConfigs (internal/config/store_postgres_ladder.go:25-36) really is ORDER BY created_at DESC, id LIMIT MaxListLimit. The new MonthlySummary comment matches the openapi total_commitment / total_usage descriptions: upfront amortized per month, recurring fees, committed spend = sum, nil = unknown, and a known zero is 0.
• The warning check comes before the empty check and doesn't change the run's control flow or result. The test's disabled configs still flow into SkippedDisabled.
• Ran at b358cf4: go build ./...; go vet on internal/server, internal/analytics and internal/config, with and without -tags integration; go test -race -short -count=1 on those three packages (ok); golangci-lint v2.10.1 on those three packages (0 issues); gocyclo -over 10 internal/server/handler_ladder.go (silent).
• Fail-before: with internal/server/handler_ladder.go from origin/main, TestHandleLadderRun_WarnsWhenConfigsHitListCap/at_the_cap fails. It passes with the change restored.

Body-stale:

• none

_{Reviewer: rJ1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at b358cf412df8c24b4dbe5ee08765121438a67b98 merged onto main (f50f1d3e), together with #1573, #1574, #1575 in that order (combined tree 1f343bb5, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on b358cf41, green CI, and the combined-tree verification above. Squash-merged at exactly b358cf412df8c24b4dbe5ee08765121438a67b98.