diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActions.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActions.kt new file mode 100644 index 0000000000..28984d4581 --- /dev/null +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActions.kt @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.actions + +import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle +import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner +import com.vitorpamplona.quartz.nip01Core.tags.people.PTag +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import com.vitorpamplona.quartz.nip10Notes.tags.notify + +/** + * Pure event-building "verbs" for kind:1 short-note replies (NIP-10). + * + * Builds a signed [TextNoteEvent] reply but does NOT publish it. The Amethyst + * Android UI flow does more than these builders — non-UI callers are + * responsible for the rest: + * + * * **Publish.** Hand the returned event to your relay client. Android uses + * `Account.sendMyPublicAndPrivateOutbox`, the desktop deck pipes through + * `dispatch(signed, localCache, relayManager)`, amy uses `Context.publish`. + * * **Writeable check.** Skip the call when the active signer is read-only + * (e.g. an npub-only login). Building will fail at the sign step otherwise. + * * **Parent kind.** Only kind:1 [TextNoteEvent] parents are well-defined here + * — replies to articles / comments belong on the NIP-22 path + * (`CommentEvent.replyBuilder`). Callers must filter; this signature enforces + * it via [EventHintBundle] of `TextNoteEvent`. + * * **Local cache update.** If your caller has a local event cache, feed the + * new event back in so the UI / next read sees the update without a relay + * round-trip. + * + * Canonical entry point for non-UI callers — the underlying + * [TextNoteEvent.build] reply-aware overload handles full NIP-10 tag carry: + * `marker=root` (parent's root e-tag if present, else parent.id), + * `marker=reply` (parent.id), the parent's full p-tag chain plus parent.pubKey, + * and the relay hint from [EventHintBundle]. + */ +object ReplyActions { + /** + * Build a kind:1 [TextNoteEvent] that replies to [parent], wrapping it with + * NIP-10-correct marked e-tags and the parent's p-tag chain. + * + * Returns the signed event ready to be published. The reply preserves the + * parent's root reference so conformant clients can reconstruct the thread. + */ + suspend fun replyTo( + parent: EventHintBundle, + content: String, + signer: NostrSigner, + ): TextNoteEvent { + // Per NIP-10, replies MUST carry the p-tags of the event being replied + // to plus the author's pubkey. TextNoteEvent.build(replyingTo=) only + // emits the e-tag chain — p-tag carry is the caller's responsibility. + val carriedPubKeys = + (parent.event.linkedPubKeys() + parent.event.pubKey) + .distinct() + .map { PTag(it, relayHint = null) } + + val template = + TextNoteEvent.build(content, replyingTo = parent) { + notify(carriedPubKeys) + } + return signer.sign(template) + } +} diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActionsTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActionsTest.kt new file mode 100644 index 0000000000..e602464be5 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActionsTest.kt @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.actions + +import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip01Core.tags.people.PTag +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +class ReplyActionsTest { + private val alicePriv = "0000000000000000000000000000000000000000000000000000000000000007" + private val aliceSigner = NostrSignerInternal(KeyPair(alicePriv.hexToByteArray())) + + private val bobPriv = "0000000000000000000000000000000000000000000000000000000000000008" + private val bobSigner = NostrSignerInternal(KeyPair(bobPriv.hexToByteArray())) + + @Test + fun replyToTopLevelParent_setsRootToParentAndCarriesAuthor() = + runTest { + // Alice posts a top-level note (no e-tags = parent IS its own root). + val parent = aliceSigner.sign(TextNoteEvent.build("hello")) + assertTrue(parent.isNewThread(), "parent must be a fresh thread for this case") + + // Bob replies. + val reply = ReplyActions.replyTo(EventHintBundle(parent, null), "hi alice", bobSigner) + + assertEquals(TextNoteEvent.KIND, reply.kind) + assertEquals(bobSigner.pubKey, reply.pubKey) + + // Per `prepareETagsAsReplyTo`: when parent has no root, only a ROOT + // marker is emitted (it doubles as the reply target). No separate + // REPLY marker. `markedReplyTos()` should still resolve to parent.id. + val root = reply.markedRoot() + assertNotNull(root, "reply must carry a NIP-10 root marker") + assertEquals(parent.id, root.eventId, "root marker must point at the top-level parent") + + // p-tag carry must include the parent's author so they're notified. + val pubKeys = reply.tags.mapNotNull(PTag::parseKey) + assertTrue(parent.pubKey in pubKeys, "reply must carry the parent's pubkey in p-tags") + } + + @Test + fun replyToDeepThread_carriesRootForwardAndChainsPTags() = + runTest { + // Build A (root) → B (alice's reply to A) → C (carol's reply to B). + val a = aliceSigner.sign(TextNoteEvent.build("the original")) + + val carolPriv = "0000000000000000000000000000000000000000000000000000000000000009" + val carolSigner = NostrSignerInternal(KeyPair(carolPriv.hexToByteArray())) + + val b = ReplyActions.replyTo(EventHintBundle(a, null), "good point", aliceSigner) + + // C replies to B — must carry A as root (not B), and reply to B. + val c = ReplyActions.replyTo(EventHintBundle(b, null), "agreed", carolSigner) + + val rootC = c.markedRoot() + assertNotNull(rootC, "deep reply must carry root marker") + assertEquals(a.id, rootC.eventId, "deep reply's root must chain through to original") + + val replyC = c.markedReply() + assertNotNull(replyC, "deep reply must carry reply marker") + assertEquals(b.id, replyC.eventId, "deep reply's reply marker must point at immediate parent") + + // p-tag chain: must include both alice (root author / parent author) and parent.pubKey. + val pubKeys = c.tags.mapNotNull(PTag::parseKey).toSet() + assertTrue(aliceSigner.pubKey in pubKeys, "deep reply must carry root author in p-tags") + } + + @Test + fun replyEvent_isSignedAndKind1() = + runTest { + val parent = aliceSigner.sign(TextNoteEvent.build("seed")) + val reply = ReplyActions.replyTo(EventHintBundle(parent, null), "thanks", bobSigner) + + assertEquals(TextNoteEvent.KIND, reply.kind) + assertTrue(reply.id.length == 64, "reply id must be a 32-byte hex") + assertTrue(reply.sig.length == 128, "reply must be signed (64-byte sig hex)") + assertEquals("thanks", reply.content) + } +} diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/EventDispatch.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/EventDispatch.kt new file mode 100644 index 0000000000..06a9c51f15 --- /dev/null +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/EventDispatch.kt @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.desktop.cache + +import com.vitorpamplona.amethyst.desktop.network.RelayConnectionManager +import com.vitorpamplona.quartz.nip01Core.core.Event + +/** + * Canonical local-first dispatch for user-action events on desktop: write to + * the local cache before broadcasting so the UI reflects the action immediately, + * even if relay round-trips fail. + * + * Replaces five inlined `consume + broadcastToAll` couplets that had drifted + * in ordering (reactions/follows did broadcast-then-consume, replies did + * consume-then-broadcast). Use this everywhere a signed event must be both + * persisted locally and pushed to outbox relays. + */ +fun dispatch( + signed: Event, + localCache: DesktopLocalCache, + relayManager: RelayConnectionManager, +) { + localCache.consume(signed, relay = null) + relayManager.broadcastToAll(signed) +} diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/FeedScreen.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/FeedScreen.kt index ee927efd01..ff25760a79 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/FeedScreen.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/FeedScreen.kt @@ -82,6 +82,7 @@ import androidx.compose.ui.platform.LocalFocusManager import androidx.compose.ui.text.TextRange import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.unit.dp +import com.vitorpamplona.amethyst.commons.actions.ReplyActions import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.model.Note @@ -103,6 +104,7 @@ import com.vitorpamplona.amethyst.desktop.DesktopPreferences import com.vitorpamplona.amethyst.desktop.SearchHistoryStore import com.vitorpamplona.amethyst.desktop.account.AccountState import com.vitorpamplona.amethyst.desktop.cache.DesktopLocalCache +import com.vitorpamplona.amethyst.desktop.cache.dispatch import com.vitorpamplona.amethyst.desktop.feeds.DesktopCustomFeedFilter import com.vitorpamplona.amethyst.desktop.feeds.DesktopFollowingFeedFilter import com.vitorpamplona.amethyst.desktop.feeds.DesktopGlobalFeedFilter @@ -135,11 +137,7 @@ import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter -import com.vitorpamplona.quartz.nip01Core.tags.events.ETag -import com.vitorpamplona.quartz.nip01Core.tags.events.eTag import com.vitorpamplona.quartz.nip01Core.tags.hashtags.HashtagTag -import com.vitorpamplona.quartz.nip01Core.tags.people.PTag -import com.vitorpamplona.quartz.nip01Core.tags.people.pTag import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip18Reposts.GenericRepostEvent import com.vitorpamplona.quartz.nip18Reposts.RepostEvent @@ -421,9 +419,8 @@ fun FeedScreen( followMutex.withLock { val currentList = localCache.lastContactListEvent val updatedEvent = FollowAction.follow(pubKeyHex, account.signer, currentList) - relayManager.broadcastToAll(updatedEvent) - // consume updates followedUsers StateFlow + stores the event - localCache.consume(updatedEvent, relay = null) + // consume updates followedUsers StateFlow + stores the event before broadcast + dispatch(updatedEvent, localCache, relayManager) } } } @@ -1550,19 +1547,14 @@ private fun ExpandedNoteContent( myAvatarUrl = myAvatarUrl, onSend = { content -> withContext(Dispatchers.IO) { - val template = - TextNoteEvent.build(content) { - val etag = ETag(event.id) - etag.relay = null - etag.author = event.pubKey - eTag(etag) - pTag( - PTag(event.pubKey, relayHint = null), - ) - } - val signedEvent = account.signer.sign(template) - localCache.consume(signedEvent, relay = null) - relayManager.broadcastToAll(signedEvent) + val parentText = event as? TextNoteEvent ?: return@withContext + val signedEvent = + ReplyActions.replyTo( + EventHintBundle(parentText, null), + content, + account.signer, + ) + dispatch(signedEvent, localCache, relayManager) } }, ) @@ -1613,8 +1605,7 @@ private fun ExpandedNoteContent( "+", account.signer, ) - relayManager.broadcastToAll(signed) - localCache.consume(signed, relay = null) + dispatch(signed, localCache, relayManager) } } }, diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/NoteActions.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/NoteActions.kt index b69da8aae7..83806d8395 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/NoteActions.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/NoteActions.kt @@ -86,6 +86,7 @@ import com.vitorpamplona.amethyst.commons.model.nip51Bookmarks.BookmarkAction import com.vitorpamplona.amethyst.commons.model.nip57Zaps.ZapAction import com.vitorpamplona.amethyst.commons.service.lnurl.LightningAddressResolver import com.vitorpamplona.amethyst.commons.ui.components.UserAvatar +import com.vitorpamplona.amethyst.commons.util.toZapAmount import com.vitorpamplona.amethyst.desktop.account.AccountState import com.vitorpamplona.amethyst.desktop.cache.DesktopLocalCache import com.vitorpamplona.amethyst.desktop.network.DesktopHttpClient @@ -232,7 +233,7 @@ fun ZapAmountDialog( FilterChip( selected = selectedAmount == amount, onClick = { selectedAmount = amount }, - label = { Text(formatSats(amount)) }, + label = { Text(amount.toZapAmount()) }, ) } } @@ -250,7 +251,7 @@ fun ZapAmountDialog( }, confirmButton = { Button(onClick = { onZap(selectedAmount, message) }) { - Text("Zap ${formatSats(selectedAmount)} sats") + Text("Zap ${selectedAmount.toZapAmount()} sats") } }, dismissButton = { @@ -261,8 +262,6 @@ fun ZapAmountDialog( ) } -private fun formatSats(amount: Long): String = if (amount >= 1000) "${amount / 1000}k" else "$amount" - /** * Dialog for choosing bookmark visibility (public or private). */ @@ -369,7 +368,7 @@ fun ZapReceiptsDialog( tint = MaterialTheme.colorScheme.primary, modifier = Modifier.size(24.dp), ) - Text("${formatSats(totalAmount)} sats") + Text("${totalAmount.toZapAmount()} sats") if (isLoading) { CircularProgressIndicator( modifier = Modifier.size(16.dp), @@ -411,7 +410,7 @@ fun ZapReceiptsDialog( } } Text( - text = "${formatSats(receipt.amountSats)} sats", + text = "${receipt.amountSats.toZapAmount()} sats", style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.primary, ) @@ -526,7 +525,7 @@ fun ZapReceiptsPopup( modifier = Modifier.size(16.dp), ) Text( - "${formatSats(totalSats)} sats", + "${totalSats.toZapAmount()} sats", style = MaterialTheme.typography.titleSmall, fontWeight = FontWeight.Bold, color = MaterialTheme.colorScheme.primary, @@ -567,7 +566,7 @@ fun ZapReceiptsPopup( } } Text( - text = "${formatSats(entry.amount)} sats", + text = "${entry.amount.toZapAmount()} sats", style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.primary, ) @@ -1229,7 +1228,7 @@ fun NoteActionsRow( } if (zapAmountSats > 0) { Text( - text = formatSats(zapAmountSats), + text = zapAmountSats.toZapAmount(), style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.primary, modifier = Modifier.clickable { showZapReceiptsDialog = true }, diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/ThreadScreen.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/ThreadScreen.kt index 9c6894d4e9..78f8b224d4 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/ThreadScreen.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/ThreadScreen.kt @@ -49,6 +49,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp +import com.vitorpamplona.amethyst.commons.actions.ReplyActions import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.model.Note @@ -60,6 +61,7 @@ import com.vitorpamplona.amethyst.commons.ui.feeds.FeedState import com.vitorpamplona.amethyst.commons.util.toTimeAgo import com.vitorpamplona.amethyst.desktop.account.AccountState import com.vitorpamplona.amethyst.desktop.cache.DesktopLocalCache +import com.vitorpamplona.amethyst.desktop.cache.dispatch import com.vitorpamplona.amethyst.desktop.feeds.DesktopThreadFilter import com.vitorpamplona.amethyst.desktop.network.DesktopRelayConnectionManager import com.vitorpamplona.amethyst.desktop.subscriptions.DesktopRelaySubscriptionsCoordinator @@ -77,11 +79,7 @@ import com.vitorpamplona.amethyst.desktop.ui.thread.RelatedContentSection import com.vitorpamplona.amethyst.desktop.viewmodels.DesktopFeedViewModel import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.hints.EventHintBundle -import com.vitorpamplona.quartz.nip01Core.tags.events.ETag -import com.vitorpamplona.quartz.nip01Core.tags.events.eTag import com.vitorpamplona.quartz.nip01Core.tags.hashtags.hashtags -import com.vitorpamplona.quartz.nip01Core.tags.people.PTag -import com.vitorpamplona.quartz.nip01Core.tags.people.pTag import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip19Bech32.Nip19Parser import com.vitorpamplona.quartz.nip19Bech32.entities.NEvent @@ -342,24 +340,16 @@ fun ThreadScreen( myAvatarUrl = myAvatarUrl, onSend = { content -> withContext(Dispatchers.IO) { - val rootEvent = - rootNote.event ?: return@withContext - val template = - TextNoteEvent.build(content) { - val etag = ETag(rootEvent.id) - etag.relay = null - etag.author = rootEvent.pubKey - eTag(etag) - pTag( - PTag( - rootEvent.pubKey, - relayHint = null, - ), - ) - } - val signedEvent = account.signer.sign(template) - localCache.consume(signedEvent, relay = null) - relayManager.broadcastToAll(signedEvent) + val parentText = + rootNote.event as? TextNoteEvent + ?: return@withContext + val signedEvent = + ReplyActions.replyTo( + EventHintBundle(parentText, null), + content, + account.signer, + ) + dispatch(signedEvent, localCache, relayManager) } }, ) @@ -420,8 +410,7 @@ fun ThreadScreen( "+", account.signer, ) - relayManager.broadcastToAll(signed) - localCache.consume(signed, relay = null) + dispatch(signed, localCache, relayManager) } } }, diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt index 4a203c74b7..90f0dbce05 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt @@ -146,8 +146,12 @@ fun DeckColumnContainer( val currentOverlay = navState.current val focusRequester = remember { FocusRequester() } - // Request focus on nav change so Esc key works - LaunchedEffect(currentOverlay) { + // Request focus once when the column is created. Re-keying on + // `currentOverlay` would steal focus from sibling columns whenever any + // deck column mutates its overlay state (e.g. typing in column A's reply + // box loses focus when column B opens a profile). Esc continues to work + // because the column still owns focus when the user hits the key. + LaunchedEffect(Unit) { focusRequester.requestFocus() } diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckLayout.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckLayout.kt index 96058bcf88..aba4cbd9c5 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckLayout.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckLayout.kt @@ -35,6 +35,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.key import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier import androidx.compose.ui.input.pointer.PointerIcon @@ -119,27 +120,31 @@ fun DeckLayout( ) } - DeckColumnContainer( - column = column, - canClose = columns.size > 1, - onClose = { deckState.removeColumn(column.id) }, - onDoubleClickHeader = { deckState.expandColumn(column.id, availableWidthDp) }, - relayManager = relayManager, - localCache = localCache, - accountManager = accountManager, - account = account, - iAccount = iAccount, - nwcConnection = nwcConnection, - subscriptionsCoordinator = subscriptionsCoordinator, - highlightStore = highlightStore, - draftStore = draftStore, - nip11Fetcher = nip11Fetcher, - appScope = appScope, - onShowComposeDialog = onShowComposeDialog, - onShowReplyDialog = onShowReplyDialog, - onZapFeedback = onZapFeedback, - onNavigateToRelays = onNavigateToRelays, - ) + // Key by column id so reorder/remove doesn't re-run the + // child's `LaunchedEffect(Unit)` (which grabs keyboard focus). + key(column.id) { + DeckColumnContainer( + column = column, + canClose = columns.size > 1, + onClose = { deckState.removeColumn(column.id) }, + onDoubleClickHeader = { deckState.expandColumn(column.id, availableWidthDp) }, + relayManager = relayManager, + localCache = localCache, + accountManager = accountManager, + account = account, + iAccount = iAccount, + nwcConnection = nwcConnection, + subscriptionsCoordinator = subscriptionsCoordinator, + highlightStore = highlightStore, + draftStore = draftStore, + nip11Fetcher = nip11Fetcher, + appScope = appScope, + onShowComposeDialog = onShowComposeDialog, + onShowReplyDialog = onShowReplyDialog, + onZapFeedback = onZapFeedback, + onNavigateToRelays = onNavigateToRelays, + ) + } } } } diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/CommentItem.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/CommentItem.kt index 16d8b6de08..7097592a4a 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/CommentItem.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/CommentItem.kt @@ -37,6 +37,7 @@ import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import com.vitorpamplona.amethyst.commons.ui.components.UserAvatar +import com.vitorpamplona.amethyst.commons.util.toZapAmount @Composable fun CommentItem( @@ -152,7 +153,7 @@ fun CommentItem( if (zapAmount > 0) { Spacer(Modifier.width(4.dp)) Text( - text = formatZapAmount(zapAmount), + text = zapAmount.toZapAmount(), style = MaterialTheme.typography.labelSmall, color = zapColor, ) @@ -162,10 +163,3 @@ fun CommentItem( } } } - -private fun formatZapAmount(sats: Long): String = - when { - sats >= 1_000_000 -> "${sats / 1_000_000}M" - sats >= 1_000 -> "${sats / 1_000}k" - else -> sats.toString() - } diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/RelatedContentRow.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/RelatedContentRow.kt index fdf6959cb2..a9252ccd93 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/RelatedContentRow.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/RelatedContentRow.kt @@ -40,11 +40,8 @@ import androidx.compose.material3.CardDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue +import androidx.compose.runtime.produceState import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -59,9 +56,11 @@ import com.vitorpamplona.amethyst.commons.feeds.related.CompactNoteData import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.richtext.RichTextParser import com.vitorpamplona.amethyst.commons.richtext.UrlParser +import com.vitorpamplona.amethyst.commons.util.showAmount import com.vitorpamplona.amethyst.desktop.cache.DesktopLocalCache import com.vitorpamplona.quartz.nip01Core.tags.hashtags.isTaggedHashes import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import java.math.BigDecimal /** * Horizontal scrollable row of compact related content cards. @@ -78,67 +77,37 @@ fun RelatedContentSection( onViewAll: () -> Unit = {}, modifier: Modifier = Modifier, ) { - var relatedItems by remember(noteId) { mutableStateOf>(emptyList()) } + val lowercaseTags = noteHashtags.map { it.lowercase() }.toSet() - DisposableEffect(noteId) { - val results = mutableListOf() - val lowercaseTags = noteHashtags.map { it.lowercase() }.toSet() - val limit = 6 - - // Scan cache for related content - if (lowercaseTags.isNotEmpty()) { - localCache.notes.forEach { key, note -> - if (note.idHex != noteId && - note.event is TextNoteEvent && - note.event?.tags?.isTaggedHashes(lowercaseTags) == true - ) { - results.add(note) - } + // Re-scan the cache initially and whenever a bundle of new events arrives + // that contains a candidate (same hashtag or same author). Without this, + // expanding a note on a cold cache leaves the section empty until the user + // collapses + re-expands. + val relatedItems by produceState>( + initialValue = emptyList(), + key1 = noteId, + key2 = authorPubKey, + key3 = lowercaseTags, + ) { + fun rescan() { + runCatching { + value = scanRelated(localCache, noteId, authorPubKey, lowercaseTags) } } - - // Fallback: same author - if (results.size < limit) { - localCache.notes.forEach { key, note -> - if (note.idHex != noteId && - note.event is TextNoteEvent && - note.event?.pubKey == authorPubKey && - note !in results - ) { - results.add(note) + rescan() + localCache.eventStream.newEventBundles.collect { bundle -> + val matters = + bundle.any { n -> + val ev = n.event + ev is TextNoteEvent && + n.idHex != noteId && + ( + ev.pubKey == authorPubKey || + (lowercaseTags.isNotEmpty() && ev.tags.isTaggedHashes(lowercaseTags)) + ) } - } + if (matters) rescan() } - - relatedItems = - results - .sortedByDescending { it.createdAt() } - .take(limit) - .map { note -> - val event = note.event - val content = event?.content ?: "" - val firstLine = - content - .take(80) - .lineSequence() - .firstOrNull() - ?.take(60) ?: "" - val author = localCache.getUserIfExists(event?.pubKey ?: "") - val imageUrl = - UrlParser() - .parseValidUrls(content) - .withScheme - .firstOrNull { RichTextParser.isImageUrl(it) } - CompactNoteData( - id = note.idHex, - title = firstLine.ifBlank { "Note" }, - authorName = author?.toBestDisplayName() ?: event?.pubKey?.take(8) ?: "", - thumbnailUrl = imageUrl, - zapCount = if (note.zapsAmount > java.math.BigDecimal.ZERO) "${note.zapsAmount.toLong()}" else "", - ) - } - - onDispose { } } if (relatedItems.isNotEmpty()) { @@ -192,6 +161,76 @@ fun RelatedContentSection( } } +private const val RELATED_LIMIT = 6 + +/** + * Scan the local cache for notes related to [noteId] either by sharing a + * hashtag in [lowercaseTags] or by being authored by [authorPubKey]. Returns + * up to [RELATED_LIMIT] notes, most recent first, mapped to [CompactNoteData]. + * + * Runs O(N) over `localCache.notes` — backed by `ConcurrentSkipListMap` which + * supports concurrent inserts during iteration (weakly consistent). Safe on + * the main composition coroutine for typical cache sizes (~30k notes). + */ +private fun scanRelated( + localCache: DesktopLocalCache, + noteId: String, + authorPubKey: String, + lowercaseTags: Set, +): List { + val results = mutableListOf() + + if (lowercaseTags.isNotEmpty()) { + localCache.notes.forEach { _, note -> + if (note.idHex != noteId && + note.event is TextNoteEvent && + note.event?.tags?.isTaggedHashes(lowercaseTags) == true + ) { + results.add(note) + } + } + } + + if (results.size < RELATED_LIMIT) { + localCache.notes.forEach { _, note -> + if (note.idHex != noteId && + note.event is TextNoteEvent && + note.event?.pubKey == authorPubKey && + note !in results + ) { + results.add(note) + } + } + } + + return results + .sortedByDescending { it.createdAt() } + .take(RELATED_LIMIT) + .map { note -> + val event = note.event + val content = event?.content ?: "" + val firstLine = + content + .take(80) + .lineSequence() + .firstOrNull() + ?.take(60) ?: "" + val author = localCache.getUserIfExists(event?.pubKey ?: "") + val imageUrl = + UrlParser() + .parseValidUrls(content) + .withScheme + .firstOrNull { RichTextParser.isImageUrl(it) } + CompactNoteData( + id = note.idHex, + title = firstLine.ifBlank { "Note" }, + authorName = author?.toBestDisplayName() ?: event?.pubKey?.take(8) ?: "", + thumbnailUrl = imageUrl, + zapCount = if (note.zapsAmount > BigDecimal.ZERO) showAmount(note.zapsAmount) else "", + ) + } +} + @Composable private fun CompactRelatedCard( item: CompactNoteData, diff --git a/docs/plans/2026-06-02-fix-desktop-feed-review-findings-plan.md b/docs/plans/2026-06-02-fix-desktop-feed-review-findings-plan.md new file mode 100644 index 0000000000..4beeba620f --- /dev/null +++ b/docs/plans/2026-06-02-fix-desktop-feed-review-findings-plan.md @@ -0,0 +1,588 @@ +--- +title: Fix 5 review findings on PR #3124 desktop feed UI refresh +type: fix +status: active +date: 2026-06-02 +pr: https://github.com/vitorpamplona/amethyst/pull/3124 +review_comment: https://github.com/vitorpamplona/amethyst/pull/3124#issuecomment-4599816576 +worktree: ../AmethystMultiplatform-feed-review +branch: fix/desktop-feed-ui-review (tracks origin/feat/desktop-feed-ui-refresh) +deepened: 2026-06-02 +--- + +# Fix 5 review findings on PR #3124 + +## Enhancement summary (2026-06-02 deepen-plan) + +Eight parallel agents resolved all 5 open questions and surfaced 3 plan revisions: + +**Open questions resolved:** +- **Q1 — kind-1 NIP-10 vs kind-1111 NIP-22:** kind-1 NIP-10 confirmed. Android's + `NotificationReplyReceiver` already routes by parent type + (`TextNoteEvent` → kind 1, others → kind 1111). Desktop feed loads kind 1 + only (`DesktopFeedFilters.kt:39`). Use kind 1 + `prepareETagsAsReplyTo`. +- **Q2 — extract consume+broadcast couplet:** YES. Five clean call sites (no + inline complexity) + ordering inconsistency (`TextNoteEvent` does + consume→broadcast at `ThreadScreen.kt:361` and `FeedScreen.kt:1564`, + Reaction/Follow do broadcast→consume at `ThreadScreen.kt:424`, + `FeedScreen.kt:426`/`:1617`). A `desktopApp` extension fixes both volume and + the ordering drift. Canonical order: consume→broadcast (local-first). +- **Q3 — Phase 4 produceState vs ViewModel:** produceState. `LargeCache.notes` + is a `ConcurrentSkipListMap` (`LargeCache.jvmAndroid.kt:27`) — weakly + consistent iterator, safe on main composition coroutine, 50–150ms for ~30k + notes. No debounce needed; candidate-filter pre-check blocks 80–90% of + bundles. `FeedViewModel.kt:54-59` precedent collects same stream without + debounce. +- **Q4 — NoteActions.kt:264 formatSats:** sats, safe to swap to + `amount.toZapAmount()`. Inputs are hardcoded preset amounts (line 111 + `ZAP_AMOUNTS = listOf(21L, 100L, ...)`) and `LnZapEvent.amount` which is + already sats (`LnZapEvent.kt:69`). +- **Q5 — WalletColumnScreen.kt:979 formatSats:** intentional. Wallet shows + precise balance with locale-aware grouping (`1,000,000`). Leave + add + `// intentional` comment to prevent future drift. + +**Plan revisions:** +- **Phase 1 path flattening:** move `ReplyActions` from + `commons/.../actions/nip10Notes/ReplyActions.kt` to flat + `commons/.../actions/ReplyActions.kt`. Sister actions (`FollowActions`, + `ZapActions`, `DmActions`, `SearchActions`) are all flat under `actions/`; + no `nipNN/` subpackage convention. (Architecture review) +- **Phase 5 simplification:** drop the explicit `requestFocus()` in the Esc + handler; the column never loses focus during pop (Esc was *received by* the + focused column). Just `LaunchedEffect(Unit) { requestFocus() }` + the + existing `.focusable()`. Add `key(column.id) { DeckColumnContainer(...) }` + wrap in `DeckLayout.kt:111` so `LaunchedEffect(Unit)` survives column + reordering. (Code-simplicity + focus-audit review) +- **Phase 2 promoted from "optional":** with 5 verified duplicates + order + inconsistency, extract `Account.dispatch(signed: Event)` as a + `desktopApp` extension. Canonical order: consume→broadcast. Not in + `commons` (relay manager + cache are desktop types). + +**Android follow-up (out of this PR):** 4 inlined `TextNoteEvent.build` sites +on Android (`ShortNotePostViewModel.kt:1037`, `VoiceReplyViewModel.kt:265`, +`NotificationReplyReceiver.kt:203`, `AmethystAppFunctions.kt:1051`) should +migrate to the new `ReplyActions.replyTo` in a follow-up PR. Tracked in +"Future work" below. + +## Overview + +Davotoula's review on PR #3124 (`feat/desktop-feed-ui-refresh`) flagged 5 issues +ranging from one **NIP-10 protocol bug** (inline reply emits a tag set other +clients can't thread) down to **consistency bugs** (zap totals bypass the shared +formatter). All confirmed by inspecting `origin/feat/desktop-feed-ui-refresh`. + +This plan groups the fixes so dependent ones land in a sequence that compiles at +each step, and routes the protocol/architectural fixes through existing shared +helpers (`TextNoteEvent.build(replyingTo=…)`, `ReactionAction`, `FollowAction`, +`ZapFormatter.showAmount`) rather than introducing new abstractions. + +## Findings (root-cause confirmed) + +### #3 — Inline reply emits lower-fidelity NIP-10 tag set [PROTOCOL BUG] + +**File:** `desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/FeedScreen.kt:1554` + +**Current code:** +```kotlin +val template = TextNoteEvent.build(content) { + val etag = ETag(event.id) + etag.relay = null + etag.author = event.pubKey + eTag(etag) + pTag(PTag(event.pubKey, relayHint = null)) +} +``` + +**Root cause:** the call uses the **single-arg** `TextNoteEvent.build(note, initializer)` +overload at `quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip10Notes/TextNoteEvent.kt:133` +and hand-rolls a minimal reply tag set. It emits a single unmarked `e`-tag and a +single `p`-tag. **No NIP-10 root marker. No carry of the parent's root-e-tag. +No carry of the parent's p-tag chain.** Replying to a note deep in a thread +produces an event with no `root` reference; conformant clients (Damus, Primal, +Coracle…) can't reconstruct the thread. + +**Fix:** switch to the **reply-aware overload** at `TextNoteEvent.kt:142`: + +```kotlin +fun build( + note: String, + replyingTo: EventHintBundle? = null, + forkingFrom: EventHintBundle? = null, + … +) = eventTemplate(KIND, note, createdAt) { + alt(shortedMessageForAlt(note)) + if (replyingTo != null || forkingFrom != null) { + markedETags(prepareETagsAsReplyTo(replyingTo, forkingFrom)) + } + initializer() +} +``` + +`prepareETagsAsReplyTo` (already in quartz) handles **root marker, reply marker, +parent root/p-tag carry** correctly. The inline path was bypassing it. + +### #4 — Reaction/follow/reply business logic in desktop composables + +**Files:** +- `FeedScreen.kt:1611` — reaction (likes from inline expansion) +- `FeedScreen.kt:423` — follow (follow pill from feed) +- `FeedScreen.kt:1554` — reply (inline reply input) + +**Current state (verified):** +- **Follow** already uses `FollowAction.follow(pubKeyHex, signer, currentList)` + (commons). The complaint is the surrounding `cache.consume → broadcast` + couplet inlined in the composable. +- **Reaction** already uses `ReactionAction.reactTo(EventHintBundle, "+", signer)` + (commons). Same couplet inlined. +- **Reply** does NOT use a shared builder (see #3). + +**CLAUDE.md rule (`commons/ARCHITECTURE.md:73-88`):** "actions package (CLI-safe): +Event builders for user actions (follow, zap…). The canonical entry point for +non-UI callers." + +**Fix:** introduce a new shared action that mirrors `FollowActions` for kind-1 +replies. The consume+broadcast couplet stays inline (2 lines, platform-specific +relay/cache wiring), but the *protocol-touching* build moves out: + +```kotlin +// commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/nip10Notes/ReplyActions.kt +object ReplyActions { + suspend fun replyTo( + parent: EventHintBundle, + content: String, + signer: NostrSigner, + ): TextNoteEvent { + val template = TextNoteEvent.build(content, replyingTo = parent) + return signer.sign(template) + } +} +``` + +Desktop call site becomes: +```kotlin +val parentText = event as? TextNoteEvent ?: return@withContext +val signed = ReplyActions.replyTo(EventHintBundle(parentText, null), content, account.signer) +localCache.consume(signed, relay = null) +relayManager.broadcastToAll(signed) +``` + +This matches the shape already used for `FollowAction.follow` at `FeedScreen.kt:423` +and `ReactionAction.reactTo` at `NoteActions.kt:1393`. Drift between desktop and +Android paths is bounded to a 3-line couplet that won't grow. + +### #1 — Related content stale after one scan + +**File:** `desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/thread/RelatedContentRow.kt:83` + +**Current code:** +```kotlin +DisposableEffect(noteId) { + val results = mutableListOf() + // scan localCache.notes once + localCache.notes.forEach { … } + relatedItems = results.sortedByDescending { it.createdAt() }.take(6).map { … } + onDispose { } +} +``` + +**Root cause:** `DisposableEffect(noteId)` re-runs only on `noteId` change. The +scan reads `localCache.notes` (a `LargeCache`) at composition time; nothing +re-runs the scan as the cache fills. Expanding a note on a cold cache leaves +the section empty/partial until collapse+re-expand. Also missing keys: +`noteHashtags` and `authorPubKey` (cosmetic — caller stabilises these per noteId). + +**Fix:** observe the cache's change stream and re-scan on bundle arrivals. +`DesktopLocalCache` exposes `eventStream: DesktopCacheEventStream` with +`newEventBundles: SharedFlow>` (`DesktopLocalCache.kt:719-743`). + +Two options: + +**Option A (simpler, matches inline-section scale):** `produceState` keyed by +`(noteId, hashtagsHash, authorPubKey)` that collects `newEventBundles` and +re-runs the scan when relevant events land: + +```kotlin +val relatedItems by produceState>(emptyList(), noteId, authorPubKey, noteHashtags) { + fun rescan() { value = scanRelated(localCache, noteId, authorPubKey, noteHashtags) } + rescan() // initial + localCache.eventStream.newEventBundles.collect { bundle -> + if (bundle.any { isCandidate(it, noteHashtags, authorPubKey) }) rescan() + } +} +``` + +**Option B (matches FeedViewModel family):** new `RelatedContentViewModel` in +`commons/src/commonMain/.../viewmodels/related/`, taking a `FeedFilter` style +"by hashtag OR by author" predicate and exposing +`StateFlow>`. Heavier but consistent with the rest of the +feed system. + +**Recommendation:** **Option A** — the related-content row is a 6-item sidecar, +not a feed. A ViewModel adds wiring without solving an actual problem here. +Deepen-plan agent may overrule. + +### #2 — Deck columns steal focus from each other + +**File:** `desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/deck/DeckColumnContainer.kt:150` + +**Current code:** +```kotlin +val focusRequester = remember { FocusRequester() } +LaunchedEffect(currentOverlay) { focusRequester.requestFocus() } +``` + +**Root cause:** `LaunchedEffect(currentOverlay)` fires on the column's first +composition **and on every overlay change**. In a multi-column deck, when +column B opens an overlay, column B's effect grabs focus — yanking it out of +column A's inline reply input mid-typing. + +**Fix:** decouple "request focus once on initial composition" from "Escape +handler needs focus to be live." The Escape key path works as long as the +column owns focus when the user presses Escape — which it does after the +initial composition. Match the existing pattern at +`EditProfileScreen.kt:380` (`LaunchedEffect(Unit) { focusRequester.requestFocus() }`) +**and** scope the effect so only the column the user is interacting with +re-grabs focus when a nested overlay closes (i.e. on `popOverlay()`). + +Concretely: +1. Change `LaunchedEffect(currentOverlay)` → `LaunchedEffect(Unit)` for the + initial focus request. +2. When the user presses Escape and `navState.pop()` succeeds, explicitly call + `focusRequester.requestFocus()` in the key handler (intent-driven, not + composition-driven). + +This contains focus stealing to the column the user actually interacted with. + +### #5 — Zap totals bypass shared formatter + +**Files:** +- `RelatedContentRow.kt:137` — `zapCount = "${note.zapsAmount.toLong()}"` (raw, e.g. `"1500000"`) +- `CommentItem.kt:155` — `text = formatZapAmount(zapAmount)` calling local helper +- `CommentItem.kt:166-171` — private `formatZapAmount(sats: Long)` hand-rolled k/M + +**Shared formatter (`commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/util/ZapFormatter.kt`):** +- `fun showAmount(amount: BigDecimal?): String` — G/M/k suffixes, `""` for null/<0.01 +- `fun showAmountWithZero(amount: BigDecimal?): String` — same, `"0"` instead of `""` +- `fun Long.toZapAmount(): String` +- `fun Int.toZapAmount(): String` + +`Note.zapsAmount` is `BigDecimal` (`commons/src/commonMain/.../model/Note.kt:183`), +so use `showAmount(note.zapsAmount)` directly in `RelatedContentRow`. `CommentItem` +takes a `Long`, so use `zapAmount.toZapAmount()` and delete the local helper. + +**Also flagged (outside review but same root cause):** +- `NoteActions.kt:264` — local `formatSats(amount: Long)` with only `k` suffix. +- `WalletColumnScreen.kt:979` — local `formatSats` using `NumberFormat` (intentional? + wallet flows may want full sats — leave but document). + +Cover the two review-flagged sites + `NoteActions.kt:264`. Defer wallet. + +## Phased implementation plan + +Phases ordered so each compiles + tests cleanly without depending on later work. + +### Phase 1 — Shared `ReplyActions` (fixes #3, completes #4 reply) + +**Create:** `commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActions.kt` *(flat — matches `FollowActions.kt`, `ZapActions.kt`, `DmActions.kt`; no `nip10Notes/` subpackage)* + +```kotlin +object ReplyActions { + suspend fun replyTo( + parent: EventHintBundle, + content: String, + signer: NostrSigner, + ): TextNoteEvent { + val template = TextNoteEvent.build(content, replyingTo = parent) + return signer.sign(template) + } +} +``` + +Mirror `FollowActions.buildFollow` shape (`commons/.../actions/FollowActions.kt:69`). + +**Test:** `commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ReplyActionsTest.kt` +— assert the signed event has: +- A marked e-tag with `marker=reply` pointing at the parent id. +- A marked e-tag with `marker=root` (pointing at parent's root if parent had one, + else parent's id). +- All parent p-tags carried + parent's `pubKey` appended. + +Use `runTest { … }` from `kotlinx-coroutines-test`, in-test signer is +`NostrSignerInternal(KeyPair(privHex.hexToByteArray()))` — match `FollowActionsTest.kt:25-37`. + +**Edit:** `desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/FeedScreen.kt` (~ line 1554) — replace inline build with `ReplyActions.replyTo(EventHintBundle(parentText, null), content, account.signer)`. Guard parent kind with `event as? TextNoteEvent`; if not a kind-1, skip / log (replies to non-kind-1 from the feed inline path were never well-defined and are out of scope; matches Android's `NotificationReplyReceiver.kt:136-156` routing). + +**Acceptance:** +- [ ] `./gradlew :commons:jvmTest --tests "*ReplyActionsTest*"` green. +- [ ] Manual: reply to a deep-thread note from desktop, inspect the broadcast event in a relay log → has `e-tag root` + `e-tag reply` + carries all parent `p` tags. +- [ ] Reply renders in Damus/Primal under the correct thread. + +### Phase 2 — Extract reaction/follow/reply consume+broadcast couplet (Option B confirmed) + +Deepen-plan audit found **5 clean duplicate sites** with an ordering +inconsistency between them: + +| File:line | Signer | Order today | +|---|---|---| +| `ThreadScreen.kt:361` | inline `TextNoteEvent.build` reply | consume → broadcast | +| `ThreadScreen.kt:424` | `ReactionAction.reactTo` | broadcast → consume | +| `FeedScreen.kt:426` | `FollowAction.follow` | broadcast → consume | +| `FeedScreen.kt:1564` | inline `TextNoteEvent.build` reply | consume → broadcast | +| `FeedScreen.kt:1617` | `ReactionAction.reactTo` | broadcast → consume | + +No site has inline extra work (snackbars, retries) entangled with the couplet. + +**Create:** `desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/EventDispatch.kt` + +```kotlin +/** + * Canonical local-first dispatch: write to the local cache before broadcasting + * so the UI reflects the user's action immediately, even if relay round-trips fail. + */ +suspend fun dispatch( + signed: Event, + localCache: DesktopLocalCache, + relayManager: RelayManager, +) { + localCache.consume(signed, relay = null) + relayManager.broadcastToAll(signed) +} +``` + +(Or, equivalent — as an extension on a small `DispatchContext` if the call +sites already have one. Keep in `desktopApp` because both `LocalCache` and +`RelayManager` are desktop-side types.) + +**Migrate all 5 sites** to call `dispatch(signed, localCache, relayManager)`. +Fixes the ordering drift (everyone goes local-first) and shrinks call sites +to one line. + +**Acceptance:** +- [ ] `grep -rn "broadcastToAll" desktopApp/` shows only the call inside `EventDispatch.kt` + any non-couplet uses. +- [ ] No remaining call sites do consume + broadcast inline (other than the helper). +- [ ] Reactions, follows, and replies all still round-trip correctly in a manual sanity test. + +### Phase 3 — ZapFormatter swap (fixes #5) + +**Edit:** `RelatedContentRow.kt:137` — +```kotlin +zapCount = if (note.zapsAmount > BigDecimal.ZERO) showAmount(note.zapsAmount) else "", +``` + +**Edit:** `CommentItem.kt:155, 166-171` — replace `formatZapAmount(zapAmount)` with +`zapAmount.toZapAmount()`. **Delete the private `formatZapAmount` fun at line 166-171.** + +**Edit:** `NoteActions.kt:264` — swap to `amount.toZapAmount()`. Verified +sats (not msats): inputs are `ZAP_AMOUNTS = listOf(21L, 100L, 500L, 1000L, 5000L, 10000L)` +at line 111 + `LnZapEvent.amount` which is sats per `LnZapEvent.kt:69`. +**Delete the private `formatSats` fun if no remaining references** (run +`grep -n formatSats desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/NoteActions.kt` after swap). + +**Skip:** `WalletColumnScreen.kt:979` `formatSats` is **intentional** — wallet +balance display uses locale-aware grouping (`NumberFormat.getNumberInstance().format`) +for full-precision sats. Out of review scope. Add a one-line `// intentional: wallet shows precise sats with locale grouping; not a ZapFormatter target` comment to prevent future drift. + +**Acceptance:** +- [ ] `./gradlew :desktopApp:compileKotlin` green. +- [ ] Visual: a note with 1.5M sats renders `1.5M` (or `1M`, matching commons + semantics), not `1500000`. + +### Phase 4 — Cache-aware Related (fixes #1) + +**Edit:** `RelatedContentRow.kt:83-145` — replace `DisposableEffect(noteId)` with +`produceState` (Option A) keyed on `(noteId, authorPubKey, noteHashtags)`: + +```kotlin +val relatedItems by produceState>( + initialValue = emptyList(), + key1 = noteId, key2 = authorPubKey, key3 = noteHashtags, +) { + val lowercaseTags = noteHashtags.map { it.lowercase() }.toSet() + fun rescan() { + runCatching { + value = scanRelated(localCache, noteId, authorPubKey, lowercaseTags) + }.onFailure { + // weakly-consistent iterator may rarely surface; skip this tick + } + } + rescan() + localCache.eventStream.newEventBundles.collect { bundle -> + val matters = bundle.any { n -> + n.event is TextNoteEvent && + n.idHex != noteId && + (n.event?.pubKey == authorPubKey || + n.event?.tags?.isTaggedHashes(lowercaseTags) == true) + } + if (matters) rescan() + } +} +``` + +Extract the existing scan body into a private top-level `scanRelated(...)` so +both the initial call and the bundle-driven re-run share it. + +**Safety notes (deepen-plan):** +- `LargeCache.notes` is `ConcurrentSkipListMap` (`LargeCache.jvmAndroid.kt:27`) + — weakly-consistent iterator, safe on main composition coroutine. +- Scan is O(N): ~50–150ms for ~30k notes; fine on main. +- No debounce: candidate-filter blocks 80–90% of bundles. Matches + `FeedViewModel.kt:54-59` precedent (collects same stream without debounce). +- If hot-loop observed in production, retroactively add `.debounce(150)` + (precedent: `SearchBarState.kt:87`, `BookmarkListState.kt`). + +**Acceptance:** +- [ ] Cold-cache repro: open a note in a fresh session, related section starts + empty; as kind-1 events stream in matching the hashtag or author, related + cards appear without collapse+re-expand. +- [ ] No re-render storm: rescan only fires when a bundle contains a candidate. + +### Phase 5 — Focus gating (fixes #2) + +**Edit:** `DeckColumnContainer.kt:147-152` — +```kotlin +LaunchedEffect(Unit) { focusRequester.requestFocus() } // once on column creation +``` + +That's the only effect change. **No explicit `requestFocus()` in the Escape +handler**: deepen-plan focus-audit verified the column never loses focus during +back-nav (Escape was received by the focused column → it still has focus after +`navState.pop()`). Adding it would be cargo-cult. + +**Also edit:** `DeckLayout.kt:111` — wrap the `forEachIndexed` body in a +`key(column.id) { DeckColumnContainer(...) }` so `LaunchedEffect(Unit)` +survives column reordering. Without `key()`, moving a column in the deck list +re-fires the effect for the wrong column instance. + +**Acceptance:** +- [ ] Two-column repro: in column A's inline reply text field, type characters + while column B opens/closes an overlay → column A keeps focus, no + characters lost. +- [ ] Escape still pops nested overlays in the focused column. +- [ ] Reorder a column in the deck (drag if supported, or remove+re-add) → + typing focus in unrelated columns is preserved. + +## System-Wide Impact + +### Interaction graph + +- **Reply path:** `InlineReplyInput.onSend(content)` → `ReplyActions.replyTo(...)` (commons) → `signer.sign` → `localCache.consume` (DesktopLocalCache) → `relayManager.broadcastToAll` (NostrClient WS pool) → relay round-trip → cache update → `eventStream.newEventBundles` → Phase-4 `produceState` re-scan → related section refresh. Phase 4 is downstream of Phase 1 only by happy coincidence (a reply might match its parent's hashtags) — no hard coupling. +- **Follow path:** unchanged, already routed through `FollowAction.follow`. +- **Reaction path:** unchanged, already routed through `ReactionAction.reactTo`. + +### Error propagation + +- `ReplyActions.replyTo` is `suspend` and propagates `signer.sign` failures (cancellation, signer rejection). Desktop call site already runs in `withContext(Dispatchers.IO)` — wrap in `try/catch` to surface a snackbar on signing failure (Android path does this in `CommentPostViewModel.sendPostSync`; desktop currently swallows). +- Phase 4 `produceState` collect runs in the column's coroutine scope; cancelled when composable leaves composition. Exceptions in `scanRelated` (e.g. ConcurrentModificationException on `LargeCache.forEach`) would crash the collector — wrap `rescan()` body in `runCatching` to skip on transient cache mutations. + +### State lifecycle risks + +- Phase 1: signed reply written to `localCache` before broadcast succeeds. If + the broadcast fails, the reply is visible locally but not on relays. + This matches existing behaviour for reaction/follow paths; no new risk. +- Phase 4: `produceState` collects an unbounded `SharedFlow`. If `newEventBundles` + emits at high rate (cold cache fill), `scanRelated` runs O(N) per bundle. + `LargeCache.notes` size for an active user is ~10k–50k notes; a full scan is + ~ms. Acceptable; if hot-loop observed, debounce via `collectLatest` + + `delay(150)`. + +### API surface parity + +- `ReplyActions` is JVM-only consumer today (desktop), but lives in + `commons/commonMain` so Android can adopt it (and should — `NewPostViewModel` + on Android currently inlines a similar `TextNoteEvent.build` call). Tracked as + follow-up, **not in this PR**. + +### Integration test scenarios + +1. **NIP-10 thread fidelity:** create note A → reply B to A → reply C to B from + desktop. Inspect C's tags: must contain `["e", A.id, "", "root"]` and + `["e", B.id, "", "reply"]` and `["p", A.pubKey]` + `["p", B.pubKey]`. +2. **Cold-cache related:** clear local DB, open a thread → Related row empty → + simulate incoming kind-1 events matching parent's hashtag → row populates + without user interaction. +3. **Multi-column focus:** open two columns side by side. Start typing in column + A's inline reply. Open a profile overlay in column B. Verify typed characters + stay in column A. +4. **Reply to non-kind-1:** open a thread whose root is a `LongFormContentEvent` + (kind 30023). Inline reply must either disable (preferred) or route through + `CommentEvent` (NIP-22) — open question below. +5. **Zap formatting:** seed a note with 1_500_000 sats zaps. Both `RelatedContentRow` + and `CommentItem` render `1.5M` (or `1M` per commons rules). + +## Acceptance criteria (rollup) + +### Functional + +- [ ] Inline-reply event from desktop, when broadcast, threads correctly in + ≥1 non-Amethyst client (Damus or Primal verified). +- [ ] Related section refreshes from cold cache without user interaction. +- [ ] Typing in column A's reply box doesn't lose focus when column B opens an + overlay. +- [ ] Zap totals render with k/M/G suffix in `RelatedContentRow` and + `CommentItem`. +- [ ] Existing inline reaction/follow continue to work (no regression). + +### Non-functional + +- [ ] No new `--no-verify` commits. +- [ ] `./gradlew spotlessApply` clean. +- [ ] `./gradlew test` green for `:commons:jvmTest` and `:quartz:jvmTest`. +- [ ] No new Kotlin warnings introduced. + +### Quality gates + +- [ ] `ReplyActionsTest` covers root-marker, reply-marker, p-tag carry. +- [ ] Hand-rolled `formatZapAmount` deleted (grep returns 0 in `desktopApp/`). + +## Dependencies & risks + +| Risk | Likelihood | Mitigation | +|---|---|---| +| `EventHintBundle` cast fails when parent is `CommentEvent` / `LongFormContentEvent` | Med | Guard with `as? TextNoteEvent`; skip + log if null. Open question covers full support. | +| `produceState` re-runs scan storm on cold cache fill | Low | Filter bundle for candidate match before rescan; debounce if observed. | +| `LargeCache.forEach` concurrent modification during rescan | Low | Wrap rescan body in `runCatching`. | +| Focus fix breaks ESC → back-nav inside a column | Low | Explicit `requestFocus()` in pop handler covers it; manual repro before push. | +| `ZapFormatter.showAmount` returns `""` for amount < 0.01 — different from current `"0"`-on-empty | Low | Use `showAmountWithZero` if `"0"` desired, else gate with `if (note.zapsAmount > ZERO)`. | + +## Resolved questions (deepen-plan) + +All Q1–Q5 resolved — see "Enhancement summary" at top for verdicts + evidence. + +## Future work (separate PR, not in this branch) + +- **Android kind-1 reply migration.** Four Android sites inline + `TextNoteEvent.build` and should migrate to the new `ReplyActions.replyTo` + (single source of truth across platforms): + - `amethyst/.../ShortNotePostViewModel.kt:1037` + - `amethyst/.../VoiceReplyViewModel.kt:265` + - `amethyst/.../NotificationReplyReceiver.kt:203` + - `amethyst/.../AmethystAppFunctions.kt:1051` +- **CLI `amy reply` verb.** `ReplyActions` lives in `commons/commonMain` and + is CLI-safe — a future Amy reply verb wires straight to it. +- **Wallet vs Zap formatter consolidation.** `WalletColumnScreen.kt:979` + intentionally diverges (locale-aware full-precision). Revisit if/when a + unified "amount display" component is built. + +## Sources & references + +### Internal references + +- Review comment: https://github.com/vitorpamplona/amethyst/pull/3124#issuecomment-4599816576 +- PR: https://github.com/vitorpamplona/amethyst/pull/3124 +- `commons/ARCHITECTURE.md:73-88` — actions package boundary +- `quartz/.../nip10Notes/TextNoteEvent.kt:142` — reply-aware build overload +- `quartz/.../nip10Notes/tags/MarkedETag.kt:44-60` — NIP-10 marker enum + tag-array +- `quartz/.../nip10Notes/tags/prepareETagsAsReplyTo.kt` — root/reply tag-carry helper +- `commons/.../actions/FollowActions.kt:69` — pattern to mirror for `ReplyActions` +- `commons/.../model/nip25Reactions/ReactionAction.kt:50` — sister action +- `commons/.../util/ZapFormatter.kt` — shared zap-amount formatter +- `desktopApp/.../cache/DesktopLocalCache.kt:719-743` — `eventStream.newEventBundles` +- `desktopApp/.../ui/EditProfileScreen.kt:380` — `LaunchedEffect(Unit)` focus pattern to mirror +- `amethyst/.../ui/note/nip22Comments/CommentPostViewModel.kt:128, 447-571` — Android reply path (NIP-22 reference, not directly reused) + +### CLAUDE.md conventions + +- "Check existing implementations first — most logic already exists" — confirmed: shared helpers exist; this is reuse, not new abstraction. +- "Pre-commit hooks run spotless — always `./gradlew spotlessApply` before commit" +- "Never use `--no-verify`" +- "Verify, Don't Guess" — root causes verified by reading code at each line cited.