fix(mobile): settle hydrated threads on latest reply

[loganj]

🤖

Summary

Mobile threads could open above the newest reply because the reply query hydrates across relay pages while the list is still being laid out. Ordinary thread opens now wait for authoritative hydration and late layout before settling on the latest reply.

The initial settle is generation-guarded: if another reply arrives while it is pending, the stale target is discarded and the current tail becomes the target. Explicit deep links still own their requested position, existing threads only follow remote replies when the previous tail was visible, and local sends remain visible.

Related issue

No matching issue found. This is separate from the channel unread-navigation behavior in #4239.

Originating Buzz thread: buzz://message?channel=a9081ecd-9be0-400b-8bf9-2e8e0d385b80&id=bfb289fc53754f62f641fbf58bf2d7a9c181a3e6eb09a6ba762aeb6904b6cde4&thread=bfb289fc53754f62f641fbf58bf2d7a9c181a3e6eb09a6ba762aeb6904b6cde4

Testing

• Added a widget regression covering paginated hydration plus a live reply arriving during the initial settle.
• Full mobile Flutter test suite passed; flutter analyze passed.
• GitHub CI passed, including the Mobile job.
• Built, installed, and launched the debug app on an iPad Pro 11-inch (M4), iOS 18.6 simulator. An authenticated manual thread traversal was not performed because the fresh app was not paired to a relay account.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01989b93e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 290f4b6a5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[wesbillman]

Reviewing on Wes's behalf. The hydration/deep-link generation handling looks sound, but these two user-visible scroll-state defects need correction before merge. I am requesting changes rather than approving.

[brow]

🤖 Review of 290f4b6a (mobile initial thread tail settle). Comment only: no approve and no request-changes from us. Two independent agent reviews plus a third confirming run by me. Every number below is execution on the host VM (flutter analyze plus the FULL flutter test suite, Flutter 3.41.7 via the repo hermit shim). No simulator run, see the scope bound at the end.

The fix is real

At the merge base, after paginated hydration completes the tail widgets are not built at all and the list stays on the top anchor. At this head they are built and land near the dock. So the change addresses that mechanism, not a nearby one. flutter analyze is clean and the full suite is 1163 passed / 0 failed / 0 skipped at this head.

The two P2s in the changes-requested review reproduce, but both are pre-existing at the merge base

Stating this first because it decides whether they are this PR's to fix.

Composer overlay classified as visible space. Reproduced. At 400x800 with the dock top at 738 logical px, sweeping reply counts gives a tail bottom of 720 at n=13 (clear), 756 at n=14 and 792 at n=15; at n=14/15 the newest reply's text is not hit-testable, and the settle declines to scroll even though a manual drag afterwards does move the tail, so spare extent existed. Control: those numbers are byte-identical at the merge base for n=1..15. In that band a top-anchored list already parks the tail under the dock with or without this change. The visibility predicate neither causes nor fixes it.

User opt-out after the settle. Reproduced at this head: after the settle, a slow multi-step drag toward older replies leaves the tail, and then opening the keyboard yanks the reader back to it. Control: reaching the tail by user drags alone on a fully hydrated thread (no settle involved, identical code path in both revisions) and then repeating the drag plus keyboard yanks at this head AND at the merge base. The yank lives in the existing followsThreadTail / realignThreadTailAfterMetricsChange logic. It is also bounded: a single large drag that clears the tail in one position update does not yank, which confines it to slow or short drags. This change raises the encounter rate by making the tail reachable more often; it does not introduce the behavior.

One geometry problem that IS new here, and it is the larger half of the composer-overlay comment

alignment: 0.8 positions the target's leading edge at 80% of the viewport, so a tall newest reply (436 px in our probe) settles with its top at 640 and its bottom 338 px below the dock top. At the merge base that reply is not built at all, so unlike the n=14/15 band this one is head-only. Comparing against the usable area above the dock, as the bot comment suggests, would fix it; the alignment constant against the dock height is where it bites, not the visibility predicate.

Three mechanisms this change introduces are not pinned by any test

Mutants run against the full suite; shipped tests untouched in every run.

• Removing both generation != _generation checks in InitialThreadTailSettle, and the generation == _generation completion latch with them: full suite stays 1163/0/0 GREEN. Measured independently by two reviewers and then reproduced by me in a third worktree, with a pristine control and a post-restore control both at 1163/0/0. The generation guard the PR description leads with is unpinned.
• Disabling the fully-visible skip (the targetIsFullyVisible early return added for short-thread anchoring): GREEN. The short-thread regression test added alongside it passes either way, because two replies fit above the dock in both branches.
• alignment: 0.8 changed to 0.0: GREEN.
• Controls that do fire: disabling the settle entirely goes RED (initial thread hydration settles on the latest reply after pagination), and collapsing the two chained post-frame callbacks into one goes RED. So the headline behavior and the pagination wait ARE pinned. This is a coverage gap on a demonstrated fix, not a doubt about the fix.

Undocumented behavior change

The diff also tightens wasAtTail in ThreadDetailPage from "positions empty, or any position at or past the previous last index" to an exact-index match. That changes when an already-open thread follows a remote reply, is not mentioned in the PR description, and reverting it to the merge-base form leaves the full 1163-test suite green. Worth either a sentence in the description or a test, since it ships inside a PR whose stated subject is the initial settle.

Interleavings: nothing unsound found

Hydration page N arriving after N+1, a live remote reply mid-settle, a local send mid-settle, two settles racing, and dispose mid-settle (popping the route between hydration and settle completion produces no exception and no scroll-after-dispose; context.mounted plus controller.isAttached cover it). Each schedule bumps the generation so superseded callbacks return early, and completion only latches for the current generation. Bound: five named interleavings, searched by probe, not an exhaustive proof.

Scope bound, not verified

All geometry above is host-VM at 400x800, 400x700 and 400x900 logical px with devicePixelRatio: 1 and synthetic uniform replies. Real safe-area and home-indicator insets, iOS keyboard animation timing, and dynamic type were not exercised, because this review ran without a simulator. Multi-page hydration was simulated with a completer plus staged live arrivals; genuine 200-per-page relay paging was not driven end to end. A device or simulator run would still be needed to show the settled tail's bottom edge against the real dock on an inset device, and whether the slow-drag keyboard yank survives a real keyboard animation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25cc503e15

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Re-enable tail following when users return to tail

Since this flag is set on every drag start and is never cleared, a user who scrolls up to read older replies and then manually scrolls back to the newest reply remains opted out of tail following. The new guards around composer and keyboard realignment then keep returning false even while the tail is visible, so focusing the composer after returning to the bottom can obscure the latest reply; clear the opt-out when the tail becomes visible again or distinguish drags away from drags back to the tail.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c3696b811

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Document the public settle members

The new InitialThreadTailSettle helper exposes public members (isComplete and schedule) without member-level documentation. The repo requires doc comments for new public API, so either make this helper/members private or add docs that explain when callers should schedule and how completion is determined.

AGENTS.md reference: AGENTS.md:L113-L116

Useful? React with 👍 / 👎.

[brow]

🤖 Correction to our review above, which was pinned at 290f4b6a5fcaddc91eae76daa7e466476e14b64b. One nit we filed there does not hold up, and we would rather retract it than leave it standing. Comment only, and our recommendation is unchanged.

Retracting the wasAtTail nit. We said the tightening of wasAtTail from "positions empty, or any position at or past the previous last index" to an exact-index match "changes when an already-open thread follows a remote reply" and asked for a test or a description sentence. The first half of that is wrong. The two forms are behaviorally identical in every state we could reach, so it is a simplification rather than a behavior change, and it does not need a test.

The reason is that the effect runs synchronously during build rather than after layout, so at predicate time the reported item positions still describe the previous frame, which had exactly the previous reply count. The maximum present index therefore cannot exceed the previous last index, which collapses "at or past" into "equal to". Your retained comment in that code states this, and we should have taken it at its word instead of scoring it as an unpinned change.

How we checked, rather than just reasoning about it: we instrumented the live predicate to compute both forms at every evaluation and log any divergence, then drove 13 constructed states plus the full suite. 23 evaluations, 0 divergences. States included scrolling away then receiving a two-reply batch (so the index skips by more than one), zero-reply hydration then a first reply, shrink-then-grow, a covered/never-laid-out route, three tiny viewports, and replies arriving at 1ms granularity across 14 consecutive frames during an active scroll animation. We also wrote the candidate test: it is green at head, green under the reverted form (so it does not discriminate), and red under a vacuity mutant that forces the predicate true, which tells us the test does exercise the follow decision and simply has nothing to catch. We chose not to ship a test that only looks like coverage.

Bound on that negative, stated plainly: we did not prove the dropped empty-positions short-circuit is dead code. We proved we could not reach it in 13 constructed states. In our probes an opaque covering route stopped rebuilding the page altogether, which closes that route rather than showing the case impossible.

So the honest version of this item is: a no-op simplification worth at most one sentence in the description. It is strictly less serious than we filed it, and the lead nit from our review is unaffected, which is that the generation guard, the PR's central race protection, stays green when it is removed.

Also noting the head has moved to 3c3696b811a8b3588dd66439824ff140c62098ba since we reviewed. Read-level only, not re-measured: the composer-dock geometry is now handled by a hidden-bottom-fraction parameter subtracted from the visibility test, alignment went from 0.8 to 0.0, and a new opt-out latch gates the follow paths once the reader scrolls away. That appears to address both of the scroll defects raised in review, including the one piece of geometry our review identified as new in this PR. We have not re-run the suite at that head, so treat it as an observation and not a verdict.