Skip to content

Conversation

@Delgado74
Copy link

@Delgado74 Delgado74 commented Oct 25, 2025

πŸ“„ Description

This PR fully implements Issue #316 by providing a user-accessible logging screen and the ability to export logs as a text file.

βœ… Benefits:

  • Users can send logs without ADB or dev setup
  • Faster debugging for developers
  • More informative bug reports

πŸ” Included in this PR:

  • πŸ“ New features/logs/ module following existing architecture
  • βœ… Persistent LogsService using ChangeNotifierProvider
  • πŸͺ΅ debugPrint interception for automatic error/info logging
  • πŸ“± New LogsScreen UI showing logs in realtime
  • πŸ“€ Buttons for clearing and sharing log file via share_plus
  • 🌍 Full i18n support (ES / EN / IT)
  • πŸͺ› Drawer navigation integrated
  • πŸ”’ Fix for StreamSink is bound to a stream logging errors
  • βœ… 4 unit tests added and passing

🎯 Architectural alignment
This implementation respects project structure:

βœ” Riverpod for service injection
βœ” Modular feature pattern
βœ” UI separated from business logic
βœ” i18n required for user-facing strings
βœ” Feature delivered with tests

πŸ“Œ No core logic or navigation architecture was modified.


βœ… Checklist


Screenshots

IMG_20251025_062911
Screenshot_2025-10-25-06-33-27-791_network mostro app
IMG_20251025_063345
Screenshot_2025-10-25-06-34-54-360_android

Summary by CodeRabbit

  • New Features

    • Logs screen: view selectable, monospace, color‑coded log lines with icons, delete all, and export/share; accessible via a new Logs route.
    • Persistent in‑app logging with optional native Android capture; settings toggle for app and native logging.
  • Localization

    • English, Spanish, and Italian translations for Logs UI strings.
  • UI

    • Drawer overlay tweaks: dimming and drag‑to‑close behavior improved.
  • Tests

    • Widget tests covering Logs screen display, clearing, and export actions.

- Adds reusable menu tile widget for navigating to LogsScreen
- Supports localization for title and subtitle
- Keeps drawer code clean and modular

Related to #316
…rception

- Creates logs file storage in app data directory
- Saves console output automatically into file
- Ensures persistence between app restarts
- Provides safe log writing protection to avoid StreamSink errors
- Exposes clean log export

Resolves #316 (part 1)
- Scrollable log view with color-coded severities
- Delete logs action with snackbar confirmation
- Export and share log file for debugging
- Dark/Light theme compatibility

Resolves #316 (part 2)
- Adds navigation to LogsScreen from Drawer
- Localized title and icon support

Resolves #316 (part 3)
Adds:
- logsScreenTitle
- deleteLogsTooltip
- logsDeletedMessage
- shareLogsTooltip
- logsShareText
- noLogsMessage
- logsMenuTitle
- logsMenuSubtitle

Related to #316
- Ensures logging is enabled immediately
- Prevents missing startup error logs

Related to #316
- Ensures initial state behavior
- Verifies log rendering
- Confirms delete and export actions
- Mocks LogsService correctly

Resolves #316 (final)
- Adds navigation to LogsScreen from Drawer
- Localized title and icon support
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 25, 2025

Walkthrough

Adds a file-backed LogsService with optional native capture, Riverpod providers and notifiers, a LogsScreen UI (view/clear/share), settings integration and startup wiring, Android EventChannel for native logs, localization (en/es/it), tests/mocks updates, and small theme/provider UI tweaks.

Changes

Cohort / File(s) Summary
Logs core & native
lib/features/logs/logs_service.dart, lib/features/logs/native_log_service.dart
New singleton LogsService managing in-memory + file-backed logs (init, append, clear, read/write, getLogFile, enable flags, dispose) and NativeLogService exposing a broadcast stream from Android EventChannel native_logcat_stream.
Providers & state
lib/features/logs/logs_provider.dart
New Riverpod providers: logsServiceProvider (ChangeNotifierProvider), logsProvider (Provider<UnmodifiableListView>), logsEnabledProvider and nativeLogsEnabledProvider (StateNotifierProviders) plus LogsState, LogsEnabledNotifier, and NativeLogsEnabledNotifier.
UI: Logs & Settings
lib/features/logs/logs_screen.dart, lib/features/settings/settings_screen.dart
New LogsScreen (reverse-ordered, monospace selectable rows with colored border/icon per level, delete/share actions, empty state) and a Logs card in Settings with a toggle wired to logsEnabledProvider and navigation to LogsScreen.
Startup & routing
lib/main.dart, lib/core/app_routes.dart
Wires LogsService at app startup (init, provider override, error handlers, _startApp), and registers /logs route to LogsScreen.
Android native capture & manifest
android/app/src/main/kotlin/.../MainActivity.kt, android/app/src/main/AndroidManifest.xml
MainActivity implements logcat capture and streams lines via EventChannel; AndroidManifest reorganized (activity, intent-filters, services, queries).
Localization
lib/l10n/intl_en.arb, lib/l10n/intl_es.arb, lib/l10n/intl_it.arb
Adds logs-related localization keys/translations (title, tooltips, messages, menu texts, enable/disable descriptions, share/error text, view button).
Tests & mocks
test/features/logs/logs_screen_test.dart, test/mocks.dart, test/mocks.mocks.dart
Adds widget tests for LogsScreen (empty/list, clear/share) and includes LogsService in mock generation; regenerated mocks include MockLogsService and supporting fakes/type index updates.
Theme & UI tweaks
lib/core/app_theme.dart, lib/shared/widgets/custom_drawer_overlay.dart
Adds AppTheme.statusNative color constant; minor drawer overlay changes (PopScope, drag-to-close, alpha/layout tweaks).
Shared providers export
lib/shared/providers/biometrics_provider.dart, lib/shared/providers/providers.dart
Adds biometrics provider scaffold and re-exports it from providers.dart.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant App as App Startup
    participant LS as LogsService
    participant NL as NativeLogService
    participant Android as MainActivity
    participant FS as File System
    participant Provider as Riverpod
    participant UI as LogsScreen

    App->>LS: init()
    LS->>FS: open/create log file, load lines
    LS-->>Provider: publish logs

    Android->>NL: EventChannel emits lines
    NL-->>LS: forward native lines
    LS->>FS: append formatted line
    LS-->>Provider: update logs

    User->>UI: open /logs
    UI->>Provider: subscribe to logsProvider
    Provider-->>UI: logs list
    UI-->>User: render color-coded rows

    User->>UI: tap Delete
    UI->>LS: clearLogs(clean: true)
    LS->>FS: truncate/recreate file
    LS-->>Provider: empty list
    UI-->>User: show SnackBar

    User->>UI: tap Share
    UI->>LS: getLogFile(clean: true)
    LS->>FS: create cleaned copy
    FS-->>UI: return file
    UI->>System: invoke share intent
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Extra attention:
    • File I/O, sink lifecycle, concurrency, and error handling in lib/features/logs/logs_service.dart.
    • Native capture lifecycle, threading, and EventChannel behavior in android/.../MainActivity.kt and lib/features/logs/native_log_service.dart.
    • Startup wiring: provider override/disposal and global error handlers in lib/main.dart.
    • Regenerated mocks in test/mocks.mocks.dart for updated indices/types and MockLogsService.
    • ARB files for duplicate keys or formatting issues.

Possibly related PRs

Suggested reviewers

  • Catrya
  • grunch

Poem

πŸ₯•πŸ“œ
I hop through timestamps, whiskers on the keys,
I color the oops and hush the gentle breeze.
I bundle the trail, tidy every line,
Share a crunchy log β€” nibble, prune, align.
Thump once for safety, thump twice β€” all fine.

Pre-merge checks and finishing touches

βœ… Passed checks (2 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The PR title clearly and directly describes the main change: adding a persistent logging system with export capability, matching the primary objective of implementing Issue #316.
✨ Finishing touches
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🧹 Nitpick comments (4)
lib/features/logs/logs_service.dart (4)

17-18: Expose an unmodifiable view for logs to prevent external mutation.

Prevents accidental external writes to your internal state.

Apply this diff:

-  List<String> get logs => _logs;
+  UnmodifiableListView<String> get logs => UnmodifiableListView(_logs);

Add this import at the top of the file:

import 'dart:collection';

95-98: Make β€œclean” export less destructive: strip ANSI/control chars but keep Unicode text.

Current regex drops all non‑ASCII (e.g., accented letters), harming readability.

Apply this diff:

-  String _cleanLine(String line) {
-    // Quita emojis y caracteres no imprimibles, dejando solo texto visible
-    return line.replaceAll(RegExp(r'[^\x20-\x7E]'), '');
-  }
+  String _cleanLine(String line) {
+    // Remove ANSI color codes and non-printable control chars; keep Unicode text.
+    final ansi = RegExp(r'\x1B\[[0-?]*[ -/]*[@-~]');
+    final noAnsi = line.replaceAll(ansi, '');
+    return noAnsi.replaceAll(RegExp(r'[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]'), '');
+  }

Also translate the above comment to English everywhere to meet guidelines. As per coding guidelines.


19-22: Set _initialized only after successful setup.

Current early assignment can leave the service β€œinitialized” even if setup fails mid-way.

Apply these diffs:

   Future<void> init() async {
     if (_initialized) return;
-    _initialized = true;

And at the end of init(), just before the closing brace:

-  }
+    _initialized = true;
+  }

Also applies to: 48-48


35-41: Avoid flushing on every line; it’s I/O heavy.

Let the OS buffer, or flush periodically/on demand (e.g., before export).

Apply this diff:

-      _sink?.writeln(line); // guardar con emojis/colores originales
-      _sink?.flush();
+      _sink?.writeln(line); // persist with original emojis/colors
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between b872221 and 14df3b3.

πŸ“’ Files selected for processing (9)
  • lib/features/logs/logs_menu_item.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/main.dart (3 hunks)
  • lib/shared/widgets/custom_drawer_overlay.dart (2 hunks)
  • test/features/logs/logs_screen_test.dart (1 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (8)
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/logs/logs_screen.dart
  • lib/features/logs/logs_menu_item.dart
  • lib/features/logs/logs_service.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/features/logs/logs_screen.dart
  • lib/features/logs/logs_menu_item.dart
  • test/features/logs/logs_screen_test.dart
  • lib/features/logs/logs_service.dart
  • lib/shared/widgets/custom_drawer_overlay.dart
  • lib/main.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/features/logs/logs_screen.dart
  • lib/features/logs/logs_menu_item.dart
  • lib/features/logs/logs_service.dart
  • lib/shared/widgets/custom_drawer_overlay.dart
  • lib/main.dart
test/**/*.dart

πŸ“„ CodeRabbit inference engine (AGENTS.md)

test/**/*.dart: Mirror feature layout for tests under test/
Prefer Mockito fakes generated via build_runner in tests

test/**/*.dart: Unit tests belong in test/; integration tests in integration_test/
Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes

Files:

  • test/features/logs/logs_screen_test.dart
test/**/*_test.dart

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Name unit/widget test files with the *_test.dart suffix and colocate beside feature code under test/

Files:

  • test/features/logs/logs_screen_test.dart
lib/l10n/*.arb

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

lib/l10n/*.arb: Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)
When adding a new localization key, update all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
After modifying ARB files, run dart run build_runner build -d to regenerate localization

Files:

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

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Put shared utilities in lib/shared/

Files:

  • lib/shared/widgets/custom_drawer_overlay.dart
lib/main.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Configure timeago locales and app initialization in main.dart

Files:

  • lib/main.dart
🧠 Learnings (4)
πŸ“š Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to test/**/*.dart : Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes

Applied to files:

  • test/features/logs/logs_screen_test.dart
πŸ“š Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/l10n/*.arb : When adding a new localization key, update all three ARB files (en, es, it)

Applied to files:

  • lib/l10n/intl_it.arb
πŸ“š Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/l10n/*.arb : Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)

Applied to files:

  • lib/l10n/intl_es.arb
πŸ“š Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
PR: MostroP2P/mobile#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/main.dart
πŸͺ› RuboCop (1.81.1)
lib/l10n/intl_it.arb

[fatal] 1182-1182: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

(Lint/Syntax)

lib/l10n/intl_es.arb

[fatal] 1128-1128: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

(Lint/Syntax)

lib/l10n/intl_en.arb

[fatal] 1149-1149: unexpected token tCOLON
(Using Ruby 2.7 parser; configure using TargetRubyVersion parameter, under AllCops)

(Lint/Syntax)

πŸ”‡ Additional comments (3)
test/features/logs/logs_screen_test.dart (1)

1-98: LGTM! Well-structured widget tests.

The test suite provides comprehensive coverage of LogsScreen functionality with proper mocking, localization setup, and verification of key user interactions. The tests correctly handle Riverpod provider overrides and safely extract localization strings from the test context.

lib/features/logs/logs_screen.dart (1)

1-106: LGTM! Clean implementation following Flutter best practices.

The LogsScreen implementation demonstrates good practices:

  • Captures S.of(context) before async operations (line 33) to avoid BuildContext usage after async gaps
  • Uses withAlpha instead of deprecated withOpacity (lines 89-90) following latest Flutter APIs
  • Implements proper theme integration with dark mode support
  • Color-codes logs by severity for better readability
  • Provides both clear and export functionality with appropriate user feedback
lib/main.dart (1)

18-79: LGTM! Clean integration of LogsService into app startup.

The changes properly integrate LogsService into the application lifecycle with good practices:

  • Initializes LogsService early in startup before runApp
  • Sets up global error handling with FlutterError.onError and runZonedGuarded
  • Correctly wires LogsService into the dependency injection graph via logsProvider override
  • Refactors startup flow into _startApp for better organization
  • Maintains all existing initialization steps

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

can you address @coderabbitai suggestions?

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

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

Hi @Delgado74 great that you want to make your first collaboration! however it doesn't compile, this is the error that appears:

FormatException: Unexpected character (at line 1149, character 3)
  "@_comment_logsScreenTitle": "Title for the logs screen in the app",
  ^

Launching lib/main.dart on Linux in debug mode...
ERROR: Target gen_localizations failed: Error: The arb file /home/catry/mobile/lib/l10n/intl_en.arb has the following formatting issue: 
Building Linux application...                                           
Error: Build process failed

@Catrya
Copy link
Member

Catrya commented Oct 27, 2025

@Delgado74 In the image you share are shown Flutter errors "StreamSink is bound to a stream" that are not app logs, but in the logging implementation, it must be fixed
image_2025-10-27_16-26-20

@Delgado74
Copy link
Author

Thank you. I will follow your recommendations and those of the coderabbi bot as suggested @grunch

@Delgado74
Copy link
Author

Gracia Catrya por el feedback. Voy a seguir trabajando porque hay alguans sugerencias de coderabbi que aun me quedan. Gracias por la oportunidad.

- Replaced Navigator.push with context.push('/logs')
- Ensured drawer closes before navigation
- Maintained localization and UI styling
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
lib/shared/widgets/custom_drawer_overlay.dart (4)

19-26: Verify PopScope API; consider onPopInvoked for wider compatibility.

onPopInvokedWithResult needs a newer Flutter. If your min SDK doesn’t guarantee it, switch to onPopInvoked to keep analyzer green.

-    return PopScope(
-      canPop: !isDrawerOpen,
-      onPopInvokedWithResult: (didPop, result) {
+    return PopScope(
+      canPop: !isDrawerOpen,
+      onPopInvoked: (didPop) {
         if (!didPop && isDrawerOpen) {
           ref.read(drawerProvider.notifier).closeDrawer();
         }
       },

39-39: Use Color.withValues(...) instead of withAlpha(...) per guidelines.

Express alpha as 0–1 and align with β€œlatest APIs” rule. Confirm your Flutter version supports it.

-                color: Colors.black.withAlpha(80),
+                color: Colors.black.withValues(alpha: 0.31),
@@
-                      color: Colors.white.withAlpha(25),
+                      color: Colors.white.withValues(alpha: 0.10),
@@
-                        color: Colors.white.withAlpha(25),
+                        color: Colors.white.withValues(alpha: 0.10),

Also applies to: 63-63, 92-92


51-56: Make swipe-to-close RTL-aware.

Close on β€œtowards the edge” regardless of text direction.

-              onHorizontalDragEnd: (details) {
-                if (details.primaryVelocity != null &&
-                    details.primaryVelocity! < 0) {
-                  ref.read(drawerProvider.notifier).closeDrawer();
-                }
-              },
+              onHorizontalDragEnd: (details) {
+                final v = details.primaryVelocity ?? 0;
+                final isRtl = Directionality.of(context) == TextDirection.rtl;
+                final shouldClose = (!isRtl && v < 0) || (isRtl && v > 0);
+                if (shouldClose) {
+                  ref.read(drawerProvider.notifier).closeDrawer();
+                }
+              },

17-18: Avoid repeated MediaQuery calls; cache width.

Minor perf/readability win.

   final isDrawerOpen = ref.watch(drawerProvider);
-  final statusBarHeight = MediaQuery.of(context).padding.top;
+  final statusBarHeight = MediaQuery.of(context).padding.top;
+  final screenWidth = MediaQuery.of(context).size.width;
@@
-            left: isDrawerOpen ? 0 : -MediaQuery.of(context).size.width * 0.7,
+            left: isDrawerOpen ? 0 : -screenWidth * 0.7,
@@
-                width: MediaQuery.of(context).size.width * 0.7,
+                width: screenWidth * 0.7,

Also applies to: 47-47, 58-58

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between cc92620 and 5a28533.

πŸ“’ Files selected for processing (1)
  • lib/shared/widgets/custom_drawer_overlay.dart (4 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (3)
lib/shared/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Put shared utilities in lib/shared/

Files:

  • lib/shared/widgets/custom_drawer_overlay.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/custom_drawer_overlay.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/custom_drawer_overlay.dart
🧠 Learnings (1)
πŸ“š Learning: 2025-10-22T12:29:26.971Z
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-22T12:29:26.971Z
Learning: Applies to lib/core/app_routes.dart : Use GoRouter for navigation configured in core/app_routes.dart

Applied to files:

  • lib/shared/widgets/custom_drawer_overlay.dart
πŸ”‡ Additional comments (1)
lib/shared/widgets/custom_drawer_overlay.dart (1)

121-142: LGTM: logs entry uses go_router + i18n consistently.

Navigation and strings align with project rules. Nice fix after the earlier Navigator push note. Based on learnings.

@grunch
Copy link
Member

grunch commented Oct 30, 2025

@Delgado74 No me gusta la secciΓ³n de logs en el drawer, crea una nueva card en settings justo arriba de la card de mostro (que sea la penΓΊltima), se llama Logs con el icono que consideres correcto (el del bichito creo que estΓ‘ bien), en la card debe haber un switch que por defecto estΓ© activado, el switch activado quiere decir que estΓ‘ guardando los logs, si el usuario lo apaga ya no guarda nada, y luego un botΓ³n o link (manteniendo el mismo estilo que usa la app) que enviΓ© al usuario a la pantalla donde visualizas los logs y puedes compartir

@Delgado74
Copy link
Author

Perfecto. Me pongo a trabajar en esa sugerencia. Gracias por la oportunidad.

- Updated `logs_screen.dart` to display, delete, and export logs.
- Integrated a toggle in `settings_screen.dart` to enable/disable logging.
- Improved `logs_service.dart` for better file handling and Android compatibility.
- Added `biometrics_provider.dart` and updated `providers.dart` to unify shared service access.
- Updated `custom_drawer_overlay.dart` and `main.dart` to include new dependencies and navigation logic.
- Added translations to `intl_en.arb`, `intl_es.arb`, and `intl_it.arb`.
- Modified `MainActivity.kt` to support file operations from Flutter.
- Updated tests (`logs_screen_test.dart`, `mocks.mocks.dart`) to reflect new services and structure.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
lib/shared/providers/biometrics_provider.dart (1)

4-6: Remove dead code: unused shared biometrics provider and duplicate definition.

This provider is exported from lib/shared/providers/providers.dart but unusedβ€”the codebase only uses the identical provider defined in lib/features/auth/providers/auth_notifier_provider.dart, which is overridden at runtime in lib/main.dart.

Actions:

  • Remove lib/shared/providers/biometrics_provider.dart (file is dead code)
  • Remove its export from lib/shared/providers/providers.dart
  • Keep the definition in auth_notifier_provider.dart (actively used by AuthRepository)
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 5a28533 and a457c3c.

πŸ“’ Files selected for processing (14)
  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (1 hunks)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
  • lib/features/settings/settings_screen.dart (3 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/main.dart (3 hunks)
  • lib/shared/providers/biometrics_provider.dart (1 hunks)
  • lib/shared/providers/providers.dart (1 hunks)
  • lib/shared/widgets/custom_drawer_overlay.dart (4 hunks)
  • test/features/logs/logs_screen_test.dart (1 hunks)
  • test/mocks.mocks.dart (36 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • test/features/logs/logs_screen_test.dart
  • lib/l10n/intl_en.arb
  • lib/features/logs/logs_screen.dart
  • lib/l10n/intl_es.arb
  • lib/shared/widgets/custom_drawer_overlay.dart
🧰 Additional context used
πŸ““ Path-based instructions (11)
lib/shared/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Put shared utilities in lib/shared/

Files:

  • lib/shared/providers/providers.dart
  • lib/shared/providers/biometrics_provider.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/providers/providers.dart
  • lib/features/settings/settings_screen.dart
  • lib/shared/providers/biometrics_provider.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • test/mocks.mocks.dart
  • lib/main.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/providers/providers.dart
  • lib/features/settings/settings_screen.dart
  • lib/shared/providers/biometrics_provider.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • lib/main.dart
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
lib/l10n/*.arb

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

lib/l10n/*.arb: Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)
When adding a new localization key, update all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
After modifying ARB files, run dart run build_runner build -d to regenerate localization

Files:

  • lib/l10n/intl_it.arb
test/**/*.dart

πŸ“„ CodeRabbit inference engine (AGENTS.md)

test/**/*.dart: Mirror feature layout for tests under test/
Prefer Mockito fakes generated via build_runner in tests

test/**/*.dart: Unit tests belong in test/; integration tests in integration_test/
Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes

Files:

  • test/mocks.mocks.dart
test/mocks.mocks.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Do not manually edit test/mocks.mocks.dart (Mockito-generated)

Files:

  • test/mocks.mocks.dart
**/*.mocks.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Do not manually edit any *.mocks.dart generated Mockito files

Files:

  • test/mocks.mocks.dart
**/*.{g.dart,mocks.dart}

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Never add per-item ignore comments to generated files; rely on file-level ignores or regenerate

Files:

  • test/mocks.mocks.dart
{android,ios,web}/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Keep platform wrapper code in android/, ios/, and web/

Files:

  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt
lib/main.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Configure timeago locales and app initialization in main.dart

Files:

  • lib/main.dart
🧠 Learnings (35)
πŸ“š 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/shared/providers/providers.dart
  • lib/features/logs/logs_provider.dart
  • test/mocks.mocks.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/shared/providers/providers.dart
  • test/mocks.mocks.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/shared/providers/providers.dart
  • lib/shared/providers/biometrics_provider.dart
  • lib/features/logs/logs_provider.dart
  • test/mocks.mocks.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/shared/providers/providers.dart
  • lib/shared/providers/biometrics_provider.dart
  • lib/main.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/**/screens/**/*.dart : Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)

Applied to files:

  • lib/features/settings/settings_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:

  • lib/features/settings/settings_screen.dart
  • lib/main.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_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/settings/settings_screen.dart
  • lib/main.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/settings/settings.dart : Settings model must include blacklistedRelays and maintain backward-compatible serialization

Applied to files:

  • lib/features/settings/settings_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/settings/settings_screen.dart
  • test/mocks.mocks.dart
  • lib/main.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/shared/providers/biometrics_provider.dart
  • lib/features/logs/logs_provider.dart
  • test/mocks.mocks.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/shared/providers/biometrics_provider.dart
  • lib/features/logs/logs_provider.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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • test/mocks.mocks.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/logs/logs_provider.dart
  • lib/main.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/services/nostr_service.dart : All Nostr protocol interactions must go through NostrService (services/nostr_service.dart)

Applied to files:

  • lib/features/logs/logs_provider.dart
  • test/mocks.mocks.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/l10n/*.arb : When adding a new localization key, update all three ARB files (en, es, it)

Applied to files:

  • lib/l10n/intl_it.arb
πŸ“š 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/l10n/*.arb : Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)

Applied to files:

  • lib/l10n/intl_it.arb
πŸ“š 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/l10n/*.arb : After modifying ARB files, run dart run build_runner build -d to regenerate localization

Applied to files:

  • lib/l10n/intl_it.arb
πŸ“š 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/l10n/*.arb : Use proper ARB metadata for parameterized strings

Applied to files:

  • lib/l10n/intl_it.arb
πŸ“š 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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/features/logs/logs_service.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:

  • test/mocks.mocks.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/**/*.dart : Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes

Applied to files:

  • test/mocks.mocks.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 **/*.mocks.dart : Do not manually edit any *.mocks.dart generated Mockito files

Applied to files:

  • test/mocks.mocks.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 test/**/*.dart : Prefer Mockito fakes generated via build_runner in tests

Applied to files:

  • test/mocks.mocks.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:

  • test/mocks.mocks.dart
  • lib/main.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:

  • test/mocks.mocks.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 test/**/*.dart : Mirror feature layout for tests under test/

Applied to files:

  • test/mocks.mocks.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: Run flutter test integration_test/ for significant changes only

Applied to files:

  • test/mocks.mocks.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 integration_test/**/*.dart : Ensure integration coverage for flows involving networking, background work, or session recovery

Applied to files:

  • test/mocks.mocks.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:

  • test/mocks.mocks.dart
  • lib/main.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:

  • test/mocks.mocks.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:

  • test/mocks.mocks.dart
πŸ“š Learning: 2025-05-08T16:31:29.582Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.

Applied to files:

  • test/mocks.mocks.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/relay.dart : Relay model must track source (user, mostro, defaultConfig) and expose canDelete/canBlacklist semantics

Applied to files:

  • test/mocks.mocks.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/subscriptions/subscription_manager.dart : SubscriptionManager manages subscriptions, including relay list stream; ensure proper initialization and cleanup

Applied to files:

  • test/mocks.mocks.dart
  • lib/main.dart
πŸ”‡ Additional comments (16)
lib/shared/providers/providers.dart (1)

3-3: LGTM - Export added correctly.

The export follows the existing pattern and is correctly placed alphabetically. The biometrics provider implementation will be reviewed separately.

lib/l10n/intl_it.arb (1)

1182-1195: LGTM - Italian localization added correctly.

The new logs-related localization keys are properly structured and the JSON syntax issue from previous reviews has been resolved. All required keys for the logs feature UI are present with Italian translations.

lib/features/settings/settings_screen.dart (2)

13-13: LGTM - Provider import added correctly.

The logs_provider import is properly placed with other feature imports.


103-105: LGTM - Logs card positioned correctly in settings.

The Logs card is placed before the Mostro card as requested, with appropriate spacing.

lib/features/logs/logs_provider.dart (2)

27-53: LogsNotifier implementation looks correct.

The notifier properly delegates to LogsService and maintains local state. The initialization pattern and method implementations are sound.


55-70: LogsEnabledNotifier implementation looks correct.

The enabled state management correctly syncs with SharedPreferences through LogsService, and the toggle method properly updates both the service and local state.

lib/main.dart (3)

24-53: Comprehensive error capture setup - good approach.

The initialization of LogsService and global error handlers is well-structured:

  • LogsService initialized early before UI
  • FlutterError.onError captures framework errors
  • PlatformDispatcher.onError captures Dart errors
  • runZonedGuarded catches zone errors
  • All routes errors to persistent logs

This provides good diagnostic capability for the logs feature.

However, translate the Spanish comments to English (lines 24, 28, 31, 39, 49-50) as per coding guidelines.


55-95: LGTM - App initialization refactored correctly.

The new _startApp entry point properly receives the LogsService instance and correctly wires it through provider overrides. The initialization sequence is appropriate and the provider overrides are correctly structured.

Note: Line 68's initializeNotifications() is now active (previously commented per the AI summary context). Ensure this is intentional for this PR's scope.


97-104: LGTM - Relay initialization error now logged.

The relay synchronization initialization now properly logs failures through LogsService instead of silently failing. This improves debuggability.

lib/features/logs/logs_service.dart (6)

8-22: LGTM - Singleton pattern and state management correct.

The singleton implementation is appropriate for a logging service. The use of UnmodifiableListView for the logs getter properly prevents external mutation of internal state.


24-50: LGTM - Initialization is robust and idempotent.

The init method:

  • Properly guards against re-initialization
  • Loads persisted enabled state from SharedPreferences
  • Loads existing logs from file
  • Sets up file sink for appending
  • Has appropriate error handling with rethrow

The initialization sequence is correct and well-structured.


66-84: LGTM - Logging methods implemented correctly.

The log() method properly:

  • Checks initialization and enabled state
  • Formats with ISO8601 timestamp
  • Updates in-memory list
  • Writes to file with error handling
  • Provides backward-compatible writeLog() alias

91-109: LGTM - Safe log clearing with proper resource management.

The clearLogs() method correctly:

  • Closes the current sink before truncating
  • Clears both file and memory
  • Reopens the sink for future writes

This addresses the previous critical issue about unsafe truncation while sink was open.


111-149: LGTM - File retrieval and cleaning implemented correctly.

The getLogFile() method:

  • Flushes before reading to ensure data consistency
  • Supports optional cleaning of ANSI codes and control characters
  • Returns null safely if not initialized
  • Has proper error handling

The _cleanLine() helper appropriately removes ANSI escape sequences and control characters while preserving Unicode text.


151-158: LGTM - Proper resource cleanup in dispose.

The dispose method correctly:

  • Flushes pending writes
  • Closes the sink
  • Resets the initialized flag

This ensures resources are properly released when the service is torn down.

test/mocks.mocks.dart (1)

1-3182: LGTM! Properly regenerated mock file.

This file is correctly generated by Mockito and follows project guidelines. The addition of MockLogsService and related type realignments are consistent with the new logging feature. As per coding guidelines, this file should only be updated by running dart run build_runner build -d after changes to test/mocks.dart, which appears to have been done correctly.

