diff --git a/src/core/family-graph.ts b/src/core/family-graph.ts index b47b4526..dd10faee 100755 --- a/src/core/family-graph.ts +++ b/src/core/family-graph.ts @@ -17,6 +17,7 @@ import { parseMediaRefs } from './media-service'; import { isSourceNote, isEventNote, isPlaceNote, isOrganizationNote, isProofSummaryNote, isUniverseNote, isCitationNote, isPersonNote } from '../utils/note-type-detection'; import type { RawRelationship, FamilyGraphMapping } from '../relationships/types/relationship-types'; import { getRelationshipType, getAllRelationshipTypesWithCustomizations } from '../relationships/constants/default-relationship-types'; +import { RelationshipQueryService } from './relationship-query-service'; const logger = getLogger('FamilyGraph'); @@ -248,12 +249,26 @@ export class FamilyGraphService { private propertyAliases: Record = {}; private valueAliases: ValueAliasSettings = { eventType: {}, sex: {}, gender_identity: {}, placeCategory: {}, noteType: {} }; private settings: CanvasRootsSettings | null = null; + private _queryService: RelationshipQueryService | null = null; constructor(app: App) { this.app = app; this.personCache = new Map(); } + /** + * Lazily-instantiated relationship-query service. Captures `this` so + * the closure resolves `personCache` at call time — `loadPersonCache` + * mutates the same Map via `clear()`/`set()`, never reassigns it, so + * the lookup stays valid across reload cycles. Tracking issue: #546. + */ + private getQueryService(): RelationshipQueryService { + if (!this._queryService) { + this._queryService = new RelationshipQueryService(crId => this.personCache.get(crId)); + } + return this._queryService; + } + /** * Set the folder filter service for filtering person notes by folder */ @@ -797,13 +812,23 @@ export class FamilyGraphService { } } - // Add step-parents if enabled (with distinct edge type) + // Step-parents — edge emission decoupled from node-presence check + // (#546): a step-parent already in the tree via a sibling's walk + // must still get THIS person's relationship edge emitted, not just + // be silently dropped. Step-parents still don't get their own + // ancestors walked (they're not blood relatives). if (options.includeStepParents) { // Step-fathers for (const stepfatherCrId of node.stepfatherCrIds) { const stepfather = this.personCache.get(stepfatherCrId); - if (stepfather && this.shouldIncludePerson(stepfather, options) && !nodes.has(stepfatherCrId)) { + if (!stepfather || !this.shouldIncludePerson(stepfather, options)) continue; + if (!nodes.has(stepfatherCrId)) { nodes.set(stepfatherCrId, stepfather); + } + if (!edges.some(e => + e.from === stepfatherCrId && e.to === node.crId && + e.relationshipTypeId === 'step_parent' + )) { edges.push({ from: stepfather.crId, to: node.crId, @@ -811,15 +836,20 @@ export class FamilyGraphService { relationshipTypeId: 'step_parent', relationshipLabel: 'Step-father' }); - // Don't recurse ancestors for step-parents (they're not blood relatives) } } // Step-mothers for (const stepmotherCrId of node.stepmotherCrIds) { const stepmother = this.personCache.get(stepmotherCrId); - if (stepmother && this.shouldIncludePerson(stepmother, options) && !nodes.has(stepmotherCrId)) { + if (!stepmother || !this.shouldIncludePerson(stepmother, options)) continue; + if (!nodes.has(stepmotherCrId)) { nodes.set(stepmotherCrId, stepmother); + } + if (!edges.some(e => + e.from === stepmotherCrId && e.to === node.crId && + e.relationshipTypeId === 'step_parent' + )) { edges.push({ from: stepmother.crId, to: node.crId, @@ -827,50 +857,67 @@ export class FamilyGraphService { relationshipTypeId: 'step_parent', relationshipLabel: 'Step-mother' }); - // Don't recurse ancestors for step-parents (they're not blood relatives) } } } - // Add adoptive parents if enabled (with distinct edge type) + // Adoptive parents — same edge/node decoupling as step-parents above. if (options.includeAdoptiveParents) { // Adoptive father (gender-specific) if (node.adoptiveFatherCrId) { const adoptiveFather = this.personCache.get(node.adoptiveFatherCrId); - if (adoptiveFather && this.shouldIncludePerson(adoptiveFather, options) && !nodes.has(node.adoptiveFatherCrId)) { - nodes.set(node.adoptiveFatherCrId, adoptiveFather); - edges.push({ - from: adoptiveFather.crId, - to: node.crId, - type: 'relationship', - relationshipTypeId: 'adoptive_parent', - relationshipLabel: 'Adoptive father' - }); - // Don't recurse ancestors for adoptive parents (they're not blood relatives) + if (adoptiveFather && this.shouldIncludePerson(adoptiveFather, options)) { + if (!nodes.has(node.adoptiveFatherCrId)) { + nodes.set(node.adoptiveFatherCrId, adoptiveFather); + } + if (!edges.some(e => + e.from === node.adoptiveFatherCrId && e.to === node.crId && + e.relationshipTypeId === 'adoptive_parent' + )) { + edges.push({ + from: adoptiveFather.crId, + to: node.crId, + type: 'relationship', + relationshipTypeId: 'adoptive_parent', + relationshipLabel: 'Adoptive father' + }); + } } } // Adoptive mother (gender-specific) if (node.adoptiveMotherCrId) { const adoptiveMother = this.personCache.get(node.adoptiveMotherCrId); - if (adoptiveMother && this.shouldIncludePerson(adoptiveMother, options) && !nodes.has(node.adoptiveMotherCrId)) { - nodes.set(node.adoptiveMotherCrId, adoptiveMother); - edges.push({ - from: adoptiveMother.crId, - to: node.crId, - type: 'relationship', - relationshipTypeId: 'adoptive_parent', - relationshipLabel: 'Adoptive mother' - }); - // Don't recurse ancestors for adoptive parents (they're not blood relatives) + if (adoptiveMother && this.shouldIncludePerson(adoptiveMother, options)) { + if (!nodes.has(node.adoptiveMotherCrId)) { + nodes.set(node.adoptiveMotherCrId, adoptiveMother); + } + if (!edges.some(e => + e.from === node.adoptiveMotherCrId && e.to === node.crId && + e.relationshipTypeId === 'adoptive_parent' + )) { + edges.push({ + from: adoptiveMother.crId, + to: node.crId, + type: 'relationship', + relationshipTypeId: 'adoptive_parent', + relationshipLabel: 'Adoptive mother' + }); + } } } // Gender-neutral adoptive parents for (const adoptiveParentCrId of node.adoptiveParentCrIds) { const adoptiveParent = this.personCache.get(adoptiveParentCrId); - if (adoptiveParent && this.shouldIncludePerson(adoptiveParent, options) && !nodes.has(adoptiveParentCrId)) { + if (!adoptiveParent || !this.shouldIncludePerson(adoptiveParent, options)) continue; + if (!nodes.has(adoptiveParentCrId)) { nodes.set(adoptiveParentCrId, adoptiveParent); + } + if (!edges.some(e => + e.from === adoptiveParentCrId && e.to === node.crId && + e.relationshipTypeId === 'adoptive_parent' + )) { edges.push({ from: adoptiveParent.crId, to: node.crId, @@ -878,7 +925,6 @@ export class FamilyGraphService { relationshipTypeId: 'adoptive_parent', relationshipLabel: 'Adoptive parent' }); - // Don't recurse ancestors for adoptive parents (they're not blood relatives) } } } @@ -955,27 +1001,44 @@ export class FamilyGraphService { } } - // Add children - for (const childCrId of node.childrenCrIds) { - const child = this.personCache.get(childCrId); - if (child && this.shouldIncludePerson(child, options)) { + // Children — route bio + adopted + step through the unified query + // service (#546) so a single canonical walk feeds the tree-builder. + // Bio recurses; adopted and step are added as nodes with a + // relationship edge but their own descendants are NOT recursed — + // preserves the long-standing intent that adopted children have + // their own family line and shouldn't bleed descendant trees of + // the adoptive parent. Bio takes precedence: a child appearing in + // both `childrenCrIds` and `adoptedChildCrIds` of the same parent + // renders via the bio edge, not duplicated as adoptive. + const queryService = this.getQueryService(); + for (const { person: child, kind } of queryService.getChildren(node, { include: 'all' })) { + if (!this.shouldIncludePerson(child, options)) continue; + + if (kind === 'bio') { edges.push({ from: node.crId, to: child.crId, type: 'parent' }); this.buildDescendantTree(child, nodes, edges, options, currentGeneration + 1, visited); + continue; } - } - // Add adopted children - for (const adoptedChildCrId of node.adoptedChildCrIds) { - const adoptedChild = this.personCache.get(adoptedChildCrId); - if (adoptedChild && this.shouldIncludePerson(adoptedChild, options) && !nodes.has(adoptedChildCrId)) { + // Adopted or step: skip if already added via bio (precedence) + if (nodes.has(child.crId)) continue; + nodes.set(child.crId, child); + + const relationshipTypeId = kind === 'adopted' ? 'adoptive_parent' : 'step_parent'; + const relationshipLabel = kind === 'adopted' ? 'Adopted child' : 'Stepchild'; + const alreadyEmitted = edges.some(e => + e.from === node.crId && + e.to === child.crId && + e.relationshipTypeId === relationshipTypeId + ); + if (!alreadyEmitted) { edges.push({ from: node.crId, - to: adoptedChild.crId, + to: child.crId, type: 'relationship', - relationshipTypeId: 'adoptive_parent', - relationshipLabel: 'Adopted child' + relationshipTypeId, + relationshipLabel }); - // Don't recurse descendants for adopted children (they have their own family line) } } } @@ -1053,25 +1116,30 @@ export class FamilyGraphService { } } - // Step-parents (with distinct edge type, default enabled for full trees) + // Step-parents (with distinct edge type, default enabled for full trees). + // Edge emission is decoupled from the cycle-check (#546): a step-parent + // reached via a different BFS path first must still get its relationship + // edge emitted on this traversal — otherwise stepchild→stepparent edges + // silently disappear depending on visit order. The cycle-check only + // gates whether to queue the parent for further processing. if (includeStepParents) { // Step-fathers for (const stepfatherCrId of currentPerson.stepfatherCrIds) { const stepfather = this.personCache.get(stepfatherCrId); - if (stepfather && !visited.has(stepfatherCrId)) { - // Add relationship edge (avoid duplicates) - if (!edges.some(e => - e.from === stepfatherCrId && e.to === currentCrId && - e.relationshipTypeId === 'step_parent' - )) { - edges.push({ - from: stepfatherCrId, - to: currentCrId, - type: 'relationship', - relationshipTypeId: 'step_parent', - relationshipLabel: 'Step-father' - }); - } + if (!stepfather) continue; + if (!edges.some(e => + e.from === stepfatherCrId && e.to === currentCrId && + e.relationshipTypeId === 'step_parent' + )) { + edges.push({ + from: stepfatherCrId, + to: currentCrId, + type: 'relationship', + relationshipTypeId: 'step_parent', + relationshipLabel: 'Step-father' + }); + } + if (!visited.has(stepfatherCrId)) { toProcess.push(stepfatherCrId); } } @@ -1079,32 +1147,31 @@ export class FamilyGraphService { // Step-mothers for (const stepmotherCrId of currentPerson.stepmotherCrIds) { const stepmother = this.personCache.get(stepmotherCrId); - if (stepmother && !visited.has(stepmotherCrId)) { - // Add relationship edge (avoid duplicates) - if (!edges.some(e => - e.from === stepmotherCrId && e.to === currentCrId && - e.relationshipTypeId === 'step_parent' - )) { - edges.push({ - from: stepmotherCrId, - to: currentCrId, - type: 'relationship', - relationshipTypeId: 'step_parent', - relationshipLabel: 'Step-mother' - }); - } + if (!stepmother) continue; + if (!edges.some(e => + e.from === stepmotherCrId && e.to === currentCrId && + e.relationshipTypeId === 'step_parent' + )) { + edges.push({ + from: stepmotherCrId, + to: currentCrId, + type: 'relationship', + relationshipTypeId: 'step_parent', + relationshipLabel: 'Step-mother' + }); + } + if (!visited.has(stepmotherCrId)) { toProcess.push(stepmotherCrId); } } } - // Adoptive parents (with distinct edge type, default enabled for full trees) + // Adoptive parents — same edge/cycle decoupling as step-parents above. if (includeAdoptiveParents) { // Adoptive father (gender-specific) if (currentPerson.adoptiveFatherCrId) { const adoptiveFather = this.personCache.get(currentPerson.adoptiveFatherCrId); - if (adoptiveFather && !visited.has(currentPerson.adoptiveFatherCrId)) { - // Add relationship edge (avoid duplicates) + if (adoptiveFather) { if (!edges.some(e => e.from === currentPerson.adoptiveFatherCrId && e.to === currentCrId && e.relationshipTypeId === 'adoptive_parent' @@ -1117,15 +1184,16 @@ export class FamilyGraphService { relationshipLabel: 'Adoptive father' }); } - toProcess.push(currentPerson.adoptiveFatherCrId); + if (!visited.has(currentPerson.adoptiveFatherCrId)) { + toProcess.push(currentPerson.adoptiveFatherCrId); + } } } // Adoptive mother (gender-specific) if (currentPerson.adoptiveMotherCrId) { const adoptiveMother = this.personCache.get(currentPerson.adoptiveMotherCrId); - if (adoptiveMother && !visited.has(currentPerson.adoptiveMotherCrId)) { - // Add relationship edge (avoid duplicates) + if (adoptiveMother) { if (!edges.some(e => e.from === currentPerson.adoptiveMotherCrId && e.to === currentCrId && e.relationshipTypeId === 'adoptive_parent' @@ -1138,27 +1206,29 @@ export class FamilyGraphService { relationshipLabel: 'Adoptive mother' }); } - toProcess.push(currentPerson.adoptiveMotherCrId); + if (!visited.has(currentPerson.adoptiveMotherCrId)) { + toProcess.push(currentPerson.adoptiveMotherCrId); + } } } // Gender-neutral adoptive parents for (const adoptiveParentCrId of currentPerson.adoptiveParentCrIds) { const adoptiveParent = this.personCache.get(adoptiveParentCrId); - if (adoptiveParent && !visited.has(adoptiveParentCrId)) { - // Add relationship edge (avoid duplicates) - if (!edges.some(e => - e.from === adoptiveParentCrId && e.to === currentCrId && - e.relationshipTypeId === 'adoptive_parent' - )) { - edges.push({ - from: adoptiveParentCrId, - to: currentCrId, - type: 'relationship', - relationshipTypeId: 'adoptive_parent', - relationshipLabel: 'Adoptive parent' - }); - } + if (!adoptiveParent) continue; + if (!edges.some(e => + e.from === adoptiveParentCrId && e.to === currentCrId && + e.relationshipTypeId === 'adoptive_parent' + )) { + edges.push({ + from: adoptiveParentCrId, + to: currentCrId, + type: 'relationship', + relationshipTypeId: 'adoptive_parent', + relationshipLabel: 'Adoptive parent' + }); + } + if (!visited.has(adoptiveParentCrId)) { toProcess.push(adoptiveParentCrId); } } @@ -1209,7 +1279,9 @@ export class FamilyGraphService { } } - // Children + // Children — bio. The dedicated `child` edge type is used for + // the BFS-side bookkeeping; canvas-generator filters these out + // in favor of the symmetric parent edge. for (const childCrId of currentPerson.childrenCrIds) { const child = this.personCache.get(childCrId); if (child) { @@ -1217,6 +1289,57 @@ export class FamilyGraphService { toProcess.push(childCrId); } } + + // Adopted children — walked from the parent's side so an adopted + // child appears in the adoptive parent's full-tree even when not + // reachable via any other path (#545 / #546). Edge dedupes against + // the symmetric one that gets pushed when the child's own + // `adoptive_X` walk fires. + if (includeAdoptiveParents) { + for (const adoptedChildCrId of currentPerson.adoptedChildCrIds) { + const adoptedChild = this.personCache.get(adoptedChildCrId); + if (!adoptedChild) continue; + if (!edges.some(e => + e.from === currentCrId && e.to === adoptedChildCrId && + e.relationshipTypeId === 'adoptive_parent' + )) { + edges.push({ + from: currentCrId, + to: adoptedChildCrId, + type: 'relationship', + relationshipTypeId: 'adoptive_parent', + relationshipLabel: 'Adopted child' + }); + } + if (!visited.has(adoptedChildCrId)) { + toProcess.push(adoptedChildCrId); + } + } + } + + // Stepchildren — symmetric with the step-parent walk above + // (the reverse-walk in `loadPersonCache` keeps these in sync). + if (includeStepParents) { + for (const stepchildCrId of currentPerson.stepchildrenCrIds) { + const stepchild = this.personCache.get(stepchildCrId); + if (!stepchild) continue; + if (!edges.some(e => + e.from === currentCrId && e.to === stepchildCrId && + e.relationshipTypeId === 'step_parent' + )) { + edges.push({ + from: currentCrId, + to: stepchildCrId, + type: 'relationship', + relationshipTypeId: 'step_parent', + relationshipLabel: 'Stepchild' + }); + } + if (!visited.has(stepchildCrId)) { + toProcess.push(stepchildCrId); + } + } + } } } diff --git a/tests/family-graph-tree-builders.test.ts b/tests/family-graph-tree-builders.test.ts new file mode 100644 index 00000000..3e2b4952 --- /dev/null +++ b/tests/family-graph-tree-builders.test.ts @@ -0,0 +1,479 @@ +import { describe, expect, it } from 'vitest'; +import { App } from 'obsidian'; +import { FamilyGraphService, type PersonNode } from '../src/core/family-graph'; + +/** + * Golden-output regression coverage for the three bugs surfaced via + * #545 / #546 (and DigitalDreamn's follow-up): + * + * 1. **Bug A1** — descendant trees: adopted children get an edge but no + * node, so canvas-generator's edge-rendering pass (which drops edges + * where either endpoint isn't positioned) silently discards the + * relationship. Galen never appears on Marie's descendant tree. + * + * 2. **Bug A2** — full trees: the BFS doesn't walk a parent's + * `adoptedChildCrIds` at all. Adopted children only enter the tree + * when reached via some other path, then their `adoptive_X` field + * walks back to the parent. Generated full-tree from Marie misses + * Galen entirely. + * + * 3. **Bug B** — full trees: the `!visited.has(stepX)` / + * `!visited.has(adoptiveX)` guards on the relationship-edge-emission + * branches double up as cycle-check AND edge-gate. When the step or + * adoptive parent has been visited via a different path first + * (e.g., Shmi as Anakin's bio mother before Owen's processing + * discovers her as his stepmother), the relationship edge is never + * emitted. + * + * Tests use direct private-access to inject a pre-built `personCache` + * and invoke the tree-builders without going through Obsidian I/O. + * Same `as unknown as PrivateAccess` pattern as the existing + * extract-person tests. + */ + +interface FamilyEdge { + from: string; + to: string; + type: 'parent' | 'spouse' | 'child' | 'relationship'; + relationshipTypeId?: string; + relationshipLabel?: string; +} + +interface TreeOptions { + rootCrId: string; + treeType: 'ancestors' | 'descendants' | 'full'; + maxGenerations?: number; + includeSpouses?: boolean; + includeStepParents?: boolean; + includeAdoptiveParents?: boolean; +} + +interface PrivateTreeBuilderAccess { + personCache: Map; + buildDescendantTree( + node: PersonNode, + nodes: Map, + edges: FamilyEdge[], + options: TreeOptions, + currentGeneration: number, + visited?: Set + ): void; + buildFullTree( + node: PersonNode, + nodes: Map, + edges: FamilyEdge[], + options: TreeOptions + ): void; + buildAncestorTree( + node: PersonNode, + nodes: Map, + edges: FamilyEdge[], + options: TreeOptions, + currentGeneration: number, + visited?: Set + ): void; +} + +function makePerson(overrides: Partial & { crId: string }): PersonNode { + return { + crId: overrides.crId, + name: overrides.name ?? overrides.crId, + file: overrides.file ?? ({} as PersonNode['file']), + fatherCrId: overrides.fatherCrId, + motherCrId: overrides.motherCrId, + parentCrIds: overrides.parentCrIds ?? [], + stepfatherCrIds: overrides.stepfatherCrIds ?? [], + stepmotherCrIds: overrides.stepmotherCrIds ?? [], + adoptiveFatherCrId: overrides.adoptiveFatherCrId, + adoptiveMotherCrId: overrides.adoptiveMotherCrId, + adoptiveParentCrIds: overrides.adoptiveParentCrIds ?? [], + spouseCrIds: overrides.spouseCrIds ?? [], + spouses: overrides.spouses, + childrenCrIds: overrides.childrenCrIds ?? [], + adoptedChildCrIds: overrides.adoptedChildCrIds ?? [], + stepchildrenCrIds: overrides.stepchildrenCrIds ?? [], + birthDate: overrides.birthDate, + deathDate: overrides.deathDate + } as PersonNode; +} + +function makeService(people: PersonNode[]): { + service: FamilyGraphService; + access: PrivateTreeBuilderAccess; +} { + const service = new FamilyGraphService(new App()); + const access = service as unknown as PrivateTreeBuilderAccess; + access.personCache = new Map(people.map(p => [p.crId, p])); + return { service, access }; +} + +describe('FamilyGraphService.buildDescendantTree — Bug A1: adopted child must be added as a node', () => { + it('adds an adopted child to the nodes map (so canvas-generator can position them)', () => { + // Marie adopts Galen. Generating Marie's descendant tree must + // surface Galen — both as a node AND with the adoptive-parent + // edge. Without the node, canvas-generator's edge-renderer + // drops the edge for missing endpoint position. + const marie = makePerson({ + crId: 'marie', + adoptedChildCrIds: ['galen'] + }); + const galen = makePerson({ + crId: 'galen', + adoptiveMotherCrId: 'marie' + }); + const { access } = makeService([marie, galen]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildDescendantTree( + marie, + nodes, + edges, + { rootCrId: 'marie', treeType: 'descendants' }, + 0 + ); + + expect(nodes.has('marie')).toBe(true); + expect(nodes.has('galen')).toBe(true); + expect( + edges.some( + e => + e.from === 'marie' && + e.to === 'galen' && + e.relationshipTypeId === 'adoptive_parent' + ) + ).toBe(true); + }); + + it('adds adopted child to nodes even when ALSO reachable via bio child path (no double-counting)', () => { + // Pathological: a child appears in BOTH `childrenCrIds` and + // `adoptedChildCrIds` of the same parent. Should produce one + // node and one edge (preferring the bio relationship). + const parent = makePerson({ + crId: 'parent', + childrenCrIds: ['kid'], + adoptedChildCrIds: ['kid'] + }); + const kid = makePerson({ crId: 'kid', fatherCrId: 'parent' }); + const { access } = makeService([parent, kid]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildDescendantTree( + parent, + nodes, + edges, + { rootCrId: 'parent', treeType: 'descendants' }, + 0 + ); + + expect(nodes.has('kid')).toBe(true); + // Bio edge wins; no duplicate adoptive edge for the same pair + const parentEdges = edges.filter(e => e.from === 'parent' && e.to === 'kid'); + expect(parentEdges).toHaveLength(1); + expect(parentEdges[0].type).toBe('parent'); + }); + + it('preserves the existing "do not recurse adopted-child descendants" intent', () => { + // Galen has bio children of his own. They should NOT appear in + // Marie's descendant tree by default (preserves the intent + // codified in the old comment at family-graph.ts:978). + const marie = makePerson({ + crId: 'marie', + adoptedChildCrIds: ['galen'] + }); + const galen = makePerson({ + crId: 'galen', + adoptiveMotherCrId: 'marie', + childrenCrIds: ['galen-jr'] + }); + const galenJr = makePerson({ crId: 'galen-jr', fatherCrId: 'galen' }); + const { access } = makeService([marie, galen, galenJr]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildDescendantTree( + marie, + nodes, + edges, + { rootCrId: 'marie', treeType: 'descendants' }, + 0 + ); + + expect(nodes.has('galen')).toBe(true); + expect(nodes.has('galen-jr')).toBe(false); + }); + + it('still includes bio children correctly (regression guard)', () => { + const dad = makePerson({ crId: 'dad', childrenCrIds: ['kid'] }); + const kid = makePerson({ crId: 'kid', fatherCrId: 'dad' }); + const { access } = makeService([dad, kid]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildDescendantTree( + dad, + nodes, + edges, + { rootCrId: 'dad', treeType: 'descendants' }, + 0 + ); + + expect(nodes.has('kid')).toBe(true); + expect( + edges.some(e => e.from === 'dad' && e.to === 'kid' && e.type === 'parent') + ).toBe(true); + }); +}); + +describe('FamilyGraphService.buildFullTree — Bug A2: adopted children walked from parent\'s side', () => { + it('full tree from Marie includes Galen (her adopted child)', () => { + // In a full tree starting from Marie, Galen should appear + // regardless of whether his bio parents happen to be reachable. + const marie = makePerson({ + crId: 'marie', + adoptedChildCrIds: ['galen'] + }); + const galen = makePerson({ + crId: 'galen', + adoptiveMotherCrId: 'marie' + }); + const { access } = makeService([marie, galen]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildFullTree(marie, nodes, edges, { + rootCrId: 'marie', + treeType: 'full' + }); + + expect(nodes.has('marie')).toBe(true); + expect(nodes.has('galen')).toBe(true); + expect( + edges.some( + e => + ((e.from === 'marie' && e.to === 'galen') || + (e.from === 'galen' && e.to === 'marie')) && + e.relationshipTypeId === 'adoptive_parent' + ) + ).toBe(true); + }); +}); + +describe('FamilyGraphService.buildFullTree — Bug B: step-parent edge emitted regardless of visit order', () => { + it('Shmi-Owen step-parent edge is emitted even though Shmi was visited first via Anakin\'s bio-mother walk', () => { + // The Lars/Skywalker scenario from DigitalDreamn's vault: + // Cliegg + Shmi (married) + // Anakin = Shmi's bio son (Cliegg's stepson) + // Owen = Cliegg's bio son (Shmi's stepson) + // + // Full tree from Anakin: + // - BFS visits Anakin → walks bio mother Shmi (Shmi visited) + // - BFS walks Anakin's stepfather Cliegg (edge Cliegg→Anakin emitted; Cliegg visited) + // - BFS visits Cliegg → walks bio child Owen (Owen visited) + // - BFS visits Owen → walks his stepmother Shmi + // - Pre-fix: `!visited.has('shmi')` is FALSE → edge NEVER emitted + // - Post-fix: edge emitted unconditionally (with dedup); visited only gates recursion + const cliegg = makePerson({ + crId: 'cliegg', + spouseCrIds: ['shmi'], + childrenCrIds: ['owen'], + stepchildrenCrIds: ['anakin'] + }); + const shmi = makePerson({ + crId: 'shmi', + spouseCrIds: ['cliegg'], + childrenCrIds: ['anakin'], + stepchildrenCrIds: ['owen'] + }); + const anakin = makePerson({ + crId: 'anakin', + motherCrId: 'shmi', + stepfatherCrIds: ['cliegg'] + }); + const owen = makePerson({ + crId: 'owen', + fatherCrId: 'cliegg', + stepmotherCrIds: ['shmi'] + }); + const { access } = makeService([cliegg, shmi, anakin, owen]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildFullTree(anakin, nodes, edges, { + rootCrId: 'anakin', + treeType: 'full', + includeSpouses: true + }); + + // Both step relationships must produce edges, regardless of BFS order + expect( + edges.some( + e => + e.from === 'cliegg' && + e.to === 'anakin' && + e.relationshipTypeId === 'step_parent' + ) + ).toBe(true); + expect( + edges.some( + e => + e.from === 'shmi' && + e.to === 'owen' && + e.relationshipTypeId === 'step_parent' + ) + ).toBe(true); + }); + + it('dedupes step-parent edges if reachable via multiple paths', () => { + // Symmetric: Cliegg and Shmi each declare each other's child + // via stepchildrenCrIds and the children declare via stepX. The + // edge should appear exactly once per (from, to) pair. + const cliegg = makePerson({ + crId: 'cliegg', + childrenCrIds: ['owen'], + stepchildrenCrIds: ['anakin'] + }); + const shmi = makePerson({ + crId: 'shmi', + childrenCrIds: ['anakin'], + stepchildrenCrIds: ['owen'] + }); + const anakin = makePerson({ + crId: 'anakin', + motherCrId: 'shmi', + stepfatherCrIds: ['cliegg'] + }); + const owen = makePerson({ + crId: 'owen', + fatherCrId: 'cliegg', + stepmotherCrIds: ['shmi'] + }); + const { access } = makeService([cliegg, shmi, anakin, owen]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildFullTree(anakin, nodes, edges, { + rootCrId: 'anakin', + treeType: 'full' + }); + + const cliegg_anakin = edges.filter( + e => + e.from === 'cliegg' && + e.to === 'anakin' && + e.relationshipTypeId === 'step_parent' + ); + const shmi_owen = edges.filter( + e => + e.from === 'shmi' && + e.to === 'owen' && + e.relationshipTypeId === 'step_parent' + ); + expect(cliegg_anakin).toHaveLength(1); + expect(shmi_owen).toHaveLength(1); + }); + + it('Bug B applies to adoptive parents the same way (regression guard)', () => { + // Same shape with adoptive: if the adoptive parent has been + // visited via a different path before the adopted child's + // `adoptive_X` walk fires, the edge must still be emitted. + const marie = makePerson({ + crId: 'marie', + adoptedChildCrIds: ['galen'], + childrenCrIds: ['bio-kid'] + }); + const bioKid = makePerson({ + crId: 'bio-kid', + motherCrId: 'marie', + // Some random sibling-like adoptive link to surface Marie via a non-adopted path + adoptiveMotherCrId: 'marie' + }); + const galen = makePerson({ + crId: 'galen', + adoptiveMotherCrId: 'marie' + }); + const { access } = makeService([marie, bioKid, galen]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildFullTree(bioKid, nodes, edges, { + rootCrId: 'bio-kid', + treeType: 'full' + }); + + // Marie should have an adoptive-parent edge to Galen even though + // she was first reached as bio-kid's mother + expect( + edges.some( + e => + e.from === 'marie' && + e.to === 'galen' && + e.relationshipTypeId === 'adoptive_parent' + ) + ).toBe(true); + }); + + it('preserves working Anakin-Cliegg case (regression guard)', () => { + // The existing-correct path: Cliegg is reached only via Anakin's + // stepfather walk, never via any prior path. Pre-fix worked here; + // post-fix must keep working. + const cliegg = makePerson({ crId: 'cliegg' }); + const anakin = makePerson({ + crId: 'anakin', + stepfatherCrIds: ['cliegg'] + }); + const { access } = makeService([cliegg, anakin]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildFullTree(anakin, nodes, edges, { + rootCrId: 'anakin', + treeType: 'full' + }); + + expect( + edges.some( + e => + e.from === 'cliegg' && + e.to === 'anakin' && + e.relationshipTypeId === 'step_parent' + ) + ).toBe(true); + }); +}); + +describe('FamilyGraphService — cycle handling', () => { + it('buildDescendantTree handles a circular relationship without infinite recursion', () => { + const a = makePerson({ crId: 'a', childrenCrIds: ['b'] }); + const b = makePerson({ crId: 'b', fatherCrId: 'a', childrenCrIds: ['a'] }); + const { access } = makeService([a, b]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildDescendantTree( + a, + nodes, + edges, + { rootCrId: 'a', treeType: 'descendants' }, + 0 + ); + expect(nodes.has('a')).toBe(true); + expect(nodes.has('b')).toBe(true); + }); + + it('buildFullTree handles a circular relationship without infinite recursion', () => { + const a = makePerson({ crId: 'a', childrenCrIds: ['b'] }); + const b = makePerson({ crId: 'b', fatherCrId: 'a', childrenCrIds: ['a'] }); + const { access } = makeService([a, b]); + + const nodes = new Map(); + const edges: FamilyEdge[] = []; + access.buildFullTree(a, nodes, edges, { + rootCrId: 'a', + treeType: 'full' + }); + expect(nodes.has('a')).toBe(true); + expect(nodes.has('b')).toBe(true); + }); +});