Skip to content

fix(purchase): preserve unknown provider upfront costs - #450

Merged
cristim merged 1 commit into
mainfrom
fix/446-nullable-purchase-cost
Sep 29, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/446-nullable-purchase-cost

Conversation

@cristim

@cristim cristim commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Preserve unknown provider purchase costs as NULL instead of treating them as zero. Pin all four cloud-commitments-go modules to ce951361290141aa53b528dfce862fbc22a1d01b and carry nullable upfront costs through purchase history, complete API totals, snapshots, coverage, logs and frontend rendering. Known zero remains numeric zero. Marketplace pricing rejects an unknown basis before any listing side effect, including custom schedules.

Migration 000101 removes NOT NULL and zero defaults from purchase_history.upfront_cost and savings_snapshots.total_commitment. Historical zeros remain unchanged. Existing materialized-view total_commitment columns remain known-sample statistics without a completeness guarantee; current APIs do not read those columns. No views are rebuilt.

Apply the migration before deploying nullable writers. Deploy the matching frontend and backend together, and avoid old readers once NULL rows exist. The down migration refuses NULL-bearing data rather than inventing costs. This PR does not apply production migrations.

Validation:

  • Backend race tests, build, vet, pinned golangci-lint and normal commit hooks.
  • Real PostgreSQL 17 migrations and store/query round trips: NULL, zero, positive, legacy zero, known monthly cost, grouped totals, snapshot COPY(NULL), explicit JSON null and rollback refusal. Local test overlay replaces only Docker setup with a session-owned PostgreSQL connection; committed tests retain the CI container harness.
  • Five regression mutations fail the intended assertions. The monthly-cost reconciliation regression also fails its existing test before correction.
  • Frontend: 3,000 tests passed, production build, lint and a Chromium history/inventory amortization scenario using fixture API responses.
  • Independent gpt-6-astra review and independent focused race/PostgreSQL verification. CodeRabbit is replaced by local independent review for this session.

Closes #446

Summary by CodeRabbit

  • Bug Fixes
    • Unknown upfront costs are now kept distinct from zero across purchase history, inventory, and savings totals. Cost filters no longer match unknown values, and amortized monthly costs remain unavailable when the upfront cost is unknown.
    • Marketplace sales are blocked when the purchase’s upfront cost is unknown; the Sell button is disabled and an error is shown.
    • Coverage percentages are left unavailable when covered costs are unknown.

Propagate nullable purchase costs through storage, complete totals,
coverage, logs and UI. Reject marketplace pricing without a known basis.
Keep known zero distinct and pin the reviewed provider library revision.

Closes #446
@cristim cristim added type/chore Maintenance / non-user-visible priority/p1 Next up; this sprint effort/s Hours labels Sep 29, 2026
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 8a8948e6-bab3-4533-91b3-df0204c82e3c

📥 Commits

Reviewing files that changed from the base of the PR and between 0959adc and 83fc563.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (46)
  • frontend/src/__tests__/history-column-filters.test.ts
  • frontend/src/__tests__/history-marketplace-sell-button.test.ts
  • frontend/src/__tests__/history.test.ts
  • frontend/src/__tests__/inventory.test.ts
  • frontend/src/api/types.ts
  • frontend/src/history.ts
  • frontend/src/inventory.ts
  • frontend/src/types.ts
  • frontend/tests-e2e/nullable-purchase-cost.spec.ts
  • go.mod
  • internal/analytics/collector.go
  • internal/analytics/collector_test.go
  • internal/analytics/interfaces.go
  • internal/analytics/nullable_purchase_cost_test.go
  • internal/analytics/postgres_analytics.go
  • internal/analytics/postgres_analytics_db_test.go
  • internal/analytics/postgres_analytics_integration_test.go
  • internal/analytics/postgres_analytics_pgxmock_test.go
  • internal/analytics/postgres_analytics_test.go
  • internal/api/analytics_postgres.go
  • internal/api/analytics_postgres_test.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_marketplace.go
  • internal/api/handler_marketplace_test.go
  • internal/api/handler_per_account_perms_test.go
  • internal/api/nullable_purchase_cost_integration_test.go
  • internal/api/nullable_purchase_cost_test.go
  • internal/api/openapi.yaml
  • internal/api/purchase_pricing.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/types.go
  • internal/config/types_test.go
  • internal/database/postgres/migrations/000078_monthly_summary_nested_rollup_test.go
  • internal/database/postgres/migrations/000101_nullable_purchase_upfront_cost.down.sql
  • internal/database/postgres/migrations/000101_nullable_purchase_upfront_cost.up.sql
  • internal/purchase/execution.go
  • internal/purchase/execution_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

Purchase upfront costs now distinguish unknown values from zero across database storage, analytics, API responses, and frontend behavior. Unknown costs remain null in totals and coverage calculations, and marketplace listing rejects purchases with unknown upfront costs.

Changes

Nullable purchase-cost input and storage

Layer / File(s) Summary
Purchase cost input and database schema
go.mod, internal/purchase/execution.go, internal/purchase/execution_test.go, internal/database/postgres/migrations/000101_nullable_purchase_upfront_cost.*.sql
The updated purchase dependency supplies nullable costs. Purchase success logs format known costs to two decimal places and log unknown for nil costs. Migration 101 allows null upfront costs and snapshot commitments; its down migration fails if null values prevent restoring NOT NULL.
Nullable purchase-history storage
internal/config/types.go, internal/config/store_postgres.go, internal/config/*test.go
Purchase-history records use *float64 for upfront cost. PostgreSQL readers convert SQL NULL to nil and preserve valid values, including zero.

Analytics and API behavior

Layer / File(s) Summary
Nullable commitment analytics
internal/analytics/*
Commitment aggregation marks a bucket unknown if any contributing purchase has a nil upfront cost. Snapshot construction and PostgreSQL reads preserve null commitment values.
History analytics and API responses
internal/api/analytics_postgres.go, internal/api/handler_history.go, internal/api/types.go, internal/api/openapi.yaml, internal/api/nullable_purchase_cost_test.go
History and breakdown totals remain null when contributing costs are unknown. API response fields support nullable upfront totals, and history summaries accumulate costs with nullable values.
Inventory coverage and marketplace handling
internal/api/handler_inventory.go, internal/api/handler_marketplace.go, internal/api/purchase_pricing.go, internal/api/nullable_purchase_cost*.go, internal/api/handler_*_test.go
Coverage calculations preserve unknown covered costs and percentages. Marketplace pricing rejects nil upfront costs before creating a listing. Integration tests cover database migration, nullable values, analytics, snapshots, and rollback behavior.

Frontend handling

Layer / File(s) Summary
Nullable-cost displays, filters, and actions
frontend/src/api/types.ts, frontend/src/types.ts, frontend/src/history.ts, frontend/src/inventory.ts, frontend/src/__tests__/*, frontend/tests-e2e/nullable-purchase-cost.spec.ts
Frontend types accept null cost values. History filters exclude unknown values from numeric matches, amortized displays show unavailable values when cost is unknown, and marketplace handling disables Sell without opening the pricing dialog.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 83fc5

Unknown upfront costs are now kept as null instead of being turned into zero, across storage, totals, and the UI. No blocking issues were found. Follow the documented order: apply the migration first, then deploy the backend and frontend together.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.95% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 91 functions across 42 files. (4 skipped:… 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 and concisely describes the primary change: preserving unknown provider upfront costs.
Linked Issues check ✅ Passed PR #450 satisfies the coding requirements in directly linked issue #446. go.mod re-pins all four cloud-commitments-go modules to ce951361290141aa53b528dfce862fbc22a1d01b. `PurchaseHistoryRecord.…
Out of Scope Changes check ✅ Passed The changes remain within the linked issue and PR scope. Storage migration, API propagation, analytics totals, coverage, logging, frontend rendering, marketplace guards, and regression tests all suppo…
Full details: Docstring Coverage

Explanation

Docstring coverage is 54.95% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 91 functions across 42 files. (4 skipped: 4 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours priority/p1 Next up; this sprint type/chore Maintenance / non-user-visible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build(deps): re-pin cloud-commitments-go past #151 and migrate PurchaseResult.Cost (*float64)

1 participant