Structured build diagnostics by Enkidu93 · Pull Request #1012 · sillsdev/serval

Enkidu93 · 2026-07-28T20:30:34Z

Fixes #965.

See https://docs.google.com/document/d/1719NBOAx0NRvk8WpcciTqvFPPZDRSK-oZXbrIKUgL20/edit?tab=t.0#heading=h.dsj2hidolq5 for details.

This work so far does not include:

Adding tests
Updating documentation - I'm figuring we'll want this in the wiki?
The correct confidence threshold

This change is

codecov-commenter · 2026-07-28T20:34:56Z

Codecov Report

❌ Patch coverage is 39.44444% with 327 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.89%. Comparing base (8ddabc5) to head (28753aa).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...rval.Machine.Shared/Services/PreprocessBuildJob.cs	34.92%	121 Missing and 2 partials ⚠️
...src/Serval.Translation/Services/PlatformService.cs	26.92%	54 Missing and 3 partials ⚠️
...l.Machine.Shared/Services/NmtPreprocessBuildJob.cs	77.55%	18 Missing and 4 partials ⚠️
...Shared/Services/WordAlignmentPreprocessBuildJob.cs	0.00%	18 Missing ⚠️
...red/Services/ServalWordAlignmentPlatformService.cs	0.00%	12 Missing ⚠️
...ed/Services/EchoWordAlignmentPreprocessBuildJob.cs	44.44%	8 Missing and 2 partials ⚠️
...hared/Services/ServalTranslationPlatformService.cs	0.00%	10 Missing ⚠️
...e.Shared/Services/TranslationPreprocessBuildJob.cs	44.44%	8 Missing and 2 partials ⚠️
src/Serval/src/Serval.Client/Client.g.cs	16.66%	10 Missing ⚠️
...erval/src/Serval.Translation/Services/DtoMapper.cs	9.09%	10 Missing ⚠️
... and 9 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1012      +/-   ##
==========================================
- Coverage   71.72%   70.89%   -0.84%     
==========================================
  Files         370      377       +7     
  Lines       20287    20776     +489     
  Branches     2686     2711      +25     
==========================================
+ Hits        14551    14729     +178     
- Misses       4670     4975     +305     
- Partials     1066     1072       +6

☔ 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 31 files and all commit messages, and made 19 comments.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on Enkidu93).

a discussion (no related file):
Are you planning on adding unit tests? And yes, we will want to add a page for the diagnostics to the wiki page. It would probably be good to have a link to the wiki page in the Swagger docs.

src/Serval/src/Serval.Translation/Dtos/ExecutionDataDto.cs line 14 at r1 (raw file):

    [Obsolete]
    public IReadOnlyList<string> Warnings { get; init; } = [];
    public IReadOnlyList<DiagnosticDto> Diagnostics { get; init; } = [];

This should be optional.

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

            sourceLanguageTag,
            targetLanguageTag,
            true,

Make the parameter name explicit.

src/Serval/src/Serval.Translation/Services/PlatformService.cs line 475 at r1 (raw file):

            .Select(b => new Diagnostic
            {
                Code = "MODEL-004",

I think this is supposed to be MODEL-003.

src/Serval/src/Serval.Translation/Services/PlatformService.cs line 479 at r1 (raw file):

                Severity = Models.DiagnosticSeverity.Warn,
                Message =
                    $"The average pretranslation model confidence {b.averageConfidence} in book {b.bookId} is unusually low for the base model {ModelName}",

This confidence should be formatted.

src/Serval/src/Serval.Translation/Services/PlatformService.cs line 488 at r1 (raw file):

            });

        if (badBookConfidences.Any())

This deferred LINQ query is getting run multiple times.

src/Serval/src/Serval.Translation/Services/PlatformService.cs line 492 at r1 (raw file):

            Build? currentBuild = await _builds.GetAsync(b => b.Id == buildId, cancellationToken);

            await _builds.UpdateAsync(

Can we perform this update and the next in a single Mongo call?

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

            sourceLanguageTag,
            targetLanguageTag,
            true,

Make the parameter name explicit.

src/Serval/src/Serval.Translation/Dtos/DiagnosticDto.cs line 3 at r1 (raw file):

namespace Serval.Translation.Dtos;

public record DiagnosticDto

We should call this TranslationDiagnosticDto, so the name doesn't collide with the word alignment diagnostic in Serval.Client. ExecutionDataDto should really be renamed too, but that would be a breaking change at this point.

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

    }

    protected virtual IReadOnlyList<BuildDiagnostic> GetDiagnostics(

We have a MaxWarnings. We should probably have a MaxDiagnostics.

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

                string versificationName,
                IReadOnlyList<UsfmVersificationDiagnosticContract> usfmDiagnostics
            ) in ParallelCorpusService.AnalyzeUsfmVersification(parallelCorpora)

I don't love that we are running AnalyzeUsfmVersification twice for a build. Could we do this once and pass it to GetWarnings and GetDiagnostics or derive the warnings from the diagnostics?

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

                        Serval.Shared.Contracts.UsfmVersificationDiagnosticType.InvalidChapter => new BuildDiagnostic
                        {
                            Code = "USFM-001",

This is probably overkill, but I would feel a bit less anxious if the number part of the code was 4 digits.

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

        }

        if (projectVersifications.Count > 1)

I don't think this is correct. It is checking the number of projects, not the number of versifications.

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

                    Severity = BuildDiagnosticSeverity.Info,
                    Message =
                        $"There are multiple versifications represented among Paratext projects selected for training or inferencing: {string.Join(", ", projectVersifications)}.",

Are you intending to print out the dictionary entries?

src/Serval/src/Serval.WordAlignment/Dtos/WordAlignmentExecutionDataDto.cs line 14 at r1 (raw file):

    [Obsolete]
    public IReadOnlyList<string> Warnings { get; init; } = [];
    public IReadOnlyList<DiagnosticDto> Diagnostics { get; init; } = [];

This should be optional.

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

    private readonly ILanguageTagService _languageTagService = languageTagService;
    private const string ModelName = "NLLB";
    private const string MinimumTrainCount = "600"; //TODO move to options?

This should be an int and probably be an option.

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

                    Data = new Dictionary<string, object>
                    {
                        { "resolvedCode", sourceLanguageTag },

Is this supposed to be resolvedCode instead of sourceLanguageTag?

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

                    Data = new Dictionary<string, object>
                    {
                        { "resolvedCode", targetLanguageTag },

Is this supposed to be resolvedCode instead of targetLanguageTag?

src/Serval/src/Serval.WordAlignment/Dtos/DiagnosticDto.cs line 3 at r1 (raw file):

namespace Serval.WordAlignment.Dtos;

public record DiagnosticDto

This should be called WordAlignmentDiagnosticDto.

Enkidu93

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

a discussion (no related file):