Reading window 1/3: app database and restore after a kill #15

Merged

schaefera merged 16 commits from phase1/reading-window into main 2026-10-20 21:19:44 +00:00

Conversation 1 Commits 16 Files changed 15 +3056 -69

claude-bot commented 2026-10-20 21:00:55 +00:00

Collaborator

• Copy link
• Copy Markdown

Part of #11.

What

The app now remembers where you were reading.

• The core saves the module, the top verse and the fraction of it scrolled into a new app.sqlite3: 1 s after a change, at once when the app goes inactive, and on exit.
• On start the app reopens that chapter at that verse, even after kill -9.
• A corrupt file is set aside (.corrupt-<time>) and a fresh one is started, with a notice. Each migration is backed up first (VACUUM INTO .bak-v<N>, the last 2 are kept).

Blast radius

Tier: trunk
Trunk paths touched: core/src/db/ (new, migration 001), core/src/reader.rs, core/src/worker.rs, qt/ glue (reading_model.rs), qml/pages/ReadingPane.qml, Cargo.toml + both lockfiles (rusqlite 0.40.2 bundled, tempfile for tests), tools/mem-check/
Gate / rollback: no flag: this is the D8 baseline behaviour. To undo, revert this merge. app.sqlite3 is a new file that the previous build ignores, so reinstalling the previous .click is a clean rollback.

Differences from the plan:

• Own migration runner: we use our own small runner instead of rusqlite_migration, whose MSRV (1.95) is above ours (1.85).
• Save on going inactive: QML flushes on Qt.application.state leaving Active, since there is no Lomiri-specific suspend hook.

Budgets (docs/07):

• Memory: CI peak PSS is 145–147 MB, against main's 143–149 MB (within noise; the baseline is 165000 kB).
• Start-up: 84–132 ms, against 113 ms on main (noise). The restart after kill is 49–63 ms.
• Package: the arm64 click is 1.96 MB (rusqlite bundled). Main's size was not measured, so this is not a measured delta.

Proof

• Rust CI jobs: cargo make (fmt, clippy -D warnings, tests + golden tests with coverage, release check, cargo-deny on both workspaces, cargo-audit) passes. 27 core tests, 94.20% line coverage.
• App job: cargo make app-docker (clang-format, qmllint, qt clippy, -Werror build, clang-tidy, mem-check) on 2bcf5b6 exits 0:

  mem-check: start-up 84 ms, showed 11 chapters (last: Gen 11), peak PSS 146567 kB, budget 165000 kB
  mem-check: inactive right after a scroll: 1 save(s), seen 187 ms after the scroll
  mem-check: after kill -9: restart 49 ms, saved 'BSB 296 0.818', restored 'BSB 296 0.818', showing Gen 11
  mem-check: on screen after the restore: 'BSB 296 0.818'
  

• Fails without the change:
  • Inactive save: with reading.flush() commented out, mem-check fails (0 save(s), seen 700 ms after the scroll, exit 105). The wait is bounded by the clock, below the 1 s debounce, so only the flush can pass it.
  • On-screen restore: this check found a real bug, the ListView's own reset undoing the restore. It was fixed in c6b8709.
  • Core: 17 deliberate breaks. 15 are caught by tests and 2 are equivalent mutants. The two breaks that first survived (skipping quick_check, dropping the fraction clamp) got new tests and are now caught.
• Package: the arm64 click built from f222cb8 passes click-review.
• On the device: the user installed f222cb8 on the Volla Phone X23 (Plinius) and checked by hand that the app reopens at the reading position. The result was "seems to work", with no log captured.
• Not proven: the order of reporting a scroll still on the 100 ms timer before the flush (ReadingPane.qml:125-128). Removing it alone still passes mem-check.

Review

Reviewer: 5 fresh-context rounds: high, then medium for each round of trunk fixes. 23 findings: 12 fixed, 11 declined or acknowledged.

• Declined: GoTo after a failed Step is unreachable until PR 3 adds the picker, and is noted for PR 3. A failed backup falls back to memory with a notice by design. The rest are cosmetic or near-impossible, with file:line evidence given in the audit.
• The last fix, 2bcf5b6 (the mem-check wait bounded by the clock), was not reviewed again.

Review-Confidence: 65/100. Core db and worker code is well reviewed, and its tests were checked to fail when the code is broken. The remaining gaps are an unreviewed final mem-check script edit (2bcf5b6) and an unproven reportTop-before-flush ordering.

Confidence by part

• core/src/db/, core/src/worker.rs, core/src/reader.rs: high. Tested, the deliberate breaks are caught, and they were reviewed 3 times.
• qt/src/reading_model.rs: medium. Exercised through mem-check and the user's device check. The stale-chapter-after-a-failed-GoTo case is a known gap for PR 3.
• qml/pages/ReadingPane.qml: medium. The restore and the inactive save are proven under Xvfb, and the user checked the restore on the Plinius by hand.
• tools/mem-check/run.sh: medium. It depends on timing, and the last edit was not reviewed.

Merge with Create merge commit: squash would drop the Review-Confidence trailer.

🤖 Generated with Claude Code

Part of #11. ## What The app now remembers where you were reading. - The core saves the module, the top verse and the fraction of it scrolled into a new `app.sqlite3`: 1 s after a change, at once when the app goes inactive, and on exit. - On start the app reopens that chapter at that verse, even after `kill -9`. - A corrupt file is set aside (`.corrupt-<time>`) and a fresh one is started, with a notice. Each migration is backed up first (`VACUUM INTO .bak-v<N>`, the last 2 are kept). ## Blast radius **Tier:** trunk **Trunk paths touched:** `core/src/db/` (new, migration 001), `core/src/reader.rs`, `core/src/worker.rs`, `qt/` glue (`reading_model.rs`), `qml/pages/ReadingPane.qml`, `Cargo.toml` + both lockfiles (rusqlite 0.40.2 bundled, tempfile for tests), `tools/mem-check/` **Gate / rollback:** no flag: this is the D8 baseline behaviour. To undo, revert this merge. `app.sqlite3` is a new file that the previous build ignores, so reinstalling the previous `.click` is a clean rollback. Differences from the plan: - **Own migration runner:** we use our own small runner instead of rusqlite_migration, whose MSRV (1.95) is above ours (1.85). - **Save on going inactive:** QML flushes on `Qt.application.state` leaving Active, since there is no Lomiri-specific suspend hook. **Budgets (docs/07):** - **Memory:** CI peak PSS is 145–147 MB, against main's 143–149 MB (within noise; the baseline is 165000 kB). - **Start-up:** 84–132 ms, against 113 ms on main (noise). The restart after kill is 49–63 ms. - **Package:** the arm64 click is 1.96 MB (rusqlite bundled). Main's size was not measured, so this is not a measured delta. ## Proof - **Rust CI jobs:** `cargo make` (fmt, clippy -D warnings, tests + golden tests with coverage, release check, cargo-deny on both workspaces, cargo-audit) passes. 27 core tests, 94.20% line coverage. - **App job:** `cargo make app-docker` (clang-format, qmllint, qt clippy, -Werror build, clang-tidy, mem-check) on 2bcf5b6 exits 0: ``` mem-check: start-up 84 ms, showed 11 chapters (last: Gen 11), peak PSS 146567 kB, budget 165000 kB mem-check: inactive right after a scroll: 1 save(s), seen 187 ms after the scroll mem-check: after kill -9: restart 49 ms, saved 'BSB 296 0.818', restored 'BSB 296 0.818', showing Gen 11 mem-check: on screen after the restore: 'BSB 296 0.818' ``` - **Fails without the change:** - **Inactive save:** with `reading.flush()` commented out, mem-check fails (`0 save(s), seen 700 ms after the scroll`, exit 105). The wait is bounded by the clock, below the 1 s debounce, so only the flush can pass it. - **On-screen restore:** this check found a real bug, the ListView's own reset undoing the restore. It was fixed in c6b8709. - **Core:** 17 deliberate breaks. 15 are caught by tests and 2 are equivalent mutants. The two breaks that first survived (skipping `quick_check`, dropping the fraction clamp) got new tests and are now caught. - **Package:** the arm64 click built from f222cb8 passes click-review. - **On the device:** the user installed f222cb8 on the Volla Phone X23 (Plinius) and checked by hand that the app reopens at the reading position. The result was "seems to work", with no log captured. - **Not proven:** the order of reporting a scroll still on the 100 ms timer before the flush (ReadingPane.qml:125-128). Removing it alone still passes mem-check. ## Review **Reviewer:** 5 fresh-context rounds: high, then medium for each round of trunk fixes. 23 findings: 12 fixed, 11 declined or acknowledged. - Declined: GoTo after a failed Step is unreachable until PR 3 adds the picker, and is noted for PR 3. A failed backup falls back to memory with a notice by design. The rest are cosmetic or near-impossible, with file:line evidence given in the audit. - The last fix, 2bcf5b6 (the mem-check wait bounded by the clock), was not reviewed again. **Review-Confidence:** 65/100. Core db and worker code is well reviewed, and its tests were checked to fail when the code is broken. The remaining gaps are an unreviewed final mem-check script edit (2bcf5b6) and an unproven reportTop-before-flush ordering. ## Confidence by part - **`core/src/db/`, `core/src/worker.rs`, `core/src/reader.rs`: high.** Tested, the deliberate breaks are caught, and they were reviewed 3 times. - **`qt/src/reading_model.rs`: medium.** Exercised through mem-check and the user's device check. The stale-chapter-after-a-failed-GoTo case is a known gap for PR 3. - **`qml/pages/ReadingPane.qml`: medium.** The restore and the inactive save are proven under Xvfb, and the user checked the restore on the Plinius by hand. - **`tools/mem-check/run.sh`: medium.** It depends on timing, and the last edit was not reviewed. Merge with **Create merge commit**: squash would drop the Review-Confidence trailer. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 16 commits 2026-10-20 21:00:55 +00:00

docs: spec for the reading window (#11) ... b808711a38

Picker (touch grids, typed reference behind an off-by-default setting),
jump-only Back history, continuous scrolling as one flat list with a
sliding window of chapters, and the first database (migration 001,
backups, debounced position saves, restore after a kill). Delivered as
three stacked PRs. Adds the touch-first rule to 04's design direction.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: reading-window spec stores positions without a KJV column ... 42f303fcb7

Mapping is a pure function of (v11n, ordinal), so window and history
positions compute it when needed. The KJV-mapped range stays the rule for
bookmarks and notes (v0.3), which need indexed queries across editions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: plan for reading window PR 1 (database and restore) ... 46858c12ad

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: reading-window spec: fraction column, first-change debounce, migration failure keeps the file ... a5a77e8dd2

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core: app database with migration 001, backups and corrupt-file recovery ... 7ed915cec7

Our own small migration runner instead of rusqlite_migration, whose
current release needs Rust 1.95 (MSRV 1.85; the clickable image has
1.94).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core: anchors, ordinal moves and a versification fallback for the reader ... 04fce53827

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core: worker saves the position within a second and restores it on open ... 863cffa888

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qt, qml: report the top verse, restore it after a load, flush when inactive ... d01ebdd44d

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tools: mem-check kills the app and checks it restores the saved verse ... effac9e9ef

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core: test quick_check on a damaged page and the scroll-fraction clamp ... 3533e06142

Two deliberate breaks (skipping quick_check, dropping the clamp)
passed the tests before; these catch them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

core, qml: flush the pending scroll first, guard NaN fractions ... 76da06fc7a

From review:
- ReadingPane reports a scroll still waiting for its 100 ms timer
  before it flushes, so going inactive right after a scroll keeps it.
- A NaN fraction (zero-height row) saves as 0 instead of failing the
  NOT NULL constraint.
- A second Open saves the pending position before it reads the file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qml: open the notice tip once per new notice ... e84a6e6f0e

The tip was bound to notice !== "", which re-evaluates on the model's
one change signal (every chapter) and is never cleared, so an old
notice could come back. A declared string property only signals a
real change; checked with qml6: 10 same-value re-evaluations, 0 opens.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

qml: restore the scroll after the ListView's own reset; mem-check checks it ... c6b8709ee0

The restore ran in a Connections onModelReset handler that can fire
before the ListView handles the reset, which then put the list back
at the top: the core restored the verse but the screen showed the
chapter's start. The move now waits for the event loop.

mem-check now also checks the screen: after the restart, QML's first
top-verse report must name the saved verse and fraction (within 0.02).
Without the fix it fails (no report: the list never moved); with it:
  on screen after the restore: 'BSB 296 0.818'

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tools: mem-check checks that going inactive saves at once ... a69eaa8cea

A scroll, then the window is unmapped (the app goes inactive, as on
suspension): a save must follow within 0.4 s, before the 1 s debounce
could have made it. Without the flush on inactive it fails (0 saves);
with it, 1. ReadingPane logs 'utsb: inactive, saving' for the device.

Not covered: that a report still waiting for its 100 ms timer goes in
before the flush; xdotool alone takes about that long. The check runs
in the first app: in the restarted one xdotool's wheel events never
reached the list under Xvfb.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tools: mem-check polls for the inactive save and checks the app went inactive ... 30cf663ee7

Review of a69eaa8: the fixed 0.4 s wait could flake on a slow runner,
any save counted even if Qt never left Active, and windowmap had no
timeout. Now it polls up to 0.8 s (under the 1 s debounce, so only the
flush passes it), requires 'utsb: inactive, saving', and times out the
remap. Fails with the flush commented out (0 saves within 800 ms).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tools: mem-check bounds the inactive-save wait by the clock ... f222cb822a

Review of 30cf663: 16 polls of 50 ms is not 0.8 s on a slow runner,
so the 1 s debounce save could land inside the window and pass the
check without the flush. Now the wait is measured from the scroll with
date +%s%N (0.7 s, and a save seen at 1 s or later fails), the coupling
to SAVE_DELAY is stated, and a failed remap says so. Fails with the
flush commented out (0 saves, 700 ms).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review-Confidence: 65
Review-Tier: trunk

schaefera approved these changes 2026-10-20 21:19:40 +00:00

schaefera merged commit a900ae8d1a into main 2026-10-20 21:19:44 +00:00

schaefera deleted branch phase1/reading-window 2026-10-20 21:19:44 +00:00

schaefera referenced this pull request from a commit 2026-10-20 21:19:44 +00:00

Merge pull request 'Reading window 1/3: app database and restore after a kill' (#15) from phase1/reading-window into main

claude-bot referenced this pull request 2026-10-22 08:07:25 +00:00

Reading window: picker, continuous scrolling, history, restore after kill #11

Sign in to join this conversation.

Reviewers

No reviewers

schaefera

Labels

Clear labels   

priority

Could

MoSCoW: nice to have, done when time allows

  

priority

Must

MoSCoW: the release isn't viable without it

  

priority

Should

MoSCoW: important, but the release works without it

  

priority

Won't

MoSCoW: not this release (kept for later)

  

type

Bug

No labels

priority

Could

priority

Must

priority

Should

priority

Won't

type

Bug

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

schaefera/utStudyBIble!15

No description provided.