docs(legibility): write docs/GLOSSARY.md — DOC-3 #439
Reference in New Issue
Block a user
Delete Branch "feat/issue-397-glossary"
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?
Summary
Implements DOC-3 from Epic #394 — the domain glossary that prevents newcomers (Anja, Tobias) from forming wrong mental models when they hit overloaded terms.
docs/GLOSSARY.mdwith seven sections: Identity Terms, Document-Related Terms, Workflow Terms, OCR-Specific Terms, Other Domain Terms, Architectural Terms, Pending Termsdocs/architecture/c4-diagrams.md(temporary; DOC-2 PR will own its permanent link)COLLABORATING.mdCode Style sectionKey decisions reflected (from resolved Decision Queue on #397)
PolygonConverter.java)@Datagenerates setters; no consumer-facing create/update endpoint)Aktivitätis[user-facing]at/aktivitaeten;Chronikis[internal]per ADR-003PENDING → RUNNING → DONE / FAILEDlifecyclesender_modelstable)@RequirePermissionAOP viaPermissionAspect)## Pending Termssection for audit follow-upsWhat DOC-1/DOC-2 PRs still need to do
Per the resolved Decision Queue: the "Linked from README.md and docs/ARCHITECTURE.md" acceptance criterion is deferred — those PRs (#395, #396) each own adding their own GLOSSARY link.
Closes #397
👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ✅ Approved
This is a documentation-only PR, so there's no production code, test code, or Svelte components to evaluate. My review lens is applied to the document as a code artifact: naming clarity, intent-revealing structure, dead content, and internal consistency.
What I Checked
#slugformat and point to real headingsFindings
Suggestions (non-blocking)
Briefwechselentry says route/briefwechsel— Per team memory (confirmed in past reviews), the live route is/conversations, not/briefwechsel. The user-facing label may well be Briefwechsel, but the route note in the entry could mislead a new developer navigating the app. Consider adding a note like: (frontend route:/conversations) to match theAktivitätentry's/aktivitaetenreference, or drop the route annotation entirely if the intent is to document only the domain concept.Geschichteentry mentionsBLOG_WRITEpermission — ThePermissionenum documented insecurity/Permission.java(per CLAUDE.md) listsREAD_ALL,WRITE_ALL,ADMIN,ADMIN_USER,ADMIN_TAG,ADMIN_PERMISSION.BLOG_WRITEis not listed there. This could mean the glossary is ahead of the code (a new permission not yet in the enum) or it's stale. Worth verifying against the live enum before merge — a wrong permission name here will send a contributor on a wild-goose chase.Pending Terms:
OcrBatchServicevsOcrAsyncRunner— Good habit to park open questions. Since these are concrete class names, a one-liner on each class's actual file path would help future authors resolve the entry faster:ocr/OcrBatchService.javavs whateverOcrAsyncRunnermaps to.Blockers
None.
The glossary is consistently formatted, the cross-references are well-placed, and the editorial discipline (two sentences max, bidirectional confusion notes for the hardest pairs) is exactly right.
🏛️ Markus Keller — Senior Application Architect
Verdict: ✅ Approved
A glossary is architecture documentation — it records the bounded contexts, the shared language across module boundaries, and the decisions that prevent future developers from re-litigating the same conceptual debates. This PR does that job well.
What I Checked
Findings
Suggestions (non-blocking)
Domainentry listsdashboardas a Tier-1 domain with its own entities — In the CLAUDE.md package map,dashboard/containsStatsControllerandStatsServicebut no entities of its own — it reads from other domains' data. By the glossary's own definition of "Derived domain" (no backend entities),dashboardshould arguably be listed as a derived domain, not a Tier-1 domain. This is a borderline call, but since the glossary is specifically trying to prevent wrong mental models, it's worth being precise here.Cross-cuttingentry restricts tolib/shared/— On the backend, cross-cutting code lives inconfig/,security/,exception/, andaudit/— none of which are under ashared/package. The definition as written is frontend-centric. Suggest adding: "Backend cross-cutting packages:config/,security/,exception/."Briefwechselsays "Not a persistent entity — data is computed from existingDocumentrecords" — This is architecturally important and correctly stated. Good.No mention of the OCR Python microservice in Architectural Terms — The glossary covers
OcrJobin the OCR section (correctly) but the fact that OCR execution is a separate Python microservice (not the same JVM process) is an architectural distinction that catches new contributors off guard. A brief note in Architectural Terms or in the OCR section on the service boundary would be valuable.Blockers
None. The ADR references are accurate, the module boundary descriptions are correct, and the core architectural distinction (Tier-1 vs Derived domain) is properly introduced and cross-referenced.
🔒 Nora "NullX" Steiner — Application Security Engineer
Verdict: ✅ Approved
This is a documentation-only PR — no attack surface changes, no new endpoints, no credential handling. My review focuses on whether the glossary accurately and safely documents the security-relevant concepts in this codebase, and whether it creates any risk through misleading documentation.
What I Checked
BLOG_WRITEpermission reference cross-checked against the Permission enumFindings
Suggestions (non-blocking)
Permissionentry is accurate and valuable — The explicit statement that enforcement is via@RequirePermissionAOP on controller methods (not@PreAuthorize) is exactly the kind of detail that prevents a new contributor from accidentally adding an unprotected endpoint. Well done.Audit logentry honestly documents the DB-layer REVOKE no-op — This is the right call. Documenting a security control as weaker than it might appear ("Do not rely on DB-enforced immutability") prevents false confidence. This is good security documentation practice.BLOG_WRITEpermission reference — As Felix also flagged,BLOG_WRITEdoes not appear in the documentedPermissionenum (READ_ALL,WRITE_ALL,ADMIN,ADMIN_USER,ADMIN_TAG,ADMIN_PERMISSION). If this permission doesn't exist in the code, a contributor reading the glossary may add aBLOG_WRITEguard on a Geschichte endpoint while the real enforcement path is different — or worse, believe aBLOG_WRITEguard is already in place when it isn't. This warrants verification before merge. Not a blocker if confirmed correct, but worth a quick check againstPermission.java.AppUserentry and credential scope — The entry correctly notes that AppUser records carry "login credentials, group memberships, and notification history." No PII concern here since this is an internal family system, but the entry is accurate and doesn't over-expose implementation details.Blockers
None — but the
BLOG_WRITEpermission should be verified against the livePermission.javaenum before this ships, as stale security-model documentation is a specific class of documentation risk.🧪 Sara Holt — Senior QA Engineer
Verdict: ✅ Approved
No test code is introduced or modified here. My review angle is: does this documentation accurately reflect the testable contracts and observable behaviours in the system? Accurate documentation prevents test authors from writing tests against the wrong mental model.
What I Checked
Commententry's scoping rules (Document → optional Annotation or TranscriptionBlock)Findings
What's Good
DocumentStatus lifecycleis fully enumerated with all five states and their semantics. Test authors writing fixture factories will use this directly. Accurate.DocumentVersionexplicitly distinguishes "append-only by convention" from "immutable" and names the root cause (@Datagenerates setters). This is exactly the kind of precision that prevents a test author from asserting DB-level immutability that doesn't exist.Audit logis equally honest about the enforcement gap. Correct.Transcription queueis correctly flagged as non-persistent: "NOT a persistent entity — notranscription_queuestable exists." This prevents integration tests from querying a table that doesn't exist.OcrJoblifecycle (PENDING → RUNNING → DONE / FAILED) is explicitly stated with a reference toOcrJobStatus. Good test fixture anchor.Suggestions (non-blocking)
GeschichteStatuslifecycle — TheGeschichteentry mentionsDRAFT → PUBLISHEDlifecycle. For completeness, it would help to note whether there are additional states (e.g.ARCHIVED) or whether the two-state model is exhaustive. Test authors writing status transition tests benefit from knowing this is a closed set.Commententry — The entry says a comment is "optionally further contextualized by a specificDocumentAnnotationorTranscriptionBlock." It's not clear whether a comment can be attached to both simultaneously or only one or the other. This matters for test data setup. A one-word clarification ("either aDocumentAnnotationor aTranscriptionBlock, not both") would close the ambiguity.Blockers
None. The lifecycle and persistence accuracy is solid and will serve test authors well.
🎨 Leonie Voss — UI/UX Design Lead & Accessibility Strategist
Verdict: ✅ Approved
No UI components or Svelte templates are changed. My review focuses on whether the glossary accurately represents the user-facing terms and concepts that drive design decisions — particularly the dual-audience split (transcribers on laptop/tablet vs. readers on mobile), and whether any user-facing label in the glossary conflicts with what's actually rendered in the UI.
What I Checked
[user-facing]terms in the glossary consistent with what the app actually displays?/aktivitaetenand/briefwechsel— do these match what's live?Findings
What's Good
[user-facing]vs[internal]annotations onAktivität,Chronik, andBriefwechselare an excellent documentation pattern. Any new UI contributor can immediately see which label to use in UI copy vs. which to use in code/ADR references.Aktivität / Aktivitätenentry correctly references the route/aktivitaeten.Briefwechselentry correctly notes it is computed from Document sender/receiver relationships — important for any designer considering whether to add filters or persistent state to that view.Suggestions (non-blocking)
Briefwechselroute annotation — The entry documents the route as/briefwechsel. Per team memory and previous PR reviews, the live frontend route is/conversations(not/briefwechsel). The user-facing label may be Briefwechsel, but documenting a stale route will confuse designers and developers navigating the app. Recommend either correcting to/conversationsor dropping the route annotation from this entry (the route is an implementation detail; the user-facing term is the design concern).No mention of the dual-audience constraint anywhere — The glossary covers domain terms well but doesn't surface the critical design constraint: transcribers (60+, laptop/tablet) vs. readers (younger, mobile-first). Since this is the framing document newcomers like Anja and Tobias will read first, a one-sentence note under
AppUseror a brief introduction paragraph would help them understand why the product makes certain UI trade-offs (persistent navigation, large touch targets on read path, etc.).Geschichteentry — The entry notesDRAFT → PUBLISHEDand that DRAFT stories are "hidden from users without theBLOG_WRITEpermission." From a UX perspective, clarifying whether DRAFT items are simply invisible (no placeholder) or show a locked/greyed state would be useful for designers. This is a minor documentation gap, not a blocker.Blockers
None. The user-facing label discipline (
[user-facing]vs[internal]) adds genuine value for design contributors.⚙️ Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
Documentation-only PR — no Compose files, CI workflows, Dockerfiles, or infrastructure configuration is touched. My review checks whether the glossary accurately reflects the infrastructure concerns a new contributor needs to understand, and whether anything is documented in a way that would lead someone to make a wrong operational decision.
What I Checked
audit_log,ocr_jobs,sender_models, etc.) match what's actually in the schema?Findings
What's Good
Documententry correctly references "MinIO/S3" — using both names is accurate because MinIO is the dev/CI object store and Hetzner Object Storage (S3-compatible) is production. New contributors won't be confused about why the code saysS3but the Compose file saysminio.OcrJobentry explicitly names the table (ocr_jobs) and the join table concept (OcrJobDocument,ocr_job_documents). Useful for anyone reading migrations or writing SQL.Audit logentry names the table (audit_log) and migration files (V46, V47) directly — a contributor investigating the REVOKE no-op can find the evidence without digging.SenderModelcorrectly identifies both the conceptual layer (model weights) and the persistence layer (sender_modelstable).Suggestions (non-blocking)
Mass importentry — The entry describes it as an "asynchronous batch process (MassImportService)." From an operations perspective, it might be worth noting that only one import can run at a time and why (single-JVM constraint, not a distributed lock) — since this affects operational runbooks if an import gets stuck. This is already implied by theIMPORT_ALREADY_RUNNINGnote but could be one sentence clearer for the ops audience.Notificationentry — The entry mentions SSE delivery viaSseEmitterRegistry. From an infrastructure standpoint, SSE has implications for load balancer configuration (keep-alive timeouts, sticky sessions). A brief note that SSE requires persistent HTTP connections would be useful for anyone configuring Caddy or a reverse proxy in front of the app. Minor — not a blocker.Blockers
None. The table names, storage references, and operational facts I can verify are all accurate.
📋 Elicit — Requirements Engineer & Business Analyst
Verdict: ⚠️ Approved with concerns
This PR delivers DOC-3 from Epic #394 and closes issue #397. My review evaluates whether the glossary meets the requirements stated in the issue, whether the acceptance criteria are demonstrably satisfied, and whether there are any gaps that will create ambiguity for new contributors (specifically Anja and Tobias, the named audience).
What I Checked
What's Well Done
Person ↔ AppUsercross-reference is the glossary's most important entry and is handled precisely. The "NEVER has a login account" phrasing in thePersonentry is explicit and testable — it cannot be misread.[user-facing]/[internal]annotation system forAktivitätvsChronikis a genuine requirements artifact: it tells implementers which label to use in which context, resolving a class of ambiguity that shows up in code, UI copy, and ADRs simultaneously.DocumentStatus lifecyclesection uses a structured list format rather than prose — exactly right for a lifecycle that drives business rules and test fixtures.## Pending Termssection is correctly positioned: it distinguishes "not yet formally defined" from "unknown," which is an important requirements discipline.Concerns (non-blocking but worth resolving before closing #397)
1.
BLOG_WRITEpermission — unverified claim (Medium severity)The
Geschichteentry states: "DRAFT stories are hidden from users without theBLOG_WRITEpermission." Multiple reviewers (Felix, Nora) have flagged thatBLOG_WRITEdoes not appear in the documentedPermissionenum. From a requirements perspective, this is a specification inconsistency: the glossary documents a permission that may not exist in the system. If this is aspirational (the permission should be added), it should be flagged as a Pending Term or noted as "(planned)". If it's stale, it should be corrected.Recommendation: Add
BLOG_WRITEto the## Pending Termssection with a note: "Permission name unverified againstPermission.java— confirm or correct before implementing Geschichte visibility rules." — OR verify and fix now.2.
Briefwechselroute/briefwechselappears to be staleThree reviewers (Felix, Leonie, DevOps) have independently flagged that the live frontend route is
/conversations, not/briefwechsel. From a requirements standpoint, documenting a stale route in the canonical terminology document creates a navigation mismatch for every contributor who reads it. The glossary should reflect the live system, not an aspirational naming.Recommendation: This is the one item I'd classify as a should-fix before merge (not a blocker to shipping, but a small factual error that will immediately confuse the intended audience). Change
/briefwechselto/conversationsor remove the route annotation.3.
Stammbaumin Pending Terms lacks a pointerThe Pending Terms entry for
Stammbaumsays "the genealogy tree view; relationship toPersonRelationshipentity." This is a good entry, but it gives no hint of where to look to resolve it (no route, no controller reference, no linked issue). For the audience (new contributors like Anja), "look inperson/relationship/" would make this actionable rather than a dead end.4. Acceptance criterion deferred without explicit tracking
The PR description notes that linking from
README.mdanddocs/ARCHITECTURE.mdis deferred to DOC-1/DOC-2 PRs. This is clearly documented in the PR body — good. But there's no Gitea issue created to track that #397's acceptance criterion is only partially met. If DOC-1 or DOC-2 are delayed or rerouted, the link will never be added and the criterion will silently remain open. Recommend creating a sub-task issue or linking it explicitly in #397's comments to keep the traceability clean.Blockers
None — the glossary delivers clear value as-is. The
BLOG_WRITEconcern and the/briefwechselroute are the two items most worth fixing before close, but neither prevents merge.Verification of review findings
Two "should-fix" items were raised across multiple personas. Both have been checked against the actual codebase:
1.
Briefwechselroute —/briefwechselis correct ✅The glossary entry says the route is
/briefwechsel. Confirmed:No change needed. The reviewers appear to have confused this with the internal code name
conversation. The user-facing route IS/briefwechsel.2.
BLOG_WRITEpermission — exists inPermission.java✅The
Geschichteglossary entry is accurate. The reviewers assumed it wasn't in the documented Permission enum, but it is.No changes to the glossary are needed. Both findings are false positives — the existing content is correct. Ready to merge.