Skip to content

fix(flags): evaluate device-bucketed flags in person and test tabs - #81618

Draft
posthog[bot] wants to merge 3 commits into
masterfrom
posthog/device-bucketing-flag-debug-surfaces
Draft

fix(flags): evaluate device-bucketed flags in person and test tabs#81618
posthog[bot] wants to merge 3 commits into
masterfrom
posthog/device-bucketing-flag-debug-surfaces

Conversation

@posthog

@posthog posthog Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Problem

Someone opening a person's Flags tab sees a device-bucketed flag evaluate to false with 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_id instead of distinct_id, so its value survives the anonymous-to-identified transition. SDKs send $device_id on every /flags call. 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, reporting OutOfRolloutBound. At 100% rollout that reason is impossible, so the label was actively misleading.

$device_id is not a person property, because one person can have several device ids, so it has to be read off the person's events.

Changes

  • The person profile Flags tab now evaluates device-bucketed flags with the most recent $device_id on that distinct ID's events, matching what the SDK sends.
  • A flag's Test evaluation tab had the same gap and is fixed on the same path. It is the surface we would otherwise recommend as the workaround.
  • get_flags_from_service gained a device_id argument, sent as a top-level request field. The service ignores a device id nested inside person_properties, which is the same trap as posthog-js#3318.
  • evaluation_reasons accepts an optional device_id query param to check a specific device.
  • Projects with no active device-bucketed flag skip the events query entirely, so the common case costs one indexed flag lookup.
  • The Match evaluation tooltip now explains the behavior and its one gap. Added copy: "Flags that use device bucketing are evaluated with the most recent $device_id on 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."
  • Regenerated OpenAPI artifacts for the new query param.

Two decisions worth a reviewer's attention:

  • Resolution is per distinct ID, not per person. $device_id is 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.
  • No fallback to distinct_id when 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 to None, and the tooltip covers that case instead.

Note

This does not add a distinct missing_device_id reason 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
products/feature_flags/backend/api/test/test_feature_flag.py::TestResolveDeviceId              3 passed
products/feature_flags/backend/api/test/test_feature_flag.py::TestFeatureFlagEvaluationReasons 14 passed
products/feature_flags/backend/api/test/test_feature_flag.py::TestFeatureFlagTestEvaluation    18 passed

Regressions each group catches, none of which an existing test covered:

  • TestFeatureFlagEvaluationReasons, 3 new parameterized cases: drop the resolution and device_id goes back to None, 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 the ORDER BY and evaluation hashes on a stale device, silently returning the wrong variant; break the before bound and point-in-time evaluation uses today's device; return "" instead of None and the caller cannot tell "no device" from a resolved one.
  • The 11 pre-existing TestFeatureFlagEvaluationReasons cases and all 18 TestFeatureFlagTestEvaluation cases guard the untouched call path.

What I did not verify:

  • No screenshot of the tooltip. The change is text-only and the added copy is quoted verbatim above. Capturing it needs a seeded person with flags plus a hover state, and there is no story that renders this table directly.
  • Repo-wide mypy did not complete. uv run mypy --cache-fine-grained . was OOM-killed in this environment. Scoped mypy on device_bucketing.py and flags_service.py passes. CI's full run is the real gate.
  • No end-to-end check against the live Rust service. The endpoint tests assert the device_id reaching the service boundary; they mock the service itself.
  • hogli ci:preflight --fix reports 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

  • Publish to changelog?

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 its help_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 OutOfRolloutBound is 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 finding test_evaluation had 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.

posthog Bot added 2 commits August 12, 2026 00:50
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
@trunk-io

trunk-io Bot commented Aug 12, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

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

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

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Bundle size — no change

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

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 budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.26 MiB · 22 files no change ███░░░░░░░ 28.0% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.36 MiB · 3,140 files no change █████████░ 86.1% of 9.71 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/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

@trunk-io

trunk-io Bot commented Aug 12, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

0 participants