chore(workflows): drop the dead email reputation snapshot table - #75277
chore(workflows): drop the dead email reputation snapshot table#75277meikelmosby wants to merge 11 commits into
Conversation
…ation snapshots Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
…low rows Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
…letion Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
…ation tab Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
…dering Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
…den findings fetch Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
Generated-By: PostHog Code Task-Id: 0f3e9835-ed9b-469d-aa52-1152818572a2
🦔 ReviewHog reviewed this pull requestFound 1 must fix, 0 should fix, 0 consider. Published 1 finding (view the review). |
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
posthog/tasks/email.py |
20.0% | 642–649, 654, 664–666 |
products/workflows/backend/tasks/ses_tenant_state.py |
50.0% | 16, 27, 33–42 |
products/workflows/backend/services/sns_verification.py |
73.6% | 44–55, 86–87, 90, 94, 100–102 |
products/workflows/backend/api/ses_events_webhook.py |
83.9% | 45, 52, 61–62, 64, 82, 86–87, 93–94 |
products/workflows/backend/services/ses_tenant_state.py |
89.2% | 29–32 |
products/workflows/backend/api/test/test_email_reputation.py |
89.7% | 21–22, 126, 128, 142, 148, 152–154 |
products/workflows/backend/providers/ses.py |
93.8% | 63, 78 |
products/workflows/backend/api/hog_flow.py |
98.7% | 1173 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 30533499998 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
batch_exports |
████████░░░░░░░░░░░░ 39.4% |
8,621 / 21,874 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
██████████████░░░░░░ 69.7% |
31,947 / 45,819 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
cdp |
████████████████░░░░ 81.0% |
3,146 / 3,883 |
signals |
████████████████░░░░ 81.2% |
23,854 / 29,387 |
wizard |
█████████████████░░░ 84.8% |
935 / 1,102 |
data_modeling |
█████████████████░░░ 85.3% |
7,343 / 8,610 |
notebooks |
█████████████████░░░ 86.0% |
7,794 / 9,060 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
exports |
██████████████████░░ 88.2% |
7,046 / 7,986 |
data_warehouse |
██████████████████░░ 88.7% |
12,095 / 13,635 |
business_knowledge |
██████████████████░░ 89.0% |
4,391 / 4,936 |
engineering_analytics |
██████████████████░░ 89.4% |
6,441 / 7,202 |
visual_review |
██████████████████░░ 89.5% |
5,837 / 6,522 |
conversations |
██████████████████░░ 89.6% |
17,234 / 19,241 |
alerts |
██████████████████░░ 90.0% |
4,342 / 4,827 |
mcp_analytics |
██████████████████░░ 90.2% |
2,883 / 3,198 |
links |
██████████████████░░ 90.6% |
183 / 202 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
error_tracking |
██████████████████░░ 91.0% |
10,925 / 12,004 |
slack_app |
██████████████████░░ 91.1% |
9,554 / 10,492 |
stamphog |
██████████████████░░ 91.1% |
4,056 / 4,450 |
marketing_analytics |
██████████████████░░ 91.2% |
12,058 / 13,222 |
mcp_store |
██████████████████░░ 92.3% |
4,279 / 4,634 |
product_analytics |
███████████████████░ 92.5% |
5,849 / 6,321 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
early_access_features |
███████████████████░ 92.6% |
1,287 / 1,390 |
notifications |
███████████████████░ 92.6% |
1,017 / 1,098 |
ai_observability |
███████████████████░ 93.0% |
15,354 / 16,517 |
surveys |
███████████████████░ 93.1% |
5,771 / 6,197 |
web_analytics |
███████████████████░ 93.2% |
14,829 / 15,906 |
posthog_ai |
███████████████████░ 93.2% |
1,326 / 1,422 |
approvals |
███████████████████░ 93.3% |
3,437 / 3,682 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
workflows |
███████████████████░ 93.6% |
7,348 / 7,853 |
legal_documents |
███████████████████░ 93.8% |
1,628 / 1,736 |
tracing |
███████████████████░ 94.5% |
2,670 / 2,826 |
endpoints |
███████████████████░ 94.6% |
8,696 / 9,192 |
skills |
███████████████████░ 94.6% |
3,158 / 3,337 |
review_hog |
███████████████████░ 94.6% |
6,912 / 7,303 |
messaging |
███████████████████░ 94.7% |
2,885 / 3,048 |
logs |
███████████████████░ 95.5% |
10,439 / 10,928 |
experiments |
███████████████████░ 95.6% |
25,831 / 27,031 |
growth |
███████████████████░ 96.1% |
3,245 / 3,376 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
revenue_analytics |
███████████████████░ 96.3% |
1,887 / 1,960 |
feature_flags |
███████████████████░ 96.5% |
17,387 / 18,023 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
replay_vision |
███████████████████░ 96.6% |
15,889 / 16,455 |
access_control |
███████████████████░ 96.9% |
870 / 898 |
customer_analytics |
███████████████████░ 97.1% |
9,797 / 10,085 |
warehouse_sources |
███████████████████░ 97.2% |
346,998 / 356,853 |
data_catalog |
████████████████████ 97.7% |
2,555 / 2,615 |
analytics_platform |
████████████████████ 98.0% |
2,153 / 2,197 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
⚠️ Django migration SQL — 1 new migration to review
We've detected new migrations on this PR. Review the SQL output for each migration:
products/workflows/backend/migrations/0018_drop_emailreputationsnapshot_table.py
BEGIN;
--
-- Raw SQL operation
--
DROP TABLE IF EXISTS "posthog_emailreputationsnapshot";
COMMIT;Last updated: 2026-07-30 10:12 UTC (74b0611)
✅ Django migration risk — migration analysis complete
We've analyzed your migrations for potential risks.
Summary: 2 Safe | 2 Needs Review | 0 Blocked
⚠️ Needs Review
May have performance impact
workflows.0016_drop_emailreputationsnapshot_fk
└─ #1 ✅ SeparateDatabaseAndState
Wrapper operation - see nested operations for risk: RunSQL
database_operations: RunSQL
└─ #2 ⚠️ RunSQL: DROP CONSTRAINT is fast but needs deployment safety review
workflows.0018_drop_emailreputationsnapshot_table
└─ #1 ⚠️ RunSQL
DROP TABLE IF EXISTS - properly staged (prior state removal found)
table: posthog_emailreputationsnapshot
✅ Safe
Brief or no lock, backwards compatible
workflows.0015_delete_emailreputationsnapshot
└─ #1 ✅ SeparateDatabaseAndState: Only state operations (no database changes)
workflows.0017_teamworkflowsconfig_ses_tenant_state
└─ #1 ✅ AddField
Adding NOT NULL field with constant default (safe in PG11+)
model: teamworkflowsconfig, field: ses_tenant_reputation_impact
└─ #2 ✅ AddField
Adding NOT NULL field with constant default (safe in PG11+)
model: teamworkflowsconfig, field: ses_tenant_sending_status
└─ #3 ✅ AddField
Adding nullable field requires brief lock
model: teamworkflowsconfig, field: ses_tenant_state_synced_at
📚 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.
RunSQL:
DROP CONSTRAINT is instant (no table lock), dropping constraints can break running code during rolling deployments.
Safe pattern:
- Ensure no running code relies on the constraint (uniqueness checks, foreign key validation, etc.)
- If replacing with a new constraint, deploy the new one first
- Wait at least one full deployment cycle before dropping the old constraint
- Consider keeping unused constraints if removal risk outweighs benefits
Common scenarios:
- Dropping UNIQUE constraints: Ensure code handles potential duplicates
- Dropping FOREIGN KEY constraints: Ensure code doesn't assume referential integrity
- Replacing constraints: Add new → deploy → wait → drop old
See the migration safety guide
Last updated: 2026-07-30 10:12 UTC (74b0611)
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
| operations = [ | ||
| migrations.RunSQL( | ||
| sql='DROP TABLE IF EXISTS "posthog_emailreputationsnapshot";', | ||
| reverse_sql=migrations.RunSQL.noop, | ||
| ), |
There was a problem hiding this comment.
A separate Node.js/Temporal worker service actively reads and writes this table — not just the Python API endpoint
Why we think it's a valid issue
- Checked: The actual base branch this migration deploys onto and merges into (
posthog-code/ses-tenant-event-comms, the stack tip), not local master. Verified the Nodeemail-reputation.service.ts, its Temporal worker, and the capability/mode/server wiring directly via the GitHub API on that ref. - Found: Unlike the Python side (model deleted, endpoint rewritten), the Node service still runs live SQL against the table on the base branch:
SELECT DISTINCT team_id FROM posthog_emailreputationsnapshot(email-reputation.service.ts:121), a secondSELECT DISTINCT ON (team_id) ... FROM posthog_emailreputationsnapshot(:418), andINSERT INTO posthog_emailreputationsnapshot ... ON CONFLICT DO NOTHING(:442). Nothing in the stack repointed or removed these. - Found: It is wired and scheduled, not dead code —
EmailReputationWorkerServiceis started inserver.ts:400-425undercapabilities.emailReputationEvaluator, the temporal worker registers a daily schedule (scheduleId: EMAIL_REPUTATION_SCHEDULE_ID,spec: { calendars: [{ hour }] }, email-reputation-worker.service.ts:118-135), and there is a dedicated deployment modePluginServerMode.email_reputation_evaluator = 'email-reputation-evaluator'(config.ts:54). The server.ts comment "Dedicated deployments should crash and restart" confirms a dedicated production deployment path (dev merely rides along viaisDevEnv()and degrades gracefully; dedicated deploymentsthrow). - Impact: Confirmed reliability/data defect. The PR's premise "Nothing reads or writes it anymore" is false for the Node runtime — a scheduled, deployed Temporal worker both reads and writes the table on the exact branch this drop lands on. Running
DROP TABLE IF EXISTS "posthog_emailreputationsnapshot"makes the next daily activity fail withrelation "posthog_emailreputationsnapshot" does not exist, producing repeated Temporal retries/failures — and at minimum breaks the local dev CDP pipeline where the capability defaults on. This is the genuine blind spot (invisible to Python-only investigation), the inverse of the first finding which was a false positive because the Python endpoint had actually been rewritten. The drop must wait until this Node evaluator is decommissioned or repointed.
Issue description
Beyond the Python HogFlowViewSet.team_reputation endpoint, there is an entirely separate, cross-runtime consumer of posthog_emailreputationsnapshot: the Node.js Temporal worker EmailReputationService (nodejs/src/cdp/services/email-reputation/email-reputation.service.ts), registered as the email-reputation-evaluator capability (nodejs/src/common/config.ts:54, nodejs/src/capabilities.ts:203, nodejs/src/server.ts:417, and nodejs/src/cdp/services/email-reputation/temporal/email-reputation-worker.service.ts:216) — a fully wired, deployed service, not dead code. Its own docstring says it 'runs as Temporal activities' with 'each daily run' appending snapshot rows, and its code does both SELECT DISTINCT team_id FROM posthog_emailreputationsnapshot ... (line ~121) and INSERT INTO posthog_emailreputationsnapshot ... ON CONFLICT DO NOTHING (line ~442). This means the PR's central claim — 'Nothing reads or writes it anymore' — is false in a second, independent way that is invisible to any Python-only investigation: a scheduled background job in a completely different language/runtime/deployment unit depends on this table. Running DROP TABLE IF EXISTS "posthog_emailreputationsnapshot"; while this evaluator is still deployed and scheduled would turn its next Temporal activity run into a hard Postgres error (relation "posthog_emailreputationsnapshot" does not exist), likely producing repeated Temporal activity retries/failures and metric/alerting noise on a completely separate service surface than the one already identified for the Django API endpoint.
Suggested fix
Before merging this drop, confirm the Node.js email-reputation-evaluator Temporal worker has also been decommissioned or repointed away from this table (not just the Python model/API usage). If this worker is still running in any environment, this migration must wait until it is disabled/removed, exactly as the safe-migrations guide's safety-window requirement intends — but the window needs to cover this Node/Temporal consumer specifically, which the PR description and its stacked prerequisites (#74762/#75237/#75249) never mention.
Prompt to fix with AI (copy-paste)
## Context
@products/workflows/backend/migrations/0018_drop_emailreputationsnapshot_table.py#L15-19
<issue_description>
Beyond the Python `HogFlowViewSet.team_reputation` endpoint, there is an entirely separate, cross-runtime consumer of `posthog_emailreputationsnapshot`: the Node.js Temporal worker `EmailReputationService` (nodejs/src/cdp/services/email-reputation/email-reputation.service.ts), registered as the `email-reputation-evaluator` capability (nodejs/src/common/config.ts:54, nodejs/src/capabilities.ts:203, nodejs/src/server.ts:417, and nodejs/src/cdp/services/email-reputation/temporal/email-reputation-worker.service.ts:216) — a fully wired, deployed service, not dead code. Its own docstring says it 'runs as Temporal activities' with 'each daily run' appending snapshot rows, and its code does both `SELECT DISTINCT team_id FROM posthog_emailreputationsnapshot ...` (line ~121) and `INSERT INTO posthog_emailreputationsnapshot ... ON CONFLICT DO NOTHING` (line ~442). This means the PR's central claim — 'Nothing reads or writes it anymore' — is false in a second, independent way that is invisible to any Python-only investigation: a scheduled background job in a completely different language/runtime/deployment unit depends on this table. Running `DROP TABLE IF EXISTS "posthog_emailreputationsnapshot";` while this evaluator is still deployed and scheduled would turn its next Temporal activity run into a hard Postgres error (`relation "posthog_emailreputationsnapshot" does not exist`), likely producing repeated Temporal activity retries/failures and metric/alerting noise on a completely separate service surface than the one already identified for the Django API endpoint.
</issue_description>
<issue_validation>
- **Checked:** The actual base branch this migration deploys onto and merges into (`posthog-code/ses-tenant-event-comms`, the stack tip), not local master. Verified the Node `email-reputation.service.ts`, its Temporal worker, and the capability/mode/server wiring directly via the GitHub API on that ref.
- **Found:** Unlike the Python side (model deleted, endpoint rewritten), the Node service still runs live SQL against the table on the base branch: `SELECT DISTINCT team_id FROM posthog_emailreputationsnapshot` (email-reputation.service.ts:121), a second `SELECT DISTINCT ON (team_id) ... FROM posthog_emailreputationsnapshot` (:418), and `INSERT INTO posthog_emailreputationsnapshot ... ON CONFLICT DO NOTHING` (:442). Nothing in the stack repointed or removed these.
- **Found:** It is wired and scheduled, not dead code — `EmailReputationWorkerService` is started in `server.ts:400-425` under `capabilities.emailReputationEvaluator`, the temporal worker registers a daily schedule (`scheduleId: EMAIL_REPUTATION_SCHEDULE_ID`, `spec: { calendars: [{ hour }] }`, email-reputation-worker.service.ts:118-135), and there is a dedicated deployment mode `PluginServerMode.email_reputation_evaluator = 'email-reputation-evaluator'` (config.ts:54). The server.ts comment "Dedicated deployments should crash and restart" confirms a dedicated production deployment path (dev merely rides along via `isDevEnv()` and degrades gracefully; dedicated deployments `throw`).
- **Impact:** Confirmed reliability/data defect. The PR's premise "Nothing reads or writes it anymore" is false for the Node runtime — a scheduled, deployed Temporal worker both reads and writes the table on the exact branch this drop lands on. Running `DROP TABLE IF EXISTS "posthog_emailreputationsnapshot"` makes the next daily activity fail with `relation "posthog_emailreputationsnapshot" does not exist`, producing repeated Temporal retries/failures — and at minimum breaks the local dev CDP pipeline where the capability defaults on. This is the genuine blind spot (invisible to Python-only investigation), the inverse of the first finding which was a false positive because the Python endpoint had actually been rewritten. The drop must wait until this Node evaluator is decommissioned or repointed.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Before merging this drop, confirm the Node.js `email-reputation-evaluator` Temporal worker has also been decommissioned or repointed away from this table (not just the Python model/API usage). If this worker is still running in any environment, this migration must wait until it is disabled/removed, exactly as the safe-migrations guide's safety-window requirement intends — but the window needs to cover this Node/Temporal consumer specifically, which the PR description and its stacked prerequisites (#74762/#75237/#75249) never mention.
</potential_solution>
|
Reviews (1): Last reviewed commit: "chore(workflows): drop the dead email re..." | Re-trigger Greptile |
0fa3620 to
f20392b
Compare
c128f3d to
6871147
Compare
Problem
Phase 2 of the two-phase table drop for
EmailReputationSnapshot. #74762 removed the model from Django state (migration 0015,SeparateDatabaseAndState) and dropped its orphaned FK toposthog_hogflow(0016), but deliberately left the physicalposthog_emailreputationsnapshottable in place per the safe-migrations guide. Nothing reads or writes it anymore — reputation moved to AWS SES tenants and the tab computes rates on the fly — so the dead table is just clutter in schema introspection.Changes
One migration (0018) that runs
DROP TABLE IF EXISTS "posthog_emailreputationsnapshot". The reverse is a no-op (a table drop can't be meaningfully undone, and nothing depends on the table), so a rollback past this point simply leaves it absent. The table already has zero FK constraints after #74762's 0016, so no dependency cleanup is needed first.Important
Deploy ordering — do not merge until #74762 has been deployed for at least one full cycle. The safe window ensures no rollback or hotfix can reintroduce code that expects the table. This branch is stacked on the #74762 → #75237 → #75249 chain (base is #75249) purely so the migration graph stays linear (0015→0016→0017→0018); its only real prerequisite is that #74762's state removal is live. Merge this last, and rebase onto master once the stack lands.
How did you test this code?
I'm an agent.
manage.py sqlmigrate workflows 0018emits exactlyDROP TABLE IF EXISTS "posthog_emailreputationsnapshot";;makemigrations --checkreports no drift and a linear graph. I did not run it against a production-shaped dataset — the drop is idempotent (IF EXISTS) and irreversible by design.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with PostHog Code (Claude). Skill followed: django-migrations (phase 2 of the documented table-drop: RunSQL DROP TABLE IF EXISTS, referencing the model-removal PR and its safety window). Stacked on the tip of the reputation stack so the migration chain is linear and this destructive drop deploys last.
Created with PostHog Code