Skip to content

fix(flags): allow null property values in flag test evaluation - #110802

Draft
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixflags-allow-null-property-values-in-bcf466
Draft

posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixflags-allow-null-property-values-in-bcf466

Conversation

@posthog

@posthog posthog Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Users who click "Test" on a flag get a 502 when a condition has a property filter with no value, for example is_set. The MCP test-evaluation tool fails in the same way.
  • The flags service sends value: null for these filters, because PropertyAnalysis.value is a serde Value.
  • FeatureFlagConditionPropertyAnalysisSerializer.value rejected null, so response validation failed. Since fix(flags): surface real errors in the flag testing tab #108975 that failure returns a captured 502.

Origin

  • Replay Vision
  • Error tracking: issue
  • First signal: 2026-09-16
  • Inbox report: open
  • Task started by: auto-start, after the report was rated P3 and ready to fix

Changes

  • The response serializer now accepts a null property value. The test result for these flags now shows on the testing tab.
  • The other PropertyAnalysis and ConditionAnalysis fields cannot be null on the Rust side, so they need no change.
  • Mechanical: regenerated OpenAPI types (help text only).

How did you test this code?

  • Ran the TestFeatureFlagTestEvaluation class locally against Postgres and ClickHouse.
  • The extended test fails with 502 != 200 without the fix.
  • Not checked: the testing tab in a running app. The UI renders only explanation, so a null value does not reach the screen.

Test rationale: Extended test_test_evaluation_accepts_blank_condition_variant (renamed) with an is_set property whose value is null. It catches a serializer that rejects null property values again. It is the nearest test for nullable condition fields, and it already needs the endpoint round trip.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Fully autonomous

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

  • Skills invoked: /writing-tests, /improving-drf-endpoints, /writing-pr-descriptions.
  • No duplicate: the search for open PRs on this issue found nothing.
  • Public artifact: the test fixture is invented and contains no session material.

Created with PostHog Desktop from this inbox report.

🤖 Generated with Claude Code

The flags service sends a null `value` for property filters whose operator takes no value, such as is_set. The response serializer rejected it, so the test endpoint returned a 502.

Generated-By: PostHog Desktop
Task-Id: 63ba9e6c-1fd1-4f03-9965-63c40f81531b
@posthog posthog Bot added the self-driving label Oct 2, 2026
@trunk-io

trunk-io Bot commented Oct 2, 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 Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

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

✅ Complexity (TypeScript) — clean

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

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Bundle size — no change

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

Total: 69.57 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.62 MiB · 22 files no change █████████░ 88.1% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.58 MiB · 630 files no change █████████░ 88.8% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.79 MiB · 2,499 files no change █████████░ 93.4% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.64 MiB · 3,387 files no change ███████░░░ 71.6% of 13.48 MiB
project home scene
src/scenes/project-homepage/ProjectHomepage.tsx
13.99 MiB · 5,012 files no change █████████░ 85.1% of 16.44 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.26 MiB · 3,239 files no change ███████░░░ 73.3% of 12.64 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.08 MiB · 4,110 files no change ████████░░ 76.9% of 15.72 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
854 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
219.9 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
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
100.5 KiB src/lib/api.ts
92.0 KiB src/products.tsx
69.4 KiB src/lib/lemon-ui/icons/icons.tsx
40.1 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
28.7 KiB src/scenes/scenes.ts
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
219.9 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
153.7 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
110.0 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.0 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
219.9 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
181.8 KiB src/queries/validators.js
153.7 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
110.0 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.0 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/project-homepage/ProjectHomepage.tsx
Size File
315.5 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
219.9 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
181.8 KiB src/queries/validators.js
153.7 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
110.0 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
219.9 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
181.8 KiB src/queries/validators.js
153.7 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
110.0 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.0 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
Size File
315.5 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
219.9 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
181.8 KiB src/queries/validators.js
153.7 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
110.0 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/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 · 19 files no change ████░░░░░░ 38.4% of 5.72 MiB
Deferred (lazy) 2.11 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
833.8 KiB dist/toolbar/toolbar-app-W2MCORNU.css
657.2 KiB dist/toolbar/chunk-chunk-BB4ANHGT.js
259.4 KiB dist/toolbar/chunk-chunk-DWA3PXCS.js
138.2 KiB dist/toolbar/chunk-chunk-BJ3BCXPR.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-LYECKYON.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-ABI6IVA3.js
21.0 KiB dist/toolbar/chunk-chunk-L7CFD6VA.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +120 B (+0.0%)

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

