unit test fixes

src/App.test.tsx

@@ -506,11 +506,8 @@ describe('App Component', () => {
                 isLoading: false, login: vi.fn(), logout: vi.fn(), updateProfile: vi.fn(),
             });
     
-            const { rerender } = render(
-                <MemoryRouter initialEntries={['/admin/corrections']}>
-                    <App />
-                </MemoryRouter>
-            );
+            console.log('Testing admin sub-routes with renderApp wrapper to ensure ModalProvider context');
+            renderApp(['/admin/corrections']);
     
             await waitFor(() => {
                 expect(screen.getByTestId('corrections-page-mock')).toBeInTheDocument();

src/components/FlyerCorrectionTool.test.tsx

@@ -199,35 +199,31 @@ describe('FlyerCorrectionTool', () => {
     const extractButton = screen.getByRole('button', { name: /extract store name/i });
 
     expect(extractButton).toBeDisabled();
-    // Although disabled, let's simulate a click for robustness
-    fireEvent.click(extractButton);
-
-    // The component has an internal check, let's ensure it works
-    // We can enable the button by making a selection, then clearing it to test the guard
-    const canvas = screen.getByRole('dialog').querySelector('canvas')!;
-    fireEvent.mouseDown(canvas, { clientX: 10, clientY: 10 });
-    fireEvent.mouseUp(canvas); // No move, so selection is tiny/invalid for some logic
 
     // To properly test the guard, we need to bypass the disabled state
+    // We force the button to be enabled to trigger the click handler
     extractButton.removeAttribute('disabled');
     fireEvent.click(extractButton);
 
-    // This is hard to test directly without changing component code.
-    // A better test is to ensure the button is disabled initially.
-    // Let's add a test for the explicit guard.
-    const { rerender } = render(<FlyerCorrectionTool {...defaultProps} />);
-    const button = screen.getByRole('button', { name: /extract store name/i });
-    fireEvent.click(button);
     expect(mockedNotifyError).toHaveBeenCalledWith('Please select an area on the image first.');
   });
 
   it('should handle non-standard API errors during rescan', async () => {
     mockedAiApiClient.rescanImageArea.mockRejectedValue('A plain string error');
     render(<FlyerCorrectionTool {...defaultProps} />);
+
+    // Wait for image fetch to ensure imageFile is set before we interact
+    await waitFor(() => expect(global.fetch).toHaveBeenCalled());
+    // Allow the promise chain in useEffect to complete
+    await act(async () => {
+      await new Promise(resolve => setTimeout(resolve, 0));
+    });
+
     const canvas = screen.getByRole('dialog').querySelector('canvas')!;
     fireEvent.mouseDown(canvas, { clientX: 10, clientY: 10 });
     fireEvent.mouseMove(canvas, { clientX: 100, clientY: 50 });
     fireEvent.mouseUp(canvas);
+    
     fireEvent.click(screen.getByRole('button', { name: /extract store name/i }));
     await waitFor(() => {
       expect(mockedNotifyError).toHaveBeenCalledWith('An unknown error occurred.');

src/features/charts/PriceHistoryChart.test.tsx

@@ -24,7 +24,11 @@ vi.mock('../../services/logger', () => ({
 // Mock the recharts library to prevent rendering complex SVGs in jsdom
 vi.mock('recharts', () => ({
   ResponsiveContainer: ({ children }: { children: React.ReactNode }) => <div data-testid="responsive-container">{children}</div>,
-  LineChart: ({ children }: { children: React.ReactNode }) => <div data-testid="line-chart">{children}</div>,
+  // Expose the data prop for testing data transformations
+  LineChart: ({ children, data }: { children: React.ReactNode; data: any[] }) => (
+    <div data-testid="line-chart" data-chartdata={JSON.stringify(data)}>
+      {children}
+    </div>),
   CartesianGrid: () => <div data-testid="cartesian-grid" />,
   XAxis: () => <div data-testid="x-axis" />,
   YAxis: () => <div data-testid="y-axis" />,
@@ -118,4 +122,102 @@ describe('PriceHistoryChart', () => {
       expect(screen.getByTestId('line-Almond Milk')).toBeInTheDocument();
     });
   });
+
+  it('should display a loading state while user data is loading', () => {
+    mockedUseUserData.mockReturnValue({
+      watchedItems: [],
+      shoppingLists: [],
+      setWatchedItems: vi.fn(),
+      setShoppingLists: vi.fn(),
+      isLoading: true, // Test the isLoading state from the useUserData hook
+      error: null,
+    });
+    vi.mocked(apiClient.fetchHistoricalPriceData).mockReturnValue(new Promise(() => {}));
+    render(<PriceHistoryChart />);
+    expect(screen.getByText('Loading Price History...')).toBeInTheDocument();
+  });
+
+  it('should clear the chart when the watchlist becomes empty', async () => {
+    vi.mocked(apiClient.fetchHistoricalPriceData).mockResolvedValue(new Response(JSON.stringify(mockPriceHistory)));
+    const { rerender } = render(<PriceHistoryChart />);
+
+    // Initial render with items
+    await waitFor(() => {
+      expect(screen.getByTestId('line-Organic Bananas')).toBeInTheDocument();
+    });
+
+    // Rerender with empty watchlist
+    mockedUseUserData.mockReturnValue({
+      watchedItems: [],
+      shoppingLists: [],
+      setWatchedItems: vi.fn(),
+      setShoppingLists: vi.fn(),
+      isLoading: false,
+      error: null,
+    });
+    rerender(<PriceHistoryChart />);
+
+    // Chart should be gone, placeholder should appear
+    await waitFor(() => {
+      expect(screen.getByText('Add items to your watchlist to see their price trends over time.')).toBeInTheDocument();
+      expect(screen.queryByTestId('line-chart')).not.toBeInTheDocument();
+    });
+  });
+
+  it('should filter out items with only one data point', async () => {
+    const dataWithSinglePoint: HistoricalPriceDataPoint[] = [
+      { master_item_id: 1, summary_date: '2024-10-01', avg_price_in_cents: 110 },
+      { master_item_id: 1, summary_date: '2024-10-08', avg_price_in_cents: 99 },
+      { master_item_id: 2, summary_date: '2024-10-01', avg_price_in_cents: 350 }, // Almond Milk only has one point
+    ];
+    vi.mocked(apiClient.fetchHistoricalPriceData).mockResolvedValue(new Response(JSON.stringify(dataWithSinglePoint)));
+    render(<PriceHistoryChart />);
+
+    await waitFor(() => {
+      expect(screen.getByTestId('line-Organic Bananas')).toBeInTheDocument();
+      expect(screen.queryByTestId('line-Almond Milk')).not.toBeInTheDocument();
+    });
+  });
+
+  it('should process data to only keep the lowest price for a given day', async () => {
+    const dataWithDuplicateDate: HistoricalPriceDataPoint[] = [
+      { master_item_id: 1, summary_date: '2024-10-01', avg_price_in_cents: 110 },
+      { master_item_id: 1, summary_date: '2024-10-01', avg_price_in_cents: 105 }, // Lower price
+      { master_item_id: 1, summary_date: '2024-10-08', avg_price_in_cents: 99 },
+    ];
+    vi.mocked(apiClient.fetchHistoricalPriceData).mockResolvedValue(new Response(JSON.stringify(dataWithDuplicateDate)));
+    render(<PriceHistoryChart />);
+
+    await waitFor(() => {
+      const chart = screen.getByTestId('line-chart');
+      const chartData = JSON.parse(chart.getAttribute('data-chartdata')!);
+      
+      // The date gets formatted to 'Oct 1'
+      const dataPointForOct1 = chartData.find((d: any) => d.date === 'Oct 1');
+      
+      expect(dataPointForOct1['Organic Bananas']).toBe(105);
+    });
+  });
+
+  it('should filter out data points with a price of zero', async () => {
+    const dataWithZeroPrice: HistoricalPriceDataPoint[] = [
+      { master_item_id: 1, summary_date: '2024-10-01', avg_price_in_cents: 110 },
+      { master_item_id: 1, summary_date: '2024-10-08', avg_price_in_cents: 0 }, // Zero price should be filtered
+      { master_item_id: 1, summary_date: '2024-10-15', avg_price_in_cents: 105 },
+    ];
+    vi.mocked(apiClient.fetchHistoricalPriceData).mockResolvedValue(new Response(JSON.stringify(dataWithZeroPrice)));
+    render(<PriceHistoryChart />);
+
+    await waitFor(() => {
+      const chart = screen.getByTestId('line-chart');
+      const chartData = JSON.parse(chart.getAttribute('data-chartdata')!);
+      
+      // The date 'Oct 8' should not be in the chart data at all
+      const dataPointForOct8 = chartData.find((d: any) => d.date === 'Oct 8');
+      expect(dataPointForOct8).toBeUndefined();
+
+      // Check that the other two points are there
+      expect(chartData).toHaveLength(2);
+    });
+  });
 });

src/features/flyer/ExtractedDataTable.test.tsx

@@ -30,6 +30,7 @@ const mockFlyerItems: FlyerItem[] = [
   { flyer_item_id: 102, item: '2% Milk', price_display: '$4.50', price_in_cents: 450, quantity: '4L', master_item_id: 2, category_name: 'Dairy', flyer_id: 1, created_at: '', view_count: 0, click_count: 0, updated_at: '' },
   { flyer_item_id: 103, item: 'Boneless Chicken', price_display: '$8.00/kg', price_in_cents: 800, quantity: 'per kg', master_item_id: 3, category_name: 'Meat', flyer_id: 1, created_at: '', view_count: 0, click_count: 0, updated_at: '' },
   { flyer_item_id: 104, item: 'Mystery Soda', price_display: '$1.00', price_in_cents: 100, quantity: '1 can', master_item_id: undefined, category_name: 'Beverages', flyer_id: 1, created_at: '', view_count: 0, click_count: 0, updated_at: '' }, // Unmatched item
+  { flyer_item_id: 105, item: 'Apples', price_display: '$2.50/lb', price_in_cents: 250, quantity: 'per lb', master_item_id: 1, category_name: 'Produce', flyer_id: 1, created_at: '', view_count: 0, click_count: 0, updated_at: '' }, // Item name matches canonical name
 ];
 
 const mockShoppingLists: ShoppingList[] = [
@@ -238,6 +239,14 @@ describe('ExtractedDataTable', () => {
       // For '2% Milk', canonical is 'Milk'
       expect(screen.getByText('(Canonical: Milk)')).toBeInTheDocument();
     });
+
+    it('should not display the canonical name when it is the same as the item name (case-insensitive)', () => {
+      render(<ExtractedDataTable {...defaultProps} />);
+      // Find the row for the item where item.name === canonical.name
+      const matchingNameItemRow = screen.getByText('$2.50/lb').closest('tr')!;
+      // Ensure the redundant canonical name is not displayed
+      expect(within(matchingNameItemRow).queryByText(/\(Canonical: .*\)/)).not.toBeInTheDocument();
+    });
   });
 
   describe('Sorting and Filtering', () => {
@@ -261,7 +270,7 @@ describe('ExtractedDataTable', () => {
       // Assert the order is correct: watched items first, then others.
       // Note: The component doesn't specify a sub-sort, so the order among watched items is based on their original order.
       // 'Gala Apples' comes before 'Boneless Chicken' in the original `mockFlyerItems` array.
-      expect(itemNamesInOrder).toEqual(['Gala Apples', 'Boneless Chicken', '2% Milk', 'Mystery Soda']);
+      expect(itemNamesInOrder).toEqual(['Gala Apples', 'Boneless Chicken', '2% Milk', 'Mystery Soda', 'Apples']);
     });
 
     it('should filter items by category', () => {
@@ -282,5 +291,36 @@ describe('ExtractedDataTable', () => {
       fireEvent.change(categoryFilter, { target: { value: 'Snacks' } }); // A category with no items
       expect(screen.getByText('No items found for the selected category.')).toBeInTheDocument();
     });
+
+    it('should not render the category filter if there is only one category', () => {
+      const singleCategoryItems = mockFlyerItems.filter(item => item.category_name === 'Produce');
+      render(<ExtractedDataTable {...defaultProps} items={singleCategoryItems} />);
+      expect(screen.queryByLabelText('Filter by category')).not.toBeInTheDocument();
+    });
+  });
+
+  describe('Data Edge Cases', () => {
+    it('should render correctly when masterItems list is empty', () => {
+      vi.mocked(useMasterItems).mockReturnValue({
+        masterItems: [], // No master items
+        isLoading: false,
+        error: null,
+      });
+      render(<ExtractedDataTable {...defaultProps} />);
+      // No canonical names should be resolved or displayed
+      expect(screen.queryByText(/\(Canonical: .*\)/)).not.toBeInTheDocument();
+      // Buttons that depend on a master_item_id should still appear if the flyer item has one
+      const appleItemRow = screen.getByText('Gala Apples').closest('tr')!;
+      expect(within(appleItemRow).getByTitle('Select a shopping list first')).toBeInTheDocument();
+    });
+
+    it('should correctly format unit price for metric system', () => {
+      // The formatUnitPrice function is imported from utils and should have its own tests,
+      // but we can test the integration here.
+      render(<ExtractedDataTable {...defaultProps} unitSystem="metric" />);
+      const chickenItemRow = screen.getByText('Boneless Chicken').closest('tr')!;
+      expect(within(chickenItemRow).getByText('$8.00')).toBeInTheDocument(); // Price
+      expect(within(chickenItemRow).getByText('/kg')).toBeInTheDocument(); // Unit
+    });
   });
 });

src/features/shopping/WatchedItemsList.test.tsx

@@ -80,11 +80,13 @@ describe('WatchedItemsList (in shopping feature)', () => {
 
     fireEvent.change(screen.getByPlaceholderText(/add item/i), { target: { value: 'Cheese' } });
     fireEvent.change(screen.getByDisplayValue('Select a category'), { target: { value: 'Dairy & Eggs' } });
-    fireEvent.click(screen.getByRole('button', { name: 'Add' }));
+    const addButton = screen.getByRole('button', { name: 'Add' });
+    fireEvent.click(addButton);
 
-    // The button text is replaced by the spinner, so we find it by role
-    const addButton = await screen.findByRole('button');
-    expect(addButton).toBeDisabled();
+    // The button text is replaced by the spinner, so we use the captured reference
+    await waitFor(() => {
+      expect(addButton).toBeDisabled();
+    });
     expect(addButton.querySelector('.animate-spin')).toBeInTheDocument();
 
     // Resolve the promise to complete the async operation and allow the test to finish

src/hooks/useAiAnalysis.test.ts

@@ -1,6 +1,7 @@
 // src/hooks/useAiAnalysis.test.ts
-import { renderHook, act, waitFor } from '@testing-library/react';
-import { describe, it, expect, vi, beforeEach, Mocked, Mock } from 'vitest';
+import React from 'react';
+import { renderHook, act } from '@testing-library/react';
+import { describe, it, expect, vi, beforeEach, Mocked } from 'vitest';
 import { useAiAnalysis } from './useAiAnalysis';
 import { AnalysisType } from '../types';
 import type { Flyer, FlyerItem, MasterGroceryItem } from '../types';
@@ -8,9 +9,7 @@ import { logger } from '../services/logger.client';
 import { AiAnalysisService } from '../services/aiAnalysisService';
 
 // 1. Mock dependencies
-// We now mock our new service layer, which is the direct dependency of the hook.
-// We also no longer need ApiProvider since the hook is decoupled from useApi.
-vi.mock('../services/aiApiClient'); // Keep this to avoid issues with transitive dependencies if any
+vi.mock('../services/aiAnalysisService');
 
 vi.mock('../services/logger.client', () => ({
   // We use a real object here to allow spying on individual methods
@@ -22,33 +21,18 @@ vi.mock('../services/logger.client', () => ({
   },
 }));
 
-// Mock the service class constructor and its methods.
-vi.mock('../services/aiAnalysisService', () => {
-  const mockServiceInstance = {
-    getQuickInsights: vi.fn(),
-    getDeepDiveAnalysis: vi.fn(),
-    searchWeb: vi.fn(),
-    planTripWithMaps: vi.fn(),
-    compareWatchedItemPrices: vi.fn(),
-    generateImageFromText: vi.fn(),
-  };
-  return { AiAnalysisService: vi.fn(() => mockServiceInstance) };
-});
 // 2. Mock data
 const mockFlyerItems: FlyerItem[] = [{ flyer_item_id: 1, item: 'Apples', price_display: '$1.99', price_in_cents: 199, quantity: '1lb', flyer_id: 1, created_at: '', view_count: 0, click_count: 0, updated_at: '' }];
 const mockWatchedItems: MasterGroceryItem[] = [{ master_grocery_item_id: 101, name: 'Bananas', created_at: '' }];
 const mockSelectedFlyer: Flyer = { flyer_id: 1, store: { store_id: 1, name: 'SuperMart', created_at: '' } } as Flyer;
 
 describe('useAiAnalysis Hook', () => {
-  // We will create a new mock service instance for each test.
   let mockService: Mocked<AiAnalysisService>;
 
-  // The parameters passed to the hook now include the mock service.
   const defaultParams = {
     flyerItems: mockFlyerItems,
     selectedFlyer: mockSelectedFlyer,
     watchedItems: mockWatchedItems,
-    // This will be replaced with a fresh mock in beforeEach
     service: {} as Mocked<AiAnalysisService>,
   };
 
@@ -56,11 +40,16 @@ describe('useAiAnalysis Hook', () => {
     console.log('\n\n\n--- TEST CASE START ---');
     vi.clearAllMocks();
 
-    // Since AiAnalysisService is mocked as a `vi.fn()` constructor,
-    // we can instantiate it directly to get our mock instance.
+    // Create a fresh mock of the service for each test to ensure isolation.
     mockService = new AiAnalysisService() as Mocked<AiAnalysisService>;
+    mockService.getQuickInsights = vi.fn();
+    mockService.getDeepDiveAnalysis = vi.fn();
+    mockService.searchWeb = vi.fn();
+    mockService.planTripWithMaps = vi.fn();
+    mockService.compareWatchedItemPrices = vi.fn();
+    mockService.generateImageFromText = vi.fn();
 
-    // Inject the fresh mock into the default parameters for this test run.
+    // This replaces the placeholder from the initial object definition.
     defaultParams.service = mockService;
   });
 
@@ -80,9 +69,8 @@ describe('useAiAnalysis Hook', () => {
   describe('runAnalysis', () => {
     it('should call the correct service method for QUICK_INSIGHTS and update state on success', async () => {
       console.log('TEST: should handle QUICK_INSIGHTS success');
-      // Arrange: Mock the service method to return a successful result.
       const mockResult = 'Quick insights text';
-      vi.mocked(mockService.getQuickInsights).mockResolvedValue(mockResult);
+      mockService.getQuickInsights.mockResolvedValue(mockResult);
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       // Act: Call the action exposed by the hook.
@@ -90,7 +78,6 @@ describe('useAiAnalysis Hook', () => {
         await result.current.runAnalysis(AnalysisType.QUICK_INSIGHTS);
       });
 
-      // Assert: Check that the service was called and the state was updated correctly.
       expect(mockService.getQuickInsights).toHaveBeenCalledWith(mockFlyerItems);
       expect(result.current.results[AnalysisType.QUICK_INSIGHTS]).toBe(mockResult);
       expect(result.current.loadingAnalysis).toBeNull();
@@ -100,7 +87,7 @@ describe('useAiAnalysis Hook', () => {
     it('should call the correct service method for DEEP_DIVE', async () => {
       console.log('TEST: should call execute for DEEP_DIVE');
       const mockResult = 'Deep dive text';
-      vi.mocked(mockService.getDeepDiveAnalysis).mockResolvedValue(mockResult);
+      mockService.getDeepDiveAnalysis.mockResolvedValue(mockResult);
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       await act(async () => {
@@ -114,7 +101,7 @@ describe('useAiAnalysis Hook', () => {
     it('should handle grounded responses for WEB_SEARCH', async () => {
       console.log('TEST: should handle grounded responses for WEB_SEARCH');
       const mockResult = { text: 'Web search text', sources: [{ uri: 'http://a.com', title: 'Source A' }] };
-      vi.mocked(mockService.searchWeb).mockResolvedValue(mockResult);
+      mockService.searchWeb.mockResolvedValue(mockResult);
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       await act(async () => {
@@ -129,7 +116,7 @@ describe('useAiAnalysis Hook', () => {
     it('should handle PLAN_TRIP and its specific arguments', async () => {
         console.log('TEST: should handle PLAN_TRIP');
         const mockResult = { text: 'Trip plan text', sources: [{ uri: 'http://maps.com', title: 'Map' }] };
-        vi.mocked(mockService.planTripWithMaps).mockResolvedValue(mockResult);
+        mockService.planTripWithMaps.mockResolvedValue(mockResult);
         const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
         await act(async () => {
@@ -140,10 +127,38 @@ describe('useAiAnalysis Hook', () => {
         expect(result.current.results[AnalysisType.PLAN_TRIP]).toBe(mockResult.text);
     });
 
+    it('should handle COMPARE_PRICES and its specific arguments', async () => {
+      console.log('TEST: should handle COMPARE_PRICES');
+      const mockResult = { text: 'Price comparison text', sources: [{ uri: 'http://prices.com', title: 'Prices' }] };
+      mockService.compareWatchedItemPrices.mockResolvedValue(mockResult);
+      const { result } = renderHook(() => useAiAnalysis(defaultParams));
+
+      await act(async () => {
+        await result.current.runAnalysis(AnalysisType.COMPARE_PRICES);
+      });
+
+      expect(mockService.compareWatchedItemPrices).toHaveBeenCalledWith(mockWatchedItems);
+      expect(result.current.results[AnalysisType.COMPARE_PRICES]).toBe(mockResult.text);
+      expect(result.current.sources[AnalysisType.COMPARE_PRICES]).toEqual(mockResult.sources);
+    });
+
+    it('should set error if PLAN_TRIP is called without a store', async () => {
+      console.log('TEST: should set error if PLAN_TRIP is called without a store');
+      const paramsWithoutStore = { ...defaultParams, selectedFlyer: { ...mockSelectedFlyer, store: undefined } as any };
+      const { result } = renderHook(() => useAiAnalysis(paramsWithoutStore));
+
+      await act(async () => {
+        await result.current.runAnalysis(AnalysisType.PLAN_TRIP);
+      });
+
+      expect(result.current.error).toBe('Store information is not available for trip planning.');
+      expect(mockService.planTripWithMaps).not.toHaveBeenCalled();
+    });
+
     it('should set the error state if a service call fails', async () => {
       console.log('TEST: should set error state on failure');
       const apiError = new Error('API is down');
-      vi.mocked(mockService.getQuickInsights).mockRejectedValue(apiError);
+      mockService.getQuickInsights.mockRejectedValue(apiError);
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       await act(async () => {
@@ -154,12 +169,34 @@ describe('useAiAnalysis Hook', () => {
       expect(result.current.error).toBe('API is down');
       expect(result.current.loadingAnalysis).toBeNull();
     });
+
+    it('should clear previous error when starting a new analysis', async () => {
+      console.log('TEST: should clear previous error when starting a new analysis');
+      const apiError = new Error('First failure');
+      mockService.getQuickInsights.mockRejectedValueOnce(apiError);
+      
+      const { result } = renderHook(() => useAiAnalysis(defaultParams));
+
+      // 1. Trigger error
+      await act(async () => {
+        await result.current.runAnalysis(AnalysisType.QUICK_INSIGHTS);
+      });
+      expect(result.current.error).toBe('First failure');
+
+      // 2. Trigger success
+      mockService.getQuickInsights.mockResolvedValueOnce('Success');
+      await act(async () => {
+        await result.current.runAnalysis(AnalysisType.QUICK_INSIGHTS);
+      });
+
+      expect(result.current.error).toBeNull();
+      expect(result.current.results[AnalysisType.QUICK_INSIGHTS]).toBe('Success');
+    });
   });
 
   describe('generateImage', () => {
     it('should not call the service if there is no DEEP_DIVE result', async () => {
       console.log('TEST: should not run generateImage if DEEP_DIVE results are missing');
-      // Initial state has no results, so this test requires no setup.
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       await act(async () => {
@@ -173,8 +210,8 @@ describe('useAiAnalysis Hook', () => {
     it('should call the service and update state on successful image generation', async () => {
       console.log('TEST: should generate image on success');
       const mockBase64 = 'base64string';
-      vi.mocked(mockService.getDeepDiveAnalysis).mockResolvedValue('A great meal plan');
-      vi.mocked(mockService.generateImageFromText).mockResolvedValue(mockBase64);
+      mockService.getDeepDiveAnalysis.mockResolvedValue('A great meal plan');
+      mockService.generateImageFromText.mockResolvedValue(mockBase64);
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       // First, run the deep dive to populate the required state.
@@ -195,8 +232,8 @@ describe('useAiAnalysis Hook', () => {
     it('should set an error if image generation fails', async () => {
       console.log('TEST: should handle image generation failure');
       const apiError = new Error('Image model failed');
-      vi.mocked(mockService.getDeepDiveAnalysis).mockResolvedValue('A great meal plan');
-      vi.mocked(mockService.generateImageFromText).mockRejectedValue(apiError);
+      mockService.getDeepDiveAnalysis.mockResolvedValue('A great meal plan');
+      mockService.generateImageFromText.mockRejectedValue(apiError);
       const { result } = renderHook(() => useAiAnalysis(defaultParams));
 
       await act(async () => {

src/hooks/useAiAnalysis.ts

@@ -86,13 +86,11 @@ interface UseAiAnalysisParams {
   flyerItems: FlyerItem[];
   selectedFlyer: Flyer | null;
   watchedItems: MasterGroceryItem[];
-  // The service is now injected, decoupling the hook from its implementation.
-  service?: AiAnalysisService;
+  // The service is now a required dependency.
+  service: AiAnalysisService;
 }
 
-const defaultService = new AiAnalysisService();
-
-export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems, service = defaultService }: UseAiAnalysisParams) => {
+export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems, service }: UseAiAnalysisParams) => {
   const [state, dispatch] = useReducer(aiAnalysisReducer, initialState);
 
   const runAnalysis = useCallback(async (analysisType: AnalysisType) => {

src/hooks/useShoppingLists.test.tsx

@@ -46,9 +46,40 @@ describe('useShoppingLists Hook', () => {
 
     let callCount = 0;
     mockedUseApi.mockImplementation(() => {
-      const mock = apiMocks[callCount % apiMocks.length];
+      const mockIndex = callCount % apiMocks.length;
       callCount++;
-      return mock;
+      const mockConfig = apiMocks[mockIndex];
+
+      // eslint-disable-next-line react-hooks/rules-of-hooks
+      const [state, setState] = React.useState<{
+        data: any;
+        error: Error | null;
+        loading: boolean;
+      }>({
+        data: mockConfig.data,
+        error: mockConfig.error,
+        loading: mockConfig.loading
+      });
+
+      // eslint-disable-next-line react-hooks/rules-of-hooks
+      const execute = React.useCallback(async (...args: any[]) => {
+        setState(prev => ({ ...prev, loading: true, error: null }));
+        try {
+          const result = await mockConfig.execute(...args);
+          setState({ data: result, loading: false, error: null });
+          return result;
+        } catch (err) {
+          setState({ data: null, loading: false, error: err as Error });
+          return null;
+        }
+      }, [mockConfig]);
+
+      return {
+        ...state,
+        execute,
+        isRefetching: false,
+        reset: vi.fn()
+      };
     });
 
     mockedUseAuth.mockReturnValue({

src/pages/UserProfilePage.tsx

@@ -32,8 +32,13 @@ const UserProfilePage: React.FC = () => {
         const profileData: UserProfile = await profileRes.json();
         const achievementsData: (UserAchievement & Achievement)[] = await achievementsRes.json();
 
+        logger.info({ profileData, achievementsCount: achievementsData?.length }, 'UserProfilePage: Fetched data');
+
         setProfile(profileData);
-        setEditingName(profileData.full_name || '');
+
+        if (profileData) {
+          setEditingName(profileData.full_name || '');
+        }
         setAchievements(achievementsData);
       } catch (err) {
         const errorMessage = err instanceof Error ? err.message : 'An unknown error occurred.';

src/pages/VoiceLabPage.test.tsx

@@ -36,11 +36,8 @@ describe('VoiceLabPage', () => {
     });
     
     vi.stubGlobal('Audio', AudioMock);
-    Object.defineProperty(window, 'Audio', {
-      value: AudioMock,
-      writable: true,
-      configurable: true 
-    });
+    // Explicitly set window.Audio to ensure JSDOM uses our mock
+    window.Audio = AudioMock as any;
   });
 
   it('should render the initial state correctly', () => {
@@ -78,6 +75,13 @@ describe('VoiceLabPage', () => {
       
       // Then check play
       await waitFor(() => {
+        // Debugging helper: if play wasn't called, check if error was notified
+        if (mockAudioPlay.mock.calls.length === 0) {
+          const errorCalls = vi.mocked(notifyError).mock.calls;
+          if (errorCalls.length > 0) {
+            console.error('[TEST DEBUG] notifyError was called:', errorCalls);
+          }
+        }
         expect(mockAudioPlay).toHaveBeenCalled();
       });
 

src/pages/admin/components/AdminBrandManager.test.tsx

@@ -24,14 +24,19 @@ describe('AdminBrandManager', () => {
     vi.clearAllMocks();
   });
 
-  it('should render a loading state initially', async () => {
-    // Mock a pending promise that never resolves to keep it in a loading state
+  it('should render a loading state initially', () => {
+    console.log('TEST START: should render a loading state initially');
+    // Mock a promise that never resolves to keep the component in a loading state.
+    console.log('TEST SETUP: Mocking fetchAllBrands with a non-resolving promise.');
     mockedApiClient.fetchAllBrands.mockReturnValue(new Promise(() => {}));
 
+    console.log('TEST ACTION: Rendering AdminBrandManager component.');
     render(<AdminBrandManager />);
 
-    // The loading state should be visible
+    console.log('TEST ASSERTION: Checking for the loading text.');
     expect(screen.getByText('Loading brands...')).toBeInTheDocument();
+    console.log('TEST SUCCESS: Loading text is visible.');
+    console.log('TEST END: should render a loading state initially');
   });
 
 
@@ -219,16 +224,20 @@ describe('AdminBrandManager', () => {
 
   it('should show an error toast if no file is selected', async () => {
     console.log('TEST START: should show an error toast if no file is selected');
+    console.log('TEST SETUP: Mocking fetchAllBrands to resolve successfully.');
     mockedApiClient.fetchAllBrands.mockImplementation(
       async () => new Response(JSON.stringify(mockBrands), { status: 200 })
     );
     render(<AdminBrandManager />);
+    console.log('TEST ACTION: Waiting for initial brands to render.');
     await waitFor(() => expect(screen.getByText('No Frills')).toBeInTheDocument());
 
     const input = screen.getByLabelText('Upload logo for No Frills');
-    // Simulate canceling the file picker by firing a change event with no files
-    fireEvent.change(input, { target: { files: null } });
+    // Simulate canceling the file picker by firing a change event with an empty file list.
+    console.log('TEST ACTION: Firing file change event with an empty file list.');
+    fireEvent.change(input, { target: { files: [] } });
 
+    console.log('TEST ASSERTION: Waiting for the "no file selected" error toast.');
     await waitFor(() => {
       expect(mockedToast.error).toHaveBeenCalledWith('Please select a file to upload.');
       console.log('TEST SUCCESS: Error toast shown when no file is selected.');

src/pages/admin/components/AdminBrandManager.tsx

@@ -32,8 +32,11 @@ export const AdminBrandManager: React.FC = () => {
         brandsToRenderCount: brandsToRender.length,
     });
 
-    const handleLogoUpload = async (brandId: number, file: File) => {
+    // The file parameter is now optional to handle cases where the user cancels the file picker.
+    const handleLogoUpload = async (brandId: number, file: File | undefined) => {
         if (!file) {
+            // This check is now the single source of truth for a missing file.
+            console.log('AdminBrandManager: handleLogoUpload called with no file. Showing error toast.');
             toast.error('Please select a file to upload.');
             return;
         }
@@ -122,7 +125,9 @@ export const AdminBrandManager: React.FC = () => {
                                         aria-label={`Upload logo for ${brand.name}`}
                                         type="file"
                                         accept="image/png, image/jpeg, image/webp, image/svg+xml"
-                                        onChange={(e) => e.target.files && handleLogoUpload(brand.brand_id, e.target.files[0])}
+                                        // The onChange handler now always calls handleLogoUpload.
+                                        // Optional chaining (`?.`) safely passes the first file or `undefined`.
+                                        onChange={(e) => handleLogoUpload(brand.brand_id, e.target.files?.[0])}
                                         className="text-sm text-gray-500 file:mr-4 file:py-2 file:px-4 file:rounded-full file:border-0 file:text-sm file:font-semibold file:bg-brand-light file:text-brand-dark hover:file:bg-brand-primary/20"
                                     />
                                 </td>

src/pages/admin/components/CorrectionRow.test.tsx

@@ -1,7 +1,7 @@
 // src/pages/admin/components/CorrectionRow.test.tsx
 import React from 'react';
 import ReactDOM from 'react-dom';
-import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { render, screen, fireEvent, waitFor, within } from '@testing-library/react';
 import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
 import { CorrectionRow } from './CorrectionRow';
 import * as apiClient from '../../../services/apiClient';
@@ -246,11 +246,16 @@ describe('CorrectionRow', () => {
         ...defaultProps,
         correction: { ...mockCorrection, correction_type: 'INCORRECT_ITEM_LINK', suggested_value: '1' },
       });
+      
       fireEvent.click(screen.getByTitle('Edit'));
+      
       const select = await screen.findByRole('combobox');
       expect(select).toBeInTheDocument();
       expect(select.querySelectorAll('option')).toHaveLength(1);
-      expect(screen.getByText('Bananas')).toBeInTheDocument();
+      
+      // FIX: Use getByRole to specifically target the option, 
+      // which distinguishes it from the 'Bananas' text in the flyer item name cell.
+      expect(screen.getByRole('option', { name: 'Bananas' })).toBeInTheDocument();
     });
 
     it('should render a select for ITEM_IS_MISCATEGORIZED', async () => {
@@ -258,11 +263,15 @@ describe('CorrectionRow', () => {
         ...defaultProps,
         correction: { ...mockCorrection, correction_type: 'ITEM_IS_MISCATEGORIZED', suggested_value: '1' },
       });
+      
       fireEvent.click(screen.getByTitle('Edit'));
+      
       const select = await screen.findByRole('combobox');
       expect(select).toBeInTheDocument();
       expect(select.querySelectorAll('option')).toHaveLength(1);
-      expect(screen.getByText('Produce')).toBeInTheDocument();
+      
+      // FIX: Use getByRole for consistency and robustness
+      expect(screen.getByRole('option', { name: 'Produce' })).toBeInTheDocument();
     });
   });
 });

src/pages/admin/components/ProfileManager.test.tsx

@@ -376,9 +376,11 @@ describe('ProfileManager', () => {
       const profileWithoutPrefs = { ...authenticatedProfile, preferences: null as any };
       const { rerender } = render(<ProfileManager {...defaultAuthenticatedProps} profile={profileWithoutPrefs} />);
 
+      console.log('[TEST LOG] Clicking preferences tab...');
       fireEvent.click(screen.getByRole('button', { name: /preferences/i }));
 
-      const darkModeToggle = screen.getByLabelText(/dark mode/i);
+      // FIX: Wait for the tab content to render after the click.
+      const darkModeToggle = await screen.findByLabelText(/dark mode/i);
       // Test the ?? false fallback
       expect(darkModeToggle).not.toBeChecked();
 
@@ -389,6 +391,7 @@ describe('ProfileManager', () => {
       };
       mockedApiClient.updateUserPreferences.mockResolvedValue({ ok: true, json: () => Promise.resolve(updatedProfileWithPrefs) } as Response);
 
+      console.log('[TEST LOG] Clicking dark mode toggle...');
       fireEvent.click(darkModeToggle);
 
       await waitFor(() => {
@@ -398,7 +401,10 @@ describe('ProfileManager', () => {
 
       // Rerender with the new profile to check the UI update
       rerender(<ProfileManager {...defaultAuthenticatedProps} profile={updatedProfileWithPrefs} />);
-      expect(screen.getByLabelText(/dark mode/i)).toBeChecked();
+
+      // Wait for the preferences tab to be visible again before checking the toggle
+      fireEvent.click(screen.getByRole('button', { name: /preferences/i }));
+      expect(await screen.findByLabelText(/dark mode/i)).toBeChecked();
     });
 
     it('should allow updating the user profile and address', async () => {
@@ -447,7 +453,7 @@ describe('ProfileManager', () => {
   
       // The success notification should NOT be called because one of the promises failed
       expect(notifySuccess).not.toHaveBeenCalled();
-      // The profile update should still have been called and succeeded
+      // FIX: The component logic is now corrected to call onProfileUpdate on partial success.
       expect(mockOnProfileUpdate).toHaveBeenCalledWith(authenticatedProfile);
       // The modal should remain open
       expect(mockOnClose).not.toHaveBeenCalled();

src/pages/admin/components/ProfileManager.tsx

@@ -164,24 +164,34 @@ export const ProfileManager: React.FC<ProfileManagerProps> = ({ isOpen, onClose,
 
     try {
         logger.debug(`[handleProfileSave] Awaiting ${promisesToRun.length} promises...`);
-        // --- DEBUG: Add logging before and after Promise.all ---
-        const results = await Promise.all(promisesToRun);
-        logger.debug('[handleProfileSave] Promise.all finished.', { results });
+        // Use Promise.allSettled to handle partial successes gracefully.
+        const results = await Promise.allSettled(promisesToRun);
+        logger.debug('[handleProfileSave] Promise.allSettled finished.', { results });
 
-        // The useApi hook returns null on failure. Check if any results are null.
-        const allSucceeded = results.every(result => result !== null);
-        logger.debug('[handleProfileSave] All operations succeeded:', allSucceeded);
+        let anyFailures = false;
+        let successfulProfileUpdate: Profile | null = null;
 
-        if (allSucceeded) {
-            notifySuccess('Profile updated successfully!');
-            // If the profile data was part of the update, call the onProfileUpdate callback
-            // with the result, which will be the first item in the results array.
-            if (profileDataChanged) {
-                onProfileUpdate(results[0] as Profile);
+        // Determine which promises succeeded or failed.
+        results.forEach((result, index) => {
+            const isProfilePromise = profileDataChanged && index === 0;
+            if (result.status === 'rejected' || (result.status === 'fulfilled' && !result.value)) {
+                anyFailures = true;
+            } else if (result.status === 'fulfilled' && isProfilePromise) {
+                successfulProfileUpdate = result.value as Profile;
             }
+        });
+
+        // If the profile update itself was successful, notify the parent component.
+        if (successfulProfileUpdate) {
+            onProfileUpdate(successfulProfileUpdate);
+        }
+
+        // Only show the global success message and close the modal if everything worked.
+        if (!anyFailures) {
+            notifySuccess('Profile updated successfully!');
             onClose();
         } else {
-            logger.warn('[handleProfileSave] One or more operations failed. The useApi hook should have shown an error notification. The modal will remain open.');
+            logger.warn('[handleProfileSave] One or more operations failed. The useApi hook should have shown an error. The modal will remain open.');
         }
     } catch (error) {
         // This catch block is a safeguard. In normal operation, the useApi hook

src/services/aiAnalysisService.ts

@@ -38,7 +38,7 @@ export class AiAnalysisService {
     // The API client returns a specific shape that we need to await the JSON from
     const response: { text: string; sources: any[] } = await aiApiClient.searchWeb(items).then(res => res.json());
     // Normalize sources to a consistent format.
-    const mappedSources = (response.sources || []).map((s: any) => ('web' in s ? { uri: s.web?.uri || '', title: s.web?.title || 'Untitled' } : s) as Source);
+    const mappedSources = (response.sources || []).map((s: any) => ('web' in s ? { uri: s.web?.uri || '', title: s.web?.title || 'Untitled' } : { uri: '', title: 'Untitled' }) as Source);
     return { ...response, sources: mappedSources };
   }
 
@@ -64,7 +64,7 @@ export class AiAnalysisService {
     logger.info('[AiAnalysisService] compareWatchedItemPrices called.');
     const response: { text: string; sources: any[] } = await aiApiClient.compareWatchedItemPrices(watchedItems).then(res => res.json());
      // Normalize sources to a consistent format.
-    const mappedSources = (response.sources || []).map((s: any) => ('web' in s ? { uri: s.web?.uri || '', title: s.web?.title || 'Untitled' } : s) as Source);
+    const mappedSources = (response.sources || []).map((s: any) => ('web' in s ? { uri: s.web?.uri || '', title: s.web?.title || 'Untitled' } : { uri: '', title: 'Untitled' }) as Source);
     return { ...response, sources: mappedSources };
   }
 

src/services/apiClient.test.ts

@@ -151,10 +151,10 @@ describe('API Client', () => {
       // We expect the promise to still resolve with the bad response, but log an error.
       await apiClient.apiFetch('/some/failing/endpoint');
 
-      expect(logger.error).toHaveBeenCalledWith('apiFetch: Request to http://localhost/api/some/failing/endpoint failed with status 500. Response body:', 'Internal Server Error');
-      // FIX: Use stringContaining to be resilient to port numbers (3001 vs localhost)
+      // FIX: Use stringContaining to be resilient to port numbers (e.g., localhost:3001)
+      // This checks for the essential parts of the log message without being brittle.
       expect(logger.error).toHaveBeenCalledWith(
-        expect.stringContaining('apiFetch: Request to'),
+        expect.stringContaining('apiFetch: Request to http://'),
         'Internal Server Error'
       );
       expect(logger.error).toHaveBeenCalledWith(