diff --git a/docs/empty-deck-cleanup-fix-decisions.md b/docs/empty-deck-cleanup-fix-decisions.md new file mode 100644 index 0000000..b34368a --- /dev/null +++ b/docs/empty-deck-cleanup-fix-decisions.md @@ -0,0 +1,67 @@ +# Empty Deck Cleanup Fix Decisions + +## 1. DeckStat 语义 + +`DeckStat.noteCount` 改为可选值。 + +语义固定为： + +1. `0` 表示明确为空 +2. 正数表示明确非空 +3. `undefined` 表示未知或缺失统计 + +## 2. Gateway 解析策略 + +`AnkiConnectGateway.getDeckStats` 不再使用任何把缺失统计映射成 `0` 的 fallback。 + +解析规则： + +1. 只接受对象形态的 `rawStats[deckName]` +2. 只接受数值型 `total_in_deck` +3. 缺 key、字段缺失、字段类型不对、整体结构不对，都返回 `noteCount: undefined` + +这保证未知不会伪装成 empty。 + +## 3. Candidate 筛选策略 + +`CleanupEmptyDecksUseCase.listCandidates()` 只接受： + +1. `stat.noteCount === 0` + +其他情况全部跳过： + +1. `undefined` +2. 正数 + +## 4. 删除前再校验策略 + +`CleanupEmptyDecksUseCase.execute()` 在实际删除前继续重新读取 deck stats，但只把满足以下条件的 deck 视为可删： + +1. deck 当前仍存在 +2. `currentDeckStatsByName.get(deckName) === 0` + +不再使用 `?? 0`。 + +因此： + +1. missing stats -> skipped +2. unknown stats -> skipped +3. nonzero stats -> skipped + +## 5. 测试策略 + +新增并锁定以下 fail-safe 测试： + +1. stats 缺 key 时不判空 +2. stats 空对象时候选为空 +3. 只返回部分 deck 时，只接受显式为 0 的 deck +4. 返回结构缺少 `total_in_deck` 时，不把该 deck 视为空 + +## 6. 范围控制 + +本次只修空牌组候选误判，不扩展： + +1. deck cleanup UI +2. deck cleanup 交互文案 +3. 普通 sync/rebuild 行为 +4. 其他 AnkiConnect payload 兼容逻辑 \ No newline at end of file diff --git a/docs/empty-deck-cleanup-fix-gap-report.md b/docs/empty-deck-cleanup-fix-gap-report.md new file mode 100644 index 0000000..f787374 --- /dev/null +++ b/docs/empty-deck-cleanup-fix-gap-report.md @@ -0,0 +1,61 @@ +# Empty Deck Cleanup Fix Gap Report + +## 审查范围 + +本次修复只覆盖空牌组候选判定链路： + +1. `AnkiGateway.getDeckStats` +2. `AnkiConnectGateway.getDeckStats` +3. `CleanupEmptyDecksUseCase` +4. 空牌组清理相关测试 + +## 当前缺口 + +### 1. gateway 把缺失统计直接映射成 0 + +`AnkiConnectGateway.getDeckStats` 当前实现： + +1. 调用 `getDeckStats` +2. 对每个请求 deck 读取 `rawStats[deckName]?.total_in_deck ?? 0` + +问题： + +1. 返回对象缺 key 时，会把 unknown 误判成 `0` +2. 返回结构不符合预期时，也会把 unknown 误判成 `0` +3. 这违反了 `missing != empty` + +### 2. use case 继续把 unknown 当 empty + +`CleanupEmptyDecksUseCase` 当前实现有两处同类问题： + +1. `listCandidates()` 直接用 `noteCount === 0`，但上游已经把 unknown 压成了 0 +2. `execute()` 用 `(currentDeckStatsByName.get(deckName) ?? 0) === 0` 判定可删除，missing stats 会再次被当成空牌组 + +### 3. test fake 也把缺失统计默认成 0 + +`FakeManualSyncAnkiGateway.getDeckStats()` 目前对缺失 deck 返回 `{ noteCount: 0 }`。 + +这让测试层也继承了同样的危险默认值，无法覆盖“deck 列表存在，但 stats 缺失”的场景。 + +### 4. 现有测试只覆盖理想返回 + +当前测试只覆盖： + +1. 请求 deck 与返回 stats 一一对应 +2. `total_in_deck` 明确为 0 或正数 + +缺少： + +1. 返回缺 key +2. 返回空对象 +3. 只返回部分 deck +4. 返回结构不含 `total_in_deck` + +## 修复目标 + +本次修复必须把判定收紧为： + +1. 只有 Anki 明确返回 `total_in_deck === 0` 才算 empty +2. 缺失/未知/结构不匹配一律视为 unknown +3. unknown 不进入 cleanup candidates +4. stats 不完整时宁可少列，不可多列 \ No newline at end of file diff --git a/src/application/ports/AnkiGateway.ts b/src/application/ports/AnkiGateway.ts index 45eb5cb..c8fdfbb 100644 --- a/src/application/ports/AnkiGateway.ts +++ b/src/application/ports/AnkiGateway.ts @@ -27,7 +27,7 @@ export interface ChangeDeckInput { export interface DeckStat { deckName: string; - noteCount: number; + noteCount?: number; } export interface AnkiGateway { diff --git a/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts b/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts index 05ced11..c1d201a 100644 --- a/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts +++ b/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts @@ -7,6 +7,7 @@ import { CleanupEmptyDecksUseCase } from "./CleanupEmptyDecksUseCase"; describe("CleanupEmptyDecksUseCase", () => { it("detects empty decks from Anki stats", async () => { const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Busy", "Empty", "Another::Empty"]; ankiGateway.deckStatsByName.set("Busy", { deckName: "Busy", noteCount: 2 }); ankiGateway.deckStatsByName.set("Empty", { deckName: "Empty", noteCount: 0 }); ankiGateway.deckStatsByName.set("Another::Empty", { deckName: "Another::Empty", noteCount: 0 }); @@ -19,6 +20,7 @@ describe("CleanupEmptyDecksUseCase", () => { it("deletes only the user-selected empty decks", async () => { const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Empty", "Another::Empty"]; ankiGateway.deckStatsByName.set("Empty", { deckName: "Empty", noteCount: 0 }); ankiGateway.deckStatsByName.set("Another::Empty", { deckName: "Another::Empty", noteCount: 0 }); const useCase = new CleanupEmptyDecksUseCase(ankiGateway); @@ -38,6 +40,7 @@ describe("CleanupEmptyDecksUseCase", () => { it("skips decks that disappeared or became non-empty before deletion", async () => { const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Empty", "Gone"]; ankiGateway.deckStatsByName.set("Empty", { deckName: "Empty", noteCount: 0 }); ankiGateway.deckStatsByName.set("Gone", { deckName: "Gone", noteCount: 0 }); const useCase = new CleanupEmptyDecksUseCase(ankiGateway); @@ -54,4 +57,43 @@ describe("CleanupEmptyDecksUseCase", () => { expect(result.skippedDeckNames).toEqual(["Empty", "Gone"]); expect(ankiGateway.deletedDecks).toEqual([]); }); + + it("skips decks with missing stats when building candidates", async () => { + const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Empty", "Busy", "Unknown"]; + ankiGateway.deckStatsByName.set("Empty", { deckName: "Empty", noteCount: 0 }); + ankiGateway.deckStatsByName.set("Busy", { deckName: "Busy", noteCount: 4 }); + const useCase = new CleanupEmptyDecksUseCase(ankiGateway); + + const candidates = await useCase.listCandidates(); + + expect(candidates).toEqual(["Empty"]); + }); + + it("returns zero candidates when Anki returns no stats for the listed decks", async () => { + const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Deck A", "Deck B", "Deck C"]; + const useCase = new CleanupEmptyDecksUseCase(ankiGateway); + + const candidates = await useCase.listCandidates(); + + expect(candidates).toEqual([]); + }); + + it("skips selected decks whose current stats are unknown at delete time", async () => { + const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Empty", "Unknown"]; + ankiGateway.deckStatsByName.set("Empty", { deckName: "Empty", noteCount: 0 }); + const useCase = new CleanupEmptyDecksUseCase(ankiGateway); + + const candidates = await useCase.listCandidates(); + const result = await useCase.execute(["Empty", "Unknown"], candidates); + + expect(result.candidateCount).toBe(1); + expect(result.selectedCount).toBe(2); + expect(result.deletedCount).toBe(1); + expect(result.deletedDeckNames).toEqual(["Empty"]); + expect(result.skippedDeckNames).toEqual(["Unknown"]); + expect(ankiGateway.deletedDecks).toEqual([["Empty"]]); + }); }); \ No newline at end of file diff --git a/src/application/use-cases/CleanupEmptyDecksUseCase.ts b/src/application/use-cases/CleanupEmptyDecksUseCase.ts index 2aa9ce3..cd5b019 100644 --- a/src/application/use-cases/CleanupEmptyDecksUseCase.ts +++ b/src/application/use-cases/CleanupEmptyDecksUseCase.ts @@ -34,8 +34,8 @@ export class CleanupEmptyDecksUseCase { const currentDeckNameSet = new Set(currentDeckNames); const currentDeckStatsByName = new Map(currentDeckStats.map((stat) => [stat.deckName, stat.noteCount])); - const deletableDeckNames = uniqueSelectedDeckNames.filter((deckName) => currentDeckNameSet.has(deckName) && (currentDeckStatsByName.get(deckName) ?? 0) === 0); - const skippedDeckNames = uniqueSelectedDeckNames.filter((deckName) => !currentDeckNameSet.has(deckName) || (currentDeckStatsByName.get(deckName) ?? 0) > 0); + const deletableDeckNames = uniqueSelectedDeckNames.filter((deckName) => currentDeckNameSet.has(deckName) && currentDeckStatsByName.get(deckName) === 0); + const skippedDeckNames = uniqueSelectedDeckNames.filter((deckName) => !currentDeckNameSet.has(deckName) || currentDeckStatsByName.get(deckName) !== 0); const deletedDeckNames: string[] = []; for (const deckName of deletableDeckNames) { diff --git a/src/infrastructure/anki/AnkiConnectGateway.test.ts b/src/infrastructure/anki/AnkiConnectGateway.test.ts index bc62661..346ee80 100644 --- a/src/infrastructure/anki/AnkiConnectGateway.test.ts +++ b/src/infrastructure/anki/AnkiConnectGateway.test.ts @@ -134,6 +134,64 @@ describe("AnkiConnectGateway", () => { }); }); + it("does not treat missing requested deck stats as empty", async () => { + requestUrlMock.mockResolvedValue({ + json: { + error: null, + result: { + Empty: { total_in_deck: 0 }, + }, + }, + }); + + const gateway = new AnkiConnectGateway(() => "http://127.0.0.1:8765"); + const stats = await gateway.getDeckStats(["Empty", "Missing"]); + + expect(stats).toEqual([ + { deckName: "Empty", noteCount: 0 }, + { deckName: "Missing", noteCount: undefined }, + ]); + }); + + it("treats an empty stats object as unknown instead of empty", async () => { + requestUrlMock.mockResolvedValue({ + json: { + error: null, + result: {}, + }, + }); + + const gateway = new AnkiConnectGateway(() => "http://127.0.0.1:8765"); + const stats = await gateway.getDeckStats(["Deck A", "Deck B"]); + + expect(stats).toEqual([ + { deckName: "Deck A", noteCount: undefined }, + { deckName: "Deck B", noteCount: undefined }, + ]); + }); + + it("treats malformed deck stats entries as unknown instead of zero", async () => { + requestUrlMock.mockResolvedValue({ + json: { + error: null, + result: { + Broken: {}, + WrongType: { total_in_deck: "0" }, + Empty: { total_in_deck: 0 }, + }, + }, + }); + + const gateway = new AnkiConnectGateway(() => "http://127.0.0.1:8765"); + const stats = await gateway.getDeckStats(["Broken", "WrongType", "Empty"]); + + expect(stats).toEqual([ + { deckName: "Broken", noteCount: undefined }, + { deckName: "WrongType", noteCount: undefined }, + { deckName: "Empty", noteCount: 0 }, + ]); + }); + it("batches add note calls through AnkiConnect multi", async () => { requestUrlMock.mockResolvedValue({ json: { diff --git a/src/infrastructure/anki/AnkiConnectGateway.ts b/src/infrastructure/anki/AnkiConnectGateway.ts index a851871..bfd88d1 100644 --- a/src/infrastructure/anki/AnkiConnectGateway.ts +++ b/src/infrastructure/anki/AnkiConnectGateway.ts @@ -66,13 +66,13 @@ export class AnkiConnectGateway implements AnkiGateway { return []; } - const rawStats = await this.invoke>("getDeckStats", { + const rawStats = await this.invoke("getDeckStats", { decks: deckNames, }); return deckNames.map((deckName) => ({ deckName, - noteCount: rawStats[deckName]?.total_in_deck ?? 0, + noteCount: extractDeckNoteCount(rawStats, deckName), })); } @@ -240,4 +240,22 @@ export class AnkiConnectGateway implements AnkiGateway { return this.invoke("multi", { actions }); } +} + +function extractDeckNoteCount(rawStats: unknown, deckName: string): number | undefined { + if (!rawStats || typeof rawStats !== "object") { + return undefined; + } + + const rawDeckStat = (rawStats as Record)[deckName]; + if (!rawDeckStat || typeof rawDeckStat !== "object") { + return undefined; + } + + const totalInDeck = (rawDeckStat as RawDeckStats).total_in_deck; + if (typeof totalInDeck !== "number" || !Number.isFinite(totalInDeck)) { + return undefined; + } + + return totalInDeck; } \ No newline at end of file diff --git a/src/test-support/manualSyncFakes.ts b/src/test-support/manualSyncFakes.ts index 6dbe0ec..107e186 100644 --- a/src/test-support/manualSyncFakes.ts +++ b/src/test-support/manualSyncFakes.ts @@ -144,6 +144,7 @@ export class FakeManualSyncAnkiGateway implements AnkiGateway { public storedMedia: MediaAsset[] = []; public noteSummariesById = new Map(); public deckStatsByName = new Map(); + public listedDeckNames: string[] | null = null; public modelDetailsByName: Record = { Basic: { fieldNames: ["Front", "Back"], isCloze: false }, Cloze: { fieldNames: ["Text", "Extra"], isCloze: true }, @@ -164,7 +165,7 @@ export class FakeManualSyncAnkiGateway implements AnkiGateway { } async listDeckNames(): Promise { - return Array.from(this.deckStatsByName.keys()); + return this.listedDeckNames ? [...this.listedDeckNames] : Array.from(this.deckStatsByName.keys()); } async getModelDetails(modelName: string): Promise { @@ -172,7 +173,7 @@ export class FakeManualSyncAnkiGateway implements AnkiGateway { } async getDeckStats(deckNames: string[]): Promise { - return deckNames.map((deckName) => this.deckStatsByName.get(deckName) ?? { deckName, noteCount: 0 }); + return deckNames.map((deckName) => this.deckStatsByName.get(deckName) ?? { deckName }); } async getNoteSummaries(noteIds: number[]): Promise {