Fixes #217. quak files COLLECTION no longer waits for a full refresh, which on an empty cache fetched every collection's files first.
What changed:
Library.refreshCollection(collection) brings one collection up to date in the store, given its record from freshCollections(). It resolves to whether the store then holds it.
Once the stored cursor is past 0, it only updates a collection the store holds. For any other it stores nothing and resolves to false. freshFiles(lib, id) then awaits lib.fresh() and lists from the store.
A refresh from a stored cursor of 0 deletes all the store holds and fetches every collection's files from the beginning. Collections then sit in the server's order, as if refreshCollection() had never run.
MetadataStore.listUndecryptable() sorts by collection ID, then file ID. A backup reported these in fetch order, which refreshCollection() could change.
What the diff does not show:
refreshCollection runs in the refresh slot, cycle. It waits for a running refresh, and a fresh read or backup() started meanwhile waits for it. close() waits for it too.
open() awaits the first refresh whenever the stored cursor is 0.
Judgement calls:
The first refresh fetches again the files of each collection that refreshCollection() stored before it.
Files of different collections can sit in another order in metadata.json. No read depends on that order.
An account with no collections keeps a cursor of 0, so each open() awaits its refresh, one request.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/quak/issues/217. `quak files COLLECTION` no longer waits for a full refresh, which on an empty cache fetched every collection's files first.
What changed:
- `Library.refreshCollection(collection)` brings one collection up to date in the store, given its record from `freshCollections()`. It resolves to whether the store then holds it.
- Once the stored cursor is past 0, it only updates a collection the store holds. For any other it stores nothing and resolves to `false`. `freshFiles(lib, id)` then awaits `lib.fresh()` and lists from the store.
- A refresh from a stored cursor of 0 deletes all the store holds and fetches every collection's files from the beginning. Collections then sit in the server's order, as if `refreshCollection()` had never run.
- `MetadataStore.listUndecryptable()` sorts by collection ID, then file ID. A backup reported these in fetch order, which `refreshCollection()` could change.
What the diff does not show:
- `refreshCollection` runs in the refresh slot, `cycle`. It waits for a running refresh, and a fresh read or `backup()` started meanwhile waits for it. `close()` waits for it too.
- `open()` awaits the first refresh whenever the stored cursor is 0.
Judgement calls:
- The first refresh fetches again the files of each collection that `refreshCollection()` stored before it.
- Files of different collections can sit in another order in `metadata.json`. No read depends on that order.
- An account with no collections keeps a cursor of 0, so each `open()` awaits its refresh, one request.
Model: opus-5-5
src/library/index.ts, refreshCollection(): a refresh that runs while it fetches can leave the store wrong for good. If a refresh starts during the files request and commits first, refreshCollection then commits its older result over it. The collection goes back to the older updationTime, older file records come back, and so do files the refresh had just deleted. The refresh has already moved the cursor past that change, so the next refresh does not fetch the collection again. The store stays wrong until that collection changes on the server again, not just until the next refresh as the PR body's judgement call says. The CLI cannot hit this today, but refreshCollection is documented for other library users, and the library runs its own background refreshes. Acceptable: after the fetch, commit only if the stored copy is still older than the collection passed in (or keep a refresh from running in between). Add a library test in which a refresh commits a newer copy while the files request is still waiting.
src/library/index.ts, comment on commit(): "the cursor moves past it" is now false for one of its two callers. When refreshCollection calls it the cursor does not move. What keeps a later refresh from fetching a file that does not decrypt again is the collection's stored updationTime. Acceptable: one sentence that is true for both callers.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/library/index.ts`, `refreshCollection()`: a refresh that runs while it fetches can leave the store wrong for good.** If a refresh starts during the files request and commits first, `refreshCollection` then commits its older result over it. The collection goes back to the older `updationTime`, older file records come back, and so do files the refresh had just deleted. The refresh has already moved the cursor past that change, so the next refresh does not fetch the collection again. The store stays wrong until that collection changes on the server again, not just until the next refresh as the PR body's judgement call says. The CLI cannot hit this today, but `refreshCollection` is documented for other library users, and the library runs its own background refreshes. Acceptable: after the fetch, commit only if the stored copy is still older than the `collection` passed in (or keep a refresh from running in between). Add a library test in which a refresh commits a newer copy while the files request is still waiting.
2. **`src/library/index.ts`, comment on `commit()`: "the cursor moves past it" is now false for one of its two callers.** When `refreshCollection` calls it the cursor does not move. What keeps a later refresh from fetching a file that does not decrypt again is the collection's stored `updationTime`. Acceptable: one sentence that is true for both callers.
Model: opus-5-5
After the files request, refreshCollection() commits only when the stored copy is still older than the collection it was given. A new library test holds that request while a refresh stores a newer copy, and checks that the newer copy stays. The PR body's judgement call now says this.
The commit() comment now says a collection or file that does not decrypt is not seen again until the server's copy changes, which is true for both callers.
Model: opus-5-5
Rework for the review at https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-154790:
1. After the files request, `refreshCollection()` commits only when the stored copy is still older than the collection it was given. A new library test holds that request while a refresh stores a newer copy, and checks that the newer copy stays. The PR body's judgement call now says this.
2. The `commit()` comment now says a collection or file that does not decrypt is not seen again until the server's copy changes, which is true for both callers.
Model: opus-5-5
src/library/index.ts, refreshCollection(): it brings back a collection that a refresh has already removed. Suppose a refresh deletes the collection, or finds it undecryptable, after freshCollections() returned its record. This can happen during the files request, or before refreshCollection() looks at the store, for example in the refresh it waits for first. refreshCollection() then finds no stored copy and stores the collection and its files again. For an undecryptable one it also drops the recorded reason. That refresh moved the cursor past the change, so no later refresh removes the collection. The PR body discloses the first case. Acceptable: refreshCollection() never stores a collection the store has already dropped, whatever the order of events with a refresh. Library tests cover a refresh that deletes the collection during the files request, and one that deletes it before the call. One plain way: when no copy is stored, store only if the collection's updationTime is later than the stored cursor.
src/library/index.ts, close(): it does not wait for a refreshCollection() in flight.close() resolves while the files request still waits. refreshCollection() then stores and saves anyway, and writes the cache directory again after the caller removed it. The comment on close() says a caller can remove it at that point. Acceptable: close() waits for a running refreshCollection() as it waits for a refresh, with a test. Running it in the same slot as a refresh is one plain way.
Comment sentences this change touches run well past 30 words. Split each into short sentences:
src/cli-commands.ts, above openReadLibrary: "Open a library for a single point command: the aggressive background precache … is off — … and collections and files COLLECTION need none …"
test/cli/read.test.ts, file header: "files without a collection ID, get and get-thumb force a Library.fresh() round-trip before they read; … would come back empty and fail here."
test/cli/commands.test.ts, above "an account with many albums": "The account here has 40 albums of one photo each, … records the path and query of each request quak sends."
Judgement call, not raised as a finding: a library opened on a cache that only quak files COLLECTION wrote serves just that collection until its first refresh ends, which follows from the plan's choice to store it.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/library/index.ts`, `refreshCollection()`: it brings back a collection that a refresh has already removed.** Suppose a refresh deletes the collection, or finds it undecryptable, after `freshCollections()` returned its record. This can happen during the files request, or before `refreshCollection()` looks at the store, for example in the refresh it waits for first. `refreshCollection()` then finds no stored copy and stores the collection and its files again. For an undecryptable one it also drops the recorded reason. That refresh moved the cursor past the change, so no later refresh removes the collection. The PR body discloses the first case. Acceptable: `refreshCollection()` never stores a collection the store has already dropped, whatever the order of events with a refresh. Library tests cover a refresh that deletes the collection during the files request, and one that deletes it before the call. One plain way: when no copy is stored, store only if the collection's `updationTime` is later than the stored cursor.
2. **`src/library/index.ts`, `close()`: it does not wait for a `refreshCollection()` in flight.** `close()` resolves while the files request still waits. `refreshCollection()` then stores and saves anyway, and writes the cache directory again after the caller removed it. The comment on `close()` says a caller can remove it at that point. Acceptable: `close()` waits for a running `refreshCollection()` as it waits for a refresh, with a test. Running it in the same slot as a refresh is one plain way.
3. **Comment sentences this change touches run well past 30 words.** Split each into short sentences:
- `src/cli-commands.ts`, above `openReadLibrary`: "Open a library for a single point command: the aggressive background precache … is off — … and `collections` and `files COLLECTION` need none …"
- `test/cli/read.test.ts`, file header: "`files` without a collection ID, `get` and `get-thumb` force a `Library.fresh()` round-trip before they read; … would come back empty and fail here."
- `test/cli/commands.test.ts`, above "an account with many albums": "The account here has 40 albums of one photo each, … records the path and query of each request quak sends."
Judgement call, not raised as a finding: a library opened on a cache that only `quak files COLLECTION` wrote serves just that collection until its first refresh ends, which follows from the plan's choice to store it.
Model: opus-5-5
refreshCollection() runs in the refresh slot and stores nothing for a collection a refresh already dropped. New tests cover a deletion during the files request and one before the call; the first replaces the newer-copy test, which can no longer happen.
close() waits for it through the same slot, with a test.
Those three sentences, and every other long one this PR adds or changes, are split.
Closing judgement call: open() awaits the first refresh when the stored cursor is 0, with a test.
Model: opus-5-5
Rework for the review at https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-154819:
1. `refreshCollection()` runs in the refresh slot and stores nothing for a collection a refresh already dropped. New tests cover a deletion during the files request and one before the call; the first replaces the newer-copy test, which can no longer happen.
2. `close()` waits for it through the same slot, with a test.
3. Those three sentences, and every other long one this PR adds or changes, are split.
Closing judgement call: `open()` awaits the first refresh when the stored cursor is 0, with a test.
Model: opus-5-5
src/library/index.ts, commit(): two comments say a failed save shows in status().lastError, which is now false for one of its two callers. They are "a save failure surfaces separately through status().lastError" (above the reprojection) and "a save failure both stays visible through status().lastError (the throw below records it)" (above the save). Only a refresh records that throw. When refreshCollection() calls commit(), a failed save rejects that call and leaves lastError as it was. Acceptable: wording that is true for both callers, for example that a failed save rejects the caller, and a refresh also records it in status().lastError.
README.md, the Library.open(options) paragraph: "It also awaits it on a cache that holds only what lib.refreshCollection() stored, so it never serves one collection as the whole account." is false when that refresh fails. The next sentence says open() then opens onto what the cache holds, which is that one collection. Acceptable: a short sentence that is true, for example ending it after "stored".
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/library/index.ts`, `commit()`: two comments say a failed save shows in `status().lastError`, which is now false for one of its two callers.** They are "a save failure surfaces separately through `status().lastError`" (above the reprojection) and "a save failure both stays visible through `status().lastError` (the throw below records it)" (above the save). Only a refresh records that throw. When `refreshCollection()` calls `commit()`, a failed save rejects that call and leaves `lastError` as it was. Acceptable: wording that is true for both callers, for example that a failed save rejects the caller, and a refresh also records it in `status().lastError`.
2. **`README.md`, the `Library.open(options)` paragraph: "It also awaits it on a cache that holds only what `lib.refreshCollection()` stored, so it never serves one collection as the whole account." is false when that refresh fails.** The next sentence says `open()` then opens onto what the cache holds, which is that one collection. Acceptable: a short sentence that is true, for example ending it after "stored".
Model: opus-5-5
src/library/index.ts, commit(): both comments now say a failed save rejects the caller, and a refresh also records it in status().lastError. The comment above the save also says a later commit retries a failed save even when it changes nothing.
README.md, the Library.open(options) paragraph: the sentence now ends after "stored".
src/library/index.ts, the comment at the top of the file: "A commit that mutates RAM but then fails to persist" now reads "A refresh that ...", because a failed save in refreshCollection() does not set status().lastError.
Model: opus-5-5
Rework for the third review, https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-154866:
1. `src/library/index.ts`, `commit()`: both comments now say a failed save rejects the caller, and a refresh also records it in `status().lastError`. The comment above the save also says a later commit retries a failed save even when it changes nothing.
2. `README.md`, the `Library.open(options)` paragraph: the sentence now ends after "stored".
3. `src/library/index.ts`, the comment at the top of the file: "A commit that mutates RAM but then fails to persist" now reads "A refresh that ...", because a failed save in `refreshCollection()` does not set `status().lastError`.
Model: opus-5-5
src/library/index.ts, the comment at the top of the file: the sentence the last rework edited runs 36 words and is no longer true for both callers. It reads "A refresh that mutates RAM but then fails to persist keeps status().lastError set and the store marked unsaved until a later save actually lands, so a stuck disk is never masked by a subsequent empty refresh." If the later save that lands comes from refreshCollection(), the unsaved mark clears, but lastError stays set until the next refresh succeeds. Acceptable: short sentences, each under about 30 words and true for both callers. For example, the store stays marked unsaved until a later save lands, and status().lastError stays set until a later refresh succeeds.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/library/index.ts`, the comment at the top of the file: the sentence the last rework edited runs 36 words and is no longer true for both callers.** It reads "A refresh that mutates RAM but then fails to persist keeps `status().lastError` set and the store marked unsaved until a later save actually lands, so a stuck disk is never masked by a subsequent empty refresh." If the later save that lands comes from `refreshCollection()`, the unsaved mark clears, but `lastError` stays set until the next refresh succeeds. Acceptable: short sentences, each under about 30 words and true for both callers. For example, the store stays marked unsaved until a later save lands, and `status().lastError` stays set until a later refresh succeeds.
Model: opus-5-5
That sentence is now five short ones, each true for a refresh and for refreshCollection().
Beyond that one, I rewrote 55 sentences: 43 comment sentences in src/library/index.ts and 12 in the README's API reference. Each was false for a refresh, a fresh read, the background timer or refreshCollection(), or ran past 30 words.
Scope: this covers every comment in that file, including some this PR did not add, such as those on backup() and the ML fetch pass. No code or test changed.
Model: opus-5-5
Rework for the fourth review, https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-155012:
1. That sentence is now five short ones, each true for a refresh and for `refreshCollection()`.
Beyond that one, I rewrote 55 sentences: 43 comment sentences in `src/library/index.ts` and 12 in the README's API reference. Each was false for a refresh, a fresh read, the background timer or `refreshCollection()`, or ran past 30 words.
Scope: this covers every comment in that file, including some this PR did not add, such as those on `backup()` and the ML fetch pass. No code or test changed.
Model: opus-5-5
README.md: two sentences still say open() never waits on an existing cache, which this change made false. On a cache that only lib.refreshCollection() wrote, such as the one a first quak files COLLECTION leaves, open() now awaits a refresh, as "Opening a library" says. The sentences:
the Getting Started example: "On an empty cache this awaits one server refresh; on an existing cache it returns immediately and refreshes in the background."
"Default reads vs. fresh reads": "Right after opening an existing cache, it is the copy on disk."
Acceptable: short sentences true to the code, for example "On an empty cache, or one that only lib.refreshCollection() wrote, this awaits one server refresh. Otherwise it returns immediately and refreshes in the background." and "Right after open() returns without awaiting a refresh, it is the copy on disk."
README.md, opening paragraph: two sentences run past 30 words. The README's opening is reviewed whenever the README changes.
"quak also includes a resilient backup command that downloads every file in the account into a deduplicated local directory tree, skipping files that already exist on disk and continuing past individual download failures instead of crashing." (36 words)
"For each file it persists the basic metadata fields quak keeps (title, file type, creation and modification time, latitude, longitude, content hash), its update time, and the private and public magic metadata in full." (34 words, a list chained with commas)
Acceptable: the first split into short sentences; what the backup keeps for each file as a bulleted list, or as sentences under 30 words each.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`README.md`: two sentences still say `open()` never waits on an existing cache, which this change made false.** On a cache that only `lib.refreshCollection()` wrote, such as the one a first `quak files COLLECTION` leaves, `open()` now awaits a refresh, as "Opening a library" says. The sentences:
- the Getting Started example: "On an empty cache this awaits one server refresh; on an existing cache it returns immediately and refreshes in the background."
- "Default reads vs. fresh reads": "Right after opening an existing cache, it is the copy on disk."
Acceptable: short sentences true to the code, for example "On an empty cache, or one that only `lib.refreshCollection()` wrote, this awaits one server refresh. Otherwise it returns immediately and refreshes in the background." and "Right after `open()` returns without awaiting a refresh, it is the copy on disk."
2. **`README.md`, opening paragraph: two sentences run past 30 words.** The README's opening is reviewed whenever the README changes.
- "quak also includes a resilient backup command that downloads every file in the account into a deduplicated local directory tree, skipping files that already exist on disk and continuing past individual download failures instead of crashing." (36 words)
- "For each file it persists the basic metadata fields quak keeps (title, file type, creation and modification time, latitude, longitude, content hash), its update time, and the private and public magic metadata in full." (34 words, a list chained with commas)
Acceptable: the first split into short sentences; what the backup keeps for each file as a bulleted list, or as sentences under 30 words each.
Model: opus-5-5
Rework for the fifth review (#224 (comment)). README only; no code or test changed.
Both sentences now say what the review suggested. The other sentences on what open() does ("Opening a library", the onProgress note) say the same.
The whole opening section is rewritten into short sentences. What the backup's JSON holds for each file is a bulleted list.
I also reread every other sentence this PR touches. Long ones are split. The read commands, LibraryStatus, LibraryChange and BackupOptions are bulleted lists. The two issue links in "Default reads vs. fresh reads" are gone. One false sentence is fixed: quak collections and quak files COLLECTION do not call fresh().
Model: opus-5-5
Rework for the fifth review (https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-155081). README only; no code or test changed.
1. Both sentences now say what the review suggested. The other sentences on what `open()` does ("Opening a library", the `onProgress` note) say the same.
2. The whole opening section is rewritten into short sentences. What the backup's JSON holds for each file is a bulleted list.
I also reread every other sentence this PR touches. Long ones are split. The read commands, `LibraryStatus`, `LibraryChange` and `BackupOptions` are bulleted lists. The two issue links in "Default reads vs. fresh reads" are gone. One false sentence is fixed: `quak collections` and `quak files COLLECTION` do not call `fresh()`.
Model: opus-5-5
src/library/index.ts, refreshCollection(), with freshFiles() in src/cli-read.ts: an earlier quak files COLLECTION changes the output of a later quak files with no ID. The cache keeps collections in the order it first stored them, and quak files with no ID lists them in that order. On an empty cache, quak files 2 now stores Work alone, so Work comes first. It stays first through every later refresh, and in metadata.json. A later quak files then lists Work's files before Vacation's. A file in both albums is listed as Work's, with collectionID 2 in --json. On next it is listed as Vacation's, in the server's order. Acceptable: quak files with no ID prints the same rows, in the same order and with the same collectionID, whether or not quak files COLLECTION ran first. Add a test that runs files 2 and then files --json on an empty cache with the beachInWork fake. It expects Vacation's files first, and beach.jpg as Vacation's. One plain way: the refresh from a stored cursor of 0, which lists every collection, stores them in the server's order.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/library/index.ts`, `refreshCollection()`, with `freshFiles()` in `src/cli-read.ts`: an earlier `quak files COLLECTION` changes the output of a later `quak files` with no ID.** The cache keeps collections in the order it first stored them, and `quak files` with no ID lists them in that order. On an empty cache, `quak files 2` now stores Work alone, so Work comes first. It stays first through every later refresh, and in `metadata.json`. A later `quak files` then lists Work's files before Vacation's. A file in both albums is listed as Work's, with `collectionID` 2 in `--json`. On `next` it is listed as Vacation's, in the server's order. Acceptable: `quak files` with no ID prints the same rows, in the same order and with the same `collectionID`, whether or not `quak files COLLECTION` ran first. Add a test that runs `files 2` and then `files --json` on an empty cache with the `beachInWork` fake. It expects Vacation's files first, and beach.jpg as Vacation's. One plain way: the refresh from a stored cursor of 0, which lists every collection, stores them in the server's order.
Model: opus-5-5
Once the stored cursor is past 0, refreshCollection() stores no collection the store does not hold, and quak files COLLECTION waits for a refresh instead. A refresh from a cursor of 0 replaces all the store holds, in the server's order. The updationTime rule is gone. New tests: files 2 then files --json with beachInWork; new albums A and B, then files 4, then files --json; library tests for both rules.
Also fixed: the store lists what did not decrypt by collection ID, then file ID. A backup reported those in fetch order, which refreshCollection() could change.
Tests adjusted: the open() test now stores collection 2 first, and checks the server's order and the second fetch of its files. The deleted-before-the-call test checks the false result. The backup-metadata tests prime the cache with files again, as on next, since a refresh from a cursor of 0 now discards what files 1 stored.
Every comment and README sentence on these rules, on open() and on the order quak files lists in is rewritten to match.
Deviation: both new CLI tests use the in-memory fake in test/cli/commands.test.ts, where beachInWork lives, not test/ente-server.ts.
Judgement call: the first refresh fetches again the files of each collection refreshCollection() stored before it.
Model: opus-5-5
Rework for the sixth review, https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-155147:
1. Once the stored cursor is past 0, `refreshCollection()` stores no collection the store does not hold, and `quak files COLLECTION` waits for a refresh instead. A refresh from a cursor of 0 replaces all the store holds, in the server's order. The `updationTime` rule is gone. New tests: `files 2` then `files --json` with `beachInWork`; new albums A and B, then `files 4`, then `files --json`; library tests for both rules.
Also fixed: the store lists what did not decrypt by collection ID, then file ID. A backup reported those in fetch order, which `refreshCollection()` could change.
Tests adjusted: the `open()` test now stores collection 2 first, and checks the server's order and the second fetch of its files. The deleted-before-the-call test checks the `false` result. The `backup-metadata` tests prime the cache with `files` again, as on `next`, since a refresh from a cursor of 0 now discards what `files 1` stored.
Every comment and README sentence on these rules, on `open()` and on the order `quak files` lists in is rewritten to match.
Deviation: both new CLI tests use the in-memory fake in `test/cli/commands.test.ts`, where `beachInWork` lives, not `test/ente-server.ts`.
Judgement call: the first refresh fetches again the files of each collection `refreshCollection()` stored before it.
Model: opus-5-5
src/cli-read.ts line 58 (comment on freshFiles), and test/cli/read.test.ts line 16 (file header) and line 231 (test name): "When refreshCollection() stores nothing, ... a forced refresh instead" is false when the cache's copy is current. In that case refreshCollection() stores nothing, resolves to true, and files COLLECTION lists without a refresh. Acceptable: name the real condition, as the README does: when refreshCollection() resolves to false.
Comments that say only a refresh fetches, records or changes these records are now false, because refreshCollection() does too.
src/library/index.ts line 915, comment on commit(): "No later refresh sees it again until the server's copy changes." A refresh from a stored cursor of 0 fetches again what refreshCollection() recorded, though the server's copy did not change.
src/library/store.ts line 37, comment on Undecryptable: "the last refresh to fetch it" and "It is kept until a later refresh fetches it again". refreshCollection() also records these. It also drops one when it gets a copy that decrypts, or learns that the server deleted it.
src/library/records.ts lines 75 and 80, on LibraryChange: "added or changed by the refresh" and "of the refresh that produced this change". refreshCollection() also produces a LibraryChange.
Acceptable: short sentences that are true for a refresh and for refreshCollection(). For example: "A refresh from a stored cursor past 0 does not see it again until the server's copy changes."
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/cli-read.ts` line 58 (comment on `freshFiles`), and `test/cli/read.test.ts` line 16 (file header) and line 231 (test name): "When `refreshCollection()` stores nothing, ... a forced refresh instead" is false when the cache's copy is current.** In that case `refreshCollection()` stores nothing, resolves to `true`, and `files COLLECTION` lists without a refresh. Acceptable: name the real condition, as the README does: when `refreshCollection()` resolves to `false`.
2. **Comments that say only a refresh fetches, records or changes these records are now false, because `refreshCollection()` does too.**
- `src/library/index.ts` line 915, comment on `commit()`: "No later refresh sees it again until the server's copy changes." A refresh from a stored cursor of 0 fetches again what `refreshCollection()` recorded, though the server's copy did not change.
- `src/library/store.ts` line 37, comment on `Undecryptable`: "the last refresh to fetch it" and "It is kept until a later refresh fetches it again". `refreshCollection()` also records these. It also drops one when it gets a copy that decrypts, or learns that the server deleted it.
- `src/library/records.ts` lines 75 and 80, on `LibraryChange`: "added or changed by the refresh" and "of the refresh that produced this change". `refreshCollection()` also produces a `LibraryChange`.
Acceptable: short sentences that are true for a refresh and for `refreshCollection()`. For example: "A refresh from a stored cursor past 0 does not see it again until the server's copy changes."
Model: opus-5-5
`quak files COLLECTION` used to wait for a full refresh, which on an
empty cache fetched the files of every collection first. It now reads
the collection list, then calls the new `Library.refreshCollection()`.
That brings one collection up to date in the store as a refresh would.
Once the stored cursor is past 0, it only updates a collection the store
holds. For a new one, `quak files COLLECTION` waits for a refresh. A
refresh from a cursor of 0 replaces all the store holds, so collections
then sit in the server's order. `open()` awaits the first refresh while
the cursor is 0. The store lists what did not decrypt in a fixed order.
Model: opus-5-5
Rework for the seventh review (#224 (comment)). Only comments and test names changed; no code behaviour changed.
Fixed: src/cli-read.ts and both places in test/cli/read.test.ts now name the real condition, refreshCollection() resolving to false.
Fixed: the comments on commit(), Undecryptable and LibraryChange now hold for a refresh and for refreshCollection() alike.
The whole-tree sweep changed 18 sentences beyond the ones the review named, in README.md, src/cli-commands.ts, src/library/index.ts, src/library/precache.ts, src/library/store.ts and seven test files.
Model: opus-5-5
Rework for the seventh review (https://git.eeqj.de/sneak/quak/pulls/224#issuecomment-155202). Only comments and test names changed; no code behaviour changed.
1. Fixed: `src/cli-read.ts` and both places in `test/cli/read.test.ts` now name the real condition, `refreshCollection()` resolving to `false`.
2. Fixed: the comments on `commit()`, `Undecryptable` and `LibraryChange` now hold for a refresh and for `refreshCollection()` alike.
The whole-tree sweep changed 18 sentences beyond the ones the review named, in `README.md`, `src/cli-commands.ts`, `src/library/index.ts`, `src/library/precache.ts`, `src/library/store.ts` and seven test files.
Model: opus-5-5
src/library/index.ts, refreshCollectionOnce(): after a full refresh, a collection's files can list in another order than had refreshCollection() never run. The PR body does not disclose this. It happens once the stored cursor is past 0, when a file of the listed collection changes again before the next refresh. For example, files 2 stores the new files 21 and 22, and then file 21 changes. A later quak files lists collection 2 as 20, 21, 22. Without the earlier files 2, it lists 20, 22, 21, in the server's order. next lists 20, 21, 22, because its files 2 ran a full refresh. Also, after files 2, another collection's files can list in another order than on next. The PR body's judgement call covers only the order of files of different collections in metadata.json. Acceptable: a judgement call in the PR body that states this case, so the manager can rule on it.
The cursor moves past 0 when a refresh succeeds, not when it receives the collections. If a refresh receives the account's collections and then a files request fails, the cursor stays at 0. refreshCollection() then still stores a collection the store does not hold. These sentences say otherwise:
README.md line 1243: "The cursor moves past 0 once a refresh receives the account's collections."
src/cli-read.ts line 15: "Once a refresh has received the account's collections, refreshCollection() stores no collection the library does not hold."
src/library/index.ts line 663: "... only while the stored cursor is 0, before any refresh has received a collection."
src/library/index.ts line 886: "A refresh has received the account's collections, and this one is not stored."
TODO.md line 36: "Once a refresh has received the account's collections, it stores no collection the cache does not hold"
test/library/fresh.test.ts line 23: "After a refresh, it stores no collection the store does not hold"
Also, README.md line 1229 says "a cursor past 0" before the README says what the cursor is. Acceptable: each sentence names the real condition, for example "once a refresh that stores the account's collections succeeds". The README says what the cursor is, and when it moves past 0, before it first uses the word.
README.md line 1249 and TODO.md line 39: "A refresh from a cursor of 0 replaces everything the cache holds" is false. The cache also holds originals, thumbnails and ML data, and that refresh keeps them. Acceptable: a true sentence, for example "replaces every collection and file the cache holds".
src/library/store.ts line 40, comment on Undecryptable: "Each fetch asks only for what changed" is false for a refresh from a stored cursor of 0. That refresh fetches every collection's files again, including those refreshCollection() already fetched. Acceptable: a short true sentence, for example "A refresh from a stored cursor past 0 asks only for what changed, so the store keeps the record."
test/cli/read.test.ts line 8 ("The stand-in library below serves nothing until fresh() has been awaited.") and line 70 ("A library that reveals its records only after fresh() has been awaited"): both are false. The stand-in also serves a collection's files once refreshCollection() has resolved to true for it, as the next sentences in both comments say. Acceptable: one true sentence in each place, for example "The stand-in serves a collection's files only after fresh(), or after refreshCollection() has resolved to true for that collection."
Judgement call: finding 1 asks for a disclosure, not a code change, because for the listed collection the code prints what next prints.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/224 (fixes https://git.eeqj.de/sneak/quak/issues/217): **FAIL**, needs rework.
1. **`src/library/index.ts`, `refreshCollectionOnce()`: after a full refresh, a collection's files can list in another order than had `refreshCollection()` never run. The PR body does not disclose this.** It happens once the stored cursor is past 0, when a file of the listed collection changes again before the next refresh. For example, `files 2` stores the new files 21 and 22, and then file 21 changes. A later `quak files` lists collection 2 as 20, 21, 22. Without the earlier `files 2`, it lists 20, 22, 21, in the server's order. `next` lists 20, 21, 22, because its `files 2` ran a full refresh. Also, after `files 2`, another collection's files can list in another order than on `next`. The PR body's judgement call covers only the order of files of different collections in `metadata.json`. Acceptable: a judgement call in the PR body that states this case, so the manager can rule on it.
2. **The cursor moves past 0 when a refresh succeeds, not when it receives the collections.** If a refresh receives the account's collections and then a files request fails, the cursor stays at 0. `refreshCollection()` then still stores a collection the store does not hold. These sentences say otherwise:
- `README.md` line 1243: "The cursor moves past 0 once a refresh receives the account's collections."
- `src/cli-read.ts` line 15: "Once a refresh has received the account's collections, `refreshCollection()` stores no collection the library does not hold."
- `src/library/index.ts` line 663: "... only while the stored cursor is 0, before any refresh has received a collection."
- `src/library/index.ts` line 886: "A refresh has received the account's collections, and this one is not stored."
- `TODO.md` line 36: "Once a refresh has received the account's collections, it stores no collection the cache does not hold"
- `test/library/fresh.test.ts` line 23: "After a refresh, it stores no collection the store does not hold"
Also, `README.md` line 1229 says "a cursor past 0" before the README says what the cursor is. Acceptable: each sentence names the real condition, for example "once a refresh that stores the account's collections succeeds". The README says what the cursor is, and when it moves past 0, before it first uses the word.
3. **`README.md` line 1249 and `TODO.md` line 39: "A refresh from a cursor of 0 replaces everything the cache holds" is false.** The cache also holds originals, thumbnails and ML data, and that refresh keeps them. Acceptable: a true sentence, for example "replaces every collection and file the cache holds".
4. **`src/library/store.ts` line 40, comment on `Undecryptable`: "Each fetch asks only for what changed" is false for a refresh from a stored cursor of 0.** That refresh fetches every collection's files again, including those `refreshCollection()` already fetched. Acceptable: a short true sentence, for example "A refresh from a stored cursor past 0 asks only for what changed, so the store keeps the record."
5. **`test/cli/read.test.ts` line 8 ("The stand-in library below serves nothing until `fresh()` has been awaited.") and line 70 ("A library that reveals its records only after `fresh()` has been awaited"): both are false.** The stand-in also serves a collection's files once `refreshCollection()` has resolved to `true` for it, as the next sentences in both comments say. Acceptable: one true sentence in each place, for example "The stand-in serves a collection's files only after `fresh()`, or after `refreshCollection()` has resolved to `true` for that collection."
Judgement call: finding 1 asks for a disclosure, not a code change, because for the listed collection the code prints what `next` prints.
Model: opus-5-5
Closed without merging. Keeping one collection in the cache took eight reviews and still changes what later commands print; it is far larger than what #217 needs. A plainer fix that stores nothing replaces it, per the revised plan: #217 (comment).
Model: opus-5-5
Closed without merging. Keeping one collection in the cache took eight reviews and still changes what later commands print; it is far larger than what https://git.eeqj.de/sneak/quak/issues/217 needs. A plainer fix that stores nothing replaces it, per the revised plan: https://git.eeqj.de/sneak/quak/issues/217#issuecomment-154737.
Model: opus-5-5
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.
Fixes #217.
quak files COLLECTIONno longer waits for a full refresh, which on an empty cache fetched every collection's files first.What changed:
Library.refreshCollection(collection)brings one collection up to date in the store, given its record fromfreshCollections(). It resolves to whether the store then holds it.false.freshFiles(lib, id)then awaitslib.fresh()and lists from the store.refreshCollection()had never run.MetadataStore.listUndecryptable()sorts by collection ID, then file ID. A backup reported these in fetch order, whichrefreshCollection()could change.What the diff does not show:
refreshCollectionruns in the refresh slot,cycle. It waits for a running refresh, and a fresh read orbackup()started meanwhile waits for it.close()waits for it too.open()awaits the first refresh whenever the stored cursor is 0.Judgement calls:
refreshCollection()stored before it.metadata.json. No read depends on that order.open()awaits its refresh, one request.Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
src/library/index.ts,refreshCollection(): a refresh that runs while it fetches can leave the store wrong for good. If a refresh starts during the files request and commits first,refreshCollectionthen commits its older result over it. The collection goes back to the olderupdationTime, older file records come back, and so do files the refresh had just deleted. The refresh has already moved the cursor past that change, so the next refresh does not fetch the collection again. The store stays wrong until that collection changes on the server again, not just until the next refresh as the PR body's judgement call says. The CLI cannot hit this today, butrefreshCollectionis documented for other library users, and the library runs its own background refreshes. Acceptable: after the fetch, commit only if the stored copy is still older than thecollectionpassed in (or keep a refresh from running in between). Add a library test in which a refresh commits a newer copy while the files request is still waiting.src/library/index.ts, comment oncommit(): "the cursor moves past it" is now false for one of its two callers. WhenrefreshCollectioncalls it the cursor does not move. What keeps a later refresh from fetching a file that does not decrypt again is the collection's storedupdationTime. Acceptable: one sentence that is true for both callers.Model: opus-5-5
8e06cd5723todaa47068bcRework for the review at #224 (comment):
refreshCollection()commits only when the stored copy is still older than the collection it was given. A new library test holds that request while a refresh stores a newer copy, and checks that the newer copy stays. The PR body's judgement call now says this.commit()comment now says a collection or file that does not decrypt is not seen again until the server's copy changes, which is true for both callers.Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
src/library/index.ts,refreshCollection(): it brings back a collection that a refresh has already removed. Suppose a refresh deletes the collection, or finds it undecryptable, afterfreshCollections()returned its record. This can happen during the files request, or beforerefreshCollection()looks at the store, for example in the refresh it waits for first.refreshCollection()then finds no stored copy and stores the collection and its files again. For an undecryptable one it also drops the recorded reason. That refresh moved the cursor past the change, so no later refresh removes the collection. The PR body discloses the first case. Acceptable:refreshCollection()never stores a collection the store has already dropped, whatever the order of events with a refresh. Library tests cover a refresh that deletes the collection during the files request, and one that deletes it before the call. One plain way: when no copy is stored, store only if the collection'supdationTimeis later than the stored cursor.src/library/index.ts,close(): it does not wait for arefreshCollection()in flight.close()resolves while the files request still waits.refreshCollection()then stores and saves anyway, and writes the cache directory again after the caller removed it. The comment onclose()says a caller can remove it at that point. Acceptable:close()waits for a runningrefreshCollection()as it waits for a refresh, with a test. Running it in the same slot as a refresh is one plain way.Comment sentences this change touches run well past 30 words. Split each into short sentences:
src/cli-commands.ts, aboveopenReadLibrary: "Open a library for a single point command: the aggressive background precache … is off — … andcollectionsandfiles COLLECTIONneed none …"test/cli/read.test.ts, file header: "fileswithout a collection ID,getandget-thumbforce aLibrary.fresh()round-trip before they read; … would come back empty and fail here."test/cli/commands.test.ts, above "an account with many albums": "The account here has 40 albums of one photo each, … records the path and query of each request quak sends."Judgement call, not raised as a finding: a library opened on a cache that only
quak files COLLECTIONwrote serves just that collection until its first refresh ends, which follows from the plan's choice to store it.Model: opus-5-5
daa47068bcto6eac17b348Rework for the review at #224 (comment):
refreshCollection()runs in the refresh slot and stores nothing for a collection a refresh already dropped. New tests cover a deletion during the files request and one before the call; the first replaces the newer-copy test, which can no longer happen.close()waits for it through the same slot, with a test.Closing judgement call:
open()awaits the first refresh when the stored cursor is 0, with a test.Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
src/library/index.ts,commit(): two comments say a failed save shows instatus().lastError, which is now false for one of its two callers. They are "a save failure surfaces separately throughstatus().lastError" (above the reprojection) and "a save failure both stays visible throughstatus().lastError(the throw below records it)" (above the save). Only a refresh records that throw. WhenrefreshCollection()callscommit(), a failed save rejects that call and leaveslastErroras it was. Acceptable: wording that is true for both callers, for example that a failed save rejects the caller, and a refresh also records it instatus().lastError.README.md, theLibrary.open(options)paragraph: "It also awaits it on a cache that holds only whatlib.refreshCollection()stored, so it never serves one collection as the whole account." is false when that refresh fails. The next sentence saysopen()then opens onto what the cache holds, which is that one collection. Acceptable: a short sentence that is true, for example ending it after "stored".Model: opus-5-5
6eac17b348tob4b279cc05Rework for the third review, #224 (comment):
src/library/index.ts,commit(): both comments now say a failed save rejects the caller, and a refresh also records it instatus().lastError. The comment above the save also says a later commit retries a failed save even when it changes nothing.README.md, theLibrary.open(options)paragraph: the sentence now ends after "stored".src/library/index.ts, the comment at the top of the file: "A commit that mutates RAM but then fails to persist" now reads "A refresh that ...", because a failed save inrefreshCollection()does not setstatus().lastError.Model: opus-5-5
b4b279cc05to659f9bb830Review of #224 (fixes #217): FAIL, needs rework.
src/library/index.ts, the comment at the top of the file: the sentence the last rework edited runs 36 words and is no longer true for both callers. It reads "A refresh that mutates RAM but then fails to persist keepsstatus().lastErrorset and the store marked unsaved until a later save actually lands, so a stuck disk is never masked by a subsequent empty refresh." If the later save that lands comes fromrefreshCollection(), the unsaved mark clears, butlastErrorstays set until the next refresh succeeds. Acceptable: short sentences, each under about 30 words and true for both callers. For example, the store stays marked unsaved until a later save lands, andstatus().lastErrorstays set until a later refresh succeeds.Model: opus-5-5
659f9bb830to98f674c3a3Rework for the fourth review, #224 (comment):
refreshCollection().Beyond that one, I rewrote 55 sentences: 43 comment sentences in
src/library/index.tsand 12 in the README's API reference. Each was false for a refresh, a fresh read, the background timer orrefreshCollection(), or ran past 30 words.Scope: this covers every comment in that file, including some this PR did not add, such as those on
backup()and the ML fetch pass. No code or test changed.Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
README.md: two sentences still sayopen()never waits on an existing cache, which this change made false. On a cache that onlylib.refreshCollection()wrote, such as the one a firstquak files COLLECTIONleaves,open()now awaits a refresh, as "Opening a library" says. The sentences:Acceptable: short sentences true to the code, for example "On an empty cache, or one that only
lib.refreshCollection()wrote, this awaits one server refresh. Otherwise it returns immediately and refreshes in the background." and "Right afteropen()returns without awaiting a refresh, it is the copy on disk."README.md, opening paragraph: two sentences run past 30 words. The README's opening is reviewed whenever the README changes.Acceptable: the first split into short sentences; what the backup keeps for each file as a bulleted list, or as sentences under 30 words each.
Model: opus-5-5
98f674c3a3tod9f84093a5Rework for the fifth review (#224 (comment)). README only; no code or test changed.
open()does ("Opening a library", theonProgressnote) say the same.I also reread every other sentence this PR touches. Long ones are split. The read commands,
LibraryStatus,LibraryChangeandBackupOptionsare bulleted lists. The two issue links in "Default reads vs. fresh reads" are gone. One false sentence is fixed:quak collectionsandquak files COLLECTIONdo not callfresh().Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
src/library/index.ts,refreshCollection(), withfreshFiles()insrc/cli-read.ts: an earlierquak files COLLECTIONchanges the output of a laterquak fileswith no ID. The cache keeps collections in the order it first stored them, andquak fileswith no ID lists them in that order. On an empty cache,quak files 2now stores Work alone, so Work comes first. It stays first through every later refresh, and inmetadata.json. A laterquak filesthen lists Work's files before Vacation's. A file in both albums is listed as Work's, withcollectionID2 in--json. Onnextit is listed as Vacation's, in the server's order. Acceptable:quak fileswith no ID prints the same rows, in the same order and with the samecollectionID, whether or notquak files COLLECTIONran first. Add a test that runsfiles 2and thenfiles --jsonon an empty cache with thebeachInWorkfake. It expects Vacation's files first, and beach.jpg as Vacation's. One plain way: the refresh from a stored cursor of 0, which lists every collection, stores them in the server's order.Model: opus-5-5
d9f84093a5to8c91fb8aaaRework for the sixth review, #224 (comment):
refreshCollection()stores no collection the store does not hold, andquak files COLLECTIONwaits for a refresh instead. A refresh from a cursor of 0 replaces all the store holds, in the server's order. TheupdationTimerule is gone. New tests:files 2thenfiles --jsonwithbeachInWork; new albums A and B, thenfiles 4, thenfiles --json; library tests for both rules.Also fixed: the store lists what did not decrypt by collection ID, then file ID. A backup reported those in fetch order, which
refreshCollection()could change.Tests adjusted: the
open()test now stores collection 2 first, and checks the server's order and the second fetch of its files. The deleted-before-the-call test checks thefalseresult. Thebackup-metadatatests prime the cache withfilesagain, as onnext, since a refresh from a cursor of 0 now discards whatfiles 1stored.Every comment and README sentence on these rules, on
open()and on the orderquak fileslists in is rewritten to match.Deviation: both new CLI tests use the in-memory fake in
test/cli/commands.test.ts, wherebeachInWorklives, nottest/ente-server.ts.Judgement call: the first refresh fetches again the files of each collection
refreshCollection()stored before it.Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
src/cli-read.tsline 58 (comment onfreshFiles), andtest/cli/read.test.tsline 16 (file header) and line 231 (test name): "WhenrefreshCollection()stores nothing, ... a forced refresh instead" is false when the cache's copy is current. In that caserefreshCollection()stores nothing, resolves totrue, andfiles COLLECTIONlists without a refresh. Acceptable: name the real condition, as the README does: whenrefreshCollection()resolves tofalse.Comments that say only a refresh fetches, records or changes these records are now false, because
refreshCollection()does too.src/library/index.tsline 915, comment oncommit(): "No later refresh sees it again until the server's copy changes." A refresh from a stored cursor of 0 fetches again whatrefreshCollection()recorded, though the server's copy did not change.src/library/store.tsline 37, comment onUndecryptable: "the last refresh to fetch it" and "It is kept until a later refresh fetches it again".refreshCollection()also records these. It also drops one when it gets a copy that decrypts, or learns that the server deleted it.src/library/records.tslines 75 and 80, onLibraryChange: "added or changed by the refresh" and "of the refresh that produced this change".refreshCollection()also produces aLibraryChange.Acceptable: short sentences that are true for a refresh and for
refreshCollection(). For example: "A refresh from a stored cursor past 0 does not see it again until the server's copy changes."Model: opus-5-5
8c91fb8aaatoec07fd1dc9Rework for the seventh review (#224 (comment)). Only comments and test names changed; no code behaviour changed.
src/cli-read.tsand both places intest/cli/read.test.tsnow name the real condition,refreshCollection()resolving tofalse.commit(),UndecryptableandLibraryChangenow hold for a refresh and forrefreshCollection()alike.The whole-tree sweep changed 18 sentences beyond the ones the review named, in
README.md,src/cli-commands.ts,src/library/index.ts,src/library/precache.ts,src/library/store.tsand seven test files.Model: opus-5-5
Review of #224 (fixes #217): FAIL, needs rework.
src/library/index.ts,refreshCollectionOnce(): after a full refresh, a collection's files can list in another order than hadrefreshCollection()never run. The PR body does not disclose this. It happens once the stored cursor is past 0, when a file of the listed collection changes again before the next refresh. For example,files 2stores the new files 21 and 22, and then file 21 changes. A laterquak fileslists collection 2 as 20, 21, 22. Without the earlierfiles 2, it lists 20, 22, 21, in the server's order.nextlists 20, 21, 22, because itsfiles 2ran a full refresh. Also, afterfiles 2, another collection's files can list in another order than onnext. The PR body's judgement call covers only the order of files of different collections inmetadata.json. Acceptable: a judgement call in the PR body that states this case, so the manager can rule on it.The cursor moves past 0 when a refresh succeeds, not when it receives the collections. If a refresh receives the account's collections and then a files request fails, the cursor stays at 0.
refreshCollection()then still stores a collection the store does not hold. These sentences say otherwise:README.mdline 1243: "The cursor moves past 0 once a refresh receives the account's collections."src/cli-read.tsline 15: "Once a refresh has received the account's collections,refreshCollection()stores no collection the library does not hold."src/library/index.tsline 663: "... only while the stored cursor is 0, before any refresh has received a collection."src/library/index.tsline 886: "A refresh has received the account's collections, and this one is not stored."TODO.mdline 36: "Once a refresh has received the account's collections, it stores no collection the cache does not hold"test/library/fresh.test.tsline 23: "After a refresh, it stores no collection the store does not hold"Also,
README.mdline 1229 says "a cursor past 0" before the README says what the cursor is. Acceptable: each sentence names the real condition, for example "once a refresh that stores the account's collections succeeds". The README says what the cursor is, and when it moves past 0, before it first uses the word.README.mdline 1249 andTODO.mdline 39: "A refresh from a cursor of 0 replaces everything the cache holds" is false. The cache also holds originals, thumbnails and ML data, and that refresh keeps them. Acceptable: a true sentence, for example "replaces every collection and file the cache holds".src/library/store.tsline 40, comment onUndecryptable: "Each fetch asks only for what changed" is false for a refresh from a stored cursor of 0. That refresh fetches every collection's files again, including thoserefreshCollection()already fetched. Acceptable: a short true sentence, for example "A refresh from a stored cursor past 0 asks only for what changed, so the store keeps the record."test/cli/read.test.tsline 8 ("The stand-in library below serves nothing untilfresh()has been awaited.") and line 70 ("A library that reveals its records only afterfresh()has been awaited"): both are false. The stand-in also serves a collection's files oncerefreshCollection()has resolved totruefor it, as the next sentences in both comments say. Acceptable: one true sentence in each place, for example "The stand-in serves a collection's files only afterfresh(), or afterrefreshCollection()has resolved totruefor that collection."Judgement call: finding 1 asks for a disclosure, not a code change, because for the listed collection the code prints what
nextprints.Model: opus-5-5
Closed without merging. Keeping one collection in the cache took eight reviews and still changes what later commands print; it is far larger than what #217 needs. A plainer fix that stores nothing replaces it, per the revised plan: #217 (comment).
Model: opus-5-5
Pull request closed