From 1682ce5fa02cbf83f80b48e0cecfd9b73c6d6820 Mon Sep 17 00:00:00 2001 From: Dusk Date: Sat, 18 Apr 2026 22:53:17 +0800 Subject: [PATCH] fix(sync): fix deck stats structure, empty deck identification, and test false positives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 修复了 deck stats 返回结构读取错误、修复了空牌组候选识别错误、修复了对应测试的假阳性问题。 --- docs/fix-gap-report.md | 66 +++++++++++ .../CleanupEmptyDecksUseCase.test.ts | 13 +++ .../anki/AnkiConnectGateway.test.ts | 108 +++++++++++++++++- src/infrastructure/anki/AnkiConnectGateway.ts | 26 +++-- 4 files changed, 199 insertions(+), 14 deletions(-) create mode 100644 docs/fix-gap-report.md diff --git a/docs/fix-gap-report.md b/docs/fix-gap-report.md new file mode 100644 index 0000000..c0b616b --- /dev/null +++ b/docs/fix-gap-report.md @@ -0,0 +1,66 @@ +# Fix Gap Report + +## 审查结论 + +本次真实故障链路与 review 结论一致。 + +### 1. 当前 gateway 仍按 deckName 直接读取 `rawStats[deckName]` + +`AnkiConnectGateway.getDeckStats()` 当前通过 `extractDeckNoteCount(rawStats, deckName)` 取统计值，而该 helper 仍然使用： + +1. `rawStats[deckName]` +2. 再读取 `total_in_deck` + +这隐含假设：`getDeckStats` 的顶层 key 是 deck 名称。 + +### 2. 实际 AnkiConnect 返回结构不是 deckName-keyed + +根据 `get_deck_stats` 文档示例，真实返回结构是： + +1. 顶层 key 为 `deck_id` 字符串 +2. deck 名称在 value 的 `name` 字段 +3. 卡片总数在 value 的 `total_in_deck` 字段 + +因此当前实现对真实返回值会出现： + +1. `rawStats[deckName]` 取不到 +2. `noteCount` 变成 `undefined` +3. 上层候选筛选 `noteCount === 0` 全部失败 +4. UI 提示“未发现可清理的空牌组” + +### 3. 当前测试仍是假阳性 + +`AnkiConnectGateway.test.ts` 里的 mock 还在用错误结构： + +1. 顶层 key 直接写 deckName +2. value 里没有 `name` +3. 正好迎合了错误实现 + +所以测试绿，不代表真实环境可用。 + +### 4. Use case 当前安全语义本身没有问题 + +`CleanupEmptyDecksUseCase` 现在已经保持了 fail-safe 语义： + +1. 只有 `noteCount === 0` 才进入候选 +2. `undefined` 不会被当成 empty + +因此这次真正需要修的是： + +1. gateway 对真实返回结构的读取方式 +2. gateway tests 的假数据结构 +3. 覆盖真实 payload 形状的回归测试 + +## 修复范围 + +本次只修： + +1. `getDeckStats` 的真实 payload 读取 +2. 空牌组候选识别的真实返回链路 +3. 对应测试假阳性 + +不扩展到： + +1. 空牌组清理 UI +2. 其他 deck API +3. 普通 sync / rebuild 逻辑 \ No newline at end of file diff --git a/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts b/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts index c1d201a..9bc7082 100644 --- a/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts +++ b/src/application/use-cases/CleanupEmptyDecksUseCase.test.ts @@ -70,6 +70,19 @@ describe("CleanupEmptyDecksUseCase", () => { expect(candidates).toEqual(["Empty"]); }); + it("includes only decks explicitly confirmed with zero notes", async () => { + const ankiGateway = new FakeManualSyncAnkiGateway(); + ankiGateway.listedDeckNames = ["Empty A", "Busy", "Unknown", "Empty B"]; + ankiGateway.deckStatsByName.set("Empty A", { deckName: "Empty A", noteCount: 0 }); + ankiGateway.deckStatsByName.set("Busy", { deckName: "Busy", noteCount: 2 }); + ankiGateway.deckStatsByName.set("Empty B", { deckName: "Empty B", noteCount: 0 }); + const useCase = new CleanupEmptyDecksUseCase(ankiGateway); + + const candidates = await useCase.listCandidates(); + + expect(candidates).toEqual(["Empty A", "Empty B"]); + }); + 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"]; diff --git a/src/infrastructure/anki/AnkiConnectGateway.test.ts b/src/infrastructure/anki/AnkiConnectGateway.test.ts index 346ee80..b3b4cef 100644 --- a/src/infrastructure/anki/AnkiConnectGateway.test.ts +++ b/src/infrastructure/anki/AnkiConnectGateway.test.ts @@ -112,8 +112,16 @@ describe("AnkiConnectGateway", () => { json: { error: null, result: { - Empty: { total_in_deck: 0 }, - Busy: { total_in_deck: 3 }, + "1651445861967": { + deck_id: 1651445861967, + name: "Empty", + total_in_deck: 0, + }, + "1651445861968": { + deck_id: 1651445861968, + name: "Busy", + total_in_deck: 3, + }, }, }, }); @@ -139,7 +147,11 @@ describe("AnkiConnectGateway", () => { json: { error: null, result: { - Empty: { total_in_deck: 0 }, + "1651445861967": { + deck_id: 1651445861967, + name: "Empty", + total_in_deck: 0, + }, }, }, }); @@ -175,9 +187,20 @@ describe("AnkiConnectGateway", () => { json: { error: null, result: { - Broken: {}, - WrongType: { total_in_deck: "0" }, - Empty: { total_in_deck: 0 }, + "1651445861967": { + deck_id: 1651445861967, + name: "Broken", + }, + "1651445861968": { + deck_id: 1651445861968, + name: "WrongType", + total_in_deck: "0", + }, + "1651445861969": { + deck_id: 1651445861969, + name: "Empty", + total_in_deck: 0, + }, }, }, }); @@ -192,6 +215,79 @@ describe("AnkiConnectGateway", () => { ]); }); + it("matches deck stats by the value.name field when top-level keys are deck ids", async () => { + requestUrlMock.mockResolvedValue({ + json: { + error: null, + result: { + "1651445861967": { + deck_id: 1651445861967, + name: "Scoped::Empty", + total_in_deck: 0, + }, + "1651445861968": { + deck_id: 1651445861968, + name: "Scoped::Busy", + total_in_deck: 7, + }, + }, + }, + }); + + const gateway = new AnkiConnectGateway(() => "http://127.0.0.1:8765"); + const stats = await gateway.getDeckStats(["Scoped::Empty", "Scoped::Busy"]); + + expect(stats).toEqual([ + { deckName: "Scoped::Empty", noteCount: 0 }, + { deckName: "Scoped::Busy", noteCount: 7 }, + ]); + }); + + it("treats partial deck-id keyed responses as unknown for unmatched deck names", async () => { + requestUrlMock.mockResolvedValue({ + json: { + error: null, + result: { + "1651445861967": { + deck_id: 1651445861967, + name: "Scoped::Empty", + total_in_deck: 0, + }, + }, + }, + }); + + const gateway = new AnkiConnectGateway(() => "http://127.0.0.1:8765"); + const stats = await gateway.getDeckStats(["Scoped::Empty", "Scoped::Missing", "Scoped::Busy"]); + + expect(stats).toEqual([ + { deckName: "Scoped::Empty", noteCount: 0 }, + { deckName: "Scoped::Missing", noteCount: undefined }, + { deckName: "Scoped::Busy", noteCount: undefined }, + ]); + }); + + it("treats deck-id keyed stats without a matching name as unknown", async () => { + requestUrlMock.mockResolvedValue({ + json: { + error: null, + result: { + "1651445861967": { + deck_id: 1651445861967, + total_in_deck: 0, + }, + }, + }, + }); + + const gateway = new AnkiConnectGateway(() => "http://127.0.0.1:8765"); + const stats = await gateway.getDeckStats(["Scoped::Empty"]); + + expect(stats).toEqual([ + { deckName: "Scoped::Empty", noteCount: undefined }, + ]); + }); + 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 bfd88d1..9cc160b 100644 --- a/src/infrastructure/anki/AnkiConnectGateway.ts +++ b/src/infrastructure/anki/AnkiConnectGateway.ts @@ -16,6 +16,8 @@ interface NoteInfo { } interface RawDeckStats { + deck_id?: number; + name?: string; total_in_deck?: number; } @@ -247,15 +249,23 @@ function extractDeckNoteCount(rawStats: unknown, deckName: string): number | und return undefined; } - const rawDeckStat = (rawStats as Record)[deckName]; - if (!rawDeckStat || typeof rawDeckStat !== "object") { + for (const rawDeckStat of Object.values(rawStats as Record)) { + if (!rawDeckStat || typeof rawDeckStat !== "object") { + continue; + } + + const deckStat = rawDeckStat as RawDeckStats; + if (deckStat.name !== deckName) { + continue; + } + + const totalInDeck = deckStat.total_in_deck; + if (typeof totalInDeck === "number" && Number.isFinite(totalInDeck)) { + return totalInDeck; + } + return undefined; } - const totalInDeck = (rawDeckStat as RawDeckStats).total_in_deck; - if (typeof totalInDeck !== "number" || !Number.isFinite(totalInDeck)) { - return undefined; - } - - return totalInDeck; + return undefined; } \ No newline at end of file