fix: guard kokoro-local provider init timing on direct navigation - #2273
fix: guard kokoro-local provider init timing on direct navigation#2273stz-2044 wants to merge 10 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 582212637e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!providerStore.getProviderConfig(providerId)) { | ||
| await providerStore.ensureProvider(providerId, providerId, {}) |
There was a problem hiding this comment.
When a user opens this route directly on WebGPU hardware with fp16 support and no saved kokoro-local config, this new path creates an empty config instead of the provider's default config. The fallback below then persists getDefaultKokoroModel(hasWebGPU.value) without fp16Supported.value, so the saved model becomes fp32-webgpu even though the provider schema and computed getter would choose fp16-webgpu. Seed the provider with its default config or pass fp16Supported.value when filling config.model.
Useful? React with 👍 / 👎.
|
Thanks for the review! Fixed in |
lietblue
added
bug
When navigating directly to the speech settings page, the provider may
not be registered from the catalog yet, so getProviderConfig() returns
undefined and reading config.model throws. Also, the model combobox can
write back v-model before onMounted runs (provider registration is an
async synced action).
- await ensureProvider() when the config is missing, then re-read it
- guard set()/watch() against missing config
- fall back to {} where a read is unavoidable
Verified locally: direct navigation no longer throws; model selection
persists after registration.
…t navigation
When opening this route directly with no saved config, ensureProvider() was
seeded with an empty object, dropping the schema defaults. The fallback then
persisted getDefaultKokoroModel(hasWebGPU) without fp16Supported, so fp16-
capable hardware saved fp32-webgpu while the computed getter showed fp16-webgpu.
Seed with { model: getDefaultKokoroModel(hasWebGPU, fp16Supported), voiceId: '' }
matching createProviderConfig defaults, and pass fp16Supported in the fallback
too so the persisted default always matches what the getter would choose.
20e8a83 to
afddffd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afddffddff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await providerStore.ensureProvider(providerId, providerId, { | ||
| model: getDefaultKokoroModel(hasWebGPU.value, fp16Supported.value), | ||
| voiceId: '', | ||
| }) |
There was a problem hiding this comment.
When this direct-navigation path creates Kokoro config, it omits the baseUrl: '' field that getDefaultProviderConfig() adds for providers without a base URL. shouldListProvider() compares the saved object to that full default, so this partial config stays dirty and can make a passive page open list and revalidate Kokoro as configured. Seed from the provider defaults, or include the same default fields here.
Useful? React with 👍 / 👎.
| // The combobox can write back the v-model before onMounted runs, at which | ||
| // point the provider may not be registered yet and synced actions are still | ||
| // async. Skip the write in that window; onMounted persists the default model | ||
| // right after registration, and later user selections always hit a config. |
There was a problem hiding this comment.
These lines document an initialization-timing workaround, but the repo requires every workaround comment to use the // NOTICE: block with root cause and removal condition. Without that format, later readers lose the required context for when this skipped write is safe.
AGENTS.md reference: AGENTS.md:L244-L250
Useful? React with 👍 / 👎.
- Seed provider with full default config shape (baseUrl: '') so shouldListProvider() comparison stays clean on direct navigation - Convert the init-timing workaround comment to the repo's // NOTICE: format with root cause and removal condition Addresses PR moeru-ai#2273 codex review suggestions.
|
Both suggestions addressed in
Thanks for the review! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1d977bbf4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // of an empty object, so fp16-capable hardware keeps its fp16-webgpu default. | ||
| if (!providerStore.getProviderConfig(providerId)) { | ||
| await providerStore.ensureProvider(providerId, providerId, { | ||
| model: getDefaultKokoroModel(hasWebGPU.value, fp16Supported.value), |
There was a problem hiding this comment.
When a user opens this page directly on an FP16-capable WebGPU browser with no saved kokoro-local config, this branch still uses fp16Supported.value before async WebGPU detection runs. Fresh evidence: repo-wide search found detectWebGPU() only in useInferencePreload.triggerPreload(), which root App.vue calls from onMounted, after this route has mounted. As a result, the cached capability is null, FP16 stays false, and this line persists fp32-webgpu while the fp16-webgpu option is absent until a reload. Await detectWebGPU() before fetching models and seeding the config, or delay persistence until the capability cache exists.
Useful? React with 👍 / 👎.
Direct navigation can mount the kokoro-local settings page before the app-level preload (useInferencePreload -> detectWebGPU, from App.vue onMounted) populates the capability cache. Reading the sync cache then yields null, fp16Supported falls back to false, and fp16-capable hardware gets seeded with fp32-webgpu until a reload. Await detectWebGPU() before fetching models and seeding the config. detectWebGPU() deduplicates concurrent calls, so this shares the same promise as the app preload when both race.
|
Thanks for the catch — the race is real. Direct navigation mounts this page before the app-level preload ( Fixed in Verified: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e608514bc3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // of an empty object, so fp16-capable hardware keeps its fp16-webgpu default. | ||
| if (!providerStore.getProviderConfig(providerId)) { | ||
| await providerStore.ensureProvider(providerId, providerId, { | ||
| model: getDefaultKokoroModel(hasWebGPU.value, fp16Supported.value), |
There was a problem hiding this comment.
When direct navigation happens on fp16-capable WebGPU hardware, this saves fp16-webgpu, but useProviderStore() is created at module setup before this awaited detection runs. Its providerMetadata default for Kokoro was already selected from getCachedWebGPUCapabilities() === null, so it remains fp32-webgpu; shouldListProvider() then treats this passive seed as a user-modified config and lists Kokoro as dirty/unconfigured. Use the same default source for seeding and dirty comparison, or refresh the provider default after detectWebGPU().
Useful? React with 👍 / 👎.
The provider metadata (including Kokoro's capability-dependent default model) is selected at store setup, before WebGPU capability detection completes. Seeding a capability-accurate default after awaiting detectWebGPU() therefore drifted from providerMetadata.defaultConfig, so isProviderConfigDirty() treated the passive seed as a user-modified config and shouldListProvider() listed Kokoro as dirty/unconfigured. - add refreshProviderDefaultConfig() to the providers store: re-selects a provider's default config with current runtime state - kokoro-local page: refresh the default after await detectWebGPU(), then seed with getDefaultProviderConfig() - the same source the dirty comparison uses - so the seed is never flagged as a user edit, while fp16-capable hardware still keeps its fp16-webgpu default
|
Good catch — the drift was real. Fixed in
Verified: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 971bb969ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Source: PR #2273 review (codex bot), rounds 1 & 4. | ||
| // Removal condition: when the provider is guaranteed to be registered with | ||
| // capability-accurate defaults before this page can mount. | ||
| if (!providerStore.getProviderConfig(providerId)) { |
There was a problem hiding this comment.
When a user opens this page directly on FP16-capable WebGPU hardware with no saved Kokoro config, the child SpeechProviderSettings mounts first and calls initializeProvider() with the stale pre-detection default. Fresh evidence in this patch: this branch then skips ensureProvider(), while refreshProviderDefaultConfig() above changes the dirty-comparison baseline to fp16-webgpu. As a result, the page keeps and loads the stale fp32-webgpu config, and shouldListProvider() treats that passive seed as a user edit. Replace the existing default config when it still matches the old provider default, or move capability-aware initialization before the child can seed the provider.
Useful? React with 👍 / 👎.
On direct navigation, the child SpeechProviderSettings mounts first and calls initializeProvider() with the stale pre-detection default (fp32-webgpu) before the parent page awaits detectWebGPU(). The page then skips ensureProvider() (config exists), while refreshProviderDefaultConfig() moves the dirty-comparison baseline to fp16-webgpu - leaving a stale fp32 config loaded and flagged as a user edit by shouldListProvider(). refreshProviderDefaultConfig() now snapshots the previous default and, when the saved config still exactly matches it (a passive seed, not a user edit), replaces it with the refreshed default via a new synchronous replaceProviderConfig() on the config store (local mutation, no remote round-trip, by-reference so configs/providerCredentials pick it up).
|
Confirmed — the child Fixed in
A real user edit never matches the stale default, so it is never clobbered. Verified: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e37763c24
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const previousDefault = getDefaultProviderConfig(providerId) | ||
| meta.defaultConfig = getSchemaDefault(definition.createProviderConfig({ t })) as Record<string, unknown> | ||
| const saved = providerCredentials.value[providerId] | ||
| if (saved && JSON.stringify(saved) === JSON.stringify(previousDefault)) { |
There was a problem hiding this comment.
When a user opens the Kokoro page directly on FP16 WebGPU hardware with no saved config, SpeechProviderSettings mounts first and writes an empty apiKey into the stale default config. Fresh evidence in this version: this exact JSON check then fails because saved has passive fields that previousDefault does not. As a result, replaceProviderConfig() does not run, the page keeps fp32-webgpu, and shouldListProvider() treats the passive seed as a user edit after the default changes to fp16-webgpu. Compare only the provider default fields, or update model when it still equals the old default.
Useful? React with 👍 / 👎.
SpeechProviderSettings mounts before the parent page and writes passive fields (e.g. apiKey: '') into the seeded config via its computed setters. A full JSON equality check between the saved config and the stale default then fails, so replaceProviderConfig() never runs and the page keeps fp32-webgpu while the refreshed baseline moves to fp16-webgpu. Compare only the fields that make up the provider default: if they are all untouched the config is still the passive seed and safe to replace. Any deviation means a user edit and is never clobbered.
|
Confirmed — Fixed in Verified: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b322aff5c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const isUntouchedPassiveSeed = saved && Object.keys(previousDefault) | ||
| .every((key) => { | ||
| const savedValue = (saved as Record<string, unknown>)[key] | ||
| const defaultValue = (previousDefault as Record<string, unknown>)[key] | ||
| return savedValue === defaultValue | ||
| }) |
There was a problem hiding this comment.
If an fp16 WebGPU user manually selects fp32-webgpu, a later reload starts with an empty WebGPU cache, so the stale previousDefault is also fp32-webgpu. This predicate then treats the saved model choice as an untouched passive seed and replaces it with fp16-webgpu, because it has no state that distinguishes a persisted user config from an auto-seeded config. Track the direct-navigation seed explicitly, or only replace configs created during this mount.
AGENTS.md reference: AGENTS.md:L198-L200
Useful? React with 👍 / 👎.
On reload with an empty WebGPU cache, the stale default snapshot equals fp32-webgpu - the same value a user on fp16 hardware may have manually selected. The untouched-seed predicate could not distinguish the two, so refreshProviderDefaultConfig() would replace the user's deliberate choice with fp16-webgpu. Replacement now requires the caller to opt in via replaceUntouchedSeed: the kokoro-local page captures whether a persisted config already existed before this mount (setup runs before the child SpeechProviderSettings can seed one) and only allows replacement when it did not - a config created during this mount is a passive seed; a persisted one is user-owned and never replaced.
|
Confirmed — on reload with an empty WebGPU cache, the stale default snapshot is also Fixed in
So a manually saved model choice survives reloads, while the direct-navigation seed still gets corrected. Verified: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62f1dcea11
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| providersStore.refreshProviderDefaultConfig(providerId, { | ||
| replaceUntouchedSeed: !hadConfigBeforeMount, | ||
| }) |
There was a problem hiding this comment.
When direct navigation has no saved Kokoro config on WebGPU, SpeechProviderSettings mounts first and seeds the stale default, which changes model and lets the watcher below start loading fp32-webgpu while this parent onMounted still awaits capability detection. This refresh then switches the config to the capability-aware default, so the correct load queues behind an unnecessary stale model load. Gate the watcher until initial seeding finishes, or initialize the provider before the child can seed it.
Useful? React with 👍 / 👎.
On direct navigation without a saved config, the child SpeechProviderSettings seeds the stale default, which changes `model` and fires the watcher - loading fp32-webgpu while the parent onMounted is still awaiting capability detection. The refresh then swaps in the capability-aware default, queueing the correct load behind an unnecessary stale model load. Gate the watcher with an initialSeeding flag until onMounted finishes initial seeding; the parent already loads the correct model explicitly after refresh, so no model load is lost.
|
Confirmed — the child Fixed in Verified: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5fc19e39
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!definition || !meta) | ||
| return | ||
| const previousDefault = getDefaultProviderConfig(providerId) | ||
| meta.defaultConfig = getSchemaDefault(definition.createProviderConfig({ t })) as Record<string, unknown> |
There was a problem hiding this comment.
In the synced Electron runtime, this method changes store-local providerMetadata. But refreshProviderDefaultConfig is not in this store's synced.actions block. I checked requestListedProviderValidation(): background validation routes through the synced public store. Thus a direct navigation from a non-leader renderer can refresh Kokoro's default to fp16-webgpu only locally, while the leader still compares the saved config with the stale fp32-webgpu default and lists or validates the passive seed as dirty. Add this action to the synced list, or move the refresh into an already synced mutation.
Useful? React with 👍 / 👎.
In the synced Electron runtime, refreshProviderDefaultConfig() mutated store-local providerMetadata and replaceProviderConfig() mutated the config store without being in either store's synced.actions block. A direct navigation from a non-leader renderer would refresh the default to fp16-webgpu only locally, while the leader kept comparing the saved config against the stale fp32-webgpu default - listing or validating the passive seed as dirty. Add both actions to their stores' synced lists so the refresh (and the config replacement it performs) runs on the leader and the resulting state propagates to every renderer.
|
Confirmed — in the synced Electron runtime, Fixed in One note: Verified: |
2 participants
Summary
When navigating directly to the speech settings page, the provider may not be registered from the catalog yet, so
getProviderConfig()returnsundefinedand readingconfig.modelthrows. The model combobox can also write backv-modelbeforeonMountedruns (provider registration is an async synced action).Changes
packages/stage-pages/.../kokoro-local.vue: awaitensureProvider()when config is missing, then re-read; guardset()/watch()against missing config; fall back to{}where a read is unavoidable.pnpm-lock.yaml: remove iOSDerivedDatalock entries that were accidentally committed (Xcode build cache), add missingweb-workeroptional marker.Verification
pnpm install --frozen-lockfilepasses locally.