Skip to content

fix(flags): surface real errors in the flag testing tab - #108975

Merged
trunk-io[bot] merged 5 commits into
masterfrom
posthog-self-driving/fixflags-surface-real-errors-in-the-46897c
Oct 1, 2026
Merged

trunk-io[bot] merged 5 commits into
masterfrom
posthog-self-driving/fixflags-surface-real-errors-in-the-46897c

Conversation

@posthog

@posthog posthog Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Users who test a flag in the testing tab often get only "Failed to evaluate flag". They cannot see what went wrong, so they retry and then give up.
  • Error tracking shows the main cause. When a condition has no variant override, the flags service sends variant: "". The response serializer rejects the blank value, and the catch-all turns the rejection into a generic 500.
  • Other causes also reach the same catch-all: connection resets to the flags service and 400 responses from it.
  • evaluationErrorMessage applies the "person not found" and timestamp rewrites only to detail messages. The endpoint sends its errors as error, so those rewrites never match.
  • Invalid groups JSON throws inside the loader. loadersPlugin.onFailure reports errors that have no status, so "Invalid JSON format for groups" goes to error tracking. The user already sees the message inline.

Origin

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

Changes

Failure Before After
Condition with a blank variant 500 "Failed to evaluate flag" 200 with the evaluation result
Flags service connection error or timeout 500 "Failed to evaluate flag" One retry, then 503 "Flag evaluation service temporarily unavailable. Please retry."
Flags service returns 429, 503 or 504 500 "Failed to evaluate flag" 503 "Flag evaluation service temporarily unavailable. Please retry." (not captured)
Flags service returns another error status, 400 included 500 "Failed to evaluate flag" 502 with the service status (captured)
Flags service returns a body that is not JSON 500 "Failed to evaluate flag" 502 "Unexpected response format from flag evaluation service" (captured)
Other requests failure, such as ChunkedEncodingError 500 "Failed to evaluate flag" 503 (captured)
API or MCP caller sends groups that is not a JSON object Service 400, shown as 500 "Failed to evaluate flag" 400 from the request serializer on groups
Response fails serializer validation 500 "Failed to evaluate flag" 502 "Unexpected response format from flag evaluation service" (still captured)
Invalid groups JSON (null included) or timestamp in the tab Loader throws, exception reported Inline testError, loader does not run
  • The variant field on FeatureFlagConditionAnalysisSerializer now allows a blank value. This removes the most frequent 500.
  • The flags service error mapping follows evaluation_reasons in the same file.
  • A service 400 counts as a fault on the PostHog side. Django builds the request body, and the request serializer now rejects a groups value that is not an object.
  • evaluationErrorMessage now applies every rewrite to one message. ApiError.message already holds the error field. The "invalid timestamp" rewrite matches only real invalid-timestamp errors. Specific reasons such as "did not exist at the specified timestamp" now show as they are.
  • A new submitTestEvaluation action checks the form first. It sets testError for invalid input, or else runs the single or the batch evaluation. The "Test" button in the tab now uses it.
  • Mechanical: the choice between single and batch evaluation moved from FeatureFlagTestingTab into the logic listener.

How did you test this code?

  • Ran TestFeatureFlagTestEvaluation in test_feature_flag.py and featureFlagTestingLogic.test.ts locally. Both pass.
  • New backend tests:
    • A blank condition variant returns 200. Before this change it returned 500.
    • Each flags service failure in the table returns its own status and message. Each case also checks whether the error is captured.
    • A response that fails serializer validation returns 502 and is captured. The older non-dict test exits before that branch.
    • groups as a string or a list returns 400 on groups.
    • The happy path asserts max_retries=1.
  • New and changed frontend tests:
    • Invalid groups or timestamp set testError and do not dispatch testFlagEvaluation. This catches a return of the throw inside the loader.
    • Backend error messages go through the friendly rewrites, and a specific timestamp reason is not rewritten.
    • JSON null groups set testError.
    • A person with merged distinct IDs runs the batch evaluation. This test fails when the listener's single-or-batch branch is flipped.
  • The frontend type check reports no errors in the changed files.
  • Not done: no manual test in a browser. The visible change is the message text in the existing error banner.

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. No doc under docs/ covers the flag testing tab errors.

🤖 Agent context

Autonomy: Fully autonomous

Agent: PostHog Desktop (Claude Code), claude-opus-5-5

  • Started from an inbox report about generic test evaluation failures. Error tracking identified the blank-variant serializer failure as the main cause of the 500s, so the fix covers that cause and does not only improve the message.
  • Duplicate search: no open PR changes the test_evaluation error handling.
  • Skills: writing-simplified-technical-english, writing-pr-descriptions.
  • Public artifact: test data is invented. No session material is in the diff.
  • Review follow-up, Claude Code (claude-opus-5-5): aligned the error mapping with evaluation_reasons, moved groups validation into the request serializer, and added the missing tests. Skills: improving-drf-endpoints, writing-tests, writing-code-comments, writing-pr-descriptions.

Created with PostHog Desktop from this inbox report.

🤖 Generated with Claude Code

- Accept a blank condition variant in the test evaluation response serializer. It failed validation and fell into the generic 500.
- Return specific errors when the flags service is unreachable (503), rejects the request (400), or fails (502). Retry one connection reset.
- Read the `error` field in evaluationErrorMessage, so the friendly rewrites match backend errors.
- Validate groups and timestamp before the loaders run, so invalid input sets testError and does not go to error tracking.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 1c61883c-4d71-4872-8504-7cafa662900a
@posthog posthog Bot added the self-driving label Sep 30, 2026
@trunk-io

trunk-io Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Sep 30, 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) — 1 function above the limit (max 33)

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.

Function Location Complexity Limit
FeatureFlagTestingTab frontend/src/scenes/feature-flags/FeatureFlagTestingTab.tsx:70 33 10
✅ 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.

⚠️ Comment density — 5% of added code lines are comments (17 of 350)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
frontend/src/scenes/feature-flags/featureFlagTestingLogic.ts 9 47
products/feature_flags/backend/api/feature_flag.py 6 79
frontend/src/scenes/feature-flags/featureFlagTestingLogic.test.ts 2 83

This check does not block merging. It updates on every push and clears when the share drops.

ℹ️ Bundle size — no base branch to compare

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

Total: 70.24 MiB (no base branch measurement to compare against yet)

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 base measurement) █████████░ 87.8% 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.57 MiB · 630 files (no base measurement) █████████░ 88.7% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.75 MiB · 2,486 files (no base measurement) █████████░ 92.9% of 8.34 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
91.9 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.6 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
109.9 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
91.9 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

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.19 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.19 MiB · 19 files (no base measurement) ████░░░░░░ 38.3% of 5.72 MiB
Deferred (lazy) 2.10 MiB · 44 files (no base measurement) n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB (no base measurement) █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
828.7 KiB dist/toolbar/toolbar-app-JMWOYNWY.css
656.9 KiB dist/toolbar/chunk-chunk-HCJ6OHTE.js
259.4 KiB dist/toolbar/chunk-chunk-TUDDG3HU.js
138.2 KiB dist/toolbar/chunk-chunk-DJSGW5RR.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-U3GEO6MT.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-LB3WNQSR.js
21.0 KiB dist/toolbar/chunk-chunk-J6LZJAFQ.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 — no base branch to compare

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

Total: 964.84 MiB (no base branch measurement to compare against yet)

ℹ️ 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.3 KB
action 454.1 KB 199.3 KB
action-list 564.2 KB 199.3 KB
cohort 453.1 KB 199.3 KB
cohort-list 563.2 KB 199.3 KB
email-template 452.9 KB 199.3 KB
error-details 469.6 KB 199.3 KB
error-issue 454.5 KB 199.3 KB
error-issue-list 564.8 KB 199.3 KB
experiment 561.3 KB 199.3 KB
experiment-list 564.9 KB 199.3 KB
experiment-results 566.3 KB 199.3 KB
feature-flag 566.8 KB 199.3 KB
feature-flag-list 570.5 KB 199.3 KB
feature-flag-testing 457.3 KB 199.3 KB
inline-scan 453.6 KB 199.3 KB
insight-actors 562.3 KB 199.3 KB
invite-email-preview 452.3 KB 199.3 KB
llm-costs 559.3 KB 199.3 KB
session-recording 455.3 KB 199.3 KB
survey 454.7 KB 199.3 KB
survey-global-stats 561.9 KB 199.3 KB
survey-list 564.9 KB 199.3 KB
survey-stats 561.9 KB 199.3 KB
trace-span 453.5 KB 199.3 KB
trace-span-list 564.1 KB 199.3 KB
vision-observation-list 563.3 KB 199.3 KB
workflow 453.4 KB 199.3 KB
workflow-list 563.5 KB 199.3 KB
loops-review 457.8 KB 199.3 KB
query-results 774.1 KB 199.3 KB
render-ui 858.1 KB 199.3 KB
visual-review-snapshots 457.9 KB 199.3 KB
✅ Playwright — all passed

