Skip to content

fix: keyboard/composer (#156), auto-scroll (#155), dead SSE (#186), blocked SSE startup (#189), stale transcripts, uncopyable text, axis-perfect table scrolling, 50-session list cap, transcript flash, dark-mode contrast - #182

Open
omgoshjosh wants to merge 11 commits into
dzianisv:mainfrom
omgoshjosh:fix/android-keyboard-inset-offset

Conversation

@omgoshjosh

@omgoshjosh omgoshjosh commented Aug 15, 2026

Copy link
Copy Markdown

Five Android chat-surface defects, each with an isolated pure-helper module and tests. Grouped into one PR rather than five to keep review traffic on this repo low — they were found together while making the app usable on a Pixel 3 XL (Android 12), and three of them close filed issues.

Every policy module under src/lib/ deliberately has no runtime imports, so it runs under plain node --test with no RN harness.


1. Keyboard covers the composer — closes #156

Expo's mandatory edge-to-edge display changed which coordinate space KeyboardAvoidingView reasons in. The composer was offset against window coordinates while the keyboard reports screen coordinates, so on a device with a translucent status bar the composer sat under the keyboard.

keyboardVerticalOffset(platform, insetTop) returns insetTop + 12dp on Android. Verified against real measurements on a Pixel 3 XL, recorded in the file's comments: window 748.857, screen 845.714, insets.top 48.857, keyboard screenY 509.429 / height 288.286 → computed 288.285 against a required 288.286. The 12dp is a deliberate visual gap, not slack in the arithmetic.

2. Transcript does not follow new messages — closes #155

The transcript is an inverted FlatList, so "scroll to newest" is offset 0, not the end. Nothing observed content growth, so nothing scrolled. A signature over (message count, last message id, last part count, last part text length) changes both when a message arrives and when the last one is edited in place, so a streaming reply follows too.

shouldAutoScroll only follows when the view is already within 200px of the newest message, so a user scrolled up reading history is never yanked away mid-sentence. The scroll-to-bottom button shares the same predicate rather than keeping its own copy.

3. Dead SSE streams never recovered — closes #186

Three defects compounded into a green indicator over a dead transport:

  • The read was unbounded. A half-open socket — routine when a phone moves between Wi-Fi and cellular, or wakes from doze — yields no bytes, no done, no error. reader.read() parked forever. readWithTimeout races it against a 35s deadline (three missed ~10s heartbeats) and rejects, putting the failure on the same path as a real transport error so existing reconnect logic needs no special case.
  • connected meant "we tried". It was set when a connect began. It now flips only once bytes arrive; a transport state (idle | connecting | live) distinguishes dialling from live, and only live reads as healthy.
  • Backoff reset on a timer, so a silently-failing connection kept resetting its own backoff.

Also reconnects on AppState active, deduplicated against network-restore so the two don't open duplicate streams. Measured on an emulator with an airplane-mode toggle: ~16s to recover, versus never.

4. Stale transcript after reconnect

SSE resumes from "now" and does not replay. If another client (CLI, TUI, another device) posted while this client's stream was down, the open session's transcript stayed stale indefinitely — this client never saw the busy event, so the existing busy-session resync didn't cover it. reconcileOpenSession() refetches the open session on reconnect.

5. Assistant text could not be selected or copied

React Native #46999: selectable <Text> inside a FlatList row misbehaves on Android. Rather than fight it, SelectableTextModal renders the text outside the list, where selection works normally. Long-press a bubble to open it. Reasoning blocks are extracted separately so they can be copied without the answer text.

6. SSE startup blocked behind metadata fetches — closes #189

loadConnections() awaited project.current() and path.get() before committing the client to the store, and the SSE stream (plus the catalog load) only starts once client appears there. Neither request is needed to stream events, and each is capped at 30s — so a slow or hanging metadata response delayed live events by up to that long. The client now commits immediately and metadata fills in behind, guarded so a stale response can't clobber state after a mid-fetch server switch.


Testing: 322 tests pass, typecheck clean. Keyboard offset and copy extraction validated on physical Pixel 3 XL hardware (Android 12); SSE recovery validated on an emulator via airplane-mode toggle. Wi-Fi↔cellular handover on real hardware is still pending.

…#156)

The composer (text field, attachment buttons, mic) is completely hidden
behind the keyboard on Android. Reproduced on an Android 12 emulator on
current main, i.e. with 6c103ac's behavior="padding" already applied.

The cause is a coordinate-space mismatch rather than the behavior prop.
KeyboardAvoidingView computes

    padding = frame.y + frame.height - (keyboardFrame.screenY - offset)

`frame` comes from its own onLayout, in **window** coordinates (origin
below the status bar). `keyboardFrame.screenY` is in **screen**
coordinates (true top of the display). Before Expo's mandatory
edge-to-edge those origins coincided and the OS also resized the window,
so the difference was invisible. Under edge-to-edge the window spans the
whole display, the two spaces disagree by exactly the status-bar inset,
and the computed padding lands short by that amount.

Measured on an Android 12 emulator (scale 3.5), keyboard open on the
session screen:

    screen height     845.71 dp      window height  748.86 dp
    insets.top         48.86 dp      insets.bottom      48 dp
    keyboard screenY  511.71 dp      height            286 dp

    computed padding = 748.86 - 511.71 = 237.14 dp
    required padding =                   286.00 dp
    shortfall        =                    48.86 dp   === insets.top

48.86dp x 3.5 = 171px, which is exactly the composer row.

This also explains why the problem keeps returning under new issue
numbers: dzianisv#53/dzianisv#70, dzianisv#147/dzianisv#148 and the closed dzianisv#74 each only changed the
`behavior` value, so none of them addressed the mismatch.

Add keyboardVerticalOffset(platform, insetTop): iOS keeps its existing
empirical 90 (it has no such mismatch), Android returns insets.top,
clamped at 0 so a bogus inset can never push content downward. Pure and
unit-tested, including a guard asserting the corrected arithmetic lands
exactly on the real keyboard height.

Verified on device: composer, attachment and mic controls all visible
and usable above the keyboard, in portrait and at 1.5x font scale.
Landscape is not applicable since app.json locks portrait orientation.
omgoshjosh pushed a commit to omgoshjosh/opencode-mobile that referenced this pull request Aug 15, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Joshua Castaneda and others added 3 commits August 15, 2026 13:55
A prompt submitted from another client (CLI, TUI, another device) while
this client's stream was down never appeared in an already-open session
until the user navigated away and back.

resyncBusySessions() was the only reconnect-time reconciliation, and it
only considers sessions this client has marked "busy". A client only
learns a session is busy from an SSE `session.status` event -- so if the
stream was down when the other client prompted, this client never saw
that event, the session is still "idle" in its store, and
resyncBusySessions() finds nothing to do and returns immediately.
Reconnect resumes the stream from "now" without replaying missed events,
so the messages from the gap are never fetched by anything.

Add reconcileOpenSession(), invoked alongside resyncBusySessions() in the
same once-per-reconnect block: refetch the currently-open session's
transcript unconditionally. refreshMessages() replaces messages/parts
without touching isLoading, so this is a silent background reconcile
rather than a spinner over content the user is already reading -- which
only holds because dzianisv#150's fix stopped same-session refreshes from
forcing the loading state.

Distinct from dzianisv#150: that fixed a spinner hiding content that was
arriving. This fixes content that never arrives at all. Same symptom,
different layer.

Server-side contract was verified to already pass (an already-connected
/global/event subscriber does receive another client's prompt
immediately), so this closes the remaining client-side gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Assistant prose had no copy path at all. src/components/markdown/Markdown.tsx
defines a CustomRenderer whose entire purpose is to strip the `selectable`
prop that react-native-marked hardcodes on every plain-text node, because a
selectable <Text> nested in a FlatList row hits react/react-native#46999
on Android. Chat messages are rows of the session screen's inverted FlatList,
so every markdown text node hits it.

That workaround is correct as far as it goes, but its stated justification --
"code content is still copyable via CodeBlock's explicit Copy button, so
dropping `selectable` on plain text costs little" -- undercounts the cost.
User messages are plainly `selectable` and tool output is `selectable`, but
assistant prose, the thing users most want to copy, could not be selected,
copied, or shared by any means.

Rather than re-enabling `selectable` inside the FlatList row (which is what
RN#46999 punishes), add a copy path that reads the source text from the
message parts:

- message-copy-text.ts: pure extractCopyText/extractReasoningText/
  hasCopyableText over a message's parts. Dependency-free, unit-tested.
- SelectableTextModal: renders that text in a `selectable` <Text> inside a
  <Modal>. The modal renders outside the transcript FlatList, so RN#46999
  does not apply and real partial selection works.
- MessageBubble: long-press enabled for both roles (was user-only).
- session/[id].tsx: the action sheet offers "Copy message" and "Select text"
  for either role; "Edit message" stays user-only since reverting to an
  assistant message is unsupported. Returns early when a message has no
  prose so tool-only messages don't open an empty sheet.

Known gap: app/demo.tsx renders MessageBubble without onLongPress, so the
demo conversation is still uncopyable. Left out to keep this reviewable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Under Expo's mandatory edge-to-edge display the modal sheet extends
beneath the system navigation bar, so the fixed paddingBottom:20 left
the hint drawn behind it -- confirmed on an Android 12 emulator.

Pad by the real safe-area bottom inset instead, with a floor so the hint
still has breathing room on devices reporting an inset of 0.

Same class of edge-to-edge inset bug as the composer/keyboard issue
(dzianisv#156).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@omgoshjosh omgoshjosh changed the title fix(ui): stop the keyboard covering the composer on Android (#156) fix(android): keyboard covering composer (#156), stale transcript after reconnect, and uncopyable assistant text Aug 15, 2026
omgoshjosh pushed a commit to omgoshjosh/opencode-mobile that referenced this pull request Aug 15, 2026
Our `main` deliberately carries fork-only commits (the evaluation
distribution workflow), so it is permanently ahead of upstream and can
never fast-forward. That makes it a poor base for upstream contributions:
branching from it drags fork-only commits into the PR, which is exactly
what had to be untangled by hand for dzianisv#182.

Add a mirror branch, force-updated to upstream/main and never committed
to directly, so `git worktree add -b fix/x main-for-opencodex` yields a
branch containing only the fix. Force-push is correct precisely because
nothing is ever authored there — it is a mirror, not a line of
development. No-ops when already in sync.

Added as a separate job rather than changing the existing sync-main
logic, which keeps doing what it did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Joshua Castaneda and others added 2 commits August 15, 2026 21:09
The transcript did not follow new content. Reported in dzianisv#155 as "message
cannot be scrolled automatically": a reply would stream in below the fold
and the user had to drag down to read it, every turn.

The transcript is an inverted FlatList, so "scroll to the newest message"
is scrolling to offset 0, not to the end. Two things were missing:

  - Nothing observed content growth, so nothing ever scrolled. A signature
    over (message count, last message id, last part count, last part text
    length) changes on both a new message and a streaming edit to the last
    one, which is what makes a reply that grows in place follow too.

  - Following unconditionally would fight a user who has scrolled up to
    read history. shouldAutoScroll only follows when the view is already
    within AT_BOTTOM_THRESHOLD_PX of the newest message, so scrolling back
    to read is never yanked away mid-sentence.

The 200px threshold is deliberately generous: it has to absorb a partially
rendered incoming bubble without the view counting as "scrolled away".

The scroll-to-bottom button uses the same predicate rather than its own
copy, so the button and the follow behaviour can't disagree about where
"bottom" is.

The policy lives in src/lib/auto-scroll.ts with no runtime imports, so the
threshold and signature behaviour are covered by plain node --test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sitions (dzianisv#186)

Three separate defects combined to leave a mobile client showing a green
connection indicator over a stream that was dead, with no bounded path back:

  1. **The read was unbounded.** The SSE loop awaited `reader.read()` with
     no deadline. A half-open socket -- routine when a phone moves between
     Wi-Fi and cellular, or wakes from doze -- produces no bytes, no `done`
     and no error. The loop parked indefinitely and nothing ever triggered
     a reconnect. `readWithTimeout` races the read against
     LIVENESS_TIMEOUT_MS and rejects, which puts the failure on the same
     path as a genuine transport error so the existing reconnect logic
     needs no special case.

  2. **`connected` meant "we tried".** It was set at the moment a connect
     began, before a single byte arrived, so the indicator reflected an
     intention rather than a verified transport. It now flips only once
     the stream has actually delivered something, and a finer-grained
     `transport` state ("idle" | "connecting" | "live") distinguishes
     dialling from live. Only "live" reads as healthy.

  3. **Backoff reset on a timer.** A 10s `setTimeout` cleared
     `reconnectAttempts` whether or not anything had been received, so a
     connection failing silently kept resetting its own backoff and looked
     stable. Retries now reset only on demonstrated liveness.

LIVENESS_TIMEOUT_MS is 35s: three missed ~10s heartbeats. Long enough that
a single late heartbeat or a brief stall doesn't churn the connection,
short enough that recovery is bounded rather than "eventually".

Also wires an AppState "active" handler to `resume()`, since a phone
returning to foreground is exactly when a stale stream needs replacing.
`shouldReconnectOnResume` deduplicates: foreground and network-restore
often fire together, and two attempts would open duplicate streams and
double-handle every event.

The policy lives in src/lib/sse-liveness.ts with no runtime imports, so
staleness, backoff, resume and health are covered by plain node --test.

Measured on an emulator with an airplane-mode toggle: recovery in ~16s,
versus never before this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@omgoshjosh omgoshjosh changed the title fix(android): keyboard covering composer (#156), stale transcript after reconnect, and uncopyable assistant text fix(android): keyboard covering composer (#156), no auto-scroll (#155), dead SSE streams (#186), stale transcript after reconnect, and uncopyable assistant text Aug 16, 2026
…ianisv#189)

loadConnections() built the API client and then AWAITED project.current()
and path.get() before committing the client to the store. Everything
downstream of startup keys off `client` appearing there -- the SSE event
stream, the catalog load -- so the entire live pipeline was serialized
behind two metadata requests it does not need: neither the project name nor
the server home path is required to stream events. Each request is capped
at the 30s REQUEST_TIMEOUT_MS, so a slow or hanging metadata response
delayed live events by up to that long, on exactly the flaky networks where
prompt connection matters most.

The client is now committed immediately after building; the metadata fetch
fills in currentProject/serverHome behind it. Guarded by comparing the
store's client to the one the fetch was issued with, so switching servers
while a metadata request is in flight discards the stale response instead
of letting it clobber the new connection's state.

Closes dzianisv#189.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@omgoshjosh omgoshjosh changed the title fix(android): keyboard covering composer (#156), no auto-scroll (#155), dead SSE streams (#186), stale transcript after reconnect, and uncopyable assistant text fix(android): keyboard covering composer (#156), no auto-scroll (#155), dead SSE streams (#186), blocked SSE startup (#189), stale transcript after reconnect, and uncopyable assistant text Aug 17, 2026
@Qiiks

Qiiks commented Aug 17, 2026

Copy link
Copy Markdown

The keyboard portion has been extracted into focused PR #190 for easier review: #190. The other fixes in this bundled PR remain separate.

Joshua Castaneda and others added 4 commits August 17, 2026 22:34
Wide content -- markdown tables, code blocks, diffs -- scrolled horizontally
only on a near-perfect left-right swipe. Both the nested horizontal
ScrollView and the vertical transcript claim the gesture at ~10dp of
movement on their own axis, so any diagonal drift handed the touch to the
list and scrolled the page instead.

WideScroll replaces the plain nested ScrollView everywhere wide content
renders. A capture-phase PanResponder claims the touch the moment the drag
is horizontal-DOMINANT -- dx past 6dp and beating 70% of dy, so up to ~55°
off-axis still reads as sideways -- and drives the ScrollView by ref, with a
projected-velocity fling on release and edge clamping from the first frame
(content and layout widths are tracked from onContentSizeChange/onLayout, so
the very first drag cannot overshoot into empty space). A perfectly straight
swipe still takes the native path; the responder only decides the sloppy
ones. The transcript cannot steal the touch back mid-drag.

Markdown tables needed their own renderer override: react-native-marked's
MDTable brings its own plain ScrollView, so the same table structure is
rebuilt inside WideScroll rather than nesting two horizontal scrollers.

Padding: scrollable content gains right-side breathing room (24dp code,
16dp tables/diffs) INSIDE the scroll extent -- without it the longest line
sat flush against the clipped edge when scrolled fully, reading as cut off
even when it wasn't.

The issue-dzianisv#21 regression tripwire is updated to guard the new invariant
(wide content lives in WideScroll, never truncated) and now covers tables
too; the old scroll-config module it guarded is deleted rather than left as
dead weight.

624 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The transport downloads the full session list, then limit:50 threw away
every root past the newest fifty — the list's search and filters could only
see what survived, so older sessions were unfindable. FlatList virtualizes,
so row count is not a render concern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The store holds one transcript globally and still contains the previously
viewed session's messages for the first frames after navigating (the
select runs in an effect, after render) — so opening a session briefly
showed the LAST session's messages under the new title. The transcript now
binds to the route id and renders nothing until the store has switched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every file that needed dim text in dark mode had independently picked
#666666 — roughly 3:1 against the app's near-black surfaces, below the
4.5:1 floor for small text (model picker meta, tool card timers, diff
prefixes, popover hints, chevrons). Floor is now #9a9a9a (5.5–7:1 on every
surface used), still visually secondary next to #fff.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@omgoshjosh omgoshjosh changed the title fix(android): keyboard covering composer (#156), no auto-scroll (#155), dead SSE streams (#186), blocked SSE startup (#189), stale transcript after reconnect, and uncopyable assistant text fix: keyboard/composer (#156), auto-scroll (#155), dead SSE (#186), blocked SSE startup (#189), stale transcripts, uncopyable text, axis-perfect table scrolling, 50-session list cap, transcript flash, dark-mode contrast Aug 18, 2026
@omgoshjosh

Copy link
Copy Markdown
Author

Added four more universal fixes (each reproduced against this repo's code, tests green: 329 pass, tsc clean):

  • fix(scroll): horizontal scrollers (tables/code/diffs) only won the gesture race on a near-perfect left-right swipe — both the scroller and the vertical list claim at ~10dp on their own axis, so any diagonal drift scrolled the page. New WideScroll wrapper claims via capture-phase PanResponder when the drag is horizontal-dominant (dx ≥ 6dp beating 70% of dy, ~55° tolerance), drives the ScrollView by ref with a projected-velocity fling, and blocks mid-drag termination. Straight swipes still use the native path. Also adds right-edge padding so fully-scrolled content doesn't sit flush against the clip edge.
  • fix(list): session.list downloads the full list, then limit: 50 sliced away every root past the newest fifty — search/filters only saw survivors, so older sessions were unfindable. Slice removed; FlatList virtualizes.
  • fix(chat): opening a session flashed the previous session's transcript for the first frames (the store's single transcript is still the old one until the post-render select lands). Transcript now binds to the route id.
  • style(dark): dim text in dark mode was systematically #666/#777 (~3:1 on near-black, below the 4.5:1 floor for small text). Floored at #9a9a9a (5.5–7:1) across pickers, tool cards, diff prefixes, hints, chevrons.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants