diff --git a/.ai/LESSONS.txt b/.ai/LESSONS.txt index cb909d3..0409958 100644 --- a/.ai/LESSONS.txt +++ b/.ai/LESSONS.txt @@ -114,6 +114,14 @@ Invariant Rule: Run `npm run validate` before finishing and remove all unused im **Root Cause:** Smart playlist queries were stored as raw JSON without schema versioning, so parser changes could break old records or misinterpret fields. **Invariant Rule:** Any JSON-in-column stored as user data MUST include a version column and be parsed only through a versioned resolver, never through raw JSON.parse. +### Problem: Negative cache died with the process while rate-limit failures risked being cached as absent +**Root Cause:** Tag resolve outcomes collapsed to found/miss; confirmed empty API responses lived only in an in-memory Map (TTL), and unresolved failures (429/network) could be confused with not_found if classification stayed binary. +**Invariant Rule:** Cacheable outcomes of external lookups distinguish `not_found` (persist with TTL in the same table as hits) from `unresolved` (do not persist as absence). Never mix the two — mixing permanently corrupts categorization. One source of truth for negative state (SQLite), no split-brain RAM Map alongside it. + +### Problem: Raw-SQL maintenance deleted every fresh not_found row within seconds of startup +**Root Cause:** Drizzle `mode: "timestamp"` stores seconds while maintenance DELETE compared `resolved_at` to a `Date.now()`-based millisecond cutoff — seconds are always below that cutoff, so the first tick wiped the entire negative cache. +**Invariant Rule:** Timestamp columns touched by raw SQL use `mode: "timestamp_ms"` with an explicit unit comment in the schema. Any raw-SQL time comparison MUST have a test that writes via the ORM path and asserts the row survives/expires against the production DELETE helper — never a hand-copied SQL string alone. + ## Security (credentials) ### Problem: API decrypt failure sends ciphertext to Booru APIs diff --git a/docs/api-guide.md b/docs/api-guide.md index 1e2b97c..4ae3a72 100644 --- a/docs/api-guide.md +++ b/docs/api-guide.md @@ -1283,8 +1283,9 @@ const artistTags = await window.api.resolveTags(["tag1", "tag2", "tag3"]); - Missing tags are fetched one at a time via `name=` (not batched `names=`). - All tag lookups share the same `ProviderThrottle` instance as `fetchPosts`. - Concurrent IPC calls for the same tag share one in-flight promise (cross-call dedup). -- HTTP 429 is retried with `Retry-After` or exponential backoff; rate-limited tags are **not** written to `tag_metadata`. -- Confirmed empty API responses are negatively cached in memory (24h TTL) to avoid hammering nonexistent tags. +- HTTP 429 is retried with `Retry-After` or exponential backoff; rate-limited / network failures are **unresolved** — not written to `tag_metadata` (must not be confused with `not_found`). +- Confirmed empty API responses (`not_found`) are persisted in `tag_metadata` with `status='not_found'` and `resolved_at` in **milliseconds** (`mode: "timestamp_ms"`, TTL `TAG_RESOLVE_NOT_FOUND_TTL_MS`, 7 days). Expired rows are cache-misses; maintenance DELETEs them via `deleteExpiredNotFoundTagMetadata`. + --- diff --git a/docs/architecture.md b/docs/architecture.md index 7d1a8e9..9084587 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1614,7 +1614,7 @@ src/ │ │ ├── maintenance-scheduler.ts # Daily checkpoint/optimize scheduler │ │ ├── MaintenanceService.ts # User-triggered VACUUM status/run logic │ │ ├── updater-service.ts # Auto-updater service -│ │ ├── tag-resolve-coordinator.ts # Tag metadata resolve dedup / rate limit +│ │ ├── tag-resolve-coordinator.ts # Tag metadata resolve: found/not_found persist, unresolved not cached │ │ └── video-proxy-server.ts # Local video proxy + disk cache │ ├── workers/ # Worker threads │ │ ├── downloadWorker.ts # Batch download worker @@ -1738,7 +1738,7 @@ Root: **Database & Schema:** -- **Schema:** Core tables `artists`, `posts`, `settings`; also `tag_metadata`, `playlists`, `playlist_entries`, and FTS5 for post tags +- **Schema:** Core tables `artists`, `posts`, `settings`; also `tag_metadata` (`status` found|not_found + `resolved_at` TTL for misses), `playlists`, `playlist_entries`, and FTS5 for post tags - **Migrations:** Fully functional migration system using `drizzle-kit` 0.30+ (`drizzle.config.ts`, `npm run db:generate` / `db:migrate`) - **Testing & CI:** Vitest (unit, integration, property), Playwright (E2E); CI runs `validate`, `npm test`, and production `npm audit` - **Indexes:** Optimized indexes on `artistId`, `isViewed`, `publishedAt`, `isFavorited`, `lastChecked`, `createdAt` diff --git a/docs/database.md b/docs/database.md index c9d064b..dbce65e 100644 --- a/docs/database.md +++ b/docs/database.md @@ -271,6 +271,28 @@ export type Settings = typeof settings.$inferSelect; export type NewSettings = typeof settings.$inferInsert; ``` +### Table: `tag_metadata` + +Persistent cache for Rule34 tag type resolution (viewer TagsDrawer / resolve IPC). One row per tag name. + +| Column | Type | Description | +| -------------- | --------------------------------- | --------------------------------------------------------------------------- | +| `name` | TEXT (PK, NOT NULL) | Tag string (lowercase provider form) | +| `type` | INTEGER (NOT NULL) | Rule34 tag type for `found` rows; placeholder (`0`) when `status=not_found` | +| `status` | TEXT (NOT NULL, DEFAULT `found`) | `found` or `not_found` — never store unresolved (429/network) as `not_found` | +| `resolved_at` | INTEGER (TIMESTAMP_MS, NOT NULL) | When this outcome was written (**milliseconds**); TTL gate for `not_found` | + + +**Semantics:** + +- `found` — API returned the tag; used for categorization. +- `not_found` — API answered successfully with empty/miss; TTL `TAG_RESOLVE_NOT_FOUND_TTL_MS` (7 days). Expired rows are treated as cache-miss (re-resolve); maintenance DELETEs them. +- Unresolved failures are **not** written — they remain candidates for the next session. + +**Indexes:** + +- `tag_metadata_type_idx` — Index on `type` for filtering (e.g. all artists) + ### Table: `playlists` Stores playlist/collection information for curated post collections. diff --git a/drizzle/0031_tag_metadata_status.sql b/drizzle/0031_tag_metadata_status.sql new file mode 100644 index 0000000..365f3c3 --- /dev/null +++ b/drizzle/0031_tag_metadata_status.sql @@ -0,0 +1,3 @@ +ALTER TABLE `tag_metadata` ADD `status` text DEFAULT 'found' NOT NULL;--> statement-breakpoint +ALTER TABLE `tag_metadata` ADD `resolved_at` integer DEFAULT 0 NOT NULL;--> statement-breakpoint +UPDATE `tag_metadata` SET `resolved_at` = CAST((strftime('%s', 'now') * 1000) AS INTEGER) WHERE `resolved_at` = 0; diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index 5b78e68..48e1f8d 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -211,6 +211,13 @@ "when": 1777800000000, "tag": "0030_add_artists_last_sync_incomplete", "breakpoints": true + }, + { + "idx": 31, + "version": "6", + "when": 1777900000000, + "tag": "0031_tag_metadata_status", + "breakpoints": true } ] } \ No newline at end of file diff --git a/src/main/config/tag-resolve-constants.ts b/src/main/config/tag-resolve-constants.ts index 5190afe..08d380b 100644 --- a/src/main/config/tag-resolve-constants.ts +++ b/src/main/config/tag-resolve-constants.ts @@ -1,5 +1,8 @@ -/** In-memory TTL for tags confirmed absent from Rule34 tag DAPI (well-formed empty). */ -export const TAG_RESOLVE_NEGATIVE_CACHE_TTL_MS = 24 * 60 * 60 * 1000; +/** + * SQLite TTL for confirmed not_found tag_metadata rows. + * Expired rows are treated as cache-misses (re-resolve); maintenance DELETEs them. + */ +export const TAG_RESOLVE_NOT_FOUND_TTL_MS = 7 * 24 * 60 * 60 * 1000; /** HTTP timeout for a single tag metadata lookup. */ export const TAG_RESOLVE_REQUEST_TIMEOUT_MS = 10_000; diff --git a/src/main/db/queries/tag-metadata.ts b/src/main/db/queries/tag-metadata.ts new file mode 100644 index 0000000..b98db74 --- /dev/null +++ b/src/main/db/queries/tag-metadata.ts @@ -0,0 +1,25 @@ +import type Database from "better-sqlite3"; +import { TAG_RESOLVE_NOT_FOUND_TTL_MS } from "../../config/tag-resolve-constants"; + +type SqliteDatabase = InstanceType; + +/** + * Delete expired not_found rows from tag_metadata. + * `resolved_at` is stored in milliseconds (schema mode timestamp_ms). + * Cutoff uses Date.now()-based ms — must stay aligned with Drizzle writes. + * + * @returns number of deleted rows + */ +export function deleteExpiredNotFoundTagMetadata( + sqlite: SqliteDatabase, + nowMs: number = Date.now() +): number { + const cutoffMs = nowMs - TAG_RESOLVE_NOT_FOUND_TTL_MS; + const result = sqlite + .prepare( + `DELETE FROM tag_metadata + WHERE status = 'not_found' AND resolved_at < ?` + ) + .run(cutoffMs); + return result.changes; +} diff --git a/src/main/db/schema.ts b/src/main/db/schema.ts index ebabbf9..0643002 100644 --- a/src/main/db/schema.ts +++ b/src/main/db/schema.ts @@ -28,6 +28,10 @@ export const TAG_TYPES = { export type TagType = (typeof TAG_TYPES)[keyof typeof TAG_TYPES]; +/** Cache outcome for tag_metadata rows (unresolved API failures are never stored). */ +export const TAG_METADATA_STATUSES = ["found", "not_found"] as const; +export type TagMetadataStatus = (typeof TAG_METADATA_STATUSES)[number]; + // Settings ID constant for single profile design export const SETTINGS_ID = 1; @@ -156,6 +160,13 @@ export const tagMetadata = sqliteTable( { name: text("name").primaryKey(), type: integer("type").notNull(), // Use TAG_TYPES constants: 0=General, 1=Artist, 3=Copyright, 4=Character, 5=Meta + status: text("status", { enum: TAG_METADATA_STATUSES }) + .notNull() + .default("found"), + // Units: milliseconds since epoch. Raw SQL (maintenance DELETE) compares with Date.now()-based cutoffs. + resolvedAt: integer("resolved_at", { mode: "timestamp_ms" }) + .notNull() + .$defaultFn(() => new Date()), }, (t) => ({ typeIdx: index("tag_metadata_type_idx").on(t.type), // Index for filtering by type (e.g., all artists) diff --git a/src/main/ipc/controllers/SearchController.ts b/src/main/ipc/controllers/SearchController.ts index 82cc469..d9c647a 100644 --- a/src/main/ipc/controllers/SearchController.ts +++ b/src/main/ipc/controllers/SearchController.ts @@ -3,7 +3,11 @@ import log from "electron-log"; import { z } from "zod"; import { BaseController } from "../../core/ipc/BaseController"; import { container, DI_TOKENS } from "../../core/di/Container"; -import { settings, SETTINGS_ID, posts, tagMetadata, TAG_TYPES, type TagType } from "../../db/schema"; +import { settings, SETTINGS_ID, posts, TAG_TYPES, type TagType } from "../../db/schema"; +import { + loadTagMetadataCache, + resolveTagMetadataWave, +} from "../../services/tag-resolve-coordinator"; import { eq, inArray, and } from "drizzle-orm"; import { getProvider } from "../../providers"; import { IPC_CHANNELS } from "../channels"; @@ -26,7 +30,6 @@ import { } from "../../../shared/utils/provider-tag-sanitize"; import { getAllBlacklistedTags } from "../../db/queries/blacklist"; import { getDecryptedCredentialsFromRecord } from "../../utils/decrypted-credentials"; -import { resolveTagMetadataWave } from "../../services/tag-resolve-coordinator"; import type { ProviderSettings } from "../../providers/types"; import { isProviderSearchError, @@ -171,19 +174,6 @@ export class SearchController extends BaseController { ]; } - private loadCachedTagMap( - db: AppDatabase, - uniqueTags: string[] - ): Map { - const cachedTags = db - .select() - .from(tagMetadata) - .where(inArray(tagMetadata.name, uniqueTags)) - .all(); - - return new Map(cachedTags.map((t) => [t.name, t.type])); - } - private toProviderSettings( settings: NonNullable>> ): ProviderSettings { @@ -209,25 +199,25 @@ export class SearchController extends BaseController { } const db = this.getDb(); - const cachedMap = this.loadCachedTagMap(db, uniqueTags); + const cache = loadTagMetadataCache(db, uniqueTags); const settings = await this.getDecryptedSettings(); if (!settings) { log.warn( `[SearchController] Cannot resolve tags (${context}): no settings available` ); - return uniqueTags.filter((tag) => cachedMap.get(tag) === tagType); + return uniqueTags.filter((tag) => cache.foundTypes.get(tag) === tagType); } await resolveTagMetadataWave( db, uniqueTags, - cachedMap, + cache, this.toProviderSettings(settings), context ); - return uniqueTags.filter((tag) => cachedMap.get(tag) === tagType); + return uniqueTags.filter((tag) => cache.foundTypes.get(tag) === tagType); } catch (error) { log.error(`[SearchController] Failed to resolve tags (${context}):`, error); return []; diff --git a/src/main/services/maintenance-scheduler.ts b/src/main/services/maintenance-scheduler.ts index c02709c..d799183 100644 --- a/src/main/services/maintenance-scheduler.ts +++ b/src/main/services/maintenance-scheduler.ts @@ -1,5 +1,6 @@ import log from "electron-log"; import { getSqliteInstance } from "../db/client"; +import { deleteExpiredNotFoundTagMetadata } from "../db/queries/tag-metadata"; import type { VideoProxyServer } from "./video-proxy-server"; const STARTUP_DELAY_MS = 10_000; @@ -43,6 +44,14 @@ export class MaintenanceScheduler { // PRAGMA/VACUUM: no Drizzle equivalent, raw SQL required sqlite.exec("PRAGMA wal_checkpoint(PASSIVE);"); sqlite.exec("PRAGMA optimize;"); + + const deletedExpiredNotFound = deleteExpiredNotFoundTagMetadata(sqlite); + if (deletedExpiredNotFound > 0) { + log.info( + `[MaintenanceScheduler] Deleted ${deletedExpiredNotFound} expired not_found tag_metadata rows` + ); + } + log.info(`[MaintenanceScheduler] Maintenance complete (trigger=${trigger})`); } catch (error) { log.error("[MaintenanceScheduler] Maintenance failed:", error); diff --git a/src/main/services/tag-resolve-coordinator.ts b/src/main/services/tag-resolve-coordinator.ts index 3a0fd99..4a5a199 100644 --- a/src/main/services/tag-resolve-coordinator.ts +++ b/src/main/services/tag-resolve-coordinator.ts @@ -1,8 +1,8 @@ import log from "electron-log"; -import { sql } from "drizzle-orm"; +import { inArray, sql } from "drizzle-orm"; import type { BetterSQLite3Database } from "drizzle-orm/better-sqlite3"; import type * as schema from "../db/schema"; -import { tagMetadata } from "../db/schema"; +import { TAG_TYPES, tagMetadata } from "../db/schema"; import { getProvider } from "../providers"; import type { IBooruProvider, ProviderSettings } from "../providers/types"; import type { ProviderThrottle } from "../providers/provider-throttle"; @@ -17,7 +17,7 @@ import { TAG_RESOLVE_DEFAULT_RETRY_AFTER_MS, TAG_RESOLVE_MAX_RATE_LIMIT_RETRIES, TAG_RESOLVE_MAX_RETRY_AFTER_MS, - TAG_RESOLVE_NEGATIVE_CACHE_TTL_MS, + TAG_RESOLVE_NOT_FOUND_TTL_MS, } from "../config/tag-resolve-constants"; type AppDatabase = BetterSQLite3Database; @@ -32,10 +32,19 @@ type TagResolveWaveStats = { rateLimitedCount: number; }; +/** Session view of tag_metadata for one resolve wave (single SQLite source of truth). */ +export type TagMetadataCacheState = { + foundTypes: Map; + /** Confirmed not_found within TTL — skip API; drawer leaves tag uncategorized. */ + activeNotFound: Set; +}; + const inFlightLookups = new Map>(); -const negativeCacheUntil = new Map(); let last429BurstLogAtMs = 0; +/** Placeholder type for not_found rows; status is the authority. */ +const NOT_FOUND_TYPE_PLACEHOLDER = TAG_TYPES.GENERAL; + function sleep(ms: number): Promise { return new Promise((resolve) => { setTimeout(resolve, ms); @@ -91,23 +100,43 @@ function recordRateLimitBurst(retryAfterMs: number, attempt: number): void { } } -function isNegativeCacheActive(tagName: string): boolean { - const until = negativeCacheUntil.get(tagName); - if (until === undefined) { - return false; +function isActiveNotFound(resolvedAt: Date, nowMs: number): boolean { + return nowMs - resolvedAt.getTime() < TAG_RESOLVE_NOT_FOUND_TTL_MS; +} + +/** + * Single read of tag_metadata for the requested names. + * found → foundTypes; not_found within TTL → activeNotFound; expired not_found → miss. + */ +export function loadTagMetadataCache( + db: AppDatabase, + uniqueTags: string[], + nowMs: number = Date.now() +): TagMetadataCacheState { + const foundTypes = new Map(); + const activeNotFound = new Set(); + + if (uniqueTags.length === 0) { + return { foundTypes, activeNotFound }; } - if (until <= Date.now()) { - negativeCacheUntil.delete(tagName); - return false; + + const rows = db + .select() + .from(tagMetadata) + .where(inArray(tagMetadata.name, uniqueTags)) + .all(); + + for (const row of rows) { + if (row.status === "found") { + foundTypes.set(row.name, row.type); + continue; + } + if (row.status === "not_found" && isActiveNotFound(row.resolvedAt, nowMs)) { + activeNotFound.add(row.name); + } } - return true; -} -function rememberNegativeCache(tagName: string): void { - negativeCacheUntil.set( - tagName, - Date.now() + TAG_RESOLVE_NEGATIVE_CACHE_TTL_MS - ); + return { foundTypes, activeNotFound }; } async function lookupTagFromApi( @@ -149,10 +178,6 @@ async function lookupTagCoordinated( settings: ProviderSettings, stats: TagResolveWaveStats ): Promise { - if (isNegativeCacheActive(tagName)) { - return { status: "not_found" }; - } - const existing = inFlightLookups.get(tagName); if (existing) { stats.inFlightHits += 1; @@ -162,11 +187,7 @@ async function lookupTagCoordinated( const lookupPromise = (async (): Promise => { stats.apiCalls += 1; try { - const result = await lookupTagFromApi(tagName, settings); - if (result.status === "not_found") { - rememberNegativeCache(tagName); - } - return result; + return await lookupTagFromApi(tagName, settings); } catch (error) { if (error instanceof Rule34TagRateLimitError) { stats.rateLimitedCount += 1; @@ -184,34 +205,82 @@ async function lookupTagCoordinated( } } -function upsertTagMetadataEntries( +function upsertFoundEntries( db: AppDatabase, entries: Rule34TagMetadataEntry[], - cachedMap: Map + foundTypes: Map ): void { if (entries.length === 0) { return; } + const resolvedAt = new Date(); + const values = entries.map((entry) => ({ + name: entry.name, + type: entry.type, + status: "found" as const, + resolvedAt, + })); + db.transaction((tx) => { tx.insert(tagMetadata) - .values(entries) + .values(values) .onConflictDoUpdate({ target: tagMetadata.name, - set: { type: sql`excluded.type` }, + set: { + type: sql`excluded.type`, + status: sql`excluded.status`, + resolvedAt: sql`excluded.resolved_at`, + }, }) .run(); }); for (const entry of entries) { - cachedMap.set(entry.name, entry.type); + foundTypes.set(entry.name, entry.type); + } +} + +function upsertNotFoundEntries( + db: AppDatabase, + tagNames: string[], + activeNotFound: Set +): void { + if (tagNames.length === 0) { + return; + } + + const resolvedAt = new Date(); + const values = tagNames.map((name) => ({ + name, + type: NOT_FOUND_TYPE_PLACEHOLDER, + status: "not_found" as const, + resolvedAt, + })); + + db.transaction((tx) => { + tx.insert(tagMetadata) + .values(values) + .onConflictDoUpdate({ + target: tagMetadata.name, + set: { + type: sql`excluded.type`, + status: sql`excluded.status`, + resolvedAt: sql`excluded.resolved_at`, + }, + }) + .run(); + }); + + for (const name of tagNames) { + activeNotFound.add(name); } } export async function resolveTagMetadataWave( db: AppDatabase, uniqueTags: string[], - cachedMap: Map, + cache: TagMetadataCacheState, settings: ProviderSettings, context: string ): Promise { @@ -223,14 +292,13 @@ export async function resolveTagMetadataWave( rateLimitedCount: 0, }; + const { foundTypes, activeNotFound } = cache; + const missingTags = uniqueTags.filter((tag) => { - if (cachedMap.has(tag)) { + if (foundTypes.has(tag) || activeNotFound.has(tag)) { stats.tagMetadataHits += 1; return false; } - if (isNegativeCacheActive(tag)) { - return false; - } return true; }); @@ -265,8 +333,21 @@ export async function resolveTagMetadataWave( const resolvedEntries = lookupResults.flatMap((item) => item.result?.status === "found" ? [item.result.entry] : [] ); + const notFoundNames = lookupResults.flatMap((item) => + item.result?.status === "not_found" ? [item.tagName] : [] + ); + const unresolvedNames = lookupResults.flatMap((item) => + item.result === null ? [item.tagName] : [] + ); + + upsertFoundEntries(db, resolvedEntries, foundTypes); + upsertNotFoundEntries(db, notFoundNames, activeNotFound); - upsertTagMetadataEntries(db, resolvedEntries, cachedMap); + for (const tagName of unresolvedNames) { + log.debug( + `[TagResolve] ${context}: unresolved "${tagName}" (not persisted as not_found)` + ); + } log.info( `[TagResolve] ${context}: requested=${stats.requested} tag_metadata=${stats.tagMetadataHits} in_flight=${stats.inFlightHits} api_calls=${stats.apiCalls} rate_limited=${stats.rateLimitedCount}` @@ -278,7 +359,6 @@ export async function resolveTagMetadataWave( /** Test-only reset of module-level coordination state. */ export function resetTagResolveCoordinatorForTests(): void { inFlightLookups.clear(); - negativeCacheUntil.clear(); last429BurstLogAtMs = 0; try { getRule34TagProvider().getRequestThrottle().resetRateLimitGateForTests(); diff --git a/tests/unit/services/tag-resolve-coordinator.test.ts b/tests/unit/services/tag-resolve-coordinator.test.ts index 2f7b96c..039e4fc 100644 --- a/tests/unit/services/tag-resolve-coordinator.test.ts +++ b/tests/unit/services/tag-resolve-coordinator.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createMockDb } from "../../helpers/mock-db"; import { tagMetadata, TAG_TYPES } from "@/main/db/schema"; +import { TAG_RESOLVE_NOT_FOUND_TTL_MS } from "@/main/config/tag-resolve-constants"; const { fetchRule34TagMetadataMock, MockRule34TagRateLimitError } = vi.hoisted( () => { @@ -26,6 +27,7 @@ vi.mock("electron-log", () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), + debug: vi.fn(), }, })); @@ -51,9 +53,11 @@ vi.mock("@/main/providers/rule34-tag-metadata", () => ({ })); import { + loadTagMetadataCache, resetTagResolveCoordinatorForTests, resolveTagMetadataWave, } from "@/main/services/tag-resolve-coordinator"; +import { deleteExpiredNotFoundTagMetadata } from "@/main/db/queries/tag-metadata"; describe("tag-resolve-coordinator", () => { let mockDb: ReturnType; @@ -72,6 +76,63 @@ describe("tag-resolve-coordinator", () => { } }); + it("does not persist 429 give-up as not_found (unresolved ≠ miss)", async () => { + fetchRule34TagMetadataMock.mockRejectedValue( + new MockRule34TagRateLimitError(1000) + ); + + const cache = loadTagMetadataCache(mockDb.db, ["colored_skin"]); + await resolveTagMetadataWave( + mockDb.db, + ["colored_skin"], + cache, + { userId: "1", apiKey: "key" }, + "test-429-not-not-found" + ); + + const rows = mockDb.db.select().from(tagMetadata).all(); + expect(rows).toHaveLength(0); + expect(cache.foundTypes.has("colored_skin")).toBe(false); + expect(cache.activeNotFound.has("colored_skin")).toBe(false); + + resetTagResolveCoordinatorForTests(); + fetchRule34TagMetadataMock.mockReset(); + fetchRule34TagMetadataMock.mockResolvedValue({ + status: "found", + entry: { name: "colored_skin", type: TAG_TYPES.GENERAL }, + }); + + const cacheAfterRestart = loadTagMetadataCache(mockDb.db, ["colored_skin"]); + await resolveTagMetadataWave( + mockDb.db, + ["colored_skin"], + cacheAfterRestart, + { userId: "1", apiKey: "key" }, + "test-429-retry-next-session" + ); + + expect(fetchRule34TagMetadataMock).toHaveBeenCalledTimes(1); + expect(cacheAfterRestart.foundTypes.get("colored_skin")).toBe( + TAG_TYPES.GENERAL + ); + }); + + it("does not persist network failures as not_found", async () => { + fetchRule34TagMetadataMock.mockRejectedValue(new Error("socket hang up")); + + const cache = loadTagMetadataCache(mockDb.db, ["flaky_tag"]); + await resolveTagMetadataWave( + mockDb.db, + ["flaky_tag"], + cache, + { userId: "1", apiKey: "key" }, + "test-network-unresolved" + ); + + expect(mockDb.db.select().from(tagMetadata).all()).toHaveLength(0); + expect(cache.activeNotFound.has("flaky_tag")).toBe(false); + }); + it("deduplicates concurrent lookups for the same tag", async () => { let resolveFetch: (() => void) | undefined; const fetchGate = new Promise((resolve) => { @@ -87,19 +148,19 @@ describe("tag-resolve-coordinator", () => { }); const settings = { userId: "1", apiKey: "key" }; - const cachedMap = new Map(); + const cache = loadTagMetadataCache(mockDb.db, ["artist_one"]); const firstWave = resolveTagMetadataWave( mockDb.db, ["artist_one"], - cachedMap, + cache, settings, "test-dedup-1" ); const secondWave = resolveTagMetadataWave( mockDb.db, ["artist_one"], - cachedMap, + cache, settings, "test-dedup-2" ); @@ -108,49 +169,139 @@ describe("tag-resolve-coordinator", () => { await Promise.all([firstWave, secondWave]); expect(fetchRule34TagMetadataMock).toHaveBeenCalledTimes(1); - expect(cachedMap.get("artist_one")).toBe(TAG_TYPES.ARTIST); + expect(cache.foundTypes.get("artist_one")).toBe(TAG_TYPES.ARTIST); }); - it("does not write tag_metadata when rate limited", async () => { - fetchRule34TagMetadataMock.mockRejectedValue( - new MockRule34TagRateLimitError(1000) - ); + it("persists confirmed not_found in SQLite and survives coordinator reset", async () => { + fetchRule34TagMetadataMock.mockResolvedValue({ status: "not_found" }); + + const settings = { userId: "1", apiKey: "key" }; + const firstCache = loadTagMetadataCache(mockDb.db, ["ghost_tag"]); - const cachedMap = new Map(); await resolveTagMetadataWave( mockDb.db, - ["missing_tag"], - cachedMap, - { userId: "1", apiKey: "key" }, - "test-429" + ["ghost_tag"], + firstCache, + settings, + "test-not-found-persist-1" ); const rows = mockDb.db.select().from(tagMetadata).all(); - expect(rows).toHaveLength(0); - expect(cachedMap.has("missing_tag")).toBe(false); - }); + expect(rows).toHaveLength(1); + expect(rows[0]?.name).toBe("ghost_tag"); + expect(rows[0]?.status).toBe("not_found"); + expect(firstCache.activeNotFound.has("ghost_tag")).toBe(true); + expect(firstCache.foundTypes.has("ghost_tag")).toBe(false); - it("uses in-memory negative cache for confirmed not_found tags", async () => { - fetchRule34TagMetadataMock.mockResolvedValue({ status: "not_found" }); + resetTagResolveCoordinatorForTests(); + fetchRule34TagMetadataMock.mockClear(); - const settings = { userId: "1", apiKey: "key" }; - const cachedMap = new Map(); + const secondCache = loadTagMetadataCache(mockDb.db, ["ghost_tag"]); + expect(secondCache.activeNotFound.has("ghost_tag")).toBe(true); await resolveTagMetadataWave( mockDb.db, ["ghost_tag"], - cachedMap, + secondCache, settings, - "test-negative-1" + "test-not-found-persist-2" ); + + expect(fetchRule34TagMetadataMock).not.toHaveBeenCalled(); + }); + + it("treats expired not_found as cache-miss and re-resolves", async () => { + const expiredAt = new Date(Date.now() - TAG_RESOLVE_NOT_FOUND_TTL_MS - 1_000); + mockDb.db + .insert(tagMetadata) + .values({ + name: "stale_ghost", + type: TAG_TYPES.GENERAL, + status: "not_found", + resolvedAt: expiredAt, + }) + .run(); + + fetchRule34TagMetadataMock.mockResolvedValue({ + status: "found", + entry: { name: "stale_ghost", type: TAG_TYPES.CHARACTER }, + }); + + const cache = loadTagMetadataCache(mockDb.db, ["stale_ghost"]); + expect(cache.activeNotFound.has("stale_ghost")).toBe(false); + expect(cache.foundTypes.has("stale_ghost")).toBe(false); + await resolveTagMetadataWave( mockDb.db, - ["ghost_tag"], - cachedMap, - settings, - "test-negative-2" + ["stale_ghost"], + cache, + { userId: "1", apiKey: "key" }, + "test-expired-not-found" ); expect(fetchRule34TagMetadataMock).toHaveBeenCalledTimes(1); + expect(cache.foundTypes.get("stale_ghost")).toBe(TAG_TYPES.CHARACTER); + + const row = mockDb.db + .select() + .from(tagMetadata) + .all() + .find((entry) => entry.name === "stale_ghost"); + expect(row?.status).toBe("found"); + }); + + it("migration defaults existing rows to status=found", () => { + mockDb.db + .insert(tagMetadata) + .values({ + name: "legacy_artist", + type: TAG_TYPES.ARTIST, + }) + .run(); + + const row = mockDb.db.select().from(tagMetadata).all()[0]; + expect(row?.status).toBe("found"); + expect(row?.resolvedAt).toBeInstanceOf(Date); + }); + + it("maintenance DELETE keeps fresh Drizzle not_found and removes expired (ms units aligned)", async () => { + fetchRule34TagMetadataMock.mockResolvedValue({ status: "not_found" }); + + const freshCache = loadTagMetadataCache(mockDb.db, ["fresh_miss"]); + await resolveTagMetadataWave( + mockDb.db, + ["fresh_miss"], + freshCache, + { userId: "1", apiKey: "key" }, + "test-maintenance-fresh" + ); + + const rawFresh = mockDb.sqlite + .prepare("SELECT resolved_at FROM tag_metadata WHERE name = ?") + .get("fresh_miss") as { resolved_at: number } | undefined; + expect(rawFresh?.resolved_at).toBeGreaterThan(1_000_000_000_000); + + const deletedFresh = deleteExpiredNotFoundTagMetadata(mockDb.sqlite); + expect(deletedFresh).toBe(0); + expect( + mockDb.db.select().from(tagMetadata).all().some((row) => row.name === "fresh_miss") + ).toBe(true); + + const expiredAt = new Date(Date.now() - TAG_RESOLVE_NOT_FOUND_TTL_MS - 60_000); + mockDb.db + .insert(tagMetadata) + .values({ + name: "expired_miss", + type: TAG_TYPES.GENERAL, + status: "not_found", + resolvedAt: expiredAt, + }) + .run(); + + const deletedExpired = deleteExpiredNotFoundTagMetadata(mockDb.sqlite); + expect(deletedExpired).toBe(1); + const remaining = mockDb.db.select().from(tagMetadata).all(); + expect(remaining.some((row) => row.name === "fresh_miss")).toBe(true); + expect(remaining.some((row) => row.name === "expired_miss")).toBe(false); }); });