[JamesPatrickGill]

Problem

• A flag author opening release conditions saw an audience total several times larger than the real one.
• The "users" total came from an all-time person count (Team.persons_seen_so_far), with no activity bound.
• Projects that mint a fresh anonymous distinct id per page load (posthog-js persistence: "memory") build a long tail of one-shot persons that never return. The count read far above the reachable audience.

Supersedes #81547, an autonomous PR that first found this. That branch predated the master rewrite of the same function (#87287) and could not rebase past the signed-commit rule. This is the same fix re-authored on current master.

Changes

• A flag author now sees the audience sized by identified persons active in the last 60 days. The stale one-shot tail drops out of both the matched count and the total.
• The estimate tooltip now names the basis the number actually came from. It reads "active in the last 60 days" only where the window ran, and drops that clause everywhere else. The response carries activity_window_days, which is null for an all-time count.
• The window sits behind the flags-size-by-active-persons kill switch, on for project 2 and off everywhere else. Off restores the all-time count, so a disable costs accuracy and never correctness.
• The window is an opt-in on get_user_blast_radius, and only the flags endpoint passes it. The workflows audience preview keeps the all-time count, because the batch send it previews has no activity window.
• The sizing endpoint now carries FlagSizingBurstRateThrottle and FlagSizingSustainedRateThrottle, a per-project bucket on _TeamBucketRateThrottle. The ClickHouse*RateThrottle pair it replaces derives from PersonalApiKeyRateThrottle, which lets an authenticated request carrying no personal API key through before it counts the request. The release-conditions editor authenticates by session cookie, so it skipped that pair entirely.
• Both queries exclude personless events. An event captured with $process_person_profile: false carries a synthetic person_id and no persons row. Counting it would put traffic in the denominator that the matched subquery can never return.
• The filtered path runs one events scan for both numbers, via uniq(person_id) and uniqIf(person_id, person_id IN <matched persons>). This is the design choice worth a look. One scan keeps the numerator and denominator on the same rows.
• The unfiltered count is cached per project for 5 minutes.
• Both scans run on the offline ClickHouse pool, so neither competes with interactive analytics. Both throw on timeout, so neither returns a partial count as a real one.
• The window ends one day ahead of now, so an event with a corrupt far-future timestamp cannot hold a person "active" for years.
• Mechanical: one source each for the two duplicated predicates, the generated API call in place of a handwritten type, and regenerated ClickHouse and OpenAPI snapshots. The activity-window query text is a module constant, so no f-string reaches parse_select and the hogql-fstring-audit rule has nothing to audit.
• Only the tooltip text changes on screen, so there is no screenshot.

Warning

Two known costs, both bounded rather than removed.

• person_id IN (SELECT persons.id ...) makes ClickHouse materialize the matched-person set, which the previous count(DISTINCT persons.id) never did. A broad person filter on a large project can exceed the per-query memory cap. The cap is per query and the pool is the offline one, so the panel shows an error and nothing else is affected.
• The release editor fires one request per condition group with no dedup. A 60-day scan holds a worker far longer than the old persons count did. The cache covers the empty-properties groups, the new throttles bound the rest per project, and the kill switch reverts the cost. Batching every condition into one request would remove the repeat work, and changes the API on both ends.

Warning

Two product decisions are open.

• Is 60 days the right window? It removes the stale over-count. But it undercounts real users a flag targets who were not active in 60 days, for example monthly-login customers.
• The audience number moves for every flag author with no action from them, so it may need an in-app change notice (see /announcing-behavior-changes). This PR ships none.

Known gap, not addressed here: "View matching users" still lists every matching person, including the inactive ones the count now excludes. Windowing that list means adding an activity filter to both the persons URL and the persons query.

How did you test this code?

