mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-04-01 06:12:07 -07:00
fix: address CodeRabbit review feedback
This commit is contained in:
@@ -487,3 +487,88 @@ test('CardCreationService tracks pre-add duplicate note ids for kiku sentence ca
|
|||||||
assert.deepEqual(duplicateLookupExpressions, ['重複文']);
|
assert.deepEqual(duplicateLookupExpressions, ['重複文']);
|
||||||
assert.deepEqual(trackedDuplicates, [{ noteId: 42, duplicateNoteIds: [7, 18, 30] }]);
|
assert.deepEqual(trackedDuplicates, [{ noteId: 42, duplicateNoteIds: [7, 18, 30] }]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('CardCreationService does not track duplicate ids when pre-add lookup returns none', async () => {
|
||||||
|
const trackedDuplicates: Array<{ noteId: number; duplicateNoteIds: number[] }> = [];
|
||||||
|
|
||||||
|
const service = new CardCreationService({
|
||||||
|
getConfig: () =>
|
||||||
|
({
|
||||||
|
deck: 'Mining',
|
||||||
|
fields: {
|
||||||
|
word: 'Expression',
|
||||||
|
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 () => [],
|
||||||
|
updateNoteFields: async () => undefined,
|
||||||
|
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: () => undefined,
|
||||||
|
mergeFieldValue: (_existing, newValue) => newValue,
|
||||||
|
formatMiscInfoPattern: () => '',
|
||||||
|
getEffectiveSentenceCardConfig: () => ({
|
||||||
|
model: 'Sentence',
|
||||||
|
sentenceField: 'Sentence',
|
||||||
|
audioField: 'SentenceAudio',
|
||||||
|
lapisEnabled: false,
|
||||||
|
kikuEnabled: true,
|
||||||
|
kikuFieldGrouping: 'manual',
|
||||||
|
kikuDeleteDuplicateInAuto: false,
|
||||||
|
}),
|
||||||
|
getFallbackDurationSeconds: () => 10,
|
||||||
|
appendKnownWordsFromNoteInfo: () => undefined,
|
||||||
|
isUpdateInProgress: () => false,
|
||||||
|
setUpdateInProgress: () => undefined,
|
||||||
|
trackLastAddedNoteId: () => undefined,
|
||||||
|
findDuplicateNoteIds: async () => [],
|
||||||
|
trackLastAddedDuplicateNoteIds: (noteId, duplicateNoteIds) => {
|
||||||
|
trackedDuplicates.push({ noteId, duplicateNoteIds });
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const created = await service.createSentenceCard('重複なし', 0, 1);
|
||||||
|
|
||||||
|
assert.equal(created, true);
|
||||||
|
assert.deepEqual(trackedDuplicates, []);
|
||||||
|
});
|
||||||
|
|||||||
@@ -602,11 +602,13 @@ export class CardCreationService {
|
|||||||
log.warn('Failed to track last added note:', (error as Error).message);
|
log.warn('Failed to track last added note:', (error as Error).message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (duplicateNoteIds.length > 0) {
|
||||||
try {
|
try {
|
||||||
this.deps.trackLastAddedDuplicateNoteIds?.(noteId, duplicateNoteIds);
|
this.deps.trackLastAddedDuplicateNoteIds?.(noteId, duplicateNoteIds);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
log.warn('Failed to track duplicate note ids:', (error as Error).message);
|
log.warn('Failed to track duplicate note ids:', (error as Error).message);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
this.deps.recordCardsMinedCallback?.(1, [noteId]);
|
this.deps.recordCardsMinedCallback?.(1, [noteId]);
|
||||||
|
|||||||
@@ -267,3 +267,33 @@ test('findDuplicateNote does not disable retries on findNotes calls', async () =
|
|||||||
assert.ok(seenOptions.length > 0);
|
assert.ok(seenOptions.length > 0);
|
||||||
assert.ok(seenOptions.every((options) => options?.maxRetries !== 0));
|
assert.ok(seenOptions.every((options) => options?.maxRetries !== 0));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('findDuplicateNote stops after the first exact-match chunk', async () => {
|
||||||
|
const currentNote: NoteInfo = {
|
||||||
|
noteId: 100,
|
||||||
|
fields: {
|
||||||
|
Expression: { value: '貴様' },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
let notesInfoCalls = 0;
|
||||||
|
const candidateIds = Array.from({ length: 51 }, (_, index) => 200 + index);
|
||||||
|
const duplicateId = await findDuplicateNote('貴様', 100, currentNote, {
|
||||||
|
findNotes: async () => candidateIds,
|
||||||
|
notesInfo: async (noteIds) => {
|
||||||
|
notesInfoCalls += 1;
|
||||||
|
return noteIds.map((noteId) => ({
|
||||||
|
noteId,
|
||||||
|
fields: {
|
||||||
|
Expression: { value: noteId === 200 ? '貴様' : `別単語-${noteId}` },
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
},
|
||||||
|
getDeck: () => 'Japanese::Mining',
|
||||||
|
resolveFieldName: (noteInfo, preferredName) => createFieldResolver(noteInfo, preferredName),
|
||||||
|
logWarn: () => {},
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.equal(duplicateId, 200);
|
||||||
|
assert.equal(notesInfoCalls, 1);
|
||||||
|
});
|
||||||
|
|||||||
@@ -24,7 +24,13 @@ export async function findDuplicateNote(
|
|||||||
noteInfo: NoteInfo,
|
noteInfo: NoteInfo,
|
||||||
deps: DuplicateDetectionDeps,
|
deps: DuplicateDetectionDeps,
|
||||||
): Promise<number | null> {
|
): Promise<number | null> {
|
||||||
const duplicateNoteIds = await findDuplicateNoteIds(expression, excludeNoteId, noteInfo, deps);
|
const duplicateNoteIds = await findDuplicateNoteIds(
|
||||||
|
expression,
|
||||||
|
excludeNoteId,
|
||||||
|
noteInfo,
|
||||||
|
deps,
|
||||||
|
1,
|
||||||
|
);
|
||||||
return duplicateNoteIds[0] ?? null;
|
return duplicateNoteIds[0] ?? null;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -33,6 +39,7 @@ export async function findDuplicateNoteIds(
|
|||||||
excludeNoteId: number,
|
excludeNoteId: number,
|
||||||
noteInfo: NoteInfo,
|
noteInfo: NoteInfo,
|
||||||
deps: DuplicateDetectionDeps,
|
deps: DuplicateDetectionDeps,
|
||||||
|
maxMatches?: number,
|
||||||
): Promise<number[]> {
|
): Promise<number[]> {
|
||||||
const configuredWordFieldCandidates = deps.getWordFieldCandidates?.() ?? ['Expression', 'Word'];
|
const configuredWordFieldCandidates = deps.getWordFieldCandidates?.() ?? ['Expression', 'Word'];
|
||||||
const sourceCandidates = getDuplicateSourceCandidates(
|
const sourceCandidates = getDuplicateSourceCandidates(
|
||||||
@@ -99,6 +106,7 @@ export async function findDuplicateNoteIds(
|
|||||||
sourceCandidates.map((candidate) => candidate.value),
|
sourceCandidates.map((candidate) => candidate.value),
|
||||||
configuredWordFieldCandidates,
|
configuredWordFieldCandidates,
|
||||||
deps,
|
deps,
|
||||||
|
maxMatches,
|
||||||
);
|
);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
deps.logWarn('Duplicate search failed:', error);
|
deps.logWarn('Duplicate search failed:', error);
|
||||||
@@ -112,6 +120,7 @@ function findExactDuplicateNoteIds(
|
|||||||
sourceValues: string[],
|
sourceValues: string[],
|
||||||
candidateFieldNames: string[],
|
candidateFieldNames: string[],
|
||||||
deps: DuplicateDetectionDeps,
|
deps: DuplicateDetectionDeps,
|
||||||
|
maxMatches?: number,
|
||||||
): Promise<number[]> {
|
): Promise<number[]> {
|
||||||
const candidates = Array.from(candidateNoteIds).filter((id) => id !== excludeNoteId);
|
const candidates = Array.from(candidateNoteIds).filter((id) => id !== excludeNoteId);
|
||||||
deps.logDebug?.(`[duplicate] candidateIds=${candidates.length} exclude=${excludeNoteId}`);
|
deps.logDebug?.(`[duplicate] candidateIds=${candidates.length} exclude=${excludeNoteId}`);
|
||||||
@@ -145,6 +154,9 @@ function findExactDuplicateNoteIds(
|
|||||||
);
|
);
|
||||||
deps.logInfo?.(`[duplicate] matched noteId=${noteInfo.noteId} field=${resolvedField}`);
|
deps.logInfo?.(`[duplicate] matched noteId=${noteInfo.noteId} field=${resolvedField}`);
|
||||||
matches.push(noteInfo.noteId);
|
matches.push(noteInfo.noteId);
|
||||||
|
if (maxMatches !== undefined && matches.length >= maxMatches) {
|
||||||
|
return matches;
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1390,3 +1390,32 @@ test('addYomitanNoteViaSearch returns note and duplicate ids from the bridge pay
|
|||||||
duplicateNoteIds: [18, 7, 18],
|
duplicateNoteIds: [18, 7, 18],
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('addYomitanNoteViaSearch rejects invalid numeric note ids from the bridge shortcut', async () => {
|
||||||
|
const deps = createDeps(async () => NaN);
|
||||||
|
|
||||||
|
const result = await addYomitanNoteViaSearch('食べる', deps, {
|
||||||
|
error: () => undefined,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.deepEqual(result, {
|
||||||
|
noteId: null,
|
||||||
|
duplicateNoteIds: [],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('addYomitanNoteViaSearch sanitizes invalid payload note ids while keeping valid duplicate ids', async () => {
|
||||||
|
const deps = createDeps(async (_script) => ({
|
||||||
|
noteId: -1,
|
||||||
|
duplicateNoteIds: [18, 0, 7.5, 7],
|
||||||
|
}));
|
||||||
|
|
||||||
|
const result = await addYomitanNoteViaSearch('食べる', deps, {
|
||||||
|
error: () => undefined,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.deepEqual(result, {
|
||||||
|
noteId: null,
|
||||||
|
duplicateNoteIds: [18, 7],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -2010,7 +2010,10 @@ export async function addYomitanNoteViaSearch(
|
|||||||
try {
|
try {
|
||||||
const result = await parserWindow.webContents.executeJavaScript(script, true);
|
const result = await parserWindow.webContents.executeJavaScript(script, true);
|
||||||
if (typeof result === 'number') {
|
if (typeof result === 'number') {
|
||||||
return { noteId: result, duplicateNoteIds: [] };
|
return {
|
||||||
|
noteId: Number.isInteger(result) && result > 0 ? result : null,
|
||||||
|
duplicateNoteIds: [],
|
||||||
|
};
|
||||||
}
|
}
|
||||||
if (result && typeof result === 'object' && !Array.isArray(result)) {
|
if (result && typeof result === 'object' && !Array.isArray(result)) {
|
||||||
const envelope = result as {
|
const envelope = result as {
|
||||||
@@ -2018,7 +2021,12 @@ export async function addYomitanNoteViaSearch(
|
|||||||
duplicateNoteIds?: unknown;
|
duplicateNoteIds?: unknown;
|
||||||
};
|
};
|
||||||
return {
|
return {
|
||||||
noteId: typeof envelope.noteId === 'number' ? envelope.noteId : null,
|
noteId:
|
||||||
|
typeof envelope.noteId === 'number' &&
|
||||||
|
Number.isInteger(envelope.noteId) &&
|
||||||
|
envelope.noteId > 0
|
||||||
|
? envelope.noteId
|
||||||
|
: null,
|
||||||
duplicateNoteIds: Array.isArray(envelope.duplicateNoteIds)
|
duplicateNoteIds: Array.isArray(envelope.duplicateNoteIds)
|
||||||
? envelope.duplicateNoteIds.filter(
|
? envelope.duplicateNoteIds.filter(
|
||||||
(entry): entry is number => typeof entry === 'number' && Number.isInteger(entry) && entry > 0,
|
(entry): entry is number => typeof entry === 'number' && Number.isInteger(entry) && entry > 0,
|
||||||
|
|||||||
@@ -549,6 +549,31 @@ test('paused configured subtitle-jump keybinding re-applies pause after backward
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('configured subtitle-jump keybinding preserves pause when pause state is unknown', async () => {
|
||||||
|
const { handlers, testGlobals } = createKeyboardHandlerHarness();
|
||||||
|
|
||||||
|
try {
|
||||||
|
await handlers.setupMpvInputForwarding();
|
||||||
|
handlers.updateKeybindings([
|
||||||
|
{
|
||||||
|
key: 'Shift+KeyH',
|
||||||
|
command: ['sub-seek', -1],
|
||||||
|
},
|
||||||
|
] as never);
|
||||||
|
testGlobals.setPlaybackPausedResponse(null);
|
||||||
|
|
||||||
|
testGlobals.dispatchKeydown({ key: 'H', code: 'KeyH', shiftKey: true });
|
||||||
|
await wait(0);
|
||||||
|
|
||||||
|
assert.deepEqual(testGlobals.mpvCommands.slice(-2), [
|
||||||
|
['sub-seek', -1],
|
||||||
|
['set_property', 'pause', 'yes'],
|
||||||
|
]);
|
||||||
|
} finally {
|
||||||
|
testGlobals.restore();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
test('visible-layer y-t dispatches mpv plugin toggle while overlay owns focus', async () => {
|
test('visible-layer y-t dispatches mpv plugin toggle while overlay owns focus', async () => {
|
||||||
const { handlers, testGlobals } = createKeyboardHandlerHarness();
|
const { handlers, testGlobals } = createKeyboardHandlerHarness();
|
||||||
|
|
||||||
|
|||||||
@@ -376,7 +376,7 @@ export function createKeyboardHandlers(
|
|||||||
.getPlaybackPaused()
|
.getPlaybackPaused()
|
||||||
.then((paused) => {
|
.then((paused) => {
|
||||||
window.electronAPI.sendMpvCommand(command);
|
window.electronAPI.sendMpvCommand(command);
|
||||||
if (paused === true) {
|
if (paused !== false) {
|
||||||
window.electronAPI.sendMpvCommand(['set_property', 'pause', 'yes']);
|
window.electronAPI.sendMpvCommand(['set_property', 'pause', 'yes']);
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user