Repository navigation
Code health: split useThread and the big screens, typed error codes, pure src/lib - #175
Merged
Merged
Conversation
5 of 6 tasks
9 of 10 tasks
woosal1337
force-pushed
the
hc-46-speed
branch
from
October 16, 2026 15:35
7c99a05 to
1e949bd
Compare
woosal1337
force-pushed
the
hc-47-code-health
branch
from
October 16, 2026 15:35
6479a62 to
8be5ffc
Compare
woosal1337
force-pushed
the
hc-46-speed
branch
from
October 16, 2026 16:52
1e949bd to
06eaded
Compare
woosal1337
force-pushed
the
hc-47-code-health
branch
from
October 16, 2026 16:52
8be5ffc to
4ba24b9
Compare
…time
Two comments rested on a client that runs one command at a time: the slash
scan's delay ("a scan sent with it would hold the transcript up") and the
tail-error ordering ("serialized ahead of the poll's calls"). Neither native
module queues commands; each exec opens a channel of its own and they run in
parallel. The decisions stand, for the reasons now given: the scan competes
with the transcript read for the host and the link, and the tail's first
call simply starts first.
(cherry picked from commit 6243c6c)
`useThread.reload` ran its own `DELETE FROM messages` and `DELETE FROM tail_cursors` inside a transaction, the only SQL in a feature hook; everything else the thread stores goes through `threadCache`. It is `clearThreadCache` there now, tested against a real SQLite, and the hook's test checks the call instead of the SQL text. (cherry picked from commit 442d9d1)
Route files stay thin, and the root layout was the one that was not: 241 lines, most of them the `RootStack` navigator (its screens, their presentation, the three surfaces told what colour the app is) and the `Booting` screen shown while the database opens. Both move unchanged into src/features/root/, and the route keeps the providers and the module-scope notification handler: 59 lines. The test that fails on a screen declared twice reads RootStack.tsx now, where the screens are. The fix that test came with is on this branch too, the same change as in #173. Merged after #173, the two still conflict in app/_layout.tsx and rootLayout.test.ts, an edit against a move; this branch's side of both is the right one, since the fix is already in RootStack.tsx. (cherry picked from commit 5c052bd)
CLAUDE.md: src/lib imports nothing from React or the SSH module, which is what lets the core run against a canned host. Two files broke that at runtime: - `haptics.ts` imported expo-haptics and react-native, and read the app's settings store. It is UI feedback, used by components and screens, and moves to src/components. - `herdr/sshTransport.ts` is the adapter onto the native SSH module itself; only the connection store builds one. It moves to src/state, beside it. Every import and jest mock follows. Five src/lib files still name the module's `ExecResult` type; those are `import type`, erased at build, and a copy of the type would be worse than the reference. (cherry picked from commit 5de30a9)
CLAUDE.md allows no colour, radius, duration or spacing value outside src/theme, and a few had stayed in components: - Markdown's washes of white over the outgoing bubble (a code or table fill, a rule, a table header, a quote's bar) are `onTintWash`, and the quote bar's width is `size.quoteBar`. - The typing dots' stagger and pulse, the waiting bar's sweep, the presence dot's breath and the swipe hint's rest, nudge and return are `motion` tokens, beside the durations already there. Values unchanged, so nothing on screen moves. The one `rgba` left in a component is Glass's computed transparency mask, not a colour. (cherry picked from commit 060fe4d)
HerdrError.code was a plain string, so a misspelt code in a comparison was simply never true, and nothing said so. The code is now a union of the codes the SSH layer, herdr and this app raise. A typo in a `new HerdrError`, a comparison or a switch fails the typecheck; a test pins that with @ts-expect-error. herdr can still send a code no list has: it is kept, matches no comparison, and reaches a person in herdr's own words (`wireCode`). The holders of a code follow: the chat list's state and feeds, the host editor's test result, connectionRecovery, needsTheUser, friendlyMessage. `errorText` moves from useWorkspaces to protocol.ts and replaces the 26 copies of `x instanceof (HerdrError|Error) ? x.message : String(x)`. The HerdrError form showed a plain Error as "Error: message". The thread path's three plain Errors (an invalid session id, an unreadable transcript) are HerdrErrors with a code now, like the rest of it. (cherry picked from commit bfc00e4)
useThread was one hook of 1,710 lines doing about fourteen jobs, and every fix to one of them meant reading all of them. It is now 307 lines that wire the parts together; the logic moved as it was, comments with it: - src/lib/threadWindow.ts: the messages on screen, apart from React. Arrivals and echoes, dedupe, a receipt confirming an echo, continuing a re-read window. Jest tests it directly now. - useThreadHistory: the list state, the cache writes, paging up. - useTranscriptTails: the live streams, cursors, restarts, the watchdog and the header's model. - useSessionBinding: which session the thread shows, rotation, the waiting/missing/replaced state, the open from the list's cache. - useAgentPoll: the status loop and its backoff, handing each snapshot on. - useBlockedPrompt, usePaneOverlay: a question, or a command's panel. - useDelivery: send, retry, interrupt, and whether a message landed. The refs the parts share are named aliveRef, boundSigRef and kickRef, so the compiler's lint rules treat them as refs; split up, the hooks are small enough for those rules to read. No behaviour change: the 63 useThread tests and the command budget test pass as they were. (cherry picked from commit 0f2ecbb)
ThreadScreen was one component of 790 lines: the header, the message list, the floating controls, the keyboard, pictures, the integration install and the host lookup, so every change to one of them meant reading all of them. It is 367 lines now and composes: - ThreadHeader, with threadHeading (the title and its line) and statusText. - ThreadMessages: the list, paging up, retry, the agent at work. - ThreadControls: a panel, the palette, a question, around the composer. - useThreadKeyboard, useAttachments, useIntegrationInstall, useThreadConnection, useReadMarker. The list takes the thread's fields one by one, not the thread, so a keystroke in the composer still re-renders no row. Every part compiles under the React Compiler, checked with its logger. The testIDs are unchanged and the screen's tests pass as they were. (cherry picked from commit e6dc7b4)
…rServer ChatsForServer was 590 lines, most of it one renderItem. It is 380 now: - ChatListItem draws a row: group, tab, terminal, pane or chat. - ChatSearch is the search field. - useHostRecovery holds the fix for a failed connection and the changed-key pin it shows. - useListReads loads the read markers, on focus and on a new detail. Both components and both hooks compile under the React Compiler. The list's tests pass as they were. (cherry picked from commit 99ddc70)
… 400 lines ServerEditScreen was 504 lines; the field state, validation and dirty check move to useHostForm and the three field blocks to HostFields, leaving 352. A test failure there now keeps the message the error already carries, rather than reading it twice. A test reads every component in src/features and fails on one over 400 lines, so the screens split here do not grow back unnoticed. It fails on the host editor as it was. (cherry picked from commit ad10682)
3 of 4 tasks
cobanov
force-pushed
the
hc-47-code-health
branch
from
October 26, 2026 12:53
4ba24b9 to
cba9a05
Compare
Owner
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
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
Rebased onto
mainat 2ce4a9a (#178), with the machine code main removed left out of the split hooks. Commit ids below are the rebased ones.The architecture review found a sound core and a few places where the code had outgrown its own rules: one hook of 1,710 lines, three screens of 500 to 790 lines,
src/libfiles that import React and the SSH module, and an error code that was a plain string. This branch fixes each of them, one change per commit. Nothing on screen changes; the existing tests pass as they were, and each change adds a test that pins it.Stacked on #174. It changes
useThreadandThreadScreen, which #174 changes too, so it is based onhc-46-speed. It retargets tomainonce #172 and #174 merge.Changes, one commit each
Structure
dacc14cuseThreadis split into focused hooks and a pure message window. It was one hook of 1,688 lines with about fourteen jobs. It is 302 lines now and only connects the parts:src/lib/threadWindow.tskeeps the messages on screen apart from React: arrivals and echoes, dedupe, receipt matching, and continuing a re-read window. Jest tests it directly.useThreadHistory,useTranscriptTails,useSessionBinding,useAgentPoll,useBlockedPrompt,usePaneOverlayanduseDeliveryeach own one job.useThreadtests and the command budget test pass without edits.082fbc5ThreadScreenis split into its header, list and controls (886 → 401 lines). The list takes the thread's fields one by one, so a keystroke in the composer still re-renders no row. Every new component and hook compiles under the React Compiler; I checked this with the compiler's logger.9ee0480ChatsForServeris split into a row component, the search field, host recovery and read markers (the file, 714 → 470 lines).4ba24b9The host editor's fields are split out (558 → 406 lines). A new test reads every component insrc/featuresand fails on one over 400 lines. It fails on the host editor as it was.9a3f727app/_layout.tsxis thin again (237 → 59 lines). The root navigator and the boot screen move tosrc/features/root/.Rules from CLAUDE.md
d5bbe9csrc/libis pure.hapticsmoves tosrc/components, and the adapter onto the native SSH module moves tosrc/state. Onlyimport typereferences to the module remain insrc/lib.e63feb7No raw colours or durations outsidesrc/theme. These are now tokens with the same values: Markdown's washes over the outgoing bubble, the quote bar's width, and the timing of the typing dots, the waiting bar, the presence dot and the swipe hint.cb7c8abThe thread's reload SQL lives in the thread cache module (clearThreadCache). That SQL was the only SQL in a feature hook. It is now tested against a real SQLite.Errors
413766dHerdrError.codeis a typed union. A misspelt code in anew HerdrError, a comparison or aswitchnow fails the typecheck, and a test pins this with@ts-expect-error. herdr can still send a code that is not in the list. That code is kept, matches no comparison, and reaches the person in herdr's own words.errorText, replaces 26 copies ofx instanceof (HerdrError|Error) ? x.message : String(x).Errors are nowHerdrErrors with a code.Small fixes
19ba0d6Two comments said the SSH layer runs one command at a time. It does not: each command opens its own channel. The comments now give the real reasons for the decisions they explain.Test plan
npx tsc --noEmit: no errorsnpx expo lint: no errors, no warningsnpx jest: 2,017 passed, 14 skippedpython3 -m unittest discover -s scripts -p 'test_*.py': passesThreadScreen,ChatsForServerand every new component and hook compile.useThread's parts and the host editor do not, the same as before the split (try/finally).npm run e2ein Dark and Light on the iPhone 17 Pro simulator (iOS 27), Release build of the rebased stack (Make every agent herdr detects visible: Grok and Antigravity chats, terminal rows for the rest, Codex fixes #172 to Close the remaining product gaps: one socket bridge, Antigravity dialogs, encrypted keys, accessibility #177): 52 of 52, the four flows5a4cf24fixes rerun on their own after it. Screenshots opened in both modes.