Skip to content

feat(warehouse-suggestions): suggested models strip and catalog suggested rows - #113447

Merged
trunk-io[bot] merged 38 commits into
masterfrom
thiagosalvatore/warehouse-suggestions-frontend
Oct 23, 2026
Merged

trunk-io[bot] merged 38 commits into
masterfrom
thiagosalvatore/warehouse-suggestions-frontend

Conversation

@thiagosalvatore

@thiagosalvatore thiagosalvatore commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Warehouse suggestions have an API but no UI, so nobody sees them.
  • This adds the two surfaces the design review chose: the views tab on /models and the certifications tab in the data catalog.

Changes

Behind the warehouse-suggestions flag:

  • Models tab: one slim, orange-tinted banner above the views table: "4 views could save ~2 hours of query time and 2.73 TB of scanning a month", with the totals in green and a "Review 4" button.
    • Review opens a dialog with one row per view: the saving in one aligned column, reads and people, and the data age after materializing.
    • Unmaterialize suggestions are in the same list. Their row shows the saving (the refresh cost minus the estimated cost of computing the view on each read) and how long one read takes. "Stop materializing" opens a dialog with the refresh cost now, the estimated cost when computed on read, and the saving.
    • Hovering or focusing "214 reads · 6 people" shows reads by surface as a bar, days read, and the query-log caveat.
    • Rows are sorted by what they save, time savers first.
  • Catalog: the same banner ("3 certification suggestions: 2 to certify · 1 to mark deprecated") above the filters, and the same dialog with Certify or Mark deprecated and Dismiss. Deprecate rows say "No reads recorded in 30 days" and carry the caveat.
    • Accepting a deprecate for a view now also stops its refreshes and deletes its stored table by default. The backend does this only when the view still materializes and no other view, sync, direct source or reader access depends on it, checked again at accept time.
    • So Mark deprecated on a view opens a dialog with "Stop refreshing it" (the default) and "Keep refreshing it". Keep sends keep_materialized: true, so a person can deprecate a view and keep it materialized. A table deprecation stays one click.
    • The toast reads created_asset.stopped_materialization: "…and stopped its refreshes" when it did, otherwise "Marked X as deprecated."
  • Materialize dialog: the data age follows the picked schedule, and the saving is labeled as an estimate at the suggested settings.
    • The hint under the schedule gives the backend's reason for the suggested interval (refresh_interval_basis): sources sync no faster, people read no more often, it is the fastest schedule, or it is the shortest one that saves more than it costs. A shorter pick shows a warning with the same reason.
    • A "Refresh mode" section reuses IncrementalConfigOptions from the SQL editor, starting from the proposed mode, key, unique key and lookback. It runs check_incremental on the view's query to list the key candidates.
    • Materialize sends refresh_interval_seconds plus only the refresh options that differ from the proposal, so an unchanged suggestion is accepted as proposed.
    • When the backend refuses any of these options, the dialog stays open and shows its message.
  • Incremental proposals say "Incremental by <key>" in the row.
  • After an action: the row collapses out over 150 ms, and the toast offers Undo (dismiss) or View (accept). View opens the view in the SQL editor after a materialize or unmaterialize.
  • A 409 on accept shows the backend's reason, such as a query that changed since the suggestion or another view that now reads it. The confirm dialog closes and the list reloads.
    • Undo calls the existing resume endpoint, so a dismiss is never delayed and closing the tab never loses it.
    • View closes the dialog. On the catalog it then clears the filters, opens the page that holds the new certification, highlights it and scrolls to it.
    • The last handled row shows "All caught up" in the dialog. The banner goes away once nothing is left.
  • Data age is qualified when a source has no sync schedule. A live view still reads "Data up to 1 day old" once materialized daily.
  • Keyboard focus moves to the next row's main action after a row leaves, then to the section header.
  • Rows and banners use the colored product icons from the sidebar: Models for views, Data warehouse for tables, Data catalog on the catalog banner.
  • Error, warming-up and not-eligible states each have their own one-line notice.
  • New events: warehouse suggestion review opened (banner to dialog), and warehouse suggestion confirm opened and warehouse suggestion confirm cancelled for the deprecate and stop-materializing dialogs.
  • The dialog computes data age with the backend's formula from backend/logic/candidates/materialize.py (interval plus today's source age). If the backend changes it, dataAgeLabel needs the same change.
  • Mechanical: merges of master (typed evidence and no score, the refresh mode and accept overrides, then the deprecate stop and the unmaterialize kind from feat(warehouse-suggestions): stop refreshes when accepting a deprecate #121459 and feat(warehouse-suggestions): suggest unmaterializing rarely read views #121460), and lockfile syncs of this package's importer to master's catalog versions.

Suggested models banner
Review dialog with materialize and unmaterialize suggestions
Stop materializing dialog
Catalog banner
Catalog review dialog
Deprecate dialog
Materialize dialog with an incremental proposal
Materialize dialog with a longer schedule and an unscheduled source

How did you test this code?

Test rationale:

  • warehouseSuggestionsLogic.test.ts, extended. Each new case guards one regression:
    • rows out of order;
    • a bytes saver counted toward the time total;
    • a leaving row still counted, or the section unmounting while the last row leaves;
    • a failed dismiss hidden later by its own leave timer (mutation-checked);
    • a second same-reason toast deduped away with its Undo;
    • Undo failing after a reload, or restoring a row whose resume failed;
    • View going to the wrong place, leaving the dialog open over the table, or offered with no certification id;
    • the schedule warning firing on equal or longer picks;
    • the dialog closing when the backend refuses any refresh option;
    • Materialize sending unchanged proposal options, or dropping the keys when a person switches between full and incremental;
    • the incremental check not running when the dialog opens;
    • an unmaterialize row missing from the models list or sorted by the wrong saving;
    • "Keep refreshing" not sent, or sent inverted, and a toast that misreports whether refreshes stopped;
    • a 409 on accept hiding the backend's reason, or leaving the confirm dialog open;
    • View after an unmaterialize going anywhere but the SQL editor.
  • suggestionCopy.test.ts (new): data age for known, live and unscheduled sources, and a hint for each interval reason.
  • certificationsLogic.test.ts: View for a row on page 2 opens page 2 of the certifications table, with the filters and sort cleared.
  • Storybook: the banner, the review dialog and the state notices on both surfaces, rendered headless at 1000 px and 520 px. The incremental and longer-schedule dialogs were rendered after the refresh-mode change. The deprecate and stop-materializing dialogs and the mixed review list were rendered at 1300 px and 520 px.
  • Local app on a private database, driven headless before the banner change: dismiss then Undo (server dismissed, then proposed), the schedule warning, certify then View with a filter on, and the last row from the keyboard. The rows and actions are unchanged since; the banner and review dialog were checked in Storybook only.
  • Not run: a deprecate or unmaterialize accept against a real backend. The logic tests cover the request bodies and toasts with a mocked API.
  • Not run: a successful materialize. The local stack has no materialization backend, so accept hit the existing error path: an error toast, the dialog stays open, the row stays.

Release status

  • This change is behind a feature flag and is not available to users

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

  • The first version showed suggestions as prose cards in a grey panel; a design review replaced it with a collapsible section of rows. That section still took too much space, so it became the banner and dialog after comparing three mockups (banner, inline table tags, compact cards). Earlier screenshots in this description were replaced each time.
  • /plan-design-review (20 decisions) and /plan-eng-review (4 decisions, plus a Codex outside voice) shaped the plan. The eng review found the resume endpoint, so Undo needs no delayed send, and the outside voice found the toast dedupe and the last-row unmount.
  • Skills invoked: /writing-ui-components, /writing-user-facing-copy, /writing-tests, /using-kea-disposables, /browse, /plan-design-review, /plan-eng-review, /writing-pr-descriptions.
  • Left out: the paused state, re-expanding on a new batch, and loading a view's allowed schedules into the picker.

🤖 Generated with Claude Code

@thiagosalvatore thiagosalvatore self-assigned this Oct 7, 2026
@thiagosalvatore
thiagosalvatore added this pull request to stack #113448 October 7, 2026 15:42
@github-actions

github-actions Bot commented Oct 7, 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) — 5 functions above the limit (max 53)

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
describerFor frontend/src/lib/components/ActivityLog/describers.tsx:56 53 10
CertificationSuggestionRow products/warehouse_suggestions/frontend/components/CertificationSuggestionRow.tsx:27 14 10
MaterializeSuggestionModal products/warehouse_suggestions/frontend/components/MaterializeSuggestionModal.tsx:25 14 10
changedRefreshOptions products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts:126 12 10
<anonymous> products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts:651 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) — 1 new duplicated block (worst 138 tokens)

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.

