feat(tag-typeahead): tree-aware results — expand children & surface ancestor path #251

Merged

marcel merged 6 commits from feat/issue-250-tag-typeahead-tree-aware into main 2026-04-17 12:25:55 +02:00

Conversation 7 Commits 6 Files Changed 4 +276 -61

marcel commented 2026-04-17 11:59:26 +02:00

Owner

Copy Link

Closes #250

Summary

Implements tree-aware tag typeahead: root matches now expand their descendants in the dropdown, and child matches prepend their ancestor path so users always see hierarchy context. The backend enriches TagService.search() with a new enrichWithRelatives() helper that reuses existing CTE infrastructure. The frontend rewrites orderedSuggestions as a recursive DFS with a SuggestionEntry type carrying depth and isDirectMatch — direct matches render with font-medium, context nodes with text-ink-3 and a › prefix, indented by depth * 16 + 12 px inline style. A role="listbox" accessibility fix is included.

Closes #250 ## Summary Implements tree-aware tag typeahead: root matches now expand their descendants in the dropdown, and child matches prepend their ancestor path so users always see hierarchy context. The backend enriches `TagService.search()` with a new `enrichWithRelatives()` helper that reuses existing CTE infrastructure. The frontend rewrites `orderedSuggestions` as a recursive DFS with a `SuggestionEntry` type carrying depth and `isDirectMatch` — direct matches render with `font-medium`, context nodes with `text-ink-3` and a `›` prefix, indented by `depth * 16 + 12` px inline style. A `role="listbox"` accessibility fix is included.

marcel added 2 commits 2026-04-17 11:59:27 +02:00

feat(tag-search): expand children and surface ancestor path in search results ... d075bf390a

Modifies TagService.search() to enrich name-matches with tree relatives:
root matches expand descendants, child matches prepend ancestors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(tag-input): tree-aware DFS ordering, depth indentation, and direct-match styling ... 5120dd19a1

Rewrites orderedSuggestions to a recursive DFS with SuggestionEntry type,
adds role=listbox, depth indentation via inline style, font-medium for direct
matches, text-ink-3 for context nodes, and › prefix for root-level ancestors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-04-17 12:03:26 +02:00

Author

Owner

Copy Link

🏛️ Markus Keller — Senior Application Architect

Verdict: ✅ Approved

What I checked

Layer boundaries, abstraction level, query complexity, and frontend state design.

Findings

enrichWithRelatives() extraction is correct — the method does one job, has an accurate complexity comment (1 CTE + 1 batch per matched tag, 3–5 queries total for typical searches), and keeps search() down to 4 lines. For a family archive with O(hundreds) of tags, this query budget is well within acceptable bounds.

Service boundary respected — TagService.search() only touches TagRepository. No cross-domain calls introduced, no boundary leaks.

Frontend orderedSuggestions is the right place for tree-ordering logic — the DFS lives inside $derived.by() where it belongs: derived state, not scattered template conditionals. The SuggestionEntry type cleanly separates the UI concern (depth, isDirectMatch) from the domain concern (tag).

One observation (not a blocker): SvelteMap inside $derived.by() is technically unnecessary for reactivity — the entire block re-runs when suggestions or fetchedForQuery changes anyway, so the reactive collection semantics of SvelteMap are not exercised. It satisfies the svelte/prefer-svelte-reactivity ESLint rule, but it may confuse a future reader who wonders why a Map that never mutates after construction needs to be reactive. Worth a one-line comment at the declaration site to explain the ESLint constraint.

## 🏛️ Markus Keller — Senior Application Architect **Verdict: ✅ Approved** ### What I checked Layer boundaries, abstraction level, query complexity, and frontend state design. ### Findings **`enrichWithRelatives()` extraction is correct** — the method does one job, has an accurate complexity comment (1 CTE + 1 batch per matched tag, 3–5 queries total for typical searches), and keeps `search()` down to 4 lines. For a family archive with O(hundreds) of tags, this query budget is well within acceptable bounds. **Service boundary respected** — `TagService.search()` only touches `TagRepository`. No cross-domain calls introduced, no boundary leaks. **Frontend `orderedSuggestions` is the right place for tree-ordering logic** — the DFS lives inside `$derived.by()` where it belongs: derived state, not scattered template conditionals. The `SuggestionEntry` type cleanly separates the UI concern (`depth`, `isDirectMatch`) from the domain concern (`tag`). **One observation (not a blocker):** `SvelteMap` inside `$derived.by()` is technically unnecessary for reactivity — the entire block re-runs when `suggestions` or `fetchedForQuery` changes anyway, so the reactive collection semantics of `SvelteMap` are not exercised. It satisfies the `svelte/prefer-svelte-reactivity` ESLint rule, but it may confuse a future reader who wonders why a `Map` that never mutates after construction needs to be reactive. Worth a one-line comment at the declaration site to explain the ESLint constraint.

marcel commented 2026-04-17 12:03:31 +02:00

Author

Owner

Copy Link

🔧 Tobias Wendt — DevOps & Platform Engineer

Verdict: ✅ Approved

Nothing to flag from my side.

This PR is entirely application-layer — no docker-compose.yml changes, no CI workflow changes, no new infrastructure dependencies, no new environment variables. The recursive CTE queries (findDescendantIds, findAncestorIds) already existed in TagRepository.java and run on the existing PostgreSQL 16 stack. No new indexes added (the findByNameContainingIgnoreCase query uses the existing table scan, which is fine at family-archive scale).

Checked: no secrets committed, no bind mounts, no new services. CI pipeline is unaffected. Good to go.

## 🔧 Tobias Wendt — DevOps & Platform Engineer **Verdict: ✅ Approved** Nothing to flag from my side. This PR is entirely application-layer — no `docker-compose.yml` changes, no CI workflow changes, no new infrastructure dependencies, no new environment variables. The recursive CTE queries (`findDescendantIds`, `findAncestorIds`) already existed in `TagRepository.java` and run on the existing PostgreSQL 16 stack. No new indexes added (the `findByNameContainingIgnoreCase` query uses the existing table scan, which is fine at family-archive scale). Checked: no secrets committed, no bind mounts, no new services. CI pipeline is unaffected. Good to go.

marcel commented 2026-04-17 12:03:40 +02:00

Author

Owner

Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ✅ Approved

What I checked

TDD discipline, naming, single-responsibility, Svelte 5 correctness, and template cleanliness.

Strengths

