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

[cristim]

Summary

Four money-integrity defects found by adversarial review of origin/main, all confirmed still present before this fix. All verified to fail pre-fix and pass post-fix.

Defect #2 (HIGH) - revoked/refunded commitments counted as ACTIVE on every money path:

• GetActivePurchaseHistory (store_postgres.go): added revoked_at IS NULL to SQL WHERE clause - single shared predicate covering dashboard KPIs, inventory list, and analytics snapshots simultaneously.
• isActiveCommitment (handler_dashboard.go): added p.RevokedAt != nil early-return as defense-in-depth for callers using GetPurchaseHistoryFiltered (e.g. the History page).
• summarizePurchaseHistory (handler_history.go): added revoked branch that increments new TotalRevoked counter and skips dollar totals.
• HistorySummary (types.go): added TotalRevoked int field ("total_revoked" JSON key).

Defect #3 (HIGH) - by_service.current_savings fabricated as full potential for services with no active commitments:

• Removed svc.CurrentSavings += scaled from the summarizeRecommendationsWithCoverage reducer. This was setting CurrentSavings = PotentialSavings for ALL services with recommendations, regardless of whether any commitment existed. The getDashboardSummary loop already overwrites from real purchase_history data for services with commitments; services without commitments now correctly default to 0.
• Updated the stale comment and the tests that asserted the buggy behavior (TestSummarizeRecommendationsWithCoverage_PopulatesCurrentSavings renamed to TestSummarizeRecommendationsWithCoverage_CurrentSavingsIsZero).

Defect #4 (MEDIUM) - provider chip filters only recs, not commitment KPIs:

• Added provider string parameter to calculateCommitmentMetrics and fetchCommitmentPurchases.
• Applied in-memory provider filter after the SQL fetch, mirroring the existing pattern in fetchCommitmentRecords.
• getDashboardSummary now passes params["provider"] so all KPIs (ActiveCommitments, CommittedMonthly, YTDSavings, CurrentSavings, CurrentCoverage) are scoped to the selected provider.

Defect #7 (LOW) - GetActivePurchaseHistory error silently swallowed:

• fetchCommitmentPurchases now calls logging.Errorf on error so the cause is visible in logs. KPIs still zero out (non-fatal) to avoid breaking the dashboard request, but with an observable signal.

Test plan

• go test ./internal/... -count=1 - 5018 tests pass (was run and verified)
• go test ./... -count=1 - 5833 tests pass (was run and verified)
• New regression tests confirmed to fail on pre-fix code, pass post-fix:
  • TestIsActiveCommitment_RevokedReturnsFalse
  • TestHandler_calculateCommitmentMetrics_RevokedExcluded
  • TestHandler_calculateCommitmentMetrics_ProviderFilter
  • TestSummarizeRecommendationsWithCoverage_CurrentSavingsIsZero
  • TestSummarizePurchaseHistory_RevokedExcludedFromKPIs
• SQL regex tests updated in store_postgres_pgxmock_test.go to pin the new revoked_at IS NULL clause
• Revoke a commitment, assert it drops out of active count, committed monthly, YTD, by_service current_savings, inventory active list

Summary by CodeRabbit

• New Features

  • Added provider-specific filtering to dashboard commitment metrics.
  • Added a total_revoked field to purchase history summaries.

• Bug Fixes

  • Revoked commitments are now excluded from active commitment counts, savings, completion totals, and financial KPIs.
  • Purchase history correctly reports revoked records without including them in completed or dollar totals.

[cristim]

@coderabbitai review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 063e6a09-a4e7-4fc2-bbe2-b354b704d865

📥 Commits

Reviewing files that changed from the base of the PR and between 1d9f1b8 and 27a7151.

📒 Files selected for processing (7)

• internal/api/handler_dashboard.go
• internal/api/handler_dashboard_test.go
• internal/api/handler_history.go
• internal/api/handler_history_test.go
• internal/api/types.go
• internal/config/store_postgres.go
• internal/config/store_postgres_pgxmock_test.go

---

📝 Walkthrough

Walkthrough

The change excludes revoked commitments from active queries and KPI calculations, adds revoked totals to purchase-history summaries, and filters dashboard commitment metrics by the selected provider.

Changes

Commitment KPI consistency

Layer / File(s)	Summary
Exclude revoked active purchases internal/config/store_postgres.go, internal/config/store_postgres_pgxmock_test.go	Active purchase-history queries require revoked_at IS NULL, with updated query expectations.
Apply provider and revocation filters to dashboard metrics internal/api/handler_dashboard.go, internal/api/handler_dashboard_test.go	Dashboard commitment metrics forward the selected provider, filter purchases accordingly, and exclude revoked commitments from active and savings KPIs.
Expose revoked history totals internal/api/handler_history.go, internal/api/handler_history_test.go, internal/api/types.go	Revoked records increment TotalRevoked while excluding completion and dollar totals; the response adds total_revoked.

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

Possibly related PRs

• LeanerCloud/CUDly#821: Related purchase-history KPI exclusions for cancelled records.
• LeanerCloud/CUDly#1041: Related dashboard commitment-metrics flow changes.
• LeanerCloud/CUDly#1221: Related active purchase-history filtering and dashboard commitment handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: excluding revoked commitments and applying provider-filtered dashboard KPI logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/dashboard-savings-integrity

---

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

[coderabbitai]

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

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 57 minutes.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 24 minutes.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.