[IAmJSD]

Root cause

getHistory built summary with summarizePurchaseHistory(all), which only sees the page: at most limit purchase_history rows, 100 terminal executions and 1000 actionable ones. With more rows than a cap, the cards under-report, and truncated only says so.

Fix

• One store method. HistorySummaryAggregate (internal/config/store_postgres_history_summary.go) runs one grouped COUNT/SUM per source and returns groups, not totals:
  • purchase_history, grouped by (account_id, cloud_account_id, provider, revoked). Upfront is NULL when any row in the group has an unknown cost, the same way sumKnownCosts handles it.
  • purchase_executions, grouped by (status, cloud_account_id, created_by_user_id, ordered [provider, cloud_account_id] of each recommendation).
  • There's one group per account/provider/status combination, so the result size doesn't grow with row count.
• Shared SQL predicates.
  • purchaseHistoryFilterWhere is now the only WHERE builder for both GetPurchaseHistoryFiltered and the aggregate.
  • historyExecutionsPredicate is shared with GetExecutionsByStatuses.
  • On the handler side, historyFilters.purchaseHistoryFilter() builds the store filter for both the row fetch and the aggregate.
• Same scope check, same code. The row path applies the account scope in Go: allowed.Allows(accountID, historyRowAccountName(...)), plus the ambient-row ownership gate. That logic now lives in historyScope.allows. It is resolved once per request and applied to the listed rows and to every aggregate group.
  • Execution groups are rebuilt into a PurchaseExecution and passed through the same matchesExecutionProviderAndAccount and executionToHistoryRow the list uses.
  • Group totals are only added up after the scope check, so a scoped user never gets counts from another tenant. Nothing is returned for groups outside the scope.
  • I did not rewrite the scope check in SQL. That would be a second copy that could drift from the Go one.
• Same bucketing rules. summarizePurchaseHistory now loops over addHistoryRowToSummary(row, n), and the aggregate calls that same function for each group. Its rules are unchanged:
  • revoked rows have no dollars
  • audit-gap and partially completed rows count as completed, with no dollars
  • canceled rows count only toward total_purchases
  • pending/notified/scheduled count as pending
  • approved/running/paused count as in progress
• Concurrency.
  • Both aggregate queries run in one REPEATABLE READ, READ ONLY transaction. Without it, a purchase moving from a running execution to a purchase_history row mid-read gets counted 0 or 2 times; there's a race test for this.
  • The stale-approval sweep is now deferred. On Lambda it used to run synchronously before the summary, so the cards would have counted as expired the rows the list still showed as pending.
• Response. HistoryResponse.summary_complete (also in openapi.yaml) is true when the summary covers every matching row. If the aggregate query fails, the summary falls back to the page with a warning log and summary_complete: false, instead of failing the whole History load. The openapi HistorySummary schema also lists the per-bucket counters it was missing.
• Cost. One grouped scan per table. The default date window is unchanged and there is no new index. An unfiltered view scans all of purchase_history, which is bounded by the number of purchases ever made.
• Interface. Implemented in internal/mocks/stores.go, internal/analytics/collector_test.go and internal/server/test_helpers_test.go. With no expectation set, the shared mock returns ErrHistorySummaryAggregateNotStubbed, so older handler tests keep checking the page summary through the fallback.

Closes #675

Verification

All on macOS against real Postgres via testcontainers (local integration evidence). Tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because this machine's default AWS profile is broken. There's no real-cloud or deployed-stack coverage; the change is DB and handler only.

Fail before, pass after. I ran TestIntegration_HistorySummary_TotalsCoverAllInScopeRows against a git archive origin/main copy with only the new test file added.

• Seed: two tenants. Each has 11 purchases (2 revoked, 3 legacy rows with no cloud_account_id), 105 canceled executions (over the 100 terminal cap), and pending/notified/scheduled, approved/running/paused, failed, expired, audit-gap completed, partially_completed, clean-completed (excluded), canceled and web-bulk rows. There are also ambient executions owned by the scoped user and by another user. The request uses limit=5.
• On main:
  • scoped user total_purchases expected 130, got 61
  • admin expected 260, got 120
  • total_completed, upfront and savings also wrong
  • summary_complete missing
• On this branch it passes. The scoped user gets exactly the manual sum over tenant-A rows plus their own ambient row, the admin gets the sum over all rows, and a scoped user filtering on tenant B gets zero.

Drift guards.

