Link every warning to the operator it came from (#440) (#446)

A warning did not record where it came from, so on a large plan there was no way to
get from a finding to the thing that produced it. pgfiore's framing was exactly
right: "if the plan is huge and the warning origin is murky, it would help to click
a warning and be beamed up to a specific step."

PlanWarning.OriginNodeIds carries it, and the interesting half of this feature is
knowing when to say nothing. Three honest answers, not one:

- A key lookup came from exactly one operator.
- A table variable warning came from every operator that touched one, which on a
  real plan is several. That rule already walked the tree and knew precisely which
  ones; it threw the answer away before emitting. It does not any more.
- "High Compile CPU" happened before a single row was read, and SQL Server reports
  "UDF Execution" at the statement level only. Those have NO operator origin and
  now say so, so the UI offers no link rather than one that goes somewhere
  arbitrary. Sending a reader to the wrong operator is worse than sending them
  nowhere, because they would believe it.

Operator warnings are stamped in ONE place, at the end of AnalyzeNode, rather than
at the 26 sites that add one - the same reasoning as the provenance stamp in #439
and the ceiling in #438. A rule you have to remember at every construction site is
a rule that eventually gets forgotten, and the once it is forgotten the UI quietly
drops a link that existed. It only fills what a rule left empty, so a rule that
knows better keeps its own answer.

The scope call worth reviewing. The pop-up the issue is about shows STATEMENT
warnings, and few of those can attribute to an operator - so linking only those
would not have served the huge-plan case that motivated the request at all. The
warnings that do have origins are the operator ones, and until now the only way to
see one was to have already clicked the operator carrying it, which is no help when
you do not know which operator to click. So the statement panel also gains an
"Operator Warnings" section indexing every warning in the tree, each one a link.
Nothing is removed from the per-operator panel; this is an index into it. It is
collapsed by default because on a large plan it is the longest section in the panel
and expanding it would push the statement's own details off screen, which is the
opposite of the problem being solved.

The tree walk lives in Core as WarningIndex rather than beside the panel that
renders it: it is a walk over Core's own models with nothing UI about it, and there
it can be tested without standing up Avalonia. It uses an explicit stack rather
than recursion, because a deep plan is precisely the case this feature exists for
and #430 was a crash caused by assuming operator trees are shallow.

CLI output contract: "origin_node_ids" is additive on every warning, so
ExpectedCompactOutputSha256 is rolled - second time, both additive, both
deliberate. Verified that origin_node_ids is the ONLY new key rather than assuming
it: the full key set on a warning is otherwise unchanged.

WarningBaseline.txt does not move. It digests type, severity and message, none of
which changed, so no committed plan's verdict is affected.

Tested: 314 total, 312 passed, 0 failed. The new tests pin both directions - that
across every committed plan no operator warning is left without an origin or points
away from its own node, that the table variable warning names the operators that
touch one, and that High Compile CPU and UDF Execution claim none. The index is
compared against an independent recursive walk rather than a hand-written count, so
it cannot drift as fixtures are added.

Not verified: the click itself. This box has no reachable display session, so
screencapture fails and I could not watch a warning navigate. The app was launched
on a plan carrying both kinds of warning and ran clean with no exceptions, and the
logic underneath is covered, but someone with a screen should confirm the scroll
lands where it should before this is trusted.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: erikdarlingdata

src/PlanViewer.App/Controls/PlanViewerControl.Interaction.cs

@@ -46,6 +46,49 @@ private void SelectNode(Border border, PlanNode node)
         UpdateMinimapSelection(node);
     }
 
