diff --git a/docs/features/ai-review.md b/docs/features/ai-review.md index f9c9f8b381..e5e0aff3e4 100644 --- a/docs/features/ai-review.md +++ b/docs/features/ai-review.md @@ -78,6 +78,7 @@ The **Edit prompt...** button opens the review instructions sent to the model. ` ## Safety rails - Nothing is applied automatically - you decide per suggestion. -- Suggestions that would add or remove formatting tags are discarded. +- ASSA override blocks at the start or end of a line (`{\pos(...)\fs54...}`) are never sent to the model - it only sees the text, and the blocks are restored unchanged around its correction. Lines that are pure drawings or tags are skipped. The ASSA actor and style name are sent as read-only context so the model knows who is speaking and whether a line is dialogue or a sign. +- Suggestions that would add or remove the remaining formatting tags (``, inline blocks) are discarded. - Suggestions that change a line's length a lot are flagged with a warning and start unchecked, since they are usually rewrites rather than corrections. - Replies from the model that do not follow the expected format are retried once and then skipped. diff --git a/src/ui/Features/Main/MainViewModel.cs b/src/ui/Features/Main/MainViewModel.cs index a95acc9101..1d5b266d9e 100644 --- a/src/ui/Features/Main/MainViewModel.cs +++ b/src/ui/Features/Main/MainViewModel.cs @@ -7219,7 +7219,7 @@ private async Task ShowAiAssistant() continue; } - var text = Subtitles[i].Text; + var text = StrippedLine.RemoveAllBlocks(Subtitles[i].Text); if (!string.IsNullOrWhiteSpace(text)) { context.AppendLine(text.Replace(Environment.NewLine, " ").Replace("\n", " ")); @@ -7229,9 +7229,12 @@ private async Task ShowAiAssistant() var languageName = TwoLetterCodeToLanguageName( LanguageAutoDetect.AutoDetectGoogleLanguageOrNull(GetUpdateSubtitle())); + // ASSA override blocks around the line stay on our side (#13927): the model only gets + // the text, and the blocks are glued back on around whatever it returns. + var stripped = StrippedLine.Strip(current.Text); var result = await ShowDialogAsync(vm => vm.Initialize( - current.Text, + stripped.Text, context.ToString().TrimEnd(), languageName, Se.Settings.General.SubtitleMaximumCharactersPerSeconds, @@ -7239,7 +7242,7 @@ private async Task ShowAiAssistant() if (result.ApplyPressed && !string.IsNullOrEmpty(result.ResultToApply)) { - current.Text = result.ResultToApply; + current.Text = stripped.Restore(result.ResultToApply); SelectAndScrollToRow(index); } } diff --git a/src/ui/Features/Tools/AiReview/AiReviewChunker.cs b/src/ui/Features/Tools/AiReview/AiReviewChunker.cs index 316683b6c0..c93755c708 100644 --- a/src/ui/Features/Tools/AiReview/AiReviewChunker.cs +++ b/src/ui/Features/Tools/AiReview/AiReviewChunker.cs @@ -3,7 +3,12 @@ namespace Nikse.SubtitleEdit.Features.Tools.AiReview; -public record ReviewLine(int Number, string Text); +/// +/// One line as the model sees it: is the tag-stripped text, and +/// / are optional read-only context (ASSA actor +/// field and style name) that tell the model who speaks and whether a line is dialogue or a sign. +/// +public record ReviewLine(int Number, string Text, string? Actor = null, string? Style = null); public class ReviewChunk { diff --git a/src/ui/Features/Tools/AiReview/AiReviewProtocol.cs b/src/ui/Features/Tools/AiReview/AiReviewProtocol.cs index 1039a4cec5..acfa686e3c 100644 --- a/src/ui/Features/Tools/AiReview/AiReviewProtocol.cs +++ b/src/ui/Features/Tools/AiReview/AiReviewProtocol.cs @@ -28,8 +28,9 @@ public static class AiReviewProtocol public const string ProtocolText = "\n\nThe user message is a JSON object. \"lines\" holds the subtitle lines to review, each with a line number \"n\" and its \"text\" " + - "(a \"\\n\" inside a text is a line break inside that subtitle and must be kept). \"context_before\" and \"context_after\" are " + - "read-only surrounding lines - never change or return those. A sentence may continue across several lines; correct it across " + + "(a \"\\n\" inside a text is a line break inside that subtitle and must be kept). A line may also carry an \"actor\" " + + "(who speaks) and a \"style\" (e.g. a sign or song style) - use them only as context, never return them. " + + "\"context_before\" and \"context_after\" are read-only surrounding lines - never change or return those. A sentence may continue across several lines; correct it across " + "the lines, but never move words from one line to another.\n" + "Answer with ONLY a JSON object, no other text: {\"changes\":[{\"n\":,\"orig\":\"\"," + "\"text\":\"\",\"reason\":\"\",\"category\":\"spelling|grammar|punctuation|casing|other\"}]}. " + @@ -65,6 +66,16 @@ private static void WriteLines(Utf8JsonWriter writer, string name, List _allSuggestions = new(); private Subtitle _subtitle = new(); + private SubtitleFormat? _subtitleFormat; + + /// Leading/trailing ASSA blocks cut off each sent line, keyed by line number, glued back on in . + private readonly Dictionary _strippedByNumber = new(); private string _languageCode = "en"; private CancellationTokenSource _cancellationTokenSource = new(); private bool _syncingSelection; @@ -154,6 +158,7 @@ public void Initialize( Action? applyCallback = null) { _subtitle = subtitle; + _subtitleFormat = subtitleFormat; _playLine = playLine; _stopPlayback = stopPlayback; _applyCallback = applyCallback; @@ -375,13 +380,19 @@ await MessageBox.Show(Window, Se.Language.General.Error, ProgressValue = 0; var lines = new List(); + _strippedByNumber.Clear(); + var isAssa = _subtitleFormat is AdvancedSubStationAlpha or SubStationAlpha; for (var i = 0; i < _subtitle.Paragraphs.Count; i++) { - var text = _subtitle.Paragraphs[i].Text; - if (!string.IsNullOrWhiteSpace(text)) + var p = _subtitle.Paragraphs[i]; + var stripped = StrippedLine.Strip(p.Text); + if (string.IsNullOrWhiteSpace(stripped.Text)) { - lines.Add(new ReviewLine(i + 1, text)); + continue; // empty, or a pure override/drawing line - nothing to proofread } + + _strippedByNumber[i + 1] = stripped; + lines.Add(new ReviewLine(i + 1, stripped.Text, p.Actor, isAssa ? p.Extra : null)); } var unitIds = AiReviewChunker.BuildUnitIds(lines); @@ -596,7 +607,9 @@ private void AddSuggestion(AiReviewChange change, Dictionary unitIdByN } var before = _subtitle.Paragraphs[paragraphIndex].Text; - var after = change.NewText; + var after = _strippedByNumber.TryGetValue(change.Number, out var stripped) + ? stripped.Restore(change.NewText) + : change.NewText; if (before.Trim() == after.Trim()) { return; diff --git a/src/ui/Features/Tools/AiReview/AssaTagStripper.cs b/src/ui/Features/Tools/AiReview/AssaTagStripper.cs new file mode 100644 index 0000000000..9a1e0c988e --- /dev/null +++ b/src/ui/Features/Tools/AiReview/AssaTagStripper.cs @@ -0,0 +1,65 @@ +using System.Text.RegularExpressions; + +namespace Nikse.SubtitleEdit.Features.Tools.AiReview; + +/// +/// Keeps ASSA override blocks ("{\pos(946.5,250.8)\fs54\1c&HFDF9AA&}") away from the model +/// (#13927): leading and trailing blocks are cut off before a line is sent and glued back on +/// verbatim around the model's answer. They carry no meaning for proofreading, cost a lot of +/// tokens, and small models "normalize" them so often that the tag guard used to drop the +/// whole correction. Blocks in the middle of the text are left in place - they are rare, and +/// the guard still verifies them. +/// +public readonly record struct StrippedLine(string Prefix, string Text, string Suffix) +{ + private static readonly Regex LeadingBlocks = new Regex(@"^(\s*\{\\[^}]*\})+", RegexOptions.Compiled); + private static readonly Regex TrailingBlocks = new Regex(@"(\{\\[^}]*\}\s*)+$", RegexOptions.Compiled); + private static readonly Regex AnyBlock = new Regex(@"\{\\[^}]*\}", RegexOptions.Compiled); + private static readonly Regex DrawingMode = new Regex(@"\\p[1-9]", RegexOptions.Compiled); + + public static StrippedLine Strip(string? input) + { + var text = input ?? string.Empty; + var prefix = string.Empty; + var suffix = string.Empty; + + var lead = LeadingBlocks.Match(text); + if (lead.Success) + { + if (DrawingMode.IsMatch(lead.Value)) + { + // vector drawing ("{\p1}m 0 0 l 100 0 ...") - the "text" is shape commands, not words + return new StrippedLine(text, string.Empty, string.Empty); + } + + prefix = lead.Value; + text = text.Substring(lead.Length); + } + + var trail = TrailingBlocks.Match(text); + if (trail.Success) + { + suffix = trail.Value; + text = text.Substring(0, trail.Index); + } + + // keep the whitespace next to the blocks with the blocks, so the restored line is byte-identical + var trimmedStart = text.TrimStart(); + prefix += text.Substring(0, text.Length - trimmedStart.Length); + var trimmed = trimmedStart.TrimEnd(); + suffix = trimmedStart.Substring(trimmed.Length) + suffix; + + return new StrippedLine(prefix, trimmed, suffix); + } + + public string Restore(string newText) + { + return Prefix + newText + Suffix; + } + + /// Removes every override block - for read-only context the model must not edit. + public static string RemoveAllBlocks(string? input) + { + return AnyBlock.Replace(input ?? string.Empty, string.Empty); + } +} diff --git a/src/ui/Logic/Config/SeAiReview.cs b/src/ui/Logic/Config/SeAiReview.cs index 4a7c2ba5d8..f6d76cdeb3 100644 --- a/src/ui/Logic/Config/SeAiReview.cs +++ b/src/ui/Logic/Config/SeAiReview.cs @@ -20,7 +20,7 @@ public class SeAiReview public static string DefaultPrompt => "You are a subtitle proofreader. Fix typos, spelling, grammar and punctuation in {language}." + "\n\nDo not rephrase, do not change meaning, tone or style. Keep names, slang and intentional dialect as they are. " + - "Keep all formatting tags (like or {\\an8}) and line breaks exactly as they are. Only correct actual errors."; + "Keep all formatting tags (like ) and line breaks exactly as they are. Only correct actual errors."; public SeAiReview() { diff --git a/tests/UI/Features/Tools/AiReview/AssaTagStripperTests.cs b/tests/UI/Features/Tools/AiReview/AssaTagStripperTests.cs new file mode 100644 index 0000000000..70857f5635 --- /dev/null +++ b/tests/UI/Features/Tools/AiReview/AssaTagStripperTests.cs @@ -0,0 +1,97 @@ +using Nikse.SubtitleEdit.Features.Tools.AiReview; +using System.Collections.Generic; + +namespace UITests.Features.Tools.AiReview; + +public class AssaTagStripperTests +{ + [Fact] + public void Strip_LeadingBlock_RestoresVerbatim() + { + const string input = @"{\bord0\blur0.8\pos(946.5,250.8)\fs54\fax0.13\frz348\fry358\frx4\1c&HFDF9AA&\3c&HFDF9AA&}Overboard"; + + var stripped = StrippedLine.Strip(input); + + Assert.Equal("Overboard", stripped.Text); + Assert.Equal(input, stripped.Restore(stripped.Text)); + Assert.Equal(@"{\bord0\blur0.8\pos(946.5,250.8)\fs54\fax0.13\frz348\fry358\frx4\1c&HFDF9AA&\3c&HFDF9AA&}Overbored", stripped.Restore("Overbored")); + } + + [Fact] + public void Strip_LeadingAndTrailingBlocks_WithWhitespace() + { + const string input = @"{\an8}{\i1} Hello there {\i0} "; + + var stripped = StrippedLine.Strip(input); + + Assert.Equal("Hello there", stripped.Text); + Assert.Equal(input, stripped.Restore(stripped.Text)); + } + + [Fact] + public void Strip_InlineBlock_StaysInText() + { + var stripped = StrippedLine.Strip(@"{\an8}He {\i1}really{\i0} said so."); + + Assert.Equal(@"He {\i1}really{\i0} said so.", stripped.Text); + Assert.Equal(@"{\an8}", stripped.Prefix); + } + + [Fact] + public void Strip_DrawingLine_IsEmpty() + { + Assert.Equal(string.Empty, StrippedLine.Strip(@"{\p1}m 0 0 l 100 0 100 100{\p0}").Text); + Assert.Equal(string.Empty, StrippedLine.Strip(@"{\pos(10,10)\p1}m 0 0 l 100 0{\p0}").Text); + } + + [Fact] + public void Strip_NoTags_Unchanged() + { + var stripped = StrippedLine.Strip("Plain text"); + + Assert.Equal("Plain text", stripped.Text); + Assert.Equal(string.Empty, stripped.Prefix); + Assert.Equal(string.Empty, stripped.Suffix); + } + + [Fact] + public void Strip_CurlyBracesWithoutBackslash_NotATag() + { + var stripped = StrippedLine.Strip("{laughs} Hello"); + + Assert.Equal("{laughs} Hello", stripped.Text); + } + + [Fact] + public void RemoveAllBlocks_RemovesInlineToo() + { + Assert.Equal("He really said so.", StrippedLine.RemoveAllBlocks(@"{\an8}He {\i1}really{\i0} said so.")); + } + + [Fact] + public void BuildUserContent_WritesActorAndStyleOnlyWhenPresent() + { + var chunk = new ReviewChunk(); + chunk.Lines.Add(new ReviewLine(1, "Overboard", "Narrator", "Sign")); + chunk.Lines.Add(new ReviewLine(2, "Hello", null, " ")); + + var json = AiReviewProtocol.BuildUserContent(chunk); + + Assert.Contains("\"n\":1,\"text\":\"Overboard\",\"actor\":\"Narrator\",\"style\":\"Sign\"", json); + Assert.Contains("\"n\":2,\"text\":\"Hello\"}", json); + Assert.DoesNotContain("\"actor\":\"\"", json); + } + + [Fact] + public void ParseChanges_StrippedText_EchoMatchesStrippedLine() + { + var editable = new Dictionary { { 1, "Overbored" } }; + + var changes = AiReviewProtocol.ParseChanges( + "{\"changes\":[{\"n\":1,\"orig\":\"Overbored\",\"text\":\"Overboard\",\"reason\":\"typo\",\"category\":\"spelling\"}]}", + editable); + + Assert.Single(changes); + Assert.Equal("Overboard", changes[0].NewText); + } +}