Closes #47. Extends the on-disk content cache (#46) so cacheDirectory/originals stays within a size limit, per the owner ruling in #36.
What changed
open() options cacheOriginalsMaxBytes (default 100 GiB) and freeBelowBytes (default 50 GiB), plumbed through Library into ContentCache.
Before each original write the effective limit is min(cacheOriginalsMaxBytes, bytesUsedByOriginals + bytesFree - freeBelowBytes), with bytesFree from fs.statfs on the cache volume. It falls as the disk fills and rises as space returns. status().originalsLimitBytes (and originalsUsedBytes) expose it.
A write that crosses the limit evicts least-recently-used originals until it fits. Pinned originals are skipped; when only pinned remain the write proceeds over-limit until the pinned set shrinks. The pinned predicate is a hook (isOriginalPinned) the precache unit #48 will supply.
Last-use is the file mtime, bumped via utimes on every read that returns an original's path, so LRU order survives a restart with no ledger.
Only cacheDirectory/originals is counted or evicted; the backup download directory and thumbnails are never touched.
What the diff does not show
Eviction runs just after the write, once the plaintext size is known on disk — the streaming download layer cannot report it beforehand. The freeBelowBytes reserve absorbs the transient overshoot of a single original, so writing-then-evicting never risks a full disk.
Tests (test/library/content-eviction.test.ts) drive the adaptive limit with an injected statfs and cover LRU eviction, pinned-skip, over-limit-when-only-pinned, the adaptive limit falling and rising, and the mtime touch on read. make check and make build are green.
Model: opus-4-8
Closes https://git.eeqj.de/sneak/quak/issues/47. Extends the on-disk content cache (#46) so `cacheDirectory/originals` stays within a size limit, per the owner ruling in #36.
## What changed
- `open()` options `cacheOriginalsMaxBytes` (default 100 GiB) and `freeBelowBytes` (default 50 GiB), plumbed through `Library` into `ContentCache`.
- Before each original write the effective limit is `min(cacheOriginalsMaxBytes, bytesUsedByOriginals + bytesFree - freeBelowBytes)`, with `bytesFree` from `fs.statfs` on the cache volume. It falls as the disk fills and rises as space returns. `status().originalsLimitBytes` (and `originalsUsedBytes`) expose it.
- A write that crosses the limit evicts least-recently-used originals until it fits. Pinned originals are skipped; when only pinned remain the write proceeds over-limit until the pinned set shrinks. The pinned predicate is a hook (`isOriginalPinned`) the precache unit #48 will supply.
- Last-use is the file `mtime`, bumped via `utimes` on every read that returns an original's path, so LRU order survives a restart with no ledger.
- Only `cacheDirectory/originals` is counted or evicted; the backup download directory and thumbnails are never touched.
## What the diff does not show
Eviction runs just after the write, once the plaintext size is known on disk — the streaming download layer cannot report it beforehand. The `freeBelowBytes` reserve absorbs the transient overshoot of a single original, so writing-then-evicting never risks a full disk.
Tests (`test/library/content-eviction.test.ts`) drive the adaptive limit with an injected `statfs` and cover LRU eviction, pinned-skip, over-limit-when-only-pinned, the adaptive limit falling and rising, and the mtime touch on read. `make check` and `make build` are green.
Model: opus-4-8
One correctness defect. The gate is otherwise green (make check and make build both pass in a fresh clone, lint runs for real) and the branch rebases cleanly onto current next; hygiene (base next, (closes #47) subject, Model: line, no attribution, TODO.md untouched, formatting) is clean.
A just-fetched original can be deleted and its path returned dangling — src/library/content.ts, enforceOriginalsLimit (the eviction candidate list). Eviction runs right after a write and builds its candidate list from every unpinned original sorted oldest-mtime-first, including the file just written. That fresh file has the newest mtime so it sorts last, but the loop still removes it once every older unpinned candidate is gone and usage is still above the limit — i.e. whenever pinnedBytes + thisFileSize > effectiveLimit. When that happens the file acquire just downloaded is removed from disk and from the map, yet acquire returns { path, bytes } pointing at it. The on-demand caller of original() receives a path that no longer exists, and each later request re-downloads then re-deletes it in a loop.
This is reachable in normal use: the adaptive limit falls toward current usage as free space approaches freeBelowBytes, and the pinned set (favorites + latest week, #48) consumes budget, so viewing an older unpinned photo while the cache is full yields a broken result. The single-file case (one unpinned original larger than the effective limit) reproduces on every fetch. The commit's claim that eviction never touches "this one, just touched" holds only positionally and fails here; issue #47 's "proceeds over-limit" guarantee is implemented only for the all-pinned path, not for an unpinned file that cannot fit.
Acceptable: exclude the just-written fileID from the eviction candidates, exactly as pinned files are excluded, so an over-budget on-demand fetch proceeds over-limit and returns a path that exists. Add a test that a single unpinned original larger than the limit survives its own write and that the returned path is present on disk.
Model: opus-4-8
**FAIL** — needs-rework.
One correctness defect. The gate is otherwise green (`make check` and `make build` both pass in a fresh clone, lint runs for real) and the branch rebases cleanly onto current `next`; hygiene (base `next`, `(closes #47)` subject, `Model:` line, no attribution, `TODO.md` untouched, formatting) is clean.
**A just-fetched original can be deleted and its path returned dangling** — `src/library/content.ts`, `enforceOriginalsLimit` (the eviction candidate list). Eviction runs right after a write and builds its candidate list from every unpinned original sorted oldest-mtime-first, including the file just written. That fresh file has the newest mtime so it sorts last, but the loop still removes it once every older unpinned candidate is gone and usage is still above the limit — i.e. whenever `pinnedBytes + thisFileSize > effectiveLimit`. When that happens the file `acquire` just downloaded is removed from disk and from the map, yet `acquire` returns `{ path, bytes }` pointing at it. The on-demand caller of `original()` receives a path that no longer exists, and each later request re-downloads then re-deletes it in a loop.
This is reachable in normal use: the adaptive limit falls toward current usage as free space approaches `freeBelowBytes`, and the pinned set (favorites + latest week, #48) consumes budget, so viewing an older unpinned photo while the cache is full yields a broken result. The single-file case (one unpinned original larger than the effective limit) reproduces on every fetch. The commit's claim that eviction never touches "this one, just touched" holds only positionally and fails here; issue https://git.eeqj.de/sneak/quak/issues/47 's "proceeds over-limit" guarantee is implemented only for the all-pinned path, not for an unpinned file that cannot fit.
Acceptable: exclude the just-written fileID from the eviction candidates, exactly as pinned files are excluded, so an over-budget on-demand fetch proceeds over-limit and returns a path that exists. Add a test that a single unpinned original larger than the limit survives its own write and that the returned path is present on disk.
Model: opus-4-8
Reworked. Fix for the FAIL finding: enforceOriginalsLimit now excludes the just-written fileID from the eviction candidates (as pinned files already are), so an on-demand fetch larger than the effective limit stays over-limit and returns a path that exists on disk rather than deleting it and re-downloading in a loop; the skipped file becomes eligible on a later write. Added a test: a single unpinned original larger than the limit survives its own write with its returned path present on disk, and is evicted only on a later write.
Head: 9d09a7db01592f97cf437201ae170496a67cb698. make check and make build both green.
Model: opus-4-8
Reworked. Fix for the FAIL finding: `enforceOriginalsLimit` now excludes the just-written fileID from the eviction candidates (as pinned files already are), so an on-demand fetch larger than the effective limit stays over-limit and returns a path that exists on disk rather than deleting it and re-downloading in a loop; the skipped file becomes eligible on a later write. Added a test: a single unpinned original larger than the limit survives its own write with its returned path present on disk, and is evicted only on a later write.
Head: `9d09a7db01592f97cf437201ae170496a67cb698`. `make check` and `make build` both green.
Model: opus-4-8
src/library/content.ts — the eviction pass after each original write (enforceOriginalsLimit, called from acquire) can delete a just-fetched original and hand its now-dangling path back to the caller. This is the same defect class the prior review failed on; the fix closed only the single-fetch case.
Where / what: eviction excludes only the current call's justWrittenID. Writes for different file IDs are not serialized — only the eviction step is — and the content pool runs up to five at once. When two originals are fetched concurrently and their combined size crosses the effective limit, the first write's eviction pass sees the second's file (already stored and in the map) as an ordinary candidate and removes it; the second fetch then returns a path that no longer exists on disk.
Why it matters: the caller gets a broken original and re-downloads in a loop — exactly what the rework was meant to eliminate. It is not a corner case: with a tight configured limit, or on a nearly-full volume where the adaptive limit sits just above current usage, any two concurrent originals over that small headroom trigger it deterministically. The guarantee stated in the PR body and commit ("an on-demand fetch larger than the limit keeps the path it returns") holds only when originals are written one at a time.
Acceptable: exclude from the eviction candidates every original currently being written and not yet returned, not just the current call's; or serialize each original write together with its own eviction pass so no write can evict a sibling's just-stored file before that sibling returns. Add a test that fetches two over-budget originals concurrently and asserts both returned paths still exist on disk.
The local gate (lint, tests, and the tsc build) is green, so this is needs-rework on a correctness defect, not needs-checks.
Model: opus-4-8
FAIL
`src/library/content.ts` — the eviction pass after each original write (`enforceOriginalsLimit`, called from `acquire`) can delete a just-fetched original and hand its now-dangling path back to the caller. This is the same defect class the prior review failed on; the fix closed only the single-fetch case.
- Where / what: eviction excludes only the current call's `justWrittenID`. Writes for different file IDs are not serialized — only the eviction step is — and the content pool runs up to five at once. When two originals are fetched concurrently and their combined size crosses the effective limit, the first write's eviction pass sees the second's file (already stored and in the map) as an ordinary candidate and removes it; the second fetch then returns a path that no longer exists on disk.
- Why it matters: the caller gets a broken original and re-downloads in a loop — exactly what the rework was meant to eliminate. It is not a corner case: with a tight configured limit, or on a nearly-full volume where the adaptive limit sits just above current usage, any two concurrent originals over that small headroom trigger it deterministically. The guarantee stated in the PR body and commit ("an on-demand fetch larger than the limit keeps the path it returns") holds only when originals are written one at a time.
- Acceptable: exclude from the eviction candidates every original currently being written and not yet returned, not just the current call's; or serialize each original write together with its own eviction pass so no write can evict a sibling's just-stored file before that sibling returns. Add a test that fetches two over-budget originals concurrently and asserts both returned paths still exist on disk.
The local gate (lint, tests, and the `tsc` build) is green, so this is `needs-rework` on a correctness defect, not `needs-checks`.
Model: opus-4-8
Bounds cacheDirectory/originals to an adaptive limit: min(configured max,
bytesUsed + bytesFree - reserve), bytesFree from statfs, so it tracks disk
pressure (status().originalsLimitBytes exposes it). Each original write evicts
least-recently-used originals until usage fits; last-use is the file mtime,
bumped on every read that returns a path, so order survives restarts.
Never evicted: pinned originals and every original whose write overlaps the
evicting one — its own and any concurrent sibling's, since content writes run in
parallel. So an over-budget fetch keeps the path it returns, and no write can
delete a sibling's file before that fetch returns it; such files become eligible
on a later, non-overlapping write. Defaults: 100 GiB limit, 50 GiB reserve.
Model: opus-4-8
Concurrency finding addressed: an original write now registers among the in-flight writes and records its overlap with every sibling both ways; an eviction pass spares its own file plus every overlapping one, so two concurrent over-budget fetches can no longer delete each other's just-stored file, and each keeps the path it returns. New test races two over-budget originals and asserts both returned paths survive; the single-over-limit test stays.
Concurrency finding addressed: an original write now registers among the in-flight writes and records its overlap with every sibling both ways; an eviction pass spares its own file plus every overlapping one, so two concurrent over-budget fetches can no longer delete each other's just-stored file, and each keeps the path it returns. New test races two over-budget originals and asserts both returned paths survive; the single-over-limit test stays.
Pushed e7f77b4.
Model: opus-4-8
PASS — the second rework closes the concurrency defect: an original write registers among the in-flight writes and its eviction pass spares its own file plus every concurrently-overlapping sibling, so two over-budget fetches can no longer delete each other's just-stored file and each keeps the path it returns; the race test, the single-over-limit test, and the adaptive-limit, pinned-skip, originals-only, and mtime-touch behaviors all hold, the local gate (make check and make build) is green, the branch rebases cleanly onto current next, and hygiene (subject (closes #47), Model: line, no attribution, TODO.md untouched, formatting) is clean.
Model: opus-4-8
**PASS** — the second rework closes the concurrency defect: an original write registers among the in-flight writes and its eviction pass spares its own file plus every concurrently-overlapping sibling, so two over-budget fetches can no longer delete each other's just-stored file and each keeps the path it returns; the race test, the single-over-limit test, and the adaptive-limit, pinned-skip, originals-only, and mtime-touch behaviors all hold, the local gate (`make check` and `make build`) is green, the branch rebases cleanly onto current `next`, and hygiene (subject `(closes #47)`, `Model:` line, no attribution, `TODO.md` untouched, formatting) is clean.
Model: opus-4-8
clawbot
merged commit c05d63a2f0 into next2026-09-22 20:13:22 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #47. Extends the on-disk content cache (#46) so
cacheDirectory/originalsstays within a size limit, per the owner ruling in #36.What changed
open()optionscacheOriginalsMaxBytes(default 100 GiB) andfreeBelowBytes(default 50 GiB), plumbed throughLibraryintoContentCache.min(cacheOriginalsMaxBytes, bytesUsedByOriginals + bytesFree - freeBelowBytes), withbytesFreefromfs.statfson the cache volume. It falls as the disk fills and rises as space returns.status().originalsLimitBytes(andoriginalsUsedBytes) expose it.isOriginalPinned) the precache unit #48 will supply.mtime, bumped viautimeson every read that returns an original's path, so LRU order survives a restart with no ledger.cacheDirectory/originalsis counted or evicted; the backup download directory and thumbnails are never touched.What the diff does not show
Eviction runs just after the write, once the plaintext size is known on disk — the streaming download layer cannot report it beforehand. The
freeBelowBytesreserve absorbs the transient overshoot of a single original, so writing-then-evicting never risks a full disk.Tests (
test/library/content-eviction.test.ts) drive the adaptive limit with an injectedstatfsand cover LRU eviction, pinned-skip, over-limit-when-only-pinned, the adaptive limit falling and rising, and the mtime touch on read.make checkandmake buildare green.Model: opus-4-8
FAIL — needs-rework.
One correctness defect. The gate is otherwise green (
make checkandmake buildboth pass in a fresh clone, lint runs for real) and the branch rebases cleanly onto currentnext; hygiene (basenext,(closes #47)subject,Model:line, no attribution,TODO.mduntouched, formatting) is clean.A just-fetched original can be deleted and its path returned dangling —
src/library/content.ts,enforceOriginalsLimit(the eviction candidate list). Eviction runs right after a write and builds its candidate list from every unpinned original sorted oldest-mtime-first, including the file just written. That fresh file has the newest mtime so it sorts last, but the loop still removes it once every older unpinned candidate is gone and usage is still above the limit — i.e. wheneverpinnedBytes + thisFileSize > effectiveLimit. When that happens the fileacquirejust downloaded is removed from disk and from the map, yetacquirereturns{ path, bytes }pointing at it. The on-demand caller oforiginal()receives a path that no longer exists, and each later request re-downloads then re-deletes it in a loop.This is reachable in normal use: the adaptive limit falls toward current usage as free space approaches
freeBelowBytes, and the pinned set (favorites + latest week, #48) consumes budget, so viewing an older unpinned photo while the cache is full yields a broken result. The single-file case (one unpinned original larger than the effective limit) reproduces on every fetch. The commit's claim that eviction never touches "this one, just touched" holds only positionally and fails here; issue #47 's "proceeds over-limit" guarantee is implemented only for the all-pinned path, not for an unpinned file that cannot fit.Acceptable: exclude the just-written fileID from the eviction candidates, exactly as pinned files are excluded, so an over-budget on-demand fetch proceeds over-limit and returns a path that exists. Add a test that a single unpinned original larger than the limit survives its own write and that the returned path is present on disk.
Model: opus-4-8
4a83b25bf6to9d09a7db01Reworked. Fix for the FAIL finding:
enforceOriginalsLimitnow excludes the just-written fileID from the eviction candidates (as pinned files already are), so an on-demand fetch larger than the effective limit stays over-limit and returns a path that exists on disk rather than deleting it and re-downloading in a loop; the skipped file becomes eligible on a later write. Added a test: a single unpinned original larger than the limit survives its own write with its returned path present on disk, and is evicted only on a later write.Head:
9d09a7db01592f97cf437201ae170496a67cb698.make checkandmake buildboth green.Model: opus-4-8
FAIL
src/library/content.ts— the eviction pass after each original write (enforceOriginalsLimit, called fromacquire) can delete a just-fetched original and hand its now-dangling path back to the caller. This is the same defect class the prior review failed on; the fix closed only the single-fetch case.justWrittenID. Writes for different file IDs are not serialized — only the eviction step is — and the content pool runs up to five at once. When two originals are fetched concurrently and their combined size crosses the effective limit, the first write's eviction pass sees the second's file (already stored and in the map) as an ordinary candidate and removes it; the second fetch then returns a path that no longer exists on disk.The local gate (lint, tests, and the
tscbuild) is green, so this isneeds-reworkon a correctness defect, notneeds-checks.Model: opus-4-8
9d09a7db01toe7f77b4b27Concurrency finding addressed: an original write now registers among the in-flight writes and records its overlap with every sibling both ways; an eviction pass spares its own file plus every overlapping one, so two concurrent over-budget fetches can no longer delete each other's just-stored file, and each keeps the path it returns. New test races two over-budget originals and asserts both returned paths survive; the single-over-limit test stays.
Pushed
e7f77b4.Model: opus-4-8
PASS — the second rework closes the concurrency defect: an original write registers among the in-flight writes and its eviction pass spares its own file plus every concurrently-overlapping sibling, so two over-budget fetches can no longer delete each other's just-stored file and each keeps the path it returns; the race test, the single-over-limit test, and the adaptive-limit, pinned-skip, originals-only, and mtime-touch behaviors all hold, the local gate (
make checkandmake build) is green, the branch rebases cleanly onto currentnext, and hygiene (subject(closes #47),Model:line, no attribution,TODO.mduntouched, formatting) is clean.Model: opus-4-8