Repository navigation
fix(mobile): a finished thread leaves the Working section after its grace - #877
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
This review includes 4 billable files and costs up to $1.00.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 8 minutes for your next included review. View limit details
|
Jacksondr5
left a comment
There was a problem hiding this comment.
Combined panel review (reviewer-opus + reviewer-sol) at ebc3853
Verdict: no correctness findings, and the panel approves the source. One evidence gate stays open (below).
- Cause confirmed. The React Compiler memoizes on the values a computation reads, so a
useMemothat listsinboxVersionwithout reading it lost that dependency in the release build. When the 2 s exit grace expired, Chats re-rendered with the old observation.observeInbox(threads, inboxVersion)makes the version an argument of a module-level call. The compiler keys its cache on that argument and can't see that the body ignores it, so a bumped version recomputes. Dropping theuseMemois safe uncompiled too:observeis what web calls on every render. - Failing test first.
useThreadListV2.test.tsxjoinsreactTests, so the production hook is transformed by the release compiler. reviewer-opus swapped trunk'suseThreadListV2.tsback in: it fails with "expected 1 to be +0". It passes on the branch. - Zone 2, recorded. The upstream edit is the smallest one possible. It's recorded as a FORK.md temporary in-place patch with a watch signal and an exit, has ledger rows for both files and an
upstreamSeams.test.tsreach, and is listed for give-back in #276. Jackson hit this on device, so the patch has the direct impact our rule asks for. - Reviewer-run: the new test and the seams test pass (48), mobile
vp run typecheckexits 0, and lint is clean on both files.
Evidence gate (raised by reviewer-sol)
The PR ticks "UI changes: none", but this changes what the Chats list shows over time: a finished thread now leaves Working once its grace ends. Jackson's rule calls for before/after evidence on every UI change, and a short MP4 for timing. Please have the tester record a before/after clip on Android (trunk: the thread stays under Working; branch: it leaves after the grace), state that iOS waits for Checkpoint 2, and update the checklist to match.
Reviewed by Claude Opus 5.5 (reviewer-opus) and Sol 6.1 (reviewer-sol) in J5 Code.
Jacksondr5
left a comment
There was a problem hiding this comment.
Panel delta check (reviewer-opus + reviewer-sol) at 10c502c
Verdict: no findings, and the panel's source approval stands. The video gate from our first review is unchanged: an Android before/after MP4 is needed, and the "UI changes" line needs fixing.
- Upstream's fix taken unchanged.
useThreadListV2.tsis now blob1a0c67062f, identical to upstream's file atbe58795d37(pingdotgg#28169). That commit touches only this file, so nothing else is missing. The fix is the same approach: anobserveInboxhelper that takes the version as an argument, inside auseMemo. - Record. The FORK.md bullet now records an upstream fix taken ahead of the pin, with the exit "take upstream's at the next advance". The seams-test row is removed, because no J5 code remains in the file. #276 marks the entry as fixed upstream.
- Test. J5's release-compiled
useThreadListV2.test.tsxpasses against upstream's version (both reviewers). With the seams test, 47 pass. Mobilevp run typecheckexits 0.
Reviewed by Claude Opus 5.5 (reviewer-opus) and Sol 6.1 (reviewer-sol) in J5 Code.
…race The Chats list kept a thread under Working after its turn until some thread shell changed. useThreadListV2 memoized the inbox tracker's observation with the tracker's version as an unread useMemo dependency; the React Compiler memoizes on what a computation reads, so the compiled memo dropped the version, and the re-render after the 2 s grace reused the old answer. The version is now an argument to the observation. The new test runs under the release compiler and fails on trunk. Recorded as a temporary in-place patch in FORK.md, with a seams-test row. Closes #842 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Upstream fixed the same bug in pingdotgg#28169 (be58795). The hook now matches upstream's file at that commit byte for byte, so the next advance resolves trivially; the FORK.md entry records it as upstream's fix taken ahead of the pin, and the seams-test row is gone because no J5 code is involved. J5's compiled test stays and passes. Refs #842 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
10c502c to
f4b2a7c
Compare
Problem
In the mobile Chats list, a thread could stay under the Working section after its turn ended. It came back only when some other thread changed, for example after visiting another chat. Jackson hit it on his iPhone at Checkpoint 1, and two of three Android debug runs reproduced it. Closes #842.
Cause
useThreadListV2memoized the inbox tracker's observation like this:inboxVersionwas listed but never read inside the callback. The React Compiler memoizes on the values a computation reads, not on the listed dependencies. Compiling the file with Metro's release settings shows the memo guarded only bythreadsandworkingShelfEnabled. When a finished thread's 2 s exit grace expired, the tracker bumped its version and Chats re-rendered, but it reused the old observation, which still said "working". The thread stayed there until a thread shell changed and invalidated the memo.That's why it was intermittent. In the debug run that didn't reproduce, the thread's title regeneration happened to land after the grace and freed it. The debug logs (#842) showed the tracker firing correctly, with one listener, every time.
What changed
be58795d37, merged 2026-10-31) fixed the same bug the same way: anobserveInbox(threads, inboxVersion)helper that reads the version. This PR applies that hunk unchanged, souseThreadListV2.tsequals upstream's file at that commit (blob1a0c67062f), and the next advance resolves trivially. The give-back agent found it.observeon every render.Tests
useThreadListV2.test.tsxruns under the release React Compiler (it's added toreactTestsin the mobilevite.config.ts). It renders the hook with a working thread, then the finished thread, then advances only the grace timer. On trunk it fails ("expected 1 to be 0": the thread is still in Working). With the fix it passes. It also passes against upstream's version of the hook. The existing thread-list tests pass.UI changes
This changes timing in the Chats list. A thread that finishes its turn now leaves the Working section after the 2 s grace, instead of waiting for another thread to change.
Android, recorded by a crew tester on the same test home: a follow-up message ("Say hello again.") sent from web to an existing Haiku thread, with Chats left untouched. A follow-up was used because a new thread's title regeneration can free the thread by accident.
Before (
j5/main): the finished thread stays under Working.android-working-before.mp4
After (this branch): Working empties within seconds of the turn ending.
android-working-after.mp4
On the before run, Working was collapsed, then expanded at 53 s to show the thread. The after run kept the expanded preference. iOS waits for Checkpoint 2.
Live testing
Android: PASS at 10c502c (wave-1a tester, report
test-reports/j5code-mobile-wave1a-tester/10c502c88b.md). It used the same existing Haiku thread and the same prompt on both heads, with no new-thread title creation, and the focused tests pass. The compiled test is the fix's proof; the videos are the live check.Upstream impact
apps/mobile/src/features/threads/useThreadListV2.ts: upstream's fix from fix(mobile): finished threads leave Working once the grace ends pingdotgg/t3code#28169, unchanged (FORK.md temporary patch).apps/mobile/vite.config.ts: onereactTestsentry, so the test runs under the compiler.Upstream's product behaviour doesn't change; the shelf now works as upstream designed it.
Checklist
FORK.md(temporary patch and file-table rows) in this PRBuilt by Claude Opus 5.5 in Claude Code (J5 Code crew seat builder-composer).
🤖 Generated with Claude Code