mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 08:04:45 +00:00
refactor(notifications): tighten public-chat reply walk and its docs
Audit follow-ups, no behaviour change: - isNotifiablePublicChatReply bails before allocating the HashSet/ArrayDeque scratch when the channel message has no parents (top-level posts — the common case), and builds the deque straight from the parent list. - Correct the docstring: a kind-42 replyTo holds only the immediate parent (the channel root is filtered out), so the chain is walked hop-by-hop through each cached ancestor — the previous wording implied replyTo already held the ancestors. - Trim the over-long inline comment on the acceptableEvent gate. - Make the multi-hop test prove what it claims: assert the immediate parent is not me, so the walk only passes by climbing to the grandparent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PxDVCSWe1RwZ51vABBwqbG
This commit is contained in:
+20
-21
@@ -148,35 +148,37 @@ class NotificationFeedFilter(
|
||||
) + ADDRESSABLE_KINDS
|
||||
|
||||
// How deep to walk a public chat reply chain looking for one of the
|
||||
// user's own messages. Bounds the cost on very long threads and the
|
||||
// user's own messages. Bounds the cost on very long threads; the
|
||||
// visited-set guards against malformed cyclic replyTo links.
|
||||
private const val PUBLIC_CHAT_ANCESTOR_SCAN_LIMIT = 30
|
||||
|
||||
/**
|
||||
* Public chats (NIP-28, kind 42) routinely reply to a user without
|
||||
* adding a `p` tag, so the normal mention gate ([Event.isTaggedUser])
|
||||
* misses them. Treat a channel message as "for me" when one of my own
|
||||
* misses them. Treats a channel message as "for me" when one of my own
|
||||
* messages appears in its reply chain — a direct reply to my message
|
||||
* (the common case the user described as "the previous message was
|
||||
* mine") or a later message in a thread I'm already part of ("an
|
||||
* active thread"). Reactions/zaps/reposts target via `replyTo` and are
|
||||
* handled by the generic rules below; this is scoped to channel
|
||||
* messages only.
|
||||
* (the common case: "the previous message was mine") or a later message
|
||||
* in a thread I'm already part of (an "active thread"). A kind-42
|
||||
* `replyTo` holds only the immediate parent, so the chain is walked
|
||||
* hop-by-hop through each cached ancestor.
|
||||
*
|
||||
* Cache-only (reads [Note.replyTo] + author, never the account), so the
|
||||
* push dispatcher and the in-app feed can both call it to relax the
|
||||
* p-tag gate without loading the account or decrypting anything.
|
||||
* push dispatcher and the in-app feed can both relax their p-tag gate
|
||||
* with it without loading the account or decrypting anything.
|
||||
*/
|
||||
fun isNotifiablePublicChatReply(
|
||||
note: Note,
|
||||
authorHex: HexKey,
|
||||
): Boolean {
|
||||
if (note.event !is ChannelMessageEvent) return false
|
||||
// Top-level channel posts have no parent to reply to — bail before
|
||||
// allocating the walk's scratch structures (the common case).
|
||||
val parents = note.replyTo
|
||||
if (parents.isNullOrEmpty()) return false
|
||||
|
||||
var scanned = 0
|
||||
val seen = HashSet<HexKey>()
|
||||
val toVisit = ArrayDeque<Note>()
|
||||
note.replyTo?.let { toVisit.addAll(it) }
|
||||
val toVisit = ArrayDeque(parents)
|
||||
|
||||
while (toVisit.isNotEmpty() && scanned < PUBLIC_CHAT_ANCESTOR_SCAN_LIMIT) {
|
||||
val ancestor = toVisit.removeFirst()
|
||||
@@ -403,16 +405,13 @@ class NotificationFeedFilter(
|
||||
// follow/list modes) also applies the per-kind relevance heuristics.
|
||||
val isRawGlobal = followList() is TopFilter.Global
|
||||
|
||||
// Channel messages may reply to one of my messages without a p-tag
|
||||
// (common in NIP-28 clients), so the p-tag gate is OR'd with the
|
||||
// public-chat reply check. Kept inline (not a pre-computed val) so the
|
||||
// cheap `kind in NOTIFICATION_KINDS` check short-circuits ahead of it —
|
||||
// otherwise the tag scan + reply-chain walk would run on every note in
|
||||
// the cache. Within the OR, the cheaper tag scan runs before the
|
||||
// reply-chain walk. tagsAnEventByUser below (also gated for
|
||||
// Selected/follow modes) returns true for exactly the same events, so
|
||||
// unrelated channel chatter never leaks through — even in Global mode,
|
||||
// where it is the only relevance check.
|
||||
// The p-tag gate is OR'd with isNotifiablePublicChatReply so channel
|
||||
// replies into my messages still notify without a p-tag. Kept inline
|
||||
// (not a pre-computed val) so the cheap kind check short-circuits ahead
|
||||
// of the tag scan + reply walk, which is also why the cheaper tag scan
|
||||
// is ordered first within the OR. In Global mode this is the only
|
||||
// relevance check (tagsAnEventByUser is skipped below); it still scopes
|
||||
// to genuine replies, so unrelated channel chatter never leaks through.
|
||||
return noteEvent?.kind in NOTIFICATION_KINDS &&
|
||||
(noteEvent is LnZapEvent || notifAuthor != loggedInUserHex) &&
|
||||
(isChessEvent || filterParams.isGlobal() || notifAuthor == null || filterParams.isAuthorInFollows(notifAuthor)) &&
|
||||
|
||||
+5
-1
@@ -78,13 +78,17 @@ class NotificationPublicChatReplyTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `reply deeper in a thread i posted in is notifiable`() {
|
||||
fun `reply two hops above me in a thread i posted in is notifiable`() {
|
||||
// root(other) <- myMessage(me) <- someoneElse(other) <- newReply(other)
|
||||
// newReply's immediate parent is `other`, so a single-hop check would
|
||||
// miss me — this only passes if the walk climbs to the grandparent.
|
||||
val root = channelMessage("d".repeat(64), other, emptyList())
|
||||
val myMessage = channelMessage("e".repeat(64), me, listOf(root))
|
||||
val someoneElse = channelMessage("9".repeat(64), other, listOf(myMessage))
|
||||
val newReply = channelMessage("8".repeat(64), other, listOf(someoneElse))
|
||||
|
||||
// Sanity: the immediate parent is not me, so this is a true multi-hop hit.
|
||||
assertFalse(newReply.replyTo?.any { it.author?.pubkeyHex == me } == true)
|
||||
assertTrue(NotificationFeedFilter.isNotifiablePublicChatReply(newReply, me))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user