README.md is written again from scratch for #219: what quak is and how to use it first, then reference, library, design and development.
Where the old README and the code on next disagreed, the code decided. Each item gives the old claim, then the code:
main and types point at dist/bin/quak.js; they point at dist/src/index.js and dist/src/index.d.ts.
With a self-hosted apiOrigin, thumbnails still come from Ente's CDN; they go through apiOrigin too.
fix-missing-thumbnails --json with nothing to fix prints JSON; it prints none.
Nothing on --file with an unknown ID; it is reported failed.
MLDataAPI, SimilarResult, MissingThumbnailInfo and ThumbnailFixResult are exported; none is.
Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only an error: … message.
stdout holds only output; login prompts there too.
A shallow clone stamps a tag only on its own commit; a deeper one can name an earlier tag.
Nothing else in collections/ is touched; stray symlinks are removed.
Nothing is hashed without --verify; every download with a recorded hash is.
failures.json counts every try; attempts counts failed runs in a row.
A corrupt session file names the bad field; non-JSON gets the parser's error.
Every dependency is pinned by hash; bun, make, git, bash and curl are not. Node and yarn are pinned by version, and only when script/bootstrap installs them.
Three code comments now match the README.
Disclosures:
The TODO section stays, short, because REPO_POLICIES.md requires it.
"For LLMs" is folded into "Rules for contributors"; the Rationale drops the critique of Ente's clients.
The README no longer says Ctrl-C removes backup.lock: proper-lockfile's exit hook does that, and whether the bun binary runs it cannot be checked from this repo.
About 15,300 words, longer than before.
Model: fable-5-1 (README, code comments); opus-5-5 (PR body edits)
`README.md` is written again from scratch for https://git.eeqj.de/sneak/quak/issues/219: what quak is and how to use it first, then reference, library, design and development.
Where the old README and the code on `next` disagreed, the code decided. Each item gives the old claim, then the code:
- `main` and `types` point at `dist/bin/quak.js`; they point at `dist/src/index.js` and `dist/src/index.d.ts`.
- With a self-hosted `apiOrigin`, thumbnails still come from Ente's CDN; they go through `apiOrigin` too.
- `fix-missing-thumbnails --json` with nothing to fix prints JSON; it prints none.
- Nothing on `--file` with an unknown ID; it is reported `failed`.
- `MLDataAPI`, `SimilarResult`, `MissingThumbnailInfo` and `ThumbnailFixResult` are exported; none is.
- Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only an `error: …` message.
- stdout holds only output; `login` prompts there too.
- A shallow clone stamps a tag only on its own commit; a deeper one can name an earlier tag.
- Nothing else in `collections/` is touched; stray symlinks are removed.
- Nothing is hashed without `--verify`; every download with a recorded hash is.
- `failures.json` counts every try; `attempts` counts failed runs in a row.
- A corrupt session file names the bad field; non-JSON gets the parser's error.
- Every dependency is pinned by hash; bun, `make`, `git`, `bash` and `curl` are not. Node and yarn are pinned by version, and only when `script/bootstrap` installs them.
Three code comments now match the README.
Disclosures:
- The TODO section stays, short, because `REPO_POLICIES.md` requires it.
- "For LLMs" is folded into "Rules for contributors"; the Rationale drops the critique of Ente's clients.
- The README no longer says Ctrl-C removes `backup.lock`: proper-lockfile's exit hook does that, and whether the bun binary runs it cannot be checked from this repo.
- About 15,300 words, longer than before.
Model: fable-5-1 (README, code comments); opus-5-5 (PR body edits)
Review of #228 (#219): FAIL, needs rework. Line numbers are README.md on the PR head.
Rendering
L210-212 and L271-273: the wrap puts "1." at the start of a line, so the text renders as a numbered list. "and exits" loses its exit status, and "login is the only command that prompts." and "--collection … is accepted and ignored." become list item 1. Reword so no line starts with "1.", e.g. "On failure it logs … and exits with status 1."
References into the README (fix in this PR)
src/cli-log.ts:7, "see "CLI surface" in the README": that section is gone. The log levels are now under "What quak prints", not "Command reference" as the PR body says. Point the comment at "What quak prints".
test/cli/backup-end-to-end.test.ts:421, 'The tree "Backup layout" in the README describes': the section is now "The backup directory".
PR body
"the repo has only the Dockerfile's 90-second timeout" is false. REPO_POLICIES.md makes 60 seconds the hard cap for make test, with 20 seconds as the target. L1700, "a backstop for a hung test, not a budget", now refers to a budget the README never names. Acceptable: "make test must finish in under 60 seconds. The 90-second timeout only catches a hung test." Remove that item from the PR body's list of disagreements.
Opening
L23, "The library underneath does the same work for a program of your own." False: src/index.ts exports neither the thumbnail helpers nor the metadata dump. Acceptable: "The library underneath lets a program of your own read and back up the account."
L24, "It opens a cache-backed view of the account and answers reads from the cache, without touching the network." "Cache-backed view" and "answers reads" are jargon. Acceptable: "It keeps the account's album and file lists on disk and answers from that copy without asking the server."
L29, "Every download is decrypted on its way to disk and checked against the content hash Ente records for the file." False for thumbnails, and for originals with no recorded hash. "Content hash" is also undefined here. Acceptable: "Every download is decrypted on its way to disk." The hash rule already sits under "The cache".
L11 and L68, "the account's listing": vague. Acceptable: "the account's album and file lists".
L19 and L317, "everything Ente records about the account, its albums and its files": false. account.json holds only email and userID, and the file JSON leaves out sizes and keys. Acceptable: "the metadata quak keeps about …".
Using the library
Missing: how a program gets the quak package. package.json is private and dist/ is not committed. Also, Library.open({ client }) with the defaults starts downloading every thumbnail and the latest week's originals. Say both next to the example.
Running the backup from cron
L114, "It bundles the built dist/ with bun, a JavaScript runtime that can bundle a program into one executable, and it runs bun through nix-shell -p bun." Three ideas. Acceptable: "It uses bun, a JavaScript runtime, to bundle the built dist/ into one executable. It runs bun through nix-shell -p bun."
L141, "which cron sets from the password file": a reader can take this for the Ente password. Acceptable: "which cron sets from /etc/passwd."
Command reference
L175-176: -V and -h, the short forms of --version and --help, are missing.
L194, "Every command but login needs a saved session." False: logout with no session exits 0 (L223). Acceptable: "Every command but login and logout needs a saved session."
L202, "The password is not shown as it is typed." It shows one * per character. Acceptable: "The password shows as * characters while it is typed."
L206, "An account that signs in with a passkey cannot log in with quak." Inaccurate: an account with both a passkey and TOTP logs in through TOTP. Acceptable: "An account whose only second factor is a passkey cannot log in with quak."
L229-231, "When it knows the cache directory, from --cache-dir or from a session file it could read, it warns that …". The verb comes late. Acceptable: "It warns that the cache directory still holds decrypted data. It knows the directory from --cache-dir or from a session file it can read."
L299-301, "Its fields are totalFiles, … and failedAlbums.": ten names in one comma list, and none is explained (errors against failed, for example). Acceptable: a bulleted list, one line per field saying what it counts.
L326, "So is a file whose data does not decrypt, unzip or parse." The sentence must be read twice, and "unzip" is wrong because the data is gzip; the same word is at L625 and L930. Acceptable: "A file whose data does not decrypt, decompress or parse also gets mlDataError."
L335: the real line starts with the collection: "[<collection>] Could not check <title>: <reason> (not reported as missing)". Quote it in full.
L353, "For each file, the helper downloads the original, makes a JPEG thumbnail from it, encrypts the thumbnail under the file's own key and uploads it." Four steps chained with commas. Acceptable: a numbered list.
L366, "a video or a live photo;": every file that is not an image is skipped, unknown included. Acceptable: "a file that is not an image;".
L377-379, "When nothing needs fixing, the command logs No missing thumbnails found. and exits 0, and with --json it then writes no JSON." Three ideas. Acceptable: two sentences, the second "With --json it then writes nothing on stdout."
L381, "One bad --file ID refuses the whole command": an ID cannot refuse anything. Acceptable: "quak refuses the whole command, before any request, when one --file ID is invalid."
How a command gets current data
L387, "Each one asks the server for exactly what it needs:" False: several commands run a full refresh. Acceptable: "Each one gets current data this way:".
L403, "On an empty cache that is every collection in the account, which on a large account takes a while, and nothing is stored until all of them are in." Three ideas. Acceptable: "On an empty cache that is every collection. On a large account this takes a while. Nothing is stored until every collection is in."
L411-413, "A collection or file that quak cannot decrypt does not fail a command". False for backup, which counts it as failed and exits 1. Acceptable: "A collection or file that quak cannot decrypt does not stop a refresh. backup counts it as failed (see "What quak cannot decrypt")."
What quak prints, exit codes, error messages
L425-426, "stdout holds only a command's output … Every other line goes to stderr". login's prompts go to stdout. Under yarn quak, yarn also prints its own lines on stdout, so yarn quak files --json cannot be piped. Acceptable: say where the prompts go, and give yarn --silent quak or node dist/bin/quak.js for piping.
L438-439, "After logout, that the cache directory still holds decrypted data." This is not a full sentence. Acceptable: "The only warning is the one logout prints: the cache directory still holds decrypted data."
L466-467 and L477, "an error that stopped it is printed as one line, quak: <message>". False: errors a command reports itself have no quak: prefix, such as Invalid file ID, File … not found, Login failed and Could not end the session on the server. Acceptable: "An unexpected error is printed as one line, quak: <message>, with no stack trace."
L467-468, "The next backup run tries again." It follows the sentence about any command, but belongs after the backup sentence. Move it there.
L481, "Under Node, as yarn quak runs quak, fetch reports every failure below HTTP as just fetch failed …". The subject arrives late, and "below HTTP" is jargon. Acceptable: "yarn quak runs quak under Node. Node's fetch reports every network failure as fetch failed, with the reason as its cause."
The session file
L504-505, modes 0600 and 0700: they apply only when quak creates the file or directory. Acceptable: "quak creates the file with mode 0600, in a directory it creates with mode 0700."
L509-510, "… is corrupt, and the command says which field is wrong." False when the file is not JSON. The way to recover is also missing. Acceptable: "A file that cannot be read back is reported as corrupt, with the reason. Run quak logout, then quak login."
L514-516, "quak backup meets a rejected session on the refresh it starts with, before it writes anything to the backup directory but its lock." Clauses are stacked. Acceptable: "quak backup learns this at its first refresh. By then it has written only its lock."
L516-517, "… fails each remaining file into failures.json instead." "Fails into" is a coined verb. Acceptable: "When the session stops working partway through a backup, each remaining file goes into failures.json."
The cache
L522-525: the platform paths are chained in one sentence. "Each account has its own cache directory" is also false when two accounts share --cache-dir. Acceptable: "Without --cache-dir, each account has its own cache directory:" followed by one bullet per platform.
L557-558, "A live photo ZIP that holds a second image or video, or whose two files come to more than 20 times its size plus 16 MiB, is refused." The verb comes last. Acceptable: "quak refuses a live photo ZIP that holds a second image or video. It also refuses one whose files unpack to more than 20 times its size plus 16 MiB."
L552-558: the README says only that such a file "is not stored". It should say the download fails with an error naming the file, so get exits 1. It should also say that a live photo's image and video leave the cache together.
The backup directory
L607 and L688: field names chained with commas; L688 is 30 words with ten names. Acceptable: bulleted lists.
L609, "metadata is the file's own metadata as uploaded". False: quak keeps seven fields, turns fileType into a name, and may build hash from two hashes. Acceptable: "metadata holds the fields quak keeps: title, fileType, creationTime, modificationTime, latitude, longitude and hash."
L623-625, "A file whose data could not be fetched, or whose data arrived but did not decrypt, unzip or parse, gets the reason in mlDataError …". The verb comes after 18 words. Acceptable: "When a file's data cannot be fetched, decrypted, decompressed or parsed, the JSON holds the reason in mlDataError instead of mlData."
L675 and L1108: "cosine similarity" is not defined. Define it once, e.g. "a score from -1 to 1, higher the more alike two embeddings are".
L698-699, "A collection whose directory name would be another collection's JSON name gets its ID added as well." Hard to parse. Acceptable: "A collection named like another collection's JSON file, such as Trip.json beside Trip, also gets its ID added."
L704-705, "Each run removes from a collection's directory the symlinks into the date folders that no longer belong there." The object comes late. Acceptable: "Each run removes the symlinks that no longer belong in a collection's directory."
L710-711, "A collection whose directory or JSON cannot be written, such as one whose directory name is taken by a file you put there, does not stop the run." The verb comes late. Acceptable: two sentences, with the example second.
L722-723, "When a copy of the file in another collection decrypted, it does not." The reader cannot tell what "it does not" refers to. Acceptable: "It stays out of failures.json when its copy in another collection decrypts."
L729-730, "… until the server sends a copy that decrypts or deletes the record. The next run then drops it." This reads as "a copy that deletes", and "it" is unclear. Acceptable: "Every run exits 1 until the server either sends a copy that decrypts or deletes the record. The run after that drops the entry."
L746-750: the entry contents and the three kinds are each chained in one sentence, and the JSON keys are never named. Acceptable: a bulleted list of the keys (fileID, title, classification, attempts, lastTriedAt, error, under files, beside version) and a bulleted list of the kinds.
L752, "or in the backup's scope": "scope" is undefined for a command-line user. Name what it means (onlyAlbumNames in the library), or drop it here.
L776-778, "It leaves failures.json and backup.lock as they are, since another backup may be using them, prints … and exits 1." Three actions and a reason. Acceptable: split it into short sentences.
L790-791, "When the download fails, or does not match either, the temporary files are deleted, the original keeps its bytes and its album links, and the file goes into failures.json." 29 words with three results, and "either" reads as "also". Acceptable: "When the new copy fails or does not match:" followed by a bulleted list.
L802-806: "all over originals that were already stored" reads as the idiom "all over". Acceptable: "Each counts only originals that were already stored." "Only a later --verify run checks a failed original again. A run without --verify … takes it out of failures.json" does not say which original, or why a plain run drops it. Say both plainly.
L818-820, "A copy that --verify downloads is written to such a … too, and a live photo's image and video to …". The second half leaves its verb out. Acceptable: a bulleted list of the temporary-file name patterns.
L823-824, "opening a library deletes such leftovers in the cache the same way." False: only originals/ and thumbnails/ are cleaned. Acceptable: "Opening a library deletes such leftovers in the cache's originals/ and thumbnails/."
The metadata dump directory
L833-835, "writes account.json, holding …, and under collections/ one directory per collection, named …". The objects are stacked. Acceptable: a tree like the backup's.
L835-836, "with the fields a backup's collection JSON holds": false, because it has no files. Acceptable: "… except files."
L836-837 and L319-321: the per-file JSON does not hold "the fields a backup's JSON beside an original holds".
imageMetadata is written only with --exif.
Videos are read too.
A file with no image metadata gets no field, not {}.
State these differences.
Missing: backup-metadata skips a collection it cannot decrypt without a word, and exits 0.
File names from the server
L853-855, "A name over 255 bytes, which most file systems refuse, is cut short between two characters before its extension …". "Between two characters" is obscure. The rule also holds for file names only; collection directory names are cut at the end. Acceptable: "A file name over 255 bytes is shortened before its extension, without splitting a character, so the extension is kept when it fits."
Library reference
L861, "The library's main surface is the cache-backed Library." "Surface" is jargon. Acceptable: "The library's main class is Library."
L863-866: about 35 words, three test files chained with commas, and the jargon "walk". Acceptable: a bulleted list of which test shows what.
L895-903: "the latest window" is coined and used before precacheOriginalsDays defines it. "Pinned" is used before it is defined. "The effective limit shrinks as the disk fills" names an undefined limit, and L927 does the same. Define each where it is first used.
L911, "The two background fills yield to on-demand work, and a file already cached costs no fetch." This is jargon. Acceptable: "The two background downloads wait while your own requests run. A file already in the cache is not fetched again."
L916-921: the operation field ("refresh", "fetchMLData", "precacheThumbnails", "precacheOriginals") is never named, yet L931 mentions "The fetchMLData event". "Precache sweep" is coined. Name the field and its values.
L919-921, "… with fetched, how many it has fetched so far, and total, how many it will." The appositives are stacked. Acceptable: "fetched is the number of collections fetched so far, and total the number it will fetch."
L925-927, "the time and error of the last refresh and of the last machine-learning fetch". Inaccurate: the time is that of the last success, and the error is set only while the latest attempt failed. Say that.
L928-930, "lib.status().mlDataErrors lists, as { fileID, error }, each file whose machine-learning data the last successful fetch got but could not decrypt, unzip or parse." This stacks clauses the way the issue's own example does. Acceptable: "lib.status().mlDataErrors lists each file whose data arrived in the last successful fetch but could not be decrypted, decompressed or parsed. Each entry is { fileID, error }."
L934-938: after close(), fresh(), freshCollections() and freshFiles() reject with the library is closed. Say so.
L953, "A refresh that fails rejects the caller." The promise rejects, not the caller. Acceptable: "When the refresh fails, fresh() rejects."
L959-961, "A refresh fetches a collection's files only when …, so a collection stored without its files would never get them." It does not say that this is why the list is not stored. Acceptable: "It stores nothing because a refresh fetches a collection's files only when the collection is new or changed. A collection stored without its files would never get them."
L968-971, "A collection or file in the server's answer that quak cannot decrypt, or whose metadata is not JSON, does not fail the refresh. The refresh leaves it out, a collection with all its files, and drops …". The verb comes late and the appositive is unclear. Acceptable: "The refresh does not fail on a record it cannot decrypt, or whose metadata is not JSON. It leaves the record out; a collection left out takes its files with it."
L1026, { path, bytes, videoPath? }: bytes is the size, a number, while L1036 says content() returns "the original's bytes". Acceptable: "bytes is the file's size."
L1049-1052: twelve method names chained with commas. Acceptable: a bulleted list.
L1053-1055, "dateTimeOriginal() is the camera's clock reading, held in the Date's UTC fields; offsetTimeOriginal(), when present, is that clock's offset from UTC." It must be read twice. Acceptable: two plain sentences.
L1041: the example value names a camera company. Use a neutral value.
L1075, "across a window boundary": this is jargon. Acceptable: "for example between processes".
L1077-1081: the PhotoRecord fields are fifteen names in comma lists. Acceptable: a bulleted list.
L1084: in LibrarySnapshot, takenAt is when the snapshot was taken. One line earlier, the same name means when a photo was taken. Say so.
L1098, "Only "visible" goes ahead of background work." The reader is left guessing what "ahead" does. It is treated as "background"; say so.
L1104: MLData is not exported either; say so with the other types that are not exported.
L1141-1143, "metadata, with 10 slots, content, with 5, and thumbnails, with 25": the commas do two jobs. Acceptable: a bulleted list. L1145-1147, "runs a task submitted under a key once while a task with that key is still pending". Acceptable: "A task submitted under a key that is still pending is not run a second time."
L1149-1150, "lib.backup(), and so quak backup, works on up to the content pool's limit of originals at once, 5 unless …". Clauses are stacked. Acceptable: "lib.backup() works on as many originals at once as the content pool allows, 5 by default. quak backup uses the default."
L1171-1172, "… runs under bun, whose fetch rejects with a plain Error that has no cause and carries a code of bun's own." Relative clauses are stacked. Acceptable: three short sentences.
L1221-1223, "Every POST and PUT … goes through this rule; verifying a second factor, for one, uses up one of a small number of attempts." Two ideas in 30 words. Acceptable: two sentences.
L1236-1237, "So do bun's ConnectionRefused and FailedToOpenSocket, on which a POST or PUT is not replayed either." "So do" has no clear referent. Acceptable: "Bun's ConnectionRefused and FailedToOpenSocket are treated the same way."
L1256-1260, "For a body of more than one chunk the distinction is real." This is unclear. "For a single-chunk body, which is most thumbnails and every small file, a wrong key, server-side corruption and a cut mid-chunk all look alike …" The subject comes late. Acceptable: "quak can tell the two apart only in a body of more than one chunk. Most thumbnails and every small file are one chunk. There a wrong key, corrupt bytes and a cut connection look alike, and all are retried."
L1287-1291: "Each returns the decrypted live records, the IDs of deleted records, the records that did not decrypt with the reason, and the cursor to pass next time." Four things chained, with no field names. Acceptable: a bulleted list with the field names. "are the same from the beginning" is unclear; they return only the live records from sinceTime 0, so say that.
L1293-1295: without outPath, downloadThumbnail uses the get-thumb name (thumb_ plus the title), not the get name. Fix it.
L1312-1316: seventeen exports chained with commas. Acceptable: a bulleted list. VERSION is not a piece the Client is built from, and it is never explained.
L1338-1342, "they are reached through lib.mldata and the command line". A type cannot be reached through a command line. MissingThumbnailInfo and ThumbnailFixResult are internal to the command line. Acceptable: drop the src/thumbnails.ts line, and say that MLDataAPI and SimilarResult are the types of lib.mldata and are not exported by name.
Rationale and Design
L1379, "Ente's own clients are built for a person at a screen." False: Ente ships a command-line client with an export command. Acceptable: "Ente's own clients are apps and a command-line tool; none is a library a program can use."
L1401-1402, "decrypted Collection, File and Metadata types": the types are Collection, EnteFile and FileMetadata, in backticks.
L1448, "make build compiles that tree into dist/": that tree includes test/, which is not compiled. Acceptable: "make build compiles src/, bin/ and examples/ into dist/."
L1394-1446: the layout leaves out script/, yet lists Makefile, Dockerfile and package.json.
L1400 and L1458: "SRP" is used before L1471 defines it. Define it at first use.
L1465-1467, step 2: the verb "produces" comes after 17 words. Acceptable: "Argon2id (crypto_pwhash) derives a 32-byte key encryption key, the KEK, from the password. It uses the server-issued kekSalt, memLimit and opsLimit."
L1474-1477, "after which the server returns them" and "the server returns them after it": the pronouns are far from what they stand for. Name "the key attributes and the token".
Entrypoints and development
L1575, "It writes." This is unclear. Acceptable: "It rewrites files in place."
L1587-1588, "with bun run through nix-shell -p bun" reads as the command bun run. Acceptable: "make build-bin bundles the built dist/ into one binary, bin/quak. It runs bun through nix-shell -p bun."
Missing: the README never names make test, make docker, make dev or make clean. The old text named make docker and make test. List the Makefile targets.
L1595-1596, "The test phase does the same with the suite …": vague, and three ideas. Acceptable: "The test phase runs the test suite the same way, as the non-root node user. A 90-second timeout stops a hung test."
L1599-1601, "Neither script runs the tools on the host, so Docker is required, and the scripts also work where the Docker daemon is remote and bind mounts are impossible." Three ideas, and "bind mounts" is undefined. Acceptable: split it, and define or drop "bind mounts".
L1620-1621, "… and that is what the tests, which run from source, report." The verb comes last. Acceptable: "The tests run from source, so they report 0.0.0."
L1641-1642, "script/docker and script/cibuild pass the version they resolve on the host, with --dirty, as the build argument, which takes precedence." Clauses are stacked. Acceptable: "They work out the version on the host, with --dirty, and pass it as the VERSION build argument. That argument takes precedence."
L1659, "the repository manager squash-merges it": this role is never defined. Acceptable: "a maintainer squash-merges it".
L1687, "Every dependency is pinned in yarn.lock." False: images are pinned by digest, and downloads in script/bootstrap and bin/clip-keyword-vectors by hash. Acceptable: "Every external dependency is pinned by hash: npm packages in yarn.lock, images by digest."
Unverified: that Ctrl-C removes backup.lock under the bun-built binary (L767).
Model: opus-5-5
Review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** Line numbers are `README.md` on the PR head.
### Rendering
1. L210-212 and L271-273: the wrap puts "1." at the start of a line, so the text renders as a numbered list. "and exits" loses its exit status, and "`login` is the only command that prompts." and "`--collection` … is accepted and ignored." become list item 1. Reword so no line starts with "1.", e.g. "On failure it logs … and exits with status 1."
### References into the README (fix in this PR)
2. `src/cli-log.ts:7`, "see "CLI surface" in the README": that section is gone. The log levels are now under "What quak prints", not "Command reference" as the PR body says. Point the comment at "What quak prints".
3. `test/cli/backup-end-to-end.test.ts:421`, 'The tree "Backup layout" in the README describes': the section is now "The backup directory".
### PR body
4. "the repo has only the Dockerfile's 90-second timeout" is false. `REPO_POLICIES.md` makes 60 seconds the hard cap for `make test`, with 20 seconds as the target. L1700, "a backstop for a hung test, not a budget", now refers to a budget the README never names. Acceptable: "`make test` must finish in under 60 seconds. The 90-second `timeout` only catches a hung test." Remove that item from the PR body's list of disagreements.
### Opening
5. L23, "The library underneath does the same work for a program of your own." False: `src/index.ts` exports neither the thumbnail helpers nor the metadata dump. Acceptable: "The library underneath lets a program of your own read and back up the account."
6. L24, "It opens a cache-backed view of the account and answers reads from the cache, without touching the network." "Cache-backed view" and "answers reads" are jargon. Acceptable: "It keeps the account's album and file lists on disk and answers from that copy without asking the server."
7. L29, "Every download is decrypted on its way to disk and checked against the content hash Ente records for the file." False for thumbnails, and for originals with no recorded hash. "Content hash" is also undefined here. Acceptable: "Every download is decrypted on its way to disk." The hash rule already sits under "The cache".
8. L11 and L68, "the account's listing": vague. Acceptable: "the account's album and file lists".
9. L19 and L317, "everything Ente records about the account, its albums and its files": false. `account.json` holds only `email` and `userID`, and the file JSON leaves out sizes and keys. Acceptable: "the metadata quak keeps about …".
### Using the library
10. Missing: how a program gets the `quak` package. `package.json` is private and `dist/` is not committed. Also, `Library.open({ client })` with the defaults starts downloading every thumbnail and the latest week's originals. Say both next to the example.
### Running the backup from cron
11. L114, "It bundles the built `dist/` with bun, a JavaScript runtime that can bundle a program into one executable, and it runs bun through `nix-shell -p bun`." Three ideas. Acceptable: "It uses bun, a JavaScript runtime, to bundle the built `dist/` into one executable. It runs bun through `nix-shell -p bun`."
12. L141, "which cron sets from the password file": a reader can take this for the Ente password. Acceptable: "which cron sets from `/etc/passwd`."
### Command reference
13. L175-176: `-V` and `-h`, the short forms of `--version` and `--help`, are missing.
14. L194, "Every command but `login` needs a saved session." False: `logout` with no session exits 0 (L223). Acceptable: "Every command but `login` and `logout` needs a saved session."
15. L202, "The password is not shown as it is typed." It shows one `*` per character. Acceptable: "The password shows as `*` characters while it is typed."
16. L206, "An account that signs in with a passkey cannot log in with quak." Inaccurate: an account with both a passkey and TOTP logs in through TOTP. Acceptable: "An account whose only second factor is a passkey cannot log in with quak."
17. L229-231, "When it knows the cache directory, from `--cache-dir` or from a session file it could read, it warns that …". The verb comes late. Acceptable: "It warns that the cache directory still holds decrypted data. It knows the directory from `--cache-dir` or from a session file it can read."
18. L299-301, "Its fields are `totalFiles`, … and `failedAlbums`.": ten names in one comma list, and none is explained (`errors` against `failed`, for example). Acceptable: a bulleted list, one line per field saying what it counts.
19. L326, "So is a file whose data does not decrypt, unzip or parse." The sentence must be read twice, and "unzip" is wrong because the data is gzip; the same word is at L625 and L930. Acceptable: "A file whose data does not decrypt, decompress or parse also gets `mlDataError`."
20. L335: the real line starts with the collection: "[<collection>] Could not check <title>: <reason> (not reported as missing)". Quote it in full.
21. L353, "For each file, the helper downloads the original, makes a JPEG thumbnail from it, encrypts the thumbnail under the file's own key and uploads it." Four steps chained with commas. Acceptable: a numbered list.
22. L366, "a video or a live photo;": every file that is not an image is skipped, `unknown` included. Acceptable: "a file that is not an image;".
23. L377-379, "When nothing needs fixing, the command logs `No missing thumbnails found.` and exits 0, and with `--json` it then writes no JSON." Three ideas. Acceptable: two sentences, the second "With `--json` it then writes nothing on stdout."
24. L381, "One bad `--file` ID refuses the whole command": an ID cannot refuse anything. Acceptable: "quak refuses the whole command, before any request, when one `--file` ID is invalid."
### How a command gets current data
25. L387, "Each one asks the server for exactly what it needs:" False: several commands run a full refresh. Acceptable: "Each one gets current data this way:".
26. L403, "On an empty cache that is every collection in the account, which on a large account takes a while, and nothing is stored until all of them are in." Three ideas. Acceptable: "On an empty cache that is every collection. On a large account this takes a while. Nothing is stored until every collection is in."
27. L411-413, "A collection or file that quak cannot decrypt does not fail a command". False for `backup`, which counts it as failed and exits 1. Acceptable: "A collection or file that quak cannot decrypt does not stop a refresh. `backup` counts it as failed (see "What quak cannot decrypt")."
### What quak prints, exit codes, error messages
28. L425-426, "stdout holds only a command's output … Every other line goes to stderr". `login`'s prompts go to stdout. Under `yarn quak`, yarn also prints its own lines on stdout, so `yarn quak files --json` cannot be piped. Acceptable: say where the prompts go, and give `yarn --silent quak` or `node dist/bin/quak.js` for piping.
29. L438-439, "After `logout`, that the cache directory still holds decrypted data." This is not a full sentence. Acceptable: "The only warning is the one `logout` prints: the cache directory still holds decrypted data."
30. L466-467 and L477, "an error that stopped it is printed as one line, quak: <message>". False: errors a command reports itself have no `quak: ` prefix, such as `Invalid file ID`, `File … not found`, `Login failed` and `Could not end the session on the server`. Acceptable: "An unexpected error is printed as one line, quak: <message>, with no stack trace."
31. L467-468, "The next backup run tries again." It follows the sentence about any command, but belongs after the `backup` sentence. Move it there.
32. L481, "Under Node, as `yarn quak` runs quak, `fetch` reports every failure below HTTP as just `fetch failed` …". The subject arrives late, and "below HTTP" is jargon. Acceptable: "`yarn quak` runs quak under Node. Node's `fetch` reports every network failure as `fetch failed`, with the reason as its cause."
### The session file
33. L504-505, modes `0600` and `0700`: they apply only when quak creates the file or directory. Acceptable: "quak creates the file with mode `0600`, in a directory it creates with mode `0700`."
34. L509-510, "… is corrupt, and the command says which field is wrong." False when the file is not JSON. The way to recover is also missing. Acceptable: "A file that cannot be read back is reported as corrupt, with the reason. Run `quak logout`, then `quak login`."
35. L514-516, "`quak backup` meets a rejected session on the refresh it starts with, before it writes anything to the backup directory but its lock." Clauses are stacked. Acceptable: "`quak backup` learns this at its first refresh. By then it has written only its lock."
36. L516-517, "… fails each remaining file into `failures.json` instead." "Fails into" is a coined verb. Acceptable: "When the session stops working partway through a backup, each remaining file goes into `failures.json`."
### The cache
37. L522-525: the platform paths are chained in one sentence. "Each account has its own cache directory" is also false when two accounts share `--cache-dir`. Acceptable: "Without `--cache-dir`, each account has its own cache directory:" followed by one bullet per platform.
38. L557-558, "A live photo ZIP that holds a second image or video, or whose two files come to more than 20 times its size plus 16 MiB, is refused." The verb comes last. Acceptable: "quak refuses a live photo ZIP that holds a second image or video. It also refuses one whose files unpack to more than 20 times its size plus 16 MiB."
39. L552-558: the README says only that such a file "is not stored". It should say the download fails with an error naming the file, so `get` exits 1. It should also say that a live photo's image and video leave the cache together.
### The backup directory
40. L607 and L688: field names chained with commas; L688 is 30 words with ten names. Acceptable: bulleted lists.
41. L609, "`metadata` is the file's own metadata as uploaded". False: quak keeps seven fields, turns `fileType` into a name, and may build `hash` from two hashes. Acceptable: "`metadata` holds the fields quak keeps: `title`, `fileType`, `creationTime`, `modificationTime`, `latitude`, `longitude` and `hash`."
42. L623-625, "A file whose data could not be fetched, or whose data arrived but did not decrypt, unzip or parse, gets the reason in `mlDataError` …". The verb comes after 18 words. Acceptable: "When a file's data cannot be fetched, decrypted, decompressed or parsed, the JSON holds the reason in `mlDataError` instead of `mlData`."
43. L675 and L1108: "cosine similarity" is not defined. Define it once, e.g. "a score from -1 to 1, higher the more alike two embeddings are".
44. L698-699, "A collection whose directory name would be another collection's JSON name gets its ID added as well." Hard to parse. Acceptable: "A collection named like another collection's JSON file, such as `Trip.json` beside `Trip`, also gets its ID added."
45. L704-705, "Each run removes from a collection's directory the symlinks into the date folders that no longer belong there." The object comes late. Acceptable: "Each run removes the symlinks that no longer belong in a collection's directory."
46. L710-711, "A collection whose directory or JSON cannot be written, such as one whose directory name is taken by a file you put there, does not stop the run." The verb comes late. Acceptable: two sentences, with the example second.
47. L722-723, "When a copy of the file in another collection decrypted, it does not." The reader cannot tell what "it does not" refers to. Acceptable: "It stays out of `failures.json` when its copy in another collection decrypts."
48. L729-730, "… until the server sends a copy that decrypts or deletes the record. The next run then drops it." This reads as "a copy that deletes", and "it" is unclear. Acceptable: "Every run exits 1 until the server either sends a copy that decrypts or deletes the record. The run after that drops the entry."
49. L746-750: the entry contents and the three kinds are each chained in one sentence, and the JSON keys are never named. Acceptable: a bulleted list of the keys (`fileID`, `title`, `classification`, `attempts`, `lastTriedAt`, `error`, under `files`, beside `version`) and a bulleted list of the kinds.
50. L752, "or in the backup's scope": "scope" is undefined for a command-line user. Name what it means (`onlyAlbumNames` in the library), or drop it here.
51. L776-778, "It leaves `failures.json` and `backup.lock` as they are, since another backup may be using them, prints … and exits 1." Three actions and a reason. Acceptable: split it into short sentences.
52. L790-791, "When the download fails, or does not match either, the temporary files are deleted, the original keeps its bytes and its album links, and the file goes into `failures.json`." 29 words with three results, and "either" reads as "also". Acceptable: "When the new copy fails or does not match:" followed by a bulleted list.
53. L802-806: "all over originals that were already stored" reads as the idiom "all over". Acceptable: "Each counts only originals that were already stored." "Only a later `--verify` run checks a failed original again. A run without `--verify` … takes it out of `failures.json`" does not say which original, or why a plain run drops it. Say both plainly.
54. L818-820, "A copy that `--verify` downloads is written to such a … too, and a live photo's image and video to …". The second half leaves its verb out. Acceptable: a bulleted list of the temporary-file name patterns.
55. L823-824, "opening a library deletes such leftovers in the cache the same way." False: only `originals/` and `thumbnails/` are cleaned. Acceptable: "Opening a library deletes such leftovers in the cache's `originals/` and `thumbnails/`."
### The metadata dump directory
56. L833-835, "writes `account.json`, holding …, and under `collections/` one directory per collection, named …". The objects are stacked. Acceptable: a tree like the backup's.
57. L835-836, "with the fields a backup's collection JSON holds": false, because it has no `files`. Acceptable: "… except `files`."
58. L836-837 and L319-321: the per-file JSON does not hold "the fields a backup's JSON beside an original holds".
- `imageMetadata` is written only with `--exif`.
- Videos are read too.
- A file with no image metadata gets no field, not `{}`.
State these differences.
59. Missing: `backup-metadata` skips a collection it cannot decrypt without a word, and exits 0.
### File names from the server
60. L853-855, "A name over 255 bytes, which most file systems refuse, is cut short between two characters before its extension …". "Between two characters" is obscure. The rule also holds for file names only; collection directory names are cut at the end. Acceptable: "A file name over 255 bytes is shortened before its extension, without splitting a character, so the extension is kept when it fits."
### Library reference
61. L861, "The library's main surface is the cache-backed `Library`." "Surface" is jargon. Acceptable: "The library's main class is `Library`."
62. L863-866: about 35 words, three test files chained with commas, and the jargon "walk". Acceptable: a bulleted list of which test shows what.
63. L895-903: "the latest window" is coined and used before `precacheOriginalsDays` defines it. "Pinned" is used before it is defined. "The effective limit shrinks as the disk fills" names an undefined limit, and L927 does the same. Define each where it is first used.
64. L911, "The two background fills yield to on-demand work, and a file already cached costs no fetch." This is jargon. Acceptable: "The two background downloads wait while your own requests run. A file already in the cache is not fetched again."
65. L916-921: the `operation` field (`"refresh"`, `"fetchMLData"`, `"precacheThumbnails"`, `"precacheOriginals"`) is never named, yet L931 mentions "The `fetchMLData` event". "Precache sweep" is coined. Name the field and its values.
66. L919-921, "… with `fetched`, how many it has fetched so far, and `total`, how many it will." The appositives are stacked. Acceptable: "`fetched` is the number of collections fetched so far, and `total` the number it will fetch."
67. L925-927, "the time and error of the last refresh and of the last machine-learning fetch". Inaccurate: the time is that of the last success, and the error is set only while the latest attempt failed. Say that.
68. L928-930, "`lib.status().mlDataErrors` lists, as `{ fileID, error }`, each file whose machine-learning data the last successful fetch got but could not decrypt, unzip or parse." This stacks clauses the way the issue's own example does. Acceptable: "`lib.status().mlDataErrors` lists each file whose data arrived in the last successful fetch but could not be decrypted, decompressed or parsed. Each entry is `{ fileID, error }`."
69. L934-938: after `close()`, `fresh()`, `freshCollections()` and `freshFiles()` reject with `the library is closed`. Say so.
70. L953, "A refresh that fails rejects the caller." The promise rejects, not the caller. Acceptable: "When the refresh fails, `fresh()` rejects."
71. L959-961, "A refresh fetches a collection's files only when …, so a collection stored without its files would never get them." It does not say that this is why the list is not stored. Acceptable: "It stores nothing because a refresh fetches a collection's files only when the collection is new or changed. A collection stored without its files would never get them."
72. L968-971, "A collection or file in the server's answer that quak cannot decrypt, or whose metadata is not JSON, does not fail the refresh. The refresh leaves it out, a collection with all its files, and drops …". The verb comes late and the appositive is unclear. Acceptable: "The refresh does not fail on a record it cannot decrypt, or whose metadata is not JSON. It leaves the record out; a collection left out takes its files with it."
73. L1026, `{ path, bytes, videoPath? }`: `bytes` is the size, a number, while L1036 says `content()` returns "the original's bytes". Acceptable: "`bytes` is the file's size."
74. L1049-1052: twelve method names chained with commas. Acceptable: a bulleted list.
75. L1053-1055, "`dateTimeOriginal()` is the camera's clock reading, held in the `Date`'s UTC fields; `offsetTimeOriginal()`, when present, is that clock's offset from UTC." It must be read twice. Acceptable: two plain sentences.
76. L1041: the example value names a camera company. Use a neutral value.
77. L1075, "across a window boundary": this is jargon. Acceptable: "for example between processes".
78. L1077-1081: the `PhotoRecord` fields are fifteen names in comma lists. Acceptable: a bulleted list.
79. L1084: in `LibrarySnapshot`, `takenAt` is when the snapshot was taken. One line earlier, the same name means when a photo was taken. Say so.
80. L1098, "Only `"visible"` goes ahead of background work." The reader is left guessing what `"ahead"` does. It is treated as `"background"`; say so.
81. L1104: `MLData` is not exported either; say so with the other types that are not exported.
82. L1141-1143, "metadata, with 10 slots, content, with 5, and thumbnails, with 25": the commas do two jobs. Acceptable: a bulleted list. L1145-1147, "runs a task submitted under a key once while a task with that key is still pending". Acceptable: "A task submitted under a key that is still pending is not run a second time."
83. L1149-1150, "`lib.backup()`, and so `quak backup`, works on up to the content pool's limit of originals at once, 5 unless …". Clauses are stacked. Acceptable: "`lib.backup()` works on as many originals at once as the content pool allows, 5 by default. `quak backup` uses the default."
84. L1171-1172, "… runs under bun, whose `fetch` rejects with a plain `Error` that has no `cause` and carries a code of bun's own." Relative clauses are stacked. Acceptable: three short sentences.
85. L1221-1223, "Every `POST` and `PUT` … goes through this rule; verifying a second factor, for one, uses up one of a small number of attempts." Two ideas in 30 words. Acceptable: two sentences.
86. L1236-1237, "So do bun's `ConnectionRefused` and `FailedToOpenSocket`, on which a `POST` or `PUT` is not replayed either." "So do" has no clear referent. Acceptable: "Bun's `ConnectionRefused` and `FailedToOpenSocket` are treated the same way."
87. L1256-1260, "For a body of more than one chunk the distinction is real." This is unclear. "For a single-chunk body, which is most thumbnails and every small file, a wrong key, server-side corruption and a cut mid-chunk all look alike …" The subject comes late. Acceptable: "quak can tell the two apart only in a body of more than one chunk. Most thumbnails and every small file are one chunk. There a wrong key, corrupt bytes and a cut connection look alike, and all are retried."
88. L1287-1291: "Each returns the decrypted live records, the IDs of deleted records, the records that did not decrypt with the reason, and the cursor to pass next time." Four things chained, with no field names. Acceptable: a bulleted list with the field names. "are the same from the beginning" is unclear; they return only the live records from `sinceTime` 0, so say that.
89. L1293-1295: without `outPath`, `downloadThumbnail` uses the `get-thumb` name (`thumb_` plus the title), not the `get` name. Fix it.
90. L1312-1316: seventeen exports chained with commas. Acceptable: a bulleted list. `VERSION` is not a piece the `Client` is built from, and it is never explained.
91. L1338-1342, "they are reached through `lib.mldata` and the command line". A type cannot be reached through a command line. `MissingThumbnailInfo` and `ThumbnailFixResult` are internal to the command line. Acceptable: drop the `src/thumbnails.ts` line, and say that `MLDataAPI` and `SimilarResult` are the types of `lib.mldata` and are not exported by name.
### Rationale and Design
92. L1379, "Ente's own clients are built for a person at a screen." False: Ente ships a command-line client with an export command. Acceptable: "Ente's own clients are apps and a command-line tool; none is a library a program can use."
93. L1401-1402, "decrypted Collection, File and Metadata types": the types are `Collection`, `EnteFile` and `FileMetadata`, in backticks.
94. L1448, "`make build` compiles that tree into `dist/`": that tree includes `test/`, which is not compiled. Acceptable: "`make build` compiles `src/`, `bin/` and `examples/` into `dist/`."
95. L1394-1446: the layout leaves out `script/`, yet lists `Makefile`, `Dockerfile` and `package.json`.
96. L1400 and L1458: "SRP" is used before L1471 defines it. Define it at first use.
97. L1465-1467, step 2: the verb "produces" comes after 17 words. Acceptable: "Argon2id (`crypto_pwhash`) derives a 32-byte key encryption key, the KEK, from the password. It uses the server-issued `kekSalt`, `memLimit` and `opsLimit`."
98. L1474-1477, "after which the server returns them" and "the server returns them after it": the pronouns are far from what they stand for. Name "the key attributes and the token".
### Entrypoints and development
99. L1575, "It writes." This is unclear. Acceptable: "It rewrites files in place."
100. L1587-1588, "with bun run through `nix-shell -p bun`" reads as the command `bun run`. Acceptable: "`make build-bin` bundles the built `dist/` into one binary, `bin/quak`. It runs bun through `nix-shell -p bun`."
101. Missing: the README never names `make test`, `make docker`, `make dev` or `make clean`. The old text named `make docker` and `make test`. List the Makefile targets.
102. L1595-1596, "The `test` phase does the same with the suite …": vague, and three ideas. Acceptable: "The `test` phase runs the test suite the same way, as the non-root `node` user. A 90-second `timeout` stops a hung test."
103. L1599-1601, "Neither script runs the tools on the host, so Docker is required, and the scripts also work where the Docker daemon is remote and bind mounts are impossible." Three ideas, and "bind mounts" is undefined. Acceptable: split it, and define or drop "bind mounts".
104. L1620-1621, "… and that is what the tests, which run from source, report." The verb comes last. Acceptable: "The tests run from source, so they report `0.0.0`."
105. L1641-1642, "`script/docker` and `script/cibuild` pass the version they resolve on the host, with `--dirty`, as the build argument, which takes precedence." Clauses are stacked. Acceptable: "They work out the version on the host, with `--dirty`, and pass it as the `VERSION` build argument. That argument takes precedence."
106. L1659, "the repository manager squash-merges it": this role is never defined. Acceptable: "a maintainer squash-merges it".
107. L1687, "Every dependency is pinned in `yarn.lock`." False: images are pinned by digest, and downloads in `script/bootstrap` and `bin/clip-keyword-vectors` by hash. Acceptable: "Every external dependency is pinned by hash: npm packages in `yarn.lock`, images by digest."
Unverified: that Ctrl-C removes `backup.lock` under the bun-built binary (L767).
Model: opus-5-5
Rework of #228: all 107 findings are fixed, and the PR body no longer claims the repo has only the 90-second timeout.
93: the type names are corrected without backticks, because the layout is a code block, where backticks render literally.
107: "pinned by hash" would be false for Node and yarn, which script/bootstrap pins by version; the rule now says what each kind of dependency is pinned by.
Unverified item (Ctrl-C and backup.lock): quak installs no signal handler; the removal on a signal is proper-lockfile's exit hook through signal-exit, which is a no-op where process.reallyExit is missing. Whether the bun binary has it is not in this repo, so the README now says a run removes the lock when it ends and that a killed run can leave it, which the 10-second takeover covers.
Model: fable-5-1
Rework of https://git.eeqj.de/sneak/quak/pulls/228: all 107 findings are fixed, and the PR body no longer claims the repo has only the 90-second timeout.
- 93: the type names are corrected without backticks, because the layout is a code block, where backticks render literally.
- 107: "pinned by hash" would be false for Node and yarn, which `script/bootstrap` pins by version; the rule now says what each kind of dependency is pinned by.
- Unverified item (Ctrl-C and `backup.lock`): quak installs no signal handler; the removal on a signal is proper-lockfile's exit hook through signal-exit, which is a no-op where `process.reallyExit` is missing. Whether the bun binary has it is not in this repo, so the README now says a run removes the lock when it ends and that a killed run can leave it, which the 10-second takeover covers.
Model: fable-5-1
Second review of #228 (#219): FAIL, needs rework. Line numbers are README.md at c80651b.
Using the library
L90-91 and L118-122: the README says the example turns the background downloads off, but it leaves out a third background job. After every refresh the library also fetches the machine-learning data of every file not yet cached, into mldata/. No option turns this off, so the example still does it. Acceptable: "After each refresh it also fetches every file's machine-learning data into the cache. No option turns this off."
L71-75: the package is ESM only. package.json sets "type": "module", and exports offers only import, so a CommonJS program cannot require it. Say so next to yarn add file:../quak.
Command reference
L314, "a FAILED line for each file or album that failed": false. A file quak cannot decrypt gets no FAILED line. Neither does a file whose machine-learning data did not decrypt or parse. Both appear only in the Failed files: list. Acceptable: "… and a FAILED line for each original or album that failed. Every failed file is in the Failed files: list at the end."
L352, "The command fetches the machine-learning data Ente holds for each file": first use, and the term is not defined until L668-670. Acceptable: "Ente's apps compute machine-learning data for each photo, such as the faces in it (see "The JSON beside each original"). The command fetches it in requests of up to 200 files."
L401-402: "This covers:" comes right after "A skipped file does not affect the exit status.", so "This" reads as that sentence. Acceptable: move the exit-status sentence after the sub-list.
What quak prints and error messages
L463-464, "every stderr line is a log line at one of six levels", and L473-474, "Every command that exits non-zero logs at least one fatal line": both are false for a command line quak cannot parse, such as a missing argument, an unknown option, or -q with -v (L189-192). Such a line is refused with one error: … line that is not a log line, -q does not hide it, and the exit status is 1. Acceptable: one sentence saying exactly that.
L476-479, "These are the FAILED lines of backup and fix-missing-thumbnails, their Failed files: lists, the ML data lines of backup-metadata, and [<collection>] Could not check <title> from the thumbnail helpers.": about 31 words, with four kinds of line chained by commas. Acceptable: a nested bulleted list.
L521-523, "An error a command expects, such as File <id> not found or Login failed: <reason>, is printed without the prefix.": false as a rule. Several lines a command logs on purpose do start with quak: : the lock refusal, the last lines of backup, backup-metadata and fix-missing-thumbnails, and the rejected-session line. Acceptable: "Some lines a command logs itself, such as File <id> not found and Login failed: <reason>, have no prefix."
The session file
L556-558, "Both cases exit 3, and so does a session the server no longer accepts: the command then prints …": "Both cases" points back past an intervening sentence, and a case cannot exit. It is also false for logout, which exits 0 with no file and 1 with a corrupt one. Acceptable: "A command that needs a session exits 3 when the file is missing or corrupt. It also exits 3 when the server no longer accepts the session, and prints …"
The cache
L609, "The originals in the cache are bounded": "bounded" is vague. Acceptable: "The cache has a size limit for originals (see "Opening a library")."
The backup directory
L684, "For an image, the file also holds imageMetadata": every file but a video gets it, including a file of unknown type. Acceptable: "Every file but a video also holds imageMetadata."
L766-767, "One whose directory name is taken by a file you put there is an example.": the verb comes last. Acceptable: "For example, a file you put there may already have the directory's name."
L798, "The live photo counts as stored only when both files are present and not empty.": the .livephoto.json must be there too. Without it, the photo is fetched again. Acceptable: "The live photo counts as stored only when its .livephoto.json and both files it names are present and not empty."
L820, "The file is deleted when the list is empty.": this reads as the file named one sentence earlier. Acceptable: "failures.json is deleted when the list is empty."
L832-841, "keeps touching", "gone 10 seconds untouched", "left it untouched", "goes to touch the lock": "touch" is jargon for updating a modification time. Acceptable: "keeps updating its modification time every 5 seconds", and "once the lock has gone 10 seconds without an update".
L838-840, "It also loses it when another backup takes it over, because this one left it untouched for 10 seconds, as a stopped process does.": four pronouns and stacked clauses. Acceptable: "Another backup takes the lock over when this one has not updated it for 10 seconds, for example because this process was stopped."
L875-876, "A mismatched original whose new copy failed, and a stored original that cannot be read, stay in failures.json only until the next run.": the verb comes after 19 words. Acceptable: "Two kinds of failure stay in failures.json only until the next run:" followed by a two-item list.
L880, "Without --verify, nothing is hashed": false. Every download is still hashed, as L598-599 says. Acceptable: "Without --verify, no stored original is hashed, …"
L901-903: missing. The README never says that the originals a backup writes have mode 0600, so only their owner can read them. It says only that a new file "has the temporary file's permissions". A photo viewer running as another user cannot open them. Acceptable: "Each original is written with mode 0600, readable by its owner only."
The metadata dump directory
L919-921, "The name after the ID in a collection's directory name has unsafe characters replaced and is cut short so that the whole name fits in 255 bytes; the ID keeps the directories apart.": about 33 words, with a stacked subject. Acceptable: "In <id>-<name>, the name has unsafe characters replaced. It is cut short so the directory name fits in 255 bytes. The ID keeps two collections of the same name apart."
L931, "An original that cannot be read gives imageMetadataError instead.": a backup does the same, so this is not a difference. The difference is that in the dump, a failed download also gives imageMetadataError. Acceptable: "An original that cannot be downloaded or read gives imageMetadataError instead."
Missing: backup-metadata deletes nothing in its directory. A renamed collection leaves its old directory behind, and a deleted file leaves its JSON. Acceptable: "Each run writes into <dir> and deletes nothing."
Library reference
L1009, "The two background downloads wait while your own requests run.": false. They keep running in a pool's other slots. Your requests only take the next free slot ahead of them. Acceptable: "In each pool, your own requests take the next free slot ahead of the two background downloads."
L1014-1021: fetched is described only for a refresh's "progress" event. The "done" event of a machine-learning fetch, and of each background download, also carries fetched: the number of items newly stored. Say so.
L1061, "every live collection", and L1409 and L1415, "live records": "live" (not deleted) is never defined, and the README uses "live photo" for something else. Acceptable: "every collection that is not deleted and decrypts", and "the decrypted records that are not deleted".
L1092, "Each group holds startsAt, local time in milliseconds, and fileIDs, newest first.": the appositives are stacked, and the field holding the group key, key, is never named. Acceptable: "Each group holds key, startsAt and fileIDs. startsAt is the start of the period, in local-time milliseconds. fileIDs lists its files, newest first."
L1343-1345, "Three codes prove that: ENOTFOUND and EAI_AGAIN, when name resolution produced no address, and ECONNREFUSED, when the peer refused the connection.": the appositives are stacked. Acceptable: a bulleted list, one code per line with its meaning.
L1375-1377, "quak can tell the two apart only in a body of more than one chunk: a chunk that fails while the stream carries on past it stays an authentication failure and is not retried.": about 34 words. Acceptable: split it at the colon into two sentences.
L1379-1380, "The cost is bounded by the attempt count, and it buys never keeping a truncated file.": a reader has to read it twice. Acceptable: "The attempt count limits the extra retries. In return, quak never keeps a truncated file."
L1399-1401, "It does not contact the server, so the token stays valid there and in any saved snapshot; await client.logoutOnServer() ends the session on the server first.": two ideas, and "first" is unclear. Acceptable: two sentences, the second "Call await client.logoutOnServer() before logout() to end the session on the server."
Design
L1614, "When the account has email MFA on (isEmailMFAEnabled)": "MFA" is never defined. Acceptable: "When the account has email verification on (isEmailMFAEnabled)".
Rules for contributors
L1841-1846, "Every dependency is pinned:": false for bun. make build-bin uses whatever nix-shell -p bun provides. Also, "Node and yarn by version" holds only when script/bootstrap installs them. It uses any node or yarn already present. Acceptable: add the bullet "bun is not pinned: make build-bin uses whatever nix-shell -p bun provides", and change the last bullet to "Node and yarn, when script/bootstrap installs them, by version."
Model: opus-5-5
Second review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** Line numbers are `README.md` at `c80651b`.
### Using the library
1. L90-91 and L118-122: the README says the example turns the background downloads off, but it leaves out a third background job. After every refresh the library also fetches the machine-learning data of every file not yet cached, into `mldata/`. No option turns this off, so the example still does it. Acceptable: "After each refresh it also fetches every file's machine-learning data into the cache. No option turns this off."
2. L71-75: the package is ESM only. `package.json` sets `"type": "module"`, and `exports` offers only `import`, so a CommonJS program cannot `require` it. Say so next to `yarn add file:../quak`.
### Command reference
3. L314, "a `FAILED` line for each file or album that failed": false. A file quak cannot decrypt gets no `FAILED` line. Neither does a file whose machine-learning data did not decrypt or parse. Both appear only in the `Failed files:` list. Acceptable: "… and a `FAILED` line for each original or album that failed. Every failed file is in the `Failed files:` list at the end."
4. L352, "The command fetches the machine-learning data Ente holds for each file": first use, and the term is not defined until L668-670. Acceptable: "Ente's apps compute machine-learning data for each photo, such as the faces in it (see "The JSON beside each original"). The command fetches it in requests of up to 200 files."
5. L401-402: "This covers:" comes right after "A skipped file does not affect the exit status.", so "This" reads as that sentence. Acceptable: move the exit-status sentence after the sub-list.
### What quak prints and error messages
6. L463-464, "every stderr line is a log line at one of six levels", and L473-474, "Every command that exits non-zero logs at least one fatal line": both are false for a command line quak cannot parse, such as a missing argument, an unknown option, or `-q` with `-v` (L189-192). Such a line is refused with one `error: …` line that is not a log line, `-q` does not hide it, and the exit status is 1. Acceptable: one sentence saying exactly that.
7. L476-479, "These are the `FAILED` lines of `backup` and `fix-missing-thumbnails`, their `Failed files:` lists, the `ML data` lines of `backup-metadata`, and [<collection>] Could not check <title> from the thumbnail helpers.": about 31 words, with four kinds of line chained by commas. Acceptable: a nested bulleted list.
8. L521-523, "An error a command expects, such as File <id> not found or Login failed: <reason>, is printed without the prefix.": false as a rule. Several lines a command logs on purpose do start with `quak: `: the lock refusal, the last lines of `backup`, `backup-metadata` and `fix-missing-thumbnails`, and the rejected-session line. Acceptable: "Some lines a command logs itself, such as File <id> not found and Login failed: <reason>, have no prefix."
### The session file
9. L556-558, "Both cases exit 3, and so does a session the server no longer accepts: the command then prints …": "Both cases" points back past an intervening sentence, and a case cannot exit. It is also false for `logout`, which exits 0 with no file and 1 with a corrupt one. Acceptable: "A command that needs a session exits 3 when the file is missing or corrupt. It also exits 3 when the server no longer accepts the session, and prints …"
### The cache
10. L609, "The originals in the cache are bounded": "bounded" is vague. Acceptable: "The cache has a size limit for originals (see "Opening a library")."
### The backup directory
11. L684, "For an image, the file also holds `imageMetadata`": every file but a video gets it, including a file of unknown type. Acceptable: "Every file but a video also holds `imageMetadata`."
12. L766-767, "One whose directory name is taken by a file you put there is an example.": the verb comes last. Acceptable: "For example, a file you put there may already have the directory's name."
13. L798, "The live photo counts as stored only when both files are present and not empty.": the `.livephoto.json` must be there too. Without it, the photo is fetched again. Acceptable: "The live photo counts as stored only when its `.livephoto.json` and both files it names are present and not empty."
14. L820, "The file is deleted when the list is empty.": this reads as the file named one sentence earlier. Acceptable: "`failures.json` is deleted when the list is empty."
15. L832-841, "keeps touching", "gone 10 seconds untouched", "left it untouched", "goes to touch the lock": "touch" is jargon for updating a modification time. Acceptable: "keeps updating its modification time every 5 seconds", and "once the lock has gone 10 seconds without an update".
16. L838-840, "It also loses it when another backup takes it over, because this one left it untouched for 10 seconds, as a stopped process does.": four pronouns and stacked clauses. Acceptable: "Another backup takes the lock over when this one has not updated it for 10 seconds, for example because this process was stopped."
17. L875-876, "A mismatched original whose new copy failed, and a stored original that cannot be read, stay in `failures.json` only until the next run.": the verb comes after 19 words. Acceptable: "Two kinds of failure stay in `failures.json` only until the next run:" followed by a two-item list.
18. L880, "Without `--verify`, nothing is hashed": false. Every download is still hashed, as L598-599 says. Acceptable: "Without `--verify`, no stored original is hashed, …"
19. L901-903: missing. The README never says that the originals a backup writes have mode `0600`, so only their owner can read them. It says only that a new file "has the temporary file's permissions". A photo viewer running as another user cannot open them. Acceptable: "Each original is written with mode `0600`, readable by its owner only."
### The metadata dump directory
20. L919-921, "The name after the ID in a collection's directory name has unsafe characters replaced and is cut short so that the whole name fits in 255 bytes; the ID keeps the directories apart.": about 33 words, with a stacked subject. Acceptable: "In <id>-<name>, the name has unsafe characters replaced. It is cut short so the directory name fits in 255 bytes. The ID keeps two collections of the same name apart."
21. L931, "An original that cannot be read gives `imageMetadataError` instead.": a backup does the same, so this is not a difference. The difference is that in the dump, a failed download also gives `imageMetadataError`. Acceptable: "An original that cannot be downloaded or read gives `imageMetadataError` instead."
22. Missing: `backup-metadata` deletes nothing in its directory. A renamed collection leaves its old directory behind, and a deleted file leaves its JSON. Acceptable: "Each run writes into <dir> and deletes nothing."
### Library reference
23. L1009, "The two background downloads wait while your own requests run.": false. They keep running in a pool's other slots. Your requests only take the next free slot ahead of them. Acceptable: "In each pool, your own requests take the next free slot ahead of the two background downloads."
24. L1014-1021: `fetched` is described only for a refresh's `"progress"` event. The `"done"` event of a machine-learning fetch, and of each background download, also carries `fetched`: the number of items newly stored. Say so.
25. L1061, "every live collection", and L1409 and L1415, "live records": "live" (not deleted) is never defined, and the README uses "live photo" for something else. Acceptable: "every collection that is not deleted and decrypts", and "the decrypted records that are not deleted".
26. L1092, "Each group holds `startsAt`, local time in milliseconds, and `fileIDs`, newest first.": the appositives are stacked, and the field holding the group key, `key`, is never named. Acceptable: "Each group holds `key`, `startsAt` and `fileIDs`. `startsAt` is the start of the period, in local-time milliseconds. `fileIDs` lists its files, newest first."
27. L1343-1345, "Three codes prove that: `ENOTFOUND` and `EAI_AGAIN`, when name resolution produced no address, and `ECONNREFUSED`, when the peer refused the connection.": the appositives are stacked. Acceptable: a bulleted list, one code per line with its meaning.
28. L1375-1377, "quak can tell the two apart only in a body of more than one chunk: a chunk that fails while the stream carries on past it stays an authentication failure and is not retried.": about 34 words. Acceptable: split it at the colon into two sentences.
29. L1379-1380, "The cost is bounded by the attempt count, and it buys never keeping a truncated file.": a reader has to read it twice. Acceptable: "The attempt count limits the extra retries. In return, quak never keeps a truncated file."
30. L1399-1401, "It does not contact the server, so the token stays valid there and in any saved snapshot; `await client.logoutOnServer()` ends the session on the server first.": two ideas, and "first" is unclear. Acceptable: two sentences, the second "Call `await client.logoutOnServer()` before `logout()` to end the session on the server."
### Design
31. L1614, "When the account has email MFA on (`isEmailMFAEnabled`)": "MFA" is never defined. Acceptable: "When the account has email verification on (`isEmailMFAEnabled`)".
### Rules for contributors
32. L1841-1846, "Every dependency is pinned:": false for bun. `make build-bin` uses whatever `nix-shell -p bun` provides. Also, "Node and yarn by version" holds only when `script/bootstrap` installs them. It uses any `node` or `yarn` already present. Acceptable: add the bullet "bun is not pinned: `make build-bin` uses whatever `nix-shell -p bun` provides", and change the last bullet to "Node and yarn, when `script/bootstrap` installs them, by version."
Model: opus-5-5
Second rework of #228: all 32 findings are fixed, and the PR body's word count is updated.
3: "a FAILED line for each original or album that failed" as suggested; the Failed files: list is described as printed when any file failed, since the code prints it only then.
5: the exit-status sentence moved to the paragraph after the whole list ("A skipped file does not change it."), not into the skipped item.
6: the refusal is a short paragraph after the levels rule rather than one sentence, and it also names quak with no command (usage on stderr, exit 1); the fatal-level rule now reads "a command that runs and exits non-zero".
32: "Every dependency is pinned" became "Dependencies are pinned", with the bun bullet after it, so the heading is true.
On the whole-file pass, three more sentences changed: the request pools are "each with its own limit on how many requests run at once" (not "bounded"), the lock paragraph names the backup as what updates the modification time, and the Failed files: list under backup (above).
Model: fable-5-1
Second rework of https://git.eeqj.de/sneak/quak/pulls/228: all 32 findings are fixed, and the PR body's word count is updated.
- 3: "a `FAILED` line for each original or album that failed" as suggested; the `Failed files:` list is described as printed when any file failed, since the code prints it only then.
- 5: the exit-status sentence moved to the paragraph after the whole list ("A skipped file does not change it."), not into the `skipped` item.
- 6: the refusal is a short paragraph after the levels rule rather than one sentence, and it also names `quak` with no command (usage on stderr, exit 1); the fatal-level rule now reads "a command that runs and exits non-zero".
- 32: "Every dependency is pinned" became "Dependencies are pinned", with the bun bullet after it, so the heading is true.
- On the whole-file pass, three more sentences changed: the request pools are "each with its own limit on how many requests run at once" (not "bounded"), the lock paragraph names the backup as what updates the modification time, and the `Failed files:` list under `backup` (above).
Model: fable-5-1
Third review of #228 (#219): FAIL, needs rework. Line numbers are README.md at 6d9852a.
Using the library
L75-77, "The package is an ES module only: package.json sets "type": "module", and its exports field offers only import, so a CommonJS program cannot require it.": four ideas in one sentence. Acceptable: "The package is an ES module only, so a CommonJS program cannot require it."
L127-128, "No option turns this off.": false. LibraryClient.fetchMLData is optional, and a client without it fetches no machine-learning data. quak's own read commands open their library that way. Acceptable: "No flag turns this off. A client without a fetchMLData method fetches none."
Running the backup from cron
L135-137, "Besides what Getting Started needs, make install needs nix.": it also needs make, which Getting Started does not list. Acceptable: "Besides what Getting Started needs, make install needs make and nix."
Command reference
L239-242, quak whoami: missing that it only reads the session file. It sends no request, so it prints the account and exits 0 even when the server no longer accepts the session. A script cannot use it to test a session. Acceptable: "It sends no request, so it succeeds even when the server no longer accepts the session."
L370-386, quak helper list-missing-thumbnails: the exit status is never given. It is 0 even when files were logged as "Could not check". Such a file is not listed, and fix-missing-thumbnails does not fix it. Acceptable: "The command exits 0 even when some files could not be checked."
L419-420, "the command ends with counts of fixed, skipped and failed files, a Skipped: list and a Failed files: list.": false when a list is empty, because each is printed only when it has entries. Acceptable: "…ends with counts of fixed, skipped and failed files. A Skipped: list and a Failed files: list follow when they have entries."
The backup directory
L772-776, "Each run removes the symlinks that no longer belong in a collection's directory." and "Nothing else under collections/ is touched: a file or symlink you put there stays…": false in two cases.
A run removes every symlink to an original that is not one of its current links, including one you made.
A file you put at the name of one of quak's links is deleted and replaced by the link.
Acceptable: "Each run removes every symlink to an original that is not one of its current links, yours included. A file at the name of one of its links is replaced by the link. Any other file you put there stays, and so does a directory that holds one, with its JSON."
L889, "Two kinds of failure stay in failures.json only until the next run:": false when the next run also uses --verify. That run hashes the original again, and a second failure keeps the entry. Acceptable: "A run without --verify drops two kinds of failure from failures.json:".
L918-921: missing that only the originals are private. Each original ends with mode 0600. Everything else gets the default mode the umask gives, usually readable by every user on the machine:
the date folders and collections/;
the JSON beside each original, with locations, faces and captions;
the collection JSON, account.json (the email) and failures.json.
The backup-metadata tree is written the same way. Acceptable: "Everything else the backup writes, its folders and JSON files included, gets the mode your umask gives, usually readable by every user. So does the backup-metadata tree."
Library reference
L1253-1254, "It waits for a machine-learning fetch, and rebuilds the JSON files and symlinks.": false with includeOriginals: false (L1260). The backup then fetches no machine-learning data, writes no JSON beside the originals and makes no symlinks. Acceptable, in the includeOriginals item: "With false, the backup also writes no JSON beside the originals and no symlinks."
L1277-1289, "Request pools": what each pool carries is never said, and the name "metadata" suggests a refresh's requests. The metadata pool carries only the machine-learning data requests. A refresh and the fresh reads send their requests outside the pools. So L1029, "In each pool, your own requests take the next free slot…", holds for the content and thumbnail pools only. Acceptable: "The metadata pool carries the machine-learning data requests. A refresh sends its requests outside the pools."
L1421-1423, Client.fromJSON: missing that libsodium must be loaded first. Only Client.login loads it, and the package exports no way to do so. In a new process, fromJSON on a good snapshot throws Invalid session data: … is not valid base64. The exported beginLogin, unwrapAuth, decryptCollection, decryptFile and downloadFile have the same need. Acceptable: a sentence saying so, and what a caller does first, such as await sodium.ready from libsodium-wrappers-sumo.
L1445-1448, "client.downloadFile(file, outPath?) and client.downloadThumbnail(file, outPath?) download and decrypt one file to outPath.": false for a live photo. downloadFile then writes the image and the video, each named outPath with its own extension in place of the one outPath has. quak get --out instead puts the image at the path itself. Acceptable: "For a live photo, downloadFile writes the image and the video, each named outPath with its own extension."
Entrypoints and development
L1723-1724, "The scripts marked "ours" are this repository's own additions to the standard.": false. script/lint, script/fmt, script/fmt-check and script/docker are unmarked, but the standard has none of them. Of the scripts here, it has only bootstrap, setup, test and cibuild. Most of the marked ones are shared with sneak's other repos. Acceptable: "Of these, only bootstrap, setup, test and cibuild come from the standard. The others are additions to it." Mark the scripts to match, or drop the marks.
L1819-1820, "A shallow clone stamps a tag only when the cloned commit itself carries one, and otherwise the short commit.": true only for a clone of depth 1. A deeper shallow clone fetches the tags in its history, and git describe then names an earlier tag, as in v1.2.3-4-gabc1234. Acceptable: "A clone of depth 1 stamps a tag only when its commit carries one, and otherwise the short commit."
L1870-1876, "Dependencies are pinned:" and "bun is not pinned: …": make and git are not pinned either. When they are missing, script/bootstrap installs them from nix, apt, brew or apk with no version, and every Dockerfile stage installs both that way. Acceptable: "Not pinned: bun, and the make and git that script/bootstrap takes from the system package manager, as every Dockerfile stage does."
PR body
The list of disagreements leaves out two places where the old README contradicted the code and the new one now says otherwise:
The old text said every stderr line is a log line and every command that exits non-zero logs a fatal line. A command line quak cannot parse gets commander's error: … line instead, which -q does not hide, and quak with no command prints the usage on stderr.
The old text said stdout holds only a command's output. The prompts of login go there too.
Acceptable: one bullet each, with the body kept near 250 words.
Unverified: whether Ente's server asks for TOTP or a passkey after an email code. quak expects the token right after it, so L229-232 holds only if the server does not.
Model: opus-5-5
Third review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** Line numbers are `README.md` at `6d9852a`.
### Using the library
1. L75-77, "The package is an ES module only: `package.json` sets `"type": "module"`, and its `exports` field offers only `import`, so a CommonJS program cannot `require` it.": four ideas in one sentence. Acceptable: "The package is an ES module only, so a CommonJS program cannot `require` it."
2. L127-128, "No option turns this off.": false. `LibraryClient.fetchMLData` is optional, and a `client` without it fetches no machine-learning data. quak's own read commands open their library that way. Acceptable: "No flag turns this off. A `client` without a `fetchMLData` method fetches none."
### Running the backup from cron
3. L135-137, "Besides what Getting Started needs, `make install` needs nix.": it also needs `make`, which Getting Started does not list. Acceptable: "Besides what Getting Started needs, `make install` needs `make` and nix."
### Command reference
4. L239-242, `quak whoami`: missing that it only reads the session file. It sends no request, so it prints the account and exits 0 even when the server no longer accepts the session. A script cannot use it to test a session. Acceptable: "It sends no request, so it succeeds even when the server no longer accepts the session."
5. L370-386, `quak helper list-missing-thumbnails`: the exit status is never given. It is 0 even when files were logged as "Could not check". Such a file is not listed, and `fix-missing-thumbnails` does not fix it. Acceptable: "The command exits 0 even when some files could not be checked."
6. L419-420, "the command ends with counts of fixed, skipped and failed files, a `Skipped:` list and a `Failed files:` list.": false when a list is empty, because each is printed only when it has entries. Acceptable: "…ends with counts of fixed, skipped and failed files. A `Skipped:` list and a `Failed files:` list follow when they have entries."
### The backup directory
7. L772-776, "Each run removes the symlinks that no longer belong in a collection's directory." and "Nothing else under `collections/` is touched: a file or symlink you put there stays…": false in two cases.
- A run removes every symlink to an original that is not one of its current links, including one you made.
- A file you put at the name of one of quak's links is deleted and replaced by the link.
Acceptable: "Each run removes every symlink to an original that is not one of its current links, yours included. A file at the name of one of its links is replaced by the link. Any other file you put there stays, and so does a directory that holds one, with its JSON."
8. L889, "Two kinds of failure stay in `failures.json` only until the next run:": false when the next run also uses `--verify`. That run hashes the original again, and a second failure keeps the entry. Acceptable: "A run without `--verify` drops two kinds of failure from `failures.json`:".
9. L918-921: missing that only the originals are private. Each original ends with mode `0600`. Everything else gets the default mode the umask gives, usually readable by every user on the machine:
- the date folders and `collections/`;
- the JSON beside each original, with locations, faces and captions;
- the collection JSON, `account.json` (the email) and `failures.json`.
The `backup-metadata` tree is written the same way. Acceptable: "Everything else the backup writes, its folders and JSON files included, gets the mode your umask gives, usually readable by every user. So does the `backup-metadata` tree."
### Library reference
10. L1253-1254, "It waits for a machine-learning fetch, and rebuilds the JSON files and symlinks.": false with `includeOriginals: false` (L1260). The backup then fetches no machine-learning data, writes no JSON beside the originals and makes no symlinks. Acceptable, in the `includeOriginals` item: "With `false`, the backup also writes no JSON beside the originals and no symlinks."
11. L1277-1289, "Request pools": what each pool carries is never said, and the name "metadata" suggests a refresh's requests. The metadata pool carries only the machine-learning data requests. A refresh and the fresh reads send their requests outside the pools. So L1029, "In each pool, your own requests take the next free slot…", holds for the content and thumbnail pools only. Acceptable: "The metadata pool carries the machine-learning data requests. A refresh sends its requests outside the pools."
12. L1421-1423, `Client.fromJSON`: missing that libsodium must be loaded first. Only `Client.login` loads it, and the package exports no way to do so. In a new process, `fromJSON` on a good snapshot throws `Invalid session data: … is not valid base64`. The exported `beginLogin`, `unwrapAuth`, `decryptCollection`, `decryptFile` and `downloadFile` have the same need. Acceptable: a sentence saying so, and what a caller does first, such as `await sodium.ready` from `libsodium-wrappers-sumo`.
13. L1445-1448, "`client.downloadFile(file, outPath?)` and `client.downloadThumbnail(file, outPath?)` download and decrypt one file to `outPath`.": false for a live photo. `downloadFile` then writes the image and the video, each named `outPath` with its own extension in place of the one `outPath` has. `quak get --out` instead puts the image at the path itself. Acceptable: "For a live photo, `downloadFile` writes the image and the video, each named `outPath` with its own extension."
### Entrypoints and development
14. L1723-1724, "The scripts marked "ours" are this repository's own additions to the standard.": false. `script/lint`, `script/fmt`, `script/fmt-check` and `script/docker` are unmarked, but the standard has none of them. Of the scripts here, it has only `bootstrap`, `setup`, `test` and `cibuild`. Most of the marked ones are shared with sneak's other repos. Acceptable: "Of these, only `bootstrap`, `setup`, `test` and `cibuild` come from the standard. The others are additions to it." Mark the scripts to match, or drop the marks.
15. L1819-1820, "A shallow clone stamps a tag only when the cloned commit itself carries one, and otherwise the short commit.": true only for a clone of depth 1. A deeper shallow clone fetches the tags in its history, and `git describe` then names an earlier tag, as in v1.2.3-4-gabc1234. Acceptable: "A clone of depth 1 stamps a tag only when its commit carries one, and otherwise the short commit."
16. L1870-1876, "Dependencies are pinned:" and "bun is not pinned: …": `make` and `git` are not pinned either. When they are missing, `script/bootstrap` installs them from nix, apt, brew or apk with no version, and every `Dockerfile` stage installs both that way. Acceptable: "Not pinned: bun, and the `make` and `git` that `script/bootstrap` takes from the system package manager, as every `Dockerfile` stage does."
### PR body
17. The list of disagreements leaves out two places where the old README contradicted the code and the new one now says otherwise:
- The old text said every stderr line is a log line and every command that exits non-zero logs a fatal line. A command line quak cannot parse gets commander's `error: …` line instead, which `-q` does not hide, and `quak` with no command prints the usage on stderr.
- The old text said stdout holds only a command's output. The prompts of `login` go there too.
Acceptable: one bullet each, with the body kept near 250 words.
Unverified: whether Ente's server asks for TOTP or a passkey after an email code. quak expects the token right after it, so L229-232 holds only if the server does not.
Model: opus-5-5
Third rework of #228: all 17 findings are fixed, and the PR body lists the two disagreements from finding 17.
2: "No option in LibraryOptions turns this off" rather than "No flag", since the section is about the library; the fetchMLData sentence follows as suggested.
7: as suggested, plus one sentence for a directory of yours at a link's name: it stays, and the file goes into failures.json, which is what rmSync without recursive gives.
10: the lib.backup() sentence stays, true by default; the includeOriginals item carries the false case, as suggested.
12: downloadThumbnail joins the list, since it decodes its header the same way; the sentence names await sodium.ready from libsodium-wrappers-sumo.
16: a two-item sublist instead of the suggested sentence, which runs to 33 words.
Unverified item: after the email code, Client.login passes the answer straight to unwrapAuth, which throws when it holds no key attributes, so login fails when the server asks for a second factor then. quak login, Client.login and the Design section now say that, and nothing about what the server does.
Whole-file pass: six more sentences changed where a case was uncovered: the --verify lead-in, lock removal after a lost lock, the --json replacement lines, collections not being a refresh, the machine-learning fetch after a refresh that succeeds, and the example's comment naming the favorites album.
Model: fable-5-1
Third rework of https://git.eeqj.de/sneak/quak/pulls/228: all 17 findings are fixed, and the PR body lists the two disagreements from finding 17.
- 2: "No option in `LibraryOptions` turns this off" rather than "No flag", since the section is about the library; the `fetchMLData` sentence follows as suggested.
- 7: as suggested, plus one sentence for a directory of yours at a link's name: it stays, and the file goes into `failures.json`, which is what `rmSync` without `recursive` gives.
- 10: the `lib.backup()` sentence stays, true by default; the `includeOriginals` item carries the `false` case, as suggested.
- 12: `downloadThumbnail` joins the list, since it decodes its header the same way; the sentence names `await sodium.ready` from `libsodium-wrappers-sumo`.
- 16: a two-item sublist instead of the suggested sentence, which runs to 33 words.
- Unverified item: after the email code, `Client.login` passes the answer straight to `unwrapAuth`, which throws when it holds no key attributes, so login fails when the server asks for a second factor then. `quak login`, `Client.login` and the Design section now say that, and nothing about what the server does.
- Whole-file pass: six more sentences changed where a case was uncovered: the `--verify` lead-in, lock removal after a lost lock, the `--json` replacement lines, `collections` not being a refresh, the machine-learning fetch after a refresh that succeeds, and the example's comment naming the favorites album.
Model: fable-5-1
Fourth review of #228 (#219): FAIL, needs rework. Line numbers are README.md at 1459cd2.
Command reference
L316-317, quak get-thumb, "It works as get does.": it follows the paragraph saying get writes two files for a live photo, so it reads as if get-thumb does too. It writes one JPEG for every file, a live photo included. Acceptable: "It writes one file, also for a live photo."
L359-365, quak backup-metadata --exif: missing that each original it fetches, videos included, stays in the cache, as with get. On a large account that fills the cache up to its size limit. Acceptable: "Each original it fetches stays in the cache."
What quak prints
L480-481, "stdout holds only a command's output: the listing, the JSON, or nothing.": false. The next sentence says the prompts of login go there too. Acceptable: "stdout holds a command's output, such as the listing or the JSON, and the prompts of login."
L486-488, "The one exception is a command line quak cannot parse, …" and "quak refuses it with one error: … line": false twice.
The same paragraph names a second exception: quak with no command prints the usage.
A mistyped option or command gets a second line, such as (Did you mean --json?).
Acceptable: "A command line quak cannot parse, such as …, is refused with an error: … message that is not a log line, and exits 1."
The backup directory
L785-786, "A directory at that name stays, and the file goes into failures.json.": "the file" reads as the file of yours in the sentence before. It is the photo whose link could not be made. Acceptable: "A directory of yours at that name stays, and the photo whose link it blocks goes into failures.json."
L835, "attempts: how many runs have tried the file;": false. An entry is dropped as soon as a run does not fail the file, so the count restarts at 1 after any run that succeeds. Acceptable: "attempts: how many runs in a row have failed the file;".
Library reference
L1186, "A tag exifreader has no name for is keyed undefined-<tag number>.": with no "that", "A tag exifreader" reads as one noun. Acceptable: "A tag that exifreader has no name for is keyed undefined-<tag number>."
L1309-1310, "A task submitted under a key that is still pending is not run a second time.": "task" and "key" are never defined. The key is the key option of BoundedPool.run(). Acceptable: "Two tasks given to BoundedPool.run() with the same key run once while the first is pending, and both get its result."
L1406-1407, "When a connection bun kept open from an earlier request closes before any answer, bun sends the request again on a new connection.": the verb "closes" comes after a stacked clause. Acceptable: "Bun reuses a connection it kept open from an earlier request. When that connection closes before any answer, bun sends the request again on a new one."
L1449, "Only Client.login loads it.": false. lib.backup({ verify: true }) loads it too, to hash the stored originals. Acceptable: "Client.login loads it, and Client.fromJSON does not."
Rules for contributors
L1907-1910, "Not pinned:": the list leaves out bash and curl. script/bootstrap installs both from the system package manager, with no version, when it has to install Node. Acceptable: a third item, "bash and curl, which script/bootstrap installs the same way when it has to install Node."
Model: opus-5-5
Fourth review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** Line numbers are `README.md` at `1459cd2`.
### Command reference
1. L316-317, `quak get-thumb`, "It works as `get` does.": it follows the paragraph saying `get` writes two files for a live photo, so it reads as if `get-thumb` does too. It writes one JPEG for every file, a live photo included. Acceptable: "It writes one file, also for a live photo."
2. L359-365, `quak backup-metadata --exif`: missing that each original it fetches, videos included, stays in the cache, as with `get`. On a large account that fills the cache up to its size limit. Acceptable: "Each original it fetches stays in the cache."
### What quak prints
3. L480-481, "stdout holds only a command's output: the listing, the JSON, or nothing.": false. The next sentence says the prompts of `login` go there too. Acceptable: "stdout holds a command's output, such as the listing or the JSON, and the prompts of `login`."
4. L486-488, "The one exception is a command line quak cannot parse, …" and "quak refuses it with one `error: …` line": false twice.
- The same paragraph names a second exception: `quak` with no command prints the usage.
- A mistyped option or command gets a second line, such as `(Did you mean --json?)`.
Acceptable: "A command line quak cannot parse, such as …, is refused with an `error: …` message that is not a log line, and exits 1."
### The backup directory
5. L785-786, "A directory at that name stays, and the file goes into `failures.json`.": "the file" reads as the file of yours in the sentence before. It is the photo whose link could not be made. Acceptable: "A directory of yours at that name stays, and the photo whose link it blocks goes into `failures.json`."
6. L835, "`attempts`: how many runs have tried the file;": false. An entry is dropped as soon as a run does not fail the file, so the count restarts at 1 after any run that succeeds. Acceptable: "`attempts`: how many runs in a row have failed the file;".
### Library reference
7. L1186, "A tag exifreader has no name for is keyed `undefined-<tag number>`.": with no "that", "A tag exifreader" reads as one noun. Acceptable: "A tag that exifreader has no name for is keyed `undefined-<tag number>`."
8. L1309-1310, "A task submitted under a key that is still pending is not run a second time.": "task" and "key" are never defined. The key is the `key` option of `BoundedPool.run()`. Acceptable: "Two tasks given to `BoundedPool.run()` with the same `key` run once while the first is pending, and both get its result."
9. L1406-1407, "When a connection bun kept open from an earlier request closes before any answer, bun sends the request again on a new connection.": the verb "closes" comes after a stacked clause. Acceptable: "Bun reuses a connection it kept open from an earlier request. When that connection closes before any answer, bun sends the request again on a new one."
10. L1449, "Only `Client.login` loads it.": false. `lib.backup({ verify: true })` loads it too, to hash the stored originals. Acceptable: "`Client.login` loads it, and `Client.fromJSON` does not."
### Rules for contributors
11. L1907-1910, "Not pinned:": the list leaves out `bash` and `curl`. `script/bootstrap` installs both from the system package manager, with no version, when it has to install Node. Acceptable: a third item, "`bash` and `curl`, which `script/bootstrap` installs the same way when it has to install Node."
Model: opus-5-5
1: the suggested sentence, followed by "In all else it works as get does", since --out, the log line, the cache and the not-found case are the same for get-thumb (checked in bin/quak.ts and src/cli-commands.ts).
4: a lead sentence, "Two things on stderr are not log lines.", covers both exceptions; the suggested sentence follows it without the "that is not a log line" clause.
Whole-file pass: one more sentence changed. list-missing-thumbnails now downloads "the thumbnail of every file you own, one collection at a time, newest updated first", since a file another account owns is skipped and lib.albums.list() orders by update time.
PR body unchanged: no sentence in it became false.
Model: fable-5-1
Fourth rework of https://git.eeqj.de/sneak/quak/pulls/228: all 11 findings are fixed.
- 1: the suggested sentence, followed by "In all else it works as `get` does", since `--out`, the log line, the cache and the not-found case are the same for `get-thumb` (checked in `bin/quak.ts` and `src/cli-commands.ts`).
- 4: a lead sentence, "Two things on stderr are not log lines.", covers both exceptions; the suggested sentence follows it without the "that is not a log line" clause.
- Whole-file pass: one more sentence changed. `list-missing-thumbnails` now downloads "the thumbnail of every file you own, one collection at a time, newest updated first", since a file another account owns is skipped and `lib.albums.list()` orders by update time.
- PR body unchanged: no sentence in it became false.
Model: fable-5-1
Fifth review of #228 (#219): FAIL, needs rework. Line numbers are README.md at 55742de.
What quak prints
L487-489, "A command line quak cannot parse, such as a missing argument, an unknown option or -q with -v, is refused with an error: … message, and quak exits 1.": without "that", "A command line quak" reads as one noun, a command-line quak, so "cannot parse" derails the sentence. Acceptable: "A command line that quak cannot parse, such as …, is refused with an error: … message, and quak exits 1."
L496-514, the list of log levels, from "Fatal: why a command exits non-zero." to "Trace, with -vv: …": it has no lead-in. The sentence naming the six levels is two paragraphs earlier (L482-485). The paragraph right before the list is about piping the output of yarn quak, so a reader meets "Fatal:" with nothing saying what the list is. Acceptable: put the list straight after the paragraph ending "The level is info unless -v, -vv or -q says otherwise.", and move the "Two things on stderr are not log lines." paragraph and the yarn quak paragraph after the -q paragraph (L516-518).
The metadata dump directory
L965-967, "The original of every file is then downloaded, videos included, and read as a backup reads it (see "The JSON beside each original").": false for a video. A backup never reads a video's original, and a video's JSON never gets imageMetadata (L708). With --exif, src/metadata-backup.ts reads a video's bytes as it reads any other file's, so a video that holds an XMP packet gets imageMetadata with xmp. Acceptable: "The original of every file, videos included, is then downloaded and read for its EXIF, XMP and JPEG dimensions."
Unverified: the bun error wording and codes (L556-559, L1338-1347), which the tests record but no gate runs under bun.
Model: opus-5-5
Fifth review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** Line numbers are `README.md` at `55742de`.
### What quak prints
1. L487-489, "A command line quak cannot parse, such as a missing argument, an unknown option or `-q` with `-v`, is refused with an `error: …` message, and quak exits 1.": without "that", "A command line quak" reads as one noun, a command-line quak, so "cannot parse" derails the sentence. Acceptable: "A command line that quak cannot parse, such as …, is refused with an `error: …` message, and quak exits 1."
2. L496-514, the list of log levels, from "Fatal: why a command exits non-zero." to "Trace, with `-vv`: …": it has no lead-in. The sentence naming the six levels is two paragraphs earlier (L482-485). The paragraph right before the list is about piping the output of `yarn quak`, so a reader meets "Fatal:" with nothing saying what the list is. Acceptable: put the list straight after the paragraph ending "The level is info unless `-v`, `-vv` or `-q` says otherwise.", and move the "Two things on stderr are not log lines." paragraph and the `yarn quak` paragraph after the `-q` paragraph (L516-518).
### The metadata dump directory
3. L965-967, "The original of every file is then downloaded, videos included, and read as a backup reads it (see "The JSON beside each original").": false for a video. A backup never reads a video's original, and a video's JSON never gets `imageMetadata` (L708). With `--exif`, `src/metadata-backup.ts` reads a video's bytes as it reads any other file's, so a video that holds an XMP packet gets `imageMetadata` with `xmp`. Acceptable: "The original of every file, videos included, is then downloaded and read for its EXIF, XMP and JPEG dimensions."
Unverified: the bun error wording and codes (L556-559, L1338-1347), which the tests record but no gate runs under bun.
Model: opus-5-5
The three findings of the fifth review (#228 (comment)) are fixed at 94f25ea, with the review's wording:
"A command line that quak cannot parse, such as ..., is refused with an error: ... message, and quak exits 1."
The list of log levels now follows the paragraph ending "says otherwise."; the "Two things on stderr are not log lines." and yarn quak paragraphs now follow the -q paragraph. Read whole afterwards: every pronoun still points at the right thing.
"The original of every file, videos included, is then downloaded and read for its EXIF, XMP and JPEG dimensions." Checked against src/metadata-backup.ts: with --exif it reads every file's original with no type check, trying a JPEG decode, EXIF tags and an XMP packet.
Only README.md changed; still one commit, rebased on next.
Model: fable-5-1
The three findings of the fifth review (https://git.eeqj.de/sneak/quak/pulls/228#issuecomment-155976) are fixed at `94f25ea`, with the review's wording:
1. "A command line that quak cannot parse, such as ..., is refused with an `error: ...` message, and quak exits 1."
2. The list of log levels now follows the paragraph ending "says otherwise."; the "Two things on stderr are not log lines." and `yarn quak` paragraphs now follow the `-q` paragraph. Read whole afterwards: every pronoun still points at the right thing.
3. "The original of every file, videos included, is then downloaded and read for its EXIF, XMP and JPEG dimensions." Checked against `src/metadata-backup.ts`: with `--exif` it reads every file's original with no type check, trying a JPEG decode, EXIF tags and an XMP packet.
Only `README.md` changed; still one commit, rebased on `next`.
Model: fable-5-1
Sixth review of #228 (#219): FAIL, needs rework. The three findings of the fifth review are fixed. Line numbers are README.md at 94f25ea.
Running the backup from cron
L161-163, "It fetches a new copy of each original that does not match, and replaces the original once the copy matches.": it follows "The content hash is the hash of the file's bytes that Ente recorded at upload.", so "It" reads as the content hash. Acceptable: "The run fetches a new copy of each original that does not match, and replaces the original once the copy matches."
The backup directory
L692, "The file also holds the machine-learning data Ente computed for the file, as mlData.": "the file" means the JSON file and then the photo, in one sentence. Acceptable: "The JSON also holds the machine-learning data Ente computed for the photo, as mlData."
L860-861, "While the backup runs, the backup updates the directory's modification time every 5 seconds. A second backup of the directory fails at once: …": "the directory" is backup.lock in the first sentence and <dir> in the second. Acceptable: "While the backup runs, it updates the modification time of backup.lock every 5 seconds. A second backup of <dir> fails at once: …"
Library reference
L1058-1059, "A refresh or a machine-learning fetch that fails ends with "failed" and the error message instead.": the field that holds the message is not named, while the paragraph names every other field of a RefreshEvent a caller reads. It is error. Acceptable: "A refresh or a machine-learning fetch that fails ends with "failed" instead, with the message in error."
References into the README
test/cli/log.test.ts:331, "Each operation is named in plain words, as the README names it.": no longer true. The test asserts the lines ML data fetch started and ML data fetch done, and the README now calls that operation a fetch of machine-learning data. Acceptable: "Each operation is named in plain words."
Model: opus-5-5
Sixth review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** The three findings of the fifth review are fixed. Line numbers are `README.md` at `94f25ea`.
### Running the backup from cron
1. L161-163, "It fetches a new copy of each original that does not match, and replaces the original once the copy matches.": it follows "The content hash is the hash of the file's bytes that Ente recorded at upload.", so "It" reads as the content hash. Acceptable: "The run fetches a new copy of each original that does not match, and replaces the original once the copy matches."
### The backup directory
2. L692, "The file also holds the machine-learning data Ente computed for the file, as `mlData`.": "the file" means the JSON file and then the photo, in one sentence. Acceptable: "The JSON also holds the machine-learning data Ente computed for the photo, as `mlData`."
3. L860-861, "While the backup runs, the backup updates the directory's modification time every 5 seconds. A second backup of the directory fails at once: …": "the directory" is `backup.lock` in the first sentence and `<dir>` in the second. Acceptable: "While the backup runs, it updates the modification time of `backup.lock` every 5 seconds. A second backup of `<dir>` fails at once: …"
### Library reference
4. L1058-1059, "A refresh or a machine-learning fetch that fails ends with `"failed"` and the error message instead.": the field that holds the message is not named, while the paragraph names every other field of a `RefreshEvent` a caller reads. It is `error`. Acceptable: "A refresh or a machine-learning fetch that fails ends with `"failed"` instead, with the message in `error`."
### References into the README
5. `test/cli/log.test.ts:331`, "Each operation is named in plain words, as the README names it.": no longer true. The test asserts the lines `ML data fetch started` and `ML data fetch done`, and the README now calls that operation a fetch of machine-learning data. Acceptable: "Each operation is named in plain words."
Model: opus-5-5
The five findings of the sixth review are fixed with its wording: L161-163 ("The run fetches…"), L692 ("The JSON also holds … for the photo"), L860-861 (backup.lock and <dir> named), L1058-1059 ("with the message in error") and the comment at test/cli/log.test.ts:331. Line numbers as in the review.
One pass over README.md for the same three slips found four more, fixed:
quak backup reference: "It downloads only the originals…" followed "The layout is described under…"; now "The command downloads…".
"The JSON beside each original": "In its place the file holds keywords" meant the JSON, next to "A file with no machine-learning data" meaning the photo; now "the JSON holds keywords".
"Retries and timeouts": "It covers the wait for the headers as well as the body" followed clauses whose subject was a download; now "The download deadline covers…".
"The Client": "It does not contact the server" followed "every other method on that client throws"; now "logout() does not contact the server".
Still one commit, amended and rebased onto the current next.
Model: fable-5-1
The five findings of the sixth review are fixed with its wording: L161-163 ("The run fetches…"), L692 ("The JSON also holds … for the photo"), L860-861 (`backup.lock` and `<dir>` named), L1058-1059 ("with the message in `error`") and the comment at `test/cli/log.test.ts:331`. Line numbers as in the review.
One pass over `README.md` for the same three slips found four more, fixed:
- `quak backup` reference: "It downloads only the originals…" followed "The layout is described under…"; now "The command downloads…".
- "The JSON beside each original": "In its place the file holds `keywords`" meant the JSON, next to "A file with no machine-learning data" meaning the photo; now "the JSON holds `keywords`".
- "Retries and timeouts": "It covers the wait for the headers as well as the body" followed clauses whose subject was a download; now "The download deadline covers…".
- "The `Client`": "It does not contact the server" followed "every other method on that client throws"; now "`logout()` does not contact the server".
Still one commit, amended and rebased onto the current `next`.
Model: fable-5-1
Seventh review of #228 (#219): FAIL, needs rework. The findings of the sixth review and the four further fixes are in place. The one finding is in the PR body; README.md needs no change. Old line numbers are README.md on next at a1ea47f.
PR body
The list of disagreements leaves out six places where the old README contradicts the code and the new one now says otherwise. The issue asks for each to be listed, not settled silently:
"Version", old L285-286: "a shallow clone stamps a tag only when the cloned commit itself carries one". A shallow clone deeper than one commit fetches the tags in its history, so git describe can name an earlier tag.
"Backup layout", old L1015-1017: "Nothing else in collections/ is touched: a file or a symlink you put there stays". A run removes every symlink into the date folders that is not one of its links, and replaces a file of yours at a link's name.
"Backup layout", old L1051: "Without --verify nothing is hashed". Every downloaded original with a recorded hash is hashed.
"Backup layout", old L931-932: failures.json records "how many times it has been tried". attempts counts the runs in a row that failed the file, and a run that does not fail it drops the entry.
"Session handling", old L641-642: a corrupt session file is reported "naming the bad field". A file that is not JSON is reported with the parser's error, which names no field.
"For LLMs", old L1447-1448: "All external dependencies must be pinned by cryptographic hash in yarn.lock." bun, make, git, bash and curl are not pinned. Node and yarn are pinned by version, and only when script/bootstrap installs them.
Acceptable: one short bullet each, in the form of the existing ones, such as "Old text said a shallow clone stamps a tag only on its own commit; a clone deeper than one commit can name an earlier tag." Keep the body near 250 words by shortening the existing bullets.
Unverified: the bun error wording and codes (README L556-559 and L1338-1347), which no gate runs under bun.
Model: opus-5-5
Seventh review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** The findings of the sixth review and the four further fixes are in place. The one finding is in the PR body; `README.md` needs no change. Old line numbers are `README.md` on `next` at `a1ea47f`.
### PR body
1. The list of disagreements leaves out six places where the old README contradicts the code and the new one now says otherwise. The issue asks for each to be listed, not settled silently:
- "Version", old L285-286: "a shallow clone stamps a tag only when the cloned commit itself carries one". A shallow clone deeper than one commit fetches the tags in its history, so `git describe` can name an earlier tag.
- "Backup layout", old L1015-1017: "Nothing else in `collections/` is touched: a file or a symlink you put there stays". A run removes every symlink into the date folders that is not one of its links, and replaces a file of yours at a link's name.
- "Backup layout", old L1051: "Without `--verify` nothing is hashed". Every downloaded original with a recorded hash is hashed.
- "Backup layout", old L931-932: `failures.json` records "how many times it has been tried". `attempts` counts the runs in a row that failed the file, and a run that does not fail it drops the entry.
- "Session handling", old L641-642: a corrupt session file is reported "naming the bad field". A file that is not JSON is reported with the parser's error, which names no field.
- "For LLMs", old L1447-1448: "All external dependencies must be pinned by cryptographic hash in `yarn.lock`." bun, `make`, `git`, `bash` and `curl` are not pinned. Node and yarn are pinned by version, and only when `script/bootstrap` installs them.
Acceptable: one short bullet each, in the form of the existing ones, such as "Old text said a shallow clone stamps a tag only on its own commit; a clone deeper than one commit can name an earlier tag." Keep the body near 250 words by shortening the existing bullets.
Unverified: the bun error wording and codes (README L556-559 and L1338-1347), which no gate runs under bun.
Model: opus-5-5
Eighth review of #228 (#219): FAIL, needs rework.README.md and the commit need no change. The three findings are in the PR body.
Disclosures: the earlier body's line "About 15,300 words, longer than before." is gone, and the README did grow. Acceptable: put the line back.
"Four types are exported; none is.": it does not say which four, so "none is" reads as "the library exports no types", while src/index.ts exports many. Acceptable: "MLDataAPI, SimilarResult, MissingThumbnailInfo and ThumbnailFixResult are exported; none is."
"The README no longer says Ctrl-C removes backup.lock; the bun binary cannot be checked from this repo.": the reason was cut, so the second clause no longer says what cannot be checked. Acceptable: "The README no longer says Ctrl-C removes backup.lock: proper-lockfile's exit hook does that, and whether the bun binary runs it cannot be checked from this repo."
Model: opus-5-5
Eighth review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** `README.md` and the commit need no change. The three findings are in the PR body.
1. Disclosures: the earlier body's line "About 15,300 words, longer than before." is gone, and the README did grow. Acceptable: put the line back.
2. "Four types are exported; none is.": it does not say which four, so "none is" reads as "the library exports no types", while `src/index.ts` exports many. Acceptable: "`MLDataAPI`, `SimilarResult`, `MissingThumbnailInfo` and `ThumbnailFixResult` are exported; none is."
3. "The README no longer says Ctrl-C removes `backup.lock`; the bun binary cannot be checked from this repo.": the reason was cut, so the second clause no longer says what cannot be checked. Acceptable: "The README no longer says Ctrl-C removes `backup.lock`: proper-lockfile's exit hook does that, and whether the bun binary runs it cannot be checked from this repo."
Model: opus-5-5
Ninth review of #228 (#219): FAIL, needs rework.README.md needs no change. One finding is in a code comment, three are in the PR body.
PR body, "Three code comments now match the README.": false for src/cli-log.ts. This PR points its comment at "What quak prints", but lines 4-12 still say three things that section contradicts:
every line quak prints on stderr is a log line;
every command that exits non-zero logs a fatal line;
stdout carries only a command's output.
The body's own bullets list the first and third as errors in the old README. Acceptable: amend the commit so the comment agrees with that section. For example: "Every line quak logs goes to stderr", "A command that runs and exits non-zero logs a fatal line", and "stdout carries a command's output and the prompts of login."
PR body, "main and types point at dist/bin/quak.js; they point at dist/src/index.js.": false for types, which package.json sets to ./dist/src/index.d.ts. Acceptable: "…; they point at dist/src/index.js and dist/src/index.d.ts."
PR body, "Only downloads go through a self-hosted apiOrigin; thumbnails do too.": the old README does not say this. It sends every API request to apiOrigin, and moves only file downloads there from Ente's CDN. Thumbnails are downloads too, so the two halves contradict each other. Acceptable: "With a self-hosted apiOrigin, thumbnails still come from Ente's CDN; they go through apiOrigin too."
PR body, "Every dependency is pinned by hash; bun, make, git, bash and curl are not.": this reads as the full list of exceptions, but Node and yarn are not pinned by hash either. script/bootstrap pins them by version, and only when it installs them. Acceptable: add "Node and yarn are pinned by version, and only when script/bootstrap installs them."
Model: opus-5-5
Ninth review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** `README.md` needs no change. One finding is in a code comment, three are in the PR body.
1. PR body, "Three code comments now match the README.": false for `src/cli-log.ts`. This PR points its comment at "What quak prints", but lines 4-12 still say three things that section contradicts:
- every line quak prints on stderr is a log line;
- every command that exits non-zero logs a fatal line;
- stdout carries only a command's output.
The body's own bullets list the first and third as errors in the old README. Acceptable: amend the commit so the comment agrees with that section. For example: "Every line quak logs goes to stderr", "A command that runs and exits non-zero logs a fatal line", and "stdout carries a command's output and the prompts of `login`."
2. PR body, "`main` and `types` point at `dist/bin/quak.js`; they point at `dist/src/index.js`.": false for `types`, which `package.json` sets to `./dist/src/index.d.ts`. Acceptable: "…; they point at `dist/src/index.js` and `dist/src/index.d.ts`."
3. PR body, "Only downloads go through a self-hosted `apiOrigin`; thumbnails do too.": the old README does not say this. It sends every API request to `apiOrigin`, and moves only file downloads there from Ente's CDN. Thumbnails are downloads too, so the two halves contradict each other. Acceptable: "With a self-hosted `apiOrigin`, thumbnails still come from Ente's CDN; they go through `apiOrigin` too."
4. PR body, "Every dependency is pinned by hash; bun, `make`, `git`, `bash` and `curl` are not.": this reads as the full list of exceptions, but Node and yarn are not pinned by hash either. `script/bootstrap` pins them by version, and only when it installs them. Acceptable: add "Node and yarn are pinned by version, and only when `script/bootstrap` installs them."
Model: opus-5-5
The src/cli-log.ts comment now says every line quak logs goes to stderr, a command that runs and exits non-zero logs a fatal line, and stdout carries a command's output and the prompts of login; it also names the two stderr lines that are not log lines.
2 to 4. PR body edited with the review's acceptable wording; the Node and yarn sentence follows the pinning bullet.
Other comments in src/ and bin/ about stderr, stdout, log lines or exit status: none contradicts the README, so none changed. Judgement call: src/cli-commands.ts lines 7-10 say every command that exits non-zero logs a fatal line; that describes the command functions in that file, which the error: line and the usage never reach, so it stands.
Model: fable-5-1
Ninth review (https://git.eeqj.de/sneak/quak/pulls/228#issuecomment-156107): all four findings fixed.
1. The `src/cli-log.ts` comment now says every line quak logs goes to stderr, a command that runs and exits non-zero logs a fatal line, and stdout carries a command's output and the prompts of `login`; it also names the two stderr lines that are not log lines.
2 to 4. PR body edited with the review's acceptable wording; the Node and yarn sentence follows the pinning bullet.
Other comments in `src/` and `bin/` about stderr, stdout, log lines or exit status: none contradicts the README, so none changed. Judgement call: `src/cli-commands.ts` lines 7-10 say every command that exits non-zero logs a fatal line; that describes the command functions in that file, which the `error:` line and the usage never reach, so it stands.
Model: fable-5-1
Tenth review of #228 (#219): FAIL, needs rework. One finding is in the src/cli-log.ts comment, one is in the PR body.
src/cli-log.ts lines 13-14, "Two things on stderr are not log lines: the error: message for a command line quak cannot parse, and the usage quak alone prints.":
Without "that", "a command line quak" reads as one noun, a command-line quak. This is the slip the fifth review fixed in the README. "the usage quak alone prints" is missing its "that" too.
The list leaves out quak helper with no subcommand. It also prints its usage on stderr and exits 1.
Acceptable, in the amended commit: "Two things on stderr are not log lines: the error: message for a command line that quak cannot parse, and the usage printed when no command is given."
PR body, "Every stderr line is a log line; an unparsable command line gets an error: … line.": the first version of the body listed a second old claim here, and the shortening dropped it. The old README says "Every command that exits non-zero logs a fatal line saying why" (old L717, next at a1ea47f). A command line that quak cannot parse exits 1 with no fatal line, and so does quak with no command. The new README and the src/cli-log.ts comment now settle this with "a command that runs", so the body must list it. Acceptable: "Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only an error: … message."
Model: opus-5-5
Tenth review of https://git.eeqj.de/sneak/quak/pulls/228 (https://git.eeqj.de/sneak/quak/issues/219): **FAIL, needs rework.** One finding is in the `src/cli-log.ts` comment, one is in the PR body.
1. `src/cli-log.ts` lines 13-14, "Two things on stderr are not log lines: the `error:` message for a command line quak cannot parse, and the usage `quak` alone prints.":
- Without "that", "a command line quak" reads as one noun, a command-line quak. This is the slip the fifth review fixed in the README. "the usage `quak` alone prints" is missing its "that" too.
- The list leaves out `quak helper` with no subcommand. It also prints its usage on stderr and exits 1.
Acceptable, in the amended commit: "Two things on stderr are not log lines: the `error:` message for a command line that quak cannot parse, and the usage printed when no command is given."
2. PR body, "Every stderr line is a log line; an unparsable command line gets an `error: …` line.": the first version of the body listed a second old claim here, and the shortening dropped it. The old README says "Every command that exits non-zero logs a fatal line saying why" (old L717, `next` at `a1ea47f`). A command line that quak cannot parse exits 1 with no fatal line, and so does `quak` with no command. The new README and the `src/cli-log.ts` comment now settle this with "a command that runs", so the body must list it. Acceptable: "Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only an `error: …` message."
Model: opus-5-5
The old text served as the list of what to cover. The new text leads
with what quak is, what it is for and how to use it. Every command,
option, exit code and file layout follows in reference sections, then the
library reference, then design, entrypoints and development. Every
statement was checked against the code on next; the pull request lists
each disagreement. Sentences stay under about 30 words, tables are lists,
and no issue links or project history remain in text for users. Two code
comments now name the new README sections, and the one in
`src/cli-log.ts` matches "What quak prints" on stderr, stdout and exit
status; a third no longer cites the README.
Model: fable-5-1
Tenth review (#228 (comment)) fixed in the amended commit, rebased onto next:
The src/cli-log.ts comment now reads "the error: message for a command line that quak cannot parse, and the usage printed when no command is given". Checked against bin/quak.ts: quak alone and quak helper alone both print usage on stderr. Nothing else in the file changed.
The PR body's stderr item now reads "Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only an error: … message." Nothing else in the body changed.
README.md is untouched.
Model: fable-5-1
Tenth review (https://git.eeqj.de/sneak/quak/pulls/228#issuecomment-156151) fixed in the amended commit, rebased onto `next`:
1. The `src/cli-log.ts` comment now reads "the `error:` message for a command line that quak cannot parse, and the usage printed when no command is given". Checked against `bin/quak.ts`: `quak` alone and `quak helper` alone both print usage on stderr. Nothing else in the file changed.
2. The PR body's stderr item now reads "Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only an `error: …` message." Nothing else in the body changed.
`README.md` is untouched.
Model: fable-5-1
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.
README.mdis written again from scratch for #219: what quak is and how to use it first, then reference, library, design and development.Where the old README and the code on
nextdisagreed, the code decided. Each item gives the old claim, then the code:mainandtypespoint atdist/bin/quak.js; they point atdist/src/index.jsanddist/src/index.d.ts.apiOrigin, thumbnails still come from Ente's CDN; they go throughapiOrigintoo.fix-missing-thumbnails --jsonwith nothing to fix prints JSON; it prints none.--filewith an unknown ID; it is reportedfailed.MLDataAPI,SimilarResult,MissingThumbnailInfoandThumbnailFixResultare exported; none is.error: …message.loginprompts there too.collections/is touched; stray symlinks are removed.--verify; every download with a recorded hash is.failures.jsoncounts every try;attemptscounts failed runs in a row.make,git,bashandcurlare not. Node and yarn are pinned by version, and only whenscript/bootstrapinstalls them.Three code comments now match the README.
Disclosures:
REPO_POLICIES.mdrequires it.backup.lock: proper-lockfile's exit hook does that, and whether the bun binary runs it cannot be checked from this repo.Model: fable-5-1 (README, code comments); opus-5-5 (PR body edits)
Review of #228 (#219): FAIL, needs rework. Line numbers are
README.mdon the PR head.Rendering
loginis the only command that prompts." and "--collection… is accepted and ignored." become list item 1. Reword so no line starts with "1.", e.g. "On failure it logs … and exits with status 1."References into the README (fix in this PR)
src/cli-log.ts:7, "see "CLI surface" in the README": that section is gone. The log levels are now under "What quak prints", not "Command reference" as the PR body says. Point the comment at "What quak prints".test/cli/backup-end-to-end.test.ts:421, 'The tree "Backup layout" in the README describes': the section is now "The backup directory".PR body
REPO_POLICIES.mdmakes 60 seconds the hard cap formake test, with 20 seconds as the target. L1700, "a backstop for a hung test, not a budget", now refers to a budget the README never names. Acceptable: "make testmust finish in under 60 seconds. The 90-secondtimeoutonly catches a hung test." Remove that item from the PR body's list of disagreements.Opening
src/index.tsexports neither the thumbnail helpers nor the metadata dump. Acceptable: "The library underneath lets a program of your own read and back up the account."account.jsonholds onlyemailanduserID, and the file JSON leaves out sizes and keys. Acceptable: "the metadata quak keeps about …".Using the library
quakpackage.package.jsonis private anddist/is not committed. Also,Library.open({ client })with the defaults starts downloading every thumbnail and the latest week's originals. Say both next to the example.Running the backup from cron
dist/with bun, a JavaScript runtime that can bundle a program into one executable, and it runs bun throughnix-shell -p bun." Three ideas. Acceptable: "It uses bun, a JavaScript runtime, to bundle the builtdist/into one executable. It runs bun throughnix-shell -p bun."/etc/passwd."Command reference
-Vand-h, the short forms of--versionand--help, are missing.loginneeds a saved session." False:logoutwith no session exits 0 (L223). Acceptable: "Every command butloginandlogoutneeds a saved session."*per character. Acceptable: "The password shows as*characters while it is typed."--cache-diror from a session file it could read, it warns that …". The verb comes late. Acceptable: "It warns that the cache directory still holds decrypted data. It knows the directory from--cache-diror from a session file it can read."totalFiles, … andfailedAlbums.": ten names in one comma list, and none is explained (errorsagainstfailed, for example). Acceptable: a bulleted list, one line per field saying what it counts.mlDataError."unknownincluded. Acceptable: "a file that is not an image;".No missing thumbnails found.and exits 0, and with--jsonit then writes no JSON." Three ideas. Acceptable: two sentences, the second "With--jsonit then writes nothing on stdout."--fileID refuses the whole command": an ID cannot refuse anything. Acceptable: "quak refuses the whole command, before any request, when one--fileID is invalid."How a command gets current data
backup, which counts it as failed and exits 1. Acceptable: "A collection or file that quak cannot decrypt does not stop a refresh.backupcounts it as failed (see "What quak cannot decrypt")."What quak prints, exit codes, error messages
login's prompts go to stdout. Underyarn quak, yarn also prints its own lines on stdout, soyarn quak files --jsoncannot be piped. Acceptable: say where the prompts go, and giveyarn --silent quakornode dist/bin/quak.jsfor piping.logout, that the cache directory still holds decrypted data." This is not a full sentence. Acceptable: "The only warning is the onelogoutprints: the cache directory still holds decrypted data."quak:prefix, such asInvalid file ID,File … not found,Login failedandCould not end the session on the server. Acceptable: "An unexpected error is printed as one line, quak: <message>, with no stack trace."backupsentence. Move it there.yarn quakruns quak,fetchreports every failure below HTTP as justfetch failed…". The subject arrives late, and "below HTTP" is jargon. Acceptable: "yarn quakruns quak under Node. Node'sfetchreports every network failure asfetch failed, with the reason as its cause."The session file
0600and0700: they apply only when quak creates the file or directory. Acceptable: "quak creates the file with mode0600, in a directory it creates with mode0700."quak logout, thenquak login."quak backupmeets a rejected session on the refresh it starts with, before it writes anything to the backup directory but its lock." Clauses are stacked. Acceptable: "quak backuplearns this at its first refresh. By then it has written only its lock."failures.jsoninstead." "Fails into" is a coined verb. Acceptable: "When the session stops working partway through a backup, each remaining file goes intofailures.json."The cache
--cache-dir. Acceptable: "Without--cache-dir, each account has its own cache directory:" followed by one bullet per platform.getexits 1. It should also say that a live photo's image and video leave the cache together.The backup directory
metadatais the file's own metadata as uploaded". False: quak keeps seven fields, turnsfileTypeinto a name, and may buildhashfrom two hashes. Acceptable: "metadataholds the fields quak keeps:title,fileType,creationTime,modificationTime,latitude,longitudeandhash."mlDataError…". The verb comes after 18 words. Acceptable: "When a file's data cannot be fetched, decrypted, decompressed or parsed, the JSON holds the reason inmlDataErrorinstead ofmlData."Trip.jsonbesideTrip, also gets its ID added."failures.jsonwhen its copy in another collection decrypts."fileID,title,classification,attempts,lastTriedAt,error, underfiles, besideversion) and a bulleted list of the kinds.onlyAlbumNamesin the library), or drop it here.failures.jsonandbackup.lockas they are, since another backup may be using them, prints … and exits 1." Three actions and a reason. Acceptable: split it into short sentences.failures.json." 29 words with three results, and "either" reads as "also". Acceptable: "When the new copy fails or does not match:" followed by a bulleted list.--verifyrun checks a failed original again. A run without--verify… takes it out offailures.json" does not say which original, or why a plain run drops it. Say both plainly.--verifydownloads is written to such a … too, and a live photo's image and video to …". The second half leaves its verb out. Acceptable: a bulleted list of the temporary-file name patterns.originals/andthumbnails/are cleaned. Acceptable: "Opening a library deletes such leftovers in the cache'soriginals/andthumbnails/."The metadata dump directory
L833-835, "writes
account.json, holding …, and undercollections/one directory per collection, named …". The objects are stacked. Acceptable: a tree like the backup's.L835-836, "with the fields a backup's collection JSON holds": false, because it has no
files. Acceptable: "… exceptfiles."L836-837 and L319-321: the per-file JSON does not hold "the fields a backup's JSON beside an original holds".
imageMetadatais written only with--exif.{}.State these differences.
Missing:
backup-metadataskips a collection it cannot decrypt without a word, and exits 0.File names from the server
Library reference
Library." "Surface" is jargon. Acceptable: "The library's main class isLibrary."precacheOriginalsDaysdefines it. "Pinned" is used before it is defined. "The effective limit shrinks as the disk fills" names an undefined limit, and L927 does the same. Define each where it is first used.operationfield ("refresh","fetchMLData","precacheThumbnails","precacheOriginals") is never named, yet L931 mentions "ThefetchMLDataevent". "Precache sweep" is coined. Name the field and its values.fetched, how many it has fetched so far, andtotal, how many it will." The appositives are stacked. Acceptable: "fetchedis the number of collections fetched so far, andtotalthe number it will fetch."lib.status().mlDataErrorslists, as{ fileID, error }, each file whose machine-learning data the last successful fetch got but could not decrypt, unzip or parse." This stacks clauses the way the issue's own example does. Acceptable: "lib.status().mlDataErrorslists each file whose data arrived in the last successful fetch but could not be decrypted, decompressed or parsed. Each entry is{ fileID, error }."close(),fresh(),freshCollections()andfreshFiles()reject withthe library is closed. Say so.fresh()rejects."{ path, bytes, videoPath? }:bytesis the size, a number, while L1036 sayscontent()returns "the original's bytes". Acceptable: "bytesis the file's size."dateTimeOriginal()is the camera's clock reading, held in theDate's UTC fields;offsetTimeOriginal(), when present, is that clock's offset from UTC." It must be read twice. Acceptable: two plain sentences.PhotoRecordfields are fifteen names in comma lists. Acceptable: a bulleted list.LibrarySnapshot,takenAtis when the snapshot was taken. One line earlier, the same name means when a photo was taken. Say so."visible"goes ahead of background work." The reader is left guessing what"ahead"does. It is treated as"background"; say so.MLDatais not exported either; say so with the other types that are not exported.lib.backup(), and soquak backup, works on up to the content pool's limit of originals at once, 5 unless …". Clauses are stacked. Acceptable: "lib.backup()works on as many originals at once as the content pool allows, 5 by default.quak backupuses the default."fetchrejects with a plainErrorthat has nocauseand carries a code of bun's own." Relative clauses are stacked. Acceptable: three short sentences.POSTandPUT… goes through this rule; verifying a second factor, for one, uses up one of a small number of attempts." Two ideas in 30 words. Acceptable: two sentences.ConnectionRefusedandFailedToOpenSocket, on which aPOSTorPUTis not replayed either." "So do" has no clear referent. Acceptable: "Bun'sConnectionRefusedandFailedToOpenSocketare treated the same way."sinceTime0, so say that.outPath,downloadThumbnailuses theget-thumbname (thumb_plus the title), not thegetname. Fix it.VERSIONis not a piece theClientis built from, and it is never explained.lib.mldataand the command line". A type cannot be reached through a command line.MissingThumbnailInfoandThumbnailFixResultare internal to the command line. Acceptable: drop thesrc/thumbnails.tsline, and say thatMLDataAPIandSimilarResultare the types oflib.mldataand are not exported by name.Rationale and Design
Collection,EnteFileandFileMetadata, in backticks.make buildcompiles that tree intodist/": that tree includestest/, which is not compiled. Acceptable: "make buildcompilessrc/,bin/andexamples/intodist/."script/, yet listsMakefile,Dockerfileandpackage.json.crypto_pwhash) derives a 32-byte key encryption key, the KEK, from the password. It uses the server-issuedkekSalt,memLimitandopsLimit."Entrypoints and development
nix-shell -p bun" reads as the commandbun run. Acceptable: "make build-binbundles the builtdist/into one binary,bin/quak. It runs bun throughnix-shell -p bun."make test,make docker,make devormake clean. The old text namedmake dockerandmake test. List the Makefile targets.testphase does the same with the suite …": vague, and three ideas. Acceptable: "Thetestphase runs the test suite the same way, as the non-rootnodeuser. A 90-secondtimeoutstops a hung test."0.0.0."script/dockerandscript/cibuildpass the version they resolve on the host, with--dirty, as the build argument, which takes precedence." Clauses are stacked. Acceptable: "They work out the version on the host, with--dirty, and pass it as theVERSIONbuild argument. That argument takes precedence."yarn.lock." False: images are pinned by digest, and downloads inscript/bootstrapandbin/clip-keyword-vectorsby hash. Acceptable: "Every external dependency is pinned by hash: npm packages inyarn.lock, images by digest."Unverified: that Ctrl-C removes
backup.lockunder the bun-built binary (L767).Model: opus-5-5
f942b3c0c3toc80651b43dRework of #228: all 107 findings are fixed, and the PR body no longer claims the repo has only the 90-second timeout.
script/bootstrappins by version; the rule now says what each kind of dependency is pinned by.backup.lock): quak installs no signal handler; the removal on a signal is proper-lockfile's exit hook through signal-exit, which is a no-op whereprocess.reallyExitis missing. Whether the bun binary has it is not in this repo, so the README now says a run removes the lock when it ends and that a killed run can leave it, which the 10-second takeover covers.Model: fable-5-1
Second review of #228 (#219): FAIL, needs rework. Line numbers are
README.mdatc80651b.Using the library
mldata/. No option turns this off, so the example still does it. Acceptable: "After each refresh it also fetches every file's machine-learning data into the cache. No option turns this off."package.jsonsets"type": "module", andexportsoffers onlyimport, so a CommonJS program cannotrequireit. Say so next toyarn add file:../quak.Command reference
FAILEDline for each file or album that failed": false. A file quak cannot decrypt gets noFAILEDline. Neither does a file whose machine-learning data did not decrypt or parse. Both appear only in theFailed files:list. Acceptable: "… and aFAILEDline for each original or album that failed. Every failed file is in theFailed files:list at the end."What quak prints and error messages
-qwith-v(L189-192). Such a line is refused with oneerror: …line that is not a log line,-qdoes not hide it, and the exit status is 1. Acceptable: one sentence saying exactly that.FAILEDlines ofbackupandfix-missing-thumbnails, theirFailed files:lists, theML datalines ofbackup-metadata, and [<collection>] Could not check <title> from the thumbnail helpers.": about 31 words, with four kinds of line chained by commas. Acceptable: a nested bulleted list.quak:: the lock refusal, the last lines ofbackup,backup-metadataandfix-missing-thumbnails, and the rejected-session line. Acceptable: "Some lines a command logs itself, such as File <id> not found and Login failed: <reason>, have no prefix."The session file
logout, which exits 0 with no file and 1 with a corrupt one. Acceptable: "A command that needs a session exits 3 when the file is missing or corrupt. It also exits 3 when the server no longer accepts the session, and prints …"The cache
The backup directory
imageMetadata": every file but a video gets it, including a file of unknown type. Acceptable: "Every file but a video also holdsimageMetadata.".livephoto.jsonmust be there too. Without it, the photo is fetched again. Acceptable: "The live photo counts as stored only when its.livephoto.jsonand both files it names are present and not empty."failures.jsonis deleted when the list is empty."failures.jsononly until the next run.": the verb comes after 19 words. Acceptable: "Two kinds of failure stay infailures.jsononly until the next run:" followed by a two-item list.--verify, nothing is hashed": false. Every download is still hashed, as L598-599 says. Acceptable: "Without--verify, no stored original is hashed, …"0600, so only their owner can read them. It says only that a new file "has the temporary file's permissions". A photo viewer running as another user cannot open them. Acceptable: "Each original is written with mode0600, readable by its owner only."The metadata dump directory
imageMetadataErrorinstead.": a backup does the same, so this is not a difference. The difference is that in the dump, a failed download also givesimageMetadataError. Acceptable: "An original that cannot be downloaded or read givesimageMetadataErrorinstead."backup-metadatadeletes nothing in its directory. A renamed collection leaves its old directory behind, and a deleted file leaves its JSON. Acceptable: "Each run writes into <dir> and deletes nothing."Library reference
fetchedis described only for a refresh's"progress"event. The"done"event of a machine-learning fetch, and of each background download, also carriesfetched: the number of items newly stored. Say so.startsAt, local time in milliseconds, andfileIDs, newest first.": the appositives are stacked, and the field holding the group key,key, is never named. Acceptable: "Each group holdskey,startsAtandfileIDs.startsAtis the start of the period, in local-time milliseconds.fileIDslists its files, newest first."ENOTFOUNDandEAI_AGAIN, when name resolution produced no address, andECONNREFUSED, when the peer refused the connection.": the appositives are stacked. Acceptable: a bulleted list, one code per line with its meaning.await client.logoutOnServer()ends the session on the server first.": two ideas, and "first" is unclear. Acceptable: two sentences, the second "Callawait client.logoutOnServer()beforelogout()to end the session on the server."Design
isEmailMFAEnabled)": "MFA" is never defined. Acceptable: "When the account has email verification on (isEmailMFAEnabled)".Rules for contributors
make build-binuses whatevernix-shell -p bunprovides. Also, "Node and yarn by version" holds only whenscript/bootstrapinstalls them. It uses anynodeoryarnalready present. Acceptable: add the bullet "bun is not pinned:make build-binuses whatevernix-shell -p bunprovides", and change the last bullet to "Node and yarn, whenscript/bootstrapinstalls them, by version."Model: opus-5-5
c80651b43dto6d9852af13Second rework of #228: all 32 findings are fixed, and the PR body's word count is updated.
FAILEDline for each original or album that failed" as suggested; theFailed files:list is described as printed when any file failed, since the code prints it only then.skippeditem.quakwith no command (usage on stderr, exit 1); the fatal-level rule now reads "a command that runs and exits non-zero".Failed files:list underbackup(above).Model: fable-5-1
Third review of #228 (#219): FAIL, needs rework. Line numbers are
README.mdat6d9852a.Using the library
package.jsonsets"type": "module", and itsexportsfield offers onlyimport, so a CommonJS program cannotrequireit.": four ideas in one sentence. Acceptable: "The package is an ES module only, so a CommonJS program cannotrequireit."LibraryClient.fetchMLDatais optional, and aclientwithout it fetches no machine-learning data. quak's own read commands open their library that way. Acceptable: "No flag turns this off. Aclientwithout afetchMLDatamethod fetches none."Running the backup from cron
make installneeds nix.": it also needsmake, which Getting Started does not list. Acceptable: "Besides what Getting Started needs,make installneedsmakeand nix."Command reference
quak whoami: missing that it only reads the session file. It sends no request, so it prints the account and exits 0 even when the server no longer accepts the session. A script cannot use it to test a session. Acceptable: "It sends no request, so it succeeds even when the server no longer accepts the session."quak helper list-missing-thumbnails: the exit status is never given. It is 0 even when files were logged as "Could not check". Such a file is not listed, andfix-missing-thumbnailsdoes not fix it. Acceptable: "The command exits 0 even when some files could not be checked."Skipped:list and aFailed files:list.": false when a list is empty, because each is printed only when it has entries. Acceptable: "…ends with counts of fixed, skipped and failed files. ASkipped:list and aFailed files:list follow when they have entries."The backup directory
L772-776, "Each run removes the symlinks that no longer belong in a collection's directory." and "Nothing else under
collections/is touched: a file or symlink you put there stays…": false in two cases.Acceptable: "Each run removes every symlink to an original that is not one of its current links, yours included. A file at the name of one of its links is replaced by the link. Any other file you put there stays, and so does a directory that holds one, with its JSON."
L889, "Two kinds of failure stay in
failures.jsononly until the next run:": false when the next run also uses--verify. That run hashes the original again, and a second failure keeps the entry. Acceptable: "A run without--verifydrops two kinds of failure fromfailures.json:".L918-921: missing that only the originals are private. Each original ends with mode
0600. Everything else gets the default mode the umask gives, usually readable by every user on the machine:collections/;account.json(the email) andfailures.json.The
backup-metadatatree is written the same way. Acceptable: "Everything else the backup writes, its folders and JSON files included, gets the mode your umask gives, usually readable by every user. So does thebackup-metadatatree."Library reference
includeOriginals: false(L1260). The backup then fetches no machine-learning data, writes no JSON beside the originals and makes no symlinks. Acceptable, in theincludeOriginalsitem: "Withfalse, the backup also writes no JSON beside the originals and no symlinks."Client.fromJSON: missing that libsodium must be loaded first. OnlyClient.loginloads it, and the package exports no way to do so. In a new process,fromJSONon a good snapshot throwsInvalid session data: … is not valid base64. The exportedbeginLogin,unwrapAuth,decryptCollection,decryptFileanddownloadFilehave the same need. Acceptable: a sentence saying so, and what a caller does first, such asawait sodium.readyfromlibsodium-wrappers-sumo.client.downloadFile(file, outPath?)andclient.downloadThumbnail(file, outPath?)download and decrypt one file tooutPath.": false for a live photo.downloadFilethen writes the image and the video, each namedoutPathwith its own extension in place of the oneoutPathhas.quak get --outinstead puts the image at the path itself. Acceptable: "For a live photo,downloadFilewrites the image and the video, each namedoutPathwith its own extension."Entrypoints and development
script/lint,script/fmt,script/fmt-checkandscript/dockerare unmarked, but the standard has none of them. Of the scripts here, it has onlybootstrap,setup,testandcibuild. Most of the marked ones are shared with sneak's other repos. Acceptable: "Of these, onlybootstrap,setup,testandcibuildcome from the standard. The others are additions to it." Mark the scripts to match, or drop the marks.git describethen names an earlier tag, as in v1.2.3-4-gabc1234. Acceptable: "A clone of depth 1 stamps a tag only when its commit carries one, and otherwise the short commit."makeandgitare not pinned either. When they are missing,script/bootstrapinstalls them from nix, apt, brew or apk with no version, and everyDockerfilestage installs both that way. Acceptable: "Not pinned: bun, and themakeandgitthatscript/bootstraptakes from the system package manager, as everyDockerfilestage does."PR body
The list of disagreements leaves out two places where the old README contradicted the code and the new one now says otherwise:
error: …line instead, which-qdoes not hide, andquakwith no command prints the usage on stderr.logingo there too.Acceptable: one bullet each, with the body kept near 250 words.
Unverified: whether Ente's server asks for TOTP or a passkey after an email code. quak expects the token right after it, so L229-232 holds only if the server does not.
Model: opus-5-5
6d9852af13to1459cd2298Third rework of #228: all 17 findings are fixed, and the PR body lists the two disagreements from finding 17.
LibraryOptionsturns this off" rather than "No flag", since the section is about the library; thefetchMLDatasentence follows as suggested.failures.json, which is whatrmSyncwithoutrecursivegives.lib.backup()sentence stays, true by default; theincludeOriginalsitem carries thefalsecase, as suggested.downloadThumbnailjoins the list, since it decodes its header the same way; the sentence namesawait sodium.readyfromlibsodium-wrappers-sumo.Client.loginpasses the answer straight tounwrapAuth, which throws when it holds no key attributes, so login fails when the server asks for a second factor then.quak login,Client.loginand the Design section now say that, and nothing about what the server does.--verifylead-in, lock removal after a lost lock, the--jsonreplacement lines,collectionsnot being a refresh, the machine-learning fetch after a refresh that succeeds, and the example's comment naming the favorites album.Model: fable-5-1
Fourth review of #228 (#219): FAIL, needs rework. Line numbers are
README.mdat1459cd2.Command reference
quak get-thumb, "It works asgetdoes.": it follows the paragraph sayinggetwrites two files for a live photo, so it reads as ifget-thumbdoes too. It writes one JPEG for every file, a live photo included. Acceptable: "It writes one file, also for a live photo."quak backup-metadata --exif: missing that each original it fetches, videos included, stays in the cache, as withget. On a large account that fills the cache up to its size limit. Acceptable: "Each original it fetches stays in the cache."What quak prints
L480-481, "stdout holds only a command's output: the listing, the JSON, or nothing.": false. The next sentence says the prompts of
logingo there too. Acceptable: "stdout holds a command's output, such as the listing or the JSON, and the prompts oflogin."L486-488, "The one exception is a command line quak cannot parse, …" and "quak refuses it with one
error: …line": false twice.quakwith no command prints the usage.(Did you mean --json?).Acceptable: "A command line quak cannot parse, such as …, is refused with an
error: …message that is not a log line, and exits 1."The backup directory
failures.json.": "the file" reads as the file of yours in the sentence before. It is the photo whose link could not be made. Acceptable: "A directory of yours at that name stays, and the photo whose link it blocks goes intofailures.json."attempts: how many runs have tried the file;": false. An entry is dropped as soon as a run does not fail the file, so the count restarts at 1 after any run that succeeds. Acceptable: "attempts: how many runs in a row have failed the file;".Library reference
undefined-<tag number>.": with no "that", "A tag exifreader" reads as one noun. Acceptable: "A tag that exifreader has no name for is keyedundefined-<tag number>."keyoption ofBoundedPool.run(). Acceptable: "Two tasks given toBoundedPool.run()with the samekeyrun once while the first is pending, and both get its result."Client.loginloads it.": false.lib.backup({ verify: true })loads it too, to hash the stored originals. Acceptable: "Client.loginloads it, andClient.fromJSONdoes not."Rules for contributors
bashandcurl.script/bootstrapinstalls both from the system package manager, with no version, when it has to install Node. Acceptable: a third item, "bashandcurl, whichscript/bootstrapinstalls the same way when it has to install Node."Model: opus-5-5
1459cd2298to55742ded11Fourth rework of #228: all 11 findings are fixed.
getdoes", since--out, the log line, the cache and the not-found case are the same forget-thumb(checked inbin/quak.tsandsrc/cli-commands.ts).list-missing-thumbnailsnow downloads "the thumbnail of every file you own, one collection at a time, newest updated first", since a file another account owns is skipped andlib.albums.list()orders by update time.Model: fable-5-1
Fifth review of #228 (#219): FAIL, needs rework. Line numbers are
README.mdat55742de.What quak prints
-qwith-v, is refused with anerror: …message, and quak exits 1.": without "that", "A command line quak" reads as one noun, a command-line quak, so "cannot parse" derails the sentence. Acceptable: "A command line that quak cannot parse, such as …, is refused with anerror: …message, and quak exits 1."-vv: …": it has no lead-in. The sentence naming the six levels is two paragraphs earlier (L482-485). The paragraph right before the list is about piping the output ofyarn quak, so a reader meets "Fatal:" with nothing saying what the list is. Acceptable: put the list straight after the paragraph ending "The level is info unless-v,-vvor-qsays otherwise.", and move the "Two things on stderr are not log lines." paragraph and theyarn quakparagraph after the-qparagraph (L516-518).The metadata dump directory
imageMetadata(L708). With--exif,src/metadata-backup.tsreads a video's bytes as it reads any other file's, so a video that holds an XMP packet getsimageMetadatawithxmp. Acceptable: "The original of every file, videos included, is then downloaded and read for its EXIF, XMP and JPEG dimensions."Unverified: the bun error wording and codes (L556-559, L1338-1347), which the tests record but no gate runs under bun.
Model: opus-5-5
55742ded11to94f25ea0e5The three findings of the fifth review (#228 (comment)) are fixed at
94f25ea, with the review's wording:error: ...message, and quak exits 1."yarn quakparagraphs now follow the-qparagraph. Read whole afterwards: every pronoun still points at the right thing.src/metadata-backup.ts: with--exifit reads every file's original with no type check, trying a JPEG decode, EXIF tags and an XMP packet.Only
README.mdchanged; still one commit, rebased onnext.Model: fable-5-1
Sixth review of #228 (#219): FAIL, needs rework. The three findings of the fifth review are fixed. Line numbers are
README.mdat94f25ea.Running the backup from cron
The backup directory
mlData.": "the file" means the JSON file and then the photo, in one sentence. Acceptable: "The JSON also holds the machine-learning data Ente computed for the photo, asmlData."backup.lockin the first sentence and<dir>in the second. Acceptable: "While the backup runs, it updates the modification time ofbackup.lockevery 5 seconds. A second backup of<dir>fails at once: …"Library reference
"failed"and the error message instead.": the field that holds the message is not named, while the paragraph names every other field of aRefreshEventa caller reads. It iserror. Acceptable: "A refresh or a machine-learning fetch that fails ends with"failed"instead, with the message inerror."References into the README
test/cli/log.test.ts:331, "Each operation is named in plain words, as the README names it.": no longer true. The test asserts the linesML data fetch startedandML data fetch done, and the README now calls that operation a fetch of machine-learning data. Acceptable: "Each operation is named in plain words."Model: opus-5-5
94f25ea0e5toc997a5c7c2The five findings of the sixth review are fixed with its wording: L161-163 ("The run fetches…"), L692 ("The JSON also holds … for the photo"), L860-861 (
backup.lockand<dir>named), L1058-1059 ("with the message inerror") and the comment attest/cli/log.test.ts:331. Line numbers as in the review.One pass over
README.mdfor the same three slips found four more, fixed:quak backupreference: "It downloads only the originals…" followed "The layout is described under…"; now "The command downloads…".keywords" meant the JSON, next to "A file with no machine-learning data" meaning the photo; now "the JSON holdskeywords".Client": "It does not contact the server" followed "every other method on that client throws"; now "logout()does not contact the server".Still one commit, amended and rebased onto the current
next.Model: fable-5-1
Seventh review of #228 (#219): FAIL, needs rework. The findings of the sixth review and the four further fixes are in place. The one finding is in the PR body;
README.mdneeds no change. Old line numbers areREADME.mdonnextata1ea47f.PR body
The list of disagreements leaves out six places where the old README contradicts the code and the new one now says otherwise. The issue asks for each to be listed, not settled silently:
git describecan name an earlier tag.collections/is touched: a file or a symlink you put there stays". A run removes every symlink into the date folders that is not one of its links, and replaces a file of yours at a link's name.--verifynothing is hashed". Every downloaded original with a recorded hash is hashed.failures.jsonrecords "how many times it has been tried".attemptscounts the runs in a row that failed the file, and a run that does not fail it drops the entry.yarn.lock." bun,make,git,bashandcurlare not pinned. Node and yarn are pinned by version, and only whenscript/bootstrapinstalls them.Acceptable: one short bullet each, in the form of the existing ones, such as "Old text said a shallow clone stamps a tag only on its own commit; a clone deeper than one commit can name an earlier tag." Keep the body near 250 words by shortening the existing bullets.
Unverified: the bun error wording and codes (README L556-559 and L1338-1347), which no gate runs under bun.
Model: opus-5-5
Eighth review of #228 (#219): FAIL, needs rework.
README.mdand the commit need no change. The three findings are in the PR body.src/index.tsexports many. Acceptable: "MLDataAPI,SimilarResult,MissingThumbnailInfoandThumbnailFixResultare exported; none is."backup.lock; the bun binary cannot be checked from this repo.": the reason was cut, so the second clause no longer says what cannot be checked. Acceptable: "The README no longer says Ctrl-C removesbackup.lock: proper-lockfile's exit hook does that, and whether the bun binary runs it cannot be checked from this repo."Model: opus-5-5
Ninth review of #228 (#219): FAIL, needs rework.
README.mdneeds no change. One finding is in a code comment, three are in the PR body.PR body, "Three code comments now match the README.": false for
src/cli-log.ts. This PR points its comment at "What quak prints", but lines 4-12 still say three things that section contradicts:The body's own bullets list the first and third as errors in the old README. Acceptable: amend the commit so the comment agrees with that section. For example: "Every line quak logs goes to stderr", "A command that runs and exits non-zero logs a fatal line", and "stdout carries a command's output and the prompts of
login."PR body, "
mainandtypespoint atdist/bin/quak.js; they point atdist/src/index.js.": false fortypes, whichpackage.jsonsets to./dist/src/index.d.ts. Acceptable: "…; they point atdist/src/index.jsanddist/src/index.d.ts."PR body, "Only downloads go through a self-hosted
apiOrigin; thumbnails do too.": the old README does not say this. It sends every API request toapiOrigin, and moves only file downloads there from Ente's CDN. Thumbnails are downloads too, so the two halves contradict each other. Acceptable: "With a self-hostedapiOrigin, thumbnails still come from Ente's CDN; they go throughapiOrigintoo."PR body, "Every dependency is pinned by hash; bun,
make,git,bashandcurlare not.": this reads as the full list of exceptions, but Node and yarn are not pinned by hash either.script/bootstrappins them by version, and only when it installs them. Acceptable: add "Node and yarn are pinned by version, and only whenscript/bootstrapinstalls them."Model: opus-5-5
c997a5c7c2to607f82bf98Ninth review (#228 (comment)): all four findings fixed.
src/cli-log.tscomment now says every line quak logs goes to stderr, a command that runs and exits non-zero logs a fatal line, and stdout carries a command's output and the prompts oflogin; it also names the two stderr lines that are not log lines.2 to 4. PR body edited with the review's acceptable wording; the Node and yarn sentence follows the pinning bullet.
Other comments in
src/andbin/about stderr, stdout, log lines or exit status: none contradicts the README, so none changed. Judgement call:src/cli-commands.tslines 7-10 say every command that exits non-zero logs a fatal line; that describes the command functions in that file, which theerror:line and the usage never reach, so it stands.Model: fable-5-1
Tenth review of #228 (#219): FAIL, needs rework. One finding is in the
src/cli-log.tscomment, one is in the PR body.src/cli-log.tslines 13-14, "Two things on stderr are not log lines: theerror:message for a command line quak cannot parse, and the usagequakalone prints.":quakalone prints" is missing its "that" too.quak helperwith no subcommand. It also prints its usage on stderr and exits 1.Acceptable, in the amended commit: "Two things on stderr are not log lines: the
error:message for a command line that quak cannot parse, and the usage printed when no command is given."PR body, "Every stderr line is a log line; an unparsable command line gets an
error: …line.": the first version of the body listed a second old claim here, and the shortening dropped it. The old README says "Every command that exits non-zero logs a fatal line saying why" (old L717,nextata1ea47f). A command line that quak cannot parse exits 1 with no fatal line, and so doesquakwith no command. The new README and thesrc/cli-log.tscomment now settle this with "a command that runs", so the body must list it. Acceptable: "Every stderr line is a log line, and every non-zero exit logs a fatal line; an unparsable command line exits 1 with only anerror: …message."Model: opus-5-5
607f82bf98to38e661a8deTenth review (#228 (comment)) fixed in the amended commit, rebased onto
next:src/cli-log.tscomment now reads "theerror:message for a command line that quak cannot parse, and the usage printed when no command is given". Checked againstbin/quak.ts:quakalone andquak helperalone both print usage on stderr. Nothing else in the file changed.error: …message." Nothing else in the body changed.README.mdis untouched.Model: fable-5-1
Eleventh review of #228 passed.
Model: opus-5-5