Skip to content

refactor(config): unify setting declarations and built-in handler registration - #171

Merged
Lucas1479 merged 18 commits into
mainfrom
codex/configuration-convergence
Oct 10, 2026
Merged

Lucas1479 merged 18 commits into
mainfrom
codex/configuration-convergence

Conversation

@Lucas1479

@Lucas1479 Lucas1479 commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

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

  • Routine fix, documentation, test, maintenance, or presentation-only UI
  • Product-semantic or public-contract change discussed in the linked Issue
  • Isolated, default-off experiment

Owning layer: configuration declarations/parsing, desktop persistence and presentation, and built-in application composition.

User-visible effects:

  • Settings show the authoritative planned startup value and environment locks, with unresolved offline inputs shown as unknown.
  • Live settings retain pending revisions until a matching application acknowledgment.
  • Ordinary fields and built-in TTS backends can be added through their declaration and implementation; new handler factories register without editing app.py.
  • Existing desktop keys remain covered through canonical declarations, documented aliases or explicit structured owners. This is not a third-party plugin loader.

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:

  1. Saved legacy names could lose to canonical dotenv keys; Provider summaries could display undefined for a legacy-only saved selection.
  2. Changing a graphics preset could substitute its automatic sampling default for an explicit dotenv sampling value.
  3. Voice card states disagreed with their controls for 1/yes inputs and pending clears.
  4. Enabling vision offline could replace a previously saved watching mode with on_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.ts and startupSettings.ts source precedence; computed versus effective values; live-setting acknowledgments; server/handlers/composition.py and 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 by 96921cf, with the exact evidence and limitations recorded in 41d4537.

  • Empty enum examples now parse as empty strings, and credential examples remain commented out. Tests use python-dotenv, import settings from a copied template with only one synthetic key, and check desktop credential readiness.
  • Desktop Wake/AEC defaults live in the declarations and feed both launch arguments and offline snapshots. Process, saved-user and dotenv overrides retain priority. Removing the redundant AEC delay injection also fixes the pre-existing suppression of device-class calibration.
  • Generated, explicitly typed Python bindings replace dynamic globals injection. All 50 Ruff findings are fixed without disabling undefined-name checks. The pure microphone settings test uses an isolated audio stub.
  • The missing persona heading and three additional shared Chinese labels are restored and checked through the actual translation provider. Qwen's historical on spelling again enforces required CUDA; optional CPU fallback remains covered.
  • A single recursive package-data rule replaces per-folder entries. An actual wheel-build test adds a previously unknown nested catalog folder and imports it from the extracted wheel.

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

ae0f8e1 retains 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

  • Full Python isolated runner, four shards: 5,457 collected. First pass: 5,437 passed, 5 skipped, 5 expected failures, 10 failures in four fixture files. After repairs, all 78 tests in those four files passed. This is not reported as a single all-green initial run.
  • Clean committed snapshot: 373 Electron tests passed, production build/type checks passed, plus 106 Python configuration/visual/repair tests passed. Unrelated working-tree changes were excluded from this snapshot.
  • Seven isolated synthetic startup profiles matched the pre-refactor public settings values. All 149 previous desktop keys are covered by declarations/aliases/explicit owners.
  • Real offline Electron Settings smoke, shipping model-less Electron/backend smoke, and authenticated new-handler WebSocket/shutdown smoke passed.
  • Wheel catalog resources and source-release selection verified; 13 release-tooling tests passed.

Commands:

python -X utf8 tools/run_tests.py --shard-count 4 --shard-index N
# N = 0, 1, 2, 3
python -m pytest -q tests/test_character_rag.py tests/test_chat_ingress_lifecycle.py tests/test_gpt_sovits_sidecar_backend.py tests/test_vts_worker_lifecycle.py
cd electron
npm test
npm run build
electron scripts/smoke-startup-settings.cjs
cd ..
python -X utf8 tools/smoke_electron_model_less.py
python tools/smoke_builtin_handler_catalog.py
  • Relevant Python tests pass after the documented repairs
  • CPU/model-less baseline remains supported
  • Electron production build passes
  • Documentation/examples and screenshot evidence updated
  • Dependency audit: no dependency changes in this branch.
  • Not exercised: real GPU inference, paid remote APIs or a full installer release. The existing bundle-size warning remains.

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.

Before (c07e009) After (d0b2944)
Before After

Final check

  • One coherent configuration/composition problem; unrelated local changes excluded
  • No new extension execution authority or generic dependency resolver
  • No credentials, personal runtime data, model weights or voice assets included
  • Existing third-party notices preserved

@Lucas1479
Lucas1479 marked this pull request as ready for review October 10, 2026 07:54
@Lucas1479
Lucas1479 merged commit 19550be into main Oct 10, 2026
17 checks passed
@Lucas1479
Lucas1479 deleted the codex/configuration-convergence branch October 10, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant