Settings split pass 9: hoist AI settings logic into a hook (SettingsView 5.2k) - #156
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>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| aiTimeoutMergeMs, | ||
| setAiTimeoutMergeMs, | ||
| aiTestStatus, | ||
| setAiTestStatus, |
| aiTestStatus, | ||
| setAiTestStatus, | ||
| aiTestMessage, | ||
| setAiTestMessage, |
| aiTestMessage, | ||
| setAiTestMessage, | ||
| aiTestLatency, | ||
| setAiTestLatency, |
| aiTestLatency, | ||
| setAiTestLatency, | ||
| aiSampleStatus, | ||
| setAiSampleStatus, |
| aiSampleStatus, | ||
| setAiSampleStatus, | ||
| aiSampleMessage, | ||
| setAiSampleMessage, |
| aiSampleMessage, | ||
| setAiSampleMessage, | ||
| aiSampleResult, | ||
| setAiSampleResult, |
| aiSampleResult, | ||
| setAiSampleResult, | ||
| discoveredModels, | ||
| setDiscoveredModels, |
| discoveredModels, | ||
| setDiscoveredModels, | ||
| modelsStatus, | ||
| setModelsStatus, |
| modelsStatus, | ||
| setModelsStatus, | ||
| modelsError, | ||
| setModelsError, |
| import { useSettingsAutoSave } from "@/lib/use-settings-auto-save"; | ||
| import { fmtDuration } from "@/app/components/settings/format"; | ||
| import type { useTaskCenter } from "@/app/components/TaskCenter"; | ||
| import { pushSettingsToast } from "@/app/components/SettingsAutoSaveToast"; |
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 9 — AI Summaries, stage 1 (logic)
SettingsView.tsx5,643 → 5,204 (−439), with zero JSX changes. Branched from a cleanmain— passes 6–8 are all merged.What moved
The AI Summaries card's whole 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. That's ~480 lines spread across six regions of SettingsView.
The JSX is deliberately untouched — hook fields are renamed back on destructure, so all nine diff hunks land above the first
return (at line 3037. This reviews as "did the logic move intact?" without a markup diff. 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, andloadSettings. Three more that the identifier sweep suggested —base,schedule,toast— turned out to be matches inside comments, checked before threading them.Deliberate leaks, returned rather than kept private:
aiProvider— the timeouts panel disables its inputs when AI is offsaveAiSettings+debugLogging— the Developer Options debug toggle round-trips through this same blobaiTimeout*values and their auto-savers, which the timeouts panel owns visually but which belong to this blobhydrate()is the seam for the shared settings loader: it reads every settings key in one request, so it hands the AI slice over in one call rather than this hook mounting a duplicate fetch.StoredAiSettingsandAiSamplePolicyResultmove tosettings/types.ts— both sides need them now.The hook call sits below
showToastandloadSettingsrather than where the state block was; both are const arrows, so the original position was a TDZ error. Same fix asuse-import-history.Verification
tsc, biome, 441 unit tests, full Playwright suite 45 passed.
main(not caused by this PR)pnpm installfails with:It arrived via the Renovate bump in #154. CI is unaffected — #154 merged green, so the policy is local, not in the repo config. It resolves itself as the package ages.
Worth knowing because it fails confusingly:
pnpm typecheckexits 0 while never runningtsc, so a broken tree can look clean. Worse, the aborted install had already removed 83 packages fromnode_modules, which then surfaced as an unrelated "cannot find@axe-core/playwright" error.Until it ages out:
and
--config.verifyDepsBeforeRun=falseonpnpm test/build/test:e2e.Phase 3 running total
main)−55%. Stage 2 moves the remaining 791-line AI markup, and that completes the section extraction.
🤖 Generated with Claude Code