refactor: unify type system — drizzle → zod schemas → inferred TS types#2414
Merged
Merged
Conversation
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 selected for processing (4)
WalkthroughThe PR consolidates core type definitions and Zod schemas from scattered local definitions and ChangesAPI Type Consolidation & Centralization
Monorepo Import Path Updates to Centralized Types
Web App userId Type Change: Number → String
API Route Error Handling & Schema Validation Standardization
Guide & Pack-Template Type Updates
Schema Validation & Format Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
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 |
github-actions
Bot
added
dependencies
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. |
![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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/expo/app/(app)/current-pack/[id].tsx (1)
196-198:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the unnecessary type assertion.
The comment claims
PackItemschema requirescreatedAt: string, but thePackIteminterface intypes.ts(line 28-29) defines bothcreatedAt?: Date | stringandupdatedAt?: Date | stringas optional. The double cast throughunknownis unnecessarily permissive and the comment is misleading.If the Treaty response type actually matches
PackItem, remove the cast entirely. If there's a genuine mismatch, handle it explicitly rather than forcing a cast.🔧 Proposed fix
- // safe-cast: Treaty response type has createdAt?: string but PackItem schema requires string - <ItemRow item={item as unknown as PackItem} index={index} /> + <ItemRow item={item} index={index} />🤖 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 `@apps/expo/app/`(app)/current-pack/[id].tsx around lines 196 - 198, The double type assertion "item as unknown as PackItem" and the accompanying comment are unnecessary and misleading because the PackItem interface (createdAt?: Date | string, updatedAt?: Date | string) already allows optional Date|string; remove the cast and comment and pass item directly to ItemRow (i.e., <ItemRow item={item} index={index} />). If there is an actual shape mismatch, explicitly map/convert the Treaty response to a PackItem (e.g., ensure createdAt/updatedAt are strings or Dates) before passing to ItemRow instead of using an unknown cast; use the PackItem type from types.ts to annotate the conversion.
🤖 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/src/routes/catalog/index.ts`:
- Around line 419-421: The current check in the handler that throws a generic
Error when data.weight <= 0 causes a 500; replace this with a client error
response or schema validation: either update
UpdateCatalogItemRequestSchema.weight to z.number().positive().optional() and
remove this manual branch, or change the throw to an Elysia-aware bad request
(e.g., return status(400, ...) or throw a BadRequestError) in the block that
checks data.weight to ensure invalid client input yields a 400 instead of a 500.
- Line 32: This file mixes throwing NotFoundError and returning status(...)
across handlers (e.g., the handlers for '/vector-search', '/etl',
'/:id/similar', 'DELETE /:id' versus handlers that throw NotFoundError around
lines noted), breaking unified error handling and response schemas; pick one
approach and make all catalog routes consistent: either (A) replace status(...)
usages in the routes mentioned with throw NotFoundError(...) so every
missing-resource path uses the same thrown error (ensure messages/metadata match
existing NotFoundError usage), or (B) convert the handlers that throw
NotFoundError to return status(status.NOT_FOUND, {...}) to match the existing
pattern — update the same endpoints ('/vector-search', '/etl', '/:id/similar',
'DELETE /:id') and any sibling handlers in this file accordingly and adjust the
documented response schemas to reflect the chosen error shape. Ensure you update
the specific functions/route handlers in index.ts where NotFoundError or
status(...) is used so the file is no longer half-migrated.
- Around line 226-237: Remove the redundant manual request validations that
duplicate CreateCatalogItemRequestSchema (the checks using data.name,
data.weight, data.weightUnit and the weight <= 0 check) so the route relies on
the schema/validator to return 422/400; then replace the plain throw new
Error('OpenAI API key not configured') (the OPENAI_API_KEY guard after getEnv())
with a specific error type (e.g., a ConfigError or an HttpError with an explicit
500 status) so missing API key remains a server/config error but is
distinguishable from generic failures.
In `@packages/api/src/routes/guides/index.ts`:
- Around line 203-205: Replace the generic Error thrown for empty/missing q with
a client-validation error (HTTP 400) so it is not treated as a server error;
specifically, in the guides search handler where q is validated (reference the q
variable and GuideSearchQuerySchema), throw or return a BadRequest/validation
error (e.g., a framework HttpError/BadRequest variant or Response with status
400) and include a clear message like "Search query parameter q is required"; if
GuideSearchQuerySchema already marks q as required, remove this redundant
runtime check instead.
In `@packages/api/src/routes/packs/index.ts`:
- Line 42: This file is half-migrated to throwing errors and should be
consistent: either revert the partially migrated handlers or finish converting
them to throw errors — pick finishing the migration. For each handler still
using status(...) (handlers for PUT /:packId, DELETE /:packId, item-suggestions,
POST weight-history, gap-analysis, GET /:packId/items, POST /:packId/items,
DELETE /items/:itemId, similar), replace the status(...) return paths with
thrown errors (use throw new NotFoundError(...) for 404 cases and throw new
Error(...) or a more specific thrown error for 400/500 cases), remove the
status(...) usage, and ensure the function handlers (the route callbacks in this
file) consistently throw instead of returning error-shaped payloads; since
NotFoundError is already imported from Elysia at the top, reuse it and keep
error messages descriptive so OpenAPI and consumers see a single error shape.
- Line 120: computePacksWeights([packWithItems])[0] can be undefined under
noUncheckedIndexedAccess, causing PackWithWeightsSchema.parse to throw an opaque
Zod error; change the code to call computePacksWeights once into a local
variable (e.g., const weights = computePacksWeights([packWithItems])), assert
the first element is defined (if undefined throw a clear error mentioning
packWithItems and computePacksWeights), then pass that defined value to
PackWithWeightsSchema.parse instead of indexing inline; alternatively call the
single-pack variant of the weight computation if available to avoid the
undefined risk.
- Around line 240-246: The try/catch around the pack fetch is redundant because
it only rethrows every error; remove the surrounding try/catch and let errors
bubble to the global handler, or if you want to keep logging, replace the catch
with a single catch that logs and rethrows (referencing
PackWithWeightsSchema.parse, computePackWeights, and NotFoundError) — i.e.,
eliminate the entire try { ... } catch block and simply perform the fetch,
existence check (throw new NotFoundError('Pack not found')), and return
PackWithWeightsSchema.parse(computePackWeights(pack)).
- Around line 685-690: The authorization check currently throws a generic Error
which Elysia maps to 500; instead, when neither isOwner nor isPublic is true
return a 403 response using the framework helper (e.g., return status(403, {
error: 'Unauthorized' }) or whatever local status helper pattern is used
elsewhere in this file) rather than throwing; update the block around
isOwner/isPublic and ensure the function still returns
PackItemSchema.parse(item) for allowed requests.
In `@packages/api/src/routes/packTemplates/index.ts`:
- Around line 142-143: The router-wide application of adminAuthPlugin is
blocking non-admin access; remove the global .use(adminAuthPlugin) so authPlugin
remains but adminAuthPlugin is not applied to the entire pack-templates router,
then apply admin-only protection specifically to the POST
'/generate-from-online-content' route (the handler using
GenerateFromOnlineContentRequestSchema) either by passing an isAdmin:true option
on that route or explicitly wrapping that single route with adminAuthPlugin;
ensure other handlers (GET '/', POST '/', GET '/:templateId', PATCH/DELETE item
handlers) keep normal auth behavior so their internal userId / isAppTemplate
logic continues to work.
In `@packages/api/src/routes/trips/index.ts`:
- Around line 186-190: Currently the flow hard-deletes after a separate
ownership check which can leak existence; change to a scoped soft-delete update
that matches both tripId and user.userId in a single query (e.g., replace
db.delete(trips).where(eq(trips.id, tripId)) with an update that sets the
soft-delete marker such as deletedAt or isDeleted and includes
where(eq(trips.id, tripId), eq(trips.userId, user.userId))). Remove the separate
ownership-only check or use the update result to decide: if no rows were
updated, throw NotFoundError('Trip not found') to avoid exposing whether the
trip exists for other users.
In `@packages/api/src/routes/upload.ts`:
- Around line 29-49: The route currently throws generic Errors for
validation/auth failures in the upload handler (checks around
fileName/contentType, ALLOWED_IMAGE_TYPES, MAX_FILE_SIZE, and the filename
prefix check against user.userId) which Elysia maps to 500; change those throws
to use Elysia's status(...) helper to return 400 for missing/invalid
fileName/contentType, 400 for disallowed content type, 400 for invalid/oversized
size, and 403 for the unauthorized filename prefix; also import and apply the
ErrorResponseSchema in the route's response schema so 400/403 are reflected in
the route typing for clients.
In `@packages/api/src/routes/user/index.ts`:
- Line 77: Replace the generic throw new Error('Email already in use by another
user') with a specific ConflictError (e.g., create class ConflictError extends
Error) and throw new ConflictError('Email already in use by another user'); then
update the central error-handling middleware to map ConflictError instances to
HTTP 409 responses (check instanceof ConflictError and return res.status(409)
with a clear message) so email conflicts produce a 409 Conflict instead of a
generic error.
In `@packages/api/src/routes/weather.ts`:
- Around line 136-138: Replace the generic throw new Error in the validation
block that checks idParam and id with a framework-specific 400-level error so
middleware can map it to a Bad Request; specifically, in the handler where
idParam and id are validated (the if (!idParam || Number.isNaN(id)) block),
throw or return a BadRequest/HTTP 400 error (e.g., BadRequestError or
createError(400, ...)) with a clear message like "Valid location ID is required"
so it matches how other routes handle client errors and is correctly interpreted
by the error middleware.
- Around line 148-157: The catch block around
WeatherAPIForecastResponseSchema.parse currently lumps all errors together;
update the error handling in the function that fetches/parses the forecast so it
checks for ZodError specifically (importing ZodError from zod), logs the
validation details (error.errors) and throws a clearer validation error (e.g.,
include invalid paths from error.errors), and otherwise retains the existing
logging/throw for non-validation errors; reference
WeatherAPIForecastResponseSchema.parse and the catch block where you currently
console.error('Error fetching weather forecast:', error).
---
Outside diff comments:
In `@apps/expo/app/`(app)/current-pack/[id].tsx:
- Around line 196-198: The double type assertion "item as unknown as PackItem"
and the accompanying comment are unnecessary and misleading because the PackItem
interface (createdAt?: Date | string, updatedAt?: Date | string) already allows
optional Date|string; remove the cast and comment and pass item directly to
ItemRow (i.e., <ItemRow item={item} index={index} />). If there is an actual
shape mismatch, explicitly map/convert the Treaty response to a PackItem (e.g.,
ensure createdAt/updatedAt are strings or Dates) before passing to ItemRow
instead of using an unknown cast; use the PackItem type from types.ts to
annotate the conversion.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fe86a705-669d-4361-8d84-fc4c86b40b65
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!bun.lock
📒 Files selected for processing (49)
apps/expo/app/(app)/current-pack/[id].tsxapps/expo/components/initial/UserAvatar.tsxapps/expo/components/initial/WeightBadge.tsxapps/expo/data/mockData.tsapps/expo/features/guides/types.tsapps/expo/features/pack-templates/packTemplateListAtoms.tsapps/expo/features/pack-templates/screens/CreatePackTemplateItemForm.tsxapps/expo/features/packs/components/TemplateItemsSection.tsxapps/expo/features/packs/input.tsapps/expo/features/packs/packListAtoms.tsapps/expo/features/packs/screens/CreatePackItemForm.tsxapps/expo/features/packs/types.tsapps/expo/lib/utils/__tests__/compute-pack.test.tsapps/expo/lib/utils/compute-pack.tsapps/expo/types/index.tsapps/expo/utils/__tests__/weight.test.tsapps/expo/utils/weight.tsapps/web/lib/data.tsapps/web/lib/types.tspackage.jsonpackages/api/src/db/schema.tspackages/api/src/routes/catalog/index.tspackages/api/src/routes/feed/index.tspackages/api/src/routes/guides/index.tspackages/api/src/routes/packTemplates/index.tspackages/api/src/routes/packs/index.tspackages/api/src/routes/trips/index.tspackages/api/src/routes/upload.tspackages/api/src/routes/user/index.tspackages/api/src/routes/weather.tspackages/api/src/schemas/catalog.tspackages/api/src/schemas/guides.tspackages/api/src/schemas/packs.tspackages/api/src/schemas/upload.tspackages/api/src/services/packService.tspackages/api/src/types/constants.tspackages/api/src/types/index.tspackages/api/src/utils/__tests__/itemCalculations.test.tspackages/api/src/utils/__tests__/weight.test.tspackages/api/src/utils/itemCalculations.tspackages/api/src/utils/routeParams.tspackages/api/src/utils/weight.tspackages/app/src/entities/catalog/schema.tspackages/app/src/entities/feed/schema.tspackages/app/src/entities/pack/schema.tspackages/app/src/entities/trip/schema.tspackages/app/src/entities/user/schema.tspackages/cli/src/args.tspackages/overpass/src/schemas.ts
💤 Files with no reviewable changes (1)
- apps/expo/types/index.ts
Comment on lines
29
to
49
| if (!fileName || !contentType) { | ||
| return status(400, { error: 'fileName and contentType are required' }); | ||
| throw new Error('fileName and contentType are required'); | ||
| } | ||
|
|
||
| // Validate content type - only allow images | ||
| if (!ALLOWED_IMAGE_TYPES.includes(contentType.toLowerCase())) { | ||
| return status(400, { error: 'Invalid content type. Only image files are allowed.' }); | ||
| throw new Error('Invalid content type. Only image files are allowed.'); | ||
| } | ||
|
|
||
| // Validate file size - max 10MB | ||
| if (size) { | ||
| const fileSize = Number.parseInt(String(size), 10); | ||
| if (Number.isNaN(fileSize) || fileSize <= 0 || fileSize > MAX_FILE_SIZE) { | ||
| return status(400, { error: 'File size must be greater than 0 and not exceed 10MB' }); | ||
| throw new Error('File size must be greater than 0 and not exceed 10MB'); | ||
| } | ||
| } | ||
|
|
||
| // Security check: Ensure the filename starts with the user's ID | ||
| if (!fileName.startsWith(`${user.userId}-`)) { | ||
| return status(403, { error: 'Unauthorized' }); | ||
| throw new Error('Unauthorized'); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Elysia v1 throw status helper for 400 and 403 from route handler
💡 Result:
Elysia v1: use the handler context status(...) helper (typed) to return or throw HTTP 400/403 from a route handler. 1) Return 400/403 (does NOT go through onError) Use return status(400, payload) or return status(403, payload) in the route handler. Example: const app = new Elysia.get('/bad', ({ status }) => { return status(400, { error: 'Bad Request' }) }).get('/forbidden', ({ status }) => { return status(403, { error: 'Forbidden' }) }) Elysia’s docs explain that returning status(...) is not caught by onError middleware. [1] 2) Throw 400/403 (DOES go through onError) Use throw status(400, payload) or throw status(403, payload) in the route handler, and handle it in .onError(...). Example: new Elysia.onError(({ code }) => { if (code === 403) return { error: 'Forbidden' } }).get('/forbidden-throw', ({ status }) => { throw status(403, { error: 'Forbidden' }) }) Elysia’s docs explicitly state: if status is thrown, it will be caught by onError; if status is returned, it will not. [1] 3) If TypeScript won’t allow status(403, ...) Some setups/type definitions may only include certain status codes for the route response schema, and then status(403, ...) causes a TS error (e.g., 403 not assignable). An Elysia issue reports this behavior and that you may need to include 403 in the route’s allowed response/status schema. [2] In that case, two options are: - Update the route’s declared response/status union to include 400/403. [2] - Use set.status = 403 instead (less type-narrowing, but works as a fallback). The docs note set.status as a way to set response status in handlers. [3][4]
Citations:
- 1: https://elysiajs.com/patterns/error-handling
- 2: Allow status method to return 403,401 elysiajs/elysia#1615
- 3: https://elysiajs.com/essential/handler
- 4: https://mintlify.com/elysiajs/elysia/concepts/handlers
🏁 Script executed:
wc -l packages/api/src/routes/upload.tsRepository: PackRat-AI/PackRat
Length of output: 99
🏁 Script executed:
cat -n packages/api/src/routes/upload.tsRepository: PackRat-AI/PackRat
Length of output: 3205
🏁 Script executed:
rg -A 5 "onError" packages/api/src/routes/upload.tsRepository: PackRat-AI/PackRat
Length of output: 44
🏁 Script executed:
rg "response\s*:" packages/api/src/routes/upload.ts | head -20Repository: PackRat-AI/PackRat
Length of output: 116
🏁 Script executed:
cat -n packages/api/src/middleware/auth.ts | head -80Repository: PackRat-AI/PackRat
Length of output: 3184
🏁 Script executed:
cat -n packages/api/src/schemas/upload.tsRepository: PackRat-AI/PackRat
Length of output: 608
🏁 Script executed:
rg "PresignedUploadQuerySchema" packages/api/src/schemas/upload.ts -A 10Repository: PackRat-AI/PackRat
Length of output: 353
🏁 Script executed:
rg "onError|ErrorResponseSchema" packages/api/src/routes/index.ts | head -20Repository: PackRat-AI/PackRat
Length of output: 44
Validation and authorization errors incorrectly return 500 instead of 400/403.
All four throw new Error(...) sites (lines 30, 35, 42, 48) are validation or authorization checks that should return 4xx codes, not 500:
- Line 30: missing
fileNameorcontentType→ should be 400 - Line 35: disallowed content type → should be 400
- Line 42: invalid/oversized
size→ should be 400 - Line 48: filename doesn't start with
user.userId→ should be 403
Elysia's default error handler maps bare Error to 500. This breaks client status-code-based error handling and hides legitimate access violations in monitoring as server faults.
Use Elysia's status(...) helper (already imported in auth middleware) to return the correct code. Also add 400 and 403 to the route's response schema so Eden Treaty clients get proper typing:
Fix
- if (!fileName || !contentType) {
- throw new Error('fileName and contentType are required');
- }
- if (!ALLOWED_IMAGE_TYPES.includes(contentType.toLowerCase())) {
- throw new Error('Invalid content type. Only image files are allowed.');
- }
- if (size) {
- const fileSize = Number.parseInt(String(size), 10);
- if (Number.isNaN(fileSize) || fileSize <= 0 || fileSize > MAX_FILE_SIZE) {
- throw new Error('File size must be greater than 0 and not exceed 10MB');
- }
- }
- if (!fileName.startsWith(`${user.userId}-`)) {
- throw new Error('Unauthorized');
- }
+ if (!fileName || !contentType) {
+ throw status(400, { error: 'fileName and contentType are required' });
+ }
+ if (!ALLOWED_IMAGE_TYPES.includes(contentType.toLowerCase())) {
+ throw status(400, { error: 'Invalid content type. Only image files are allowed.' });
+ }
+ if (size) {
+ const fileSize = Number.parseInt(String(size), 10);
+ if (Number.isNaN(fileSize) || fileSize <= 0 || fileSize > MAX_FILE_SIZE) {
+ throw status(400, { error: 'File size must be greater than 0 and not exceed 10MB' });
+ }
+ }
+ if (!fileName.startsWith(`${user.userId}-`)) {
+ throw status(403, { error: 'Unauthorized' });
+ }And update the response schema:
- response: { 200: PresignedUploadResponseSchema },
+ response: {
+ 200: PresignedUploadResponseSchema,
+ 400: ErrorResponseSchema,
+ 403: ErrorResponseSchema,
+ },(Import status from 'elysia' and ErrorResponseSchema from the schemas.)
🤖 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/src/routes/upload.ts` around lines 29 - 49, The route currently
throws generic Errors for validation/auth failures in the upload handler (checks
around fileName/contentType, ALLOWED_IMAGE_TYPES, MAX_FILE_SIZE, and the
filename prefix check against user.userId) which Elysia maps to 500; change
those throws to use Elysia's status(...) helper to return 400 for
missing/invalid fileName/contentType, 400 for disallowed content type, 400 for
invalid/oversized size, and 403 for the unauthorized filename prefix; also
import and apply the ErrorResponseSchema in the route's response schema so
400/403 are reflected in the route typing for clients.
| error: 'Email already in use by another user', | ||
| code: 'EMAIL_CONFLICT', | ||
| }); | ||
| throw new Error('Email already in use by another user'); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider a more specific error type for email conflicts.
Line 77 throws a generic Error for email conflicts, but this should ideally result in a 409 Conflict HTTP response. Ensure your error-handling middleware can distinguish this case, or use a more specific error type (e.g., ConflictError) for clearer intent.
Suggested improvement
- throw new Error('Email already in use by another user');
+ throw new ConflictError('Email already in use by another user');
+ // Or include status hint in message for middleware:
+ // throw new Error('[409] Email already in use by another user');🤖 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/src/routes/user/index.ts` at line 77, Replace the generic throw
new Error('Email already in use by another user') with a specific ConflictError
(e.g., create class ConflictError extends Error) and throw new
ConflictError('Email already in use by another user'); then update the central
error-handling middleware to map ConflictError instances to HTTP 409 responses
(check instanceof ConflictError and return res.status(409) with a clear message)
so email conflicts produce a 409 Conflict instead of a generic error.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the monorepo type system to derive runtime validation and TypeScript types from a single Zod/Drizzle-driven source, and wires Elysia route response schemas to enable end-to-end typing with Eden Treaty clients. Also updates ID types (e.g., userId) to UUID strings and reduces duplicate schema/type definitions across apps/packages.
Changes:
- Upgrade to Zod v4 and centralize enum tuples + shared utility types in
@packrat/api/types/constants(with@packrat/api/typesas a compatibility barrel). - Re-export app entity schemas from canonical
@packrat/api/schemas/*and remove Expo’s duplicatetypes/index.ts. - Add response schemas + runtime parsing to multiple API routes to enforce response shapes and improve Treaty typing (including stripping sensitive fields like embeddings).
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/overpass/src/schemas.ts | Updates Zod record typing for Overpass element tags (Zod v4 signature). |
| packages/cli/src/args.ts | Updates generic Zod type annotation for parsing helpers (Zod v4). |
| packages/app/src/entities/user/schema.ts | Replaces local User schemas with re-exports from @packrat/api/schemas/users. |
| packages/app/src/entities/trip/schema.ts | Replaces local Trip schemas with re-exports from @packrat/api/schemas/trips. |
| packages/app/src/entities/pack/schema.ts | Replaces local Pack schemas with re-exports from @packrat/api/schemas/packs. |
| packages/app/src/entities/feed/schema.ts | Replaces local Feed schemas with re-exports from @packrat/api/schemas/feed. |
| packages/app/src/entities/catalog/schema.ts | Replaces local Catalog schemas with re-exports from @packrat/api/schemas/catalog. |
| packages/api/src/utils/weight.ts | Switches PackItem type import to @packrat/api/types/constants. |
| packages/api/src/utils/routeParams.ts | Changes integer param parsing to transform + refine for int4 range enforcement. |
| packages/api/src/utils/itemCalculations.ts | Switches CatalogItem/PackItem/WeightUnit type imports to @packrat/api/types/constants. |
| packages/api/src/utils/tests/weight.test.ts | Updates PackItem type import and userId fixture type to string. |
| packages/api/src/utils/tests/itemCalculations.test.ts | Updates type imports and userId fixture type to string. |
| packages/api/src/types/index.ts | Turns @packrat/api/types into a barrel re-export of ./constants. |
| packages/api/src/types/constants.ts | Introduces centralized enum tuples + shared Zod schemas/types for utilities and consumers. |
| packages/api/src/services/packService.ts | Switches PACK_CATEGORIES import to @packrat/api/types/constants. |
| packages/api/src/schemas/upload.ts | Expands presigned upload response schema (adds objectKey, publicUrl). |
| packages/api/src/schemas/packs.ts | Switches constants import source; documents datetime coercion behavior. |
| packages/api/src/schemas/guides.ts | Adds GuideCategoriesResponseSchema. |
| packages/api/src/schemas/catalog.ts | Switches constants import source; standardizes created/updated to ISO string coercion. |
| packages/api/src/routes/weather.ts | Adds response schema for forecast and parses output; refactors error handling. |
| packages/api/src/routes/user/index.ts | Adds response schemas and parses outputs for profile/update endpoints; refactors error handling. |
| packages/api/src/routes/upload.ts | Adds response schema + parsing for presigned upload; refactors error handling. |
| packages/api/src/routes/trips/index.ts | Moves request/response validation to shared schemas; adds response schemas; refactors error handling. |
| packages/api/src/routes/packTemplates/index.ts | Adds admin macro usage (adminAuthPlugin + isAdmin: true) for generate-from-online endpoint. |
| packages/api/src/routes/packs/index.ts | Adds response schemas + parsing for selected endpoints; refactors some error handling; strips extra fields via schema parsing. |
| packages/api/src/routes/guides/index.ts | Adds response schemas + parsing for list/search/detail/categories; refactors error handling. |
| packages/api/src/routes/feed/index.ts | Adds FeedResponseSchema response typing and parses feed list responses. |
| packages/api/src/routes/catalog/index.ts | Adds response schemas + parsing for list/categories/get/create/update; refactors error handling; strips embedding via schema parsing. |
| packages/api/src/db/schema.ts | Switches PackCategory/WeightUnit type imports to @packrat/api/types/constants. |
| package.json | Upgrades root Zod dependency range to v4. |
| bun.lock | Updates lockfile to Zod v4 resolution and related dependency graph. |
| apps/web/lib/types.ts | Updates userId fields in shared web types from number → string. |
| apps/web/lib/data.ts | Updates mock data to use string userId values. |
| apps/expo/utils/weight.ts | Switches PackItem type import from Expo-local types to @packrat/api/types/constants. |
| apps/expo/utils/tests/weight.test.ts | Switches PackItem type import to @packrat/api/types/constants. |
| apps/expo/types/index.ts | Removes duplicate Expo-local type/schema definitions. |
| apps/expo/lib/utils/compute-pack.ts | Switches Pack type import to @packrat/api/types/constants. |
| apps/expo/lib/utils/tests/compute-pack.test.ts | Switches Pack/PackItem type imports to @packrat/api/types/constants. |
| apps/expo/features/packs/types.ts | Switches PackCategory/WeightUnit type imports to @packrat/api/types/constants. |
| apps/expo/features/packs/screens/CreatePackItemForm.tsx | Switches WeightUnit type import to @packrat/api/types/constants. |
| apps/expo/features/packs/packListAtoms.ts | Switches PackCategory type import to @packrat/api/types/constants. |
| apps/expo/features/packs/input.ts | Switches WeightUnit type import to @packrat/api/types/constants. |
| apps/expo/features/packs/components/TemplateItemsSection.tsx | Switches WeightUnit type import to @packrat/api/types/constants. |
| apps/expo/features/pack-templates/screens/CreatePackTemplateItemForm.tsx | Switches WeightUnit type import to @packrat/api/types/constants. |
| apps/expo/features/pack-templates/packTemplateListAtoms.ts | Switches PackCategory type import to @packrat/api/types/constants. |
| apps/expo/features/guides/types.ts | Aligns readingTime type to number (matches API schema). |
| apps/expo/data/mockData.ts | Switches User type import to @packrat/api/types/constants. |
| apps/expo/components/initial/WeightBadge.tsx | Switches WeightUnit type import to @packrat/api/types/constants. |
| apps/expo/components/initial/UserAvatar.tsx | Switches User type import to @packrat/api/types/constants. |
| apps/expo/app/(app)/current-pack/[id].tsx | Switches PackItem type import to @packrat/api/types/constants. |
Comments suppressed due to low confidence (1)
packages/api/src/routes/packs/index.ts:690
- Authorization failures in this endpoint throw a generic
Error('Unauthorized'), which will be returned as a 500 by the globalonErrorhandler. This should be a 403 (or 401) so clients can handle it correctly and so it doesn’t look like a server outage.
const isOwner = item.userId === user.userId;
const isPublic = item.pack.isPublic;
if (!isOwner && !isPublic) throw new Error('Unauthorized');
return PackItemSchema.parse(item);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
133
to
138
| const idParam = query.id; | ||
| const id = Number(idParam); | ||
|
|
||
| if (!idParam || Number.isNaN(id)) { | ||
| return status(400, { error: 'Valid location ID is required' }); | ||
| throw new Error('Valid location ID is required'); | ||
| } |
Comment on lines
69
to
78
| if (email) { | ||
| const [existingUser] = await db | ||
| .select({ id: users.id }) | ||
| .from(users) | ||
| .where(eq(users.email, email.toLowerCase())) | ||
| .limit(1); | ||
|
|
||
| if (existingUser && existingUser.id !== user.userId) { | ||
| return status(409, { | ||
| error: 'Email already in use by another user', | ||
| code: 'EMAIL_CONFLICT', | ||
| }); | ||
| throw new Error('Email already in use by another user'); | ||
| } |
Comment on lines
184
to
188
| }); | ||
|
|
||
| if (!trip) return status(404, { error: 'Trip not found' }); | ||
| if (trip.userId !== user.userId) return status(403, { error: 'Forbidden' }); | ||
| if (!trip) throw new NotFoundError('Trip not found'); | ||
| if (trip.userId !== user.userId) throw new Error('Forbidden'); | ||
|
|
Comment on lines
27
to
49
| const { fileName, contentType, size } = query; | ||
|
|
||
| if (!fileName || !contentType) { | ||
| return status(400, { error: 'fileName and contentType are required' }); | ||
| throw new Error('fileName and contentType are required'); | ||
| } | ||
|
|
||
| // Validate content type - only allow images | ||
| if (!ALLOWED_IMAGE_TYPES.includes(contentType.toLowerCase())) { | ||
| return status(400, { error: 'Invalid content type. Only image files are allowed.' }); | ||
| throw new Error('Invalid content type. Only image files are allowed.'); | ||
| } | ||
|
|
||
| // Validate file size - max 10MB | ||
| if (size) { | ||
| const fileSize = Number.parseInt(String(size), 10); | ||
| if (Number.isNaN(fileSize) || fileSize <= 0 || fileSize > MAX_FILE_SIZE) { | ||
| return status(400, { error: 'File size must be greater than 0 and not exceed 10MB' }); | ||
| throw new Error('File size must be greater than 0 and not exceed 10MB'); | ||
| } | ||
| } | ||
|
|
||
| // Security check: Ensure the filename starts with the user's ID | ||
| if (!fileName.startsWith(`${user.userId}-`)) { | ||
| return status(403, { error: 'Unauthorized' }); | ||
| throw new Error('Unauthorized'); | ||
| } |
Comment on lines
201
to
205
| async ({ query }) => { | ||
| const { q, page, limit, category } = query; | ||
| if (!q || q.trim() === '') { | ||
| return status(400, { error: 'Search query parameter q is required' }); | ||
| throw new Error('Search query parameter q is required'); | ||
| } |
Comment on lines
224
to
231
| const db = createDb(); | ||
| const data = body; | ||
| if (!data.name || data.weight === undefined || data.weight === null || !data.weightUnit) { | ||
| return status(400, { error: 'name, weight, and weightUnit are required' }); | ||
| throw new Error('name, weight, and weightUnit are required'); | ||
| } | ||
| if (data.weight <= 0) { | ||
| return status(400, { error: 'weight must be a positive number' }); | ||
| throw new Error('weight must be a positive number'); | ||
| } |
Comment on lines
93
to
118
| const db = createDb(); | ||
| const data = body; | ||
|
|
||
| const packId = data.id as string; | ||
| if (!packId) return status(400, { error: 'Pack ID is required' }); | ||
| if (!packId) throw new Error('Pack ID is required'); | ||
|
|
||
| // Zod validates all fields at runtime; cast through the Standard Schema | ||
| // inference gap so drizzle's insert accepts the values. | ||
| const [newPack] = await db | ||
| .insert(packs) | ||
| .values({ | ||
| id: packId, | ||
| userId: user.userId, | ||
| name: data.name, | ||
| description: data.description, | ||
| category: data.category, | ||
| isPublic: data.isPublic, | ||
| image: data.image, | ||
| tags: data.tags, | ||
| localCreatedAt: new Date(data.localCreatedAt as string), | ||
| localUpdatedAt: new Date(data.localUpdatedAt as string), | ||
| } as typeof packs.$inferInsert) | ||
| .returning(); | ||
|
|
||
| if (!newPack) return status(400, { error: 'Failed to create pack' }); | ||
| if (!newPack) throw new Error('Failed to create pack'); | ||
|
|
Comment on lines
+11
to
+17
| export const integerIdSchema = z | ||
| .string() | ||
| .regex(/^[1-9]\d*$/) | ||
| .pipe(z.coerce.number().int().positive().max(PG_INT4_MAX)); | ||
| .transform((s) => parseInt(s, 10)) | ||
| .refine((n) => n > 0 && n <= PG_INT4_MAX, { | ||
| message: `Must be a positive integer up to ${PG_INT4_MAX}`, | ||
| }); |
Comment on lines
14
to
18
| export const PresignedUploadResponseSchema = z.object({ | ||
| url: z.string().url(), | ||
| url: z.string(), | ||
| objectKey: z.string(), | ||
| publicUrl: z.string(), | ||
| }); |
| "ts-extras": "^1.0.0", | ||
| "typescript": "~5.9.2", | ||
| "zod": "^3.24.2" | ||
| "zod": "^4.0.0" |
andrew-bierman
force-pushed
the
refactor/unify-schema-type-system
branch
from
b095372 to
b59605c
Compare
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | 3cd4af4 | Commit Preview URL Branch Preview URL |
May 17 2026, 02:52 AM |
github-actions
Bot
removed
the
dependencies
- expo/current-pack: switch PackItem import to expo features type so pack.items matches without the misleading `as unknown as` double cast. - api/weather: handle ZodError separately when forecast response fails validation, surfacing the invalid paths instead of a generic throw. - api/catalog: remove redundant manual validations on POST/PUT (now fully enforced by Create/UpdateCatalogItemRequestSchema). Make UpdateCatalogItemRequestSchema.weight positive to match Create. Clarify config-error message when OPENAI_API_KEY is missing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Addressed the remaining CodeRabbit + Copilot review comments in commit Fixed:
Verified already addressed in
Left as-is:
|
Now targeting development (retargeted from main). Resolved 2 conflicts: - current-pack/[id].tsx: kept the local expo-app/features/packs/types PackItem since usePackDetailsFromStore returns the local-store shape (description?: string vs @packrat/types' description: string | null). Will revisit once the store is migrated to the unified type. - catalog/index.ts: kept HEAD's pattern of relying on schema validation and throwing for config errors (3 hunks: dropped dev's redundant data.name/weight manual checks; restored HEAD's 'throw new Error' for missing OPENAI_API_KEY). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Deploying packrat-guides with
|
| Latest commit: |
3cd4af4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bf389822.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://refactor-unify-schema-type-s.packrat-guides-6gq.pages.dev |
Contributor
Deploying packrat-landing with
|
| Latest commit: |
3cd4af4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b971962d.packrat-landing.pages.dev |
| Branch Preview URL: | https://refactor-unify-schema-type-s.packrat-landing.pages.dev |
8 tasks
andrew-bierman
pushed a commit
that referenced
this pull request
May 17, 2026
Resolves conflict in packages/api/src/routes/trailConditions/reports.ts caused by #2414 schema refactor (schemas moved to @packrat/schemas). Re-applies #2434's .optional().default() thickening to the new schema location (packages/schemas/src/trailConditions.ts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
andrew-bierman
pushed a commit
that referenced
this pull request
May 17, 2026
…factor) Resolves conflicts from #2414 (extract @packrat/db + @packrat/schemas): - Route files: keep dev's @packrat/schemas imports, retain mintId + queryBoolean - packages/schemas/src/catalog.ts: keep my T6 CatalogCompare* schemas + dev's CatalogETLSchema - packages/schemas/src/packs.ts: apply T9 (id optional with trim/min(1)) to CreatePackBody + AddPackItemBody - packages/schemas/src/trips.ts: apply T9 to CreateTripBodySchema - packages/schemas/src/trailConditions.ts: apply T9 + T-thickening (drop .default for hazards/waterCrossings/photos) - packs/index.ts: keep AddPackItemFromCatalogSchema (T8) local, drop redundant CreatePack/AddPackItem local schemas (now in @packrat/schemas) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
andrew-bierman
added a commit
that referenced
this pull request
May 18, 2026
Phase 1 of the client/server ID split per docs/design/client-uuid-split.md §3 Option C. Each affected table gains a `client_uuid text UNIQUE NOT NULL` column, backfilled from the existing `id`, with a format CHECK constraint enforcing the URL-safe nanoid charset (≤64 chars). Tables touched: packs, pack_items, weight_history, pack_templates, pack_template_items, trips, trail_condition_reports. Also restores packages/api/src/db/schema.ts as a 1-line re-export shim (load-bearing for drizzle-kit per the #2414 plan §"Migration Infra"). The shim was deleted in 0154b87 along with all other re-export shims, which silently broke drizzle-kit generate since 2026-05-14. Refs: docs/plans/2026-05-17-001-feat-client-uuid-split-phase1-plan.md U1 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
andrew-bierman
added a commit
that referenced
this pull request
May 20, 2026
Substantial rebase covering 225 dev commits — #2414 type unification, #2422 single-param refactor, #2433 MCP+CLI Eden Treaty rewrite, #2439 OG meta validation, #2441/#2442 OG URL fix, plus many smaller. Conflicts resolved: - apps/expo/features/packs/utils/uploadImage.ts: kept HEAD's userId cache, used dev's object-arg getPresignedUrl call (matches function signature). - apps/expo/features/trips/hooks/useDeleteTrip.ts: kept HEAD's async + optimistic-delete comment, used dev's object-arg obs() call (matches current obs signature in apps/expo/lib/store.ts). Post-merge cleanup of dev-introduced single-param violations: - apps/expo/lib/utils/__tests__/getRelativeTime.test.ts: rewrote 3 test call sites to object args matching the refactored getRelativeTime. - packages/api/src/utils/__tests__/embeddingHelper.test.ts: rewrote 7 test call sites to object args matching the refactored getEmbeddingText; updated Parameters<> type indexes. - packages/overpass/src/client.test.ts: converted makeResponse to single object param and updated all 11 call sites. - scripts/lint/no-owned-max-params.ts: added apps/trails/scripts/generate-og-images.ts to EXCLUDED_FILES (same globalThis.fetch shim pattern as the existing landing/guides entries). Verification: bun install ok; bun check-types 0 errors; biome check 0 errors (2 unrelated warnings); no-owned-max-params 0 violations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
andrew-bierman
added a commit
that referenced
this pull request
May 21, 2026
…rite migrations Hand-written SQL migrations drift across environments and break the unified drizzle-schema → drizzle-zod → inferred-TS-types pipeline established by PR #2414. Drizzle-kit is the only sanctioned path: 1. Change schema in packages/db/src/schema/*.ts 2. bun --cwd packages/db drizzle-kit generate 3. Review the generated SQL 4. Commit both schema + migration together If drizzle-kit emits a migration you disagree with, fix the schema or the generator config, not the SQL output. Also updates the Database section to reflect the post-extraction schema location (`packages/db/src/schema/` instead of `packages/api/src/db/schema.ts`).
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
drizzle schema → drizzle-zod → @packrat/api/schemas/* → inferred TypeScript types. No more hand-rolled duplicates.response: { 200: Schema }to every Elysia route soeden.route.get()returns typeddatainstead ofunknown.CatalogItemSchema.parse(drizzleRow)strips theembeddingvector field before it reaches the API response.apps/expo/types/index.ts(15 consumers redirected to@packrat/api/types/constants) and collapsedpackages/app/src/entities/*/schema.tsfiles into thin re-exports of the canonical API schemas.@packrat/api/types/constantsis now the single home for enum tuples (WEIGHT_UNITS,PACK_CATEGORIES, etc.) and their Zod/TS types.types/index.tsis a barrel re-export for backward compat.packTemplatesgenerate-from-online-content usesadminAuthPlugin+isAdmin: truedeclaratively instead of a manual role check.Commits
chore(deps)— Zod ^3 → ^4.3.6fix(types)— userId/author id types:z.number()→z.string()(better-auth uses UUIDs)feat(api)—response: { 200: Schema }on all Elysia routesrefactor(api)— extract constants totypes/constants.tsrefactor(app)— entity schemas become re-exports from@packrat/api/schemasfix(catalog)— embedding leak fix + admin macro for packTemplatesrefactor(expo)— deleteapps/expo/types/index.ts, fixuserIdstring typesTest Plan
bun check-typesfrom monorepo root: 0 errorsbun test packages/api/src/utils/__tests__/: 191 pass, 0 failPost-Deploy Monitoring & Validation
embeddingfield is absent in response payloads. Eden Treaty client calls —datashould be typed, notunknown.curl /api/catalog/1 | jq 'has("embedding")'→ should befalseembeddingappears in a response, theCatalogItemSchema.parse()call was bypassed.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores