Skip to content

Conversation

@Catrya
Copy link
Member

@Catrya Catrya commented Jul 29, 2025

  • Add real-time timeout detection for orders in waiting-buyer-invoice and waiting-payment states
  • Implement differentiated handling: makers preserve sessions and revert to pending, takers clean up sessions completely
  • Add race condition protection with _isProcessingTimeout flag to prevent concurrent rocessing
  • Integrate synthetic message enrichment for complete order data persistence across app restarts
  • Add operation timeouts (8s detection, 5s storage) to prevent application hangs
  • Implement maker/taker role determination based on session role and order type matching
  • Add contextual timeout notifications in 3 languages with appropriate user messaging

System automatically monitors public events (38383) and compares timestamps with latest gift wrap messages to detect when orders timeout from waiting states, ensuring UI consistency with mostrod state without requiring app restarts.

Summary by CodeRabbit

  • New Features

    • Introduced robust order timeout detection and handling with real-time notifications when orders are republished due to inactivity.
    • Added new localized timeout notification messages in English, Spanish, and Italian.
    • Enhanced notification system to display custom, context-aware messages for timeout events.
    • Added a new order action for timeout reversal, enabling specialized handling of order state after timeouts.
  • Improvements

    • Updated user interface to clearly indicate when an order is republished after a timeout.
    • Improved clarity of logging and notification messages.
    • Refined order status display to handle timeout reversal states distinctly.
  • Bug Fixes

    • Ensured notifications for timeout events are properly localized and displayed without repetition.

  - Add real-time timeout detection for orders in waiting-buyer-invoice and waiting-payment states
  - Implement differentiated handling: makers preserve sessions and revert to pending, takers clean up sessions completely
  - Add race condition protection with _isProcessingTimeout flag to prevent concurrent rocessing
  - Integrate synthetic message enrichment for complete order data persistence across app restarts
  - Add operation timeouts (8s detection, 5s storage) to prevent application hangs
  - Implement maker/taker role determination based on session role and order type matching
  - Add contextual timeout notifications in 3 languages with appropriate user messaging

  System automatically monitors public events (38383) and compares timestamps with latest gift wrap messages to detect when orders timeout from waiting states, ensuring UI consistency with mostrod state without requiring app restarts.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 29, 2025

"""

Walkthrough

This change set introduces comprehensive timeout detection and handling for orders, including new timeout constants, a new enum action (timeoutReversal), a factory constructor for timeout reversal messages, and enhanced notifier and widget logic for real-time detection, state updates, and user notifications. Localization files are updated to support new timeout-related messages in English, Spanish, and Italian.

Changes

Cohort / File(s) Change Summary
Timeout Constants and Comments
lib/core/config.dart
Updated comments from Spanish to English; added two new timeout constants for detection and message storage.
Action Enum Extension
lib/data/models/enums/action.dart
Added new enum member timeoutReversal.
Timeout Reversal Message Factory
lib/data/models/mostro_message.dart
Added factory constructor MostroMessage.createTimeoutReversal for generating timeout reversal messages from public events.
Order State Enhancements
lib/features/order/models/order_state.dart
Refined logging; extended status mapping for timeoutReversal; updated actions for pending status.
Abstract Notifier Update
lib/features/order/notfiers/abstract_mostro_notifier.dart
Added handling for timeoutReversal action in event handler (no notification, handled elsewhere).
OrderNotifier Timeout Logic
lib/features/order/notfiers/order_notifier.dart
Introduced robust timeout detection, session cleanup, real-time public event subscription, and user notifications for timeouts.
Message Detail Widget
lib/features/trades/widgets/mostro_message_detail_widget.dart
Added UI handling for timeoutReversal action and status display.
English Localization
lib/l10n/intl_en.arb
Added localized timeout notification messages and related comment.
Spanish Localization
lib/l10n/intl_es.arb
Added localized timeout notification messages and related comment.
Italian Localization
lib/l10n/intl_it.arb
Added localized timeout notification messages and related comment.
Notification Notifier
lib/shared/notifiers/notification_notifier.dart
Added customMessage field to notification state and method for showing custom messages.
Notification Listener Widget
lib/shared/widgets/notification_listener_widget.dart
Enhanced notification logic for timeout actions and custom messages with localization support.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant OrderNotifier
    participant PublicEventStream
    participant NotificationNotifier
    participant UI

    loop Real-time
        PublicEventStream->>OrderNotifier: New public order event
        OrderNotifier->>OrderNotifier: _checkTimeoutAndCleanup()
        alt Timeout detected
            OrderNotifier->>NotificationNotifier: showCustomMessage(orderTimeoutTaker/Maker)
            NotificationNotifier->>UI: Display localized timeout notification
            OrderNotifier->>OrderNotifier: Update state (timeoutReversal)
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • chebizarro

Poem

🕰️
A timeout ticked, the orders paused—
The rabbit watched and swiftly gauged,
With messages new and notifiers tuned,
The system now is well immune.
In English, Spanish, and Italiano too,
Notifications hop right through!
🐇✨
"""

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69e76c7 and e4abffc.

