fix(documents): filter inputs don't sync with URL — Sender/Receiver blank on load, fields don't clear on reset #482

New Issue

marcel · 2026-05-08T12:14:41+02:00

marcel commented

2026-05-08 12:14:41 +02:00

Context

The advanced filter panel on /documents (SearchFilterBar.svelte, embedded in documents/+page.svelte) keeps its visible display text in component-local state that is initialised once and never re-synced from the bound value prop. The applied filter (URL query params + result list) and the visible filter inputs therefore drift apart in three observable ways:

Symptom 1 — Sender/Receiver blank when URL has IDs (initial load)

Apply a Sender filter on /documents, e.g. Anna Müller → URL becomes /documents?senderId=p-xxx.
Copy the URL, open it in a new tab.
Expand "Filter".

Expected: Sender input shows Anna Müller.
Actual: Sender input is empty even though the result list is correctly restricted to that sender. Same for Receiver with ?receiverId=....

Symptom 2 — Reset (×) leaves stale Sender/Receiver text

On /documents, expand "Filter", select Sender = Anna Müller, Receiver = Peter Schmidt.
Click the × icon (right of the Filter button).

Expected: Sender and Receiver fields go blank.
Actual: URL clears to /documents and the result list refreshes, but both typeahead fields keep displaying the previously selected names.

Symptom 3 — Reset (×) leaves stale From/To text

Same setup, with From = 01.01.1920 and To = 31.12.1930.
Click the × icon.

Expected: From and To fields go blank.
Actual: URL and results clear, but both date fields keep displaying the typed German dates.

(Initial-load URL→UI for from / to works today because DateInput initialises display from value at mount — only subsequent prop changes are missed.)

Root cause

Three concrete points, all the same anti-pattern (display state initialised once, never re-derived from the bound value prop):

frontend/src/routes/documents/+page.svelte:238-254 binds senderId / receiverId (IDs) to SearchFilterBar but does not pass initialSenderName / initialReceiverName. The receiving PersonTypeahead therefore always sees initialName="".
frontend/src/lib/person/PersonTypeahead.svelte:49,52-54 keeps searchTerm in sync only with initialName. Since that prop is constant (""), the visible name persists across navigations and resets and is never populated from a URL-supplied ID.
frontend/src/lib/shared/primitives/DateInput.svelte:25 does let display = $state(isoToGerman(value ?? '')) exactly once. There is no $effect re-deriving display when the bound value prop changes externally (reset, timeline drag in TimelineDensityFilter, future programmatic updates).

Approach

Sender / Receiver

In documents/+page.server.ts, when senderId / receiverId query params are non-empty, resolve the corresponding Person.displayName server-side (single GET /api/persons/{id} per id is fine; tolerate 404 by returning empty string and clearing the param, or just return empty name). Return as initialSenderName / initialReceiverName.
In documents/+page.svelte, bind these to SearchFilterBar's initialSenderName / initialReceiverName props.
In PersonTypeahead.svelte, harden the existing $effect: also clear searchTerm when the bound value becomes empty while initialName is empty (covers reset). One pattern:
```
$effect(() => {
    if (!value) searchTerm = initialName; // covers reset and external-clear
});
```
Keep the existing init from initialName. Don't introduce loops with the handleInput value = '' reset path.

From / To

In DateInput.svelte, re-derive display whenever the bound value prop changes from outside (i.e. when value !== germanToIso(display)):
```
$effect(() => {
    const externalIso = value ?? '';
    if (germanToIso(display) !== externalIso) {
        display = isoToGerman(externalIso);
    }
});
```
The guard prevents the user's own keystrokes (which set value from display) from re-overwriting display.

Critical files

frontend/src/routes/documents/+page.server.ts — resolve names from IDs
frontend/src/routes/documents/+page.svelte — pass initialSenderName / initialReceiverName to SearchFilterBar
frontend/src/lib/person/PersonTypeahead.svelte — clear searchTerm when bound value is cleared externally
frontend/src/lib/shared/primitives/DateInput.svelte — re-derive display from external value changes

Acceptance criteria

AC1 — Loading /documents?senderId={id} directly shows the corresponding person's display name in the Sender input. The Filter panel auto-expands as today.
AC2 — Loading /documents?receiverId={id} directly shows the corresponding person's display name in the Receiver input.
AC3 — Loading /documents?senderId={non-existent-id} shows the input empty (not "undefined" or an error) and the result list as if no sender filter were applied.
AC4 — Clicking the × reset icon on /documents (with any combination of Sender / Receiver / From / To set) clears all four input fields visually, in addition to clearing the URL and refreshing results.
AC5 — When the timeline density filter (TimelineDensityFilter) commits a new range via drag, the From and To input fields display the new German-format dates.
AC6 — Existing typing flows are unaffected: typing 01.01.1920 into From still updates the URL to ?from=1920-01-01 and the typed value remains visible while the user types (no flicker / cursor-jump from the new $effect).
AC7 — Component tests cover AC1-AC6 in frontend/src/routes/documents/page.svelte.spec.ts (and/or PersonTypeahead.svelte.spec.ts, DateInput.svelte.spec.ts as appropriate).

Verification

cd frontend && npm run check — green.
cd frontend && npm run test — green (new specs included).
Manual / Playwright walkthrough as admin@familyarchive.local:
- Apply Sender + Receiver + From + To, copy URL, open in new tab → all four inputs populated.
- Click × → all four inputs empty.
- Drag the timeline density filter to a new range → From / To inputs reflect the dragged range.

Risk if not addressed

P1: confusing UX. URL-sharing of a filtered view appears broken (recipient sees empty inputs and a filtered list, looks like a stale page). Reset is also broken — users see "old text" linger after clearing, eroding trust in the filter UI. This is one of the most-used screens in the app.

Out of scope

Tag-filter URL/UI sync (works today via reactive tagNames array).
Restyling or repositioning the reset button.
Any change to the search API contract.

## Context The advanced filter panel on `/documents` (`SearchFilterBar.svelte`, embedded in `documents/+page.svelte`) keeps its visible **display text** in component-local state that is initialised once and never re-synced from the bound `value` prop. The applied filter (URL query params + result list) and the visible filter inputs therefore drift apart in three observable ways: ### Symptom 1 — Sender/Receiver blank when URL has IDs (initial load) 1. Apply a Sender filter on `/documents`, e.g. `Anna Müller` → URL becomes `/documents?senderId=p-xxx`. 2. Copy the URL, open it in a new tab. 3. Expand "Filter". **Expected**: Sender input shows `Anna Müller`. **Actual**: Sender input is empty even though the result list is correctly restricted to that sender. Same for Receiver with `?receiverId=...`. ### Symptom 2 — Reset (×) leaves stale Sender/Receiver text 1. On `/documents`, expand "Filter", select Sender = `Anna Müller`, Receiver = `Peter Schmidt`. 2. Click the **×** icon (right of the Filter button). **Expected**: Sender and Receiver fields go blank. **Actual**: URL clears to `/documents` and the result list refreshes, but both typeahead fields keep displaying the previously selected names. ### Symptom 3 — Reset (×) leaves stale From/To text 1. Same setup, with `From = 01.01.1920` and `To = 31.12.1930`. 2. Click the **×** icon. **Expected**: From and To fields go blank. **Actual**: URL and results clear, but both date fields keep displaying the typed German dates. (Initial-load URL→UI for `from` / `to` works today because `DateInput` initialises `display` from `value` at mount — only subsequent prop changes are missed.) ## Root cause Three concrete points, all the same anti-pattern (display state initialised once, never re-derived from the bound `value` prop): - **`frontend/src/routes/documents/+page.svelte:238-254`** binds `senderId` / `receiverId` (IDs) to `SearchFilterBar` but does **not** pass `initialSenderName` / `initialReceiverName`. The receiving `PersonTypeahead` therefore always sees `initialName=""`. - **`frontend/src/lib/person/PersonTypeahead.svelte:49,52-54`** keeps `searchTerm` in sync only with `initialName`. Since that prop is constant (`""`), the visible name persists across navigations and resets and is never populated from a URL-supplied ID. - **`frontend/src/lib/shared/primitives/DateInput.svelte:25`** does `let display = $state(isoToGerman(value ?? ''))` exactly once. There is no `$effect` re-deriving `display` when the bound `value` prop changes externally (reset, timeline drag in `TimelineDensityFilter`, future programmatic updates). ## Approach ### Sender / Receiver 1. In `documents/+page.server.ts`, when `senderId` / `receiverId` query params are non-empty, resolve the corresponding `Person.displayName` server-side (single `GET /api/persons/{id}` per id is fine; tolerate 404 by returning empty string and clearing the param, or just return empty name). Return as `initialSenderName` / `initialReceiverName`. 2. In `documents/+page.svelte`, bind these to `SearchFilterBar`'s `initialSenderName` / `initialReceiverName` props. 3. In `PersonTypeahead.svelte`, harden the existing `$effect`: also clear `searchTerm` when the bound `value` becomes empty while `initialName` is empty (covers reset). One pattern: ```svelte $effect(() => { if (!value) searchTerm = initialName; // covers reset and external-clear }); ``` Keep the existing init from `initialName`. Don't introduce loops with the `handleInput` `value = ''` reset path. ### From / To 4. In `DateInput.svelte`, re-derive `display` whenever the bound `value` prop changes from outside (i.e. when `value !== germanToIso(display)`): ```svelte $effect(() => { const externalIso = value ?? ''; if (germanToIso(display) !== externalIso) { display = isoToGerman(externalIso); } }); ``` The guard prevents the user's own keystrokes (which set `value` from `display`) from re-overwriting `display`. ## Critical files - `frontend/src/routes/documents/+page.server.ts` — resolve names from IDs - `frontend/src/routes/documents/+page.svelte` — pass `initialSenderName` / `initialReceiverName` to `SearchFilterBar` - `frontend/src/lib/person/PersonTypeahead.svelte` — clear `searchTerm` when bound `value` is cleared externally - `frontend/src/lib/shared/primitives/DateInput.svelte` — re-derive `display` from external `value` changes ## Acceptance criteria - [ ] **AC1** — Loading `/documents?senderId={id}` directly shows the corresponding person's display name in the Sender input. The Filter panel auto-expands as today. - [ ] **AC2** — Loading `/documents?receiverId={id}` directly shows the corresponding person's display name in the Receiver input. - [ ] **AC3** — Loading `/documents?senderId={non-existent-id}` shows the input empty (not "undefined" or an error) and the result list as if no sender filter were applied. - [ ] **AC4** — Clicking the **×** reset icon on `/documents` (with any combination of Sender / Receiver / From / To set) clears all four input fields visually, in addition to clearing the URL and refreshing results. - [ ] **AC5** — When the timeline density filter (`TimelineDensityFilter`) commits a new range via drag, the From and To input fields display the new German-format dates. - [ ] **AC6** — Existing typing flows are unaffected: typing `01.01.1920` into From still updates the URL to `?from=1920-01-01` and the typed value remains visible while the user types (no flicker / cursor-jump from the new `$effect`). - [ ] **AC7** — Component tests cover AC1-AC6 in `frontend/src/routes/documents/page.svelte.spec.ts` (and/or `PersonTypeahead.svelte.spec.ts`, `DateInput.svelte.spec.ts` as appropriate). ## Verification 1. `cd frontend && npm run check` — green. 2. `cd frontend && npm run test` — green (new specs included). 3. Manual / Playwright walkthrough as `admin@familyarchive.local`: - Apply Sender + Receiver + From + To, copy URL, open in new tab → all four inputs populated. - Click × → all four inputs empty. - Drag the timeline density filter to a new range → From / To inputs reflect the dragged range. ## Risk if not addressed P1: confusing UX. URL-sharing of a filtered view appears broken (recipient sees empty inputs and a filtered list, looks like a stale page). Reset is also broken — users see "old text" linger after clearing, eroding trust in the filter UI. This is one of the most-used screens in the app. ## Out of scope - Tag-filter URL/UI sync (works today via reactive `tagNames` array). - Restyling or repositioning the reset button. - Any change to the search API contract.

marcel added the P1-high bug ui labels 2026-05-08 12:14:49 +02:00

marcel commented

2026-05-08 16:00:46 +02:00

🏛️ Markus Keller — Senior Application Architect

Observations

This is a state-ownership question, not a layering one. Three components keep a private "display" copy of an externally-bound value; none of them re-derive on prop change. Same anti-pattern, three places — exactly what an ADR exists to prevent next time.
PersonTypeahead (frontend/src/lib/person/PersonTypeahead.svelte:52-54) already has $effect(() => { searchTerm = initialName; }). So the initial-load fix collapses to "feed the prop" — +page.server.ts resolves the displayName, parent passes it in, existing effect fires. No new effect needed for AC1/AC2.
The reset case is the exception: when initialName was '' already (typeahead-selected sender, never came from URL) and the user clicks reset, initialName doesn't change, so the existing effect doesn't re-fire. That's the only case the proposed if (!value) searchTerm = initialName adds value for — at the cost of a new feedback loop with handleInput's value = '' write (Felix will detail).
+page.server.ts does its own param parsing inline (whitelist for sort, fallback for dir, etc.). Adding two GET /api/persons/{id} calls per request is fine; it's the same pattern the rest of the file uses.

Recommendations

Resolve sender/receiver names server-side in +page.server.ts, in parallel with the existing search call (Promise.all). Tolerate 404 by returning '' and clearing the param from the returned data — the user lands on a clean URL, not ?senderId=garbage.
Don't add a new $effect to PersonTypeahead. Make the parent the source of truth: always pass initialName derived from senderId. If senderId === '', initialName === '', and the existing effect already resets searchTerm. This removes the AC6 risk entirely.
Handle the "user-typeahead-selected, then reset" gap differently. When the SvelteKit navigation completes after reset, the parent's senderId flips to ''. A $effect in the parent (or in SearchFilterBar) that watches senderId going to empty can re-pass a new initialName='' — even setting it to a sentinel string like a nav-counter key — to force the existing typeahead effect to re-fire. Cleaner than adding a second effect inside the leaf component.
For DateInput, the proposed $effect with the germanToIso(display) !== externalIso guard is correct and stays inside the component — no architectural concern. Document the why in a one-line comment ("re-derive display on external value change without clobbering in-flight typing"), since this is the kind of code reviewers will want to delete in 6 months.
No new ADR required. This is a bug fix, not a structural decision. But: if you find yourself adding a fourth bound-prop-vs-display-state component, that is an ADR moment ("two-way bound primitives must re-derive display from value on external change") and a candidate for a shared pattern.

Open Decisions

Where does the displayName-from-id resolve happen — in +page.server.ts or in a shared loader helper? The Briefwechsel and Aktivitäten routes will eventually face the same URL-share problem with person filters. A resolvePersonName(fetch, id) helper in $lib/person/server.ts costs ~10 lines and gets reused; inline costs nothing today but pays for the next two callers. Either is defensible — depends on whether issue 482 is followed by similar URL-share fixes elsewhere.

## 🏛️ Markus Keller — Senior Application Architect ### Observations - This is a state-ownership question, not a layering one. Three components keep a private "display" copy of an externally-bound value; none of them re-derive on prop change. Same anti-pattern, three places — exactly what an ADR exists to prevent next time. - `PersonTypeahead` (`frontend/src/lib/person/PersonTypeahead.svelte:52-54`) already has `$effect(() => { searchTerm = initialName; })`. So the *initial-load* fix collapses to "feed the prop" — `+page.server.ts` resolves the displayName, parent passes it in, existing effect fires. No new effect needed for AC1/AC2. - The reset case is the exception: when `initialName` was `''` already (typeahead-selected sender, never came from URL) and the user clicks reset, `initialName` doesn't change, so the existing effect doesn't re-fire. That's the only case the proposed `if (!value) searchTerm = initialName` adds value for — at the cost of a new feedback loop with `handleInput`'s `value = ''` write (Felix will detail). - `+page.server.ts` does its own param parsing inline (whitelist for `sort`, fallback for `dir`, etc.). Adding two `GET /api/persons/{id}` calls per request is fine; it's the same pattern the rest of the file uses. ### Recommendations - **Resolve sender/receiver names server-side** in `+page.server.ts`, in parallel with the existing search call (`Promise.all`). Tolerate 404 by returning `''` and clearing the param from the returned data — the user lands on a clean URL, not `?senderId=garbage`. - **Don't add a new $effect to `PersonTypeahead`.** Make the parent the source of truth: always pass `initialName` derived from `senderId`. If `senderId === ''`, `initialName === ''`, and the existing effect already resets `searchTerm`. This removes the AC6 risk entirely. - **Handle the "user-typeahead-selected, then reset" gap differently.** When the SvelteKit navigation completes after reset, the parent's `senderId` flips to `''`. A `$effect` in the *parent* (or in `SearchFilterBar`) that watches `senderId` going to empty can re-pass a new `initialName=''` — even setting it to a sentinel string like a nav-counter key — to force the existing typeahead effect to re-fire. Cleaner than adding a second effect inside the leaf component. - **For `DateInput`**, the proposed `$effect` with the `germanToIso(display) !== externalIso` guard is correct and stays inside the component — no architectural concern. Document the *why* in a one-line comment ("re-derive display on external value change without clobbering in-flight typing"), since this is the kind of code reviewers will want to delete in 6 months. - **No new ADR required.** This is a bug fix, not a structural decision. But: if you find yourself adding a fourth bound-prop-vs-display-state component, that *is* an ADR moment ("two-way bound primitives must re-derive display from value on external change") and a candidate for a shared pattern. ### Open Decisions - **Where does the displayName-from-id resolve happen — in `+page.server.ts` or in a shared loader helper?** The Briefwechsel and Aktivitäten routes will eventually face the same URL-share problem with person filters. A `resolvePersonName(fetch, id)` helper in `$lib/person/server.ts` costs ~10 lines and gets reused; inline costs nothing today but pays for the next two callers. Either is defensible — depends on whether issue 482 is followed by similar URL-share fixes elsewhere.

marcel commented

2026-05-08 16:01:10 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

PersonTypeahead.svelte already has the effect the issue proposes adding — line 52-54 sets searchTerm = initialName whenever initialName changes. The eslint-disable comment above it explicitly documents why it's $state + $effect and not $derived.
handleInput (line 100-108) does value = '' when the user types in a field that previously held a selected ID. That write becomes a tracked dep of any effect that reads value.
+page.server.ts already calls api.GET with the typed client and the right error handling. Adding two more GET /api/persons/{id} calls fits cleanly; bonus points for Promise.all so they don't serialize behind the search call.
Specs already exist: PersonTypeahead.svelte.spec.ts, DateInput.svelte.spec.ts, page.svelte.spec.ts. No new test infrastructure needed — TDD path is straight.

Recommendations

The proposed PersonTypeahead effect has a typing-loop bug. Do not ship this version:
```
$effect(() => {
    if (!value) searchTerm = initialName; // ← wipes user typing
});
```
Trace: user types "A" → bind:value={searchTerm} makes searchTerm = 'A' → handleInput runs → value = '' → effect re-runs (it reads value) → searchTerm = initialName → user sees their "A" wiped. AC6 fails.
Prefer the parent-driven approach. Always pass initialName derived from senderId. The existing effect at line 52-54 then handles AC1, AC2, AC4 (when reset clears senderId → parent recomputes initialName='' → effect fires → display clears). No new effect inside PersonTypeahead. This is also fewer lines than the issue's plan.
For the "user-typeahead-selected, then reset" gap (where initialName was '' before and after reset, so the effect doesn't re-fire), bump a key on every navigation. Easiest: pass a resetKey={data.url ?? Math.random()} prop and read it inside the existing effect: $effect(() => { void resetKey; searchTerm = initialName; });. Honest, no loop.
DateInput fix is fine as proposed. The germanToIso(display) !== externalIso guard correctly stops the round-trip. Add the test for typing 01.01.1920 (AC6 for dates) — write it red against main first to confirm there's no flicker risk.
TDD order — write these red first:
1. PersonTypeahead.svelte.spec.ts — "external value clear resets the input display"
2. PersonTypeahead.svelte.spec.ts — "typing into a field with a selected id keeps the typed text" (this is the one that catches the loop bug)
3. DateInput.svelte.spec.ts — "external value change re-derives display"
4. DateInput.svelte.spec.ts — "typing keeps the typed text while value updates"
5. page.svelte.spec.ts — "loads with senderId param and shows the resolved person name"
6. page.svelte.spec.ts — "reset clears all four inputs"

Server-side name resolution: keep it parallel to the search:

const [searchResult, senderName, receiverName] = await Promise.all([
    api.GET('/api/documents/search', { ... }),
    senderId ? resolvePersonName(api, senderId) : Promise.resolve(''),
    receiverId ? resolvePersonName(api, receiverId) : Promise.resolve('')
]);

On 404, return ''. Don't try to clear the param mid-load — let the search return what it returns.

Don't add a comment that says "covers reset and external-clear" — name the effect's intent through code structure, or expand the existing comment block at line 51 to mention the new contract.
AC7 should be split. Tests for PersonTypeahead and DateInput belong in their own spec files (those exist); page.svelte.spec.ts covers the integration (sender name resolved server-side).

Open Decisions

(none — all items above have a clear correct answer)

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - `PersonTypeahead.svelte` already has the effect the issue proposes adding — line 52-54 sets `searchTerm = initialName` whenever `initialName` changes. The eslint-disable comment above it explicitly documents why it's `$state + $effect` and not `$derived`. - `handleInput` (line 100-108) does `value = ''` when the user types in a field that previously held a selected ID. That write becomes a tracked dep of any effect that reads `value`. - `+page.server.ts` already calls `api.GET` with the typed client and the right error handling. Adding two more `GET /api/persons/{id}` calls fits cleanly; bonus points for `Promise.all` so they don't serialize behind the search call. - Specs already exist: `PersonTypeahead.svelte.spec.ts`, `DateInput.svelte.spec.ts`, `page.svelte.spec.ts`. No new test infrastructure needed — TDD path is straight. ### Recommendations - **The proposed `PersonTypeahead` effect has a typing-loop bug. Do not ship this version:** ```svelte $effect(() => { if (!value) searchTerm = initialName; // ← wipes user typing }); ``` Trace: user types "A" → `bind:value={searchTerm}` makes `searchTerm = 'A'` → `handleInput` runs → `value = ''` → effect re-runs (it reads `value`) → `searchTerm = initialName` → user sees their "A" wiped. AC6 fails. - **Prefer the parent-driven approach.** Always pass `initialName` derived from `senderId`. The existing effect at line 52-54 then handles AC1, AC2, AC4 (when reset clears `senderId` → parent recomputes `initialName=''` → effect fires → display clears). No new effect inside `PersonTypeahead`. This is also fewer lines than the issue's plan. - **For the "user-typeahead-selected, then reset" gap** (where `initialName` was `''` before *and* after reset, so the effect doesn't re-fire), bump a key on every navigation. Easiest: pass a `resetKey={data.url ?? Math.random()}` prop and read it inside the existing effect: `$effect(() => { void resetKey; searchTerm = initialName; });`. Honest, no loop. - **`DateInput` fix is fine as proposed.** The `germanToIso(display) !== externalIso` guard correctly stops the round-trip. Add the test for typing `01.01.1920` (AC6 for dates) — write it red against `main` first to confirm there's no flicker risk. - **TDD order — write these red first:** 1. `PersonTypeahead.svelte.spec.ts` — "external value clear resets the input display" 2. `PersonTypeahead.svelte.spec.ts` — "typing into a field with a selected id keeps the typed text" (this is the one that catches the loop bug) 3. `DateInput.svelte.spec.ts` — "external value change re-derives display" 4. `DateInput.svelte.spec.ts` — "typing keeps the typed text while value updates" 5. `page.svelte.spec.ts` — "loads with senderId param and shows the resolved person name" 6. `page.svelte.spec.ts` — "reset clears all four inputs" - **Server-side name resolution**: keep it parallel to the search: ```typescript const [searchResult, senderName, receiverName] = await Promise.all([ api.GET('/api/documents/search', { ... }), senderId ? resolvePersonName(api, senderId) : Promise.resolve(''), receiverId ? resolvePersonName(api, receiverId) : Promise.resolve('') ]); ``` On 404, return `''`. Don't try to clear the param mid-load — let the search return what it returns. - **Don't add a comment that says "covers reset and external-clear"** — name the effect's intent through code structure, or expand the existing comment block at line 51 to mention the new contract. - **AC7 should be split.** Tests for `PersonTypeahead` and `DateInput` belong in their own spec files (those exist); `page.svelte.spec.ts` covers the integration (sender name resolved server-side). ### Open Decisions _(none — all items above have a clear correct answer)_

marcel commented

2026-05-08 16:01:34 +02:00

🧪 Sara Holt — Senior QA Engineer

Observations

The acceptance criteria are testable; AC6 ("typed value remains visible while the user types — no flicker / cursor-jump from the new $effect") is exactly the regression that the proposed PersonTypeahead patch risks introducing. Good that it's named — the test must be written before the fix.
All three target spec files exist: PersonTypeahead.svelte.spec.ts, DateInput.svelte.spec.ts, page.svelte.spec.ts — no new harness work.
+page.server.ts does a graceful try/catch fallback on the search call. The new person-name resolution must follow the same shape (one bad lookup ≠ broken page).
TipTap memory: from prior work in this codebase, vitest-browser bind:value updates need explicit dispatchEvent(new Event('input')) to fire oninput handlers. DateInput uses oninput, so its tests must dispatch input events, not just set .value.

Recommendations

Add three regression tests that the issue does not call out explicitly:
1. PersonTypeahead.svelte.spec.ts — "typing 'A' into a field with a selected sender does not wipe the typed character on the next tick" — directly catches the proposed-fix typing loop (assert input.value === 'A' after a microtask, plus a 50ms await to let the effect run).
2. DateInput.svelte.spec.ts — "typing while value is bound does not flicker the display" — type 01.01.1920 keystroke by keystroke, assert intermediate displays 0, 01, 01., 01.0, ... never get overwritten.
3. page.svelte.spec.ts — "unknown senderId clears the param without breaking the page" — server returns empty initialSenderName, page renders, no error toast.
AC3 is underspecified. "Shows the input empty" — fine. "Result list as if no sender filter were applied" — verify: does the spec want the unknown ID to be silently dropped at the server (recommended), or passed through to the API which then returns 0 docs? The two paths are observably different; pick one and lock it with a test.
AC5 (timeline density drag) needs explicit coverage. Use vitest-browser to drag the density filter range and assert German-format dates appear in From/To. This is the symptom that the issue-as-written might miss in CI even after the fix lands, because no current test exercises that drag → display path.
AC4 must assert all four inputs in one test, not four separate tests. The bug is "any combination" — a test that only sets sender and clicks reset would have passed against main for the date fields (DateInput initial-load worked, only re-derive was broken). Set all four, click reset, assert all four blank in one assertion block.
Quality gates check: npm run check and npm run test are listed in §Verification — also run npm run lint (Prettier picks up the new $effect indentation) and npm run test:e2e -- documents if there's a Playwright suite covering the documents page (verify before merging).
Do not skip the Playwright walkthrough. This is a UX-trust bug; test with real navigation (copy URL → open new tab) at least once, headed, before claiming AC1/AC2 done. Browser-mode component tests don't fully exercise SvelteKit navigation re-renders.
No flaky tolerance. If any of these new tests is flaky on first run, fix or quarantine with a ticket — do not merge with .skip().

Open Decisions

AC3 server behaviour: silently drop unknown senderId (cleaner URL, better UX) vs. pass through and return 0 docs (preserves user intent to come back to a re-typed name). Worth a one-line decision before tests are written.

## 🧪 Sara Holt — Senior QA Engineer ### Observations - The acceptance criteria are testable; AC6 ("typed value remains visible while the user types — no flicker / cursor-jump from the new `$effect`") is exactly the regression that the proposed `PersonTypeahead` patch risks introducing. Good that it's named — the test must be written before the fix. - All three target spec files exist: `PersonTypeahead.svelte.spec.ts`, `DateInput.svelte.spec.ts`, `page.svelte.spec.ts` — no new harness work. - `+page.server.ts` does a graceful try/catch fallback on the search call. The new person-name resolution must follow the same shape (one bad lookup ≠ broken page). - TipTap memory: from prior work in this codebase, `vitest-browser` `bind:value` updates need explicit `dispatchEvent(new Event('input'))` to fire `oninput` handlers. `DateInput` uses `oninput`, so its tests must dispatch input events, not just set `.value`. ### Recommendations - **Add three regression tests that the issue does not call out explicitly:** 1. `PersonTypeahead.svelte.spec.ts` — *"typing 'A' into a field with a selected sender does not wipe the typed character on the next tick"* — directly catches the proposed-fix typing loop (assert `input.value === 'A'` after a microtask, plus a 50ms `await` to let the effect run). 2. `DateInput.svelte.spec.ts` — *"typing while value is bound does not flicker the display"* — type `01.01.1920` keystroke by keystroke, assert intermediate displays `0`, `01`, `01.`, `01.0`, ... never get overwritten. 3. `page.svelte.spec.ts` — *"unknown senderId clears the param without breaking the page"* — server returns empty `initialSenderName`, page renders, no error toast. - **AC3 is underspecified.** "Shows the input empty" — fine. "Result list as if no sender filter were applied" — verify: does the spec want the unknown ID to be silently dropped at the server (recommended), or passed through to the API which then returns 0 docs? The two paths are observably different; pick one and lock it with a test. - **AC5 (timeline density drag) needs explicit coverage.** Use `vitest-browser` to drag the density filter range and assert German-format dates appear in From/To. This is the symptom that the issue-as-written might miss in CI even after the fix lands, because no current test exercises that drag → display path. - **AC4 must assert all four inputs in one test, not four separate tests.** The bug is "any combination" — a test that only sets sender and clicks reset would have passed against `main` for the date fields (DateInput initial-load worked, only re-derive was broken). Set all four, click reset, assert all four blank in one assertion block. - **Quality gates check**: `npm run check` and `npm run test` are listed in §Verification — also run `npm run lint` (Prettier picks up the new $effect indentation) and `npm run test:e2e -- documents` if there's a Playwright suite covering the documents page (verify before merging). - **Do not skip the Playwright walkthrough.** This is a UX-trust bug; test with real navigation (`copy URL → open new tab`) at least once, headed, before claiming AC1/AC2 done. Browser-mode component tests don't fully exercise SvelteKit navigation re-renders. - **No flaky tolerance.** If any of these new tests is flaky on first run, fix or quarantine with a ticket — do not merge with `.skip()`. ### Open Decisions - **AC3 server behaviour**: silently drop unknown `senderId` (cleaner URL, better UX) vs. pass through and return 0 docs (preserves user intent to come back to a re-typed name). Worth a one-line decision before tests are written.

marcel commented

2026-05-08 16:01:52 +02:00

🛡️ Nora Steiner — Application Security

Observations

+page.server.ts reads senderId and receiverId directly from url.searchParams and forwards them to the typed API client as query params. The typed client uses openapi-fetch, which URL-encodes properly — no SQLi or query-string injection vector.
The new code path will issue GET /api/persons/{id} with a user-controlled id. The backend PersonController.getById is annotated with @RequirePermission(Permission.READ_ALL) (verified pattern across the codebase) and the cookie-forwarded createApiClient(fetch) carries the auth header — no IDOR exposure beyond what already exists.
404 leakage: telling the user "this senderId doesn't exist" is fine for this app (every authenticated user has READ_ALL, no per-tenant isolation). Worth a sentence in the PR description so this assumption is explicit and not silently inherited if RLS or per-user scoping ever lands.
Reset link is <a href="/documents"> — a same-origin GET, no CSRF concern.

Recommendations

Use the typed client for the name lookup, not raw fetch. api.GET('/api/persons/{id}', { params: { path: { id } } }) — the id flows through the OpenAPI-generated path-param encoder, which is the only injection-safe form. Avoid string templating like fetch(`/api/persons/${senderId}`).
Treat 404 as "param invalid, drop it." Do not surface the backend error message to the page. Return initialSenderName: '' and proceed. This is consistent with the rest of the codebase's getErrorMessage(code) philosophy — no class names, no SQL, no stack traces reach the user.
Validate the UUID shape before issuing the lookup. Saves one round-trip and avoids a backend error log per malformed ?senderId=garbage. Cheap regex check at the top of the load function: /^[0-9a-f-]{36}$/i. If it fails, treat the same as 404 (param dropped, name '').
Don't log raw query params at INFO. The pattern logger.info("senderId={}", senderId) is fine — SLF4J parameterized — but if anyone is tempted to add an error("Failed for " + url) style during debugging, that's a Log4Shell-shaped foot-gun. Keep parameterized form throughout.
Regression tests for the lookup path: at least one test that ?senderId={non-uuid} returns 200 with empty inputs and the search results unaffected. This is a regression test for a security smell (input validation at the boundary), even though it's primarily a UX guard.

Open Decisions

(none — input validation, error handling, and 404 treatment all have clear correct answers from the existing codebase patterns)

## 🛡️ Nora Steiner — Application Security ### Observations - `+page.server.ts` reads `senderId` and `receiverId` directly from `url.searchParams` and forwards them to the typed API client as query params. The typed client uses `openapi-fetch`, which URL-encodes properly — no SQLi or query-string injection vector. - The new code path will issue `GET /api/persons/{id}` with a user-controlled `id`. The backend `PersonController.getById` is annotated with `@RequirePermission(Permission.READ_ALL)` (verified pattern across the codebase) and the cookie-forwarded `createApiClient(fetch)` carries the auth header — no IDOR exposure beyond what already exists. - 404 leakage: telling the user "this senderId doesn't exist" is fine for *this* app (every authenticated user has READ_ALL, no per-tenant isolation). Worth a sentence in the PR description so this assumption is explicit and not silently inherited if RLS or per-user scoping ever lands. - Reset link is `<a href="/documents">` — a same-origin GET, no CSRF concern. ### Recommendations - **Use the typed client for the name lookup, not raw `fetch`.** `api.GET('/api/persons/{id}', { params: { path: { id } } })` — the `id` flows through the OpenAPI-generated path-param encoder, which is the only injection-safe form. Avoid string templating like `` fetch(`/api/persons/${senderId}`) ``. - **Treat 404 as "param invalid, drop it."** Do not surface the backend error message to the page. Return `initialSenderName: ''` and proceed. This is consistent with the rest of the codebase's `getErrorMessage(code)` philosophy — no class names, no SQL, no stack traces reach the user. - **Validate the UUID shape before issuing the lookup.** Saves one round-trip and avoids a backend error log per malformed `?senderId=garbage`. Cheap regex check at the top of the load function: `/^[0-9a-f-]{36}$/i`. If it fails, treat the same as 404 (param dropped, name `''`). - **Don't log raw query params at INFO.** The pattern `logger.info("senderId={}", senderId)` is fine — SLF4J parameterized — but if anyone is tempted to add an `error("Failed for " + url)` style during debugging, that's a Log4Shell-shaped foot-gun. Keep parameterized form throughout. - **Regression tests for the lookup path**: at least one test that `?senderId={non-uuid}` returns 200 with empty inputs and the search results unaffected. This is a regression test for a security smell (input validation at the boundary), even though it's primarily a UX guard. ### Open Decisions _(none — input validation, error handling, and 404 treatment all have clear correct answers from the existing codebase patterns)_

marcel commented

2026-05-08 16:02:03 +02:00

⚙️ Tobias Wendt — DevOps & Platform Engineer

Observations

Pure frontend bug fix — no Compose, no infrastructure, no CI workflow, no env vars.
The two new GET /api/persons/{id} calls per /documents page load are negligible at the current scale (single-VPS, family-sized user base). Each is a primary-key lookup; PostgreSQL handles it in <2ms warm.
One thing to watch: +page.server.ts is on the SSR critical path. Adding latency here delays first paint.

Recommendations

Run the two name lookups in Promise.all with the search call, not serially. Sequential adds 4–10ms per request to TTFB; parallel adds zero (capped by the slowest dep).
No backend Caddy/firewall changes needed. /api/persons/{id} is already a permitted authenticated route.
Smoke-test the existing CI run after merge. The frontend/ Vitest suite picks up new specs automatically — no workflow changes. Confirm frontend-test and frontend-e2e jobs (if present) stay green.
Watch for SSR error rate post-deploy. If the backend /api/persons/{id} ever 500s (unlikely for a primary-key lookup, but), the page must still render — the issue's plan tolerates 404 by returning empty name; ensure 5xx is also caught and degraded the same way, otherwise a backend hiccup blanks the whole /documents page.

Open Decisions

(none — operational impact is near zero)

## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Observations - Pure frontend bug fix — no Compose, no infrastructure, no CI workflow, no env vars. - The two new `GET /api/persons/{id}` calls per `/documents` page load are negligible at the current scale (single-VPS, family-sized user base). Each is a primary-key lookup; PostgreSQL handles it in <2ms warm. - One thing to watch: `+page.server.ts` is on the SSR critical path. Adding latency here delays first paint. ### Recommendations - **Run the two name lookups in `Promise.all` with the search call**, not serially. Sequential adds 4–10ms per request to TTFB; parallel adds zero (capped by the slowest dep). - **No backend Caddy/firewall changes needed.** `/api/persons/{id}` is already a permitted authenticated route. - **Smoke-test the existing CI run after merge.** The `frontend/` Vitest suite picks up new specs automatically — no workflow changes. Confirm `frontend-test` and `frontend-e2e` jobs (if present) stay green. - **Watch for SSR error rate post-deploy.** If the backend `/api/persons/{id}` ever 500s (unlikely for a primary-key lookup, but), the page must still render — the issue's plan tolerates 404 by returning empty name; ensure 5xx is also caught and degraded the same way, otherwise a backend hiccup blanks the whole `/documents` page. ### Open Decisions _(none — operational impact is near zero)_

marcel commented

2026-05-08 16:02:31 +02:00

🎨 Leonie Voss — UX & Accessibility

Observations

This is exactly the kind of "stale state" trust bug that erodes confidence in a UI more than it costs in objective minutes lost. A senior researcher (60+ on a tablet) shares a filtered URL with a sibling, and the sibling sees "the field is empty but the list is filtered" — the inconsistency is harder to recover from than no filtering at all.
Reset (×) showing stale text is a Nielsen heuristic #1 violation (visibility of system status). The user tells the system "clear" and the system says "no" by leaving the text in place.
<a href="/documents"> for reset is the right semantic — it works without JS, is keyboard-reachable, screen readers announce it as a link, and back-button restores the prior filter state.
The current reset button is an icon-only <a> with title={m.docs_btn_reset_title()} and an icon marked aria-hidden="true". The link itself has no accessible name from text content, only the title attribute (which is unreliable for screen readers and invisible on touch). This is not in scope for this issue, but fixing the AC4 reset bug is a natural moment to also make this link labelled properly (separate ticket if needed).

Recommendations

Announce the reset to assistive tech. When AC4 lands and all four fields visibly clear, also fire an aria-live="polite" toast/region: "Filter zurückgesetzt" / "Filters cleared". A keyboard user with screen reader gets no signal today even after the fix; the screen reader announces a navigation, but not "your filter is now blank."
Add aria-label={m.docs_btn_reset_label()} to the reset <a> (separate from title). Right now the only accessible name is the title attribute, which iOS VoiceOver and Android TalkBack handle inconsistently. Tracked separately if you prefer — not blocking — but the in-scope test suite is the right moment to surface it.
For PersonTypeahead, after reset, the input should not retain aria-expanded="true" or any open dropdown. The current closeDropdown() is called via clickOutside, but a programmatic reset (URL nav) doesn't trigger that. Verify: after reset, the input is empty AND aria-expanded="false" AND no listbox is in the DOM. Add to the AC4 test.
Touch-target on the reset <a>: it's px-3 py-2.5 with a h-5 w-5 icon — that's roughly 36×30px hit area. Below the 44×44 WCAG 2.5.5 threshold. On a phone, missing this and accidentally hitting the Filter toggle next to it is exactly the failure mode that makes seniors give up. Bump to min-h-[44px] min-w-[44px] and add explicit aria-label. Outside the AC list as written but trivially in scope of "reset works for users."
Test in dark mode. The reset icon currently uses text-ink-3 and opacity-40 — verify contrast against the dark-mode surface stays ≥3:1 (large icon threshold). Easy axe-playwright check.
For DateInput, when the value is externally cleared, the input visibly empties — good. But also clear errorMessage (currently $bindable<string | null>(null)). If the user typed an invalid date, saw the inline error, then clicked reset, the error label would persist alongside the now-empty input. That's confusing. Add to AC4 test.
Mobile validation: the issue's manual walkthrough doesn't mention 320px. Verify reset and URL-share both work at small phone widths — the typeahead dropdown's fixed positioning is known to be fussy under viewport changes.

Open Decisions

Toast vs. aria-live region for "Filters cleared" announcement: a visible toast (better for everyone, costs ~1 component) vs. a screen-reader-only aria-live region (smaller change, only helps AT users). Both are correct WCAG-wise; the visible toast is more inclusive. Worth picking before implementation since it touches SearchFilterBar.svelte either way.

## 🎨 Leonie Voss — UX & Accessibility ### Observations - This is exactly the kind of "stale state" trust bug that erodes confidence in a UI more than it costs in objective minutes lost. A senior researcher (60+ on a tablet) shares a filtered URL with a sibling, and the sibling sees "the field is empty but the list is filtered" — the inconsistency is harder to recover from than no filtering at all. - Reset (×) showing stale text is a Nielsen heuristic #1 violation (visibility of system status). The user tells the system "clear" and the system says "no" by leaving the text in place. - `<a href="/documents">` for reset is the right semantic — it works without JS, is keyboard-reachable, screen readers announce it as a link, and back-button restores the prior filter state. - The current reset button is an icon-only `<a>` with `title={m.docs_btn_reset_title()}` and an icon marked `aria-hidden="true"`. The link itself has no accessible name from text content, only the title attribute (which is unreliable for screen readers and invisible on touch). This is *not in scope* for this issue, but fixing the AC4 reset bug is a natural moment to also make this link labelled properly (separate ticket if needed). ### Recommendations - **Announce the reset to assistive tech.** When AC4 lands and all four fields visibly clear, also fire an `aria-live="polite"` toast/region: "Filter zurückgesetzt" / "Filters cleared". A keyboard user with screen reader gets no signal today even after the fix; the screen reader announces a navigation, but not "your filter is now blank." - **Add `aria-label={m.docs_btn_reset_label()}`** to the reset `<a>` (separate from `title`). Right now the only accessible name is the title attribute, which iOS VoiceOver and Android TalkBack handle inconsistently. Tracked separately if you prefer — not blocking — but the in-scope test suite is the right moment to surface it. - **For `PersonTypeahead`, after reset, the input should not retain `aria-expanded="true"` or any open dropdown.** The current `closeDropdown()` is called via `clickOutside`, but a programmatic reset (URL nav) doesn't trigger that. Verify: after reset, the input is empty AND `aria-expanded="false"` AND no listbox is in the DOM. Add to the AC4 test. - **Touch-target on the reset `<a>`**: it's `px-3 py-2.5` with a `h-5 w-5` icon — that's roughly 36×30px hit area. Below the 44×44 WCAG 2.5.5 threshold. On a phone, missing this and accidentally hitting the Filter toggle next to it is exactly the failure mode that makes seniors give up. Bump to `min-h-[44px] min-w-[44px]` and add explicit `aria-label`. Outside the AC list as written but trivially in scope of "reset works for users." - **Test in dark mode.** The reset icon currently uses `text-ink-3` and `opacity-40` — verify contrast against the dark-mode surface stays ≥3:1 (large icon threshold). Easy axe-playwright check. - **For `DateInput`, when the value is externally cleared, the input visibly empties — good. But also clear `errorMessage`** (currently `$bindable<string | null>(null)`). If the user typed an invalid date, saw the inline error, then clicked reset, the error label would persist alongside the now-empty input. That's confusing. Add to AC4 test. - **Mobile validation**: the issue's manual walkthrough doesn't mention 320px. Verify reset and URL-share both work at small phone widths — the typeahead dropdown's fixed positioning is known to be fussy under viewport changes. ### Open Decisions - **Toast vs. aria-live region for "Filters cleared" announcement**: a visible toast (better for everyone, costs ~1 component) vs. a screen-reader-only `aria-live` region (smaller change, only helps AT users). Both are correct WCAG-wise; the visible toast is more inclusive. Worth picking before implementation since it touches `SearchFilterBar.svelte` either way.

marcel commented

2026-05-08 16:02:58 +02:00

📋 Elicit — Requirements Engineering (Brownfield)

Observations

Issue is well-structured: three distinct symptoms, a shared root cause, file:line citations, an approach section, and seven acceptance criteria. This is closer to a spec than a bug report — exactly the standard the project's feedback_spec_as_issue memory suggests.
All seven ACs are observable and testable. AC6 in particular is unusually good — it explicitly names the regression risk introduced by the fix ("no flicker / cursor-jump from the new $effect"). Most issues forget to constrain the fix's side-effects.
Out-of-scope section is present and helpful (tag-filter URL/UI sync, restyle, API contract). Good scope discipline.
Three coverage gaps in the AC list — none are blockers, but worth closing before implementation:
1. AC3 ambiguity: "shows the input empty (not 'undefined' or an error)" — what does "the result list as if no sender filter were applied" mean operationally? Is the unknown id silently dropped at the server, or sent to the API which then returns 0 docs? Two different observable outcomes; pick one. (Sara also flagged this.)
2. No AC for AT/screen reader behaviour after reset. AC4 is visual-only ("clears all four input fields visually"). Should the screen reader hear "Filter zurückgesetzt"? Today: no, and there's no AC requiring it. (Leonie flagged this; this is the requirements-side counterpart.)
3. No AC for the typeahead's open-dropdown state on reset. If the user has the dropdown open with results visible, then clicks reset, what should happen? Likely: dropdown closes, aria-expanded="false". Not covered.

Recommendations

Tighten AC3 with a clear behavioural commitment. Suggested rewrite:

AC3 — Loading /documents?senderId={non-existent-id} shows the Sender input empty (not "undefined", no error toast). The senderId param is silently dropped server-side; the result list is identical to /documents without any filter.
Add AC8 (screen-reader / assistive-tech announcement on reset) if you want the reset to be inclusive without a separate ticket:

AC8 — Clicking reset announces "Filter zurückgesetzt" via aria-live="polite" (or equivalent). Tested with axe-playwright.
Add AC9 (open-dropdown state on reset):

AC9 — If the typeahead dropdown is open at the moment of reset, it closes; aria-expanded="false" after navigation completes.
Acceptance Criteria → INVEST quick-check:
- Independent ✅ (each can fail alone)
- Negotiable ✅ (you can drop AC5 if timeline drag isn't a critical path)
- Valuable ✅ (P1 trust bug)
- Estimable ✅ (file:line cites, ~4 files, ~7 tests)
- Small ⚠️ (right at the boundary — 4 files, 7 ACs, 7+ new tests; if the typing-loop concern Felix raised forces a different approach, scope can grow)
- Testable ✅ (every AC has an observable outcome)
Risk-if-not-addressed paragraph is good — concrete user-facing impact, P1 justified. No change needed.
Workflow signal: the structure of this issue (Context → Symptoms → Root cause → Approach → ACs → Verification → Risk → Out of scope) is a useful template for future bug issues in this project. Worth lifting into a .gitea/ISSUE_TEMPLATE/bug.md if not already present.

Open Decisions

AC3 semantics (silently drop unknown id vs. forward to API). One-line decision unblocks the test.
Whether to expand scope to AC8/AC9 (a11y announcement + dropdown close on reset) within this PR, or to defer to follow-up tickets. Trade-off: bigger PR but related fixes land together vs. crisper PR but two more open tickets.

## 📋 Elicit — Requirements Engineering (Brownfield) ### Observations - Issue is well-structured: three distinct symptoms, a shared root cause, file:line citations, an approach section, and seven acceptance criteria. This is closer to a spec than a bug report — exactly the standard the project's `feedback_spec_as_issue` memory suggests. - All seven ACs are observable and testable. AC6 in particular is unusually good — it explicitly names the regression risk introduced by the fix ("no flicker / cursor-jump from the new `$effect`"). Most issues forget to constrain the fix's side-effects. - Out-of-scope section is present and helpful (tag-filter URL/UI sync, restyle, API contract). Good scope discipline. - Three coverage gaps in the AC list — none are blockers, but worth closing before implementation: 1. **AC3 ambiguity**: "shows the input empty (not 'undefined' or an error)" — what does "the result list as if no sender filter were applied" mean operationally? Is the unknown id silently dropped at the server, or sent to the API which then returns 0 docs? Two different observable outcomes; pick one. (Sara also flagged this.) 2. **No AC for AT/screen reader behaviour after reset**. AC4 is visual-only ("clears all four input fields visually"). Should the screen reader hear "Filter zurückgesetzt"? Today: no, and there's no AC requiring it. (Leonie flagged this; this is the requirements-side counterpart.) 3. **No AC for the typeahead's open-dropdown state on reset**. If the user has the dropdown open with results visible, then clicks reset, what should happen? Likely: dropdown closes, `aria-expanded="false"`. Not covered. ### Recommendations - **Tighten AC3** with a clear behavioural commitment. Suggested rewrite: > AC3 — Loading `/documents?senderId={non-existent-id}` shows the Sender input empty (not "undefined", no error toast). The senderId param is silently dropped server-side; the result list is identical to `/documents` without any filter. - **Add AC8 (screen-reader / assistive-tech announcement on reset)** if you want the reset to be inclusive without a separate ticket: > AC8 — Clicking reset announces "Filter zurückgesetzt" via `aria-live="polite"` (or equivalent). Tested with axe-playwright. - **Add AC9 (open-dropdown state on reset)**: > AC9 — If the typeahead dropdown is open at the moment of reset, it closes; `aria-expanded="false"` after navigation completes. - **Acceptance Criteria → INVEST quick-check:** - Independent ✅ (each can fail alone) - Negotiable ✅ (you can drop AC5 if timeline drag isn't a critical path) - Valuable ✅ (P1 trust bug) - Estimable ✅ (file:line cites, ~4 files, ~7 tests) - Small ⚠️ (right at the boundary — 4 files, 7 ACs, 7+ new tests; if the typing-loop concern Felix raised forces a different approach, scope can grow) - Testable ✅ (every AC has an observable outcome) - **Risk-if-not-addressed paragraph is good** — concrete user-facing impact, P1 justified. No change needed. - **Workflow signal**: the structure of this issue (Context → Symptoms → Root cause → Approach → ACs → Verification → Risk → Out of scope) is a useful template for future bug issues in this project. Worth lifting into a `.gitea/ISSUE_TEMPLATE/bug.md` if not already present. ### Open Decisions - **AC3 semantics** (silently drop unknown id vs. forward to API). One-line decision unblocks the test. - **Whether to expand scope to AC8/AC9** (a11y announcement + dropdown close on reset) within this PR, or to defer to follow-up tickets. Trade-off: bigger PR but related fixes land together vs. crisper PR but two more open tickets.

marcel commented

2026-05-08 16:03:22 +02:00

🗳️ Decision Queue — Action Required

3 decisions need your input before implementation starts.

Architecture

Where does displayName-from-id resolution live? Inline in +page.server.ts (zero extra files, but code duplicates the next time Briefwechsel/Aktivitäten need it) vs. a resolvePersonName(api, id) helper in $lib/person/server.ts (~10 lines, one place to update on contract changes). Pick "shared helper" if more URL-share fixes are coming; pick "inline" if 482 is the last of these. (Raised by: Markus)

Requirements

AC3 server behaviour for unknown senderId/receiverId — silently drop the param at the server (cleaner URL, recommended) vs. forward to API and let it return 0 docs (preserves the user's intent to refine the search). The two paths are observably different and need a test either way. (Raised by: Sara, Elicit)

UX / Scope

Expand scope to a11y polish — should this PR also (a) announce the reset via toast or aria-live="polite" (AC8), and (b) close the typeahead dropdown on programmatic reset (AC9)? Bigger PR but related fixes land together; smaller PR creates two follow-up tickets. If "yes," choose visible toast (more inclusive) vs. SR-only aria-live region (smaller change). (Raised by: Leonie, Elicit)

## 🗳️ Decision Queue — Action Required _3 decisions need your input before implementation starts._ ### Architecture - **Where does displayName-from-id resolution live?** Inline in `+page.server.ts` (zero extra files, but code duplicates the next time Briefwechsel/Aktivitäten need it) vs. a `resolvePersonName(api, id)` helper in `$lib/person/server.ts` (~10 lines, one place to update on contract changes). Pick "shared helper" if more URL-share fixes are coming; pick "inline" if 482 is the last of these. _(Raised by: Markus)_ ### Requirements - **AC3 server behaviour for unknown senderId/receiverId** — silently drop the param at the server (cleaner URL, recommended) vs. forward to API and let it return 0 docs (preserves the user's intent to refine the search). The two paths are observably different and need a test either way. _(Raised by: Sara, Elicit)_ ### UX / Scope - **Expand scope to a11y polish** — should this PR also (a) announce the reset via toast or `aria-live="polite"` (AC8), and (b) close the typeahead dropdown on programmatic reset (AC9)? Bigger PR but related fixes land together; smaller PR creates two follow-up tickets. If "yes," choose visible toast (more inclusive) vs. SR-only `aria-live` region (smaller change). _(Raised by: Leonie, Elicit)_

marcel referenced a pull request that will close this issue

2026-05-09 10:30:56 +02:00

fix(documents): filter inputs don't sync with URL on navigation (#482) #487

marcel referenced this issue

2026-05-09 10:35:45 +02:00

fix(documents): filter inputs don't sync with URL on navigation (#482) #487

marcel referenced this issue

2026-05-09 14:26:48 +02:00

fix(fts): paginate FTS match-set in SQL instead of loading all matching IDs #488

marcel closed this issue

2026-05-09 14:27:26 +02:00

marcel referenced this issue