+    /// <summary>
+    /// Selects the operator with <paramref name="nodeId"/> and scrolls it into view, so a warning
+    /// can take you to where it came from (#440). Returns false when the plan has no such operator,
+    /// which is what keeps a stale or wrong origin from silently scrolling somewhere arbitrary.
+    /// </summary>
+    private bool TryNavigateToNode(int nodeId)
+    {
+        foreach (var child in PlanCanvas.Children)
+        {
+            if (child is not Border border ||
+                !_nodeBorderMap.TryGetValue(border, out var node) ||
+                node.NodeId != nodeId)
+            {
+                continue;
+            }
+
+            SelectNode(border, node);
+            ScrollNodeIntoView(node);
+            return true;
+        }
+
+        return false;
+    }
+
+    /// <summary>
+    /// Centres the operator in the viewport. Node coordinates are unscaled layout positions, so they
+    /// are multiplied by the zoom level to get canvas pixels; the offset is then clamped, because
+    /// asking a ScrollViewer for a negative offset on a plan smaller than the viewport just leaves
+    /// it where it was and looks like the navigation did nothing.
+    /// </summary>
+    private void ScrollNodeIntoView(PlanNode node)
+    {
+        var targetX = node.X * _zoomLevel - (PlanScrollViewer.Bounds.Width / 2);
+        var targetY = node.Y * _zoomLevel - (PlanScrollViewer.Bounds.Height / 2);
+
+        var maxX = Math.Max(0, PlanScrollViewer.Extent.Width - PlanScrollViewer.Viewport.Width);
+        var maxY = Math.Max(0, PlanScrollViewer.Extent.Height - PlanScrollViewer.Viewport.Height);
+
+        PlanScrollViewer.Offset = new Vector(
+            Math.Clamp(targetX, 0, maxX),
+            Math.Clamp(targetY, 0, maxY));
+    }
+
     private ContextMenu BuildNodeContextMenu(PlanNode node)
     {
         var menu = new ContextMenu();

src/PlanViewer.App/Controls/PlanViewerControl.Properties.cs

@@ -822,13 +822,15 @@ private void ShowPropertiesPanel(PlanNode node)
                     var planWarnHeader = w.MaxBenefitPercent.HasValue
                         ? $"\u26A0 {w.WarningType}{sourceTag}{legacyTag} \u2014 up to {FormatBenefitPercent(w.MaxBenefitPercent.Value)}% benefit"
                         : $"\u26A0 {w.WarningType}{sourceTag}{legacyTag}";
-                    warnPanel.Children.Add(new TextBlock
+                    var planWarnHeaderBlock = new TextBlock
                     {
                         Text = planWarnHeader,
                         FontWeight = FontWeight.SemiBold,
                         FontSize = 11,
                         Foreground = new SolidColorBrush(Color.Parse(warnColor))
-                    });
+                    };
+                    AttachOriginNavigation(planWarnHeaderBlock, planWarnHeader, w.OriginNodeIds);
+                    warnPanel.Children.Add(planWarnHeaderBlock);
                     warnPanel.Children.Add(new TextBlock
                     {
                         Text = w.Message,
@@ -875,6 +877,78 @@ private void ShowPropertiesPanel(PlanNode node)
                 PropertiesContent.Children.Add(planWarningsExpander);
             }
 
+            /* === Operator Warnings (#440) ===
+               Every warning hanging off an operator anywhere in this statement, gathered in one
+               place and each one a link to its operator.
+
+               This section is the point of #440. The reporter's case was "the plan is huge and the
+               warning origin is murky", and the operator warnings are exactly the ones with a
+               murky origin - but until now the only way to see one was to already have clicked the
+               operator it was on, which is no help when you do not know which operator to click.
+               Nothing is removed from the per-operator panel; this is an index into it. */
+            var operatorWarnings = WarningIndex.CollectOperatorWarnings(s.RootNode);
+            if (operatorWarnings.Count > 0)
+            {
+                var operatorWarningsPanel = new StackPanel();
+                foreach (var (originNode, w) in operatorWarnings
+                             .OrderByDescending(x => x.Warning.MaxBenefitPercent ?? -1)
+                             .ThenByDescending(x => x.Warning.Severity)
+                             .ThenBy(x => x.Warning.WarningType))
+                {
+                    var opWarnColor = w.Severity == PlanWarningSeverity.Critical ? "#E57373"
+                        : w.Severity == PlanWarningSeverity.Warning ? "#FFB347" : "#6BB5FF";
+                    var opWarnPanel = new StackPanel { Margin = new Thickness(10, 2, 10, 2) };
+                    var opBenefit = w.MaxBenefitPercent.HasValue
+                        ? $" \u2014 up to {FormatBenefitPercent(w.MaxBenefitPercent.Value)}% benefit"
+                        : "";
+                    var opHeaderText =
+                        $"\u26A0 {w.WarningType}{WarningSourceTag(w)}{(w.IsLegacy ? " [legacy]" : "")}{opBenefit}";
+                    var opHeader = new TextBlock
+                    {
+                        Text = opHeaderText,
+                        FontWeight = FontWeight.SemiBold,
+                        FontSize = 11,
+                        Foreground = new SolidColorBrush(Color.Parse(opWarnColor))
+                    };
+                    AttachOriginNavigation(opHeader, opHeaderText, w.OriginNodeIds);
+                    opWarnPanel.Children.Add(opHeader);
+                    opWarnPanel.Children.Add(new TextBlock
+                    {
+                        Text = OperatorOriginLabel(originNode),
+                        FontSize = 11,
+                        Foreground = SectionHeaderBrush,
+                        Margin = new Thickness(16, 0, 0, 0)
+                    });
+                    operatorWarningsPanel.Children.Add(opWarnPanel);
+                }
+
+                var operatorWarningsExpander = new Expander
+                {
+                    /* Collapsed by default, unlike Plan Warnings. On a large plan this is the
+                       longest section in the panel, and expanding it by default would push the
+                       statement's own details off screen - the opposite of the problem #440 is
+                       about. */
+                    IsExpanded = false,
+                    Header = new TextBlock
+                    {
+                        Text = $"Operator Warnings ({operatorWarnings.Count})",
+                        FontWeight = FontWeight.SemiBold,
+                        FontSize = 11,
+                        Foreground = SectionHeaderBrush
+                    },
+                    Content = operatorWarningsPanel,
+                    Margin = new Thickness(0, 2, 0, 0),
+                    Padding = new Thickness(0),
+                    Foreground = SectionHeaderBrush,
+                    Background = new SolidColorBrush(Color.FromArgb(0x18, 0x4F, 0xA3, 0xFF)),
+                    BorderBrush = PropSeparatorBrush,
+                    BorderThickness = new Thickness(0, 0, 0, 1),
+                    HorizontalAlignment = HorizontalAlignment.Stretch,
+                    HorizontalContentAlignment = HorizontalAlignment.Stretch
+                };
+                PropertiesContent.Children.Add(operatorWarningsExpander);
+            }
+
             // === Missing Indexes ===
             if (s.MissingIndexes.Count > 0)
             {

src/PlanViewer.App/Controls/PlanViewerControl.Rendering.cs

@@ -516,6 +516,46 @@ private static string FormatBytes(double bytes)
     private static string FormatBenefitPercent(double pct) =>
         pct >= 100 ? $"{pct:N0}" : $"{pct:N1}";
 
+    /// <summary>
+    /// How an operator is named in the aggregated warning index (#440) — enough to recognise it
+    /// before clicking, matching what the operator's own panel puts in its header.
+    /// </summary>
+    private static string OperatorOriginLabel(PlanNode node) =>
+        string.IsNullOrEmpty(node.FullObjectName)
+            ? $"Node {node.NodeId} \u00B7 {node.PhysicalOp}"
+            : $"Node {node.NodeId} \u00B7 {node.PhysicalOp} on {node.FullObjectName}";
+
+    /// <summary>
+    /// Turns a warning header into a link to the operator it came from (#440).
+    ///
+    /// <para>Only when the warning actually knows. Findings with no operator origin — "High Compile
+    /// CPU" happened before a row was read — are left as plain text rather than given a link that
+    /// goes somewhere arbitrary, because a reader would believe it.</para>
+    ///
+    /// <para>Where a warning came from several operators, the first is the navigation target and the
+    /// rest are named in the tooltip, so the count is visible rather than silently dropped.</para>
+    /// </summary>
+    private void AttachOriginNavigation(TextBlock header, string headerText, List<int> originNodeIds)
+    {
+        if (originNodeIds.Count == 0)
+            return;
+
+        header.Text = headerText + "  \u2192";
+        header.Cursor = new Cursor(StandardCursorType.Hand);
+        ToolTip.SetTip(header, originNodeIds.Count == 1
+            ? $"Go to operator (Node {originNodeIds[0]})"
+            : $"Go to Node {originNodeIds[0]} — also from {string.Join(", ", originNodeIds.Skip(1).Select(id => "Node " + id))}");
+
+        header.PointerPressed += (_, e) =>
+        {
+            if (!e.GetCurrentPoint(header).Properties.IsLeftButtonPressed)
+                return;
+
+            if (TryNavigateToNode(originNodeIds[0]))
+                e.Handled = true;
+        };
+    }
+
     /// <summary>
     /// #436: marks the warnings SQL Server itself wrote into the plan, so they are not read as one of
     /// our inferences. Only the engine's are tagged — they are the minority, and a badge on every

src/PlanViewer.Core/Models/PlanModels.cs

@@ -391,6 +391,21 @@ public class PlanWarning
     /// </summary>
     public PlanWarningSource Source { get; set; } = PlanWarningSource.PerformanceStudio;
 
+    /// <summary>
+    /// The operators this finding actually came from, so a reader can be taken to them (#440).
+    ///
+    /// <para>A LIST rather than a single id, because the three honest answers are genuinely
+    /// different. A key lookup came from exactly one operator. A table variable warning came from
+    /// every operator that touched one, which on a big plan is several. And some findings have no
+    /// operator at all — "High Compile CPU" happened before a single row was read, and
+    /// "UDF Execution" is reported by SQL Server at the statement level only. Those keep this
+    /// empty, and the UI offers no navigation rather than picking somewhere arbitrary.</para>
+    ///
+    /// <para>Sending a reader to the wrong operator is worse than sending them nowhere, because
+    /// they would believe it.</para>
+    /// </summary>
+    public List<int> OriginNodeIds { get; set; } = [];
+
     /// <summary>
     /// Maximum percentage of elapsed time that could be saved by addressing this finding.
     /// null = not quantifiable, 0 = calculated as negligible.

src/PlanViewer.Core/Output/AnalysisResult.cs

@@ -256,6 +256,15 @@ public class WarningResult
     /// </summary>
     [JsonPropertyName("source")]
     public string Source { get; set; } = "";
+
+    /// <summary>
+    /// Node ids of the operators this finding came from, empty when it has no operator origin —
+    /// "High Compile CPU" happened before any operator ran (#440). Distinct from
+    /// <see cref="NodeId"/>, which says where the warning is ATTACHED in this output; these say
+    /// where it came FROM, and for a statement-level warning there is no NodeId at all.
+    /// </summary>
+    [JsonPropertyName("origin_node_ids")]
+    public List<int> OriginNodeIds { get; set; } = [];
 }
 
 public class MissingIndexResult

src/PlanViewer.Core/Output/ResultMapper.cs

@@ -196,7 +196,8 @@ private static StatementResult MapStatement(
                 MaxBenefitPercent = w.MaxBenefitPercent,
                 ActionableFix = w.ActionableFix,
                 IsLegacy = w.IsLegacy,
-                Source = w.Source.ToString()
+                Source = w.Source.ToString(),
+                OriginNodeIds = w.OriginNodeIds
             });
         }
 
