feat(transcription): decouple @mention display text from person search #380

New Issue

marcel · 2026-04-30T10:54:43+02:00

marcel commented

2026-04-30 10:54:43 +02:00

Context

When a transcriber types @WdG in a transcription block, the dropdown filters persons using "WdG" as the search string — and finds nothing, because the stored person is "Walter de Gruyter". The same problem occurs with relative names like @Vater von Hilde or any nickname that doesn't match the stored display name.

The display text typed in the document is historically correct and must be preserved verbatim. What needs to improve is the lookup — decoupling it from the typed text.

User Story

As a transcriber, I want to type a short form or nickname after @ and still find and link the correct person via a separate search field, so that the transcription preserves the original text while the system resolves the correct identity.

Acceptance Criteria

Given the @mention dropdown is open, when it appears, then a search input field is visible and pre-filled with the text the user typed after @.
Given the search field contains text, when the user views the person list, then the list is filtered by the search field content (not the @-text).
Given the user edits the search field, when the content changes, then the person list updates in real time to match the new search string.
Given the user clears the search field completely, when the field is empty, then the person list shows no results and a prompt such as "Namen eingeben…" is displayed.
Given the user selects a person from the list, when the mention is saved, then the display text in the transcription remains exactly as the user typed it after @ (e.g. "WdG"), and the selected person is stored as the linked entity.
Given the user closes the dropdown without selecting a person, when the mention is saved, then the display text is preserved and no person link is stored (unlinked mention remains valid).
Given an existing @mention is re-opened for editing, when the dropdown appears, then the search field is pre-filled with the saved display text.

NFR Notes

