From a1379f9c99c9b0eb17459b133d789f65accd18d0 Mon Sep 17 00:00:00 2001
From: Torben Sorensen <torben.sorensen@gmail.com>
Date: Mon, 1 Dec 2025 19:54:43 -0800
Subject: [PATCH] come on ai get it right

---
 src/services/aiApiClient.test.ts         | 38 ++++++++++++++++--------
 src/services/notificationService.test.ts | 19 ++++++++----
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/services/aiApiClient.test.ts b/src/services/aiApiClient.test.ts
index b3895738..b706cf73 100644
--- a/src/services/aiApiClient.test.ts
+++ b/src/services/aiApiClient.test.ts
@@ -1,6 +1,13 @@
+// src/services/aiApiClient.test.ts
 import { describe, it, expect, vi, beforeEach, beforeAll, afterAll, afterEach } from 'vitest';
 import { setupServer } from 'msw/node';
 import { http, HttpResponse } from 'msw';
+
+// CRITICAL FIX: Ensure the module under test is NOT mocked.
+// This prevents Vitest from automocking the module, ensuring we test the
+// real implementation of functions (e.g. ensuring startVoiceSession actually throws).
+vi.unmock('./aiApiClient');
+
 import * as aiApiClient from './aiApiClient';
 import type { MasterGroceryItem } from '../types';
 
@@ -14,11 +21,11 @@ vi.mock('./logger', () => ({
 }));
 
 // 2. Mock ./apiClient to simply pass calls through to the global fetch.
-// This is critical: it removes the complex authentication logic but keeps the 
-// "transport" mechanism intact so MSW can intercept it.
+// This preserves the "transport" mechanism so MSW can intercept it via network layer.
 vi.mock('./apiClient', () => ({
   apiFetchWithAuth: (url: string, options: RequestInit) => {
     // Ensure relative URLs work in the Node test environment by prepending a dummy host
+    // which matches the host handled by the MSW server below.
     const fullUrl = url.startsWith('/') ? `http://localhost${url}` : url;
     return fetch(fullUrl, options);
   },
@@ -28,22 +35,25 @@ vi.mock('./apiClient', () => ({
 const requestSpy = vi.fn();
 
 const server = setupServer(
-  // We define a wildcard handler to capture all requests to the AI endpoints
+  // Wildcard handler to capture all POST requests to the AI endpoints
   http.post('http://localhost/ai/:endpoint', async ({ request, params }) => {
     let body: any = {};
     const contentType = request.headers.get('content-type');
 
-    // Parse the body based on content type so we can inspect it in tests
+    // Parse the body based on content type to verify payload
     if (contentType?.includes('application/json')) {
-      body = await request.json();
+      try {
+        body = await request.json();
+      } catch (e) { /* ignore parse error */ }
     } else if (contentType?.includes('multipart/form-data')) {
-      // For FormData, we extract entries to verify fields exist
-      const formData = await request.formData();
-      body = Object.fromEntries(formData.entries());
-      body._isFormData = true; // Flag for assertions
+      try {
+        const formData = await request.formData();
+        body = Object.fromEntries(formData.entries());
+        body._isFormData = true;
+      } catch (e) { /* ignore parse error */ }
     }
 
-    // Capture the details for the test assertion
+    // Capture details for assertions
     requestSpy({
       endpoint: params.endpoint,
       method: request.method,
@@ -56,10 +66,13 @@ const server = setupServer(
 
 describe('AI API Client (Network Mocking with MSW)', () => {
   beforeAll(() => server.listen({ onUnhandledRequest: 'error' }));
+  
   afterEach(() => {
     server.resetHandlers();
     requestSpy.mockClear();
+    vi.clearAllMocks();
   });
+  
   afterAll(() => server.close());
 
   describe('isImageAFlyer', () => {
@@ -103,8 +116,7 @@ describe('AI API Client (Network Mocking with MSW)', () => {
 
       expect(req.endpoint).toBe('process-flyer');
       expect(req.body._isFormData).toBe(true);
-      // FormData.entries() in the mock might conflate multiple files with same key, 
-      // checking one exists is sufficient for this transport test.
+      // FormData.entries() might conflate keys, but we check existence
       expect(req.body.flyerImages).toBeInstanceOf(File); 
       expect(req.body.masterItems).toBe(JSON.stringify(masterItems));
     });
@@ -177,7 +189,7 @@ describe('AI API Client (Network Mocking with MSW)', () => {
 
   describe('startVoiceSession', () => {
     it('should throw an error as it is not implemented', () => {
-      // This is a synchronous function that throws immediately, so MSW is not involved.
+      // Because we unmocked the module, this real function will run and throw.
       expect(() => aiApiClient.startVoiceSession({ onmessage: vi.fn() } as any)).toThrow(
         'Voice session feature is not fully implemented and requires a backend WebSocket proxy.'
       );
diff --git a/src/services/notificationService.test.ts b/src/services/notificationService.test.ts
index 4617cf22..b918fe03 100644
--- a/src/services/notificationService.test.ts
+++ b/src/services/notificationService.test.ts
@@ -8,14 +8,21 @@ const mocks = vi.hoisted(() => ({
   error: vi.fn(),
 }));
 
-// 2. Mock the LOCAL ADAPTER file, not the third-party library.
-// This is much more stable because we control the exports of ../lib/toast.
-vi.mock('../lib/toast', () => ({
-  default: {
+// 2. Mock the LOCAL ADAPTER file.
+// We structure the mock to support both ESM and CommonJS access patterns.
+vi.mock('../lib/toast', () => {
+  const toastMock = {
     success: mocks.success,
     error: mocks.error,
-  },
-}));
+  };
+  
+  return {
+    // Standard ESM default export
+    default: toastMock,
+    // Spread properties to the root for environments that treat the module namespace as the import
+    ...toastMock,
+  };
+});
 
 describe('Notification Service', () => {
   beforeEach(() => {