Prevent mobile app test files from becoming routes - #2446
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe mobile app now keeps Expo Router routes separate from tests, Storybook stories, and shared fixtures. Storybook paths and imports were updated. Route validation runs through workspace and mobile lint scripts. ChangesMobile route hygiene
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Contributor
Reviewer's GuideThis PR moves Expo Router screen tests and stories out of the route tree into dedicated app-tests/app-stories directories, updates imports and Storybook config accordingly, and adds a lint-time guard plus documentation to enforce route hygiene in the mobile app. Flow diagram for lint-time mobile app route hygiene guardflowchart TD
lint_script["npm run lint"]
biome_check["biome check ."]
route_guard["tsx scripts/check-mobile-app-route-files.ts"]
app_dir["packages/mobile/app"]
violation_found["Forbidden *.test.* or *.stories.* file found"]
lint_fail["Lint fails (exit 1)"]
lint_pass["Lint passes"]
lint_script --> biome_check
lint_script --> route_guard
route_guard --> app_dir
app_dir -->|no forbidden files| lint_pass
app_dir -->|has forbidden files| violation_found --> lint_fail
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
LGTM! The changes cleanly relocate mobile app stories into 🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does. |
Contributor
Mobile PreviewScan to open on device:
To test on device:
|
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
Asherlc
marked this pull request as ready for review
August 7, 2026 20:40
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @Asherlc, your pull request is larger than the review limit of 150000 diff characters
PR Summary by QodoEnforce Expo Router route hygiene by moving tests/stories out of app/
AI Description
Diagram
High-Level Assessment
Files changed (76)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@package.json`:
- Around line 216-217: Update the validator invoked by check:mobile-app-routes
in check-mobile-app-route-files.ts to enforce the documented route-only policy
for packages/mobile/app/. Extend its filename checks to reject fixtures and
helper-only files, or define and apply a canonical route allowlist covering all
permitted route names; keep the validator and AGENTS.md documentation
consistent.
- Around line 216-217: Update the package script check:mobile-app-routes to
invoke the repository’s TypeScript runner through pnpm, using pnpm tsx
scripts/check-mobile-app-route-files.ts instead of calling tsx directly.
In `@packages/mobile/app-tests/food/add-types.test.ts`:
- Line 2: Update the canonical AGENTS.md guidance with a scoped
packages/mobile/app-tests/** exception allowing mobile route tests to remain
outside packages/mobile/app, consistent with the mobile README. Preserve the
general colocated-test rule and do not move
packages/mobile/app-tests/food/add-types.test.ts.
In `@scripts/check-mobile-app-route-files.ts`:
- Line 5: Update forbiddenRouteFilePattern to match any filename containing the
.test. or .stories. marker with a trailing extension, removing the [jt]sx?
whitelist so files such as .mts and .d.ts are detected.
🪄 Autofix
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 Plus
Run ID: ed4e3b0a-79b4-4442-9115-162a10eb7cff
📒 Files selected for processing (76)
.gitignoreAGENTS.mdpackage.jsonpackages/mobile/.rnstorybook/main.tspackages/mobile/.storybook/main.tspackages/mobile/AGENTS.mdpackages/mobile/README.mdpackages/mobile/app-stories/(tabs)/activities.stories.tsxpackages/mobile/app-stories/(tabs)/food.stories.tsxpackages/mobile/app-stories/(tabs)/index.stories.tsxpackages/mobile/app-stories/(tabs)/recovery.stories.tsxpackages/mobile/app-stories/(tabs)/strain.stories.tsxpackages/mobile/app-stories/activity/ActivitySourceDecisionCard.stories.tsxpackages/mobile/app-stories/activity/ZoneCharts.stories.tsxpackages/mobile/app-stories/alerts.stories.tsxpackages/mobile/app-stories/behavior-associations.stories.tsxpackages/mobile/app-stories/breathwork.stories.tsxpackages/mobile/app-stories/correlation.stories.tsxpackages/mobile/app-stories/cycle.stories.tsxpackages/mobile/app-stories/experiments.stories.tsxpackages/mobile/app-stories/login.stories.tsxpackages/mobile/app-stories/more.stories.tsxpackages/mobile/app-stories/nutrition-analytics.stories.tsxpackages/mobile/app-stories/providers/index.stories.tsxpackages/mobile/app-stories/providers/provider-danger-zone.stories.tsxpackages/mobile/app-stories/providers/provider-detail-actions-card.stories.tsxpackages/mobile/app-stories/providers/sync-all-controls.stories.tsxpackages/mobile/app-stories/settings.stories.tsxpackages/mobile/app-stories/tracking.stories.tsxpackages/mobile/app-tests/(tabs)/_fixture-dates.test.tspackages/mobile/app-tests/(tabs)/_food-story-fixture.test.tspackages/mobile/app-tests/(tabs)/_layout.test.tsxpackages/mobile/app-tests/(tabs)/_processing-status-story-fixture.test.tspackages/mobile/app-tests/(tabs)/activities.test.tsxpackages/mobile/app-tests/(tabs)/food.test.tsxpackages/mobile/app-tests/(tabs)/index.test.tsxpackages/mobile/app-tests/(tabs)/recovery.test.tsxpackages/mobile/app-tests/(tabs)/strain.test.tsxpackages/mobile/app-tests/+native-intent.test.tspackages/mobile/app-tests/_layout.cleanup.test.tsxpackages/mobile/app-tests/_layout.telemetry-guard.test.tspackages/mobile/app-tests/_layout.test.tspackages/mobile/app-tests/activity/ActivityDetailCharts.test.tsxpackages/mobile/app-tests/activity/ZoneDistributionCharts.test.tsxpackages/mobile/app-tests/activity/[id].test.tsxpackages/mobile/app-tests/alerts.test.tsxpackages/mobile/app-tests/behavior-associations.test.tsxpackages/mobile/app-tests/ble-probe.test.tsxpackages/mobile/app-tests/breathwork.test.tsxpackages/mobile/app-tests/correlation.test.tsxpackages/mobile/app-tests/cycle.test.tsxpackages/mobile/app-tests/daily-heart-rate.test.tsxpackages/mobile/app-tests/data-quality.test.tsxpackages/mobile/app-tests/experiments.test.tsxpackages/mobile/app-tests/food/FoodResultCard.test.tsxpackages/mobile/app-tests/food/QuickAddTab.test.tsxpackages/mobile/app-tests/food/add-types.test.tspackages/mobile/app-tests/heart-rate-visualization.test.tsxpackages/mobile/app-tests/imu-visualization.test.tsxpackages/mobile/app-tests/login.stories.test.tsxpackages/mobile/app-tests/login.test.tsxpackages/mobile/app-tests/more.test.tsxpackages/mobile/app-tests/nutrition-analytics.test.tsxpackages/mobile/app-tests/onboarding.test.tsxpackages/mobile/app-tests/preview.test.tsxpackages/mobile/app-tests/providers/[id].test.tsxpackages/mobile/app-tests/providers/auth-modals.test.tsxpackages/mobile/app-tests/providers/index.test.tsxpackages/mobile/app-tests/providers/sync-all-controls.test.tsxpackages/mobile/app-tests/reports.test.tsxpackages/mobile/app-tests/settings.test.tsxpackages/mobile/app-tests/sleep.test.tsxpackages/mobile/app-tests/supplements.test.tsxpackages/mobile/app-tests/tracking.test.tsxpackages/mobile/package.jsonscripts/check-mobile-app-route-files.ts
Asherlc
enabled auto-merge (squash)
August 7, 2026 22:33
|
🤖 Review skipped: Repository Owner rate limit exceeded. Free accounts are limited to 3 reviews per 4 hours across all repositories. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/mobile/AGENTS.md`:
- Line 16: Update the Storybook guidance in packages/mobile/AGENTS.md to
distinguish configuration directories from story-file locations: retain
.storybook and .rnstorybook for Storybook configuration, explicitly place route
stories in app-stories/, and specify the approved location for component story
files.
In `@packages/mobile/app-tests/`(tabs)/_fixture-dates.test.ts:
- Line 2: Move packages/mobile/app-tests/(tabs)/_fixture-dates.test.ts to
packages/mobile/app-fixtures/(tabs)/fixture-dates.test.ts and
packages/mobile/app-tests/(tabs)/_processing-status-story-fixture.test.ts to
packages/mobile/app-fixtures/(tabs)/processing-status-story-fixture.test.ts,
removing the leading underscores while preserving each test’s existing imports
and behavior.
In `@packages/mobile/app-tests/`(tabs)/_food-story-fixture.test.ts:
- Line 3: Move the fixture test importing seedFoodStoryQuery from the app-tests
location to app-fixtures/(tabs)/food-story-fixture.test.ts, colocating it with
food-story-fixture.ts; preserve the test contents and behavior unchanged.
🪄 Autofix
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 Plus
Run ID: 9539dc66-9710-4416-8e25-f245b71c9856
📒 Files selected for processing (15)
AGENTS.mdpackage.jsonpackages/mobile/AGENTS.mdpackages/mobile/README.mdpackages/mobile/app-fixtures/(tabs)/fixture-dates.tspackages/mobile/app-fixtures/(tabs)/food-story-fixture.tspackages/mobile/app-fixtures/(tabs)/processing-status-story-fixture.tspackages/mobile/app-stories/(tabs)/food.stories.tsxpackages/mobile/app-stories/(tabs)/index.stories.tsxpackages/mobile/app-stories/(tabs)/recovery.stories.tsxpackages/mobile/app-stories/(tabs)/strain.stories.tsxpackages/mobile/app-tests/(tabs)/_fixture-dates.test.tspackages/mobile/app-tests/(tabs)/_food-story-fixture.test.tspackages/mobile/app-tests/(tabs)/_processing-status-story-fixture.test.tsscripts/check-mobile-app-route-files.ts
|
🤖 Review skipped: Repository Owner rate limit exceeded. Free accounts are limited to 3 reviews per 4 hours across all repositories. Upgrade to a paid plan for unlimited reviews. |
|
🤖 Review skipped: Repository Owner rate limit exceeded. Free accounts are limited to 3 reviews per 4 hours across all repositories. Upgrade to a paid plan for unlimited reviews. |
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
packages/mobile/app/intopackages/mobile/app-tests/.packages/mobile/app/intopackages/mobile/app-stories/and update Storybook globs.packages/mobile/app/.Root Cause
Expo Router treats files under
packages/mobile/app/as route candidates. Colocated*.test.*and*.stories.*files can therefore become accidental native screens or iOS tabs.Validation
Passed locally:
biome check . --max-diagnostics=500tsx scripts/check-mobile-app-route-files.tstsc --noEmitcd packages/server && tsc --noEmitcd packages/web && tsc --noEmittsc --noEmit -p packages/mobile/tsconfig.jsonvitest run --project mobile(66 files, 570 tests)Notes:
pnpmshim, so I ran the equivalent commands with repo-local binaries onPATH.vitest run --changed origin/mainwas started, made progress through a broad unrelated affected set, then was interrupted after hanging for a long time in integration coverage that was mostly timing out/skipping without Docker dependencies. CI should provide the clean full gate on the pushed branch.Summary by Sourcery
Enforce Expo Router route hygiene for the mobile app by moving screen tests and stories out of app/, tightening Storybook globs, and adding a lint-time guard that rejects non-route test/story files under the app/ directory.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Summary by cubic
Prevents non-route files in
packages/mobile/app/from becoming Expo Router screens by moving tests, stories, and fixtures out, updating Storybook globs, and adding a strict lint guard. Also pinsnanoid@3.3.17and ignores two no‑patch advisories to quiet dependency audit noise.Bug Fixes
packages/mobile/app-tests/, stories topackages/mobile/app-stories/, and shared fixtures topackages/mobile/app-fixtures/with their tests colocated; updated imports.app-stories/**in RN (.rnstorybook) and web (.storybook) configs.scripts/check-mobile-app-route-files.tsand wired it into rootlint:sandboxand mobilelint; it fails on*.test.*,*.stories.*, and files named like fixtures/helpers underapp/.app/convention inpackages/mobile/AGENTS.mdandpackages/mobile/README.md.Migration
packages/mobile/app-tests/, screen stories inpackages/mobile/app-stories/, and shared fixtures (and their tests) inpackages/mobile/app-fixtures/. Only real routes/layouts belong inpackages/mobile/app/.Written for commit 26a5159. Summary will update on new commits.
Summary by CodeRabbit
Documentation
Chores