[erikdarlingdata]

Claude posting for Erik Darling.

Closes #651.

Adds per-finding suppressions to .planview.json, keyed on the statement query hash, as designed and approved on 2026-10-14. This body describes the final state, including the review follow-up.

What changed

Config and loader (AnalyzerConfig.cs, ConfigLoader.cs)

• New suppressions list with SuppressionEntry (query_hash, rule, type, operator, reason). Hash normalization (trim, strip 0x/0X, OrdinalIgnoreCase) lives in one helper used by both sides.
• Validation at load, naming the entry by zero-based index and the file that was actually read: Suppression 2 in C:\work\team.json has no "reason"., ... needs "rule" or "type"., ... has an empty "query_hash". The path is the real configPath, so --config team.json and the current-directory or home copy are told apart.
• Loader failures are InvalidDataException (bad suppression, or malformed JSON, which used to escape as a raw JsonException).

Where a loader failure is handled

• analyze and query-store: one line to stderr, exit code 1.
• planview repl: Program.cs catches FileNotFoundException and InvalidDataException around PlanReplApp.Create() (the PlanOperations constructor reads the config), prints the message to stderr, returns 1. Checked by hand with a bad .planview.json in the current directory: one line, exit 1.
• Desktop app: LoadPlan catches InvalidDataException and FileNotFoundException, analyzes with AnalyzerConfig.Default, and shows Config ignored: <message> in a new toolbar notice (ConfigNoticeText, warning colour, full message as tooltip). The plan still opens and there is no dialog. All five callers of LoadPlan are covered. The notice hides again after a good load or when the plan is cleared. The app had no existing notice area (CostText is cleared on every statement render), so I added one small text block beside it.

Core

• PlanWarning gains IsSuppressed and SuppressionReason.
• SuppressionApplier (new) is the last step of PlanAnalysisPipeline.AnalyzeParsed, after the scorer. It walks PlanStatements.EnumerateAll like ApplySeverityOverrides. It flags and never removes, and never touches severity.
• OperatorLabel.Format (new, internal) is the one home for the operator label, used by ResultMapper and the suppression pass. It is linked into PlanViewer.Web.csproj beside the other Services files (the Web project links Core files one by one; this fixes the CS0103 CI failure). SuppressionApplier.cs needs no link because Web does not compile PlanAnalysisPipeline.cs.

JSON and MCP

• WarningResult gains rule (omitted when null, set at both mapper sites), suppressed (always written) and suppression_reason (omitted when null). AnalysisSummary gains suppressed_warnings; total_warnings, critical_warnings and warning_types exclude suppressed findings. Suppressed warnings stay in statements[].warnings and the operator tree.
• MCP: PlanSummaryResult.suppressed_warnings; PlanWarningItem.rule, suppressed, suppression_reason. The MCP host itself still passes AnalyzerConfig.Default, so MCP sessions never apply a config file (out of scope, and the README now says so).

Counts versus lists. Rule: a count excludes suppressed warnings; a list keeps them with the tag. Counts now excluding suppressed: the JSON summary, the desktop app's MCP session counts, the HTML severity badges, the statement grid Critical and Warnings columns (PlanViewerControl.Statements.cs), the "N warnings" badge on the root node (PlanViewerControl.Rendering.cs), the advice statement chips for critical, warning and spill counts (AdviceContentBuilder.WaitStats.cs) and the compare "Warnings" metric (ComparisonFormatter.cs). Lists that keep suppressed warnings with the tag: text, HTML, advice cards, properties panel, and the MCP warning list.

Renderers: [suppressed: <reason>] in the text output (all three sites, plus a Suppressed warnings: N summary line when above zero), a suppressed: <reason> badge in HTML (reason encoded), and the tag in the advice cards and the three properties-panel sites.

Docs: README ### Configuration file (.planview.json) section (search order, which tools read the file, rules.disabled, severity_overrides values, every suppression field, matching rules, one example, where to find query_hash, the error format with a real path). --config <path> added to the planview analyze usage block; the Analysis Rules one-liner links to the section. llms.txt lists no CLI options, so it is unchanged. The section passes the plain-English checker.

Judgment calls

• Golden hash. HistoricalCliContractTests pins the SHA-256 of compact CLI output. The new keys (suppressed, rule, suppressed_warnings) change it, as they must. I rolled the constant and documented it. Stripping exactly those keys from the new output gives the old hash back. The test now writes a temp {} config and passes --config, so a developer's own ~/.planview.json cannot change the bytes. The hash did not change by that.
• get_plan_warnings (MCP) lists and counts every warning, each flagged suppressed.
• Extra validation: a blank operator, a null entry, and a hash that is only 0x are rejected. When several entries match one warning, the first in file order sets the reason. Statements with no QueryHash are never matched.
• App-side: the config notice is a text block in the plan toolbar, since the plan control had no status or notification area to reuse.

Tests

New: SuppressionTests, ConfigLoaderTests, a CLI test in HistoricalCliContractTests (analyze --config with a missing reason: one stderr line naming the file, empty stdout, exit 1), a text test for the grouped and single operator-warning paths, TheStatementChipsDoNotCountSuppressedWarnings (advice chips) and TheWarningsMetricDoesNotCountSuppressedWarnings (compare). The xUnit1051 warnings in SuppressionTests are fixed. No existing App test drives the statement grid or the root-node badge, so I added no App test for those two counts; the one-line change at each site is covered by review only.

Results:

• dotnet build PlanViewer.sln: 0 errors, including PlanViewer.Web.
• Full tests/PlanViewer.Core.Tests: 1498 total, 1467 passed, 0 failed, 31 skipped.

Out of scope, not built: unused-suppression reporting, --show-suppressed, the MCP host passing a loaded config.

Review round 2 follow-up

An unreadable .planview.json (locked, or permission denied) threw IOException or UnauthorizedAccessException out of ConfigLoader.Load, past every caller that catches InvalidDataException: the two CLI commands, the REPL entry and the desktop app's LoadPlan. The loader now wraps the file read the same way it wraps the parse, so all four sites print one line naming the file. ConfigLoaderTests gains AnUnreadableFileFailsWithAMessageNotAStackTrace, which holds the file open with FileShare.None and asserts the one-line message.

🤖 Generated with Claude Code

https://claude.ai/code/session_018MrD9SGvLFx4kJaeGZyXG9

[claude]

Reviewed the config loader, suppression applier, Web csproj link, CLI/REPL/app error handling, and the count/list changes. I found nothing that needs changing.

• No generated T-SQL or SQL concatenation is involved. Suppression values are only compared or rendered, and the HTML exporter encodes the reason.
• OperatorLabel.cs is linked into PlanViewer.Web.csproj. SuppressionApplier.cs doesn't need a link because Web doesn't compile PlanAnalysisPipeline.cs.
• Malformed JSON, I/O errors and bad entries all become InvalidDataException. Every caller catches it: the analyze and query-store commands, the REPL, and LoadPlan in the app.
• A null QueryHash never matches a suppression.
• Nothing here touches the version, so the SSMS version check doesn't apply. I didn't build the solution or run the tests, so the build and test results in the PR description are unchecked.

One small gap: the statement-grid and root-node-badge count changes in the desktop app have no tests, as the PR description says. That's acceptable given the one-line changes.