Feat: Logger service with basic integration #403
Conversation
WalkthroughThis PR adds a centralized in-app logger with isolate-to-main shipping, sanitization and in-memory buffering, Riverpod providers for real-time logs and filtering, updates the logs UI to use the providers, replaces local logger usages with the central logger, and updates docs/version and a config flag. Changes
Sequence DiagramsequenceDiagram
actor Isolate as Background Isolate
participant ILO as IsolateLogOutput
participant SP as SendPort
participant LS as LoggerService
participant MLO as MemoryLogOutput
participant LN as LogsNotifier
participant UI as Logs Screen
Isolate->>ILO: emit log entry
ILO->>ILO: sanitize and package payload
ILO->>SP: send log payload
SP->>LS: receive isolate log
LS->>LS: addLogFromIsolate() -> create LogEntry
LS->>MLO: addEntry(LogEntry)
MLO->>MLO: maintain buffer & notify listeners
MLO->>LN: ChangeNotifier triggers listener
LN->>LN: _onLogsChanged() updates state
UI->>LN: subscribe via logsProvider / filteredLogsProvider
LN->>UI: provide updated log list to render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
BraCR10
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lib/features/settings/settings_notifier.dart:
- Around line 153-156: The updateLoggingEnabled method updates state and
MemoryLogOutput.isLoggingEnabled but doesn't persist the change; modify
updateLoggingEnabled to call await _saveToPrefs() after setting state (and
MemoryLogOutput.isLoggingEnabled) so the logging preference is saved across
restarts—ensure you keep the method async and await _saveToPrefs() to persist
the change.
🧹 Nitpick comments (5)
lib/features/logs/screens/logs_screen.dart (1)
64-72: Refactor: Redundant MemoryLogOutput updates.Lines 65 and 70 directly set
MemoryLogOutput.isLoggingEnabled, then lines 66 and 71 callupdateLoggingEnabled()which also setsMemoryLogOutput.isLoggingEnabled(insettings_notifier.dartline 155). This creates duplicate updates and couples the UI to internal implementation details of the logger service.Once
SettingsNotifier.updateLoggingEnabled()properly persists the setting (see my comment onsettings_notifier.dart), these helpers can be simplified to just callupdateLoggingEnabled().♻️ Proposed simplification (after settings_notifier.dart fix)
-void _enableLogging() { - MemoryLogOutput.isLoggingEnabled = true; +Future<void> _enableLogging() async { + await ref.read(settingsProvider.notifier).updateLoggingEnabled(true); - ref.read(settingsProvider.notifier).updateLoggingEnabled(true); } -void _disableLogging() { - MemoryLogOutput.isLoggingEnabled = false; +Future<void> _disableLogging() async { + await ref.read(settingsProvider.notifier).updateLoggingEnabled(false); - ref.read(settingsProvider.notifier).updateLoggingEnabled(false); }lib/features/logs/logs_provider.dart (1)
70-78:copyWithcannot explicitly setlevelFiltertonull.Using
??means passinglevelFilter: nullexplicitly will preserve the old value instead of clearing it. If the UI needs to reset the filter to "all levels" by passingnull, this won't work as expected.♻️ Suggested fix using sentinel pattern
LogsFilter copyWith({ - String? levelFilter, + Object? levelFilter = _sentinel, String? searchQuery, }) { return LogsFilter( - levelFilter: levelFilter ?? this.levelFilter, + levelFilter: levelFilter == _sentinel + ? this.levelFilter + : levelFilter as String?, searchQuery: searchQuery ?? this.searchQuery, ); } + +const _sentinel = Object();lib/services/logger_service.dart (3)
33-50: Sanitization is solid but consider addingnpubpattern.The
nsecpattern is covered for private keys, butnpub(public keys) may also be considered sensitive in some contexts. Additionally, the patterns cover common JSON formats forprivateKeyandmnemonic.Consider whether public keys should also be redacted:
.replaceAll(RegExp(r'npub[0-9a-z]+'), '[PUBLIC_KEY]')
155-164: Buffer maintenance logic may not trim correctly when slightly over limit.When
_buffer.length > Config.logMaxEntriesbut_buffer.length < Config.logBatchDeleteSize, the calculation_buffer.length - Config.logMaxEntriesonly removes the excess, not a batch. This is actually correct behavior, but the conditional structure is confusing. IflogMaxEntries=1000andlogBatchDeleteSize=100, adding the 1001st entry removes only 1 entry instead of 100.The logic appears intentional (trim to max when batch delete would over-trim), but consider simplifying:
♻️ Clearer implementation
void _maintainBufferSize() { if (_buffer.length > Config.logMaxEntries) { - final deleteCount = _buffer.length < Config.logBatchDeleteSize - ? _buffer.length - Config.logMaxEntries - : Config.logBatchDeleteSize; - if (deleteCount > 0) { - _buffer.removeRange(0, deleteCount); - } + final excess = _buffer.length - Config.logMaxEntries; + final deleteCount = excess.clamp(1, Config.logBatchDeleteSize); + _buffer.removeRange(0, deleteCount); } }
283-311: InstantiatingSimplePrinteron every output call is inefficient.Each call to
output()creates a newSimplePrinterinstance just to callextractFromStackTrace. Consider making it a field.♻️ Reuse SimplePrinter instance
class IsolateLogOutput extends LogOutput { final SendPort? sendPort; + final SimplePrinter _printer = SimplePrinter(); IsolateLogOutput(this.sendPort); @override void output(OutputEvent event) { for (final line in event.lines) { // ignore: avoid_print print(line); } if (sendPort != null) { - final printer = SimplePrinter(); - final serviceAndLine = printer.extractFromStackTrace(event.origin.stackTrace); + final serviceAndLine = _printer.extractFromStackTrace(event.origin.stackTrace); final rawMessage = event.origin.message.toString(); final sanitizedMessage = cleanMessage(rawMessage); // ... rest unchanged } } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/LOGGING_IMPLEMENTATION.mdlib/core/config.dartlib/features/logs/logs_provider.dartlib/features/logs/screens/logs_screen.dartlib/features/relays/relays_notifier.dartlib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/services/logger_service.dart
💤 Files with no reviewable changes (1)
- lib/core/config.dart
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/services/logger_service.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartlib/features/logs/logs_provider.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/services/logger_service.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartlib/features/logs/logs_provider.dart
lib/features/subscriptions/subscription_manager.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use SubscriptionManager with
fireImmediately: falseduring SessionNotifier initialization to prevent premature execution
Files:
lib/features/subscriptions/subscription_manager.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/logger_service.dart
lib/features/relays/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/relays/**/*.dart: Implement relay URL normalization by removing trailing slashes in all blacklist operations to ensure consistent matching
Use dual storage strategy: store Mostro/default relays insettings.relaysand user relays insettings.userRelayswith full JSON metadata viatoJson()/fromJson()
Implement two-tier relay validation: primary Nostr protocol test (REQ/EVENT/EOSE) with WebSocket fallback for connectivity testing
Track relay sources (user, mostro, default) using RelaySource enum for appropriate handling and storage strategy
Files:
lib/features/relays/relays_notifier.dart
lib/features/relays/relays_notifier.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use
removeRelayWithBlacklist()for Mostro/default relays (adds to blacklist) andremoveRelay()for permanent user relay deletion
Files:
lib/features/relays/relays_notifier.dart
lib/features/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/screens/**/*.dart: Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
UseS.of(context)!.yourKeyfor all user-facing strings instead of hardcoded text
Files:
lib/features/logs/screens/logs_screen.dart
🧠 Learnings (25)
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/features/settings/settings_notifier.dartlib/services/logger_service.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartdocs/LOGGING_IMPLEMENTATION.mdlib/features/logs/logs_provider.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/relays/relays_notifier.dartdocs/LOGGING_IMPLEMENTATION.mdlib/features/logs/logs_provider.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/settings/settings_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartdocs/LOGGING_IMPLEMENTATION.mdlib/features/logs/logs_provider.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Use dual storage strategy: store Mostro/default relays in `settings.relays` and user relays in `settings.userRelays` with full JSON metadata via `toJson()`/`fromJson()`
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartlib/features/logs/logs_provider.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/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartdocs/LOGGING_IMPLEMENTATION.mdlib/features/logs/logs_provider.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/settings/settings.dart : Use null-aware operators (`??`) in Settings `copyWith()` method to preserve existing values for selectedLanguage and defaultLightningAddress when not explicitly overridden
Applied to files:
lib/features/settings/settings_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dartdocs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/**/*.dart : Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dartdocs/LOGGING_IMPLEMENTATION.md
📚 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/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dartlib/features/logs/screens/logs_screen.dartlib/features/logs/logs_provider.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Track relay sources (user, mostro, default) using RelaySource enum for appropriate handling and storage strategy
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 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:
lib/features/subscriptions/subscription_manager.dartdocs/LOGGING_IMPLEMENTATION.mdlib/features/logs/logs_provider.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Start 10-second cleanup timer automatically when taking orders via `startSessionTimeoutCleanup()` to prevent orphan sessions
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Use `removeRelayWithBlacklist()` for Mostro/default relays (adds to blacklist) and `removeRelay()` for permanent user relay deletion
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-08-19T17:54:15.016Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/relays_notifier.dart:41-49
Timestamp: 2025-08-19T17:54:15.016Z
Learning: In the Mostro mobile app relay synchronization system, the hardcoded 'wss://relay.mostro.network' relay in RelaysNotifier._loadRelays() is intentional for bootstrapping. Config.nostrRelays only contains this single active relay anyway (other entries are commented-out dev relays), so hardcoding is functionally equivalent and more explicit about the bootstrap requirement for fetching kind 10002 relay list events.
Applied to files:
lib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Implement relay URL normalization by removing trailing slashes in all blacklist operations to ensure consistent matching
Applied to files:
lib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Implement two-tier relay validation: primary Nostr protocol test (REQ/EVENT/EOSE) with WebSocket fallback for connectivity testing
Applied to files:
lib/features/relays/relays_notifier.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile 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<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Use `startSessionTimeoutCleanupForRequestId()` for order creation timeout protection and cancel timer automatically when any Mostro response received
Applied to files:
lib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/orders/notifiers/add_order_notifier.dart : Start session timeout cleanup in `submitOrder()` method to prevent orphan sessions when Mostro doesn't respond within 10 seconds
Applied to files:
lib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 (kind 10002) events with robust handling of different timestamp formats and null-safe parsing for malformed events
Applied to files:
lib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Name Riverpod providers using the `<Feature>Provider` or `<Feature>Notifier` convention
Applied to files:
lib/features/logs/logs_provider.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 (15)
lib/features/subscriptions/subscription_manager.dart (1)
5-5: LGTM: Centralized logger migration.The migration from a local
_loggerinstance to the centralizedloggerservice is clean and consistent with the PR's logging architecture.docs/LOGGING_IMPLEMENTATION.md (1)
18-29: LGTM: Documentation accurately reflects Phase 2 completion.The phase status updates correctly reflect the implemented logging service, UI integration, and initial file migrations (RelaysNotifier, SubscriptionManager). The Phase 3 plan is clear and specific.
lib/features/relays/relays_notifier.dart (1)
9-9: LGTM: Centralized logger migration with proper resource cleanup.The migration to the centralized
loggerservice is consistent throughout the file. The new_retryTimeris properly cancelled indispose()(line 860) and defensively cancelled before re-creation (line 481), preventing resource leaks.lib/features/logs/screens/logs_screen.dart (1)
431-441: LGTM: Clean log rendering with reverse chronological order.The logs list correctly reverses the order (line 437:
logs[logs.length - 1 - index]) to display newest logs first, which is the expected UX for log viewers.lib/features/logs/logs_provider.dart (3)
1-7: LGTM - Provider setup follows conventions.Provider naming (
logsProvider) and use ofStateNotifierProvideralign with the project's Riverpod conventions. Based on learnings, the<Feature>Providernaming is correctly applied.
9-28: LGTM - Proper lifecycle management.The
LogsNotifiercorrectly subscribes toMemoryLogOutputchanges in the constructor and removes the listener indispose(). The initial sync via_onLogsChanged()in the constructor ensures state is populated immediately.
30-49: LGTM - Clean filtering implementation.The
filteredLogsProvidercorrectly derives filtered state fromlogsProviderusingref.watch. The level and search filtering logic is straightforward and case-insensitive search covers both message and service fields.lib/services/logger_service.dart (8)
11-29: LGTM - Idempotent initialization with error handling.The early return guards against duplicate initialization. Error handling in the listener prevents log processing failures from crashing the app, though using
52-81: LGTM - Defensive parsing with sensible defaults.The isolate log ingestion handles missing/malformed timestamp gracefully by falling back to
DateTime.now(). Message sanitization is applied before storage.
94-114: LGTM - Clean data class.
LogEntryis immutable and theformat()method provides a consistent string representation. The timestamp substring extraction is safe for ISO 8601 formatted strings.
261-281: Cached logger won't reflect runtime config changes.The logger is cached on first access. If
Config.isDebugor logging level needs to change at runtime, the cached logger will use stale settings. This is likely acceptable for debug/release mode distinction but worth noting.
169-180: LGTM - Clean multi-output routing.
_MultiOutputcorrectly forwards events to both memory and optional console outputs. Null-safety is properly handled withconsoleOutput?.output(event).
182-248: LGTM - Stack trace extraction is thorough.The regex patterns cover both standard and package-prefixed stack trace formats. Skipping logger-internal frames ensures the reported service/line reflects the actual caller.
250-259: LGTM - Filter logic correctly gates logging.In debug mode, all logs pass through. In release mode, only logs when
isLoggingEnabledis true. This aligns with the PR objectives.
116-167: MemoryLogOutput as singleton correctly avoids disposal since listeners are managed by consumers.The
MemoryLogOutput.instancesingleton is never disposed, which is correct for a component meant to live throughout the app's lifetime. Consuming code likeLogsNotifierproperly manages its listener lifecycle by callingaddListener()in the constructor andremoveListener()indispose(). No changes needed.
|
@BraCR10 cuando llega el ultimo log hay overlapping con los botones del telefono, se debe agregar un margen como en las demas pantallas para que eso no suceda |
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
@BraCR10 sobre que los logs no persistan al cerrar y abrir la app, pensaba que iba a ser temporal, la idea es que siempre sea asi?
Creo que si un usuario activa la recoleccion de logs de forma manual, entonces para desactivarla lo deberia hacer tambien manualmente cuando ya no quiera tenerlo, no al cerrar la app.
Eso no tiene que hacerse en esta fase, pero lo debemos discutir para futuras fases
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/logs/screens/logs_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/features/logs/screens/logs_screen.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/features/logs/screens/logs_screen.dart
lib/features/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/screens/**/*.dart: Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
UseS.of(context)!.yourKeyfor all user-facing strings instead of hardcoded text
Files:
lib/features/logs/screens/logs_screen.dart
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/features/logs/screens/logs_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/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2026-01-09T22:09:06.887Z
Learnt from: BraCR10
Repo: MostroP2P/mobile PR: 403
File: lib/features/settings/settings_notifier.dart:153-156
Timestamp: 2026-01-09T22:09:06.887Z
Learning: In lib/features/settings/settings_notifier.dart, the updateLoggingEnabled method intentionally does NOT call _saveToPrefs() because the logging toggle should not persist across app restarts. The isLoggingEnabled field is deliberately excluded from Settings.toJson() and always returns false in fromJson() for security and performance reasons, requiring users to explicitly enable logging each session.
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/logs/screens/logs_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 (11)
lib/features/logs/screens/logs_screen.dart (11)
1-23: LGTM!Imports and state setup follow Riverpod patterns correctly. The use of
ConsumerStatefulWidgetis appropriate for this screen that needs both local state (scroll position, search query) and provider-based state (logs, settings).
25-54: LGTM!Lifecycle methods properly initialize and dispose of controllers. The scroll listener includes appropriate
hasClientscheck before accessing position.
56-72: LGTM!The dual update to both
MemoryLogOutput.isLoggingEnabledand the settings provider aligns with the intentional design where logging state doesn't persist across app restarts. Based on learnings, this is deliberate for security and performance reasons.
74-150: LGTM!All async operations properly check
mountedbefore usingBuildContext, and the SnackBar is correctly shown viaaddPostFrameCallback. This follows the coding guidelines for preventing errors on disposed widgets.
152-219: LGTM!The build method correctly separates
allLogs(for total count) from filtered logs (for display). TheSafeAreawrapper and provider watching pattern are appropriate.
221-299: LGTM!Stats header properly uses localized strings via
S.of(context)!and displays both total and filtered counts clearly.
301-395: LGTM!Search and filter functionality properly resets scroll position on changes and uses localized labels for display.
397-431: LGTM!Empty state properly uses localized strings for all user-facing text.
445-504: LGTM!Log item widget provides good visual structure with level indicator, source location, timestamp, and message content.
506-534: LGTM!Level-to-color and level-to-icon mappings are well-organized with appropriate fallback defaults.
433-443: Verify bottom padding issue — suggested fix appears excessive.The current
bottom: 16padding may be insufficient if the last log entry overlaps with system navigation, but the suggested change tobottom: 100is inconsistent with the app's design patterns. Throughout the codebase, bottom padding ranges from 0–16px (notifications: 8px, relays: 12px, logs: 16px). Additionally, the ListView is already wrapped inSafeArea(line 187), which automatically handles safe area insets for system navigation.If overlap is confirmed, consider:
- Increasing bottom padding incrementally (e.g., 24–32px) rather than to 100px
- Verifying that
SafeAreapadding is correctly applied to the ListView- Checking if the
Expandedwidget affects how the bottom padding is rendered
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @lib/features/logs/screens/logs_screen.dart:
- Around line 434-444: The ListView built by _buildLogsList is missing
sufficient bottom inset causing the last log to overlap system navigation;
update the ListView.padding to include a larger bottom safe inset (e.g., replace
bottom: 16 with bottom: MediaQuery.of(context).padding.bottom + 64 or a fixed
value like 80) or compute inset via MediaQuery.of(context).viewPadding.bottom,
ensuring the change is made where _buildLogsList constructs ListView.builder
(reference _scrollController and _buildLogItem) so the scrollable content
respects the device safe area.
🧹 Nitpick comments (1)
lib/features/logs/screens/logs_screen.dart (1)
106-110: Consider simplifying by removing unnecessary async.This method doesn't perform any async operations, so the
Future<void>return type andasynckeyword are redundant.♻️ Suggested simplification
- Future<void> _disableLoggingAndSave() async { + void _disableLoggingAndSave() { if (mounted) { _disableLogging(); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/logs/screens/logs_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/features/logs/screens/logs_screen.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/features/logs/screens/logs_screen.dart
lib/features/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/screens/**/*.dart: Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
UseS.of(context)!.yourKeyfor all user-facing strings instead of hardcoded text
Files:
lib/features/logs/screens/logs_screen.dart
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/main.dart : Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Use exact `order_expires_at` timestamps from Mostro protocol for DynamicCountdownWidget precision and localize display with `S.of(context)!.timeLeftLabel()`
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Use `S.of(context)!.yourKey` for all user-facing strings instead of hardcoded text
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Localize all user-facing strings via ARB files and access them with `S.of(context)` rather than hard-coded literals
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Implement DynamicCountdownWidget with automatic day/hour scaling: day scale (>24h) shows 'd h m' format, hour scale (≤24h) shows HH:MM:SS format
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/features/logs/screens/logs_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/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2026-01-09T22:09:06.887Z
Learnt from: BraCR10
Repo: MostroP2P/mobile PR: 403
File: lib/features/settings/settings_notifier.dart:153-156
Timestamp: 2026-01-09T22:09:06.887Z
Learning: In lib/features/settings/settings_notifier.dart, the updateLoggingEnabled method intentionally does NOT call _saveToPrefs() because the logging toggle should not persist across app restarts. The isLoggingEnabled field is deliberately excluded from Settings.toJson() and always returns false in fromJson() for security and performance reasons, requiring users to explicitly enable logging each session.
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/logs/screens/logs_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 (13)
lib/features/logs/screens/logs_screen.dart (13)
1-24: LGTM!Imports and state declarations are well-organized. The switch from
LogLevel?enum toString?for_selectedLevelaligns with the string-based filtering approach used byLogsFilter.
26-55: LGTM!Lifecycle methods are correctly implemented with proper cleanup in
dispose()and safe scroll position checks usinghasClients.
57-73: LGTM!The dual-update pattern correctly synchronizes both
MemoryLogOutput.isLoggingEnabledand the settings provider state. Based on learnings, this toggle intentionally doesn't persist across restarts for security reasons.
75-104: LGTM!Correctly checks
mountedafter the async dialog operation before proceeding. Good adherence to Flutter best practices for async operations in StatefulWidgets.
112-151: LGTM!Excellent implementation following best practices:
- Checks
mountedafter async dialog- Uses
addPostFrameCallbackfor the SnackBar side effect (per coding guidelines)- Double
mountedcheck ensures safety in the callback- All strings properly localized
153-220: LGTM!Build method correctly:
- Watches multiple providers for reactive updates
- Wraps body in
SafeAreafor safe area insets- Uses conditional rendering for empty/populated states
- Properly positions the scroll-to-top FAB
222-300: LGTM!Stats header is well-implemented with proper localization, clean layout, and uses the modern
withValues(alpha:)API.
302-347: LGTM!Search bar correctly handles state updates with safe scroll controller access via
hasClientscheck beforejumpTo.
349-396: LGTM!Filter chips correctly use string-based level filtering matching
LogsFilterdesign, with all labels properly localized.
398-432: LGTM!Empty state is cleanly implemented with proper localization.
446-505: LGTM!Log item rendering is well-structured with clear visual hierarchy: level indicator, service/line info, timestamp, and message content.
507-535: LGTM!Color and icon mapping methods correctly handle the common log levels with sensible defaults for edge cases.
537-539: LGTM!Clean delegation to the
timeAgoWithLocaleextension for localized relative time formatting, aligning with the PR's commit for localizing relative time strings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/logs/screens/logs_screen.dart (1)
508-536: Consider handling additional log levels.The
loggerpackage'sLevelenum includes additional values likeLevel.traceandLevel.fatalthat may appear in logs. The default cases provide safe fallbacks, but you may want explicit handling forLevel.fatal(critical errors) to ensure they're visually distinct.♻️ Optional: Add explicit handling for fatal level
Color _getLogLevelColor(Level level) { switch (level) { + case Level.fatal: + return AppTheme.statusError; case Level.error: return AppTheme.statusError; ... } } IconData _getLogLevelIcon(Level level) { switch (level) { + case Level.fatal: + return Icons.dangerous; case Level.error: return Icons.error; ... } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/logs/screens/logs_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/features/logs/screens/logs_screen.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/features/logs/screens/logs_screen.dart
lib/features/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/screens/**/*.dart: Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
UseS.of(context)!.yourKeyfor all user-facing strings instead of hardcoded text
Files:
lib/features/logs/screens/logs_screen.dart
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/main.dart : Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Use exact `order_expires_at` timestamps from Mostro protocol for DynamicCountdownWidget precision and localize display with `S.of(context)!.timeLeftLabel()`
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Use `S.of(context)!.yourKey` for all user-facing strings instead of hardcoded text
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Localize all user-facing strings via ARB files and access them with `S.of(context)` rather than hard-coded literals
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/widgets/dynamic_countdown_widget.dart : Implement DynamicCountdownWidget with automatic day/hour scaling: day scale (>24h) shows 'd h m' format, hour scale (≤24h) shows HH:MM:SS format
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/features/logs/screens/logs_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/logs/screens/logs_screen.dart
📚 Learning: 2026-01-09T22:09:06.887Z
Learnt from: BraCR10
Repo: MostroP2P/mobile PR: 403
File: lib/features/settings/settings_notifier.dart:153-156
Timestamp: 2026-01-09T22:09:06.887Z
Learning: In lib/features/settings/settings_notifier.dart, the updateLoggingEnabled method intentionally does NOT call _saveToPrefs() because the logging toggle should not persist across app restarts. The isLoggingEnabled field is deliberately excluded from Settings.toJson() and always returns false in fromJson() for security and performance reasons, requiring users to explicitly enable logging each session.
Applied to files:
lib/features/logs/screens/logs_screen.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/logs/screens/logs_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). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (9)
lib/features/logs/screens/logs_screen.dart (9)
1-10: LGTM!Imports are well-organized and all appear to be used. The new dependencies (
logs_provider,logger_service,datetime_extensions_utils) align with the logging infrastructure integration.
19-55: LGTM!State management follows Riverpod patterns correctly. The
ScrollControllerandTextEditingControllerare properly disposed, preventing memory leaks. The switch toString?for_selectedLevelaligns with the new provider-based filtering approach.
65-73: LGTM!The dual-state update approach (both
MemoryLogOutput.isLoggingEnabledand settings provider) ensures the logging service and UI state remain synchronized. Based on learnings, the intentional non-persistence of the logging toggle is by design for security/performance reasons.
141-150: LGTM!Excellent use of
WidgetsBinding.instance.addPostFrameCallbackfor the SnackBar, following the coding guideline for side effects. The nestedmountedcheck inside the callback is a defensive measure that prevents errors if the widget is disposed between the confirmation and the callback execution.
153-220: LGTM!The provider integration is well-structured:
logsProviderprovides the full log set for total count, whilefilteredLogsProvider(filter)handles the filtered view. Wrapping the body inSafeAreais good practice for handling device insets. The conditional disabling of the clear button when logs are empty prevents unnecessary user interaction.
316-344: LGTM!The defensive
hasClientscheck beforejumpToprevents potential errors if the scroll controller isn't attached yet. Resetting scroll position on search/filter changes provides a good UX by showing results from the top.
434-445: Good fix for the navigation overlap issue.The bottom padding calculation (
bottomInset + 64) properly addresses the reported issue where logs overlapped with phone navigation buttons. The log reversal patternlogs[logs.length - 1 - index]efficiently displays newest entries first without requiring a separate reversed list allocation.
447-506: LGTM!The log item UI is well-structured with clear visual hierarchy: level indicator with color/icon, source location (
service:line), timestamp, and message. The color-coded borders provide quick visual scanning of log severity.
538-540: LGTM!Using an extension method with locale-aware formatting is a clean approach that aligns with the app's internationalization strategy.
|
Hola @Catrya
Se corrigió el overlapping detrás de los botones de navegación del sistema utilizando
¿Te refieres a que la recolección de logs persista incluso si la app se cierra o se cae? |
Sí, incluso se pudiera poner como un issue al terminar todas las fases, para ser implementado después, porque cuando alguien quiere tener los logs es para registrar cualquier bug que le pase y si por ejemplo el bug implica que se cierre la app, entonces al abrirla el usuario no va a tener lo que pasó con ese bug. |
3 participants
Phase 2: Logger Service & Integration (Basic Implementation)
This PR implements the core logging service with memory buffer and connects it to the UI. This is a basic implementation, only 2 services are currently logging, so you'll see limited log entries.
What's Included
Service Migrations (2 files only):
lib/features/relays/relays_notifier.dart- Migrated to logger singleton (45+ log calls)lib/features/subscriptions/subscription_manager.dart- Migrated to logger singleton (13 log calls)Key Behaviors
Debug Mode:
Release Mode:
Features:
Testing
Navigate to: Settings → Dev Tools → Logs Report
Expected logs:
Next Phase
Phase 3: Core Services Migration - Migrate NostrService, MostroService, DeepLinkService + 2 additional core files.
See
docs/LOGGING_IMPLEMENTATION.mdfor complete architecture details.Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.