refactor(frontend): restructure lib/ from flat-by-type to domain-based #408
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
Part of Epic #406 — Big-bang restructure. This is REFACTOR-2: move every component, util, hook, and store in
frontend/src/lib/from flat-by-type folders into per-domain folders mirroring the backend (per D-FE decisions ratified in #387 first comment).Single big-bang PR. All tests green throughout.
Target lib/ structure (mirrors backend domain names per D2 stack-symmetry)
Out of scope
Generated code stays where it is:
frontend/src/lib/paraglide/— generated by Paraglide; do NOT moveRoutes are NOT moved.
frontend/src/routes/keeps its current shape (already domain-organized). Onlylib/moves.Route-local components (e.g.,
routes/briefwechsel/ConversationFilterBar.svelte) stay route-local — they're tightly coupled to their route's data contract.Process
git mv+ global search-and-replace on$lib/components/Xpathsnpm run check && npm run lint && npm run test— must be greennpx playwright testafter the full move — e2e is the integration safety netlib/components/once everything has movedfrontend/CLAUDE.md"UI Component Library" section to reflect the new layoutAnti-patterns
npm run check— TypeScript will catch missed imports.paraglide/content.Acceptance criteria
lib/components/,lib/components/document/,lib/components/chronik/,lib/components/user/, etc.) no longer existconversation/,activity/) exist with README.md explaining the patternlib/shared/contains only items meeting the admission criterianpm run check && npm run lint && npm run testall greennpx playwright testall green (excluding any documented quarantined tests)frontend/CLAUDE.mdreflects the new structureDependency
Same as REFACTOR-1: blocked by AUDIT-6 verdict (#393) and Epic 3 (#402).
Can run in parallel with REFACTOR-1 — the two stacks don't share files. Both PRs can be in flight simultaneously.
Definition of Done
PR merged. Closing comment confirms domain folder list and
lib/shared/membership for DOC-2 and DOC-6 to consume.🏗️ Markus Keller — Application Architect
Observations
lib/by domain) is the right call. This completes the structural alignment that was started on the backend side and makes module ownership visible across the stack.conversation/,activity/) with README.md files is a good explicit contract. Without this documentation, future developers will either collapse these into Tier 1 domains (wrong) or create new top-level domains (inconsistent). The READMEs are load-bearing here.shared/as an admission-controlled zone is architecturally sound. The issue lists admission criteria implicitly (cross-cutting, not domain-owned) but never makes them explicit. Worth adding a README.md toshared/too — same reason as the Tier 2 domains.vite.config.tscoverage include path currently targetssrc/lib/utils/**andsrc/lib/server/**. After the move, utilities scatter acrosssrc/lib/document/,src/lib/person/,src/lib/shared/utils/, etc. The coverage config must be updated as part of this issue, not as follow-up — otherwise CI will silently stop measuring utility coverage. I'd update the include tosrc/lib/**and excludesrc/lib/paraglide/**andsrc/lib/generated/**.api.server.ts,errors.ts,types.ts,relativeTime.ts,search.ts,utils.ts,person-validation.ts,relationshipLabels.ts) are currently imported by ~139 files in the codebase. The target paths ($lib/shared/api.server,$lib/person/person-validation,$lib/document/search, etc.) represent a large but mechanical rename.Recommendations
lib/shared/README.mdwith an explicit admission criteria list (mirrors the decision ratified in #387). Without it,shared/becomes a dumping ground within 3 months.vite.config.tscoverageincludeas part of this PR — not separately. Proposed:['src/lib/**/*.ts', '!src/lib/paraglide/**', '!src/lib/generated/**'].npm run check && npm run lint && npm run testafter each) is correct. Enforce the per-domain green check by making it a hard step in the PR description — not just an anti-pattern note.__mocks__/navigatingStore.tscurrently lives insidelib/components/__mocks__/. After the move this directory disappears. Verify where this mock goes — probablylib/shared/primitives/__mocks__/or adjacent to the hook that uses it.👨💻 Felix Brandt — Senior Fullstack Developer
Observations
src/lib/importing from the existing paths, and 105 files importing from flat root-level items ($lib/api.server,$lib/errors,$lib/types, etc.). That's ~220 import path changes. Mechanical, but large — worth tracking vianpm run checkoutput rather than manual review.git mv+ global search-and-replace. The VS Code approach is safer because it updates imports automatically.git mv+ sed risks missing*.svelte.tsdouble-extension files or imports spread across test-host stubs (e.g.PersonMentionEditor.test-host.svelte). Recommend VS Code move for.sveltefiles;git mv+sed -ifor.ts-only modules.lib/ocr/directory already partially exists — currently it only containstranslateOcrProgress.tsand its spec. The issue's target addsOcrProgress.svelte,OcrTrigger.svelte,OcrTrainingCard.svelte,SegmentationTrainingCard.svelte,TrainingHistory.svelte,training.ts. All are inlib/components/right now. The existinglib/ocr/means the target folder exists but is incomplete — easy trap when doing a "create all domain folders first" pass.utils/saveBlockWithConflictRetry.ts,utils/blockConflictMerge.ts,utils/transcriptionMarkers.ts,hooks/useBlockAutoSave.svelte.ts,hooks/useBlockDragDrop.svelte.tscurrently live underlib/utils/orlib/hooks/— the issue moves them intolib/document/transcription/. This changes both the folder and the$lib/utils/import prefix to$lib/document/transcription/. That's a semantic improvement, butnpm run checkwill catch any miss.src/lib/utils/**invite.config.tswill silently break coverage reporting after the move. That file is not in scope per the issue text but it must be updated in the same PR.Recommendations
npm run checkper domain, (4) Delete empty old folders, (5) Updatevite.config.tscoverage glob, (6) Run full E2E.npx tsc --noEmitto establish a clean baseline. Any existing type errors become noise during the migration.__mocks__/navigatingStore.tsinsidelib/components/__mocks__/is referenced by test files in that directory. Find its consumers withgrep -r "navigatingStore" frontend/src --include="*.ts"before moving the parent folder.refactor: move person domain components to lib/person/). This makes the git history auditable andgit bisectsurvivable.🔒 Nora "NullX" Steiner — Application Security Engineer
Observations
This is a pure structural refactor with no new security surface — no new endpoints, no new auth logic, no changed data flows. From a security posture perspective, the move is neutral.
That said, three items are worth calling out:
$lib/api.server.tsmoves to$lib/shared/api.server.ts. This file creates the typed API client and is server-only. The.server.tssuffix is SvelteKit's mechanism for preventing server-only code from being bundled into the client. After the move, the suffix stays, so the protection is preserved. Worth verifying withnpm run checkthat SvelteKit does not flag any accidental client-side import of the moved file.$lib/server/locale.tsmoves to$lib/shared/server/locale.ts. Same server-only concern. The subdirectory pathshared/server/preserves SvelteKit's convention (anything under aserver/folder is server-only). This is correct.$lib/utils/sanitize.tscurrently lives inlib/utils/. The issue places it inlib/shared/utils/sanitize.ts. The file name implies HTML sanitization — ensure that its import path is updated in all consumers and that no route-local copy is accidentally created during the move (a duplicate sanitize util with different behavior would be a real risk).Recommendations
grep -r "sanitize" frontend/src --include="*.svelte" --include="*.ts"to confirm all consumers point to a single canonical location..server.tssuffix convention is the only thing protecting server-side secrets (API client, locale detection) from client bundle inclusion. Document this explicitly in thelib/shared/server/README.mdor inlib/shared/README.mdadmission criteria.🧪 Sara Holt — QA Engineer & Test Strategist
Observations
.spec.tsand.test.ts) that import from$lib/paths. Each one will need its import paths updated alongside the component move. The issue's per-domainnpm run checkstep will catch TypeScript import errors, butnpm run testis the gate that catches test runtime failures (e.g. a mock path that resolves differently after the move).lib/components/__mocks__/navigatingStore.tsis a Vitest mock. Afterlib/components/is deleted, this mock disappears. If any test file uses a relative or absolute path to this mock, those tests silently stop mocking the store and may pass vacuously or fail with confusing errors. Locate all consumers before moving.vite.config.tscurrently measures coverage forsrc/lib/utils/**andsrc/lib/server/**only. After the move, most of the utils scatter into domain subfolders (e.g.$lib/document/transcription/transcriptionMarkers.ts). The coverage measurement will drop to near zero — not because coverage dropped, but because the glob no longer matches the files. This is a silent CI quality regression. Fix the glob in the same PR.*.svelte.{test,spec}.{js,ts}for the browser project and*.{test,spec}.{js,ts}for the server project. These globs are path-independent — they'll continue to pick up test files regardless of where they live. The test runner config itself does not need updating.frontend/e2e/) do not import from$lib/— they import only from@playwright/testand project-local helpers. E2E is unaffected by the move.Recommendations
vite.config.tsupdated to cover the new domain-based paths." This is currently missing from the acceptance criteria list and will cause a silent CI regression if skipped.npm run testafter each domain move, not justnpm run check && npm run lint. The issue currently only listscheck && lint && testas a combined step — maketestnon-skippable in the process.npm run test -- --reporter=verbose 2>&1 | grep -c "✓". Compare after each domain move. If the count drops, a test file got orphaned or a mock broke.lib/components/test-results/directory exists in the codebase but should not be moved — it's a Playwright artifact output folder and is not source code. Confirm it's in.gitignore.🎨 Leonie Voss — UX Designer & Accessibility Strategist
Observations
This is a pure file-system restructure with no UI or accessibility impact at runtime. From a design perspective, this is invisible to users — components behave identically before and after.
The one thing I'll flag from a long-term maintainability standpoint: the issue anti-pattern list says "Do NOT modify component behavior or props." This is the right constraint. However, the move is an opportunity to document accessibility contracts in each domain folder. Right now there's no canonical place to track things like "all person components must support keyboard navigation for the persona of our 60+ users." A
lib/person/ACCESSIBILITY.mdor inline@a11yJSDoc block per component would make the constraint explicit rather than assumed.Specifically:
PersonChip.svelte,PersonHoverCard.svelte,PersonTypeahead.svelte, andPersonMultiSelect.svelteall handle keyboard interaction. These are among the most interaction-rich components in the codebase. After the move they live together inlib/person/— a good moment to confirm each hasaria-label, focus management, and keyboard escape behavior documented.Recommendations
lib/person/README.md(the issue already requires one forlib/conversation/andlib/activity/) that notes the keyboard/a11y contract for the typeahead and multiselect components. This pays dividends during future edits.npx playwright testpass. Even though paths are unchanged at runtime, a missed import could render a component without its interactive JS, producing a visually identical but non-functional page. E2E catches this; unit tests may not.⚙️ Tobias Wendt — DevOps & Platform Engineer
Observations
actions/upload-artifact@v3in CI — theci.ymlworkflow uses the deprecated v3 artifact upload action. This is not caused by this issue but the PR is a good moment to bump to v4. Deprecated actions accumulate security vulnerabilities and will eventually be removed.vite.config.tssetsinclude: ['src/lib/utils/**', 'src/lib/server/**']for coverage measurement. After the move,src/lib/utils/**no longer exists at that path (utilities scatter to domain folders). The CI unit-test job will still pass green, but the coverage report will be near-empty. This is a silent quality regression in CI. The fix belongs in the same PR as the move.npm run lint— linting will catch any remaining$lib/components/imports that weren't updated. This is a good gate. The issue process already relies onnpm run lintpassing.frontend/src/lib/. Docker Compose, the backend, and the CI runner configuration are unaffected.vite.config.tsoutputs Paraglide to./src/lib/paraglide. This does not change and the issue correctly calls it out-of-scope. No risk there.Recommendations
actions/upload-artifact@v3→@v4in.gitea/workflows/ci.ymlin this PR. It's a one-line change and the PR is touching the codebase anyway.vite.config.tscoverage glob before merging. Proposed replacement:include: ['src/lib/**/*.ts']withexclude: ['src/lib/paraglide/**', 'src/lib/generated/**', 'src/**/*.{test,spec}.ts']. This captures all domain utility files without needing to enumerate each domain folder.$libalias resolves at build time — SvelteKit/Vite handles the remapping transparently.📋 Elicit (Requirements) — Issue Quality Review
Observations
The issue is dense and well-specified for a structural refactor. The target file tree is exhaustive and the anti-patterns list is exactly the right kind of constraint for a mechanical move. The process steps are clear and ordered.
A few gaps from a requirements perspective:
Acceptance criteria item for
vite.config.tsis missing. The coverage include pathssrc/lib/utils/**andsrc/lib/server/**will silently stop working after the move. There is no AC line requiring this file to be updated. Multiple reviewers have flagged this — it belongs in the AC checklist.No explicit AC for the
__mocks__/navigatingStore.tsrelocation. The mock lives atlib/components/__mocks__/navigatingStore.ts. The AC says "old flat folders no longer exist" but doesn't explicitly require that the mock file and its consumers are verified. This is a concrete risk with no coverage.The "Closing comment" in Definition of Done references "DOC-2 and DOC-6." These are mentioned as downstream consumers but are not linked or described. A reader can't evaluate the dependency risk without knowing what these are. If they're issue numbers, link them.
"Can run in parallel with REFACTOR-1" — the parallel claim is correct (different file sets) but there is an implicit risk: if REFACTOR-1 and REFACTOR-2 are both in flight and both modify
CLAUDE.md, there's a merge conflict risk on the documentation update. Not a blocker, but worth noting in the issue so the author can sequence the doc update.Recommendations
[ ] vite.config.ts coverage include paths updated to cover the new domain-based structure[ ] lib/shared/README.md created with explicit admission criteria for what belongs in shared/[ ] __mocks__/navigatingStore.ts verified to be referenced by its new path in all consumersImplementation complete ✅
All
lib/source files have been moved to the domain-based layout described in this issue. Branch:feat/issue-408-frontend-lib-domainsCommits (15 total, all on the feature branch)
a843d276422e86fbe7f8aa581e656d2d8ff5d6f8051d2f24920742bad5d36e667dd05af87cb922e9d6db7a07efcc347c56761276410b91e2Final structure
Test gate
npm run check: 270 errors (was 272 at baseline — 2 fewer)npm run test: 1 failed file / 2 failed tests — pre-existingTranscriptionEditViewfailures confirmed onmainbranch; no regressions introducednpm run lint: cleanAcceptance criteria
$lib/components/,$lib/hooks/,$lib/server/,$lib/services/,$lib/stores/,$lib/utils/,$lib/types/,$lib/actions/removedfrontend/CLAUDE.mdupdated with new structurevite.config.tscoverage paths updatedupload-artifact@v3→@v4