[Improvement: SparseTree] Implement Next Planned Feature (#70)

* feat(relationships): add manual relationship linking for parents, spouses, and children

Implements Phase 15.20 - users can now link existing people or create new
person stubs as parents, spouses, or children from the PersonDetail page.

- Add link-relationship/unlink-relationship API endpoints with validation
- Add quick-search endpoint (FTS5 with database scoping) for modal search
- Add createPersonStub to id-mapping service for manual person creation
- Create RelationshipModal with search + create tabs, debounced search
- Add "+ Add" buttons to Parents, Spouses, and Children sections
- Replace placeholder modal with functional RelationshipModal

* docs: mark Phase 15.20 (relationship linking) as complete

* fix(relationships): address PR #70 review feedback

Server fixes:
- quick-search: replace LEFT JOIN vital_event with subquery (MIN+GROUP BY)
  to prevent duplicate rows when persons have multiple birth records
- link-relationship: scope source/target to dbId, reject cross-database
  links, add new stubs to database_membership in same transaction
- link-relationship: validate gender against CHECK constraint values
  (male/female/unknown), coerce based on relationshipType when invalid
- link-relationship: wrap stub creation + membership + edge insertion
  in a single transaction so failures roll back all writes
- link-relationship: pre-check duplicates before any writes; extract
  checkDuplicateEdge helper
- link/unlink-relationship: validate targetId as canonical ULID
- unlink-relationship: scope deletes via membership pre-checks; remove
  redundant EXISTS guards in DELETE statements for consistency
- createPersonStub: wrap insert + FTS update in transaction
- extract isPersonInDatabase helper shared by link/unlink

Client fixes:
- RelationshipModal: handle quickSearchPersons errors with toast,
  ensure searching state always clears via finally
- RelationshipModal: reset searching/linkingId on modal open so a
  re-opened modal never shows stale spinner
- RelationshipModal: drop unused color field from TYPE_CONFIG
- api.ts: type linkRelationship/unlinkRelationship as RelationshipType
  instead of string; move type to client/src/types/relationship.ts
- PersonDetail: extract reloadPerson helper; refresh parent/spouse/child
  data after linking instead of only updating the person object
- PersonDetail: handle errors in onLinked with toast feedback
- PersonDetail: determine missing parent role from gender, not array
  index (parents.filter(Boolean) collapses sparse positions)

* fix(relationships): address round 2 review feedback

- person.routes: resolve route :dbId to internal db_id via resolveDbId()
  in quick-search, link-relationship, and unlink-relationship handlers,
  matching the pattern used by integrity/auditor/search services. Without
  this, callers passing a legacy/FamilySearch ID silently fail membership
  checks and create orphan database_membership rows.
- quick-search: add ORDER BY display_name, person_id for deterministic
  autocomplete results
- PersonDetail: import RelationshipType from types/relationship instead
  of from RelationshipModal (reduces coupling)
- RelationshipModal: drop now-unused re-export of RelationshipType

* fix(relationships): address round 3 review feedback

- person.routes link-relationship: use INSERT OR IGNORE for edge inserts
  to defend against races between pre-check and write; check changes count
  and return 409 if no row was inserted (duplicate snuck in)
- person.routes link-relationship: refresh database_info.person_count
  cache when a new membership row is added so the database listing
  reflects new persons immediately
- api.ts: tighten linkRelationship/unlinkRelationship response types from
  string to RelationshipType for full type-safety end to end

* fix(relationships): address round 4 review feedback + add tests

- person.routes link-relationship: trim newPerson.name and reject blank
  or whitespace-only values to prevent stub persons with empty display names
- RelationshipModal: type onLinked as () => void | Promise<void> and
  await it before closing the modal, so the modal stays open visually
  until the parent's reload completes
- tests: add tests/integration/api/relationships.spec.ts with 17 cases
  covering quick-search (db scoping, query length, no-match), link
  (validation, self-link, cross-db, stub creation, duplicate, parent),
  and unlink (delete, 404, cross-db rejection)
- tests/integration/setup: register inline simplified versions of
  quick-search, link-relationship, and unlink-relationship endpoints
  matching production validation, scoping, and idempotent insert behavior

* fix(relationships): address round 5 review feedback

- person.routes link-relationship: insert membership only AFTER confirming
  edge insert was new. Previously, INSERT OR IGNORE racing with a
  concurrent edge insert would still write the membership row even when
  returning 409. Now race-induced 409s never mutate state.
- tests/setup link-relationship: same ordering fix; reword 'mirrors
  production validation' comment to explicitly call out divergences
  (no canonical-ULID checks, no resolveDbId, simple stub IDs)

* fix(relationships): guard doSearch against out-of-order responses

Track a monotonically increasing search request id and drop responses
whose id no longer matches the latest. Without this, fast typing could
let an earlier search resolve after a later one and overwrite results
or flip the spinner off prematurely.

* test(relationships): include birthYear in test quick-search response

Match production response shape so client/contract regressions on the
birthYear field are caught in tests.

* fix(relationships): guard against late results and mid-link unmount

- doSearch: bump request id (and clear searching) when query falls below
  2 chars, so a previously-issued >=2-char request resolving late cannot
  repopulate stale results
- RelationshipModal: introduce safeClose() that no-ops while a link/create
  is in flight, and disable the X close button when linking. Backdrop
  click and X button no longer unmount the modal mid-await, preventing
  setState/onLinked from running on a dead component

* fix(relationships): drop unused async on synchronous route handlers

quick-search, link-relationship, and unlink-relationship handlers do
only synchronous SQLite work but were marked async, which would cause
thrown exceptions to become unhandled Promise rejections in Express 4
(no express-async-errors registered). Removing async lets Express
catch synchronous throws via the standard error middleware.

* fix(relationships): reject cross-database link targets

Edge tables (parent_edge, spouse_edge) are not scoped by db_id, so
silently auto-importing an existing person into the source database
during link-relationship would leak relationships across databases.

- person.routes link-relationship: require existing targets to already
  be members of the source database; return 403 otherwise. Stub creation
  remains the only path that adds a new membership row, since stubs are
  brand-new persons that didn't exist before this request.
- tests/setup link-relationship: mirror the rejection
- tests: add coverage for the cross-database rejection (existing person
  with no membership in source db → 403 + no edge inserted)

* fix(relationships): handle unknown gender + birthYear=0 in modal

- PersonDetail parent inference: filter to known male/female genders
  before deciding which parent role is missing. Previously, an existing
  parent with gender 'unknown' or with parentData not yet loaded would
  fall through and default to 'father' even when the existing parent
  was actually the father, leading to a confusing duplicate error.
- RelationshipModal: use != null instead of truthy check for birthYear,
  so a valid birthYear of 0 isn't hidden and replaced with the truncated
  ID fallback

* fix(relationships): normalize quick-search query param

req.query.q may be string | string[] | undefined; calling .trim()
directly on the cast crashed with a 500 if a client sent multiple
?q= parameters. Normalize to the first value before length checks.

Author: atomantic

PLAN.md

@@ -36,7 +36,7 @@ High-level project roadmap. For detailed phase documentation, see [docs/roadmap.
 | 15.17 | Data integrity + bulk discovery | ✅ |
 | 15.18 | Separate provider download from auto-apply | ✅ |
 | 15.19 | Normalize FamilySearch as downstream provider | ✅ |
-| 15.20 | Relationship linking (parents, spouses, children) | 📋 |
+| 15.20 | Relationship linking (parents, spouses, children) | ✅ |
 | 15.22 | Ancestry free hints automation | ✅ |
 | 15.23 | Migration Map visualization | ✅ |
 | 16 | Multi-platform sync architecture | 📋 |

client/src/components/person/PersonDetail.tsx

@@ -14,6 +14,9 @@ import { UploadToFamilySearchDialog } from './UploadToFamilySearchDialog';
 import { UploadToAncestryDialog } from './UploadToAncestryDialog';
 import { ProviderDataTable } from './ProviderDataTable';
 import { LinkPlatformDialog } from './LinkPlatformDialog';
+import { RelationshipModal } from './RelationshipModal';
+import type { RelationshipType } from '../../types/relationship';
+
 import { PersonAuditIssues } from './PersonAuditIssues';
 
 interface CachedLineage {
@@ -210,7 +213,7 @@ export function PersonDetail() {
   const [syncLoading, setSyncLoading] = useState(false);
   const [showUploadDialog, setShowUploadDialog] = useState(false);
   const [showAncestryUploadDialog, setShowAncestryUploadDialog] = useState(false);
-  const [showRelationshipModal, setShowRelationshipModal] = useState(false);
+  const [relationshipModalType, setRelationshipModalType] = useState<RelationshipType | null>(null);
   const [hintsProcessing, setHintsProcessing] = useState(false);
 
   // Local overrides state
@@ -221,6 +224,81 @@ export function PersonDetail() {
   const [addingAlias, setAddingAlias] = useState(false);
   const [addingOccupation, setAddingOccupation] = useState(false);
 
+  // Refreshes person + all family data (parents, spouses, children, family photos).
+  // Called on initial load and after a relationship is linked.
+  const reloadPerson = useCallback(async (signal?: AbortSignal) => {
+    if (!dbId || !personId) return;
+
+    const personData = await api.getPerson(dbId, personId);
+    if (signal?.aborted) return;
+    setPerson(personData);
+
+    const validParentIds = personData.parents.filter((id): id is string => id != null);
+    const allFamilyIds: string[] = [
+      ...validParentIds,
+      ...(personData.spouses || []),
+      ...personData.children,
+    ];
+
+    // Fetch parent data
+    if (validParentIds.length > 0) {
+      const parentResults = await Promise.all(
+        validParentIds.map((pid: string) => api.getPerson(dbId, pid).catch(() => null))
+      );
+      if (signal?.aborted) return;
+      const parents: Record<string, PersonWithId> = {};
+      parentResults.forEach((p: PersonWithId | null, idx: number) => {
+        if (p) parents[validParentIds[idx]] = p;
+      });
+      setParentData(parents);
+    } else {
+      setParentData({});
+    }
+
+    // Fetch spouse data
+    if (personData.spouses && personData.spouses.length > 0) {
+      const spouseResults = await Promise.all(
+        personData.spouses.map((sid: string) => api.getPerson(dbId, sid).catch(() => null))
+      );
+      if (signal?.aborted) return;
+      const spouses: Record<string, PersonWithId> = {};
+      spouseResults.forEach((s: PersonWithId | null, idx: number) => {
+        if (s && personData.spouses) spouses[personData.spouses[idx]] = s;
+      });
+      setSpouseData(spouses);
+    } else {
+      setSpouseData({});
+    }
+
+    // Fetch children data
+    if (personData.children.length > 0) {
+      const childResults = await Promise.all(
+        personData.children.map((cid: string) => api.getPerson(dbId, cid).catch(() => null))
+      );
+      if (signal?.aborted) return;
+      const children: Record<string, PersonWithId> = {};
+      childResults.forEach((c: PersonWithId | null, idx: number) => {
+        if (c) children[personData.children[idx]] = c;
+      });
+      setChildData(children);
+    } else {
+      setChildData({});
+    }
+
+    // Check photos for all family members (batch)
+    if (allFamilyIds.length > 0) {
+      const photoChecks = await Promise.all(
+        allFamilyIds.map((id: string) => api.hasPhoto(id).then(r => ({ id, exists: r?.exists ?? false })).catch(() => ({ id, exists: false })))
+      );
+      if (signal?.aborted) return;
+      const photos: Record<string, boolean> = {};
+      photoChecks.forEach(({ id, exists }) => { photos[id] = exists; });
+      setFamilyPhotos(photos);
+    } else {
+      setFamilyPhotos({});
+    }
+  }, [dbId, personId]);
+
   useEffect(() => {
     if (!dbId || !personId) return;
 
@@ -260,77 +338,21 @@ export function PersonDetail() {
     }).catch(() => []);
 
     Promise.all([
-      api.getPerson(dbId, personId),
       api.getDatabase(dbId),
       api.getScrapedData(personId).catch(() => null),
       api.getAugmentation(personId).catch(() => null),
       fetchPhotoStatus(personId),
     ])
-      .then(async ([personData, dbData, _scraped, augment, photos]) => {
+      .then(async ([dbData, _scraped, augment, photos]) => {
         if (signal.aborted) return;
 
-        setPerson(personData);
         setDatabase(dbData);
         setPhotoStatus(photos);
         setAugmentation(augment);
 
-        // Collect all family member IDs for batch photo check
-        const validParentIds = personData.parents.filter((id): id is string => id != null);
-        const allFamilyIds: string[] = [
-          ...validParentIds,
-          ...(personData.spouses || []),
-          ...personData.children,
-        ];
-
-        // Fetch parent data
-        if (validParentIds.length > 0) {
-          const parentResults = await Promise.all(
-            validParentIds.map((pid: string) => api.getPerson(dbId, pid).catch(() => null))
-          );
-          if (signal.aborted) return;
-          const parents: Record<string, PersonWithId> = {};
-          parentResults.forEach((p: PersonWithId | null, idx: number) => {
-            if (p) parents[validParentIds[idx]] = p;
-          });
-          setParentData(parents);
-        }
-
-        // Fetch spouse data
-        if (personData.spouses && personData.spouses.length > 0) {
-          const spouseResults = await Promise.all(
-            personData.spouses.map((sid: string) => api.getPerson(dbId, sid).catch(() => null))
-          );
-          if (signal.aborted) return;
-          const spouses: Record<string, PersonWithId> = {};
-          spouseResults.forEach((s: PersonWithId | null, idx: number) => {
-            if (s && personData.spouses) spouses[personData.spouses[idx]] = s;
-          });
-          setSpouseData(spouses);
-        }
-
-        // Fetch children data
-        if (personData.children.length > 0) {
-          const childResults = await Promise.all(
-            personData.children.map((cid: string) => api.getPerson(dbId, cid).catch(() => null))
-          );
-          if (signal.aborted) return;
-          const children: Record<string, PersonWithId> = {};
-          childResults.forEach((c: PersonWithId | null, idx: number) => {
-            if (c) children[personData.children[idx]] = c;
-          });
-          setChildData(children);
-        }
-
-        // Check photos for all family members (batch)
-        if (allFamilyIds.length > 0) {
-          const photoChecks = await Promise.all(
-            allFamilyIds.map((id: string) => api.hasPhoto(id).then(r => ({ id, exists: r?.exists ?? false })).catch(() => ({ id, exists: false })))
-          );
-          if (signal.aborted) return;
-          const photos: Record<string, boolean> = {};
-          photoChecks.forEach(({ id, exists }) => { photos[id] = exists; });
-          setFamilyPhotos(photos);
-        }
+        // Load person + family data via shared helper
+        await reloadPerson(signal);
+        if (signal.aborted) return;
 
         // Check for cached lineage
         const cached = getCachedLineage(dbId, personId);
@@ -347,7 +369,7 @@ export function PersonDetail() {
       });
 
     return () => controller.abort();
-  }, [dbId, personId]);
+  }, [dbId, personId, reloadPerson]);
 
   const calculateLineage = async () => {
     if (!dbId || !personId || !database?.rootId) return;
@@ -960,9 +982,35 @@ export function PersonDetail() {
           <div className="mt-3 pt-3 border-t border-app-border/50 grid grid-cols-1 md:grid-cols-3 gap-3">
             {/* Parents */}
             <div className="flex flex-wrap items-start gap-2 md:flex-col md:items-start md:gap-2 md:bg-app-bg/30 md:border md:border-app-border/50 md:rounded-lg md:p-2">
-              <div className="flex items-center gap-1 text-xs text-app-text-muted w-16 shrink-0 pt-2 md:w-full md:pt-0 md:pb-1 md:border-b md:border-app-border/40">
+              <div className="flex items-center justify-between gap-2 text-xs text-app-text-muted w-16 shrink-0 pt-2 md:w-full md:pt-0 md:pb-1 md:border-b md:border-app-border/40">
+                <div className="flex items-center gap-1">
                 <Users size={12} />
                 Parents
+                </div>
+                {person.parents.filter(id => id != null).length < 2 && (
+                  <button
+                    type="button"
+                    className="text-[10px] text-app-accent hover:underline"
+                    title="Add or link a parent"
+                    onClick={() => {
+                      // Determine which parent role is missing by checking the
+                      // *known* genders of existing parents — parents.filter(
+                      // Boolean) on the server collapses sparse positions so
+                      // array index is unreliable. Only infer when we have a
+                      // definite male/female gender; ignore 'unknown' and
+                      // un-loaded parents and fall back to 'father'.
+                      const knownGenders = person.parents
+                        .filter((id): id is string => id != null)
+                        .map(id => parentData[id]?.gender)
+                        .filter((g): g is 'male' | 'female' => g === 'male' || g === 'female');
+                      const nextRole: RelationshipType =
+                        knownGenders.includes('male') ? 'mother' : 'father';
+                      setRelationshipModalType(nextRole);
+                    }}
+                  >
+                    + Add
+                  </button>
+                )}
               </div>
               {person.parents.some(id => id != null) ? (
                 <div className="flex flex-wrap gap-1.5 flex-1">
@@ -993,7 +1041,7 @@ export function PersonDetail() {
                   type="button"
                   className="text-[10px] text-app-accent hover:underline"
                   title="Add or link a spouse"
-                  onClick={() => setShowRelationshipModal(true)}
+                  onClick={() => setRelationshipModalType('spouse')}
                 >
                   + Add
                 </button>
@@ -1017,9 +1065,19 @@ export function PersonDetail() {
 
             {/* Children */}
             <div className="flex flex-wrap items-start gap-2 md:flex-col md:items-start md:gap-2 md:bg-app-bg/30 md:border md:border-app-border/50 md:rounded-lg md:p-2">
-              <div className="flex items-center gap-1 text-xs text-app-text-muted w-16 shrink-0 pt-2 md:w-full md:pt-0 md:pb-1 md:border-b md:border-app-border/40">
+              <div className="flex items-center justify-between gap-2 text-xs text-app-text-muted w-16 shrink-0 pt-2 md:w-full md:pt-0 md:pb-1 md:border-b md:border-app-border/40">
+                <div className="flex items-center gap-1">
                 <Users size={12} />
                 Children
+                </div>
+                <button
+                  type="button"
+                  className="text-[10px] text-app-accent hover:underline"
+                  title="Add or link a child"
+                  onClick={() => setRelationshipModalType('child')}
+                >
+                  + Add
+                </button>
               </div>
               {person.children.length > 0 ? (
                 <div className="flex flex-wrap gap-1.5 flex-1">
@@ -1281,40 +1339,22 @@ export function PersonDetail() {
         loading={linkingLoading}
       />
 
-      {/* Relationship placeholder modal */}
-      {showRelationshipModal && (
-        <div
-          className="fixed inset-0 z-50 flex items-center justify-center bg-black/50"
-          onClick={(e) => e.target === e.currentTarget && setShowRelationshipModal(false)}
-        >
-          <div className="bg-app-card rounded-lg border border-app-border shadow-xl max-w-md w-full mx-4">
-            <div className="flex items-center justify-between px-4 py-3 border-b border-app-border">
-              <h3 className="font-semibold text-app-text">Add Relationship</h3>
-              <button
-                onClick={() => setShowRelationshipModal(false)}
-                className="p-1 text-app-text-muted hover:text-app-text hover:bg-app-hover rounded transition-colors"
-                aria-label="Close"
-              >
-                <X size={18} />
-              </button>
-            </div>
-            <div className="p-4 space-y-3">
-              <p className="text-sm text-app-text-muted">
-                Coming soon: link existing people or create new profiles for parents, spouses, and children.
-              </p>
-              <div className="flex justify-end">
-                <button
-                  type="button"
-                  onClick={() => setShowRelationshipModal(false)}
-                  className="px-3 py-1.5 text-sm text-app-text-secondary hover:bg-app-hover rounded transition-colors"
-                >
-                  Close
-                </button>
-              </div>
-            </div>
-          </div>
-        </div>
-      )}
+      {/* Relationship linking modal */}
+      <RelationshipModal
+        open={relationshipModalType !== null}
+        dbId={dbId!}
+        personId={personId!}
+        initialType={relationshipModalType ?? undefined}
+        onClose={() => setRelationshipModalType(null)}
+        onLinked={async () => {
+          try {
+            await reloadPerson();
+            toast.success('Relationship linked');
+          } catch (error) {
+            toast.error('Failed to refresh person after linking relationship');
+          }
+        }}
+      />
     </div>
   );
 }