feat(subscriptions): scheduled AI delivery via Temporal (email + Slack) by vdekrijger · Pull Request #59631 · PostHog/posthog

vdekrijger · 2026-05-22T14:22:07Z

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

vdekrijger · 2026-05-22T14:22:22Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2026-05-22T14:22:53Z

🎭 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 · 2026-05-22T14:31:46Z

🔍 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-22 14:31 UTC (a6394b0)

vdekrijger · 2026-05-26T12:07:03Z

-            "reason": reason,
+            "reason": reason.description,
+            # The re-enable guidance doubles as the "what to do next" line in the email —
+            # `{target_type}` is interpolated for the channel-specific reasons.


I dont think we need to keep this 2 liner comment here, it's clear from reading the code 😃

vdekrijger · 2026-05-26T12:09:18Z

+
+        ctx = email_cls.call_args.kwargs["template_context"]
+        assert ctx["reason"] == AI_PROMPT_INVALID_DISABLE_REASON.description
+        assert "Edit the subscription with a valid prompt" in ctx["action_message"]


This assertion feels prone to become outdated, can we do better here?

Now asserts against AI_PROMPT_INVALID_DISABLE_REASON.user_message.format(target_type=...) instead of a hardcoded substring.

vdekrijger · 2026-05-26T12:22:16Z

+
+    @parameterized.expand(
+        [
+            # "Injection-shaped" phrasings used to be regex-rejected. We now accept them:


Honestly I think we should remove this test, it doesn't add any value and is solely there for legacy / historical reasons.

Removed; folded the valid injection-shaped cases into the parameterized test_accepts_valid (which now also asserts the salient content survives sanitization, not just non-empty).

vdekrijger · 2026-05-26T12:22:26Z

+    @patch("ee.hogai.ai_reports.MaxChatOpenAI")
+    @patch("ee.hogai.ai_reports.AssistantQueryExecutor")
+    @patch("ee.hogai.ai_reports.build_enriched_prompt")
+    def test_orchestrates_plan_query_synthesis(self, mock_build, mock_executor_cls, mock_llm_cls):


It feels like we can parameterize all htese tests! Please do so.

Parameterized into test_pipeline_returns_synthesized_markdown and test_query_fix_retry_loop.

vdekrijger · 2026-05-26T12:28:23Z

+        fix_llm.invoke.assert_called_once()
+
+
+class TestEmailHtmlSanitization(APIBaseTest):


Are we testing a library here? I don't think there is that much value into that.

Removed — that was effectively testing nh3.

vdekrijger · 2026-05-26T12:49:28Z

+                    )
+                )
+                last_error = exc
+        # If every recipient failed (typical AI sub has a single recipient), the


This makes an assumption that we have no clue about as we have 0 prod data.

Removed the assumption-laden comment.

vdekrijger · 2026-05-26T12:50:48Z

@@ -1,173 +0,0 @@
-import pytest
-


It feels odd that we added this in the last PR only to fully remove it from here; can we just completely remove it from the previously stacked PR and not have to deal with cleanup here? (incl the test)

Agreed the clean fix is to never-add it downstack. That's a multi-branch Graphite restack, so I've flagged it rather than doing it unilaterally (see top-level comment) — left the functional removal here for now. Happy to do the downstack cleanup on your go-ahead.

vdekrijger · 2026-05-26T12:51:41Z

+    # (snapshot) and go straight to `deliver_subscription`, instead of the
+    # default empty-assets SKIPPED short-circuit.
+    is_ai_prompt: bool = False
+    # Deprecated (TODO slug: subscriptions-patched-cleanup) — kept only so


This has since been cleaned up and can be rmeoved here? Probably a merge artifact

vdekrijger · 2026-05-26T12:52:08Z

    target_type: str = ""
+    # Set by `create_export_assets` for AI subscriptions, which have no insights
+    # to export. The workflow uses this to skip Phase 2 (export) + Phase 2.5
+    # (snapshot) and go straight to `deliver_subscription`, instead of the


As mentioned, would suggest a different flow in these cases.

delivery_id kept but reframed as the generate→deliver report reference — delivery reads the report back from the row rather than receiving it on the wire.

vdekrijger · 2026-05-26T12:53:08Z


+            # AI prompt subscriptions skip the per-insight export + snapshot phases;
+            # the LLM output is the report.
+            if prepare_result.is_ai_prompt:


As mentioned, this feels very iffy, I would suggest to split it in 2 separate workflows but reuse activities and the same scheduler mechanism.

Done — added ProcessAISubscriptionWorkflow, sharing the scheduler and the underlying activities.

Address review feedback on the AI delivery PR: - Route AI-prompt subscriptions to a dedicated ProcessAISubscriptionWorkflow (scheduler fan-out + value-change), reusing the shared activities and the same scheduler rather than threading AI flags through the insight/dashboard pipeline. - Move LLM work out of delivery into a generate_ai_subscription_report activity: consent gated up front (before any LLM cost), idempotent on retry, the report persisted to the SubscriptionDelivery row by reference (same content_snapshot / 2 MiB-boundary pattern insights use). deliver_subscription now just ships the already-generated report. - Drop the is_ai_prompt flag, the redundant delivery-time target check, and the workflow_run_id try/except fallback; fail loud (non-retryable) on missing report. - Hoist shared retry-policy constants + a recipient-dict helper used by both workflows. - Reorganize tests: sync unit tests in test_ai_subscriptions.py; async activity + end-to-end workflow tests in test_subscriptions_workflows.py (parameterized).

vdekrijger · 2026-05-27T08:01:00Z

Pushed c180251 addressing the review. Summary of how each thread was handled:

Architecture (the big one)

AI-prompt subscriptions now run a dedicated ProcessAISubscriptionWorkflow instead of being threaded through the insight/dashboard pipeline with flags:

The scheduler (ScheduleAllSubscriptionsWorkflow) and HandleSubscriptionValueChangeWorkflow route by content_type — AI subs → ProcessAISubscriptionWorkflow, everything else → ProcessSubscriptionWorkflow. Both share the same scheduler and the same activities; distinct child-ID prefixes keep the overlap-prevention guarantee per type.
A new generate_ai_subscription_report activity does the "decide what to send" work up front: it gates AI-data-processing consent before any LLM cost, runs the planner/HogQL/synthesis pipeline once, and persists the report markdown onto the SubscriptionDelivery row by reference (the same content_snapshot / ~2 MiB-boundary pattern insight snapshots already use). It's idempotent on Temporal redispatch (short-circuits if the report is already persisted, so retries don't re-bill the LLM).
deliver_subscription now just ships the already-generated report — no LLM work, no consent check (done up front), no redundant target check (done in validate_subscription_for_delivery). Terminal failures (consent revoked, prompt invalid, Slack disconnected) auto-disable; transient failures retry; a missing report fails non-retryable (re-running delivery can't regenerate it).
Dropped the is_ai_prompt flag and the insight_snapshots merge artifact; delivery_id is kept but reframed as the generate→deliver handoff reference.
Hoisted shared RetryPolicy constants + a _to_recipient_dicts helper used by both workflows; the AI workflow's docstring documents the deliberately-parallel scaffolding + keep-in-sync requirement (run-method control flow isn't extracted because Temporal workflow classes can't share it without sandbox-determinism risk).

Tests

Reorganized to test at the right level: sync unit tests stay in test_ai_subscriptions.py (sanitization, the generate_ai_report pipeline via the wrapper — parameterized, the email/Slack send helpers), and the activity + end-to-end workflow tests moved to test_subscriptions_workflows.py (async / ActivityEnvironment / WorkflowEnvironment), including a full scheduler→routing→generate→deliver→COMPLETED run and the fetch_due activity confirming an AI sub is picked up with its content_type. Removed the library (nh3) test and the legacy injection-shaped test (folded the valid cases into the accepts-valid parameterization).

One thing I did not do unilaterally

The prompt_sanitization add-then-remove: removing it from the downstack PR so it's never added is a multi-branch Graphite restack. I left the functional removal in place here and am flagging it rather than restructuring the stack — happy to do the downstack cleanup if you want, just confirm.

Verification

ruff clean; 78 tests collect cleanly. The full test execution run is best left to CI here — the shared local test DB was too contended to get a clean end-to-end run locally (every failure was at DB-create / connection-drop, never an assertion).

Reviewed via a local multi-persona review swarm; iterated until the remaining findings were all low-severity.

feat(subscriptions): scheduled AI delivery via Temporal (email + Slack)

a6394b0

This was referenced May 22, 2026

feat(subscriptions): bring subscriptions to the free tier (freemium, synced to alerts) #59624

Open

feat(subscriptions): gate AI summaries on AI credit budget #59625

Open

vdekrijger mentioned this pull request May 22, 2026

feat: add AI prompt-based subscriptions #58809

Closed

vdekrijger commented May 26, 2026

View reviewed changes

		fix_llm.invoke.assert_called_once()


		class TestEmailHtmlSanitization(APIBaseTest):

Conversation

vdekrijger commented May 22, 2026

Problem

Changes

How did you test this code?

Publish to changelog?

Docs update

🤖 Agent context

Uh oh!

vdekrijger commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 22, 2026

Uh oh!

github-actions Bot commented May 22, 2026

🔍 Migration Risk Analysis

❌ Blocked

📚 How to Deploy These Changes Safely

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdekrijger commented May 27, 2026

Architecture (the big one)

Tests

One thing I did not do unilaterally

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vdekrijger commented May 22, 2026 •

edited

Loading