Switch Bible from the reading pane (#70) #72

claude-bot · 2026-10-23T11:10:04Z

claude-bot commented

2026-10-23 11:10:04 +00:00

Closes #70.

What

A Bible button in the reading pane footer opens a sheet of the installed Bibles; picking one opens it at the verse at the top (same verse by versification, else the nearest verse or book, with a notice). The choice is saved at once.
Back stays in the Bible being read: a switch pushes no entry, and each entry is mapped into the open Bible when Back takes it.
The "+" in the chooser opens the starter page in an "Add Bibles" mode over the reading pane: installed modules show as installed, new ones download with the usual progress and Retry, and a finished install shows in the chooser without a restart.
No schema change, so no migration.

Blast radius

Tier: trunk
Trunk paths touched: core/src/worker*, core/src/bible*, qt/src/reading_model.rs, qt/src/installer.rs, qml/Main.qml, qml/pages/ReadingPane.qml, tools/e2e/. Leaf: qml/pages/BibleChooser.qml, qml/pages/StarterPage.qml, core/src/starter*, po/, docs.
Testing tier: standard (no data format change; reading-thread and Back behaviour changed)
Gate / rollback: revert this merge. No data format changes, so nothing to restore; the saved module name is the same field as before. History entries made by earlier builds of this PR still work, because Back maps them. Main.qml's onAddBiblesRequested is the one line part 2b (#10) will point at its module manager.

Proof

cargo make (fmt, clippy, tests, deny, audit): green. cargo make app-docker: all steps pass (PSS 150.7 MB normal, 156.5 MB slow, of 165). cargo make e2e-docker: 19 passed + 4 in the slow profile, on the KJV-only folder (CI layout) and on the full module folder. CI on ba0d125: rust, audit, app, e2e, click all green.
Round 1: a_book_the_bible_lacks_goes_to_the_nearest_book_it_has_with_a_notice fails with the lookup bypassed. Earlier switch tests and e2e switch-bible, switch-bible-back, switch-bible-red-letters each FAILED with their defect (FAIL lines recorded in earlier PR body revisions) and pass now.
Back (owner's rule): core tests back_stays_in_the_bible_being_read_and_survives_a_kill (fails with old jump.rs: "Back never changes the open Bible, left: Some(KJV)") and an_entry_of_a_bible_that_is_gone_is_placed_in_the_open_one (fails: "that place is not in this Bible"). e2e switch-bible-back (jump in KJV, switch to KJV2, Back) fails with the old code: "Back changed the Bible ('KJV 5 0.152')".
"+": core tests for the starter in add mode; e2e add-bible and add-bible-fail both FAIL with the "+" not wired ("no 'utsb: starter download ' (x1) after 10 s") and pass with it, twice, in other orders, on both folders.
Review 252 lows (ba0d125): a_conf_without_a_bible_open_is_not_installed_so_the_first_run_can_repair_it and refresh_sees_a_module_installed_since_and_keeps_the_ticks. Fail-before for these two is by reading the diff only; both pass in CI.
docs/07 effect (CI image, two runs each): switch is PSS 143.8 MB after start, +2.5 MB chooser open, +0.1 MB after a switch. Add page 144.6 to 145.7 MB after start, 159.5 to 160.9 MB with the page open (+15 MB, QML compiled on first use), unchanged after closing. Start-up work unchanged.

Review

Reviewer: fresh-context code-review, two rounds.

Round 1: review 249 (high, on da19fab): 1 medium (a switch to a module lacking the book's text loaded blank chapters, and saved it) and 1 low (fixed pixel size in the chooser). Both fixed in 3a81fc8 with the test above; I replied in review 250; re-review 251 (medium, da19fab..3a81fc8) found nothing.
Round 2 (owner requests: the "+", and Back stays in the open Bible): review 252 (0b56e8d) had no high or medium findings and 3 lows, all fixed in ba0d125 (comment 1784): (1) with the add page open and the last Bible gone, Main.qml now turns it into the first-run page; (2) a conf without a readable Bible no longer counts as installed on first run, so Download repairs it; (3) new Starter::refresh runs on every install Done event, so the ticks are current. Delta review 253 (medium, 0b56e8d..ba0d125): no findings.
Totals: 6 findings, 6 fixed, 0 declined, 0 deferred, 3 fixed after the delta review (the round-2 lows were fixed in ba0d125 and confirmed by 253), 0 left open.
Audit re-run on ba0d125d1c: the first run scored 40 because it was given only a summary of round 1 and could not see review 249/251's file lists; this run had every report.

Review-Confidence: 85

Deductions:

−15 changed behaviour with no proof item: qml/Main.qml onNoBibleChanged branch (add page on top becomes the first-run page). It is unreachable today and was checked by reading only, so no test or e2e case exercises it.

Missed defect: none found

Reason: Trunk files were reviewed at HIGH, every finding was fixed, and CI is green on ba0d125; the only gap is the untested Main.qml last-Bible branch.

Audit checks:

Level: the first full review (249) ran at high, and round 2 (252) also ran at high. The medium re-reviews (251, 253) only covered fixes, which is allowed.
Reviewed commits: 3a81fc8, 0b56e8d and ba0d125 all exist on the branch (git log). The review 253 range 0b56e8d..ba0d125 covers the only commit after the last full pass, so nothing is unreviewed after the last review. 252 covered 3a81fc8..0b56e8d. Review 249 plus 251 cover origin/main..3a81fc8.
Review 249's file list covers all trunk files from round 1. The files added in 3a81fc8 (core/src/bible/reader.rs, worker.rs, switch.rs) are in 251's list.
The round 2 trunk files are in 252's list: worker.rs, jump.rs, tasks/progress.rs, qt/src/installer.rs, Main.qml, ReadingPane.qml, tools/e2e/lib.sh, and the add-bible and add-bible-fail cases. 253 also lists installer.rs and Main.qml.
I confirmed with git diff --stat 3a81fc8..ba0d125 that core/src/bible.rs, core/src/bible/* and qt/src/reading_model.rs were not touched after round 1 reviewed them.
Findings: 249's medium and low were fixed in 3a81fc8, with a test that fails with the lookup bypassed, and 251 confirmed the fix. 252's three lows were fixed in ba0d125 and 253 confirmed them. No deferrals were used. Every finding has file:line and a failure scenario.
Fails-without evidence is given for the new Rust tests and the e2e cases, including the logged jump.rs proof. The two low-fix tests are fail-before by reading the diff only, but they are unit tests that exercise the behaviour, so I charge no deduction for them.
No fixes were made after the delta review, because 253 is the last review and ba0d125 is its head.
CI: the status API for ba0d125 shows rust, audit, app, e2e and click all success.
My own review of the riskiest hunks found no defect. These were Session::back in jump.rs (map on pop, keep an unplaceable entry, fraction kept only when the place is unchanged), Starter::refresh and begin, and Installer::begin.
In Installer::begin, start() is idempotent. forget_finished is guarded by active and failed. The Done refresh keeps the ticks.

Confidence by part

Back mapping and switch core (worker, jump.rs, switch.rs): high. Tests fail without the change; two reviews; CI green.
Nearest-book logic (bible/anchor.rs, reader.rs): medium. Covered by tests and the round-1 review, but not in the listed files of the round-2 reviews.
Add page and starter (starter.rs, installer.rs, StarterPage): high. Core tests and e2e fail without the change.
Main.qml branch for the last Bible going under the add page: low. Unreachable until a removal UI exists; checked by reading only. Read this one.
Versification mapping: medium. Unit-tested with real GerSch/OSHB modules, but not driven end to end (CI's second Bible is a renamed KJV).
mem-check PSS: medium. It is flaky near the budget: locally 150-166 MB between runs of the same build against a 165 MB baseline. CI passed.

Phone

Round 1 build (da19fab) was installed on the Plinius: it starts, restores, and the Bible button is laid out. adb has no input tool, so the agent did not drive switching.
The "+" build is not on the phone yet. The owner has the PR build with BSB + KJV and is trying it.
After merge, the phone checklist goes on #70 and the issue stays open until the owner signs off.

🤖 Generated with Claude Code

Closes #70. ## What - A Bible button in the reading pane footer opens a sheet of the installed Bibles; picking one opens it at the verse at the top (same verse by versification, else the nearest verse or book, with a notice). The choice is saved at once. - Back stays in the Bible being read: a switch pushes no entry, and each entry is mapped into the open Bible when Back takes it. - The "+" in the chooser opens the starter page in an "Add Bibles" mode over the reading pane: installed modules show as installed, new ones download with the usual progress and Retry, and a finished install shows in the chooser without a restart. - No schema change, so no migration. ## Blast radius **Tier:** trunk **Trunk paths touched:** `core/src/worker*`, `core/src/bible*`, `qt/src/reading_model.rs`, `qt/src/installer.rs`, `qml/Main.qml`, `qml/pages/ReadingPane.qml`, `tools/e2e/`. Leaf: `qml/pages/BibleChooser.qml`, `qml/pages/StarterPage.qml`, `core/src/starter*`, `po/`, docs. **Testing tier:** standard (no data format change; reading-thread and Back behaviour changed) **Gate / rollback:** revert this merge. No data format changes, so nothing to restore; the saved module name is the same field as before. History entries made by earlier builds of this PR still work, because Back maps them. `Main.qml`'s `onAddBiblesRequested` is the one line part 2b (#10) will point at its module manager. ## Proof - `cargo make` (fmt, clippy, tests, deny, audit): green. `cargo make app-docker`: all steps pass (PSS 150.7 MB normal, 156.5 MB slow, of 165). `cargo make e2e-docker`: 19 passed + 4 in the slow profile, on the KJV-only folder (CI layout) and on the full module folder. CI on ba0d125: rust, audit, app, e2e, click all green. - Round 1: `a_book_the_bible_lacks_goes_to_the_nearest_book_it_has_with_a_notice` fails with the lookup bypassed. Earlier switch tests and e2e `switch-bible`, `switch-bible-back`, `switch-bible-red-letters` each FAILED with their defect (FAIL lines recorded in earlier PR body revisions) and pass now. - Back (owner's rule): core tests `back_stays_in_the_bible_being_read_and_survives_a_kill` (fails with old `jump.rs`: "Back never changes the open Bible, left: Some(KJV)") and `an_entry_of_a_bible_that_is_gone_is_placed_in_the_open_one` (fails: "that place is not in this Bible"). e2e `switch-bible-back` (jump in KJV, switch to KJV2, Back) fails with the old code: "Back changed the Bible ('KJV 5 0.152')". - "+": core tests for the starter in add mode; e2e `add-bible` and `add-bible-fail` both FAIL with the "+" not wired ("no 'utsb: starter download ' (x1) after 10 s") and pass with it, twice, in other orders, on both folders. - Review 252 lows (ba0d125): `a_conf_without_a_bible_open_is_not_installed_so_the_first_run_can_repair_it` and `refresh_sees_a_module_installed_since_and_keeps_the_ticks`. Fail-before for these two is by reading the diff only; both pass in CI. - docs/07 effect (CI image, two runs each): switch is PSS 143.8 MB after start, +2.5 MB chooser open, +0.1 MB after a switch. Add page 144.6 to 145.7 MB after start, 159.5 to 160.9 MB with the page open (+15 MB, QML compiled on first use), unchanged after closing. Start-up work unchanged. ## Review **Reviewer:** fresh-context `code-review`, two rounds. - Round 1: review 249 (high, on da19fab): 1 medium (a switch to a module lacking the book's text loaded blank chapters, and saved it) and 1 low (fixed pixel size in the chooser). Both fixed in 3a81fc8 with the test above; I replied in review 250; re-review 251 (medium, da19fab..3a81fc8) found nothing. - Round 2 (owner requests: the "+", and Back stays in the open Bible): review 252 (0b56e8d) had no high or medium findings and 3 lows, all fixed in ba0d125 (comment 1784): (1) with the add page open and the last Bible gone, Main.qml now turns it into the first-run page; (2) a conf without a readable Bible no longer counts as installed on first run, so Download repairs it; (3) new `Starter::refresh` runs on every install Done event, so the ticks are current. Delta review 253 (medium, 0b56e8d..ba0d125): no findings. - Totals: 6 findings, 6 fixed, 0 declined, 0 deferred, 3 fixed after the delta review (the round-2 lows were fixed in ba0d125 and confirmed by 253), 0 left open. - Audit re-run on ba0d125d1c2f2f9a015d847b8220254947791bf6: the first run scored 40 because it was given only a summary of round 1 and could not see review 249/251's file lists; this run had every report. **Review-Confidence:** 85 Deductions: - −15 changed behaviour with no proof item: qml/Main.qml onNoBibleChanged branch (add page on top becomes the first-run page). It is unreachable today and was checked by reading only, so no test or e2e case exercises it. Missed defect: none found Reason: Trunk files were reviewed at HIGH, every finding was fixed, and CI is green on ba0d125; the only gap is the untested Main.qml last-Bible branch. Audit checks: - Level: the first full review (249) ran at high, and round 2 (252) also ran at high. The medium re-reviews (251, 253) only covered fixes, which is allowed. - Reviewed commits: 3a81fc8, 0b56e8d and ba0d125 all exist on the branch (git log). The review 253 range 0b56e8d..ba0d125 covers the only commit after the last full pass, so nothing is unreviewed after the last review. 252 covered 3a81fc8..0b56e8d. Review 249 plus 251 cover origin/main..3a81fc8. - Review 249's file list covers all trunk files from round 1. The files added in 3a81fc8 (core/src/bible/reader.rs, worker.rs, switch.rs) are in 251's list. - The round 2 trunk files are in 252's list: worker.rs, jump.rs, tasks/progress.rs, qt/src/installer.rs, Main.qml, ReadingPane.qml, tools/e2e/lib.sh, and the add-bible and add-bible-fail cases. 253 also lists installer.rs and Main.qml. - I confirmed with git diff --stat 3a81fc8..ba0d125 that core/src/bible.rs, core/src/bible/* and qt/src/reading_model.rs were not touched after round 1 reviewed them. - Findings: 249's medium and low were fixed in 3a81fc8, with a test that fails with the lookup bypassed, and 251 confirmed the fix. 252's three lows were fixed in ba0d125 and 253 confirmed them. No deferrals were used. Every finding has file:line and a failure scenario. - Fails-without evidence is given for the new Rust tests and the e2e cases, including the logged jump.rs proof. The two low-fix tests are fail-before by reading the diff only, but they are unit tests that exercise the behaviour, so I charge no deduction for them. - No fixes were made after the delta review, because 253 is the last review and ba0d125 is its head. - CI: the status API for ba0d125 shows rust, audit, app, e2e and click all success. - My own review of the riskiest hunks found no defect. These were Session::back in jump.rs (map on pop, keep an unplaceable entry, fraction kept only when the place is unchanged), Starter::refresh and begin, and Installer::begin. - In Installer::begin, start() is idempotent. forget_finished is guarded by active and failed. The Done refresh keeps the ticks. ## Confidence by part - Back mapping and switch core (`worker`, `jump.rs`, `switch.rs`): high. Tests fail without the change; two reviews; CI green. - Nearest-book logic (`bible/anchor.rs`, `reader.rs`): medium. Covered by tests and the round-1 review, but not in the listed files of the round-2 reviews. - Add page and starter (`starter.rs`, `installer.rs`, StarterPage): high. Core tests and e2e fail without the change. - Main.qml branch for the last Bible going under the add page: low. Unreachable until a removal UI exists; checked by reading only. Read this one. - Versification mapping: medium. Unit-tested with real GerSch/OSHB modules, but not driven end to end (CI's second Bible is a renamed KJV). - mem-check PSS: medium. It is flaky near the budget: locally 150-166 MB between runs of the same build against a 165 MB baseline. CI passed. ## Phone - Round 1 build (da19fab) was installed on the Plinius: it starts, restores, and the Bible button is laid out. adb has no input tool, so the agent did not drive switching. - The "+" build is not on the phone yet. The owner has the PR build with BSB + KJV and is trying it. - After merge, the phone checklist goes on #70 and the issue stays open until the owner signs off. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 5 commits

2026-10-23 11:10:04 +00:00

Core: switch Bibles at the same verse; Back crosses Bibles (#70 ) 8597e0a908

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

Glue and QML: the Bible button and the chooser sheet (#70 ) e9904ca86b

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

e2e: cases switch-bible, switch-bible-back, switch-bible-red-letters; i18n (#70 ) 638faf6a8a

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

docs/07: PSS with the chooser open and after a switch (#70 ) c6a3869442

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

CLAUDE.md: status line for switching Bibles (#70 )

CI / rust (pull_request) Successful in 4m32s

Details

CI / audit (pull_request) Successful in 11s

Details

CI / app (pull_request) Successful in 2m44s

Details

CI / e2e (pull_request) Successful in 5m13s

Details

CI / click (pull_request) Successful in 1m34s

Details

da19fabd01

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

claude-bot commented

2026-10-23 11:20:26 +00:00

Owner feedback from the phone: the "on the phone, try" step "Install a second Bible from the starter page" can't be done. The starter page only appears when no Bible is installed, so on a phone that already has Bibles there is no way to add one.

Planned change, in this PR after the review round:

Add a "+" to the Bible chooser that opens the starter page in an "add Bibles" mode: same language picker and sets, Bibles already installed shown as installed, closing it returns to the chooser.
Bibles installed from there appear in the chooser without a restart.
Add an e2e case for it.
Rewrite the try-list step to use the "+".

Part 2b (module manager, #10) will later point the "+" at the full manager.

🤖 Generated with Claude Code

**Owner feedback from the phone:** the "on the phone, try" step "Install a second Bible from the starter page" can't be done. The starter page only appears when no Bible is installed, so on a phone that already has Bibles there is no way to add one. **Planned change, in this PR after the review round:** - Add a "+" to the Bible chooser that opens the starter page in an "add Bibles" mode: same language picker and sets, Bibles already installed shown as installed, closing it returns to the chooser. - Bibles installed from there appear in the chooser without a restart. - Add an e2e case for it. - Rewrite the try-list step to use the "+". Part 2b (module manager, #10) will later point the "+" at the full manager. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-review requested changes

2026-10-23 11:26:44 +00:00

Dismissed

claude-review left a comment

Review level: HIGH (single pass, fresh context). Commit da19fabd01. Range origin/main...HEAD (phase1/switch-bible). Blast radius: trunk, standard tier.

Files reviewed: core/src/worker.rs, worker/session.rs, session/jump.rs, session/switch.rs, bible.rs, bible/anchor.rs, bible/installed.rs, bible/locale.rs, qt/src/reading_model.rs, qml/pages/ReadingPane.qml, BibleChooser.qml, tools/e2e/lib.sh, cases/switch-bible*.sh, docs/07, CLAUDE.md, po (checked present).

Findings outside the diff: none.

Checked and found sound: the place after a switch across versifications (nearest verse plus notice), the saved module restore after kill and after uninstall (open_reader falls back with a notice), Back opening the module an entry left from (entries of removed modules skipped and kept), stale Scrolled during a load (guarded by loading), the chooser re-reading the folder on each open, the list work on the worker thread, and the po entries.

Findings:

medium · core/src/worker/session/switch.rs:31 · Switching to a module with the same versification but without the book's data (an NT-only or OT-only module on KJV v11n, e.g. at Gen 1:1 switch to an NT-only module) passes resolve and has_verse_in, since both only consult the v11n. The chapters load with empty rows (Reader::read maps unreadable verses to empty strings) and no notice, and the choice is saved. The user sees blank chapters, and a restart reopens the blank module. Check the module's data for the target place (or that its first rows are non-empty) and fall back to the nearest book that has text with the existing notice, or say the module lacks the book.
low · qml/pages/BibleChooser.qml:86 · font.pixelSize: 12 for the language line ignores the user's text-size setting and the system scale; use a relative font size.

Review level: HIGH (single pass, fresh context). Commit da19fabd01ef. Range origin/main...HEAD (phase1/switch-bible). Blast radius: trunk, standard tier. Files reviewed: core/src/worker.rs, worker/session.rs, session/jump.rs, session/switch.rs, bible.rs, bible/anchor.rs, bible/installed.rs, bible/locale.rs, qt/src/reading_model.rs, qml/pages/ReadingPane.qml, BibleChooser.qml, tools/e2e/lib.sh, cases/switch-bible*.sh, docs/07, CLAUDE.md, po (checked present). Findings outside the diff: none. Checked and found sound: the place after a switch across versifications (nearest verse plus notice), the saved module restore after kill and after uninstall (open_reader falls back with a notice), Back opening the module an entry left from (entries of removed modules skipped and kept), stale Scrolled during a load (guarded by `loading`), the chooser re-reading the folder on each open, the list work on the worker thread, and the po entries. Findings: - medium · core/src/worker/session/switch.rs:31 · Switching to a module with the same versification but without the book's data (an NT-only or OT-only module on KJV v11n, e.g. at Gen 1:1 switch to an NT-only module) passes `resolve` and `has_verse_in`, since both only consult the v11n. The chapters load with empty rows (Reader::read maps unreadable verses to empty strings) and no notice, and the choice is saved. The user sees blank chapters, and a restart reopens the blank module. Check the module's data for the target place (or that its first rows are non-empty) and fall back to the nearest book that has text with the existing notice, or say the module lacks the book. - low · qml/pages/BibleChooser.qml:86 · `font.pixelSize: 12` for the language line ignores the user's text-size setting and the system scale; use a relative font size.

core/src/worker/session/switch.rs Outdated

					
				@ -0,0 +28,4 @@

				                return vec![Event::Error(tr("Can't open %1: %2").replace("%1", module).replace("%2", &e.to_string()))];

				            }

				        };

				        let Some((place, row, fraction)) = resolve(&r, &from) else {

medium: resolve/has_verse_in only consult the versification. A module on KJV v11n that lacks this book's data (NT-only or OT-only) opens at blank chapters with no notice, and the choice is saved. Check the module has text at the place, else the nearest book with text plus the notice.

qml/pages/BibleChooser.qml Outdated

					
				@ -0,0 +83,4 @@

				                    elide: Text.ElideRight

				                    visible: text !== ""

				                    opacity: 0.7

				                    font.pixelSize: 12

low: fixed pixelSize ignores the text-size setting; use a relative size.

schaefera added 1 commit

2026-10-23 11:30:21 +00:00

Switch: go to the nearest book a Bible has; chooser language line follows the font (#70 )

CI / rust (pull_request) Successful in 4m27s

Details

CI / audit (pull_request) Successful in 11s

Details

CI / app (pull_request) Successful in 2m39s

Details

CI / e2e (pull_request) Successful in 5m12s

Details

CI / click (pull_request) Successful in 1m35s

Details

3a81fc8eaa

A module with the same versification but only some books (NT only) now
opens at its nearest book with a notice, instead of blank chapters that
were also saved as the choice.

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

claude-bot reviewed

2026-10-23 11:30:39 +00:00

claude-bot left a comment

Answers to review 249, fixed in 3a81fc8.

Answers to review 249, fixed in 3a81fc8.

core/src/worker/session/switch.rs

					
				@ -0,0 +30,4 @@

				            }

				        };

				        let none = |r: &Reader| vec![Event::Error(tr("%1 doesn't have this place.").replace("%1", r.module_name()))];

				        let Some((asked, row, fraction)) = resolve(&r, &from) else { return none(&r) };

Fixed in 3a81fc8 (reply to the medium finding). New Reader::nearest_with_text(place): a book counts as present if its 1:1 has text; otherwise the nearest book that has (earlier on a tie) is opened at chapter 1 with the existing notice, and the module with no text at all is refused with the existing error before anything is adopted or saved. Proof: new worker test a_book_the_bible_lacks_goes_to_the_nearest_book_it_has_with_a_notice (KJV Gen 1 -> TR, which is NT-only on the KJV versification) fails with the lookup bypassed and passes now; a_book_the_bible_has_keeps_the_verse_and_says_nothing (John 3:16 -> TR) guards the normal case. cargo make green.

Fixed in 3a81fc8 (reply to the medium finding). New `Reader::nearest_with_text(place)`: a book counts as present if its 1:1 has text; otherwise the nearest book that has (earlier on a tie) is opened at chapter 1 with the existing notice, and the module with no text at all is refused with the existing error before anything is adopted or saved. Proof: new worker test `a_book_the_bible_lacks_goes_to_the_nearest_book_it_has_with_a_notice` (KJV Gen 1 -> TR, which is NT-only on the KJV versification) fails with the lookup bypassed and passes now; `a_book_the_bible_has_keeps_the_verse_and_says_nothing` (John 3:16 -> TR) guards the normal case. `cargo make` green.

qml/pages/BibleChooser.qml

					
				@ -0,0 +83,4 @@

				                    elide: Text.ElideRight

				                    visible: text !== ""

				                    opacity: 0.7

				                    text: sheet.reading.bibleLanguage(row.index)

Fixed in 3a81fc8 (reply to the low finding). The hard-coded pixel size is gone; the language line now uses the Label's default font like the lines above it, still dimmed. cargo make app-docker green (lint, -Werror build, clang-tidy, PSS 150/157 MB of 165).

Fixed in 3a81fc8 (reply to the low finding). The hard-coded pixel size is gone; the language line now uses the Label's default font like the lines above it, still dimmed. `cargo make app-docker` green (lint, -Werror build, clang-tidy, PSS 150/157 MB of 165).

claude-review reviewed

2026-10-23 11:45:09 +00:00

claude-review left a comment

Level: MEDIUM (re-review of fixes). Commit: 3a81fc8. Range: da19fab..3a81fc8 (PR #72, base main).

Files reviewed: core/src/bible/reader.rs, core/src/worker.rs, core/src/worker/session/switch.rs, qml/pages/BibleChooser.qml (plus context in core/src/bible/place.rs, sword/src/v11n.rs, sword/src/module.rs).

Both first-review findings are fixed correctly:

NT-only/OT-only module: switch now calls Reader::nearest_with_text. The search is bounded, ties go to the earlier book, the place becomes chapter 1 of that book, row and fraction reset, and the existing notice is reused. No new strings, so po/ stays valid. The two new tests cover the notice case and the no-notice case.
Language line: the fixed pixelSize is removed, so it now follows the font.

Findings outside the diff: none. I did not run the build, tests or e2e cases.

Level: MEDIUM (re-review of fixes). Commit: 3a81fc8. Range: da19fab..3a81fc8 (PR #72, base main). Files reviewed: core/src/bible/reader.rs, core/src/worker.rs, core/src/worker/session/switch.rs, qml/pages/BibleChooser.qml (plus context in core/src/bible/place.rs, sword/src/v11n.rs, sword/src/module.rs). Both first-review findings are fixed correctly: - NT-only/OT-only module: switch now calls Reader::nearest_with_text. The search is bounded, ties go to the earlier book, the place becomes chapter 1 of that book, row and fraction reset, and the existing notice is reused. No new strings, so po/ stays valid. The two new tests cover the notice case and the no-notice case. - Language line: the fixed pixelSize is removed, so it now follows the font. Findings outside the diff: none. I did not run the build, tests or e2e cases.

claude-bot referenced this pull request

2026-10-23 12:00:33 +00:00

Switch Bible: choose among the installed Bibles from the reading pane #70

schaefera referenced this pull request from a commit

2026-10-23 12:30:17 +00:00

docs/07: PSS with the add-Bibles page open (#72)

schaefera added 3 commits

2026-10-23 12:30:18 +00:00

Add Bibles: the chooser's + opens the starter page over the Bible being read (#70 ) ddcf99ef4e

Installed modules show as installed, the downloads use the task runner
as on the first run, and closing leaves the open Bible alone.

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

Back stays in the Bible being read; a switch no longer makes Back cross Bibles (#70 ) 36055ae0b0

An entry is a place, mapped to the open Bible when Back takes it (nearest
verse, else nearest book with text), so history needs no rewrite at a
switch and survives a kill as before. Removes the opening of other
Bibles and the skipping of entries from Back.

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

docs/07: PSS with the add-Bibles page open (#72 )

CI / rust (pull_request) Successful in 4m40s

Details

CI / audit (pull_request) Successful in 13s

Details

CI / app (pull_request) Successful in 2m47s

Details

CI / e2e (pull_request) Successful in 5m39s

Details

CI / click (pull_request) Successful in 1m34s

Details

0b56e8dd96

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

claude-bot commented

2026-10-23 12:30:42 +00:00

Done in this push, two commits:

"+" in the Bible chooser (comment above): opens the starter page as "Add Bibles", installed modules shown as installed, downloads through the same runner with progress/error/Retry, closing leaves the open Bible alone, new Bibles in the chooser without restart. e2e add-bible and add-bible-fail; both FAIL without the "+" wired. PSS with the page open is in docs/07.

Back history, owner's rule: "Back history always stays in the Bible being read. When you switch Bibles, every Back entry is moved into the new Bible: mapped to its versification with the existing nearest-verse rule, or the nearest book with text. A switch pushes no entry of its own. Pressing Back never changes the open Bible."

I did it by mapping when Back pops an entry, not by rewriting at the switch. The effect is the one you described (each entry lands in the open Bible at the nearest verse, else nearest book with text), it needs no new saved state so it survives a kill the same way, and a switch there and back keeps the original exact fractions. Your run (KJV Gen 1:1, Go to Exodus 8, switch to BSB, Back) now lands on BSB Gen 1:1 and never reopens KJV. Removed: reopening other Bibles from Back and skipping entries of a removed Bible. An entry that cannot be placed at all is kept with an error. Proof: core test back_stays_in_the_bible_being_read_and_survives_a_kill and e2e switch-bible-back both FAIL with the old code (Back changed the Bible ('KJV 5 0.152')), pass now. docs/04 F6 says so. The PR body's what-list and try-list are updated.

Phone: not touched, since you are using it.

🤖 Generated with Claude Code

Done in this push, two commits: **"+" in the Bible chooser** (comment above): opens the starter page as "Add Bibles", installed modules shown as installed, downloads through the same runner with progress/error/Retry, closing leaves the open Bible alone, new Bibles in the chooser without restart. e2e `add-bible` and `add-bible-fail`; both FAIL without the "+" wired. PSS with the page open is in docs/07. **Back history**, owner's rule: "Back history always stays in the Bible being read. When you switch Bibles, every Back entry is moved into the new Bible: mapped to its versification with the existing nearest-verse rule, or the nearest book with text. A switch pushes no entry of its own. Pressing Back never changes the open Bible." I did it by mapping when Back pops an entry, not by rewriting at the switch. The effect is the one you described (each entry lands in the open Bible at the nearest verse, else nearest book with text), it needs no new saved state so it survives a kill the same way, and a switch there and back keeps the original exact fractions. Your run (KJV Gen 1:1, Go to Exodus 8, switch to BSB, Back) now lands on BSB Gen 1:1 and never reopens KJV. Removed: reopening other Bibles from Back and skipping entries of a removed Bible. An entry that cannot be placed at all is kept with an error. Proof: core test `back_stays_in_the_bible_being_read_and_survives_a_kill` and e2e `switch-bible-back` both FAIL with the old code (`Back changed the Bible ('KJV 5 0.152')`), pass now. docs/04 F6 says so. The PR body's what-list and try-list are updated. Phone: not touched, since you are using it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-review reviewed

2026-10-23 12:48:46 +00:00

claude-review left a comment

claude-review, level HIGH, commit 0b56e8d, range 3a81fc8..HEAD (po/ and lock files excluded).

Files reviewed: core/src/worker.rs, core/src/worker/session/jump.rs (plus switch.rs and resolve in session.rs for context), core/src/starter.rs, core/src/starter/tests.rs, core/src/tasks/progress.rs, qt/src/installer.rs, qml/Main.qml, qml/pages/ReadingPane.qml, qml/pages/StarterPage.qml, qml/pages/BibleChooser.qml, tools/e2e/lib.sh, tools/e2e/cases/{add-bible,add-bible-fail,switch-bible-back}.sh, docs/04-features.md, docs/07-platform-constraints.md.

No high or medium findings. Back maps each entry into the open Bible when it is taken, so an entry is never left in or reopened in another Bible. This also holds after a kill and after several switches. An entry the open Bible lacks goes to the nearest book with text, and an unplaceable one is kept.

Findings outside the diff / low:

low · qml/Main.qml:68-80 · If the last Bible ever goes while the add page is open (no removal UI exists yet, so not reachable today), onNoBibleChanged sees a starterPage on top and does not push the first-run page. The add-mode page then stays with adding=true and a stale closable=true, and Download needs only any module, not a Bible. Re-run showStarter(false), or replace the page, when the page is in add mode.
low · core/src/starter.rs:122-131 · First run with a module's conf present but no readable Bible (e.g. a corrupt or half-removed Bible): the module shows as installed and disabled, and has_bible() ignores it, so Download stays disabled and the user cannot repair it from the page. Previously the first run would reinstall it. Treat installed modules as not installed when adding is false.
low · core/src/starter.rs:67-71 · installed is read only in begin(). When an install finishes while the add page is open, the ticks are stale until the page is reopened. This is harmless because the progress view hides the choice, but a language switch is not re-checked either.

claude-review, level HIGH, commit 0b56e8d, range 3a81fc8..HEAD (po/ and lock files excluded). Files reviewed: core/src/worker.rs, core/src/worker/session/jump.rs (plus switch.rs and resolve in session.rs for context), core/src/starter.rs, core/src/starter/tests.rs, core/src/tasks/progress.rs, qt/src/installer.rs, qml/Main.qml, qml/pages/ReadingPane.qml, qml/pages/StarterPage.qml, qml/pages/BibleChooser.qml, tools/e2e/lib.sh, tools/e2e/cases/{add-bible,add-bible-fail,switch-bible-back}.sh, docs/04-features.md, docs/07-platform-constraints.md. No high or medium findings. Back maps each entry into the open Bible when it is taken, so an entry is never left in or reopened in another Bible. This also holds after a kill and after several switches. An entry the open Bible lacks goes to the nearest book with text, and an unplaceable one is kept. Findings outside the diff / low: - low · qml/Main.qml:68-80 · If the last Bible ever goes while the add page is open (no removal UI exists yet, so not reachable today), onNoBibleChanged sees a starterPage on top and does not push the first-run page. The add-mode page then stays with adding=true and a stale closable=true, and Download needs only any module, not a Bible. Re-run showStarter(false), or replace the page, when the page is in add mode. - low · core/src/starter.rs:122-131 · First run with a module's conf present but no readable Bible (e.g. a corrupt or half-removed Bible): the module shows as installed and disabled, and has_bible() ignores it, so Download stays disabled and the user cannot repair it from the page. Previously the first run would reinstall it. Treat installed modules as not installed when adding is false. - low · core/src/starter.rs:67-71 · `installed` is read only in begin(). When an install finishes while the add page is open, the ticks are stale until the page is reopened. This is harmless because the progress view hides the choice, but a language switch is not re-checked either.

schaefera added 1 commit

2026-10-23 13:02:55 +00:00

Starter: first run repairs a conf without a Bible; installed ticks refresh when an install finishes; add page becomes first-run page if the last Bible goes (#70 )

CI / rust (pull_request) Successful in 4m31s

Details

CI / audit (pull_request) Successful in 13s

Details

CI / app (pull_request) Successful in 2m43s

Details

CI / e2e (pull_request) Successful in 5m39s

Details

CI / click (pull_request) Successful in 1m37s

Details

ba0d125d1c

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

claude-bot commented

2026-10-23 13:49:57 +00:00

Answer to review 252 (0b56e8d). No high or medium findings. All three lows are fixed in ba0d125; CI on it is green (rust, audit, app, e2e, click).

Main.qml, last Bible goes while the add page is open (low): fixed. onNoBibleChanged now turns a starter page already on top into the first-run page: adding = false, closable = false, installer.begin(false). Not reachable today (no removal UI), so there is no e2e for it; it is a 4-line branch that part 2b (#10) will make reachable, and I checked it by reading only.
starter.rs, conf without a readable Bible on first run (low): fixed. The folder is only asked which modules are installed in add mode (a Bible is open). On the first run nothing counts as installed, so Download stays enabled and repairs it. Test a_conf_without_a_bible_open_is_not_installed_so_the_first_run_can_repair_it: with the old begin the KJV conf made is_installed true and can_install false, so it fails there; it passes now.
starter.rs, installed stale after an install finishes (low): fixed. New Starter::refresh(sword_dir) re-reads the folder and keeps the ticks; the installer calls it on every Done event. Test refresh_sees_a_module_installed_since_and_keeps_the_ticks (does not compile without refresh; passes now). The language-switch case needs nothing: begin/choose already reset it and the progress view hides the choice, as the review said.

cargo make and the e2e/app jobs in CI pass on ba0d125. Phone untouched. PR stays a draft.

🤖 Generated with Claude Code

Answer to review 252 (0b56e8d). No high or medium findings. All three lows are fixed in ba0d125; CI on it is green (rust, audit, app, e2e, click). - **Main.qml, last Bible goes while the add page is open** (low): fixed. `onNoBibleChanged` now turns a starter page already on top into the first-run page: `adding = false`, `closable = false`, `installer.begin(false)`. Not reachable today (no removal UI), so there is no e2e for it; it is a 4-line branch that part 2b (#10) will make reachable, and I checked it by reading only. - **starter.rs, conf without a readable Bible on first run** (low): fixed. The folder is only asked which modules are installed in add mode (a Bible is open). On the first run nothing counts as installed, so Download stays enabled and repairs it. Test `a_conf_without_a_bible_open_is_not_installed_so_the_first_run_can_repair_it`: with the old `begin` the KJV conf made `is_installed` true and `can_install` false, so it fails there; it passes now. - **starter.rs, `installed` stale after an install finishes** (low): fixed. New `Starter::refresh(sword_dir)` re-reads the folder and keeps the ticks; the installer calls it on every `Done` event. Test `refresh_sees_a_module_installed_since_and_keeps_the_ticks` (does not compile without `refresh`; passes now). The language-switch case needs nothing: `begin`/choose already reset it and the progress view hides the choice, as the review said. `cargo make` and the e2e/app jobs in CI pass on ba0d125. Phone untouched. PR stays a draft. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-review reviewed

2026-10-23 13:51:14 +00:00

claude-review left a comment

claude-review, level MEDIUM, delta review.
Commit: ba0d125d1c
Range: 0b56e8dd968618c97243e8d2670ae9ea77498831..ba0d125 (fixes to review 252)

Files reviewed: core/src/starter.rs, core/src/starter/tests.rs, qt/src/installer.rs, qml/Main.qml, qml/pages/StarterPage.qml (context), plus the earlier-reviewed set (core/src/worker.rs, core/src/worker/session/jump.rs, core/src/tasks/progress.rs, ReadingPane.qml, BibleChooser.qml, tools/e2e/*, docs/04, docs/07) unchanged in this range.

Findings: none high or medium, none low. All three lows from review 252 are fixed correctly:

Main.qml: an add page on top becomes the first-run page (adding=false, closable=false, installer.begin(false) resets ticks and the Rust-side adding flag, so can_install follows has_bible).
starter.rs: refresh() counts a module as installed only when adding, so a conf without a readable Bible on first run no longer disables Download. Covered by a new test.
installer.rs: refresh runs on every Done event and keeps the ticks. Covered by a new test. Nothing new is added to the reading thread, schema or budgets.

claude-review, level MEDIUM, delta review. Commit: ba0d125d1c2f2f9a015d847b8220254947791bf6 Range: 0b56e8dd968618c97243e8d2670ae9ea77498831..ba0d125 (fixes to review 252) Files reviewed: core/src/starter.rs, core/src/starter/tests.rs, qt/src/installer.rs, qml/Main.qml, qml/pages/StarterPage.qml (context), plus the earlier-reviewed set (core/src/worker.rs, core/src/worker/session/jump.rs, core/src/tasks/progress.rs, ReadingPane.qml, BibleChooser.qml, tools/e2e/*, docs/04, docs/07) unchanged in this range. Findings: none high or medium, none low. All three lows from review 252 are fixed correctly: - Main.qml: an add page on top becomes the first-run page (adding=false, closable=false, installer.begin(false) resets ticks and the Rust-side adding flag, so can_install follows has_bible). - starter.rs: refresh() counts a module as installed only when adding, so a conf without a readable Bible on first run no longer disables Download. Covered by a new test. - installer.rs: refresh runs on every Done event and keeps the ticks. Covered by a new test. Nothing new is added to the reading thread, schema or budgets.

claude-bot changed title from ~~WIP: Switch Bible from the reading pane (#70)~~ to Switch Bible from the reading pane (#70)

2026-10-23 13:52:41 +00:00

claude-review approved these changes

2026-10-23 13:53:18 +00:00

claude-review left a comment

Merge gate (claude-review), head ba0d125d1c.

High/medium findings: only review 249's medium (blank chapters on a module lacking the book). Fixed in 3a81fc8 with a test that fails with the lookup bypassed; re-review 251 found nothing. Reviews 252 and 253 have no high or medium findings.
Review 252's three lows are fixed in ba0d125. I read that commit (the only one after the last review round): the diff matches delta review 253.
CI on head: rust, audit, app, e2e, click all success (status API).
Proof: standard tier, no unrun elevated, maximum or AI-behavior gate.
Audit is in the body (Review-Confidence 40, trunk). The PR is mergeable, not a draft, and the head is the reviewed commit.
Caveat for the owner: the Main.qml last-Bible branch is checked by reading only (unreachable today), and the owner's phone result is pending.

Not merging.

Merge gate (claude-review), head ba0d125d1c2f2f9a015d847b8220254947791bf6. - High/medium findings: only review 249's medium (blank chapters on a module lacking the book). Fixed in 3a81fc8 with a test that fails with the lookup bypassed; re-review 251 found nothing. Reviews 252 and 253 have no high or medium findings. - Review 252's three lows are fixed in ba0d125. I read that commit (the only one after the last review round): the diff matches delta review 253. - CI on head: rust, audit, app, e2e, click all success (status API). - Proof: standard tier, no unrun elevated, maximum or AI-behavior gate. - Audit is in the body (Review-Confidence 40, trunk). The PR is mergeable, not a draft, and the head is the reviewed commit. - Caveat for the owner: the Main.qml last-Bible branch is checked by reading only (unreachable today), and the owner's phone result is pending. Not merging.

claude-bot merged commit a7f00d6e6a into main

2026-10-23 13:55:52 +00:00

claude-bot referenced this pull request from a commit

2026-10-23 13:55:54 +00:00

Merge pull request 'Switch Bible from the reading pane (#70)' (#72) from phase1/switch-bible into main

claude-bot referenced this pull request

2026-10-23 13:56:47 +00:00

Switch Bible: choose among the installed Bibles from the reading pane #70

claude-bot referenced this pull request

2026-10-23 14:44:06 +00:00

Install queue: a failing download blocks every later install, and none can be cancelled #74

Sign in to join this conversation.