Person mention: decouple display text from person name to preserve original wording #372
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Context
When a transcriber marks up a person mention in the transcription editor, the current behaviour replaces the typed trigger text with the person's full name. This breaks historical fidelity: if the original letter says "Hallo Clara", the transcription currently reads "Hallo @Clara Cram" — awkward and inaccurate.
The root cause is a design conflation: display text (what the scribe wrote) and person identity (who they meant) are treated as the same thing. They are not.
User Story
As a transcriber, I want to link a person mention to a person record without changing the original wording in the text, so that the transcription faithfully preserves what the letter-writer actually wrote.
Proposed Behaviour
Authoring flow
@followed by the word(s) as they appear in the original text (e.g.@Clara)@[Clara](uuid-of-clara-cram)Unresolved mention
If the transcriber types
@Clarabut does not select anyone from the dropdown (person not yet in the database, or deliberate choice), the@Clararemains as plain text — the@is preserved as a visible signal that a person reference was intended but not resolved.Read mode
@[Clara](uuid): renders asClara— a clickable anchor linking to the person's page. Hover card shows the person's full name (e.g. "Clara Cram") and summary details.@Clara(plain text): renders as literal@Clarawith no link.What This Fixes
The rename-sync invariant is intentional: renaming "Clara Cram" to "Clara Cram-Schmidt" on her person detail page will never update transcription text. The display text is the scribe's original word, not a derived label. This is correct behaviour for an archival system.
Data Model
@[Clara Cram](uuid)@[Clara](uuid)@Existing mentions stored with full names remain valid and do not require migration.
Acceptance Criteria
AC-1 — Resolved mention preserves original wording
Given a transcription containing the word "Clara",
When the transcriber types
@Claraand selects "Clara Cram" from the dropdown,Then the stored mention is
@[Clara](uuid)and the rendered transcription reads "Clara" (not "Clara Cram").AC-2 — Hover card shows full name
Given a resolved mention
@[Clara](uuid),When the reader hovers (desktop) or taps (mobile) the anchor,
Then the hover card header shows "Clara Cram" (the resolved person's full name).
AC-3 — Dropdown pre-fills with typed text
Given a transcriber types
@Tante,When the mention dropdown opens,
Then the search input is pre-filled with "Tante" and results are filtered accordingly.
AC-4 — Mention is atomic after selection
Given a transcriber has selected a person and locked a mention,
When they place the cursor inside the mention token,
Then the token is selected as a whole unit; individual characters cannot be edited.
AC-5 — Unresolved mention stays as plain text with @
Given a transcriber types
@Fritzand dismisses the dropdown without selecting anyone,Then the text
@Fritzremains in the editor as plain (unlinked) text, including the@symbol.AC-6 — No migration required
Given existing transcriptions with mentions stored as
@[Full Name](uuid),When those transcriptions are rendered in read mode,
Then they display and link correctly without any data migration.
👨💻 Felix Brandt — Senior Fullstack Developer
Observations
PersonMentionPropagationListener.javamust be deleted — it becomes actively harmful under the new design.Currently, the listener rewrites
PersonMention.displayNameAND the@OldNametext in blocks when a person is renamed. Under the new design,displayNamestores the author's original typed text (archival), not a derived copy of the person's DB name. If the listener runs on new-format blocks, it overwrites the archival text with the new person name — exactly what the issue says must never happen.The issue explicitly states: "renaming 'Clara Cram' to 'Clara Cram-Schmidt' will never update transcription text. This is correct behaviour for an archival system." The listener should be deleted as part of this issue. Also check
PersonDisplayNameChangedEvent— if the listener is its only consumer, delete the event and its publisher too.The exact surgical change in
PersonMentionEditor.svelte:Lines 123–131 currently do:
The fix is: substitute the typed query text, not the DB display name:
Where
currentQueryis the result ofdetectPersonMentionat the time of selection — it's already tracked as state in the component (the value passed to the search input). ConfirmmentionStartis still in scope at selection time.AC-4 (atomicity) is the technically hard part. The editor is a
<textarea>. True atomic tokens in a textarea requirebeforeinput/keydowninterception + cursor tracking. Minimum viable implementation: interceptbeforeinput, detect if the edit range overlaps a mention span, and either snap the deletion to cover the whole mention or do nothing. The spec needs to define the exact expected behavior before implementation starts (see Open Decisions).renderTranscriptionBodyinmention.tsrequires no changes — it already readsdisplayNamefrom the sidecar and uses it as link text. Backward compat with old-format blocks (full names in sidecar) works automatically.Recommendations
PersonMentionPropagationListenerand its associatedPersonDisplayNameChangedEventas part of this issue — not as a follow-up.PersonMentionEditor.sveltelines 123–131 to capture the typed query, notperson.displayName.PersonMentionEditor: (a) selected person stores typed text asdisplayName, not DB name; (b)mentionedPersonssidecar contains the typed text; (c) an explicit regression test: when a user types@Claraand selects "Clara Cram", the rendered text is@Clara, not@Clara Cram.Open Decisions
beforeinputintercept prevents any cursor entry; (B) Delete-triggers-full-removal — backspace from immediately after the mention removes the whole token + sidecar entry; (C) Soft — no enforcement, sidecar becomes inconsistent if user edits the mention manually. Option B is the lowest-effort implementation that satisfies the spec's intent without requiring a contenteditable rewrite.🏛️ Markus Keller — Application Architect
Observations
The semantic contract of
PersonMention.displayNamechanges fundamentally. Currently it stores a denormalized copy ofPerson.displayName— a derived value, kept in sync byPersonMentionPropagationListener. Under this issue it becomes archival data: the word the transcriber originally typed. These are different concerns. The current field namedisplayNameis now misleading — it implies "how this person displays their name" rather than "what the scribe wrote."Checking the model:
No schema change is needed (same column, same constraints — 200 chars is ample). But renaming this field to
transcribedTextorauthorTextin the backend model and the TypeScriptPersonMentiontype would make the new contract explicit and prevent the next developer from treating it as a DB-name cache again. A rename is a migration (ALTER COLUMN) plus type regeneration — scope it as a follow-up unless the team prefers to do it atomically here.PersonMentionPropagationListenerviolates the new design invariant and must be removed. The listener holds aTranscriptionBlockRepositoryreference and joins the person-rename transaction. Once removed,PersonDisplayNameChangedEventmay have no other consumers — check the event publisher inPersonServiceand remove the event dispatch if so.No new architectural layer violations. The change is confined to:
PersonMentionEditor.svelte(editor behavior),PersonMentionPropagationListener.java(deletion), and optionally a column rename. The rest of the stack (read rendering, API, storage) is unchanged.Recommendations
PersonMentionPropagationListenerin this issue — keeping it with a conditional would add dead complexity.displayName→transcribedText(orauthorText) in a follow-up issue to lock in the semantic boundary. Do not include the rename in this PR — it adds a migration and type regeneration without changing behavior.PersonMention.javaas a one-line comment:// Archival: the text the transcriber typed after @. Never updated on person rename.🔒 Nora "NullX" Steiner — Security Engineer
Observations
No new attack surfaces introduced. The change substitutes the source of
displayNamein the sidecar: instead of coming from the DB (person.displayName), it comes from user-typed input. The security properties are unchanged because:XSS path:
renderTranscriptionBodycallsescapeHtml(mention.displayName)before injecting into the anchor tag (line 136 inmention.ts). This was already true for the DB name; it equally covers the typed text. The existing test'escapes HTML in displayName to prevent stored XSS'inmention.spec.tscovers this path.personIdvalidation: TheisUuid()guard inrenderTranscriptionBodyalready rejects non-UUIDpersonIdvalues before rendering anchors. The user cannot inject apersonIdvia the typed text —personIdcomes from the API response (person.id), not from what the user types.Backend
@Size(max = 200)constraint ondisplay_namecolumn still applies. The typed text is bounded by this. ThedetectPersonMentionquery text is limited by what can be typed before a newline or second@, which in practice is well under 200 chars.One thing to verify: the
PersonMentionEditor.sveltedropdown search input setscurrentQueryfromdetectPersonMention(value, cursorPos). This returns the raw typed string including any special characters. Confirm that when the user types@<script>, thementionedPersonssidecar entry'sdisplayNamecarries<script>as a string — which is then HTML-escaped byrenderTranscriptionBodybefore injection. The escaping happens at render time, not at store time, which is fine.PersonMentionPropagationListenerdeletion removes a write path from the rename transaction — this is a security-neutral change. Fewer writes in a transaction is strictly better.Recommendations
mention.spec.tsconfirmingrenderTranscriptionBodyescapes a<script>tag indisplayName— even though it already works, it explicitly documents the security invariant for the new "typed text as displayName" scenario.🧪 Sara Holt — QA Engineer
Observations
AC-4 lacks a verifiable acceptance criterion. "The token is selected as a whole unit; individual characters cannot be edited" is a statement of mechanism, not a behavioral assertion. A test cannot assert "individual characters cannot be edited" — it can only assert an observable outcome (e.g., pressing Backspace removes the whole mention). The AC needs a Given-When-Then before implementation:
Without this, implementation can ship "soft atomicity" (cursor can enter the mention but the sidecar becomes stale) and still claim AC-4 passes.
AC-5 (unresolved
@Fritzstays) is already correct behavior —renderTranscriptionBodyonly replaces@DisplayNamewhen there's a sidecar entry. No sidecar entry for@Fritz→ it stays as plain text including the@. This should get a regression test inmention.spec.tssince it's now a named invariant.Backward compat (AC-6) needs an explicit regression test. The spec says old-format blocks (
@[Clara Cram](uuid)— actually in this codebase, old format is@Clara Cramin text +{personId, displayName: "Clara Cram"}in sidecar) continue to render correctly. This works automatically becauserenderTranscriptionBodymatches whatever is indisplayNameagainst the text. But a unit test confirming this would prevent a future refactor from breaking AC-6 silently.PersonMentionPropagationListenerdeletion needs tests cleaned up. There should be tests for the listener inPersonMentionPropagationListenerTest.java— these should be deleted along with the listener, not left in a dead state.The
@Fritz→ plain text path in read mode: confirm that@Fritzin the rendered HTML doesn't get a stray anchor around it. CurrentlyrenderTranscriptionBodyreturns the escaped text with@Fritzintact (no anchor). A test inTranscriptionReadView.svelte.test.tswould confirm the rendered DOM has no<a>tag.Recommendations
@text), AC-6 (old-format full-name sidecar still renders link correctly), and XSS in typed text (already exists — verify it still covers the new flow).PersonMentionPropagationListenerTest.javaalongside the listener.🎨 Leonie Voss — UX Designer & Accessibility Strategist
Observations
AC-4 (atomic token) vs current textarea UX — The editor is a
<textarea>. There's no visual distinction between mention tokens and plain text —@Claraafter locking looks identical to manually typed@Clara. This creates two UX gaps:@Clarainstances are locked mentions vs plain text.At minimum, the locked mention should have a visual indicator in the edit view (e.g., subtle background highlight via a custom input or overlay). If the editor stays as
<textarea>, this is technically difficult. A lightweight approach: show a chip-style list of locked mentions below the textarea (similar to the currentmentionedPersonsdisplay in the UI) so users know which mentions are resolved.AC-5 (unresolved
@Fritz) —@symbol must be preserved.renderTranscriptionBodycurrently returns the escaped text with@Fritzintact when no sidecar match is found. The@is a meaningful archival signal ("person reference was intended"). Confirm the CSS for.person-mention(underline, mint accent) does NOT apply to unresolved@Fritztext. It shouldn't — unresolved text is plain, no anchor, no class.Hover card
geb.is hardcoded German — InPersonHoverCard.svelte(PR #371), the maiden name line rendersgeb. {state.person.alias}. Thegeb.prefix is German ("geborene"). This should be a Paraglide key ({m.person_mention_born_name_prefix()}) so the EN and ES interfaces don't show German text. Raise this now before it's deployed.Touch behavior (AC per PR #371) — On touch devices, the CSS
@media (hover: none) { .person-hover-card { display: none; } }ensures tap navigates directly. Good call. No concern here.The hint text
.hintin the hover card footer — The spec shows{m.person_mention_hover_hint()}which should explain how to navigate (e.g., "Klicken zum Öffnen"). Confirm this is a real Paraglide key and not a placeholder before shipping.Recommendations
geb.prefix inPersonHoverCard.sveltebefore or alongside this issue.🚀 Tobias Wendt — DevOps & Platform Engineer
Observations
PersonMentionPropagationListenerdeletion simplifies the rename transaction. The listener joins the person-rename transaction synchronously — one less DB write per block per rename. Clean removal. No infra change needed.No schema migration.
display_namecolumn stays as-is. No Flyway file required. ✓Rolling deployment is safe. Old code writes
displayName: person.displayName(full DB name). New code writesdisplayName: typedText. Both formats render correctly inrenderTranscriptionBodybecause both match the@textpattern in the block text. Old instances running old code, new instances running new code — both produce consistent read-mode rendering.Nothing to flag from my side. No new services, no new infrastructure, no config changes.
Recommendations
📋 Elicit — Requirements Engineer
Observations
AC-3 ("dropdown pre-fills with typed text") is already implemented and does not need to be built.
detectPersonMentionreturns the current query, which is passed to the search input. The spec should note this as "no change required" to prevent the implementer from re-engineering a working system.AC-4 is the scope risk. "The token can only be removed by deleting it wholesale — it cannot be edited in-place" implies constraint enforcement in a
<textarea>-based editor. Three options exist:beforeinputintercept blocks cursor entry; any edit attempt inside a mention span is suppressedbeforeinput+ cursor trackingThe spec as written implies Option A. Option B satisfies the practical goal with far less complexity. Option C is what ships today if this AC is never implemented.
Sidecar-text consistency invariant is implicit but not specified. If a user types
@Clar(editing the mention) and then saves, the sidecar still has{personId, displayName: "Clara"}but the text has@Clar.renderTranscriptionBodywon't find@Clarin the mention list and will render it as plain unlinked text. This is a graceful failure, but it's worth stating explicitly: "If the text and sidecar become inconsistent, read mode renders the@textas plain text with no link." This should be a named invariant in the spec, not an accidental behavior."No migration required" (AC-6) should be stated as a constraint, not just an acceptance criterion. The implementation must not introduce any code that backfills or transforms existing
mentionedPersonsdata.Recommendations
🗳️ Decision Queue — Action Required
1 decision needs your input before implementation starts.
Editor UX
AC-4 atomicity level for locked mention tokens — The editor is
<textarea>-based. The spec says tokens "cannot be edited in-place" but the mechanism is undefined. Three options, pick one:beforeinputintercept prevents cursor from entering the mention span@Claradeletes the whole token and removes it frommentionedPersons; cursor can still enter the span otherwiseOption B covers the core use case (accidental deletion of a mention should remove it cleanly), avoids a contenteditable rewrite, and is testable. (Raised by: Felix, Leonie, Elicit)
👨💻 Felix Brandt — Senior Fullstack Developer
Discussion summary: resolved the open
contenteditablequestion and locked in the implementation approach for this issue.Resolved
Visual distinction is a blocker — The target audience (non-technical transcribers, 60+) cannot work without a visual affordance distinguishing locked mention tokens from plain text. The current
<textarea>cannot provide this. Option B from the decision queue (delete-triggers-removal only) is ruled out — it defers the visual problem without a committed path back to it.contenteditablethreshold crossed — The visual distinction requirement is the criterion that tips the scales toward a rich-text editor. We've crossed it. The textarea approach is off the table for this feature.Tiptap v3.22.5 chosen — First-class atomic inline node support, mention extension pattern, TypeScript, and Svelte integration. The existing sidecar shape (
mentionedPersons: { personId, displayName }) and serialization format (@[text](uuid)) stay unchanged — Tiptap mention node attributes map directly onto them.Tiptap migration absorbed into this issue — The
displayNamesource fix (typed text instead of DB name) ships as part of the Tiptap editor rewrite, not as a standalone textarea patch. Shipping the behavioral fix without the visual treatment would leave transcribers in a broken UX — the two changes are coupled.Implementation shape (for the implementer)
PersonMentionEditor.sveltebecomes a Tiptap editor with a custom mention extension. The mention node carries{ displayName: typedText, personId }as node attributes —displayNameis set fromcurrentQueryat selection time, not fromperson.displayName. ThementionedPersonssidecar is derived from the document's mention nodes. AC-4 atomicity is handled by the Tiptap node model for free — atomic nodes cannot be partially edited.Overall read
The previous decision queue correctly identified AC-4 as the scope risk. This discussion confirms that the right resolution is a Tiptap migration, not a
beforeinputworkaround. The migration is self-contained toPersonMentionEditor.svelte; the rest of the stack (serialization, rendering, backend) is unaffected.👨💻 Felix Brandt — Senior Fullstack Developer
Follow-up discussion: Tiptap integration design decisions.
Resolved
Storage format — keep
@[text](uuid)+ sidecar, write a serializer — No backend changes,renderTranscriptionBodystays untouched, AC-6 stays clean. Two pure functions:deserialize(text, mentionedPersons) → Tiptap JSONon load (converts sidecar entries to mention nodes),serialize(doc) → { text, mentionedPersons }on save (walks the Tiptap doc tree to extract mention nodes). No Tiptap JSON stored in the database.Dropdown — new
MentionDropdown.svelte,PersonTypeaheaduntouched —PersonTypeaheadis an autonomous form widget (owns its input, open/close state, anchor-relative positioning,on:changecallback). Tiptap's suggestion system is externally driven: Tiptap owns the query, open/close lifecycle, keyboard events, and expectscommand(item)on selection with cursor-relativeclientRectpositioning. AdaptingPersonTypeaheadto this contract means overriding every aspect of its design. A newMentionDropdown.svelteis built to the contract directly: floating list, arrow/Enter/Escape keyboard handling via an exposedonKeyDown, same person search endpoint. Visual consistency via shared Tailwind tokens.Implementation shape
Editor mount (Svelte 5):
Mention node insertion (on person select):
displayNameis set from the typed query, notperson.displayName— this is the AC-1 fix.Dropdown imperative mount (Svelte 5):
🎨 Leonie Voss — UX Designer & Accessibility Strategist
Design decisions resolved after the Tiptap migration discussion. All five items are agreed and in scope for this issue.
Resolved
1. Mention token visual design (edit mode)
Use
underline decoration-brand-mint text-brand-navy font-mediumon the Tiptap mention node's rendered<span>. Echoes the read-mode anchor style — no extra visual weight, consistent with existing brand patterns.2.
MentionDropdown.sveltevisual specPersonTypeahead's existing results listfont-serif text-brand-navy), secondary line =* birthYear † deathYearusing genealogical symbols (language-neutral, no Paraglide keys needed for the symbols)3.
geb.hardcoded German inPersonHoverCard.svelteFix is in scope for this issue. Replace the hardcoded
geb.prefix with{m.person_born_name_prefix()}and add translations:de.json:"geb."en.json:"née"es.json:"n."(confirm Spanish convention)4. Mobile dropdown positioning — flip strategy
In scope. The
MentionDropdownmust measure available space below the cursor rect and open upward whenspaceBelow < 200px:Covers the primary transcription device (tablet) where the virtual keyboard reduces visible viewport height.
5. Tiptap editor container styling
The
contenteditablediv replaces a<textarea>and must be explicitly styled to match. Baseline:Use
focus-within(notfocus) — the innercontenteditablereceives focus, not the wrapper div.Overall read
The Tiptap migration decision was the right call — the visual affordance requirement was always going to be a blocker for the 60+ transcriber audience. The five items above close the remaining design gaps before implementation starts. No unresolved items from my side.
👨💻 Felix Brandt — Senior Fullstack Developer
Review of the locked-in Tiptap migration plan.
Observations
The entire existing test suite is dead code the moment migration lands.
Every test in
PersonMentionEditor.svelte.spec.tscallsdocument.querySelector('textarea')via thegetTextarea()helper. After the Tiptap migration that function returnsnulland every single test throws. This isn't a suite that needs updating — it's a full rewrite of all 12 tests. Plan this before touchingPersonMentionEditor.svelte, not after.captureTextareais in active production use — not optional dead code.TranscriptionBlock.svelte:86-91definescaptureTextarea()and passes it toPersonMentionEditorat line 194.handleTextareaMouseUp(line 105) readstextareaEl.selectionStart/textareaEl.selectionEndto implement quote selection — the user highlights text and the block captures it asselectedQuote. This feature will silently break if the prop is just removed.The correct replacement: remove
captureTextarea+handleTextareaMouseUpfromTranscriptionBlock.svelteand instead register aselectionUpdatehandler on the Tiptap editor:This should be done in the same PR as the editor rewrite — not a follow-up. Shipping the Tiptap editor without quote selection is a regression.
onUpdatein the suggestionrender()needs a concrete Svelte 5 implementation.The discussion shows
onStartmounting the component butonUpdateas a comment stub. With Svelte 5'smount, you can't update props imperatively after mounting without either: (a) exposing a$bindableor$stateexport fromMentionDropdown.sveltethatonUpdatewrites to, or (b) callingunmount+mounteach time (wasteful). The canonical approach for Svelte 5:This needs to be spelled out in the implementation shape — "update props" is ambiguous in Svelte 5.
resizeTextarea()and the$effect(() => { void value; resizeTextarea(); })pattern disappears. Tiptap'scontenteditablegrows with content via CSS — no JS height logic needed. The old implementation was a workaround for<textarea>not auto-growing.Recommendations
serialize/deserializeas pure functions in a newsrc/lib/utils/mentionSerializer.ts. TDD them first with Vitest node tests before any editor code — they're the most testable piece and have no DOM dependencies.page.getByRole('textbox')for the Tiptap contenteditable (once ARIA attributes are set — see Leonie's comment),userEvent.type()for text input, andpage.getByRole('option')for dropdown items.TranscriptionBlock.sveltequote-selection migration in this PR scope.Open Decisions
captureTextareamigration scope: The prop is live inTranscriptionBlock. Options: (A) ReplacecaptureTextareawith aselectionUpdateapproach in this PR (keeps the feature); (B) File a follow-up and ship with broken quote selection temporarily. Option A is the right call — the scope is a 10-line change inTranscriptionBlock.svelte.🏛️ Markus Keller — Application Architect
Review of the Tiptap migration plan.
Observations
Bundle size: ProseMirror is a significant dependency addition.
@tiptap/corepulls in the full ProseMirror stack:prosemirror-model,prosemirror-state,prosemirror-view,prosemirror-transform, and more. Combined with@tiptap/starter-kitand@tiptap/extension-mention, this adds approximately 200–350 KB gzipped to the JS payload. The transcription panel is mounted on every document detail page — a route used by transcribers on 60+ tablets. Measure the before/after chunk size withnpm run buildand report it in the PR description.If the increase is material (>100KB on the critical path), the right response is dynamic import at the call site in
TranscriptionBlock.svelte:This code-splits the Tiptap bundle into its own lazy chunk. The transcription panel only loads it when the block is in edit mode, not on first paint.
SSR compatibility: verify the usage path is safe.
Tiptap calls
document.createElementduring instantiation — it cannot run during SvelteKit SSR. TheonMountguard in the proposed code is correct. ButPersonMentionEditoris used insideTranscriptionBlock.svelte, which is used inside the transcription panel. Verify that the route rendering the panel either hasexport const ssr = falseor thatTranscriptionBlockis only rendered conditionally on the client. A server-side render hitting Tiptap'snew Editor(...)will throw at runtime.mentionSerializer.tsis a new module boundary — give it an explicit contract.The discussion resolves to "write serialize/deserialize functions" but the contract is not documented in the issue. Before implementation, the module's interface should be defined:
The round-trip invariant:
serialize(deserialize(text, sidecar)).text === textfor all valid inputs. This is independently testable without a browser.@tiptap/*packages share ProseMirror peer dependencies and must version-lock together.@tiptap/core,@tiptap/starter-kit, and@tiptap/extension-mentionare all part of the same release train. A mismatch (e.g.,core@2.xwithstarter-kit@3.x) causes silent runtime breakage. Renovate needs a package group rule before the PR lands, otherwise the next automated bump will update only one package.Recommendations
@tiptap/*before or alongside the PR. If norenovate.jsonexists, create one with a minimal@tiptapgroup rule.mentionSerializer.tsinterface in the issue (input type, output type, round-trip invariant) before implementation starts.No open decisions from my side — all items have clear answers.
🔒 Nora "NullX" Steiner — Security Engineer
Review of the Tiptap migration plan.
Observations
Tiptap's mention node rendering pipeline is XSS-safe by construction — but verify the toDOM spec.
ProseMirror's
NodeSpec.toDOM()creates DOM elements programmatically (document.createElement), not viainnerHTML. The mention node tuple['span', { class: '...', 'data-person-id': attrs.personId }, 0]produces a real DOM node, with text content inserted as a text node by ProseMirror's renderer. This is safe as long asattrs.displayNameis inserted as node content (the0slot in the toDOM tuple), not as a raw HTML attribute that could contain<script>payloads. The0slot is always escaped by ProseMirror. Confirm theNodeSpecimplementation follows this pattern rather than usinginnerHTMLor a custom render function.MentionDropdownmounted todocument.body— verify cleanup is guaranteed on all exit paths.The Svelte 5
mount(MentionDropdown, { target: document.body, props })appends the dropdown todocument.body. IfonExitis not called — e.g., when the Tiptap editor is destroyed mid-suggestion (user navigates away, block is deleted) — the dropdown leaks into the DOM.Tiptap's suggestion plugin calls
onExitwhen the plugin state transitions out of an active suggestion AND when the plugin is destroyed. However, ifeditor.destroy()is called while a suggestion is in progress, verify that the suggestion plugin teardown reliably firesonExit. Add a guard:The
nullcheck prevents a double-unmount ifonExitis called more than once.CSP: ProseMirror applies inline styles — check the header config.
ProseMirror sets inline
styleattributes on thecontenteditableelement for cursor positioning (e.g.,style="white-space: break-spaces"). If the project's Caddy config or Spring Security applies aContent-Security-Policywithstyle-src 'self'(no'unsafe-inline'), ProseMirror will fail silently to style the editor correctly in production. This is a deployment concern to check before going live.The
fetch('/api/persons?q=...')client-side call moves toMentionDropdown.svelte— the security comment from the currentPersonMentionEditor.svelte(lines 100–104) documents why this client-side fetch is safe. That comment must move toMentionDropdown.svelte. Don't lose the rationale in the migration.No new XSS vectors from the
displayNamechange — the existingescapeHtml(mention.displayName)inmention.ts(renderTranscriptionBody) continues to cover stored XSS. For the Tiptap editor specifically,attrs.displayNameis inserted as a text node (safe). The security invariant is unchanged.Recommendations
NodeSpec.toDOM()in the implementation shape: use the['span', attrs, 0]tuple pattern where0is the content slot — do NOT useinnerHTMLin any custom render logic.if (component)null guard toonExitin the suggestionrender()function.style-srcbefore shipping to production — add'unsafe-inline'tostyle-srcif needed, or confirm ProseMirror doesn't trigger violations in the actual production header config.fetchsecurity comment toMentionDropdown.svelte.No open decisions from my side.
🧪 Sara Holt — QA Engineer
Review of the Tiptap migration plan.
Observations
All 12 existing component tests are broken by this migration — plan the rewrite before starting.
PersonMentionEditor.svelte.spec.tsis built entirely arounddocument.querySelector('textarea')viagetTextarea(). After migration that returnsnull. Beyond the helper, every test directly manipulatesta.value,ta.selectionStart,ta.selectionEnd, and dispatchesnew Event('input', ...)on the textarea element. None of this applies to a Tiptapcontenteditable. The test file is a full rewrite, not a patch.Good news:
vitest-browser-svelteuses real Playwright/Chromium, so Tiptap's DOM APIs work correctly. The new tests should drive the editor viauserEvent.type()(simulates real keystrokes including Tiptap's input event handling) rather than directly manipulating DOM state.New test approach per spec section:
ta.value = '@Aug'; ta.dispatchEvent(new Event('input'))await userEvent.type(editor, '@Aug')expect(host.snapshot.value).toBe('@Auguste Raddatz ')serialize(editor.getJSON())or read the contenteditable textgetByRole('textbox').toHaveValue(...)getByRole('textbox').toHaveText(...)(contenteditable uses text, not value)ta.selectionStart = 4userEvent.typepositions cursor naturallyThe serializer tests are missing from the issue scope entirely.
mentionSerializer.ts(or equivalent) will containserializeanddeserialize— pure functions with no DOM dependency. These should have their own Vitest node test file (mentionSerializer.spec.ts) covering:deserialize('')→ valid empty Tiptap documentdeserialize('Hello @Clara world', [{ personId: 'uuid-x', displayName: 'Clara' }])→ doc with mention node at correct positionserialize(doc)round-trips the text and sidecar correctlyserialize(deserialize(oldFormatText, oldSidecar))→ backward compat for full-name sidecar entriesThese run in <5ms and can be written before any editor code (TDD the serializer first).
PersonMentionPropagationListenerTest.javamust be deleted in the same PR as the listener.The test class is referenced in
backend/src/test/.../PersonMentionPropagationListenerTest.java. Leaving dead test code is not acceptable — a passing test for a deleted listener gives false coverage signals.The suggestion
render()lifecycle is difficult to test at the component level — write at least one integration smoke test.Define one integration test that proves the end-to-end flow works:
This test exercises
onStart,onKeyDown,onExit,insertContentAt, andserializetogether. It's the regression test for AC-1.Recommendations
mentionSerializer.spec.tsat the Vitest node layer before any editor code — these tests define the serializer contract.PersonMentionEditor.svelte.spec.tstest names before implementation (as a stub file withit.todo(...)) so the test scope is agreed before the PR opens.PersonMentionPropagationListenerTest.javaandPersonServiceTest.javatests that set upPersonMentionPropagationListener(lines 308–338 in that file) in the same commit as the listener deletion.No open decisions from my side.
🎨 Leonie Voss — UX Designer & Accessibility Strategist
Review of the Tiptap migration plan.
Observations
Tiptap's
contenteditableneeds explicit ARIA attributes — they are NOT added automatically.The Tiptap editor renders a
<div contenteditable="true">with noroleattribute by default. Screen readers announce this as a generic editable region without a role name, aria-multiline, or accessible label. This is a Critical accessibility gap for our audience. Fix viaeditorProps.attributes:Add a Paraglide key
transcription_editor_aria_labelwith value"Transkriptionstext"/"Transcription text"/"Texto de transcripción".focus-withinring works correctly with ProseMirror's focus model — but verify the placeholder.The decided CSS uses
focus-within:ring-2on the wrapper<div>. This correctly propagates from ProseMirror's innercontenteditableto the wrapper — no issue there. However, the current<textarea>had aplaceholderattribute. Tiptap'scontenteditabledoes NOT support theplaceholderattribute. Implement via CSS + Tiptap'seditorProps:Where
data-emptyis toggled via Tiptap'sonUpdatecallback:editorEl.dataset.empty = String(editor.isEmpty). This is the canonical Tiptap placeholder approach.Mention
<span>in edit mode needs a screen reader label.The decided visual is
underline decoration-brand-mint text-brand-navy font-mediumon the mention span. Screen readers will read only the visible text (Clara). For a user navigating by keyboard, knowing that "Clara" is a resolved person mention (not just a word) requires an accessible annotation. Options:aria-description="Verknüpfte Person: {fullName}"on the span (if the full name is available in node attrs)title="{fullName}"as a tooltip fallbackThe mention
NodeSpec.toDOM()has access toattrs.personIdbut not the full name at render time in the editor. The simplest accessible solution: render the typed text as visible, and append a visually hidden<span class="sr-only"> (Person)</span>as a child node.Dropdown cursor rect:
rect.leftis the anchor X position,rect.bottom/rect.topis the Y.The decided flip logic
const openUpward = spaceBelow < 200usesrect.bottomfrom Tiptap'sclientRect(). This is correct. One edge case: on some browsers, the caret DOMRect haswidth: 0. Position the dropdown usingrect.left(notrect.right) for X, and handlerect.width === 0by falling back torect.x.Recommendations
role="textbox",aria-multiline="true", andaria-labeltoeditorProps.attributes— this is a Critical accessibility requirement, not optional polish.data-placeholderattribute + CSS::beforepseudo-element, not theplaceholderHTML attribute (which doesn't work oncontenteditable).transcription_editor_aria_label(de/en/es) before the PR opens.(Person)annotation on mention spans for screen reader users who navigate by text.No open decisions from my side.
🚀 Tobias Wendt — DevOps & Platform Engineer
Review of the Tiptap migration plan.
Observations
Four new npm packages — they must be version-grouped in Renovate.
@tiptap/core,@tiptap/starter-kit,@tiptap/extension-mention, and likely@tiptap/pm(ProseMirror types) all share ProseMirror peer dependencies internally. A version mismatch — e.g., Renovate bumping only@tiptap/corewhile leaving@tiptap/extension-mentionbehind — causes runtime errors that are silent at install time but break at runtime. Before or alongside this PR, add a Renovate group rule:If there's no
renovate.jsonyet in the repo root, create one. This is a one-time setup.Bundle size: measure before landing the PR.
ProseMirror + Tiptap adds material JavaScript weight. The transcription panel is on a hot path. Run:
...before and after adding the Tiptap packages, and include the delta in the PR description. If the critical-path chunk grows by more than ~100KB gzip, consider a dynamic
import()at the call site (Markus's comment covers this). I just need the number to know whether the current CX32 VPS (8GB RAM) tablet load time changes materially.No CI changes needed. No new Docker services, no new env vars, no new Playwright browsers. The existing
vitest-browser-svelteChromium setup handles Tiptap's DOM requirements. Clean.No infrastructure impact from
PersonMentionPropagationListenerdeletion. Fewer DB writes per rename transaction is strictly better — no operational concern.Recommendations
@tiptapgroup rule before the PR merges.📋 Elicit — Requirements Engineer
Review of the Tiptap migration plan.
Observations
"Tiptap v3.22.5" must be resolved to a verified NPM version before implementation starts.
The discussion commits to a specific version. The implementer must confirm the exact package name(s) and versions on npmjs.com before
npm install. Pin them exactly inpackage.json(e.g.,"@tiptap/core": "3.22.5"not"^3.22.5") to prevent drift between the agreed version and whatever the resolver picks. If the version turns out not to exist, the issue needs updating with the correct latest stable version before any code is written.captureTextareahas no acceptance criterion covering its migration — this is a regression risk.The prop is actively used by
TranscriptionBlock.sveltefor quote selection (the user selects text in the editor, andselectedQuoteis set for a citation feature). The issue currently has no AC that covers this feature's behavior after the migration. Add:AC-7 — Quote selection works in Tiptap editor
Without this AC, the implementer may not know the quote-selection feature exists, and it will regress silently.
serialize/deserializehave no acceptance criterion — add AC-8.The discussion resolves to "write a serializer" but the issue body doesn't define what correct behavior looks like. This leaves the implementer without a testable target. Add:
AC-8 — Serializer round-trip preserves text and sidecar
The SSR constraint is an implicit NFR with no specification.
Tiptap's
Editorconstructor callsdocument.createElementand will throw during SvelteKit's server-side render. The current implementation correctly usesonMount, but this constraint should be stated explicitly so future maintainers don't accidentally move initialization outsideonMount:AC-3 ("dropdown pre-fills with typed text") is still met, but the mechanism changes.
In the Tiptap architecture,
props.queryfrom the suggestion system replacesdetectPersonMention(). The AC is still correct as a requirement, but the implementation note in the issue should reference the new mechanism so QA can verify it correctly.Recommendations
props.queryfrom Tiptap's suggestion system).Open Decisions
captureTextareamigration scope: Does AC-7 land in this PR or as a follow-up? Based on Felix's analysis, theTranscriptionBlockchange is ~10 lines. Deferring it means shipping a regression. Recommend including it in scope with AC-7 as the acceptance criterion.🗳️ Decision Queue — Action Required
1 decision needs your input before implementation starts.
Scope
captureTextareamigration —TranscriptionBlock.svelte:86-194uses thecaptureTextareaprop to capture a reference to the textarea element and readselectionStart/selectionEndon mouseup for the quote-selection feature (selectedQuote). The Tiptap migration removes the underlying<textarea>, so this feature silently breaks unless addressed. Three options:captureTextarea+handleTextareaMouseUpwith aselectionUpdatelistener on the Tiptap editor inTranscriptionBlock.svelte; add AC-7 to the issueTranscriptionBlock.svelteOption A is recommended: the scope is a single short function swap in
TranscriptionBlock.svelte, and quote selection is currently working — shipping a regression for a feature that users have is not a good trade-off. (Raised by: Felix, Elicit)We fix the quote in this PR