fix(flags): evaluate device-bucketed flags in person and test tabs - #81618
fix(flags): evaluate device-bucketed flags in person and test tabs#81618posthog[bot] wants to merge 3 commits into
Conversation
A device-bucketed flag hashes on $device_id rather than distinct_id. The two flag debug surfaces in the app only know a distinct id, so they sent no device id, and the evaluation engine skips a person-aggregated device-bucketed condition when none is supplied and reports out_of_rollout_bound. That reads as "this person isn't in the rollout", which is impossible at 100% rollout, so the tabs disagreed with what the SDK actually returns. Resolve the most recent $device_id off the distinct id's events and pass it to the flags service as a top-level field, which is where the service reads it from. The person profile flags tab and a flag's test evaluation tab both go through this path. evaluation_reasons also accepts an explicit device_id for checking a specific device. Projects with no device-bucketed flag skip the events query entirely. A distinct id with no client-side events still resolves to no device id, and the tooltip now explains that case rather than the UI implying the person missed the rollout. Generated-By: PostHog Desktop Task-Id: a8e23c1a-c394-49ad-9db5-c350c16858b9
Also picks up an import-grouping blank line from ruff format. Generated-By: PostHog Desktop Task-Id: a8e23c1a-c394-49ad-9db5-c350c16858b9
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report✅ Bundle size — no changeUncompressed size of every built Total: 67.75 MiB · no change No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 285.3 KiB | ../node_modules/.pnpm/posthog-js@1.410.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 238.4 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 231.5 KiB | ../node_modules/.pnpm/posthog-js@1.410.1/node_modules/posthog-js/dist/module.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.5 KiB | src/lib/api.ts |
| 94.6 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.20 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.20 MiB · 17 files | no change | ████░░░░░░ 38.4% of 5.72 MiB |
| Deferred (lazy) | 2.08 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 725.7 KiB | dist/toolbar/toolbar-app-VGLJI6DC.css |
| 552.7 KiB | dist/toolbar/chunk-chunk-EZ5S4CFD.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-7QEIU5OS.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-GBTDYOYN.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.2 KiB | dist/toolbar/toolbar-app-EXXK5YIQ.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-N5Q3HTBN.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-6JVOILCC.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +1.1 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1421.45 MiB · 🔺 +1.1 KiB (+0.0%)
ℹ️ MCP UI apps size — 32 app(s), 17083.7 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 599.6 KB | 185.4 KB |
| action | 457.8 KB | 185.4 KB |
| action-list | 564.4 KB | 185.4 KB |
| cohort | 456.8 KB | 185.4 KB |
| cohort-list | 563.4 KB | 185.4 KB |
| email-template | 456.6 KB | 185.4 KB |
| error-details | 472.4 KB | 185.4 KB |
| error-issue | 457.5 KB | 185.4 KB |
| error-issue-list | 564.3 KB | 185.4 KB |
| experiment | 561.5 KB | 185.4 KB |
| experiment-list | 565.2 KB | 185.4 KB |
| experiment-results | 566.7 KB | 185.4 KB |
| feature-flag | 567.2 KB | 185.4 KB |
| feature-flag-list | 570.9 KB | 185.4 KB |
| feature-flag-testing | 461.0 KB | 185.4 KB |
| insight-actors | 562.2 KB | 185.4 KB |
| invite-email-preview | 456.0 KB | 185.4 KB |
| llm-costs | 559.5 KB | 185.4 KB |
| session-recording | 458.6 KB | 185.4 KB |
| session-summary | 463.9 KB | 185.4 KB |
| survey | 458.4 KB | 185.4 KB |
| survey-global-stats | 562.2 KB | 185.4 KB |
| survey-list | 565.1 KB | 185.4 KB |
| survey-stats | 562.2 KB | 185.4 KB |
| trace-span | 457.2 KB | 185.4 KB |
| trace-span-list | 564.3 KB | 185.4 KB |
| workflow | 457.1 KB | 185.4 KB |
| workflow-list | 563.7 KB | 185.4 KB |
| loops-review | 461.4 KB | 185.4 KB |
| query-results | 750.5 KB | 185.4 KB |
| render-ui | 834.6 KB | 185.4 KB |
| visual-review-snapshots | 461.6 KB | 185.4 KB |
The evaluation_reasons device_id query param changed the generated MCP tool schema, so its snapshot needed regenerating alongside the OpenAPI artifacts. Generated-By: PostHog Desktop Task-Id: a8e23c1a-c394-49ad-9db5-c350c16858b9
Problem
Someone opening a person's Flags tab sees a device-bucketed flag evaluate to
falsewith reason "Out of rollout bound", even when the flag is at 100% rollout with no property filters. Calling the flags API for the same person with their device ID returns the correct variant. The tab and the SDK disagree, which makes it look like the person was excluded from an experiment they are actually enrolled in.A device-bucketed flag hashes on
$device_idinstead ofdistinct_id, so its value survives the anonymous-to-identified transition. SDKs send$device_idon every/flagscall. The two debug surfaces in the app know only a distinct ID and sent none, and the engine skips a person-aggregated device-bucketed condition when no device id is supplied, reportingOutOfRolloutBound. At 100% rollout that reason is impossible, so the label was actively misleading.$device_idis not a person property, because one person can have several device ids, so it has to be read off the person's events.Changes
$device_idon that distinct ID's events, matching what the SDK sends.get_flags_from_servicegained adevice_idargument, sent as a top-level request field. The service ignores a device id nested insideperson_properties, which is the same trap as posthog-js#3318.evaluation_reasonsaccepts an optionaldevice_idquery param to check a specific device.$device_idon this distinct ID's events. A distinct ID with no client-side events has no device ID to bucket on, so those flags show as not matched here."Two decisions worth a reviewer's attention:
$device_idis stable across the identify boundary for one browser, so both distinct IDs of a person normally carry the same value. Scoping to the distinct ID keeps the answer specific to the identity being evaluated and keeps the query on an indexed column.distinct_idwhen no device id exists. Hashing on the distinct ID would return a confidently wrong variant, which is worse than an honest "not matched". A server-only identity resolves toNone, and the tooltip covers that case instead.Note
This does not add a distinct
missing_device_idreason code to the evaluation engine. That is the cleaner fix for the misleading label, but it changes an SDK-facing response field and belongs with the flags team rather than this PR.How did you test this code?
Automated tests, all run locally against the dev stack:
Test runs
Regressions each group catches, none of which an existing test covered:
TestFeatureFlagEvaluationReasons, 3 new parameterized cases: drop the resolution anddevice_idgoes back toNone, restoring the wrong reason; ignore the query param and an explicitly named device is silently overridden; drop the flag-definition gate and every person profile load pays for an events query.TestResolveDeviceId: flip theORDER BYand evaluation hashes on a stale device, silently returning the wrong variant; break thebeforebound and point-in-time evaluation uses today's device; return""instead ofNoneand the caller cannot tell "no device" from a resolved one.TestFeatureFlagEvaluationReasonscases and all 18TestFeatureFlagTestEvaluationcases guard the untouched call path.What I did not verify:
uv run mypy --cache-fine-grained .was OOM-killed in this environment. Scopedmypyondevice_bucketing.pyandflags_service.pypasses. CI's full run is the real gate.device_idreaching the service boundary; they mock the service itself.hogli ci:preflight --fixreports 0 failures. It advises merging master in, since master moved after this branch was cut. Master changed no serializer and none of the regenerated files, so there is no OpenAPI drift.Automatic notifications
Docs update
The device bucketing docs describe SDK behavior, which is unchanged. No update needed.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code. A PostHog support engineer asked whether the Flags tab could be made to work after a customer reported the mismatch; the diagnosis and this fix came out of that request. The DRI should self-assign, as this session had no way to verify a GitHub handle for them.
Skills invoked:
/improving-drf-endpoints(the new query param and itshelp_text),/writing-tests(the regression justifications above),/writing-user-facing-copy(tooltip),/writing-code-comments,/writing-pr-descriptions.The investigation started from the assumption that the tab was hashing the wrong identifier, which is what the symptom suggests. Reading the engine showed it was not hashing at all: the condition is skipped outright when no device id is present, and
OutOfRolloutBoundis a stand-in reason. That reframed the fix from "pass the right identifier" to "supply the missing input, and be honest when it cannot be supplied", which is where the no-fallback decision came from. Scope grew once by one endpoint, after findingtest_evaluationhad the identical gap.Public artifact: this work drew on a customer support conversation. No part of this PR reproduces it. All test data is invented (
user-1,example-flag,device-from-events), no customer, account, ticket, or operational figure is referenced anywhere in the diff or this description, and the linked GitHub issue is a public one found by search.