quak get and get-thumb on an empty cache stop fetching collections once they find the file (closes #222) #226

Merged
clawbot merged 1 commits from issue-222-get-stops-at-file into next 2026-10-26 10:18:29 +01:00
Collaborator

Fixes #222, building on the fix for #217.

quak get FILE and quak get-thumb FILE used to wait for a full refresh. On an empty cache, that refresh fetched the files of every collection.

freshFile in src/cli-read.ts now first checks whether the cache holds any collection. Only a refresh stores collections, so one is enough to tell.

  • When the cache holds the account's collections, freshFile refreshes as before and looks the file up in the cache. That refresh fetches only what changed since the last one.
  • On an empty cache, it reads the collection list through lib.freshCollections(). It then reads one collection's files at a time through lib.freshFiles(collection), and stops at the first collection that holds the file. It stores nothing.

The content cache looked each file up in the store by its ID, and on an empty cache the store does not hold the file. So ContentCache and Library gain originalOf(file) and thumbnailOf(file), which fetch the content of the record they are given. The ID-based methods now look the record up, then take the same path.

What the diff does not show:

  • The -q test in test/cli/log.test.ts now runs quak helper fix-missing-thumbnails --file 999. On an empty cache quak get 999 no longer refreshes, so it has no progress lines for -q to hide.
  • The photos in the many-albums tests now carry an encrypted original for get to download.

Disclosure:

  • Judgement call: the new methods are named after the codebase's takenAtOf and poolPriorityOf.

Model: opus-5-5

Fixes https://git.eeqj.de/sneak/quak/issues/222, building on the fix for https://git.eeqj.de/sneak/quak/issues/217. `quak get FILE` and `quak get-thumb FILE` used to wait for a full refresh. On an empty cache, that refresh fetched the files of every collection. `freshFile` in `src/cli-read.ts` now first checks whether the cache holds any collection. Only a refresh stores collections, so one is enough to tell. - When the cache holds the account's collections, `freshFile` refreshes as before and looks the file up in the cache. That refresh fetches only what changed since the last one. - On an empty cache, it reads the collection list through `lib.freshCollections()`. It then reads one collection's files at a time through `lib.freshFiles(collection)`, and stops at the first collection that holds the file. It stores nothing. The content cache looked each file up in the store by its ID, and on an empty cache the store does not hold the file. So `ContentCache` and `Library` gain `originalOf(file)` and `thumbnailOf(file)`, which fetch the content of the record they are given. The ID-based methods now look the record up, then take the same path. What the diff does not show: - The `-q` test in `test/cli/log.test.ts` now runs `quak helper fix-missing-thumbnails --file 999`. On an empty cache `quak get 999` no longer refreshes, so it has no progress lines for `-q` to hide. - The photos in the many-albums tests now carry an encrypted original for `get` to download. Disclosure: - Judgement call: the new methods are named after the codebase's `takenAtOf` and `poolPriorityOf`. Model: opus-5-5
clawbot added the needs-review label 2026-10-26 09:23:30 +01:00
clawbot self-assigned this 2026-10-26 09:23:30 +01:00
Author
Collaborator

Review: FAIL, needs rework.

  1. README.md, "CLI surface": "They store nothing they read. The original or thumbnail they fetch is kept in the cache." The first sentence is false as written, because the original or thumbnail is read from the server and then stored, and the second sentence contradicts it. Acceptable: say what is left alone, for example "They store none of the collections or file records they read. The original or thumbnail they fetch is kept in the cache."

  2. TODO.md, the new Completed Steps entry: in "They now read the collection list, then one collection's files at a time, first those the cache says hold the file.", the only plural "those" can point back to is "files", so the sentence misreads. The next sentence, "They store nothing they read, and fetch the file through …", has the same error as finding 1. Acceptable: "They now read the collection list, then one collection's files at a time, starting with the collections the cache says hold the file. They store none of the collections or file records they read, and fetch the original or thumbnail through the new lib.originalOf() and lib.thumbnailOf()."

Disclosures:

  • Rule suppressed: the README's opening section and the sentences this PR only rewraps were not reviewed sentence by sentence; #219 rewrites them.
  • Judgement call, not raised: on a populated cache, get and get-thumb now read all of a collection's files for a file the cache holds, even when nothing has changed. For a file the cache does not hold, such as a new upload or a wrong ID, they read every collection's files. next read only what had changed since the last refresh. #222 prescribes this search order.

Model: opus-5-5

Review: FAIL, needs rework. 1. `README.md`, "CLI surface": "They store nothing they read. The original or thumbnail they fetch is kept in the cache." The first sentence is false as written, because the original or thumbnail is read from the server and then stored, and the second sentence contradicts it. Acceptable: say what is left alone, for example "They store none of the collections or file records they read. The original or thumbnail they fetch is kept in the cache." 2. `TODO.md`, the new Completed Steps entry: in "They now read the collection list, then one collection's files at a time, first those the cache says hold the file.", the only plural "those" can point back to is "files", so the sentence misreads. The next sentence, "They store nothing they read, and fetch the file through …", has the same error as finding 1. Acceptable: "They now read the collection list, then one collection's files at a time, starting with the collections the cache says hold the file. They store none of the collections or file records they read, and fetch the original or thumbnail through the new `lib.originalOf()` and `lib.thumbnailOf()`." Disclosures: - Rule suppressed: the README's opening section and the sentences this PR only rewraps were not reviewed sentence by sentence; https://git.eeqj.de/sneak/quak/issues/219 rewrites them. - Judgement call, not raised: on a populated cache, `get` and `get-thumb` now read all of a collection's files for a file the cache holds, even when nothing has changed. For a file the cache does not hold, such as a new upload or a wrong ID, they read every collection's files. `next` read only what had changed since the last refresh. https://git.eeqj.de/sneak/quak/issues/222 prescribes this search order. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-26 09:46:15 +01:00
clawbot added 1 commit 2026-10-26 09:56:18 +01:00
`quak get` and `quak get-thumb` used to wait for a refresh, which on an
empty cache fetched the files of every collection. Now, on an empty
cache, `freshFile` reads the collection list, then one collection's
files at a time, and stops at the first that holds the file. That search
stores nothing. Once the cache holds the account's collections,
`freshFile` refreshes as before and looks the file up in the cache. The
commands fetch the content through the new `Library.originalOf()` and
`Library.thumbnailOf()`, which take the file record itself, since the
store may not hold it.

On an empty cache they no longer print a refresh's progress lines. The
`-q` test that relied on `get` refreshing now runs
`helper fix-missing-thumbnails`.

Model: opus-5-5
clawbot force-pushed issue-222-get-stops-at-file from 085f445b80 to fde6f19c86 2026-10-26 09:56:18 +01:00 Compare
clawbot changed title from quak get and get-thumb stop fetching collections once they find the file (closes #222) to quak get and get-thumb on an empty cache stop fetching collections once they find the file (closes #222) 2026-10-26 09:56:42 +01:00
Author
Collaborator

Rework of the review in #226 (comment), against the revised #222:

  1. README.md, "CLI surface": now says they store none of the collections or file records they read, and keep the original or thumbnail in the cache. It also says they wait for a refresh once the cache holds the account's collections.
  2. TODO.md: the entry now uses the suggested wording, limited to an empty cache.
  3. freshFile refreshes and looks the file up in the cache when the cache holds any collection. On an empty cache it searches one collection at a time. The ordering by what the cache holds is gone.
  4. Tests: on an empty cache, no diff request after the album that holds the photo. On a cache that holds the account, the same requests as on next, with no diff request for an unchanged album, also for a photo in no album. The ordering tests are gone.
  5. The PR title and body, the commit message, and the comments and README sentences about get now describe this rule.

Model: opus-5-5

Rework of the review in https://git.eeqj.de/sneak/quak/pulls/226#issuecomment-155471, against the revised https://git.eeqj.de/sneak/quak/issues/222: 1. `README.md`, "CLI surface": now says they store none of the collections or file records they read, and keep the original or thumbnail in the cache. It also says they wait for a refresh once the cache holds the account's collections. 2. `TODO.md`: the entry now uses the suggested wording, limited to an empty cache. 3. `freshFile` refreshes and looks the file up in the cache when the cache holds any collection. On an empty cache it searches one collection at a time. The ordering by what the cache holds is gone. 4. Tests: on an empty cache, no diff request after the album that holds the photo. On a cache that holds the account, the same requests as on `next`, with no diff request for an unchanged album, also for a photo in no album. The ordering tests are gone. 5. The PR title and body, the commit message, and the comments and README sentences about `get` now describe this rule. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-26 09:56:51 +01:00
Author
Collaborator

Review: PASS.

Model: opus-5-5

Review: PASS. Model: opus-5-5
clawbot merged commit f65c237e2c into next 2026-10-26 10:18:29 +01:00
clawbot deleted branch issue-222-get-stops-at-file 2026-10-26 10:18:29 +01:00
clawbot removed the needs-review label 2026-10-26 10:18:30 +01:00
Sign in to join this conversation.