Conversation
|
Updates to Preview Branch (feature/optimize-publishing) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
There was a problem hiding this comment.
6 issues found across 38 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/db/src/schema/products/product-passports.ts">
<violation number="1" location="packages/db/src/schema/products/product-passports.ts:131">
P2: The partial index already fixes `dirty = true`, so indexing `dirty` as a key is redundant and adds avoidable index/write overhead.</violation>
</file>
<file name="apps/api/src/trpc/routers/dpp-public/index.ts">
<violation number="1" location="apps/api/src/trpc/routers/dpp-public/index.ts:180">
P1: Inline projection in the public route can race under concurrent requests and throw unique-version errors, causing public 500s.</violation>
</file>
<file name="apps/api/src/trpc/routers/products/publish.ts">
<violation number="1" location="apps/api/src/trpc/routers/products/publish.ts:279">
P2: `totalProductsPublished` reports input count rather than actual updates. If some productIds don't exist or belong to another brand, the response will incorrectly claim they were published. Consider returning the actual affected row count from the UPDATE operation, or validating productIds exist beforehand.</violation>
</file>
<file name="apps/api/supabase/migrations/20260306111521_white_iron_lad.sql">
<violation number="1" location="apps/api/supabase/migrations/20260306111521_white_iron_lad.sql:1">
P1: Dropping `data_snapshot` NOT NULL without an alternative CHECK constraint allows snapshot-less versions to be stored.</violation>
</file>
<file name="packages/db/src/queries/dpp/public.ts">
<violation number="1" location="packages/db/src/queries/dpp/public.ts:339">
P2: `dirty` is hardcoded to `false` in `getPublicDppVersion`, which can return incorrect passport state.</violation>
</file>
<file name="apps/api/src/trpc/routers/products/index.ts">
<violation number="1" location="apps/api/src/trpc/routers/products/index.ts:144">
P3: Remove unreachable camelCase checks in bulk snapshot-change detection to avoid dead branches.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR replaces the previous "publish-on-write" model with a dirty-flag + deferred projection architecture. Rather than materializing passport snapshots synchronously on every product edit or via a delayed Trigger.dev fan-out job, mutations now mark affected published passports as Key changes:
Two logic bugs identified:
Confidence Score: 3/5
Sequence DiagramsequenceDiagram
participant Client
participant CatalogRouter
participant ProductsRouter
participant PublishRouter
participant DB_MarkDirty as DB (mark-dirty)
participant DB_Projector as DB (projector)
participant PublicDPP as Public DPP Router
participant ProjectorJob as Projector Job (hourly)
Note over CatalogRouter,DB_MarkDirty: Catalog mutation path (inline dirty marking)
Client->>CatalogRouter: update/delete manufacturer/material/etc.
CatalogRouter->>DB_MarkDirty: markPassportsDirtyByProductIds()
DB_MarkDirty-->>CatalogRouter: {marked, passportIds}
CatalogRouter-->>Client: success
Note over PublishRouter,DB_Projector: Single/product publish (inline projection)
Client->>PublishRouter: publish.variant / publish.product
PublishRouter->>DB_MarkDirty: markPassportDirty() / markPassportsDirtyByProductIds()
PublishRouter->>DB_Projector: projectSinglePassport() / projectDirtyPassports()
DB_Projector-->>PublishRouter: {snapshot, version, upids, barcodes}
PublishRouter-->>Client: {success, version, passportsProjected}
Note over PublishRouter,DB_MarkDirty: Bulk publish (deferred projection)
Client->>PublishRouter: publish.bulk
PublishRouter->>DB_MarkDirty: markPassportsDirtyByProductIds()
PublishRouter-->>Client: {success, productsMarkedDirty}
Note over PublicDPP,DB_Projector: Public read with inline dirty projection
Client->>PublicDPP: getByPassportUpid / getByBarcode
PublicDPP->>DB_Projector: getPublicDppByUpid()
alt passport.dirty == true AND productStatus == published
PublicDPP->>DB_Projector: projectSinglePassport()
PublicDPP->>DB_Projector: getPublicDppByUpid() (re-fetch)
end
PublicDPP-->>Client: DPP snapshot + theme
Note over ProjectorJob,DB_Projector: Background sweep (hourly)
ProjectorJob->>DB_Projector: projectDirtyPassportsAllBrands()
DB_Projector-->>ProjectorJob: {brands[], upids, barcodes}
ProjectorJob->>ProjectorJob: revalidatePassports() + revalidateBarcodes() per brand
Last reviewed commit: 5732504 |
🚀 Preview Deployment SummaryApp: https://avelero-9jczi80of-avelero.vercel.app Preview environments will auto-delete when PR closes. |
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/trpc/routers/products/publish.ts">
<violation number="1" location="apps/api/src/trpc/routers/products/publish.ts:288">
P2: `totalFailed` can be overcounted when `productIds` contains duplicates, producing incorrect bulk publish results.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
🚀 Preview Deployment SummaryApp: https://avelero-59winoy5n-avelero.vercel.app Preview environments will auto-delete when PR closes. |
🚀 Preview Deployment SummaryApp: https://avelero-j89jp75ml-avelero.vercel.app Preview environments will auto-delete when PR closes. |
Summary by cubic
Switches publishing to a “dirty passport” pipeline: write paths mark affected passports dirty and a new projector builds snapshots in the background or on demand. Adds historical version compression and updates publish flows and jobs to flip status, mark dirty, project, and revalidate caches.
New Features
Migration
Written for commit bd802c3. Summary will update on new commits.