fix: address latest claude review findings

This commit is contained in:
2026-02-28 20:07:42 -08:00
parent fd77f8f6a2
commit 55c577e911
7 changed files with 137 additions and 10 deletions

View File

@@ -265,6 +265,36 @@ test('proxy does not fallback-enqueue latest note for multi requests without add
assert.deepEqual(processed, []);
});
test('proxy fallback-enqueues latest note for addNote responses without note IDs and escapes deck quotes', async () => {
const processed: number[] = [];
const findNotesQueries: string[] = [];
const proxy = new AnkiConnectProxyServer({
shouldAutoUpdateNewCards: () => true,
processNewCard: async (noteId) => {
processed.push(noteId);
},
getDeck: () => 'My "Japanese" Deck',
findNotes: async (query) => {
findNotesQueries.push(query);
return [500, 501];
},
logInfo: () => undefined,
logWarn: () => undefined,
logError: () => undefined,
});
(proxy as unknown as {
maybeEnqueueFromRequest: (request: Record<string, unknown>, responseBody: Buffer) => void;
}).maybeEnqueueFromRequest(
{ action: 'addNote' },
Buffer.from(JSON.stringify({ result: 0, error: null }), 'utf8'),
);
await waitForCondition(() => processed.length === 1);
assert.deepEqual(findNotesQueries, ['"deck:My \\"Japanese\\" Deck" added:1']);
assert.deepEqual(processed, [501]);
});
test('proxy detects self-referential loop configuration', () => {
const proxy = new AnkiConnectProxyServer({
shouldAutoUpdateNewCards: () => true,

View File

@@ -233,7 +233,8 @@ export class AnkiConnectProxyServer {
try {
const deck = this.deps.getDeck ? this.deps.getDeck() : undefined;
const query = deck ? `"deck:${deck}" added:1` : 'added:1';
const escapedDeck = deck ? deck.replace(/"/g, '\\"') : undefined;
const query = escapedDeck ? `"deck:${escapedDeck}" added:1` : 'added:1';
const noteIds = await findNotes(query, { maxRetries: 0 });
if (!noteIds || noteIds.length === 0) {
return;

View File

@@ -81,6 +81,7 @@ function isExcludedByTagSet(
if (parts.length === 0) {
return false;
}
// Composite tags like "助詞|名詞" stay eligible unless every component is excluded.
return parts.every((part) => exclusions.has(part));
}

View File

@@ -13,6 +13,17 @@ function shouldAutoConnectJellyfinRemote(config: {
return config.enabled && config.remoteControlEnabled && config.remoteControlAutoConnect;
}
function createDeferred(): {
promise: Promise<void>;
resolve: () => void;
} {
let resolve!: () => void;
const promise = new Promise<void>((nextResolve) => {
resolve = nextResolve;
});
return { promise, resolve };
}
test('launchBackgroundWarmupTask logs completion timing', async () => {
const debugLogs: string[] = [];
const launchTask = createLaunchBackgroundWarmupTaskHandler({
@@ -194,3 +205,60 @@ test('startBackgroundWarmups logs per-stage progress for enabled tokenization wa
assert.ok(debugLogs.includes('[startup-warmup] stage start: jellyfin-remote-session'));
assert.ok(debugLogs.includes('[startup-warmup] stage ready: jellyfin-remote-session'));
});
test('startBackgroundWarmups starts mecab and dictionary warmups without waiting for yomitan warmup', async () => {
const startedStages: string[] = [];
let started = false;
let subtitleTokenizationTask: Promise<void> | null = null;
const yomitanDeferred = createDeferred();
const mecabDeferred = createDeferred();
const subtitleDictionariesDeferred = createDeferred();
const startWarmups = createStartBackgroundWarmupsHandler({
getStarted: () => started,
setStarted: (value) => {
started = value;
},
isTexthookerOnlyMode: () => false,
launchTask: (label, task) => {
if (label === 'subtitle-tokenization') {
subtitleTokenizationTask = task();
}
},
createMecabTokenizerAndCheck: async () => {
startedStages.push('mecab');
await mecabDeferred.promise;
},
ensureYomitanExtensionLoaded: async () => {
startedStages.push('yomitan-extension');
await yomitanDeferred.promise;
},
prewarmSubtitleDictionaries: async () => {
startedStages.push('subtitle-dictionaries');
await subtitleDictionariesDeferred.promise;
},
shouldWarmupMecab: () => true,
shouldWarmupYomitanExtension: () => true,
shouldWarmupSubtitleDictionaries: () => true,
shouldWarmupJellyfinRemoteSession: () => false,
shouldAutoConnectJellyfinRemote: () => false,
startJellyfinRemoteSession: async () => {},
});
startWarmups();
await Promise.resolve();
await Promise.resolve();
assert.ok(subtitleTokenizationTask);
assert.equal(startedStages.includes('yomitan-extension'), true);
assert.equal(startedStages.includes('mecab'), true);
assert.equal(startedStages.includes('subtitle-dictionaries'), true);
yomitanDeferred.resolve();
mecabDeferred.resolve();
subtitleDictionariesDeferred.resolve();
if (!subtitleTokenizationTask) {
throw new Error('Expected subtitle tokenization warmup task');
}
await subtitleTokenizationTask;
});

View File

@@ -47,15 +47,16 @@ export function createStartBackgroundWarmupsHandler(deps: {
warmupMecab || warmupYomitanExtension || warmupSubtitleDictionaries;
if (shouldWarmupTokenization) {
deps.launchTask('subtitle-tokenization', async () => {
if (warmupYomitanExtension) {
deps.logDebug?.('[startup-warmup] stage start: yomitan-extension');
await deps.ensureYomitanExtensionLoaded();
deps.logDebug?.('[startup-warmup] stage ready: yomitan-extension');
} else {
deps.logDebug?.('[startup-warmup] stage skipped: yomitan-extension');
}
await Promise.all([
warmupYomitanExtension
? (async () => {
deps.logDebug?.('[startup-warmup] stage start: yomitan-extension');
await deps.ensureYomitanExtensionLoaded();
deps.logDebug?.('[startup-warmup] stage ready: yomitan-extension');
})()
: Promise.resolve().then(() => {
deps.logDebug?.('[startup-warmup] stage skipped: yomitan-extension');
}),
warmupMecab
? (async () => {
deps.logDebug?.('[startup-warmup] stage start: mecab');

View File

@@ -180,3 +180,29 @@ test('dictionary prewarm does not show OSD when notifications are disabled', asy
assert.deepEqual(osdMessages, []);
});
test('dictionary prewarm clears loading OSD timer even if notifications are disabled before completion', async () => {
const clearedTimers: unknown[] = [];
const jlptDeferred = createDeferred();
const freqDeferred = createDeferred();
let shouldShowNotification = true;
const prewarm = createPrewarmSubtitleDictionariesMainHandler({
ensureJlptDictionaryLookup: async () => jlptDeferred.promise,
ensureFrequencyDictionaryLookup: async () => freqDeferred.promise,
shouldShowOsdNotification: () => shouldShowNotification,
showMpvOsd: () => undefined,
setInterval: () => 'loading-timer',
clearInterval: (timer) => {
clearedTimers.push(timer);
},
});
const prewarmPromise = prewarm({ showLoadingOsd: true });
shouldShowNotification = false;
jlptDeferred.resolve();
freqDeferred.resolve();
await prewarmPromise;
assert.deepEqual(clearedTimers, ['loading-timer']);
});

View File

@@ -113,7 +113,7 @@ export function createPrewarmSubtitleDictionariesMainHandler(deps: {
};
const endLoadingOsd = (): void => {
if (!showMpvOsd || !shouldShowOsdNotification()) {
if (!showMpvOsd) {
return;
}