First copy Second copy Lines Tokens
products/warehouse_suggestions/frontend/components/MaterializeSuggestionRow.tsx:31 products/warehouse_suggestions/frontend/components/UnmaterializeSuggestionRow.tsx:36 32 138
⚠️ Bundle size — 🔺 +16.9 KiB (+0.0%)

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

Total: 76.17 MiB · 🔺 +16.9 KiB (+0.0%)

File Size Δ vs base
posthog-app/_parent/products/data_modeling/frontend/ModelsScene.js 83.6 KiB 🔺 +9.8 KiB (+13.3%)
posthog-app/_parent/products/data_catalog/frontend/DataCatalogScene.js 38.6 KiB 🔺 +5.1 KiB (+15.1%)

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
1017.3 KiB · 24 files 🔺 +933 B (+0.1%) █████████░ 86.8% of 1.14 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
2.92 MiB · 620 files 🔺 +1.0 KiB (+0.0%) ██████████ 96.2% of 3.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.28 MiB · 2,543 files 🔺 +1.2 KiB (+0.0%) ██████████ 97.2% of 7.50 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.67 MiB · 3,706 files 🔺 +1.3 KiB (+0.0%) ██████████ 96.5% of 10.01 MiB
today home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
7.45 MiB · 2,635 files 🔺 +1.2 KiB (+0.0%) ██████████ 96.3% of 7.74 MiB
AI-first home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
7.53 MiB · 2,616 files 🔺 +1.2 KiB (+0.0%) ████████░░ 84.0% of 8.96 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.14 MiB · 3,497 files 🔺 +1.2 KiB (+0.0%) ██████████ 95.3% of 9.59 MiB
web analytics scene
src/scenes/web-analytics/WebAnalyticsScene.tsx
10.00 MiB · 3,715 files 🔺 +1.2 KiB (+0.0%) █████████░ 92.3% of 10.83 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
8.21 MiB · 3,077 files 🔺 +1.2 KiB (+0.0%) █████████░ 86.7% of 9.47 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ 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/zod/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/billing/billingLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/ProductEmptyState/productSetupPreloadLogic.ts 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
🟢 products/dashboards/frontend/widgets/previews/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 products/wizard/frontend/runs/WizardRunDetailsDrawer.tsx stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/layout/today/TodayShell.tsx stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/layout/today/TodayContextPanelFloating.tsx stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/layout/today/TodaySidePanel.tsx stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/layout/today/TodayTopBar.tsx stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/layout/today/todayRailItems.tsx stays out of src/scenes/AuthenticatedShell.tsx
🟢 zod/v4/locales/de.js stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/queries/validators.js stays out of src/scenes/dashboard/Dashboard.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
🟢 src/queries/Query/Query.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 packages/quill/packages/charts/ stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 products/posthog_ai/frontend/components/tool/widgets/VisualizationWidget.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
🟢 products/posthog_ai/frontend/components/tool/widgets/RecordingsWidget.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
🟢 src/queries/Query/Query.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
🟢 products/posthog_ai/frontend/api/primitives.ts stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
🟢 src/scenes/notebooks/Notebook/notebookLogic.ts stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/activity/explore/EventsScene.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/activity/explore/EventsScene.tsx
🟢 src/queries/validators.js stays out of src/scenes/activity/explore/EventsScene.tsx
🟢 src/scenes/web-analytics/LiveMetricsDashboard/LiveWebAnalyticsMetrics.tsx stays out of src/scenes/web-analytics/WebAnalyticsScene.tsx
🟢 products/product_analytics/frontend/insights/trends/WorldMap/countryVectors.tsx stays out of src/scenes/web-analytics/WebAnalyticsScene.tsx
🟢 src/scenes/settings/SettingsMap.tsx stays out of src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
🟢 @jspm/core/nodelibs/browser/fs stays out of src/scenes/session-recordings/detail/SessionRecordingDetail.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/[email protected]/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/[email protected]/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@[email protected]/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/[email protected]/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/[email protected]/node_modules/base64-js/index.js
1.4 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
1.3 KiB src/lib/utils/bootTiming.ts
912 B ../node_modules/.pnpm/[email protected]/node_modules/ieee754/index.js
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
69.4 KiB src/lib/lemon-ui/icons/icons.tsx
38.7 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/[email protected]_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
29.4 KiB src/scenes/scenes.ts
27.2 KiB src/lib/constants.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/[email protected]/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@[email protected]_@[email protected]/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
143.2 KiB ../packages/quill/packages/primitives/dist/index.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/[email protected]/node_modules/prosemirror-view/dist/index.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/[email protected]/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@[email protected]_@[email protected]/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/[email protected]/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@[email protected]_@[email protected]/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
143.2 KiB ../packages/quill/packages/primitives/dist/index.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/[email protected]/node_modules/prosemirror-view/dist/index.js
Largest files eagerly shipped from src/scenes/web-analytics/WebAnalyticsScene.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
183.2 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
143.2 KiB ../packages/quill/packages/primitives/dist/index.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
Size File
322.5 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/module.mjs
316.1 KiB ../node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/posthog-js/dist/rrweb.js
286.8 KiB src/taxonomy/core-filter-definitions-by-group.json
238.5 KiB ../packages/quill/packages/quill/dist/index.js
220.9 KiB ../node_modules/.pnpm/@[email protected][email protected][email protected][email protected]/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/[email protected]/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.production.min.js
106.3 KiB src/products.tsx
98.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/[email protected]/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.29 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.29 MiB · 18 files 🔺 +979 B (+0.0%) ████░░░░░░ 40.0% of 5.72 MiB
Deferred (lazy) 2.11 MiB · 40 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
902.3 KiB dist/toolbar/toolbar-app-M4RZCOGY.css
692.2 KiB dist/toolbar/chunk-chunk-72MXZPGR.js
260.7 KiB dist/toolbar/chunk-chunk-L6XVLVLY.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
130.4 KiB dist/toolbar/chunk-chunk-C5NTA2OX.js
74.8 KiB dist/toolbar/toolbar-app-3CZKXE2Y.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-HSGY7DL5.js
21.7 KiB dist/toolbar/chunk-chunk-VFPSU72W.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 — 🔺 +454.2 KiB (+0.0%)

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

