AI review/assistant: strip ASSA override blocks, send actor/style as context (#13927)

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 (`<i>`, 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.

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,17 +7229,20 @@ 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<AiAssistant.AiAssistantWindow, AiAssistant.AiAssistantViewModel>(vm =>
             vm.Initialize(
-                current.Text,
+                stripped.Text,
                 context.ToString().TrimEnd(),
                 languageName,
                 Se.Settings.General.SubtitleMaximumCharactersPerSeconds,
                 Se.Settings.General.SubtitleLineMaximumLength));
 
         if (result.ApplyPressed && !string.IsNullOrEmpty(result.ResultToApply))
         {
-            current.Text = result.ResultToApply;
+            current.Text = stripped.Restore(result.ResultToApply);
             SelectAndScrollToRow(index);
         }
     }

src/ui/Features/Tools/AiReview/AiReviewChunker.cs

@@ -3,7 +3,12 @@
 
 namespace Nikse.SubtitleEdit.Features.Tools.AiReview;
 
-public record ReviewLine(int Number, string Text);
+/// <summary>
+/// One line as the model sees it: <paramref name="Text"/> is the tag-stripped text, and
+/// <paramref name="Actor"/>/<paramref name="Style"/> 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.
+/// </summary>
+public record ReviewLine(int Number, string Text, string? Actor = null, string? Style = null);
 
 public class ReviewChunk
 {

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\":<line number>,\"orig\":\"<that line's text, copied unchanged>\"," +
         "\"text\":\"<full corrected text>\",\"reason\":\"<short reason>\",\"category\":\"spelling|grammar|punctuation|casing|other\"}]}. " +
@@ -65,6 +66,16 @@ private static void WriteLines(Utf8JsonWriter writer, string name, List<ReviewLi
             writer.WriteStartObject();
             writer.WriteNumber("n", line.Number);
             writer.WriteString("text", line.Text.Replace(Environment.NewLine, "\n"));
+            if (!string.IsNullOrWhiteSpace(line.Actor))
+            {
+                writer.WriteString("actor", line.Actor.Trim());
+            }
+
+            if (!string.IsNullOrWhiteSpace(line.Style))
+            {
+                writer.WriteString("style", line.Style.Trim());
+            }
+
             writer.WriteEndObject();
         }
 

src/ui/Features/Tools/AiReview/AiReviewViewModel.cs

@@ -74,6 +74,10 @@ public partial class AiReviewViewModel : ObservableObject
     private readonly IWindowService _windowService;
     private readonly List<ReviewSuggestionItem> _allSuggestions = new();
     private Subtitle _subtitle = new();
+    private SubtitleFormat? _subtitleFormat;
+
+    /// <summary>Leading/trailing ASSA blocks cut off each sent line, keyed by line number, glued back on in <see cref="AddSuggestion"/>.</summary>
+    private readonly Dictionary<int, StrippedLine> _strippedByNumber = new();
     private string _languageCode = "en";
     private CancellationTokenSource _cancellationTokenSource = new();
     private bool _syncingSelection;
@@ -154,6 +158,7 @@ public void Initialize(
         Action<Subtitle>? 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<ReviewLine>();
+        _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<int, int> 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;

src/ui/Features/Tools/AiReview/AssaTagStripper.cs

@@ -0,0 +1,65 @@
+using System.Text.RegularExpressions;
+
+namespace Nikse.SubtitleEdit.Features.Tools.AiReview;
+
+/// <summary>
+/// Keeps ASSA override blocks ("{\pos(946.5,250.8)\fs54\1c&amp;HFDF9AA&amp;}") 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.
+/// </summary>
+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;
+    }
+
+    /// <summary>Removes every override block - for read-only context the model must not edit.</summary>
+    public static string RemoveAllBlocks(string? input)
+    {
+        return AnyBlock.Replace(input ?? string.Empty, string.Empty);
+    }
+}

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 <i> or {\\an8}) and line breaks exactly as they are. Only correct actual errors.";
+        "Keep all formatting tags (like <i>) and line breaks exactly as they are. Only correct actual errors.";
 
     public SeAiReview()
     {

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("<i>Plain</i> text");
+
+        Assert.Equal("<i>Plain</i> 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<int, string> { { 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);
+    }
+}