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

docs(legibility): write docs/ARCHITECTURE.md with diagram and domain list #396

New Issue
Closed
opened 2026-05-04 16:07:41 +02:00 by marcel · 10 comments
marcel commented 2026-05-04 16:07:41 +02:00
Owner
Copy Link

Context

Part of Epic #394 — Documentation. This is DOC-2: the architecture document Tobias will open second (after the README). It must let him reproduce the system on a whiteboard from memory after one read.

Per the Legibility Rubric, this addresses C1.3, C3.1, C3.2, C3.6 (all Critical or Major).

Required content

A single docs/ARCHITECTURE.md containing, in this order:

1. High-level diagram

A diagram (ASCII art is fine; Mermaid preferred if rendered) showing:

  • The 5 subsystems as boxes (frontend, backend, ocr-service, db, minio)
  • The arrows between them (HTTP API, JDBC, S3 API, OCR HTTP, SSE notifications)
  • External boundaries (the user's browser, MinIO presigned URLs to OCR)

The reader should be able to redraw this from memory.

2. Canonical domain set

List every domain from the Canonical Domain Set in #387 first comment. For each Tier-1 domain (document, person, tag, user, geschichte, notification, ocr):

  • One paragraph: what it owns, what it does NOT own
  • Key entities (and the table they map to)
  • Cross-domain dependencies (e.g., "Document references Person via sender/receivers")

For each Tier-2 (derived) domain (conversation, activity):

  • Explain the "derived domain" concept: a domain that exists conceptually and has its own UI but is built from other domains' data
  • What it's derived from

3. Cross-cutting layer

List every shared/ member (audit, file-storage, import, dashboard, transcription-queue, security, error-handling, config, i18n, discussion). For each, one sentence on what it does and the admission criteria it satisfies.

4. Stack-symmetry principle

Explain that domain names are identical across backend/src/main/java/.../ and frontend/src/lib/. State the rule: "Adding a new domain means adding it on both sides under the same name."

5. Key architectural decisions

Pull existing ADRs and rules from CLAUDE.md files (root + per-subsystem) and present them here in human-readable form. Examples:

  • Layering rule (no controller→repo direct injection; no cross-domain repo access)
  • Single-node OCR service (ADR-001 from ocr-service/CLAUDE.md)
  • Permission system (@RequirePermission, Permission enum, AOP enforcement)
  • Big-bang refactor decision (one PR per stack — link to #387)

6. Data flow walkthroughs

Two concrete walkthroughs (1 paragraph each):

  • "What happens when a user uploads a document" (from browser → backend → MinIO → OCR queue → notification)
  • "What happens when a user transcribes a block" (autosave, conflict resolution, audit log, SSE)

Anti-patterns

  • Do NOT put implementation details (code, file paths, line numbers). This is conceptual.
  • Do NOT exceed ~400 lines. If you need more, split into per-domain READMEs (DOC-6).

Acceptance criteria

  • docs/ARCHITECTURE.md exists with all 6 sections
  • Diagram is reproducible from memory (Tobias-test)
  • Every canonical-set domain is named and one-paragraph defined
  • Stack-symmetry principle is stated as a rule
  • All ADRs from existing CLAUDE.md files are migrated here in human-readable form
  • PR opened and merged

Dependency

Soft dependency on AUDIT-2 (#389), AUDIT-3 (#390), AUDIT-4 (#391), AUDIT-5 (#392) for findings about existing architectural decisions. Can be drafted in parallel and refined when audits land.

Definition of Done

docs/ARCHITECTURE.md committed on main; closing comment links to it. Issue closed via Closes #N in commit.

## Context Part of **Epic #394** — Documentation. This is **DOC-2**: the architecture document Tobias will open second (after the README). It must let him **reproduce the system on a whiteboard from memory** after one read. Per the Legibility Rubric, this addresses **C1.3, C3.1, C3.2, C3.6** (all Critical or Major). ## Required content A single `docs/ARCHITECTURE.md` containing, in this order: ### 1. High-level diagram A diagram (ASCII art is fine; Mermaid preferred if rendered) showing: - The 5 subsystems as boxes (frontend, backend, ocr-service, db, minio) - The arrows between them (HTTP API, JDBC, S3 API, OCR HTTP, SSE notifications) - External boundaries (the user's browser, MinIO presigned URLs to OCR) The reader should be able to redraw this from memory. ### 2. Canonical domain set List every domain from the Canonical Domain Set in #387 first comment. For each Tier-1 domain (`document`, `person`, `tag`, `user`, `geschichte`, `notification`, `ocr`): - One paragraph: what it owns, what it does NOT own - Key entities (and the table they map to) - Cross-domain dependencies (e.g., "Document references Person via sender/receivers") For each Tier-2 (derived) domain (`conversation`, `activity`): - Explain the "derived domain" concept: a domain that exists conceptually and has its own UI but is built from other domains' data - What it's derived from ### 3. Cross-cutting layer List every `shared/` member (audit, file-storage, import, dashboard, transcription-queue, security, error-handling, config, i18n, discussion). For each, one sentence on what it does and the admission criteria it satisfies. ### 4. Stack-symmetry principle Explain that domain names are identical across `backend/src/main/java/.../` and `frontend/src/lib/`. State the rule: "Adding a new domain means adding it on both sides under the same name." ### 5. Key architectural decisions Pull existing ADRs and rules from `CLAUDE.md` files (root + per-subsystem) and present them here in human-readable form. Examples: - Layering rule (no controller→repo direct injection; no cross-domain repo access) - Single-node OCR service (ADR-001 from `ocr-service/CLAUDE.md`) - Permission system (`@RequirePermission`, `Permission` enum, AOP enforcement) - Big-bang refactor decision (one PR per stack — link to #387) ### 6. Data flow walkthroughs Two concrete walkthroughs (1 paragraph each): - "What happens when a user uploads a document" (from browser → backend → MinIO → OCR queue → notification) - "What happens when a user transcribes a block" (autosave, conflict resolution, audit log, SSE) ## Anti-patterns - Do NOT put implementation details (code, file paths, line numbers). This is conceptual. - Do NOT exceed ~400 lines. If you need more, split into per-domain READMEs (DOC-6). ## Acceptance criteria - [ ] `docs/ARCHITECTURE.md` exists with all 6 sections - [ ] Diagram is reproducible from memory (Tobias-test) - [ ] Every canonical-set domain is named and one-paragraph defined - [ ] Stack-symmetry principle is stated as a rule - [ ] All ADRs from existing CLAUDE.md files are migrated here in human-readable form - [ ] PR opened and merged ## Dependency Soft dependency on AUDIT-2 (#389), AUDIT-3 (#390), AUDIT-4 (#391), AUDIT-5 (#392) for findings about existing architectural decisions. Can be drafted in parallel and refined when audits land. ## Definition of Done `docs/ARCHITECTURE.md` committed on `main`; closing comment links to it. Issue closed via `Closes #N` in commit.
marcel added this to the Codebase Legibility milestone 2026-05-04 16:07:41 +02:00
marcel added the P0-criticaldocumentationlegibility labels 2026-05-04 16:09:49 +02:00
marcel commented 2026-05-04 21:04:01 +02:00
Author
Owner
Copy Link

🏛️ Markus Keller — Senior Application Architect

Observations

  • docs/ARCHITECTURE.md does not yet exist. The nearest equivalent is docs/architecture/c4-diagrams.md (266 lines, last updated before OCR service, Notification, Geschichte, and the full canonical domain set were added). That file omits: the OCR container and its HTTP+SSE protocol, SSE notifications path, the Tier-2 derived domains (conversation, activity), and the seven cross-cutting members in shared/.
  • Six ADRs already exist under docs/adr/ (001–006). None of them are referenced from any human-readable architecture document. Section 5 ("Key architectural decisions") is the place to consolidate their conclusions into prose — not to duplicate them, but to summarise the what and link to the ADR for the why.
  • The backend package structure in backend/src/main/java/.../ is still layer-first (controller/, service/, model/, repository/) rather than domain-first. Section 4 ("Stack-symmetry principle") should state this as the target state, not the current state. Tobias reading the doc should know: "the code doesn't look like this yet — the refactor is what gets it there."
  • The docs/architecture/c4-diagrams.md Mermaid diagrams are GitHub/Gitea-renderable and already describe the L1/L2/L3 structure. Section 1 of the new doc should reference or embed the L2 diagram rather than redrawing it as ASCII. Mermaid in the existing file is the better starting point.
  • The issue asks for two data-flow walkthroughs (upload and transcription block autosave). The upload flow already exists as a sequence diagram in docs/architecture/c4-diagrams.md. Reuse it (updated to include OCR queue step) rather than rewriting in prose.

Recommendations

  • Scope the audience explicitly at the top. The issue says "Tobias can reproduce the system on a whiteboard from memory." Add one sentence: "Target reader: a PM-with-CS background who has read the README. Goal: accurate mental model after one read." This prevents the doc from drifting into API reference territory.
  • Section 1 diagram: update the existing Mermaid L2, do not create ASCII art. The existing C4 container diagram is missing ocr-service (Python, port 8000), the SSE path (backend → frontend), and the presigned-URL path (MinIO → ocr-service). These three additions make the diagram current. Keep it at C4 Level 2 — that is the whiteboard-reproducible level.
  • Section 2 (domain set): state current vs. target clearly. The canonical domain set from #387 is the target of the refactor. The current backend has a flat model/ package with Geschichte.java, Notification.java, OcrJob.java, etc. The doc should say: "The following domains have been ratified as the target structure; code organisation will match this after the Refactor epic."
  • Section 5 (ADRs): include all six existing ADRs. ADR-001 through ADR-006 already capture the single-node OCR constraint, polygon JSONB storage, unified activity feed, PDFBox thumbnails, thumbnail aspect ratio, and synchronous domain events. Summarise each in 2–3 sentences in the doc body and link to the full ADR file. Do not copy the full ADR text.
  • Keep the layering rule in Section 5, not Section 4. The issue puts it in "key architectural decisions" — that is the right place. Section 4 should stay narrowly focused on the stack-symmetry naming convention.

Open Decisions

  • Should docs/ARCHITECTURE.md describe the current state or the target state of the domain structure? Option A: describe current state (layer-first packages) and note the target. Option B: describe only the target (domain-first packages) and note "not yet implemented." Option A is safer for Tobias — a discrepancy between doc and code is worse than a clearly labelled aspiration. (This affects how Section 2 and Section 4 are framed.)
## 🏛️ Markus Keller — Senior Application Architect ### Observations - `docs/ARCHITECTURE.md` does not yet exist. The nearest equivalent is `docs/architecture/c4-diagrams.md` (266 lines, last updated before OCR service, Notification, Geschichte, and the full canonical domain set were added). That file omits: the OCR container and its HTTP+SSE protocol, SSE notifications path, the Tier-2 derived domains (`conversation`, `activity`), and the seven cross-cutting members in `shared/`. - Six ADRs already exist under `docs/adr/` (001–006). None of them are referenced from any human-readable architecture document. Section 5 ("Key architectural decisions") is the place to consolidate their conclusions into prose — not to duplicate them, but to summarise the *what* and link to the ADR for the *why*. - The backend package structure in `backend/src/main/java/.../` is still **layer-first** (`controller/`, `service/`, `model/`, `repository/`) rather than **domain-first**. Section 4 ("Stack-symmetry principle") should state this as the *target* state, not the current state. Tobias reading the doc should know: "the code doesn't look like this yet — the refactor is what gets it there." - The `docs/architecture/c4-diagrams.md` Mermaid diagrams are GitHub/Gitea-renderable and already describe the L1/L2/L3 structure. Section 1 of the new doc should reference or embed the L2 diagram rather than redrawing it as ASCII. Mermaid in the existing file is the better starting point. - The issue asks for two data-flow walkthroughs (upload and transcription block autosave). The upload flow already exists as a sequence diagram in `docs/architecture/c4-diagrams.md`. Reuse it (updated to include OCR queue step) rather than rewriting in prose. ### Recommendations - **Scope the audience explicitly at the top.** The issue says "Tobias can reproduce the system on a whiteboard from memory." Add one sentence: "Target reader: a PM-with-CS background who has read the README. Goal: accurate mental model after one read." This prevents the doc from drifting into API reference territory. - **Section 1 diagram: update the existing Mermaid L2, do not create ASCII art.** The existing C4 container diagram is missing `ocr-service` (Python, port 8000), the SSE path (backend → frontend), and the presigned-URL path (MinIO → ocr-service). These three additions make the diagram current. Keep it at C4 Level 2 — that is the whiteboard-reproducible level. - **Section 2 (domain set): state current vs. target clearly.** The canonical domain set from #387 is the *target* of the refactor. The current backend has a flat `model/` package with `Geschichte.java`, `Notification.java`, `OcrJob.java`, etc. The doc should say: "The following domains have been ratified as the target structure; code organisation will match this after the Refactor epic." - **Section 5 (ADRs): include all six existing ADRs.** ADR-001 through ADR-006 already capture the single-node OCR constraint, polygon JSONB storage, unified activity feed, PDFBox thumbnails, thumbnail aspect ratio, and synchronous domain events. Summarise each in 2–3 sentences in the doc body and link to the full ADR file. Do not copy the full ADR text. - **Keep the layering rule in Section 5, not Section 4.** The issue puts it in "key architectural decisions" — that is the right place. Section 4 should stay narrowly focused on the stack-symmetry naming convention. ### Open Decisions - **Should `docs/ARCHITECTURE.md` describe the current state or the target state of the domain structure?** Option A: describe current state (layer-first packages) and note the target. Option B: describe only the target (domain-first packages) and note "not yet implemented." Option A is safer for Tobias — a discrepancy between doc and code is worse than a clearly labelled aspiration. _(This affects how Section 2 and Section 4 are framed.)_
marcel commented 2026-05-04 21:04:33 +02:00
Author
Owner
Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

  • The issue explicitly forbids implementation details (code, file paths, line numbers) — this is correct. Architecture docs that list line numbers become stale within a sprint.
  • The backend currently lives in a layer-first package structure (controller/, service/, model/, repository/) while the issue asks the doc to describe a domain-first structure. The Geschichte.java, OcrJob.java, Notification.java entities exist in the flat model/ package. The doc must not mislead: if it says document/DocumentService.java that file does not exist yet at that path.
  • The frontend src/lib/ already has closer-to-domain organisation (geschichten/ route exists, components/ directory has domain-shaped components). The asymmetry between frontend and backend organisation should be called out honestly in Section 4.
  • Section 6 asks for "What happens when a user transcribes a block (autosave, conflict resolution, audit log, SSE)." I looked at the service layer: TranscriptionService.java, TranscriptionBlockQueryService.java, OcrProgressService.java, SseEmitterRegistry.java all exist. The autosave and SSE path are real and worth documenting. The walkthrough should name the chain: TranscriptionService → persists block → SseEmitterRegistry broadcasts → NotificationService on mention. This is conceptual enough to write without line numbers.
  • The 400-line limit is tight for 6 sections covering 9 Tier-1 domains + 2 Tier-2 + 11 cross-cutting members. I'd budget it as: diagram (~15 lines), domain set (~120 lines at ~11 lines/domain), cross-cutting (~33 lines), stack-symmetry (~10 lines), ADRs (~80 lines for 6 ADRs), data flows (~40 lines). That's ~298 lines — achievable. Do not add prose padding.

Recommendations

  • Write the domain definitions against what the code actually contains today. The model/ package has: Document, Person, Tag, AppUser, Geschichte, Notification, OcrJob, TranscriptionBlock, DocumentAnnotation, DocumentComment. Map each to the canonical domain it belongs to — do not invent a structure that doesn't exist. When the domain is "not yet extracted to its own package," say so in one parenthetical.
  • The two walkthroughs in Section 6 should name the services, not the classes. "The backend delegates file storage to FileService, which calls MinIO via the AWS SDK v2 S3Client." That level of naming is helpful without being a code reference. Avoid: "see FileService.java:104."
  • Derived domain concept in Section 2 needs one crisp sentence. Tobias needs to understand conversation and activity without understanding the code. Draft: "A derived domain has its own UI but no dedicated database tables; it is assembled by the frontend from data owned by other domains." That is the whole concept in one sentence — do not pad it.
  • The ~400-line limit should be treated as a hard ceiling. If it looks like it will be exceeded, cut Section 3 (cross-cutting) to a bullet-point table rather than a paragraph per member. The table is more scannable and uses fewer lines.

Open Decisions (omit this section entirely if none)

  • Level of detail for the OCR domain walkthrough. The OCR pipeline involves five service classes (OcrService, OcrBatchService, OcrAsyncRunner, OcrClient, OcrProgressService). How much of that chain belongs in a conceptual architecture doc vs. an OCR-specific domain README (DOC-6)? If this doc stays conceptual, one paragraph suffices. If it tries to document the OCR pipeline in full, the 400-line limit breaks. (Raised by Felix — affects Section 6.)
## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - The issue explicitly forbids implementation details (code, file paths, line numbers) — this is correct. Architecture docs that list line numbers become stale within a sprint. - The backend currently lives in a **layer-first** package structure (`controller/`, `service/`, `model/`, `repository/`) while the issue asks the doc to describe a **domain-first** structure. The `Geschichte.java`, `OcrJob.java`, `Notification.java` entities exist in the flat `model/` package. The doc must not mislead: if it says `document/DocumentService.java` that file does not exist yet at that path. - The frontend `src/lib/` already has closer-to-domain organisation (`geschichten/` route exists, `components/` directory has domain-shaped components). The asymmetry between frontend and backend organisation should be called out honestly in Section 4. - Section 6 asks for "What happens when a user transcribes a block (autosave, conflict resolution, audit log, SSE)." I looked at the service layer: `TranscriptionService.java`, `TranscriptionBlockQueryService.java`, `OcrProgressService.java`, `SseEmitterRegistry.java` all exist. The autosave and SSE path are real and worth documenting. The walkthrough should name the chain: `TranscriptionService` → persists block → `SseEmitterRegistry` broadcasts → `NotificationService` on mention. This is conceptual enough to write without line numbers. - The 400-line limit is tight for 6 sections covering 9 Tier-1 domains + 2 Tier-2 + 11 cross-cutting members. I'd budget it as: diagram (~15 lines), domain set (~120 lines at ~11 lines/domain), cross-cutting (~33 lines), stack-symmetry (~10 lines), ADRs (~80 lines for 6 ADRs), data flows (~40 lines). That's ~298 lines — achievable. Do not add prose padding. ### Recommendations - **Write the domain definitions against what the code actually contains today.** The `model/` package has: `Document`, `Person`, `Tag`, `AppUser`, `Geschichte`, `Notification`, `OcrJob`, `TranscriptionBlock`, `DocumentAnnotation`, `DocumentComment`. Map each to the canonical domain it belongs to — do not invent a structure that doesn't exist. When the domain is "not yet extracted to its own package," say so in one parenthetical. - **The two walkthroughs in Section 6 should name the services, not the classes.** "The backend delegates file storage to `FileService`, which calls MinIO via the AWS SDK v2 S3Client." That level of naming is helpful without being a code reference. Avoid: "see `FileService.java:104`." - **Derived domain concept in Section 2 needs one crisp sentence.** Tobias needs to understand `conversation` and `activity` without understanding the code. Draft: "A derived domain has its own UI but no dedicated database tables; it is assembled by the frontend from data owned by other domains." That is the whole concept in one sentence — do not pad it. - **The ~400-line limit should be treated as a hard ceiling.** If it looks like it will be exceeded, cut Section 3 (cross-cutting) to a bullet-point table rather than a paragraph per member. The table is more scannable and uses fewer lines. ### Open Decisions _(omit this section entirely if none)_ - **Level of detail for the OCR domain walkthrough.** The OCR pipeline involves five service classes (`OcrService`, `OcrBatchService`, `OcrAsyncRunner`, `OcrClient`, `OcrProgressService`). How much of that chain belongs in a conceptual architecture doc vs. an OCR-specific domain README (DOC-6)? If this doc stays conceptual, one paragraph suffices. If it tries to document the OCR pipeline in full, the 400-line limit breaks. _(Raised by Felix — affects Section 6.)_
marcel commented 2026-05-04 21:04:52 +02:00
Author
Owner
Copy Link

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

This is a documentation issue, not an implementation issue. There are no code-level security vulnerabilities to flag in ARCHITECTURE.md itself. My concern is what the document should say about security — and what it must not accidentally expose.

  • The issue lists @RequirePermission, the Permission enum, and AOP enforcement as content for Section 5. This is correct and necessary — the permission system is an architectural decision that Tobias needs to understand to avoid bypassing it when sketching a feature.
  • The existing docs/security-guide.md is already in the repo. Section 5 of the architecture doc should reference it rather than repeating its content. Duplication means two files to maintain and the risk that they drift out of sync.
  • The Basic Auth token is stored as a cookie (auth_token, httpOnly, SameSite=strict, maxAge=86400). The C4 container diagram calls it "Basic Auth token" which is technically accurate but omits the httpOnly/SameSite properties. These matter architecturally — they are why CSRF is disabled. The authentication section should state: "Sessions are implemented as an httpOnly, SameSite=strict cookie containing the Base64-encoded Basic Auth token. CSRF protection is disabled because this cookie configuration structurally prevents cross-origin credential theft."
  • The issue says "No implementation details (code, file paths, line numbers)" — but architectural security decisions require enough specificity to be auditable. "AOP enforcement via PermissionAspect" is the right level: it tells Tobias where enforcement happens without exposing the implementation.
  • The OCR service's security model needs one sentence: it is only accessible from within the Docker network (no external port), and requests to it originate from the backend only. This is an architectural boundary that prevents direct client access to the OCR service.

Recommendations

  • Add one paragraph on the security model to Section 5. Cover: (1) authentication mechanism (Basic Auth token in httpOnly cookie), (2) permission system (@RequirePermission + PermissionAspect + Permission enum), (3) why CSRF is disabled (not "we disabled it" but "the cookie configuration makes it structurally impossible"). Three sentences, no code.
  • State the OCR service network boundary explicitly. "The OCR service is accessible only within the Docker Compose internal network. No external port is exposed. The backend is the only caller." This is a security architecture decision that belongs in the document.
  • Reference docs/security-guide.md rather than duplicating it. One line: "For a full security audit reference, see docs/security-guide.md." Tobias doesn't need to know rate limiting implementation details — just that a security guide exists.
  • Do not include default credentials or environment variable names in the architecture doc. The issue doesn't ask for them, but they sometimes creep in when documenting "how to run locally." That belongs in the README bootstrap section, not in the architecture document.

No open decisions from a security perspective — the recommendations above are all concrete.

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations This is a documentation issue, not an implementation issue. There are no code-level security vulnerabilities to flag in `ARCHITECTURE.md` itself. My concern is what the document should say about security — and what it must not accidentally expose. - The issue lists `@RequirePermission`, the `Permission` enum, and AOP enforcement as content for Section 5. This is correct and necessary — the permission system is an architectural decision that Tobias needs to understand to avoid bypassing it when sketching a feature. - The existing `docs/security-guide.md` is already in the repo. Section 5 of the architecture doc should reference it rather than repeating its content. Duplication means two files to maintain and the risk that they drift out of sync. - The Basic Auth token is stored as a cookie (`auth_token`, httpOnly, SameSite=strict, maxAge=86400). The C4 container diagram calls it "Basic Auth token" which is technically accurate but omits the httpOnly/SameSite properties. These matter architecturally — they are why CSRF is disabled. The authentication section should state: "Sessions are implemented as an httpOnly, SameSite=strict cookie containing the Base64-encoded Basic Auth token. CSRF protection is disabled because this cookie configuration structurally prevents cross-origin credential theft." - The issue says "No implementation details (code, file paths, line numbers)" — but architectural security decisions require enough specificity to be auditable. "AOP enforcement via `PermissionAspect`" is the right level: it tells Tobias where enforcement happens without exposing the implementation. - The OCR service's security model needs one sentence: it is only accessible from within the Docker network (no external port), and requests to it originate from the backend only. This is an architectural boundary that prevents direct client access to the OCR service. ### Recommendations - **Add one paragraph on the security model to Section 5.** Cover: (1) authentication mechanism (Basic Auth token in httpOnly cookie), (2) permission system (`@RequirePermission` + `PermissionAspect` + `Permission` enum), (3) why CSRF is disabled (not "we disabled it" but "the cookie configuration makes it structurally impossible"). Three sentences, no code. - **State the OCR service network boundary explicitly.** "The OCR service is accessible only within the Docker Compose internal network. No external port is exposed. The backend is the only caller." This is a security architecture decision that belongs in the document. - **Reference `docs/security-guide.md` rather than duplicating it.** One line: "For a full security audit reference, see `docs/security-guide.md`." Tobias doesn't need to know rate limiting implementation details — just that a security guide exists. - **Do not include default credentials or environment variable names in the architecture doc.** The issue doesn't ask for them, but they sometimes creep in when documenting "how to run locally." That belongs in the README bootstrap section, not in the architecture document. No open decisions from a security perspective — the recommendations above are all concrete.
marcel commented 2026-05-04 21:05:20 +02:00
Author
Owner
Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

This is a documentation deliverable, not a code feature, so the test strategy question is: how do we verify that docs/ARCHITECTURE.md actually achieves its acceptance criteria?

The issue has five acceptance criteria:

  1. File exists with all 6 sections
  2. Diagram is reproducible from memory (Tobias-test)
  3. Every canonical-set domain is named and one-paragraph defined
  4. Stack-symmetry principle is stated as a rule
  5. All ADRs from existing CLAUDE.md files are migrated

Criteria 1, 3, 4, and 5 are grep-testable. Criterion 2 ("Tobias-test") is a human judgment call that has no automated test equivalent — it requires a real reader.

Looking at the rubric checks this issue addresses: C1.3 (ARCHITECTURE.md with diagram, Tobias can reproduce), C3.1 (domains named), C3.2 (per-domain definitions), C3.6 (shared/ justification). All four are in the Critical/Major range.

The issue dependency on AUDIT-2 through AUDIT-5 (#389–#392) is soft — the doc can be drafted before audits land and refined after. But if the audits find that the canonical domain set described in the doc doesn't match the current code, the doc will need a revision pass.

Recommendations

  • Add a verification step to the PR checklist. Before merging, the author should do a manual "Tobias test": hand the doc to someone (or read it cold after 24 hours) and try to sketch the architecture on paper. If the sketch is wrong, revise the diagram. This is the only meaningful test for C1.3.
  • Make criteria 3 and 5 grep-testable in the PR description. List every canonical domain name and confirm each has a paragraph. List all 6 ADR numbers and confirm each appears as a summary in Section 5. This takes 5 minutes and turns a vague "did you include everything?" into a checklist.
  • Flag the soft dependency clearly. If the audits (#389–#392) are not yet complete when this PR is opened, the PR description should say: "Section 2 domain definitions will be updated after audits land — tracking via [issue link]." An architecture doc that silently describes an incorrect domain boundary is worse than an honest "to be refined."
  • The Definition of Done says "committed on main" — confirm the PR does not include the audit reports. The issue scope is docs/ARCHITECTURE.md only. A PR that bundles audit report content with the architecture doc makes review harder. One file, one PR.
  • Verify the 400-line limit before merge. Run wc -l docs/ARCHITECTURE.md in the PR. Over 400 lines is a scope failure, not a style preference.

No open decisions — all recommendations are concrete verification steps.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations This is a documentation deliverable, not a code feature, so the test strategy question is: **how do we verify that `docs/ARCHITECTURE.md` actually achieves its acceptance criteria?** The issue has five acceptance criteria: 1. File exists with all 6 sections 2. Diagram is reproducible from memory (Tobias-test) 3. Every canonical-set domain is named and one-paragraph defined 4. Stack-symmetry principle is stated as a rule 5. All ADRs from existing CLAUDE.md files are migrated Criteria 1, 3, 4, and 5 are grep-testable. Criterion 2 ("Tobias-test") is a human judgment call that has no automated test equivalent — it requires a real reader. Looking at the rubric checks this issue addresses: C1.3 (ARCHITECTURE.md with diagram, Tobias can reproduce), C3.1 (domains named), C3.2 (per-domain definitions), C3.6 (shared/ justification). All four are in the Critical/Major range. The issue dependency on AUDIT-2 through AUDIT-5 (#389–#392) is soft — the doc can be drafted before audits land and refined after. But if the audits find that the canonical domain set described in the doc doesn't match the current code, the doc will need a revision pass. ### Recommendations - **Add a verification step to the PR checklist.** Before merging, the author should do a manual "Tobias test": hand the doc to someone (or read it cold after 24 hours) and try to sketch the architecture on paper. If the sketch is wrong, revise the diagram. This is the only meaningful test for C1.3. - **Make criteria 3 and 5 grep-testable in the PR description.** List every canonical domain name and confirm each has a paragraph. List all 6 ADR numbers and confirm each appears as a summary in Section 5. This takes 5 minutes and turns a vague "did you include everything?" into a checklist. - **Flag the soft dependency clearly.** If the audits (#389–#392) are not yet complete when this PR is opened, the PR description should say: "Section 2 domain definitions will be updated after audits land — tracking via [issue link]." An architecture doc that silently describes an incorrect domain boundary is worse than an honest "to be refined." - **The Definition of Done says "committed on main" — confirm the PR does not include the audit reports.** The issue scope is `docs/ARCHITECTURE.md` only. A PR that bundles audit report content with the architecture doc makes review harder. One file, one PR. - **Verify the 400-line limit before merge.** Run `wc -l docs/ARCHITECTURE.md` in the PR. Over 400 lines is a scope failure, not a style preference. No open decisions — all recommendations are concrete verification steps.
marcel commented 2026-05-04 21:05:48 +02:00
Author
Owner
Copy Link

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

This issue is about writing a Markdown document, so my usual concerns about touch targets, contrast ratios, and responsive layouts don't directly apply. What I do care about is the information architecture and readability of the document — the UX of the documentation itself.

  • The six-section structure in the issue is well-ordered. It follows the "general-to-specific" pattern: diagram first (spatial), then domain vocabulary, then layers, then principle, then decisions, then flows. That is exactly how a new reader builds a mental model. No changes needed to the section order.
  • The issue mentions "Mermaid preferred if rendered" for the diagram. Gitea renders Mermaid in Markdown — confirmed by the existing docs/architecture/c4-diagrams.md. Use Mermaid. ASCII art forces readers to mentally parse spatial relationships that a rendered diagram communicates instantly.
  • The domain list in Section 2 is the highest-density section: 7 Tier-1 domains + 2 Tier-2 + 11 cross-cutting members = 20 concepts. A wall of paragraphs will lose Tobias. The UX of this section matters.
  • The issue says "one paragraph" per Tier-1 domain and "explain the derived domain concept" for Tier-2. That structure works. But the cross-cutting list in Section 3 needs a different treatment — 11 one-sentence entries work better as a table (Name | What it does | Admission criteria met) than as 11 bullet points that blur together.

Recommendations

  • Use a Mermaid diagram in Section 1, not ASCII. The existing docs/architecture/c4-diagrams.md has the C4 L2 diagram. Update it to add ocr-service, the SSE arrow, and the presigned-URL path. Then embed or link it from Section 1. Do not create a second diagram file.
  • Section 2 Tier-1 domains: use a consistent paragraph template. Each paragraph should answer the same three questions in the same order: (1) What entities/data does it own? (2) What does it NOT own? (3) What are its cross-domain dependencies? Consistent structure makes the section scannable. Tobias's eye can jump to "does NOT own" for any domain in 2 seconds.
  • Section 3 cross-cutting: use a table, not a paragraph per member. Columns: Member | One sentence | Admission criteria (a/b/c). 11 rows at ~1 line each = 12 lines including header. A paragraph per member would be ~44 lines. The table format saves 32 lines toward the 400-line budget and is easier to scan.
  • Section 6 walkthroughs: write them as numbered steps, not prose paragraphs. "1. User submits form → 2. SvelteKit SSR sends multipart PUT → 3. PermissionAspect checks WRITE_ALL → 4. DocumentService calls FileService → 5. FileService calls MinIO via S3 API → 6. Backend updates document status to UPLOADED" is more memorable than a paragraph because it maps to the mental model of a sequence. The issue says "1 paragraph each" but a numbered list achieves the same goal more memorably. Tobias can redraw a numbered list as arrows on a whiteboard.
  • Add a one-line "last updated" note at the top. Architecture docs decay. A simple <!-- Last reviewed: YYYY-MM-DD, against commit SHA --> at the top of the file tells future readers whether the doc is likely to still be accurate. This is a documentation hygiene convention, not code.

No open decisions from a UX perspective — all recommendations are concrete.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations This issue is about writing a Markdown document, so my usual concerns about touch targets, contrast ratios, and responsive layouts don't directly apply. What I do care about is the **information architecture and readability** of the document — the UX of the documentation itself. - The six-section structure in the issue is well-ordered. It follows the "general-to-specific" pattern: diagram first (spatial), then domain vocabulary, then layers, then principle, then decisions, then flows. That is exactly how a new reader builds a mental model. No changes needed to the section order. - The issue mentions "Mermaid preferred if rendered" for the diagram. Gitea renders Mermaid in Markdown — confirmed by the existing `docs/architecture/c4-diagrams.md`. Use Mermaid. ASCII art forces readers to mentally parse spatial relationships that a rendered diagram communicates instantly. - The domain list in Section 2 is the highest-density section: 7 Tier-1 domains + 2 Tier-2 + 11 cross-cutting members = 20 concepts. A wall of paragraphs will lose Tobias. The UX of this section matters. - The issue says "one paragraph" per Tier-1 domain and "explain the derived domain concept" for Tier-2. That structure works. But the cross-cutting list in Section 3 needs a different treatment — 11 one-sentence entries work better as a **table** (Name | What it does | Admission criteria met) than as 11 bullet points that blur together. ### Recommendations - **Use a Mermaid diagram in Section 1, not ASCII.** The existing `docs/architecture/c4-diagrams.md` has the C4 L2 diagram. Update it to add `ocr-service`, the SSE arrow, and the presigned-URL path. Then embed or link it from Section 1. Do not create a second diagram file. - **Section 2 Tier-1 domains: use a consistent paragraph template.** Each paragraph should answer the same three questions in the same order: (1) What entities/data does it own? (2) What does it NOT own? (3) What are its cross-domain dependencies? Consistent structure makes the section scannable. Tobias's eye can jump to "does NOT own" for any domain in 2 seconds. - **Section 3 cross-cutting: use a table, not a paragraph per member.** Columns: `Member | One sentence | Admission criteria (a/b/c)`. 11 rows at ~1 line each = 12 lines including header. A paragraph per member would be ~44 lines. The table format saves 32 lines toward the 400-line budget and is easier to scan. - **Section 6 walkthroughs: write them as numbered steps, not prose paragraphs.** "1. User submits form → 2. SvelteKit SSR sends multipart PUT → 3. PermissionAspect checks WRITE_ALL → 4. DocumentService calls FileService → 5. FileService calls MinIO via S3 API → 6. Backend updates document status to UPLOADED" is more memorable than a paragraph because it maps to the mental model of a sequence. The issue says "1 paragraph each" but a numbered list achieves the same goal more memorably. Tobias can redraw a numbered list as arrows on a whiteboard. - **Add a one-line "last updated" note at the top.** Architecture docs decay. A simple `<!-- Last reviewed: YYYY-MM-DD, against commit SHA -->` at the top of the file tells future readers whether the doc is likely to still be accurate. This is a documentation hygiene convention, not code. No open decisions from a UX perspective — all recommendations are concrete.
marcel commented 2026-05-04 21:06:17 +02:00
Author
Owner
Copy Link

⚙️ Tobias Wendt — DevOps & Platform Engineer

Observations

I'm the named "target reader" for this document — the issue literally says "Tobias can reproduce the diagram from memory." So I'm reviewing both the deliverable requirements and the infrastructure/deployment side of what needs to be documented.

  • Section 1 diagram: the existing docs/architecture/c4-diagrams.md shows the L2 container view but is missing the OCR service. The docker-compose.yml orchestrates: frontend, backend, db, minio, ocr-service, and the mc init container. The architecture doc should describe all six containers. Tobias looking at the Compose file and the architecture diagram should see the same boxes.
  • The issue lists "SSE notifications" as an arrow between subsystems. From the code, SseEmitterRegistry.java handles server-sent events from the backend. The SvelteKit frontend connects to the backend SSE endpoint directly. This is a backend → browser path, not backend → frontend SSR. The diagram should show this as a direct arrow from backend to browser, not backend → frontend → browser.
  • The issue mentions "MinIO presigned URLs to OCR" as an external boundary. This is correct: the OCR service fetches PDF files from MinIO using a presigned URL that the backend generates and passes in the OCR request. That arrow (ocr-service → minio, "presigned URL fetch") is currently missing from c4-diagrams.md.
  • Section 6, walkthrough 1 ("document upload") already exists as a sequence diagram in docs/architecture/c4-diagrams.md. That diagram doesn't include the OCR queue step. The new walkthrough should either update that diagram or be written as a numbered prose list that extends the existing diagram's description.
  • The issue says "Tobias is the second reader after README." That means the architecture doc will be read before docs/infrastructure/ — so it should reference, but not duplicate, the deployment documentation.

Recommendations

  • Update the existing L2 Mermaid diagram in docs/architecture/c4-diagrams.md to add: (1) ocr-service container with Python FastAPI / port 8000, (2) Rel(backend, ocr-service, "OCR requests", "HTTP / REST / JSON"), (3) Rel(ocr-service, storage, "Fetch PDF via presigned URL", "HTTP / S3 presigned"), (4) Rel(backend, user, "SSE notifications", "HTTP / SSE"). Then embed or reference this updated diagram in Section 1 of ARCHITECTURE.md.
  • State the single-node OCR constraint in Section 5 (not just in ADR-001). "The OCR service runs as a single container because training reloads the model in-process. Multiple replicas would cause model-state divergence." This is the most operationally significant architectural constraint I need to know as the person who might one day be asked to scale the service.
  • Reference docs/infrastructure/ for deployment details. Section 6 walkthroughs should end with: "For production deployment, see docs/infrastructure/production-compose.md." The architecture doc describes the what; the infra docs describe the how to operate.
  • The "presigned URL" flow between backend, MinIO, and OCR service needs one sentence in the doc. "The backend generates a MinIO presigned URL and passes it to the OCR service in the request body. The OCR service fetches the PDF directly from MinIO over the internal Docker network, without the PDF data passing through the backend." This is a bandwidth and security decision that is non-obvious and worth stating explicitly.

No open decisions from my angle — all gaps have clear answers.

## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Observations I'm the named "target reader" for this document — the issue literally says "Tobias can reproduce the diagram from memory." So I'm reviewing both the deliverable requirements and the infrastructure/deployment side of what needs to be documented. - Section 1 diagram: the existing `docs/architecture/c4-diagrams.md` shows the L2 container view but is **missing the OCR service**. The docker-compose.yml orchestrates: `frontend`, `backend`, `db`, `minio`, `ocr-service`, and the `mc` init container. The architecture doc should describe all six containers. Tobias looking at the Compose file and the architecture diagram should see the same boxes. - The issue lists "SSE notifications" as an arrow between subsystems. From the code, `SseEmitterRegistry.java` handles server-sent events from the backend. The SvelteKit frontend connects to the backend SSE endpoint directly. This is a backend → browser path, not backend → frontend SSR. The diagram should show this as a direct arrow from `backend` to `browser`, not `backend → frontend → browser`. - The issue mentions "MinIO presigned URLs to OCR" as an external boundary. This is correct: the OCR service fetches PDF files from MinIO using a presigned URL that the backend generates and passes in the OCR request. That arrow (`ocr-service → minio`, "presigned URL fetch") is currently missing from `c4-diagrams.md`. - Section 6, walkthrough 1 ("document upload") already exists as a sequence diagram in `docs/architecture/c4-diagrams.md`. That diagram doesn't include the OCR queue step. The new walkthrough should either update that diagram or be written as a numbered prose list that extends the existing diagram's description. - The issue says "Tobias is the second reader after README." That means the architecture doc will be read before `docs/infrastructure/` — so it should reference, but not duplicate, the deployment documentation. ### Recommendations - **Update the existing L2 Mermaid diagram** in `docs/architecture/c4-diagrams.md` to add: (1) `ocr-service` container with `Python FastAPI / port 8000`, (2) `Rel(backend, ocr-service, "OCR requests", "HTTP / REST / JSON")`, (3) `Rel(ocr-service, storage, "Fetch PDF via presigned URL", "HTTP / S3 presigned")`, (4) `Rel(backend, user, "SSE notifications", "HTTP / SSE")`. Then embed or reference this updated diagram in Section 1 of `ARCHITECTURE.md`. - **State the single-node OCR constraint in Section 5 (not just in ADR-001).** "The OCR service runs as a single container because training reloads the model in-process. Multiple replicas would cause model-state divergence." This is the most operationally significant architectural constraint I need to know as the person who might one day be asked to scale the service. - **Reference `docs/infrastructure/` for deployment details.** Section 6 walkthroughs should end with: "For production deployment, see `docs/infrastructure/production-compose.md`." The architecture doc describes the *what*; the infra docs describe the *how to operate*. - **The "presigned URL" flow between backend, MinIO, and OCR service needs one sentence in the doc.** "The backend generates a MinIO presigned URL and passes it to the OCR service in the request body. The OCR service fetches the PDF directly from MinIO over the internal Docker network, without the PDF data passing through the backend." This is a bandwidth and security decision that is non-obvious and worth stating explicitly. No open decisions from my angle — all gaps have clear answers.
marcel commented 2026-05-04 21:06:41 +02:00
Author
Owner
Copy Link

📋 Elicit — Requirements Engineer

Observations

  • The issue is well-specified by this project's standards. The "Required content" section maps to clear acceptance criteria. The anti-patterns section correctly identifies the doc vs. API-reference scope boundary. The 400-line constraint is measurable. The Tobias-test is a valid acceptance criterion, though it requires a human reviewer.
  • One ambiguity: Section 2 says "List every domain from the Canonical Domain Set in #387 first comment." The canonical domain set in #387's first comment includes 7 Tier-1, 2 Tier-2, and 11 cross-cutting members — a total of 20 concepts. The instruction "one paragraph per Tier-1 domain" × 7 = potentially 7 paragraphs, plus Tier-2 explanation, plus Section 3 (cross-cutting). That is a lot of the 400-line budget. There is a tension between completeness and the line limit.
  • One missing acceptance criterion: The issue says "All ADRs from existing CLAUDE.md files are migrated here in human-readable form." But there are six ADRs in docs/adr/ (001–006) and additional architectural rules in backend/CLAUDE.md and ocr-service/CLAUDE.md that are not formal ADRs. The AC says "CLAUDE.md files" — should every rule in every CLAUDE.md file be migrated to ARCHITECTURE.md? That would produce a very long doc. Or should only decisions (not conventions) be migrated?
  • The soft dependency wording is slightly ambiguous: "Can be drafted in parallel and refined when audits land." This implies the doc will need a revision after audits land. If #389–#392 have not landed by the time this PR is opened, the PR should explicitly be marked as draft/pending-refinement, not merged as complete. The Definition of Done ("committed on main") may be premature without audit findings.
  • The "Section 4: Stack-symmetry principle" describes a target state that does not match the current backend package structure (which is layer-first). The acceptance criterion says "Stack-symmetry principle is stated as a rule" — but a rule describing something that isn't true yet needs a qualifier, or the document will be misleading for Anja (who will scan the actual package structure and find it contradicts the doc).

Recommendations

  • Clarify the "CLAUDE.md migration" scope. Recommend limiting Section 5 to: (a) all six formal ADRs in docs/adr/ summarised in 2–3 sentences each, (b) the layering rule (controller → service → repository), (c) the single-node OCR constraint, and (d) the permission system. Everything else in CLAUDE.md is convention, not an architectural decision — conventions belong in a separate conventions doc (DOC-3 or DOC-5), not in ARCHITECTURE.md.
  • Add a qualifier to the stack-symmetry principle. Section 4 should state: "Target state: domain names are identical across backend/src/main/java/.../ and frontend/src/lib/. The backend is currently organised by layer; the refactor epic will restructure it to match this convention." Without this qualifier, Anja opens the backend, sees controller/DocumentController.java, and concludes the doc is wrong.
  • The Tobias-test acceptance criterion needs a named reviewer. "Tobias can reproduce the diagram" — but Tobias is a persona, not a person. The PR review process should include one reviewer who reads the doc cold (after a 24-hour break) and tries to sketch the architecture. This should be noted in the PR description, not just implied.
  • Split the 400-line concern proactively. The issue says "if you need more, split into per-domain READMEs (DOC-6)." Given 20 concepts to cover, the risk of overrun is real. Recommend: use the table format for cross-cutting (Section 3), use 3-sentence max per domain (Section 2), and save prose only for the two walkthroughs (Section 6) and the ADR summaries (Section 5). Budget allocation: diagram 15 lines, domains 90 lines, cross-cutting 15 lines, symmetry 8 lines, ADRs 80 lines, walkthroughs 40 lines = 248 lines. That leaves 150 lines of headroom.

Open Decisions

  • Should Section 5 include all CLAUDE.md rules, or only formal ADRs and explicitly architectural decisions? Option A (all CLAUDE.md rules): comprehensive but risks duplicating convention docs and pushing past 400 lines. Option B (ADRs + 3 key architectural decisions): concise, maintainable, and clearly scoped. Recommend Option B — the full CLAUDE.md rules should become a CONTRIBUTING.md or conventions doc, not be embedded in the architecture doc. (Decision needed before authoring starts.)
## 📋 Elicit — Requirements Engineer ### Observations - The issue is well-specified by this project's standards. The "Required content" section maps to clear acceptance criteria. The anti-patterns section correctly identifies the doc vs. API-reference scope boundary. The 400-line constraint is measurable. The Tobias-test is a valid acceptance criterion, though it requires a human reviewer. - **One ambiguity**: Section 2 says "List every domain from the Canonical Domain Set in #387 first comment." The canonical domain set in #387's first comment includes 7 Tier-1, 2 Tier-2, and 11 cross-cutting members — a total of 20 concepts. The instruction "one paragraph per Tier-1 domain" × 7 = potentially 7 paragraphs, plus Tier-2 explanation, plus Section 3 (cross-cutting). That is a lot of the 400-line budget. There is a tension between completeness and the line limit. - **One missing acceptance criterion**: The issue says "All ADRs from existing CLAUDE.md files are migrated here in human-readable form." But there are **six ADRs** in `docs/adr/` (001–006) and additional architectural rules in `backend/CLAUDE.md` and `ocr-service/CLAUDE.md` that are not formal ADRs. The AC says "CLAUDE.md files" — should every rule in every CLAUDE.md file be migrated to `ARCHITECTURE.md`? That would produce a very long doc. Or should only *decisions* (not conventions) be migrated? - **The soft dependency wording is slightly ambiguous**: "Can be drafted in parallel and refined when audits land." This implies the doc will need a revision after audits land. If #389–#392 have not landed by the time this PR is opened, the PR should explicitly be marked as draft/pending-refinement, not merged as complete. The Definition of Done ("committed on main") may be premature without audit findings. - The "Section 4: Stack-symmetry principle" describes a *target state* that does not match the current backend package structure (which is layer-first). The acceptance criterion says "Stack-symmetry principle is stated as a rule" — but a rule describing something that isn't true yet needs a qualifier, or the document will be misleading for Anja (who will scan the actual package structure and find it contradicts the doc). ### Recommendations - **Clarify the "CLAUDE.md migration" scope.** Recommend limiting Section 5 to: (a) all six formal ADRs in `docs/adr/` summarised in 2–3 sentences each, (b) the layering rule (controller → service → repository), (c) the single-node OCR constraint, and (d) the permission system. Everything else in CLAUDE.md is *convention*, not an *architectural decision* — conventions belong in a separate conventions doc (DOC-3 or DOC-5), not in ARCHITECTURE.md. - **Add a qualifier to the stack-symmetry principle.** Section 4 should state: "Target state: domain names are identical across `backend/src/main/java/.../` and `frontend/src/lib/`. The backend is currently organised by layer; the refactor epic will restructure it to match this convention." Without this qualifier, Anja opens the backend, sees `controller/DocumentController.java`, and concludes the doc is wrong. - **The Tobias-test acceptance criterion needs a named reviewer.** "Tobias can reproduce the diagram" — but Tobias is a persona, not a person. The PR review process should include one reviewer who reads the doc cold (after a 24-hour break) and tries to sketch the architecture. This should be noted in the PR description, not just implied. - **Split the 400-line concern proactively.** The issue says "if you need more, split into per-domain READMEs (DOC-6)." Given 20 concepts to cover, the risk of overrun is real. Recommend: use the table format for cross-cutting (Section 3), use 3-sentence max per domain (Section 2), and save prose only for the two walkthroughs (Section 6) and the ADR summaries (Section 5). Budget allocation: diagram 15 lines, domains 90 lines, cross-cutting 15 lines, symmetry 8 lines, ADRs 80 lines, walkthroughs 40 lines = 248 lines. That leaves 150 lines of headroom. ### Open Decisions - **Should Section 5 include all CLAUDE.md rules, or only formal ADRs and explicitly architectural decisions?** Option A (all CLAUDE.md rules): comprehensive but risks duplicating convention docs and pushing past 400 lines. Option B (ADRs + 3 key architectural decisions): concise, maintainable, and clearly scoped. Recommend Option B — the full CLAUDE.md rules should become a `CONTRIBUTING.md` or conventions doc, not be embedded in the architecture doc. _(Decision needed before authoring starts.)_
marcel commented 2026-05-04 21:07:15 +02:00
Author
Owner
Copy Link

🗳️ Decision Queue — Action Required

3 decisions need your input before implementation starts.

Architecture / Scope

  • Current state vs. target state in Sections 2 and 4 — The backend is still layer-first (controller/, service/, model/), but the doc describes a domain-first structure. Option A: describe current state and label the target as aspirational. Option B: describe only the target and note "not yet implemented." Option A is recommended — a discrepancy between doc and code is worse than a clearly labelled aspiration. Without this choice, the doc will either mislead Anja (who opens the backend and sees layer packages) or confuse Tobias (who sees a target he can't verify). (Raised by: Markus, Felix, Elicit)

  • Section 5 scope: all CLAUDE.md rules vs. ADRs + key architectural decisions only — Option A (all CLAUDE.md rules): comprehensive but risks exceeding 400 lines and duplicating a future conventions doc. Option B (ADRs + 3 key architectural decisions — layering rule, permission system, single-node OCR): concise, maintainable, and clearly scoped. Recommend Option B. Everything else in CLAUDE.md (entity code style, DTO conventions, error handling patterns) belongs in a CONTRIBUTING.md or conventions doc, not in ARCHITECTURE.md. (Raised by: Markus, Elicit)

Scope / Detail Level

  • OCR domain depth in Section 6 — The OCR pipeline has five service classes (OcrService, OcrBatchService, OcrAsyncRunner, OcrClient, OcrProgressService). Option A: one paragraph in Section 6 at the conceptual level ("backend queues the OCR job, OCR service fetches PDF via presigned URL, streams results back"). Option B: full pipeline walkthrough, which will consume significant line budget and blur with the domain README scope (DOC-6). Recommend Option A — keep Section 6 conceptual; the per-domain OCR README (DOC-6) is the right place for the full pipeline. (Raised by: Felix)
## 🗳️ Decision Queue — Action Required _3 decisions need your input before implementation starts._ ### Architecture / Scope - **Current state vs. target state in Sections 2 and 4** — The backend is still layer-first (`controller/`, `service/`, `model/`), but the doc describes a domain-first structure. Option A: describe current state and label the target as aspirational. Option B: describe only the target and note "not yet implemented." Option A is recommended — a discrepancy between doc and code is worse than a clearly labelled aspiration. Without this choice, the doc will either mislead Anja (who opens the backend and sees layer packages) or confuse Tobias (who sees a target he can't verify). _(Raised by: Markus, Felix, Elicit)_ - **Section 5 scope: all CLAUDE.md rules vs. ADRs + key architectural decisions only** — Option A (all CLAUDE.md rules): comprehensive but risks exceeding 400 lines and duplicating a future conventions doc. Option B (ADRs + 3 key architectural decisions — layering rule, permission system, single-node OCR): concise, maintainable, and clearly scoped. Recommend Option B. Everything else in CLAUDE.md (entity code style, DTO conventions, error handling patterns) belongs in a `CONTRIBUTING.md` or conventions doc, not in `ARCHITECTURE.md`. _(Raised by: Markus, Elicit)_ ### Scope / Detail Level - **OCR domain depth in Section 6** — The OCR pipeline has five service classes (`OcrService`, `OcrBatchService`, `OcrAsyncRunner`, `OcrClient`, `OcrProgressService`). Option A: one paragraph in Section 6 at the conceptual level ("backend queues the OCR job, OCR service fetches PDF via presigned URL, streams results back"). Option B: full pipeline walkthrough, which will consume significant line budget and blur with the domain README scope (DOC-6). Recommend Option A — keep Section 6 conceptual; the per-domain OCR README (DOC-6) is the right place for the full pipeline. _(Raised by: Felix)_
marcel commented 2026-05-05 21:16:46 +02:00
Author
Owner
Copy Link

✅ Decision Queue — Resolved

The 3 decisions raised in #396#issuecomment-6331:

1. Current vs target state in Sections 2 and 4 → describe current state + label target as aspirational (Option A)

Markus, Felix, and Elicit all converge on this. A discrepancy between doc and code is worse than a clearly labelled aspiration. Anja will scan the actual backend package structure on day one — if the doc says document/DocumentService.java and the file lives at service/DocumentService.java, the doc is misleading.

Implementation pattern:

  • Section 2 (domains): describe each domain by what entities/data it owns today. Use parenthetical notes like "(currently in model/Document.java; will move to document/ after the Refactor epic — see #387)" where the location differs from the target.
  • Section 4 (stack-symmetry): state the target rule, then immediately add: "The backend is currently organised by layer (controller/, service/, model/, repository/); the Refactor epic restructures it to match this convention."

2. Section 5 scope → ADRs + 3 key architectural decisions only (Option B)

Section 5 covers:

  • Summaries of all six ADRs (docs/adr/001..006) — 2–3 sentences each, link to the full ADR for the why.
  • The layering rule (Controller → Service → Repository; no service touches another domain's repo).
  • The single-node OCR constraint (per ADR-001).
  • The permission system (@RequirePermission + PermissionAspect + Permission enum).

Everything else in CLAUDE.md (entity Lombok template, DTO conventions, error-handling patterns, Svelte 5 patterns) is convention, not architectural decision — those go to CONTRIBUTING.md (DOC-4 / #398), not here. This caps Section 5 around ~80 lines per Felix's budget.

3. OCR domain depth in Section 6 → one paragraph conceptual (Option A)

Section 6's OCR walkthrough is conceptual: "The backend queues an OCR job, generates a MinIO presigned URL, and calls the OCR service. The OCR service fetches the PDF directly from MinIO over the internal Docker network and streams transcription blocks back. The backend persists blocks and broadcasts SSE notifications." That level of naming services is enough.

The full five-class pipeline (OcrService, OcrBatchService, OcrAsyncRunner, OcrClient, OcrProgressService) belongs in backend/.../ocr/README.md and ocr-service/README.md (both DOC-6 / #400).

📌 Additional persona feedback to fold into implementation

  • Markus: scope the audience explicitly at the top: "Target reader: a PM-with-CS background who has read the README. Goal: accurate mental model after one read."
  • Markus + Tobias: Section 1 — update the existing docs/architecture/c4-diagrams.md Mermaid L2 diagram (do not redraw as ASCII). Add: ocr-service container (Python FastAPI / port 8000), Rel(backend, ocr-service, "OCR requests", HTTP/REST/JSON), Rel(ocr-service, storage, "Fetch PDF via presigned URL", HTTP/S3 presigned), Rel(backend, user, "SSE notifications", HTTP/SSE). Embed/link from Section 1.
  • Leonie: Section 3 (cross-cutting) as a table (Member | One sentence | Admission criteria a/b/c), not paragraph-per-member — saves ~30 lines.
  • Leonie: Section 6 walkthroughs as numbered steps, not paragraphs — Tobias can redraw a numbered list as arrows on a whiteboard.
  • Nora: Section 5 includes one paragraph on the security model — auth (Basic Auth in httpOnly+SameSite=strict cookie), why CSRF is disabled (cookie config makes it structurally impossible), permission enforcement (@RequirePermission + PermissionAspect). Reference docs/security-guide.md; do not duplicate it.
  • Nora + Tobias: Section 1 must state OCR network boundary: "The OCR service is reachable only on the internal Docker network. No external port. Only the backend calls it."
  • Sara: PR checklist — wc -l docs/ARCHITECTURE.md < 400; named human reviewer for the Tobias-test (read cold after 24h, sketch architecture). Mark forward-deps to AUDIT-2..5 (#389-#392) explicitly if those haven't landed.
  • Felix: budget allocation — diagram 15 / domains 90 / cross-cutting 15 / symmetry 8 / ADRs 80 / walkthroughs 40 = ~248 lines. ~150 lines headroom in the 400-line cap.
  • Leonie: add <!-- Last reviewed: YYYY-MM-DD against commit SHA --> at the top.

Status: Ready for implementation.

## ✅ Decision Queue — Resolved The 3 decisions raised in [#396#issuecomment-6331](http://heim-nas:3005/marcel/familienarchiv/issues/396#issuecomment-6331): ### 1. Current vs target state in Sections 2 and 4 → **describe current state + label target as aspirational (Option A)** Markus, Felix, and Elicit all converge on this. A discrepancy between doc and code is worse than a clearly labelled aspiration. Anja will scan the actual backend package structure on day one — if the doc says `document/DocumentService.java` and the file lives at `service/DocumentService.java`, the doc is misleading. **Implementation pattern:** - Section 2 (domains): describe each domain by what entities/data it owns *today*. Use parenthetical notes like "_(currently in `model/Document.java`; will move to `document/` after the Refactor epic — see #387)_" where the location differs from the target. - Section 4 (stack-symmetry): state the target rule, then immediately add: "_The backend is currently organised by layer (`controller/`, `service/`, `model/`, `repository/`); the Refactor epic restructures it to match this convention._" ### 2. Section 5 scope → **ADRs + 3 key architectural decisions only (Option B)** Section 5 covers: - Summaries of all six ADRs (`docs/adr/001..006`) — 2–3 sentences each, link to the full ADR for the why. - The layering rule (Controller → Service → Repository; no service touches another domain's repo). - The single-node OCR constraint (per ADR-001). - The permission system (`@RequirePermission` + `PermissionAspect` + `Permission` enum). Everything else in CLAUDE.md (entity Lombok template, DTO conventions, error-handling patterns, Svelte 5 patterns) is **convention**, not architectural decision — those go to `CONTRIBUTING.md` (DOC-4 / #398), not here. This caps Section 5 around ~80 lines per Felix's budget. ### 3. OCR domain depth in Section 6 → **one paragraph conceptual (Option A)** Section 6's OCR walkthrough is conceptual: "_The backend queues an OCR job, generates a MinIO presigned URL, and calls the OCR service. The OCR service fetches the PDF directly from MinIO over the internal Docker network and streams transcription blocks back. The backend persists blocks and broadcasts SSE notifications._" That level of naming services is enough. The full five-class pipeline (`OcrService`, `OcrBatchService`, `OcrAsyncRunner`, `OcrClient`, `OcrProgressService`) belongs in `backend/.../ocr/README.md` and `ocr-service/README.md` (both **DOC-6 / #400**). --- ## 📌 Additional persona feedback to fold into implementation - **Markus:** scope the audience explicitly at the top: "_Target reader: a PM-with-CS background who has read the README. Goal: accurate mental model after one read._" - **Markus + Tobias:** Section 1 — update the existing `docs/architecture/c4-diagrams.md` Mermaid L2 diagram (do not redraw as ASCII). Add: `ocr-service` container (Python FastAPI / port 8000), `Rel(backend, ocr-service, "OCR requests", HTTP/REST/JSON)`, `Rel(ocr-service, storage, "Fetch PDF via presigned URL", HTTP/S3 presigned)`, `Rel(backend, user, "SSE notifications", HTTP/SSE)`. Embed/link from Section 1. - **Leonie:** Section 3 (cross-cutting) as a **table** (`Member | One sentence | Admission criteria a/b/c`), not paragraph-per-member — saves ~30 lines. - **Leonie:** Section 6 walkthroughs as numbered steps, not paragraphs — Tobias can redraw a numbered list as arrows on a whiteboard. - **Nora:** Section 5 includes one paragraph on the security model — auth (Basic Auth in httpOnly+SameSite=strict cookie), why CSRF is disabled (cookie config makes it structurally impossible), permission enforcement (`@RequirePermission` + `PermissionAspect`). Reference `docs/security-guide.md`; do not duplicate it. - **Nora + Tobias:** Section 1 must state OCR network boundary: "_The OCR service is reachable only on the internal Docker network. No external port. Only the backend calls it._" - **Sara:** PR checklist — `wc -l docs/ARCHITECTURE.md` < 400; named human reviewer for the Tobias-test (read cold after 24h, sketch architecture). Mark forward-deps to AUDIT-2..5 (#389-#392) explicitly if those haven't landed. - **Felix:** budget allocation — diagram 15 / domains 90 / cross-cutting 15 / symmetry 8 / ADRs 80 / walkthroughs 40 = ~248 lines. ~150 lines headroom in the 400-line cap. - **Leonie:** add `<!-- Last reviewed: YYYY-MM-DD against commit SHA -->` at the top. **Status:** Ready for implementation.
marcel commented 2026-05-05 22:50:12 +02:00
Author
Owner
Copy Link

DOC-2 implemented — PR #441

What was written:

docs/ARCHITECTURE.md — 146 lines, 6 sections:

  1. High-level diagram reference (links to updated c4-diagrams.md + two invisible facts: OCR network boundary, SSE path)
  2. Domain set — 7 Tier-1 domains with owned entities, exclusions, cross-domain deps; 2 Tier-2 derived domains with definition
  3. Cross-cutting layer table (7 backend packages + frontend shared/) with admission criteria column
  4. Stack-symmetry principle (backend/frontend naming rule, references #408)
  5. Key architectural decisions — ADR-001–006 summaries + layering rule + permission system
  6. Two data-flow walkthroughs: document upload (9 steps) + transcription block autosave (7 steps)

Also updated: docs/architecture/c4-diagrams.md L2 Mermaid diagram — added ocr-service container, SSE notification arrow (backend → browser direct), presigned-URL arrow (ocr → minio).

Commits:

  • aedae2a7 — update c4-diagrams.md L2
  • dc6910a3 — write docs/ARCHITECTURE.md

PR: http://heim-nas:3005/marcel/familienarchiv/pulls/441

## DOC-2 implemented — PR #441 **What was written:** `docs/ARCHITECTURE.md` — 146 lines, 6 sections: 1. High-level diagram reference (links to updated c4-diagrams.md + two invisible facts: OCR network boundary, SSE path) 2. Domain set — 7 Tier-1 domains with owned entities, exclusions, cross-domain deps; 2 Tier-2 derived domains with definition 3. Cross-cutting layer table (7 backend packages + frontend shared/) with admission criteria column 4. Stack-symmetry principle (backend/frontend naming rule, references #408) 5. Key architectural decisions — ADR-001–006 summaries + layering rule + permission system 6. Two data-flow walkthroughs: document upload (9 steps) + transcription block autosave (7 steps) **Also updated:** `docs/architecture/c4-diagrams.md` L2 Mermaid diagram — added ocr-service container, SSE notification arrow (backend → browser direct), presigned-URL arrow (ocr → minio). **Commits:** - `aedae2a7` — update c4-diagrams.md L2 - `dc6910a3` — write docs/ARCHITECTURE.md PR: http://heim-nas:3005/marcel/familienarchiv/pulls/441
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 P0-critical documentation 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#396
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?