No upload UI: the browser cannot put a photograph into the archive #142

Closed
opened 2026-08-04 15:13:03 +00:00 by claude-bot · 1 comment

Severity: MEDIUM

Split out of #101, which bundled it with logout and manual evidence as a third bullet of "missing
chrome". It is not chrome. It is the browser's first write path into the archive — the one place
where a person hands the software a file it will keep — and it deserves its own design rather than
being finished off at the end of a ticket about a sign-out button.

The problem

frontend/src/api/client.ts:92 defines api.upload for multipart form data. It has no callers
anywhere in frontend/src. POST /api/ingest exists and is tested, so the only way to put a
photograph into the archive today is curl.

Why it needs care

Ingest is not a form submission that either works or doesn't:

  • It is the one place a reviewer can lose data. A half-completed batch, a browser tab closed
    mid-upload, a file that fails validation on the ninetieth of a hundred — each needs an answer
    that is not "start again".
  • Its failures are per-file, not per-request. IngestOut reports accepted and rejected
    separately. A UI that collapses that into one green tick tells the reviewer their scans are in
    when some are not.
  • It runs into the image-validation and sandbox paths (services/image_validation.py,
    services/image_sandbox.py), which refuse files deliberately. A refusal has to be legible —
    "this is not an image we can read" rather than a 400.
  • Progress matters at this size. A flatbed batch is hundreds of megabytes; an upload with no
    progress reads as a hang.
  • It is the natural home for front/back pairing (#105) and album derivation (#140), which
    currently only happen for files that arrive by other means.

Open question, worth deciding before building

Whether browser upload is the intended ingest path at all, or whether a watch-directory worker
is. A scanner writes to a folder; a worker picks it up. If that is the real plan, then the browser
upload is a convenience for one-off additions rather than the main road, and its design should say
so — and the README should stop implying curl is the only option either way.

Done when

  • The ingest story is decided: browser upload, watch directory, or both with a stated division
  • Whatever is decided is reachable from the UI or documented in the README
  • Per-file outcomes are reported individually, and a rejection says why in the reviewer's terms
  • An interrupted batch does not require redoing the files that already landed

References

  • frontend/src/api/client.ts:92 (api.upload, no callers)
  • backend/app/api/routes/ingest.py; backend/app/services/ingest.py
  • Split from #101. Related: #105 (front/back pairing), #140 (album derivation).
## Severity: MEDIUM Split out of #101, which bundled it with logout and manual evidence as a third bullet of "missing chrome". It is not chrome. It is the browser's first *write* path into the archive — the one place where a person hands the software a file it will keep — and it deserves its own design rather than being finished off at the end of a ticket about a sign-out button. ## The problem `frontend/src/api/client.ts:92` defines `api.upload` for multipart form data. It has no callers anywhere in `frontend/src`. `POST /api/ingest` exists and is tested, so the only way to put a photograph into the archive today is `curl`. ## Why it needs care Ingest is not a form submission that either works or doesn't: - **It is the one place a reviewer can lose data.** A half-completed batch, a browser tab closed mid-upload, a file that fails validation on the ninetieth of a hundred — each needs an answer that is not "start again". - **Its failures are per-file, not per-request.** `IngestOut` reports accepted and rejected separately. A UI that collapses that into one green tick tells the reviewer their scans are in when some are not. - **It runs into the image-validation and sandbox paths** (`services/image_validation.py`, `services/image_sandbox.py`), which refuse files deliberately. A refusal has to be legible — "this is not an image we can read" rather than a 400. - **Progress matters at this size.** A flatbed batch is hundreds of megabytes; an upload with no progress reads as a hang. - **It is the natural home for front/back pairing** (#105) and album derivation (#140), which currently only happen for files that arrive by other means. ## Open question, worth deciding before building Whether browser upload is the intended ingest path at all, or whether a **watch-directory worker** is. A scanner writes to a folder; a worker picks it up. If that is the real plan, then the browser upload is a convenience for one-off additions rather than the main road, and its design should say so — and the README should stop implying `curl` is the only option either way. ## Done when - [ ] The ingest story is decided: browser upload, watch directory, or both with a stated division - [ ] Whatever is decided is reachable from the UI or documented in the README - [ ] Per-file outcomes are reported individually, and a rejection says why in the reviewer's terms - [ ] An interrupted batch does not require redoing the files that already landed ## References - `frontend/src/api/client.ts:92` (`api.upload`, no callers) - `backend/app/api/routes/ingest.py`; `backend/app/services/ingest.py` - Split from #101. Related: #105 (front/back pairing), #140 (album derivation).
Author

Done in 50c3eb6.

The open question, answered — and the constraint that answered it

@rbrooks: "this is not going to be running on a server that somebody scanning will have access to… the browser upload should be primary… the people scanning and uploading will be older and will get very frustrated with the tool if it's not very straightforward."

That settles it, and it settles it differently from how the issue framed the choice. The watch directory is not the alternative to browser upload — it is unreachable for the same reason the CLI is. The CLI folder scan #105 shipped stays for one-off use by someone with shell access; the browser is the road.

Measured before designing around it, because the issue left open whether browser upload was viable at all. A realistic 4×6 print at 400dpi (2.8 MB JPEG) costs 403 ms server-side through POST /api/ingest — hashing, sandboxed validation, thumbnail and review derivatives, evidence, and the OCR enqueue (#144) included. Median of twelve, warm.

box sequential
100 files 0.7 min
500 files 3.4 min
2,000 files 13.4 min

So a box is a few minutes behind a progress bar, and no batch endpoint was built: one request per file gives per-file outcomes and resumability for free. The "API and maybe some easy-to-use remote tool" fallback is not needed.

Done when

  • The ingest story is decided — browser primary, CLI for one-offs, watch directory dropped, all three stated in the README
  • Reachable from the UI ("Add photos" in the rail, reviewer and above) and documented
  • Per-file outcomes reported individually, with rejections in the reviewer's terms
  • An interrupted batch does not redo what landed — this was already true via #105's ingest ledger; re-adding the same folder reports already_ingested

Ingest drops from admin to reviewer

A reviewer already dates photographs, records evidence, and can exclude a scan from the collection outright. Adding one is the smaller act, so admin-only read as a Phase 1 convenience rather than a considered boundary — and admin additionally carries role management and session revocation, which nobody needs in order to hand the archive a photograph.

The words, because they are the feature

#105's outcome vocabulary is written for a bulk-import log and never reaches the screen. orphan_back becomes "Added — this is the back of a photo, and its front isn't here yet", and it is filed under Needs a look rather than Added, because it is the only outcome the person can act on while the box is still open beside them. The mapping is a total Record over the generated union, so a new outcome is a compile error rather than an orphan_back in front of somebody's grandmother.

Five things built that this issue did not ask for, each worth the space

  • A dropped folder is walked through the entry API. DataTransfer.files reports a dropped directory as one contentless entry — the naive version answers a dropped box of scans with a single "Skipped — not a photo file", which would have defeated the entire feature. readEntries also returns at most a hundred per call, the quiet way to lose everything past the hundredth photograph.
  • A "Ready to add" step before anything is sent. Auto-starting a thirteen-minute operation on a folder drop leaves someone who picked the wrong folder no remedy but closing the tab.
  • A Stop button, for the same reason. It stops between files, so the one in flight finishes rather than being abandoned.
  • A 401 mid-run is special-cased. A session expiring partway would otherwise report a hundred files as "Couldn't be added" and leave someone believing the archive rejected their photographs.
  • The live region is separate and coarse. aria-live on "Adding 47 of 312" would announce two thousand times — the opposite of the intent. The visible bar carries role="progressbar"; the hidden region speaks about ten times a run.

Files are sorted fronts before backs as scan_folder does, or every back arrives first, becomes an orphan, and is folded and re-thumbnailed when its front turns up. Uploads run one at a time deliberately — the server holds SQLite's single write lock through each derivative, so parallelism would contend rather than help; that is commented so it is not later "optimised".

A second hole in the safety net

Every case in test_api_contract.py asserts who is refused. So declaring a route REVIEWER constrained viewers and admins and said nothing about reviewers — reverting ingest to admin-only was caught by exactly one new test and nothing else. TestARoleFloorHolds.test_a_reviewer_is_not_locked_out_of_a_reviewer_route closes it, mirroring the admin variant.

Twelve mutations, all caught, including one that reverts the role and is killed by the end-to-end journey.

Not verified

  • A real folder drag-and-drop in a browser. The entry-API walk is covered by a hand-built fake in jsdom, including readEntries returning an empty second batch, but Playwright cannot drop a real directory. That path is reviewed code, not exercised code.
  • webkitdirectory opening a real folder picker — the e2e journey uses the plain multi-file input.
  • Anything above nine files. The 100-row cap on the "Needs a look" list and the announcement pacing are reasoned, not observed. Nothing here has been run at box scale, because nothing here has ever ingested a real collection.
Done in 50c3eb6. ## The open question, answered — and the constraint that answered it @rbrooks: *"this is not going to be running on a server that somebody scanning will have access to… the browser upload should be primary… the people scanning and uploading will be older and will get very frustrated with the tool if it's not very straightforward."* That settles it, and it settles it differently from how the issue framed the choice. The watch directory is not the alternative to browser upload — **it is unreachable for the same reason the CLI is.** The CLI folder scan #105 shipped stays for one-off use by someone with shell access; the browser is the road. **Measured before designing around it**, because the issue left open whether browser upload was viable at all. A realistic 4×6 print at 400dpi (2.8 MB JPEG) costs **403 ms** server-side through `POST /api/ingest` — hashing, sandboxed validation, thumbnail and review derivatives, evidence, and the OCR enqueue (#144) included. Median of twelve, warm. | box | sequential | |---|---| | 100 files | 0.7 min | | 500 files | 3.4 min | | 2,000 files | 13.4 min | So a box is a few minutes behind a progress bar, and **no batch endpoint was built**: one request per file gives per-file outcomes and resumability for free. The "API and maybe some easy-to-use remote tool" fallback is not needed. ## Done when - [x] The ingest story is decided — browser primary, CLI for one-offs, watch directory dropped, all three stated in the README - [x] Reachable from the UI ("Add photos" in the rail, reviewer and above) and documented - [x] Per-file outcomes reported individually, with rejections in the reviewer's terms - [x] An interrupted batch does not redo what landed — this was **already true** via #105's ingest ledger; re-adding the same folder reports `already_ingested` ## Ingest drops from admin to reviewer A reviewer already dates photographs, records evidence, and can **exclude a scan from the collection outright**. Adding one is the smaller act, so admin-only read as a Phase 1 convenience rather than a considered boundary — and admin additionally carries role management and session revocation, which nobody needs in order to hand the archive a photograph. ## The words, because they are the feature #105's outcome vocabulary is written for a bulk-import log and **never reaches the screen**. `orphan_back` becomes *"Added — this is the back of a photo, and its front isn't here yet"*, and it is filed under **Needs a look** rather than Added, because it is the only outcome the person can act on while the box is still open beside them. The mapping is a total `Record` over the generated union, so a new outcome is a compile error rather than an `orphan_back` in front of somebody's grandmother. ## Five things built that this issue did not ask for, each worth the space - **A dropped folder is walked through the entry API.** `DataTransfer.files` reports a dropped *directory* as one contentless entry — the naive version answers a dropped box of scans with a single "Skipped — not a photo file", which would have defeated the entire feature. `readEntries` also returns at most a hundred per call, the quiet way to lose everything past the hundredth photograph. - **A "Ready to add" step before anything is sent.** Auto-starting a thirteen-minute operation on a folder drop leaves someone who picked the wrong folder no remedy but closing the tab. - **A Stop button**, for the same reason. It stops between files, so the one in flight finishes rather than being abandoned. - **A 401 mid-run is special-cased.** A session expiring partway would otherwise report a hundred files as "Couldn't be added" and leave someone believing the archive rejected their photographs. - **The live region is separate and coarse.** `aria-live` on "Adding 47 of 312" would announce two thousand times — the opposite of the intent. The visible bar carries `role="progressbar"`; the hidden region speaks about ten times a run. Files are sorted **fronts before backs** as `scan_folder` does, or every back arrives first, becomes an orphan, and is folded and re-thumbnailed when its front turns up. Uploads run **one at a time deliberately** — the server holds SQLite's single write lock through each derivative, so parallelism would contend rather than help; that is commented so it is not later "optimised". ## A second hole in the safety net Every case in `test_api_contract.py` asserts who is **refused**. So declaring a route `REVIEWER` constrained viewers and admins and said *nothing about reviewers* — reverting ingest to admin-only was caught by exactly one new test and nothing else. `TestARoleFloorHolds.test_a_reviewer_is_not_locked_out_of_a_reviewer_route` closes it, mirroring the admin variant. Twelve mutations, all caught, including one that reverts the role and is killed by the end-to-end journey. ## Not verified - **A real folder drag-and-drop in a browser.** The entry-API walk is covered by a hand-built fake in jsdom, including `readEntries` returning an empty second batch, but Playwright cannot drop a real directory. That path is reviewed code, not exercised code. - **`webkitdirectory` opening a real folder picker** — the e2e journey uses the plain multi-file input. - **Anything above nine files.** The 100-row cap on the "Needs a look" list and the announcement pacing are reasoned, not observed. Nothing here has been run at box scale, because nothing here has ever ingested a real collection.
Sign in to join this conversation.
No description provided.