fix(etl): use raw SQL for completion write to bypass neon-http enum issue#2395
Merged
Conversation
…ssue
The Drizzle ORM .set({ status: 'completed' }) with the neon-http driver
appears to fail silently (triggering the catch block which then sets
status='failed'), even though the identical pattern with 'failed' works.
Switch to a raw sql`UPDATE ... SET status = 'completed'::etl_job_status`
to match the pattern already used in updateEtlJobProgress, bypassing any
Drizzle/neon-http enum serialization difference.
Also isolate the completion write in its own try-catch so a transient
failure here logs the error and leaves the job 'running' (for Reset Stuck)
rather than cascading to 'failed'.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (20)
WalkthroughThis PR makes catalog item weight fields nullable in DB and Drizzle schema, adds pack-service defaults, isolates ETL job completion persistence, updates the Drizzle snapshot/journal, and adds tests verifying ETL handles items without weight and embedding failures. ChangesSupport Nullable Catalog Weights
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Contributor
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the catalog ETL worker so that marking a job as completed no longer fails due to a neon-http + Drizzle enum serialization issue, preventing successfully-processed jobs from being incorrectly marked failed.
Changes:
- Replace Drizzle
.set({ status: 'completed' })completion update with a raw SQLUPDATE ... 'completed'::etl_job_status. - Isolate the completion-status write in its own
try/catchso errors there don’t trigger the outer failure handler.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| } catch (completionErr) { | ||
| console.error( | ||
| `[ETL] Failed to mark job ${jobId} completed — will be reset by stuck-job sweep:`, |
The weight NOT NULL constraint on catalog_items was causing ETL job failures for any item missing weight data (common for clothing/footwear brands). The CatalogItemValidator explicitly marks weight as optional, but the DB would reject the INSERT, causing processValidItemsBatch's fallback to also fail, which propagated to the outer catch and set status='failed'. Migration 0037 drops NOT NULL from weight and weight_unit on catalog_items. Adds full ETL integration test suite confirming: happy path completes, no-weight items don't fail, invalid-only runs still complete, exact/multi-batch row counts work, and the embedding fallback doesn't throw to the outer caller. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replaces the handwritten 0037_nullable_catalog_weight.sql with the drizzle-kit generated equivalent — same ALTER TABLE statements but now tracked in the drizzle journal with the proper snapshot. Social feed table CREATE statements were stripped from the generated output because 0033_social_feed_tables.sql was applied manually and not tracked in the journal, causing drizzle-kit to emit duplicate DDL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/drizzle/0037_nullable_catalog_weight.sql`:
- Around line 1-5: Delete this duplicate migration file that applies ALTER TABLE
"catalog_items" ALTER COLUMN "weight" DROP NOT NULL and ALTER COLUMN
"weight_unit" DROP NOT NULL and keep the canonical 0037_rich_electro migration
tracked by the journal; remove the duplicate 0037 nullable catalog migration
from the repository, ensure the migration journal still references only
0037_rich_electro, and run the migration tests (or a dry-run) to confirm no
drift remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6a9960c-66ed-4434-a61b-934b60aad1fc
📒 Files selected for processing (7)
packages/api/drizzle/0037_nullable_catalog_weight.sqlpackages/api/drizzle/0037_rich_electro.sqlpackages/api/drizzle/meta/0037_snapshot.jsonpackages/api/drizzle/meta/_journal.jsonpackages/api/src/db/schema.tspackages/api/src/services/etl/processCatalogEtl.tspackages/api/test/etl.test.ts
Comment on lines
+1
to
+5
| -- catalog_items.weight and weight_unit: drop NOT NULL to allow items without weight data. | ||
| -- The validator intentionally skips weight (clothing/footwear often omit it), but the | ||
| -- NOT NULL constraint was causing upserts to throw, which cascaded to ETL job failures. | ||
| ALTER TABLE "catalog_items" ALTER COLUMN "weight" DROP NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "catalog_items" ALTER COLUMN "weight_unit" DROP NOT NULL; |
Contributor
There was a problem hiding this comment.
Remove the duplicate 0037 migration file to avoid drift/confusion.
This file duplicates the 0037_rich_electro DDL while the journal tracks 0037_rich_electro. Keeping both increases the risk of future divergence during migration maintenance.
🧹 Suggested cleanup
--- a/packages/api/drizzle/0037_nullable_catalog_weight.sql
+++ /dev/null
@@
--- catalog_items.weight and weight_unit: drop NOT NULL to allow items without weight data.
--- The validator intentionally skips weight (clothing/footwear often omit it), but the
--- NOT NULL constraint was causing upserts to throw, which cascaded to ETL job failures.
-ALTER TABLE "catalog_items" ALTER COLUMN "weight" DROP NOT NULL;--> statement-breakpoint
-ALTER TABLE "catalog_items" ALTER COLUMN "weight_unit" DROP NOT NULL;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -- catalog_items.weight and weight_unit: drop NOT NULL to allow items without weight data. | |
| -- The validator intentionally skips weight (clothing/footwear often omit it), but the | |
| -- NOT NULL constraint was causing upserts to throw, which cascaded to ETL job failures. | |
| ALTER TABLE "catalog_items" ALTER COLUMN "weight" DROP NOT NULL;--> statement-breakpoint | |
| ALTER TABLE "catalog_items" ALTER COLUMN "weight_unit" DROP NOT NULL; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/drizzle/0037_nullable_catalog_weight.sql` around lines 1 - 5,
Delete this duplicate migration file that applies ALTER TABLE "catalog_items"
ALTER COLUMN "weight" DROP NOT NULL and ALTER COLUMN "weight_unit" DROP NOT NULL
and keep the canonical 0037_rich_electro migration tracked by the journal;
remove the duplicate 0037 nullable catalog migration from the repository, ensure
the migration journal still references only 0037_rich_electro, and run the
migration tests (or a dry-run) to confirm no drift remains.
…from catalog Making catalog_items.weight/weight_unit nullable caused a TypeScript error in packService.ts — pack items require non-null weight. Fall back to 0/'g' so the AI-generated pack flow still compiles; user can edit after generation.
… 0037 conflict Main added 0037_trips_trail_osm_id after this branch was cut. Kept main's journal/snapshot; will regenerate weight-nullable migration at correct number.
…ight + regenerate weight migration CORS: admin scoped cors was silently dropping Access-Control-Allow-Origin on preflight (two stacked cors plugins conflicted — root sets credentials:false/*, admin sets credentials:true/specific-origin, header got dropped). Switch to origin function so Elysia reflects the exact origin back; bypass auth guard for OPTIONS preflights. Fixes admin app CORS errors from https://admin.packratai.com. Migration: regenerate weight/weight_unit nullable migration as 0047 — main merged 0037–0046 after this branch was cut.
Contributor
Deploying packrat-guides with
|
| Latest commit: |
6f84a52
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://94c64abd.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://fix-etl-completion-and-retry.packrat-guides-6gq.pages.dev |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | fdbc495 | Commit Preview URL Branch Preview URL |
May 12 2026, 03:02 PM |
Contributor
Deploying packrat-landing with
|
| Latest commit: |
6f84a52
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d1895024.packrat-landing.pages.dev |
| Branch Preview URL: | https://fix-etl-completion-and-retry.packrat-landing.pages.dev |
- admin api.ts: double-cast Eden Treaty responses to PaginatedResponse<T> (treaty infers wide union types that don't overlap with the interface) - packages/app user queries: remove deleted auth hooks (useLoginMutation, useRegisterMutation) that called Better Auth routes not in Elysia; redirect useCurrentUser to client.user.profile.get() - apps/web auth page: implement login/register locally via Better Auth REST endpoints instead of importing from @packrat/app - apps/trails useAuth: replace (apiClient as any).auth.* calls with typed trailsAuthClient (createAuthClient from better-auth/react); add auth-client.ts - apps/trails UserInfoSchema: id is UUID string not number (Better Auth) - apps/web types: Post.userId and PostAuthor.id are UUID strings not numbers - apps/web data.ts: update mock post IDs to match string type Co-Authored-By: Claude <noreply@anthropic.com>
- admin api.ts: add safe-cast annotations on Eden Treaty PaginatedResponse double casts; res.json() returns Promise<any> so no explicit cast needed - web auth page: drop superfluous `as Promise<unknown>` on res.json() Co-Authored-By: Claude <noreply@anthropic.com>
- UserSchema.id: z.number() → z.string() (Better Auth uses UUID strings; aligns with Drizzle schema's text('id') PK)
- mapToUser/applySessionUser: add missing emailVerified/createdAt/updatedAt fields; replace `as` casts with asString/asBoolean guards from @packrat/guards
- useAuthActions signInWithGoogle: replace dead apiClient/setToken/UserSchema references with authClient.signIn.social
- ItemReviews: guard nullable review.title/text/date per CatalogItemSchema
Fixes 3 TypeScript errors in nativewindui source: - Icon/types: SymbolViewPropsWithStringName satisfies IconMapper constraint - AdaptiveSearchHeader, LargeTitleHeader: map 'systemDefault' autoCapitalize to 'none' Also widens expo-router peer dep to >=6.0.23 (was ~6.0.23) so bun does not install expo-router@6.0.23 into apps/expo/node_modules, which was causing TypeScript to resolve to the old API and produce 1184+ false positives. 2.0.3-2 is an exact pin; the root override prevents range resolution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
github-actions
Bot
added
the
dependencies
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.set({ status: 'completed' })with raw SQLUPDATE etl_jobs SET status = 'completed'::etl_job_statusto bypass a neon-http driver enum serialization issue that was causing the try block to throw, triggering the catch block and incorrectly marking jobs asfailedstatus='failed'— logged but non-fatal; stuck-job sweep will reset if neededRoot Cause
Jobs that processed all rows successfully were ending up
status='failed'because:db.update(etlJobs).set({ status: 'completed', completedAt: new Date() })threw via neon-http driver (enum serialization bug)status='failed'The identical
.set({ status: 'failed' })in the catch block works fine — the failure is specific to the'completed'enum value through the Drizzle ORM neon-http code path.Fix
Switched to raw SQL (same pattern already used in
updateEtlJobProgress.ts):Post-Deploy Steps
runningjobs asfailedfailedjobs to replay from R2Post-Deploy Monitoring & Validation
[ETL] Failed to mark job— should stop appearing/analytics/catalog/etl— new jobs should showstatus: completednotfailedcompletedAtis set andstatus = 'completed'failedwith non-nulltotalProcessed, the raw SQL path is also failing — check neon connectivity🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Improvements
Tests