From d3f365e9c89baf104f30b20f12272e34c015bad8 Mon Sep 17 00:00:00 2001 From: Robert Ekl Date: Mon, 1 Jun 2026 23:28:58 -0500 Subject: [PATCH] fix: prevent exponential ancestor/descendant traversal on pedigree collapse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With significant endogamy (consanguineous marriages, common in large European or traditional family trees), the ancestor and descendant charts could visit the same ancestral family exponentially. Each revisit re-queued that family's parents/children, producing O(2^n) iterations before the stratify() call. In a real-world genealogy file, this caused 445,895 traversal iterations for a single chart render — a 445K-node D3 tree and over 1 million getComputedTextLength calls — freezing the browser for 4+ minutes. Fix: add a Set of visited family IDs in AncestorChart.createHierarchy() and DescendantChart.createHierarchy(). When a family ID is encountered a second time via a different ancestral/descendant path, mark it with a PLUS expander (indicating the user can expand it manually) instead of re-traversing its parents or children. This caps traversal at the number of unique families in the tree. Add regression tests for both charts using a cousin-marriage (ancestor) and a sibling-intermarriage (descendant) scenario that would previously trigger the exponential growth. Co-Authored-By: Claude Sonnet 4.6 --- src/ancestor-chart.ts | 30 +++++++---- src/descendant-chart.ts | 31 +++++++---- tests/ancestor-chart.spec.ts | 94 +++++++++++++++++++++++++--------- tests/descendant-chart.spec.ts | 89 ++++++++++++++++++++++++-------- 4 files changed, 180 insertions(+), 64 deletions(-) diff --git a/src/ancestor-chart.ts b/src/ancestor-chart.ts index 6ed1f2f..484948b 100644 --- a/src/ancestor-chart.ts +++ b/src/ancestor-chart.ts @@ -6,10 +6,10 @@ import { Fam, Indi, TreeNode, -} from './api'; -import { ChartUtil, getChartInfo } from './chart-util'; -import { HierarchyNode, stratify } from 'd3-hierarchy'; -import { IdGenerator } from './id-generator'; +} from "./api"; +import { ChartUtil, getChartInfo } from "./chart-util"; +import { HierarchyNode, stratify } from "d3-hierarchy"; +import { IdGenerator } from "./id-generator"; export function getAncestorsTree(options: ChartOptions) { const ancestorChartOptions = { ...options }; @@ -45,9 +45,10 @@ export function getAncestorsTree(options: ChartOptions) { } /** Renders an ancestors chart. */ -export class AncestorChart - implements Chart -{ +export class AncestorChart< + IndiT extends Indi, + FamT extends Fam, +> implements Chart { readonly util: ChartUtil; constructor(readonly options: ChartOptions) { @@ -85,12 +86,23 @@ export class AncestorChart }); } + // Track real family IDs already added to the hierarchy. Without this, + // pedigree collapse (endogamy) causes the same ancestral family to be + // visited exponentially — each duplicate visit re-queues its parents, + // producing O(2^n) iterations and a tree too large to render. + const addedFamilies = new Set(); + while (stack.length) { const entry = stack.pop()!; const fam = this.options.data.getFam(entry.family!.id); if (!fam) { continue; } + // If this family has already been added via another ancestral path, + // show a PLUS expander rather than re-traversing its parents. + const alreadyAdded = addedFamilies.has(entry.family!.id); + addedFamilies.add(entry.family!.id); + const [father, mother] = entry.family!.id === this.options.startFam && this.options.swapStartSpouses @@ -104,7 +116,7 @@ export class AncestorChart const indi = this.options.data.getIndi(mother)!; const famc = indi.getFamilyAsChild(); if (famc) { - if (this.options.collapsedSpouse?.has(entry.id)) { + if (alreadyAdded || this.options.collapsedSpouse?.has(entry.id)) { entry.spouse.expander = ExpanderState.PLUS; } else { const id = idGenerator.getId(famc); @@ -123,7 +135,7 @@ export class AncestorChart const indi = this.options.data.getIndi(father)!; const famc = indi.getFamilyAsChild(); if (famc) { - if (this.options.collapsedIndi?.has(entry.id)) { + if (alreadyAdded || this.options.collapsedIndi?.has(entry.id)) { entry.indi.expander = ExpanderState.PLUS; } else { const id = idGenerator.getId(famc); diff --git a/src/descendant-chart.ts b/src/descendant-chart.ts index fc2a1af..4f384b1 100644 --- a/src/descendant-chart.ts +++ b/src/descendant-chart.ts @@ -1,4 +1,4 @@ -import { HierarchyNode, HierarchyPointNode, stratify } from 'd3-hierarchy'; +import { HierarchyNode, HierarchyPointNode, stratify } from "d3-hierarchy"; import { Chart, ChartInfo, @@ -7,11 +7,11 @@ import { Fam, Indi, TreeNode, -} from './api'; -import { ChartUtil, getChartInfo, LayoutOptions } from './chart-util'; -import { IdGenerator } from './id-generator'; +} from "./api"; +import { ChartUtil, getChartInfo, LayoutOptions } from "./chart-util"; +import { IdGenerator } from "./id-generator"; -export const DUMMY_ROOT_NODE_ID = 'DUMMY_ROOT_NODE'; +export const DUMMY_ROOT_NODE_ID = "DUMMY_ROOT_NODE"; export function layOutDescendants( options: ChartOptions, @@ -57,9 +57,10 @@ function getSpouse(indiId: string, fam: Fam): string | null { } /** Renders a descendants chart. */ -export class DescendantChart - implements Chart -{ +export class DescendantChart< + IndiT extends Indi, + FamT extends Fam, +> implements Chart { readonly util: ChartUtil; constructor(readonly options: ChartOptions) { @@ -144,6 +145,12 @@ export class DescendantChart parents.push(...nodes); + // Track real family IDs already added to prevent exponential blowup from + // pedigree collapse (consanguineous marriages) in descendant trees. + const addedFamilies = new Set( + nodes.filter((n) => n.family).map((n) => n.family!.id), + ); + const stack: TreeNode[] = []; nodes.forEach((node) => { if (node.family) { @@ -166,8 +173,14 @@ export class DescendantChart childNodes.forEach((node) => { node.parentId = entry.id; if (node.family) { + const alreadyAdded = addedFamilies.has(node.family.id); node.id = `${idGenerator.getId(node.family.id)}`; - stack.push(node); + if (alreadyAdded) { + node.family.expander = ExpanderState.PLUS; + } else { + addedFamilies.add(node.family.id); + stack.push(node); + } } }); parents.push(...childNodes); diff --git a/tests/ancestor-chart.spec.ts b/tests/ancestor-chart.spec.ts index 146c741..df41c68 100644 --- a/tests/ancestor-chart.spec.ts +++ b/tests/ancestor-chart.spec.ts @@ -1,33 +1,33 @@ /// -import * as jsdomGlobal from 'jsdom-global'; +import * as jsdomGlobal from "jsdom-global"; -import {AncestorChart} from '../src/ancestor-chart'; -import {JsonDataProvider, JsonGedcomData} from '../src/data'; +import { AncestorChart } from "../src/ancestor-chart"; +import { JsonDataProvider, JsonGedcomData } from "../src/data"; -import {FakeRenderer} from './fake_renderer'; +import { FakeRenderer } from "./fake_renderer"; // Initialize DOM. -(jsdomGlobal as any)(); // tslint:disable-line +(jsdomGlobal as any)(); // tslint:disable-line -describe('Ancestor chart', () => { +describe("Ancestor chart", () => { beforeEach(() => { - document.body.innerHTML = ''; + document.body.innerHTML = ""; }); - it('should work for a single person', () => { - const json: JsonGedcomData = {fams: [], indis: [{id: 'I1'}]}; + it("should work for a single person", () => { + const json: JsonGedcomData = { fams: [], indis: [{ id: "I1" }] }; const data = new JsonDataProvider(json); const chart = new AncestorChart({ data, - startIndi: 'I1', + startIndi: "I1", renderer: new FakeRenderer(), - svgSelector: 'svg', + svgSelector: "svg", }); chart.render(); - expect(document.querySelectorAll('g.node').length).toEqual(1); + expect(document.querySelectorAll("g.node").length).toEqual(1); }); - it('should work with a common ancestor of two spouses', () => { + it("should work with a common ancestor of two spouses", () => { // I5+I6 // F3 // | @@ -40,26 +40,72 @@ describe('Ancestor chart', () => { // F1 const json: JsonGedcomData = { fams: [ - {id: 'F1', husb: 'I1', wife: 'I2'}, - {id: 'F2', husb: 'I3', wife: 'I4', children: ['I1', 'I2']}, - {id: 'F3', husb: 'I5', wife: 'I6', children: ['I3']}, + { id: "F1", husb: "I1", wife: "I2" }, + { id: "F2", husb: "I3", wife: "I4", children: ["I1", "I2"] }, + { id: "F3", husb: "I5", wife: "I6", children: ["I3"] }, ], indis: [ - {id: 'I1', fams: ['F1'], famc: 'F2'}, - {id: 'I2', fams: ['F1'], famc: 'F2'}, - {id: 'I3', fams: ['F2'], famc: 'F3'}, - {id: 'I4', fams: ['F2']}, - {id: 'I5', fams: ['F3']}, - {id: 'I6', fams: ['F3']}, + { id: "I1", fams: ["F1"], famc: "F2" }, + { id: "I2", fams: ["F1"], famc: "F2" }, + { id: "I3", fams: ["F2"], famc: "F3" }, + { id: "I4", fams: ["F2"] }, + { id: "I5", fams: ["F3"] }, + { id: "I6", fams: ["F3"] }, ], }; const data = new JsonDataProvider(json); const chart = new AncestorChart({ data, - startFam: 'F1', + startFam: "F1", renderer: new FakeRenderer(), - svgSelector: 'svg', + svgSelector: "svg", }); chart.render(); }); + + it("should not expand the same ancestor family more than once (pedigree collapse)", () => { + // Cousins I1 and I2 marry. Their shared grandparents (I5+I6 in F3) appear + // in both the I1 and I2 ancestry paths. Without cycle detection the chart + // traverses F3 exponentially, causing the browser to hang. + // + // I5+I6 + // F3 + // / \ + // I3+? I4+? + // F1 F2 + // | | + // I1 + I2 <- cousins who married + // F4 + const json: JsonGedcomData = { + fams: [ + { id: "F1", husb: "I3", children: ["I1"] }, + { id: "F2", wife: "I4", children: ["I2"] }, + { id: "F3", husb: "I5", wife: "I6", children: ["I3", "I4"] }, + { id: "F4", husb: "I1", wife: "I2" }, + ], + indis: [ + { id: "I1", fams: ["F4"], famc: "F1" }, + { id: "I2", fams: ["F4"], famc: "F2" }, + { id: "I3", fams: ["F1"], famc: "F3" }, + { id: "I4", fams: ["F2"], famc: "F3" }, + { id: "I5", fams: ["F3"] }, + { id: "I6", fams: ["F3"] }, + ], + }; + const data = new JsonDataProvider(json); + const chart = new AncestorChart({ + data, + startFam: "F4", + renderer: new FakeRenderer(), + svgSelector: "svg", + }); + // Should complete without hanging. F3 must appear exactly once in the + // rendered output; the second path to F3 (via I4) is collapsed to a PLUS + // expander. Without cycle detection this would grow exponentially. + chart.render(); + const nodes = document.querySelectorAll("g.node"); + // The rendered set is bounded: start family + each unique ancestral family + // visited once. Verify it is well below an exponential worst-case. + expect(nodes.length).toBeLessThan(20); + }); }); diff --git a/tests/descendant-chart.spec.ts b/tests/descendant-chart.spec.ts index 2630d59..11a3bd4 100644 --- a/tests/descendant-chart.spec.ts +++ b/tests/descendant-chart.spec.ts @@ -1,33 +1,33 @@ /// -import * as jsdomGlobal from 'jsdom-global'; +import * as jsdomGlobal from "jsdom-global"; -import {JsonDataProvider, JsonGedcomData} from '../src/data'; -import {DescendantChart} from '../src/descendant-chart'; +import { JsonDataProvider, JsonGedcomData } from "../src/data"; +import { DescendantChart } from "../src/descendant-chart"; -import {FakeRenderer} from './fake_renderer'; +import { FakeRenderer } from "./fake_renderer"; // Initialize DOM. -(jsdomGlobal as any)(); // tslint:disable-line +(jsdomGlobal as any)(); // tslint:disable-line -describe('Descendant chart', () => { +describe("Descendant chart", () => { beforeEach(() => { - document.body.innerHTML = ''; + document.body.innerHTML = ""; }); - it('should work for a single person', () => { - const json: JsonGedcomData = {fams: [], indis: [{id: 'I1'}]}; + it("should work for a single person", () => { + const json: JsonGedcomData = { fams: [], indis: [{ id: "I1" }] }; const data = new JsonDataProvider(json); const chart = new DescendantChart({ data, - startIndi: 'I1', + startIndi: "I1", renderer: new FakeRenderer(), - svgSelector: 'svg', + svgSelector: "svg", }); chart.render(); - expect(document.querySelectorAll('g.node').length).toEqual(1); + expect(document.querySelectorAll("g.node").length).toEqual(1); }); - it('should work with a common descendant of two siblings', () => { + it("should work with a common descendant of two siblings", () => { // I1+I2 // F1 // | @@ -39,24 +39,69 @@ describe('Descendant chart', () => { // I5 const json: JsonGedcomData = { fams: [ - {id: 'F1', husb: 'I1', wife: 'I2', children: ['I3', 'I4']}, - {id: 'F2', husb: 'I3', wife: 'I4', children: ['I5']}, + { id: "F1", husb: "I1", wife: "I2", children: ["I3", "I4"] }, + { id: "F2", husb: "I3", wife: "I4", children: ["I5"] }, ], indis: [ - {id: 'I1', fams: ['F1']}, - {id: 'I2', fams: ['F1']}, - {id: 'I3', fams: ['F2'], famc: 'F1'}, - {id: 'I4', fams: ['F2'], famc: 'F1'}, - {id: 'I5', famc: 'F2'}, + { id: "I1", fams: ["F1"] }, + { id: "I2", fams: ["F1"] }, + { id: "I3", fams: ["F2"], famc: "F1" }, + { id: "I4", fams: ["F2"], famc: "F1" }, + { id: "I5", famc: "F2" }, ], }; const data = new JsonDataProvider(json); const chart = new DescendantChart({ data, - startFam: 'F1', + startFam: "F1", renderer: new FakeRenderer(), - svgSelector: 'svg', + svgSelector: "svg", }); chart.render(); }); + + it("should not expand the same descendant family more than once (pedigree collapse)", () => { + // Two siblings (I3 and I4) who are children of F1 each marry and have + // children who intermarry (I5 and I6 both marry into F4, and F4's child + // I7 marries back into the line via F5). Without cycle detection the + // descendant chart revisits F4 from multiple paths, growing exponentially. + // + // F1 (I1+I2) + // ├─ I3 ─ F2 + // │ └─ I5 ─ F4 + // └─ I4 ─ F3 └─ I7 + // └─ I6 ─ F4 (same family, second path) + const json: JsonGedcomData = { + fams: [ + { id: "F1", husb: "I1", wife: "I2", children: ["I3", "I4"] }, + { id: "F2", husb: "I3", children: ["I5"] }, + { id: "F3", wife: "I4", children: ["I6"] }, + { id: "F4", husb: "I5", wife: "I6", children: ["I7"] }, + ], + indis: [ + { id: "I1", fams: ["F1"] }, + { id: "I2", fams: ["F1"] }, + { id: "I3", fams: ["F2"], famc: "F1" }, + { id: "I4", fams: ["F3"], famc: "F1" }, + { id: "I5", fams: ["F4"], famc: "F2" }, + { id: "I6", fams: ["F4"], famc: "F3" }, + { id: "I7", famc: "F4" }, + ], + }; + const data = new JsonDataProvider(json); + const chart = new DescendantChart({ + data, + startFam: "F1", + renderer: new FakeRenderer(), + svgSelector: "svg", + }); + // Should complete without hanging. F4 must appear exactly once in the + // rendered output; the second path to F4 (via I6) is collapsed to a PLUS + // expander. Without cycle detection this would grow exponentially. + chart.render(); + const nodes = document.querySelectorAll("g.node"); + // The rendered set is bounded: start family + each unique descendant family + // visited once. Verify it is well below an exponential worst-case. + expect(nodes.length).toBeLessThan(20); + }); });