Phase 3: client feature slices and one typed API layer - #48
Merged
Conversation
Each phase of this refactor lands its reconciled plan alongside the work, so a reader can see what was intended before reading what changed. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Everything that talks to the server is moving under client/api/, and the transport that resolves the base URL and wraps fetch belongs there first. The module is unchanged apart from its location, so the existing tests carry over with it. Import specifiers were rewritten with a TypeScript compiler API codemod rather than a text substitution, which is what lets dynamic imports and type-position imports move along with the static ones. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Pins the behaviour the twenty-odd hand written response checks scattered across the client should collapse into. The interesting cases are the ones those hand written checks disagree about today, namely which field of the error body wins, what happens when the body is not JSON at all, and what a no-content response should resolve to. Also pins the trace opt out. A GET that carries a custom header stops being a CORS simple request and costs a preflight round trip, which the one second library poll cannot afford. This commit is deliberately red because request and ApiError do not exist yet. The pre-commit hook is bypassed here for that reason alone, and the next commit turns it green with hooks running normally. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Collapses the response check that appears twenty-odd times across the client into one place. Those copies had drifted, some reading the error field of the body and some reading a message field the routes never send, so the ones reading message always showed their own generic fallback instead of the reason the server gave. ApiError reads both, preferring the framework's more specific message when it is present, and keeps the parsed body for callers that need the detail. The former tracedFetch becomes apiFetch, the single fetch primitive the whole client will route through, with a trace opt out for the polls that cannot afford a preflight round trip. Its diagnostic behaviour is unchanged. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Every poll cadence, debounce, animation duration and retry delay the client uses now has a name and a reason recorded next to it. Gathering them makes the relationships between them legible, which is the part no single call site could show. The transport adopts its five values here, and the remaining call sites follow once the feature slices settle. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Five components read streams by hand today, each with its own copy of the reader loop. The cases that matter are the ones a copy tends to get wrong, namely an event split across chunk boundaries, a multi-byte character split across chunk boundaries, and a final line that arrives with no trailing newline. The profile interview sends its finished profile as that last line, so dropping it would lose the whole result. The task stream tests pin reconnection against a stand-in EventSource, since node does not provide one, and cover the case where a reconnect is already scheduled when the subscriber goes away. This commit is deliberately red because the module under test does not exist yet. The pre-commit hook is bypassed here for that reason alone, and the next commit turns it green with hooks running normally. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Three encodings, one reader loop each, replacing five hand written copies that live inside components today. The shared decoder keeps streaming mode on, which is what stops a multi-byte character split across network chunks from arriving as a replacement character. Reconnection for the background task stream moves here as well. Owning the timer inside the subscription rather than inside a component effect means an unrelated re-render can no longer tear the stream down and rebuild it. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Covers and audiobooks are overwritten in place on the server, so the version tag in these URLs is the only thing stopping a browser from serving the old bytes after a regeneration. Pinning it here records why the tag exists, which is not visible from the six call sites that build these strings inline today. This commit is deliberately red because the module under test does not exist yet. The pre-commit hook is bypassed here for that reason alone, and the next commit turns it green with hooks running normally. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Gathers the six inline template literals that address covers, audiobook files and voice previews. The asymmetry between them is preserved as it stands, since the version tag is only a cache key and changing how it is encoded would change every URL for no gain. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This is the one part of the client where a refactoring mistake destroys user data instead of showing a wrong pixel. Renaming the storage key, reordering the transforms or dropping one of them silently discards reading positions, model choices and library filters that people accumulated over months, and nothing fails loudly when it happens. The direction of the fold is the subtle part. Redux persist applies transforms left to right on the way to storage and right to left on the way back, so the last transform in the list is the first to see stored data. Both orders look equally plausible in source, which is why the legacy provider migration is covered together with the library filter migration that runs ahead of it. This commit is deliberately red because the modules under test do not exist yet. The pre-commit hook is bypassed here for that reason alone, and the next commit turns it green with hooks running normally. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The store was a single 604 line file holding five slices, four persistence transforms, the storage adapter and the store itself. Each slice now owns a file, persistence owns a file, and index.ts composes them. The public surface is unchanged. Around forty files import from '@client/store' and not one import line moved, because index.ts re-exports exactly what the old file did. That indirection is the point, since it is what lets a slice be split again later without touching a call site. Every reducer, selector and comment was moved by slicing the original file by line range rather than retyped, so the transforms are byte for byte what they were. The persistence spec committed ahead of this change passes unaltered, including the assertion about which direction the transform list folds. One behaviour-neutral edit was needed. Reading window at module scope threw under the node test environment, so the storage adapter now checks that window exists first, matching the guard the http transport already uses. In a browser the result is identical, and it is what makes the persistence contract testable at all. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Three assumptions in the plan did not survive contact with the code, and a plan that stays wrong is worse than no plan. The plan treats tracedFetch as the path most requests already took. It had one call site out of eighty four, so routing everything through the typed client genuinely changes the wire rather than merely tidying it, and the preflight mitigation is load bearing rather than precautionary. The plan assigns Phase 3 the job of moving the generation event union into shared. Phase 2 already did it, and split one loose union into five per-stream unions, so Phase 3 consumes rather than moves. The plan lists lib/api.ts as three functions. It also declares three response types that shared now owns. Two constants are pinned to the values the code turned out to hold, and the cover URL builder gains a comment recording that its missing percent encoding is deliberate, so the later simplification sweep does not normalise it and change every cover URL in every install for nothing. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The client declared one loose union covering every streaming endpoint, so a handler for the table of contents stream could claim to receive a chapter event and nothing would object. Phase 2 replaced that with one union per stream in shared/events.ts, read off the route handlers that write them. The parser becomes generic and the six call sites name the union they expect. That the typecheck passes with no handler change is itself the useful result, since it confirms the shared unions match what the existing handlers already destructure. The parser also stops reporting a missing response body as a synthetic error event. Every caller already established the body exists before calling, and the api zone now raises a real ApiError for that case instead, which carries the status and the server's reason rather than a fixed string. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
These tests describe the exported functions client/api/books.ts and client/api/chapters.ts must provide, before either module exists. The commit is deliberately red because ./books and ./chapters cannot be imported yet, and the pre-commit hook is bypassed for that reason alone. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Gives the client a typed home for the book- and chapter-level HTTP calls, built on the http and sse primitives phase 3 already landed. No existing call site changes yet, later tasks migrate the components onto these. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Specifies client/api/creation.ts, client/api/covers.ts and client/api/import.ts before either module exists. This commit is deliberately red, confirmed by running the suite against the missing modules, and the pre-commit hook is bypassed for that reason alone. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Turns the creation wizard, cover art, and EPUB import call sites into three typed API modules, each built on the shared http and sse helpers rather than a direct fetch call. Request bodies are inferred from the existing shared/contracts schemas so the client and server cannot drift apart on shape. No call site is changed yet, a later task migrates the components onto these modules. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
These tests describe client/api/settings.ts, client/api/tasks.ts and client/api/chat.ts before any of the three modules exist, so the suite is deliberately red right now. The pre-commit hook is bypassed for that reason alone, since it would otherwise fail this commit on a typecheck for modules the next commit adds. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
These make the previous commit's tests pass. Each module is a thin layer over request, apiFetch and streamText from http.ts and sse.ts, so components can stop calling fetch and EventSource directly once a later task migrates them. No existing call site changes here. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
These three files pin the behavior of client/api/audiobook.ts, client/api/profile.ts and client/api/progress.ts before either exists, following this repo's practice of committing the test first. Every test here imports from a module that is not yet on disk, so tsc and the suite both fail right now. That is deliberate rather than a mistake, and it is the only reason this commit skips the pre-commit hook, which would otherwise block a commit that does not yet typecheck. The next commit adds the three modules and turns everything green. Coverage includes the method, URL and serialized body for every function about to exist, the trace header genuinely absent from getBookAudiobook's poll, installEngine resolving on a 409 rather than throwing while still throwing on a 500, an ApiError carrying the server's status and reason in each module, and streamInterview forwarding both value types including a profile that arrives with no trailing newline. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Adds client/api/audiobook.ts, client/api/profile.ts and client/api/progress.ts, the typed modules the tests committed just before this one already specify. All three go through request, apiFetch or the sse helpers already in this zone, so nothing here calls fetch directly. installEngine treats a 409 as success rather than an error, since a 409 from that endpoint means the engine is already installed or an install is already running, which is the outcome the caller wants. It reaches for apiFetch and an explicit status check instead of request for that reason. getBookAudiobook passes trace false, since it polls every four seconds while an audiobook generates and the trace header would otherwise turn a CORS-simple GET into a preflighted one. getProfile and saveProfile use a ProfileResponse type declared in this module rather than LearningProfile from shared/domain. That domain type's fields are style and identity, the shape the profile is persisted as, while the route folds those into a single aboutMe string before it answers, which is the field every existing caller reads. Widening the shared type to cover this would have made it lie about what the server persists, so the wire shape is named locally instead. No existing call site changes here. Every component still calls fetch directly, and migrating them to these modules is later work. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The endpoint modules were written in parallel and landed with two idioms for the same job. Some inferred their request bodies from the Zod schema the route validates against, and some restated the shape by hand. Restating it is how a client and a server drift apart, so they all infer now. The inference immediately paid for itself. Provider was typed as a plain string in the hand written versions and is really a union of three names, and three test fixtures were quietly passing a value the server would have rejected. The schemas are imported as types only, so they compile away and no validator is added to the browser bundle by this change. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Fifty eight functions across fourteen modules, reachable through one import path. Components import from '@client/api' and never from a module inside it, which is what makes the boundary greppable and gives a new endpoint exactly one obvious home. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The client had one flat components directory of sixty one files, one hooks directory, and a pages directory, so the only way to find out which parts of the reader belonged together was to read all of them. Each feature now owns its components, its hooks and its page. Sixty two files moved and nothing inside them changed. Git records every one as a rename, so history follows the code. Imports were rewritten with a TypeScript compiler API codemod rather than a text substitution, and the rewrite deliberately ran before the move, because a relative specifier can only be resolved against the directory the importing file is still in. Two files went instead of moving. PageTurnOverlay and usePageTurnGesture had no importers at all. What stayed put is as deliberate as what moved. The shadcn primitives, the two hooks used by more than one feature, and the helpers in lib are shared by everything, so putting them under any one feature would be a lie. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The library shows sixteen dialogs and two context menus and tracks them in twenty one separate pieces of component state. The bugs that shape allows are the ones where two dialogs are open at once, or where a dialog opens carrying the payload of the previous one. Holding one dialog at a time makes both unrepresentable, and these tests say so. The exiting field is the subtle part. Several of these dialogs stay mounted and fade out over a hundred milliseconds, so clearing the payload the instant they close makes them render empty on the way out. That is a visual regression, and it is the kind that only shows up on a real screen, so it is pinned here instead. This commit is deliberately red because the module under test does not exist yet. The pre-commit hook is bypassed here for that reason alone, and a later commit turns it green with hooks running normally. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Sixteen dialogs and two context menus in one tagged union, so the states that had no meaning stop being representable. Two dialogs open at once, and a dialog open carrying the payload of the one before it, were both reachable before and are not now. The exiting slot exists purely so an always-mounted dialog keeps rendering while it fades. Wiring it into the library view is a separate change. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
WizardModal and CreationView called fetch(apiUrl(...)) directly for every topic suggestion, table of contents stream, and first chapter stream. They now call the matching functions in @client/api, so these requests share the same trace id, retry, and error handling as the rest of the app instead of each hand-rolling res.ok checks. The create-book and revise-toc streams, and the streaming buffer they write into, move into a new useTocStream hook. The first-chapter stream and the phase transitions around it, including the phase-driven auto-advance into the reader, move into a new useChapterOneStream hook. CreationView keeps the resume-from-existing-book path and the phase state both hooks transition, since it is the one state a chapter-one phase and a table of contents phase can never both be in at once. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The voice modal, the settings dialog, and useChapterAudio called fetch(apiUrl(...)) directly for engine status, install, voice listing, and generation. They now call the matching functions in @client/api, and the hand-built voice preview and audiobook file URLs become voicePreviewUrl and audiobookFileUrl. installEngine already treats a 409 as success, so the settings dialog's own 409 check is gone rather than duplicated. A new useAudiobookEngine hook owns engine status, install, and voice listing, so the settings dialog and the voice modal share that fetching logic instead of each keeping its own copy. Loading stays imperative rather than effect-driven, since the settings dialog combines status and voices with the learning profile in one Promise.all on open, and this keeps that combination exact. useChapterAudio now polls at AUDIOBOOK_POLL_MS rather than a literal 4000, with its running-only polling and final-refresh-on-stop behavior unchanged. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
SettingsMenu, ModelAssignmentDialog's useProviderModels, ProfileDialog, SkillsPanel, ProfileUpdatePage and useInterviewChat all called fetch or apiUrl directly. They now call the matching function from @client/api, so this slice has one fewer thing in common with the fetch call it used to hand-roll. SettingsMenu fetched /api/profile twice on its own, once on mount and once whenever the profile or interview dialog closed, purely to learn whether About Me was non-empty for the settings badge. ProfileDialog and SkillsPanel each fetched the same endpoint again to seed their own edit state. A new hook, useLearningProfile, owns that GET and exposes a refresh function, so all three call it on their own trigger instead of each rolling its own request. The two debounce timers, API_KEY_DEBOUNCE_MS for the API key field and SKILL_SAVE_DEBOUNCE_MS for skill edits, now come from lib/constants instead of the literals 200 and 300 that used to live next to them. useInterviewChat used to read its own response stream by hand, including a trailing-buffer flush for the final line the profile arrives on with no newline after it. streamInterview already does exactly that, so the hand-rolled reader is gone and the hook just forwards streamInterview's values. The AbortController and the AbortError check are unchanged. SettingsMenu still needed to shrink to fit the 420 line ceiling once its fetch calls were this much shorter, so the API key dialog, the four tick-slider rows and the texture toggle moved out into their own small presentational components under features/settings/components. Nothing in their markup or class names changed, only where it lives. One dead branch came out along the way. ProfileUpdatePage read profileData.aboutMe ?? profileData.identity ?? '', but the profile route folds identity into the computed aboutMe string and never sends an identity field on its own, so that fallback could never fire. The typed ProfileResponse doesn't have an identity field at all, which is what surfaced it. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
ReviewProgressPage, SkillDetailPage, QuizReviewPage and useStreamingChat were the last four call sites in this slice still going through fetch or apiUrl directly. They now call getSkillProgress, getToc and streamChat from @client/api instead. ReviewProgressPage and SkillDetailPage each declared their own local SkillProgress and SkillsResponse interfaces, byte for byte the same duplicate pair in both files. getSkillProgress already returns the real SkillProgress type from shared/responses, so both local declarations are gone and the state is typed from that instead. useStreamingChat used to read its own response stream by hand with a plain reader loop. streamChat already does that with a streaming text decoder, which is the safer of the two across a multi-byte character split at a chunk boundary, so the hand-rolled loop is gone in favor of forwarding streamChat's decoded chunks. The AbortController, the AbortError check and the "Something went wrong. Please try again." fallback are unchanged. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
ReaderPage and useSectionNavigation called fetch(apiUrl(...)) directly at thirteen call sites. They now call the matching function from @client/api instead, so error handling, tracing, and the CORS retry are the same code path as the rest of the app. The three generation call sites (next chapter, regeneration, final quiz) each read body.message from a server that only ever answers with error, so their fallback strings never actually fired. ApiError now carries the server's real reason, and the dead fallbacks are gone. handleKeepGoing keeps its old behaviour precisely, distinguishing a non-2xx answer (silently degrade to feedback, same as an empty question list) from a genuine transport failure (show the toast), since ApiError now covers a case the old code handled without throwing. The mount effect's AbortController still aborts the resume stream, since streamGenerationResume accepts a signal for exactly that. getBook and getToc take no signal, so their two requests use a plain cancelled flag instead, matching the same StrictMode double-invoke guard by a different mechanism. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
ReaderPage carried the mount-time fetch-and-resume effect, the external generation poll, the next-chapter and regenerate streams, and all of the scroll handling inline. Four hooks now own that logic instead, each with a doc comment naming the constraint that makes it non-obvious: - useGenerationResume: the mount effect that fetches the book and TOC and reattaches to a generation already running server-side. - useExternalGenerationPoll: the interval that notices a chapter written by another window or the MCP server, now driven by EXTERNAL_CHAPTER_POLL_MS from lib/constants rather than a bare 5000. - useChapterGeneration: generate-next, regenerate, and the retry path. - useReaderScroll: smoothScrollBy, the streaming autoscroll effect, and the scroll-position detection, now driven by SMOOTH_SCROLL_MS and AT_BOTTOM_EPSILON_PX rather than bare 320 and 40. The keyboard-navigation effect that calls smoothScrollBy stays in ReaderPage but now reads PAGE_SCROLL_FRACTION, PAGE_SCROLL_MS, READER_LINE_HEIGHT, LINE_SCROLL_LINES and LINE_SCROLL_MS instead of 2/3, 420, 1.625, 5 and 240. This is pure code motion. Every effect keeps its exact dependency array and cleanup behaviour, including the AbortController/cancelled-flag guards against React StrictMode's double-invoke, and the buffered-content boundary that suppresses autoscroll when useGenerationResume reattaches to a stream mid-generation. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…yped api client Both dialogs called fetch(apiUrl(...)) directly for TOC/book metadata and for cover suggest/generate/upload/delete/patch. They now call getToc, getBook, suggestCoverPrompt, generateCover, uploadCover, deleteCover and updateBook from @client/api instead, so every request in this feature goes through the one typed door. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…pi calls and named constants BackgroundTasksFooter's cancel button called fetch(apiUrl(...)) directly; it now calls cancelTask from @client/api. Both files also had timing literals that belong in lib/constants.ts: the footer's 10s auto-dismiss becomes TASK_ROW_DISMISS_MS, and the toolbar's two post-mount focus delays become SEARCH_FOCUS_DELAY_MS. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
useBackgroundTasks and GenerateAllModal each opened their own EventSource against /api/tasks/stream by hand. Both now call subscribeToTaskEvents from @client/api, which owns the reconnect-on-drop timer and the TASK_STREAM_RECONNECT_MS delay in one place instead of two. GenerateAllModal's event handling is now typed against the shared TaskEvent union rather than whatever JSON.parse happened to return, which is why the two ad hoc "does this event belong to our task" checks became one small helper that switches on the discriminant. Sanctioned improvement: useBackgroundTasks previously took its callbacks straight into the subscribing effect's dependency array. App.tsx passes those callbacks as fresh inline closures on every render, so the stream was tearing down and reconnecting far more often than the task list actually changed. The callbacks now live behind a ref that the effect reads without depending on, so the effect's dependency array is just [dispatch] and the subscription opens once. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…e shared LibraryBook type
BookListRow, BookListView, SeriesView, SeriesStackCard and SortableSeriesCard
each declared their own ad hoc Book shape. All five now import LibraryBook
from @shared/responses, matching the type the /api/books list actually
returns instead of a hand-maintained guess at its fields.
SeriesView and SeriesStackCard also each built a cover URL with a raw
apiUrl(`/api/books/${id}/cover?v=...`) template literal; both now call
coverUrl() from @client/api. SeriesStackCard already had a local variable
named coverUrl, so the import is aliased to buildCoverUrl there to avoid
shadowing it.
Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
…Search hooks Four hooks pulled out of App.tsx, each covering one self-contained piece of what it was doing: - useBooks owns the library's book list: the initial fetch, a refetch on window focus, a one-second poll while anything is generating, and the optimistic merge that keeps a just-created book visible until the server reports it. The merge logic is carried over unchanged from the existing fetchBooks, only the transport and the GENERATING_POLL_MS constant are new. - useHealthCheck polls server reachability behind the offline banner, now on HEALTH_POLL_MS and through the api layer's checkHealth, which already collapses a thrown request into false. - useElectronApiKeys is the Electron IPC bootstrap that loads keys from secure storage into Redux and the server on startup. window.electronAPI stays optional throughout, unchanged from the original. - useLibrarySearch owns the deferred search query and the full-text content-search result set it produces. Not used yet — App.tsx starts calling these in a later commit once LibraryPage exists to hand their output to. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Two hooks beyond the four originally scoped, both needed to get App.tsx down to routing alone without changing what the app does. useBackgroundTaskEffects wires the background-task stream to the audiobook dialogs and the EPUB auto-download, plus the Electron quit-confirmation prompt. It has to be called from a component that never unmounts: a narrator install or an EPUB export can finish minutes after it starts, often while the user has moved on to reading a different book, and the task stream never replays an event once it's sent. That's also why the audiobook state (audiobookExists, the three audiobook dialogs, and pendingAudiobookForBookId) lives here rather than in LibraryPage, which unmounts whenever the view switches away from the library. useLibraryNavigation owns the view union and the handful of transitions that are more than a bare setView: resuming the last-viewed book on launch, opening a book from the grid, and the create-book handoff that bridges the wizard's cover-generation opt-in to landing in the reader. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
App.tsx was routing, the entire library UI, twenty-one dialog useStates, health polling, book polling, Electron IPC bootstrap, drag-and-drop reordering, search, series handling, EPUB import/export and audiobook orchestration all in one 2245-line file. The library UI — grid, list, series drill-in, drag-and-drop, toolbar, context menus and every dialog — moves to the new LibraryPage.tsx, which is deliberately exempt from this feature's usual 500-line ceiling since splitting it further is a later task's job. App.tsx now only calls useBooks, useHealthCheck, useElectronApiKeys, useBackgroundTaskEffects and useLibraryNavigation, and renders whichever top-level view is current. All 29 of its network call sites now go through @client/api instead of fetch(apiUrl(...)), the local Book interface is gone in favor of the shared LibraryBook type, and the two remaining apiUrl() cover-URL template literals are now coverUrl() calls. client/lib/api.ts was App.tsx's only importer (deleted in an earlier commit once nothing referenced it) — its searchBooks/previewEpub/ confirmImport functions and SearchResult/SearchResponse/EpubPreview types are superseded by searchBooks and previewEpubImport/ confirmEpubImport in @client/api, and by SearchResults/EpubPreview in @shared/responses. A handful of error-reporting call sites collapse two branches into one as a side effect of every api/* function throwing on a non-2xx response where the old raw fetch() calls only checked res.ok for the success path: rename/delete/reset/save-tags/save-series/rate-book previously showed one message for "server answered but rejected it" and a second, separate message only on a thrown network error. Both paths now share the network-error message, since there is no longer a way to tell them apart. This mirrors the same accepted tradeoff already made when the api/* modules were built. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The reader declared its own Book interface, one of seven near-copies across the client, and this is the last of them. LibraryBook is the right one rather than BookDetail, because the reader reads chaptersRead on mount and only the library list response carries that field. This change was written while the reader slice was being migrated but could not land then, since the library still built its prop from a local interface that was missing two fields the shared type requires. It compiles now that the library uses the shared type too, which is the useful part, the two sides disagreed and the compiler is what said so. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
ReaderPage.tsx still carried the chapter quiz, final quiz, feedback, and rating handlers as inline useCallback soup. Moving them into useReaderQuiz and useChapterCompletion continues the hook-extraction pass that useGenerationResume, useExternalGenerationPoll, useChapterGeneration, and useReaderScroll started, and keeps the non-2xx-vs-transport-failure distinction in handleKeepGoing intact along with its comment. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
ReaderPage.tsx's ~495-line return block rendered every phase inline, one giant JSX tree with no seams. Splitting it into ChapterRail (the tab strip), TableOfContents, ChapterReader (the reading phase's article body), GenerationPanel (generating/generation-error), and EndOfBookFlow (final-quiz/rating/complete) gives each phase its own file. ReaderBody gathers the phase-driven content area, its overlay chrome, and the chat panel into the one region below the chapter rail, which is what brings ReaderPage.tsx itself under its 400-line target without changing what it owns. No state moved: every value rendered here is still declared exactly where ReaderPage.tsx already had it. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
WizardModal was 920 lines and 521 of them were literal data, one list of cover art directions and one list of topics for the surprise me button. Neither is logic, and they change for entirely different reasons than the wizard does, so reading the component meant scrolling past five hundred strings to reach the next line of code. The component is 399 lines now. The data module is longer than the size limit this phase applies to components, deliberately, because a rule that forces a list of five hundred topic strings to be split across files would be the rule being wrong rather than the file. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
LibraryPage tracked its sixteen dialogs and two context menus as twenty two independent useState calls, which made two dialogs open at once, or a dialog showing the previous dialog's payload, both representable even though neither has any meaning in the UI. This replaces them with the already-tested dialog-machine reducer, using isOpen for open checks and payloadOf for a dialog's data, including while it fades out on close. The mutating flag stays ordinary useState with a comment explaining why, since it is shared by rename, delete, reset and rate rather than owned by any single dialog. Dialog markup itself is unchanged and still lives inline in this file. Moving it into dialogs/ follows in the next commit. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
LibraryPage rendered every dialog and both context menus inline, which was most of why the file ran past sixteen hundred lines. Each of the five hand-rolled dialogs, rename, rename series, delete, reset and rate, and both context menus now have their own component in dialogs/, driven by the dialog reducer's isOpen and payloadOf rather than local state. The dialogs that already delegated to their own component, edit tags, set series, overview, cover, generate all and import, are unchanged apart from reading the reducer instead of the old per-dialog state, and are now rendered through a new LibraryDialogs bundle rather than twice inline, since LibraryPage has two return branches that both show them. The three audiobook modals are untouched, since that state belongs to App.tsx's background task hook and was never part of this reducer. AudiobookEffects moves out of LibraryPage and is exported from useBackgroundTaskEffects instead, since the new BookContextMenu and LibraryDialogs components need the same type LibraryPage does. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The copy button's reset delay and the toast that explains a copied MCP command were the only two magic durations the slice migrations did not already reach. Every timing value in the client now comes from lib/constants.ts. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The client held eighty four scattered fetch calls at the start of this phase and holds none now. A convention would drift back. This rule is what makes that stick, and it was verified in both directions, clean on the tree as it stands and failing on a fetch added to a hook on purpose. EventSource is covered for the same reason. The background task stream used to be constructed in two separate components, each with its own idea of when to reconnect, and reconnection now lives in exactly one place. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
Three of the file-size numbers in the phase gate were not reachable by the work the plan assigned to reach them, and each one took an implementer to discover. The reader's four named hooks leave 857 lines because roughly 495 of them are a render block no hook touches, so two more hooks and a render split were needed to reach 362. The library's dialog extraction cannot reach its target either, since roughly 760 of its remaining lines are drag and drop, filtering, sorting and grid markup that the plan never assigns to anyone. The wizard breached the limit at 920 lines without appearing in any task at all, and turned out to be mostly literal data. Also recorded, the reason the audiobook dialogs stay in the application shell rather than the library view, and the one dialog whose exit animation changed. A plan that keeps its wrong numbers teaches the next phase to under budget. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
41 buttons across the client rendered an icon and nothing else, with no aria-label and no title, so a screen reader announced each one as just button. Each now gets an aria-label naming what its onClick actually does rather than what its icon looks like, close buttons say what they close, back buttons say where they go, and remove and cancel actions name the item they act on. A handful of the flagged lines already rendered real visible text at runtime, a suggestion list item, a chapter count, a dynamic save label, but used single quoted strings, template literals, or a passed through children prop, none of which the survey script's double quote based text check could see. Those get an aria-label that mirrors the same text so the accessible name matches what is already on screen instead of inventing new copy. Four toggles whose on and off state was conveyed only by background colour, the two boolean preference switches, the two filter chips in the library toolbar, and the texture control, also get aria-pressed so that state is exposed to assistive tech too. No className, layout, colour, size, element, or visible text changed. Verified with a diff of every static className in client against the pre-change tree, and confirmed by hand that no cn() call or template-literal className was touched. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The saving chapter and creating quiz messages in GenerationPanel change on their own while the user is just sitting there watching the chapter stream in, with nothing to alert a screen reader that the text updated. The wrapping div now carries role status and aria-live polite so that change gets announced. The toaster in client/app/main.tsx needed no change. Sonner 2.0.7 already wraps every toast in a section with aria-live polite and gives its close button a default aria-label of Close toast, confirmed by reading node_modules/sonner/dist/index.mjs directly rather than assuming, so toasts were already announced before this branch. No progress counter or other per-second value was given a live region, per the instruction that doing so is worse than none. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
The revise panel's send button had no accessible name, and the survey that found the other forty one missed it. The check asked whether an expression in child position contained a double quote, which a ternary choosing between two icons always does, because both branches carry class names. Every button of that shape was reported as already named. The survey now asks what an expression can actually render, recursing through ternaries into their branches, and it finds this one. Claude-Session: https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5
This was referenced Jul 21, 2026
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.
Objective
Turn the client into feature slices with one typed API layer, without changing a single thing the user sees.
At the start, 84 raw
fetchcalls were scattered across 23 files,App.tsxwas 2245 lines,ReaderPage.tsxwas 1083, seven files each declared their owninterface Book, and every poll interval and animation duration was a bare number at its call site.Gate evidence
pnpm testpnpm exec tsc --noEmitpnpm lintrg 'fetch\(|new EventSource' client --glob '!client/api/**'App.tsxReaderPage.tsxSettingsMenu.tsxLibraryPage.tsxLibraryPage.tsx1190, pluswizard-suggestions.ts535, a pure data module, see deviationsinterface Bookinclient/electron:previewPersisted state, measured rather than asserted
The store was one 604-line file holding five slices, four persistence transforms and the storage adapter. Splitting it is the change in this PR that could destroy user data rather than misplace a pixel.
shared/book-status.tsaside, master'sstore.tswas extracted as a reference module with two behaviour-neutral edits, exporting the config and guarding awindowread, and both pipelines were run over the realpersist:tutorpayload from a live install:stablemeans the round trip is a fixed point, so loading and re-saving real state changes nothing. 16 unit tests pin the contract, including the direction the transform list folds, which is left to right on the way to storage and right to left on the way back. Both orders look equally plausible in source and only one is correct.Zero visual change, measured against a control
Screenshots were captured over CDP from an invisible Chrome, against a throwaway data directory, comparing this branch to a build of master.
The library's difference from master is smaller than the branch's own run to run variance, which is the strongest form this measurement takes when one of the surfaces is deliberately random.
The library and settings numbers sit at the same order as the noise floor the control establishes, because
NoiseOverlayregenerates its grain withMath.random()on every mount and renders on the library shell but not the reader. The reader has no overlay, which is why it gets to be exact, and it stayed exact after its entire 495-line render block was split across eight components.Rehydration, proven with a control
The headless harness runs in web mode, where redux-persist falls back to
localStorageand a fresh CDP profile has none, so it cannot exercise the Electron IPC storage path at all. That path was tested directly instead, by running Electron against two data directories that differ only inredux-state.json:redux-state.jsonSame build, same book, same server data. That is the IPC storage handler, redux-persist, the split store and the reader, verified end to end.
Sanctioned behaviour changes
Five, all previously agreed, none on a success path.
tracedFetchexisted but had exactly one call site out of 84, so routing everything through the typed client genuinely changes the wire. POSTs already sentContent-Type: application/jsonand were already preflighted. GETs would newly preflight, so the four hot polls passtrace: false, namely the library poll at 1s, health at 10s, the audiobook poll at 4s and the chapter status poll. The retry fires only whenfetchitself threw, which on loopback means a refused connection rather than a dropped in-flight request, so it cannot duplicate a side effect the server already performed.{ error }and none answers{ message }, yet roughly five call sites readbody?.message || 'Generation failed'and so always showed their own generic fallback.ApiErrorreadsmessage, thenerror, then a caller fallback. Affected paths are generate-next, regenerate, generation resume, final quiz, and cover upload and delete.useBackgroundTaskshad inline callbacks fromApp.tsxin its effect dependencies, so theEventSourcetore down and rebuilt on unrelated renders. Reconnection now lives inside the subscription inapi/sse.tsand cannot be disturbed by a re-render.installEnginetreats HTTP 409 as success. This is an idempotency fix rather than a swallowed error.POST /api/audiobook/installanswers 409 in exactly two cases, read offserver/routes/audiobook.ts, eitherAudiobook engine already installedwhen it is present and no force flag was sent, orAudiobook install already in progresswhen a task is already running. Both mean the narrator either is there or is on its way, which is precisely what the caller asked for, so an error toast was the wrong response to both. The audiobook settings dialog already treated 409 this way and the library did not. They agree now. Any other failure status still raises.if (!preview) return null, so it used to cut instantly while its siblings faded over 100ms. Driving its content frompayloadOfmakes it consistent. Special casing one dialog to stay fade-less would be the worse outcome, since the whole point of the dialog machine is that these sixteen stop behaving differently from each other for no reason.What changed structurally
client/api/— 14 modules, 58 functions behind one@client/apientry point. Request bodies are inferred from the Zod schemas the server validates against, so a body the client sends cannot drift from what the route accepts. That inference immediately caughtprovidertyped as a plainstringwhere it is really a three-name union, and three test fixtures passing a value the server would have rejected.client/features/— ten slices, 62 files moved, every one recorded by git as a rename so history follows.PageTurnOverlayandusePageTurnGesturedeleted, zero importers.client/store/— one file per slice pluspersist.ts, withindex.tsre-exporting the exact previous surface so not one of about 40 import lines changed.client/lib/constants.tsnow holds every timing value in the client.fetchadded to a hook deliberately.Deviations from the plan
Three of the plan's size targets were not reachable by the work the plan assigned to reach them, and each took an implementer to discover. All three are now recorded in
docs/plans/refactor/phase-3.mdso the next phase budgets honestly.wizard-suggestions.ts. That module is over 500 lines on purpose, since a rule that splits a list of topic strings across files is the rule being wrong.Also, S4 needed two hooks beyond the four specified. The background task stream must be subscribed from a component that never unmounts, because a narrator install or an EPUB export can finish minutes later while the reader is on another screen and the stream never replays a missed event. That pins the audiobook dialog cluster to
App.tsx.Notes for reviewers
client/api/profile.tsdeclares a localProfileResponserather than reusingLearningProfile. The shared type hasstyleandidentity, which is the on-disk shape, while the route folds those into a computedaboutMebefore answering.shared/responses.tsshould own the wire shape, which is a Phase 2 change.shared/book-status.ts, which value-imports it to build a schema that six one-line predicates do not need. Pre-existing and assigned elsewhere. Every module inclient/api/imports zod asimport typeonly, verified against the built bundle.https://claude.ai/code/session_01FSkESkJTRpt3Ye2cM7c5n5