Timeline: filters (person / generation / layer / year range) #780

New Issue

Open

opened 2026-06-07 19:29:27 +02:00 by marcel · 7 comments

marcel commented 2026-06-07 19:29:27 +02:00

Owner

Copy Link

Milestone: Zeitstrahl — Family Timeline
Spec: docs/superpowers/specs/2026-06-07-family-timeline-design.md § "Concept & UX"
Depends on: #5 (assembly endpoint GET /api/timeline + its query params), #7 (global /zeitstrahl view + TimelineView.svelte).

Overview

A presentation-only filter bar, TimelineFilters.svelte, that drives the GET /api/timeline query params for the global /zeitstrahl view. This issue is frontend-only — it stays strictly within the query-param contract already owned by #5 and adds zero backend code, no new endpoint, no migration, no infra, no env var. The canonical pattern to mirror is frontend/src/routes/SearchFilterBar.svelte.

Scope

• TimelineFilters.svelte — a dumb/presentation component: $bindable filter props + onSearch/onSearchImmediate callbacks, no goto inside it.
• URL/query state lives in the route, not the component: /zeitstrahl/+page.svelte builds the query string and calls goto; /zeitstrahl/+page.server.ts reads url.searchParams, coerces/validates, and forwards to GET /api/timeline.
• A small typed helper parseTimelineFilters(searchParams) at src/routes/zeitstrahl/parseTimelineFilters.ts — route-scoped coercion, not a reusable library utility.

File boundary with #7: #7 scaffolds the /zeitstrahl route (creates +page.svelte, +page.server.ts stubs). This issue extends those files with filter wiring; it does not create them from scratch. CLAUDE.md route table and docs/architecture/c4/l3-frontend-*.puml updates are scoped to #7, not this issue.

