Close the backup mutation gate's thirteen survivors - #2102
Conversation
The gate on the backups PR finished after merge with 13 survivors across the files it changed. Seven were real test gaps, now pinned: - A dump did not prove it doubles single quotes, so broken escaping (and the corrupt restore it causes) went undetected. A settings row seeded with bound args now asserts the doubled quotes in the dumped SQL. - The keyset cursor could become a running sum of last row ids and only misbehave from the third page on, silently dropping rows. A five-row, three-page export now pins the exact row count. - createBackup's batched first pages could start their cursor at 1 and drop every table's first row. The listings dump now proves row one is included. - The BACKUP_PAGE_SIZE env key and the newline between dumped statements were unasserted; both are now pinned. - storageZoneName's ternary arms could swap unnoticed; unit tests now hold both directions (a zone name under Bunny, null under local). The other six are proven equivalents, recorded in the registry: two exportTable intermediates that are overwritten before any read, a ?? → || over an array-or-undefined operand, and the four backups-table column keys, which the page never reads back (same class as the questions table's recorded keys). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5kRwqm349kpsgkcQLeSpw
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: 0 reviews are currently available. Based on recent review activity, included reviews refill at 3 per hour. 📝 WalkthroughWalkthroughThe pull request adds regression tests for backup dump behavior and storage zone reporting. It also records equivalent mutations for backup export logic and backup table column keys. ChangesBackup validation
Storage zone validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR only adds test coverage and records proven equivalent mutations; it does not change production behavior, and no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
The verification run promised in the description has finished: Generated by Claude Code |
2 participants
Why
The mutation gate for #2101 finished after that PR merged (the run was interrupted by a container restart and re-run), scoring 97.2% with 13 survivors across the files it changed. Survivors on touched files are ours to close: each is either a real assertion gap or a proven equivalent. This PR does both — no production code changes, only tests and registry entries.
Seven real gaps, now pinned
escapeSql'' → ""): nothing proved a dumped value's single quotes are doubled — broken escaping would corrupt every restore of real-world text. A settings row seeded with bound args (so nothing escapes the quote on the way in) now asserts'O''Brien''s Gala'appears in the dumped SQL.cursor =→+=): a running-sum cursor agrees with the real one for the first two pages and only starts dropping rows from page three. A five-row, page-size-two export (three pages) now pinsrowCount5 and the last row's presence.tablePageStatement(table, 0, …)→1): a cursor starting at 1 silently drops every table's first row from every backup. The listings dump now proves row one is included.BACKUP_PAGE_SIZEenv key (name →""): the default page size is now asserted to actually read that variable.join("\n")→join("")): the newline between dumped statements is pinned.storageZoneNameternary arms swapped: unit tests now hold both directions — the zone name under Bunny storage, null under local. (The whole-file mutation re-run forstorage.tswas skipped as disproportionate for one two-line function; these two tests fail under any arm swap.)Six proven equivalents, recorded
exportTable'scolListinitializer and its= → +=assignment: the value is written in therowCount === 0branch before any read, that branch runs at most once, and"" + x === x.suppliedPage ?? …→||: the operand isBackupRow[] | undefined; arrays (even empty) are truthy andundefinedis falsy, so both operators agree on every possible value.keys: the page renders the table withoutcolumnKeys,hiddenKeys, or filters, so the keys are never read — the same class as the questions table's recorded keys.deno task check:equivalentsconfirms every new entry resolves to a real mutant. A targeteddeno task mutation src/shared/db/backup-snapshot.ts … --harnessre-run verifying all eight of that file's survivors is running; its score will be confirmed on this PR before merge.🤖 Generated with Claude Code
https://claude.ai/code/session_01A5kRwqm349kpsgkcQLeSpw
Generated by Claude Code
Summary by CodeRabbit
Tests
Mutation Testing