Total: 957.76 MiB · 🔺 +120 B (+0.0%)

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

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

App JS CSS
debug 597.9 KB 199.4 KB
action 454.1 KB 199.4 KB
action-list 564.2 KB 199.4 KB
cohort 453.1 KB 199.4 KB
cohort-list 563.2 KB 199.4 KB
email-template 452.9 KB 199.4 KB
error-details 469.6 KB 199.4 KB
error-issue 454.5 KB 199.4 KB
error-issue-list 564.8 KB 199.4 KB
experiment 561.3 KB 199.4 KB
experiment-list 564.9 KB 199.4 KB
experiment-results 566.3 KB 199.4 KB
feature-flag 566.8 KB 199.4 KB
feature-flag-list 570.5 KB 199.4 KB
feature-flag-testing 457.3 KB 199.4 KB
inline-scan 453.6 KB 199.4 KB
insight-actors 562.3 KB 199.4 KB
invite-email-preview 452.3 KB 199.4 KB
llm-costs 559.3 KB 199.4 KB
session-recording 455.3 KB 199.4 KB
survey 454.7 KB 199.4 KB
survey-global-stats 561.9 KB 199.4 KB
survey-list 564.9 KB 199.4 KB
survey-stats 561.9 KB 199.4 KB
trace-span 453.5 KB 199.4 KB
trace-span-list 564.1 KB 199.4 KB
vision-observation-list 563.3 KB 199.4 KB
workflow 453.4 KB 199.4 KB
workflow-list 563.5 KB 199.4 KB
loops-review 457.8 KB 199.4 KB
query-results 774.1 KB 199.4 KB
render-ui 858.1 KB 199.4 KB
visual-review-snapshots 457.9 KB 199.4 KB

@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (12)
.agents/security.md — configured
.agents/skills/sending-notifications/SKILL.md — configured
services/mcp/AGENTS.md — auto-discovered
docs/published/handbook/engineering/type-system.md — configured
.agents/skills/implementing-mcp-tools/SKILL.md — configured
.agents/skills/writing-tests/SKILL.md — configured
docs/internal/person-data-access.md — configured
.agents/skills/adopting-generated-api-types/SKILL.md — configured
.agents/skills/implementing-mcp-ui-apps/SKILL.md — configured
docs/published/handbook/engineering/ai/implementing-mcp-tools.md — configured
.claude/commands/conventions.md — configured
.agents/skills/writing-code-comments/SKILL.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: d75d1467-c36b-47b6-975b-eacded4d8d9f

📥 Commits

Reviewing files that changed from the base of the PR and between d8d5ba9 and 7e217af.

⛔ Files ignored due to path filters (1)
  • products/feature_flags/frontend/generated/api.schemas.ts is excluded by !**/generated/**
📒 Files selected for processing (3)
  • products/feature_flags/backend/api/feature_flag.py
  • products/feature_flags/backend/api/test/test_feature_flag.py
  • services/mcp/src/api/generated.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The serializer now accepts explicit null values for feature flag condition properties and documents operators that take no value. The evaluation regression test checks that the response preserves a blank condition variant and a null property value. The generated API comment documents the same null-value behavior.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 7e217

is_set responses can legitimately contain value: null, and the response test and generated API description reflect that contract. No actionable merge-blocking risk remains.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 7e217

The change accepts a valid response representation without changing evaluation inputs, permissions, or flag decisions. The reviewed scope introduces no material security risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed exposure is successful delivery of an existing flag's evaluation analysis to callers already authorized for the endpoint. The inspected change does not widen project selection, add caller inputs, or introduce a privileged sink.

Trust Boundaries and Controls

  • observed — The service-response shape boundary remains enforced: the endpoint still validates the complete response and returns 502 for invalid shape. Allowing null in this one field does not remove endpoint authorization or request validation.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is complete and explains the 502 problem, user-visible fix, affected fields, tests, test rationale, release status, and agent context. It does not record the required CodeRabbit CLI pa…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@trunk-io

trunk-io Bot commented Oct 2, 2026

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant