Add build warnings

[Enkidu93]

Fixes #768

I'd like to give this some more scrutiny, but I thought I should try to get it in so review wouldn't be blocked.

---

This change is 

[ddaspit]

@ddaspit reviewed 4 of 26 files at r1, all commit messages.
Reviewable status: 4 of 26 files reviewed, 2 unresolved discussions

---

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

public interface ILanguageTagService
{
    (bool LanguageInScriptIsKnown, bool ScriptIsKnown) ConvertToFlores200Code(

This should return an enum with three options rather than two booleans.

---

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

        if (!_languageTagService.ConvertToFlores200Code(sourceLanguageTag, out string resolvedCode).ScriptIsKnown)
        {
            warnings.Add($"The script for the source language '{resolvedCode}' is not in Flores-200");

Was this specific warning requested? I'm not sure I understand the specific purpose of this warning.

[Enkidu93]

Also fixes #802

Reviewable status: 3 of 26 files reviewed, 2 unresolved discussions (waiting on @ddaspit)

---

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

Previously, ddaspit (Damien Daspit) wrote…

This should return an enum with three options rather than two booleans.

Done. Open to suggestions on the enum name.

---

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

Previously, ddaspit (Damien Daspit) wrote…

Was this specific warning requested? I'm not sure I understand the specific purpose of this warning.

Yes, it was - or at least as I understood it. It's one of the three Nathaniel requested in issue #772: 1) low-data, 2) versification, 3) unsupported script. Michael also requested this a while back but we decided to hold off at the time and later, we rolled it into the build warnings.

This would be more common on the target side, if that's what you mean, but I figured it was worth including since setups like these are possible for adaptation-type projects.

[ddaspit]

@ddaspit reviewed 14 of 26 files at r1, 9 of 9 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Enkidu93)

---

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

Previously, Enkidu93 (Eli C. Lowry) wrote…

Yes, it was - or at least as I understood it. It's one of the three Nathaniel requested in issue #772: 1) low-data, 2) versification, 3) unsupported script. Michael also requested this a while back but we decided to hold off at the time and later, we rolled it into the build warnings.

This would be more common on the target side, if that's what you mean, but I figured it was worth including since setups like these are possible for adaptation-type projects.

I was confused by the two booleans. The enum made it clearer what the warning is for.

---

src/Machine/test/Serval.Machine.Shared.Tests/Services/LanguageTagServiceTests.cs line 56 at r2 (raw file):

        {
            Assert.That(internalCode, Is.EqualTo(resolvedLanguageCode));
            Assert.That(flores200Support, Is.EqualTo(support));

flores200Support is the expected value and support is the actual value. By convention, the variables should be swapped in the assertion.

---

src/Serval/src/Serval.Shared/Services/ZipParatextProjectFileHandler.cs line 3 at r2 (raw file):

namespace Serval.Shared.Services;

public class ZipParatextProjectFileHandler(IZipContainer container, ParatextProjectSettings? settings = null)

This change seems unrelated to the rest of the PR.

---

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

        await UpdateParallelCorpusAnalysisAsync(engineId, buildId, data, cancellationToken);

        if (trainCount == 0 && (!sourceTagInBaseModel || !targetTagInBaseModel))

I know this is unrelated to your current change, but this check is specific to the NMT engine and needs to be moved to NmtPreprocessBuildJob. Since you are already working in that part of the codebase, would you mind making the change? Thanks.

[Enkidu93]

This needs to be merged first: sillsdev/machine#354

Reviewable status: 19 of 27 files reviewed, 3 unresolved discussions (waiting on @ddaspit)

---

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

Previously, ddaspit (Damien Daspit) wrote…

I was confused by the two booleans. The enum made it clearer what the warning is for.

Great

---

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

Previously, ddaspit (Damien Daspit) wrote…

I know this is unrelated to your current change, but this check is specific to the NMT engine and needs to be moved to NmtPreprocessBuildJob. Since you are already working in that part of the codebase, would you mind making the change? Thanks.

Sure! Sorry - this took me a little longer than I thought. I realized that we really don't need the language code resolution except in NMT, so there was a bit more reworking than expected. Let me know how it looks to you.

---

src/Machine/test/Serval.Machine.Shared.Tests/Services/LanguageTagServiceTests.cs line 56 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

flores200Support is the expected value and support is the actual value. By convention, the variables should be swapped in the assertion.

Done.

---

src/Serval/src/Serval.Shared/Services/ZipParatextProjectFileHandler.cs line 3 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This change seems unrelated to the rest of the PR.

The PR in machine to add the build warnings also refactored the Zip/File classes, so it's necessary to make the corresponding updates in this PR. I've now pushed the complete changes.

[Enkidu93]

Sorry for the formatting-related commits. My csharpier is on the fritz - I think I've fixed it.

Reviewable status: 15 of 27 files reviewed, 3 unresolved discussions (waiting on @ddaspit)

[ddaspit]

@ddaspit reviewed 12 of 12 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)

---

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

Previously, Enkidu93 (Eli C. Lowry) wrote…

Sure! Sorry - this took me a little longer than I thought. I realized that we really don't need the language code resolution except in NMT, so there was a bit more reworking than expected. Let me know how it looks to you.

Thanks. It looks good.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 72.06478% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.13%. Comparing base (fa5448c) to head (9a70627).

Files with missing lines	Patch %	Lines
...hared/Services/ServalTranslationPlatformService.cs	0.00%	26 Missing ⚠️
...l.Shared/Services/ZipParatextProjectFileHandler.cs	41.86%	21 Missing and 4 partials ⚠️
...l.Machine.Shared/Services/NmtPreprocessBuildJob.cs	87.87%	6 Missing and 2 partials ⚠️
...anslation/Services/TranslationPlatformServiceV1.cs	42.85%	7 Missing and 1 partial ⚠️
...e.Shared/Services/TranslationPreprocessBuildJob.cs	93.75%	1 Missing ⚠️
...Shared/Services/WordAlignmentPreprocessBuildJob.cs	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
- Coverage   66.13%   66.13%   -0.01%     
==========================================
  Files         373      374       +1     
  Lines       20088    20273     +185     
  Branches     2625     2648      +23     
==========================================
+ Hits        13286    13407     +121     
- Misses       5866     5923      +57     
- Partials      936      943       +7     

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