diff --git a/docs/anki-integration.md b/docs/anki-integration.md index 327d09d..65c307b 100644 --- a/docs/anki-integration.md +++ b/docs/anki-integration.md @@ -267,7 +267,7 @@ When you mine the same word multiple times, SubMiner can merge the cards instead **Disabled** (`"disabled"`): No duplicate detection. Each card is independent. -**Auto** (`"auto"`): When a duplicate expression is found, SubMiner merges the new card into the existing one automatically. Both sentences, audio clips, and images are preserved. If `deleteDuplicateInAuto` is true, the new card is deleted after merging. +**Auto** (`"auto"`): When a duplicate expression is found, SubMiner merges the new card into the existing one automatically. Both sentences, audio clips, and images are preserved, and exact duplicate values are collapsed to one entry. If `deleteDuplicateInAuto` is true, the new card is deleted after merging. **Manual** (`"manual"`): A modal appears in the overlay showing both cards. You choose which card to keep, preview the merge result, then confirm. The modal has a 90-second timeout, after which it cancels automatically. @@ -275,9 +275,9 @@ When you mine the same word multiple times, SubMiner can merge the cards instead | Field | Merge behavior | | -------- | -------------------------------------------------------------- | -| Sentence | Both sentences preserved, labeled `[Original]` / `[Duplicate]` | -| Audio | Both `[sound:...]` entries kept | -| Image | Both images kept | +| Sentence | Both sentences preserved (exact duplicate text is deduplicated) | +| Audio | Both `[sound:...]` entries kept (exact duplicates deduplicated) | +| Image | Both images kept (exact duplicates deduplicated) | ### Keyboard Shortcuts in the Modal diff --git a/src/anki-integration.test.ts b/src/anki-integration.test.ts index a955400..519d3e3 100644 --- a/src/anki-integration.test.ts +++ b/src/anki-integration.test.ts @@ -284,3 +284,35 @@ test('FieldGroupingMergeCollaborator uses generated media fallback when source l assert.equal(merged.SentenceAudio, '[sound:generated.mp3]'); }); + +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: '' }, + ExpressionAudio: { value: '[sound:same.mp3]' }, + }, + }, + { + noteId: 101, + fields: { + Sentence: { value: 'same sentence' }, + SentenceAudio: { value: '[sound:same.mp3]' }, + Picture: { value: '' }, + }, + }, + false, + ); + + assert.equal(merged.Sentence, 'same sentence'); + assert.equal(merged.SentenceAudio, '[sound:same.mp3]'); + assert.equal(merged.Picture, ''); + assert.equal(merged.ExpressionAudio, merged.SentenceAudio); +}); diff --git a/src/anki-integration/anki-connect-proxy.test.ts b/src/anki-integration/anki-connect-proxy.test.ts index 35c84c6..9a8de47 100644 --- a/src/anki-integration/anki-connect-proxy.test.ts +++ b/src/anki-integration/anki-connect-proxy.test.ts @@ -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, 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, diff --git a/src/anki-integration/anki-connect-proxy.ts b/src/anki-integration/anki-connect-proxy.ts index df1f99b..4d75df9 100644 --- a/src/anki-integration/anki-connect-proxy.ts +++ b/src/anki-integration/anki-connect-proxy.ts @@ -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): 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) + : 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).action; + return actionName === 'addNote' || actionName === 'addNotes'; + }); + } + private async enqueueMostRecentAddedNote(): Promise { const findNotes = this.deps.findNotes; if (!findNotes) { diff --git a/src/anki-integration/field-grouping-merge.ts b/src/anki-integration/field-grouping-merge.ts index 06bff96..e570ec8 100644 --- a/src/anki-integration/field-grouping-merge.ts +++ b/src/anki-integration/field-grouping-merge.ts @@ -302,7 +302,7 @@ export class FieldGroupingMergeCollaborator { const unique: { groupId: number; content: string }[] = []; const seen = new Set(); 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 { 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); diff --git a/src/anki-integration/field-grouping-workflow.test.ts b/src/anki-integration/field-grouping-workflow.test.ts index 08abf07..519990b 100644 --- a/src/anki-integration/field-grouping-workflow.test.ts +++ b/src/anki-integration/field-grouping-workflow.test.ts @@ -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; }; +type ManualChoice = { + keepNoteId: number; + deleteNoteId: number; + deleteDuplicate: boolean; + cancelled: boolean; +}; + +type FieldGroupingCallback = (data: { + original: KikuDuplicateCardInfo; + duplicate: KikuDuplicateCardInfo; +}) => Promise; + function createWorkflowHarness() { const updates: Array<{ noteId: number; fields: Record }> = []; 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) => { const out: Record = {}; 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, + }, + ]); +}); diff --git a/src/anki-integration/field-grouping-workflow.ts b/src/anki-integration/field-grouping-workflow.ts index 3576acd..e9555d0 100644 --- a/src/anki-integration/field-grouping-workflow.ts +++ b/src/anki-integration/field-grouping-workflow.ts @@ -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 { - 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,