core: reference parser for osisRef and TSK scripRef text (#13, 6b) #128
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!128
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/13-6b-refs"
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?
Refs #13 (PR 6b of 4)
What
This is a pure parser in
core/src/bible/refs. It turns note references into verse ranges, as JSword ordinals plus the KJV-mapped range. It reads two forms: osisRef attributes, and TSK's free-text<scripRef>, including relative forms like1:14,18and15. A reference that the versification lacks is kept and marked missing. Text it can't read is kept as text. Nothing calls the parser yet; 6c wires it into the reader.Blast radius
Tier: trunk
Trunk paths touched:
core/src/bible.rs,core/src/bible/refs.rs, andcore/src/bible/refs/{books,osis,sweep,text}.rs(all new)Testing tier: standard
Gate / rollback: revert this merge. There's no flag, because no runtime code calls the parser yet.
Budgets (docs/07): none. Nothing runs at start-up or on the reading path until 6c.
Proof
cargo makeon2a40743: fmt, clippy, tests (core lib 265 passed, sword lib 95, golden 15), release check, deny and audit all pass.2a40743: changes, rust, audit, app (with mem-check), e2e and click all succeeded.refs::sweep):f9e41d9implementation: 4 fail (a_suffixed_range…,chapters_ranges_and_single_chapter_books,unreadable_pieces_are_kept_as_text,osis::one_verse_and_ranges).66a9a70fix reverted:a_suffixed_range…fails.2a40743clause disabled: the same test fails.66a9a70build restored BSB John 10 (27575) and crossed John 10 ↔ 11 both ways. The text is unchanged and the journal has no errors.2a40743changes only the parser, which nothing calls yet.Choices and deviations:
Kap 6,5stay unparsed text for now (#117).Review
Reviewer:
f9e41d9, found 1 medium and 4 lows. All were fixed in865da84(comment 2504).865da84, had no findings.66a9a70.2a40743(comment 2512).Review-Confidence: 90/100, from the second audit. It took 5 off because the
2a40743fix to the delta review's finding got no further review. It took another 5 off for one speculative edge, which is still open. A comma piece shaped like "18 and 20" is read as an unknown book: it goes to unparsed and clears the context. It does not give a wrong verse. The TSK sweep has no such text.Confidence by part
🤖 Generated with Claude Code
Review level: high. Blast radius: trunk (core/src/bible.rs, core/src/bible/refs.rs, core/src/bible/refs/). Testing tier: standard.
Reviewed commit:
f9e41d9bcbRange: origin/main...HEAD (1 commit)
Files: core/src/bible.rs, core/src/bible/refs.rs, core/src/bible/refs/books.rs, core/src/bible/refs/osis.rs, core/src/bible/refs/sweep.rs, core/src/bible/refs/text.rs
I read every hunk in context, followed the code into sword::Versification (ordinal, chapter_count, verse_count, book_by_osis) and checked the docs (docs/04-features.md F6). The behaviours below were confirmed by running a throwaway probe test, now removed. The module is pure and nothing calls it yet, so nothing is on screen from this PR; the findings matter once the reader uses it.
Findings (all inline):
segment: an unreadable book name does not reset the book context, so a following relative reference is silently attached to the wrong book.split_at_books: a verse with a letter suffix after a comma is taken for a book name.Piece::parse: a letter suffix on the start of a range makes the whole piece unparsed.;.osis_item:!and@suffixes cut off the end of a range.Findings outside the diff: none. The docs say Elberfelder
Kap 6,5needs the same fallback, and the tests pin it as unparsed here. I assume a later 6x PR handles that, so it is not counted against this one.@ -0,0 +43,4 @@fn osis_item(item: &str) -> Option<Cite> {let item = item.split_once(':').map_or(item, |(_, r)| r);let item = item.split(['!', '@']).next()?;LOW.
item.split(['!', '@']).next()cuts the item at the first!/@, so a sub-ID on the start of a range drops the range's end.Gen.1.1!a-Gen.1.1!cbecomesGen.1.1alone, and the reference silently shrinks. Strip the suffix from each side of the-after splitting on-.@ -0,0 +15,4 @@impl Piece {fn parse(s: &str) -> Option<Piece> {// `16a` and `16ff` are verse 16.let s = s.trim().trim_end_matches(|c: char| c.is_alphabetic() || c == '.' || c.is_whitespace());LOW. The suffix trim runs only on the end of the whole piece, not on the start of a range. Probe:
"Joh 3:16a-18"is unparsed whole, becausenums("3:16a")fails. Trim the suffix per side of the range (asEnd::parsedoes for OSIS).@ -0,0 +67,4 @@let starts_book = |s: &str| {let s = s.trim_start();let rest = s.trim_start_matches(|c: char| c.is_ascii_digit());!s.is_empty() && rest.trim_start().starts_with(|c: char| c.is_alphabetic())LOW.
starts_booktreats9ffor18aas a book name, because digits are skipped and a letter follows. Probe:"Joh 3:16, 18a"givesJohn 3:16plus unparsed"18a", and"Ro 5:8, 9ff"givesRom 5:8plus unparsed"9ff". The verse is lost tounparsed. A book-leading digit is followed by a space or a letter run that is a book name, so a digit run followed only by a suffix likea/b/ff/fshould not count as a book start. The suffix is already handled for semicolon-separated pieces (see theJoh 3:16a; 1 Co 13:4ff.test).@ -0,0 +87,4 @@let mut chapter = if name.is_empty() {self.chapter} else {let Some(book) = books::osis(name) else {MEDIUM. When the book name is unknown, the segment is pushed to
unparsedand returns, butself.bookandself.chapterkeep the previous values. A later relative reference then resolves against the wrong book with no sign. Probe, entry John 3:"Zz 1:1; 8:32"givesunparsed ["Zz 1:1"]and a found refJohn 8:32. The author meant Zz 8:32. This breaks the module's promise that nothing is silently wrong. A modest example is a German or other-language abbreviation missing fromBOOKS. Fix: on an unknown name setself.book = None(andself.chapter = None) before returning, so the relative refs that follow go tounparsed. Add a test. Related: a name with no tail ("Ro"alone) sets the book but leaves the stale chapter, so a following bare15becomesRom <entry chapter>:15.@ -0,0 +104,4 @@match Piece::parse(text).and_then(|p| Self::cite(book, chapter, &p)) {Some(cite) => {// After a whole chapter, a bare number is another chapter.chapter = cite.last.verse.and(cite.last.chapter);LOW. Inconsistent chapter context. Inside one segment, after a whole-chapter ref a bare number is a chapter (
Ps 23, 24gives chapters 23 and 24). Across;it is a verse, becauseself.chapteris set toSome(23). Probe:"Ps 23; 5"givesPs 23thenPs 23:5. Pick one rule. The test comment says a bare number is a verse of the previous chapter, but the in-segment path disagrees.Round 1 answers, fixed in
865da84(all five; none deferred):Zz 1:1; 8:32gives two unparsed pieces. A bare book name (Ro) resets the chapter to none (or 1 for a one-chapter book). Tests added.18aread as a book): fixed. A piece that starts with digits then letters is a book only if digits follow its letters (2 Co 5);18aand9ffstay verses. Test added.Joh 3:16a-18). Test added.Ps 23; 5is Ps 23 and Ps 5). Test added.!/@cut a range): fixed. The range is split first and the suffix is dropped from each end (Gen.1.1!a-Gen.1.3!c). Test added.cargo makepasses (fmt, clippy, tests with coverage, release check, deny, audit). Trunk code changed, so a medium re-review applies.Review level: medium. Blast radius: trunk (core/src/bible/refs/). Testing tier: standard.
Reviewed commit:
865da848d6Range: f9e41d9bcbf5d34c35bcb8d9b89c77e1d833306b..HEAD (re-review of the fixes for review 330)
Files: core/src/bible/refs/osis.rs, core/src/bible/refs/text.rs
I read each hunk in context and traced it through RefParser::push, Piece::cite and the tests. All five earlier findings are addressed:
18aand9ffafter a comma are no longer taken for a book name.,and;alike.!and@suffixes are cut per range end.Each fix has a test. I found no defect with a concrete failure scenario.
Findings outside the diff: none.
Residual (not a defect I would block on):
split_at_booksno longer splits before a numbered book name that has no digits after it (Ps 23, 1 John). No known module text writes it that way, since TSK always follows a name with a chapter. The same goes for a prose segment such assee above, which now resets the context. Both are consistent with the new rule that unreadable text drops the context.claude-review, level medium (delta), commit
66a9a70108, range 865da84..66a9a70.Files reviewed: core/src/bible/refs/text.rs, core/src/bible/refs/books.rs (osis), core/src/bible/refs/osis.rs (context only).
The fix is correct for the audited case. "Joh 3:16, 18a-20" now stays one segment, and "Ro 5:8, 2 Co 5:19" still splits.
cargo test -p utsb-core --lib refspasses with the sweeps. The sweep rates are unchanged (TSK 99.9735 %, BSB 99.7614 %, GerElb1871 99.1968 %), so the sweeps don't exercise this path.Findings outside the diff: none.
Findings in the diff:
books::osisdoesn't know, after a comma, no longer splits. Example: "Ro 5:8, 2 Xx 5:19; 6". Before this commit the digit test split it off,segmenthit the unknown book and cleared book and chapter, and the later "6" went to unparsed. Now it stays in the Romans segment. The piece failsPiece::parseand is pushed to unparsed as "Ro 2 Xx 5:19", but self.book and self.chapter stay at Rom 5. The following bare "6" then resolves to Rom 5:6, a wrong cross-reference instead of an unparsed one. This needs an unknown numbered abbreviation after a comma, which is rare in TSK, so it is low. A fix would reset the context when the piece starts digits-then-letters andosisfails but the text is not a suffix pattern. For example, treat it as a book start when the letters after the digits are followed by a space or a digit rather than a-,,or the end.Not a defect: "9ff" and "18a" without a range still go through
rest.len() == s.len()orosis(...)=None and stay as verses.🤖 Generated with Claude Code
@ -0,0 +76,4 @@}// `18a`, `9ff` and `18a-20` are verses with a suffix: a piece that// starts with digits is a book only when it names one (`2 Co 5`).rest.len() == s.len() || books::osis(split_book(s).0).is_some()low: an unknown numbered book after a comma (e.g. "Ro 5:8, 2 Xx 5:19; 6") is no longer split, so the context is not reset and the later bare "6" resolves to Rom 5:6 instead of being unparsed.
Answers to the audit and review 332:
text.rsstarts_book: "Joh 3:16, 18a-20" was split off as a book named "18a-"). Fixed in66a9a70. A piece that starts with digits is now a book only when it names one. The testa_suffixed_range_after_a_comma_is_a_verse_rangefails on865da84.2a40743. A name, a space and a number also start a new reference, so the unknown book clears the context and both pieces go to unparsed. The added assertion fails without the change.f9e41d9implementation oftext.rsandosis.rsand the current tests, 4 tests fail:a_suffixed_range…,chapters_ranges_and_single_chapter_books,unreadable_pieces_are_kept_as_textandosis::one_verse_and_ranges.🤖 Generated with Claude Code
WIP: core: reference parser for osisRef and TSK scripRef text (#13, 6b)to core: reference parser for osisRef and TSK scripRef text (#13, 6b)Merge gate (medium) on
2a40743.Checked:
865da84. Review 331 re-reviewed that commit and found nothing, so no high or medium finding is open.66a9a70changesstarts_bookincore/src/bible/refs/text.rs. A piece that starts with digits now starts a book only ifbooks::osisknows the name, so "Joh 3:16, 18a-20" parses as a verse range.2a40743adds thenamedclause (name, whitespace, number). An unknown numbered book such as "2 Xx 5:19" now ends the reference, so the bare "6" no longer becomes Rom 5:6. Review 332's low is closed.text.rschanged after the last review round, and nothing calls the parser yet.2a40743: all six statuses are success (changes, rust, audit, app, e2e, click).🤖 Generated with Claude Code