Skip to content

fix(billing): atomize usage_log and userStats writes via central recordUsage()#3767

Merged
waleedlatif1 merged 6 commits intostagingfrom
waleedlatif1/billing-metering-fix
Mar 25, 2026
Merged

fix(billing): atomize usage_log and userStats writes via central recordUsage()#3767
waleedlatif1 merged 6 commits intostagingfrom
waleedlatif1/billing-metering-fix

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Central recordUsage() wraps usage_log INSERT + userStats UPDATE in a single Postgres transaction, fixing drift between currentPeriodCost and usage_log
  • All cost mutation paths (workflow execution, copilot, wand) now go through recordUsage()
  • Removed duplicate tool hosting cost logging (logFixedUsage) — tool costs already included in model totals via provider response
  • Cleaned up dead code: skipFixedUsageLog, old helper functions, stale test mocks

Type of Change

  • Bug fix

Testing

  • All 4557 tests pass
  • Verified no remaining references to removed functions

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Mar 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 25, 2026 8:41pm

Request Review

@cursor
Copy link

cursor bot commented Mar 25, 2026

PR Summary

Medium Risk
Touches billing-critical persistence by moving multiple call sites to a new transactional recordUsage() path; mistakes could under/over-count cost or fail updates if userStats rows are missing.

Overview
Centralizes and atomizes billing writes. Replaces logModelUsage/logFixedUsage/logWorkflowUsageBatch plus ad-hoc userStats updates with a single transactional recordUsage() that inserts usage_log entries and updates userStats (including reserved-field protection and rollback if the userStats row is missing).

Migrates callers to the new path. Updates the billing cost update API, wand usage tracking, and workflow execution completion (ExecutionLogger) to build entries + additionalStats and call recordUsage().

Prevents tool-cost double counting and removes dead flags. Stops logging hosted-key tool costs as separate fixed usage entries (costs are expected to flow through provider totals), removes skipFixedUsageLog and related tool config/tests, and updates mocks/expectations accordingly.

Written by Cursor Bugbot for commit d1e72b4. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 25, 2026

Greptile Summary

This PR fixes cost-drift between usage_log and userStats.currentPeriodCost by centralizing all billing mutations through a new recordUsage() function that wraps the usage_log INSERT and userStats UPDATE in a single Postgres transaction. It also eliminates the double-counting that existed when tool hosting costs were logged as separate fixed entries via logFixedUsage while also flowing into the provider's cost.total.

Key changes:

  • usage-log.ts: Replaces logModelUsage, logFixedUsage, and logWorkflowUsageBatch with recordUsage(), which atomically writes all entries and updates userStats in one transaction. Includes a RESERVED_KEYS guard to prevent callers from overriding core billing fields via additionalStats, and re-throws on missing userStats rows so callers can handle the failure explicitly.
  • update-cost/route.ts (copilot) & wand/route.ts: Both paths migrate to recordUsage(). The wand route correctly renames its local streaming closure from recordUsage to flushUsage to avoid shadowing the import.
  • logger.ts (execution): Migrates from logWorkflowUsageBatch to recordUsage() and introduces TRIGGER_COUNTER_MAP to replace a verbose switch-case. The call is wrapped in a try/catch so billing failures do not abort workflow execution.
  • tools/index.ts: Removes the logFixedUsage call from processHostedKeyCost since tool costs already roll up into the execution's costSummary and are recorded atomically at completion.
  • Minor: the JSDoc for recordUsage() slightly misrepresents the early-return condition after the additionalStats guard was added, and the !additionalStats check in the early-return path does not catch empty {} objects. The sql.raw() pattern used for trigger column names in TRIGGER_COUNTER_MAP is safe with current hardcoded values but bypasses Drizzle ORM type safety.

Confidence Score: 5/5

  • Safe to merge — all three previously-identified P0/P1 concerns are resolved and only minor style nits remain.
  • The core atomicity fix is correct and well-structured. The three prior review concerns (reserved-key override, zero-cost counter skipping, silent failure swallowing) have each been addressed with appropriate guards and re-throw behavior. The remaining comments are non-blocking P2 style suggestions: a slightly inaccurate JSDoc, an early-return guard that misses empty objects, and sql.raw() usage for hardcoded column names. None of these affect correctness or production reliability in current usage.
  • No files require special attention — usage-log.ts and logger.ts have minor style nits but no correctness issues.

