fix: address latest PR review feedback

This commit is contained in:
2026-03-19 20:06:52 -07:00
parent 9ad3ccfa38
commit 1227706ac9
11 changed files with 306 additions and 33 deletions

View File

@@ -1048,10 +1048,6 @@ export class AnkiIntegration {
return getConfiguredWordFieldCandidates(this.config);
}
private getPreferredWordValue(fields: Record<string, string>): string {
return getPreferredWordValueFromExtractedFields(fields, this.config);
}
private async getAnimatedImageLeadInSeconds(noteInfo: NoteInfo): Promise<number> {
return resolveAnimatedImageLeadInSeconds({
config: this.config,

View File

@@ -7,6 +7,21 @@ import path from 'node:path';
import type { AnkiConnectConfig } from '../types';
import { KnownWordCacheManager } from './known-word-cache';
async function waitForCondition(
condition: () => boolean,
timeoutMs = 500,
intervalMs = 10,
): Promise<void> {
const startedAt = Date.now();
while (Date.now() - startedAt < timeoutMs) {
if (condition()) {
return;
}
await new Promise((resolve) => setTimeout(resolve, intervalMs));
}
throw new Error('Timed out waiting for condition');
}
function createKnownWordCacheHarness(config: AnkiConnectConfig): {
manager: KnownWordCacheManager;
calls: {
@@ -17,6 +32,7 @@ function createKnownWordCacheHarness(config: AnkiConnectConfig): {
clientState: {
findNotesResult: number[];
notesInfoResult: Array<{ noteId: number; fields: Record<string, { value: string }> }>;
findNotesByQuery: Map<string, number[]>;
};
cleanup: () => void;
} {
@@ -29,11 +45,15 @@ function createKnownWordCacheHarness(config: AnkiConnectConfig): {
const clientState = {
findNotesResult: [] as number[],
notesInfoResult: [] as Array<{ noteId: number; fields: Record<string, { value: string }> }>,
findNotesByQuery: new Map<string, number[]>(),
};
const manager = new KnownWordCacheManager({
client: {
findNotes: async () => {
findNotes: async (query) => {
calls.findNotes += 1;
if (clientState.findNotesByQuery.has(query)) {
return clientState.findNotesByQuery.get(query) ?? [];
}
return clientState.findNotesResult;
},
notesInfo: async (noteIds) => {
@@ -57,10 +77,11 @@ function createKnownWordCacheHarness(config: AnkiConnectConfig): {
};
}
test('KnownWordCacheManager startLifecycle loads persisted cache without immediate rebuild', () => {
test('KnownWordCacheManager startLifecycle keeps fresh persisted cache without immediate refresh', async () => {
const config: AnkiConnectConfig = {
knownWords: {
highlightEnabled: true,
refreshMinutes: 60,
},
};
const { manager, calls, statePath, cleanup } = createKnownWordCacheHarness(config);
@@ -71,7 +92,7 @@ test('KnownWordCacheManager startLifecycle loads persisted cache without immedia
JSON.stringify({
version: 2,
refreshedAtMs: Date.now(),
scope: '{"refreshMinutes":1440,"scope":"is:note","fieldsWord":""}',
scope: '{"refreshMinutes":60,"scope":"is:note","fieldsWord":""}',
words: ['猫'],
notes: {
'1': ['猫'],
@@ -81,6 +102,7 @@ test('KnownWordCacheManager startLifecycle loads persisted cache without immedia
);
manager.startLifecycle();
await new Promise((resolve) => setTimeout(resolve, 25));
assert.equal(manager.isKnownWord('猫'), true);
assert.equal(calls.findNotes, 0);
@@ -91,6 +113,54 @@ test('KnownWordCacheManager startLifecycle loads persisted cache without immedia
}
});
test('KnownWordCacheManager startLifecycle immediately refreshes stale persisted cache', async () => {
const config: AnkiConnectConfig = {
fields: {
word: 'Word',
},
knownWords: {
highlightEnabled: true,
refreshMinutes: 1,
},
};
const { manager, calls, statePath, clientState, cleanup } = createKnownWordCacheHarness(config);
try {
fs.writeFileSync(
statePath,
JSON.stringify({
version: 2,
refreshedAtMs: Date.now() - 61_000,
scope: '{"refreshMinutes":1,"scope":"is:note","fieldsWord":"Word"}',
words: ['猫'],
notes: {
'1': ['猫'],
},
}),
'utf-8',
);
clientState.findNotesResult = [1];
clientState.notesInfoResult = [
{
noteId: 1,
fields: {
Word: { value: '犬' },
},
},
];
manager.startLifecycle();
await waitForCondition(() => calls.findNotes === 1 && calls.notesInfo === 1);
assert.equal(manager.isKnownWord('猫'), false);
assert.equal(manager.isKnownWord('犬'), true);
} finally {
manager.stopLifecycle();
cleanup();
}
});
test('KnownWordCacheManager invalidates persisted cache when fields.word changes', () => {
const config: AnkiConnectConfig = {
deck: 'Mining',
@@ -235,6 +305,49 @@ test('KnownWordCacheManager invalidates persisted cache when per-deck fields cha
}
});
test('KnownWordCacheManager preserves deck-specific field mappings during refresh', async () => {
const config: AnkiConnectConfig = {
knownWords: {
highlightEnabled: true,
decks: {
Mining: ['Expression'],
Reading: ['Word'],
},
},
};
const { manager, clientState, cleanup } = createKnownWordCacheHarness(config);
try {
clientState.findNotesByQuery.set('deck:"Mining"', [1]);
clientState.findNotesByQuery.set('deck:"Reading"', [2]);
clientState.notesInfoResult = [
{
noteId: 1,
fields: {
Expression: { value: '猫' },
Word: { value: 'should-not-count' },
},
},
{
noteId: 2,
fields: {
Word: { value: '犬' },
Expression: { value: 'also-ignored' },
},
},
];
await manager.refresh(true);
assert.equal(manager.isKnownWord('猫'), true);
assert.equal(manager.isKnownWord('犬'), true);
assert.equal(manager.isKnownWord('should-not-count'), false);
assert.equal(manager.isKnownWord('also-ignored'), false);
} finally {
cleanup();
}
});
test('KnownWordCacheManager skips immediate append when addMinedWordsImmediately is disabled', () => {
const config: AnkiConnectConfig = {
knownWords: {

View File

@@ -98,6 +98,11 @@ interface KnownWordCacheDeps {
showStatusNotification: (message: string) => void;
}
type KnownWordQueryScope = {
query: string;
fields: string[];
};
export class KnownWordCacheManager {
private knownWordsLastRefreshedAtMs = 0;
private knownWordsStateKey = '';
@@ -219,18 +224,11 @@ export class KnownWordCacheManager {
this.isRefreshingKnownWords = true;
try {
const query = this.buildKnownWordsQuery();
log.debug('Refreshing known-word cache', `query=${query}`);
const noteIds = (await this.deps.client.findNotes(query, {
maxRetries: 0,
})) as number[];
const currentNoteIds = Array.from(
new Set(noteIds.filter((noteId) => Number.isInteger(noteId) && noteId > 0)),
).sort((a, b) => a - b);
const noteFieldsById = await this.fetchKnownWordNoteFieldsById();
const currentNoteIds = Array.from(noteFieldsById.keys()).sort((a, b) => a - b);
if (this.noteWordsById.size === 0) {
await this.rebuildFromCurrentNotes(currentNoteIds);
await this.rebuildFromCurrentNotes(currentNoteIds, noteFieldsById);
} else {
const currentNoteIdSet = new Set(currentNoteIds);
for (const noteId of Array.from(this.noteWordsById.keys())) {
@@ -244,7 +242,10 @@ export class KnownWordCacheManager {
for (const noteInfo of noteInfos) {
this.replaceNoteSnapshot(
noteInfo.noteId,
this.extractNormalizedKnownWordsFromNoteInfo(noteInfo),
this.extractNormalizedKnownWordsFromNoteInfo(
noteInfo,
noteFieldsById.get(noteInfo.noteId),
),
);
}
}
@@ -307,6 +308,31 @@ export class KnownWordCacheManager {
return [...new Set([configuredWordField, 'Word', 'Reading', 'Word Reading'])];
}
private getKnownWordQueryScopes(): KnownWordQueryScope[] {
const configuredDecks = this.deps.getConfig().knownWords?.decks;
if (configuredDecks && typeof configuredDecks === 'object' && !Array.isArray(configuredDecks)) {
const scopes: KnownWordQueryScope[] = [];
for (const [deckName, fields] of Object.entries(configuredDecks)) {
const trimmedDeckName = deckName.trim();
if (!trimmedDeckName) {
continue;
}
const normalizedFields = Array.isArray(fields)
? [...new Set(fields.map(String).map((field) => field.trim()).filter(Boolean))]
: [];
scopes.push({
query: `deck:"${escapeAnkiSearchValue(trimmedDeckName)}"`,
fields: normalizedFields.length > 0 ? normalizedFields : this.getConfiguredFields(),
});
}
if (scopes.length > 0) {
return scopes;
}
}
return [{ query: this.buildKnownWordsQuery(), fields: this.getConfiguredFields() }];
}
private buildKnownWordsQuery(): string {
const decks = this.getKnownWordDecks();
if (decks.length === 0) {
@@ -338,6 +364,31 @@ export class KnownWordCacheManager {
return Date.now() - this.knownWordsLastRefreshedAtMs >= this.getKnownWordRefreshIntervalMs();
}
private async fetchKnownWordNoteFieldsById(): Promise<Map<number, string[]>> {
const scopes = this.getKnownWordQueryScopes();
const noteFieldsById = new Map<number, string[]>();
log.debug('Refreshing known-word cache', `queries=${scopes.map((scope) => scope.query).join(' | ')}`);
for (const scope of scopes) {
const noteIds = (await this.deps.client.findNotes(scope.query, {
maxRetries: 0,
})) as number[];
for (const noteId of noteIds) {
if (!Number.isInteger(noteId) || noteId <= 0) {
continue;
}
const existingFields = noteFieldsById.get(noteId) ?? [];
noteFieldsById.set(
noteId,
[...new Set([...existingFields, ...scope.fields])],
);
}
}
return noteFieldsById;
}
private scheduleKnownWordRefreshLifecycle(): void {
const refreshIntervalMs = this.getKnownWordRefreshIntervalMs();
const scheduleInterval = () => {
@@ -366,7 +417,10 @@ export class KnownWordCacheManager {
return Math.max(0, remainingMs);
}
private async rebuildFromCurrentNotes(noteIds: number[]): Promise<void> {
private async rebuildFromCurrentNotes(
noteIds: number[],
noteFieldsById: Map<number, string[]>,
): Promise<void> {
this.clearInMemoryState();
if (noteIds.length === 0) {
return;
@@ -374,7 +428,10 @@ export class KnownWordCacheManager {
const noteInfos = await this.fetchKnownWordNotesInfo(noteIds);
for (const noteInfo of noteInfos) {
this.replaceNoteSnapshot(noteInfo.noteId, this.extractNormalizedKnownWordsFromNoteInfo(noteInfo));
this.replaceNoteSnapshot(
noteInfo.noteId,
this.extractNormalizedKnownWordsFromNoteInfo(noteInfo, noteFieldsById.get(noteInfo.noteId)),
);
}
}
@@ -562,10 +619,12 @@ export class KnownWordCacheManager {
return true;
}
private extractNormalizedKnownWordsFromNoteInfo(noteInfo: KnownWordCacheNoteInfo): string[] {
private extractNormalizedKnownWordsFromNoteInfo(
noteInfo: KnownWordCacheNoteInfo,
preferredFields = this.getConfiguredFields(),
): string[] {
const words: string[] = [];
const configuredFields = this.getConfiguredFields();
for (const preferredField of configuredFields) {
for (const preferredField of preferredFields) {
const fieldName = resolveFieldName(Object.keys(noteInfo.fields), preferredField);
if (!fieldName) continue;