• TestBlastRadius passes locally, 54 cases with 22 ClickHouse snapshots unchanged. The snapshots not moving is the check on the extracted predicates: same SQL, one source.
• test_user_blast_radius_excludes_inactive_matching_persons fixes the window edges at 59 and 61 days. A typo in RECENTLY_ACTIVE_DAYS moves the boundary past a fixture and fails the test.
• The same test asserts the unwindowed path still counts every matching person, so a later change cannot move the workflows preview onto the window unnoticed.
• test_user_blast_radius_excludes_personless_events asserts an event with no backing persons row counts toward neither number, on both paths. Replacing the predicate with 1 = 1 fails it.
• test_user_blast_radius_caches_the_unfiltered_active_count posts the same empty condition twice from one project and once from a second, then asserts two events scans. An unscoped cache key serves one project's count as the other's.
• TestRecentlyActiveSizingFlag covers the kill switch on, off, and raising. A flag client that raises has to read as "window off" rather than a 500 on the sizing panel.
• Three cases in FeatureFlagReleaseConditions.matchingUsersLink.test.tsx assert the estimate tooltip appears only next to a person count, including where a condition overrides an unset flag level.
• Not run: the full backend matrix. CI reports the authoritative counts.

Automatic notifications

• Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

• Started from fix(flags): size flags by recently active persons, not all-time count #81547, reconciled onto current master. One commit replaces the four bot commits, because a rebase across the master gap tripped the signed-commit rule.
• Design decision: one events scan with uniqIf replaces the original two queries. One query removes the shared-Database concern from fix(flags): count blast radius via HogQL, drop legacy PersonQuery #87287 by construction.
• The matched-persons subquery stays on property_to_expr rather than events.person.properties, to avoid disturbing the tested cohort and group paths.
• From review comments on this PR: the workflows opt-in, the personless filter, the per-project cache, the offline workload, the window's upper bound, throw-on-timeout, the window-edge test, the throttle that covers session callers, activity_window_days, and the two deduplicated predicates.
• The personless filter came from a review comment this session first argued against, then retracted. Without it the total counted anonymous traffic that the old persons count never saw, which raised the number for projects on posthog-js's identified_only default.
• Skills invoked: /address-pr-reviews, /writing-tests, /writing-user-facing-copy, /writing-code-comments, /adopting-generated-api-types, /writing-pr-descriptions, /ste-writing.
• No customer data appears in the code, fixtures, tests, or this description. The fixtures are invented, and the anonymous-churn framing is public product behavior.

---

Created with PostHog Desktop

[trunk-io]

Merging to master in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[github-actions]

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Complexity (TypeScript) — 7 functions above the limit (max 37)

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

Function	Location	Complexity	Limit
renderReleaseConditionGroup	frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx:198	37	10
ConditionContent	frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx:423	32	10
FeatureFlagReleaseConditionsCollapsible	frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx:878	25	10
FeatureFlagReleaseConditions	frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx:110	20	10
<anonymous>	frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx:140	17	10
<anonymous>	frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx:455	13	10
<anonymous>	frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts:1145	11	10

🚨 Comment density — 14% of added code lines are comments (71 of 496)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File	Comment lines	Added lines
products/feature_flags/backend/api/test/test_feature_flag.py	29	209
products/feature_flags/backend/user_blast_radius.py	27	156
posthog/rate_limit.py	7	13
frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.matchingUsersLink.test.tsx	2	25
frontend/src/scenes/feature-flags/constants.tsx	2	6
frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts	2	20
products/feature_flags/backend/api/feature_flag.py	2	35

This check does not block merging. It updates on every push and clears when the share drops.

⚠️ Bundle size — 🔺 +568 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.59 MiB · 🔺 +568 B (+0.0%)

No file changed by more than 1000 B.

_{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.54 MiB · 22 files	🔺 +49 B (+0.0%)	███░░░░░░░ 34.1% of 4.51 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login) src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts	5.33 MiB · 669 files	🔺 +49 B (+0.0%)	█████████░ 94.8% of 5.63 MiB
authenticated shell (every logged-in page) src/scenes/AuthenticatedShell.tsx	7.71 MiB · 2,688 files	🔺 +286 B (+0.0%)	█████████░ 87.9% of 8.77 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
🟢 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

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
291.3 KiB	../node_modules/.pnpm/posthog-js@1.429.2_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
267.7 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
261.1 KiB	src/taxonomy/core-filter-definitions-by-group.json
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
103.5 KiB	src/lib/api.ts
79.7 KiB	src/products.tsx
68.9 KiB	src/lib/lemon-ui/icons/icons.tsx
62.6 KiB	src/lib/utils/eventUsageLogic.ts
38.8 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

Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx

Size	File
291.3 KiB	../node_modules/.pnpm/posthog-js@1.429.2_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
267.7 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
261.1 KiB	src/taxonomy/core-filter-definitions-by-group.json
153.8 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
103.5 KiB	src/lib/api.ts
96.4 KiB	../packages/quill/packages/quill/dist/index.js
93.3 KiB	../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB	../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
79.7 KiB	src/products.tsx

