feat(mobile): rebuild the chat tab on the harness SDK - #5895
Open
iscekic wants to merge 17 commits into
Open
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Previous Review Summaries (9 snapshots, latest commit e585a7e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e585a7e)Status: No Issues Found | Recommendation: Merge Files Reviewed (31 files)
Previous review (commit 326f2c3)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit a7e6c90)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit 02d4575)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit fee11c2)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit 5c686fd)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit 737d3e6)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit a5fa1e0)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (10 files)
Fix these issues in Kilo Cloud Previous review (commit 3f6eee7)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (38 files)
Plus locale JSON, drizzle/db snapshots, lockfile, and deleted quick-chat sources. Reviewed by grok-4.6 · Input: 247.3K · Output: 8.7K · Cached: 370.6K Review guidance: REVIEW.md from base branch |
iscekic
force-pushed
the
feat/mobile-harness-chat
branch
from
September 5, 2026 23:04
ef4251c to
326f2c3
Compare
The chat tab is rebuilt on the harness SDK, so nothing of the previous implementation is kept: the screen and its hooks, the tRPC router that stored a thread per user, both Postgres tables, and every string. The tab itself goes with it and returns with the new surface.
The tab keeps its label and its icon and gains a beta pill. Behind it is a new surface: a list of chats, and one screen per conversation that runs on the device through the harness SDK rather than through a server session. A chat is not tied to its screen. The live sessions live in a module registry, so a person can ask something, leave for another tab, and come back to the answer, and several chats can be answering at once. The layering is the point of the split: - The SDK's store saves one conversation and reads it back. - The app owns everything about many of them: the list, the title, the order, the delete, the scope per account and organization, and the wipe on sign-out. That is SQL in `lib/chat/store.ts`, over both owners' tables on the one encrypted database. Switching models mid-conversation clones the session onto the new model and moves the chat row onto it, so one conversation stays one row. A question whose answer never arrived is remembered outside the store and drawn with a Retry, because the store writes a question and its answer together or neither.
Everything here came out of running the tab against the real gateway on a simulator. React Native has no global `crypto`, so the SDK's default entropy raised on the first session. The app supplies expo-crypto, which is what already encrypts the database. The SDK's store creates its tables when its layer is built, so a list drawn before any chat had ever opened read tables that did not exist; the list asks for the runtime first. A failed answer logged nothing, because an Effect cause carries its own text and this package's errors carry their fields — the failure value is read instead. The header drew no title, and the composer kept the text it had sent. The list is read from the database, and a chat writes its turns when its answer ends. Nothing told the list that had happened, so a chat started from the list kept the row it was given: no title, because the title is the first thing said in it. The registry now says when a chat starts or stops working — not on every word of an answer — and the list reads the database again, whichever screen the answer arrived on. FlashList holds the scroll position when content is added at the top, which is right for a transcript and wrong for a list of chats newest first: the new row went above the top of the list, behind the header.
apps/mobile typechecks against the built package, the way it already does against trpc. The build now runs in the same place, under the same rule.
The mobile suite opens the SDK's store against the real schema, so it needs the built package the same way the typecheck does.
Four things the review found, all in what a person sees while a chat works. The question being answered was hidden until its answer landed: the screen passed it to the transcript only when the chat was idle. It is passed always now — the screen hands over the whole state and decides nothing. A second question sent while an answer arrived started a second read of the same session. The composer is deliberately open while the model works, so this is ordinary: the question now joins a line, is drawn where it will be asked, and is asked when the session is free. Stopping an answer moves the line on; a failed one holds it, because that question keeps the Retry. A question whose send failed before the model saw it vanished with the composer's text. It now stays on screen with a Retry under it, the same as one whose answer never arrived. An account signing in over another left the first account's chats running. They are ended, before the account's cache is cleared. The rows stay, scoped to the account that made them, exactly as the read cache on disk does. The registry outgrew one file: what a chat looks like is now its own, so the registry is only what drives one.
Two things the line got wrong, both found by review. Closing a chat went through the stop, and the stop asks what is waiting. So signing out, or deleting a chat, opened a round on a session whose scope was closing under it. The interrupt is now its own step: a chat that is stopped asks the next question, a chat that is closed drops it. A question typed while an answer arrived was asked on whatever model the session was on by then. It is asked on the model that was on screen when it was typed, which is what the person picked. The registry has its first tests: the SDK is faked, because none of this is about the SDK.
A chat is a session of a plainer kind, so it now looks like one. The row is the SessionRow every other list uses: the model names it and colours its strip, the same live dot marks an answer still arriving, and the rows run edge to edge with one hairline between them. The header loses the context control and the corner button. Starting a chat is the floating button the sessions list already puts in that corner, and it stays out of the way of an empty list, which carries its own.
A conversation puts its composer where the tab bar sits, so the bar goes while one is open, the way it already does for the other full-screen routes. The composer then sits on the home indicator instead of on the bar that is no longer there. The rule reads the route segments rather than the path: a conversation's path is the session id and nothing else, which no path rule can tell from any other one-segment route.
A model has no clock. Asked what day it is, how old something is or whether a deadline has passed, it answers from the date it was trained on, with the same confidence it says everything else and no sign that it has moved. A phone is where that gets asked. So every chat is opened with the SDK's time tool, reporting local time in the zone the device is set to and UTC beside it. The system prompt stops claiming there are no tools and says to read the clock rather than assume it. The other three tools the SDK ships stay off: the composer is already how a person is asked something, a subagent costs a second session, and a to-do list is working memory for a long run a chat does not have.
The eleven models the SDK's live runs use are the ones a chat change is proved against, and the model picker's accessibility tree is what takes the Appium server down when a run reads it whole.
…uestion Two things a question could fall through. A question typed before `enterChat` finished found no chat, and `say` returned the identifier it was given as though it had asked. The composer had already emptied, so the question was gone with nothing on screen to say so. It now waits for the open, and a chat that is genuinely not there raises rather than reporting success — which puts the question back on screen with a Retry under it, the same as one whose answer never arrived. A queued question asked on another model clones the conversation, and `drain` ignored the new identifier: the transcript moved to a session the screen was not watching. The chat that was left now points at the one it became, and `useChat` follows that, so `say` and `retryChat` hand nothing back. The screen is not always the mover, which is why the return value was the wrong seam.
iscekic
force-pushed
the
feat/mobile-harness-chat
branch
from
September 6, 2026 02:11
326f2c3 to
2bf4d8d
Compare
Resolutions: - the chat tab keeps this branch's showChatTab and takes main's move of the label to common.chat - the quick-chat screen, its hook and its test stay deleted; this branch replaces them - every locale takes both sides: this branch's chat keys and main's additions, with quickChat removed - the drop of the quick chat tables is regenerated as 0238, after main's 0236 and 0237
…/mobile-harness-chat
Main added common.chat, common.working, common.delete, common.message and common.couldNotLoadModels while this branch carried its own wording for the same English. The catalog check holds one wording per string, so the chat screens now read the shared keys and the duplicates are gone from all 87 catalogs.
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.
Stacked on #5878. Review that one first — this is its first consumer.
The chat tab is rebuilt on
@kilocode/harness-sdk. The old quick chat is gone:the screens, the tRPC router, the two database tables, and the rows in them. It
is not migrated, it is removed, and the new tab starts empty.
The tab keeps its label and its icon, and gains a BETA pill in the header.
It is behind
FEATURE_FLAG_CHAT, off by default.What a person can do
+, or open one they had.uses.
still shows one chat.
closed and reopened.
Several chats run at once. A conversation lives in a module registry rather
than in the screen showing it, so leaving the screen while an answer is
arriving does not stop it, and coming back finds it.
Where the code lives, and why
The split the SDK asks for, applied:
So
src/lib/chat/store.tsholds every list and CRUD statement, and it is theone file that reads across both owners' tables on the one encrypted database —
the app's
chatstable and the SDK store'ssessions/turns/parts. The SDKsaves a conversation; it does not know there is a list.
The platform pieces are the app's too: the token comes from the app's auth, the
randomness from expo-crypto, and the database is the app's existing SQLCipher
one, so a chat is encrypted at rest like everything else.
What proves it
the line a chat keeps while it is answering.
The list and the delete run against the real schema of both owners rather
than a copy of it, so a schema that moves breaks the suite.
chat, a three turn conversation, a mid-conversation model switch, a retry
that survived closing the app, a second question sent while the first was
still being answered, deleting a chat, and signing out, which takes the
chats with it.
Three defects came out of those runs and are fixed here: a chat's row had no
title until something else refreshed the list; a new row was drawn behind the
header, because FlashList holds the scroll position when content arrives at
the top; and a second question raced the first rather than waiting for it.
Outside the app
packages/db— a migration dropping the two quick chat tables.packages/trpc,apps/web— the quick chat router and its feature flag,removed.
.github/workflows— the harness SDK is built before the jobs that typecheckor build the app.