Cluster event letters inline in the chronological /zeitstrahl (no grouping toggle) #851

Merged
marcel merged 20 commits from feat/issue-850-inline-event-clustering into main 2026-06-16 14:38:11 +02:00
Owner

Closes #850

Summary

On /zeitstrahl, a curated event that has letters linked to it now renders as a contained event card — the event is the card header (accent glyph, title, {date} · {kuratiert|abgeleitet} subtitle, count, and a curator edit link), with its linked letters listed inside (first 5, then a keyboard-operable show-more/less toggle). Letters in a year other than the event's band get a lighter cross-year ✉ title card. Every other letter stays a plain, alternating, density-folding chronological letter. There is no grouping control — clustering is automatic and always on. The meta-line drops its Gruppierung: Datum segment.

This supersedes #827: it keeps that branch's event-card clustering and the computed linkedEventId, and drops the toggle, the Thema mode, and the "Weitere Briefe" drawer.

What changed

Backend

  • TimelineEntryDTO gains a nullable linkedEventId (UUID; not @Schema(REQUIRED)).
  • TimelineService.resolveLetterEventLinks resolves each letter's curated event in one batched pass over the events it already loads — no per-letter query, no new column, no Flyway migration.
  • Regenerated the single linkedEventId? field in api.ts.

Frontend

  • New eventClustering.ts (buildEventLookup, splitYearLetters, CLUSTER_PREVIEW=5) — filter-then-cluster: a letter clusters only if its linkedEventId is set AND present in the lookup, otherwise it stays loose.
  • New EventCluster.svelte — the contained event card (same-year event header + edit link, or cross-year ✉ text header; first-5 + show-more).
  • LetterCard.svelte gains compact + variant='event' (the .lcard.ev in-card letter).
  • YearBand.svelte rebuilt to render event clusters inline; loose letters keep the alternating layout and density strip, and the strip counts only loose letters (no duplication).
  • TimelineView.svelte builds the event lookup once and threads it + canWrite to each band.
  • +page.svelte drops the grouping meta segment; the unused timeline_grouping_date key removed from de/en/es.
  • New timeline_bucket_show_more/_less keys in all locales.
  • REQ-010 {@html} grep gate over lib/timeline/.

Tests (real runs)

  • Backend TimelineServiceTest: 30 passed (incl. the 2 new linkedEventId tests); DerivedEventsAssemblyTest: 17 passed; backend main sources compile.
  • Frontend client sweep (LetterCard, EventCluster, YearBand, TimelineView, zeitstrahl/page): 81 passed (5 files).
  • Frontend server sweep (eventClustering, messages, timeline-no-raw-html): 18 passed (3 files).
  • svelte-check: no new errors in the touched files (pre-existing baseline noise elsewhere unchanged).

RTM: thirteen REQ-001..013 rows added for #850 (feature inline-event-clustering), Status Done.

🤖 Generated with Claude Code

Closes #850 ## Summary On `/zeitstrahl`, a curated event that has letters linked to it now renders as a contained event card — the event is the card header (accent glyph, title, `{date} · {kuratiert|abgeleitet}` subtitle, count, and a curator edit link), with its linked letters listed inside (first 5, then a keyboard-operable show-more/less toggle). Letters in a year *other* than the event's band get a lighter cross-year `✉ title` card. Every other letter stays a plain, alternating, density-folding chronological letter. There is **no grouping control** — clustering is automatic and always on. The meta-line drops its `Gruppierung: Datum` segment. This supersedes #827: it keeps that branch's event-card clustering and the computed `linkedEventId`, and drops the toggle, the Thema mode, and the "Weitere Briefe" drawer. ## What changed **Backend** - `TimelineEntryDTO` gains a nullable `linkedEventId` (UUID; not `@Schema(REQUIRED)`). - `TimelineService.resolveLetterEventLinks` resolves each letter's curated event in one batched pass over the events it already loads — no per-letter query, no new column, no Flyway migration. - Regenerated the single `linkedEventId?` field in `api.ts`. **Frontend** - New `eventClustering.ts` (`buildEventLookup`, `splitYearLetters`, `CLUSTER_PREVIEW=5`) — filter-then-cluster: a letter clusters only if its `linkedEventId` is set AND present in the lookup, otherwise it stays loose. - New `EventCluster.svelte` — the contained event card (same-year event header + edit link, or cross-year ✉ text header; first-5 + show-more). - `LetterCard.svelte` gains `compact` + `variant='event'` (the `.lcard.ev` in-card letter). - `YearBand.svelte` rebuilt to render event clusters inline; loose letters keep the alternating layout and density strip, and the strip counts **only** loose letters (no duplication). - `TimelineView.svelte` builds the event lookup once and threads it + `canWrite` to each band. - `+page.svelte` drops the grouping meta segment; the unused `timeline_grouping_date` key removed from de/en/es. - New `timeline_bucket_show_more`/`_less` keys in all locales. - REQ-010 `{@html}` grep gate over `lib/timeline/`. ## Tests (real runs) - Backend `TimelineServiceTest`: **30 passed** (incl. the 2 new `linkedEventId` tests); `DerivedEventsAssemblyTest`: 17 passed; backend main sources compile. - Frontend client sweep (`LetterCard`, `EventCluster`, `YearBand`, `TimelineView`, `zeitstrahl/page`): **81 passed** (5 files). - Frontend server sweep (`eventClustering`, `messages`, `timeline-no-raw-html`): **18 passed** (3 files). - `svelte-check`: no new errors in the touched files (pre-existing baseline noise elsewhere unchanged). RTM: thirteen `REQ-001..013` rows added for #850 (feature `inline-event-clustering`), Status Done. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
marcel reviewed 2026-06-15 21:01:50 +02:00
marcel left a comment
Author
Owner

Automated /code-review high (recall-biased, 7 finder angles → verify). 10 findings below, most-severe first. Several are behavior/UX judgement calls rather than crashes — flagging for you to confirm intent. The backend lazy-access is safe (assemble() is @Transactional(readOnly=true)), all 16-arg DTO constructor sites compile, the removed timeline_grouping_date key has no dangling refs, and i18n {count} parity is correct.

