Felix #7: handleMentionClick unconditionally preventDefault'd and goto'd,
breaking ctrl-click / cmd-click / shift-click / alt-click / middle-click —
"open in new tab" is a real workflow for researchers comparing two persons.
Add isPlainPrimaryClick() guard. Modified clicks fall through to the
browser's default anchor handling (the <a href="/persons/{id}"> opens in
the new tab as expected). Plain left-clicks still SPA-navigate via goto().
Three new tests assert ctrl-click, meta-click, and middle-click are not
preventDefault'd.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Felix #1: fetchHoverData was doing four things — cache lookup, fetch, JSON
parsing, 404 normalisation. Split into:
loadHoverData(personId) — pure fetch + 404→null + non-OK→throw
getOrFetchHoverData(personId) — five-line cache wrapper around the above
Also document the cache-lifetime trade-off (Markus #4, Elicit OQ-372-02):
the cache is per-mount, so closing and reopening the transcription panel
rebuilds it. That's intentional given the read-only nature of the view —
revisit if stale-card user reports surface.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three reviewer concerns land here:
- Felix #2: magic numbers 0.7 and 300 belong in named constants
- Sara #6: the position function had 4 branches and 2 thresholds with zero tests
- Leonie FINDING-05: at 320px viewport the flip-left could push the card
past the right edge — needed a viewport clamp
Move the function to src/lib/utils/hoverCardPosition.ts as a pure
(rect, viewport) → {top, left} mapping, with named exports CARD_WIDTH_PX,
CARD_HEIGHT_PX, CARD_GAP_PX, BOTTOM_BAND_RATIO, RIGHT_FLIP_THRESHOLD_PX.
Add a viewport clamp so left + CARD_WIDTH never exceeds the right edge.
Ten unit tests cover default placement, flip-up (both triggers), flip-left,
flip-right-edge clamp, and scroll offset. TranscriptionReadView passes the
current window viewport in on each call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Markus flagged that 'a.person-mention' is a magic string repeated four times
in TranscriptionReadView, plus the CSS rule, plus tests. Extract into a single
exported constant so the renderer template, the delegated event handlers,
and the consumer-side selectors all import the same value.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Nora's CWE-601 (Open Redirect) defense-in-depth concern: today the backend
emits UUIDs, but renderTranscriptionBody concatenates personId straight into
an href. If a future "external person" feature ever flows a non-UUID through
the sidecar, the renderer would happily emit `<a href="javascript:…">`.
Add a strict UUID regex check before substituting. Non-UUID entries fall
through unchanged so the @-trigger remains as plain text — no silent data
loss, no clickable redirect.
Three new failing→passing tests cover javascript: scheme, absolute URL, and
the positive case (well-formed UUID still renders). Existing tests that used
synthetic IDs ("p-short", "p-first", etc.) updated to real UUIDs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Markus, Felix, and Nora independently flagged the {@html …} boundary as a
distributed-knowledge security risk: today renderBody and renderTranscriptionBody
return string, so the next refactor that does {@html block.text} (instead of
{@html renderBlockHtml(block)}) is one typo away from a stored-XSS regression.
Introduce a SafeHtml brand type (string with a phantom __brand) returned by
both renderers and by renderBlockHtml in TranscriptionReadView. Compile-time
enforcement of the escape invariant — costs zero runtime, makes the contract
auditable in one file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Markus flagged the LoadState export from PersonHoverCard.svelte as a
view-vs-orchestrator boundary smell — both files own the same shape, and a
third caller (admin previews, briefwechsel cards) would create a circular
import. Move the types into src/lib/types/personHoverCard.ts so the contract
is module-stable.
Also harden .prettierignore + eslint.config.js so a stray .svelte-kit.old/
backup directory (rotated by SvelteKit during dev) doesn't break the lint
hook — matches the existing .svelte-kit-backup/ convention.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Composes splitByMarkers + renderTranscriptionBody so [unleserlich]
markers render as <em data-marker> siblings of the mention anchor —
neither nested inside the other (B19b).
Hover card lifecycle on each .person-mention anchor:
mouseenter → set aria-describedby, place card via getBoundingClientRect
(default below-right; flip up if <200px from bottom or
mention is in bottom 30% of viewport; flip left if
<300px from right), fire fetch, mount card with
skeleton state
resolved → swap card to loaded state with person + family
relationships (PARENT_OF / SPOUSE_OF / SIBLING_OF only)
404 → degrade: mark anchor with data-person-deleted="true",
unmount card, suppress future hovers/clicks
network → swap card to error state — link still navigates
mouseleave → drop aria-describedby, unmount card
Per-page SvelteMap<personId, Promise> cache (B15.5) so a sweep across
N mentions of the same person fires the backend once. Click handler
calls goto() so SvelteKit handles routing without a full reload.
Event listeners are attached once per article via a Svelte action
because the anchor HTML is injected via {@html ...} and would not
receive declarative bindings. The eslint-disable comment mirrors
the rationale on CommentMessage.svelte:88-89.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The card has three render states:
- loading → 320×180 skeleton with three pulse-animated bars; respects
prefers-reduced-motion (animation disabled, opacity dimmed)
- error → generic load-error message in the body; the footer link
still navigates (click works regardless of fetch outcome)
- loaded → navy header with name, life-date range, and "geb. <alias>";
family-only relationship chips (PARENT_OF / SPOUSE_OF /
SIBLING_OF) — non-family types are filtered out;
notes excerpt capped at 120 chars with ellipsis;
footer with "Zur Person →" + hover hint
aria-live="polite" on the card root so screen readers announce loaded
content when the fetch resolves; the host's id is the cardId so the
parent anchor can use aria-describedby. The card is hidden via
@media (hover: none) on touch devices — tap navigates directly per
spec.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces every @DisplayName in a transcription block's text with an anchor
link to /persons/{personId}, sourced from the mentionedPersons sidecar.
The @ prefix is stripped from the rendered link text per spec — it is an
editor affordance, not part of the historical text.
Stored-XSS hardening: HTML-escapes block text, displayName, and personId
before injection. Word-boundary lookahead avoids prefix collisions
(@Hans vs @HansMüller). Longest-displayName-first + first-sidecar-wins
make rendering deterministic for the OQ-1 collision case (#5339).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tester #5506 nit pile:
- '@Aug @Bert' with cursor past the second @ — confirm the most
recent @ wins (this is the canonical case for typing two mentions
separated by a space).
- '@Aug\\nfoo' with cursor exactly at the newline (index 4) — the
query still reads 'Aug' because the newline is past the cursor.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tester #5506 §2 + Markus #5504 §2: the 409 orchestration was inline in
+page.svelte and untested. Extract into a pure module that takes the
fetch function as a dependency, so the full happy path / 409 path / 500
path / refetch-fails path / UUID-guard path can be unit-tested with
mock Responses. The route file now reads as 12 lines: call the helper,
on conflict apply the merged snapshot to local state, re-throw.
BlockConflictResolvedError now carries the merged block on its
`merged` property so callers don't have to redo the refetch.
6 new unit tests cover every branch.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tester #5506 §5: the existing test only asserted the final 'saved'
state, which would also pass if the hook skipped the saving state
altogether. Hold the second mocked saveFn promise so we can assert the
intermediate transition.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tester #5506 §4: there was a test for fetch returning ok:false but no
test for the broad catch covering thrown rejections (DNS failure,
TypeError: Failed to fetch). Pin that path so a future refactor can't
accidentally bubble the error and crash the editor.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tester #5506 §1: 14 tests × 250ms real-timer waits = 3.5s wall-clock,
also racing the 200ms internal debounce by only 50ms — a flake on a
busy CI runner. Switch to vi.useFakeTimers + advanceTimersByTimeAsync;
test execution now 236ms (was 3.08s), determinism guaranteed because
the debounce runs against the fake clock.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leonie #5507 §5 + ReqEng #5510 §3: when the typeahead returned zero
results, the user was told their search failed and given no path to
recovery. Mirror PersonTypeahead's behaviour: offer a "Neue Person
anlegen →" link that opens /persons/new?name={query} in a new tab so
the transcriber doesn't lose their in-progress block.
Adds person_mention_create_new in de/en/es.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leonie #5507 concern 7: on slow networks the popup sat empty for up to
1.5s while the user wondered if anything was happening. Add a loading
flag that flips on as soon as scheduleSearch is asked to query and
back off in the fetch's finally branch. Reuses the existing
comp_typeahead_loading message ("Suche…") so no new i18n keys.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leonie #5507 concern 3: hover and aria-selected both used bg-canvas, so
a tablet user sweeping the trackpad couldn't tell where the keyboard
cursor was. Use bg-brand-mint/20 + a 2px ring-inset for the highlighted
row — keeps hover affordance, adds a distinct keyboard-cursor token
that meets WCAG 1.4.11 Non-Text Contrast.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leonie #5507 concerns 4 + 6:
- The textarea had outline-none and no focus indicator — broken for
keyboard-only navigation now that the typeahead is fully keyboard-driven.
- A rows=1 textarea is ~24px tall (Merriweather + 1.625 line-height),
below the WCAG 2.2 AA Target Size (44×44) requirement for the focused
actionable element.
Add focus-visible ring/border in brand-mint and a min-h of 44px with
py-2.5 padding so the empty-state textarea hits the target.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leonie #5507 concern 1: tabbing away from the editor left the popup
hanging over the next field. Add a 150ms-deferred close on blur — the
delay lets onmousedown on a result fire before the popup unmounts (the
race that the existing onmousedown+e.preventDefault() pattern depends on).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sina #5505 concern 2: the typeahead silently relies on the Vite-proxy
cookie injection + same-origin policy for auth. Spell that out in the
fetch site so the next reader doesn't have to derive it from the proxy
config.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sina #5505 action item: escapeHtml escaped the four common entities but
not the apostrophe. Today every consumer uses double-quoted attributes,
but a future renderer change to single quotes would silently open a
stored-XSS hole. Cheaper to fix now, with a regression test.
Also pin the idempotence-by-composition property: a second call
re-escapes the & introduced by the first.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Felix #5: TranscriptionBlock had a `\$effect(() => { void localText; ... })`
hack to re-trigger autoresize on text change, plus a captureTextarea
callback that the parent only used to size a node it didn't own.
The editor owns the textarea — it should also size it. Move the
autoresize \$effect into PersonMentionEditor so the parent only
captures the node when it genuinely needs to read selection bounds
(quote selection still works).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Felix #3: the 409 path was throwing a human-prose Error which read like
an i18n string that escaped translation. Replace with a named class
carrying code='CONFLICT_RESOLVED' so callers can branch on intent and
future error reporters can map the structured code instead of grepping
strings.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Felix #2: both were exported anticipating a future use that never came —
the editor only emits text+mentions through handleTextChange. Dead public
surface invites stale code; ship the smaller API.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same fix as 79349644 — the bind:mentionedPersons setter parameter `m`
shadowed the imported Paraglide m helper used two lines later in
placeholder={m.transcription_block_placeholder()}. Functionally fine
because the inner scope ends before the outer reference, but a clarity
trap. Renamed to next.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Felix #1: inside selectPerson the .some((m) => ...) parameter shadowed the
imported Paraglide m helper. Functionally fine, but a footgun. Rename to
existing for clarity.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The b2 fixture in the second describe block had been missed when the
TranscriptionBlockData type added the mentionedPersons field.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When PersonService renames a person while a transcriber is editing a
block that mentions them, the block-save endpoint returns 409 (carrying
the new ErrorCode.PERSON_RENAME_CONFLICT from PR-A). saveBlock now:
1. Refetches the latest server snapshot of the block.
2. Calls mergeBlockOnConflict to combine: server's mentionedPersons
(post-rename displayNames win) + transcriber's unsaved text + any
local-only mentions added since the last save.
3. Updates the local block state with the merged result.
4. Re-throws so the autosave indicator surfaces the conflict and the
pending payload is preserved for retry (B12).
The merge logic is a pure function so it can be unit-tested in
isolation and reused for any future conflict-resolution scenarios.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Locks in the behaviour added with the saveFn signature widening: a
rejected save keeps the in-flight payload around so handleRetry resends
it without the caller having to re-pass anything.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- TranscriptionBlockData now carries mentionedPersons (matches backend
schema added in PR-A).
- useBlockAutoSave.saveFn signature widens to (blockId, text, mentions);
pendingMentions is tracked alongside pendingTexts and is preserved on
failure so a retry resends the in-flight payload (B12).
- TranscriptionBlock.svelte renders <PersonMentionEditor>, exposing the
textarea node back through a captureTextarea callback so the existing
quote-selection feature still works.
- saveBlock in routes/documents/[id]/+page.svelte forwards mentions on
PUT.
- flushOnUnload sends mentions in the keepalive payload too.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mirrors MentionEditor for users but searches /api/persons?q=, allows
multi-word queries (delegated to detectPersonMention), displays life
dates next to each result, and uses min-h-[44px] rows for WCAG 2.2 AA
touch targets. Selection writes both the @DisplayName text and a
{personId, displayName} sidecar entry.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment mentions stop at a space; person mentions must accept spaces
because historical display names are commonly multi-word.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
openapi-typescript regenerated against the dev backend now exposes:
- components.schemas.PersonMention with personId + displayName
- TranscriptionBlock and CreateTranscriptionBlockDTO/UpdateTranscriptionBlockDTO
carry the optional mentionedPersons array
- (No new path entries: hover-card and typeahead reuse existing endpoints
GET /api/persons, GET /api/persons/{id}, GET /api/persons/{id}/relationships.)
Sealed inside PR-A so the frontend PR-B can import the new types from main
without rebasing across an unrelated regen. Per Tobias' chain-tightening
note in the consolidation summary.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the structured error code returned when a rename rolls back because a
referenced transcription block was edited concurrently (OptimisticLockException
on transcription_blocks.version). Mirrors the contract in
frontend src/lib/errors.ts and adds the localised message keys
error_person_rename_conflict in de/en/es so the UI surfaces a retry hint
instead of a generic 500.
The actual translation of OptimisticLockException → DomainException
(PERSON_RENAME_CONFLICT) lands in the next commit alongside the integration
test that proves the rollback semantics.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the 86-line duplicated inline add-relationship form with
<AddRelationshipForm onSubmit={handleAddRelationship}>. The {#key node.id}
wrapper resets the form's open state when the selected tree node changes.
Year inputs now have <label> elements (WCAG 1.3.1) via the shared component.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When onSubmit is provided the form has no server action and calls the
callback with typed RelFormData instead. Uses a shared {#snippet} for
the form body so the two submission paths share one template.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CSS box-shadow rings (focus-visible:ring-*) are invisible inside SVG.
Replace with a conditional <rect> drawn at -3px offset that renders in
all browsers. Name font-size bumped from 14 to 16px for the 60+
transcriber audience (WCAG readability, Leonie medium concerns).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The <span> in the derived-relationships list is replaced with <a href>
so keyboard and pointer users can navigate directly from the edit card,
consistent with PersonRelationshipsCard.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
h-8 w-8 (32px) replaced with h-11 w-11 (44px) to meet the minimum
touch target for the 60+ transcriber audience. Test added to prevent
regression.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Hardcoded 'Stammbaum & Beziehungen' heading replaced with
m.stammbaum_relationships_heading(); new key added to all
three message files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
