feat(timeline): regroup /zeitstrahl by Ereignis or Thema (#827) #847

marcel · 2026-06-15T11:11:25+02:00

marcel commented

2026-06-15 11:11:25 +02:00

Closes #827.

Adds the Datum · Ereignis · Thema grouping toggle to /zeitstrahl. The control re-bundles only the loose letters; the axis-fixed layers (life-events, event pills, world-bands) stay put. Regrouping is a pure client-side transform over the layer-filtered view (#780) — filter-then-group, no refetch.

What changed

Backend (one field)

TimelineEntryDTO gains a nullable linkedEventId (17 components) — the curated event whose documents set contains the letter — resolved in one batched membership pass in TimelineService over the events it already loads (no per-letter query, no new column/migration). Reuses the existing timeline_event_documents join.
Regenerated frontend/src/lib/generated/api.ts committed.

Frontend

timelineGrouping.ts — pure transform: buildEventLookup, hasLooseLetters, bucketLetters (cluster by linkedEventId / primary rootTagId, with Weitere Briefe / Ohne Thema fallback).
New components: GroupingControl (a role="radiogroup", arrow-key navigable, ≥44px, ≤320px abbreviations with full-word aria-label, disabled-but-retained when letters are hidden), LetterBucket, BucketHeaderChip (tinted root-tag header).
LetterCard gains a variant="event" (.lcard.ev) and suppressTagChip.
TimelineView/YearBand render per-year buckets off Datum; +page.svelte owns the grouping $state beside the filter state, drives the dynamic meta-line label, and stacks the control above the filter trigger.
New de/en/es Paraglide keys (no collision with the existing timeline_grouping_date / timeline_tag_chip_label / timeline_filter_*).

Docs: ADR-045 (client-side regroup transport, computed letter→event link, filter-then-group composition) + RTM rows REQ-001..019 (+005b), all Done.

Composition with #780 (filter-then-group)

Letters layer off → the control is disabled but kept in place, mode retained (REQ-018).
A letter whose only linking event was filtered out → falls back to Weitere Briefe (REQ-019).

Tests

Backend: TimelineServiceTest (linked / unlinked → linkedEventId); timeline package green; clean package builds.
Frontend: 120 unit/component tests across the new/changed specs; messages.spec.ts parity (REQ-012); REQ-001 event-layer structural-identity gate; REQ-009 {@html} grep gate; npm run check clean for the changed files; lint passes.
E2E (zeitstrahl-grouping.spec.ts, local-only like the #780 filter e2e): zero-refetch on mode switch (REQ-002), 320px no-overflow + aria-labels (REQ-011), 320px axe light+dark (REQ-010g).

Notes / interpretations

REQ-001 "pixel-identical": no VRT harness in the repo → implemented as a structural-identity assertion (the event/world-band DOM is built from the same entries in every mode). Flagging for review.
The undated bucket renders unchanged in every mode (its letters have no year, so per-year bucketing does not apply).
REQ-015's tinted-chip contrast is held by construction (saturated --c-tag-* text over a subtle color-mix wash) + the component test; the e2e axe pass covers the control. The e2e doesn't seed a tagged-and-rooted letter, so a tinted bucket header isn't exercised in the axe run.

🤖 Generated with Claude Code

Closes #827. Adds the **Datum · Ereignis · Thema** grouping toggle to `/zeitstrahl`. The control re-bundles only the loose letters; the axis-fixed layers (life-events, event pills, world-bands) stay put. Regrouping is a pure client-side transform over the **layer-filtered** view (#780) — **filter-then-group**, no refetch. ## What changed **Backend (one field)** - `TimelineEntryDTO` gains a nullable `linkedEventId` (17 components) — the curated event whose `documents` set contains the letter — resolved in **one batched membership pass** in `TimelineService` over the events it already loads (no per-letter query, no new column/migration). Reuses the existing `timeline_event_documents` join. - Regenerated `frontend/src/lib/generated/api.ts` committed. **Frontend** - `timelineGrouping.ts` — pure transform: `buildEventLookup`, `hasLooseLetters`, `bucketLetters` (cluster by `linkedEventId` / primary `rootTagId`, with `Weitere Briefe` / `Ohne Thema` fallback). - New components: `GroupingControl` (a `role="radiogroup"`, arrow-key navigable, ≥44px, ≤320px abbreviations with full-word `aria-label`, disabled-but-retained when letters are hidden), `LetterBucket`, `BucketHeaderChip` (tinted root-tag header). - `LetterCard` gains a `variant="event"` (`.lcard.ev`) and `suppressTagChip`. - `TimelineView`/`YearBand` render per-year buckets off Datum; `+page.svelte` owns the grouping `$state` beside the filter state, drives the dynamic meta-line label, and stacks the control above the filter trigger. - New de/en/es Paraglide keys (no collision with the existing `timeline_grouping_date` / `timeline_tag_chip_label` / `timeline_filter_*`). **Docs:** ADR-045 (client-side regroup transport, computed letter→event link, filter-then-group composition) + RTM rows REQ-001..019 (+005b), all `Done`. ## Composition with #780 (filter-then-group) - Letters layer off → the control is **disabled but kept in place**, mode retained (REQ-018). - A letter whose only linking event was filtered out → falls back to **Weitere Briefe** (REQ-019). ## Tests - Backend: `TimelineServiceTest` (linked / unlinked → `linkedEventId`); timeline package green; `clean package` builds. - Frontend: 120 unit/component tests across the new/changed specs; `messages.spec.ts` parity (REQ-012); REQ-001 event-layer structural-identity gate; REQ-009 `{@html}` grep gate; `npm run check` clean for the changed files; lint passes. - E2E (`zeitstrahl-grouping.spec.ts`, local-only like the #780 filter e2e): zero-refetch on mode switch (REQ-002), 320px no-overflow + aria-labels (REQ-011), 320px axe light+dark (REQ-010g). ## Notes / interpretations - **REQ-001 "pixel-identical":** no VRT harness in the repo → implemented as a **structural-identity** assertion (the event/world-band DOM is built from the same entries in every mode). Flagging for review. - The undated bucket renders unchanged in every mode (its letters have no year, so per-year bucketing does not apply). - REQ-015's tinted-chip contrast is held by construction (saturated `--c-tag-*` text over a subtle `color-mix` wash) + the component test; the e2e axe pass covers the control. The e2e doesn't seed a tagged-and-rooted letter, so a tinted bucket header isn't exercised in the axe run. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

marcel referenced this pull request

2026-06-15 11:12:04 +02:00

As a family member I want to regroup the /zeitstrahl by Ereignis or Thema so I can read letters clustered narratively, not just chronologically #827

marcel reviewed 2026-06-15 11:25:00 +02:00

marcel left a comment

Automated /code-review high (recall-biased). Two findings worth a look; both are edge-case correctness/UX, not crashers. The transaction boundary for the new ev.getDocuments() access is fine (assemble is @Transactional(readOnly = true)), and the 17-arg TimelineEntryDTO constructor is updated at every call site.

Automated `/code-review high` (recall-biased). Two findings worth a look; both are edge-case correctness/UX, not crashers. The transaction boundary for the new `ev.getDocuments()` access is fine (`assemble` is `@Transactional(readOnly = true)`), and the 17-arg `TimelineEntryDTO` constructor is updated at every call site.

backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java

						
				@@ -258,0 +284,4 @@

				            if (linkedDocs == null) continue;

				            for (Document linked : linkedDocs) {

				                if (letterDocIds.contains(linked.getId())) {

				                    eventByDocId.putIfAbsent(linked.getId(), ev.getId());

marcel commented

2026-06-15 11:25:00 +02:00

Multi-event letters link to one arbitrary event — nondeterministic, and a filter-then-group hole. resolveLetterEventLinks picks linkedEventId via putIfAbsent over eventRepository.findAll(), which has no ORDER BY. A document is @ManyToMany with TimelineEvent, so a letter can belong to several curated events, yet only the first one Hibernate happens to return wins.

Nondeterminism: Postgres gives no row order without an explicit ORDER BY, so the winning event — and thus the Ereignis bucket the letter lands in — can flip between page loads for a multi-event document.
filter-then-group gap (REQ-019): if the chosen event E1 is filtered off-screen but a second linking event E2 is still visible, the frontend only sees linkedEventId = E1, misses it in eventLookup, and drops the letter into 'Weitere Briefe' — even though E2 (which also contains it) is on screen. The letter should cluster under E2.

Consider an ORDER BY to at least make the pick deterministic, or carrying all linking event ids so the frontend can choose a surviving one.

**Multi-event letters link to one arbitrary event — nondeterministic, and a filter-then-group hole.** `resolveLetterEventLinks` picks `linkedEventId` via `putIfAbsent` over `eventRepository.findAll()`, which has no `ORDER BY`. A document is `@ManyToMany` with `TimelineEvent`, so a letter can belong to several curated events, yet only the first one Hibernate happens to return wins. - **Nondeterminism:** Postgres gives no row order without an explicit `ORDER BY`, so the winning event — and thus the Ereignis bucket the letter lands in — can flip between page loads for a multi-event document. - **filter-then-group gap (REQ-019):** if the chosen event E1 is filtered off-screen but a *second* linking event E2 is still visible, the frontend only sees `linkedEventId = E1`, misses it in `eventLookup`, and drops the letter into 'Weitere Briefe' — even though E2 (which also contains it) is on screen. The letter should cluster under E2. Consider an `ORDER BY` to at least make the pick deterministic, or carrying all linking event ids so the frontend can choose a surviving one.

frontend/src/lib/timeline/YearBand.svelte Outdated

						
				@@ -31,0 +58,4 @@

						for (const entry of year.entries) {

							if (entry.kind === 'EVENT') out.push({ t: 'event', entry });

						}

						for (const bucket of bucketLetters(letters, bucketMode, eventLookup)) {

marcel commented

2026-06-15 11:25:00 +02:00

The dense-year density-strip safety valve (REQ-011/012) is silently dropped in Ereignis/Thema mode. In Datum mode a year with >12 letters collapses to a single YearLetterStrip via isDense(letters.length). This grouped branch bypasses that entirely — every loose letter renders as a full LetterCard inside its bucket with no upper bound. A year with, say, 200 letters all under one curated event becomes a wall of 200 cards in Ereignis mode (same in Thema), reintroducing exactly the scannability/perf problem the density strip was built to prevent. Worth confirming this is intended, or applying a per-bucket density cap.

**The dense-year density-strip safety valve (REQ-011/012) is silently dropped in Ereignis/Thema mode.** In Datum mode a year with >12 letters collapses to a single `YearLetterStrip` via `isDense(letters.length)`. This `grouped` branch bypasses that entirely — every loose letter renders as a full `LetterCard` inside its bucket with no upper bound. A year with, say, 200 letters all under one curated event becomes a wall of 200 cards in Ereignis mode (same in Thema), reintroducing exactly the scannability/perf problem the density strip was built to prevent. Worth confirming this is intended, or applying a per-bucket density cap.

marcel commented

2026-06-15 11:36:16 +02:00

📋 Requirements Engineer — PR Review

Verdict: ⚠️ Approved with concerns

Traceability is in excellent shape: every REQ-001…019 (incl. REQ-005b) is implemented, tested, and mirrored into .specify/rtm.md as Done, and ADR-045 is committed (Accepted). No untested or orphan requirement; no behaviour without a backing REQ.

REQ	Impl?	Test?	RTM	Note
001	✅	✅	Done	structural-identity (no VRT harness) — see below
002	✅	✅	Done	route spec + e2e zero-refetch
003/004/006/007/008/019	✅	✅	Done	`timelineGrouping.spec.ts`
005/005b	✅	✅	Done	backend 2 new tests
009	✅	✅	Done	grep gate + inert-text
010/011	✅	✅	Done	a11y a–g + 320px
012/016	✅	✅	Done	i18n parity + meta-line
013	✅	✅(#779)	Done	no independent failure mode
014	⚠️	✅(class only)	Done	AC passes literally; see below
015	⚠️	partial	Done	contrast clause not verified
017/018	✅	✅	Done

Concerns (not traceability blockers):

REQ-015 AC partially unmet. The AC requires the tinted header label to keep ≥4.5:1 contrast, verified in the same axe pass. The e2e never seeds a tagged-and-rooted letter, so the tinted chip is never in the axe DOM, and the component test asserts the var(--c-tag-*) binding, not contrast. By calculation it fails for several tokens (see UI/UX). The contrast clause of REQ-015 is therefore unverified and partly violated.
REQ-014 intent vs. letter. "event-letter card style #833 deferred to this issue" is implemented as a bare .lcard.ev marker class with no CSS (visually identical to a plain card). The literal AC ("carries the .lcard.ev class") passes — confirm the style intent is satisfied.
Multi-tag hint (Resolved Decision 3) — timeline_grouping_multitag_hint exists in all three locales (so REQ-012 is met) but is never rendered, so the "documented UI hint" never reaches the reader.
REQ-001 is implemented as a structural-identity assertion rather than a pixel VRT — acknowledged in the PR; acceptable given no VRT harness exists.

### 📋 Requirements Engineer — PR Review **Verdict: ⚠️ Approved with concerns** Traceability is in excellent shape: every `REQ-001…019` (incl. `REQ-005b`) is implemented, tested, and mirrored into `.specify/rtm.md` as `Done`, and ADR-045 is committed (Accepted). No untested or orphan requirement; no behaviour without a backing REQ. | REQ | Impl? | Test? | RTM | Note | |---|---|---|---|---| | 001 | ✅ | ✅ | Done | structural-identity (no VRT harness) — see below | | 002 | ✅ | ✅ | Done | route spec + e2e zero-refetch | | 003/004/006/007/008/019 | ✅ | ✅ | Done | `timelineGrouping.spec.ts` | | 005/005b | ✅ | ✅ | Done | backend 2 new tests | | 009 | ✅ | ✅ | Done | grep gate + inert-text | | 010/011 | ✅ | ✅ | Done | a11y a–g + 320px | | 012/016 | ✅ | ✅ | Done | i18n parity + meta-line | | 013 | ✅ | ✅(#779) | Done | no independent failure mode | | 014 | ⚠️ | ✅(class only) | Done | AC passes literally; see below | | 015 | ⚠️ | partial | Done | contrast clause not verified | | 017/018 | ✅ | ✅ | Done | | **Concerns (not traceability blockers):** - **REQ-015 AC partially unmet.** The AC requires the tinted header label to keep **≥4.5:1 contrast, verified in the same axe pass.** The e2e never seeds a tagged-and-rooted letter, so the tinted chip is never in the axe DOM, and the component test asserts the `var(--c-tag-*)` binding, not contrast. By calculation it fails for several tokens (see UI/UX). The contrast clause of REQ-015 is therefore unverified and partly violated. - **REQ-014 intent vs. letter.** "event-letter card style #833 deferred to this issue" is implemented as a bare `.lcard.ev` marker class with no CSS (visually identical to a plain card). The literal AC ("carries the `.lcard.ev` class") passes — confirm the *style* intent is satisfied. - **Multi-tag hint** (Resolved Decision 3) — `timeline_grouping_multitag_hint` exists in all three locales (so REQ-012 is met) but is never rendered, so the "documented UI hint" never reaches the reader. - **REQ-001** is implemented as a structural-identity assertion rather than a pixel VRT — acknowledged in the PR; acceptable given no VRT harness exists.

marcel commented

2026-06-15 11:36:24 +02:00

👨‍💻 Developer (Felix Brandt) — PR Review

Verdict: ⚠️ Approved with concerns

Clean, well-factored work. timelineGrouping.ts is a pure, side-effect-free transform unit-tested independently of the DOM; layering holds (TimelineService injects only its own repo + sibling services); eventRepository.findAll() is reused for both the event-entry loop and resolveLetterEventLinks, so the membership pass costs no extra query and there is no per-letter N+1 (constitution §1.3, issue Non-Functional note honoured). npm run generate:api was run and committed — the api.ts diff is exactly linkedEventId?: string, nothing hand-edited (§3.5 / §4.1). No new ErrorCode needed (read-only). frontend/CLAUDE.md updated.

Concerns (suggestions):

Dead styling hook: .lcard.ev. LetterCard.svelte:42 adds class:ev={isEventVariant}, but there is no .ev / .lcard.ev CSS rule anywhere in the tree (verified by grep). Since #833 already gives every card serif title + mint left-border, the event variant renders identically to plain. Either add the distinct style REQ-014 implies, or drop the prop and document that the base card is the event-letter card. As-is it reads as an unfinished hook.
Dead i18n key: timeline_grouping_multitag_hint. Present in de/en/es and parity-tested, but referenced by no component or route. Render it (Thema mode) or remove it.
Minor (no action): GroupingControl's default-prop ariaLabel and the segments array resolve m.*() at init, not reactively — consistent with the project's per-request SSR-locale pattern.

### 👨‍💻 Developer (Felix Brandt) — PR Review **Verdict: ⚠️ Approved with concerns** Clean, well-factored work. `timelineGrouping.ts` is a pure, side-effect-free transform unit-tested independently of the DOM; layering holds (`TimelineService` injects only its own repo + sibling services); `eventRepository.findAll()` is reused for both the event-entry loop and `resolveLetterEventLinks`, so the membership pass costs **no extra query** and there is no per-letter N+1 (constitution §1.3, issue Non-Functional note honoured). `npm run generate:api` was run and committed — the `api.ts` diff is exactly `linkedEventId?: string`, nothing hand-edited (§3.5 / §4.1). No new `ErrorCode` needed (read-only). `frontend/CLAUDE.md` updated. **Concerns (suggestions):** - **Dead styling hook: `.lcard.ev`.** `LetterCard.svelte:42` adds `class:ev={isEventVariant}`, but there is **no `.ev` / `.lcard.ev` CSS rule anywhere** in the tree (verified by grep). Since #833 already gives every card serif title + mint left-border, the `event` variant renders identically to `plain`. Either add the distinct style REQ-014 implies, or drop the prop and document that the base card *is* the event-letter card. As-is it reads as an unfinished hook. - **Dead i18n key: `timeline_grouping_multitag_hint`.** Present in de/en/es and parity-tested, but referenced by no component or route. Render it (Thema mode) or remove it. - Minor (no action): `GroupingControl`'s default-prop `ariaLabel` and the `segments` array resolve `m.*()` at init, not reactively — consistent with the project's per-request SSR-locale pattern.

marcel commented

2026-06-15 11:36:32 +02:00

🧪 Tester — PR Review

Verdict: ⚠️ Approved with concerns

Strong, multi-level coverage. Backend adds two genuine REQ-005/006 tests (letter_in_a_curated_events_documents_carries_that_events_id, letter_in_no_curated_event_has_null_linkedEventId) that would fail without the implementation. The pure-transform suite (timelineGrouping.spec.ts) covers cluster/fallback/exactly-once/filtered-out-event paths; component specs cover the chip, bucket, card variants, and control a11y; the route spec exercises REQ-016/002/018 through the #780 filter; the e2e proves zero-refetch (REQ-002) and 320px + light/dark axe (REQ-010g). Levels are well chosen.

Concerns:

No test for the REQ-015 contrast clause (blocker-adjacent). BucketHeaderChip.svelte.spec.ts asserts the var(--c-tag-*) binding and the neutral-null fallback, but never the ≥4.5:1 label contrast the AC mandates. The only axe scan (zeitstrahl-grouping.spec.ts:103) runs in default Datum mode over an untagged letter, so no tinted bucket header is ever present in the DOM it scans. The single token the chip test uses — sienna (≈4.6:1) — happens to pass while lighter tokens fail (see UI/UX). Add a contrast assertion and seed a tagged+rooted letter, then switch to Thema before the axe pass.
REQ-001 is a structural-identity assertion, not a pixel diff — acknowledged in the PR (no VRT harness); acceptable.

### 🧪 Tester — PR Review **Verdict: ⚠️ Approved with concerns** Strong, multi-level coverage. Backend adds two genuine REQ-005/006 tests (`letter_in_a_curated_events_documents_carries_that_events_id`, `letter_in_no_curated_event_has_null_linkedEventId`) that would fail without the implementation. The pure-transform suite (`timelineGrouping.spec.ts`) covers cluster/fallback/exactly-once/filtered-out-event paths; component specs cover the chip, bucket, card variants, and control a11y; the route spec exercises REQ-016/002/018 *through the #780 filter*; the e2e proves zero-refetch (REQ-002) and 320px + light/dark axe (REQ-010g). Levels are well chosen. **Concerns:** - **No test for the REQ-015 contrast clause (blocker-adjacent).** `BucketHeaderChip.svelte.spec.ts` asserts the `var(--c-tag-*)` binding and the neutral-null fallback, but never the **≥4.5:1 label contrast** the AC mandates. The only axe scan (`zeitstrahl-grouping.spec.ts:103`) runs in **default Datum mode over an untagged letter**, so no tinted bucket header is ever present in the DOM it scans. The single token the chip test uses — `sienna` (≈4.6:1) — happens to pass while lighter tokens fail (see UI/UX). Add a contrast assertion and seed a tagged+rooted letter, then switch to Thema before the axe pass. - **REQ-001** is a structural-identity assertion, not a pixel diff — acknowledged in the PR (no VRT harness); acceptable.

marcel commented

2026-06-15 11:36:39 +02:00

🔐 Security (Nora "NullX") — PR Review

Verdict: ✅ Approved

Checked and clean:

No new attack surface. GET /api/timeline path, method, and @RequirePermission(Permission.READ_ALL) are unchanged; no new mutating endpoint, ErrorCode, or Permission (constitution §2.1–2.2). No audit field bound from a body (§2.4 — N/A, read path).
No new IDOR. linkedEventId exposes only an event id every READ_ALL reader already sees in the same payload; there is no per-user scoping to break (matches the issue STRIDE table).
XSS / CWE-79. Tag names + bucket/fallback labels render through Svelte default {...} escaping; BucketHeaderChip's title={name} is an attribute binding (auto-escaped), and the inert-text test (<img onerror=…>) passes. The timeline-no-raw-html.spec.ts grep gate forbids {@html} across all of lib/timeline/ (§2.5).
No style injection. The dynamic style strings in BucketHeaderChip.svelte:31-36 are built only from a hard-coded TAG_COLORS allow-list — an unknown/null token degrades to the neutral chip, never an interpolated raw value.
PII/logging (§2.7). resolveLetterEventLinks logs nothing; the derived-events log emits counts only.

No findings.

### 🔐 Security (Nora "NullX") — PR Review **Verdict: ✅ Approved** Checked and clean: - **No new attack surface.** `GET /api/timeline` path, method, and `@RequirePermission(Permission.READ_ALL)` are unchanged; no new mutating endpoint, `ErrorCode`, or `Permission` (constitution §2.1–2.2). No audit field bound from a body (§2.4 — N/A, read path). - **No new IDOR.** `linkedEventId` exposes only an event id every `READ_ALL` reader already sees in the same payload; there is no per-user scoping to break (matches the issue STRIDE table). - **XSS / CWE-79.** Tag names + bucket/fallback labels render through Svelte default `{...}` escaping; `BucketHeaderChip`'s `title={name}` is an attribute binding (auto-escaped), and the inert-text test (`<img onerror=…>`) passes. The `timeline-no-raw-html.spec.ts` grep gate forbids `{@html}` across **all** of `lib/timeline/` (§2.5). - **No style injection.** The dynamic `style` strings in `BucketHeaderChip.svelte:31-36` are built only from a hard-coded `TAG_COLORS` allow-list — an unknown/`null` token degrades to the neutral chip, never an interpolated raw value. - **PII/logging (§2.7).** `resolveLetterEventLinks` logs nothing; the derived-events log emits counts only. No findings.

marcel commented

2026-06-15 11:36:45 +02:00

🚀 DevOps — PR Review

Verdict: ✅ Approved

Checked: no Flyway migration (reuses the existing timeline_event_documents join — consistent with the spec's Data-Model-Changes: None); no env-var changes; no CI workflow edits and no guard step removed/weakened (§4.4); no new dependency (@axe-core/playwright + @playwright/test already in use by the #779/#780 e2e); frontend/src/lib/generated/api.ts was regenerated by the tool, not hand-edited — the diff is a single linkedEventId? field (§4.1 respected); actions/(upload|download)-artifact pins untouched (§4.3).

The new frontend/e2e/zeitstrahl-grouping.spec.ts is local-only (e2e is not yet wired into CI), explicitly mirroring the #780 layer-filter e2e and flagged in the spec — no regression to the gated suite.

Minor observation (no action): the membership pass now traverses each curated event's lazy documents collection on every timeline load, including Datum mode where linkedEventId is unused. This is inherent to the single mode-agnostic payload the zero-refetch design requires (ADR-045 §1) and is bounded by @BatchSize(50).

### 🚀 DevOps — PR Review **Verdict: ✅ Approved** Checked: **no Flyway migration** (reuses the existing `timeline_event_documents` join — consistent with the spec's Data-Model-Changes: None); **no env-var** changes; **no CI workflow** edits and no guard step removed/weakened (§4.4); **no new dependency** (`@axe-core/playwright` + `@playwright/test` already in use by the #779/#780 e2e); `frontend/src/lib/generated/api.ts` was **regenerated by the tool**, not hand-edited — the diff is a single `linkedEventId?` field (§4.1 respected); `actions/(upload|download)-artifact` pins untouched (§4.3). The new `frontend/e2e/zeitstrahl-grouping.spec.ts` is **local-only** (e2e is not yet wired into CI), explicitly mirroring the #780 layer-filter e2e and flagged in the spec — no regression to the gated suite. Minor observation (no action): the membership pass now traverses each curated event's lazy `documents` collection on **every** timeline load, including Datum mode where `linkedEventId` is unused. This is inherent to the single mode-agnostic payload the zero-refetch design requires (ADR-045 §1) and is bounded by `@BatchSize(50)`.

marcel commented

2026-06-15 11:36:56 +02:00

🎨 UI/UX — PR Review

Verdict: 🚫 Changes requested

Blocker

REQ-015 — the tinted bucket-header chip fails WCAG AA (≥4.5:1) for light palette tokens. BucketHeaderChip.svelte:31-33 renders the saturated token as the label text over a 14% color-mix wash of the same token. On the light theme (canvas #f0efe9 / surface #ffffff) the 12px font-semibold label (normal-size text → needs 4.5:1) falls well short for multiple tokens:
- amber #c17a00 ≈ 3.0:1
- sand #9a8040 ≈ 3.2:1
- sage #5a8a6a ≈ 3.4:1
- slate #607080 ≈ 4.4:1
The component test uses sienna (≈4.6:1), which passes and masks the rest. REQ-015 explicitly requires "the tinted header's label text keeps ≥4.5:1 contrast against its tint." Suggested fix: use a fixed dark ink (text-ink/text-ink-2) for the label and tint only the chip background + dot, or darken/clamp the text token; then add a contrast test and seed a tinted header into the axe run. (Dark mode is fine — light text over the dark-tinted wash.)

Suggestions

.lcard.ev is visually a no-op. No .ev/.lcard.ev CSS exists, and #833 already applied serif + mint-border to every card, so the "event-letter card" looks identical to a plain one. Confirm intent or give the event-cluster card a real distinction (spec §2 shows a stronger mint left-edge).
Multi-tag hint never shown. timeline_grouping_multitag_hint is localized but not rendered, so Thema mode never explains why a multi-tagged letter appears once (Resolved Decision 3). Render it in Thema mode.
Disabled control loses its selected highlight. bg-brand-navy/text-white are gated on !disabled (GroupingControl.svelte:86-89), so while the Letters layer is off all three segments look unselected. aria-checked stays correct, but sighted users lose the mode indicator. Consider a muted-but-visible selected style.

Good

Radiogroup semantics + roving tabindex + arrow-key nav, ≥44×44px targets, 320px abbreviations each carrying the full word as aria-label, semantic tokens throughout, sr-only disabled-reason role=status, and a light+dark axe pass on the control. Filter-then-group placement (control stacked above the #780 trigger) reads as one cluster as specified.

### 🎨 UI/UX — PR Review **Verdict: 🚫 Changes requested** ### Blocker - **REQ-015 — the tinted bucket-header chip fails WCAG AA (≥4.5:1) for light palette tokens.** `BucketHeaderChip.svelte:31-33` renders the **saturated token as the label text** over a 14% `color-mix` wash of the *same* token. On the light theme (canvas `#f0efe9` / surface `#ffffff`) the 12px `font-semibold` label (normal-size text → needs 4.5:1) falls well short for multiple tokens: - amber `#c17a00` ≈ **3.0:1** - sand `#9a8040` ≈ **3.2:1** - sage `#5a8a6a` ≈ **3.4:1** - slate `#607080` ≈ **4.4:1** The component test uses `sienna` (≈4.6:1), which passes and masks the rest. REQ-015 explicitly requires "the tinted header's **label text keeps ≥4.5:1 contrast** against its tint." Suggested fix: use a fixed dark ink (`text-ink`/`text-ink-2`) for the label and tint only the chip background + dot, or darken/clamp the text token; then add a contrast test and seed a tinted header into the axe run. (Dark mode is fine — light text over the dark-tinted wash.) ### Suggestions - **`.lcard.ev` is visually a no-op.** No `.ev`/`.lcard.ev` CSS exists, and #833 already applied serif + mint-border to every card, so the "event-letter card" looks identical to a plain one. Confirm intent or give the event-cluster card a real distinction (spec §2 shows a stronger mint left-edge). - **Multi-tag hint never shown.** `timeline_grouping_multitag_hint` is localized but not rendered, so Thema mode never explains why a multi-tagged letter appears once (Resolved Decision 3). Render it in Thema mode. - **Disabled control loses its selected highlight.** `bg-brand-navy`/`text-white` are gated on `!disabled` (`GroupingControl.svelte:86-89`), so while the Letters layer is off all three segments look unselected. `aria-checked` stays correct, but sighted users lose the mode indicator. Consider a muted-but-visible selected style. ### Good Radiogroup semantics + roving tabindex + arrow-key nav, ≥44×44px targets, 320px abbreviations each carrying the full word as `aria-label`, semantic tokens throughout, sr-only disabled-reason `role=status`, and a light+dark axe pass on the control. Filter-then-group placement (control stacked above the #780 trigger) reads as one cluster as specified.

marcel commented

2026-06-15 11:37:03 +02:00

🏛️ Architect — PR Review

Verdict: ✅ Approved

ADR present & correct. ADR-045 (Accepted, 2026-06-15) records the three #827-specific forks (client-side regroup transport, computed letter→event link over timeline_event_documents, filter-then-group composition). Number verified free on disk (043/044/045). No Accepted ADR edited (§4.2).
No new cross-domain edge. linkedEventId is derived from data TimelineService already loads (curated events + their documents); the TagService resolver edge belongs to #835 and is reused, not re-introduced (§1.3). TimelineService injects only TimelineEventRepository (own domain) plus sibling services.
Views, not entities (§1.6 / ADR-036). The DTO is assembled in-transaction; no lazy-collection entity crosses the controller boundary.
Minimal surface. One nullable record component (TimelineEntryDTO 16→17); no table/column/migration/endpoint/Permission/ErrorCode change. No new backend package → no ArchitectureTest allow-list change required (§1.7). Frontend stays inside lib/timeline/ (no cross-domain import).
The single mode-agnostic payload (ADR-045 §1) is the right call for REQ-002 zero-refetch — keeping grouping a pure read-view presentation transform bounds the blast radius.

No architectural findings.

### 🏛️ Architect — PR Review **Verdict: ✅ Approved** - **ADR present & correct.** ADR-045 (Accepted, 2026-06-15) records the three #827-specific forks (client-side regroup transport, computed letter→event link over `timeline_event_documents`, filter-then-group composition). Number verified free on disk (043/044/045). No `Accepted` ADR edited (§4.2). - **No new cross-domain edge.** `linkedEventId` is derived from data `TimelineService` already loads (curated events + their `documents`); the `TagService` resolver edge belongs to #835 and is reused, not re-introduced (§1.3). `TimelineService` injects only `TimelineEventRepository` (own domain) plus sibling services. - **Views, not entities (§1.6 / ADR-036).** The DTO is assembled in-transaction; no lazy-collection entity crosses the controller boundary. - **Minimal surface.** One nullable record component (`TimelineEntryDTO` 16→17); no table/column/migration/endpoint/`Permission`/`ErrorCode` change. No new backend package → no `ArchitectureTest` allow-list change required (§1.7). Frontend stays inside `lib/timeline/` (no cross-domain import). - The single mode-agnostic payload (ADR-045 §1) is the right call for REQ-002 zero-refetch — keeping grouping a pure read-view presentation transform bounds the blast radius. No architectural findings.

marcel referenced this issue from a commit

2026-06-15 12:12:04 +02:00

docs(rtm): trace the #827 grouped-view redesign (nesting, contrast, density)

marcel referenced this pull request

2026-06-15 12:12:17 +02:00

As a family member I want to regroup the /zeitstrahl by Ereignis or Thema so I can read letters clustered narratively, not just chronologically #827

marcel referenced this issue from a commit

2026-06-15 14:59:54 +02:00

docs(timeline): design doc for the grouped-view contained-card layout

marcel referenced this issue from a commit

2026-06-15 14:59:55 +02:00

docs(timeline): implementation plan for the grouped-view contained-card layout

marcel referenced this pull request

2026-06-15 15:02:18 +02:00

As a family member I want to regroup the /zeitstrahl by Ereignis or Thema so I can read letters clustered narratively, not just chronologically #827

marcel added 5 commits 2026-06-15 16:35:21 +02:00

feat(timeline): cap grouped clusters at 5 letters with a show-more toggle 5a8bee3970

Replaces the in-bucket month-density sparkline with a first-5 preview + show-more
/ show-less toggle, the agreed grouped-view pattern. Datum mode keeps the >12
YearLetterStrip.

Refs #827

feat(timeline): collapse the leftover Weitere-Briefe/Ohne-Thema bin to a drawer bea9acfe63

The catch-all bucket renders count-only by default behind a reveal control, then
expands to the first-5 + show-more body. Keeps the junk drawer quiet instead of
flooding the timeline.

Refs #827

feat(timeline): make a grouped cluster one contained card e100213760

Wraps each cluster in a bordered, rounded surface card (keeping the colour rail)
so the header and its letters read as a single unit.

Refs #827

feat(timeline): render a same-year curated event as its cluster card header 70794616d2

A curated event with letters in its own band now becomes the contained card header
(glyph, title, date, provenance, edit pencil) instead of a separate floating pill —
the title reads once. Derived life-events, world-bands, and letterless event pills
are unchanged (REQ-001 amended for curated-with-letters; the identity fixture now
links its letter to the curated event so the letterless world band stays a band).

Refs #827

docs(rtm): trace the grouped-view contained-card layout (#827 )

CI / Unit & Component Tests (pull_request) Successful in 7m39s

Details

CI / OCR Service Tests (pull_request) Successful in 47s

Details

CI / Backend Unit Tests (pull_request) Failing after 15m5s

Details

CI / fail2ban Regex (pull_request) Failing after 2m6s

Details

CI / Semgrep Security Scan (pull_request) Successful in 44s

Details

CI / Compose Bucket Idempotency (pull_request) Successful in 2m8s

Details

SDD Gate / RTM Check (pull_request) Successful in 33s

Details

SDD Gate / Contract Validate (pull_request) Successful in 43s

Details

SDD Gate / Constitution Impact (pull_request) Successful in 34s

Details

be4bf8edc0

Refs #827

marcel force-pushed feat/issue-827-zeitstrahl-grouping from 093c942f67 to be4bf8edc0

2026-06-15 16:35:21 +02:00

Compare

CI / Unit & Component Tests (pull_request) Successful in 7m39s

Details

CI / OCR Service Tests (pull_request) Successful in 47s

Details

CI / Backend Unit Tests (pull_request) Failing after 15m5s

Details

CI / fail2ban Regex (pull_request) Failing after 2m6s

Details

CI / Semgrep Security Scan (pull_request) Successful in 44s

Details

CI / Compose Bucket Idempotency (pull_request) Successful in 2m8s

Details

SDD Gate / RTM Check (pull_request) Successful in 33s

Details

SDD Gate / Contract Validate (pull_request) Successful in 43s

Details

SDD Gate / Constitution Impact (pull_request) Successful in 34s

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/issue-827-zeitstrahl-grouping:feat/issue-827-zeitstrahl-grouping

git checkout feat/issue-827-zeitstrahl-grouping

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#847