fix(db): normalize diffResults on import

docs/WORKLOG.md

@@ -1,3 +1,11 @@
+2025-12-24 11:15 UTC - Fix diffResults import + test hardening
+- Files: services/db/operations/imports.ts; tests/current-system/export-import.test.ts; tests/services/comparisonService.test.ts; tests/adapters/providers/OpenAIAdapter.test.ts; tests/contracts/provider.contract.test.ts; tests/hooks/useChapterTelemetry.test.tsx; docs/WORKLOG.md
+- Why: Imported diffResults could throw `DataError` because export emits `fanVersionId: null` but IndexedDB keys must be valid strings; plus expand coverage for provider/adversarial parsing paths.
+- Details:
+  - Normalized diffResults records during full-session import (coerce `fanVersionId` to `''`, fill hash nulls) so composite keys remain valid.
+  - Strengthened tests around diffResults export/import, OpenAI adapter error paths, comparison JSON extraction, and chapter telemetry perf/logging.
+- Tests: `npx tsc --noEmit`; `npx vitest run tests/current-system/export-import.test.ts`; `npx vitest run tests/services/comparisonService.test.ts`; `npx vitest run tests/adapters/providers/OpenAIAdapter.test.ts`; `npx vitest run tests/hooks/useChapterTelemetry.test.tsx`
+
 2025-11-21 01:00 UTC - TranslationStatusPanel spacing tweak
 - Files: components/chapter/TranslationStatusPanel.tsx (wrapper div class)
 - Why: Chapter body sat flush against the translation/image metric lines; user requested extra spacing after the status rows.

services/db/operations/imports.ts

@@ -9,6 +9,7 @@ import type {
   ExportedImageAsset,
 } from '../types';
 import type { StableSessionData } from '../../stableIdService';
+import type { DiffResult } from '../../diff/types';
 import { getConnection } from '../core/connection';
 import { STORE_NAMES } from '../core/schema';
 import { ImageCacheStore } from '../../imageCacheService';
@@ -25,6 +26,16 @@ const BATCH_SIZE = 50;
 
 const nowIso = () => new Date().toISOString();
 
+type StoredDiffResult = DiffResult & { fanVersionId: string };
+
+const prepareDiffResultForStorage = (record: DiffResult): StoredDiffResult => ({
+  ...record,
+  fanVersionId: record.fanVersionId ?? '',
+  aiHash: record.aiHash ?? null,
+  fanHash: record.fanHash ?? null,
+  rawHash: record.rawHash ?? record.rawVersionId,
+});
+
 const putSettingsRecord = (
   store: IDBObjectStore,
   key: string,
@@ -223,8 +234,8 @@ export class ImportOps {
         tx.oncomplete = () => resolve();
         tx.onerror = () => reject(tx.error);
         const store = tx.objectStore(STORE_NAMES.DIFF_RESULTS);
-        for (const record of diffResults) {
-          store.put(record);
+        for (const record of diffResults as DiffResult[]) {
+          store.put(prepareDiffResultForStorage(record) as StoredDiffResult);
         }
       });
     }

tests/adapters/providers/OpenAIAdapter.test.ts

@@ -166,3 +166,87 @@ describe('OpenAIAdapter translate() parameter handling', () => {
     expect(recordMetricMock).toHaveBeenCalledWith(expect.objectContaining({ success: true }));
   });
 });
