Photo browser cannot reach past the first 100 photos #94

Closed
opened 2026-07-28 06:03:03 +00:00 by claude-bot · 3 comments

Severity: HIGH - functional wall

The bug

frontend/src/pages/PhotoBrowserPage.tsx:48-52 calls listPhotos({ limit: 100 }) once. The API
returns meta.next_cursor, and the frontend never reads it — a repo-wide grep found exactly
one 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_at and 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. The
sidebar offers status only.

Scope

  • Convert to useInfiniteQuery with getNextPageParam: (last) => last.meta.next_cursor.
  • Add a load-more sentinel or infinite scroll.
  • Virtualize the grid (@tanstack/react-virtual pairs naturally with the existing stack) — needed
    once tiles render real images.
  • Add album and date-range filters, and a sort control.
  • Preserve scroll position and cursor when navigating to a photo and back — today the grid
    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

  • Every photo in the collection is reachable by scrolling
  • The grid stays responsive with thousands of tiles
  • Album, date-range, and sort controls work
  • Returning from a photo restores scroll position

References

  • frontend/src/pages/PhotoBrowserPage.tsx:48-52
  • frontend/src/types/index.ts:103
  • docs/circa-spec.md §8.3

Depends on: the thumbnail pipeline. Related: #48.

## Severity: HIGH - functional wall ## The bug `frontend/src/pages/PhotoBrowserPage.tsx:48-52` calls `listPhotos({ limit: 100 })` once. The API returns `meta.next_cursor`, and **the frontend never reads it** — a repo-wide grep found exactly one 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_at` and **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. The sidebar offers status only. ## Scope - Convert to `useInfiniteQuery` with `getNextPageParam: (last) => last.meta.next_cursor`. - Add a load-more sentinel or infinite scroll. - Virtualize the grid (`@tanstack/react-virtual` pairs naturally with the existing stack) — needed once tiles render real images. - Add album and date-range filters, and a sort control. - Preserve scroll position and cursor when navigating to a photo and back — today the grid 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 - [ ] Every photo in the collection is reachable by scrolling - [ ] The grid stays responsive with thousands of tiles - [ ] Album, date-range, and sort controls work - [ ] Returning from a photo restores scroll position ## References - `frontend/src/pages/PhotoBrowserPage.tsx:48-52` - `frontend/src/types/index.ts:103` - `docs/circa-spec.md` §8.3 Depends on: the thumbnail pipeline. Related: #48.
claude-bot added this to the v0.3.0 milestone 2026-07-28 06:03:03 +00:00
Author

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 example rescan_requested)
alongside the album, date-range, and confidence filters. The browser API currently supports only
status and album_id.

Add flag filters to the scope of this issue. Confidence sort depends on #111.

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 example `rescan_requested`) alongside the album, date-range, and confidence filters. The browser API currently supports only `status` and `album_id`. Add flag filters to the scope of this issue. Confidence sort depends on #111.
Author

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_cursor is 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), not docs/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 an Album row or assigns photo.album_id — ingest does not even call parse_filename, so the album_slug it extracts is discarded. album_id is therefore NULL in every deployment: the existing ?album_id= query parameter matches nothing, ix_photo_collection_album indexes 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_low is 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 with limit=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:

page 1                     0.015 ms
deep page                  2.674 ms
undated tail               4.231 ms

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:

page 1                     0.016 ms
deep page                  0.018 ms   (was 2.674)
undated tail               0.020 ms   (was 4.231)

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:

tiles build+layout median frame p95 frame long frames >50ms DOM nodes
100 23.1 ms 16.7 ms 16.7 ms 0 612
1,000 106.1 ms 16.7 ms 16.7 ms 0 6,012
2,000 221.3 ms 16.7 ms 16.8 ms 0 12,012
5,000 465.3 ms 16.7 ms 16.8 ms 0 30,012
10,000 1,187.6 ms 33.3 ms 50.0 ms 0 60,012
30,000 3,499.7 ms 83.2 ms 99.9 ms 20 180,012

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: useInfiniteQuery over next_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.

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_cursor` is 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*), not `docs/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 an `Album` row or assigns `photo.album_id` — ingest does not even call `parse_filename`, so the `album_slug` it extracts is discarded. `album_id` is therefore NULL in every deployment: the existing `?album_id=` query parameter matches nothing, `ix_photo_collection_album` indexes 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_low` is 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 with `limit=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: page 1 0.015 ms deep page 2.674 ms undated tail 4.231 ms 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: page 1 0.016 ms deep page 0.018 ms (was 2.674) undated tail 0.020 ms (was 4.231) `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: | tiles | build+layout | median frame | p95 frame | long frames >50ms | DOM nodes | |---|---|---|---|---|---| | 100 | 23.1 ms | 16.7 ms | 16.7 ms | 0 | 612 | | 1,000 | 106.1 ms | 16.7 ms | 16.7 ms | 0 | 6,012 | | 2,000 | 221.3 ms | 16.7 ms | 16.8 ms | 0 | 12,012 | | 5,000 | 465.3 ms | 16.7 ms | 16.8 ms | 0 | 30,012 | | 10,000 | 1,187.6 ms | 33.3 ms | 50.0 ms | 0 | 60,012 | | 30,000 | 3,499.7 ms | 83.2 ms | 99.9 ms | 20 | 180,012 | 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: `useInfiniteQuery` over `next_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.
Author

Done in d878952 (auto-closed by the commit trailer).

What shipped. The grid follows meta.next_cursor through useInfiniteQuery and is virtualized; scroll position is remembered per filter view in sessionStorage. 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 as COALESCE(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 took disputed from 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_high before the sentinel, which I had not planned. fold takes 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, so undated became a lookup on the indexed key rather than a predicate over both date columns.

Against the "done when" list:

  • Every photo in the collection is reachable by scrolling
  • The grid stays responsive with thousands of tiles
  • Album, date-range, and sort controls work — album deferred to #105, agreed with @rbrooks, because nothing in the application has ever created an Album row
  • Returning from a photo restores scroll position

Scroll 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 PhotoSort itself, 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 the id tiebreak, removing the status-paired index, containment instead of overlap, and undated ignoring the high bound.

CI is currently red, and not on this work. The backend job fails at the dependency audit on a cryptography advisory (CVE-2026-69247, 49.0.0 → 50.0.0) published since the previous push; lint passed and the tests had not run yet. frontend is green. Tracking separately — this commit does not touch requirements.txt.

Done in d878952 (auto-closed by the commit trailer). **What shipped.** The grid follows `meta.next_cursor` through `useInfiniteQuery` and is virtualized; scroll position is remembered per filter view in `sessionStorage`. 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 as `COALESCE(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 took `disputed` from 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_high` before the sentinel, which I had not planned. `fold` takes 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, so `undated` became a lookup on the indexed key rather than a predicate over both date columns. **Against the "done when" list:** - [x] Every photo in the collection is reachable by scrolling - [x] The grid stays responsive with thousands of tiles - [x] ~~Album~~, date-range, and sort controls work — album deferred to #105, agreed with @rbrooks, because nothing in the application has ever created an `Album` row - [x] Returning from a photo restores scroll position Scroll 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 `PhotoSort` itself, 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 the `id` tiebreak, removing the status-paired index, containment instead of overlap, and `undated` ignoring the high bound. **CI is currently red, and not on this work.** The `backend` job fails at the dependency audit on a `cryptography` advisory (CVE-2026-69247, 49.0.0 → 50.0.0) published since the previous push; lint passed and the tests had not run yet. `frontend` is green. Tracking separately — this commit does not touch `requirements.txt`.
Sign in to join this conversation.
No description provided.