Feat/subscription monitoring #382
Conversation
WalkthroughAdds crate-visible subscription-count and relay-connectivity helpers to NostrManager; widens visibility for pubkey-hash and group-relay extraction; replaces a relay-status API with an account-scoped variant; exposes ensure/is methods for subscription orchestration; and adds unit tests covering these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Whitenoise
participant NostrManager
participant RelayClientPool as RelayPool
rect rgba(230,245,255,0.5)
note right of Caller: ensure_account_subscriptions(account)
Caller->>Whitenoise: ensure_account_subscriptions(account)
Whitenoise->>Whitenoise: is_account_subscriptions_operational(account)
Whitenoise->>NostrManager: count_subscriptions_for_account(pubkey)
Whitenoise->>Whitenoise: get_account_relay_statuses(account) %% calls into relay helper
Whitenoise->>RelayPool: has_any_relay_connected(relays)
alt Not operational
Whitenoise->>Whitenoise: refresh per-account subscriptions
Whitenoise-->>Caller: Ok(()) / Err
else Operational
Whitenoise-->>Caller: Ok(())
end
end
sequenceDiagram
autonumber
participant Caller
participant Whitenoise
participant NostrManager
participant RelayClientPool as RelayPool
rect rgba(240,255,240,0.5)
note right of Caller: ensure_global_subscriptions()
Caller->>Whitenoise: ensure_global_subscriptions()
Whitenoise->>Whitenoise: is_global_subscriptions_operational()
Whitenoise->>NostrManager: count_global_subscriptions()
Whitenoise->>RelayPool: has_any_relay_connected(all_relays)
alt Not operational
Whitenoise->>Whitenoise: setup_global_users_subscriptions()
Whitenoise-->>Caller: Ok(()) / Err
else Operational
Whitenoise-->>Caller: Ok(())
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/whitenoise/relays.rs (1)
129-158: Consider stabilizing relay status ordering.Iterating a
HashSetproduces a non-deterministic order, so callers may see the relay list jump around between invocations. Sorting the deduplicated URLs before querying the client keeps the output stable for logs/UI.- for relay_url in unique_relay_urls { + let mut relay_urls: Vec<RelayUrl> = unique_relay_urls.into_iter().collect(); + relay_urls.sort_by(|a, b| a.as_str().cmp(b.as_str())); + + for relay_url in relay_urls {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/nostr_manager/mod.rs(1 hunks)src/nostr_manager/subscriptions.rs(1 hunks)src/whitenoise/accounts.rs(2 hunks)src/whitenoise/mod.rs(4 hunks)src/whitenoise/relays.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/rust-code-style.mdc)
**/*.rs: Write all trait bounds in a where clause for functions and impl blocks
Prefer using Self when referring to the implementing type (including return types and enum variant matches)
For public types in libraries, derive Debug, Clone, Copy, PartialEq, Eq, and Hash (in this order) when possible
Derive Default only when a reasonable default value exists
Use full paths for tracing macros (tracing::!(...)) instead of importing macro names
Prefer .to_string() or .to_owned() for &str -> String conversion over .into() or String::from
Order imports: (1) core/alloc/std, (2) external crates, (3) current sub-modules (mod x;), (4) internal crate modules (use crate::, super::, self::)
When implementing fmt traits, import core::fmt and use fmt::Display, fmt::Formatter, etc.
After declaring mod x;, import sub-items via self::x::Y instead of x::Y
Avoid if let ... { } else { } in favor of match when both branches do work
Use if let ... { } when the alternative match arm is intentionally empty
Avoid inline sub-modules (mod x { ... }); prefer a separate file x.rs with mod x; (except tests and benches)
For tests, prefer inline #[cfg(test)] mod tests { ... } and avoid mod tests;
For benches, prefer inline #[cfg(bench)] mod benches { ... } and avoid mod benches;
Files:
src/nostr_manager/subscriptions.rssrc/nostr_manager/mod.rssrc/whitenoise/mod.rssrc/whitenoise/accounts.rssrc/whitenoise/relays.rs
🧠 Learnings (2)
📚 Learning: 2025-09-14T19:36:21.619Z
Learnt from: jgmontoya
PR: parres-hq/whitenoise#365
File: src/whitenoise/accounts.rs:296-298
Timestamp: 2025-09-14T19:36:21.619Z
Learning: Repository parres-hq/whitenoise: For the process_user_event_streams method in src/whitenoise/accounts.rs, the team prefers to address the concurrent stream processing improvement (processing streams concurrently with per-event error logging instead of sequential processing with early returns) in a separate PR, as requested by jgmontoya in PR #365.
Applied to files:
src/whitenoise/mod.rs
📚 Learning: 2025-08-11T10:23:59.406Z
Learnt from: jgmontoya
PR: parres-hq/whitenoise#310
File: src/whitenoise/accounts/relays.rs:260-278
Timestamp: 2025-08-11T10:23:59.406Z
Learning: In `src/whitenoise/accounts/relays.rs`, the `publish_relay_list_for_account` method is only called after the account setup is complete, which guarantees that `account.nip65_relays` is already populated and will never be empty. Therefore, no fallback logic is needed when `target_relays` is None.
Applied to files:
src/whitenoise/relays.rs
🧬 Code graph analysis (2)
src/whitenoise/mod.rs (1)
src/whitenoise/accounts.rs (3)
account(1032-1038)account(1130-1136)inbox_relays(116-120)
src/whitenoise/accounts.rs (3)
src/whitenoise/mod.rs (2)
create_mock_whitenoise(580-639)new(56-70)src/whitenoise/groups.rs (1)
relays(201-201)src/whitenoise/database/users.rs (1)
relays(166-198)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Test (macos-latest)
- GitHub Check: Test (macos-14)
- GitHub Check: Security Audit
🔇 Additional comments (10)
src/nostr_manager/subscriptions.rs (1)
19-25: Helper visibility change looks good.Making
create_pubkey_hashcrate-visible lets the monitoring helpers reuse it without expanding the public API surface.src/whitenoise/accounts.rs (2)
770-788: Sharing group relay extractor is appropriate.Opening this helper to the crate keeps the monitoring paths from reimplementing MLS group plumbing while still limiting visibility to internal callers.
1413-1486: Great to see coverage for both group/no-group cases.These tests lock in the expected outputs and give confidence while the extractor is reused by the monitoring flows.
src/nostr_manager/mod.rs (2)
427-447: Global subscription and relay health helpers look solid.The lightweight filters over the client state keep these checks cheap while reusing existing primitives.
451-519: Thanks for the smoke tests around the new helpers.Even the empty-path assertions provide useful guardrails against regressions in future nostr-sdk updates.
src/whitenoise/mod.rs (5)
235-265: Adjusting the global subscription helper signature makes it reusable.Dropping the
'staticrequirement lets the ensure path share the same setup logic without spinning up duplicate plumbing.
444-471: Ensure helpers are a good safety net.The early exit prevents unnecessary teardown, and falling back to the existing refresh/setup flows keeps behaviour consistent for callers.
473-513: Per-account operational check balances presence and connectivity well.Combining the subscription-count gate with a deduped relay set makes the predicate deterministic once the counting helper matches by prefix.
515-528: Global health check reads clean.Reusing the client’s relay inventory with the shared connectivity helper keeps the logic compact and composable.
966-1138: The new subscription monitoring tests add valuable coverage.Exercising healthy, broken, and idempotent paths ensures the ensure/is helpers stay honest over time.
jgmontoya
force-pushed
the
feat/subscription-monitoring
branch
from
4b0ee1d to
72b8742
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/nostr_manager/mod.rs (1)
417-424: Filter by prefix, not substring, for pubkey-scoped subscriptions.Reusing
containsmeans any unrelated ID embedding the 12-char hash fragment (e.g.global_users_<hash>_...) keeps the account marked healthy, so we skip refreshes despite missing subscriptions. Match on the canonical prefix{hash}_we emit when creating IDs to avoid these false positives.Apply this diff:
- .filter(|id| id.as_str().contains(&hash)) + .filter(|id| id.as_str().starts_with(&format!("{}_", hash)))
🧹 Nitpick comments (1)
src/nostr_manager/mod.rs (1)
428-433: Usestarts_withfor global subscription IDs as well.All global IDs are emitted as
global_users_<suffix>, so prefix matching is tighter while keeping behavior identical.Apply this diff:
- .filter(|id| id.as_str().contains("global_users_")) + .filter(|id| id.as_str().starts_with("global_users_"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/nostr_manager/mod.rs(1 hunks)src/nostr_manager/subscriptions.rs(1 hunks)src/whitenoise/accounts.rs(2 hunks)src/whitenoise/mod.rs(4 hunks)src/whitenoise/relays.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/nostr_manager/subscriptions.rs
- src/whitenoise/relays.rs
- src/whitenoise/accounts.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/rust-code-style.mdc)
**/*.rs: Write all trait bounds in a where clause for functions and impl blocks
Prefer using Self when referring to the implementing type (including return types and enum variant matches)
For public types in libraries, derive Debug, Clone, Copy, PartialEq, Eq, and Hash (in this order) when possible
Derive Default only when a reasonable default value exists
Use full paths for tracing macros (tracing::!(...)) instead of importing macro names
Prefer .to_string() or .to_owned() for &str -> String conversion over .into() or String::from
Order imports: (1) core/alloc/std, (2) external crates, (3) current sub-modules (mod x;), (4) internal crate modules (use crate::, super::, self::)
When implementing fmt traits, import core::fmt and use fmt::Display, fmt::Formatter, etc.
After declaring mod x;, import sub-items via self::x::Y instead of x::Y
Avoid if let ... { } else { } in favor of match when both branches do work
Use if let ... { } when the alternative match arm is intentionally empty
Avoid inline sub-modules (mod x { ... }); prefer a separate file x.rs with mod x; (except tests and benches)
For tests, prefer inline #[cfg(test)] mod tests { ... } and avoid mod tests;
For benches, prefer inline #[cfg(bench)] mod benches { ... } and avoid mod benches;
Files:
src/nostr_manager/mod.rssrc/whitenoise/mod.rs
🧠 Learnings (1)
📚 Learning: 2025-09-04T03:38:54.953Z
Learnt from: jgmontoya
PR: parres-hq/whitenoise#340
File: src/nostr_manager/subscriptions.rs:72-89
Timestamp: 2025-09-04T03:38:54.953Z
Learning: In the global subscription batching system for Whitenoise (src/nostr_manager/subscriptions.rs), the current hash(pubkey) % batch_count approach causes batch ID reshuffling when user count changes, potentially leading to stale/duplicate author lists since only the triggering user's batch is refreshed. This is a known limitation documented in PR #340 that the team is aware of and will address in future iterations.
Applied to files:
src/nostr_manager/mod.rs
🧬 Code graph analysis (2)
src/nostr_manager/mod.rs (1)
src/whitenoise/event_tracker.rs (2)
new(127-129)new(246-248)
src/whitenoise/mod.rs (2)
src/whitenoise/accounts.rs (3)
account(1033-1039)account(1131-1137)inbox_relays(116-120)src/nostr_manager/subscriptions.rs (1)
user_relays(338-343)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (macos-14)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Security Audit
…thod to pub(crate)
jgmontoya
force-pushed
the
feat/subscription-monitoring
branch
from
72b8742 to
ac3f240
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/whitenoise/mod.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/rust-code-style.mdc)
**/*.rs: Write all trait bounds in a where clause for functions and impl blocks
Prefer using Self when referring to the implementing type (including return types and enum variant matches)
For public types in libraries, derive Debug, Clone, Copy, PartialEq, Eq, and Hash (in this order) when possible
Derive Default only when a reasonable default value exists
Use full paths for tracing macros (tracing::!(...)) instead of importing macro names
Prefer .to_string() or .to_owned() for &str -> String conversion over .into() or String::from
Order imports: (1) core/alloc/std, (2) external crates, (3) current sub-modules (mod x;), (4) internal crate modules (use crate::, super::, self::)
When implementing fmt traits, import core::fmt and use fmt::Display, fmt::Formatter, etc.
After declaring mod x;, import sub-items via self::x::Y instead of x::Y
Avoid if let ... { } else { } in favor of match when both branches do work
Use if let ... { } when the alternative match arm is intentionally empty
Avoid inline sub-modules (mod x { ... }); prefer a separate file x.rs with mod x; (except tests and benches)
For tests, prefer inline #[cfg(test)] mod tests { ... } and avoid mod tests;
For benches, prefer inline #[cfg(bench)] mod benches { ... } and avoid mod benches;
Files:
src/whitenoise/mod.rs
🧬 Code graph analysis (1)
src/whitenoise/mod.rs (2)
src/whitenoise/accounts.rs (3)
account(1033-1039)account(1131-1137)inbox_relays(116-120)src/whitenoise/database/accounts.rs (1)
all(93-103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Test (macos-latest)
- GitHub Check: Security Audit
Adds
ensure_all_subscriptionsto be called from a background periodic task in an attempt to keep the connections/subscriptions alive and functioning.It also fixes docstrings/comment and changes renames the
fetch_relay_statusmethod toget_account_relay_statusesto be more explicit.Summary by CodeRabbit
New Features
Tests