Skip to content

Commit 2f69e6f

Browse files
authored
Merge pull request #143 from atomantic/cos/sys-a5765c61-feature-ideas-msa7a2gn/agent-a34e3f0c
feat: audit cross-provider place conflicts
2 parents b684401 + 7f24196 commit 2f69e6f

6 files changed

Lines changed: 317 additions & 25 deletions

File tree

‎.changelog/NEXT.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
- Nominatim geocoding requests now time out after 15 seconds and are cancelled when their map-stream client disconnects, preventing stalled upstream sockets from blocking the shared geocoding queue.
5757
- AI discovery now rejects unsafe batch settings, limits background runs to one per family database, and lets an active run be cancelled without leaving provider work behind.
5858
- **[issue-158] Unknown API routes now return JSON errors** — Requests to unrecognized `/api` paths receive a stable 404 error envelope instead of the browser app's HTML, while client-side navigation continues to use the SPA fallback.
59+
- The tree auditor now detects genuine birth/death/etc. place disagreements across providers. It reuses the place comparison normalization, so aliases such as `TX`/`Texas`, country synonyms, and a provider's less-detailed place do not create review noise. Date mismatch checks now likewise require a disagreement between sources rather than conflicting values from one source alone.
5960
- Search results now keep their alphabetical ordering. The batch person-loader (`getPersonsBatch`) re-orders rows back to the requested order, fixing a regression where SQLite's `WHERE person_id IN (...)` returned rows in table order and silently discarded the search query's `ORDER BY display_name` (so the default, unsorted search view appeared randomly ordered).
6061
- Platform comparison now treats equivalent place spellings as matches: "Dallas, Texas, USA" vs "Dallas, Texas, United States" (and U.S.A. / United States of America / state abbreviations like TX vs Texas, UK vs United Kingdom, etc.) — no longer flagged as `different`. Place containment is now suffix-based, so "Texas" no longer falsely matches "Texarkana"
6162
- Platform comparison now treats equivalent date formats as matches (e.g., "1979-07-31" vs "31 JUL 1979")

‎PLAN.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ For phase-by-phase implementation history, see [docs/roadmap.md](./docs/roadmap.
1010

1111
1. **Reverse god-file regression** — `PersonDetail.tsx` (1360), `ProviderDataTable.tsx` (1243), `database.service.ts` (1618), `auditor-agent.service.ts` (1233 — new), `multi-platform-comparison.service.ts` (1099), `api.ts` (1249), `VerticalFamilyView.tsx` (977), `favorites.service.ts` (872), `person.routes.ts` (1119). Extract `usePersonData` / `usePersonOverrides` hooks, `PhotoThumbnail` / `ComparisonCell` / `ProviderRow` sub-components, and split `database.service.ts` along entity lines. Split `auditor-agent.service.ts` into walker + per-check modules.
1212
2. **Critical-path unit tests** — `credentials.service.ts` (encryption), `validation.ts` (input sanitization), `errorHandler.ts`, `requestTimeout.ts`, `augmentation.service.ts`, `auditor-agent.service.ts` all currently have **zero** tests. (`database.service.ts` and `search.service.ts` now have integration coverage but no unit tests of internal helpers.)
13-
3. **Phase 18 remaining checks** — implement `place_mismatch`, `name_mismatch`, `missing_parents`, `duplicate_suspect`, `stale_record` checks in `auditor-agent.service.ts` (types are declared in `shared/src/index.ts:918` but not yet wired). Reuse `multi-platform-comparison.service.ts` for the `*_mismatch` family.
13+
3. **Phase 18 remaining checks** — implement `name_mismatch`, `missing_parents`, `duplicate_suspect`, `stale_record` checks in `auditor-agent.service.ts` (types are declared in `shared/src/index.ts:918` but not yet wired). `place_mismatch` now detects only genuine cross-source vital-place conflicts, reusing the comparison service's alias and detail normalization to avoid noisy results.
1414
4. ~~**Search N+1**~~ — done. `searchWithSqlite` now uses `getPersonsBatch()` (6 queries vs 7×N), the batch loader preserves caller order (was silently dropping `ORDER BY display_name` because SQLite `IN (...)` returns table order), and the two dead N+1 service methods (`quickSearch`, `searchGlobal`) were removed. _Deferred:_ `getPersonsBatch()` does not populate `externalId` the way `getPerson()` does — fine for current consumers (SearchPage doesn't render it), but restore parity (one batched `external_identity` query) if a batch consumer ever needs it.
1515
5. **Phase 19 Guided Verification** — review-session schema (`verification_session`, `person_review`, `edge_review`, `provider_match_review`, `review_decision`) and root-to-ancestor BFS review queue (19.1 + 19.2).
1616

‎server/src/services/auditor-agent.service.ts‎

Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ import { sqliteService } from '../db/sqlite.service.js';
2424
import { resolveDbId } from './database.service.js';
2525
import { logger } from '../lib/logger.js';
2626
import { createOperationTracker } from '../utils/operationTracker.js';
27+
import {
28+
findCrossSourceMismatches,
29+
type EventSourceValue,
30+
} from '../utils/auditMismatches.js';
31+
import { placeContains, placesMatch } from '../utils/normalizePlace.js';
32+
import { getCachedProviderVitalValues } from '../utils/auditProviderVitals.js';
2733
import config from '../lib/config.js';
2834

2935
const tracker = createOperationTracker('auditor');
@@ -518,40 +524,67 @@ function checkUnlinkedProviders(
518524
return { issues, linkedSources };
519525
}
520526

521-
function checkDateMismatches(runId: string, personId: string, displayName: string): AuditIssue[] {
527+
function getCrossSourceVitalValues(personId: string): EventSourceValue[] {
522528
const events = sqliteService.queryAll<{
523529
event_type: string;
524530
date_year: number | null;
525-
source: string;
531+
place: string | null;
532+
source: string | null;
526533
}>(
527-
`SELECT event_type, date_year, source FROM vital_event
528-
WHERE person_id = @personId AND date_year IS NOT NULL
534+
`SELECT event_type, date_year, place, source FROM vital_event
535+
WHERE person_id = @personId
529536
ORDER BY event_type, source`,
530537
{ personId }
531538
);
532539

533-
const issues: AuditIssue[] = [];
534-
const byType = new Map<string, { year: number; source: string }[]>();
540+
const values: EventSourceValue[] = events.flatMap(event => {
541+
const eventValues: EventSourceValue[] = [];
542+
if (event.date_year !== null) {
543+
eventValues.push({ eventType: event.event_type, value: event.date_year, source: event.source });
544+
}
545+
const place = event.place?.trim();
546+
if (place) eventValues.push({ eventType: event.event_type, value: place, source: event.source });
547+
return eventValues;
548+
});
535549

536-
for (const e of events) {
537-
if (!e.date_year) continue;
538-
const list = byType.get(e.event_type) ?? [];
539-
list.push({ year: e.date_year, source: e.source ?? 'unknown' });
540-
byType.set(e.event_type, list);
541-
}
550+
return [...values, ...getCachedProviderVitalValues(personId)];
551+
}
542552

543-
for (const [eventType, entries] of byType) {
544-
if (entries.length < 2) continue;
545-
const years = new Set(entries.map(e => e.year));
546-
if (years.size > 1) {
547-
const details = entries.map(e => `${e.source}: ${e.year}`).join(', ');
548-
issues.push(makeIssue(runId, personId, 'date_mismatch', 'warning',
549-
`${displayName}: ${eventType} date differs across sources (${details})`,
550-
details, null));
551-
}
552-
}
553+
function checkDateMismatches(
554+
runId: string,
555+
personId: string,
556+
displayName: string,
557+
events: EventSourceValue[],
558+
): AuditIssue[] {
559+
const dateEvents = events
560+
.filter((event): event is EventSourceValue & { value: number } => typeof event.value === 'number');
553561

554-
return issues;
562+
return findCrossSourceMismatches(dateEvents, (left, right) => left === right)
563+
.map(({ eventType, details }) => makeIssue(runId, personId, 'date_mismatch', 'warning',
564+
`${displayName}: ${eventType} date differs across sources (${details})`,
565+
details, null));
566+
}
567+
568+
function checkPlaceMismatches(
569+
runId: string,
570+
personId: string,
571+
displayName: string,
572+
events: EventSourceValue[],
573+
): AuditIssue[] {
574+
const placeEvents = events
575+
.filter((event): event is EventSourceValue & { value: string } => typeof event.value === 'string');
576+
577+
const samePlace = (left: string | number, right: string | number) => {
578+
const a = String(left);
579+
const b = String(right);
580+
return placesMatch(a, b) || placeContains(a, b) || placeContains(b, a);
581+
};
582+
583+
return findCrossSourceMismatches(placeEvents, samePlace).map(({ eventType, details }) => makeIssue(
584+
runId, personId, 'place_mismatch', 'warning',
585+
`${displayName}: ${eventType} place differs across sources (${details})`,
586+
details, null,
587+
));
555588
}
556589

557590
// ============================================================================
@@ -787,6 +820,7 @@ const DEFAULT_CONFIG: AuditRunConfig = {
787820
'missing_gender',
788821
'orphaned_edge',
789822
'date_mismatch',
823+
'place_mismatch',
790824
],
791825
autoAccept: false,
792826
autoAcceptTypes: [],
@@ -811,6 +845,9 @@ function auditPerson(
811845

812846
const issues: AuditIssue[] = [];
813847
let linkedSources = new Set<string>();
848+
const crossSourceVitalValues = checksEnabled.includes('date_mismatch') || checksEnabled.includes('place_mismatch')
849+
? getCrossSourceVitalValues(vitals.personId)
850+
: [];
814851

815852
if (checksEnabled.includes('impossible_date')) {
816853
issues.push(...checkImpossibleDates(runId, vitals));
@@ -840,7 +877,10 @@ function auditPerson(
840877
linkedSources = new Set(linked.map(l => l.source));
841878
}
842879
if (checksEnabled.includes('date_mismatch')) {
843-
issues.push(...checkDateMismatches(runId, vitals.personId, vitals.displayName));
880+
issues.push(...checkDateMismatches(runId, vitals.personId, vitals.displayName, crossSourceVitalValues));
881+
}
882+
if (checksEnabled.includes('place_mismatch')) {
883+
issues.push(...checkPlaceMismatches(runId, vitals.personId, vitals.displayName, crossSourceVitalValues));
844884
}
845885

846886
return { issues, displayName: vitals.displayName, linkedSources };
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* Helpers for detecting disagreements between provider-sourced vital events.
3+
*
4+
* A disagreement must be between sources. Multiple values from one source are
5+
* not enough evidence to ask the user to resolve a cross-provider conflict.
6+
*/
7+
8+
import { parseYear } from './parseYear.js';
9+
10+
export interface EventSourceValue {
11+
eventType: string;
12+
value: string | number;
13+
source: string | null;
14+
}
15+
16+
export interface EventMismatch {
17+
eventType: string;
18+
details: string;
19+
}
20+
21+
type CachedProviderVitalData = {
22+
provider: string;
23+
scrapedData?: {
24+
birth?: { date?: string; place?: string };
25+
death?: { date?: string; place?: string };
26+
};
27+
};
28+
29+
type ValuesMatch = (a: string | number, b: string | number) => boolean;
30+
31+
/**
32+
* Convert a cached provider record into the vital values used by the auditor.
33+
* Cached scrapes are deliberately kept separate from the local vital_event
34+
* table until a user explicitly applies them, but they are still evidence for
35+
* a read-only cross-provider audit.
36+
*/
37+
export function cachedProviderVitalValues(cache: CachedProviderVitalData): EventSourceValue[] {
38+
const values: EventSourceValue[] = [];
39+
40+
for (const [eventType, event] of [
41+
['birth', cache.scrapedData?.birth],
42+
['death', cache.scrapedData?.death],
43+
] as const) {
44+
const year = parseYear(event?.date);
45+
if (year !== null) values.push({ eventType, value: year, source: cache.provider });
46+
47+
const place = event?.place?.trim();
48+
if (place) values.push({ eventType, value: place, source: cache.provider });
49+
}
50+
51+
return values;
52+
}
53+
54+
function sourceName(source: string | null): string {
55+
return source?.trim() || 'local';
56+
}
57+
58+
function formatSourceValues(entries: EventSourceValue[]): string {
59+
const bySource = new Map<string, Set<string>>();
60+
61+
for (const entry of entries) {
62+
const source = sourceName(entry.source);
63+
const values = bySource.get(source) ?? new Set<string>();
64+
values.add(String(entry.value));
65+
bySource.set(source, values);
66+
}
67+
68+
return [...bySource.entries()]
69+
.sort(([a], [b]) => a.localeCompare(b))
70+
.map(([source, values]) => `${source}: ${[...values].sort().join(' / ')}`)
71+
.join(', ');
72+
}
73+
74+
/**
75+
* Find event types where at least two sources have no matching value.
76+
*
77+
* A source can contain more than one value (notably legacy records with a
78+
* null source). We suppress a mismatch when the two sources share any value,
79+
* because there is no unambiguous cross-provider disagreement to resolve.
80+
*/
81+
export function findCrossSourceMismatches(
82+
entries: EventSourceValue[],
83+
valuesMatch: ValuesMatch,
84+
): EventMismatch[] {
85+
const byEventType = new Map<string, EventSourceValue[]>();
86+
87+
for (const entry of entries) {
88+
const values = byEventType.get(entry.eventType) ?? [];
89+
values.push(entry);
90+
byEventType.set(entry.eventType, values);
91+
}
92+
93+
const mismatches: EventMismatch[] = [];
94+
95+
for (const [eventType, eventEntries] of byEventType) {
96+
const bySource = new Map<string, EventSourceValue[]>();
97+
for (const entry of eventEntries) {
98+
const source = sourceName(entry.source);
99+
const values = bySource.get(source) ?? [];
100+
values.push(entry);
101+
bySource.set(source, values);
102+
}
103+
104+
const sourceValues = [...bySource.values()];
105+
const hasDisagreement = sourceValues.some((values, index) =>
106+
sourceValues.slice(index + 1).some(otherValues =>
107+
values.every(value => otherValues.every(other => !valuesMatch(value.value, other.value)))
108+
)
109+
);
110+
111+
if (hasDisagreement) {
112+
mismatches.push({ eventType, details: formatSourceValues(eventEntries) });
113+
}
114+
}
115+
116+
return mismatches;
117+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import type { BuiltInProvider } from '@fsf/shared';
2+
import { sqliteService } from '../db/sqlite.service.js';
3+
import { logger } from '../lib/logger.js';
4+
import { json2person } from '../lib/familysearch/index.js';
5+
import { getCachedProviderData } from './providerCache.js';
6+
import { cachedProviderVitalValues, type EventSourceValue } from './auditMismatches.js';
7+
8+
const BUILT_IN_PROVIDERS = new Set<BuiltInProvider>([
9+
'familysearch', 'ancestry', 'wikitree', '23andme',
10+
]);
11+
12+
function isBuiltInProvider(source: string): source is BuiltInProvider {
13+
return BUILT_IN_PROVIDERS.has(source as BuiltInProvider);
14+
}
15+
16+
export function providerCacheVitalValues(source: BuiltInProvider, cache: unknown): EventSourceValue[] {
17+
const record = cache as { scrapedData?: unknown };
18+
if (record.scrapedData) return cachedProviderVitalValues(cache as Parameters<typeof cachedProviderVitalValues>[0]);
19+
if (source !== 'familysearch') return [];
20+
21+
const person = json2person(cache);
22+
return cachedProviderVitalValues({
23+
provider: source,
24+
scrapedData: {
25+
birth: person?.birth,
26+
death: person?.death,
27+
},
28+
});
29+
}
30+
31+
/** Read provider-cache vitals for the linked external identities of a person. */
32+
export function getCachedProviderVitalValues(personId: string): EventSourceValue[] {
33+
const links = sqliteService.queryAll<{ source: string; external_id: string }>(
34+
'SELECT source, external_id FROM external_identity WHERE person_id = @personId',
35+
{ personId },
36+
);
37+
38+
return links.flatMap(link => {
39+
if (!isBuiltInProvider(link.source)) return [];
40+
try {
41+
const cache = getCachedProviderData(link.source, link.external_id);
42+
return cache ? providerCacheVitalValues(link.source, cache) : [];
43+
} catch (error) {
44+
logger.warn('auditor', `Ignoring unreadable ${link.source} cache for ${personId}: ${String(error)}`);
45+
return [];
46+
}
47+
});
48+
}

0 commit comments

Comments
 (0)