Collapse the sidebar dot onto the status the Overview card renders (#2458)

[erikdarlingdata]

Fixes #2458. Part 2 of #2452; part 1 was #2457, and the type this collapses onto arrived in #2451.

The last copy of the ladder

Lite/Models/ServerConnection.cs:199 computed "Unknown"/"Online"/"Warning"/"Offline" from its own (IsOnline, HasCollectorErrors) pair — the same four-word ladder the Overview card derives, on a different type, on a different surface, from a different instance of the same flags. The sidebar and the card could say different things about one server and nothing would notice. That is the drift #2429 collapsed the viewer's card for and #2451 collapsed Lite's; this was the fourth copy, and #2451's own PR body named it as the next place it could go wrong.

It is the last of them in Lite, and not the last of them anywhere — an earlier draft of this description said otherwise and review was right to correct it. See the last section.

The collapse is the ladder, not the return type

The change that would have looked like a fix and wasn't: give ServerConnection a CardStatus => IsOnline switch { … } and have DotStatus render it. That is still a second ladder — the same flag pair read in a second place, just typed better — and it can still drift from the card's.

So the ladder itself moved into one function:

public static class ServerCardStatusRules
{
    public static ServerCardStatus Classify(bool? isOnline, bool hasCollectorErrors) => isOnline switch { … };
    public static string Word(this ServerCardStatus status) => status switch { … };
    public static string Headline(this ServerCardStatus status) => status switch { … };
}

ServerSummaryItem.CardStatus and ServerConnection.CardStatus both call Classify. StatusDisplay and DotStatus are both CardStatus.Word(); StatusHeadline and the first line of DotTooltip are both CardStatus.Headline(). The card's brushes stay where they are — hex colours are the card's business, and the sidebar paints its dot from theme resources through XAML DataTriggers — but they still switch on CardStatus rather than on the flags, exactly as #2451 left them.

HasCollectorErrors is bool? on ServerConnection and bool on the card, so "not yet determined" folds to false. That is the string ladder's own reading, kept deliberately rather than inherited by accident: a server whose collector health nobody has established is not a server reporting failing collectors, and painting it amber would say it was.

The dot says what it means, and says what it is not about

The sidebar Ellipse gets the tooltip the card's dot got in #2451, opening on the same sentence word for word. It is the thing a reader points at first, which is @ehaar's #2422 complaint one surface over from where it was reported.

Online — the last connection check succeeded
It reports the connection check only, not collection freshness — the Overview card's Last Collect row bands that.
Double-click the row to open this server's tab

The middle line is what this surface specifically needs, and it is #2457's separation being preserved rather than restated. #2457 kept collection freshness out of the status word and gave it its own banded row on the card, because folding it in recreates the #2429/#2422 conflation of a stale collection with a failing one. The sidebar has no such row and ServerConnection carries no last-collection time to build one from — so a green dot here is a connection answer being read somewhere that offers no freshness answer at all. Naming that, and naming where the freshness answer lives, is what stops a reader inferring one from the other.

Classify takes two arguments and TheDot_CannotBandFreshnessEvenByAccident pins that signature, so freshness cannot be folded in without changing a line a test names. The failure mode being guarded is a plausible, well-meant edit, not a typo.

The closing line names the gesture this surface supports: ServerListView_MouseDoubleClick connects, a single click only selects. Same sentence as the card's with the noun the reader is pointing at.

The pin moved with the ladder, and got wider — because of how it missed this

#2451 wrote Assert.Equal(1, CountOccurrences(source, "IsOnline switch")) specifically so a second copy could not reappear. It was true, it stayed true, and the copy it existed to forbid was already there:

• it was in another file, which the scan never opened; and
• it was written as a chain of if statements, not a switch, so even a repo-wide scan for that literal would have found nothing.

It evaded the pin on both axes at once, and went on evading it through #2451 and #2457. Loosening it was never on the table; it needed to be pointed at the right invariant.

The count now spans both files and pins the classifier's own parameter list:

Assert.Equal(1, CountOccurrences(source, "isOnline switch"));
Assert.Equal(0, CountOccurrences(source, "IsOnline switch"));
Assert.Equal(0, CountOccurrences(sidebar, "sOnline switch"));

and TheFourWords_AreWrittenInExactlyOnePlace holds the invariant the count was standing in for, syntax-agnostically: the four words are string literals that appear in exactly one function, and none of them is written in ServerConnection.cs any more. An if chain, a switch, a dictionary and a ternary all have to spell them. Comments are stripped first, because DotStatus's own doc comment legitimately names all four while the code writes none.

Verification

Lite.Tests targets net10.0-windows and cannot run on macOS, so the logic was run for real anyway: a throwaway net10.0 harness splices ServerCardStatus, ServerCardStatusRules, ServerSummaryItem and the ServerConnection members straight out of the shipped files (a generator reads the source and emits them — never retyped) behind a System.Windows.Media shim, with a real project reference to PerformanceMonitor.Common. 35 checks, 0 failures, including every reachable (IsOnline, HasCollectorErrors) combination agreeing between the two surfaces on state, on word, and on the tooltip's first line.

Against dev that harness does not compile at all, because none of the members exist there — so the comparison runs as the identically-expressible subset with the new members reached reflectively:

	dev	this branch
the reflective subset	7 passed / 6 failed	13 / 0
the source + XAML scan	5 passed / 13 failed	18 / 0

The 7 that pass on both sides are the controls, and they are the ones a wrong fix breaks first. Six of them are the two ladders agreeing on every flag combination today — which is the honest shape of this defect: it is drift potential, not a live disagreement. That is precisely why the assertions separating the branches are about construction rather than about output, and why a behavioural test alone could never have justified this change.

The seventh control is #2457's: a card whose collection stopped four hours ago still reads a green "Online", so this is not option 2 sneaking in through the sidebar.

The XAML half has to text-scan for the reason #2429 gave — a ToolTip attribute lives where no assertion about a C# object can reach it, and removing it compiles perfectly clean.

Whole solution builds, 0 errors. CHANGELOG deliberately untouched — #2395 is an open release-prep PR that owns that file for 3.5.1.

Two things noted and not fixed here

The Darling viewer has a fifth copy, and its two surfaces already disagree. Raised by review, verified, and it is worse than duplication. DarlingServer.DotStatus (Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs:116) derives the same four words from its own flag pair, bound the same way Lite's was. Both it and the viewer's Overview card are stamped from the same ClassifyFreshness(LastCollectionTime, nowUtc) call — and they answer differently for a never-collected server:

	viewer sidebar dot	viewer Overview card
ServerFreshness.NeverCollected	IsOnline = null → "Unknown" → grey muted dot	AwaitingFirstCollection = true → "Awaiting first collection", amber

That is on dev today, not a latent risk. It is a different app, a different ServerCardStatus (five members, Stale where Lite has CollectorErrors), and a different vocabulary decision — the viewer's word is freshness-derived where Lite's is connection-derived — so folding it in here would mean picking that vocabulary inside a Lite PR. Filed separately.

Lite/Mcp/McpDiscoveryTools.cs:28 renders a third vocabulary for the same axis — "Online"/"Offline"/"Status not checked" — off ConnectionStatus.IsOnline. It is a different type, it never reads collector health so it has no "Warning" state at all, and MCP text is a consumer API whose words downstream automation may key on. Changing them is a decision, not a cleanup, so it is left alone rather than folded in.

🤖 Generated with Claude Code

[claude]

Lite/Darling parity: the fixed drift still has a live copy in Darling.Viewer

This PR collapses Lite's sidebar dot (ServerConnection.DotStatus) onto the shared ServerCardStatusRules.Classify ladder, closing out the class of bug where a sidebar and an overview card independently derive the same (IsOnline, HasCollectorErrors) pair and can therefore disagree about one server. The PR description states this was "the last copy" (after #2429 for the viewer's card and #2451 for Lite's card).

That's not quite true — Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.cs:116 still has an independent copy of the exact same four-word ladder on DarlingServer.DotStatus:

public string DotStatus => IsOnline switch
{
    true => HasCollectorErrors ? "Warning" : "Online",
    false => "Offline",
    _ => "Unknown"
};

It's bound the same way Lite's used to be — Darling/PerformanceMonitor.Darling.Viewer/MainWindow.xaml:531-537 has a DataTrigger on Server.DotStatus for Online/Offline/Warning on the fleet-list Ellipse, off a DarlingServer instance populated via ApplyFreshness, while the Overview card's ServerSummaryItem.CardStatus (in ViewerDataService.Overview.cs) resolves the same-shaped flag pair independently (and with an extra AwaitingFirstCollection state). Since these are two different types populated at different times from freshness classification, this is structurally the same drift class #2429/#2451/#2458 keep chasing down — just not yet chased down here.

Given the pattern in this repo of doing this collapse one surface at a time, this may be intentionally deferred, but flagging it now so it doesn't get lost — and so the "last copy" claim in the PR description gets corrected before it's read as "done everywhere."

---

Otherwise the change itself looks solid: ServerConnection.CardStatus correctly folds HasCollectorErrors == true (nullable → non-nullable, treating "not yet determined" as not-Warning, same as the old inline ladder), the Word()/Headline() extension methods are visible from ServerConnection.cs via the existing using PerformanceMonitorLite.Services;, and the new tests (LiteSidebarDotRendersTheCardStatusTests) cover the flag-combination matrix, the XAML binding/tooltip wiring, and the "freshness must not fold into this classifier" invariant. No T-SQL or Darling-service (Postgres) changes in this PR, so those style rules don't apply here.

[erikdarlingdata]

Verified, and you were right to insist the claim get corrected — it turned out to be worse than duplication.

DarlingServer.ApplyFreshness and ServerSummaryItem.ApplyFreshness in the viewer are stamped from the same ClassifyFreshness(LastCollectionTime, nowUtc) call, and they answer differently for NeverCollected:

	viewer sidebar dot	viewer Overview card
	IsOnline = null → "Unknown"	AwaitingFirstCollection = true
word	"Unknown"	"Awaiting first collection"
colour	no DataTrigger matches → ForegroundMutedBrush (grey)	amber #FFD54F

So it is not drift potential there the way it was in Lite — it is a live disagreement on dev, and the dot is silently rendering the pre-#2429 answer for the one state AwaitingFirstCollection was added to fix.

Filed as #2473 with the vocabulary decision it needs made first (what a dot should say for "queued, not dead" — the card chose amber and wrote down why; matching it is the default but worth saying out loud). Not folded in here because it is a different app, a different five-member ServerCardStatus, and a freshness-derived word where Lite's is connection-derived — #2457 turned down fusing those two axes in Lite specifically, so choosing the viewer's inside a Lite PR would have been choosing it quietly.

The overclaim is corrected in both the commit message and the description. Thanks for catching it — it is the same shape of error #2451 got caught on, which is the reason the pin in this PR is written the way it is.

[claude]

The leading comment on the StackPanel two lines above this (``, line 68) still says the dot is about "collection freshness." That's now directly contradicted by the DotTooltip text this PR wires up: "It reports the connection check only, not collection freshness — the Overview card's Last Collect row bands that." Since this file is already being edited to make that exact distinction, it'd be worth fixing the stale leading comment too (e.g. "Status dot (connection) + ...") so a reader doesn't get pointed the wrong way by the first line above the block.

[erikdarlingdata]

Fixed. Worth more than a nit, I think — it is #2422's complaint arriving through the comment rather than through the UI: the first line a reader met above that block sent them to the wrong axis.

It is true in the Darling viewer, where the dot genuinely is freshness-derived, and it came over with the rest of the ported row template. In Lite, IsOnline is ServerManager.GetConnectionStatus. The label now says which one it is and records that it used to say the other, because a reader who knows the viewer's version will otherwise assume the two match — which is the same wrong inference the tooltip's middle line exists to prevent.

[claude]

Review

Went through the diff against CONTRIBUTING.md (no T-SQL here, so the collector-style rules don't apply — this is a pure C#/WPF change in Lite). Traced the flag-classification logic end to end.

Correctness: ServerCardStatusRules.Classify is a faithful extraction of the two prior copies. Verified by hand that ServerConnection.CardStatus (Classify(IsOnline, HasCollectorErrors == true)) and ServerSummaryItem.CardStatus (Classify(IsOnline, HasCollectorErrors)) now produce identical results for every (IsOnline, HasCollectorErrors) combination, including the nullable-fold-to-false case on ServerConnection, which matches the prior DotStatus getter's behavior exactly. DotTooltip's first line is byte-for-byte CardStatus.Headline(), matching StatusTooltip's first line (StatusHeadline => CardStatus.Headline()) — confirmed by reading both call sites, not just trusting the new tests.

No other call sites missed. Grepped the whole tree for DotStatus/CardStatus/DotTooltip — only MainWindow.xaml, ServerConnection.cs, and LocalDataService.Overview.cs reference them, and all three are updated consistently. ServerConnection.cs already had using PerformanceMonitorLite.Services;, so the cross-namespace reference to ServerCardStatusRules compiles without a new using.

Lite/Darling parity: The PR body discloses that Darling's viewer has a fifth, still-divergent copy of this ladder (DarlingServer.DotStatus in ViewerDataService.cs) and defers it intentionally rather than silently. Confirmed that file still derives its own ladder from ClassifyFreshness independently — so this is known, disclosed scope, not missed drift.

One nit: left an inline comment on Lite/MainWindow.xaml — a pre-existing leading comment ("Status dot (collection freshness)...") two lines above the new block is now contradicted by the tooltip text this PR adds, which explicitly says the dot is not about collection freshness.

No security, performance, or null-handling concerns — this is UI-layer string/enum plumbing with no I/O, no user input, and cheap computed properties.

[claude]

Reviewed. This is a clean, well-scoped collapse of the fourth copy of the status ladder — no correctness issues found.

Verified:

• ServerConnection.CardStatus => ServerCardStatusRules.Classify(IsOnline, HasCollectorErrors == true) preserves the old DotStatus semantics exactly for every (IsOnline, HasCollectorErrors) combination, including the null HasCollectorErrors → "Online" fold that the old string ladder had.
• ServerSummaryItem.CardStatus => ServerCardStatusRules.Classify(IsOnline, HasCollectorErrors) is unchanged in behavior from before the extraction.
• No stale references to the old IsOnline switch literal remain in ServerConnection.cs; the MainWindow.xaml ToolTip/DataTrigger bindings resolve against Server.Connection.DotTooltip/DotStatus, consistent with the existing Server.Connection.IsSilenced binding pattern used one block above, and against the same ListBox that owns ServerListView_MouseDoubleClick — so the tooltip's "Double-click the row..." closing line matches the actual wired-up gesture.
• C# style (naming, XML docs on public members, nullable handling) matches CONTRIBUTING.md. No T-SQL in this PR.

Lite/Darling parity: this PR only touches Lite, but it doesn't quietly leave a Darling gap unaddressed — the description explicitly surfaces that the Darling viewer has its own (pre-existing, dev-today) fifth copy of this ladder, with the two Darling surfaces already disagreeing for NeverCollected, and states it's filed separately rather than folded in here (a different ServerCardStatus shape and a freshness- vs. connection-derived vocabulary, so collapsing it inside a Lite PR would be smuggling in a vocabulary decision). Same treatment for the third vocabulary in Lite/Mcp/McpDiscoveryTools.cs. Both call-outs are correct reasons to defer rather than omissions.

Test coverage (LiteSidebarDotRendersTheCardStatusTests) exercises every reachable flag combination for dot/card agreement, the XAML tooltip binding location, and a syntax-agnostic scan that the four words are written in exactly one place — good pin against this regressing the same way it snuck in the first time.

No blocking findings.