Chat UI Enhancements #162
Chat UI Enhancements #162
Conversation
…stent dark theme colors
…sage input UI positioning
|
Warning Rate limit exceeded@AndreaDiazCorreia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 3 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 (9)
WalkthroughThis update introduces a comprehensive refactor and modularization of the chat feature. It replaces monolithic and inline UI code with dedicated widgets for each chat component, enhances state management, and improves keyboard/bottom navigation handling. Additional localization support for chat-related strings in English, Spanish, and Italian is provided. Several files receive formatting, comment translations, and robustness improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatRoomsList
participant ChatListItem
participant ChatRoomScreen
participant PeerHeader
participant InfoButtons
participant TradeInformationTab
participant UserInformationTab
participant ChatMessagesList
participant MessageInput
User->>ChatRoomsList: Opens chat list
ChatRoomsList->>ChatListItem: Display sorted chat summaries
User->>ChatListItem: Selects chat
ChatListItem->>ChatRoomScreen: Navigates to chat room
ChatRoomScreen->>PeerHeader: Show peer info
ChatRoomScreen->>InfoButtons: Show info tab toggles
User->>InfoButtons: Toggle info tab
InfoButtons->>TradeInformationTab: Show trade info (if selected)
InfoButtons->>UserInformationTab: Show user info (if selected)
ChatRoomScreen->>ChatMessagesList: Display messages
User->>MessageInput: Type and send message
MessageInput->>ChatRoomScreen: Notify new message
ChatRoomScreen->>ChatMessagesList: Scroll to bottom on new message/keyboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (4)
lib/features/chat/widgets/trade_information_tab.dart (1)
179-181: Consider date formatting consistencyThe date formatting uses a hardcoded format pattern. Consider using localized date formatting or making the format configurable.
- DateFormat('MMMM d, yyyy').format( - DateTime.fromMillisecondsSinceEpoch( - order!.createdAt! * 1000)) + DateFormat.yMMMMd().format( + DateTime.fromMillisecondsSinceEpoch( + order!.createdAt! * 1000))lib/features/chat/widgets/chat_list_item.dart (1)
58-67: Consider performance optimization for unread detectionThe current logic checks all messages for unread status. For large chat histories, consider breaking early once an unread message is found or limiting the search scope.
+ bool hasUnreadMessages = false; for (final message in sortedMessages) { if (message.pubkey == peerPubkey && message.createdAt != null && message.createdAt is int) { final messageTime = DateTime.fromMillisecondsSinceEpoch((message.createdAt as int) * 1000); final now = DateTime.now(); final oneHourAgo = now.subtract(const Duration(hours: 1)); if (messageTime.isAfter(oneHourAgo)) { hasUnreadMessages = true; break; + } else { + // Messages are sorted by time, so if this one is older than 1 hour, + // all subsequent messages will be too + break; } } }lib/features/chat/widgets/message_input.dart (1)
60-69: Use const constructor for BoxDecorationThe BoxDecoration can be declared as const since all its values are compile-time constants, which improves performance.
- decoration: BoxDecoration( + decoration: const BoxDecoration( color: AppTheme.backgroundDark, boxShadow: [ BoxShadow( - color: Colors.black.withValues(alpha: 0.1), + color: Color.fromRGBO(0, 0, 0, 0.1), blurRadius: 4, - offset: const Offset(0, -1), + offset: Offset(0, -1), ), ], ),Note: Since
Colors.black.withValues()is not a const constructor, replace it withColor.fromRGBO()for const compatibility.lib/features/chat/screens/chat_room_screen.dart (1)
129-135: Improve keyboard visibility handling for smoother transitionsThe current implementation switches between
SizedBox()andSafeAreawhich could cause layout jumps. Consider usingVisibilityorAnimatedContainerfor smoother transitions.- child: MediaQuery.of(context).viewInsets.bottom > 0 - ? const SizedBox() // Hide BottomNavBar when keyboard is open - : SafeArea( - top: false, - bottom: true, - child: const BottomNavBar(), - ), + child: AnimatedContainer( + duration: const Duration(milliseconds: 200), + height: MediaQuery.of(context).viewInsets.bottom > 0 ? 0 : null, + child: SafeArea( + top: false, + bottom: true, + child: const BottomNavBar(), + ), + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
lib/features/chat/screens/chat_room_screen.dart(1 hunks)lib/features/chat/screens/chat_rooms_list.dart(1 hunks)lib/features/chat/widgets/chat_list_item.dart(1 hunks)lib/features/chat/widgets/chat_messages_list.dart(1 hunks)lib/features/chat/widgets/chat_tabs.dart(1 hunks)lib/features/chat/widgets/empty_state_view.dart(1 hunks)lib/features/chat/widgets/info_buttons.dart(1 hunks)lib/features/chat/widgets/message_bubble.dart(1 hunks)lib/features/chat/widgets/message_input.dart(1 hunks)lib/features/chat/widgets/peer_header.dart(1 hunks)lib/features/chat/widgets/trade_information_tab.dart(1 hunks)lib/features/chat/widgets/user_information_tab.dart(1 hunks)lib/features/order/screens/take_order_screen.dart(1 hunks)lib/l10n/intl_en.arb(1 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/shared/utils/currency_utils.dart(1 hunks)lib/shared/widgets/order_cards.dart(1 hunks)test/mocks.mocks.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/l10n/intl_{en,es,it}.arb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
lib/l10n/*.arb
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (11)
test/mocks.mocks.dart (5)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to test/mocks.dart : Mocks are generated using Mockito in `test/mocks.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.g.dart : Do not manually edit files ending with `.g.dart`
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/shared/widgets/order_cards.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
lib/features/order/screens/take_order_screen.dart (5)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
lib/features/chat/widgets/message_bubble.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
lib/features/chat/widgets/peer_header.dart (1)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
lib/l10n/intl_en.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for strings with parameters
lib/features/chat/widgets/trade_information_tab.dart (1)
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
lib/l10n/intl_it.arb (1)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
lib/features/chat/widgets/message_input.dart (1)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Use Riverpod for all state management
lib/l10n/intl_es.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (en, es, it)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for strings with parameters
lib/features/chat/screens/chat_rooms_list.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in `main.dart` with `timeago.setLocaleMessages()`
🪛 RuboCop (1.76.1)
lib/l10n/intl_en.arb
[warning] 583-583: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 600-600: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 612-612: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 613-613: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 639-639: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 656-656: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 657-657: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 668-668: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 669-669: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_es.arb
[warning] 631-631: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 648-648: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 649-649: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 660-660: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 661-661: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🔇 Additional comments (30)
test/mocks.mocks.dart (1)
404-404: Lint suppression in generated mock file is appropriate.The addition of the
// ignore: must_be_immutablecomment above theMockSharedPreferencesAsyncclass is a valid approach to suppress lint warnings for mock classes. Mock classes generated by Mockito often trigger immutability warnings but don't need to follow the same constraints as regular classes since they're used for testing purposes.lib/shared/utils/currency_utils.dart (3)
1-1: LGTM: Import added for NumberFormat usage.The
intlpackage import is correctly added to support the newformatSatsmethod.
10-10: Good improvement: Explicit join clarifies intent.Making the join operation explicit with an empty string improves code readability and removes ambiguity.
17-22: Well-implemented utility method.The
formatSatsmethod properly usesNumberFormat('#,###')to add thousand separators. The documentation comment and method signature are clear and appropriate.lib/shared/widgets/order_cards.dart (1)
44-44: Good adherence to localization guidelines.The change to pass
amountandcurrencyas separate parameters toS.of(context)!.forAmountfollows the coding guidelines and simplifies the localization logic by removing the currency flag concatenation.lib/features/order/screens/take_order_screen.dart (1)
91-91: Consistent localization pattern implementation.The modification to pass
order.fiatAmount.toString()andorder.currency!as separate parameters toS.of(context)!.forAmountmaintains consistency with similar changes across the codebase and follows the established localization guidelines.lib/features/chat/widgets/empty_state_view.dart (1)
4-21: Well-structured empty state widget.The
EmptyStateViewwidget follows Flutter best practices with:
- Proper const constructor usage
- Single responsibility principle
- Appropriate use of theme colors
- Clean, readable implementation
lib/features/chat/widgets/message_bubble.dart (2)
7-16: Well-designed message bubble widget.The widget follows Flutter best practices with proper const constructor usage and clear parameter naming.
41-55: Good clipboard functionality with proper localization.The clipboard copy feature is well-implemented with:
- Proper use of localization strings following coding guidelines
- Appropriate snackbar styling with theme colors
- Good user experience with floating behavior and rounded corners
lib/features/chat/widgets/chat_tabs.dart (1)
1-68: Well-structured tab widget implementation.The
ChatTabswidget follows Flutter best practices with proper separation of concerns, correct localization usage, and good theming. The implementation handles tab state management appropriately.lib/features/chat/widgets/peer_header.dart (1)
1-62: Clean peer header implementation with proper Riverpod usage.The widget correctly uses
ConsumerWidgetand handles peer information appropriately by receivingpeerPubkeyas a parameter. The UI structure and localization usage follow best practices.lib/features/chat/widgets/chat_messages_list.dart (1)
1-73: Excellent message list implementation with proper lifecycle management.The widget correctly handles ScrollController disposal, implements auto-scrolling for new messages, and safely handles timestamp sorting with appropriate null checks. The use of
didUpdateWidgetfor auto-scroll detection is a good pattern.lib/l10n/intl_en.arb (1)
560-618: Localization keys verification passedI’ve confirmed that all the new chat keys added to
intl_en.arbare already present in bothintl_es.arbandintl_it.arb. No further changes needed.lib/features/chat/widgets/info_buttons.dart (1)
1-76: Well-implemented toggle button widget with proper state management.The
InfoButtonswidget correctly handles selection state, provides good visual feedback for selected/unselected states, and uses proper localization. The toggle logic and styling implementation are well done.lib/features/chat/widgets/trade_information_tab.dart (4)
1-6: LGTM: Clean import structureThe imports are well-organized and all appear to be necessary for the widget's functionality. No unused imports detected.
12-16: LGTM: Proper const constructor usageThe constructor correctly uses
constand follows Flutter best practices with thesuper.keyparameter. The required parameters are appropriately marked.
20-26: LGTM: Proper null handling with loading stateThe null check for the order parameter is handled correctly, showing a loading spinner when data is not available. The use of
AppTheme.mostroGreenfor consistent theming is good practice.
46-46: LGTM: Consistent localization usageAll user-facing strings correctly use the
S.of(context)!.keyNamepattern as specified in the coding guidelines. The localization keys are descriptive and follow proper naming conventions.Also applies to: 82-83, 114-114, 138-138, 170-170, 182-182
lib/features/chat/widgets/chat_list_item.dart (4)
1-12: LGTM: Well-organized importsAll imports appear necessary and are properly organized. The use of Riverpod, routing, and other Flutter/Dart libraries is appropriate for the widget's functionality.
16-16: LGTM: Proper const constructorThe constructor follows Flutter best practices with
constandsuper.key.
40-44: LGTM: Robust message sorting logicThe sorting logic properly handles both int and non-int values for
createdAt, with appropriate fallbacks. The type checking ensures safety.
198-217: LGTM: Comprehensive date formatting helperThe
formatDateTimemethod handles all the different time scenarios appropriately and uses proper localization for relative dates.lib/features/chat/widgets/user_information_tab.dart (4)
1-8: LGTM: Clean import structureAll imports are necessary and well-organized for the widget's functionality.
14-18: LGTM: Proper const constructorThe constructor follows Flutter best practices with appropriate use of
constand required parameters.
61-61: LGTM: Consistent localization usageAll user-facing strings properly use
S.of(context)!.keyNamepattern as required by the coding guidelines.Also applies to: 94-94, 103-103, 118-118, 126-126
67-70: LGTM: Good use of ClickableText for key displayThe use of
ClickableTextwidgets for displaying public keys and shared keys is appropriate, making them easily copyable. The null safety handling forsharedKeywith fallback to localized "not available" text is well done.Also applies to: 124-127
lib/l10n/intl_it.arb (2)
616-673: LGTM: Comprehensive chat localization supportThe new localization keys provide excellent coverage for the chat UI enhancements, including proper metadata for parameterized strings. The translations appear accurate and consistent with the existing Italian localizations.
668-669: Fix duplicate keys: yourInformation and yourHandleThese keys appear to be duplicated from earlier in the file.
- "yourInformation": "Le Tue Informazioni", - "yourHandle": "Il Tuo Identificativo:",Remove these duplicate entries as they are already defined earlier in the file.
Likely an incorrect or invalid review comment.
lib/l10n/intl_es.arb (2)
608-665: LGTM: Excellent Spanish localization coverageThe new chat-related localization keys provide comprehensive support for the UI enhancements. The Spanish translations are accurate and maintain consistency with existing terminology. The proper use of ARB metadata for parameterized strings follows best practices.
660-661: Fix duplicate keys: yourInformation and yourHandleThese keys are duplicated from earlier in the file.
- "yourInformation": "Tu Información", - "yourHandle": "Tu Identificador:",Remove these duplicate entries as they are already defined earlier in the file.
Likely an incorrect or invalid review comment.
| }, | ||
| "yourInformation": "Your Information", | ||
| "yourHandle": "Your Handle:", | ||
| "yourSharedKey": "Your Shared Key:", |
There was a problem hiding this comment.
Fix duplicate localization key.
The key "yourSharedKey" appears to be duplicated (also at line 362). This duplication needs to be resolved.
Remove the duplicate entry and ensure unique keys throughout the file.
🧰 Tools
🪛 RuboCop (1.76.1)
[warning] 613-613: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🤖 Prompt for AI Agents
In lib/l10n/intl_en.arb at line 613, there is a duplicate localization key
"yourSharedKey" which also appears at line 362. To fix this, remove the
duplicate entry at line 613 and ensure that all localization keys in the file
are unique to avoid conflicts.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/features/chat/widgets/chat_list_item.dart (1)
20-28: Null safety properly implemented.The code correctly handles nullable session and peer values with early return for null cases, making the subsequent
session.peer!usage safe.
🧹 Nitpick comments (3)
lib/features/chat/widgets/chat_list_item.dart (3)
82-82: Remove empty else block.The empty else block serves no purpose and should be removed for cleaner code.
- } else {}
60-74: Consider improving unread message detection logic.The current logic only marks messages as unread if they're from the last hour. Consider implementing a proper read/unread state tracking mechanism instead of using time-based heuristics, as users might want to see unread indicators for older messages too.
210-210: Clarify opacity comment.The comment
// 0.05 opacitynext toalpha: 13is confusing since 13/255 ≈ 0.051, not exactly 0.05. Consider using the actual calculated value or usingwithValues(alpha: 0.05)for consistency.- color: Colors.white.withValues(alpha: 13), // 0.05 opacity + color: Colors.white.withValues(alpha: 0.05),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/chat/widgets/chat_list_item.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.dart
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (1)
lib/features/chat/widgets/chat_list_item.dart (3)
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Target zero `flutter analyze` issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-08T17:55:04.549Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom `timeAgoWithLocale()` method is implemented in NostrEvent extension
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
lib/features/chat/widgets/chat_list_item.dart (3)
1-11: LGTM! Clean and necessary imports.All imports are used and properly organized following Dart conventions.
35-35: Excellent localization compliance.All user-facing strings properly use
S.of(context).yourKeyas required by the coding guidelines.Also applies to: 38-39, 57-57, 272-272
261-281: Well-implemented date formatting helper.The contextual date formatting provides excellent UX with appropriate granularity (time for today, "Yesterday", weekday names, dates) and proper localization.
|
@Catrya @grunch Please test it thoroughly beforehand. For some reason, the chats are not being ordered as they should (the most recent one at the top), and the red dot indicating an unread message is also not showing. I'm not entirely sure what's missing, @chebizarro could you check if I forgot to add something, please? |
There was a problem hiding this comment.
Hola @AndreaDiazCorreia
- El nym del vendedor no se está mostrando
- Al seleccionar el botón de Contactar (contactar al vendedor) la primera vez salió así:
- Al seleceeionar el botón del chat en la barra, no se muestra el chat con la contraparte (al inicio de la conversación, luego sí se mostraba)
-
A veces cuando se envía un mensaje no le aparece al que lo envía, ni a la contraparte, no se envía
-
A veces al entrar al detalle de la orden se muestra esto:
- Los textos de Información del intercambio, e Información del usuario se salen de su espacio
- Al ir a la pestaña de Disputas, aunque no esté hecha esa pantalla, debería resaltar la pestaña en verde, como lo hace con Mensajes
- Cuando envío un mensaje, si es corto, queda tapado y parece que desapareció, pero al volver a escribir se muestra. Si el mensaje es más grande no queda tapado por completo
- Debe mantenerse como en la rama main, que en dependencia del color del logo del nym de la contraparte, es el color del fondo del cuadro de texto de la contraparte
… list scroll handling
|
@Catrya para el punto 1 y 5 abreles un issue nuevo porque esos archivos no se tocaron en este pr, el punto 2 no lo pude emular, lo probe en emulador, en mi celular y en otro celular y nunca me paso, lo demas debio haber quedado pero prueba y me dices cualquier cambio en los chats |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
lib/l10n/intl_en.arb (1)
583-583: Fix duplicate localization keys identified by static analysis.The following keys are duplicated and need to be removed:
createdOnat line 583 (also at line 257)forAmountat line 600 (also at line 204)yourHandleat line 612 (also at line 361)yourSharedKeyat line 613 (also at line 362)Remove the duplicate entries to ensure unique keys throughout the file:
- "createdOn": "Created on:",- "forAmount": "for {amount}",- "yourHandle": "Your handle: {handle}",- "yourSharedKey": "Your shared key:",Also applies to: 600-600, 612-612, 613-613
lib/features/chat/widgets/chat_list_item.dart (1)
98-98: Ensure Flutter SDK compatibility for Color.withValues API usage.The
withValues(alpha:...)API requires Flutter 3.27.0 or higher. Please verify yourpubspec.yamlspecifies the minimum Flutter version constraint or consider usingwithOpacity()for broader compatibility.Also applies to: 172-172, 221-221
lib/l10n/intl_es.arb (1)
638-638: Fix duplicate keys identified in Spanish localization.The following duplicate keys need to be removed to maintain ARB file integrity:
createdOnat line 638 (duplicated from earlier in file)forAmountat line 655 (duplicated from earlier in file)yourHandleat line 667 (duplicated from earlier in file)yourSharedKeyat line 668 (duplicated from earlier in file)Remove these duplicate entries as they cause localization conflicts:
- "createdOn": "Creado el:",- "forAmount": "por {amount} {currency}", - "@forAmount": { - "placeholders": { - "amount": { - "type": "String" - }, - "currency": { - "type": "String" - } - } - },- "yourHandle": "Tu Identificador:",- "yourSharedKey": "Tu Clave Compartida:",Also applies to: 655-655, 667-667, 668-668
lib/l10n/intl_it.arb (2)
646-646: Remove duplicate key: createdOnThis key is duplicated - it already exists earlier in the file. Remove this duplicate entry to avoid key conflicts.
- "createdOn": "Creato il:",
663-673: Remove duplicate key and metadata: forAmountThe key "forAmount" and its metadata are duplicated - they already exist earlier in the file. Remove this entire duplicate entry.
- "forAmount": "per {amount} {currency}", - "@forAmount": { - "placeholders": { - "amount": { - "type": "String" - }, - "currency": { - "type": "String" - } - } - },
🧹 Nitpick comments (3)
lib/features/chat/notifiers/chat_room_notifier.dart (1)
106-131: Good UX improvement with immediate local state update.The immediate local state update before network send provides better user experience. The error handling and logging are well-implemented.
Consider implementing the retry logic mentioned in the TODO comment:
- // TODO: Could implement retry logic or show error to user + // TODO: Implement retry logic and show error to user + // Could add exponential backoff retry or user notificationlib/features/chat/notifiers/chat_rooms_notifier.dart (1)
59-90: New refreshChatList method enables real-time chat updates.The refresh mechanism with delay and fresh state reads supports the chat UI requirements. The force update with spread operator
[...chats]ensures UI refresh triggers properly.Consider making the delay configurable:
- await Future.delayed(const Duration(milliseconds: 100)); + await Future.delayed(const Duration(milliseconds: 100)); // Consider making configurablelib/features/chat/widgets/message_input.dart (1)
78-96: Consider more specific localization keys for better user experienceThe disabled message logic is functional, but could be improved with more specific localization keys for each order state instead of reusing
invalidOrderStatusfor multiple scenarios.The current implementation is safe with proper fallbacks, but consider using the new specific keys that were added to the localization files:
orderCompletedMessagingDisabledfor completed ordersorderExpiredMessagingDisabledfor expired ordersorderSettledMessagingDisabledfor admin-settled ordersThis would provide clearer user feedback about why messaging is disabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
lib/data/models/nostr_event.dart(1 hunks)lib/features/chat/notifiers/chat_room_notifier.dart(3 hunks)lib/features/chat/notifiers/chat_rooms_notifier.dart(1 hunks)lib/features/chat/screens/chat_room_screen.dart(1 hunks)lib/features/chat/screens/chat_rooms_list.dart(1 hunks)lib/features/chat/widgets/chat_list_item.dart(1 hunks)lib/features/chat/widgets/chat_messages_list.dart(1 hunks)lib/features/chat/widgets/info_buttons.dart(1 hunks)lib/features/chat/widgets/message_bubble.dart(1 hunks)lib/features/chat/widgets/message_input.dart(1 hunks)lib/features/order/models/order_state.dart(1 hunks)lib/features/order/screens/add_order_screen.dart(1 hunks)lib/l10n/intl_en.arb(2 hunks)lib/l10n/intl_es.arb(1 hunks)lib/l10n/intl_it.arb(1 hunks)lib/shared/providers/legible_handle_provider.dart(1 hunks)lib/shared/utils/nostr_utils.dart(7 hunks)test/mocks.mocks.dart(1 hunks)
📓 Path-based instructions (7)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/order/models/order_state.dartlib/shared/providers/legible_handle_provider.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dartlib/features/chat/widgets/chat_messages_list.dartlib/features/chat/screens/chat_rooms_list.dartlib/features/chat/widgets/message_input.dartlib/data/models/nostr_event.dartlib/features/chat/screens/chat_room_screen.dartlib/features/chat/widgets/info_buttons.dartlib/features/chat/widgets/chat_list_item.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/order/models/order_state.dartlib/shared/providers/legible_handle_provider.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dartlib/features/chat/widgets/chat_messages_list.dartlib/features/chat/screens/chat_rooms_list.dartlib/features/chat/widgets/message_input.dartlib/data/models/nostr_event.dartlib/features/chat/screens/chat_room_screen.dartlib/features/chat/widgets/info_buttons.dartlib/features/chat/widgets/chat_list_item.dart
lib/features/*/notifiers/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Notifiers for complex state logic are placed in
features/{feature}/notifiers/
Files:
lib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dart
lib/data/models/nostr_event.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Custom
timeAgoWithLocale()method is implemented in the NostrEvent extension
Files:
lib/data/models/nostr_event.dart
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
Add new localization keys to all three ARB files (
intl_en.arb,intl_es.arb,intl_it.arb)
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use proper ARB metadata for parameterized strings
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
lib/l10n/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Internationalization files are located in
lib/l10n/
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
🧠 Learnings (13)
lib/features/order/models/order_state.dart (3)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart
Learnt from: chebizarro
PR: #110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: #110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via tradeState.peer?.publicKey from the OrderState instance rather than through session?.peer?.publicKey, as the OrderState encapsulates peer information directly.
lib/shared/providers/legible_handle_provider.dart (1)
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/features/chat/notifiers/chat_room_notifier.dart (15)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: #74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The reload() method in the chat_room_notifier is synchronous, so await is not required or possible.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, eventStorageProvider is exported from package:mostro_mobile/shared/providers/mostro_service_provider.dart and not from a separate event_storage_provider.dart file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//providers/**/.dart : Providers are organized by feature in features/{feature}/providers/
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with @Riverpod annotations. Providers like eventStorageProvider are generated in .g.dart files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., mostro_service_provider.dart), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/background/**/*.dart : Background services are implemented in the background/ directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/repositories/**/*.dart : All data access should go through repository classes in data/repositories/
lib/features/chat/notifiers/chat_rooms_notifier.dart (3)
Learnt from: chebizarro
PR: #74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The reload() method in the chat_room_notifier is synchronous, so await is not required or possible.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
lib/features/chat/widgets/chat_messages_list.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
lib/features/chat/screens/chat_rooms_list.dart (8)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Use S.of(context).yourKey for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Target zero flutter analyze issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in main.dart with timeago.setLocaleMessages()
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Use Riverpod for all state management
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart
lib/features/chat/widgets/message_input.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Use S.of(context).yourKey for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Target zero flutter analyze issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Use Riverpod for all state management
lib/data/models/nostr_event.dart (9)
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, eventStorageProvider is exported from package:mostro_mobile/shared/providers/mostro_service_provider.dart and not from a separate event_storage_provider.dart file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom timeAgoWithLocale() method is implemented in the NostrEvent extension
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
lib/features/chat/screens/chat_room_screen.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to **/*.dart : Use const constructors where possible
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
lib/features/chat/widgets/chat_list_item.dart (6)
Learnt from: chebizarro
PR: #110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via tradeState.peer?.publicKey from the OrderState instance rather than through session?.peer?.publicKey, as the OrderState encapsulates peer information directly.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Target zero flutter analyze issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Always check mounted before using context after async operations
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Always use latest APIs (e.g., withValues() instead of withOpacity())
lib/l10n/intl_en.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
lib/l10n/intl_es.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
lib/l10n/intl_it.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
🪛 RuboCop (1.76.1)
lib/l10n/intl_en.arb
[warning] 583-583: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 600-600: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 612-612: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 613-613: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_es.arb
[warning] 638-638: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 655-655: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 656-656: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 667-667: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 668-668: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 646-646: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 663-663: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 664-664: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 675-675: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 676-676: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
✅ Files skipped from review due to trivial changes (3)
- test/mocks.mocks.dart
- lib/shared/utils/nostr_utils.dart
- lib/features/order/screens/add_order_screen.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/chat/widgets/message_bubble.dart
🧰 Additional context used
📓 Path-based instructions (7)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/order/models/order_state.dartlib/shared/providers/legible_handle_provider.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dartlib/features/chat/widgets/chat_messages_list.dartlib/features/chat/screens/chat_rooms_list.dartlib/features/chat/widgets/message_input.dartlib/data/models/nostr_event.dartlib/features/chat/screens/chat_room_screen.dartlib/features/chat/widgets/info_buttons.dartlib/features/chat/widgets/chat_list_item.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/order/models/order_state.dartlib/shared/providers/legible_handle_provider.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dartlib/features/chat/widgets/chat_messages_list.dartlib/features/chat/screens/chat_rooms_list.dartlib/features/chat/widgets/message_input.dartlib/data/models/nostr_event.dartlib/features/chat/screens/chat_room_screen.dartlib/features/chat/widgets/info_buttons.dartlib/features/chat/widgets/chat_list_item.dart
lib/features/*/notifiers/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Notifiers for complex state logic are placed in
features/{feature}/notifiers/
Files:
lib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/notifiers/chat_rooms_notifier.dart
lib/data/models/nostr_event.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
Custom
timeAgoWithLocale()method is implemented in the NostrEvent extension
Files:
lib/data/models/nostr_event.dart
lib/l10n/intl_{en,es,it}.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
Add new localization keys to all three ARB files (
intl_en.arb,intl_es.arb,intl_it.arb)
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use proper ARB metadata for parameterized strings
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
lib/l10n/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Internationalization files are located in
lib/l10n/
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
🧠 Learnings (13)
lib/features/order/models/order_state.dart (3)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart
Learnt from: chebizarro
PR: #110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Learnt from: chebizarro
PR: #110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via tradeState.peer?.publicKey from the OrderState instance rather than through session?.peer?.publicKey, as the OrderState encapsulates peer information directly.
lib/shared/providers/legible_handle_provider.dart (1)
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
lib/features/chat/notifiers/chat_room_notifier.dart (15)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: #74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The reload() method in the chat_room_notifier is synchronous, so await is not required or possible.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, eventStorageProvider is exported from package:mostro_mobile/shared/providers/mostro_service_provider.dart and not from a separate event_storage_provider.dart file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//providers/**/.dart : Providers are organized by feature in features/{feature}/providers/
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with @Riverpod annotations. Providers like eventStorageProvider are generated in .g.dart files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., mostro_service_provider.dart), not by importing a separate provider file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models.dart : Models should be exported through data/models.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/background/**/*.dart : Background services are implemented in the background/ directory
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/repositories/**/*.dart : All data access should go through repository classes in data/repositories/
lib/features/chat/notifiers/chat_rooms_notifier.dart (3)
Learnt from: chebizarro
PR: #74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The reload() method in the chat_room_notifier is synchronous, so await is not required or possible.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
lib/features/chat/widgets/chat_messages_list.dart (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
lib/features/chat/screens/chat_rooms_list.dart (8)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Use S.of(context).yourKey for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Target zero flutter analyze issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/main.dart : Locales for the timeago package are configured in main.dart with timeago.setLocaleMessages()
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Use Riverpod for all state management
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart
lib/features/chat/widgets/message_input.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Use S.of(context).yourKey for all user-facing strings
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Target zero flutter analyze issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Use Riverpod for all state management
lib/data/models/nostr_event.dart (9)
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: Catrya
PR: #215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Learnt from: chebizarro
PR: #74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, eventStorageProvider is exported from package:mostro_mobile/shared/providers/mostro_service_provider.dart and not from a separate event_storage_provider.dart file.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/data/models/nostr_event.dart : Custom timeAgoWithLocale() method is implemented in the NostrEvent extension
Learnt from: chebizarro
PR: #127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Learnt from: chebizarro
PR: #127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: chebizarro
PR: #74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
lib/features/chat/screens/chat_room_screen.dart (4)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to **/*.dart : Use const constructors where possible
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/services/nostr_service.dart : Nostr protocol interactions go through services/nostr_service.dart
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
lib/features/chat/widgets/chat_list_item.dart (6)
Learnt from: chebizarro
PR: #110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via tradeState.peer?.publicKey from the OrderState instance rather than through session?.peer?.publicKey, as the OrderState encapsulates peer information directly.
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Target zero flutter analyze issues
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/**/*.dart : Always check mounted before using context after async operations
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to test/mocks.mocks.dart : DO NOT modify test/mocks.mocks.dart - it already has file-level ignores
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: All changes should pass flutter analyze before commit
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Always use latest APIs (e.g., withValues() instead of withOpacity())
lib/l10n/intl_en.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
lib/l10n/intl_es.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
lib/l10n/intl_it.arb (2)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/intl_{en,es,it}.arb : Add new localization keys to all three ARB files (intl_en.arb, intl_es.arb, intl_it.arb)
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
🪛 RuboCop (1.76.1)
lib/l10n/intl_en.arb
[warning] 583-583: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 600-600: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 612-612: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 613-613: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_es.arb
[warning] 638-638: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 655-655: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 656-656: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 667-667: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 668-668: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 646-646: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 663-663: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 664-664: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 675-675: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 676-676: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🔇 Additional comments (39)
lib/features/order/models/order_state.dart (1)
142-149: LGTM: Comment translation improves code clarity.The Spanish-to-English comment translation enhances code readability for the development team while preserving all functional logic.
lib/shared/providers/legible_handle_provider.dart (1)
120-151: Excellent input validation and error handling.The enhanced validation makes the function robust against malformed hex keys with appropriate fallback handles. The try-catch around
BigInt.parseand regex filtering for non-hex characters are particularly well-implemented safeguards.lib/features/chat/notifiers/chat_room_notifier.dart (1)
72-72: Message sorting fixed to show newest first.The descending sort order addresses the PR objective about incorrect chat ordering, ensuring the most recent messages appear at the top.
lib/data/models/nostr_event.dart (1)
102-131: Enhanced event unwrapping supports nested encryption patterns.The improved
mostroUnWrapmethod now handles NIP-17 direct messages (kind 14) nested within kind 13 events, essential for the chat functionality. The fallback approach for unknown kinds is more robust than throwing exceptions.lib/features/chat/notifiers/chat_rooms_notifier.dart (1)
42-56: Refined filtering logic improves chat list management.The consistent filtering criteria between
loadChatsand the newrefreshChatListmethod ensures reliable chat room visibility based on peer presence and recency.lib/features/chat/widgets/chat_messages_list.dart (8)
1-21: Well-structured widget design with proper imports.The class structure and constructor design are excellent, particularly the optional
ScrollControllerparameter which provides flexibility for external scroll management while maintaining internal fallback capability.
27-37: Proper initState implementation with post-frame callback.The initialization logic correctly handles the optional scroll controller and uses
addPostFrameCallbackfor safe initial scrolling operations.
39-46: Excellent resource management in dispose method.The conditional disposal logic correctly prevents disposing externally provided scroll controllers while ensuring proper cleanup of internally created ones.
50-66: Well-implemented widget update handling with smart auto-scroll logic.The method correctly differentiates between new message arrivals and first load scenarios, applying appropriate animation settings for each case.
68-91: Robust public API with comprehensive safety checks.The method provides excellent error handling and safety checks, including controller client verification and scroll extent validation. The try-catch block prevents UI crashes from scroll errors.
93-96: Clean delegation pattern for internal scroll operations.The private method provides a clean internal interface while delegating to the public API implementation.
98-107: Proper message sorting with null safety considerations.The sorting logic correctly handles timestamp null safety and creates a defensive copy of the messages list to avoid mutating the original data.
108-127: Well-optimized ListView implementation for chat messages.The ListView configuration includes appropriate performance optimizations with
AlwaysScrollableScrollPhysicsandcacheExtentfor smooth scrolling with many messages.lib/l10n/intl_en.arb (2)
560-618: Comprehensive chat localization strings with proper ARB metadata.The new chat section provides excellent coverage of chat functionality with well-structured keys and proper placeholder metadata for dynamic content.
635-640: Clear and consistent order disabled messaging strings.The disabled messaging strings provide clear user feedback for different order states with consistent formatting and helpful context.
lib/features/chat/widgets/info_buttons.dart (4)
5-31: Clean widget API design with proper Flutter conventions.The constructor parameters and class structure follow Flutter best practices with clear naming and appropriate nullability.
33-41: Excellent responsive design with accessibility considerations.The dynamic font sizing based on text length and clamped text scale factor demonstrates thoughtful accessibility support and prevents layout overflow issues.
42-63: Modern styling with proper accessibility considerations.The button styling uses the modern
withValuesAPI and includes proper minimum tap target size (44px) for accessibility compliance.
64-91: Well-structured button content with overflow protection.The layout properly handles text overflow with
Flexibleand uses appropriate icons. ThetextScaleFactor: 1.0override works well after the dynamic font size calculation.lib/features/chat/widgets/chat_list_item.dart (6)
19-28: Null safety properly addressed from previous review feedback.The code now correctly handles nullable session and peer data by checking for null values and returning a placeholder widget, which resolves the previous null safety concerns.
40-88: Well-implemented message processing with smart unread detection.The logic correctly identifies the most recent message for preview, implements a reasonable 2-hour window for unread detection, and properly handles message truncation and user prefixes.
108-149: Effective visual status indicators with proper positioning.The stacked indicator system clearly communicates both user role (seller) and unread message status with appropriate positioning adjustments when both indicators are present.
152-206: Well-structured text layout with proper visual hierarchy.The typography and layout create clear information hierarchy with appropriate font sizes, weights, and overflow handling for different text elements.
215-270: Good UX practice with consistent placeholder design.The placeholder widget maintains visual consistency with the actual item while providing appropriate loading state feedback to users.
272-291: Smart contextual date formatting for enhanced UX.The date formatting method provides intuitive time representations (time for today, "Yesterday", day names, dates) that improve the chat experience with contextually appropriate formatting.
lib/l10n/intl_es.arb (2)
615-672: Comprehensive and well-translated Spanish chat strings.The Spanish translations are contextually appropriate and maintain consistency with the English localization structure, including proper ARB metadata for parameterized strings.
608-613: Clear Spanish translations for order disabled states.The Spanish translations for disabled messaging states are well-crafted and provide clear user feedback consistent with the English version structure.
lib/l10n/intl_it.arb (1)
614-681: Excellent localization support for chat featuresThe new chat-related localization keys are well-structured and comprehensive. The Italian translations properly support:
- Order messaging disabled states
- Chat UI elements (messages, disputes, tabs)
- Trade and user information displays
- Placeholder metadata for dynamic content
The ARB metadata follows proper conventions with correct type definitions for placeholders.
lib/features/chat/screens/chat_rooms_list.dart (4)
16-37: Good refactoring to support tabbed interfaceThe conversion from
ConsumerWidgettoConsumerStatefulWidgetwithTabControllerproperly supports the new tabbed interface. The lifecycle management withinitState()anddispose()correctly handles the TabController resources.
43-121: Well-structured tabbed interface implementationThe UI restructure effectively implements a tabbed interface with:
- Proper header section with chat title
- Modular
ChatTabswidget for tab navigation- Descriptive text section
TabBarViewwith messages and disputes tabs- Correctly positioned
BottomNavBarusing Stack/PositionedThe localization string usage with null-aware operators and fallback values (
S.of(context)?.chat ?? 'Chat') is the safest approach for handling potential null values.
123-156: Effective data fetching and UI building patternThe implementation correctly:
- Fetches fresh chat data for each order using individual providers
- Sorts chat rooms by session start time (most recent first)
- Uses the modular
ChatListItemwidget for consistent rendering- Applies proper styling and scrolling physics
The approach ensures users see the most current chat information and recent conversations at the top.
158-172: Robust session time extraction with proper error handlingThe
_getSessionStartTimemethod is well-implemented with:
- Proper try-catch error handling
- Sensible fallback to current time for new chats
- Correct time conversion from milliseconds to seconds
This ensures consistent sorting behavior even when session data is unavailable.
lib/features/chat/widgets/message_input.dart (3)
25-51: Excellent keyboard visibility handling implementationThe FocusNode listener approach effectively addresses the previous architectural concern about keyboard detection in the build method. The implementation properly:
- Detects keyboard visibility changes through focus events
- Closes info panels when keyboard opens via callback
- Manages resources correctly with proper disposal
This is much more efficient than previous Future.microtask approaches.
52-76: Comprehensive message sending with proper order status validationThe implementation correctly:
- Validates input text (trim and empty check)
- Prevents messaging for all appropriate terminal order states
- Uses proper Riverpod provider pattern for message sending
- Covers comprehensive status scenarios (completed, canceled, expired, admin-settled)
This ensures users cannot send messages when orders are no longer active.
98-221: Well-designed input UI with proper state managementThe build method implementation effectively provides:
- Clear disabled state messaging with informative banner
- Proper TextField styling and state management
- Appropriate visual feedback for enabled/disabled states
- Safe localization usage with null-aware operators and fallbacks
- Consistent theming throughout
The UI clearly communicates to users when and why messaging is disabled while maintaining a polished appearance.
lib/features/chat/screens/chat_room_screen.dart (4)
24-39: Excellent implementation addressing previous feedbackThe class structure properly addresses the past review comment by defining
bottomNavBarHeightas a constant. The additional state management for scroll control and keyboard visibility is well-organized with proper resource disposal.
49-75: Robust keyboard detection and auto-scroll implementationThe keyboard visibility handling is excellently implemented with:
- Proper MediaQuery-based detection
- Delayed execution ensuring list is properly built
- Comprehensive validation before attempting scroll
- Error handling to prevent UI crashes
- Smooth animation with appropriate duration and curve
This provides a polished user experience when the keyboard appears.
77-147: Well-architected modular UI compositionThe UI structure effectively demonstrates:
- Proper Stack-based layout for complex positioning
- Excellent modular widget composition promoting reusability
- Consistent theming throughout the interface
- Correct padding calculation using the defined constant
- Conditional rendering of info tabs based on state
The separation of concerns with dedicated widgets (PeerHeader, InfoButtons, etc.) greatly improves maintainability.
149-184: Effective positioning logic with proper keyboard adaptationThe positioned elements are well-implemented with:
- Dynamic MessageInput positioning based on keyboard visibility
- Consistent use of the
bottomNavBarHeightconstant throughout- Proper BottomNavBar hiding when keyboard is open
- Appropriate SafeArea handling for system UI
This creates a seamless user experience that adapts well to keyboard interactions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
lib/l10n/intl_en.arb (3)
582-582: Fix duplicate localization key "createdOn".This key is duplicated from an earlier occurrence in the file (line 267). Remove this duplicate entry.
- "createdOn": "Created on:",
599-609: Fix duplicate localization key "forAmount".The key "forAmount" and its metadata are duplicated from an earlier occurrence (line 214). Remove this duplicate entry.
- "forAmount": "for {amount} {currency}", - "@forAmount": { - "placeholders": { - "amount": { - "type": "String" - }, - "currency": { - "type": "String" - } - } - },
611-612: Fix duplicate localization keys "yourSharedKey".The key "yourSharedKey" is duplicated from an earlier occurrence (line 372). Remove this duplicate entry.
- "yourSharedKey": "Your Shared Key:",lib/l10n/intl_es.arb (2)
638-638: Fix duplicate localization key "createdOn".This key is duplicated from an earlier occurrence in the file. Remove this duplicate entry.
- "createdOn": "Creado el:",
655-665: Fix duplicate localization key "forAmount".The key "forAmount" and its metadata are duplicated from an earlier occurrence. Remove this duplicate entry including the metadata block.
- "forAmount": "por {amount} {currency}", - "@forAmount": { - "placeholders": { - "amount": { - "type": "String" - }, - "currency": { - "type": "String" - } - } - },lib/l10n/intl_it.arb (1)
642-642: Verify and resolve duplicate key issuesStatic analysis tools are flagging potential duplicate keys. Based on past review comments, there were known duplicates for
createdOn,forAmount, andyourHandlekeys. Please verify these are not duplicates in the current file state.#!/bin/bash # Check for duplicate keys in the Italian localization file echo "Checking for duplicate keys in intl_it.arb:" echo -e "\n1. Checking 'createdOn' key:" grep -n '"createdOn"' lib/l10n/intl_it.arb echo -e "\n2. Checking 'forAmount' key:" grep -n '"forAmount"' lib/l10n/intl_it.arb echo -e "\n3. Checking 'yourHandle' key:" grep -n '"yourHandle"' lib/l10n/intl_it.arb echo -e "\n4. Checking 'yourInformation' key:" grep -n '"yourInformation"' lib/l10n/intl_it.arb echo -e "\n5. Checking 'yourSharedKey' key:" grep -n '"yourSharedKey"' lib/l10n/intl_it.arbAlso applies to: 659-669, 671-671
🧹 Nitpick comments (2)
lib/features/chat/screens/chat_rooms_list.dart (1)
168-182: Consider using a consistent fallback time for sorting.Using
DateTime.now()as fallback could cause inconsistent sorting behavior when the list refreshes. Consider using a fixed fallback like 0 or the chat room's first message timestamp.int _getSessionStartTime(ChatRoom chatRoom) { try { // Get the session for this chat room to access startTime final session = ref.read(sessionProvider(chatRoom.orderId)); if (session != null) { // Return the session start time (when the order was taken/contacted) return session.startTime.millisecondsSinceEpoch ~/ 1000; } } catch (e) { // If we can't get the session, fall back to current time } - // Fallback: use current time so new chats appear at top - return DateTime.now().millisecondsSinceEpoch ~/ 1000; + // Fallback: use 0 to place chats without sessions at the bottom + // or use the first message timestamp if available + if (chatRoom.messages.isNotEmpty) { + final firstMessageTime = chatRoom.messages.first.createdAt; + if (firstMessageTime is int) { + return firstMessageTime; + } + } + return 0; }lib/features/chat/notifiers/chat_room_notifier.dart (1)
145-189: Smart implementation with optimistic UI updates.The approach of immediately updating local state before network send provides excellent UX. Consider tracking message send status for the TODO on line 187.
Would you like me to help implement retry logic or message status tracking to handle network failures gracefully?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
lib/features/chat/notifiers/chat_room_notifier.dart(4 hunks)lib/features/chat/providers/chat_room_providers.dart(1 hunks)lib/features/chat/screens/chat_room_screen.dart(1 hunks)lib/features/chat/screens/chat_rooms_list.dart(1 hunks)lib/features/chat/widgets/chat_messages_list.dart(1 hunks)lib/features/chat/widgets/trade_information_tab.dart(1 hunks)lib/features/chat/widgets/user_information_tab.dart(1 hunks)lib/l10n/intl_en.arb(4 hunks)lib/l10n/intl_es.arb(3 hunks)lib/l10n/intl_it.arb(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/features/chat/widgets/chat_messages_list.dart
- lib/features/chat/widgets/user_information_tab.dart
- lib/features/chat/screens/chat_room_screen.dart
🧰 Additional context used
📓 Path-based instructions (5)
lib/features/*/providers/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Providers are organized by feature in
features/{feature}/providers/
Files:
lib/features/chat/providers/chat_room_providers.dart
lib/features/*/{screens,providers,notifiers,widgets}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/widgets/trade_information_tab.dartlib/features/chat/screens/chat_rooms_list.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/widgets/trade_information_tab.dartlib/features/chat/screens/chat_rooms_list.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/widgets/trade_information_tab.dartlib/features/chat/screens/chat_rooms_list.dart
lib/l10n/*.arb
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/l10n/*.arb: Internationalization ARB files must be located inlib/l10n/
Add new localization keys to all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
Files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
🧠 Learnings (24)
📚 Learning: applies to lib/shared/providers/mostro_database_provider.dart : database initialization must occur i...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization must occur in `shared/providers/mostro_database_provider.dart`
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: the `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible.
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/services/nostr_service.dart : all nostr protocol interactions must go through `servic...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/services/nostr_service.dart : All Nostr protocol interactions must go through `services/nostr_service.dart`
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in abstractmostronotifier, the handleevent method is synchronous and returns void, not future....
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to test/mocks.mocks.dart : do not add individual `// ignore: must_be_immutable` comments to ...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to test/mocks.mocks.dart : Do not add individual `// ignore: must_be_immutable` comments to classes in `test/mocks.mocks.dart`
Applied to files:
lib/features/chat/providers/chat_room_providers.dart
📚 Learning: in abstractmostronotifier, state updates occur for all messages regardless of timestamp to hydrate t...
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dartlib/features/chat/screens/chat_rooms_list.dart
📚 Learning: in the mostro mobile background service architecture, events aren't stored by the background process...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/**/*.dart : always check `mounted` before using context after async operations...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/**/*.dart : Always check `mounted` before using context after async operations
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to **/*.dart : use `const` constructors where possible...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro mobile codebase, `eventstorageprovider` is exported from `package:mostro_mobile/shared...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/data/models.dart : models must be exported through `data/models.dart`...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/data/models.dart : Models must be exported through `data/models.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro mobile codebase, riverpod code generation is used with `@riverpod` annotations. provid...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to **/*.dart : remove unused imports and dependencies...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/core/mostro_fsm.dart : order state transitions must be managed by `core/mostro_fsm.da...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions must be managed by `core/mostro_fsm.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the nostr protocol, event.id will never be null in events returned by relay subscriptions, so nul...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the context of the mostro mobile app, the `tradestate` class is specifically constructed using th...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/trades/models/trade_state.dart:1-15
Timestamp: 2025-05-08T16:06:33.665Z
Learning: In the context of the Mostro Mobile app, the `TradeState` class is specifically constructed using the `tradeStateProvider`. While some fields are nullable (`lastAction` and `orderPayload`), they are still marked as required parameters to ensure they are explicitly considered during state construction.
Applied to files:
lib/features/chat/widgets/trade_information_tab.dart
📚 Learning: applies to lib/main.dart : locales for the timeago package must be configured in `main.dart` with `t...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/main.dart : Locales for the timeago package must be configured in `main.dart` with `timeago.setLocaleMessages()`
Applied to files:
lib/features/chat/widgets/trade_information_tab.dartlib/features/chat/screens/chat_rooms_list.dart
📚 Learning: all changes should pass `flutter analyze` before commit...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: All changes should pass `flutter analyze` before commit
Applied to files:
lib/features/chat/widgets/trade_information_tab.dartlib/features/chat/screens/chat_rooms_list.dart
📚 Learning: applies to lib/**/*.dart : use `s.of(context).yourkey` for all user-facing strings...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
📚 Learning: zero `flutter analyze` issues must be maintained...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Zero `flutter analyze` issues must be maintained
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
📚 Learning: applies to lib/l10n/*.arb : add new localization keys to all three arb files (en, es, it)...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/l10n/*.arb : Add new localization keys to all three ARB files (en, es, it)
Applied to files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
📚 Learning: applies to lib/l10n/*.arb : use proper arb metadata for parameterized strings...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/l10n/*.arb : Use proper ARB metadata for parameterized strings
Applied to files:
lib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arb
📚 Learning: applies to lib/l10n/*.arb : internationalization arb files must be located in `lib/l10n/`...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Applies to lib/l10n/*.arb : Internationalization ARB files must be located in `lib/l10n/`
Applied to files:
lib/l10n/intl_en.arb
📚 Learning: regenerate localization files after arb modifications...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.714Z
Learning: Regenerate localization files after ARB modifications
Applied to files:
lib/l10n/intl_en.arb
🪛 RuboCop (1.76.1)
lib/l10n/intl_en.arb
[warning] 582-582: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 599-599: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 611-611: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_es.arb
[warning] 638-638: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 655-655: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 656-656: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
lib/l10n/intl_it.arb
[warning] 642-642: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 659-659: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 660-660: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
[warning] 671-671: Duplicated key in hash literal.
(Lint/DuplicateHashKey)
🔇 Additional comments (19)
lib/l10n/intl_en.arb (2)
371-371: LGTM - Updated localization key.The removal of the
{handle}placeholder fromyourHandlesimplifies the string structure appropriately.
559-640: LGTM - Comprehensive chat localization additions.The new chat-related localization keys provide excellent coverage for the chat UI functionality, including:
- Chat section labels (messages, disputes)
- Dynamic chat participant strings
- Trade and user information labels
- Order status messaging for disabled states
- Timestamps and message prefixes
The parameterized strings use proper ARB metadata format.
lib/features/chat/widgets/trade_information_tab.dart (5)
8-16: LGTM - Well-structured widget constructor.The widget properly accepts both optional
Orderand requiredorderIdparameters, providing flexibility for different usage scenarios.
18-37: LGTM - Excellent null safety and date handling.The utility methods demonstrate good defensive programming:
_formatOrderDatesafely handles null dates with localized fallback_getOrderCreationDateprovides reasonable fallback logic with Unix timestamp conversion- Proper handling of edge cases with meaningful defaults
40-47: LGTM - Appropriate loading state handling.Clean loading indicator implementation using consistent app theming when order data is not available.
102-104: LGTM - Proper localization and formatting integration.Good use of the new
CurrencyUtils.formatSatsutility and conditional localization strings based on order type.
198-199: LGTM - Safe date formatting implementation.The combination of
_getOrderCreationDate()and_formatOrderDate()properly handles all edge cases for date display, addressing potential null safety concerns.lib/l10n/intl_es.arb (2)
327-327: LGTM - Updated Spanish localization.The removal of the
{handle}placeholder fromyourHandlematches the English version appropriately.
607-671: LGTM - Comprehensive Spanish chat translations.Excellent Spanish translations for all the new chat functionality, maintaining consistency with the English version while being culturally appropriate.
lib/features/chat/screens/chat_rooms_list.dart (2)
19-41: LGTM! Proper state management implementation.The conversion to
ConsumerStatefulWidgetis well implemented with proper lifecycle management for theTabController.
143-153: Good approach for ensuring fresh chat data.Watching individual chat providers and sorting by session start time ensures the UI always displays the most current state.
lib/features/chat/notifiers/chat_room_notifier.dart (4)
31-41: Great improvement! Async operations properly moved out of constructor.The refactoring to move async initialization to a separate
initialize()method addresses the race condition concerns and improves testability.
43-86: Excellent reactive subscription implementation.The listener-based approach for waiting for session availability is clean and efficient. Proper cleanup of listeners prevents memory leaks.
88-143: Robust event handling with proper deduplication.The implementation correctly:
- Prevents duplicate event processing
- Stores events with complete metadata including order_id
- Maintains message list integrity with deduplication
- Triggers UI updates across the app
191-314: Excellent historical message loading with backward compatibility.The implementation handles edge cases well:
- Backward compatibility for events without order_id
- Detailed logging for troubleshooting
- Resilient to individual message processing failures
- Clear user feedback through logging
lib/l10n/intl_it.arb (4)
332-332: LGTM: Placeholder removal from yourHandle keyThe modification to remove the
{handle}placeholder from the"yourHandle"key is consistent with the changes described in the AI summary. This change aligns with the new chat UI implementation.
612-617: LGTM: Order disabled messaging keysThese new localization keys provide appropriate Italian translations for various order states where messaging is disabled. The translations are clear and consistent with the overall tone of the application.
619-676: LGTM: Comprehensive chat localization additionsThe new chat section provides extensive Italian translations for the chat functionality including:
- Chat tabs (messages, disputes)
- Dynamic content with proper ARB metadata for placeholders
- User and trade information labels
- Temporal references (today, yesterday)
- Action feedback messages
The ARB metadata is properly structured for parameterized strings like
youAreChatting,sellingSats,buyingSats, andforAmount.
619-676: Localization consistency verified across all languagesAll chat-related keys (
messages,disputes,noDisputesAvailable,youAreChatting,tradeInformation,userInformation,orderCompletedMessagingDisabled,messageCopiedToClipboard) are present inlib/l10n/intl_en.arb,intl_es.arb, andintl_it.arb. No further changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
lib/features/chat/notifiers/chat_room_notifier.dart (1)
48-89: Prevent cross‐chat contamination by using a session‐specific subscriptionThe current implementation listens to
subscriptionManager.chat(the global chat stream), which broadcasts events for all chat sessions. This means every ChatRoomNotifier will receive every chat event, regardless oforderId. To fix, subscribe to a per‐session filtered stream or filter incoming events by the current session’s shared key before handling them.Locations to update:
- lib/features/chat/notifiers/chat_room_notifier.dart – subscribe() method (lines 60–62 and 84–86)
Suggested change—replace:
final subscriptionManager = ref.read(subscriptionManagerProvider); _subscription = subscriptionManager.chat.listen(_onChatEvent);With a session‐filtered subscription:
final subscriptionManager = ref.read(subscriptionManagerProvider); _subscription = subscriptionManager.subscribeSession( type: SubscriptionType.chat, session: session!, createFilter: (s) => NostrFilter( kinds: [1059], p: [s.sharedKey!.public], ), ).listen(_onChatEvent);This ensures each ChatRoomNotifier only receives events for its own
orderId.
♻️ Duplicate comments (2)
lib/features/chat/notifiers/chat_room_notifier.dart (2)
29-44: Excellent resolution of constructor async operations issue!The new
initialize()method properly addresses the previous concern by moving async operations out of the constructor. This eliminates race conditions and makes testing easier.
91-146: Solid event handling with storage integration.Good implementation of duplicate prevention and proper event storage. The chat rooms list integration keeps the UI synchronized.
Consider checking the shared key before storing the event to avoid storing irrelevant events:
void _onChatEvent(NostrEvent event) async { try { + final session = ref.read(sessionProvider(orderId)); + if (session == null || session.sharedKey == null) { + _logger.e('Session or shared key is null when processing chat event'); + return; + } + + if (session.sharedKey?.public != event.recipient) { + return; + } + // Check if event is already processed to prevent duplicate notifications final eventStore = ref.read(eventStorageProvider); if (await eventStore.hasItem(event.id!)) { return; } // Store the complete event... await eventStore.putItem(/*...*/); - final session = ref.read(sessionProvider(orderId)); - // ... rest of session checks
🧹 Nitpick comments (1)
lib/features/chat/notifiers/chat_room_notifier.dart (1)
194-317: Comprehensive historical message loading with excellent error handling.The implementation handles multiple edge cases gracefully including migration from older storage formats and incomplete events. The detailed logging will be valuable for debugging.
Consider breaking this large method into smaller, focused functions for better maintainability:
Future<void> _loadHistoricalMessages() async { try { if (!_validateSession()) return; final chatEvents = await _findChatEvents(); if (chatEvents.isEmpty) return; final historicalMessages = await _processStoredEvents(chatEvents); await _updateStateWithHistoricalMessages(historicalMessages); } catch (e) { _logger.e('Error loading historical messages: $e'); } } + bool _validateSession() { /* session validation logic */ } + Future<List<dynamic>> _findChatEvents() { /* event finding logic */ } + Future<List<NostrEvent>> _processStoredEvents(List<dynamic> events) { /* processing logic */ } + Future<void> _updateStateWithHistoricalMessages(List<NostrEvent> messages) { /* state update logic */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/features/chat/notifiers/chat_room_notifier.dart(4 hunks)lib/features/chat/providers/chat_room_providers.dart(1 hunks)lib/features/chat/screens/chat_room_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/chat/screens/chat_room_screen.dart
🧰 Additional context used
📓 Path-based instructions (4)
lib/features/*/providers/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Providers are organized by feature in
features/{feature}/providers/
Files:
lib/features/chat/providers/chat_room_providers.dart
lib/features/*/{screens,providers,notifiers,widgets}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
🧠 Learnings (15)
📚 Learning: applies to lib/shared/providers/mostro_database_provider.dart : database initialization must occur i...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization must occur in `shared/providers/mostro_database_provider.dart`
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in abstractmostronotifier, the handleevent method is synchronous and returns void, not future....
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/**/*.dart : always check `mounted` before using context after async operations...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Always check `mounted` before using context after async operations
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: the `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible.
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to test/mocks.mocks.dart : do not add individual `// ignore: must_be_immutable` comments to ...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to test/mocks.mocks.dart : Do not add individual `// ignore: must_be_immutable` comments to classes in `test/mocks.mocks.dart`
Applied to files:
lib/features/chat/providers/chat_room_providers.dart
📚 Learning: applies to **/*.dart : use `const` constructors where possible...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
Applied to files:
lib/features/chat/providers/chat_room_providers.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in abstractmostronotifier, state updates occur for all messages regardless of timestamp to hydrate t...
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/services/nostr_service.dart : all nostr protocol interactions must go through `servic...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/services/nostr_service.dart : All Nostr protocol interactions must go through `services/nostr_service.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro mobile background service architecture, events aren't stored by the background process...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro mobile codebase, `eventstorageprovider` is exported from `package:mostro_mobile/shared...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/data/models.dart : models must be exported through `data/models.dart`...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/data/models.dart : Models must be exported through `data/models.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro mobile codebase, riverpod code generation is used with `@riverpod` annotations. provid...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to **/*.dart : remove unused imports and dependencies...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/core/mostro_fsm.dart : order state transitions must be managed by `core/mostro_fsm.da...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions must be managed by `core/mostro_fsm.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the nostr protocol, event.id will never be null in events returned by relay subscriptions, so nul...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
🔇 Additional comments (5)
lib/features/chat/providers/chat_room_providers.dart (3)
13-14: LGTM! Clean initialization tracking.The new provider properly tracks initialization status per chat room using a family provider pattern.
16-46: Excellent resolution of the async initialization race condition!This implementation properly addresses the previous issue by:
- Creating the notifier synchronously
- Handling async initialization in a separate function without blocking provider creation
- Tracking initialization status for consumers to react appropriately
- Including proper error handling in the fire-and-forget context
The solution follows best practices for async operations in Riverpod providers.
48-51: LGTM! Useful helper provider.The helper provider provides a clean API for UI components to check chat room readiness status.
lib/features/chat/notifiers/chat_room_notifier.dart (2)
148-192: Excellent optimistic UI update pattern.The immediate local state update followed by asynchronous network send provides the responsive user experience expected in modern chat applications. Good error handling with appropriate logging.
319-326: LGTM! Proper resource cleanup.The addition of session listener cleanup prevents memory leaks with the new reactive session handling pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/features/chat/widgets/chat_list_item.dart (2)
86-194: LGTM with Flutter version compatibility concern.The UI layout is well-structured with proper spacing, styling, and the FutureBuilder integration for unread status works correctly.
However, the
Color.withValues(alpha: ...)usage on lines 90, 126 requires Flutter 3.27.0+. This was flagged in previous reviews.
197-252: LGTM with same Flutter version concern.Excellent placeholder design that maintains layout consistency while providing clear loading state feedback.
The
withValues(alpha: 13)on line 203 has the same Flutter 3.27.0+ compatibility issue noted elsewhere.
🧹 Nitpick comments (1)
lib/services/chat_read_status_service.dart (1)
22-47: LGTM with a minor optimization suggestion.The unread message detection logic is sound and handles edge cases well, including null safety and excluding current user messages.
For better performance with large message lists, consider breaking early when the first unread message is found:
// Check if any peer messages are newer than the last read time for (final message in messages) { // Skip messages from the current user if (message.pubkey == currentUserPubkey) continue; // Check if message timestamp is newer than last read time if (message.createdAt != null) { final messageTime = message.createdAt!.millisecondsSinceEpoch; if (messageTime > lastReadTime) { return true; } } }The current implementation already returns early, so this is more of a readability note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/features/chat/widgets/chat_list_item.dart(1 hunks)lib/services/chat_read_status_service.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/services/chat_read_status_service.dartlib/features/chat/widgets/chat_list_item.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/services/chat_read_status_service.dartlib/features/chat/widgets/chat_list_item.dart
lib/features/*/{screens,providers,notifiers,widgets}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/chat/widgets/chat_list_item.dart
🧠 Learnings (7)
📚 Learning: applies to lib/services/nostr_service.dart : all nostr protocol interactions must go through `servic...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/services/nostr_service.dart : All Nostr protocol interactions must go through `services/nostr_service.dart`
Applied to files:
lib/services/chat_read_status_service.dart
📚 Learning: in the orderstate refactor, public keys should be accessed via `tradestate.peer?.publickey` from the...
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Applied to files:
lib/features/chat/widgets/chat_list_item.dart
📚 Learning: applies to test/mocks.mocks.dart : do not add individual `// ignore: must_be_immutable` comments to ...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to test/mocks.mocks.dart : Do not add individual `// ignore: must_be_immutable` comments to classes in `test/mocks.mocks.dart`
Applied to files:
lib/features/chat/widgets/chat_list_item.dart
📚 Learning: zero `flutter analyze` issues must be maintained...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Zero `flutter analyze` issues must be maintained
Applied to files:
lib/features/chat/widgets/chat_list_item.dart
📚 Learning: applies to lib/**/*.dart : always check `mounted` before using context after async operations...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Always check `mounted` before using context after async operations
Applied to files:
lib/features/chat/widgets/chat_list_item.dart
📚 Learning: always use latest apis (e.g., `withvalues()` instead of `withopacity()`)...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Always use latest APIs (e.g., `withValues()` instead of `withOpacity()`)
Applied to files:
lib/features/chat/widgets/chat_list_item.dart
📚 Learning: all changes should pass `flutter analyze` before commit...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: All changes should pass `flutter analyze` before commit
Applied to files:
lib/features/chat/widgets/chat_list_item.dart
🔇 Additional comments (9)
lib/services/chat_read_status_service.dart (4)
1-2: LGTM!The imports are appropriate for the service's functionality - SharedPreferences for local persistence and dart_nostr for the NostrEvent type.
4-5: LGTM!Good use of static methods for a utility service and appropriate key prefix to avoid SharedPreferences collisions.
7-13: LGTM!The method correctly stores the current timestamp for read status tracking with proper async handling and clear documentation.
15-20: LGTM!Clean implementation with appropriate nullable return type and proper async handling.
lib/features/chat/widgets/chat_list_item.dart (5)
1-11: LGTM!All imports are necessary and properly organized for the widget's functionality.
13-24: LGTM!Appropriate use of ConsumerStatefulWidget for Riverpod integration and local state management for immediate UI feedback.
27-36: LGTM!Good null safety implementation with proper guard clauses and placeholder handling. The null assertion on line 34 is safe because of the preceding null check.
39-75: LGTM!Comprehensive message processing with proper handling of message ordering, preview truncation, user prefixing, and timestamp conversion. The explanatory comments are helpful.
254-273: LGTM!Excellent date formatting logic with proper handling of different time ranges and good use of localization. The DateTime comparisons and DateFormat usage are correct.
…h better error handling
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/data/models/nostr_event.dart (1)
119-122: Verify NIP-17 message handling aligns with Mostro protocol.The code validates
kind == 14as NIP-17 direct messages, but based on previous feedback, Mostro chat DMs are a protocol variant using shared keys, not standard NIP-17 messages. Ensure this validation aligns with the actual Mostro protocol requirements.What is the Mostro protocol specification for chat messages and how do they differ from NIP-17?lib/features/chat/notifiers/chat_room_notifier.dart (1)
8-11: Address the circular import dependency.The import of
chat_room_providers.dartcreates a circular dependency as previously flagged. Consider extracting the provider declarations to break this cycle and improve maintainability.
🧹 Nitpick comments (1)
lib/data/models/nostr_event.dart (1)
155-158: Verify tag filtering logic preserves necessary tags.The code filters out existing "p" tags before adding the new one. Ensure this doesn't inadvertently remove other important "p" tags that should be preserved in the wrapped event.
Consider making the filtering more explicit:
tags: [ ["p", sharedKey.public], - ...(tags?.where((tag) => tag.isNotEmpty && tag[0] != 'p') ?? []), + // Add other non-p tags, or filter more specifically if needed + ...(tags?.where((tag) => tag.isNotEmpty && tag[0] != 'p') ?? []), ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/data/models/nostr_event.dart(1 hunks)lib/features/chat/notifiers/chat_room_notifier.dart(4 hunks)lib/shared/notifiers/session_notifier.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
lib/shared/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared utilities and widgets must be placed in
shared/
Files:
lib/shared/notifiers/session_notifier.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
lib/features/*/{screens,providers,notifiers,widgets}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/chat/notifiers/chat_room_notifier.dart
🧠 Learnings (21)
📚 Learning: in the orderstate refactor, public keys should be accessed via `tradestate.peer?.publickey` from the...
Learnt from: chebizarro
PR: MostroP2P/mobile#110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Applied to files:
lib/shared/notifiers/session_notifier.dart
📚 Learning: applies to lib/services/nostr_service.dart : all nostr protocol interactions must go through `servic...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/services/nostr_service.dart : All Nostr protocol interactions must go through `services/nostr_service.dart`
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
📚 Learning: in the mostro mobile codebase, `eventstorageprovider` is exported from `package:mostro_mobile/shared...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
📚 Learning: in abstractmostronotifier, state updates occur for all messages regardless of timestamp to hydrate t...
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
📚 Learning: applies to lib/shared/providers/mostro_database_provider.dart : database initialization must occur i...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization must occur in `shared/providers/mostro_database_provider.dart`
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in abstractmostronotifier, the handleevent method is synchronous and returns void, not future....
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
📚 Learning: in the mostro mobile codebase, riverpod code generation is used with `@riverpod` annotations. provid...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro protocol, recipient keys (trade keys) are used only once, ensuring no collisions when ...
Learnt from: chebizarro
PR: MostroP2P/mobile#151
File: lib/services/trade_history_restoration_service.dart:67-67
Timestamp: 2025-07-20T23:45:23.811Z
Learning: In the Mostro protocol, recipient keys (trade keys) are used only once, ensuring no collisions when merging session maps in trade history restoration.
Applied to files:
lib/shared/notifiers/session_notifier.dartlib/data/models/nostr_event.dart
📚 Learning: the `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the mostro mobile background service architecture, events aren't stored by the background process...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/**/*.dart : always check `mounted` before using context after async operations...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Always check `mounted` before using context after async operations
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to **/*.dart : use `const` constructors where possible...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to **/*.dart : Use `const` constructors where possible
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to **/*.dart : remove unused imports and dependencies...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to **/*.dart : Remove unused imports and dependencies
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to test/mocks.mocks.dart : do not add individual `// ignore: must_be_immutable` comments to ...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to test/mocks.mocks.dart : Do not add individual `// ignore: must_be_immutable` comments to classes in `test/mocks.mocks.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: zero `flutter analyze` issues must be maintained...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Zero `flutter analyze` issues must be maintained
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to test/mocks.mocks.dart : mock files must have file-level `// ignore_for_file: must_be_immu...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to test/mocks.mocks.dart : Mock files must have file-level `// ignore_for_file: must_be_immutable` and must not add more
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/data/models.dart : models must be exported through `data/models.dart`...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/data/models.dart : Models must be exported through `data/models.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: applies to lib/core/mostro_fsm.dart : order state transitions must be managed by `core/mostro_fsm.da...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/core/mostro_fsm.dart : Order state transitions must be managed by `core/mostro_fsm.dart`
Applied to files:
lib/features/chat/notifiers/chat_room_notifier.dart
📚 Learning: in the cantdo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'ca...
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/data/models/cant_do.dart:10-27
Timestamp: 2025-07-08T05:40:47.527Z
Learning: In the CantDo model parsing in lib/data/models/cant_do.dart, the inconsistent key naming between 'cant_do' (top-level) and 'cant-do' (nested) is required by the protocol specification and should not be changed as it would break message parsing compatibility.
Applied to files:
lib/data/models/nostr_event.dart
📚 Learning: in the nostr protocol, event.id will never be null in events returned by relay subscriptions, so nul...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Applied to files:
lib/data/models/nostr_event.dart
📚 Learning: in the mostro mobile codebase, mostromessage objects have an 'action' member that can be directly ac...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/shared/providers/app_init_provider.dart:78-95
Timestamp: 2025-05-06T16:08:19.352Z
Learning: In the Mostro Mobile codebase, MostroMessage objects have an 'action' member that can be directly accessed (e.g., mostroMessage.action).
Applied to files:
lib/data/models/nostr_event.dart
🔇 Additional comments (10)
lib/shared/notifiers/session_notifier.dart (4)
9-12: LGTM! Import additions are appropriate.All new imports are used in the added functionality - Logger for logging operations, nostr_utils for shared key computation, dart_nostr for key pair types, and Peer model for session updates.
22-22: LGTM! Logger field properly declared.The private final Logger field follows established patterns and is appropriately used for debugging and error logging in the new methods.
160-172: LGTM! Shared key calculation properly implemented.The method correctly delegates to
NostrUtils.computeSharedKey, includes appropriate error handling with logging, and maintains good separation of concerns by rethrowing exceptions for caller handling.
174-197: Verify shared key storage in session object.The method calculates a shared key but only assigns the peer to the session. If the session needs to store the shared key for later use (as suggested by the method name and chat functionality requiring
session.sharedKey), you may need to assign it.final peer = Peer(publicKey: counterpartyPublicKey); session.peer = peer; +session.sharedKey = sharedKey; // If Session has a sharedKey propertyPlease verify if the
Sessionmodel has asharedKeyproperty that should be updated, based on the chat functionality that checkssession.sharedKey.lib/data/models/nostr_event.dart (1)
187-191: LGTM! Timestamp randomization properly implemented.The randomized timestamp generation provides reasonable privacy protection by obscuring exact send times within a 48-hour window while maintaining chronological ordering.
lib/features/chat/notifiers/chat_room_notifier.dart (5)
39-44: LGTM! Initialization properly refactored.The separate
initialize()method addresses previous concerns about async operations in the constructor. The sequential loading of historical messages followed by subscription setup prevents race conditions and improves testability.
64-91: LGTM! Reactive session handling well implemented.The session listening logic properly waits for session availability with shared keys before subscribing to chat events. The automatic cleanup and detailed logging make this robust and debuggable.
106-120: LGTM! Event storage properly implemented.The complete event storage with metadata (including
order_idandtype: 'chat') ensures events are tracked to prevent duplicate notifications, addressing previous feedback about saving chat events to EventStore.
185-208: LGTM! Optimistic message sending with proper error handling.The optimistic local updates provide immediate UI feedback while the rollback mechanism on send failure ensures state consistency. The integration with chat rooms list refresh maintains overall app state coherence.
211-349: LGTM! Comprehensive historical message loading.The method handles multiple scenarios robustly - legacy data without
order_id, incomplete events from older app versions, and shared key filtering. While complex, the extensive logging and error handling make it maintainable and debuggable.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/chat/chat_room_provider.dart (1)
37-37: Consider adding error state tracking.The current boolean initialization provider only tracks success/failure but doesn't distinguish between "not initialized", "initializing", "initialized", and "error" states. This could be useful for UI state management.
Consider enhancing the provider to track more granular states:
-final chatRoomInitializedProvider = StateProvider.family<bool, String>((ref, chatId) => false); +enum ChatRoomInitializationState { + notInitialized, + initializing, + initialized, + error, +} + +final chatRoomInitializedProvider = StateProvider.family<ChatRoomInitializationState, String>( + (ref, chatId) => ChatRoomInitializationState.notInitialized, +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/features/chat/chat_room_provider.dart(1 hunks)lib/features/chat/providers/chat_room_providers.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/chat/providers/chat_room_providers.dart
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/chat/chat_room_provider.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/chat/chat_room_provider.dart
🧠 Learnings (4)
📚 Learning: in the mostro mobile codebase, riverpod code generation is used with `@riverpod` annotations. provid...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: applies to lib/shared/providers/mostro_database_provider.dart : database initialization must occur i...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization must occur in `shared/providers/mostro_database_provider.dart`
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: use riverpod for all state management...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Use Riverpod for all state management
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: in the mostro mobile codebase, `eventstorageprovider` is exported from `package:mostro_mobile/shared...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/features/chat/chat_room_provider.dart
🔇 Additional comments (1)
lib/features/chat/chat_room_provider.dart (1)
1-3: LGTM - Clean imports.The imports are well-organized and all appear to be used in the implementation.
… and safety checks
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/chat/chat_room_provider.dart (1)
26-26: Use const constructor for the default value function.-final chatRoomInitializedProvider = StateProvider.family<bool, String>((ref, chatId) => false); +final chatRoomInitializedProvider = StateProvider.family<bool, String>((ref, chatId) => const false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/chat/chat_room_provider.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/chat/chat_room_provider.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/chat/chat_room_provider.dart
🧠 Learnings (8)
📚 Learning: in the mostro mobile codebase, riverpod code generation is used with `@riverpod` annotations. provid...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `@Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: applies to lib/shared/providers/mostro_database_provider.dart : database initialization must occur i...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Database initialization must occur in `shared/providers/mostro_database_provider.dart`
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: use riverpod for all state management...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Use Riverpod for all state management
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: applies to lib/**/*.dart : always check `mounted` before using context after async operations...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Always check `mounted` before using context after async operations
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: in abstractmostronotifier, the handleevent method is synchronous and returns void, not future....
Learnt from: Catrya
PR: MostroP2P/mobile#215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: the `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible...
Learnt from: chebizarro
PR: MostroP2P/mobile#74
File: lib/features/chat/notifiers/chat_rooms_notifier.dart:0-0
Timestamp: 2025-05-06T22:23:06.684Z
Learning: The `reload()` method in the chat_room_notifier is synchronous, so await is not required or possible.
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: applies to test/mocks.mocks.dart : do not add individual `// ignore: must_be_immutable` comments to ...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to test/mocks.mocks.dart : Do not add individual `// ignore: must_be_immutable` comments to classes in `test/mocks.mocks.dart`
Applied to files:
lib/features/chat/chat_room_provider.dart
📚 Learning: encapsulate business logic in notifiers...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Encapsulate business logic in Notifiers
Applied to files:
lib/features/chat/chat_room_provider.dart
… shared key calculation
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
lib/features/chat/screens/chat_rooms_list.dart (4)
65-65: Fix localization string access patternUsing the force unwrap operator (!) with localization strings violates the coding guidelines and can cause runtime errors.
- S.of(context)?.chat ?? 'Chat', + S.of(context).chat,
91-91: Fix localization string access patternUsing null-aware operator with fallback violates the coding guidelines. Use the standard
S.of(context).yourKeypattern.- S.of(context)?.conversationsDescription ?? 'Your conversations with other users will appear here.', + S.of(context).conversationsDescription,
109-109: Fix localization string access patternUsing null-aware operator with fallback violates the coding guidelines.
- message: S.of(context)?.noDisputesAvailable ?? 'No disputes available', + message: S.of(context).noDisputesAvailable,
136-136: Fix localization string access patternUsing null-aware operator with fallback violates the coding guidelines.
- message: S.of(context)?.noMessagesAvailable ?? 'No messages available', + message: S.of(context).noMessagesAvailable,
🧹 Nitpick comments (1)
lib/features/chat/screens/chat_rooms_list.dart (1)
116-117: Hardcoded bottom padding may cause layout issuesThe hardcoded
80 + MediaQuery.of(context).viewPadding.bottompadding assumes a fixed height for the BottomNavBar, which could break if the component height changes.Consider using a more flexible approach:
- SizedBox( - height: 80 + MediaQuery.of(context).viewPadding.bottom), + // Consider using a more dynamic approach or defining a constant + SizedBox( + height: BottomNavBar.height + MediaQuery.of(context).viewPadding.bottom),Or use SafeArea instead of manual padding calculations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/features/chat/notifiers/chat_room_notifier.dart(4 hunks)lib/features/chat/screens/chat_room_screen.dart(1 hunks)lib/features/chat/screens/chat_rooms_list.dart(1 hunks)lib/features/chat/widgets/chat_list_item.dart(1 hunks)lib/features/chat/widgets/chat_messages_list.dart(1 hunks)lib/shared/notifiers/session_notifier.dart(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/features/chat/widgets/chat_messages_list.dart
- lib/features/chat/notifiers/chat_room_notifier.dart
- lib/shared/notifiers/session_notifier.dart
- lib/features/chat/widgets/chat_list_item.dart
- lib/features/chat/screens/chat_room_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
lib/features/*/{screens,providers,notifiers,widgets}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Feature-based organization:
features/{feature}/{screens|providers|notifiers|widgets}/
Files:
lib/features/chat/screens/chat_rooms_list.dart
lib/**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
lib/**/*.dart: UseS.of(context).yourKeyfor all user-facing strings
Always checkmountedbefore using context after async operations
Files:
lib/features/chat/screens/chat_rooms_list.dart
**/*.dart
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.dart: Remove unused imports and dependencies
Useconstconstructors where possible
Files:
lib/features/chat/screens/chat_rooms_list.dart
🧠 Learnings (5)
📚 Learning: applies to lib/**/*.dart : use `s.of(context).yourkey` for all user-facing strings...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/**/*.dart : Use `S.of(context).yourKey` for all user-facing strings
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
📚 Learning: zero `flutter analyze` issues must be maintained...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Zero `flutter analyze` issues must be maintained
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
📚 Learning: all changes should pass `flutter analyze` before commit...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: All changes should pass `flutter analyze` before commit
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
📚 Learning: applies to lib/main.dart : locales for the timeago package must be configured in `main.dart` with `t...
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-03T09:43:38.727Z
Learning: Applies to lib/main.dart : Locales for the timeago package must be configured in `main.dart` with `timeago.setLocaleMessages()`
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
📚 Learning: in abstractmostronotifier, state updates occur for all messages regardless of timestamp to hydrate t...
Learnt from: chebizarro
PR: MostroP2P/mobile#127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Applied to files:
lib/features/chat/screens/chat_rooms_list.dart
🔇 Additional comments (1)
lib/features/chat/screens/chat_rooms_list.dart (1)
19-40: Well-structured StatefulWidget implementationThe refactor to ConsumerStatefulWidget with proper TabController lifecycle management is well implemented. The initState and dispose methods correctly handle the TabController setup and cleanup.
4 participants
New chat screen according to issue #87, and issue #76 has been resolved.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores