Resolve out-of-range verse refs to the nearest preceding segment#147
Merged
Merged
Conversation
The host's next-verse button emits verseNum + 1 without clamping, so bumping forward from a chapter's last verse delivered a verse past the chapter's end; the loader only resolved verse-0 refs, and the segment window's anchor fallback jumped to the start of the book. Any unmatched verse now resolves to the nearest preceding segment start in its chapter, so the view holds at the chapter's end (and a verse inside a merged multi-verse segment resolves to its owning segment).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
imnasnainaec
approved these changes
Jul 6, 2026
imnasnainaec
left a comment
imnasnainaec
left a comment
Contributor
There was a problem hiding this comment.
@imnasnainaec reviewed 4 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
alex-rawlings-yyc
added a commit
that referenced
this pull request
Jul 6, 2026
#134) * Treat verse 0 as an ordinary verse; remove liveScrRef stickiness guard A chapter's pre-verse-1 superscription is a real, focusable verse-0 segment, but `liveScrRef` held a stickiness guard that swallowed any same-chapter `verseNum: 0` reference naming the verse already shown. That guard was added (b9dec18, "fix verse-0 echo nav") before verse 0 was a parsed verse, to absorb the host's spurious post-verse-nav chapter echo. It could not distinguish that echo from a genuine external `<` (previous-verse) from verse 1 — both are same-chapter, verse 0, and markerless on the global selector path — so it also ate the intentional `<`, leaving the extension stuck on verse 1 instead of moving to the superscription. Verified in a live session (probe logging liveScrRef/rawScrRef) that the host no longer emits the spurious echo: exactly one delivery per navigation, never an unsolicited trailing verse 0. With the echo gone the guard defended against nothing, so remove it. Verse 0 now passes through verbatim; the host's `<` from verse 1 lands on the chapter's superscription (the loader resolves verse 0 to the superscription segment, else to verse 1). The internal-nav marker machinery is retained — it still classifies internal/external navigations for the recenter fade. Only its former role inside the verse-0 guard is gone. Tests: collapse the two sticky-behavior tests into one pass-through regression test; flip the mid-reveal fade test to expect the curtain to re-engage for a verse-0 navigation arriving during fade-in (verse 0 is now an ordinary mid-reveal move). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix inaccurate doc * Get rid of unnecessary memoization of liveScrRef * Resolve out-of-range verse refs to the nearest preceding segment (#147) The host's next-verse button emits verseNum + 1 without clamping, so bumping forward from a chapter's last verse delivered a verse past the chapter's end; the loader only resolved verse-0 refs, and the segment window's anchor fallback jumped to the start of the book. Any unmatched verse now resolves to the nearest preceding segment start in its chapter, so the view holds at the chapter's end (and a verse inside a merged multi-verse segment resolves to its owning segment). * Merge the rawScrRef/liveScrRef pair into a single scrRef With verse-0 stickiness gone the two refs held the same object at every read, so one ref now serves both the duplicate-delivery identity guard and the mid-reveal prev comparison. The loader's activeScrRef owns the only raw→active mapping, which needs the loaded book anyway. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The host's next-verse button emits verseNum + 1 without clamping, so bumping forward from a chapter's last verse delivered a verse past the chapter's end; the loader only resolved verse-0 refs, and the segment window's anchor fallback jumped to the start of the book. Any unmatched verse now resolves to the nearest preceding segment start in its chapter, so the view holds at the chapter's end (and a verse inside a merged multi-verse segment resolves to its owning segment).
This change is