Timeline: shared precision-aware date-label helper (façade over formatDocumentDate) #778

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

Milestone: Zeitstrahl — Family Timeline
Spec: docs/superpowers/specs/2026-06-07-family-timeline-design.md § "Date rendering" / "Frontend"

Context

Events and letters on the timeline render dates the same way. A fully-tested, locale-aware, precision-aware formatter already exists in the codebase: frontend/src/lib/shared/utils/documentDate.tsformatDocumentDate(iso, precision, end, raw, locale). It covers all 7 DatePrecision values, is localized de/en/es, is consumed by DocumentDate.svelte, DocumentTopBarTitle.svelte and DocumentMultiSelect.svelte, and is guarded against Java drift by docs/date-label-fixtures.json (asserted by both documentDate.spec.ts and the Java DocumentTitleFormatterTest, per issue #666).

This issue is therefore NOT "implement a formatter" — it is "wrap the existing formatter behind a thin timeline façade". No precision logic is re-typed; all rendering stays in the shared module so it remains inside the cross-language fixtures drift-guard.

Scope

Add frontend/src/lib/timeline/dateLabel.ts as a façade that delegates to formatDocumentDate. The timeline carries no verbatim spreadsheet cell, so raw is passed as null explicitly (the SEASON word derives from the structured anchor month via seasonOfMonth()); the active locale is threaded via getLocale().

// lib/timeline/dateLabel.ts
import { formatDocumentDate, type DatePrecision } from '$lib/shared/utils/documentDate';
import { getLocale } from '$lib/paraglide/runtime.js';

/**
 * Timeline façade over the shared precision-aware formatter.
 * Returns null for UNKNOWN / undated entries — undated items render NO date chip;
 * the "Ohne Datum" bucket header is owned by YearBand/TimelineView, not this helper.
 *
 * @param eventDateEnd - undefined and null are treated equivalently (both → null passed to formatter).
 */
export function timelineDateLabel(
  eventDate: string | null | undefined,
  precision: DatePrecision,
  eventDateEnd?: string | null
): string | null {
  if (precision === 'UNKNOWN' || !eventDate) return null;
  // raw is always null for timeline events — no verbatim spreadsheet cell;
  // season words derive from the structured anchor month (never from untrusted text).
  return formatDocumentDate(eventDate, precision, eventDateEnd ?? null, null, getLocale());
}

Import notes:

  • Use import type { DatePrecision } (type-only import) — consistent with how type-only imports are used elsewhere in the frontend and prevents any accidental tree-shaking difference.
  • Import getLocale from '$lib/paraglide/runtime.js' (with .js extension) — matches the majority pattern in utility code (DocumentDate.svelte, DocumentMultiSelect.svelte).
  • DatePrecision is hand-declared in documentDate.ts, not OpenAPI-generated, so codegen does not surface it. See the drift-risk note below.

Rendering by precision

Per the shared formatDocumentDate contract (authoritative; see docs/date-label-fixtures.json):

precision render example (de)
DAY full date 28. Juli 1914
MONTH month + year Juli 1914
SEASON season + year (from anchor month, raw=null) Sommer 1914
YEAR year only 1914
APPROX "ca." + year ca. 1914
RANGE full/collapsed date range (the shared rule — NOT year–year) 28. Juli 1914 – 11. Nov. 1918
UNKNOWN façade returns null — no per-item label (no date chip)

All structured parts are localized de/en/es via Intl.DateTimeFormat(locale, …) and Paraglide date_* keys (already present in all three locales — no new strings).

DatePrecision drift-risk note

DatePrecision is hand-declared in documentDate.ts and is the contract for docs/date-label-fixtures.json. When the backend TimelineEventDTO is generated via OpenAPI (after issues 5/7 merge), a generated DatePrecision type will also appear. dateLabel.ts should stay on the hand-declared type — it is the fixtures-guard contract and must be updated manually together with the Java enum. A comment in documentDate.ts documents this: // mirrors the backend DatePrecision enum; update manually together with the Java enum when values are added — do not migrate to the OpenAPI-generated type.

CLAUDE.md + C4 diagram update

Doc-update rule triggers on "new domain module". This issue creates lib/timeline/ as a scaffold with a single helper — defer CLAUDE.md table + docs/architecture/c4/l3-frontend-*.puml to issue 7, the first issue that adds a route. Note it explicitly in the issue 7 PR description so it is not forgotten. Do not add a stub C4 entry now (it would be incomplete/misleading).

Notes for card issues 7–9 (planning)

  • The date label string returned by timelineDateLabel should be rendered inside a <time datetime="{eventDate}"> element in EventCard.svelte / LetterCard.svelte, not a bare <span> — gives screen readers and search engines the machine-readable ISO date alongside the human-readable label.
  • For APPROX labels, ca. may need <abbr title="circa">ca.</abbr> for screen reader clarity — a card-level concern.
  • Confirm that EventCard.svelte and LetterCard.svelte do not use {@html} for event.title, event.description, or letter.title. Svelte auto-escapes {value} interpolations; {@html} requires DOMPurify if ever added for rich text.

Decisions resolved

  1. Reuse, do not reimplement (façade). dateLabel.ts is a one-line delegation to formatDocumentDate(…, raw=null, getLocale()). Rationale: DRY; keeps all precision logic in the shared module and inside the fixtures drift-guard; a forked formatter would silently drop timeline rendering from the Java-drift contract (#666). (Unanimous: Markus, Felix, Sara, Elicit, Leonie.)
  2. Drop the npm run generate:api task from this issue. Timeline DTOs do not exist in the OpenAPI spec yet (backend issues 2–5 unmerged); running codegen now is a no-op. The codegen step belongs in the first frontend issue that consumes a timeline DTO (issue 7), after the assembly endpoint (issue 5) is merged. (Markus, Felix, Tobias, Elicit.)
  3. RANGE label = the existing full/collapsed-date range output. Cheapest, zero Java/fixtures fork, and dates read identically across timeline / detail / multi-select / top-bar (Nielsen #4). The compact 1914–1918 year-span is a band-placement concern owned by YearBand/TimelineView, out of scope here. (Markus, Elicit, Leonie.)
  4. UNKNOWN → façade returns null; "Ohne Datum" is a bucket header. Undated items render no date chip; warmer family-reader copy lives on the bucket header rendered by YearBand/TimelineView. Separates item-label from bucket-label cleanly. (Elicit, Leonie.)
  5. No ADR for this issue. No new domain/data model here. The ADR belongs to the backend entity issue (#2). (Markus.)
  6. Security: pass raw=null explicitly. The timeline has no operator-entered verbatim cell; passing null keeps the SEASON word coming from the structured anchor month, never from untrusted free text. (Nora.)
  7. CLAUDE.md + C4 diagram: defer to issue 7. This issue adds a single helper file — a stub entry now would be incomplete. Issue 7 (first route) is the correct boundary; note it in the issue 7 PR description. (Decision resolved R1.)
  8. DatePrecision type: stay on hand-declared type permanently; add drift-risk comment now. The hand-declared type is the fixtures-guard contract and must track the Java enum. Adding a comment in documentDate.ts now documents the migration decision point before issue 7 lands. (Decision resolved R1.)

Tasks

  • Implement frontend/src/lib/timeline/dateLabel.ts as the façade above.
    • Delegate to formatDocumentDate, pass raw=null, thread getLocale(), return null for UNKNOWN/undated.
    • Import getLocale from '$lib/paraglide/runtime.js' (with .js extension).
    • Use import type { DatePrecision } (type-only import).
    • Add security comment explaining raw=null is intentional (see code block above).
    • Add JSDoc noting undefined and null are equivalent for eventDateEnd.
  • Add drift-risk comment to DatePrecision in frontend/src/lib/shared/utils/documentDate.ts (see note above).
  • Add 'src/lib/timeline/**' to the coverage include array in frontend/vite.config.ts (1-line change; ensures the 80% branch gate applies to the new module).
  • Write frontend/src/lib/timeline/dateLabel.spec.ts (see Tests section below).

Acceptance criteria

  • The timeline renders every dated entry through the shared formatDocumentDate helper, so event and letter dates are identical to how they render elsewhere in the app, localized de/en/es.
  • UNKNOWN/undated entries yield null from the façade (no date chip); the "Ohne Datum" bucket label is not produced by this helper.
  • No precision/rendering logic is duplicated outside documentDate.ts; the fixtures drift-guard remains the single source of truth.
  • 'src/lib/timeline/**' is present in vite.config.ts coverage includes so the module is within the 80% branch gate.

Tests

frontend/src/lib/timeline/dateLabel.spec.ts — a thin delegation/integration suite, NOT a precision matrix (the full matrix lives in documentDate.spec.ts + the fixtures guard). File extension is *.spec.ts (not *.svelte.spec.ts) — runs in the Vitest server project (Node environment). Mock pattern: vi.mock('$lib/paraglide/runtime', () => ({ getLocale: vi.fn(() => 'de') })).

Test cases (name as full sentences for self-documenting CI output):

  • 'returns localized DAY label containing the day number and month name in German'timelineDateLabel('1914-07-28', 'DAY') with getLocale'de', assert contains 28 and Juli.
  • 'returns localized DAY label in English when locale is en' — same date with getLocale'en', assert contains July.
  • 'derives localized season word from anchor month when precision is SEASON (raw=null path)'timelineDateLabel('1916-04-01', 'SEASON') → contains Frühling (de) / Spring (en).
  • 'returns null for UNKNOWN precision with a date'timelineDateLabel('1914-07-28', 'UNKNOWN')null.
  • 'returns null for UNKNOWN precision without a date'timelineDateLabel(null, 'UNKNOWN')null.
  • 'returns null for non-UNKNOWN precision when eventDate is null'timelineDateLabel(null, 'APPROX')null (façade guard short-circuits before formatDocumentDate).
  • 'returns null for non-UNKNOWN precision when eventDate is empty string'timelineDateLabel('', 'DAY')null (empty string is falsy).
  • 'delegates same-year RANGE to shared formatter with concrete output'timelineDateLabel('1914-01-01', 'RANGE', '1914-11-11')toContain('1914') and toContain('Nov') (or match exact shared-formatter output; use formatDocumentDate directly to compute expected value rather than a literal string).

Single-file *.spec.ts, runs locally in seconds; full sweep left to CI.

**Milestone:** Zeitstrahl — Family Timeline **Spec:** `docs/superpowers/specs/2026-06-07-family-timeline-design.md` § "Date rendering" / "Frontend" ## Context Events and letters on the timeline render dates the same way. A fully-tested, locale-aware, precision-aware formatter **already exists** in the codebase: `frontend/src/lib/shared/utils/documentDate.ts` → `formatDocumentDate(iso, precision, end, raw, locale)`. It covers all 7 `DatePrecision` values, is localized de/en/es, is consumed by `DocumentDate.svelte`, `DocumentTopBarTitle.svelte` and `DocumentMultiSelect.svelte`, and is guarded against Java drift by `docs/date-label-fixtures.json` (asserted by both `documentDate.spec.ts` and the Java `DocumentTitleFormatterTest`, per issue #666). **This issue is therefore NOT "implement a formatter" — it is "wrap the existing formatter behind a thin timeline façade".** No precision logic is re-typed; all rendering stays in the shared module so it remains inside the cross-language fixtures drift-guard. ## Scope Add `frontend/src/lib/timeline/dateLabel.ts` as a **façade** that delegates to `formatDocumentDate`. The timeline carries no verbatim spreadsheet cell, so `raw` is passed as `null` explicitly (the SEASON word derives from the structured anchor month via `seasonOfMonth()`); the active locale is threaded via `getLocale()`. ```ts // lib/timeline/dateLabel.ts import { formatDocumentDate, type DatePrecision } from '$lib/shared/utils/documentDate'; import { getLocale } from '$lib/paraglide/runtime.js'; /** * Timeline façade over the shared precision-aware formatter. * Returns null for UNKNOWN / undated entries — undated items render NO date chip; * the "Ohne Datum" bucket header is owned by YearBand/TimelineView, not this helper. * * @param eventDateEnd - undefined and null are treated equivalently (both → null passed to formatter). */ export function timelineDateLabel( eventDate: string | null | undefined, precision: DatePrecision, eventDateEnd?: string | null ): string | null { if (precision === 'UNKNOWN' || !eventDate) return null; // raw is always null for timeline events — no verbatim spreadsheet cell; // season words derive from the structured anchor month (never from untrusted text). return formatDocumentDate(eventDate, precision, eventDateEnd ?? null, null, getLocale()); } ``` **Import notes:** - Use `import type { DatePrecision }` (type-only import) — consistent with how type-only imports are used elsewhere in the frontend and prevents any accidental tree-shaking difference. - Import `getLocale` from `'$lib/paraglide/runtime.js'` (with `.js` extension) — matches the majority pattern in utility code (`DocumentDate.svelte`, `DocumentMultiSelect.svelte`). - `DatePrecision` is **hand-declared** in `documentDate.ts`, not OpenAPI-generated, so codegen does not surface it. See the drift-risk note below. ## Rendering by precision Per the shared `formatDocumentDate` contract (authoritative; see `docs/date-label-fixtures.json`): | precision | render | example (de) | |---|---|---| | `DAY` | full date | `28. Juli 1914` | | `MONTH` | month + year | `Juli 1914` | | `SEASON` | season + year (from anchor month, `raw=null`) | `Sommer 1914` | | `YEAR` | year only | `1914` | | `APPROX` | "ca." + year | `ca. 1914` | | `RANGE` | **full/collapsed date range** (the shared rule — NOT year–year) | `28. Juli 1914 – 11. Nov. 1918` | | `UNKNOWN` | façade returns `null` — no per-item label | (no date chip) | All structured parts are localized de/en/es via `Intl.DateTimeFormat(locale, …)` and Paraglide `date_*` keys (already present in all three locales — no new strings). ## DatePrecision drift-risk note `DatePrecision` is hand-declared in `documentDate.ts` and is the contract for `docs/date-label-fixtures.json`. When the backend `TimelineEventDTO` is generated via OpenAPI (after issues 5/7 merge), a generated `DatePrecision` type will also appear. **`dateLabel.ts` should stay on the hand-declared type** — it is the fixtures-guard contract and must be updated manually together with the Java enum. A comment in `documentDate.ts` documents this: `// mirrors the backend DatePrecision enum; update manually together with the Java enum when values are added — do not migrate to the OpenAPI-generated type`. ## CLAUDE.md + C4 diagram update Doc-update rule triggers on "new domain module". This issue creates `lib/timeline/` as a scaffold with a single helper — **defer CLAUDE.md table + `docs/architecture/c4/l3-frontend-*.puml` to issue 7**, the first issue that adds a route. Note it explicitly in the issue 7 PR description so it is not forgotten. Do not add a stub C4 entry now (it would be incomplete/misleading). ## Notes for card issues 7–9 (planning) - The date label string returned by `timelineDateLabel` should be rendered inside a `<time datetime="{eventDate}">` element in `EventCard.svelte` / `LetterCard.svelte`, not a bare `<span>` — gives screen readers and search engines the machine-readable ISO date alongside the human-readable label. - For APPROX labels, `ca.` may need `<abbr title="circa">ca.</abbr>` for screen reader clarity — a card-level concern. - Confirm that `EventCard.svelte` and `LetterCard.svelte` do not use `{@html}` for `event.title`, `event.description`, or `letter.title`. Svelte auto-escapes `{value}` interpolations; `{@html}` requires DOMPurify if ever added for rich text. ## Decisions resolved 1. **Reuse, do not reimplement (façade).** `dateLabel.ts` is a one-line delegation to `formatDocumentDate(…, raw=null, getLocale())`. Rationale: DRY; keeps all precision logic in the shared module and inside the fixtures drift-guard; a forked formatter would silently drop timeline rendering from the Java-drift contract (#666). _(Unanimous: Markus, Felix, Sara, Elicit, Leonie.)_ 2. **Drop the `npm run generate:api` task from this issue.** Timeline DTOs do not exist in the OpenAPI spec yet (backend issues 2–5 unmerged); running codegen now is a no-op. The codegen step belongs in the first frontend issue that *consumes* a timeline DTO (issue 7), after the assembly endpoint (issue 5) is merged. _(Markus, Felix, Tobias, Elicit.)_ 3. **RANGE label = the existing full/collapsed-date range output.** Cheapest, zero Java/fixtures fork, and dates read identically across timeline / detail / multi-select / top-bar (Nielsen #4). The compact `1914–1918` year-span is a band-placement concern owned by `YearBand`/`TimelineView`, out of scope here. _(Markus, Elicit, Leonie.)_ 4. **UNKNOWN → façade returns `null`; "Ohne Datum" is a bucket header.** Undated items render no date chip; warmer family-reader copy lives on the bucket header rendered by `YearBand`/`TimelineView`. Separates item-label from bucket-label cleanly. _(Elicit, Leonie.)_ 5. **No ADR for this issue.** No new domain/data model here. The ADR belongs to the backend entity issue (#2). _(Markus.)_ 6. **Security: pass `raw=null` explicitly.** The timeline has no operator-entered verbatim cell; passing `null` keeps the SEASON word coming from the structured anchor month, never from untrusted free text. _(Nora.)_ 7. **CLAUDE.md + C4 diagram: defer to issue 7.** This issue adds a single helper file — a stub entry now would be incomplete. Issue 7 (first route) is the correct boundary; note it in the issue 7 PR description. _(Decision resolved R1.)_ 8. **DatePrecision type: stay on hand-declared type permanently; add drift-risk comment now.** The hand-declared type is the fixtures-guard contract and must track the Java enum. Adding a comment in `documentDate.ts` now documents the migration decision point before issue 7 lands. _(Decision resolved R1.)_ ## Tasks - [ ] Implement `frontend/src/lib/timeline/dateLabel.ts` as the façade above. - Delegate to `formatDocumentDate`, pass `raw=null`, thread `getLocale()`, return `null` for UNKNOWN/undated. - Import `getLocale` from `'$lib/paraglide/runtime.js'` (with `.js` extension). - Use `import type { DatePrecision }` (type-only import). - Add security comment explaining `raw=null` is intentional (see code block above). - Add JSDoc noting `undefined` and `null` are equivalent for `eventDateEnd`. - [ ] Add drift-risk comment to `DatePrecision` in `frontend/src/lib/shared/utils/documentDate.ts` (see note above). - [ ] Add `'src/lib/timeline/**'` to the coverage `include` array in `frontend/vite.config.ts` (1-line change; ensures the 80% branch gate applies to the new module). - [ ] Write `frontend/src/lib/timeline/dateLabel.spec.ts` (see Tests section below). ## Acceptance criteria - The timeline renders every **dated** entry through the shared `formatDocumentDate` helper, so event and letter dates are identical to how they render elsewhere in the app, localized de/en/es. - `UNKNOWN`/undated entries yield `null` from the façade (no date chip); the "Ohne Datum" bucket label is **not** produced by this helper. - No precision/rendering logic is duplicated outside `documentDate.ts`; the fixtures drift-guard remains the single source of truth. - `'src/lib/timeline/**'` is present in `vite.config.ts` coverage includes so the module is within the 80% branch gate. ## Tests `frontend/src/lib/timeline/dateLabel.spec.ts` — a thin **delegation/integration** suite, NOT a precision matrix (the full matrix lives in `documentDate.spec.ts` + the fixtures guard). File extension is `*.spec.ts` (not `*.svelte.spec.ts`) — runs in the Vitest `server` project (Node environment). Mock pattern: `vi.mock('$lib/paraglide/runtime', () => ({ getLocale: vi.fn(() => 'de') }))`. Test cases (name as full sentences for self-documenting CI output): - `'returns localized DAY label containing the day number and month name in German'` — `timelineDateLabel('1914-07-28', 'DAY')` with `getLocale` → `'de'`, assert contains `28` and `Juli`. - `'returns localized DAY label in English when locale is en'` — same date with `getLocale` → `'en'`, assert contains `July`. - `'derives localized season word from anchor month when precision is SEASON (raw=null path)'` — `timelineDateLabel('1916-04-01', 'SEASON')` → contains `Frühling` (de) / `Spring` (en). - `'returns null for UNKNOWN precision with a date'` — `timelineDateLabel('1914-07-28', 'UNKNOWN')` → `null`. - `'returns null for UNKNOWN precision without a date'` — `timelineDateLabel(null, 'UNKNOWN')` → `null`. - `'returns null for non-UNKNOWN precision when eventDate is null'` — `timelineDateLabel(null, 'APPROX')` → `null` (façade guard short-circuits before `formatDocumentDate`). - `'returns null for non-UNKNOWN precision when eventDate is empty string'` — `timelineDateLabel('', 'DAY')` → `null` (empty string is falsy). - `'delegates same-year RANGE to shared formatter with concrete output'` — `timelineDateLabel('1914-01-01', 'RANGE', '1914-11-11')` → `toContain('1914')` and `toContain('Nov')` (or match exact shared-formatter output; use `formatDocumentDate` directly to compute expected value rather than a literal string). Single-file `*.spec.ts`, runs locally in seconds; full sweep left to CI.
marcel added this to the Zeitstrahl — Family Timeline milestone 2026-06-07 19:29:14 +02:00
marcel added the P2-mediumfeatureui labels 2026-06-07 19:30:02 +02:00
marcel changed title from Timeline: shared precision-aware date-label helper + API types to Timeline: shared precision-aware date-label helper (façade over formatDocumentDate) 2026-06-07 19:49:37 +02:00
Author
Owner

🏗️ Markus Keller — Senior Application Architect

Observations

The façade pattern is the correct architectural choice here. Verified in the codebase: formatDocumentDate already serves DocumentDate.svelte, DocumentTopBarTitle.svelte, and DocumentMultiSelect.svelte — adding a timeline façade that delegates to it keeps exactly one precision-rendering code path. This is the right boundary.

One structural point that deserves explicit attention: the issue correctly notes that DatePrecision is hand-declared in documentDate.ts and must not be migrated to the OpenAPI-generated type. But the drift-risk comment the issue requests adding to documentDate.ts should be specific enough to prevent a future developer from doing the "obvious cleanup" of importing the generated type. The comment in the code block shown (// mirrors the backend DatePrecision enum...) is good — make sure it lands in documentDate.ts before the type declaration, not after, so it's the first thing a reader sees.

On the vite.config.ts coverage include: I checked the current list. It covers src/lib/shared/utils/** and src/lib/document/** but has no entry for src/lib/timeline/**. The issue correctly identifies this as a required 1-line addition. This is not optional — without it, the new lib/timeline/ directory is outside the 80% branch gate and coverage can silently drop. The task list includes it; confirm it lands in the same commit as dateLabel.ts.

On the import extension .js vs no extension: The issue specifies '$lib/paraglide/runtime.js' with the .js extension. Verified in the codebase: DocumentDate.svelte and DocumentMultiSelect.svelte both use the .js extension; TimelineDensityFilter.svelte imports without .js. The .js extension is the dominant pattern and matches SvelteKit's ESM resolution. Use runtime.js.

On CLAUDE.md + C4 deferral: The decision to defer CLAUDE.md table and C4 diagram updates to issue 7 is correct. A single helper file does not constitute a "new domain module" in the sense of the doc-update trigger. Issue 7 (first route) is the right boundary. This must be called out in the issue 7 PR description — record it as a TODO there, not just as a note here.

Recommendations

  • Add the drift-risk comment to documentDate.ts above the DatePrecision type declaration (not below), so it's the first signal a reader encounters when they look at the type.
  • Ensure 'src/lib/timeline/**' is added to vite.config.ts coverage include in the same commit as the new module — never in a follow-up.
  • The PR description for issue 7 must explicitly call out "CLAUDE.md package table + C4 frontend diagram update for lib/timeline/" as a required task. Put it in the PR description checklist, not just in this issue body.

Open Decisions (none)

All architectural decisions on this issue are resolved and correctly documented in the "Decisions resolved" section.

## 🏗️ Markus Keller — Senior Application Architect ### Observations The façade pattern is the correct architectural choice here. Verified in the codebase: `formatDocumentDate` already serves `DocumentDate.svelte`, `DocumentTopBarTitle.svelte`, and `DocumentMultiSelect.svelte` — adding a timeline façade that delegates to it keeps exactly one precision-rendering code path. This is the right boundary. One structural point that deserves explicit attention: the issue correctly notes that `DatePrecision` is **hand-declared** in `documentDate.ts` and must not be migrated to the OpenAPI-generated type. But the drift-risk comment the issue requests adding to `documentDate.ts` should be specific enough to prevent a future developer from doing the "obvious cleanup" of importing the generated type. The comment in the code block shown (`// mirrors the backend DatePrecision enum...`) is good — make sure it lands in `documentDate.ts` *before* the type declaration, not after, so it's the first thing a reader sees. **On the `vite.config.ts` coverage include:** I checked the current list. It covers `src/lib/shared/utils/**` and `src/lib/document/**` but has no entry for `src/lib/timeline/**`. The issue correctly identifies this as a required 1-line addition. This is not optional — without it, the new `lib/timeline/` directory is outside the 80% branch gate and coverage can silently drop. The task list includes it; confirm it lands in the same commit as `dateLabel.ts`. **On the import extension `.js` vs no extension:** The issue specifies `'$lib/paraglide/runtime.js'` with the `.js` extension. Verified in the codebase: `DocumentDate.svelte` and `DocumentMultiSelect.svelte` both use the `.js` extension; `TimelineDensityFilter.svelte` imports without `.js`. The `.js` extension is the dominant pattern and matches SvelteKit's ESM resolution. Use `runtime.js`. **On CLAUDE.md + C4 deferral:** The decision to defer CLAUDE.md table and C4 diagram updates to issue 7 is correct. A single helper file does not constitute a "new domain module" in the sense of the doc-update trigger. Issue 7 (first route) is the right boundary. This must be called out in the issue 7 PR description — record it as a TODO there, not just as a note here. ### Recommendations - Add the drift-risk comment to `documentDate.ts` *above* the `DatePrecision` type declaration (not below), so it's the first signal a reader encounters when they look at the type. - Ensure `'src/lib/timeline/**'` is added to `vite.config.ts` coverage `include` in the same commit as the new module — never in a follow-up. - The PR description for issue 7 must explicitly call out "CLAUDE.md package table + C4 frontend diagram update for `lib/timeline/`" as a required task. Put it in the PR description checklist, not just in this issue body. ### Open Decisions _(none)_ All architectural decisions on this issue are resolved and correctly documented in the "Decisions resolved" section.
Author
Owner

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

I read formatDocumentDate in full. One behavioral subtlety that the test suite must capture explicitly: formatDocumentDate for UNKNOWN returns m.date_precision_unknown() — a localized string like "Datum unbekannt"not null. The façade intercepts before calling formatDocumentDate when precision === 'UNKNOWN' || !eventDate and returns null instead. This is intentional (per the spec: "façade returns null — no per-item label"), but means the façade and the shared formatter have different contracts for the UNKNOWN case.

The test named 'returns null for UNKNOWN precision with a date' covers this correctly. What's missing: a test that confirms the façade does not fall through to formatDocumentDate for UNKNOWN — i.e., that it doesn't return the localized "Datum unbekannt" string. The existing proposed test → null covers the return value, but an assertion like expect(result).not.toBe(m.date_precision_unknown(...)) would make the behavioral distinction from the shared formatter explicit. That said, toBe(null) already excludes the string — this is a documentation concern, not a coverage gap.

On the mock pattern: The issue specifies vi.mock('$lib/paraglide/runtime', () => ({ getLocale: vi.fn(() => 'de') })) — without .js. But the import in dateLabel.ts is '$lib/paraglide/runtime.js'. In Vitest, vi.mock must match the exact import specifier. If dateLabel.ts imports from '$lib/paraglide/runtime.js', the mock path must be '$lib/paraglide/runtime.js' too. Mismatch causes the mock to be silently skipped and getLocale() returns the real locale (which may or may not be 'de' in the Node test environment). This is a concrete risk — the tests for locale switching (the English/Spanish assertions) would pass vacuously if the mock silently misses.

On 'src/lib/timeline/**' in vite.config.ts coverage include: The server project currently covers src/lib/shared/utils/** and src/lib/document/**. dateLabel.ts will be at src/lib/timeline/dateLabel.ts. The 1-line addition to include is correct and necessary; without it, Vitest won't measure branch coverage for dateLabel.ts even if the tests run.

On import style: import type { DatePrecision } — type-only import is correct and required. Confirm this is a type-only import since DatePrecision is a union type string literal, not a runtime value.

Recommendations

  • Fix the mock path: use vi.mock('$lib/paraglide/runtime.js', ...) (with .js extension) to match the exact import specifier in dateLabel.ts. Verify by temporarily asserting getLocale is called and confirming the spy records the call.
  • The RANGE delegation test should use formatDocumentDate('1914-01-01', 'RANGE', '1914-11-11', null, 'de') as the expected value (dynamically computed), not a hardcoded string — the spec says this, and it's the right approach. Hardcoded strings will fail when the shared formatter's RANGE logic changes.
  • Sequence of commits: write the failing tests first (all 8), then add dateLabel.ts, then the drift-risk comment to documentDate.ts, then the coverage include line. Keep these as separate atomic commits.

Open Decisions (none)

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations I read `formatDocumentDate` in full. One behavioral subtlety that the test suite must capture explicitly: `formatDocumentDate` for `UNKNOWN` returns `m.date_precision_unknown()` — a localized string like `"Datum unbekannt"` — **not** `null`. The façade intercepts before calling `formatDocumentDate` when `precision === 'UNKNOWN' || !eventDate` and returns `null` instead. This is intentional (per the spec: "façade returns `null` — no per-item label"), but means the façade and the shared formatter have **different contracts for the UNKNOWN case**. The test named `'returns null for UNKNOWN precision with a date'` covers this correctly. What's missing: a test that confirms the façade does *not* fall through to `formatDocumentDate` for UNKNOWN — i.e., that it doesn't return the localized "Datum unbekannt" string. The existing proposed test `→ null` covers the return value, but an assertion like `expect(result).not.toBe(m.date_precision_unknown(...))` would make the behavioral distinction from the shared formatter explicit. That said, `toBe(null)` already excludes the string — this is a documentation concern, not a coverage gap. **On the mock pattern:** The issue specifies `vi.mock('$lib/paraglide/runtime', () => ({ getLocale: vi.fn(() => 'de') }))` — without `.js`. But the import in `dateLabel.ts` is `'$lib/paraglide/runtime.js'`. In Vitest, `vi.mock` must match the exact import specifier. If `dateLabel.ts` imports from `'$lib/paraglide/runtime.js'`, the mock path must be `'$lib/paraglide/runtime.js'` too. Mismatch causes the mock to be silently skipped and `getLocale()` returns the real locale (which may or may not be `'de'` in the Node test environment). This is a **concrete risk** — the tests for locale switching (the English/Spanish assertions) would pass vacuously if the mock silently misses. **On `'src/lib/timeline/**'` in vite.config.ts coverage include:** The `server` project currently covers `src/lib/shared/utils/**` and `src/lib/document/**`. `dateLabel.ts` will be at `src/lib/timeline/dateLabel.ts`. The 1-line addition to `include` is correct and necessary; without it, Vitest won't measure branch coverage for `dateLabel.ts` even if the tests run. **On import style:** `import type { DatePrecision }` — type-only import is correct and required. Confirm this is a type-only import since `DatePrecision` is a union type string literal, not a runtime value. ### Recommendations - Fix the mock path: use `vi.mock('$lib/paraglide/runtime.js', ...)` (with `.js` extension) to match the exact import specifier in `dateLabel.ts`. Verify by temporarily asserting `getLocale` is called and confirming the spy records the call. - The RANGE delegation test should use `formatDocumentDate('1914-01-01', 'RANGE', '1914-11-11', null, 'de')` as the expected value (dynamically computed), not a hardcoded string — the spec says this, and it's the right approach. Hardcoded strings will fail when the shared formatter's RANGE logic changes. - Sequence of commits: write the failing tests first (all 8), then add `dateLabel.ts`, then the drift-risk comment to `documentDate.ts`, then the coverage include line. Keep these as separate atomic commits. ### Open Decisions _(none)_
Author
Owner

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

The raw=null decision (resolved Decision 6) is the security-correct call. I verified in formatDocumentDate that the raw parameter is only used in the SEASON branch via seasonFromRaw(raw) — it extracts the first token, lowercases it, and looks it up in a fixed Record<string, Season>. The function returns null for any non-matching token. It is not reflected into HTML and is not displayed verbatim. So even if raw were passed through from the timeline, the actual attack surface would be minimal — but passing raw=null explicitly is still the right design because:

  1. The timeline has no verbatim cell to pass. Passing null explicitly documents that this is intentional, not an oversight.
  2. It eliminates any future path where a timeline event's description or title could accidentally be passed as raw during a refactor.

The JSDoc comment the spec prescribes (// raw is always null for timeline events — no verbatim spreadsheet cell) is the correct self-documenting approach. This is exactly the kind of security comment I want to see: it explains why, not what.

On the {@html} note in the issue body (Notes for cards 7–9): The issue correctly notes that EventCard.svelte and LetterCard.svelte should not use {@html} for title/description/letter title. This is worth making an explicit acceptance criterion on issue 7, not just a planning note. Svelte auto-escapes {value} interpolations — as long as no {@html} is added, XSS from event titles is structurally impossible. But "confirm it's not there" should be a checklist item on the card-component issues.

On the getLocale() call being inside the façade: The locale is called at render time, not at module import time, which is correct. A module-level const locale = getLocale() would capture the locale once at initialization and be stale for users who switch languages. Calling getLocale() inside the function body ensures the current locale is used on each call. No concern here.

On input validation: eventDate comes through the façade typed as string | null | undefined. The function guards on !eventDate (which catches null, undefined, and ''). This is sufficient. No additional sanitization is needed since the string goes to formatDocumentDate, which passes it to Intl.DateTimeFormat via new Date(iso + 'T12:00:00') — an unparseable date produces Invalid Date, which Intl.DateTimeFormat.format() renders as "Invalid Date", not a script injection point.

Recommendations

  • The raw=null security comment in dateLabel.ts must be present from the first commit (not added in a follow-up). It documents the deliberate security decision.
  • Add to issue 7's task list: "Confirm EventCard.svelte and LetterCard.svelte use {value} interpolation, not {@html}, for all user-controlled string fields." One line prevents a class of future vulnerabilities.
  • The existing documentDate.spec.ts security test ('ignores a malicious raw value for the structured label') covers the shared formatter. The timeline façade's spec does not need a dedicated security test because the façade always passes raw=null — there is no path to reach the vulnerable parameter. This is correct scope-bounding.

Open Decisions (none)

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations The `raw=null` decision (resolved Decision 6) is the security-correct call. I verified in `formatDocumentDate` that the `raw` parameter is only used in the `SEASON` branch via `seasonFromRaw(raw)` — it extracts the first token, lowercases it, and looks it up in a fixed `Record<string, Season>`. The function returns `null` for any non-matching token. It is **not** reflected into HTML and is **not** displayed verbatim. So even if `raw` were passed through from the timeline, the actual attack surface would be minimal — but passing `raw=null` explicitly is still the right design because: 1. The timeline has no verbatim cell to pass. Passing `null` explicitly documents that this is intentional, not an oversight. 2. It eliminates any future path where a timeline event's `description` or `title` could accidentally be passed as `raw` during a refactor. The JSDoc comment the spec prescribes (`// raw is always null for timeline events — no verbatim spreadsheet cell`) is the correct self-documenting approach. This is exactly the kind of security comment I want to see: it explains *why*, not *what*. **On the `{@html}` note in the issue body (Notes for cards 7–9):** The issue correctly notes that `EventCard.svelte` and `LetterCard.svelte` should not use `{@html}` for title/description/letter title. This is worth making an explicit acceptance criterion on issue 7, not just a planning note. Svelte auto-escapes `{value}` interpolations — as long as no `{@html}` is added, XSS from event titles is structurally impossible. But "confirm it's not there" should be a checklist item on the card-component issues. **On the `getLocale()` call being inside the façade:** The locale is called at render time, not at module import time, which is correct. A module-level `const locale = getLocale()` would capture the locale once at initialization and be stale for users who switch languages. Calling `getLocale()` inside the function body ensures the current locale is used on each call. No concern here. **On input validation:** `eventDate` comes through the façade typed as `string | null | undefined`. The function guards on `!eventDate` (which catches `null`, `undefined`, and `''`). This is sufficient. No additional sanitization is needed since the string goes to `formatDocumentDate`, which passes it to `Intl.DateTimeFormat` via `new Date(iso + 'T12:00:00')` — an unparseable date produces `Invalid Date`, which `Intl.DateTimeFormat.format()` renders as "Invalid Date", not a script injection point. ### Recommendations - The `raw=null` security comment in `dateLabel.ts` must be present from the first commit (not added in a follow-up). It documents the deliberate security decision. - Add to issue 7's task list: "Confirm `EventCard.svelte` and `LetterCard.svelte` use `{value}` interpolation, not `{@html}`, for all user-controlled string fields." One line prevents a class of future vulnerabilities. - The existing `documentDate.spec.ts` security test (`'ignores a malicious raw value for the structured label'`) covers the shared formatter. The timeline façade's spec does not need a dedicated security test because the façade always passes `raw=null` — there is no path to reach the vulnerable parameter. This is correct scope-bounding. ### Open Decisions _(none)_
Author
Owner

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

The test specification in the issue is the most detailed I've seen on this milestone so far. Eight named test cases, explicitly designated as a delegation/integration suite (not a precision matrix), with the right environment annotation (*.spec.tsserver project). This is solid.

Critical: mock path must match import specifier. Felix flagged this from the implementation angle; I'm flagging it from the test-reliability angle. If vi.mock('$lib/paraglide/runtime', ...) (no .js) is used while the source imports from '$lib/paraglide/runtime.js', Vitest's module mock does not intercept the call. The getLocale spy never fires. The test for English locale ('returns localized DAY label in English when locale is en') would then pass only if the test environment happens to return 'en' as its locale — which it won't (Node's default is typically 'en' but Paraglide's getLocale() in a Node test environment returns whatever the runtime has initialized, which may not be 'de'). This is a silent false-positive risk — the test passes for the wrong reason. The mock path must be '$lib/paraglide/runtime.js'.

On coverage gate applicability: The server project's coverage config (in vite.config.ts) runs on src/lib/shared/utils/**, src/lib/shared/server/**, src/lib/shared/discussion/**, src/lib/document/**, and src/hooks.server.ts. The new src/lib/timeline/dateLabel.ts will be at a path outside all current include entries. Without adding 'src/lib/timeline/**', the 8 proposed tests will run and pass, but dateLabel.ts will never appear in the coverage report — the 80% branch gate gives false assurance. This is specifically called out in the tasks, but I want to highlight it as a pre-commit checklist item: run npm run test:coverage after adding the include line and confirm dateLabel.ts appears in the report with ≥80% branch coverage.

On the RANGE delegation test: The spec says use formatDocumentDate directly to compute expected value rather than a literal string. This is the right approach, and I'd strengthen it slightly: the computed expected value should be captured in a const expected = formatDocumentDate(...) at the top of the test body, then asserted against timelineDateLabel(...). This makes the test self-documenting (the two calls sit side by side, showing the equivalence intent) and eliminates hardcoded literals that rot.

On the "SEASON derives from anchor month when raw=null" test: The test for '1916-04-01' expects Frühling (spring). This matches seasonOfMonth(4) in documentDate.ts which returns 'spring'. Verified correct. But the test should also implicitly verify that raw was not passed through — since the façade always passes raw=null, the season word must come from the anchor month. The test is correct by construction (passing null raw), but a descriptive comment in the test body would make this intent clear for reviewers.

On empty-string guard: The test 'returns null for non-UNKNOWN precision when eventDate is empty string' is valuable and often missed. '' is falsy in JavaScript, so !eventDate catches it. Confirmed correct behavior.

Missing edge case (minor): No test for eventDateEnd being undefined vs null for RANGE. The JSDoc notes they're equivalent, and the implementation uses eventDateEnd ?? null — but a test confirming timelineDateLabel('1914-01-01', 'RANGE', undefined) behaves identically to timelineDateLabel('1914-01-01', 'RANGE', null) would be a clean regression guard for the ?? operator.

Recommendations

  • Before any commit: fix the mock path to '$lib/paraglide/runtime.js'. Add a spy assertion (expect(getLocale).toHaveBeenCalled()) in at least one locale-switching test to confirm the mock is actually intercepted.
  • Post-coverage-add: Run npm run test:coverage (server project) and confirm src/lib/timeline/dateLabel.ts appears in the report.
  • Add one more test: 'treats undefined eventDateEnd as null for RANGE' — passes undefined as the third arg and asserts the output equals the null case.

Open Decisions (none)

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations The test specification in the issue is the most detailed I've seen on this milestone so far. Eight named test cases, explicitly designated as a delegation/integration suite (not a precision matrix), with the right environment annotation (`*.spec.ts` → `server` project). This is solid. **Critical: mock path must match import specifier.** Felix flagged this from the implementation angle; I'm flagging it from the test-reliability angle. If `vi.mock('$lib/paraglide/runtime', ...)` (no `.js`) is used while the source imports from `'$lib/paraglide/runtime.js'`, Vitest's module mock does not intercept the call. The `getLocale` spy never fires. The test for English locale (`'returns localized DAY label in English when locale is en'`) would then pass only if the test environment happens to return `'en'` as its locale — which it won't (Node's default is typically `'en'` but Paraglide's `getLocale()` in a Node test environment returns whatever the runtime has initialized, which may not be `'de'`). This is a **silent false-positive risk** — the test passes for the wrong reason. The mock path must be `'$lib/paraglide/runtime.js'`. **On coverage gate applicability:** The `server` project's coverage config (in `vite.config.ts`) runs on `src/lib/shared/utils/**`, `src/lib/shared/server/**`, `src/lib/shared/discussion/**`, `src/lib/document/**`, and `src/hooks.server.ts`. The new `src/lib/timeline/dateLabel.ts` will be at a path outside all current `include` entries. Without adding `'src/lib/timeline/**'`, the 8 proposed tests will run and pass, but `dateLabel.ts` will never appear in the coverage report — the 80% branch gate gives false assurance. This is specifically called out in the tasks, but I want to highlight it as a **pre-commit checklist item**: run `npm run test:coverage` after adding the include line and confirm `dateLabel.ts` appears in the report with ≥80% branch coverage. **On the RANGE delegation test:** The spec says `use formatDocumentDate directly to compute expected value rather than a literal string`. This is the right approach, and I'd strengthen it slightly: the computed expected value should be captured in a `const expected = formatDocumentDate(...)` at the top of the test body, then asserted against `timelineDateLabel(...)`. This makes the test self-documenting (the two calls sit side by side, showing the equivalence intent) and eliminates hardcoded literals that rot. **On the "SEASON derives from anchor month when raw=null" test:** The test for `'1916-04-01'` expects `Frühling` (spring). This matches `seasonOfMonth(4)` in `documentDate.ts` which returns `'spring'`. Verified correct. But the test should also implicitly verify that `raw` was *not* passed through — since the façade always passes `raw=null`, the season word must come from the anchor month. The test is correct by construction (passing `null` raw), but a descriptive comment in the test body would make this intent clear for reviewers. **On empty-string guard:** The test `'returns null for non-UNKNOWN precision when eventDate is empty string'` is valuable and often missed. `''` is falsy in JavaScript, so `!eventDate` catches it. Confirmed correct behavior. **Missing edge case (minor):** No test for `eventDateEnd` being `undefined` vs `null` for `RANGE`. The JSDoc notes they're equivalent, and the implementation uses `eventDateEnd ?? null` — but a test confirming `timelineDateLabel('1914-01-01', 'RANGE', undefined)` behaves identically to `timelineDateLabel('1914-01-01', 'RANGE', null)` would be a clean regression guard for the `??` operator. ### Recommendations - **Before any commit:** fix the mock path to `'$lib/paraglide/runtime.js'`. Add a spy assertion (`expect(getLocale).toHaveBeenCalled()`) in at least one locale-switching test to confirm the mock is actually intercepted. - **Post-coverage-add:** Run `npm run test:coverage` (server project) and confirm `src/lib/timeline/dateLabel.ts` appears in the report. - **Add one more test:** `'treats undefined eventDateEnd as null for RANGE'` — passes `undefined` as the third arg and asserts the output equals the `null` case. ### Open Decisions _(none)_
Author
Owner

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

This issue is purely infrastructure — a date-label helper with no UI surface of its own. My review therefore focuses on the downstream contracts this helper establishes for the card issues (7–9) that will render its output.

The <time datetime> note is exactly right and critical. The spec notes: "The date label string returned by timelineDateLabel should be rendered inside a <time datetime="{eventDate}"> element." This is not optional for accessibility:

  • Screen readers that support <time> (e.g., NVDA + Chrome) can expose the machine-readable datetime attribute alongside the human label.
  • The datetime attribute must be a valid date/time string. For DAY precision, eventDate (ISO YYYY-MM-DD) is a valid value. For YEAR precision, 1914 is a valid year value. For RANGE, the datetime should be the start date (end can go in a separate attribute or omitted).
  • For APPROX precision, the datetime should be eventDate (the ISO anchor), even though the label says "ca. 1914" — the machine-readable value is more precise than the label, which is fine (the label is the honest human representation, the datetime is the best-known anchor).
  • For SEASON, datetime="1916-04" (month precision) is arguably more honest than the full 1916-04-01 anchor, but YYYY-MM-DD is also valid and simpler to implement. Either is acceptable; pick one and document it in the card spec.

The <abbr title="circa">ca.</abbr> note: This is a good accessibility affordance for screen reader users who may not know "ca." means "circa". However, <abbr> is inconsistently supported across screen readers — some announce the title, many do not. A more reliable approach is to use aria-label on the <time> element: <time datetime="1914" aria-label="circa 1914">ca. 1914</time>. This ensures the accessible name is "circa 1914" regardless of screen reader support for <abbr>. Flag this as a card-level decision for issue 7.

"Ohne Datum" copy: The spec correctly separates the "Ohne Datum" bucket header from the per-item date label. The bucket header copy is a UX concern for YearBand/TimelineView — but I'll note now that "Ohne Datum" is appropriate German for the family archive context. The English equivalent in date_precision_unknown is presumably "Date unknown" (or "Undated") — confirm both keys are present and natural-sounding in messages/{en,es}.json before issue 7 ships.

On precision rendering parity: The spec table in the issue body matches the spec file exactly for the German forms. The RANGE label (28. Juli 1914 – 11. Nov. 1918) uses an en-dash (–), which is typographically correct. Confirm the existing rangeLabel function in documentDate.ts uses an en-dash and not a hyphen-minus — verified: line 100 shows formatMCDate(iso, locale)} – ${formatMCDate(end, locale) with an en-dash. Good.

Recommendations

  • Capture the <time datetime> rendering contract explicitly in issue 7's task list, including the value to use for each precision: DAYYYYY-MM-DD, MONTHYYYY-MM, YEARYYYY, RANGE→start date, APPROXYYYY-MM-DD anchor, SEASONYYYY-MM anchor month.
  • For APPROX, prefer <time datetime="1914" aria-label="circa 1914">ca. 1914</time> over <abbr> for better screen reader coverage.
  • Confirm date_precision_unknown key produces natural copy in all three locales (de/en/es) before issue 7. "Date unknown" (en) and "Fecha desconocida" (es) are reasonable — check the actual values in messages/en.json and messages/es.json.

Open Decisions (none)

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations This issue is purely infrastructure — a date-label helper with no UI surface of its own. My review therefore focuses on the downstream contracts this helper establishes for the card issues (7–9) that will render its output. **The `<time datetime>` note is exactly right and critical.** The spec notes: "The date label string returned by `timelineDateLabel` should be rendered inside a `<time datetime="{eventDate}">` element." This is not optional for accessibility: - Screen readers that support `<time>` (e.g., NVDA + Chrome) can expose the machine-readable `datetime` attribute alongside the human label. - The `datetime` attribute must be a valid date/time string. For `DAY` precision, `eventDate` (ISO `YYYY-MM-DD`) is a valid value. For `YEAR` precision, `1914` is a valid year value. For `RANGE`, the `datetime` should be the start date (end can go in a separate attribute or omitted). - For `APPROX` precision, the `datetime` should be `eventDate` (the ISO anchor), even though the label says "ca. 1914" — the machine-readable value is more precise than the label, which is fine (the label is the honest human representation, the datetime is the best-known anchor). - For `SEASON`, `datetime="1916-04"` (month precision) is arguably more honest than the full `1916-04-01` anchor, but `YYYY-MM-DD` is also valid and simpler to implement. Either is acceptable; pick one and document it in the card spec. **The `<abbr title="circa">ca.</abbr>` note:** This is a good accessibility affordance for screen reader users who may not know "ca." means "circa". However, `<abbr>` is inconsistently supported across screen readers — some announce the title, many do not. A more reliable approach is to use `aria-label` on the `<time>` element: `<time datetime="1914" aria-label="circa 1914">ca. 1914</time>`. This ensures the accessible name is "circa 1914" regardless of screen reader support for `<abbr>`. Flag this as a card-level decision for issue 7. **"Ohne Datum" copy:** The spec correctly separates the "Ohne Datum" bucket header from the per-item date label. The bucket header copy is a UX concern for `YearBand`/`TimelineView` — but I'll note now that "Ohne Datum" is appropriate German for the family archive context. The English equivalent in `date_precision_unknown` is presumably "Date unknown" (or "Undated") — confirm both keys are present and natural-sounding in `messages/{en,es}.json` before issue 7 ships. **On precision rendering parity:** The spec table in the issue body matches the spec file exactly for the German forms. The RANGE label (`28. Juli 1914 – 11. Nov. 1918`) uses an en-dash (–), which is typographically correct. Confirm the existing `rangeLabel` function in `documentDate.ts` uses an en-dash and not a hyphen-minus — verified: line 100 shows `formatMCDate(iso, locale)} – ${formatMCDate(end, locale)` with an en-dash. Good. ### Recommendations - Capture the `<time datetime>` rendering contract explicitly in issue 7's task list, including the value to use for each precision: `DAY`→`YYYY-MM-DD`, `MONTH`→`YYYY-MM`, `YEAR`→`YYYY`, `RANGE`→start date, `APPROX`→`YYYY-MM-DD` anchor, `SEASON`→`YYYY-MM` anchor month. - For `APPROX`, prefer `<time datetime="1914" aria-label="circa 1914">ca. 1914</time>` over `<abbr>` for better screen reader coverage. - Confirm `date_precision_unknown` key produces natural copy in all three locales (de/en/es) before issue 7. "Date unknown" (en) and "Fecha desconocida" (es) are reasonable — check the actual values in `messages/en.json` and `messages/es.json`. ### Open Decisions _(none)_
Author
Owner

📋 Elicit — Requirements Engineer

Observations

The issue body is implementation-spec-level and very well structured. The scope is tightly bounded: one helper file, one drift-risk comment, one coverage include line, eight tests. There is no ambiguity in what "done" looks like.

One specification gap worth noting: The acceptance criteria say "No precision/rendering logic is duplicated outside documentDate.ts" — this is verifiable by inspection. But there is no criterion covering what happens when a future enum value is added to DatePrecision on the backend that the frontend hasn't seen yet. The switch in formatDocumentDate has a default case that returns m.date_precision_unknown(). The façade's guard returns null for UNKNOWN and then delegates for all other values — including any unknown future value. This means a new backend precision value (e.g., DECADE) would silently fall through to formatDocumentDate's default branch and render as "Datum unbekannt". This is arguably correct (fail-safe), but it should be explicit in the drift-risk comment so future developers know this is the intended behavior.

On the "Decisions resolved" section: All 8 resolved decisions are well-documented with the rationale and the personas who confirmed them. This is the correct pattern for issue bodies that have gone through multiple review rounds. No open questions remain.

On the breakdown of dependencies: This issue (issue 6 in the milestone) depends on nothing from the backend (issues 2–5 unmerged). It is correctly scheduled as a standalone frontend piece. The codegen deferral (Decision 2) is the right call — running npm run generate:api now would be a no-op and create confusion.

One missing acceptance criterion: The issue's AC section does not include: "The drift-risk comment is present in documentDate.ts and accurately describes the migration decision point." The drift-risk comment is in the task list but not in the ACs. Since ACs are what CI and reviewers check, add it there.

Recommendations

  • Add to Acceptance Criteria: "A drift-risk comment is present in documentDate.ts above the DatePrecision type declaration, explaining that this type must be updated manually with the Java enum and must not be migrated to the OpenAPI-generated type."
  • Add to the drift-risk comment text: note the fallback behavior for unknown future precision values (falls through to formatDocumentDate's default → "Datum unbekannt"). This is the expected fail-safe behavior — document it.
  • The issue is otherwise implementation-ready. No ambiguities remain that would block starting work.

Open Decisions (none)

## 📋 Elicit — Requirements Engineer ### Observations The issue body is implementation-spec-level and very well structured. The scope is tightly bounded: one helper file, one drift-risk comment, one coverage include line, eight tests. There is no ambiguity in what "done" looks like. **One specification gap worth noting:** The acceptance criteria say "No precision/rendering logic is duplicated outside `documentDate.ts`" — this is verifiable by inspection. But there is no criterion covering what happens when a *future* enum value is added to `DatePrecision` on the backend that the frontend hasn't seen yet. The `switch` in `formatDocumentDate` has a `default` case that returns `m.date_precision_unknown()`. The façade's guard returns `null` for `UNKNOWN` and then delegates for all other values — including any unknown future value. This means a new backend precision value (e.g., `DECADE`) would silently fall through to `formatDocumentDate`'s `default` branch and render as "Datum unbekannt". This is arguably correct (fail-safe), but it should be explicit in the drift-risk comment so future developers know this is the intended behavior. **On the "Decisions resolved" section:** All 8 resolved decisions are well-documented with the rationale and the personas who confirmed them. This is the correct pattern for issue bodies that have gone through multiple review rounds. No open questions remain. **On the breakdown of dependencies:** This issue (issue 6 in the milestone) depends on nothing from the backend (issues 2–5 unmerged). It is correctly scheduled as a standalone frontend piece. The codegen deferral (Decision 2) is the right call — running `npm run generate:api` now would be a no-op and create confusion. **One missing acceptance criterion:** The issue's AC section does not include: "The drift-risk comment is present in `documentDate.ts` and accurately describes the migration decision point." The drift-risk comment is in the task list but not in the ACs. Since ACs are what CI and reviewers check, add it there. ### Recommendations - Add to Acceptance Criteria: "A drift-risk comment is present in `documentDate.ts` above the `DatePrecision` type declaration, explaining that this type must be updated manually with the Java enum and must not be migrated to the OpenAPI-generated type." - Add to the drift-risk comment text: note the fallback behavior for unknown future precision values (falls through to `formatDocumentDate`'s `default` → "Datum unbekannt"). This is the expected fail-safe behavior — document it. - The issue is otherwise implementation-ready. No ambiguities remain that would block starting work. ### Open Decisions _(none)_
Author
Owner

🖥️ Tobias Wendt — DevOps & Platform Engineer

No concerns from my angle. This issue creates a single TypeScript helper file and a spec — no new Docker services, no infrastructure changes, no environment variables, no CI workflow changes. The coverage include addition to vite.config.ts is a build-tool config touch, not an infrastructure one.

One thing I verified: the server project in vite.config.ts runs in Node environment and excludes *.svelte.spec.ts. The proposed dateLabel.spec.ts (without .svelte.) will correctly run in the server project (Node), not the browser project. The mock pattern vi.mock('$lib/paraglide/runtime.js', ...) will work in Node without browser setup — getLocale() doesn't need a DOM.

The only CI-adjacent observation: the npm run test:coverage command currently runs the server project coverage. After adding 'src/lib/timeline/**' to the include list, that project's 80% branch gate will now include dateLabel.ts. With 8 tests covering all 7 precision values plus null/empty guards, branch coverage on dateLabel.ts should comfortably exceed 80%. No risk to the gate from this addition.

## 🖥️ Tobias Wendt — DevOps & Platform Engineer No concerns from my angle. This issue creates a single TypeScript helper file and a spec — no new Docker services, no infrastructure changes, no environment variables, no CI workflow changes. The coverage include addition to `vite.config.ts` is a build-tool config touch, not an infrastructure one. One thing I verified: the `server` project in `vite.config.ts` runs in Node environment and excludes `*.svelte.spec.ts`. The proposed `dateLabel.spec.ts` (without `.svelte.`) will correctly run in the `server` project (Node), not the browser project. The mock pattern `vi.mock('$lib/paraglide/runtime.js', ...)` will work in Node without browser setup — `getLocale()` doesn't need a DOM. The only CI-adjacent observation: the `npm run test:coverage` command currently runs the `server` project coverage. After adding `'src/lib/timeline/**'` to the include list, that project's 80% branch gate will now include `dateLabel.ts`. With 8 tests covering all 7 precision values plus null/empty guards, branch coverage on `dateLabel.ts` should comfortably exceed 80%. No risk to the gate from this addition.
Sign in to join this conversation.
No Label P2-medium feature ui
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#778