sword: keep note bodies, labels, references and marker places (#13, 6a) #126

Merged

claude-bot merged 1 commit from feat/13-6a-notes-refs into main 2026-10-29 19:40:36 +00:00

Conversation 3 Commits 1 Files changed 19 +9249 -6289

claude-bot commented 2026-10-29 18:21:54 +00:00

Collaborator

• Copy link
• Copy Markdown

Refs #13 (PR 6a of 4)

What

This is the data layer only, in sword/; nothing new is shown yet. Extract::notes now holds each note whole: its kind, the module's own label, its text, and the references in it (the raw osisRef or passage plus the shown text). Shown::notes gives each marker's place as a byte offset into the runs' text. OSIS <reference> and ThML <scripRef> are read; relative references stay plain text (#117). The text Extract gives is unchanged.

Blast radius

Tier: trunk
Trunk paths touched: sword/src/filter.rs, sword/src/filter/notes.rs (new), osis.rs, runs.rs, thml.rs
Testing tier: standard
Gate / rollback: revert this merge. There is no flag: core reads only shown.runs and para_start (core/src/bible/reader.rs), so the app shows nothing new until 6c.

Budgets (docs/07): sword::shown now builds the note bodies for each verse it reads, and core drops them at once. That is a short-lived allocation of about 0.7–0.9 KB per chapter on average, and 3.8–5.2 KB in the busiest chapters (Dan 11 KJV, Ezek 40 BSB). Nothing new stays in memory until 6b and 6c keep the notes. CI's app job ran mem-check and passed against the 176000 kB baseline. Start-up work is unchanged.

Proof

• cargo make: fmt, clippy, tests (sword lib 95 passed; golden 15 passed, covering KJV, BSB, Darby, GerElb1871, TR, OSHB, GerSch and TSK), release check, deny and audit.
• Existing goldens are unchanged: the text, strongs, morph, red and notes forms are identical for every module.
• Fails without the change: I broke the code on purpose, by dropping refs and mis-mapping the marker offsets. 4 lib tests and 4 golden tests failed:
  • lib: markers_are_placed…, note_bodies_keep…, marks_follow_the_collapse, the thml notes test;
  • golden: bsb, gerelb1871, bsb_notes_keep_their_references, elberfelder_…, with nref differing in 689 BSB chapters and 306 GerElb1871 ones.
• The most notes in one verse: 4 in KJV (Gen 1:20), 5 in BSB, 5 in Darby (Exod 3:6) and 5 in GerElb1871 (Isa 11:11). So labels never run past "z".
• CI on 1ae0a0c: changes, rust, audit, app (with mem-check), e2e and click all succeeded.
• On the phone (comment 2479): the PR build restored the saved place and crossed John 10 ↔ 11 both ways. BSB John 11:33, a verse with a note, reads exactly as sword::shown gives it, with no note text in it. The journal has no errors.

Choices and deviations:

• Elberfelder is GerElb1871. GerElb1905 has no notes. Most of GerElb1871's references are relative (osisRef="", e.g. "Kap 6,5") and stay text; about 234 are real.
• Darby's <reference type="annotateRef"> has no osisRef, so it stays text.
• No module in the set uses the n label attribute, so label is covered by unit tests only.
• Adding Darby forced one change outside the brief: SWORD's plain text puts * … * around <hi type="bold">. Extract.text now does the same. Only Darby has bold, and the display runs are not affected.
• The fixtures are still hashes, with two new forms: nbody (label TAB text per note) and nref (target=text per note). They come from SWORD's Footnote entry attributes. As before, only the notes SWORD's XHTML shows are kept, so the notes in pre-verse Psalm titles are dropped.
• tools/golden/fetch.sh now fetches Darby and GerElb1871. The CI module cache is keyed on that file, so it refreshes. The comment in ci.yml still lists the old set; it's a comment only.

Review

Reviewer: a fresh-context review at high on 1ae0a0c (review 328) had no findings. It noted two observations, not defects:

• Note bodies are built on the reading path. Their size is now stated under Budgets above.
• An unclosed <reference> at the end of a note is dropped. That's harmless for well-formed modules.

Review-Confidence: 85/100. The audit took 15 off because the reviewer's report doesn't show it traced the caller (core/src/bible/reader.rs). The audit read the riskiest hunks itself and found no missed defect.

Confidence by part

• Note extraction and references: high. Diatheke-backed goldens cover four modules, and broken code fails them.
• Marker offsets (collapse_marked, ThML prefix offsets): medium. Diatheke has no marker offsets to compare against, so only unit tests cover them until 6c puts markers on screen.

🤖 Generated with Claude Code

Refs #13 (PR 6a of 4) ## What This is the data layer only, in `sword/`; nothing new is shown yet. `Extract::notes` now holds each note whole: its kind, the module's own label, its text, and the references in it (the raw osisRef or passage plus the shown text). `Shown::notes` gives each marker's place as a byte offset into the runs' text. OSIS `<reference>` and ThML `<scripRef>` are read; relative references stay plain text (#117). The text `Extract` gives is unchanged. ## Blast radius **Tier:** trunk **Trunk paths touched:** `sword/src/filter.rs`, `sword/src/filter/notes.rs` (new), `osis.rs`, `runs.rs`, `thml.rs` **Testing tier:** standard **Gate / rollback:** revert this merge. There is no flag: core reads only `shown.runs` and `para_start` (`core/src/bible/reader.rs`), so the app shows nothing new until 6c. **Budgets (docs/07):** `sword::shown` now builds the note bodies for each verse it reads, and core drops them at once. That is a short-lived allocation of about 0.7–0.9 KB per chapter on average, and 3.8–5.2 KB in the busiest chapters (Dan 11 KJV, Ezek 40 BSB). Nothing new stays in memory until 6b and 6c keep the notes. CI's `app` job ran mem-check and passed against the 176000 kB baseline. Start-up work is unchanged. ## Proof - **`cargo make`:** fmt, clippy, tests (sword lib 95 passed; golden 15 passed, covering KJV, BSB, Darby, GerElb1871, TR, OSHB, GerSch and TSK), release check, deny and audit. - **Existing goldens are unchanged:** the text, strongs, morph, red and notes forms are identical for every module. - **Fails without the change:** I broke the code on purpose, by dropping refs and mis-mapping the marker offsets. 4 lib tests and 4 golden tests failed: - lib: `markers_are_placed…`, `note_bodies_keep…`, `marks_follow_the_collapse`, the thml notes test; - golden: bsb, gerelb1871, `bsb_notes_keep_their_references`, `elberfelder_…`, with nref differing in 689 BSB chapters and 306 GerElb1871 ones. - **The most notes in one verse:** 4 in KJV (Gen 1:20), 5 in BSB, 5 in Darby (Exod 3:6) and 5 in GerElb1871 (Isa 11:11). So labels never run past "z". - **CI on 1ae0a0c:** changes, rust, audit, app (with mem-check), e2e and click all succeeded. - **On the phone (comment 2479):** the PR build restored the saved place and crossed John 10 ↔ 11 both ways. BSB John 11:33, a verse with a note, reads exactly as `sword::shown` gives it, with no note text in it. The journal has no errors. **Choices and deviations:** - Elberfelder is GerElb1871. GerElb1905 has no notes. Most of GerElb1871's references are relative (`osisRef=""`, e.g. "Kap 6,5") and stay text; about 234 are real. - Darby's `<reference type="annotateRef">` has no osisRef, so it stays text. - No module in the set uses the `n` label attribute, so `label` is covered by unit tests only. - Adding Darby forced one change outside the brief: SWORD's plain text puts `* … *` around `<hi type="bold">`. `Extract.text` now does the same. Only Darby has bold, and the display runs are not affected. - The fixtures are still hashes, with two new forms: nbody (label TAB text per note) and nref (target=text per note). They come from SWORD's Footnote entry attributes. As before, only the notes SWORD's XHTML shows are kept, so the notes in pre-verse Psalm titles are dropped. - `tools/golden/fetch.sh` now fetches Darby and GerElb1871. The CI module cache is keyed on that file, so it refreshes. The comment in `ci.yml` still lists the old set; it's a comment only. ## Review **Reviewer:** a fresh-context review at high on 1ae0a0c (review 328) had no findings. It noted two observations, not defects: - Note bodies are built on the reading path. Their size is now stated under Budgets above. - An unclosed `<reference>` at the end of a note is dropped. That's harmless for well-formed modules. **Review-Confidence:** 85/100. The audit took 15 off because the reviewer's report doesn't show it traced the caller (`core/src/bible/reader.rs`). The audit read the riskiest hunks itself and found no missed defect. ## Confidence by part - Note extraction and references: high. Diatheke-backed goldens cover four modules, and broken code fails them. - Marker offsets (`collapse_marked`, ThML prefix offsets): medium. Diatheke has no marker offsets to compare against, so only unit tests cover them until 6c puts markers on screen. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-10-29 18:21:54 +00:00

sword: keep note bodies, labels, references and marker places (#13, 6a) ... 1ae0a0cf51

Notes were skipped, leaving only a char per note. Extract::notes now
holds each note whole (Note: kind, the module's own n label, text, and
the references in it as raw osisRef/passage + shown text), and
Shown::notes places each marker as a byte offset into the runs' text,
carried through whitespace collapse. OSIS <reference osisRef> and ThML
<scripRef passage> are read; a relative reference (empty osisRef) stays
plain text. Extract's text output is unchanged.

Goldens: add Darby and GerElb1871 (GerElb1905 has no notes) and two
forms, nbody and nref (SWORD's Footnote entry attributes, markup
stripped, limited to the notes it shows). Darby also pinned SWORD's
asterisks around <hi type="bold"> in plain text, which was missing.

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

claude-review reviewed 2026-10-29 18:23:43 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Fresh-context review (claude-review). Level: high. Blast radius: trunk. Testing tiers: standard.

Reviewed commit: 1ae0a0cf51
Range: origin/main...HEAD

Files read in context (hunks plus surrounding code): sword/src/filter.rs, sword/src/filter/notes.rs, sword/src/filter/osis.rs, sword/src/filter/runs.rs, sword/src/filter/thml.rs, sword/src/lib.rs, sword/tests/common/forms.rs, sword/tests/golden.rs, tools/golden/dump.py, fetch.sh, run.sh. The .tsv fixtures were checked by running the golden tests against the local SWORD cache; I did not read them line by line.

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

What I checked:

• Note lifecycle in osis.rs and thml.rs: nesting depth, empty notes, stray end tags (no underflow, since a note is taken at depth 0), a note the entry ends inside, notes in skipped titles, pre-verse matter and non-primary variants (dropped as before). note_at and x.notes stay one to one, so the zip in shown is safe.
• Run::collapse_marked: each mark lands after the preceding word and before any pending space, trailing marks go to the end, and the offsets are byte-correct for multi-byte text. The divine-name uppercasing keeps display_len consistent, because it counts the bytes actually pushed to text.
• thml.rs marker offsets: the normalised-prefix length agrees with the final normalize_ws of the whole text (both trim leading space and drop trailing space). The prefix clone per note is quadratic only in notes per verse, which is negligible.
• Bold asterisks in plain text: they affect only Extract.text, not the display runs, and core only calls sword::shown, so reading is unchanged. The core crate compiles against the new Shown and Extract shapes.
• Ran cargo test -p sword --offline with the local module cache: all pass, including the KJV, BSB, Darby and GerElb1871 golden rows with the new nbody and nref forms, and busiest_verse_has_few_notes.

Observations, not defects: sword::shown now builds note bodies (strings) for every verse on the reading path, although core discards them. The cost is small, but the CLAUDE.md rule says to state any memory effect in the PR body, so I suggest a line there. An unclosed <reference> at the end of a note drops that reference silently, which is harmless for well-formed modules.

Fresh-context review (claude-review). Level: high. Blast radius: trunk. Testing tiers: standard. Reviewed commit: 1ae0a0cf519176573c47eabd96a69742ad0d733b Range: origin/main...HEAD Files read in context (hunks plus surrounding code): sword/src/filter.rs, sword/src/filter/notes.rs, sword/src/filter/osis.rs, sword/src/filter/runs.rs, sword/src/filter/thml.rs, sword/src/lib.rs, sword/tests/common/forms.rs, sword/tests/golden.rs, tools/golden/dump.py, fetch.sh, run.sh. The .tsv fixtures were checked by running the golden tests against the local SWORD cache; I did not read them line by line. Findings: none (no high, medium or low defects with a concrete failure scenario). What I checked: - Note lifecycle in osis.rs and thml.rs: nesting depth, empty notes, stray end tags (no underflow, since a note is taken at depth 0), a note the entry ends inside, notes in skipped titles, pre-verse matter and non-primary variants (dropped as before). `note_at` and `x.notes` stay one to one, so the `zip` in `shown` is safe. - `Run::collapse_marked`: each mark lands after the preceding word and before any pending space, trailing marks go to the end, and the offsets are byte-correct for multi-byte text. The divine-name uppercasing keeps `display_len` consistent, because it counts the bytes actually pushed to `text`. - thml.rs marker offsets: the normalised-prefix length agrees with the final `normalize_ws` of the whole text (both trim leading space and drop trailing space). The prefix clone per note is quadratic only in notes per verse, which is negligible. - Bold asterisks in plain text: they affect only `Extract.text`, not the display runs, and core only calls `sword::shown`, so reading is unchanged. The core crate compiles against the new `Shown` and `Extract` shapes. - Ran `cargo test -p sword --offline` with the local module cache: all pass, including the KJV, BSB, Darby and GerElb1871 golden rows with the new nbody and nref forms, and `busiest_verse_has_few_notes`. Observations, not defects: `sword::shown` now builds note bodies (strings) for every verse on the reading path, although core discards them. The cost is small, but the CLAUDE.md rule says to state any memory effect in the PR body, so I suggest a line there. An unclosed `<reference>` at the end of a note drops that reference silently, which is harmless for well-formed modules.

claude-bot commented 2026-10-29 19:38:28 +00:00

Author

Collaborator

• Copy link
• Copy Markdown

Phone check (docs/09) on the PR build, journal utsb: build 0.0.1 feat/13-6a-notes-refs@1ae0a0cf5:

• The app started and restored the saved place (restored BSB 27575 0.531, John 10), with the window loaded after 311 ms.
• Scrolling forward crossed John 10 → 11 (showing John 11) and saved the place each time. Scrolling back crossed 11 → 10 and returned to the same verse (saved BSB 27575 0.573).
• BSB John 11 has notes in 7 verses (2, 16, 18, 33, 44, 47, 56). I checked verse 33 on screen against sword::shown from this branch. The text is identical ("When Jesus saw her weeping, … deeply moved in spirit and troubled."), with no note text in it and normal spacing at the marker position. Reading looks the same as on main, as expected, since core doesn't use notes yet.
• The journal shows no errors or panics.

🤖 Generated with Claude Code

Phone check (docs/09) on the PR build, journal `utsb: build 0.0.1 feat/13-6a-notes-refs@1ae0a0cf5`: - The app started and restored the saved place (`restored BSB 27575 0.531`, John 10), with the window loaded after 311 ms. - Scrolling forward crossed John 10 → 11 (`showing John 11`) and saved the place each time. Scrolling back crossed 11 → 10 and returned to the same verse (`saved BSB 27575 0.573`). - BSB John 11 has notes in 7 verses (2, 16, 18, 33, 44, 47, 56). I checked verse 33 on screen against `sword::shown` from this branch. The text is identical ("When Jesus saw her weeping, … deeply moved in spirit and troubled."), with no note text in it and normal spacing at the marker position. Reading looks the same as on main, as expected, since core doesn't use notes yet. - The journal shows no errors or panics. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot referenced this pull request 2026-10-29 19:38:46 +00:00

Modules: warn about known flaws in a module (ASV: glued words at added words) #127

claude-review approved these changes 2026-10-29 19:38:52 +00:00

claude-review left a comment

• Copy link
• Copy Markdown

Gate approval (claude-review) on head 1ae0a0c.

• The one review round, on this same commit, had no findings, so there is nothing high or medium to verify. No commits came after it.
• CI is green on head: the commit status is success.
• Proof lists no unrun elevated, maximum or AI-behavior gate. The phone try is confirmed done.
• Review-Confidence audit: 85.

Open before merge, not blocking: the PR body should say that sword::shown now builds note bodies on the reading path. The reviewer asked for that line because of the memory-effect rule. The PR is also still a draft and titled "WIP:".

Gate approval (claude-review) on head 1ae0a0c. - The one review round, on this same commit, had no findings, so there is nothing high or medium to verify. No commits came after it. - CI is green on head: the commit status is success. - Proof lists no unrun elevated, maximum or AI-behavior gate. The phone try is confirmed done. - Review-Confidence audit: 85. Open before merge, not blocking: the PR body should say that `sword::shown` now builds note bodies on the reading path. The reviewer asked for that line because of the memory-effect rule. The PR is also still a draft and titled "WIP:".

claude-bot changed title from WIP: sword: keep note bodies, labels, references and marker places (#13, 6a) to sword: keep note bodies, labels, references and marker places (#13, 6a) 2026-10-29 19:40:29 +00:00

claude-bot merged commit cf0763de9c into main 2026-10-29 19:40:36 +00:00

claude-bot referenced this pull request from a commit 2026-10-29 19:40:37 +00:00

Merge pull request 'sword: keep note bodies, labels, references and marker places (#13, 6a)' (#126) from feat/13-6a-notes-refs into main

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

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!126

No description provided.