Skip to content

add technical documentation#284

Merged
grunch merged 1 commit into
mainfrom
add-documentation
Aug 28, 2025
Merged

add technical documentation#284
grunch merged 1 commit into
mainfrom
add-documentation

Conversation

@Catrya

@Catrya Catrya commented Aug 28, 2025

Copy link
Copy Markdown
Member

added:

  • APP_INITIALIZATION_ANALYSIS.md
  • ARCHITECTURE.md
  • RELAY_SYNC_IMPLEMENTATION.md
  • TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md

Summary by CodeRabbit

  • Documentation
    • Added comprehensive architecture documentation covering app initialization flow, communication channels, state management, security, data storage, and background services.
    • Introduced a technical reference for relay synchronization, including discovery, validation, blacklist strategy, persistence, and resilience patterns.
    • Documented timeout detection and session cleanup, with real-time monitoring, recovery mechanisms, and UI countdown integration.
    • Added documentation standards and update processes to ensure consistency, testing guidance, and long-term maintainability.
    • Clarified performance optimizations, internationalization support, and future architecture considerations.

added:
- APP_INITIALIZATION_ANALYSIS.md
- ARCHITECTURE.md
- RELAY_SYNC_IMPLEMENTATION.md
- TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
@coderabbitai

coderabbitai Bot commented Aug 28, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adds four new architecture documents detailing app initialization, overall architecture, relay synchronization, and timeout/session cleanup. No code or public API changes. All updates are documentation-only.

Changes

Cohort / File(s) Summary
Architecture Overview Docs
docs/architecture/ARCHITECTURE.md, docs/architecture/README.md
Introduces a comprehensive architecture overview (modules, state management, data/security layers, background services) and a README describing documentation standards and processes.
System Deep-Dives
docs/architecture/APP_INITIALIZATION_ANALYSIS.md, docs/architecture/RELAY_SYNC_IMPLEMENTATION.md, docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md
Adds detailed references on app initialization flow (including SubscriptionManager listener behavior), relay synchronization mechanics, and timeout detection/session cleanup with synthetic event handling.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant App
  participant AppInit as AppInitializerProvider
  participant Nostr as NostrService
  participant Keys as KeyManager
  participant Subs as SubscriptionManager
  participant Session as SessionNotifier
  participant Orders as OrderNotifier

  User->>App: Launch
  App->>AppInit: Initialize()
  AppInit->>Keys: Load/derive keys
  AppInit->>Nostr: Connect relays
  AppInit->>Session: Load sessions / cleanup expired
  AppInit->>Subs: Setup private subscriptions\n(fireImmediately: false)
  AppInit->>Orders: Setup public subscriptions
  AppInit-->>App: Ready
  note over Subs,Orders: Dual-channel: Private (encrypted) / Public (orders)
Loading
sequenceDiagram
  autonumber
  participant Orders as OrderNotifier
  participant PublicStream as orderEventsProvider (public)
  participant Session as SessionNotifier
  participant Repo as OpenOrdersRepository
  participant Store as Storage

  Orders->>PublicStream: Subscribe to Kind 38383 events
  PublicStream-->>Orders: Order status update
  Orders->>Orders: _checkTimeoutAndCleanup()
  alt Maker-owned order
    Orders->>Store: Persist Synthetic Timeout Reversal
    Orders->>Session: Preserve session state
  else Taker or not user-created
    Orders->>Repo: Delete session/order data
    Session->>Session: Cleanup references
  end
  note over Orders,Store: Synthetic events recovered on restart
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

A rabbit taps keys with a whisker’s delight,
Mapping the flows from dawn’s cold start to night.
Relays in sync, sessions tidy and tight,
Synthetic echoes keep state in sight.
Thump-thump—docs bloom, architecture alight. 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-documentation

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

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

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

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

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

CodeRabbit Configuration File (.coderabbit.yaml)

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

Status, Documentation and Community

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 (12)
docs/architecture/README.md (2)

1-3: Minor grammar tweak for clarity.

“serve as reference” → “serve as a reference”.

-These documents serve as reference for understanding the system's design, implementation patterns, and core functionality.
+These documents serve as a reference for understanding the system's design, implementation patterns, and core functionality.

66-67: Add a Last Updated footer per your own standards.

Helps keep docs trustworthy.

 **Remember**: Documentation is code infrastructure. Treat it with the same care and attention as the application code itself. Well-maintained documentation accelerates development, reduces bugs, and enables confident architectural changes.
+
+---
+Last Updated: 2025-08-28
docs/architecture/APP_INITIALIZATION_ANALYSIS.md (3)

155-161: Add language identifiers to fenced code blocks (markdownlint MD040).

Use text for pseudo-logs and BEFORE/AFTER sequences.

-**BEFORE (Problematic - fireImmediately: true)**:
-```
+**BEFORE (Problematic - fireImmediately: true)**:
+```text
 ...
-**AFTER (Fixed - fireImmediately: false)**:
-```
+**AFTER (Fixed - fireImmediately: false)**:
+```text
 ...

Also applies to: 166-176, 179-189


352-358: Unify variable naming: mostroPubkey vs mostroPublicKey.

Use one name consistently across samples; this block uses mostroPublicKey while others use mostroPubkey.

-  return NostrFilter(
-    kinds: [10002], // Relay list events
-    authors: [mostroPublicKey], // From Mostro instance
-  );
+  return NostrFilter(
+    kinds: [10002], // Relay list events
+    authors: [mostroPubkey], // From Mostro instance
+  );

295-305: Add language to code fence (markdownlint MD040).

-```dart
+```dart
 NostrFilter? _createFilterForType(SubscriptionType type, List<Session> sessions) {
docs/architecture/RELAY_SYNC_IMPLEMENTATION.md (3)

60-84: Add language to ASCII diagram (markdownlint MD040).

-```
+```text
 ┌─────────────────┐    NIP-65 Events     ┌─────────────────┐
 ...
 └─────────────────┘                       └─────────────────┘

768-769: Translate non-English comment.

Keep docs monolingual (English).

-      // 🔥 RESET COMPLETO: Limpiar todos los relays y hacer sync fresco
+      // 🔥 COMPLETE RESET: Clear all relays and perform a fresh sync

1036-1056: Policy mismatch: user relays reset on instance change.

This code clears blacklist and user relays unconditionally; elsewhere the doc mentions “user relays survive instance changes (when configured)”. Clarify the intended policy or gate this with a setting.

-    state = state.copyWith(
-      mostroPublicKey: newValue,
-      blacklistedRelays: const [],
-      userRelays: const [],
-    );
+    state = state.copyWith(
+      mostroPublicKey: newValue,
+      blacklistedRelays: preserveUserRelaysOnInstanceChange ? state.blacklistedRelays : const [],
+      userRelays: preserveUserRelaysOnInstanceChange ? state.userRelays : const [],
+    );
docs/architecture/ARCHITECTURE.md (2)

20-66: Add language to directory tree (markdownlint MD040).

-```
+```text
 lib/
 ├── core/                    # Central configuration and FSM
 ...
 └── generated/             # Generated files (DO NOT EDIT)

71-77: Add language to feature structure tree (markdownlint MD040).

-```
+```text
 features/{feature}/
 ├── screens/              # UI screens for the feature
 ...
docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (2)

603-616: Remove trailing colons from headings (markdownlint MD026) and add language tags (MD040).

-### Why This "Trick" is Necessary
-```
+### Why This "Trick" is Necessary
+```text
 ...
-#### The Problem Without Synthetic Events:
-```
+#### The Problem Without Synthetic Events
+```text
 ...
-#### The Solution With Synthetic Events:
-```
+#### The Solution With Synthetic Events
+```text

189-199: Add language identifiers to fenced blocks (markdownlint MD040).

-```
+```text
 // Sequences and flow blocks in this section are plain text diagrams.

Apply to fenced blocks starting at lines 189, 541, 682, 693.

Also applies to: 541-551, 682-701, 693-701

Comment on lines +45 to +46
- Include file paths: `lib/features/order/notfiers/order_notifier.dart`
- Add line numbers for specific references: `// Line 45-67`

@coderabbitai coderabbitai Bot Aug 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix path typo: notfiers → notifiers.

The file path should be lib/features/order/notifiers/order_notifier.dart.

- - Include file paths: `lib/features/order/notfiers/order_notifier.dart`
+ - Include file paths: `lib/features/order/notifiers/order_notifier.dart`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Include file paths: `lib/features/order/notfiers/order_notifier.dart`
- Add line numbers for specific references: `// Line 45-67`
- Include file paths: `lib/features/order/notifiers/order_notifier.dart`
- Add line numbers for specific references: `// Line 45-67`
🤖 Prompt for AI Agents
In docs/architecture/README.md around lines 45 to 46, the file path contains a
typo: "lib/features/order/notfiers/order_notifier.dart"; update the path to the
correct spelling "lib/features/order/notifiers/order_notifier.dart" wherever it
appears (including code fences/backticks) and ensure any related references or
indexes that depend on this path are updated to match.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Currently the directory is called "notfiers" and the documentation was faithful to reality, the directory name and then the documentation must be corrected to reflect the change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's verify the current directory structure to confirm the typo
echo "=== Current directory structure around order feature ==="
fd -t d notf --max-depth 5

echo -e "\n=== Checking if the misspelled directory exists ==="
ls -la lib/features/order/ 2>/dev/null || echo "order directory not found"

echo -e "\n=== Looking for any directories with 'notif' pattern ==="
fd -t d notif --max-depth 5

echo -e "\n=== Searching for import statements that reference 'notfiers' ==="
rg -n "notfiers" --type=dart

Length of output: 1740


🏁 Script executed:

#!/bin/bash
# Let's see what files are in the misspelled directory and check for more references
echo "=== Files in the misspelled notfiers directory ==="
ls -la lib/features/order/notfiers/

echo -e "\n=== All files that reference the misspelled path ==="
rg -n "notfiers" --type=dart -C 2

echo -e "\n=== Check if there are any test files referencing this path ==="
rg -n "notfiers" --type=dart test/

echo -e "\n=== Look for any documentation files that mention this path ==="
rg -n "notfiers" docs/

Length of output: 3313


@Catrya, you're absolutely right! The documentation was indeed faithful to reality. I can confirm that the directory is currently named notfiers (with the typo), and there are several files that need to be updated when we fix this.

Here's what needs to be corrected:

Files affected by the directory rename:

  1. Directory to rename: lib/features/order/notfiers/lib/features/order/notifiers/
  2. Files with import statements to update:
    • lib/features/order/notfiers/add_order_notifier.dart (self-reference)
    • lib/features/order/notfiers/order_notifier.dart (self-reference)
    • lib/features/order/providers/order_notifier_provider.dart (imports both notifier files)

Documentation files to update after the directory fix:

  • docs/architecture/README.md (Line 45)
  • docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md (Lines 50 and 922)

Recommended correction process:

  1. Rename the directory: notfiersnotifiers
  2. Update all import statements in the affected Dart files
  3. Update the documentation references to reflect the corrected path
  4. Run tests to ensure all references are properly updated

This is a good catch - the codebase should have the correct spelling, and then the documentation will accurately reflect the corrected structure.

🐰💡
Learning something new - the documentation was actually correct about the codebase state!


🔥 Failed to add learnings

Learnings cannot be added due to server error. Please try again.


🧠 Learnings used
Learnt from: CR
PR: MostroP2P/mobile#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T16:01:53.137Z
Learning: Update CLAUDE.md and documentation when making architectural changes

Comment on lines +885 to +892
List<String> get validRelays {
return relays
.where((url) => url.startsWith('wss://') || url.startsWith('ws://')) // Line 50
.map((url) => url.trim()) // Line 51
.map((url) => url.endsWith('/') ? url.substring(0, url.length - 1) : url) // Line 52
.toList(); // Line 53
}
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enforce wss:// consistently; current sample admits ws://.

Docs say “Only accepts wss://”, but validRelays allows ws://. Normalize or reject ws:// here.

 List<String> get validRelays {
   return relays
-      .where((url) => url.startsWith('wss://') || url.startsWith('ws://')) // Line 50
-      .map((url) => url.trim())                             // Line 51
+      .where((url) => url.startsWith('wss://') || url.startsWith('ws://'))
+      .map((url) => url.trim())
+      // Normalize ws:// → wss://
+      .map((url) => url.startsWith('ws://') ? 'wss://${url.substring(5)}' : url)
       .map((url) => url.endsWith('/') ? url.substring(0, url.length - 1) : url) // Line 52
       .toList();                                            // Line 53
 }

Alternatively, drop ws:// entirely by removing it from the where clause.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
List<String> get validRelays {
return relays
.where((url) => url.startsWith('wss://') || url.startsWith('ws://')) // Line 50
.map((url) => url.trim()) // Line 51
.map((url) => url.endsWith('/') ? url.substring(0, url.length - 1) : url) // Line 52
.toList(); // Line 53
}
```
List<String> get validRelays {
return relays
.where((url) => url.startsWith('wss://') || url.startsWith('ws://'))
.map((url) => url.trim())
// Normalize ws:// → wss://
.map((url) => url.startsWith('ws://') ? 'wss://${url.substring(5)}' : url)
.map((url) => url.endsWith('/') ? url.substring(0, url.length - 1) : url)
.toList();
}
🤖 Prompt for AI Agents
In docs/architecture/RELAY_SYNC_IMPLEMENTATION.md around lines 885 to 892, the
validRelays getter currently accepts both "wss://" and "ws://" despite the docs
stating only "wss://" is allowed; update the filter to enforce only "wss://"
(remove "ws://" from the where clause) and keep the subsequent trim and
trailing-slash removal steps, or if you prefer automatic normalization convert
any "ws://" entries to "wss://" before trimming/stripping the trailing slash so
only secure wss:// endpoints remain in the returned list.

Comment on lines +50 to +51
// lib/features/order/notfiers/order_notifier.dart
class OrderNotifier extends AbstractMostroNotifier {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix path typo: notfiers → notifiers.

-// lib/features/order/notfiers/order_notifier.dart
+// lib/features/order/notifiers/order_notifier.dart

And in “Related Documentation” list:

-**`lib/features/order/notfiers/order_notifier.dart`** - Core timeout detection and synthetic event creation
+**`lib/features/order/notifiers/order_notifier.dart`** - Core timeout detection and synthetic event creation

Also applies to: 922-924

🤖 Prompt for AI Agents
In docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md around lines 50-51
(and also apply the same fix to lines 922-924), correct the path typo from
"lib/features/order/notfiers/order_notifier.dart" to
"lib/features/order/notifiers/order_notifier.dart"; update any other occurrences
in the "Related Documentation" list to use the corrected "notifiers" directory
name so paths are consistent and accurate.

Comment on lines +250 to +253
**Key Features**:
- **Automatic expiration**: Removes sessions older than 36 hours (Config.sessionExpirationHours)
- **Periodic cleanup**: Scheduled cleanup every 30 minutes to prevent memory leaks
- **State synchronization**: Updates trigger automatic UI updates

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Session expiration inconsistency: 36h vs Config=72h.

Align text with Config.sessionExpirationHours (72) or change the Config snippet. Suggest using 72h here.

-**Automatic expiration**: Removes sessions older than 36 hours (Config.sessionExpirationHours)
+**Automatic expiration**: Removes sessions older than 72 hours (Config.sessionExpirationHours)

Also applies to: 831-839

🧰 Tools
🪛 LanguageTool

[grammar] ~250-~250: There might be a mistake here.
Context: ... UI updates } } ``` Key Features: - Automatic expiration: Removes sessions...

(QB_NEW_EN)


[grammar] ~251-~251: There might be a mistake here.
Context: ...36 hours (Config.sessionExpirationHours) - Periodic cleanup: Scheduled cleanup ev...

(QB_NEW_EN)


[grammar] ~252-~252: There might be a mistake here.
Context: ...every 30 minutes to prevent memory leaks - State synchronization: Updates trigger...

(QB_NEW_EN)


[grammar] ~253-~253: There might be a mistake here.
Context: ...**: Updates trigger automatic UI updates - Storage persistence: Sessions survive ...

(QB_NEW_EN)

🤖 Prompt for AI Agents
In docs/architecture/TIMEOUT_DETECTION_AND_SESSION_CLEANUP.md around lines 250
to 253 (and also update the same mismatch at lines 831-839), the doc states
sessions expire after 36 hours while Config.sessionExpirationHours is set to 72;
update the wording to reflect the actual config value (72 hours) or make the
text reference Config.sessionExpirationHours instead of a hardcoded duration;
ensure any Config snippet in the doc shows 72 (or change the config if the
intended timeout is 36) so both doc and config are consistent.

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@grunch grunch merged commit ac10d94 into main Aug 28, 2025
2 checks passed
@grunch grunch deleted the add-documentation branch August 28, 2025 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants