feat(stammbaum): highlight the selected person's bloodline (#703) #704
Reference in New Issue
Block a user
Delete Branch "feat/issue-703-stammbaum-lineage-highlight"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #703.
What
Adds a focus + dim (lineage highlight) layer bound to the Stammbaum side panel. While a person is selected, their bloodline stays at full strength and everyone else fades back so a reader can trace one line of heritage.
Active set = the selected person + their full pedigree upward + their full descendant tree downward + the spouses of every one of those blood people (spouses are active leaves — we never climb into a married-in spouse's own ancestry). Everyone else, and every connector with a dimmed endpoint, renders at ~0.4 opacity. No panel open → full-strength tree.
How
frontend/src/lib/person/genealogy/layout/highlightLineage.ts(besidebuildLayout.ts):buildLineageIndex(edges)builds the adjacency once;highlightLineage(index, rootId)returns the activeSet<string>and anisConnectorActive(a, b)predicate. The ancestor/descendant walk is guarded by the accumulating visited set, so cyclicPARENT_OFdata terminates (REQ-STAMMBAUM-04 / AC10).SIBLING_OFand social relations are ignored, so collaterals never enter the active set.StammbaumNodegains adimmedprop (group-level opacity on its root<g>);StammbaumConnectorsgains anisConnectorActivepredicate and wraps each logical connector in a<g opacity>(the shared sibling bar stays lit to a lineage child while dimming to a collateral sibling on the same row). Both fade via alineage-fadeCSS transition gated byprefers-reduced-motion.StammbaumTreederives the active set from the rawselectedIdrune ($derived, not$effect); the index rebuilds only whenedgeschange.?focus={id}paints already dimmed on load (selectedId seeded server-side, AC9).recentreAbovehelper inpanZoom.tslifts the tapped anchor above the bottom sheet; the page drives it from amatchMediaflag (desktop side panel never overlaps the canvas, so no centring there).Opacity, not a hue swap — theme-correct in light/dark and a non-colour lightness cue (NFR-A11Y-001). View-only: no backend / endpoint / schema /
generate:apichange.Tests
TDD throughout. New node-project tests (run locally, all green):
highlightLineage.test.ts— isolated person, ancestors-only, descendants-only, pedigree fan-out, multiple spouses/remarriage, spouse boundary,SIBLING_OFexcluded, connector both-endpoints rule, cyclic-data termination with exact membership.panZoom.test.ts—recentreAbovebias geometry.StammbaumTree.svelte.test.ts— AC1 (no dimming unselected), AC2 (bloodline + spouses full, collaterals dim), AC6 (re-select recomputes), AC7 (close clears). Browser project — runs in CI.npm run buildandnpm run check(no new errors in touched files) pass.Acceptance criteria
AC1–AC7, AC9, AC10 covered by automated tests; AC8 by the
recentreAboveunit test plus the mobile wiring. Manual 320px light/dark legibility check recommended before merge (bumpDIMMED_OPACITYfrom 0.4 to 0.45–0.5 if it reads too faint in either theme).🤖 Generated with Claude Code
👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ✅ Approved with minor suggestions
TDD discipline is visible and honest: the pure
highlightLineage.test.tsand therecentreAbovecases land before/with their implementations, and the traversal is the right shape —$derivedover$effect, rawselectedIdnot a.find()lookup, adjacency built once and keyed onedges, the walk guarded by the accumulating visited set. Components stay presentation-only and receive plaindimmed/ predicate props. Names reveal intent. Good.Suggestions (non-blocking)
StammbaumConnectors.svelte(the shared child loop, ~L134-140).childActivereadsgroup.childIds[i]while iterating the filteredchildCenters. If any child lacks a position,childCentersis shorter thanchildIdsand the index drifts — the same latent assumption the existing(group.childIds[i])key already makes, but now it also drives the active lookup. Pre-existing, low-risk (every node gets a position today), but worth a follow-up: carry the id on the mapped object ({ id, x, y }) so the center and id never desync.lineage-fadestyle block inStammbaumNode.svelteandStammbaumConnectors.svelte(identicaltransition: opacity 200ms+ reduced-motion). It's 6 lines × 2 — KISS-acceptable, but a single global utility class inapp.csswould remove the drift risk if the duration ever changes. Your call.DIMMED_OPACITYis exercised through the components but the literal'0.4'is hard-coded in the test assertion (StammbaumTree.svelte.test.ts) — if the constant moves to 0.45 the test won't follow. Minor; importing the constant into the test would keep them in lock-step.Clean work. None of the above blocks merge.
🏛️ Markus Keller — Application Architect
Verdict: ✅ Approved
Altitude and boundaries are right. One pure, DOM-free module (
highlightLineage.ts) at the samelayout/boundary asbuildLayout.ts; the adjacency index is built once and the walk is a cheap O(V+E) pass; the components carry no traversal logic. No backend, endpoint, schema, orgenerate:apichange — the/api/networkgraph is already delivered client-side, so this is a view-only transform. No new route (Stammbaum exists), no new package, no transport change.Documentation obligation checked: a new domain term was introduced →
docs/GLOSSARY.mdupdated with the "lineage highlight" entry. ✅ No ADR is warranted — opacity-dim reuses the established$derived/recentreOnpatterns and introduces no lasting structural decision.Minor note (non-blocking)
DIMMED_OPACITY(a presentation value) lives in the otherwise-pure traversal module. It's cohesive with the feature and harmless, but ifhighlightLineage.tsis ever framed as "pure graph logic," that constant is the one thing that isn't. Leave it; just noting the seam.No concerns, no open decisions from my angle.
🛡️ Nora Steiner ("NullX") — Application Security Engineer
Verdict: ✅ Approved
No new attack surface. This is a client-side transform over a graph the user is already authorized to see (
/api/network) — no new input, no fetch, no DOM sink. Theopacityattribute is a numeric constant, never user-controlled;aria-labelreusesdisplayNameexactly as the unchanged code already did. The only string-interpolated selector (g[role="button"][aria-label="${displayName}"]) is in test code against fixed fixtures, not a production sink.My prior finding is codified and holds: CWE-835 (uncontrolled recursion) is closed by the
into.has(id)visited guard incollectReachable, with REQ-STAMMBAUM-04 / AC10 backed by two explicit tests (a↔bcycle andx→xself-loop) that assert exact membership, not just "doesn't hang." That's the right way to prove a termination fix.No injection, no auth/permission change, no secrets. Nothing for me to block.
🧪 Sara Holt — QA Engineer & Test Strategist
Verdict: ⚠️ Approved with concerns
The unit layer is strong and at the right altitude:
highlightLineage.test.tscovers isolated / ancestors / descendants / pedigree fan-out / multiple-spouse / spouse-boundary /SIBLING_OF-excluded / connector predicate / cyclic-termination, each with one behaviour per test and readable fixtures mirroringbuildLayout.test.ts. My earlier tightening landed — AC10 asserts the exact active set (['a','b']), not just non-hang.recentreAbovegeometry is unit-tested. Component tests assert via theopacityattribute, not screenshots. Good pyramid shape.Concerns (coverage gaps, not blockers)
highlightLineage.test.tsproves theisConnectorActivepredicate, but no component test asserts that a connector<g>between an active and a dimmed person actually rendersopacity="0.4". TheStammbaumTree.svelte.test.tscases only inspect node opacity. One assertion on a spouse-connector group (active↔in-law) would close the loop and guard the<g opacity>wiring against regression.recentreAbovemath is covered. The actual wiring —selectPersonfiringcentreOnIdonly whenisMobile, andcentreBiasFractionflipping with thematchMediaflag — has no test. A small browser test (set a narrow viewport, tap a node, assert the view recentred with the bias) would move AC8 from "geometry proven" to "behaviour proven."rerender, which the codebase documents as a full re-mount rather than an in-place prop update — fine for asserting per-selection output (AC6/AC7), just be aware it isn't exercising reactive update paths.None block merge, but #1 and #2 are the gaps I'd want filled before this is "done-done." Browser tests run CI-only here, so trust CI for the new component cases.
🎨 Leonie Voss — UI/UX & Accessibility Lead
Verdict: ⚠️ Approved with concerns
The mechanism is theme-safe and the right call: opacity (a lightness cue), not a hue swap, so the active/dimmed distinction survives greyscale and holds in both themes (WCAG 1.4.1 / NFR-A11Y-001). The dim transition is gated behind
prefers-reduced-motionvia pure CSS — correct, no JS, zero cost. The selected-node accent bar is preserved as the anchor cue, and AC8 centres the tapped person above the bottom sheet so the anchor isn't trapped behind the 60dvh sheet on a phone. Good mobile thinking.Concern — must verify before merge (not a code blocker)
bg-surfacein both light and dark mode (dark dims mint, which is already light, so it's the riskier case). Please do the 320px pass in both themes during build; if it reads too faint in either, bumpDIMMED_OPACITY(highlightLineage.ts:23) to 0.45–0.5. The requirement is "clearly de-emphasised but still legible," not invisible. This is a single constant, so the fix is trivial if the eye test fails.Minor
opacitymultiplies the ring too, so a tabbed-to dimmed node at 0.4 may have a faint outline. Likely fine since focus moves attention, but worth a glance at 320px.No brand or token violations. Approve once the legibility check is done.
🔧 Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
Nothing in my domain. No Compose service, image tag, volume, port, env var, secret, or CI-workflow change — purely a client-side SvelteKit change shipped inside the existing frontend build. No new infrastructure to size or operate, no new failure mode.
One operational note, not a blocker: the new behavioural coverage is in
*.svelte.test.ts(the browser/Playwrightclientproject), which runs CI-only per project convention — so the newStammbaumTreehighlight cases will execute for the first time in CI on this PR. Worth a glance at that job's result before merge. Build artifact and deploy path are unchanged.📋 Elicit — Requirements Engineer
Verdict: ✅ Approved (traceability complete; one criterion partially verified)
Requirement → verification trace for #703:
StammbaumTree.svelte.test.ts— all nodesopacitynullStammbaumTree.svelte.test.ts+highlightLineage.test.tshighlightLineage.test.ts(spouse boundary)highlightLineage.test.ts(remarriage)highlightLineage.test.ts(predicate) — ⚠️ render layer not assertedStammbaumTree.svelte.test.tsStammbaumTree.svelte.test.tsrecentreAbovegeometry unit-tested — ⚠️ interaction not asserted?focuspaints already dimmed)selectedId-seeded$derived(no click)highlightLineage.test.ts— exact membershipEvery acceptance criterion has a verification path and no Must-have requirement is left ambiguous. Two criteria (AC5, AC8) are proven at the logic/geometry layer but not yet at the rendered-behaviour layer — this overlaps Sara's coverage concerns and is the only open item. NFR-A11Y-001 is structurally satisfied; NFR-PERF-001 (in-memory O(V+E)) holds. No scope creep beyond the issue's stated boundaries.
The shared parent-pair child loop read group.childIds[i] while iterating the filtered childCenters, so a child without a position would desync the id from the centre — and that index now also drives the active-connector lookup. Ride the id on the mapped {id,x,y} centre so the two never drift; a positionless child drops out of both together. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>👨💻 Felix Brandt — review follow-up
Pushed 5 commits addressing the open concerns plus a visual defect Marcel caught during testing.
Defect fixed (Marcel, screenshot)
opacityon the node<g>made the whole node — including its white card fill — translucent, so the connector lines drawn beneath a dimmed box showed through it. Now the card fill renders at full strength outside the dim group, and the lineage focus+dim sits on an inner content group (outline + labels) only. Same opacity-based lightness cue (theme-safe, survives greyscale — Leonie's a11y rationale still holds), but the box is never see-through. —9c12f623Reviewer concerns
<g opacity>wiring)da306715window.matchMedia: a tap recentres the canvas when the mobile breakpoint matches, and leaves the view untouched on desktop81224829DIMMED_OPACITYhard-coded'0.4'in assertions10249c33DIMMED_OPACITY0.4 → 0.4510249c33childIds[i]vs filteredchildCenters){id, x, y}centre, so id and centre can never desync — a positionless child drops out of both together7cc2ddc6Deferred (with reason)
lineage-fadestyle block (@Felix #2) — left as-is per KISS (6 lines × 2; I'd flagged it as acceptable). Happy to extract to a global utility if preferred.Verification
npm run check— no new errors in touched files (the remainingpage.svelte.test.tserrors are pre-existingsampleNodesbaseline debt).npm run build— green.highlightLineage,panZoom) — 46 green locally. The new*.svelte.test.tscases (node opacity, AC5 connectors, AC8 wiring) run CI-only per project convention — watch this PR's CI job.⚠️ One manual eye-check still recommended before merge (@Leonie): confirm the 0.45 dim reads as clearly de-emphasised but legible at 320px in both themes against
bg-surface. The dim now lives on the outline + labels over a solid box, so it should read cleaner than the old translucent box — but it's a single constant if it needs nudging.🤖 Generated with Claude Code