fix: address PR review feedback round 7

Author: atomantic

client/src/components/ancestry-tree/views/MigrationMapView.tsx

@@ -18,25 +18,17 @@ import { useEffect, useRef, useState, useMemo, useCallback } from 'react';
 import { MapContainer, TileLayer, Marker, Polyline, Popup, useMap } from 'react-leaflet';
 import type { MapPerson, MapData } from '@fsf/shared';
 import { GeocodeProgressBar } from '../../map/GeocodeProgressBar';
+import { Link } from 'react-router-dom';
 import {
   createPersonMarker,
-  buildPopupHtml,
   getMigrationLineStyle,
   buildMigrationLines,
   calculateBounds,
 } from '../../map/mapUtils';
 
 import 'leaflet/dist/leaflet.css';
 
-// Fix Leaflet default icon issue with Vite
 import L from 'leaflet';
-// @ts-expect-error Leaflet icon path fix for bundlers
-delete L.Icon.Default.prototype._getIconUrl;
-L.Icon.Default.mergeOptions({
-  iconRetinaUrl: 'https://unpkg.com/leaflet@1.9.4/dist/images/marker-icon-2x.png',
-  iconUrl: 'https://unpkg.com/leaflet@1.9.4/dist/images/marker-icon.png',
-  shadowUrl: 'https://unpkg.com/leaflet@1.9.4/dist/images/marker-shadow.png',
-});
 
 interface MigrationMapViewProps {
   mapData: MapData | null;
@@ -67,6 +59,43 @@ function FitBounds({ persons }: { persons: MapPerson[] }) {
   return null;
 }
 
+function PersonPopupContent({ person, dbId }: { person: MapPerson; dbId: string }) {
+  const genderIcon = person.gender === 'male' ? '\u2642' : person.gender === 'female' ? '\u2640' : '';
+  const lineageLabel = person.lineage === 'paternal' ? 'Paternal' : person.lineage === 'maternal' ? 'Maternal' : '';
+
+  return (
+    <div style={{ minWidth: 180, maxWidth: 280 }}>
+      {person.photoUrl && (
+        <img
+          src={person.photoUrl}
+          alt={person.name}
+          style={{ width: 48, height: 48, borderRadius: '50%', objectFit: 'cover', marginRight: 8, float: 'left' }}
+        />
+      )}
+      <div>
+        <Link
+          to={`/person/${encodeURIComponent(dbId)}/${encodeURIComponent(person.id)}`}
+          style={{ color: '#4A90D9', fontWeight: 600, fontSize: 14, textDecoration: 'none' }}
+        >
+          {person.name}{person.isFavorite && ' \u2B50'}
+        </Link>
+        <div style={{ color: '#888', fontSize: 12, marginTop: 2 }}>
+          {genderIcon} {person.lifespan}
+          {lineageLabel && ` \u00B7 ${lineageLabel}`}
+          {person.generation > 0 && ` \u00B7 Gen ${person.generation}`}
+        </div>
+        {(person.birthPlace || person.deathPlace) && (
+          <div style={{ color: '#aaa', fontSize: 11, marginTop: 4 }}>
+            {person.birthPlace && <div>Born: {person.birthPlace}</div>}
+            {person.deathPlace && <div>Died: {person.deathPlace}</div>}
+          </div>
+        )}
+      </div>
+      <div style={{ clear: 'both' }} />
+    </div>
+  );
+}
+
 export function MigrationMapView({ mapData, dbId, loading, onReload }: MigrationMapViewProps) {
   const [timeRange, setTimeRange] = useState<[number, number]>([0, 2100]);
   const [showPaternal, setShowPaternal] = useState(true);
@@ -277,7 +306,7 @@ export function MigrationMapView({ mapData, dbId, loading, onReload }: Migration
                 icon={createPersonMarker(person)}
               >
                 <Popup>
-                  <div dangerouslySetInnerHTML={{ __html: buildPopupHtml(person, dbId) }} />
+                  <PersonPopupContent person={person} dbId={dbId} />
                 </Popup>
               </Marker>
             );

client/src/components/map/mapUtils.ts

@@ -12,13 +12,6 @@ import { PATERNAL_COLORS, MATERNAL_COLORS } from '../ancestry-tree/utils/lineage
 // Self color (for root person)
 const SELF_COLOR = '#A37FDB'; // Purple
 
-/**
- * Escape HTML special characters to prevent XSS
- */
-function escapeHtml(str: string): string {
-  return str.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#x27;');
-}
-
 /**
  * Get the lineage color for a person
  */
@@ -58,42 +51,6 @@ export function createPersonMarker(person: MapPerson): L.DivIcon {
   });
 }
 
-/**
- * Build popup HTML for a person marker
- */
-export function buildPopupHtml(person: MapPerson, dbId: string): string {
-  const name = escapeHtml(person.name);
-  const photoUrl = person.photoUrl ? escapeHtml(person.photoUrl) : '';
-  const photoHtml = photoUrl
-    ? `<img src="${photoUrl}" alt="${name}" style="width:48px;height:48px;border-radius:50%;object-fit:cover;margin-right:8px;float:left;" />`
-    : '';
-
-  const genderIcon = person.gender === 'male' ? '\u2642' : person.gender === 'female' ? '\u2640' : '';
-  const lineageLabel = person.lineage === 'paternal' ? 'Paternal' : person.lineage === 'maternal' ? 'Maternal' : '';
-  const favoriteLabel = person.isFavorite ? ' &#11088;' : '';
-  const lifespan = escapeHtml(person.lifespan);
-
-  const places: string[] = [];
-  if (person.birthPlace) places.push(`Born: ${escapeHtml(person.birthPlace)}`);
-  if (person.deathPlace) places.push(`Died: ${escapeHtml(person.deathPlace)}`);
-
-  return `
-    <div style="min-width:180px;max-width:280px;">
-      ${photoHtml}
-      <div>
-        <a href="/person/${encodeURIComponent(dbId)}/${encodeURIComponent(person.id)}" style="color:#4A90D9;font-weight:600;font-size:14px;text-decoration:none;">
-          ${name}${favoriteLabel}
-        </a>
-        <div style="color:#888;font-size:12px;margin-top:2px;">
-          ${genderIcon} ${lifespan}${lineageLabel ? ` &middot; ${lineageLabel}` : ''}${person.generation > 0 ? ` &middot; Gen ${person.generation}` : ''}
-        </div>
-        ${places.length > 0 ? `<div style="color:#aaa;font-size:11px;margin-top:4px;">${places.join('<br/>')}</div>` : ''}
-      </div>
-      <div style="clear:both;"></div>
-    </div>
-  `;
-}
-
 /**
  * Migration line style based on lineage
  */

client/src/services/api.ts

@@ -722,23 +722,39 @@ export const api = {
       body: JSON.stringify({ personId, whyInteresting, tags })
     }),
 
-  applyDiscoveryBatch: (dbId: string, candidates: DiscoveryCandidate[]) =>
-    fetchJson<{ applied: number }>(`/ai-discovery/${dbId}/apply-batch`, {
-      method: 'POST',
-      body: JSON.stringify({ candidates })
-    }),
+  applyDiscoveryBatch: async (dbId: string, candidates: DiscoveryCandidate[]): Promise<{ applied: number }> => {
+    const CHUNK_SIZE = 1000;
+    let totalApplied = 0;
+    for (let i = 0; i < candidates.length; i += CHUNK_SIZE) {
+      const chunk = candidates.slice(i, i + CHUNK_SIZE);
+      const result = await fetchJson<{ applied: number }>(`/ai-discovery/${dbId}/apply-batch`, {
+        method: 'POST',
+        body: JSON.stringify({ candidates: chunk })
+      });
+      totalApplied += result.applied;
+    }
+    return { applied: totalApplied };
+  },
 
   dismissDiscoveryCandidate: (dbId: string, personId: string, whyInteresting?: string, suggestedTags?: string[]) =>
     fetchJson<{ success: boolean }>(`/ai-discovery/${dbId}/dismiss`, {
       method: 'POST',
       body: JSON.stringify({ personId, whyInteresting, suggestedTags })
     }),
 
-  dismissDiscoveryBatch: (dbId: string, candidates: Array<{ personId: string; whyInteresting?: string; suggestedTags?: string[] }>) =>
-    fetchJson<{ dismissed: number }>(`/ai-discovery/${dbId}/dismiss-batch`, {
-      method: 'POST',
-      body: JSON.stringify({ candidates })
-    }),
+  dismissDiscoveryBatch: async (dbId: string, candidates: Array<{ personId: string; whyInteresting?: string; suggestedTags?: string[] }>): Promise<{ dismissed: number }> => {
+    const CHUNK_SIZE = 1000;
+    let totalDismissed = 0;
+    for (let i = 0; i < candidates.length; i += CHUNK_SIZE) {
+      const chunk = candidates.slice(i, i + CHUNK_SIZE);
+      const result = await fetchJson<{ dismissed: number }>(`/ai-discovery/${dbId}/dismiss-batch`, {
+        method: 'POST',
+        body: JSON.stringify({ candidates: chunk })
+      });
+      totalDismissed += result.dismissed;
+    }
+    return { dismissed: totalDismissed };
+  },
 
   getDismissedCandidates: (dbId: string) =>
     fetchJson<{ dismissed: DismissedCandidate[]; count: number }>(`/ai-discovery/${dbId}/dismissed`),

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

@@ -207,7 +207,7 @@ router.delete('/:dbId/dismissed', async (req: Request, res: Response) => {
  * Debug endpoints - gated behind ENABLE_AI_DEBUG env var or non-production mode.
  * These expose AI run metadata, prompts, and outputs.
  */
-const debugEnabled = process.env.ENABLE_AI_DEBUG === '1' || process.env.NODE_ENV !== 'production';
+const debugEnabled = process.env.ENABLE_AI_DEBUG === '1';
 
 /**
  * Get recent AI run logs for debugging

server/src/routes/map.routes.ts

@@ -41,7 +41,7 @@ mapRouter.post('/geocode/reset-not-found', (_req: Request, res: Response) => {
  * Use POST /geocode/reset-not-found first to retry previously failed places.
  */
 mapRouter.get('/geocode/stream', async (req: Request, res: Response) => {
-  const dbId = req.query.dbId as string;
+  const dbId = typeof req.query.dbId === 'string' ? req.query.dbId : '';
 
   if (!dbId) {
     res.status(400).json({ success: false, error: 'dbId query param required' });

server/src/services/ai-discovery.service.ts

@@ -550,10 +550,12 @@ export const aiDiscoveryService = {
     candidates: Array<{ personId: string; whyInteresting?: string; suggestedTags?: string[] }>
   ): { dismissed: number } {
     let dismissed = 0;
-    for (const candidate of candidates) {
-      this.dismissCandidate(dbId, candidate.personId, candidate.whyInteresting, candidate.suggestedTags);
-      dismissed++;
-    }
+    sqliteService.transaction(() => {
+      for (const candidate of candidates) {
+        this.dismissCandidate(dbId, candidate.personId, candidate.whyInteresting, candidate.suggestedTags);
+        dismissed++;
+      }
+    });
     return { dismissed };
   },