From 845b42e5fea39ce7b60722cf63023052f65f9e2b Mon Sep 17 00:00:00 2001 From: Nick Beckley Date: Sat, 29 Aug 2026 21:25:28 +0000 Subject: [PATCH] Fix trailing scene boundary normalisation --- PlotLine.Tests/Program.cs | 175 +++++++++++------- .../ChapterStructureBoundaryNormaliser.cs | 89 +++++---- .../ManuscriptStructureAnalysisService.cs | 12 ++ 3 files changed, 176 insertions(+), 100 deletions(-) diff --git a/PlotLine.Tests/Program.cs b/PlotLine.Tests/Program.cs index 26e5fb9..04d74c6 100644 --- a/PlotLine.Tests/Program.cs +++ b/PlotLine.Tests/Program.cs @@ -29,7 +29,11 @@ var tests = new (string Name, Action Test)[] ("Chapter Structure semantic validation still rejects out-of-range repaired confidence", ChapterStructureOutOfRangeRepairedConfidenceFailsValidation), ("Structural scene detection parser failure uses friendly diagnostics and retry", StructuralSceneDetectionUsesFriendlyParserDiagnosticsAndRetry), ("Chapter Structure boundary normaliser repairs gaps and overlaps", ChapterStructureBoundaryNormaliserRepairsGapsAndOverlaps), + ("Chapter Structure boundary normaliser preserves valid coverage", ChapterStructureBoundaryNormaliserPreservesValidCoverage), ("Chapter Structure boundary normaliser handles terminal ghost paragraph", ChapterStructureBoundaryNormaliserHandlesTerminalGhostParagraph), + ("Chapter Structure boundary normaliser discards multiple terminal ghost paragraphs", ChapterStructureBoundaryNormaliserDiscardsMultipleTerminalGhostParagraphs), + ("Chapter Structure boundary normaliser replays full book terminal empty failure", ChapterStructureBoundaryNormaliserReplaysFullBookTerminalEmptyFailure), + ("Chapter Structure boundary normaliser keeps meaningful end overflow repair", ChapterStructureBoundaryNormaliserKeepsMeaningfulEndOverflowRepair), ("Chapter Structure boundary normaliser preserves genuine inverted ranges for validation", ChapterStructureBoundaryNormaliserPreservesGenuineInvertedRanges), ("Shared parser reports raw chapter output on unrecoverable JSON", SharedParserReportsRawOutput), ("Character filtering rejects generic groups", CharacterFilteringRejectsGenericGroups), @@ -1822,81 +1826,108 @@ static void ChapterStructureBoundaryNormaliserRepairsGapsAndOverlaps() Assert(result.Issues.Count >= 2, "Boundary repairs should be reported as warnings."); } +static void ChapterStructureBoundaryNormaliserPreservesValidCoverage() +{ + var chapter = ChapterWithBoundaries( + Boundary(1, 1, 25), + Boundary(2, 26, 50), + Boundary(3, 51, 100)); + + var result = ChapterStructureBoundaryNormaliser.Normalise(chapter, 100); + var validation = new ChapterStructureValidator().Validate(result.ChapterStructure, 100); + ChapterStructureBoundaryNormaliser.AddIssuesTo(validation, result); + + Assert(validation.IsValid, "Valid exact coverage should remain valid."); + Assert(result.ChapterStructure?.SceneBoundaries is { Count: 3 }, "Valid exact coverage should preserve scene count."); + Assert(result.Issues.Count == 0, "Valid exact coverage should not create normalisation warnings."); +} + static void ChapterStructureBoundaryNormaliserHandlesTerminalGhostParagraph() { - var chapter = new ChapterStructureModel - { - SchemaVersion = "1.0", - ChapterSummary = "A chapter with a terminal ghost paragraph response.", - SceneBoundaries = - [ - new ChapterSceneBoundary - { - SceneNumber = 1, - StartParagraph = 1, - EndParagraph = 618, - StructuralSummary = "The only real scene covers the valid structural paragraphs.", - Confidence = 0.8m, - Reason = "Continuous scene." - }, - new ChapterSceneBoundary - { - SceneNumber = 2, - StartParagraph = 619, - EndParagraph = 618, - StructuralSummary = "Invalid terminal ghost paragraph.", - Confidence = 0.1m, - Reason = "Terminal off-by-one." - } - ] - }; + var chapter = ChapterWithBoundaries( + Boundary(1, 1, 618), + Boundary(2, 619, 618)); var result = ChapterStructureBoundaryNormaliser.Normalise(chapter, 618); var boundaries = result.ChapterStructure?.SceneBoundaries; + var validation = new ChapterStructureValidator().Validate(result.ChapterStructure, 618); + ChapterStructureBoundaryNormaliser.AddIssuesTo(validation, result); Assert(boundaries is { Count: 1 }, "Terminal 619 of 618 ghost boundary should be discarded."); boundaries = result.ChapterStructure!.SceneBoundaries!; Assert(boundaries[0].StartParagraph == 1 && boundaries[0].EndParagraph == 618, "Final valid scene should still end at structural paragraph 618."); - Assert(result.Issues.Any(issue => issue.Message.Contains("terminal off-by-one", StringComparison.OrdinalIgnoreCase)), "Terminal off-by-one repair should be reported."); + Assert(validation.IsValid, "Discarding a terminal ghost boundary should leave valid chapter coverage."); + Assert(result.Issues.Any(issue => issue.Message.Contains("Discarded", StringComparison.OrdinalIgnoreCase)), "Terminal ghost repair should be reported."); +} + +static void ChapterStructureBoundaryNormaliserDiscardsMultipleTerminalGhostParagraphs() +{ + var chapter = ChapterWithBoundaries( + Boundary(1, 1, 100), + Boundary(2, 101, 100), + Boundary(3, 101, 100)); + + var result = ChapterStructureBoundaryNormaliser.Normalise(chapter, 100); + var validation = new ChapterStructureValidator().Validate(result.ChapterStructure, 100); + ChapterStructureBoundaryNormaliser.AddIssuesTo(validation, result); + var boundaries = result.ChapterStructure?.SceneBoundaries; + + Assert(validation.IsValid, "Multiple terminal no-content boundaries should be discarded without failing validation."); + Assert(boundaries is { Count: 1 }, "Multiple terminal no-content boundaries should not become canonical scenes."); + Assert(result.Issues.Any(issue => issue.Message == "Discarded 2 empty trailing scene boundaries after paragraph 100."), "Multiple terminal discards should be auditable as a grouped warning."); +} + +static void ChapterStructureBoundaryNormaliserReplaysFullBookTerminalEmptyFailure() +{ + var chapter = ChapterWithBoundaries( + Boundary(1, 1, 11, "Returned 1"), + Boundary(2, 12, 41, "Returned 2"), + Boundary(3, 42, 66, "Returned 3"), + Boundary(4, 67, 107, "Returned 4"), + Boundary(5, 108, 154, "Returned 5"), + Boundary(6, 155, 200, "Returned 6"), + Boundary(7, 201, 276, "Returned 7"), + Boundary(8, 277, 332, "Returned 8"), + Boundary(9, 333, 346, "Returned 9"), + Boundary(10, 347, 366, "Returned 10"), + Boundary(11, 367, 366, "Returned 11"), + Boundary(12, 367, 366, "Returned 12")); + + var result = ChapterStructureBoundaryNormaliser.Normalise(chapter, 366); + var validation = new ChapterStructureValidator().Validate(result.ChapterStructure, 366); + ChapterStructureBoundaryNormaliser.AddIssuesTo(validation, result); + var boundaries = result.ChapterStructure?.SceneBoundaries; + + Assert(validation.IsValid, "Full-book terminal empty replay should validate after discarding empty trailing boundaries."); + Assert(boundaries is { Count: 10 }, "Full-book terminal empty replay should keep only the ten real scenes."); + boundaries = result.ChapterStructure!.SceneBoundaries!; + Assert(boundaries[9].StartParagraph == 347 && boundaries[9].EndParagraph == 366, "Final real scene coverage should remain unchanged."); + Assert(result.Issues.Any(issue => issue.Message == "Discarded 2 empty trailing scene boundaries after paragraph 366."), "Full-book terminal empty replay should record the discard count."); +} + +static void ChapterStructureBoundaryNormaliserKeepsMeaningfulEndOverflowRepair() +{ + var chapter = ChapterWithBoundaries( + Boundary(1, 1, 80), + Boundary(2, 81, 105)); + + var result = ChapterStructureBoundaryNormaliser.Normalise(chapter, 100); + var validation = new ChapterStructureValidator().Validate(result.ChapterStructure, 100); + ChapterStructureBoundaryNormaliser.AddIssuesTo(validation, result); + var boundaries = result.ChapterStructure?.SceneBoundaries; + + Assert(validation.IsValid, "Existing end-overflow clamp should remain valid when the range owns real chapter paragraphs."); + Assert(boundaries is { Count: 2 }, "A content-bearing overflow scene should not be discarded."); + boundaries = result.ChapterStructure!.SceneBoundaries!; + Assert(boundaries[1].StartParagraph == 81 && boundaries[1].EndParagraph == 100, "Content-bearing overflow should be clamped to the chapter end."); } static void ChapterStructureBoundaryNormaliserPreservesGenuineInvertedRanges() { - var chapter = new ChapterStructureModel - { - SchemaVersion = "1.0", - ChapterSummary = "A chapter with a malformed middle boundary.", - SceneBoundaries = - [ - new ChapterSceneBoundary - { - SceneNumber = 1, - StartParagraph = 1, - EndParagraph = 10, - StructuralSummary = "Opening valid scene summary with enough detail.", - Confidence = 0.8m, - Reason = "Opening." - }, - new ChapterSceneBoundary - { - SceneNumber = 2, - StartParagraph = 11, - EndParagraph = 10, - StructuralSummary = "Malformed scene summary with inverted range.", - Confidence = 0.8m, - Reason = "Malformed." - }, - new ChapterSceneBoundary - { - SceneNumber = 3, - StartParagraph = 12, - EndParagraph = 20, - StructuralSummary = "Final valid scene summary with enough detail.", - Confidence = 0.8m, - Reason = "Ending." - } - ] - }; + var chapter = ChapterWithBoundaries( + Boundary(1, 1, 10), + Boundary(2, 11, 10), + Boundary(3, 12, 20)); var normalised = ChapterStructureBoundaryNormaliser.Normalise(chapter, 20); var validation = new ChapterStructureValidator().Validate(normalised.ChapterStructure, 20); @@ -1906,6 +1937,26 @@ static void ChapterStructureBoundaryNormaliserPreservesGenuineInvertedRanges() Assert(validation.Errors.Any(error => error.Message.Contains("Start paragraph cannot be after end paragraph", StringComparison.Ordinal)), "Validation should report the inverted range."); } +static ChapterStructureModel ChapterWithBoundaries(params ChapterSceneBoundary[] boundaries) + => new() + { + SchemaVersion = "1.0", + ChapterSummary = "A chapter with enough structural context for validation.", + SceneBoundaries = boundaries.ToList() + }; + +static ChapterSceneBoundary Boundary(int sceneNumber, int start, int end, string? title = null) + => new() + { + SceneNumber = sceneNumber, + StartParagraph = start, + EndParagraph = end, + SuggestedTitle = title, + StructuralSummary = "This scene summary contains enough words for the chapter structure validator to accept it.", + Confidence = 0.8m, + Reason = "Structural boundary identified from the submitted chapter text." + }; + static void SharedParserReportsRawOutput() { const string raw = """{"schemaVersion":"1.0","sceneBoundaries":["""; diff --git a/PlotLine/Services/ChapterStructureBoundaryNormaliser.cs b/PlotLine/Services/ChapterStructureBoundaryNormaliser.cs index 4bf0064..8f7db31 100644 --- a/PlotLine/Services/ChapterStructureBoundaryNormaliser.cs +++ b/PlotLine/Services/ChapterStructureBoundaryNormaliser.cs @@ -15,40 +15,34 @@ public static class ChapterStructureBoundaryNormaliser var issues = new List(); var boundaries = chapterStructure.SceneBoundaries; - var finalIndex = boundaries.Count - 1; - var final = boundaries[finalIndex]; - var previous = boundaries[finalIndex - 1]; var phantomParagraph = paragraphCount + 1; - if (IsTrailingEmptyBoundary(final, previous, paragraphCount, phantomParagraph) - || IsTrailingInvalidPlaceholder(final, previous, paragraphCount, phantomParagraph)) + var trimmedTrailingEmptyCount = 0; + while (boundaries.Count >= 2) { + var finalIndex = boundaries.Count - 1; + var final = boundaries[finalIndex]; + var previous = boundaries[finalIndex - 1]; + if (!IsDiscardableTrailingBoundary(final, previous, paragraphCount, phantomParagraph)) + { + break; + } + boundaries = boundaries .Take(finalIndex) .Select((boundary, index) => CopyBoundary(boundary, index + 1)) .ToList(); - - issues.Add(new ValidationIssue - { - Severity = "Warning", - Path = $"sceneBoundaries[{finalIndex}]", - Message = BuildNormalisationMessage(final, finalIndex, paragraphCount, phantomParagraph), - SuggestedFix = "Do not return a final scene boundary unless it references an actual supplied paragraph." - }); + trimmedTrailingEmptyCount++; } - else if (IsTrailingTerminalOffByOne(final, previous, paragraphCount, phantomParagraph)) - { - boundaries = boundaries - .Take(finalIndex) - .Select((boundary, index) => CopyBoundary(boundary, index + 1)) - .ToList(); + if (trimmedTrailingEmptyCount > 0) + { issues.Add(new ValidationIssue { Severity = "Warning", - Path = $"sceneBoundaries[{finalIndex}]", - Message = $"Discarded trailing terminal off-by-one scene boundary {phantomParagraph}-{paragraphCount} because previous scene already ended at the final structural paragraph {paragraphCount}.", - SuggestedFix = $"Do not return a boundary starting after the final supplied paragraph {paragraphCount}." + Path = "sceneBoundaries", + Message = $"Discarded {trimmedTrailingEmptyCount:N0} empty trailing scene boundaries after paragraph {paragraphCount:N0}.", + SuggestedFix = $"Do not return scene boundaries after the final supplied paragraph {paragraphCount:N0} has already been covered." }); } @@ -131,6 +125,7 @@ public static class ChapterStructureBoundaryNormaliser } var issues = new List(); + var discardedTrailingEmptyCount = 0; var ordered = sourceBoundaries .Select((boundary, index) => new { Boundary = boundary, OriginalIndex = index }) .OrderBy(item => item.Boundary.StartParagraph ?? int.MaxValue) @@ -162,6 +157,13 @@ public static class ChapterStructureBoundaryNormaliser var start = boundary.StartParagraph.Value; var end = boundary.EndParagraph.Value; + if (expectedStart > paragraphCount && ContainsNoRemainingParagraphs(start, end, paragraphCount)) + { + discardedTrailingEmptyCount++; + changed = true; + continue; + } + if (start != expectedStart) { issues.Add(new ValidationIssue @@ -215,6 +217,22 @@ public static class ChapterStructureBoundaryNormaliser expectedStart = end + 1; } + if (discardedTrailingEmptyCount > 0) + { + issues.Add(new ValidationIssue + { + Severity = "Warning", + Path = "sceneBoundaries", + Message = $"Discarded {discardedTrailingEmptyCount:N0} empty trailing scene boundaries after paragraph {paragraphCount:N0}.", + SuggestedFix = $"Do not return scene boundaries after the final supplied paragraph {paragraphCount:N0} has already been covered." + }); + + for (var index = 0; index < repaired.Count; index++) + { + repaired[index] = CopyBoundary(repaired[index], index + 1); + } + } + if (expectedStart <= paragraphCount && repaired.Count > 0) { var final = repaired[^1]; @@ -257,6 +275,15 @@ public static class ChapterStructureBoundaryNormaliser }, issues); } + private static bool IsDiscardableTrailingBoundary( + ChapterSceneBoundary final, + ChapterSceneBoundary previous, + int paragraphCount, + int phantomParagraph) + => IsTrailingEmptyBoundary(final, previous, paragraphCount, phantomParagraph) + || IsTrailingInvalidPlaceholder(final, previous, paragraphCount, phantomParagraph) + || IsTrailingTerminalOffByOne(final, previous, paragraphCount, phantomParagraph); + private static bool IsTrailingEmptyBoundary( ChapterSceneBoundary final, ChapterSceneBoundary previous, @@ -310,22 +337,8 @@ public static class ChapterStructureBoundaryNormaliser || reason.Contains("coverage", StringComparison.OrdinalIgnoreCase); } - private static string BuildNormalisationMessage( - ChapterSceneBoundary final, - int finalIndex, - int paragraphCount, - int phantomParagraph) - { - var start = final.StartParagraph ?? 0; - var end = final.EndParagraph ?? 0; - if (start == phantomParagraph && end == phantomParagraph) - { - return $"Discarded trailing empty scene boundary {phantomParagraph}-{phantomParagraph} because submitted chapter has {paragraphCount} paragraphs and previous scene already ended at {paragraphCount}."; - } - - var sceneNumber = final.SceneNumber ?? finalIndex + 1; - return $"Discarded trailing invalid placeholder scene {sceneNumber} with range {start}-{end} because previous scene already ended at paragraph {paragraphCount}."; - } + private static bool ContainsNoRemainingParagraphs(int start, int end, int paragraphCount) + => start > paragraphCount || end <= paragraphCount; } public sealed record ChapterStructureBoundaryNormalisationResult( diff --git a/PlotLine/Services/ManuscriptStructureAnalysisService.cs b/PlotLine/Services/ManuscriptStructureAnalysisService.cs index 8326c0e..284870d 100644 --- a/PlotLine/Services/ManuscriptStructureAnalysisService.cs +++ b/PlotLine/Services/ManuscriptStructureAnalysisService.cs @@ -91,6 +91,18 @@ public sealed partial class ManuscriptStructureAnalysisService( var explicitPov = DetectExplicitNarrator(text); var validation = validator.Validate(chapter, paragraphs.Count); ChapterStructureBoundaryNormaliser.AddIssuesTo(validation, normalisation); + if (normalisation.Issues.Count > 0) + { + logger.LogWarning( + "Structural scene-boundary normalisation warnings. ProjectID={ProjectID} BookID={BookID} PreviewID={PreviewID} TemporaryChapterKey={TemporaryChapterKey} StructuralParagraphCount={StructuralParagraphCount} Warnings={Warnings}", + request.ProjectID, + request.BookID, + request.PreviewID, + request.TemporaryChapterKey, + paragraphs.Count, + string.Join("; ", normalisation.Issues.Select(issue => issue.Message))); + } + var persistedJson = normalisation.Issues.Count > 0 ? ChapterStructureBoundaryNormaliser.SerialiseParsedModel(chapter, normalisation, JsonOptions) : parseResult.RepairedJson ?? parseResult.RawJson;