Skip to content

fix(flags): size flags by recently active persons, not all-time count #92668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JamesPatrickGill wants to merge 14 commits into from
Closed

fix(flags): size flags by recently active persons, not all-time count #92668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c44b598
fix(flags): size flags by recently active persons, not all-time count
JamesPatrickGill Sep 1, 2026
a387722
chore: update OpenAPI generated types
tests-posthog[bot] Sep 1, 2026
c1b3c03
fix(flags): widen active-persons window to 60 days
JamesPatrickGill Sep 2, 2026
21a9fb6
Merge remote-tracking branch 'origin/master' into posthog/flags-size-…
JamesPatrickGill Sep 4, 2026
3a8b062
test(backend): update query snapshots
tests-posthog[bot] Sep 4, 2026
138263a
fix(flags): keep the anonymous-visitor caveat on the sizing tooltip
posthog[bot] Sep 4, 2026
0960146
fix(flags): scope the activity window to flag sizing only
JamesPatrickGill Sep 4, 2026
f36c7bd
fix(flags): cache the blast-radius active count and fail loud on timeout
JamesPatrickGill Sep 8, 2026
43c7895
fix(flags): keep personless events out of the blast-radius counts
JamesPatrickGill Sep 8, 2026
0497dd2
fix(flags): gate the activity window behind a kill switch and throttl…
JamesPatrickGill Sep 8, 2026
77cbb4e
fix(flags): throttle session callers and report the sizing basis
JamesPatrickGill Sep 11, 2026
1ef3da9
Merge remote-tracking branch 'origin/master' into posthog/flags-size-…
JamesPatrickGill Sep 11, 2026
a38c04c
fix(flags): build the activity-window query text at import time
JamesPatrickGill Sep 11, 2026
a0d7fc4
fix(flags): clear the CI checks the sizing changes tripped
JamesPatrickGill Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.matchingUsersLink.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand All @@ -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],
Expand All @@ -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 },
],
},
})
})
Expand Down Expand Up @@ -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(
<Provider>
<FeatureFlagReleaseConditions id="1234" filters={filters} onChange={jest.fn()} />
</Provider>
)

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) => {
Expand Down
15 changes: 11 additions & 4 deletions frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

import { resolveAggregationGroupTypeIndex } from './aggregation'
import { BlastRadiusErrorMessage } from './BlastRadiusErrorMessage'
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,
Expand Down Expand Up @@ -107,7 +107,7 @@
)
}