Automated `/code-review high` (recall-biased, 7 finder angles → verify). 10 findings below, most-severe first. Several are behavior/UX judgement calls rather than crashes — flagging for you to confirm intent. The backend lazy-access is safe (`assemble()` is `@Transactional(readOnly=true)`), all 16-arg DTO constructor sites compile, the removed `timeline_grouping_date` key has no dangling refs, and i18n `{count}` parity is correct.
@@ -258,0 +279,4 @@
if (letterDocIds.isEmpty()) return Map.of();
Map<UUID, UUID> eventByDocId = new HashMap<>();
for (TimelineEvent ev : events) {
Author
Owner

[Efficiency] resolveLetterEventLinks hydrates ev.getDocuments() for ALL events, including those filtered out of the timeline. It iterates the full allEvents set and touches the lazy documents collection on each, even events skipped by the type/person/generation/year predicates in the event loop. @BatchSize(50) caps this at ceil(N/50) queries (no N+1) but still transfers every event's join rows. On a large archive (e.g. 1000 events, 50 visible) it fires ~20 batch queries to load collections that are mostly discarded. Building the map only over events that survived filtering — or only those with ≥1 document — would bound the I/O to what's shown.

**[Efficiency] `resolveLetterEventLinks` hydrates `ev.getDocuments()` for ALL events, including those filtered out of the timeline.** It iterates the full `allEvents` set and touches the lazy `documents` collection on each, even events skipped by the type/person/generation/year predicates in the event loop. `@BatchSize(50)` caps this at ceil(N/50) queries (no N+1) but still transfers every event's join rows. On a large archive (e.g. 1000 events, 50 visible) it fires ~20 batch queries to load collections that are mostly discarded. Building the map only over events that survived filtering — or only those with ≥1 document — would bound the I/O to what's shown.
@@ -258,0 +284,4 @@
if (linkedDocs == null) continue;
for (Document linked : linkedDocs) {
if (letterDocIds.contains(linked.getId())) {
eventByDocId.putIfAbsent(linked.getId(), ev.getId());
Author
Owner

[Determinism] Multi-event letter link is non-deterministic. When a document is in the documents set of two curated events, putIfAbsent over events (from eventRepository.findAll(), whose iteration order JPA does not guarantee) keeps whichever event is seen first. Across re-seeds / VACUUM / deploys the same letter can cluster under a different event with no data change — it visibly jumps between event cards. The javadoc acknowledges 'first by repository iteration order wins', but that order is undefined; consider an explicit, stable tiebreak (e.g. lowest eventId, or earliest event date).

**[Determinism] Multi-event letter link is non-deterministic.** When a document is in the `documents` set of two curated events, `putIfAbsent` over `events` (from `eventRepository.findAll()`, whose iteration order JPA does not guarantee) keeps whichever event is seen first. Across re-seeds / VACUUM / deploys the same letter can cluster under a different event with no data change — it visibly jumps between event cards. The javadoc acknowledges 'first by repository iteration order wins', but that order is undefined; consider an explicit, stable tiebreak (e.g. lowest eventId, or earliest event date).
@@ -0,0 +42,4 @@
// Event-as-header: a same-year curated event renders as this card's header, mirroring EventPill —
// glyph + title + date · provenance + an edit pencil for a curator. The title is never repeated
// as a separate floating pill (REQ-002).
const accent = $derived(event ? getAccentConfig(event) : null);
Author
Owner

[Reuse / DRY] EventCluster's same-year header re-implements EventPill's visual + logic contract. The accent/provenance/eventSubtitle/canEdit deriveds (and the canWrite && !derived && eventId != null edit gate) plus the glyph-circle markup, serif title, subtitle, and data-testid="event-edit" anchor to /zeitstrahl/events/{eventId}/edit are copied near-verbatim from EventPill.svelte. The curator-affordance edit gate is called out as load-bearing in CLAUDE.md's DTO contract — having it stated in two components invites drift. Consider extracting a shared <EventHeader> (or an eventHeaderModel(entry, canWrite) helper alongside getAccentConfig) and using GlyphLabel for the glyph/sr-only pairs.

**[Reuse / DRY] EventCluster's same-year header re-implements EventPill's visual + logic contract.** The `accent`/`provenance`/`eventSubtitle`/`canEdit` deriveds (and the `canWrite && !derived && eventId != null` edit gate) plus the glyph-circle markup, serif title, subtitle, and `data-testid="event-edit"` anchor to `/zeitstrahl/events/{eventId}/edit` are copied near-verbatim from `EventPill.svelte`. The curator-affordance edit gate is called out as load-bearing in CLAUDE.md's DTO contract — having it stated in two components invites drift. Consider extracting a shared `<EventHeader>` (or an `eventHeaderModel(entry, canWrite)` helper alongside `getAccentConfig`) and using `GlyphLabel` for the glyph/sr-only pairs.
@@ -0,0 +107,4 @@
class="flex items-center gap-2 border-b border-line px-3 py-2"
>
<span class="font-serif text-sm font-bold whitespace-pre-line text-ink">
<span aria-hidden="true"></span>
Author
Owner

[Accessibility] The cross-year glyph is aria-hidden with no sr-only label, unlike the same-year header. The same-year header pairs <span aria-hidden>{accent.glyph}</span> with <span class="sr-only">{accent.label}</span> (lines 80–81), but this cross-year branch emits a bare aria-hidden . A screen-reader user hears only the title (Briefe von der Front) with no cue that this is a letter group. Reuse GlyphLabel here (as LetterCard already does: glyph="✉" label={m.timeline_letter_glyph_label()}). Same gap on the bare · {count} spans (lines 88, 113) — · 2 announces with no semantic context.

**[Accessibility] The cross-year `✉` glyph is `aria-hidden` with no `sr-only` label, unlike the same-year header.** The same-year header pairs `<span aria-hidden>{accent.glyph}</span>` with `<span class="sr-only">{accent.label}</span>` (lines 80–81), but this cross-year branch emits a bare `aria-hidden` `✉`. A screen-reader user hears only the title (`Briefe von der Front`) with no cue that this is a letter group. Reuse `GlyphLabel` here (as `LetterCard` already does: `glyph="✉" label={m.timeline_letter_glyph_label()}`). Same gap on the bare `· {count}` spans (lines 88, 113) — `· 2` announces with no semantic context.
@@ -19,0 +35,4 @@
// Inside an event card the year frames the time, and these archive titles already
// embed the date — so the compact in-card letter drops the redundant date chip when a
// title is present, halving the row height and killing the duplicate date (#850).
const showDate = $derived(!compact || !entry.title);
Author
Owner

[Correctness — info loss] showDate = !compact || !entry.title drops the date for any titled compact letter, even when the title has no date in it. The comment assumes archive titles always embed the date (H-0023 – 6. Juli 1916), but title is free-form OCR/import text. A clustered letter titled e.g. Brief an Mutter renders with no date chip at all — and inside an event card the year band only frames the year, so the reader loses the letter's month/day entirely. The component can't infer date-redundancy from title presence alone; consider a explicit flag from the caller, or only suppress when the title actually contains the formatted date.

**[Correctness — info loss] `showDate = !compact || !entry.title` drops the date for *any* titled compact letter, even when the title has no date in it.** The comment assumes archive titles always embed the date (`H-0023 – 6. Juli 1916`), but `title` is free-form OCR/import text. A clustered letter titled e.g. `Brief an Mutter` renders with **no date chip at all** — and inside an event card the year band only frames the *year*, so the reader loses the letter's month/day entirely. The component can't infer date-redundancy from title presence alone; consider a explicit flag from the caller, or only suppress when the title actually contains the formatted date.
@@ -42,0 +67,4 @@
// A curated event whose letters live in THIS band becomes the contained card's
// header — its title reads once, no separate pill (REQ-002). Otherwise it stays a
// plain pill/world-band (REQ-005).
const cluster = entry.eventId ? clusters.find((c) => c.eventId === entry.eventId) : undefined;
Author
Owner

[Correctness — possible REQ-005 regression] A HISTORICAL curated event with ≥1 linked letter silently loses its WorldBand treatment. buildEventLookup keys on kind === 'EVENT' && eventId without checking type, so HISTORICAL curated events enter the lookup and produce clusters. Here entry.eventId ? clusters.find(...) matches regardless of type and pushes { t: 'eventcard' }, which always renders <EventCluster>. The type === 'HISTORICAL' → <WorldBand> branch (line 121) is only reachable on the plain { t: 'event' } path.

So the moment any letter links to a world event, it downgrades from the full-width world band to a mint family-style card (and EventCluster's header has no HISTORICAL-specific styling beyond the navy/mint glyph swap). If world events were meant to keep WorldBand chrome, this is a regression — confirm intent or gate clustering to type !== 'HISTORICAL'.

**[Correctness — possible REQ-005 regression] A HISTORICAL curated event with ≥1 linked letter silently loses its WorldBand treatment.** `buildEventLookup` keys on `kind === 'EVENT' && eventId` without checking `type`, so HISTORICAL curated events enter the lookup and produce clusters. Here `entry.eventId ? clusters.find(...)` matches regardless of type and pushes `{ t: 'eventcard' }`, which always renders `<EventCluster>`. The `type === 'HISTORICAL' → <WorldBand>` branch (line 121) is only reachable on the plain `{ t: 'event' }` path. So the moment any letter links to a world event, it downgrades from the full-width world band to a mint family-style card (and EventCluster's header has no HISTORICAL-specific styling beyond the navy/mint glyph swap). If world events were meant to keep WorldBand chrome, this is a regression — confirm intent or gate clustering to `type !== 'HISTORICAL'`.
@@ -42,0 +74,4 @@
} else {
out.push({ t: 'event', entry });
}
} else if (loose.includes(entry)) {
Author
Owner

[Efficiency] loose.includes(entry) inside the loop over year.entries makes row assembly O(L²) in loose letters. loose is an Array, so .includes is O(L), run once per LETTER entry. A dense year band (the >12 path already exists) with a few hundred loose letters does hundreds×hundreds reference comparisons every time the rows $derived.by recomputes (e.g. on any layer-filter change that re-renders bands). Build a Set from split.loose once and test membership in O(1). (Same array-scan smell on clusters.find at line 72 and consumed.includes at line 94, both lower-impact.)

**[Efficiency] `loose.includes(entry)` inside the loop over `year.entries` makes row assembly O(L²) in loose letters.** `loose` is an Array, so `.includes` is O(L), run once per LETTER entry. A dense year band (the >12 path already exists) with a few hundred loose letters does hundreds×hundreds reference comparisons every time the `rows` `$derived.by` recomputes (e.g. on any layer-filter change that re-renders bands). Build a `Set` from `split.loose` once and test membership in O(1). (Same array-scan smell on `clusters.find` at line 72 and `consumed.includes` at line 94, both lower-impact.)
@@ -42,0 +90,4 @@
// Cross-year clusters: a cluster whose event is NOT a same-year EVENT entry renders as a
// text-header card (no pill, no edit link) holding this year's linked letters (REQ-004).
for (const cluster of clusters) {
Author
Owner

[Correctness — chronology, REQ-003] Cross-year clusters are appended after every event and loose letter, breaking within-band time order. The backend sorts each band by WITHIN_BAND_ORDER (chronological), and the main loop emits events + loose letters in that order — but this second pass pushes every cross-year eventcard to the end of out, regardless of its letters' dates.

Scenario: 1917 band has a loose letter dated 1917-11 plus two letters dated 1917-02 linked to a 1916 event. The Feb letters render in a card placed below the November letter — earlier-dated content sits visually below later-dated content, contradicting the strict-time reading the old YearBand guaranteed. Consider interleaving cross-year cards at the position of their earliest letter instead of appending.

**[Correctness — chronology, REQ-003] Cross-year clusters are appended after every event and loose letter, breaking within-band time order.** The backend sorts each band by `WITHIN_BAND_ORDER` (chronological), and the main loop emits events + loose letters in that order — but this second pass pushes every cross-year `eventcard` to the *end* of `out`, regardless of its letters' dates. Scenario: 1917 band has a loose letter dated 1917-11 plus two letters dated 1917-02 linked to a 1916 event. The Feb letters render in a `✉` card placed *below* the November letter — earlier-dated content sits visually below later-dated content, contradicting the strict-time reading the old YearBand guaranteed. Consider interleaving cross-year cards at the position of their earliest letter instead of appending.
@@ -0,0 +32,4 @@
const lookup = new Map<string, string>();
const collect = (entries: TimelineEntryDTO[]) => {
for (const entry of entries) {
if (entry.kind === 'EVENT' && entry.eventId) lookup.set(entry.eventId, entry.title ?? '');
Author
Owner

[Edge case] An empty/whitespace event title produces a bare cross-year card. lookup.set(entry.eventId, entry.title ?? '') stores '' for a titleless event; in splitYearLetters the title !== undefined check is still true for '', so the letters cluster, and a cross-year EventCluster renders with no label — an unlabeled mystery card. Curated event titles are normally required, so low-probability, but worth a guard (skip empty titles, or fall back to a placeholder).

**[Edge case] An empty/whitespace event title produces a bare `✉` cross-year card.** `lookup.set(entry.eventId, entry.title ?? '')` stores `''` for a titleless event; in `splitYearLetters` the `title !== undefined` check is still true for `''`, so the letters cluster, and a cross-year EventCluster renders `✉ ` with no label — an unlabeled mystery card. Curated event titles are normally required, so low-probability, but worth a guard (skip empty titles, or fall back to a placeholder).
@@ -0,0 +36,4 @@
}
};
for (const band of timeline.years) collect(band.entries);
collect(timeline.undated);
Author
Owner

[UX — orphaned presentation] Undated curated events drive cross-year cards. buildEventLookup collects timeline.undated too, so an undated event (which renders as an EventPill in the undated bucket, explicitly out of clustering scope per TimelineView's comment) still seeds clusters: its dated linked letters scatter into year bands and each collapses into a ✉ {title} cross-year card with no edit link and no spatial tie to the pill. The reader sees the event title twice with no relationship, and the letters are detached from their event. Consider excluding undated-bucket events from the lookup, or otherwise linking the two.

**[UX — orphaned presentation] Undated curated events drive cross-year cards.** `buildEventLookup` collects `timeline.undated` too, so an undated event (which renders as an `EventPill` in the undated bucket, explicitly out of clustering scope per TimelineView's comment) still seeds clusters: its *dated* linked letters scatter into year bands and each collapses into a `✉ {title}` cross-year card with no edit link and no spatial tie to the pill. The reader sees the event title twice with no relationship, and the letters are detached from their event. Consider excluding undated-bucket events from the lookup, or otherwise linking the two.
Author
Owner

📋 Requirements Engineer — PR Review

Verdict: ⚠️ Approved with concerns

Traceability against the #850 issue body (the spec). Every REQ-NNN is implemented, tested, and the RTM rows are added with Status: Done and in sync with the code.

REQ Implemented Test RTM Done
001 no grouping control +page.svelte drops segment; GroupingControl never ported page.svelte.spec.ts
002 contained event card EventCluster.svelte EventCluster/YearBand specs
003 first-5 + show-more CLUSTER_PREVIEW=5 8→5 toggle + 44px
004 cross-year ✉ card EventCluster else-branch YearBand spec
005 no letters → plain pill/band YearBand spec
006 unlinked → loose splitYearLetters eventClustering.spec
007 strip counts only loose "15 not 18"
008 filter-then-cluster lookup-presence gate eventClustering.spec
009 nullable linkedEventId, batched, no migration backend 2 TimelineServiceTest
010 {...} escaping, no {@html} grep-gate + XSS spec
011 keep CTA+filter, drop grouping page.svelte.spec.ts
012 show-more keys, drop grouping key de/en/es messages.spec.ts
013 error state unchanged (regression) ⚠️ no new test — relies on #779

Nothing is unimplemented or untested, so this is not a traceability blocker. Two spec gaps the code surfaced (behavior exists that no REQ pins down — decide and add a REQ, or accept):

  • HISTORICAL event + linked letters is unspecified. REQ-002 says "a curated event … contained card" without distinguishing PERSONAL vs HISTORICAL, while #779 REQ-009 mandates HISTORICAL → full-width WorldBand. The implementation clusters HISTORICAL events too (downgrading the band). The issue's own User Journey says world-bands "render exactly as before." Add a REQ stating which wins.
  • Cross-year card ordering is unspecified. REQ-001 promises a "single chronological timeline" but no REQ states where a cross-year card sits relative to loose letters in the same band; the code appends it at the end. Add an ordering REQ or accept the grouping order explicitly.

Minor: REQ-013 carries no new test on this branch — fine as a regression req, but note it.

Blockers: none (traceability complete).
Concerns: two under-specified interactions above.

### 📋 Requirements Engineer — PR Review **Verdict: ⚠️ Approved with concerns** Traceability against the #850 issue body (the spec). Every `REQ-NNN` is implemented, tested, and the RTM rows are added with `Status: Done` and in sync with the code. | REQ | Implemented | Test | RTM Done | |---|---|---|---| | 001 no grouping control | ✅ `+page.svelte` drops segment; GroupingControl never ported | ✅ `page.svelte.spec.ts` | ✅ | | 002 contained event card | ✅ `EventCluster.svelte` | ✅ `EventCluster`/`YearBand` specs | ✅ | | 003 first-5 + show-more | ✅ `CLUSTER_PREVIEW=5` | ✅ 8→5 toggle + 44px | ✅ | | 004 cross-year ✉ card | ✅ `EventCluster` else-branch | ✅ `YearBand` spec | ✅ | | 005 no letters → plain pill/band | ✅ | ✅ `YearBand` spec | ✅ | | 006 unlinked → loose | ✅ `splitYearLetters` | ✅ `eventClustering.spec` | ✅ | | 007 strip counts only loose | ✅ | ✅ "15 not 18" | ✅ | | 008 filter-then-cluster | ✅ lookup-presence gate | ✅ `eventClustering.spec` | ✅ | | 009 nullable linkedEventId, batched, no migration | ✅ backend | ✅ 2 `TimelineServiceTest` | ✅ | | 010 `{...}` escaping, no `{@html}` | ✅ | ✅ grep-gate + XSS spec | ✅ | | 011 keep CTA+filter, drop grouping | ✅ | ✅ `page.svelte.spec.ts` | ✅ | | 012 show-more keys, drop grouping key | ✅ de/en/es | ✅ `messages.spec.ts` | ✅ | | 013 error state unchanged | ✅ (regression) | ⚠️ no new test — relies on #779 | ✅ | Nothing is unimplemented or untested, so this is not a traceability blocker. Two **spec gaps** the code surfaced (behavior exists that no REQ pins down — decide and add a REQ, or accept): - **HISTORICAL event + linked letters is unspecified.** REQ-002 says "a curated event … contained card" without distinguishing PERSONAL vs HISTORICAL, while #779 REQ-009 mandates HISTORICAL → full-width WorldBand. The implementation clusters HISTORICAL events too (downgrading the band). The issue's own User Journey says world-bands "render exactly as before." Add a REQ stating which wins. - **Cross-year card ordering is unspecified.** REQ-001 promises a "single chronological timeline" but no REQ states where a cross-year card sits relative to loose letters in the same band; the code appends it at the end. Add an ordering REQ or accept the grouping order explicitly. Minor: REQ-013 carries no new test on this branch — fine as a regression req, but note it. **Blockers:** none (traceability complete). **Concerns:** two under-specified interactions above.
Author
Owner

💻 Developer — PR Review (Felix Brandt)

Verdict: 🚫 Changes requested

Confirmed clean:

  • All six positional new TimelineEntryDTO(...) sites updated (3× TimelineEventService, 2× TimelineService, 3× test helpers) — backend compiles, TimelineServiceTest 30 / DerivedEventsAssemblyTest 17 green.
  • api.ts carries only the regenerated linkedEventId? — a generate:api regen, not a hand-edit, so §3.5 satisfied and §4.1 (Do-Not-Touch) not violated.
  • linkedEventId nullable and deliberately not @Schema(REQUIRED) — correct.
  • No new ErrorCode, endpoint, or migration. eventClustering.ts is small, pure, well-named.

Blockers

  1. [Correctness] LetterCard.svelte:showDate = !compact || !entry.title drops the date for any titled compact letter. entry.title is free-form OCR/import text — the comment's assumption that archive titles always embed the date (H-0023 – 6. Juli 1916) does not hold for a letter titled e.g. Brief an Mutter. Inside an event card the band frames only the year, so the reader loses month/day entirely. Suppress only when the title actually contains the formatted date, or take an explicit hideDate prop from the caller. (matches automated finding #4)
  2. [Correctness / confirm intent] HISTORICAL curated event with ≥1 linked letter downgrades from WorldBand to a mint EventCluster card. buildEventLookup keys on kind==='EVENT' && eventId with no type check, so the type==='HISTORICAL' → WorldBand branch in YearBand.svelte:121 becomes unreachable once any letter links to a world event. Gate clustering to type !== 'HISTORICAL', or confirm this is intended (it contradicts #779 REQ-009). (finding #2)
  3. [Correctness / confirm intent] Cross-year clusters are appended after every event + loose letter, breaking within-band chronological order. A 1917 band with a loose Nov letter + two Feb letters linked to a 1916 event renders the Feb card below the Nov letter. Interleave the cross-year card at its earliest letter's position, or document the grouping order as deliberate. (finding #1)

Suggestions

  • O(L²): loose.includes(entry) runs once per LETTER inside the year.entries loop; build a Set(split.loose) once (clusters.find / consumed.includes are lower-impact variants of the same smell). (finding #3)
  • DRY (see Architect): EventCluster's same-year header re-implements EventPill's accent/provenance/canEdit gate and edit anchor near-verbatim — third caller, time to extract.
### 💻 Developer — PR Review (Felix Brandt) **Verdict: 🚫 Changes requested** **Confirmed clean:** - All six positional `new TimelineEntryDTO(...)` sites updated (3× `TimelineEventService`, 2× `TimelineService`, 3× test helpers) — backend compiles, `TimelineServiceTest` 30 / `DerivedEventsAssemblyTest` 17 green. - `api.ts` carries only the regenerated `linkedEventId?` — a `generate:api` regen, not a hand-edit, so §3.5 satisfied and §4.1 (Do-Not-Touch) not violated. - `linkedEventId` nullable and deliberately **not** `@Schema(REQUIRED)` — correct. - No new `ErrorCode`, endpoint, or migration. `eventClustering.ts` is small, pure, well-named. **Blockers** 1. **[Correctness] `LetterCard.svelte:showDate = !compact || !entry.title` drops the date for *any* titled compact letter.** `entry.title` is free-form OCR/import text — the comment's assumption that archive titles always embed the date (`H-0023 – 6. Juli 1916`) does not hold for a letter titled e.g. `Brief an Mutter`. Inside an event card the band frames only the *year*, so the reader loses month/day entirely. Suppress only when the title actually contains the formatted date, or take an explicit `hideDate` prop from the caller. (matches automated finding #4) 2. **[Correctness / confirm intent] HISTORICAL curated event with ≥1 linked letter downgrades from `WorldBand` to a mint `EventCluster` card.** `buildEventLookup` keys on `kind==='EVENT' && eventId` with no `type` check, so the `type==='HISTORICAL' → WorldBand` branch in `YearBand.svelte:121` becomes unreachable once any letter links to a world event. Gate clustering to `type !== 'HISTORICAL'`, or confirm this is intended (it contradicts #779 REQ-009). (finding #2) 3. **[Correctness / confirm intent] Cross-year clusters are appended after every event + loose letter, breaking within-band chronological order.** A 1917 band with a loose Nov letter + two Feb letters linked to a 1916 event renders the Feb card *below* the Nov letter. Interleave the cross-year card at its earliest letter's position, or document the grouping order as deliberate. (finding #1) **Suggestions** - O(L²): `loose.includes(entry)` runs once per LETTER inside the `year.entries` loop; build a `Set(split.loose)` once (`clusters.find` / `consumed.includes` are lower-impact variants of the same smell). (finding #3) - DRY (see Architect): `EventCluster`'s same-year header re-implements `EventPill`'s `accent`/`provenance`/`canEdit` gate and edit anchor near-verbatim — third caller, time to extract.
Author
Owner

🧪 Tester — PR Review (Sara Holt)

Verdict: 🚫 Changes requested

Strong baseline: each REQ has a behavior-named test, factory-driven setup (makeEntry/makeEvent/linkedLetters), real-DOM browser specs, a pure-logic eventClustering.spec.ts, and a self-testing {@html} grep gate. 81 client + 18 server + 30+17 backend green.

Blockers

  1. A test cements a bug. LetterCard.svelte.spec.ts#drops the redundant date line in the compact variant when a title is present asserts the date disappears for any title — it locks in the info-loss the Developer flagged (finding #4). The correct test is the inverse: the date must survive when the title does not embed a date.
  2. Missing coverage for the cross-year ordering case (finding #1). Every cross-year test (YearBand/EventCluster) renders a band of only linked letters, so the append-at-end ordering bug is structurally invisible. Add a band mixing loose + cross-year-linked letters in interleaved dates and assert DOM order.
  3. Missing coverage for HISTORICAL + linked letters (finding #2). REQ-005's test only covers "no linked letters." There is no test asserting what a HISTORICAL curated event with a same-year letter renders — the exact case that changes behavior.

Suggestions

  • REQ-008 is proven only at the splitYearLetters unit level. A YearBand/TimelineView test that toggles a layer off through the real #780 path would prove filter-then-cluster end-to-end (the user-visible contract).
  • REQ-009 is covered by Mockito unit tests only; the analogous #835 root-tag work added a TagServiceIntegrationTest proving the @BatchSize batched pass on real Postgres (Testcontainers). One here would also pin the multi-event determinism gap (finding #9), which currently has no test.
### 🧪 Tester — PR Review (Sara Holt) **Verdict: 🚫 Changes requested** Strong baseline: each REQ has a behavior-named test, factory-driven setup (`makeEntry`/`makeEvent`/`linkedLetters`), real-DOM browser specs, a pure-logic `eventClustering.spec.ts`, and a self-testing `{@html}` grep gate. 81 client + 18 server + 30+17 backend green. **Blockers** 1. **A test cements a bug.** `LetterCard.svelte.spec.ts#drops the redundant date line in the compact variant when a title is present` asserts the date disappears for *any* title — it locks in the info-loss the Developer flagged (finding #4). The correct test is the inverse: the date must **survive** when the title does not embed a date. 2. **Missing coverage for the cross-year ordering case (finding #1).** Every cross-year test (`YearBand`/`EventCluster`) renders a band of *only* linked letters, so the append-at-end ordering bug is structurally invisible. Add a band mixing loose + cross-year-linked letters in interleaved dates and assert DOM order. 3. **Missing coverage for HISTORICAL + linked letters (finding #2).** REQ-005's test only covers "no linked letters." There is no test asserting what a HISTORICAL curated event with a same-year letter renders — the exact case that changes behavior. **Suggestions** - REQ-008 is proven only at the `splitYearLetters` unit level. A `YearBand`/`TimelineView` test that toggles a layer off through the real #780 path would prove filter-then-cluster end-to-end (the user-visible contract). - REQ-009 is covered by Mockito unit tests only; the analogous #835 root-tag work added a `TagServiceIntegrationTest` proving the `@BatchSize` batched pass on real Postgres (Testcontainers). One here would also pin the multi-event determinism gap (finding #9), which currently has no test.
Author
Owner

🔐 Security — PR Review (Nora "NullX" Steiner)

Verdict: Approved

What I checked, adversarially:

  • AuthZ unchanged (§2.1/2.2). GET /api/timeline keeps READ_ALL; no new mutating endpoint, so no @RequirePermission gap. The 401/403 paths from #777 are untouched.
  • No mass-assignment / authorship forgery (CWE-639). linkedEventId is computed server-side in resolveLetterEventLinks, never bound from a request body. Audit fields untouched.
  • No IDOR. A letter only clusters under an event that is present in the caller's own filtered timeline (buildEventLookup is built from the response the reader already receives). linkedEventId discloses only an event id a READ_ALL reader can already see; there is no cross-tenant model here.
  • XSS (CWE-79, §2.5). {@html} appears nowhere in lib/timeline/ — enforced by the new timeline-no-raw-html.spec.ts grep gate, and EventCluster.svelte.spec.ts renders <img src=x onerror=alert(1)> as inert text. Letter/sender/receiver/tag titles all go through {...}.
  • Logging (§2.7). resolveLetterEventLinks logs nothing; no names/ids enter logs.
  • No secrets, no new dependency.

Non-security note: an empty/whitespace event title produces a bare card (finding #8) — a UX edge case, not a vuln.

### 🔐 Security — PR Review (Nora "NullX" Steiner) **Verdict: ✅ Approved** What I checked, adversarially: - **AuthZ unchanged (§2.1/2.2).** `GET /api/timeline` keeps `READ_ALL`; no new mutating endpoint, so no `@RequirePermission` gap. The 401/403 paths from #777 are untouched. - **No mass-assignment / authorship forgery (CWE-639).** `linkedEventId` is computed server-side in `resolveLetterEventLinks`, never bound from a request body. Audit fields untouched. - **No IDOR.** A letter only clusters under an event that is present in the caller's *own* filtered timeline (`buildEventLookup` is built from the response the reader already receives). `linkedEventId` discloses only an event id a `READ_ALL` reader can already see; there is no cross-tenant model here. - **XSS (CWE-79, §2.5).** `{@html}` appears nowhere in `lib/timeline/` — enforced by the new `timeline-no-raw-html.spec.ts` grep gate, and `EventCluster.svelte.spec.ts` renders `<img src=x onerror=alert(1)>` as inert text. Letter/sender/receiver/tag titles all go through `{...}`. - **Logging (§2.7).** `resolveLetterEventLinks` logs nothing; no names/ids enter logs. - **No secrets, no new dependency.** Non-security note: an empty/whitespace event title produces a bare `✉` card (finding #8) — a UX edge case, not a vuln.
Author
Owner

🚀 DevOps — PR Review (Tobias Wendt)

Verdict: Approved

Deployability is clean:

  • No Flyway migration. REQ-009 reuses the existing timeline_event_documents join (ADR-040) and computes the link on read — correct, nothing to roll back, no V<n> to verify.
  • No new env var, CI step, dependency, or artifact-action bump. Nothing for the Gitea act_runner to choke on; no CI guard weakened (§4.4 intact).
  • api.ts change is a generate:api regen, not a hand-edit — consistent with the single backend field added.
  • Management/app port split (8081/8080) untouched; no /actuator/* exposure change.

Note (operational, minor): resolveLetterEventLinks iterates the full eventRepository.findAll() and touches the lazy ev.getDocuments() on every event, including those filtered out of the response. @BatchSize(50) caps this at ⌈N/50⌉ queries (no N+1), but it still transfers join rows for events that are never shown. On a large archive (e.g. 1000 events, 50 visible) that's ~20 batch queries loading collections that get discarded. Bounding the map to events that survived the filter (or have ≥1 document) keeps the I/O proportional to what's rendered. (finding #10)

### 🚀 DevOps — PR Review (Tobias Wendt) **Verdict: ✅ Approved** Deployability is clean: - **No Flyway migration.** REQ-009 reuses the existing `timeline_event_documents` join (ADR-040) and computes the link on read — correct, nothing to roll back, no `V<n>` to verify. - **No new env var, CI step, dependency, or artifact-action bump.** Nothing for the Gitea `act_runner` to choke on; no CI guard weakened (§4.4 intact). - **`api.ts` change is a `generate:api` regen**, not a hand-edit — consistent with the single backend field added. - Management/app port split (8081/8080) untouched; no `/actuator/*` exposure change. **Note (operational, minor):** `resolveLetterEventLinks` iterates the full `eventRepository.findAll()` and touches the lazy `ev.getDocuments()` on *every* event, including those filtered out of the response. `@BatchSize(50)` caps this at ⌈N/50⌉ queries (no N+1), but it still transfers join rows for events that are never shown. On a large archive (e.g. 1000 events, 50 visible) that's ~20 batch queries loading collections that get discarded. Bounding the map to events that survived the filter (or have ≥1 document) keeps the I/O proportional to what's rendered. (finding #10)
Author
Owner

🎨 UI/UX — PR Review (Leonie Voss)

Verdict: ⚠️ Approved with concerns

Good work: semantic tokens throughout (bg-surface, border-line, border-l-brand-mint, text-ink-3, text-brand-navy — no raw hex); the show-more toggle has aria-expanded + a real 44px min-height (REQ-003); new strings added to de/en/es with {count} placeholders (REQ-012); the same-year edit ✎ pairs an aria-hidden glyph with an sr-only label.

Concerns

  1. [a11y] The cross-year header glyph is aria-hidden with no sr-only label — unlike the same-year header (which pairs glyph + sr-only) and LetterCard (which uses GlyphLabel). A screen-reader user hears only the title with no cue that this is a letter group. Reuse GlyphLabel glyph="✉" label={m.timeline_letter_glyph_label()}. The bare · {count} spans likewise announce as "· 2" with no context. (finding #6)
  2. [Info loss / senior audience] Compact letters drop the date whenever a title is present (finding #4). Inside the card the band frames only the year, so the 60+ reader loses the letter's month/day. Keep the date chip unless the title demonstrably contains it.
  3. [Chronology] Cross-year cards render at the bottom of the band regardless of their letters' dates (finding #1) — earlier-dated content sits visually below later-dated content, which the strict-time timeline otherwise guarantees.
  4. [Regression] A HISTORICAL event with linked letters loses its full-width world band and becomes a family-style mint card (finding #2) — the world layer's distinct treatment disappears.

Minor: in-card letter text drops to text-xs (12px) — exactly the accessibility floor for this audience; acceptable but at the edge.

Biggest gap in one line: the cross-year card is the only timeline group with no screen-reader cue for its glyph.

### 🎨 UI/UX — PR Review (Leonie Voss) **Verdict: ⚠️ Approved with concerns** Good work: semantic tokens throughout (`bg-surface`, `border-line`, `border-l-brand-mint`, `text-ink-3`, `text-brand-navy` — no raw hex); the show-more toggle has `aria-expanded` + a real 44px `min-height` (REQ-003); new strings added to de/en/es with `{count}` placeholders (REQ-012); the same-year edit ✎ pairs an `aria-hidden` glyph with an `sr-only` label. **Concerns** 1. **[a11y] The cross-year `✉` header glyph is `aria-hidden` with no `sr-only` label** — unlike the same-year header (which pairs glyph + `sr-only`) and `LetterCard` (which uses `GlyphLabel`). A screen-reader user hears only the title with no cue that this is a letter group. Reuse `GlyphLabel glyph="✉" label={m.timeline_letter_glyph_label()}`. The bare `· {count}` spans likewise announce as "· 2" with no context. (finding #6) 2. **[Info loss / senior audience] Compact letters drop the date whenever a title is present** (finding #4). Inside the card the band frames only the *year*, so the 60+ reader loses the letter's month/day. Keep the date chip unless the title demonstrably contains it. 3. **[Chronology] Cross-year cards render at the bottom of the band** regardless of their letters' dates (finding #1) — earlier-dated content sits visually below later-dated content, which the strict-time timeline otherwise guarantees. 4. **[Regression] A HISTORICAL event with linked letters loses its full-width world band** and becomes a family-style mint card (finding #2) — the world layer's distinct treatment disappears. **Minor:** in-card letter text drops to `text-xs` (12px) — exactly the accessibility floor for this audience; acceptable but at the edge. Biggest gap in one line: the cross-year card is the only timeline group with no screen-reader cue for its glyph.
Author
Owner

🏛️ Architect — PR Review (Markus Keller)

Verdict: ⚠️ Approved with concerns

Systemic fit is good: EventCluster lives in lib/timeline/, linkedEventId is a computed DTO field (no DB column, reuses timeline_event_documents / ADR-040), no new domain, no controller→repository or cross-domain repository access, and the view is assembled inside the @Transactional(readOnly=true) assemble() (§1.6). No Accepted ADR is contradicted and none is required. No doc-table update is triggered — no new route, domain, migration, ErrorCode, or Permission.

Concerns

  1. [DRY / rule-of-three] EventCluster re-implements EventPill's header model. The accent/provenance/eventSubtitle deriveds and — load-bearing — the edit gate canWrite && !event.derived && event.eventId != null plus the /zeitstrahl/events/{eventId}/edit anchor are copied near-verbatim. That gate is the curator-affordance contract documented in CLAUDE.md's TimelineEntryDTO row; it now lives in EventPill, WorldBand, and EventCluster — the third caller. §3.2 (KISS-over-DRY, extract on the third real caller) now favours a shared EventHeader / eventHeaderModel(entry, canWrite) so the security-relevant gate has one source of truth. (finding #5)
  2. [Behavioral boundary] Clustering HISTORICAL events blurs the #779 layer distinction (HISTORICAL → WorldBand, PERSONAL → pill). This is a deliberate-design decision, not an incidental one — make it explicitly (gate to type !== 'HISTORICAL' if world events should never cluster) so the boundary stays legible. (finding #2)
  3. [Determinism] The multi-event letter link resolves by eventRepository.findAll() iteration order (JPA does not guarantee it) via putIfAbsent. The javadoc acknowledges "first by repository iteration order wins," but that order is undefined — a letter can jump between event cards across re-seeds/VACUUM with no data change. Add a stable tiebreak (lowest eventId or earliest event date) so the link is a deterministic property of the data. (finding #9)

No ADR is owed; these are refinements within the chosen design.

### 🏛️ Architect — PR Review (Markus Keller) **Verdict: ⚠️ Approved with concerns** Systemic fit is good: `EventCluster` lives in `lib/timeline/`, `linkedEventId` is a computed DTO field (no DB column, reuses `timeline_event_documents` / ADR-040), no new domain, no controller→repository or cross-domain repository access, and the view is assembled inside the `@Transactional(readOnly=true)` `assemble()` (§1.6). No Accepted ADR is contradicted and none is required. No doc-table update is triggered — no new route, domain, migration, `ErrorCode`, or `Permission`. **Concerns** 1. **[DRY / rule-of-three] `EventCluster` re-implements `EventPill`'s header model.** The `accent`/`provenance`/`eventSubtitle` deriveds and — load-bearing — the edit gate `canWrite && !event.derived && event.eventId != null` plus the `/zeitstrahl/events/{eventId}/edit` anchor are copied near-verbatim. That gate is the curator-affordance contract documented in CLAUDE.md's `TimelineEntryDTO` row; it now lives in `EventPill`, `WorldBand`, and `EventCluster` — the third caller. §3.2 (KISS-over-DRY, extract on the third real caller) now favours a shared `EventHeader` / `eventHeaderModel(entry, canWrite)` so the security-relevant gate has one source of truth. (finding #5) 2. **[Behavioral boundary] Clustering HISTORICAL events blurs the #779 layer distinction** (HISTORICAL → WorldBand, PERSONAL → pill). This is a deliberate-design decision, not an incidental one — make it explicitly (gate to `type !== 'HISTORICAL'` if world events should never cluster) so the boundary stays legible. (finding #2) 3. **[Determinism] The multi-event letter link resolves by `eventRepository.findAll()` iteration order** (JPA does not guarantee it) via `putIfAbsent`. The javadoc acknowledges "first by repository iteration order wins," but that order is undefined — a letter can jump between event cards across re-seeds/VACUUM with no data change. Add a stable tiebreak (lowest `eventId` or earliest event date) so the link is a deterministic property of the data. (finding #9) No ADR is owed; these are refinements within the chosen design.
Author
Owner

Review concerns addressed (pushed bf73d8d..bcf95e43, 13 atomic commits)

All 10 automated findings and the persona blockers/concerns are resolved. Four were product/scope calls — decided with the author before implementing:

  • HISTORICAL events → gate to PERSONAL only (world events keep their WorldBand) — new REQ-014.
  • Cross-year cards → interleave at the earliest linked letter — new REQ-015.
  • Compact letter date → hide only when the title actually embeds the formatted date.
  • DRY → extract a shared EventHeader now.

REQ-014/REQ-015 were added to the #850 issue body (the spec) + RTM (179ada13).

Blockers

# Concern (reviewer) Fix Commit
4 Compact letter drops the date for any title — OCR titles ≠ dates (Dev-1/Tester-1/UX-2) Date chip drops only when the title contains the formatted date; the bug-cementing spec rewritten to the inverse + the embeds-date case a9027cea
2 HISTORICAL + linked letter downgrades WorldBand → mint card (Dev-2/Tester-3/UX-4/Arch-2) buildEventLookup excludes HISTORICAL → world events keep WorldBand, letters stay loose (REQ-014); eventClustering + TimelineView tests 0fc7ef5d
1 Cross-year cards appended at band end, breaking chronology (Dev-3/Tester-2/UX-3) Cross-year card now emits at its earliest linked letter's position (REQ-015); YearBand interleave test (loose Nov before/after Feb cluster) 81e0dfb9

Suggestions / concerns

# Concern Fix Commit
9 Multi-event link non-deterministic (putIfAbsent over findAll() order) (Arch-3) Stable order: earliest event date, then eventId; determinism unit test (order-independent) 5a21843c
10 Link pass hydrates getDocuments() for all events (DevOps) Bounded to events that survive the filter; off-screen events never hydrated b5319876
7 Undated events seed orphaned cross-year cards Undated bucket excluded from the lookup 8cc11aec
8 Empty/whitespace title → bare card Empty-title events skipped in the lookup 4e704ae4
6 Cross-year + · {count} unlabeled for screen readers (UX-1) GlyphLabel for the glyph; sr-only {count} Briefe via a new Paraglide key (de/en/es) 30384fa5, a68f7ee5
3 loose.includes() O(L²) splitYearLetters returns its byEvent map; O(1) membership/lookup in row assembly 70a76904
5 EventCluster re-implements EventPill's header + edit gate (Arch-1) canEditEvent() single-sources the security gate (EventPill/WorldBand/EventCluster); shared EventHeader component d450f97b, bcf95e43

Verification (real runs)

  • Backend TimelineServiceTest: 32 passed (was 30; +determinism, +filtered-out-event), clean compile / BUILD SUCCESS.
  • Frontend client (LetterCard, EventCluster, EventHeader, EventPill, WorldBand, YearBand, TimelineView): 99 passed.
  • Frontend server (eventClustering, eventCardConfig, timeline-no-raw-html, messages): 30 passed.
  • svelte-check: no new errors in touched files; new .svelte validated through the Svelte MCP autofixer; npm run lint green (pre-commit) on every commit.
  • RTM REQ-014/REQ-015 → Done; REQ-009 row now cites the determinism test.

Consciously deferred (Tester suggestions, not blockers)

  • REQ-008 end-to-end filter-toggle test through the #780 layer path — currently proven at the splitYearLetters unit level (lookup-presence gate). Happy to add the TimelineView-level test if you'd like it in this PR.
  • REQ-009 Testcontainers integration test — the determinism gap is now closed by construction (deterministic sort + unit test), so this is optional; say the word and I'll add the real-Postgres @BatchSize pass like #835's TagServiceIntegrationTest.
## Review concerns addressed (pushed `bf73d8d..bcf95e43`, 13 atomic commits) All 10 automated findings and the persona blockers/concerns are resolved. Four were product/scope calls — decided with the author before implementing: - **HISTORICAL events** → gate to PERSONAL only (world events keep their WorldBand) — new **REQ-014**. - **Cross-year cards** → interleave at the earliest linked letter — new **REQ-015**. - **Compact letter date** → hide only when the title actually embeds the formatted date. - **DRY** → extract a shared `EventHeader` now. REQ-014/REQ-015 were added to the #850 issue body (the spec) + RTM (`179ada13`). ### Blockers | # | Concern (reviewer) | Fix | Commit | |---|---|---|---| | 4 | Compact letter drops the date for *any* title — OCR titles ≠ dates (Dev-1/Tester-1/UX-2) | Date chip drops **only** when the title contains the formatted date; the bug-cementing spec rewritten to the inverse + the embeds-date case | `a9027cea` | | 2 | HISTORICAL + linked letter downgrades WorldBand → mint card (Dev-2/Tester-3/UX-4/Arch-2) | `buildEventLookup` excludes HISTORICAL → world events keep WorldBand, letters stay loose (REQ-014); `eventClustering` + `TimelineView` tests | `0fc7ef5d` | | 1 | Cross-year cards appended at band end, breaking chronology (Dev-3/Tester-2/UX-3) | Cross-year card now emits at its earliest linked letter's position (REQ-015); `YearBand` interleave test (loose Nov before/after Feb cluster) | `81e0dfb9` | ### Suggestions / concerns | # | Concern | Fix | Commit | |---|---|---|---| | 9 | Multi-event link non-deterministic (`putIfAbsent` over `findAll()` order) (Arch-3) | Stable order: earliest event date, then `eventId`; determinism unit test (order-independent) | `5a21843c` | | 10 | Link pass hydrates `getDocuments()` for all events (DevOps) | Bounded to events that survive the filter; off-screen events never hydrated | `b5319876` | | 7 | Undated events seed orphaned cross-year cards | Undated bucket excluded from the lookup | `8cc11aec` | | 8 | Empty/whitespace title → bare `✉` card | Empty-title events skipped in the lookup | `4e704ae4` | | 6 | Cross-year `✉` + `· {count}` unlabeled for screen readers (UX-1) | `GlyphLabel` for the glyph; sr-only `{count} Briefe` via a new Paraglide key (de/en/es) | `30384fa5`, `a68f7ee5` | | 3 | `loose.includes()` O(L²) | `splitYearLetters` returns its `byEvent` map; O(1) membership/lookup in row assembly | `70a76904` | | 5 | `EventCluster` re-implements `EventPill`'s header + edit gate (Arch-1) | `canEditEvent()` single-sources the security gate (EventPill/WorldBand/EventCluster); shared `EventHeader` component | `d450f97b`, `bcf95e43` | ### Verification (real runs) - **Backend** `TimelineServiceTest`: **32 passed** (was 30; +determinism, +filtered-out-event), clean compile / BUILD SUCCESS. - **Frontend client** (LetterCard, EventCluster, **EventHeader**, EventPill, WorldBand, YearBand, TimelineView): **99 passed**. - **Frontend server** (eventClustering, eventCardConfig, timeline-no-raw-html, messages): **30 passed**. - `svelte-check`: no new errors in touched files; new `.svelte` validated through the Svelte MCP autofixer; `npm run lint` green (pre-commit) on every commit. - RTM REQ-014/REQ-015 → `Done`; REQ-009 row now cites the determinism test. ### Consciously deferred (Tester *suggestions*, not blockers) - **REQ-008 end-to-end filter-toggle test** through the #780 layer path — currently proven at the `splitYearLetters` unit level (lookup-presence gate). Happy to add the `TimelineView`-level test if you'd like it in this PR. - **REQ-009 Testcontainers integration test** — the determinism gap is now closed by construction (deterministic sort + unit test), so this is optional; say the word and I'll add the real-Postgres `@BatchSize` pass like #835's `TagServiceIntegrationTest`.
marcel added 20 commits 2026-06-16 13:38:26 +02:00
For each LETTER entry, resolve the curated event whose documents set
contains the letter, in one batched pass over the events already loaded
(no per-letter query, no new column). The DTO gains a nullable
linkedEventId; non-letter entries keep null.

Refs #850
The event variant adds the .lcard.ev marker for letters living inside a
contained event card; the compact variant tightens the row (py-1, text-xs
title) and drops the redundant date chip when the title already embeds the
date. suppressTagChip lets a caller that already conveys the topic hide the
per-letter root-tag chip. Plain Datum letters are unchanged.

Refs #850
buildEventLookup maps each curated event in the (already layer-filtered)
timeline to its title; splitYearLetters partitions a year's letters into
event clusters (keyed by a linkedEventId present in the lookup) and the
loose chronological remainder. A letter linking to a filtered-out event
falls back to loose (filter-then-cluster); each letter appears once.

Refs #850
A curated event with linked letters renders as one contained card: the
event is the header (accent glyph, title, date · provenance, count, and a
curator edit link), its letters sit inside as compact .lcard.ev cards. The
first CLUSTER_PREVIEW (5) show, then a keyboard-operable show-more/less
toggle reveals the rest. A cross-year card (no event prop) gets a plain
'✉ title' text header with no edit link. Titles render through default
{...} escaping. Adds timeline_bucket_show_more/less keys to de/en/es.

Refs #850
A curated event with same-year linked letters renders as one EventCluster
card (no separate pill); a cluster whose event lives in another band renders
as a cross-year text-header card. Letterless/derived/world events stay plain
pills/world-bands. Loose letters keep the alternating left/right layout and
fold into one density strip past 12 — and the layout + strip count ONLY the
loose letters, so a clustered letter never re-appears loose.

Refs #850
TimelineView builds the event lookup once over the whole timeline and threads
it (plus canWrite) to every YearBand, so a curated event's letters cluster
under it inline. The /zeitstrahl meta-line drops its 'Gruppierung: Datum'
segment (toggle-free view, REQ-011); the now-unused timeline_grouping_date
key is removed from de/en/es and the messages parity guard, which now asserts
the new show-more/less keys.

Refs #850
The grep gate fails if any lib/timeline component reaches for the raw-HTML
directive (CWE-79, REQ-010). The RTM gains thirteen rows tracing every #850
requirement to its implementation file(s) and test(s), Status Done.

Refs #850
Two spec gaps the Requirements Engineer flagged on PR #851 are now pinned
as requirements in the #850 issue body and traced here (Status Planned,
flipped to Done as the tasks land):

- REQ-014: a HISTORICAL curated event keeps its full-width WorldBand and
  never clusters into a card, even with linked letters.
- REQ-015: a cross-year card sits at its earliest linked letter's
  chronological position, never appended after later-dated loose letters.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A letter whose document is in more than one curated event's `documents`
set was linked by `putIfAbsent` over `eventRepository.findAll()`, whose
iteration order JPA does not guarantee — so the same letter could cluster
under a different event across re-seeds/VACUUM with no data change.

resolveLetterEventLinks now runs over a stably-ordered copy (earliest
event date, undated last, then lowest id), so the link is a deterministic
property of the data. Fixes review finding #9 (Architect-3).

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
resolveLetterEventLinks iterated `eventRepository.findAll()` and touched
the lazy `documents` collection on every event — including events removed
by the type/person/generation/year filters and never shown. On a large
archive that hydrates join rows that are immediately discarded.

It now runs over the events that survived the filter (collected once in
the existing event loop). A letter whose only linking event was filtered
out links to nothing, which matches the frontend's filter-then-cluster
(the letter renders loose either way). Fixes review finding #10 (DevOps).

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buildEventLookup keyed on `kind === 'EVENT' && eventId` with no type
check, so a HISTORICAL curated event with ≥1 linked letter entered the
lookup and rendered as a mint EventCluster card — silently downgrading
from the full-width WorldBand that #779 REQ-009 mandates ("world-bands
render exactly as before").

The lookup now excludes `type === 'HISTORICAL'`, so a world event always
keeps its WorldBand and its letters stay loose chronological. Closes the
spec gap pinned as REQ-014. Fixes review finding #2.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buildEventLookup also collected `timeline.undated`, so an undated curated
event — which renders as a plain EventPill in the undated bucket, out of
clustering scope — still seeded clusters: its dated linked letters
scattered into year bands and each collapsed into a ✉ cross-year card
with no edit link and no spatial tie to the pill, showing the event title
twice with no relationship.

Only year-band events are collected now. Fixes review finding #7.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A titleless or whitespace-only event stored `''` in the lookup, so its
letters still clustered and rendered a label-less `✉` mystery card. The
lookup now skips events whose trimmed title is empty — those letters stay
loose. Fixes review finding #8.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`showDate = !compact || !entry.title` dropped the date chip for ANY titled
compact letter. But titles are free-form OCR/import text — a letter titled
"Brief an Mutter" lost its month/day entirely, and inside an event card
the band frames only the year. The chip now drops only when the formatted
date actually appears in the title (e.g. "H-0023 – 6. Juli 1916"), so the
row-height win holds where valid and no information is lost otherwise.

The spec that asserted the date vanishes for any title is rewritten to the
correct contract, plus an inverse test. Fixes review finding #4.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cross-year card header emitted a bare aria-hidden ✉ with no sr-only
label, unlike the same-year header and LetterCard — a screen-reader user
heard only the title with no cue that this is a letter group. It now uses
the shared GlyphLabel (✉ + sr-only "Brief"). Fixes review finding #6
(glyph half).

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bare "· {count}" spans in both the same-year and cross-year headers
announced as "· 2" with no context. Each now pairs the aria-hidden visible
count with an sr-only "{count} Briefe" via a new Paraglide key
(timeline_cluster_letter_count, present in de/en/es). Fixes review
finding #6 (count half).

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`loose.includes(entry)` ran once per LETTER inside the band loop — O(L²)
on a dense band of hundreds of loose letters, recomputed on every layer
re-render. splitYearLetters now also returns its `byEvent` map, so a
letter's disposition is `byEvent.has(linkedEventId)` and an event's card
is `byEvent.get(eventId)`, both O(1); `consumed` is a plain object. No
behavior change. Fixes review finding #3.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cross-year clusters were appended after every event and loose letter, so a
band with a loose November letter plus February letters linked to another
year's event rendered the February ✉ card BELOW the November letter —
earlier-dated content sitting visually below later-dated content, breaking
the strict-time reading the band guarantees.

A cross-year cluster (no same-year EVENT anchor in this band) now emits its
card at the position of its earliest linked letter, in the band's
chronological order. Closes the spec gap pinned as REQ-015. Fixes review
finding #1.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The security-relevant edit-affordance gate (canWrite && !derived && eventId
!= null) was copied into EventPill, WorldBand, and EventCluster — three
places for one load-bearing contract, inviting drift. It now lives once as
canEditEvent(entry, canWrite) in eventCardConfig, and all three call it. No
behavior change (HISTORICAL is never derived, so WorldBand's gate is
unchanged). First half of review finding #5 (Architect-1).

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
refactor(timeline): extract shared EventHeader for pill + event-card
Some checks failed
CI / Unit & Component Tests (pull_request) Successful in 8m52s
CI / OCR Service Tests (pull_request) Successful in 2m58s
CI / Backend Unit Tests (pull_request) Failing after 13m32s
CI / fail2ban Regex (pull_request) Successful in 1m52s
CI / Semgrep Security Scan (pull_request) Successful in 43s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m51s
SDD Gate / RTM Check (pull_request) Successful in 26s
SDD Gate / Contract Validate (pull_request) Successful in 40s
SDD Gate / Constitution Impact (pull_request) Successful in 31s
621248f941
EventCluster's same-year header re-implemented EventPill's glyph circle,
serif title, provenance subtitle, and the curator edit anchor near-
verbatim — the third copy of that markup. They now share a single
EventHeader component (glyph via GlyphLabel, title, `{date} · provenance`
subtitle, optional sr-only letter count, and the canEditEvent-gated edit
pencil); EventPill keeps only its pill border, EventCluster only its card
chrome. Second half of review finding #5 (Architect-1). No behavior
change — EventPill/EventCluster/YearBand/TimelineView specs stay green.

Refs #850
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
marcel force-pushed feat/issue-850-inline-event-clustering from bcf95e4399 to 621248f941 2026-06-16 13:38:26 +02:00 Compare
marcel merged commit 49d8ab78b4 into main 2026-06-16 14:38:11 +02:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#851