Repository navigation
feat(flags): read last_called_at sync calls from flag_evaluations behind a setting - #110150
Conversation
…ind a setting Claude-Session: https://claude.ai/code/session_01PV6xJfFwzQEXqiTE2d1t4i
|
😎 Merged successfully - details. |
🤖 CI report
|
| File | Comment lines | Added lines |
|---|---|---|
posthog/settings/feature_flags.py |
9 | 14 |
posthog/tasks/tasks.py |
4 | 33 |
products/feature_flags/backend/test/test_feature_flag_sync.py |
4 | 100 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +51.2 KiB (+0.1%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 70.09 MiB · 🔺 +51.2 KiB (+0.1%)
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/posthog_ai/frontend/scenes/TaskTracker/components/ArtifactObjectEmbed.js |
19.5 KiB | 🔺 +19.5 KiB (new) |
exporter/_parent/products/posthog_ai/frontend/scenes/TaskTracker/components/ArtifactObjectEmbed.js |
6.8 KiB | 🔺 +6.8 KiB (new) |
posthog-app/_parent/products/workflows/frontend/Broadcasts/BroadcastScene.js |
81.6 KiB | 🔺 +6.5 KiB (+8.7%) |
exporter/_parent/products/posthog_ai/frontend/scenes/TaskTracker/TaskTracker.js |
43.4 KiB | 🔺 +6.3 KiB (+17.1%) |
posthog-app/_parent/products/posthog_ai/frontend/scenes/TaskTracker/TaskTracker.js |
51.2 KiB | 🔺 +6.3 KiB (+14.0%) |
posthog-app/_parent/products/tasks/frontend/spaces/SpaceScene.js |
66.6 KiB | 🔺 +4.2 KiB (+6.8%) |
exporter/src/exporter/scenes/ExporterNotebookScene.js |
3.70 MiB | 🔺 +1.8 KiB (+0.0%) |
render-query/src/render-query/render-query.js |
20.22 MiB | 🔺 +1.8 KiB (+0.0%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.61 MiB · 22 files | 🔺 +225 B (+0.0%) | █████████░ 87.5% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.56 MiB · 630 files | 🔺 +237 B (+0.0%) | █████████░ 88.5% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.60 MiB · 2,452 files | 🔺 +629 B (+0.0%) | █████████░ 91.1% of 8.34 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 217.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.5 KiB | src/lib/api.ts |
| 91.6 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.1 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.5 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 279.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 217.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.5 KiB | ../packages/quill/packages/quill/dist/index.js |
| 100.5 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 91.6 KiB | src/products.tsx |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.19 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.19 MiB · 19 files | 🔺 +237 B (+0.0%) | ████░░░░░░ 38.2% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | 🔺 +28 B (+0.0%) | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 824.6 KiB | dist/toolbar/toolbar-app-6MIBOQDK.css |
| 656.8 KiB | dist/toolbar/chunk-chunk-K7TY7VDS.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-VEMKODO5.js |
| 138.2 KiB | dist/toolbar/chunk-chunk-2726HMK6.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-UDGNCPET.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-OLBPSHBJ.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-GKFTYVXL.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +460.1 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 962.09 MiB · 🔺 +460.1 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17633.1 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 199.3 KB |
| action | 454.1 KB | 199.3 KB |
| action-list | 564.2 KB | 199.3 KB |
| cohort | 453.1 KB | 199.3 KB |
| cohort-list | 563.2 KB | 199.3 KB |
| email-template | 452.9 KB | 199.3 KB |
| error-details | 469.6 KB | 199.3 KB |
| error-issue | 454.5 KB | 199.3 KB |
| error-issue-list | 564.8 KB | 199.3 KB |
| experiment | 561.3 KB | 199.3 KB |
| experiment-list | 564.9 KB | 199.3 KB |
| experiment-results | 566.3 KB | 199.3 KB |
| feature-flag | 566.8 KB | 199.3 KB |
| feature-flag-list | 570.5 KB | 199.3 KB |
| feature-flag-testing | 457.3 KB | 199.3 KB |
| inline-scan | 453.6 KB | 199.3 KB |
| insight-actors | 562.3 KB | 199.3 KB |
| invite-email-preview | 452.3 KB | 199.3 KB |
| llm-costs | 559.3 KB | 199.3 KB |
| session-recording | 455.3 KB | 199.3 KB |
| survey | 454.7 KB | 199.3 KB |
| survey-global-stats | 561.9 KB | 199.3 KB |
| survey-list | 564.9 KB | 199.3 KB |
| survey-stats | 561.9 KB | 199.3 KB |
| trace-span | 453.5 KB | 199.3 KB |
| trace-span-list | 564.1 KB | 199.3 KB |
| vision-observation-list | 563.3 KB | 199.3 KB |
| workflow | 453.4 KB | 199.3 KB |
| workflow-list | 563.5 KB | 199.3 KB |
| loops-review | 457.8 KB | 199.3 KB |
| query-results | 774.1 KB | 199.3 KB |
| render-ui | 858.1 KB | 199.3 KB |
| visual-review-snapshots | 457.9 KB | 199.3 KB |
⚠️ Playwright — 1 flaky
🎭 Playwright report · View test results →
- Edit mode button enters and exits edit mode (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ Backend coverage — 85.0% of changed backend lines covered — 10 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): █████████████████░░░ 85.0% (60 / 70)
| File | Patch | Uncovered changed lines |
|---|---|---|
posthog/tasks/tasks.py |
0.0% | 1299, 1317–1320, 1380, 1385, 1421, 1437, 1452 |
🤖 Agents: add a test only if an uncovered line exposes a realistic regression that existing tests miss. Otherwise explain why no new test is needed under "How did you test this code?". Gap list: the patch-coverage artifact on this run (gh run download 36940776791 -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 |
demo |
███████████░░░░░░░░░ 53.4% |
1,445 / 2,707 |
data_tools |
████████████░░░░░░░░ 61.2% |
90 / 147 |
warehouse_sources_queue |
█████████████░░░░░░░ 65.7% |
1,608 / 2,446 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
aeo |
███████████████░░░░░ 76.3% |
617 / 809 |
batch_exports |
████████████████░░░░ 81.2% |
21,572 / 26,551 |
apm |
█████████████████░░░ 84.1% |
1,306 / 1,553 |
cdp |
██████████████████░░ 88.3% |
4,559 / 5,164 |
ml_inference |
██████████████████░░ 88.8% |
539 / 607 |
mcp_analytics |
██████████████████░░ 89.2% |
5,038 / 5,651 |
product_tours |
██████████████████░░ 89.3% |
1,340 / 1,500 |
dashboards |
██████████████████░░ 89.6% |
6,924 / 7,727 |
notebooks |
██████████████████░░ 90.2% |
15,304 / 16,971 |
data_warehouse |
██████████████████░░ 90.3% |
14,298 / 15,839 |
signals |
██████████████████░░ 90.4% |
59,629 / 65,990 |
cohorts |
██████████████████░░ 90.5% |
8,534 / 9,434 |
streamlit_apps |
██████████████████░░ 90.8% |
2,684 / 2,956 |
managed_warehouse |
██████████████████░░ 91.0% |
10,252 / 11,263 |
data_modeling |
██████████████████░░ 91.2% |
10,562 / 11,584 |
tasks |
██████████████████░░ 91.3% |
78,944 / 86,502 |
today |
██████████████████░░ 91.4% |
894 / 978 |
exports |
██████████████████░░ 91.7% |
9,684 / 10,566 |
business_knowledge |
██████████████████░░ 92.0% |
8,446 / 9,180 |
engineering_analytics |
██████████████████░░ 92.2% |
11,497 / 12,475 |
ai_training |
██████████████████░░ 92.2% |
356 / 386 |
conversations |
███████████████████░ 92.6% |
29,137 / 31,466 |
early_access_features |
███████████████████░ 92.6% |
1,339 / 1,446 |
managed_migrations |
███████████████████░ 92.7% |
1,581 / 1,705 |
stamphog |
███████████████████░ 92.8% |
8,109 / 8,742 |
visual_review |
███████████████████░ 92.9% |
9,534 / 10,265 |
canvas |
███████████████████░ 92.9% |
7,155 / 7,703 |
approvals |
███████████████████░ 93.0% |
3,974 / 4,271 |
mcp_registry |
███████████████████░ 93.1% |
1,670 / 1,794 |
notifications |
███████████████████░ 93.2% |
1,144 / 1,228 |
error_tracking |
███████████████████░ 93.2% |
16,370 / 17,557 |
surveys |
███████████████████░ 93.4% |
6,644 / 7,113 |
slack_app |
███████████████████░ 93.6% |
14,560 / 15,557 |
autoresearch |
███████████████████░ 93.6% |
8,837 / 9,442 |
context_layer |
███████████████████░ 93.8% |
3,415 / 3,639 |
web_analytics |
███████████████████░ 93.9% |
23,680 / 25,229 |
billing_alerts |
███████████████████░ 94.1% |
2,094 / 2,226 |
mcp_store |
███████████████████░ 94.3% |
8,959 / 9,501 |
ai_observability |
███████████████████░ 94.6% |
24,974 / 26,409 |
alerts |
███████████████████░ 94.6% |
9,267 / 9,796 |
wizard |
███████████████████░ 94.7% |
6,150 / 6,496 |
workflows |
███████████████████░ 94.7% |
15,159 / 16,006 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
review_hog |
███████████████████░ 95.0% |
11,750 / 12,362 |
annotations |
███████████████████░ 95.1% |
817 / 859 |
endpoints |
███████████████████░ 95.1% |
9,222 / 9,694 |
customer_analytics |
███████████████████░ 95.2% |
25,991 / 27,301 |
legal_documents |
███████████████████░ 95.2% |
2,311 / 2,427 |
marketing_analytics |
███████████████████░ 95.3% |
19,435 / 20,398 |
posthog_ai |
███████████████████░ 95.3% |
2,491 / 2,614 |
actions |
███████████████████░ 95.5% |
756 / 792 |
logs |
███████████████████░ 95.5% |
15,468 / 16,200 |
experiments |
███████████████████░ 95.5% |
32,931 / 34,481 |
data_catalog |
███████████████████░ 95.6% |
4,402 / 4,606 |
tracing |
███████████████████░ 95.6% |
3,536 / 3,699 |
replay_vision |
███████████████████░ 95.6% |
29,355 / 30,692 |
growth |
███████████████████░ 95.7% |
11,381 / 11,888 |
skills |
███████████████████░ 95.8% |
6,972 / 7,274 |
messaging |
███████████████████░ 95.9% |
3,824 / 3,989 |
product_analytics |
███████████████████░ 96.0% |
28,521 / 29,696 |
revenue_analytics |
███████████████████░ 96.4% |
1,889 / 1,959 |
user_interviews |
███████████████████░ 96.5% |
2,870 / 2,974 |
feature_flags |
███████████████████░ 96.6% |
26,872 / 27,815 |
access_control |
███████████████████░ 96.7% |
7,738 / 8,005 |
warehouse_sources |
███████████████████░ 97.3% |
468,809 / 481,716 |
data_quality |
████████████████████ 97.5% |
7,701 / 7,895 |
links |
████████████████████ 97.9% |
234 / 239 |
security |
████████████████████ 98.0% |
1,283 / 1,309 |
metrics |
████████████████████ 98.0% |
4,252 / 4,338 |
analytics_platform |
████████████████████ 98.3% |
2,784 / 2,833 |
pulse |
████████████████████ 98.5% |
2,046 / 2,078 |
live_debugger |
████████████████████ 99.2% |
626 / 631 |
field_notes |
████████████████████ 99.4% |
172 / 173 |
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/feature_flags/backend/migrations/0023_alter_featureflag_last_called_at_help_text.py
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
return result
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
ed = p(logger, meth_name, ed) # type: ignore[arg-type]
2026-10-01T17:01:45.533706Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=8122 tid=140698970577792
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
return (ssh_host,
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Alter field last_called_at on featureflag
--
-- (no-op)
COMMIT;Last updated: 2026-10-01 17:02 UTC (f8ce8a2)
✅ Django migration risk — no migrations to analyze
No Django migrations need risk analysis.
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 1 should fix, 0 consider. Published 1 finding (view the review). Resolved comments: 4 declined, 2 left for you |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The shared checkpoint skips existing flag-evaluation history during rollout, leaving affected flags incorrectly stale.
Review effort: Balanced
Findings: 1
What changed in this PR
Adds a configurable ClickHouse source for feature flag last_called_at synchronization.
Changes:
- Adds
events/flag_evaluationssource selection with fallback logging. - Queries typed
flag_keyvalues fromflag_evaluations. - Adds real ClickHouse coverage and updates generated API documentation.
| File | Description |
|---|---|
services/mcp/src/api/generated.ts |
Updates generated field documentation. |
products/feature_flags/frontend/generated/api.schemas.ts |
Updates generated frontend schema documentation. |
products/feature_flags/backend/test/test_feature_flag_sync.py |
Tests source selection and key filtering. |
products/feature_flags/backend/models/feature_flag.py |
Updates last_called_at help text. |
products/feature_flags/backend/migrations/max_migration.txt |
Advances the migration marker. |
products/feature_flags/backend/migrations/0023_alter_featureflag_last_called_at_help_text.py |
Records the help-text change. |
posthog/tasks/tasks.py |
Selects and queries the configured source. |
posthog/settings/feature_flags.py |
Defines the source setting and operational caveats. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
[High risk] Adds a configurable data source to the feature flag sync task. The PR is not safe to merge until the missed-call paths and required production setting wiring are addressed. Reviews (1) · Last reviewed commit: "feat(flags): read last_called_at sync ca..." |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe sync task now reads flag calls from either Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to Enabling the new Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The default behavior remains unchanged, and the inspected update path preserves team and flag identity. The main risk is operational: selecting a source that lacks calls, or falls too far behind, can leave usage timestamps inaccurate across teams. Switching back does not automatically recover missed windows. The inspected downstream effects are stale-flag status and cleanup guidance, not access-control decisions. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
posthog/tasks/tasks.py-1301-1301 (1)
1301-1301: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove the new import to module scope.
Move
FLAG_EVALUATIONS_TABLEbeside the module-level imports.As per coding guidelines: “Always place imports at the top of the file (module level), never inside functions or methods (local imports).”
Source: Coding guidelines
services/mcp/src/api/generated.ts-48015-48015 (1)
48015-48015: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe description omits the
flag_evaluationscoverage limit, so both fields can imply thatlast_called_atreflects calls that the selected source does not contain.
services/mcp/src/api/generated.ts#L48015-L48015: Clarify in the source help text thatflag_evaluationsreflects only calls forked into that table on real-time ingestion lanes, then regenerate this API description.services/mcp/src/api/generated.ts#L48418-L48418: Regenerate this API description from the clarified source help text.
🧹 Nitpick comments (1)
posthog/tasks/tasks.py (1)
1440-1440: 🩺 Stability & Availability | 🔵 TrivialEnsure
flag_evaluationsis readable on every offline query host before enabling this source.When
FEATURE_FLAG_LAST_CALLED_AT_SYNC_SOURCEis"flag_evaluations", the task queries the Distributedflag_evaluationstable. If the selected host or its remote tables are unavailable, the first chunk can fail and leave the Redis checkpoint unchanged. Ensure the table and its remote-table configuration exist on each host selected by the offline workload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 93c09ff6-67f0-4ed3-bce9-0d4e8b33688a
⛔ Files ignored due to path filters (1)
products/feature_flags/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (7)
posthog/settings/feature_flags.pyposthog/tasks/tasks.pyproducts/feature_flags/backend/migrations/0023_alter_featureflag_last_called_at_help_text.pyproducts/feature_flags/backend/migrations/max_migration.txtproducts/feature_flags/backend/models/feature_flag.pyproducts/feature_flags/backend/test/test_feature_flag_sync.pyservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 1a05e57b-7b13-4d7c-a3d5-73fd95f4c2ba
📒 Files selected for processing (1)
posthog/settings/feature_flags.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: c1178565-4a85-441c-9b58-47486de83bf8
📒 Files selected for processing (2)
posthog/settings/feature_flags.pyposthog/tasks/tasks.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.
| # Both sources share this checkpoint, so a source switch resumes where the old source stopped. | ||
| # last_called_at only moves forward, so a call that both sources read at the switch changes nothing. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep independent progress for each source.
While flag_evaluations is selected, the shared checkpoint can advance past calls from historical and async lanes, which are absent from that table. Switching to events then starts after the checkpoint, so those calls are never replayed and last_called_at can remain stale until another call arrives. Switching back before disabling the fork does not recover calls skipped during earlier runs. Maintain source-specific checkpoints or backfill the skipped event interval before resuming.
The PR objective states that historical and async lane calls are not written to flag_evaluations.
|
Risk: No findings This PR adds a Sentinel reviewed |
There was a problem hiding this comment.
Approved.
The new source setting defaults to the existing behavior, so nothing changes until it is flipped. It only affects a background sync read path and adds real-ClickHouse tests. The rollout caveats about the shared checkpoint are documented in the PR and settings comments, and the change is easy to revert.
- Author wrote 33% of the modified lines and has 52 merged PRs in these paths (familiarity MODERATE).
- coderabbitai[bot] reviewed the current head.
- CodeRabbit's unresolved inline thread says the checkpoint is shared across sources. The PR documents this tradeoff and defaults to the old behavior, so it is a rollout caveat and not a blocker.
- Minor: the new function-local import in tasks.py follows the surrounding local-import pattern, but CodeRabbit suggested moving it to module scope.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 55L, 2F substantive, 176L/3F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (176L, 3F, two-areas, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ 1bc93f1 · reviewed head 1bc93f1 |

Problem
flag_evaluationsnever get alast_called_at.$feature_flag_calledonly inflag_evaluations, whilesync_feature_flag_last_calledreads onlydistributed_events_recent.Changes
FEATURE_FLAG_LAST_CALLED_AT_SYNC_SOURCE, picks the table the sync reads:events(default) orflag_evaluations.flag_evaluations, the sync reads every team's calls from that one table, grouped on its typedflag_keycolumn.flag_evaluations, the scan window ends 15 minutes before now instead of 60 seconds (FEATURE_FLAG_LAST_CALLED_AT_SYNC_FLAG_EVALUATIONS_BUFFER_SECONDS).inserted_atwith the Kafka message time, so the buffer must also cover ClickHouse consumer lag. Otherwise a late row lands behind the checkpoint.inserted_atcheckpoint, time chunks, the row limit, the batched update and the metrics are unchanged.last_called_atonly moves forward, so a small gap or overlap at the switch has no lasting effect.events. The task does not fail the deploy over this knob.$feature_flagof JSON null stores the string'null'inflag_key. The new query keeps it, so a flag keyednullstill gets calls.flag_evaluations.Warning
Ingestion forks flag calls into
flag_evaluationsonly on the real-time lanes. With the new source, calls on the historical and async lanes no longer movelast_called_at.Rollout: Cloud sets
FEATURE_FLAG_LAST_CALLED_AT_SYNC_SOURCE=flag_evaluationsafter deploy, US first, then EU. Self-hosted keepsevents, since it does not fillflag_evaluations.Warning
After the flip, disabling the
flag_evaluationsingestion fork also stopslast_called_at, and the checkpoint moves past the missed calls. The charts change that sets the source also adds a step to the fork's produce-failure runbook: switch the sync source back toeventsbefore disabling the fork.Note
A ClickHouse consumer stall longer than 15 minutes still drops calls behind the checkpoint. Only flags with no calls after the stall keep a stale
last_called_at. A column that ClickHouse stamps at write time would close this gap, and it is a follow-up.The
flag_evaluationsquery was run on the production ClickHouse clusters that Celery queries, in US and EU, and returned results in both.One 5-minute chunk, the most recent window a normal run reads, took about 2 seconds end to end in each region. The query timeout that fails a run is 120 seconds.
Unchecked: a catch-up run after a long gap reads windows up to 6 hours old. Those parts have merged more, because
inserted_atis not in the table's sort key, so a chunk there may read more.How did you test this code?
Ran
hogli test products/feature_flags/backend/test/test_feature_flag_sync.pylocally against real ClickHouse.Test rationale: Every existing sync test mocks the ClickHouse query, so none catches a wrong table, column or filter.
TestSyncFeatureFlagLastCalledSourceruns the real query.test_reads_flag_calls_from_the_configured_sourceputs one call only inevents_recentand one only inflag_evaluations. It fails if a source reads the wrong table, or if the unknown-value fallback breaks.timestampinstead ofinserted_atfails it.test_call_without_a_flag_key_does_not_take_the_row_limitruns per source with a row limit of 1. It fails if the empty-key filter is dropped.test_call_inside_the_buffer_waits_for_a_later_runstamps a call 5 minutes ago. Theflag_evaluationscase fails when its buffer is set back to 60 seconds.flag_evaluationscases.Not checked: the warning log line for an unknown value, and metrics on the new path.
Release status
Automatic notifications
Docs update
None. No doc under
docs/covers the sync source.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5)/django-migrations,/writing-tests,/writing-user-facing-copy,/writing-pr-descriptions,/simplify,/reviewing-with-coderabbit.--deepagainstmaster. It reported no findings.FEATURE_FLAG_LAST_CALLED_AT_SYNC_*settings, rather than an instance setting.last_called_athelp text, with a migration and regenerated types. That change was dropped, because the old text is still accurate.last_called_atbut does not touch the sync.https://claude.ai/code/session_01PV6xJfFwzQEXqiTE2d1t4i