• TestIntegration_HistorySummary_MatchesListedRowsUnderTheCap (api): with every in-window row listed, summary == summarizePurchaseHistory(listed rows) for scoped and admin viewers across date, provider aws/azure, account A/B and legacy account_id filters, including rows exactly on and 1µs outside the date edges.
• TestIntegration_HistorySummaryAggregate_MatchesRowFetch (config): aggregate groups == uncapped GetPurchaseHistoryFiltered / GetExecutionsByStatuses rows grouped in Go, across 7 filter combinations.
• TestIntegration_HistorySummaryAggregate_DateBoundsInclusive: the SQL date bound is inclusive at both ends, the same as the Go check, and JSON-null recommendations project to an empty list.
• TestIntegration_HistorySummaryAggregate_SnapshotUnderConcurrentCompletion: a writer moves one purchase between a running execution and a purchase_history row while 1500 aggregates run, and each must count it exactly once.
• Unit (mock):
  • the aggregate gets the scope applied
  • the aggregate's purchase filter equals the row fetch's filter
  • the summary falls back when the aggregate fails
  • on Lambda the sweep runs after the aggregate
  • no listed execution status can add dollars, which is why rebuilt execution groups are safe
  • a group of n rows equals n single rows

Mutation probes. Each one was applied by hand, run, and reverted.

• Removing SET TRANSACTION ... REPEATABLE READ: race test fails ("counted 0 times").
• Skipping the scope check on execution groups: 3 tests fail.
• Shifting the SQL date bound by a day: 3 tests fail.
• Dropping the purchase_history WHERE from the aggregate: 3 tests fail.

go build ./...
go vet ./internal/api/ ./internal/config/ ./internal/mocks/ ./internal/analytics/ ./internal/server/
go vet -tags integration ./internal/api/ ./internal/config/
go test ./internal/api/ ./internal/config/ ./internal/mocks/ ./internal/analytics/ ./internal/server/ -count=1
go test -tags integration ./internal/config/ -run 'TestIntegration_HistorySummaryAggregate|TestIntegration_GetExecutionsByStatuses|History|PurchaseHistory' -count=1
go test -tags integration ./internal/api/ -run 'TestIntegration_HistorySummary_|History' -count=1
golangci-lint v2.10.1 run --allow-parallel-runners --build-tags integration ./internal/{api,config,mocks,analytics,server}/...   # 0 issues
gocyclo -over 10 -ignore "_test\.go" <changed non-test files>   # silent
bash scripts/gosec-hook.sh <changed files>                       # exit 0
git-secrets --scan (fresh temp repo, isolated HOME, repo patterns) # clean; scripts/test-git-secrets-allowlist.sh 42 PASS

Out of scope / notes

• Pre-existing scope behavior, kept as is. For purchase_history rows the Go scope check compares the row's external account_id to the scope entries, so a scope that lists only an account UUID matches those rows only through the unique-display-name path. The aggregate copies this behavior instead of changing it. Open PR fix(auth): bind group account scope entries to account IDs #1116 (bind group scope entries to account IDs) is the place to change it.
• fix(history): pending approvals can fall off the first page because filters are applied after the executions cap; add paging and push provider/account/date filters into the executions SQL #632 is not done here. It covers pushing the provider/account/scope predicates into the executions row SQL, and paging. The executions row fetch still filters in Go after the cap.
• Separate reads. The row list and the aggregate are still two reads. A row committed between them, or a sweep fired by a concurrent request, can make the cards differ from the list by that row until the next load.
• JSON keys. The execution grouping reads the lowercase JSON keys provider / cloud_account_id, which is what json.Marshal writes for RecommendationRecord. encoding/json matches keys case-insensitively, so hand-written JSONB with other casing would match the list but not the aggregate.

Frontend follow-up for the UI owner

No change under frontend/ in this PR. The cards already read summary.*, so they show the full totals with no UI change. Suggested follow-up in frontend/src/history.ts renderTruncationNotices and frontend/src/types.ts:

• Add summary_complete?: boolean to HistoryResponse.
• When truncated && summary_complete, change the banner to say only that older rows aren't listed, and that the summary cards still cover all matching purchases. Drop "and totals may be incomplete".
• Keep the current wording when summary_complete is false.

Maintainers: please mirror the issue's triage labels on this PR: priority/p2, severity/medium, urgency/this-quarter, impact/many, effort/m, type/feat, triaged.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• 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]

Independent review 2 (claude-fleet, cold): APPROVE abec6cd

Findings (none blocking):