Important Files Changed

Filename Overview
apps/sim/lib/billing/core/usage-log.ts Replaces three separate helpers (logModelUsage, logFixedUsage, logWorkflowUsageBatch) with a single recordUsage() that wraps usage_log INSERT + userStats UPDATE in one Postgres transaction; includes RESERVED_KEYS guard and re-throws on missing userStats row. Minor: JSDoc comment slightly inaccurate after additionalStats fix; early-return guard doesn't check for empty {} objects.
apps/sim/lib/logs/execution/logger.ts Migrated from logWorkflowUsageBatch to recordUsage(); added TRIGGER_COUNTER_MAP for concise trigger-to-column mapping. The sql.raw(triggerCounter.column) pattern is safe with current hardcoded constants but bypasses Drizzle ORM type-safety for column names. recordUsage() call is inside a try/catch so billing failures don't break execution.
apps/sim/app/api/billing/update-cost/route.ts Migrated copilot billing path to recordUsage(); removed inline userStats check and two separate write operations. Copilot-specific counters correctly placed in additionalStats so they're always incremented even on zero-cost calls.
apps/sim/app/api/wand/route.ts Migrated wand billing to recordUsage(); correctly renamed local recordUsage closure to flushUsage to avoid name shadowing with the imported function.
apps/sim/tools/index.ts Removed logFixedUsage from processHostedKeyCost since tool costs are now included in the provider's cost.total and recorded atomically at execution completion, preventing double-counting.
apps/sim/tools/types.ts Removed skipFixedUsageLog field from ToolHostingConfig — no longer needed now that separate fixed usage logging is eliminated.
apps/sim/tools/index.test.ts Cleaned up test mocks for logFixedUsage and related assertions; test coverage for the removed behavior is correctly dropped.

Sequence Diagram

sequenceDiagram
    participant Caller as Caller<br/>(ExecutionLogger / update-cost / wand)
    participant RU as recordUsage()
    participant TX as Postgres Transaction
    participant UL as usage_log table
    participant US as userStats table

    Caller->>RU: recordUsage({ userId, entries, additionalStats })
    alt billing disabled
        RU-->>Caller: return (no-op)
    else no valid entries AND no additionalStats
        RU-->>Caller: return (no-op)
    else
        RU->>RU: filter entries (cost > 0), sum totalCost
        RU->>RU: strip RESERVED_KEYS from additionalStats → safeStats
        RU->>TX: begin transaction
        alt validEntries.length > 0
            TX->>UL: INSERT rows (one per entry)
        end
        TX->>US: UPDATE SET totalCost+=, currentPeriodCost+=, lastActive, ...safeStats WHERE userId
        alt userStats row not found
            TX-->>RU: result = []
            RU->>TX: THROW (rollback)
            TX-->>Caller: error propagated
        else
            TX-->>RU: commit
            RU-->>Caller: void (success)
        end
    end
Loading

Reviews (2): Last reviewed commit: "chore(lint): fix formatting in hubspot l..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 force-pushed the waleedlatif1/billing-metering-fix branch from cd79dcf to d1e72b4 Compare March 25, 2026 20:27
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@waleedlatif1 waleedlatif1 merged commit f94be08 into staging Mar 25, 2026
3 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/billing-metering-fix branch March 25, 2026 20:41
Sg312 pushed a commit that referenced this pull request Mar 25, 2026
…rdUsage (#3767)

* fix(billing): atomize usage_log and userStats writes via central recordUsage()

* fix(billing): address PR review — re-throw errors, guard reserved keys, handle zero-cost counters

* chore(lint): fix formatting in hubspot list_lists.ts from staging

* fix(billing): tighten early-return guard to handle empty additionalStats object

* lint

* chore(billing): remove implementation-decision comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant