perf(frontend): load the hog vm only for tables that use it - #110094
trunk-io[bot] merged 8 commits into
Conversation
|
FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review 🦔 PostHog Review reviewed this pull requestNothing worth raising this time. Enjoy the moment: |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
applyVisualizationType |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:512 |
28 | 10 |
columns |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:2071 |
21 | 10 |
mergeChartSettings |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:407 |
17 | 10 |
computeConditionalFormattingBackground |
frontend/src/queries/nodes/DataVisualization/Components/Table.tsx:182 |
13 | 10 |
compareTableCells |
frontend/src/queries/nodes/DataVisualization/Components/Table.tsx:69 |
11 | 10 |
formatDataWithSettings |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:160 |
11 | 10 |
convertTableValue |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:203 |
11 | 10 |
toFriendlyClickhouseTypeName |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:242 |
11 | 10 |
<anonymous> |
frontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.ts:1561 |
11 | 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 — 3% of added code lines are comments (5 of 149)
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/queries/nodes/DataVisualization/dataVisualizationLogic.ts |
4 | 67 |
frontend/src/scenes/insights/stories/SQLTableConditionalFormatting.stories.tsx |
1 | 2 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +425.0 KiB (+0.6%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 70.65 MiB · 🔺 +425.0 KiB (+0.6%)
| File | Size | Δ vs base |
|---|---|---|
exporter/src/lib/hog.js |
435.1 KiB | 🔺 +435.1 KiB (new) |
render-query/src/render-query/render-query.js |
20.22 MiB | 🔺 +1.5 KiB (+0.0%) |
posthog-app/src/lib/hog.js |
1.0 KiB | 🔺 +1.0 KiB (new) |
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 | █████████░ 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 change | █████████░ 88.7% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.75 MiB · 2,486 files | 🔺 +635 B (+0.0%) | █████████░ 92.9% of 8.34 MiB |
dashboard scenesrc/scenes/dashboard/Dashboard.tsx |
11.68 MiB · 4,477 files | 🟢 -617.2 KiB (-4.9%) | █████████░ 86.7% of 13.48 MiB |
project home scenesrc/scenes/project-homepage/ProjectHomepage.tsx |
14.42 MiB · 5,347 files | 🟢 -573.0 KiB (-3.7%) | █████████░ 87.7% of 16.44 MiB |
events scenesrc/scenes/activity/explore/EventsScene.tsx |
10.91 MiB · 4,136 files | 🟢 -617.2 KiB (-5.2%) | █████████░ 86.4% of 12.64 MiB |
replay detail scenesrc/scenes/session-recordings/detail/SessionRecordingDetail.tsx |
13.77 MiB · 5,043 files | 🟢 -573.0 KiB (-3.9%) | █████████░ 87.6% 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 |
| 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.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 |
| 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 |
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.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 |
| 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 |
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 |
| 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 |
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.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 |
| 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 |
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 |
| 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 |
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 change | ████░░░░░░ 38.3% 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 |
|---|---|
| 828.7 KiB | dist/toolbar/toolbar-app-JMWOYNWY.css |
| 656.8 KiB | dist/toolbar/chunk-chunk-GGHNBOXG.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-7EIYIDXQ.js |
| 138.2 KiB | dist/toolbar/chunk-chunk-5LHZX24R.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-MYVEDPG7.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-W4A23KPO.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-7CMZVTBI.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 — 🟢 -1.3 KiB (-0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 964.78 MiB · 🟢 -1.3 KiB (-0.0%)
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f363ab5a22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Note 🤖 Automated comment by QA Swarm — not written by a human Multi-perspective review: router (cheap-first pass) + delegated lenses (qa-team, paul-reviewer, xp-reviewer, security-audit, engineering-systems-thinking as warranted) Verdict: ✅ APPROVE (round 1 @ f363ab5)The lazy hog load follows the hook rules, guards the null path, and cannot loop. The jest mock still applies to the dynamic import, and the story selector matches the inline cell style. Key findingsNone. ConvergenceNone. Only the router ran. Reviewer summaries
Automated by QA Swarm — not a human review |
f363ab5 to
ba5c5a2
Compare
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe data visualization logic loads HogVM when the effective visualization is a table with conditional-formatting rules. The table skips conditional-formatting background computation when HogVM is unavailable or the cell is a transposed header. It runs formatting bytecode through Priority: ➖ Normal Merge Risk: 🔵 Low · up to Failed loads receive retries and a reload recovery. Changing formatting rules during an outstanding load can still start duplicate import work, creating a bounded loading inefficiency; overall merge risk is low. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change delays browser code loading without a demonstrated change to execution permissions or data access. Load failures leave tables readable or reject the existing asynchronous widget operation. Overlapping loads and recovery remain bounded uncertainties. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 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.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 95a07011-02a3-43de-8584-856c63463736
📒 Files selected for processing (3)
frontend/src/queries/nodes/DataVisualization/Components/Table.tsxfrontend/src/scenes/insights/stories/SQLTableConditionalFormatting.stories.tsxproducts/notebooks/frontend/ReusableWidget/reusableWidgetBindings.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.
|
|
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/nodes/DataVisualization/dataVisualizationLogic.ts-2029-2034 (1)
2029-2034: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear
hogVmLoadErrorafter a successful load.
loadHogVmstores the loaded VM but does not clearhogVmLoadError.Tablethrows that stale error before it useshogVm, so a later successful load cannot recover the table.Suggested fix
try { - actions.setHogVm(await retryImport(() => import('lib/hog'))) + actions.setHogVm(await retryImport(() => import('lib/hog'))) + actions.setHogVmLoadError(null) } catch (error) { actions.setHogVmLoadError(error) }Do not guard retries on
hogVmLoadError, because that would prevent recovery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 90f13d55-233c-44f2-9acf-300c4d6f32f9
📒 Files selected for processing (3)
frontend/src/queries/nodes/DataVisualization/Components/Table.tsxfrontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.test.tsfrontend/src/queries/nodes/DataVisualization/dataVisualizationLogic.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.
|
Note 🤖 Automated comment by QA Swarm — not written by a human Verdict: ✅ APPROVE after fixes (round 2 @ 145af6e, fixed in 0b9df12)Key findings
Reviewer summaries
Automated by QA Swarm — not a human review |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Note 🤖 Automated comment by QA Swarm — not written by a human Verdict: ✅ APPROVE (round 3 @ b204742)Re-review of the follow-up fixes since round 2: the warning banner, Key findingsNone. Reviewer summaries
Automated by QA Swarm — not a human review |
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
Contained frontend performance change (lazy-loading the Hog VM) outside risky territory. Helpers exist, failure path is handled, tests cover the selector, and no unresolved substantive reviewer concerns remain.
- Author wrote 0% of the modified lines and has 13 merged PRs in these paths (familiarity MODERATE).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 91L, 4F substantive, 162L/5F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (162L, 5F, two-areas, perf) |
| stamphog 2.3.1 | .stamphog/policy.yml @ b204742 · reviewed head b204742 |
The SQL table imported lib/hog to run conditional formatting rules, which put the Hog VM, crypto-browserify, bn.js, elliptic and luxon on the static graph of every scene that renders an insight. The table now imports lib/hog when it has formatting rules, and the reusable widget bindings import it when they run. The formatting story now waits for a colored cell, so the snapshot cannot fire before the rules apply. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
A table whose Hog VM chunk fails after retries now throws that error during render, so an error boundary shows it and a stale chunk reloads the page, instead of quietly leaving the formatting off. The reusable widget bindings load the VM through retryImport, as the table does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
The table held the Hog VM module and its load error in React state. The load now lives in dataVisualizationLogic: a subscription on the formatting rules starts it, and a listener imports the module through retryImport. The table reads the module from the logic and still throws a load error during render. A test checks that only a table with rules loads the VM. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
Throwing the load error in render did not reach ChunkLoadErrorBoundary, because the nearer query ErrorBoundary caught it and replaced the whole table. Its "Try again" also stayed broken, because the logic outlives the table and kept the error. The table now stays on screen and shows a warning banner with a "Reload page" action. Errors that are not chunk-load errors go to error tracking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
Saved formatting rules stay in the query after a switch to a chart, so a chart view also downloaded the Hog VM. A needsHogVm selector now requires the table view as well as rules, and the subscription watches it. A successful load also clears an earlier load error, so the warning banner goes away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
kea-typegen wrote the `typeof import('lib/hog')` alias as an absolute path in
the generated block, so CI's typegen pass changed the file and the schema
diff checks failed. A named local interface with a type-only import keeps
the generated block stable, and the import is erased at build time.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-By: PostHog Desktop
Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
After a failed load, removing every formatting rule left the warning banner up, offering a reload for formatting that was no longer in use. A hogVmLoadFailed selector now requires needsHogVm as well as a load error, and the table shows the banner from it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
With the Auto visualization and no columns yet, the effective type resolves to a table, so a query with formatting rules started the Hog VM download before its data showed it was a chart. needsHogVm now treats Auto as undecided until columns arrive. An explicit table view still starts the load at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: acec8cb2-f000-4747-a659-05b72b42513e
b204742 to
65fccf0
Compare
|
This pull request was merged into |
This branch was successfully deployed
1 participant
Problem
lib/hogstatically to run conditional formatting rules.crypto-browserify,bn.js,elliptic,luxonandreadable-stream.Refs #104853
Changes
lib/hogafter it renders. Its colored cells appear a moment later.lib/hog.dataVisualizationLogic: aneedsHogVmselector (table view and rules) drives a subscription, and a listener imports the module throughretryImport.lib/hogthroughretryImport, inside the function that runs the binding.How did you test this code?
Table.test.tsandreusableWidgetBindings.test.tspass.SQLTableConditionalFormatting. CI runs it.Test rationale: No new test. The
SQLTableConditionalFormattingstories cover the lazy load. If the rules never apply,waitForSelectortimes out and the visual test fails.Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Opus 5.5 (
claude-opus-5-5)lib/hogimporters on the dashboard graph./writing-tests,/writing-pr-descriptions.gh pr list --state openfound no PR that does this.Created with PostHog Desktop
🤖 Generated with Claude Code