Show replies in context of their threads #4871

gaearon · 2024-08-03T00:14:12Z

Resubmit of #4835.

What

This rewrites our feed slicing logic which determines how posts (and in particular replies) appear in the feed.

It aims to fix a few problems:

Replies now appear within the context of their parent threads. This avoids out-of-context replies.
Replies from the same thread don't appear again in your Following feed. In other words, replies from friends "bump" threads but you only see the most recent reply to any given thread. This avoids a single thread dominating the timeline.
Replies no longer get "torn off" from their parent conversations due to a pagination boundary.
Replies from friends no longer get "torn off" from the original "author threads" that they belong to. In fact we're removing special casing of an "author thread". This means Connect replies from other users #2914 can be closed.
~~Lists now use the Following feed settings (and no longer show replies from non-follows).~~
- We probably still want to do this but I'll send a separate PR later — not part of this change.
"Hide replies" no longer hides reposted replies (since someone you follow wanted to treat it as a top-level).

How

This builds on changes from #4834, #4864, #4868, and #4869. I've made the "feed slice" encapsulate more of the "building up items" logic so that it's able to reuse parent and root from the response despite those not having a reply.

What we show on the screen is now much closer to the data shape returned by the server — essentially, we show [root, ..., parent, reply] for each item. In Following feeds, feedgens, and lists, we deduplicate by the root so that each thread acts as a forum thread — you only see it once (but replies from friends bump it). In author feeds (Posts and Replies), we use a different deduplication strategy: you want to see all relevant posts, so we instead hide contiguous leading sequences of posts that you have already seen. See comments in code for specific scenarios.

Examples

Here's a broken up thread (before):

Here's how this PR fixes it (after):

Here's an out-of-context reply (before):

Here's how this PR adds context to it (after):

Here's a reply from somebody I follow to somebody I don't follow (before):

There is no "after" for this example because we'd no longer show the above replies in Following in this PR.

Here is a "torn" reply to another person's thread (before):

Here is how this PR fixes it:

Test Plan

Here's some things I encourage you to test:

Following feed
- Different settings (hide replies, hide reposts, hide quotes)
- Mergefeed on and off
- Move Following to first position on an account that doesn't follow anyone, verify Discover fallback works
- Verify you only see people you follow for each root/parent/reply
- Replies from a seen thread should not appear below
- Repost of a non-leaf reply should not break the thread, both should remain visible
- Reposts of same post by different follow should remain deduped
- Replies with deleted and blocked parents or roots should not show up
- Replies with blocked grandparents should not show up
- When parent and grandparent authors are different in a >=3 post thread, "in reply to" is displayed
- "Has new" polling and button works
Starter Packs
- Posts tab should work like Following
Profile
- Should deduplicate individual posts but should not deduplicate threads
- Contiguous leading blocks should get deduplicated top-down (threads like [ABC], [ABD], [AEF] become [ABC], [D], [EF]). You should see "in reply to" for [D] and [EF] since their parent/roots got deduplicated.
- Replies with deleted and blocked parents or roots should (!) show up for Replies tab
- Replies with blocked grandparents should (!) show up for Replies tab
Lists
- Should work like Profile (deduplicate by leading posts, but not by threads)
- We'll probably change it to work more like Following later, but out of scope of this PR
Feedgens
- Should work like Profile (deduplicate by leading posts, but not by threads)

render · 2024-08-03T00:14:16Z

Your Render PR Server URL is https://social-app-pr-4871.onrender.com.

Follow its progress at https://dashboard.render.com/web/srv-cqmnclo8fa8c73e031m0.

gaearon · 2024-08-03T00:15:18Z

src/state/queries/post-feed.ts

