Skip to content

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

Closed
thiagosalvatore wants to merge 1 commit into
masterfrom
thiagosalvatore/pasted-text-file
Closed

thiagosalvatore wants to merge 1 commit into
masterfrom
thiagosalvatore/pasted-text-file

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. 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.
    • The filter runs in SQL before pagination. That is why the model stores the subject in two columns, subject_kind and subject_id.
  • Dismiss and resume need edit access to the view or table. 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 inserts new rows in one bulk_create(ignore_conflicts=True). When two runs overlap, the table still has one row per fingerprint.
    • It refreshes only rows that are still proposed, so a dismissal that a person writes at the same time stays.
    • It writes no activity log rows. The log holds only what people did.
  • 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, leaves decided rows unchanged and writes no activity row.
  • 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.
  • 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.
    • An invalid status filter and a dismiss without reason both 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.
  • 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

…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>
@thiagosalvatore thiagosalvatore self-assigned this Oct 6, 2026
@trunk-io

trunk-io Bot commented Oct 6, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@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 — 🔺 +42 B (+0.0%)

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

Total: 76.31 MiB · 🔺 +42 B (+0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.64 MiB · 23 files no change █████████░ 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 no change █████████░ 92.6% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.62 MiB · 2,410 files 🔺 +42 B (+0.0%) █████████░ 91.4% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.85 MiB · 3,484 files 🔺 +42 B (+0.0%) █████████░ 90.2% 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,422 files 🔺 +42 B (+0.0%) █████████░ 89.0% of 8.58 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.47 MiB · 3,336 files 🔺 +42 B (+0.0%) █████████░ 90.1% of 10.51 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.18 MiB · 4,145 files 🔺 +42 B (+0.0%) ████████░░ 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.3 KiB src/products.tsx
68.7 KiB src/lib/lemon-ui/icons/icons.tsx
40.7 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
280.3 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.3 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
280.3 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.3 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
280.3 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.3 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
280.3 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.3 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
280.3 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 no change ████░░░░░░ 38.7% of 5.72 MiB
Deferred (lazy) 2.12 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
853.0 KiB dist/toolbar/toolbar-app-XAP2G6X3.css
657.6 KiB dist/toolbar/chunk-chunk-FSU22SGF.js
259.4 KiB dist/toolbar/chunk-chunk-ZMPDGZER.js
138.2 KiB dist/toolbar/chunk-chunk-F3KCKEFA.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-L5CNMFJP.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-VDLNERFK.js
21.0 KiB dist/toolbar/chunk-chunk-4MYVTG33.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +1.0 KiB (+0.0%)

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

Total: 1001.68 MiB · 🔺 +1.0 KiB (+0.0%)

ℹ️ 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

@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 is not safe to merge until activity-log visibility and overlapping-ingest behavior are fixed.

Reviews (1) · Last reviewed commit: "feat(warehouse-suggestions): bootstrap p..."

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

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

Adds warehouse-suggestion persistence, ingestion, access checks, and status transitions. Exposes list, retrieve, dismiss, and resume operations through a project API. Registers the product and activity logging, and adds generated client types and product configuration.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 7cc8b

Overlapping ingestion may leave a suggestion with stale scoring data. The feature is gated and its later workflows are not part of this layer; address the refresh race and required receiver comment before enabling it broadly.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 7cc8b

The new review endpoints preserve project and subject access checks, and concurrent updates protect human decisions. Exposure is bounded to suggestion metadata and review state, not warehouse execution. Some secondary audit visibility and future ingestion guarantees remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected HTTP surface exposes readable suggestion metadata and allows subject editors to modify suggestion review state within a canonical project scope. These operations do not execute certification, deprecation, materialization, or warehouse queries.

Trust Boundaries and Controls

  • observed — All review endpoints retain authentication, project membership, and common routing checks. The facade removes unreadable subjects before counting or producing response contracts, returns not-found for inaccessible suggestions, and requires editor access for decisions.
  • observed — Edit authorization is checked before entering the locked suggestion transition. The lock serializes suggestion state, but does not establish atomic ordering with subject-permission revocation.

Resilience and Maintainability Implications

  • observed — Ingestion preserves stored subject identity while refreshing payload and evidence by team and fingerprint. The storage and update contracts do not verify that a reused fingerprint still represents the same kind and subject; the future producer's identity guarantee was not available.

Hardening Proposals

  • proposed — Before connecting the ingestion producer, define and enforce the immutable fingerprint/kind/subject identity tuple so a conflicting draft cannot refresh another subject's stored payload or evidence.
  • proposed — Verify that activity-log readers apply the intended access policy to suggestion review metadata, including dismissal notes, before relying on the primary API's subject filtering as an end-to-end privacy guarantee.
🚥 Pre-merge checks | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the problem, changes, tests, release status, docs, and agent context. However, its claim that a manual API check ran conflicts with the PR objectives, which say it has not yet r… Confirm whether the manual API check ran, then update the description or objectives so both state the same test status. Add the required agent-session link if available.
Full details: Description check

Explanation

The description covers the problem, changes, tests, release status, docs, and agent context. However, its claim that a manual API check ran conflicts with the PR objectives, which say it has not yet run.

✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • 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.

Actionable comments posted: 1

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/backend/logic/suggestions.py-94-107 (1)

94-107: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Refresh only when the incoming run is newer.

A run can miss a fingerprint inserted after its known query. Refreshing every draft fixes that race, but an older overlapping run could then overwrite data from a newer run.

Refresh proposed rows only when last_seen_at is older than this run's seen_at. Keep the status filter to protect decided rows.

Suggested fix
     for draft in drafts:
-        if draft.fingerprint in known:
-            suggestions.filter(fingerprint=draft.fingerprint, status=WarehouseSuggestionStatus.PROPOSED).update(
-                last_seen_at=seen_at, **_ingest_updates(draft)
-            )
+        suggestions.filter(
+            fingerprint=draft.fingerprint,
+            status=WarehouseSuggestionStatus.PROPOSED,
+            last_seen_at__lt=seen_at,
+        ).update(last_seen_at=seen_at, **_ingest_updates(draft))

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 7eb2d6dd-437a-49aa-9120-7beb60971949
📥 Commits

Reviewing files that changed from the base of the PR and between d27b586 and 7cc8b10.

⛔ 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; 9 remain after this review.

@trunk-io

trunk-io Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

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

@thiagosalvatore

Copy link
Copy Markdown
Contributor Author

Replaced by #112705, which has the same commits plus the review fixes, on a branch with a readable name.

@thiagosalvatore
thiagosalvatore deleted the thiagosalvatore/pasted-text-file branch October 6, 2026 15:27
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.

1 participant