feat(transcribe): show visible per-document transcription progress in the panel header #321

Open
opened 2026-04-24 13:23:47 +02:00 by marcel · 8 comments
Owner

Context

The Transcribe panel header currently shows 0 sections as its only progress indicator. Transcribers can't tell:

  • How many regions still need work on this document.
  • Whether they're on the last region or the first.
  • When a document counts as "done" from the transcription perspective.

Meanwhile the document detail card elsewhere in the app shows 100% reviewed as a progress ring — those two signals don't connect. A transcriber could be "100% reviewed" but still have un-transcribed regions, or vice versa.

Motivation loss is the real cost in a volunteer workflow. Visible progress is the cheapest way to keep people moving.

The hard question: how is total-section-count known?

Three options considered:

Option A — segmentation pre-pass on upload

Run the segmentation model (already exists; see /admin/ocr Train segmentation) once per document on upload. Store Document.expectedSectionCount. Accurate counts but:

  • Adds a sync pipeline step (or a new async job).
  • Segmentation model error rate is currently 70–81 % — sometimes the pre-pass will be wrong.
  • Model improvements over time would require re-running on old docs.

Option B — flexible / open-ended (v1 recommendation)

No predicted total. Panel header shows:

  • "3 Abschnitte gezeichnet · 2 transkribiert"
  • When all drawn regions are transcribed: call-out "Alles transkribiert — [Für abgeschlossen markieren]".
  • Transcriber decides when done and clicks the button.

Simpler; doesn't lie to the user; doesn't depend on a model accuracy level we can't guarantee.

Option C — M grows as they draw

Progress = completed / drawn. Meaningless until they've drawn every region — which is exactly when they'd want the signal.

Ship Option B in this issue. Option A is a follow-up once segmentation reliability is higher; hook into Document.expectedSectionCount then.

Non-goals

  • No segmentation pre-pass (Option A).
  • No change to the document-level progress ring shown on cards.
  • No rollup into milestones / reports — just the header in the panel.

Implementation plan

Backend

  • New column: documents.transcription_complete BOOLEAN NOT NULL DEFAULT false.
  • New endpoint: POST /api/documents/{id}/mark-transcription-complete → sets the flag. Permission: WRITE_ALL. Idempotent.
  • New endpoint (or extend existing): POST /api/documents/{id}/unmark-transcription-complete → unsets.
  • Extend Document entity + @Schema(requiredMode = REQUIRED) on the new boolean.
  • Flyway migration V<next>__document_transcription_complete.sql.
  • Home-page "Ready to Read?" queue already filters on documentStatus; extend its service method to also include transcription_complete = true documents (or use a union in the query).

Frontend

  • TranscribePanel.svelte header:
    • Replace {sections.length} sections with:
      {sectionsDrawn} Abschnitte · {sectionsTranscribed} transkribiert
    • When sectionsDrawn > 0 && sectionsTranscribed === sectionsDrawn && !document.transcriptionComplete:
      render the call-out "Alles transkribiert — [Für abgeschlossen markieren]".
    • When document.transcriptionComplete: render a chip "Abgeschlossen" with an "Erneut öffnen" unmark link.
  • Regenerate API types (npm run generate:api).

i18n

6–8 new Paraglide keys:
transcribe_progress_sections_drawn, transcribe_progress_sections_transcribed, transcribe_all_done_title, transcribe_mark_complete_button, transcribe_completed_chip, transcribe_reopen_button.

Tests

  • Backend unit (DocumentServiceTest): markTranscriptionComplete sets flag; idempotent; unmark reverses.
  • Backend controller test (DocumentControllerTest): endpoints are permission-gated to WRITE_ALL.
  • Flyway: migration applies cleanly on a clean DB and on the current dev DB (confirm via ./mvnw test).
  • Frontend component: TranscribePanel header shows expected strings across the three states (none drawn / drawn-but-not-all-transcribed / all-transcribed-not-complete / marked complete).
  • E2E: log in, open a doc with 0 regions → "0 Abschnitte · 0 transkribiert". Draw 2 regions → "2 · 0". Transcribe one → "2 · 1". Transcribe second → "2 · 2" + mark-complete CTA. Click it → chip appears. Visit Home → the document is in Ready-to-Read.

Verification

Manual: open three documents in different states (fresh, mid-flight, all-transcribed). Walk the UI and confirm each state message matches. Mark one complete and confirm it surfaces in Home's Ready-to-Read list.

Acceptance criteria

  • Panel header shows drawn vs transcribed counts
  • "Mark as complete" CTA appears when all drawn regions are transcribed
  • "Abgeschlossen" chip + unmark link appears when marked complete
  • Backend endpoints exist, permission-gated, idempotent
  • Flyway migration adds transcription_complete column with sensible default
  • Home "Ready to Read" queue respects the new flag
  • i18n complete for de/en/es

Critical files

frontend/src/lib/components/TranscribePanel.svelte
frontend/src/lib/generated/api.ts                                         (regen)
backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java
backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java
backend/src/main/java/org/raddatz/familienarchiv/model/Document.java
backend/src/main/resources/db/migration/V<next>__document_transcription_complete.sql
backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java
backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java
frontend/messages/{de,en,es}.json
  • #320 (coach empty state) — sibling transcriber QoL work.
  • #327 (keyboard shortcuts) — the mark-complete CTA should be reachable by shortcut.
## Context The Transcribe panel header currently shows `0 sections` as its only progress indicator. Transcribers can't tell: - How many regions still need work on this document. - Whether they're on the last region or the first. - When a document counts as "done" from the transcription perspective. Meanwhile the document detail card elsewhere in the app shows `100% reviewed` as a progress ring — those two signals don't connect. A transcriber could be "100% reviewed" but still have un-transcribed regions, or vice versa. Motivation loss is the real cost in a volunteer workflow. Visible progress is the cheapest way to keep people moving. ## The hard question: how is total-section-count known? Three options considered: ### Option A — segmentation pre-pass on upload Run the segmentation model (already exists; see `/admin/ocr` Train segmentation) once per document on upload. Store `Document.expectedSectionCount`. Accurate counts but: - Adds a sync pipeline step (or a new async job). - Segmentation model error rate is currently 70–81 % — sometimes the pre-pass will be wrong. - Model improvements over time would require re-running on old docs. ### Option B — flexible / open-ended (v1 recommendation) No predicted total. Panel header shows: - "3 Abschnitte gezeichnet · 2 transkribiert" - When all drawn regions are transcribed: call-out **"Alles transkribiert — [Für abgeschlossen markieren]"**. - Transcriber decides when done and clicks the button. Simpler; doesn't lie to the user; doesn't depend on a model accuracy level we can't guarantee. ### Option C — M grows as they draw Progress = completed / drawn. Meaningless until they've drawn every region — which is exactly when they'd want the signal. **Ship Option B in this issue.** Option A is a follow-up once segmentation reliability is higher; hook into `Document.expectedSectionCount` then. ## Non-goals - No segmentation pre-pass (Option A). - No change to the document-level progress ring shown on cards. - No rollup into milestones / reports — just the header in the panel. ## Implementation plan ### Backend - New column: `documents.transcription_complete BOOLEAN NOT NULL DEFAULT false`. - New endpoint: `POST /api/documents/{id}/mark-transcription-complete` → sets the flag. Permission: `WRITE_ALL`. Idempotent. - New endpoint (or extend existing): `POST /api/documents/{id}/unmark-transcription-complete` → unsets. - Extend `Document` entity + `@Schema(requiredMode = REQUIRED)` on the new boolean. - Flyway migration `V<next>__document_transcription_complete.sql`. - Home-page "Ready to Read?" queue already filters on `documentStatus`; extend its service method to also include `transcription_complete = true` documents (or use a union in the query). ### Frontend - `TranscribePanel.svelte` header: - Replace `{sections.length} sections` with: `{sectionsDrawn} Abschnitte · {sectionsTranscribed} transkribiert` - When `sectionsDrawn > 0 && sectionsTranscribed === sectionsDrawn && !document.transcriptionComplete`: render the call-out "Alles transkribiert — [Für abgeschlossen markieren]". - When `document.transcriptionComplete`: render a chip "Abgeschlossen" with an "Erneut öffnen" unmark link. - Regenerate API types (`npm run generate:api`). ### i18n 6–8 new Paraglide keys: `transcribe_progress_sections_drawn`, `transcribe_progress_sections_transcribed`, `transcribe_all_done_title`, `transcribe_mark_complete_button`, `transcribe_completed_chip`, `transcribe_reopen_button`. ## Tests - **Backend unit (DocumentServiceTest):** `markTranscriptionComplete` sets flag; idempotent; unmark reverses. - **Backend controller test (DocumentControllerTest):** endpoints are permission-gated to `WRITE_ALL`. - **Flyway:** migration applies cleanly on a clean DB and on the current dev DB (confirm via `./mvnw test`). - **Frontend component:** TranscribePanel header shows expected strings across the three states (none drawn / drawn-but-not-all-transcribed / all-transcribed-not-complete / marked complete). - **E2E:** log in, open a doc with 0 regions → "0 Abschnitte · 0 transkribiert". Draw 2 regions → "2 · 0". Transcribe one → "2 · 1". Transcribe second → "2 · 2" + mark-complete CTA. Click it → chip appears. Visit Home → the document is in Ready-to-Read. ## Verification Manual: open three documents in different states (fresh, mid-flight, all-transcribed). Walk the UI and confirm each state message matches. Mark one complete and confirm it surfaces in Home's Ready-to-Read list. ## Acceptance criteria - [ ] Panel header shows drawn vs transcribed counts - [ ] "Mark as complete" CTA appears when all drawn regions are transcribed - [ ] "Abgeschlossen" chip + unmark link appears when marked complete - [ ] Backend endpoints exist, permission-gated, idempotent - [ ] Flyway migration adds `transcription_complete` column with sensible default - [ ] Home "Ready to Read" queue respects the new flag - [ ] i18n complete for de/en/es ## Critical files ``` frontend/src/lib/components/TranscribePanel.svelte frontend/src/lib/generated/api.ts (regen) backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java backend/src/main/java/org/raddatz/familienarchiv/model/Document.java backend/src/main/resources/db/migration/V<next>__document_transcription_complete.sql backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java frontend/messages/{de,en,es}.json ``` ## Related - #320 (coach empty state) — sibling transcriber QoL work. - #327 (keyboard shortcuts) — the mark-complete CTA should be reachable by shortcut.
marcel added this to the Transcriber Experience v1 milestone 2026-04-24 13:23:47 +02:00
marcel added the P1-highfeatureui labels 2026-04-24 13:28:08 +02:00
marcel modified the milestone from Transcriber Experience v1 to Demo Day — family get-together 2026-04-24 13:35:15 +02:00
Author
Owner

🏗️ Markus Keller — Application Architect

Observations

  • The transcription_complete flag creates a semantic gap with the existing ready-to-read queue. The current findReadyToReadQueue query in DocumentRepository uses a 90%-reviewed heuristic (reviewed_pct >= 0.90) to decide if a document is "Lesefertig". The issue proposes adding a manual boolean flag that also feeds this queue. Now the queue will contain two different kinds of "ready" documents — ones that crossed 90% by the algorithm, and ones the transcriber clicked a button for. The issue text says to "extend its service method to also include transcription_complete = true documents (or use a union in the query)", but these two signals measure different things: review coverage vs transcriber intent.

  • The TranscriptionBlock.reviewed field and the new Document.transcriptionComplete flag risk causing contradictory state. A document can have transcriptionComplete = true but only 2 out of 10 blocks reviewed. Or a document can be 100% reviewed (auto-eligible for the queue) and also have transcriptionComplete = false. The issue non-goals say to not change the document-level progress ring, but the document-level DocumentStatus lifecycle (UPLOADED → TRANSCRIBED → REVIEWED) is already in play. The flag adds a parallel state machine.

  • Two REST endpoints for a boolean toggle is clean and idempotent as designed. POST /mark-transcription-complete and POST /unmark-transcription-complete on DocumentController is the right call; it keeps the URL surface stable even if the underlying state management changes later. The alternative — PATCH /{id} with { "transcriptionComplete": true } — would work but couples this flag to the existing metadata update DTO.

  • The Flyway migration for transcription_complete BOOLEAN NOT NULL DEFAULT false is straightforward. No migration complexity here; it lands cleanly on existing rows without backfill logic. That's correct behavior — all historical documents default to false.

  • Architecture diagrams will need updating. The documents table changes require docs/architecture/db/db-orm.puml and docs/architecture/db/db-relationships.puml updates. The new endpoints on DocumentController require updating the matching C4 L3 backend diagram. These are mandatory per the architecture documentation rules, not optional.

  • The GLOSSARY needs a new entry. "Transcription complete" is a new domain concept that transcribers will use differently from "reviewed" and "ready to read". Per the glossary convention, add it to docs/GLOSSARY.md under Workflow Terms.

Recommendations

  • Resolve the dual-signal problem before implementation. Pick one of two clean models: (A) transcription_complete = true replaces the 90%-heuristic in the ready-to-read queue — if the transcriber says done, trust them; OR (B) transcription_complete feeds the ready-to-read queue in addition to the 90%-heuristic, but the two signals are clearly disjoint in the query (e.g., WHERE transcription_complete = true OR reviewed_pct >= 0.90). Model A is simpler and more honest — the heuristic was always a proxy for "transcriber declares done", and Option B now provides the real signal.

  • If going with Model A, remove the 90% heuristic from findReadyToReadQueue entirely and replace with WHERE d.transcription_complete = true. This eliminates the dual-state problem and makes the ready-to-read queue semantically clean.

  • Add the GLOSSARY and C4 diagram updates to the acceptance criteria before implementation starts. These are blocked PRs otherwise.

Open Decisions

  • Should transcription_complete = true automatically appear in the ready-to-read queue, or should it only appear when the document also has a minimum number of blocks with text? A document could be marked complete with 0 actual text content — is that a valid "ready to read"? Minimal guard: WHERE transcription_complete = true AND EXISTS (SELECT 1 FROM transcription_blocks WHERE document_id = d.id AND text IS NOT NULL AND text <> '').
## 🏗️ Markus Keller — Application Architect ### Observations - **The `transcription_complete` flag creates a semantic gap with the existing ready-to-read queue.** The current `findReadyToReadQueue` query in `DocumentRepository` uses a 90%-reviewed heuristic (`reviewed_pct >= 0.90`) to decide if a document is "Lesefertig". The issue proposes adding a *manual* boolean flag that also feeds this queue. Now the queue will contain two different kinds of "ready" documents — ones that crossed 90% by the algorithm, and ones the transcriber clicked a button for. The issue text says to "extend its service method to also include `transcription_complete = true` documents (or use a union in the query)", but these two signals measure different things: review coverage vs transcriber intent. - **The `TranscriptionBlock.reviewed` field and the new `Document.transcriptionComplete` flag risk causing contradictory state.** A document can have `transcriptionComplete = true` but only 2 out of 10 blocks reviewed. Or a document can be 100% reviewed (auto-eligible for the queue) and also have `transcriptionComplete = false`. The issue non-goals say to not change the document-level progress ring, but the document-level `DocumentStatus` lifecycle (`UPLOADED → TRANSCRIBED → REVIEWED`) is already in play. The flag adds a parallel state machine. - **Two REST endpoints for a boolean toggle is clean and idempotent as designed.** `POST /mark-transcription-complete` and `POST /unmark-transcription-complete` on `DocumentController` is the right call; it keeps the URL surface stable even if the underlying state management changes later. The alternative — `PATCH /{id}` with `{ "transcriptionComplete": true }` — would work but couples this flag to the existing metadata update DTO. - **The Flyway migration for `transcription_complete BOOLEAN NOT NULL DEFAULT false` is straightforward.** No migration complexity here; it lands cleanly on existing rows without backfill logic. That's correct behavior — all historical documents default to `false`. - **Architecture diagrams will need updating.** The `documents` table changes require `docs/architecture/db/db-orm.puml` and `docs/architecture/db/db-relationships.puml` updates. The new endpoints on `DocumentController` require updating the matching C4 L3 backend diagram. These are mandatory per the architecture documentation rules, not optional. - **The GLOSSARY needs a new entry.** "Transcription complete" is a new domain concept that transcribers will use differently from "reviewed" and "ready to read". Per the glossary convention, add it to `docs/GLOSSARY.md` under Workflow Terms. ### Recommendations - **Resolve the dual-signal problem before implementation.** Pick one of two clean models: (A) `transcription_complete = true` *replaces* the 90%-heuristic in the ready-to-read queue — if the transcriber says done, trust them; OR (B) `transcription_complete` feeds the ready-to-read queue *in addition to* the 90%-heuristic, but the two signals are clearly disjoint in the query (e.g., `WHERE transcription_complete = true OR reviewed_pct >= 0.90`). Model A is simpler and more honest — the heuristic was always a proxy for "transcriber declares done", and Option B now provides the real signal. - **If going with Model A, remove the 90% heuristic from `findReadyToReadQueue` entirely** and replace with `WHERE d.transcription_complete = true`. This eliminates the dual-state problem and makes the ready-to-read queue semantically clean. - **Add the GLOSSARY and C4 diagram updates to the acceptance criteria** before implementation starts. These are blocked PRs otherwise. ### Open Decisions - **Should `transcription_complete = true` *automatically* appear in the ready-to-read queue, or should it only appear when the document also has a minimum number of blocks with text?** A document could be marked complete with 0 actual text content — is that a valid "ready to read"? Minimal guard: `WHERE transcription_complete = true AND EXISTS (SELECT 1 FROM transcription_blocks WHERE document_id = d.id AND text IS NOT NULL AND text <> '')`.
Author
Owner

👨‍💻 Felix Brandt — Fullstack Developer

Observations

  • TranscriptionPanelHeader.svelte already receives blockCount (total drawn) and lastEditedAt. The prop interface will need two new props: sectionsTranscribed (blocks with non-null, non-empty text) and transcriptionComplete (the new document flag). The change to the status line is a straightforward swap — the existing "N Abschnitte" string becomes "N Abschnitte · M transkribiert".

  • sectionsTranscribed should be computed in TranscriptionEditView.svelte as a $derived, not passed from the route. TranscriptionEditView already owns the blocks array and derives reviewedCount, totalCount, allReviewed. Adding const sectionsTranscribed = $derived(blocks.filter(b => b.text !== null && b.text !== '').length) follows the existing pattern exactly. The parent view should not re-derive this from raw data.

  • The call-to-action and "Abgeschlossen" chip are two distinct visual states that warrant their own $derived guards. Following the existing allReviewed pattern: const allTranscribed = $derived(sectionsDrawn > 0 && sectionsTranscribed === sectionsDrawn) and const showMarkCompleteCta = $derived(allTranscribed && !document.transcriptionComplete). Keep these named boolean derivations — don't inline the logic into the template.

  • The CTA and chip belong in TranscriptionPanelHeader.svelte only if they remain as header-strip elements. Looking at the existing header, it is a single 44px-tall bar. A CTA with a button and an "Erneut öffnen" link is likely too wide for that bar, especially on mobile. If these elements grow beyond a single line, they belong in a dedicated TranscriptionCompletionBanner.svelte component below the header, not inside it. The issue says "render the call-out" in the header — but the header's flex layout currently has three zones (toggle, status text, close). Adding a fourth zone risks overflow on small screens. Worth verifying with Leonie.

  • The two new backend endpoints need @Schema(requiredMode = REQUIRED) on the Document entity's new field for TypeScript codegen to produce a non-optional transcriptionComplete: boolean (not transcriptionComplete?: boolean). This is already specified in the issue but is worth flagging as a first implementation check — it's the most common cause of TypeScript type bugs after regen.

  • The blockCount prop passed to TranscriptionPanelHeader from TranscriptionEditView at line 289 is currently blocks.length. After this change, the header needs to know both sectionsDrawn (all blocks) and sectionsTranscribed (blocks with text). Rename blockCount to sectionsDrawn in the prop interface for clarity — the existing name is ambiguous.

Recommendations

  • Derive sectionsTranscribed in TranscriptionEditView alongside the existing reviewedCount derivation, not in the header component. The header component should receive pre-computed counts, not raw block arrays — this keeps the header stateless and easily testable.

  • Extract the mark-complete CTA into TranscriptionCompletionBanner.svelte if it contains more than one interactive element (button + unmark link). The 44px header is not the right container for a multi-element action bar.

  • Rename the blockCount prop to sectionsDrawn in the TranscriptionPanelHeader interface when making changes — this is a good opportunity to align naming with the issue's domain language.

  • Write the backend service tests in strict TDD order: markTranscriptionComplete_sets_flag, markTranscriptionComplete_is_idempotent, unmarkTranscriptionComplete_clears_flag — in that order, red first each time.

  • After npm run generate:api, verify that transcriptionComplete becomes a required (non-optional) field in the generated TypeScript before writing any frontend code that depends on it.

## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations - **`TranscriptionPanelHeader.svelte` already receives `blockCount` (total drawn) and `lastEditedAt`.** The prop interface will need two new props: `sectionsTranscribed` (blocks with non-null, non-empty text) and `transcriptionComplete` (the new document flag). The change to the status line is a straightforward swap — the existing "N Abschnitte" string becomes "N Abschnitte · M transkribiert". - **`sectionsTranscribed` should be computed in `TranscriptionEditView.svelte` as a `$derived`, not passed from the route.** `TranscriptionEditView` already owns the blocks array and derives `reviewedCount`, `totalCount`, `allReviewed`. Adding `const sectionsTranscribed = $derived(blocks.filter(b => b.text !== null && b.text !== '').length)` follows the existing pattern exactly. The parent view should not re-derive this from raw data. - **The call-to-action and "Abgeschlossen" chip are two distinct visual states that warrant their own `$derived` guards.** Following the existing `allReviewed` pattern: `const allTranscribed = $derived(sectionsDrawn > 0 && sectionsTranscribed === sectionsDrawn)` and `const showMarkCompleteCta = $derived(allTranscribed && !document.transcriptionComplete)`. Keep these named boolean derivations — don't inline the logic into the template. - **The CTA and chip belong in `TranscriptionPanelHeader.svelte` only if they remain as header-strip elements.** Looking at the existing header, it is a single 44px-tall bar. A CTA with a button and an "Erneut öffnen" link is likely too wide for that bar, especially on mobile. If these elements grow beyond a single line, they belong in a dedicated `TranscriptionCompletionBanner.svelte` component below the header, not inside it. The issue says "render the call-out" in the header — but the header's flex layout currently has three zones (toggle, status text, close). Adding a fourth zone risks overflow on small screens. Worth verifying with Leonie. - **The two new backend endpoints need `@Schema(requiredMode = REQUIRED)` on the `Document` entity's new field** for TypeScript codegen to produce a non-optional `transcriptionComplete: boolean` (not `transcriptionComplete?: boolean`). This is already specified in the issue but is worth flagging as a first implementation check — it's the most common cause of TypeScript type bugs after regen. - **The `blockCount` prop passed to `TranscriptionPanelHeader` from `TranscriptionEditView` at line 289 is currently `blocks.length`.** After this change, the header needs to know both `sectionsDrawn` (all blocks) and `sectionsTranscribed` (blocks with text). Rename `blockCount` to `sectionsDrawn` in the prop interface for clarity — the existing name is ambiguous. ### Recommendations - **Derive `sectionsTranscribed` in `TranscriptionEditView` alongside the existing `reviewedCount` derivation, not in the header component.** The header component should receive pre-computed counts, not raw block arrays — this keeps the header stateless and easily testable. - **Extract the mark-complete CTA into `TranscriptionCompletionBanner.svelte`** if it contains more than one interactive element (button + unmark link). The 44px header is not the right container for a multi-element action bar. - **Rename the `blockCount` prop to `sectionsDrawn`** in the `TranscriptionPanelHeader` interface when making changes — this is a good opportunity to align naming with the issue's domain language. - **Write the backend service tests in strict TDD order:** `markTranscriptionComplete_sets_flag`, `markTranscriptionComplete_is_idempotent`, `unmarkTranscriptionComplete_clears_flag` — in that order, red first each time. - **After `npm run generate:api`, verify that `transcriptionComplete` becomes a required (non-optional) field in the generated TypeScript before writing any frontend code that depends on it.**
Author
Owner

🔒 Nora "NullX" Steiner — Security Engineer

Observations

  • Both new write endpoints require @RequirePermission(Permission.WRITE_ALL). The issue correctly specifies this. However, the current DocumentController shows that @GetMapping("/{id}") (read) has no permission check — any authenticated user can read a document. The new POST /{id}/mark-transcription-complete must not inherit this pattern by accident. The pattern on the existing write endpoints in DocumentController (createDocument, updateDocument, deleteDocument) correctly uses @RequirePermission(Permission.WRITE_ALL) — follow that exactly.

  • No IDOR risk for the mark/unmark endpoints. The document ID is in the path, the service will call getDocumentById(id) which throws notFound for unknown IDs. There is no user-scoped ownership check needed here (per CLAUDE.md: this is a family archive, not a multi-tenant system with per-user document ownership). The only check needed is permission level, which @RequirePermission(Permission.WRITE_ALL) handles.

  • The two-endpoint design (POST /mark-transcription-complete and POST /unmark-transcription-complete) is slightly odd semantically — these are state transitions on a resource, and a PATCH /{id} with { "transcriptionComplete": true/false } would be equally valid and simpler. Both are safe. The current design adds two URL surfaces but is idempotent as specified.

  • No injection surface on these endpoints. The payload is just a document UUID from the path — no user-controlled string reaches the database as an interpolated value. The flag write is a simple doc.setTranscriptionComplete(true) + save. No SQL injection risk.

  • Audit logging. The existing DocumentService calls auditService for document updates. The markTranscriptionComplete service method should also emit an audit event — both for accountability (who declared it done, and when) and to make the "Aktivitäten" feed aware of this milestone. Check whether a new AuditKind enum value is needed for this transition or whether an existing kind covers it.

  • The frontend CTA button should disable during the in-flight API call to prevent double-submission. A double-click could send two POST /mark-transcription-complete requests simultaneously. Since the endpoint is idempotent this is not a data corruption risk, but it is a UX smell and can confuse audit logs with duplicate entries. Disable the button while marking state is true.

Recommendations

  • Add @RequirePermission(Permission.WRITE_ALL) to both endpoints as the first line of review after implementation — before any other functionality is verified.

  • Emit an audit event from markTranscriptionComplete and unmarkTranscriptionComplete service methods, using the existing AuditService pattern. Log AuditKind (or a new TRANSCRIPTION_MARKED_COMPLETE kind if warranted) with the document ID and the acting user ID.

  • Disable the mark-complete button and unmark link in the frontend during the in-flight API call using a local $state boolean (let marking = $state(false)) with try/finally.

## 🔒 Nora "NullX" Steiner — Security Engineer ### Observations - **Both new write endpoints require `@RequirePermission(Permission.WRITE_ALL)`.** The issue correctly specifies this. However, the current `DocumentController` shows that `@GetMapping("/{id}")` (read) has *no permission check* — any authenticated user can read a document. The new `POST /{id}/mark-transcription-complete` must not inherit this pattern by accident. The pattern on the existing write endpoints in `DocumentController` (`createDocument`, `updateDocument`, `deleteDocument`) correctly uses `@RequirePermission(Permission.WRITE_ALL)` — follow that exactly. - **No IDOR risk for the mark/unmark endpoints.** The document ID is in the path, the service will call `getDocumentById(id)` which throws `notFound` for unknown IDs. There is no user-scoped ownership check needed here (per CLAUDE.md: this is a family archive, not a multi-tenant system with per-user document ownership). The only check needed is permission level, which `@RequirePermission(Permission.WRITE_ALL)` handles. - **The two-endpoint design (`POST /mark-transcription-complete` and `POST /unmark-transcription-complete`) is slightly odd semantically** — these are state transitions on a resource, and a `PATCH /{id}` with `{ "transcriptionComplete": true/false }` would be equally valid and simpler. Both are safe. The current design adds two URL surfaces but is idempotent as specified. - **No injection surface on these endpoints.** The payload is just a document UUID from the path — no user-controlled string reaches the database as an interpolated value. The flag write is a simple `doc.setTranscriptionComplete(true)` + save. No SQL injection risk. - **Audit logging.** The existing `DocumentService` calls `auditService` for document updates. The `markTranscriptionComplete` service method should also emit an audit event — both for accountability (who declared it done, and when) and to make the "Aktivitäten" feed aware of this milestone. Check whether a new `AuditKind` enum value is needed for this transition or whether an existing kind covers it. - **The frontend CTA button should disable during the in-flight API call** to prevent double-submission. A double-click could send two `POST /mark-transcription-complete` requests simultaneously. Since the endpoint is idempotent this is not a data corruption risk, but it is a UX smell and can confuse audit logs with duplicate entries. Disable the button while `marking` state is true. ### Recommendations - **Add `@RequirePermission(Permission.WRITE_ALL)` to both endpoints as the first line of review after implementation** — before any other functionality is verified. - **Emit an audit event from `markTranscriptionComplete` and `unmarkTranscriptionComplete` service methods**, using the existing `AuditService` pattern. Log `AuditKind` (or a new `TRANSCRIPTION_MARKED_COMPLETE` kind if warranted) with the document ID and the acting user ID. - **Disable the mark-complete button and unmark link in the frontend during the in-flight API call** using a local `$state` boolean (`let marking = $state(false)`) with try/finally.
Author
Owner

🧪 Sara Holt — QA Engineer

Observations

  • The issue's test plan is well-structured and covers all three layers (unit, controller, frontend component, E2E). This is better than most issue specs. The specific test names for the backend are clear enough to implement directly.

  • Four frontend states are identified for the panel header, but the existing test file (TranscriptionPanelHeader.svelte.test.ts) will need significant expansion. The current tests cover mode-toggle behavior and basic block count display. The new tests need to cover: (1) "N Abschnitte · M transkribiert" status line format, (2) mark-complete CTA visibility conditions (allTranscribed && !complete), (3) "Abgeschlossen" chip visibility when transcriptionComplete = true, (4) "Erneut öffnen" link click triggers onUnmarkComplete callback. That's at least 6 new test cases in the component spec.

  • The E2E test scenario described is comprehensive but the steps rely on drawing regions. Drawing regions in Playwright against a PDF canvas is non-trivial and is likely already tested elsewhere (or not at all). Consider splitting the E2E into: (A) a focused E2E that starts with a document that already has 2 pre-existing blocks (seeded via API calls) — tests the header display and mark-complete CTA; and (B) a separate, optional E2E (or integration test mocking the annotation layer) for the full draw → transcribe → mark-complete flow.

  • The backend DocumentServiceTest must test the idempotency case explicitly, not just assert on first call. The recommended approach: call markTranscriptionComplete(id) twice in the same test, verify no exception is thrown on the second call, and verify the final state is still true. A separate test for unmark-after-mark should also be present.

  • The DocumentControllerTest for the new endpoints needs both the authorized (200) and unauthorized (403) cases. The existing DocumentControllerTest pattern uses @WebMvcTest + @MockBean DocumentService. Verify that both POST /mark-transcription-complete and POST /unmark-transcription-complete return 403 when the caller has only READ_ALL, and 200 when the caller has WRITE_ALL.

  • The "Home Ready-to-Read queue respects the new flag" acceptance criterion needs an integration test in DocumentRepositoryTest, not just a service-layer unit test. The ready-to-read query is a native SQL query — it needs to be tested against a real PostgreSQL container to verify the flag filtering works. This is exactly the scenario where H2 would silently lie and Testcontainers would catch the bug.

  • There are no rollback tests specified. What happens if markTranscriptionComplete succeeds but a downstream call (e.g., audit logging) fails? If the service method is @Transactional, the whole thing rolls back and the flag is not set. That should be tested explicitly: mock auditService to throw, verify the flag is NOT persisted.

Recommendations

  • Add a DocumentRepositoryTest integration test case (using the existing Testcontainers PostgreSQL setup) that verifies findReadyToReadQueue returns a document when transcription_complete = true — after whatever query change Markus decides on.

  • Split the E2E scenario. Use API seeding (REST calls in beforeEach) to create a document with pre-existing blocks in known states, rather than relying on UI-driven region drawing for the progress display tests. The UI-driven draw flow belongs in a separate, clearly labeled "full workflow" E2E.

  • Add the 403 controller tests for both endpoints before the 200 tests — red/green means writing the permission-rejection test first.

  • Add a test for the transcriptionComplete = true case in TranscriptionPanelHeader where sectionsDrawn === 0 — can a document be marked complete with no drawn sections? The UI should handle this edge case gracefully even if the service prevents it.

## 🧪 Sara Holt — QA Engineer ### Observations - **The issue's test plan is well-structured and covers all three layers (unit, controller, frontend component, E2E).** This is better than most issue specs. The specific test names for the backend are clear enough to implement directly. - **Four frontend states are identified for the panel header, but the existing test file (`TranscriptionPanelHeader.svelte.test.ts`) will need significant expansion.** The current tests cover mode-toggle behavior and basic block count display. The new tests need to cover: (1) "N Abschnitte · M transkribiert" status line format, (2) mark-complete CTA visibility conditions (`allTranscribed && !complete`), (3) "Abgeschlossen" chip visibility when `transcriptionComplete = true`, (4) "Erneut öffnen" link click triggers `onUnmarkComplete` callback. That's at least 6 new test cases in the component spec. - **The E2E test scenario described is comprehensive but the steps rely on drawing regions.** Drawing regions in Playwright against a PDF canvas is non-trivial and is likely already tested elsewhere (or not at all). Consider splitting the E2E into: (A) a focused E2E that starts with a document that already has 2 pre-existing blocks (seeded via API calls) — tests the header display and mark-complete CTA; and (B) a separate, optional E2E (or integration test mocking the annotation layer) for the full draw → transcribe → mark-complete flow. - **The backend `DocumentServiceTest` must test the idempotency case explicitly, not just assert on first call.** The recommended approach: call `markTranscriptionComplete(id)` twice in the same test, verify no exception is thrown on the second call, and verify the final state is still `true`. A separate test for unmark-after-mark should also be present. - **The `DocumentControllerTest` for the new endpoints needs both the authorized (200) and unauthorized (403) cases.** The existing `DocumentControllerTest` pattern uses `@WebMvcTest` + `@MockBean DocumentService`. Verify that both `POST /mark-transcription-complete` and `POST /unmark-transcription-complete` return 403 when the caller has only `READ_ALL`, and 200 when the caller has `WRITE_ALL`. - **The "Home Ready-to-Read queue respects the new flag" acceptance criterion needs an integration test in `DocumentRepositoryTest`**, not just a service-layer unit test. The ready-to-read query is a native SQL query — it needs to be tested against a real PostgreSQL container to verify the flag filtering works. This is exactly the scenario where H2 would silently lie and Testcontainers would catch the bug. - **There are no rollback tests specified.** What happens if `markTranscriptionComplete` succeeds but a downstream call (e.g., audit logging) fails? If the service method is `@Transactional`, the whole thing rolls back and the flag is not set. That should be tested explicitly: mock `auditService` to throw, verify the flag is NOT persisted. ### Recommendations - **Add a `DocumentRepositoryTest` integration test case** (using the existing Testcontainers PostgreSQL setup) that verifies `findReadyToReadQueue` returns a document when `transcription_complete = true` — after whatever query change Markus decides on. - **Split the E2E scenario.** Use API seeding (REST calls in `beforeEach`) to create a document with pre-existing blocks in known states, rather than relying on UI-driven region drawing for the progress display tests. The UI-driven draw flow belongs in a separate, clearly labeled "full workflow" E2E. - **Add the 403 controller tests for both endpoints** before the 200 tests — red/green means writing the permission-rejection test first. - **Add a test for the `transcriptionComplete = true` case in `TranscriptionPanelHeader` where `sectionsDrawn === 0`** — can a document be marked complete with no drawn sections? The UI should handle this edge case gracefully even if the service prevents it.
Author
Owner

🎨 Leonie Voss — UI/UX Design Lead

Observations

  • The TranscriptionPanelHeader is currently a single 44px-tall bar with three zones. Adding a fourth element — a multi-line CTA with a button and an unmark link — into this bar will either overflow on mobile or force the existing elements to collapse. The mobile layout must be addressed upfront: on a 320px screen, "2 Abschnitte · 2 transkribiert — [Für abgeschlossen markieren]" does not fit in one line next to the mode toggle and close button.

  • The status text is already hidden on mobile (class="hidden text-xs text-ink-2 md:block"). This means mobile users currently see zero progress feedback from the header. The new "Alles transkribiert" CTA will also need a mobile-visible location — if both the status line and the CTA are desktop-only, transcribers on tablets won't know they can mark the document complete.

  • The "Abgeschlossen" chip and "Erneut öffnen" link are two distinct interaction elements. The chip signals state (passive); the link triggers an action (active). These must be visually differentiated with sufficient contrast and separate touch targets (minimum 44px height for the link). Using brand-mint as the chip fill color is risky for text — brand-mint on white fails WCAG AA for normal text (approximately 2.8:1 contrast ratio). Use brand-mint as a border and use brand-navy for the text inside the chip.

  • The "Alles transkribiert — [Für abgeschlossen markieren]" call-out pattern is a good candidate for a distinct visual treatment, not just inline text in the status zone. Consider a highlighted banner strip below the header (e.g., bg-brand-mint/10 border-t border-brand-mint/30 px-4 py-2) with the message and a clearly labeled primary action button. This visually separates the persistent header from the transient action nudge.

  • The i18n keys cover German but the issue doesn't mention the gender/plural considerations in German. "Abschnitte" is already plural-handled in the existing i18n system. "Alle transkribiert" is gender-neutral. Confirm the Spanish keys follow the same pattern — Spanish has grammatical gender that can affect "transkribiert" equivalents.

  • Touch target for the "Erneut öffnen" unmark link. If rendered as a plain <a> or <span> inside the chip, the touch target will be smaller than 44px. Wrap it in a <button> with min-h-[44px] px-3 styling.

Recommendations

  • Place the mark-complete CTA as a separate visual strip below the panel header, not inside it. Use bg-brand-mint/10 border-t border-brand-mint/30 text-brand-navy font-sans text-sm px-4 py-3 flex items-center justify-between — this visually separates state signaling from navigation controls and avoids overloading the 44px header.

  • Make the status text "N Abschnitte · M transkribiert" visible on mobile. Remove the hidden md:block class, shorten the format to just the counts (e.g., "2/2 transkribiert") for narrow screens, and use a responsive breakpoint to show the full text on desktop. Transcribers on tablets are the primary audience — they must see progress.

  • Use brand-navy text on a brand-mint/20 background for the "Abgeschlossen" chip. Avoid using brand-mint as text color — it fails contrast checks against both white and sand backgrounds. The navy-on-mint-tint combination passes WCAG AA.

  • Render the "Erneut öffnen" action as a <button> with min-h-[44px], even if it looks like a text link visually. Size the touch target appropriately for senior users on touch devices.

Open Decisions

  • Should the mark-complete strip also appear in read mode (not just edit mode)? A transcriber in read mode should arguably also see the completion status and be able to unmark — otherwise they have to switch to edit mode just to access the unmark action. Confirm intended behavior for read-mode display of the chip.
## 🎨 Leonie Voss — UI/UX Design Lead ### Observations - **The `TranscriptionPanelHeader` is currently a single 44px-tall bar with three zones.** Adding a fourth element — a multi-line CTA with a button and an unmark link — into this bar will either overflow on mobile or force the existing elements to collapse. The mobile layout must be addressed upfront: on a 320px screen, "2 Abschnitte · 2 transkribiert — [Für abgeschlossen markieren]" does not fit in one line next to the mode toggle and close button. - **The status text is already hidden on mobile** (`class="hidden text-xs text-ink-2 md:block"`). This means mobile users currently see *zero* progress feedback from the header. The new "Alles transkribiert" CTA will also need a mobile-visible location — if both the status line and the CTA are desktop-only, transcribers on tablets won't know they can mark the document complete. - **The "Abgeschlossen" chip and "Erneut öffnen" link are two distinct interaction elements.** The chip signals state (passive); the link triggers an action (active). These must be visually differentiated with sufficient contrast and separate touch targets (minimum 44px height for the link). Using `brand-mint` as the chip fill color is risky for text — `brand-mint` on white fails WCAG AA for normal text (approximately 2.8:1 contrast ratio). Use `brand-mint` as a *border* and use `brand-navy` for the text inside the chip. - **The "Alles transkribiert — [Für abgeschlossen markieren]" call-out pattern is a good candidate for a distinct visual treatment**, not just inline text in the status zone. Consider a highlighted banner strip below the header (e.g., `bg-brand-mint/10 border-t border-brand-mint/30 px-4 py-2`) with the message and a clearly labeled primary action button. This visually separates the persistent header from the transient action nudge. - **The i18n keys cover German but the issue doesn't mention the gender/plural considerations in German.** "Abschnitte" is already plural-handled in the existing i18n system. "Alle transkribiert" is gender-neutral. Confirm the Spanish keys follow the same pattern — Spanish has grammatical gender that can affect "transkribiert" equivalents. - **Touch target for the "Erneut öffnen" unmark link.** If rendered as a plain `<a>` or `<span>` inside the chip, the touch target will be smaller than 44px. Wrap it in a `<button>` with `min-h-[44px] px-3` styling. ### Recommendations - **Place the mark-complete CTA as a separate visual strip below the panel header, not inside it.** Use `bg-brand-mint/10 border-t border-brand-mint/30 text-brand-navy font-sans text-sm px-4 py-3 flex items-center justify-between` — this visually separates state signaling from navigation controls and avoids overloading the 44px header. - **Make the status text "N Abschnitte · M transkribiert" visible on mobile.** Remove the `hidden md:block` class, shorten the format to just the counts (e.g., "2/2 transkribiert") for narrow screens, and use a responsive breakpoint to show the full text on desktop. Transcribers on tablets are the primary audience — they must see progress. - **Use `brand-navy` text on a `brand-mint/20` background for the "Abgeschlossen" chip.** Avoid using `brand-mint` as text color — it fails contrast checks against both white and sand backgrounds. The navy-on-mint-tint combination passes WCAG AA. - **Render the "Erneut öffnen" action as a `<button>` with `min-h-[44px]`**, even if it looks like a text link visually. Size the touch target appropriately for senior users on touch devices. ### Open Decisions - **Should the mark-complete strip also appear in read mode (not just edit mode)?** A transcriber in read mode should arguably also see the completion status and be able to unmark — otherwise they have to switch to edit mode just to access the unmark action. Confirm intended behavior for read-mode display of the chip.
Author
Owner

📋 Elicit — Requirements Engineer

Observations

  • The issue is well-specified and largely implementation-ready, with a clear rationale, three options evaluated, a chosen path (Option B), acceptance criteria, test plan, and critical files. This is above average for a solo-dev issue tracker. The EARS-style framing is implicit but present.

  • One acceptance criterion has an ambiguous boundary: "Home Ready-to-Read queue respects the new flag." Currently the ready-to-read queue uses a 90%-reviewed heuristic. "Respects the flag" could mean: (A) flag = true AND heuristic are OR'd together; (B) flag = true replaces the heuristic; (C) flag = true gates entry, heuristic alone is insufficient. The issue says "extend its service method to also include transcription_complete = true documents" — this implies OR behavior (A), but the word "also" is load-bearing and could be interpreted either way. This needs to be explicit before the query is written.

  • Missing acceptance criterion: the unmark flow clears the document from the ready-to-read queue. The issue says "Erneut öffnen" unmarks the document, but there is no AC confirming that an unmarked document disappears from the Home ready-to-read list (assuming model A from above). This is an observable outcome a family member could experience: "I thought this was done but it disappeared from the list."

  • The i18n section specifies 6–8 new keys but does not specify the parameter signatures. The existing keys with interpolation (e.g., transcription_status_sections({ count: blockCount })) follow a named-parameter convention. The new keys will need the same — for example, transcribe_progress_sections_drawn({ drawn: number, transcribed: number }) vs. two separate keys. The distinction matters for translator-friendliness (single pluralized string vs. two separate counts). A single key with both parameters is preferred for languages where the two numbers interact grammatically.

  • "When all drawn regions are transcribed" — what does "transcribed" mean precisely? The issue implies text !== null && text !== ''. But a block with only whitespace (" ") would pass that check. Is a whitespace-only block considered transcribed? The TranscriptionBlock entity has text as a nullable TEXT column — there is no strip() or blank check in the existing data model. This edge case should be clarified.

  • The keyboard shortcut integration is noted as a related issue (#327) but not reflected in this issue's acceptance criteria. If #327 will implement a shortcut for the mark-complete CTA, either add a forward-compatible data-action="mark-transcription-complete" attribute to the button in this issue, or explicitly call out that the button is keyboard-accessible by tab focus in this issue's scope.

Recommendations

  • Add an explicit AC for the unmark flow's effect on the ready-to-read queue: "Given a document marked complete that appears in the Home ready-to-read list, when unmark is clicked, then the document is removed from the ready-to-read list on next page load."

  • Specify the definition of "transcribed" in the issue body. Recommend: a block is transcribed when text IS NOT NULL AND trim(text) <> ''. This is consistent with the existing findTranscriptionQueue query which uses text IS NOT NULL AND text <> '' (the query does not strip — confirm whether the frontend trims before saving or not).

  • Specify the i18n key parameter signatures for all 6–8 keys before implementation: name the interpolation parameters explicitly (e.g., { drawn, transcribed } vs. separate keys). This avoids a key-rename after the Paraglide compilation step.

  • Add a note to the implementation plan about the behavior when sectionsDrawn === 0. Should the "0 Abschnitte · 0 transkribiert" string appear in the header from the first moment the panel opens, even before any regions are drawn? The issue's E2E scenario confirms yes ("open a doc with 0 regions → 0 Abschnitte · 0 transkribiert") — add this as an explicit AC.

Open Decisions

  • How should the ready-to-read queue behave after this change? If transcription_complete = true OR reviewed_pct >= 90% both qualify, a document could appear in the queue via the heuristic and then the transcriber marks it complete — or vice versa. Should a transcription_complete = true document always appear in the queue, even if it has zero reviewed blocks? The business rule needs to be stated precisely before the SQL is written.
## 📋 Elicit — Requirements Engineer ### Observations - **The issue is well-specified and largely implementation-ready**, with a clear rationale, three options evaluated, a chosen path (Option B), acceptance criteria, test plan, and critical files. This is above average for a solo-dev issue tracker. The EARS-style framing is implicit but present. - **One acceptance criterion has an ambiguous boundary: "Home Ready-to-Read queue respects the new flag."** Currently the ready-to-read queue uses a 90%-reviewed heuristic. "Respects the flag" could mean: (A) flag = true AND heuristic are OR'd together; (B) flag = true replaces the heuristic; (C) flag = true gates entry, heuristic alone is insufficient. The issue says "extend its service method to also include `transcription_complete = true` documents" — this implies OR behavior (A), but the word "also" is load-bearing and could be interpreted either way. This needs to be explicit before the query is written. - **Missing acceptance criterion: the unmark flow clears the document from the ready-to-read queue.** The issue says "Erneut öffnen" unmarks the document, but there is no AC confirming that an unmarked document disappears from the Home ready-to-read list (assuming model A from above). This is an observable outcome a family member could experience: "I thought this was done but it disappeared from the list." - **The i18n section specifies 6–8 new keys but does not specify the parameter signatures.** The existing keys with interpolation (e.g., `transcription_status_sections({ count: blockCount })`) follow a named-parameter convention. The new keys will need the same — for example, `transcribe_progress_sections_drawn({ drawn: number, transcribed: number })` vs. two separate keys. The distinction matters for translator-friendliness (single pluralized string vs. two separate counts). A single key with both parameters is preferred for languages where the two numbers interact grammatically. - **"When all drawn regions are transcribed" — what does "transcribed" mean precisely?** The issue implies `text !== null && text !== ''`. But a block with only whitespace (`" "`) would pass that check. Is a whitespace-only block considered transcribed? The `TranscriptionBlock` entity has `text` as a nullable `TEXT` column — there is no `strip()` or blank check in the existing data model. This edge case should be clarified. - **The keyboard shortcut integration is noted as a related issue (#327) but not reflected in this issue's acceptance criteria.** If #327 will implement a shortcut for the mark-complete CTA, either add a forward-compatible `data-action="mark-transcription-complete"` attribute to the button in this issue, or explicitly call out that the button is keyboard-accessible by tab focus in this issue's scope. ### Recommendations - **Add an explicit AC for the unmark flow's effect on the ready-to-read queue**: "Given a document marked complete that appears in the Home ready-to-read list, when unmark is clicked, then the document is removed from the ready-to-read list on next page load." - **Specify the definition of "transcribed" in the issue body.** Recommend: a block is transcribed when `text IS NOT NULL AND trim(text) <> ''`. This is consistent with the existing `findTranscriptionQueue` query which uses `text IS NOT NULL AND text <> ''` (the query does not strip — confirm whether the frontend trims before saving or not). - **Specify the i18n key parameter signatures** for all 6–8 keys before implementation: name the interpolation parameters explicitly (e.g., `{ drawn, transcribed }` vs. separate keys). This avoids a key-rename after the Paraglide compilation step. - **Add a note to the implementation plan about the behavior when `sectionsDrawn === 0`.** Should the "0 Abschnitte · 0 transkribiert" string appear in the header from the first moment the panel opens, even before any regions are drawn? The issue's E2E scenario confirms yes ("open a doc with 0 regions → `0 Abschnitte · 0 transkribiert`") — add this as an explicit AC. ### Open Decisions - **How should the ready-to-read queue behave after this change?** If `transcription_complete = true` OR `reviewed_pct >= 90%` both qualify, a document could appear in the queue via the heuristic and then the transcriber marks it complete — or vice versa. Should a `transcription_complete = true` document *always* appear in the queue, even if it has zero reviewed blocks? The business rule needs to be stated precisely before the SQL is written.
Author
Owner

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

  • No infrastructure changes are required for this issue. No new services, no new Docker Compose entries, no new environment variables, no CI pipeline changes. This is purely an application-layer feature — Flyway migration + backend endpoints + frontend components. The existing CI pipeline handles Flyway migration testing via Testcontainers on every build run.

  • The Flyway migration (V<next>__document_transcription_complete.sql) is the only infrastructure-adjacent change. The next available version number after V60 is V61. Confirm this by checking the actual migration directory before writing the file. Using a duplicate version number causes Flyway to fail on startup with a checksum error — a hard failure that blocks the whole stack from starting. This check takes 5 seconds and prevents a painful rollback.

  • DEFAULT false on the new column means no data migration is needed and the migration runs in milliseconds against a large documents table. No downtime concern. PostgreSQL sets boolean defaults at DDL time without rewriting the table. The migration is safe to run on production without a maintenance window.

  • The Flyway migration should include a column comment (via COMMENT ON COLUMN) for operator reference — schema comments are visible in pgAdmin and psql's \d+ documents, which helps whoever is on-call understand unfamiliar columns without opening the codebase.

Recommendations

  • Confirm the next Flyway version number before writing the migration file by running ls backend/src/main/resources/db/migration/ | sort -t_ -k1,1V | tail -1. As of the last visible migration V60__rename_users_to_app_users.sql, the next is V61. Do not guess.

  • Add a COMMENT ON COLUMN to the migration:

    ALTER TABLE documents ADD COLUMN transcription_complete BOOLEAN NOT NULL DEFAULT false;
    COMMENT ON COLUMN documents.transcription_complete IS
      'Set by transcriber via POST /api/documents/{id}/mark-transcription-complete. '
      'Signals that all drawn regions have been typed up, regardless of the reviewed flag.';
    
  • No additional CI or Docker Compose changes needed. The existing ./mvnw test run will pick up the migration test automatically via Testcontainers.

## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations - **No infrastructure changes are required for this issue.** No new services, no new Docker Compose entries, no new environment variables, no CI pipeline changes. This is purely an application-layer feature — Flyway migration + backend endpoints + frontend components. The existing CI pipeline handles Flyway migration testing via Testcontainers on every build run. - **The Flyway migration (`V<next>__document_transcription_complete.sql`) is the only infrastructure-adjacent change.** The next available version number after `V60` is `V61`. Confirm this by checking the actual migration directory before writing the file. Using a duplicate version number causes Flyway to fail on startup with a checksum error — a hard failure that blocks the whole stack from starting. This check takes 5 seconds and prevents a painful rollback. - **`DEFAULT false` on the new column means no data migration is needed** and the migration runs in milliseconds against a large documents table. No downtime concern. PostgreSQL sets boolean defaults at DDL time without rewriting the table. The migration is safe to run on production without a maintenance window. - **The Flyway migration should include a column comment** (via `COMMENT ON COLUMN`) for operator reference — schema comments are visible in pgAdmin and psql's `\d+ documents`, which helps whoever is on-call understand unfamiliar columns without opening the codebase. ### Recommendations - **Confirm the next Flyway version number before writing the migration file** by running `ls backend/src/main/resources/db/migration/ | sort -t_ -k1,1V | tail -1`. As of the last visible migration `V60__rename_users_to_app_users.sql`, the next is `V61`. Do not guess. - **Add a `COMMENT ON COLUMN` to the migration:** ```sql ALTER TABLE documents ADD COLUMN transcription_complete BOOLEAN NOT NULL DEFAULT false; COMMENT ON COLUMN documents.transcription_complete IS 'Set by transcriber via POST /api/documents/{id}/mark-transcription-complete. ' 'Signals that all drawn regions have been typed up, regardless of the reviewed flag.'; ``` - **No additional CI or Docker Compose changes needed.** The existing `./mvnw test` run will pick up the migration test automatically via Testcontainers.
Author
Owner

Decision Queue

Three open decisions surfaced across the reviews. All touch the same root question — what does the transcription_complete flag mean for the ready-to-read queue — so they are grouped here.


Theme 1: Ready-to-Read queue semantics after this change

Raised by: Markus (architect), Elicit (requirements)

The current findReadyToReadQueue query qualifies documents with reviewed_pct >= 90%. This issue adds a manual transcription_complete flag. Three behaviors are possible:

  • Model A (replace): WHERE transcription_complete = true — the heuristic is retired; transcriber intent is the sole gate. Cleanest state machine, no dual-signal confusion.
  • Model B (OR): WHERE transcription_complete = true OR reviewed_pct >= 0.90 — both signals qualify. Documents can enter the queue two ways.
  • Model C (AND-gate): transcription_complete = true is required, heuristic is dropped entirely, but a minimum-content guard is added (AND EXISTS transcription blocks with text).

Decide before writing the SQL query. The native query in DocumentRepository is what needs updating, and the right model determines whether the existing findReadyToReadQueue query is extended or replaced.


Theme 2: Should transcription_complete = true require at least one block with text?

Raised by: Markus (architect)

A transcriber can theoretically click "Für abgeschlossen markieren" on a document with zero drawn regions and zero text. Should the backend prevent this, or should the frontend CTA be gated on sectionsDrawn > 0 && sectionsTranscribed > 0?

Options:

  • Frontend-only gate: Only show the CTA when sectionsDrawn > 0 && sectionsTranscribed === sectionsDrawn. The backend accepts the POST regardless.
  • Backend guard: The service throws a DomainException.conflict(...) if sectionsDrawn === 0 or sectionsTranscribed === 0. The frontend CTA is still gated, but the API also protects against API clients.

The existing code precedent leans toward frontend-only gates for UI-centric checks, with backend guards for data integrity. Since a "complete with 0 blocks" is likely meaningless data rather than a security issue, a frontend CTA gate may be sufficient.


Theme 3: Mark-complete strip visibility in read mode

Raised by: Leonie (UX)

The mark-complete CTA and the "Abgeschlossen" chip — should they appear in read mode as well as edit mode?

  • Edit-mode only: Transcribers in read mode cannot see or change the completion status without switching to edit mode.
  • Both modes: The chip and the unmark action are visible in read mode, allowing transcribers to change their mind without a mode switch.

The "Abgeschlossen" chip in particular (passive state indicator) seems useful in both modes. The CTA button (active) is primarily a transcriber action. A reasonable split: show the chip in both modes; show the mark/unmark button only in edit mode.


To move forward: resolve Theme 1 first (it unblocks the repository query), then Theme 2 (it determines whether a backend guard is needed), then Theme 3 (it determines the frontend layout scope).

## Decision Queue Three open decisions surfaced across the reviews. All touch the same root question — what does the `transcription_complete` flag mean for the ready-to-read queue — so they are grouped here. --- ### Theme 1: Ready-to-Read queue semantics after this change Raised by: Markus (architect), Elicit (requirements) The current `findReadyToReadQueue` query qualifies documents with `reviewed_pct >= 90%`. This issue adds a manual `transcription_complete` flag. Three behaviors are possible: - **Model A (replace):** `WHERE transcription_complete = true` — the heuristic is retired; transcriber intent is the sole gate. Cleanest state machine, no dual-signal confusion. - **Model B (OR):** `WHERE transcription_complete = true OR reviewed_pct >= 0.90` — both signals qualify. Documents can enter the queue two ways. - **Model C (AND-gate):** `transcription_complete = true` is required, heuristic is dropped entirely, but a minimum-content guard is added (`AND EXISTS transcription blocks with text`). **Decide before writing the SQL query.** The native query in `DocumentRepository` is what needs updating, and the right model determines whether the existing `findReadyToReadQueue` query is extended or replaced. --- ### Theme 2: Should `transcription_complete = true` require at least one block with text? Raised by: Markus (architect) A transcriber can theoretically click "Für abgeschlossen markieren" on a document with zero drawn regions and zero text. Should the backend prevent this, or should the frontend CTA be gated on `sectionsDrawn > 0 && sectionsTranscribed > 0`? Options: - **Frontend-only gate:** Only show the CTA when `sectionsDrawn > 0 && sectionsTranscribed === sectionsDrawn`. The backend accepts the POST regardless. - **Backend guard:** The service throws a `DomainException.conflict(...)` if `sectionsDrawn === 0` or `sectionsTranscribed === 0`. The frontend CTA is still gated, but the API also protects against API clients. The existing code precedent leans toward frontend-only gates for UI-centric checks, with backend guards for data integrity. Since a "complete with 0 blocks" is likely meaningless data rather than a security issue, a frontend CTA gate may be sufficient. --- ### Theme 3: Mark-complete strip visibility in read mode Raised by: Leonie (UX) The mark-complete CTA and the "Abgeschlossen" chip — should they appear in **read mode** as well as edit mode? - **Edit-mode only:** Transcribers in read mode cannot see or change the completion status without switching to edit mode. - **Both modes:** The chip and the unmark action are visible in read mode, allowing transcribers to change their mind without a mode switch. The "Abgeschlossen" chip in particular (passive state indicator) seems useful in both modes. The CTA button (active) is primarily a transcriber action. A reasonable split: show the chip in both modes; show the mark/unmark button only in edit mode. --- **To move forward:** resolve Theme 1 first (it unblocks the repository query), then Theme 2 (it determines whether a backend guard is needed), then Theme 3 (it determines the frontend layout scope).
Sign in to join this conversation.
No Label P1-high feature ui
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#321