mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-01 06:22:44 -08:00
fix(anki): avoid unintended kiku grouping on lookup
This commit is contained in:
@@ -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.
|
**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.
|
**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 |
|
| Field | Merge behavior |
|
||||||
| -------- | -------------------------------------------------------------- |
|
| -------- | -------------------------------------------------------------- |
|
||||||
| Sentence | Both sentences preserved, labeled `[Original]` / `[Duplicate]` |
|
| Sentence | Both sentences preserved (exact duplicate text is deduplicated) |
|
||||||
| Audio | Both `[sound:...]` entries kept |
|
| Audio | Both `[sound:...]` entries kept (exact duplicates deduplicated) |
|
||||||
| Image | Both images kept |
|
| Image | Both images kept (exact duplicates deduplicated) |
|
||||||
|
|
||||||
### Keyboard Shortcuts in the Modal
|
### Keyboard Shortcuts in the Modal
|
||||||
|
|
||||||
|
|||||||
@@ -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>');
|
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);
|
||||||
|
});
|
||||||
|
|||||||
@@ -230,6 +230,41 @@ test('proxy ignores addNote when upstream response reports error', async () => {
|
|||||||
assert.deepEqual(processed, []);
|
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', () => {
|
test('proxy detects self-referential loop configuration', () => {
|
||||||
const proxy = new AnkiConnectProxyServer({
|
const proxy = new AnkiConnectProxyServer({
|
||||||
shouldAutoUpdateNewCards: () => true,
|
shouldAutoUpdateNewCards: () => true,
|
||||||
|
|||||||
@@ -179,6 +179,7 @@ export class AnkiConnectProxyServer {
|
|||||||
if (action !== 'addNote' && action !== 'addNotes' && action !== 'multi') {
|
if (action !== 'addNote' && action !== 'addNotes' && action !== 'multi') {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
const shouldFallbackToLatestAdded = this.requestIncludesAddAction(action, requestJson);
|
||||||
|
|
||||||
const parsedResponse = this.tryParseJsonValue(responseBody);
|
const parsedResponse = this.tryParseJsonValue(responseBody);
|
||||||
if (parsedResponse === null || parsedResponse === undefined) {
|
if (parsedResponse === null || parsedResponse === undefined) {
|
||||||
@@ -194,7 +195,7 @@ export class AnkiConnectProxyServer {
|
|||||||
action === 'multi'
|
action === 'multi'
|
||||||
? this.collectMultiResultIds(requestJson, responseResult)
|
? this.collectMultiResultIds(requestJson, responseResult)
|
||||||
: this.collectNoteIdsForAction(action, responseResult);
|
: this.collectNoteIdsForAction(action, responseResult);
|
||||||
if (noteIds.length === 0) {
|
if (noteIds.length === 0 && shouldFallbackToLatestAdded) {
|
||||||
void this.enqueueMostRecentAddedNote();
|
void this.enqueueMostRecentAddedNote();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -202,6 +203,28 @@ export class AnkiConnectProxyServer {
|
|||||||
this.enqueueNotes(noteIds);
|
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> {
|
private async enqueueMostRecentAddedNote(): Promise<void> {
|
||||||
const findNotes = this.deps.findNotes;
|
const findNotes = this.deps.findNotes;
|
||||||
if (!findNotes) {
|
if (!findNotes) {
|
||||||
|
|||||||
@@ -302,7 +302,7 @@ export class FieldGroupingMergeCollaborator {
|
|||||||
const unique: { groupId: number; content: string }[] = [];
|
const unique: { groupId: number; content: string }[] = [];
|
||||||
const seen = new Set<string>();
|
const seen = new Set<string>();
|
||||||
for (const entry of entries) {
|
for (const entry of entries) {
|
||||||
const key = `${entry.groupId}::${entry.content}`;
|
const key = entry.content;
|
||||||
if (seen.has(key)) continue;
|
if (seen.has(key)) continue;
|
||||||
seen.add(key);
|
seen.add(key);
|
||||||
unique.push(entry);
|
unique.push(entry);
|
||||||
@@ -361,6 +361,10 @@ export class FieldGroupingMergeCollaborator {
|
|||||||
return ungrouped;
|
return ungrouped;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private getPictureDedupKey(tag: string): string {
|
||||||
|
return tag.replace(/\sdata-group-id="[^"]*"/gi, '').trim();
|
||||||
|
}
|
||||||
|
|
||||||
private getStrictSpanGroupingFields(): Set<string> {
|
private getStrictSpanGroupingFields(): Set<string> {
|
||||||
const strictFields = new Set(this.strictGroupingFieldDefaults);
|
const strictFields = new Set(this.strictGroupingFieldDefaults);
|
||||||
const sentenceCardConfig = this.deps.getEffectiveSentenceCardConfig();
|
const sentenceCardConfig = this.deps.getEffectiveSentenceCardConfig();
|
||||||
@@ -394,11 +398,12 @@ export class FieldGroupingMergeCollaborator {
|
|||||||
const mergedTags = keepEntries.map((entry) =>
|
const mergedTags = keepEntries.map((entry) =>
|
||||||
this.ensureImageGroupId(entry.tag, entry.groupId),
|
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) {
|
for (const entry of sourceEntries) {
|
||||||
const normalized = this.ensureImageGroupId(entry.tag, entry.groupId);
|
const normalized = this.ensureImageGroupId(entry.tag, entry.groupId);
|
||||||
if (seen.has(normalized)) continue;
|
const dedupKey = this.getPictureDedupKey(normalized);
|
||||||
seen.add(normalized);
|
if (seen.has(dedupKey)) continue;
|
||||||
|
seen.add(dedupKey);
|
||||||
mergedTags.push(normalized);
|
mergedTags.push(normalized);
|
||||||
}
|
}
|
||||||
return mergedTags.join('');
|
return mergedTags.join('');
|
||||||
@@ -415,9 +420,9 @@ export class FieldGroupingMergeCollaborator {
|
|||||||
.join('');
|
.join('');
|
||||||
}
|
}
|
||||||
const merged = [...keepEntries];
|
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) {
|
for (const entry of sourceEntries) {
|
||||||
const key = `${entry.groupId}::${entry.content}`;
|
const key = entry.content;
|
||||||
if (seen.has(key)) continue;
|
if (seen.has(key)) continue;
|
||||||
seen.add(key);
|
seen.add(key);
|
||||||
merged.push(entry);
|
merged.push(entry);
|
||||||
|
|||||||
@@ -1,16 +1,36 @@
|
|||||||
import test from 'node:test';
|
import test from 'node:test';
|
||||||
import assert from 'node:assert/strict';
|
import assert from 'node:assert/strict';
|
||||||
import { FieldGroupingWorkflow } from './field-grouping-workflow';
|
import { FieldGroupingWorkflow } from './field-grouping-workflow';
|
||||||
|
import type { KikuDuplicateCardInfo, KikuFieldGroupingChoice } from '../types';
|
||||||
|
|
||||||
type NoteInfo = {
|
type NoteInfo = {
|
||||||
noteId: number;
|
noteId: number;
|
||||||
fields: Record<string, { value: string }>;
|
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() {
|
function createWorkflowHarness() {
|
||||||
const updates: Array<{ noteId: number; fields: Record<string, string> }> = [];
|
const updates: Array<{ noteId: number; fields: Record<string, string> }> = [];
|
||||||
const deleted: number[][] = [];
|
const deleted: number[][] = [];
|
||||||
const statuses: string[] = [];
|
const statuses: string[] = [];
|
||||||
|
const mergeCalls: Array<{
|
||||||
|
keepNoteId: number;
|
||||||
|
deleteNoteId: number;
|
||||||
|
keepNoteInfoNoteId: number;
|
||||||
|
deleteNoteInfoNoteId: number;
|
||||||
|
}> = [];
|
||||||
|
let manualChoice: ManualChoice | null = null;
|
||||||
|
|
||||||
const deps = {
|
const deps = {
|
||||||
client: {
|
client: {
|
||||||
@@ -47,11 +67,28 @@ function createWorkflowHarness() {
|
|||||||
kikuDeleteDuplicateInAuto: true,
|
kikuDeleteDuplicateInAuto: true,
|
||||||
}),
|
}),
|
||||||
getCurrentSubtitleText: () => 'subtitle-text',
|
getCurrentSubtitleText: () => 'subtitle-text',
|
||||||
getFieldGroupingCallback: () => null,
|
getFieldGroupingCallback: (): FieldGroupingCallback | null => {
|
||||||
|
const choice = manualChoice;
|
||||||
|
if (choice === null) return null;
|
||||||
|
return async () => choice;
|
||||||
|
},
|
||||||
setFieldGroupingCallback: () => undefined,
|
setFieldGroupingCallback: () => undefined,
|
||||||
computeFieldGroupingMergedFields: async () => ({
|
computeFieldGroupingMergedFields: async (
|
||||||
Sentence: 'merged sentence',
|
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 }>) => {
|
extractFields: (fields: Record<string, { value: string }>) => {
|
||||||
const out: Record<string, string> = {};
|
const out: Record<string, string> = {};
|
||||||
for (const [key, value] of Object.entries(fields)) {
|
for (const [key, value] of Object.entries(fields)) {
|
||||||
@@ -77,6 +114,10 @@ function createWorkflowHarness() {
|
|||||||
updates,
|
updates,
|
||||||
deleted,
|
deleted,
|
||||||
statuses,
|
statuses,
|
||||||
|
mergeCalls,
|
||||||
|
setManualChoice: (choice: typeof manualChoice) => {
|
||||||
|
manualChoice = choice;
|
||||||
|
},
|
||||||
deps,
|
deps,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -112,3 +153,31 @@ test('FieldGroupingWorkflow manual mode returns false when callback unavailable'
|
|||||||
assert.equal(handled, false);
|
assert.equal(handled, false);
|
||||||
assert.equal(harness.updates.length, 0);
|
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,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|||||||
@@ -69,7 +69,6 @@ export class FieldGroupingWorkflow {
|
|||||||
await this.performMerge(
|
await this.performMerge(
|
||||||
originalNoteId,
|
originalNoteId,
|
||||||
newNoteId,
|
newNoteId,
|
||||||
newNoteInfo,
|
|
||||||
this.getExpression(newNoteInfo),
|
this.getExpression(newNoteInfo),
|
||||||
sentenceCardConfig.kikuDeleteDuplicateInAuto,
|
sentenceCardConfig.kikuDeleteDuplicateInAuto,
|
||||||
);
|
);
|
||||||
@@ -112,12 +111,10 @@ export class FieldGroupingWorkflow {
|
|||||||
|
|
||||||
const keepNoteId = choice.keepNoteId;
|
const keepNoteId = choice.keepNoteId;
|
||||||
const deleteNoteId = choice.deleteNoteId;
|
const deleteNoteId = choice.deleteNoteId;
|
||||||
const deleteNoteInfo = deleteNoteId === newNoteId ? newNoteInfo : originalNoteInfo;
|
|
||||||
|
|
||||||
await this.performMerge(
|
await this.performMerge(
|
||||||
keepNoteId,
|
keepNoteId,
|
||||||
deleteNoteId,
|
deleteNoteId,
|
||||||
deleteNoteInfo,
|
|
||||||
expression,
|
expression,
|
||||||
choice.deleteDuplicate,
|
choice.deleteDuplicate,
|
||||||
);
|
);
|
||||||
@@ -132,18 +129,22 @@ export class FieldGroupingWorkflow {
|
|||||||
private async performMerge(
|
private async performMerge(
|
||||||
keepNoteId: number,
|
keepNoteId: number,
|
||||||
deleteNoteId: number,
|
deleteNoteId: number,
|
||||||
deleteNoteInfo: FieldGroupingWorkflowNoteInfo,
|
|
||||||
expression: string,
|
expression: string,
|
||||||
deleteDuplicate = true,
|
deleteDuplicate = true,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
const keepNotesInfoResult = await this.deps.client.notesInfo([keepNoteId]);
|
const notesInfoResult = await this.deps.client.notesInfo([keepNoteId, deleteNoteId]);
|
||||||
const keepNotesInfo = keepNotesInfoResult as FieldGroupingWorkflowNoteInfo[];
|
const notesInfo = notesInfoResult as FieldGroupingWorkflowNoteInfo[];
|
||||||
if (!keepNotesInfo || keepNotesInfo.length === 0) {
|
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);
|
this.deps.logInfo('Keep note not found:', keepNoteId);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if (!deleteNoteInfo) {
|
||||||
|
this.deps.logInfo('Delete note not found:', deleteNoteId);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const keepNoteInfo = keepNotesInfo[0]!;
|
|
||||||
const mergedFields = await this.deps.computeFieldGroupingMergedFields(
|
const mergedFields = await this.deps.computeFieldGroupingMergedFields(
|
||||||
keepNoteId,
|
keepNoteId,
|
||||||
deleteNoteId,
|
deleteNoteId,
|
||||||
|
|||||||
Reference in New Issue
Block a user