Conversation
A query can now choose the native JSON events table or the legacy one without flipping the instance settings. When the modifier is unset, the settings decide as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
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
|
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
posthog/hogql_queries/insight_actors_query_options_runner.py:32 |
posthog/hogql_queries/insight_actors_query_runner.py:51 |
16 | 97 |
✅ 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 — 4% of added code lines are comments (4 of 101)
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 |
|---|---|---|
posthog/hogql_queries/query_runner.py |
2 | 7 |
frontend/src/queries/schema/schema-general.ts |
1 | 2 |
posthog/hogql/printer/test/test_printer.py |
1 | 19 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +906 B (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.88 MiB · 🔺 +906 B (+0.0%)
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.57 MiB · 22 files | no change | █████████░ 85.2% 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 · 628 files | no change | █████████░ 88.8% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.37 MiB · 2,326 files | no change | █████████░ 88.4% 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 |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 267.6 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.4 KiB | src/lib/api.ts |
| 85.5 KiB | src/products.tsx |
| 69.1 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 63.9 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.3 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 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 |
| 100.4 KiB | src/lib/api.ts |
| 98.5 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 |
| 85.5 KiB | src/products.tsx |
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.37 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.37 MiB · 19 files | no change | ████░░░░░░ 41.4% of 5.72 MiB |
| Deferred (lazy) | 2.10 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 |
|---|---|
| 791.8 KiB | dist/toolbar/toolbar-app-TDLXUZLE.css |
| 650.8 KiB | dist/toolbar/chunk-chunk-OLZXKW3U.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-LP5DDLVQ.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-FVYKO6VU.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-M6TDBA3J.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-GL4SRUHV.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-Z4YQYAC3.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 — 🔺 +5.0 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 944.92 MiB · 🔺 +5.0 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17630.1 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 196.2 KB |
| action | 454.1 KB | 196.2 KB |
| action-list | 564.2 KB | 196.2 KB |
| cohort | 453.1 KB | 196.2 KB |
| cohort-list | 563.2 KB | 196.2 KB |
| email-template | 452.9 KB | 196.2 KB |
| error-details | 469.1 KB | 196.2 KB |
| error-issue | 453.8 KB | 196.2 KB |
| error-issue-list | 564.1 KB | 196.2 KB |
| experiment | 561.3 KB | 196.2 KB |
| experiment-list | 564.9 KB | 196.2 KB |
| experiment-results | 566.3 KB | 196.2 KB |
| feature-flag | 566.8 KB | 196.2 KB |
| feature-flag-list | 570.5 KB | 196.2 KB |
| feature-flag-testing | 457.3 KB | 196.2 KB |
| inline-scan | 453.6 KB | 196.2 KB |
| insight-actors | 562.3 KB | 196.2 KB |
| invite-email-preview | 452.3 KB | 196.2 KB |
| llm-costs | 559.3 KB | 196.2 KB |
| session-recording | 455.3 KB | 196.2 KB |
| survey | 454.7 KB | 196.2 KB |
| survey-global-stats | 561.9 KB | 196.2 KB |
| survey-list | 564.9 KB | 196.2 KB |
| survey-stats | 561.9 KB | 196.2 KB |
| trace-span | 453.5 KB | 196.2 KB |
| trace-span-list | 564.1 KB | 196.2 KB |
| vision-observation-list | 563.3 KB | 196.2 KB |
| workflow | 453.4 KB | 196.2 KB |
| workflow-list | 563.5 KB | 196.2 KB |
| loops-review | 457.8 KB | 196.2 KB |
| query-results | 774.1 KB | 196.2 KB |
| render-ui | 857.0 KB | 196.2 KB |
| visual-review-snapshots | 457.9 KB | 196.2 KB |
✅ Django migration risk — no migrations to analyze
We analyzed the migrations for potential risks.
Summary: 0 Safe | 0 Needs Review | 0 Blocked
Last updated: 2026-09-28 07:04 UTC (8e6dc05)
|
This PR should not merge until funnel correlation honors the source funnel’s explicit events-schema selection. Reviews (1) · Last reviewed commit: "feat(hogql): add a useNewEventsSchema qu..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe change adds the optional Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The implementation has no demonstrated runtime failure, but the public descriptions are incomplete and a meaningful override path lacks regression coverage. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Queries can choose either events table regardless of rollout settings. This enables comparisons, but it also means operators cannot rely on those settings alone to stop reads from the new table. Native-table readiness and rollback guarantees need confirmation. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
frontend/src/queries/schema/schema-general.ts-533-533 (1)
533-533: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the team fallback and regenerate both schema artifacts.
When
useNewEventsSchemais unset, the team modifier is checked before the instance setting and team allowlist. Update the TypeScript source, then regeneratefrontend/src/queries/schema.jsonandposthog/schema.py; changing only this JSDoc leaves both checked-in descriptions stale.Suggested source update
- /** Read events from the native JSON events table (`true`) or the legacy events table (`false`). When unset, the `CLICKHOUSE_HOGQL_USE_NEW_EVENTS_SCHEMA` instance settings decide. */ + /** Read events from the native JSON events table (`true`) or the legacy events table (`false`). When unset, the team modifier is used first; if it is also unset, the `CLICKHOUSE_HOGQL_USE_NEW_EVENTS_SCHEMA` instance setting or team allowlist decides. */Regenerate with
schema:build:json, followed byschema:build:python.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: fb57af1b-2675-485e-ba0a-9ee5b144e3ab
⛔ Files ignored due to path filters (3)
products/customer_analytics/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/dashboards/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/product_analytics/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (13)
frontend/src/queries/schema.jsonfrontend/src/queries/schema/schema-general.tsposthog/hogql/context.pyposthog/hogql/printer/test/test_printer.pyposthog/hogql_queries/ai/ai_table_resolver.pyposthog/hogql_queries/ai/event_taxonomy_query_runner.pyposthog/hogql_queries/ai/session_query_runner.pyposthog/hogql_queries/property_values_query_runner.pyposthog/hogql_queries/sessions_timeline_query_runner.pyposthog/models/event/new_events_schema.pyposthog/schema.pyproducts/product_analytics/backend/hogql_queries/funnels/funnel_correlation_query_runner.pyservices/mcp/src/api/generated.ts
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.
|
|
A funnel correlation query has no modifiers of its own, and neither does the persons modal options query, so both ran with team defaults and could read a different events table than the insight they came from. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
Forwarding *args alongside extract_modifiers could pass that argument twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
posthog/schema.py-5804-5805 (1)
5804-5805: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the canonical schema source, not
posthog/schema.py.The team allowlist is a valid fallback when the modifier is unset and the instance setting is false.
posthog/schema.pyis generated from the frontend schema, so changing it directly does not fix the frontend description or R1.Suggested fix
- /** Read events from the native JSON events table (`true`) or the legacy events table (`false`). When unset, the project's value for this modifier applies. If the project has no value, the `CLICKHOUSE_HOGQL_USE_NEW_EVENTS_SCHEMA` instance settings decide. */ + /** Read events from the native JSON events table (`true`) or the legacy events table (`false`). When unset, the project's value for this modifier applies. If the project has no value, the `CLICKHOUSE_HOGQL_USE_NEW_EVENTS_SCHEMA` instance setting or team allowlist determines selection. */Apply this change in
frontend/src/queries/schema/schema-general.ts, then regeneratefrontend/src/queries/schema.jsonandposthog/schema.py. Regenerateservices/mcp/src/api/generated.tsthroughhogli build:openapi.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 96e7541b-2b32-40ce-9df8-bc37052828f2
⛔ Files ignored due to path filters (3)
products/customer_analytics/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/dashboards/frontend/generated/api.schemas.tsis excluded by!**/generated/**products/product_analytics/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
frontend/src/queries/schema.jsonposthog/hogql/printer/test/test_printer.pyposthog/hogql_queries/insight_actors_query_options_runner.pyposthog/schema.pyservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 2 remain after this review.
A test outside the product must not run its query runners. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
products/product_analytics/backend/hogql_queries/funnels/test/test_funnel_correlation_actors.py (1)
76-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest propagation of the explicit
Falseoverride.The test checks only
useNewEventsSchema=True. A wrapper that dropsFalsewould still pass. Parameterize the modifier value withTrueandFalse, and assert that the runner retains the selected value. As per path instructions, “Prefer parameterised tests over near-duplicate test functions.”Also applies to: 92-92
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 0c10f4f5-507e-4cbe-b9f4-8133008221e1
📒 Files selected for processing (1)
products/product_analytics/backend/hogql_queries/funnels/test/test_funnel_correlation_actors.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 0 remain after this review.
This branch has not been deployed
1 participant
Problem
Nobody can run a single query against the native JSON events table today. The only switches are two instance settings, one for every team and one for listed teams. Comparing one query on both tables means flipping a setting that every query of that team, or of the whole instance, follows within a minute.
Changes
A query now picks its events table with the
useNewEventsSchemamodifier. True reads the native table, false reads the legacy one, and unset leaves the choice to the instance settings as before. The modifier wins over both settings, so it works in either direction during a rollout.HogQL resolves the choice once per query context, so one query never mixes tables. Six query runners pick the table outside that context: property values, sessions timeline, event taxonomy, the AI session query, the AI events fallback, and funnel correlation. Each now reads the modifier too. Modifiers are part of the query cache key, so results from the two tables never share a cache entry.
A funnel correlation query and the persons modal options query carry no modifiers of their own, so they ran with team defaults. They now take the modifiers of the funnel or insight they come from, the way actor queries already did. That covers every modifier, not only this one, so a funnel that sets modifiers now gets correlation results computed under them.
Team modifiers fill any query modifier left unset. Setting this one on a team therefore moves that team without the allowlist. Team admins can write team modifiers through the API, so a customer could opt in or out this way. Queries whose insight sets no modifiers behave as before.
How did you test this code?
The two instance-setting tests in the printer suite now check both directions. A modifier set to false keeps a query on the legacy table while the setting is on. A modifier set to true moves a query to the native table for a team outside the allowlist. Together they catch the modifier being ignored, including a truthiness check that would drop false.
A parameterized case in the insight actors suite checks that a correlation query, a correlation actors query and an actors options query each take
useNewEventsSchemafrom the source funnel. It fails if any of them falls back to team defaults. It was not run locally because Postgres was down. A script without a database showed all three dropping the modifier before the fix and keeping it after.Ran locally: those tests in both schema modes, and the property values, sessions timeline, event taxonomy, AI table resolver, AI session and funnel correlation suites in legacy mode. Not run: the rest of the backend suites, which CI covers.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release status
Automatic notifications
Docs update
None. No existing doc covers the events table switch.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5)The ask was a per-query way to test queries on the native table without touching instance settings. A toggle in the /debug modifiers panel was drafted and left out to keep this PR to the modifier. The query JSON and the API already accept it.
Skills invoked:
/writing-code-comments,/writing-tests,/writing-user-facing-copy,/reviewing-with-coderabbit,/writing-pr-descriptions.CodeRabbit CLI, one
--deeprun: one finding, on the dropped /debug toggle (no way to go back to unset). It does not apply to this diff. A search found no other open PR adding this modifier.🤖 Generated with Claude Code