fix(api): compute active commitments via uncapped SQL active-only read by cristim · Pull Request #1221 · LeanerCloud/CUDly

cristim · 2026-06-11T05:16:19Z

Problem

Closes #1140 (review finding PERF-01, P2).

The dashboard KPI path (fetchCommitmentPurchases in internal/api/handler_dashboard.go) and the inventory commitments/coverage endpoints (fetchCommitmentRecords in internal/api/handler_inventory.go) computed "active commitments" from a newest-first 1000-row capped fetch (GetAllPurchaseHistory / GetPurchaseHistoryFiltered with Limit: 1000), filtering expiry in Go. Once purchase_history exceeds 1000 rows, the first rows dropped are exactly the oldest still-active 1y/3y commitments, so the active-commitment count, committed-monthly KPI, YTD savings, per-service chart, renew-next list, and coverage breakdown all silently understate reality. For restricted users the account filter was applied after the global capped fetch, shrinking their visible set further.

Fix

Reuse the store's SQL active-only primitive, as recommended by the review:

GetActivePurchaseHistory gains the same dual-column account predicate as GetPurchaseHistoryFiltered (cloud_account_id UUIDs OR provider + external account_id, issues bug(api/history): handler ignores provider/account_ids/start/end query params from Purchase History + Purchases-page Global filters #701/ux(home): Account filter doesn't affect Home page Savings-over-time graph (data identical across accounts) #498/Main Header global filter not propagated to Inventory & Coverage subpages (QA 2.7+2.14, 2.8+2.15) #866) via the shared appendAccountPredicate helper. Empty scope means all accounts; the analytics collector passes nil/nil.
The dashboard KPI path and both inventory endpoints now route through it, so the active filter and account scope run in SQL with no LIMIT; the result is bounded by the number of live commitments, not by all history ever recorded. The Go-side isActiveCommitment gate remains as the status filter.
Latent bug found while extending the query: GetActivePurchaseHistory selected 17 columns while queryPurchaseHistory scans 21 destinations (the issue-feat(purchases): in-app revocation flow — direct API for free-cancel window + AWS Support case drafter for week window #290 revocation columns were missing), so every call failed at Scan. The SELECT now lists all 21 columns and the pgxmock test pins them.

Test evidence

Regression tests replicate the real failure shape (1000 newer-but-expired rows plus one older still-active 3y commitment outside the newest-1000 window) for both the dashboard KPI path (TestHandler_calculateCommitmentMetrics_NoTruncationBeyond1000) and the inventory endpoint (TestHandler_listActiveCommitments_NoTruncationBeyond1000). Verified they FAIL against the pre-fix handler code (restored the origin/main handlers, both failed with 0 active commitments instead of 1) and pass post-fix.
New pgxmock tests assert the emitted SQL: active predicate in WHERE, dual-column account scope, full 21-column SELECT, and no trailing LIMIT clause.
go build ./... clean; go test ./internal/api/... ./internal/config/... ./internal/analytics/... ./internal/server/...: 2772 passed, 0 failed (after rebase onto current main).

Summary by CodeRabbit

New Features
- Enhanced active commitment retrieval to support account-scoped filtering using both UUIDs and provider external IDs.
Bug Fixes
- Prevented missing still-active commitments when history exceeds 1000 rows.
- Improved consistency of active commitment and metrics/coverage calculations across dashboard and inventory endpoints under scoped sessions.
Refactor
- Moved active-only purchase-history selection and scoping into the data query layer, ensuring consistent “as-of” timing.
Tests
- Added regression coverage for truncation and scoped/zero-account query behavior.
- Updated mocks to match the expanded active-history retrieval contract.

coderabbitai · 2026-06-11T05:16:33Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 56 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b37a7a66-698a-46a2-b89d-9cd3005c7597

📥 Commits

Reviewing files that changed from the base of the PR and between ae75614 and c236d31.

📒 Files selected for processing (1)

internal/api/handler_per_account_perms_test.go

📝 Walkthrough

Walkthrough

PR #1221 extends GetActivePurchaseHistory with account scoping and updates dashboard, inventory, analytics, mocks, and tests to use uncapped SQL-level active-history retrieval.

Changes

Active Commitment Query Path Refactor

Layer / File(s)	Summary
Store contract and PostgreSQL query implementation `internal/config/interfaces.go`, `internal/config/store_postgres.go`, `internal/config/store_postgres_pgxmock_test.go`	The active-history contract accepts account and external-ID scopes. PostgreSQL applies inclusive active-time and dual-identifier predicates without `LIMIT`, with SQL tests covering scoped and unscoped queries.
Dashboard commitment metrics `internal/api/handler_dashboard.go`, `internal/api/handler_dashboard_test.go`	Dashboard metrics use a shared timestamp and uncapped active-history queries, preserving zero-scope handling and validating account routing and no-truncation behavior.
Inventory commitments and coverage `internal/api/handler_inventory.go`, `internal/api/handler_inventory_test.go`, `internal/api/handler_per_account_perms_test.go`	Inventory flows use shared timestamps, resolved account scope, and active-only retrieval; tests cover filtering, permissions, zero scopes, and records beyond 1000 rows.
Analytics and test contract propagation `internal/analytics/*`, `internal/mocks/stores.go`, `internal/server/test_helpers_test.go`, `internal/api/handler_test.go`	The collector uses an unscoped active-history query, and affected mocks and tests adopt the expanded method signature.
Allowlist fixture support `.gitallowed`	Adds the secret-scanner placeholder for a repeating-pair-block fixture.

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

Sequence Diagram(s)

sequenceDiagram
  participant Handler
  participant MetricsOrInventory
  participant Store
  Handler->>MetricsOrInventory: Start commitment calculation
  MetricsOrInventory->>MetricsOrInventory: Capture asOf timestamp
  MetricsOrInventory->>Store: GetActivePurchaseHistory(asOf, account scope)
  Store-->>MetricsOrInventory: Uncapped active records
  MetricsOrInventory-->>Handler: Metrics, commitments, or coverage

Possibly related issues

PERF-01: Dashboard/inventory active commitments computed from 1000-row capped fetch; silent truncation #1140: This PR directly addresses capped active-commitment history reads and silent truncation.
fix(analytics): collector undercounts past 100k history rows and stores schedule-dependent units #1074: Both changes modify analytics purchase-history retrieval related to active commitment aggregation.
bug(api/dashboard): calculateCommitmentMetrics does not intersect allowed_accounts for scoped sessions #959: Both changes apply allowed-account scope to commitment metrics and active purchase-history queries.

Possibly related PRs

LeanerCloud/CUDly#1126: Both modify inventory coverage computation, with this PR changing active-history retrieval.
LeanerCloud/CUDly#1049: Both modify the analytics collector’s active-history flow and contract.
LeanerCloud/CUDly#956: Both modify account-scope resolution and filtering for dashboard and inventory commitment paths.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The diff also changes analytics collector behavior and updates .gitallowed, which are unrelated to the active-commitments fix.	Split the collector change and .gitallowed edit into separate PRs, or remove them if they were not intended as part of this fix.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: switching active-commitment reads to uncapped SQL active-only queries.
Linked Issues check	✅ Passed	The PR satisfies `#1140` by routing dashboard and inventory reads through GetActivePurchaseHistory, applying SQL account scoping, removing LIMIT truncation, and adding regression coverage.
Docstring Coverage	✅ Passed	Docstring coverage is 90.32% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/perf-01-active-commitments-sql

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

cristim · 2026-06-11T05:16:37Z

@coderabbitai review

coderabbitai · 2026-06-11T05:16:43Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/api/handler_inventory.go`:
- Around line 91-98: fetchCommitmentRecords currently only scopes SQL by the
explicit account_id param and calls GetActivePurchaseHistory(ctx, asOf, nil,
nil) when no account_id is present, which causes restricted sessions to read all
tenants; change it to derive the session's allowed-account scope from ctx (or
accept an allowedAccounts argument) and pass that scope into
GetActivePurchaseHistory instead of nil so SQL is constrained up-front; keep the
resolveSingleAccountFilterIDs behavior for explicit account_id, and ensure later
calls to filterPurchaseHistoryByAllowedAccounts remain consistent with the same
allowed-account set.

In `@internal/config/store_postgres.go`:
- Around line 1628-1633: The SQL expiry predicate in conds currently uses
"timestamp + make_interval(hours => term * 8760) > $1" which excludes
commitments that expire exactly at asOf; change that comparison to ">=" (i.e.
make it inclusive) so the SQL result matches the Go-side active checks that
treat asOf == expiry as active, and update the related docstring describing the
inclusive contract; relevant symbols: conds, args, asOf, appendAccountPredicate.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9bd0f1a6-345e-4bf5-9ce0-c9928c5b457d

📥 Commits

Reviewing files that changed from the base of the PR and between 4108e51 and e4fca83.

📒 Files selected for processing (13)

internal/analytics/collector.go
internal/analytics/collector_test.go
internal/api/handler_dashboard.go
internal/api/handler_dashboard_test.go
internal/api/handler_inventory.go
internal/api/handler_inventory_test.go
internal/api/handler_per_account_perms_test.go
internal/api/handler_test.go
internal/config/interfaces.go
internal/config/store_postgres.go
internal/config/store_postgres_pgxmock_test.go
internal/mocks/stores.go
internal/server/test_helpers_test.go

For a restricted session with no explicit account_id chip, the inventory endpoints (/api/inventory/commitments and /coverage) still called GetActivePurchaseHistory(ctx, asOf, nil, nil), reading every tenant's active commitments and only trimming them in memory via filterPurchaseHistoryByAllowedAccounts. Thread the session into fetchCommitmentRecords and resolve the allowed-account scope through resolveAllowedAccountScope (same issue #956 contract as the dashboard KPI path), including the zero-accessible-accounts sentinel that short-circuits without querying. The in-memory filter stays as defence in depth and still trims an explicit out-of-scope account_id. Regression tests pin the scoped store args for both endpoints and the no-query sentinel path; they fail on the pre-fix handler (unexpected unscoped mock call) and pass post-fix. Addresses CodeRabbit review on PR #1221 (handler_inventory.go).

GetActivePurchaseHistory excluded a commitment expiring exactly at asOf (`> $1`) while the API layer's isActiveCommitment treats that instant as still active (!now.After(expiry)). Align the SQL predicate to `>= $1` so both sides share one boundary definition, and document the inclusive contract in the store and interface docstrings. The pgxmock tests pin the new predicate. Addresses CodeRabbit review on PR #1221 (store_postgres.go).

cristim · 2026-06-11T06:44:40Z

@coderabbitai review

coderabbitai · 2026-06-11T06:44:46Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

For a restricted session with no explicit account_id chip, the inventory endpoints (/api/inventory/commitments and /coverage) still called GetActivePurchaseHistory(ctx, asOf, nil, nil), reading every tenant's active commitments and only trimming them in memory via filterPurchaseHistoryByAllowedAccounts. Thread the session into fetchCommitmentRecords and resolve the allowed-account scope through resolveAllowedAccountScope (same issue #956 contract as the dashboard KPI path), including the zero-accessible-accounts sentinel that short-circuits without querying. The in-memory filter stays as defence in depth and still trims an explicit out-of-scope account_id. Regression tests pin the scoped store args for both endpoints and the no-query sentinel path; they fail on the pre-fix handler (unexpected unscoped mock call) and pass post-fix. Addresses CodeRabbit review on PR #1221 (handler_inventory.go).

GetActivePurchaseHistory excluded a commitment expiring exactly at asOf (`> $1`) while the API layer's isActiveCommitment treats that instant as still active (!now.After(expiry)). Align the SQL predicate to `>= $1` so both sides share one boundary definition, and document the inclusive contract in the store and interface docstrings. The pgxmock tests pin the new predicate. Addresses CodeRabbit review on PR #1221 (store_postgres.go).

cristim · 2026-06-19T15:18:23Z

@coderabbitai review

coderabbitai · 2026-06-19T15:18:30Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

For a restricted session with no explicit account_id chip, the inventory endpoints (/api/inventory/commitments and /coverage) still called GetActivePurchaseHistory(ctx, asOf, nil, nil), reading every tenant's active commitments and only trimming them in memory via filterPurchaseHistoryByAllowedAccounts. Thread the session into fetchCommitmentRecords and resolve the allowed-account scope through resolveAllowedAccountScope (same issue #956 contract as the dashboard KPI path), including the zero-accessible-accounts sentinel that short-circuits without querying. The in-memory filter stays as defence in depth and still trims an explicit out-of-scope account_id. Regression tests pin the scoped store args for both endpoints and the no-query sentinel path; they fail on the pre-fix handler (unexpected unscoped mock call) and pass post-fix. Addresses CodeRabbit review on PR #1221 (handler_inventory.go).

GetActivePurchaseHistory excluded a commitment expiring exactly at asOf (`> $1`) while the API layer's isActiveCommitment treats that instant as still active (!now.After(expiry)). Align the SQL predicate to `>= $1` so both sides share one boundary definition, and document the inclusive contract in the store and interface docstrings. The pgxmock tests pin the new predicate. Addresses CodeRabbit review on PR #1221 (store_postgres.go).

cristim · 2026-06-26T15:47:43Z

@coderabbitai review

coderabbitai · 2026-06-26T15:47:50Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

For a restricted session with no explicit account_id chip, the inventory endpoints (/api/inventory/commitments and /coverage) still called GetActivePurchaseHistory(ctx, asOf, nil, nil), reading every tenant's active commitments and only trimming them in memory via filterPurchaseHistoryByAllowedAccounts. Thread the session into fetchCommitmentRecords and resolve the allowed-account scope through resolveAllowedAccountScope (same issue #956 contract as the dashboard KPI path), including the zero-accessible-accounts sentinel that short-circuits without querying. The in-memory filter stays as defence in depth and still trims an explicit out-of-scope account_id. Regression tests pin the scoped store args for both endpoints and the no-query sentinel path; they fail on the pre-fix handler (unexpected unscoped mock call) and pass post-fix. Addresses CodeRabbit review on PR #1221 (handler_inventory.go).

GetActivePurchaseHistory excluded a commitment expiring exactly at asOf (`> $1`) while the API layer's isActiveCommitment treats that instant as still active (!now.After(expiry)). Align the SQL predicate to `>= $1` so both sides share one boundary definition, and document the inclusive contract in the store and interface docstrings. The pgxmock tests pin the new predicate. Addresses CodeRabbit review on PR #1221 (store_postgres.go).

cristim · 2026-07-10T13:57:04Z

@coderabbitai review

coderabbitai · 2026-07-10T13:57:11Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

The dashboard KPI path (fetchCommitmentPurchases) and the inventory commitments/coverage endpoints (fetchCommitmentRecords) read purchase history through a newest-first 1000-row capped fetch (GetAllPurchaseHistory / GetPurchaseHistoryFiltered with Limit 1000) and filtered "active" in Go. Once purchase_history exceeded the cap, the first rows dropped were exactly the oldest still-active 1y/3y commitments, silently understating the active-commitment count, committed-monthly KPI, YTD savings, per-service chart, the renew-next list, and the coverage breakdown, with no error or banner. Reuse the store's SQL active-only primitive instead: extend GetActivePurchaseHistory with the same dual-column account predicate as GetPurchaseHistoryFiltered (cloud_account_id UUIDs OR provider plus external account_id, issues #701/#498/#866) via the shared appendAccountPredicate helper, and route the dashboard KPI path and both inventory endpoints through it. The active filter and account scope now run in SQL with no LIMIT, so the result is bounded by the number of live commitments rather than by all history ever recorded. The Go-side isActiveCommitment gate stays as the status filter. Also fixes a latent Scan failure found while extending the query: GetActivePurchaseHistory selected 17 columns while queryPurchaseHistory scans 21 destinations (the issue-#290 revocation columns were missing), so every call failed at Scan. The pgxmock test now pins the full 21-column SELECT and the absence of a LIMIT clause. Regression tests replicate the real failure shape (1000 newer expired rows plus one older still-active 3y commitment outside the newest-1000 window) for both the dashboard KPI path and the inventory endpoint, and fail against the pre-fix handler code. Closes #1140

For a restricted session with no explicit account_id chip, the inventory endpoints (/api/inventory/commitments and /coverage) still called GetActivePurchaseHistory(ctx, asOf, nil, nil), reading every tenant's active commitments and only trimming them in memory via filterPurchaseHistoryByAllowedAccounts. Thread the session into fetchCommitmentRecords and resolve the allowed-account scope through resolveAllowedAccountScope (same issue #956 contract as the dashboard KPI path), including the zero-accessible-accounts sentinel that short-circuits without querying. The in-memory filter stays as defence in depth and still trims an explicit out-of-scope account_id. Regression tests pin the scoped store args for both endpoints and the no-query sentinel path; they fail on the pre-fix handler (unexpected unscoped mock call) and pass post-fix. Addresses CodeRabbit review on PR #1221 (handler_inventory.go).

GetActivePurchaseHistory excluded a commitment expiring exactly at asOf (`> $1`) while the API layer's isActiveCommitment treats that instant as still active (!now.After(expiry)). Align the SQL predicate to `>= $1` so both sides share one boundary definition, and document the inclusive contract in the store and interface docstrings. The pgxmock tests pin the new predicate. Addresses CodeRabbit review on PR #1221 (store_postgres.go).

The TestHandler_calculateCommitmentMetrics_NoTruncationBeyond1000 regression test registered GetAllPurchaseHistory with a bare On() call (documentation only -- the point of the test is that the old capped path must NOT be called), but without AssertExpectations the guarantee was one-sided: a future regression restoring the old path would be caught by AssertNotCalled, but the GetActivePurchaseHistory mock could silently never be called without being flagged. Add t.Cleanup(func(){ mockStore.AssertExpectations(t) }) so the mock framework enforces the GetActivePurchaseHistory call, and mark the documentation-only GetAllPurchaseHistory On() with .Maybe() so AssertExpectations does not treat its absence as a failure.

Apply feedback_mock_assert_expectations to the two new regression tests added in this PR so a future store call slipping into the active-commitment path (or the zero-allowed-accounts short-circuit) cannot pass as a silent false green: - TestHandler_listActiveCommitments_NoTruncationBeyond1000 registers AssertExpectations cleanup, and the documentation-only GetAllPurchaseHistory expectation is now .Maybe() so the cleanup does not fail on the (expected) zero call count. The dashboard's parallel _NoTruncationBeyond1000 already pairs Maybe with cleanup; this brings the inventory test to the same shape. - TestPerAccountPerms_InventoryCommitments_ZeroAllowedAccountsNoQuery registers AssertExpectations on both mockStore and mockAuth so a stray store call (or a missing auth On registration) surfaces as a test failure. No behavior change; tests still pass.

cristim · 2026-07-10T21:46:08Z

@coderabbitai review

coderabbitai · 2026-07-10T21:46:13Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/api/handler_per_account_perms_test.go`:
- Around line 503-509: The dashboard summary test should explicitly verify that
scoped commitment history is read, rather than only asserting that the unscoped
method is not called. In the test using the GetActivePurchaseHistory mock
expectation, add a targeted AssertCalled or AssertExpectations assertion
confirming GetActivePurchaseHistory receives the expected context, time
argument, permsAccA scope, and nil external-ID filter.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d2e4e2ad-57c5-4236-a8bd-e9f86d2ba7e1

📥 Commits

Reviewing files that changed from the base of the PR and between e4fca83 and ae75614.

📒 Files selected for processing (14)

.gitallowed
internal/analytics/collector.go
internal/analytics/collector_test.go
internal/api/handler_dashboard.go
internal/api/handler_dashboard_test.go
internal/api/handler_inventory.go
internal/api/handler_inventory_test.go
internal/api/handler_per_account_perms_test.go
internal/api/handler_test.go
internal/config/interfaces.go
internal/config/store_postgres.go
internal/config/store_postgres_pgxmock_test.go
internal/mocks/stores.go
internal/server/test_helpers_test.go

✅ Files skipped from review due to trivial changes (1)

.gitallowed

🚧 Files skipped from review as they are similar to previous changes (12)

internal/api/handler_test.go
internal/analytics/collector.go
internal/server/test_helpers_test.go
internal/mocks/stores.go
internal/config/store_postgres_pgxmock_test.go
internal/api/handler_dashboard.go
internal/config/store_postgres.go
internal/config/interfaces.go
internal/analytics/collector_test.go
internal/api/handler_inventory.go
internal/api/handler_inventory_test.go
internal/api/handler_dashboard_test.go

The AggregatesAllowedSubsetOnly test only asserted the absence of the unscoped GetAllPurchaseHistory call; it could pass if commitment metrics were skipped entirely. Assert the scoped call actually happened (CodeRabbit finding on #1221).

cristim · 2026-07-16T14:01:00Z

@coderabbitai review

coderabbitai · 2026-07-16T14:01:06Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-07-16T15:53:06Z

Merged to main after pre-merge adversarial review (FIX-THEN-MERGE -> fixed): active commitments are now computed via an uncapped, account-scoped, active-only SQL read (WHERE term-based expiry >= asOf, no LIMIT) instead of the capped 1000-row fetch that silently dropped older-but-active commitments from dashboard KPIs. The regression test reproduces the real 1500-row truncation scenario; the review's one finding (positively assert the scoped read) was fixed in c236d31. Union with current main verified green (2890 tests). Follow-up #1377 filed for the pre-existing zeroed-KPI-on-store-error silent fallback found during review.

…ECT (#1472) GetActivePurchaseHistory SELECTed 21 columns but shares scanPurchaseHistoryRow, which scans 24 destinations (offering_class, listing_id, listing_state were added by #808). pgx/v5 Rows.Scan requires field count == destination count, so every real-Postgres call to this query failed with "number of field descriptions must equal number of destinations, got 21 and 24". Impact (all broken on real Postgres, silently in some paths): - dashboard commitment KPIs (fetchCommitmentPurchases swallows the error and renders 0 active commitments / $0 committed monthly / $0 YTD), - GET /api/inventory/commitments and /api/inventory/coverage (return errors), - the analytics collector's Collect (fails every run). #808 (marketplace) added the three columns to the scanner and to GetPurchaseHistory / GetAllPurchaseHistory / GetPurchaseHistoryFiltered but missed GetActivePurchaseHistory. This is the same class of defect #1221 fixed for the 17-vs-21 case. Fix: append offering_class, listing_id, listing_state so the column list is identical to the other purchase-history queries and matches the scanner. Update the pgxmock regex to pin the corrected column list so a future drop of these columns fails the test. Note: pgxmock cannot reproduce the field-count mismatch (it fabricates result columns), so the regression is only guarded via the SQL-text regex here; a shared "emitted SELECT column list == purchaseHistoryCols" assertion across all purchase-history queries is a worthwhile follow-up. Found by the adversarial (Fable) review sweep of recently-merged PRs.

cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/high Significant harm urgency/this-quarter Within the quarter impact/many Affects most users effort/m Days type/bug Defect labels Jun 11, 2026

coderabbitai Bot reviewed Jun 11, 2026

View reviewed changes

Comment thread internal/api/handler_inventory.go Outdated

Comment thread internal/config/store_postgres.go

cristim force-pushed the fix/perf-01-active-commitments-sql branch from 81dafca to 5761674 Compare June 19, 2026 15:16

cristim force-pushed the fix/perf-01-active-commitments-sql branch from 5761674 to 6807418 Compare June 26, 2026 15:44

cristim mentioned this pull request Jun 26, 2026

fix(api): current_savings zero for services with no active commitments (closes #1031) #1041

Merged

7 tasks

cristim force-pushed the fix/perf-01-active-commitments-sql branch from 0f67a2e to 9041456 Compare July 10, 2026 13:56

cristim added 5 commits July 10, 2026 23:37

cristim force-pushed the fix/perf-01-active-commitments-sql branch from 9041456 to ae75614 Compare July 10, 2026 21:46

coderabbitai Bot reviewed Jul 10, 2026

View reviewed changes

Comment thread internal/api/handler_per_account_perms_test.go

cristim mentioned this pull request Jul 16, 2026

api: fetchCommitmentPurchases swallows store errors into zeroed dashboard KPIs #1377

Open

cristim merged commit f90a738 into main Jul 16, 2026
13 of 16 checks passed

coderabbitai Bot mentioned this pull request Jul 17, 2026

fix(api/dashboard): exclude revoked commitments, stop fabricating current_savings, provider-filter KPIs (adversarial-review follow-ups) #1452

Merged

5 tasks

cristim mentioned this pull request Jul 19, 2026

fix(db): add missing offering columns to GetActivePurchaseHistory SELECT (production-down) #1472

Merged

cristim deleted the fix/perf-01-active-commitments-sql branch July 27, 2026 11:10

Uh oh!

Conversation

cristim commented Jun 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Test evidence

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

cristim commented Jun 11, 2026

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cristim commented Jun 11, 2026

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 19, 2026

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 26, 2026

Uh oh!

coderabbitai Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cristim commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cristim commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Jun 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

coderabbitai Bot commented Jun 26, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading