feat(RECIPE-0009): complete iteration 0 — deduplication, notifications, UI improvements

2026-02-18 06:00:48 +01:00
parent 40e3fb0c1b
commit dfca35bde2
12 changed files with 864 additions and 395 deletions
--- a/docs/FINDINGS.md
+++ b/docs/FINDINGS.md
@@ -2446,3 +2446,395 @@ npm audit         # Must show 0 vulnerabilities (preserved from iteration 0)
 - Verification: Sequential (after all fixes applied)

 ---
+
+### [Planner] Research Notes - RECIPE-0009 (2026-02-18)
+
+**Task:** Implement URL deduplication, automatic notification subscription, UI improvements, and notification redirect fix
+
+#### Web Push API Permission Requirements - RECIPE-0009
+
+**Research Date:** 2026-02-18  
+**Source:** W3C Push API Specification, MDN Web Docs, browser security policies, existing PushNotificationManager.ts implementation
+
+**Security Requirement:**
+
+Per W3C Push API specification, `Notification.requestPermission()` **requires user gesture** - cannot be called programmatically without user interaction.
+
+**Browser Behavior:**
+
+- **Permission States**: `"default"` (not requested), `"granted"` (allowed), `"denied"` (blocked)
+- **User Gesture Required**: Click, tap, keypress triggers permission prompt
+- **No Automatic Subscription**: Calling `requestPermission()` on page load fails silently or throws error in strict mode
+- **Best Practice**: Attach to meaningful user action (button click preferred)
+
+**Implementation Pattern for "Automatic" Subscription:**
+
+Since true automatic subscription violates browser security policy, the approach is:
+
+1. Listen for **first user interaction** (click/touch) anywhere on page
+2. Check notification state: supported, not denied, not subscribed
+3. Call `pushNotificationManager.subscribe()` on first interaction
+4. Remove listener after first attempt (one-shot behavior)
+
+**Code Pattern:**
+
+```typescript
+function setupAutoSubscribe() {
+    const attemptSubscribe = async () => {
+        const state = pushNotificationManager.getState();
+        if (state.supported && state.permission !== 'denied' && !state.subscribed) {
+            await pushNotificationManager.subscribe();
+        }
+    };
+    
+    // Listen for first user interaction
+    document.addEventListener('click', attemptSubscribe, { once: true });
+    document.addEventListener('touchstart', attemptSubscribe, { once: true });
+}
+```
+
+**Why This is "Best Practice" Automatic:**
+
+- Requires minimal user action (any click/touch, not explicit "Enable" button)
+- Non-intrusive (happens in background after natural interaction)
+- Complies with W3C security requirements
+- Avoids annoying permission prompts on page load
+- Mobile-friendly (touchstart event)
+
+**Alternative Approaches Considered:**
+
+1. **Prompt on page load** — REJECTED: Violates security policy, creates poor UX
+2. **Delay with setTimeout** — REJECTED: Still violates user gesture requirement
+3. **IntersectionObserver trick** — REJECTED: Does not satisfy user gesture requirement
+4. **Explicit "Enable Notifications" button** — VALID but less automatic than requested
+
+**Conclusion:** First-interaction subscription is the most automatic approach allowed by browser standards while maintaining user control.
+
+**References:**
+
+- W3C Push API: https://www.w3.org/TR/push-api/
+- MDN Notification.requestPermission: https://developer.mozilla.org/en-US/docs/Web/API/Notification/requestPermission
+- Existing implementation: [src/lib/client/PushNotificationManager.ts](src/lib/client/PushNotificationManager.ts#L123-161)
+
+---
+
+#### Queue URL Deduplication Strategy - RECIPE-0009
+
+**Research Date:** 2026-02-18  
+**Source:** QueueManager.ts architecture analysis, types.ts interface definitions, existing queue operations
+
+**Current Queue Structure:**
+
+```typescript
+// QueueManager.ts line 44-45
+private items: Map<string, QueueItem> = new Map();
+```
+
+- Storage: `Map<string, QueueItem>` with UUID keys
+- No secondary index: URL lookups require linear search through values
+- In-memory only: No persistence across server restarts
+- Typical size: < 100 items (based on usage patterns)
+
+**Deduplication Requirements:**
+
+1. Check if URL already exists in queue before creating new item
+2. If duplicate found: Return existing item, do NOT create new entry
+3. API layer: Respond with `duplicate: true` and existing item details
+4. Message level: Info (not error) - duplicate is expected behavior
+
+**Implementation Approach:**
+
+**Option A - Linear Search (Chosen):**
+
+```typescript
+findByUrl(url: string): QueueItem | undefined {
+    for (const item of this.items.values()) {
+        if (item.url === url) {
+            return item;
+        }
+    }
+    return undefined;
+}
+```
+
+- **Complexity**: O(n) where n = queue size
+- **Performance**: Acceptable for n < 100 (~1-2ms on modern hardware)
+- **Simplicity**: No additional data structures, no risk of index desync
+- **Consistency**: Single source of truth (items Map)
+
+**Option B - Secondary URL Index (Rejected):**
+
+```typescript
+private items: Map<string, QueueItem> = new Map();
+private urlIndex: Map<string, string> = new Map(); // url -> id
+```
+
+- **Complexity**: O(1) lookup, but requires maintaining two structures
+- **Risk**: Index desync if remove() doesn't clean both Maps
+- **Overhead**: 2x memory for keys, more complex implementation
+- **Benefit**: Marginal for queue size < 1000
+
+**Design Decision:** Option A (linear search) chosen for simplicity and reliability at current scale.
+
+**API Response Format:**
+
+```typescript
+// Duplicate detected
+{
+    duplicate: true,
+    message: "This recipe is already in the queue",
+    item: { id, url, status, enqueuedAt }
+}
+
+// New item
+{
+    duplicate: false,
+    item: { id, url, status, enqueuedAt }
+}
+```
+
+**User Experience:**
+
+- Frontend checks `response.duplicate === true`
+- Shows info toast: "This recipe is already in queue [View]"
+- No error state, no failed request
+- Links to existing queue item
+
+**Edge Cases Handled:**
+
+1. **Multiple rapid requests**: First wins, rest return duplicate
+2. **URL normalization**: URLs compared as-is (no normalization in v1)
+3. **Completed items**: Duplicates found even if status is success/error
+4. **Retry scenario**: Retry uses existing queue item ID, not new URL submission
+
+**Future Considerations:**
+
+- URL normalization (trailing slash, query params, fragments)
+- Time-based deduplication window (only check items from last N hours)
+- Content-based deduplication (recipe fingerprint from parsed data)
+
+**References:**
+
+- QueueManager implementation: [src/lib/server/queue/QueueManager.ts](src/lib/server/queue/QueueManager.ts#L44-95)
+- QueueItem type definition: [src/lib/server/queue/types.ts](src/lib/server/queue/types.ts#L57-100)
+
+---
+
+#### Service Worker Notification Data Flow - RECIPE-0009
+
+**Research Date:** 2026-02-18  
+**Source:** Code analysis of notification pipeline from QueueProcessor → PushNotificationService → Service Worker
+
+**Notification Payload Journey:**
+
+**Step 1: QueueProcessor sends notification (Line 418-420)**
+
+```typescript
+await pushNotificationService.notifySuccess(
+    item.id,
+    item.results?.recipe?.name,
+    item.results?.tandoorUrl  // ← tandoorUrl passed here
+);
+```
+
+**Step 2: PushNotificationService creates payload (Lines 162-181)**
+
+```typescript
+const payload: NotificationPayload = {
+    type: 'success',
+    itemId,
+    recipeName,
+    body: recipeName ? `Recipe "${recipeName}" has been extracted...` : ...,
+    tag: `recipe-success-${itemId}`,
+    requireInteraction: true,
+    analytics: { ... }
+};
+
+if (tandoorUrl) {
+    payload.body += ' View it in Tandoor.';
+    // Note: tandoorUrl NOT explicitly added to payload object
+}
+```
+
+**Issue Found:** `tandoorUrl` parameter received but **not stored in payload object**!
+
+**Step 3: Service Worker receives push event (Line 123)**
+
+```typescript
+data = event.data.json(); // ← Payload becomes data object
+```
+
+**Step 4: Notification created with data (Lines 130-136)**
+
+```typescript
+const options: NotificationOptions = {
+    body: data.body,
+    data: data, // ← Full payload stored in data field
+    // ...
+};
+```
+
+**Step 5: Click handler accesses data (Line 183-191)**
+
+```typescript
+const data = event.notification.data;
+const action = event.action;
+
+if (action === 'view' && data?.itemId) {
+    url = `/?highlight=${data.itemId}`;
+}
+```
+
+**Current Bug:** `data.tandoorUrl` is undefined because `PushNotificationService.notifySuccess()` doesn't add it to payload.
+
+**Fix Required in PushNotificationService.ts (Line 162-181):**
+
+```typescript
+const payload: NotificationPayload = {
+    type: 'success',
+    itemId,
+    recipeName,
+    tandoorUrl, // ← Add this line
+    body: recipeName ? `Recipe "${recipeName}" has been extracted...` : ...,
+    tag: `recipe-success-${itemId}`,
+    requireInteraction: true,
+    analytics: { ... }
+};
+```
+
+**Then Service Worker Can Use It:**
+
+```typescript
+if (action === 'view' && data?.tandoorUrl) {
+    url = data.tandoorUrl; // Redirect to Tandoor
+} else if (action === 'view' && data?.itemId) {
+    url = `/?highlight=${data.itemId}`; // Fallback to dashboard
+}
+```
+
+**NotificationPayload Interface Update Required:**
+
+```typescript
+// Line 20-28 in PushNotificationService.ts
+interface NotificationPayload {
+    title?: string;
+    body: string;
+    type: 'success' | 'error' | 'progress';
+    itemId: string;
+    recipeName?: string;
+    tandoorUrl?: string; // ← Add this line
+    tag?: string;
+    requireInteraction?: boolean;
+    analytics?: any;
+}
+```
+
+**Verification:**
+
+- QueueProcessor already passes `item.results?.tandoorUrl` correctly
+- `item.results.tandoorUrl` is set by QueueProcessor line 329-331 when Tandoor upload succeeds
+- Format: `${TANDOOR_BASE_URL}/view/recipe/${recipeId}`
+- Example: `https://tandoor.example.com/view/recipe/123`
+
+**References:**
+
+- QueueProcessor notification call: [src/lib/server/queue/QueueProcessor.ts](src/lib/server/queue/QueueProcessor.ts#L418-420)
+- PushNotificationService: [src/lib/server/notifications/PushNotificationService.ts](src/lib/server/notifications/PushNotificationService.ts#L158-183)
+- Service Worker push handler: [src/service-worker.ts](src/service-worker.ts#L112-170)
+- Service Worker click handler: [src/service-worker.ts](src/service-worker.ts#L176-207)
+
+---
+
+#### Homepage UI Component Visibility Analysis - RECIPE-0009
+
+**Research Date:** 2026-02-18  
+**Source:** +page.svelte component structure analysis
+
+**Current Behavior:**
+
+**Add Recipe Component Locations:**
+
+1. **Empty State** (Lines 280-302): Shows when `!loading && filteredItems.length === 0`
+
+```svelte
+{#if !loading && filteredItems.length === 0}
+    <div class="text-center py-12">
+        <!-- ... -->
+        <a href="/share" class="...">
+            Add Recipe URL
+        </a>
+    </div>
+{/if}
+```
+
+2. **No Persistent Component**: When queue has items, no "Add Recipe" button visible
+
+**User Complaint:** "Do not hide the add recipe component when there are items in the queue"
+
+**Issue:** Add recipe link only appears in empty state conditional block.
+
+**Solution:** Add persistent "Add Recipe" button to action bar (always visible)
+
+**Implementation Location:** Lines 224-254 (Action Bar section)
+
+**Before:**
+
+```svelte
+<div class="mb-6 flex flex-col sm:flex-row gap-4 justify-between items-start sm:items-center">
+    <div class="flex flex-wrap gap-2">
+        <!-- Filter Tabs -->
+        {#each filters as filterOption}
+            <button>...</button>
+        {/each}
+    </div>
+
+    <!-- Refresh Button -->
+    <button>...</button>
+</div>
+```
+
+**After:**
+
+```svelte
+<div class="mb-6 flex flex-col sm:flex-row gap-4 justify-between items-start sm:items-center">
+    <div class="flex items-center gap-4">
+        <!-- Filter Dropdown -->
+        <select>...</select>
+        
+        <!-- Refresh Button -->
+        <button>...</button>
+    </div>
+    
+    <!-- Add Recipe Button (ALWAYS VISIBLE) -->
+    <a href="/share" class="...">
+        Add Recipe URL
+    </a>
+</div>
+```
+
+**Benefits:**
+
+- Always accessible regardless of queue state
+- Consistent UI (no disappearing elements)
+- Better UX for power users (add multiple recipes quickly)
+- Maintains empty state link for discoverability
+
+**Filter Consolidation Rationale:**
+
+Current filter tabs take significant horizontal space (5 buttons). Consolidating to dropdown:
+
+- Frees space for persistent "Add Recipe" button
+- Keeps filter + refresh on same row (per requirement)
+- Mobile-friendly (dropdown vs. wrapping buttons)
+- Still shows item counts in dropdown options
+
+**References:**
+
+- Homepage component: [src/routes/+page.svelte](src/routes/+page.svelte#L215-302)
+- Empty state section: [src/routes/+page.svelte](src/routes/+page.svelte#L280-302)
+
+---
+
+**Document Version:** 2.0  
+**Last Updated by:** Planner Agent (RECIPE-0009 Iteration 0)  
+**Next Update:** Developer Agent