feat: improve countdown timer with order_expires_at and dynamic scali… #354
Conversation
…ng in pending orders - Use exact timestamps from order_expires_at tag for precise calculations - Add dynamic day/hour scaling: 14d 20h 06m (>24h) or HH:MM:SS (≤24h) - Refactor: Create shared DynamicCountdownWidget to remove duplication - Apply consistent behavior across TakeOrderScreen and TradeDetailScreen
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Screen as TakeOrder/TradeDetailScreen
participant Dynamic as DynamicCountdownWidget
participant Circular as CircularCountdown
Note over Screen: Screen reads Order (order.expiresAt, order.createdAt)
Screen->>Dynamic: render(expiration, createdAt)
alt remaining > 24h (day-scale)
Dynamic->>Circular: render(totalDays, daysLeft)
Dynamic->>Dynamic: format "Xd Yh Zm"
else remaining ≤ 24h (hour-scale)
Dynamic->>Circular: render(totalHours, hoursLeft)
Dynamic->>Dynamic: format "HH:MM:SS"
end
Circular-->>Dynamic: progress updates
Dynamic-->>Screen: widget UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/data/models/nostr_event.dart(1 hunks)lib/data/models/order.dart(1 hunks)lib/features/order/screens/take_order_screen.dart(4 hunks)lib/features/trades/screens/trade_detail_screen.dart(2 hunks)lib/shared/widgets/dynamic_countdown_widget.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
lib/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared utilities in lib/shared/
Files:
lib/shared/widgets/dynamic_countdown_widget.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dartlib/data/models/nostr_event.dartlib/data/models/order.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dartlib/data/models/nostr_event.dartlib/data/models/order.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dart
lib/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)
Files:
lib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Store persistence and API code under lib/data/
Files:
lib/data/models/nostr_event.dartlib/data/models/order.dart
🧠 Learnings (15)
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/main.dart : Configure timeago locales and app initialization in main.dart
Applied to files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/**/*.dart : Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
Applied to files:
lib/shared/widgets/dynamic_countdown_widget.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/data/models.dart : Export models through lib/data/models.dart
Applied to files:
lib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-07T17:31:12.117Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T17:31:12.117Z
Learning: Applies to lib/**/*.dart : Name Riverpod providers as <Feature>Provider or <Feature>Notifier
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/*/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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<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.
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
lib/features/order/screens/take_order_screen.dartlib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/shared/providers/mostro_database_provider.dart : Initialize Sembast database in shared/providers/mostro_database_provider.dart
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/generated/l10n.dart : Generated localization entrypoint file is lib/generated/l10n.dart
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/notifications/notification_service.dart : Notification service configuration and custom icons should be defined in notification_service.dart
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-08-15T01:37:12.243Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.243Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to test/mocks.mocks.dart : Do not manually edit test/mocks.mocks.dart (Mockito-generated)
Applied to files:
lib/features/trades/screens/trade_detail_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 kind 10002 events, validate WS URLs, and handle timestamps/null-safety
Applied to files:
lib/data/models/order.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
🔇 Additional comments (1)
lib/features/trades/screens/trade_detail_screen.dart (1)
982-999: Clamp countdown inputs before building the widget.
expiresAtTimestampis already in milliseconds. IfcreatedAtis missing (defaults to 0)totalDurationinsideDynamicCountdownWidgetbecomes extremely large, but the bigger problem is that malformed timestamps (e.g., negative, zero, or in milliseconds instead of seconds) bubble straight into the widget where they can produce invalid totals. Please validate the parsed value (e.g., ensure it’s positive and reasonably nearexpiration) or skip rendering. This prevents the dynamic countdown from receiving junk data and avoids exploding progress denominators.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
lib/shared/widgets/dynamic_countdown_widget.dart (1)
31-34: Clarify the 1-hour grace period for expired orders.The code returns an empty widget only if expiration is more than 1 hour in the past. This allows expired orders (0-60 minutes past expiration) to still display a countdown, which might confuse users. Consider either:
- Showing a "Expired" state instead of hiding the widget for 0-60 minute window
- Hiding immediately when expiration passes (remove the 1-hour buffer)
- Documenting why the 1-hour grace period is intentional
If immediate hiding is preferred:
- // Handle edge case: expiration in the past - if (expiration.isBefore(now.subtract(const Duration(hours: 1)))) { - // If expiration is more than 1 hour in the past, likely invalid + // Hide widget if expiration has passed + if (expiration.isBefore(now)) { return const SizedBox.shrink(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CLAUDE.md(1 hunks)docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md(1 hunks)lib/data/models/order.dart(1 hunks)lib/features/order/screens/take_order_screen.dart(4 hunks)lib/shared/widgets/dynamic_countdown_widget.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/data/models/order.dart
🧰 Additional context used
📓 Path-based instructions (6)
lib/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared utilities in lib/shared/
Files:
lib/shared/widgets/dynamic_countdown_widget.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/features/order/screens/take_order_screen.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/features/order/screens/take_order_screen.dart
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/order/screens/take_order_screen.dart
lib/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)
Files:
lib/features/order/screens/take_order_screen.dart
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Store additional project guidance and documentation in docs/
Files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
🧠 Learnings (10)
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Document architectural changes in CLAUDE.md and related docs (NOSTR.md, README.md)
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
CLAUDE.mdlib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/main.dart : Configure timeago locales and app initialization in main.dart
Applied to files:
CLAUDE.mdlib/shared/widgets/dynamic_countdown_widget.dartlib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Implement relay operations: syncWithMostroInstance, removeRelayWithBlacklist, addRelayWithSmartValidation, _handleMostroRelayListUpdate
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 kind 10002 events, validate WS URLs, and handle timestamps/null-safety
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/data/models.dart : Export models through lib/data/models.dart
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-07T17:31:12.117Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T17:31:12.117Z
Learning: Applies to lib/**/*.dart : Name Riverpod providers as <Feature>Provider or <Feature>Notifier
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/*/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/
Applied to files:
lib/features/order/screens/take_order_screen.dart
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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<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.
Applied to files:
lib/features/order/screens/take_order_screen.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
🔇 Additional comments (10)
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (1)
343-351: Remove unusedcreatedAtparameter or document its purpose.The
DynamicCountdownWidgetsignature includescreatedAt(line 320), but it is never used in the build method. The countdown logic only usesexpirationto calculate remaining time. Either remove this parameter or add documentation explaining its intended use (e.g., for audit logging, total elapsed time tracking).CLAUDE.md (1)
248-259: LGTM! Clear documentation of the Dynamic Countdown Timer System.The documentation accurately describes the DynamicCountdownWidget implementation, including the dual display modes (day scale for >24h, hour scale for ≤24h), data source (order_expires_at), and integration points. This will help future developers understand the countdown system.
lib/features/order/screens/take_order_screen.dart (4)
21-21: LGTM! Clean import for the new shared countdown widget.The import follows the project's structure and enables the countdown refactor.
89-91: LGTM! Constructor properly refactored to accept the order object.The countdown widget now receives the complete NostrEvent order, allowing it to extract both expiration and creation timestamps. This is cleaner than passing a derived DateTime.
Also applies to: 444-449
480-484: LGTM! Clean delegation to the shared countdown widget.The DynamicCountdownWidget encapsulates all countdown logic, eliminating duplication. The parameters (expiration, createdAt) provide the widget with everything it needs.
465-477: I need to gather more information about the Mostro protocol's timestamp format expectations.<!-- [scratchpad_start]
[task verify_timestamp_format_2/2]
[observations]
- Found Mostro protocol documentation showing "expiration" tag format
- In mostro.network documentation (search result 3 & 10): both show expiration as
["expiration", "1719391096"]- Unix timestamp 1719391096 is June 24, 2024 when converted (seconds since epoch)
- This confirms seconds format, not milliseconds
- Code multiplies by 1000 to convert to DateTime.fromMillisecondsSinceEpoch, which expects milliseconds
- The assumption of seconds is CORRECT
[analysis]- Mostro protocol sends expiration timestamp in seconds (standard Unix timestamp)
- Multiplication by 1000 is CORRECT to convert seconds to milliseconds for DateTime
- No need for dual-format handling - protocol is consistent with seconds
[pending]- None - sufficient evidence found
[done]- Verification complete: timestamp format assumption is correct, code is safe
[scratchpad_start] -->Based on the Mostro protocol documentation, the expiration tag contains a Unix timestamp like "1719391096", confirming that
order_expires_atis sent in seconds. The codebase consistently treats this value as seconds and multiplies by 1000 to convert to milliseconds forDateTime.fromMillisecondsSinceEpoch(), which is correct.The timestamp format assumption is verified and the code change is safe. No dual-format detection logic is needed because the Mostro protocol guarantees seconds format.
lib/shared/widgets/dynamic_countdown_widget.dart (4)
1-24: LGTM! Well-structured countdown widget with clear documentation.The widget properly encapsulates the countdown logic with clean dependencies (circular_countdown, Riverpod, localization). The documentation clearly explains the dual-mode behavior and scope (pending orders only).
36-46: LGTM! Robust edge-case handling.The early return for invalid time ranges (expiration ≤ createdAt) prevents crashes. The remainingTime calculation correctly clamps to zero when expired.
52-70: Excellent fix! Day scale rendering now handles zero/negative totals safely.The ceiling division with clamping (line 54:
((totalDuration.inSeconds + 86399) ~/ 86400).clamp(1, double.infinity).toInt()) ensurestotalDaysis always ≥ 1, and line 55 clampsdaysLeftwithin valid range. This addresses the previous critical issue where zero totals caused NaN/Infinity errors in CircularCountdown.
71-92: Excellent fix! Hour scale rendering now handles zero/negative totals safely.The ceiling division with clamping (line 73:
((totalDuration.inSeconds + 3599) ~/ 3600).clamp(1, double.infinity).toInt()) ensurestotalHoursis always ≥ 1, and line 74 clampshoursLeftwithin valid range. This prevents the division-by-zero errors that plagued the previous implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CLAUDE.md(2 hunks)docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md(4 hunks)lib/features/order/screens/take_order_screen.dart(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/order/screens/take_order_screen.dart
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Store additional project guidance and documentation in docs/
Files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
🧠 Learnings (10)
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Document architectural changes in CLAUDE.md and related docs (NOSTR.md, README.md)
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
CLAUDE.mddocs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
CLAUDE.mddocs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Implement relay operations: syncWithMostroInstance, removeRelayWithBlacklist, addRelayWithSmartValidation, _handleMostroRelayListUpdate
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/main.dart : Configure timeago locales and app initialization in main.dart
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/**/*.dart : Target zero flutter analyze issues
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/mostro_fsm.dart : MostroFSM manages order state transitions (core/mostro_fsm.dart)
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-05-06T15:49:55.079Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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<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.
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
⏰ 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)
CLAUDE.md (1)
248-259: ✅ Well-documented Dynamic Countdown Timer System section.The new section clearly explains the DynamicCountdownWidget, its location, data source, dual display modes, automatic transition point, localization approach, scope, and integration points. The use of method names (rather than line numbers) and implementation file paths makes this documentation resilient to refactors.
Verify that the localization key
S.of(context)!.timeLeftLabel()is defined in all three ARB files (intl_en.arb,intl_es.arb,intl_it.arb) to ensure the documentation aligns with implementation.docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (2)
354-372: Reject this review comment—it contradicts itself and the code already follows the recommended practice.The current documentation (lines 354–372) already includes method names alongside the code snippets, exactly as recommended. The proposed diff removes these descriptive comments, which contradicts the stated guidance to "use method/class names alongside line numbers for resilience."
Additionally, the line references cited (477–480, 87–93, 862–1062) don't appear in the current documentation. The actual implementation confirms:
- TakeOrderScreen: DynamicCountdownWidget usage at lines 483–486
- TradeDetailScreen: _CountdownWidget usage at lines 87–90
No changes are needed.
Likely an incorrect or invalid review comment.
364-378: Clarify architectural relationship between _CountdownWidget and DynamicCountdownWidget.These are distinct but related components:
_CountdownWidget(line 87 in trade_detail_screen.dart): Private screen-specific orchestrator that determines which countdown logic to use based on order statusDynamicCountdownWidget(lib/shared/widgets/dynamic_countdown_widget.dart): Shared public countdown display component for pending ordersThe documentation shows code using
_CountdownWidgetbut then describesDynamicCountdownWidgetwithout clarifying that_CountdownWidgetorchestrates and internally callsDynamicCountdownWidget. Update the documentation to explain this relationship explicitly so developers understand the architectural separation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (2)
313-341: Fix incomplete pseudocode (previously flagged).The pseudocode still references undefined variables:
- Line 323:
nowis used but not defined- Line 331:
totalDaysis used but not computed- Line 337:
totalHoursis used but not computedThis was already flagged in a previous review. Please either:
- Add the missing variable definitions to make it runnable code
- Clearly mark this as "conceptual pseudocode" with a note that it's simplified
364-372: Clarify component naming confusion (previously flagged).Line 367 references
_CountdownWidgetin the TradeDetailScreen example, but this section is documentingDynamicCountdownWidget. This creates confusion:
- Are
_CountdownWidgetandDynamicCountdownWidgetthe same component with different names?- Are they distinct components? If so, why is
_CountdownWidgetshown in a section aboutDynamicCountdownWidget?Based on the scope section (lines 374-378) which states "DynamicCountdownWidget is specifically designed for orders in Status.pending," it appears there may be multiple countdown components. Please clarify the relationship and ensure examples use the correct component name.
🧹 Nitpick comments (1)
lib/shared/widgets/dynamic_countdown_widget.dart (1)
26-46: Clarify the rebuild/reactivity strategy for countdown updates.The widget captures
DateTime.now()once during build (line 28) but doesn't watchcountdownTimeProvideror any periodic time stream. This means the countdown won't actually update every second unless the parent widget triggers rebuilds.Based on the documentation (TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md lines 387-388), it appears the parent screens are responsible for watching
countdownTimeProviderand triggering rebuilds. Consider adding a doc comment clarifying this requirement:/// Note: This widget captures time at build and does not auto-rebuild. /// Parent widgets should watch countdownTimeProvider to trigger periodic rebuilds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md(4 hunks)lib/data/models/nostr_event.dart(1 hunks)lib/data/models/order.dart(1 hunks)lib/features/order/screens/take_order_screen.dart(4 hunks)lib/features/trades/screens/trade_detail_screen.dart(2 hunks)lib/shared/widgets/dynamic_countdown_widget.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/features/trades/screens/trade_detail_screen.dart
- lib/data/models/order.dart
- lib/features/order/screens/take_order_screen.dart
🧰 Additional context used
📓 Path-based instructions (5)
lib/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared utilities in lib/shared/
Files:
lib/shared/widgets/dynamic_countdown_widget.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/data/models/nostr_event.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/data/models/nostr_event.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Store persistence and API code under lib/data/
Files:
lib/data/models/nostr_event.dart
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Store additional project guidance and documentation in docs/
Files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
🧠 Learnings (6)
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/main.dart : Configure timeago locales and app initialization in main.dart
Applied to files:
lib/shared/widgets/dynamic_countdown_widget.dartlib/data/models/nostr_event.dart
📚 Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/mostro_fsm.dart : MostroFSM manages order state transitions (core/mostro_fsm.dart)
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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.
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-05-06T15:49:55.079Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile 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<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.
Applied to files:
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
⏰ 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 (5)
lib/data/models/nostr_event.dart (1)
42-42: LGTM! Clean implementation following established patterns.The
expiresAtgetter follows the same pattern as other tag accessors in the extension (likeorderId,currency, etc.) and correctly retrieves theexpires_attag value.lib/shared/widgets/dynamic_countdown_widget.dart (3)
6-24: Well-documented widget with clear scope and responsibilities.The documentation clearly specifies that this widget is for pending orders only and describes the dual display modes. The constructor properly requires both
expirationandcreatedAttimestamps for precise calculations.
52-70: Day scale logic correctly implements ceiling division and bounds checking.The implementation properly addresses the previous concern about zero/negative totals:
- Line 54: Ceiling division with
.clamp(1, double.infinity)ensurestotalDays >= 1- Line 55:
.clamp(0, totalDays)preventsdaysLeftfrom exceeding the totalThe format string "14d 20h 06m" matches the PR objectives, and the code correctly uses localized strings via
S.of(context)!.timeLeftLabel().
71-91: Hour scale logic is well-implemented with proper formatting.The hour scale implementation mirrors the day scale's robust approach:
- Line 73: Ceiling division ensures
totalHours >= 1- Line 74: Clamping prevents overflow
- Lines 78-79: Correctly formats as "HH:MM:SS" with zero-padding using
padLeft(2, '0')The consistent use of localized strings and the clean separation between day/hour modes makes this code maintainable.
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (1)
374-378: Clear scope definition for the countdown widget.The scope and limitations are well-articulated:
- Clearly states
DynamicCountdownWidgetis forStatus.pendingorders only- Explains that waiting states use separate countdown logic based on
expirationSeconds- Specifies the data source as the
expires_atNostr tagThis helps developers understand when to use this widget versus other countdown mechanisms.
#354) * feat: improve countdown timer with order_expires_at and dynamic scaling in pending orders - Use exact timestamps from order_expires_at tag for precise calculations - Add dynamic day/hour scaling: 14d 20h 06m (>24h) or HH:MM:SS (≤24h) - Refactor: Create shared DynamicCountdownWidget to remove duplication - Apply consistent behavior across TakeOrderScreen and TradeDetailScreen * fix: use int.tryParse for order_expires_at parsing and update documentation * fix: safe parsing for order countdown timestamps * fix: prevent countdown crashes with zero totals from short orders * update documentation * coderabbit suggestion * use expires_at
…ng in pending orders
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation