-
Notifications
You must be signed in to change notification settings - Fork 49
Outbox foundation #1186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Outbox foundation #1186
Conversation
fdab426 to
5d678d4
Compare
|
fixed seen on relay commit. just need to swap in an aesthetic icon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements an outbox fallback mechanism for Nostr relay discovery, enabling automatic fetching of missing content from author-specific relays. The implementation adds NIP-65 and observed relay tracking, a configurable relay connection budget, and UI elements to display relay information.
- Introduces
OutboxManagerto coordinate relay discovery and temporary connections based on NIP-65 hints and observed relay patterns - Adds a "Seen on relay" badge showing which relays delivered each note with interactive tooltip/popup UI
- Integrates outbox fallback into search, profile, and thread views to automatically fetch missing notes/profiles
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/notedeck_ui/src/note/mod.rs | Adds "Seen on relay" badge rendering with popup UI and relay simplification helper |
| crates/notedeck_ui/src/app_images.rs | Adds relay badge icon image |
| crates/notedeck_columns/src/ui/settings.rs | Adds outbox enabled/disabled toggle in settings UI |
| crates/notedeck_columns/src/ui/search/mod.rs | Integrates outbox fallback for missing search results |
| crates/notedeck_columns/src/ui/profile/mod.rs | Triggers outbox fetch for missing profiles |
| crates/notedeck_columns/src/timeline/thread.rs | Captures relay hints from note references for outbox |
| crates/notedeck_columns/src/ui/note/post.rs | Adds outbox field to NoteContext |
| crates/notedeck_columns/src/nav.rs | Threads outbox through navigation actions |
| crates/notedeck_columns/src/app.rs | Replaces basic unknown ID handling with outbox-aware dispatch |
| crates/notedeck/src/outbox/mod.rs | Core outbox dispatch logic |
| crates/notedeck/src/outbox/manager.rs | Relay selection, connection lifecycle, and request management |
| crates/notedeck/src/outbox/hints.rs | Relay hint caching with NIP-65 and observed relay support |
| crates/notedeck/src/unknowns.rs | Adds relay hint storage for unknown IDs |
| crates/notedeck/src/persist/settings_handler.rs | Persists outbox enabled preference with default true |
| crates/notedeck/src/note/mod.rs | Adds drive_unknown_ids helper to NoteContext |
| crates/notedeck/src/lib.rs | Exports outbox types |
| crates/notedeck/src/context.rs | Adds outbox to AppContext |
| crates/notedeck/src/app.rs | Initializes OutboxManager with persisted settings |
| assets/icons/seen_on_relay.svg | SVG icon for relay badge |
| assets/translations/en-US/main.ftl | English translations for new UI elements |
| assets/translations/en-XA/main.ftl | Pseudo-localized translations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let relays_for_hover = relays.clone(); | ||
| let tooltip_heading_clone = tooltip_heading.clone(); | ||
| badge_response.clone().on_hover_ui(move |ui| { | ||
| ui.label(RichText::new(&tooltip_heading_clone).strong()); | ||
| for relay in &relays_for_hover { | ||
| ui.label(simplify_relay_label(relay)); | ||
| } | ||
| }); | ||
|
|
||
| if badge_response.clicked() { | ||
| ui.memory_mut(|mem| mem.toggle_popup(popup_id)); | ||
| } | ||
|
|
||
| let relays_for_popup = relays.clone(); |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relays vector is cloned three times (lines 976, 989, and implicitly in the closure on line 978). Since the hover callback is executed on every frame when hovering, this creates unnecessary allocations. Consider using Rc<Vec<String>> or restructure the code to share the same data between hover and popup UIs without cloning.
| if entry.saturating_sub(1) == 0 { | ||
| self.ephemeral_relays.remove(&relay); | ||
| to_remove.insert(relay); | ||
| } else { | ||
| *entry -= 1; | ||
| } |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic checks if entry.saturating_sub(1) == 0 but then decrements *entry again in the else branch. If entry is 1, the condition is true and the relay is removed, but if entry is greater than 1, it's decremented in the else branch. However, the decrement should happen in both cases conceptually. The current code works but is confusing because the check on line 239 doesn't actually modify entry, yet the else branch decrements it. Consider restructuring: check if *entry == 1 to remove, otherwise decrement in an else branch for clarity.
| pub fn is_empty(&self) -> bool { | ||
| self.relays.is_empty() | ||
| } | ||
|
|
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The len() method is implemented without the corresponding #[must_use] attribute. Since is_empty() is implemented, adding #[must_use] to len() follows Rust conventions and helps catch cases where the return value is ignored.
| #[must_use] |
Summary
relays on EOSE)
Testing