mirror of
https://github.com/panatgithub/AnkiHeadingSync.git
synced 2026-07-22 06:51:43 +00:00
fix: prevent treating unknown deck stats as cleanup candidates | 修复:防止把未知 deck 统计当成空牌组 cleanup candidates
English: Ensure that decks with unknown statistics are not incorrectly treated as empty candidates for cleanup. 中文:确保统计信息未知的牌组不会被错误地视为待清理的空牌组。
This commit is contained in:
parent
412fddebc1
commit
a5056dd71c
8 changed files with 254 additions and 7 deletions
67
docs/empty-deck-cleanup-fix-decisions.md
Normal file
67
docs/empty-deck-cleanup-fix-decisions.md
Normal file
|
|
@ -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 兼容逻辑
|
||||
61
docs/empty-deck-cleanup-fix-gap-report.md
Normal file
61
docs/empty-deck-cleanup-fix-gap-report.md
Normal file
|
|
@ -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 不完整时宁可少列,不可多列
|
||||
|
|
@ -27,7 +27,7 @@ export interface ChangeDeckInput {
|
|||
|
||||
export interface DeckStat {
|
||||
deckName: string;
|
||||
noteCount: number;
|
||||
noteCount?: number;
|
||||
}
|
||||
|
||||
export interface AnkiGateway {
|
||||
|
|
|
|||
|
|
@ -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"]]);
|
||||
});
|
||||
});
|
||||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -66,13 +66,13 @@ export class AnkiConnectGateway implements AnkiGateway {
|
|||
return [];
|
||||
}
|
||||
|
||||
const rawStats = await this.invoke<Record<string, RawDeckStats>>("getDeckStats", {
|
||||
const rawStats = await this.invoke<unknown>("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<TResult[]>("multi", { actions });
|
||||
}
|
||||
}
|
||||
|
||||
function extractDeckNoteCount(rawStats: unknown, deckName: string): number | undefined {
|
||||
if (!rawStats || typeof rawStats !== "object") {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const rawDeckStat = (rawStats as Record<string, unknown>)[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;
|
||||
}
|
||||
|
|
@ -144,6 +144,7 @@ export class FakeManualSyncAnkiGateway implements AnkiGateway {
|
|||
public storedMedia: MediaAsset[] = [];
|
||||
public noteSummariesById = new Map<number, AnkiNoteSummary>();
|
||||
public deckStatsByName = new Map<string, DeckStat>();
|
||||
public listedDeckNames: string[] | null = null;
|
||||
public modelDetailsByName: Record<string, NoteModelDetails> = {
|
||||
Basic: { fieldNames: ["Front", "Back"], isCloze: false },
|
||||
Cloze: { fieldNames: ["Text", "Extra"], isCloze: true },
|
||||
|
|
@ -164,7 +165,7 @@ export class FakeManualSyncAnkiGateway implements AnkiGateway {
|
|||
}
|
||||
|
||||
async listDeckNames(): Promise<string[]> {
|
||||
return Array.from(this.deckStatsByName.keys());
|
||||
return this.listedDeckNames ? [...this.listedDeckNames] : Array.from(this.deckStatsByName.keys());
|
||||
}
|
||||
|
||||
async getModelDetails(modelName: string): Promise<NoteModelDetails> {
|
||||
|
|
@ -172,7 +173,7 @@ export class FakeManualSyncAnkiGateway implements AnkiGateway {
|
|||
}
|
||||
|
||||
async getDeckStats(deckNames: string[]): Promise<DeckStat[]> {
|
||||
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<AnkiNoteSummary[]> {
|
||||
|
|
|
|||
Loading…
Reference in a new issue