From a0ac40471230c4a7a7ab7cd827b007c6f355ac52 Mon Sep 17 00:00:00 2001 From: Dusk Date: Sat, 18 Apr 2026 10:20:16 +0800 Subject: [PATCH] fix: tighten module 3 indexing identity and caching --- docs/implementation-decisions.md | 9 +- .../services/AnkiBatchExecutor.test.ts | 105 ++++++++++++++++++ src/application/services/AnkiBatchExecutor.ts | 26 ++++- .../services/FileIndexerService.ts | 52 +++++++-- .../services/CardIndexingService.test.ts | 50 ++++++++- .../services/CardIndexingService.ts | 29 ----- 6 files changed, 224 insertions(+), 47 deletions(-) create mode 100644 src/application/services/AnkiBatchExecutor.test.ts diff --git a/docs/implementation-decisions.md b/docs/implementation-decisions.md index 583f5d6..7eaa829 100644 --- a/docs/implementation-decisions.md +++ b/docs/implementation-decisions.md @@ -24,9 +24,10 @@ ## 4. pending write-back 恢复策略 -- 计划要求 pending write-back 可恢复，但 marker 缺失时无法仅靠 marker 重新定位 -- 因此增加一个严格受限的内部恢复规则：仅允许在“同一文件 + 相同 `rawBlockHash`”时复用既有 `cardId/noteId` -- 这不是通用无 marker 身份推断，只用于本地已知 pending 或已存在状态的恢复 +- 模块 3 的正式身份只认 marker 中的 `cardId` +- 若 marker 丢失，即视为新卡片块，重新生成新的 `cardId` +- pending write-back 仅在 marker 仍保留 `cardId` 时恢复对应 `noteId` +- 不按 `rawBlockHash` 对无 marker 块做长期身份复绑 ## 5. 渲染策略 @@ -60,7 +61,7 @@ - 它会： - 全量重建文件状态与卡片状态 - 为缺失 `cardId` 的块补齐 marker - - 尝试用本地状态恢复已知 `noteId` + - 仅对仍保留 `cardId` 的块恢复本地已知 `noteId` ## 9. 命令与提示 diff --git a/src/application/services/AnkiBatchExecutor.test.ts b/src/application/services/AnkiBatchExecutor.test.ts new file mode 100644 index 0000000..67f3668 --- /dev/null +++ b/src/application/services/AnkiBatchExecutor.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; + +import type { NoteModelDetails } from "@/application/dto/NoteModelDetails"; +import type { IndexedCard } from "@/domain/manual-sync/entities/IndexedCard"; +import type { RenderedSyncCard } from "@/domain/manual-sync/entities/RenderedSyncCard"; +import type { ManualSyncPlan, PlannedCard } from "@/domain/manual-sync/value-objects/ManualSyncPlan"; +import { createModule3Settings, FakeManualSyncAnkiGateway } from "@/test-support/manualSyncFakes"; + +import { AnkiBatchExecutor } from "./AnkiBatchExecutor"; + +class CountingAnkiGateway extends FakeManualSyncAnkiGateway { + public getModelDetailsCalls: string[] = []; + + override async getModelDetails(modelName: string): Promise { + this.getModelDetailsCalls.push(modelName); + return super.getModelDetails(modelName); + } +} + +describe("AnkiBatchExecutor", () => { + it("caches note model details within a single execute run", async () => { + const ankiGateway = new CountingAnkiGateway(); + ankiGateway.noteSummariesById.set(300, { + noteId: 300, + modelName: "Basic", + cardIds: [700], + }); + + const executor = new AnkiBatchExecutor(ankiGateway); + const createOne = createPlannedCard("ahs_1"); + const createTwo = createPlannedCard("ahs_2"); + const updateOne = createPlannedCard("ahs_3", 300); + const renderedCards = new Map([ + [createOne.card.cardId, createRenderedSyncCard(createOne)], + [createTwo.card.cardId, createRenderedSyncCard(createTwo)], + [updateOne.card.cardId, createRenderedSyncCard(updateOne)], + ]); + const plan: ManualSyncPlan = { + toCreate: [createOne, createTwo], + toUpdate: [updateOne], + toRewriteMarker: [], + toOrphan: [], + unchangedCards: 0, + }; + + const result = await executor.execute( + plan, + renderedCards, + async (plannedCard) => createRenderedSyncCard(plannedCard), + createModule3Settings().noteFieldMappings, + ); + + expect(result.created).toBe(2); + expect(result.updated).toBe(1); + expect(ankiGateway.getModelDetailsCalls).toEqual(["Basic"]); + }); +}); + +function createPlannedCard(cardId: string, noteId?: number): PlannedCard { + return { + card: createIndexedCard(cardId, noteId), + noteId, + deck: "Obsidian", + noteModel: "Basic", + renderConfigHash: "render-config", + }; +} + +function createRenderedSyncCard(plannedCard: PlannedCard): RenderedSyncCard { + return { + card: plannedCard.card, + noteId: plannedCard.noteId, + deck: plannedCard.deck, + noteModel: plannedCard.noteModel, + renderConfigHash: plannedCard.renderConfigHash, + renderedFields: { + title: plannedCard.card.heading, + body: plannedCard.card.bodyMarkdown, + }, + media: [], + }; +} + +function createIndexedCard(cardId: string, noteId?: number): IndexedCard { + return { + cardId, + noteId, + markerNoteId: noteId, + filePath: "notes/example.md", + cardType: "basic", + heading: `Heading ${cardId}`, + headingLevel: 4, + bodyMarkdown: `Body ${cardId}`, + blockStartOffset: 0, + blockEndOffset: 10, + blockStartLine: 1, + bodyStartLine: 2, + blockEndLine: 2, + contentEndLine: 2, + rawBlockText: `#### Heading ${cardId}\nBody ${cardId}`, + rawBlockHash: `hash-${cardId}`, + tagsHint: [], + markerState: noteId ? "card-and-note" : "card-only", + }; +} \ No newline at end of file diff --git a/src/application/services/AnkiBatchExecutor.ts b/src/application/services/AnkiBatchExecutor.ts index a34fee8..7b45c12 100644 --- a/src/application/services/AnkiBatchExecutor.ts +++ b/src/application/services/AnkiBatchExecutor.ts @@ -28,6 +28,7 @@ export class AnkiBatchExecutor { renderOnDemand: (plannedCard: PlannedCard) => Promise, noteFieldMappings: Record, ): Promise { + const modelDetailsCache = new Map>>>(); const resolvedNoteIds = new Map(); const touchedCardIds = new Set(); const markerWriteMap = new Map(); @@ -84,7 +85,7 @@ export class AnkiBatchExecutor { await Promise.all(batch.map(async (renderedCard) => ({ deckName: renderedCard.deck, modelName: renderedCard.noteModel, - fields: await this.mapFields(renderedCard, noteFieldMappings), + fields: await this.mapFields(renderedCard, noteFieldMappings, modelDetailsCache), tags: renderedCard.card.tagsHint, }))), ); @@ -114,7 +115,7 @@ export class AnkiBatchExecutor { await Promise.all(batch.map(async ({ plannedCard, renderedCard }) => ({ noteId: plannedCard.noteId ?? 0, deckName: renderedCard.deck, - fields: await this.mapFields(renderedCard, noteFieldMappings), + fields: await this.mapFields(renderedCard, noteFieldMappings, modelDetailsCache), }))), ); }); @@ -182,8 +183,9 @@ export class AnkiBatchExecutor { private async mapFields( renderedCard: RenderedSyncCard, noteFieldMappings: Record, + modelDetailsCache: Map>>>, ): Promise> { - const modelDetails = await this.ankiGateway.getModelDetails(renderedCard.noteModel); + const modelDetails = await this.getModelDetails(renderedCard.noteModel, modelDetailsCache); return this.noteFieldMappingService.mapRenderedCard( { type: renderedCard.card.cardType, @@ -195,6 +197,24 @@ export class AnkiBatchExecutor { ); } + private async getModelDetails( + noteModel: string, + modelDetailsCache: Map>>>, + ): Promise>> { + const cached = modelDetailsCache.get(noteModel); + if (cached) { + return cached; + } + + const pending = this.ankiGateway.getModelDetails(noteModel).catch((error) => { + modelDetailsCache.delete(noteModel); + throw error; + }); + + modelDetailsCache.set(noteModel, pending); + return pending; + } + private async uploadMedia(renderedCards: RenderedSyncCard[]): Promise { const uniqueMedia = new Map(); diff --git a/src/application/services/FileIndexerService.ts b/src/application/services/FileIndexerService.ts index d338d06..e7a8e92 100644 --- a/src/application/services/FileIndexerService.ts +++ b/src/application/services/FileIndexerService.ts @@ -3,7 +3,7 @@ import type { ManualSyncVaultGateway } from "@/application/ports/ManualSyncVault import { ScanScopeService } from "@/application/services/ScanScopeService"; import type { IndexedCard } from "@/domain/manual-sync/entities/IndexedCard"; import type { IndexedFile } from "@/domain/manual-sync/entities/IndexedFile"; -import type { CardState, PluginState } from "@/domain/manual-sync/entities/PluginState"; +import type { CardState, PendingWriteBackState, PluginState } from "@/domain/manual-sync/entities/PluginState"; import { CardIndexingService } from "@/domain/manual-sync/services/CardIndexingService"; export interface FileIndexerResult { @@ -15,6 +15,11 @@ export interface FileIndexerResult { skippedUnchangedCards: number; } +interface StateIndex { + cardsByFilePath: Map; + pendingByFilePath: Map; +} + export class FileIndexerService { constructor( private readonly vaultGateway: ManualSyncVaultGateway, @@ -24,7 +29,7 @@ export class FileIndexerService { async indexVault(settings: PluginSettings, state: PluginState, forceReadAll = false): Promise { const refs = this.scanScopeService.filter(await this.vaultGateway.listMarkdownFileRefs(), settings.includeFolders, settings.excludeFolders); - return this.indexRefs(refs, settings, state, forceReadAll); + return this.indexRefs(refs, settings, state, this.buildStateIndex(state), forceReadAll); } async indexFile(filePath: string, settings: PluginSettings, state: PluginState): Promise { @@ -37,12 +42,13 @@ export class FileIndexerService { } const fileStamp = reference ? createFileStamp(reference.mtime, reference.size) : `${Date.now()}:${sourceFile.content.length}`; + const stateIndex = this.buildStateIndex(state); const indexedFile = this.cardIndexingService.index(sourceFile, { qaHeadingLevel: settings.qaHeadingLevel, clozeHeadingLevel: settings.clozeHeadingLevel, fileStamp, - knownCards: Object.values(state.cards).filter((card) => card.filePath === filePath), - pendingWriteBack: state.pendingWriteBack.filter((pending) => pending.filePath === filePath), + knownCards: stateIndex.cardsByFilePath.get(filePath) ?? [], + pendingWriteBack: stateIndex.pendingByFilePath.get(filePath) ?? [], }); return { @@ -59,6 +65,7 @@ export class FileIndexerService { refs: Awaited>, settings: PluginSettings, state: PluginState, + stateIndex: StateIndex, forceReadAll: boolean, ): Promise { const indexedFiles: IndexedFile[] = []; @@ -69,8 +76,9 @@ export class FileIndexerService { for (const ref of refs) { const fileStamp = createFileStamp(ref.mtime, ref.size); const existingFileState = state.files[ref.path]; - const hasPendingWriteBack = state.pendingWriteBack.some((pending) => pending.filePath === ref.path); - const knownCards = Object.values(state.cards).filter((card) => card.filePath === ref.path); + const pendingWriteBack = stateIndex.pendingByFilePath.get(ref.path) ?? []; + const hasPendingWriteBack = pendingWriteBack.length > 0; + const knownCards = stateIndex.cardsByFilePath.get(ref.path) ?? []; const hasMissingKnownCard = existingFileState?.cardIds.some((cardId) => !state.cards[cardId]) ?? false; const shouldRead = forceReadAll || @@ -107,7 +115,7 @@ export class FileIndexerService { clozeHeadingLevel: settings.clozeHeadingLevel, fileStamp, knownCards, - pendingWriteBack: state.pendingWriteBack.filter((pending) => pending.filePath === ref.path), + pendingWriteBack, }); indexedFiles.push(indexedFile); @@ -123,6 +131,36 @@ export class FileIndexerService { skippedUnchangedCards, }; } + + private buildStateIndex(state: PluginState): StateIndex { + const cardsByFilePath = new Map(); + const pendingByFilePath = new Map(); + + for (const card of Object.values(state.cards)) { + const cards = cardsByFilePath.get(card.filePath); + if (cards) { + cards.push(card); + continue; + } + + cardsByFilePath.set(card.filePath, [card]); + } + + for (const pending of state.pendingWriteBack) { + const pendings = pendingByFilePath.get(pending.filePath); + if (pendings) { + pendings.push(pending); + continue; + } + + pendingByFilePath.set(pending.filePath, [pending]); + } + + return { + cardsByFilePath, + pendingByFilePath, + }; + } } function restoreIndexedCard(card: CardState): IndexedCard { diff --git a/src/domain/manual-sync/services/CardIndexingService.test.ts b/src/domain/manual-sync/services/CardIndexingService.test.ts index 67da62b..1fd253d 100644 --- a/src/domain/manual-sync/services/CardIndexingService.test.ts +++ b/src/domain/manual-sync/services/CardIndexingService.test.ts @@ -1,7 +1,5 @@ import { describe, expect, it } from "vitest"; -import { hashString } from "@/domain/shared/hash"; - import { CardIndexingService } from "./CardIndexingService"; describe("CardIndexingService", () => { @@ -28,7 +26,7 @@ describe("CardIndexingService", () => { expect(indexedFile.cards[0]?.bodyMarkdown).toBe("Answer"); }); - it("reuses cardId and noteId from pending write-back when marker is still missing", () => { + it("treats a missing marker as a new card even if local pending state has the same raw block hash", () => { const service = new CardIndexingService(); const indexedFile = service.index( { @@ -48,12 +46,56 @@ describe("CardIndexingService", () => { noteId: 42, expectedFileHash: "hash", targetMarker: "", - rawBlockHash: hashString(["#### Prompt", "Answer"].join("\n")), + rawBlockHash: "hash-card", }, ], }, ); + expect(indexedFile.cards[0]?.cardId).not.toBe("ahs_known"); + expect(indexedFile.cards[0]?.noteId).toBeUndefined(); + }); + + it("restores noteId when the marker still carries cardId", () => { + const service = new CardIndexingService(); + const indexedFile = service.index( + { + path: "notes/example.md", + basename: "example", + content: ["#### Prompt", "Answer", ""].join("\n"), + }, + { + qaHeadingLevel: 4, + clozeHeadingLevel: 5, + fileStamp: "1:1", + knownCards: [ + { + cardId: "ahs_known", + noteId: 42, + filePath: "notes/example.md", + heading: "Prompt", + headingLevel: 4, + bodyMarkdown: "Answer", + cardType: "basic", + blockStartOffset: 0, + blockEndOffset: 13, + blockStartLine: 1, + bodyStartLine: 2, + blockEndLine: 3, + contentEndLine: 2, + rawBlockText: ["#### Prompt", "Answer"].join("\n"), + rawBlockHash: "hash-card", + renderConfigHash: "render-hash", + deck: "Obsidian", + tagsHint: [], + lastSyncedAt: 1, + orphan: false, + }, + ], + pendingWriteBack: [], + }, + ); + expect(indexedFile.cards[0]).toMatchObject({ cardId: "ahs_known", noteId: 42, diff --git a/src/domain/manual-sync/services/CardIndexingService.ts b/src/domain/manual-sync/services/CardIndexingService.ts index 4bdd174..116af47 100644 --- a/src/domain/manual-sync/services/CardIndexingService.ts +++ b/src/domain/manual-sync/services/CardIndexingService.ts @@ -45,7 +45,6 @@ export class CardIndexingService { const cards: IndexedCard[] = []; const knownCardsById = new Map(context.knownCards.map((card) => [card.cardId, card])); const pendingByCardId = new Map(context.pendingWriteBack.map((pending) => [pending.cardId, pending])); - const reusableKnownCards = context.knownCards.filter((card) => !card.orphan); const usedCardIds = new Set(); for (let headingIndex = 0; headingIndex < headings.length; headingIndex += 1) { @@ -63,14 +62,10 @@ export class CardIndexingService { const rawBlockText = [lines[heading.lineIndex], ...trimmedBodyLines].join("\n").trimEnd(); const rawBlockHash = hashString(rawBlockText); const resolvedIdentity = this.resolveIdentity( - sourceFile.path, - rawBlockHash, marker.markerCardId, marker.markerNoteId, knownCardsById, pendingByCardId, - reusableKnownCards, - usedCardIds, ); usedCardIds.add(resolvedIdentity.cardId); @@ -110,14 +105,10 @@ export class CardIndexingService { } private resolveIdentity( - filePath: string, - rawBlockHash: string, markerCardId: string | undefined, markerNoteId: number | undefined, knownCardsById: Map, pendingByCardId: Map, - reusableKnownCards: CardState[], - usedCardIds: Set, ): { cardId: string; noteId?: number } { if (markerCardId) { const pending = pendingByCardId.get(markerCardId); @@ -129,26 +120,6 @@ export class CardIndexingService { }; } - const pendingMatch = Array.from(pendingByCardId.values()).find( - (pending) => pending.filePath === filePath && pending.rawBlockHash === rawBlockHash && !usedCardIds.has(pending.cardId), - ); - if (pendingMatch) { - return { - cardId: pendingMatch.cardId, - noteId: pendingMatch.noteId, - }; - } - - const knownMatch = reusableKnownCards.find( - (card) => card.filePath === filePath && card.rawBlockHash === rawBlockHash && !usedCardIds.has(card.cardId), - ); - if (knownMatch) { - return { - cardId: knownMatch.cardId, - noteId: knownMatch.noteId, - }; - } - return { cardId: this.markerService.generateCardId(), };