perf(file_storage): one scan, cached, for the browse screen's bucket totals

[antosubash]

Closes #299

The problem

Rendering /file-storage/ issued five queries against file_storage_stored_file, three of which deliberately ignored the active filters and scanned the whole table:

Query	Cost
list_files count	filtered COUNT(*)
list_files page	filtered page
content_type_facets	GROUP BY content_type over every row
uploader_facets	GROUP BY created_by over every row
used_bytes	SUM(size_bytes) over every row

None was cached and none was bounded by the page size, so the cost of the screen grew with the bucket rather than with the page — on every render, including the ones that only changed ?page=. A page requested past the end made it seven, because the clamp re-ran the whole listing after fetching and discarding a page of rows nobody would ever see.

The reasoning for ignoring the filters was right and is kept: a facet list that hides its own alternatives is a dead end, and a usage figure that shrinks when you type in the search box describes nothing. What changes is what that costs.

The fix

One scan instead of three. aggregates.compute answers all three from a single GROUP BY content_type, created_by carrying a count and a byte sum, then folds the grid into the three shapes in Python. Not GROUPING SETS: SQLite has none, and the grid is already bounded by cardinality the filter dropdowns have to be able to render anyway.

A short TTL cache with write-driven invalidation. AggregateCache memoises the result per app with a 30s TTL, dropped by any commit that wrote a StoredFile.

Invalidation hangs off the DB write rather than off FileUploaded/FileDeleted — the issue's suggestion — for two reasons, both improvements on it:

• it also catches writes that publish nothing (a seed script, a back-fill, a fix-up in the shell), and
• it fires after the commit, so a concurrent reader cannot re-cache the pre-commit numbers for a whole TTL. Publishing happens inside the endpoint, before CommitBeforeResponseMiddleware commits; invalidating there would have left that window open.

The cache lives on FileStorageServices (per app), not at module scope, so a process running two apps never serves one app's totals out of the other's database. A FileStorageService constructed directly — a test, a script — gets no cache and reads through, which is what a caller checking "did my write land?" wants.

Count before paging. The view now counts, clamps, then fetches one page, instead of fetching a page and re-fetching after the clamp.

Net effect

Render	Before	After
Cold (first hit)	5	3
Warm (paging)	5	2
Past-the-end ?page=	7	3

reads.py is a mechanical extraction: the read half of FileStorageService moves to a mixin to stay under the 300-line cap. It is a real responsibility split — nothing in it touches a storage backend or mutates a row.

queries.uploader_facets / queries.used_bytes are removed (subsumed by aggregates.compute); queries.content_type_facets stays for the created_by-filtered case, which is not what the browse dropdown renders.

Tests

Assertions are on the shape of the work — how many statements name the table, and that exactly one of them is grouped — not on wall-clock time, which would be flaky in CI and would still pass on the day someone adds a fourth scan. A record_statements fixture (tests/conftest.py) hooks before_cursor_execute.

Verified failing on main before the change:

test_a_cold_render_scans_the_table_once            assert 5 == 3
test_the_second_render_reuses_the_totals           assert 5 == 2
test_clamping_does_not_fetch_the_page_twice[2]     assert 7 == 3
test_clamping_does_not_fetch_the_page_twice[99]    assert 7 == 3

Also covered: the folded totals match per-type / per-uploader / byte counts, soft-deleted rows stop counting, uploaderless rows count their bytes but are not offered as a filter option, facet ordering is preserved, and upload / delete / bulk-delete / out-of-band-commit are all reflected on the next render.

One bug found and fixed while writing these: the first version guarded re-registration with event.contains, whose key is id(target). A torn-down app's session class can be collected and its address reused, so a later app read back as "already registered" and was left with a cache nothing invalidated — order-dependent, and it did fail in the full-suite run. The guard is now a flag on the cache. _drop_on_commit also reads rather than pops the session flag, since a session can commit more than once.

Verification

Command	Result
uv run pytest modules/file_storage -q	88 passed (was 68; 20 new) — run 3x, stable
uv run pytest -q (full suite)	2802 passed, 2 skipped, 60 deselected in 4m34s
uv run ruff format --check modules/file_storage/	38 files already formatted
uv run ruff check modules/file_storage/	All checks passed
uv run ty check modules/file_storage	All checks passed
uv run python scripts/check_file_size.py	OK: no files exceed 300 lines
make doctor	clean, exit 0

No .tsx was touched — the browse props keep their exact wire shape — so vitest / ci-check-untranslated were not applicable.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-09-04T21:25:21.356779Z	fb0c295	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[cloudflare-workers-and-pages]

Deploying simple-module-python with    Cloudflare Pages

Latest commit:	fb0c295
Status:	✅  Deploy successful!
Preview URL:	https://e7ecb6a5.simple-module-python.pages.dev
Branch Preview URL:	https://fix-file-storage-aggregates.simple-module-python.pages.dev

View logs