+
+describe('OpenAIAdapter adversarial scenarios', () => {
+  beforeEach(() => {
+    openAiMocks.create.mockReset();
+    openAiMocks.ctor.mockClear();
+    recordMetricMock.mockClear();
+    supportsStructuredOutputsMock.mockResolvedValue(false);
+  });
+
+  const makeRequest = (): TranslationRequest => ({
+    title: 'Test',
+    content: 'Content',
+    settings: baseSettings,
+    history: [],
+  });
+
+  it('handles rate limit (429) errors with informative message', async () => {
+    const rateLimitError = new Error('Rate limit exceeded');
+    (rateLimitError as any).status = 429;
+    (rateLimitError as any).headers = { 'retry-after': '30' };
+
+    openAiMocks.create.mockRejectedValueOnce(rateLimitError);
+
+    const adapter = new OpenAIAdapter();
+
+    await expect(adapter.translate(makeRequest())).rejects.toThrow('Rate limit exceeded');
+    expect(recordMetricMock).toHaveBeenCalledWith(
+      expect.objectContaining({ success: false })
+    );
+  });
+
+  it('handles network timeout errors', async () => {
+    const timeoutError = new Error('Request timed out');
+    (timeoutError as any).code = 'ETIMEDOUT';
+
+    openAiMocks.create.mockRejectedValueOnce(timeoutError);
+
+    const adapter = new OpenAIAdapter();
+
+    await expect(adapter.translate(makeRequest())).rejects.toThrow('timed out');
+    expect(recordMetricMock).toHaveBeenCalledWith(
+      expect.objectContaining({ success: false })
+    );
+  });
+
+  it('handles empty response gracefully', async () => {
+    openAiMocks.create.mockResolvedValueOnce({
+      choices: [],
+      usage: { prompt_tokens: 0, completion_tokens: 0 },
+    });
+
+    const adapter = new OpenAIAdapter();
+
+    await expect(adapter.translate(makeRequest())).rejects.toThrow();
+  });
+
+  it('handles null message content', async () => {
+    openAiMocks.create.mockResolvedValueOnce({
+      choices: [{ finish_reason: 'stop', message: { content: null } }],
+      usage: { prompt_tokens: 10, completion_tokens: 0 },
+    });
+
+    const adapter = new OpenAIAdapter();
+
+    await expect(adapter.translate(makeRequest())).rejects.toThrow();
+  });
+
+  it('handles response with missing required fields', async () => {
+    // JSON is valid but missing translatedTitle
+    openAiMocks.create.mockResolvedValueOnce({
+      choices: [{
+        finish_reason: 'stop',
+        message: { content: '{"translation": "content only"}' },
+      }],
+      usage: { prompt_tokens: 10, completion_tokens: 5 },
+    });
+
+    const adapter = new OpenAIAdapter();
+    const result = await adapter.translate(makeRequest());
+
+    // Should not crash - missing fields should be handled gracefully
+    expect(result.translation).toBe('content only');
+  });
+});

tests/contracts/provider.contract.test.ts

@@ -1,20 +1,25 @@
 /**
  * Provider Contract Tests
  *
- * TEST-QUALITY: 8.5/10 (Target: High, integration-style)
+ * STATUS: SCAFFOLD - All tests skipped pending VCR infrastructure
+ * TEST-QUALITY: 2/10 (current) → 8.5/10 (target when implemented)
  *
- * Construct: "Given a prompt + text, provider returns well-formed TranslationResult
- * with correct token accounting and typed errors within timeout."
+ * This file defines the STRUCTURE for provider contract tests but the VCR
+ * (Video Cassette Recording) infrastructure for deterministic replay is not built.
+ * All tests are currently skipped.
  *
- * Ecological validity: Uses VCR-style replay for determinism, but tests real adapter logic.
- * Can optionally run live with LIVE_API_TEST=1 env var.
+ * Target construct: "Given a prompt + text, provider returns well-formed TranslationResult
+ * with correct token accounting and typed errors within timeout."
  *
- * Decision-useful: Catches adapter bugs, API contract changes, token counting errors.
+ * NOTE: Adversarial tests (rate limits, timeouts, malformed responses) are now
+ * implemented in the individual adapter test files where they can be properly tested:
+ * - tests/adapters/providers/OpenAIAdapter.test.ts (adversarial scenarios section)
+ * - tests/adapters/providers/GeminiAdapter.test.ts
  *
- * Gaps addressed from audit:
- * - Mock overuse: Tests real adapter code (only network is replayed)
- * - No ground truth: Validates token counts, cost calculations
- * - No adversarial: Tests rate limits, timeouts, unknown models
+ * To make these contract tests real:
+ * 1. Implement VCR cassette recording/replay (see TODO at bottom)
+ * 2. Hook up actual adapters instead of inline mock responses
+ * 3. Remove .skip from test cases
  */
 
 import { describe, it, expect, beforeEach, vi } from 'vitest';
@@ -278,49 +283,11 @@ describe('Provider Contract: Gemini', () => {
   });
 });
 
