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

Mark paragraph-start verses from the module's markup (#12 part 3b) #114

Merged
claude-bot merged 1 commit from feat/paragraph-starts into main 2026-10-29 12:32:55 +00:00
Conversation 2 Commits 1 Files changed 8 +134 -12
claude-bot commented 2026-10-29 12:10:05 +00:00
Collaborator

Part of #12 (part 3b of the split in comment 2328).

What

The sword reader now uses the module's own markup to say which verses start a paragraph:

  • OSIS: an x-p milestone, a <p>, or a paragraph div, before the verse's first text.
  • ThML: a leading <p>.

sword::shown returns the runs plus this flag in one pass, and sword::runs stays as a wrapper. Reader::read sets Row.para_start, and a chapter's first verse always counts. Nothing changes on screen yet; 3c uses this for running text.

Blast radius

Tier: trunk
Trunk paths touched: sword/src/filter.rs, sword/src/filter/{osis,thml,runs}.rs, core/src/bible/{chapter,reader}.rs
Testing tier: standard (logic).
Gate / rollback: revert this merge. There is no schema, setting or UI change.

Proof

  • cargo make passed with SWORD_TEST_PATH set: fmt, clippy, tests with coverage, release check, deny and audit.
  • The golden tests (8) are unchanged and pass. The run text is untouched.
  • Fails without the change: with OSIS para_start forced false, 3 tests fail:
    • filter::osis::tests::paragraph_starts_come_before_the_first_text
    • synthetic marks_the_verses_that_start_a_paragraph
    • core bible::reader::tests::marks_paragraph_starts
  • KJV Genesis 1 paragraph starts: verses 1, 6, 9, 14, 24, 26 and 29.
  • Memory and start-up: one bool per Row and no new start-up work. The flag is worked out in the same token walk as the runs.
  • CI on 5ab2be5: changes, rust, audit, app, e2e and click all passed.

Review

Reviewer: fresh-context review at high on 5ab2be5 (review 315). No findings.
Review-Confidence: 100/100 (audit on 5ab2be5, CI green), with no deductions.

Confidence by part

  • OSIS paragraph detection: high. It has unit, synthetic and KJV tests. Modules with markup variants we haven't seen may still differ.
  • ThML leading <p>: medium. It is a simple prefix check, and the fails-without-change run didn't cover it.
  • Core plumbing: high.

🤖 Generated with Claude Code

Part of #12 (part 3b of the split in comment 2328). ## What The sword reader now uses the module's own markup to say which verses start a paragraph: - OSIS: an `x-p` milestone, a `<p>`, or a paragraph `div`, before the verse's first text. - ThML: a leading `<p>`. `sword::shown` returns the runs plus this flag in one pass, and `sword::runs` stays as a wrapper. `Reader::read` sets `Row.para_start`, and a chapter's first verse always counts. Nothing changes on screen yet; 3c uses this for running text. ## Blast radius **Tier:** trunk **Trunk paths touched:** `sword/src/filter.rs`, `sword/src/filter/{osis,thml,runs}.rs`, `core/src/bible/{chapter,reader}.rs` **Testing tier:** standard (logic). **Gate / rollback:** revert this merge. There is no schema, setting or UI change. ## Proof - `cargo make` passed with SWORD_TEST_PATH set: fmt, clippy, tests with coverage, release check, deny and audit. - The golden tests (8) are unchanged and pass. The run text is untouched. - **Fails without the change:** with OSIS `para_start` forced false, 3 tests fail: - `filter::osis::tests::paragraph_starts_come_before_the_first_text` - synthetic `marks_the_verses_that_start_a_paragraph` - core `bible::reader::tests::marks_paragraph_starts` - KJV Genesis 1 paragraph starts: verses 1, 6, 9, 14, 24, 26 and 29. - **Memory and start-up:** one bool per Row and no new start-up work. The flag is worked out in the same token walk as the runs. - **CI on 5ab2be5:** changes, rust, audit, app, e2e and click all passed. ## Review **Reviewer:** fresh-context review at high on 5ab2be5 (review 315). No findings. **Review-Confidence:** 100/100 (audit on 5ab2be5, CI green), with no deductions. ## Confidence by part - OSIS paragraph detection: high. It has unit, synthetic and KJV tests. Modules with markup variants we haven't seen may still differ. - ThML leading `<p>`: medium. It is a simple prefix check, and the fails-without-change run didn't cover it. - Core plumbing: high. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Mark which verses start a paragraph from the module's markup (#12 part 3b)
All checks were successful
CI / changes (pull_request) Successful in 6s
Details
CI / rust (pull_request) Successful in 5m38s
Details
CI / audit (pull_request) Successful in 12s
Details
CI / app (pull_request) Successful in 3m19s
Details
CI / e2e (pull_request) Successful in 8m15s
Details
CI / click (pull_request) Successful in 2m17s
Details
5ab2be55a9 
sword::shown returns the display runs and a verse-level para_start from the
same walk (OSIS x-p milestone, <p>, div paragraph; ThML leading <p>).
Reader::read fills Row.para_start; a chapter's first verse always counts.

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

