fix(anki): avoid unintended kiku grouping on lookup

This commit is contained in:
2026-02-28 02:19:27 -08:00
parent 40787e8b71
commit 870acb45d5
7 changed files with 188 additions and 23 deletions

View File

@@ -284,3 +284,35 @@ test('FieldGroupingMergeCollaborator uses generated media fallback when source l
assert.equal(merged.SentenceAudio, '<span data-group-id="22">[sound:generated.mp3]</span>');
});
test('FieldGroupingMergeCollaborator deduplicates identical sentence, audio, and image values when merging into a new duplicate card', async () => {
const collaborator = createFieldGroupingMergeCollaborator();
const merged = await collaborator.computeFieldGroupingMergedFields(
202,
101,
{
noteId: 202,
fields: {
Sentence: { value: 'same sentence' },
SentenceAudio: { value: '[sound:same.mp3]' },
Picture: { value: '<img src="same.png">' },
ExpressionAudio: { value: '[sound:same.mp3]' },
},
},
{
noteId: 101,
fields: {
Sentence: { value: 'same sentence' },
SentenceAudio: { value: '[sound:same.mp3]' },
Picture: { value: '<img src="same.png">' },
},
},
false,
);
assert.equal(merged.Sentence, '<span data-group-id="202">same sentence</span>');
assert.equal(merged.SentenceAudio, '<span data-group-id="202">[sound:same.mp3]</span>');
assert.equal(merged.Picture, '<img data-group-id="202" src="same.png">');
assert.equal(merged.ExpressionAudio, merged.SentenceAudio);
});

View File