export function FeatureFlagReleaseConditions({

Check warning on line 110 in frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`FeatureFlagReleaseConditions` has cyclomatic complexity 20 (warn >10)
id,
readOnly,
excludeTitle,
Expand Down Expand Up @@ -144,6 +144,7 @@
properties,
filterGroups,
getDistinctIdName,
activityWindowDays,
} = useValues(releaseConditionsLogic)

const {
Expand Down Expand Up @@ -194,7 +195,7 @@
groupsAccessStatus
)

const renderReleaseConditionGroup = (group: FeatureFlagGroupType, index: number): JSX.Element => {

Check warning on line 198 in frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`renderReleaseConditionGroup` has cyclomatic complexity 37 (warn >10)
const rolloutValue = group.rollout_percentage ?? 100

return (
Expand Down Expand Up @@ -451,7 +452,7 @@
)}
/>
of <b>{aggregationTargetName(group.aggregation_group_type_index)}</b> in this set.
{(() => {

Check warning on line 455 in frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`<anonymous>` has cyclomatic complexity 13 (warn >10)
const targetIndex = group.aggregation_group_type_index
const resolvedGroupTypeIndex = resolveAggregationGroupTypeIndex(
targetIndex,
Expand Down Expand Up @@ -507,9 +508,15 @@
<div className="basis-full flex flex-col mt-1 text-secondary tabular-nums">
<span>
Filters match: <b>~{pluralize(affected, singularName, pluralName)}</b>
{filters.aggregation_group_type_index == null && (
<Tooltip title={MATCHING_ESTIMATE_TOOLTIP} interactive>
<IconInfo className="text-muted text-xs ml-0.5" />
{resolvedGroupTypeIndex == null && (

@haacked haacked Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@JamesPatrickGill JamesPatrickGill Sep 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread
haacked marked this conversation as resolved.
Resolved
<Tooltip
title={matchingEstimateTooltip(activityWindowDays)}
interactive
>
<IconInfo
className="text-muted text-xs ml-0.5"
data-attr="matching-estimate-info"
/>
</Tooltip>
)}
</span>
Expand Down
18 changes: 15 additions & 3 deletions frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@

import { resolveAggregationGroupTypeIndex } from './aggregation'
import { BlastRadiusErrorMessage } from './BlastRadiusErrorMessage'
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'
Expand Down Expand Up @@ -137,7 +137,7 @@
return `All ${capitalizedTarget}`
}

const parts = properties.slice(0, 2).map((property) => {

Check warning on line 140 in frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`<anonymous>` has cyclomatic complexity 17 (warn >10)
let key: string
if (property.type === PropertyFilterType.Cohort) {
key = 'Cohort'
Expand Down Expand Up @@ -348,6 +348,7 @@
affectedCounts: Record<string, number | undefined>
totalCounts: Record<string, number | undefined>
blastRadiusErrors: Record<string, BlastRadiusError | undefined>
activityWindowDays: number | null
calculateBlastRadiusForCondition: (
sortKey: string,
properties: AnyPropertyFilter[] | undefined,
Expand Down Expand Up @@ -419,13 +420,14 @@
)
}

const ConditionContent = ({

Check warning on line 423 in frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`ConditionContent` has cyclomatic complexity 32 (warn >10)
group,
index,
totalGroups,
affectedCounts,
totalCounts,
blastRadiusErrors,
activityWindowDays,
calculateBlastRadiusForCondition,
aggregationTargetName,
getDistinctIdName,
Expand Down Expand Up @@ -751,10 +753,15 @@
</b>
{resolvedGroupTypeIndex == null && (
<Tooltip
title={MATCHING_ESTIMATE_TOOLTIP}
title={matchingEstimateTooltip(
activityWindowDays
)}
interactive
>
<IconInfo className="text-muted text-xs ml-0.5" />
<IconInfo
className="text-muted text-xs ml-0.5"
data-attr="matching-estimate-info"
/>
</Tooltip>
)}
</span>
Expand Down Expand Up @@ -868,7 +875,7 @@
)
}

export function FeatureFlagReleaseConditionsCollapsible({

Check warning on line 878 in frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsCollapsible.tsx

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`FeatureFlagReleaseConditionsCollapsible` has cyclomatic complexity 25 (warn >10)
id,
flagId,
filters,
Expand Down Expand Up @@ -896,6 +903,7 @@
affectedCounts,
totalCounts,
blastRadiusErrors,
activityWindowDays,
aggregationTargetName,
getDistinctIdName,
getFlagKey,
Expand Down Expand Up @@ -1299,6 +1307,7 @@
affectedCounts={affectedCounts}
totalCounts={totalCounts}
blastRadiusErrors={blastRadiusErrors}
activityWindowDays={activityWindowDays}
calculateBlastRadiusForCondition={calculateBlastRadiusForCondition}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
Expand Down Expand Up @@ -1380,6 +1389,7 @@
affectedCounts={affectedCounts}
totalCounts={totalCounts}
blastRadiusErrors={blastRadiusErrors}
activityWindowDays={activityWindowDays}
calculateBlastRadiusForCondition={calculateBlastRadiusForCondition}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
Expand Down Expand Up @@ -1435,6 +1445,7 @@
affectedCounts={affectedCounts}
totalCounts={totalCounts}
blastRadiusErrors={blastRadiusErrors}
activityWindowDays={activityWindowDays}
calculateBlastRadiusForCondition={calculateBlastRadiusForCondition}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
Expand Down Expand Up @@ -1472,6 +1483,7 @@
affectedCounts={affectedCounts}
totalCounts={totalCounts}
blastRadiusErrors={blastRadiusErrors}
activityWindowDays={activityWindowDays}
calculateBlastRadiusForCondition={calculateBlastRadiusForCondition}
aggregationTargetName={aggregationTargetName}
getDistinctIdName={getDistinctIdName}
Expand Down
9 changes: 6 additions & 3 deletions frontend/src/scenes/feature-flags/constants.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => (
<>
<div>
A user may have{' '}
Expand All @@ -18,8 +20,9 @@ export const MATCHING_ESTIMATE_TOOLTIP = (
Estimated from{' '}
<Link to="https://posthog.com/docs/data/anonymous-vs-identified-events" target="_blank">
identified users
</Link>{' '}
only. Anonymous visitors can still match this flag, so the actual number may be higher.
</Link>
{activityWindowDays === null ? '.' : ` active in the last ${activityWindowDays} days.`}
</div>
<div className="mt-1">Anonymous visitors can still match this flag, so the actual number may be higher.</div>
</>
)
9 changes: 6 additions & 3 deletions frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,12 +549,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()
Expand Down Expand Up @@ -597,7 +599,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'),
Expand Down
30 changes: 21 additions & 9 deletions frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@
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'
Expand Down Expand Up @@ -172,6 +173,7 @@
aggregationLabel: (groupTypeIndex: number | null | undefined, deferToUserWording?: boolean) => Noun // groupsModel
groupTypes: Map<GroupTypeIndex, GroupType> // groupsModel
currentProjectId: number | null // projectLogic
activityWindowDays: number | null
affectedCounts: Record<string, number | undefined>
aggregationTargetName: (conditionGroupTypeIndex?: number | null | undefined) => string
blastRadiusErrors: Record<string, BlastRadiusError | undefined>
Expand Down Expand Up @@ -252,6 +254,9 @@
activeId: string
overId: string
}
setActivityWindowDays: (activityWindowDays: number | null) => {
activityWindowDays: number | null
}
setAffectedCount: (
sortKey: string,
count?: number
Expand Down Expand Up @@ -341,7 +346,7 @@
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 & {
Expand Down Expand Up @@ -430,6 +435,7 @@
count,
}),
setBlastRadiusError: (sortKey: string, error: BlastRadiusError) => ({ sortKey, error }),
setActivityWindowDays: (activityWindowDays: number | null) => ({ activityWindowDays }),
calculateBlastRadius: true,
calculateBlastRadiusForCondition: (
sortKey: string,
Expand Down Expand Up @@ -659,6 +665,14 @@
}),
},
],
// The basis depends on the project, not the condition, so every response carries the same
// value and one reducer covers all of them.
activityWindowDays: [
null as number | null,
{
setActivityWindowDays: (_, { activityWindowDays }) => activityWindowDays,
},
],
// Tracks conditions whose blast-radius estimate failed, keyed by sort_key. Holds the
// caught error so the UI can explain why (and decide whether a retry is worth offering)
// instead of just distinguishing failure from the still-loading (undefined) state.
Expand Down Expand Up @@ -855,15 +869,13 @@
actions.setTotalCount(sortKey, undefined)

try {
const response: UserBlastRadiusType = await api.create(
`api/projects/${values.currentProjectId}/feature_flags/user_blast_radius`,
{
condition: { properties },
group_type_index: groupTypeIndex,
}
)
const response = await featureFlagsUserBlastRadiusCreate(String(values.currentProjectId), {

@haacked haacked Sep 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

condition: { properties },
group_type_index: groupTypeIndex,
})
actions.setAffectedCount(sortKey, response.affected)
actions.setTotalCount(sortKey, response.total)
actions.setActivityWindowDays(response.activity_window_days ?? null)

@haacked haacked Sep 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

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

} catch (error) {
// Keep the caught error so the UI can show what went wrong and hide a retry that
// can't help, rather than masking it as -1, which the render path can't tell
Expand Down Expand Up @@ -1130,7 +1142,7 @@
computeBlastRadiusPercentage: [
(s) => [s.affectedCounts, s.totalCounts],
(affectedCounts: Record<string, number | undefined>, totalCounts: Record<string, number | undefined>) =>
(rolloutPercentage, sortKey) => {

Check warning on line 1145 in frontend/src/scenes/feature-flags/featureFlagReleaseConditionsLogic.ts

View workflow job for this annotation

GitHub Actions / Frontend formatting

lint:complexity 

`<anonymous>` has cyclomatic complexity 11 (warn >10)
let effectiveRolloutPercentage = rolloutPercentage
if (
rolloutPercentage === undefined ||
Expand Down
5 changes: 0 additions & 5 deletions frontend/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4596,11 +4596,6 @@ export interface MinimalEarlyAccessFeatureType {
payload?: Record<string, any>
}

export interface UserBlastRadiusType {
affected: number
total: number
}

export enum ScheduledChangeModels {
FeatureFlag = 'FeatureFlag',
}
Expand Down
17 changes: 17 additions & 0 deletions posthog/rate_limit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,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"

@haacked haacked Sep 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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



class FlagSizingSustainedRateThrottle(_TeamBucketRateThrottle):
scope = "flag_sizing_sustained"
rate = "600/hour"


# Feature flag request usage scans up to 31 days of billing events in ClickHouse. Its primary
# caller is the session-authenticated feature flags UI, which the generic ClickHouse throttle
# pair does not cover. Use a team-wide bucket so users and API keys share one query budget.
Expand Down
Loading
Loading