mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-19 01:50:51 -05:00
feat(mobile): resume in-flight chat runs + auto-name new sessions (#12585)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
c5ee750866
commit
c183828acb
9 files changed
+831
-35
No files matched your search
@@ -143,6 +143,11 @@ PR 3 is the spine; PR 4/5/6 and all of PR 9 fan out from it independently. PR 7
|
||||
- **Tests on merge:** RN Testing Library — resume re-attaches to a mocked live run; pagination loads older mocked pages; guard prevents cross-session writes.
|
||||
- **Before you start (grill on):** `resume-stream` cursor semantics + `current_run` shape; how to detect a live run on open. Auto-name endpoint + trigger timing. Which endpoint/params page older messages.
|
||||
- **Drift checkpoint:** Confirm resume is still wanted for v1 (it's polish; could defer if scope tightens).
|
||||
- **As built (2026-07-01, "grilled before coding" — scope tightened to resume + auto-name):** The grill collapsed the "history pagination" line item. **Within-session message pagination was DROPPED** — web has none (it loads the whole `get-chat-session` snapshot and lets virtualization handle long chats; there is no older-messages endpoint to page against), and mobile already gets the full conversation in one call + FlashList already virtualizes. Implementing `onStartReached` would have been a mobile-only divergence, so `MessageList.tsx` is **unchanged**. The **session-list** cursor (sidebar recents, built in PR 1) keeps its `before=time_updated` cursor as-is — the owner chose to leave the rare same-timestamp tie rather than add a `(time_updated, id)` compound cursor to the shared backend route (the chat port stays **backend-free**; comment updated in `api/chat/sessions.ts`). So PR 4 = **resume in-flight run + auto-name** only (~190 source LOC + tests, under the ~450 est).
|
||||
- **Resume (faithful web port):** new `hooks/useChatSessionController.ts` — a module-scope `runResumeStream` (survives re-renders, like PR 3's `runChatStream`) + a module `resumingRuns: Set<number>` dedupe. The hook is a **read-only observer** (`useQuery` `enabled:false`) of the `get-chat-session` snapshot that `useChatController` fetches (same query key → React Query dedups; no second network call). When the snapshot's `current_run.run_id` maps to an **assistant** node (single-model only — a multi-model run_id is the user message and fails the node-type check, matching web), it re-attaches: `resumeChatMessage(sessionId, cursor=0, signal)` (full buffer replay + live tail), ~50 ms batched flush, a `stillCurrent = currentSessionId===sessionId && !aborted` guard (drops writes once the user switches away). **Heartbeats pass through on the resume path** (web does the same via `resumeStream`, unlike send) so the loop re-checks focus during quiet phases and releases the connection promptly on navigate-away; they're excluded from render via `isHeartbeat`. On finish/error (**404 = nothing to resume**, swallowed by the bare catch) it settles from `get-chat-session` (`processRawChatHistory` → `hydrateSession`) and `setQueryData`s the fresh snapshot so a remount can't re-resume a finished run. **abortController is reused** (not a second field): send and resume are mutually exclusive per session (resume only fires on cold-open hydration, guarded by `data.abortController == null`), so PR 3's existing `stop()` aborts a resume for free. The settle-block guards `hydrateSession`/`setQueryData`/controller-release on **`stillOurs()` — the resume's own `AbortController` object used as an ownership token held through the settle await** — so a send that races in (and even completes) during the `get-chat-session` fetch is not clobbered by the now-stale snapshot (a completed send leaves a `null` controller, which ≠ the resume's token).
|
||||
- **Auto-name:** `runChatStream` now takes an `AutoNameContext | null` (set only when `sessionId == null` at submit — a brand-new session). In its `finally`, when the run produced an answer (`!signal.aborted && sawStreaming && !hadError`), it fires `nameNewSession`: a 200 ms delay (mirrors web's `handleNewSessionNaming` — backend needs a beat to persist the row), then `renameChatSession` → `PUT /chat/rename-chat-session {name:null}` (backend LLM-generates the title), then invalidates `chatSessions` so the sidebar + the header title (which reads from `useChatSessions()`) update and the new chat appears. `api/chat/sessions.ts` gains `renameChatSession`; `stream.ts` refactors the NDJSON reader into a shared `readNdjson` used by both send + resume, adds `resumeChatMessage` (GET, bearer) + `StreamHttpError`.
|
||||
- **Divergences from web (per WEB-PARITY PRINCIPLE):** (1) no within-session message pagination (neither does web — parity preserved); (2) auto-name fires only after a **new** session's first answer — web additionally has a "catch-up" rename on the 2nd message if the first naming failed (`useChatSessionController.ts` `newMessageHistory.length >= 2 && !description`); mobile defers that minor recovery path; (3) mobile reuses the single `abortController` for resume (web uses a dedicated one) — safe because the two are mutually exclusive per session on mobile.
|
||||
- **Files:** `hooks/useChatSessionController.ts` (new), `hooks/__tests__/useChatSessionController.test.tsx` (new, 9 tests); modified `hooks/useChatController.ts` (auto-name), `api/chat/stream.ts` (`resumeChatMessage` + `readNdjson` refactor + `keepHeartbeats` + exported `isHeartbeat`), `api/chat/sessions.ts` (`renameChatSession` + cursor comment), `components/chat/ChatConversation.tsx` (mounts the resume hook), `hooks/__tests__/useChatController.test.tsx` (+3 auto-name tests). **Web untouched; no backend change.** Verified: mobile `tsc` clean, lint clean (2 pre-existing `_layout.tsx` warnings), **141 jest pass** (12 new). **Two adversarial multi-agent reviews** (each: 3 dimensions × independent verification). Review 1 (19 findings) surfaced 2 confirmed-real bugs, FIXED — (1) the settle-refetch could clobber a send that completed during its await → fixed with the `stillOurs()` ownership-token guard above; (2) heartbeats were filtered on the resume path, dropping web's quiet-phase liveness re-check → fixed with `keepHeartbeats` passthrough (a stop-during-resume `setQueryData` "uncertain" was resolved by the same guard). Review 2 (post-fix) confirmed **both fixes correct (0 fix-correctness findings)** and the comment cut behavior-neutral; its only actionable items were code-quality nits — applied: dropped an unused `readNdjson(signal)` param, tightened the auto-name comment (in-band error packets still name, web-faithfully), and added the observer post-mount-reactivity test. **A GitHub-bot review round** (Greptile + cubic, 4 P2 nits) followed: `authHeaders` no longer sends `Content-Type` on the bodyless resume GET (added only on the send POST); `FLUSH_INTERVAL_MS` extracted to `chat/constants.ts` (shared by both stream hooks); the observer `queryFn` uses `skipToken` instead of a non-null assertion; and the resume catch now logs non-404 failures (`StreamHttpError` reintroduced so the expected 404 stays quiet). **143 jest pass.** On-device resume/auto-name against a live backend is the remaining manual check.
|
||||
|
||||
## PR 5 — Agent selection
|
||||
- **Goal:** Browse available agents, pick one, start a chat with it; use its starter prompts.
|
||||
|
||||
Reference in new issue
Block a user