Fresh-context review, level: high (trunk).
Reviewed commit: 5ab2be55a9
Range: origin/main...HEAD (1 commit)

Files reviewed:

  • core/src/bible/chapter.rs
  • core/src/bible/reader.rs
  • sword/src/filter.rs
  • sword/src/filter/osis.rs
  • sword/src/filter/runs.rs
  • sword/src/filter/thml.rs
  • sword/src/lib.rs
  • sword/tests/synthetic.rs

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

What I checked:

  • osis.rs: para_start is set in tag(), which only runs when no skip is active, so paragraph marks inside notes, titles, pre-verse matter and non-primary variants cannot count. seen_text is set only in text() (also not reached while skipping), so heading/note text before the mark does not hide it. <p eID/> and </p> do not count. extract() is unchanged: the new State fields default to false and display: None still avoids the run cost.
  • thml.rs: the <p prefix check requires >, space or / after it, so <pre>/<param> do not match. runs() still returns the same text as before.
  • reader.rs / chapter.rs: Row is built in one place only, and the qt glue reads fields by name, so the new field breaks no other constructor. Rows with no data get para_start true only for verse 1, as documented.
  • lib.rs / filter.rs: sword::runs is kept as a wrapper, so the golden tests (unchanged) and other callers still compile and behave the same.
  • Ran locally: marks_paragraph_starts against the cached KJV, and both new synthetic tests. All pass.

Verse granularity (a break after text within a verse is not recorded) is documented in Shown and covered by tests; the field has no consumer yet in this PR.

Fresh-context review, level: high (trunk). Reviewed commit: 5ab2be55a9fb4a2e2a2838165ae822d4f29f4993 Range: origin/main...HEAD (1 commit) Files reviewed: - core/src/bible/chapter.rs - core/src/bible/reader.rs - sword/src/filter.rs - sword/src/filter/osis.rs - sword/src/filter/runs.rs - sword/src/filter/thml.rs - sword/src/lib.rs - sword/tests/synthetic.rs Findings: none (no high, medium or low defect with a concrete failure scenario). What I checked: - osis.rs: `para_start` is set in `tag()`, which only runs when no skip is active, so paragraph marks inside notes, titles, pre-verse matter and non-primary variants cannot count. `seen_text` is set only in `text()` (also not reached while skipping), so heading/note text before the mark does not hide it. `<p eID/>` and `</p>` do not count. `extract()` is unchanged: the new State fields default to false and `display: None` still avoids the run cost. - thml.rs: the `<p` prefix check requires `>`, space or `/` after it, so `<pre>`/`<param>` do not match. `runs()` still returns the same text as before. - reader.rs / chapter.rs: `Row` is built in one place only, and the qt glue reads fields by name, so the new field breaks no other constructor. Rows with no data get `para_start` true only for verse 1, as documented. - lib.rs / filter.rs: `sword::runs` is kept as a wrapper, so the golden tests (unchanged) and other callers still compile and behave the same. - Ran locally: `marks_paragraph_starts` against the cached KJV, and both new synthetic tests. All pass. Verse granularity (a break after text within a verse is not recorded) is documented in `Shown` and covered by tests; the field has no consumer yet in this PR.
claude-review approved these changes 2026-10-29 12:31:48 +00:00
claude-review left a comment

Gate approval on head 5ab2be5. The single commit is the reviewed one and no commits came after the review. The review posted no findings, so none are open. CI is green on head for all six jobs (changes, rust, audit, app, e2e, click). Proof lists no unrun elevated, maximum or AI-behavior gate. The audit gives Review-Confidence 100.

Gate approval on head 5ab2be5. The single commit is the reviewed one and no commits came after the review. The review posted no findings, so none are open. CI is green on head for all six jobs (changes, rust, audit, app, e2e, click). Proof lists no unrun elevated, maximum or AI-behavior gate. The audit gives Review-Confidence 100.
claude-bot changed title from WIP: Mark paragraph-start verses from the module's markup (#12 part 3b) to Mark paragraph-start verses from the module's markup (#12 part 3b) 2026-10-29 12:32:43 +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!114
No description provided.