Skip to content

Make the node row label agree with its percentage - #615

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/611-row-label-agrees
Sep 29, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
fix/611-row-label-agrees

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Summary

Closes #611

Changes

  • src/PlanViewer.Core/Services/PlanRowAccuracy.cs (new): FormatActualOfExpected returns the whole label. PrintActualOfExpected returns the two numbers and the percentage, for a surface that words the label differently.
  • src/PlanViewer.App/Controls/PlanViewerControl.Rendering.cs and src/PlanViewer.Web/Pages/Index.razor: the App and Web node labels use FormatActualOfExpected. The label color does not change.
  • src/PlanViewer.Core/Output/HtmlExporter.cs: the HTML export uses PrintActualOfExpected and keeps its "X of Y rows (P%)" wording.
  • src/PlanViewer.Web/PlanViewer.Web.csproj: links the new Core file, because the Web project compiles Core sources directly.
  • tests/PlanViewer.Core.Tests/PlanRowAccuracyTests.cs (new): PerformanceMonitor's rule cases and its 2,000-pair sweep, the issue's examples, and the App label and the HTML export for key_lookup_plan.sqlplan. It also checks the source of the Web label, because the test project does not reference the Web project and cannot render that label.

Test Plan

  • PlanRowAccuracyTests, NodeLabelRowAccuracyTests and HtmlExporterTests pass on this branch (56 tests).
  • The full suite had 0 failures with this change and the [FEATURE] The tiniest modification ever: "Early abort" #613 change together.
  • Open tests/PlanViewer.Core.Tests/Plans/key_lookup_plan.sqlplan. The Key Lookup's label reads "1 of 1.128 (89%)".

Generated with Claude Code

https://claude.ai/code/session_019tS6P95Dtzs4aeMpb1xqzX

The label printed both row counts N0 but took the percentage from the
unrounded values, so a Key Lookup that ran 117 times for 1 row read
"1 of 1 (89%)". PlanRowAccuracy, ported from PerformanceMonitor's
PlanRowAccuracy (#4684), adds the fewest decimals (up to 4, fixed-point)
at which the printed numbers give the printed percentage, and never
prints a non-zero value as 0. That label now reads "1 of 1.128 (89%)".

The App and Web node labels and the HTML export all use it, so all three
print the same numbers, and PerformanceMonitor prints the same string
for the same plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019tS6P95Dtzs4aeMpb1xqzX
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Reviewed the diff. I found nothing blocking.

  • PlanRowAccuracy.cs is linked into PlanViewer.Web.csproj, as the repo convention requires.
  • It has no dependencies beyond BCL, so it compiles in the Blazor project.
  • Extreme inputs are handled: NaN, Infinity and subnormals. The tests pin the cap and the no-exponent behaviour.
  • No SQL is generated and no version files are touched.
  • The tests cover the App label, the HTML export and the shared formatter. The Web label is only checked by a source-text assertion, which is brittle but acceptable given that Web isn't referenced by the test project.

Minor, non-blocking: WebNodeLabel_UsesTheSharedFormatter will fail on any harmless reformat of that line in Index.razor.

@erikdarlingdata
erikdarlingdata merged commit 82d3799 into dev Sep 29, 2026
4 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/611-row-label-agrees branch September 29, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant