Albums endpoint, and the browser's album filter and album-position sort #141
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: MEDIUM
Split out of #105, and the second half of what #94 deferred. Blocked on #140 — there is no point building a control until something creates the rows it would show.
What #94 left behind
#94 built the browser's filter and sort controls to
docs/circa-ui-spec.md§8.3 and deliberately omitted two of them, becausealbum_idis NULL in every deployment. The reasoning and measurements are on #94's thread; the short version is that a dropdown which can only ever say "no albums" is a control that cannot answer.Scope
GET /api/albums— a listing scoped to the active collection, so the browser can populate a filter. This is a new route, and it will trip two suites by design:test_api_contract.py'sROUTE_ACCESSfails until the route declares who may reach it.test_concurrency_protocol.py'sTestNoWriteEscapesTheProtocolsweeps the write surface; a read-only route needs noreview_versionbut the matrix still has to know about it.Both failures are the tests working, not obstacles.
The browser's album filter — a select populated from that endpoint, wired into the existing
PhotoQueryshape, which already carriesalbum_idand already sends it. The chips and clear-all inPhotoBrowserPagederive from one list, so an album chip should fall out of adding it there.Album-position sort —
PhotoSortcurrently hasingestanddate. Addingalbumneeds a keyset, and #94 established what that costs:Both
album_idandsequence_in_albumare nullable, so album-position sort needs the same treatmentdate_sortgot in migration 012: 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 archive (measured: 0.018 ms against 2.674 ms at 50,000 rows).test_pagination.py::TestPaginationHoldsForEverySortOrderis parametrized overPhotoSortitself, so a new member with a broken keyset fails the suite automatically. That is deliberate and is the safety net for this work.Done when
GET /api/albumslists the active collection's albums, with its access level declaredReferences
backend/alembic/versions/012_photo_date_sort.py— the pattern to followbackend/tests/test_pagination.py,test_query_plans.pyMeasured while building #140:
ix_photo_collection_albumdoes not work, and this issue has to fix itNot a new regression — it has never been exercised, because until #140 no row had an
album_id. Now that they will, here is what the album filter actually costs. 50,000 rows, 200 albums, 250 photos each, 4% excluded, best of 7, against the SQLlist_photosreally emits (captured from the repository rather than retyped).Without
ANALYZE— which is every real deployment, since nothing inapp/oralembic/ever runs one:~230x on the deep page. With
ANALYZEpresent the planner does reach the index but still sorts —… | USE TEMP B-TREE FOR ORDER BY, 0.311 / 0.104 ms.Cause is the one this repository has now hit four times.
ix_photo_collection_albumis(collection_id, album_id)and does not carry the(created_at, id)sort key, so using it costs a temporary B-tree and the planner prefers the index that gives the order for free. That is migration 010's finding (#88), migration 012's (ix_photo_collection_status_datesort, #94), and #106's excluded-view index — the same lesson each time: the index has to carry the order the query asks for, not just the predicate it filters on.ix_photo_collection_albumis the one listing index nobody ever widened, because nothing could exercise it.What this issue therefore needs
Widen it to
(collection_id, album_id, created_at, id), and — since #106 — partial onWHERE deleted_at IS NULL, or it will not be used at all. #106 measured what a non-partial listing index costs once a partial sibling exists: SQLite mis-chose it and every live listing went 0.06 ms → ~250 ms.Album-position sort (
sequence_in_album) is thePhotoSortmember this issue adds, and it is nullable — so it needs the migration-012 pattern: a VIRTUAL generated column with a COALESCE sentinel, indexed as a column, not an index on the expression. SQLite will not seek on a row-value comparison whose leading term is an expression; migration 012 measured 2.674 ms → 0.018 ms for exactly that.test_pagination.pyis parametrized overPhotoSort, so a new member with a broken keyset fails the suite by itself.test_query_plans.py::test_the_album_filter_is_not_left_scanningpasses today and will keep passing, because the fallback isSEARCH … ix_photo_collection_created, notSCAN photo— it only forbids a scan. A companion assertion in the shape oftest_the_status_paired_index_is_what_answers_a_filtered_date_sortis what would catch this, and it belongs with the migration rather than before it, since it would be red today.Recorded here rather than fixed under #140: it is a schema change, and #140's remit was linkage.
Done in
1aabafb, CI green (run 67).GET /api/albumslists the active collection's albums, with its access level declaredThe photo count is what the grid will actually show — the working set only, and for a viewer approved only. A count that promised photographs the grid then withheld would be the failure the dropdown exists to prevent. Albums with zero visible photographs are still listed at zero rather than hidden, matching
/photos/stats' "report 0, don't omit". And the dropdown is absent entirely from a collection with no albums, which is #100's lesson rather than a special case.The sort key, and the trap in it
Every piece is load-bearing.
~(0x7E) is the last printable ASCII character, so it beats any UUID under BINARY collation and unfiled photographs land at the end — the useful end, as 012 put undated ones there. Thirty-six wide because an album id is exactly 36, making the key a fixed 44 characters so one album's rows cannot interleave with another's by prefix. The sequence sentinel is eight tildes rather than'99999999'becauseprintf('%08d', …)pads but does not truncate, so a nine-digit sequence renders nine characters and would sort before a numeric sentinel.And it is a
CASE, not the obviousCOALESCE(printf(…), sentinel), because of the sharpest thing found here:printf('%08d', NULL)returns'00000000'in SQLite, not NULL. TheCOALESCEnever fires — and it fails silently: pagination stays complete, every row remains reachable, and unsequenced photographs simply sit at position zero at the front of their album. Pinned by a schema assertion so the tidier-looking form cannot come back.ix_photo_collection_albumhad never workedReported on this issue before the work started, and confirmed:
(collection_id, album_id)carries no sort key, so the planner preferredix_photo_collection_createdand an album filter cost 11.788 ms against 0.064 ms widened. Nothing had noticed because until #140 no row had analbum_idto exercise it.The status-paired index is included on measurement rather than on 012's precedent, and the difference is worth recording: 012 found that adding only the unfiltered index made
disputed2.6× slower than having no index at all, and that regression does not recur here, because the pre-existing status index did not carry album order either. The pair is justified instead by 6.3× and by the plan shape — without it SQLite reads the entire collection in album order filtering for status.Three things the tests said
limit=4— only at 1, 2 and 3. A single limit would have shipped it.printf/COALESCEtrap, because that bug misplaces rows without losing any. Completeness and placement are different properties, which is why a dedicated placement class exists rather than relying on the parametrized net.test_the_album_filter_is_not_left_scanning, exactly as predicted above: it forbids aSCAN, and the fallback is aSEARCH. The new companion assertion is what catches it.One cost recorded and deliberately not paid
An album filter combined with album-position sort walks the album-sorted index, so page one of an album late in id order costs 15.401 ms against 0.062 ms with a fourth index on
(collection_id, album_id, album_sort, id). Left out because deep pages are unaffected — the cursor seeks into the album's own range — the album view's default order is ingest, which the widened index answers in 0.064 ms, and it would be an eighthphotoindex paid on every write. That is the trade 012 declined for its filter-only shapes and #106 declined for the excluded queue in date order.I considered overruling that and did not. It is a one-line migration, the numbers are in
015's docstring, and a deliberately loose plan test marks the spot — so if the combination turns out to be what reviewers actually do, adding it is cheap.Backend tests 1320 → 1376, frontend 212 → 223, end-to-end 11 → 12.