Merge pull request #42 from atomantic/better/bugs-perf

fix: resolve N+1 queries, memory leak, missing error forwarding

Author: atomantic

server/src/lib/graph/pathLongest.ts

@@ -11,39 +11,56 @@ export const pathLongest = (
   source: string,
   target: string
 ): string[] => {
-  let longest: string[] = [];
-  const queue: string[][] = [[source]];
-  const depthMap: Record<string, number> = {
-    [source]: 0,
-  };
+  // Track the best (longest) depth at which we've visited each node
+  const depthMap: Record<string, number> = { [source]: 0 };
+  // Parent pointers keyed by "node" storing the parent on the longest known path
+  const parentOf: Record<string, string | null> = { [source]: null };
+  // Track visited nodes on the current BFS path to detect cycles
+  // Each queue entry is [node, depth, Set of ancestors on this path]
+  const queue: Array<[string, number, Set<string>]> = [[source, 0, new Set([source])]];
+  let longestDepth = -1;
+  let foundTarget = false;
 
   while (queue.length) {
-    const path = queue.shift()!;
-    const node = path[path.length - 1];
+    const [node, depth, ancestors] = queue.shift()!;
 
     if (node === target) {
-      if (path.length > longest.length) {
-        longest = path;
+      if (depth > longestDepth) {
+        longestDepth = depth;
+        foundTarget = true;
+      }
+      continue;
+    }
+
+    const children = graph[node]?.children || [];
+    for (const child of children) {
+      if (ancestors.has(child)) {
+        logger.error('graph', `TIME TRAVELER! Cyclic relationship: ${child} <-> ${node}`);
+        continue;
+      }
+      if (!depthMap[child]) depthMap[child] = 0;
+      // only queue this child if it is a further distance than we have seen already
+      if (depthMap[child] < depth + 1) {
+        depthMap[child] = depth + 1;
+        parentOf[child] = node;
+        const childAncestors = new Set(ancestors);
+        childAncestors.add(child);
+        queue.push([child, depth + 1, childAncestors]);
       }
-    } else {
-      const children = graph[node]?.children || [];
-      children.forEach((child) => {
-        if (!depthMap[child]) depthMap[child] = 0;
-        if (path.includes(child)) {
-          logger.error('graph', `TIME TRAVELER! Cyclic relationship: ${child} <-> ${node} | ${path.slice(path.indexOf(child)).join(' -> ')}`);
-          return;
-        }
-        // only queue this child if it is a further distance than we have seen already
-        // for this child relationship to the source
-        if (depthMap[child] < path.length) {
-          queue.push([...path, child]);
-          depthMap[child] = path.length;
-        }
-      });
     }
   }
 
-  return longest;
+  if (!foundTarget) return [];
+
+  // Reconstruct path from parent pointers
+  const path: string[] = [];
+  let current: string | null = target;
+  while (current !== null) {
+    path.push(current);
+    current = parentOf[current] ?? null;
+  }
+  path.reverse();
+  return path;
 };
 
 export default pathLongest;

server/src/routes/ai-discovery.routes.ts

@@ -2,14 +2,15 @@ import { Router, Request, Response } from 'express';
 import { aiDiscoveryService } from '../services/ai-discovery.service.js';
 import { favoritesService } from '../services/favorites.service.js';
 import { logger } from '../lib/logger.js';
+import { asyncHandler } from '../utils/asyncHandler.js';
 
 const router = Router();
 
 /**
  * Start a quick AI discovery to find interesting ancestors
  * POST /api/ai-discovery/:dbId/quick
  */
-router.post('/:dbId/quick', async (req: Request, res: Response) => {
+router.post('/:dbId/quick', asyncHandler(async (req: Request, res: Response) => {
   const { dbId } = req.params;
   const { sampleSize, excludeBiblical, minBirthYear, maxGenerations, customPrompt } = req.body;
 
@@ -31,13 +32,13 @@ router.post('/:dbId/quick', async (req: Request, res: Response) => {
     logger.done('ai-discovery', `Quick discovery complete: analyzed=${result.totalAnalyzed} candidates=${result.candidates.length}`);
     res.json({ success: true, data: result });
   }
-});
+}));
 
 /**
  * Start a full AI discovery run (async)
  * POST /api/ai-discovery/:dbId/start
  */
-router.post('/:dbId/start', async (req: Request, res: Response) => {
+router.post('/:dbId/start', asyncHandler(async (req: Request, res: Response) => {
   const { dbId } = req.params;
   const { batchSize, maxPersons } = req.body;
 
@@ -52,7 +53,7 @@ router.post('/:dbId/start', async (req: Request, res: Response) => {
   if (result !== null) {
     res.json({ success: true, data: result });
   }
-});
+}));
 
 /**
  * Get progress of a discovery run

server/src/routes/ancestry-hints.routes.ts

@@ -8,14 +8,15 @@ import { Router, Request, Response } from 'express';
 import { ancestryHintsService } from '../services/ancestry-hints.service.js';
 import { logger } from '../lib/logger.js';
 import { initSSE } from '../utils/sseHelpers.js';
+import { asyncHandler } from '../utils/asyncHandler.js';
 
 const router = Router();
 
 /**
  * POST /:dbId/:personId - Process free hints for a single person
  * Returns the result with counts of hints processed
  */
-router.post('/:dbId/:personId', async (req: Request, res: Response) => {
+router.post('/:dbId/:personId', asyncHandler(async (req: Request, res: Response) => {
   const { personId } = req.params;
 
   logger.start('ancestry-hints', `Processing hints for person ${personId}`);
@@ -38,13 +39,13 @@ router.post('/:dbId/:personId', async (req: Request, res: Response) => {
   }
 
   res.json({ success: true, data: result });
-});
+}));
 
 /**
  * GET /:dbId/:personId/events - SSE stream for hints processing progress
  * Streams real-time progress events during hint processing
  */
-router.get('/:dbId/:personId/events', async (req: Request, res: Response) => {
+router.get('/:dbId/:personId/events', asyncHandler(async (req: Request, res: Response) => {
   const { personId } = req.params;
 
   initSSE(res);
@@ -82,7 +83,7 @@ router.get('/:dbId/:personId/events', async (req: Request, res: Response) => {
   }
 
   res.end();
-});
+}));
 
 /**
  * POST /:dbId/cancel - Cancel the running hints operation

server/src/routes/augmentation.routes.ts

@@ -29,7 +29,14 @@ function servePhoto(getPath: (id: string) => string | null, label: string) {
 
     res.setHeader('Content-Type', contentType);
     res.setHeader('Cache-Control', 'public, max-age=86400');
-    fs.createReadStream(photoPath).pipe(res);
+    const stream = fs.createReadStream(photoPath);
+    stream.on('error', (err) => {
+      logger.error('augment', `Stream error serving ${label} photo: ${err.message}`);
+      if (!res.headersSent) {
+        res.status(500).json({ success: false, error: 'Error reading photo file' });
+      }
+    });
+    stream.pipe(res);
   };
 }
 
@@ -47,7 +54,7 @@ router.get('/:personId', async (req: Request, res: Response) => {
 });
 
 // Link a Wikipedia article to a person
-router.post('/:personId/wikipedia', async (req: Request, res: Response) => {
+router.post('/:personId/wikipedia', asyncHandler(async (req: Request, res: Response) => {
   const personId = sanitizePersonId(req.params.personId);
   const { url } = req.body;
 
@@ -70,7 +77,7 @@ router.post('/:personId/wikipedia', async (req: Request, res: Response) => {
   if (data) {
     res.json({ success: true, data });
   }
-});
+}));
 
 // Update custom augmentation data
 router.put('/:personId', async (req: Request, res: Response) => {

server/src/services/browser.service.ts

@@ -79,7 +79,7 @@ export const browserService = {
     }
 
     connectedBrowser = await chromium.connectOverCDP(url);
-    broadcastStatusUpdate();
+    await broadcastStatusUpdate();
     return connectedBrowser;
   },
 
@@ -88,7 +88,7 @@ export const browserService = {
       await connectedBrowser.close();
       connectedBrowser = null;
       workerPage = null;
-      broadcastStatusUpdate();
+      await broadcastStatusUpdate();
     }
   },
 
@@ -255,7 +255,7 @@ export const browserService = {
 
     // Navigation may change FamilySearch login status
     if (url.includes('familysearch.org')) {
-      broadcastStatusUpdate();
+      await broadcastStatusUpdate();
     }
 
     return page;
@@ -303,7 +303,7 @@ export const browserService = {
     await new Promise(resolve => setTimeout(resolve, 2000));
 
     const nowRunning = await checkBrowserProcessRunning();
-    broadcastStatusUpdate();
+    await broadcastStatusUpdate();
     return {
       success: nowRunning,
       message: nowRunning ? 'Browser launched successfully' : 'Browser may still be starting...'

server/src/services/search.service.ts

@@ -171,14 +171,11 @@ async function searchWithSqlite(
     offset,
   });
 
-  // Build full person objects
-  const results: PersonWithId[] = [];
-  for (const { person_id } of personIds) {
-    const person = await databaseService.getPerson(dbId, person_id);
-    if (person) {
-      results.push(person);
-    }
-  }
+  // Build full person objects (batch load in parallel)
+  const persons = await Promise.all(
+    personIds.map(({ person_id }) => databaseService.getPerson(dbId, person_id))
+  );
+  const results: PersonWithId[] = persons.filter((p): p is PersonWithId => p !== null);
 
   const totalPages = Math.ceil(total / limit);
   return { results, total, page, limit, totalPages };
@@ -313,14 +310,10 @@ export const searchService = {
         }
       );
 
-      const results: PersonWithId[] = [];
-      for (const { person_id } of personIds) {
-        const person = await databaseService.getPerson(dbId, person_id);
-        if (person) {
-          results.push(person);
-        }
-      }
-      return results;
+      const persons = await Promise.all(
+        personIds.map(({ person_id }) => databaseService.getPerson(dbId, person_id))
+      );
+      return persons.filter((p): p is PersonWithId => p !== null);
     }
 
     // Fall back to simple prefix search