-// Adversarial contract tests
-describe('Provider Contract: Adversarial Cases', () => {
-  it.skip('[Adversarial] rate limit: backs off correctly', async () => {
-    // TODO: Implement rate limit testing
-    // Should test:
-    // 1. Adapter receives 429 response
-    // 2. Waits appropriate time (respecting Retry-After header)
-    // 3. Retries with exponential backoff
-    // 4. Eventually succeeds or returns typed error
-
-    expect(true).toBe(true); // Placeholder
-  });
-
-  it.skip('[Adversarial] timeout: returns partial result or error', async () => {
-    // TODO: Test timeout handling
-    // Should test:
-    // 1. Request exceeds timeout threshold
-    // 2. Adapter cancels request
-    // 3. Returns typed error with timeout info
-
-    expect(true).toBe(true); // Placeholder
-  });
-
-  it.skip('[Adversarial] unknown model: maps or fails predictably', async () => {
-    // TODO: Test unknown model handling
-    // Should test:
-    // 1. Adapter receives unknown model name
-    // 2. Either maps to known model (with warning log)
-    // 3. Or returns typed error (not crash)
-
-    expect(true).toBe(true); // Placeholder
-  });
-
-  it.skip('[Adversarial] malformed API response: handles gracefully', async () => {
-    // TODO: Test malformed response handling
-    // Should test:
-    // 1. API returns invalid JSON
-    // 2. API returns unexpected structure
-    // 3. Adapter returns typed error (not crash)
-
-    expect(true).toBe(true); // Placeholder
-  });
-});
+// NOTE: Adversarial contract tests (rate limits, timeouts, malformed responses)
+// have been moved to individual adapter test files where they can be properly tested:
+// - tests/adapters/providers/OpenAIAdapter.test.ts → "adversarial scenarios" describe block
+// - tests/adapters/providers/GeminiAdapter.test.ts
+// This avoids placeholder stubs that inflate test counts without testing behavior.
 
 /**
  * Implementation TODO (for full 8.5/10 score):

tests/current-system/export-import.test.ts

@@ -130,6 +130,24 @@ describe('Session export/import', () => {
     });
 
     it('should restore diffResults from imported session data', async () => {
+      const testDiffResult: DiffResult = {
+        chapterId: 'stable-import',
+        aiVersionId: '9876543210',
+        fanVersionId: null,
+        rawVersionId: 'xyz98765',
+        algoVersion: '1.0.0',
+        markers: [{
+          chunkId: 'para-0-xyz',
+          colors: ['orange'],
+          reasons: ['raw-divergence'],
+          aiRange: { start: 0, end: 20 },
+          position: 0
+        }],
+        analyzedAt: Date.now(),
+        costUsd: 0.002,
+        model: 'gpt-4o-mini'
+      };
+
       const sessionData = {
         metadata: { format: 'lexiconforge-full-1', exportedAt: new Date().toISOString() },
         settings: null,
@@ -138,42 +156,21 @@ describe('Session export/import', () => {
         novels: [],
         chapters: [],
         promptTemplates: [],
-        diffResults: [
-          {
-            chapterId: 'stable-import',
-            aiVersionId: '9876543210',
-            fanVersionId: null,
-            rawVersionId: 'xyz98765',
-            algoVersion: '1.0.0',
-            markers: [{
-              chunkId: 'para-0-xyz',
-              colors: ['orange'],
-              reasons: ['raw-divergence'],
-              aiRange: { start: 0, end: 20 },
-              position: 0
-            }],
-            analyzedAt: Date.now(),
-            costUsd: 0.002,
-            model: 'gpt-4o-mini'
-          }
-        ] satisfies DiffResult[]
+        diffResults: [testDiffResult]
       };
 
-      // Import should succeed or gracefully skip if diffResults store doesn't exist
-      // (older DB versions may not have the store)
-      try {
-        await ImportOps.importFullSessionData(sessionData);
-        // If we get here, import succeeded
-        expect(true).toBe(true);
-      } catch (error: any) {
-        // If error is because diffResults store doesn't exist, that's acceptable in tests
-        // Otherwise, rethrow
-        if (!error.message?.includes('diffResults') && !error.message?.includes('Data provided')) {
-          throw error;
-        }
-        // Store doesn't exist in test DB - this is OK
-        expect(true).toBe(true);
-      }
+      // Import the session data
+      await ImportOps.importFullSessionData(sessionData);
+
+      // Verify the diffResult was actually imported by reading it back
+      const allDiffResults = await DiffOps.getAll();
+      const imported = allDiffResults.find(r => r.chapterId === 'stable-import');
+
+      expect(imported).toBeDefined();
+      expect(imported?.aiVersionId).toBe('9876543210');
+      expect(imported?.model).toBe('gpt-4o-mini');
+      expect(imported?.markers).toHaveLength(1);
+      expect(imported?.markers[0].reasons).toContain('raw-divergence');
     });
 
     it('should handle export when no diffResults exist', async () => {