Fix chat header delay #750
Conversation
|
Warning Rate limit exceeded@untreu2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 40 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughAdds per-group DMChatData caching and preload/get/clear APIs to ChatNotifier and ChatState, replaces FutureBuilder-based DM loading in chat UI with provider-driven synchronous reads, and adds a small String? extension for null-safe formatting. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant ChatScreen
participant ChatNotifier
participant ChatState
participant API
rect rgb(240,248,255)
Note over User,API: Old flow (FutureBuilder)
User->>ChatScreen: Open DM group
ChatScreen->>ChatScreen: Build with FutureBuilder
ChatScreen->>API: Fetch remote DM/profile data
API-->>ChatScreen: Return data
ChatScreen->>ChatScreen: Rebuild UI with data
ChatScreen-->>User: Show header (delayed)
end
rect rgb(240,255,240)
Note over User,API: New flow (preload + cache)
User->>ChatScreen: Open DM group
ChatScreen->>ChatNotifier: preloadDMChatData(groupId)
alt not cached
ChatNotifier->>ChatNotifier: create/load per-group future (dedupe)
ChatNotifier->>API: fetch user profile(s)
API-->>ChatNotifier: return profiles
ChatNotifier->>ChatState: write dmChatDataCache[groupId]
end
ChatScreen->>ChatState: read dmChatDataCache[groupId]
ChatScreen-->>User: Render header immediately if cached, else show loading stub
note right of ChatNotifier: clearDMChatDataForGroup invalidates cache when called
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/ui/chat/chat_screen.dart (1)
361-388: Consider error state for cached-null DMChatData.The code correctly shows a loading state when data is not cached. However, if
isDataCachedis true butdmChatDatais null (cached failure), the header will display with empty strings (''). Consider showing a placeholder or error indicator for cached-null cases to improve UX.Example:
if (groupType == GroupType.directMessage && !isDataCached) { return const UserProfileInfo.loading(); } + + if (groupType == GroupType.directMessage && dmChatData == null) { + // Handle cached-null case (failed to load) + return UserProfileInfo( + title: 'ui.unknownUser'.tr(), + image: '', + onTap: () => context.push('/chats/${widget.groupId}/info'), + ); + } return UserProfileInfo(lib/config/providers/chat_provider.dart (2)
754-786: Consider early return for non-DM groups.The method loads and caches data for all group types. For efficiency, consider checking if the group is a DM before attempting to load, as group chats will always result in a cached null.
Future<DMChatData?> getDMChatData(String groupId) async { // Return cached data if available if (state.isDMChatDataCached(groupId)) { return state.getDMChatData(groupId); } + // Early return for non-DM groups + final groupType = await ref.read(groupsProvider.notifier).getGroupTypeById(groupId); + if (groupType != GroupType.directMessage) { + return null; + } + // Load data asynchronously and cache it try {
835-836: Add error logging for better debuggability.The catch block silently returns null without logging the error. This makes debugging failed DM data loads difficult. Consider adding a logger call similar to other methods in this class.
} catch (e) { + _logger.warning('Failed to load DMChatData for group $groupId: $e'); return null; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
lib/config/providers/chat_provider.dart(3 hunks)lib/config/states/chat_state.dart(3 hunks)lib/config/states/chat_state.freezed.dart(13 hunks)lib/ui/chat/chat_screen.dart(3 hunks)lib/ui/chat/invite/chat_invite_screen.dart(0 hunks)lib/ui/chat/widgets/chat_header_widget.dart(2 hunks)
💤 Files with no reviewable changes (1)
- lib/ui/chat/invite/chat_invite_screen.dart
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/widgets/chat_header_widget.dartlib/config/states/chat_state.freezed.dartlib/config/states/chat_state.dartlib/config/providers/chat_provider.dartlib/ui/chat/chat_screen.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/widgets/chat_header_widget.dartlib/config/states/chat_state.freezed.dartlib/config/states/chat_state.dartlib/config/providers/chat_provider.dartlib/ui/chat/chat_screen.dart
🧠 Learnings (3)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/chat/widgets/chat_header_widget.dart
📚 Learning: 2025-09-14T21:22:00.962Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.962Z
Learning: In the whitenoise_flutter codebase, the _updateGroupInfo method in GroupsNotifier performs optimistic updates that directly modify Group objects in the provider state (groups list and groupsMap). For non-DM groups, the display name comes from group.name, so updating the Group object directly is sufficient to reflect name changes in the UI without needing to refresh the separate display name cache.
Applied to files:
lib/ui/chat/widgets/chat_header_widget.dart
📚 Learning: 2025-10-12T22:44:49.606Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.606Z
Learning: Applies to lib/**/*.dart : Use freezed to model/manage UI states
Applied to files:
lib/config/states/chat_state.dart
🔇 Additional comments (13)
lib/config/states/chat_state.dart (3)
2-2: LGTM!The import follows the project structure and naming conventions.
28-29: LGTM!The cache field follows the established pattern for per-group state and correctly uses a nullable value type to distinguish between uncached and cached-null entries.
87-95: LGTM!The helper methods correctly implement cache semantics. Note that
isDMChatDataCachedreturnstrueeven when the cached value isnull, which is appropriate for distinguishing "never fetched" from "fetched but null". Verify this semantic is correctly handled in UI code.lib/ui/chat/chat_screen.dart (2)
79-80: LGTM!Correctly preloads DM chat data when switching to a new group, ensuring the header displays immediately.
63-64: Preload correctly handles non-DM groups.Verification confirms
preloadDMChatDatasafely handles all group types: for non-DM groups,getOtherGroupMember()returns null, triggering the null check at line 814. Failed or null results are cached to avoid repeated attempts. The UI properly validates group type before consuming DM data (chat_screen.dart:369). No code changes needed.lib/config/providers/chat_provider.dart (5)
12-12: LGTM!The new imports support the DM chat data caching functionality and use appropriate aliasing.
Also applies to: 14-14, 21-21
243-245: LGTM!Formatting change with no functional impact.
788-793: LGTM!The preload method is straightforward and ensures data availability for immediate UI rendering.
795-798: LGTM!The synchronous accessor provides side-effect-free cache access.
800-807: LGTM!The cache invalidation method correctly maintains state immutability and includes a defensive check.
lib/ui/chat/widgets/chat_header_widget.dart (3)
5-5: LGTM!The import is necessary for the provider-based data access pattern.
120-122: LGTM!The preload calls are correctly placed to ensure data availability on initial render and group switches.
Also applies to: 129-129
135-183: No issues found. TheformatPublicKeyextension method is properly defined and imported.The extension is defined in
lib/utils/string_extensions.dartand imported at line 12 ofchat_header_widget.dart. The usage at line 173 (dmChatData.publicKey?.formatPublicKey() ?? '') correctly applies the null-aware operator and provides a fallback empty string. The code will compile without errors.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/config/providers/chat_provider.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/config/providers/chat_provider.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/config/providers/chat_provider.dart
🔇 Additional comments (2)
lib/config/providers/chat_provider.dart (2)
12-12: LGTM!The new imports are appropriate for the DM chat data caching functionality being added.
Also applies to: 14-14, 21-21
243-245: LGTM!The formatting change improves readability without altering behavior.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Tried it locally and it feels a lot faster 🏃🏻♂️ Left a comment related to logic that I think it's duplicated and already available in the userProfile provider
| final otherMember = ref.read(groupsProvider.notifier).getOtherGroupMember(groupId); | ||
|
|
||
| if (otherMember != null) { | ||
| final user = await wn_users_api.getUser(pubkey: otherMember.publicKey); |
There was a problem hiding this comment.
Why not using user profile provider? it looks like the getUserProfile does the same but it is tested (alles the get user method, then transforms the response to a UserProfile object
There was a problem hiding this comment.
You're right, fixed.
Description
Problem:
Solution:
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Fixes #741
Summary by CodeRabbit
Performance Improvements
UI/UX Enhancements