feat(sql-editor): add compact worksheet filters and ranges - #111027
mariusandra wants to merge 4 commits into
Conversation
🤖 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 |
|---|---|---|---|
<anonymous> |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3730 |
76 | 10 |
createQueryTab |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3881 |
72 | 10 |
parseBIEditorState |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:426 |
38 | 10 |
getBIFilterSummary |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:674 |
22 | 10 |
filterExpression |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:719 |
22 | 10 |
createTab |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:2052 |
21 | 10 |
runQuery |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:2263 |
21 | 10 |
saveAsViewSubmit |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:2560 |
20 | 10 |
getBIChartFit |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:322 |
18 | 10 |
updateInsight |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3038 |
18 | 10 |
discardChanges |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3125 |
18 | 10 |
runAfterChange |
frontend/src/scenes/data-warehouse/editor/bi/biEditorLogic.ts:860 |
17 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:4209 |
16 | 10 |
<anonymous> |
products/data_warehouse/frontend/bi/BIFilterValueInput.tsx:73 |
16 | 10 |
parseBIFieldValue |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:394 |
15 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:485 |
15 | 10 |
buildBIQuery |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:986 |
15 | 10 |
persistEditorDraft |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:1815 |
15 | 10 |
BIFilterValueInput |
products/data_warehouse/frontend/bi/BIFilterValueInput.tsx:16 |
14 | 10 |
renderQueryOutline |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:166 |
13 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.tsx:3553 |
13 | 10 |
addFieldToConfig |
frontend/src/scenes/data-warehouse/editor/bi/biEditorLogic.ts:96 |
11 | 10 |
⚠️ Duplication (Python) — 12 new duplicated blocks (worst 177 tokens)
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.
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/tests/e2e/test_import_data.py:139 |
products/warehouse_sources/backend/temporal/data_imports/tests/e2e/test_import_data.py:259 |
47 | 177 |
products/warehouse_sources/backend/temporal/data_imports/sources/acculynx/source.py:17 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_systems_manager/source.py:22 |
15 | 139 |
products/warehouse_sources/backend/temporal/data_imports/sources/acculynx/source.py:18 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_compute_optimizer/source.py:22 |
14 | 137 |
products/warehouse_sources/backend/temporal/data_imports/sources/acculynx/source.py:18 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_sagemaker/source.py:23 |
14 | 137 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_sagemaker/source.py:113 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_systems_manager/source.py:114 |
35 | 131 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_ses/source.py:152 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_systems_manager/source.py:117 |
33 | 119 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_sagemaker/source.py:113 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_security_hub/source.py:127 |
28 | 103 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_sagemaker/source.py:114 |
28 | 101 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_glue_data_catalog/aws_glue_data_catalog.py:111 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_sagemaker/aws_sagemaker.py:123 |
12 | 90 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_compute_optimizer/aws_compute_optimizer.py:57 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_glue_data_catalog/aws_glue_data_catalog.py:70 |
11 | 84 |
products/autoresearch/backend/inference/sandbox.py:622 |
products/autoresearch/backend/inference/scoring.py:577 |
13 | 76 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_compute_optimizer/aws_compute_optimizer.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_glue_data_catalog/aws_glue_data_catalog.py:113 |
11 | 70 |
✅ 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 — 🔺 +9.2 KiB (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 69.80 MiB · 🔺 +9.2 KiB (+0.0%)
| File | Size | Δ vs base |
|---|---|---|
exporter/src/exporter/scenes/ExporterNotebookScene.js |
3.91 MiB | 🔺 +7.3 KiB (+0.2%) |
render-query/src/render-query/render-query.js |
18.76 MiB | 🔺 +1.2 KiB (+0.0%) |
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.63 MiB · 22 files | 🔺 +659 B (+0.0%) | █████████░ 88.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.72 MiB · 661 files | 🔺 +701 B (+0.0%) | █████████░ 92.4% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.59 MiB · 2,409 files | 🔺 +3.3 KiB (+0.0%) | █████████░ 91.0% of 8.34 MiB |
dashboard scenesrc/scenes/dashboard/Dashboard.tsx |
9.67 MiB · 3,392 files | 🔺 +2.0 KiB (+0.0%) | ███████░░░ 71.8% of 13.48 MiB |
today home pathsrc/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx |
7.61 MiB · 2,417 files | 🔺 +3.3 KiB (+0.0%) | █████████░ 88.6% of 8.58 MiB |
events scenesrc/scenes/activity/explore/EventsScene.tsx |
9.29 MiB · 3,244 files | 🔺 +2.0 KiB (+0.0%) | ███████░░░ 73.5% of 12.64 MiB |
replay detail scenesrc/scenes/session-recordings/detail/SessionRecordingDetail.tsx |
12.12 MiB · 4,130 files | 🔺 +2.0 KiB (+0.0%) | ████████░░ 77.1% 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
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.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 |
| 220.3 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.7 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 |
| 29.0 KiB | ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js |
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 |
| 220.3 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.1 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.7 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 |
| 220.3 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.1 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.7 KiB | src/products.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.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 |
| 220.3 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.1 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.7 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/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 |
| 220.3 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.1 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.7 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 |
| 220.3 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.1 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 | 🔺 +701 B (+0.0%) | ████░░░░░░ 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 |
|---|---|
| 836.2 KiB | dist/toolbar/toolbar-app-5MSXT3MN.css |
| 657.5 KiB | dist/toolbar/chunk-chunk-SLUTN3OK.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-7JWMBALG.js |
| 138.2 KiB | dist/toolbar/chunk-chunk-JZ43POKQ.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-LJ7FFVU6.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-P52PYKZI.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-BO2MEODF.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 — 🔺 +145.8 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 960.52 MiB · 🔺 +145.8 KiB (+0.0%)
|
[Medium risk] Adds filter operators and UI for worksheet data filtering. The PR appears safe to merge, with a non-blocking calculated-measure column-naming issue. Reviews (2) · Last reviewed commit: "fix(sql-editor): validate numeric worksh..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
84686da to
9ee12a5
Compare
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughBI worksheet filters now support multi-value inclusion and exclusion, inclusive ranges, and a disabled state that retains filter settings. The editor renders controls for these filters and loads distinct-value suggestions on demand. Filter changes integrate with worksheet state and URL persistence. Added tests, Storybook stories, layout adjustments, and handbook documentation cover the updated behavior. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Quick filters now validate numeric bounds and block invalid runs, so no merge-blocking risk remains. The only open item is a minor tooltip formatting inconsistency. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected paths preserve source scoping and reject invalid numeric filters before normal worksheet execution. No introduced security issue was established, but server-side authorization and compatibility when reverting to an older version were not fully verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 (2)
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts-669-672 (1)
669-672: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRange bounds on numeric fields send any string that is not a number as a string literal.
literalfalls back toescapeHogQLStringfor numeric fields when the input is not a finite number. For example, a user can typeabcinto arevenuerange. The generated SQL is thenproperties.revenue >= 'abc'. ClickHouse either rejects the query or compares it as a string, so the range silently returns wrong rows. The scalar path behaves the same way, butbetweenuses a free-textLemonInputwith no number validation. Reject the invalid bound, or skip it and show the input as invalid.products/data_warehouse/frontend/bi/BIFilterValueInput.tsx-16-18 (1)
16-18: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReload suggestions when the query changes while the selector is open.
BIFilterValueInputcallsloadOptions()only fromLemonInputSelect.onFocus. A changed query creates a logic state withoptions === null, butLemonInputSelectkeeps the popover open and does not fireonFocuswhenoptionschanges. The menu can therefore show empty or stale suggestions until the user focuses the selector again.Trigger loading when the popover opens or when its query changes. The query excludes the current filter, so remove the claim that selecting this filter's own values changes the key. The inspected code also does not establish that old Kea instances remain mounted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 8a548893-2384-4e2b-ba87-bdf578ca8caa
📒 Files selected for processing (14)
docs/published/handbook/engineering/data-warehouse.mdfrontend/src/scenes/data-warehouse/editor/SQLEditorScene.stories.tsxfrontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsxfrontend/src/scenes/data-warehouse/editor/bi/biEditorLogic.tsfrontend/src/scenes/data-warehouse/editor/bi/biEditorOptions.tsxfrontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.test.tsfrontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.tsfrontend/src/scenes/data-warehouse/editor/bi/components/BIFilterEditor.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIFilterPill.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIFiltersCard.tsxfrontend/src/scenes/data-warehouse/editor/sqlEditorLogic.test.tsproducts/data_warehouse/frontend/bi/BIFilterControl.tsxproducts/data_warehouse/frontend/bi/BIFilterValueInput.tsxproducts/data_warehouse/frontend/bi/biFilterValuesLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.
9ee12a5 to
85d889a
Compare
|
Risk: No findings This increment is presentational: it adds a compact filter summary ( Sentinel reviewed |
Comments Outside DiffThese findings could not be posted inline.
|
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
mariusandra
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
products/data_warehouse/frontend/bi/BIFilterControl.tsx (1)
68-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove duplicated
betweentooltip formatting.The tooltip builds its own range string from raw
filter.valueandfilter.valueTo.getBIFilterSummaryalready formatsbetweenranges. The tooltip shows raw ISO strings for date-time fields, while the button shows formatted dates.Use the full-precision raw bounds only if that is intended. Otherwise use
summaryfor all operators.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: d4136880-5a1d-43eb-8973-7f72753838c9
📒 Files selected for processing (11)
docs/published/handbook/engineering/data-warehouse.mdfrontend/src/scenes/data-warehouse/editor/SQLEditorScene.stories.tsxfrontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsxfrontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.tsfrontend/src/scenes/data-warehouse/editor/bi/components/BIFilterEditor.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIFilterPill.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIFiltersCard.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIMarksCard.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIShelfCard.tsxfrontend/src/scenes/data-warehouse/editor/bi/components/BIShowMe.tsxproducts/data_warehouse/frontend/bi/BIFilterControl.tsx
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
|
👋 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. Install the Visual Review Chrome extension to see visual review results at the top of your pull requests. |
This branch was successfully deployed
1 participant
Problem
BI worksheets need category selection and ranges without sacrificing space for fields and results. Builds on calculated measures in #110995.
Changes
Current layout: eight filters at 1600px and 1050px, using synthetic Storybook data.
Earlier screenshots, superseded by the compact layout above
Original single-value filter editor:
Earlier category selection layout:
Date range and narrow layout
Earlier numeric validation layout:
How did you test this code?
Local checks: scoped query/editor Jest suites, frontend lint/format, typecheck, and CI preflight.
Playwright exercised suggestions, multi-selection, clear-all, disable/re-enable, date changes, numeric ranges including zero, and manual Run with auto-update disabled. Screenshots show the rendered controls at 1600px and 1050px.
Test rationale: Extended the existing query-generation tests for escaping, range bounds, suggestion scoping, and persisted-state validation. Extended the URL-restoration test for filter toggles and operator conversion.
Regression cases cover large integers, high-precision decimals, invalid numeric values, and preventing stale-query execution after validation fails.
Chromium verified exact large-integer bounds, invalid range/list errors, blocked automatic/manual runs, recovery after correction, and wide/narrow error layouts.
The compact layout was checked with eight filters, editing from both sides, toggles, operator changes, numeric validation, and a 520px worksheet container.
Live ClickHouse and external warehouse execution were not tested; browser query responses use synthetic fixtures.
Release status
Automatic notifications
Docs update
Updated the existing data warehouse engineering guide with filter behavior.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Codex, GPT-6
Skills: stacking-prs, writing-ui-components, writing-kea-logics, placing-product-frontend-code, writing-user-facing-copy, writing-tests, writing-clickhouse-queries, adopting-generated-api-types, setting-feature-flags-in-storybook, running-ci-preflight, reviewing-with-coderabbit, writing-pr-descriptions.
The layout reference informed control types only. Committed fixtures and uploaded screenshots contain invented sample data, with no reference image content.
Local CodeRabbit review skipped: CLI signed out.