• [non-blocking, pre-existing] frontend/src/history.ts:270 and frontend/src/api/history.ts:22 vs internal/api/handler_history.go:739 (parseHistoryFilters) - plan_id is sent but never parsed, so "View plan history" lists every purchase.
  • Before this PR, the plan view's cards summed only the page. Now they show tenant-wide all-time totals, which makes the bug more visible.
  • This is not this PR's bug, but it is worth a known-issues entry or a follow-up issue.
• [non-blocking, test gap] internal/config/store_postgres_history_summary.go:85 - The NULL-upfront rule (CASE WHEN COUNT(upfront_cost)=COUNT(*)) is pinned only by the config test. The api-level integration seed has no unknown-cost rows, so probe P2b survives there.
• [non-blocking] internal/config/store_postgres.go:2116 vs store_postgres_history_summary.go:96-101 - If one execution row's recommendations JSON will not decode, GetExecutionsByStatuses errors and the handler drops that whole class from the list.
  • The aggregate still counts the row and reports summary_complete=true.
  • The same holds whenever a class listing fails. This is an edge case, and the handler logs it.
• [nit] store_postgres_history_summary.go:132-137 - Begin followed by SET TRANSACTION ... works, and the race probe shows the snapshot holds. BeginTx(pgx.TxOptions{IsoLevel: RepeatableRead, AccessMode: ReadOnly}) would do it in one call, if s.db exposes it.
• [non-blocking] The cost claim holds for the UI only, because the frontend sends a 7-day window by default (history.ts:228).
  • A no-date API call runs the jsonb projection over every purchase_executions row in the listed statuses.
  • The purchase_history half was already a full scan.

Against #675: the summary now comes from grouped SQL, and the groups go through the same code the list uses: historyScope.allows, matchesExecutionProviderAndAccount, executionToHistoryRow and addHistoryRowToSummary.

• The group keys cover every field those functions read.
  • historyRowAccountName reads CloudAccountID, AccountID and Provider.
  • accountMatchesFilters and executionProvider read exec.CloudAccountID and the rec provider/cloud_account_id.
  • The ambient gate reads CreatedByUserID.
• Execution groups can never add dollars. Completed and partially_completed set IsAuditGap, and every other status lands in a non-dollar bucket.
• The JSON nil, "" and absent cloud_account_id cases are equivalent through collapseRecommendationAccount.
• The legacy account_id is folded into AccountIDs before purchaseHistoryFilter(), so the no-filter fast path matches too (GetAllPurchaseHistory has no WHERE).

Attacked:

• Deferred sweep: staleExecs is bound when the defer is set, and the error paths behave as before.
• Blast radius:
  • All 4 StoreInterface implementers are updated: mocks, server, the analytics test mocks and postgres.
  • filterPurchaseHistoryByAllowedAccounts is also called by handler_inventory.go:48,191, and its behaviour there is unchanged.
• Mutation probes on a scratch copy, each restored and confirmed identical with cmp:
  • P1, scope skipped on execution groups: FAILS (scoped 249 vs 130; tenant-B filter 118 vs 0).
  • P2, the NULL-upfront CASE dropped: the config test FAILS. P2b at the api level survives (the gap above).
  • P3, <= changed to < on the end bound: FAILS.
  • P4, the sweep not deferred: FAILS (the Lambda ordering unit test).
  • P5, purchaseGroupRow CloudAccountID=nil: survives. This is effectively an equivalent mutant, because name resolution falls back to provider plus external ID.
  • P6, the execution date bound dropped: FAILS.
  • P7, READ ONLY without REPEATABLE READ: FAILS ("counted 0 times").
  • P8, scope skipped on purchase groups: FAILS.
• Merge: git merge-tree --write-tree origin/main <head> is clean.
  • Main changed interfaces.go, the mocks and the test mocks (RI exchange direct-exec methods), plus unrelated text in types.go and openapi.yaml.
  • handler_history.go and store_postgres.go are unchanged on main.
  • Built and tested on a merged export.

Verified locally, on head and merged exports with no cloud credentials, under the heavy lock, using the repo's testcontainers harness:

• Build and static checks:
  • go build ./...: OK.
  • go vet on api, config, mocks, analytics and server, plus -tags integration on api and config: OK.
  • gofmt -l on the 12 changed files: clean.
• Unit tests: go test -count=1 on the 5 packages: ok.
• Integration tests:
  • config -run 'TestIntegration_HistorySummaryAggregate|TestIntegration_GetExecutionsByStatuses|PurchaseHistory': PASS.
  • api -run 'TestIntegration_HistorySummary_|History': PASS.
• Fail-before: the new api integration test on base FAILS as the PR says: scoped total_purchases 130 vs 61, admin 260 vs 120, and summary_complete is missing.
• No migration and no frontend changes.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

