Bulk ingest, front/back pairing, and album linkage #105
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: HIGH - this is the front door for the whole collection
The problem
docs/circa-spec.md§8 requires bulk and incremental ingest, pairing_f/_bfiles by album andsequence, and a watch directory. The implementation:
backend/app/api/routes/ingest.py)sequence_in_album, despiteservices/filename_parser.pysuccessfully extractingalbum_slug,sequence, andis_backSo the parser tests one thing and the pipeline delivers another. A
xxx_b.jpgback scan uploadedalone becomes an orphan photo rather than pairing with its front. Nothing in the codebase ever
creates an
Album.Why it blocks Phase 2
Album ordering (#18) and constraint propagation (#19) are built on album context. With no
ingestion path for it, both start from empty data and someone must hand-assign thousands of
photos. This is far cheaper to fix before the collection is ingested than after.
Scope
Albumfrom the parsed slug; setsequence_in_album.is_backfiles to the matching front photo by album and sequence, across requests — orreject them with a clear message explaining why.
Done when
References
backend/app/api/routes/ingest.pybackend/app/services/filename_parser.py(output currently discarded)backend/app/services/ingest.py:133-149docs/circa-spec.md§8Blocks: #18, #19.
Scope inherited from #94 — the album filter and album-position sort in the photo browser belong here, not there.
#94 set out to build the browser filter and sort controls that
docs/circa-ui-spec.md§8.3 asks for, including filter by album and sort by album position. Probing found there is nothing behind them: nothing inbackend/app/ever creates anAlbumrow or assignsphoto.album_id. Ingest does not callparse_filenameat all, so thealbum_slugit extracts from names likeredbk42_001_1983.jpgis discarded, andalbum_idis NULL in every deployment.The consequences today:
GET /api/photos?album_id=…is a working, tested filter that matches nothing, ever.ix_photo_collection_albumindexes a permanently-NULL column.So #94 is landing without them, and this issue picks them up alongside the album linkage already in its title. What that means concretely here:
parse_filenameand creates/linksAlbumrows fromalbum_slug, and setssequence_in_albumfrom the parsed sequence.GET /api/albumslisting, so the browser can populate the filter. New route — it will tripROUTE_ACCESSintest_api_contract.pyand the write-surface sweep intest_concurrency_protocol.pyuntil its access level is declared, which is deliberate.On (3), a note from #94's measurements that will save re-deriving it: sorting on a nullable column breaks keyset pagination — a row-value cursor comparison against NULL matches nothing and the listing terminates at that page rather than skipping rows.
sequence_in_albumandalbum_idare both nullable, so album-position sort needs the same treatment #94 gave estimated date: a VIRTUAL generated column carrying aCOALESCE(...)sentinel, indexed as a column. An index on the expression is not enough — SQLite will not seek on a row-value whose leading term is an expression, and the deep page degrades to a scan linear in the size of the archive (measured: 0.018 ms against 2.674 ms at 50,000 rows). See #94's thread for the numbers and the plan output.Split, per @rbrooks. This issue had grown to four separable units — its own original scope plus everything #94 handed it — and was too large to pick up as one piece.
Split out:
Albumor setsphoto.album_id. The foundation, and the one with the most surprising finding:parse_filenameis thorough, tested, and never called. Everything album-shaped in the application is downstream of that.Left here: the original scope this issue was filed for — bulk ingest and front/back pairing. That is genuine, separate work: pairing
redbk42_001_1983.jpgwithredbk42_001_1983_b.jpgis about how scans arrive, not about how they are filed, and it does not depend on either of the above.Suggest re-titling this issue to match what it now holds, since "album linkage" has moved out of it.
Nothing has been built for any of the three yet.
Picking this up — the last open issue on v0.3.0.
Probe first, since the body predates the split. What it says is still true of the parts it now covers, and one claim has become buildable rather than stale:
parsed.is_backcomputed and discarded: still true, andservices/ingest.pysays so in a comment that names this issue.album_idandsequence_in_albumare populated on ingest now, which is what makes the rest of this issue tractable —docs/circa-spec.md§8.1 pairs front and back by album ID + sequence, and until last week both were NULL on every row.Also worth recording, because it bounds the scope: ingest enqueues no jobs at all. Spec §8.3 step 5 says OCR runs on the back scan, but nothing in
ingest_photocallsenqueue; OCR is driven byPOST /api/photos/{id}/evidence/ocr-rerun. So attaching a back scan after the fact does not need to trigger OCR to match what ingest does today. Not fixing that here — it is a separate gap and belongs in its own issue.Two decisions taken with @rbrooks before building:
Bulk ingest is a service layer plus a CLI folder scan (
python -m app.cli.ingest_folder), sitting besidebackfill_albumsandrebuild_projections. The HTTP batch endpoint is deferred to #142, where the browser-upload-vs-watch-directory question is still open — building it here would commit that decision by accident. The pairing and batch-outcome logic goes in the service layer so whichever transport #142 picks gets it for free, exactly asalbum_slug_foris shared between ingest andbackfill_albums.A folder scan is also the only form that answers "thousands in one operation" honestly: derivatives are generated inline at roughly 300 ms per scan, so five thousand scans is a twenty-five minute operation. That is not a shape an HTTP request should have.
An orphan back — a
_bscan whose front is not in the archive — is ingested and flagged, not rejected. The bytes of an irreplaceable original get kept.That decision carries a consequence worth stating plainly, because it is the part that costs work: it needs a fold. Spec §8.2 requires incremental ingest, so the front for an orphan back will routinely arrive in a later batch. Without a fold, that produces two rows for one photograph — the orphan back and the new front — and the archive would assert two photographs where the box holds one. So a front that matches an unpaired back at the same album and sequence merges into that row rather than creating its own, which also preserves any OCR evidence already recorded against the back.
Pairing needs both album and sequence to be non-NULL, and refuses when more than one candidate matches — an ambiguous pair is reported, not guessed at.
Done in
e50af5d.What landed
python -m app.cli.ingest_folder <dir>— walks a folder, orders it fronts-before-backs, commits per file, reports every file as one ofingested/already_ingested/duplicate/paired/folded/orphan_back/back_duplicate/ambiguous/skipped/failed.--dry-run,--collection,--no-recursive. Exits non-zero if anything failed.Pairing, batching and the ledger sit in
services/ingest.pyandservices/ingest_batch.py, not behind an endpoint, so whichever transport #142 picks inherits them.Against the "Done when" list:
The four ways a back can arrive
Attach (front already here) writes onto that row — never a second row, because two rows for one print put the same photograph in the grid twice with nothing to say one is the reverse of the other. Orphan back (front not here) is kept, not rejected, per @rbrooks' decision: a row with no front, thumbnail generated from the back since that is the only image the archive has. Fold (front arrives later) writes onto the waiting row — this is what makes §8.2's incremental ingest actually hold, since without it a front in a later batch makes a second row. Ambiguous (more than one candidate) is never guessed: ingested unpaired and reported.
Both orders now leave the archive saying exactly the same thing. Attach and fold both read the incoming file's EXIF and both record its filename date guarded against one already on the row. An operator does not choose which half of a box the scanner reaches first. Pinned by a test that ingests the same two files in both orders into two collections and compares every evidence row's claim.
The ingest ledger
Built here rather than deferred, per @rbrooks. It records every file presented to the archive, including the ones that could not be read — ingest wrote to the audit log only on success, so an undecodable scan left no trace at all once the terminal scrolled. Same argument
derivatives_errorwas added on (#78).It also closes the half of resumability
sha256_backdid not. A re-fed front still made a duplicate row every time, so resuming a 5,000-file import that died at 3,000 would have put 3,000 rows in the reviewer's duplicate queue. Bytes cannot distinguish "the same print scanned twice" from "the same file fed in twice", so it is recorded instead of inferred. Recording rather than inferring has a second property that decided the design: it still holds when the photograph was later excluded, so re-presenting a file does not resurrect a scan somebody deliberately took out of the collection (#106) — which a check against thephototable would have done silently.The table is explicitly not a source of truth about photographs. Where it and the
photorow disagree, thephotorow wins.Verified rather than assumed
Ran the CLI against a real folder, not only the tests: three runs showing pair → orphan → fold, and a re-run reporting
already_ingestedfor every file with no new rows. Three prints ended as three photo rows, #9 holding both sides in one row after its front arrived in a later run. That run is also what caught two things the tests did not — the ledger was recording../.sandbox/scans/..., a path meaningless without the working directory nobody records, so the CLI now resolves to absolute; and the outcome column was one character too narrow foralready_ingested.Every new test mutation-checked — nine backend mutations and one frontend, each breaking exactly one behaviour, all caught. Backend 1376 → 1452, frontend 223 → 224, e2e 12.
Index discipline.
ix_photo_sha256_backleads with the hash rather thancollection_id, because migration 014 measured what a collection-led index costs when the planner reaches for it on a listing — 0.06 ms to ~250 ms, with the answers still correct so nothing above the query plan would report it.ingest_record's index may lead withcollection_id: the hazard is about displacing the index carrying a listing's ORDER BY, and that table has no ordered listing. Confirmed rather than reasoned about — all 16photolisting plans are byte-identical across revisions 015, 016 and 017.Two things found on the way, fixed here
test_an_identical_upload_does_not_report_the_existing_filefed the same file twice under one name, which the ledger now short-circuits — so it passed without ever reaching the content-addressed store it exists to protect (#91). Found by instrumenting the short-circuit and asking which tests in the whole suite take it; exactly two do, and this was the one that did not mean to.core.autocrlfchecked the runner scripts out with CRLF and the container's bash died onset -euo pipefail\rbefore a single test started — green in CI the whole time, because CI runs on Linux. A.gitattributespinning*.shtoeol=lffixes it, and the twelve journeys pass locally again.Known limitation, stated rather than discovered later
Camera-prefixed names cannot pair.
IMG_0042_1987.jpghas no album under #140's rule, so it and its_bnever meet: the front ingests normally and the back becomes an orphan back. That is the right answer — those come off a memory card and have no physical back — and nothing is lost, but it is worth knowing before someone reports it as a bug.Filed separately