Features/colored links

[rferraton]

What does this PR do?

Accuracy-ratio colored links for plan viewer

Summary

Adds visual feedback on plan viewer links (edges) by coloring them according to the divergence between actual and estimated row counts. This helps immediately spot cardinality estimation issues in actual execution plans.

How it works

A new setting accuracy_ratio_divergence_limit (default: 10) defines the neutral band. Links where the accuracy ratio (ActualRows / EstimateRows) falls between 1/limit and limit keep the standard theme color. Outside that band, links are colored across 6 severity levels:

Accuracy ratio range	Meaning	Color
[0, 1/(limit×100))	Severe overestimation	Fluo Blue
[1/(limit×100), 1/(limit×10))	Moderate overestimation	Light Blue
[1/(limit×10), 1/limit)	Mild overestimation	Blue
[1/limit, limit]	Within tolerance	Default (grey)
(limit, limit×10)	Mild underestimation	Light Orange
[limit×10, limit×100)	Moderate underestimation	Fluo Orange
[limit×100, +∞)	Severe underestimation	Fluo Red

"Overestimation" = optimizer estimated more rows than actually produced; "Underestimation" = the opposite.

The coloring only activates on actual execution plans (where HasActualStats is true). Estimated plans are unaffected.

Changes

AppSettingsService.cs

• Added AccuracyRatioDivergenceLimit property to AppSettings (persisted as accuracy_ratio_divergence_limit, default 10).

PlanViewerControl.axaml.cs

• Added 6 new SolidColorBrush fields for link accuracy coloring (dark theme).
• Added GetLinkColorBrush(PlanNode, double) — maps a child node's accuracy ratio to the appropriate color band.
• RenderEdges / CreateElbowConnector — links now use accuracy-based coloring instead of the static EdgeBrush.
• RenderMinimapEdges — minimap links use the same accuracy-based coloring.
• Node-level row accuracy highlighting now uses the same AccuracyRatioDivergenceLimit setting instead of hardcoded 10/0.1 thresholds.
• Settings are loaded once per render pass (not per edge) and the divergence limit is clamped to ≥ 2.0 to prevent degenerate band boundaries.

Not in scope

• Blazor/WebAssembly (Index.razor) connector coloring — the web project still uses a static CSS class for links. This can be addressed in a follow-up.
• Light theme link colors — only dark theme brushes are defined for now.
• UI for editing the accuracy_ratio_divergence_limit setting (currently requires manual edit of appsettings.json).

Testing

• Verified on actual execution plans with known cardinality skew: links correctly transition through the 6 color bands.
• Estimated plans render with unchanged default edge color.
• Build passes with no errors or warnings.

Which component(s) does this affect?

• Desktop App (PlanViewer.App)
• Core Library (PlanViewer.Core)
• CLI Tool (PlanViewer.Cli)
• SSMS Extension (PlanViewer.Ssms)
• Tests
• Documentation

How was this tested?

Describe the testing you've done. Include:

• Plan files tested (estimated, actual, Query Store, etc.) : actual. estimated not impacted (tested)
• Platforms tested (Windows, macOS, Linux) : windows only

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• All tests pass (dotnet test)
• I have not introduced any hardcoded credentials or server names

[erikdarlingdata]

PR #289 Review — Features/colored links

Overview

Adds accuracy-ratio-based coloring to plan viewer connector lines (edges) on actual execution plans. Introduces a configurable accuracy_ratio_divergence_limit setting (default 10) that defines a neutral band; estimates outside that band shade through 6 severity levels (blue family for overestimation, orange/red for underestimation). Also unifies the per-node row accuracy highlight with the same setting.

What's good

• GetLinkColorBrush is a pure static helper — clean, easy to follow, easy to test.
• Branching is straightforward and the band order (severe → mild) is logical.
• Edge cases are handled sensibly: EstimateRows == 0 && ActualRows > 0 → double.MaxValue → severe underestimate; both zero → neutral.
• Minimap and main canvas use the same coloring logic (no drift).
• Estimated plans are explicitly opted out via HasActualStats, preserving prior behavior.
• Sensible clamp Math.Max(2.0, …) to prevent inverted/degenerate band boundaries.

Issues / suggestions

Performance: AppSettingsService.Load() called once per node — measured, not a blocker

Initial concern: Load() reads + deserializes appsettings.json from disk on every call, and the PR moves it inside CreateNodeVisual, which runs once per node.

I benchmarked the actual Load() cost (mirroring the implementation in AppSettingsService.cs:36-51) against the real appsettings.json (1.4 KB):

    1 loads:     0.08 ms
  100 loads:     6.08 ms   (~0.061 ms/call)
  376 loads:    19.01 ms   (~0.051 ms/call)
 1000 loads:    49.56 ms   (~0.050 ms/call)

The largest plan in .internal/examples/ is sp-whoisactive.sqlplan at 376 RelOp nodes. Worst-case added cost per render: ~19 ms — imperceptible against the Avalonia layout + text shaping pass for that many nodes. RenderStatement and RenderMinimap only fire on statement switch / minimap toggle, not per frame.

Still worth threading divergenceLimit from RenderStatement into CreateNodeVisual / RenderNodes — mirrors how it's already done for RenderEdges, removes I/O from a hot recursion, single source of truth — but it shouldn't block the PR.

Redundant clamp

Math.Max(2.0, …) is applied in four places: RenderStatement (~406), CreateNodeVisual (~662), RenderMinimap (~3617), and again inside GetLinkColorBrush (~750). With a single Load-per-render at the top of RenderStatement, you can clamp once at the source and trust callees. The defensive clamp inside GetLinkColorBrush becomes dead code.

Magic numbers in band boundaries

The × 10 and × 100 multipliers inside GetLinkColorBrush are not named. Two private const double constants (e.g. MildBandMultiplier = 10, SevereBandMultiplier = 100) would document intent and let a future tweak happen in one place.

Setting field placement / docs

AppSettings.AccuracyRatioDivergenceLimit is double but the doc comment says "Default 10" — fine, but consider documenting the clamp behavior (< 2.0 is silently treated as 2.0) so users editing appsettings.json don't get confused if 1.5 appears not to take effect.

Color choice — accessibility

LinkFluoRedBrush = #FF1744 against the dark theme background is high contrast, good. But consider whether the orange-vs-red distinction is colorblind-accessible; deuteranopia/protanopia users will see fluo-orange and fluo-red as similar. Not a blocker, but a future enhancement could vary stroke thickness or dash pattern in addition to color.

Test coverage

GetLinkColorBrush is a pure function — perfect candidate for unit tests covering the band boundaries (esp. the >= vs > inclusivity, which is asymmetric between the under/over branches and worth pinning down). I don't see tests added.

Boundary asymmetry — worth confirming intent

• Underestimation: >= divergenceLimit * 100 → FluoRed; >= divergenceLimit * 10 → FluoOrange; else (i.e., > divergenceLimit) → LightOrange.
• Overestimation: < 1.0 / (divergenceLimit * 100) → FluoBlue; < 1.0 / (divergenceLimit * 10) → LightBlue; else → Blue.

Looks symmetric, and matches the description table. Just flagging that exact-boundary cases (accuracyRatio == divergenceLimit * 10) classify as moderate (FluoOrange), the more alarming bucket — fine, but worth confirming that's the intended tiebreak. The neutral band's upper bound is inclusive (<= divergenceLimit), so accuracyRatio == divergenceLimit returns the default edge brush.

Out of scope (called out by author, no action needed)

• Blazor/WebAssembly path unaffected.
• Light theme brushes not defined.
• No UI for editing the setting.

Risk / blast radius

Low. Visual-only change, gated on HasActualStats, no data path or persistence changes beyond a single new optional setting (back-compat: missing key → default 10).

Recommendation

LGTM. The per-node Load() is worth cleaning up but not a blocker — measured at ~19 ms on the largest example plan. Magic-number constants, redundant clamp, and unit tests on the pure helper are all polish items.