sudacode/SubMiner
1
0
Fork 0
mirror of https://github.com/ksyasuda/SubMiner.git synced 2026-03-20 12:11:28 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(review): address latest CodeRabbit comments

Browse Source
This commit is contained in:
Kyle Yasuda
2026-03-19 23:13:43 -07:00
parent 544cd8aaa0
commit 3995c396f8
6 changed files with 379 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

97
launcher/commands/stats-command.ts
View File

@@ -29,6 +29,11 @@ type StatsCommandDeps = {
const STATS_STARTUP_RESPONSE_TIMEOUT_MS = 12_000;
type StatsResponseWait = {
controller: AbortController;
promise: Promise<{ kind: 'response'; response: StatsCommandResponse }>;
};
const defaultDeps: StatsCommandDeps = {
createTempDir: (prefix) => fs.mkdtempSync(path.join(os.tmpdir(), prefix)),
joinPath: (...parts) => path.join(...parts),
@@ -62,6 +67,41 @@ const defaultDeps: StatsCommandDeps = {
},
};
async function performStartupHandshake(
createResponseWait: () => StatsResponseWait,
attachedExitPromise: Promise<number>,
): Promise<boolean> {
const responseWait = createResponseWait();
const startupResult = await Promise.race([
responseWait.promise,
attachedExitPromise.then((status) => ({ kind: 'exit' as const, status })),
]);
if (startupResult.kind === 'exit') {
if (startupResult.status !== 0) {
responseWait.controller.abort();
throw new Error(`Stats app exited before startup response (status ${startupResult.status}).`);
}
const response = await responseWait.promise.then((result) => result.response);
if (!response.ok) {
throw new Error(response.error || 'Stats dashboard failed to start.');
}
return true;
}
if (!startupResult.response.ok) {
throw new Error(startupResult.response.error || 'Stats dashboard failed to start.');
}
const exitStatus = await attachedExitPromise;
if (exitStatus !== 0) {
throw new Error(`Stats app exited with status ${exitStatus}.`);
}
return true;
}
export async function runStatsCommand(
context: LauncherCommandContext,
deps: Partial<StatsCommandDeps> = {},
@@ -120,62 +160,7 @@ export async function runStatsCommand(
return true;
}
if (!args.statsCleanup && !args.statsStop) {
const responseWait = createResponseWait();
const startupResult = await Promise.race([
responseWait.promise,
attachedExitPromise.then((status) => ({ kind: 'exit' as const, status })),
]);
if (startupResult.kind === 'exit') {
if (startupResult.status !== 0) {
responseWait.controller.abort();
throw new Error(
`Stats app exited before startup response (status ${startupResult.status}).`,
);
}
const response = await responseWait.promise.then((result) => result.response);
if (!response.ok) {
throw new Error(response.error || 'Stats dashboard failed to start.');
}
return true;
}
if (!startupResult.response.ok) {
throw new Error(startupResult.response.error || 'Stats dashboard failed to start.');
}
const exitStatus = await attachedExitPromise;
if (exitStatus !== 0) {
throw new Error(`Stats app exited with status ${exitStatus}.`);
}
return true;
}
const attachedExitPromiseCleanup = attachedExitPromise;
const responseWait = createResponseWait();
const startupResult = await Promise.race([
responseWait.promise,
attachedExitPromiseCleanup.then((status) => ({ kind: 'exit' as const, status })),
]);
if (startupResult.kind === 'exit') {
if (startupResult.status !== 0) {
responseWait.controller.abort();
throw new Error(
`Stats app exited before startup response (status ${startupResult.status}).`,
);
}
const response = await responseWait.promise.then((result) => result.response);
if (!response.ok) {
throw new Error(response.error || 'Stats dashboard failed to start.');
}
return true;
}
if (!startupResult.response.ok) {
throw new Error(startupResult.response.error || 'Stats dashboard failed to start.');
}
const exitStatus = await attachedExitPromiseCleanup;
if (exitStatus !== 0) {
throw new Error(`Stats app exited with status ${exitStatus}.`);
}
return true;
return await performStartupHandshake(createResponseWait, attachedExitPromise);
} finally {
resolvedDeps.removeDir(tempDir);
}

25
src/anki-integration.ts
View File

@@ -199,6 +199,25 @@ export class AnkiIntegration {
});
}
private recordCardsMinedSafely(
count: number,
noteIds: number[] | undefined,
source: string,
): void {
if (!this.recordCardsMinedCallback) {
return;
}
try {
this.recordCardsMinedCallback(count, noteIds);
} catch (error) {
log.warn(
`recordCardsMined callback failed during ${source}:`,
(error as Error).message,
);
}
}
private createKnownWordCache(knownWordCacheStatePath?: string): KnownWordCacheManager {
return new KnownWordCacheManager({
client: {
@@ -221,7 +240,7 @@ export class AnkiIntegration {
shouldAutoUpdateNewCards: () => this.config.behavior?.autoUpdateNewCards !== false,
processNewCard: (noteId) => this.processNewCard(noteId),
recordCardsAdded: (count, noteIds) => {
this.recordCardsMinedCallback?.(count, noteIds);
this.recordCardsMinedSafely(count, noteIds, 'polling');
},
isUpdateInProgress: () => this.updateInProgress,
setUpdateInProgress: (value) => {
@@ -245,7 +264,7 @@ export class AnkiIntegration {
shouldAutoUpdateNewCards: () => this.config.behavior?.autoUpdateNewCards !== false,
processNewCard: (noteId: number) => this.processNewCard(noteId),
recordCardsAdded: (count, noteIds) => {
this.recordCardsMinedCallback?.(count, noteIds);
this.recordCardsMinedSafely(count, noteIds, 'proxy');
},
getDeck: () => this.config.deck,
findNotes: async (query, options) =>
@@ -345,7 +364,7 @@ export class AnkiIntegration {
this.previousNoteIds.add(noteId);
},
recordCardsMinedCallback: (count, noteIds) => {
this.recordCardsMinedCallback?.(count, noteIds);
this.recordCardsMinedSafely(count, noteIds, 'card creation');
},
});
}

198
src/anki-integration/card-creation.test.ts
View File

@@ -85,3 +85,201 @@ test('CardCreationService counts locally created sentence cards', async () => {
assert.equal(created, true);
assert.deepEqual(minedCards, [{ count: 1, noteIds: [42] }]);
});
test('CardCreationService keeps updating after trackLastAddedNoteId throws', async () => {
const calls = {
notesInfo: 0,
updateNoteFields: 0,
};
const service = new CardCreationService({
getConfig: () =>
({
deck: 'Mining',
fields: {
sentence: 'Sentence',
audio: 'SentenceAudio',
},
media: {
generateAudio: false,
generateImage: false,
},
behavior: {},
ai: false,
}) as AnkiConnectConfig,
getAiConfig: () => ({}),
getTimingTracker: () => ({}) as never,
getMpvClient: () =>
({
currentVideoPath: '/video.mp4',
currentSubText: '字幕',
currentSubStart: 1,
currentSubEnd: 2,
currentTimePos: 1.5,
currentAudioStreamIndex: 0,
}) as never,
client: {
addNote: async () => 42,
addTags: async () => undefined,
notesInfo: async () => {
calls.notesInfo += 1;
return [
{
noteId: 42,
fields: {
Sentence: { value: 'existing' },
},
},
];
},
updateNoteFields: async () => {
calls.updateNoteFields += 1;
},
storeMediaFile: async () => undefined,
findNotes: async () => [],
retrieveMediaFile: async () => '',
},
mediaGenerator: {
generateAudio: async () => null,
generateScreenshot: async () => null,
generateAnimatedImage: async () => null,
},
showOsdNotification: () => undefined,
showUpdateResult: () => undefined,
showStatusNotification: () => undefined,
showNotification: async () => undefined,
beginUpdateProgress: () => undefined,
endUpdateProgress: () => undefined,
withUpdateProgress: async (_message, action) => action(),
resolveConfiguredFieldName: () => null,
resolveNoteFieldName: () => null,
getAnimatedImageLeadInSeconds: async () => 0,
extractFields: () => ({}),
processSentence: (sentence) => sentence,
setCardTypeFields: (updatedFields) => {
updatedFields.CardType = 'sentence';
},
mergeFieldValue: (_existing, newValue) => newValue,
formatMiscInfoPattern: () => '',
getEffectiveSentenceCardConfig: () => ({
model: 'Sentence',
sentenceField: 'Sentence',
audioField: 'SentenceAudio',
lapisEnabled: false,
kikuEnabled: false,
kikuFieldGrouping: 'disabled',
kikuDeleteDuplicateInAuto: false,
}),
getFallbackDurationSeconds: () => 10,
appendKnownWordsFromNoteInfo: () => undefined,
isUpdateInProgress: () => false,
setUpdateInProgress: () => undefined,
trackLastAddedNoteId: () => {
throw new Error('track failed');
},
});
const created = await service.createSentenceCard('テスト', 0, 1);
assert.equal(created, true);
assert.equal(calls.notesInfo, 1);
assert.equal(calls.updateNoteFields, 1);
});
test('CardCreationService keeps updating after recordCardsMinedCallback throws', async () => {
const calls = {
notesInfo: 0,
updateNoteFields: 0,
};
const service = new CardCreationService({
getConfig: () =>
({
deck: 'Mining',
fields: {
sentence: 'Sentence',
audio: 'SentenceAudio',
},
media: {
generateAudio: false,
generateImage: false,
},
behavior: {},
ai: false,
}) as AnkiConnectConfig,
getAiConfig: () => ({}),
getTimingTracker: () => ({}) as never,
getMpvClient: () =>
({
currentVideoPath: '/video.mp4',
currentSubText: '字幕',
currentSubStart: 1,
currentSubEnd: 2,
currentTimePos: 1.5,
currentAudioStreamIndex: 0,
}) as never,
client: {
addNote: async () => 42,
addTags: async () => undefined,
notesInfo: async () => {
calls.notesInfo += 1;
return [
{
noteId: 42,
fields: {
Sentence: { value: 'existing' },
},
},
];
},
updateNoteFields: async () => {
calls.updateNoteFields += 1;
},
storeMediaFile: async () => undefined,
findNotes: async () => [],
retrieveMediaFile: async () => '',
},
mediaGenerator: {
generateAudio: async () => null,
generateScreenshot: async () => null,
generateAnimatedImage: async () => null,
},
showOsdNotification: () => undefined,
showUpdateResult: () => undefined,
showStatusNotification: () => undefined,
showNotification: async () => undefined,
beginUpdateProgress: () => undefined,
endUpdateProgress: () => undefined,
withUpdateProgress: async (_message, action) => action(),
resolveConfiguredFieldName: () => null,
resolveNoteFieldName: () => null,
getAnimatedImageLeadInSeconds: async () => 0,
extractFields: () => ({}),
processSentence: (sentence) => sentence,
setCardTypeFields: (updatedFields) => {
updatedFields.CardType = 'sentence';
},
mergeFieldValue: (_existing, newValue) => newValue,
formatMiscInfoPattern: () => '',
getEffectiveSentenceCardConfig: () => ({
model: 'Sentence',
sentenceField: 'Sentence',
audioField: 'SentenceAudio',
lapisEnabled: false,
kikuEnabled: false,
kikuFieldGrouping: 'disabled',
kikuDeleteDuplicateInAuto: false,
}),
getFallbackDurationSeconds: () => 10,
appendKnownWordsFromNoteInfo: () => undefined,
isUpdateInProgress: () => false,
setUpdateInProgress: () => undefined,
recordCardsMinedCallback: () => {
throw new Error('record failed');
},
});
const created = await service.createSentenceCard('テスト', 0, 1);
assert.equal(created, true);
assert.equal(calls.notesInfo, 1);
assert.equal(calls.updateNoteFields, 1);
});

14
src/anki-integration/card-creation.ts
View File

@@ -551,14 +551,24 @@ export class CardCreationService {
this.getConfiguredAnkiTags(),
);
log.info('Created sentence card:', noteId);
this.deps.trackLastAddedNoteId?.(noteId);
this.deps.recordCardsMinedCallback?.(1, [noteId]);
} catch (error) {
log.error('Failed to create sentence card:', (error as Error).message);
this.deps.showUpdateResult(`Sentence card failed: ${(error as Error).message}`, false);
return false;
}
try {
this.deps.trackLastAddedNoteId?.(noteId);
} catch (error) {
log.warn('Failed to track last added note:', (error as Error).message);
}
try {
this.deps.recordCardsMinedCallback?.(1, [noteId]);
} catch (error) {
log.warn('Failed to record mined card:', (error as Error).message);
}
try {
const noteInfoResult = await this.deps.client.notesInfo([noteId]);
const noteInfos = noteInfoResult as CardCreationNoteInfo[];

95
src/anki-integration/known-word-cache.test.ts
View File

@@ -263,6 +263,101 @@ test('KnownWordCacheManager refresh incrementally reconciles deleted and edited
}
});
test('KnownWordCacheManager preserves cache state key captured before refresh work', async () => {
const config: AnkiConnectConfig = {
fields: {
word: 'Word',
},
knownWords: {
highlightEnabled: true,
refreshMinutes: 1,
},
};
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-known-word-cache-key-'));
const statePath = path.join(stateDir, 'known-words-cache.json');
let notesInfoStarted = false;
let releaseNotesInfo!: () => void;
const notesInfoGate = new Promise<void>((resolve) => {
releaseNotesInfo = resolve;
});
const manager = new KnownWordCacheManager({
client: {
findNotes: async () => [1],
notesInfo: async () => {
notesInfoStarted = true;
await notesInfoGate;
return [
{
noteId: 1,
fields: {
Word: { value: '猫' },
},
},
];
},
},
getConfig: () => config,
knownWordCacheStatePath: statePath,
showStatusNotification: () => undefined,
});
try {
const refreshPromise = manager.refresh(true);
await waitForCondition(() => notesInfoStarted);
config.fields = {
...config.fields,
word: 'Expression',
};
releaseNotesInfo();
await refreshPromise;
const persisted = JSON.parse(fs.readFileSync(statePath, 'utf-8')) as {
scope: string;
words: string[];
};
assert.equal(
persisted.scope,
'{"refreshMinutes":1,"scope":"is:note","fieldsWord":"Word"}',
);
assert.deepEqual(persisted.words, ['猫']);
} finally {
fs.rmSync(stateDir, { recursive: true, force: true });
}
});
test('KnownWordCacheManager does not borrow fields from other decks during refresh', async () => {
const config: AnkiConnectConfig = {
knownWords: {
highlightEnabled: true,
decks: {
Mining: [],
Reading: ['AltWord'],
},
},
};
const { manager, clientState, cleanup } = createKnownWordCacheHarness(config);
try {
clientState.findNotesByQuery.set('deck:"Mining"', [1]);
clientState.findNotesByQuery.set('deck:"Reading"', []);
clientState.notesInfoResult = [
{
noteId: 1,
fields: {
AltWord: { value: '猫' },
},
},
];
await manager.refresh(true);
assert.equal(manager.isKnownWord('猫'), false);
} finally {
cleanup();
}
});
test('KnownWordCacheManager invalidates persisted cache when per-deck fields change', () => {
const config: AnkiConnectConfig = {
fields: {

30
src/anki-integration/known-word-cache.ts
View File

@@ -227,6 +227,7 @@ export class KnownWordCacheManager {
return;
}
const frozenStateKey = this.getKnownWordCacheStateKey();
this.isRefreshingKnownWords = true;
try {
const noteFieldsById = await this.fetchKnownWordNoteFieldsById();
@@ -257,7 +258,7 @@ export class KnownWordCacheManager {
}
this.knownWordsLastRefreshedAtMs = Date.now();
this.knownWordsStateKey = this.getKnownWordCacheStateKey();
this.knownWordsStateKey = frozenStateKey;
this.persistKnownWordCacheState();
log.info(
'Known-word cache refreshed',
@@ -284,6 +285,11 @@ export class KnownWordCacheManager {
return getKnownWordCacheRefreshIntervalMinutes(this.deps.getConfig()) * 60_000;
}
private getDefaultKnownWordFields(): string[] {
const configuredWordField = getConfiguredWordFieldName(this.deps.getConfig());
return [...new Set([configuredWordField, 'Word', 'Reading', 'Word Reading'])];
}
private getKnownWordDecks(): string[] {
const configuredDecks = this.deps.getConfig().knownWords?.decks;
if (configuredDecks && typeof configuredDecks === 'object' && !Array.isArray(configuredDecks)) {
@@ -297,20 +303,7 @@ export class KnownWordCacheManager {
}
private getConfiguredFields(): string[] {
const configuredDecks = this.deps.getConfig().knownWords?.decks;
if (configuredDecks && typeof configuredDecks === 'object' && !Array.isArray(configuredDecks)) {
const allFields = new Set<string>();
for (const fields of Object.values(configuredDecks)) {
if (Array.isArray(fields)) {
for (const f of fields) {
if (typeof f === 'string' && f.trim()) allFields.add(f.trim());
}
}
}
if (allFields.size > 0) return [...allFields];
}
const configuredWordField = getConfiguredWordFieldName(this.deps.getConfig());
return [...new Set([configuredWordField, 'Word', 'Reading', 'Word Reading'])];
return this.getDefaultKnownWordFields();
}
private getImmediateAppendFields(): string[] | null {
@@ -344,8 +337,7 @@ export class KnownWordCacheManager {
}
}
const configuredWordField = getConfiguredWordFieldName(this.deps.getConfig());
return [...new Set([configuredWordField, 'Word', 'Reading', 'Word Reading'])];
return this.getDefaultKnownWordFields();
}
return this.getConfiguredFields();
@@ -365,7 +357,7 @@ export class KnownWordCacheManager {
: [];
scopes.push({
query: `deck:"${escapeAnkiSearchValue(trimmedDeckName)}"`,
fields: normalizedFields.length > 0 ? normalizedFields : this.getConfiguredFields(),
fields: normalizedFields.length > 0 ? normalizedFields : this.getDefaultKnownWordFields(),
});
}
if (scopes.length > 0) {
@@ -373,7 +365,7 @@ export class KnownWordCacheManager {
}
}
return [{ query: this.buildKnownWordsQuery(), fields: this.getConfiguredFields() }];
return [{ query: this.buildKnownWordsQuery(), fields: this.getDefaultKnownWordFields() }];
}
private buildKnownWordsQuery(): string {