fix(config): align live save feedback with hot reload policy (#255)

This commit is contained in:
2026-09-20 19:51:45 -07:00
committed by GitHub
parent 4f0762e840
commit 76e43d7e6d
8 changed files with 205 additions and 137 deletions
@@ -0,0 +1,4 @@
type: docs
area: config
- Clarified live-setting save feedback, mixed restart warnings, and subtitle-generation reload behavior.
+5
View File
@@ -0,0 +1,5 @@
type: fixed
area: config
- Settings marked LIVE now use the same reload policy as save results, fixing false restart warnings for notifications and subtitle generation.
- Mixed saves apply live changes and list only sections with changed fields that require a restart.
+3 -3
View File
@@ -61,7 +61,7 @@ The Settings window preserves existing JSONC comments, trailing commas, and unre
Secret fields do not display stored values. They show whether a value is configured; entering a new value writes it, and reset clears the explicit path. Prefer command-based secret options such as `jimaku.apiKeyCommand` when available. Secret fields do not display stored values. They show whether a value is configured; entering a new value writes it, and reset clears the explicit path. Prefer command-based secret options such as `jimaku.apiKeyCommand` when available.
Saving validates the candidate config before writing. Live-reloadable changes are applied immediately; other changes return a restart-required banner in the window. Saving validates the candidate config before writing. Saving only fields marked **LIVE** shows "Saved. Live settings applied." If a save also changes fields that need a restart, the banner lists only the sections containing those changed fields. Live changes still apply in the same save.
## Configuration file ## Configuration file
@@ -103,7 +103,7 @@ SubMiner watches the active config file (`config.jsonc` or `config.json`) while
Hot-reloadable settings include subtitle appearance, sidebar controls, keybindings, Hot-reloadable settings include subtitle appearance, sidebar controls, keybindings,
shortcuts, notifications, logging level, selected source-language preferences, shortcuts, notifications, logging level, selected source-language preferences,
Jimaku/Subsync settings, AniSkip settings (`mpv.aniskipEnabled`, `mpv.aniskipButtonKey`), Jimaku/Subsync and subtitle-generation settings, AniSkip settings (`mpv.aniskipEnabled`, `mpv.aniskipButtonKey`),
stats keys (`stats.toggleKey`, `stats.markWatchedKey`), the secondary-subtitle default stats keys (`stats.toggleKey`, `stats.markWatchedKey`), the secondary-subtitle default
mode, and the Anki deck, known-word, N+1, field, sentence-card, and Kiku options mode, and the Anki deck, known-word, N+1, field, sentence-card, and Kiku options
listed in the reference tables below. listed in the reference tables below.
@@ -975,7 +975,7 @@ This example is intentionally compact. The option table below documents availabl
| `tags` | array of strings | Tags automatically added to cards mined/updated by SubMiner (default: `['SubMiner']`; set `[]` to disable automatic tagging). | | `tags` | array of strings | Tags automatically added to cards mined/updated by SubMiner (default: `['SubMiner']`; set `[]` to disable automatic tagging). |
| `ankiConnect.deck` | string | Restrict duplicate detection and card enrichment to this Anki deck. Leave empty to use the Yomitan mining deck when available. In Settings, this dropdown auto-fills and persists Yomitan's current mining deck when available. | | `ankiConnect.deck` | string | Restrict duplicate detection and card enrichment to this Anki deck. Leave empty to use the Yomitan mining deck when available. In Settings, this dropdown auto-fills and persists Yomitan's current mining deck when available. |
| `fields.word` | string | Card field for mined word / expression text (default: `Expression`) | | `fields.word` | string | Card field for mined word / expression text (default: `Expression`) |
| `fields.audio` | string | Card field for the generated sentence audio clip (default: `ExpressionAudio`). Set this to a dedicated field such as `SentenceAudio` so it does not collide with the word audio Yomitan writes. | | `fields.audio` | string | Card field for the generated sentence audio clip (default: `ExpressionAudio`). Set this to a dedicated field such as `SentenceAudio` so it does not collide with the word audio Yomitan writes. |
| `fields.image` | string | Card field for images (default: `Picture`) | | `fields.image` | string | Card field for images (default: `Picture`) |
| `fields.sentence` | string | Card field for sentences (default: `Sentence`) | | `fields.sentence` | string | Card field for sentences (default: `Sentence`) |
| `fields.miscInfo` | string | Card field for metadata (default: `"MiscInfo"`, set to `null` to disable) | | `fields.miscInfo` | string | Card field for metadata (default: `"MiscInfo"`, set to `null` to disable) |
+64
View File
@@ -0,0 +1,64 @@
function pathStartsWith(path: string, prefix: string): boolean {
return path === prefix || path.startsWith(`${prefix}.`);
}
const HOT_RELOAD_ROOTS = ['subtitleStyle', 'keybindings', 'shortcuts', 'subtitleSidebar'] as const;
const HOT_RELOAD_EXACT_OR_PREFIX_PATHS = [
'secondarySub.defaultMode',
'mpv.aniskipEnabled',
'mpv.aniskipButtonKey',
'ankiConnect.ai.enabled',
'stats.toggleKey',
'stats.markWatchedKey',
'logging.level',
'logging.rotation',
'logging.files',
'youtube.primarySubLanguages',
'ankiConnect.deck',
'ankiConnect.media.normalizeAudio',
'ankiConnect.media.mirrorMpvVolume',
'ankiConnect.media.reviewTiming',
'ankiConnect.behavior.autoUpdateNewCards',
'ankiConnect.knownWords.highlightEnabled',
'ankiConnect.knownWords.refreshMinutes',
'ankiConnect.knownWords.addMinedWordsImmediately',
'ankiConnect.knownWords.matchMode',
'ankiConnect.knownWords.decks',
'ankiConnect.nPlusOne.enabled',
'ankiConnect.nPlusOne.minSentenceWords',
'ankiConnect.fields.word',
'ankiConnect.fields.audio',
'ankiConnect.fields.image',
'ankiConnect.fields.sentence',
'ankiConnect.fields.miscInfo',
'ankiConnect.isLapis.sentenceCardModel',
'ankiConnect.isKiku.fieldGrouping',
'ankiConnect.isSenren.fieldGrouping',
'ankiConnect.lapisKiku.wordCardKind',
] as const;
export function getConfigHotReloadField(path: string): string | null {
for (const root of HOT_RELOAD_ROOTS) {
if (pathStartsWith(path, root)) {
return root;
}
}
for (const hotPath of HOT_RELOAD_EXACT_OR_PREFIX_PATHS) {
if (pathStartsWith(path, hotPath)) {
return hotPath;
}
}
// These consumers read the current config when the next operation starts.
if (
['jimaku', 'subsync', 'notifications', 'subtitleGeneration'].some((root) =>
pathStartsWith(path, root),
)
) {
return path;
}
return null;
}
+2 -49
View File
@@ -1,9 +1,9 @@
import { getConfigHotReloadField } from '../hot-reload';
import type { ResolvedConfig } from '../../types/config'; import type { ResolvedConfig } from '../../types/config';
import type { import type {
ConfigSettingsCategory, ConfigSettingsCategory,
ConfigSettingsControl, ConfigSettingsControl,
ConfigSettingsField, ConfigSettingsField,
ConfigSettingsRestartBehavior,
} from '../../types/settings'; } from '../../types/settings';
import { CONFIG_OPTION_REGISTRY, DEFAULT_CONFIG } from '../definitions'; import { CONFIG_OPTION_REGISTRY, DEFAULT_CONFIG } from '../definitions';
import { import {
@@ -693,53 +693,6 @@ function compareFields(a: ConfigSettingsField, b: ConfigSettingsField): number {
return a.configPath.localeCompare(b.configPath); return a.configPath.localeCompare(b.configPath);
} }
function restartBehaviorForPath(path: string): ConfigSettingsRestartBehavior {
if (
path === 'keybindings' ||
pathStartsWith(path, 'shortcuts') ||
pathStartsWith(path, 'subtitleStyle') ||
pathStartsWith(path, 'subtitleSidebar') ||
path === 'secondarySub.defaultMode' ||
path === 'ankiConnect.deck' ||
path === 'ankiConnect.ai.enabled' ||
path === 'ankiConnect.media.normalizeAudio' ||
path === 'ankiConnect.media.mirrorMpvVolume' ||
path === 'ankiConnect.media.reviewTiming' ||
path === 'ankiConnect.behavior.autoUpdateNewCards' ||
path === 'ankiConnect.knownWords.highlightEnabled' ||
path === 'ankiConnect.knownWords.refreshMinutes' ||
path === 'ankiConnect.knownWords.addMinedWordsImmediately' ||
path === 'ankiConnect.knownWords.matchMode' ||
path === 'ankiConnect.knownWords.decks' ||
path === 'ankiConnect.nPlusOne.enabled' ||
path === 'ankiConnect.nPlusOne.minSentenceWords' ||
path === 'ankiConnect.fields.word' ||
path === 'ankiConnect.fields.audio' ||
path === 'ankiConnect.fields.image' ||
path === 'ankiConnect.fields.sentence' ||
path === 'ankiConnect.fields.miscInfo' ||
path === 'ankiConnect.isLapis.sentenceCardModel' ||
path === 'ankiConnect.isKiku.fieldGrouping' ||
path === 'ankiConnect.isSenren.fieldGrouping' ||
path === 'ankiConnect.lapisKiku.wordCardKind' ||
path === 'mpv.aniskipEnabled' ||
path === 'mpv.aniskipButtonKey' ||
path === 'stats.toggleKey' ||
path === 'stats.markWatchedKey' ||
path === 'logging.level' ||
path === 'logging.rotation' ||
pathStartsWith(path, 'logging.files') ||
pathStartsWith(path, 'notifications') ||
path === 'youtube.primarySubLanguages' ||
pathStartsWith(path, 'jimaku') ||
pathStartsWith(path, 'subsync') ||
pathStartsWith(path, 'subtitleGeneration')
) {
return 'hot-reload';
}
return 'restart';
}
function fieldForLeaf(leaf: Leaf): ConfigSettingsField { function fieldForLeaf(leaf: Leaf): ConfigSettingsField {
const option = OPTION_BY_PATH.get(leaf.path); const option = OPTION_BY_PATH.get(leaf.path);
const { category, section } = categoryAndSection(leaf.path); const { category, section } = categoryAndSection(leaf.path);
@@ -758,7 +711,7 @@ function fieldForLeaf(leaf: Leaf): ConfigSettingsField {
? { enumValues: option.settingsEnumValues ?? option.enumValues } ? { enumValues: option.settingsEnumValues ?? option.enumValues }
: {}), : {}),
...(option?.enumLabels ? { enumLabels: option.enumLabels } : {}), ...(option?.enumLabels ? { enumLabels: option.enumLabels } : {}),
restartBehavior: restartBehaviorForPath(leaf.path), restartBehavior: getConfigHotReloadField(leaf.path) ? 'hot-reload' : 'restart',
advanced: advanced:
leaf.path.startsWith('controller.') || leaf.path.startsWith('controller.') ||
leaf.path.startsWith('immersionTracking.retention.') || leaf.path.startsWith('immersionTracking.retention.') ||
+41 -2
View File
@@ -1,12 +1,47 @@
import test from 'node:test'; import test from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { DEFAULT_CONFIG, deepCloneConfig } from '../../config'; import { DEFAULT_CONFIG, deepCloneConfig } from '../../config';
import { buildConfigSettingsRegistry, getConfigValueAtPath } from '../../config/settings/registry';
import { import {
classifyConfigHotReloadDiff, classifyConfigHotReloadDiff,
createConfigHotReloadRuntime, createConfigHotReloadRuntime,
type ConfigHotReloadRuntimeDeps, type ConfigHotReloadRuntimeDeps,
} from './config-hot-reload'; } from './config-hot-reload';
test('every LIVE settings field is classified without a restart warning', () => {
for (const field of buildConfigSettingsRegistry(DEFAULT_CONFIG)) {
if (field.restartBehavior !== 'hot-reload') continue;
const next = deepCloneConfig(DEFAULT_CONFIG);
const segments = field.configPath.split('.');
const leaf = segments.pop();
assert.ok(leaf);
const parent = segments.length ? getConfigValueAtPath(next, segments.join('.')) : next;
assert.ok(parent && typeof parent === 'object', field.configPath);
// The classifier compares structure; validation of field values is tested separately.
Object.defineProperty(parent, leaf, {
value: getConfigValueAtPath(next, field.configPath) === null ? 'changed' : null,
enumerable: true,
});
const diff = classifyConfigHotReloadDiff(DEFAULT_CONFIG, next);
assert.deepEqual(diff.restartRequiredFields, [], field.configPath);
assert.ok(diff.hotReloadFields.length > 0, field.configPath);
}
});
test('live notifications and subtitle generation changes preserve unrelated restart warnings', () => {
const next = deepCloneConfig(DEFAULT_CONFIG);
next.notifications.overlayPosition = 'top';
next.subtitleGeneration.threads += 1;
next.websocket.port += 1;
const diff = classifyConfigHotReloadDiff(DEFAULT_CONFIG, next);
assert.deepEqual(
new Set(diff.hotReloadFields),
new Set(['notifications.overlayPosition', 'subtitleGeneration.threads']),
);
assert.deepEqual(diff.restartRequiredFields, ['websocket.port']);
});
test('classifyConfigHotReloadDiff separates hot and restart-required fields', () => { test('classifyConfigHotReloadDiff separates hot and restart-required fields', () => {
const prev = deepCloneConfig(DEFAULT_CONFIG); const prev = deepCloneConfig(DEFAULT_CONFIG);
const next = deepCloneConfig(DEFAULT_CONFIG); const next = deepCloneConfig(DEFAULT_CONFIG);
@@ -15,7 +50,7 @@ test('classifyConfigHotReloadDiff separates hot and restart-required fields', ()
const diff = classifyConfigHotReloadDiff(prev, next); const diff = classifyConfigHotReloadDiff(prev, next);
assert.deepEqual(diff.hotReloadFields, ['subtitleStyle']); assert.deepEqual(diff.hotReloadFields, ['subtitleStyle']);
assert.deepEqual(diff.restartRequiredFields, ['websocket']); assert.deepEqual(diff.restartRequiredFields, ['websocket.port']);
}); });
test('classifyConfigHotReloadDiff treats safe nested config paths as hot-reloadable', () => { test('classifyConfigHotReloadDiff treats safe nested config paths as hot-reloadable', () => {
@@ -101,7 +136,11 @@ test('classifyConfigHotReloadDiff keeps unsafe nested siblings restart-required'
const diff = classifyConfigHotReloadDiff(prev, next); const diff = classifyConfigHotReloadDiff(prev, next);
assert.deepEqual(diff.hotReloadFields, []); assert.deepEqual(diff.hotReloadFields, []);
assert.deepEqual(diff.restartRequiredFields, ['ankiConnect', 'stats']); assert.deepEqual(diff.restartRequiredFields, [
'ankiConnect.url',
'ankiConnect.ai.model',
'stats.serverPort',
]);
}); });
test('config hot reload runtime debounces rapid watch events', () => { test('config hot reload runtime debounces rapid watch events', () => {
+4 -83
View File
@@ -1,3 +1,4 @@
import { getConfigHotReloadField } from '../../config/hot-reload';
import { type ReloadConfigStrictResult } from '../../config'; import { type ReloadConfigStrictResult } from '../../config';
import type { ConfigValidationWarning } from '../../types'; import type { ConfigValidationWarning } from '../../types';
import type { ResolvedConfig } from '../../types'; import type { ResolvedConfig } from '../../types';
@@ -33,10 +34,6 @@ function isRecord(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === 'object' && !Array.isArray(value); return value !== null && typeof value === 'object' && !Array.isArray(value);
} }
function pathStartsWith(path: string, prefix: string): boolean {
return path === prefix || path.startsWith(`${prefix}.`);
}
function collectChangedPaths(prev: unknown, next: unknown, prefix = ''): string[] { function collectChangedPaths(prev: unknown, next: unknown, prefix = ''): string[] {
if (isEqual(prev, next)) { if (isEqual(prev, next)) {
return []; return [];
@@ -52,60 +49,6 @@ function collectChangedPaths(prev: unknown, next: unknown, prefix = ''): string[
); );
} }
const HOT_RELOAD_ROOTS = ['subtitleStyle', 'keybindings', 'shortcuts', 'subtitleSidebar'] as const;
const HOT_RELOAD_EXACT_OR_PREFIX_PATHS = [
'secondarySub.defaultMode',
'mpv.aniskipEnabled',
'mpv.aniskipButtonKey',
'ankiConnect.ai.enabled',
'stats.toggleKey',
'stats.markWatchedKey',
'logging.level',
'logging.rotation',
'logging.files',
'youtube.primarySubLanguages',
'jimaku',
'subsync',
'ankiConnect.deck',
'ankiConnect.media.normalizeAudio',
'ankiConnect.media.mirrorMpvVolume',
'ankiConnect.media.reviewTiming',
'ankiConnect.behavior.autoUpdateNewCards',
'ankiConnect.knownWords.highlightEnabled',
'ankiConnect.knownWords.refreshMinutes',
'ankiConnect.knownWords.addMinedWordsImmediately',
'ankiConnect.knownWords.matchMode',
'ankiConnect.knownWords.decks',
'ankiConnect.nPlusOne.enabled',
'ankiConnect.nPlusOne.minSentenceWords',
'ankiConnect.fields.word',
'ankiConnect.fields.audio',
'ankiConnect.fields.image',
'ankiConnect.fields.sentence',
'ankiConnect.fields.miscInfo',
'ankiConnect.isLapis.sentenceCardModel',
'ankiConnect.isKiku.fieldGrouping',
'ankiConnect.isSenren.fieldGrouping',
'ankiConnect.lapisKiku.wordCardKind',
] as const;
function hotReloadFieldForChangedPath(path: string): string | null {
for (const root of HOT_RELOAD_ROOTS) {
if (pathStartsWith(path, root)) {
return root;
}
}
for (const hotPath of HOT_RELOAD_EXACT_OR_PREFIX_PATHS) {
if (pathStartsWith(path, hotPath)) {
return hotPath === 'jimaku' || hotPath === 'subsync' ? path : hotPath;
}
}
return null;
}
function classifyDiff(prev: ResolvedConfig, next: ResolvedConfig): ConfigHotReloadDiff { function classifyDiff(prev: ResolvedConfig, next: ResolvedConfig): ConfigHotReloadDiff {
const hotReloadFields: string[] = []; const hotReloadFields: string[] = [];
const restartRequiredFields: string[] = []; const restartRequiredFields: string[] = [];
@@ -113,33 +56,11 @@ function classifyDiff(prev: ResolvedConfig, next: ResolvedConfig): ConfigHotRelo
const changedPaths = collectChangedPaths(prev, next); const changedPaths = collectChangedPaths(prev, next);
for (const path of changedPaths) { for (const path of changedPaths) {
const hotReloadField = hotReloadFieldForChangedPath(path); const hotReloadField = getConfigHotReloadField(path);
if (hotReloadField) { if (hotReloadField) {
hotReloadFieldSet.add(hotReloadField); hotReloadFieldSet.add(hotReloadField);
} } else {
} restartRequiredFields.push(path);
const keys = new Set([
...(Object.keys(prev) as Array<keyof ResolvedConfig>),
...(Object.keys(next) as Array<keyof ResolvedConfig>),
]);
for (const key of keys) {
if (
key === 'subtitleStyle' ||
key === 'keybindings' ||
key === 'shortcuts' ||
key === 'subtitleSidebar'
) {
continue;
}
const changedPathsForKey = changedPaths.filter((path) => pathStartsWith(path, String(key)));
const hasRestartRequiredChange = changedPathsForKey.some(
(path) => !hotReloadFieldForChangedPath(path),
);
if (hasRestartRequiredChange) {
restartRequiredFields.push(String(key));
} }
} }
@@ -5,9 +5,91 @@ import os from 'node:os';
import path from 'node:path'; import path from 'node:path';
import { DEFAULT_CONFIG, deepCloneConfig } from '../../config'; import { DEFAULT_CONFIG, deepCloneConfig } from '../../config';
import { resolveConfig } from '../../config/resolve'; import { resolveConfig } from '../../config/resolve';
import { buildConfigSettingsRegistry } from '../../config/settings/registry';
import type { RawConfig } from '../../types/config';
import { IPC_CHANNELS } from '../../shared/ipc/contracts'; import { IPC_CHANNELS } from '../../shared/ipc/contracts';
import { createConfigSettingsRuntime } from './config-settings-runtime'; import { createConfigSettingsRuntime } from './config-settings-runtime';
test('settings saves report live changes and only the sections that actually need restart', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-settings-live-'));
const configPath = path.join(dir, 'config.jsonc');
let rawConfig: RawConfig = {};
let resolvedConfig = resolveConfig(rawConfig).resolved;
const applied: string[][] = [];
const runtime = createConfigSettingsRuntime({
fields: buildConfigSettingsRegistry(DEFAULT_CONFIG),
getConfigPath: () => configPath,
getRawConfig: () => rawConfig,
getConfig: () => resolvedConfig,
getWarnings: () => [],
reloadConfigStrict: () => {
rawConfig = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
const result = resolveConfig(rawConfig);
resolvedConfig = result.resolved;
return { ok: true, config: resolvedConfig, warnings: result.warnings, path: configPath };
},
onHotReloadApplied: (diff) => {
applied.push(diff.hotReloadFields);
},
getSettingsWindow: () => null,
setSettingsWindow: () => {},
createSettingsWindow: () => {
throw new Error('Save must not open a window');
},
settingsHtmlPath: '/tmp/settings.html',
openPath: async () => '',
defaultAnkiConnectUrl: DEFAULT_CONFIG.ankiConnect.url,
createAnkiClient: () => {
throw new Error('Save must not query Anki');
},
ipcMain: { handle: () => {} },
ipcChannels: IPC_CHANNELS.request,
});
try {
const live = runtime.savePatch({
operations: [
{ op: 'set', path: 'notifications.overlayPosition', value: 'top' },
{
op: 'set',
path: 'subtitleGeneration.threads',
value: DEFAULT_CONFIG.subtitleGeneration.threads + 1,
},
],
});
assert.equal(live.ok, true);
assert.deepEqual(live.restartRequiredFields, []);
assert.deepEqual(live.restartRequiredSections, []);
assert.deepEqual(
new Set(live.hotReloadFields),
new Set(['notifications.overlayPosition', 'subtitleGeneration.threads']),
);
assert.deepEqual(applied, [live.hotReloadFields]);
const mixed = runtime.savePatch({
operations: [
{ op: 'set', path: 'ankiConnect.deck', value: 'Mining' },
{ op: 'set', path: 'ankiConnect.url', value: 'http://127.0.0.1:9999' },
],
});
assert.equal(mixed.ok, true);
assert.deepEqual(mixed.hotReloadFields, ['ankiConnect.deck']);
assert.deepEqual(mixed.restartRequiredSections, ['AnkiConnect']);
const reset = runtime.savePatch({
operations: [
{ op: 'reset', path: 'notifications.overlayPosition' },
{ op: 'reset', path: 'subtitleGeneration.threads' },
],
});
assert.equal(reset.ok, true);
assert.deepEqual(reset.restartRequiredSections, []);
assert.deepEqual(new Set(reset.hotReloadFields), new Set(live.hotReloadFields));
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});
test('config settings runtime exposes inferred Yomitan Anki deck lookup', async () => { test('config settings runtime exposes inferred Yomitan Anki deck lookup', async () => {
const handlers = new Map<string, (event: unknown, ...args: unknown[]) => unknown>(); const handlers = new Map<string, (event: unknown, ...args: unknown[]) => unknown>();
const runtime = createConfigSettingsRuntime({ const runtime = createConfigSettingsRuntime({