From c44b598f66d90c03d8436075f0873614f05af362 Mon Sep 17 00:00:00 2001 From: James Gill Date: Tue, 1 Sep 2026 16:09:41 +0100 Subject: [PATCH 01/12] fix(flags): size flags by recently active persons, not all-time count The release-conditions panel sized a flag's audience from an all-time person count. Anonymous one-shot persons inflate that count, so it read larger than the real audience. Size it from persons active in the last 30 days instead, computed in one events pass that yields the active total and the matched subset together. Generated-By: PostHog Desktop Task-Id: b5f0f648-33a7-4ac4-b279-2635e8546833 --- .../FeatureFlagReleaseConditions.tsx | 2 +- .../src/scenes/feature-flags/constants.tsx | 7 +- .../feature_flags/backend/api/feature_flag.py | 13 +- .../test/__snapshots__/test_feature_flag.ambr | 326 +++++++++++------- .../backend/api/test/test_feature_flag.py | 101 ++++-- .../backend/user_blast_radius.py | 121 +++++-- .../frontend/generated/api.schemas.ts | 4 +- 7 files changed, 396 insertions(+), 178 deletions(-) diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx index 7d812d56689a..ad7f4697f2cf 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx @@ -499,7 +499,7 @@ export function FeatureFlagReleaseConditions({
Filters match: ~{pluralize(affected, singularName, pluralName)} - {filters.aggregation_group_type_index == null && ( + {resolvedGroupTypeIndex == null && ( diff --git a/frontend/src/scenes/feature-flags/constants.tsx b/frontend/src/scenes/feature-flags/constants.tsx index eb002b3efdec..88c25e0cec57 100644 --- a/frontend/src/scenes/feature-flags/constants.tsx +++ b/frontend/src/scenes/feature-flags/constants.tsx @@ -15,11 +15,8 @@ export const MATCHING_ESTIMATE_TOOLTIP = (
- Estimated from{' '} - - identified users - {' '} - only. Anonymous visitors can still match this flag, so the actual number may be higher. + Estimated from persons active in the last 30 days, so people who have not been seen recently are not + counted.
) diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index b529eb70d224..412ff782575c 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -2813,9 +2813,18 @@ class UserBlastRadiusRequestSerializer(serializers.Serializer): class UserBlastRadiusResponseSerializer(serializers.Serializer): affected = serializers.IntegerField( - help_text="Number of entities matching the condition (users or groups depending on group_type_index)" + help_text=( + "Number of entities matching the condition. For person-based flags this counts persons active in the " + "last 30 days; for group-based flags it counts all groups of this type." + ) + ) + total = serializers.IntegerField( + help_text=( + "Denominator the affected count is shown against. For person-based flags this is the number of persons " + "active in the last 30 days; for group-based flags it is the all-time number of groups of this type in " + "the project." + ) ) - total = serializers.IntegerField(help_text="Total number of entities of this type in the project") # HYPERCACHE CONTRACT: This serializer defines the JSON schema that the Rust feature-flags diff --git a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr index 5ec5a4b38d26..54feceb180a8 100644 --- a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr +++ b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr @@ -24,19 +24,28 @@ # --- # name: TestBlastRadius.test_persons_seen_so_far_ignores_team_v2_argmax_modifier.1 ''' - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, - argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), '0'), 0))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), ifNull(equals(persons.properties___group, '0'), 0)) + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), '0'), 0))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), ifNull(equals(persons.properties___group, '0'), 0))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT tupleElement(argMax(tuple(properties.group), raw_persons.version), 1) AS properties___group, argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), equals(properties.group, '0')))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), equals(properties.group, '0')))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -77,19 +86,28 @@ # name: TestBlastRadius.test_user_blast_radius ''' /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, - argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), tuple('0', '1', '2', '3')))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), in(persons.properties___group, tuple('0', '1', '2', '3'))) + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), tuple('0', '1', '2', '3')))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), in(persons.properties___group, tuple('0', '1', '2', '3')))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT tupleElement(argMax(tuple(properties.group), raw_persons.version), 1) AS properties___group, argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), in(properties.group, tuple('0', '1', '2', '3'))))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), in(properties.group, tuple('0', '1', '2', '3'))))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -288,30 +306,40 @@ # name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.2 ''' /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))), in(where_optimization.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), and(in(persons.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))), in(persons.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))) + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))), in(where_optimization.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), and(in(persons.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))), in(persons.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0))))))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0)))), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0))))))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), and(in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0)))), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0)))))))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -363,25 +391,34 @@ # name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.1 ''' /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, - argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, - (SELECT person_static_cohort.person_id AS person_id - FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), tuple('1', '2', '4', '5', '6')))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), and(in(persons.id, - (SELECT person_static_cohort.person_id AS person_id - FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(persons.properties___group, tuple('1', '2', '4', '5', '6')))) + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, + (SELECT person_static_cohort.person_id AS person_id + FROM person_static_cohort + WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), tuple('1', '2', '4', '5', '6')))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), and(in(persons.id, + (SELECT person_static_cohort.person_id AS person_id + FROM person_static_cohort + WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(persons.properties___group, tuple('1', '2', '4', '5', '6'))))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT tupleElement(argMax(tuple(properties.group), raw_persons.version), 1) AS properties___group, argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), in(id, (SELECT person_id FROM static_cohort_people WHERE equals(cohort_id, 99999))), in(properties.group, tuple('1', '2', '4', '5', '6'))))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), and(in(id, (SELECT person_id FROM static_cohort_people WHERE equals(cohort_id, 99999))), in(properties.group, tuple('1', '2', '4', '5', '6')))))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -398,14 +435,51 @@ # --- # name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.2 ''' + + SELECT count(DISTINCT person_id) + FROM cohortpeople + WHERE team_id = 99999 + AND cohort_id = 99999 + AND version = 0 + ''' +# --- +# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.3 + ''' /* user_id:0 request:_snapshot_ */ - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, + (SELECT person_static_cohort.person_id AS person_id + FROM person_static_cohort + WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(where_optimization.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), and(in(persons.id, + (SELECT person_static_cohort.person_id AS person_id + FROM person_static_cohort + WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(persons.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0))))))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), in(id, (SELECT person_id FROM static_cohort_people WHERE equals(cohort_id, 99999))), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0))))))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), and(in(id, (SELECT person_id FROM static_cohort_people WHERE equals(cohort_id, 99999))), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0)))))))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -420,16 +494,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.3 - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 99999 - AND cohort_id = 99999 - AND version = 0 - ''' -# --- # name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.4 ''' /* user_id:0 request:_snapshot_ */ @@ -498,19 +562,28 @@ # name: TestBlastRadius.test_user_blast_radius_with_single_cohort ''' /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, - argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), or(ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), 'none'), 0), in(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), tuple('1', '2', '3'))))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), or(ifNull(equals(persons.properties___group, 'none'), 0), in(persons.properties___group, tuple('1', '2', '3')))) + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), or(ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), 'none'), 0), in(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), tuple('1', '2', '3'))))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), or(ifNull(equals(persons.properties___group, 'none'), 0), in(persons.properties___group, tuple('1', '2', '3'))))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT tupleElement(argMax(tuple(properties.group), raw_persons.version), 1) AS properties___group, argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), or(equals(properties.group, 'none'), in(properties.group, tuple('1', '2', '3')))))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), or(equals(properties.group, 'none'), in(properties.group, tuple('1', '2', '3')))))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -527,14 +600,45 @@ # --- # name: TestBlastRadius.test_user_blast_radius_with_single_cohort.1 ''' + + SELECT count(DISTINCT person_id) + FROM cohortpeople + WHERE team_id = 99999 + AND cohort_id = 99999 + AND version = 0 + ''' +# --- +# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.2 + ''' /* user_id:0 request:_snapshot_ */ - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons + SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, + uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), + (SELECT persons.id AS id + FROM + (SELECT argMax(person.team_id, person.version) AS team_id, person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), in(persons.id, + (SELECT cohortpeople.person_id AS person_id + FROM cohortpeople + WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0))))))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), in(id, (SELECT person_id FROM raw_cohort_people WHERE and(equals(cohort_id, 99999), equals(version, 0))))))))` + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 99999) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -549,16 +653,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.2 - ''' - - SELECT count(DISTINCT person_id) - FROM cohortpeople - WHERE team_id = 99999 - AND cohort_id = 99999 - AND version = 0 - ''' -# --- # name: TestBlastRadius.test_user_blast_radius_with_single_cohort.3 ''' /* user_id:0 request:_snapshot_ */ diff --git a/products/feature_flags/backend/api/test/test_feature_flag.py b/products/feature_flags/backend/api/test/test_feature_flag.py index 0d02dc8e3f74..730af4520f66 100644 --- a/products/feature_flags/backend/api/test/test_feature_flag.py +++ b/products/feature_flags/backend/api/test/test_feature_flag.py @@ -8,6 +8,7 @@ APIBaseTest, ClickhouseTestMixin, FuzzyInt, + _create_event, _create_person, flush_persons_and_events, snapshot_clickhouse_queries, @@ -9330,11 +9331,23 @@ def test_insert_failure_is_recorded_as_failure_not_success(self, mock_batch_eval self.assertEqual(history.error_code, CohortErrorCode.UNKNOWN) +def _create_active_person(*args, **kwargs): + # Flag sizing counts persons active in the recent window, so every person a sizing test relies + # on needs a recent event. Wrap person creation to emit one $pageview per person. + person = _create_person(*args, **kwargs) + team = kwargs.get("team") + team_id = kwargs.get("team_id") or (team.pk if team else None) + distinct_ids = kwargs.get("distinct_ids") or [] + if distinct_ids: + _create_event(team_id=team_id, event="$pageview", distinct_id=distinct_ids[0]) + return person + + class TestBlastRadius(ClickhouseTestMixin, APIBaseTest): @snapshot_clickhouse_queries def test_user_blast_radius(self): for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9366,7 +9379,8 @@ def test_user_blast_radius(self): def test_persons_seen_so_far_ignores_team_v2_argmax_modifier(self): team = Team.objects.create(organization=self.organization, modifiers={"personsArgMaxVersion": "v2"}) for i in range(3): - _create_person(team_id=team.pk, distinct_ids=[f"seen_person_{i}"], properties={"group": f"{i}"}) + _create_active_person(team_id=team.pk, distinct_ids=[f"seen_person_{i}"], properties={"group": f"{i}"}) + flush_persons_and_events() with self.capture_select_queries() as queries: assert team.persons_seen_so_far == 3 @@ -9378,8 +9392,9 @@ def test_persons_seen_so_far_ignores_team_v2_argmax_modifier(self): # the assertion would always pass. assert "in(tuple(person.id, person.version)" not in query - # The filtered path builds one database and shares it between its two counts. - # The denominator must keep the pinned v1 shape even then. + # The filtered path builds one database and shares it across its numerator and denominator + # counts. The matched count still reads the persons table, so every query it emits must keep + # the pinned v1 person shape — otherwise the v2 modifier truncates the persons it matches. with patch.object(Database, "create_for", wraps=Database.create_for) as create_for_spy: with self.capture_select_queries() as queries: result = get_user_blast_radius( @@ -9389,11 +9404,37 @@ def test_persons_seen_so_far_ignores_team_v2_argmax_modifier(self): assert (result.affected, result.total) == (1, 3) assert create_for_spy.call_count == 1 - denominator_queries = [query for query in queries if "count(DISTINCT" not in query] - assert denominator_queries - for query in denominator_queries: + assert queries + for query in queries: assert "in(tuple(person.id, person.version)" not in query + def test_user_blast_radius_excludes_inactive_matching_persons(self): + # A person matching the filter but with no recent activity is not reachable audience, so it + # must not count toward "affected" — otherwise inactive matches inflate the sized audience, + # which is the bug this fix guards against. + _create_active_person(team_id=self.team.pk, distinct_ids=["active-match-1"], properties={"group": "match"}) + _create_active_person(team_id=self.team.pk, distinct_ids=["active-match-2"], properties={"group": "match"}) + # Matches the filter but emits no event in the window, so it drops out of the count. + _create_person(team_id=self.team.pk, distinct_ids=["inactive-match"], properties={"group": "match"}) + # Active but does not match, so it keeps the total above the matched count. + _create_active_person(team_id=self.team.pk, distinct_ids=["active-nonmatch"], properties={"group": "other"}) + flush_persons_and_events() + + response = self.client.post( + f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", + { + "condition": { + "properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}], + "rollout_percentage": 100, + } + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + # Only the two active matches count; the inactive match is excluded, and the active + # non-match keeps the total at three. + self.assertLessEqual({"affected": 2, "total": 3}.items(), response.json().items()) + @parameterized.expand( [ ( @@ -9448,7 +9489,7 @@ def test_user_blast_radius_endpoint_returns_400_for_unevaluable_filters(self): def test_user_blast_radius_with_flag_dependency(self): for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9484,7 +9525,7 @@ def test_user_blast_radius_with_flag_dependency(self): def test_user_blast_radius_with_flag_dependency_and_person_property(self): for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9630,7 +9671,7 @@ def test_user_blast_radius_persons_with_groups_and_flag_dependency(self): @freeze_time("2024-01-11") def test_user_blast_radius_with_relative_date_filters(self): for i in range(8): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}", "created_at": f"2023-0{i + 1}-04"}, @@ -9683,7 +9724,7 @@ def test_user_blast_radius_with_zero_users(self): def test_user_blast_radius_with_zero_selected_users(self): for i in range(5): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9713,7 +9754,7 @@ def test_user_blast_radius_with_zero_selected_users(self): def test_user_blast_radius_with_all_selected_users(self): for i in range(5): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9734,7 +9775,7 @@ def test_user_blast_radius_with_distinct_id_filter(self): # person_distinct_id2 table and must be joined via pdi. Filtering by distinct_id # in a release condition should match the persons that own that distinct_id. for i in range(5): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9765,12 +9806,12 @@ def test_user_blast_radius_with_distinct_id_filter(self): def test_user_blast_radius_with_distinct_id_filter_multiple_distinct_ids_per_person(self): # A single person can own multiple distinct_ids; filtering by any one should still # count that person exactly once. - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=["alias-a", "alias-b"], properties={"group": "0"}, ) - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=["other"], properties={"group": "1"}, @@ -9802,7 +9843,7 @@ def test_user_blast_radius_with_distinct_id_filter_multiple_distinct_ids_per_per def test_user_blast_radius_with_single_cohort(self): # Just to shake things up, we're using integers for the group property for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": i}, @@ -9868,7 +9909,7 @@ def test_user_blast_radius_with_single_cohort(self): @snapshot_clickhouse_queries def test_user_blast_radius_with_multiple_precalculated_cohorts(self): for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -9945,7 +9986,7 @@ def test_user_blast_radius_with_multiple_precalculated_cohorts(self): @snapshot_clickhouse_queries def test_user_blast_radius_with_multiple_static_cohorts(self): for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"group": f"{i}"}, @@ -10269,17 +10310,17 @@ def test_user_blast_radius_with_group_key_property(self): def test_user_blast_radius_with_integer_property_values(self): """Test that integer property values are correctly normalized to strings for matching""" - _create_person( + _create_active_person( distinct_ids=["p1"], team_id=self.team.pk, properties={"age": 25, "score": 100}, ) - _create_person( + _create_active_person( distinct_ids=["p2"], team_id=self.team.pk, properties={"age": "25", "score": "100"}, ) - _create_person( + _create_active_person( distinct_ids=["p3"], team_id=self.team.pk, properties={"age": 30, "score": 200}, @@ -10453,12 +10494,12 @@ def test_user_blast_radius_with_dynamic_cohort(self): }, ) - _create_person( + _create_active_person( distinct_ids=["p1"], team_id=self.team.pk, properties={"email": "user@posthog.com"}, ) - _create_person( + _create_active_person( distinct_ids=["p2"], team_id=self.team.pk, properties={"email": "user@example.com"}, @@ -10786,7 +10827,7 @@ def test_user_blast_radius_with_semver_operators(self): "2.1.0", ] for version in versions: - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person_{version}"], properties={"app_version": version}, @@ -11004,7 +11045,7 @@ def test_user_blast_radius_with_semver_caret_0x_versions(self): "1.0.0", ] for version in versions: - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person_{version}"], properties={"app_version": version}, @@ -11074,7 +11115,7 @@ def test_user_blast_radius_with_semver_operators_on_groups(self): # Create persons in organizations for i, version in enumerate(versions): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person_{i}"], properties={"$group_0": f"org-{version}"}, @@ -11113,7 +11154,7 @@ def test_user_blast_radius_person_condition_separate_from_group_condition(self): ) for i in range(10): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"plan": "pro" if i < 6 else "free"}, @@ -11174,7 +11215,7 @@ def test_user_blast_radius_person_condition_separate_from_group_condition(self): def test_user_blast_radius_pure_person_condition_has_no_group_counts(self): for i in range(5): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"plan": "pro" if i < 3 else "free"}, @@ -11254,7 +11295,7 @@ def test_user_blast_radius_cohort_condition_and_group_condition_separate(self): ) for i in range(8): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"plan": "pro" if i < 5 else "free"}, @@ -11331,7 +11372,7 @@ def test_user_blast_radius_cohort_condition_and_group_condition_separate(self): def test_user_blast_radius_no_error_fields_for_successful_queries(self): for i in range(3): - _create_person( + _create_active_person( team_id=self.team.pk, distinct_ids=[f"person{i}"], properties={"plan": "pro"}, diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index b317e7dcd954..ae28204ae368 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -1,8 +1,10 @@ from collections.abc import Iterator from contextlib import contextmanager +from datetime import datetime, timedelta from typing import Optional from django.core.exceptions import ObjectDoesNotExist +from django.utils import timezone from rest_framework.exceptions import ValidationError @@ -31,6 +33,11 @@ class BlastRadiusResult: total: int +# Window for the flag-sizing denominator. An all-time person count is inflated by anonymous, +# one-shot persons that never return; a recent-activity window drops them and is defensible. +RECENTLY_ACTIVE_DAYS = 30 + + # ClickHouse codes for "this literal can't be parsed as the column's type": 6 CANNOT_PARSE_TEXT, # 72 CANNOT_PARSE_NUMBER — e.g. a numeric operator (gt/lt) against a null or non-numeric filter # value casts 'None' to Float64 and fails deterministically. Both are classified USER_ERROR in @@ -141,39 +148,109 @@ def get_user_blast_radius_persons( return _get_person_blast_radius_persons(team, cleaned_filter, cursor=cursor) +def _recently_active_cutoff() -> datetime: + """Start of the flag-sizing activity window, computed in Python so it respects freeze_time in + tests and does not depend on ClickHouse server time.""" + return timezone.now() - timedelta(days=RECENTLY_ACTIVE_DAYS) + + +def recently_active_persons_count(team: Team, cutoff: Optional[datetime] = None) -> int: + """Count distinct persons active in the last RECENTLY_ACTIVE_DAYS days. + + This is the flag-sizing denominator. It replaces an all-time person count so anonymous churn + does not inflate the audience shown to a flag author (see RECENTLY_ACTIVE_DAYS). `uniq` is an + estimate, which matches the "~" the sizing panel already renders. + """ + from posthog.hogql import ast + from posthog.hogql.parser import parse_select + from posthog.hogql.query import execute_hogql_query + + if cutoff is None: + cutoff = _recently_active_cutoff() + + query = parse_select( + "SELECT uniq(person_id) FROM events WHERE timestamp >= {cutoff}", + placeholders={"cutoff": ast.Constant(value=cutoff)}, + ) + + tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) + response = execute_hogql_query(query=query, team=team) + + return response.results[0][0] if response.results else 0 + + +def _matched_persons_query(team: Team, filter: Filter): + """Subquery of the person ids matching the condition. Reuses property_to_expr in person scope, + so it resolves cohorts, static cohorts, and group properties exactly like the person listing.""" + from posthog.hogql import ast + from posthog.hogql.property import property_to_expr + + return ast.SelectQuery( + select=[ast.Field(chain=["persons", "id"])], + select_from=ast.JoinExpr(table=ast.Field(chain=["persons"])), + where=ast.And( + exprs=[ + ast.CompareOperation( + op=ast.CompareOperationOp.Eq, + left=ast.Field(chain=["persons", "team_id"]), + right=ast.Constant(value=team.pk), + ), + property_to_expr(filter.property_groups, team, scope="person"), + ] + ), + ) + + def _get_person_blast_radius(team: Team, filter: Filter) -> BlastRadiusResult: - """Calculate blast radius for person-based feature flags using HogQL.""" - from posthog.hogql.context import HogQLContext - from posthog.hogql.database.database import Database + """Calculate blast radius for person-based feature flags using HogQL. + + Both counts are of persons active in the same recent window, so the matched count is never a + different population from the total it is shown against. + """ + from posthog.hogql import ast from posthog.hogql.query import execute_hogql_query + cutoff = _recently_active_cutoff() properties = filter.property_groups.flat if len(properties) == 0: - # No filters means all persons are affected - total_users = team.persons_seen_so_far + # No filters means every recently active person is affected. + total_users = recently_active_persons_count(team, cutoff) return BlastRadiusResult(affected=total_users, total=total_users) - # Build the SELECT query - property_to_expr handles all properties including cohorts - select_query = _build_person_query(team, filter, return_count=True) - - # Execute the query - tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) - # Build the team's HogQL database once and share it between the two counts below. - # Each execute_hogql_query call would otherwise build its own, and the build cost - # scales with the team's warehouse size. - database = Database.create_for(team=team) - response = execute_hogql_query( - query=select_query, - team=team, - context=HogQLContext(team_id=team.pk, database=database), + # One pass over the recent event window yields both the active total and the matched subset, so + # the denominator is never a second scan and both numbers come from the same rows. + matched_persons = _matched_persons_query(team, filter) + query = ast.SelectQuery( + select=[ + ast.Call(name="uniq", args=[ast.Field(chain=["person_id"])]), + ast.Call( + name="uniqIf", + args=[ + ast.Field(chain=["person_id"]), + ast.CompareOperation( + op=ast.CompareOperationOp.In, + left=ast.Field(chain=["person_id"]), + right=matched_persons, + ), + ], + ), + ], + select_from=ast.JoinExpr(table=ast.Field(chain=["events"])), + where=ast.CompareOperation( + op=ast.CompareOperationOp.GtEq, + left=ast.Field(chain=["timestamp"]), + right=ast.Constant(value=cutoff), + ), ) - total_count = response.results[0][0] if response.results else 0 - total_users = team.count_persons_seen_so_far(database=database) - blast_radius = min(total_count, total_users) + tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) + response = execute_hogql_query(query=query, team=team) - return BlastRadiusResult(affected=blast_radius, total=total_users) + total_users, affected = (response.results[0][0], response.results[0][1]) if response.results else (0, 0) + # affected is a strict subset of total_users, but both are uniq() estimates, so clamp to keep + # the frontend percentage coherent. + return BlastRadiusResult(affected=min(affected, total_users), total=total_users) def _build_person_query(team: Team, filter: Filter, return_count: bool = True, cursor: Optional[str] = None): diff --git a/products/feature_flags/frontend/generated/api.schemas.ts b/products/feature_flags/frontend/generated/api.schemas.ts index 8bc124870478..033f85fcce88 100644 --- a/products/feature_flags/frontend/generated/api.schemas.ts +++ b/products/feature_flags/frontend/generated/api.schemas.ts @@ -1576,9 +1576,9 @@ export interface UserBlastRadiusRequestApi { } export interface UserBlastRadiusResponseApi { - /** Number of entities matching the condition (users or groups depending on group_type_index) */ + /** Number of entities matching the condition. For person-based flags this counts persons active in the last 30 days; for group-based flags it counts all groups of this type. */ affected: number - /** Total number of entities of this type in the project */ + /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 30 days; for group-based flags it is the all-time number of groups of this type in the project. */ total: number } From a387722e03d46a8a0ddfb11554d0808d0ad37637 Mon Sep 17 00:00:00 2001 From: "tests-posthog[bot]" <250237707+tests-posthog[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:35:06 +0000 Subject: [PATCH 02/12] chore: update OpenAPI generated types --- services/mcp/src/api/generated.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index 0b200a2e25fc..33dde5b61f32 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -85768,9 +85768,9 @@ export namespace Schemas { } export interface UserBlastRadiusResponse { - /** Number of entities matching the condition (users or groups depending on group_type_index) */ + /** Number of entities matching the condition. For person-based flags this counts persons active in the last 30 days; for group-based flags it counts all groups of this type. */ affected: number; - /** Total number of entities of this type in the project */ + /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 30 days; for group-based flags it is the all-time number of groups of this type in the project. */ total: number; } From c1b3c03b98e9ace0c68d2c8c223b0a796d937ea4 Mon Sep 17 00:00:00 2001 From: James Gill Date: Wed, 2 Sep 2026 08:41:04 +0100 Subject: [PATCH 03/12] fix(flags): widen active-persons window to 60 days Size flags by persons active in the last 60 days, up from 30. A 30-day window drops real users a flag targets who log in monthly. 60 days keeps those users in the denominator while still excluding the anonymous one-shot tail. Generated-By: PostHog Desktop Task-Id: b5f0f648-33a7-4ac4-b279-2635e8546833 --- frontend/src/scenes/feature-flags/constants.tsx | 2 +- products/feature_flags/backend/api/feature_flag.py | 4 ++-- products/feature_flags/backend/user_blast_radius.py | 2 +- products/feature_flags/frontend/generated/api.schemas.ts | 4 ++-- services/mcp/src/api/generated.ts | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frontend/src/scenes/feature-flags/constants.tsx b/frontend/src/scenes/feature-flags/constants.tsx index 88c25e0cec57..b3492724e330 100644 --- a/frontend/src/scenes/feature-flags/constants.tsx +++ b/frontend/src/scenes/feature-flags/constants.tsx @@ -15,7 +15,7 @@ export const MATCHING_ESTIMATE_TOOLTIP = (
- Estimated from persons active in the last 30 days, so people who have not been seen recently are not + Estimated from persons active in the last 60 days, so people who have not been seen recently are not counted.
diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index 412ff782575c..c8ffd376bc63 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -2815,13 +2815,13 @@ class UserBlastRadiusResponseSerializer(serializers.Serializer): affected = serializers.IntegerField( help_text=( "Number of entities matching the condition. For person-based flags this counts persons active in the " - "last 30 days; for group-based flags it counts all groups of this type." + "last 60 days; for group-based flags it counts all groups of this type." ) ) total = serializers.IntegerField( help_text=( "Denominator the affected count is shown against. For person-based flags this is the number of persons " - "active in the last 30 days; for group-based flags it is the all-time number of groups of this type in " + "active in the last 60 days; for group-based flags it is the all-time number of groups of this type in " "the project." ) ) diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index ae28204ae368..bdad14aab3aa 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -35,7 +35,7 @@ class BlastRadiusResult: # Window for the flag-sizing denominator. An all-time person count is inflated by anonymous, # one-shot persons that never return; a recent-activity window drops them and is defensible. -RECENTLY_ACTIVE_DAYS = 30 +RECENTLY_ACTIVE_DAYS = 60 # ClickHouse codes for "this literal can't be parsed as the column's type": 6 CANNOT_PARSE_TEXT, diff --git a/products/feature_flags/frontend/generated/api.schemas.ts b/products/feature_flags/frontend/generated/api.schemas.ts index 033f85fcce88..5e80fc0c0184 100644 --- a/products/feature_flags/frontend/generated/api.schemas.ts +++ b/products/feature_flags/frontend/generated/api.schemas.ts @@ -1576,9 +1576,9 @@ export interface UserBlastRadiusRequestApi { } export interface UserBlastRadiusResponseApi { - /** Number of entities matching the condition. For person-based flags this counts persons active in the last 30 days; for group-based flags it counts all groups of this type. */ + /** Number of entities matching the condition. For person-based flags this counts persons active in the last 60 days; for group-based flags it counts all groups of this type. */ affected: number - /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 30 days; for group-based flags it is the all-time number of groups of this type in the project. */ + /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 60 days; for group-based flags it is the all-time number of groups of this type in the project. */ total: number } diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index 33dde5b61f32..98cf104054d4 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -85768,9 +85768,9 @@ export namespace Schemas { } export interface UserBlastRadiusResponse { - /** Number of entities matching the condition. For person-based flags this counts persons active in the last 30 days; for group-based flags it counts all groups of this type. */ + /** Number of entities matching the condition. For person-based flags this counts persons active in the last 60 days; for group-based flags it counts all groups of this type. */ affected: number; - /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 30 days; for group-based flags it is the all-time number of groups of this type in the project. */ + /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 60 days; for group-based flags it is the all-time number of groups of this type in the project. */ total: number; } From 3a8b062f8492b1425f95a4910cb4805036c965a5 Mon Sep 17 00:00:00 2001 From: "tests-posthog[bot]" <250237707+tests-posthog[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:04:13 +0000 Subject: [PATCH 04/12] test(backend): update query snapshots --- .../test/__snapshots__/test_feature_flag.ambr | 195 ------------------ 1 file changed, 195 deletions(-) diff --git a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr index d3bacc08e1b2..650577a0f864 100644 --- a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr +++ b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr @@ -60,29 +60,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_persons_seen_so_far_ignores_team_v2_argmax_modifier.2 - ''' - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- # name: TestBlastRadius.test_user_blast_radius ''' /* user_id:0 request:_snapshot_ */ @@ -122,30 +99,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_user_blast_radius.1 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- # name: TestBlastRadius.test_user_blast_radius_with_group_key_property ''' /* user_id:0 request:_snapshot_ */ @@ -407,30 +360,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_precalculated_cohorts.3 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- # name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts ''' SELECT person_id @@ -547,71 +476,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.4 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, - (SELECT person_static_cohort.person_id AS person_id - FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(where_optimization.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), and(in(persons.id, - (SELECT person_static_cohort.person_id AS person_id - FROM person_static_cohort - WHERE and(equals(person_static_cohort.team_id, 99999), equals(person_static_cohort.cohort_id, 99999)))), in(persons.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))) - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_multiple_static_cohorts.5 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- # name: TestBlastRadius.test_user_blast_radius_with_single_cohort ''' /* user_id:0 request:_snapshot_ */ @@ -706,65 +570,6 @@ use_hive_partitioning=0 ''' # --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.3 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` - FROM - (SELECT argMax(person.team_id, person.version) AS team_id, - person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), in(where_optimization.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0)))))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), in(persons.id, - (SELECT cohortpeople.person_id AS person_id - FROM cohortpeople - WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, 99999), equals(cohortpeople.version, 0))))) - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- -# name: TestBlastRadius.test_user_blast_radius_with_single_cohort.4 - ''' - /* user_id:0 request:_snapshot_ */ - SELECT count() AS `count()` - FROM - (SELECT person.id AS id - FROM person - WHERE equals(person.team_id, 99999) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons - LIMIT 100 SETTINGS readonly=2, - max_execution_time=60, - allow_experimental_object_type=1, - max_ast_elements=4000000, - max_expanded_ast_elements=4000000, - max_bytes_before_external_group_by=0, - transform_null_in=1, - optimize_min_equality_disjunction_chain_length=4294967295, - optimize_rewrite_aggregate_function_with_if=0, - optimize_min_inequality_conjunction_chain_length=4294967295, - allow_experimental_join_condition=1, - use_hive_partitioning=0 - ''' -# --- # name: TestFeatureFlag.test_creating_static_cohort ''' SELECT "posthog_user"."id", From 138263a59814e61daaae9dbda5c3c12727a5f17a Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:16:53 +0000 Subject: [PATCH 05/12] fix(flags): keep the anonymous-visitor caveat on the sizing tooltip The filtered estimate matches persons rows, so a visitor without a stored profile can receive the flag without entering the count. The recent-activity line alone reads as if inactivity were the only reason someone is left out. Generated-By: PostHog Desktop Task-Id: f46fd1ce-288f-46fe-ae47-93a1d4eddf09 --- frontend/src/scenes/feature-flags/constants.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/src/scenes/feature-flags/constants.tsx b/frontend/src/scenes/feature-flags/constants.tsx index b3492724e330..0af45bf8ab71 100644 --- a/frontend/src/scenes/feature-flags/constants.tsx +++ b/frontend/src/scenes/feature-flags/constants.tsx @@ -18,5 +18,12 @@ export const MATCHING_ESTIMATE_TOOLTIP = ( Estimated from persons active in the last 60 days, so people who have not been seen recently are not counted. +
+ Conditions that filter on properties or cohorts count{' '} + + identified users + {' '} + only. Anonymous visitors can still match this flag, so the actual number may be higher. +
) From 0960146683618dd99989e38633df6fa226500a52 Mon Sep 17 00:00:00 2001 From: James Gill Date: Fri, 4 Sep 2026 11:32:39 +0100 Subject: [PATCH 06/12] fix(flags): scope the activity window to flag sizing only The 60-day window reached the workflows audience preview through the shared get_user_blast_radius, so a preview could show fewer people than the unwindowed batch send enumerates. The window is now an opt-in that only the feature-flags endpoint uses; workflows keep the all-time count. Also bound the window from above so a far-future event timestamp cannot keep a person active for years, and route the window's event scans to the offline ClickHouse pool like the group path already does. Co-Authored-By: Claude Fable 5 --- .../feature_flags/backend/api/feature_flag.py | 2 +- .../test/__snapshots__/test_feature_flag.ambr | 70 +++++++++------ .../backend/api/test/test_feature_flag.py | 29 ++++-- .../backend/user_blast_radius.py | 90 ++++++++++++++----- 4 files changed, 134 insertions(+), 57 deletions(-) diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index affdf355c3ab..249c7acd21f8 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -4560,7 +4560,7 @@ def user_blast_radius(self, request: request.Request, **kwargs): condition = request.data.get("condition") or {} group_type_index = request.data.get("group_type_index", None) - result = get_user_blast_radius(self.team, condition, group_type_index) + result = get_user_blast_radius(self.team, condition, group_type_index, recently_active_only=True) return Response({"affected": result.affected, "total": result.total}) diff --git a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr index 650577a0f864..632adc123860 100644 --- a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr +++ b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr @@ -24,28 +24,42 @@ # --- # name: TestBlastRadius.test_persons_seen_so_far_ignores_team_v2_argmax_modifier.1 ''' - SELECT uniq(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id)) AS `uniq(if(not(empty(override.distinct_id)), override.person_id, event_person_id))`, - uniqIf(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), in(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), - (SELECT persons.id AS id - FROM - (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, argMax(person.team_id, person.version) AS team_id, person.id AS id - FROM person - WHERE and(equals(person.team_id, 99999), in(id, - (SELECT where_optimization.id AS id - FROM person AS where_optimization - WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), '0'), 0))))) - GROUP BY person.id - HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons - WHERE and(equals(persons.team_id, 99999), ifNull(equals(persons.properties___group, '0'), 0))))) AS `uniqIf(if(not(empty(override.distinct_id)), override.person_id, event_person_id), in(if(not(empty(override.distinct_id)), override.person_id, event_person_id), (SELECT persons.id FROM (SELECT tupleElement(argMax(tuple(properties.group), raw_persons.version), 1) AS properties___group, argMax(raw_persons.team_id, raw_persons.version) AS team_id, raw_persons.id AS id FROM raw_persons WHERE in(id, (SELECT id FROM raw_persons AS where_optimization WHERE and(equals(team_id, 99999), equals(properties.group, '0')))) GROUP BY raw_persons.id HAVING and(equals(argMax(raw_persons.is_deleted, raw_persons.version), 0), less(argMax(toTimeZone(raw_persons.created_at, 'UTC'), raw_persons.version), plus(now(), toIntervalDay(1))))) AS persons WHERE and(equals(persons.team_id, 99999), equals(properties.group, '0')))))` - FROM events - LEFT OUTER JOIN - (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, - person_distinct_id_overrides.distinct_id AS distinct_id - FROM person_distinct_id_overrides - WHERE equals(person_distinct_id_overrides.team_id, 99999) - GROUP BY person_distinct_id_overrides.distinct_id - HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + SELECT count(DISTINCT persons.id) AS `count(DISTINCT persons.id)` + FROM + (SELECT tupleElement(argMax(tuple(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'group'), ''), 'null'), '^"|"$', '')), person.version), 1) AS properties___group, + argMax(person.team_id, person.version) AS team_id, + person.id AS id + FROM person + WHERE and(equals(person.team_id, 99999), in(id, + (SELECT where_optimization.id AS id + FROM person AS where_optimization + WHERE and(equals(where_optimization.team_id, 99999), equals(where_optimization.team_id, 99999), ifNull(equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(where_optimization.properties, 'group'), ''), 'null'), '^"|"$', ''), '0'), 0))))) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))))) AS persons + WHERE and(equals(persons.team_id, 99999), ifNull(equals(persons.properties___group, '0'), 0)) + LIMIT 100 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0, + transform_null_in=1, + optimize_min_equality_disjunction_chain_length=4294967295, + optimize_rewrite_aggregate_function_with_if=0, + optimize_min_inequality_conjunction_chain_length=4294967295, + allow_experimental_join_condition=1, + use_hive_partitioning=0 + ''' +# --- +# name: TestBlastRadius.test_persons_seen_so_far_ignores_team_v2_argmax_modifier.2 + ''' + SELECT count() AS `count()` + FROM + (SELECT person.id AS id + FROM person + WHERE equals(person.team_id, 99999) + GROUP BY person.id + HAVING and(equals(argMax(person.is_deleted, person.version), 0), less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1)))) SETTINGS optimize_aggregation_in_order=1) AS persons LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -84,7 +98,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -345,7 +359,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -400,7 +414,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -461,7 +475,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -500,7 +514,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, @@ -555,7 +569,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, diff --git a/products/feature_flags/backend/api/test/test_feature_flag.py b/products/feature_flags/backend/api/test/test_feature_flag.py index 03342ea10276..b7d44ed8e94b 100644 --- a/products/feature_flags/backend/api/test/test_feature_flag.py +++ b/products/feature_flags/backend/api/test/test_feature_flag.py @@ -9446,23 +9446,36 @@ def test_user_blast_radius_excludes_inactive_matching_persons(self): _create_person(team_id=self.team.pk, distinct_ids=["inactive-match"], properties={"group": "match"}) # Active but does not match, so it keeps the total above the matched count. _create_active_person(team_id=self.team.pk, distinct_ids=["active-nonmatch"], properties={"group": "other"}) + # A corrupt far-future timestamp must not keep a person "active" past the window's end. + _create_person(team_id=self.team.pk, distinct_ids=["future-match"], properties={"group": "match"}) + _create_event( + team_id=self.team.pk, + event="$pageview", + distinct_id="future-match", + timestamp=now() + timedelta(days=365), + ) flush_persons_and_events() + condition = { + "properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}], + "rollout_percentage": 100, + } response = self.client.post( f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", - { - "condition": { - "properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}], - "rollout_percentage": 100, - } - }, + {"condition": condition}, ) self.assertEqual(response.status_code, status.HTTP_200_OK) - # Only the two active matches count; the inactive match is excluded, and the active - # non-match keeps the total at three. + # Only the two active matches count; the inactive and far-future matches are excluded, and + # the active non-match keeps the total at three. self.assertLessEqual({"affected": 2, "total": 3}.items(), response.json().items()) + # The window is an opt-in for the flags endpoint only. The workflows audience preview calls + # this function without it and must keep counting every matching person, because the batch + # send it previews enumerates persons with no activity window. + unwindowed = get_user_blast_radius(self.team, condition) + self.assertEqual((unwindowed.affected, unwindowed.total), (4, 5)) + @parameterized.expand( [ ( diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index dfeee50727c7..8ead9e3d4364 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -127,15 +127,20 @@ def get_user_blast_radius( team: Team, feature_flag_condition: dict, group_type_index: Optional[GroupTypeIndex] = None, + *, + recently_active_only: bool = False, ) -> BlastRadiusResult: # No rollout % calculations here, since it makes more sense to compute that on the frontend + # recently_active_only stays off by default: the workflows audience preview shares this + # function, and its count must match the unwindowed send enumeration in batch_audience.py. with unevaluable_filters_as_validation_errors(): cleaned_filter = replace_proxy_properties(team, feature_flag_condition) if group_type_index is not None: return _get_group_blast_radius(team, cleaned_filter, group_type_index) - else: - return _get_person_blast_radius(team, cleaned_filter) + if recently_active_only: + return _get_person_blast_radius_recently_active(team, cleaned_filter) + return _get_person_blast_radius(team, cleaned_filter) def get_user_blast_radius_persons( @@ -154,13 +159,15 @@ def get_user_blast_radius_persons( return _get_person_blast_radius_persons(team, cleaned_filter, cursor=cursor) -def _recently_active_cutoff() -> datetime: - """Start of the flag-sizing activity window, computed in Python so it respects freeze_time in - tests and does not depend on ClickHouse server time.""" - return timezone.now() - timedelta(days=RECENTLY_ACTIVE_DAYS) +def _recently_active_window() -> tuple[datetime, datetime]: + """Bounds of the flag-sizing activity window, computed in Python so they respect freeze_time in + tests and do not depend on ClickHouse server time. The upper bound allows a day of clock skew + but stops a far-future event timestamp from keeping a person "active" for years.""" + now = timezone.now() + return now - timedelta(days=RECENTLY_ACTIVE_DAYS), now + timedelta(days=1) -def recently_active_persons_count(team: Team, cutoff: Optional[datetime] = None) -> int: +def recently_active_persons_count(team: Team, window: Optional[tuple[datetime, datetime]] = None) -> int: """Count distinct persons active in the last RECENTLY_ACTIVE_DAYS days. This is the flag-sizing denominator. It replaces an all-time person count so anonymous churn @@ -168,16 +175,15 @@ def recently_active_persons_count(team: Team, cutoff: Optional[datetime] = None) estimate, which matches the "~" the sizing panel already renders. """ - if cutoff is None: - cutoff = _recently_active_cutoff() + cutoff, upper = window if window is not None else _recently_active_window() query = parse_select( - "SELECT uniq(person_id) FROM events WHERE timestamp >= {cutoff}", - placeholders={"cutoff": ast.Constant(value=cutoff)}, + "SELECT uniq(person_id) FROM events WHERE timestamp >= {cutoff} AND timestamp < {upper}", + placeholders={"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)}, ) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) - response = execute_hogql_query(query=query, team=team) + response = execute_hogql_query(query=query, team=team, workload=Workload.OFFLINE) return response.results[0][0] if response.results else 0 @@ -201,18 +207,19 @@ def _matched_persons_query(team: Team, filter: Filter): ) -def _get_person_blast_radius(team: Team, filter: Filter) -> BlastRadiusResult: +def _get_person_blast_radius_recently_active(team: Team, filter: Filter) -> BlastRadiusResult: """Calculate blast radius for person-based feature flags using HogQL. Both counts are of persons active in the same recent window, so the matched count is never a different population from the total it is shown against. """ - cutoff = _recently_active_cutoff() + window = _recently_active_window() + cutoff, upper = window properties = filter.property_groups.flat if len(properties) == 0: # No filters means every recently active person is affected. - total_users = recently_active_persons_count(team, cutoff) + total_users = recently_active_persons_count(team, window) return BlastRadiusResult(affected=total_users, total=total_users) # One pass over the recent event window yields both the active total and the matched subset, so @@ -234,15 +241,26 @@ def _get_person_blast_radius(team: Team, filter: Filter) -> BlastRadiusResult: ), ], select_from=ast.JoinExpr(table=ast.Field(chain=["events"])), - where=ast.CompareOperation( - op=ast.CompareOperationOp.GtEq, - left=ast.Field(chain=["timestamp"]), - right=ast.Constant(value=cutoff), + where=ast.And( + exprs=[ + ast.CompareOperation( + op=ast.CompareOperationOp.GtEq, + left=ast.Field(chain=["timestamp"]), + right=ast.Constant(value=cutoff), + ), + ast.CompareOperation( + op=ast.CompareOperationOp.Lt, + left=ast.Field(chain=["timestamp"]), + right=ast.Constant(value=upper), + ), + ] ), ) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) - response = execute_hogql_query(query=query, team=team) + # OFFLINE for the same reason as _get_group_blast_radius below: a 60-day events scan is too + # heavy for the pool that serves interactive analytics. + response = execute_hogql_query(query=query, team=team, workload=Workload.OFFLINE) total_users, affected = (response.results[0][0], response.results[0][1]) if response.results else (0, 0) # affected is a strict subset of total_users, but both are uniq() estimates, so clamp to keep @@ -250,6 +268,38 @@ def _get_person_blast_radius(team: Team, filter: Filter) -> BlastRadiusResult: return BlastRadiusResult(affected=min(affected, total_users), total=total_users) +def _get_person_blast_radius(team: Team, filter: Filter) -> BlastRadiusResult: + """Calculate all-time blast radius for person-based feature flags using HogQL.""" + + properties = filter.property_groups.flat + + if len(properties) == 0: + # No filters means all persons are affected + total_users = team.persons_seen_so_far + return BlastRadiusResult(affected=total_users, total=total_users) + + # Build the SELECT query - property_to_expr handles all properties including cohorts + select_query = _build_person_query(team, filter, return_count=True) + + # Execute the query + tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) + # Build the team's HogQL database once and share it between the two counts below. + # Each execute_hogql_query call would otherwise build its own, and the build cost + # scales with the team's warehouse size. + database = Database.create_for(team=team) + response = execute_hogql_query( + query=select_query, + team=team, + context=HogQLContext(team_id=team.pk, database=database), + ) + + total_count = response.results[0][0] if response.results else 0 + total_users = team.count_persons_seen_so_far(database=database) + blast_radius = min(total_count, total_users) + + return BlastRadiusResult(affected=blast_radius, total=total_users) + + def _build_person_query(team: Team, filter: Filter, return_count: bool = True, cursor: Optional[str] = None): """Build HogQL AST query to count or select distinct persons matching filters.""" From f36c7bd0af1eccc3b4a3f13a95754d082a8885c1 Mon Sep 17 00:00:00 2001 From: James Gill Date: Tue, 8 Sep 2026 08:26:48 +0100 Subject: [PATCH 07/12] fix(flags): cache the blast-radius active count and fail loud on timeout The empty-properties count depends only on the team, and the flag editor asks for it once per condition group on every mount, so a flag with five plain rollout conditions ran five identical 60-day event scans. Cache it on team.pk for 5 minutes, following flag_analytics. Both scans now throw on timeout, so neither returns a partial count as a real audience number. Also from review: the combined query moves to parse_select with the matched-persons subquery as a placeholder, which generates identical SQL; the count helper is private and computes its own window; the fixture helper is keyword-only; the window-edge test uses fixed 59 and 61 day offsets, so a typo in RECENTLY_ACTIVE_DAYS fails it. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/scenes/feature-flags/constants.tsx | 5 +- .../test/__snapshots__/test_feature_flag.ambr | 6 ++ .../backend/api/test/test_feature_flag.py | 76 ++++++++++----- .../backend/user_blast_radius.py | 94 +++++++++---------- 4 files changed, 107 insertions(+), 74 deletions(-) diff --git a/frontend/src/scenes/feature-flags/constants.tsx b/frontend/src/scenes/feature-flags/constants.tsx index 0af45bf8ab71..53f75e7bec6b 100644 --- a/frontend/src/scenes/feature-flags/constants.tsx +++ b/frontend/src/scenes/feature-flags/constants.tsx @@ -14,10 +14,7 @@ export const MATCHING_ESTIMATE_TOOLTIP = ( multiple profiles -
- Estimated from persons active in the last 60 days, so people who have not been seen recently are not - counted. -
+
Estimated from persons active in the last 60 days.
Conditions that filter on properties or cohorts count{' '} diff --git a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr index 632adc123860..de03d657f2c6 100644 --- a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr +++ b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr @@ -101,6 +101,7 @@ WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, + timeout_overflow_mode='throw', allow_experimental_object_type=1, max_ast_elements=4000000, max_expanded_ast_elements=4000000, @@ -362,6 +363,7 @@ WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, + timeout_overflow_mode='throw', allow_experimental_object_type=1, max_ast_elements=4000000, max_expanded_ast_elements=4000000, @@ -417,6 +419,7 @@ WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, + timeout_overflow_mode='throw', allow_experimental_object_type=1, max_ast_elements=4000000, max_expanded_ast_elements=4000000, @@ -478,6 +481,7 @@ WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, + timeout_overflow_mode='throw', allow_experimental_object_type=1, max_ast_elements=4000000, max_expanded_ast_elements=4000000, @@ -517,6 +521,7 @@ WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, + timeout_overflow_mode='throw', allow_experimental_object_type=1, max_ast_elements=4000000, max_expanded_ast_elements=4000000, @@ -572,6 +577,7 @@ WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, + timeout_overflow_mode='throw', allow_experimental_object_type=1, max_ast_elements=4000000, max_expanded_ast_elements=4000000, diff --git a/products/feature_flags/backend/api/test/test_feature_flag.py b/products/feature_flags/backend/api/test/test_feature_flag.py index b7d44ed8e94b..aa865d6efe47 100644 --- a/products/feature_flags/backend/api/test/test_feature_flag.py +++ b/products/feature_flags/backend/api/test/test_feature_flag.py @@ -9359,15 +9359,11 @@ def test_insert_failure_is_recorded_as_failure_not_success(self, mock_batch_eval self.assertEqual(history.error_code, CohortErrorCode.UNKNOWN) -def _create_active_person(*args, **kwargs): - # Flag sizing counts persons active in the recent window, so every person a sizing test relies - # on needs a recent event. Wrap person creation to emit one $pageview per person. - person = _create_person(*args, **kwargs) - team = kwargs.get("team") - team_id = kwargs.get("team_id") or (team.pk if team else None) - distinct_ids = kwargs.get("distinct_ids") or [] - if distinct_ids: - _create_event(team_id=team_id, event="$pageview", distinct_id=distinct_ids[0]) +def _create_active_person(*, team_id: int, distinct_ids: list[str], **kwargs): + # Blast radius counts persons active in the recent window, so every person a blast-radius test + # relies on needs a recent event. Wrap person creation to emit one $pageview per person. + person = _create_person(team_id=team_id, distinct_ids=distinct_ids, **kwargs) + _create_event(team_id=team_id, event="$pageview", distinct_id=distinct_ids[0]) return person @@ -9407,8 +9403,7 @@ def test_user_blast_radius(self): def test_persons_seen_so_far_ignores_team_v2_argmax_modifier(self): team = Team.objects.create(organization=self.organization, modifiers={"personsArgMaxVersion": "v2"}) for i in range(3): - _create_active_person(team_id=team.pk, distinct_ids=[f"seen_person_{i}"], properties={"group": f"{i}"}) - flush_persons_and_events() + _create_person(team_id=team.pk, distinct_ids=[f"seen_person_{i}"], properties={"group": f"{i}"}) with self.capture_select_queries() as queries: assert team.persons_seen_so_far == 3 @@ -9420,9 +9415,8 @@ def test_persons_seen_so_far_ignores_team_v2_argmax_modifier(self): # the assertion would always pass. assert "in(tuple(person.id, person.version)" not in query - # The filtered path builds one database and shares it across its numerator and denominator - # counts. The matched count still reads the persons table, so every query it emits must keep - # the pinned v1 person shape — otherwise the v2 modifier truncates the persons it matches. + # The filtered path builds one database and shares it between its two counts. + # The denominator must keep the pinned v1 shape even then. with patch.object(Database, "create_for", wraps=Database.create_for) as create_for_spy: with self.capture_select_queries() as queries: result = get_user_blast_radius( @@ -9432,17 +9426,33 @@ def test_persons_seen_so_far_ignores_team_v2_argmax_modifier(self): assert (result.affected, result.total) == (1, 3) assert create_for_spy.call_count == 1 - assert queries - for query in queries: + denominator_queries = [query for query in queries if "count(DISTINCT" not in query] + assert denominator_queries + for query in denominator_queries: assert "in(tuple(person.id, person.version)" not in query def test_user_blast_radius_excludes_inactive_matching_persons(self): # A person matching the filter but with no recent activity is not reachable audience, so it # must not count toward "affected" — otherwise inactive matches inflate the sized audience, # which is the bug this fix guards against. - _create_active_person(team_id=self.team.pk, distinct_ids=["active-match-1"], properties={"group": "match"}) - _create_active_person(team_id=self.team.pk, distinct_ids=["active-match-2"], properties={"group": "match"}) - # Matches the filter but emits no event in the window, so it drops out of the count. + _create_active_person(team_id=self.team.pk, distinct_ids=["active-match"], properties={"group": "match"}) + # The two window edges use fixed day offsets rather than RECENTLY_ACTIVE_DAYS +/- 1, so a + # typo in the constant moves the boundary past a fixture instead of moving both together. + _create_person(team_id=self.team.pk, distinct_ids=["edge-match"], properties={"group": "match"}) + _create_event( + team_id=self.team.pk, + event="$pageview", + distinct_id="edge-match", + timestamp=now() - timedelta(days=59), + ) + _create_person(team_id=self.team.pk, distinct_ids=["stale-match"], properties={"group": "match"}) + _create_event( + team_id=self.team.pk, + event="$pageview", + distinct_id="stale-match", + timestamp=now() - timedelta(days=61), + ) + # Matches the filter but emits no event at all, so it drops out of the count. _create_person(team_id=self.team.pk, distinct_ids=["inactive-match"], properties={"group": "match"}) # Active but does not match, so it keeps the total above the matched count. _create_active_person(team_id=self.team.pk, distinct_ids=["active-nonmatch"], properties={"group": "other"}) @@ -9466,15 +9476,37 @@ def test_user_blast_radius_excludes_inactive_matching_persons(self): ) self.assertEqual(response.status_code, status.HTTP_200_OK) - # Only the two active matches count; the inactive and far-future matches are excluded, and - # the active non-match keeps the total at three. + # Only the two in-window matches count; the stale, inactive, and far-future matches are + # excluded, and the active non-match keeps the total at three. self.assertLessEqual({"affected": 2, "total": 3}.items(), response.json().items()) # The window is an opt-in for the flags endpoint only. The workflows audience preview calls # this function without it and must keep counting every matching person, because the batch # send it previews enumerates persons with no activity window. unwindowed = get_user_blast_radius(self.team, condition) - self.assertEqual((unwindowed.affected, unwindowed.total), (4, 5)) + self.assertEqual((unwindowed.affected, unwindowed.total), (5, 6)) + + def test_user_blast_radius_caches_the_unfiltered_active_count(self): + # The flag editor sizes one condition group per mount, so repeat requests inside the TTL + # must not re-scan events each time. + _create_active_person(team_id=self.team.pk, distinct_ids=["active"], properties={"group": "match"}) + flush_persons_and_events() + + with self.capture_select_queries() as queries: + responses = [ + self.client.post( + f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", + {"condition": {"properties": [], "rollout_percentage": 50}}, + ) + for _ in range(2) + ] + + for response in responses: + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertLessEqual({"affected": 1, "total": 1}.items(), response.json().items()) + + events_scans = [query for query in queries if "uniq(" in query] + self.assertEqual(len(events_scans), 1) @parameterized.expand( [ diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index 8ead9e3d4364..a543b6f6b316 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -3,6 +3,7 @@ from datetime import datetime, timedelta from typing import Optional +from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist from django.utils import timezone @@ -11,6 +12,7 @@ from posthog.schema import PropertyOperator from posthog.hogql import ast +from posthog.hogql.constants import HogQLGlobalSettings from posthog.hogql.context import HogQLContext from posthog.hogql.database.database import Database from posthog.hogql.errors import ( @@ -39,10 +41,14 @@ class BlastRadiusResult: total: int -# Window for the flag-sizing denominator. An all-time person count is inflated by anonymous, +# Window for the blast-radius denominator. An all-time person count is inflated by anonymous, # one-shot persons that never return; a recent-activity window drops them and is defensible. RECENTLY_ACTIVE_DAYS = 60 +# The recently-active count depends on the team alone, and the flag editor asks for it once per +# empty-properties condition group on every mount, so a short TTL collapses those into one scan. +_RECENTLY_ACTIVE_COUNT_CACHE_TTL = 300 + # ClickHouse codes for "this literal can't be parsed as the column's type": 6 CANNOT_PARSE_TEXT, # 72 CANNOT_PARSE_NUMBER — e.g. a numeric operator (gt/lt) against a null or non-numeric filter @@ -160,37 +166,47 @@ def get_user_blast_radius_persons( def _recently_active_window() -> tuple[datetime, datetime]: - """Bounds of the flag-sizing activity window, computed in Python so they respect freeze_time in + """Bounds of the blast-radius activity window, computed in Python so they respect freeze_time in tests and do not depend on ClickHouse server time. The upper bound allows a day of clock skew but stops a far-future event timestamp from keeping a person "active" for years.""" now = timezone.now() return now - timedelta(days=RECENTLY_ACTIVE_DAYS), now + timedelta(days=1) -def recently_active_persons_count(team: Team, window: Optional[tuple[datetime, datetime]] = None) -> int: +def _recently_active_persons_count(team: Team) -> int: """Count distinct persons active in the last RECENTLY_ACTIVE_DAYS days. - This is the flag-sizing denominator. It replaces an all-time person count so anonymous churn + This is the blast-radius denominator. It replaces an all-time person count so anonymous churn does not inflate the audience shown to a flag author (see RECENTLY_ACTIVE_DAYS). `uniq` is an - estimate, which matches the "~" the sizing panel already renders. + estimate, which matches the "~" the release conditions panel already renders. """ + cache_key = f"flag_blast_radius:recently_active_persons:{team.pk}" + cached = cache.get(cache_key) + if cached is not None: + return cached - cutoff, upper = window if window is not None else _recently_active_window() - + cutoff, upper = _recently_active_window() query = parse_select( "SELECT uniq(person_id) FROM events WHERE timestamp >= {cutoff} AND timestamp < {upper}", placeholders={"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)}, ) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) - response = execute_hogql_query(query=query, team=team, workload=Workload.OFFLINE) + response = execute_hogql_query( + query=query, + team=team, + workload=Workload.OFFLINE, + settings=HogQLGlobalSettings(timeout_overflow_mode="throw"), + ) - return response.results[0][0] if response.results else 0 + count = response.results[0][0] if response.results else 0 + cache.set(cache_key, count, timeout=_RECENTLY_ACTIVE_COUNT_CACHE_TTL) + return count -def _matched_persons_query(team: Team, filter: Filter): +def _matched_persons_query(team: Team, filter: Filter) -> ast.SelectQuery: """Subquery of the person ids matching the condition. Reuses property_to_expr in person scope, - so it resolves cohorts, static cohorts, and group properties exactly like the person listing.""" + so it resolves cohorts, static cohorts, and group properties exactly like _build_person_query.""" return ast.SelectQuery( select=[ast.Field(chain=["persons", "id"])], select_from=ast.JoinExpr(table=ast.Field(chain=["persons"])), @@ -208,59 +224,41 @@ def _matched_persons_query(team: Team, filter: Filter): def _get_person_blast_radius_recently_active(team: Team, filter: Filter) -> BlastRadiusResult: - """Calculate blast radius for person-based feature flags using HogQL. + """Calculate blast radius for person-based feature flags over the recent activity window. Both counts are of persons active in the same recent window, so the matched count is never a different population from the total it is shown against. """ - window = _recently_active_window() - cutoff, upper = window properties = filter.property_groups.flat if len(properties) == 0: # No filters means every recently active person is affected. - total_users = recently_active_persons_count(team, window) + total_users = _recently_active_persons_count(team) return BlastRadiusResult(affected=total_users, total=total_users) + cutoff, upper = _recently_active_window() # One pass over the recent event window yields both the active total and the matched subset, so # the denominator is never a second scan and both numbers come from the same rows. - matched_persons = _matched_persons_query(team, filter) - query = ast.SelectQuery( - select=[ - ast.Call(name="uniq", args=[ast.Field(chain=["person_id"])]), - ast.Call( - name="uniqIf", - args=[ - ast.Field(chain=["person_id"]), - ast.CompareOperation( - op=ast.CompareOperationOp.In, - left=ast.Field(chain=["person_id"]), - right=matched_persons, - ), - ], - ), - ], - select_from=ast.JoinExpr(table=ast.Field(chain=["events"])), - where=ast.And( - exprs=[ - ast.CompareOperation( - op=ast.CompareOperationOp.GtEq, - left=ast.Field(chain=["timestamp"]), - right=ast.Constant(value=cutoff), - ), - ast.CompareOperation( - op=ast.CompareOperationOp.Lt, - left=ast.Field(chain=["timestamp"]), - right=ast.Constant(value=upper), - ), - ] - ), + query = parse_select( + "SELECT uniq(person_id), uniqIf(person_id, person_id IN {matched}) " + "FROM events WHERE timestamp >= {cutoff} AND timestamp < {upper}", + placeholders={ + "matched": _matched_persons_query(team, filter), + "cutoff": ast.Constant(value=cutoff), + "upper": ast.Constant(value=upper), + }, ) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) # OFFLINE for the same reason as _get_group_blast_radius below: a 60-day events scan is too - # heavy for the pool that serves interactive analytics. - response = execute_hogql_query(query=query, team=team, workload=Workload.OFFLINE) + # heavy for the pool that serves interactive analytics. A partial result on timeout would be a + # confidently wrong audience number, so overflow throws instead. + response = execute_hogql_query( + query=query, + team=team, + workload=Workload.OFFLINE, + settings=HogQLGlobalSettings(timeout_overflow_mode="throw"), + ) total_users, affected = (response.results[0][0], response.results[0][1]) if response.results else (0, 0) # affected is a strict subset of total_users, but both are uniq() estimates, so clamp to keep From 43c7895b9ed4fde2c3685d38f78a78d034bf13a0 Mon Sep 17 00:00:00 2001 From: James Gill Date: Tue, 8 Sep 2026 08:37:44 +0100 Subject: [PATCH 08/12] fix(flags): keep personless events out of the blast-radius counts An event captured with $process_person_profile: false carries a synthetic person_id derived from its distinct id, and no persons row is ever written for it. uniq(person_id) counted every one of those, while the matched subquery selects from persons and never can, so the denominator held a population the numerator could not reach. It also moved the number the wrong way. On a project using posthog-js's identified_only default, the old persons-based denominator never saw anonymous traffic and the new events-based one counted all of it, so the total rose for the projects this fix is meant to help. Both queries now filter person_mode != 'propertyless'. The tooltip says the estimate covers identified users, and keeps the note that anonymous visitors can still match the flag. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/scenes/feature-flags/constants.tsx | 6 ++-- .../test/__snapshots__/test_feature_flag.ambr | 12 ++++---- .../backend/api/test/test_feature_flag.py | 30 +++++++++++++++++++ .../backend/user_blast_radius.py | 11 +++++-- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/frontend/src/scenes/feature-flags/constants.tsx b/frontend/src/scenes/feature-flags/constants.tsx index 53f75e7bec6b..b6683cb8be76 100644 --- a/frontend/src/scenes/feature-flags/constants.tsx +++ b/frontend/src/scenes/feature-flags/constants.tsx @@ -14,13 +14,13 @@ export const MATCHING_ESTIMATE_TOOLTIP = ( multiple profiles
-
Estimated from persons active in the last 60 days.
- Conditions that filter on properties or cohorts count{' '} + Estimated from{' '} identified users {' '} - only. Anonymous visitors can still match this flag, so the actual number may be higher. + active in the last 60 days.
+
Anonymous visitors can still match this flag, so the actual number may be higher.
) diff --git a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr index de03d657f2c6..893692a3bd91 100644 --- a/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr +++ b/products/feature_flags/backend/api/test/__snapshots__/test_feature_flag.ambr @@ -98,7 +98,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC')), notEquals(events.person_mode, 'propertyless')) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, timeout_overflow_mode='throw', @@ -360,7 +360,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC')), notEquals(events.person_mode, 'propertyless')) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, timeout_overflow_mode='throw', @@ -416,7 +416,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC')), notEquals(events.person_mode, 'propertyless')) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, timeout_overflow_mode='throw', @@ -478,7 +478,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC')), notEquals(events.person_mode, 'propertyless')) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, timeout_overflow_mode='throw', @@ -518,7 +518,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC')), notEquals(events.person_mode, 'propertyless')) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, timeout_overflow_mode='throw', @@ -574,7 +574,7 @@ WHERE equals(person_distinct_id_overrides.team_id, 99999) GROUP BY person_distinct_id_overrides.distinct_id HAVING equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) - WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC'))) + WHERE and(equals(events.team_id, 99999), greaterOrEquals(events.timestamp, toDateTime64('explicit_redacted_timestamp', 6, 'UTC')), less(events.timestamp, toDateTime64('today', 6, 'UTC')), notEquals(events.person_mode, 'propertyless')) LIMIT 100 SETTINGS readonly=2, max_execution_time=60, timeout_overflow_mode='throw', diff --git a/products/feature_flags/backend/api/test/test_feature_flag.py b/products/feature_flags/backend/api/test/test_feature_flag.py index aa865d6efe47..cc19dc999ac9 100644 --- a/products/feature_flags/backend/api/test/test_feature_flag.py +++ b/products/feature_flags/backend/api/test/test_feature_flag.py @@ -1,4 +1,5 @@ import json +import uuid from datetime import UTC, datetime, timedelta from typing import Any, Optional, cast @@ -9486,6 +9487,35 @@ def test_user_blast_radius_excludes_inactive_matching_persons(self): unwindowed = get_user_blast_radius(self.team, condition) self.assertEqual((unwindowed.affected, unwindowed.total), (5, 6)) + def test_user_blast_radius_excludes_personless_events(self): + # An event captured with $process_person_profile: false carries a synthetic person_id and no + # persons row, so counting it would put anonymous traffic in a denominator the matched + # subquery can never return. + _create_active_person(team_id=self.team.pk, distinct_ids=["identified"], properties={"group": "match"}) + _create_event( + team_id=self.team.pk, + event="$pageview", + distinct_id="anonymous-visitor", + person_id=uuid.uuid4(), + person_mode="propertyless", + ) + flush_persons_and_events() + + for condition in ( + {"properties": [], "rollout_percentage": 100}, + { + "properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}], + "rollout_percentage": 100, + }, + ): + response = self.client.post( + f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", + {"condition": condition}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertLessEqual({"affected": 1, "total": 1}.items(), response.json().items()) + def test_user_blast_radius_caches_the_unfiltered_active_count(self): # The flag editor sizes one condition group per mount, so repeat requests inside the TTL # must not re-scan events each time. diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index a543b6f6b316..d125b08d8ab6 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -49,6 +49,12 @@ class BlastRadiusResult: # empty-properties condition group on every mount, so a short TTL collapses those into one scan. _RECENTLY_ACTIVE_COUNT_CACHE_TTL = 300 +# A personless event carries a synthetic person_id derived from its distinct id, and no persons row +# is ever written for it. Counting it would put anonymous traffic in a denominator that the +# matched-persons subquery can never return, and would raise the total for projects on posthog-js's +# identified_only default, which is the opposite of what the window is for. +_IDENTIFIED_PERSONS_ONLY = "person_mode != 'propertyless'" + # ClickHouse codes for "this literal can't be parsed as the column's type": 6 CANNOT_PARSE_TEXT, # 72 CANNOT_PARSE_NUMBER — e.g. a numeric operator (gt/lt) against a null or non-numeric filter @@ -187,7 +193,8 @@ def _recently_active_persons_count(team: Team) -> int: cutoff, upper = _recently_active_window() query = parse_select( - "SELECT uniq(person_id) FROM events WHERE timestamp >= {cutoff} AND timestamp < {upper}", + "SELECT uniq(person_id) FROM events " + f"WHERE timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}", placeholders={"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)}, ) @@ -241,7 +248,7 @@ def _get_person_blast_radius_recently_active(team: Team, filter: Filter) -> Blas # the denominator is never a second scan and both numbers come from the same rows. query = parse_select( "SELECT uniq(person_id), uniqIf(person_id, person_id IN {matched}) " - "FROM events WHERE timestamp >= {cutoff} AND timestamp < {upper}", + f"FROM events WHERE timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}", placeholders={ "matched": _matched_persons_query(team, filter), "cutoff": ast.Constant(value=cutoff), From 0497dd23d30803148c553326224c3601dc7ea9d0 Mon Sep 17 00:00:00 2001 From: James Gill Date: Tue, 8 Sep 2026 09:35:32 +0100 Subject: [PATCH 09/12] fix(flags): gate the activity window behind a kill switch and throttle sizing The sizing endpoint held a Django worker for up to the 60 second query timeout, and the release editor fires one request per condition group with no dedup. The window replaced a persons-table count with a 60-day events scan, so worker hold time per page view went up while concurrency stayed the same. The endpoint now carries the ClickHouse throttles its sibling test_evaluation already uses. recently_active_sizing_enabled gates the window on flags-size-by-active-persons, evaluated locally so a sizing request never waits on a flag fetch. Off restores the all-time count, which is the path the workflows audience preview already takes, so a disable costs accuracy and never correctness. TestBlastRadius turns the gate on for the class, because it evaluates to False without a flag client and the endpoint tests would otherwise exercise the all-time path. Co-Authored-By: Claude Opus 5 (1M context) --- .../feature_flags/backend/api/feature_flag.py | 16 +++++++-- .../backend/api/test/test_feature_flag.py | 34 +++++++++++++++++++ .../backend/user_blast_radius.py | 34 +++++++++++++++++++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index 249c7acd21f8..1cc82f55ffc6 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -130,7 +130,7 @@ teams_linking_flag_in_project, ) from products.feature_flags.backend.types import PropertyFilterType -from products.feature_flags.backend.user_blast_radius import get_user_blast_radius +from products.feature_flags.backend.user_blast_radius import get_user_blast_radius, recently_active_sizing_enabled from products.feature_flags.backend.version_history import ( VersionHistoryIncomplete, VersionNotFound, @@ -4552,7 +4552,12 @@ def evaluation_reasons(self, request: request.Request, **kwargs): request=UserBlastRadiusRequestSerializer, responses={200: UserBlastRadiusResponseSerializer}, ) - @action(methods=["POST"], detail=False, required_scopes=["feature_flag:read"]) + @action( + methods=["POST"], + detail=False, + required_scopes=["feature_flag:read"], + throttle_classes=[ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle], + ) def user_blast_radius(self, request: request.Request, **kwargs): if "condition" not in request.data: raise exceptions.ValidationError("Missing condition for which to get blast radius") @@ -4560,7 +4565,12 @@ def user_blast_radius(self, request: request.Request, **kwargs): condition = request.data.get("condition") or {} group_type_index = request.data.get("group_type_index", None) - result = get_user_blast_radius(self.team, condition, group_type_index, recently_active_only=True) + result = get_user_blast_radius( + self.team, + condition, + group_type_index, + recently_active_only=recently_active_sizing_enabled(self.team), + ) return Response({"affected": result.affected, "total": result.total}) diff --git a/products/feature_flags/backend/api/test/test_feature_flag.py b/products/feature_flags/backend/api/test/test_feature_flag.py index cc19dc999ac9..bd90951477eb 100644 --- a/products/feature_flags/backend/api/test/test_feature_flag.py +++ b/products/feature_flags/backend/api/test/test_feature_flag.py @@ -9369,6 +9369,17 @@ def _create_active_person(*, team_id: int, distinct_ids: list[str], **kwargs): class TestBlastRadius(ClickhouseTestMixin, APIBaseTest): + def setUp(self): + super().setUp() + # The activity window sits behind a kill switch that evaluates to False without a flag + # client, so the endpoint tests below would silently exercise the all-time path instead. + gate = patch( + "products.feature_flags.backend.api.feature_flag.recently_active_sizing_enabled", + return_value=True, + ) + gate.start() + self.addCleanup(gate.stop) + @snapshot_clickhouse_queries def test_user_blast_radius(self): for i in range(10): @@ -9487,6 +9498,29 @@ def test_user_blast_radius_excludes_inactive_matching_persons(self): unwindowed = get_user_blast_radius(self.team, condition) self.assertEqual((unwindowed.affected, unwindowed.total), (5, 6)) + def test_user_blast_radius_falls_back_to_the_all_time_count_when_gated_off(self): + # The kill switch has to restore the count the window replaced, so a disable gives back the + # inactive matches rather than leaving the window half applied. + _create_active_person(team_id=self.team.pk, distinct_ids=["active-match"], properties={"group": "match"}) + _create_person(team_id=self.team.pk, distinct_ids=["inactive-match"], properties={"group": "match"}) + flush_persons_and_events() + + condition = { + "properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}], + "rollout_percentage": 100, + } + with patch( + "products.feature_flags.backend.api.feature_flag.recently_active_sizing_enabled", + return_value=False, + ): + response = self.client.post( + f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", + {"condition": condition}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertLessEqual({"affected": 2, "total": 2}.items(), response.json().items()) + def test_user_blast_radius_excludes_personless_events(self): # An event captured with $process_person_profile: false carries a synthetic person_id and no # persons row, so counting it would put anonymous traffic in a denominator the matched diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index d125b08d8ab6..90028d562b28 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -7,6 +7,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.utils import timezone +import structlog from rest_framework.exceptions import ValidationError from posthog.schema import PropertyOperator @@ -30,10 +31,13 @@ from posthog.models.filters import Filter from posthog.models.property import GroupTypeIndex, Property, PropertyGroup, PropertyValidationError from posthog.models.team.team import Team +from posthog.ph_client import feature_enabled_or_false from posthog.queries.base import relative_date_parse_for_feature_flag_matching from products.cohorts.backend.models.cohort import Cohort +logger = structlog.get_logger(__name__) + @frozen class BlastRadiusResult: @@ -45,6 +49,8 @@ class BlastRadiusResult: # one-shot persons that never return; a recent-activity window drops them and is defensible. RECENTLY_ACTIVE_DAYS = 60 +RECENTLY_ACTIVE_SIZING_FLAG = "flags-size-by-active-persons" + # The recently-active count depends on the team alone, and the flag editor asks for it once per # empty-properties condition group on every mount, so a short TTL collapses those into one scan. _RECENTLY_ACTIVE_COUNT_CACHE_TTL = 300 @@ -135,6 +141,34 @@ def replace_proxy_properties(team: Team, feature_flag_condition: dict): raise ValidationError({"filters": str(e) or "These filters cannot be evaluated."}) from e +def recently_active_sizing_enabled(team: Team) -> bool: + """Kill switch for the activity window on the flags sizing endpoint. + + False restores the all-time count the window replaced, which is still the behavior the + workflows audience preview depends on, so a disable costs accuracy and never correctness. + Evaluation is local-only, so a sizing request never waits on a flag fetch. + """ + try: + return feature_enabled_or_false( + RECENTLY_ACTIVE_SIZING_FLAG, + f"team-{team.pk}", + groups={"project": str(team.pk)}, + group_properties={"project": {"id": str(team.pk)}}, + only_evaluate_locally=True, + send_feature_flag_events=False, + ) + except Exception: + # Log so a fleet-wide silent disable shows up in Sentry rather than as a number that + # quietly went back to its old value. + logger.warning( + "flag_blast_radius_sizing_flag_evaluation_failed", + team_id=team.pk, + flag=RECENTLY_ACTIVE_SIZING_FLAG, + exc_info=True, + ) + return False + + def get_user_blast_radius( team: Team, feature_flag_condition: dict, From 77cbb4eb35759900cb8cca7614d0b6324ba7d304 Mon Sep 17 00:00:00 2001 From: James Gill Date: Fri, 11 Sep 2026 10:42:52 +0100 Subject: [PATCH 10/12] fix(flags): throttle session callers and report the sizing basis From review on #92668. - Replace the ClickHouse throttle pair on user_blast_radius with a team bucket. ClickHouse*RateThrottle extends PersonalApiKeyRateThrottle, which lets an authenticated request with no personal API key through before it counts the request. The release-conditions editor uses a session cookie, so it skipped the throttle. FlagSizing*RateThrottle extends _TeamBucketRateThrottle, which counts every auth method and buckets per project. - Add activity_window_days to the sizing response. The window is gated per project, so the tooltip and the help text now follow the basis the response reports. Neither states a window when the all-time path ran. - Build the activity-window predicate in one place, and the matching-persons predicate in one place. Flag sizing and the workflows preview both read the second one. - Call the generated featureFlagsUserBlastRadiusCreate and drop the handwritten UserBlastRadiusType. - Test the kill switch, the per-project cache key, and the tooltip guard for a condition that overrides an unset flag level. Co-Authored-By: Claude Opus 5 (1M context) --- ...leaseConditions.matchingUsersLink.test.tsx | 30 ++++++- .../FeatureFlagReleaseConditions.tsx | 13 ++- ...eatureFlagReleaseConditionsCollapsible.tsx | 18 +++- .../src/scenes/feature-flags/constants.tsx | 8 +- .../featureFlagReleaseConditionsLogic.test.ts | 9 +- .../featureFlagReleaseConditionsLogic.ts | 28 ++++-- frontend/src/types.ts | 5 -- posthog/rate_limit.py | 17 ++++ .../feature_flags/backend/api/feature_flag.py | 28 ++++-- .../backend/api/test/test_feature_flag.py | 65 +++++++++++++- .../backend/user_blast_radius.py | 87 ++++++++++--------- .../frontend/generated/api.schemas.ts | 9 +- services/mcp/src/api/generated.ts | 9 +- 13 files changed, 243 insertions(+), 83 deletions(-) diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.matchingUsersLink.test.tsx b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.matchingUsersLink.test.tsx index 8a61d6a3b5da..47f0a887a878 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.matchingUsersLink.test.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.matchingUsersLink.test.tsx @@ -20,7 +20,10 @@ jest.mock('lib/components/AutoSizer', () => ({ // `aggregation_group_type_index` is set at flag level only, so the condition has to inherit it. That // resolution happens at the call site and is invisible to `matchingActorsUrl`'s own tests. -function buildFilters(aggregationGroupTypeIndex?: number): FeatureFlagType['filters'] { +function buildFilters( + aggregationGroupTypeIndex?: number, + conditionGroupTypeIndex?: number +): FeatureFlagType['filters'] { const group: FeatureFlagGroupType = { properties: [ { @@ -33,6 +36,7 @@ function buildFilters(aggregationGroupTypeIndex?: number): FeatureFlagType['filt rollout_percentage: 100, variant: null, sort_key: 'group-1', + aggregation_group_type_index: conditionGroupTypeIndex, } return { groups: [group], @@ -59,7 +63,10 @@ describe('feature flag release conditions matching users link', () => { }, post: { '/api/environments/:team/query': { results: [] }, - '/api/projects/:team/feature_flags/user_blast_radius': () => [200, { affected: 0, total: 2 }], + '/api/projects/:team/feature_flags/user_blast_radius': () => [ + 200, + { affected: 0, total: 2, activity_window_days: 60 }, + ], }, }) }) @@ -105,6 +112,25 @@ describe('feature flag release conditions matching users link', () => { ], ] as const + // The guard has to read the resolved index: reading the flag-level field instead puts the + // person tooltip beside a group count whenever a condition overrides an unset flag level. + const tooltipCases = [ + ['flag-level person targeting', buildFilters(), true], + ['condition overrides an unset flag level with a group', buildFilters(undefined, 0), false], + ['flag-level group targeting', buildFilters(0), false], + ] as const + + test.each(tooltipCases)('shows the estimate tooltip for %s', async (_name, filters, expectTooltip) => { + const { container, getByText } = render( + + + + ) + + await waitFor(() => expect(getByText(/Filters match:/)).toBeInTheDocument()) + expect(!!container.querySelector('[data-attr="matching-estimate-info"]')).toBe(expectTooltip) + }) + test.each(cases)( '%s links a condition to the matching actors once counts load', async (_name, component, linkText, expectedPath) => { diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx index ad7f4697f2cf..606302c4f161 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx @@ -47,7 +47,7 @@ import { import { FractionalRolloutWarning } from 'products/feature_flags/frontend/FractionalRolloutWarning' import { resolveAggregationGroupTypeIndex } from './aggregation' -import { EARLY_ACCESS_GROUP_TARGETING_DISABLED_REASON, MATCHING_ESTIMATE_TOOLTIP } from './constants' +import { EARLY_ACCESS_GROUP_TARGETING_DISABLED_REASON, matchingEstimateTooltip } from './constants' import { featureFlagLogic } from './featureFlagLogic' import { FeatureFlagReleaseConditionsLogicProps, @@ -142,6 +142,7 @@ export function FeatureFlagReleaseConditions({ properties, filterGroups, getDistinctIdName, + activityWindowDays, } = useValues(releaseConditionsLogic) const { @@ -500,8 +501,14 @@ export function FeatureFlagReleaseConditions({ Filters match: ~{pluralize(affected, singularName, pluralName)} {resolvedGroupTypeIndex == null && ( - - + + )} diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx index a86f72ee9b85..209931b341c0 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx @@ -74,7 +74,7 @@ import { INTENT_METADATA } from 'products/feature_flags/frontend/featureFlagTemp import { FractionalRolloutWarning } from 'products/feature_flags/frontend/FractionalRolloutWarning' import { resolveAggregationGroupTypeIndex } from './aggregation' -import { EARLY_ACCESS_GROUP_TARGETING_DISABLED_REASON, MATCHING_ESTIMATE_TOOLTIP } from './constants' +import { EARLY_ACCESS_GROUP_TARGETING_DISABLED_REASON, matchingEstimateTooltip } from './constants' import { EarlyExitIndicator } from './EarlyExitIndicator' import { FeatureFlagConditionDragHandle } from './FeatureFlagConditionDragHandle' import { FeatureFlagConditionWarning } from './FeatureFlagConditionWarning' @@ -345,6 +345,7 @@ interface ConditionProps { affectedCounts: Record totalCounts: Record blastRadiusErrors: Record + activityWindowDays: number | null calculateBlastRadiusForCondition: ( sortKey: string, properties: AnyPropertyFilter[] | undefined, @@ -423,6 +424,7 @@ const ConditionContent = ({ affectedCounts, totalCounts, blastRadiusErrors, + activityWindowDays, calculateBlastRadiusForCondition, aggregationTargetName, getDistinctIdName, @@ -742,10 +744,15 @@ const ConditionContent = ({ {resolvedGroupTypeIndex == null && ( - + )} @@ -887,6 +894,7 @@ export function FeatureFlagReleaseConditionsCollapsible({ affectedCounts, totalCounts, blastRadiusErrors, + activityWindowDays, aggregationTargetName, getDistinctIdName, getFlagKey, @@ -1290,6 +1298,7 @@ export function FeatureFlagReleaseConditionsCollapsible({ affectedCounts={affectedCounts} totalCounts={totalCounts} blastRadiusErrors={blastRadiusErrors} + activityWindowDays={activityWindowDays} calculateBlastRadiusForCondition={calculateBlastRadiusForCondition} aggregationTargetName={aggregationTargetName} getDistinctIdName={getDistinctIdName} @@ -1371,6 +1380,7 @@ export function FeatureFlagReleaseConditionsCollapsible({ affectedCounts={affectedCounts} totalCounts={totalCounts} blastRadiusErrors={blastRadiusErrors} + activityWindowDays={activityWindowDays} calculateBlastRadiusForCondition={calculateBlastRadiusForCondition} aggregationTargetName={aggregationTargetName} getDistinctIdName={getDistinctIdName} @@ -1426,6 +1436,7 @@ export function FeatureFlagReleaseConditionsCollapsible({ affectedCounts={affectedCounts} totalCounts={totalCounts} blastRadiusErrors={blastRadiusErrors} + activityWindowDays={activityWindowDays} calculateBlastRadiusForCondition={calculateBlastRadiusForCondition} aggregationTargetName={aggregationTargetName} getDistinctIdName={getDistinctIdName} @@ -1463,6 +1474,7 @@ export function FeatureFlagReleaseConditionsCollapsible({ affectedCounts={affectedCounts} totalCounts={totalCounts} blastRadiusErrors={blastRadiusErrors} + activityWindowDays={activityWindowDays} calculateBlastRadiusForCondition={calculateBlastRadiusForCondition} aggregationTargetName={aggregationTargetName} getDistinctIdName={getDistinctIdName} diff --git a/frontend/src/scenes/feature-flags/constants.tsx b/frontend/src/scenes/feature-flags/constants.tsx index b6683cb8be76..710e6263ac3f 100644 --- a/frontend/src/scenes/feature-flags/constants.tsx +++ b/frontend/src/scenes/feature-flags/constants.tsx @@ -6,7 +6,9 @@ export const COHORT_BEHAVIORAL_LIMITATIONS_URL = export const EARLY_ACCESS_GROUP_TARGETING_DISABLED_REASON = 'This flag is linked to an early access feature, so it can only target users. Remove the early access feature to target groups.' -export const MATCHING_ESTIMATE_TOOLTIP = ( +// The activity window is switched on per project, so the copy follows the basis the response +// reports. A null window means the estimate is all-time. +export const matchingEstimateTooltip = (activityWindowDays: number | null): JSX.Element => ( <>
A user may have{' '} @@ -18,8 +20,8 @@ export const MATCHING_ESTIMATE_TOOLTIP = ( Estimated from{' '} identified users - {' '} - active in the last 60 days. + + {activityWindowDays === null ? '.' : ` active in the last ${activityWindowDays} days.`}
Anonymous visitors can still match this flag, so the actual number may be higher.
diff --git a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts index 4cc7c056c9e1..6e156878f681 100644 --- a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts +++ b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts @@ -491,12 +491,14 @@ describe('the feature flag release conditions logic', () => { // Condition A has its own aggregation_group_type_index=1, should override flag-level 0 expect(createSpy).toHaveBeenCalledWith( expect.stringContaining('user_blast_radius'), - expect.objectContaining({ group_type_index: 1 }) + expect.objectContaining({ group_type_index: 1 }), + expect.anything() ) // Condition B has no condition-level override, should fall back to flag-level 0 expect(createSpy).toHaveBeenCalledWith( expect.stringContaining('user_blast_radius'), - expect.objectContaining({ group_type_index: 0 }) + expect.objectContaining({ group_type_index: 0 }), + expect.anything() ) } finally { createSpy.mockRestore() @@ -539,7 +541,8 @@ describe('the feature flag release conditions logic', () => { expect(createSpy).toHaveBeenCalledWith( expect.stringContaining('user_blast_radius'), - expect.objectContaining({ group_type_index: null }) + expect.objectContaining({ group_type_index: null }), + expect.anything() ) expect(createSpy).not.toHaveBeenCalledWith( expect.stringContaining('user_blast_radius'), diff --git a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts index e98e7982a5ef..1c8247a19442 100644 --- a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts +++ b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts @@ -34,9 +34,10 @@ import { MultivariateFlagVariant, PropertyFilterType, PropertyOperator, - UserBlastRadiusType, } from '~/types' +import { featureFlagsUserBlastRadiusCreate } from 'products/feature_flags/frontend/generated/api' + import type { SimpleOption } from '../../lib/components/TaxonomicFilter/types' import type { Noun } from '../../models/groupsModel' import type { GroupType } from '../../types' @@ -162,6 +163,7 @@ export interface featureFlagReleaseConditionsLogicValues { rollout_percentage: string | undefined variant: null }[] + activityWindowDays: number | null taxonomicGroupTypes: TaxonomicFilterGroupType[] taxonomicGroupTypesForCondition: (conditionGroupTypeIndex: number | null | undefined) => TaxonomicFilterGroupType[] totalCounts: Record @@ -219,6 +221,9 @@ export interface featureFlagReleaseConditionsLogicActions { count: number | undefined sortKey: string } + setActivityWindowDays: (activityWindowDays: number | null) => { + activityWindowDays: number | null + } setAggregationGroupTypeIndex: (value: number | null) => { value: number | null } @@ -386,6 +391,7 @@ export const featureFlagReleaseConditionsLogic = kea ({ sortKey }), + setActivityWindowDays: (activityWindowDays: number | null) => ({ activityWindowDays }), calculateBlastRadius: true, calculateBlastRadiusForCondition: ( sortKey: string, @@ -615,6 +621,14 @@ export const featureFlagReleaseConditionsLogic = kea activityWindowDays, + }, + ], // Tracks conditions whose blast-radius estimate failed, so the UI can distinguish a // genuine error from the still-loading (undefined) state instead of spinning forever. blastRadiusErrors: [ @@ -810,15 +824,13 @@ export const featureFlagReleaseConditionsLogic = kea } -export interface UserBlastRadiusType { - affected: number - total: number -} - export enum ScheduledChangeModels { FeatureFlag = 'FeatureFlag', } diff --git a/posthog/rate_limit.py b/posthog/rate_limit.py index 61044676e03c..5b30b7e077b8 100644 --- a/posthog/rate_limit.py +++ b/posthog/rate_limit.py @@ -625,6 +625,23 @@ class SessionContextsSustainedRateThrottle(_TeamBucketRateThrottle): rate = "600/hour" +# Flag sizing scans a 60-day events window once per condition group, and the release-conditions +# editor fans one request out per group on every mount with no dedup. That editor is the only real +# caller, and the ClickHouse*RateThrottle pair does not cover it: those derive from +# PersonalApiKeyRateThrottle, which lets an authenticated request carrying no personal API key +# through before counting it. A team bucket caps the project's scan spend whatever the auth method. +# Repeat empty-properties requests are served from a 5-minute cache, so a legitimate session needs +# one cold scan per distinct condition group. +class FlagSizingBurstRateThrottle(_TeamBucketRateThrottle): + scope = "flag_sizing_burst" + rate = "60/minute" + + +class FlagSizingSustainedRateThrottle(_TeamBucketRateThrottle): + scope = "flag_sizing_sustained" + rate = "600/hour" + + # Fingerprint projection runs t-SNE synchronously over up to 250 high-dimensional embeddings. # Query endpoint defaults only cover personal API keys, so use a team-wide bucket to include # session callers and prevent forced refreshes from consuming application workers without bound. diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index 1cc82f55ffc6..f7b3e08cffe8 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -76,6 +76,8 @@ from posthog.rate_limit import ( ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle, + FlagSizingBurstRateThrottle, + FlagSizingSustainedRateThrottle, PersonalOrProjectSecretApiKeyRateThrottle, ProjectSecretApiKeyTeamRateThrottle, ) @@ -2894,17 +2896,23 @@ class UserBlastRadiusRequestSerializer(serializers.Serializer): class UserBlastRadiusResponseSerializer(serializers.Serializer): affected = serializers.IntegerField( help_text=( - "Number of entities matching the condition. For person-based flags this counts persons active in the " - "last 60 days; for group-based flags it counts all groups of this type." + "Number of entities matching the condition. Bounded by activity_window_days when that field is set, " + "and all-time otherwise." ) ) total = serializers.IntegerField( help_text=( - "Denominator the affected count is shown against. For person-based flags this is the number of persons " - "active in the last 60 days; for group-based flags it is the all-time number of groups of this type in " - "the project." + "Denominator the affected count is shown against: persons for person-based flags, groups of this type " + "for group-based ones. Bounded by activity_window_days when that field is set, and all-time otherwise." ) ) + activity_window_days = serializers.IntegerField( + allow_null=True, + help_text=( + "Number of days of recent activity both counts are drawn from. Null means they are all-time, which is " + "the case for group-based flags and for projects where the recent-activity basis is switched off." + ), + ) # HYPERCACHE CONTRACT: This serializer defines the JSON schema that the Rust feature-flags @@ -4556,7 +4564,7 @@ def evaluation_reasons(self, request: request.Request, **kwargs): methods=["POST"], detail=False, required_scopes=["feature_flag:read"], - throttle_classes=[ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle], + throttle_classes=[FlagSizingBurstRateThrottle, FlagSizingSustainedRateThrottle], ) def user_blast_radius(self, request: request.Request, **kwargs): if "condition" not in request.data: @@ -4572,7 +4580,13 @@ def user_blast_radius(self, request: request.Request, **kwargs): recently_active_only=recently_active_sizing_enabled(self.team), ) - return Response({"affected": result.affected, "total": result.total}) + return Response( + { + "affected": result.affected, + "total": result.total, + "activity_window_days": result.activity_window_days, + } + ) @action(methods=["POST"], detail=True) def create_static_cohort_for_flag(self, request: request.Request, **kwargs): diff --git a/products/feature_flags/backend/api/test/test_feature_flag.py b/products/feature_flags/backend/api/test/test_feature_flag.py index bd90951477eb..99e598423052 100644 --- a/products/feature_flags/backend/api/test/test_feature_flag.py +++ b/products/feature_flags/backend/api/test/test_feature_flag.py @@ -75,7 +75,12 @@ from products.feature_flags.backend.flag_status import FeatureFlagStatus from products.feature_flags.backend.models.feature_flag import FeatureFlag, FeatureFlagDashboards from products.feature_flags.backend.models.team_feature_flags_config import TeamFeatureFlagsConfig -from products.feature_flags.backend.user_blast_radius import get_user_blast_radius, get_user_blast_radius_persons +from products.feature_flags.backend.user_blast_radius import ( + RECENTLY_ACTIVE_DAYS, + get_user_blast_radius, + get_user_blast_radius_persons, + recently_active_sizing_enabled, +) from products.product_analytics.backend.facade.models import Insight from products.product_tours.backend.models import ProductTour from products.surveys.backend.models import Survey @@ -9552,25 +9557,61 @@ def test_user_blast_radius_excludes_personless_events(self): def test_user_blast_radius_caches_the_unfiltered_active_count(self): # The flag editor sizes one condition group per mount, so repeat requests inside the TTL - # must not re-scan events each time. + # must not re-scan events each time. A second project asserts the key is scoped per team: + # an unscoped key would serve this project's count as the other one's. + other_team = Team.objects.create(organization=self.organization) _create_active_person(team_id=self.team.pk, distinct_ids=["active"], properties={"group": "match"}) + for i in range(2): + _create_active_person(team_id=other_team.pk, distinct_ids=[f"other-active-{i}"]) flush_persons_and_events() + condition = {"properties": [], "rollout_percentage": 50} with self.capture_select_queries() as queries: responses = [ self.client.post( f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", - {"condition": {"properties": [], "rollout_percentage": 50}}, + {"condition": condition}, ) for _ in range(2) ] + other_response = self.client.post( + f"/api/projects/{other_team.id}/feature_flags/user_blast_radius", + {"condition": condition}, + ) for response in responses: self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertLessEqual({"affected": 1, "total": 1}.items(), response.json().items()) + self.assertEqual(other_response.status_code, status.HTTP_200_OK) + self.assertLessEqual({"affected": 2, "total": 2}.items(), other_response.json().items()) + events_scans = [query for query in queries if "uniq(" in query] - self.assertEqual(len(events_scans), 1) + self.assertEqual(len(events_scans), 2) + + def test_user_blast_radius_reports_the_basis_that_produced_the_counts(self): + # The window is gated per project, so the response has to say which basis ran: the tooltip + # beside the number reads from this field rather than asserting the window unconditionally. + _create_active_person(team_id=self.team.pk, distinct_ids=["active"], properties={"group": "match"}) + flush_persons_and_events() + + condition = { + "properties": [{"key": "group", "type": "person", "value": ["match"], "operator": "exact"}], + "rollout_percentage": 100, + } + windowed = self.client.post( + f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", {"condition": condition} + ) + with patch( + "products.feature_flags.backend.api.feature_flag.recently_active_sizing_enabled", + return_value=False, + ): + all_time = self.client.post( + f"/api/projects/{self.team.id}/feature_flags/user_blast_radius", {"condition": condition} + ) + + self.assertEqual(windowed.json()["activity_window_days"], RECENTLY_ACTIVE_DAYS) + self.assertIsNone(all_time.json()["activity_window_days"]) @parameterized.expand( [ @@ -14876,3 +14917,19 @@ def test_freeing_a_tombstoned_key_relinks_teams_to_the_tombstone(self, mode: str assert old_flag.key == f"replay-gate:deleted:{old_flag.id}" self.team.refresh_from_db() assert self.team.session_recording_linked_flag == {"id": old_flag.id, "key": old_flag.key} + + +class TestRecentlyActiveSizingFlag(APIBaseTest): + @parameterized.expand( + [ + ("enabled", True, True), + ("disabled", False, False), + ("client_raises", Exception("flag client down"), False), + ] + ) + def test_kill_switch_never_propagates_a_flag_client_failure(self, _name, flag_result, expected): + # A broken flag client must read as "window off", not as a 500 on the sizing panel. Both + # sides of the endpoint patch this symbol, so nothing else runs its body. + kwargs = {"side_effect": flag_result} if isinstance(flag_result, Exception) else {"return_value": flag_result} + with patch("products.feature_flags.backend.user_blast_radius.feature_enabled_or_false", **kwargs): + self.assertEqual(recently_active_sizing_enabled(self.team), expected) diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index 90028d562b28..bc2d411056af 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -43,6 +43,10 @@ class BlastRadiusResult: affected: int total: int + # Days of activity both counts are drawn from, or None when they are all-time. The kill switch + # decides which basis ran, so the copy shown beside the number has to follow this rather than + # assume a window. + activity_window_days: Optional[int] = None # Window for the blast-radius denominator. An all-time person count is inflated by anonymous, @@ -213,6 +217,19 @@ def _recently_active_window() -> tuple[datetime, datetime]: return now - timedelta(days=RECENTLY_ACTIVE_DAYS), now + timedelta(days=1) +def _recently_active_events_where() -> tuple[str, dict[str, ast.Expr]]: + """The events predicate both activity-window queries select on, with its placeholders. + + Both can answer the same panel in one request cycle, so two copies could drift into reporting + two different totals for one team. + """ + cutoff, upper = _recently_active_window() + return ( + f"timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}", + {"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)}, + ) + + def _recently_active_persons_count(team: Team) -> int: """Count distinct persons active in the last RECENTLY_ACTIVE_DAYS days. @@ -225,12 +242,8 @@ def _recently_active_persons_count(team: Team) -> int: if cached is not None: return cached - cutoff, upper = _recently_active_window() - query = parse_select( - "SELECT uniq(person_id) FROM events " - f"WHERE timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}", - placeholders={"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)}, - ) + where, placeholders = _recently_active_events_where() + query = parse_select(f"SELECT uniq(person_id) FROM events WHERE {where}", placeholders=placeholders) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) response = execute_hogql_query( @@ -245,22 +258,29 @@ def _recently_active_persons_count(team: Team) -> int: return count +def _matching_persons_exprs(team: Team, filter: Filter) -> list[ast.Expr]: + """Team scope plus the condition's property filters, in person scope. + + Sizing and the workflows preview both decide who matches a condition from this predicate, so a + change to one copy alone would make the previewed audience differ from the sized one. + property_to_expr resolves cohorts, static cohorts, and group properties. + """ + return [ + ast.CompareOperation( + op=ast.CompareOperationOp.Eq, + left=ast.Field(chain=["persons", "team_id"]), + right=ast.Constant(value=team.pk), + ), + property_to_expr(filter.property_groups, team, scope="person"), + ] + + def _matched_persons_query(team: Team, filter: Filter) -> ast.SelectQuery: - """Subquery of the person ids matching the condition. Reuses property_to_expr in person scope, - so it resolves cohorts, static cohorts, and group properties exactly like _build_person_query.""" + """Subquery of the person ids matching the condition.""" return ast.SelectQuery( select=[ast.Field(chain=["persons", "id"])], select_from=ast.JoinExpr(table=ast.Field(chain=["persons"])), - where=ast.And( - exprs=[ - ast.CompareOperation( - op=ast.CompareOperationOp.Eq, - left=ast.Field(chain=["persons", "team_id"]), - right=ast.Constant(value=team.pk), - ), - property_to_expr(filter.property_groups, team, scope="person"), - ] - ), + where=ast.And(exprs=_matching_persons_exprs(team, filter)), ) @@ -275,19 +295,14 @@ def _get_person_blast_radius_recently_active(team: Team, filter: Filter) -> Blas if len(properties) == 0: # No filters means every recently active person is affected. total_users = _recently_active_persons_count(team) - return BlastRadiusResult(affected=total_users, total=total_users) + return BlastRadiusResult(affected=total_users, total=total_users, activity_window_days=RECENTLY_ACTIVE_DAYS) - cutoff, upper = _recently_active_window() + where, placeholders = _recently_active_events_where() # One pass over the recent event window yields both the active total and the matched subset, so # the denominator is never a second scan and both numbers come from the same rows. query = parse_select( - "SELECT uniq(person_id), uniqIf(person_id, person_id IN {matched}) " - f"FROM events WHERE timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}", - placeholders={ - "matched": _matched_persons_query(team, filter), - "cutoff": ast.Constant(value=cutoff), - "upper": ast.Constant(value=upper), - }, + f"SELECT uniq(person_id), uniqIf(person_id, person_id IN {{matched}}) FROM events WHERE {where}", + placeholders={"matched": _matched_persons_query(team, filter), **placeholders}, ) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) @@ -304,7 +319,9 @@ def _get_person_blast_radius_recently_active(team: Team, filter: Filter) -> Blas total_users, affected = (response.results[0][0], response.results[0][1]) if response.results else (0, 0) # affected is a strict subset of total_users, but both are uniq() estimates, so clamp to keep # the frontend percentage coherent. - return BlastRadiusResult(affected=min(affected, total_users), total=total_users) + return BlastRadiusResult( + affected=min(affected, total_users), total=total_users, activity_window_days=RECENTLY_ACTIVE_DAYS + ) def _get_person_blast_radius(team: Team, filter: Filter) -> BlastRadiusResult: @@ -355,19 +372,7 @@ def _build_person_query(team: Team, filter: Filter, return_count: bool = True, c distinct=True, ) - # Build WHERE clause with team_id and property filters - # property_to_expr handles all property types including cohorts - where_exprs: list[ast.Expr] = [ - ast.CompareOperation( - op=ast.CompareOperationOp.Eq, - left=ast.Field(chain=["persons", "team_id"]), - right=ast.Constant(value=team.pk), - ) - ] - - # Add all property filters (including cohorts) via property_to_expr - property_expr = property_to_expr(filter.property_groups, team, scope="person") - where_exprs.append(property_expr) + where_exprs: list[ast.Expr] = _matching_persons_exprs(team, filter) # Add cursor-based pagination when returning IDs if not return_count and cursor is not None: diff --git a/products/feature_flags/frontend/generated/api.schemas.ts b/products/feature_flags/frontend/generated/api.schemas.ts index 1dc48a8b8e5c..aeed085a7ac9 100644 --- a/products/feature_flags/frontend/generated/api.schemas.ts +++ b/products/feature_flags/frontend/generated/api.schemas.ts @@ -1578,10 +1578,15 @@ export interface UserBlastRadiusRequestApi { } export interface UserBlastRadiusResponseApi { - /** Number of entities matching the condition. For person-based flags this counts persons active in the last 60 days; for group-based flags it counts all groups of this type. */ + /** Number of entities matching the condition. Bounded by activity_window_days when that field is set, and all-time otherwise. */ affected: number - /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 60 days; for group-based flags it is the all-time number of groups of this type in the project. */ + /** Denominator the affected count is shown against: persons for person-based flags, groups of this type for group-based ones. Bounded by activity_window_days when that field is set, and all-time otherwise. */ total: number + /** + * Number of days of recent activity both counts are drawn from. Null means they are all-time, which is the case for group-based flags and for projects where the recent-activity basis is switched off. + * @nullable + */ + activity_window_days: number | null } export interface FlagValueItemApi { diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index b7f0dadc4862..e6c6af5e9aa9 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -86842,10 +86842,15 @@ export namespace Schemas { } export interface UserBlastRadiusResponse { - /** Number of entities matching the condition. For person-based flags this counts persons active in the last 60 days; for group-based flags it counts all groups of this type. */ + /** Number of entities matching the condition. Bounded by activity_window_days when that field is set, and all-time otherwise. */ affected: number; - /** Denominator the affected count is shown against. For person-based flags this is the number of persons active in the last 60 days; for group-based flags it is the all-time number of groups of this type in the project. */ + /** Denominator the affected count is shown against: persons for person-based flags, groups of this type for group-based ones. Bounded by activity_window_days when that field is set, and all-time otherwise. */ total: number; + /** + * Number of days of recent activity both counts are drawn from. Null means they are all-time, which is the case for group-based flags and for projects where the recent-activity basis is switched off. + * @nullable + */ + activity_window_days: number | null; } export interface UserFacetSettings { From a38c04c56833e29c160d43c785a3a346febc300f Mon Sep 17 00:00:00 2001 From: James Gill Date: Fri, 11 Sep 2026 11:13:53 +0100 Subject: [PATCH 11/12] fix(flags): build the activity-window query text at import time The semgrep rule hogql-fstring-audit flags any f-string that reaches parse_select, so a reviewer checks whether caller input is interpolated. Hold the two query strings as module constants instead. Both still derive from one shared predicate, so they cannot drift apart. The window bounds keep arriving as placeholders, so no caller-supplied value reaches the query text on either path. Co-Authored-By: Claude Opus 5 (1M context) --- .../backend/user_blast_radius.py | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index aa28685e150e..a0acfff8fb5a 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -65,6 +65,17 @@ class BlastRadiusResult: # identified_only default, which is the opposite of what the window is for. _IDENTIFIED_PERSONS_ONLY = "person_mode != 'propertyless'" +# Both activity-window queries select on this events predicate. They can answer one panel in a +# single request cycle, so two copies could drift into reporting two different totals for one team. +# The query text is fixed at import time and the bounds arrive as placeholders, so no caller-supplied +# value ever reaches the string. +_RECENTLY_ACTIVE_EVENTS_WHERE = f"timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}" +_RECENTLY_ACTIVE_COUNT_QUERY = f"SELECT uniq(person_id) FROM events WHERE {_RECENTLY_ACTIVE_EVENTS_WHERE}" +_RECENTLY_ACTIVE_MATCHED_QUERY = ( + f"SELECT uniq(person_id), uniqIf(person_id, person_id IN {{matched}}) " + f"FROM events WHERE {_RECENTLY_ACTIVE_EVENTS_WHERE}" +) + # ClickHouse codes for "this literal can't be parsed as the column's type": 6 CANNOT_PARSE_TEXT, # 72 CANNOT_PARSE_NUMBER — e.g. a numeric operator (gt/lt) against a null or non-numeric filter @@ -217,17 +228,10 @@ def _recently_active_window() -> tuple[datetime, datetime]: return now - timedelta(days=RECENTLY_ACTIVE_DAYS), now + timedelta(days=1) -def _recently_active_events_where() -> tuple[str, dict[str, ast.Expr]]: - """The events predicate both activity-window queries select on, with its placeholders. - - Both can answer the same panel in one request cycle, so two copies could drift into reporting - two different totals for one team. - """ +def _recently_active_window_placeholders() -> dict[str, ast.Expr]: + """Bind the window bounds for the query text above.""" cutoff, upper = _recently_active_window() - return ( - f"timestamp >= {{cutoff}} AND timestamp < {{upper}} AND {_IDENTIFIED_PERSONS_ONLY}", - {"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)}, - ) + return {"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)} def _recently_active_persons_count(team: Team) -> int: @@ -242,8 +246,7 @@ def _recently_active_persons_count(team: Team) -> int: if cached is not None: return cached - where, placeholders = _recently_active_events_where() - query = parse_select(f"SELECT uniq(person_id) FROM events WHERE {where}", placeholders=placeholders) + query = parse_select(_RECENTLY_ACTIVE_COUNT_QUERY, placeholders=_recently_active_window_placeholders()) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) response = execute_hogql_query( @@ -297,12 +300,14 @@ def _get_person_blast_radius_recently_active(team: Team, filter: Filter) -> Blas total_users = _recently_active_persons_count(team) return BlastRadiusResult(affected=total_users, total=total_users, activity_window_days=RECENTLY_ACTIVE_DAYS) - where, placeholders = _recently_active_events_where() # One pass over the recent event window yields both the active total and the matched subset, so # the denominator is never a second scan and both numbers come from the same rows. query = parse_select( - f"SELECT uniq(person_id), uniqIf(person_id, person_id IN {{matched}}) FROM events WHERE {where}", - placeholders={"matched": _matched_persons_query(team, filter), **placeholders}, + _RECENTLY_ACTIVE_MATCHED_QUERY, + placeholders={ + "matched": _matched_persons_query(team, filter), + **_recently_active_window_placeholders(), + }, ) tag_queries(product=Product.FEATURE_FLAGS, feature=Feature.QUERY) From a0d7fc4eb588c070c22a5ce8994faa01123035f9 Mon Sep 17 00:00:00 2001 From: James Gill Date: Fri, 11 Sep 2026 11:29:42 +0100 Subject: [PATCH 12/12] fix(flags): clear the CI checks the sizing changes tripped Three separate checks, all on code this branch adds. Sort the two new members into the slots kea-typegen writes them in. The generated interfaces are alphabetical, and hand-placing a member leaves a file that build:products rewrites, which fails the up-to-date check. Return the sizing response through UserBlastRadiusResponseSerializer instead of a hand-built dict. The OpenAPI schema comes from that declaration, so a dict beside it can drift from the frontend and MCP types without any check catching it. Fold the window bounds into the placeholder helper. The tuple of two datetimes could be swapped at the call site, and it had one caller. Co-Authored-By: Claude Opus 5 (1M context) --- .../featureFlagReleaseConditionsLogic.ts | 10 ++++---- .../feature_flags/backend/api/feature_flag.py | 10 +++----- .../backend/user_blast_radius.py | 24 +++++++++---------- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts index e032f55efda3..d3e90aca67ce 100644 --- a/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts +++ b/frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts @@ -173,6 +173,7 @@ export interface featureFlagReleaseConditionsLogicValues { aggregationLabel: (groupTypeIndex: number | null | undefined, deferToUserWording?: boolean) => Noun // groupsModel groupTypes: Map // groupsModel currentProjectId: number | null // projectLogic + activityWindowDays: number | null affectedCounts: Record aggregationTargetName: (conditionGroupTypeIndex?: number | null | undefined) => string blastRadiusErrors: Record @@ -203,7 +204,6 @@ export interface featureFlagReleaseConditionsLogicValues { rollout_percentage: string | undefined variant: null }[] - activityWindowDays: number | null taxonomicGroupTypes: TaxonomicFilterGroupType[] taxonomicGroupTypesForCondition: (conditionGroupTypeIndex: number | null | undefined) => TaxonomicFilterGroupType[] totalCounts: Record @@ -254,6 +254,9 @@ export interface featureFlagReleaseConditionsLogicActions { activeId: string overId: string } + setActivityWindowDays: (activityWindowDays: number | null) => { + activityWindowDays: number | null + } setAffectedCount: ( sortKey: string, count?: number @@ -261,9 +264,6 @@ export interface featureFlagReleaseConditionsLogicActions { count: number | undefined sortKey: string } - setActivityWindowDays: (activityWindowDays: number | null) => { - activityWindowDays: number | null - } setAggregationGroupTypeIndex: (value: number | null) => { value: number | null } @@ -346,7 +346,7 @@ export interface featureFlagReleaseConditionsLogicMeta { filters: FeatureFlagFilters & { groups: FeatureFlagGroupTypeWithSortKey[] }, - aggregationLabel: (groupTypeIndex: number | null | undefined, deferToUserWording?: boolean) => Noun + aggregationLabel: (groupTypeIndex: number | null | undefined, deferToUserWording?: boolean) => Noun // groupsModel ) => (conditionGroupTypeIndex?: number | null | undefined) => string taxonomicGroupTypesForCondition: ( filters: FeatureFlagFilters & { diff --git a/products/feature_flags/backend/api/feature_flag.py b/products/feature_flags/backend/api/feature_flag.py index cb56c50923a7..9673a0bc9b88 100644 --- a/products/feature_flags/backend/api/feature_flag.py +++ b/products/feature_flags/backend/api/feature_flag.py @@ -4701,13 +4701,9 @@ def user_blast_radius(self, request: request.Request, **kwargs): recently_active_only=recently_active_sizing_enabled(self.team), ) - return Response( - { - "affected": result.affected, - "total": result.total, - "activity_window_days": result.activity_window_days, - } - ) + # Serialize through the declared response serializer, so the wire shape cannot drift from + # the OpenAPI schema that the frontend and MCP types are generated from. + return Response(UserBlastRadiusResponseSerializer(result).data) @action(methods=["POST"], detail=True) def create_static_cohort_for_flag(self, request: request.Request, **kwargs): diff --git a/products/feature_flags/backend/user_blast_radius.py b/products/feature_flags/backend/user_blast_radius.py index a0acfff8fb5a..6b2ba9480126 100644 --- a/products/feature_flags/backend/user_blast_radius.py +++ b/products/feature_flags/backend/user_blast_radius.py @@ -1,6 +1,6 @@ from collections.abc import Iterator from contextlib import contextmanager -from datetime import datetime, timedelta +from datetime import timedelta from typing import Optional from django.core.cache import cache @@ -220,18 +220,18 @@ def get_user_blast_radius_persons( return _get_person_blast_radius_persons(team, cleaned_filter, cursor=cursor) -def _recently_active_window() -> tuple[datetime, datetime]: - """Bounds of the blast-radius activity window, computed in Python so they respect freeze_time in - tests and do not depend on ClickHouse server time. The upper bound allows a day of clock skew - but stops a far-future event timestamp from keeping a person "active" for years.""" - now = timezone.now() - return now - timedelta(days=RECENTLY_ACTIVE_DAYS), now + timedelta(days=1) - - def _recently_active_window_placeholders() -> dict[str, ast.Expr]: - """Bind the window bounds for the query text above.""" - cutoff, upper = _recently_active_window() - return {"cutoff": ast.Constant(value=cutoff), "upper": ast.Constant(value=upper)} + """Bind the activity window bounds for the query text above. + + Python computes them, so they respect freeze_time in tests and do not depend on ClickHouse + server time. The upper bound allows a day of clock skew, but stops a far-future event timestamp + from keeping a person "active" for years. + """ + now = timezone.now() + return { + "cutoff": ast.Constant(value=now - timedelta(days=RECENTLY_ACTIVE_DAYS)), + "upper": ast.Constant(value=now + timedelta(days=1)), + } def _recently_active_persons_count(team: Team) -> int: