feat: unify /notifications and dashboard activity feed into a /chronik page #285

Closed
opened 2026-04-20 11:02:12 +02:00 by marcel · 10 comments
Owner

Motivation

The Bell icon and /notifications page today only surface personal mentions/replies and waste a lot of horizontal space. In parallel, the dashboard now shows an ambient activity feed (uploads, transcription, annotations, comments, mentions) whose "Alle anzeigen" link currently points to /documents — wrong.

We want one coherent page that:

  • merges both streams into a single timeline
  • keeps personal mentions visually loud so they never drown in ambient noise
  • uses the empty vertical space on desktop with a richer layout
  • fixes the broken /documents link on the dashboard activity feed

Design spec

docs/specs/chronik-spec.html (committed on main, 2 043 lines, 11 states × 3 viewports, light + dark, full impl-ref tables and i18n keys).

View locally:

xdg-open docs/specs/chronik-spec.html

Key design decisions (locked via interview)

Decision Choice
Route / title /chronik · "Chronik" (301 redirect from /notifications)
Layout Stacked single column, max-w-3xl, mobile-first
Mental model Unified timeline (model C) — mentions pinned in a Für dich box above, rest in chronological timeline
Activity kinds shown 6 of 8 — adds BLOCK_REVIEWED; excludes STATUS_CHANGED and METADATA_UPDATED
Rollup Same actor + doc + kind within 2 h → count badge ("20 Blöcke"). Rolled up for TEXT_SAVED / ANNOTATION_CREATED / BLOCK_REVIEWED / FILE_UPLOADED; never for COMMENT_ADDED / MENTION_CREATED
Time grouping Day headers: Heute / Gestern / Diese Woche / Älter
Row density Comfortable (~72 px), rich preview inline for comments + mentions only
Filter pills 5 radio pills: Alle · Für dich · Hochgeladen · Transkription · Kommentare
"Für dich" behavior Shows unread only. After read → row migrates into timeline with for-you lane (accent border, @ marker, tinted bg). History reachable via "Für dich" filter pill or empty-state link
Bell icon Unchanged. Count = unread mentions/replies. Dropdown content stays; footer link retargets to /chronik with label "Zur Chronik →"

Scope / implementation plan (high level)

Backend

  • Add kinds query param to GET /api/dashboard/activity (CSV of AuditKind, default = 6 kinds).
  • Raise server cap on that endpoint from 20 → 40.
  • Implement rollup at repository level in AuditLogQueryRepository.findActivityFeed: group consecutive rows by (actor_id, document_id, kind) within a 120-min window.
  • Extend ActivityFeedRow / ActivityFeedItemDTO with count (int) and happenedAtUntil (nullable OffsetDateTime) — singletons: count=1, happenedAtUntil=null.
  • Regenerate OpenAPI types for the frontend.

Frontend

  • Route: src/routes/chronik/+page.svelte + +page.server.ts.
  • Components (each = one visible region):
    • ChronikFuerDichBox.svelte — unread mentions card with empty-state variant
    • ChronikFilterPills.svelte — 5 pills, role="radiogroup", URL-synced via ?filter=
    • ChronikTimeline.svelte — day-grouped list
    • ChronikRow.svelte — 4 row types (simple / rollup / comment / for-you)
    • ChronikEmptyState.svelte — first-run / filter-empty / inbox-zero
    • ChronikErrorCard.svelte — warning card with retry
  • hooks.server.ts: 301 redirect /notifications/chronik.
  • NotificationDropdown.svelte: footer link → /chronik, relabel to "Zur Chronik".
  • DashboardActivityFeed.svelte: fix broken link — change href="/documents"href="/chronik".

i18n

  • New Paraglide keys (de / en / es) — full list in spec §7.5.

Testing

  • axe-playwright on /chronik in both light AND dark mode.
  • Visual regression snapshots at 320 / 768 / 1440 px for Default + Inbox-Zero + Für-dich filter states.
  • Unit tests for rollup grouping (backend) and for timeline rendering with rollup DTOs (frontend).

Acceptance

  • /chronik renders all 11 content states specified in §02
  • Dark-mode parity verified by axe-core at all three viewports
  • Rollup observable: saving ≥ 2 transcription blocks in one doc within 2 h shows as a single rollup row with count badge and time range
  • "Für dich" box empties to reassurance card after marking unreads read; filter pill surfaces full history
  • /notifications redirects to /chronik (301)
  • Dashboard activity feed's "Alle anzeigen" now correctly goes to /chronik
  • Bell dropdown footer label updated to "Zur Chronik →"

References

## Motivation The Bell icon and `/notifications` page today only surface personal mentions/replies and waste a lot of horizontal space. In parallel, the dashboard now shows an ambient **activity feed** (uploads, transcription, annotations, comments, mentions) whose "Alle anzeigen" link currently points to `/documents` — wrong. We want one coherent page that: - merges both streams into a single timeline - keeps personal mentions visually loud so they never drown in ambient noise - uses the empty vertical space on desktop with a richer layout - fixes the broken `/documents` link on the dashboard activity feed ## Design spec **`docs/specs/chronik-spec.html`** (committed on main, 2 043 lines, 11 states × 3 viewports, light + dark, full `impl-ref` tables and i18n keys). View locally: ``` xdg-open docs/specs/chronik-spec.html ``` ## Key design decisions (locked via interview) | Decision | Choice | |---|---| | Route / title | `/chronik` · "Chronik" (301 redirect from `/notifications`) | | Layout | Stacked single column, `max-w-3xl`, mobile-first | | Mental model | Unified timeline (model C) — mentions pinned in a **Für dich** box above, rest in chronological timeline | | Activity kinds shown | 6 of 8 — adds `BLOCK_REVIEWED`; excludes `STATUS_CHANGED` and `METADATA_UPDATED` | | Rollup | Same actor + doc + kind within 2 h → count badge ("20 Blöcke"). Rolled up for TEXT_SAVED / ANNOTATION_CREATED / BLOCK_REVIEWED / FILE_UPLOADED; never for COMMENT_ADDED / MENTION_CREATED | | Time grouping | Day headers: Heute / Gestern / Diese Woche / Älter | | Row density | Comfortable (~72 px), rich preview inline for comments + mentions only | | Filter pills | 5 radio pills: Alle · Für dich · Hochgeladen · Transkription · Kommentare | | "Für dich" behavior | Shows **unread only**. After read → row migrates into timeline with for-you lane (accent border, @ marker, tinted bg). History reachable via "Für dich" filter pill or empty-state link | | Bell icon | **Unchanged.** Count = unread mentions/replies. Dropdown content stays; footer link retargets to `/chronik` with label "Zur Chronik →" | ## Scope / implementation plan (high level) ### Backend - Add `kinds` query param to `GET /api/dashboard/activity` (CSV of `AuditKind`, default = 6 kinds). - Raise server cap on that endpoint from 20 → 40. - Implement rollup at repository level in `AuditLogQueryRepository.findActivityFeed`: group consecutive rows by (actor_id, document_id, kind) within a 120-min window. - Extend `ActivityFeedRow` / `ActivityFeedItemDTO` with `count` (int) and `happenedAtUntil` (nullable OffsetDateTime) — singletons: `count=1`, `happenedAtUntil=null`. - Regenerate OpenAPI types for the frontend. ### Frontend - Route: `src/routes/chronik/+page.svelte` + `+page.server.ts`. - Components (each = one visible region): - `ChronikFuerDichBox.svelte` — unread mentions card with empty-state variant - `ChronikFilterPills.svelte` — 5 pills, `role="radiogroup"`, URL-synced via `?filter=` - `ChronikTimeline.svelte` — day-grouped list - `ChronikRow.svelte` — 4 row types (simple / rollup / comment / for-you) - `ChronikEmptyState.svelte` — first-run / filter-empty / inbox-zero - `ChronikErrorCard.svelte` — warning card with retry - `hooks.server.ts`: 301 redirect `/notifications` → `/chronik`. - `NotificationDropdown.svelte`: footer link → `/chronik`, relabel to "Zur Chronik". - `DashboardActivityFeed.svelte`: fix broken link — change `href="/documents"` → `href="/chronik"`. ### i18n - New Paraglide keys (de / en / es) — full list in spec §7.5. ### Testing - axe-playwright on `/chronik` in **both** light AND dark mode. - Visual regression snapshots at 320 / 768 / 1440 px for Default + Inbox-Zero + Für-dich filter states. - Unit tests for rollup grouping (backend) and for timeline rendering with rollup DTOs (frontend). ## Acceptance - [ ] `/chronik` renders all 11 content states specified in §02 - [ ] Dark-mode parity verified by axe-core at all three viewports - [ ] Rollup observable: saving ≥ 2 transcription blocks in one doc within 2 h shows as a single rollup row with count badge and time range - [ ] "Für dich" box empties to reassurance card after marking unreads read; filter pill surfaces full history - [ ] `/notifications` redirects to `/chronik` (301) - [ ] Dashboard activity feed's "Alle anzeigen" now correctly goes to `/chronik` - [ ] Bell dropdown footer label updated to "Zur Chronik →" ## References - Design spec: [`docs/specs/chronik-spec.html`](../src/branch/main/docs/specs/chronik-spec.html) — commit 5f30807e - Replaces page: [`src/routes/notifications/+page.svelte`](../src/branch/main/frontend/src/routes/notifications/+page.svelte) - Related component: [`src/lib/components/DashboardActivityFeed.svelte`](../src/branch/main/frontend/src/lib/components/DashboardActivityFeed.svelte) (broken link fix) - Related model: [`AuditKind.java`](../src/branch/main/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java)
marcel added the collaborationfeaturenotificationui labels 2026-04-20 11:02:20 +02:00
Author
Owner

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

  • Repository method is AuditLogQueryRepository.findDedupedActivityFeed (L25), not findActivityFeed as the issue says. Service wrapper AuditLogQueryService.findActivityFeed() calls it.
  • The existing query already dedupes by date_trunc('hour', ...) — a different mechanism than the proposed 2 h rollup. Two aggregation strategies side by side would be confusing.
  • ActivityFeedItemDTO is a Java record; extending it means updating the call site at DashboardService.java:133-140 + OpenAPI regen.
  • Notification is a persisted entity, distinct from audit_log. Für-dich (notifications table) and Timeline (audit_log) legitimately need two queries — not a duplication to refactor.
  • accessibility.spec.ts already parameterizes over AUTHENTICATED_PAGES × 3 modes (light, system dark, manual dark). Adding /chronik to that array is a one-line win.
  • hooks.server.ts uses sequence(). A redirect there would compete with the simpler +page.server.ts approach.

Recommendations

  • Rename the repo method in the scope: the target is findDedupedActivityFeed. Since it will no longer dedupe but roll up, rename to findRolledUpActivityFeed along with the change.
  • Replace the dedupe, don't stack: new query groups by (actor_id, document_id, kind, floor(epoch/7200)) for 2 h buckets. Keeping DISTINCT ON leaves zombie logic.
  • Redirect at the route, not in hooks: src/routes/notifications/+page.server.ts with throw redirect(301, '/chronik'). Colocated, no hook ordering concerns.
  • Avoid nested-link anti-pattern (spec §5.4): outer <a href="/documents/{id}"> wraps the row; the document title is a styled <span>, not an inner <a> with pointer-events:none. Valid HTML + same UX.
  • Keep ChronikRow.svelte as one file with a $derived variant discriminator unless it crosses 60 lines. 4 variants = one orchestrator, not 4 components.
  • Namespace under $lib/components/chronik/ — 7 components in a shared folder is noise.
  • TDD names spelled out:
    • rolledUpFeed_combines_same_actor_same_doc_within_2h
    • rolledUpFeed_splits_at_2h_boundary
    • rolledUpFeed_never_rolls_up_COMMENT_ADDED_or_MENTION_CREATED
    • rolledUpFeed_exposes_count_and_happenedAtUntil
    • chronik_load_redirects_from_notifications_path
  • Reactive Alle gelesen visibility via $derived($page.data.unreadCount > 0) so SSE-delivered new notifications re-show the button without a reload.

Open Decisions

  • None — all items are concrete recommendations.
## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - Repository method is `AuditLogQueryRepository.findDedupedActivityFeed` (L25), not `findActivityFeed` as the issue says. Service wrapper `AuditLogQueryService.findActivityFeed()` calls it. - The existing query already dedupes by `date_trunc('hour', ...)` — a **different** mechanism than the proposed 2 h rollup. Two aggregation strategies side by side would be confusing. - `ActivityFeedItemDTO` is a Java record; extending it means updating the call site at `DashboardService.java:133-140` + OpenAPI regen. - `Notification` is a persisted entity, distinct from `audit_log`. Für-dich (notifications table) and Timeline (audit_log) legitimately need two queries — not a duplication to refactor. - `accessibility.spec.ts` already parameterizes over `AUTHENTICATED_PAGES` × 3 modes (light, system dark, manual dark). Adding `/chronik` to that array is a one-line win. - `hooks.server.ts` uses `sequence()`. A redirect there would compete with the simpler `+page.server.ts` approach. ### Recommendations - **Rename the repo method** in the scope: the target is `findDedupedActivityFeed`. Since it will no longer dedupe but roll up, rename to `findRolledUpActivityFeed` along with the change. - **Replace the dedupe, don't stack**: new query groups by `(actor_id, document_id, kind, floor(epoch/7200))` for 2 h buckets. Keeping `DISTINCT ON` leaves zombie logic. - **Redirect at the route, not in hooks**: `src/routes/notifications/+page.server.ts` with `throw redirect(301, '/chronik')`. Colocated, no hook ordering concerns. - **Avoid nested-link anti-pattern** (spec §5.4): outer `<a href="/documents/{id}">` wraps the row; the document title is a styled `<span>`, not an inner `<a>` with `pointer-events:none`. Valid HTML + same UX. - **Keep `ChronikRow.svelte` as one file** with a `$derived` variant discriminator unless it crosses 60 lines. 4 variants = one orchestrator, not 4 components. - **Namespace under `$lib/components/chronik/`** — 7 components in a shared folder is noise. - **TDD names spelled out**: - `rolledUpFeed_combines_same_actor_same_doc_within_2h` - `rolledUpFeed_splits_at_2h_boundary` - `rolledUpFeed_never_rolls_up_COMMENT_ADDED_or_MENTION_CREATED` - `rolledUpFeed_exposes_count_and_happenedAtUntil` - `chronik_load_redirects_from_notifications_path` - **Reactive `Alle gelesen` visibility** via `$derived($page.data.unreadCount > 0)` so SSE-delivered new notifications re-show the button without a reload. ### Open Decisions - None — all items are concrete recommendations.
Author
Owner

🏛️ Markus Keller — Senior Application Architect

Observations

  • Two systems of record: notifications table (persisted per-recipient delivery with read state) and audit_log (global event log). A MENTION_CREATED audit event produces one or more notifications rows. Intentional duplication — the notifications system owns delivery; audit owns history. Spec correctly keeps both.
  • Authorization asymmetry: DashboardController requires READ_ALL (permission-gated); NotificationController is user-scoped without @RequirePermission (deliberate, see L39-41 comment). A user lacking READ_ALL hits 403 on /api/dashboard/activity but 200 on /api/notifications.
  • Paraglide routing: src/hooks.ts uses deLocalizeUrl. Route registration for /chronik needs localized slugs (Paraglide messages + routing strategy entries for en/es).
  • No new DB migration strictly required — but a covering index for the rollup query will pay for itself on day one.

Recommendations

  • Keep two endpoints, don't unify: resist creating /api/chronik as an orchestrator. Let +page.server.ts do the composition. Merging creates a coupling that hurts both domains.
  • Harden the page, not the endpoint: /chronik load function should branch on locals.user.permissions. Users without READ_ALL see only Für-dich and an explicit "Mehr Zugriff nötig" empty timeline state. Do not weaken DashboardController's permission requirement.
  • Add a partial covering index as part of this issue:
    CREATE INDEX idx_audit_log_rollup
        ON audit_log (actor_id, document_id, kind, happened_at DESC)
        WHERE kind IN ('TEXT_SAVED','FILE_UPLOADED','ANNOTATION_CREATED','BLOCK_REVIEWED','COMMENT_ADDED','MENTION_CREATED');
    
    Ships in a new Flyway migration V{next}__add_audit_log_rollup_index.sql. Partial index matches the WHERE clause exactly; DESC supports the outer ORDER BY without a sort step.
  • Day grouping is a presentation concern — compute in the component, not the backend. API returns a flat sorted list; ChronikTimeline.svelte buckets into Heute/Gestern/Diese Woche/Älter using locale-aware date math. Keeps the API decoupled from UI iteration.
  • Feature-package on the backend later: for this iteration, rollup lives in dashboard/ since it shares the /api/dashboard/activity endpoint. If Chronik acquires its own endpoint in v2, extract to a new chronik/ package then.

Open Decisions

  • Permission model for /chronik page load. Option A: require READ_ALL (mirror dashboard, users without it get 403 on the page). Option B: any authenticated user can see the page, timeline degrades gracefully when the endpoint returns 403. Tradeoff: A is consistent, B is forgiving to lower-permission roles (e.g., users who can only comment). Affects both routing and empty-state design.
## 🏛️ Markus Keller — Senior Application Architect ### Observations - Two systems of record: `notifications` table (persisted per-recipient delivery with read state) and `audit_log` (global event log). A `MENTION_CREATED` audit event produces one or more `notifications` rows. Intentional duplication — the notifications system owns delivery; audit owns history. Spec correctly keeps both. - Authorization asymmetry: `DashboardController` requires `READ_ALL` (permission-gated); `NotificationController` is user-scoped without `@RequirePermission` (deliberate, see L39-41 comment). A user lacking `READ_ALL` hits 403 on `/api/dashboard/activity` but 200 on `/api/notifications`. - Paraglide routing: `src/hooks.ts` uses `deLocalizeUrl`. Route registration for `/chronik` needs localized slugs (Paraglide messages + routing strategy entries for en/es). - No new DB migration strictly required — but a covering index for the rollup query will pay for itself on day one. ### Recommendations - **Keep two endpoints, don't unify**: resist creating `/api/chronik` as an orchestrator. Let `+page.server.ts` do the composition. Merging creates a coupling that hurts both domains. - **Harden the page, not the endpoint**: `/chronik` load function should branch on `locals.user.permissions`. Users without `READ_ALL` see only Für-dich and an explicit "Mehr Zugriff nötig" empty timeline state. Do not weaken `DashboardController`'s permission requirement. - **Add a partial covering index as part of this issue**: ```sql CREATE INDEX idx_audit_log_rollup ON audit_log (actor_id, document_id, kind, happened_at DESC) WHERE kind IN ('TEXT_SAVED','FILE_UPLOADED','ANNOTATION_CREATED','BLOCK_REVIEWED','COMMENT_ADDED','MENTION_CREATED'); ``` Ships in a new Flyway migration `V{next}__add_audit_log_rollup_index.sql`. Partial index matches the WHERE clause exactly; DESC supports the outer ORDER BY without a sort step. - **Day grouping is a presentation concern — compute in the component**, not the backend. API returns a flat sorted list; `ChronikTimeline.svelte` buckets into Heute/Gestern/Diese Woche/Älter using locale-aware date math. Keeps the API decoupled from UI iteration. - **Feature-package on the backend later**: for this iteration, rollup lives in `dashboard/` since it shares the `/api/dashboard/activity` endpoint. If Chronik acquires its own endpoint in v2, extract to a new `chronik/` package then. ### Open Decisions - **Permission model for `/chronik` page load.** Option A: require `READ_ALL` (mirror dashboard, users without it get 403 on the page). Option B: any authenticated user can see the page, timeline degrades gracefully when the endpoint returns 403. Tradeoff: A is consistent, B is forgiving to lower-permission roles (e.g., users who can only comment). Affects both routing and empty-state design.
Author
Owner

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

  • NotificationController intentionally skips @RequirePermission and scopes by user.getId() (L39-41). markRead (L74-80) passes user.getId() to the service so ownership can be enforced there — the service implementation should be covered by an explicit ownership test.
  • Comment bodies are stored raw and rendered via {@html renderBody(...)} in CommentThread.svelte (that render path is trusted to sanitize). The Chronik preview is a different display context with stricter needs.
  • SvelteKit redirect(status, ...) must be called with 301 explicitly — default for Location redirects in SK is 307, not 301.
  • "Alle gelesen" is destructive (mass-mark-read) but reversible. Low risk, already exists.

Recommendations

  • Sanitize the Für-dich preview server-side to plain text — never render as HTML in the Chronik context. Truncate at 140 chars on the backend, strip tags, return as plain string. Frontend wraps it in literal German quotes in a normal {text} interpolation — never {@html}.
  • Codify the ownership boundary as a test:
    @Test
    void markRead_returns403_when_user_marks_others_notification() {
        UUID otherNotif = createNotificationForUser(otherUserId);
        mockMvc.perform(patch("/api/notifications/{id}/read", otherNotif)
            .with(user("eve")))
            .andExpect(status().isForbidden());
    }
    
    This locks in the non-@RequirePermission trust model — if someone refactors the service and forgets the ownership check, this fails.
  • Explicit 301 + test:
    // src/routes/notifications/+page.server.ts
    export const load: PageServerLoad = () => { throw redirect(301, '/chronik'); };
    
    Unit-test that the response status is 301, not 302/307. 301 enables caching.
  • Document titles stay as plain text: titles reach the component via server data; Svelte's {text} interpolation escapes by default. Confirm no {@html} creeps in for titles in the new components.
  • CSRF path: use SvelteKit form action (<form method="POST" use:enhance>) for "Alle gelesen" and the per-row Dismiss — not a raw client fetch() — so CSRF protection is automatic and consistent with existing admin patterns.
  • Stop propagation on Dismiss button: the button lives inside a linked row. Click must not bubble to the parent <a>. Add a test that asserts clicking Dismiss marks read without navigating.

Open Decisions

  • None — all items are fixed with clear implementation.
## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations - `NotificationController` intentionally skips `@RequirePermission` and scopes by `user.getId()` (L39-41). `markRead` (L74-80) passes `user.getId()` to the service so ownership can be enforced there — the service implementation should be covered by an explicit ownership test. - Comment bodies are stored raw and rendered via `{@html renderBody(...)}` in `CommentThread.svelte` (that render path is trusted to sanitize). The Chronik preview is a **different** display context with stricter needs. - SvelteKit `redirect(status, ...)` must be called with `301` explicitly — default for `Location` redirects in SK is `307`, not `301`. - "Alle gelesen" is destructive (mass-mark-read) but reversible. Low risk, already exists. ### Recommendations - **Sanitize the Für-dich preview server-side to plain text** — never render as HTML in the Chronik context. Truncate at 140 chars on the backend, strip tags, return as plain string. Frontend wraps it in literal German quotes in a normal `{text}` interpolation — never `{@html}`. - **Codify the ownership boundary as a test**: ```java @Test void markRead_returns403_when_user_marks_others_notification() { UUID otherNotif = createNotificationForUser(otherUserId); mockMvc.perform(patch("/api/notifications/{id}/read", otherNotif) .with(user("eve"))) .andExpect(status().isForbidden()); } ``` This locks in the non-`@RequirePermission` trust model — if someone refactors the service and forgets the ownership check, this fails. - **Explicit 301 + test**: ```typescript // src/routes/notifications/+page.server.ts export const load: PageServerLoad = () => { throw redirect(301, '/chronik'); }; ``` Unit-test that the response status is `301`, not `302`/`307`. 301 enables caching. - **Document titles stay as plain text**: titles reach the component via server data; Svelte's `{text}` interpolation escapes by default. Confirm no `{@html}` creeps in for titles in the new components. - **CSRF path**: use SvelteKit form action (`<form method="POST" use:enhance>`) for "Alle gelesen" and the per-row Dismiss — not a raw client `fetch()` — so CSRF protection is automatic and consistent with existing admin patterns. - **Stop propagation on Dismiss button**: the button lives inside a linked row. Click must not bubble to the parent `<a>`. Add a test that asserts clicking Dismiss marks read without navigating. ### Open Decisions - None — all items are fixed with clear implementation.
Author
Owner

🧪 Sara Holt — Senior QA Engineer

Observations

  • e2e/accessibility.spec.ts already parameterizes over AUTHENTICATED_PAGES × 3 modes (light, system dark, manual dark). Adding /chronik to that array gives us three new a11y checks with ~one line of code — that fulfills the "axe-playwright both modes" acceptance item immediately.
  • No visual regression suite exists today. The acceptance item "visual regression snapshots at 320/768/1440" implies new infrastructure (Playwright toHaveScreenshot, baseline images, CI artifact pipeline, flake handling around font rendering).
  • Co-located spec pattern is canonical: DashboardActivityFeed.svelte.spec.ts lives next to the component.
  • Testcontainers with postgres:16-alpine is canonical for integration tests — no migration needed to adopt it here.

Recommendations

  • Add /chronik to AUTHENTICATED_PAGES as part of this issue. Three test cases light up for free.
  • Move visual regression to a follow-up issue. Building the VR infrastructure (baseline commit strategy, Docker font pinning, flaky-test strategy) is its own 1-2-day workstream; it should not block Chronik. Remove from this issue's acceptance checklist.
  • Concrete test file list:
    frontend/src/lib/components/chronik/ChronikFuerDichBox.svelte.spec.ts
    frontend/src/lib/components/chronik/ChronikFilterPills.svelte.spec.ts
    frontend/src/lib/components/chronik/ChronikTimeline.svelte.spec.ts
    frontend/src/lib/components/chronik/ChronikRow.svelte.spec.ts
    frontend/src/lib/components/chronik/ChronikEmptyState.svelte.spec.ts
    frontend/src/lib/components/chronik/ChronikErrorCard.svelte.spec.ts
    frontend/src/routes/chronik/page.server.spec.ts
    frontend/src/routes/notifications/page.server.spec.ts  (redirect)
    backend/src/test/java/org/raddatz/familienarchiv/audit/AuditLogQueryRepositoryRolledUpTest.java
    
  • Rollup boundary test fixture: helper insertAuditEvents(actor, doc, kind, startTime, count, gapMinutes). The test that matters: insert 20 TEXT_SAVED events 8 min apart starting at T0 (span: 152 min, crosses a 2 h boundary). Assert 2 rollup rows produced, counts summing to 20, time ranges non-overlapping.
  • E2E for the migration moment: after clicking Dismiss, the row leaves Für-dich and appears in the timeline with the for-you lane. This is the most fragile UX transition in the design.
  • SSE real-time test: new notification arrives via /api/notifications/stream while user is on /chronik — Für-dich box prepends it without reload. Currently only NotificationBell consumes the stream; this may surface missing plumbing.
  • Pagination test: "Mehr laden" preserves the active filter and day-grouping.
  • Per-kind plural test: chronik_rollup_text_saved with count=1 vs count=20 renders correct German plural (Paraglide handles pluralization).

Open Decisions

  • Visual regression scope. Ship in this issue (delays merge by the VR setup time) vs. split into follow-up issue (faster Chronik delivery, acceptance checklist trimmed). Recommend split.
## 🧪 Sara Holt — Senior QA Engineer ### Observations - `e2e/accessibility.spec.ts` already parameterizes over `AUTHENTICATED_PAGES` × 3 modes (light, system dark, manual dark). Adding `/chronik` to that array gives us three new a11y checks with ~one line of code — that fulfills the "axe-playwright both modes" acceptance item immediately. - **No visual regression suite exists today.** The acceptance item "visual regression snapshots at 320/768/1440" implies new infrastructure (Playwright `toHaveScreenshot`, baseline images, CI artifact pipeline, flake handling around font rendering). - Co-located spec pattern is canonical: `DashboardActivityFeed.svelte.spec.ts` lives next to the component. - Testcontainers with `postgres:16-alpine` is canonical for integration tests — no migration needed to adopt it here. ### Recommendations - **Add `/chronik` to `AUTHENTICATED_PAGES`** as part of this issue. Three test cases light up for free. - **Move visual regression to a follow-up issue.** Building the VR infrastructure (baseline commit strategy, Docker font pinning, flaky-test strategy) is its own 1-2-day workstream; it should not block Chronik. Remove from this issue's acceptance checklist. - **Concrete test file list**: ``` frontend/src/lib/components/chronik/ChronikFuerDichBox.svelte.spec.ts frontend/src/lib/components/chronik/ChronikFilterPills.svelte.spec.ts frontend/src/lib/components/chronik/ChronikTimeline.svelte.spec.ts frontend/src/lib/components/chronik/ChronikRow.svelte.spec.ts frontend/src/lib/components/chronik/ChronikEmptyState.svelte.spec.ts frontend/src/lib/components/chronik/ChronikErrorCard.svelte.spec.ts frontend/src/routes/chronik/page.server.spec.ts frontend/src/routes/notifications/page.server.spec.ts (redirect) backend/src/test/java/org/raddatz/familienarchiv/audit/AuditLogQueryRepositoryRolledUpTest.java ``` - **Rollup boundary test fixture**: helper `insertAuditEvents(actor, doc, kind, startTime, count, gapMinutes)`. The test that matters: insert 20 TEXT_SAVED events 8 min apart starting at T0 (span: 152 min, crosses a 2 h boundary). Assert 2 rollup rows produced, counts summing to 20, time ranges non-overlapping. - **E2E for the migration moment**: after clicking Dismiss, the row leaves Für-dich and appears in the timeline with the for-you lane. This is the most fragile UX transition in the design. - **SSE real-time test**: new notification arrives via `/api/notifications/stream` while user is on `/chronik` — Für-dich box prepends it without reload. Currently only `NotificationBell` consumes the stream; this may surface missing plumbing. - **Pagination test**: "Mehr laden" preserves the active filter and day-grouping. - **Per-kind plural test**: `chronik_rollup_text_saved` with `count=1` vs `count=20` renders correct German plural (Paraglide handles pluralization). ### Open Decisions - **Visual regression scope.** Ship in this issue (delays merge by the VR setup time) vs. split into follow-up issue (faster Chronik delivery, acceptance checklist trimmed). Recommend split.
Author
Owner

🎨 Leonie Voss — UX/Design Lead

I wrote this spec, so I'm reviewing my own work with a critical second pass. Five gaps / refinements found.

Observations

  • Spec state 06 at 320 px shows "Keine neuen" — grammatically incomplete (" …neuen was?"). Larger viewports correctly say "Keine neuen Erwähnungen". This is an editorial drift to fix before ship.
  • Real-time behavior is not specified anywhere in the spec. The existing NotificationBell consumes /api/notifications/stream; Chronik's Für-dich box should behave the same way — but it's not written down.
  • Spec state 10 shows initial-load skeleton. Pagination-load ("Mehr laden" clicked, waiting for next batch) has no rendering.
  • Mobile (320 px) hides the Dismiss ✓ button; the whole row navigates and marks read. No gesture to mark read without opening.
  • Icon mapping hard-codes @ for MENTION and for REPLY. A future NotificationType enum value renders as "UNKNOWN" unless we add a fallback.

Recommendations

  • Normalize the "inbox zero" copy to "Keine neuen Erwähnungen" at all widths. Link copy stays "Ältere Erwähnungen ansehen →" on tablet/desktop, tighten to "Ältere ansehen →" only on 320 px.
  • Add SSE real-time to the acceptance checklist: "Neue Erwähnung, die per SSE eingeht während der Nutzer auf /chronik ist, erscheint ohne Reload am Anfang der Für-dich-Box." Store layer: createNotificationStream() already exists — reuse it in ChronikFuerDichBox.svelte, don't fork.
  • Pagination-loading micro-state: on click, replace "Mehr laden" label with "Lädt …" and add aria-busy="true" to the button. Keyboard focus stays on the button after load so screen-reader users don't lose their place.
  • Icon fallback: add a generic bell icon for unknown notification types. The switch lives in ChronikFuerDichBox — add a default branch. Future-proofs against new NotificationType values.
  • Day-header locale: "Diese Woche" implies a Monday-anchored week in German. Paraglide doesn't help with week-start; use Intl.DateTimeFormat with weekday + Temporal (or dayjs) to compute. Confirm week-start is locale-aware (Monday de/es, Sunday en-US) before hard-coding.
  • Verify #a1dcd8 active-pill text contrast in axe CI on /chronik — ratio should be 9.8:1 against #012851, but confirm empirically at all three viewports.

Open Decisions

  • Mobile Dismiss button at 320 px. Option A: show it (preserves parity, may squeeze other columns). Option B: long-press → action sheet (new gesture, senior-unfriendly). Option C: defer to v2 (current spec, mobile users must navigate to mark read). Recommend A for launch.
## 🎨 Leonie Voss — UX/Design Lead I wrote this spec, so I'm reviewing my own work with a critical second pass. Five gaps / refinements found. ### Observations - Spec state 06 at 320 px shows "Keine neuen" — grammatically incomplete (" …neuen was?"). Larger viewports correctly say "Keine neuen Erwähnungen". This is an editorial drift to fix before ship. - Real-time behavior is not specified anywhere in the spec. The existing `NotificationBell` consumes `/api/notifications/stream`; Chronik's Für-dich box should behave the same way — but it's not written down. - Spec state 10 shows initial-load skeleton. Pagination-load ("Mehr laden" clicked, waiting for next batch) has no rendering. - Mobile (320 px) hides the Dismiss ✓ button; the whole row navigates and marks read. No gesture to mark read without opening. - Icon mapping hard-codes `@` for MENTION and `↩` for REPLY. A future `NotificationType` enum value renders as "UNKNOWN" unless we add a fallback. ### Recommendations - **Normalize the "inbox zero" copy** to "Keine neuen Erwähnungen" at all widths. Link copy stays "Ältere Erwähnungen ansehen →" on tablet/desktop, tighten to "Ältere ansehen →" only on 320 px. - **Add SSE real-time to the acceptance checklist**: "Neue Erwähnung, die per SSE eingeht während der Nutzer auf /chronik ist, erscheint ohne Reload am Anfang der Für-dich-Box." Store layer: `createNotificationStream()` already exists — reuse it in `ChronikFuerDichBox.svelte`, don't fork. - **Pagination-loading micro-state**: on click, replace "Mehr laden" label with "Lädt …" and add `aria-busy="true"` to the button. Keyboard focus stays on the button after load so screen-reader users don't lose their place. - **Icon fallback**: add a generic bell icon for unknown notification types. The switch lives in `ChronikFuerDichBox` — add a default branch. Future-proofs against new `NotificationType` values. - **Day-header locale**: "Diese Woche" implies a Monday-anchored week in German. Paraglide doesn't help with week-start; use `Intl.DateTimeFormat` with `weekday` + `Temporal` (or dayjs) to compute. Confirm week-start is locale-aware (Monday de/es, Sunday en-US) before hard-coding. - **Verify `#a1dcd8` active-pill text contrast** in axe CI on /chronik — ratio should be 9.8:1 against `#012851`, but confirm empirically at all three viewports. ### Open Decisions - **Mobile Dismiss button at 320 px.** Option A: show it (preserves parity, may squeeze other columns). Option B: long-press → action sheet (new gesture, senior-unfriendly). Option C: defer to v2 (current spec, mobile users must navigate to mark read). Recommend A for launch.
Author
Owner

🏗️ Tobias Wendt — DevOps & Platform Engineer

Observations

  • No new containers, volumes, or env vars. No Compose changes. This is pure app code.
  • The 301 redirect can live in SvelteKit (+page.server.ts) or Caddy (redir /notifications /chronik permanent). App-level wins: testable in CI, self-contained, doesn't couple to the reverse-proxy config.
  • Once Chronik becomes the default, /api/dashboard/activity takes on more traffic. The rollup query becomes a new hot path.
  • /api/notifications/stream SSE already works through Caddy (Bell dropdown uses it). No config change expected.

Recommendations

  • Redirect at the app, not Caddy — one line in +page.server.ts, survives reverse-proxy refactors, CI-testable.
  • Ship Markus's index via Flyway (V{next}__add_audit_log_rollup_index.sql). Never via CREATE INDEX IF NOT EXISTS at app boot — migrations are the single source of truth.
  • Add a Grafana panel for /api/dashboard/activity: request rate + p95 latency. Alert at p95 > 500 ms sustained 5 min. The rollup is the new hot path; if the index is missing or the query regresses, this fires first.
  • No change to env vars, no new secrets, no Compose overlay work. Leaving this flagged only because the issue body mentions env vars in passing.

Open Decisions

  • None — no infrastructure changes required.
## 🏗️ Tobias Wendt — DevOps & Platform Engineer ### Observations - No new containers, volumes, or env vars. No Compose changes. This is pure app code. - The 301 redirect can live in SvelteKit (`+page.server.ts`) or Caddy (`redir /notifications /chronik permanent`). App-level wins: testable in CI, self-contained, doesn't couple to the reverse-proxy config. - Once Chronik becomes the default, `/api/dashboard/activity` takes on more traffic. The rollup query becomes a new hot path. - `/api/notifications/stream` SSE already works through Caddy (Bell dropdown uses it). No config change expected. ### Recommendations - **Redirect at the app**, not Caddy — one line in `+page.server.ts`, survives reverse-proxy refactors, CI-testable. - **Ship Markus's index via Flyway** (`V{next}__add_audit_log_rollup_index.sql`). Never via `CREATE INDEX IF NOT EXISTS` at app boot — migrations are the single source of truth. - **Add a Grafana panel for `/api/dashboard/activity`**: request rate + p95 latency. Alert at p95 > 500 ms sustained 5 min. The rollup is the new hot path; if the index is missing or the query regresses, this fires first. - **No change to env vars, no new secrets, no Compose overlay work.** Leaving this flagged only because the issue body mentions env vars in passing. ### Open Decisions - None — no infrastructure changes required.
Author
Owner

🗳️ Decision Queue — Action Required

3 decisions need your input before implementation starts.

Architecture & Access

  • Permission model for /chronik page load.
    • Option A: Require READ_ALL on the page (mirror DashboardController). Users without it get 403 on the page itself.
    • Option B: Any authenticated user can load the page; timeline degrades to an empty "Mehr Zugriff nötig" state when /api/dashboard/activity returns 403, Für-dich still renders.
    • Tradeoff: A is consistent with existing dashboard gating; B is forgiving to lower-permission roles (e.g., users with only comment rights). Affects both the route guard and the empty-state design.
    • (Raised by: Markus)

UX

  • Mobile Dismiss (✓) button at 320 px.
    • Option A (recommended): Show the button at 320 px too — preserves behavior parity across viewports but squeezes column widths; must verify truncation still works.
    • Option B: Long-press → action sheet — new gesture, senior-unfriendly, not in the spec's dual-audience philosophy.
    • Option C: Defer to v2 (current spec) — mobile users must open the document to mark it read.
    • (Raised by: Leonie)

Testing Scope

  • Visual regression in this issue, or a follow-up?
    • Option A: Split — remove "visual regression at 320/768/1440" from this issue's acceptance checklist, open a new devops-labeled issue for VR infrastructure (Playwright toHaveScreenshot, baseline strategy, Docker font pinning, flake tooling). Chronik ships faster.
    • Option B: Ship together — Chronik blocks on setting up the VR suite from scratch. Adds 1-2 days.
    • (Raised by: Sara)

All other persona feedback consists of concrete recommendations that Felix can implement during the TDD cycle without blocking discussion.

## 🗳️ Decision Queue — Action Required _3 decisions need your input before implementation starts._ ### Architecture & Access - **Permission model for `/chronik` page load.** - **Option A:** Require `READ_ALL` on the page (mirror `DashboardController`). Users without it get 403 on the page itself. - **Option B:** Any authenticated user can load the page; timeline degrades to an empty "Mehr Zugriff nötig" state when `/api/dashboard/activity` returns 403, Für-dich still renders. - _Tradeoff:_ A is consistent with existing dashboard gating; B is forgiving to lower-permission roles (e.g., users with only comment rights). Affects both the route guard and the empty-state design. - _(Raised by: Markus)_ ### UX - **Mobile Dismiss (✓) button at 320 px.** - **Option A (recommended):** Show the button at 320 px too — preserves behavior parity across viewports but squeezes column widths; must verify truncation still works. - **Option B:** Long-press → action sheet — new gesture, senior-unfriendly, not in the spec's dual-audience philosophy. - **Option C:** Defer to v2 (current spec) — mobile users must open the document to mark it read. - _(Raised by: Leonie)_ ### Testing Scope - **Visual regression in this issue, or a follow-up?** - **Option A:** Split — remove "visual regression at 320/768/1440" from this issue's acceptance checklist, open a new devops-labeled issue for VR infrastructure (Playwright `toHaveScreenshot`, baseline strategy, Docker font pinning, flake tooling). Chronik ships faster. - **Option B:** Ship together — Chronik blocks on setting up the VR suite from scratch. Adds 1-2 days. - _(Raised by: Sara)_ All other persona feedback consists of concrete recommendations that Felix can implement during the TDD cycle without blocking discussion.
Author
Owner

🏛️ Markus Keller — Architectural Decisions (Discussion Outcome)

Walked through the five architect-scope open items with Marcel. All five resolved — no lingering architectural questions block implementation.

Resolved

  • Permission model for /chronik. No explicit @RequirePermission guard on the page — handleAuth in hooks.server.ts is sufficient. Every user has at least READ_ALL, so the degraded-access state is academic and gets cut. DashboardController.@RequirePermission(READ_ALL) stays untouched — belt-and-braces at the endpoint layer.

  • Rollup SQL algorithm. Session-style grouping via LAG() window function + 120 min gap detection. A new session starts whenever the gap to the previous event (same actor / document / kind) exceeds 120 min. Rejected: fixed 2 h wall-clock buckets — they split a natural session like 13:58 / 13:59 / 14:01 into two rollup rows. No hard cap on total span (one long transcription sitting = one row).

  • Rollup scope — everywhere. The query change affects both /chronik and the dashboard side-rail (DashboardActivityFeed.svelte). The existing findDedupedActivityFeed is renamed to findRolledUpActivityFeed and the DISTINCT ON … date_trunc('hour', ...) logic is deleted. One aggregation strategy across the whole app. Dashboard side-rail benefits: count badges replace granular hour-deduped rows — improvement, not regression.

  • Localized URL slugs. Universal /chronik across all locales. No Paraglide URL pattern entry. Consistent with the existing German-URL-only convention (/dokumente, /personen, /briefwechsel). Page content is Paraglide-translated; the URL is a stable German identifier.

  • ADR in scope. Write docs/adr/003-chronik-unified-activity-feed.md as part of the implementation branch, following the ADR-001/002 format (Status / Context / Decision / Alternatives / Consequences). Captures: session-style rollup choice, hour-dedupe deletion, single-endpoint-for-both-surfaces decision, German-URL-only rationale. Bundled with the rollup SQL commit so the record is never orphaned from its code.

Skipped / unresolved

None.

Overall read

The spec is architecturally sound. The riskiest call was the rollup algorithm — session-grouping via LAG() is 15 extra lines of SQL that prevent a class of "two rollup rows for one burst" bugs the fixed-bucket approach would produce on day one. The rest of the architectural surface (single endpoint, universal URL, no new package, index-backed partial SELECT) is the path of minimum operational overhead. Felix can start implementation.

## 🏛️ Markus Keller — Architectural Decisions (Discussion Outcome) Walked through the five architect-scope open items with Marcel. All five resolved — no lingering architectural questions block implementation. ### Resolved - **Permission model for `/chronik`.** No explicit `@RequirePermission` guard on the page — `handleAuth` in `hooks.server.ts` is sufficient. Every user has at least `READ_ALL`, so the degraded-access state is academic and gets cut. `DashboardController.@RequirePermission(READ_ALL)` stays untouched — belt-and-braces at the endpoint layer. - **Rollup SQL algorithm.** Session-style grouping via `LAG()` window function + 120 min gap detection. A new session starts whenever the gap to the previous event (same actor / document / kind) exceeds 120 min. **Rejected:** fixed 2 h wall-clock buckets — they split a natural session like `13:58 / 13:59 / 14:01` into two rollup rows. No hard cap on total span (one long transcription sitting = one row). - **Rollup scope — everywhere.** The query change affects both `/chronik` and the dashboard side-rail (`DashboardActivityFeed.svelte`). The existing `findDedupedActivityFeed` is renamed to `findRolledUpActivityFeed` and the `DISTINCT ON … date_trunc('hour', ...)` logic is **deleted**. One aggregation strategy across the whole app. Dashboard side-rail benefits: count badges replace granular hour-deduped rows — improvement, not regression. - **Localized URL slugs.** Universal `/chronik` across all locales. No Paraglide URL pattern entry. Consistent with the existing German-URL-only convention (`/dokumente`, `/personen`, `/briefwechsel`). Page *content* is Paraglide-translated; the URL is a stable German identifier. - **ADR in scope.** Write `docs/adr/003-chronik-unified-activity-feed.md` as part of the implementation branch, following the ADR-001/002 format (Status / Context / Decision / Alternatives / Consequences). Captures: session-style rollup choice, hour-dedupe deletion, single-endpoint-for-both-surfaces decision, German-URL-only rationale. Bundled with the rollup SQL commit so the record is never orphaned from its code. ### Skipped / unresolved None. ### Overall read The spec is architecturally sound. The riskiest call was the rollup algorithm — session-grouping via `LAG()` is 15 extra lines of SQL that prevent a class of "two rollup rows for one burst" bugs the fixed-bucket approach would produce on day one. The rest of the architectural surface (single endpoint, universal URL, no new package, index-backed partial SELECT) is the path of minimum operational overhead. Felix can start implementation.
Author
Owner

🎨 Leonie Voss — UX/Design Decisions (Discussion Outcome)

Walked through the five UX-scope open items with Marcel. All five resolved — no unresolved design decisions block implementation. Several items are spec addenda that I'll fold into docs/specs/chronik-spec.html before Felix starts.

