Conversation
The chat list drove autoscroll from a followTail flag recomputed as `!listState.canScrollForward` whenever scrolling settled. Streamed markdown grows after each measure pass, so the pass right after a scroll to the bottom made the list scrollable again and cleared the flag by itself, stranding the viewport above the newest message. Two further problems came from the same design: scrollToItem takes the ScrollableState mutex, so it raced user gestures and flings and could be cancelled mid-animation, and the layout -> scroll -> layout loop only terminated because snapshotFlow deduplicates. Invert the ownership instead of patching the flag. With reverseLayout the newest row is index 0 and is drawn against the bottom edge, and the list anchors scrolling on the first visible item, so staying at the bottom becomes a property of measurement rather than something code chases: a growing row is the anchor, so it expands upwards on its own. Only insertion still needs handling, since stable keys keep the anchor on the previously newest row. requestScrollToItem does that without taking the scroll mutex, so it cannot race gestures. Keys are kept, unlike Jetchat, because ReasoningCard and code blocks hold rememberSaveable state that would otherwise shuffle between rows. Flatten the transcript into ChatRow up front so indices are meaningful, and key the list state on a new sessionId so each session keeps its own position -- the old reset compared title and project, which collides between sessions sharing a name.
Replace the single-line PIN field with six per-digit cells, backed by one BasicTextField holding the whole PIN rather than one field per cell. That backing choice is what makes backspace erase the PIN digit by digit. With six real fields an empty cell never receives the key event at all -- there is no text to delete, so the IME emits nothing -- and deletion stalls on the first empty cell unless keys are forwarded between fields by hand with onKeyEvent and FocusRequester. One field keeps the caret after the last digit, so deleting right to left is the native behaviour. The caret is also pinned past the last digit on every recomposition, so tapping a cell cannot drop it mid-PIN and desynchronise typing from the cell being looked at, and input is filtered in onValueChange instead of relying on the keyboard type, which covers paste and autofill. Cells divide the available width so they stay equal on any screen; fixed widths overflowed the dialog, and an overflowing Row shrinks whichever child runs out of space last, which left the sixth cell narrower. Height stays fixed because aspectRatio grew the cells vertically once their width was shared. Also give the connect button room for its spinner and label: the default content padding wrapped "Connecting..." onto a second line, which broke the button's vertical centring. The spinner was drawn in colors.bg while the button sits disabled on surface2, making it nearly invisible, so it now uses the disabled content colour.
BasicTextField's TextFieldValue overload is fully controlled: its internal buffer only follows the passed value on recomposition. The filter skipped onValueChange when the digits were unchanged, which is exactly the case that needs the resync -- so no recomposition happened and the buffer kept characters the PIN had rejected. Typing a 7th digit into a full PIN left 7 characters in the buffer while 6 were displayed, so the next backspace deleted the invisible one and the visible digit needed a second press. Pasting letters into an empty field left them buffered the same way. Hold the TextFieldValue locally and assign it on every change, so the field is corrected even when the PIN itself does not change.
buildChatRows appended the orphan tool pile before reversing, so whenever it existed it became index 0 for good, and its key is a constant. That defeated both halves of the pin: an incoming message landed at index 1 leaving the newest key unchanged, so the LaunchedEffect never fired and the message stayed off screen, and requestScrollToItem(0) on submit scrolled to the tool pile instead of the sent message. Orphans are reachable in production -- SnapshotReceived takes event.tools as-is without requiring the tools to appear in parts, so any desktop snapshot carrying a non-empty tools list produces them. They have no chronological anchor, so prepend them instead: they render at the oldest end of the transcript and the newest row is always a real message. Fixing the ordering rather than excluding them from the newest-key computation also corrects the submit scroll, which the narrower fix would have left pointing at the pile.
TurnChanged minted a fresh UUID for the in-flight text part on every event, not just when the turn closed. Each streaming chunk therefore changed the row key, which the list reads as a removal plus an insertion: the row was rebuilt, its layout state discarded, and LaunchedEffect on the newest key re-fired spuriously. The old autoscroll survived this by re-pinning every frame by brute force. Anchoring the viewport on keys makes their stability a correctness requirement, so fix it here. Reuse the containing message's id, which is already stable across the turn, and shape the part id like the snapshot parser's "\$id-text-\$p" so the key also survives the streamed turn being replaced by the server-sent version when the turn closes.
isError was wired to errorMessage != null, but connectionError is a single field shared with QR scanning, so "Invalid QR code: no Roxy connection token found." and "QR Scanner error: ..." painted the PIN cells red despite saying nothing about the PIN. Nothing cleared it as the user typed either, since the error lives in the view model and is only reset by another connection attempt. Drop the cell marking once the PIN is edited. The banner still shows the message, so no information is lost, but the cells stop attributing the failure to the digits. Also let the active cell outrank the error state in PinCell. The error arm came first, so an errored field lost its active-cell highlight entirely and the user had no visible entry point while retrying.
RoxyApp switches destinations with a when, so ChatFullScreen leaves the composition on the way back to Main and its list state is recreated anyway. openSession is only reachable from the session list, so there is no chat-to-chat transition: the sessionId could only change while mounted during disconnect, which clears the messages regardless. The field and its four assignments existed purely to key the list state on something navigation already guarantees, and the comment claimed a behaviour that was never the reason it worked. Remove it rather than document it as defensive -- if in-place session switching is added, the list state can be keyed then, at which point it will actually do something.
rows describes renderable rows, not whether the session has a transcript. An assistant message with no parts and blank text legitimately produces zero rows, but the session is not empty and should not show "No messages yet. Send a prompt to get started." The previous condition regressed the old messages.isEmpty() && toolCalls.isEmpty() check by conflating those two meanings. Restore the semantic check against the source UI state and leave the list branch responsible only for rendering whatever rows exist.
buildChatRows is rebuilt on every streaming message update, so it does an O(n) transcript walk per chunk. Keep that transformation in the UI layer: rows are presentation state for LazyColumn, and moving the same walk into the ViewModel would couple business state to composable row shapes without reducing the work. Make the trade-off explicit, and remove the avoidable extra pass over rows used to recover rendered tool ids. Record those ids while walking message parts instead, and only allocate the set when the snapshot actually has tools to compare against.
Move PaddingValues back into the capitalized layout import group so the file keeps its existing import ordering convention.
A reversed chat list should keep following the newest row when the viewport is effectively resting on it, not only when its scroll offset is exactly zero. A one-pixel offset after a fling or nested scroll should not opt the user out of autoscroll. Use a viewport-relative threshold for the follow-tail check, persist the LazyListState with its saver, and show an explicit Latest affordance when new rows arrive while the user is reading history. Sending remains the only forced jump to row 0 because a user-submitted prompt is explicit navigation back to the live edge. Expose buildChatRows internally and cover row ordering/key behaviour with unit tests, including orphan tools at the oldest end.
The turn frame parser collapsed text and reasoning parts into one
inFlightText blob. That made ReasoningCard disappear during streaming and
then reappear only after the final snapshot, and it made the stable row
key work depend on a synthetic single text part that did not match the
snapshot shape.
Emit the live parts from RemoteWorkspaceClient alongside the legacy
inFlightText summary, and let the ViewModel rebuild their ids from the
stable containing message id plus the source part index. That keeps text
and reasoning rows stable across chunks and aligned with the snapshot ids
("-text-" / "-reasoning-").
Add regression coverage for parser output and ViewModel key stability.
Keep the pairing PIN length in one shared constant instead of scattering literal 6 checks across the dialog, parser, client validation, and PinInput default. Make the token field's Next action focus the PIN, and focus the PIN immediately when a QR scan prefills the token but still needs manual PIN entry. PinInput now keeps the editable text field's own semantics and marks it as a password instead of overriding the same node with a custom contentDescription that conflicted with EditableText in TalkBack. Add an instrumented Compose regression test for rejected input: pasting letters plus a seventh digit should not leave an invisible character in the IME buffer that requires an extra backspace.
# Conflicts: # app/src/main/java/gg/roxy/chatFullscreen/components/ChatComposer.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing