Skip to content

fix(soup): keep loaded pages when a mutation refreshes a list - #8243

Closed
synoet wants to merge 14 commits into
synoet/email-compact-rowsfrom
synoet/soup-keep-pages-on-refresh
Closed

synoet wants to merge 14 commits into
synoet/email-compact-rowsfrom
synoet/soup-keep-pages-on-refresh

Conversation

@synoet

@synoet synoet commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Stacked on #8242 (email polish stack). Includes #8165, merged in because these list-stability fixes build on it (its per-view sort keys, chain refetch that keeps pages mounted, and done-row release). Review this PR's own commit, fix(soup): keep loaded pages when a mutation refreshes a list; once #8165 lands on main this diff shrinks to it.

Why

Every committed mutation (mark done, mark read or unread, creating a task) refreshed each active Soup list after resetting it to its first page. A list scrolled past its first 100 rows collapsed back to them, losing the rows and the scroll position, after each Done and after each unread thread opened from the list.

What

  • lib/queries/soup/items.ts: the mutation-driven refresh refetches the loaded page chain without resetToInitialPage(). With fix(email): stop Signal rows flickering and jumping after mark done #8165's replacePage, a page whose cursor changed is fetched before it replaces the old one, so the list keeps its height while the chain rebuilds.
  • The refresh test asserts the reset no longer happens.

Measured on a seeded inbox: after e, exactly the archived row leaves and one row fills in at the bottom; no rows remount or change date group.

A held E key auto-repeats at the OS rate, and each repeat marked the
row that had just moved under the cursor: the prod database shows
bursts of 7 and 30 archives inside one second from one user. Every one
of those archives also refetched the whole loaded list, which is what
kept the stale sources on screen for seconds and made rows flicker.

The list hotkeys for Mark done, Mark as not done and Delete, and the
thread view's Mark done and Mark as not done, now act once per press
and swallow the repeat events.
…sortTs

GraphqlSoupEmailThread.sortTs is the server's per-view sort key
(INBOX: GREATEST(latest_inbound_message_ts, reminder_returned_at); ALL:
latest_non_spam_message_ts), but it is normalized onto the shared thread
record, so it belongs to whichever view fetched the thread last. The mail
backfill runs for everyone with emailView ALL and rewrites every thread's
sortTs with the ALL value. A cache re-read of the Signal page (the first
paint, or the affected re-read after an archive commits) then dated rows
by that value: threads whose reminder had returned jumped from "Earlier
this month" to "Last month" for the length of a network round trip.

INBOX queries now derive each row's sortTs from latestInboundMessageTs
and reminderReturnedAt, which every Soup page already selects, and keep
the server value only when neither is present. Other views keep the
server sortTs; the schema does not expose their inputs.
The done projection released its hide intent as soon as the thread's
record read done, even though the cached page membership still listed
the row. The row then rendered in Signal with the undo icon until a live
server page replaced the membership, and on a slow connection that took
seconds.

An excludes-done list now acknowledges Done only once the row is absent
from its raw page. Independently, the Signal and Noise tabs drop rows
whose record is done, which is the server's own inbox predicate, so a
stale page can never show an archived thread there.
…sor chain

When a refetch found that page 0's end cursor had changed, which any
archive inside page 0 causes, it destroyed every later page before
fetching the replacements one at a time. The list collapsed to one page,
the viewport's scroll position clamped to the new bottom, and the pages
then regrew beneath it: the user was dropped onto a different part of the
list while scrolling.

A page behind a changed cursor is now fetched first and swapped in only
once its rows have arrived, so the stale page keeps the list's height
meanwhile. A chain that turns out shorter still drops its tail.
Every committed archive refreshes every mounted Soup list, and each
refresh refetches the whole loaded page chain. A burst of archives
queued one chain refetch per commit, which is what piled up pending
requests against an already slow service and kept stale pages on screen.

A reader now runs one refresh at a time. A request made while one is in
flight shares a single follow-up that starts after it, so each caller
still gets a fetch issued after its own write without the burst.
The entry that marks a reader's refresh as in flight referenced itself
from its own initializer, which TypeScript rejects as a use before
assignment. A generation counter identifies the entry instead, so the
cleanup after a refresh still removes only its own entry.
…ase it on cancel

A replacement page whose refetch resolved with an error and no rows was
still swapped in, so the loaded rows it replaced vanished, which is the
collapse the rebuild was meant to avoid. The stale page now stays, the
error is reported through the refetch result, and the chain stops at
that index; the next refetch clears it.

A cancelled refetch (reset, query change, disable, destroy) also left
the detached replacement subscribed until its network result landed.
The abort signal now destroys it at once.
Two list refreshes requested in the same tick now share one follow-up
that starts after the refresh in flight, so the second one's effect is
observed after a microtask rather than synchronously.
The Soup email thread exposed `latestInboundMessageTs` and
`reminderReturnedAt`, the INBOX view's sort inputs, but not the
timestamps the other views sort by: `latest_outbound_message_ts` for
SENT and `latest_non_spam_message_ts` for every other view (see
`get_sort_timestamp_field`). Without them a client can only recompute
the INBOX key from the thread's own facts and has to trust the
per-view `sortTs` the record was last written with.

Both now ride the same thread metadata loader, so selecting them costs
no extra query.
`sortTs` is computed per view on the server, but the normalized cache
stored it as one field on the shared thread record, so whichever view
fetched the thread last decided how every other view dated it. The
INBOX-only derivation added earlier in this branch now covers SENT and
the ALL family too, mirroring `get_sort_timestamp_field` the way the
Rust mail index already does, and `sortTs` leaves the Soup fragment so
no per-view derivation is written to the record at all. The local Mail
page and the optimistic draft thread stop stamping one for the same
reason; their rows carry the facts the key derives from.
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: macro-inc/macro/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fb421ce8-879a-411f-828b-230ef6aa066d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

@synoet
synoet force-pushed the synoet/email-compact-rows branch from f19f220 to 7ab72f8 Compare October 17, 2026 12:43
@synoet
synoet force-pushed the synoet/soup-keep-pages-on-refresh branch from f509dfa to 77fd288 Compare October 17, 2026 12:43
Why: every committed mutation (mark done, mark read or unread, a new
task) refreshed each active Soup list after resetting it to its first
page. A list scrolled past its first 100 rows collapsed to them, losing
the rows and the scroll position, after each Done and each unread
thread opened.

What: the mutation-driven refresh refetches the loaded page chain in
cursor order without the reset. A page whose cursor changed is fetched
before it replaces the old one, so the list keeps its height while the
chain rebuilds.
@synoet
synoet force-pushed the synoet/soup-keep-pages-on-refresh branch from 77fd288 to fc29458 Compare October 17, 2026 12:56
@synoet

synoet commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Consolidated into #8247 with the other list stability fixes.

@synoet synoet closed this Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants