Settings split pass 10: extract AI Summaries markup — section extraction complete (SettingsView 4.3k, -62%) - #157
Merged
Merged
Conversation
Stage 1 of the last section. The AI Summaries card's state machine — provider/base-URL/model/key form state, the connection test, model discovery against a custom endpoint, the sample-policy dry run, the three per-phase request timeouts, and the debounced blob auto-save — moves to lib/use-ai-settings.ts. That is ~480 lines spread across six regions of SettingsView. The JSX is untouched: hook fields are renamed back on destructure, so all nine diff hunks land above the first `return (` at line 3037. The 791-line markup move is the next pass. Inputs are five: showToast, three translators used by the auto-save toast labels, the task-center handle, and loadSettings. Nothing else crossed the boundary. Three more candidates the identifier sweep suggested — `base`, `schedule`, `toast` — turned out to be matches inside comments. Several bindings leak deliberately and are returned rather than kept private: `aiProvider` (the timeouts panel disables its inputs when AI is off), `saveAiSettings` + `debugLogging` (the Developer Options debug toggle round-trips through this same blob), and the three `aiTimeout*` values with their auto-savers. `hydrate()` is the seam for the shared settings loader: it reads every settings key in one request, so it hands the AI slice over in a single call rather than this hook mounting a duplicate fetch. Four types move to ./settings/types.ts — StoredAiSettings and AiSamplePolicyResult, which both sides now need. The hook call sits below `showToast` and `loadSettings` rather than where the state block was, for the same TDZ reason as use-import-history: both are const arrows. SettingsView 5,643 -> 5,204. Verified with tsc, biome, 441 unit tests and the full Playwright suite (45 passed). Note for anyone verifying locally: `pnpm install` currently fails on this repo with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION for ultracite@7.10.0, which arrived via the Renovate bump in #154 and is newer than pnpm's release-age cutoff. It is a local policy, not a CI one (#154 merged green), and it resolves itself as the package ages. Until then, `--config.minimumReleaseAge=0` on install and `--config.verifyDepsBeforeRun=false` on run get you moving. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stage 2, and the last section. The 791-line AI Summaries card moves to AiSummariesSection, which takes a single prop: the whole `useAiSettings` return value as `ai`. The hook cannot be called inside the section — Developer Options needs the same blob for its debug-logging toggle and the timeouts panel reads `aiProvider`, so two call sites would mean two independent copies of the state. One object prop beats thirty-five. Its two sub-flags (`provider_selector`, `summarize_on_import`) are resolved in the section; SettingsView keeps the section-level `flag.settings.ai.enabled` gate, per the rule in ./settings/README.md. `hydrate()` now actually exists. The previous commit's docstring described it as the seam for the shared settings loader, but it was never implemented — the loader still called eight setters directly, which is why they were leaking out of the hook. It is implemented here and the loader calls it, so the destructure at the call site drops from 57 bindings to 14. Also removed: two params I threaded into the hook out of habit (`showToast`, `tToast`) that nothing in the moved code ever used, and nine translator declarations left stranded in SettingsView. Verified with the DOM diff, and this one needed a configured provider to be worth running: the card's `disabled` state is a fraction of its markup, so comparing that would have proved almost nothing — the same trap as the empty import list in the previous pass. With a custom provider, base URL and model set, both sides render identical at 7,491 chars with zero console errors. Also: tsc, biome, 441 unit tests, full Playwright suite 45 passed, and all 14 translation namespaces resolved against locales/en.json. SettingsView 5,204 -> 4,322. Every section is now extracted: 11,460 -> 4,322 across the phase, a 62% reduction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
Pass 10 — AI Summaries markup. Section extraction complete.
SettingsView.tsx5,204 → 4,322 (−882). Every section is now out of the file.What moved
The 791-line card becomes
AiSummariesSection, taking one prop — the wholeuseAiSettingsreturn value asai. The hook can't be called inside the section: Developer Options needs the same blob for its debug-logging toggle and the timeouts panel readsaiProvider, so two call sites would mean two independent copies of the state.Its two sub-flags are resolved in the section; SettingsView keeps the section-level
flag.settings.ai.enabledgate.hydrate()now actually existsWorth calling out because I got this wrong in #156. That PR's docstring described
hydrate()as "the seam for the shared settings loader" — but it was never implemented. The loader still called eight setters directly, which is exactly why they were leaking out of the hook.I noticed when trimming the destructure and finding those setters still in use. It's implemented here and the loader calls it, so the call site drops from 57 bindings to 14.
Two other things cleaned up:
showToastandtToast, which I threaded into the hook out of habit in #156 and which nothing in the moved code ever used; and nine translator declarations left stranded in SettingsView.Verification
The DOM diff needed a configured provider to be worth running. The card's
disabledstate is a fraction of its markup — comparing that would have proved almost nothing, the same trap as the empty import list in #153. With a custom provider, base URL and model set, both sides render identical at 7,491 chars, zero console errors.Plus tsc, biome, 441 unit tests, full Playwright 45 passed, and all 14 translation namespaces resolved against
locales/en.json.A note on the local toolchain:
pnpm typecheckcurrently exits 0 without runningtscbecause the deps gate aborts first. Everything above was run against the binaries directly (./node_modules/.bin/tsc,biome,next,playwright) so the results are real.Phase 3 — section extraction complete
main)−62%, across 21 section components and 3 hooks, every pass verified by rendering and diffing rather than asserted.
What's left in Phase 3 is no longer extraction: the route split from
SECTION_GROUPSwith hash redirects, theOnboardWizardsplit, moving CSS out of the 32k-lineglobals.css, and the 224-key flag audit.🤖 Generated with Claude Code