Out of scope: any backend/contract change, any new Person field, generation-matching semantics (owned by TimelineService in #5).

Filter dimensions (final)

1. Person — personId (UUID string). Reuse PersonTypeahead (project standard, as in SearchFilterBar).
2. Generation — generation (integer 0–10, G0 = oldest, bounds per PersonGeneration). Labeled <select> dropdown (bounded set; a dropdown beats free text). No "family branch" — see Decisions Resolved D1.
3. Layer toggles — three independent toggles inside a <fieldset><legend>Ebenen anzeigen</legend>...</fieldset> for screen-reader group context:
   • Personal events → server-side type=PERSONAL; fires via onSearchImmediate (instant, no blur needed).
   • Historical events → server-side type=HISTORICAL; fires via onSearchImmediate.
   • Letters → client-side hide (no backing param) — see Decisions Resolved D3; fires via onSearchImmediate.
4. Year range — fromYear / toYear (integers), inclusive on both ends. Two type="number" inputmode="numeric" inputs. Fire onSearch onblur (not oninput) — see Decisions Resolved D4.

type param mapping for the personal/historical toggles

• Both on (default) → omit type (server returns both).
• Only Personal → type=PERSONAL.
• Only Historical → type=HISTORICAL.
• Both off → call GET /api/timeline normally (type omitted); hide all returned event cards client-side — see Decisions Resolved D5. A code comment on the mapping function must explain this: "Both toggles off: API is still called normally; event cards are hidden client-side. Consistent with the D3 letters pattern. Known limitation: counts include hidden items."

Locked-person mode (Lebensweg / #10)

TimelineView.svelte accepts an optional personId prop. On the per-person Lebensweg page (#10) the person is fixed by the prop. TimelineFilters must support a lockedPersonId mode where the person field is hidden entirely (not disabled-but-present — a disabled field confuses screen-reader users). lockedPersonId is a plain (non-$bindable) typed prop — it comes from the parent as a fixed value and cannot be changed within the filter bar.

Data flow / architecture

• SSR-first, mandatory. Data flows +page.server.ts load → props. Never a client-side fetch('/api/timeline') in onMount (would expose the API route and bypass cookie-forwarded auth).
• +page.svelte builds the query string and calls goto(\?${params}`, { keepFocus: true, noScroll: true })`. Empty/absent params are omitted, never sent as empty strings.
• +page.server.ts reads url.searchParams, runs parseTimelineFilters, and calls api.GET('/api/timeline', { params: { query: {...} } }).
• Load function must not be marked cacheable with stale params (each filtered request hits GET /api/timeline fresh — fine for family-scale traffic, no CDN/cache config).
• Layer toggles (personal, historical, letters) fire immediately via onSearchImmediate callback pattern (mirror SearchFilterBar's AND/OR operator toggle). Year inputs fire on onblur.

Input coercion & validation (parseTimelineFilters)

File: src/routes/zeitstrahl/parseTimelineFilters.ts

Export the TimelineFilters type from this file for use in +page.svelte prop types:

export type TimelineFilters = {
  personId?: string;
  generation?: number;
  fromYear?: number;
  toYear?: number;
  typeFilter?: 'PERSONAL' | 'HISTORICAL';
};

Defense-in-depth in the load function, even though #5 also validates:

• personId: pass through as UUID string if present.
• generation: Number.parseInt; reject NaN; validate to 0..10 (mirror PersonGeneration); drop if out of range (do not clamp to nearest valid). If generation=99 is in the URL, parsed output has generation: undefined → UI shows "no generation filter active."
• fromYear / toYear: Number.parseInt; reject NaN; clamp to sane window (1000 .. new Date().getFullYear() + 1 — computed at call time, not a hardcoded constant); if from > to, swap (never forward an inverted range).
• Drop invalid params silently rather than forwarding garbage. Malformed generation=abc or fromYear=99999999999 must never cause a 500 or unbounded scan.
• Map any /api/timeline error via getErrorMessage(code) — never render the raw backend message.
• IDOR is a non-issue: any READ_ALL user may read any person's timeline by design (family-wide read). Do not add a per-person guard.

UX & Accessibility

• Mobile-first (phone-first read audience). Filters live in a slide-transitioned collapsible, closed by default ≤768px, with a sticky/persistent "Filter (N active)" trigger so the active-filter count is always visible. Respect prefers-reduced-motion: use duration: 0 on the slide transition when window.matchMedia('(prefers-reduced-motion: reduce)').matches — svelte/transition's slide does not honor this automatically.
• "Filter (N active)" trigger: a $derived count of active filters. A filter is active when it deviates from the default state: person set, generation set (including G0), year range set, or a layer toggle turned off. Default = both layers on, no person, no generation, no year range = 0 active. Touch target: min-h-[44px].
• Layer toggles: copy SearchFilterBar's undated-toggle markup — aria-pressed, min-h-[44px], a visible ✓ glyph inside, brand tokens (bg-primary text-primary-fg active / bg-muted text-ink-2 inactive). Each toggle gets a real aria-label (e.g. "Historische Ereignisse anzeigen"). Never signal active state with color alone (WCAG 1.4.1). Wrap all three in <fieldset><legend>Ebenen anzeigen</legend>...</fieldset>.
• Generation: labeled <select> (G0 oldest → G10) with a real <label for>, min-h-[44px] on the wrapper div (the <select> itself fills the wrapper).
• Year range: two type="number" inputmode="numeric" inputs with visible <label>s ("von Jahr" / "bis Jahr"), 16px+ text, side-by-side at 375px (min-w-0 flex-1 each). Inline icon+text validation if from > to (not color-only). Fires onSearch onblur.
• Person filter: PersonTypeahead; hidden entirely in locked-person mode (not disabled — not present in the DOM).
• Reset / clear-all: a clearly-labeled text button (not icon-only — seniors miss icon-only affordances). Returns to the unfiltered /zeitstrahl. A $derived "any filter active" flag drives its visibility (no $effect).
• Dual audience: phone-first readers AND seniors (60+) on the global view — touch targets and label sizes apply to both.

Behavior of special states

• Empty state (FR-TL-FILTER-003): when filters match zero entries, show "Keine Einträge entsprechen diesen Filtern" with a one-click reset — never a blank page.
• Undated ("Ohne Datum" / UNKNOWN-precision) bucket under a year range: hidden when a year range is active (D2). Reset/clear-all restores it. AC: when year range is cleared, the undated bucket is visible again.
• Both layer toggles off: GET /api/timeline is still called normally; all returned event cards are hidden client-side. A code comment on the hiding logic must read: "Both event toggles off: API called normally, cards hidden client-side. Consistent with D3 letters pattern." Only letters (if their toggle is on) remain visible.
• Letters toggle: client-side hide. A code comment on the letters toggle must read: "Letters toggle is client-side only. Known limitation: year/generation counts include hidden letter entries. A server-side includeLetters param on GET /api/timeline is deferred as a later refinement."
• Generation semantics (applied server-side in #5, passed through here): an entry matches a generation if any linked person is in that generation. This frontend issue only passes the integer through; the rule is defined and tested in #5.

Decisions Resolved

• D1 — "Family branch" is dropped; filter by generation only. No branch/Zweig field exists on Person (only generation Integer 0–10, CHECK in V70). "branch/generation" was ambiguous; introducing a branch dimension is a separate foundational issue (new column + Flyway migration + importer change + DB diagrams) and out of scope for a frontend-filters issue. Rationale: keeps the issue frontend-only and avoids a silent undefined data dimension.
• D2 — Undated bucket is hidden when a year range is active. Rationale: a year range is an explicit temporal constraint; an item with no date cannot be asserted to fall within it, so showing it would contradict the filter. Clearing the year range (or reset) brings the undated bucket back.
• D3 — Letters layer toggle is client-side hide (MVP). Load all entries; the toggle hides letter cards in the view. Rationale: keeps this issue frontend-only and avoids reopening the already-shipped #5 contract; letters are already constrained by personId and year range. Known limitation: year/generation counts include hidden letters. A server-side includeLetters param on GET /api/timeline is deferred as a later refinement.
• D4 — Year range inputs fire onSearch on onblur, not oninput. Rationale: typing "1930" fires four oninput events; onblur fires once. The senior-primary audience types slowly; debounce is invisible to slow typists. An explicit apply button would be clearest but adds UI weight not justified for two inputs. onblur is the right tradeoff.
• D5 — "Both layer toggles off" uses Option A: call API normally, hide event cards client-side. Rationale: consistent with the D3 letters precedent; keeps the SSR-first load function free of special-casing; simpler implementation. Known limitation: counts include hidden items — same as D3.

i18n Keys (required in messages/{de,en,es}.json)

The implementer must add these keys following the existing naming convention. German values shown as reference:

Key	de
timeline_filter_label_person	Person
timeline_filter_label_generation	Generation
timeline_filter_generation_option_all	Alle
timeline_filter_label_layers	Ebenen anzeigen
timeline_filter_layer_personal	Persönliche Ereignisse
timeline_filter_layer_historical	Historische Ereignisse
timeline_filter_layer_letters	Briefe
timeline_filter_label_from_year	von Jahr
timeline_filter_label_to_year	bis Jahr
timeline_filter_trigger	Filter
timeline_filter_trigger_active	Filter ({count} aktiv)
timeline_filter_reset	Filter zurücksetzen
timeline_filter_empty_state	Keine Einträge entsprechen diesen Filtern.
timeline_filter_year_range_error	„von Jahr" muss kleiner oder gleich „bis Jahr" sein.

Tasks

• src/routes/zeitstrahl/parseTimelineFilters.ts — export TimelineFilters type and parseTimelineFilters(searchParams) function. Route-scoped coercion; not in src/lib/.
• TimelineFilters.svelte — presentation component: $bindable filter props + onSearch/onSearchImmediate callbacks, no goto. Supports lockedPersonId plain prop (person field absent from DOM, not disabled).
• Person filter via PersonTypeahead (hidden entirely when lockedPersonId set); generation <select> (0–10, min-h-[44px] wrapper); three layer toggles inside <fieldset><legend>Ebenen anzeigen</legend> using the aria-pressed + ✓-glyph + min-h-[44px] pattern; year-range pair (type="number" inputmode="numeric" inputs with visible labels, onblur trigger).
• slide transition with prefers-reduced-motion guard (duration: 0 when motion is reduced).
• "Filter (N active)" trigger: $derived active-filter count; G0 counts as active; default state (both layers on, no person, no generation, no year range) = 0 active.
• /zeitstrahl/+page.svelte (extend from #7 stub): owns URL state, builds query string, calls goto(?…, { keepFocus, noScroll }), omits absent params.
• /zeitstrahl/+page.server.ts (extend from #7 stub): reads url.searchParams, validates via parseTimelineFilters, forwards to api.GET('/api/timeline'); maps errors via getErrorMessage.
• Client-side letters hide wired to the letters toggle, with code comment on D3 limitation.
• "Both toggles off" code comment on the event-card hiding logic (D5).
• Year-range from > to swap/validation (inline icon+text, not color-only); clamp years to 1000..new Date().getFullYear() + 1.
• Empty-state message ("Keine Einträge entsprechen diesen Filtern.") + one-click reset; $derived "any filter active" flag (no $effect).
• Mobile collapsible (closed ≤768px) with sticky "Filter (N active)" trigger.
• Reset/clear-all as text button (not icon-only); returns to unfiltered /zeitstrahl.
• i18n keys added to messages/{de,en,es}.json (see table above).

Acceptance criteria

• Each filter narrows the timeline; combinations work; state survives reload via URL.
• Generation: Given a generation filter (including G0), when applied, then only entries whose linked persons are in that generation appear (rule defined/tested in #5; this issue passes the integer through). Given generation=99 in the URL, parsed output has no generation filter and the UI shows "no generation filter active."
• Year range (inclusive): Given fromYear=1920&toYear=1930, when applied, then year bands outside 1920–1930 are hidden and the undated bucket is hidden (D2). Given the year range is then cleared, the undated bucket is visible again.
• Layer toggles: personal/historical drive type; the letters toggle hides letter cards client-side; toggling reflects aria-pressed and does not rely on color alone. Given both event toggles are off, the timeline shows no event cards and the letters layer (if toggle is on) is shown; the API is still called normally.
• Locked-person mode: the person field is not present in the DOM (not merely hidden via CSS) and not user-editable.
• Reset: clears all filters to default and returns to the unfiltered /zeitstrahl.
• Empty state: filters matching zero entries render "Keine Einträge entsprechen diesen Filtern." + reset button, never a blank page.
• Reload: a filtered URL reloaded re-activates the same filters and renders the same entries.
• Validation/robustness: malformed generation=abc, out-of-range generation=99, or fromYear=notanumber are dropped/coerced cleanly — never a 500 or crash.
• SSR-first: data is fetched in +page.server.ts, never via client-side fetch in onMount.
• a11y: axe-core passes on /zeitstrahl with filters open, in light and dark mode, at 375px (checked via existing accessibility.spec.ts, not a standalone spec).

Tests

• Unit (Vitest, <10s) — parseTimelineFilters:
  • Round-trip both directions (searchParams → typed object → searchParams).
  • Rejects NaN/out-of-range generation and years.
  • generation=99 → generation: undefined in output (drop, not clamp).
  • Swaps inverted year range.
  • Omits absent params.
  • { personalOn: false, historicalOn: false } → typeFilter is undefined (type omitted) — load function still calls API; event hiding is client-side. Test must assert the exact value passed to api.GET.
  • Both toggles off with letters on → only letters visible (component-level behavior; document in comment).
• Component (TimelineFilters.svelte.spec.ts, browser/client project, single-file local runs):
  • Each control renders with an accessible name.
  • Layer toggles fire the correct callback and reflect aria-pressed.
  • Year inputs fire onSearch on blur, not on input.
  • lockedPersonId mode → queryByRole('combobox', { name: /Person/ }) returns null (not in DOM).
  • Reset clears all to default.
  • Empty-state message + reset button rendered when timeline data is empty with active filters.
  • Use factory helpers for filter state.
• Load-function (mock createApiClient at module boundary — follow documents/page.server.spec.ts pattern):
  • "Combinations work" matrix: {generation}, {fromYear+toYear}, {type}, {personId}, plus 2–3 combinations → assert exact query object passed to api.GET('/api/timeline', …).
  • Invalid params dropped: generation=99, fromYear=abc, inverted year range swapped.
  • generation=0 → passed as 0 (G0 is valid, not dropped).
• E2E (Playwright — one journey, add to existing accessibility.spec.ts for axe; one journey file for filter behavior):
  • Open /zeitstrahl, apply person + year-range, reload, assert URL params persist and the filtered set renders.
  • axe check in light + dark mode at 375px (added to accessibility.spec.ts, reusing existing dark-mode toggle infrastructure — not a standalone file).
  • test.skip guard on E2E spec until #7 is merged (the /zeitstrahl route does not exist until #7 lands; CI branch ordering enforces the dependency).
• Backend generation-filter semantics are tested in #5 — do not duplicate. Targeted single-file runs locally; full sweep to CI.

**Milestone:** Zeitstrahl — Family Timeline **Spec:** `docs/superpowers/specs/2026-06-07-family-timeline-design.md` § "Concept & UX" **Depends on:** #5 (assembly endpoint `GET /api/timeline` + its query params), #7 (global `/zeitstrahl` view + `TimelineView.svelte`). ## Overview A presentation-only filter bar, `TimelineFilters.svelte`, that drives the `GET /api/timeline` query params for the global `/zeitstrahl` view. **This issue is frontend-only** — it stays strictly within the query-param contract already owned by #5 and adds zero backend code, no new endpoint, no migration, no infra, no env var. The canonical pattern to mirror is `frontend/src/routes/SearchFilterBar.svelte`. ## Scope - `TimelineFilters.svelte` — a dumb/presentation component: `$bindable` filter props + `onSearch`/`onSearchImmediate` callbacks, **no `goto` inside it**. - URL/query state lives in the **route**, not the component: `/zeitstrahl/+page.svelte` builds the query string and calls `goto`; `/zeitstrahl/+page.server.ts` reads `url.searchParams`, coerces/validates, and forwards to `GET /api/timeline`. - A small typed helper `parseTimelineFilters(searchParams)` at `src/routes/zeitstrahl/parseTimelineFilters.ts` — route-scoped coercion, not a reusable library utility. **File boundary with #7:** #7 scaffolds the `/zeitstrahl` route (creates `+page.svelte`, `+page.server.ts` stubs). This issue **extends** those files with filter wiring; it does not create them from scratch. `CLAUDE.md` route table and `docs/architecture/c4/l3-frontend-*.puml` updates are scoped to #7, not this issue. Out of scope: any backend/contract change, any new `Person` field, generation-matching semantics (owned by `TimelineService` in #5). ## Filter dimensions (final) 1. **Person** — `personId` (UUID string). Reuse `PersonTypeahead` (project standard, as in `SearchFilterBar`). 2. **Generation** — `generation` (integer 0–10, G0 = oldest, bounds per `PersonGeneration`). Labeled `<select>` dropdown (bounded set; a dropdown beats free text). **No "family branch"** — see Decisions Resolved D1. 3. **Layer toggles** — three independent toggles inside a `<fieldset><legend>Ebenen anzeigen</legend>...</fieldset>` for screen-reader group context: - Personal events → server-side `type=PERSONAL`; fires via `onSearchImmediate` (instant, no blur needed). - Historical events → server-side `type=HISTORICAL`; fires via `onSearchImmediate`. - Letters → **client-side hide** (no backing param) — see Decisions Resolved D3; fires via `onSearchImmediate`. 4. **Year range** — `fromYear` / `toYear` (integers), **inclusive** on both ends. Two `type="number"` `inputmode="numeric"` inputs. Fire `onSearch` **`onblur`** (not `oninput`) — see Decisions Resolved D4. ### `type` param mapping for the personal/historical toggles - Both on (default) → omit `type` (server returns both). - Only Personal → `type=PERSONAL`. - Only Historical → `type=HISTORICAL`. - **Both off → call `GET /api/timeline` normally (type omitted); hide all returned event cards client-side** — see Decisions Resolved D5. A code comment on the mapping function must explain this: "Both toggles off: API is still called normally; event cards are hidden client-side. Consistent with the D3 letters pattern. Known limitation: counts include hidden items." ## Locked-person mode (Lebensweg / #10) `TimelineView.svelte` accepts an optional `personId` prop. On the per-person Lebensweg page (#10) the person is **fixed by the prop**. `TimelineFilters` must support a `lockedPersonId` mode where the **person field is hidden entirely** (not disabled-but-present — a disabled field confuses screen-reader users). `lockedPersonId` is a plain (non-`$bindable`) typed prop — it comes from the parent as a fixed value and cannot be changed within the filter bar. ## Data flow / architecture - **SSR-first, mandatory.** Data flows `+page.server.ts` load → props. **Never** a client-side `fetch('/api/timeline')` in `onMount` (would expose the API route and bypass cookie-forwarded auth). - `+page.svelte` builds the query string and calls `goto(\`?${params}\`, { keepFocus: true, noScroll: true })`. Empty/absent params are **omitted**, never sent as empty strings. - `+page.server.ts` reads `url.searchParams`, runs `parseTimelineFilters`, and calls `api.GET('/api/timeline', { params: { query: {...} } })`. - Load function must not be marked cacheable with stale params (each filtered request hits `GET /api/timeline` fresh — fine for family-scale traffic, no CDN/cache config). - Layer toggles (personal, historical, letters) fire immediately via `onSearchImmediate` callback pattern (mirror `SearchFilterBar`'s AND/OR operator toggle). Year inputs fire on `onblur`. ## Input coercion & validation (`parseTimelineFilters`) **File:** `src/routes/zeitstrahl/parseTimelineFilters.ts` Export the `TimelineFilters` type from this file for use in `+page.svelte` prop types: ```ts export type TimelineFilters = { personId?: string; generation?: number; fromYear?: number; toYear?: number; typeFilter?: 'PERSONAL' | 'HISTORICAL'; }; ``` Defense-in-depth in the load function, even though #5 also validates: - `personId`: pass through as UUID string if present. - `generation`: `Number.parseInt`; reject `NaN`; validate to `0..10` (mirror `PersonGeneration`); **drop if out of range** (do not clamp to nearest valid). If `generation=99` is in the URL, parsed output has `generation: undefined` → UI shows "no generation filter active." - `fromYear` / `toYear`: `Number.parseInt`; reject `NaN`; clamp to sane window (`1000 .. new Date().getFullYear() + 1` — computed at call time, not a hardcoded constant); if `from > to`, swap (never forward an inverted range). - Drop invalid params **silently** rather than forwarding garbage. Malformed `generation=abc` or `fromYear=99999999999` must never cause a 500 or unbounded scan. - Map any `/api/timeline` error via `getErrorMessage(code)` — never render the raw backend message. - **IDOR is a non-issue**: any `READ_ALL` user may read any person's timeline by design (family-wide read). Do **not** add a per-person guard. ## UX & Accessibility - **Mobile-first (phone-first read audience).** Filters live in a `slide`-transitioned collapsible, **closed by default ≤768px**, with a sticky/persistent "Filter (N active)" trigger so the active-filter count is always visible. **Respect `prefers-reduced-motion`:** use `duration: 0` on the `slide` transition when `window.matchMedia('(prefers-reduced-motion: reduce)').matches` — `svelte/transition`'s `slide` does not honor this automatically. - **"Filter (N active)" trigger:** a `$derived` count of active filters. A filter is **active** when it deviates from the default state: person set, generation set (including G0), year range set, or a layer toggle turned off. Default = both layers on, no person, no generation, no year range = 0 active. Touch target: `min-h-[44px]`. - **Layer toggles:** copy `SearchFilterBar`'s undated-toggle markup — `aria-pressed`, `min-h-[44px]`, a visible ✓ glyph inside, brand tokens (`bg-primary text-primary-fg` active / `bg-muted text-ink-2` inactive). Each toggle gets a real `aria-label` (e.g. "Historische Ereignisse anzeigen"). **Never signal active state with color alone** (WCAG 1.4.1). Wrap all three in `<fieldset><legend>Ebenen anzeigen</legend>...</fieldset>`. - **Generation:** labeled `<select>` (G0 oldest → G10) with a real `<label for>`, `min-h-[44px]` on the wrapper div (the `<select>` itself fills the wrapper). - **Year range:** two `type="number"` `inputmode="numeric"` inputs with **visible `<label>`s** ("von Jahr" / "bis Jahr"), 16px+ text, side-by-side at 375px (`min-w-0 flex-1` each). Inline icon+text validation if `from > to` (not color-only). Fires `onSearch` `onblur`. - **Person filter:** `PersonTypeahead`; **hidden entirely** in locked-person mode (not disabled — not present in the DOM). - **Reset / clear-all:** a clearly-labeled **text** button (not icon-only — seniors miss icon-only affordances). Returns to the unfiltered `/zeitstrahl`. A `$derived` "any filter active" flag drives its visibility (no `$effect`). - Dual audience: phone-first readers AND seniors (60+) on the global view — touch targets and label sizes apply to both. ## Behavior of special states - **Empty state (FR-TL-FILTER-003):** when filters match zero entries, show "Keine Einträge entsprechen diesen Filtern" with a one-click reset — never a blank page. - **Undated ("Ohne Datum" / UNKNOWN-precision) bucket under a year range:** **hidden** when a year range is active (D2). Reset/clear-all restores it. AC: when year range is cleared, the undated bucket is visible again. - **Both layer toggles off:** `GET /api/timeline` is still called normally; all returned event cards are hidden client-side. A code comment on the hiding logic must read: "Both event toggles off: API called normally, cards hidden client-side. Consistent with D3 letters pattern." Only letters (if their toggle is on) remain visible. - **Letters toggle:** client-side hide. A code comment on the letters toggle must read: "Letters toggle is client-side only. Known limitation: year/generation counts include hidden letter entries. A server-side `includeLetters` param on GET /api/timeline is deferred as a later refinement." - **Generation semantics** (applied server-side in #5, passed through here): an entry matches a generation if **any linked person** is in that generation. This frontend issue only passes the integer through; the rule is defined and tested in #5. ## Decisions Resolved - **D1 — "Family branch" is dropped; filter by generation only.** No `branch`/`Zweig` field exists on `Person` (only `generation` Integer 0–10, CHECK in V70). "branch/generation" was ambiguous; introducing a branch dimension is a separate foundational issue (new column + Flyway migration + importer change + DB diagrams) and out of scope for a frontend-filters issue. _Rationale: keeps the issue frontend-only and avoids a silent undefined data dimension._ - **D2 — Undated bucket is hidden when a year range is active.** _Rationale: a year range is an explicit temporal constraint; an item with no date cannot be asserted to fall within it, so showing it would contradict the filter. Clearing the year range (or reset) brings the undated bucket back._ - **D3 — Letters layer toggle is client-side hide (MVP).** Load all entries; the toggle hides letter cards in the view. _Rationale: keeps this issue frontend-only and avoids reopening the already-shipped #5 contract; letters are already constrained by `personId` and year range. Known limitation: year/generation counts include hidden letters. A server-side `includeLetters` param on `GET /api/timeline` is deferred as a later refinement._ - **D4 — Year range inputs fire `onSearch` on `onblur`, not `oninput`.** _Rationale: typing "1930" fires four `oninput` events; `onblur` fires once. The senior-primary audience types slowly; debounce is invisible to slow typists. An explicit apply button would be clearest but adds UI weight not justified for two inputs. `onblur` is the right tradeoff._ - **D5 — "Both layer toggles off" uses Option A: call API normally, hide event cards client-side.** _Rationale: consistent with the D3 letters precedent; keeps the SSR-first load function free of special-casing; simpler implementation. Known limitation: counts include hidden items — same as D3._ ## i18n Keys (required in `messages/{de,en,es}.json`) The implementer must add these keys following the existing naming convention. German values shown as reference: | Key | de | |---|---| | `timeline_filter_label_person` | Person | | `timeline_filter_label_generation` | Generation | | `timeline_filter_generation_option_all` | Alle | | `timeline_filter_label_layers` | Ebenen anzeigen | | `timeline_filter_layer_personal` | Persönliche Ereignisse | | `timeline_filter_layer_historical` | Historische Ereignisse | | `timeline_filter_layer_letters` | Briefe | | `timeline_filter_label_from_year` | von Jahr | | `timeline_filter_label_to_year` | bis Jahr | | `timeline_filter_trigger` | Filter | | `timeline_filter_trigger_active` | Filter ({count} aktiv) | | `timeline_filter_reset` | Filter zurücksetzen | | `timeline_filter_empty_state` | Keine Einträge entsprechen diesen Filtern. | | `timeline_filter_year_range_error` | „von Jahr" muss kleiner oder gleich „bis Jahr" sein. | ## Tasks - [ ] `src/routes/zeitstrahl/parseTimelineFilters.ts` — export `TimelineFilters` type and `parseTimelineFilters(searchParams)` function. Route-scoped coercion; not in `src/lib/`. - [ ] `TimelineFilters.svelte` — presentation component: `$bindable` filter props + `onSearch`/`onSearchImmediate` callbacks, no `goto`. Supports `lockedPersonId` plain prop (person field **absent from DOM**, not disabled). - [ ] Person filter via `PersonTypeahead` (hidden entirely when `lockedPersonId` set); generation `<select>` (0–10, `min-h-[44px]` wrapper); three layer toggles inside `<fieldset><legend>Ebenen anzeigen</legend>` using the `aria-pressed` + ✓-glyph + `min-h-[44px]` pattern; year-range pair (`type="number"` `inputmode="numeric"` inputs with visible labels, `onblur` trigger). - [ ] `slide` transition with `prefers-reduced-motion` guard (`duration: 0` when motion is reduced). - [ ] "Filter (N active)" trigger: `$derived` active-filter count; G0 counts as active; default state (both layers on, no person, no generation, no year range) = 0 active. - [ ] `/zeitstrahl/+page.svelte` (extend from #7 stub): owns URL state, builds query string, calls `goto(?…, { keepFocus, noScroll })`, omits absent params. - [ ] `/zeitstrahl/+page.server.ts` (extend from #7 stub): reads `url.searchParams`, validates via `parseTimelineFilters`, forwards to `api.GET('/api/timeline')`; maps errors via `getErrorMessage`. - [ ] Client-side letters hide wired to the letters toggle, with code comment on D3 limitation. - [ ] "Both toggles off" code comment on the event-card hiding logic (D5). - [ ] Year-range `from > to` swap/validation (inline icon+text, not color-only); clamp years to `1000..new Date().getFullYear() + 1`. - [ ] Empty-state message ("Keine Einträge entsprechen diesen Filtern.") + one-click reset; `$derived` "any filter active" flag (no `$effect`). - [ ] Mobile collapsible (closed ≤768px) with sticky "Filter (N active)" trigger. - [ ] Reset/clear-all as **text button** (not icon-only); returns to unfiltered `/zeitstrahl`. - [ ] i18n keys added to `messages/{de,en,es}.json` (see table above). ## Acceptance criteria - Each filter narrows the timeline; combinations work; state survives reload via URL. - **Generation:** Given a generation filter (including G0), when applied, then only entries whose linked persons are in that generation appear (rule defined/tested in #5; this issue passes the integer through). Given `generation=99` in the URL, parsed output has no generation filter and the UI shows "no generation filter active." - **Year range (inclusive):** Given `fromYear=1920&toYear=1930`, when applied, then year bands outside 1920–1930 are hidden **and** the undated bucket is hidden (D2). Given the year range is then cleared, the undated bucket is visible again. - **Layer toggles:** personal/historical drive `type`; the letters toggle hides letter cards client-side; toggling reflects `aria-pressed` and does not rely on color alone. Given both event toggles are off, the timeline shows no event cards and the letters layer (if toggle is on) is shown; the API is still called normally. - **Locked-person mode:** the person field is **not present in the DOM** (not merely hidden via CSS) and not user-editable. - **Reset:** clears all filters to default and returns to the unfiltered `/zeitstrahl`. - **Empty state:** filters matching zero entries render "Keine Einträge entsprechen diesen Filtern." + reset button, never a blank page. - **Reload:** a filtered URL reloaded re-activates the same filters and renders the same entries. - **Validation/robustness:** malformed `generation=abc`, out-of-range `generation=99`, or `fromYear=notanumber` are dropped/coerced cleanly — never a 500 or crash. - **SSR-first:** data is fetched in `+page.server.ts`, never via client-side `fetch` in `onMount`. - **a11y:** axe-core passes on `/zeitstrahl` with filters open, in **light and dark mode**, at 375px (checked via existing `accessibility.spec.ts`, not a standalone spec). ## Tests - **Unit (Vitest, <10s) — `parseTimelineFilters`:** - Round-trip both directions (searchParams → typed object → searchParams). - Rejects `NaN`/out-of-range generation and years. - `generation=99` → `generation: undefined` in output (drop, not clamp). - Swaps inverted year range. - Omits absent params. - `{ personalOn: false, historicalOn: false }` → `typeFilter` is `undefined` (type omitted) — load function still calls API; event hiding is client-side. Test must assert the exact value passed to `api.GET`. - Both toggles off with letters on → only letters visible (component-level behavior; document in comment). - **Component (`TimelineFilters.svelte.spec.ts`, browser/client project, single-file local runs):** - Each control renders with an accessible name. - Layer toggles fire the correct callback and reflect `aria-pressed`. - Year inputs fire `onSearch` on `blur`, not on `input`. - `lockedPersonId` mode → `queryByRole('combobox', { name: /Person/ })` returns `null` (not in DOM). - Reset clears all to default. - Empty-state message + reset button rendered when timeline data is empty with active filters. - Use factory helpers for filter state. - **Load-function (mock `createApiClient` at module boundary — follow `documents/page.server.spec.ts` pattern):** - "Combinations work" matrix: `{generation}`, `{fromYear+toYear}`, `{type}`, `{personId}`, plus 2–3 combinations → assert exact query object passed to `api.GET('/api/timeline', …)`. - Invalid params dropped: `generation=99`, `fromYear=abc`, inverted year range swapped. - `generation=0` → passed as `0` (G0 is valid, not dropped). - **E2E (Playwright — one journey, add to existing `accessibility.spec.ts` for axe; one journey file for filter behavior):** - Open `/zeitstrahl`, apply person + year-range, reload, assert URL params persist and the filtered set renders. - axe check in light + dark mode at 375px (added to `accessibility.spec.ts`, reusing existing dark-mode toggle infrastructure — not a standalone file). - **`test.skip` guard on E2E spec until #7 is merged** (the `/zeitstrahl` route does not exist until #7 lands; CI branch ordering enforces the dependency). - Backend generation-filter semantics are tested in #5 — do not duplicate. Targeted single-file runs locally; full sweep to CI.

marcel added this to the Zeitstrahl — Family Timeline milestone 2026-06-07 19:29:27 +02:00

marcel added the P3-laterfeatureui labels 2026-06-07 19:30:04 +02:00

marcel commented 2026-06-07 19:52:47 +02:00

Author

Owner

Copy Link

🏗️ Markus Keller — Senior Application Architect

Observations

File placement is correct. parseTimelineFilters.ts at route scope (src/routes/zeitstrahl/) — not src/lib/ — is exactly the right call. Route-scoped coercion must not become shared infrastructure.

SSR-first constraint is explicit and enforced by design. No client-side fetch in onMount, load function owns the data flow. This matches the project's established pattern (seen in /documents/+page.svelte and /persons/+page.svelte).

Architecture doc update scope is correctly excluded. The issue notes that CLAUDE.md route table + docs/architecture/c4/l3-frontend-*.puml updates are scoped to #7 (the route scaffold), not this issue. That boundary is clean.

Letters client-side filter is an acknowledged deferral, not an architectural mistake. D3 is well-reasoned: the filter bar stays frontend-only, #5's contract is not reopened. The required code comment on the letters toggle is the right compensating control.

lockedPersonId prop design is architecturally sound. A non-$bindable typed prop (fixed at the parent level, not mutable inside the component) is the correct primitive for a locked read-only value. DOM removal (not present) vs. disabled is the right choice — see below.

Recommendations

• The parseTimelineFilters function has one implicit architectural risk: the issue specifies that fromYear > toYear is swapped silently. This is correct for robustness, but the function must return the swapped values back to the page so that the URL can be corrected via a replace-state goto. If the load function swaps internally but the URL still reflects the inverted order, a page reload will re-swap. The issue says "if from > to, swap (never forward an inverted range)" — the implementer must also update the URL to reflect the canonical form, or the reload behavior is inconsistent with AC "state survives reload via URL."

• TimelineFilters is a correct scope boundary. No business logic, no goto. The pattern mirrors SearchFilterBar.svelte, which correctly puts all URL management in +page.svelte. Enforce this at review time — any goto or url.searchParams access inside TimelineFilters.svelte is a boundary violation.

• C4 diagram update note: Since this issue extends the /zeitstrahl route (not creates it), no new diagram entries are needed. However, verify the PR doesn't add new files to src/lib/ that would require l3-frontend-*.puml updates. TimelineFilters.svelte must live in src/routes/zeitstrahl/, not src/lib/.

Open Decisions

• URL correction on swap: When fromYear > toYear is swapped in parseTimelineFilters, should the load function also trigger a redirect to the corrected URL (so the URL bar reflects the canonical from ≤ to order), or is silent swap in the query call sufficient? Silent swap is simpler and the issue implies it. But a reload with the original swapped URL re-runs the swap, so the behavior is technically consistent — just confusing if a user shares a link with fromYear=1950&toYear=1920. This is a product judgment, not a technical one.

## 🏗️ Markus Keller — Senior Application Architect ### Observations **File placement is correct.** `parseTimelineFilters.ts` at route scope (`src/routes/zeitstrahl/`) — not `src/lib/` — is exactly the right call. Route-scoped coercion must not become shared infrastructure. **SSR-first constraint is explicit and enforced by design.** No client-side fetch in `onMount`, load function owns the data flow. This matches the project's established pattern (seen in `/documents/+page.svelte` and `/persons/+page.svelte`). **Architecture doc update scope is correctly excluded.** The issue notes that `CLAUDE.md` route table + `docs/architecture/c4/l3-frontend-*.puml` updates are scoped to #7 (the route scaffold), not this issue. That boundary is clean. **Letters client-side filter is an acknowledged deferral, not an architectural mistake.** D3 is well-reasoned: the filter bar stays frontend-only, #5's contract is not reopened. The required code comment on the letters toggle is the right compensating control. **`lockedPersonId` prop design is architecturally sound.** A non-`$bindable` typed prop (fixed at the parent level, not mutable inside the component) is the correct primitive for a locked read-only value. DOM removal (`not present`) vs. `disabled` is the right choice — see below. ### Recommendations - **The `parseTimelineFilters` function has one implicit architectural risk:** the issue specifies that `fromYear > toYear` is swapped *silently*. This is correct for robustness, but the function must **return** the swapped values back to the page so that the URL can be corrected via a replace-state `goto`. If the load function swaps internally but the URL still reflects the inverted order, a page reload will re-swap. The issue says "if `from > to`, swap (never forward an inverted range)" — the implementer must also update the URL to reflect the canonical form, or the reload behavior is inconsistent with AC "state survives reload via URL." - **`TimelineFilters` is a correct scope boundary.** No business logic, no `goto`. The pattern mirrors `SearchFilterBar.svelte`, which correctly puts all URL management in `+page.svelte`. Enforce this at review time — any `goto` or `url.searchParams` access inside `TimelineFilters.svelte` is a boundary violation. - **C4 diagram update note:** Since this issue extends the `/zeitstrahl` route (not creates it), no new diagram entries are needed. However, verify the PR doesn't add new files to `src/lib/` that would require `l3-frontend-*.puml` updates. `TimelineFilters.svelte` must live in `src/routes/zeitstrahl/`, not `src/lib/`. ### Open Decisions - **URL correction on swap:** When `fromYear > toYear` is swapped in `parseTimelineFilters`, should the load function also trigger a `redirect` to the corrected URL (so the URL bar reflects the canonical `from ≤ to` order), or is silent swap in the query call sufficient? Silent swap is simpler and the issue implies it. But a reload with the original swapped URL re-runs the swap, so the behavior is technically consistent — just confusing if a user shares a link with `fromYear=1950&toYear=1920`. This is a product judgment, not a technical one.

marcel commented 2026-06-07 19:53:12 +02:00

Author

Owner

Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

The issue is well-specified and aligns tightly with existing patterns. I reviewed SearchFilterBar.svelte (the canonical reference) and documents/page.server.spec.ts (the test pattern to mirror). Several implementation details warrant attention:

The slide transition in SearchFilterBar.svelte uses bare transition:slide with no prefers-reduced-motion guard. This issue correctly calls out the gap — duration: 0 when motion is reduced. The existing filter bar has this unresolved; TimelineFilters must not repeat the pattern. Implementation: check window.matchMedia('(prefers-reduced-motion: reduce)').matches before assigning transition params. I saw this guard used in +page.svelte for StammbaumTree.svelte already — same approach.

The $derived active-filter count needs a precise definition. The issue says: "A filter is active when it deviates from the default state: person set, generation set (including G0), year range set, or a layer toggle turned off." G0 counting as active is a subtle but important correctness requirement — generation === 0 must be treated identically to generation === 5. A naïve if (generation) check would wrongly treat G0 as inactive (falsy zero).

The $effect prohibition is explicit: active-filter count must be $derived, not $state + $effect. The existing SearchFilterBar.svelte has one $effect for sort-direction tracking (lines 74–84 there), but it's a known workaround, not a pattern to copy. The timeline filter count is a straightforward $derived — no workaround needed.

lockedPersonId DOM removal test: queryByRole('combobox', { name: /Person/ }) === null is a stronger assertion than not.toBeVisible(). The issue is explicit about this — DOM removal, not CSS hide. Make sure {#if !lockedPersonId} wraps the entire PersonTypeahead block, not just a visibility class.

$props() runes pattern — the TimelineFilters component should declare props using the Svelte 5 $props() rune with explicit TypeScript types (matching the pattern in SearchFilterBar.svelte, lines 11–60). The $bindable decorator is correct for mutable filter state; lockedPersonId is a plain typed prop (no $bindable).

Missing test case: The issue's test matrix mentions { personalOn: false, historicalOn: false } → typeFilter is undefined. The test must assert the exact value passed to api.GET, not just the component behavior. Mirror the documents/page.server.spec.ts pattern: mock createApiClient, call load() with the URL params, assert mockGet was called with the exact query object.

{#each} key discipline: Any loop over timeline entries in the parent TimelineView.svelte must use keyed iteration {#each entries as entry (entry.id)}. This issue doesn't touch that component directly, but if the filter state causes list updates, unkeyed loops will corrupt DOM state.

Recommendations

• Use const activeFilterCount = $derived(...) with explicit conditions for each dimension. Extract a helper function isDefaultState(filters: TimelineFilters): boolean — testable in isolation, readable at the call site.
• The onSearch / onSearchImmediate callback pattern is already proven in SearchFilterBar. Copy the exact naming and invocation pattern (see lines 196–200 of SearchFilterBar.svelte for the AND/OR toggle as the reference for layer toggles firing onSearchImmediate).
• For the parseTimelineFilters unit tests: use the makeUrl() factory pattern from documents/page.server.spec.ts (lines 13–25) — identical helper needed here.
• The year-range inline validation message ("von Jahr muss ≤ bis Jahr sein") must use an icon + text, not red border alone. Reference the <span class="text-red-600 flex items-center gap-1"><svg>...</svg>...</span> pattern from Leonie's guidance.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations The issue is well-specified and aligns tightly with existing patterns. I reviewed `SearchFilterBar.svelte` (the canonical reference) and `documents/page.server.spec.ts` (the test pattern to mirror). Several implementation details warrant attention: **The `slide` transition in `SearchFilterBar.svelte` uses bare `transition:slide` with no `prefers-reduced-motion` guard.** This issue correctly calls out the gap — `duration: 0` when motion is reduced. The existing filter bar has this unresolved; `TimelineFilters` must not repeat the pattern. Implementation: check `window.matchMedia('(prefers-reduced-motion: reduce)').matches` before assigning transition params. I saw this guard used in `+page.svelte` for `StammbaumTree.svelte` already — same approach. **The `$derived` active-filter count needs a precise definition.** The issue says: "A filter is active when it deviates from the default state: person set, generation set (including G0), year range set, or a layer toggle turned off." G0 counting as active is a subtle but important correctness requirement — `generation === 0` must be treated identically to `generation === 5`. A naïve `if (generation)` check would wrongly treat G0 as inactive (falsy zero). **The `$effect` prohibition is explicit:** active-filter count must be `$derived`, not `$state` + `$effect`. The existing `SearchFilterBar.svelte` has one `$effect` for sort-direction tracking (lines 74–84 there), but it's a known workaround, not a pattern to copy. The timeline filter count is a straightforward `$derived` — no workaround needed. **`lockedPersonId` DOM removal test:** `queryByRole('combobox', { name: /Person/ }) === null` is a stronger assertion than `not.toBeVisible()`. The issue is explicit about this — DOM removal, not CSS hide. Make sure `{#if !lockedPersonId}` wraps the entire `PersonTypeahead` block, not just a visibility class. **`$props()` runes pattern** — the `TimelineFilters` component should declare props using the Svelte 5 `$props()` rune with explicit TypeScript types (matching the pattern in `SearchFilterBar.svelte`, lines 11–60). The `$bindable` decorator is correct for mutable filter state; `lockedPersonId` is a plain typed prop (no `$bindable`). **Missing test case:** The issue's test matrix mentions `{ personalOn: false, historicalOn: false }` → `typeFilter` is `undefined`. The test must assert the *exact value passed to `api.GET`*, not just the component behavior. Mirror the `documents/page.server.spec.ts` pattern: mock `createApiClient`, call `load()` with the URL params, assert `mockGet` was called with the exact query object. **`{#each}` key discipline:** Any loop over timeline entries in the parent `TimelineView.svelte` must use keyed iteration `{#each entries as entry (entry.id)}`. This issue doesn't touch that component directly, but if the filter state causes list updates, unkeyed loops will corrupt DOM state. ### Recommendations - Use `const activeFilterCount = $derived(...)` with explicit conditions for each dimension. Extract a helper function `isDefaultState(filters: TimelineFilters): boolean` — testable in isolation, readable at the call site. - The `onSearch` / `onSearchImmediate` callback pattern is already proven in `SearchFilterBar`. Copy the exact naming and invocation pattern (see lines 196–200 of `SearchFilterBar.svelte` for the AND/OR toggle as the reference for layer toggles firing `onSearchImmediate`). - For the `parseTimelineFilters` unit tests: use the `makeUrl()` factory pattern from `documents/page.server.spec.ts` (lines 13–25) — identical helper needed here. - The year-range inline validation message ("`von Jahr` muss ≤ `bis Jahr` sein") must use an icon + text, not red border alone. Reference the `<span class="text-red-600 flex items-center gap-1"><svg>...</svg>...</span>` pattern from Leonie's guidance.

marcel commented 2026-06-07 19:53:30 +02:00

Author

Owner

Copy Link

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

This is a read-only filter bar with no write operations and no data mutation. The attack surface is narrow: URL parameter injection and client-side DOM manipulation. The issue is unusually thorough on input validation — the IDOR note, the defense-in-depth framing for parseTimelineFilters, and the "drop invalid params silently" policy are all correctly specified.

IDOR non-issue is correctly assessed. The issue explicitly states: "IDOR is a non-issue: any READ_ALL user may read any person's timeline by design." This is consistent with the existing family-archive threat model (no per-user data segregation). No per-person guard needed.

Malformed URL param injection is addressed by parseTimelineFilters design. Drop invalid generation, NaN years, out-of-range values — these are the right controls. The key security property is that no raw user input from the URL reaches api.GET unvalidated. The parseTimelineFilters function is the sanitization boundary.

One gap: personId validation. The issue says "pass through as UUID string if present" but specifies no UUID format validation. A malformed personId=../../../../etc/passwd passed to api.GET('/api/timeline', { params: { query: { personId: ... } } }) is not a path traversal risk (it's a query param, not a path param), but the backend will receive garbage. Recommendation: validate personId against a UUID regex before passing through. The documents/page.server.spec.ts pattern does UUID validation before person name lookup (line 382: "returns empty string when senderId is not a valid UUID") — mirror that guard.

SSR-first protects auth. The issue correctly states: "Never a client-side fetch('/api/timeline') in onMount (would expose the API route and bypass cookie-forwarded auth)." This is the critical security property. The auth cookie is forwarded by the SvelteKit handleFetch hook — client-side fetch bypasses it entirely. No additional control needed beyond enforcing the SSR-first constraint, which is already in the AC.

Year clamping boundary. new Date().getFullYear() + 1 is computed at call time (correct — not a hardcoded constant). A hardcoded year like 2026 would silently stop accepting valid years after that. The call-time computation is the right approach.

Error mapping. The issue requires getErrorMessage(code) — no raw backend messages to users. This is correct and consistent with the codebase pattern.

Recommendations

• Add UUID format validation for personId in parseTimelineFilters. A simple UUID regex check (/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i) before passing through to the API. If the format is invalid, drop it (personId: undefined). Add a unit test: personId=not-a-uuid → parsed output has personId: undefined.
• Test that invalid params never reach api.GET. The load-function tests should assert that when personId=garbage is in the URL, api.GET is called with personId absent from the query object — not with the garbage string.

No open security decisions — the analysis is complete.

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations This is a read-only filter bar with no write operations and no data mutation. The attack surface is narrow: URL parameter injection and client-side DOM manipulation. The issue is unusually thorough on input validation — the IDOR note, the defense-in-depth framing for `parseTimelineFilters`, and the "drop invalid params silently" policy are all correctly specified. **IDOR non-issue is correctly assessed.** The issue explicitly states: "IDOR is a non-issue: any `READ_ALL` user may read any person's timeline by design." This is consistent with the existing family-archive threat model (no per-user data segregation). No per-person guard needed. **Malformed URL param injection is addressed by `parseTimelineFilters` design.** Drop invalid `generation`, NaN years, out-of-range values — these are the right controls. The key security property is that *no raw user input from the URL reaches `api.GET` unvalidated*. The `parseTimelineFilters` function is the sanitization boundary. **One gap: `personId` validation.** The issue says "pass through as UUID string if present" but specifies no UUID format validation. A malformed `personId=../../../../etc/passwd` passed to `api.GET('/api/timeline', { params: { query: { personId: ... } } })` is not a path traversal risk (it's a query param, not a path param), but the backend will receive garbage. Recommendation: validate `personId` against a UUID regex before passing through. The `documents/page.server.spec.ts` pattern does UUID validation before person name lookup (line 382: "returns empty string when senderId is not a valid UUID") — mirror that guard. **SSR-first protects auth.** The issue correctly states: "Never a client-side `fetch('/api/timeline')` in `onMount` (would expose the API route and bypass cookie-forwarded auth)." This is the critical security property. The auth cookie is forwarded by the SvelteKit `handleFetch` hook — client-side fetch bypasses it entirely. No additional control needed beyond enforcing the SSR-first constraint, which is already in the AC. **Year clamping boundary.** `new Date().getFullYear() + 1` is computed at call time (correct — not a hardcoded constant). A hardcoded year like `2026` would silently stop accepting valid years after that. The call-time computation is the right approach. **Error mapping.** The issue requires `getErrorMessage(code)` — no raw backend messages to users. This is correct and consistent with the codebase pattern. ### Recommendations - **Add UUID format validation for `personId` in `parseTimelineFilters`.** A simple UUID regex check (`/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i`) before passing through to the API. If the format is invalid, drop it (`personId: undefined`). Add a unit test: `personId=not-a-uuid` → parsed output has `personId: undefined`. - **Test that invalid params never reach `api.GET`.** The load-function tests should assert that when `personId=garbage` is in the URL, `api.GET` is called with `personId` absent from the query object — not with the garbage string. No open security decisions — the analysis is complete.

marcel commented 2026-06-07 19:53:57 +02:00

Author

Owner

Copy Link

📋 Elicit — Requirements Engineer

Observations

The spec is unusually dense and well-formed for a frontend-only filter issue. Most requirements are testable, the decisions are explicitly resolved, and the i18n table is complete. This round focuses on what is still underspecified or potentially contradictory given the spec's current richness.

AC gap — year-range swap behavior on reload. The AC says "a filtered URL reloaded re-activates the same filters and renders the same entries." But the spec also says fromYear > toYear is swapped silently in parseTimelineFilters. If a user shares ?fromYear=1950&toYear=1920, the load function swaps to 1920–1950 for the API call, but the URL still shows 1950–1920. On reload, the same swap happens — behavior is consistent, but the filter bar will show the corrected values (von: 1920, bis: 1950) while the URL shows the original inverted values. This is a requirements gap: the AC says "state survives reload via URL" but doesn't specify whether the URL is also corrected. Worth clarifying explicitly.

Missing i18n key for year-range error. The table has timeline_filter_year_range_error ("„von Jahr" muss kleiner oder gleich „bis Jahr" sein.") — this is the inline validation message. But the spec doesn't specify when this message is shown vs. hidden. Presumably: shown when fromYear > toYear (before the swap happens client-side, during typing before blur), hidden once the values are valid. This behavior should be in the AC or tasks to avoid ambiguity in implementation.

"Filter (N active)" trigger localization. The i18n table shows timeline_filter_trigger ("Filter") and timeline_filter_trigger_active ("Filter ({count} aktiv)"). Two separate keys for 0 vs. ≥1 active filters is the correct pattern (avoids awkward "Filter (0 aktiv)"). The {count} placeholder in Paraglide — verify this uses the project's existing pluralization helper or the m.timeline_filter_trigger_active({ count: n }) call signature matches how other count-carrying messages are defined in de.json.

"Reset" AC edge case. The AC says reset "returns to the unfiltered /zeitstrahl." But what does "unfiltered" mean for the URL? Is it goto('/zeitstrahl') (bare, no params) or goto('/zeitstrahl?') (empty params)? The existing SearchFilterBar.svelte uses <a href="/documents"> (bare path). Recommend specifying goto('/zeitstrahl') explicitly in the task.

"Both toggles off" with letters toggle also off. The spec says: "Given both event toggles are off, the timeline shows no event cards and the letters layer (if toggle is on) is shown." This covers the case where letters toggle is ON. But what if all three toggles are off? The spec says letters are client-side hidden when their toggle is off — so all three off → zero visible items → empty state should render. The AC should include: "Given all three layer toggles are off, the empty-state message is shown." Currently absent.

Undated bucket visibility in locked-person mode. The spec addresses undated bucket behavior with year range (hidden) and without (visible), but doesn't address what happens in locked-person mode (lockedPersonId set). Presumably the same rules apply — but worth confirming since the Lebensweg view (#10) may have different expectations for the undated bucket.

Recommendations

• Add AC: "Given fromYear > toYear, when the page is reloaded with those URL params, the filter bar shows the corrected (swapped) year values, and the timeline displays entries for the canonical (correct) year range."
• Add AC: "Given all three layer toggles are off, the empty-state message 'Keine Einträge entsprechen diesen Filtern.' is shown."
• Clarify task for reset: explicitly state goto('/zeitstrahl') (bare path, no query string).
• Specify inline validation message visibility: shown when fromYear > toYear and both inputs have been touched; hidden when values are valid or when only one input has a value.

Open Decisions

• Inverted year range URL correction: When fromYear > toYear is in the URL, should +page.server.ts emit a redirect(301, corrected_url) to fix the URL, or is silent swap (correct API call, wrong URL) acceptable? Redirect is more correct but adds a round-trip. Silent swap is simpler and the issue implies it. The current spec says "swap" — confirm this means silent swap only, not URL correction.

## 📋 Elicit — Requirements Engineer ### Observations The spec is unusually dense and well-formed for a frontend-only filter issue. Most requirements are testable, the decisions are explicitly resolved, and the i18n table is complete. This round focuses on what is *still* underspecified or potentially contradictory given the spec's current richness. **AC gap — year-range swap behavior on reload.** The AC says "a filtered URL reloaded re-activates the same filters and renders the same entries." But the spec also says `fromYear > toYear` is swapped silently in `parseTimelineFilters`. If a user shares `?fromYear=1950&toYear=1920`, the load function swaps to `1920–1950` for the API call, but the URL still shows `1950–1920`. On reload, the same swap happens — behavior is consistent, but the filter bar will show the *corrected* values (`von: 1920`, `bis: 1950`) while the URL shows the *original* inverted values. This is a requirements gap: the AC says "state survives reload via URL" but doesn't specify whether the URL is also corrected. Worth clarifying explicitly. **Missing i18n key for year-range error.** The table has `timeline_filter_year_range_error` ("„von Jahr" muss kleiner oder gleich „bis Jahr" sein.") — this is the inline validation message. But the spec doesn't specify when this message is *shown vs. hidden*. Presumably: shown when `fromYear > toYear` (before the swap happens client-side, during typing before blur), hidden once the values are valid. This behavior should be in the AC or tasks to avoid ambiguity in implementation. **"Filter (N active)" trigger localization.** The i18n table shows `timeline_filter_trigger` ("Filter") and `timeline_filter_trigger_active` ("Filter ({count} aktiv)"). Two separate keys for 0 vs. ≥1 active filters is the correct pattern (avoids awkward "Filter (0 aktiv)"). The `{count}` placeholder in Paraglide — verify this uses the project's existing pluralization helper or the `m.timeline_filter_trigger_active({ count: n })` call signature matches how other count-carrying messages are defined in `de.json`. **"Reset" AC edge case.** The AC says reset "returns to the unfiltered `/zeitstrahl`." But what does "unfiltered" mean for the URL? Is it `goto('/zeitstrahl')` (bare, no params) or `goto('/zeitstrahl?')` (empty params)? The existing `SearchFilterBar.svelte` uses `<a href="/documents">` (bare path). Recommend specifying `goto('/zeitstrahl')` explicitly in the task. **"Both toggles off" with letters toggle also off.** The spec says: "Given both event toggles are off, the timeline shows no event cards and the letters layer (if toggle is on) is shown." This covers the case where letters toggle is ON. But what if all three toggles are off? The spec says letters are client-side hidden when their toggle is off — so all three off → zero visible items → empty state should render. The AC should include: "Given all three layer toggles are off, the empty-state message is shown." Currently absent. **Undated bucket visibility in locked-person mode.** The spec addresses undated bucket behavior with year range (hidden) and without (visible), but doesn't address what happens in locked-person mode (`lockedPersonId` set). Presumably the same rules apply — but worth confirming since the Lebensweg view (#10) may have different expectations for the undated bucket. ### Recommendations - Add AC: "Given `fromYear > toYear`, when the page is reloaded with those URL params, the filter bar shows the corrected (swapped) year values, and the timeline displays entries for the canonical (correct) year range." - Add AC: "Given all three layer toggles are off, the empty-state message 'Keine Einträge entsprechen diesen Filtern.' is shown." - Clarify task for reset: explicitly state `goto('/zeitstrahl')` (bare path, no query string). - Specify inline validation message visibility: shown when `fromYear > toYear` *and* both inputs have been touched; hidden when values are valid or when only one input has a value. ### Open Decisions - **Inverted year range URL correction:** When `fromYear > toYear` is in the URL, should `+page.server.ts` emit a `redirect(301, corrected_url)` to fix the URL, or is silent swap (correct API call, wrong URL) acceptable? Redirect is more correct but adds a round-trip. Silent swap is simpler and the issue implies it. The current spec says "swap" — confirm this means silent swap only, not URL correction.

marcel commented 2026-06-07 19:54:24 +02:00

Author

Owner

Copy Link

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

The spec is exceptionally detailed on accessibility requirements — more so than most issues in this codebase. The dual-audience concern (phone-first readers + seniors on desktop) is explicitly addressed. I reviewed SearchFilterBar.svelte as the canonical reference and have specific observations:

slide transition in SearchFilterBar.svelte has no reduced-motion guard (line 173). The issue correctly mandates duration: 0 when prefers-reduced-motion: reduce is active. Important: this must be a reactive value (reading window.matchMedia at mount time), not a hardcoded constant, because media preference can change while the page is open. The implementation should be:

<script lang="ts">
  import { slide } from 'svelte/transition';
  const reducedMotion = typeof window !== 'undefined' 
    && window.matchMedia('(prefers-reduced-motion: reduce)').matches;
</script>
{#if open}
  <div transition:slide={{ duration: reducedMotion ? 0 : 300 }}>
    ...
  </div>
{/if}

Note: I found this exact window.matchMedia pattern already in /documents/[id]/+page.svelte (line 58 approximately) — copy from there.

Year-range inputs side-by-side at 375px. The spec mandates min-w-0 flex-1 each. min-w-0 is essential — without it, a flex child won't shrink below its content width, causing overflow on 375px screens. The numbers "1920" and "1930" fit fine, but typed partial values ("19") may show 4-char placeholder text. Verify the placeholder text for "von Jahr" and "bis Jahr" doesn't overflow its flex container at 375px.

min-h-[44px] on the filter trigger. The spec requires this. Also apply min-h-[44px] to all three layer toggle buttons and the generation <select> wrapper — not just the collapsible trigger. All four interactive elements are in the senior user's critical path.

Generation <select> touch target. The spec says min-h-[44px] on the wrapper div. Native <select> elements on iOS have inconsistent touch target sizes. The wrapper div approach (wrapping <select> to fill it) is correct — but verify the <select> itself uses class="w-full h-full" inside the wrapper. Without this, the wrapper is 44px tall but the select element inside may be smaller, creating a dead zone.

✓ glyph in layer toggles. The spec mandates a visible ✓ glyph inside active toggle buttons (mirroring the SearchFilterBar's undated toggle at lines 295–301). This must be visible to both sighted and non-sighted users: aria-pressed covers screen readers; the ✓ glyph covers sighted users. Never rely on color alone (WCAG 1.4.1). The existing undated toggle pattern is the exact template to copy.

"Filter (N active)" trigger — seniors miss disappearing UI. The spec says the trigger is "sticky/persistent" — visible even when the collapsible is closed. Ensure the trigger stays in the document flow (not position: sticky with a fixed offset that clips it on small screens). At 375px in landscape, if the sticky trigger overlaps the timeline content, that's a usability regression.

Reset button text vs. icon. The spec mandates a text button, not icon-only. The existing SearchFilterBar uses an icon-only reset (<a href="/documents"> with just a close icon). The timeline filter must do better: a text button labeled "Filter zurücksetzen" is required. This is an explicit accessibility call-out — seniors miss icon-only affordances.

Dark mode for the filter collapsible. All color classes must use semantic tokens (bg-surface, border-line, text-ink-2) — no raw Tailwind palette colors. The layer toggles use bg-primary text-primary-fg (active) and bg-muted text-ink-2 (inactive) — confirm these tokens exist in layout.css and have dark-mode remappings. The bg-primary / text-primary-fg tokens are used in SearchFilterBar (line 196), so they exist.

Empty state with filters open. When filters are open and zero results are returned, the empty-state message "Keine Einträge entsprechen diesen Filtern." + reset button should appear below the open filter bar, not replacing it. Users need to see their active filters to understand why nothing matched. The reset button in the empty state and the reset button in the filter bar must both work consistently.

Recommendations

• Copy the window.matchMedia('(prefers-reduced-motion: reduce)') guard from /documents/[id]/+page.svelte exactly.
• At 375px, test the two year inputs side by side with the labels "von Jahr" and "bis Jahr" — the label text is longer than typical inputs; use text-sm or text-xs for labels if needed to prevent wrapping.
• The generation <select> should use a <label for="generation"> (not aria-label) — native label association works better across assistive technologies than ARIA workarounds.

No open UX decisions — the spec is specific enough to implement without further input.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations The spec is exceptionally detailed on accessibility requirements — more so than most issues in this codebase. The dual-audience concern (phone-first readers + seniors on desktop) is explicitly addressed. I reviewed `SearchFilterBar.svelte` as the canonical reference and have specific observations: **`slide` transition in `SearchFilterBar.svelte` has no reduced-motion guard (line 173).** The issue correctly mandates `duration: 0` when `prefers-reduced-motion: reduce` is active. Important: this must be a reactive value (reading `window.matchMedia` at mount time), not a hardcoded constant, because media preference can change while the page is open. The implementation should be: ```svelte <script lang="ts"> import { slide } from 'svelte/transition'; const reducedMotion = typeof window !== 'undefined' && window.matchMedia('(prefers-reduced-motion: reduce)').matches; </script> {#if open} <div transition:slide={{ duration: reducedMotion ? 0 : 300 }}> ... </div> {/if} ``` Note: I found this exact `window.matchMedia` pattern already in `/documents/[id]/+page.svelte` (line 58 approximately) — copy from there. **Year-range inputs side-by-side at 375px.** The spec mandates `min-w-0 flex-1` each. `min-w-0` is essential — without it, a flex child won't shrink below its content width, causing overflow on 375px screens. The numbers "1920" and "1930" fit fine, but typed partial values ("19") may show 4-char placeholder text. Verify the placeholder text for "von Jahr" and "bis Jahr" doesn't overflow its flex container at 375px. **`min-h-[44px]` on the filter trigger.** The spec requires this. Also apply `min-h-[44px]` to all three layer toggle buttons and the generation `<select>` wrapper — not just the collapsible trigger. All four interactive elements are in the senior user's critical path. **Generation `<select>` touch target.** The spec says `min-h-[44px]` on the wrapper div. Native `<select>` elements on iOS have inconsistent touch target sizes. The wrapper `div` approach (wrapping `<select>` to fill it) is correct — but verify the `<select>` itself uses `class="w-full h-full"` inside the wrapper. Without this, the wrapper is 44px tall but the select element inside may be smaller, creating a dead zone. **✓ glyph in layer toggles.** The spec mandates a visible ✓ glyph inside active toggle buttons (mirroring the `SearchFilterBar`'s undated toggle at lines 295–301). This must be visible to both sighted and non-sighted users: `aria-pressed` covers screen readers; the ✓ glyph covers sighted users. Never rely on color alone (WCAG 1.4.1). The existing undated toggle pattern is the exact template to copy. **"Filter (N active)" trigger — seniors miss disappearing UI.** The spec says the trigger is "sticky/persistent" — visible even when the collapsible is closed. Ensure the trigger stays in the document flow (not `position: sticky` with a fixed offset that clips it on small screens). At 375px in landscape, if the sticky trigger overlaps the timeline content, that's a usability regression. **Reset button text vs. icon.** The spec mandates a **text** button, not icon-only. The existing `SearchFilterBar` uses an icon-only reset (`<a href="/documents">` with just a close icon). The timeline filter must do better: a text button labeled "Filter zurücksetzen" is required. This is an explicit accessibility call-out — seniors miss icon-only affordances. **Dark mode for the filter collapsible.** All color classes must use semantic tokens (`bg-surface`, `border-line`, `text-ink-2`) — no raw Tailwind palette colors. The layer toggles use `bg-primary text-primary-fg` (active) and `bg-muted text-ink-2` (inactive) — confirm these tokens exist in `layout.css` and have dark-mode remappings. The `bg-primary` / `text-primary-fg` tokens are used in `SearchFilterBar` (line 196), so they exist. **Empty state with filters open.** When filters are open and zero results are returned, the empty-state message "Keine Einträge entsprechen diesen Filtern." + reset button should appear *below the open filter bar*, not replacing it. Users need to see their active filters to understand why nothing matched. The reset button in the empty state and the reset button in the filter bar must both work consistently. ### Recommendations - Copy the `window.matchMedia('(prefers-reduced-motion: reduce)')` guard from `/documents/[id]/+page.svelte` exactly. - At 375px, test the two year inputs side by side with the labels "von Jahr" and "bis Jahr" — the label text is longer than typical inputs; use `text-sm` or `text-xs` for labels if needed to prevent wrapping. - The generation `<select>` should use a `<label for="generation">` (not `aria-label`) — native label association works better across assistive technologies than ARIA workarounds. No open UX decisions — the spec is specific enough to implement without further input.

marcel commented 2026-06-07 19:54:52 +02:00

Author

Owner

Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

The test plan in the issue is the most complete I've seen in this codebase. The three-layer structure (unit/component/E2E) maps directly to the project's test pyramid. I validated the file patterns against existing tests before commenting.

documents/page.server.spec.ts is the correct pattern to mirror — the makeUrl() factory, vi.mock('$lib/shared/api.server'), beforeEach(vi.clearAllMocks), and mockGet.mock.calls[0][1].params.query assertion style are all established conventions. The load-function tests for this issue should look identical structurally.

Missing test case for generation=0 (G0 valid, not dropped). The issue mentions this in the task list: "generation=0 → passed as 0 (G0 is valid, not dropped)." Confirming this must be a unit test in parseTimelineFilters, not just implied by the component behavior. A naïve implementation using parseInt + truthiness check would drop G0. The test must assert: parseTimelineFilters(makeUrl({ generation: '0' })).generation === 0.

Missing test case for personId UUID validation. As flagged by Nora: if personId gets UUID validation in parseTimelineFilters (recommended), there must be a unit test: personId=not-a-uuid → personId: undefined in output.

lockedPersonId DOM absence test is correctly specified. queryByRole('combobox', { name: /Person/ }) === null is the right assertion. The component test must not use toBeVisible() — that only checks CSS visibility, not DOM presence. queryByRole returning null proves the element is not in the DOM.

The test.skip guard on E2E spec is essential. Since /zeitstrahl doesn't exist until #7 merges, any E2E test without test.skip will fail in CI. The guard must be:

test.skip(true, 'Depends on #7: /zeitstrahl route not yet merged');

or conditionally skip based on an env var. The implementation note says "CI branch ordering enforces the dependency" — clarify this in a code comment on the skip, otherwise future readers won't know when to remove it.

axe check in accessibility.spec.ts — additive, not a new file. The issue correctly scopes this: add to AUTHENTICATED_PAGES array in accessibility.spec.ts once #7 lands. The existing buildAxe() helper and dark-mode test pattern in that file are already the right infrastructure. No new spec file needed.

Component test file naming. The issue specifies TimelineFilters.svelte.spec.ts — this is correct for the client Vitest project (browser-based). Confirm the Vitest config maps *.svelte.spec.ts to the browser project. I checked: PersonTypeahead.svelte.spec.ts exists at src/lib/person/, confirming the naming convention is established.

Load-function test isolation gap. The issue specifies mocking createApiClient at module boundary. One edge case not in the matrix: what happens when api.GET('/api/timeline') throws (network error)? The existing documents/page.server.spec.ts covers this (lines 316–329) — mirror that pattern. The load function should return { error: ... } not throw.

"Combinations work" matrix. The issue says "2–3 combinations." Minimum recommended combinations to cover:

1. { generation: 2, fromYear: 1920, toYear: 1940 } — year + generation together
2. { personId: UUID, type: 'PERSONAL' } — person + type filter
3. { fromYear: 1900 } — single bound (no toYear) — ensure toYear is omitted, not sent as undefined string

CI time impact. Unit tests for parseTimelineFilters will be fast (<10s). Browser component tests add ~3s per run (single-file local). E2E (skipped until #7) adds 0s to the current CI budget. Load impact is minimal.

Recommendations

• Add test: generation=0 → passed as 0 (not dropped).
• Add test: personId=not-a-uuid → personId: undefined (if UUID validation is added per Nora's recommendation).
• Add test: single-bound year range (fromYear only, no toYear) ��� toYear absent from api.GET call, not sent as undefined.
• Add test: all layer toggles off → API called normally with no type param, event cards hidden client-side (component-level).
• Document the test.skip removal condition in a code comment: "Remove skip when #7 (/zeitstrahl route) is merged."

No open test strategy decisions.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations The test plan in the issue is the most complete I've seen in this codebase. The three-layer structure (unit/component/E2E) maps directly to the project's test pyramid. I validated the file patterns against existing tests before commenting. **`documents/page.server.spec.ts` is the correct pattern to mirror** — the `makeUrl()` factory, `vi.mock('$lib/shared/api.server')`, `beforeEach(vi.clearAllMocks)`, and `mockGet.mock.calls[0][1].params.query` assertion style are all established conventions. The load-function tests for this issue should look identical structurally. **Missing test case for `generation=0` (G0 valid, not dropped).** The issue mentions this in the task list: "`generation=0` → passed as `0` (G0 is valid, not dropped)." Confirming this must be a unit test in `parseTimelineFilters`, not just implied by the component behavior. A naïve implementation using `parseInt` + truthiness check would drop G0. The test must assert: `parseTimelineFilters(makeUrl({ generation: '0' })).generation === 0`. **Missing test case for `personId` UUID validation.** As flagged by Nora: if `personId` gets UUID validation in `parseTimelineFilters` (recommended), there must be a unit test: `personId=not-a-uuid` → `personId: undefined` in output. **`lockedPersonId` DOM absence test is correctly specified.** `queryByRole('combobox', { name: /Person/ }) === null` is the right assertion. The component test must not use `toBeVisible()` — that only checks CSS visibility, not DOM presence. `queryByRole` returning `null` proves the element is not in the DOM. **The `test.skip` guard on E2E spec is essential.** Since `/zeitstrahl` doesn't exist until #7 merges, any E2E test without `test.skip` will fail in CI. The guard must be: ```typescript test.skip(true, 'Depends on #7: /zeitstrahl route not yet merged'); ``` or conditionally skip based on an env var. The implementation note says "CI branch ordering enforces the dependency" — clarify this in a code comment on the skip, otherwise future readers won't know when to remove it. **axe check in `accessibility.spec.ts` — additive, not a new file.** The issue correctly scopes this: add to `AUTHENTICATED_PAGES` array in `accessibility.spec.ts` once #7 lands. The existing `buildAxe()` helper and dark-mode test pattern in that file are already the right infrastructure. No new spec file needed. **Component test file naming.** The issue specifies `TimelineFilters.svelte.spec.ts` — this is correct for the `client` Vitest project (browser-based). Confirm the Vitest config maps `*.svelte.spec.ts` to the browser project. I checked: `PersonTypeahead.svelte.spec.ts` exists at `src/lib/person/`, confirming the naming convention is established. **Load-function test isolation gap.** The issue specifies mocking `createApiClient` at module boundary. One edge case not in the matrix: what happens when `api.GET('/api/timeline')` throws (network error)? The existing `documents/page.server.spec.ts` covers this (lines 316–329) — mirror that pattern. The load function should return `{ error: ... }` not throw. **"Combinations work" matrix.** The issue says "2–3 combinations." Minimum recommended combinations to cover: 1. `{ generation: 2, fromYear: 1920, toYear: 1940 }` — year + generation together 2. `{ personId: UUID, type: 'PERSONAL' }` — person + type filter 3. `{ fromYear: 1900 }` — single bound (no `toYear`) — ensure `toYear` is omitted, not sent as `undefined` string **CI time impact.** Unit tests for `parseTimelineFilters` will be fast (<10s). Browser component tests add ~3s per run (single-file local). E2E (skipped until #7) adds 0s to the current CI budget. Load impact is minimal. ### Recommendations - Add test: `generation=0` → passed as `0` (not dropped). - Add test: `personId=not-a-uuid` → `personId: undefined` (if UUID validation is added per Nora's recommendation). - Add test: single-bound year range (`fromYear` only, no `toYear`) ��� `toYear` absent from `api.GET` call, not sent as `undefined`. - Add test: all layer toggles off → API called normally with no `type` param, event cards hidden client-side (component-level). - Document the `test.skip` removal condition in a code comment: "Remove skip when #7 (`/zeitstrahl` route) is merged." No open test strategy decisions.

marcel commented 2026-06-07 19:55:07 +02:00

Author

Owner

Copy Link

🖥️ Tobias Wendt — DevOps & Platform Engineer

Observations

This is a frontend-only issue with no new infrastructure, no new Docker services, no new env vars, and no new backend endpoints. From an ops perspective, the scope is clean.

No CI impact on the current pipeline. E2E tests are explicitly test.skipped until #7 lands — confirmed by the issue. The unit and component tests for parseTimelineFilters and TimelineFilters.svelte will run in the existing Vitest pipeline. No pipeline changes needed.

No new env vars. The issue explicitly states this. The year clamping uses new Date().getFullYear() + 1 computed at call time — no env var needed. Correct.

No new Docker service or infrastructure component. The filter bar calls GET /api/timeline (owned by #5). If #5 isn't deployed, the filter bar returns empty — it doesn't crash CI.

test.skip CI ordering concern. The E2E filter behavior test is skipped until #7 merges. But the issue says "CI branch ordering enforces the dependency." This needs to be more concrete: if #8 (this issue) merges before #7, the CI for main will have an E2E test file that is skipped but present. That's acceptable — skipped tests don't fail CI. What's not acceptable is if the test.skip is removed without #7 in place. Recommend the skip comment explicitly state the dependency: // Remove when issue #7 merges: /zeitstrahl route does not exist until then.

No observability changes needed. The filter bar is presentation-only. The GET /api/timeline calls will appear in the existing Loki/Grafana log pipeline with no additional configuration.

Pre-commit hook note. The issue adds new files to frontend/src/routes/zeitstrahl/ and new i18n keys to messages/{de,en,es}.json. The pre-commit hook runs cd frontend && npm run lint. A fresh worktree (per project memory: feedback_precommit_needs_frontend_install.md) needs npm install before the first commit. If this is implemented in a worktree, run npm install before committing.

Recommendations

• No infrastructure changes needed. Confirm no new .env entries are introduced.
• Add the removal condition to the test.skip comment as a code note.
• Verify frontend/messages/en.json and messages/es.json get the i18n keys added alongside de.json — the pre-commit hook runs lint but not paraglide compilation; a missing key in one language file is a silent runtime regression.

No open DevOps decisions — this issue is correctly scoped as frontend-only.

## 🖥️ Tobias Wendt — DevOps & Platform Engineer ### Observations This is a frontend-only issue with no new infrastructure, no new Docker services, no new env vars, and no new backend endpoints. From an ops perspective, the scope is clean. **No CI impact on the current pipeline.** E2E tests are explicitly `test.skip`ped until #7 lands — confirmed by the issue. The unit and component tests for `parseTimelineFilters` and `TimelineFilters.svelte` will run in the existing Vitest pipeline. No pipeline changes needed. **No new env vars.** The issue explicitly states this. The year clamping uses `new Date().getFullYear() + 1` computed at call time — no env var needed. Correct. **No new Docker service or infrastructure component.** The filter bar calls `GET /api/timeline` (owned by #5). If #5 isn't deployed, the filter bar returns empty — it doesn't crash CI. **`test.skip` CI ordering concern.** The E2E filter behavior test is skipped until #7 merges. But the issue says "CI branch ordering enforces the dependency." This needs to be more concrete: if #8 (this issue) merges before #7, the CI for `main` will have an E2E test file that is skipped but present. That's acceptable — skipped tests don't fail CI. What's not acceptable is if the `test.skip` is removed without #7 in place. Recommend the skip comment explicitly state the dependency: `// Remove when issue #7 merges: /zeitstrahl route does not exist until then`. **No observability changes needed.** The filter bar is presentation-only. The `GET /api/timeline` calls will appear in the existing Loki/Grafana log pipeline with no additional configuration. **Pre-commit hook note.** The issue adds new files to `frontend/src/routes/zeitstrahl/` and new i18n keys to `messages/{de,en,es}.json`. The pre-commit hook runs `cd frontend && npm run lint`. A fresh worktree (per project memory: `feedback_precommit_needs_frontend_install.md`) needs `npm install` before the first commit. If this is implemented in a worktree, run `npm install` before committing. ### Recommendations - No infrastructure changes needed. Confirm no new `.env` entries are introduced. - Add the removal condition to the `test.skip` comment as a code note. - Verify `frontend/messages/en.json` and `messages/es.json` get the i18n keys added alongside `de.json` — the pre-commit hook runs lint but not paraglide compilation; a missing key in one language file is a silent runtime regression. No open DevOps decisions — this issue is correctly scoped as frontend-only.

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

feat/issue-753-journey-editor

feat/issue-750-lesereisen-data-model

worktree-feat+issue-738-nl-search-backend

feat/issue-286-notification-bell-form-actions

feat/issue-580-sentry-backend

fix/issue-593-management-port-zero

worktree-feat+issue-557-upload-artifact-v3-pin

worktree-chore+issue-556-drop-client-branches-coverage-gate

fix/issue-514-prerender-crawl-bakes-redirects

fix/issue-472-prerender-entries

feat/issue-395-readme

feat/issue-345-bulk-mark-reviewed

feat/issue-344-bell-tooltip

feat/issue-341-annotieren-contrast

feat/issue-225-bulk-metadata-edit

feat/issue-317-bulk-upload

feat/issue-271-dashboard-redesign

docs/issue-240-mission-control-spec

refactor/issues-193-200

feat/issue-162-korrespondenz-redesign

feature/68-new-document-file-first

feat/81-discussion-discoverability

feat/62-document-bottom-panel

feature/56-backfill-file-hashes

feat/38-document-edit-history

fix/svelte5-test-delegation-and-login-auth

No results found.

Labels

Clear labels

P0-critical

Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.

P1-high

Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.

P2-medium

Noticeable improvement; doesn't block anything; schedule opportunistically.

P3-later

Cosmetic or parking-lot work; fine to stay open indefinitely.

audit

Read-only audit / assessment work; produces a report rather than changing code

bug

Something isn't working

cleanup

Removal of dead code, vague comments, naming violations, and scratch artifacts

collaboration

We want to extend the family archive to have more collaboration tools

conversation

descoped

We will do that later

devops

documentation

README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs

epic

Marker for epic-level issues that group multiple child issues

feature

file-upload

legibility

Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction

needs-discussion

Has an open decision or design question that must be resolved before implementation can start.

notification

person

phase-1: security

Security hygiene — must be done first

phase-2: container-images

Production-ready multi-stage Docker images

phase-3: prod-compose

Production compose overlay + Caddy reverse proxy

phase-4: spring-prod-profile

Spring Boot production configuration profile

phase-5: backups

Database and object storage backup strategy

phase-6: deployment-docs

.env.example, DEPLOYMENT.md, runbook

phase-7: monitoring

Prometheus, Loki, Grafana, Alertmanager

refactor

Code restructuring without behaviour change

security

test

ui

UI/UX and accessibility issues

user

No Label P3-later feature ui

Milestone

No items

No Milestone Zeitstrahl — Family Timeline

Projects

Clear projects

No project

Assignees

Clear assignees

Jens marcel

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#780