Skip to content

Conversation

@Enkidu93
Copy link
Collaborator

@Enkidu93 Enkidu93 commented Jan 30, 2026

Fixes #806

I know this won't get into the release today but I wanted to at least push my work and get the review process started since it was slated for this sprint.

There are a lot of options regarding where to keep the refs property and where to replace it (e.g., do we migrate the db?). The work currently on this branch removes the generic ref property from the machine engine side of things, but still allows old pretranslations to be properly deserialized from the bucket and keeps the refs property in the model (just adding source and target ref properties and giving them preference when utilizing them on the usfm endpoint). I've also deprecated the refs property in the dto.


This change is Reviewable

@Enkidu93 Enkidu93 requested a review from ddaspit January 30, 2026 19:01
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2026

Codecov Report

❌ Patch coverage is 85.61644% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.53%. Comparing base (e195a31) to head (fe659b3).

Files with missing lines Patch % Lines
...rval.Translation/Services/PretranslationService.cs 69.04% 12 Missing and 1 partial ⚠️
...kit/Services/ParallelCorpusPreprocessingService.cs 86.48% 4 Missing and 1 partial ⚠️
...erviceToolkit/src/SIL.ServiceToolkit/Models/Row.cs 75.00% 2 Missing ⚠️
...Shared/Services/WordAlignmentPreprocessBuildJob.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #863      +/-   ##
==========================================
+ Coverage   66.46%   66.53%   +0.06%     
==========================================
  Files         382      382              
  Lines       20791    20882      +91     
  Branches     2714     2714              
==========================================
+ Hits        13819    13893      +74     
- Misses       6005     6022      +17     
  Partials      967      967              

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

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

Good job on this PR. I couldn't find any issues, which makes me a little nervous. 😁

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

Copy link
Collaborator Author

@Enkidu93 Enkidu93 left a comment

Choose a reason for hiding this comment

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

Hahaha, thank you! I don't know whether I should take that as a compliment or not 😆, but yes - you always feel better when there's at least one change requested haha.

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

@Enkidu93 Enkidu93 force-pushed the target_and_source_refs_for_pretranslations branch from 9e4038d to a488920 Compare January 30, 2026 20:07
@Enkidu93 Enkidu93 merged commit 4bafb5f into main Jan 30, 2026
2 of 3 checks passed
@Enkidu93 Enkidu93 deleted the target_and_source_refs_for_pretranslations branch January 30, 2026 20:36
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.

Reconsider whether pretranslations should be referenced in the target versification

4 participants