Skip to content

Commit 8bddca1

Browse files
committed
fix: address PR review feedback round 4
- Add max batch size (1000) validation for apply-batch and dismiss-batch - Convert correlated subqueries to JOINs in getPersonsWithPlaces - Add error handling for SSE geocode stream with error event to client
1 parent fdec014 commit 8bddca1

3 files changed

Lines changed: 29 additions & 11 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ router.post('/:dbId/apply-batch', async (req: Request, res: Response) => {
107107
res.status(400).json({ success: false, error: 'candidates array is required' });
108108
return;
109109
}
110+
if (candidates.length > 1000) {
111+
res.status(400).json({ success: false, error: 'Maximum 1000 candidates per batch' });
112+
return;
113+
}
110114

111115
let applied = 0;
112116
for (const candidate of candidates) {
@@ -159,6 +163,10 @@ router.post('/:dbId/dismiss-batch', async (req: Request, res: Response) => {
159163
res.status(400).json({ success: false, error: 'candidates array is required' });
160164
return;
161165
}
166+
if (candidates.length > 1000) {
167+
res.status(400).json({ success: false, error: 'Maximum 1000 candidates per batch' });
168+
return;
169+
}
162170

163171
const result = aiDiscoveryService.dismissCandidatesBatch(dbId, candidates);
164172
res.json({ success: true, data: result });

‎server/src/routes/map.routes.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,21 @@ mapRouter.get('/geocode/stream', async (req: Request, res: Response) => {
7979
let cancelled = false;
8080
req.on('close', () => { cancelled = true; });
8181

82-
for await (const progress of geocodeService.batchGeocode(placesToGeocode)) {
83-
if (cancelled) break;
84-
res.write(`data: ${JSON.stringify(progress)}\n\n`);
85-
}
86-
87-
if (!cancelled) {
82+
const streamResult = await (async () => {
83+
for await (const progress of geocodeService.batchGeocode(placesToGeocode)) {
84+
if (cancelled) return 'cancelled';
85+
res.write(`data: ${JSON.stringify(progress)}\n\n`);
86+
}
87+
return 'ok';
88+
})().catch((err: Error) => {
89+
logger.api('map', `❌ Batch geocode stream error: ${err.message}`);
90+
if (!cancelled) {
91+
res.write(`data: ${JSON.stringify({ type: 'error', message: err.message })}\n\n`);
92+
}
93+
return 'error';
94+
});
95+
96+
if (streamResult === 'ok') {
8897
res.write(`data: ${JSON.stringify({ type: 'complete', current: placesToGeocode.length, total: placesToGeocode.length })}\n\n`);
8998
}
9099

‎server/src/services/map.service.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,13 @@ function getPersonsWithPlaces(personIds: string[]): Map<string, {
4646
death_year: number | null;
4747
}>(
4848
`SELECT p.person_id, p.display_name, p.gender,
49-
(SELECT place FROM vital_event WHERE person_id = p.person_id AND event_type = 'birth' LIMIT 1) as birth_place,
50-
(SELECT date_year FROM vital_event WHERE person_id = p.person_id AND event_type = 'birth' LIMIT 1) as birth_year,
51-
(SELECT place FROM vital_event WHERE person_id = p.person_id AND event_type = 'death' LIMIT 1) as death_place,
52-
(SELECT date_year FROM vital_event WHERE person_id = p.person_id AND event_type = 'death' LIMIT 1) as death_year
49+
vb.place AS birth_place, vb.date_year AS birth_year,
50+
vd.place AS death_place, vd.date_year AS death_year
5351
FROM person p
54-
WHERE p.person_id IN (${placeholders})`,
52+
LEFT JOIN vital_event vb ON vb.person_id = p.person_id AND vb.event_type = 'birth'
53+
LEFT JOIN vital_event vd ON vd.person_id = p.person_id AND vd.event_type = 'death'
54+
WHERE p.person_id IN (${placeholders})
55+
GROUP BY p.person_id`,
5556
params
5657
);
5758

0 commit comments

Comments
 (0)