Update SIL.Machine to 3.9.0

[pmachapman]

Fixes #955. I also fixed a bug where Serval.Shared.Tests was appearing in codecov as code requiring coverage.

---

This change is 

[codecov-commenter]

Codecov Report

❌ Patch coverage is 75.22936% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.91%. Comparing base (7bcfe04) to head (c447b1b).

Files with missing lines	Patch %	Lines
...rc/Serval.Shared/Services/ParallelCorpusService.cs	78.87%	15 Missing ⚠️
...rval.Machine.Shared/Services/PreprocessBuildJob.cs	63.63%	10 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #999      +/-   ##
==========================================
+ Coverage   66.06%   69.91%   +3.84%     
==========================================
  Files         370      363       -7     
  Lines       21261    20134    -1127     
  Branches     2699     2686      -13     
==========================================
+ Hits        14047    14077      +30     
+ Misses       6201     5048    -1153     
+ Partials     1013     1009       -4     

☔ View full report in Codecov by Harness.
📢 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ddaspit]

@ddaspit reviewed 10 files and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on Enkidu93 and pmachapman).

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 151 at r1 (raw file):

            foreach (UsfmVersificationDiagnosticContract diagnostic in diagnostics)
            {
                var sb = new StringBuilder();

This would be easier to read if string interpolation was used.

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 169 at r1 (raw file):

                sb.Append(" of ");
                sb.Append(diagnostic.Filename);
                sb.Append("(parallel corpus ");

I think there should be a space here.

---

src/Serval/src/Serval.Shared/Services/ParallelCorpusService.cs line 36 at r1 (raw file):

            {
                using ZipArchive zipArchive = ZipFile.OpenRead(file.Location);
                UsfmVersificationAnalysis? analysis = new ZipUsfmVersificationAnalyzer(

I don't think this needs to be nullable.

---

src/Serval/src/Serval.Shared/Services/ParallelCorpusService.cs line 604 at r1 (raw file):

            foreach ((string textId, HashSet<int> chapters) in corpus.TrainOnChapters)
            {
                if (!bookIdsAndChapters.TryAdd(textId, chapters))

Is this correct? I can't really remember, but I think that if chapters is empty, then it means that all chapters should be included. Also, you should copy chapters. We probably need some test coverage on this method.

---

src/Serval/src/Serval.Shared/Services/ParallelCorpusService.cs line 621 at r1 (raw file):

            foreach ((string textId, HashSet<int> chapters) in corpus.InferenceChapters)
            {
                if (!bookIdsAndChapters.TryAdd(textId, chapters))

See comment above.

---

src/Serval/src/Serval.Shared.Contracts/UsfmVersificationDiagnosticContract.cs line 16 at r1 (raw file):

    public required UsfmVersificationDiagnosticType Type { get; init; }
    public required int NumAffectedVerses { get; init; }
    public required List<string> References { get; init; }

IReadOnlyList would be better.

[pmachapman]

@pmachapman made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on ddaspit and Enkidu93).

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 151 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This would be easier to read if string interpolation was used.

Done.

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 169 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I think there should be a space here.

Done.

---

src/Serval/src/Serval.Shared/Services/ParallelCorpusService.cs line 36 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I don't think this needs to be nullable.

Done. Thanks!

---

src/Serval/src/Serval.Shared/Services/ParallelCorpusService.cs line 604 at r1 (raw file):

Is this correct? I can't really remember, but I think that if chapters is empty, then it means that all chapters should be included.

Yes, that is correct. Updated.

Also, you should copy chapters.

Thanks - updated.

We probably need some test coverage on this method.

Done. I made the method internal and wrote a test to cover all of the combinations I could discover. I could also simplify the logic based on the tests`.

---

src/Serval/src/Serval.Shared/Services/ParallelCorpusService.cs line 621 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

See comment above.

Done.

---

src/Serval/src/Serval.Shared.Contracts/UsfmVersificationDiagnosticContract.cs line 16 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

IReadOnlyList would be better.

Done.

[ddaspit]

@ddaspit reviewed 5 files and all commit messages, made 1 comment, and resolved 6 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on Enkidu93).

[Enkidu93]

Thank you for doing this, Peter! Sorry that this fell on you while I was gone. I was hoping to also add a warning when there were different versifications among the Paratext projects represented in the training and inferencing data. (It's listed in the doc I shared). If you'd rather, I can add that in a separate PR. Or you could just add it here.

@Enkidu93 reviewed 10 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on pmachapman).

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 152 at r2 (raw file):

            {
                string diagnosticDetails =
                    (diagnostic.NumAffectedVerses > 1 ? $"for {diagnostic.NumAffectedVerses} verses " : string.Empty)

I know we haven't finalized things here, but if you could align the messages so they match, I'd appreciate it. I made you editor on the doc, so use your discretion.

[pmachapman]

I was hoping to also add a warning when there were different versifications among the Paratext projects represented in the training and inferencing data.

Done.

@pmachapman made 2 comments and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on pmachapman).

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 152 at r2 (raw file):

Previously, Enkidu93 (Eli C. Lowry) wrote…

I know we haven't finalized things here, but if you could align the messages so they match, I'd appreciate it. I made you editor on the doc, so use your discretion.

Done. I haven't implemented the Diagnostic class/record, just the message wording. Let me know if you want me to do the full structure or if we will save that for a later PR.

I had to make a couple of minor tweaks to wording in the document, and I added a question via a comment.

[Enkidu93]

@Enkidu93 reviewed 6 files and all commit messages, and made 2 comments.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on pmachapman).

---

src/Machine/src/Serval.Machine.Shared/Services/PreprocessBuildJob.cs line 152 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Done. I haven't implemented the Diagnostic class/record, just the message wording. Let me know if you want me to do the full structure or if we will save that for a later PR.

I had to make a couple of minor tweaks to wording in the document, and I added a question via a comment.

OK, sounds good. Thanks!