Wire kind-1009 message editing into the conversation#152
Conversation
|
Warning Review limit reached
More reviews will be available in 2 hours, 44 minutes, and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds in-place message editing: aggregates edit records into per-message edit state, surfaces edited text in message bubbles and an edit-history sheet, provides composer edit mode and publish path, hides edit events as standalone rows, and exposes a Rust FFI to send edits. ChangesIn-Place Message Editing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Ready to review this PR? Stage has broken it down into 6 individual chapters for you: Chapters generated by Stage for commit f9354a3 on Jun 12, 2026 4:52pm UTC. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt (1)
5738-5745:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEdited captions never render for messages with attachments.
The menu now allows editing any owned kind-9 message, but the media-caption branch still uses
record.plaintextinstead of the edit-aware body. For an image/document message, the edit publishes successfully yet the bubble keeps showing the original caption.💡 Suggested fix
val bodyTextToRender: String? = when { // Deleted/invalidated tombstones show only the // tombstone copy, never an inline image/caption. deleted || invalidated -> displayedBody mediaPendingName != null && !anyConfirmedMedia -> null - anyConfirmedMedia -> record.plaintext.takeIf { it.isNotBlank() } + anyConfirmedMedia -> + editState?.latestText?.takeIf { it.isNotBlank() } + ?: record.plaintext.takeIf { it.isNotBlank() } else -> displayedBody }Also applies to: 5822-5841
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt` around lines 5738 - 5745, The media-caption branch uses record.plaintext (old caption) so edited captions aren’t shown; update the anyConfirmedMedia branch inside the bodyTextToRender when-expression to use the edit-aware displayedBody (or same edit-resolved value used elsewhere) instead of record.plaintext, and make the identical change at the other occurrence referenced (the block around lines 5822-5841) to ensure edited captions render for messages with attachments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt`:
- Around line 6442-6455: When entering edit mode (LaunchedEffect block that sets
preEditDraft/text) you must prevent normal draft persistence and send completion
side effects from running while editing: stop onDraftChange from updating the
persisted draft and prevent submit logic from clearing the persisted draft or
invoking onAfterSend when editingMessageId != null; instead keep changes only in
the transient text/preEditDraft and, on cancel or successful edit-submit,
restore preEditDraft and only then re-enable normal draft persistence and call
onAfterSend if appropriate. Apply this guard around the onDraftChange handler
and the submit/clear logic (the code that clears stored drafts and calls
onAfterSend) referenced by editingMessageId and the LaunchedEffect, and mirror
the same behavior in the other similar blocks mentioned (the sections around the
other LaunchedEffect usages).
---
Outside diff comments:
In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt`:
- Around line 5738-5745: The media-caption branch uses record.plaintext (old
caption) so edited captions aren’t shown; update the anyConfirmedMedia branch
inside the bodyTextToRender when-expression to use the edit-aware displayedBody
(or same edit-resolved value used elsewhere) instead of record.plaintext, and
make the identical change at the other occurrence referenced (the block around
lines 5822-5841) to ensure edited captions render for messages with attachments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 832fa244-02c4-47ed-b297-7845920c4ef1
⛔ Files ignored due to path filters (4)
app/src/main/jniLibs/arm64-v8a/libmarmot_uniffi.sois excluded by!**/*.soapp/src/main/jniLibs/armeabi-v7a/libmarmot_uniffi.sois excluded by!**/*.soapp/src/main/jniLibs/x86/libmarmot_uniffi.sois excluded by!**/*.soapp/src/main/jniLibs/x86_64/libmarmot_uniffi.sois excluded by!**/*.so
📒 Files selected for processing (17)
app/src/main/java/dev/ipf/darkmatter/core/MessageEdits.ktapp/src/main/java/dev/ipf/darkmatter/core/MessageProjector.ktapp/src/main/java/dev/ipf/darkmatter/state/Controllers.ktapp/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.ktapp/src/main/java/dev/ipf/marmotkit/marmot_uniffi.ktapp/src/main/res/values-b+zh+Hant/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-pt/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-tr/strings.xmlapp/src/main/res/values-zh/strings.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/dev/ipf/darkmatter/LocalizationResourceTest.ktapp/src/test/java/dev/ipf/darkmatter/core/MessageEditsTest.kt
|
@coderabbitai full review please |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt (1)
4158-4170:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFilter edit records before they enter the lazy list.
return@itemsskips composition, but it does not remove the lazy-list slot. WithverticalArrangement = Arrangement.spacedBy(8.dp), each kind-1009 record can still leave a visible gap, so edits still affect transcript layout even though the bubble is hidden. The safer fix is to derive one rendered timeline and use that same filtered list for theLazyColumnand any index/scroll math that targets visible rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt` around lines 4158 - 4170, The lazy list is still reserving slots for edit records because you call return@items inside items; instead, filter out edits before composing the LazyColumn so they never enter the list. Create a derived list (e.g. val renderedTimeline = controller.timeline.filterNot { MessageProjector.isEdit(it.record) }) and use renderedTimeline (with items(renderedTimeline, key = { it.id })) for the LazyColumn and any index/scroll calculations (such as the unread-divider logic and entryFirstUnreadMessageId lookups) so visible row indexes and spacing match the rendered rows.
♻️ Duplicate comments (1)
app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt (1)
6547-6560:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep edit mode out of normal draft persistence and send side effects.
While
editingMessageId != null, keystrokes still flow throughonDraftChange(...), and submit still clears the stored draft plus runsonAfterSend(). That leaves the persisted draft wrong after cancel/submit and still triggers the “scroll to newest” path for an in-place edit.Proposed fix
OutlinedTextField( value = text, - onValueChange = { - text = it - onDraftChange(it) + onValueChange = { value -> + text = value + if (editingMessageId == null) { + onDraftChange(value) + } }, modifier = Modifier.weight(1f), placeholder = { Text(stringResource(R.string.message)) }, maxLines = 5, @@ FloatingActionButton( onClick = { if (text.isNotBlank()) { + val sendingEdit = editingMessageId != null onSend(text) - text = "" - onDraftChange("") - onAfterSend() + if (!sendingEdit) { + text = "" + onDraftChange("") + onAfterSend() + } } },Also applies to: 6663-6693
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt` around lines 6547 - 6560, When entering edit mode (editingMessageId != null) stop routing keystrokes and send-side effects through the normal draft persistence and post-send path: add an early guard in the draft change handler (the onDraftChange/handler that writes persisted drafts) to return immediately when editingMessageId != null so typing only updates the local text and preEditDraft, and modify the submit/send handler (the function that clears persisted draft and calls onAfterSend) to branch on editingMessageId — when non-null do not clear the persisted draft or call onAfterSend but instead run the edit-commit flow (update message/edit state) and then restore/clear preEditDraft as appropriate; apply the same guard/branching to any other places that trigger scroll-to-newest or persistence (the other LaunchedEffect/submit-like path referenced in the diff) so edits do not affect normal draft persistence or trigger send-side effects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt`:
- Around line 6117-6158: The edit history sheet currently lays out all entries
in a plain Column causing unbounded content to overflow and become unreachable;
replace the inner static Column that iterates rows with a scrollable list (e.g.,
LazyColumn or Column + verticalScroll) inside ModalBottomSheet so older
revisions can be scrolled to. Locate the current ModalBottomSheet block and
change the child that contains rows.forEach to a LazyColumn (using items or
itemsIndexed on rows) or wrap the Column with
Modifier.verticalScroll(rememberScrollState()) and ensure proper padding/spacing
is preserved (keep the Texts that use IdentityFormatter.relativeTime and
stringResource unchanged).
---
Outside diff comments:
In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt`:
- Around line 4158-4170: The lazy list is still reserving slots for edit records
because you call return@items inside items; instead, filter out edits before
composing the LazyColumn so they never enter the list. Create a derived list
(e.g. val renderedTimeline = controller.timeline.filterNot {
MessageProjector.isEdit(it.record) }) and use renderedTimeline (with
items(renderedTimeline, key = { it.id })) for the LazyColumn and any
index/scroll calculations (such as the unread-divider logic and
entryFirstUnreadMessageId lookups) so visible row indexes and spacing match the
rendered rows.
---
Duplicate comments:
In `@app/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.kt`:
- Around line 6547-6560: When entering edit mode (editingMessageId != null) stop
routing keystrokes and send-side effects through the normal draft persistence
and post-send path: add an early guard in the draft change handler (the
onDraftChange/handler that writes persisted drafts) to return immediately when
editingMessageId != null so typing only updates the local text and preEditDraft,
and modify the submit/send handler (the function that clears persisted draft and
calls onAfterSend) to branch on editingMessageId — when non-null do not clear
the persisted draft or call onAfterSend but instead run the edit-commit flow
(update message/edit state) and then restore/clear preEditDraft as appropriate;
apply the same guard/branching to any other places that trigger scroll-to-newest
or persistence (the other LaunchedEffect/submit-like path referenced in the
diff) so edits do not affect normal draft persistence or trigger send-side
effects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 567d5af9-2540-418e-b404-a3dd34f8d928
⛔ Files ignored due to path filters (4)
app/src/main/jniLibs/arm64-v8a/libmarmot_uniffi.sois excluded by!**/*.soapp/src/main/jniLibs/armeabi-v7a/libmarmot_uniffi.sois excluded by!**/*.soapp/src/main/jniLibs/x86/libmarmot_uniffi.sois excluded by!**/*.soapp/src/main/jniLibs/x86_64/libmarmot_uniffi.sois excluded by!**/*.so
📒 Files selected for processing (5)
app/src/main/java/dev/ipf/darkmatter/core/MessageProjector.ktapp/src/main/java/dev/ipf/darkmatter/state/Controllers.ktapp/src/main/java/dev/ipf/darkmatter/ui/DarkMatterApp.ktapp/src/main/java/dev/ipf/marmotkit/marmot_uniffi.ktapp/src/test/java/dev/ipf/darkmatter/core/MessageEditsTest.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/test/java/dev/ipf/darkmatter/core/MessageEditsTest.kt
- app/src/main/java/dev/ipf/darkmatter/state/Controllers.kt
- app/src/main/java/dev/ipf/marmotkit/marmot_uniffi.kt
- app/src/main/java/dev/ipf/darkmatter/core/MessageProjector.kt
… the lazy list, refined history sheet, caret-at-end on entry
Wires kind-1009 message editing into the Android client. Sender taps Edit on their own kind-9 chat, the composer pre-fills with the message's current text, submitting publishes a kind-1009 edit which the timeline picks up and the bubble rebinds to in place.
What changes
.soregenerated across all four ABIs andmarmot_uniffi.ktre-emitted so the newMarmot.editMessage(...)FFIsurface is callable from Kotlin.
MessageEdits— pure aggregator that walks the timeline, groupskind-1009 events by their target message id (single
etag per spec),applies client-side authorship enforcement (edit signer must match
target signer, case-insensitive on the hex pubkey), drops blank
replacements, and orders versions chronologically. Returns a
target → EditState(latestText, count, versions[])map.MessageProjector—KindEdit = kind-1009constant,isEditpredicate,
editTargetMessageIdextractor.Controllers—projectedPreviewTextskips kind-1009 (in-placeedits must never bump the chat-list "last message" or reorder the
conversation, per the Rust runtime's docstring on
edit_message).Unread-count helpers (
firstUnreadReceivedIndex,countUnreadIncoming) skip derived-state kinds (1009 edits + 1210group system events) so an edit doesn't inflate the badge or shift the
read anchor away from real chat.
ConversationController—editsByTargetmap recomputed on everytimeline publish;
editingMessageIdmutable state;editMessage()posts the kind-1009 via
marmotIo;displayedText(record)returnsthe latest edited text when one exists. Submitting the composer with
edit-mode active short-circuits the normal send path and routes through
editMessage().MessageActionMenu— new "Edit" action button gated oncanEdit = mine && record.kind == 9uL && record.messageIdHex.isNotBlank() && !deleted.ComposerBar—editingMessageId+editingInitialText+onCancelEditparameters. Snapshots the in-flight draft on edit-mode entry so cancel
restores it; pre-fills the input with the message's current text on
entry. Edit banner ("Editing message" with cancel-X) replaces the
reply banner when edit mode is active (mutually exclusive).
controller.editsByTargetso any edit echoes into the rendered text in place; reply previews
read through the same path automatically.
bubble timestamp opens a
ModalBottomSheetlisting each version with itsrelative timestamp, newest first, with the original at the bottom.
MessageProjector.isEditskip in the bubble timelineiteration so kind-1009 events never render as a standalone bubble (same
derived-state trap kind-1210 hit at last review).
Test plan
./gradlew :app:testDebugUnitTest :app:ktlintMainSourceSetCheck :app:assembleDebug— greenMessageEditsTestcases (empty, single edit, multiple, authorship rejection, case-insensitive hex match, blank rejected, missing target, multi-target)Depends on
marmot-protocol/darkmatter#299— the uniffiedit_messageexport plus the storage-sqlite materialization of kind-1009 into the message timeline. Without that storage fix the FFI accepts the send but the row never reaches the timeline subscription. The.so+ Kotlin binding in this branch are regenerated fromfeature/uniffi-edit-messageand carry that fix.Summary by CodeRabbit
New Features
Localization
Tests