Repository navigation
refactor(config): unify setting declarations and built-in handler registration - #171
Merged
Merged
Conversation
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.
What and why
Adding a setting previously required synchronized edits to Python defaults, desktop allowlists, status descriptors, Settings controls and translations. This PR makes packaged JSON declarations the shared source for 145 fields in 42 groups, and moves built-in request-handler construction/registration to an explicit factory table.
The desktop can configure these fields without a running backend. Existing setting names, stored settings version, process/user/dotenv precedence and secret-storage boundaries remain in place. Computed defaults, live application, structured ACP/MCP records and runtime readiness stay with their existing owners.
Linked Issue: N/A for this internal consolidation. Scope, acceptance criteria and the full audit are recorded in the implementation plan. This remains a draft for independent review.
Change class
Owning layer: configuration declarations/parsing, desktop persistence and presentation, and built-in application composition.
User-visible effects:
app.py.Compatibility: no persisted-format migration. Protocol-specific validation and computed defaults remain ordinary owner code. Credentials use the shared secret-normalization boundary and never return plaintext to the renderer.
Audit fixes
The additional audit found and fixed:
undefinedfor a legacy-only saved selection.1/yesinputs and pending clears.watchingmode withon_demand.Regression coverage was added. Older AST shutdown fixtures now bind the real extracted voice cleanup function; the RAG test checks its declared disclosure; a synthesis fixture explicitly isolates itself from a developer's experimental emotion setting.
Review focus:
desktopSettings.tsandstartupSettings.tssource precedence; computed versus effective values; live-setting acknowledgments;server/handlers/composition.pyand shutdown ownership. Read declarations and handwritten adapters before the generated TypeScript.Presentation preservation
Existing UI copy and choices are preserved during the metadata migration: both theme explanations and translations, TTS setup guidance, engine-specific executable labels, VN default-selection hints, dropdown ordering and Vision icons. Existing backend diagnostic descriptions remain available separately. The 24 unnecessarily reworded Chinese translations were restored.
Accepted configuration values are distinguished from offered UI choices: the existing Vision and AUIP forms keep their original choices while previously supported saved/environment inputs remain valid. The comparison covers 120 fields and 64 existing explanations across local-engine and Codex transport/authentication variants.
Previous verification (d0b2944): 373 Electron tests and 71 Python configuration/status tests pass in the isolated snapshot; production build and real offline GUI smoke pass. The smoke asserts both theme explanations, and the screenshot below shows them restored.
Claude review follow-up
All ten findings were checked against
d0b2944. Item 6 (VN default-provider wording) was already fixed there; the other nine are addressed by96921cf, with the exact evidence and limitations recorded in41d4537.onspelling again enforces required CUDA; optional CPU fallback remains covered.Verification of the clean committed snapshot: 141 Python tests, 376 Electron tests, Ruff, architecture-view checks, type checks, production build and real offline Settings GUI smoke passed. The working tree also passed the real model-less Electron/backend startup, authentication, navigation and shutdown smoke. Eight voice tests passed with optional audio/model imports explicitly unavailable; this simulates the dependency boundary rather than claiming a fresh full core installation.
The earlier audit missed the template-copy, static-check and optional-dependency boundaries; its test counts did not establish these contracts. The full Python suite was not rerun for this follow-up. All 17 remote checks for 41d4537 subsequently passed, including the three Python shards and cpu-model-less; this does not assert success for later commits. The PR remains a draft for independent review.
Second review follow-up
ae0f8e1retains the existing device-class delay selection and explicit override priority. It adds no fixed desktop delay override and makes no new claim of acoustic validation across devices. Startup no longer labels the configured 280 ms fallback as the effective delay; the AEC owner logs the selected delay, device class and reason after successfully configuring the native processor. Device switches continue to log the newly applied delay.Both README setup paths now explicitly say to uncomment the DeepSeek key line and replace its placeholder, while leaving unused providers commented out. The copied-template regression test edits that existing line instead of appending a second assignment.
Validation: 48 related Python tests passed; all 12 voice snapshot tests also passed with optional audio/model imports unavailable. Ruff, generated-output consistency, and real isolated backend startup/shutdown smoke passed. The new commit's remote CI must complete separately. No internal raw logs or private evidence are attached.
Evidence
Commands:
Settings before / after
Same isolated light-theme configuration with the original theme explanations retained; the after image also exercises environment locking. The baseline harness was supplied its missing empty-backend IPC fixture.
c07e009)d0b2944)Final check