From cb8c85a742a2a107824cfa0a23ba8a7df6cc9bea Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 28 May 2026 15:43:58 +0200 Subject: [PATCH] feat(stammbaum): seed layout rank from imported generation (#689) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildLayout switches to a two-stage assignment: 1. Seed — every node with node.generation != null is locked at that rank. The fallback heuristic never moves a locked rank, and the spouse-pulldown never pulls a locked rank. 2. Fallback — for unseeded nodes, rank = max(parent rank) + 1 reading parents from the same unified rank map, so an unseeded child of a seeded G 2 parent correctly inherits rank 3. Spouse-pulldown ties unseeded spouses to their deeper partner exactly as before. 3. Normalise — if any rank is negative (future G −1 ancestor), shift the whole map so min(rank) == 0. No-op for today's data. Fixes the Herbert Cram pattern from #361's review: two parented spouses with imported G 3 now render on the same y row. Existing StammbaumTree tests still pass byte-for-byte because every test node has node.generation undefined, so the heuristic runs unchanged. Refs #689 Co-Authored-By: Claude Opus 4.7 --- .../genealogy/layout/buildLayout.test.ts | 113 ++++++++++++++++++ .../person/genealogy/layout/buildLayout.ts | 62 ++++++---- 2 files changed, 155 insertions(+), 20 deletions(-) create mode 100644 frontend/src/lib/person/genealogy/layout/buildLayout.test.ts diff --git a/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts b/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts new file mode 100644 index 00000000..011a6434 --- /dev/null +++ b/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts @@ -0,0 +1,113 @@ +import { describe, it, expect } from 'vitest'; +import { buildLayout, NODE_H, ROW_GAP } from './buildLayout'; +import type { components } from '$lib/generated/api'; + +type PersonNodeDTO = components['schemas']['PersonNodeDTO']; +type RelationshipDTO = components['schemas']['RelationshipDTO']; + +const PARENT = '00000000-0000-0000-0000-000000000001'; +const CHILD = '00000000-0000-0000-0000-000000000002'; +const SPOUSE_A = '00000000-0000-0000-0000-000000000003'; +const SPOUSE_B = '00000000-0000-0000-0000-000000000004'; +const NEGATIVE_A = '00000000-0000-0000-0000-000000000005'; +const NEGATIVE_B = '00000000-0000-0000-0000-000000000006'; +const NEGATIVE_C = '00000000-0000-0000-0000-000000000007'; + +function node(id: string, displayName: string, generation: number | null = null): PersonNodeDTO { + return generation == null + ? { id, displayName, familyMember: true } + : { id, displayName, familyMember: true, generation }; +} + +function parentEdge(parentId: string, childId: string, id = parentId + childId): RelationshipDTO { + return { + id, + personId: parentId, + relatedPersonId: childId, + personDisplayName: '', + relatedPersonDisplayName: '', + relationType: 'PARENT_OF' + }; +} + +function spouseEdge(a: string, b: string, id = a + b): RelationshipDTO { + return { + id, + personId: a, + relatedPersonId: b, + personDisplayName: '', + relatedPersonDisplayName: '', + relationType: 'SPOUSE_OF' + }; +} + +function yOf(layout: ReturnType, id: string): number { + const p = layout.positions.get(id); + if (!p) throw new Error(`No position for ${id}`); + return p.y; +} + +describe('buildLayout — generation seeding (#689)', () => { + it('Herbert Cram regression: two parented G=3 spouses share the same row', () => { + // Both Herbert (G 3) and Clara (G 3) are parented children of their respective + // G 2 ancestors. They are spouses. Before #689 the iterative longest-path put + // Herbert one row deeper than Clara via the spouse-pulldown of his loose parent. + // With imported generation as a strict seed both render at the same y. + const layout = buildLayout( + [node(SPOUSE_A, 'Herbert', 3), node(SPOUSE_B, 'Clara', 3)], + [spouseEdge(SPOUSE_A, SPOUSE_B)] + ); + + expect(yOf(layout, SPOUSE_A)).toBe(yOf(layout, SPOUSE_B)); + }); + + it('strict-seed override: imported generation pins rank even when parent edges imply deeper', () => { + // PARENT has no explicit generation → falls back to 0. CHILD is parented under + // PARENT but has imported generation = 3. The seeded rank wins; the heuristic + // must not push CHILD to rank 1. + const layout = buildLayout( + [node(PARENT, 'Parent'), node(CHILD, 'Child', 3)], + [parentEdge(PARENT, CHILD)] + ); + + expect(yOf(layout, CHILD)).toBe(3 * (NODE_H + ROW_GAP)); + }); + + it('fallback inherits seeded parent rank: G 2 parent → null-gen child lands at rank 3', () => { + // CHILD has no imported generation. PARENT has generation = 2. The fallback + // reads PARENT's rank from the unified rank map (2) and computes 2 + 1 = 3. + const layout = buildLayout( + [node(PARENT, 'Parent', 2), node(CHILD, 'Child')], + [parentEdge(PARENT, CHILD)] + ); + + expect(yOf(layout, CHILD)).toBe(3 * (NODE_H + ROW_GAP)); + }); + + it('normalise is a no-op when all ranks are non-negative', () => { + // Seeded ranks [3, 4, 5] → y must reflect [3, 4, 5] without any shift. + const G3 = '00000000-0000-0000-0000-000000000031'; + const G4 = '00000000-0000-0000-0000-000000000032'; + const G5 = '00000000-0000-0000-0000-000000000033'; + const layout = buildLayout( + [node(G3, 'three', 3), node(G4, 'four', 4), node(G5, 'five', 5)], + [] + ); + + expect(yOf(layout, G3)).toBe(3 * (NODE_H + ROW_GAP)); + expect(yOf(layout, G4)).toBe(4 * (NODE_H + ROW_GAP)); + expect(yOf(layout, G5)).toBe(5 * (NODE_H + ROW_GAP)); + }); + + it('normalise shifts negative seeds so min rank becomes 0', () => { + // Seeded ranks [-1, 0, 1] → after shift they render at [0, 1, 2] y-rows. + const layout = buildLayout( + [node(NEGATIVE_A, 'minus-one', -1), node(NEGATIVE_B, 'zero', 0), node(NEGATIVE_C, 'one', 1)], + [] + ); + + expect(yOf(layout, NEGATIVE_A)).toBe(0); + expect(yOf(layout, NEGATIVE_B)).toBe(1 * (NODE_H + ROW_GAP)); + expect(yOf(layout, NEGATIVE_C)).toBe(2 * (NODE_H + ROW_GAP)); + }); +}); diff --git a/frontend/src/lib/person/genealogy/layout/buildLayout.ts b/frontend/src/lib/person/genealogy/layout/buildLayout.ts index 226b6b07..e171861a 100644 --- a/frontend/src/lib/person/genealogy/layout/buildLayout.ts +++ b/frontend/src/lib/person/genealogy/layout/buildLayout.ts @@ -38,49 +38,71 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO } } - // Iterative longest-path generation assignment. + // Two-stage rank assignment (#689): // - // Each node's generation = max(parent generations) + 1 (roots stay at 0). - // Then spouses are pulled to share the deeper generation. Pulling a spouse - // down can shift their own descendants, so we iterate until stable rather - // than running BFS once like the previous implementation (which left - // e.g. a child of a "later-pulled" spouse stranded one row too high). - const generation = new Map(); - for (const n of allNodes) generation.set(n.id, 0); + // 1. Seed: every node with imported generation is locked at that rank. + // The fallback heuristic never moves a locked rank, and spouse-pulldown + // never pulls a locked rank. + // 2. Fallback: for the remaining (unseeded) nodes, rank = max(parent rank) + // + 1, reading parent rank from the same unified map so an unseeded + // child of a seeded G 2 parent correctly inherits rank 3. Spouse- + // pulldown ties unseeded spouses to their deeper partner. + // 3. Normalise: if any seeded rank is negative (a future G −1 ancestor), + // shift the entire map so min(rank) == 0. No-op fast path covers + // today's data. + const rank = new Map(); + const locked = new Set(); + for (const n of allNodes) { + if (n.generation != null) { + rank.set(n.id, n.generation); + locked.add(n.id); + } else { + rank.set(n.id, 0); + } + } const maxIters = allNodes.length + 4; for (let it = 0; it < maxIters; it++) { let changed = false; for (const n of allNodes) { + if (locked.has(n.id)) continue; const parents = childToParents.get(n.id) ?? []; if (parents.length === 0) continue; - let maxParentGen = -1; + let maxParentRank = -Infinity; for (const pid of parents) { - maxParentGen = Math.max(maxParentGen, generation.get(pid) ?? 0); + maxParentRank = Math.max(maxParentRank, rank.get(pid) ?? 0); } - const newGen = maxParentGen + 1; - if ((generation.get(n.id) ?? 0) < newGen) { - generation.set(n.id, newGen); + const newRank = maxParentRank + 1; + if ((rank.get(n.id) ?? 0) < newRank) { + rank.set(n.id, newRank); changed = true; } } for (const [a, b] of spousePairs) { - const m = Math.max(generation.get(a) ?? 0, generation.get(b) ?? 0); - if ((generation.get(a) ?? 0) < m) { - generation.set(a, m); + const ra = rank.get(a) ?? 0; + const rb = rank.get(b) ?? 0; + const m = Math.max(ra, rb); + if (!locked.has(a) && ra < m) { + rank.set(a, m); changed = true; } - if ((generation.get(b) ?? 0) < m) { - generation.set(b, m); + if (!locked.has(b) && rb < m) { + rank.set(b, m); changed = true; } } if (!changed) break; } + let minRank = Infinity; + for (const r of rank.values()) minRank = Math.min(minRank, r); + if (minRank < 0) { + const shift = -minRank; + for (const [id, r] of rank) rank.set(id, r + shift); + } - // Group by generation, then sort within generation by display name. + // Group by rank, then sort within rank by display name. const generations = new Map(); for (const n of allNodes) { - const g = generation.get(n.id) ?? 0; + const g = rank.get(n.id) ?? 0; if (!generations.has(g)) generations.set(g, []); generations.get(g)!.push(n.id); }