Total: 1022.85 MiB · 🔺 +454.2 KiB (+0.0%)

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • create experiment via wizard, add metrics, and launch (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

✅ Django migration risk — no migrations to analyze

No Django migrations need risk analysis.

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Adds warehouse suggestions UI and feature flag.

Fix the missing suggestions, failed-status recovery, dismissal race, and repository-rule violations before merging.

Reviews (1) · Last reviewed commit: "chore(frontend): sort the feature flag t..." · Reviewed by Greptile

Comment thread products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts Outdated
Comment thread products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts Outdated
Comment thread products/warehouse_suggestions/frontend/components/MaterializeSuggestionCard.tsx Outdated
Comment thread products/warehouse_suggestions/frontend/components/SuggestedModelsStrip.tsx Outdated
Comment thread products/warehouse_suggestions/frontend/components/SuggestedModelsStrip.tsx Outdated
@trunk-io

trunk-io Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Experiment lifecycle › creation and launch › create experiment via wizard, add metrics, and launch The test failed because waiting for the URL to change timed out after 15 seconds. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

The frontend adds warehouse suggestion loading, status handling, acceptance and dismissal actions, and materialization controls. A feature flag enables suggestion views in data modeling and the catalog. The change also adds suggestion activity descriptions, evidence and duration formatting, interval mappings, tests, and Storybook examples.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 916c4

The suggestion UI sits behind a feature flag. Some freshness and savings copy may read oddly or understate data age. Merging carries low risk, and these copy fixes can follow.

Security architecture risk: 🔵 Low · up to 916c4

The new actions retain project and subject permissions, server-side schedule validation, and safeguards against repeated decisions. No security bypass was demonstrated. Uncertainty remains around interrupted requests, navigation, and downstream recovery.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — Reachable state changes include scheduled materialization of editable saved queries and certification or deprecation of editable views or tables. An authorized user can act on multiple accessible subjects, but each traced request is resolved under its project and subject permissions.

Trust Boundaries and Controls

  • observed — Browser-supplied suggestion IDs and refresh intervals cross into an API protected by authentication, API scopes, project membership, feature enablement, and subject authorization. Team-scoped lookup prevents a stale suggestion ID submitted under a different project from directly selecting that suggestion for mutation.

Resilience and Maintainability Implications

  • observed — The existing acceptance transition locks the team-scoped suggestion, permits new acceptance only from proposed state, and returns an already accepted suggestion without repeating its acceptor. Dismissal also locks the row and rejects disallowed or repeated decisions. These controls contain concurrent decision races; they do not alone prove atomic recovery of downstream external jobs.
Pre-merge checks: Passed 1
Check name Status Explanation
Description check Passed The pull request description is complete and follows the repository template. It includes the problem, user-visible changes, screenshots, testing scope and rationale, release status, docs status, agen…
Full details: Description check

Explanation

The pull request description is complete and follows the repository template. It includes the problem, user-visible changes, screenshots, testing scope and rationale, release status, docs status, agent context, invoked skills, and out-of-scope items. It also clearly identifies tests that were not run.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (3)
products/warehouse_suggestions/frontend/components/MaterializeSuggestionModal.tsx (1)

23-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Block modal dismissal while the accept request is in flight.

The Cancel button is disabled while inFlight is true. Esc, an overlay click, and the X button still call closeMaterializeModal. If the request then fails with a refused schedule, materializeError is set on a modal that is already closed, and the user never sees it. Pass closable={!inFlight} to LemonModal.

products/warehouse_suggestions/frontend/suggestionCopy.ts (1)

42-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle the case where both savings are zero.

If saves_seconds_per_month is below 60 and saves_bytes_per_month is 0, humanizeBytes(0) returns "0 bytes". The card then says "saves about 0 bytes of scanning". Fall back to "under a minute of query time", or hide the saving clause.

products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts (1)

218-238: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Load every proposed suggestion page.

The list endpoint caps each response at 100 items. The loader requests one page and keeps only results. If an enabled team has more than 100 proposed suggestions, later actionable suggestions do not reach surfaceSuggestions, so users cannot see or act on them.

Suggested fix
-                    loadSuggestions: async () =>
-                        (await warehouseSuggestionsList(projectId(), { status: 'proposed' })).results,
+                    loadSuggestions: async () => {
+                        const suggestions: WarehouseSuggestionApi[] = []
+                        let offset = 0
+                        let page
+                        do {
+                            page = await warehouseSuggestionsList(projectId(), {
+                                status: 'proposed',
+                                offset,
+                            })
+                            suggestions.push(...page.results)
+                            offset += page.results.length
+                        } while (page.results.length > 0 && suggestions.length < page.count)
+                        return suggestions
+                    },

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: b48924da-4d65-4223-9629-e927e3c7ae37
📥 Commits

Reviewing files that changed from the base of the PR and between 71ced80 and a2acd1d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • frontend/src/lib/components/ActivityLog/describers.tsx
  • frontend/src/lib/constants.tsx
  • frontend/src/types.ts
  • products/data_catalog/frontend/tabs/CertificationsTab.tsx
  • products/data_modeling/frontend/ViewsTab.tsx
  • products/warehouse_suggestions/frontend/components/DismissSuggestionMenu.tsx
  • products/warehouse_suggestions/frontend/components/MaterializeSuggestionCard.tsx
  • products/warehouse_suggestions/frontend/components/MaterializeSuggestionModal.tsx
  • products/warehouse_suggestions/frontend/components/SuggestedCertificationsTable.stories.tsx
  • products/warehouse_suggestions/frontend/components/SuggestedCertificationsTable.tsx
  • products/warehouse_suggestions/frontend/components/SuggestedModelsStrip.stories.tsx
  • products/warehouse_suggestions/frontend/components/SuggestedModelsStrip.tsx
  • products/warehouse_suggestions/frontend/components/WhyThisSuggestion.tsx
  • products/warehouse_suggestions/frontend/refreshIntervals.ts
  • products/warehouse_suggestions/frontend/suggestionCopy.ts
  • products/warehouse_suggestions/frontend/warehouseSuggestionActivityDescriber.tsx
  • products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.test.ts
  • products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts
  • products/warehouse_suggestions/package.json

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

@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from a2acd1d to 0814268 Compare October 7, 2026 16:47
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 0814268 to 916c49e Compare October 7, 2026 18:29

@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.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
products/warehouse_suggestions/frontend/suggestionCopy.ts (1)

63-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix spokenDuration for values that are not whole multiples of the units.

When parts.length reaches maxUnits, the loop keeps iterating. This is harmless for output. But a duration with a remainder below one minute, such as 90 seconds, prints "1 minute". The 30 seconds are dropped without any rounding. Example: freshnessSentence with freshness_after_seconds of 3659 prints "1 hour" for a value that is 59 seconds over. This understates the age of the data by up to 59 seconds, and by up to a full unit when maxUnits cuts off a lower unit. Use ceiling rounding for the last kept unit, or state the value as "about". The copy "up to X old" is an upper bound. Truncation therefore shows a bound that is too low.

Also, the input 0 or a value under 60 seconds returns 'under a minute'. For freshnessSentence, spokenDuration(payload.refresh_interval_seconds, 1) then reads "Refreshes every under a minute". Handle this case in the sentence.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 9ce3612c-ed3e-404d-b91c-048c781e31f3
📥 Commits

Reviewing files that changed from the base of the PR and between 0814268 and 916c49e.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • products/warehouse_suggestions/frontend/components/MaterializeSuggestionModal.tsx
  • products/warehouse_suggestions/frontend/suggestionCopy.ts

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

@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 916c49e to a2ab980 Compare October 7, 2026 18:52
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch 2 times, most recently from a277878 to 4d51206 Compare October 8, 2026 06:58
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 4d51206 to 4deba8e Compare October 8, 2026 10:36
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 4deba8e to 8920252 Compare October 8, 2026 12:52
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 8920252 to 360e7c9 Compare October 8, 2026 13:13
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 360e7c9 to 4fbb9d4 Compare October 8, 2026 13:36
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 4fbb9d4 to 0558044 Compare October 8, 2026 14:36
@thiagosalvatore
thiagosalvatore force-pushed the thiagosalvatore/warehouse-suggestions-frontend branch from 0558044 to 6be3abc Compare October 8, 2026 16:46
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 23, 2026 13:07

@posthog posthog 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.

PostHog Review · Standard · reviewed ee1faca: 2 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

Comment thread products/data_catalog/frontend/certificationsLogic.tsx
thiagosalvatore and others added 5 commits October 23, 2026 10:19
…ng it

After a materialize accept, the views list reloaded once, but the
view's is_materialized flip lands asynchronously. A reload that came
too early kept the view labeled as a plain view, without Sync now.

suggestionAccepted(viewId, startsMaterializing) replaces reloadViews.
For a materialize accept it adds the view to
dataWarehouseViewsLogic's materializingViewIds before reloading, which
is the polling flow the materialize button already uses: the list
reloads every 5 s until the view reports it is materialized. An
unmaterialize accept only reloads, because its revert is synchronous.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Accepting a certify or deprecate suggestion reloaded only the
certifications list. Badges elsewhere read certification state from
the shared schema in databaseTableListLogic, so they kept the old
status, such as Certified after a deprecation.

A suggestionAccepted action now reloads the list and forces a schema
refresh with loadDatabase({ force: true }), the same as the existing
certification review and revoke actions.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…s, recover from 404s

Catalog edit access
- can_act describes access to the subject. Accepting a certify or
  deprecate suggestion also needs data_catalog editor access
  (_require_catalog_edit_access in the facade), so a catalog viewer
  saw an enabled Certify button that always returned 403.
- The accept button now also checks userHasAccess for data_catalog at
  editor level. Dismiss still only needs subject access.
- data_catalog is in the backend's ACCESS_CONTROL_RESOURCES, so the app
  context already carries its level. The frontend
  AccessControlResourceType enum gains DataCatalog, and the Storybook
  app context gains a data_catalog level so stories render enabled
  buttons.

Pending dialogs
- Escape, the overlay and the close icon closed the materialize dialog
  while its accept was in flight. A refused option then set an error in
  a hidden dialog, and reopening cleared it. The materialize,
  deprecate and stop-materializing dialogs set closable={!inFlight}.

404 on accept, dismiss or undo
- When a view or table is deleted after the list loads, the backend
  answers 404. Only 409 reloaded the list, so the row stayed
  actionable and every retry failed. isStale now covers 404 and 409,
  and a 404 explains that the suggestion or its subject is gone.

Table preview link
- escapePropertyAsHogQLIdentifier passes a name through when it starts
  and ends with a quote, so a stored table name like
  "x" UNION SELECT ... --" changed the preview query. subjectUrl now
  escapes each dot-separated part with
  escapeRawPropertyAsHogQLIdentifier, so demo.orders still resolves
  and a quoted name stays one identifier. It moves out of the row
  component so it can be tested directly.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…tification

A catalog opened from a ?tab=certifications&table=... link kept the
table parameter through the highlight navigation. The urlToAction
handler reapplies it as a search filter on the next URL change, so it
hid a certification for another table right after highlightCertification
cleared the filters. The highlight now removes table along with the
sort parameter.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
… is ineligible

When the incremental check said a view can no longer refresh
incrementally, IncrementalConfigOptions showed the ineligible banner
and a full refresh, but the draft kept the incremental proposal.
Materialize then sent no refresh_mode, accept applied the old
incremental config, and the backend answered 409.

The check handler now sets the draft to full refresh under the same
condition the component uses (not eligible, or no key candidates), so
Materialize sends refresh_mode: full.

The test file now resets the saved query and check mocks before each
test. clearAllMocks keeps implementations, so one test's ineligible
check leaked into the tests after it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@posthog posthog 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.

PostHog Review · Standard · reviewed 3ee3431: no findings. This pull request is large, so the review ran in 4 parts with less depth than usual.

@posthog

posthog Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🕓 This approval covered an earlier revision. There are new visual changes to review in the newer comment below.

✅ Visual changes approved by @thiagosalvatore — baseline updated in 52fd270.

View this run in PostHog

8 changed, 4 new.

Install the Visual Review Chrome extension to see visual review results at the top of your pull requests.

12 updated
Run: 088af3ff-396b-43c5-90c6-cabe2408bb86

Co-authored-by: thiagosalvatore <[email protected]>
@thiagosalvatore

Copy link
Copy Markdown
Contributor Author

@PostHog review --deep

@posthog

posthog Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

@thiagosalvatore PostHog Review is reviewing this pull request.

@posthog

posthog Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

PostHog Review · Deep · reviewed 52fd270

Posted 5 findings: 1 Should fix, 4 Consider.

Step Status Result
Prepare the diff done 36 files, 7 chunks
Pick perspectives done 3 perspectives
Run review passes done 13 issues (+2 blind-spot)
Merge overlapping findings done 15 → 11
Validate findings done 5 kept, 6 dismissed
Publish done 5 posted (view review)

@posthog posthog 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.

This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.

Comment thread products/data_modeling/frontend/viewsTabLogic.ts
Comment thread products/warehouse_suggestions/frontend/components/SuggestionsReviewModal.tsx Outdated
Comment thread products/warehouse_suggestions/frontend/warehouseSuggestionsLogic.ts Outdated
thiagosalvatore and others added 2 commits October 23, 2026 11:44
Accepting a suggestion can move a view out of the active type filter.
With eleven views under the Views filter on page 2, materializing the
one on page 2 leaves ten. currentPage stayed 2, visibleViews was empty,
and LemonTable, which does not clamp controlled pagination, showed "No
views match your filters." Unmaterializing under Materialized views did
the same.

After every saved-query reload, the logic now moves to the last page of
filteredViews when the current page is past it. This also covers a
delete or a polling reload that shrinks the list.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…s and check guards

Not now
- "Suggested again if reads grow" was wrong for most kinds. A not-now
  dismissal returns when its score reaches a multiple of the score at
  dismissal (_earns_reproposal), and each kind scores something else:
  certify scores reads, materialize and unmaterialize score the
  saving, and deprecate scores refresh cost.
- notNowCondition(kind) now gives the menu label and the toast the
  condition for that kind. It does not name the multiple, because that
  is a backend rule.
- "All caught up" no longer says new suggestions come "as reads build
  up", for the same reason.

Undo after unmount
- Switching tabs after a dismiss unmounted the logic while the toast
  stayed. Undo dispatched to a logic with no listener, so nothing ran
  and the suggestion stayed dismissed.
- The toast button now calls resumeDismissedSuggestion with the
  project id taken when the toast was made. It sends the resume
  request on its own, and updates suggestion state only through a
  logic that is still mounted (findMounted).

Focus after the last suggestion
- Handling the last suggestion removes the banner and its Review
  button, so the dialog returned focus to a detached button and focus
  fell to the page body.
- The review dialog now renders a focus anchor beside itself. On
  onAfterClose, if focus is on the body, the anchor takes it, so the
  next Tab continues from the surface.

Incremental check in flight
- Materialize was enabled while the incremental check loaded. A
  refusal could arrive first, and the check's draft seeding then
  cleared it through setRefreshDraft.
- materializeDisabledReason is now "Checking whether it can refresh
  incrementally" while the check loads, and confirmMaterialize already
  refuses while it is set. Tests that confirm now wait for the check.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@sinan-ku sinan-ku 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.

Really exciting!! Got some nits but happy to ship!

thiagosalvatore and others added 4 commits October 23, 2026 12:03
…efetch

The backend evaluates warehouse-suggestions for the organization
(logic/flags.py) and answers 403 unless the result is True. The flag
is released per organization, so the frontend and backend normally
agree, but they can still differ: a flag-service timeout makes
feature_enabled return None on the backend, and a personal override
in the toolbar turns the frontend on alone. Either way a 403 became a
permanent "Couldn't load..." error, and Retry could never succeed.

- A 403 from the list or status request now sets featureForbidden,
  and stripState is "hidden".
- The error state now shows only while the list or status has never
  loaded. A failed background reload, such as the one after a 409 or
  404, keeps the current list, so it no longer unmounts an open review
  or materialize dialog.
- A successful load clears loadError, not only reload.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
featureForbidden took the value of each failure on its own. When the
status request answered 403 and the list request then failed with a
500, the second failure reset featureForbidden to false, and both
surfaces showed "Couldn't load…" while the flag was off. The reducer
now keeps a 403 until Retry, and the test pairs a 403 with a 500.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ures

Master made realized_seconds_saved_per_month and
realized_bytes_saved_per_month required on WarehouseSuggestionApi, so
the typecheck of this branch merged with master failed on the story
and logic test fixtures. Both fields are null until a suggestion is
accepted, so the fixtures set them to null.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@posthog

posthog Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

👋 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.

…t succeeds

e6db462 made loadSuggestionsSuccess and loadStatusSuccess clear
loadError. When the list request failed and the status request
succeeded after it, the success cleared the error while the list was
still null, so stripState stayed "loading" and the strip rendered
nothing, with no notice and no Retry. Visual review caught it in the
LoadFailed story.

The error screen already shows only while the list or the status has
never loaded, so clearing on success is not needed. A failed
background reload still keeps the loaded list. The error test now
covers either request failing, with the other one settling after the
failure.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
thiagosalvatore and others added 2 commits October 23, 2026 14:16
…1.438.21

Master bumped posthog-js to 1.438.21 in #122426. That bump rewrote the
lockfile entry of every importer already on master, but not this
branch's products/warehouse_suggestions importer. The merge queue
builds this PR merged with master, so its Storybook build failed with
ERR_PNPM_OUTDATED_LOCKFILE and the queue removed the PR. The importer
now resolves the same posthog-js version as the other importers.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@trunk-io
trunk-io Bot merged commit bc651e0 into master Oct 23, 2026
233 checks passed
@trunk-io
trunk-io Bot deleted the thiagosalvatore/warehouse-suggestions-frontend branch October 23, 2026 18:25
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-23 18:54 UTC Run
prod-us ✅ Deployed 2026-10-23 19:10 UTC Run
prod-eu ✅ Deployed 2026-10-23 19:06 UTC Run

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants