Fix slow iOS loading across mobile pages - #1471
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughMobile pull-to-refresh flows on activities, food, provider detail, and providers list screens now perform targeted tRPC query invalidation via ChangesTargeted pull-to-refresh invalidation
Non-blocking loading via shouldShowBlockingLoading
Estimated code review effort: 3 (Moderate) | ~25 minutes Assessment against linked issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
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 |
|
Storybook previews for This comment updates automatically on each PR push. |
Mobile PreviewScan to open on device:
To test on device:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 10 files
Confidence score: 3/5
- In
packages/mobile/app/providers/[id].tsx, pull-to-refresh currently missestrpcUtils.sync.providers.invalidate(), so provider metadata (name/status/last sync) can stay stale after refresh and users may see outdated connection state — add that invalidation call inuseProviderDetailActionsbefore merging. - In
packages/mobile/app/(tabs)/food.test.tsx, the invalidation mocks are configured but never asserted, so regressions in cache invalidation behavior could slip through unnoticed — add assertions forinvalidateFoodByDateMockandinvalidatesettingsGetMock(including expected args) to de-risk future changes.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Addressed Cubic review #1471 (review). Fixed.
|
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Auto-approved: Improves mobile performance with non-blocking cached loads and scoped refresh invalidation; includes tests.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mobile/app/(tabs)/strain.tsx (1)
319-320: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the computed
isLoadinginstead of rawtrainingQuery.isLoading.The "Recent Activities" section still branches on
trainingQuery.isLoadingdirectly, duplicating the concept the newshouldShowBlockingLoading-derivedisLoadingwas introduced to centralize. Practically harmless today (react-query'sisLoadingis only true when there's no data), but it's an inconsistency waiting to bite if the query's caching behavior changes.♻️ Proposed fix
- {trainingQuery.isLoading ? ( + {isLoading ? ( <ActivityIndicator color={colors.accent} style={styles.activitiesLoader} />🤖 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/mobile/app/`(tabs)/strain.tsx around lines 319 - 320, The Recent Activities loading branch still uses trainingQuery.isLoading directly instead of the centralized isLoading state derived from shouldShowBlockingLoading. Update the strain.tsx render path to use the computed isLoading consistently wherever the loading UI is gated, so the ActivityIndicator and related logic all rely on the same source of truth.
🤖 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/app/`(tabs)/index.tsx:
- Around line 77-82: The refetch failure state is being hidden once
dashboardData exists because isError only checks for dashboardData == null.
Update the dashboard screen logic in index.tsx to distinguish initial-load
errors from stale-data refetch errors using dashboardQuery.isError with
dashboardData != null, and render a lightweight non-blocking inline error
indicator near the top of the ScrollView for that stale-data case. Keep the
existing blocking loading/error behavior for the initial load, and reuse
QueryStatePanel or a compact Text/banner-style state tied to dashboardQuery and
dashboardData.
In `@packages/mobile/app/providers/index.tsx`:
- Around line 485-494: The providers screen now has targeted invalidate wiring
in useRefresh, but there is no matching test coverage like the sibling screens.
Update providers/index.test.tsx to mirror the [id].test.tsx pattern by mocking
useRefresh and asserting that the refresh callback triggers invalidation of
sync.providers, sync.providerStats, sync.logs, sync.dataHealth, and
sync.activeSyncs from providers/index.tsx.
---
Outside diff comments:
In `@packages/mobile/app/`(tabs)/strain.tsx:
- Around line 319-320: The Recent Activities loading branch still uses
trainingQuery.isLoading directly instead of the centralized isLoading state
derived from shouldShowBlockingLoading. Update the strain.tsx render path to use
the computed isLoading consistently wherever the loading UI is gated, so the
ActivityIndicator and related logic all rely on the same source of truth.
🪄 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 Plus
Run ID: b678cc6b-96a9-4959-803d-2ba0b49b1d49
📒 Files selected for processing (11)
packages/mobile/app/(tabs)/activities.test.tsxpackages/mobile/app/(tabs)/activities.tsxpackages/mobile/app/(tabs)/food.test.tsxpackages/mobile/app/(tabs)/food.tsxpackages/mobile/app/(tabs)/index.test.tsxpackages/mobile/app/(tabs)/index.tsxpackages/mobile/app/(tabs)/strain.test.tsxpackages/mobile/app/(tabs)/strain.tsxpackages/mobile/app/providers/[id].test.tsxpackages/mobile/app/providers/[id].tsxpackages/mobile/app/providers/index.tsx
👮 Files not reviewed due to content moderation or server errors (1)
- packages/mobile/app/(tabs)/strain.test.tsx
|
Addressed CodeRabbit review #1471 (review). Fixed.
|
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Auto-approved: Optimizes mobile loading by keeping cached data visible during refresh and scoping pull-to-refresh invalidation to relevant queries.
Re-trigger cubic
# Conflicts: # packages/mobile/app/(tabs)/food.test.tsx # packages/mobile/app/(tabs)/index.test.tsx # packages/mobile/app/(tabs)/index.tsx # packages/mobile/app/(tabs)/strain.test.tsx # packages/mobile/app/(tabs)/strain.tsx
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: Improves iOS loading UX by keeping cached data visible during refreshes and scoping pull-to-refresh invalidation across mobile screens. Low risk, well-tested.
Re-trigger cubic
Summary
Verification
Fixes #1429
Summary by cubic
Improve iOS load times by keeping cached content visible during background refreshes and tightening pull-to-refresh scopes. Prevents skeleton flicker on Today and Strain, adds an inline loader for Strain activities, and speeds up refresh on Providers, Activities, and Food. Fixes #1429.
Written for commit 1bf6179. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes