refactor: Improve type inference and error handling in various services and tests

src/routes/admin.routes.ts

@@ -224,10 +224,9 @@ router.get('/unmatched-items', async (req, res, next: NextFunction) => {
  * DELETE /api/admin/recipes/:recipeId - Admin endpoint to delete any recipe.
  */
 router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipeId')), async (req: Request, res: Response, next: NextFunction) => {
-    // Define schema locally to simplify type inference
-    const schema = numericIdParamSchema('recipeId');
     const adminUser = req.user as UserProfile;
-    const { params } = req as unknown as z.infer<typeof schema>;
+    // Infer the type directly from the schema generator function.
+    const { params } = req as unknown as z.infer<ReturnType<typeof numericIdParamSchema>>;
     try {
         // The isAdmin flag bypasses the ownership check in the repository method.
         await db.recipeRepo.deleteRecipe(params.recipeId, adminUser.user_id, true, req.log);
@@ -241,9 +240,8 @@ router.delete('/recipes/:recipeId', validateRequest(numericIdParamSchema('recipe
  * DELETE /api/admin/flyers/:flyerId - Admin endpoint to delete a flyer and its items.
  */
 router.delete('/flyers/:flyerId', validateRequest(numericIdParamSchema('flyerId')), async (req: Request, res: Response, next: NextFunction) => {
-    // Define schema locally to simplify type inference
-    const schema = numericIdParamSchema('flyerId');
-    const { params } = req as unknown as z.infer<typeof schema>;
+    // Infer the type directly from the schema generator function.
+    const { params } = req as unknown as z.infer<ReturnType<typeof numericIdParamSchema>>;
     try {
         await db.flyerRepo.deleteFlyer(params.flyerId, req.log);
         res.status(204).send();

src/routes/price.routes.ts

@@ -1,5 +1,5 @@
 // src/routes/price.routes.ts
-import { Router, Request, Response, NextFunction } from 'express';
+import { Router, Request, Response } from 'express';
 import { z } from 'zod';
 import { validateRequest } from '../middleware/validation.middleware';
 

src/routes/system.routes.test.ts

@@ -60,9 +60,9 @@ describe('System Routes (/api/system)', () => {
         callback?: ExecCallback | null
       ) => {
         // The actual callback can be the second or third argument.
-        const cb = (typeof options === 'function' ? options : callback) as ExecCallback;
-        if (callback) {
-          callback(null, pm2OnlineOutput, '');
+        const actualCallback = (typeof options === 'function' ? options : callback) as ExecCallback;
+        if (actualCallback) {
+          actualCallback(null, pm2OnlineOutput, '');
         }
         // Return a minimal object that satisfies the ChildProcess type for .unref()
         return { unref: () => {} } as ReturnType<typeof exec>;
@@ -84,9 +84,9 @@ describe('System Routes (/api/system)', () => {
         options?: ExecOptions | ((error: ExecException | null, stdout: string, stderr: string) => void) | null,
         callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null
       ) => {
-        const cb = (typeof options === 'function' ? options : callback) as ((error: ExecException | null, stdout: string, stderr: string) => void);
-        if (callback) {
-          callback(null, pm2StoppedOutput, '');
+        const actualCallback = (typeof options === 'function' ? options : callback) as ((error: ExecException | null, stdout: string, stderr: string) => void);
+        if (actualCallback) {
+          actualCallback(null, pm2StoppedOutput, '');
         }
         return { unref: () => {} } as ReturnType<typeof exec>;
       });
@@ -104,9 +104,9 @@ describe('System Routes (/api/system)', () => {
         options?: ExecOptions | ((error: ExecException | null, stdout: string, stderr: string) => void) | null,
         callback?: ((error: ExecException | null, stdout: string, stderr: string) => void) | null
       ) => {
-        const cb = (typeof options === 'function' ? options : callback) as ((error: ExecException | null, stdout: string, stderr: string) => void);
-        if (callback) {
-          callback(new Error('System error') as ExecException, '', 'stderr output');
+        const actualCallback = (typeof options === 'function' ? options : callback) as ((error: ExecException | null, stdout: string, stderr: string) => void);
+        if (actualCallback) {
+          actualCallback(new Error('System error') as ExecException, '', 'stderr output');
         }
         return { unref: () => {} } as ReturnType<typeof exec>;
       });

src/services/aiService.server.test.ts

@@ -26,9 +26,6 @@ describe('AI Service (Server)', () => {
 
   // Instantiate the service with our mock dependencies
   const aiServiceInstance = new AIService(mockLoggerInstance, mockAiClient, mockFileSystem);
-  // Use a type assertion to 'any' to bypass private member access restrictions for testing.
-  // This is a controlled use of 'any' specifically for testing private implementation details.
-  const testableAiService = aiServiceInstance as any;
 
   beforeEach(() => {
     vi.clearAllMocks();
@@ -284,7 +281,7 @@ describe('AI Service (Server)', () => {
 
     it('should throw a "feature disabled" error', async () => {
       // This test verifies the current implementation which has the feature disabled.
-      await expect(aiServiceInstance.planTripWithMaps([], mockStore, mockUserLocation as any, mockLoggerInstance))
+      await expect(aiServiceInstance.planTripWithMaps([], mockStore, mockUserLocation, mockLoggerInstance))
         .rejects.toThrow("The 'planTripWithMaps' feature is currently disabled due to API costs.");
     });
   });

src/services/aiService.server.ts

@@ -8,9 +8,29 @@
 import { GoogleGenAI, type GenerateContentResponse, type Content, type Tool } from '@google/genai';
 import fsPromises from 'node:fs/promises';
 import type { Logger } from 'pino';
+import { z } from 'zod';
 import { pRateLimit } from 'p-ratelimit';
 import type { FlyerItem, MasterGroceryItem, ExtractedFlyerItem } from '../types';
 
+// --- Zod Schemas for AI Response Validation (exported for the transformer) ---
+const ExtractedFlyerItemSchema = z.object({
+  item: z.string(),
+  price_display: z.string(),
+  price_in_cents: z.number().nullable(),
+  quantity: z.string(),
+  category_name: z.string(),
+  master_item_id: z.number().nullish(), // .nullish() allows null or undefined
+});
+
+export const AiFlyerDataSchema = z.object({
+  store_name: z.string().min(1, { message: "Store name cannot be empty" }),
+  valid_from: z.string().nullable(),
+  valid_to: z.string().nullable(),
+  store_address: z.string().nullable(),
+  items: z.array(ExtractedFlyerItemSchema),
+});
+
+
 /**
  * Defines the contract for a file system utility. This interface allows for
  * dependency injection, making the AIService testable without hitting the real file system.
@@ -41,7 +61,7 @@ type RawFlyerItem = {
   price_in_cents: number | null;
   quantity: string | null | undefined;
   category_name: string | null | undefined;
-  master_item_id: number | null;
+  master_item_id?: number | null | undefined;
 };
 
 export class AIService {
@@ -215,7 +235,7 @@ export class AIService {
         }));
       // The response from the SDK is structured, we need to access the text part.
       const text = result.text;
-      const parsedJson = this._parseJsonFromAiResponse<any[]>(text, logger);
+      const parsedJson = this._parseJsonFromAiResponse<{ raw_item_description: string; price_paid_cents: number }[]>(text, logger);
 
       if (!parsedJson) {
         throw new Error('AI response did not contain a valid JSON array.');
@@ -267,18 +287,21 @@ export class AIService {
 
         logger.debug(`[aiService.server] Raw Gemini response text (first 500 chars): ${text?.substring(0, 500)}`);
         
-        const extractedData = this._parseJsonFromAiResponse<any>(text, logger);
+        const extractedData = this._parseJsonFromAiResponse<z.infer<typeof AiFlyerDataSchema>>(text, logger);
 
         if (!extractedData) {
           logger.error({ responseText: text }, "AI response for flyer processing did not contain a valid JSON object.");
           throw new Error('AI response did not contain a valid JSON object.');
         }
 
-        // Normalize the extracted items to handle potential nulls from the AI.
-        if (extractedData && Array.isArray(extractedData.items)) {
-            extractedData.items = this._normalizeExtractedItems(extractedData.items, logger);
-        }
-        return extractedData;
+        // Normalize the items to create a clean data structure.
+        const normalizedItems = Array.isArray(extractedData.items)
+            ? this._normalizeExtractedItems(extractedData.items)
+            : [];
+
+        // Return a new, correctly typed object, rather than mutating the original.
+        // This makes the data flow explicit and satisfies TypeScript.
+        return { ...extractedData, items: normalizedItems };
     } catch (apiError) {
         logger.error({ err: apiError }, "Google GenAI API call failed in extractCoreDataFromFlyerImage");
         throw apiError;
@@ -290,13 +313,13 @@ export class AIService {
    * @param items An array of raw flyer items from the AI.
    * @returns A normalized array of flyer items.
    */
-  private _normalizeExtractedItems(items: RawFlyerItem[], logger: Logger): ExtractedFlyerItem[] {
+  private _normalizeExtractedItems(items: RawFlyerItem[]): ExtractedFlyerItem[] {
     return items.map((item: RawFlyerItem) => ({
       ...item,
       price_display: item.price_display === null || item.price_display === undefined ? "" : String(item.price_display),
       quantity: item.quantity === null || item.quantity === undefined ? "" : String(item.quantity),
       category_name: item.category_name === null || item.category_name === undefined ? "Other/Miscellaneous" : String(item.category_name),
-      master_item_id: item.master_item_id === null ? undefined : item.master_item_id,
+      master_item_id: item.master_item_id ?? undefined,
     }));
   }
 

src/services/backgroundJobService.test.ts

@@ -30,7 +30,6 @@ import { BackgroundJobService, startBackgroundJobs } from './backgroundJobServic
 import type { Queue } from 'bullmq';
 import type { PersonalizationRepository } from './db/personalization.db';
 import type { NotificationRepository } from './db/notification.db';
-import type { WatchedItemDeal } from '../types';
 import { logger as globalMockLogger } from '../services/logger.server'; // Import the mocked logger
 
 describe('Background Job Service', () => {

src/services/db/admin.db.ts

@@ -3,7 +3,7 @@ import type { Pool, PoolClient } from 'pg';
 import { getPool, withTransaction } from './connection.db';
 import { ForeignKeyConstraintError, NotFoundError } from './errors.db';
 import type { Logger } from 'pino';
-import { SuggestedCorrection, MostFrequentSaleItem, Recipe, RecipeComment, UnmatchedFlyerItem, ActivityLogItem, Receipt, User, AdminUserView, Profile } from '../../types';
+import { SuggestedCorrection, MostFrequentSaleItem, Recipe, RecipeComment, UnmatchedFlyerItem, ActivityLogItem, Receipt, AdminUserView, Profile } from '../../types';
 
 export class AdminRepository {
   private db: Pool | PoolClient;

src/services/db/errors.db.ts

@@ -50,7 +50,7 @@ export class NotFoundError extends DatabaseError {
 export interface ValidationIssue {
   path: (string | number)[];
   message: string;
-  [key: string]: any; // Allow other properties that might exist on the error object
+  [key: string]: unknown; // Allow other properties that might exist on the error object
 }
 
 /**

src/services/db/gamification.db.test.ts

@@ -1,5 +1,6 @@
 // src/services/db/gamification.db.test.ts
 import { describe, it, expect, vi, beforeEach } from 'vitest';
+import type { Pool } from 'pg';
 import { mockPoolInstance } from '../../tests/setup/tests-setup-unit';
 
 // FIX 2: Un-mock the module we are testing.
@@ -26,7 +27,7 @@ describe('Gamification DB Service', () => {
     // Reset the global mock's call history before each test.
     vi.clearAllMocks();
     // Instantiate the repository with the mock pool for each test
-    gamificationRepo = new GamificationRepository(mockPoolInstance as any);
+    gamificationRepo = new GamificationRepository(mockPoolInstance as unknown as Pool);
   });
 
   describe('getAllAchievements', () => {
@@ -81,7 +82,7 @@ describe('Gamification DB Service', () => {
 
     it('should throw ForeignKeyConstraintError if user or achievement does not exist', async () => {
       const dbError = new Error('violates foreign key constraint');
-      (dbError as any).code = '23503';
+      (dbError as Error & { code: string }).code = '23503';
       mockPoolInstance.query.mockRejectedValue(dbError);
       await expect(gamificationRepo.awardAchievement('non-existent-user', 'Non-existent Achievement', mockLogger)).rejects.toThrow('The specified user or achievement does not exist.');
       expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError, userId: 'non-existent-user', achievementName: 'Non-existent Achievement' }, 'Database error in awardAchievement');

src/services/db/notification.db.test.ts

@@ -1,5 +1,6 @@
 // src/services/db/notification.db.test.ts
 import { describe, it, expect, vi, beforeEach } from 'vitest';
+import type { Pool } from 'pg';
 
 // Un-mock the module we are testing to ensure we use the real implementation.
 vi.unmock('./notification.db');
@@ -26,7 +27,7 @@ describe('Notification DB Service', () => {
   beforeEach(() => {
     vi.clearAllMocks();
     // Instantiate the repository with the mock pool for each test
-    notificationRepo = new NotificationRepository(mockPoolInstance as any);
+    notificationRepo = new NotificationRepository(mockPoolInstance as unknown as Pool);
   });
 
   describe('getNotificationsForUser', () => {
@@ -83,7 +84,7 @@ describe('Notification DB Service', () => {
 
     it('should throw ForeignKeyConstraintError if user does not exist', async () => {
       const dbError = new Error('violates foreign key constraint');
-      (dbError as any).code = '23503';
+      (dbError as Error & { code: string }).code = '23503';
       mockPoolInstance.query.mockRejectedValueOnce(dbError);
       await expect(notificationRepo.createNotification('non-existent-user', 'Test', mockLogger)).rejects.toThrow('The specified user does not exist.');
       expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError, userId: 'non-existent-user', content: 'Test', linkUrl: undefined }, 'Database error in createNotification');
@@ -117,7 +118,7 @@ describe('Notification DB Service', () => {
 
     it('should throw ForeignKeyConstraintError if any user does not exist', async () => {
       const dbError = new Error('violates foreign key constraint');
-      (dbError as any).code = '23503';
+      (dbError as Error & { code: string }).code = '23503';
       mockPoolInstance.query.mockRejectedValue(dbError);
       const notificationsToCreate = [{ user_id: 'non-existent', content: "msg" }];
       await expect(notificationRepo.createBulkNotifications(notificationsToCreate, mockLogger)).rejects.toThrow(ForeignKeyConstraintError);

src/services/db/personalization.db.ts

@@ -227,13 +227,10 @@ export class PersonalizationRepository {
         }
       });
     } catch (error) {
+      // Check for a foreign key violation, which would mean an invalid ID was provided.
       if (error instanceof Error && 'code' in error && error.code === '23503') {
         throw new ForeignKeyConstraintError('One or more of the specified restriction IDs are invalid.');
       }
-      // The patch requested this specific error handling.
-      if ((error as any).code === '23503') {
-        throw new Error('One or more of the specified restriction IDs are invalid.');
-      }
       logger.error({ err: error, userId, restrictionIds }, 'Database error in setUserDietaryRestrictions');
       throw new Error('Failed to set user dietary restrictions.');
     }
@@ -261,8 +258,8 @@ export class PersonalizationRepository {
         return newAppliances;
       });
     } catch (error) {
-      // The patch requested this specific error handling - check for foreign key violation
-      if ((error as any).code === '23503') {
+      // Check for a foreign key violation, which would mean an invalid ID was provided.
+      if (error instanceof Error && 'code' in error && error.code === '23503') {
         throw new ForeignKeyConstraintError('Invalid appliance ID');
       }
       logger.error({ err: error, userId, applianceIds }, 'Database error in setUserAppliances');

src/services/db/user.db.test.ts

@@ -1,5 +1,5 @@
 // src/services/db/user.db.test.ts
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest';
 import { PoolClient } from 'pg';
 
 // Mock the logger to prevent stderr noise during tests
@@ -154,9 +154,13 @@ describe('User DB Service', () => {
       try {
         await userRepo.createUser('exists@example.com', 'pass', {}, mockLogger);
         expect.fail('Expected createUser to throw UniqueConstraintError');
-      } catch (error: any) {
+      } catch (error: unknown) {
         expect(error).toBeInstanceOf(UniqueConstraintError);
-        expect(error.message).toBe('A user with this email address already exists.');
+        // After confirming the error type, we can safely access its properties.
+        // This satisfies TypeScript's type checker for the 'unknown' type.
+        if (error instanceof Error) {
+          expect(error.message).toBe('A user with this email address already exists.');
+        }
       }
 
       expect(withTransaction).toHaveBeenCalledTimes(1);

src/services/emailService.server.test.ts

@@ -35,6 +35,7 @@ vi.mock('./logger.server', () => ({
 
 // Now that mocks are set up, we can import the service under test.
 import { sendPasswordResetEmail, sendWelcomeEmail, sendDealNotificationEmail } from './emailService.server';
+import type { WatchedItemDeal } from '../types';
 import { logger } from './logger.server';
 
 describe('Email Service (Server)', () => {
@@ -114,7 +115,7 @@ describe('Email Service (Server)', () => {
       const to = 'deal.hunter@example.com';
       const name = 'Deal Hunter';
 
-      await sendDealNotificationEmail(to, name, mockDeals as any, logger);
+      await sendDealNotificationEmail(to, name, mockDeals as Partial<WatchedItemDeal>[] as WatchedItemDeal[], logger);
 
       expect(mocks.sendMail).toHaveBeenCalledTimes(1);
       const mailOptions = mocks.sendMail.mock.calls[0][0] as { to: string, subject: string, text: string, html: string };
@@ -136,7 +137,7 @@ describe('Email Service (Server)', () => {
     it('should send a generic email when name is null', async () => {
       const to = 'anonymous.user@example.com';
 
-      await sendDealNotificationEmail(to, null, mockDeals as any, logger);
+      await sendDealNotificationEmail(to, null, mockDeals as Partial<WatchedItemDeal>[] as WatchedItemDeal[], logger);
 
       expect(mocks.sendMail).toHaveBeenCalledTimes(1);
       const mailOptions = mocks.sendMail.mock.calls[0][0] as { to: string, subject: string, html: string };
@@ -151,7 +152,7 @@ describe('Email Service (Server)', () => {
       const emailError = new Error('SMTP Connection Failed');
       mocks.sendMail.mockRejectedValue(emailError);
 
-      await expect(sendDealNotificationEmail(to, name, mockDeals as any, logger)).rejects.toThrow(emailError);
+      await expect(sendDealNotificationEmail(to, name, mockDeals as Partial<WatchedItemDeal>[] as WatchedItemDeal[], logger)).rejects.toThrow(emailError);
 
       expect(logger.error).toHaveBeenCalledWith(
         { err: emailError, to, subject: 'New Deals Found on Your Watched Items!' },

src/services/flyerDataTransformer.test.ts

@@ -5,6 +5,7 @@ import { logger as mockLogger } from './logger.server';
 import { generateFlyerIcon } from '../utils/imageProcessor';
 import type { z } from 'zod';
 import type { AiFlyerDataSchema } from './flyerProcessingService.server';
+import type { FlyerItemInsert } from '../types';
 
 // Mock the dependencies
 vi.mock('../utils/imageProcessor', () => ({
@@ -78,8 +79,8 @@ describe('FlyerDataTransformer', () => {
       master_item_id: undefined, // null should be converted to undefined
       view_count: 0,
       click_count: 0,
-    }));
-    expect((itemsForDb[0] as any).updated_at).toBeTypeOf('string');
+    })); // Use a more specific type assertion to check for the added property.
+    expect((itemsForDb[0] as FlyerItemInsert & { updated_at: string }).updated_at).toBeTypeOf('string');
 
     // 3. Check that generateFlyerIcon was called correctly
     expect(generateFlyerIcon).toHaveBeenCalledWith('/uploads/flyer-page-1.jpg', '/uploads/icons', mockLogger);

src/services/flyerProcessingService.server.test.ts

@@ -254,7 +254,7 @@ describe('FlyerProcessingService', () => {
       const paths = ['/tmp/file1.jpg', '/tmp/file2.pdf'];
       
       // Access and call the private method for testing
-      await (service as any)._enqueueCleanup(flyerId, paths, logger);
+      await (service as unknown as { _enqueueCleanup: (flyerId: number, paths: string[], logger: Logger) => Promise<void> })._enqueueCleanup(flyerId, paths, logger);
 
       expect(mockCleanupQueue.add).toHaveBeenCalledWith(
         'cleanup-flyer-files',
@@ -293,10 +293,20 @@ describe('FlyerProcessingService', () => {
 
       // The transformer is already spied on in beforeEach, we can just check its call.
       const transformerSpy = vi.spyOn(FlyerDataTransformer.prototype, 'transform');
-
+      
       // The DB create function is also mocked in beforeEach.
-      const mockNewFlyer = { flyer_id: 1, file_name: 'flyer.jpg', store_name: 'Mock Store' };
-      vi.mocked(createFlyerAndItems).mockResolvedValue({ flyer: mockNewFlyer, items: [] } as any);
+      // Create a complete mock that satisfies the Flyer type.
+      const mockNewFlyer: Flyer = {
+        flyer_id: 1,
+        file_name: 'flyer.jpg',
+        image_url: '/flyer-images/flyer.jpg',
+        icon_url: '/flyer-images/icons/icon-flyer.webp',
+        checksum: 'checksum-123',
+        store_id: 1,
+        item_count: 1,
+        created_at: new Date().toISOString(),
+      };
+      vi.mocked(createFlyerAndItems).mockResolvedValue({ flyer: mockNewFlyer, items: [] });
 
       // Act: Access and call the private method for testing
       const result = await (service as unknown as { _saveProcessedFlyerData: (extractedData: any, imagePaths: any, jobData: any, logger: Logger) => Promise<Flyer> })._saveProcessedFlyerData(mockExtractedData, mockImagePaths, mockJobData, logger);

src/services/flyerProcessingService.server.ts

@@ -4,7 +4,6 @@ import path from 'path';
 import type { Dirent } from 'node:fs';
 import { z } from 'zod';
 
-import { logger } from './logger.server';
 import type { AIService } from './aiService.server';
 import type * as db from './db/index.db';
 import { createFlyerAndItems } from './db/flyer.db';

src/tests/setup/integration-global-setup.ts

@@ -26,6 +26,10 @@ export async function setup() {
     process.exit(1);
   }
 
+  // Initialize the global pool instance once.
+  console.log(`[PID:${process.pid}] Initializing global database pool...`);
+  globalPool = getPool();
+
   // Programmatically start the server within the same process.
   const port = process.env.PORT || 3001;
   await new Promise<void>((resolve) => {

src/utils/pdfConverter.test.ts

@@ -195,7 +195,12 @@ describe('pdfConverter', () => {
     // Mock FileReader to simulate an error
     // FIX: The mock must be a class (or a function that can be called with `new`).
     // This mock simulates the behavior of a FileReader that immediately errors out.
-    const MockErrorReader = vi.fn(function(this: any) {
+    interface MockFileReader {
+      readAsArrayBuffer: () => void;
+      onerror: (() => void) | null;
+      error: { message: string };
+    }
+    const MockErrorReader = vi.fn(function(this: MockFileReader) {
       this.readAsArrayBuffer = () => {
         // The `onerror` property is a function that gets called by the browser.
         // We simulate this by calling it ourselves.
@@ -204,7 +209,7 @@ describe('pdfConverter', () => {
         }
       };
       this.error = { message: 'Simulated FileReader error' };
-    } as any);
+    });
     vi.stubGlobal('FileReader', MockErrorReader);
 
     await expect(convertPdfToImageFiles(pdfFile)).rejects.toThrow('FileReader error: Simulated FileReader error');