mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 08:04:45 +00:00
fix(edits): unlink deleted edits from their message; one collector per row
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013HdLnAa4Pa1pFV9FTYVTB6
This commit is contained in:
@@ -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<Note>) {
|
||||
nextToBeRemoved.forEach { note -> unlinkAndRemove(note) }
|
||||
}
|
||||
|
||||
@@ -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 }
|
||||
}
|
||||
|
||||
+28
-10
@@ -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<Note?>(initialValue = null, note.idHex) {
|
||||
note.flow().edits.stateFlow.collect {
|
||||
value = note.latestConcordEdit() ?: note.latestBuzzEdit()
|
||||
}
|
||||
}
|
||||
return latest
|
||||
}
|
||||
|
||||
-23
@@ -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<Note?>(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)"
|
||||
|
||||
-31
@@ -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<Note?>(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)"
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -163,6 +163,7 @@ open class Note(
|
||||
removeReply(note)
|
||||
removeBoost(note)
|
||||
removeReaction(note)
|
||||
removeEdit(note)
|
||||
removeZap(note)
|
||||
removeZapPayment(note)
|
||||
removeReport(note)
|
||||
|
||||
Reference in New Issue
Block a user