All tests passed.

View test results →

✅ Backend coverage — all changed backend lines covered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (64 / 64)

All changed backend lines are covered ✅

Per-product line coverage (touched products)
Product Coverage Lines
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
demo ███████████░░░░░░░░░ 53.4% 1,445 / 2,707
data_tools ████████████░░░░░░░░ 61.2% 90 / 147
warehouse_sources_queue █████████████░░░░░░░ 65.9% 1,611 / 2,446
ai_gateway ███████████████░░░░░ 75.0% 9 / 12
aeo ███████████████░░░░░ 76.3% 617 / 809
batch_exports ████████████████░░░░ 81.2% 21,571 / 26,551
apm █████████████████░░░ 84.1% 1,306 / 1,553
cdp ██████████████████░░ 88.3% 4,559 / 5,164
ml_inference ██████████████████░░ 88.8% 539 / 607
mcp_analytics ██████████████████░░ 89.2% 5,038 / 5,651
product_tours ██████████████████░░ 89.3% 1,340 / 1,500
dashboards ██████████████████░░ 89.6% 6,924 / 7,727
notebooks ██████████████████░░ 90.2% 15,304 / 16,971
data_warehouse ██████████████████░░ 90.3% 14,298 / 15,839
signals ██████████████████░░ 90.3% 59,285 / 65,644
cohorts ██████████████████░░ 90.5% 8,534 / 9,434
streamlit_apps ██████████████████░░ 90.8% 2,684 / 2,956
managed_warehouse ██████████████████░░ 91.0% 10,252 / 11,263
data_modeling ██████████████████░░ 91.2% 10,562 / 11,584
tasks ██████████████████░░ 91.3% 78,937 / 86,495
exports ██████████████████░░ 91.7% 9,684 / 10,566
today ██████████████████░░ 91.8% 1,034 / 1,126
business_knowledge ██████████████████░░ 92.0% 8,446 / 9,180
engineering_analytics ██████████████████░░ 92.1% 11,458 / 12,436
ai_training ██████████████████░░ 92.2% 356 / 386
conversations ███████████████████░ 92.6% 29,137 / 31,466
early_access_features ███████████████████░ 92.6% 1,339 / 1,446
managed_migrations ███████████████████░ 92.7% 1,581 / 1,705
stamphog ███████████████████░ 92.8% 8,109 / 8,742
visual_review ███████████████████░ 92.9% 9,534 / 10,265
canvas ███████████████████░ 92.9% 7,155 / 7,703
approvals ███████████████████░ 93.0% 3,974 / 4,271
mcp_registry ███████████████████░ 93.1% 1,670 / 1,794
notifications ███████████████████░ 93.2% 1,144 / 1,228
error_tracking ███████████████████░ 93.2% 16,368 / 17,555
surveys ███████████████████░ 93.4% 6,644 / 7,113
slack_app ███████████████████░ 93.6% 14,560 / 15,557
autoresearch ███████████████████░ 93.6% 8,837 / 9,442
context_layer ███████████████████░ 93.8% 3,415 / 3,639
web_analytics ███████████████████░ 93.9% 23,680 / 25,229
billing_alerts ███████████████████░ 94.1% 2,094 / 2,226
mcp_store ███████████████████░ 94.3% 8,959 / 9,501
ai_observability ███████████████████░ 94.6% 24,974 / 26,409
alerts ███████████████████░ 94.6% 9,267 / 9,796
wizard ███████████████████░ 94.7% 6,150 / 6,496
workflows ███████████████████░ 94.7% 15,159 / 16,006
reminders ███████████████████░ 94.8% 760 / 802
review_hog ███████████████████░ 95.0% 11,623 / 12,235
annotations ███████████████████░ 95.1% 817 / 859
endpoints ███████████████████░ 95.1% 9,222 / 9,694
customer_analytics ███████████████████░ 95.2% 25,991 / 27,301
legal_documents ███████████████████░ 95.2% 2,311 / 2,427
posthog_ai ███████████████████░ 95.3% 2,491 / 2,614
marketing_analytics ███████████████████░ 95.3% 19,598 / 20,559
actions ███████████████████░ 95.5% 756 / 792
logs ███████████████████░ 95.5% 15,468 / 16,200
experiments ███████████████████░ 95.5% 32,931 / 34,481
data_catalog ███████████████████░ 95.5% 4,401 / 4,606
tracing ███████████████████░ 95.6% 3,536 / 3,699
replay_vision ███████████████████░ 95.6% 29,078 / 30,404
growth ███████████████████░ 95.7% 11,381 / 11,888
skills ███████████████████░ 95.8% 6,972 / 7,274
messaging ███████████████████░ 95.9% 3,824 / 3,989
product_analytics ███████████████████░ 96.0% 28,521 / 29,696
revenue_analytics ███████████████████░ 96.4% 1,889 / 1,959
access_control ███████████████████░ 96.5% 7,246 / 7,512
user_interviews ███████████████████░ 96.5% 2,870 / 2,974
feature_flags ███████████████████░ 96.6% 26,809 / 27,748
warehouse_sources ███████████████████░ 97.3% 468,087 / 480,989
data_quality ████████████████████ 97.5% 7,701 / 7,895
links ████████████████████ 97.9% 234 / 239
security ████████████████████ 98.0% 1,283 / 1,309
metrics ████████████████████ 98.0% 4,245 / 4,331
analytics_platform ████████████████████ 98.3% 2,784 / 2,833
pulse ████████████████████ 98.5% 2,046 / 2,078
live_debugger ████████████████████ 99.2% 626 / 631
field_notes ████████████████████ 99.4% 172 / 173

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 1c61883c-4d71-4872-8504-7cafa662900a
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

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: 793028ad-be04-4dc0-8a8d-d2fe72834eef

📥 Commits

Reviewing files that changed from the base of the PR and between b4c220b and 407703c.

📒 Files selected for processing (4)
  • frontend/src/scenes/feature-flags/featureFlagTestingLogic.test.ts
  • frontend/src/scenes/feature-flags/featureFlagTestingLogic.ts
  • products/feature_flags/backend/api/feature_flag.py
  • products/feature_flags/backend/api/test/test_feature_flag.py

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


📝 Walkthrough

Walkthrough

The frontend now validates feature-flag test input before starting single-ID or batch evaluation and formats evaluation errors. The backend validates groups, accepts blank condition variants, retries the flags-service request once, and maps serializer, connection, timeout, and upstream HTTP failures to API responses. Tests cover frontend validation and error messages, plus backend request validation, variant handling, and service-failure responses.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 40770

Invalid test inputs are stopped before evaluation, and service failures receive explicit responses. No remaining merge-blocking issue is established; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 40770

The changes preserve the existing project-scoped access configuration and improve validation and error handling. Only one additional service attempt is permitted. No introduced security vulnerability was established, but the effects of retrying a request that already completed remotely remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The additional outbound exposure is bounded to a second sequential evaluation request for the same selected project, identity and flag. The generated MCP change is documentation rather than new tool authority. Broader downstream propagation is not established by the incomplete graph evidence.

Trust Boundaries and Controls

  • observed — Caller-controlled groups face stricter server-side validation, while service credentials remain backend-selected and unchanged across retries. Newly exposed upstream information is limited to a numeric HTTP status for selected failures; these response paths do not return upstream bodies or detailed exception text.

Resilience and Maintainability Implications

  • inferred — Retry can hold a worker for approximately two proxy timeouts plus backoff. A timeout after remote completion can cause another identical POST, and no idempotency key is passed. The inspected caller and helper establish bounded local execution, but not whether remote side effects are absent or safely deduplicated.

