Repository navigation
feat(mobile): mark edited messages after the body - #8863
Conversation
This comment has been minimized.
This comment has been minimized.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blockers at 5276d96. I found nothing here that should stop the merge. Mobile CI is green. Three low or optional notes are inline.
What we checked:
- Integration: all six
MessageContentcallers. Both surfaces that used to show the header marker now passedited, and togglingeditedchanges the render key. - Parser: a probe through the real
MessageContentfor unclosed inline code, math delimiters, links, mentions, HTML,~~~, setext, lists, hard breaks and CRLF. Each produced exactly one marker when edited and none when not. Authored U+E0ED is stripped before rendering, and copy text is unchanged. - Layout: at 220px wide, wide tables, long code lines and unbreakable words threw no layout exceptions in own or received bubbles. Big emoji and headings keep the 13.1sp, w400 marker.
Not covered: live delivery of an edit after the page mounts (the journey seeds edits before mounting), iOS, and VoiceOver.
76adc93 to
89dce35
Compare
…#8870) ## Summary In the mobile thread view, if a reply was edited before you opened the thread, the reply showed its original text and no "(edited)" marker. An edit that arrived while the thread was open worked. The thread `/query` asked only for message kinds. The channel window carries edits, reactions and deletions (aux events) only for top-level messages, so an earlier edit to a reply was in neither result. The thread query now sends `include_aux: true`, as desktop does (`buzz-app` `src/features/relay/threads.ts`). The relay already supports this on the `depth_limit` thread route: it returns the aux events for the root and every reply, plus deletions of those (`crates/buzz-relay/src/api/bridge.rs`, `bridge_thread_aux`). A relay that ignores the flag gives the same result as before. The relay sends a page's rows (at most 200 reply-marked events, of any kind) before that page's aux events. It also repeats the root's aux on every page. So `fetchCompleteThread` treats the leading run of events reply-marked into this thread, other than reactions and NIP-09 deletions, as the page's rows. Only rows count toward a page, move the cursor and feed the reply cache. A reply-marked huddle event still fills a page, and a reply-marked reaction never becomes a reply. A reply-marked edit of the root in the repeated aux can follow a short final page's rows and look like a full page, so the scan also stops when the next cursor does not advance. The thread-summary scan (`fetchCompleteThreadReplies`) still asks for replies only and pages exactly as on `main`. ### Related issue Found during the emulator testing of #8863. ### Testing - `just mobile-check`, `just file-size-check` and `just mobile-test` pass. - `thread_replies_provider_test.dart` adds two tests. The first checks that the query sends `include_aux` and that an edit made before the thread opens folds into its reply. The second covers pagination: a reply-marked huddle row filling a page, a reply-marked edit that is a row on one page and aux on the others, a reply-marked reaction that stays aux, 200 reply-marked reactions after 199 replies (one query), 200 reply-marked root edits after a short final page (the progress guard ends the scan), an edit reply-marked into another thread that stays aux, and that the summary scan doesn't send `include_aux`. It fails if any of these is removed: the reaction/deletion exclusion, the root check, the 200 cap, the progress guard, or the separate row and aux id sets. - `channel_detail_page_test.dart` adds a journey: it opens `ThreadDetailPage` with the real thread query and a fake relay session that sends the reply's earlier edit only when `include_aux` is set. It checks the edited text, that the original text is gone, and the marker. It fails when the flag is removed or when the provider drops aux. - Android emulator (AVD, debug build) against a local relay. Morty posts "Who can review the release notes?", Demo User replies "I can, after lunhc" and then edits the reply to "I can, after lunch", all before the thread is opened. These shots come from the first revision of this branch. The pagination changes after it give the same result for a one-page thread like this one. | Before (unfixed: the #8863 build) | After (this branch) | Live edit while the thread is open | | --- | --- | --- | | <img src="https://raw.githubusercontent.com/block/buzz/4daefdf9a427c66dd80edcc59ee664c1da63824c/pr-8870--thread-before.png" width="300" alt="Before: original reply text, no marker"> | <img src="https://raw.githubusercontent.com/block/buzz/4daefdf9a427c66dd80edcc59ee664c1da63824c/pr-8870--thread-after.png" width="300" alt="After: edited reply text with (edited)"> | <img src="https://raw.githubusercontent.com/block/buzz/4daefdf9a427c66dd80edcc59ee664c1da63824c/pr-8870--thread-live.png" width="300" alt="Live edit while the thread is open"> | Before, the thread shows the original reply text with no marker. After, it shows the edited text and "(edited)". A live edit still lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Logan Johnson <[email protected]> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <[email protected]>
89dce35 to
d6a9891
Compare
d6a9891 to
fca7e1b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fca7e1b to
79798dd
Compare
This comment has been minimized.
This comment has been minimized.
79798dd to
cf1858d
Compare
Edited messages showed "(edited)" only in the author header, so a grouped follow-up with no header carried no marker at all. Draw it after the body's last line instead, as desktop does (block/buzz-app#2020): inline after prose, on its own line inside the bubble after code, a table, a rule, display math or a bare block marker, and after trailing media outside the bubble. Bodies with a code fence always take the own-line form, since gpt_markdown's fence rules differ from CommonMark. It uses the metadata colour, or the bubble ink on your own accent bubble. Copy still copies the message text. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
Desktop draws the marker on its own line after a heading; mobile joined it inline. Do the same here. A last line that ends in a link also takes the own-line form: links render as widget spans, so a joined marker was read by screen readers before the link. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
Main now shields backslash escapes before rendering (#8925), so a trailing `\]` reaches the marker rule as text, never as display math. Drop that own-line rule; an escaped bracket now takes the inline marker. The thread edit-reload test from #8870 now expects the marker joined to the reply text. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
Two unanchored patterns, a URL scheme and a Markdown link, restarted at every character of the last line, so an edit with a long crafted last line took quadratic time during build: 10 s at 40,000 characters in a test, and minutes at the relay's 256 KiB limit. Codex Security Review reported the scheme case. Check both endings with one scan. A fuzz run of 276,000 short lines gave the same result as the old patterns. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
Main grew message_content.dart to 1169 lines (#9259), and the edited marker took it past the 1200-line ceiling. Move the marker's layout helpers into edited_marker.dart and keep main's trailing-gallery code in place, adding the marker as its last child. No behavior change. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
cf1858d to
4cde2c1
Compare
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent — integrated review of 44b3c4823f0cf1f7715f85dbf5566cb64e848162..4cde2c12991472b1fccb3832c1f9f97c4caa853b.
Verdict: REQUEST CHANGES (P2 content fidelity). The mobile edited marker is correctly wired through channel and thread rows, added at render time rather than persisted, and excluded from copied message text. However, the new renderer silently deletes a valid character from any authored message: mobile/lib/features/channels/message_content.dart:364 calls withoutEditedSlot(markdownContent) regardless of edited, and mobile/lib/features/channels/message_content/edited_marker.dart:11 removes every U+E0ED (). Thus an unedited message containing that character is displayed differently from its event; edited messages lose it too. The widget case in mobile/test/features/channels/message_content_bubble_cases.dart checks only that no synthetic (edited) marker appears for an unedited body containing the slot; it does not assert that the authored character survives.
Author action: Preserve authored characters while distinguishing the renderer's synthetic marker without stripping a reserved code point. Add rendered-widget regressions that assert U+E0ED survives in both edited and unedited bodies and that only actual edits show (edited).
Validation and scope: Independent systems and UI passes traced edit folding, channel/thread rendering, media/Markdown placement, and copy behavior. UI lane reports full just mobile-test, mobile format/analyze, file-size and git diff --check passed at this head; systems lane's git diff --check 44b3c482...HEAD passed on a clean checkout, while local offline flutter pub get timed out there. Live GitHub mobile checks/tests/Android build are green at the stated head. Those passes do not protect the authored-character invariant; the existing widget assertion encodes the omission. No other author-actionable finding was established in the traced paths.
Verification owner: Author for the rendering fix and regressions; reviewer for exact-head follow-up, affected mobile journey, and native/assistive-technology observation. Android screenshots in the PR body are from older 5276d968, not exact-head native proof; iOS/VoiceOver were not observed by this review. This evidence gap is not the reason for requesting changes. A changed head requires a fresh review.
The renderer removed every U+E0ED from message bodies, so an authored private-use character was lost, even in unedited messages. Now the marker component applies only to an edited body without that character. An edited body that has one keeps it and shows "(edited)" on its own line. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
|
🤖 Fixed in 82746fe, in reply to #8863 (review). The renderer no longer removes U+E0ED from authored content. The marker component is applied only to an edited body that has no authored U+E0ED, and |
When an authored U+E0ED was in an earlier part of a message, before an attachment, the marker component was off for the whole message, but the last part still got a slot. That slot drew as a plain character and the edited marker disappeared. The last part now uses the same decision, so the marker takes its own line. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Logan Johnson <[email protected]>
|
🤖 The Codex Security Review LOW finding on 82746fe (authored U+E0ED in an earlier segment, before an attachment) is correct. It is fixed in d4eed93. |
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent — integrated review of 44b3c4823f0cf1f7715f85dbf5566cb64e848162..d4eed93d34edcbfbaf6cc01677e84fd6eb3e1fe4.
Verdict: APPROVE. The previously requested authored-U+E0ED content-loss fix is present, and both systems/content and UI/accessibility review lanes found no remaining author-actionable defect at this head. The unedited and edited rendered-widget regressions preserve the authored character; the additional text → image → trailing-text regression exercises the message-wide marker decision. I checked the intervening patch: MessageContent passes markerSlot derived from the complete content to the last rendered text segment, so the fallback renders a distinct (edited) label rather than a literal sentinel. Copy continues to use message.content, not the synthetic label.
Evidence at d4eed93: the systems lane ran full just mobile-test on a clean tree (4,602 passed, 73 skipped in Flutter; auxiliary suites 4 and 27 passed), then deliberately reverted the production call-site flag and observed the new regression fail before restoring a clean exact-head tree. The UI lane passed focused message_content_test.dart, just mobile-check, and git diff --check on a clean exact-head tree. GitHub checks were settled green when this review was prepared. Relevant seams: mobile/lib/features/channels/message_content.dart:412–415,475–487; message_content/edited_marker.dart:85–102; mobile/test/features/channels/message_content_bubble_cases.dart:426–442; message_actions.dart:157–164.
Verification gap, not author rework: no reviewer observed this exact build on a native device or with iOS VoiceOver/Android TalkBack. Reviewer owns the human mobile/assistive-technology handoff; confirm visual order, spoken order, and copy in channel and thread before treating that journey as verified. Any new head needs a new review.
Co-Authored-By: Claude Opus 5.5 <[email protected]> * origin/main: feat(mobile/ios): show the message's own text in notifications (#9346) feat(mobile): prefill the agents you last tagged in a conversation (#9248) feat(mobile): start a Solo chat from a message or inbox row (#9251) feat(mobile): link to Builderlab web from Settings (#9300) fix(relay): bound invite preview host lookup (#9308) fix(relay): saturate filter limits instead of casting (#9230) fix(mobile): tell same-name agents apart in Solo (#9321) fix(mobile): join an open channel from the chat footer (#9253) fix(relay): fence pending subscriptions on close or replacement (#2382) fix(mobile): hide archived people and agents from choices; give them their own members section (#9320) Add video stacks and native iOS zoom playback (#9316) Support PNG and HEIC photos in the native iOS zoom gallery (#9314) fix(mobile): list known people in recipient pickers before a name is typed (#9311) feat(mobile): mark edited messages after the body (#8863) Signed-off-by: Philip Azar <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]> * origin/main: (22 commits) chore(deps): bump hickory-proto to 0.26.2 for RUSTSEC-2026-0487..0491 (#9351) feat(mobile/ios): show the message's own text in notifications (#9346) feat(mobile): prefill the agents you last tagged in a conversation (#9248) feat(mobile): start a Solo chat from a message or inbox row (#9251) feat(mobile): link to Builderlab web from Settings (#9300) fix(relay): bound invite preview host lookup (#9308) fix(relay): saturate filter limits instead of casting (#9230) fix(mobile): tell same-name agents apart in Solo (#9321) fix(mobile): join an open channel from the chat footer (#9253) fix(relay): fence pending subscriptions on close or replacement (#2382) fix(mobile): hide archived people and agents from choices; give them their own members section (#9320) Add video stacks and native iOS zoom playback (#9316) Support PNG and HEIC photos in the native iOS zoom gallery (#9314) fix(mobile): list known people in recipient pickers before a name is typed (#9311) feat(mobile): mark edited messages after the body (#8863) Add swipeable photo stacks and native iOS gallery preview (#9259) ci(docker): skip relay image publishes for main pushes outside the build context (#9310) Add manual-only relay image publishing (#9303) fix(mobile): bump Flutter to 3.47.9 for Android TLS fix (#9309) Serve video and poster previews for invite links (#9306) ... Signed-off-by: Philip Azar <[email protected]>
Summary
Mobile showed "(edited)" only in the author header. A grouped follow-up message has no header, so an edited follow-up had no marker at all. This ports the placement from desktop block/buzz-app#2020 to the channel and thread message bodies:
Hi how are ya. (edited). This includes a final list item or quote, as on desktop. It is drawn in the timestamp style, with the metadata colour on received messages and the bubble ink on your own messages.```always uses its own line. gpt_markdown's fence rules differ from CommonMark, so putting the marker inside the Markdown could change how the code renders.Out of scope: the mobile forum models (
ForumPost,ThreadReply) have no edited state and don't fold edits, so forum posts still show no marker. Search and inbox previews also show no marker, which matches the preview rules.Related issue
Desktop parity for block/buzz-app#2020. I found no existing mobile issue or PR for this.
Testing
At
5276d968:just mobile-check,just file-size-checkandjust mobile-testall exit 0.edited_marker_test.dart: a table test of which endings join inline and which take their own line, including```npm install```,~~~, indented code, tables, rules, math and bare block markers.message_content_bubble_cases.dart: rendered widget checks. For each risky ending, the edited render is the unedited render plus one(edited)line inside the same text bubble. The tests also cover colours, trailing media, an open<u>and authored slot characters. With the guards removed, this test fails.channel_detail_page_test.dart, edit journey: an edited message and an edited grouped follow-up are marked and an unedited message isn't. Copy text copiesEdited textwithout the marker. Opening the thread marks the edited head and the edited reply. With the thread wiring removed, this test fails.Android emulator (debug build of
5276d968on a local relay, seeded with edited messages):Widget renders (throwaway test, not committed):
🤖 Generated with Claude Code