@@ -306,7 +307,8 @@ private static OperatorResult MapNode(PlanNode node, CancellationToken cancellat
                 MaxBenefitPercent = w.MaxBenefitPercent,
                 ActionableFix = w.ActionableFix,
                 IsLegacy = w.IsLegacy,
-                Source = w.Source.ToString()
+                Source = w.Source.ToString(),
+                OriginNodeIds = w.OriginNodeIds
             });
         }
 

src/PlanViewer.Core/Services/PlanAnalyzer.Detection.cs

@@ -21,23 +21,31 @@ private static bool HasBatchModeNode(PlanNode node)
         return false;
     }
 
+    /* #440: collects the operators it found, because this walk already knows exactly which ones
+       touched a table variable and used to throw that away. Two lists rather than one, since the
+       two warnings this feeds are about different operators: every operator referencing a table
+       variable, versus only the ones modifying it (which is what forces the plan serial). */
     private static void CheckForTableVariables(PlanNode node, bool isModification,
-        ref bool hasTableVar, ref bool modifiesTableVar)
+        ref bool hasTableVar, ref bool modifiesTableVar,
+        List<int>? referencingNodeIds = null, List<int>? modifyingNodeIds = null)
     {
         if (!string.IsNullOrEmpty(node.ObjectName) && node.ObjectName.StartsWith("@"))
         {
             hasTableVar = true;
+            referencingNodeIds?.Add(node.NodeId);
             // The modification target is typically an Insert/Update/Delete operator on a table variable
             if (isModification && (node.PhysicalOp.Contains("Insert", StringComparison.OrdinalIgnoreCase)
                 || node.PhysicalOp.Contains("Update", StringComparison.OrdinalIgnoreCase)
                 || node.PhysicalOp.Contains("Delete", StringComparison.OrdinalIgnoreCase)
                 || node.PhysicalOp.Contains("Merge", StringComparison.OrdinalIgnoreCase)))
             {
                 modifiesTableVar = true;
+                modifyingNodeIds?.Add(node.NodeId);
             }
         }
         foreach (var child in node.Children)
-            CheckForTableVariables(child, isModification, ref hasTableVar, ref modifiesTableVar);
+            CheckForTableVariables(child, isModification, ref hasTableVar, ref modifiesTableVar,
+                referencingNodeIds, modifyingNodeIds);
     }
 
     /// <summary>

