Repository navigation
fix(mobile): a streaming reply can't pull a drag back to the bottom - #23832
brennanb2025 wants to merge 4 commits into
Conversation
Track the user's scroll gesture as phases (dragging, released, momentum, idle) and honor each native scroll event only in the phase that expects it. iOS emits a momentum-end after every non-animated tail pin, even while a finger is down; it used to end the drag and re-arm tail-follow, so the next pin yanked the user back to the bottom. Gesture-settling tests move beside the hook they exercise.
 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe mobile chat tail-follow hook now tracks scroll activity through idle, dragging, released, and momentum phases. It accepts drag release only while dragging, starts momentum only after release, and finishes a scroll only during momentum. Tests cover programmatic momentum events, release metrics, settling without momentum, and content growth before settling. Three related tests were removed from the view test file. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The change stops streaming replies from pulling an active drag back to the bottom, and the hook tests cover the new behavior. No merge-blocking issue was found. Android and physical-device behavior were not verified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is confined to on-device chat scrolling. No new access to sensitive data, privileges, or services was identified. Some native event ordering remains unverified on devices. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The new use-mobile-native-chat-tail-follow.test.ts fails tsc -p tsconfig.test.json, so the check:tests-typecheck ratchet (mobile.yml:127) is red and this can't merge. The PR body's "typecheck passes" is tsc --noEmit (mobile/tsconfig.json), which excludes tests. Fixes are inline.