@@ -313,53 +313,22 @@ export function usePostFeedQuery(
                  const feedPostSlice: FeedPostSlice = {


This is actually getting close to the shape of the "feed slice" class so I think we can unify these concepts in the future and maybe represent moderation as a feed tune.

gaearon · 2024-08-03T00:16:06Z

src/lib/api/feed-manip.ts

+          if (i === 0) {
+            // Omit contiguous seen leading items.
+            // For example, [A -> B -> C], [A -> D -> E], [A -> D -> F]
+            // would turn into [A -> B -> C], [D -> E], [F].


This heuristic is interesting and worth noticing. It only kicks in for author feeds because other feeds already get deduped by the root.

gaearon · 2024-08-03T00:16:16Z

src/lib/api/feed-manip.ts

+            // If the last item in the slice was already seen, omit the whole slice.
+            // This means we'd miss its parents, but the user can "show more" to see it.
+            // For example, [A ... E -> F], [A ... D -> E], [A ... C -> D], [A -> B -> C]
+            // would get collapsed into [A ... E -> F], with B/C/D considered seen.


This one is interesting too.

github-actions · 2024-08-03T00:20:30Z

Old size	New size	Diff
7.15 MB	7.15 MB	-563 B (-0.01%)

src/lib/api/feed-manip.ts

gaearon · 2024-08-03T16:22:57Z

Pushed a commit that turns off thread deduping for feedgens. The motivation is that feedgens are already able to do thread deduping if they want to. So if a feedgen is serving a bunch of replies from the same thread, it must have a reason to do so and we shouldn't hide those. Instead we'll rely on contiguous leading post deduping for feedgens, same as for author feeds.

It is now meaningless because there's nothing special about author threads.

It was called rootUri but it was actually the leaf URI. Regardless, it's not used anymore.

This is easier to think about.

This is already being taken care of by item-level deduping. It's also now wrong and removing too much (since it wasn't filtering for reposts directly).

This reverts commit aff86be. Let's not do this yet and have a bit more discussion. This is a chunky change already.

gaearon · 2024-08-05T17:52:03Z

Pushed a revert to the Lists part of the deal. We probably still want to do it but decoupling from this already chunky change.

pfrazee

My review is pretty high level but the logic checks out!

commit af52626 Author: Minseo Lee <itoupluk427@gmail.com> Date: Thu Aug 8 21:12:23 2024 +0900 Added trans (bluesky-social#4890) commit a864f69 Author: dan <dan.abramov@gmail.com> Date: Thu Aug 8 06:20:24 2024 +0100 Keep interstitial fresh on refresh (bluesky-social#4888) commit 00fea10 Author: dan <dan.abramov@gmail.com> Date: Thu Aug 8 05:56:22 2024 +0100 Include popcluster in suggestion ranking (bluesky-social#4887) commit b309241 Author: Hailey <me@haileyok.com> Date: Wed Aug 7 17:13:29 2024 -0700 Add logging of selected feed preference when displaying the following feed (bluesky-social#4789) commit 1b02f81 Author: Hailey <me@haileyok.com> Date: Wed Aug 7 14:45:06 2024 -0700 [Video] Visibility detection view (bluesky-social#4741) Co-authored-by: Samuel Newman <10959775+mozzius@users.noreply.github.com> commit fff2c07 Author: Samuel Newman <mozzius@protonmail.com> Date: Wed Aug 7 18:47:51 2024 +0100 [Videos] Video player - PR #2 - better web support (bluesky-social#4732) * attempt some sort of "usurping" system * polling-based active video approach * split into inner component again * click to steal active video * disable findAndActivateVideo on native * new intersectionobserver approach - wip * fix types * disable perf optimisation to allow overflow * make active player indicator subtler, clean up video utils * partially fix double-playing * start working on controls * fullscreen API * get buttons working somewhat * rm source from where it shouldn't be * use video elem as source of truth * fix keyboard nav + mute state * new icons, add fullscreen + time + fix play * unmount when far offscreen + round 2dp * listen globally to clicks rather than blur event * move controls to new file * reduce quality when not active * add hover state to buttons * stop propagation of videoplayer click * move around autoplay effects * increase background contrast * add subtitles button * add stopPropagation to root of video player * clean up VideoWebControls * fix chrome * change quality based on focused state * use autoLevelCapping instead of nextLevel * get subtitle track from stream * always use hlsjs * rework hls into a ref * render player earlier, allowing preload * add error boundary * clean up component structure and organisation * rework fullscreen API * disable fullscreen on iPhone * don't play when ready on pause * debounce buffering * simplify giant list of event listeners * update pref * reduce prop drilling * minimise rerenders in `ActiveViewContext` * restore prop drilling --------- Co-authored-by: Samuel Newman <10959775+mozzius@users.noreply.github.com> Co-authored-by: Hailey <me@haileyok.com> commit b701e8c Author: Samuel Newman <mozzius@protonmail.com> Date: Wed Aug 7 16:56:12 2024 +0100 [Video] Authed video upload (bluesky-social#4885) * add service auth call * update API package --------- Co-authored-by: Samuel Newman <10959775+mozzius@users.noreply.github.com> commit 753a233 Author: Hailey <me@haileyok.com> Date: Tue Aug 6 11:21:59 2024 -0700 Tweak feed manip to show cases of A -> B without further children (bluesky-social#4883) commit 5845e08 Author: dan <dan.abramov@gmail.com> Date: Tue Aug 6 17:12:27 2024 +0100 Show own replies before follows' replies in threads (bluesky-social#4882) commit b291a1e Author: dan <dan.abramov@gmail.com> Date: Tue Aug 6 16:42:42 2024 +0100 Show more replies in Following (different heuristic) (bluesky-social#4880) commit 686d5eb Author: dan <dan.abramov@gmail.com> Date: Tue Aug 6 01:30:52 2024 +0100 [Persisted] Make broadcast subscriptions granular by key (bluesky-social#4874) * Add fast path for guaranteed noop updates * Change persisted.onUpdate() API to take a key * Implement granular broadcast listeners commit 966f6c5 Author: dan <dan.abramov@gmail.com> Date: Tue Aug 6 01:03:27 2024 +0100 [Persisted] Fix the race condition causing clobbered writes between tabs (bluesky-social#4873) * Broadcast the update in the same tick The motivation for the original code is unclear. I was not able to reproduce the described behavior and have not seen it mentioned on the web. I'll assume that this was a misunderstanding. * Remove defensive programming The only places in this code that we can expect to throw are schema.parse(), JSON.parse(), JSON.stringify(), and localStorage.getItem/setItem/removeItem. Let's push try/catch'es where we expect them to be necessary. * Don't write or clobber defaults Writing defaults to local storage is unnecessary. We would write them as a part of next update anyway. So I'm removing that to reduce the number of moving pieces. However, we do need to be wary of _state being set to defaults. Because _state gets mutated on write. We don't want to mutate the defaults object. To avoid having to think about this, let's copy on write. We don't write to this object very often. * Refactor: extract tryParse * Refactor: move string parsing into tryParse * Extract tryStringify, split logging by platform Shared data parsing/stringification errors are always logged. Storage errors are only logged on native because we trust the web APIs to work. * Add a layer of caching to readFromStorage to web We're going to be doing a read on every write so let's add a fast path that avoids parsing and validating. * Fix the race condition causing clobbered writes between tabs commit 5bf7f37 Author: dan <dan.abramov@gmail.com> Date: Tue Aug 6 00:30:58 2024 +0100 [Persisted] Fork web and native, make it synchronous on the web (bluesky-social#4872) * Delete logic for legacy storage * Delete superfluous tests At this point these tests aren't testing anything useful, let's just get rid of them. * Inline store.ts methods into persisted/index.ts * Fork persisted/index.ts into index.web.ts * Remove non-essential code and comments from both forks * Remove async/await from web fork of persisted/index.ts * Remove unused return * Enforce that forked types match commit 74b0318 Author: dan <dan.abramov@gmail.com> Date: Mon Aug 5 20:51:41 2024 +0100 Show replies in context of their threads (bluesky-social#4871) * Don't reconstruct threads from separate posts * Remove post-level dedupe for now * Change repost dedupe condition to look just at length * Delete unused isThread * Delete another isThread field It is now meaningless because there's nothing special about author threads. * Narrow down slice item shape so it does not need reply * Consolidate slice validation criteria in one place * Show replies in context * Make fallback marker work * Remove misleading and now-unused property It was called rootUri but it was actually the leaf URI. Regardless, it's not used anymore. * Add by-thread dedupe to non-author feeds * Add post-level dedupe * Always count from the start This is easier to think about. * Only tuner state need to be untouched on dry run * Account for threads in reply filtering * Remove repost deduping This is already being taken care of by item-level deduping. It's also now wrong and removing too much (since it wasn't filtering for reposts directly). * Calculate rootUri correctly * Apply Following settings to all lists * Don't dedupe intentional reposts by thread * Show reply parent when ambiguous * Explicitly remove orphaned replies from following/lists * Fix thread dedupe to work across pages * Mark grandparent-blocked as orphaned * Guard tuner state change by dryRun * Remove dead code * Don't dedupe feedgen threads * Revert "Apply Following settings to all lists" This reverts commit aff86be. Let's not do this yet and have a bit more discussion. This is a chunky change already. * Reason belongs to a slice, not item * Logically feedContext belongs to the slice * Update comment to reflect latest behavior commit 18b4233 Author: Hailey <me@haileyok.com> Date: Mon Aug 5 12:21:34 2024 -0700 Add `PlatformInfo` module (bluesky-social#4877) commit fb27838 Author: bnewbold <bnewbold@robocracy.org> Date: Fri Aug 2 15:57:50 2024 -0700 bskyweb: optional basic auth password middleware (bluesky-social#4759) commit 6298e68 Author: Samuel Newman <mozzius@protonmail.com> Date: Sat Aug 3 00:33:45 2024 +0200 tweak list header (bluesky-social#4870) Co-authored-by: Samuel Newman <10959775+mozzius@users.noreply.github.com> commit c3d8bee Author: Eric Bailey <git@esb.lol> Date: Fri Aug 2 13:05:33 2024 -0500 Respect labels on feeds and lists (bluesky-social#4818) * Prep * Pass in optional moderation to FeedCard * Compute moderation decision, filter contentList contexts, pass into card * Let's go a different route * Filter from within search queries * Use same search query for starter packs * Filter lists from profile tabs * Cleanup * Filter from profile feeds * Moderate post embeds * Memoize * Use ScreenHider on lists * Hide both list types * Fix crash on iOS in screen hider, fix lineheight * Memoize renderItem * Reuse objects to prevent re-renders commit 293ac6f Author: dan <dan.abramov@gmail.com> Date: Fri Aug 2 17:13:31 2024 +0100 Only show replies in Following if following all involved actors (bluesky-social#4869) * Only show replies in Following for followed root and grandparent * Remove now-unnecessary check * Simplify condition commit 7f292ab Author: dan <dan.abramov@gmail.com> Date: Thu Aug 1 22:05:40 2024 +0100 Always limit Following replies to the people you follow (bluesky-social#4868) * Limit feed replies to people you follow * Remove dead code commit f056cb6 Author: Hailey <me@haileyok.com> Date: Thu Aug 1 10:32:36 2024 -0700 Fix missing header on Likes/Reposted By, add missing perf optimizations (bluesky-social#4867) * fix liked by list * fix lists * tweaks to style * change string commit c78e9e3 Author: Samuel Newman <mozzius@protonmail.com> Date: Thu Aug 1 19:14:32 2024 +0200 Move theme controls to its own screen (bluesky-social#4866) commit 388c157 Author: dan <dan.abramov@gmail.com> Date: Thu Aug 1 17:49:43 2024 +0100 Display second-to-last rather than second post in a slice (bluesky-social#4864) commit b0e130a Author: Eric Bailey <git@esb.lol> Date: Thu Aug 1 10:29:27 2024 -0500 Update muted words dialog with `expiresAt` and `actorTarget` (bluesky-social#4801) * WIP not working dropdown * Update MutedWords dialog * Add i18n formatDistance * Comments * Handle text wrapping * Update label copy Co-authored-by: Hailey <me@haileyok.com> * Fix alignment * Improve translation output * Revert toggle changes * Better types for useFormatDistance * Tweaks * Integrate new sdk version into TagMenu * Use ampersand Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com> * Bump SDK --------- Co-authored-by: Hailey <me@haileyok.com> Co-authored-by: surfdude29 <149612116+surfdude29@users.noreply.github.com> commit d2e88cc Author: dan <dan.abramov@gmail.com> Date: Thu Aug 1 02:27:25 2024 +0100 Fetch enough pages to fill a page's worth of items (bluesky-social#4863) * Fetch enough pages to fill a page's worth of items * Add failsafe in case of appview bug commit 70ffd38 Author: Hailey <me@haileyok.com> Date: Wed Jul 31 11:16:14 2024 -0700 Only show "followed you back" when appropriate (bluesky-social#4849) * only show followed back when we should * try/catch * log * Update FeedItem.tsx * tweak commit 576cef8 Author: dan <dan.abramov@gmail.com> Date: Wed Jul 31 19:10:24 2024 +0100 [Web] Retrigger onEndReached if needed when content height changes (bluesky-social#4859) * Extract EdgeVisibility * Key Visibility by container height instead of item count commit c75bb65 Author: dan <dan.abramov@gmail.com> Date: Wed Jul 31 13:00:22 2024 +0100 Remove unused NoopFeedTuner (bluesky-social#4856) commit c3e77b5 Author: GSMT <samaritanojr006@gmail.com> Date: Wed Jul 31 00:19:23 2024 +0200 useDedupe callback (bluesky-social#4855) commit 8ddb28d Author: Hailey <me@haileyok.com> Date: Tue Jul 30 08:25:31 2024 -0700 [Video] Uploads (bluesky-social#4754) * state for video uploads * get upload working * add a debug log * add post progress * progress * fetch data * add some progress info, web uploads * post on finished uploading (wip) * add a note * add some todos * clear video * merge some stuff * convert to `createUploadTask` * patch expo modules core * working native upload progress * platform fork * upload progress for web * cleanup * cleanup * more tweaks * simplify * fix type errors --------- Co-authored-by: Samuel Newman <10959775+mozzius@users.noreply.github.com> Merge remote-tracking branch 'upstream/main' into Improve-notification-localization

* origin/main: (45 commits) Added trans (#4890) Keep interstitial fresh on refresh (#4888) Include popcluster in suggestion ranking (#4887) Add logging of selected feed preference when displaying the following feed (#4789) [Video] Visibility detection view (#4741) [Videos] Video player - PR #2 - better web support (#4732) [Video] Authed video upload (#4885) Tweak feed manip to show cases of A -> B without further children (#4883) Show own replies before follows' replies in threads (#4882) Show more replies in Following (different heuristic) (#4880) [Persisted] Make broadcast subscriptions granular by key (#4874) [Persisted] Fix the race condition causing clobbered writes between tabs (#4873) [Persisted] Fork web and native, make it synchronous on the web (#4872) Show replies in context of their threads (#4871) Add `PlatformInfo` module (#4877) bskyweb: optional basic auth password middleware (#4759) tweak list header (#4870) Respect labels on feeds and lists (#4818) Only show replies in Following if following all involved actors (#4869) Always limit Following replies to the people you follow (#4868) ...

gaearon commented Aug 3, 2024

View reviewed changes

src/lib/api/feed-manip.ts Outdated Show resolved Hide resolved

gaearon added 25 commits August 5, 2024 18:33

Don't reconstruct threads from separate posts

3633b5f

Remove post-level dedupe for now

33be208

Change repost dedupe condition to look just at length

c1ec1c8

Delete unused isThread

ca40052

Delete another isThread field

684f9e3

It is now meaningless because there's nothing special about author threads.

Narrow down slice item shape so it does not need reply

5b0101e

Consolidate slice validation criteria in one place

738cd3c

Show replies in context

79b7e4f

Make fallback marker work

b815d0b

Remove misleading and now-unused property

d953747

It was called rootUri but it was actually the leaf URI. Regardless, it's not used anymore.

Add by-thread dedupe to non-author feeds

cb22f77

Add post-level dedupe

e6d6241

Always count from the start

5b4eb01

This is easier to think about.

Only tuner state need to be untouched on dry run

40d8a97

Account for threads in reply filtering

c971a16

Remove repost deduping

094ad48

This is already being taken care of by item-level deduping. It's also now wrong and removing too much (since it wasn't filtering for reposts directly).

Calculate rootUri correctly

b7328e2

Apply Following settings to all lists

8d8ce68

Don't dedupe intentional reposts by thread

c87fc1d

Show reply parent when ambiguous

da90049

Explicitly remove orphaned replies from following/lists

4bc3ee7

Fix thread dedupe to work across pages

177410d

Mark grandparent-blocked as orphaned

3b2b2d9

Guard tuner state change by dryRun

765f3a0

Remove dead code

c1b42cb

gaearon added 2 commits August 5, 2024 18:33

Don't dedupe feedgen threads

a7d0f37

Revert "Apply Following settings to all lists"

d64b584

This reverts commit aff86be. Let's not do this yet and have a bit more discussion. This is a chunky change already.

gaearon force-pushed the threads branch from ebb4cce to d64b584 Compare August 5, 2024 17:48

gaearon added 2 commits August 5, 2024 19:49

Reason belongs to a slice, not item

eea8256

Logically feedContext belongs to the slice

cd6f518

gaearon force-pushed the threads branch from 64e62ba to cd6f518 Compare August 5, 2024 18:57

pfrazee approved these changes Aug 5, 2024

View reviewed changes

Update comment to reflect latest behavior

e06361b

gaearon merged commit 74b0318 into main Aug 5, 2024
5 of 6 checks passed

gaearon deleted the threads branch August 5, 2024 19:51

This was referenced Aug 5, 2024

Connect replies from other users #2914

Closed

RFC: Show more replies in Following (different heuristic) #4880

Merged

estrattonbailey mentioned this pull request Aug 15, 2024

Fix orphaned feed slices, handle blocks #4944

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show replies in context of their threads #4871

Show replies in context of their threads #4871

gaearon commented Aug 3, 2024 •

edited

Loading

render bot commented Aug 3, 2024

gaearon Aug 3, 2024

gaearon Aug 3, 2024

gaearon Aug 3, 2024

github-actions bot commented Aug 3, 2024 •

edited

Loading

gaearon commented Aug 3, 2024

gaearon commented Aug 5, 2024

pfrazee left a comment

		@@ -313,53 +313,22 @@ export function usePostFeedQuery(
		const feedPostSlice: FeedPostSlice = {

Show replies in context of their threads #4871

Show replies in context of their threads #4871

Conversation

gaearon commented Aug 3, 2024 • edited Loading

What

How

Examples

Test Plan

render bot commented Aug 3, 2024

gaearon Aug 3, 2024

Choose a reason for hiding this comment

gaearon Aug 3, 2024

Choose a reason for hiding this comment

gaearon Aug 3, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 3, 2024 • edited Loading

gaearon commented Aug 3, 2024

gaearon commented Aug 5, 2024

pfrazee left a comment

Choose a reason for hiding this comment

gaearon commented Aug 3, 2024 •

edited

Loading

github-actions bot commented Aug 3, 2024 •

edited

Loading