📒 Files selected for processing (1)
  • lib/data/models/mostro_message.dart (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/data/models/mostro_message.dart
⏰ 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
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch timeout-detection

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
lib/features/order/notfiers/order_notifier.dart (2)

40-94: Consider adjusting log level for empty messages

The warning log for empty messages might be too verbose for new orders where no messages exist yet. Otherwise, the implementation with timeout detection and race condition prevention is well done.

       if (messages.isEmpty) {
-        logger.w('No messages found for order $orderId');
+        logger.d('No messages found for order $orderId');
         return;
       }

157-269: Consider refactoring this method for better maintainability

While the implementation is correct, this method is quite long (110+ lines) and handles multiple responsibilities. Consider extracting the maker and taker handling logic into separate methods.

Example refactoring approach:

Future<bool> _checkTimeoutAndCleanup(OrderState currentState, MostroMessage? latestGiftWrap) async {
  // Early returns and validation...
  
  if (publicTimestamp != null && publicTimestamp.isAfter(giftWrapTimestamp)) {
    final currentSession = ref.read(sessionProvider(orderId));
    if (currentSession == null) return false;
    
    final isCreatedByUser = _isCreatedByUser(currentSession, publicEvent);
    
    return isCreatedByUser 
      ? await _handleMakerTimeout(currentState, publicEvent, publicTimestamp)
      : await _handleTakerTimeout();
  }
  
  return false;
}

Future<bool> _handleMakerTimeout(OrderState currentState, NostrEvent publicEvent, DateTime publicTimestamp) async {
  // Maker-specific logic...
  return false; // Session preserved
}

Future<bool> _handleTakerTimeout() async {
  // Taker-specific logic...
  return true; // Session cleaned up
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fd78ee and f2b29bf.

📒 Files selected for processing (12)
  • lib/core/config.dart (2 hunks)
  • lib/data/models/enums/action.dart (1 hunks)
  • lib/data/models/mostro_message.dart (2 hunks)
  • lib/features/order/models/order_state.dart (7 hunks)
  • lib/features/order/notfiers/abstract_mostro_notifier.dart (1 hunks)
  • lib/features/order/notfiers/order_notifier.dart (2 hunks)
  • lib/features/trades/widgets/mostro_message_detail_widget.dart (2 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/shared/notifiers/notification_notifier.dart (2 hunks)
  • lib/shared/widgets/notification_listener_widget.dart (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/**/*.dart: Use S.of(context).yourKey for all user-facing strings
Always check mounted before using context after async operations

Files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/trades/widgets/mostro_message_detail_widget.dart
  • lib/data/models/enums/action.dart
  • lib/shared/notifiers/notification_notifier.dart
  • lib/features/order/models/order_state.dart
  • lib/shared/widgets/notification_listener_widget.dart
  • lib/core/config.dart
  • lib/data/models/mostro_message.dart
  • lib/features/order/notfiers/order_notifier.dart
**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.dart: Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/features/order/notfiers/abstract_mostro_notifier.dart
  • lib/features/trades/widgets/mostro_message_detail_widget.dart
  • lib/data/models/enums/action.dart
  • lib/shared/notifiers/notification_notifier.dart
  • lib/features/order/models/order_state.dart
  • lib/shared/widgets/notification_listener_widget.dart
  • lib/core/config.dart
  • lib/data/models/mostro_message.dart
  • lib/features/order/notfiers/order_notifier.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.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
lib/l10n/*.arb

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use proper ARB metadata for parameterized strings

Files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
lib/l10n/**

📄 CodeRabbit Inference Engine (CLAUDE.md)

Internationalization files are located in lib/l10n/

Files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
🧠 Learnings (13)
📓 Common learnings
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.
lib/features/order/notfiers/abstract_mostro_notifier.dart (8)

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: 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 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/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart

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: 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/trades/widgets/mostro_message_detail_widget.dart (7)

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: #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: #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: #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).

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.

lib/data/models/enums/action.dart (1)

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.

lib/shared/notifiers/notification_notifier.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: Encapsulate business logic in Notifiers

lib/features/order/models/order_state.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/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart

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: 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/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).

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/shared/widgets/notification_listener_widget.dart (17)

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/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: 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: 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 lib/core/app_routes.dart : GoRouter navigation is configured in core/app_routes.dart

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: 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/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart

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: #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 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/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: Pass BuildContext to methods that need localization

lib/core/config.dart (5)

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: 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: 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: #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/l10n/intl_it.arb (1)

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)

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/data/models/mostro_message.dart (13)

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: 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: 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: 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, 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.dart : Models should be exported through data/models.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 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/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/shared/providers/mostro_database_provider.dart : Database initialization is in shared/providers/mostro_database_provider.dart

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/

Learnt from: chebizarro
PR: #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/features/order/notfiers/order_notifier.dart (12)

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: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/

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 lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Encapsulate business logic in Notifiers

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: 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.

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: 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.

⏰ 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 (26)
lib/core/config.dart (1)

41-43: LGTM! Well-defined timeout constants for critical operations.

The new timeout constants are appropriately configured for the timeout detection system:

  • timeoutDetectionTimeout (8s) provides adequate detection window
  • messageStorageTimeout (5s) prevents hanging during storage operations

The comments clearly indicate these are for critical operations, which aligns with the timeout detection feature being implemented.

lib/features/order/notfiers/abstract_mostro_notifier.dart (1)

221-223: LGTM! Correct handling for timeoutReversal action.

The explicit no-op case with a clear comment explaining that handling is done manually in OrderNotifier is the right approach. This follows the established pattern in the codebase where some actions require specialized handling rather than the standard notification flow.

lib/data/models/enums/action.dart (1)

42-42: LGTM! Proper enum extension following established conventions.

The new timeoutReversal enum member correctly follows the kebab-case naming convention used throughout the enum and integrates seamlessly with the existing fromString method via the _valueMap.

lib/features/trades/widgets/mostro_message_detail_widget.dart (2)

43-45: LGTM! Appropriate conditional display logic for timeout reversal.

The conditional logic correctly displays only the order status for timeoutReversal actions, avoiding redundant text display. This makes sense since the action text already provides the necessary context about the timeout.


196-197: LGTM! Proper localization usage for timeout reversal message.

The case correctly returns the localized string orderTimeoutMaker explaining that the counterpart didn't respond in time and the order will be republished. The implementation follows the established pattern and uses S.of(context)! as required by the coding guidelines.

lib/l10n/intl_en.arb (1)

659-661: Localization keys present in all ARB files

Verified that both orderTimeoutTaker and orderTimeoutMaker appear in:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_es.arb
  • lib/l10n/intl_it.arb

No further changes needed.

lib/l10n/intl_es.arb (1)

689-692: LGTM! Timeout localization keys properly added.

The Spanish translations for the timeout notification messages are clear and contextually appropriate. The messages correctly inform users when either they or their counterpart failed to respond within the timeout window, and that the order will be republished.

lib/l10n/intl_it.arb (1)

697-700: LGTM! Italian timeout localization keys properly added.

The Italian translations for the timeout notification messages are accurate and contextually appropriate. The messages clearly communicate that either the user or their counterpart didn't respond in time and that the order will be republished.

lib/features/order/models/order_state.dart (7)

85-85: LGTM! Simplified logging output.

Removing emojis from logging makes the debug output cleaner and more professional.


96-98: LGTM! Clear status mapping debug information.

The simplified debug logging provides clear information about action to status mapping without unnecessary visual clutter.


103-103: LGTM! Consistent logging style.

The PaymentRequest logging is now consistent with the simplified logging approach.


140-143: LGTM! Enhanced state logging.

The improved logging provides better visibility into the new state creation and PaymentRequest preservation, which is valuable for debugging timeout scenarios.


229-232: LGTM! Proper timeout reversal handling.

The timeoutReversal action correctly returns the payload status (which should be pending for timeout reversals) or defaults to Status.pending. This aligns with the expected behavior of reverting orders back to pending state after a timeout.


252-255: LGTM! Timeout reversal action added to seller pending state.

The timeoutReversal action is properly added to the seller's pending state with cancel as the available action, which is appropriate since users should be able to cancel after a timeout reversal.


373-376: LGTM! Timeout reversal action added to buyer pending state.

The timeoutReversal action is consistently added to the buyer's pending state with cancel as the available action, maintaining symmetry with the seller state.

lib/shared/notifiers/notification_notifier.dart (3)

11-11: LGTM! Appropriate nullable field addition.

The customMessage field is correctly nullable, allowing for both custom and standard notification messages.


18-20: LGTM! Constructor properly updated.

The constructor correctly includes the new customMessage parameter while maintaining backward compatibility.


31-35: LGTM! Well-designed custom message method.

The showCustomMessage method follows the same pattern as existing methods, correctly setting the state with an informational notification containing the custom message. This enables timeout notifications to display localized custom messages.

lib/data/models/mostro_message.dart (2)

8-11: LGTM! Necessary imports for timeout reversal functionality.

The new imports are required for the timeout reversal factory constructor implementation and are properly organized.


32-81: Well-implemented timeout reversal factory constructor.

The createTimeoutReversal factory constructor is comprehensively designed to create synthetic timeout reversal messages with complete order context. Key strengths:

  • Properly extracts order information from public events
  • Correctly sets status to pending for timeout reversals
  • Handles both buy and sell order types appropriately
  • Reconstructs all necessary order fields for proper UI display
  • Properly handles nullable fields and provides sensible defaults

The implementation ensures that timeout reversal messages contain all the order information needed for proper state management and UI display.

lib/shared/widgets/notification_listener_widget.dart (2)

4-4: LGTM!

Good use of import alias to avoid naming conflicts.


43-45: LGTM!

Fixed 2-second duration is appropriate for notification messages.

lib/features/order/notfiers/order_notifier.dart (4)

2-15: LGTM!

Good approach using a flag to prevent concurrent timeout processing and proper subscription management.


271-286: LGTM!

Clear logic for determining maker vs taker role, consistent with the codebase patterns.


288-344: LGTM!

Well-implemented real-time timeout detection with proper state validation and timeout handling.


346-370: LGTM!

Proper notification handling and cleanup implementation.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
lib/features/order/notfiers/order_notifier.dart (1)

23-41: Proper synchronous implementation with async scheduling

The handleEvent method correctly maintains synchronous behavior while scheduling async timeout checks using Future.microtask. This addresses the previous review comment about keeping the method synchronous.

🧹 Nitpick comments (1)
lib/features/order/notfiers/order_notifier.dart (1)

51-54: Consider reducing log noise for empty messages

The warning log for no messages might be too verbose for normal operation, especially during initial app startup.

-      if (messages.isEmpty) {
-        logger.w('No messages found for order $orderId');
-        return;
-      }
+      if (messages.isEmpty) {
+        logger.d('No messages found for order $orderId');
+        return;
+      }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2b29bf and 8473c02.

📒 Files selected for processing (1)
  • lib/features/order/notfiers/order_notifier.dart (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
lib/**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

lib/**/*.dart: Use S.of(context).yourKey for all user-facing strings
Always check mounted before using context after async operations

Files:

  • lib/features/order/notfiers/order_notifier.dart
**/*.dart

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.dart: Remove unused imports and dependencies
Use const constructors where possible

Files:

  • lib/features/order/notfiers/order_notifier.dart
🧠 Learnings (2)
📓 Common learnings
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.
lib/features/order/notfiers/order_notifier.dart (15)

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: Applies to lib/features//notifiers/**/.dart : Notifiers for complex state logic are placed in features/{feature}/notifiers/

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 lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Encapsulate business logic in Notifiers

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: 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: 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.

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: 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.

⏰ 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 (15)
lib/features/order/notfiers/order_notifier.dart (15)

2-7: Imports look good with proper organization

The new imports are correctly added for the timeout detection functionality, including dart_nostr for public events, config for timeout constants, and models for data structures.


14-15: Instance variables properly declared for timeout handling

The subscription variable and processing flag are well-defined to prevent race conditions and manage resource cleanup.


20-21: Constructor enhancement is appropriate

Adding the public events subscription initialization in the constructor ensures real-time timeout detection starts immediately.


44-47: Good guard against concurrent timeout processing

The early return when _isProcessingTimeout is true prevents race conditions during sync operations.


56-70: Message sorting and state building logic is sound

The timestamp-based sorting and state building approach correctly processes messages in chronological order while tracking the latest gift wrap message.


72-82: Proper session cleanup integration in sync method

The timeout cleanup check is well-integrated into the sync flow with appropriate session validation and provider invalidation.


94-96: Proper cleanup in finally block

The finally block ensures the processing flag is reset even if exceptions occur, preventing permanent blocking of timeout processing.


162-172: Robust input validation for timeout checking

The method properly validates the order state and gift wrap message before proceeding with timeout detection logic.


174-187: Proper public event validation

The code correctly retrieves and validates the public event, ensuring it exists and has the expected pending status before proceeding.


189-195: Sound timestamp comparison logic

The timestamp comparison between public event and gift wrap message is implemented correctly to detect timeout conditions.


205-243: Comprehensive maker scenario handling

The maker scenario properly preserves the session while updating state and persisting timeout reversal messages. The error handling around message persistence is robust.


245-257: Proper taker scenario cleanup

The taker scenario correctly removes the session and returns appropriate cleanup indication.


275-289: Correct user role determination logic

The _isCreatedByUser method properly determines maker vs taker based on the relationship between session role and order type.


292-347: Well-implemented real-time subscription handling

The public events subscription includes proper concurrency control, timeout handling, and state verification at multiple points to ensure reliable timeout detection.


369-373: Proper resource cleanup in dispose

The dispose method correctly closes the subscription and calls super.dispose() to ensure proper cleanup.

Catrya added 2 commits July 29, 2025 12:54
  - Document real-time timeout detection for waiting-buyer-invoice and waiting-payment states
  - Explain maker vs taker handling scenarios and session management
  - Include detection mechanism, persistence strategy, and key implementation details
  - Clarify system monitors 38383 events vs gift wrap timestamps for mostrod consistency
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
CLAUDE.md (3)

54-59: Document the hard-coded timeout constants

The PR description mentions an 8 s detection window and 5 s storage window, but those values are missing here.
Readers will look for them in this section—please add the actual constants and reference their location in lib/core/config.dart (or wherever they live) so future changes remain discoverable.


57-59: Clarify the “38383 public events” data source

“Monitors: 38383 public events” is terse and may confuse newcomers:

  • Specify that 38383 is the Mostro relay public channel ID.
  • Optionally link to / describe the provider or service that exposes these events (e.g. orderEventsProvider in features/order/providers/order_events_provider.dart).

This small clarification prevents onboarding friction.


65-66: Add a file reference for the synthetic message factory

MostroMessage.createTimeoutReversal() is called out, but the file path isn’t.
Appending “(see lib/data/models/mostro_message.dart)" after the method name will help future maintainers locate the implementation quickly.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 789a97d and 69e76c7.

📒 Files selected for processing (1)
  • CLAUDE.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
CLAUDE.md

📄 CodeRabbit Inference Engine (CLAUDE.md)

Update CLAUDE.md when making architectural changes

Files:

  • CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
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.
CLAUDE.md (5)

Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T01:48:43.335Z
Learning: Applies to CLAUDE.md : Update CLAUDE.md when making architectural changes

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: #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: Applies to lib/core/mostro_fsm.dart : Order state transitions are managed by core/mostro_fsm.dart

Learnt from: chebizarro
PR: #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.

⏰ 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants