Skip to content

feat(subscriptions): add AI-prompt subscription fields + activity logging#59629

Draft
vdekrijger wants to merge 2 commits into
subscriptions-ai-summary-credit-gatefrom
ai-model
Draft

feat(subscriptions): add AI-prompt subscription fields + activity logging#59629
vdekrijger wants to merge 2 commits into
subscriptions-ai-summary-credit-gatefrom
ai-model

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 SQL Changes

Hey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:

posthog/migrations/1177_subscription_ai_fields.py

BEGIN;
--
-- Add field resource_type to subscription
--
ALTER TABLE "posthog_subscription" ADD COLUMN "resource_type" varchar(20) DEFAULT 'insight' NOT NULL;
ALTER TABLE "posthog_subscription" ALTER COLUMN "resource_type" DROP DEFAULT;
--
-- Add field prompt to subscription
--
ALTER TABLE "posthog_subscription" ADD COLUMN "prompt" text NULL;
--
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL
COMMIT;

Last updated: 2026-05-26 13:49 UTC (f4952a3)

@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: resource_type
  │  └─ #2 ✅ AddField
  │     Adding nullable field requires brief lock
  │     model: subscription, field: prompt
  │  └─ #3 ⚠️ RunPython: RunPython data migration needs review for performance
  │
  └──> �[91m⚠️  COMBINATION RISKS:�[0m
       ❌ BLOCKED: #3 RunPython + #1 AddField, #2 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-26 13:50 UTC (f4952a3)

Comment thread posthog/migrations/1177_subscription_ai_fields.py Outdated
Comment thread posthog/models/subscription.py Outdated
Comment thread posthog/models/subscription.py Outdated
if not created and is_scheduler_save:
return
try:
from posthog.models.activity_logging.activity_log import Detail, log_activity
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.

If we decide to do this activity log thing, we should at least move this to a global import instead of a local one.

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.

Done — activity logging now lives in a top-level @mutable_receiver(model_activity_signal) handler, so log_activity / Detail / changes_between are imported at module level (no inline import).

Comment thread posthog/models/subscription.py Outdated
detail=Detail(
name=instance.title or (instance.prompt or "")[:60],
short_id=None,
changes=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.

Honestly, given that you claim that the AI subscriptions carry value to log activities (which I think is fair in order to allow undos to prompts and what not), I think we should track the changes to the prompt / model in the details here as well so that we can track what changed and undo / redo them?

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.

Done — switching to ModelActivityMixin means changes_between auto-computes the field diff (before/after for prompt, etc.) into Detail.changes, so prompt edits are recoverable from the audit trail. Added tests covering create, prompt edits, and None↔prompt transitions. (target_value stays masked via the existing config.)

Comment thread posthog/models/subscription.py Outdated
organization_id=instance.team.organization_id,
team_id=instance.team_id,
user=instance.created_by,
was_impersonated=False,
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 do we do this in other placeS? Shouldn't this be some function call?

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.

Good call — it now follows the standard pattern: Subscription inherits ModelActivityMixin and a @mutable_receiver(model_activity_signal, sender=Subscription) handler logs the activity, mirroring AlertConfiguration. Scheduler-only saves (next_delivery_date) are suppressed via signal_exclusions so a schedule bump isn't logged.

Comment thread posthog/migrations/1177_subscription_ai_fields.py Outdated
Comment thread posthog/models/subscription.py Outdated
SubscriptionFrequency.YEARLY: YEARLY,
}

content_type = models.CharField(
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.

In this PR, we don't set the content type in the API / during sub creation, which means that there will be a gap in the column until the frontend has caught up; can we consider adding a backend thing here so that there is no gap between the migration and the frontend? and that we can consider all subs having a content type?

- rename content_type -> resource_type (avoids clash with django contenttypes)
- drop ai_config (YAGNI; not part of MVP)
- move activity logging onto ModelActivityMixin + a model_activity_signal
  receiver (matches AlertConfiguration), so changes are auto-diffed for the
  audit trail and the activity_log import is module-level
- exclude next_delivery_date from both the signal and the change diff so
  scheduler bumps are not logged
- fix AI resource kind ("AI" not "AI report") so email subjects read correctly
- extract ai_display_name + AI_PROMPT_DISPLAY_MAX_LEN to remove duplication
- add activity-logging tests (create/update/scheduler/soft-delete/transitions)
@vdekrijger vdekrijger changed the title feat(subscriptions): add AI-prompt content_type, prompt, ai_config fields feat(subscriptions): add AI-prompt subscription fields + activity logging May 27, 2026
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