Skip to content

feat(ui): follow arrivals at the end statelessly, with explicit history navigation - #753

Open
glesage wants to merge 54 commits into
freenet:mainfrom
glesage:feat/simplify-message-and-scroll--anchoring
Open

glesage wants to merge 54 commits into
freenet:mainfrom
glesage:feat/simplify-message-and-scroll--anchoring

Conversation

@glesage

@glesage glesage commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
  • Test lines: +3541 -1041
  • Functionality: +1966 -1655

Why did we add lines of code for functionality? because we fixed some bugs/handled cases that were broken before. Otherwise we would have only reduced the line count 🥳

Summary

This PR replaces the latched follow pin with stateless rules. An arrival follows a reader at the end of a conversation in the foreground, and any other arrival keeps the reader's place. Room read markers advance only after the rendered range reaches the latest message, its bottom is visible, and nothing covers the conversation; reading part of the unread history does not advance the marker step by step. DM threads share the same follow, Latest and read rules, with the exceptions below.

Per the review discussion, following is kept, but stateless. An arrival scrolls to the newest message only when both of these hold:

  1. The conversation is in the foreground: the tab is visible, the chat panel is showing, and no modal covers it. Popovers attached to a message (the action menu, the reaction picker) count as modals; composer popovers do not.
  2. The reader is at or near the end: before the patch, the newest message's bottom is within 100px (room) or 50px (DM) of the view's bottom edge. These are the values used before this PR, now measured from the message rather than the scroller's absolute end.

The check reads the pre-patch DOM in the render that brings the arrival, and nothing is remembered between arrivals. That rules out the latched pin behind #486, #508 and #723.

Only an arrival follows. A tab becoming visible, a modal closing or a mobile panel reopening never scrolls: arrivals in the meantime stay below, Latest appears, and they stay unread until the reader reaches them.

