Support VerseRefs with segments across versifications

[Enkidu93]

Includes:

• Add test to cover alignment of verses across versifications with segments (e.g., a,b,c...).
• Strip segments when converting/comparing refs with verse segments across versifications because VerseRef does not support it properly

This fixes an issue Michael recently brought up on slack: https://sil-language-software.slack.com/archives/C06A8JFBU5S/p1778328868601589. (I can create a retroactive issue if you would like). I decided to prioritize this because depending on the problem, it could be causing catastrophic misalignment - especially for silnlp - and it was in this case.

The root of this problem is that ChangeVersification() (and by extension VerseRef.CompareTo()) exhibit very odd behavior when working with verse segments. Basically,

var vrS = new VerseRef("NUM 17:1a", ScrVers.Original);
var vr = new VerseRef("NUM 17:1", ScrVers.Original);

vrS.ChangeVersification(ScrVers.English);
vr.ChangeVersification(ScrVers.English);

// Now
// vr == new VerseRef("NUM 16:36", ScrVers.English) :heavy_check_mark: 
// but 
// vrS == new VerseRef("NUM 17:1a", ScrVers.English) :exploding_head: 

---

This change is 

[ddaspit]

Did you look into SIL.Scripture to see if segments are being handled in any specific way by the ChangeVersification method?

@ddaspit reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.

[Enkidu93]

Did you look into SIL.Scripture to see if segments are being handled in any specific way by the ChangeVersification method?

@ddaspit reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.

I did take a look - although it's difficult sometimes to dig through all the layers in SIL.Scripture to see what's really going on. From what I could tell, the look-ups for mappings between verses across versifications were exact such that if a verse had a verse segment, it didn't have a mapping to the other versification even if the verse without a verse segment did. I don't think there's any other explicit handling of verses with segments. My strategy here is to just strip the segments before converting since there isn't a guaranteed way to map those to another versification anyways.

[ddaspit]

I don't love entirely stripping off the segments. Maybe we could add them back in after changing the versification?

@ddaspit made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.

[Enkidu93]

I don't love entirely stripping off the segments. Maybe we could add them back in after changing the versification?

@ddaspit made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.

I did consider that. Is there a reliable way to do so? I don't think we're guaranteed that the segmentation of a verse in one versification is the same in another, are we? We're definitely not guaranteed that the segment naming is the same (i.e., in some versions, it's not a,b,c - I've seen other characters used, I'm pretty sure), but I'm not even sure we're guaranteed that they really refer to the same chunk of text.

[ddaspit]

There probably isn't a 100% reliable way to transfer the segments, but I feel like we have to do it anyway. In most cases, it would be more correct to transfer the segment, rather than strip it. Also, there are cases where the versification has mappings specifically for verse segments. We don't want to break those mappings, so we need to detect that case and use the normal change versification logic.

@ddaspit made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 88.63636% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.13%. Comparing base (dde7214) to head (b02ad59).

Files with missing lines	Patch %	Lines
src/SIL.Machine/Corpora/VerseRefExtensions.cs	87.17%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #415      +/-   ##
==========================================
+ Coverage   73.11%   73.13%   +0.01%     
==========================================
  Files         439      440       +1     
  Lines       36787    36823      +36     
  Branches     5058     5060       +2     
==========================================
+ Hits        26898    26931      +33     
- Misses       8777     8781       +4     
+ Partials     1112     1111       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Enkidu93]

I believe this is working now as we discussed yesterday. Explicit segment-to-segment (where the segments differ), cross-verse mappings will still not quite work (e.g., JON 1:1a = JON 1:3b), but I don't see any examples of this in the standard .vrs files nor do I expect it's likely (or likely to cause problems).

@Enkidu93 made 1 comment.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on ddaspit).

[ddaspit]

@ddaspit reviewed 7 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on Enkidu93).

---

src/SIL.Machine/Corpora/VerseRefComparer.cs line 23 at r3 (raw file):

        public int Compare(VerseRef x, VerseRef y)
        {
            y = y.ChangeVersificationWithSegments(x.Versification);

Should you check if the versification is different first?

---

src/SIL.Machine/Corpora/VerseRefExtensions.cs line 7 at r3 (raw file):

namespace SIL.Machine.Corpora
{
    public static class VerseRefExtensions

I like the use of extension methods for this.

---

src/SIL.Machine/Corpora/VerseRefExtensions.cs line 13 at r3 (raw file):

            if (string.IsNullOrEmpty(verseRef.Segment()))
            {
                return verseRef.Clone();

I don't think the Clone is necessary. VerseRef is a struct which is a value type. We probably do this in other places, but we probably shouldn't. It is just unnecessary overhead.