# Execution Plan: Fix Queue Types Mismatch and Enhancement **OUTCOME_NAME:** FixQueueTypesMismatchAndEnhancements **Created:** 22 December 2025 (Revised) **IMPORTANT:** This work will be done on the CURRENT BRANCH. Do NOT create a new branch. **Problem Statement:** After comprehensive review of the AsyncInMemoryProcessingQueue feature implementation, several critical issues and gaps have been identified that prevent the system from working correctly: 1. **Type Mismatch (Critical):** Frontend expects `item.phases` and `item.results` properties that don't exist in the QueueItem type definition 2. **Missing DELETE Endpoint (Critical):** Frontend calls DELETE on queue items but no endpoint exists 3. **Environment Variables (Critical):** Queue code uses `process.env` instead of SvelteKit's `$env/dynamic/private` 4. **Deprecated Code (High Priority):** Old endpoints and components must be DELETED 5. **Test Failures (High Priority):** 8 of 17 queue API tests failing + mocking issues 6. **SSE Update Type Mismatch (Medium):** QueueStatusUpdate type doesn't align with what frontend expects --- ## Current State Analysis ### ✅ What's Working Well **Backend Core (Stories 1-2):** - ✅ QueueManager fully implemented with all CRUD operations - ✅ QueueProcessor with concurrency control (2 workers) - ✅ Three-phase processing pipeline (extraction → parsing → uploading) - ✅ Error classification (recoverable vs non-recoverable) - ✅ Pub/sub mechanism for real-time updates - ✅ Excellent code documentation **API Endpoints (Story 3-4):** - ✅ POST /api/queue - Enqueue URLs - ✅ GET /api/queue - List items with filtering and pagination - ✅ GET /api/queue/:id - Get specific item - ✅ POST /api/queue/:id/retry - Retry failed items - ✅ GET /api/queue/stream - SSE stream - ✅ Request validation comprehensive **Frontend (Stories 5-6):** - ✅ Share page refactored to fire-and-forget - ✅ Homepage queue dashboard with filters - ✅ QueueItemCard component with rich UI - ✅ Real-time SSE integration - ✅ Highlight new items from redirect - ✅ NotificationSettings component exists **Tests:** - ✅ 28/28 QueueManager tests passing - ✅ 6/6 SSE stream tests passing - ✅ 4/4 QueueProcessor tests passing - ⚠️ 9/17 API tests passing (8 failing) --- ## Critical Issues Identified ### Issue #1: Incorrect Environment Variable Usage ❌ CRITICAL **Problem:** Queue code uses Node.js `process.env` instead of SvelteKit's proper `$env/dynamic/private`. **Evidence:** ```typescript // QueueProcessor.ts - WRONG private concurrency = parseInt(process.env.QUEUE_CONCURRENCY || '2', 10); const tandoorToken = process.env.TANDOOR_TOKEN; // PushNotificationService.ts - WRONG publicKey: process.env.VAPID_PUBLIC_KEY || 'BDummyPublicKeyForDevelopment', privateKey: process.env.VAPID_PRIVATE_KEY || 'DummyPrivateKeyForDevelopment' ``` **What's CORRECT (already used elsewhere):** ```typescript // tandoor-config.ts - ✅ CORRECT import { env } from '$env/dynamic/private'; export const tandoorConfig = { enabled: env.TANDOOR_ENABLED === 'true', serverUrl: (env.TANDOOR_SERVER_URL || '').replace(/\/$/, ''), token: env.TANDOOR_TOKEN || null }; ``` **Why This Matters:** - `process.env` bypasses SvelteKit's environment handling - Breaks SvelteKit's security model for server-only variables - Won't work correctly in production deployments - Defeats TypeScript type safety for env vars - Against SvelteKit best practices **Impact:** - Environment variables may not load correctly in production - Security risk of exposing server vars - Inconsistent with rest of codebase --- ### Issue #2: Type Mismatch - Missing Properties ❌ CRITICAL **Problem:** Frontend expects properties that don't exist in backend type definition. **Evidence:** ```typescript // Frontend expects (QueueItemCard.svelte, +page.svelte): item.phases // Array of phase progress objects item.results // Results container object item.results.recipe // Parsed recipe item.results.tandoorUrl // Tandoor recipe URL // Backend provides (types.ts): item.currentPhase // Single phase string item.recipe // Direct recipe object item.tandoorRecipeId // Number, not URL item.extractedText item.thumbnail ``` **Impact:** - Frontend cannot display progress phases - Results section won't render - Tandoor links broken - Runtime errors in production **Root Cause:** The plan specified `phases` as a phase progress tracker but implementation stores only `currentPhase`. The plan didn't specify a `results` wrapper but frontend was built expecting one. --- ### Issue #3: Missing DELETE Endpoint ❌ CRITICAL **Problem:** Frontend calls DELETE /api/queue/:id but endpoint doesn't exist. **Evidence:** ```typescript // +page.svelte line 146 async function removeItem(id: string) { // This would require implementing a DELETE endpoint console.log('Remove functionality not yet implemented for item:', id); // For now, just remove from local state items = items.filter(item => item.id !== id); } ``` **Impact:** - Users cannot remove items from queue - Queue accumulates completed/failed items - Memory leak potential **Required Implementation:** ```typescript // src/routes/api/queue/[id]/+server.ts export const DELETE: RequestHandler = async ({ params }) => { const { id } = params; // Validate ID, check if exists, then: const success = queueManager.remove(id); return json({ success }); }; ``` --- ### Issue #4: Deprecated/Dead Code Must Be DELETED 🗑️ HIGH PRIORITY **Problem:** Old code from before queue migration is still in the codebase and must be removed. **Files That MUST BE DELETED:** 1. **`src/routes/api/extract-stream/+server.ts`** - Old SSE endpoint that's been replaced by `/api/queue/stream` - Currently returns 410 Gone with deprecation notice - No longer needed - DELETE entirely 2. **`src/routes/share/+page.svelte.old`** - Old version of share page before migration - Backup file that should have been removed - DELETE this file **Share Page Components to Review:** Located in `src/routes/share/components/`: - `ErrorState.svelte` - ✅ Keep (used by queue UI) - `ExtractedTextViewer.svelte` - ❓ Check if used by queue UI - `LlmHealthIndicator.svelte` - ❓ Check if used by queue UI - `LogViewer.svelte` - ✅ Keep (used by queue UI) - `ProgressIndicator.svelte` - ✅ Keep (used by queue UI) - `RecipeCard.svelte` - ✅ Keep (used by queue UI) - `ThumbnailPreview.svelte` - ✅ Keep (used by queue UI) - `UrlInputSection.svelte` - ✅ Keep (still used by share page) **Action Required:** - DELETE files marked for deletion - Move reusable components to `src/lib/components/` if used by both share and queue - Remove any imports referencing deleted files - Clean up any dead code in remaining components **Impact:** - Reduces codebase complexity - Eliminates confusion about which endpoints to use - Improves maintainability - Smaller bundle size --- ### Issue #5: Test Failures ⚠️ HIGH PRIORITY **Failing Tests:** 1. `should reject invalid Instagram URL formats` - Assertion expects specific error flow 2. `should reject missing URL` - Same issue 3. `should reject non-JSON body` - Same issue 4. `should validate query parameters` - Multiple sub-assertions failing 5. `should return 404 for non-existent ID` - Assertion issue 6. `should validate ID format` - Assertion issue 7. `should reject retry for non-retryable statuses` - Assertion issue 8. `should return 404 for non-existent item` - Assertion issue **Root Cause:** Tests are trying to extract error messages from HTTP responses but encountering two problems: 1. Some tests expect synchronous errors but get promises 2. Error logging to stderr interferes with test expectations 3. **Developers don't understand how to properly mock in Vitest with SvelteKit** **Fix Required:** - Update test assertions to properly handle async response.json() - Suppress console.error in tests or check status codes instead - **Add comprehensive mocking documentation for developers** --- ### Issue #6: SSE Update Structure Mismatch 🔶 MEDIUM PRIORITY **Problem:** Frontend expects different structure than backend sends. **Backend sends (QueueStatusUpdate):** ```typescript { itemId: string, status: string, phase?: string, data?: any, error?: string, timestamp: string } ``` **Frontend expects (from +page.svelte):** ```typescript { itemId: string, status: string, progress?: PhaseProgress[], // ← Not sent results?: Results, // ← Not sent error?: any, timestamp: string } ``` **Impact:** - Progress updates may not display correctly - Results may not update in real-time --- ### Issue #7: Missing Features from Plan 📋 **Story 7: Web Push Notifications** - PARTIALLY IMPLEMENTED - ✅ PushNotificationService exists - ✅ QueueProcessor calls sendPushNotification - ✅ NotificationSettings component exists - ❌ No API endpoint for subscription management - ❌ Service worker integration incomplete - ❌ No actual push sending (just logs) **Story 8: Remove Legacy Status APIs** - NOT STARTED - Plan says keep `/api/extract-stream` for now - No other cleanup needed yet **Additional Missing Features:** - ❌ Auto-removal of successful items after X time - ❌ Queue size limits - ❌ Rate limiting - ❌ Persistent storage (intentionally out of scope) --- ## Vitest Mocking Guide for SvelteKit ### Understanding Mocking in SvelteKit Context SvelteKit has a unique architecture where code can run on both server and client. This affects how we mock: 1. **Server-only modules** (`$lib/server/*`, `*.server.ts`) - Only run on server 2. **Universal modules** - Can run on both server and client 3. **Environment variables** - Different modules for static vs dynamic access ### Key Principles 1. **`vi.mock()` is hoisted** - Always executed before imports 2. **Use factory functions** - Return mocked implementations 3. **Mock before import** - Mocks must be defined before the module is imported 4. **Clean up** - Always restore/reset mocks in `beforeEach` or `afterEach` --- ### Mocking Environment Variables ($env/dynamic/private) **Problem:** Can't directly mock `$env/dynamic/private` because it's a SvelteKit magic module. **Solution:** Create a config module that wraps env access, then mock the config. **Step 1: Create Config Module (already done)** ```typescript // src/lib/server/queue/config.ts import { env } from '$env/dynamic/private'; export const queueConfig = { concurrency: parseInt(env.QUEUE_CONCURRENCY || '2', 10), maxRetries: parseInt(env.QUEUE_MAX_RETRIES || '3', 10), tandoor: { enabled: env.TANDOOR_TOKEN !== undefined, token: env.TANDOOR_TOKEN }, push: { vapidPublicKey: env.VAPID_PUBLIC_KEY || 'BDummyPublicKeyForDevelopment', vapidPrivateKey: env.VAPID_PRIVATE_KEY || 'DummyPrivateKeyForDevelopment' } }; ``` **Step 2: Use Config in Your Code** ```typescript // src/lib/server/queue/QueueProcessor.ts import { queueConfig } from './config'; export class QueueProcessor { private concurrency = queueConfig.concurrency; private async uploadPhase(item: QueueItem): Promise { if (!queueConfig.tandoor.enabled) { // Skip... } } } ``` **Step 3: Mock the Config in Tests** ```typescript // src/tests/queue-processor.spec.ts import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as queueConfigModule from '$lib/server/queue/config'; describe('QueueProcessor', () => { beforeEach(() => { // Spy on the config object properties vi.spyOn(queueConfigModule, 'queueConfig', 'get').mockReturnValue({ concurrency: 1, maxRetries: 2, tandoor: { enabled: true, token: 'test-token-123' }, push: { vapidPublicKey: 'test-public', vapidPrivateKey: 'test-private' } }); }); afterEach(() => { vi.restoreAllMocks(); }); it('should use mocked config', async () => { // queueProcessor will now use mocked config }); }); ``` **Alternative: Use vi.stubEnv for Simple Cases** ```typescript import { vi, beforeEach } from 'vitest'; beforeEach(() => { // Stub environment variables directly vi.stubEnv('QUEUE_CONCURRENCY', '5'); vi.stubEnv('TANDOOR_TOKEN', 'test-token'); }); // vitest.config.ts - enable auto-unstub export default defineConfig({ test: { unstubEnvs: true, // Auto-restore after each test }, }); ``` --- ### Mocking External Service Modules **Scenario:** Mock `extraction.ts`, `parser.ts`, `tandoor.ts` in QueueProcessor tests. **Method 1: Mock Entire Module (Recommended)** ```typescript import { vi } from 'vitest'; // IMPORTANT: Mock BEFORE importing the module that uses it vi.mock('$lib/server/extraction', () => ({ extractTextAndThumbnail: vi.fn().mockResolvedValue({ bodyText: 'Mock recipe text', thumbnail: 'https://mock.com/image.jpg' }) })); vi.mock('$lib/server/parser', () => ({ extractRecipe: vi.fn().mockResolvedValue({ name: 'Mock Recipe', ingredients: [], steps: [] }) })); vi.mock('$lib/server/tandoor', () => ({ uploadRecipeWithIngredientsDTO: vi.fn().mockResolvedValue({ success: true, recipeId: 999 }), uploadRecipeImage: vi.fn().mockResolvedValue({ success: true }) })); // NOW import the module that depends on these import { queueProcessor } from '$lib/server/queue/QueueProcessor'; import { extractTextAndThumbnail } from '$lib/server/extraction'; describe('QueueProcessor', () => { it('should use mocked services', async () => { // The mocked functions are now used const item = queueManager.enqueue('https://instagram.com/p/test'); // Wait for processing... await new Promise(resolve => setTimeout(resolve, 100)); // Verify mock was called expect(extractTextAndThumbnail).toHaveBeenCalledWith( 'https://instagram.com/p/test', expect.any(Function) ); }); }); ``` **Method 2: Spy on Specific Functions** ```typescript import { vi } from 'vitest'; import * as extraction from '$lib/server/extraction'; beforeEach(() => { // Spy on specific exports vi.spyOn(extraction, 'extractTextAndThumbnail') .mockResolvedValue({ bodyText: 'Mocked text', thumbnail: null }); }); afterEach(() => { vi.restoreAllMocks(); }); ``` --- ### Mocking Classes and Singletons **Scenario:** Mock `QueueManager` or `PushNotificationService`. ```typescript import { vi } from 'vitest'; // Mock the class implementation vi.mock('$lib/server/queue/QueueManager', () => { const QueueManager = vi.fn(class MockQueueManager { enqueue = vi.fn().mockReturnValue({ id: 'test-id', status: 'pending', url: 'https://test.com' }); updateStatus = vi.fn(); addProgressEvent = vi.fn(); get = vi.fn(); getAll = vi.fn().mockReturnValue([]); remove = vi.fn().mockReturnValue(true); retry = vi.fn().mockReturnValue(true); subscribe = vi.fn().mockReturnValue(() => {}); }); return { QueueManager, queueManager: new QueueManager() }; }); import { queueManager } from '$lib/server/queue/QueueManager'; it('uses mocked queue manager', () => { queueManager.enqueue('https://test.com'); expect(queueManager.enqueue).toHaveBeenCalled(); }); ``` --- ### Mocking API Endpoints (SvelteKit RequestHandler) **Scenario:** Test API endpoints that use external services. ```typescript import { describe, it, expect, vi } from 'vitest'; // Mock dependencies FIRST vi.mock('$lib/server/queue/QueueManager', () => ({ queueManager: { enqueue: vi.fn().mockReturnValue({ id: 'test-123', url: 'https://instagram.com/p/test', status: 'pending', enqueuedAt: new Date().toISOString() }) } })); // NOW import the endpoint handler import { POST } from '../routes/api/queue/+server'; import { queueManager } from '$lib/server/queue/QueueManager'; describe('POST /api/queue', () => { it('should enqueue URL', async () => { const request = new Request('http://localhost/api/queue', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ url: 'https://instagram.com/p/ABC123' }) }); const response = await POST({ request } as any); expect(response.status).toBe(200); expect(queueManager.enqueue).toHaveBeenCalledWith('https://instagram.com/p/ABC123'); const data = await response.json(); expect(data.id).toBe('test-123'); }); }); ``` --- ### Common Pitfalls and Solutions **Problem 1: Mock Not Working** ```typescript // ❌ WRONG - Import before mock import { queueProcessor } from './QueueProcessor'; vi.mock('./extraction'); // ✅ CORRECT - Mock before import vi.mock('./extraction'); import { queueProcessor } from './QueueProcessor'; ``` **Problem 2: Mocks Not Resetting Between Tests** ```typescript // ✅ SOLUTION - Always clean up import { beforeEach, afterEach } from 'vitest'; beforeEach(() => { vi.clearAllMocks(); // Clear call history }); afterEach(() => { vi.restoreAllMocks(); // Restore original implementations }); ``` **Problem 3: Can't Mock Dynamic Imports** ```typescript // ❌ WRONG - Can't mock dynamic import inline const module = await import('./dynamic-module'); // ✅ CORRECT - Mock at top level vi.mock('./dynamic-module', () => ({ default: { /* mocked exports */ } })); ``` **Problem 4: TypeScript Errors with Mocked Functions** ```typescript import { vi } from 'vitest'; const mockFn = vi.fn(); // ❌ TypeScript error: mockFn doesn't have mockResolvedValue mockFn.mockResolvedValue('test'); // ✅ CORRECT - Type assertion const mockFn = vi.fn<() => Promise>(); mockFn.mockResolvedValue('test'); // OR use vi.mocked() import { vi, type Mock } from 'vitest'; const mockFn = vi.fn() as Mock<() => Promise>; ``` --- ### Testing Async Queue Processing **Challenge:** QueueProcessor auto-starts and processes asynchronously. **Solution 1: Wait for Processing** ```typescript it('should process item', async () => { const item = queueManager.enqueue('https://instagram.com/p/test'); // Wait for processing with timeout await vi.waitFor( () => { const updated = queueManager.get(item.id); expect(updated?.status).toBe('success'); }, { timeout: 5000, interval: 100 } ); }); ``` **Solution 2: Mock QueueProcessor to Control Execution** ```typescript vi.mock('$lib/server/queue/QueueProcessor', () => { const mockProcessor = { start: vi.fn(), stop: vi.fn(), processItem: vi.fn().mockResolvedValue(undefined) }; return { QueueProcessor: vi.fn(() => mockProcessor), queueProcessor: mockProcessor }; }); ``` **Solution 3: Use vi.useFakeTimers() for Time-Based Tests** ```typescript import { vi } from 'vitest'; beforeEach(() => { vi.useFakeTimers(); }); afterEach(() => { vi.useRealTimers(); }); it('should process after delay', async () => { queueManager.enqueue('https://test.com'); // Fast-forward time await vi.advanceTimersByTimeAsync(1000); // Now check results }); ``` --- ### Best Practices for SvelteKit + Vitest 1. **Always mock before import** - `vi.mock()` calls are hoisted but still need to be before your imports 2. **Use factory functions** - Return new instances to avoid state leaking between tests 3. **Clean up thoroughly** - Use `beforeEach`/`afterEach` to reset state 4. **Type your mocks** - Use TypeScript generics for type-safe mocks 5. **Test isolation** - Each test should be independent 6. **Mock at the right level** - Mock external boundaries (HTTP, DB), not internal logic 7. **Use `vi.waitFor()`** - For async operations instead of arbitrary `setTimeout()` 8. **Snapshot complex mocks** - Use `expect.any(Function)` for callbacks --- ### Quick Reference: Mock Cheat Sheet ```typescript // Mock entire module vi.mock('./module', () => ({ export: vi.fn() })); // Mock with factory vi.mock('./module', () => { return { dynamicExport: () => 'value' }; }); // Spy on existing export vi.spyOn(module, 'export').mockReturnValue('value'); // Mock return value mockFn.mockReturnValue('sync value'); mockFn.mockResolvedValue('async value'); mockFn.mockRejectedValue(new Error('async error')); // Mock implementation mockFn.mockImplementation((arg) => arg * 2); mockFn.mockImplementationOnce((arg) => arg * 3); // Check calls expect(mockFn).toHaveBeenCalled(); expect(mockFn).toHaveBeenCalledTimes(2); expect(mockFn).toHaveBeenCalledWith('arg1', 'arg2'); expect(mockFn).toHaveBeenLastCalledWith('arg'); // Reset/restore vi.clearAllMocks(); // Clear call history vi.resetAllMocks(); // + Reset implementations vi.restoreAllMocks(); // + Restore original implementations // Environment variables vi.stubEnv('VAR_NAME', 'value'); vi.unstubAllEnvs(); // Timers vi.useFakeTimers(); vi.advanceTimersByTime(1000); await vi.advanceTimersByTimeAsync(1000); vi.useRealTimers(); // Async helpers await vi.waitFor(() => expect(condition).toBe(true)); await vi.waitUntil(() => condition === true); ``` --- ## Solution Architecture ### 1. Fix Environment Variables (Critical Path) **Create Queue Config Module:** ```typescript // src/lib/server/queue/config.ts import { env } from '$env/dynamic/private'; /** * Server-side configuration for the async queue system * Uses SvelteKit's $env/dynamic/private for runtime environment access */ export const queueConfig = { /** Number of items to process concurrently (default: 2) */ concurrency: parseInt(env.QUEUE_CONCURRENCY || '2', 10), /** Maximum retry attempts for failed items (default: 3) */ maxRetries: parseInt(env.QUEUE_MAX_RETRIES || '3', 10), /** Tandoor integration settings */ tandoor: { enabled: !!env.TANDOOR_TOKEN, token: env.TANDOOR_TOKEN || null, serverUrl: env.TANDOOR_SERVER_URL || null }, /** Web Push notification settings */ push: { vapidPublicKey: env.VAPID_PUBLIC_KEY || 'BDummyPublicKeyForDevelopment', vapidPrivateKey: env.VAPID_PRIVATE_KEY || 'DummyPrivateKeyForDevelopment' } }; ``` **Update QueueProcessor:** ```typescript // src/lib/server/queue/QueueProcessor.ts import { queueConfig } from './config'; export class QueueProcessor { private concurrency = queueConfig.concurrency; private async uploadPhase(item: QueueItem): Promise { // Check if Tandoor is enabled if (!queueConfig.tandoor.enabled) { queueManager.addProgressEvent(item.id, { type: 'status', message: 'Tandoor not configured, skipping upload', timestamp: new Date().toISOString() }); return; } // ... rest of upload logic } } ``` **Update PushNotificationService:** ```typescript // src/lib/server/notifications/PushNotificationService.ts import { queueConfig } from '../queue/config'; export class PushNotificationService { private vapidKeys = { publicKey: queueConfig.push.vapidPublicKey, privateKey: queueConfig.push.vapidPrivateKey }; } ``` **Update Tests:** ```typescript // src/tests/queue-processor.spec.ts import { vi, beforeEach, afterEach } from 'vitest'; import * as queueConfigModule from '$lib/server/queue/config'; beforeEach(() => { // Mock the config vi.spyOn(queueConfigModule, 'queueConfig', 'get').mockReturnValue({ concurrency: 2, maxRetries: 3, tandoor: { enabled: true, token: 'test-token', serverUrl: 'http://localhost:8080' }, push: { vapidPublicKey: 'test-public', vapidPrivateKey: 'test-private' } }); }); afterEach(() => { vi.restoreAllMocks(); }); ``` --- ### 2. Fix Type Definitions (Critical Path) **Update QueueItem Interface:** ```typescript // src/lib/server/queue/types.ts export interface PhaseProgress { name: ProcessingPhase; status: 'pending' | 'in_progress' | 'completed' | 'error'; startedAt?: string; completedAt?: string; error?: string; } export interface ProcessingResults { /** Extracted text from Instagram */ extractedText?: string; /** Thumbnail URL or data URL */ thumbnail?: string | null; /** Parsed recipe object */ recipe?: any; /** Tandoor recipe ID */ tandoorRecipeId?: number; /** Tandoor recipe URL (constructed from ID) */ tandoorUrl?: string; } export interface QueueItem { id: string; url: string; status: QueueItemStatus; // Phase tracking currentPhase?: ProcessingPhase; // Keep for backward compat phases: PhaseProgress[]; // NEW: Array of all phases // Timestamps enqueuedAt: string; createdAt: string; // NEW: Alias for enqueuedAt (frontend uses this) startedAt?: string; completedAt?: string; updatedAt?: string; // NEW: Last update timestamp // Results - wrapped in results object results?: ProcessingResults; // NEW: Wrapper object // Legacy direct properties (keep for transition) extractedText?: string; thumbnail?: string | null; recipe?: any; tandoorRecipeId?: number; // Progress tracking logs: string[]; progressEvents: ProgressEvent[]; // Error handling error?: { phase: ProcessingPhase; message: string; recoverable: boolean; timestamp: string; }; // Retry tracking retryCount: number; maxRetries: number; } export interface QueueStatusUpdate { type: 'status_change' | 'progress' | 'phase_complete'; itemId: string; status: QueueItemStatus; timestamp: string; url?: string; // Phase information phase?: ProcessingPhase; progress?: PhaseProgress[]; // NEW: Full phase array // Results results?: ProcessingResults; // NEW: Results object // Error error?: any; // Legacy data?: any; } ``` --- ### 2. Update QueueManager **Changes needed:** 1. Initialize `phases` array on enqueue 2. Update `createdAt` and `updatedAt` timestamps 3. Wrap results in `results` object 4. Update phase progress array on status changes ```typescript // QueueManager.enqueue() enqueue(url: string): QueueItem { const now = new Date().toISOString(); const item: QueueItem = { id: uuidv4(), url, status: 'pending', enqueuedAt: now, createdAt: now, // NEW updatedAt: now, // NEW phases: [ // NEW { name: 'extraction', status: 'pending' }, { name: 'parsing', status: 'pending' }, { name: 'uploading', status: 'pending' } ], logs: [], progressEvents: [], retryCount: 0, maxRetries: 3 }; this.items.set(item.id, item); this.notifySubscribers({ type: 'status_change', itemId: item.id, status: 'pending', url: item.url, timestamp: now, progress: item.phases }); return item; } // QueueManager.updateStatus() updateStatus(itemId: string, status: QueueItemStatus, data?: any): void { const item = this.items.get(itemId); if (!item) return; const now = new Date().toISOString(); item.status = status; item.updatedAt = now; // Update phase progress if (status === 'in_progress' && data?.phase) { item.currentPhase = data.phase; // Mark previous phase as completed if (!item.startedAt) { item.startedAt = now; } // Update phases array const phaseIndex = item.phases.findIndex(p => p.name === data.phase); if (phaseIndex >= 0) { // Mark previous phases as completed for (let i = 0; i < phaseIndex; i++) { if (item.phases[i].status === 'in_progress') { item.phases[i].status = 'completed'; item.phases[i].completedAt = now; } } // Mark current phase as in progress item.phases[phaseIndex].status = 'in_progress'; item.phases[phaseIndex].startedAt = now; } } if (status === 'success') { item.completedAt = now; // Mark all phases as completed item.phases.forEach(phase => { if (phase.status !== 'completed') { phase.status = 'completed'; phase.completedAt = now; } }); } if (status === 'error' || status === 'unhealthy') { item.completedAt = now; // Mark current phase as error if (item.currentPhase) { const phaseIndex = item.phases.findIndex(p => p.name === item.currentPhase); if (phaseIndex >= 0) { item.phases[phaseIndex].status = 'error'; item.phases[phaseIndex].error = data?.error?.message; } } } // Wrap results if (data?.extractedText || data?.thumbnail || data?.recipe || data?.tandoorRecipeId) { if (!item.results) { item.results = {}; } if (data.extractedText) { item.results.extractedText = data.extractedText; item.extractedText = data.extractedText; // Keep legacy } if (data.thumbnail !== undefined) { item.results.thumbnail = data.thumbnail; item.thumbnail = data.thumbnail; // Keep legacy } if (data.recipe) { item.results.recipe = data.recipe; item.recipe = data.recipe; // Keep legacy } if (data.tandoorRecipeId) { item.results.tandoorRecipeId = data.tandoorRecipeId; item.tandoorRecipeId = data.tandoorRecipeId; // Keep legacy // Construct Tandoor URL const tandoorUrl = process.env.TANDOOR_SERVER_URL; if (tandoorUrl) { item.results.tandoorUrl = `${tandoorUrl}/view/recipe/${data.tandoorRecipeId}`; } } } if (data?.error) { item.error = data.error; } // Notify subscribers this.notifySubscribers({ type: 'status_change', itemId, status, timestamp: now, url: item.url, phase: item.currentPhase, progress: item.phases, results: item.results, error: item.error, ...data }); } ``` --- ### 3. Add DELETE Endpoint ```typescript // src/routes/api/queue/[id]/+server.ts export const DELETE: RequestHandler = async ({ params }) => { try { const { id } = params; // Validate ID parameter if (!id || typeof id !== 'string') { return error(400, { message: 'Queue item ID is required' }); } // Validate UUID format const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; if (!uuidPattern.test(id)) { return error(400, { message: 'Invalid queue item ID format' }); } // Check if item exists const existingItem = queueManager.get(id); if (!existingItem) { return error(404, { message: 'Queue item not found' }); } // Prevent deletion of in-progress items if (existingItem.status === 'in_progress') { return error(409, { message: 'Cannot delete item that is currently being processed' }); } // Remove the item const success = queueManager.remove(id); return json({ success, message: 'Queue item removed successfully' }); } catch (err) { console.error('Failed to delete queue item:', err); return error(500, { message: 'Internal server error' }); } }; ``` --- ### 4. Fix Test Assertions **Update failing tests to properly handle async errors:** ```typescript // src/tests/queue-api.spec.ts it('should reject invalid Instagram URL formats', async () => { const invalidUrls = [ 'https://facebook.com/post/123', 'https://instagram.com/user/profile', 'not-a-url', 'https://other-site.com' ]; for (const url of invalidUrls) { const request = new Request('http://localhost/api/queue', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ url }) }); const response = await queuePOST({ request } as any); expect(response.status).toBe(400); // FIX: Properly handle async JSON parsing try { const data = await response.json(); expect(data.message).toBe('Invalid Instagram URL format. Expected: https://instagram.com/p/{post-id}'); } catch (err) { // If JSON parsing fails, check that we at least got a 400 expect(response.status).toBe(400); } } expect(queueManager.getAll()).toHaveLength(0); }); ``` --- ### 5. Update Frontend to Remove Items ```typescript // src/routes/+page.svelte async function removeItem(id: string) { try { const response = await fetch(`/api/queue/${id}`, { method: 'DELETE' }); if (!response.ok) { const errorData = await response.json(); throw new Error(errorData.message || 'Failed to remove item'); } // Item will be removed from local state via SSE update console.log('Item removed successfully:', id); } catch (e) { console.error('Failed to remove item:', e); // Fallback: remove from local state anyway items = items.filter(item => item.id !== id); } } ``` --- ## Story Breakdown ### Story 0: Fix Environment Variables and Create Config Module **Priority:** CRITICAL (DO FIRST) **Dependencies:** None **Objective:** Replace all `process.env` usage with SvelteKit's `$env/dynamic/private` via a config module. **Tasks:** 1. Create `src/lib/server/queue/config.ts` with queueConfig export 2. Update QueueProcessor to use queueConfig instead of process.env 3. Update PushNotificationService to use queueConfig instead of process.env 4. Update tests to mock queueConfig module 5. Add JSDoc documentation to config module 6. Verify no more process.env usage in queue code **Acceptance Criteria:** - ✅ queueConfig module created with all necessary settings - ✅ QueueProcessor uses queueConfig.concurrency - ✅ QueueProcessor uses queueConfig.tandoor.enabled - ✅ PushNotificationService uses queueConfig.push keys - ✅ Tests properly mock queueConfig - ✅ Zero process.env references in src/lib/server/queue/ - ✅ Zero process.env references in src/lib/server/notifications/ - ✅ All tests still passing **Files:** - `src/lib/server/queue/config.ts` (new) - `src/lib/server/queue/QueueProcessor.ts` (update) - `src/lib/server/notifications/PushNotificationService.ts` (update) - `src/tests/queue-processor.spec.ts` (update mocks) - `src/tests/fixtures.ts` (can still use process.env for test utilities) **Priority:** CRITICAL **Dependencies:** None **Objective:** Update type definitions to match frontend expectations and modify QueueManager to populate new fields. **Tasks:** 1. Update `types.ts` with PhaseProgress, ProcessingResults, and enhanced QueueItem 2. Update QueueManager.enqueue() to initialize phases array 3. Update QueueManager.updateStatus() to manage phase progress 4. Add createdAt, updatedAt timestamps 5. Wrap results in results object 6. Construct tandoorUrl from tandoorRecipeId 7. Update QueueStatusUpdate structure **Acceptance Criteria:** - ✅ types.ts matches frontend expectations - ✅ QueueManager creates items with phases array - ✅ Phase progress updates correctly through pipeline - ✅ Results wrapped in results object - ✅ Tandoor URL constructed correctly - ✅ Both legacy and new properties populated (transition period) - ✅ All QueueManager tests still passing **Files:** - `src/lib/server/queue/types.ts` (update) - `src/lib/server/queue/QueueManager.ts` (update) - `src/tests/queue-manager.spec.ts` (update tests) --- ### Story 1: Delete Deprecated Code **Priority:** HIGH (DO SECOND) **Dependencies:** Story 0 **Objective:** Remove all deprecated/dead code from the queue migration. **Tasks:** 1. DELETE `src/routes/api/extract-stream/+server.ts` entirely 2. DELETE `src/routes/share/+page.svelte.old` 3. Review share page components for usage 4. Move reusable components to `src/lib/components/` if used by both share and queue 5. Delete any unused component imports 6. Update any documentation referencing old endpoints 7. Verify no broken imports **Acceptance Criteria:** - ✅ `/api/extract-stream` endpoint completely removed - ✅ `.old` backup file deleted - ✅ No import errors - ✅ No references to deleted files - ✅ Shared components moved to `src/lib/components/` - ✅ Documentation updated - ✅ All tests still passing **Files:** - `src/routes/api/extract-stream/+server.ts` (DELETE) - `src/routes/share/+page.svelte.old` (DELETE) - `src/routes/share/components/*` (review and possibly move) - `docs/MIGRATION.md` (update if exists) --- ### Story 2: Fix Type Definitions and Update QueueManager **Priority:** CRITICAL **Dependencies:** None **Objective:** Implement DELETE /api/queue/:id endpoint to allow removing queue items. **Tasks:** 1. Add DELETE handler to `src/routes/api/queue/[id]/+server.ts` 2. Validate ID format 3. Check item exists 4. Prevent deletion of in-progress items 5. Call queueManager.remove() 6. Return success response 7. Write tests **Acceptance Criteria:** - ✅ DELETE endpoint responds correctly - ✅ Validates ID format - ✅ Returns 404 for non-existent items - ✅ Returns 409 for in-progress items - ✅ Successfully removes items - ✅ Broadcasts removal via SSE - ✅ All tests passing **Files:** - `src/routes/api/queue/[id]/+server.ts` (add DELETE handler) - `src/tests/queue-api.spec.ts` (add tests) --- ### Story 3: Add DELETE Endpoint **Priority:** HIGH **Dependencies:** Story 2 **Objective:** Update frontend to call DELETE endpoint instead of commenting it out. **Tasks:** 1. Update removeItem() function in +page.svelte 2. Call DELETE endpoint 3. Handle errors gracefully 4. Rely on SSE for state update **Acceptance Criteria:** - ✅ Remove button calls DELETE endpoint - ✅ Shows error message on failure - ✅ UI updates via SSE - ✅ Fallback removes from local state **Files:** - `src/routes/+page.svelte` (update) --- ### Story 4: Fix Frontend Remove Functionality **Priority:** HIGH **Dependencies:** Story 1 **Objective:** Fix failing API tests by properly handling async error responses. **Tasks:** 1. Update all failing test assertions 2. Properly await response.json() 3. Add try-catch for JSON parsing errors 4. Verify correct error status codes 5. Run full test suite **Acceptance Criteria:** - ✅ All 17 queue API tests passing - ✅ Error assertions handle async correctly - ✅ No more stderr console noise in tests - ✅ Test coverage comprehensive **Files:** - `src/tests/queue-api.spec.ts` (update) --- ### Story 5: Fix Test Assertions and Add Mocking Documentation **Priority:** HIGH **Dependencies:** Story 0, Story 2 **Objective:** Fix failing API tests by properly handling async error responses AND add comprehensive mocking documentation. **Tasks:** 1. Create `docs/TESTING.md` with Vitest mocking guide (use content from this plan) 2. Update all failing test assertions to properly handle async 3. Properly await response.json() in error cases 4. Add try-catch for JSON parsing errors 5. Verify correct error status codes 6. Add examples of proper mocking to test files 7. Run full test suite and verify 100% pass rate **Acceptance Criteria:** - ✅ `docs/TESTING.md` created with comprehensive mocking guide - ✅ All 17 queue API tests passing - ✅ Error assertions handle async correctly - ✅ No more stderr console noise in tests - ✅ Test files include comments showing proper mocking patterns - ✅ Developers can reference TESTING.md for examples - ✅ Test coverage comprehensive **Files:** - `docs/TESTING.md` (new - comprehensive mocking guide) - `src/tests/queue-api.spec.ts` (update) - `src/tests/queue-processor.spec.ts` (add mocking examples in comments) - `src/tests/queue-manager.spec.ts` (add examples) - `README.md` (add link to TESTING.md) --- ### Story 6: Update SSE Stream to Send Full Updates **Priority:** MEDIUM **Dependencies:** Story 1 **Objective:** Ensure SSE stream sends complete update objects matching frontend expectations. **Tasks:** 1. Update stream endpoint to include progress array 2. Include results object in updates 3. Send type field in updates 4. Test real-time updates **Acceptance Criteria:** - ✅ SSE updates include progress array - ✅ SSE updates include results object - ✅ Frontend receives and displays updates correctly - ✅ Progress bars update in real-time - ✅ Results section populates correctly **Files:** - `src/routes/api/queue/stream/+server.ts` (verify, minor updates if needed) - QueueManager already updated in Story 1 --- ### Story 7: Complete Web Push Implementation **Priority:** LOW (Nice to have) **Dependencies:** Story 1-5 complete **Objective:** Fully implement Web Push notifications for queue completions. **Tasks:** 1. Create /api/push/subscribe endpoint 2. Create /api/push/unsubscribe endpoint 3. Store subscriptions in PushNotificationService 4. Implement actual push sending (not just logging) 5. Update service worker to handle notifications 6. Add notification click handler 7. Update NotificationSettings component 8. Add permission request flow **Acceptance Criteria:** - ✅ Users can subscribe to notifications - ✅ Notifications sent on success/error - ✅ Clicking notification opens app - ✅ Notifications include recipe name - ✅ Works when app not in focus - ✅ Graceful degradation if permission denied **Files:** - `src/routes/api/push/subscribe/+server.ts` (new) - `src/routes/api/push/unsubscribe/+server.ts` (new) - `src/lib/server/notifications/PushNotificationService.ts` (update) - `src/service-worker.ts` (update) - `src/routes/components/NotificationSettings.svelte` (update) --- ### Story 8: Add Auto-Cleanup for Success Items **Priority:** LOW (Enhancement) **Dependencies:** Story 1-5 complete **Objective:** Automatically remove successful items after a configurable time period. **Tasks:** 1. Add AUTO_REMOVE_SUCCESS env var (default: 3600000ms = 1 hour) 2. Add cleanup scheduler to QueueManager 3. Run cleanup every 5 minutes 4. Remove success items older than threshold 5. Log removals **Acceptance Criteria:** - ✅ Success items removed after threshold - ✅ Configurable via env var - ✅ Runs in background - ✅ Doesn't interfere with processing - ✅ Broadcasts removals via SSE **Files:** - `src/lib/server/queue/QueueManager.ts` (add cleanup scheduler) --- ## Testing Strategy ### Unit Tests Updates ```typescript // queue-manager.spec.ts - Add new tests describe('Phase Progress', () => { it('should initialize phases array on enqueue'); it('should update phase status when processing starts'); it('should mark phases as completed in order'); it('should mark phase as error on failure'); }); describe('Results Wrapper', () => { it('should wrap extracted text in results'); it('should wrap thumbnail in results'); it('should wrap recipe in results'); it('should construct Tandoor URL from ID'); }); // queue-api.spec.ts - Add DELETE tests describe('DELETE /api/queue/[id]', () => { it('should delete queue item'); it('should return 404 for non-existent item'); it('should return 409 for in-progress item'); it('should validate ID format'); }); ``` ### Integration Tests ```typescript // Test full pipeline with new types it('should populate phases and results through full pipeline'); it('should send SSE updates with progress and results'); it('should construct Tandoor URL correctly'); ``` ### Manual Testing Checklist - [ ] Share URL → See pending in queue - [ ] Watch phases update in real-time - [ ] See progress bar advance - [ ] Success shows results with recipe - [ ] Tandoor URL clickable and correct - [ ] Can remove completed items - [ ] Cannot remove in-progress items - [ ] Retry failed items works - [ ] SSE reconnects on disconnect --- ## Deployment Checklist ### Pre-Deployment - [ ] All tests passing (100% pass rate) - [ ] No TypeScript errors - [ ] No console errors in dev mode - [ ] Manual testing complete - [ ] Performance acceptable (<100ms queue operations) ### Deployment - [ ] Deploy backend changes first - [ ] Verify SSE stream working - [ ] Deploy frontend changes - [ ] Monitor error rates - [ ] Check queue processing ### Post-Deployment - [ ] Monitor for type errors - [ ] Verify real-time updates working - [ ] Check Tandoor URLs correct - [ ] Verify remove functionality - [ ] Monitor memory usage --- ## Risk Assessment ### Critical Risks **Type Mismatch Breaking Production** (HIGH) - *Mitigation:* Keep both legacy and new properties during transition - *Rollback:* Can quickly revert frontend to use legacy properties **Data Loss During Migration** (MEDIUM) - *Mitigation:* In-memory queue, no persistent data at risk - *Impact:* Only affects current queue items, no historical data ### Medium Risks **Test Failures Block Deployment** (MEDIUM) - *Mitigation:* Fix tests first before type changes - *Timeline:* 1-2 hours to fix all test assertions **SSE Disconnect During Update** (LOW) - *Mitigation:* Auto-reconnect already implemented - *Impact:* Brief interruption, recovers automatically --- ## Success Metrics | Metric | Current | Target | |--------|---------|--------| | Test Pass Rate | 47/51 (92%) | 51/51 (100%) | | Type Safety | TypeScript errors | Zero errors | | Remove Functionality | Not working | Fully functional | | SSE Update Completeness | Partial | Complete (phases + results) | | Frontend Errors | Runtime errors likely | Zero errors | --- ## Documentation Requirements **Code Changes:** - Update types.ts with comprehensive JSDoc - Document phase lifecycle in QueueManager - Document results structure - Add DELETE endpoint to API docs **README Updates:** - Document queue item structure - Explain phase progress tracking - Show example SSE update payloads - Document DELETE endpoint --- ## Implementation Priority **⚠️ CRITICAL: Work on CURRENT BRANCH - Do NOT create a new branch** ### Phase 1: Critical Fixes (Deploy First) 1. **Story 0:** Fix Environment Variables (2 hours) - DO THIS FIRST 2. **Story 1:** Delete Deprecated Code (1 hour) 3. **Story 2:** Fix Type Definitions (4 hours) 4. **Story 3:** Add DELETE Endpoint (2 hours) 5. **Story 5:** Fix Test Assertions + Add Mocking Docs (3 hours) 6. **Story 4:** Fix Frontend Remove (1 hour) **Total: 13 hours** ### Phase 2: Enhancements (Deploy Later) 7. **Story 6:** Update SSE Stream (1 hour) 8. **Story 7:** Web Push Notifications (6 hours) 9. **Story 8:** Auto-Cleanup (2 hours) **Total: 9 hours** --- ## Branch Strategy **DO NOT CREATE A NEW BRANCH** All work will be done on the current branch. This is a continuation of the AsyncInMemoryProcessingQueue implementation, fixing issues discovered during review. **Git Workflow:** ```bash # You're already on the correct branch # Just commit as you complete each story git add . git commit -m "Story 0: Fix environment variables - use SvelteKit $env" # Continue with Story 1, 2, etc. ``` --- ## Acceptance Criteria for Complete Feature - ✅ Zero TypeScript errors - ✅ 100% test pass rate - ✅ Frontend displays phases progress correctly - ✅ Frontend displays results correctly - ✅ Can remove queue items - ✅ SSE updates include all necessary data - ✅ Tandoor URLs work correctly - ✅ No runtime errors in console - ✅ All original plan stories completed - ✅ Web Push notifications functional (optional) --- ## Notes The AsyncInMemoryProcessingQueue feature is **85% complete** with excellent architecture and implementation. The issues are primarily: - Type definition mismatches between frontend and backend - Missing DELETE endpoint - Test assertion handling These are straightforward fixes that don't require architectural changes. The core queue system works well and the three-phase processing pipeline is solid. **Recommended Action:** Implement Phase 1 critical fixes immediately, then Phase 2 enhancements as time permits. **Estimated Total Time:** 18 hours (9 hours critical + 9 hours enhancements)