Skip to content

feat(warehouse-suggestions): add the suggestion model and review api - #112705

Merged
trunk-io[bot] merged 6 commits into
masterfrom
thiagosalvatore/warehouse-suggestions-model-api
Oct 7, 2026
Merged

trunk-io[bot] merged 6 commits into
masterfrom
thiagosalvatore/warehouse-suggestions-model-api

Conversation

@thiagosalvatore

@thiagosalvatore thiagosalvatore commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • People who own warehouse views and tables get no prompt about which ones to certify, deprecate or materialize.
  • This PR is layer 1 of that feature. It adds the storage and the review API.
  • Layer 2 adds the daily job, layer 3 adds accept, and layer 4 adds the UI.
  • Layer 1 reads no ClickHouse, so it does not wait for the reads rollup in feat(clickhouse): add warehouse_object_reads_daily table #109209.

Changes

  • A new product, warehouse_suggestions, sits behind the warehouse-suggestions flag. Nothing a person sees changes until layer 4.
  • The list endpoint returns suggestions with the highest score first, at most 100 per page. It filters by kind and status.
  • A reader who cannot read a view or table gets no suggestion about it.
    • The row is not in the list and not in its count. Retrieve returns 404.
    • A materialized view's private backing table is never shown, so the view's own access decides. The HogQL catalog hides these tables for the same reason.
    • The filter runs in SQL before pagination. That is why the model stores the subject in two columns, subject_kind and subject_id.
  • Access comes from grants on each view or table, not only from warehouse-wide access. A grant on one view is enough to see or act on its suggestion.
  • Dismiss and resume need edit access to the view or table. Resume works only on a dismissed suggestion; the daily job decides when an expired one comes back. Each row carries can_act, so the UI can disable the buttons.
  • Resume clears the dismissal fields. The activity log keeps the dismiss entry and the resume entry.
  • upsert_suggestions is the entry point for the layer 2 job.
    • It runs in one transaction: one bulk_create(ignore_conflicts=True), one locked select, one bulk_update. A failed run changes nothing.
    • It refreshes a row only when the row is still proposed, the draft is about the same kind and subject, and the draft's evidence is not older.
    • A dismissal that a person writes at the same time stays. A delayed run with older evidence leaves the row unchanged.
    • It writes no activity log rows. The log holds only what people did.
  • The activity log masks dismissal_note. The team activity feed does not check access to the subject, and a free-text note can name a hidden view.
  • ENUM_NAME_OVERRIDES gets one entry for the signals DismissalReasonEnum. Without it, the new enum on a dismissal_reason field renames the signals type.
  • The rest is mechanical: the scaffold, the migration (one new table), the activity scope, generated types and the lockfile entry.

Note

Look at products/warehouse_suggestions/backend/logic/access.py first. It holds the read and edit checks.

How did you test this code?

