Skip to content

Use correct corpus versification for chapter calculation - #1021

Open
pmachapman wants to merge 1 commit into
mainfrom
corpus_versification
Open

Use correct corpus versification for chapter calculation#1021
pmachapman wants to merge 1 commit into
mainfrom
corpus_versification

Conversation

@pmachapman

@pmachapman pmachapman commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1013

This PR fixes a crash when drafting chapters that are not in the target versification by using the source versification when relevant.


This change is Reviewable

@pmachapman
pmachapman requested a review from Enkidu93 August 6, 2026 00:53
@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.99%. Comparing base (a75539c) to head (b91c695).

Files with missing lines Patch % Lines
.../src/Serval.Translation/Services/ContractMapper.cs 0.00% 0 Missing and 1 partial ⚠️
...rc/Serval.WordAlignment/Services/ContractMapper.cs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1021      +/-   ##
==========================================
- Coverage   72.00%   71.99%   -0.02%     
==========================================
  Files         386      386              
  Lines       20886    20876      -10     
  Branches     2778     2778              
==========================================
- Hits        15040    15030      -10     
  Misses       4737     4737              
  Partials     1109     1109              

☔ 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.

@pmachapman
pmachapman force-pushed the corpus_versification branch from d66b337 to b91c695 Compare August 6, 2026 01:58

@Enkidu93 Enkidu93 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. Thank you, Peter! Will SF use this chapter filtering correctly though in cases where the source and target versifications differ? E.g., if the source corpus has a book with chapters 1, 2, 3, but the target splits 3 into 3 and 4, will the scripture range provided be BOOK1-3 on the source and BOOK1-4 on the target? A bit of me thinks that we should actually have the scripture ranges just follow the first source project since the source is the anchor between training and inferencing (i.e. you need source material for both). (If I'm understanding correctly, this bug would also keep you from drafting chapters that don't exist in the target, right?) Regardless, we should specify this in the documentation. I could see it either way. One would be simpler at the monolingual corpus level, but the other would be more straightforward at the build level. What do you think?

Related to sillsdev/machine#421.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to generate chapters from a source with non-canonical chapters

3 participants