From 893a270c65e91c0e2f0379e2f384e85a23f1010c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 20:18:53 +0000 Subject: [PATCH] fix(edits): unlink deleted edits from their message; one collector per row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit of the three edit paths (feed 1010 / Buzz 40003 / Concord 3302) found: 1. Bug (feed regression): a deleted edit kept overlaying its message. Edits anchor on the target's Note.edits with no `replyTo` back-link, and removeNote didn't cover `edits`, so unlinkAndRemove never dropped them — the old cache-scan resolver dropped deleted edits for free, Note.edits did not. Fix: removeNote now also removeEdit()s, and unlinkAndRemove resolves the edit's `e`-tag target and unlinks it there (editedTargetIdOf covers all three kinds). New test: deleting an edit un-overlays and unlinks it. 2. Perf: every chat row ran two edits-flow collectors (observeConcordEdit + observeBuzzEdit). A message is only ever one kind, so they're merged into a single observeChatEdit that resolves latestConcordEdit() ?: latestBuzzEdit() — one collector per row, dispatched by the winning edit's event type. 3. Nits: latestBuzzEdit now tie-breaks by idHex (deterministic on same-second edits, matching Concord); dropped a redundant takeIf in latestConcordEdit. The author check stays at read time on purpose: an edit can be consumed before its target loads (author unknown), so an attach-time gate would wrongly drop early-arriving legit edits. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_013HdLnAa4Pa1pFV9FTYVTB6 --- .../amethyst/model/LocalCache.kt | 14 +++++++ .../amethyst/model/NoteEditOverlays.kt | 4 +- .../loggedIn/chats/feed/ChatMessageCompose.kt | 38 ++++++++++++++----- .../chats/feed/types/RenderBuzzNotes.kt | 23 ----------- .../chats/feed/types/RenderConcordEdits.kt | 31 --------------- .../model/BuzzWorkspaceChannelTest.kt | 22 +++++++++++ .../amethyst/commons/model/Note.kt | 1 + 7 files changed, 67 insertions(+), 66 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index c1c5bf1952..99fa220f12 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -3567,6 +3567,11 @@ object LocalCache : ILocalCache, ICacheProvider { getNoteIfExists(quotedId)?.removeBoost(note) } + // Edits (1010/3302/40003) are anchored on their target's Note.edits and carry no `replyTo` + // back-link, so the unlink above can't reach them — resolve the target by the edit's `e` tag + // and drop it there, or a deleted edit would keep overlaying its message. + editedTargetIdOf(noteEvent)?.let { getNoteIfExists(it)?.removeEdit(note) } + if (noteEvent is ReportEvent) { noteEvent.reportedAuthor().forEach { getUserIfExists(it.pubkey)?.reportsOrNull()?.let { reports -> @@ -3605,6 +3610,15 @@ object LocalCache : ILocalCache, ICacheProvider { refreshDeletedNoteObservers(note) } + /** The id of the message/post an edit event targets (its `e` tag), across all three edit kinds. */ + private fun editedTargetIdOf(event: Event?): HexKey? = + when (event) { + is TextNoteModificationEvent -> event.editedNote()?.eventId + is ConcordChatEditEvent -> event.editedMessageId() + is StreamMessageEditEvent -> event.editedMessage() + else -> null + } + fun unlinkAndRemove(nextToBeRemoved: List) { nextToBeRemoved.forEach { note -> unlinkAndRemove(note) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/NoteEditOverlays.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/NoteEditOverlays.kt index aebd5bdb24..a460743c5b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/NoteEditOverlays.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/NoteEditOverlays.kt @@ -56,7 +56,8 @@ fun Note.latestBuzzEdit(): Note? { val authorHex = author?.pubkeyHex ?: return null return edits .filter { it.event is StreamMessageEditEvent && it.author?.pubkeyHex == authorHex } - .maxByOrNull { it.createdAt() ?: 0L } + // idHex tie-break so a same-second pair resolves identically on every client. + .maxWithOrNull(compareBy({ it.createdAt() ?: 0L }, { it.idHex })) } /** The kind-3302 Concord edit overlaying this message, or null — author-only, newest by CORD-02 §4 send time. */ @@ -65,5 +66,4 @@ fun Note.latestConcordEdit(): Note? { return edits .filter { it.author?.pubkeyHex == authorHex && it.event is ConcordChatEditEvent } .maxWithOrNull(compareBy({ (it.event as ConcordChatEditEvent).orderingMs() }, { it.idHex })) - ?.takeIf { it.event != null } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatMessageCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatMessageCompose.kt index dc3a8108fa..fe5f68d670 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatMessageCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatMessageCompose.kt @@ -46,6 +46,8 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.model.Note +import com.vitorpamplona.amethyst.model.latestBuzzEdit +import com.vitorpamplona.amethyst.model.latestConcordEdit import com.vitorpamplona.amethyst.ui.components.LocalInlineQuoteRenderer import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor @@ -72,13 +74,13 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.RenderMarm import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.RenderRegularTextNote import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.hasMip04Media import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.isBuzzActivityRow -import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.observeBuzzEdit -import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.types.observeConcordEdit import com.vitorpamplona.amethyst.ui.theme.ReactionRowZapraiser import com.vitorpamplona.amethyst.ui.theme.StdVertSpacer import com.vitorpamplona.quartz.buzz.forum.ForumVoteEvent import com.vitorpamplona.quartz.buzz.stream.StreamMessageDiffEvent +import com.vitorpamplona.quartz.buzz.stream.StreamMessageEditEvent import com.vitorpamplona.quartz.buzz.stream.SystemMessageEvent +import com.vitorpamplona.quartz.concord.cord03Channels.ConcordChatEditEvent import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip04Dm.messages.PrivateDmEvent import com.vitorpamplona.quartz.nip10Notes.BaseNoteEvent @@ -615,17 +617,33 @@ fun NoteRow( note.event is ChatMessageEncryptedFileHeaderEvent -> RenderEncryptedFile(note, bgColor, accountViewModel, nav) hasMip04Media(note.event) -> RenderMarmotEncryptedMedia(note, bgColor, accountViewModel, nav) else -> { - // Concord and Buzz channels overlay edits on their messages (kind-1010 and - // kind-40003 respectively): when one exists, render the newest edit's content - // instead of the stale original. Both are null for every other chat surface. - val concordEdit = observeConcordEdit(note) - val buzzEdit = observeBuzzEdit(note) - when { - concordEdit != null -> RenderConcordEditedNote(note, concordEdit, canPreview, innerQuote, bgColor, accountViewModel, nav) - buzzEdit != null -> RenderBuzzEditedNote(note, buzzEdit, canPreview, innerQuote, bgColor, accountViewModel, nav) + // Concord and Buzz channels overlay edits on their messages (kind-3302 and + // kind-40003): when one exists, render the newest edit's content instead of the + // stale original. One observer for both — a message is only ever one kind, so a + // single edits-flow collector per row covers both (and is null for other surfaces). + val edit = observeChatEdit(note) + when (edit?.event) { + is ConcordChatEditEvent -> RenderConcordEditedNote(note, edit, canPreview, innerQuote, bgColor, accountViewModel, nav) + is StreamMessageEditEvent -> RenderBuzzEditedNote(note, edit, canPreview, innerQuote, bgColor, accountViewModel, nav) else -> RenderRegularTextNote(note, canPreview, innerQuote, bgColor, accountViewModel, nav) } } } } } + +/** + * The newest edit overlaying a chat message [note] (Concord kind-3302 or Buzz kind-40003), or null + * when unedited. A message is only ever one kind, so both resolve off the same [Note.edits] and one + * collector on the note's edits flow serves both — recomposing whenever an edit is added or removed. + */ +@Composable +fun observeChatEdit(note: Note): Note? { + val latest by + produceState(initialValue = null, note.idHex) { + note.flow().edits.stateFlow.collect { + value = note.latestConcordEdit() ?: note.latestBuzzEdit() + } + } + return latest +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderBuzzNotes.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderBuzzNotes.kt index 9e9c7d3371..2afe30722f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderBuzzNotes.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderBuzzNotes.kt @@ -30,8 +30,6 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.MutableState -import androidx.compose.runtime.getValue -import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -43,7 +41,6 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.model.EmptyTagList import com.vitorpamplona.amethyst.commons.model.toImmutableListOfLists import com.vitorpamplona.amethyst.model.Note -import com.vitorpamplona.amethyst.model.latestBuzzEdit import com.vitorpamplona.amethyst.ui.components.TranslatableRichTextViewer import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel @@ -64,26 +61,6 @@ import com.vitorpamplona.quartz.buzz.stream.StreamMessageDiffEvent import com.vitorpamplona.quartz.buzz.stream.SystemMessageEvent import com.vitorpamplona.quartz.nip01Core.core.Event -/** - * Observes the newest kind-40003 edit overlaying [note], recomposing when new edits - * arrive. Returns null when the message is unedited. - * - * Each edit is anchored on the message it edits ([Note.edits], where LocalCache consumes it), - * so it is held for as long as its message and read straight off the note — no channel-keyed side - * store. [Note.latestBuzzEdit] applies only the original author's newest edit; [addEdit] - * invalidates the note's edits flow, so collecting it re-runs the fold. - */ -@Composable -fun observeBuzzEdit(note: Note): Note? { - val latest by - produceState(initialValue = null, note.idHex) { - note.flow().edits.stateFlow.collect { - value = note.latestBuzzEdit() - } - } - return latest -} - /** * A Buzz stream message whose content has been superseded by a kind-40003 edit: * renders the NEWEST edit's content (never the stale original) plus an "(edited)" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderConcordEdits.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderConcordEdits.kt index 7337f084dd..5004e87e6b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderConcordEdits.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/types/RenderConcordEdits.kt @@ -25,8 +25,6 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.MutableState -import androidx.compose.runtime.getValue -import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -34,41 +32,12 @@ import androidx.compose.ui.unit.sp import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.model.EmptyTagList import com.vitorpamplona.amethyst.commons.model.toImmutableListOfLists -import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.amethyst.model.Note -import com.vitorpamplona.amethyst.model.latestConcordEdit import com.vitorpamplona.amethyst.ui.components.TranslatableRichTextViewer import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes -/** - * Observes the newest kind-3302 Concord edit overlaying a Concord chat message [note], - * recomposing when a new edit lands. Returns null for non-Concord messages or an - * unedited one. - * - * Concord edits ride the encrypted channel plane (unlike public feed edits, there is no - * relay subscription to start — the session decrypts the wrap and lands the kind-3302 - * rumor in [LocalCache] itself). Each edit is attached to the message it edits ([Note.edits], - * like a reaction to its target), so it is held for exactly as long as the channel-retained - * message — a Concord rumor is decrypted once per session and can't be re-downloaded, so it - * must not be left orphaned in the soft cache. We recompute from that list whenever it changes. - * Only edits authored by the original message's author are applied — so a member can't rewrite - * someone else's message — and the latest by CORD-02 §4 send time wins. - */ -@Composable -fun observeConcordEdit(note: Note): Note? { - // `addEdit` invalidates this flow, so collecting it re-runs the fold on each new edit. A non-Concord - // message simply has no kind-3302 edits, so [Note.latestConcordEdit] returns null for it. - val latest by - produceState(initialValue = null, note.idHex) { - note.flow().edits.stateFlow.collect { - value = note.latestConcordEdit() - } - } - return latest -} - /** * A Concord chat message whose content has been superseded by a kind-3302 edit: * renders the NEWEST edit's content (never the stale original) plus an "(edited)" diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt index 03371a205a..6632a51f0d 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/model/BuzzWorkspaceChannelTest.kt @@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.buzz.stream.StreamMessageV2Event import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent import com.vitorpamplona.quartz.nip29RelayGroups.GroupId import io.mockk.every import io.mockk.mockk @@ -208,6 +209,27 @@ class BuzzWorkspaceChannelTest { assertEquals("the fix", target.latestBuzzEdit()?.event?.content) } + @Test + fun deletingAnEditUnlinksItFromTheMessage() = + runBlocking { + val channelId = newChannelId() + val original = streamMessage(channelId, "typo") + LocalCache.checkDeletionAndConsume(original, buzzRelay, false) + val edit = signer.sign(StreamMessageEditEvent.build(channelId, original.id, "fixed", createdAt = original.createdAt + 5)) + LocalCache.checkDeletionAndConsume(edit, buzzRelay, false) + + val target = LocalCache.getNoteIfExists(original.id)!! + assertEquals("fixed", target.latestBuzzEdit()?.event?.content) + + // The author deletes their own edit (NIP-09). It must stop overlaying the message and + // be unlinked from Note.edits, not linger as a stale overlay. + val deletion = signer.sign(DeletionEvent.build(listOf(edit))) + LocalCache.checkDeletionAndConsume(deletion, buzzRelay, false) + + assertNull("a deleted edit must no longer overlay its message", target.latestBuzzEdit()) + assertTrue("the deleted edit is unlinked from the message", target.edits.none { it.idHex == edit.id }) + } + @Test fun pruningAMessageReleasesItsEdits() = runBlocking { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index 9eb40f6176..4a97a79451 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -163,6 +163,7 @@ open class Note( removeReply(note) removeBoost(note) removeReaction(note) + removeEdit(note) removeZap(note) removeZapPayment(note) removeReport(note)