test+feat(stammbaum): preserve all SPOUSE_OF edges in layout (#361)
Switches spousePairs from Map<string, string> to Map<string, Set<string>> so multi-spouse persons (canonical case: Albert de Gruyter, 4 marriages) keep every partner instead of losing the earlier .set() values. The behavioural discriminator (now exercised by attaches_loose_multi_spouse_to_parented_partner_when_edge_order_clobbers) is a loose person with both a parented and a loose spouse: the old map clobbered to whichever edge landed last, so the loose-placement step could miss the parented partner and merge the focal node into the wrong block. Also closes the robustness gap NullX flagged: SPOUSE_OF edges referencing IDs outside allNodes are dropped at ingestion instead of leaking into the spouse-pulldown loop. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect } from 'vitest';
|
||||||
import { buildLayout, NODE_H, ROW_GAP } from './buildLayout';
|
import { buildLayout, NODE_W, NODE_H, COL_GAP, ROW_GAP } from './buildLayout';
|
||||||
|
import canonicalFixture from '../__fixtures__/stammbaum.json';
|
||||||
import type { components } from '$lib/generated/api';
|
import type { components } from '$lib/generated/api';
|
||||||
|
|
||||||
type PersonNodeDTO = components['schemas']['PersonNodeDTO'];
|
type PersonNodeDTO = components['schemas']['PersonNodeDTO'];
|
||||||
@@ -111,3 +112,106 @@ describe('buildLayout — generation seeding (#689)', () => {
|
|||||||
expect(yOf(layout, NEGATIVE_C)).toBe(2 * (NODE_H + ROW_GAP));
|
expect(yOf(layout, NEGATIVE_C)).toBe(2 * (NODE_H + ROW_GAP));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('buildLayout — multi-spouse + intra-family marriage (#361)', () => {
|
||||||
|
const FOCAL = '00000000-0000-0000-0000-000000000010';
|
||||||
|
const SPOUSE_X = '00000000-0000-0000-0000-000000000011';
|
||||||
|
const SPOUSE_Y = '00000000-0000-0000-0000-000000000012';
|
||||||
|
const UNKNOWN = '00000000-0000-0000-0000-000000000099';
|
||||||
|
|
||||||
|
it('preserves_both_marriages_when_person_has_two_SPOUSE_OF_edges', () => {
|
||||||
|
// Before #361 the spouse map was Map<string, string>; the second
|
||||||
|
// .set() clobbered the first, so a person with N spouses (Albert de
|
||||||
|
// Gruyter, 4) silently lost N-1 of them. Asserting that every spouse
|
||||||
|
// has a layout position is the minimal presence check.
|
||||||
|
const layout = buildLayout(
|
||||||
|
[node(FOCAL, 'Focal', 3), node(SPOUSE_X, 'Alice'), node(SPOUSE_Y, 'Bob')],
|
||||||
|
[spouseEdge(FOCAL, SPOUSE_X, 'fx'), spouseEdge(FOCAL, SPOUSE_Y, 'fy')]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(layout.positions.get(FOCAL)).toBeDefined();
|
||||||
|
expect(layout.positions.get(SPOUSE_X)).toBeDefined();
|
||||||
|
expect(layout.positions.get(SPOUSE_Y)).toBeDefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores_SPOUSE_OF_edge_with_unknown_relatedPersonId', () => {
|
||||||
|
// Robustness gap flagged by NullX during persona review: an edge
|
||||||
|
// pointing to a UUID not in the node list must not crash buildLayout
|
||||||
|
// and must not introduce a phantom node into the positions map.
|
||||||
|
const buildIt = () =>
|
||||||
|
buildLayout([node(FOCAL, 'Focal', 3)], [spouseEdge(FOCAL, UNKNOWN, 'fu')]);
|
||||||
|
expect(buildIt).not.toThrow();
|
||||||
|
|
||||||
|
const layout = buildIt();
|
||||||
|
expect(layout.positions.get(FOCAL)).toBeDefined();
|
||||||
|
expect(layout.positions.get(UNKNOWN)).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('attaches_loose_multi_spouse_to_parented_partner_when_edge_order_clobbers', () => {
|
||||||
|
// The behavioural discriminator for the Map<string,string> -> Map<string,
|
||||||
|
// Set<string>> shape change. LF (loose) has two spouses: PARENTED
|
||||||
|
// (parented under an ancestor) and OTHER (loose). The PARENTED edge is
|
||||||
|
// inserted before the OTHER edge. Under the old map, the second .set()
|
||||||
|
// clobbered the first, so LF's recorded spouse was OTHER and LF fell
|
||||||
|
// into the "no parented spouse" branch — merged with OTHER far from
|
||||||
|
// PARENTED. Under the Set map, both spouses are retained and the
|
||||||
|
// loose-placement step picks the parented one, putting LF adjacent to
|
||||||
|
// PARENTED in the parented sibling block while OTHER stays alone.
|
||||||
|
const G0_ANCESTOR = '00000000-0000-0000-0000-0000000000b1';
|
||||||
|
const PARENTED = '00000000-0000-0000-0000-0000000000b2';
|
||||||
|
const LOOSE_FOCAL = '00000000-0000-0000-0000-0000000000b3';
|
||||||
|
const OTHER = '00000000-0000-0000-0000-0000000000b4';
|
||||||
|
|
||||||
|
const layout = buildLayout(
|
||||||
|
[
|
||||||
|
node(G0_ANCESTOR, 'Ancestor', 0),
|
||||||
|
node(PARENTED, 'Parented', 1),
|
||||||
|
node(LOOSE_FOCAL, 'LooseFocal', 1),
|
||||||
|
node(OTHER, 'OtherLoose', 1)
|
||||||
|
],
|
||||||
|
[
|
||||||
|
parentEdge(G0_ANCESTOR, PARENTED),
|
||||||
|
spouseEdge(LOOSE_FOCAL, PARENTED, 'lf-pt'),
|
||||||
|
spouseEdge(LOOSE_FOCAL, OTHER, 'lf-oth')
|
||||||
|
]
|
||||||
|
);
|
||||||
|
|
||||||
|
const posLF = layout.positions.get(LOOSE_FOCAL)!;
|
||||||
|
const posOTH = layout.positions.get(OTHER)!;
|
||||||
|
// With the fix, OTHER is isolated in its own loose block (far from LF).
|
||||||
|
// With the bug, LF and OTHER are merged into a dual-loose block (one
|
||||||
|
// node-width + col-gap apart).
|
||||||
|
expect(Math.abs(posLF.x - posOTH.x)).toBeGreaterThan(NODE_W + COL_GAP);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('canonical_fixture_assigns_a_position_to_every_node_with_multiple_spouses', () => {
|
||||||
|
// Real-data structural assertion against the canonical Stammbaum
|
||||||
|
// snapshot. Today the only multi-spouse case is Albert de Gruyter
|
||||||
|
// (4 marriages); the assertion stays valid as the graph grows.
|
||||||
|
const fixtureNodes = canonicalFixture.nodes as unknown as PersonNodeDTO[];
|
||||||
|
const fixtureEdges = canonicalFixture.edges as unknown as RelationshipDTO[];
|
||||||
|
const layout = buildLayout(fixtureNodes, fixtureEdges);
|
||||||
|
|
||||||
|
const partners = new Map<string, Set<string>>();
|
||||||
|
for (const e of fixtureEdges) {
|
||||||
|
if (e.relationType !== 'SPOUSE_OF') continue;
|
||||||
|
addPartner(partners, e.personId, e.relatedPersonId);
|
||||||
|
addPartner(partners, e.relatedPersonId, e.personId);
|
||||||
|
}
|
||||||
|
const multi = [...partners.entries()].filter(([, set]) => set.size >= 2);
|
||||||
|
expect(multi.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
for (const [id, set] of multi) {
|
||||||
|
expect(layout.positions.get(id)).toBeDefined();
|
||||||
|
for (const partnerId of set) {
|
||||||
|
expect(layout.positions.get(partnerId)).toBeDefined();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
function addPartner(map: Map<string, Set<string>>, key: string, value: string) {
|
||||||
|
const s = map.get(key);
|
||||||
|
if (s) s.add(value);
|
||||||
|
else map.set(key, new Set([value]));
|
||||||
|
}
|
||||||
|
|||||||
@@ -23,7 +23,10 @@ export type Layout = {
|
|||||||
export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO[]): Layout {
|
export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO[]): Layout {
|
||||||
const parentToChildren = new Map<string, string[]>();
|
const parentToChildren = new Map<string, string[]>();
|
||||||
const childToParents = new Map<string, string[]>();
|
const childToParents = new Map<string, string[]>();
|
||||||
const spousePairs = new Map<string, string>();
|
// spousePairs is a Set per person so multi-spouse cases (#361) preserve all
|
||||||
|
// marriages instead of having later edges silently clobber earlier ones.
|
||||||
|
const spousePairs = new Map<string, Set<string>>();
|
||||||
|
const allNodeIds = new Set(allNodes.map((n) => n.id));
|
||||||
|
|
||||||
for (const e of allEdges) {
|
for (const e of allEdges) {
|
||||||
switch (e.relationType) {
|
switch (e.relationType) {
|
||||||
@@ -32,8 +35,12 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO
|
|||||||
mapPush(childToParents, e.relatedPersonId, e.personId);
|
mapPush(childToParents, e.relatedPersonId, e.personId);
|
||||||
break;
|
break;
|
||||||
case 'SPOUSE_OF':
|
case 'SPOUSE_OF':
|
||||||
spousePairs.set(e.personId, e.relatedPersonId);
|
// Defensive guard against edges referencing IDs outside the
|
||||||
spousePairs.set(e.relatedPersonId, e.personId);
|
// node list (stale or partial graph snapshots) — keeps every
|
||||||
|
// downstream iteration safely scoped to known nodes.
|
||||||
|
if (!allNodeIds.has(e.personId) || !allNodeIds.has(e.relatedPersonId)) break;
|
||||||
|
mapAddToSet(spousePairs, e.personId, e.relatedPersonId);
|
||||||
|
mapAddToSet(spousePairs, e.relatedPersonId, e.personId);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -77,17 +84,19 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO
|
|||||||
changed = true;
|
changed = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
for (const [a, b] of spousePairs) {
|
for (const [a, partners] of spousePairs) {
|
||||||
const ra = rank.get(a) ?? 0;
|
for (const b of partners) {
|
||||||
const rb = rank.get(b) ?? 0;
|
const ra = rank.get(a) ?? 0;
|
||||||
const m = Math.max(ra, rb);
|
const rb = rank.get(b) ?? 0;
|
||||||
if (!locked.has(a) && ra < m) {
|
const m = Math.max(ra, rb);
|
||||||
rank.set(a, m);
|
if (!locked.has(a) && ra < m) {
|
||||||
changed = true;
|
rank.set(a, m);
|
||||||
}
|
changed = true;
|
||||||
if (!locked.has(b) && rb < m) {
|
}
|
||||||
rank.set(b, m);
|
if (!locked.has(b) && rb < m) {
|
||||||
changed = true;
|
rank.set(b, m);
|
||||||
|
changed = true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (!changed) break;
|
if (!changed) break;
|
||||||
@@ -175,15 +184,26 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO
|
|||||||
// Step 2 + 3: handle loose nodes.
|
// Step 2 + 3: handle loose nodes.
|
||||||
for (const id of ids) {
|
for (const id of ids) {
|
||||||
if (memberLookup.has(id)) continue;
|
if (memberLookup.has(id)) continue;
|
||||||
const spouse = spousePairs.get(id);
|
// With multi-spouse, prefer attaching to a parented partner so the
|
||||||
const spouseLookup = spouse ? memberLookup.get(spouse) : undefined;
|
// loose node sits inside the parented sibling block. The choice is
|
||||||
|
// stable across runs because spousePairs is iterated in edge-
|
||||||
|
// insertion order; a future commit replaces this with a name/year
|
||||||
|
// sort.
|
||||||
|
const partners = spousePairs.get(id);
|
||||||
|
let parentedSpouse: string | undefined;
|
||||||
|
if (partners) {
|
||||||
|
for (const partnerId of partners) {
|
||||||
|
if (memberLookup.get(partnerId)?.parented) {
|
||||||
|
parentedSpouse = partnerId;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (spouseLookup && spouseLookup.parented) {
|
if (parentedSpouse) {
|
||||||
// Spouse is parented — attach this loose node next to them on
|
const spouseLookup = memberLookup.get(parentedSpouse)!;
|
||||||
// the outer edge of their sibling block so the marriage line
|
|
||||||
// is short and the sibling order is preserved.
|
|
||||||
const block = blocksByKey.get(spouseLookup.key)!;
|
const block = blocksByKey.get(spouseLookup.key)!;
|
||||||
const spouseIdx = block.members.findIndex((m) => m.id === spouse);
|
const spouseIdx = block.members.findIndex((m) => m.id === parentedSpouse);
|
||||||
const insertOnRight = spouseIdx >= block.members.length / 2;
|
const insertOnRight = spouseIdx >= block.members.length / 2;
|
||||||
const insertAt = insertOnRight ? spouseIdx + 1 : spouseIdx;
|
const insertAt = insertOnRight ? spouseIdx + 1 : spouseIdx;
|
||||||
block.members.splice(insertAt, 0, { id, parented: false });
|
block.members.splice(insertAt, 0, { id, parented: false });
|
||||||
@@ -200,21 +220,25 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Merge dual-loose spouse blocks into a single 2-person block.
|
// Merge dual-loose spouse blocks into a single block. With multi-spouse,
|
||||||
|
// iterate every partner so a loose person with N loose marriages ends
|
||||||
|
// up in one shared block instead of leaving stragglers behind.
|
||||||
const removed = new Set<string>();
|
const removed = new Set<string>();
|
||||||
for (const [key, block] of blocksByKey) {
|
for (const [key, block] of blocksByKey) {
|
||||||
if (!key.startsWith('__loose__')) continue;
|
if (!key.startsWith('__loose__')) continue;
|
||||||
if (removed.has(key)) continue;
|
if (removed.has(key)) continue;
|
||||||
const member = block.members[0];
|
const member = block.members[0];
|
||||||
const spouse = spousePairs.get(member.id);
|
const partners = spousePairs.get(member.id);
|
||||||
if (!spouse) continue;
|
if (!partners) continue;
|
||||||
const spouseLookup = memberLookup.get(spouse);
|
for (const partnerId of partners) {
|
||||||
if (!spouseLookup || removed.has(spouseLookup.key)) continue;
|
const spouseLookup = memberLookup.get(partnerId);
|
||||||
if (spouseLookup.key === key) continue;
|
if (!spouseLookup || removed.has(spouseLookup.key)) continue;
|
||||||
if (!spouseLookup.key.startsWith('__loose__')) continue;
|
if (spouseLookup.key === key) continue;
|
||||||
const otherBlock = blocksByKey.get(spouseLookup.key)!;
|
if (!spouseLookup.key.startsWith('__loose__')) continue;
|
||||||
block.members.push(...otherBlock.members);
|
const otherBlock = blocksByKey.get(spouseLookup.key)!;
|
||||||
removed.add(spouseLookup.key);
|
block.members.push(...otherBlock.members);
|
||||||
|
removed.add(spouseLookup.key);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
for (const key of removed) blocksByKey.delete(key);
|
for (const key of removed) blocksByKey.delete(key);
|
||||||
|
|
||||||
@@ -277,3 +301,9 @@ function mapPush<K, V>(map: Map<K, V[]>, key: K, value: V) {
|
|||||||
if (arr) arr.push(value);
|
if (arr) arr.push(value);
|
||||||
else map.set(key, [value]);
|
else map.set(key, [value]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function mapAddToSet<K, V>(map: Map<K, Set<V>>, key: K, value: V) {
|
||||||
|
const s = map.get(key);
|
||||||
|
if (s) s.add(value);
|
||||||
|
else map.set(key, new Set([value]));
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user