Test rationale: no test covered this product before, so both files are new.

  • test_suggestions.py has one parameterized table of allowed and refused status moves.
  • test_suggestions.py also covers ingest: it inserts, refreshes proposed rows and writes no activity row.
  • Ingest leaves a row unchanged when the row is decided, when the draft's evidence is older, or when the draft names another subject.
  • test_api.py covers a backing-table suggestion (hidden, 404 on retrieve, dismiss and resume) and grants on one view with warehouse-wide viewer or none.
  • test_api.py covers the kind and status filters, a 400 for an unknown status, and a 409 for resume on an expired suggestion.
  • test_api.py checks that the activity log does not hold the dismissal note text.
  • test_api.py covers the denied member: no row, a count without the row, 404 on retrieve, and a soft-deleted view that stays hidden.
  • test_api.py covers the viewer: 403 on dismiss, and can_act false in the list.
  • test_api.py also covers 409 on a decided row, 403 with the flag off, and a constant query count at 1 and 5 rows.
  • Without the filter, the readability test failed locally.
  • Without select_related, the query-count test failed locally.
  • Without the note mask or the last_seen_at guard, the matching test failed locally.
  • A manual API check ran against this branch on a local stack, with seed data in a local team:
    • An admin listed 3 suggestions, highest score first.
    • A member denied one view got count 2. Retrieve of the denied suggestion returned 404.
    • The same member had viewer access on a second view. can_act was false, and dismiss returned 403 with the edit-access message.
    • An admin dismiss returned 200 with the reason, the note and the reviewer. A second dismiss returned 409.
    • Resume cleared the dismissal fields. The activity log had one entry for each action.
    • A dismiss without reason returned 400.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

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 (1M context)

  • Skills: /django-migrations, /improving-drf-endpoints, /writing-dataclasses, /adding-activity-logging, /writing-tests, /reviewing-with-coderabbit, /stacking-prs, /writing-pr-descriptions.
  • The first viewset served the ORM model. product:lint rejects that for an isolated product, so the facade now returns frozen contracts.
  • Enum classes got a WarehouseSuggestion prefix after build:openapi reported name clashes with existing components.
  • CodeRabbit CLI ran with --deep and gave 2 findings. The agent rejected both:
    • "The list schema is a bare array": false. The generated client returns PaginatedWarehouseSuggestionListApi.
    • "Add a comment in ready()": the PR owner does not allow code comments.
  • Bot review on the first PR, feat(warehouse-suggestions): add the suggestion model and review api #112636, which this PR replaces to get a readable branch name:
    • Greptile, overlapping ingests lose updates: fixed. CodeRabbit's "refresh only when the run is newer" note: fixed in the same change.
    • Greptile, activity entries leak review details: fixed by masking the note. The entry holds no subject id or name, so the note was the only path.
    • Greptile, pagination checks every subject: rejected. Fingerprints are unique per kind and subject, so the check is bounded by the team's views and tables, not by history.
    • CodeRabbit, add a comment in ready(): rejected. The PR owner does not allow code comments.
  • PostHog review: both findings fixed. Backing tables are dropped before the access check (option 1). The viewset replaces the warehouse-wide permission check with a project access check, because the facade checks each view and table.
  • Review on this PR: all comments fixed except one.
    • Fixed: older evidence overwrote newer, a reused fingerprint mixed subjects, no page cap, title-case labels, missing filter tests, one UPDATE per draft outside a transaction, resume on expired rows, and the Callable[...] type.
    • The or DEFAULT_PAGE_SIZE fallback stays because the DRF stubs type get_limit as int | None.
    • Rejected: CodeRabbit's comment in ready(), as before.
  • Duplicate search: gh pr list for "warehouse suggestions" found no open PR.
  • Public artifact: the PR holds no material from outside this repository.

🤖 Generated with Claude Code

thiagosalvatore and others added 2 commits October 6, 2026 16:02
…del and API

Add the warehouse_suggestions product. It stores proposals to certify,
deprecate or materialize one view or table, and serves them to the
people who may see that view or table.

- WarehouseSuggestion stores its subject as subject_kind + subject_id
  columns. The list endpoint removes suggestions about views and tables
  the reader cannot read in SQL, before pagination, so the page count
  says nothing about hidden rows. A denied row is a 404 on retrieve.
- dismiss and resume need edit access to the subject. The response
  carries can_act so the UI can disable the buttons.
- upsert_suggestions inserts with bulk_create(ignore_conflicts=True)
  and refreshes only rows that are still proposed. The daily job
  therefore writes no activity log rows, an overlapping run keeps one
  row per fingerprint, and a dismissal written at the same time stays.
- Resume clears the dismissal fields. The activity log keeps the history.
- The list is ordered by score, then id, which gives stable pages.
- The facade takes ids and returns frozen contracts, so the product
  passes product:lint isolation from its first commit.
- Enum classes carry a WarehouseSuggestion prefix, because Suggestion*
  and DismissalReason components already exist in the schema. The
  signals DismissalReasonEnum gets an ENUM_NAME_OVERRIDES entry so its
  generated name does not change.
- can_act is computed for every visible subject in one batched call,
  not per page, so the viewset keeps no state between pagination and
  serialization.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tes safe

- Ingest inserts every draft with ignore_conflicts and then refreshes
  each proposed row that an older run last saw. A run that loses the
  insert race still refreshes the row, and an older run never
  overwrites a newer run's evidence. The fingerprint pre-read is gone.
- The activity log masks dismissal_note. A suggestion's log entry does
  not name its subject, but the free-text note can, and the team
  activity feed does not apply the subject access check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@thiagosalvatore thiagosalvatore self-assigned this Oct 6, 2026
@trunk-io

trunk-io Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@parameterai

parameterai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Risk: No findings

This increment adds only test code: a page-boundary test for the warehouse suggestions list endpoint and its corresponding entry in the pagination-contract invariant allowlist. No production code changed, so it introduces no security risk; the full product was reviewed in earlier rounds with no outstanding findings.

Sentinel reviewed 023eb63 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team October 6, 2026 15:29
@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These owners were selected because this PR adds a new path where owners.yaml names owners of additions. The path after each owner is the new path:

  • @PostHog/team-devex (products/warehouse_suggestions)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). For a skipped owner, the locator is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

@github-actions

github-actions Bot commented Oct 6, 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) — clean

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.

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

⚠️ Bundle size — 🔺 +50.0 KiB (+0.1%)

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

Total: 76.36 MiB · 🔺 +50.0 KiB (+0.1%)

File Size Δ vs base
posthog-app/_parent/products/replay_vision/frontend/replay_scanners/components/variants/VariantsTab.js 14.0 KiB 🔺 +14.0 KiB (new)
posthog-app/src/scenes/feature-flags/FeatureFlag.js 133.5 KiB 🔺 +12.2 KiB (+10.1%)
render-query/src/render-query/render-query.js 25.46 MiB 🔺 +8.2 KiB (+0.0%)
posthog-app/src/scenes/project-homepage/today/TodayReportPage.js 60.4 KiB 🔺 +2.3 KiB (+4.0%)
posthog-app/src/scenes/marketing-analytics/MarketingAnalyticsScene.js 156.3 KiB 🔺 +1.6 KiB (+1.0%)
posthog-app/_parent/products/tracing/frontend/TracingScene.js 187.7 KiB 🔺 +1.0 KiB (+0.5%)

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.64 MiB · 23 files 🔺 +541 B (+0.0%) █████████░ 89.2% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.73 MiB · 663 files 🔺 +614 B (+0.0%) █████████░ 92.6% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.62 MiB · 2,412 files 🟢 -1.0 KiB (-0.0%) █████████░ 91.3% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.86 MiB · 3,486 files 🔺 +4.4 KiB (+0.0%) █████████░ 90.3% of 10.92 MiB
today home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
7.64 MiB · 2,424 files 🟢 -995 B (-0.0%) █████████░ 89.0% of 8.58 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.48 MiB · 3,338 files 🔺 +4.4 KiB (+0.0%) █████████░ 90.2% of 10.51 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.18 MiB · 4,150 files 🔺 +9.7 KiB (+0.1%) ████████░░ 77.5% 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
🟢 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/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
🟢 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/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
🟢 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

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
839 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
221.0 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
99.7 KiB src/lib/api.ts
92.4 KiB src/products.tsx
68.7 KiB src/lib/lemon-ui/icons/icons.tsx
40.6 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
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
221.0 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
112.3 KiB ../packages/quill/packages/quill/dist/index.js
99.7 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.4 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
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
221.0 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
112.3 KiB ../packages/quill/packages/quill/dist/index.js
99.7 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.4 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
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
221.0 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
112.3 KiB ../packages/quill/packages/quill/dist/index.js
99.7 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.4 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
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
221.0 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
112.3 KiB ../packages/quill/packages/quill/dist/index.js
99.7 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.4 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
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
221.0 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
112.3 KiB ../packages/quill/packages/quill/dist/index.js
99.7 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.22 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.22 MiB · 19 files 🔺 +601 B (+0.0%) ████░░░░░░ 38.7% of 5.72 MiB
Deferred (lazy) 2.12 MiB · 44 files 🔺 +698 B (+0.0%) 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
853.5 KiB dist/toolbar/toolbar-app-KNGFQF4F.css
657.6 KiB dist/toolbar/chunk-chunk-UHSGJ7IG.js
259.4 KiB dist/toolbar/chunk-chunk-7HFQNZQZ.js
138.2 KiB dist/toolbar/chunk-chunk-INNLWAGL.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-Y3RRM2BU.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-2KTHMB7S.js
21.0 KiB dist/toolbar/chunk-chunk-F6PJJEI2.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 — 🔺 +626.5 KiB (+0.1%)

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

Total: 1002.29 MiB · 🔺 +626.5 KiB (+0.1%)

ℹ️ MCP UI apps size — 32 app(s), 17182.9 KB JS

Built size of each MCP UI app (main.js + styles.css).

App JS CSS
debug 597.9 KB 202.8 KB
action 454.2 KB 202.8 KB
action-list 564.3 KB 202.8 KB
cohort 453.2 KB 202.8 KB
cohort-list 563.2 KB 202.8 KB
email-template 453.0 KB 202.8 KB
error-details 469.7 KB 202.8 KB
error-issue 454.5 KB 202.8 KB
error-issue-list 564.9 KB 202.8 KB
experiment 561.4 KB 202.8 KB
experiment-list 565.0 KB 202.8 KB
experiment-results 566.4 KB 202.8 KB
feature-flag 566.8 KB 202.8 KB
feature-flag-list 570.6 KB 202.8 KB
feature-flag-testing 457.3 KB 202.8 KB
inline-scan 453.6 KB 202.8 KB
insight-actors 562.4 KB 202.8 KB
llm-costs 559.4 KB 202.8 KB
session-recording 455.4 KB 202.8 KB
survey 454.8 KB 202.8 KB
survey-global-stats 561.9 KB 202.8 KB
survey-list 565.0 KB 202.8 KB
survey-stats 561.9 KB 202.8 KB
trace-span 453.5 KB 202.8 KB
trace-span-list 564.2 KB 202.8 KB
vision-observation-list 563.3 KB 202.8 KB
workflow 453.5 KB 202.8 KB
workflow-list 563.6 KB 202.8 KB
loops-review 457.9 KB 202.8 KB
query-results 774.3 KB 202.8 KB
render-ui 857.7 KB 202.8 KB
visual-review-snapshots 458.0 KB 202.8 KB
✅ Playwright — all passed

All tests passed.

View test results →

⚠️ Backend coverage — 99.0% of changed backend lines covered — 5 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ████████████████████ 99.0% (591 / 596)

File Patch Uncovered changed lines
products/warehouse_suggestions/backend/logic/flags.py 62.5% 14–16
products/warehouse_suggestions/backend/facade/api.py 95.1% 41, 60

🤖 Agents: add a test only if an uncovered line exposes a realistic regression that existing tests miss. Otherwise explain why no new test is needed under "How did you test this code?". Gap list: the patch-coverage artifact on this run (gh run download 58542614550278 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)
Product Coverage Lines
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
demo ███████████░░░░░░░░░ 53.4% 1,445 / 2,707
data_tools ████████████░░░░░░░░ 61.2% 90 / 147
warehouse_sources_queue ██████████████░░░░░░ 67.9% 1,883 / 2,772
ai_gateway ███████████████░░░░░ 75.0% 9 / 12
aeo ███████████████░░░░░ 76.3% 617 / 809
batch_exports ████████████████░░░░ 81.2% 21,557 / 26,539
apm █████████████████░░░ 84.1% 1,306 / 1,553
cdp ██████████████████░░ 88.3% 4,582 / 5,187
ml_inference ██████████████████░░ 88.8% 539 / 607
mcp_analytics ██████████████████░░ 89.2% 5,038 / 5,651
product_tours ██████████████████░░ 89.3% 1,340 / 1,500
dashboards ██████████████████░░ 89.6% 6,927 / 7,731
notebooks ██████████████████░░ 90.2% 15,503 / 17,196
posthog_ai ██████████████████░░ 90.4% 4,342 / 4,801
cohorts ██████████████████░░ 90.5% 8,535 / 9,435
signals ██████████████████░░ 90.7% 66,529 / 73,335
managed_warehouse ██████████████████░░ 90.7% 10,412 / 11,475
data_warehouse ██████████████████░░ 90.8% 14,552 / 16,031
streamlit_apps ██████████████████░░ 90.8% 2,679 / 2,951
tasks ██████████████████░░ 91.4% 81,302 / 88,949
data_modeling ██████████████████░░ 91.6% 10,854 / 11,854
exports ██████████████████░░ 91.7% 9,773 / 10,659
business_knowledge ██████████████████░░ 92.0% 8,552 / 9,295
engineering_analytics ██████████████████░░ 92.2% 11,536 / 12,514
ai_training ██████████████████░░ 92.2% 356 / 386
webmcp ███████████████████░ 92.5% 248 / 268
stamphog ███████████████████░ 92.6% 8,283 / 8,946
early_access_features ███████████████████░ 92.6% 1,339 / 1,446
conversations ███████████████████░ 92.6% 29,254 / 31,588
managed_migrations ███████████████████░ 92.7% 1,581 / 1,705
visual_review ███████████████████░ 92.8% 10,055 / 10,837
mcp_registry ███████████████████░ 93.1% 1,670 / 1,794
notifications ███████████████████░ 93.2% 1,144 / 1,228
approvals ███████████████████░ 93.3% 4,048 / 4,341
error_tracking ███████████████████░ 93.3% 16,593 / 17,782
canvas ███████████████████░ 93.3% 7,941 / 8,510
surveys ███████████████████░ 93.4% 6,644 / 7,113
alerts ███████████████████░ 93.5% 6,688 / 7,152
slack_app ███████████████████░ 93.7% 14,611 / 15,600
autoresearch ███████████████████░ 93.8% 9,698 / 10,338
context_layer ███████████████████░ 93.8% 3,416 / 3,640
web_analytics ███████████████████░ 93.9% 23,763 / 25,313
billing_alerts ███████████████████░ 94.1% 2,094 / 2,226
mcp_store ███████████████████░ 94.3% 8,961 / 9,503
wizard ███████████████████░ 94.7% 6,169 / 6,515
reminders ███████████████████░ 94.8% 760 / 802
workflows ███████████████████░ 94.9% 15,985 / 16,837
review_hog ███████████████████░ 95.1% 11,994 / 12,615
annotations ███████████████████░ 95.1% 817 / 859
ai_observability ███████████████████░ 95.1% 26,584 / 27,947
legal_documents ███████████████████░ 95.2% 2,311 / 2,427
customer_analytics ███████████████████░ 95.2% 26,145 / 27,457
endpoints ███████████████████░ 95.3% 9,348 / 9,804
marketing_analytics ███████████████████░ 95.4% 20,105 / 21,069
actions ███████████████████░ 95.5% 756 / 792
logs ███████████████████░ 95.5% 15,666 / 16,412
replay_vision ███████████████████░ 95.5% 29,230 / 30,602
experiments ███████████████████░ 95.6% 33,622 / 35,182
tracing ███████████████████░ 95.6% 3,536 / 3,699
growth ███████████████████░ 95.7% 11,381 / 11,888
today ███████████████████░ 95.7% 3,083 / 3,220
messaging ███████████████████░ 95.9% 3,834 / 3,999
skills ███████████████████░ 95.9% 7,101 / 7,403
data_catalog ███████████████████░ 96.0% 4,441 / 4,625
product_analytics ███████████████████░ 96.0% 28,584 / 29,761
revenue_analytics ███████████████████░ 96.4% 1,889 / 1,959
alerts_platform ███████████████████░ 96.5% 4,402 / 4,564
access_control ███████████████████░ 96.6% 7,739 / 8,009
feature_flags ███████████████████░ 96.7% 27,114 / 28,052
warehouse_sources ███████████████████░ 97.4% 500,041 / 513,481
data_quality ████████████████████ 97.5% 7,767 / 7,964
cross_project_dashboards ████████████████████ 97.7% 920 / 942
security ████████████████████ 98.0% 1,304 / 1,330
metrics ████████████████████ 98.1% 4,313 / 4,398
analytics_platform ████████████████████ 98.3% 2,784 / 2,833
pulse ████████████████████ 98.5% 2,046 / 2,078
warehouse_suggestions ████████████████████ 99.2% 591 / 596
field_notes ████████████████████ 99.4% 172 / 173

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

⚠️ Django migration SQL — 1 new migration to review

We've detected new migrations on this PR. Review the SQL output for each migration:

products/warehouse_suggestions/backend/migrations/0001_initial.py

/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
  return result
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
  ed = p(logger, meth_name, ed)  # type: ignore[arg-type]
2026-10-06T17:34:22.707559Z [error    ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=6710 tid=139801903266688
Traceback (most recent call last):
  File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
    geoip: Optional[GeoIP2] = GeoIP2(cache=8)
                              ~~~~~~^^^^^^^^^
  File "/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
    raise GeoIP2Exception(
        "Path must be a valid database or directory containing databases."
    )
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
  return (ssh_host,
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
  from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:

WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model WarehouseSuggestion
--
CREATE TABLE "warehouse_suggestions_warehousesuggestion" ("id" uuid NOT NULL PRIMARY KEY, "kind" varchar(32) NOT NULL, "fingerprint" varchar(64) NOT NULL, "subject_kind" varchar(16) NOT NULL, "subject_id" uuid NOT NULL, "payload" jsonb NOT NULL, "payload_version" smallint NOT NULL CHECK ("payload_version" >= 0), "rules_version" varchar(32) NOT NULL, "evidence" jsonb NOT NULL, "evidence_window_start" timestamp with time zone NOT NULL, "evidence_window_end" timestamp with time zone NOT NULL, "last_seen_at" timestamp with time zone NOT NULL, "reproposed_count" integer NOT NULL CHECK ("reproposed_count" >= 0), "score" double precision NOT NULL, "score_inputs" jsonb NOT NULL, "status" varchar(16) NOT NULL, "surfaced_at" timestamp with time zone NULL, "reviewed_at" timestamp with time zone NULL, "dismissal_reason" varchar(16) NULL, "dismissal_note" text NULL, "dismissed_at_score" double precision NULL, "created_asset" jsonb NULL, "asset_outcome" varchar(16) NULL, "run_id" varchar(255) NOT NULL, "reviewed_by_id" integer NULL, "team_id" integer NOT NULL, CONSTRAINT "warehouse_suggestion_unique_fingerprint" UNIQUE ("team_id", "fingerprint"));
CREATE INDEX "warehouse_suggestions_ware_reviewed_by_id_1dc03506" ON "warehouse_suggestions_warehousesuggestion" ("reviewed_by_id");
CREATE INDEX "warehouse_suggestions_warehousesuggestion_team_id_abf29426" ON "warehouse_suggestions_warehousesuggestion" ("team_id");
CREATE INDEX "warehouse_sugg_status_kind" ON "warehouse_suggestions_warehousesuggestion" ("team_id", "status", "kind");
CREATE INDEX "warehouse_sugg_last_seen" ON "warehouse_suggestions_warehousesuggestion" ("team_id", "last_seen_at");
COMMIT;

Last updated: 2026-10-06 17:35 UTC (023eb63)

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 1 Safe | 0 Needs Review | 0 Blocked

✅ Safe

Brief or no lock, backwards compatible

warehouse_suggestions.0001_initial
  └─ #1 ✅ CreateModel
     Creating new table is safe
     model: WarehouseSuggestion
  │
  └──> ℹ️  INFO:
       ℹ️  Skipped operations on newly created tables (empty tables
       don't cause lock contention).

Last updated: 2026-10-06 17:35 UTC (023eb63)

✅ Hogbox preview — ready, open the preview

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds: a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit 023eb63 · box box-31a4b5ad7bc9 · ready in 689s (push → usable) · build log · rebuilds on every push, torn down on close

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[High risk] Adds a new Django app with database model and API endpoints.

The PR should not merge until ingestion preserves evidence ordering and the explicit label requirement is satisfied.

Reviews (1) · Last reviewed commit: "fix(warehouse-suggestions): keep overlap..."

@posthog

posthog Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 2 should fix, 0 consider.

Published 2 findings (view the review).

Resolved comments: 2 left for you

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a warehouse suggestions product with persistent suggestion records, ingestion and status-transition logic, subject access checks, and feature-flag gating. Adds API endpoints to list, retrieve, dismiss, and resume suggestions. Registers suggestion activity logging and adds generated client types and application configuration.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 023eb

The endpoint’s page size is bounded. Strengthening the pagination test would improve regression coverage; the receiver-import guidance also needs clarification, but neither currently establishes a production failure.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 023eb

The review API enforces project and view/table access, and lifecycle changes are serialized. The main remaining privacy question is whether activity history exposes metadata about suggestions that a reader cannot otherwise see.

Retained concerns

  • Low · security · inferred: Subject-level confidentiality is not demonstrably preserved across the audit integration. The new receiver publishes suggestion identifiers and recommendation kinds to activity feeds whose explicit filtering is project-scoped and has no WarehouseSuggestion subject-grant restriction. A caller authorized to read that feed may therefore learn metadata about suggestions hidden by the review API. The underlying generic activity-log access filter remains unresolved, so this is a bounded, inferred disclosure concern rather than a verified vulnerability.
Security review details

Security Blast Radius

  • inferred — The unresolved confidentiality exposure is bounded to metadata for logged suggestion actions visible through an authorized activity feed. The inspected project-feed filter remains team-scoped; no cross-tenant access, credential exposure, warehouse-data mutation, or grant escalation was established.

Security Findings and Attack Paths

  • inferred — A permitted activity-feed caller could request WarehouseSuggestion entries and learn suggestion identifiers, recommendation kinds, and action timing without passing the review API's subject-grant check. This potential alternate read path is newly populated by the PR. No end-to-end unauthorized disclosure was verified because generic feed authorization remains partially inspected.

Trust Boundaries and Controls

  • observed — Client-controlled suggestion IDs are resolved through team-scoped, subject-readable queries. Reviewer identity is taken from the authenticated request rather than request data. Invisible rows produce not-found responses; insufficient edit authority produces permission denial; invalid lifecycle moves produce conflict responses.
  • observed — The audit integration masks dismissal-note contents and excludes payload, evidence, and score-input fields from change diffs. These controls reduce audit-content exposure but do not remove the suggestion identifier or kind from the activity entry.

Resilience and Maintainability Implications

  • observed — Suggestion state updates are transactional, but default activity-log creation is deferred until commit and production write failures are reported without undoing the committed transition. That shared best-effort behavior is unchanged by this PR; the new product inherits it, so audit completeness should not be represented as atomic with review state.
  • observed — Subject authorization is evaluated before the suggestion row lock; the locked transition revalidates team identity and lifecycle state, not subject grants. Concurrent revocation/deletion semantics remain unproven. This alone does not establish a material authorization bypass for requests already authorized at initiation.

Hardening Proposals

  • proposed — Make the audit visibility contract explicit: either preserve warehouse-subject readability in activity feeds or document and enforce a separately privileged audit-reader boundary. Validate it with a reader who has activity-log access but no grant on the suggestion's view or table.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the user problem, visible behavior, access rules, ingestion behavior, tests, release status, documentation, and agent context. It does not include the required before-and-after …
✨ Finishing Touches
📝 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

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 (2)
products/warehouse_suggestions/backend/presentation/views.py-44-51 (1)

44-51: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Cap the page size.

LimitOffsetPagination.max_limit defaults to None. A caller can therefore send ?limit=1000000. The endpoint then loads and serializes every visible suggestion in one response. Set max_limit on the class.

Proposed fix
 class SuggestionPagination(LimitOffsetPagination):
+    default_limit = DEFAULT_PAGE_SIZE
+    max_limit = 500
+
     def paginate(self, request: Request, fetch_page: Callable[[int, int], SuggestionPage]) -> Response:
products/warehouse_suggestions/backend/apps.py-10-10 (1)

10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Explain why ready() connects this receiver.

Add a one-line comment before the import. As per coding guidelines, ready() wiring needs “a one-line comment that says why it must connect in every process.”

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 82923aae-c7ce-4eab-aadf-a4bf768d8022
📥 Commits

Reviewing files that changed from the base of the PR and between 238d03f and ab6ff37.

⛔ Files ignored due to path filters (5)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • products/platform_features/frontend/generated/api.schemas.ts is excluded by !**/generated/**
  • products/warehouse_suggestions/frontend/generated/api.schemas.ts is excluded by !**/generated/**
  • products/warehouse_suggestions/frontend/generated/api.ts is excluded by !**/generated/**
  • products/warehouse_suggestions/frontend/generated/api.zod.ts is excluded by !**/generated/**
📒 Files selected for processing (34)
  • frontend/package.json
  • posthog/models/activity_logging/activity_log.py
  • posthog/settings/web.py
  • posthog/test/repo_invariants/setup_receivers_baseline.txt
  • products/warehouse_suggestions/__init__.py
  • products/warehouse_suggestions/backend/__init__.py
  • products/warehouse_suggestions/backend/activity_logging.py
  • products/warehouse_suggestions/backend/apps.py
  • products/warehouse_suggestions/backend/facade/__init__.py
  • products/warehouse_suggestions/backend/facade/api.py
  • products/warehouse_suggestions/backend/facade/contracts.py
  • products/warehouse_suggestions/backend/facade/enums.py
  • products/warehouse_suggestions/backend/logic/__init__.py
  • products/warehouse_suggestions/backend/logic/access.py
  • products/warehouse_suggestions/backend/logic/flags.py
  • products/warehouse_suggestions/backend/logic/suggestions.py
  • products/warehouse_suggestions/backend/migrations/0001_initial.py
  • products/warehouse_suggestions/backend/migrations/__init__.py
  • products/warehouse_suggestions/backend/migrations/max_migration.txt
  • products/warehouse_suggestions/backend/models.py
  • products/warehouse_suggestions/backend/presentation/__init__.py
  • products/warehouse_suggestions/backend/presentation/serializers.py
  • products/warehouse_suggestions/backend/presentation/views.py
  • products/warehouse_suggestions/backend/routes.py
  • products/warehouse_suggestions/backend/tests/__init__.py
  • products/warehouse_suggestions/backend/tests/test_api.py
  • products/warehouse_suggestions/backend/tests/test_suggestions.py
  • products/warehouse_suggestions/manifest.tsx
  • products/warehouse_suggestions/package.json
  • products/warehouse_suggestions/product.yaml
  • products/warehouse_suggestions/tsconfig.json
  • products/warehouse_suggestions/turbo.json
  • services/mcp/src/api/generated.ts
  • tach.toml

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

@trunk-io

trunk-io Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Scenes/Code review Default play-test The test failed because a logic component was not mounted when accessed, and there were unhandled network requests intercepted by the mock service... Logs ↗︎
compareTopLevelSections() reports a modifiers change when the current query overrides the team default A TypeError occurred because the code attempted to access the 'add' property of an undefined object. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

- Ingest refreshes a proposed row only when the draft is about the same
  kind and subject and its evidence window ends no earlier than the
  row's. A delayed run with older evidence, or a draft that reuses a
  fingerprint for another subject, leaves the row unchanged.
- Ingest now runs in one transaction: one insert, one locked select,
  one bulk_update. The per-draft UPDATE loop is gone, and a failed run
  changes nothing.
- The dismiss and resume endpoints use a smaller transition table, so
  a person can resume a dismissed suggestion but not an expired one.
  Bringing back an expired row stays with the daily job.
- The list endpoint caps the page size at 100.
- Enum labels use sentence case. The unmerged 0001 migration is
  regenerated; only the choice labels changed.
- The access helpers are typed with a Protocol instead of
  Callable[..., ...].
- New API tests cover the kind and status filters and the 400 for an
  unknown status.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@posthog

posthog Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@hosthog

hosthog Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

…ne object

- A materialized view's private backing table is never a readable
  subject. The HogQL catalog hides these tables so the view's own access
  decides, and a table grant must not open a suggestion about the view's
  data. Backing tables of deleted views are excluded too.
- The viewset no longer runs AccessControlPermission on the
  warehouse_objects umbrella. That check ignores grants written on one
  view or table, so a member with an editor grant on one view saw
  can_act=true but got 403 on dismiss, and a member with only a grant on
  one view got 403 on the list. The facade already checks read and edit
  access per view and table. The viewset still checks authentication,
  API scopes, team membership and project access.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@posthog

posthog Bot commented Oct 6, 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.

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

🧹 Nitpick comments (1)
products/warehouse_suggestions/backend/tests/test_api.py (1)

225-233: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Make score order diverge from UUID order while preserving a tied page boundary.

UUIDModel uses UUID7 IDs, so assigning [3, 3, 2, 1, 1] in creation order does not force score order to differ from ID order. An ID-only query can return the expected sequence. That fixture also puts no tied-score pair across a page boundary. Assign scores after sorting by ID; the tied 2.0 rows below span the boundary at offset 2.

Suggested fix
-        suggestions = [self._suggest(self._make_view(f"view_{index}").id, score=1.0) for index in range(5)]
+        suggestions = [self._suggest(self._make_view(f"view_{index}").id, score=1.0) for index in range(5)]
+        suggestions_by_id = sorted(suggestions, key=lambda row: row.id)
+        for suggestion, score in zip(suggestions_by_id, (1.0, 2.0, 2.0, 1.0, 3.0)):
+            suggestion.score = score
+        WarehouseSuggestion.objects.bulk_update(suggestions_by_id, ["score"])
 
         pages = [self.client.get(f"{self.url}/?limit=2&offset={offset}").json() for offset in (0, 2, 4)]
 
         assert [page["count"] for page in pages] == [5, 5, 5]
         assert [len(page["results"]) for page in pages] == [2, 2, 1]
         listed = [row["id"] for page in pages for row in page["results"]]
-        assert listed == [str(row.id) for row in sorted(suggestions, key=lambda row: row.id)]
+        assert listed == [str(row.id) for row in sorted(suggestions, key=lambda row: (-row.score, row.id))]

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 1200d6af-6d4b-451d-9bda-55dd8ced86c4
📥 Commits

Reviewing files that changed from the base of the PR and between 5c7f3fb and 023eb63.

📒 Files selected for processing (2)
  • posthog/test/repo_invariants/test_pagination_contract.py
  • products/warehouse_suggestions/backend/tests/test_api.py

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

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

Looks okay from Devex side.

CERTIFY = "certify"
DEPRECATE = "deprecate"
MATERIALIZE = "materialize"

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.

as i'm reviewing top to bottom - we'll be able to extend this with Kinds like CREATE or UNMATERIALIZE or EDIT??

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yess


class WarehouseSuggestionStatus(LabeledStrEnum):
PROPOSED = "proposed"
ACCEPTED = "accepted"

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.

what happens when it's accepted? do we need an APPLIED or something?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my idea is that once accepted we will dispatch some sort of event and the listeners will react (for example data modeling will materialize, or data catalog will create a metric, or data calog will deprecate something, etc).

Comment on lines +20 to +22
class WarehouseSuggestionSubjectKind(LabeledStrEnum):
SAVED_QUERY = ("saved_query", "Saved query")
TABLE = "table"

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.

appreciate it's going to be an extension - but want to make sure current design is compatible with suggesting to create something that doesn't yet exist in the project?

also - feel like this should already include ENDPOINT SubjectKind, so that we can suggest to materialize them too. Slightly weird that an Endpoint does not have a SavedQuery by default if not materialized, but hey..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should support, the only thing is that in that case the subject id would be null as it doesn't exist yet

self.team_id, self.user_access_control, UUID(pk), user_id=cast(User, request.user).id
)
)

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.

um are we missing approve?

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.

could be in a follow-up :)) 👍

@trunk-io
trunk-io Bot merged commit b1844e0 into master Oct 7, 2026
@trunk-io
trunk-io Bot deleted the thiagosalvatore/warehouse-suggestions-model-api branch October 7, 2026 10:22
@deployment-status-posthog

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

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-07 10:51 UTC Run
prod-us ✅ Deployed 2026-10-07 11:06 UTC Run
prod-eu ✅ Deployed 2026-10-07 11:07 UTC Run

This branch was successfully deployed

1 active deployment
preview-pr-112705 — 023eb637 Deployed Oct 6, 2026 by github-actions[bot]
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