src/PlanViewer.Core/Services/PlanAnalyzer.Node.cs

@@ -48,6 +48,20 @@ private static void AnalyzeNode(PlanNode node, PlanStatement stmt, AnalyzerConfi
         Rule28_RowCountSpool(node, stmt, cfg);
         Rule29_ImplicitConversionSeek(node, stmt, cfg);
         Rule35_ExpensiveOperator(node, stmt, cfg);
+
+        /* #440: an operator warning's origin is the operator it is hanging off, so it is stamped
+           here rather than at each of the 26 places above that add one. Same reasoning as the
+           provenance stamp in ShowPlanParser: a rule you have to remember at every construction
+           site is a rule that eventually gets forgotten, and the one time it is forgotten the UI
+           quietly offers no link on a warning that has a perfectly good one.
+
+           Only fills what a rule left empty, so a rule that knows better - one pointing at the
+           operator that CAUSED the problem rather than the one reporting it - keeps its own answer. */
+        foreach (var warning in node.Warnings)
+        {
+            if (warning.OriginNodeIds.Count == 0)
+                warning.OriginNodeIds.Add(node.NodeId);
+        }
     }
 
     private static void Rule01_FilterOperator(PlanNode node, PlanStatement stmt, AnalyzerConfig cfg)

src/PlanViewer.Core/Services/PlanAnalyzer.Statement.cs

@@ -488,15 +488,19 @@ private static void Rule22Stmt_TableVariable(PlanStatement stmt, AnalyzerConfig
             var hasTableVar = false;
             var isModification = stmt.StatementType is "INSERT" or "UPDATE" or "DELETE" or "MERGE";
             var modifiesTableVar = false;
-            CheckForTableVariables(stmt.RootNode, isModification, ref hasTableVar, ref modifiesTableVar);
+            var referencingNodeIds = new List<int>();
+            var modifyingNodeIds = new List<int>();
+            CheckForTableVariables(stmt.RootNode, isModification, ref hasTableVar, ref modifiesTableVar,
+                referencingNodeIds, modifyingNodeIds);
 
             if (hasTableVar && !modifiesTableVar)
             {
                 stmt.PlanWarnings.Add(new PlanWarning
                 {
                     WarningType = "Table Variable",
                     Message = "Table variable detected. Table variables lack column-level statistics, which causes bad row estimates, join choices, and memory grant decisions. Replace with a #temp table.",
-                    Severity = PlanWarningSeverity.Warning
+                    Severity = PlanWarningSeverity.Warning,
+                    OriginNodeIds = referencingNodeIds
                 });
             }
 
@@ -506,7 +510,8 @@ private static void Rule22Stmt_TableVariable(PlanStatement stmt, AnalyzerConfig
                 {
                     WarningType = "Table Variable",
                     Message = "This query modifies a table variable, which forces the entire plan to run single-threaded. SQL Server cannot use parallelism for modifications to table variables. Replace with a #temp table to allow parallel execution.",
-                    Severity = PlanWarningSeverity.Critical
+                    Severity = PlanWarningSeverity.Critical,
+                    OriginNodeIds = modifyingNodeIds
                 });
             }
         }

src/PlanViewer.Core/Services/WarningIndex.cs

@@ -0,0 +1,41 @@
+using System.Collections.Generic;
+using PlanViewer.Core.Models;
+
+namespace PlanViewer.Core.Services;
+
+/// <summary>
+/// Gathers the warnings scattered across an operator tree into one list (#440).
+///
+/// <para>Lives in Core rather than next to the panel that renders it for two reasons: it is a plan
+/// tree walk over Core's own models with nothing UI about it, and putting it here means it can be
+/// tested without standing up Avalonia.</para>
+/// </summary>
+public static class WarningIndex
+{
+    /// <summary>
+    /// Every warning hanging off an operator beneath <paramref name="root"/>, paired with the
+    /// operator carrying it, in no particular order — callers sort for themselves, because the
+    /// statement panel wants them by benefit while other callers may not.
+    /// </summary>
+    public static List<(PlanNode Node, PlanWarning Warning)> CollectOperatorWarnings(PlanNode? root)
+    {
+        var collected = new List<(PlanNode, PlanWarning)>();
+        if (root == null)
+            return collected;
+
+        /* Explicit stack rather than recursion: a deep plan is exactly the case this feature exists
+           for, and #430 was a crash caused by assuming operator trees are shallow. */
+        var pending = new Stack<PlanNode>();
+        pending.Push(root);
+        while (pending.Count > 0)
+        {
+            var node = pending.Pop();
+            foreach (var warning in node.Warnings)
+                collected.Add((node, warning));
+            foreach (var child in node.Children)
+                pending.Push(child);
+        }
+
+        return collected;
+    }
+}