Hardening Proposals

  • proposed — Document whether the remote evaluation operation is safe to repeat after timeout or interruption, including any billing, telemetry or persistent effects. If consequential effects exist, define deduplication or a narrower retry policy before extending retries to additional callers.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description includes the required Problem, Changes, testing, release status, documentation, and agent context sections. It explains user impact, behavior changes, test coverage, and known limitati…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 3c5ca3cf-3b90-48a2-87bb-ab7c9d0b1963

📥 Commits

Reviewing files that changed from the base of the PR and between dec1ca0 and 095e6c4.

⛔ Files ignored due to path filters (1)
  • products/feature_flags/frontend/generated/api.schemas.ts is excluded by !**/generated/**
📒 Files selected for processing (6)
  • frontend/src/scenes/feature-flags/FeatureFlagTestingTab.tsx
  • frontend/src/scenes/feature-flags/featureFlagTestingLogic.test.ts
  • frontend/src/scenes/feature-flags/featureFlagTestingLogic.ts
  • 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; 11 remain after this review.

Comment thread products/feature_flags/backend/api/feature_flag.py
@trunk-io

trunk-io Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Scenes-App/Notebooks/Nodes/Support Tickets WithTickets smoke-test The test timed out while waiting for loading indicators to disappear. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

@coderabbitai coderabbitai Bot left a comment

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.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Reject JSON null for groups. · featureFlagTestingLogic.ts:115-118

frontend/src/scenes/feature-flags/featureFlagTestingLogic.ts:115-118
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject JSON null for groups.

JSON.parse('null') returns null, but typeof null === 'object', so this check accepts it. The new submit guard then dispatches the evaluation loader with groups: null instead of setting testError. Reject parsed === null here.

Proposed fix
-        if (typeof parsed !== 'object' || Array.isArray(parsed)) {
+        if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) {

ℹ️ Review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 8ab94835-312e-494e-81c2-b96304e28cfe

📥 Commits

Reviewing files that changed from the base of the PR and between 095e6c4 and b4c220b.

📒 Files selected for processing (1)
  • frontend/src/scenes/feature-flags/featureFlagTestingLogic.ts

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

@gustavohstrassburger
gustavohstrassburger marked this pull request as ready for review October 1, 2026 17:19
Copilot AI balanced review requested due to automatic review settings October 1, 2026 17:19
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 1, 2026 17:20

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Malformed JSON responses from the flags service still produce the generic 500 instead of the intended 502.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Improves feature flag test evaluation errors across the backend and frontend.

Changes:

  • Accepts blank condition variants and classifies service failures.
  • Validates forms before loaders run.
  • Displays backend error messages with targeted rewrites.
File Description
services/​mcp/​src/​api/​generated.ts Updates generated variant documentation.
products/​feature_flags/​frontend/​generated/​api.schemas.ts Updates generated frontend schema documentation.
products/​feature_flags/​backend/​api/​test/​test_feature_flag.py Tests blank variants and service failures.
products/​feature_flags/​backend/​api/​feature_flag.py Adds retry and upstream error handling.
frontend/​src/​scenes/​feature-flags/​FeatureFlagTestingTab.tsx Routes submissions through logic validation.
frontend/​src/​scenes/​feature-flags/​featureFlagTestingLogic.ts Validates input and improves error mapping.
frontend/​src/​scenes/​feature-flags/​featureFlagTestingLogic.test.ts Tests validation and error messages.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

{"error": "Flag evaluation service temporarily unavailable. Please retry."},
status=status.HTTP_503_SERVICE_UNAVAILABLE,
)
except requests.exceptions.HTTPError as e:
@trunk-io
trunk-io Bot merged commit 1d83b7d into master Oct 1, 2026
315 checks passed
@trunk-io
trunk-io Bot deleted the posthog-self-driving/fixflags-surface-real-errors-in-the-46897c branch October 1, 2026 20:02
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-01 20:24 UTC Run
prod-us ✅ Deployed 2026-10-01 20:33 UTC Run
prod-eu ✅ Deployed 2026-10-01 20:34 UTC Run

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants