Fix trailing scene boundary normalisation
This commit is contained in:
parent
d32de17dcc
commit
845b42e5fe
@ -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":[""";
|
||||
|
||||
@ -15,40 +15,34 @@ public static class ChapterStructureBoundaryNormaliser
|
||||
|
||||
var issues = new List<ValidationIssue>();
|
||||
var boundaries = chapterStructure.SceneBoundaries;
|
||||
var phantomParagraph = paragraphCount + 1;
|
||||
|
||||
var trimmedTrailingEmptyCount = 0;
|
||||
while (boundaries.Count >= 2)
|
||||
{
|
||||
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))
|
||||
if (!IsDiscardableTrailingBoundary(final, previous, paragraphCount, phantomParagraph))
|
||||
{
|
||||
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."
|
||||
});
|
||||
break;
|
||||
}
|
||||
else if (IsTrailingTerminalOffByOne(final, previous, paragraphCount, phantomParagraph))
|
||||
{
|
||||
|
||||
boundaries = boundaries
|
||||
.Take(finalIndex)
|
||||
.Select((boundary, index) => CopyBoundary(boundary, index + 1))
|
||||
.ToList();
|
||||
trimmedTrailingEmptyCount++;
|
||||
}
|
||||
|
||||
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<ValidationIssue>();
|
||||
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(
|
||||
|
||||
@ -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;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user