[cristim]

VERDICT: APPROVE abec6cd

Reviewed against origin/main f5ad8c6. git merge-tree is clean (tree e9f2c22e). The merged tree builds, and go vet passes with and without -tags integration, on Go 1.26.10.

Findings

• medium (pre-existing, not introduced) internal/api/handler_history.go:739 (main :729) - parseHistoryFilters never reads plan_id, and historyFilters has no plan field. The frontend still sends it: frontend/src/api/history.ts:22, and viewPlanHistory (frontend/src/history.ts:282, :294) passes { planId }. Failure scenario: "View plan history" lists every in-scope purchase, not just the plan's, and has no date bound. With this PR the cards on that view now total every in-scope purchase ever made and report summary_complete: true, where before they summed one capped page. The list and the totals are wrong in the same way, so they stay consistent with each other. This needs a follow-up issue. It does not block this PR.
• nit (pre-existing, sec(money): currency is dropped at every layer boundary, so USD-denominated caps are compared against non-USD amounts [platform part] #361) internal/config/store_postgres_history_summary.go:85-86 - upfront_cost and estimated_savings are summed with no currency, as the old page summary did. purchase_history has no currency column, so mixed-currency rows still produce one unlabeled number, now over every matching row.
• nit (test gap) internal/api/handler_history_summary.go:164 - mutation M3 sets the group CloudAccountID to nil, and no unit or integration test fails. The scope-name lookup then falls back to (provider, account_id). That only differs when a row's cloud_account_id and its external id point at different accounts. Add one fixture for that case.
• nit (process) - the single commit is 1656 changed lines, over the 400-line cap.

Checked and found correct

• Scope: historyScope.allows (handler_history_summary.go:63-72) is the same code for listed rows and for aggregate groups. Execution groups go through matchesExecutionProviderAndAccount and executionToHistoryRow before the scope check (:121-136). The groups carry every field those functions read: status, cloud_account_id, created_by_user_id and the provider/cloud_account_id of each rec.
• Filters: purchaseHistoryFilterWhere and historyExecutionsPredicate are shared with the row SQL. The SQL date bound is inclusive and matches the Go check. An empty filter is equivalent to GetAllPurchaseHistory.
• Unknown upfront: a group with any NULL upfront_cost gets a NULL sum, which sumKnownCosts treats as unknown, the same as a single unknown row.
• Execution dollars: every listed execution status is either a non-dollar bucket or an IsAuditGap row, so rebuilt groups never add dollars.

Tests (merged tree)

• Unit: go test ./internal/{api,config,mocks,analytics,server}/: 5763 passed.
• Integration (testcontainers Postgres): go test -tags integration ./internal/config/ -run 'TestIntegration_HistorySummaryAggregate|TestIntegration_GetExecutionsByStatuses|History|PurchaseHistory' ok. ./internal/api/ -run 'TestIntegration_HistorySummary_|History' ok.

Attacked (mutation probes, each in a scratch copy and reverted)

• M1: skip the scope check on purchase groups. FAIL TestGetHistory_SummaryComesFromScopedAggregate (caught).
• M2: skip the scope check on execution groups. FAIL, same test (caught).
• M3: drop the group CloudAccountID. Unit and integration tests both pass (survived, see the test-gap nit).
• M4: make the stale sweep run before the aggregate instead of deferred. FAIL TestGetHistory_LambdaSweepRunsAfterSummaryAggregate (caught).
• M5: replace the NULL-upfront CASE with a plain SUM. FAIL TestIntegration_HistorySummaryAggregate_MatchesRowFetch (caught).
• Also read: the snapshot transaction (REPEATABLE READ, READ ONLY, begun before any query), the stale-sweep defer on error paths, API-key sessions (UserID is never empty, so the ownership gate holds), and the mock fallback.

[cristim]

Local verification at abec6cdc96e68901aaae657a706cc6445f275764 merged onto current main (00a3f408; merge-tree 05693d46; macOS):

• go build ./..., go vet -tags integration ./... and go test -race -short ./... pass (full suite).
• Nothing was applied to any cloud.

[cristim]

Local verification at abec6cdc96e68901aaae657a706cc6445f275764 merged onto current main (00a3f408; merge-tree 05693d46; macOS):

• go build ./..., go vet -tags integration ./... and go test -race -short ./... pass (full suite).
• Nothing was applied to any cloud.

[cristim]

Marked ready and merged by the owner's agent after 2 independent reviews and local verification (see the review and verification comments above, all at abec6cdc96e68901aaae657a706cc6445f275764).