• enrichWithRelatives() is well-named and does exactly one thing. The early-return guard in search() (if (matched.isEmpty()) return matched) prevents unnecessary processing ✅
• orderedSuggestions as a $derived.by() with SuggestionEntry type is clean — the separation between tag (domain), depth (UI), and isDirectMatch (UI) is correct ✅
• {#each orderedSuggestions as s, i (s.tag.id ?? s.tag.name)} — keyed correctly ✅
• addTag(orderedSuggestions[activeIndex].tag) — the .tag unwrap is necessary and correct given the new SuggestionEntry[] type; the regression test for this is good ✅
• All {#each} blocks keyed, no business logic in the template beyond the isDirectMatch conditional ✅

Suggestions (no blockers)

1. SvelteMap in $derived.by() — add a comment (TagInput.svelte:26–27):

// SvelteMap required by svelte/prefer-svelte-reactivity ESLint rule;
// reactivity is provided by the surrounding $derived.by() re-evaluation
const byId = new SvelteMap(...)

Without this, a future reader will wonder why a non-mutating derived Map is reactive.

2. › prefix condition is non-obvious (TagInput.svelte:195):

{#if !s.isDirectMatch && s.depth === 0}

This shows › only on root-level context ancestors — at depth > 0, the indentation already communicates hierarchy. The logic is correct, but a one-liner above it would prevent confusion during refactoring.

3. fetchedForQuery latent race window (TagInput.svelte:57–74):
Inside fetchSuggestions, suggestions is updated first, then fetchedForQuery. Between the two assignments there's a single microtask where orderedSuggestions recomputes against the stale query. In practice harmless (the derived fires twice, the second is correct), but worth documenting with a comment at the assignment site.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer **Verdict: ✅ Approved** ### What I checked TDD discipline, naming, single-responsibility, Svelte 5 correctness, and template cleanliness. ### Strengths - `enrichWithRelatives()` is well-named and does exactly one thing. The early-return guard in `search()` (`if (matched.isEmpty()) return matched`) prevents unnecessary processing ✅ - `orderedSuggestions` as a `$derived.by()` with `SuggestionEntry` type is clean — the separation between `tag` (domain), `depth` (UI), and `isDirectMatch` (UI) is correct ✅ - `{#each orderedSuggestions as s, i (s.tag.id ?? s.tag.name)}` — keyed correctly ✅ - `addTag(orderedSuggestions[activeIndex].tag)` — the `.tag` unwrap is necessary and correct given the new `SuggestionEntry[]` type; the regression test for this is good ✅ - All `{#each}` blocks keyed, no business logic in the template beyond the `isDirectMatch` conditional ✅ ### Suggestions (no blockers) **1. `SvelteMap` in `$derived.by()` — add a comment (TagInput.svelte:26–27):** ```svelte // SvelteMap required by svelte/prefer-svelte-reactivity ESLint rule; // reactivity is provided by the surrounding $derived.by() re-evaluation const byId = new SvelteMap(...) ``` Without this, a future reader will wonder why a non-mutating derived Map is reactive. **2. `›` prefix condition is non-obvious (TagInput.svelte:195):** ```svelte {#if !s.isDirectMatch && s.depth === 0} ``` This shows `›` only on root-level context ancestors — at depth > 0, the indentation already communicates hierarchy. The logic is correct, but a one-liner above it would prevent confusion during refactoring. **3. `fetchedForQuery` latent race window (TagInput.svelte:57–74):** Inside `fetchSuggestions`, `suggestions` is updated first, then `fetchedForQuery`. Between the two assignments there's a single microtask where `orderedSuggestions` recomputes against the stale query. In practice harmless (the derived fires twice, the second is correct), but worth documenting with a comment at the assignment site.

marcel commented 2026-04-17 12:03:47 +02:00

Author

Owner

Copy Link

🔐 Nora "NullX" Steiner — Application Security Engineer

Verdict: ✅ Approved

What I checked

OWASP Top 10, injection surfaces, IDOR, XSS vectors, authorization scope, and data exposure.

Findings — all clear

Injection (CWE-89): The frontend uses encodeURIComponent(query) before inserting into the query string — correct. On the backend, findByNameContainingIgnoreCase(query) is a Spring Data derived query: the framework binds the parameter, no SQL concatenation. No injection surface.

XSS (CWE-79): The › prefix is a hardcoded Unicode character in the template. Tag names, color token names, and depth values are all rendered through Svelte's default HTML-escaping template engine. No {@html} used. No XSS surface.

IDOR / Authorization (CWE-284): The tag search endpoint returns data from the global tag catalog — no per-user data, no documents, no PII. The existing permission model is unchanged. No new authorization surface introduced.

Sensitive data exposure: The Tag type exposed to the frontend includes {id, name, color, parentId} — no internal state, no server-side secrets.

Parameterized logging (CWE-117): The log.info(...) calls in TagService.mergeTags() already use SLF4J {} parameterization. No new logging added in this PR.

Nothing to flag. Clean PR from a security standpoint.

## 🔐 Nora "NullX" Steiner — Application Security Engineer **Verdict: ✅ Approved** ### What I checked OWASP Top 10, injection surfaces, IDOR, XSS vectors, authorization scope, and data exposure. ### Findings — all clear **Injection (CWE-89):** The frontend uses `encodeURIComponent(query)` before inserting into the query string — correct. On the backend, `findByNameContainingIgnoreCase(query)` is a Spring Data derived query: the framework binds the parameter, no SQL concatenation. No injection surface. **XSS (CWE-79):** The `›` prefix is a hardcoded Unicode character in the template. Tag names, color token names, and depth values are all rendered through Svelte's default HTML-escaping template engine. No `{@html}` used. No XSS surface. **IDOR / Authorization (CWE-284):** The tag search endpoint returns data from the global tag catalog — no per-user data, no documents, no PII. The existing permission model is unchanged. No new authorization surface introduced. **Sensitive data exposure:** The `Tag` type exposed to the frontend includes `{id, name, color, parentId}` — no internal state, no server-side secrets. **Parameterized logging (CWE-117):** The `log.info(...)` calls in `TagService.mergeTags()` already use SLF4J `{}` parameterization. No new logging added in this PR. Nothing to flag. Clean PR from a security standpoint.

marcel commented 2026-04-17 12:03:58 +02:00

Author

Owner

Copy Link

🧪 Sara Holt — Senior QA Engineer

Verdict: ⚠️ Approved with concerns

What I checked

Backend test coverage, frontend component test coverage, test naming, reliability, and missing edge cases.

Strengths

• 4 backend tests with descriptive snake_case names covering the key behaviors: root→descendants expansion, child→ancestors surfacing, deduplication when seed is in descendant result, and resolveEffectiveColors invocation ✅
• mockFetchWithTagObjects(tags: Tag[]) helper fills the gap from mockFetchWithTags (which only built name-only objects) — good extension of the test infrastructure ✅
• keyboard Enter on context node adds it as a tag is a smart regression guard: if .tag unwrap is ever removed from addTag(orderedSuggestions[activeIndex].tag), the input won't clear and the test fails ✅
• shows orphaned children at depth 0 when their parent is already selected covers the client-side filter + tree-order interaction cleanly ✅
• Switching from getByRole('option', { name: 'Briefe' }) to querySelectorAll + texts array for the ordering assertions is the right call — the substring matching behavior of getByRole would cause false positives here ✅

Concern — waitForDebounce naming and duration

const waitForDebounce = () => new Promise((r) => setTimeout(r, 350));

fetchSuggestions has no debounce — it fires on every input event. The 350ms wait is just letting the async mock resolve (which happens as a microtask, well within 1ms). The name waitForDebounce implies a debounce that doesn't exist, and 350ms adds ~4.5 seconds to the test suite (13 uses × 350ms). This is not a blocker, but I'd rename it to waitForFetch and drop it to 50ms:

const waitForFetch = () => new Promise((r) => setTimeout(r, 50));

This keeps the intent clear and speeds up the suite.

Missing edge case — allowCreation=false with context-only suggestion nodes

keyboard Enter on context node adds it as a tag runs with the default allowCreation=true. When allowCreation=false, pressing Enter on an item from the suggestion list should still add it (it's a real catalog tag, not a free-text creation). The current code handles this correctly (handleKeydown checks activeIndex >= 0 before the allowCreation guard), but there's no test proving it. A single additional variant would close this gap — not a blocker, but worth filing.

## 🧪 Sara Holt — Senior QA Engineer **Verdict: ⚠️ Approved with concerns** ### What I checked Backend test coverage, frontend component test coverage, test naming, reliability, and missing edge cases. ### Strengths - 4 backend tests with descriptive `snake_case` names covering the key behaviors: root→descendants expansion, child→ancestors surfacing, deduplication when seed is in descendant result, and `resolveEffectiveColors` invocation ✅ - `mockFetchWithTagObjects(tags: Tag[])` helper fills the gap from `mockFetchWithTags` (which only built name-only objects) — good extension of the test infrastructure ✅ - `keyboard Enter on context node adds it as a tag` is a smart regression guard: if `.tag` unwrap is ever removed from `addTag(orderedSuggestions[activeIndex].tag)`, the input won't clear and the test fails ✅ - `shows orphaned children at depth 0 when their parent is already selected` covers the client-side filter + tree-order interaction cleanly ✅ - Switching from `getByRole('option', { name: 'Briefe' })` to `querySelectorAll + texts array` for the ordering assertions is the right call — the substring matching behavior of `getByRole` would cause false positives here ✅ ### Concern — `waitForDebounce` naming and duration ```typescript const waitForDebounce = () => new Promise((r) => setTimeout(r, 350)); ``` `fetchSuggestions` has **no debounce** — it fires on every input event. The 350ms wait is just letting the async mock resolve (which happens as a microtask, well within 1ms). The name `waitForDebounce` implies a debounce that doesn't exist, and 350ms adds ~4.5 seconds to the test suite (13 uses × 350ms). This is not a blocker, but I'd rename it to `waitForFetch` and drop it to 50ms: ```typescript const waitForFetch = () => new Promise((r) => setTimeout(r, 50)); ``` This keeps the intent clear and speeds up the suite. ### Missing edge case — `allowCreation=false` with context-only suggestion nodes `keyboard Enter on context node adds it as a tag` runs with the default `allowCreation=true`. When `allowCreation=false`, pressing Enter on an item from the suggestion list should still add it (it's a real catalog tag, not a free-text creation). The current code handles this correctly (`handleKeydown` checks `activeIndex >= 0` before the `allowCreation` guard), but there's no test proving it. A single additional variant would close this gap — not a blocker, but worth filing.

marcel commented 2026-04-17 12:04:09 +02:00

Author

Owner

Copy Link

🎨 Leonie Voss — UI/UX Design Lead & Accessibility Strategist

Verdict: ⚠️ Approved with concerns

What I checked

ARIA semantics, touch targets, contrast ratios, visual hierarchy, brand token usage, and mobile usability.

Strengths

• role="listbox" on <ul>, role="option" on <li>, aria-selected={i === activeIndex} — correct ARIA combobox/listbox pattern, satisfies WCAG 1.3.1 (Info and Relationships) ✅
• aria-hidden="true" on the › decorative span — correct, screen readers won't read it ✅
• aria-label on the × remove buttons — already present from before, maintained ✅
• Inline style for depth indentation (depth * 16 + 12px) — necessary and correct; dynamic Tailwind class interpolation is unreliable for arbitrary runtime values ✅
• Color dot opacity-50 for context nodes vs opacity-100 for direct matches gives a subtle but effective visual hierarchy without adding noise ✅
• font-medium text-ink-2 for direct matches vs text-ink-3 for context nodes is a clean differentiation using semantic tokens ✅

Concern — Touch target height in suggestion items

The suggestion <li> items use py-2 pr-3 with text-sm content:

• py-2 = 8px top + 8px bottom = 16px padding
• text-sm line height ≈ 20px
• Total: ~36px

Our design system targets 44px minimum for touch targets (WCAG 2.2 SC 2.5.8, and critical for the 60+ audience). Increase to py-3 (12px × 2 + 20px = 44px):

class="cursor-pointer py-3 pr-3 text-sm ..."

This is a High priority fix — the tag input is used on the document edit form, which older family members access on tablets and phones.

Concern — text-ink-3 contrast on dropdown background

Context nodes render with text-ink-3 which maps to --c-ink-3: #6b7280 (Gray-500). On a white background this is 4.48:1 — marginal for WCAG AA (threshold: 4.5:1). On the dropdown's bg-surface background (check the exact token value in layout.css) this may drop below the threshold. Please verify with a contrast checker. If it fails, bump to text-ink-2 (--c-ink-2: #374151, Gray-700, 10.7:1 on white) for context nodes and use a lighter font-weight to maintain the visual distinction instead.

## 🎨 Leonie Voss — UI/UX Design Lead & Accessibility Strategist **Verdict: ⚠️ Approved with concerns** ### What I checked ARIA semantics, touch targets, contrast ratios, visual hierarchy, brand token usage, and mobile usability. ### Strengths - `role="listbox"` on `<ul>`, `role="option"` on `<li>`, `aria-selected={i === activeIndex}` — correct ARIA combobox/listbox pattern, satisfies WCAG 1.3.1 (Info and Relationships) ✅ - `aria-hidden="true"` on the `›` decorative span — correct, screen readers won't read it ✅ - `aria-label` on the × remove buttons — already present from before, maintained ✅ - Inline `style` for depth indentation (`depth * 16 + 12`px) — necessary and correct; dynamic Tailwind class interpolation is unreliable for arbitrary runtime values ✅ - Color dot `opacity-50` for context nodes vs `opacity-100` for direct matches gives a subtle but effective visual hierarchy without adding noise ✅ - `font-medium text-ink-2` for direct matches vs `text-ink-3` for context nodes is a clean differentiation using semantic tokens ✅ ### Concern — Touch target height in suggestion items The suggestion `<li>` items use `py-2 pr-3` with `text-sm` content: - `py-2` = 8px top + 8px bottom = 16px padding - `text-sm` line height ≈ 20px - Total: ~36px Our design system targets **44px minimum** for touch targets (WCAG 2.2 SC 2.5.8, and critical for the 60+ audience). Increase to `py-3` (12px × 2 + 20px = **44px**): ```svelte class="cursor-pointer py-3 pr-3 text-sm ..." ``` This is a **High** priority fix — the tag input is used on the document edit form, which older family members access on tablets and phones. ### Concern — `text-ink-3` contrast on dropdown background Context nodes render with `text-ink-3` which maps to `--c-ink-3: #6b7280` (Gray-500). On a white background this is 4.48:1 — marginal for WCAG AA (threshold: 4.5:1). On the dropdown's `bg-surface` background (check the exact token value in `layout.css`) this may drop below the threshold. Please verify with a contrast checker. If it fails, bump to `text-ink-2` (`--c-ink-2: #374151`, Gray-700, 10.7:1 on white) for context nodes and use a lighter `font-weight` to maintain the visual distinction instead.

marcel added 4 commits 2026-04-17 12:18:48 +02:00

fix(tag-input): increase suggestion item padding to py-3 for 44px touch target ... 1b617aa08b

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(tag-input): rename waitForDebounce to waitForFetch and reduce to 50ms ... 6b7829d5c8

fetchSuggestions has no debounce; the wait is purely for the async mock to
resolve. The old name implied semantics that don't exist and added ~4.5s to
the suite (13 uses × 350ms).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(tag-input): add regression guard for allowCreation=false + Enter on suggestion ... b01761d800

Confirms that Enter on a suggestion item adds the tag even when allowCreation is
false — the activeIndex guard in handleKeydown runs before the allowCreation check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(tag-input): add clarifying comments for non-obvious design decisions ... bf010a23c3

- SvelteMap satisfies svelte/prefer-svelte-reactivity; $derived.by() handles reactivity
- ‹›› prefix only on depth=0 context ancestors; indentation serves deeper nodes
- fetchedForQuery set after suggestions causes harmless double $derived evaluation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-04-17 12:19:03 +02:00

Author

Owner

Copy Link

Review concerns addressed

All concerns from the review round have been resolved. Here's what was done:

✅ @leonievoss — Touch target height

Changed py-2 → py-3 on suggestion <li> items, bringing total height from ~36px to ~44px (WCAG 2.2 SC 2.5.8 minimum).
→ commit 1b617aa

Contrast note: text-ink-3 (#6b7280) on bg-surface (#ffffff) = 4.8:1 — already documented in layout.css as "WCAG AA ✓". No change needed.

✅ @saraholt — waitForDebounce naming and duration

Renamed waitForDebounce → waitForFetch throughout the spec file; reduced timeout from 350ms → 50ms. fetchSuggestions has no debounce — the wait is purely for the async mock to resolve. Saves ~3.9s off the suite (13 uses × 300ms saved).
→ commit 6b7829d

✅ @saraholt — Missing edge case: allowCreation=false + Enter on suggestion

Added regression test confirming that Enter on a highlighted suggestion adds the tag even when allowCreation=false. The test passed immediately — confirming the existing activeIndex >= 0 guard runs before the allowCreation check.
→ commit b01761d

✅ @mkeller + @felixbrandt — SvelteMap in $derived.by() comment

Added inline comment at the SvelteMap declarations explaining the ESLint rule requirement.
→ commit bf010a2

✅ @felixbrandt — › prefix condition comment

Added comment above {#if !s.isDirectMatch && s.depth === 0} explaining that the arrow is root-only because depth > 0 nodes already have indentation as a hierarchy cue.
→ commit bf010a2

✅ @felixbrandt — fetchedForQuery latent race comment

Added comment at the fetchedForQuery assignment explaining the intentional update order and the harmless double $derived evaluation.
→ commit bf010a2

All 948 frontend tests pass. Branch pushed.

## Review concerns addressed All concerns from the review round have been resolved. Here's what was done: ### ✅ @leonievoss — Touch target height Changed `py-2` → `py-3` on suggestion `<li>` items, bringing total height from ~36px to ~44px (WCAG 2.2 SC 2.5.8 minimum). → commit `1b617aa` **Contrast note:** `text-ink-3` (#6b7280) on `bg-surface` (#ffffff) = 4.8:1 — already documented in `layout.css` as "WCAG AA ✓". No change needed. ### ✅ @saraholt — `waitForDebounce` naming and duration Renamed `waitForDebounce` → `waitForFetch` throughout the spec file; reduced timeout from 350ms → 50ms. `fetchSuggestions` has no debounce — the wait is purely for the async mock to resolve. Saves ~3.9s off the suite (13 uses × 300ms saved). → commit `6b7829d` ### ✅ @saraholt — Missing edge case: `allowCreation=false` + Enter on suggestion Added regression test confirming that `Enter` on a highlighted suggestion adds the tag even when `allowCreation=false`. The test passed immediately — confirming the existing `activeIndex >= 0` guard runs before the `allowCreation` check. → commit `b01761d` ### ✅ @mkeller + @felixbrandt — `SvelteMap` in `$derived.by()` comment Added inline comment at the `SvelteMap` declarations explaining the ESLint rule requirement. → commit `bf010a2` ### ✅ @felixbrandt — `›` prefix condition comment Added comment above `{#if !s.isDirectMatch && s.depth === 0}` explaining that the arrow is root-only because depth > 0 nodes already have indentation as a hierarchy cue. → commit `bf010a2` ### ✅ @felixbrandt — `fetchedForQuery` latent race comment Added comment at the `fetchedForQuery` assignment explaining the intentional update order and the harmless double `$derived` evaluation. → commit `bf010a2` All 948 frontend tests pass. Branch pushed.

marcel merged commit bf010a23c3 into main 2026-04-17 12:25:55 +02:00

marcel deleted branch feat/issue-250-tag-typeahead-tree-aware 2026-04-17 12:25:57 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

P0-critical

Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.

P1-high

Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.

P2-medium

Noticeable improvement; doesn't block anything; schedule opportunistically.

P3-later

Cosmetic or parking-lot work; fine to stay open indefinitely.

audit

Read-only audit / assessment work; produces a report rather than changing code

bug

Something isn't working

cleanup

Removal of dead code, vague comments, naming violations, and scratch artifacts

collaboration

We want to extend the family archive to have more collaboration tools

conversation

descoped

We will do that later

devops

documentation

README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs

epic

Marker for epic-level issues that group multiple child issues

feature

file-upload

legibility

Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction

notification

person

phase-1: security

Security hygiene — must be done first

phase-2: container-images

Production-ready multi-stage Docker images

phase-3: prod-compose

Production compose overlay + Caddy reverse proxy

phase-4: spring-prod-profile

Spring Boot production configuration profile

phase-5: backups

Database and object storage backup strategy

phase-6: deployment-docs

.env.example, DEPLOYMENT.md, runbook

phase-7: monitoring

Prometheus, Loki, Grafana, Alertmanager

refactor

Code restructuring without behaviour change

security

test

ui

UI/UX and accessibility issues

user

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Jens marcel

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#251