fix(purchases): exclude cancelled rows from summarizePurchaseHistory (closes #625)

[cristim]

Summary

• summarizePurchaseHistory already excludes cancelled rows from dollar KPI totals via a dedicated switch case (landed in PR fix(api/history): exclude cancelled executions from KPI aggregation (closes #736) #737, which cited bug(api/purchases): Total Upfront/Monthly/Annual Savings KPIs include cancelled purchases #736 but not the original duplicate bug(purchases): summarizePurchaseHistory counts cancelled rows into dollar totals #625).
• This PR closes bug(purchases): summarizePurchaseHistory counts cancelled rows into dollar totals #625 by updating the two existing regression-test comments and assertion messages to cite both issues, providing an explicit audit trail.
• No logic changes; the guard was already correct.

What the fix does (from #737)

summarizePurchaseHistory in internal/api/handler_history.go now has an explicit "cancelled" case that continues past the dollar-accumulation lines, so cancelled rows contribute 0 to TotalUpfront, TotalMonthlySavings, and TotalAnnualSavings.

Test plan

• go test ./internal/api/... -run TestSummarizePurchaseHistory passes (2 tests)
• go build ./... succeeds
• TestSummarizePurchaseHistory_CancelledExcludedFromKPIs: fixture with 3 active rows + 2 cancelled asserts totals sum only the active rows
• TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs: adding a cancelled row to a baseline set leaves all KPI totals unchanged

Summary by CodeRabbit

• Tests
  • Strengthened regression coverage for history/audit-gap behavior, including verification of synthesized audit-gap markers.
  • Improved KPI summary testing for purchase history by excluding cancelled scenarios and extending datasets with additional cancelled rows.
  • Refined and reorganized history filter-related test cases and updated assertion formatting for clearer, more maintainable expectations.

[coderabbitai]

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: 58 minutes

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: f8826e5f-3882-4884-86ad-66537628444b

📥 Commits

Reviewing files that changed from the base of the PR and between f3185b9 and 855a9bf.

📒 Files selected for processing (1)

• internal/api/handler_history_test.go

📝 Walkthrough

Walkthrough

Test-only changes in internal/api/handler_history_test.go: tightens the IsAuditGap marker assertion, rewraps comments and reorders filter validation test-case struct fields, fixes comment wording in a duplication test, extends the cancelled-KPI dataset with two explicit cancelled rows, and refactors CancelPendingDoesNotChangeKPIs to use make/append slice construction.

Changes

History handler test updates

Layer / File(s)	Summary
Audit-gap assertion and filter test refinements internal/api/handler_history_test.go	Tightens the IsAuditGap marker assertion in TestHandler_getHistory_AuditGapCompletedVisible, rewraps the FilterParams comment block, and reorders the FilterValidation struct-literal field layout in the test-case table.
KPI summary test data and structure for cancelled rows internal/api/handler_history_test.go	Fixes comment wording in TestHandler_getHistory_CompletedExecutionNotDuplicated; updates the introductory comment in TestSummarizePurchaseHistory_CancelledExcludedFromKPIs, extends the purchase dataset with two explicit cancelled rows (annotated with //nolint:misspell), and reformats KPI equality assertions; refactors TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs to construct baseline and withCancelled via make/append instead of composite literals, and updates assertion message strings.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~4 minutes

Possibly related PRs

• LeanerCloud/CUDly#737: Directly introduced the "cancelled excluded from KPIs" and "cancel pending doesn't change KPIs" test datasets and assertions that this PR refines and extends.
• LeanerCloud/CUDly#623: Introduced the underlying audit-gap and in-flight logic in handler_history.go whose test expectations this PR is aligned with.

Poem

🐇 A cancelled row must never sneak
Into the totals that we seek,
With make and append the slices grow,
And tests now guard what we don't show.
The audit gap is marked with care —
No double-counts shall linger there! ✨

🚥 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 describes the main change: excluding cancelled rows from summarizePurchaseHistory and closing issue #625.
Linked Issues check	✅ Passed	The PR addresses issue #625 by confirming the logic fix (cancelled case in summarizePurchaseHistory) was already implemented, and updating regression tests to document this fix.
Out of Scope Changes check	✅ Passed	All changes are scoped to test comments and assertion messages in handler_history_test.go, directly supporting the linked issue #625.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/625-wave8

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

✅ Actions performed

Full review triggered.

[cristim]

@coderabbitai review

[coderabbitai]

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 39 minutes and 17 seconds before sending another message.

[cristim]

@coderabbitai full review

---

Generated by Claude Code

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR details and diff for PR `#821`
gh pr view 821 --repo LeanerCloud/CUDly --json title,body,files,state,baseRefName,headRefName

echo "---DIFF---"
gh pr diff 821 --repo LeanerCloud/CUDly

Length of output: 5116

---

---

@cristim: This is a clean, minimal PR. Here's the full review:

Review of PR #821 — fix(purchases): exclude cancelled rows from summarizePurchaseHistory (closes #625)

Summary

Only internal/api/handler_history_test.go is touched. All 24 changed lines are pure comment and assertion-message text updates — no logic, no imports, no struct or API changes.

Changes reviewed

Location	Before	After	Verdict
TestSummarizePurchaseHistory_CancelledExcludedFromKPIs doc comment	regression test for issue #736``	regression test for issues #625and#736``	✅ Accurate
TestSummarizePurchaseHistory_CancelledExcludedFromKPIs — 3 assert.InDelta messages	(issue #736)	(issues #625, #736)	✅ Accurate
TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs doc comment	QA reproduction scenario from issue #736``	QA reproduction scenario from issues #625and#736``	✅ Accurate
TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs — 4 assert.Equal messages	(issue #736)	(issues #625, #736)	✅ Accurate

Assessment

• No logic risk. The "cancelled" guard in handler_history.go is untouched and was already correct (landed in PR #737).
• Audit trail is now complete. Adding #625 to the test comments closes the gap between the original issue and the implementing PR.
• Consistent style. All seven affected strings are updated uniformly.

Nothing to flag. The PR does exactly what it says and nothing more.

[approve]

---

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:43:53.792Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), the Dashboard "Upcoming purchases" widget (`dashboard.ts`) has two Cancel entry points (card-level and modal) that both call `api.deletePlannedPurchase`. The `UpcomingPurchase` response DTO must include `CreatedByUserID *string` (omitempty, mirroring `PlannedPurchase`) so the frontend can gate these buttons via a `canCancelUpcomingPurchase` helper that mirrors `canManageScheduledPurchase` on the Plans page. Without this, the backend correctly 403s non-owners but the UI has no gate and the UX is broken. Regressions: `frontend/src/__tests__/dashboard-ownership-950.test.ts` (6 tests) and `TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID`. Fixed in commit 94326f6b9.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:43:53.792Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), `POST /api/plans/{id}/purchases` (`createPurchaseExecutionsTx`) must stamp `created_by_user_id` via `resolveCreatorUserID(session)` at creation time. Without this, every scheduled purchase created via the Plans-page "Create planned purchases" action ships a NULL creator, which the ownership gate treats as a legacy row and denies management to all non-`update-any` users — including the creator themselves. The admin API key path must set `CreatedByUserID` to nil (not the key identity). Regressions: `TestHandler_createPlannedPurchases_StampsCreator` and `TestHandler_createPlannedPurchases_AdminAPIKeyCreatorIsNil`. Fixed in commit 9ef1b4352.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T17:45:38.069Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/cloudsql/client.go`, `getSQLPricing` had the same per-hour/term-total unit mismatch as memorystore and cloudstorage: `commitmentPrice` (per-hour SKU rate) was passed directly to savings math that expected a term total, producing ~99.99% savings, and `HourlyRate` was set to `per-hour / hoursInTerm` (near-zero). Fixed in PR `#1047` commit 20590c6b4 (issue `#1078` folded in): `commitmentPriceTerm := commitmentPrice * hoursInTerm`; `HourlyRate = commitmentPrice` (raw per-hour). `convertGCPRecommendation` also hardcoded `rec.Term = "1yr"`, ignoring `params.Term`; fixed with same defaulting pattern as memorystore/cloudstorage. Regression tests: `TestGetSQLPricing_CommitmentPriceIsTermTotal` and `TestCloudSQLConvertGCPRecommendation_PropagatesTermFromParams`. PaymentOption was already correct (`if paymentOption == "" { paymentOption = "monthly" }`).

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T17:38:19.433Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/computeengine/client.go`, `convertGCPRecommendation` must force `paymentOption = "monthly"` unconditionally (logging a WARN when a non-monthly value such as "upfront", "all-upfront", or "partial-upfront" is supplied) because GCP CUDs are monthly-only and any non-monthly value passed through would be a silent misconfiguration. Fixed in PR `#1047` commit c6280c390 (F1). Regression test: `TestConvertGCPRecommendation_NonMonthlyPaymentOptionForcedToMonthly`.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[cristim]

@coderabbitai review

[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 review

[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 review

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

Rebased onto origin/main (89658a3) to pick up the #1437 tflint-cache and #1438 trivy-skip fixes. New tip: f60edc7. Local gates: go build/vet exit=0, gocyclo -over 10 on production files exit=0 (only test files touched), golangci-lint ./... exit=0 (no issues), go test ./internal/api/... 1736 passed.