Accessibility: Search field needs a visible label or aria-label; keyboard navigation through the list must remain intact.
Responsive: Dropdown must remain usable on tablet (transcribers' primary device) — minimum touch target 44 px.
Performance: Person list filtering should feel instant (≤ 150 ms debounce).
i18n: Prompt text "Namen eingeben…" needs a Paraglide translation key (de/en/es).

## Context When a transcriber types `@WdG` in a transcription block, the dropdown filters persons using "WdG" as the search string — and finds nothing, because the stored person is "Walter de Gruyter". The same problem occurs with relative names like `@Vater von Hilde` or any nickname that doesn't match the stored display name. The display text typed in the document is historically correct and must be preserved verbatim. What needs to improve is the *lookup* — decoupling it from the typed text. ## User Story As a transcriber, I want to type a short form or nickname after `@` and still find and link the correct person via a separate search field, so that the transcription preserves the original text while the system resolves the correct identity. ## Acceptance Criteria 1. **Given** the @mention dropdown is open, **when** it appears, **then** a search input field is visible and pre-filled with the text the user typed after `@`. 2. **Given** the search field contains text, **when** the user views the person list, **then** the list is filtered by the search field content (not the @-text). 3. **Given** the user edits the search field, **when** the content changes, **then** the person list updates in real time to match the new search string. 4. **Given** the user clears the search field completely, **when** the field is empty, **then** the person list shows no results and a prompt such as "Namen eingeben…" is displayed. 5. **Given** the user selects a person from the list, **when** the mention is saved, **then** the display text in the transcription remains exactly as the user typed it after `@` (e.g. "WdG"), and the selected person is stored as the linked entity. 6. **Given** the user closes the dropdown without selecting a person, **when** the mention is saved, **then** the display text is preserved and no person link is stored (unlinked mention remains valid). 7. **Given** an existing @mention is re-opened for editing, **when** the dropdown appears, **then** the search field is pre-filled with the saved display text. ## NFR Notes - **Accessibility:** Search field needs a visible label or `aria-label`; keyboard navigation through the list must remain intact. - **Responsive:** Dropdown must remain usable on tablet (transcribers' primary device) — minimum touch target 44 px. - **Performance:** Person list filtering should feel instant (≤ 150 ms debounce). - **i18n:** Prompt text "Namen eingeben…" needs a Paraglide translation key (de/en/es).

marcel added the P2-medium feature ui labels 2026-04-30 10:54:48 +02:00

marcel commented

2026-05-07 17:48:49 +02:00

🏛️ Markus Keller — Application Architect

Observations

The current PersonMentionEditor already lives in $lib/shared/discussion/ — correctly scoped as a shared component since it is used both in the transcription workflow and (via MentionEditor) in comment threads. The new feature extends the dropdown portion only.
The issue calls for adding a search input field inside MentionDropdown.svelte. This is a pure UI change; the backend GET /api/persons?q= endpoint already exists, is already called from PersonMentionEditor, and already searches across displayName, alias, and person_name_aliases. No new backend API is needed.
The backend searchWithDocumentCount query in PersonRepository already searches the person_name_aliases table and the p.alias field — so "WdG" would match if it is stored there. The real gap is in the UX flow, not the search query itself. It is worth deciding whether the alias data for historical nicknames is already populated and covering this use-case, or whether alias population is a prerequisite.
The existing PersonSummaryDTO leaks notes to the frontend (visible in the generated api.ts). This was already flagged as Nora #5618 #3 in the PersonMentionEditor source comment. Issue #380 does not make it worse, but the notes leak remains open.
Adding a <input> inside the imperatively-mounted MentionDropdown introduces a focus complexity: the transcription editor must not blur when the user clicks into the search field. The existing onmousedown + e.preventDefault() pattern on list items handles this for selection — the same pattern is needed on the search input itself.
The Tiptap suggestion lifecycle passes query via renderProps. The new feature decouples the search string from query — the typed @text stays in the editor while the search input holds a separate, editable string. This means renderProps.query is only used as the initial value of the search input; subsequent searches must be driven by the input's own state, not by Tiptap's onUpdate.

Recommendations

Keep all new markup inside MentionDropdown.svelte — no new component needed. The dropdown is already fully isolated; adding a <input> at the top is proportionate.
Pass initialQuery as a prop to MentionDropdown (set on onStart from renderProps.query). The search input is $state-local inside the dropdown. The dropdown triggers a callback prop (onSearch: (q: string) => void) when the input changes; PersonMentionEditor handles the debounced fetch and updates dropdownState.items. This keeps fetch logic centralized in PersonMentionEditor, not duplicated inside the dropdown.
Wire onmousedown={(e) => e.preventDefault()} on the search <input> element to prevent the editor from blurring when the user clicks into the field. Without this, clicking the search input closes the dropdown.
Document the client-side fetch exception in an ADR. The source comment in PersonMentionEditor already notes that Markus #5616 flagged this for an ADR. This feature adds another consumer of the pattern. Create docs/adr/ADR-0XX-client-side-fetch-editor-suggestions.md before or alongside this PR — that note has been open long enough.
No new backend endpoint needed. The GET /api/persons?q= endpoint is already correct and searches aliases. If WdG still does not match, the fix is a data issue (alias not populated), not a code issue.

Open Decisions

Alias data completeness: Is the persons.alias or person_name_aliases table already populated with short-forms like "WdG"? If not, this feature improves the UX but does not solve the root motivating example from the issue body. Worth clarifying scope: is populating alias data in scope for this issue, or a follow-up?

## 🏛️ Markus Keller — Application Architect ### Observations - The current `PersonMentionEditor` already lives in `$lib/shared/discussion/` — correctly scoped as a shared component since it is used both in the transcription workflow and (via `MentionEditor`) in comment threads. The new feature extends the dropdown portion only. - The issue calls for adding a **search input field inside `MentionDropdown.svelte`**. This is a pure UI change; the backend `GET /api/persons?q=` endpoint already exists, is already called from `PersonMentionEditor`, and already searches across `displayName`, `alias`, and `person_name_aliases`. No new backend API is needed. - The backend `searchWithDocumentCount` query in `PersonRepository` already searches the `person_name_aliases` table and the `p.alias` field — so "WdG" would match *if* it is stored there. The real gap is in the UX flow, not the search query itself. It is worth deciding whether the alias data for historical nicknames is already populated and covering this use-case, or whether alias population is a prerequisite. - The existing `PersonSummaryDTO` leaks `notes` to the frontend (visible in the generated `api.ts`). This was already flagged as `Nora #5618 #3` in the `PersonMentionEditor` source comment. Issue #380 does not make it worse, but the `notes` leak remains open. - Adding a `<input>` inside the imperatively-mounted `MentionDropdown` introduces a focus complexity: the transcription editor must not blur when the user clicks into the search field. The existing `onmousedown` + `e.preventDefault()` pattern on list items handles this for selection — the same pattern is needed on the search input itself. - The Tiptap suggestion lifecycle passes `query` via `renderProps`. The new feature *decouples* the search string from `query` — the typed `@text` stays in the editor while the search input holds a separate, editable string. This means `renderProps.query` is only used as the *initial value* of the search input; subsequent searches must be driven by the input's own state, not by Tiptap's `onUpdate`. ### Recommendations 1. **Keep all new markup inside `MentionDropdown.svelte`** — no new component needed. The dropdown is already fully isolated; adding a `<input>` at the top is proportionate. 2. **Pass `initialQuery` as a prop to `MentionDropdown`** (set on `onStart` from `renderProps.query`). The search input is `$state`-local inside the dropdown. The dropdown triggers a callback prop (`onSearch: (q: string) => void`) when the input changes; `PersonMentionEditor` handles the debounced fetch and updates `dropdownState.items`. This keeps fetch logic centralized in `PersonMentionEditor`, not duplicated inside the dropdown. 3. **Wire `onmousedown={(e) => e.preventDefault()}`** on the search `<input>` element to prevent the editor from blurring when the user clicks into the field. Without this, clicking the search input closes the dropdown. 4. **Document the client-side fetch exception in an ADR.** The source comment in `PersonMentionEditor` already notes that `Markus #5616` flagged this for an ADR. This feature adds another consumer of the pattern. Create `docs/adr/ADR-0XX-client-side-fetch-editor-suggestions.md` before or alongside this PR — that note has been open long enough. 5. **No new backend endpoint needed.** The `GET /api/persons?q=` endpoint is already correct and searches aliases. If WdG still does not match, the fix is a data issue (alias not populated), not a code issue. ### Open Decisions - **Alias data completeness:** Is the `persons.alias` or `person_name_aliases` table already populated with short-forms like "WdG"? If not, this feature improves the UX but does not solve the root motivating example from the issue body. Worth clarifying scope: is populating alias data in scope for this issue, or a follow-up?

marcel commented

2026-05-07 17:49:19 +02:00

👨‍💻 Felix Brandt — Fullstack Developer

Observations

MentionDropdown.svelte is currently 172 lines and has one visual concern (the list). Adding a search input keeps it under 220 lines — no split needed.
The dropdown uses the model shared state object pattern (a $state proxy object passed by reference from PersonMentionEditor). The new search input will need its own local state let searchQuery = $state(initialQuery), initialized from a new initialQuery prop. Adding initialQuery as a prop alongside model is clean.
The onKeyDown exported function currently handles Arrow/Enter/Escape. With a search input present, ArrowDown and Enter must only fire on the list when focus is on the list or the editor — if focus is inside the search input, Enter should not select the currently highlighted item (that would be surprising). The input should instead trigger search on input event, not on Enter.
The items async fetch in PersonMentionEditor is currently wired to Tiptap's onUpdate callback (which fires on every @ + keystroke). After decoupling, a second fetch path is needed: the search input's oninput event. Both paths land in the same dropdownState.items — the callback prop approach (Markus's rec) keeps this in one place.
debounce is already used in MentionEditor.svelte (the comment-thread variant) with 200 ms. The NFR says ≤ 150 ms debounce. Use 150 ms consistently across both editors.
AC-7 ("re-opened for editing — search field pre-filled with saved displayName"): the displayName on a stored mention node is the typed short-form. When Tiptap opens an existing mention for editing (via the suggestion plugin), renderProps.query will be the display text. Passing it as initialQuery satisfies AC-7 automatically.
The {#each model.items as person, i (person.id)} already has a key expression — good. The new search input does not change this.

Recommendations

Add initialQuery: string prop to MentionDropdown and initialize let searchQuery = $state(initialQuery). Wire oninput={(e) => onSearch(e.currentTarget.value)} on the input. Export onSearch as a prop.
Wire the search input with onmousedown={(e) => e.preventDefault()} to prevent editor blur when clicking into it.
Reset highlightedIndex to 0 when searchQuery changes — the existing $effect that watches model.items already does this for item list changes, so it is covered as long as the new search → fetch → items update path flows through dropdownState.items.
Use 150 ms debounce (as specified in the NFR) in PersonMentionEditor for the search input callback — replace the existing Tiptap-driven immediate fetch.
AC-4 (empty search → no results + "Namen eingeben…"): when searchQuery is empty, do not fire a fetch — call onSearch('') and in PersonMentionEditor set dropdownState.items = [] immediately. Show the "Namen eingeben…" prompt when searchQuery === ''. The existing {#if model.items.length === 0} block is where this condition branches — add a emptyBecauseNoQuery flag or check the query length in the dropdown.
Ensure tests are written red-first for each AC. AC-1 (display text preserved) already has solid test coverage in PersonMentionEditor.svelte.spec.ts. New tests needed: AC-3 (live filter update), AC-4 (empty prompt), AC-7 (pre-fill on re-open).

## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - `MentionDropdown.svelte` is currently 172 lines and has one visual concern (the list). Adding a search input keeps it under 220 lines — no split needed. - The dropdown uses the `model` shared state object pattern (a `$state` proxy object passed by reference from `PersonMentionEditor`). The new search input will need its own local state `let searchQuery = $state(initialQuery)`, initialized from a new `initialQuery` prop. Adding `initialQuery` as a prop alongside `model` is clean. - The `onKeyDown` exported function currently handles Arrow/Enter/Escape. With a search input present, `ArrowDown` and `Enter` must only fire on the list when focus is on the list or the editor — if focus is inside the search input, `Enter` should not select the currently highlighted item (that would be surprising). The input should instead trigger search on `input` event, not on Enter. - The `items` async fetch in `PersonMentionEditor` is currently wired to Tiptap's `onUpdate` callback (which fires on every `@` + keystroke). After decoupling, a second fetch path is needed: the search input's `oninput` event. Both paths land in the same `dropdownState.items` — the callback prop approach (Markus's rec) keeps this in one place. - `debounce` is already used in `MentionEditor.svelte` (the comment-thread variant) with 200 ms. The NFR says ≤ 150 ms debounce. Use 150 ms consistently across both editors. - AC-7 ("re-opened for editing — search field pre-filled with saved displayName"): the `displayName` on a stored mention node is the typed short-form. When Tiptap opens an existing mention for editing (via the suggestion plugin), `renderProps.query` will be the display text. Passing it as `initialQuery` satisfies AC-7 automatically. - The `{#each model.items as person, i (person.id)}` already has a key expression — good. The new search input does not change this. ### Recommendations 1. **Add `initialQuery: string` prop to `MentionDropdown`** and initialize `let searchQuery = $state(initialQuery)`. Wire `oninput={(e) => onSearch(e.currentTarget.value)}` on the input. Export `onSearch` as a prop. 2. **Wire the search input with `onmousedown={(e) => e.preventDefault()}`** to prevent editor blur when clicking into it. 3. **Reset `highlightedIndex` to 0 when `searchQuery` changes** — the existing `$effect` that watches `model.items` already does this for item list changes, so it is covered as long as the new search → fetch → items update path flows through `dropdownState.items`. 4. **Use 150 ms debounce** (as specified in the NFR) in `PersonMentionEditor` for the search input callback — replace the existing Tiptap-driven immediate fetch. 5. **AC-4 (empty search → no results + "Namen eingeben…"):** when `searchQuery` is empty, do not fire a fetch — call `onSearch('')` and in `PersonMentionEditor` set `dropdownState.items = []` immediately. Show the "Namen eingeben…" prompt when `searchQuery === ''`. The existing `{#if model.items.length === 0}` block is where this condition branches — add a `emptyBecauseNoQuery` flag or check the query length in the dropdown. 6. **Ensure tests are written red-first** for each AC. AC-1 (display text preserved) already has solid test coverage in `PersonMentionEditor.svelte.spec.ts`. New tests needed: AC-3 (live filter update), AC-4 (empty prompt), AC-7 (pre-fill on re-open).

marcel commented

2026-05-07 17:49:34 +02:00

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

This feature is entirely frontend — no new Docker service, no new infrastructure component, no schema migration, no environment variable. There is nothing to review at the infrastructure layer for the feature itself.
The fetch debounce (≤ 150 ms per the NFR) is a client-side timer. No backend rate limiting is mentioned in the issue. The /api/persons?q= endpoint is already being called on every @-keystroke; the new search input will trigger additional calls. The backend does not currently apply query rate limiting. At family-archive scale this is a non-issue, but worth noting.
The CI pipeline will pick up the new vitest-browser specs for the updated dropdown automatically — no pipeline change needed.
The PersonSummaryDTO returning notes over the wire (already flagged in the source) is a mild data-transfer overhead, not an infrastructure concern.

Recommendations

No infrastructure changes required for this issue. Keep it that way — resist any temptation to add a dedicated /api/persons/search endpoint with a different response shape just for this dropdown. The existing endpoint is correct and already proxied through Vite/Caddy.
Confirm the 150 ms debounce is enforced on the search input, not just on Tiptap's onUpdate. The current Tiptap suggestion plugin fires items() on every character — replacing that with a 150 ms debounce on the decoupled search input is a minor performance improvement for the backend.
Add a data-test-search-input attribute (or equivalent aria-label) on the new search field so Playwright E2E selectors do not rely on fragile CSS class selectors.

## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations - This feature is **entirely frontend** — no new Docker service, no new infrastructure component, no schema migration, no environment variable. There is nothing to review at the infrastructure layer for the feature itself. - The fetch debounce (≤ 150 ms per the NFR) is a client-side timer. No backend rate limiting is mentioned in the issue. The `/api/persons?q=` endpoint is already being called on every `@`-keystroke; the new search input will trigger additional calls. The backend does not currently apply query rate limiting. At family-archive scale this is a non-issue, but worth noting. - The CI pipeline will pick up the new vitest-browser specs for the updated dropdown automatically — no pipeline change needed. - The `PersonSummaryDTO` returning `notes` over the wire (already flagged in the source) is a mild data-transfer overhead, not an infrastructure concern. ### Recommendations 1. **No infrastructure changes required** for this issue. Keep it that way — resist any temptation to add a dedicated `/api/persons/search` endpoint with a different response shape just for this dropdown. The existing endpoint is correct and already proxied through Vite/Caddy. 2. **Confirm the 150 ms debounce is enforced on the search input**, not just on Tiptap's `onUpdate`. The current Tiptap suggestion plugin fires `items()` on every character — replacing that with a 150 ms debounce on the decoupled search input is a minor performance improvement for the backend. 3. **Add a `data-test-search-input` attribute** (or equivalent `aria-label`) on the new search field so Playwright E2E selectors do not rely on fragile CSS class selectors.

marcel commented

2026-05-07 17:49:57 +02:00

📋 Elicit — Requirements Engineer

Observations

The issue is well-structured: it has a clear context, a user story in Connextra format, and 7 ACs in Given-When-Then. This is above-average issue quality for this backlog.
AC-5 (display text preserved as typed): The existing implementation already satisfies this — PersonMentionEditor stores renderProps.query as displayName, not person.displayName. This was implemented as "AC-1 fix" in the current code. AC-5 should be verified rather than implemented from scratch. The test stores the typed query as displayName, not the person DB name in PersonMentionEditor.svelte.spec.ts already covers this.
AC-6 (unlinked mention remains valid): This is also already implemented — Tiptap Escape handling and the suggestion plugin's onExit preserve the raw text. No new code needed.
AC-7 (re-opened mention pre-fills search field): Partially implicit. When Tiptap fires onStart for an existing mention, renderProps.query is the current display text. The new search input would receive this as initialQuery. This is the only AC where an implementation question remains open: does Tiptap actually fire onStart again when an existing mention is clicked/focused for editing? This should be verified against Tiptap's suggestion extension behavior before claiming AC-7 is delivered.
NFR — i18n: The prompt "Namen eingeben…" needs a Paraglide key. No key exists yet in de.json for this (the existing person_mention_popup_empty = "Keine Personen gefunden" is for the no-results state, not the empty-query state). A new key such as person_mention_search_prompt (de: "Namen eingeben…", en: "Enter a name…", es: "Escribe un nombre…") is required.
AC-4 gap: The AC says empty field → no results + prompt. The current implementation shows "Keine Personen gefunden" when the API returns zero results. The new empty-query state is a different condition (user cleared the field) and needs distinct UI copy and a distinct code path, not just the existing empty-results path.
Missing AC: debounce timing. The NFR says ≤ 150 ms debounce. There is no AC that verifies this is actually implemented at 150 ms (as opposed to the current Tiptap immediate-fire behavior). This should be an explicit implementation task even if not a formal AC.

Recommendations

Clarify which ACs are new work vs. already implemented. Specifically: AC-1, AC-5, AC-6 are already delivered. The implementation work for this issue is: the search input UI (AC-1/2/3/4/7) and the new i18n key.
Add the i18n key person_mention_search_prompt to de.json, en.json, and es.json before or alongside the search input markup.
Verify AC-7 against Tiptap's suggestion lifecycle (does onStart fire for existing mentions when re-edited?) and document the finding. If it does not, AC-7 requires additional implementation.
The motivating example ("WdG" → "Walter de Gruyter") works only if "WdG" is stored in persons.alias or person_name_aliases. The issue should explicitly state whether alias population is in scope or a prerequisite handled elsewhere.

Open Decisions

AC-7 verification: Does Tiptap's suggestion plugin fire onStart when the cursor enters an existing mention node, allowing the search field to pre-fill? This is an empirical question that determines whether AC-7 is free or requires custom Tiptap extension work.

## 📋 Elicit — Requirements Engineer ### Observations - The issue is well-structured: it has a clear context, a user story in Connextra format, and 7 ACs in Given-When-Then. This is above-average issue quality for this backlog. - **AC-5 (display text preserved as typed):** The existing implementation *already* satisfies this — `PersonMentionEditor` stores `renderProps.query` as `displayName`, not `person.displayName`. This was implemented as "AC-1 fix" in the current code. AC-5 should be verified rather than implemented from scratch. The test `stores the typed query as displayName, not the person DB name` in `PersonMentionEditor.svelte.spec.ts` already covers this. - **AC-6 (unlinked mention remains valid):** This is also already implemented — Tiptap Escape handling and the suggestion plugin's `onExit` preserve the raw text. No new code needed. - **AC-7 (re-opened mention pre-fills search field):** Partially implicit. When Tiptap fires `onStart` for an existing mention, `renderProps.query` is the current display text. The new search input would receive this as `initialQuery`. This is the *only* AC where an implementation question remains open: does Tiptap actually fire `onStart` again when an existing mention is clicked/focused for editing? This should be verified against Tiptap's suggestion extension behavior before claiming AC-7 is delivered. - **NFR — i18n:** The prompt "Namen eingeben…" needs a Paraglide key. No key exists yet in `de.json` for this (the existing `person_mention_popup_empty` = "Keine Personen gefunden" is for the no-results state, not the empty-query state). A new key such as `person_mention_search_prompt` (de: "Namen eingeben…", en: "Enter a name…", es: "Escribe un nombre…") is required. - **AC-4 gap:** The AC says empty field → no results + prompt. The current implementation shows "Keine Personen gefunden" when the API returns zero results. The new empty-query state is a *different* condition (user cleared the field) and needs distinct UI copy and a distinct code path, not just the existing empty-results path. - **Missing AC: debounce timing.** The NFR says ≤ 150 ms debounce. There is no AC that verifies this is actually implemented at 150 ms (as opposed to the current Tiptap immediate-fire behavior). This should be an explicit implementation task even if not a formal AC. ### Recommendations 1. **Clarify which ACs are new work vs. already implemented.** Specifically: AC-1, AC-5, AC-6 are already delivered. The implementation work for this issue is: the search input UI (AC-1/2/3/4/7) and the new i18n key. 2. **Add the i18n key `person_mention_search_prompt`** to `de.json`, `en.json`, and `es.json` before or alongside the search input markup. 3. **Verify AC-7 against Tiptap's suggestion lifecycle** (does `onStart` fire for existing mentions when re-edited?) and document the finding. If it does not, AC-7 requires additional implementation. 4. **The motivating example ("WdG" → "Walter de Gruyter") works only if "WdG" is stored in `persons.alias` or `person_name_aliases`.** The issue should explicitly state whether alias population is in scope or a prerequisite handled elsewhere. ### Open Decisions - **AC-7 verification:** Does Tiptap's suggestion plugin fire `onStart` when the cursor enters an existing mention node, allowing the search field to pre-fill? This is an empirical question that determines whether AC-7 is free or requires custom Tiptap extension work.

marcel commented

2026-05-07 17:50:15 +02:00

🔐 Nora "NullX" Steiner — Security Engineer

Observations

No new attack surface is introduced by this feature at the backend layer. The /api/persons?q= endpoint is read-only, already protected by @RequirePermission(Permission.READ_ALL), and already parameterized (searchWithDocumentCount uses named :query parameter — not string concatenation).
PersonSummaryDTO leaks notes — this was already flagged in the PersonMentionEditor source comment as "Nora #5618 #3". The new search input will trigger more calls to this endpoint (one per search keystroke after debounce), but does not worsen the data exposure. The open issue on PersonSummaryDTO response shape remains the fix target; it is not this issue's job.
XSS in the search input: The new <input> accepts free-text from the user. This text goes into fetch(\/api/persons?q=${encodeURIComponent(query)}`)—encodeURIComponentis correct. The text is also shown as thevalue of the input element, which is text content, not innerHTML — no XSS risk there. The existing XSS test (renders a malicious displayName as text, not as HTML elements`) covers the dropdown rendering path; the search input path is safe by construction.
SSRF/injection via the search query: The query travels from client → Vite proxy (dev) / Caddy (prod) → Spring Boot → parameterized JPQL. No injection vector at any hop.
Focus management and editor blur: When the user types in the search input, the Tiptap editor must not lose auth context or session state. This is a UX concern, not a security concern — the session is cookie-based and does not change on DOM focus events.
The rel="noopener" on the "Neue Person anlegen" link is already present in MentionDropdown.svelte (rel="noopener") — note it is missing noreferrer. The referrer will leak the transcription page URL to /persons/new. This is low risk (same origin), but for completeness should be rel="noopener noreferrer".

Recommendations

Change rel="noopener" to rel="noopener noreferrer" on the "Neue Person anlegen" <a> in MentionDropdown.svelte. (CWE-116 — minor, same-origin, but correct practice.)
Confirm encodeURIComponent is used on the search input's query string before the fetch — do not allow raw interpolation.
No new security-specific tests required for this feature beyond what already exists. The XSS test, the permission boundary tests on the backend endpoint, and the query parameterization are all already in place.

## 🔐 Nora "NullX" Steiner — Security Engineer ### Observations - **No new attack surface** is introduced by this feature at the backend layer. The `/api/persons?q=` endpoint is read-only, already protected by `@RequirePermission(Permission.READ_ALL)`, and already parameterized (`searchWithDocumentCount` uses named `:query` parameter — not string concatenation). - **`PersonSummaryDTO` leaks `notes`** — this was already flagged in the `PersonMentionEditor` source comment as "Nora #5618 #3". The new search input will trigger more calls to this endpoint (one per search keystroke after debounce), but does not worsen the data exposure. The open issue on `PersonSummaryDTO` response shape remains the fix target; it is not this issue's job. - **XSS in the search input:** The new `<input>` accepts free-text from the user. This text goes into `fetch(\`/api/persons?q=${encodeURIComponent(query)}\`)` — `encodeURIComponent` is correct. The text is also shown as the `value` of the input element, which is text content, not innerHTML — no XSS risk there. The existing XSS test (`renders a malicious displayName as text, not as HTML elements`) covers the dropdown rendering path; the search input path is safe by construction. - **SSRF/injection via the search query:** The query travels from client → Vite proxy (dev) / Caddy (prod) → Spring Boot → parameterized JPQL. No injection vector at any hop. - **Focus management and editor blur:** When the user types in the search input, the Tiptap editor must not lose auth context or session state. This is a UX concern, not a security concern — the session is cookie-based and does not change on DOM focus events. - **The `rel="noopener"` on the "Neue Person anlegen" link** is already present in `MentionDropdown.svelte` (`rel="noopener"`) — note it is missing `noreferrer`. The referrer will leak the transcription page URL to `/persons/new`. This is low risk (same origin), but for completeness should be `rel="noopener noreferrer"`. ### Recommendations 1. **Change `rel="noopener"` to `rel="noopener noreferrer"`** on the "Neue Person anlegen" `<a>` in `MentionDropdown.svelte`. (CWE-116 — minor, same-origin, but correct practice.) 2. **Confirm `encodeURIComponent` is used** on the search input's query string before the fetch — do not allow raw interpolation. 3. **No new security-specific tests required** for this feature beyond what already exists. The XSS test, the permission boundary tests on the backend endpoint, and the query parameterization are all already in place.

marcel commented

2026-05-07 17:50:33 +02:00

🧪 Sara Holt — QA Engineer

Observations

PersonMentionEditor.svelte.spec.ts is already well-structured: it uses vitest-browser-svelte, factory functions (mockFetchWithPersons, renderHost), and descriptive test names. The existing 20+ tests cover: typeahead open, fetch call, empty state, keyboard navigation, disabled state, XSS resistance, placeholder, touch target, and i18n content. This is a strong base.
Missing test coverage for new ACs:
- AC-2/3: No test verifies that editing the search input (not the @-text) updates the person list. This is the core new behavior.
- AC-4: No test verifies that clearing the search input shows "Namen eingeben…" (not "Keine Personen gefunden").
- AC-7: No test verifies that re-opening an existing mention pre-fills the search field.
The PersonMentionEditor test host (PersonMentionEditor.test-host.svelte) would need to expose the search input to userEvent. Since the dropdown is mounted imperatively to document.body, the search input would be reachable via page.getByRole('searchbox') or page.getByLabel(...) — verify this during implementation.
The touch target test (each result row has min-h-[44px]) should be extended to cover the new search input: it must also meet 44px minimum height (WCAG 2.2 AA) since it is a touch target for tablet transcribers.
The debounce (150 ms) is hard to test reliably in a browser component test. Use vi.useFakeTimers() to fast-forward time and assert that fetch is not called before 150 ms elapses, then called after. The existing spec already uses vi.waitFor — fake timers complement this for timing-sensitive assertions.
Flaky risk: The search input interaction test will drive oninput → debounce → fetch mock → items update → list render. The vi.waitFor pattern already used in the file handles async DOM updates correctly. No Thread.sleep equivalents, no risk of flakiness if fake timers are used correctly for the debounce.

Recommendations

Add tests for AC-3 (live filter): userEvent.type into the search input after dropdown opens; assert fetch is called with the new query and the person list updates.
Add test for AC-4 (empty prompt): Clear the search input; assert "Namen eingeben…" appears (not "Keine Personen gefunden").
Add test for AC-7 (pre-fill): Pass an initial mention in renderHost; open the dropdown on that mention position; assert the search input value equals the saved displayName.
Extend the touch target test to cover the search input: expect(searchInput.className).toContain('min-h-[44px]') or equivalent.
Use vi.useFakeTimers() for the debounce test to avoid 150 ms real-time waits slowing the suite. One test at the boundary is sufficient — no need for an exhaustive timing matrix.

## 🧪 Sara Holt — QA Engineer ### Observations - `PersonMentionEditor.svelte.spec.ts` is already well-structured: it uses `vitest-browser-svelte`, factory functions (`mockFetchWithPersons`, `renderHost`), and descriptive test names. The existing 20+ tests cover: typeahead open, fetch call, empty state, keyboard navigation, disabled state, XSS resistance, placeholder, touch target, and i18n content. This is a strong base. - **Missing test coverage for new ACs:** - AC-2/3: No test verifies that editing the *search input* (not the `@`-text) updates the person list. This is the core new behavior. - AC-4: No test verifies that clearing the search input shows "Namen eingeben…" (not "Keine Personen gefunden"). - AC-7: No test verifies that re-opening an existing mention pre-fills the search field. - The `PersonMentionEditor` test host (`PersonMentionEditor.test-host.svelte`) would need to expose the search input to `userEvent`. Since the dropdown is mounted imperatively to `document.body`, the search input would be reachable via `page.getByRole('searchbox')` or `page.getByLabel(...)` — verify this during implementation. - The touch target test (`each result row has min-h-[44px]`) should be extended to cover the new search input: it must also meet 44px minimum height (WCAG 2.2 AA) since it is a touch target for tablet transcribers. - The debounce (150 ms) is hard to test reliably in a browser component test. Use `vi.useFakeTimers()` to fast-forward time and assert that fetch is not called before 150 ms elapses, then called after. The existing spec already uses `vi.waitFor` — fake timers complement this for timing-sensitive assertions. - **Flaky risk:** The search input interaction test will drive `oninput` → debounce → fetch mock → items update → list render. The `vi.waitFor` pattern already used in the file handles async DOM updates correctly. No `Thread.sleep` equivalents, no risk of flakiness if fake timers are used correctly for the debounce. ### Recommendations 1. **Add tests for AC-3 (live filter):** `userEvent.type` into the search input after dropdown opens; assert `fetch` is called with the new query and the person list updates. 2. **Add test for AC-4 (empty prompt):** Clear the search input; assert "Namen eingeben…" appears (not "Keine Personen gefunden"). 3. **Add test for AC-7 (pre-fill):** Pass an initial mention in `renderHost`; open the dropdown on that mention position; assert the search input value equals the saved `displayName`. 4. **Extend the touch target test** to cover the search input: `expect(searchInput.className).toContain('min-h-[44px]')` or equivalent. 5. **Use `vi.useFakeTimers()`** for the debounce test to avoid 150 ms real-time waits slowing the suite. One test at the boundary is sufficient — no need for an exhaustive timing matrix.

marcel commented

2026-05-07 17:50:51 +02:00

🎨 Leonie Voss — UI/UX Design Lead

Observations

The current MentionDropdown is w-72 (288px fixed width) — borderline tight for a search input plus life-date metadata on tablet. At 768px viewport, 288px is workable but leaves no room for an input label. Use min-w-[288px] rather than a fixed w-72 so the dropdown can flex wider if the caret position allows.
The transcriber audience is 60+ on laptops/tablets. The search input must meet the 44px minimum touch target height. Use min-h-[44px] on the input wrapper and py-2.5 for vertical padding (matching the existing list rows).
Visible label vs. aria-label: The NFR explicitly requires "a visible label or aria-label" on the search field. A visible label is strongly preferable for the senior audience — even a small inline <label> or a labelled icon (magnifying glass + "Suchen") communicates function. An invisible aria-label alone is not enough here. Recommend: a magnifying glass icon at the left of the input (decorative, aria-hidden="true") + placeholder={m.person_mention_search_prompt()} + aria-label={m.person_mention_btn_label()}.
Focus ring: the search input must have a visible focus ring. Use focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset consistent with the rest of the UI. The input should use outline-none to suppress the browser default and replace with the ring.
Keyboard flow: when the dropdown opens, focus should stay in the Tiptap editor (so the transcriber can still type). The search input should be reachable by Tab (one Tab from the editor → input). Arrow keys from the input should move highlight in the list below. This is the correct interaction model for a search-then-navigate pattern.
Empty state copy distinction: AC-4 ("Namen eingeben…") and the existing "Keine Personen gefunden" are visually similar. The empty-query prompt should look instructional (e.g., lighter ink-3 text, no icon) while the no-results state uses the same weight. Both are already font-sans text-sm text-ink-3 — that is fine.
Placement: the search input should be at the top of the dropdown, above the list — this is the standard pattern (matches browser search fields, VS Code Quick Open, etc.). Do not put it below the list.
The brand token brand-mint for the focus border on the overall PersonMentionEditor wrapper is already present. The internal search input should use brand-navy for its focus ring to differentiate it from the outer editor focus (which uses brand-mint). This also avoids the WCAG 1.4.11 contrast issue already documented in the dropdown source.

Recommendations

Search input structure:

<div class="border-b border-line px-3 py-2">
  <label class="sr-only" for="mention-search">{m.person_mention_btn_label()}</label>
  <div class="flex items-center gap-2">
    <svg aria-hidden="true" class="h-4 w-4 shrink-0 text-ink-3"><!-- magnifier --></svg>
    <input
      id="mention-search"
      type="search"
      class="min-h-[44px] w-full bg-transparent font-sans text-sm text-ink placeholder:text-ink-3
             focus:outline-none focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset"
      placeholder={m.person_mention_search_prompt()}
      bind:value={searchQuery}
      oninput={...}
      onmousedown={(e) => e.preventDefault()}
    />
  </div>
</div>

The sr-only label + visible placeholder covers both screen readers and sighted users.

Empty-query state: display "Namen eingeben…" as a <p class="px-3 py-2.5 font-sans text-sm text-ink-3"> — same class as the no-results message. The copy distinction carries the meaning without needing different styling.
Do not autofocus the search input on dropdown open. The Tiptap editor must keep focus so typing continues to update the @text cursor position in the document. The user can Tab into the search input when they want to refine.
Tablet layout check: at 768px, test that the dropdown does not overflow the viewport horizontally when opened near the right edge. The existing flip strategy (open upward when near viewport bottom) should be extended to check horizontal bounds if needed.

Open Decisions

Autofocus vs. Tab-to-search: Should the search input autofocus when the dropdown opens, or should the user Tab into it? Autofocus feels faster but interrupts typing flow. Tab-to-search is safer for keyboard-only transcribers who are mid-sentence. The recommendation above says no autofocus, but the product owner should confirm the interaction intent against real transcriber behavior.

## 🎨 Leonie Voss — UI/UX Design Lead ### Observations - The current `MentionDropdown` is `w-72` (288px fixed width) — borderline tight for a search input plus life-date metadata on tablet. At 768px viewport, 288px is workable but leaves no room for an input label. Use `min-w-[288px]` rather than a fixed `w-72` so the dropdown can flex wider if the caret position allows. - The transcriber audience is 60+ on laptops/tablets. The search input must meet the 44px minimum touch target height. Use `min-h-[44px]` on the input wrapper and `py-2.5` for vertical padding (matching the existing list rows). - **Visible label vs. `aria-label`:** The NFR explicitly requires "a visible label or `aria-label`" on the search field. A visible label is strongly preferable for the senior audience — even a small inline `<label>` or a labelled icon (magnifying glass + "Suchen") communicates function. An invisible `aria-label` alone is not enough here. Recommend: a magnifying glass icon at the left of the input (decorative, `aria-hidden="true"`) + `placeholder={m.person_mention_search_prompt()}` + `aria-label={m.person_mention_btn_label()}`. - **Focus ring:** the search input must have a visible focus ring. Use `focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset` consistent with the rest of the UI. The input should use `outline-none` to suppress the browser default and replace with the ring. - **Keyboard flow:** when the dropdown opens, focus should stay in the Tiptap editor (so the transcriber can still type). The search input should be reachable by Tab (one Tab from the editor → input). Arrow keys from the input should move highlight in the list below. This is the correct interaction model for a search-then-navigate pattern. - **Empty state copy distinction:** AC-4 ("Namen eingeben…") and the existing "Keine Personen gefunden" are visually similar. The empty-query prompt should look instructional (e.g., lighter ink-3 text, no icon) while the no-results state uses the same weight. Both are already `font-sans text-sm text-ink-3` — that is fine. - **Placement:** the search input should be at the top of the dropdown, above the list — this is the standard pattern (matches browser search fields, VS Code Quick Open, etc.). Do not put it below the list. - The brand token `brand-mint` for the focus border on the overall `PersonMentionEditor` wrapper is already present. The internal search input should use `brand-navy` for its focus ring to differentiate it from the outer editor focus (which uses `brand-mint`). This also avoids the WCAG 1.4.11 contrast issue already documented in the dropdown source. ### Recommendations 1. **Search input structure:** ```svelte <div class="border-b border-line px-3 py-2"> <label class="sr-only" for="mention-search">{m.person_mention_btn_label()}</label> <div class="flex items-center gap-2"> <svg aria-hidden="true" class="h-4 w-4 shrink-0 text-ink-3"></svg> <input id="mention-search" type="search" class="min-h-[44px] w-full bg-transparent font-sans text-sm text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset" placeholder={m.person_mention_search_prompt()} bind:value={searchQuery} oninput={...} onmousedown={(e) => e.preventDefault()} /> </div> </div> ``` The `sr-only` label + visible placeholder covers both screen readers and sighted users. 2. **Empty-query state:** display "Namen eingeben…" as a `<p class="px-3 py-2.5 font-sans text-sm text-ink-3">` — same class as the no-results message. The copy distinction carries the meaning without needing different styling. 3. **Do not autofocus the search input on dropdown open.** The Tiptap editor must keep focus so typing continues to update the `@text` cursor position in the document. The user can Tab into the search input when they want to refine. 4. **Tablet layout check:** at 768px, test that the dropdown does not overflow the viewport horizontally when opened near the right edge. The existing flip strategy (open upward when near viewport bottom) should be extended to check horizontal bounds if needed. ### Open Decisions - **Autofocus vs. Tab-to-search:** Should the search input autofocus when the dropdown opens, or should the user Tab into it? Autofocus feels faster but interrupts typing flow. Tab-to-search is safer for keyboard-only transcribers who are mid-sentence. The recommendation above says no autofocus, but the product owner should confirm the interaction intent against real transcriber behavior.

marcel commented

2026-05-07 17:51:18 +02:00

🗂️ Decision Queue — Open Items Needing Human Judgment

Three genuine tradeoffs were raised across the persona reviews. These are not implementation tasks — they are decisions that will shape what gets built.

1. Alias data completeness (Markus + Elicit)

Question: Is "WdG" (or similar short-forms) already stored in persons.alias or person_name_aliases? The backend search query already includes those fields. If the alias data is not populated, this feature improves the search UX but does not fix the motivating example from the issue description.

Why it matters: Determines whether alias population is in scope for this issue or a separate prerequisite. If it is a separate task, a follow-up issue should be created before this PR merges so the motivating use case does not fall through the cracks.

Options: (a) In scope — add alias population tooling/docs alongside this issue. (b) Out of scope — create a follow-up issue and note the dependency.

2. AC-7: Does Tiptap fire `onStart` for existing mentions? (Elicit + Felix)

Question: When a transcriber positions their cursor inside an already-saved @mention node, does Tiptap's suggestion extension fire onStart again (allowing the search field to pre-fill with the saved displayName)? Or does it fire onUpdate with the existing mention text? Or neither?

Why it matters: Determines whether AC-7 ("re-opened mention pre-fills search field") is free (works automatically via renderProps.query) or requires custom Tiptap extension code to detect cursor entry into an existing mention node. This is an empirical question — needs a quick prototype test before estimation is possible.

Options: (a) Verify during implementation spike; if free, include in this issue. (b) If custom extension work is needed, split AC-7 into a follow-up issue.

3. Autofocus vs. Tab-to-search (Leonie)

Question: When the @mention dropdown opens, should the search input receive focus automatically, or should the user Tab into it from the editor?

Why it matters: Affects all transcribers. Autofocus is faster for refinement but interrupts mid-sentence typing flow. Tab-to-search is safer for keyboard users but adds one extra action. The decision shapes the keyboard interaction model for every @mention operation.

Options: (a) No autofocus — editor retains focus, Tab reaches the input (Leonie's recommendation, safer for keyboard-only users). (b) Autofocus the search input on open — faster refinement, but requires the transcriber to re-focus the editor to resume typing.

This is a product decision about the primary transcription workflow and should be confirmed against real transcriber behavior before implementation.

## 🗂️ Decision Queue — Open Items Needing Human Judgment Three genuine tradeoffs were raised across the persona reviews. These are not implementation tasks — they are decisions that will shape what gets built. --- ### 1. Alias data completeness (Markus + Elicit) **Question:** Is "WdG" (or similar short-forms) already stored in `persons.alias` or `person_name_aliases`? The backend search query already includes those fields. If the alias data is not populated, this feature improves the search UX but does not fix the motivating example from the issue description. **Why it matters:** Determines whether alias population is in scope for this issue or a separate prerequisite. If it is a separate task, a follow-up issue should be created before this PR merges so the motivating use case does not fall through the cracks. **Options:** (a) In scope — add alias population tooling/docs alongside this issue. (b) Out of scope — create a follow-up issue and note the dependency. --- ### 2. AC-7: Does Tiptap fire `onStart` for existing mentions? (Elicit + Felix) **Question:** When a transcriber positions their cursor inside an already-saved `@mention` node, does Tiptap's suggestion extension fire `onStart` again (allowing the search field to pre-fill with the saved `displayName`)? Or does it fire `onUpdate` with the existing mention text? Or neither? **Why it matters:** Determines whether AC-7 ("re-opened mention pre-fills search field") is free (works automatically via `renderProps.query`) or requires custom Tiptap extension code to detect cursor entry into an existing mention node. This is an empirical question — needs a quick prototype test before estimation is possible. **Options:** (a) Verify during implementation spike; if free, include in this issue. (b) If custom extension work is needed, split AC-7 into a follow-up issue. --- ### 3. Autofocus vs. Tab-to-search (Leonie) **Question:** When the `@mention` dropdown opens, should the search input receive focus automatically, or should the user Tab into it from the editor? **Why it matters:** Affects all transcribers. Autofocus is faster for refinement but interrupts mid-sentence typing flow. Tab-to-search is safer for keyboard users but adds one extra action. The decision shapes the keyboard interaction model for every @mention operation. **Options:** (a) No autofocus — editor retains focus, Tab reaches the input (Leonie's recommendation, safer for keyboard-only users). (b) Autofocus the search input on open — faster refinement, but requires the transcriber to re-focus the editor to resume typing. This is a product decision about the primary transcription workflow and should be confirmed against real transcriber behavior before implementation.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#380

feat(transcription): decouple @mention display text from person search #380

Context

User Story

Acceptance Criteria

NFR Notes

🏛️ Markus Keller — Application Architect

Observations

Recommendations

Open Decisions

👨‍💻 Felix Brandt — Fullstack Developer

Observations

Recommendations

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

Recommendations

📋 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions

🔐 Nora "NullX" Steiner — Security Engineer

Observations

Recommendations

🧪 Sara Holt — QA Engineer

Observations

Recommendations

🎨 Leonie Voss — UI/UX Design Lead

Observations

Recommendations

Open Decisions

🗂️ Decision Queue — Open Items Needing Human Judgment

1. Alias data completeness (Markus + Elicit)

2. AC-7: Does Tiptap fire onStart for existing mentions? (Elicit + Felix)

3. Autofocus vs. Tab-to-search (Leonie)

2. AC-7: Does Tiptap fire `onStart` for existing mentions? (Elicit + Felix)