diff --git a/.changelog/NEXT.md b/.changelog/NEXT.md index 77a969d7..58c5ea7e 100644 --- a/.changelog/NEXT.md +++ b/.changelog/NEXT.md @@ -49,6 +49,7 @@ ## Fixed +- 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. - 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). - 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" - Platform comparison now treats equivalent date formats as matches (e.g., "1979-07-31" vs "31 JUL 1979") diff --git a/PLAN.md b/PLAN.md index 8400aab8..79f81b08 100644 --- a/PLAN.md +++ b/PLAN.md @@ -10,7 +10,7 @@ For phase-by-phase implementation history, see [docs/roadmap.md](./docs/roadmap. 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. 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.) -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. +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. 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. 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). diff --git a/server/src/services/auditor-agent.service.ts b/server/src/services/auditor-agent.service.ts index 60908496..f15b217c 100644 --- a/server/src/services/auditor-agent.service.ts +++ b/server/src/services/auditor-agent.service.ts @@ -24,6 +24,12 @@ import { sqliteService } from '../db/sqlite.service.js'; import { resolveDbId } from './database.service.js'; import { logger } from '../lib/logger.js'; import { createOperationTracker } from '../utils/operationTracker.js'; +import { + findCrossSourceMismatches, + type EventSourceValue, +} from '../utils/auditMismatches.js'; +import { placeContains, placesMatch } from '../utils/normalizePlace.js'; +import { getCachedProviderVitalValues } from '../utils/auditProviderVitals.js'; import config from '../lib/config.js'; const tracker = createOperationTracker('auditor'); @@ -518,40 +524,67 @@ function checkUnlinkedProviders( return { issues, linkedSources }; } -function checkDateMismatches(runId: string, personId: string, displayName: string): AuditIssue[] { +function getCrossSourceVitalValues(personId: string): EventSourceValue[] { const events = sqliteService.queryAll<{ event_type: string; date_year: number | null; - source: string; + place: string | null; + source: string | null; }>( - `SELECT event_type, date_year, source FROM vital_event - WHERE person_id = @personId AND date_year IS NOT NULL + `SELECT event_type, date_year, place, source FROM vital_event + WHERE person_id = @personId ORDER BY event_type, source`, { personId } ); - const issues: AuditIssue[] = []; - const byType = new Map(); + const values: EventSourceValue[] = events.flatMap(event => { + const eventValues: EventSourceValue[] = []; + if (event.date_year !== null) { + eventValues.push({ eventType: event.event_type, value: event.date_year, source: event.source }); + } + const place = event.place?.trim(); + if (place) eventValues.push({ eventType: event.event_type, value: place, source: event.source }); + return eventValues; + }); - for (const e of events) { - if (!e.date_year) continue; - const list = byType.get(e.event_type) ?? []; - list.push({ year: e.date_year, source: e.source ?? 'unknown' }); - byType.set(e.event_type, list); - } + return [...values, ...getCachedProviderVitalValues(personId)]; +} - for (const [eventType, entries] of byType) { - if (entries.length < 2) continue; - const years = new Set(entries.map(e => e.year)); - if (years.size > 1) { - const details = entries.map(e => `${e.source}: ${e.year}`).join(', '); - issues.push(makeIssue(runId, personId, 'date_mismatch', 'warning', - `${displayName}: ${eventType} date differs across sources (${details})`, - details, null)); - } - } +function checkDateMismatches( + runId: string, + personId: string, + displayName: string, + events: EventSourceValue[], +): AuditIssue[] { + const dateEvents = events + .filter((event): event is EventSourceValue & { value: number } => typeof event.value === 'number'); - return issues; + return findCrossSourceMismatches(dateEvents, (left, right) => left === right) + .map(({ eventType, details }) => makeIssue(runId, personId, 'date_mismatch', 'warning', + `${displayName}: ${eventType} date differs across sources (${details})`, + details, null)); +} + +function checkPlaceMismatches( + runId: string, + personId: string, + displayName: string, + events: EventSourceValue[], +): AuditIssue[] { + const placeEvents = events + .filter((event): event is EventSourceValue & { value: string } => typeof event.value === 'string'); + + const samePlace = (left: string | number, right: string | number) => { + const a = String(left); + const b = String(right); + return placesMatch(a, b) || placeContains(a, b) || placeContains(b, a); + }; + + return findCrossSourceMismatches(placeEvents, samePlace).map(({ eventType, details }) => makeIssue( + runId, personId, 'place_mismatch', 'warning', + `${displayName}: ${eventType} place differs across sources (${details})`, + details, null, + )); } // ============================================================================ @@ -787,6 +820,7 @@ const DEFAULT_CONFIG: AuditRunConfig = { 'missing_gender', 'orphaned_edge', 'date_mismatch', + 'place_mismatch', ], autoAccept: false, autoAcceptTypes: [], @@ -811,6 +845,9 @@ function auditPerson( const issues: AuditIssue[] = []; let linkedSources = new Set(); + const crossSourceVitalValues = checksEnabled.includes('date_mismatch') || checksEnabled.includes('place_mismatch') + ? getCrossSourceVitalValues(vitals.personId) + : []; if (checksEnabled.includes('impossible_date')) { issues.push(...checkImpossibleDates(runId, vitals)); @@ -840,7 +877,10 @@ function auditPerson( linkedSources = new Set(linked.map(l => l.source)); } if (checksEnabled.includes('date_mismatch')) { - issues.push(...checkDateMismatches(runId, vitals.personId, vitals.displayName)); + issues.push(...checkDateMismatches(runId, vitals.personId, vitals.displayName, crossSourceVitalValues)); + } + if (checksEnabled.includes('place_mismatch')) { + issues.push(...checkPlaceMismatches(runId, vitals.personId, vitals.displayName, crossSourceVitalValues)); } return { issues, displayName: vitals.displayName, linkedSources }; diff --git a/server/src/utils/auditMismatches.ts b/server/src/utils/auditMismatches.ts new file mode 100644 index 00000000..35fdf623 --- /dev/null +++ b/server/src/utils/auditMismatches.ts @@ -0,0 +1,117 @@ +/** + * Helpers for detecting disagreements between provider-sourced vital events. + * + * A disagreement must be between sources. Multiple values from one source are + * not enough evidence to ask the user to resolve a cross-provider conflict. + */ + +import { parseYear } from './parseYear.js'; + +export interface EventSourceValue { + eventType: string; + value: string | number; + source: string | null; +} + +export interface EventMismatch { + eventType: string; + details: string; +} + +type CachedProviderVitalData = { + provider: string; + scrapedData?: { + birth?: { date?: string; place?: string }; + death?: { date?: string; place?: string }; + }; +}; + +type ValuesMatch = (a: string | number, b: string | number) => boolean; + +/** + * Convert a cached provider record into the vital values used by the auditor. + * Cached scrapes are deliberately kept separate from the local vital_event + * table until a user explicitly applies them, but they are still evidence for + * a read-only cross-provider audit. + */ +export function cachedProviderVitalValues(cache: CachedProviderVitalData): EventSourceValue[] { + const values: EventSourceValue[] = []; + + for (const [eventType, event] of [ + ['birth', cache.scrapedData?.birth], + ['death', cache.scrapedData?.death], + ] as const) { + const year = parseYear(event?.date); + if (year !== null) values.push({ eventType, value: year, source: cache.provider }); + + const place = event?.place?.trim(); + if (place) values.push({ eventType, value: place, source: cache.provider }); + } + + return values; +} + +function sourceName(source: string | null): string { + return source?.trim() || 'local'; +} + +function formatSourceValues(entries: EventSourceValue[]): string { + const bySource = new Map>(); + + for (const entry of entries) { + const source = sourceName(entry.source); + const values = bySource.get(source) ?? new Set(); + values.add(String(entry.value)); + bySource.set(source, values); + } + + return [...bySource.entries()] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([source, values]) => `${source}: ${[...values].sort().join(' / ')}`) + .join(', '); +} + +/** + * Find event types where at least two sources have no matching value. + * + * A source can contain more than one value (notably legacy records with a + * null source). We suppress a mismatch when the two sources share any value, + * because there is no unambiguous cross-provider disagreement to resolve. + */ +export function findCrossSourceMismatches( + entries: EventSourceValue[], + valuesMatch: ValuesMatch, +): EventMismatch[] { + const byEventType = new Map(); + + for (const entry of entries) { + const values = byEventType.get(entry.eventType) ?? []; + values.push(entry); + byEventType.set(entry.eventType, values); + } + + const mismatches: EventMismatch[] = []; + + for (const [eventType, eventEntries] of byEventType) { + const bySource = new Map(); + for (const entry of eventEntries) { + const source = sourceName(entry.source); + const values = bySource.get(source) ?? []; + values.push(entry); + bySource.set(source, values); + } + + const sourceValues = [...bySource.values()]; + const hasDisagreement = sourceValues.some((values, index) => + sourceValues.slice(index + 1).some(otherValues => + values.every(value => otherValues.every(other => !valuesMatch(value.value, other.value))) + ) + ); + + if (hasDisagreement) { + mismatches.push({ eventType, details: formatSourceValues(eventEntries) }); + } + } + + return mismatches; +} diff --git a/server/src/utils/auditProviderVitals.ts b/server/src/utils/auditProviderVitals.ts new file mode 100644 index 00000000..26d55b9a --- /dev/null +++ b/server/src/utils/auditProviderVitals.ts @@ -0,0 +1,48 @@ +import type { BuiltInProvider } from '@fsf/shared'; +import { sqliteService } from '../db/sqlite.service.js'; +import { logger } from '../lib/logger.js'; +import { json2person } from '../lib/familysearch/index.js'; +import { getCachedProviderData } from './providerCache.js'; +import { cachedProviderVitalValues, type EventSourceValue } from './auditMismatches.js'; + +const BUILT_IN_PROVIDERS = new Set([ + 'familysearch', 'ancestry', 'wikitree', '23andme', +]); + +function isBuiltInProvider(source: string): source is BuiltInProvider { + return BUILT_IN_PROVIDERS.has(source as BuiltInProvider); +} + +export function providerCacheVitalValues(source: BuiltInProvider, cache: unknown): EventSourceValue[] { + const record = cache as { scrapedData?: unknown }; + if (record.scrapedData) return cachedProviderVitalValues(cache as Parameters[0]); + if (source !== 'familysearch') return []; + + const person = json2person(cache); + return cachedProviderVitalValues({ + provider: source, + scrapedData: { + birth: person?.birth, + death: person?.death, + }, + }); +} + +/** Read provider-cache vitals for the linked external identities of a person. */ +export function getCachedProviderVitalValues(personId: string): EventSourceValue[] { + const links = sqliteService.queryAll<{ source: string; external_id: string }>( + 'SELECT source, external_id FROM external_identity WHERE person_id = @personId', + { personId }, + ); + + return links.flatMap(link => { + if (!isBuiltInProvider(link.source)) return []; + try { + const cache = getCachedProviderData(link.source, link.external_id); + return cache ? providerCacheVitalValues(link.source, cache) : []; + } catch (error) { + logger.warn('auditor', `Ignoring unreadable ${link.source} cache for ${personId}: ${String(error)}`); + return []; + } + }); +} diff --git a/tests/unit/utils/auditMismatches.spec.ts b/tests/unit/utils/auditMismatches.spec.ts new file mode 100644 index 00000000..f02ec8bd --- /dev/null +++ b/tests/unit/utils/auditMismatches.spec.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest'; +import { cachedProviderVitalValues, findCrossSourceMismatches } from '../../../server/src/utils/auditMismatches.js'; +import { providerCacheVitalValues } from '../../../server/src/utils/auditProviderVitals.js'; +import { placeContains, placesMatch } from '../../../server/src/utils/normalizePlace.js'; + +describe('findCrossSourceMismatches', () => { + it('turns cached scraped provider vitals into source-labelled audit values', () => { + expect(cachedProviderVitalValues({ + provider: 'ancestry', + scrapedData: { + birth: { date: 'about 1875', place: 'Houston, Texas' }, + death: { date: '1930' }, + }, + })).toEqual([ + { eventType: 'birth', value: 1875, source: 'ancestry' }, + { eventType: 'birth', value: 'Houston, Texas', source: 'ancestry' }, + { eventType: 'death', value: 1930, source: 'ancestry' }, + ]); + }); + + it('reads raw FamilySearch refresh caches before comparing providers', () => { + expect(providerCacheVitalValues('familysearch', { + persons: [{ + display: { name: 'Ada Example' }, + facts: [{ + type: 'http://gedcomx.org/Birth', + date: { original: '1874' }, + place: { original: 'Dallas, Texas' }, + }], + }], + })).toEqual([ + { eventType: 'birth', value: 1874, source: 'familysearch' }, + { eventType: 'birth', value: 'Dallas, Texas', source: 'familysearch' }, + ]); + }); + + it('reports a disagreement between two provider values', () => { + const mismatches = findCrossSourceMismatches([ + { eventType: 'birth', value: 1874, source: 'familysearch' }, + { eventType: 'birth', value: 1875, source: 'ancestry' }, + ], (a, b) => a === b); + + expect(mismatches).toEqual([{ + eventType: 'birth', + details: 'ancestry: 1875, familysearch: 1874', + }]); + }); + + it('does not report values that only disagree within one source', () => { + const mismatches = findCrossSourceMismatches([ + { eventType: 'birth', value: 1874, source: null }, + { eventType: 'birth', value: 1875, source: null }, + ], (a, b) => a === b); + + expect(mismatches).toEqual([]); + }); + + it('suppresses normalized aliases and detail-only place differences', () => { + const samePlace = (a: string | number, b: string | number) => { + const left = String(a); + const right = String(b); + return placesMatch(left, right) || placeContains(left, right) || placeContains(right, left); + }; + + const mismatches = findCrossSourceMismatches([ + { eventType: 'birth', value: 'Dallas, TX, USA', source: 'familysearch' }, + { eventType: 'birth', value: 'Dallas, Texas, United States', source: 'ancestry' }, + { eventType: 'death', value: 'Texas, USA', source: 'familysearch' }, + { eventType: 'death', value: 'Austin, Texas, United States', source: 'ancestry' }, + ], samePlace); + + expect(mismatches).toEqual([]); + }); + + it('reports genuinely different places from different sources', () => { + const mismatches = findCrossSourceMismatches([ + { eventType: 'birth', value: 'Dallas, Texas, USA', source: 'familysearch' }, + { eventType: 'birth', value: 'Houston, Texas, USA', source: 'ancestry' }, + ], (a, b) => placesMatch(String(a), String(b))); + + expect(mismatches).toEqual([{ + eventType: 'birth', + details: 'ancestry: Houston, Texas, USA, familysearch: Dallas, Texas, USA', + }]); + }); +});