Situation Before After
Message arrives with the reader at the end, or within the band View follows it, unless the pin had latched off (#486) Follows, in the foreground
Message arrives with the reader further up View stays View stays, Latest appears
Message arrives under a modal, or in a hidden tab Could follow, and was marked read Stays; closing the modal or showing the tab doesn't scroll
You send Snaps to latest, keeps following Scrolls once to the end; later arrivals follow by the band rule
Composer grows or keyboard opens Follows the end Room history preserves its bottom edge, or re-lands at the end while the end hold is active; DMs have neither correction
Rows grow right after opening (decrypt, images) Follows End holds until an upward scroll leaves the scroller more than 4px from its absolute end; a followed arrival holds again
The row you're reading is deleted Hidden panels held the row below The row above holds, visible or hidden
Latest button Beyond a 100px band, smooth When the newest message’s bottom is more than 4px below the view, or the room range withholds newer items; instant
Mark read Everything, even below the fold Room marker advances to the latest message only after its bottom is visible, the rendered range reaches it and no modal covers the room; re-checked (without scrolling) when the tab shows or a modal closes
DM threads Followed near the bottom (measured after the patch, so tall DMs were missed), marked read on render Inbound DMs follow within 50px, measured before the patch; visibility- and modal-gated reads; no end hold or bottom-edge correction
Burst past the history ceiling, reader at the end Following could slide the range forward Follows to the latest range; a parked reader's range holds its end and withholds newer items
Opening a message edit form Smooth, start-aligned reveal Instant reveal with nearest alignment on both axes, moving only as far as needed

Browser scroll anchoring stays off (overflow-anchor: none). Safari only supports it from 27.0, and Playwright's WebKit anchors where iOS 26 doesn't, so CI would miss the regression.

Accepted limitation — same-second DMs: DM read markers use whole Unix seconds. Reading one inbound DM also treats other DMs from the same peer in the same room with that timestamp as read, including one that arrives later or remains below the fold. We accept this behavior; timestamp precision and read tracking are unchanged. This addresses review item 1 as an accepted limitation.

Accepted behavior for now — end hold: An upward scroll releases the hold once the scroller is more than 4px from its absolute end. Padding below the newest message means a small scroll can release the hold while that message is still visible and Latest stays hidden. We accept this behavior for now; the release boundary is unchanged. This addresses review item 2 as an accepted limitation.

Accepted trade-offs of following:

The rationale and current rules are in .claude/rules/history-scrolling.md.

Related issue dispositions

These dispositions describe the current implementation at 6fdd5d4d. No open issue is automatically closed by this PR.

Issue Disposition in this PR
#486 — room following permanently latches off Restored, without the latch. Already closed by #495. Following is now a stateless check on each arrival, so nothing can latch. The original composer-growth and arrival scenarios assert that a reader at the end is followed.
#487 — inbound DMs fail to auto-scroll Restored. Inbound DMs follow a reader within 50px of the end, measured before the patch, so tall DMs follow too.
#507 — late images and bulk removals above a parked reader Deferred; remains open. General compensation for above-viewport image growth and bulk removals is not implemented. Both scenarios remain expected-failure browser tests in conversation-history-position.spec.ts.
#508 — stale pin plus maximum-scroll clamp can yank a reader Fixed by construction. The pin, reader_moved_up_since and the old follow paths are deleted, and following reads live geometry. The shrinking at-cap burst before settle is a row-preservation regression test, with its expected-failure marker removed.
#723 — arrival before scrollend prevents re-arming following Fixed. There is nothing to re-arm: an arrival patched before the reader's settle is followed, and the test asserts it.
#748 — scroll-test follow-ups Partially addressed; remains open. Several expected-failure markers are removed and shared helpers check that rows stay put after settling. Retry handling and failure-signature hardening for the remaining #507 expected failures, static-server hardening, timing/positive-control improvements, follow-up ticket accounting, the UnevenTail Rust fixture pin and the static-server restart note are not all completed here and remain follow-up work.
#751 — history-windowing follow-ups Partially addressed; remains open. Removing the follow pin eliminates the stale-hidden-pin mechanism, and Latest is instant. The re-keyed tail-group fix, a regression test for stale captures across unrelated renders, the newer-page DOM-bound assertion, deferred writes in both paging sentinels, and coverage of Latest when newer items are withheld remain follow-up work. The hidden-drain coverage is still not parameterized over both Rooms and Members.

Next: #459, opening a room at the first unread with a divider, as a separate PR.

Current behavior details

  • Room reads: the visibility check uses the latest message’s bottom with 4px slack, after the chat panel has layout and any reveal restore has completed, and only while no modal or message popover covers the room. The marker does not advance message by message while scrolling through unread messages or a held range. Once the latest message meets the check, the marker advances to it, covering earlier messages too.
  • DM differences: DMs share the instant navigation, the follow rule (50px band) and the 4px Latest/read visibility check, but have no room-style end hold or composer/keyboard bottom-edge correction. Their cutoff is the newest rendered inbound timestamp, checked only when the newest DM’s bottom is visible and nothing but the thread covers it. A thread opened empty has no opening placement, so its first DMs follow by the same rule; in a hidden tab, a burst leaves it at the top. This resolves review item 6.
  • History ceiling: the ordinary room window has a 240-display-item arrival ceiling (message groups and event summaries, not individual messages). Crossing it holds the range’s end for a parked reader, withholding newer items rather than sliding the start. A pending request (Latest, an own send, a followed arrival) whose render would hold the end takes the latest range instead, which resolves the own-send-at-ceiling issue in review item 4. Explicitly backfilled windows may exceed 240; their arrival-growth allowance is max(240, requested window + 60).
  • Edit-form reveal: mounting a message edit form now uses instant scrolling with nearest vertical and horizontal alignment, instead of a smooth, start-aligned reveal.

Changes

  • conversation.rs:
    • Deletes the follow pin and the code that only protected it (pinned_to_bottom, ScrollMark, reader_moved_up_since, the content-follow effect, the ResizeObserver follow branch, trim_landed, RangeHold.parked).
    • force_scroll becomes a one-shot ScrollRequest that carries its room.
    • follow_arrival_at_end requests one if the render brings a newer newest message (NewestKey/is_arrival) and the room was in the foreground within the band.
    • Adds the bottom-edge rule, the post-request end hold, the 4px Latest margin, last-good rows on a contended ROOMS read, and the ceiling rule for pending requests.
    • Fixes a newer-page jump of ~6,000px found in fix(ui): keep the reader's place across hidden panels, bursts and trims #747's last review.
  • foreground.rs (new): in_foreground, the one predicate shared by following and both read rules. Each modal root and message-row popover mounts a ModalPresence; FOREGROUND_CHANGED is bumped when a modal closes or the tab becomes visible. A source-scan test fails if a modal root lacks a presence.
  • document_title.rs and the seven mark-read sites: one rule, read_marker_update, fed by NEWEST_SEEN. Room history publishes its latest message only after the rendered range reaches it and its bottom passes the visibility check; intermediate messages do not advance the marker.
  • dm_thread_modal.rs:
    • Follows an inbound DM within the band: DmKey/is_newer_dm, measured in render.
    • Adds a Latest button and instant own send.
    • Uses ThreadSeenWitness for the timestamp read cutoff.
    • The body is keyed by thread, and a thread opened empty consumes its opening placement.
  • test_hooks.rs (test build only): hook senders join as members; new hooks for clock skew, a failed send, DM threads, and admitting a DM peer with no DMs.
  • Tests:

Test coverage

  • Full Playwright suite on all 5 projects with --retries=0: 913 passed, 0 failed (28 skips, all present before this PR). cargo test -p river-ui --bins: 1078 passed.
  • Mutation-checked:
    • the follow, own-send, deletion, end-hold, Latest and read rules;
    • for following: never follow, a 1000px band, ignoring modals, measuring after the patch, the DM except name, the empty-thread rule, the modal-close and tab-visible re-checks, and the ceiling request rule.
    • The DM lifecycle specs fail without the unmount guard.
  • Before/after screenshots of 15 scenarios, captured from scripted runs against the previous head and this one.
  • Not fixed: the two fix(ui): parked reader shifts on late-loading images and bulk removals above the viewport (post-#505 anchoring scope) #507 cases remain expected failures, deferred to an anchoring follow-up. Physical iOS/Android devices have not been tested.

Screenshots

image image image image

Redeploy

UI only. No changes under common/, contracts, delegates or cli/, and the committed WASMs are untouched, so no migration or pointer-record signing. Publish with cargo make publish-river after merge.

glesage and others added 17 commits October 7, 2026 17:48
… history

Eight focused areas (A01-A08) from the 10a plan. Reproduced defects on
main 739fd68 stay executable as targeted test.fail() expected failures:
freenet#723, freenet#508, freenet#507 (image and bulk removal), hidden-panel drain, render
ceiling eviction, and trim/backfill oscillation on a tall viewport.

Adds an example-data removeMessages hook, query-gated uneven-height
fixture rooms, shared geometry/settle-gate helpers, and serves the
static release build in CI instead of dx serve.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The oscillation starts as soon as the room opens, so paging into the tall
rows first raced it (1 setup failure in 25 on Firefox). Observe the opened
room with no reader input instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The scroll specs pointed at a plan doc that is not in the repo, and the delayed-image test could hang if the route was not installed before the request.

Co-authored-by: Cursor <cursoragent@cursor.com>
Narrow module-only exports in history-geometry and drop the unread
ReadingRow.nextKey. Fulfil the captured image route directly. Share one
bounded row-count observer between the two A05 tests, installed before
the return to the end. Let openOwnMessageEdit take an explicit message
so the last-message edit test reuses it without a preparatory scroll.

Full Playwright suite: 638 passed, 28 skipped (--retries=0). Expected
failures still reach their final assertions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ui-playwright-tests job restores all of target/, so a build that wrote
nothing left the restored index on disk and the disk-vs-HTTP bundle check
passed against it. build-ui-example-no-sync now deletes the selected public
directory and touches ui/src/main.rs before dx runs, locally and in CI, and
the readiness step rejects a missing or empty index before starting the
server.

scripts/tests/playwright-bundle-provenance-test.sh extracts the real cleanup
and readiness bodies, runs them against fixtures with stubbed python3/curl,
and pins the task and CI wiring. CI runs it first in ui-playwright-tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- example_data: one fixture_requested(flag), merged Deep/UnevenTail
  retention arm, deep-history rooms built in a (name, depth) loop.
- Specs: backfillOnce, expectParkedAwayFromEnd, CSS.escape row lookups,
  exported READING_ROW_BUDGET_PX, named fixture assumptions, corrected
  A01-A08 index and stale fixture comments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the example-output cleanup and the Playwright readiness step into
scripts/clean-ui-example-output.sh and scripts/serve-playwright-ui.sh,
called from Makefile.toml and build.yml. The provenance suite now runs
those scripts directly instead of extracting shell bodies with AWK, and
keeps wiring checks for the build, CI order and server/base-URL port.

The readiness poll tolerates empty captures under pipefail, so a poll
made before the server listens retries instead of aborting the step;
covered by a new case. Drops the redundant warm no-op case and folds the
missing/empty index cases into one loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three reader-position fixes from plan 10b, on the freenet#746 regression coverage:

- Hidden panel: remember the reading row by message identity while the
  history has layout, stand geometry paths down while it is hidden, and
  restore a parked reader to that row on reveal.
- Render ceiling: the rendered range is now [start, end). At the ceiling a
  parked reader's end is held (newer items withheld) instead of the start
  sliding past their row; newer history pages in from a bottom strip, and
  jump-to-latest / own send select the latest range first.
- Trim loop: trim eligibility uses the measured retained tail, not an
  average-height estimate, and the deferred trim re-checks room, layout,
  position and eligibility before acting.

Promotes the three matching known failures to ordinary tests and adds
coverage for the Rooms route, repeated hidden bursts, breakpoint reveal,
hidden bursts at the ceiling, bounded repeated bursts, newer paging and a
stale deferred trim.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… test

- The settle callback and the newer-history sentinel call
  `remember_reading_position` instead of copying its body.
- The standalone render-ceiling test's premises (surviving row, prune,
  first arrival, catch-up button) move into `parkPastTheCeiling`, and the
  repeated-burst test keeps its first-burst row assertion, so the
  duplicate scenario is removed.
- Comments now describe the measured retained tail as the current trim
  rule, and say that `ReaderPosition` cells never subscribe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shorten the comments added for the hidden-panel, held-range and measured
trim changes: plain statements of what each piece does, no em dashes,
and no restating of the history the tests already pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Arrivals and deletions no longer move the view (10c decisions 1, 2). The
follow pin becomes one-shot, instant requests for opening, own send and
Latest (3, 6); Latest shows once the newest bottom is off screen (4); height
changes, a deleted reading row and post-request reflow keep the place (10, 7, 13).
Tests: scroll specs rewritten for no-follow, new A02-A06 and end-hold cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every arrival and every tab hide marked the open room read, even with the
newest message below the fold or behind a mobile panel. Conversation now
publishes NEWEST_SEEN once that message's bottom is on screen with the tab
visible (10c decision 5), and all seven mark-read sites stop there.
Tests: A07 read cases, hidden-tab title, mobile panels, read_marker_update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An open DM thread no longer follows inbound DMs near the bottom (10c
decision 1), gains an instant Latest control and an instant own-send jump
(12), and advances DM_LAST_SEEN only up to an inbound DM that was on screen
with the tab visible (11). The modal body is keyed by (room, peer).
Tests: new dm-thread-scroll spec on populated threads via appendDms/deliverDm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main carries freenet#746 and freenet#747 as squashes of this branch's 10a and 10b, plus
the freenet#747 review rounds made after the local 10b copy. Each conflict starts
from 10c; the review changes were ported, converted or dropped as follows.

- ea39f78 (held end released when following resumes): fix moot, no following
  and a request selects the latest range. Kept the deep-history-retention
  fixture flag. Dropped its smooth-return test: Latest is instant, and the
  idle-reader test already covers a burst past the ceiling after an explicit
  request (view stays, DOM bounded, Latest reaches the newest).
- d6b73cf (following resumes after the final newer page): fix moot. Dropped
  both final-page tests and the paging test's follow check; an arrival after
  a newer page is covered by the ported newer-page test below.
- 5be0c33 (head slide compensated on a newer page): ported. The head
  reposition subscribes to window_items, plus "a newer page keeps the
  reader's row". The test failed on all five projects without the
  subscription (row moved ~6160px).
- 33ca2dc: ported the shared key_at closure; the parked gate is gone in 10c.
- c345eb5: ported the resolve_held doc links and the window doc (now on
  resolve_held, describing the end holding at the ceiling and keep), the keep,
  anchor, probe, trim and remember_reading_position doc fixes, and the shorter
  Latest comment; dropped the no-room hamburger capture. Kept 10c's bool
  restore_reading_anchor (the bottom-edge fallback uses it) and its settle
  comment; the dead_code allow on hidden was already there.
- b02f904, 3f75aba: withheld moved into history-geometry.ts; dropped the
  room-list duplicate of the hidden drain and folded the two-burst test into
  it; parkAboveEnd; dropped implied premises (withheld after Latest, the
  paging row-held and DOM-bound checks, the hidden-ceiling chevron check).
- 36928b7, babe31b: trimmed the trim-geometry test to its boundary pair,
  folded the held-end fallback into the paging test, and kept one
  both-neighbours case asserting 10c's previous-first order.
- contract-version.txt: main's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`readerScrollsWithoutGesture` and `readerReturnsToEnd` now share one
`scrollAndSettle(page, offset | "end")` that registers the `scrollend`
listener before the move, returns at once when the offset did not change,
and removes the listener and timer on every path. `holdTestImage` moves
into `history-geometry.ts` and also drives the parked-image regression.
The DM thread spec's three openings share `prepareThread(page, count)`.
`ReadingRow`, `RowEdge` and `rowGapFromViewBottom` are no longer exported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…guard

Replace the head reposition's `(String, i32, i32)` snapshot with a private
`RepositionAnchor { probe_key, probe_top, scroll_top }`, matching
`BackfillAnchor`, so the row offset and the scroll offset are no longer
told apart by tuple order. Capture, `take()` consumption, the pre-patch
scroll offset, the request/hidden-layout guards and
`drop_pending_corrections()` are unchanged.

State `read_marker_update`'s monotonicity as an explicit guard: look up the
existing marker's index, return `None` when it is at or past the seen
message, else mark the seen message. Same semantics as the nested
`is_none_or` form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@glesage
glesage marked this pull request as ready for review October 8, 2026 02:53
glesage and others added 12 commits October 8, 2026 14:14
Delete tests that pinned the removed follow-to-bottom behaviour or that
other tests already cover, and merge pairs that ran the same scenario:

- autoscroll: drop the gesture-less scroll, parked windowed arrivals,
  parked resize and send-from-the-end tests; fold the first-burst row
  hold into parkPastTheCeiling and the arrival-after-send leg into the
  send-from-the-top test; cut the draft-open arrival loop to 2.
- dm-thread-scroll: drop the opening and short-thread-overflow tests;
  fold the at-end inbound check into the read-rule test and the Latest
  read check into the Latest jump test; tag the read-rule describe
  @chromium-only.
- room-unread-badge: drop the old-100px-band and short-arrival tests;
  keep only the members-panel A07 test and tag it @chromium-only.
- Rust: merge the same-messages end-hold test into the shrinking-range
  one, merge the two marks-nothing read-marker tests, and drop the
  hide-tab mark test that read_marker_update's tests cover.

Also drop the vestigial WheelEvent in readerScrollsTo: the app has no
wheel, pointer or touch listener.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- example-room.ts: nextFrames and settle (re-exported from
  history-geometry, which imports example-room), hiddenTitleCount and
  roomUnreadBadge, replacing per-spec copies in the DM, room-unread and
  autoscroll specs.
- history-geometry.ts: hideChatBehindMembers, now also used by the A07
  members-panel test, and one NEWEST_IN_VIEW_SLACK_PX for the room and
  DM specs. expectRowHeld and expectHeldFromViewBottom use settle.
- Rename BOTTOM_THRESHOLD_PX to WELL_AWAY_FROM_END_PX, its only meaning
  now, and drop the history of the removed bottom band.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep each scroll-anchoring rationale once, on its doc comment
(`reposition_pending`, `grown_window`, `opening_snap_done`,
`trim_is_due`, `BACKFILL_LEAD_PX`), and point to it from the other
sites. Drop comments that narrate the old behaviour.

Use `Cell::replace` for the DM auto-scroll triggers, add
`with_current_room_mut` to the test hooks, compute the DM stamp step
once, and narrow `thread_read_needs_write`, `mark_thread_read` and
`mark_current_room_as_read_on_hide` to private.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…om and DM

The room history and the DM thread each carried their own slack constant,
sentinel-in-view check, instant scroll-to-end and Latest button markup. Move
them into `components/scroll_to_latest.rs` (`NEWEST_IN_VIEW_SLACK_PX`,
`sentinel_in_view`, `scroll_to_end`, `LatestButton`) and add a
`dm_scroll_container()` helper for the DM's repeated lookup. Each caller keeps
its own handler; the room's still defers its signal writes. Classes, test ids
and aria labels are unchanged.

`sentinel_in_view` keeps the DM's zero-height guard; the room only calls it
after `history_has_layout`, so its behaviour is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mark_current_room_as_read now only advances the open room's marker to
NEWEST_SEEN, and DocumentTitleUpdater's effect (always mounted in App)
already runs it on every change to CURRENT_ROOM (.read()), ROOMS and
NEWEST_SEEN (try_read, nudged on Err) and DOCUMENT_VISIBLE (.read() in
update_document_title), behind signal_guard::anchor(). Each removed
caller sat right after a write to one of those signals, so the effect
re-ran and did the same thing:

- room-list click: right after CURRENT_ROOM.write().
- room_synchronizer apply_delta_inner / update_room_state_inner: after
  ROOMS.with_mut; their visible-and-current gate was narrower than the
  read rule, which only marks what NEWEST_SEEN says was on screen.
- response_handler post-load defer: after the ROOMS merge, also calling
  update_document_title, which the effect calls too.
- tab becoming visible: right after DOCUMENT_VISIBLE.write().

The function is now private to document_title.rs, its caller list in
the doc is trimmed, and the now-unused imports go. No source-scrape pin
referenced these call sites; signal_guard's lists are unaffected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
window_anchor, window_tail, window_rendered and window_overgrown were four
separate Rc hooks, cloned into every effect and callback that needed them.
They now live in ReaderPosition with the rest of the render's inter-render
memory, so install_scroll_settle_listener takes (window_items, reader).
ReaderPosition::forget_range replaces the three copies of "drop the last
range and end the hold".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d_held

Both scrolled to the end, recorded the hold, remembered the reading position
and checked the read rule, in the same four lines. hold_end does it once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The range calculation and its write-back to ReaderPosition move out of the
render body into resolve_rendered_range, a plain function of the display
items, the requested size, the open room and the reader's cells. The three
`if select_latest { None } else { .. }` reads become one tuple. The DOM probe
for the head reposition stays in the render, gated on the returned
head_removed exactly as before.

The history-window pin now also checks the render calls the helper, and its
write-back needles name `reader` instead of `reader_position`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ResizeObserver callback sat four closures deep inside Conversation(). It
is now on_history_resize, with observe_history_resize installing it the way
install_scroll_settle_listener installs the settle listener. Both installs
share one effect, each with its own success flag, so each still retries on
every message_groups change until it finds the container.

The settle-listener pin now matches `listening.set(install_scroll_settle_listener(`,
the call's new shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…thread

`observe_sentinel` in scroll_to_latest.rs builds the IntersectionObserver
both Latest controls use (same root margin and threshold) and returns a
`SentinelObserver`, the DM thread's `NewestDmObserver` renamed, which
disconnects on drop. The DM thread keeps holding it per mount; the room
leaks it as before, since its component is never unmounted.

Both callers now read the last queued entry, which the DM thread already
did. With one observed target, entries only queue up when the sentinel
crossed the edge more than once before the callback ran, and the last one
is its current state; the room used to apply the first, stale one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The backfill restore, the head reposition and request completion were
three effects; they are now one, in that order. `BackfillAnchor` and
`RepositionAnchor` become one `ProbeCapture`, whose `shift` measures the
probe row (the backfill keeps its `scrollHeight` fallback as
`scroll_height: Some(..)`). Each correction keeps its own rule: the
restore applies only `shift > 0`, the reposition needs layout, applies any
non-zero shift and clamps at 0, and both stand down while a request is
pending.

Subscriptions: the restore now also wakes on `message_groups`. That is
harmless: its capture is taken in the same handler that grows
`window_items`, and effects only run once no scope is dirty, so the first
pass after a capture is always the one after the grown render, as before.
The order between the three was up to Dioxus's task wake order; it is now
fixed with both corrections ahead of the completion that clears the
request they defer to.

Pins: the H3 pin now reads `correct_view_after_patch` and checks both
captures are taken and the request guard returns before either applies
(it counted two copies of the guard). The head-swap pin's measurement,
pre-patch offset and probe-restore needles follow the new code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`the_window_trims_at_the_bottom_and_backfill_waits_for_the_opening` keeps
the H2 and H3 races and the deferred trim's room re-check, which no
browser test switches rooms for. The settle-handler trim, the slack gate,
the deferred re-measure and the geometry gate go: the conversation-autoscroll
"trims once and stays bounded", "stands down when the reader moves" and
"very tall viewport" tests cover them on every project.

`head_swaps_reposition_the_reader` keeps the pre-patch scrollTop (freenet#505)
grep and the backfill probe measurement, which no browser test exercises
(it needs an arrival batched into a backfill's patch). Row identity, the
capture, the measured shift and the probe walk go: the batched at-cap
drain, newer-page and backfill paging tests cover them on every project.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
glesage and others added 4 commits October 8, 2026 16:22
holdNextDmPlacement holds the next DM thread placement (the opening or
own-send jump) inside its task, before it touches the DOM, until
releaseHeldDmPlacement runs it; heldDmPlacementCount and
releasedDmPlacementsRun let a spec wait on both ends. Release throws when
nothing is held. The thread routes the placement through the hook only
in the wasm32 example-data + no-sync build; elsewhere it runs directly.

appendDmsForPeer and deliverDmForPeer add a second fixed DM identity
(peer 1, "Other DM Peer"); appendDms and deliverDm are unchanged for
peer 0. The thread modal and its close button get data-testids.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A thread's queued opening or own-send placement finds the thread by the
global dm-scroll-container / dm-bottom-sentinel ids, so if it runs after
its thread unmounts it acts on whichever thread is open by then. The
spec holds a real placement across the unmount and releases it over a
replacement thread, over a reopened instance of the same thread, and
with nothing open; a held placement whose thread stays open must still
land, so a guard that disables every placement cannot pass.

Against the current modal the three replacement cases fail on every
project: the replacement thread jumps to its end (about 750-840px), the
closed thread is marked read from the replacement's end, and the
replacement's DM below the fold is marked read too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The opening and own-send placements run in a task queued after the
render, and they and the read witness reach the thread through global
DOM ids. A placement queued by a thread that has since closed scrolled
whichever thread was open by then, and counted that thread's visible end
as evidence that the closed one was read.

Each mounted thread body now owns a lifetime cell, cleared synchronously
by use_drop. The placement checks it as it runs, before any DOM lookup,
and ThreadSeenWitness::check before measuring. A reopened thread gets a
fresh cell, so (room, peer) equality cannot revive an old instance. Both
hooks sit above the view's early return. Read cutoffs, the deferred
mark_thread_read and the signal-safety gates are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@glesage

glesage commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

All issues from #753 (review) have been solved

@sanity sanity left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review: #753 (head 6d34ed6f)

@glesage, thanks. This is a careful piece of work, and two of the three blockers from #732 are now fixed. It can't merge yet, though, for two reasons: removing automatic following needs a maintainer decision first, and there are code and test findings to fix regardless of how that goes.

Verdict: changes requested.

How this was reviewed. Full tier.

  • Code: read by four reviewers: Claude (skeptical/state-machine, testing, big-picture + code-first) and OpenAI Codex. The diff was read for malicious changes first. Nothing was found, and the test hooks are compiled only into example-data + no-sync builds.
  • Visual check: the reviewed head was then built in a credential-free sandbox and driven in a no-network browser on iPhone 13 (WebKit), Pixel 7 and desktop (Chromium), light and dark.
  • CI is green on 6d34ed6f, and the PR merges cleanly into current main.

Blockers from #732

# #732 blocker At 6d34ed6f
1 Hiding the chat column on mobile changed the reader's position state Fixed. A hide only sets hidden, zeroes observed_height and clears the end hold. Captures and settles refuse to run without layout, and the reveal restores the last visible capture. Confirmed in the sandbox: on iPhone and Pixel, a reader scrolled 400px up who opens the room list while a message arrives comes back on the same content (distance from the end grows only by the new message).
2 Scroll-to-latest animation could clear the pin or be aborted Fixed / moot. Every ScrollBehavior::Smooth is gone, Latest is instant, and there is no pin left to clear.
3 mobile-touch-ux.spec.ts:277 failure dismissed after isolation reruns Still open. See Must 2.

Must fix

  1. Removing automatic following is a product decision, and the maintainer hasn't made it yet.
    • main records following as current policy: conversation-autoscroll.spec.ts:844 says "CURRENT POLICY: main intends to follow here. Dropping arrival-following would … change this test", and #486 treats "stops following" as the bug.
    • Nothing in this thread or the earlier ones records that decision being taken. Please don't treat it as settled until @sanity decides.
    • What it means for users, confirmed in the sandbox build: with the reader at the end, three arrivals land entirely below the fold (about 350px on desktop, about 370px on phones). The only signal is the 40px ⌄ button in the corner, with no count or label.
    • Nothing else announces the arrival either. notifications.rs:1200 skips the open, visible room, and the room-list and hamburger badges exclude the current room. A window left open on a second screen stays frozen at the last message.
    • For comparison, mainstream chat clients follow a reader who is at the end, and show a counted "new messages" pill only when the reader has scrolled up.
    • If following stays removed: the arrival needs a real signal, at least a count or a "N new" label on Latest.
    • If it comes back: the stateless pieces this PR adds (request_end, the live sentinel measure, the pre-patch DOM read) look like enough to follow at the end without the old latched pin. For example: "the sentinel was in view before the patch and the patch is an arrival → request_end(room)". That would fix #723 and #508 without removing the behaviour.
    • Either way: please retitle (this is a behaviour change, not a refactor, and the title becomes the squash commit and release-notes line), and give each of #486, #487, #507, #508, #723, #748 and #751 a stated disposition.
  2. The mobile-safari flake from #732 is unresolved, and the new end hold keeps its mechanism alive.
    • "never more than one menu open at a time" (mobile-touch-ux.spec.ts:277) is unchanged in this PR. It reads kebabs.nth(2).boundingBox() after selectRoom with no settle, then clicks that coordinate.
    • The opening now installs an end hold that re-lands the view on row growth (decryption placeholders in "Your Private Room", on_history_resize ~3916). That is exactly a late scrollTop write in the window between the read and the click.
    • Please make the test wait for stable geometry (the same box on two consecutive frames, or the settle helper), and show it holding with --retries=0 --repeat-each=20 on mobile-safari under worker load, not in isolation.
  3. The read rule's range guard has no browser test.
    • The rule says a room isn't marked read unless the rendered range reaches the latest message (if !history_window.has_newer { newest_rendered = … }, ~1371). Only the pure read_marker_update is unit-tested, and it knows nothing about ranges. Dropping the guard would pass every spec.
    • In "reading down a held range pages the withheld messages in": read to the end of the held range, leave, assert the badge is still shown. Then page to the newest, leave, assert it's gone.

Should fix

  1. An own send at the window ceiling withholds the sent message (conversation.rs ~3152–3191, ~5142).
    • select_latest reads has_newer from the previous render. With the range exactly at the cap and ending at the newest message, has_newer is false, so the send's new item makes end - start > cap.
    • With RangeHold.parked gone, hold.end.is_none() now caps the end at start + cap. The sent message is withheld and complete_scroll_request lands at the end of a range that doesn't contain it.
    • main slid the start here. When a request is pending, slide the start (start = end - cap) instead of capping the end.
  2. On mobile, an arrival in the current room while the chat column is hidden has no signal at all (document_title.rs:639, room_list.rs:601, notifications.rs:1200).
    • The read marker correctly stays behind. But the hamburger total and room badge exclude the current room, notifications are suppressed for it, and Latest is inside the display:none column.
    • Scenario: open Members or Rooms, a message arrives in the current room, and nothing indicates it until the user goes back to Chat.
  3. An empty DM thread auto-scrolls on its first inbound DM (dm_thread_modal.rs ~488).
    • The placement effect waits until a bubble mounts, then is_first = !first_scroll_done.replace(true) treats that bubble as the opening jump, whether it is inbound or outbound.
    • The first inbound DM into a thread opened empty therefore scrolls to the end, and the witness marks it read. That contradicts "inbound DMs never scroll".
    • Count the opening as done once the thread has rendered, even when it's empty.
  4. A contended DM_LAST_SEEN.try_peek() drops the read advance with no retry (direct_messages.rs:87). The room path schedules a nudge on the same contention; this path should too. Otherwise the last witness after reaching Latest can leave the thread badged until it's reopened.
  5. Deleted tests that still guarded live behaviour. Please restore these, or show what replaces them.
    • "does not drag a parked reader down when a resize reflows the history" (old conversation-autoscroll.spec.ts:365). The reflow test now only covers a reader at the end, and the new bottom-edge correction moves scrollTop on height changes.
    • The arrival after the final newer page is rendered, not withheld (old ~1205/1230, the #747 review item). "reading down a held range…" dropped its trailing arrival check.
    • "arrivals do not move a reader parked in a windowed room's history" (old ~526). Only the Rust test remains.
    • The "newer-page jump of ~6,000px" fix in the description. Please name the test that fails without it.
  6. Tests for new rules that are stated but not pinned:
    • Hidden panel with the reader at the end: hide, deliver, reveal, expect the row held and Latest shown. Then hide, grow a held image, reveal, expect the end still in view.
    • The 4px end-hold release boundary: scroll up 3px (hold kept) and 6px (released). Today a threshold of 2 or 3 passes.
    • overflow-anchor: none: the PR's own argument is that CI's WebKit anchors where iOS doesn't, yet the only guard is an include_str! scrape. Assert the computed style in Playwright.
    • test.skip(!hasScrollend, …) silently skips on an engine without onscrollend. Assert it's present on the projects that should run instead.
  7. The new agent rule files encode the contributor's choices as settled project policy.
    • .claude/rules/history-scrolling.md says the change "deliberately replaces automatic following", that it "supersedes" #486, and that "the tradeoff is explicit". Its "Accepted for now" notes (the end-hold release inside the padding, same-second DMs) record known defects as policy, so future agents will leave them alone.
    • Please reword it as a neutral description of current behaviour, at least until the maintainer decides.
    • Dangling references: "decisions 6, 12" in scroll_to_latest.rs; NEWEST_DM_IN_VIEW_SLACK_PX in history-geometry.ts (doesn't exist); KNOWN_FAILURE_SHA "main 739fd68".
  8. The description doesn't match the code in four places.
    • "Up to the newest message seen" means "only once the newest message's bottom is on screen": reading 24 of 30 new messages marks none.
    • "DM threads: same rules as rooms" isn't true. DMs have no end hold and no bottom-edge correction, as the rule file says.
    • The edit-form reveal changed from smooth/start-aligned to instant/nearest, which isn't mentioned.
    • At the 240-item ceiling, a reader idle at the end now gets a held range, which isn't mentioned.

Consider

  • A backfill releases the end hold. end_hold_survives needs now.count <= before.count. On a tall screen where the opening tail is shorter than clientHeight + BACKFILL_LEAD_PX, the first backfill clears the hold the opening just set, so images or decryption landing afterwards push the newest message off screen. Treat "count grew and the newest is unchanged" as a backfill that keeps the hold.
  • The end hold never expires. Any later growth (a reaction pill, a late image, minutes later) re-lands the view at the end until the reader scrolls up more than 4px.
  • A modal over the history still counts as seen. In a short room, an arrival that fits on screen is marked read while a DM thread or member modal covers it.
  • Sequencing. HostFat's open #743 and #745 touch conversation.rs. Plan the rebase order with them.
  • Pre-existing, not from this PR. A send in a private room without its secret falls back to a public body that river-core rejects after the draft is cleared, so the message is lost silently. The new spec at diff ~7536 relies on that path; it needs its own issue.

Verified as fine

  • Room switch with a pending request. It's replaced in render before the patch, the own send re-checks CURRENT_ROOM, and captures from the old room are dropped.
  • Read rule interleavings. Every interleaving traced came out right. Nothing marks while the tab or panel is hidden, marking never goes backwards, and all four removed mark-read call sites (hydrate, synchronizer ×2, visibility) are covered by the title effect.
  • DM lifecycle. The keyed body, the use_drop lifetime flag checked by queued placements and the witness, and inbound DMs never scrolling an open thread.
  • Signal safety. Raw callbacks defer every signal write, render touches only Cell/RefCell, and the new effects anchor before their fallible reads.
  • The rewritten #486-family tests keep their triggering events and can fail. The test hooks drive real production paths (releaseHeldDmPlacement throws if nothing is held).
  • "UI only." All 27 files are under ui/ or .claude/rules/; there are no Cargo, WASM or contract changes.

HEAD SHA reviewed: 6d34ed6fd8e6f44eca6919ba91eb20d9931c152d

[AI-assisted - Claude]

@glesage glesage changed the title refactor(ui): simplify history preservation and remove automatic following feat(ui): replace automatic message following with explicit history navigation Oct 9, 2026

glesage commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Updated the PR description for review item 11:

  • When rooms count as read, and how DM scrolling differs from rooms.
  • Instant edit-form reveal with nearest alignment.
  • Held ranges at the history ceiling.
  • The unresolved first-inbound jump in an empty DM thread.

For item 1, the description now says removing automatic following needs a maintainer decision. That decision, arrival-signal requirements, and the remaining code/test findings are still open.

@glesage

glesage commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the review @sanity !

  1. Mobile-safari flake from refactor(ui): the history keeps its newest visible message anchored #732 is unresolved: I don't think I was intending to fix that, it's already an issue on main and this PR does not change how often it happens or claim to fix
    Happy to look into it but it might make the PR quite a bit bigger and cause cycle of feedback hell again heh

  2. Read rule range guard missing test: This is actually already an issue on main, and is mostly theoretical as the render ceiling (240) is often (always?) higher than the room's max recent messages (200 for freenet I think, not sure for freenet builders?).
    Happy to try to add the test but ideally I'd like to avoid going down the cycle of hell that I often get into when I start adding too much in 1 PR :P

@glesage

glesage commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Fixed review item 8 in 39b47e9.

  • Restored the parked-reader resize and windowed-arrival tests, plus the final-page outside-band test.
  • Restored both post-paging arrival checks. They now assert that arrivals render without moving the reader.

All five scenarios passed across all five browser projects: 25 checks, retries disabled, on a fresh static build.

"A newer page keeps the reader's row" already covers the ~6,000px jump. Disabling the paging compensation made it fail in all five projects with ~6,200px shifts. I reverted that experiment.

The automatic-following decision and the other Must fix items, including the held-range unread-badge check, remain open.

@sanity

sanity commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@glesage, a decision on Must 1 from my review: keep automatic following.

If the reader is at the end of the history when a message arrives, the view should move to show it, as it does on main. If they've scrolled up, it should stay put and Latest should appear. That matches what people expect from a chat client, and it keeps a window left open on a second screen live.

The goal of fixing the follow bugs (#723, #508) is still right. Please get there without the old latched pin, using the stateless pieces this PR already adds. Roughly:

  • Before the patch, read whether the newest message is on screen (the live sentinel measure / pre-patch DOM read).
  • If it was and the patch is an arrival, call request_end(room).
  • No follow state, so nothing can latch.

The end hold, the bottom-edge correction and the reveal restore can stay as they are.

Concretely, for this PR:

The other findings in the review (the mobile-safari flake, the read-rule range test, the own send at the ceiling, the DM first-arrival scroll, and so on) still apply.

[AI-assisted - Claude]

@glesage

glesage commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@glesage, a decision on Must 1 from my review: keep automatic following.

I responded in freenet builders River room as well but posting here for archive:

Oh heh, if that's the case I'll just close the PR as the point of it is to remove automatic following (:

When checking Discord, Slack or others they don't auto-scroll down to new messages when you are at the bottom or when you open new rooms, they always keep you where you were. Personally I also felt that in river when I click on different rooms or open river in new tab I always found it frustrating that it did not stay where i was last time, so I have no way of reading up on the new messages

If we feel that's not the right decision I'm happy to close the PR

@sanity

sanity commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@glesage, thanks for the questions on River. Let's keep this discussion here on the PR, where it's easier to follow.

Mobile-safari flake: agreed, it's on main and out of scope here. It's now tracked as #762, so it doesn't block this PR.

Range-guard test: agreed, it isn't blocking. With the 240-item ceiling against a 100-message default cap, it's close to unreachable, so skip it unless a cheap unit test falls out naturally.

Following: I think two behaviours are getting mixed up here, and you're right about one of them.

  1. A message arrives while you're at the bottom. As far as I know, Slack, Discord, Element, Telegram and WhatsApp all show it: the view moves with the new message, and a "new messages" indicator only appears once you've scrolled up. That's the behaviour Ian wants kept.
  2. Where a room opens. Here you're right that those apps don't throw you to the bottom. They open at your first unread message with a "New messages" divider, so you can read up from where you left off. That's feat(ui): jump to first unread message + unread divider #459, which Ian filed for the same frustration you describe.

#753 still jumps to the end when a room opens (in the description: "Opening, sending and Latest move to the end instantly"), so removing following doesn't actually fix the problem you hit when switching rooms or opening River in a new tab. #459 does.

Please don't close this. Most of the work here stands on its own:

  • the "marked read only up to what was seen" rule;
  • the hidden-panel fix;
  • the DM lifecycle guards;
  • the end hold and the bottom-edge correction.

A path that gets you what you want:

  1. Here: keep following for a reader who's at the end. The stateless version (newest message on screen before the patch, plus an arrival, means request_end(room)) avoids the latched pin behind fix(ui): conversation stops following new messages (permanent latch, not intermittent) #486, Autoscroll: a message arriving before the reader's scrollend leaves the view one row short #723 and fix(ui): stale bottom-pin plus the max_scroll_top clamp can yank a reader who scrolls up just before a content-shrinking arrival #508.
  2. Next PR: feat(ui): jump to first unread message + unread divider #459, opening a room at the first unread with a divider. That's the "stay where I was" behaviour.

If you see it differently, say so here and Ian can weigh in.

[AI-assisted - Claude]

glesage and others added 12 commits October 10, 2026 11:32
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every modal root, and the two popovers attached to a history row, mounts a
ModalPresence. It registers the modal while mounted and bumps
FOREGROUND_CHANGED when it unmounts; the tab becoming visible bumps it too.
A source scan pins a presence in each root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The room and DM read rules now ask foreground::in_foreground, and re-check
on FOREGROUND_CHANGED (the tab became visible or a modal closed) instead of
on DOCUMENT_VISIBLE alone. An arrival on screen under a modal, or under a
popover attached to a history row, stays unread until it closes; closing it
reads what is on screen without scrolling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a render brings a newer newest message and, before the patch, the room
was in the foreground with its newest message's bottom within 100px of the
view's bottom edge, the render asks for one move to the end. Stateless: the
check reads the pre-patch DOM, so nothing can latch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A followed burst, or an own send, whose arrivals take the range past the
render ceiling used to land at the end of a held range with the newest
messages still withheld. A pending request now takes the latest range
instead, which also resolves the own-send-at-ceiling review item.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The freenet#486, freenet#501, freenet#723 and freenet#747 scenarios now assert that a reader at the end
is followed, and parked readers keep their place. Tests whose premise was an
arrival landing unread below a reader at the end park the reader first, or
deliver in a hidden tab.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The render that first shows a newer DM measures, before the patch, whether
the thread is in the foreground with its newest DM's bottom within 50px of
the view's bottom edge; the placement effect then follows it. A thread
opened empty has no opening to place, so its first DMs follow by the same
rule (review item 6). Adds an admitDmPeer test hook for an empty thread.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@glesage glesage changed the title feat(ui): replace automatic message following with explicit history navigation feat(ui): follow arrivals at the end statelessly, with explicit history navigation Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants