From d7162d3ac20ce36a3bf4ca0fb8fb456e8adb07ee Mon Sep 17 00:00:00 2001 From: afiqzudinhadi Date: Wed, 5 Aug 2026 15:24:50 +0800 Subject: [PATCH] feat(library): interactive candidate picker for ambiguous resolves --- .../src/library/CandidatePickerModal.ts | 60 ++++++++++ .../obsidian/src/library/LibraryController.ts | 39 ++++-- .../obsidian/src/library/LibraryEngine.ts | 25 +++- packages/obsidian/src/library/book.ts | 20 +++- packages/obsidian/src/library/comic.ts | 20 +++- packages/obsidian/src/library/game.ts | 29 +++-- packages/obsidian/src/library/manga.ts | 29 ++++- packages/obsidian/src/library/types.ts | 11 +- tests/library-book.test.ts | 21 ++-- tests/library-candidate-picker-modal.test.ts | 57 +++++++++ tests/library-comic.test.ts | 15 ++- tests/library-controller.test.ts | 111 ++++++++++++++++++ tests/library-engine.test.ts | 61 +++++++++- tests/library-game.test.ts | 21 ++-- tests/library-manga.test.ts | 50 +++++--- tests/setup.ts | 34 ++++-- 16 files changed, 516 insertions(+), 87 deletions(-) create mode 100644 packages/obsidian/src/library/CandidatePickerModal.ts create mode 100644 tests/library-candidate-picker-modal.test.ts diff --git a/packages/obsidian/src/library/CandidatePickerModal.ts b/packages/obsidian/src/library/CandidatePickerModal.ts new file mode 100644 index 0000000..0fb5f6f --- /dev/null +++ b/packages/obsidian/src/library/CandidatePickerModal.ts @@ -0,0 +1,60 @@ +import type { App } from 'obsidian'; +import { FuzzySuggestModal } from 'obsidian'; +import type { ResolveCandidate } from 'packages/obsidian/src/library/types'; + +const SKIP_LABEL = 'Skip'; + +interface PickerItem { + label: string; + index: number | null; // null = Skip +} + +/** + * Fuzzy picker for an ambiguous resolve() result. `pick()` resolves to the chosen candidate's + * index, or null when the user picks "Skip" / dismisses the modal (Esc, click-outside) without + * choosing -- both are treated identically by the caller (LibraryController.resolveType). + */ +export class CandidatePickerModal extends FuzzySuggestModal { + private settled = false; + private resolveFn?: (index: number | null) => void; + + constructor( + app: App, + private filename: string, + private candidates: ResolveCandidate[], + ) { + super(app); + this.setTitle(filename); + this.setPlaceholder(`Pick a match for ${filename}`); + } + + getItems(): PickerItem[] { + return [...this.candidates.map((c, index) => ({ label: c.label, index })), { label: SKIP_LABEL, index: null }]; + } + + getItemText(item: PickerItem): string { + return item.label; + } + + onChooseItem(item: PickerItem, _evt: MouseEvent | KeyboardEvent): void { + this.settle(item.index); + } + + onClose(): void { + this.settle(null); // dismissed without choosing -> treat as skip + this.contentEl.empty(); + } + + private settle(index: number | null): void { + if (this.settled) return; + this.settled = true; + this.resolveFn?.(index); + } + + pick(): Promise { + return new Promise(resolve => { + this.resolveFn = resolve; + this.open(); + }); + } +} diff --git a/packages/obsidian/src/library/LibraryController.ts b/packages/obsidian/src/library/LibraryController.ts index 9b01b2f..528d9c2 100644 --- a/packages/obsidian/src/library/LibraryController.ts +++ b/packages/obsidian/src/library/LibraryController.ts @@ -1,12 +1,14 @@ import { Notice, TFile, TFolder } from 'obsidian'; import { bookSpec } from 'packages/obsidian/src/library/book'; +import { CandidatePickerModal } from 'packages/obsidian/src/library/CandidatePickerModal'; import { comicSpec } from 'packages/obsidian/src/library/comic'; import { gameSpec } from 'packages/obsidian/src/library/game'; import { libraryFolderResolve, libraryFolderSync, type LibraryEngineDeps, type LibraryReport, type LibraryResolveReport } from 'packages/obsidian/src/library/LibraryEngine'; import { mangaSpec } from 'packages/obsidian/src/library/manga'; -import type { HttpJsonFn, HttpPostJsonFn, HttpTextFn, MediaTypeSpec, SpecDeps } from 'packages/obsidian/src/library/types'; +import type { HttpJsonFn, HttpPostJsonFn, HttpTextFn, MediaTypeSpec, ResolveCandidate, SpecDeps } from 'packages/obsidian/src/library/types'; import type MediaDbPlugin from 'packages/obsidian/src/main'; import { obsidianFetch } from 'packages/obsidian/src/utils/Utils'; +import { patchFrontmatter } from 'packages/obsidian/src/watchlist/patchFrontmatter'; import { TmdbRateLimitError } from 'packages/obsidian/src/watchlist/SyncEngine'; import { shouldNotifySync } from 'packages/obsidian/src/watchlist/WatchlistController'; @@ -34,7 +36,7 @@ function emptyReport(): LibraryReport { } function emptyResolveReport(): LibraryResolveReport { - return { resolved: [], ambiguous: [], errors: [] }; + return { resolved: [], ambiguous: [], needsChoice: [], skippedNoResolve: 0, errors: [] }; } export class LibraryController { @@ -205,6 +207,12 @@ export class LibraryController { } } + /** Opens the real candidate picker modal; tests override this method directly to avoid needing a real Obsidian modal. */ + private async pickCandidate(filename: string, candidates: ResolveCandidate[]): Promise { + const modal = new CandidatePickerModal(this.plugin.app, filename, candidates); + return await modal.pick(); + } + async resolveType(spec: MediaTypeSpec, dryRun = false): Promise { if (this.syncing) { this.notify('Library sync already running'); @@ -214,12 +222,29 @@ export class LibraryController { try { const deps = this.makeDeps(spec, dryRun); const report = await libraryFolderResolve(spec, deps, { dryRun }); + + let picked = 0; + let skipped = 0; + if (!dryRun) { + for (const entry of report.needsChoice) { + const idx = await this.pickCandidate(entry.filename, entry.candidates); + const candidate = idx != null ? entry.candidates[idx] : undefined; + if (!candidate) { + skipped++; + continue; + } + const content = await deps.readNote(entry.path); + await deps.writeNote(entry.path, patchFrontmatter(content, candidate.patches, { defaultType: spec.itemType })); + report.resolved.push(entry.path); + picked++; + } + } + const mode = dryRun ? 'DRY-RUN ' : ''; - this.notify( - `Library ${mode}resolve (${spec.typeName}): ${report.resolved.length} resolved, ${report.ambiguous.length} ambiguous/no match` + - (report.errors.length ? `, ${report.errors.length} errors (see console)` : ''), - 0, - ); + let msg = `Library ${mode}resolve (${spec.typeName}): ${report.resolved.length} resolved, ${report.ambiguous.length} ambiguous/no match`; + if (report.needsChoice.length) msg += `, ${picked} picked, ${skipped} skipped`; + if (report.errors.length) msg += `, ${report.errors.length} errors (see console)`; + this.notify(msg, 0); return report; } finally { this.syncing = false; diff --git a/packages/obsidian/src/library/LibraryEngine.ts b/packages/obsidian/src/library/LibraryEngine.ts index 6fe135d..8488d8e 100644 --- a/packages/obsidian/src/library/LibraryEngine.ts +++ b/packages/obsidian/src/library/LibraryEngine.ts @@ -1,4 +1,4 @@ -import type { LibraryNoteCtx, MediaTypeSpec, SpecDeps } from 'packages/obsidian/src/library/types'; +import type { LibraryNoteCtx, MediaTypeSpec, ResolveCandidate, SpecDeps } from 'packages/obsidian/src/library/types'; import { parseNote, stripQuotes } from 'packages/obsidian/src/watchlist/parse'; import { patchFrontmatter } from 'packages/obsidian/src/watchlist/patchFrontmatter'; import { withRateLimitRetry } from 'packages/obsidian/src/watchlist/SyncEngine'; @@ -26,6 +26,8 @@ export interface LibraryReport { export interface LibraryResolveReport { resolved: string[]; ambiguous: string[]; + needsChoice: { path: string; filename: string; candidates: ResolveCandidate[] }[]; + skippedNoResolve: number; errors: { path: string; error: string }[]; } @@ -89,8 +91,13 @@ export async function libraryFolderSync(spec: MediaTypeSpec, deps: LibraryEngine return report; } +/** `no_resolve: true` opts a note out of the entire resolve pass (all specs), same as a manual skip. */ +function hasNoResolveFlag(fm: Record): boolean { + return stripQuotes(fm['no_resolve']) === 'true'; +} + export async function libraryFolderResolve(spec: MediaTypeSpec, deps: LibraryEngineDeps, opts: { dryRun?: boolean } = {}): Promise { - const report: LibraryResolveReport = { resolved: [], ambiguous: [], errors: [] }; + const report: LibraryResolveReport = { resolved: [], ambiguous: [], needsChoice: [], skippedNoResolve: 0, errors: [] }; const notes = await deps.listNotes(); for (const note of notes) { try { @@ -98,16 +105,24 @@ export async function libraryFolderResolve(spec: MediaTypeSpec, deps: LibraryEng const { frontmatter, body } = parseNote(content); const filename = filenameOf(note.path); if (isSkippableNote(filename, frontmatter, spec.itemType)) continue; + if (hasNoResolveFlag(frontmatter)) { + report.skippedNoResolve++; + deps.log(`no_resolve flag set, skipping: ${note.path}`); + continue; + } if (spec.hasId(frontmatter)) continue; // already resolved const ctx: LibraryNoteCtx = { frontmatter, body, filename }; - const patch = await withRateLimitRetry(() => spec.resolve(ctx, deps.specDeps), deps.sleep); - if (!patch) { + const outcome = await withRateLimitRetry(() => spec.resolve(ctx, deps.specDeps), deps.sleep); + if (!outcome) { report.ambiguous.push(note.path); deps.log(`ambiguous/no match: ${note.path}`); + } else if ('candidates' in outcome) { + report.needsChoice.push({ path: note.path, filename, candidates: outcome.candidates }); + deps.log(`needs choice (${outcome.candidates.length} candidates): ${note.path}`); } else { report.resolved.push(note.path); deps.log(`${opts.dryRun ? '[dry] ' : ''}resolved ${note.path}`); - if (!opts.dryRun) await deps.writeNote(note.path, patchFrontmatter(content, patch, { defaultType: spec.itemType })); + if (!opts.dryRun) await deps.writeNote(note.path, patchFrontmatter(content, outcome.patches, { defaultType: spec.itemType })); } await deps.sleep(spec.throttleMs); } catch (e) { diff --git a/packages/obsidian/src/library/book.ts b/packages/obsidian/src/library/book.ts index 02af346..72624a7 100644 --- a/packages/obsidian/src/library/book.ts +++ b/packages/obsidian/src/library/book.ts @@ -1,4 +1,4 @@ -import type { LibraryNoteCtx, MediaTypeSpec, SpecDeps } from 'packages/obsidian/src/library/types'; +import type { LibraryNoteCtx, MediaTypeSpec, ResolveOutcome, SpecDeps } from 'packages/obsidian/src/library/types'; import { stripQuotes, extractMyNotes, extractCustomSections, type CustomSection } from 'packages/obsidian/src/watchlist/parse'; import { yamlList, yamlScalar, quotedOrNull } from 'packages/obsidian/src/watchlist/yaml'; import { deriveReadStatus, deriveRating } from 'packages/obsidian/src/library/convert'; @@ -134,6 +134,14 @@ function candidateSummary(d: any): string { return `olid=${olid} «${d.title ?? ''}»${year ? ` (${year})` : ''}${author ? ` ${author}` : ''}`; } +/** Human-readable picker label -- same title/year/author info as candidateSummary, minus the olid prefix. */ +function candidateLabel(d: any): string { + const year = d.first_publish_year != null ? String(d.first_publish_year) : ''; + const author = Array.isArray(d.author_name) ? d.author_name.filter(Boolean).join(', ') : ''; + const meta = [author, year].filter(Boolean).join(', '); + return meta ? `${d.title ?? ''} (${meta})` : (d.title ?? ''); +} + export const bookSpec: MediaTypeSpec = { typeName: 'book', itemType: 'book_item', @@ -154,7 +162,7 @@ export const bookSpec: MediaTypeSpec = { return !stripQuotes(fm['olid']) || !stripQuotes(fm['read_status']); }, - async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise | null> { + async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise { const title = stripQuotes(ctx.frontmatter['title']) || ctx.filename.replace(/\.md$/, ''); if (!title) return null; const hint = authorHint(ctx.frontmatter); @@ -167,10 +175,14 @@ export const bookSpec: MediaTypeSpec = { const pick = exacts.length === 1 ? exacts[0] : exacts.length === 0 && docs.length === 1 ? docs[0] : null; if (!pick) { deps.log(`ambiguous "${searchQuery}": candidates: ${docs.slice(0, 3).map(candidateSummary).join('; ')}`); - return null; + const candidates = docs + .slice(0, 6) + .map(d => ({ label: candidateLabel(d), patches: { olid: String(d.key ?? '').replace(/^\/works\//, '') } })) + .filter(c => c.patches.olid); + return candidates.length ? { candidates } : null; } const olid = String(pick.key ?? '').replace(/^\/works\//, ''); - return olid ? { olid } : null; + return olid ? { patches: { olid } } : null; } catch (e) { deps.log(`book resolve failed for "${searchQuery}": ${String(e)}`); return null; diff --git a/packages/obsidian/src/library/comic.ts b/packages/obsidian/src/library/comic.ts index ab27433..f279d98 100644 --- a/packages/obsidian/src/library/comic.ts +++ b/packages/obsidian/src/library/comic.ts @@ -1,4 +1,4 @@ -import type { LibraryNoteCtx, MediaTypeSpec, SpecDeps } from 'packages/obsidian/src/library/types'; +import type { LibraryNoteCtx, MediaTypeSpec, ResolveOutcome, SpecDeps } from 'packages/obsidian/src/library/types'; import { stripQuotes, extractMyNotes, extractCustomSections, type CustomSection } from 'packages/obsidian/src/watchlist/parse'; import { yamlList, yamlScalar, quotedOrNull } from 'packages/obsidian/src/watchlist/yaml'; import { deriveReadStatus, deriveRating, parseNumOrNull } from 'packages/obsidian/src/library/convert'; @@ -170,6 +170,14 @@ function candidateSummary(r: any): string { return `id=${r.id ?? ''} «${r.name ?? ''}»${year ? ` (${year})` : ''}${publisher ? ` ${publisher}` : ''}`; } +/** Human-readable picker label -- same name/year/publisher info as candidateSummary, minus the id prefix. */ +function candidateLabel(r: any): string { + const year = r.start_year != null ? String(r.start_year) : ''; + const publisher = r.publisher?.name ?? ''; + const meta = [publisher, year].filter(Boolean).join(', '); + return meta ? `${r.name ?? ''} (${meta})` : (r.name ?? ''); +} + export const comicSpec: MediaTypeSpec = { typeName: 'comic', itemType: 'comic_item', @@ -191,7 +199,7 @@ export const comicSpec: MediaTypeSpec = { return false; }, - async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise | null> { + async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise { const key = deps.getKey('comicvine'); if (!key) { deps.log('comic resolve: no Comic Vine key configured, skipping id lookup'); @@ -208,9 +216,13 @@ export const comicSpec: MediaTypeSpec = { const pick = exacts.length === 1 ? exacts[0] : exacts.length === 0 && results.length === 1 ? results[0] : null; if (!pick) { deps.log(`ambiguous "${query}": candidates: ${results.slice(0, 3).map(candidateSummary).join('; ')}`); - return null; + const candidates = results + .slice(0, 6) + .filter(r => r.id != null) + .map(r => ({ label: candidateLabel(r), patches: { comicvine_id: String(r.id) } })); + return candidates.length ? { candidates } : null; } - return pick.id != null ? { comicvine_id: String(pick.id) } : null; + return pick.id != null ? { patches: { comicvine_id: String(pick.id) } } : null; } catch (e) { deps.log(`comic resolve failed for "${query}": ${String(e)}`); return null; diff --git a/packages/obsidian/src/library/game.ts b/packages/obsidian/src/library/game.ts index d26e643..365d54f 100644 --- a/packages/obsidian/src/library/game.ts +++ b/packages/obsidian/src/library/game.ts @@ -1,4 +1,4 @@ -import type { LibraryNoteCtx, MediaTypeSpec, SpecDeps } from 'packages/obsidian/src/library/types'; +import type { LibraryNoteCtx, MediaTypeSpec, ResolveCandidate, ResolveOutcome, SpecDeps } from 'packages/obsidian/src/library/types'; import { stripQuotes, extractMyNotes, extractCustomSections, type CustomSection } from 'packages/obsidian/src/watchlist/parse'; import { yamlList, yamlScalar, quotedOrNull } from 'packages/obsidian/src/watchlist/yaml'; import { deriveRating } from 'packages/obsidian/src/library/convert'; @@ -173,6 +173,12 @@ function candidateSummary(label: string, id: unknown, name: string, year?: strin return `${label}=${id ?? ''} «${name}»${year ? ` (${year})` : ''}`; } +/** Human-readable picker label for a Steam storesearch item -- Steam's search payload carries no + * year/date, so name is all there is to show (matches what candidateSummary already logs for it). */ +function steamCandidateLabel(it: any): string { + return String(it?.name ?? ''); +} + async function fetchSteamSearch(query: string, deps: SpecDeps): Promise { const qs = new URLSearchParams({ term: query, cc: 'us', l: 'en' }); const res = await deps.http(`${STEAM_STORE_BASE}/api/storesearch/?${qs.toString()}`, {}); @@ -205,21 +211,30 @@ export const gameSpec: MediaTypeSpec = { return (!stripQuotes(fm['steam_appid']) && !stripQuotes(fm['rawg_id'])) || !stripQuotes(fm['play_status']); }, - async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise | null> { + // Steam-primary: RAWG is still tried as a fallback on a Steam ambiguous/miss (unchanged). Only + // when NEITHER source lands a unique match do Steam's ambiguous results (if any) come back as + // candidates for the user to pick from, instead of a bare null -- RAWG-only ambiguity (Steam + // had 0/1 results) still returns null unchanged. + async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise { // fastest, no-network path: 28/31 real game notes already carry a Steam store url const url = stripQuotes(ctx.frontmatter['url']); const urlMatch = /store\.steampowered\.com\/app\/(\d+)/.exec(url); - if (urlMatch) return { steam_appid: urlMatch[1] }; + if (urlMatch) return { patches: { steam_appid: urlMatch[1] } }; const query = stripQuotes(ctx.frontmatter['title']) || ctx.filename.replace(/\.md$/, ''); if (!query) return null; + let steamCandidates: ResolveCandidate[] = []; try { const items = await fetchSteamSearch(query, deps); const pick = pickUniqueExact(items, query, (it: any) => String(it.name ?? '')); - if (pick) return { steam_appid: String(pick.id) }; + if (pick) return { patches: { steam_appid: String(pick.id) } }; if (items.length > 0) { deps.log(`ambiguous "${query}": candidates: ${items.slice(0, 3).map((it: any) => candidateSummary('appid', it.id, it.name ?? '')).join('; ')}`); + steamCandidates = items + .slice(0, 6) + .filter((it: any) => it.id != null) + .map((it: any) => ({ label: steamCandidateLabel(it), patches: { steam_appid: String(it.id) } })); } } catch (e) { deps.log(`game steam storesearch failed for "${query}": ${String(e)}`); @@ -228,12 +243,12 @@ export const gameSpec: MediaTypeSpec = { const key = deps.getKey('rawg'); if (!key) { deps.log(`game resolve: no RAWG key configured, skipping RAWG fallback for "${query}"`); - return null; + return steamCandidates.length ? { candidates: steamCandidates } : null; } try { const results = await fetchRawgSearch(query, key, deps); const pick = pickUniqueExact(results, query, (it: any) => String(it.name ?? '')); - if (pick) return { rawg_id: String(pick.id) }; + if (pick) return { patches: { rawg_id: String(pick.id) } }; if (results.length > 0) { const year = (it: any) => (typeof it.released === 'string' ? it.released.slice(0, 4) : ''); deps.log(`ambiguous "${query}": candidates: ${results.slice(0, 3).map((it: any) => candidateSummary('rawg_id', it.id, it.name ?? '', year(it))).join('; ')}`); @@ -241,7 +256,7 @@ export const gameSpec: MediaTypeSpec = { } catch (e) { deps.log(`game rawg search failed for "${query}": ${String(e)}`); } - return null; + return steamCandidates.length ? { candidates: steamCandidates } : null; }, async sync(ctx: LibraryNoteCtx, deps: SpecDeps): Promise<{ content: string; flipped: boolean } | null> { diff --git a/packages/obsidian/src/library/manga.ts b/packages/obsidian/src/library/manga.ts index 3f1908b..fabdc59 100644 --- a/packages/obsidian/src/library/manga.ts +++ b/packages/obsidian/src/library/manga.ts @@ -1,4 +1,4 @@ -import type { LibraryNoteCtx, MediaTypeSpec, SpecDeps } from 'packages/obsidian/src/library/types'; +import type { LibraryNoteCtx, MediaTypeSpec, ResolveCandidate, ResolveOutcome, SpecDeps } from 'packages/obsidian/src/library/types'; import { parseFeed, latestChapter } from 'packages/obsidian/src/library/rss'; import { stripQuotes, extractMyNotes, extractCustomSections, type CustomSection } from 'packages/obsidian/src/watchlist/parse'; import { yamlList, yamlScalar, quotedOrNull } from 'packages/obsidian/src/watchlist/yaml'; @@ -256,6 +256,13 @@ function candidateAniListSummary(m: any): string { return `anilist_id=${m?.id ?? ''} «${m?.title?.romaji ?? ''}»${year ? ` (${year})` : ''}`; } +/** Human-readable picker label (AniList side) -- same title/year info as candidateAniListSummary, minus the id prefix. */ +function candidateAniListLabel(m: any): string { + const title = m?.title?.romaji || m?.title?.english || ''; + const year = m?.startDate?.year ? String(m.startDate.year) : ''; + return year ? `${title} (${year})` : title; +} + export function renderManga(r: MangaRecord, myNotes: string, customSections: CustomSection[] = []): string { const fm = [ '---', @@ -421,10 +428,14 @@ export const mangaSpec: MediaTypeSpec = { // AniList-primary: search AniList first (Page search, unique-exact vs romaji+english). A // unique hit patches anilist_id (+ mal_id via idMal, when AniList has a MAL bridge for it). // AniList miss/ambiguous/throw all fall back to the existing Jikan title-search resolve. + // When BOTH AniList and Jikan fail to land a unique match, an AniList-ambiguous result set + // (2+ exacts, or 0-exact-multi) is offered back to the caller as candidates for the user to + // pick from, instead of a bare null -- Jikan-only ambiguity (AniList had 0/1 results) still + // returns null unchanged, since AniList carries the richer id bridge worth surfacing. // TmdbRateLimitError is never treated as a fallback trigger -- it propagates so the engine's // withRateLimitRetry wrapper (around the whole resolve() call) retries instead of masking a // transient 429 as an AniList miss. - async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise | null> { + async resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise { const query = stripQuotes(ctx.frontmatter['title']) || ctx.filename.replace(/\.md$/, ''); if (!query) return null; @@ -448,11 +459,17 @@ export const mangaSpec: MediaTypeSpec = { const mangadexId = await resolveMangadexId(query, deps); if (mangadexId) patch.mangadex_id = mangadexId; } - return patch; + return { patches: patch }; } + let anilistCandidates: ResolveCandidate[] = []; if (anilistResults.length > 1) { deps.log(`ambiguous "${query}": candidates: ${anilistResults.slice(0, 3).map(candidateAniListSummary).join('; ')}`); + anilistCandidates = anilistResults.slice(0, 6).map(m => { + const patch: Record = { anilist_id: String(m.id) }; + if (m.idMal != null) patch.mal_id = String(m.idMal); + return { label: candidateAniListLabel(m), patches: patch }; + }); } // Jikan fallback (unchanged behavior) @@ -462,9 +479,9 @@ export const mangaSpec: MediaTypeSpec = { } catch (e) { if (e instanceof TmdbRateLimitError) throw e; deps.log(`manga resolve failed for "${query}": ${String(e)}`); - return null; + return anilistCandidates.length ? { candidates: anilistCandidates } : null; } - if (!malId) return null; + if (!malId) return anilistCandidates.length ? { candidates: anilistCandidates } : null; const patch: Record = { mal_id: malId }; // best-effort: only attempt when the note doesn't already carry a mangadex_id @@ -472,7 +489,7 @@ export const mangaSpec: MediaTypeSpec = { const mangadexId = await resolveMangadexId(query, deps); if (mangadexId) patch.mangadex_id = mangadexId; } - return patch; + return { patches: patch }; }, // AniList-primary: anilist_id present -> fetch by id; else mal_id -> fetch by idMal bridge. diff --git a/packages/obsidian/src/library/types.ts b/packages/obsidian/src/library/types.ts index 0a60417..5a1476f 100644 --- a/packages/obsidian/src/library/types.ts +++ b/packages/obsidian/src/library/types.ts @@ -8,6 +8,15 @@ export interface LibraryNoteCtx { filename: string; } +/** One candidate offered to the user when resolve() can't pick a unique match on its own. */ +export interface ResolveCandidate { + label: string; // human-readable (title + year/author/publisher/etc, whatever the spec already logs) + patches: Record; // full id-field patch for this candidate (e.g. anilist_id + mal_id) +} + +/** resolve() outcome: either a confident unique-match patch, or a shortlist for the user to pick from. */ +export type ResolveOutcome = { patches: Record } | { candidates: ResolveCandidate[] }; + export interface MediaTypeSpec { typeName: 'manga' | 'book' | 'game' | 'comic'; itemType: string; // 'manga_item' etc. @@ -16,7 +25,7 @@ export interface MediaTypeSpec { throttleMs: number; hasId(fm: Record): boolean; isActive(fm: Record): boolean; - resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise | null>; // returns fm patches (id fields) + resolve(ctx: LibraryNoteCtx, deps: SpecDeps): Promise; // null = no match/error sync(ctx: LibraryNoteCtx, deps: SpecDeps): Promise<{ content: string; flipped: boolean } | null>; // full new note content; null = skip } diff --git a/tests/library-book.test.ts b/tests/library-book.test.ts index 6c3e543..6c6cfaf 100644 --- a/tests/library-book.test.ts +++ b/tests/library-book.test.ts @@ -305,16 +305,16 @@ describe('bookSpec.resolve', () => { test('unique exact title match -> accepted', async () => { const deps = makeDeps({ http: async () => olFixture }); const result = await bookSpec.resolve(ctxFor({ title: '1984' }, ''), deps); - expect(result).toEqual({ olid: 'OL1168083W' }); + expect(result).toEqual({ patches: { olid: 'OL1168083W' } }); }); test('no exact match, sole result -> accepted', async () => { const deps = makeDeps({ http: async () => ({ docs: [{ key: '/works/OL999W', title: 'Some Other Title' }] }), }); const result = await bookSpec.resolve(ctxFor({ title: '1984' }, ''), deps); - expect(result).toEqual({ olid: 'OL999W' }); + expect(result).toEqual({ patches: { olid: 'OL999W' } }); }); - test('ambiguous (multiple results, no exact match) -> null', async () => { + test('ambiguous (multiple results, no exact match) -> candidates (top ≤6, label + full patches)', async () => { const deps = makeDeps({ http: async () => ({ docs: [ @@ -324,7 +324,12 @@ describe('bookSpec.resolve', () => { }), }); const result = await bookSpec.resolve(ctxFor({ title: '1984' }, ''), deps); - expect(result).toBeNull(); + expect(result).toEqual({ + candidates: [ + { label: 'Foo', patches: { olid: 'OL1W' } }, + { label: 'Bar', patches: { olid: 'OL2W' } }, + ], + }); }); test('ambiguous -> logs top candidates with olid + title', async () => { const deps = makeDeps({ @@ -336,7 +341,7 @@ describe('bookSpec.resolve', () => { }), }); const result = await bookSpec.resolve(ctxFor({ title: '1984' }, ''), deps); - expect(result).toBeNull(); + expect(result && 'candidates' in result).toBe(true); expect(deps.logCalls.some(m => m.includes('ambiguous') && m.includes('olid=OL1W') && m.includes('Foo') && m.includes('olid=OL2W') && m.includes('Bar'))).toBe(true); }); test('no results -> null', async () => { @@ -368,7 +373,7 @@ describe('bookSpec.resolve — author hint (stock `author` / canonical `authors` const result = await bookSpec.resolve(ctxFor({ title: '1984', author: 'George Orwell' }, ''), deps); const q = new URL(capturedUrl).searchParams.get('q'); expect(q).toBe('1984 George Orwell'); - expect(result).toEqual({ olid: 'OL1168083W' }); + expect(result).toEqual({ patches: { olid: 'OL1168083W' } }); }); test('canonical `authors` bracketed list -> first author used in query', async () => { @@ -382,7 +387,7 @@ describe('bookSpec.resolve — author hint (stock `author` / canonical `authors` const result = await bookSpec.resolve(ctxFor({ title: '1984', authors: '[George Orwell, Someone Else]' }, ''), deps); const q = new URL(capturedUrl).searchParams.get('q'); expect(q).toBe('1984 George Orwell'); - expect(result).toEqual({ olid: 'OL1168083W' }); + expect(result).toEqual({ patches: { olid: 'OL1168083W' } }); }); test('no author anywhere in frontmatter -> query is title only (unchanged behavior)', async () => { @@ -396,6 +401,6 @@ describe('bookSpec.resolve — author hint (stock `author` / canonical `authors` const result = await bookSpec.resolve(ctxFor({ title: '1984' }, ''), deps); const q = new URL(capturedUrl).searchParams.get('q'); expect(q).toBe('1984'); - expect(result).toEqual({ olid: 'OL1168083W' }); + expect(result).toEqual({ patches: { olid: 'OL1168083W' } }); }); }); diff --git a/tests/library-candidate-picker-modal.test.ts b/tests/library-candidate-picker-modal.test.ts new file mode 100644 index 0000000..dc95b24 --- /dev/null +++ b/tests/library-candidate-picker-modal.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, test } from 'bun:test'; +import { CandidatePickerModal } from 'packages/obsidian/src/library/CandidatePickerModal'; + +const CANDIDATES = [ + { label: 'Foo (2020)', patches: { mal_id: '1' } }, + { label: 'Bar (2021)', patches: { mal_id: '2' } }, +]; + +function fakeApp(): any { + return {}; +} + +describe('CandidatePickerModal', () => { + test('getItems: candidate labels followed by a trailing Skip item (null index)', () => { + const modal = new CandidatePickerModal(fakeApp(), 'Some Note.md', CANDIDATES); + const items = modal.getItems(); + expect(items).toEqual([ + { label: 'Foo (2020)', index: 0 }, + { label: 'Bar (2021)', index: 1 }, + { label: 'Skip', index: null }, + ]); + }); + + test('getItemText returns the item label', () => { + const modal = new CandidatePickerModal(fakeApp(), 'Some Note.md', CANDIDATES); + expect(modal.getItemText({ label: 'Foo (2020)', index: 0 })).toBe('Foo (2020)'); + }); + + test('onChooseItem(candidate) -> pick() resolves to that candidate index', async () => { + const modal = new CandidatePickerModal(fakeApp(), 'Some Note.md', CANDIDATES); + const result = modal.pick(); + modal.onChooseItem({ label: 'Bar (2021)', index: 1 }, {} as MouseEvent); + expect(await result).toBe(1); + }); + + test('onChooseItem(Skip) -> pick() resolves to null', async () => { + const modal = new CandidatePickerModal(fakeApp(), 'Some Note.md', CANDIDATES); + const result = modal.pick(); + modal.onChooseItem({ label: 'Skip', index: null }, {} as MouseEvent); + expect(await result).toBeNull(); + }); + + test('onClose without a prior choice (Esc / dismiss) -> pick() resolves to null', async () => { + const modal = new CandidatePickerModal(fakeApp(), 'Some Note.md', CANDIDATES); + const result = modal.pick(); + modal.onClose(); + expect(await result).toBeNull(); + }); + + test('onClose firing after onChooseItem does not override the already-settled choice', async () => { + const modal = new CandidatePickerModal(fakeApp(), 'Some Note.md', CANDIDATES); + const result = modal.pick(); + modal.onChooseItem({ label: 'Foo (2020)', index: 0 }, {} as MouseEvent); + modal.onClose(); // Obsidian calls onClose() after a choice too -- must not clobber the resolved value + expect(await result).toBe(0); + }); +}); diff --git a/tests/library-comic.test.ts b/tests/library-comic.test.ts index dd01f60..4b496af 100644 --- a/tests/library-comic.test.ts +++ b/tests/library-comic.test.ts @@ -266,7 +266,7 @@ describe('comicSpec.resolve', () => { getKey: () => 'cvkey', }); const result = await comicSpec.resolve(ctxFor({ title: 'Absolute Batman' }), deps); - expect(result).toEqual({ comicvine_id: '195824' }); + expect(result).toEqual({ patches: { comicvine_id: '195824' } }); }); test('no exact match, sole result -> accepted', async () => { @@ -275,10 +275,10 @@ describe('comicSpec.resolve', () => { getKey: () => 'cvkey', }); const result = await comicSpec.resolve(ctxFor({ title: 'Absolute Batman' }), deps); - expect(result).toEqual({ comicvine_id: '999' }); + expect(result).toEqual({ patches: { comicvine_id: '999' } }); }); - test('ambiguous (multiple results, no exact match) -> null', async () => { + test('ambiguous (multiple results, no exact match) -> candidates (top ≤6, label + full patches)', async () => { const deps = makeDeps({ http: async () => ({ results: [ @@ -289,7 +289,12 @@ describe('comicSpec.resolve', () => { getKey: () => 'cvkey', }); const result = await comicSpec.resolve(ctxFor({ title: 'Absolute Batman' }), deps); - expect(result).toBeNull(); + expect(result).toEqual({ + candidates: [ + { label: 'Batman', patches: { comicvine_id: '1' } }, + { label: 'Batman Beyond', patches: { comicvine_id: '2' } }, + ], + }); }); test('ambiguous -> logs top candidates with id + name', async () => { const deps = makeDeps({ @@ -302,7 +307,7 @@ describe('comicSpec.resolve', () => { getKey: () => 'cvkey', }); const result = await comicSpec.resolve(ctxFor({ title: 'Absolute Batman' }), deps); - expect(result).toBeNull(); + expect(result && 'candidates' in result).toBe(true); expect(deps.logCalls.some(m => m.includes('ambiguous') && m.includes('id=1') && m.includes('Batman') && m.includes('id=2') && m.includes('Batman Beyond'))).toBe(true); }); diff --git a/tests/library-controller.test.ts b/tests/library-controller.test.ts index e5bfac1..f9ebb51 100644 --- a/tests/library-controller.test.ts +++ b/tests/library-controller.test.ts @@ -461,3 +461,114 @@ describe('sync summary transparency', () => { expect(notices[0]).toContain('1 no-data (see console)'); }); }); + +describe('resolveType: interactive candidate picker (needsChoice)', () => { + const CANDIDATES = [ + { label: 'Foo', patches: { mal_id: '1' } }, + { label: 'Bar', patches: { mal_id: '2' } }, + ]; + + const candidateSpec: MediaTypeSpec = { + typeName: 'manga', + itemType: 'manga_item', + folderSettingKey: 'libraryMangaFolder', + enabledSettingKey: 'libraryMangaEnabled', + throttleMs: 0, + hasId: () => false, + isActive: () => true, + resolve: async () => ({ candidates: CANDIDATES }), + sync: async () => null, + }; + + function makeCandidateDeps(writes: { path: string; content: string }[]): LibraryEngineDeps { + return { + listNotes: async () => [{ path: 'Mangas/Foo.md' }], + readNote: async () => '---\ntype: manga_item\ntitle: Foo\n---\n\nbody\n', + writeNote: async (path: string, content: string) => { + writes.push({ path, content }); + }, + sleep: async () => {}, + log: () => {}, + specDeps: fakeSpecDeps(), + }; + } + + test('pickCandidate fake selects index 0 -> patch applied via patchFrontmatter, counted resolved + picked', async () => { + const c = new LibraryController(fakePlugin()); + const notices: string[] = []; + (c as any).notify = (msg: string) => notices.push(msg); + const writes: { path: string; content: string }[] = []; + (c as any).makeDeps = () => makeCandidateDeps(writes); + let pickedFilename = ''; + let pickedCandidates: unknown = undefined; + (c as any).pickCandidate = async (filename: string, candidates: unknown) => { + pickedFilename = filename; + pickedCandidates = candidates; + return 0; + }; + + const report = await c.resolveType(candidateSpec); + + expect(report.needsChoice).toEqual([{ path: 'Mangas/Foo.md', filename: 'Foo.md', candidates: CANDIDATES }]); + expect(report.resolved).toEqual(['Mangas/Foo.md']); + expect(writes.length).toBe(1); + expect(writes[0].path).toBe('Mangas/Foo.md'); + expect(writes[0].content).toContain('mal_id: 1'); + expect(pickedFilename).toBe('Foo.md'); + expect(pickedCandidates).toEqual(CANDIDATES); + expect(notices[0]).toContain('1 picked, 0 skipped'); + }); + + test('pickCandidate fake returns null (Skip) -> no write, counted skipped, not resolved', async () => { + const c = new LibraryController(fakePlugin()); + const notices: string[] = []; + (c as any).notify = (msg: string) => notices.push(msg); + const writes: { path: string; content: string }[] = []; + (c as any).makeDeps = () => makeCandidateDeps(writes); + (c as any).pickCandidate = async () => null; + + const report = await c.resolveType(candidateSpec); + + expect(report.resolved).toEqual([]); + expect(writes.length).toBe(0); + expect(notices[0]).toContain('0 picked, 1 skipped'); + }); + + test('dryRun: needsChoice collected but pickCandidate never invoked, no write', async () => { + const c = new LibraryController(fakePlugin()); + const notices: string[] = []; + (c as any).notify = (msg: string) => notices.push(msg); + const writes: { path: string; content: string }[] = []; + (c as any).makeDeps = () => makeCandidateDeps(writes); + let pickCalled = false; + (c as any).pickCandidate = async () => { + pickCalled = true; + return 0; + }; + + const report = await c.resolveType(candidateSpec, true); + + expect(pickCalled).toBe(false); + expect(report.needsChoice.length).toBe(1); + expect(writes.length).toBe(0); + expect(notices[0]).toContain('0 picked, 0 skipped'); // dry-run never picks/skips; needsChoice.length is still visible on the report + }); + + test('no needsChoice entries -> summary omits the picked/skipped clause entirely', async () => { + const c = new LibraryController(fakePlugin()); + const notices: string[] = []; + (c as any).notify = (msg: string) => notices.push(msg); + (c as any).makeDeps = () => ({ + listNotes: async () => [], + readNote: async () => '', + writeNote: async () => {}, + sleep: async () => {}, + log: () => {}, + specDeps: fakeSpecDeps(), + }); + + await c.resolveType(mangaSpec); + + expect(notices[0]).not.toContain('picked'); + }); +}); diff --git a/tests/library-engine.test.ts b/tests/library-engine.test.ts index d089fe6..dd4f566 100644 --- a/tests/library-engine.test.ts +++ b/tests/library-engine.test.ts @@ -306,7 +306,7 @@ describe('libraryFolderSync: throttle-after-success', () => { describe('libraryFolderResolve', () => { test('missing id -> spec.resolve patches written via patchFrontmatter', async () => { - const { spec } = makeFakeSpec({ resolve: async () => ({ fake_id: '99' }) }); + const { spec } = makeFakeSpec({ resolve: async () => ({ patches: { fake_id: '99' } }) }); const { deps, writes } = makeDeps([{ path: 'A.md', content: NO_ID_NOTE }]); const report = await libraryFolderResolve(spec, deps, {}); expect(report.resolved).toEqual(['A.md']); @@ -326,7 +326,7 @@ describe('libraryFolderResolve', () => { }); test('hasId already true -> resolve() not called, note excluded from both lists', async () => { - const { spec, resolveCalls } = makeFakeSpec({ resolve: async () => ({ fake_id: '99' }) }); + const { spec, resolveCalls } = makeFakeSpec({ resolve: async () => ({ patches: { fake_id: '99' } }) }); const { deps } = makeDeps([{ path: 'A.md', content: ACTIVE_NOTE }]); const report = await libraryFolderResolve(spec, deps, {}); expect(resolveCalls.length).toBe(0); @@ -335,7 +335,7 @@ describe('libraryFolderResolve', () => { }); test('dryRun: resolved counted but no writeNote call', async () => { - const { spec } = makeFakeSpec({ resolve: async () => ({ fake_id: '99' }) }); + const { spec } = makeFakeSpec({ resolve: async () => ({ patches: { fake_id: '99' } }) }); const { deps, writes } = makeDeps([{ path: 'A.md', content: NO_ID_NOTE }]); const report = await libraryFolderResolve(spec, deps, { dryRun: true }); expect(report.resolved).toEqual(['A.md']); @@ -348,7 +348,7 @@ describe('libraryFolderResolve', () => { resolve: async () => { n++; if (n === 1) throw new Error('boom'); - return { fake_id: '99' }; + return { patches: { fake_id: '99' } }; }, }); const { deps } = makeDeps([ @@ -362,7 +362,7 @@ describe('libraryFolderResolve', () => { }); test('`_`-prefixed / non-entry notes excluded from resolve pass', async () => { - const { spec, resolveCalls } = makeFakeSpec({ resolve: async () => ({ fake_id: '99' }) }); + const { spec, resolveCalls } = makeFakeSpec({ resolve: async () => ({ patches: { fake_id: '99' } }) }); const note = `---\ntype: folder_index\n---\n\n# Index\n`; const { deps } = makeDeps([{ path: 'Index.md', content: note }]); const report = await libraryFolderResolve(spec, deps, {}); @@ -381,7 +381,7 @@ describe('libraryFolderResolve', () => { e.retryAfterMs = 1200; throw e; } - return { fake_id: '99' }; + return { patches: { fake_id: '99' } }; }, }); const { deps, slept } = makeDeps([{ path: 'A.md', content: NO_ID_NOTE }]); @@ -391,3 +391,52 @@ describe('libraryFolderResolve', () => { expect(report.resolved).toEqual(['A.md']); }); }); + +describe('libraryFolderResolve: needsChoice (candidates outcome)', () => { + const CANDIDATES = [ + { label: 'Foo (2020)', patches: { fake_id: '1' } }, + { label: 'Bar (2021)', patches: { fake_id: '2' } }, + ]; + + test('spec.resolve returns candidates -> collected into report.needsChoice, no write, not counted resolved/ambiguous', async () => { + const { spec } = makeFakeSpec({ resolve: async () => ({ candidates: CANDIDATES }) }); + const { deps, writes } = makeDeps([{ path: 'A.md', content: NO_ID_NOTE }]); + const report = await libraryFolderResolve(spec, deps, {}); + expect(report.needsChoice).toEqual([{ path: 'A.md', filename: 'A.md', candidates: CANDIDATES }]); + expect(report.resolved).toEqual([]); + expect(report.ambiguous).toEqual([]); + expect(writes.length).toBe(0); + }); + + test('dryRun -> candidates still collected (collect too, no write either way)', async () => { + const { spec } = makeFakeSpec({ resolve: async () => ({ candidates: CANDIDATES }) }); + const { deps, writes } = makeDeps([{ path: 'A.md', content: NO_ID_NOTE }]); + const report = await libraryFolderResolve(spec, deps, { dryRun: true }); + expect(report.needsChoice.length).toBe(1); + expect(writes.length).toBe(0); + }); +}); + +describe('libraryFolderResolve: no_resolve flag', () => { + test('no_resolve: true -> note skipped entirely, counted skippedNoResolve, resolve() never called', async () => { + const { spec, resolveCalls } = makeFakeSpec({ resolve: async () => ({ patches: { fake_id: '99' } }) }); + const note = NO_ID_NOTE.replace('fake_id: ', 'fake_id: \nno_resolve: true'); + const { deps, writes } = makeDeps([{ path: 'A.md', content: note }]); + const report = await libraryFolderResolve(spec, deps, {}); + expect(resolveCalls.length).toBe(0); + expect(report.skippedNoResolve).toBe(1); + expect(report.resolved).toEqual([]); + expect(report.ambiguous).toEqual([]); + expect(report.needsChoice).toEqual([]); + expect(writes.length).toBe(0); + }); + + test('no_resolve absent/false -> resolve() runs normally', async () => { + const { spec, resolveCalls } = makeFakeSpec({ resolve: async () => ({ patches: { fake_id: '99' } }) }); + const { deps } = makeDeps([{ path: 'A.md', content: NO_ID_NOTE }]); + const report = await libraryFolderResolve(spec, deps, {}); + expect(resolveCalls.length).toBe(1); + expect(report.skippedNoResolve).toBe(0); + expect(report.resolved).toEqual(['A.md']); + }); +}); diff --git a/tests/library-game.test.ts b/tests/library-game.test.ts index a37c60b..82925a7 100644 --- a/tests/library-game.test.ts +++ b/tests/library-game.test.ts @@ -297,7 +297,7 @@ describe('gameSpec.resolve', () => { }, }); const result = await gameSpec.resolve(ctxFor({ url: 'https://store.steampowered.com/app/792100/7_Billion_Humans/', title: '7 Billion Humans' }), deps); - expect(result).toEqual({ steam_appid: '792100' }); + expect(result).toEqual({ patches: { steam_appid: '792100' } }); expect(httpCalls).toBe(0); }); @@ -306,7 +306,7 @@ describe('gameSpec.resolve', () => { http: async () => ({ items: [{ id: 792100, name: '7 Billion Humans' }] }), }); const result = await gameSpec.resolve(ctxFor({ title: '7 Billion Humans' }), deps); - expect(result).toEqual({ steam_appid: '792100' }); + expect(result).toEqual({ patches: { steam_appid: '792100' } }); }); test('no exact match, sole steam result -> accepted', async () => { @@ -314,10 +314,10 @@ describe('gameSpec.resolve', () => { http: async () => ({ items: [{ id: 999, name: 'Some Other Game' }] }), }); const result = await gameSpec.resolve(ctxFor({ title: '7 Billion Humans' }), deps); - expect(result).toEqual({ steam_appid: '999' }); + expect(result).toEqual({ patches: { steam_appid: '999' } }); }); - test('steam storesearch ambiguous, no key -> null, logged', async () => { + test('steam storesearch ambiguous, no key -> steam candidates returned (top ≤6, label + full patches), RAWG skip still logged', async () => { const deps = makeDeps({ http: async () => ({ items: [ @@ -327,7 +327,12 @@ describe('gameSpec.resolve', () => { }), }); const result = await gameSpec.resolve(ctxFor({ title: '7 Billion Humans' }), deps); - expect(result).toBeNull(); + expect(result).toEqual({ + candidates: [ + { label: 'Foo', patches: { steam_appid: '1' } }, + { label: 'Bar', patches: { steam_appid: '2' } }, + ], + }); expect(deps.logCalls.some(m => m.includes('RAWG'))).toBe(true); }); test('steam storesearch ambiguous -> logs top candidates with appid + name', async () => { @@ -340,7 +345,7 @@ describe('gameSpec.resolve', () => { }), }); const result = await gameSpec.resolve(ctxFor({ title: '7 Billion Humans' }), deps); - expect(result).toBeNull(); + expect(result && 'candidates' in result).toBe(true); expect(deps.logCalls.some(m => m.includes('ambiguous') && m.includes('appid=1') && m.includes('Foo') && m.includes('appid=2') && m.includes('Bar'))).toBe(true); }); @@ -353,7 +358,7 @@ describe('gameSpec.resolve', () => { getKey: () => 'rawgkey', }); const result = await gameSpec.resolve(ctxFor({ title: '7 Billion Humans' }), deps); - expect(result).toEqual({ rawg_id: '4200' }); + expect(result).toEqual({ patches: { rawg_id: '4200' } }); expect(deps.logCalls.some(m => m.includes('storesearch'))).toBe(true); }); @@ -363,7 +368,7 @@ describe('gameSpec.resolve', () => { getKey: () => 'rawgkey', }); const result = await gameSpec.resolve(ctxFor({ title: '7 Billion Humans' }), deps); - expect(result).toEqual({ rawg_id: '4200' }); + expect(result).toEqual({ patches: { rawg_id: '4200' } }); }); test('no steam results, RAWG key missing -> null, logged, no RAWG call attempted', async () => { diff --git a/tests/library-manga.test.ts b/tests/library-manga.test.ts index c0de073..46e4820 100644 --- a/tests/library-manga.test.ts +++ b/tests/library-manga.test.ts @@ -571,14 +571,14 @@ describe('mangaSpec.resolve', () => { http: async () => ({ data: [{ mal_id: 116778, title: 'Chainsaw Man', title_english: 'Chainsaw Man' }] }), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); }); test('no exact match, sole result -> accepted', async () => { const deps = makeDeps({ http: async () => ({ data: [{ mal_id: 999, title: 'Some Other Title', title_english: '' }] }), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '999' }); + expect(result).toEqual({ patches: { mal_id: '999' } }); }); test('ambiguous (multiple results, no exact match) -> null', async () => { const deps = makeDeps({ @@ -633,7 +633,7 @@ describe('mangaSpec.resolve — best-effort MangaDex id resolve (I4)', () => { }, }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778', mangadex_id: 'a1b2c3d4-uuid' }); + expect(result).toEqual({ patches: { mal_id: '116778', mangadex_id: 'a1b2c3d4-uuid' } }); }); test('mangadex search no exact match, sole result -> accepted (unique-exact fallback rule)', async () => { @@ -644,7 +644,7 @@ describe('mangaSpec.resolve — best-effort MangaDex id resolve (I4)', () => { }, }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778', mangadex_id: 'uuid-solo' }); + expect(result).toEqual({ patches: { mal_id: '116778', mangadex_id: 'uuid-solo' } }); }); test('mangadex search ambiguous (multiple results, no exact match) -> mal_id patched only', async () => { @@ -661,7 +661,7 @@ describe('mangaSpec.resolve — best-effort MangaDex id resolve (I4)', () => { }, }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); }); test('mangadex search throws -> log, mal_id patched only (best-effort, no overall failure)', async () => { @@ -672,7 +672,7 @@ describe('mangaSpec.resolve — best-effort MangaDex id resolve (I4)', () => { }, }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); expect(deps.logCalls.some(m => m.toLowerCase().includes('mangadex'))).toBe(true); }); @@ -685,7 +685,7 @@ describe('mangaSpec.resolve — best-effort MangaDex id resolve (I4)', () => { }, }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man', mangadex_id: 'existing-uuid' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); expect(mangadexCalled).toBe(false); }); }); @@ -817,7 +817,7 @@ describe('mangaSpec.resolve — AniList primary', () => { httpPostJson: async () => anilistPage([{ id: 105778, idMal: 116778, title: { romaji: 'Chainsaw Man', english: 'Chainsaw Man' } }]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778', anilist_id: '105778' }); + expect(result).toEqual({ patches: { mal_id: '116778', anilist_id: '105778' } }); }); test('unique exact match via english title only (case-insensitive) -> accepted', async () => { @@ -826,7 +826,7 @@ describe('mangaSpec.resolve — AniList primary', () => { httpPostJson: async () => anilistPage([{ id: 105778, idMal: 116778, title: { romaji: 'チェンソーマン', english: 'Chainsaw Man' } }]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778', anilist_id: '105778' }); + expect(result).toEqual({ patches: { mal_id: '116778', anilist_id: '105778' } }); }); test('no exact match, sole AniList result -> accepted (unique-exact fallback rule)', async () => { @@ -835,7 +835,7 @@ describe('mangaSpec.resolve — AniList primary', () => { httpPostJson: async () => anilistPage([{ id: 999, idMal: 888, title: { romaji: 'Some Other Title', english: '' } }]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '888', anilist_id: '999' }); + expect(result).toEqual({ patches: { mal_id: '888', anilist_id: '999' } }); }); test('AniList hit, idMal null (no MAL bridge) -> patch has anilist_id only', async () => { @@ -844,7 +844,7 @@ describe('mangaSpec.resolve — AniList primary', () => { httpPostJson: async () => anilistPage([{ id: 105778, idMal: null, title: { romaji: 'Chainsaw Man', english: 'Chainsaw Man' } }]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ anilist_id: '105778' }); + expect(result).toEqual({ patches: { anilist_id: '105778' } }); }); test('AniList ambiguous (multiple, no exact) -> falls back to Jikan search, logs anilist candidates', async () => { @@ -857,17 +857,35 @@ describe('mangaSpec.resolve — AniList primary', () => { ]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); expect(deps.logCalls.some(m => m.includes('ambiguous') && m.includes('anilist_id=1') && m.includes('Foo') && m.includes('anilist_id=2') && m.includes('Bar'))).toBe(true); }); + test('AniList ambiguous AND Jikan also fails to land a unique match -> AniList candidates returned (top 6, label + full patches)', async () => { + const deps = makeDeps({ + http: async () => ({ data: [] }), // jikan: no results either -> resolveMalId returns null + httpPostJson: async () => + anilistPage([ + { id: 1, idMal: 11, title: { romaji: 'Foo', english: '' }, startDate: { year: 2020 } }, + { id: 2, idMal: 22, title: { romaji: 'Bar', english: '' }, startDate: { year: 2021 } }, + ]), + }); + const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); + expect(result).toEqual({ + candidates: [ + { label: 'Foo (2020)', patches: { anilist_id: '1', mal_id: '11' } }, + { label: 'Bar (2021)', patches: { anilist_id: '2', mal_id: '22' } }, + ], + }); + }); + test('AniList miss (empty results) -> falls straight to Jikan, no ambiguous log from AniList side', async () => { const deps = makeDeps({ http: async () => ({ data: [{ mal_id: 116778, title: 'Chainsaw Man', title_english: 'Chainsaw Man' }] }), httpPostJson: async () => anilistPage([]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); expect(deps.logCalls.some(m => m.includes('anilist_id='))).toBe(false); }); @@ -879,7 +897,7 @@ describe('mangaSpec.resolve — AniList primary', () => { }, }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778' }); + expect(result).toEqual({ patches: { mal_id: '116778' } }); expect(deps.logCalls.some(m => m.toLowerCase().includes('anilist'))).toBe(true); }); @@ -904,7 +922,7 @@ describe('mangaSpec.resolve — AniList primary', () => { httpPostJson: async () => anilistPage([{ id: 105778, idMal: 116778, title: { romaji: 'Chainsaw Man', english: 'Chainsaw Man' } }]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man' }, ''), deps); - expect(result).toEqual({ mal_id: '116778', anilist_id: '105778', mangadex_id: 'a1b2c3d4-uuid' }); + expect(result).toEqual({ patches: { mal_id: '116778', anilist_id: '105778', mangadex_id: 'a1b2c3d4-uuid' } }); }); test('mangadex_id already present -> mangadex search skipped, even on an AniList hit', async () => { @@ -917,7 +935,7 @@ describe('mangaSpec.resolve — AniList primary', () => { httpPostJson: async () => anilistPage([{ id: 105778, idMal: 116778, title: { romaji: 'Chainsaw Man', english: 'Chainsaw Man' } }]), }); const result = await mangaSpec.resolve(ctxFor({ title: 'Chainsaw Man', mangadex_id: 'existing-uuid' }, ''), deps); - expect(result).toEqual({ mal_id: '116778', anilist_id: '105778' }); + expect(result).toEqual({ patches: { mal_id: '116778', anilist_id: '105778' } }); expect(mangadexCalled).toBe(false); }); diff --git a/tests/setup.ts b/tests/setup.ts index 6871828..f0bef47 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -36,6 +36,26 @@ function stringifySimpleYaml(value: unknown): string { .concat('\n'); } +class MockModal { + app: unknown; + titleEl: { setText: (text: string) => void } = { setText: (): void => {} }; + contentEl: { empty: () => void } = { empty: (): void => {} }; + + constructor(app: unknown) { + this.app = app; + } + + setTitle(_title: string): this { + return this; + } + + open(): void {} + close(): void { + this.onClose(); + } + onClose(): void {} +} + mock.module('obsidian', () => ({ AbstractInputSuggest: class {}, Component: class { @@ -43,18 +63,12 @@ mock.module('obsidian', () => ({ unload(): void {} }, DropdownComponent: class {}, + FuzzySuggestModal: class extends MockModal { + setPlaceholder(_text: string): void {} + }, MarkdownRenderer: { render: async (): Promise => {} }, MarkdownView: class {}, - Modal: class { - app: unknown; - - constructor(app: unknown) { - this.app = app; - } - - open(): void {} - close(): void {} - }, + Modal: MockModal, Notice: class {}, normalizePath: (path: string): string => path, moment: Object.assign((value?: unknown): unknown => value, { locale: (): void => {} }),