Skip to content

fix(agents-mobile): eliminate chat timeline flashing and fix auto-scroll#4601

Merged
msfstef merged 3 commits into
mainfrom
msfstef/optimistic-messsage-queuing-mobile
Jun 17, 2026
Merged

fix(agents-mobile): eliminate chat timeline flashing and fix auto-scroll#4601
msfstef merged 3 commits into
mainfrom
msfstef/optimistic-messsage-queuing-mobile

Conversation

@msfstef

@msfstef msfstef commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Intent

Eliminate the visible "flashing" of the agents-mobile chat timeline during normal use — sending a message, growing the composer (multiline / attachments), and queuing messages — while preserving the dynamic bottom inset, instant optimistic sending, and reliable pin-to-bottom. Also fix inconsistent auto-scroll when the composer grows.

Architecture context (read this first)

The mobile chat screen is two separate UIs:

  • A native composer + queue drawer (agents-mobile/src/screens/SessionScreen.tsx).
  • A chat timeline rendered inside an Expo DOM ('use dom') WebView embed (agents-server-ui/src/embed/SessionChatLogDomEmbed.tsxEmbedApp.tsxChatView.tsx's ChatLogViewEntityTimeline.tsx), hosted by agents-mobile/app/session.tsx.

These run in separate JS contexts. The native side and the embed communicate via props (serialized across the Expo DOM bridge) and an imperative handle.

Root cause (the key finding)

A value delivered over the Expo DOM prop bridge forces a full, memoization-busting re-render of the embed's React tree every time it changes — and with a tree as heavy as the timeline, that re-render is the visible "flash."

Confirmed against the Expo SDK 54 source (expo/src/dom/webview-wrapper.tsx + dom-entry.tsx) — it is not a native WebView reload or remount:

  • The WebView source is derived from the component's file path only, never from props, so a prop change never reloads the page; there's no key change either, so no remount.
  • Prop updates are sent as a message: the native wrapper injectJavaScripts a $$props payload and the page-side runtime handles it with a plain setProps(...) state update on a root created once. So a prop update is mechanically just a setState inside the WebView.
  • But the payload is JSON — re-serialized and re-deserialized — so every data prop arrives as a brand-new reference on every update. The page renders <App {...freshlyDeserializedProps} {...actions} />, busting every downstream useMemo / React.memo / useCallback that depends on a prop. Nothing memo-skips, so the whole heavy subtree re-does its work (virtualized-list measurement, markdown rendering, CSS masks) → a dropped frame = the flash.
  • The native wrapper only emits $$props when its marshalled props change identity. So a parent re-render that passes new inline references (style={[...]}, dom={...}, inline callbacks) ships a fresh-reference update and flashes too — which is why the earlier "froze the CSS / native no-op" tests still flashed until the embed stopped re-rendering. It was never a true no-op; the references were changing.

You cannot make a bridge-crossed prop update cheap (the fresh references are unavoidable). The only lever is to not send frequently-changing values as props at all — keep props referentially stable so the native side emits nothing, and push dynamic updates through the imperative handle (which never crosses the bridge).

So every flash traced back to something that emitted a $$props update (a changed/fresh prop reference) or re-rendered the embed:

  1. Drawer flash on send — the optimistic message was tracked via entity.status === 'running' and pruned from inline tracking while still pending, so it briefly fell into the queue drawer.
  2. Composer-resize flash — the composer height re-renders the host route on every multiline/attachment change, re-rendering the (non-memoized) embed.
  3. Inset prop flash — the bottom inset was passed as a live prop that changed on every resize.
  4. Queued-send flashscrollToBottomSignal and inlineQueuedMessages were live props that changed on send.
  5. Auto-scroll bug — the pin-to-bottom ResizeObserver watched the content-box, but the inset is applied as bottom padding; a padding-only change leaves the content-box untouched, so multiline growth didn't trigger a re-pin (it only scrolled when content also changed — hence intermittent).

Approach

Nothing that changes frequently crosses the bridge as a prop.

  • Memoize both DOM embeds and keep every native-passed prop reference-stable (memoized style/dom, value-memoized serverHeaders, stable callbacks, frozen initial inset).
  • Deliver dynamic values imperatively via a useDOMImperativeHandle ref (SessionChatLogDomRef):
    • setBottomInset(px) → direct CSS-var write (--mobile-chat-bottom-inset), no React.
    • scrollToBottom() → direct scrollTop, no React.
    • setInlineQueuedMessages(messages) → updates the embed's internal useState. The render still cascades, but every surrounding reference stays stable, so memoized subtrees (timeline, markdown, router) are skipped — cheap, no flash — unlike a bridge prop update where all references are fresh.
  • The imperative handle registers a beat after the WebView boots, so the native side pushes via a small usePushToEmbed hook that retries on animation frames until the handle exists.
  • Fix the drawer flash by keying off generationActive (matching desktop) and keeping a message in inline tracking until it actually leaves the pending inbox.
  • Fix auto-scroll by observing the border-box in the pin-to-bottom ResizeObserver, plus a synchronous scrollTop correction before the rAF re-pin (ResizeObserver fires before paint, avoiding one misaligned frame).

Key decisions & trade-offs

  • Internal state for inline queued messages re-renders the embed root. That's fine only because onNavigatePathname is useCallback'd — the embed's router is useMemo'd on it, and an unstable identity would remount (and flash) the timeline. Props are unchanged on an internal re-render, so the callback keeps its identity.
  • SessionChatLogDomRef lives in a plain .ts module, not the 'use dom' embed file: named exports from 'use dom' files aren't resolvable by the consuming app (single-default-export only).
  • expo/dom ambient shim (agents-server-ui/src/types/expo-dom.d.ts) + an inline DOMImperativeFactory index signature in sessionChatLogDomRef.ts: agents-server-ui doesn't depend on expo (the embed is only bundled by the mobile app), so expo/dom isn't resolvable there. This is deliberate — a previous attempt to import/extends from expo/dom failed mobile tsc with "Cannot find module 'expo/dom'". Do not "simplify" this back to importing from expo/dom.

Deliberately NOT done (so we don't go in circles)

  • Do not pass the inset/queued messages as live props "since the embed is memoized." React.memo re-renders when a prop value changes — live props reintroduce the flash. The frozen-initial + imperative-update split is intentional.
  • Do not freeze serverHeaders with useMemo(..., []). It must update if auth headers refresh (token rotation); hence the JSON.stringify memo key (stable identity, updates on real change). The stringify is on a tiny object — negligible.
  • Did not unify the three imperative methods into one generic updateState channel or replace the rAF retry with a "ready" event — Expo exposes no handle-ready signal, and three distinct semantic methods read clearer. Duplication was removed via the shared usePushToEmbed hook instead.
  • Did not push the ResizeObserver behavior behind a prop / create a ChatLogViewMobile wrapper. ChatLogView is already the mobile-embed-only view, and border-box + sync-scroll are harmless/general for desktop.
  • Did not extract the inline-pending projection shared by ChatLogView and SessionScreen — it's pre-existing, cross-package, and array-vs-map; extracting would over-abstract.

Files changed

  • agents-mobile/app/session.tsx — memoize embeds; stabilize props; usePushToEmbed hook + imperative inset/queued-message delivery; imperative scrollToBottom on send; async openSession.
  • agents-mobile/src/screens/SessionScreen.tsxgenerationActive gating; inline-tracking prune guard (keep while still pending).
  • agents-server-ui/src/embed/SessionChatLogDomEmbed.tsx — imperative handle (setBottomInset/scrollToBottom/setInlineQueuedMessages); internal state for queued messages; stable onNavigatePathname.
  • agents-server-ui/src/embed/sessionChatLogDomRef.ts (new) — shared imperative-handle contract + rationale.
  • agents-server-ui/src/types/expo-dom.d.ts (new) — ambient expo/dom shim for agents-server-ui's tsc.
  • agents-server-ui/src/embed/EmbedApp.tsx — inset var seeded on document root; removed dead scrollToBottomSignal plumbing.
  • agents-server-ui/src/components/views/ChatView.tsxChatLogView uses generationActive/entityStopped; dropped scrollToBottomSignal from cacheKey and props.
  • agents-server-ui/src/components/EntityTimeline.tsx — pin-to-bottom observes border-box + synchronous scroll correction.

Testing / how to verify

Manually verified on device by the author: no flash on send, multiline growth, attachments, queue add, or first message to an idle agent; auto-scroll follows composer growth; inset and instant optimistic send still work.

Important: webview-side files (SessionChatLogDomEmbed, sessionChatLogDomRef, EmbedApp, ChatView, EntityTimeline) require a DOM-bundle rebuild to take effect; native files (session.tsx, SessionScreen.tsx) hot-reload.

Both agents-mobile and agents-server-ui typecheck and lint clean.

Rebase notes

Rebased onto main after the "fork subtree" mobile feature merged (it touched the same files). Integration points to be aware of:

  • session.tsx now passes onRequestForkEntity to the embed. It is wrapped in a useCallback (handleForkEntity) rather than the inline closure main used — an inline function would be an unstable prop and defeat the embed's memo (reintroducing the flash). Keep it stable.
  • EmbedApp.tsx merged cleanly: main's ToastProvider + forkEntityImpl wiring coexists with this PR's removed scrollToBottomSignal/inline-inset-style and dropped CSSProperties import.
  • SessionScreen.tsx auto-merged: main's fork state/menu wiring is independent of this PR's generationActive gating + inline-prune guard.

Status / follow-ups

  • Draft. No automated test covers the flash behavior (it's a WebView-layer visual artifact); verification is manual.
  • The simplification pass (reuse/efficiency/altitude/comment review) is already applied — the diff is intended to be the final, consolidated form, not a stack of patches.

🤖 Generated with Claude Code

@msfstef msfstef added the claude label Jun 16, 2026
@netlify

netlify Bot commented Jun 16, 2026

Copy link
Copy Markdown

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit e528ac9
🔍 Latest deploy log https://app.netlify.com/projects/electric-next/deploys/6a317fdd50bc390009030410
😎 Deploy Preview https://deploy-preview-4601--electric-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@msfstef
msfstef force-pushed the msfstef/optimistic-messsage-queuing-mobile branch from 4e060c3 to e528ac9 Compare June 16, 2026 16:54
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Electric Agents Desktop Builds

Build artifacts for commit da9fbb8.

Platform Status Artifact
macOS Apple Silicon Passed DMG
macOS Intel Passed DMG
Windows x64 Passed Installer
Linux x64 Passed AppImage / deb

Workflow run

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.98%. Comparing base (d2418d6) to head (da9fbb8).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...nts-server-ui/src/embed/SessionChatLogDomEmbed.tsx 0.00% 25 Missing ⚠️
...agents-server-ui/src/components/views/ChatView.tsx 0.00% 3 Missing ⚠️
...agents-server-ui/src/components/EntityTimeline.tsx 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4601      +/-   ##
==========================================
+ Coverage   57.43%   58.98%   +1.55%     
==========================================
  Files         342      385      +43     
  Lines       39928    42661    +2733     
  Branches    11597    12215     +618     
==========================================
+ Hits        22931    25162    +2231     
- Misses      16960    17424     +464     
- Partials       37       75      +38     
Flag Coverage Δ
packages/agents 72.79% <ø> (ø)
packages/agents-mcp 77.70% <ø> (?)
packages/agents-mobile 80.67% <ø> (ø)
packages/agents-runtime 82.68% <ø> (ø)
packages/agents-server 75.29% <ø> (-0.21%) ⬇️
packages/agents-server-ui 7.54% <0.00%> (+0.03%) ⬆️
packages/electric-ax 47.62% <ø> (+1.19%) ⬆️
packages/experimental 87.73% <ø> (?)
packages/react-hooks 86.48% <ø> (?)
packages/start 82.83% <ø> (?)
packages/typescript-client 91.83% <ø> (?)
packages/y-electric 56.05% <ø> (?)
typescript 58.98% <0.00%> (+1.55%) ⬆️
unit-tests 58.98% <0.00%> (+1.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Electric Agents Mobile Build

Local mobile checks ran for commit da9fbb8.

The EAS Android preview build was skipped because the mobile-eas-build label is not present.
Add the mobile-eas-build label to this PR to produce an installable preview build.

Workflow run

@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown

Claude Code Review

Summary

This PR eliminates WebView "flash" in the agents-mobile chat timeline by moving every frequently-changing value off the Expo DOM prop bridge and onto an imperative handle (setBottomInset / scrollToBottom / setInlineQueuedMessages), memoizing the embeds, stabilizing remaining props, and fixing composer-growth auto-scroll by observing the border-box. Iteration 4 adds one small, correct composer-clear fix in the same imperative spirit.

Iteration 4 note

Since iteration 3 the only change is commit da9fbb8cb ("clear composer input imperatively on send"), touching packages/agents-mobile/src/screens/SessionScreen.tsx and the changeset. This is a real bug fix, and the diagnosis is correct.

The composer TextInput is controlled via childrenrenderComposerHighlights(value, slashCommands, …) at SessionScreen.tsx:884 — not the value prop (which is why the existing onContentSizeChange comment at lines 876-880 references RN issue 13732). React Native gates text-child reconciliation on a native event count and rejects JS-side updates that are stale relative to in-flight keystrokes, so a setValue('') issued on send while typing fast could be dropped, leaving the sent text in the box. The fix is the minimal correct one:

  • Adds inputRef = useRef<TextInput>(null) (line 540) and wires ref={inputRef} onto the TextInput (line 865).
  • Calls inputRef.current?.clear() immediately after each setValue('') reset in send() — both the edit path (lines 682-683) and the normal send path (lines 702-703). clear() is RN's imperative command that bypasses the event-count gate.
  • The optional-chain (?.) correctly tolerates a not-yet-mounted ref.
  • Changeset updated to mention the composer-clear fix.

Both setValue('') sites are covered; the only other setValue calls (startEditing line 733, insertSlashCommand line 650) set non-empty text and are out of scope (RN exposes no imperative setText, only clear()), so they're a pre-existing limitation, not a regression. Imports (useRef, TextInput) are already present. Nothing else changed; the prior architecture is untouched.

What's Working Well

  • Consistent root-cause discipline. This is the same "the prop path is unreliable here, drive it imperatively" reasoning that motivates the whole PR, applied to one more native-bridge quirk. The inline comment captures exactly why setValue('') alone is insufficient, which will save the next reader a debugging session.
  • All prior strengths still hold: the imperative-handle migration, border-box auto-scroll fix, boot-window crash guard (iteration 3), callback-identity stabilization, and no dead code.

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

None.

Suggestions (Nice to Have)

  • Minor consistency thought (no action needed): startEditing/insertSlashCommand set non-empty values through the same children-controlled path and could in principle hit the same event-count staleness, but RN offers no imperative setter to mirror clear(), and these aren't triggered mid-fast-typing the way send is. Worth keeping in mind only if a "populated text didn't appear" report ever surfaces.
  • The value: unknown push-helper note and the two carried-over suggestions from iterations 2-3 were all addressed in the author's reply (deliberate/accepted, with the entityStopped drawer alignment left as an optional follow-up). No action needed.

Issue Conformance

No linked issue (per linked_issues.json) — a soft warning per project convention, well compensated by an exceptionally detailed PR description. The iteration-4 commit matches the stated intent and updates the changeset accordingly. No scope creep. PR is no longer marked Draft; codecov shows 0% patch coverage, expected for a WebView-layer visual/native-bridge artifact that isn't unit-testable.

Previous Review Status

Iterations 1-3 found no Critical or Important issues; all optional suggestions were acknowledged by the author. Iteration 4's single commit is a correct, well-documented composer-clear fix that introduces no new issues.


Review iteration: 4 | 2026-06-17

The chat timeline is an Expo DOM (WebView) embed that re-posts — and
visibly flashes — on any prop change or re-render. Sending a message,
resizing the composer (multiline/attachments) and queuing messages all
mutated props on the embed and flashed it; the optimistic message also
briefly surfaced in the queue drawer.

- Memoize the embed and keep all native-passed props reference-stable.
- Deliver dynamic values (bottom inset, inline queued messages, scroll)
  imperatively via a useDOMImperativeHandle ref instead of as props, so
  they update the inner React tree without re-posting across the bridge.
- Fix the drawer flash by keying off generationActive and not dropping a
  still-pending optimistic message from inline tracking.
- Pin-to-bottom now observes the content border-box so a padding-only
  inset growth (multiline) reliably scrolls the timeline.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@msfstef
msfstef force-pushed the msfstef/optimistic-messsage-queuing-mobile branch from e528ac9 to 769298c Compare June 16, 2026 17:01
@msfstef

msfstef commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Responding to the three optional suggestions — no code changes this round, with reasoning below (happy to act on #1 if we want to close the edge):

1. Asymmetric projectedPendingMessage gating (entityStopped). Confirmed the divergence is real but narrow. The embed ChatLogView mirrors desktop GenericChatBody, which suppresses the inline pending bubble when entityStopped; the native SessionScreen.projectedPendingMessage only drives drawer de-duplication and gates on generationActive only. The only way to hit the gap: queue a message to an idle agent, then have it transition to stopped within the 15s inline-tracking window before the message syncs. In that window the embed won't render it inline (stopped) and the native drawer still hides it (via inlineQueuedKeysRef), so it's briefly invisible — then self-corrects when the 15s timeout releases tracking (it reappears in the drawer) or the message processes. Note a one-line gate change isn't a full fix: even with projectedPendingMessage nulled, inlineQueuedKeysRef still filters the key out of the drawer, so closing this properly means surfacing tracked-but-stopped messages in the drawer (matching desktop's suppress-inline-show-in-drawer behavior). Given it's a transient stop-edge (not a flash), I've left it as-is; can do the alignment as a follow-up if we want it closed.

2. scrollToBottom() single rAF racing the inline render. Agreed and intentional. The single rAF pins to the current scrollHeight as a best-effort first pin; the optimistic row arrives on a separate inner-render schedule, and the border-box pin-to-bottom ResizeObserver re-pins once the row lays out. The observer is deliberately the load-bearing mechanism, so the rAF racing the row layout is harmless.

3. SessionChatLogDomRef index signature. Accepted trade-off as documented — it's required to satisfy expo's DOMImperativeFactory without importing expo/dom from agents-server-ui (which can't resolve it). The three named methods below the index signature keep the real surface honest at the definition site; call sites lose arity checking, which we accept for this three-method handle.

@msfstef
msfstef marked this pull request as ready for review June 17, 2026 10:21
@msfstef

msfstef commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2026-06-17.at.13.33.40.mov

msfstef and others added 2 commits June 17, 2026 13:34
The chat-log embed's imperative handle registers a beat after the WebView
boots: `ref.current` becomes a truthy proxy before `useDOMImperativeHandle`
marshals its methods in. The `usePushToEmbed` guard only checked `ref.current`,
so a push during that window threw `setBottomInset is not a function`.

Check the specific method (`typeof fn === 'function'`) and retry on animation
frames until it registers; guard `scrollToBottom` with `?.()` for the same
reason.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The composer TextInput is controlled via children (for slash-command
highlighting), not the `value` prop. RN gates text reconciliation on an event
count and rejects JS updates that are stale relative to native keystrokes, so a
`setValue('')` on send while typing fast was dropped — the sent text lingered
in the box.

Clear the field via a ref (`inputRef.current?.clear()`), an imperative command
that bypasses the event-count gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@msfstef
msfstef merged commit aa6ca85 into main Jun 17, 2026
71 checks passed
@msfstef
msfstef deleted the msfstef/optimistic-messsage-queuing-mobile branch June 17, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants