feat(documents): bulk upload — split-panel with file switcher #317

Closed
opened 2026-04-24 10:33:29 +02:00 by marcel · 8 comments
Owner

Context

Extends #294 (new-document split-panel) with bulk uploads. When a user drops N files on /documents/new, each becomes its own document; every metadata field is shared across all N except the title, which is pre-filled from the filename and editable per file. A single POST to /api/documents/quick-upload creates N documents in one pass.

The backend already supports batch multipart uploads via quick-upload — this is primarily a frontend extension plus a small metadata-part addition to the endpoint.

Design spec

Final spec: docs/specs/bulk-upload-split-panel-spec.html — all states (N=0 / N=1 / N≥2), four viewports (320 / 375 / 768 / 1280), both themes (light / dark). Tokens match the real layout.css.

Exploration trail (3 concepts + decision matrix): docs/specs/bulk-upload-concepts.html.

The one-screen model

/documents/new is one route with three visual states. Same DOM skeleton; only three pieces of chrome appear/disappear based on file count:

State PDF panel Form panel File switcher
N = 0 (empty) full-panel drop zone with bulk-first copy shared card, muted / disabled hidden
N = 1 (single) PDF preview title + shared card (byte-identical to #294) hidden
N ≥ 2 (multi) PDF preview + FileSwitcherStrip "Nur diese Datei" card + "Gilt für alle N" card visible

Scope

New frontend components

  • BulkDropZone.svelte — full-panel drop target; renders on N=0 and as "add more" target on N≥1.
  • FileSwitcherStrip.svelte — horizontal chip list + prev/next arrows + aria-live announcements.
  • ScopeCard.svelte — two variants: mint-tinted "Nur diese Datei" (per-file) and neutral "Gilt für alle N" (shared).

Frontend extensions

  • documents/new/+page.svelte — state owner (files array, active index, shared metadata, mode based on files.length).
  • DocumentEditLayout.svelte — accept { files, activeIndex } props; emit select / remove events. Single-file prop shape unchanged.
  • UploadSaveBar.svelte — plural-aware label, determinate progress bar during save.

Backend (small additive change)

  • Extend POST /api/documents/quick-upload to accept an optional metadata JSON part carrying { senderId, receiverId, documentDate, location, tags[], archiveBox, archiveFolder, metadataComplete, titles[] }.
  • Backend applies shared fields to every created Document and maps titles[i]files[i] by index.
  • Response shape unchanged: { created[], updated[], errors[] }.

i18n (Paraglide)

15 new keys in de/en/es — full table in the spec. Notable: bulk_save_cta uses ICU plurals so "Speichern →" / "5 speichern →" / "Guardar 5 →" are all single messages.

Filename → title regex

basename.replace(/\.(pdf|jpe?g|png|tiff?)$/i, '').replace(/[_-]+/g, ' ').trim() — marks the input suggested (mint border, accent-bg) until the user edits it.

Acceptance criteria

  • Single-file invariant: at N=1 the screen is byte-identical to #294. No FileSwitcherStrip, no count pill, no "Nur diese Datei" card, no "Alle verwerfen" link.
  • Drop zone copy is bulk-first: "Eine oder mehrere Dateien ablegen". Explains the title vs. shared semantics up front.
  • N≥2 shows count pill "N werden erstellt", FileSwitcherStrip, two-card form split, save CTA "N speichern →".
  • Title pre-fill derives from filename without extension; user can override per file.
  • One POST saves N documents; per-file failures surface as red-dashed chips with retry.
  • Every interactive element ≥ 44×44 touch target.
  • Active / error chip states use colour and shape and ARIA (WCAG 1.4.1 redundancy).
  • Contrast passes AAA in both themes for the mint/navy combinations (7.2:1 light, 9.2:1 dark).
  • prefers-reduced-motion respected via the existing global @media rule.
  • Works cleanly from 320px upwards — mobile collapses split into "Vorschau / Angaben" tabs (reuses DocumentEditLayout's existing pattern).

Test plan

Unit (Vitest browser)

  • FileSwitcherStrip.svelte.spec.ts: renders N chips, active chip has aria-current="true" + caret prefix, arrow keys cycle focus, error chip renders dashed border + ⚠.
  • ScopeCard.svelte.spec.ts: per-file variant renders mint bg, shared variant renders neutral bg, badge text interpolates {count}.
  • BulkDropZone.svelte.spec.ts: drop of 3 files emits filesAdded with 3 entries; filename-to-title regex applied; multiple attribute on file input.
  • documents/new/page.svelte.spec.ts: at N=1 no switcher renders (lock the invariant); at N=5 switcher + two cards render; changing active index auto-focuses the title input.

E2E (Playwright)

  • new-document-bulk.spec.ts: drop 3 PDFs, fill shared fields, click save → redirect to /documents with a toast listing 3 created docs; verify all 3 have the shared sender/receiver/date in the DB (via API).
  • bulk-upload-a11y.spec.ts: axe sweep at 375 / 1280 in light + dark with N=5 state seeded.

Manual smoke

  • Drop 5 files → save → confirm 5 documents created with matching metadata but distinct titles.
  • Drop 1 file → confirm screen is indistinguishable from current #294 UX.
  • Drop file that fails server-side (duplicate) → chip goes red-dashed, others still create.

Out of scope for v1

  • Per-file overrides beyond the title (sender / date per file) — a future "expand row".
  • Drag-to-reorder files within the batch.
  • Resumable / chunked uploads.
  • Folder upload via webkitdirectory.
  • OCR-based sender/date pre-population.

References

  • Parent issue: #294 (new-document split-panel)
  • Related: #305 (briefwechsel thumbnail rows) — uses the same Paraglide + tokens conventions
  • Backend endpoint today: DocumentController.quickUpload (backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java)
## Context Extends **#294** (new-document split-panel) with bulk uploads. When a user drops N files on `/documents/new`, each becomes its own document; every metadata field is shared across all N except the **title**, which is pre-filled from the filename and editable per file. A single POST to `/api/documents/quick-upload` creates N documents in one pass. The backend already supports batch multipart uploads via `quick-upload` — this is primarily a frontend extension plus a small metadata-part addition to the endpoint. ## Design spec Final spec: [`docs/specs/bulk-upload-split-panel-spec.html`](../src/branch/main/docs/specs/bulk-upload-split-panel-spec.html) — all states (N=0 / N=1 / N≥2), four viewports (320 / 375 / 768 / 1280), both themes (light / dark). Tokens match the real `layout.css`. Exploration trail (3 concepts + decision matrix): [`docs/specs/bulk-upload-concepts.html`](../src/branch/main/docs/specs/bulk-upload-concepts.html). ## The one-screen model `/documents/new` is one route with three visual states. Same DOM skeleton; only three pieces of chrome appear/disappear based on file count: | State | PDF panel | Form panel | File switcher | |---|---|---|---| | **N = 0** (empty) | full-panel drop zone with bulk-first copy | shared card, muted / disabled | hidden | | **N = 1** (single) | PDF preview | title + shared card (byte-identical to #294) | hidden | | **N ≥ 2** (multi) | PDF preview + FileSwitcherStrip | "Nur diese Datei" card + "Gilt für alle N" card | visible | ## Scope ### New frontend components - `BulkDropZone.svelte` — full-panel drop target; renders on N=0 and as "add more" target on N≥1. - `FileSwitcherStrip.svelte` — horizontal chip list + prev/next arrows + aria-live announcements. - `ScopeCard.svelte` — two variants: mint-tinted "Nur diese Datei" (per-file) and neutral "Gilt für alle N" (shared). ### Frontend extensions - `documents/new/+page.svelte` — state owner (files array, active index, shared metadata, mode based on `files.length`). - `DocumentEditLayout.svelte` — accept `{ files, activeIndex }` props; emit `select` / `remove` events. Single-file prop shape unchanged. - `UploadSaveBar.svelte` — plural-aware label, determinate progress bar during save. ### Backend (small additive change) - Extend `POST /api/documents/quick-upload` to accept an optional `metadata` JSON part carrying `{ senderId, receiverId, documentDate, location, tags[], archiveBox, archiveFolder, metadataComplete, titles[] }`. - Backend applies shared fields to every created Document and maps `titles[i]` → `files[i]` by index. - Response shape unchanged: `{ created[], updated[], errors[] }`. ### i18n (Paraglide) 15 new keys in de/en/es — full table in the spec. Notable: `bulk_save_cta` uses ICU plurals so "Speichern →" / "5 speichern →" / "Guardar 5 →" are all single messages. ### Filename → title regex `basename.replace(/\.(pdf|jpe?g|png|tiff?)$/i, '').replace(/[_-]+/g, ' ').trim()` — marks the input `suggested` (mint border, accent-bg) until the user edits it. ## Acceptance criteria - **Single-file invariant**: at N=1 the screen is byte-identical to #294. No FileSwitcherStrip, no count pill, no "Nur diese Datei" card, no "Alle verwerfen" link. - Drop zone copy is bulk-first: "Eine oder mehrere Dateien ablegen". Explains the title vs. shared semantics up front. - N≥2 shows count pill "N werden erstellt", FileSwitcherStrip, two-card form split, save CTA "N speichern →". - Title pre-fill derives from filename without extension; user can override per file. - One POST saves N documents; per-file failures surface as red-dashed chips with retry. - Every interactive element ≥ 44×44 touch target. - Active / error chip states use colour **and** shape **and** ARIA (WCAG 1.4.1 redundancy). - Contrast passes AAA in both themes for the mint/navy combinations (7.2:1 light, 9.2:1 dark). - `prefers-reduced-motion` respected via the existing global @media rule. - Works cleanly from 320px upwards — mobile collapses split into "Vorschau / Angaben" tabs (reuses DocumentEditLayout's existing pattern). ## Test plan ### Unit (Vitest browser) - `FileSwitcherStrip.svelte.spec.ts`: renders N chips, active chip has `aria-current="true"` + caret prefix, arrow keys cycle focus, error chip renders dashed border + ⚠. - `ScopeCard.svelte.spec.ts`: per-file variant renders mint bg, shared variant renders neutral bg, badge text interpolates `{count}`. - `BulkDropZone.svelte.spec.ts`: drop of 3 files emits `filesAdded` with 3 entries; filename-to-title regex applied; `multiple` attribute on file input. - `documents/new/page.svelte.spec.ts`: at N=1 no switcher renders (lock the invariant); at N=5 switcher + two cards render; changing active index auto-focuses the title input. ### E2E (Playwright) - `new-document-bulk.spec.ts`: drop 3 PDFs, fill shared fields, click save → redirect to `/documents` with a toast listing 3 created docs; verify all 3 have the shared sender/receiver/date in the DB (via API). - `bulk-upload-a11y.spec.ts`: axe sweep at 375 / 1280 in light + dark with N=5 state seeded. ### Manual smoke - Drop 5 files → save → confirm 5 documents created with matching metadata but distinct titles. - Drop 1 file → confirm screen is indistinguishable from current #294 UX. - Drop file that fails server-side (duplicate) → chip goes red-dashed, others still create. ## Out of scope for v1 - Per-file overrides beyond the title (sender / date per file) — a future "expand row". - Drag-to-reorder files within the batch. - Resumable / chunked uploads. - Folder upload via `webkitdirectory`. - OCR-based sender/date pre-population. ## References - Parent issue: #294 (new-document split-panel) - Related: #305 (briefwechsel thumbnail rows) — uses the same Paraglide + tokens conventions - Backend endpoint today: `DocumentController.quickUpload` (`backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java`)
marcel added the featureui labels 2026-04-24 10:33:37 +02:00
Author
Owner

🏛️ Markus Keller — Senior Application Architect

Observations

  • Extending an existing endpoint instead of creating a new one fits the "simplest tool that works" principle — good call.
  • Modelling N=0 / N=1 / N≥2 as one route with one state owner (not three pages) keeps feature-package boundaries clean.
  • DocumentEditLayout.svelte currently requires a persisted Doc with an id and calls /api/documents/{id}/file in an $effect to load bytes. The spec says "accept { files, activeIndex } props" — that makes the component polymorphic across two fundamentally different lifecycle stages (pre-upload vs post-upload). Coupling rises; the single-file contract gets noisier.
  • The metadata JSON part carries titles[] matched by index to files[]. That is an implicit wire coupling — frontend and backend must agree on ordering, and the spec does not address length mismatch.
  • Blocker for "byte-identical to #294": #294 is still open. The parent issue has an unresolved design question (placeholder-first vs pre-upload mode). You can't promise parity with a layout that hasn't shipped. Order of delivery matters here.

Recommendations

  • Do not retrofit DocumentEditLayout.svelte with { files, activeIndex }. Introduce BulkDocumentEditLayout.svelte as a new wrapper that uses DocumentEditLayout internally for the preview region and owns the switcher + scope cards itself. The single-file prop shape stays untouched — which is also the cleanest way to lock the "no regression for N=1" invariant at the type level, not at the snapshot-test level.
  • Ship #294 first (or rebase #317 on top of a branch where #294's decision is baked in). Otherwise half the spec is aspirational.
  • Make the new metadata part a typed DocumentBatchMetadataDTO in the backend — @RequestPart("metadata") DocumentBatchMetadataDTO metadata. Spring deserializes JSON to the DTO, OpenAPI codegen gives the frontend a typed shape, no ad-hoc map lookups.
  • Enforce titles.size() <= files.size() server-side; return 400 with a structured ErrorCode.INVALID_INPUT. Silent truncation hides bugs.
  • No new Flyway migration is needed (confirmed — shared fields map onto existing columns). Keep it that way.

Open Decisions

  • Overload /quick-upload vs new /api/documents/batch endpoint — Current design overloads one endpoint to dispatch on presence of the metadata part, serving both the (existing) drag-anywhere tooling and the new UI. A separate endpoint would split contracts cleanly. Tradeoff: overload keeps URL surface small and doesn't deprecate; split gives each caller a typed schema. I lean overload iff the metadata part is a typed DTO (not an opaque blob) — otherwise split.
## 🏛️ Markus Keller — Senior Application Architect ### Observations - Extending an existing endpoint instead of creating a new one fits the "simplest tool that works" principle — good call. - Modelling N=0 / N=1 / N≥2 as one route with one state owner (not three pages) keeps feature-package boundaries clean. - `DocumentEditLayout.svelte` currently requires a persisted `Doc` with an `id` and calls `/api/documents/{id}/file` in an `$effect` to load bytes. The spec says "accept `{ files, activeIndex }` props" — that makes the component polymorphic across two fundamentally different lifecycle stages (pre-upload vs post-upload). Coupling rises; the single-file contract gets noisier. - The `metadata` JSON part carries `titles[]` matched by index to `files[]`. That is an implicit wire coupling — frontend and backend must agree on ordering, and the spec does not address length mismatch. - **Blocker for "byte-identical to #294"**: #294 is still `open`. The parent issue has an unresolved design question (placeholder-first vs pre-upload mode). You can't promise parity with a layout that hasn't shipped. Order of delivery matters here. ### Recommendations - Do **not** retrofit `DocumentEditLayout.svelte` with `{ files, activeIndex }`. Introduce `BulkDocumentEditLayout.svelte` as a new wrapper that uses `DocumentEditLayout` internally for the preview region and owns the switcher + scope cards itself. The single-file prop shape stays untouched — which is also the cleanest way to lock the "no regression for N=1" invariant at the type level, not at the snapshot-test level. - Ship #294 first (or rebase #317 on top of a branch where #294's decision is baked in). Otherwise half the spec is aspirational. - Make the new `metadata` part a typed `DocumentBatchMetadataDTO` in the backend — `@RequestPart("metadata") DocumentBatchMetadataDTO metadata`. Spring deserializes JSON to the DTO, OpenAPI codegen gives the frontend a typed shape, no ad-hoc map lookups. - Enforce `titles.size() <= files.size()` server-side; return 400 with a structured `ErrorCode.INVALID_INPUT`. Silent truncation hides bugs. - No new Flyway migration is needed (confirmed — shared fields map onto existing columns). Keep it that way. ### Open Decisions - **Overload `/quick-upload` vs new `/api/documents/batch` endpoint** — Current design overloads one endpoint to dispatch on presence of the `metadata` part, serving both the (existing) drag-anywhere tooling and the new UI. A separate endpoint would split contracts cleanly. Tradeoff: overload keeps URL surface small and doesn't deprecate; split gives each caller a typed schema. I lean overload *iff* the `metadata` part is a typed DTO (not an opaque blob) — otherwise split.
Author
Owner

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

  • Current /documents/new/+page.svelte is not the split-panel layout — it uses FileSectionNew.svelte + a collapsible <details>. "Byte-identical to #294 at N=1" is only meaningful once #294 ships.
  • Spec factual error: UploadSaveBar.svelte is marked as "already exists for single-file — extend". It does not exist in the repo (find frontend/src -name UploadSaveBar* returns nothing). This is a net-new component.
  • DocumentEditLayout.svelte requires doc: Doc with an id and calls fetch('/api/documents/${doc.id}/file') in an $effect (see lines 44–63). Passing { files, activeIndex } as the spec suggests breaks that contract.
  • parseFilename() and stripExtension() already live in $lib/utils/filename.ts and are already reused by the current new-doc flow. The filename-to-title regex in the issue (.replace(/\.(pdf|jpe?g|png|tiff?)$/i, '').replace(/[_-]+/g, ' ').trim()) subtly diverges from parseFilename's fallback — that's a drift risk.
  • Current N=1 flow POSTs to /api/documents and redirects to /documents/{id}. The bulk flow POSTs to /api/documents/quick-upload and redirects to /documents with a toast. Even at N=1 the wire path and post-save navigation differ — treat this as an intentional behavior change, not a "zero regression" claim.

Recommendations

  • Write failing tests first, in this order (red/green/refactor — every step):
    1. documents/new/page.svelte.spec.ts → at N=1 no FileSwitcherStrip, no count pill, no "Alle verwerfen", no "Nur diese Datei" card — asserts the invariant by the absence of test ids, not by snapshot.
    2. FileSwitcherStrip.svelte.spec.ts → N chips, active has aria-current="true" + caret, ArrowLeft/ArrowRight cycle, Enter/Space select, error chip has dashed border + ⚠.
    3. ScopeCard.svelte.spec.ts{count} interpolation; per-file variant uses bg-accent-bg border-accent, shared uses bg-muted border-line.
    4. BulkDropZone.svelte.spec.ts → drop 3 files emits filesAdded with 3 entries; <input multiple> is set.
    5. DocumentControllerTest.quickUpload_appliesSharedMetadata_toAllCreatedDocuments().
    6. DocumentControllerTest.quickUpload_mapsTitlesByIndex_whenCountsMatch() and …_rejects400_whenTitlesLongerThanFiles().
  • Reuse parseFilename() / stripExtension() — do not inline the regex. If the spec's regex is intentionally different, make that explicit by adding a named helper next to the existing ones so the two live side-by-side, not silently.
  • Create BulkDocumentEditLayout.svelte as a sibling; don't mutate DocumentEditLayout's prop shape (see Markus).
  • Files collection → SvelteMap<string, FileEntry> keyed by a generated id (e.g. crypto.randomUUID()). Arrays break when you remove the middle entry — active index and per-file state drift. Then Array.from(files.values()) for rendering.
  • Keyed {#each files.values() as f (f.id)}. Without a stable key, Svelte reconciles by position and local state corrupts on removal.
  • $derived for activeFile, isMulti = files.size >= 2, saveLabel = m.bulk_save_cta({ count: files.size }). Never $state + $effect to compute a derived value.
  • Request a DTO on the backend so the generated TypeScript types include DocumentBatchMetadata — frontend gets compile-time shape checks without hand-maintaining it.

Open Decisions (none — these are grounded in the current code and project conventions.)

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - Current `/documents/new/+page.svelte` is **not** the split-panel layout — it uses `FileSectionNew.svelte` + a collapsible `<details>`. "Byte-identical to #294 at N=1" is only meaningful once #294 ships. - **Spec factual error**: `UploadSaveBar.svelte` is marked as "already exists for single-file — extend". It does not exist in the repo (`find frontend/src -name UploadSaveBar*` returns nothing). This is a net-new component. - `DocumentEditLayout.svelte` requires `doc: Doc` with an `id` and calls `fetch('/api/documents/${doc.id}/file')` in an `$effect` (see lines 44–63). Passing `{ files, activeIndex }` as the spec suggests breaks that contract. - `parseFilename()` and `stripExtension()` already live in `$lib/utils/filename.ts` and are already reused by the current new-doc flow. The filename-to-title regex in the issue (`.replace(/\.(pdf|jpe?g|png|tiff?)$/i, '').replace(/[_-]+/g, ' ').trim()`) subtly diverges from `parseFilename`'s fallback — that's a drift risk. - Current N=1 flow POSTs to `/api/documents` and redirects to `/documents/{id}`. The bulk flow POSTs to `/api/documents/quick-upload` and redirects to `/documents` with a toast. Even at N=1 the wire path and post-save navigation differ — treat this as an intentional behavior change, not a "zero regression" claim. ### Recommendations - Write failing tests first, in this order (red/green/refactor — every step): 1. `documents/new/page.svelte.spec.ts` → at N=1 **no** `FileSwitcherStrip`, **no** count pill, **no** "Alle verwerfen", **no** "Nur diese Datei" card — asserts the invariant by the absence of test ids, not by snapshot. 2. `FileSwitcherStrip.svelte.spec.ts` → N chips, active has `aria-current="true"` + caret, `ArrowLeft`/`ArrowRight` cycle, `Enter`/`Space` select, error chip has dashed border + ⚠. 3. `ScopeCard.svelte.spec.ts` → `{count}` interpolation; per-file variant uses `bg-accent-bg border-accent`, shared uses `bg-muted border-line`. 4. `BulkDropZone.svelte.spec.ts` → drop 3 files emits `filesAdded` with 3 entries; `<input multiple>` is set. 5. `DocumentControllerTest.quickUpload_appliesSharedMetadata_toAllCreatedDocuments()`. 6. `DocumentControllerTest.quickUpload_mapsTitlesByIndex_whenCountsMatch()` and `…_rejects400_whenTitlesLongerThanFiles()`. - **Reuse `parseFilename()` / `stripExtension()`** — do not inline the regex. If the spec's regex is intentionally different, make that explicit by adding a named helper next to the existing ones so the two live side-by-side, not silently. - Create `BulkDocumentEditLayout.svelte` as a sibling; don't mutate `DocumentEditLayout`'s prop shape (see Markus). - Files collection → `SvelteMap<string, FileEntry>` keyed by a generated id (e.g. `crypto.randomUUID()`). Arrays break when you remove the middle entry — active index and per-file state drift. Then `Array.from(files.values())` for rendering. - Keyed `{#each files.values() as f (f.id)}`. Without a stable key, Svelte reconciles by position and local state corrupts on removal. - `$derived` for `activeFile`, `isMulti = files.size >= 2`, `saveLabel = m.bulk_save_cta({ count: files.size })`. Never `$state` + `$effect` to compute a derived value. - Request a DTO on the backend so the generated TypeScript types include `DocumentBatchMetadata` — frontend gets compile-time shape checks without hand-maintaining it. ### Open Decisions _(none — these are grounded in the current code and project conventions.)_
Author
Owner

🔐 Nora "NullX" Steiner — Application Security Engineer

Observations

  • Authorization is clean: /api/documents/quick-upload is already @RequirePermission(Permission.WRITE_ALL). Only writers can hit it — good.
  • Content-type whitelist (ALLOWED_CONTENT_TYPES) is enforced server-side today in DocumentController.quickUpload (line 201). The metadata-aware variant must preserve that check on every file in the batch.
  • Blind spot — request size: application.yaml sets spring.servlet.multipart.max-file-size: 50MB but not max-request-size. Spring's default total cap is 10MB. A 3-file batch at 20MB each fails with an opaque MaxUploadSizeExceededException before your controller sees the request. It is also the path by which a writer account could drive memory pressure by sending many large parts (multipart parsing buffers before the limit is checked per-file).
  • titles[] strings are user-controlled and flow into Document.title. Svelte's template escaping is the primary XSS defense — make sure title is never injected into innerHTML, email templates, or notification bodies without encoding.
  • Filenames flow back into UploadError.filename. Fine for client-rendered error lists (Svelte escapes). Verify all backend logging uses SLF4J's {} placeholder, not string concatenation — a malicious filename like ${jndi:ldap://…} should never end up in a log.info("..." + filename) call.

Recommendations

  • Config fix (blocker) — add to backend/src/main/resources/application.yaml:
    spring:
      servlet:
        multipart:
          max-file-size: 50MB
          max-request-size: 500MB   # matches ~10-file batch at documented per-file cap
          file-size-threshold: 2KB
    
    Without this the bulk feature silently caps at 10MB total.
  • Hard cap on batch count — in quickUpload add if (files.size() > 50) throw DomainException.badRequest(ErrorCode.BATCH_TOO_LARGE, "…"). Otherwise a writer can push 500× 1KB files and force O(N) DB round-trips.
  • Add ErrorCode.BATCH_TOO_LARGE (ErrorCode.java), mirror in frontend/src/lib/errors.ts, add the Paraglide keys in de/en/es.json. Wire into getErrorMessage() so the user sees "Zu viele Dateien auf einmal — bitte in Blöcken hochladen" (de), not a stack trace.
  • Regression tests (permanent in the suite — fail-close is non-negotiable):
    @Test void quickUpload_returns413_whenTotalRequestExceedsMaxRequestSize();
    @Test void quickUpload_returns400_whenBatchExceedsMaxCount();
    @Test void quickUpload_rejectsUnsupportedContentType_inMultiFileBatch();
    @Test void quickUpload_withMetadata_doesNotBypassContentTypeWhitelist();
    
  • Typed DTO for the metadata part@RequestPart("metadata") DocumentBatchMetadataDTO metadata, not Map<String, Object>. Mass-assignment protection by construction — unknown fields are ignored, no reflection-based setter discovery.
  • Structured logginglog.info("quickUpload actor={} files={} totalBytes={}", actorId, files.size(), totalBytes). Parameterized, Log4Shell-safe, useful for audit.

Open Decisions (none — these are defense-in-depth additions to an already auth-gated endpoint.)

## 🔐 Nora "NullX" Steiner — Application Security Engineer ### Observations - Authorization is clean: `/api/documents/quick-upload` is already `@RequirePermission(Permission.WRITE_ALL)`. Only writers can hit it — good. - Content-type whitelist (`ALLOWED_CONTENT_TYPES`) is enforced server-side today in `DocumentController.quickUpload` (line 201). The metadata-aware variant must preserve that check on every file in the batch. - **Blind spot — request size**: `application.yaml` sets `spring.servlet.multipart.max-file-size: 50MB` but **not** `max-request-size`. Spring's default total cap is 10MB. A 3-file batch at 20MB each fails with an opaque `MaxUploadSizeExceededException` before your controller sees the request. It is also the path by which a writer account could drive memory pressure by sending many large parts (multipart parsing buffers before the limit is checked per-file). - `titles[]` strings are user-controlled and flow into `Document.title`. Svelte's template escaping is the primary XSS defense — make sure `title` is never injected into `innerHTML`, email templates, or notification bodies without encoding. - Filenames flow back into `UploadError.filename`. Fine for client-rendered error lists (Svelte escapes). Verify all backend logging uses SLF4J's `{}` placeholder, not string concatenation — a malicious filename like `${jndi:ldap://…}` should never end up in a `log.info("..." + filename)` call. ### Recommendations - **Config fix (blocker)** — add to `backend/src/main/resources/application.yaml`: ```yaml spring: servlet: multipart: max-file-size: 50MB max-request-size: 500MB # matches ~10-file batch at documented per-file cap file-size-threshold: 2KB ``` Without this the bulk feature silently caps at 10MB total. - **Hard cap on batch count** — in `quickUpload` add `if (files.size() > 50) throw DomainException.badRequest(ErrorCode.BATCH_TOO_LARGE, "…")`. Otherwise a writer can push 500× 1KB files and force O(N) DB round-trips. - Add `ErrorCode.BATCH_TOO_LARGE` (`ErrorCode.java`), mirror in `frontend/src/lib/errors.ts`, add the Paraglide keys in `de/en/es.json`. Wire into `getErrorMessage()` so the user sees "Zu viele Dateien auf einmal — bitte in Blöcken hochladen" (de), not a stack trace. - **Regression tests** (permanent in the suite — fail-close is non-negotiable): ```java @Test void quickUpload_returns413_whenTotalRequestExceedsMaxRequestSize(); @Test void quickUpload_returns400_whenBatchExceedsMaxCount(); @Test void quickUpload_rejectsUnsupportedContentType_inMultiFileBatch(); @Test void quickUpload_withMetadata_doesNotBypassContentTypeWhitelist(); ``` - **Typed DTO for the metadata part** — `@RequestPart("metadata") DocumentBatchMetadataDTO metadata`, not `Map<String, Object>`. Mass-assignment protection by construction — unknown fields are ignored, no reflection-based setter discovery. - **Structured logging** — `log.info("quickUpload actor={} files={} totalBytes={}", actorId, files.size(), totalBytes)`. Parameterized, Log4Shell-safe, useful for audit. ### Open Decisions _(none — these are defense-in-depth additions to an already auth-gated endpoint.)_
Author
Owner

🧪 Sara Holt — Senior QA Engineer

Observations

  • The unit + E2E plan in the issue is thorough on the frontend. The backend test coverage for the new behavior is under-specified — there is no explicit test for metadata propagation, index-based title mapping, or partial-failure batches.
  • "Byte-identical to #294" via snapshot is brittle. Snapshot diffs fire on harmless DOM structure changes and miss real invariant violations (e.g. a display:none FileSwitcherStrip passes a snapshot but breaks tab order). Structural assertions are stronger.
  • Navigation regression risk — current single-file /documents/new redirects to /documents/{id}. The new bulk flow redirects to /documents with a toast, even at N=1. That's a behavioral change from today's UX baseline. If intentional, existing E2E new-document.spec.ts must be updated explicitly; if not, it is a silent regression.
  • Axe sweep at 375 and 1280 — misses our standing 320px baseline that Leonie enforces on every spec. Must include 320.
  • No test for the beforeunload protection described in the spec.
  • Existing quickUpload tests in DocumentControllerTest cover auth, content-type, duplicate-filename-goes-to-updated. None of them cover metadata — that gap must close before ship.

Recommendations

  • Backend DocumentControllerTest additions:
    • quickUpload_withMetadata_appliesSharedFieldsToAllCreatedDocuments() — 3 files in, all 3 created Documents share the sender/date/tags.
    • quickUpload_withMetadata_mapsTitlesByIndex()titles=["A","B","C"], files=[a.pdf,b.pdf,c.pdf], assert titles land correctly.
    • quickUpload_withMetadata_rejects400_whenTitlesSizeExceedsFilesSize().
    • quickUpload_withMetadata_appliesToUpdatedDocuments() or …doesNotOverrideMetadataOfMatchedPlaceholder()decide the semantics first, then lock with a test. Currently storeDocument matches by filename and returns in updated[] — unclear if shared metadata should merge into an existing doc.
    • quickUpload_withMetadata_createsPartial_whenOneFileFails() — 3 files, 1 unsupported content-type → 2 created + 1 error row, metadata applied to the 2 successes.
  • Replace the N=1 snapshot with explicit structural assertions:
    expect(queryByTestId('file-switcher-strip')).toBeNull();
    expect(queryByTestId('scope-card-per-file')).toBeNull();
    expect(queryByTestId('discard-all-link')).toBeNull();
    
    Stable against cosmetic refactors; strict on behavior.
  • Extend axe matrix to [320, 375, 768, 1280] × [light, dark] × [N=0, N=1, N=5]. Budget: ~24 runs × ~6s = well under the 8-min E2E cap.
  • Add Playwright E2E new-document-bulk.spec.ts: drop 3 PDFs → shared metadata → save → assert 3 Documents visible in the list view (user-facing path, not DB).
  • beforeunload regression test (Playwright): shared form dirty → attempt nav → page.on('dialog', …) asserts the prompt fires; explicit "Alle verwerfen" → no prompt.
  • Use Testcontainers postgres:16-alpine for the integration additions — never H2.

Open Decisions (none — all additions, no tradeoffs.)

## 🧪 Sara Holt — Senior QA Engineer ### Observations - The unit + E2E plan in the issue is thorough on the frontend. The backend test coverage for the new behavior is under-specified — there is no explicit test for metadata propagation, index-based title mapping, or partial-failure batches. - "Byte-identical to #294" via snapshot is brittle. Snapshot diffs fire on harmless DOM structure changes and miss real invariant violations (e.g. a `display:none` FileSwitcherStrip passes a snapshot but breaks tab order). Structural assertions are stronger. - **Navigation regression risk** — current single-file `/documents/new` redirects to `/documents/{id}`. The new bulk flow redirects to `/documents` with a toast, even at N=1. That's a behavioral change from today's UX baseline. If intentional, existing E2E `new-document.spec.ts` must be updated explicitly; if not, it is a silent regression. - Axe sweep at 375 and 1280 — misses our standing 320px baseline that Leonie enforces on every spec. Must include 320. - No test for the `beforeunload` protection described in the spec. - Existing `quickUpload` tests in `DocumentControllerTest` cover auth, content-type, duplicate-filename-goes-to-updated. None of them cover metadata — that gap must close before ship. ### Recommendations - **Backend `DocumentControllerTest` additions**: - `quickUpload_withMetadata_appliesSharedFieldsToAllCreatedDocuments()` — 3 files in, all 3 created `Document`s share the sender/date/tags. - `quickUpload_withMetadata_mapsTitlesByIndex()` — `titles=["A","B","C"]`, `files=[a.pdf,b.pdf,c.pdf]`, assert titles land correctly. - `quickUpload_withMetadata_rejects400_whenTitlesSizeExceedsFilesSize()`. - `quickUpload_withMetadata_appliesToUpdatedDocuments()` or `…doesNotOverrideMetadataOfMatchedPlaceholder()` — **decide the semantics first**, then lock with a test. Currently `storeDocument` matches by filename and returns in `updated[]` — unclear if shared metadata should merge into an existing doc. - `quickUpload_withMetadata_createsPartial_whenOneFileFails()` — 3 files, 1 unsupported content-type → 2 created + 1 error row, metadata applied to the 2 successes. - **Replace the N=1 snapshot** with explicit structural assertions: ```typescript expect(queryByTestId('file-switcher-strip')).toBeNull(); expect(queryByTestId('scope-card-per-file')).toBeNull(); expect(queryByTestId('discard-all-link')).toBeNull(); ``` Stable against cosmetic refactors; strict on behavior. - **Extend axe matrix** to `[320, 375, 768, 1280]` × `[light, dark]` × `[N=0, N=1, N=5]`. Budget: ~24 runs × ~6s = well under the 8-min E2E cap. - **Add Playwright E2E** `new-document-bulk.spec.ts`: drop 3 PDFs → shared metadata → save → assert 3 `Document`s visible in the list view (user-facing path, not DB). - **beforeunload regression test** (Playwright): shared form dirty → attempt nav → `page.on('dialog', …)` asserts the prompt fires; explicit "Alle verwerfen" → no prompt. - Use Testcontainers `postgres:16-alpine` for the integration additions — never H2. ### Open Decisions _(none — all additions, no tradeoffs.)_
Author
Owner

🎨 Leonie Voss — UX & Accessibility Lead

Observations

  • The spec is thorough where it counts: AAA contrast verified in both themes (7.2:1 light / 9.2:1 dark for mint + navy), 44×44 touch targets, redundant cues on state (active chip = color + caret + aria-current; error chip = color + dashed border + ⚠).
  • Rejecting role="tablist" in favor of a visually-hidden label + aria-live is the right AT pattern — chips here aren't tabs for tabbed content, they're a selection list.
  • prefers-reduced-motion respected via the global @media rule — good.
  • Concern — below minimum font size at 320px: the empty-state mockup shrinks drop-sub to 8.5px. The mock is scaled (~55%) but if a developer translates scaled pixels literally, it ships too small. Our floor is 12px.
  • Concern — 1/N tab pill overflow: at 320px in Spanish ("Vista previa" + pill), 2-digit counts (12/99) may wrap the tab label. Worth specifying tabular-nums and a minimum pill width.
  • Concern — suggested state ambiguity for seniors: mint border + 15% accent-bg on the title input reads as AAA contrast, but visually it can feel like a disabled state to users unfamiliar with the pattern. No helper text tells them "tap to replace with your own".
  • The "Alle verwerfen" destructive link uses text-danger — in dark mode some token sets don't hit 7:1 on navy surface. Must verify.
  • Auto-focus on file switch is called out as always-on. That's correct for mouse/touch but loses focus out of the keyboard cycling path — every arrow press jumps focus back to a text input, forcing the user to re-tab back into the strip to continue navigating.

Recommendations

  • Floor the drop-zone copy at 12px (text-xs) on 320px. Do not translate the 8.5px mockup value. Annotate the impl-ref to say "scaled 8.5px → ship as text-xs / 12px".
  • Stabilize the 1/N pill: tabular-nums min-w-[2.5rem] text-center. Keeps the pill width constant from 1/1 through 99/99 so the tab label doesn't jitter.
  • Add helper text under the suggested title input: a single line text-xs text-ink-3 like "Vorschlag aus Dateiname — zum Bearbeiten anklicken", auto-dismissed on first user edit. Costs nothing; removes the "is this greyed out?" confusion for seniors.
  • Verify text-danger contrast in dark mode against the dark surface token. If the Alle-verwerfen link doesn't hit 7:1, switch to text-danger-fg (or bump the weight to font-bold so the AA large-text rule applies at 3:1).
  • Reduced-motion + determinate progress: the fill still needs to update visibly (user needs feedback) but swap transition-[width] for no transition under prefers-reduced-motion: reduce. The width still animates via JS state change; the CSS easing is what gets dropped.
  • Focus on remove-X: the spec keeps X outside the chip's clickable hitbox — good. The X itself still needs focus-visible:ring-2 focus-visible:ring-danger so keyboard users can see where they are.

Open Decisions

  • Auto-focus policy on file switch — Spec currently says "always auto-focus title input on switch". That works for mouse/touch (title is the only per-file field, deserves priority). But for keyboard users cycling / in the strip, every arrow press ejects focus out of the strip. Options: (a) auto-focus only on mouse/touch selection, keep focus in strip on arrow navigation; (b) always auto-focus; (c) never auto-focus — user opts in by clicking the input. I lean (a) — matches both mental models — but confirm before implementation.
## 🎨 Leonie Voss — UX & Accessibility Lead ### Observations - The spec is thorough where it counts: AAA contrast verified in both themes (7.2:1 light / 9.2:1 dark for mint + navy), 44×44 touch targets, redundant cues on state (active chip = color + caret + `aria-current`; error chip = color + dashed border + ⚠). - Rejecting `role="tablist"` in favor of a visually-hidden label + `aria-live` is the right AT pattern — chips here aren't tabs for tabbed content, they're a selection list. - `prefers-reduced-motion` respected via the global `@media` rule — good. - **Concern — below minimum font size at 320px**: the empty-state mockup shrinks `drop-sub` to `8.5px`. The mock is scaled (~55%) but if a developer translates scaled pixels literally, it ships too small. Our floor is 12px. - **Concern — `1/N` tab pill overflow**: at 320px in Spanish ("Vista previa" + pill), 2-digit counts (`12/99`) may wrap the tab label. Worth specifying `tabular-nums` and a minimum pill width. - **Concern — `suggested` state ambiguity for seniors**: mint border + 15% accent-bg on the title input reads as AAA contrast, but visually it can feel like a disabled state to users unfamiliar with the pattern. No helper text tells them "tap to replace with your own". - The "Alle verwerfen" destructive link uses `text-danger` — in dark mode some token sets don't hit 7:1 on navy surface. Must verify. - Auto-focus on file switch is called out as always-on. That's correct for mouse/touch but loses focus out of the keyboard cycling path — every arrow press jumps focus back to a text input, forcing the user to re-tab back into the strip to continue navigating. ### Recommendations - **Floor the drop-zone copy at 12px** (`text-xs`) on 320px. Do not translate the 8.5px mockup value. Annotate the `impl-ref` to say "scaled 8.5px → ship as `text-xs` / 12px". - **Stabilize the `1/N` pill**: `tabular-nums min-w-[2.5rem] text-center`. Keeps the pill width constant from `1/1` through `99/99` so the tab label doesn't jitter. - **Add helper text under the `suggested` title input**: a single line `text-xs text-ink-3` like "Vorschlag aus Dateiname — zum Bearbeiten anklicken", auto-dismissed on first user edit. Costs nothing; removes the "is this greyed out?" confusion for seniors. - **Verify `text-danger` contrast in dark mode** against the dark surface token. If the Alle-verwerfen link doesn't hit 7:1, switch to `text-danger-fg` (or bump the weight to `font-bold` so the AA large-text rule applies at 3:1). - **Reduced-motion + determinate progress**: the fill still needs to update visibly (user needs feedback) but swap `transition-[width]` for no transition under `prefers-reduced-motion: reduce`. The width still animates via JS state change; the CSS easing is what gets dropped. - **Focus on remove-X**: the spec keeps X outside the chip's clickable hitbox — good. The X itself still needs `focus-visible:ring-2 focus-visible:ring-danger` so keyboard users can see where they are. ### Open Decisions - **Auto-focus policy on file switch** — Spec currently says "always auto-focus title input on switch". That works for mouse/touch (title is the only per-file field, deserves priority). But for keyboard users cycling `←`/`→` in the strip, every arrow press ejects focus out of the strip. Options: (a) auto-focus only on mouse/touch selection, keep focus in strip on arrow navigation; (b) always auto-focus; (c) never auto-focus — user opts in by clicking the input. I lean (a) — matches both mental models — but confirm before implementation.
Author
Owner

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

  • Blocker — multipart request limit: application.yaml sets max-file-size: 50MB but not max-request-size. Spring's default total cap is 10MB. A 3-file batch at 20MB each = 60MB total → rejected with a 413 before your controller runs. The feature cannot meet its documented "50 MB pro Datei × N" promise without this config.
  • Reverse proxy limit — Caddy/nginx in production must also accept the larger body. Not in the repo but belongs in the rollout checklist.
  • No observability on /quick-upload — no structured log of batch size, total bytes, or durations. For a new bulk feature with user-controlled N we'll diagnose "uploads feel slow" by guessing unless we instrument upfront.
  • Thumbnail fan-out — backend calls thumbnailAsyncRunner.dispatchAfterCommit per document. A 20-file batch = 20 async jobs. If AsyncConfig's thumbnail executor has a bounded queue and the default abort policy, some thumbnails get dropped silently under load.
  • Rate limitingRateLimitInterceptor exists but I don't see /quick-upload in the per-endpoint rules. Without a limit, a writer can hammer the batch endpoint; accidental double-click on the save button submits N files twice.
  • Backups — 20-file batches = 20 S3 objects per session. Existing backup strategy handles this fine. No action needed, just noted.

Recommendations

  • Config (blocker, ship in the same PR):
    spring:
      servlet:
        multipart:
          max-file-size: 50MB
          max-request-size: 500MB   # ~10 files at documented per-file max
          file-size-threshold: 2KB
    
    Annotate in a comment: "matches 10-file batch at 50 MB max; see #317". Lower is safer than higher — widen on telemetry, not speculation.
  • Structured logging on the controller:
    log.info("quickUpload actor={} files={} totalBytes={} withMetadata={} created={} updated={} errors={}",
        actorId, files.size(), totalBytes, metadata != null,
        result.created().size(), result.updated().size(), result.errors().size());
    
    Parameterized SLF4J (Log4Shell-safe). Loki queries become trivial.
  • Metrics (Micrometer, already wired via Actuator):
    • documents.quickupload.batch.size — distribution summary
    • documents.quickupload.errors — counter tagged by code
    • documents.quickupload.duration — timer
      Grafana panel follow-up — one dashboard card each.
  • Thumbnail executor — check AsyncConfig.thumbnailExecutor queue capacity and rejection policy. For bursty batches, CallerRunsPolicy means the quick-upload request blocks briefly instead of silently losing thumbnails. Confirm before ship.
  • Rate limit /quick-upload at 5 requests/minute per user. A genuine drag-and-drop workflow doesn't exceed this; double-clicks and programmatic abuse do.
  • Runbook entry in docs/infrastructure/production-compose.md: "If users report '413 Payload Too Large' on bulk upload, check both Spring max-request-size and Caddy request_body { max_size }. They must match."

Open Decisions

  • Request size cap (500MB vs tighter vs looser) — 500MB covers 10 files at max per-file size with headroom. Tighter (e.g. 300MB / 6 files) bounds memory and parse time more aggressively; looser (1GB+) enables power users but widens the DoS surface for a compromised writer account. I lean 500MB for v1 and widen on feedback — zero-telemetry sizing in either direction is a guess.
## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations - **Blocker — multipart request limit**: `application.yaml` sets `max-file-size: 50MB` but **not** `max-request-size`. Spring's default total cap is 10MB. A 3-file batch at 20MB each = 60MB total → rejected with a 413 before your controller runs. The feature cannot meet its documented "50 MB pro Datei × N" promise without this config. - **Reverse proxy limit** — Caddy/nginx in production must also accept the larger body. Not in the repo but belongs in the rollout checklist. - **No observability** on `/quick-upload` — no structured log of batch size, total bytes, or durations. For a new bulk feature with user-controlled N we'll diagnose "uploads feel slow" by guessing unless we instrument upfront. - **Thumbnail fan-out** — backend calls `thumbnailAsyncRunner.dispatchAfterCommit` per document. A 20-file batch = 20 async jobs. If `AsyncConfig`'s thumbnail executor has a bounded queue and the default abort policy, some thumbnails get dropped silently under load. - **Rate limiting** — `RateLimitInterceptor` exists but I don't see `/quick-upload` in the per-endpoint rules. Without a limit, a writer can hammer the batch endpoint; accidental double-click on the save button submits N files twice. - **Backups** — 20-file batches = 20 S3 objects per session. Existing backup strategy handles this fine. No action needed, just noted. ### Recommendations - **Config (blocker, ship in the same PR)**: ```yaml spring: servlet: multipart: max-file-size: 50MB max-request-size: 500MB # ~10 files at documented per-file max file-size-threshold: 2KB ``` Annotate in a comment: "matches 10-file batch at 50 MB max; see #317". Lower is safer than higher — widen on telemetry, not speculation. - **Structured logging** on the controller: ```java log.info("quickUpload actor={} files={} totalBytes={} withMetadata={} created={} updated={} errors={}", actorId, files.size(), totalBytes, metadata != null, result.created().size(), result.updated().size(), result.errors().size()); ``` Parameterized SLF4J (Log4Shell-safe). Loki queries become trivial. - **Metrics** (Micrometer, already wired via Actuator): - `documents.quickupload.batch.size` — distribution summary - `documents.quickupload.errors` — counter tagged by `code` - `documents.quickupload.duration` — timer Grafana panel follow-up — one dashboard card each. - **Thumbnail executor** — check `AsyncConfig.thumbnailExecutor` queue capacity and rejection policy. For bursty batches, `CallerRunsPolicy` means the quick-upload request blocks briefly instead of silently losing thumbnails. Confirm before ship. - **Rate limit** `/quick-upload` at 5 requests/minute per user. A genuine drag-and-drop workflow doesn't exceed this; double-clicks and programmatic abuse do. - **Runbook entry** in `docs/infrastructure/production-compose.md`: "If users report '413 Payload Too Large' on bulk upload, check both Spring `max-request-size` and Caddy `request_body { max_size }`. They must match." ### Open Decisions - **Request size cap (500MB vs tighter vs looser)** — 500MB covers 10 files at max per-file size with headroom. Tighter (e.g. 300MB / 6 files) bounds memory and parse time more aggressively; looser (1GB+) enables power users but widens the DoS surface for a compromised writer account. I lean 500MB for v1 and widen on feedback — zero-telemetry sizing in either direction is a guess.
Author
Owner

🗳️ Decision Queue — Action Required

4 decisions need your input before implementation starts. Everything else was turned into concrete recommendations.

Architecture

  • Pre-upload bridging strategy for DocumentEditLayout (carried over from #294)DocumentEditLayout today requires a persisted Doc with an id and loads /api/documents/{id}/file in an $effect. The new-doc flow needs a bridge. Option A (PLACEHOLDER-first): POST N placeholder Documents on first file drop, then drive the rest through the unchanged DocumentEditLayout (matches /enrich/[id]). Option B (pre-upload mode): teach the layout to accept optional doc + file props and submit one multipart POST at the end (the #317 spec implicitly picks this). A keeps the layout contract clean and aligns with sibling routes; B keeps single-submit UX but makes the component polymorphic across lifecycle stages. This decision determines the test strategy, component boundaries, and how the quick-upload endpoint is called. Pick before writing code. (Raised by: original #294 + Markus, Felix)

  • Overload /quick-upload vs new /api/documents/batch — The current plan overloads one endpoint to dispatch on presence of the metadata part (serving both drag-anywhere tooling and the new bulk UI). A separate endpoint would split contracts cleanly. Overload keeps the URL surface small and avoids deprecation churn; a split gives each caller a typed schema and simpler OpenAPI docs. Markus leans overload iff the metadata part is a typed DTO, not an opaque blob. (Raised by: Markus)

Infrastructure

  • max-request-size cap for /quick-upload — Must be set (today it defaults to 10MB, which makes the feature unshippable as specified). The question is the value. 500MB covers a 10-file batch at the documented 50MB per-file cap with headroom. 300MB bounds memory/parse time more aggressively (6-file batches). 1GB+ enables power-use but widens the DoS surface for a compromised writer account. Tobias leans 500MB for v1 and widens on telemetry. (Raised by: Tobias, Nora)

UX

  • Auto-focus policy on file switch — Spec says "always auto-focus the title input on switch". That's right for mouse/touch (title is the only per-file field and deserves priority). For keyboard users cycling / in the file switcher, every arrow press ejects focus out of the strip — forcing them to re-tab back in. Options: (a) auto-focus only on mouse/touch; keep focus in strip for arrow navigation. (b) always auto-focus (current spec). (c) never auto-focus; user clicks into the title. Leonie leans (a) — matches both mental models. (Raised by: Leonie)

Cross-cutting observations flagged by multiple personas (no decision needed, just awareness):

  • UploadSaveBar.svelte does not exist in the repo despite the spec claiming "already exists — extend". Treat as a new component. (Felix)
  • Backend metadata semantics for matched filenames: storeDocument returns existing filename-matches in updated[]. Unclear whether the new shared metadata should merge into those existing docs or leave them alone. Decide before writing the implementation, then lock with a test. (Sara, Markus)
  • Filename-derived suggestions at N≥2: current flow derives title + date + sender. For bulk, title is per-file; decide if date/sender from the first file pre-populates the shared fields (my recommendation: yes, but user can clear). (carried over from #294)
  • Sibling-route regression guard: any change to DocumentEditLayout's prop shape affects /documents/[id]/edit and /enrich/[id]. Keep backwards-compatible or scope out. (carried over from #294)
## 🗳️ Decision Queue — Action Required _4 decisions need your input before implementation starts. Everything else was turned into concrete recommendations._ ### Architecture - **Pre-upload bridging strategy for `DocumentEditLayout`** _(carried over from #294)_ — `DocumentEditLayout` today requires a persisted `Doc` with an `id` and loads `/api/documents/{id}/file` in an `$effect`. The new-doc flow needs a bridge. **Option A (PLACEHOLDER-first):** POST N placeholder Documents on first file drop, then drive the rest through the unchanged `DocumentEditLayout` (matches `/enrich/[id]`). **Option B (pre-upload mode):** teach the layout to accept optional `doc` + file props and submit one multipart POST at the end (the #317 spec implicitly picks this). A keeps the layout contract clean and aligns with sibling routes; B keeps single-submit UX but makes the component polymorphic across lifecycle stages. This decision determines the test strategy, component boundaries, and how the `quick-upload` endpoint is called. **Pick before writing code.** _(Raised by: original #294 + Markus, Felix)_ - **Overload `/quick-upload` vs new `/api/documents/batch`** — The current plan overloads one endpoint to dispatch on presence of the `metadata` part (serving both drag-anywhere tooling and the new bulk UI). A separate endpoint would split contracts cleanly. Overload keeps the URL surface small and avoids deprecation churn; a split gives each caller a typed schema and simpler OpenAPI docs. Markus leans overload *iff* the `metadata` part is a typed DTO, not an opaque blob. _(Raised by: Markus)_ ### Infrastructure - **`max-request-size` cap for `/quick-upload`** — Must be set (today it defaults to 10MB, which makes the feature unshippable as specified). The question is the value. `500MB` covers a 10-file batch at the documented 50MB per-file cap with headroom. `300MB` bounds memory/parse time more aggressively (6-file batches). `1GB+` enables power-use but widens the DoS surface for a compromised writer account. Tobias leans 500MB for v1 and widens on telemetry. _(Raised by: Tobias, Nora)_ ### UX - **Auto-focus policy on file switch** — Spec says "always auto-focus the title input on switch". That's right for mouse/touch (title is the only per-file field and deserves priority). For keyboard users cycling `←`/`→` in the file switcher, every arrow press ejects focus out of the strip — forcing them to re-tab back in. Options: **(a)** auto-focus only on mouse/touch; keep focus in strip for arrow navigation. **(b)** always auto-focus (current spec). **(c)** never auto-focus; user clicks into the title. Leonie leans (a) — matches both mental models. _(Raised by: Leonie)_ --- **Cross-cutting observations flagged by multiple personas (no decision needed, just awareness)**: - **`UploadSaveBar.svelte` does not exist** in the repo despite the spec claiming "already exists — extend". Treat as a new component. _(Felix)_ - **Backend metadata semantics for matched filenames**: `storeDocument` returns existing filename-matches in `updated[]`. Unclear whether the new shared `metadata` should merge into those existing docs or leave them alone. Decide before writing the implementation, then lock with a test. _(Sara, Markus)_ - **Filename-derived suggestions at N≥2**: current flow derives title + date + sender. For bulk, title is per-file; decide if date/sender from the first file pre-populates the shared fields (my recommendation: yes, but user can clear). _(carried over from #294)_ - **Sibling-route regression guard**: any change to `DocumentEditLayout`'s prop shape affects `/documents/[id]/edit` and `/enrich/[id]`. Keep backwards-compatible or scope out. _(carried over from #294)_
Author
Owner

📎 Carried over from #294 (now closed — superseded by this issue)

Three items from #294 that should be explicit in the #317 scope:

1. Pre-implementation design decision (was #294's "resolve first")

DocumentEditLayout today takes a persisted Doc with an id and loads /api/documents/{id}/file in an $effect. The new-doc flow needs to bridge the gap between "no document exists yet" and "edit a persisted document". Two options:

  • Option A — PLACEHOLDER-first: POST a PLACEHOLDER Document on page load (or on first file drop), then drive the rest of the flow through the unchanged DocumentEditLayout. For bulk, create N placeholders up front. Matches the /enrich/[id] pattern exactly.
  • Option B — pre-upload mode on layout: Teach DocumentEditLayout to accept an optional doc, show an UploadZone until files land, submit everything in one multipart POST at the end. The current #317 spec implicitly picks this.

Option A keeps DocumentEditLayout's contract clean and aligns with existing routes; Option B keeps the single-submit UX but makes the layout polymorphic across lifecycle stages. Decide before writing code — the test strategy and component boundaries hinge on this.

2. Acceptance criterion: no regressions in sibling routes

/documents/[id]/edit and /enrich/[id] also use DocumentEditLayout. Any prop-shape change to that component affects all three routes. The #317 spec's "accept { files, activeIndex }" proposal directly blasts into these two other routes. Add to the acceptance criteria:

No regressions in /documents/[id]/edit or /enrich/[id]. The three routes share the layout; changes to prop contract must be backwards-compatible or out-of-scope.

(My earlier Markus/Felix recommendation — create BulkDocumentEditLayout.svelte as a sibling rather than mutating DocumentEditLayout — is the direct fix for this constraint.)

3. Filename-derived suggestions beyond title

The current /documents/new flow uses parseFilename() from $lib/utils/filename.ts to suggest title, date, and sender from the filename (see FileSectionNew.svelte and +page.svelteparsedSuggestion). #317 only mentions title derivation.

Clarify the scope:

  • At N=1: keep the existing title + date + sender suggestions (parity with current behavior).
  • At N≥2: derive per-file title from each filename. Decide whether to also pre-populate the shared date/sender fields from the first file's filename (convenient but overridable) or leave them blank (safer, no guessing across a batch). The spec is silent — my recommendation is "pre-populate shared fields from the first file only; user can clear or edit".
## 📎 Carried over from #294 (now closed — superseded by this issue) Three items from #294 that should be explicit in the #317 scope: ### 1. Pre-implementation design decision (was #294's "resolve first") `DocumentEditLayout` today takes a persisted `Doc` with an `id` and loads `/api/documents/{id}/file` in an `$effect`. The new-doc flow needs to bridge the gap between "no document exists yet" and "edit a persisted document". Two options: - **Option A — PLACEHOLDER-first**: POST a `PLACEHOLDER` Document on page load (or on first file drop), then drive the rest of the flow through the unchanged `DocumentEditLayout`. For bulk, create N placeholders up front. Matches the `/enrich/[id]` pattern exactly. - **Option B — pre-upload mode on layout**: Teach `DocumentEditLayout` to accept an optional `doc`, show an UploadZone until files land, submit everything in one multipart POST at the end. The current #317 spec implicitly picks this. Option A keeps `DocumentEditLayout`'s contract clean and aligns with existing routes; Option B keeps the single-submit UX but makes the layout polymorphic across lifecycle stages. **Decide before writing code** — the test strategy and component boundaries hinge on this. ### 2. Acceptance criterion: no regressions in sibling routes `/documents/[id]/edit` and `/enrich/[id]` also use `DocumentEditLayout`. Any prop-shape change to that component affects all three routes. The #317 spec's "accept `{ files, activeIndex }`" proposal directly blasts into these two other routes. Add to the acceptance criteria: > **No regressions in `/documents/[id]/edit` or `/enrich/[id]`.** The three routes share the layout; changes to prop contract must be backwards-compatible or out-of-scope. (My earlier Markus/Felix recommendation — create `BulkDocumentEditLayout.svelte` as a sibling rather than mutating `DocumentEditLayout` — is the direct fix for this constraint.) ### 3. Filename-derived suggestions beyond title The current `/documents/new` flow uses `parseFilename()` from `$lib/utils/filename.ts` to suggest **title, date, and sender** from the filename (see `FileSectionNew.svelte` and `+page.svelte` → `parsedSuggestion`). #317 only mentions title derivation. Clarify the scope: - At N=1: keep the existing title + date + sender suggestions (parity with current behavior). - At N≥2: derive per-file title from each filename. **Decide** whether to also pre-populate the shared date/sender fields from the *first* file's filename (convenient but overridable) or leave them blank (safer, no guessing across a batch). The spec is silent — my recommendation is "pre-populate shared fields from the first file only; user can clear or edit".
Sign in to join this conversation.
No Label feature ui
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#317