Feat/profile update and others#4
Merged
Merged
Conversation
also add nsec export
…up info, and account management Add terminal image rendering for profile pictures using ratatui-image with auto-detection of Kitty/Sixel/iTerm2/Halfblocks protocols. Images display on both the profile screen and user profile popups. Includes a workaround for iTerm2 misdetecting as Kitty by checking ITERM_SESSION_ID env var. Extend profile editing with display_name, picture URL, NIP-05, and lightning address fields. Use instead of for full metadata including picture URLs. Add user profile viewing from follows list (Enter) and search results (F2), group info popup (i) on group detail screen, group picker for adding searched users to existing groups (F4), and create-group-with-user flow (F3). Add logout from profile screen (Q) and account select screen (d), create new identity from main screen (C) and account select (c), and login with nsec from account select (l). Other improvements: log panel visible on all screens, Tab switches log tabs globally, paste into popup text inputs, DM invite name resolution via welcomer_pubkey lookup. Deps: ratatui 0.29 -> 0.30, +ratatui-image 10, +image 0.25
Add react/unreact support with emoji popup (r/u keys in Messages panel), selection-based message navigation with viewport-aware scrolling (j/k moves cursor, g/G jumps to oldest/newest), and message kind filtering to only display kind-9 chat messages from the subscription stream. Remove left groups from the chat list immediately on leave, and clear stale entries when re-subscribing after group actions.
WalkthroughAdds inline image rendering and media upload/download workflows, significantly expands Action/Effect enums for messaging, reactions, profiles and account operations, introduces a new util module for JSON→hex conversion, updates UI screens and widgets to support images and richer profile display, and bumps UI/image dependencies in Cargo.toml. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/screen/user_search.rs (1)
24-29:⚠️ Potential issue | 🟡 MinorFooter exceeds 80 columns and shows mode-inappropriate shortcuts.
The footer spans 107 characters when all hints are rendered, far exceeding the standard 80-column terminal width. More critically, F3 ("New group") and F4 ("Add to group") are shown unconditionally whenever results exist, despite SearchPurpose distinguishing between Browse and AddMember modes—in AddMember, "Add to group" appears twice (as the Enter action and as F4), while in Browse mode, the F4 shortcut is misleading. Split the footer into two rows and conditionally render shortcuts based on SearchPurpose to keep it readable and accurate. Verify at 80-column width before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screen/user_search.rs` around lines 24 - 29, The footer currently renders as a single long row in user_search.rs (the Layout::vertical with Constraints and the footer render code) and unconditionally shows F3/F4 hints; update the footer to use two rows (split into two Constraint::Length(1) rows in the vertical layout and render two separate hint lines) and make hint rendering conditional based on SearchPurpose (check the enum value used in the search screen, e.g., SearchPurpose::Browse vs SearchPurpose::AddMember) so that F3 ("New group") and F4 ("Add to group") are only shown when appropriate (F4 omitted in Browse, avoid duplicating "Add to group" in AddMember since Enter already performs that action). Ensure the total width of each row fits within 80 columns by truncating or omitting less important hints when necessary and test at an 80-column terminal before merging.src/widget/message_list.rs (1)
582-593:⚠️ Potential issue | 🟠 MajorKeep at least one oversized message visible.
With
INLINE_IMAGE_ROWS = 8, the newest message can easily be taller thanvisible_height. This loop then breaks before pushing anything, so the pane renders empty even though messages exist. Include the first candidate even if it overflows, and apply the same rule incompute_visible()so inline-image positions stay aligned.💡 Suggested fix
- for i in (0..end).rev() { - let h = message_height_with_images(&self.messages[i], width, self.inline_images); - if used_rows + h > visible_height { - break; - } + for i in (0..end).rev() { + let h = message_height_with_images(&self.messages[i], width, self.inline_images); + if used_rows + h > visible_height && !visible_msgs.is_empty() { + break; + } used_rows += h; visible_msgs.push(i); + if used_rows >= visible_height { + break; + } }Apply the same condition in
MessageListWidget::compute_visible().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widget/message_list.rs` around lines 582 - 593, The loop that builds visible_msgs in MessageListWidget (using variables visible_msgs, used_rows, used_rows + h, visible_height, skip_messages and calling message_height_with_images) currently breaks before pushing any item if the first candidate's height exceeds visible_height, leaving the pane empty; change the logic so the first candidate index is always pushed into visible_msgs even if h > visible_height (i.e., allow one oversized message), and mirror this same rule inside compute_visible() so inline-image positioning stays aligned with INLINE_IMAGE_ROWS handling. Ensure you still break out after adding the first oversized message and continue preserving existing skip_messages behavior.
🧹 Nitpick comments (1)
src/widget/message_list.rs (1)
91-114: Extractextract_hash_hexinto a shared helper.This duplicates the existing attachment-hash parsing logic in
src/app.rs. Keeping two copies of the same schema handling will drift the next time the CLI payload changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widget/message_list.rs` around lines 91 - 114, Move the extract_hash_hex parsing logic into a single shared helper function (keep the current signature fn extract_hash_hex(att: &Value, field: &str) -> Option<String>) in a common module (e.g., a new util/mod or shared/helpers module), export it publicly, and replace the duplicate implementation in src/widget/message_list.rs and the copy in src/app.rs with calls to that shared function; update use/import paths where needed and run tests/compile to ensure no behavioral change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 32-34: The README build instructions currently run git clone then
cargo build but never switch to the advertised feat/cli branch; update the
instructions to explicitly check out that branch (either by adding a git
checkout feat/cli after git clone or by cloning with the branch flag) before
running the cargo build command so the CLI targets/features referenced
(feat/cli) are present for the subsequent cargo build --release --bin wn
--features cli --bin wnd step.
In `@src/action.rs`:
- Line 36: The MessagesLoaded enum variant (and the analogous success variants
for FetchProfileImage and ShowUserProfile) currently drops the original request
key causing race conditions; update MessagesLoaded(Vec<Value>) to include the
request identity (e.g., MessagesLoaded { group_id: GroupId, messages: Vec<Value>
}) and do the same for the profile image and user profile success variants
(carry url or pubkey alongside payload), then update the action creators that
dispatch LoadMessages/FetchProfileImage/ShowUserProfile to include the same key
in their success dispatches and modify the reducer handlers to match on the key
before applying payloads so stale responses are ignored.
In `@src/main.rs`:
- Around line 953-960: The ShowUserProfile branch currently forwards the raw
wn::exec output, causing mismatched shape vs LoadProfile; update
Effect::ShowUserProfile (the tokio::spawn handling that calls wn::exec and
constructs Action::UserProfileLoaded) to mirror the flattening logic from
Effect::LoadProfile: if the returned value contains a metadata object, extract
its fields (name, display_name, about, picture, nip05, lud16, npub) and promote
them to top-level keys, and if top-level npub is missing set it to the
account/pubkey fallback used in LoadProfile before constructing
Action::UserProfileLoaded so the screen receives the same normalized shape.
- Around line 807-823: Update the curl invocation and response handling inside
the Effect::FetchProfileImage branch: add the "--fail" flag to the
tokio::process::Command::new("curl") args so curl returns non-zero on HTTP
4xx/5xx, and enforce a maximum response size (e.g. define a const
MAX_PROFILE_IMAGE_SIZE) by checking output.stdout.len() before sending
Event::Action(Action::ProfileImageFetched(...)); if output.status.success() is
false or the body is empty or exceeds MAX_PROFILE_IMAGE_SIZE, send the existing
Event::Action(Action::Log(...)) instead. Reference: Effect::FetchProfileImage,
tokio::process::Command::new("curl"), and
Event::Action(Action::ProfileImageFetched).
In `@src/screen/main_screen.rs`:
- Line 356: The current call to
frame.render_widget(Paragraph::new(lines).wrap(Wrap { trim: false }), inner)
uses Wrap which lets a single log entry expand into multiple visual rows, but
your windowing uses visible based on raw entry count so long entries hide newest
logs; fix by either removing the .wrap(Wrap { trim: false }) so each entry
occupies one row (keep Paragraph::new(lines) unwrapped) or make the windowing
logic row-aware: compute visual rows for each entry (e.g., measure wrapped line
count for each entry using the same wrapping rules) and adjust the code that
computes visible to use wrapped-row counts rather than entry count (update the
logic that selects entries for rendering using the wrapped row totals).
- Around line 184-190: The code calculates preview length and slices it by bytes
which can panic on multi-byte UTF‑8; in the block that builds placeholder
(matching on app.reply_to and using preview and the byte-slice &preview[..27]),
replace the byte-length check preview.len() and byte-slice with character-aware
logic: use preview.chars().count() to decide if truncation is needed and build
the truncated string with preview.chars().take(27).collect::<String>() (or
equivalent) so slicing never splits a UTF‑8 codepoint, then use that truncated
String in the format! for placeholder.
In `@src/screen/profile.rs`:
- Around line 87-95: When has_image is true the fixed 20-column image pane can
collapse the text pane on narrow terminals; update the logic that builds
(image_area, text_area) so it measures the available width from vertical[0]
before using Layout::horizontal with Constraint::Length(20) and
Constraint::Fill(1) and, if the total width is below a safe threshold (enough to
accommodate labels like " Name: "), fall back to the no-image branch
(set image_area to None and text_area to vertical[0]) instead of splitting;
apply this check where (image_area, text_area) is computed so the code avoids
creating a too-narrow cols[1].
---
Outside diff comments:
In `@src/screen/user_search.rs`:
- Around line 24-29: The footer currently renders as a single long row in
user_search.rs (the Layout::vertical with Constraints and the footer render
code) and unconditionally shows F3/F4 hints; update the footer to use two rows
(split into two Constraint::Length(1) rows in the vertical layout and render two
separate hint lines) and make hint rendering conditional based on SearchPurpose
(check the enum value used in the search screen, e.g., SearchPurpose::Browse vs
SearchPurpose::AddMember) so that F3 ("New group") and F4 ("Add to group") are
only shown when appropriate (F4 omitted in Browse, avoid duplicating "Add to
group" in AddMember since Enter already performs that action). Ensure the total
width of each row fits within 80 columns by truncating or omitting less
important hints when necessary and test at an 80-column terminal before merging.
In `@src/widget/message_list.rs`:
- Around line 582-593: The loop that builds visible_msgs in MessageListWidget
(using variables visible_msgs, used_rows, used_rows + h, visible_height,
skip_messages and calling message_height_with_images) currently breaks before
pushing any item if the first candidate's height exceeds visible_height, leaving
the pane empty; change the logic so the first candidate index is always pushed
into visible_msgs even if h > visible_height (i.e., allow one oversized
message), and mirror this same rule inside compute_visible() so inline-image
positioning stays aligned with INLINE_IMAGE_ROWS handling. Ensure you still
break out after adding the first oversized message and continue preserving
existing skip_messages behavior.
---
Nitpick comments:
In `@src/widget/message_list.rs`:
- Around line 91-114: Move the extract_hash_hex parsing logic into a single
shared helper function (keep the current signature fn extract_hash_hex(att:
&Value, field: &str) -> Option<String>) in a common module (e.g., a new util/mod
or shared/helpers module), export it publicly, and replace the duplicate
implementation in src/widget/message_list.rs and the copy in src/app.rs with
calls to that shared function; update use/import paths where needed and run
tests/compile to ensure no behavioral change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 109d4edb-bc0b-4f04-9827-b161222180fd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
Cargo.tomlREADME.mdsrc/action.rssrc/app.rssrc/main.rssrc/screen/group_detail.rssrc/screen/login.rssrc/screen/main_screen.rssrc/screen/profile.rssrc/screen/user_search.rssrc/widget/message_list.rssrc/widget/popup.rs
jgmontoya
force-pushed
the
feat/profile-update-and-others
branch
from
March 8, 2026 17:36
031e8e7 to
fd4b90b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main.rs (1)
953-963:⚠️ Potential issue | 🟠 MajorNormalize
ShowUserProfiledata to matchLoadProfileshape.
LoadProfile(lines 727-748) flattens metadata fields to the top level, butShowUserProfilesends the raw response. This causes profile fields (name, about, picture, nip05, lud16) to render as "(not set)" when viewing other users' profiles sincesrc/screen/profile.rsexpects top-level keys.🐛 Proposed fix
Effect::ShowUserProfile { account, pubkey } => { let tx = tx.clone(); tokio::spawn(async move { let action = match wn::exec(&["--account", &account, "users", "show", &pubkey]).await { - Ok(val) => Action::UserProfileLoaded(val), + Ok(val) => { + let mut profile = val.clone(); + if let Some(meta) = val.get("metadata").and_then(|m| m.as_object()) { + for (k, v) in meta { + profile[k.clone()] = v.clone(); + } + } + if profile.get("npub").is_none() { + if let Some(pk) = val.get("pubkey").and_then(|v| v.as_str()) { + profile["npub"] = serde_json::Value::String(pk.to_string()); + } + } + Action::UserProfileLoaded(profile) + } Err(e) => Action::UserProfileError(e.to_string()), }; let _ = tx.send(Event::Action(action)); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 953 - 963, The ShowUserProfile branch returns the raw response shape, but UI expects the same flattened shape produced by LoadProfile; update the async block in Effect::ShowUserProfile so that after wn::exec(...) succeeds you transform the returned value to match LoadProfile's shape by extracting metadata fields (name, about, picture, nip05, lud16) from the response.metadata and promoting them to top-level keys (providing None/empty when absent) before constructing Action::UserProfileLoaded; keep the error branch as-is (Action::UserProfileError) and send Event::Action(action) as before.
🧹 Nitpick comments (4)
src/main.rs (2)
536-589: N+1 query pattern for DM invite resolution may cause latency.Each DM invite triggers a separate
wn exec users showcall. While pending invites are typically few, this could cause noticeable delay with many invites.For now, this is acceptable given the typical invite count. If performance becomes an issue, consider batching the queries or using a single bulk user lookup command if the CLI supports it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 536 - 589, The code performs an N+1 query by calling wn::exec(&["--account", &account, "users", "show", pubkey]) for each invite in arr; instead, collect all welcomer_pubkey values first, issue a single batched user lookup (or multiple chunked lookups) via the CLI if it supports a bulk "users show" or "users list --ids" style command, cache the returned user objects into a HashMap<pubkey, user_val>, and then iterate inv in arr to resolve display_name from the cached user_val before writing into group["name"]; update the logic around collecting pubkeys, calling wn::exec once (or few times), and replacing per-invite wn::exec calls with HashMap lookups while reusing the existing display_name extraction and group["name"] assignment.
1037-1061: Consider adding a size limit for media popup loading.
LoadMediaPopupreads the entire file and decodes it in memory. Very large images could cause memory pressure. Consider adding a file size check before loading:💡 Suggestion
Effect::LoadMediaPopup { file_path } => { let tx = tx.clone(); tokio::spawn(async move { + const MAX_POPUP_SIZE: u64 = 50 * 1024 * 1024; // 50MB + if let Ok(meta) = tokio::fs::metadata(&file_path).await { + if meta.len() > MAX_POPUP_SIZE { + send_log(&tx, format!("Image too large for popup: {} bytes", meta.len())); + return; + } + } match tokio::fs::read(&file_path).await {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 1037 - 1061, Effect::LoadMediaPopup currently reads and decodes the entire file into memory (tokio::fs::read + image::load_from_memory), which can OOM on large files; before reading, call tokio::fs::metadata(&file_path).await (or File::metadata) and compare against a defined MAX_POPUP_IMAGE_BYTES constant, and if the file is larger send_log(&tx, format!("Popup image too large: {} bytes", size)) and skip loading; only proceed to tokio::fs::read and image::load_from_memory when the size check passes, keeping existing error handling paths (send_log and Event::Action(Action::MediaPopupReady)).src/widget/message_list.rs (1)
91-114: Consider consolidatingextract_hash_hexwith the version insrc/app.rs.This function duplicates the logic from
src/app.rs(lines 894-917). While module isolation can justify duplication, having two implementations risks divergence.Consider either:
- Moving this to a shared utility module
- Re-exporting from
app.rsHowever, if the message list widget intentionally avoids depending on
app.rsfor decoupling, this duplication is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widget/message_list.rs` around lines 91 - 114, The extract_hash_hex implementation in message_list.rs duplicates the logic in app.rs; refactor by moving the shared logic into a single utility (e.g., a new function extract_hash_hex in a shared module like utils or helpers) and update both message_list.rs and app.rs to call that shared function (or alternatively re-export the function from app.rs if you prefer not to add a new module); ensure the signature (fn extract_hash_hex(att: &Value, field: &str) -> Option<String>) and behavior are preserved and update the use/import paths in both files so there is one canonical implementation.src/screen/main_screen.rs (1)
150-173: Consider avoiding duplicateMessageListWidgetconstruction.The widget is constructed twice: once to compute
image_positions()and once for rendering. While the widget itself is lightweight (holding references), theblock.clone()and repeated setup could be avoided.One option is to split
image_positionsinto a standalone function that takes the same parameters but doesn't require constructing the widget:// Could compute positions directly without building the widget twice let image_rects = compute_image_positions(&app.messages, app.message_scroll, ...);However, given the current architecture where
image_positionsencapsulates the layout logic withinMessageListWidget, this duplication is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screen/main_screen.rs` around lines 150 - 173, Construct the MessageListWidget once and reuse it: create a single let widget = MessageListWidget::new(&app.messages, app.message_scroll).block(block.clone()).my_pubkey(app.account.as_deref()).selected(selected).media_downloads(&app.media_downloads).inline_images(&app.inline_images); then call let image_rects = widget.image_positions(area); and immediately pass the same widget to frame.render_widget(widget, area) (remove the second MessageListWidget::new). This removes the duplicate construction and avoids the extra block.clone(); keep the subsequent loop that uses image_rects and frame.render_stateful_widget(StatefulImage::default(), rect, proto) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/widget/message_list.rs`:
- Around line 217-224: The truncation check uses preview.len() (bytes) but
preview was built with content(parent).chars().take(30) (characters), causing
inconsistent "..." for multi-byte chars; update the condition in the label
construction to use a character count instead (e.g., check
content(parent).chars().count() > 30 or preview.chars().count() == 30) so the
logic around preview and the appended "..." is based on characters; adjust the
if that references preview.len() to use one of those char-based checks and keep
the existing author_name(parent), content(parent), and preview variables.
---
Duplicate comments:
In `@src/main.rs`:
- Around line 953-963: The ShowUserProfile branch returns the raw response
shape, but UI expects the same flattened shape produced by LoadProfile; update
the async block in Effect::ShowUserProfile so that after wn::exec(...) succeeds
you transform the returned value to match LoadProfile's shape by extracting
metadata fields (name, about, picture, nip05, lud16) from the response.metadata
and promoting them to top-level keys (providing None/empty when absent) before
constructing Action::UserProfileLoaded; keep the error branch as-is
(Action::UserProfileError) and send Event::Action(action) as before.
---
Nitpick comments:
In `@src/main.rs`:
- Around line 536-589: The code performs an N+1 query by calling
wn::exec(&["--account", &account, "users", "show", pubkey]) for each invite in
arr; instead, collect all welcomer_pubkey values first, issue a single batched
user lookup (or multiple chunked lookups) via the CLI if it supports a bulk
"users show" or "users list --ids" style command, cache the returned user
objects into a HashMap<pubkey, user_val>, and then iterate inv in arr to resolve
display_name from the cached user_val before writing into group["name"]; update
the logic around collecting pubkeys, calling wn::exec once (or few times), and
replacing per-invite wn::exec calls with HashMap lookups while reusing the
existing display_name extraction and group["name"] assignment.
- Around line 1037-1061: Effect::LoadMediaPopup currently reads and decodes the
entire file into memory (tokio::fs::read + image::load_from_memory), which can
OOM on large files; before reading, call tokio::fs::metadata(&file_path).await
(or File::metadata) and compare against a defined MAX_POPUP_IMAGE_BYTES
constant, and if the file is larger send_log(&tx, format!("Popup image too
large: {} bytes", size)) and skip loading; only proceed to tokio::fs::read and
image::load_from_memory when the size check passes, keeping existing error
handling paths (send_log and Event::Action(Action::MediaPopupReady)).
In `@src/screen/main_screen.rs`:
- Around line 150-173: Construct the MessageListWidget once and reuse it: create
a single let widget = MessageListWidget::new(&app.messages,
app.message_scroll).block(block.clone()).my_pubkey(app.account.as_deref()).selected(selected).media_downloads(&app.media_downloads).inline_images(&app.inline_images);
then call let image_rects = widget.image_positions(area); and immediately pass
the same widget to frame.render_widget(widget, area) (remove the second
MessageListWidget::new). This removes the duplicate construction and avoids the
extra block.clone(); keep the subsequent loop that uses image_rects and
frame.render_stateful_widget(StatefulImage::default(), rect, proto) unchanged.
In `@src/widget/message_list.rs`:
- Around line 91-114: The extract_hash_hex implementation in message_list.rs
duplicates the logic in app.rs; refactor by moving the shared logic into a
single utility (e.g., a new function extract_hash_hex in a shared module like
utils or helpers) and update both message_list.rs and app.rs to call that shared
function (or alternatively re-export the function from app.rs if you prefer not
to add a new module); ensure the signature (fn extract_hash_hex(att: &Value,
field: &str) -> Option<String>) and behavior are preserved and update the
use/import paths in both files so there is one canonical implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c924402-21af-49ae-96aa-75ba9f328680
📒 Files selected for processing (7)
README.mdsrc/action.rssrc/app.rssrc/main.rssrc/screen/main_screen.rssrc/screen/profile.rssrc/widget/message_list.rs
jgmontoya
force-pushed
the
feat/profile-update-and-others
branch
from
March 8, 2026 19:08
fd4b90b to
59b9b92
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/screen/main_screen.rs (1)
24-29:⚠️ Potential issue | 🟡 MinorReserve more than one row for the expanded main-screen hints.
The footer still allocates a single
Length(1)row, but the new ChatList and Messages shortcut sets are much longer than one line. This paragraph is rendered unwrapped, so the trailing hints disappear on ordinary terminal widths.Also applies to: 215-293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screen/main_screen.rs` around lines 24 - 29, The layout currently reserves only a single row for the footer/hints (see the Layout::vertical(...) assigned to variable vertical and the trailing Constraint::Length(1) entries), which truncates the expanded ChatList/Messages shortcut hints; update the footer allocation to reserve multiple rows (e.g., replace the single Length(1) Constraint with a larger fixed Length like Length(3) or a dynamic Constraint::Min(n)) so the hints can render without truncation; apply the same change wherever the layout builds vertical with two trailing Constraint::Length(1) rows (including the other occurrences noted) so all hint areas have sufficient height.src/widget/message_list.rs (1)
247-249:⚠️ Potential issue | 🟡 MinorUse one display-width indent for follow-up lines and inline images.
Attachment/reply lines are padded with
prefix.len() + author_prefix.len(), whileimage_positionsstarts thumbnails at timestamp width only. That shifts inline images left of the attachment column even for ASCII names, and non-ASCII authors make it worse becauselen()counts bytes instead of terminal cells.Suggested fix
- let indent = prefix.len() + author_prefix.len(); + let indent = format!("{prefix}{author_prefix}").width(); ... - let ts_prefix = format!("[{ts}] "); - let ts_cols = ts_prefix.width() as u16; - let prefix_width = ts_prefix.width() + format!("{author}: ").width(); + let prefix_width = format!("[{ts}] {author}: ").width(); ... - let img_x = inner.x + ts_cols; - let img_w = inner.width.saturating_sub(ts_cols); + let indent = prefix_width as u16; + let img_x = inner.x + indent; + let img_w = inner.width.saturating_sub(indent);Also applies to: 482-516
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widget/message_list.rs` around lines 247 - 249, The code uses byte lengths (prefix.len(), author_prefix.len()) to compute indent and image positions which misaligns multi-byte/non-ASCII author names and inline images; replace these .len() usages with display cell widths using a Unicode width helper (e.g., unicode_width::UnicodeWidthStr::width) to compute prefix_width and author_prefix_width and then set indent = prefix_width + author_prefix_width, and likewise compute the timestamp/display widths used by image_positions so thumbnails align with the attachment/reply column; update all occurrences (including the other block at lines ~482-516) to use the same Unicode-aware width calculations for consistent alignment.
♻️ Duplicate comments (3)
src/action.rs (1)
36-36:⚠️ Potential issue | 🟠 MajorCarry the request key through these async success actions.
MessagesLoaded,ProfileImageFetched, andUserProfileLoadedstill drop the originatinggroup_id/url/pubkey. Once two loads overlap, the reducer cannot distinguish current data from stale responses, so older payloads can overwrite the active chat or profile.Also applies to: 83-83, 106-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/action.rs` at line 36, The async success enum variants MessagesLoaded, ProfileImageFetched, and UserProfileLoaded are losing the originating request key (group_id / url / pubkey); modify those Action enum variants to carry the original request key alongside the payload (e.g., MessagesLoaded(group_id, Vec<Value>), ProfileImageFetched(url, Vec<u8>), UserProfileLoaded(pubkey, Profile)), update every place that constructs these variants in the async handling paths to include the key, and update the reducer/handlers that match on these variants to check the carried key against the currently active key before applying state changes to avoid stale responses overwriting current data.src/main.rs (2)
953-960:⚠️ Potential issue | 🟠 MajorNormalize
users showoutput before dispatchingUserProfileLoaded.
src/screen/profile.rsreads top-levelname,display_name,about,picture,nip05,lud16, andnpub, but this branch forwards the rawusers showpayload. Metadata-only fields will render as(not set), and a missing top-levelnpubfalls back to the logged-in account's key. Mirror the same flattening/fallback logic used in the self-profile loader.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 953 - 960, The branch handling Effect::ShowUserProfile currently dispatches the raw wn::exec payload into Action::UserProfileLoaded; instead, normalize/flatten that payload the same way the self-profile loader does so src/screen/profile.rs can read top-level name, display_name, about, picture, nip05, lud16, and npub with fallbacks. After wn::exec returns Ok(val), extract/compute top-level fields (prefer direct top-level values, then metadata subfields, and ensure npub falls back to the requested account key) and build a normalized struct/object matching what the self-profile loader produces, then dispatch Action::UserProfileLoaded(normalized) (preserve the Err path to Action::UserProfileError).
810-816:⚠️ Potential issue | 🟠 MajorStill bound profile-image downloads before buffering them.
--failfixes HTTP error handling, but this branch still accepts arbitrary body sizes and forwards them straight intoProfileImageFetched. A maliciouspictureURL can force a large allocation before any decode or validation runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 810 - 816, The code currently pipes the entire curl output straight into Event::Action(Action::ProfileImageFetched(output.stdout)), allowing arbitrarily large bodies; fix by enforcing a hard size limit before sending: after the tokio::process::Command::new("curl") call and when matching Ok(output) (the branch that currently checks output.status.success() && !output.stdout.is_empty()), check output.stdout.len() against a defined MAX_PROFILE_IMAGE_BYTES constant (e.g., 1_000_000 bytes) and only send ProfileImageFetched if length <= MAX_PROFILE_IMAGE_BYTES; otherwise log/drop the image and return an error branch. Also consider adding curl-level protection (e.g., --max-filesize) to the args list as an extra safeguard. Ensure references to tokio::process::Command::new("curl"), the result match block, and Event::Action(Action::ProfileImageFetched(...)) are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.rs`:
- Around line 1040-1048: The LoadMediaPopup handler (and similarly
LoadMediaImage) currently reads and decodes the entire file via
tokio::fs::read() and image::load_from_memory() without limits; change it to
first stat the file (tokio::fs::metadata or File::metadata) and reject files
whose size exceeds a defined MAX_MEDIA_BYTES before calling read, then after
decoding (image::load_from_memory or ImageReader) validate img.width() and
img.height() (or total pixels) against a defined MAX_DIMENSION / MAX_PIXELS and
reject oversized images (send an error Event or skip sending MediaPopupReady) to
avoid OOM or decompression bombs; add constants like MAX_MEDIA_BYTES and
MAX_PIXELS and reuse the same checks in LoadMediaImage.
In `@src/util.rs`:
- Around line 17-30: The code currently uses filter_map and casts to u8 which
silently drops non-numeric entries and wraps values >255; change the logic so
the byte array is validated and rejected if any element is missing, non-integer,
or out of 0..=255. Specifically, replace the filter_map pipeline over arr with
an iterator that attempts to convert every element (use as_u64 or equivalent),
returns None if any element is not present or >255, collects a Vec<u8> (e.g.,
via map(...).and_then/collect::<Option<Vec<u8>>>()), and only then constructs
the hex string from that Vec<u8> (preserving the existing hex formatting and the
None return for empty/malformed cases). Ensure you update references to arr, val
and hex accordingly so malformed JSON yields None rather than a coerced/wrapped
value.
---
Outside diff comments:
In `@src/screen/main_screen.rs`:
- Around line 24-29: The layout currently reserves only a single row for the
footer/hints (see the Layout::vertical(...) assigned to variable vertical and
the trailing Constraint::Length(1) entries), which truncates the expanded
ChatList/Messages shortcut hints; update the footer allocation to reserve
multiple rows (e.g., replace the single Length(1) Constraint with a larger fixed
Length like Length(3) or a dynamic Constraint::Min(n)) so the hints can render
without truncation; apply the same change wherever the layout builds vertical
with two trailing Constraint::Length(1) rows (including the other occurrences
noted) so all hint areas have sufficient height.
In `@src/widget/message_list.rs`:
- Around line 247-249: The code uses byte lengths (prefix.len(),
author_prefix.len()) to compute indent and image positions which misaligns
multi-byte/non-ASCII author names and inline images; replace these .len() usages
with display cell widths using a Unicode width helper (e.g.,
unicode_width::UnicodeWidthStr::width) to compute prefix_width and
author_prefix_width and then set indent = prefix_width + author_prefix_width,
and likewise compute the timestamp/display widths used by image_positions so
thumbnails align with the attachment/reply column; update all occurrences
(including the other block at lines ~482-516) to use the same Unicode-aware
width calculations for consistent alignment.
---
Duplicate comments:
In `@src/action.rs`:
- Line 36: The async success enum variants MessagesLoaded, ProfileImageFetched,
and UserProfileLoaded are losing the originating request key (group_id / url /
pubkey); modify those Action enum variants to carry the original request key
alongside the payload (e.g., MessagesLoaded(group_id, Vec<Value>),
ProfileImageFetched(url, Vec<u8>), UserProfileLoaded(pubkey, Profile)), update
every place that constructs these variants in the async handling paths to
include the key, and update the reducer/handlers that match on these variants to
check the carried key against the currently active key before applying state
changes to avoid stale responses overwriting current data.
In `@src/main.rs`:
- Around line 953-960: The branch handling Effect::ShowUserProfile currently
dispatches the raw wn::exec payload into Action::UserProfileLoaded; instead,
normalize/flatten that payload the same way the self-profile loader does so
src/screen/profile.rs can read top-level name, display_name, about, picture,
nip05, lud16, and npub with fallbacks. After wn::exec returns Ok(val),
extract/compute top-level fields (prefer direct top-level values, then metadata
subfields, and ensure npub falls back to the requested account key) and build a
normalized struct/object matching what the self-profile loader produces, then
dispatch Action::UserProfileLoaded(normalized) (preserve the Err path to
Action::UserProfileError).
- Around line 810-816: The code currently pipes the entire curl output straight
into Event::Action(Action::ProfileImageFetched(output.stdout)), allowing
arbitrarily large bodies; fix by enforcing a hard size limit before sending:
after the tokio::process::Command::new("curl") call and when matching Ok(output)
(the branch that currently checks output.status.success() &&
!output.stdout.is_empty()), check output.stdout.len() against a defined
MAX_PROFILE_IMAGE_BYTES constant (e.g., 1_000_000 bytes) and only send
ProfileImageFetched if length <= MAX_PROFILE_IMAGE_BYTES; otherwise log/drop the
image and return an error branch. Also consider adding curl-level protection
(e.g., --max-filesize) to the args list as an extra safeguard. Ensure references
to tokio::process::Command::new("curl"), the result match block, and
Event::Action(Action::ProfileImageFetched(...)) are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 892e3b8f-bc40-4d6f-abd4-b596776c3937
📒 Files selected for processing (8)
README.mdsrc/action.rssrc/app.rssrc/main.rssrc/screen/main_screen.rssrc/screen/profile.rssrc/util.rssrc/widget/message_list.rs
- Download, decrypt, and display inline images in chat messages
- Open full-size image popup with 'o' key on selected message
- Upload media via file browser popup ('U' key) with directory navigation
- Reply to messages ('R' key) with quoted context in composer
- Delete own messages ('d' key) with confirmation
- Message selection highlights with '>' marker and visual feedback
- Auto-download image attachments when messages load or arrive
- Distinct download state indicators (downloading/loading/failed with reason)
- Fail explicitly on unexpected media download responses instead of
producing garbage file paths
- Fix README links and build instructions for whitenoise-rs rename
jgmontoya
force-pushed
the
feat/profile-update-and-others
branch
from
March 8, 2026 20:32
59b9b92 to
14ab906
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main.rs (1)
953-963:⚠️ Potential issue | 🟠 MajorNormalize
ShowUserProfiledata to matchLoadProfileshape.This forwards raw
users showoutput without flattening metadata fields to the top level. TheLoadProfileeffect at lines 727-748 performs this normalization (mergingmetadataobject fields to top level), butShowUserProfiledoesn't, causing inconsistent rendering inprofile.rswhere fields likename,picture, etc. will show "(not set)".🔧 Proposed fix to normalize the profile data
Effect::ShowUserProfile { account, pubkey } => { let tx = tx.clone(); tokio::spawn(async move { let action = match wn::exec(&["--account", &account, "users", "show", &pubkey]).await { - Ok(val) => Action::UserProfileLoaded(val), + Ok(val) => { + let mut profile = val.clone(); + if let Some(meta) = val.get("metadata").and_then(|m| m.as_object()) { + for (k, v) in meta { + profile[k.clone()] = v.clone(); + } + } + if profile.get("npub").is_none() { + if let Some(pk) = val.get("pubkey").and_then(|v| v.as_str()) { + profile["npub"] = serde_json::Value::String(pk.to_string()); + } + } + Action::UserProfileLoaded(profile) + } Err(e) => Action::UserProfileError(e.to_string()), }; let _ = tx.send(Event::Action(action)); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 953 - 963, Effect::ShowUserProfile currently forwards the raw wn::exec output into Action::UserProfileLoaded without flattening the nested metadata object, causing fields like name/picture to be missing compared to the LoadProfile path; update the ShowUserProfile handler (the tokio::spawn block that calls wn::exec) to mirror the LoadProfile normalization: parse the returned profile value, merge/hoist any fields from its "metadata" object into the top-level profile map (overwriting or setting top-level keys like "name", "picture", etc.), then send Action::UserProfileLoaded with the normalized shape (and keep the same error path to Action::UserProfileError on Err).
🧹 Nitpick comments (3)
src/main.rs (2)
536-591: Consider logging or limiting DM invite name resolution.The sequential
wn::execcalls for each DM invite work correctly but could be slow with many pending DM invites. This is likely acceptable for the invite listing use case, but consider adding a log entry or limiting resolution to the first N invites if users report sluggish invite loading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 536 - 591, The DM invite name resolution loop performs sequential wn::exec calls for every invite which can be slow; update the block that handles Ok(serde_json::Value::Array(mut arr)) (the loop over arr that calls wn::exec and then returns Action::InvitesLoaded) to either (a) log a debug/info message when many invites are being resolved and include a count, or (b) cap resolution to the first N invites (e.g., resolve only for the first N entries of arr) and leave others unchanged; ensure you reference wn::exec in the log or capping logic and preserve returning Action::InvitesLoaded(arr) after modification so behavior remains consistent.
1019-1035: Consider adding a file size check before reading media files.
LoadMediaImagereads the entire file into memory withtokio::fs::read()before sending bytes. WhileLoadMediaPopupbenefits fromimage::load_from_memory()limits (512 MiB allocation),LoadMediaImagesends raw bytes without decoding. A large file could exhaust memory before the image crate's limits apply.💡 Optional size guard
Effect::LoadMediaImage { file_hash, file_path, } => { let tx = tx.clone(); tokio::spawn(async move { + // Optional: check file size before reading + const MAX_MEDIA_BYTES: u64 = 50 * 1024 * 1024; // 50 MiB + if let Ok(meta) = tokio::fs::metadata(&file_path).await { + if meta.len() > MAX_MEDIA_BYTES { + send_log(&tx, format!("Media file too large: {} bytes", meta.len())); + return; + } + } match tokio::fs::read(&file_path).await {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 1019 - 1035, Effect::LoadMediaImage currently calls tokio::fs::read(&file_path) and can load arbitrarily large files into memory; before reading, check the file size with tokio::fs::metadata (or tokio::fs::File::metadata) and enforce a max allowed size (e.g. same cap used elsewhere) and handle violations by sending a log/error (use send_log(&tx, ...)) or an Event::Action error; only call tokio::fs::read when metadata.len() is within the limit, and keep the existing success path that sends Event::Action::MediaImageLoaded { file_hash, bytes } unchanged.src/action.rs (1)
36-36: Consider carrying request identity in success actions to prevent race conditions.
MessagesLoaded,ProfileImageFetched(line 83), andUserProfileLoaded(line 106) drop the request context (group_id, url, pubkey respectively). If the user navigates while requests are in-flight, stale responses could update the wrong chat or profile. This is a known issue from a previous review.For a "Chill" review, this can be deferred if current usage patterns make races unlikely (e.g., UI blocks navigation during loads), but worth addressing if you observe incorrect state updates.
💡 Example for MessagesLoaded
- MessagesLoaded(Vec<Value>), + MessagesLoaded { + group_id: String, + messages: Vec<Value>, + },Then in the reducer, verify
group_idmatchesapp.active_group_idbefore applying.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/action.rs` at line 36, The success actions drop request identity causing stale responses to update the wrong state; update the enum variants MessagesLoaded, ProfileImageFetched, and UserProfileLoaded to carry their request identifiers (e.g., include group_id for MessagesLoaded, url for ProfileImageFetched, and pubkey for UserProfileLoaded), ensure callers that dispatch these actions populate those fields, and in the reducer (where these actions are handled) verify the incoming identifier matches the current expected identifier (e.g., compare MessagesLoaded.group_id to app.active_group_id or analogous checks for url/pubkey) before applying the state update so in-flight responses don’t overwrite the wrong chat/profile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main.rs`:
- Around line 953-963: Effect::ShowUserProfile currently forwards the raw
wn::exec output into Action::UserProfileLoaded without flattening the nested
metadata object, causing fields like name/picture to be missing compared to the
LoadProfile path; update the ShowUserProfile handler (the tokio::spawn block
that calls wn::exec) to mirror the LoadProfile normalization: parse the returned
profile value, merge/hoist any fields from its "metadata" object into the
top-level profile map (overwriting or setting top-level keys like "name",
"picture", etc.), then send Action::UserProfileLoaded with the normalized shape
(and keep the same error path to Action::UserProfileError on Err).
---
Nitpick comments:
In `@src/action.rs`:
- Line 36: The success actions drop request identity causing stale responses to
update the wrong state; update the enum variants MessagesLoaded,
ProfileImageFetched, and UserProfileLoaded to carry their request identifiers
(e.g., include group_id for MessagesLoaded, url for ProfileImageFetched, and
pubkey for UserProfileLoaded), ensure callers that dispatch these actions
populate those fields, and in the reducer (where these actions are handled)
verify the incoming identifier matches the current expected identifier (e.g.,
compare MessagesLoaded.group_id to app.active_group_id or analogous checks for
url/pubkey) before applying the state update so in-flight responses don’t
overwrite the wrong chat/profile.
In `@src/main.rs`:
- Around line 536-591: The DM invite name resolution loop performs sequential
wn::exec calls for every invite which can be slow; update the block that handles
Ok(serde_json::Value::Array(mut arr)) (the loop over arr that calls wn::exec and
then returns Action::InvitesLoaded) to either (a) log a debug/info message when
many invites are being resolved and include a count, or (b) cap resolution to
the first N invites (e.g., resolve only for the first N entries of arr) and
leave others unchanged; ensure you reference wn::exec in the log or capping
logic and preserve returning Action::InvitesLoaded(arr) after modification so
behavior remains consistent.
- Around line 1019-1035: Effect::LoadMediaImage currently calls
tokio::fs::read(&file_path) and can load arbitrarily large files into memory;
before reading, check the file size with tokio::fs::metadata (or
tokio::fs::File::metadata) and enforce a max allowed size (e.g. same cap used
elsewhere) and handle violations by sending a log/error (use send_log(&tx, ...))
or an Event::Action error; only call tokio::fs::read when metadata.len() is
within the limit, and keep the existing success path that sends
Event::Action::MediaImageLoaded { file_hash, bytes } unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 645a7327-f072-4c33-a0b8-3682bafc0ad6
📒 Files selected for processing (9)
README.mdsrc/action.rssrc/app.rssrc/main.rssrc/screen/main_screen.rssrc/screen/profile.rssrc/util.rssrc/widget/chat_list.rssrc/widget/message_list.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/util.rs
- README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI/UX Improvements
Documentation
Chores