fix(dev): consolidated CI hotfix — check-types + biome + unit tests#2059
Conversation
The Expo Unit Tests CI job on development was failing because the v8 statement-coverage threshold (75%) dropped to 74.32% after recent feature merges. PR #1882 (social feed) and the pack-template weight helpers landed with no unit tests, and vitest.config.ts includes every .ts under `features/**/utils/**` in the coverage report. This adds pure-logic unit tests for two easy-to-cover modules: - features/feed/utils/index.ts: buildPostImageUrl, formatAuthorName, and formatRelativeDate. clientEnvs and getRelativeTime are mocked. - features/pack-templates/utils/computePacktemplateWeight.ts: covers empty templates, base gear, consumables, worn items, quantity, and unit conversion. The expo-app/features/packs/utils barrel is mocked to the two pure conversion helpers so the test does not pull in uploadImage.ts (which imports expo-file-system -> expo-sqlite). Coverage goes from 74.32% -> 79.87% statements; 330/330 tests pass. No production code changes.
The Unit Tests workflow path filter only listed `apps/expo/features/packs/utils/**`, so changes under other feature utils directories (e.g. `features/feed/utils/**`, `features/pack-templates/utils/**`) did not trigger the job even though they are included in `apps/expo/vitest.config.ts` coverage report. Broaden the filter to `apps/expo/features/**/utils/**` to match the vitest `include` pattern, and also trigger on changes to the workflow itself so edits here are verified by a run.
…ence
Self-referential FK on postComments.parentCommentId caused TypeScript
to hit recursion depth and produce `{ [x: string]: any }` for all
drizzle relation queries across the codebase, breaking check-types in
~20 files. Adding an explicit AnyPgColumn return type annotation to
the references() callback breaks the type recursion, restoring proper
relation inference without changing runtime behavior.
- Add 400 response to POST /feed/:postId/comments to match handler - Replace `example: null` with `example: undefined` on optional int (null is not assignable to number | undefined in the openapi overload)
getPresignedUrl takes (ctx, { command, signOptions }) — the wildlife
identify route was calling it with 3 positional args.
…reen The suppression was placed above the <Image> element instead of the key prop, so it had no effect and biome still reported the error in CI. Mirrors the fix already applied to PostCard.
…hotfixes # Conflicts: # apps/expo/features/feed/components/PostCard.tsx # apps/expo/features/feed/screens/PostDetailScreen.tsx
Consolidated merge of fix/dev-check-types, fix/dev-biome, and fix/dev-unit-tests into a single PR so all CI jobs pass at once (the 3 individual PRs each only fixed one category and inherited the other two failures, creating a circular dependency). Post-merge fixes needed on top of the 3 branches: - packages/analytics/src/core/catalog-cache.ts: biome --unsafe in the biome-fix branch had deleted the 'private catalogInstance' field declaration and its DuckDBInstance import, but the assignments still referenced the field. Restored both. - apps/expo/features/feed/components/PostCard.tsx: Icon name "delete" (from original dev code) was invalid in the nwui allowlist. Replaced with "trash-can-outline", matching every other delete button in the app.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
There was a problem hiding this comment.
Pull request overview
Consolidates three CI hotfix tracks (typecheck, Biome, and Expo unit test coverage) into one PR so development can return to green CI after recent merges introduced failures across those jobs.
Changes:
- Fix Drizzle schema self-referential FK typing to prevent TypeScript inference recursion issues.
- Align API OpenAPI contracts and route implementations (feed comment responses, wildlife presigned URL call shape).
- Add Expo unit tests for previously-uncovered utilities and update workflow path filters; fix invalid icon names and stale guards import.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/schemas/feed.ts | Adjusts feed request schema OpenAPI metadata for parentCommentId. |
| packages/api/src/routes/wildlife/index.ts | Updates getPresignedUrl usage to the object-opts signature. |
| packages/api/src/routes/feed/comments.ts | Adds documented 400 response for add-comment OpenAPI route. |
| packages/api/src/db/schema.ts | Breaks Drizzle self-reference type recursion via AnyPgColumn return typing on references(). |
| packages/analytics/src/core/catalog-cache.ts | Restores missing DuckDB instance tracking field/type after prior auto-fix regression. |
| apps/expo/features/pack-templates/utils/tests/computePacktemplateWeight.test.ts | Adds unit tests to raise coverage for pack-template weight utils (with safe mocking for Node env). |
| apps/expo/features/feed/utils/tests/feedUtils.test.ts | Adds unit tests covering feed utility helpers (URL building, author formatting, relative date alias). |
| apps/expo/features/feed/components/PostCard.tsx | Replaces invalid delete icon name with an allowlisted one. |
| apps/expo/features/feed/components/CommentItem.tsx | Replaces invalid delete icon name with an allowlisted one. |
| apps/expo/app/(app)/(tabs)/(home)/index.tsx | Switches assertIsString import to @packrat/guards. |
| .github/workflows/unit-tests.yml | Broadens workflow path filters to include features/**/utils/** and self-trigger on workflow edits. |
| .object({ | ||
| content: z.string().min(1).max(1000).openapi({ example: 'Looks amazing!' }), | ||
| parentCommentId: z.number().int().optional().openapi({ example: null }), | ||
| parentCommentId: z.number().int().optional().openapi({ example: undefined }), |
There was a problem hiding this comment.
openapi({ example: undefined }) is not a valid OpenAPI/JSON example value (and can be confusing to readers). Prefer either omitting the example field entirely for this optional property, or using a representative integer example (e.g. 123) and describing that the field should be omitted for top-level comments.
| parentCommentId: z.number().int().optional().openapi({ example: undefined }), | |
| parentCommentId: z.number().int().optional(), |
| 400: { | ||
| description: 'Failed to create comment', | ||
| content: { 'application/json': { schema: ErrorResponseSchema } }, | ||
| }, |
There was a problem hiding this comment.
The route now documents a 400 response, but the handler’s only 400 path (if (!newComment)) is unlikely to ever occur—Drizzle will typically throw on insert failures (e.g. FK violation when parentCommentId doesn’t exist), which would bubble as a 500. Consider validating parentCommentId (exists and belongs to the same postId) before inserting and/or catching insert errors to return a deterministic 400/404 that matches the OpenAPI contract.
fix(dev): consolidated CI hotfix — check-types + biome + unit tests
Summary
Combines #2055 (unit tests coverage), #2056 (biome errors), and #2057 (check-types errors) into a single PR to break their circular dependency — each individual PR only fixes one category and inherits the other two failures, so none of the three can show green CI on their own.
Errors fixed
Post-merge fixes
Two additional fixes on top of the 3 branch merges:
Test plan