schaefera/utStudyBIble
Watch
1
0
Fork
You've already forked utStudyBIble
 0
Code Issues 62 Pull requests 1 Projects Releases Packages Wiki Activity Actions 1

Back: do not lose a history entry on a kill or failed write (#69) #120

Merged
claude-bot merged 2 commits from fix/back-one-transaction into main 2026-10-29 16:11:56 +00:00
Conversation 3 Commits 2 Files changed 3 +87 -21
claude-bot commented 2026-10-29 15:45:42 +00:00
Collaborator

Closes #69

What

Back no longer loses a history entry on a kill or failed write. Session::back reads the newest entry with peek_history and deletes it with drop_history only once it is placed. An entry this Bible can't place is never deleted, and pop_history is removed.

Blast radius

Tier: trunk
Trunk paths touched: core/src/db/history.rs, core/src/worker.rs, core/src/worker/session/jump.rs
Testing tier: standard
Gate / rollback: revert this merge. There is no schema, migration or QML change. Memory and start-up are unaffected.

Proof

  • Fails without the change: the new test back_keeps_an_entry_this_bible_cannot_place_when_writes_fail injects a failing insert with a SQLite trigger. On base it fails (history_len 0, expected 1, and the log shows "can't keep the history entry: injected"). It passes now.
  • New db test: peek_leaves_the_entry_and_drop_removes_exactly_that_one.
  • Checks: cargo make passes locally (249 core tests), and CI is green on head 89a9113.
  • Phone (comment 2420), build fix/back-one-transaction@89a911375, checked with ute2e and the journal:
    • Go to John 3:16, then Rom 8:28.
    • Back goes to John 3:16.
    • After a kill and relaunch, John 3:16 is restored.
    • Back again goes to Matt 23:3.

Review

Reviewer: a fresh-context review on 89a9113 (review 322) found nothing.
Review-Confidence: 100/100. The trunk diff was fully reviewed at the head commit and CI is green. The reviewer's report did not state its effort level, so the auditor could not check the "below high" row and applied no deduction for it.

Confidence by part

  • db/history.rs peek and drop: high, covered by unit tests.
  • jump.rs Back flow: high, covered by the failing-insert test. If drop_history fails after the entry is placed, Back still moves and only logs the error, so the entry stays and a later Back offers it again. That needs a database write failure.

🤖 Generated with Claude Code

Closes #69 ## What Back no longer loses a history entry on a kill or failed write. `Session::back` reads the newest entry with `peek_history` and deletes it with `drop_history` only once it is placed. An entry this Bible can't place is never deleted, and `pop_history` is removed. ## Blast radius **Tier:** trunk **Trunk paths touched:** `core/src/db/history.rs`, `core/src/worker.rs`, `core/src/worker/session/jump.rs` **Testing tier:** standard **Gate / rollback:** revert this merge. There is no schema, migration or QML change. Memory and start-up are unaffected. ## Proof - **Fails without the change:** the new test `back_keeps_an_entry_this_bible_cannot_place_when_writes_fail` injects a failing insert with a SQLite trigger. On base it fails (`history_len` 0, expected 1, and the log shows "can't keep the history entry: injected"). It passes now. - **New db test:** `peek_leaves_the_entry_and_drop_removes_exactly_that_one`. - **Checks:** `cargo make` passes locally (249 core tests), and CI is green on head 89a9113. - **Phone** (comment 2420), build `fix/back-one-transaction@89a911375`, checked with ute2e and the journal: - Go to John 3:16, then Rom 8:28. - Back goes to John 3:16. - After a kill and relaunch, John 3:16 is restored. - Back again goes to Matt 23:3. ## Review **Reviewer:** a fresh-context review on 89a9113 (review 322) found nothing. **Review-Confidence:** 100/100. The trunk diff was fully reviewed at the head commit and CI is green. The reviewer's report did not state its effort level, so the auditor could not check the "below high" row and applied no deduction for it. ## Confidence by part - `db/history.rs` peek and drop: high, covered by unit tests. - `jump.rs` Back flow: high, covered by the failing-insert test. If `drop_history` fails after the entry is placed, Back still moves and only logs the error, so the entry stays and a later Back offers it again. That needs a database write failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
peek_history leaves the newest entry; drop_history removes one seq.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Back: drop the entry only once it is placed, no re-push (#69)
All checks were successful
CI / changes (pull_request) Successful in 11s
Details
CI / rust (pull_request) Successful in 4m45s
Details
CI / audit (pull_request) Successful in 9s
Details
CI / app (pull_request) Successful in 2m49s
Details
CI / e2e (pull_request) Successful in 8m52s
Details
CI / click (pull_request) Successful in 1m52s
Details
89a9113754 
An entry this Bible can't place is never deleted, so a kill or failed
write can't lose it. Test injects a failing insert with a trigger.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-review reviewed 2026-10-29 15:46:30 +00:00
claude-review left a comment

Fresh-context review (claude-review)

Level: high (blast radius: trunk)
Reviewed commit: 89a9113754
Range: origin/main...HEAD
Files reviewed: core/src/db/history.rs, core/src/worker.rs, core/src/worker/session/jump.rs (plus callers/callees: Session::jump, Session::load, history_event, save_now; docs/04-features.md Back section)

Findings: none (no high, medium or low defects with a concrete failure scenario).

Notes from reading the hunks in context:

  • Back now peeks the newest entry and drops it by seq only after it resolves and places in this Bible. The unplaceable path performs no writes, so the old pop-then-re-push loss when the history insert fails can no longer happen. No other callers of pop_history remain in code (only historical plan/spec docs mention it).
  • drop_history is a single DELETE statement, so it is atomic, and deleting an already-gone seq is a no-op (covered by the new db test). The worker runs single-threaded, so peek and drop cannot interleave with another push.
  • The new worker test (trigger rejecting inserts into history) would fail on the old code and passes on the new, and the test-only Db::pop helper keeps the existing ordering/cap tests meaningful.
  • Behaviour unchanged from before: a kill between the drop and save_now leaves the entry removed with the old saved position, as it did with the old pop.
Fresh-context review (claude-review) Level: high (blast radius: trunk) Reviewed commit: 89a9113754d878c8d36ebc0f1a2446c5271d1cbb Range: origin/main...HEAD Files reviewed: core/src/db/history.rs, core/src/worker.rs, core/src/worker/session/jump.rs (plus callers/callees: Session::jump, Session::load, history_event, save_now; docs/04-features.md Back section) Findings: none (no high, medium or low defects with a concrete failure scenario). Notes from reading the hunks in context: - Back now peeks the newest entry and drops it by seq only after it resolves and places in this Bible. The unplaceable path performs no writes, so the old pop-then-re-push loss when the history insert fails can no longer happen. No other callers of `pop_history` remain in code (only historical plan/spec docs mention it). - `drop_history` is a single DELETE statement, so it is atomic, and deleting an already-gone seq is a no-op (covered by the new db test). The worker runs single-threaded, so peek and drop cannot interleave with another push. - The new worker test (trigger rejecting inserts into history) would fail on the old code and passes on the new, and the test-only `Db::pop` helper keeps the existing ordering/cap tests meaningful. - Behaviour unchanged from before: a kill between the drop and `save_now` leaves the entry removed with the old saved position, as it did with the old pop.
claude-bot commented 2026-10-29 16:10:38 +00:00
Author
Collaborator

On the phone: build fix/back-one-transaction@89a911375 is on Plinius and stays installed. It started cleanly: the window loaded after 321 ms and restored BSB Matt 23:3.

I checked it myself with ute2e and the journal:

  • Go to John 3:16, then Rom 8:28 (jump from BSB 27182).
  • Back goes to John 3:16 (back to BSB 27182).
  • Kill and relaunch restores John 3:16, and the Back button is still shown.
  • Back again goes to Matt 23:3 (back to BSB 24916). The entry from before the kill was kept.

Nothing visible changed, so there is nothing for you to look at.

🤖 Generated with Claude Code

**On the phone:** build `fix/back-one-transaction@89a911375` is on Plinius and stays installed. It started cleanly: the window loaded after 321 ms and restored BSB Matt 23:3. I checked it myself with ute2e and the journal: - [x] Go to John 3:16, then Rom 8:28 (`jump from BSB 27182`). - [x] Back goes to John 3:16 (`back to BSB 27182`). - [x] Kill and relaunch restores John 3:16, and the Back button is still shown. - [x] Back again goes to Matt 23:3 (`back to BSB 24916`). The entry from before the kill was kept. Nothing visible changed, so there is nothing for you to look at. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-review approved these changes 2026-10-29 16:11:11 +00:00
claude-review left a comment

Gate approval on head 89a9113. The one review round (at this head) has no findings. No commits come after it. CI is green on head (changes, rust, audit, app, e2e, click). The audit gives Review-Confidence 100 with all three trunk files reviewed. Proof has a fails-without-change test and no unrun elevated, maximum or AI-behavior gate. The phone try is done. Not merging; the API reports mergeable=false, so check for a conflict with main before merging.

Gate approval on head 89a9113. The one review round (at this head) has no findings. No commits come after it. CI is green on head (changes, rust, audit, app, e2e, click). The audit gives Review-Confidence 100 with all three trunk files reviewed. Proof has a fails-without-change test and no unrun elevated, maximum or AI-behavior gate. The phone try is done. Not merging; the API reports mergeable=false, so check for a conflict with main before merging.
claude-bot changed title from WIP: Back: do not lose a history entry on a kill or failed write (#69) to Back: do not lose a history entry on a kill or failed write (#69) 2026-10-29 16:11:44 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
claude-review
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
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!120
No description provided.