Photo browser cannot reach past the first 100 photos #94
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 - functional wall
The bug
frontend/src/pages/PhotoBrowserPage.tsx:48-52callslistPhotos({ limit: 100 })once. The APIreturns
meta.next_cursor, and the frontend never reads it — a repo-wide grep found exactlyone occurrence, the type declaration in
frontend/src/types/index.ts:103.There is no load-more, no infinite query, no virtualization. At 30,000 photos the browser shows
the oldest 100 by
created_atand there is no way to reach the rest.This is a hard functional cliff at 100 photos, hit long before any query gets slow. Keyset
pagination is implemented end to end on the backend and simply ignored by the UI.
Also missing versus the spec
docs/circa-spec.md§8.3 specifies album filter, date-range filter, and a sort control. Thesidebar offers status only.
Scope
useInfiniteQuerywithgetNextPageParam: (last) => last.meta.next_cursor.@tanstack/react-virtualpairs naturally with the existing stack) — neededonce tiles render real images.
re-renders at the top and the reviewer loses their place.
Sort by estimated date depends on the combined confidence work in v0.4.0; status and date sorts
can land now.
Done when
References
frontend/src/pages/PhotoBrowserPage.tsx:48-52frontend/src/types/index.ts:103docs/circa-spec.md§8.3Depends on: the thumbnail pipeline. Related: #48.
Additional filter gap from the feature audit, folded in here rather than filed separately:
docs/circa-spec.md§8.3 specifies flag-based filters (for examplerescan_requested)alongside the album, date-range, and confidence filters. The browser API currently supports only
statusandalbum_id.Add flag filters to the scope of this issue. Confidence sort depends on #111.
Picking this up. Probed before starting; four findings, two of which change the scope.
The core bug is exactly as described.
listPhotos({ limit: 100 })is called once,meta.next_cursoris read nowhere outside the type declaration, and the backend's keyset pagination is fully implemented and entirely unused. Confirmed.1. The spec reference is to the wrong document. The filter and sort requirements are
docs/circa-ui-spec.md§8.3 (Photo Browser), notdocs/circa-spec.md§8.3, which is Ingest steps. The UI spec asks for: filter by status / album / date range / confidence-uncertainty / duplicate-disputed-missing-page flags, and sort by ingest order / estimated date / confidence / album position. §9.1 additionally requires active filters to be visible, a one-click clear-all, and filter chips.docs/circa-spec.md§11.1 agrees on the sorts.2. The album filter and album-position sort are blocked on #105, and are being deferred there. Nothing in
backend/app/ever creates anAlbumrow or assignsphoto.album_id— ingest does not even callparse_filename, so thealbum_slugit extracts is discarded.album_idis therefore NULL in every deployment: the existing?album_id=query parameter matches nothing,ix_photo_collection_albumindexes a permanently-NULL column, and an album dropdown built now could only ever render "no albums". Album linkage is already named in #105's title, so the controls belong with the data that makes them meaningful. Commented on #105.3. Sorting by estimated date breaks keyset pagination outright, and the failure is #88's, worse.
current_effective_date_lowis nullable, and undated photos are the ones this application exists to work on. A row-value cursor comparison against NULL evaluates to NULL, so the row is excluded — and when a page boundary lands on an undated photo the cursor matches nothing and the listing terminates there. Measured on a five-row fixture withlimit=2: page 1 returns the two undated rows, page 2 returns empty, and three of five photographs are unreachable. Not an error — an omission, with a grid that looks like it ended.The fix has to be indexable as well as correct, and the obvious version is not. A sentinel sort expression (
COALESCE(current_effective_date_low, '9999-12-31')) paginates correctly, but SQLite will not seek on a row-value comparison whose leading term is an expression, even with a matching expression index — it scans the collection's index range and filters. Measured on 50,000 rows, best of five:which is #84's finding again, linear in the size of the archive, and worst exactly at the undated cluster a reviewer sorting by date is trying to reach.
A VIRTUAL generated column carrying the same expression restores the seek, because the index is then on a column rather than an expression:
SEARCH photo USING INDEX ... (collection_id=? AND (date_sort,id)>(?,?))rather than(collection_id=?). It also cannot drift from the column it is derived from, since SQLite computes it — which suits a projection model whose whole rule is that projections must be derivable. VIRTUAL rather than STORED: the index carries the value, so there is nothing to gain from a second copy in the table.4. Virtualization is justified, and the metric that breaks is not the one the issue names. Measured in Chromium 141 (Playwright 1.56.1) at 1600x1000, three runs per N, median, tiles replicating the real markup and CSS:
Scrolling stays at 60 fps to 5,000 tiles; it is the initial build-and-layout pass that degrades first, past 100 ms at 1,000 tiles and past a second at 10,000. That matters more than it looks, because with infinite scroll the DOM only ever grows: the cost is not paid once at a chosen page size, it accumulates for as long as the reviewer keeps scrolling, and re-rendering the list pays it again. Understated, too — the harness used a 1x1 placeholder data URI, where a real 400x300 thumbnail decodes to roughly 480 KB of bitmap, so heap and decode cost are optimistic and jank would arrive at a lower tile count than shown.
Scope being built, therefore:
useInfiniteQueryovernext_cursor, a virtualized grid, scroll and cursor restoration on return from a photo, sort by ingest order and estimated date, filters for date range, undated, and the duplicate / missing-page / rescan flags, and the §9.1 chips and clear-all. Confidence filter and sort stay with #111; album filter and album-position sort move to #105.One addition beyond the spec's list, agreed with @rbrooks: an undated filter. It is not named in §8.3, but it is the queue this application exists to work, and the sort key above already isolates those rows.
Done in
d878952(auto-closed by the commit trailer).What shipped. The grid follows
meta.next_cursorthroughuseInfiniteQueryand is virtualized; scroll position is remembered per filter view insessionStorage. Sort by ingest order or estimated date. Filters for date range, undated, and the rescan-requested / missing-page / duplicate flags, with §9.1's always-visible chips and a clear-all that deliberately leaves the sort alone.The date sort needed a schema change, for the reason in my earlier comment. Migration 012 adds
date_sort, a VIRTUAL column generated asCOALESCE(current_effective_date_low, current_effective_date_high, '9999-12-31'), plus(collection_id, date_sort, id)and(collection_id, status, date_sort, id). The second index is the one worth remembering: with only the first, SQLite drops the status index and reads the whole collection in date order filtering for the status, which tookdisputedfrom 0.397 ms to 0.724 ms — a regression caused by adding an index. With the pair it is 0.017 ms.One correction to what I wrote earlier, from building it: the sort key falls back to
current_effective_date_highbefore the sentinel, which I had not planned.foldtakes both bounds straight off the latest decision and either may be NULL, so "before 1970" is a real row shape; sorting it with the entirely-unknown photographs would throw away the one thing the archive knows about it. It also makes the sentinel mean exactly one thing, soundatedbecame a lookup on the indexed key rather than a predicate over both date columns.Against the "done when" list:
Album, date-range, and sort controls work — album deferred to #105, agreed with @rbrooks, because nothing in the application has ever created anAlbumrowScroll restoration has a limit worth stating rather than discovering: it survives the page being unmounted, but not React Query dropping the cached pages, after which the grid holds only the first page and is too short to scroll back into. That window is five minutes away from the tab, which is longer than deciding a date.
Tests. 1186 backend (was 1135), 87 component (was 64), 7 e2e. Pagination completeness is asserted against
PhotoSortitself, so a sort order added without a working keyset fails the suite rather than shipping. Five mutations were checked and each failed exactly the tests written for it: sorting on the raw nullable column, dropping theidtiebreak, removing the status-paired index, containment instead of overlap, andundatedignoring the high bound.CI is currently red, and not on this work. The
backendjob fails at the dependency audit on acryptographyadvisory (CVE-2026-69247, 49.0.0 → 50.0.0) published since the previous push; lint passed and the tests had not run yet.frontendis green. Tracking separately — this commit does not touchrequirements.txt.