@@ -230,6 +230,41 @@ test('proxy ignores addNote when upstream response reports error', async () => {
assert.deepEqual(processed, []);
});
test('proxy does not fallback-enqueue latest note for multi requests without add actions', async () => {
const processed: number[] = [];
const findNotesQueries: string[] = [];
const proxy = new AnkiConnectProxyServer({
shouldAutoUpdateNewCards: () => true,
processNewCard: async (noteId) => {
processed.push(noteId);
},
getDeck: () => 'Mining',
findNotes: async (query) => {
findNotesQueries.push(query);
return [999];
},
logInfo: () => undefined,
logWarn: () => undefined,
logError: () => undefined,
});
(proxy as unknown as {
maybeEnqueueFromRequest: (request: Record<string, unknown>, responseBody: Buffer) => void;
}).maybeEnqueueFromRequest(
{
action: 'multi',
params: {
actions: [{ action: 'version' }, { action: 'deckNames' }],
},
},
Buffer.from(JSON.stringify({ result: [6, ['Default']], error: null }), 'utf8'),
);
await new Promise((resolve) => setTimeout(resolve, 30));
assert.deepEqual(findNotesQueries, []);
assert.deepEqual(processed, []);
});
test('proxy detects self-referential loop configuration', () => {
const proxy = new AnkiConnectProxyServer({
shouldAutoUpdateNewCards: () => true,

View File

@@ -179,6 +179,7 @@ export class AnkiConnectProxyServer {
if (action !== 'addNote' && action !== 'addNotes' && action !== 'multi') {
return;
}
const shouldFallbackToLatestAdded = this.requestIncludesAddAction(action, requestJson);
const parsedResponse = this.tryParseJsonValue(responseBody);
if (parsedResponse === null || parsedResponse === undefined) {
@@ -194,7 +195,7 @@ export class AnkiConnectProxyServer {
action === 'multi'
? this.collectMultiResultIds(requestJson, responseResult)
: this.collectNoteIdsForAction(action, responseResult);
if (noteIds.length === 0) {
if (noteIds.length === 0 && shouldFallbackToLatestAdded) {
void this.enqueueMostRecentAddedNote();
return;
}
@@ -202,6 +203,28 @@ export class AnkiConnectProxyServer {
this.enqueueNotes(noteIds);
}
private requestIncludesAddAction(action: string, requestJson: Record<string, unknown>): boolean {
if (action === 'addNote' || action === 'addNotes') {
return true;
}
if (action !== 'multi') {
return false;
}
const params =
requestJson.params && typeof requestJson.params === 'object'
? (requestJson.params as Record<string, unknown>)
: null;
const actions = Array.isArray(params?.actions) ? params.actions : [];
if (actions.length === 0) {
return false;
}
return actions.some((entry) => {
if (!entry || typeof entry !== 'object') return false;
const actionName = (entry as Record<string, unknown>).action;
return actionName === 'addNote' || actionName === 'addNotes';
});
}
private async enqueueMostRecentAddedNote(): Promise<void> {
const findNotes = this.deps.findNotes;
if (!findNotes) {

View File

@@ -302,7 +302,7 @@ export class FieldGroupingMergeCollaborator {
const unique: { groupId: number; content: string }[] = [];
const seen = new Set<string>();
for (const entry of entries) {
const key = `${entry.groupId}::${entry.content}`;
const key = entry.content;
if (seen.has(key)) continue;
seen.add(key);
unique.push(entry);
@@ -361,6 +361,10 @@ export class FieldGroupingMergeCollaborator {
return ungrouped;
}
private getPictureDedupKey(tag: string): string {
return tag.replace(/\sdata-group-id="[^"]*"/gi, '').trim();
}
private getStrictSpanGroupingFields(): Set<string> {
const strictFields = new Set(this.strictGroupingFieldDefaults);
const sentenceCardConfig = this.deps.getEffectiveSentenceCardConfig();
@@ -394,11 +398,12 @@ export class FieldGroupingMergeCollaborator {
const mergedTags = keepEntries.map((entry) =>
this.ensureImageGroupId(entry.tag, entry.groupId),
);
const seen = new Set(mergedTags);
const seen = new Set(mergedTags.map((tag) => this.getPictureDedupKey(tag)));
for (const entry of sourceEntries) {
const normalized = this.ensureImageGroupId(entry.tag, entry.groupId);
if (seen.has(normalized)) continue;
seen.add(normalized);
const dedupKey = this.getPictureDedupKey(normalized);
if (seen.has(dedupKey)) continue;
seen.add(dedupKey);
mergedTags.push(normalized);
}
return mergedTags.join('');
@@ -415,9 +420,9 @@ export class FieldGroupingMergeCollaborator {
.join('');
}
const merged = [...keepEntries];
const seen = new Set(keepEntries.map((entry) => `${entry.groupId}::${entry.content}`));
const seen = new Set(keepEntries.map((entry) => entry.content));
for (const entry of sourceEntries) {
const key = `${entry.groupId}::${entry.content}`;
const key = entry.content;
if (seen.has(key)) continue;
seen.add(key);
merged.push(entry);

View File

@@ -1,16 +1,36 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import { FieldGroupingWorkflow } from './field-grouping-workflow';
import type { KikuDuplicateCardInfo, KikuFieldGroupingChoice } from '../types';
type NoteInfo = {
noteId: number;
fields: Record<string, { value: string }>;
};
type ManualChoice = {
keepNoteId: number;
deleteNoteId: number;
deleteDuplicate: boolean;
cancelled: boolean;
};
type FieldGroupingCallback = (data: {
original: KikuDuplicateCardInfo;
duplicate: KikuDuplicateCardInfo;
}) => Promise<KikuFieldGroupingChoice>;
function createWorkflowHarness() {
const updates: Array<{ noteId: number; fields: Record<string, string> }> = [];
const deleted: number[][] = [];
const statuses: string[] = [];
const mergeCalls: Array<{
keepNoteId: number;
deleteNoteId: number;
keepNoteInfoNoteId: number;
deleteNoteInfoNoteId: number;
}> = [];
let manualChoice: ManualChoice | null = null;
const deps = {
client: {
@@ -47,11 +67,28 @@ function createWorkflowHarness() {
kikuDeleteDuplicateInAuto: true,
}),
getCurrentSubtitleText: () => 'subtitle-text',
getFieldGroupingCallback: () => null,
getFieldGroupingCallback: (): FieldGroupingCallback | null => {
const choice = manualChoice;
if (choice === null) return null;
return async () => choice;
},
setFieldGroupingCallback: () => undefined,
computeFieldGroupingMergedFields: async () => ({
Sentence: 'merged sentence',
}),
computeFieldGroupingMergedFields: async (
keepNoteId: number,
deleteNoteId: number,
keepNoteInfo: NoteInfo,
deleteNoteInfo: NoteInfo,
) => {
mergeCalls.push({
keepNoteId,
deleteNoteId,
keepNoteInfoNoteId: keepNoteInfo.noteId,
deleteNoteInfoNoteId: deleteNoteInfo.noteId,
});
return {
Sentence: 'merged sentence',
};
},
extractFields: (fields: Record<string, { value: string }>) => {
const out: Record<string, string> = {};
for (const [key, value] of Object.entries(fields)) {
@@ -77,6 +114,10 @@ function createWorkflowHarness() {
updates,
deleted,
statuses,
mergeCalls,
setManualChoice: (choice: typeof manualChoice) => {
manualChoice = choice;
},
deps,
};
}
@@ -112,3 +153,31 @@ test('FieldGroupingWorkflow manual mode returns false when callback unavailable'
assert.equal(handled, false);
assert.equal(harness.updates.length, 0);
});
test('FieldGroupingWorkflow manual keep-new uses new note as merge target and old note as source', async () => {
const harness = createWorkflowHarness();
harness.setManualChoice({
keepNoteId: 2,
deleteNoteId: 1,
deleteDuplicate: false,
cancelled: false,
});
const handled = await harness.workflow.handleManual(1, 2, {
noteId: 2,
fields: {
Expression: { value: 'word-2' },
Sentence: { value: 'line-2' },
},
});
assert.equal(handled, true);
assert.deepEqual(harness.mergeCalls, [
{
keepNoteId: 2,
deleteNoteId: 1,
keepNoteInfoNoteId: 2,
deleteNoteInfoNoteId: 1,
},
]);
});

View File

@@ -69,7 +69,6 @@ export class FieldGroupingWorkflow {
await this.performMerge(
originalNoteId,
newNoteId,
newNoteInfo,
this.getExpression(newNoteInfo),
sentenceCardConfig.kikuDeleteDuplicateInAuto,
);
@@ -112,12 +111,10 @@ export class FieldGroupingWorkflow {
const keepNoteId = choice.keepNoteId;
const deleteNoteId = choice.deleteNoteId;
const deleteNoteInfo = deleteNoteId === newNoteId ? newNoteInfo : originalNoteInfo;
await this.performMerge(
keepNoteId,
deleteNoteId,
deleteNoteInfo,
expression,
choice.deleteDuplicate,
);
@@ -132,18 +129,22 @@ export class FieldGroupingWorkflow {
private async performMerge(
keepNoteId: number,
deleteNoteId: number,
deleteNoteInfo: FieldGroupingWorkflowNoteInfo,
expression: string,
deleteDuplicate = true,
): Promise<void> {
const keepNotesInfoResult = await this.deps.client.notesInfo([keepNoteId]);
const keepNotesInfo = keepNotesInfoResult as FieldGroupingWorkflowNoteInfo[];
if (!keepNotesInfo || keepNotesInfo.length === 0) {
const notesInfoResult = await this.deps.client.notesInfo([keepNoteId, deleteNoteId]);
const notesInfo = notesInfoResult as FieldGroupingWorkflowNoteInfo[];
const keepNoteInfo = notesInfo.find((note) => note.noteId === keepNoteId);
const deleteNoteInfo = notesInfo.find((note) => note.noteId === deleteNoteId);
if (!keepNoteInfo) {
this.deps.logInfo('Keep note not found:', keepNoteId);
return;
}
if (!deleteNoteInfo) {
this.deps.logInfo('Delete note not found:', deleteNoteId);
return;
}
const keepNoteInfo = keepNotesInfo[0]!;
const mergedFields = await this.deps.computeFieldGroupingMergedFields(
keepNoteId,
deleteNoteId,