Reviewed changes
Reviewed the full a034cc0 diff (3 files) — a mobile-only change to how the native chat transcript decides tail-follow.
- Gestures tracked as phases —
use-mobile-native-chat-tail-follow.tsreplaces theuserScrollActiveRefboolean with anidle/dragging/released/momentumstate; each native scroll event is honored only in the phase that expects it. - The actual fix —
endUserDragrequiresdragging,beginMomentumrequiresreleased,endMomentumrequiresmomentum. An iOS momentum-end arriving mid-drag (emitted after every instant programmatic pin) is now ignored instead of re-enablingfollowingand letting the next pin yank the list down. - Settle source is explicit — release with no momentum settles from the drag-end position on the next frame; release with momentum settles from the momentum-end position.
finishUserScrolldrops its old "active" guard because every caller is phase-guarded.
I verified the phase tests are real coverage: swapping the hook back to origin/main's implementation makes the three programmatic-momentum / release-position tests fail; they pass on head. The three settling tests also moved from MobileNativeChatView.test.ts intact.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Review status: ready for your review (not merged)The problem. In the phone app's native chat, while an agent reply is streaming, the chat keeps itself pinned to the bottom with an instant scroll about every 60 ms. iOS reports "scroll momentum ended" after every one of those instant scrolls, even while your finger is still on the screen. The old code read that as "your drag finished at the bottom", turned following back on a few milliseconds into the drag, and the next pin pulled you back down. Scrolling up to read earlier text while a reply streamed often failed. What changes for the user. Dragging up while a reply streams now stays where your finger puts it, and the jump-to-latest button appears. Letting go near the bottom still resumes following, and tapping jump-to-latest still resumes following. Nothing changes when no reply is streaming. Review. One review round was run: two independent reviewers covered the event handling on iOS and Android, and the tests, scope and design. The final diff then got a release-readiness pass.
Live QA (iOS simulator). A test screen streamed a growing reply every 60 ms, and real finger drags were sent at 16 ms per touch point. Each drag was judged from logged hook state and scroll offsets, not by eye.
The same PR build also passed four behaviour checks while streaming:
Screenshots are in the PR description. Seen during QA; already on main, not caused by this PR. If the transcript gets shorter while you're scrolled away from the bottom, the list lands at the new bottom but following stays off. As new text streams in below, the jump-to-latest button stays hidden until you scroll. This showed up because the test screen resets its fake reply. I did not check whether a real chat ever shrinks mid-stream. Not verified: Android on a device or emulator, a physical iPhone, a production build, or a real paired desktop host. Typecheck and the full mobile suite were not re-run in this review; the two affected test files pass (35 of 35). |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review (a034cc0 → 251bb5a): a test-typecheck fix plus a merge of origin/main. These commits address both inline comments from that review; the hook's phase logic is unchanged.
actcallbacks given braced bodies — the closures passed toactno longer returnVitestUtils, satisfying thevoidcallback type at the three flagged sites inuse-mobile-native-chat-tail-follow.test.ts.FlatListresolved viaroot.find— a smallhostNamedhelper replacesfindByType('FlatList'), clearing theElementTypemismatch against the host-string name.
Verified this session: pnpm --dir mobile run check:tests-typecheck exits 0, and both changed test files pass (23 tests). The merge brought no net change to the PR's three files.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
CI fix after the review (no behavior change).
|
Scrolling up in the transcript — releasing toward the bottom, which is where a reader usually is — was pinned back to the tail mid-gesture, so the fling appeared to stop dead. Scrolling the other way drifted away from the tail, which is why only one direction looked broken at first. iOS emits a bare momentum-end after every non-animated pin and Android a begin/end pair after an animated one, so an event from a programmatic scroll could be read as the end of the user's gesture. The gesture is now tracked in phases — idle, dragging, released, momentum — and each native event is honored only in the phase that expects it, so a stray event has no effect by construction. Cherry-picked from stablyai#23832 (brennanb2025), which is open and mergeable against upstream main; the commits follow when it lands.
This branch has not been deployed
1 participant
ELI5
On iPhone, while an agent is typing a reply, the chat keeps nudging itself to the bottom so you see the newest words. If you tried to scroll up to read something earlier, the chat often yanked you right back down. Now, once your finger is on the list, the chat leaves you alone until you let go, and only goes back to following the reply if you let go at the bottom.
What Changed
Before: the chat's scroll owner tracked the user's gesture with a single "user is scrolling" flag. While a reply streams, it pins the list to the bottom with an instant (non-animated) scroll about every 60 ms. iOS reports a "momentum scroll ended" event after every instant programmatic scroll, even while a finger is still down and even though no "momentum scroll began" came first. The chat treated that event as "the user's gesture finished", decided the user was back at the bottom, and turned following back on a few milliseconds into the drag. The next pin then pulled the user back down. On the simulator, 5 to 6 of 10 upward drags were yanked while a reply was streaming, and 0 of 10 when nothing was streaming.
After: the gesture is tracked as phases: dragging, released, momentum, idle. Each native scroll event is honored only in the phase that expects it:
User effect: dragging up while a reply streams stays where the finger puts it. Letting go at the bottom still resumes following; letting go higher up shows the jump-to-latest button, as before.
The gesture-settling tests moved out of the chat view's test file into a new test file next to the hook, because the view's test file had reached the line limit.
Why
Why phases: the bug is that one flag cannot tell "the finger is down" from "the finger lifted and the list is coasting", so any end event looked like the end of the user's gesture. With phases, a stray event arriving in the wrong phase has no effect by construction, instead of needing a special-case filter.
Why each event only in its phase: the platforms send these events for more than user gestures. On iOS, momentum begin only comes from real deceleration after a finger lift, but momentum end also follows every instant programmatic scroll. On Android, a finger lift (even with zero velocity) normally sends momentum begin and later momentum end, and an animated programmatic scroll sends a begin/end pair too. Accepting momentum begin only right after a release, and momentum end only after that begin, keeps real gestures working on both platforms while ignoring the programmatic ones. Jump-to-latest is instant today; the same rule also covers it if it becomes animated.
Why drag-end vs momentum-end positions: each is the event that describes where the gesture actually ended. Re-reading the position when the frame timer fires would be wrong: if a chunk of the reply lands in that window, the content grows under a finger that was released at the bottom, and the release would read as leaving the bottom.
Alternatives rejected:
Known weaknesses
Linked Issue
None public.
Visual Proof
iOS simulator, live-streaming reply, real 16 ms touch drags (details in the review comment).
With this PR, after dragging up mid-stream: the position holds and the jump-to-latest button shows.
Without this PR (current main), one second after the same upward drag: pulled back to the bottom, no jump button.
With this PR, a small drag released near the bottom resumes following.
Testing
New tests, written against the event sequences iOS and Android send: a programmatic momentum-end (alone, and as a begin/end pair) while the finger is down does not resume following; a released drag settles from the release position even when a programmatic momentum-end arrives before the frame. All three fail on the previous code and pass with this change.
Existing gesture tests (momentum-end position beats a stale scroll sample, release position used when no momentum follows, repin when content grows between release and settle) moved unchanged in meaning to the hook's test file and pass; the chat view keeps its drag-to-momentum wiring test.
From mobile/: full vitest suite passes (889 files), typecheck passes; changed-code quality gate passes.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Mobile-only change to how the chat list reacts to scroll events; no desktop, SSH, wire, or storage impact.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)