Reading window 3/3: picker and history #27
No reviewers
Labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
schaefera/utStudyBIble!27
Loading…
Reference in a new issue
No description provided.
Delete branch "phase1/picker-history"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
The reference button opens a book → chapter → verse picker (module-language names), and a hidden-by-default typing field (Settings) that jumps to "jn 3,16"-style references. Every jump is recorded in history, and Back returns to the previous place, even after the app is killed. Part of #11 (reading window 3/3).
Blast radius
Tier: trunk
Trunk paths touched: core/src/db/ (history.rs), core/src/worker.rs + worker/session*, qt/ glue (reading_model.rs, picker_model.rs, build.rs, lib.rs), qml/Main.qml, qml/pages/ReadingPane.qml, tools/mem-check/
Testing tier: standard (no migration: the history table is in 001; no dependency or CI-config change)
Gate / rollback: revert this merge. No schema change, and the rows written to the existing history table are harmless to the previous build.
Proof
cargo makeexit 0, core lib 64 passed / 0 failed, line coverage 96.3%, deny and audit okcargo make app-dockerexit 0 (format, qmllint, -Werror build, clang-tidy, mem-check)picker: saved BSB 265 0.455, left BSB 265 0.455, Back to BSB 265 0.455, on screen BSB 265 0.455after kill -9 with history: Back to BSB 265 0.455, on screen BSB 265 0.455hidden at first, shown after the setting, jn 3,16 jumped to John 3back_keeps_an_entry_this_bible_cannot_place,back_skips_an_entry_this_bible_cannot_place_to_the_one_below,one_number_after_a_one_chapter_book_is_the_verse) each fail on the old codecargo make click→utstudybible.schaefera_0.0.1_arm64.click: passc55be1f(the last code commit)send()clearserror) and Enter on an empty field doing nothing. No arm64 phone run.Review
Reviewer: fresh-context review at high (
7ce7a1a): 4 findings (1 medium, 3 low), all 4 fixed ina9c61e5. Re-review at medium (a9c61e5): 1 low (Back dead end), fixed after review inc55be1f(pop until placeable, restore the skipped entries). 0 declined, 0 deferred, 0 left open.Review-Confidence: 50/100. The core logic is well covered and CI is green. Points off for the two untested QML/glue fixes, the unreviewed
c55be1frewrite ofback(), and findings reported without line numbers.Confidence by part
core/src/bible/picker*,db/history.rs,worker/session/jump.rs): high. Unit and worker tests with mutation evidence.Session::backafterc55be1f: medium. Tested and audited, but no review round.picker_model.rs,reading_model.rserror clearing): medium. Only mem-check exercises it.PickerPage.qml,SettingsPage.qml, the ReadingPane tooltip): low. Read these: layout is checked only through mem-check taps at 540x960, and the error tooltip paths are untested.Follow-ups: #25 (gettext) edits
tools/mem-checkandworker/session.rstoo, so it gets a hand merge after this lands. Runcargo make i18nafter both are merged.🤖 Generated with Claude Code
WIP: Reading window 3/3: picker and historyto Reading window 3/3: picker and historyFresh-context review (high), reviewed
7ce7a1a— posted as a record by the gate reviewererrorchanges; a failed jump leaves the string as it was, so typing the same bad reference twice shows no feedback the second time.Files reviewed: CLAUDE.md, core/src/bible.rs, core/src/bible/picker.rs, core/src/bible/picker/lookup.rs, core/src/bible/target.rs, core/src/bible/place.rs, core/src/bible/reader.rs, core/src/db/history.rs, core/src/worker.rs, core/src/worker/session.rs, core/src/worker/session/jump.rs, qt/src/picker_model.rs, qt/src/reading_model.rs, qml/Main.qml, qml/pages/ReadingPane.qml, qml/pages/PickerPage.qml, qml/pages/SettingsPage.qml, tools/mem-check/run.sh, docs/superpowers/specs/2026-10-20-reading-window-design.md
Fresh-context review (medium), reviewed
a9c61e5— posted as a record by the gate reviewerFiles reviewed: core/src/bible/picker/lookup.rs, core/src/bible/picker.rs, core/src/worker.rs, core/src/worker/session.rs, core/src/worker/session/jump.rs, core/src/db/ (push_history, pop_history), qml/pages/PickerPage.qml, qml/pages/ReadingPane.qml (error use), qt/src/reading_model.rs
Gate review (medium)
Prior findings, checked against the code at head:
a9c61e5. ReadingModel::send() clearserror, so a repeat failure is a change. Glue/QML, no automated test (disclosed in the PR).a9c61e5, extended inc55be1f. Test back_keeps_an_entry_this_bible_cannot_place.a9c61e5. Test one_number_after_a_one_chapter_book_is_the_verse.a9c61e5, PickerPage.qml returns early on empty trimmed text.c55be1f.c55be1fverdict: no high or medium defect. Session::back pops until an entry resolves, then re-pushes the skipped entries oldest-first, so relative order is kept (they sit above the remaining ones, which matches their original order relative to the entry consumed). A pop error ends the loop, so no infinite loop. HISTORY_CAP trimming in push_history can't drop entries here because each restore follows an equal number of pops. The error shows only when nothing was placeable. The test back_skips_an_entry_this_bible_cannot_place_to_the_one_below checks the jump target, that the skipped entry is kept and history_len is 1.CI on
c55be1f: rust success, audit success, app success, click success.8bceea5(empty trailer commit): also success on all four jobs.Testing tier: standard is right. No migration (history table is in 001), no dependency or CI change. The PR Proof lists no unrun elevated, maximum or AI-behavior gate; the untested QML/glue paths and the missing phone run are disclosed under Not covered.