- Capture native Android logs (logcat) alongside Flutter logs
- Filter logs by app PID to show only Mostro logs
- Add [NATIVE] prefix and distinctive UI styling for native logs
- Implement toggle to enable/disable native log capture
- Display Android icons and color coding for log types (V/D/I/W/E)
- Preserve existing Flutter log functionality

Technical changes:
- MainActivity.kt: EventChannel for streaming logcat output
- native_log_service.dart: Dart service to receive native logs
- logs_service.dart: Unified log collection (Flutter + Native)
- logs_provider.dart: State management for native logs toggle
- logs_screen.dart: Enhanced UI with native log indicators
- AndroidManifest.xml: Added READ_LOGS permission

Files modified:
- android/app/src/main/kotlin/network/mostro/app/MainActivity.kt
- android/app/src/main/AndroidManifest.xml
- lib/features/logs/logs_service.dart
- lib/features/logs/logs_provider.dart
- lib/features/logs/logs_screen.dart

Files created:
- lib/features/logs/native_log_service.dart

Note: Native logs include system events, Flutter logs from logcat,
performance warnings, and full stack traces. Team feedback needed
on verbosity level (currently captures V/D/I/W/E).

Testing: Run app, navigate to Logs screen, observe [NATIVE] prefix
on Android system logs alongside Flutter logs."
@Delgado74
Copy link
Author

πŸ”§ Feature: Captura de Logs Nativos de Android

Hola equipo πŸ‘‹

He implementado la captura de logs nativos de Android (logcat) integrada con nuestro sistema de logs existente.

πŸ“‹ ΒΏQuΓ© hace?

Ahora podemos ver ambos tipos de logs en la misma pantalla:

  • βœ… Logs de Flutter (los que ya tenΓ­amos)
  • βœ… Logs nativos de Android (nuevos):
    • Logs del sistema Android (UI events, Surface, etc.)
    • Logs de Flutter capturados por logcat (I/flutter)
    • Stack traces completos
    • Warnings de performance (Slow Looper, InputEventReceiver)
    • Solo logs de nuestra app (filtrado por PID)

🎨 Cómo se ve

Los logs nativos tienen:

  • Prefijo [NATIVE] en el texto
  • Ícono de Android πŸ€–
  • Color naranja distintivo
  • Borde izquierdo de color segΓΊn nivel (V/D/I/W/E)

Ejemplo de logs capturados:

[NATIVE] 11-01 02:22:29.102 V/PhoneWindow: DecorView setVisiblity...
[NATIVE] 11-01 02:22:43.116 I/flutter: πŸ’‘ No sessions yet, skipping chat load
[NATIVE] 11-01 02:23:00.385 W/Looper: Slow Looper main: doFrame is 309ms late

πŸ§ͺ CΓ³mo probar

  1. Compila y ejecuta la app
  2. Ve a la pantalla de Logs
  3. Observa que ahora hay dos tipos de logs mezclados:
    • Logs normales (de Flutter)
    • Logs con [NATIVE] y Γ­cono de Android

βš™οΈ ConfiguraciΓ³n actual

Nivel de captura: Todos los niveles (V/D/I/W/E)

  • V (Verbose): Eventos de bajo nivel del sistema
  • D (Debug): InformaciΓ³n de debugging
  • I (Info): Logs informativos (incluye nuestros logs de Flutter)
  • W (Warning): Alertas de performance
  • E (Error): Errores crΓ­ticos

πŸ€” Necesito su feedback:

  1. ΒΏEl nivel de detalle es adecuado?

    • ΒΏHay demasiados logs Verbose/Debug?
    • ΒΏPrefieren solo I/W/E?
  2. ΒΏQuieren filtrar algunos tags?

    • Ejemplo: Silenciar PhoneWindow, Surface, mali_winsys
    • O mostrar solo logs relevantes de la app
  3. ΒΏLa UI es clara?

    • ΒΏSe distinguen bien los logs nativos de los de Flutter?
    • ΒΏAlguna mejora en colores/iconos?
  4. ΒΏCasos de uso?

    • ΒΏPara debugging de producciΓ³n?
    • ΒΏSolo para desarrollo?
    • ΒΏExportar logs tΓ©cnicos para soporte?

πŸ“ Archivos modificados

Android (Kotlin):

  • MainActivity.kt - EventChannel para streaming de logcat
  • AndroidManifest.xml - Permiso READ_LOGS

Dart:

  • native_log_service.dart - (NUEVO) Servicio para recibir logs nativos
  • logs_service.dart - IntegraciΓ³n de logs nativos
  • logs_provider.dart - Provider para toggle de logs nativos
  • logs_screen.dart - UI mejorada con indicadores

🎯 Próximos pasos (según su feedback)

  • Ajustar nivel de filtrado (V/D/I/W/E)
  • Agregar switch en UI para habilitar/deshabilitar logs nativos
  • Implementar bΓΊsqueda/filtrado por palabra clave
  • Separar logs en tabs (Flutter / Native / Todos)

Por favor prueben y dΓ­ganme quΓ© piensan. Especialmente si el nivel de logs es ΓΊtil o si es demasiado verbose.

Gracias! πŸš€

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (8)
lib/main.dart (1)

1-109: Translate all Spanish comments and log messages to English.

Multiple Spanish comments and log messages throughout the file violate coding guidelines requiring English-only code. This applies to lines 19, 24, 28, 29, 31, 33, 35, 39, 41, 42, 49, 50, 68, 83, 102.

Apply this diff to translate all Spanish text:

-import 'package:mostro_mobile/features/notifications/services/background_notification_service.dart'; // πŸ”Ή AGREGAR
+import 'package:mostro_mobile/features/notifications/services/background_notification_service.dart'; // ADD

-  // Inicializa LogsService
+  // Initialize LogsService
   final logsService = LogsService();
   await logsService.init();

-  // Log de inicio
-  logsService.log('πŸš€ App iniciada');
+  // Startup log
+  logsService.log('πŸš€ App started');

-  // Captura errores globales de Flutter
+  // Capture global Flutter errors
   FlutterError.onError = (details) {
     logsService.log('❌ FlutterError: ${details.exceptionAsString()}');

-  // Captura errores de Dart no manejados
+  // Capture unhandled Dart errors
   PlatformDispatcher.instance.onError = (error, stack) {
     logsService.log('❌ Uncaught error: $error');

       (error, stackTrace) {
     logsService.log('⚠️ Zone error: $error');
     logsService.log('StackTrace: $stackTrace');

-  await initializeNotifications(); // πŸ”Ή DESCOMENTADO
+  await initializeNotifications(); // Uncommented

As per coding guidelines.

lib/features/logs/native_log_service.dart (1)

17-17: Translate Spanish error message to English.

The error message is in Spanish but coding guidelines require all comments and user-facing text to be in English.

Apply this diff:

-       print('❌ Error en stream de logcat nativo: $error');
+       print('❌ Error in native logcat stream: $error');

As per coding guidelines.

android/app/src/main/AndroidManifest.xml (1)

58-64: Translate Spanish comments to English.

Comments must be in English per coding guidelines.

Apply this diff:

- <!-- πŸ‘‡ PERMISOS EXISTENTES -->
+ <!-- Existing permissions -->
  <uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
  <uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />
  <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
  <uses-permission android:name="android.permission.INTERNET" />

- <!-- πŸ‘‡ AGREGAR ESTE PERMISO NUEVO -->
- <uses-permission android:name="android.permission.READ_LOGS" />
-

As per coding guidelines.

android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (2)

22-55: Translate Spanish comments to English.

All comments must be in English per coding guidelines.

Apply this diff:

- // EventChannel para streaming automΓ‘tico de logs nativos
+ // EventChannel for automatic native log streaming
  EventChannel(flutterEngine.dartExecutor.binaryMessenger, EVENT_CHANNEL)
      .setStreamHandler(object : EventChannel.StreamHandler {
          // ...
      })

  // In startLogCapture:
- // Limpiar logcat previo
+ // Clear previous logcat
  Runtime.getRuntime().exec("logcat -c").waitFor()

- // Capturar logs solo de esta app con timestamp
+ // Capture logs only for this app with timestamp
  logcatProcess = Runtime.getRuntime().exec(...)

- // πŸ‘‡ CORRECCIΓ“N: Usar while con asignaciΓ³n inline
+ // Use inline assignment in while loop
  while (isCapturing) {

As per coding guidelines.


35-74: Add resource cleanup and handle process termination properly.

The BufferedReader is never explicitly closed, and Process.destroy() may not terminate immediately on all Android versions. This can leak file descriptors and leave zombie processes.

Apply this approach to ensure cleanup:

  private fun startLogCapture(eventSink: EventChannel.EventSink?) {
      if (isCapturing) return
      isCapturing = true

      Thread {
+         var reader: BufferedReader? = null
          try {
              // Clear previous logcat
              Runtime.getRuntime().exec("logcat -c").waitFor()

              // Capture logs for this app with timestamp
              logcatProcess = Runtime.getRuntime().exec(
                  arrayOf(
                      "logcat",
                      "-v", "time",
                      "--pid=${android.os.Process.myPid()}"
                  )
              )

-             val reader = BufferedReader(InputStreamReader(logcatProcess?.inputStream))
+             reader = BufferedReader(InputStreamReader(logcatProcess?.inputStream))

-             // πŸ‘‡ CORRECCIΓ“N: Usar while con asignaciΓ³n inline
              while (isCapturing) {
                  val line = reader.readLine() ?: break

                  if (line.isNotEmpty()) {
                      handler.post {
                          eventSink?.success(line)
                      }
                  }
              }
          } catch (e: Exception) {
              Log.e("MostroLogCapture", "Error capturing native logs", e)
              handler.post {
                  eventSink?.error("LOGCAT_ERROR", e.message, null)
              }
          } finally {
+             reader?.close()
+             logcatProcess?.destroy()
+             // Forcibly kill if not terminated after brief wait
+             try {
+                 if (logcatProcess?.waitFor(100, java.util.concurrent.TimeUnit.MILLISECONDS) == false) {
+                     logcatProcess?.destroyForcibly()
+                 }
+             } catch (e: Exception) {
+                 Log.w("MostroLogCapture", "Error waiting for process termination", e)
+             }
              isCapturing = false
          }
      }.start()
  }
lib/features/logs/logs_screen.dart (1)

12-138: Translate Spanish comments to English.

Multiple comments are in Spanish (lines 12, 15, 29, 116, 127, 138) but must be in English per coding guidelines.

Apply this diff:

  Color _getLogColor(String line) {
-   // πŸ‘‡ AGREGAR: Detectar logs nativos primero
+   // Detect native logs first
    if (line.contains('[NATIVE]')) {
-     // Color especΓ­fico para logs nativos de Android
+     // Specific color for Android native logs
      return const Color(0xFFFF9800); // Naranja para nativos
    }

- // πŸ‘‡ AGREGAR: MΓ©todo para obtener icono segΓΊn tipo de log
+ // Get icon based on log type
  IconData _getLogIcon(String line) {

          final log = logs[i];
          final logColor = _getLogColor(log);
-         final logIcon = _getLogIcon(log); // πŸ‘ˆ USAR NUEVO MΓ‰TODO
+         final logIcon = _getLogIcon(log); // Use new method

          return Container(
            // ...
            decoration: BoxDecoration(
              // ...
-             // πŸ‘‡ AGREGAR: Borde izquierdo de color para mejor distinciΓ³n
+             // Left colored border for better distinction
              border: Border(

            child: Row(
              crossAxisAlignment: CrossAxisAlignment.start,
              children: [
-               // πŸ‘‡ AGREGAR: Icono indicador
+               // Indicator icon
                Icon(

As per coding guidelines.

lib/features/logs/logs_provider.dart (1)

7-91: Translate all Spanish comments to English.

Multiple Spanish comments remain (lines 7, 10, 16, 22, 27, 75) despite previous review feedback. All comments must be in English per coding guidelines.

Apply this diff:

-// Provider principal del servicio (singleton)
+// Main service provider (singleton)
 final logsServiceProvider = Provider<LogsService>((ref) => LogsService());

-// Provider para acceso directo a los logs (reactivo)
+// Provider for direct reactive access to logs
 final logsProvider = Provider<UnmodifiableListView<String>>((ref) {

-// Provider para el notifier (mantiene tu lΓ³gica actual)
+// Provider for the notifier (maintains current logic)
 final logsNotifierProvider =

-// Provider para el estado del switch de logs Flutter
+// Provider for Flutter logs switch state
 final logsEnabledProvider = StateNotifierProvider<LogsEnabledNotifier, bool>((ref) {

-// πŸ‘‡ AGREGAR: Provider para el estado del switch de logs nativos
+// Provider for native logs switch state
 final nativeLogsEnabledProvider = StateNotifierProvider<NativeLogsEnabledNotifier, bool>((ref) {

-// πŸ‘‡ AGREGAR CLASE COMPLETA
+// NativeLogsEnabledNotifier class
 class NativeLogsEnabledNotifier extends StateNotifier<bool> {

As per coding guidelines.

lib/features/logs/logs_service.dart (1)

7-227: Translate all Spanish comments and log messages to English.

Multiple Spanish comments and log messages remain throughout the file (lines 7, 24, 27, 52, 65, 72, 81, 85, 89, 101, 114, 125, 220). All text must be in English per coding guidelines.

Apply this diff to translate Spanish text:

-import 'native_log_service.dart'; // πŸ‘ˆ AGREGAR
+import 'native_log_service.dart'; // ADD

-  static const String _nativeLogsEnabledKey = 'native_logs_enabled'; // πŸ‘ˆ NUEVO
+  static const String _nativeLogsEnabledKey = 'native_logs_enabled'; // NEW

-  bool _nativeLogsEnabled = true; // πŸ‘ˆ NUEVO
+  bool _nativeLogsEnabled = true; // NEW

-  // πŸ‘‡ AGREGAR: Servicio de logs nativos
+  // Native log service
   final NativeLogService _nativeLogService = NativeLogService();

-      _nativeLogsEnabled = prefs.getBool(_nativeLogsEnabledKey) ?? true; // πŸ‘ˆ NUEVO
+      _nativeLogsEnabled = prefs.getBool(_nativeLogsEnabledKey) ?? true; // NEW

-      // πŸ‘‡ AGREGAR: Iniciar captura de logs nativos
+      // Start native log capture
       if (_nativeLogsEnabled) {

-// πŸ‘‡ AGREGAR MΓ‰TODO COMPLETO
+// Initialize native log capture
 void _initNativeLogsCapture() {

-          // Formatear log nativo con prefijo
+          // Format native log with prefix
           final timestamp = DateTime.now().toIso8601String();

           print('Error escribiendo log nativo: $e');
+           print('Error writing native log: $e');

           print('❌ Error en stream de logs nativos: $error');
+           print('❌ Error in native log stream: $error');

-      log('πŸ”§ Captura de logs nativos iniciada');
+      log('πŸ”§ Native log capture started');

-      print('❌ Error iniciando captura de logs nativos: $e');
+      print('❌ Error starting native log capture: $e');

-  // πŸ‘‡ AGREGAR MΓ‰TODO NUEVO
+  // Get native logs enabled state
   Future<bool> isNativeLogsEnabled() async {

-  // πŸ‘‡ AGREGAR MΓ‰TODO NUEVO
+  // Set native logs enabled state
   Future<void> setNativeLogsEnabled(bool enabled) async {

-      log('πŸ”§ Captura de logs nativos detenida');
+      log('πŸ”§ Native log capture stopped');

-      // πŸ‘‡ AGREGAR: Limpiar servicio de logs nativos
+      // Clean up native log service
       await _nativeSubscription?.cancel();

As per coding guidelines.

🧹 Nitpick comments (2)
android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (1)

76-80: Improve process termination reliability.

Calling destroy() alone may not immediately terminate the process. Consider using destroyForcibly() as a fallback to ensure cleanup.

Apply this enhancement:

  private fun stopLogCapture() {
      isCapturing = false
-     logcatProcess?.destroy()
+     logcatProcess?.let { process ->
+         process.destroy()
+         // Force kill if still alive after brief wait
+         Thread {
+             try {
+                 if (!process.waitFor(200, java.util.concurrent.TimeUnit.MILLISECONDS)) {
+                     process.destroyForcibly()
+                 }
+             } catch (e: Exception) {
+                 Log.w("MostroLogCapture", "Process cleanup interrupted", e)
+             }
+         }.start()
+     }
      logcatProcess = null
  }
lib/features/logs/logs_screen.dart (1)

15-15: Extract hardcoded color to AppTheme constant.

The native log color 0xFFFF9800 is hardcoded. For consistency and maintainability, consider adding it to AppTheme.

Add to AppTheme:

// In lib/core/app_theme.dart
static const Color statusNative = Color(0xFFFF9800);

Then use it here:

    if (line.contains('[NATIVE]')) {
-     return const Color(0xFFFF9800); // Naranja para nativos
+     return AppTheme.statusNative;
    }
πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between a457c3c and 5a96ec8.

πŸ“’ Files selected for processing (7)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (1 hunks)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
  • lib/features/logs/native_log_service.dart (1 hunks)
  • lib/main.dart (3 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (5)
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/native_log_service.dart
  • lib/features/logs/logs_service.dart
  • lib/features/logs/logs_screen.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/features/logs/logs_provider.dart
  • lib/features/logs/native_log_service.dart
  • lib/features/logs/logs_service.dart
  • lib/main.dart
  • lib/features/logs/logs_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/features/logs/logs_provider.dart
  • lib/features/logs/native_log_service.dart
  • lib/features/logs/logs_service.dart
  • lib/main.dart
  • lib/features/logs/logs_screen.dart
lib/main.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Configure timeago locales and app initialization in main.dart

Files:

  • lib/main.dart
{android,ios,web}/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Keep platform wrapper code in android/, ios/, and web/

Files:

  • android/app/src/main/AndroidManifest.xml
  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt
🧠 Learnings (18)
πŸ“š 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/logs/logs_provider.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/logs/logs_provider.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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs_provider.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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • lib/main.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/logs/logs_provider.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:

  • lib/main.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/main.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 : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text

Applied to files:

  • lib/main.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: All changes should pass flutter analyze before commit and follow conventional commits; avoid AI references in commit messages

Applied to files:

  • lib/main.dart
  • android/app/src/main/AndroidManifest.xml
πŸ“š 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/main.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/main.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/main.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:

  • lib/main.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/main.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/subscriptions/subscription_manager.dart : SubscriptionManager manages subscriptions, including relay list stream; ensure proper initialization and cleanup

Applied to files:

  • lib/main.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 pubspec.yaml : Keep flutter_launcher_icons and dependencies configuration updated in pubspec.yaml

Applied to files:

  • android/app/src/main/AndroidManifest.xml
πŸ“š 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/**/screens/**/*.dart : Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)

Applied to files:

  • lib/features/logs/logs_screen.dart

@Delgado74
Copy link
Author

DISCULPEN. LA CORRECCION DE CODERABBIT LAS REALIZARE SIN FALTA. PERO ME URGE TENER EL FEEDBACK SOBRE LA VISUALIZACION DE LOS LOGS. Y APLIQUE UNA SUGERENCIA QUE ME DIO @grunch PARA COLOCAR LOS LOGS EN SETTINGS Y NO EN EL DRAWER, PERO NO LE HICE COMMIT, DISCULPEN MI TORPEZA.

@Delagado74 and others added 2 commits November 3, 2025 03:09
- Translate all Spanish comments and log messages to English across codebase
- Enforce English-only coding guidelines in main.dart and logs_service.dart
- Improve Android logcat process termination with destroyForcibly() fallback
- Add 200ms timeout handling for graceful process cleanup
- Extract native log color (0xFFFF9800) to AppTheme.statusNative constant
- Integrate logs subtitle in settings screen using localized strings
- Replace hardcoded 'Logs' title with S.of(context)!.logsMenuTitle
- Add logsMenuSubtitle to UI for better context and user experience

Addresses CodeRabbit feedback on language consistency, process reliability,
and theme maintainability."
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (8)
android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (2)

42-45: Remove logcat -c; it’s unnecessary and may fail on user devices.

Clearing the global buffer offers no benefit and can silently fail; keep capture scoped by --pid only.

-                // Clear previous logcat
-                Runtime.getRuntime().exec("logcat -c").waitFor()

39-41: Small robustness polish (optional).

  • Name the thread and redirect error stream for completeness; reduces silent failures.
-        Thread {
+        Thread({
           var reader: BufferedReader? = null
           try {
@@
-                reader = BufferedReader(InputStreamReader(logcatProcess?.inputStream))
+                reader = BufferedReader(
+                    InputStreamReader(
+                        logcatProcess?.apply {
+                            // Ensure stderr is merged to stdout
+                        }?.inputStream
+                    )
+                )
@@
-        }.start()
+        }, "mostro-logcat").start()

Also applies to: 54-66, 75-83

android/app/src/main/AndroidManifest.xml (2)

31-36: Don’t use autoVerify on custom schemes.

android:autoVerify is only effective for https/http app links. Remove it to avoid misleading expectations.

-            <intent-filter android:autoVerify="true">
+            <intent-filter>

39-45: Confirm need for android:permission on BackgroundService.

Requiring android.permission.FOREGROUND_SERVICE to start/bind the service is unusual and can block external starts; typically not set here. If the plugin doesn’t require it, remove.

-        <service
+        <service
             android:name="id.flutter.flutter_background_service.BackgroundService"
             android:exported="false"
             android:foregroundServiceType="dataSync"
-            android:permission="android.permission.FOREGROUND_SERVICE"
             tools:replace="android:exported" />
lib/features/settings/settings_screen.dart (2)

46-53: Move ref.listen out of build to avoid repeated subscriptions.

Register the listener once in initState and remove from build.

@@
   void initState() {
     super.initState();
     final settings = ref.read(settingsProvider);
     _mostroTextController = TextEditingController(text: settings.mostroPublicKey);
     _lightningAddressController = TextEditingController(text: settings.defaultLightningAddress ?? '');
+    ref.listen<Settings>(settingsProvider, (previous, next) {
+      if (previous?.defaultLightningAddress != next.defaultLightningAddress) {
+        final newText = next.defaultLightningAddress ?? '';
+        if (_lightningAddressController.text != newText) {
+          _lightningAddressController.text = newText;
+        }
+      }
+    });
   }
@@
-    // Listen to settings changes and update controllers
-    ref.listen<Settings>(settingsProvider, (previous, next) {
-      if (previous?.defaultLightningAddress != next.defaultLightningAddress) {
-        final newText = next.defaultLightningAddress ?? '';
-        if (_lightningAddressController.text != newText) {
-          _lightningAddressController.text = newText;
-        }
-      }
-    });

575-577: Use ref.read in callbacks; avoid watch to prevent unnecessary rebuilds.

-                onChanged: (value) => ref
-                    .watch(settingsProvider.notifier)
-                    .updateMostroInstance(value),
+                onChanged: (value) => ref
+                    .read(settingsProvider.notifier)
+                    .updateMostroInstance(value),
lib/features/logs/logs_screen.dart (2)

125-126: Prefer Color.withValues for consistency with modern API.

-              color: AppTheme.backgroundCard.withAlpha(180),
+              color: AppTheme.backgroundCard.withValues(alpha: 180/255),

46-49: Avoid copying the entire log list on each build (optional).

reversed.toList() allocates on every rebuild. Consider storing logs in reverse order in state, or using ListView.builder with itemBuilder indexing from the end without copying.

Also applies to: 110-161

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 5a96ec8 and 1428f1d.

πŸ“’ Files selected for processing (12)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (1 hunks)
  • lib/core/app_theme.dart (1 hunks)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
  • lib/features/logs/native_log_service.dart (1 hunks)
  • lib/features/settings/settings_screen.dart (3 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/main.dart (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/features/logs/native_log_service.dart
🧰 Additional context used
πŸ““ Path-based instructions (7)
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/logs/logs_screen.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.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/features/logs/logs_screen.dart
  • lib/core/app_theme.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/main.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/features/logs/logs_screen.dart
  • lib/core/app_theme.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/main.dart
lib/core/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Keep dependency wiring in lib/core/

Files:

  • lib/core/app_theme.dart
{android,ios,web}/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Keep platform wrapper code in android/, ios/, and web/

Files:

  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt
  • android/app/src/main/AndroidManifest.xml
lib/l10n/*.arb

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

lib/l10n/*.arb: Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)
When adding a new localization key, update all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
After modifying ARB files, run dart run build_runner build -d to regenerate localization

Files:

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

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Configure timeago locales and app initialization in main.dart

Files:

  • lib/main.dart
🧠 Learnings (28)
πŸ“š 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 : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text

Applied to files:

  • lib/features/logs/logs_screen.dart
  • lib/features/settings/settings_screen.dart
  • lib/main.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/**/screens/**/*.dart : Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)

Applied to files:

  • lib/features/logs/logs_screen.dart
  • lib/features/settings/settings_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/logs/logs_screen.dart
  • lib/features/settings/settings_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/core/app_theme.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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.dart
  • lib/main.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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.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/l10n/*.arb : When adding a new localization key, update all three ARB files (en, es, it)

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
πŸ“š 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/l10n/*.arb : Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
πŸ“š 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/l10n/*.arb : Use proper ARB metadata for parameterized strings

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
πŸ“š 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/l10n/*.arb : After modifying ARB files, run dart run build_runner build -d to regenerate localization

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/l10n/intl_es.arb
πŸ“š 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: All changes should pass flutter analyze before commit and follow conventional commits; avoid AI references in commit messages

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/main.dart
  • android/app/src/main/AndroidManifest.xml
  • lib/l10n/intl_es.arb
πŸ“š 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: Run dart run build_runner build -d when ARB or annotated files change to regenerate localization, mocks, and models

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_es.arb
πŸ“š 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/features/settings/settings_screen.dart
  • lib/main.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/settings/settings_screen.dart
  • lib/main.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/settings/settings.dart : Settings model must include blacklistedRelays and maintain backward-compatible serialization

Applied to files:

  • lib/features/settings/settings_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/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/main.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/logs/logs_provider.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/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, 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/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/logs/logs_provider.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/logs/logs_provider.dart
  • lib/main.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/logs/logs_provider.dart
  • lib/main.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/services/nostr_service.dart : All Nostr protocol interactions must go through NostrService (services/nostr_service.dart)

Applied to files:

  • lib/features/logs/logs_provider.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/main.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:

  • lib/main.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/subscriptions/subscription_manager.dart : SubscriptionManager manages subscriptions, including relay list stream; ensure proper initialization and cleanup

Applied to files:

  • lib/main.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 pubspec.yaml : Keep flutter_launcher_icons and dependencies configuration updated in pubspec.yaml

Applied to files:

  • android/app/src/main/AndroidManifest.xml
πŸ“š 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:

  • android/app/src/main/AndroidManifest.xml
πŸͺ› RuboCop (1.81.6)
lib/l10n/intl_en.arb

[warning] 1162-1162: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

lib/l10n/intl_it.arb

[warning] 1195-1195: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

lib/l10n/intl_es.arb

[warning] 1141-1141: Duplicated key in hash literal.

(Lint/DuplicateHashKey)

πŸ”‡ Additional comments (1)
lib/core/app_theme.dart (1)

67-67: LGTM: adds statusNative color used for native logs.

…alization cleanup

- android/MainActivity.kt:
  β€’ Removed unnecessary `logcat -c` to prevent failures on user devices.
  β€’ Added thread name and ensured stderr redirection for more reliable log capture.
  β€’ Marked `isCapturing` as @volatile / AtomicBoolean for thread safety.

- android/AndroidManifest.xml:
  β€’ Removed android:autoVerify from custom scheme intent-filter.
  β€’ Reviewed service permission; removed redundant FOREGROUND_SERVICE requirement.

- lib/features/settings/settings_screen.dart:
  β€’ Moved ref.listen to initState to avoid repeated subscriptions.
  β€’ Used ref.read instead of ref.watch inside callbacks to prevent rebuilds.

- lib/features/logs/logs_screen.dart:
  β€’ Updated deprecated withAlpha() β†’ withValues().
  β€’ Suggested optimization: avoid reversed.toList() allocations on every rebuild.

- lib/features/logs/logs_provider.dart & logs_service.dart:
  β€’ Made LogsService reactive (ChangeNotifier / ValueNotifier) to notify UI updates.
  β€’ Updated provider type accordingly.

- lib/l10n/intl_{en,es,it}.arb:
  β€’ Removed duplicate "errorSharingLogs" entries.
  β€’ Regenerated localization artifacts with `dart run build_runner build -d`.

- main.dart:
  β€’ Improved global error handling: log errors but preserve original propagation.

Overall: improves reliability of log capture, reactivity of logs screen,
and ensures localization consistency across all supported languages.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
lib/features/logs/logs_provider.dart (1)

95-111: Async initialization in constructor can cause flicker.

Same issue as LogsEnabledNotifier: Line 100 calls async _loadState() without await, potentially causing UI flicker when the persisted state differs from the initial true value.

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 1428f1d and 8eeda0b.

πŸ“’ Files selected for processing (12)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt (1 hunks)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
  • lib/features/settings/settings_screen.dart (8 hunks)
  • lib/l10n/intl_en.arb (1 hunks)
  • lib/l10n/intl_es.arb (1 hunks)
  • lib/l10n/intl_it.arb (1 hunks)
  • lib/main.dart (3 hunks)
  • test/features/logs/logs_screen_test.dart (1 hunks)
  • test/mocks.mocks.dart (36 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • test/features/logs/logs_screen_test.dart
  • lib/features/logs/logs_screen.dart
  • android/app/src/main/kotlin/network/mostro/app/MainActivity.kt
  • lib/l10n/intl_es.arb
🧰 Additional context used
πŸ““ Path-based instructions (10)
lib/l10n/*.arb

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

lib/l10n/*.arb: Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)
When adding a new localization key, update all three ARB files (en, es, it)
Use proper ARB metadata for parameterized strings
After modifying ARB files, run dart run build_runner build -d to regenerate localization

Files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
**/*.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/main.dart
  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • test/mocks.mocks.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/main.dart
  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
lib/main.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Configure timeago locales and app initialization in main.dart

Files:

  • lib/main.dart
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
{android,ios,web}/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Keep platform wrapper code in android/, ios/, and web/

Files:

  • android/app/src/main/AndroidManifest.xml
test/**/*.dart

πŸ“„ CodeRabbit inference engine (AGENTS.md)

test/**/*.dart: Mirror feature layout for tests under test/
Prefer Mockito fakes generated via build_runner in tests

test/**/*.dart: Unit tests belong in test/; integration tests in integration_test/
Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes

Files:

  • test/mocks.mocks.dart
test/mocks.mocks.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Do not manually edit test/mocks.mocks.dart (Mockito-generated)

Files:

  • test/mocks.mocks.dart
**/*.mocks.dart

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Do not manually edit any *.mocks.dart generated Mockito files

Files:

  • test/mocks.mocks.dart
**/*.{g.dart,mocks.dart}

πŸ“„ CodeRabbit inference engine (CLAUDE.md)

Never add per-item ignore comments to generated files; rely on file-level ignores or regenerate

Files:

  • test/mocks.mocks.dart
🧠 Learnings (40)
πŸ“š 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/l10n/*.arb : When adding a new localization key, update all three ARB files (en, es, it)

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
πŸ“š 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/l10n/*.arb : Maintain ARB files for en/es/it in lib/l10n/ (intl_en.arb, intl_es.arb, intl_it.arb)

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
πŸ“š 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/l10n/*.arb : Use proper ARB metadata for parameterized strings

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
πŸ“š 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/l10n/*.arb : After modifying ARB files, run dart run build_runner build -d to regenerate localization

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
πŸ“š 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: All changes should pass flutter analyze before commit and follow conventional commits; avoid AI references in commit messages

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/main.dart
  • android/app/src/main/AndroidManifest.xml
πŸ“š 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: Run dart run build_runner build -d when ARB or annotated files change to regenerate localization, mocks, and models

Applied to files:

  • lib/l10n/intl_en.arb
πŸ“š 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 : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/l10n/intl_it.arb
  • lib/main.dart
  • lib/features/settings/settings_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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/l10n/intl_en.arb
  • lib/main.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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:

  • lib/main.dart
  • lib/features/settings/settings_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 : Target zero flutter analyze issues

Applied to files:

  • lib/main.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/main.dart
  • lib/features/settings/settings_screen.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/main.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • test/mocks.mocks.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/main.dart
  • lib/features/logs/logs_provider.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/main.dart
  • lib/features/settings/settings_screen.dart
  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • test/mocks.mocks.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:

  • lib/main.dart
  • test/mocks.mocks.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/subscriptions/subscription_manager.dart : SubscriptionManager manages subscriptions, including relay list stream; ensure proper initialization and cleanup

Applied to files:

  • lib/main.dart
  • test/mocks.mocks.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/settings/settings.dart : Settings model must include blacklistedRelays and maintain backward-compatible serialization

Applied to files:

  • lib/features/settings/settings_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/**/screens/**/*.dart : Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)

Applied to files:

  • lib/features/settings/settings_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/settings/settings_screen.dart
  • lib/features/logs/logs_provider.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/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs/logs_provider.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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • test/mocks.mocks.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/logs_provider.dart
  • test/mocks.mocks.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/logs/logs_provider.dart
  • test/mocks.mocks.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/logs/logs_provider.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 pubspec.yaml : Keep flutter_launcher_icons and dependencies configuration updated in pubspec.yaml

Applied to files:

  • android/app/src/main/AndroidManifest.xml
πŸ“š 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:

  • android/app/src/main/AndroidManifest.xml
πŸ“š 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:

  • test/mocks.mocks.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/**/*.dart : Mocks are generated via Mockito; regenerate with dart run build_runner build -d after mock changes

Applied to files:

  • test/mocks.mocks.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 **/*.mocks.dart : Do not manually edit any *.mocks.dart generated Mockito files

Applied to files:

  • test/mocks.mocks.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 test/**/*.dart : Prefer Mockito fakes generated via build_runner in tests

Applied to files:

  • test/mocks.mocks.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:

  • test/mocks.mocks.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/services/nostr_service.dart : All Nostr protocol interactions must go through NostrService (services/nostr_service.dart)

Applied to files:

  • test/mocks.mocks.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 test/**/*.dart : Mirror feature layout for tests under test/

Applied to files:

  • test/mocks.mocks.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 integration_test/**/*.dart : Ensure integration coverage for flows involving networking, background work, or session recovery

Applied to files:

  • test/mocks.mocks.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:

  • test/mocks.mocks.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:

  • test/mocks.mocks.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:

  • test/mocks.mocks.dart
πŸ“š Learning: 2025-05-08T16:31:29.582Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.

Applied to files:

  • test/mocks.mocks.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/relay.dart : Relay model must track source (user, mostro, defaultConfig) and expose canDelete/canBlacklist semantics

Applied to files:

  • test/mocks.mocks.dart
πŸ”‡ Additional comments (4)
lib/features/logs/logs_provider.dart (3)

7-17: LGTM! Proper reactive provider with lifecycle management.

The ChangeNotifierProvider correctly addresses previous feedback about reactivity. The service disposal is properly wired via ref.onDispose().


19-23: LGTM! Reactive logs access.

Correctly watches the ChangeNotifier service, ensuring UI rebuilds when logs change.


50-76: Review comment is based on incorrect architectural assumptions and proposes an invalid pattern for Riverpod StateNotifier.

The async call in the constructor is not a race conditionβ€”this is the standard StateNotifier initialization pattern used throughout the codebase (see LogsEnabledNotifier, NativeLogsEnabledNotifier, FirstRunNotifier). State is safely initialized via super([]) before the async method runs.

The manual _loadLogs() calls are necessary because the ref.listen callback (lines 31–33) is empty and never syncs state. The real issue is the incomplete listener implementation, not redundant updates.

More critically, the suggested pattern (using addListener/removeListener) cannot work in a StateNotifier context. StateNotifier is not a ChangeNotifier and doesn't expose those methods. The proposed fix violates Riverpod's architecture and would not compile.

The current manual syncing, while not ideal, is the correct approach given the empty ref.listen callback. If state synchronization should be automatic, the empty listener callback should be populated instead.

Likely an incorrect or invalid review comment.

test/mocks.mocks.dart (1)

1-3235: LGTM! Mockito-generated mock properly includes LogsService.

This file is auto-generated by Mockito (as indicated by the header). The new MockLogsService class and supporting fakes (_FakeUnmodifiableListView_19) are correctly generated to support testing the logs feature. No manual edits should be made to this file.

Based on learnings.

- Removed unnecessary empty ref.listen in logs_provider.dart (lines 31-33)
  since LogsNotifier already receives the service via its constructor and
  the ChangeNotifier is reactive by itself.
- Fixed async _loadState() in LogsEnabledNotifier to prevent UI flicker.
  Introduced a loading state so the initial value represents "unknown" and
  the UI renders a placeholder until storage returns the actual value.
- Corrected dispose() signature in LogsService to match ChangeNotifier
  (void, not Future<void>). Any async cleanup is now handled without breaking
  the synchronous override.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
lib/features/logs/logs_provider.dart (2)

13-13: Update outdated comment.

The comment states "Providers remain the same" but the provider was changed from a plain Provider to ChangeNotifierProvider to fix reactivity issues.

Apply this diff:

-// Providers remain the same
+// Main service provider (reactive via ChangeNotifier)
 final logsServiceProvider = ChangeNotifierProvider<LogsService>((ref) {

27-70: Consider removing LogsNotifier to eliminate duplication.

LogsNotifier maintains a separate copy of logs in its state that duplicates LogsService.logs. Since LogsService now extends ChangeNotifier and logsServiceProvider is a ChangeNotifierProvider, consumers can watch logsProvider (lines 22-25) directly for reactive updates. The LogsNotifier adds overhead by manually syncing its state copy via _loadLogs().

If LogsScreen or other consumers only need read access to logs, logsProvider alone is sufficient. Reserve StateNotifier patterns for state that requires transformation or complex business logic beyond what the service provides.

If you need to keep LogsNotifier for methods like clearLogs and getLogFile, consider whether those could be accessed directly from logsServiceProvider.notifier instead, eliminating the intermediate notifier layer.

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 8eeda0b and 7475e7c.

πŸ“’ Files selected for processing (3)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
  • lib/features/settings/settings_screen.dart (8 hunks)
🧰 Additional context used
πŸ““ Path-based instructions (3)
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_screen.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/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_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/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_screen.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/features/*/providers/**/*.dart : Organize Riverpod providers by feature under features/{feature}/providers/

Applied to files:

  • lib/features/logs/logs_provider.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/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, 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/logs_provider.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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs_provider.dart
  • lib/features/logs/logs_service.dart
  • lib/features/settings/settings_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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs/logs_provider.dart
  • lib/features/settings/settings_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/logs/logs_provider.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/logs/logs_service.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/settings/settings.dart : Settings model must include blacklistedRelays and maintain backward-compatible serialization

Applied to files:

  • lib/features/settings/settings_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 : Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text

Applied to files:

  • lib/features/settings/settings_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/**/screens/**/*.dart : Pass BuildContext to methods that need localization (S.of(context)!.keyName usage)

Applied to files:

  • lib/features/settings/settings_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:

  • lib/features/settings/settings_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/settings/settings_screen.dart
πŸ”‡ Additional comments (7)
lib/features/logs/logs_provider.dart (1)

72-116: Good implementation of loading state pattern.

Both LogsEnabledNotifier and NativeLogsEnabledNotifier properly use the LogsState enum to handle asynchronous initialization, avoiding the UI flicker issue from previous reviews. The toggle() methods correctly transition through loading state during async operations.

lib/features/settings/settings_screen.dart (2)

45-56: Listener guard pattern is acceptable here.

Using _hasSetupListener to prevent duplicate ref.listen subscriptions inside build() is a known Riverpod workaround for ensuring one-time listener setup. While this pattern isn't ideal (typically you'd rely on Riverpod's built-in deduplication or use different lifecycle hooks), it's acceptable for this specific use case of synchronizing the lightning address controller with settings changes.


400-516: Excellent implementation of the Logs settings card.

The _buildLogsCard method follows best practices:

  • Properly watches logsEnabledProvider for reactive updates
  • Uses read() for the notifier to avoid unnecessary rebuilds during toggle operations
  • Handles loading state with a spinner
  • All user-facing strings are properly localized via S.of(context)!
  • UI structure and styling match the existing settings cards
  • Clean navigation to the logs screen
lib/features/logs/logs_service.dart (4)

35-79: Robust initialization with proper guards and error handling.

The init() method properly:

  • Guards against re-initialization and disposed state
  • Limits loaded logs to _maxLogLines to prevent memory bloat
  • Notifies listeners after loading existing logs so UI updates
  • Sets _initialized = true only after successful setup
  • Handles errors with try/catch and rethrow

81-119: Native log capture properly implemented.

The _initNativeLogsCapture() method correctly:

  • Guards against disposed state
  • Formats native logs with [NATIVE] prefix for distinction
  • Maintains memory limits via _maxLogLines
  • Handles stream errors gracefully
  • Notifies listeners for UI updates

Note: Removing the first element (line 98) is O(n) for List. If native logs are extremely high-frequency, consider using a Queue for O(1) removal. For the current _maxLogLines of 5000, the performance impact is negligible.


145-163: Clean state management with proper subscription lifecycle.

The setNativeLogsEnabled() method correctly:

  • Guards enabling by checking _nativeSubscription == null (line 153)
  • Cancels existing subscription when disabling (line 156)
  • Persists state to SharedPreferences
  • Notifies listeners for UI updates

165-256: Logging and file management properly implemented.

The core logging methods correctly:

  • Guard against uninitialized/disposed/disabled states
  • Maintain memory limits to prevent bloat
  • Handle file I/O errors gracefully with try/catch
  • Flush before reading in getLogFile() to ensure data consistency
  • Close and reopen sink in clearLogs() to avoid corruption
  • Clean ANSI codes and control characters for sharing

All previous feedback regarding unsafe truncation and resource management has been properly addressed.

…ation

- Remove LogsNotifier class that duplicated LogsService.logs state
- Update LogsScreen to access LogsService directly via logsServiceProvider
- Fix async disposal in LogsService by wrapping sink.close() with unawaited()
- Update provider comments to reflect ChangeNotifier reactivity

Benefits:
- Eliminates unnecessary state duplication
- Reduces code complexity and maintenance overhead
- Improves performance by removing intermediate notifier layer
- LogsService ChangeNotifier provides direct reactive updates

Breaking changes: None - consumers now use logsServiceProvider directly

Addresses CodeRabbit suggestions for better architecture
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 7475e7c and 08a6184.

πŸ“’ Files selected for processing (3)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/features/logs/logs_screen.dart
🧰 Additional context used
πŸ““ Path-based instructions (3)
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.dart
🧠 Learnings (9)
πŸ“š 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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs/logs_provider.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/logs/logs_provider.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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/features/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs/logs_provider.dart
  • lib/features/logs/logs_service.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/logs_provider.dart
  • lib/features/logs/logs_service.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/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/logs/logs_provider.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/logs/logs_service.dart
πŸ”‡ Additional comments (2)
lib/features/logs/logs_service.dart (1)

34-79: LGTM! Initialization, logging, cleanup, and disposal are well-structured.

The service correctly:

  • Loads persisted logs and notifies listeners after initialization (lines 59-60)
  • Sets _initialized = true only after setup completes (line 72)
  • Caps in-memory logs at _maxLogLines to prevent memory bloat (lines 97-99, 175-177)
  • Calls notifyListeners() after state changes (lines 60, 108, 142, 162, 186, 221)
  • Properly guards operations with _initialized and _isDisposed checks
  • Cleans up resources in dispose() using unawaited() for async operations (lines 275, 279-280) to match ChangeNotifier's synchronous signature
  • Safely reopens the sink after clearing logs (lines 216-218)

The implementation addresses all critical issues from past reviews.

Also applies to: 165-187, 199-225, 269-289

lib/features/logs/logs_provider.dart (1)

47-49: Review comment verified: Loading state handling is properly implemented.

The logsEnabledProvider is properly consumed in settings_screen.dart (line 448-456) with explicit loading state handling: a loading spinner is shown when logsState == LogsState.loading, and a switch is shown when the state transitions to enabled/disabled.

The nativeLogsEnabledProvider has no consumers in the codebase, so it has no UI impact. The async initialization pattern in both notifiers' constructors is acceptable and follows Riverpod's StateNotifier lifecycleβ€”the providers start with LogsState.loading and transition to the actual state once _loadState() completes.

- Translated Spanish comments to English in `logs_provider.dart` (lines 27 and 41).
- Removed unnecessary Spanish comments in `logs_screen.dart`.
- Applied CodeRabbit suggestion in `logs_service.dart`:
  replaced `log()` call at line 115 with `print('πŸ”§ Native logs capture started')`
  so the initialization message is properly displayed.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

πŸ“œ Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 08a6184 and 4bcc887.

πŸ“’ Files selected for processing (3)
  • lib/features/logs/logs_provider.dart (1 hunks)
  • lib/features/logs/logs_screen.dart (1 hunks)
  • lib/features/logs/logs_service.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/features/logs/logs_screen.dart
🧰 Additional context used
πŸ““ Path-based instructions (3)
lib/features/**

πŸ“„ CodeRabbit inference engine (AGENTS.md)

Place application feature code under lib/features//, grouped by domain

Files:

  • lib/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.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/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.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/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.dart
🧠 Learnings (11)
πŸ“š 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 clear, concise English for variable names, function names, and code comments; all comments must be in English

Applied to files:

  • lib/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.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/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)

Applied to files:

  • lib/features/logs/logs_service.dart
  • lib/features/logs/logs_provider.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/logs/logs_service.dart
  • lib/features/logs/logs_provider.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/logs_service.dart
  • lib/features/logs/logs_provider.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/logs/logs_service.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/logs/logs_provider.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/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, 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/logs_provider.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: All changes should pass flutter analyze before commit and follow conventional commits; avoid AI references in commit messages

Applied to files:

  • lib/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/logs/logs_provider.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/logs/logs_provider.dart

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

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

Hola @Delgado74 los comentarios deben estar en inglΓ©s, no en espaΓ±ol, se deben traducir.
Se ven mejor los logs como los pusiste en la pantalla de settings pero aΓΊn no funcionan, en esta captura de pantalla se muestran errores que no son los logs de la app sino de la implementaciΓ³n de los logs

image

Copy link
Member

@Catrya Catrya left a comment

Choose a reason for hiding this comment

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

@Delagado74 revisa porfa esto. Hay fallas de seguridad que pueden permitir capturar logs de otras apps con datos sensibles como contraseΓ±as, tokens, info privada etc. El filtrado por PID puede no funcionar correctamente en muchos dispositivos Android y exponer informaciΓ³n que no nos corresponde. Es necesario implementar un filtrado adecuado que solo capture logs de la app de mostro.

Comment on lines 44 to 51
// Capturar logs solo de esta app con timestamp
logcatProcess = Runtime.getRuntime().exec(
arrayOf(
"logcat",
"-v", "time",
"--pid=${android.os.Process.myPid()}"
)
)
Copy link
Member

Choose a reason for hiding this comment

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

AquΓ­ introduces una falla de seguridad porque el flag --pid no funciona en Android < 7.0 y es inconsistente en versiones posteriores. En muchos dispositivos esto captura logs de TODAS las aplicaciones del sistema, incluyendo WhatsApp, bancos, etc. No debe estar capturando logcat sin filtrar sino usar tags especΓ­ficos para la app de mostro

Comment on lines 13 to 18
_nativeLogStream = _logcatStream
.receiveBroadcastStream()
.map((event) => event.toString())
.handleError((error) {
print('❌ Error en stream de logcat nativo: $error');
}).where((log) => log.isNotEmpty && log.trim().isNotEmpty);
Copy link
Member

Choose a reason for hiding this comment

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

AquΓ­ estΓ‘s aceptando cualquier log que venga del canal nativo sin validaciΓ³n. El .where() solo elimina lΓ­neas vacΓ­as pero acepta logs de otras apps con datos sensibles. No debe estar aceptando todo sino filtrar por contenido especΓ­fico de la app de mostro

Comment on lines +74 to +76
final line = '[$timestamp] [NATIVE] $nativeLog';

_logs.add(line);
Copy link
Member

Choose a reason for hiding this comment

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

AquΓ­ tambiΓ©n hay falla de seguridad porque se estΓ‘ almacenando directamente cualquier contenido sin sanitizaciΓ³n. Esto puede persistir tokens, contraseΓ±as y datos privados de otras aplicaciones que luego se exportan cuando el usuario comparte los logs. No debe estar guardando todo sin filtrar sino validar que el contenido sea seguro antes de almacenarlo.

arrayOf(
"logcat",
"-v", "time",
"--pid=${android.os.Process.myPid()}"
Copy link
Member

Choose a reason for hiding this comment

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

en el comentario del commit especΓ­fico ya quedΓ³ outdated pero aquΓ­ estΓ‘, lo pego de nuevo por si acaso:

AquΓ­ introduces una falla de seguridad porque el flag --pid no funciona en Android < 7.0 y es inconsistente en versiones posteriores. En muchos dispositivos esto captura logs de todas las aplicaciones del sistema, incluyendo WhatsApp, bancos, etc. No debe estar capturando logcat sin filtrar sino usar tags especΓ­ficos para la app de mostro

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

Este PR modica y crea archivos que no deberΓ­an modificarse, debemos atomizar los cambios

@@ -1,73 +1,79 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools">
Copy link
Member

Choose a reason for hiding this comment

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

@Delgado74 por quΓ© es necesario cambiar el manifest? me puedes comentar la ventaja de realizar estos cambios?

Copy link
Author

Choose a reason for hiding this comment

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

Fue un error de concepto a la hora de implementar el PR. Incluso hay muchas modificaciones. AsΓ­ que sobre la base de las recomendaciones del equipo estoy trabajando ahora en una soluciΓ³n que solo introduzca 3 archivos nuevos y solo modifica 3 archivos. Creando un PR mΓ‘s sencillo. Gracias por la oportunidad

import 'package:mostro_mobile/shared/utils/biometrics_helper.dart';

final biometricsHelperProvider = Provider<BiometricsHelper>((ref) {
throw UnimplementedError();
Copy link
Member

Choose a reason for hiding this comment

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

por quΓ© crear este Helper "unimplemented" ? que tiene que ver con los logs?

Copy link
Author

Choose a reason for hiding this comment

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

Innecesario. SerΓ‘ eliminado en el trabajo que estoy haciendo. Gracias

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants