mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(desktop): address PR review findings on feed UI refresh
5 issues from davotoula's review on PR #3124: - #3 (protocol): inline reply emitted a minimal e/p tag set instead of NIP-10. Extract `commons/actions/ReplyActions.replyTo` wrapping `TextNoteEvent.build(replyingTo=)` (which already encodes root marker, reply marker, parent root-e-tag carry) + carry parent's p-tag chain via `notify(...)`. Replies to deep-thread notes now thread correctly in Damus/Primal/Coracle. Covered by `ReplyActionsTest`. - #4 (architecture): reaction/follow/reply each inlined `localCache.consume + relayManager.broadcastToAll` in 5 sites with inconsistent ordering. Extract `desktopApp/cache/dispatch(...)` — canonical local-first order — and route all 5 sites through it. - #1 (UX): related-content section scanned the cache once via `DisposableEffect(noteId)` and never refreshed. Switch to `produceState` collecting `DesktopLocalCache.eventStream.newEventBundles`; re-scan only when an arriving bundle contains a candidate (matching hashtag or author). `LargeCache.notes` is a ConcurrentSkipListMap (weakly consistent iterator) so the scan stays safe on the composition coroutine. - #2 (UX): `DeckColumnContainer` re-requested focus on every `currentOverlay` change, stealing focus from sibling columns whenever any column mutated overlay state. Drop to `LaunchedEffect(Unit)` and wrap the column in `key(column.id)` in `DeckLayout` so the one-shot effect survives column reordering. - #5 (consistency): zap totals bypassed the shared `ZapFormatter`. Wire `RelatedContentRow`, `CommentItem`, and `NoteActions` to `commons/util/ZapFormatter.{showAmount,toZapAmount}`; delete `formatZapAmount` and `formatSats` desktop-local helpers. `WalletColumnScreen.formatSats` intentionally kept — locale-aware full precision for wallet balance is by design. Plan: docs/plans/2026-06-02-fix-desktop-feed-review-findings-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
70636c0f9a
commit
aeb49c3cac
+82
@@ -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<TextNoteEvent>,
|
||||
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)
|
||||
}
|
||||
}
|
||||
+105
@@ -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)
|
||||
}
|
||||
}
|
||||
+43
@@ -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)
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -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 },
|
||||
|
||||
+13
-24
@@ -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)
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
+6
-2
@@ -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()
|
||||
}
|
||||
|
||||
|
||||
+26
-21
@@ -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,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+2
-8
@@ -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()
|
||||
}
|
||||
|
||||
+99
-60
@@ -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<List<CompactNoteData>>(emptyList()) }
|
||||
val lowercaseTags = noteHashtags.map { it.lowercase() }.toSet()
|
||||
|
||||
DisposableEffect(noteId) {
|
||||
val results = mutableListOf<Note>()
|
||||
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<List<CompactNoteData>>(
|
||||
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<String>,
|
||||
): List<CompactNoteData> {
|
||||
val results = mutableListOf<Note>()
|
||||
|
||||
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,
|
||||
|
||||
@@ -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<TextNoteEvent>? = null,
|
||||
forkingFrom: EventHintBundle<TextNoteEvent>? = 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<TextNoteEvent>,
|
||||
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<Note>()
|
||||
// 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<Set<Note>>` (`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<List<CompactNoteData>>(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<List<CompactNoteData>>`. 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<TextNoteEvent>,
|
||||
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<List<CompactNoteData>>(
|
||||
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<TextNoteEvent>` 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.
|
||||
Reference in New Issue
Block a user