Resolved

  • Mobile Dismiss button at 320 px (Decision Queue item). Option A2: show the ✓ button on mobile, but drop the @/ marker at 320 px only — it's redundant with the verb text ("Mama erwähnte dich" / "Anna antwortete") and the pixels are worth ~200 px of body space instead of ~178. Redundant cues preserved: unread dot (color), accent left border (color), verb text — two non-color signals, WCAG-safe. Marker returns at ≥ 640 px.

  • Row markup (Felix's anti-pattern flag). Single outer <a href="/documents/{id}"> wraps the entire row. Document title renders as a styled <span> with the existing underline decoration-accent treatment — preserves the learned "underlined accent text = document" visual contract used elsewhere in the app (/dokumente, person pages, conversation view). Row-level hover/focus on the anchor. Dismiss button uses stopPropagation so its click does not navigate.

  • SSE live-arrival behavior (spec gap I flagged in my own review). Four sub-decisions packaged together:

    • Where: new row prepends to the Für-dich box only. Timeline picks it up after mark-read, normal flow. No duplicate treatment.
    • How: fade-in 150 ms (opacity 0→1 + translateY(-4px → 0)). Layout shift of rows below is unavoidable but handled by browser scroll anchoring for users reading the timeline; for users already looking at Für-dich, the shift is honest context ("3 → 4 unreads, here's the new one").
    • Auto-scroll: never. User owns scroll position. Count badge + "Alle gelesen" button re-appearance provide the awareness signal.
    • prefers-reduced-motion: animation dropped entirely. Row appears instantly. aria-live="polite" on the count badge still announces the update for screen readers.
    • Inbox-zero → new-mention edge case: reassurance card ("✓ Keine neuen Erwähnungen") fades out, unread row list fades in. Same 150 ms. Reduced-motion: instant swap.
  • Pagination loading micro-state (spec gap I flagged in my own review). Hybrid (Option C): button label swaps "Mehr laden" → "Lädt …" with aria-busy="true" and focus preserved on the button, plus 3 static (non-shimmer) skeleton rows render below the button during fetch to signal where the new rows will land. On completion, skeletons replaced by real rows, button label restored, focus stays on the button (does NOT auto-move to new content). aria-live="polite" wrapper around the timeline announces "X weitere Einträge geladen" for screen-reader users.

  • Dashboard side-rail rollup visual fit (from Markus's "rollup everywhere" decision). Rail is 280 px content width — slightly narrower than Chronik's 320 px mobile. Rollup rows still fit cleanly. Option B — compound timestamp: line 2 becomes "14. Apr. · 14:02–14:32" for rollup rows, stays "14. Apr. 2026" for singletons. Count badge inline on line 1 matches Chronik's styling (bg-primary text-primary-fg text-xs px-2 py-0.5 rounded-sm) — shared visual so DashboardActivityFeed.svelte and /chronik cannot drift apart. Existing "FÜR DICH" accent pill for youMentioned coexists with rollup (mentions never roll up).

Spec addenda for me to apply before implementation

  • Update 320 px mockup frames in states 01 / 02 / 05 / 08 to reflect A2 layout (Dismiss present, marker dropped).
  • Add SSE live-arrival interaction pattern to §05 (Interaction States) with reduced-motion branch.
  • Add pagination loading micro-state to §02 state 10 as a variant.

Skipped / unresolved

None.

Overall read

The design is tight now. The five items above were the gaps between "design looks great" and "developer can implement without guessing." The row-markup fix (single anchor + styled span) is the single highest-value call — it swaps an invalid HTML pattern for one that's both accessible and visually indistinguishable from what we'd originally spec'd. Felix can start.

## 🎨 Leonie Voss — UX/Design Decisions (Discussion Outcome) Walked through the five UX-scope open items with Marcel. All five resolved — no unresolved design decisions block implementation. Several items are spec addenda that I'll fold into `docs/specs/chronik-spec.html` before Felix starts. ### Resolved - **Mobile Dismiss button at 320 px (Decision Queue item).** **Option A2**: show the ✓ button on mobile, but **drop the `@`/`↩` marker at 320 px only** — it's redundant with the verb text ("Mama erwähnte dich" / "Anna antwortete") and the pixels are worth ~200 px of body space instead of ~178. Redundant cues preserved: unread dot (color), accent left border (color), verb text — two non-color signals, WCAG-safe. Marker returns at ≥ 640 px. - **Row markup (Felix's anti-pattern flag).** Single outer `<a href="/documents/{id}">` wraps the entire row. Document title renders as a **styled `<span>`** with the existing `underline decoration-accent` treatment — preserves the learned "underlined accent text = document" visual contract used elsewhere in the app (`/dokumente`, person pages, conversation view). Row-level hover/focus on the anchor. Dismiss button uses `stopPropagation` so its click does not navigate. - **SSE live-arrival behavior** (spec gap I flagged in my own review). Four sub-decisions packaged together: - **Where**: new row prepends to the Für-dich box only. Timeline picks it up after mark-read, normal flow. No duplicate treatment. - **How**: fade-in 150 ms (opacity 0→1 + `translateY(-4px → 0)`). Layout shift of rows below is unavoidable but handled by browser scroll anchoring for users reading the timeline; for users already looking at Für-dich, the shift is honest context ("3 → 4 unreads, here's the new one"). - **Auto-scroll**: never. User owns scroll position. Count badge + "Alle gelesen" button re-appearance provide the awareness signal. - **`prefers-reduced-motion`**: animation dropped entirely. Row appears instantly. `aria-live="polite"` on the count badge still announces the update for screen readers. - **Inbox-zero → new-mention edge case**: reassurance card ("✓ Keine neuen Erwähnungen") fades out, unread row list fades in. Same 150 ms. Reduced-motion: instant swap. - **Pagination loading micro-state** (spec gap I flagged in my own review). **Hybrid (Option C)**: button label swaps "Mehr laden" → "Lädt …" with `aria-busy="true"` and focus **preserved on the button**, plus 3 **static (non-shimmer) skeleton rows** render below the button during fetch to signal where the new rows will land. On completion, skeletons replaced by real rows, button label restored, focus stays on the button (does NOT auto-move to new content). `aria-live="polite"` wrapper around the timeline announces "X weitere Einträge geladen" for screen-reader users. - **Dashboard side-rail rollup visual fit** (from Markus's "rollup everywhere" decision). Rail is 280 px content width — slightly narrower than Chronik's 320 px mobile. Rollup rows still fit cleanly. **Option B — compound timestamp**: line 2 becomes `"14. Apr. · 14:02–14:32"` for rollup rows, stays `"14. Apr. 2026"` for singletons. Count badge inline on line 1 matches Chronik's styling (`bg-primary text-primary-fg text-xs px-2 py-0.5 rounded-sm`) — shared visual so `DashboardActivityFeed.svelte` and `/chronik` cannot drift apart. Existing "FÜR DICH" accent pill for `youMentioned` coexists with rollup (mentions never roll up). ### Spec addenda for me to apply before implementation - Update 320 px mockup frames in states 01 / 02 / 05 / 08 to reflect A2 layout (Dismiss present, marker dropped). - Add SSE live-arrival interaction pattern to §05 (Interaction States) with reduced-motion branch. - Add pagination loading micro-state to §02 state 10 as a variant. ### Skipped / unresolved None. ### Overall read The design is tight now. The five items above were the gaps between "design looks great" and "developer can implement without guessing." The row-markup fix (single anchor + styled span) is the single highest-value call — it swaps an invalid HTML pattern for one that's both accessible and visually indistinguishable from what we'd originally spec'd. Felix can start.
Author
Owner

👨‍💻 Felix Brandt — Implementation Decisions (Discussion Outcome)

Walked through the six dev-scope open items with Marcel. All six resolved. One follow-up issue (#286) spawned during the discussion.

Resolved

  • DTO evolution. Single ActivityFeedItemDTO extended with two fields — no discriminated union, no second DTO:

    • count: int with @Schema(requiredMode = REQUIRED) — always populated, 1 for singletons.
    • happenedAtUntil: OffsetDateTime nullable — null for singletons, end-of-session for rollups.
    • happenedAt semantically becomes start-of-session. Dashboard side-rail + Chronik share the same shape.
  • Dismiss button markup. Option C — split markup, honest semantics. Outer <div class="chronik-row"> (flex container hosts two children) with unified :hoverbg-muted:

    1. <a href="/documents/{id}"> wraps avatar + body + time (preserves Leonie's "linked row" UX for that region).
    2. <form method="POST" action="?/dismiss" use:enhance> + <button type="submit"> (CSRF-safe via SvelteKit form action).
    • Independent focus stops: anchor → dismiss → next row. Screen-reader users can reach Dismiss without entering the link.
    • "Alle gelesen" action also becomes a form action (?/mark-all).
    • Spawned follow-up #286 — rework NotificationBell to use form actions so the two surfaces don't diverge on the same endpoints.
  • SSE stream sharing — Path X, in scope of #285. Refactor useNotificationStream.svelte.ts (factory) → $lib/stores/notifications.svelte.ts (module-level singleton). Idempotent init() (ref-counted), ref-counted destroy(). Bell and Chronik Für-dich box both import the singleton. One EventSource per tab, always. Old file deleted — no zombie code.

  • Paraglide pluralization. No ICU match expressions. Split into chronik_singleton_X and chronik_rollup_X key pairs per verb. Component branches on count === 1:

    chronik_singleton_text_saved:  "{actor} transkribierte einen Block in {doc}"
    chronik_rollup_text_saved:     "{actor} transkribierte {count} Blöcke in {doc}"
    chronik_singleton_uploaded:    "{actor} lud ein Dokument hoch"
    chronik_rollup_uploaded:       "{actor} lud {count} Dokumente hoch"
    ... (reviewed, annotated: same pair pattern)
    chronik_singleton_comment_added, chronik_singleton_mention_created — single keys, never rolled up.
    

    Matches the project's existing pragmatic precedent (e.g. "vor {count} Minute(n)"). Translators need no grammatical plural rules.

  • Day-bucket helper — $lib/utils/date-buckets.ts. Pure function bucketByDay(date, now?, locale?) → DayBucket + getBucketLabel(bucket). Co-located .spec.ts covers midnight boundary, Monday-start-of-week (de/es), Sunday-start (en-US), DST transition, and "older" threshold. Pure function = clean unit tests without component harness friction.

  • Existing dedupe tests — audit result: nothing to delete. The only existing test (findDedupedActivityFeed_returnsAnnotationEntry) is a one-row smoke test that doesn't exercise dedupe behavior. Renamed to findRolledUpActivityFeed_returnsAnnotationEntry, strengthened with count == 1 + happenedAtUntil == null assertions. Six new rollup-behavior tests added on top (covering combine, split, long-session-no-cap, COMMENT/MENTION never-rolled-up, DTO exposure). DashboardActivityFeed.svelte.spec.ts gets new singleton + rollup render cases added — purely additive.

Skipped / unresolved

None.

Side effects

  • #286 created — refactor(notification-bell): use SvelteKit form actions instead of raw fetch. Blocked by this issue; addresses the same form-action convention on the Bell to prevent divergence.

Overall read

The design was already implementable before this discussion — but six dev-level unknowns would each have cost a ~20-minute decision during TDD. Resolving them up front means I write failing tests first without stopping to re-read the spec or poke at Paraglide docs. The highest-value call was the SSE singleton (Path X): shipping two EventSource connections to production would've been a silent regression; catching it at design time costs one refactor commit. Ready to start red-phase.

## 👨‍💻 Felix Brandt — Implementation Decisions (Discussion Outcome) Walked through the six dev-scope open items with Marcel. All six resolved. One follow-up issue (#286) spawned during the discussion. ### Resolved - **DTO evolution.** Single `ActivityFeedItemDTO` extended with two fields — no discriminated union, no second DTO: - `count: int` with `@Schema(requiredMode = REQUIRED)` — always populated, `1` for singletons. - `happenedAtUntil: OffsetDateTime` nullable — `null` for singletons, end-of-session for rollups. - `happenedAt` semantically becomes start-of-session. Dashboard side-rail + Chronik share the same shape. - **Dismiss button markup.** **Option C — split markup, honest semantics.** Outer `<div class="chronik-row">` (flex container hosts two children) with unified `:hover` → `bg-muted`: 1. `<a href="/documents/{id}">` wraps avatar + body + time (preserves Leonie's "linked row" UX for that region). 2. `<form method="POST" action="?/dismiss" use:enhance>` + `<button type="submit">` (CSRF-safe via SvelteKit form action). - Independent focus stops: anchor → dismiss → next row. Screen-reader users can reach Dismiss without entering the link. - "Alle gelesen" action also becomes a form action (`?/mark-all`). - Spawned **follow-up #286** — rework `NotificationBell` to use form actions so the two surfaces don't diverge on the same endpoints. - **SSE stream sharing — Path X, in scope of #285.** Refactor `useNotificationStream.svelte.ts` (factory) → `$lib/stores/notifications.svelte.ts` (module-level singleton). Idempotent `init()` (ref-counted), ref-counted `destroy()`. Bell and Chronik Für-dich box both import the singleton. **One `EventSource` per tab, always.** Old file deleted — no zombie code. - **Paraglide pluralization.** No ICU `match` expressions. Split into **`chronik_singleton_X`** and **`chronik_rollup_X`** key pairs per verb. Component branches on `count === 1`: ``` chronik_singleton_text_saved: "{actor} transkribierte einen Block in {doc}" chronik_rollup_text_saved: "{actor} transkribierte {count} Blöcke in {doc}" chronik_singleton_uploaded: "{actor} lud ein Dokument hoch" chronik_rollup_uploaded: "{actor} lud {count} Dokumente hoch" ... (reviewed, annotated: same pair pattern) chronik_singleton_comment_added, chronik_singleton_mention_created — single keys, never rolled up. ``` Matches the project's existing pragmatic precedent (e.g. `"vor {count} Minute(n)"`). Translators need no grammatical plural rules. - **Day-bucket helper — `$lib/utils/date-buckets.ts`.** Pure function `bucketByDay(date, now?, locale?) → DayBucket` + `getBucketLabel(bucket)`. Co-located `.spec.ts` covers midnight boundary, Monday-start-of-week (de/es), Sunday-start (en-US), DST transition, and "older" threshold. Pure function = clean unit tests without component harness friction. - **Existing dedupe tests — audit result: nothing to delete.** The only existing test (`findDedupedActivityFeed_returnsAnnotationEntry`) is a one-row smoke test that doesn't exercise dedupe behavior. Renamed to `findRolledUpActivityFeed_returnsAnnotationEntry`, strengthened with `count == 1` + `happenedAtUntil == null` assertions. Six new rollup-behavior tests added on top (covering combine, split, long-session-no-cap, COMMENT/MENTION never-rolled-up, DTO exposure). `DashboardActivityFeed.svelte.spec.ts` gets new singleton + rollup render cases added — purely additive. ### Skipped / unresolved None. ### Side effects - **#286** created — `refactor(notification-bell): use SvelteKit form actions instead of raw fetch`. Blocked by this issue; addresses the same form-action convention on the Bell to prevent divergence. ### Overall read The design was already implementable before this discussion — but six dev-level unknowns would each have cost a ~20-minute decision during TDD. Resolving them up front means I write failing tests first without stopping to re-read the spec or poke at Paraglide docs. The highest-value call was the SSE singleton (Path X): shipping two `EventSource` connections to production would've been a silent regression; catching it at design time costs one refactor commit. Ready to start red-phase.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#285