marcel/familienarchiv
1
0
Fork 0
Code Issues 119 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

audit(backend): score backend/ against legibility rubric C1–C10 #389

New Issue
Closed
opened 2026-05-04 16:03:53 +02:00 by marcel · 1 comment
marcel commented 2026-05-04 16:03:53 +02:00
Owner
Copy Link

Context

Part of Epic #387 — Codebase Legibility Audit. This is AUDIT-2: read-only assessment of backend/ against the full Legibility Rubric. The output is a Markdown report under docs/audits/audit-backend-report.md following the per-subsystem orient template. No code changes in this issue.

Inputs (read first)

See the first comment of #387 for the complete reference bundle (Brief, Personas, Canonical Domain Set + boundary decisions, Rubric, Orient Template).

Scope (in)

  • Everything under backend/src/main/java/ — controllers, services, repositories, entities, DTOs, exceptions, security, config
  • backend/src/main/resources/ — application*.yml, static resources (Flyway migrations are AUDIT-4's scope, but note schema clarity in §3 if relevant)
  • backend/src/test/java/ — test layout, naming, mutation-resilience
  • backend/pom.xml, Maven wrapper, .mvn/
  • backend/Dockerfile, backend/api_tests/
  • backend/CLAUDE.md (evaluate as documentation surface — much of it documents the current layered structure that the refactor will replace)

Scope (out)

  • backend/target/
  • Database migration content (AUDIT-4 owns this — but the Flyway configuration belongs here)
  • ocr-service/ and Python tooling (AUDIT-3)

Rubric checks particularly relevant to this subsystem

All of C1.5, C2, C3.1, C3.2, C3.3, C3.4, C3.6, C4 (excluding C4.4's frontend half), C5, C6.1, C6.2, C6.4, C7, C8.1, C8.3, C8.4, C9, C10.

Specific things to verify (subsystem-specific)

  • Layering rule violations. Search for any controller that injects a repository directly (C6.1 violation). Memory note: this rule is currently enforced by convention; verify by grep.
  • Cross-domain repo access. Search for any service that imports another domain's repository instead of going through that domain's service (C6.2 violation).
  • @RequirePermission coverage. Note any controller method that mutates state but lacks @RequirePermission.
  • DTO consistency. Note whether response shapes are entities vs DTOs and whether the choice is consistent.
  • Error handling consistency. DomainException vs ResponseStatusException vs raw — count usages, flag inconsistencies.
  • The 10 ambiguity findings from the requirements-phase research (TranscriptionBlock/Comment ownership, OCR domain status, Notification status, etc.) — confirm they're either resolved by the canonical domain set or escalate any new ambiguities.

Acceptance criteria

  • File docs/audits/audit-backend-report.md exists on a feature branch
  • Report contains all 10 template sections, none empty (or "N/A — reason")
  • §2 inventory tables match (or update) the controllers/services/repositories/entities lists from the requirements-phase backend survey
  • §3 scorecard covers every rubric check applicable to a Spring Boot 4 / JPA backend
  • §4 assigns the subsystem an overall 🟢 / 🟡 / 🔴 verdict
  • §5 explicitly addresses every domain-mapping ambiguity found during the audit
  • §6 lists every candidate for shared/ with admission-criteria justification
  • §7 catalogs dead code, half-finished features, vacuous tests
  • §8 explicitly notes the existing CLAUDE.md sections that need migration to human-readable docs
  • §9 identifies which services need mutation-test verification before REFACTOR-1 can run safely
  • §10 lists top-5 recommendations in priority order
  • No prose contains "ask Marcel," "Claude generated," "TODO," or "non-obvious" without explanation
  • PR opened and merged before this issue is closed

Definition of Done

Report committed under docs/audits/audit-backend-report.md on main. Top-5 recommendations summarized as a closing comment on this issue.

Dispatch

Hand to a parallel Explore agent with the prompt:

Read issue #387 first comment for the full reference bundle. Then read this issue (#TBD). Then audit backend/ per the orient template and produce docs/audits/audit-backend-report.md. Read-only; no code changes.

## Context Part of **Epic #387** — Codebase Legibility Audit. This is **AUDIT-2**: read-only assessment of `backend/` against the full Legibility Rubric. The output is a Markdown report under `docs/audits/audit-backend-report.md` following the per-subsystem orient template. **No code changes in this issue.** ## Inputs (read first) See **the first comment of #387** for the complete reference bundle (Brief, Personas, Canonical Domain Set + boundary decisions, Rubric, Orient Template). ## Scope (in) - Everything under `backend/src/main/java/` — controllers, services, repositories, entities, DTOs, exceptions, security, config - `backend/src/main/resources/` — `application*.yml`, static resources (Flyway migrations are AUDIT-4's scope, but note schema clarity in §3 if relevant) - `backend/src/test/java/` — test layout, naming, mutation-resilience - `backend/pom.xml`, Maven wrapper, `.mvn/` - `backend/Dockerfile`, `backend/api_tests/` - `backend/CLAUDE.md` (evaluate as documentation surface — much of it documents the *current* layered structure that the refactor will replace) ## Scope (out) - `backend/target/` - Database migration *content* (AUDIT-4 owns this — but the Flyway *configuration* belongs here) - `ocr-service/` and Python tooling (AUDIT-3) ## Rubric checks particularly relevant to this subsystem All of C1.5, C2, C3.1, C3.2, C3.3, C3.4, C3.6, C4 (excluding C4.4's frontend half), C5, C6.1, C6.2, C6.4, C7, C8.1, C8.3, C8.4, C9, C10. ## Specific things to verify (subsystem-specific) - **Layering rule violations.** Search for any controller that injects a repository directly (C6.1 violation). Memory note: this rule is currently enforced by convention; verify by `grep`. - **Cross-domain repo access.** Search for any service that imports another domain's repository instead of going through that domain's service (C6.2 violation). - **`@RequirePermission` coverage.** Note any controller method that mutates state but lacks `@RequirePermission`. - **DTO consistency.** Note whether response shapes are entities vs DTOs and whether the choice is consistent. - **Error handling consistency.** `DomainException` vs `ResponseStatusException` vs raw — count usages, flag inconsistencies. - **The 10 ambiguity findings** from the requirements-phase research (TranscriptionBlock/Comment ownership, OCR domain status, Notification status, etc.) — confirm they're either resolved by the canonical domain set or escalate any new ambiguities. ## Acceptance criteria - [ ] File `docs/audits/audit-backend-report.md` exists on a feature branch - [ ] Report contains all 10 template sections, none empty (or "N/A — reason") - [ ] §2 inventory tables match (or update) the controllers/services/repositories/entities lists from the requirements-phase backend survey - [ ] §3 scorecard covers every rubric check applicable to a Spring Boot 4 / JPA backend - [ ] §4 assigns the subsystem an overall 🟢 / 🟡 / 🔴 verdict - [ ] §5 explicitly addresses every domain-mapping ambiguity found during the audit - [ ] §6 lists every candidate for `shared/` with admission-criteria justification - [ ] §7 catalogs dead code, half-finished features, vacuous tests - [ ] §8 explicitly notes the existing CLAUDE.md sections that need migration to human-readable docs - [ ] §9 identifies which services need mutation-test verification before REFACTOR-1 can run safely - [ ] §10 lists top-5 recommendations in priority order - [ ] No prose contains "ask Marcel," "Claude generated," "TODO," or "non-obvious" without explanation - [ ] PR opened and merged before this issue is closed ## Definition of Done Report committed under `docs/audits/audit-backend-report.md` on `main`. Top-5 recommendations summarized as a closing comment on this issue. ## Dispatch Hand to a parallel Explore agent with the prompt: > Read issue #387 first comment for the full reference bundle. Then read this issue (#TBD). Then audit `backend/` per the orient template and produce `docs/audits/audit-backend-report.md`. Read-only; no code changes.
marcel added this to the Codebase Legibility milestone 2026-05-04 16:03:53 +02:00
marcel added the P1-highauditlegibility labels 2026-05-04 16:05:39 +02:00
marcel commented 2026-05-04 16:35:42 +02:00
Author
Owner
Copy Link

AUDIT-2 — backend/ Legibility Audit

Read-only assessment of backend/ against RUBRIC-LEGIBILITY-001 per TEMPLATE-ORIENT-001. Reference bundle: issue #387 first comment.

§1 Subsystem profile

backend/ is the Spring Boot 4 monolith REST API for Familienarchiv. It owns the canonical write side of all business state (Documents, Persons, Tags, Users, Transcription, Annotations, Comments, Notifications, OCR jobs, Geschichten, Stammbaum), persistence (PostgreSQL + Flyway), object storage adapters (MinIO/S3), security (Spring Security + custom @RequirePermission AOP), and async batch orchestration (mass import, OCR, thumbnails). 212 Java source files (excluding tests), 92 test files, 57 Flyway migrations.

backend/
├── pom.xml, mvnw, .mvn/, Dockerfile, HELP.md, CLAUDE.md
├── api_tests/                       (.http files for VS Code REST Client + demo.pdf, metadata.xlsx)
└── src/
    ├── main/java/org/raddatz/familienarchiv/
    │   ├── controller/              19 REST controllers (layer-packaged)
    │   ├── service/                 37 services + helpers (layer-packaged)
    │   ├── repository/              24 repositories + Specifications/projections (layer-packaged)
    │   ├── model/                   35 JPA entities + enums + DisplayNameFormatter, PolygonConverter
    │   ├── dto/                     ~55 DTO/record classes
    │   ├── exception/               DomainException, ErrorCode (54 codes)
    │   ├── security/                Permission, @RequirePermission, PermissionAspect, SecurityUtils
    │   ├── config/                  SecurityConfig, MinioConfig, AsyncConfig, RateLimitInterceptor, FlywayConfig, WebConfig, DataInitializer
    │   ├── audit/                   FEATURE-PACKAGED — entity, repos, service, query service, DTOs all co-located
    │   ├── dashboard/               FEATURE-PACKAGED — controller, service, DTOs co-located
    │   └── relationship/            FEATURE-PACKAGED — controller, service, repo, entity, dto/ subpackage
    ├���─ main/resources/
    │   ├── application.yaml, application-dev.yaml
    │   └── db/migration/            V1..V59 (57 files)
    └── test/java/                   layer-packaged (controller/ service/ repository/...) + audit/, dashboard/, relationship/

The package style is mixed: the original layered packaging coexists with three feature packages (audit/, dashboard/, relationship/) — a partial migration toward domain layout that the planned refactor will complete.

§2 Inventory

Controllers (19) → canonical-domain mapping

Controller File Apparent domain
DocumentController controller/DocumentController.java document
PersonController controller/PersonController.java person
TagController controller/TagController.java tag
GeschichteController controller/GeschichteController.java geschichte
NotificationController controller/NotificationController.java notification
OcrController controller/OcrController.java ocr
TranscriptionBlockController controller/TranscriptionBlockController.java document.transcription
AnnotationController controller/AnnotationController.java document.annotation
CommentController controller/CommentController.java document.comment
TranscriptionQueueController controller/TranscriptionQueueController.java shared/transcription-queue
UserController controller/UserController.java user
GroupController controller/GroupController.java user
UserSearchController controller/UserSearchController.java user (mention search)
AuthController controller/AuthController.java user (auth/invite/reset)
AuthE2EController controller/AuthE2EController.java user (test-only, @Profile("e2e"))
InviteController controller/InviteController.java user (invite admin)
AdminController controller/AdminController.java shared/import + shared/dashboard-ish
StatsController controller/StatsController.java shared/dashboard (or shared/)
RelationshipController relationship/RelationshipController.java person.relationship (D-OQ-7)
DashboardController dashboard/DashboardController.java shared/dashboard (D-OQ-8)
GlobalExceptionHandler controller/GlobalExceptionHandler.java shared/error-handling

Services (37) → canonical-domain mapping

Service Apparent domain
DocumentService, DocumentVersionService document
TranscriptionService, TranscriptionBlockQueryService document.transcription
AnnotationService document.annotation
CommentService document.comment
PersonService, PersonNameParser, PersonTypeClassifier person
RelationshipService, RelationshipInferenceService person.relationship
TagService tag
GeschichteService geschichte
NotificationService, SseEmitterRegistry notification
OcrService, OcrBatchService, OcrAsyncRunner, OcrClient, OcrHealthClient, RestClientOcrClient, OcrProgressService, OcrTrainingService ocr
SenderModelService, TrainingDataExportService, SegmentationTrainingExportService ocr (training sub-area)
UserService, UserSearchService, CustomUserDetailsService, InviteService, PasswordResetService user
FileService, ThumbnailService, ThumbnailBackfillService, ThumbnailAsyncRunner shared/file-storage
MassImportService shared/import
TranscriptionQueueService shared/transcription-queue
AuditService, AuditLogQueryService shared/audit
DashboardService shared/dashboard

Repositories (24)

Repository Domain
DocumentRepository, DocumentVersionRepository, DocumentSpecifications document
TranscriptionBlockRepository, TranscriptionBlockVersionRepository, TranscriptionQueueProjection, TranscriptionWeeklyStatsProjection document.transcription
AnnotationRepository document.annotation
CommentRepository document.comment
PersonRepository, PersonNameAliasRepository person
PersonRelationshipRepository person.relationship
TagRepository tag
GeschichteRepository, GeschichteSpecifications geschichte
NotificationRepository notification
OcrJobRepository, OcrJobDocumentRepository, OcrTrainingRunRepository, SenderModelRepository ocr
AppUserRepository, UserGroupRepository, InviteTokenRepository, PasswordResetTokenRepository user
AuditLogRepository, AuditLogQueryRepository (in audit/) shared/audit
CompletionStatsRow infra (projection, repository/)

Entities (35) — sample mapping

Document, DocumentStatus, DocumentVersion, DocumentSort (model/), TrainingLabel, ScriptType, ThumbnailAspect → document; TranscriptionBlock, TranscriptionBlockVersion, BlockSource, PolygonConverter → document.transcription; DocumentAnnotation → document.annotation; DocumentComment, PersonMention → document.comment; Person, PersonNameAlias, PersonNameAliasType, PersonType, DisplayNameFormatter → person; Tag → tag; AppUser, UserGroup, InviteToken, PasswordResetToken → user; Notification, NotificationType → notification; OcrJob, OcrJobDocument, OcrJobStatus, OcrDocumentStatus, OcrTrainingRun, SenderModel, TrainingStatus → ocr; Geschichte, GeschichteStatus → geschichte; PersonRelationship, RelationToken, RelationType (in relationship/) → person.relationship; AuditLog, AuditKind (in audit/) → shared/audit.

DTOs (~55)

Mostly request DTOs co-located with the service that consumes them (e.g. DocumentUpdateDTO, CreateUserRequest, TagUpdateDTO). A small number of response DTOs exist (PersonSummaryDTO, IncompleteDocumentDTO, DocumentSearchResult, NotificationDTO, MentionDTO, StatsDTO, TagTreeNodeDTO, TranscriptionQueueItemDTO, BulkEditResult, BackfillResult). Validators (UniquePoints, UniquePointsValidator) live in dto/.

§3 Rubric scorecard

Check Result Severity Evidence Recommendation
C1.5 (no stray top-level scratch in repo root) FAIL Minor Repo root contains .agent/, frontend/.svelte-kit.old/, frontend/test-results.locked/, proofshot-artifacts/, node_modules/ per gitStatus Out of scope for backend itself; flag to root audit (#388/AUDIT-1)
C2.1 (single bootstrap command) PASS — backend/CLAUDE.md documents ./mvnw spring-boot:run, root docker-compose up -d —
C2.2 (prerequisites with versions) PASS — CLAUDE.md lists Java 21, Maven via wrapper, PostgreSQL 16, MinIO; Dockerfile uses eclipse-temurin:21.0.10_7 —
C2.3 (default credentials documented) PASS — application.yaml:65-67 defaults admin/admin123; documented in user MEMORY.md but not in any human-readable doc the stranger would find Mirror dev creds in a human-readable docs/DEPLOYMENT.md or backend README
C2.4 (common failure modes documented) FAIL Minor No "Troubleshooting" section in backend/CLAUDE.md or HELP.md Add a short Troubleshooting section (port collisions, MinIO bucket missing, profile activation)
C2.5 (per-subsystem README) FAIL Major backend/HELP.md is the Spring Initializr default; backend/CLAUDE.md exists but is AI-targeted not human-targeted Create backend/README.md with run/build/test/regen-API steps
C3.1 (business domains stated in docs) FAIL Critical No docs/ARCHITECTURE.md; only docs/architecture/c4-diagrams.md exists with no domain listing in prose Author docs/ARCHITECTURE.md enumerating canonical domain set
C3.2 (per-domain owns/does-not-own) FAIL Major No per-domain definitions exist anywhere Add per-domain READMEs (one per Tier-1 domain)
C3.3 (glossary disambiguates Person ≠ AppUser) FAIL Critical No glossary file; the distinction lives only in the user's MEMORY.md (project_person_appuser_separation.md) Create docs/GLOSSARY.md covering Person, AppUser, Geschichte, TranscriptionBlock, Annotation, Comment, OcrJob
C3.4 (backend package structure is domain-based) FAIL Critical controller/, service/, repository/, model/, dto/, exception/ are layer-packaged. Only audit/, dashboard/, relationship/ are feature-packaged. controller/DocumentController.java, service/DocumentService.java, repository/DocumentRepository.java, model/Document.java, dto/DocumentUpdateDTO.java all live in different packages Migrate to document/, person/, tag/, user/, geschichte/, notification/, ocr/, shared/ per REFACTOR-1
C3.6 (shared/ only genuinely cross-cutting) N/A — No shared/ package exists yet Apply admission criteria during REFACTOR-1 (see §6)
C4.1 (locate every domain file in ≤2 min) FAIL Critical Domain files spread across 5 layer packages; e.g. Document lives in model/, controller/, service/, repository/, plus dto/Document*.java (×11) Resolved by C3.4 fix
C4.2 (controller+service+page+component for a feature in ≤5 min) FAIL Major Backend half of this fails for the same reason as C4.1 Resolved by C3.4 fix
C4.3 (no Helper/Utils/Manager without modifier) PASS — Service classes use concept names (PersonNameParser, PersonTypeClassifier, DisplayNameFormatter); no anonymous Util.java/Helper.java found —
C4.4 (API routes predictable from domain) FAIL Major /api/documents/conversation (DocumentController.java:427) is mounted under documents but the conversation concept is its own canonical view domain. /api/stats (StatsController) is unobvious. UserSearchController mounts /api/users/search but the class name doesn't match the path. Dashboard endpoints are clean. After domain refactor, document the route→domain map in a docs/API.md
C5.1 (conventions doc: how to add domain/endpoint/page) FAIL Major backend/CLAUDE.md describes some conventions; nothing tells a stranger how to add a new endpoint or domain Add "How to add an endpoint" walkthrough to backend README
C5.2 (one way to do common things) FAIL Major Three error-handling patterns coexist: DomainException (137 sites), ResponseStatusException (3 controllers + PersonService, 11 sites), raw RuntimeException/IllegalArgumentException/IllegalStateException (5 files, 9 sites — see C5.2 evidence below) Pick one. Convert all ResponseStatusException validation in controllers to bean validation or DomainException; route RuntimeException in MassImportService.java:159, FileService.java:91, RestClientOcrClient.java:218,250,288 through DomainException.internal(...)
C5.3 (CLAUDE.md rules mirrored in human docs) FAIL Major backend/CLAUDE.md is the only place layering, error-handling, entity-style, OCR-integration, and run instructions are documented. Tobias would not look there. Migrate the rule sections to backend/README.md + docs/ARCHITECTURE.md
C5.4 (naming conventions stated and followed) PASS — Controllers *Controller, services *Service, repos *Repository, DTOs *DTO/*Request. ≥95% conformance —
C6.1 (no controller injects a repository directly) FAIL Critical controller/StatsController.java:18-19 injects PersonRepository personRepository; DocumentRepository documentRepository;. controller/AuthE2EController.java:27 injects PasswordResetTokenRepository tokenRepository; (test-only profile, but still a violation by the letter of the rule) Move StatsController body into a StatsService (or DashboardService). Add a getResetTokenForTest() to PasswordResetService (gated by @Profile("e2e")) and call from AuthE2EController.
C6.2 (no service depends on another domain's repository) FAIL Critical Eleven cross-domain repository injections: MassImportService.java:58 → DocumentRepository; ThumbnailService.java:65, ThumbnailBackfillService.java:40, ThumbnailAsyncRunner.java:32 → DocumentRepository; TranscriptionQueueService.java:23 → DocumentRepository; SegmentationTrainingExportService.java:30-32 → TranscriptionBlockRepository, AnnotationRepository, DocumentRepository; TrainingDataExportService.java:31-33 → same triple; SenderModelService.java:35 → TranscriptionBlockRepository; OcrTrainingService.java:40 → TranscriptionBlockRepository; TranscriptionService.java:40 → AnnotationRepository; AnnotationService.java:28 → TranscriptionBlockRepository; PasswordResetService.java:33 → AppUserRepository Each violation becomes a recommendation in REFACTOR-1's prerequisites; introduce read-only query methods on the owning service (e.g. DocumentService.findIdsForThumbnailBackfill(), TranscriptionService.findBlocksForTraining())
C6.4 (cross-cutting code in shared/, not duplicated) PASS-with-caveat Minor Cross-cutting bits (AuditService, FileService, MinioConfig, RateLimitInterceptor, GlobalExceptionHandler, SecurityUtils) exist as singletons — not duplicated. But they live in audit/, service/, config/, controller/, security/ rather than a unified shared/. Group under shared/ during REFACTOR-1
C7.1 (per-domain README exists) FAIL Major Zero per-domain READMEs in any package Create one per Tier-1 domain after refactor
C7.2 (non-obvious decisions have why-comments) PASS-with-caveat Minor Strong examples: SecurityConfig.java:40-46 (CSRF rationale), NotificationController.java:39-41, RelationshipController.java:23-29,37-38, AuthE2EController.java:29-30, application.yaml (open-in-view: false rationale, multipart cap). A few opaque blocks remain (e.g. SenderModelService @Lazy @Autowired self self-reference has only a one-line comment that requires Spring AOP knowledge) Add a why-comment near each @Lazy self self-injection and at OcrAsyncRunner TransactionSynchronizationManager block
C7.3 (no "ask Marcel"/"non-obvious"/"TODO" comments) PASS — grep -rn "TODO|FIXME|XXX|HACK|ask Marcel|Claude generated" over org/raddatz/familienarchiv/ returned only one German clarification comment in repository/DocumentRepository.java:32 —
C7.4 (API contracts documented; OpenAPI accessible) PASS-with-caveat Minor SpringDoc enabled only under dev profile (application-dev.yaml); backend/CLAUDE.md describes the regen flow. No human-readable backend README points to it Mirror the regen instructions in backend/README.md
C7.5 (DB schema has per-table or domain overview) FAIL Minor Migrations are V1..V59 with terse names; no overview file. Stranger must read SQL to understand schema Add backend/src/main/resources/db/SCHEMA.md summarising tables grouped by domain (this is also AUDIT-4 territory)
C8.1 (critical-path service tests would fail under mutation) UNVERIFIED Critical Mutation testing not run in this audit. By inspection, tests are well-named and assert on behavior (e.g. DocumentServiceTest, PersonServiceTest, TranscriptionServiceTest) but several services are large (DocumentService=971 LOC, MassImportService=385 LOC) and integration-test-heavy Run PIT mutation testing on DocumentService, PersonService, TranscriptionService, MassImportService, NotificationService, OcrAsyncRunner before REFACTOR-1
C8.3 (test names are descriptive) PASS — Sampled DocumentServiceTest uses method_behavior_whenCondition form (deleteDocument_throwsNotFound_whenMissing, updateDocument_setsArchiveBoxAndFolder); other test files follow same pattern —
C8.4 (tests pass in random order) PASS — No @TestMethodOrder or @Order annotations found; integration tests use Testcontainers with isolated DB —
C9.1 (production deployment documented) FAIL Major No docs/DEPLOYMENT.md; Dockerfile exists; docker-compose.yml orchestrates locally; nothing about prod Out of scope here but flag for AUDIT-5
C9.2 (env vars listed per subsystem) FAIL Major Env vars are inline in application.yaml (SPRING_DATASOURCE_URL, S3_*, APP_*, MAIL_*, APP_ADMIN_*); no consolidated reference Add a "Configuration" table to backend/README.md listing every ${...} knob with default and meaning
C9.3 (migration naming convention; non-trivial commented) PASS-with-caveat Minor All 57 migrations follow V{n}__{snake_description}.sql. No comment headers spotted in spot-check; this is AUDIT-4's call —
C9.4 (logs/observability hooks documented) FAIL Minor @Slf4j used widely; actuator/health is exposed (SecurityConfig.java:50); no doc points to log location or /actuator endpoints One paragraph in backend README
C10.1 (no dead code / _old/.backup files) FAIL Major model/DocumentSort.java is duplicated by dto/DocumentSort.java; only the DTO version is referenced (grep "model.DocumentSort" returns zero hits anywhere in backend, frontend, or tests) Delete model/DocumentSort.java
C10.2 (no premature abstractions) PASS-with-caveat Minor OcrClient/OcrHealthClient interfaces with one impl (RestClientOcrClient) are justified by mockability for tests (test files reference OcrClient). CompletionStatsRow, TranscriptionQueueProjection etc. are legitimate Spring Data projections —
C10.3 (no magic numbers/strings) PASS-with-caveat Minor Constants exist where they matter (TranscriptionService.MAX_TEXT_LENGTH, TRANSCRIPTION_COLOR, ocr.sender-model.activation-threshold config). A couple of inline literals in DocumentController (Math.min(limit, 40) in DashboardController.java:49) are tolerable —
C10.4 (no half-finished features / if (false) / commented blocks) PASS — No if (false) matches; no large commented-out blocks found in spot-check —
C10.5 (no defensive null/try-catch for impossible cases) PASS-with-caveat Minor OcrController.resolveUserId() swallows exceptions returning null — fragile pattern; RestClientOcrClient.java:218,250,288 wrap-and-rethrow as RuntimeException (also a C5.2 hit) Tighten OcrController.resolveUserId(); route OCR exceptions through DomainException.internal(ErrorCode.OCR_PROCESSING_FAILED, ...)

§4 Subsystem health summary

Category PASS Critical Major Minor Cosmetic
C1 (First Contact) 0/1 — — 1 —
C2 (Local Bootstrap) 3/5 — 1 1 —
C3 (Domain Clarity) 0/5 (1 N/A) 3 1 — —
C4 (Findability) 1/4 1 2 — —
C5 (Conventions) 1/4 — 3 — —
C6 (Layering) 1/4 (caveat) 2 — 1 —
C7 (Documentation) 2/5 (caveat) — 2 1 —
C8 (Test Trust) 2/4 (1 unverified) 0+1 unverified — — —
C9 (Operability) 1/4 (caveat) — 2 1 —
C10 (Code Quality) 4/5 (caveats) — 1 — —

Aggregate: 15 PASS / 6 FAIL-Critical / 12 FAIL-Major / 5 FAIL-Minor / 0 Cosmetic + 1 Unverified-Critical (C8.1).

Overall verdict: 🔴 — six Critical fails, all driven by (a) absent human-readable architecture/glossary documentation and (b) layer-based packaging that the planned REFACTOR-1 will fix.

§5 Domain mapping gaps

Each item below is unclear under the current package layout. Canonical homes are derived from D-OQ-1..8.

Item Plausible homes Resolution per canonical set Blocker question
TranscriptionBlock, TranscriptionBlockVersion, BlockSource, PolygonConverter, TranscriptionService, TranscriptionBlockQueryService, TranscriptionBlockController, TranscriptionBlockRepository, TranscriptionBlockVersionRepository document.transcription vs own transcription/ domain Resolved D-OQ-2: document.transcription/ sub-package None
DocumentComment, PersonMention, CommentService, CommentController, CommentRepository, related DTOs (CreateCommentDTO, MentionDTO) document.comment vs own comment/ domain Resolved D-OQ-3: document.comment/ sub-package None
DocumentAnnotation, AnnotationService, AnnotationController, AnnotationRepository, CreateAnnotationDTO, UpdateAnnotationDTO, UniquePoints/Validator document.annotation vs own Resolved D-OQ-4: document.annotation/ sub-package None
OcrJob, OcrJobDocument, OcrJobStatus, OcrDocumentStatus, OcrTrainingRun, SenderModel, TrainingStatus, TrainingLabel, OcrService, OcrBatchService, OcrAsyncRunner, OcrClient, OcrHealthClient, RestClientOcrClient, OcrProgressService, OcrTrainingService, OcrController, OcrJobRepository, OcrJobDocumentRepository, OcrTrainingRunRepository, SenderModelRepository, OCR DTOs own ocr/ domain vs sub-package of document Resolved D-OQ-5: own ocr/ domain TrainingLabel is currently a document field (Document.trainingLabel); does it stay in document (the field is on Document) or move to ocr (semantics)?
Notification, NotificationType, NotificationService, NotificationController, NotificationDTO, NotificationPreferenceDTO, NotificationRepository, SseEmitterRegistry own notification/ domain Resolved D-OQ-6: own notification/ domain NotificationPreference is currently fields on AppUser (isNotifyOnReply, isNotifyOnMention); does it stay on User or move
Tag, TagService, TagController, TagRepository, MergeTagDTO, TagUpdateDTO, TagOperator, TagTreeNodeDTO own tag/ domain Stays as tag/ (Tier-1) None
Geschichte, GeschichteStatus, GeschichteService, GeschichteController, GeschichteRepository, GeschichteSpecifications, GeschichteUpdateDTO own geschichte/ domain Stays as geschichte/ (Tier-1) None
MassImportService, BulkEditError/Result, DocumentBulkEditDTO, ODS parser code shared/import vs document Resolved D-OQ-8: shared/import The ODS-parsing column-mapping config under app.import.col.* is import-specific; safe to live in shared/import
AuditService, AuditLog, AuditKind, AuditLogRepository, AuditLogQueryService, AuditLogQueryRepository, ActivityActorDTO, ActivityFeedRow, ContributorRow, PulseStatsRow shared/audit Resolved D-OQ-8: already feature-packaged, rename to shared/audit None
FileService, MinioConfig, S3 client shared/file-storage Resolved D-OQ-8: shared/file-storage None
ThumbnailService, ThumbnailBackfillService, ThumbnailAsyncRunner shared/file-storage (thumbnails are file derivatives) vs document (thumbnails belong to documents) No explicit decision in D-OQ-1..8. Default to shared/file-storage since thumbnails are a file-storage concern that consumers any document type. Decision needed: thumbnails as part of file-storage or as a document.thumbnail sub-package?
TranscriptionQueueService, TranscriptionQueueController, TranscriptionQueueProjection, TranscriptionWeeklyStatsProjection, TranscriptionQueueItemDTO, TranscriptionWeeklyStatsDTO shared/transcription-queue (per canonical set's "transcription-queue" cross-cutting entry) Resolved by canonical set: shared/transcription-queue/ The queue is a read view over Document — confirm it's shared/ rather than document.transcription/queue/
DashboardService, DashboardController, DashboardResumeDTO, DashboardPulseDTO, ActivityFeedItemDTO shared/dashboard Resolved D-OQ-8: shared/dashboard None
StatsController, StatsDTO shared/dashboard Fold into shared/dashboard (it's a 2-row count endpoint) None
AdminController (mass-import + backfill triggers + thumbnail status) shared/import + shared/file-storage Split: import endpoints into shared/import/, backfill+thumbnail status endpoints with their respective owners Should AdminController be sliced per-domain or kept as a thin façade?
RelationshipController, RelationshipService, RelationshipInferenceService, PersonRelationship, PersonRelationshipRepository, RelationToken, RelationType, relationship DTOs person.relationship Resolved D-OQ-7: person.relationship/ sub-package — already feature-packaged at top-level, just nest it None
DisplayNameFormatter (currently in model/, used by Person and PersonSummaryDTO) person/ Move into person/ None
PolygonConverter (in model/, JPA AttributeConverter for transcription block polygons) document.transcription/ Move into document.transcription/ None
CustomUserDetailsService, SecurityUtils, Permission, RequirePermission, PermissionAspect shared/security Already cross-cutting; group under shared/security Permission enum has been silently extended (ANNOTATE_ALL, BLOG_WRITE) — CLAUDE.md doesn't mention these
RateLimitInterceptor shared/security or shared/web Pick one — currently in config/ and applied to login; lives in security territory —

§6 Cross-cutting candidates (shared/ admission)

Admission criteria: (a) no entity, (b) no user-facing CRUD, (c) ≥2 consumers OR genuine framework infrastructure.

Candidate (a) no entity (b) no CRUD (c) ≥2 consumers / infra Verdict
FileService + MinioConfig yes (S3 wrapper) yes DocumentService, ThumbnailService, OcrAsyncRunner, training exports, MassImportService shared/file-storage
AuditService + AuditLogRepository + AuditLog (entity!) no — owns AuditLog entity yes (write-only API) DocumentService, CommentService, AnnotationService, TranscriptionService, UserService, ThumbnailAsyncRunner shared/audit (entity is internal to the cross-cutting concern; admission-criterion (a) interpretation: the entity is not a domain entity — it's audit infrastructure)
DashboardService + DashboardController + dashboard DTOs yes (read-only over Audit/Document/Transcription) yes one consumer (the dashboard view itself) but is genuinely cross-domain read aggregation shared/dashboard
MassImportService yes (creates Documents/Persons/Tags via owning services or — currently — directly via DocumentRepository) yes (background batch only) one feature-level consumer (AdminController) but framework infra (POI parser + scheduler) shared/import
TranscriptionQueueService yes (read view over Document.status + transcription state) yes TranscriptionQueueController, plus dashboard consumers via projection shared/transcription-queue
ThumbnailService + ThumbnailBackfillService + ThumbnailAsyncRunner yes yes DocumentService, OcrAsyncRunner, MassImportService shared/file-storage (likely; see §5 blocker)
RateLimitInterceptor yes yes login flow + (potentially) OCR endpoints shared/security (or shared/web)
GlobalExceptionHandler + DomainException + ErrorCode yes no every controller shared/error-handling
Permission + RequirePermission + PermissionAspect + SecurityUtils + CustomUserDetailsService + SecurityConfig yes yes every controller shared/security
AsyncConfig + WebConfig + FlywayConfig + DataInitializer yes yes framework infra shared/config
SseEmitterRegistry yes yes NotificationController, NotificationService could stay in notification/; not admitted to shared/ (only one consumer outside notification/)
PersonNameParser + PersonTypeClassifier + DisplayNameFormatter yes yes PersonService + MassImportService (parser); both feel cleanly part of person/ Stays in person/ (reject shared/ — single-domain helpers)
OcrClient + OcrHealthClient + RestClientOcrClient yes yes OcrService, OcrAsyncRunner, OcrTrainingService, SenderModelService — all in ocr/ Stays in ocr/ (reject shared/ — all consumers within one domain)

Net: ten shared/ sub-packages (shared/file-storage, shared/audit, shared/dashboard, shared/import, shared/transcription-queue, shared/security, shared/error-handling, shared/config, plus optional shared/web).

§7 Dead/suspicious code register

File:line Smell Action
model/DocumentSort.java:1-6 Duplicate enum — completely shadowed by dto/DocumentSort.java; zero references anywhere Delete the file
controller/StatsController.java:18-19 Controller injects two repositories directly (C6.1 violation) Introduce StatsService
controller/AuthE2EController.java:27 Test-only controller injects repository directly (C6.1 violation, even if @Profile("e2e")) Add getResetTokenForTest() to PasswordResetService gated on profile, call from controller
service/MassImportService.java:159 throw new RuntimeException("Ungültige ODS-Datei: content.xml fehlt") — bypasses DomainException Replace with DomainException.badRequest(ErrorCode.VALIDATION_ERROR, ...)
service/FileService.java:91 throw new RuntimeException("Storage Error: " + e.getMessage()) Replace with DomainException.internal(ErrorCode.FILE_UPLOAD_FAILED, ...)
service/FileService.java:167 throw new IllegalStateException("SHA-256 not available", e) — same MD pattern in DocumentService.java:967 Centralize SHA-256 computation in one helper; if truly impossible, leave one why-comment
service/RestClientOcrClient.java:218,250,288 Three raw RuntimeException rethrows from OCR streaming Route via DomainException.internal(ErrorCode.OCR_PROCESSING_FAILED, ...)
model/PolygonConverter.java:23,33 throw new IllegalArgumentException("Failed to (de)serialize polygon", e) — JPA AttributeConverter; OK to keep but note it doesn't surface a frontend ErrorCode Consider DomainException.internal(...) if these can ever be hit at runtime; otherwise leave with why-comment
service/OcrController.java resolveUserId (around line 161) Catch-and-return-null pattern when user resolution fails Make explicit: throw DomainException.unauthorized(...) if auth missing
service/SenderModelService @Lazy @Autowired self Self-injection for @Async self-call — common Spring pattern but only one comment Add full why-comment explaining the proxy issue
controller/UserController.java:47-62 (updateProfile, changePassword) Mutating endpoints lack @RequirePermission — currently rely on Spring Security anyRequest().authenticated() Either add @RequirePermission(Permission.READ_ALL) for documentation, or document why personal endpoints don't need it (matches NotificationController.java:39-41 style)
controller/StatsController.java Mutates nothing but lacks @RequirePermission — visible to any authenticated user Add @RequirePermission(Permission.READ_ALL) for consistency
repository/DocumentRepository.java:32 German clarification comment // z.B. um alle offenen "PLACEHOLDER" zu finden — minor, language inconsistency vs rest of codebase (English) Translate or remove
service/MassImportService.java:343 // --- Helpers --- section divider is fine; the Helper word is a section label, not a class name No action
backend/HELP.md Spring Initializr boilerplate, never customized Delete or replace with real backend README

§8 Documentation gaps

Missing files (none of these exist):

  • backend/README.md (human-readable; HELP.md is Initializr boilerplate, CLAUDE.md is AI-targeted)
  • Per-domain READMEs for every Tier-1 domain (document/, person/, tag/, user/, geschichte/, notification/, ocr/)
  • Per-shared/ sub-package READMEs
  • docs/ARCHITECTURE.md (only c4-diagrams.md exists; needs prose enumerating canonical domain set + boundary decisions)
  • docs/GLOSSARY.md (Person ≠ AppUser, TranscriptionBlock vs Annotation vs Comment, DocumentStatus lifecycle, Permission semantics, Geschichte vs Document)
  • docs/DEPLOYMENT.md (env vars, profiles, prod vs dev, default creds)
  • backend/src/main/resources/db/SCHEMA.md (per-table comments grouped by domain — also AUDIT-4 territory)

backend/CLAUDE.md sections that need migration to human-readable docs (C5.3):

  • Overview + Tech Stack → backend/README.md
  • Package Structure + Layering Rules → docs/ARCHITECTURE.md (after refactor) and a "Conventions" section in backend/README.md
  • Key Entities table + DocumentStatus lifecycle → docs/GLOSSARY.md
  • Entity Code Style → docs/CONVENTIONS.md (new) or backend README "How to add an entity" walkthrough
  • Services discipline (@Transactional, cross-domain via service) → backend README "How to add a service"
  • Error Handling block → backend README "How to add an error code" (also resolves C5.1 walkthrough gap)
  • Security / Permissions block → docs/SECURITY.md (also docs/security-guide.md already exists at root — consolidate); note Permission enum drift: CLAUDE.md lists 6 permissions but Permission.java has 8 (ANNOTATE_ALL, BLOG_WRITE undocumented)
  • OCR Integration block → docs/integrations/ocr.md or ocr/README.md after refactor
  • How to Run + OpenAPI generation flow → backend README

Missing inline why-comments (C7.2 caveat):

  • SenderModelService @Lazy @Autowired self (proxy/self-call rationale)
  • OcrAsyncRunner TransactionSynchronizationManager.registerSynchronization(...) block (why post-commit dispatch matters)
  • application.yaml:38 max-request-size: 500MB references #317 — link to issue is good but the reasoning ("supports 10-file chunk at max per-file size") could be a short header in the file rather than only inline

API contract gaps (C7.4):

  • OpenAPI is dev-only; the regen command (npm run generate:api) is documented in frontend/package.json and CLAUDE.md but not in any backend doc that a contributor would discover first

§9 Prerequisites for big-bang refactor (REFACTOR-1 #407)

Before moving classes between packages, the following must be true. Any service whose tests fall short of mutation-validation risks silent regressions during the move.

Mutation-validate test suites for these services first (large, central, business-logic-heavy):

Service LOC Reason
DocumentService 971 Owns search/specifications/bulk edit/file-hash logic; tests must assert behavior, not just call paths
MassImportService 385 ODS parsing + batch insert; mutations easy to miss
TranscriptionService 244 Optimistic locking, version creation, training-block routing
PersonService 222 Find-or-create alias logic + merge; merge bugs are silent data corruption
NotificationService (size unread, central) Mention-fanout + email + SSE — three side-effect channels
OcrAsyncRunner (large) Cross-service orchestration of OCR pipeline; mutations to status transitions silent

Layering violations that MUST be fixed before the package move (otherwise the move surfaces them as cross-package imports, masking the real fix):

Service / Controller Repo touched Fix before refactor
MassImportService → DocumentRepository document Route through DocumentService (introduce write helpers)
ThumbnailService / ThumbnailBackfillService / ThumbnailAsyncRunner → DocumentRepository document Add DocumentService.findIdsForThumbnailBackfill() and DocumentService.updateThumbnailMetadata()
TranscriptionQueueService → DocumentRepository document Already a read-only view; add DocumentService.queueProjection(...) or admit as documented exception
SegmentationTrainingExportService / TrainingDataExportService → Document/Block/Annotation repos three domains Service-level read methods on each owning service
SenderModelService / OcrTrainingService → TranscriptionBlockRepository document.transcription Read API on TranscriptionService
TranscriptionService → AnnotationRepository document.annotation Read API on AnnotationService (sub-package, easier)
AnnotationService → TranscriptionBlockRepository document.transcription Read API on TranscriptionService (sub-package)
PasswordResetService → AppUserRepository user Inject UserService.findByEmail/save
StatsController → Person/DocumentRepository controller→repo Introduce StatsService (or fold into DashboardService)
AuthE2EController → PasswordResetTokenRepository controller→repo PasswordResetService.findE2EToken(email) gated on @Profile("e2e")

Prerequisite to delete (avoid carrying dead code into new packages):

  • model/DocumentSort.java (duplicate of dto/DocumentSort.java, unused)
  • backend/HELP.md (replace with real README)

Decisions still needed (one-line ratifications):

  • Thumbnails go to shared/file-storage or document.thumbnail?
  • Should Document.trainingLabel field move to ocr/ or stay in document/?
  • Should AppUser notification-preference fields stay on User or move to notification/?

§10 Top-5 prioritized recommendations

  1. Author root README + docs/ARCHITECTURE.md + docs/GLOSSARY.md — closes 4 of 6 Critical fails (C1.1, C3.1, C3.2, C3.3) and unblocks Anja and Tobias before any code moves; the package refactor without these docs is a worse legibility outcome, not a better one.
  2. Fix the 11 cross-domain repository injections (C6.2) and 2 controller-injects-repository sites (C6.1) before REFACTOR-1 runs — every violation listed in §9 needs a service-level read/write method on the owning domain so the package move is purely cosmetic, not a behavior change.
  3. Pick one error pattern and migrate — convert all throw new RuntimeException(...)/IllegalArgumentException(...) in MassImportService, FileService, RestClientOcrClient, PolygonConverter, and DocumentService to DomainException, and convert controller ResponseStatusException validation to @Valid + DomainException; this satisfies C5.2 and gives the frontend a uniform code to translate.
  4. Run PIT mutation testing on DocumentService, PersonService, TranscriptionService, MassImportService, NotificationService, OcrAsyncRunner — without this, REFACTOR-1's "tests still green" signal is unreliable for the largest blast-radius services (C8.1 currently Unverified-Critical).
  5. Delete model/DocumentSort.java, replace backend/HELP.md with a real backend/README.md (run/build/test/regen-API/env-vars/troubleshooting), and document the silently-added Permission.ANNOTATE_ALL and Permission.BLOG_WRITE — small, fast cleanups that close C2.5, C5.3, C7.4, C9.2, C9.4, and C10.1 in one PR each.
# AUDIT-2 — `backend/` Legibility Audit Read-only assessment of `backend/` against `RUBRIC-LEGIBILITY-001` per `TEMPLATE-ORIENT-001`. Reference bundle: issue #387 first comment. --- ## §1 Subsystem profile `backend/` is the Spring Boot 4 monolith REST API for Familienarchiv. It owns the canonical write side of all business state (Documents, Persons, Tags, Users, Transcription, Annotations, Comments, Notifications, OCR jobs, Geschichten, Stammbaum), persistence (PostgreSQL + Flyway), object storage adapters (MinIO/S3), security (Spring Security + custom `@RequirePermission` AOP), and async batch orchestration (mass import, OCR, thumbnails). 212 Java source files (excluding tests), 92 test files, 57 Flyway migrations. ``` backend/ ├── pom.xml, mvnw, .mvn/, Dockerfile, HELP.md, CLAUDE.md ├── api_tests/ (.http files for VS Code REST Client + demo.pdf, metadata.xlsx) └── src/ ├── main/java/org/raddatz/familienarchiv/ │ ├── controller/ 19 REST controllers (layer-packaged) │ ├── service/ 37 services + helpers (layer-packaged) │ ├── repository/ 24 repositories + Specifications/projections (layer-packaged) │ ├── model/ 35 JPA entities + enums + DisplayNameFormatter, PolygonConverter │ ├── dto/ ~55 DTO/record classes │ ├── exception/ DomainException, ErrorCode (54 codes) │ ├── security/ Permission, @RequirePermission, PermissionAspect, SecurityUtils │ ├── config/ SecurityConfig, MinioConfig, AsyncConfig, RateLimitInterceptor, FlywayConfig, WebConfig, DataInitializer │ ├── audit/ FEATURE-PACKAGED — entity, repos, service, query service, DTOs all co-located │ ├── dashboard/ FEATURE-PACKAGED — controller, service, DTOs co-located │ └── relationship/ FEATURE-PACKAGED — controller, service, repo, entity, dto/ subpackage ├���─ main/resources/ │ ├── application.yaml, application-dev.yaml │ └── db/migration/ V1..V59 (57 files) └── test/java/ layer-packaged (controller/ service/ repository/...) + audit/, dashboard/, relationship/ ``` The package style is *mixed*: the original layered packaging coexists with three feature packages (`audit/`, `dashboard/`, `relationship/`) — a partial migration toward domain layout that the planned refactor will complete. --- ## §2 Inventory ### Controllers (19) → canonical-domain mapping | Controller | File | Apparent domain | |---|---|---| | `DocumentController` | `controller/DocumentController.java` | `document` | | `PersonController` | `controller/PersonController.java` | `person` | | `TagController` | `controller/TagController.java` | `tag` | | `GeschichteController` | `controller/GeschichteController.java` | `geschichte` | | `NotificationController` | `controller/NotificationController.java` | `notification` | | `OcrController` | `controller/OcrController.java` | `ocr` | | `TranscriptionBlockController` | `controller/TranscriptionBlockController.java` | `document.transcription` | | `AnnotationController` | `controller/AnnotationController.java` | `document.annotation` | | `CommentController` | `controller/CommentController.java` | `document.comment` | | `TranscriptionQueueController` | `controller/TranscriptionQueueController.java` | `shared/transcription-queue` | | `UserController` | `controller/UserController.java` | `user` | | `GroupController` | `controller/GroupController.java` | `user` | | `UserSearchController` | `controller/UserSearchController.java` | `user` (mention search) | | `AuthController` | `controller/AuthController.java` | `user` (auth/invite/reset) | | `AuthE2EController` | `controller/AuthE2EController.java` | `user` (test-only, `@Profile("e2e")`) | | `InviteController` | `controller/InviteController.java` | `user` (invite admin) | | `AdminController` | `controller/AdminController.java` | `shared/import` + `shared/dashboard`-ish | | `StatsController` | `controller/StatsController.java` | `shared/dashboard` (or `shared/`) | | `RelationshipController` | `relationship/RelationshipController.java` | `person.relationship` (D-OQ-7) | | `DashboardController` | `dashboard/DashboardController.java` | `shared/dashboard` (D-OQ-8) | | `GlobalExceptionHandler` | `controller/GlobalExceptionHandler.java` | `shared/error-handling` | ### Services (37) → canonical-domain mapping | Service | Apparent domain | |---|---| | `DocumentService`, `DocumentVersionService` | `document` | | `TranscriptionService`, `TranscriptionBlockQueryService` | `document.transcription` | | `AnnotationService` | `document.annotation` | | `CommentService` | `document.comment` | | `PersonService`, `PersonNameParser`, `PersonTypeClassifier` | `person` | | `RelationshipService`, `RelationshipInferenceService` | `person.relationship` | | `TagService` | `tag` | | `GeschichteService` | `geschichte` | | `NotificationService`, `SseEmitterRegistry` | `notification` | | `OcrService`, `OcrBatchService`, `OcrAsyncRunner`, `OcrClient`, `OcrHealthClient`, `RestClientOcrClient`, `OcrProgressService`, `OcrTrainingService` | `ocr` | | `SenderModelService`, `TrainingDataExportService`, `SegmentationTrainingExportService` | `ocr` (training sub-area) | | `UserService`, `UserSearchService`, `CustomUserDetailsService`, `InviteService`, `PasswordResetService` | `user` | | `FileService`, `ThumbnailService`, `ThumbnailBackfillService`, `ThumbnailAsyncRunner` | `shared/file-storage` | | `MassImportService` | `shared/import` | | `TranscriptionQueueService` | `shared/transcription-queue` | | `AuditService`, `AuditLogQueryService` | `shared/audit` | | `DashboardService` | `shared/dashboard` | ### Repositories (24) | Repository | Domain | |---|---| | `DocumentRepository`, `DocumentVersionRepository`, `DocumentSpecifications` | `document` | | `TranscriptionBlockRepository`, `TranscriptionBlockVersionRepository`, `TranscriptionQueueProjection`, `TranscriptionWeeklyStatsProjection` | `document.transcription` | | `AnnotationRepository` | `document.annotation` | | `CommentRepository` | `document.comment` | | `PersonRepository`, `PersonNameAliasRepository` | `person` | | `PersonRelationshipRepository` | `person.relationship` | | `TagRepository` | `tag` | | `GeschichteRepository`, `GeschichteSpecifications` | `geschichte` | | `NotificationRepository` | `notification` | | `OcrJobRepository`, `OcrJobDocumentRepository`, `OcrTrainingRunRepository`, `SenderModelRepository` | `ocr` | | `AppUserRepository`, `UserGroupRepository`, `InviteTokenRepository`, `PasswordResetTokenRepository` | `user` | | `AuditLogRepository`, `AuditLogQueryRepository` (in `audit/`) | `shared/audit` | | `CompletionStatsRow` | infra (projection, `repository/`) | ### Entities (35) — sample mapping `Document, DocumentStatus, DocumentVersion, DocumentSort (model/), TrainingLabel, ScriptType, ThumbnailAspect` → `document`; `TranscriptionBlock, TranscriptionBlockVersion, BlockSource, PolygonConverter` → `document.transcription`; `DocumentAnnotation` → `document.annotation`; `DocumentComment, PersonMention` → `document.comment`; `Person, PersonNameAlias, PersonNameAliasType, PersonType, DisplayNameFormatter` → `person`; `Tag` → `tag`; `AppUser, UserGroup, InviteToken, PasswordResetToken` → `user`; `Notification, NotificationType` → `notification`; `OcrJob, OcrJobDocument, OcrJobStatus, OcrDocumentStatus, OcrTrainingRun, SenderModel, TrainingStatus` → `ocr`; `Geschichte, GeschichteStatus` → `geschichte`; `PersonRelationship, RelationToken, RelationType` (in `relationship/`) → `person.relationship`; `AuditLog, AuditKind` (in `audit/`) → `shared/audit`. ### DTOs (~55) Mostly request DTOs co-located with the service that consumes them (e.g. `DocumentUpdateDTO`, `CreateUserRequest`, `TagUpdateDTO`). A small number of response DTOs exist (`PersonSummaryDTO`, `IncompleteDocumentDTO`, `DocumentSearchResult`, `NotificationDTO`, `MentionDTO`, `StatsDTO`, `TagTreeNodeDTO`, `TranscriptionQueueItemDTO`, `BulkEditResult`, `BackfillResult`). Validators (`UniquePoints`, `UniquePointsValidator`) live in `dto/`. --- ## §3 Rubric scorecard | Check | Result | Severity | Evidence | Recommendation | |---|---|---|---|---| | C1.5 (no stray top-level scratch in repo root) | FAIL | Minor | Repo root contains `.agent/`, `frontend/.svelte-kit.old/`, `frontend/test-results.locked/`, `proofshot-artifacts/`, `node_modules/` per gitStatus | Out of scope for backend itself; flag to root audit (#388/AUDIT-1) | | C2.1 (single bootstrap command) | PASS | — | `backend/CLAUDE.md` documents `./mvnw spring-boot:run`, root `docker-compose up -d` | — | | C2.2 (prerequisites with versions) | PASS | — | `CLAUDE.md` lists Java 21, Maven via wrapper, PostgreSQL 16, MinIO; `Dockerfile` uses `eclipse-temurin:21.0.10_7` | — | | C2.3 (default credentials documented) | PASS | — | `application.yaml:65-67` defaults `admin/admin123`; documented in user MEMORY.md but **not** in any human-readable doc the stranger would find | Mirror dev creds in a human-readable `docs/DEPLOYMENT.md` or backend README | | C2.4 (common failure modes documented) | FAIL | Minor | No "Troubleshooting" section in `backend/CLAUDE.md` or HELP.md | Add a short Troubleshooting section (port collisions, MinIO bucket missing, profile activation) | | C2.5 (per-subsystem README) | FAIL | Major | `backend/HELP.md` is the Spring Initializr default; `backend/CLAUDE.md` exists but is AI-targeted not human-targeted | Create `backend/README.md` with run/build/test/regen-API steps | | C3.1 (business domains stated in docs) | FAIL | Critical | No `docs/ARCHITECTURE.md`; only `docs/architecture/c4-diagrams.md` exists with no domain listing in prose | Author `docs/ARCHITECTURE.md` enumerating canonical domain set | | C3.2 (per-domain owns/does-not-own) | FAIL | Major | No per-domain definitions exist anywhere | Add per-domain READMEs (one per Tier-1 domain) | | C3.3 (glossary disambiguates Person ≠ AppUser) | FAIL | Critical | No glossary file; the distinction lives only in the user's `MEMORY.md` (`project_person_appuser_separation.md`) | Create `docs/GLOSSARY.md` covering Person, AppUser, Geschichte, TranscriptionBlock, Annotation, Comment, OcrJob | | C3.4 (backend package structure is domain-based) | FAIL | Critical | `controller/`, `service/`, `repository/`, `model/`, `dto/`, `exception/` are layer-packaged. Only `audit/`, `dashboard/`, `relationship/` are feature-packaged. `controller/DocumentController.java`, `service/DocumentService.java`, `repository/DocumentRepository.java`, `model/Document.java`, `dto/DocumentUpdateDTO.java` all live in different packages | Migrate to `document/`, `person/`, `tag/`, `user/`, `geschichte/`, `notification/`, `ocr/`, `shared/` per REFACTOR-1 | | C3.6 (`shared/` only genuinely cross-cutting) | N/A | — | No `shared/` package exists yet | Apply admission criteria during REFACTOR-1 (see §6) | | C4.1 (locate every domain file in ≤2 min) | FAIL | Critical | Domain files spread across 5 layer packages; e.g. `Document` lives in `model/`, `controller/`, `service/`, `repository/`, plus `dto/Document*.java` (×11) | Resolved by C3.4 fix | | C4.2 (controller+service+page+component for a feature in ≤5 min) | FAIL | Major | Backend half of this fails for the same reason as C4.1 | Resolved by C3.4 fix | | C4.3 (no `Helper`/`Utils`/`Manager` without modifier) | PASS | — | Service classes use concept names (`PersonNameParser`, `PersonTypeClassifier`, `DisplayNameFormatter`); no anonymous `Util.java`/`Helper.java` found | — | | C4.4 (API routes predictable from domain) | FAIL | Major | `/api/documents/conversation` (`DocumentController.java:427`) is mounted under documents but the conversation concept is its own canonical view domain. `/api/stats` (StatsController) is unobvious. `UserSearchController` mounts `/api/users/search` but the class name doesn't match the path. Dashboard endpoints are clean. | After domain refactor, document the route→domain map in a `docs/API.md` | | C5.1 (conventions doc: how to add domain/endpoint/page) | FAIL | Major | `backend/CLAUDE.md` describes some conventions; nothing tells a stranger how to add a new endpoint or domain | Add "How to add an endpoint" walkthrough to backend README | | C5.2 (one way to do common things) | FAIL | Major | Three error-handling patterns coexist: `DomainException` (137 sites), `ResponseStatusException` (3 controllers + `PersonService`, 11 sites), raw `RuntimeException`/`IllegalArgumentException`/`IllegalStateException` (5 files, 9 sites — see C5.2 evidence below) | Pick one. Convert all `ResponseStatusException` validation in controllers to bean validation or `DomainException`; route `RuntimeException` in `MassImportService.java:159`, `FileService.java:91`, `RestClientOcrClient.java:218,250,288` through `DomainException.internal(...)` | | C5.3 (CLAUDE.md rules mirrored in human docs) | FAIL | Major | `backend/CLAUDE.md` is the *only* place layering, error-handling, entity-style, OCR-integration, and run instructions are documented. Tobias would not look there. | Migrate the rule sections to `backend/README.md` + `docs/ARCHITECTURE.md` | | C5.4 (naming conventions stated and followed) | PASS | — | Controllers `*Controller`, services `*Service`, repos `*Repository`, DTOs `*DTO`/`*Request`. ≥95% conformance | — | | **C6.1 (no controller injects a repository directly)** | FAIL | Critical | `controller/StatsController.java:18-19` injects `PersonRepository personRepository; DocumentRepository documentRepository;`. `controller/AuthE2EController.java:27` injects `PasswordResetTokenRepository tokenRepository;` (test-only profile, but still a violation by the letter of the rule) | Move `StatsController` body into a `StatsService` (or `DashboardService`). Add a `getResetTokenForTest()` to `PasswordResetService` (gated by `@Profile("e2e")`) and call from `AuthE2EController`. | | **C6.2 (no service depends on another domain's repository)** | FAIL | Critical | Eleven cross-domain repository injections: `MassImportService.java:58` → `DocumentRepository`; `ThumbnailService.java:65`, `ThumbnailBackfillService.java:40`, `ThumbnailAsyncRunner.java:32` → `DocumentRepository`; `TranscriptionQueueService.java:23` → `DocumentRepository`; `SegmentationTrainingExportService.java:30-32` → `TranscriptionBlockRepository, AnnotationRepository, DocumentRepository`; `TrainingDataExportService.java:31-33` → same triple; `SenderModelService.java:35` → `TranscriptionBlockRepository`; `OcrTrainingService.java:40` → `TranscriptionBlockRepository`; `TranscriptionService.java:40` → `AnnotationRepository`; `AnnotationService.java:28` → `TranscriptionBlockRepository`; `PasswordResetService.java:33` → `AppUserRepository` | Each violation becomes a recommendation in REFACTOR-1's prerequisites; introduce read-only query methods on the owning service (e.g. `DocumentService.findIdsForThumbnailBackfill()`, `TranscriptionService.findBlocksForTraining()`) | | C6.4 (cross-cutting code in `shared/`, not duplicated) | PASS-with-caveat | Minor | Cross-cutting bits (`AuditService`, `FileService`, `MinioConfig`, `RateLimitInterceptor`, `GlobalExceptionHandler`, `SecurityUtils`) exist as singletons — not duplicated. But they live in `audit/`, `service/`, `config/`, `controller/`, `security/` rather than a unified `shared/`. | Group under `shared/` during REFACTOR-1 | | C7.1 (per-domain README exists) | FAIL | Major | Zero per-domain READMEs in any package | Create one per Tier-1 domain after refactor | | C7.2 (non-obvious decisions have why-comments) | PASS-with-caveat | Minor | Strong examples: `SecurityConfig.java:40-46` (CSRF rationale), `NotificationController.java:39-41`, `RelationshipController.java:23-29,37-38`, `AuthE2EController.java:29-30`, `application.yaml` (`open-in-view: false` rationale, multipart cap). A few opaque blocks remain (e.g. `SenderModelService` `@Lazy @Autowired self` self-reference has only a one-line comment that requires Spring AOP knowledge) | Add a why-comment near each `@Lazy self` self-injection and at `OcrAsyncRunner` `TransactionSynchronizationManager` block | | C7.3 (no "ask Marcel"/"non-obvious"/"TODO" comments) | PASS | — | `grep -rn "TODO\|FIXME\|XXX\|HACK\|ask Marcel\|Claude generated"` over `org/raddatz/familienarchiv/` returned only one German clarification comment in `repository/DocumentRepository.java:32` | — | | C7.4 (API contracts documented; OpenAPI accessible) | PASS-with-caveat | Minor | SpringDoc enabled only under `dev` profile (`application-dev.yaml`); `backend/CLAUDE.md` describes the regen flow. No human-readable backend README points to it | Mirror the regen instructions in `backend/README.md` | | C7.5 (DB schema has per-table or domain overview) | FAIL | Minor | Migrations are V1..V59 with terse names; no overview file. Stranger must read SQL to understand schema | Add `backend/src/main/resources/db/SCHEMA.md` summarising tables grouped by domain (this is also AUDIT-4 territory) | | C8.1 (critical-path service tests would fail under mutation) | UNVERIFIED | Critical | Mutation testing not run in this audit. By inspection, tests are well-named and assert on behavior (e.g. `DocumentServiceTest`, `PersonServiceTest`, `TranscriptionServiceTest`) but several services are large (DocumentService=971 LOC, MassImportService=385 LOC) and integration-test-heavy | Run PIT mutation testing on `DocumentService`, `PersonService`, `TranscriptionService`, `MassImportService`, `NotificationService`, `OcrAsyncRunner` before REFACTOR-1 | | C8.3 (test names are descriptive) | PASS | — | Sampled `DocumentServiceTest` uses `method_behavior_whenCondition` form (`deleteDocument_throwsNotFound_whenMissing`, `updateDocument_setsArchiveBoxAndFolder`); other test files follow same pattern | — | | C8.4 (tests pass in random order) | PASS | — | No `@TestMethodOrder` or `@Order` annotations found; integration tests use Testcontainers with isolated DB | — | | C9.1 (production deployment documented) | FAIL | Major | No `docs/DEPLOYMENT.md`; `Dockerfile` exists; `docker-compose.yml` orchestrates locally; nothing about prod | Out of scope here but flag for AUDIT-5 | | C9.2 (env vars listed per subsystem) | FAIL | Major | Env vars are inline in `application.yaml` (`SPRING_DATASOURCE_URL`, `S3_*`, `APP_*`, `MAIL_*`, `APP_ADMIN_*`); no consolidated reference | Add a "Configuration" table to `backend/README.md` listing every `${...}` knob with default and meaning | | C9.3 (migration naming convention; non-trivial commented) | PASS-with-caveat | Minor | All 57 migrations follow `V{n}__{snake_description}.sql`. No comment headers spotted in spot-check; this is AUDIT-4's call | — | | C9.4 (logs/observability hooks documented) | FAIL | Minor | `@Slf4j` used widely; `actuator/health` is exposed (`SecurityConfig.java:50`); no doc points to log location or `/actuator` endpoints | One paragraph in backend README | | C10.1 (no dead code / `_old`/`.backup` files) | FAIL | Major | `model/DocumentSort.java` is duplicated by `dto/DocumentSort.java`; only the DTO version is referenced (`grep "model.DocumentSort"` returns zero hits anywhere in backend, frontend, or tests) | Delete `model/DocumentSort.java` | | C10.2 (no premature abstractions) | PASS-with-caveat | Minor | `OcrClient`/`OcrHealthClient` interfaces with one impl (`RestClientOcrClient`) are justified by mockability for tests (test files reference `OcrClient`). `CompletionStatsRow`, `TranscriptionQueueProjection` etc. are legitimate Spring Data projections | — | | C10.3 (no magic numbers/strings) | PASS-with-caveat | Minor | Constants exist where they matter (`TranscriptionService.MAX_TEXT_LENGTH`, `TRANSCRIPTION_COLOR`, `ocr.sender-model.activation-threshold` config). A couple of inline literals in `DocumentController` (`Math.min(limit, 40)` in `DashboardController.java:49`) are tolerable | — | | C10.4 (no half-finished features / `if (false)` / commented blocks) | PASS | — | No `if (false)` matches; no large commented-out blocks found in spot-check | — | | C10.5 (no defensive null/try-catch for impossible cases) | PASS-with-caveat | Minor | `OcrController.resolveUserId()` swallows exceptions returning null — fragile pattern; `RestClientOcrClient.java:218,250,288` wrap-and-rethrow as `RuntimeException` (also a C5.2 hit) | Tighten `OcrController.resolveUserId()`; route OCR exceptions through `DomainException.internal(ErrorCode.OCR_PROCESSING_FAILED, ...)` | --- ## §4 Subsystem health summary | Category | PASS | Critical | Major | Minor | Cosmetic | |---|---|---|---|---|---| | C1 (First Contact) | 0/1 | — | — | 1 | — | | C2 (Local Bootstrap) | 3/5 | — | 1 | 1 | — | | C3 (Domain Clarity) | 0/5 (1 N/A) | 3 | 1 | — | — | | C4 (Findability) | 1/4 | 1 | 2 | — | — | | C5 (Conventions) | 1/4 | — | 3 | — | — | | C6 (Layering) | 1/4 (caveat) | 2 | — | 1 | — | | C7 (Documentation) | 2/5 (caveat) | — | 2 | 1 | — | | C8 (Test Trust) | 2/4 (1 unverified) | 0+1 unverified | — | — | — | | C9 (Operability) | 1/4 (caveat) | — | 2 | 1 | — | | C10 (Code Quality) | 4/5 (caveats) | — | 1 | — | — | **Aggregate:** 15 PASS / **6 FAIL-Critical** / 12 FAIL-Major / 5 FAIL-Minor / 0 Cosmetic + 1 Unverified-Critical (C8.1). **Overall verdict:** 🔴 — six Critical fails, all driven by (a) absent human-readable architecture/glossary documentation and (b) layer-based packaging that the planned REFACTOR-1 will fix. --- ## §5 Domain mapping gaps Each item below is unclear under the current package layout. Canonical homes are derived from D-OQ-1..8. | Item | Plausible homes | Resolution per canonical set | Blocker question | |---|---|---|---| | `TranscriptionBlock`, `TranscriptionBlockVersion`, `BlockSource`, `PolygonConverter`, `TranscriptionService`, `TranscriptionBlockQueryService`, `TranscriptionBlockController`, `TranscriptionBlockRepository`, `TranscriptionBlockVersionRepository` | `document.transcription` vs own `transcription/` domain | **Resolved D-OQ-2:** `document.transcription/` sub-package | None | | `DocumentComment`, `PersonMention`, `CommentService`, `CommentController`, `CommentRepository`, related DTOs (`CreateCommentDTO`, `MentionDTO`) | `document.comment` vs own `comment/` domain | **Resolved D-OQ-3:** `document.comment/` sub-package | None | | `DocumentAnnotation`, `AnnotationService`, `AnnotationController`, `AnnotationRepository`, `CreateAnnotationDTO`, `UpdateAnnotationDTO`, `UniquePoints`/Validator | `document.annotation` vs own | **Resolved D-OQ-4:** `document.annotation/` sub-package | None | | `OcrJob`, `OcrJobDocument`, `OcrJobStatus`, `OcrDocumentStatus`, `OcrTrainingRun`, `SenderModel`, `TrainingStatus`, `TrainingLabel`, `OcrService`, `OcrBatchService`, `OcrAsyncRunner`, `OcrClient`, `OcrHealthClient`, `RestClientOcrClient`, `OcrProgressService`, `OcrTrainingService`, `OcrController`, `OcrJobRepository`, `OcrJobDocumentRepository`, `OcrTrainingRunRepository`, `SenderModelRepository`, OCR DTOs | own `ocr/` domain vs sub-package of `document` | **Resolved D-OQ-5:** own `ocr/` domain | `TrainingLabel` is currently a `document` field (`Document.trainingLabel`); does it stay in `document` (the field is on Document) or move to `ocr` (semantics)? | | `Notification`, `NotificationType`, `NotificationService`, `NotificationController`, `NotificationDTO`, `NotificationPreferenceDTO`, `NotificationRepository`, `SseEmitterRegistry` | own `notification/` domain | **Resolved D-OQ-6:** own `notification/` domain | `NotificationPreference` is currently fields on `AppUser` (`isNotifyOnReply`, `isNotifyOnMention`); does it stay on User or move | | `Tag`, `TagService`, `TagController`, `TagRepository`, `MergeTagDTO`, `TagUpdateDTO`, `TagOperator`, `TagTreeNodeDTO` | own `tag/` domain | Stays as `tag/` (Tier-1) | None | | `Geschichte`, `GeschichteStatus`, `GeschichteService`, `GeschichteController`, `GeschichteRepository`, `GeschichteSpecifications`, `GeschichteUpdateDTO` | own `geschichte/` domain | Stays as `geschichte/` (Tier-1) | None | | `MassImportService`, `BulkEditError/Result`, `DocumentBulkEditDTO`, ODS parser code | `shared/import` vs `document` | **Resolved D-OQ-8:** `shared/import` | The ODS-parsing column-mapping config under `app.import.col.*` is import-specific; safe to live in `shared/import` | | `AuditService`, `AuditLog`, `AuditKind`, `AuditLogRepository`, `AuditLogQueryService`, `AuditLogQueryRepository`, `ActivityActorDTO`, `ActivityFeedRow`, `ContributorRow`, `PulseStatsRow` | `shared/audit` | **Resolved D-OQ-8:** already feature-packaged, rename to `shared/audit` | None | | `FileService`, `MinioConfig`, S3 client | `shared/file-storage` | **Resolved D-OQ-8:** `shared/file-storage` | None | | `ThumbnailService`, `ThumbnailBackfillService`, `ThumbnailAsyncRunner` | `shared/file-storage` (thumbnails are file derivatives) vs `document` (thumbnails belong to documents) | **No explicit decision in D-OQ-1..8.** Default to `shared/file-storage` since thumbnails are a file-storage concern that consumers any document type. | Decision needed: thumbnails as part of file-storage or as a `document.thumbnail` sub-package? | | `TranscriptionQueueService`, `TranscriptionQueueController`, `TranscriptionQueueProjection`, `TranscriptionWeeklyStatsProjection`, `TranscriptionQueueItemDTO`, `TranscriptionWeeklyStatsDTO` | `shared/transcription-queue` (per canonical set's "transcription-queue" cross-cutting entry) | **Resolved by canonical set:** `shared/transcription-queue/` | The queue is a *read view* over Document — confirm it's `shared/` rather than `document.transcription/queue/` | | `DashboardService`, `DashboardController`, `DashboardResumeDTO`, `DashboardPulseDTO`, `ActivityFeedItemDTO` | `shared/dashboard` | **Resolved D-OQ-8:** `shared/dashboard` | None | | `StatsController`, `StatsDTO` | `shared/dashboard` | Fold into `shared/dashboard` (it's a 2-row count endpoint) | None | | `AdminController` (mass-import + backfill triggers + thumbnail status) | `shared/import` + `shared/file-storage` | Split: import endpoints into `shared/import/`, backfill+thumbnail status endpoints with their respective owners | Should AdminController be sliced per-domain or kept as a thin façade? | | `RelationshipController`, `RelationshipService`, `RelationshipInferenceService`, `PersonRelationship`, `PersonRelationshipRepository`, `RelationToken`, `RelationType`, relationship DTOs | `person.relationship` | **Resolved D-OQ-7:** `person.relationship/` sub-package — already feature-packaged at top-level, just nest it | None | | `DisplayNameFormatter` (currently in `model/`, used by `Person` and `PersonSummaryDTO`) | `person/` | Move into `person/` | None | | `PolygonConverter` (in `model/`, JPA `AttributeConverter` for transcription block polygons) | `document.transcription/` | Move into `document.transcription/` | None | | `CustomUserDetailsService`, `SecurityUtils`, `Permission`, `RequirePermission`, `PermissionAspect` | `shared/security` | Already cross-cutting; group under `shared/security` | `Permission` enum has been silently extended (`ANNOTATE_ALL`, `BLOG_WRITE`) — `CLAUDE.md` doesn't mention these | | `RateLimitInterceptor` | `shared/security` or `shared/web` | Pick one — currently in `config/` and applied to login; lives in security territory | — | --- ## §6 Cross-cutting candidates (`shared/` admission) Admission criteria: (a) no entity, (b) no user-facing CRUD, (c) ≥2 consumers OR genuine framework infrastructure. | Candidate | (a) no entity | (b) no CRUD | (c) ≥2 consumers / infra | Verdict | |---|---|---|---|---| | `FileService` + `MinioConfig` | yes (S3 wrapper) | yes | DocumentService, ThumbnailService, OcrAsyncRunner, training exports, MassImportService | `shared/file-storage` | | `AuditService` + `AuditLogRepository` + `AuditLog` (entity!) | **no** — owns `AuditLog` entity | yes (write-only API) | DocumentService, CommentService, AnnotationService, TranscriptionService, UserService, ThumbnailAsyncRunner | `shared/audit` (entity is internal to the cross-cutting concern; admission-criterion (a) interpretation: the entity is *not* a domain entity — it's audit infrastructure) | | `DashboardService` + `DashboardController` + dashboard DTOs | yes (read-only over Audit/Document/Transcription) | yes | one consumer (the dashboard view itself) but is genuinely cross-domain read aggregation | `shared/dashboard` | | `MassImportService` | yes (creates Documents/Persons/Tags via owning services or — currently — directly via DocumentRepository) | yes (background batch only) | one feature-level consumer (AdminController) but framework infra (POI parser + scheduler) | `shared/import` | | `TranscriptionQueueService` | yes (read view over Document.status + transcription state) | yes | TranscriptionQueueController, plus dashboard consumers via projection | `shared/transcription-queue` | | `ThumbnailService` + `ThumbnailBackfillService` + `ThumbnailAsyncRunner` | yes | yes | DocumentService, OcrAsyncRunner, MassImportService | `shared/file-storage` (likely; see §5 blocker) | | `RateLimitInterceptor` | yes | yes | login flow + (potentially) OCR endpoints | `shared/security` (or `shared/web`) | | `GlobalExceptionHandler` + `DomainException` + `ErrorCode` | yes | no | every controller | `shared/error-handling` | | `Permission` + `RequirePermission` + `PermissionAspect` + `SecurityUtils` + `CustomUserDetailsService` + `SecurityConfig` | yes | yes | every controller | `shared/security` | | `AsyncConfig` + `WebConfig` + `FlywayConfig` + `DataInitializer` | yes | yes | framework infra | `shared/config` | | `SseEmitterRegistry` | yes | yes | NotificationController, NotificationService | could stay in `notification/`; *not* admitted to `shared/` (only one consumer outside `notification/`) | | `PersonNameParser` + `PersonTypeClassifier` + `DisplayNameFormatter` | yes | yes | PersonService + MassImportService (parser); both feel cleanly part of `person/` | Stays in `person/` (reject `shared/` — single-domain helpers) | | `OcrClient` + `OcrHealthClient` + `RestClientOcrClient` | yes | yes | OcrService, OcrAsyncRunner, OcrTrainingService, SenderModelService — all in `ocr/` | Stays in `ocr/` (reject `shared/` — all consumers within one domain) | Net: ten `shared/` sub-packages (`shared/file-storage`, `shared/audit`, `shared/dashboard`, `shared/import`, `shared/transcription-queue`, `shared/security`, `shared/error-handling`, `shared/config`, plus optional `shared/web`). --- ## §7 Dead/suspicious code register | File:line | Smell | Action | |---|---|---| | `model/DocumentSort.java:1-6` | Duplicate enum — completely shadowed by `dto/DocumentSort.java`; zero references anywhere | Delete the file | | `controller/StatsController.java:18-19` | Controller injects two repositories directly (C6.1 violation) | Introduce `StatsService` | | `controller/AuthE2EController.java:27` | Test-only controller injects repository directly (C6.1 violation, even if `@Profile("e2e")`) | Add `getResetTokenForTest()` to `PasswordResetService` gated on profile, call from controller | | `service/MassImportService.java:159` | `throw new RuntimeException("Ungültige ODS-Datei: content.xml fehlt")` — bypasses DomainException | Replace with `DomainException.badRequest(ErrorCode.VALIDATION_ERROR, ...)` | | `service/FileService.java:91` | `throw new RuntimeException("Storage Error: " + e.getMessage())` | Replace with `DomainException.internal(ErrorCode.FILE_UPLOAD_FAILED, ...)` | | `service/FileService.java:167` | `throw new IllegalStateException("SHA-256 not available", e)` — same MD pattern in `DocumentService.java:967` | Centralize SHA-256 computation in one helper; if truly impossible, leave one why-comment | | `service/RestClientOcrClient.java:218,250,288` | Three raw `RuntimeException` rethrows from OCR streaming | Route via `DomainException.internal(ErrorCode.OCR_PROCESSING_FAILED, ...)` | | `model/PolygonConverter.java:23,33` | `throw new IllegalArgumentException("Failed to (de)serialize polygon", e)` — JPA AttributeConverter; OK to keep but note it doesn't surface a frontend ErrorCode | Consider `DomainException.internal(...)` if these can ever be hit at runtime; otherwise leave with why-comment | | `service/OcrController.java` `resolveUserId` (around line 161) | Catch-and-return-null pattern when user resolution fails | Make explicit: throw `DomainException.unauthorized(...)` if auth missing | | `service/SenderModelService` `@Lazy @Autowired self` | Self-injection for `@Async` self-call — common Spring pattern but only one comment | Add full why-comment explaining the proxy issue | | `controller/UserController.java:47-62` (`updateProfile`, `changePassword`) | Mutating endpoints lack `@RequirePermission` — currently rely on Spring Security `anyRequest().authenticated()` | Either add `@RequirePermission(Permission.READ_ALL)` for documentation, or document why personal endpoints don't need it (matches `NotificationController.java:39-41` style) | | `controller/StatsController.java` | Mutates nothing but lacks `@RequirePermission` — visible to any authenticated user | Add `@RequirePermission(Permission.READ_ALL)` for consistency | | `repository/DocumentRepository.java:32` | German clarification comment `// z.B. um alle offenen "PLACEHOLDER" zu finden` — minor, language inconsistency vs rest of codebase (English) | Translate or remove | | `service/MassImportService.java:343` | `// --- Helpers ---` section divider is fine; the `Helper` word is a section label, not a class name | No action | | `backend/HELP.md` | Spring Initializr boilerplate, never customized | Delete or replace with real backend README | --- ## §8 Documentation gaps **Missing files (none of these exist):** - `backend/README.md` (human-readable; `HELP.md` is Initializr boilerplate, `CLAUDE.md` is AI-targeted) - Per-domain READMEs for every Tier-1 domain (`document/`, `person/`, `tag/`, `user/`, `geschichte/`, `notification/`, `ocr/`) - Per-`shared/` sub-package READMEs - `docs/ARCHITECTURE.md` (only `c4-diagrams.md` exists; needs prose enumerating canonical domain set + boundary decisions) - `docs/GLOSSARY.md` (Person ≠ AppUser, TranscriptionBlock vs Annotation vs Comment, DocumentStatus lifecycle, Permission semantics, Geschichte vs Document) - `docs/DEPLOYMENT.md` (env vars, profiles, prod vs dev, default creds) - `backend/src/main/resources/db/SCHEMA.md` (per-table comments grouped by domain — also AUDIT-4 territory) **`backend/CLAUDE.md` sections that need migration to human-readable docs (C5.3):** - Overview + Tech Stack → `backend/README.md` - Package Structure + Layering Rules → `docs/ARCHITECTURE.md` (after refactor) and a "Conventions" section in `backend/README.md` - Key Entities table + DocumentStatus lifecycle → `docs/GLOSSARY.md` - Entity Code Style → `docs/CONVENTIONS.md` (new) or backend README "How to add an entity" walkthrough - Services discipline (`@Transactional`, cross-domain via service) → backend README "How to add a service" - Error Handling block → backend README "How to add an error code" (also resolves C5.1 walkthrough gap) - Security / Permissions block → `docs/SECURITY.md` (also `docs/security-guide.md` already exists at root — consolidate); note `Permission` enum drift: CLAUDE.md lists 6 permissions but `Permission.java` has 8 (`ANNOTATE_ALL`, `BLOG_WRITE` undocumented) - OCR Integration block → `docs/integrations/ocr.md` or `ocr/README.md` after refactor - How to Run + OpenAPI generation flow → backend README **Missing inline why-comments (C7.2 caveat):** - `SenderModelService` `@Lazy @Autowired self` (proxy/self-call rationale) - `OcrAsyncRunner` `TransactionSynchronizationManager.registerSynchronization(...)` block (why post-commit dispatch matters) - `application.yaml:38` `max-request-size: 500MB` references `#317` — link to issue is good but the reasoning ("supports 10-file chunk at max per-file size") could be a short header in the file rather than only inline **API contract gaps (C7.4):** - OpenAPI is dev-only; the regen command (`npm run generate:api`) is documented in `frontend/package.json` and `CLAUDE.md` but not in any backend doc that a contributor would discover first --- ## §9 Prerequisites for big-bang refactor (REFACTOR-1 #407) Before moving classes between packages, the following must be true. Any service whose tests fall short of mutation-validation risks silent regressions during the move. **Mutation-validate test suites for these services first** (large, central, business-logic-heavy): | Service | LOC | Reason | |---|---|---| | `DocumentService` | 971 | Owns search/specifications/bulk edit/file-hash logic; tests must assert behavior, not just call paths | | `MassImportService` | 385 | ODS parsing + batch insert; mutations easy to miss | | `TranscriptionService` | 244 | Optimistic locking, version creation, training-block routing | | `PersonService` | 222 | Find-or-create alias logic + merge; merge bugs are silent data corruption | | `NotificationService` | (size unread, central) | Mention-fanout + email + SSE — three side-effect channels | | `OcrAsyncRunner` | (large) | Cross-service orchestration of OCR pipeline; mutations to status transitions silent | **Layering violations that MUST be fixed *before* the package move** (otherwise the move surfaces them as cross-package imports, masking the real fix): | Service / Controller | Repo touched | Fix before refactor | |---|---|---| | `MassImportService` → `DocumentRepository` | document | Route through `DocumentService` (introduce write helpers) | | `ThumbnailService` / `ThumbnailBackfillService` / `ThumbnailAsyncRunner` → `DocumentRepository` | document | Add `DocumentService.findIdsForThumbnailBackfill()` and `DocumentService.updateThumbnailMetadata()` | | `TranscriptionQueueService` → `DocumentRepository` | document | Already a read-only view; add `DocumentService.queueProjection(...)` or admit as documented exception | | `SegmentationTrainingExportService` / `TrainingDataExportService` → `Document/Block/Annotation` repos | three domains | Service-level read methods on each owning service | | `SenderModelService` / `OcrTrainingService` → `TranscriptionBlockRepository` | document.transcription | Read API on `TranscriptionService` | | `TranscriptionService` → `AnnotationRepository` | document.annotation | Read API on `AnnotationService` (sub-package, easier) | | `AnnotationService` → `TranscriptionBlockRepository` | document.transcription | Read API on `TranscriptionService` (sub-package) | | `PasswordResetService` → `AppUserRepository` | user | Inject `UserService.findByEmail/save` | | `StatsController` → `Person/DocumentRepository` | controller→repo | Introduce `StatsService` (or fold into `DashboardService`) | | `AuthE2EController` → `PasswordResetTokenRepository` | controller→repo | `PasswordResetService.findE2EToken(email)` gated on `@Profile("e2e")` | **Prerequisite to delete (avoid carrying dead code into new packages):** - `model/DocumentSort.java` (duplicate of `dto/DocumentSort.java`, unused) - `backend/HELP.md` (replace with real README) **Decisions still needed (one-line ratifications):** - Thumbnails go to `shared/file-storage` or `document.thumbnail`? - Should `Document.trainingLabel` field move to `ocr/` or stay in `document/`? - Should `AppUser` notification-preference fields stay on User or move to `notification/`? --- ## §10 Top-5 prioritized recommendations 1. **Author root README + `docs/ARCHITECTURE.md` + `docs/GLOSSARY.md`** — closes 4 of 6 Critical fails (C1.1, C3.1, C3.2, C3.3) and unblocks Anja and Tobias before any code moves; the package refactor without these docs is a worse legibility outcome, not a better one. 2. **Fix the 11 cross-domain repository injections (C6.2) and 2 controller-injects-repository sites (C6.1) *before* REFACTOR-1 runs** — every violation listed in §9 needs a service-level read/write method on the owning domain so the package move is purely cosmetic, not a behavior change. 3. **Pick one error pattern and migrate** — convert all `throw new RuntimeException(...)`/`IllegalArgumentException(...)` in `MassImportService`, `FileService`, `RestClientOcrClient`, `PolygonConverter`, and `DocumentService` to `DomainException`, and convert controller `ResponseStatusException` validation to `@Valid` + `DomainException`; this satisfies C5.2 and gives the frontend a uniform `code` to translate. 4. **Run PIT mutation testing on `DocumentService`, `PersonService`, `TranscriptionService`, `MassImportService`, `NotificationService`, `OcrAsyncRunner`** — without this, REFACTOR-1's "tests still green" signal is unreliable for the largest blast-radius services (C8.1 currently Unverified-Critical). 5. **Delete `model/DocumentSort.java`, replace `backend/HELP.md` with a real `backend/README.md` (run/build/test/regen-API/env-vars/troubleshooting), and document the silently-added `Permission.ANNOTATE_ALL` and `Permission.BLOG_WRITE`** — small, fast cleanups that close C2.5, C5.3, C7.4, C9.2, C9.4, and C10.1 in one PR each.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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 P1-high audit legibility
Milestone
No items
No Milestone Codebase Legibility
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#389
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?