feat(db): nullable MonthlyCost on PurchaseHistoryRecord (closes #255)

[cristim]

Summary

• Makes PurchaseHistoryRecord.MonthlyCost a *float64 (nullable) so rows from providers that do not return a monthly recurring cost (e.g. Azure/GCP all-upfront) store NULL instead of a misleading 0.0.
• Migration 000063 drops the NOT NULL constraint on purchase_history.monthly_cost; rollback coerces any NULLs back to 0.0 before re-adding the constraint.
• Scans the column via sql.NullFloat64 in all three copies of the scan loop (production, comprehensive-test mock, additional-test mock) so NULL DB values materialise as a nil pointer and 0.0 DB values materialise as &0.0, preserving the semantic distinction.
• Removes the derefFloat64 helper in execution.go that previously flattened nil to 0; now the pointer is passed through to the history record unchanged.
• Frontend: monthly_cost typed as number | null; inventory commitment cell renders — when null, $X.XX otherwise.
• All consumers that aggregate MonthlyCost skip nil entries (no distortion of totals).

Test plan

• go build ./... clean
• go test github.com/LeanerCloud/CUDly/internal/config/... github.com/LeanerCloud/CUDly/internal/api/... 1906 pass
• Migration 000063 up/down SQL reviewed: drops NOT NULL on purchase_history.monthly_cost; rollback NULLs-to-0 before re-adding constraint
• Coverage breakdown handler skips nil MonthlyCost entries rather than adding zero

Closes #255

Summary by CodeRabbit

• Bug Fixes
  • Purchase history, active commitments, and approval history now show "—" when a provider omits a monthly breakdown and avoid treating missing monthly cost as $0, preventing misleading displays and incorrect totals.
• Chores
  • Database schema and backend now preserve nullable monthly-cost values so "no data" vs explicit $0 is retained.

[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: acb510b7-1002-4443-8ead-b1c6e170e60a

📥 Commits

Reviewing files that changed from the base of the PR and between eccdc63 and f05dce0.

📒 Files selected for processing (2)

• internal/config/store_postgres_db_test.go
• internal/config/types.go

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

• internal/config/types.go

---

📝 Walkthrough

Walkthrough

Make monthly recurring cost nullable end-to-end: DB column becomes nullable, store scans map NULL to nil pointers, execution preserves pointers, API DTOs and aggregation keep nil vs zero semantics, tests updated, and frontend renders “—” for null costs.

Changes

Make MonthlyCost nullable throughout the stack

Layer / File(s)	Summary
Type definitions and database schema internal/config/types.go, internal/database/postgres/migrations/000063_purchase_history_monthly_cost_nullable.{up.sql,down.sql}	PurchaseHistoryRecord.MonthlyCost changes from float64 to *float64; migration drops NOT NULL on purchase_history.monthly_cost.
Storage layer — scanning, mapping, and tests internal/config/store_postgres.go, internal/config/store_postgres_additional_test.go, internal/config/store_postgres_comprehensive_test.go, internal/config/store_postgres_{coverage,db,test}.go, internal/config/store_postgres_unit_test.go	queryPurchaseHistory scans monthly_cost into sql.NullFloat64 and sets record.MonthlyCost only when valid; tests use pf(...) helper and assert nil round-trips.
Config types and unit tests internal/config/types_test.go, internal/config/store_postgres_unit_test.go	Type tests updated to expect pointer-form MonthlyCost, add require nil checks, and introduce pf test helper.
Execution layer — recording purchase history with pointers internal/purchase/execution.go	savePurchaseHistory now sets MonthlyCost from the source pointer and the derefFloat64 helper is removed.
API layer — DTOs, aggregation, and handler logic internal/api/types.go, internal/api/handler_history.go, internal/api/handler_history_test.go, internal/api/handler_inventory.go, internal/api/handler_inventory_test.go, internal/api/handler_per_account_perms_test.go	InventoryCommitment.MonthlyCost becomes *float64; history synthesis uses a pointer-aware summation helper that returns nil when all entries are nil; inventory coverage aggregation skips nil MonthlyCost; handler tests updated to use pointer fixtures and delta assertions.
Frontend contracts and nullable cost rendering frontend/src/api/types.ts, frontend/src/types.ts, frontend/src/inventory.ts	Frontend types allow `monthly_cost: number

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• LeanerCloud/CUDly#734: Both PRs modify internal/api/handler_history.go’s history-row field projection/summation for MonthlyCost (one preserves nil vs $0, the other populates/sums MonthlyCost for approval-queue rows).
• LeanerCloud/CUDly#792: Both PRs update frontend rendering logic for monthly_cost to display a muted dash when the API provides no monthly breakdown.

Suggested labels

impact/many

Poem

🐰
I hopped through types and DB rows,
Saved nil where the unknown grows,
Now history shows — or $ bright,
Nil and zero each in sight,
A tiny carrot for the fix.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.27% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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: making MonthlyCost nullable on PurchaseHistoryRecord, which is the primary objective of the PR.
Linked Issues check	✅ Passed	All objectives from issue #255 are met: MonthlyCost changed to *float64, SQL read/write paths updated, struct fields modified, execution.go simplified, and DB migration added.
Out of Scope Changes check	✅ Passed	All changes are directly related to making MonthlyCost nullable; no unrelated modifications found in the diff.

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

✨ 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/255-wave14

---

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

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

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

Fixed CI go-vet failure introduced by the nullable refactor:

• internal/api/handler_inventory_test.go lines 186, 196, 499, 508 still assigned untyped float constants (MonthlyCost: 80.0 etc.) after the field type changed to *float64. Wrapped each in float64Ptr(...) consistent with the helper already used elsewhere in the same file (lines 77, 90, 145, 430). Fixed in eccdc63.

Local verification: gofmt -l ./internal/api/, go vet ./..., go build ./... clean; go test ./internal/api/... ./internal/config/... -count=1 1940 pass.

@coderabbitai review

[coderabbitai]

@cristim Thanks for the clear write-up and local verification. The float64Ptr(...) wrapping at the four previously-missed assignment sites is consistent with the pattern already established elsewhere in the file — looks like a clean completion of the nullable refactor. Triggering the review now.

---

🧠 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-05-25T21:45:57.471Z
Learning: In this repository (CUDly), GCP CUD commitments are inherently monthly-billed. The GCP CUD purchase API (`providers/gcp/services/computeengine/client.go:350-373`, `buildCommitmentRequests`) takes only a `Plan` field (TWELVE_MONTH / THIRTY_SIX_MONTH) and never reads a payment-option field. Therefore, the only valid payment option for GCP in `ValidPaymentOptionsByProvider` is `"monthly"`. The `NormalizePaymentOption` GCP branch should collapse any non-monthly token (including legacy `"upfront"`) to `"monthly"`, with a WARN log at the `scheduler.convertRecommendations` boundary.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 277
File: frontend/src/__tests__/recommendations.test.ts:2472-2476
Timestamp: 2026-05-05T07:46:01.902Z
Learning: In the CUDly frontend (`frontend/src/recommendations.ts`), `effectiveSavingsPct` intentionally has NO plausibility guard (no null return or warning for reconstructed percentages above per-term ceilings). The maintainer explicitly rejected that approach in PR `#277`. The fix for inflated percentages (issue `#274`) is purely a data-path fix: plumbing the provider's canonical `on_demand_cost` through to the frontend so the denominator is not reconstructed from potentially misleading `monthly_cost + savings + amortized`. Tests that pin the reconstructed >80% behavior are intentional documentation of the pre-fix path, not bugs.

✅ 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

🧹 Nitpick comments (1)

internal/config/store_postgres_db_test.go (1)

699-737: ⚡ Quick win

Add a DB round-trip case for MonthlyCost: nil.

This subtest now pins 0.0 -> *float64(0), but the slice’s main contract is the distinction between NULL and 0.0. Please add a companion insert/read assertion with MonthlyCost: nil so the migration + store scan path proves NULL -> nil end to end.

🤖 Prompt for 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.

In `@internal/config/store_postgres_db_test.go` around lines 699 - 737, The test
currently only verifies a non-nil MonthlyCost pointer; add a companion subtest
that inserts a PurchaseHistoryRecord with MonthlyCost: nil and then reads it
back via store.SavePurchaseHistory and store.GetPurchaseHistory to assert the
retrieved record's MonthlyCost is nil (verifying NULL -> nil round-trip through
the migration and scan). Locate the existing test block that uses
PurchaseHistoryRecord, SavePurchaseHistory and GetPurchaseHistory and duplicate
the flow in a new t.Run (e.g., "MonthlyCost nil round-trip") setting MonthlyCost
to nil, calling SavePurchaseHistory(ctx, record) and then
GetPurchaseHistory(ctx, "123456789012", 10) and assert the returned record with
PurchaseID == "purchase-001" has MonthlyCost == nil.

🤖 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/config/types.go`:
- Around line 465-472: The comment on the MonthlyCost field incorrectly
references "GCP all-upfront" which is misleading; update the comment in types.go
(the block describing MonthlyCost and the Migration 000063 note) to remove the
"GCP all-upfront" example and mention only Azure (or a neutral phrasing) so it
reflects that GCP commitments are monthly-only—keep the rest of the explanation
about nil vs zero, frontend rendering, aggregation behavior, and the Migration
000063 NOT NULL change intact.

---

Nitpick comments:
In `@internal/config/store_postgres_db_test.go`:
- Around line 699-737: The test currently only verifies a non-nil MonthlyCost
pointer; add a companion subtest that inserts a PurchaseHistoryRecord with
MonthlyCost: nil and then reads it back via store.SavePurchaseHistory and
store.GetPurchaseHistory to assert the retrieved record's MonthlyCost is nil
(verifying NULL -> nil round-trip through the migration and scan). Locate the
existing test block that uses PurchaseHistoryRecord, SavePurchaseHistory and
GetPurchaseHistory and duplicate the flow in a new t.Run (e.g., "MonthlyCost nil
round-trip") setting MonthlyCost to nil, calling SavePurchaseHistory(ctx,
record) and then GetPurchaseHistory(ctx, "123456789012", 10) and assert the
returned record with PurchaseID == "purchase-001" has MonthlyCost == nil.

🪄 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: c444e917-5e59-4dac-95d4-136fbc28ef55

📥 Commits

Reviewing files that changed from the base of the PR and between 80c20d2 and eccdc63.

📒 Files selected for processing (21)

• frontend/src/api/types.ts
• frontend/src/inventory.ts
• frontend/src/types.ts
• internal/api/handler_history.go
• internal/api/handler_history_test.go
• internal/api/handler_inventory.go
• internal/api/handler_inventory_test.go
• internal/api/handler_per_account_perms_test.go
• internal/api/types.go
• internal/config/store_postgres.go
• internal/config/store_postgres_additional_test.go
• internal/config/store_postgres_comprehensive_test.go
• internal/config/store_postgres_coverage_test.go
• internal/config/store_postgres_db_test.go
• internal/config/store_postgres_test.go
• internal/config/store_postgres_unit_test.go
• internal/config/types.go
• internal/config/types_test.go
• internal/database/postgres/migrations/000063_purchase_history_monthly_cost_nullable.down.sql
• internal/database/postgres/migrations/000063_purchase_history_monthly_cost_nullable.up.sql
• internal/purchase/execution.go

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