Repository navigation
fix(mobile-native-chat): reland glued pending retirement with resolvable baselines - #14928
brennanb2025 wants to merge 1 commit into
Conversation
…ble baselines Relands #14665 (reverted by #14819). Mobile retires an optimistic bubble only when a transcript user turn matches its normalized text at the expected ordinal, so two rapid sends the host glued into ONE row matched nothing and both bubbles pinned below every later reply for the rest of the session. #14819 reverted it for two launch blockers, both fixed here rather than restored: 1. A rejected send restored a TRIMMED composer. The trim belongs to the wire — trailing whitespace the host writes verbatim can glue the next rapid send — not to the draft. The send seam now keeps `draftText` as the user typed it and trims only the bytes that go out, so a rejection puts back the original draft, blank lines included. 2. Sends issued while the transcript was still hydrating were stamped `glueBaselineTrusted: false` FOREVER, so their glued row could never retire them and they barred their neighbours from matching too (STA-4492). A hydrating transcript is not this session's history, so the flag is now `baselineResolved` and it is transient: such a send is held out of matching until the first authoritative read, which rebases it onto that read's tail and re-derives its ordinal. It then reconciles like any other send — and, as a consequence, also stops being retired by history it never saw. Fixes STA-4482, STA-4492 Refs #14665, #14819, #14262
Mobile emulator QA: BLOCKEDAttempted per What worked:
What blocked it:
Someone on a Mac host can run this in a few minutes: start Metro on this branch, |
📝 WalkthroughWalkthroughThe changes add transcript-baseline rebasing and centralized retirement for mobile native-chat pending messages. The logic handles exact echoes, glued sends, image echoes, occurrence counts, barriers, and hydration state. Pending send origins now record baseline resolution. The send flow right-trims transmitted text while preserving the original draft for clearing and rejection restoration. Tests cover rebasing, retirement, hydration, glued messages, image previews, whitespace handling, and rejected sends. Merge Risk: ⚪ Minimal · up to This PR restores correct retirement of glued pending messages, preserves rejected draft text, and defers hydration-time matching until baselines resolve. The change is merge-ready after normal checks; no actionable merge-blocking risk remains. 🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mobile/src/session/use-mobile-native-chat-message-send.test.ts (1)
39-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the send-origin fixture type-safe.
as neverhides missing fields inMobileNativeChatSendOrigin. The test can compile after the production origin contract grows, even if this mock remains incomplete. Return a complete typed origin fixture instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 202ab1f6-7ac9-4d25-8cb3-1aa9ded529a4
📒 Files selected for processing (10)
mobile/src/session/mobile-native-chat-pending-baseline.test.tsmobile/src/session/mobile-native-chat-pending-baseline.tsmobile/src/session/mobile-native-chat-pending-echo.tsmobile/src/session/mobile-native-chat-pending-retirement.test.tsmobile/src/session/mobile-native-chat-pending-retirement.tsmobile/src/session/use-mobile-native-chat-controller.test.tsmobile/src/session/use-mobile-native-chat-drafts-glued-pending.test.tsmobile/src/session/use-mobile-native-chat-drafts.tsmobile/src/session/use-mobile-native-chat-message-send.test.tsmobile/src/session/use-mobile-native-chat-message-send.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 4 remain after this review.
|
Superseded by #14936, which relands #14665 addressing both documented revert causes rather than one. The reason this matters: #14819's own body says only "launch-blocking regression". The actual causes are in a maintainer comment on #14665 —
Cause (2) is STA-4492, which provably does not reproduce on current main because #14819 deleted every file and symbol it cites — it is a constraint on the reland, not a standalone bug. #14936 fixes both and covers them with 2/3/14/6 RED across four mutations, one per revert cause plus the feature and its false-positive bound. Closing this to keep one review surface per ticket. |
Relands #14665, which #14819 reverted. Fixes STA-4482 (the reland) and STA-4492 (the hydration defect the reland was asked to carry a fix for).
STA-4492 does not reproduce on
maintoday, and that is expected:glueBaselineTrustedexists nowhere inmobile/because #14819 deleted it. It returns the moment #14665 relands, which is why it is fixed here rather than filed as won't-fix.What #14665 was for
Mobile retires an optimistic bubble only when a transcript user turn matches its normalized text at the expected ordinal. When the host glues two rapid sends into ONE row, neither text matches, so both bubbles pin below every later reply for the rest of the session.
What #14819 reverted it for, and what changed
1. A rejected send restored a trimmed composer and dropped draft text
#14665 shadowed the send parameter with
const text = rawText.trimEnd()and then handed that trimmed value torestoreRejectedDraft. The trim is a wire concern — trailing whitespace the host writes verbatim onto the agent's input line can glue the next rapid send onto this one — and it has no business touching the draft.The seam now keeps
draftTextas the user typed it and trims only the bytes that go out.clearDraftForSend/restoreRejectedDraftget the original;captureSendOrigin/acceptSend/ the RPC get the trimmed text.2. Loading-time glued sends stayed queued forever (STA-4492)
glueBaselineTrusted: !transcriptLoadingwas stamped once and never revisited. InselectGluedPendingIdsan untrusted entry becomes anullsegment — a permanent barrier — so its glued row could never retire it, and it split the run so its neighbours could not glue either.The flag is replaced by
baselineResolved, which is transient:messagesis not this session's history then (a read is in flight, or it is still the previously active tab's), so the entry is held out of matching entirely rather than judged against somebody else's rows.rebaseMobileNativeChatPendingBaselinespins it to that read's tail and re-derives its ordinal from that transcript. It is a normal send from then on.Consequence beyond the reverted defect: a hydration-time send is also no longer retired by history it never saw. On
maintoday,baselineOccurrencescaptured against an empty/foreign transcript makes a send whose text already appears in history retire the instant that history loads — the mirror of the same bug. There is a test for it.Tests: RED → GREEN, against a naive re-apply of #14665
Every test below was run against the literal #14665 tree (
git checkout 68ca17e46c -- <files>), not againstmain, so it proves the reland is different from the diff that was reverted.Regression 1 —
use-mobile-native-chat-message-send.test.ts:Regression 2 —
use-mobile-native-chat-drafts-glued-pending.test.ts:The other 8 tests in that file pass against #14665 too — the reland keeps its intended behavior. Against
origin/main(no glue handling at all) 4 of the 10 fail, including the core retirement.A separate
codexsession, given only the #14665 sources and no hint of the answer, independently traced the hang to!item.glueBaselineTrustedinselectGluedPendingIdsand concluded "both queued bubbles remain forever".Full run:
451 files / 3565 tests passedinmobile/.tsc -p mobile/tsconfig.jsonandoxlint mobile/src/session/clean (exit 0). Nomax-linesdisable and no.oxlintrc.jsonbump.Not covered
Emulator QA is blocked on this host — see the PR discussion. The change is pure reconciliation logic behind the drafts hook and is covered at the hook level (the tests drive the real
useMobileNativeChatDraftsthroughreact-test-rendererand assert on the bubbles the list actually renders), but it has not been exercised on a device.🤖 Generated with Claude Code