Add support for per-chapter remarks

pmachapman · pmachapman · commit 0d209e32e05b · 2026-05-12T07:15:18.000+12:00
diff --git a/src/Serval/src/Serval.Translation/Services/UsfmGenerationService.cs b/src/Serval/src/Serval.Translation/Services/UsfmGenerationService.cs
@@ -76,24 +76,60 @@ public async Task<string> GetUsfmAsync(
         Build? build = (await _builds.GetAllAsync(b => b.EngineRef == engineId, cancellationToken))
             .OrderByDescending(b => b.DateFinished)
             .FirstOrDefault();
-        if (build is null || build.DateFinished is null)
+        if (build?.DateFinished is null)
             throw new InvalidOperationException($"Could not find any completed builds for engine '{engineId}'.");
 
-        string disclaimerRemark = string.Format(
-            CultureInfo.InvariantCulture,
-            AIDisclaimerRemark,
-            textId,
-            build.DateFinished.Value.ToUniversalTime().ToString("u")
-        );
         string markerPlacementRemark = GenerateMarkerPlacementRemark(
             paragraphMarkerBehavior,
             embedBehavior,
             styleMarkerBehavior
         );
 
-        List<string> remarks = [disclaimerRemark, markerPlacementRemark];
+        ParallelCorpusContract[] parallelCorpora = [.. _contractMapper.Map(build, engine)];
+
+        // Get the versification for the project
+        CorpusBundle corpusBundle = new(parallelCorpora);
+        ParallelCorpusContract corpusContract = corpusBundle.ParallelCorpora.Single(c => c.Id == corpusId);
+        CorpusFileContract sourceFile = corpusContract.SourceCorpora[0].Files[0];
+        ParatextProjectSettings? sourceSettings = corpusBundle.GetSettings(sourceFile.Location);
+        ScrVers versification = sourceSettings?.Versification ?? ScrVers.Original;
+        var scriptureRangeParser = new ScriptureRangeParser(versification);
+
+        // Generate remarks for every chapter in the book
+        List<(int, string)> remarks = [];
+        List<int>? chapters =
+            build
+                .Pretranslate?.SelectMany(p => p.SourceFilters ?? [])
+                .SelectMany(s =>
+                    scriptureRangeParser
+                        .GetChapters(s.ScriptureRange)
+                        .TryGetValue(textId, out List<int>? filterChapters)
+                        ? filterChapters
+                        : []
+                )
+                .ToList()
+            ?? [];
 
-        ParallelCorpusContract[] parallelCorpora = _contractMapper.Map(build, engine).ToArray();
+        // If there are no chapters, we need to set it to null so that the USFM updater
+        if (chapters.Count == 0)
+            chapters = null;
+
+        // Get all the chapters needing remarks
+        IEnumerable<int> chaptersNeedingRemarks =
+            chapters ?? Enumerable.Range(1, versification.GetLastChapter(Canon.BookIdToNumber(textId)));
+
+        // Add remarks to each chapter
+        foreach (int chapterNum in chaptersNeedingRemarks)
+        {
+            string disclaimerRemark = string.Format(
+                CultureInfo.InvariantCulture,
+                AIDisclaimerRemark,
+                $"{textId} {chapterNum}",
+                build.DateFinished.Value.ToUniversalTime().ToString("u")
+            );
+            remarks.Add((chapterNum, disclaimerRemark));
+            remarks.Add((chapterNum, markerPlacementRemark));
+        }
 
         IReadOnlyList<Pretranslation> pretranslations = await _pretranslations.GetAllAsync(
             pt =>
@@ -126,6 +162,7 @@ public async Task<string> GetUsfmAsync(
                 corpusId,
                 textId,
                 textOrigin == PretranslationUsfmTextOrigin.OnlyExisting ? [] : pretranslations,
+                chapters,
                 textBehavior,
                 Map(paragraphMarkerBehavior),
                 Map(embedBehavior),
@@ -146,6 +183,7 @@ public async Task<string> GetUsfmAsync(
                 corpusId,
                 textId,
                 textOrigin == PretranslationUsfmTextOrigin.OnlyExisting ? [] : pretranslations,
+                chapters,
                 Map(paragraphMarkerBehavior),
                 Map(embedBehavior),
                 Map(styleMarkerBehavior),
@@ -163,11 +201,12 @@ private static string UpdateSourceUsfm(
         string corpusId,
         string bookId,
         IReadOnlyList<Pretranslation> pretranslations,
+        IReadOnlyList<int>? chapters,
         UpdateUsfmMarkerBehavior paragraphBehavior,
         UpdateUsfmMarkerBehavior embedBehavior,
         UpdateUsfmMarkerBehavior styleBehavior,
         bool placeParagraphMarkers,
-        IEnumerable<string>? remarks,
+        IEnumerable<(int, string)>? remarks,
         string? targetQuoteConvention
     )
     {
@@ -176,6 +215,7 @@ private static string UpdateSourceUsfm(
             corpusId,
             bookId,
             pretranslations,
+            chapters,
             UpdateUsfmTextBehavior.StripExisting,
             paragraphBehavior,
             embedBehavior,
@@ -192,11 +232,12 @@ private static string UpdateTargetUsfm(
         string corpusId,
         string bookId,
         IReadOnlyList<Pretranslation> pretranslations,
+        IReadOnlyList<int>? chapters,
         UpdateUsfmTextBehavior textBehavior,
         UpdateUsfmMarkerBehavior paragraphBehavior,
         UpdateUsfmMarkerBehavior embedBehavior,
         UpdateUsfmMarkerBehavior styleBehavior,
-        IEnumerable<string>? remarks,
+        IEnumerable<(int, string)>? remarks,
         string? targetQuoteConvention
     )
     {
@@ -205,6 +246,7 @@ private static string UpdateTargetUsfm(
             corpusId,
             bookId,
             pretranslations,
+            chapters,
             textBehavior,
             paragraphBehavior,
             embedBehavior,
@@ -221,12 +263,13 @@ private static string UpdateUsfm(
         string corpusId,
         string bookId,
         IEnumerable<Pretranslation> pretranslations,
+        IReadOnlyList<int>? chapters,
         UpdateUsfmTextBehavior textBehavior,
         UpdateUsfmMarkerBehavior paragraphBehavior,
         UpdateUsfmMarkerBehavior embedBehavior,
         UpdateUsfmMarkerBehavior styleBehavior,
         IEnumerable<IUsfmUpdateBlockHandler>? updateBlockHandlers,
-        IEnumerable<string>? remarks,
+        IEnumerable<(int, string)>? remarks,
         string? targetQuoteConvention,
         bool isSource
     )
@@ -258,13 +301,15 @@ bool isSource
                     .Where(row => row.Refs.Any())
                     .OrderBy(row => row.Refs[0])
                     .ToArray(),
-                isSource ? sourceSettings?.FullName : targetSettings?.FullName,
+                chapters,
+                fullName: isSource ? sourceSettings?.FullName : targetSettings?.FullName,
                 textBehavior,
                 paragraphBehavior,
                 embedBehavior,
                 styleBehavior,
-                updateBlockHandlers: updateBlockHandlers,
-                remarks: remarks,
+                preserveParagraphStyles: null,
+                updateBlockHandlers,
+                remarks,
                 errorHandler: (_) => true,
                 compareSegments: isSource
             ) ?? "";
@@ -372,31 +417,11 @@ private static string DenormalizeQuotationMarks(string usfm, string quoteConvent
         int denormalizableChapterCount = bestChapterStrategies.Count(tup =>
             tup.Strategy != QuotationMarkUpdateStrategy.Skip
         );
-        List<string> remarks = [];
-        string quotationDenormalizationRemark;
-        if (denormalizableChapterCount == bestChapterStrategies.Count)
-        {
-            quotationDenormalizationRemark =
-                "The quote style in all chapters has been automatically adjusted to match the rest of the project.";
-        }
-        else if (denormalizableChapterCount > 0)
-        {
-            quotationDenormalizationRemark =
-                "The quote style in the following chapters has been automatically adjusted to match the rest of the project: "
-                + GetChapterRangesString(
-                    bestChapterStrategies
-                        .Where(tuple => tuple.Strategy != QuotationMarkUpdateStrategy.Skip)
-                        .Select(tuple => tuple.ChapterNumber)
-                        .ToList()
-                )
-                + ".";
-        }
-        else
-        {
-            quotationDenormalizationRemark =
-                "The quote style was not automatically adjusted to match the rest of your project in any chapters.";
-        }
-        remarks.Add(quotationDenormalizationRemark);
+        List<(int, string)> remarks = [];
+        const string QuotationDenormalizationRemark =
+            "The quote style of this chapter has been automatically adjusted to match the rest of the project.";
+        for (int i = 1; i <= denormalizableChapterCount; i++)
+            remarks.Add((i, QuotationDenormalizationRemark));
 
         var updater = new UpdateUsfmParserHandler(updateBlockHandlers: [quotationMarkDenormalizer], remarks: remarks);
         UsfmParser.Parse(usfm, updater);
@@ -405,43 +430,6 @@ private static string DenormalizeQuotationMarks(string usfm, string quoteConvent
         return usfm;
     }
 
-    internal static string GetChapterRangesString(List<int> chapterNumbers)
-    {
-        chapterNumbers = chapterNumbers.Order().ToList();
-        int start = chapterNumbers[0];
-        int end = chapterNumbers[0];
-        List<string> chapterRangeStrings = [];
-        foreach (int chapterNumber in chapterNumbers[1..])
-        {
-            if (chapterNumber == end + 1)
-            {
-                end = chapterNumber;
-            }
-            else
-            {
-                if (start == end)
-                {
-                    chapterRangeStrings.Add(start.ToString(CultureInfo.InvariantCulture));
-                }
-                else
-                {
-                    chapterRangeStrings.Add($"{start}-{end}");
-                }
-                start = chapterNumber;
-                end = chapterNumber;
-            }
-        }
-        if (start == end)
-        {
-            chapterRangeStrings.Add(start.ToString(CultureInfo.InvariantCulture));
-        }
-        else
-        {
-            chapterRangeStrings.Add($"{start}-{end}");
-        }
-        return string.Join(", ", chapterRangeStrings);
-    }
-
     /// <summary>
     /// Generate a natural sounding remark/comment describing marker placement.
     /// </summary>
diff --git a/src/Serval/test/Serval.ApiServer.IntegrationTests/TranslationEngineTests.cs b/src/Serval/test/Serval.ApiServer.IntegrationTests/TranslationEngineTests.cs
@@ -2291,10 +2291,10 @@ await _env.Builds.InsertAsync(
             usfm.Replace("\r\n", "\n"),
             Is.EqualTo(
                 @"\id MAT - Test1
-\rem This draft of MAT was generated using AI on 1970-01-01 00:00:00Z. It should be reviewed and edited carefully.
-\rem Paragraph breaks and embed markers were moved to the end of the verse. Style markers were removed.
 \h
 \c 1
+\rem This draft of MAT 1 was generated using AI on 1970-01-01 00:00:00Z. It should be reviewed and edited carefully.
+\rem Paragraph breaks and embed markers were moved to the end of the verse. Style markers were removed.
 \p
 \v 1 translation
 \v 2
diff --git a/src/Serval/test/Serval.Translation.Tests/Services/UsfmGenerationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/UsfmGenerationServiceTests.cs