_{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.31 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.31 MiB · 18 files	🔺 +49 B (+0.0%)	████░░░░░░ 40.3% of 5.72 MiB
Deferred (lazy)	2.09 MiB · 45 files	no change	n/a — loads on demand
Loader dist/toolbar.js	1.1 KiB	no change	█░░░░░░░░░ 5.8% of 19.5 KiB

Largest eagerly-shipped chunks

Size	File
762.4 KiB	dist/toolbar/toolbar-app-RUAWSQGF.css
620.0 KiB	dist/toolbar/chunk-chunk-JCJC2LNQ.js
484.8 KiB	dist/toolbar/chunk-chunk-MBNPS67O.js
136.2 KiB	dist/toolbar/chunk-chunk-KFD5BJRX.js
131.8 KiB	dist/toolbar/chunk-chunk-FDH2IBXT.js
72.2 KiB	dist/toolbar/toolbar-app-27VIS43K.js
69.0 KiB	dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB	dist/toolbar/chunk-chunk-YYFVISPC.js
21.0 KiB	dist/toolbar/chunk-chunk-C3WOPC6H.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 — 🔺 +27.0 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1469.81 MiB · 🔺 +27.0 KiB (+0.0%)

ℹ️ MCP UI apps size — 33 app(s), 17726.0 KB JS

Built size of each MCP UI app (main.js + styles.css).

App	JS	CSS
debug	602.0 KB	197.2 KB
action	459.8 KB	197.2 KB
action-list	566.8 KB	197.2 KB
cohort	458.8 KB	197.2 KB
cohort-list	565.8 KB	197.2 KB
email-template	458.6 KB	197.2 KB
error-details	474.4 KB	197.2 KB
error-issue	459.4 KB	197.2 KB
error-issue-list	566.7 KB	197.2 KB
experiment	563.9 KB	197.2 KB
experiment-list	567.6 KB	197.2 KB
experiment-results	569.1 KB	197.2 KB
feature-flag	569.6 KB	197.2 KB
feature-flag-list	573.4 KB	197.2 KB
feature-flag-testing	463.0 KB	197.2 KB
inline-scan	459.2 KB	197.2 KB
insight-actors	565.0 KB	197.2 KB
invite-email-preview	458.0 KB	197.2 KB
llm-costs	561.9 KB	197.2 KB
session-recording	460.5 KB	197.2 KB
survey	460.3 KB	197.2 KB
survey-global-stats	564.7 KB	197.2 KB
survey-list	567.5 KB	197.2 KB
survey-stats	564.7 KB	197.2 KB
trace-span	459.1 KB	197.2 KB
trace-span-list	566.7 KB	197.2 KB
vision-observation-list	565.9 KB	197.2 KB
workflow	459.1 KB	197.2 KB
workflow-list	566.2 KB	197.2 KB
loops-review	463.4 KB	197.2 KB
query-results	759.0 KB	197.2 KB
render-ui	842.2 KB	197.2 KB
visual-review-snapshots	463.6 KB	197.2 KB

✅ Playwright — all passed

All tests passed.

View test results →

⚠️ Backend snapshots — 1 updated (1 modified, 0 added, 0 deleted)

Query snapshots: Backend query snapshots updated

Changes: 1 snapshots (1 modified, 0 added, 0 deleted)

What this means:

• Query snapshots have been automatically updated to match current output
• These changes reflect modifications to database queries or schema

Next steps:

• Review the query changes to ensure they're intentional
• If unexpected, investigate what caused the query to change

Review snapshot changes →

✅ Backend coverage — all changed backend lines covered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (105 / 105)

All changed backend lines are covered ✅

Per-product line coverage (touched products)

Product	Coverage	Lines
platform_features	██░░░░░░░░░░░░░░░░░░ 12.1%	7 / 58
warehouse_sources_queue	██████░░░░░░░░░░░░░░ 29.1%	92 / 316
demo	███████████░░░░░░░░░ 57.0%	1,523 / 2,671
data_tools	█████████████░░░░░░░ 66.2%	90 / 136
ai_gateway	███████████████░░░░░ 75.0%	9 / 12
batch_exports	████████████████░░░░ 79.8%	19,411 / 24,337
apm	█████████████████░░░ 83.3%	1,234 / 1,481
wizard	█████████████████░░░ 85.2%	2,770 / 3,250
cdp	█████████████████░░░ 85.3%	4,338 / 5,083
signals	█████████████████░░░ 85.5%	36,414 / 42,588
product_tours	██████████████████░░ 87.9%	1,303 / 1,482
notebooks	██████████████████░░ 88.5%	11,861 / 13,409
data_warehouse	██████████████████░░ 88.9%	12,855 / 14,456
business_knowledge	██████████████████░░ 89.2%	4,458 / 4,997
cohorts	██████████████████░░ 89.2%	7,526 / 8,433
dashboards	██████████████████░░ 89.6%	6,812 / 7,606
engineering_analytics	██████████████████░░ 90.1%	8,391 / 9,310
streamlit_apps	██████████████████░░ 90.2%	2,500 / 2,773
tasks	██████████████████░░ 90.3%	65,709 / 72,803
data_modeling	██████████████████░░ 90.4%	9,362 / 10,360
links	██████████████████░░ 90.6%	183 / 202
managed_warehouse	██████████████████░░ 90.7%	9,406 / 10,367
exports	██████████████████░░ 90.7%	9,098 / 10,027
alerts	██████████████████░░ 90.9%	5,224 / 5,745
visual_review	██████████████████░░ 91.0%	6,896 / 7,577
canvas	██████████████████░░ 91.2%	4,959 / 5,439
mcp_analytics	██████████████████░░ 91.2%	4,402 / 4,828
conversations	██████████████████░░ 91.3%	21,658 / 23,734
error_tracking	██████████████████░░ 92.0%	13,477 / 14,651
notifications	██████████████████░░ 92.4%	1,110 / 1,201
early_access_features	███████████████████░ 92.5%	1,323 / 1,430
managed_migrations	███████████████████░ 92.6%	1,556 / 1,681
slack_app	███████████████████░ 92.8%	12,596 / 13,578
surveys	███████████████████░ 93.1%	6,170 / 6,630
mcp_registry	███████████████████░ 93.2%	1,463 / 1,570
web_analytics	███████████████████░ 93.3%	19,313 / 20,695
stamphog	███████████████████░ 93.3%	6,533 / 6,999
context_layer	███████████████████░ 93.3%	2,594 / 2,779
reminders	███████████████████░ 93.4%	468 / 501
posthog_ai	███████████████████░ 93.5%	1,385 / 1,481
mcp_store	███████████████████░ 93.5%	8,053 / 8,611
marketing_analytics	███████████████████░ 93.5%	17,444 / 18,649
ai_observability	███████████████████░ 93.8%	18,194 / 19,402
autoresearch	███████████████████░ 93.9%	443 / 472
approvals	███████████████████░ 93.9%	3,560 / 3,793
billing_alerts	███████████████████░ 94.0%	2,069 / 2,200
endpoints	███████████████████░ 94.2%	8,904 / 9,456
review_hog	███████████████████░ 94.3%	10,205 / 10,817
tracing	███████████████████░ 94.5%	2,709 / 2,868
legal_documents	███████████████████░ 94.7%	2,073 / 2,190
workflows	███████████████████░ 94.7%	11,782 / 12,436
access_control	███████████████████░ 94.9%	6,052 / 6,378
customer_analytics	███████████████████░ 95.1%	19,279 / 20,274
annotations	███████████████████░ 95.2%	795 / 835
product_analytics	███████████████████░ 95.3%	19,348 / 20,304
actions	███████████████████░ 95.4%	740 / 776
replay_vision	███████████████████░ 95.5%	22,721 / 23,789
skills	███████████████████░ 95.6%	5,526 / 5,781
experiments	███████████████████░ 95.6%	30,240 / 31,626
messaging	███████████████████░ 95.8%	3,671 / 3,830
logs	███████████████████░ 95.9%	13,786 / 14,368
growth	███████████████████░ 96.0%	7,719 / 8,044
feature_flags	███████████████████░ 96.3%	20,650 / 21,439
revenue_analytics	███████████████████░ 96.3%	1,841 / 1,911
user_interviews	███████████████████░ 96.5%	2,639 / 2,735
warehouse_sources	███████████████████░ 97.2%	409,573 / 421,469
data_quality	████████████████████ 97.5%	4,217 / 4,323
analytics_platform	████████████████████ 98.1%	2,286 / 2,330
metrics	████████████████████ 98.2%	3,564 / 3,628
data_catalog	████████████████████ 98.4%	3,259 / 3,313
pulse	████████████████████ 98.4%	2,017 / 2,049
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.

✅ ClickHouse migration SQL — none

No ClickHouse migrations in the latest push.

[github-actions]

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login	test@posthog.com / 12345678 (demo data)
🧩 Running	this PR's backend and frontend, on the PostHog :master base
🔗 Link	stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access	tailnet only (PostHog VPN)
🛠️ Admin	inspect & debug state in hogland
💤 Idle	sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

_{commit a0d7fc4 · box box-58ba42f50283 · ready in 880s (push → usable) · build log · rebuilds on every push, torn down on close}

[greptile-apps]

Prompt To Fix All With AI

### Issue 1
products/feature_flags/backend/user_blast_radius.py:224
**Deleted persons inflate active totals**

When a recently active person is deleted, their retained events still contribute to `uniq(person_id)`, while the matching subquery only selects current `persons.id` values. This keeps deleted people in `total` for up to 30 days but excludes them from `affected`, inflating the displayed audience and distorting the match percentage.

### Issue 2
products/feature_flags/backend/user_blast_radius.py:157-166
**Helpers violate Python conventions**

The new helpers introduce function-local imports, and `_matched_persons_query` also lacks a return annotation. Move these imports to module scope and annotate the query builder’s return type so the new code follows the repository’s strict typing and import conventions.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(flags): size flags by recently activ..." | Re-trigger Greptile}

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs

[posthog]

🦔 PostHog Review reviewed this pull request

Found 1 must fix, 4 should fix, 2 consider.

Published 7 findings (view the review).

Resolved comments: 1 fixed, 7 left for you

[posthog]

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

PostHog Review

Found 1 must fix, 4 should fix, 2 consider.

[haacked]

Good fix for a real over-count bug, but a few blocking issues inline, plus some cleanup suggestions.

[haacked]

question: The PR description says the old all-time count "made person deletion look like it did nothing," but that claim does not hold on the unfiltered (no-properties) path introduced here. Deleting a person enqueues an async ClickHouse event delete that runs on a batch schedule; until that batch runs, the deleted person's events still contribute to uniq(person_id) in the unfiltered count, the same lag the PR says it removes, just measured against event deletion instead of person-table merge lag. On the filtered path, affected already excludes deleted persons today because the persons table applies its own deletion filter, so the claim holds there.

Should the deletion bullet in the PR description be qualified or dropped, so it does not read as fixing deletion-visibility everywhere?

[JamesPatrickGill]

Dropped. The deletion bullet is out of the description. You are right that the unfiltered path counts a deleted person's events until the async ClickHouse event delete runs. The claim only ever held on the filtered path.

[JamesPatrickGill]

The description no longer carries that bullet.

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

[haacked]

Nice fix for a real problem. Two blocking issues inline, plus a few suggestions.

[haacked]

blocking: The tooltip now says the estimate is limited to identified users active in the last 60 days, but the backend only applies that window when recently_active_sizing_enabled(self.team) returns true. Per the PR description, that's project 2 only at merge, and self-hosted instances always get False (only_evaluate_locally=True with no local flag definition ever resolves true). Everywhere else, the number behind this text is still the all-time person count this PR exists to fix, so the copy states the opposite of what the response actually computed.

The same unconditional 60-day claim is also baked into UserBlastRadiusResponseSerializer's help_text for affected and total, which regenerates into the frontend and MCP schemas.

Have the response say which basis produced the number (e.g. a nullable activity_window_days field, set only when the windowed path ran), and make the tooltip and help_text conditional on it instead of asserting the window unconditionally.

[JamesPatrickGill]

Fixed. The response carries activity_window_days, set only when the windowed path ran and null otherwise. MATCHING_ESTIMATE_TOOLTIP became matchingEstimateTooltip(activityWindowDays), which states the window only when there is one. The serializer help_text follows the same field.

[haacked]

suggestion: 'Filters match: ~N' and the 'View matching users' link right below it now disagree. The count excludes matching persons with no event in the last 60 days, but matchingActorsUrl builds the persons list from the condition's property filters alone, with no activity window, so the link lists every matching person including the ones the count just excluded. An author who clicks through to check the number finds more people than the panel said.

[JamesPatrickGill]

Correct, and not fixed here. matchingActorsUrl builds the persons list from the property filters alone, so windowing it means adding an activity filter to both the URL and the persons query. That is its own change, so it is called out as a known gap in the PR description rather than left for a reviewer to rediscover.

[haacked]

One blocking issue inline, the rest are non-blocking. I have resolved the earlier threads this push addressed.

[haacked]

blocking: On a flag where one condition targets a group type and another targets users, the person condition's estimate tooltip drops the "active in the last 60 days" clause and reads as an all-time estimate. That is the mis-statement activity_window_days was added to prevent.

The backend reports the window per request, not per project. _get_group_blast_radius never sets activity_window_days on either return branch, and get_user_blast_radius dispatches to it before the recently_active_only branch, so a group-scoped condition always answers null. Every response writes the same unkeyed activityWindowDays reducer, so the last one to land wins. Switching one condition from Users to a group type is enough on its own: setConditionAggregation fires exactly one request, for the condition that changed, so that null lands with nothing competing. The condition has no properties yet, and an empty properties array still issues a request rather than short-circuiting.

groupTypeIndex is already in scope here and typed number | null, so guarding the write is the smaller fix:

Suggested change

	actions.setActivityWindowDays(response.activity_window_days ?? null)
	// Only a person-scoped response carries a window. The group path always answers null.
	if (groupTypeIndex === null) {
	actions.setActivityWindowDays(response.activity_window_days ?? null)
	}

Keying by sort_key the way affectedCounts and totalCounts already are is the other option, and matches the neighboring reducers. Either way the reducer comment above has to go with it, since "every response carries the same value" is the assumption that breaks.

[haacked]

suggestion: The five-minute cache on the unfiltered active-person count doesn't collapse the requests it was written for, because they are all in flight before any of them writes the key. calculateBlastRadius loops the flag's condition groups and dispatches one request per group with nothing awaited between iterations, so a flag with five plain rollout conditions puts five requests on the wire on mount. Each reads the empty cache, runs its own 60-day uniq(person_id) scan, and holds a Django worker until it returns. The saving arrives from the second mount inside the TTL onward, not the first.

test_user_blast_radius_caches_the_unfiltered_active_count posts its two requests one after the other, so it passes either way.

Deduplicating in the logic removes the duplicate calls rather than the duplicate scans, and also covers two groups that happen to carry identical filters. Key an in-flight promise map on the request body and share the pending promise.

[haacked]

suggestion: This endpoint's rate limit gets eight times tighter for personal API key callers, and the comment that sizes the new number rests on a cache the throttle never sees.

On master user_blast_radius declares no throttle_classes, so it falls back to the DRF defaults of 480/minute and 4800/hour, keyed by personal API key hash so each key gets that budget to itself. The new pair replaces both with 60/minute and 600/hour in one bucket per project, shared across every key and every session user in it. The MCP tool and any integration holding a feature_flag:read key now share that 600/hour with the editor.

The last two lines of the comment justify the rate by pointing at the 5-minute cache, but that cache sits inside _recently_active_persons_count, below the view. It saves a ClickHouse scan, not a request, so a cache hit still spends a token and the editor's one-request-per-condition-group fan-out is not bounded by it.

Request volume per project is not readable from the repo, so the limit's actual impact isn't obvious without production data. Your call between raising the sustained rate toward what shipped, keeping a per-key bucket for API callers alongside the team bucket that covers session callers, or recording the 8x cut in the PR description so it reads as a decision rather than a side effect.

[haacked]

suggestion: _recently_active_persons_count and _get_person_blast_radius_recently_active each spell out the same tag_queries plus execute_hogql_query call with workload=Workload.OFFLINE and settings=HogQLGlobalSettings(timeout_overflow_mode="throw"), differing only in the query they pass. The comment explaining why the workload is offline and why overflow throws sits on the second copy only, so a reader of the first meets both settings with no reason attached. A later change to the workload or the timeout mode for activity-window scans has to land in both places; missing one leaves the two scans running under different settings.

Consider extracting a _run_activity_window_query(team, query) helper that carries the rationale comment once, and calling it from both paths; each caller's empty-result guard stays as it is.

[haacked]

nit: _IDENTIFIED_PERSONS_ONLY has exactly one use, and lifting it out is what forces the next constant to be an f-string mixing escaped HogQL placeholders ({{cutoff}}) with real interpolation. Inlining it lets _RECENTLY_ACTIVE_EVENTS_WHERE be a plain string whose braces are all placeholders:

_RECENTLY_ACTIVE_EVENTS_WHERE = "timestamp >= {cutoff} AND timestamp < {upper} AND person_mode != 'propertyless'"

The personless-event comment moves onto the surviving constant. The two query constants below still compose it with f-strings and need no change, since interpolation does not recurse.

[haacked]

nit: This docstring says "a change to one copy alone would make the previewed audience differ from the sized one", but this function is the reason there is only one copy. A reader goes looking for the second one.

"so the sized audience and the previewed one cannot diverge" says the same thing without implying a copy exists.

[haacked]

nit: This comment block runs two unrelated facts together with no break: where rollout percentages get computed, then why recently_active_only defaults to off. The second reads as a continuation of the first. A blank comment line between them, or moving the second onto the parameter, separates them.

[haacked]

nit: This loop runs the cached unfiltered count and the combined filtered scan through one shared assertion, so a failure does not say which of the two paths broke. parameterized.expand over the two conditions, the way test_user_blast_radius_rejects_unevaluable_filters just below it does, names them.

[scheduled-actions-posthog]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

[JamesPatrickGill]

Closing this branch. It is 23 days old, it conflicts with master, and it did not get a review. A fresh branch from master costs less than a rebase.

The change is still wanted. It returns as a new pull request: size the release-condition blast radius from recently active persons over a 60-day window, and add the rate-limit guard.

The two ReviewHog findings on this branch stay valid and depend on that change.