Skip to content

feat(subscriptions): gate AI subscriptions on AI credit budget#59634

Draft
vdekrijger wants to merge 1 commit into
ai-mcpfrom
ai-credit-gate
Draft

feat(subscriptions): gate AI subscriptions on AI credit budget#59634
vdekrijger wants to merge 1 commit into
ai-mcpfrom
ai-credit-gate

Conversation

@vdekrijger
Copy link
Copy Markdown
Contributor

Problem

Changes

How did you test this code?

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Publish to changelog?

Docs update

🤖 Agent context

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down.

Add the run-playwright label if you want an E2E sweep before merging — CI will pick it up automatically.

Most PRs don't need this. Real regressions still get caught on master and fix-forward.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

🔍 Migration Risk Analysis

We've analyzed your migrations for potential risks.

Summary: 0 Safe | 0 Needs Review | 1 Blocked

❌ Blocked

Causes locks or breaks compatibility

posthog.1177_subscription_ai_fields
  │  └─ #1 ✅ AddField
  │     Adding NOT NULL field with constant default (safe in PG11+)
  │     model: subscription, field: content_type
  │  └─ #2 ✅ AddField
  │     Adding nullable field requires brief lock
  │     model: subscription, field: prompt
  │  └─ #3 ✅ AddField
  │     Adding nullable field requires brief lock
  │     model: subscription, field: ai_config
  │  └─ #4 ⚠️ RunPython: RunPython data migration needs review for performance
  │
  └──> �[91m⚠️  COMBINATION RISKS:�[0m
       ❌ BLOCKED: #4 RunPython + #1 AddField, #2 AddField, #3 AddField
       RunPython data migration combined with schema changes. Data
       migrations can hold locks during execution, especially on large
       tables. Split into separate migrations: 1) schema changes, 2)
       data migration.

📚 How to Deploy These Changes Safely

AddField:

This operation acquires a brief lock but doesn't rewrite the table.

Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up.

RunPython:

Use batching for large data migrations:

  • Use .iterator() to avoid loading all rows into memory
  • Use .bulk_update() instead of saving individual objects
  • Batch size: 1,000-10,000 rows per batch
  • Add pauses between batches
  • Consider background jobs for very large updates (millions of rows)

See the migration safety guide

Last updated: 2026-05-27 08:06 UTC (cb7ecf6)

Comment thread ee/api/subscription.py
# adapted: ValidationError there → 403 Response here. Helper-driven so a
# new gate (e.g. quota) only needs adding in one place.
gate_reason = _ai_create_gate_reason(self.organization, kind="reports", verb="generating")
if gate_reason is not None:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Re: line +1015]

iirc this function will be removed, so no need to make the changes here.

See this comment inline on Graphite.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similar to the test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reverted — removed the AI-credit gate from ai_report (and its test test_ai_report_endpoint_rejects_when_over_ai_credit_limit) plus the now-unused import, since this endpoint is going away.

Comment thread ee/billing/quota_limiting.py Outdated
Comment on lines +193 to +195
"""Whether the team has exhausted its AI credit budget, per the billing quota-limiting cache —
the same signal the chat assistant enforces (ee/api/conversation.py). Shared by every
LLM-spending subscription path (AI reports, AI summaries) so the resource + cache-key pair lives
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we can simplify this pydoc imo, lets just focus on the why! and we should probably also update the checks around the AI summaries in the "normal" (insight / dashboard) subscriptions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplified the docstring to focus on the why. Also routed the AI-summary path (snapshot_activities.py) through the shared is_team_over_ai_credit_budget helper, and migrated the remaining AI-credit callers (conversation.py Max chat, subscription.py summary gate) so every LLM-spending path checks the same resource + cache-key pair.

"""Notify the subscription owner that a scheduled AI report was skipped because the org
is out of AI credits. `billing_period_key` keys the campaign so MessagingRecord dedups to
one notice per billing period even if the skip path runs more than once — this, not the
reschedule alone, is what guarantees we email at most once per credit-reset cycle."""
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can simplify this pydoc as well imo.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplified the docstring.

super().setUp()
# An AI subscription can only exist for an org that approved AI data processing — creation
# is gated on it. Delivery re-checks consent at send time, so the suite must reflect the
# real precondition; otherwise every delivery short-circuits as `ai_consent_revoked`.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

imo we can remove this pydoc!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the comment.

@patch("posthog.temporal.subscriptions.activities.send_email_ai_subscription_credit_limited")
@patch("posthog.temporal.subscriptions.activities.generate_ai_subscription_markdown")
@patch("posthog.temporal.subscriptions.activities.is_team_over_ai_credit_budget")
def test_cached_markdown_delivers_even_when_over_credit_limit(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we want to reevaluate how we do this caching behaviour thing, so we might have to change this test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Left as-is for now — per your note the caching flow is being reworked downstack (#59631), so I didn't want to collide with that in-flight change. This test will get updated when that lands.

"""When the org's AI credits next reset — the billing-period end synced from billing."""
usage = subscription.team.organization.usage
period = usage.get("period") if usage else None
if period and len(period) == 2 and period[1]:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How does the period model look like? What is on index 0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

organization.usage["period"] is [current_period_start, current_period_end] as ISO strings (set in billing_manager.py). Index 0 is the period start, index 1 the end — which is when AI credits reset, so the code reads period[1]. Added a clarifying comment.

Persisting `next_delivery_date = reset_date` is deliberate: the always-runs
`advance_next_delivery_date` activity recomputes from this value (`rrule.after(reset_date)`),
so the next delivery lands on the first on-schedule slot AFTER credits reset rather than the
next normal occurrence (which could fall before the reset and re-fire while still over-limit).
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same with this pydoc, please focus on the why

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Simplified — kept just the non-obvious why (persisting next_delivery_date = reset_date so the always-runs advance_next_delivery_date recomputes the next slot past the credit reset).

The ad-hoc ai_report endpoint and scheduled AI deliveries enforce the org's AI
credit budget via a shared is_team_over_ai_credit_budget helper — the same billing
signal the chat assistant (ee/api/conversation.py) and the AI-summary path use.

- ai_report endpoint returns 402 when over budget, before spending tokens
- scheduled deliveries fail open, skip + reschedule past the credit reset, notify
  the owner once per period, and increment a Prometheus counter (parity with the
  AI-summary skip metric). Cache hits still deliver — their tokens were already spent.
- shared helper in ee/billing/quota_limiting keeps the resource + cache-key pair in
  one place so the AI-report and AI-summary paths can't drift
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