Add modular billing credit ledger#212
Conversation
|
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a modular credit ledger system for billing, replacing the previous Razorpay routes. It adds a new src/billing package to manage credit estimates, reservations, debits, and grants, and integrates these operations into the memory ingest and job workflows. Feedback on the changes highlights several critical concurrency issues in src/billing/store.py where non-atomic state checks and updates could lead to double debits, duplicate refunds, or negative lot balances. Additionally, a severe security vulnerability was identified in the payment verification route where a missing checkout record bypasses ownership checks. Other recommendations include wrapping synchronous database calls in asyncio.to_thread to avoid blocking the FastAPI event loop, adding missing indexes on id fields in MongoDB collections, and safely parsing JSON in the webhook handler to prevent unhandled 500 errors.
|
| Filename | Overview |
|---|---|
| src/billing/store.py | 948-line core ledger. All critical mutations are now MongoDB-transaction-wrapped; DuplicateKeyError caught by concrete type; separate dicts for checkouts vs. webhook events. Previously flagged sequencing bugs are addressed. |
| src/api/routes/billing.py | Webhook handler now marks events processed after the grant, raises 400 on missing event ID, and rejects unknown package IDs; verify endpoint raises 400 when checkout is absent; payment_id fallback is empty string. |
| src/api/routes/v2/jobs.py | retry_job uses a single wide try block: InsufficientCredits returns 402, all other exceptions release the freshly-created reservation then call mark_failed. cancel_job releases billing only after the cancel+mark_cancelled block succeeds. |
| src/api/routes/v2/memory.py | Both ingest routes pre-compute job_id, set billing_reservation_created=True only on fresh reservations, and release only that flag in exception handlers — correctly avoiding release of already-running jobs' reservations. |
| src/billing/service.py | Clean thin service layer. commit_job_billing re-estimates (no buffer) at commit time; grant_pro_subscription/grant_topup use deterministic idempotency keys keyed on payment_id. |
| src/api/routes/v2/activities.py | mark_job_succeeded_activity commits billing before marking succeeded; mark_job_dead_letter_activity releases billing before marking dead-letter; both are idempotent on retry. |
Sequence Diagram
sequenceDiagram
participant Client
participant IngestRoute as ingest_memory_v2
participant BillingService
participant BillingStore
participant MongoDB
participant Temporal
Client->>IngestRoute: POST /v2/memory/ingest
IngestRoute->>BillingService: reserve_job_credits(user, job_id, job_type)
BillingService->>BillingStore: ensure_account (upsert + trial grant)
BillingStore->>MongoDB: transaction: lot + wallet + ledger
BillingService->>BillingStore: reserve_credits(account_id, job_id, amount)
BillingStore->>MongoDB: transaction: reservation(reserving→active) + wallet.$inc
BillingService-->>IngestRoute: "ReservationResult(created=True)"
IngestRoute->>Temporal: start_job_workflow(job)
alt Workflow start fails
Temporal--xIngestRoute: Exception
IngestRoute->>BillingStore: "release_reservation (if created=True)"
BillingStore->>MongoDB: transaction: reservation→released + wallet restore
IngestRoute-->>Client: 503
else Workflow starts
Temporal-->>IngestRoute: ok
IngestRoute-->>Client: 202 job handle
Note over Temporal: Job runs…
Temporal->>BillingStore: commit_debit (via mark_job_succeeded_activity)
BillingStore->>MongoDB: transaction: lots consumed + wallet updated + reservation committed + ledger
end
Reviews (13): Last reviewed commit: "Handle grant webhook and retry failure e..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
|
Addressed the relevant bot findings on the latest PR branch:
Validation run locally:
The broader API test subset could not run locally because this Windows environment is missing |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
✅ Staging Deployment Report
🟢 Staging is live and healthy! Test your changes at the staging URL above. Ready to ship? Comment |
🔍 API Schema Diff---REPORT--- 🔄 Modified
Auto-generated by API Schema Diff workflow |
1 participant
Summary
src/utils/billing.py.Tests
python -m pytest tests\test_billing.pypython -m compileall src\billing src\api\routes\billing.py src\api\routes\v2\memory.py src\api\routes\v2\activities.py src\api\routes\v2\jobs.py src\utils\billing.pygit diff --check