Say why Lite's card is amber, and which axis the word is about (#2437)

[erikdarlingdata]

Part 2 of #2437 — the Lite half. (The web dashboard half is #2447: different language, different risk.)

Lite/MainWindow.xaml:442 was the same bare binding #2429 fixed in the Darling viewer:

<TextBlock Text="{Binding StatusDisplay}" Foreground="{Binding StatusBrush}"
           FontSize="11" Margin="0,0,0,10"/>

A word, a colour, and no way to ask what it meant. A Lite user reads "Warning" and scans the metric rows for the yellow one, exactly as @ehaar had to on the viewer in #2422.

The reason is built from the card's own rows, not recomputed

This is a port, not a wiring change: Lite has no FleetRollup and no BuildReason, so the reason had to be built. The property worth copying carefully is the one that makes the viewer's version work at all — BuildReason is assembled from the card's OWN metric displays, so it cannot drift from the six rows underneath it. A Lite implementation that recomputed severity would satisfy every wording assertion in the test file and still be worse than no tooltip, because the one place it would disagree is a card the reader is staring at.

StatusReason therefore quotes CpuDisplay, BlockingDisplay and DeadlockDisplay verbatim, and is gated on the same predicates the row brushes are painted from. That meant giving each row's "not green" test a name and having both readers use it rather than each carrying its own copy of the threshold:

private bool CpuIsElevated => CpuPercentForAlert >= 50;
public SolidColorBrush CpuBrush => MakeBrush(CpuIsCritical ? "#E57373" : CpuIsElevated ? "#FFB74D" : "#81C784");

and the test asserts the invariant against the shipped brushes, not against a copy of the numbers:

Assert.Equal(card.CpuBrush.Color != green, tooltip.Contains("CPU ", StringComparison.Ordinal));

Memory is deliberately absent. The Memory row carries no severity brush on this card, so there is no band to report — inventing a threshold there is the single thing this property exists to prevent.

Lite's conflation is not the viewer's, and the tooltip has to say so

Checked in source rather than assumed. The viewer derives its status from collection freshness and re-purposes HasCollectorErrors to mean "stale" — its own doc comment says so. Lite's IsOnline comes from _serverManager.GetConnectionStatus(...), a live connection check, and its HasCollectorErrors from _collectorService.GetHealthSummary(server).ErroringCollectors > 0, which counts collectors with ConsecutiveErrors > 0. So Lite does not have the stale-collection conflation — its amber "Warning" is a real collector-failure state.

It has the inverse one instead, and it is arguably sharper. Lite's status word is a connection word, not a health band: it never reflects the metric rows at all, so a card at 96% CPU with two deadlocks reads a green "Online" while its border is red. Fusing the two axes into one clause would reproduce that ambiguity in prose, so the tooltip keeps them on separate lines — what the status word means, then what the rows say.

What the tooltips actually read, run against the shipped source:

card	word	tooltip
calm	Online	Online — the last connection check succeeded / Every metric on this card is inside its threshold
CPU 96%, 2 deadlocks	Online	Online — the last connection check succeeded / Needs attention: CPU 96%, Deadlocks 2
collectors failing	Warning	Warning — one or more collectors are failing on this server / Every metric on this card is inside its threshold
collectors failing + CPU 62%	Warning	Warning — one or more collectors are failing on this server / Needs attention: CPU 62%
offline	Offline	Offline — the last connection check failed
not yet checked	Unknown	Unknown — this server has not been connection-checked yet / Needs attention: CPU 62%

each closing on Double-click the card to open this server's tab — the viewer's line verbatim, pinned rather than left to coincidence, because the point of doing all three surfaces is that a reader moving between them meets one vocabulary.

Row 2 is the one to look at. That card is in real trouble and the only thing on it that ever said so was a border colour.

Three smaller decisions

An Offline card gets no metric line. The card draws a dimming overlay across those rows precisely because the numbers under it are the last ones collected before the server went dark; demanding attention for them would contradict the card while the reader is looking at it. StatusReason still computes them — the omission is a tooltip decision made in one place, not a hole in what the card knows.

The (IsOnline, HasCollectorErrors) pair now has one reader. The word and the colour already carried their own copy of that ladder and the tooltip would have been a third. ServerCardStatus is #2429's collapse ported; its amber member is named CollectorErrors rather than Stale because that is what it means here, and the difference is written down where the next person will find it.

Review found a fifth reading I had missed, and it turned out to be a live desync rather than a latent one. CardBorderBrush read IsOffline and HasCollectorErrors raw, so on dev a card that has never been connection-checked but carries a collector-error marker draws the amber "collectors failing" border while its word reads Unknown:

status word   : Unknown
border colour : #FFD54F      <- dev
neutral border: #2A2D35

The border and IsOffline (which the offline overlay binds) now render CardStatus too; precedence is unchanged, and the amber border is literally the same amber the word is painted because both render one state. The doc claim that the flag pair has one reader is now asserted rather than left as prose — Assert.Equal(1, CountOccurrences(source, "IsOnline switch")) — which is the assertion that would have caught the overclaim when it was written.

Background="Transparent" is load-bearing, as #2429 found: a TextBlock with a null Background hit-tests on its rendered glyphs alone, so the tooltip would have appeared over the letters of "Warning" and nowhere in the space around them. The status dot carries the same tooltip — same signal, and it is the thing a reader points at first.

Lite has no AwaitingFirstCollection flag — verified before relying on it — so the status/tooltip desync family #2429 spent four review rounds on cannot arise here. Only the unexplained-word half applied.

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 and ServerSummaryItem straight out of the shipped file (never retyped — a generator reads the source and emits it) behind a System.Windows.Media shim, and all 12 tests in the new class pass against it. The table above is that harness's output. Pointed at dev's copy of the same file it does not compile at all, because none of the members exist there.

The source- and XAML-scanning half was simulated in Python against both versions: 20 of 23 assertions fail on dev and all pass on the branch. The three that pass on dev are the two guards that locate the status TextBlock and the status dot, plus the viewer's half of the closing-line parity pin, which this PR does not touch. That half has to text-scan, because a ToolTip attribute lives in XAML 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.

One thing noted and not fixed here: ServerConnection.DotStatus (Lite/Models/ServerConnection.cs:199) is a fourth copy of the same four-word ladder, for the sidebar row. It is a different type on a different surface and unifying it is not this PR's lane, but it is the next place this can drift.

🤖 Generated with Claude Code

[claude]

Review summary

Scope: C#/XAML only (Lite's Overview card tooltip), no T-SQL touched, so the T-SQL style/parity checks in CONTRIBUTING.md don't apply here directly.

Correctness. Traced the new CardStatus → StatusDisplay/StatusBrush/StatusHeadline/StatusReason/StatusTooltip chain against all five card states (healthy, busy, collector-errors, offline, not-yet-checked) plus the offline+collector-errors combination. The refactor of CpuBrush/BlockingBrush/DeadlockBrush/CardBorderBrush into named gates (CpuIsElevated, CpuIsCritical, BlockingIsElevated, DeadlocksAreElevated) is a faithful behavior-preserving transformation of the prior inline thresholds — no regression found. The offline-card special case (no "Needs attention" line, reason still computed but not surfaced) matches the stated rationale and the test suite. Nullable IsOnline/CpuPercent handling is consistent with pre-existing patterns elsewhere in the file.

Lite/Darling parity. This is intentionally a Lite-only catch-up on work the Darling viewer already got in #2429/#2422, and the PR body is explicit and accurate about where Lite's semantics diverge (connection-word status vs. the viewer's stale-collection conflation, no AwaitingFirstCollection on Lite, wording differences). Compared the closing tooltip line against Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Fleet.cs — verbatim match, and it's test-pinned. No parity drift found; the one deliberately-deferred item (ServerConnection.DotStatus as a 4th copy of the status ladder) is already called out by the author in the PR description.

Security / performance. No I/O, network, process, or user-input handling introduced — pure computed UI properties over data that was already being read. Lite.Tests' ReadRepoFile helper reads local repo files via a compile-time [CallerFilePath] walk-up, not user input. No performance concerns; the new properties are cheap getters evaluated on data-binding refresh, same cost class as the code they replace.

Left one non-blocking inline nit on LocalDataService.Overview.cs about a doc comment overclaiming that (IsOnline, HasCollectorErrors) has a single reader, when CardBorderBrush reads both directly rather than through CardStatus. No functional divergence today, just a latent drift risk worth being aware of.

Otherwise this is a clean, well-tested port — nothing blocking.

[claude]

Reviewed the diff (Lite.Tests/LiteOverviewCardExplainsItselfTests.cs, Lite/MainWindow.xaml, Lite/Services/LocalDataService.Overview.cs). No blocking issues found.

Correctness — traced the CardStatus/StatusDisplay/StatusBrush/CardBorderBrush/StatusReason/StatusTooltip chain by hand against all five ServerCardStatus states (Online, CollectorErrors, Offline, Unknown, plus the Offline+HasCollectorErrors combination). All check out:

• CardStatus is the single read of (IsOnline, HasCollectorErrors) — confirmed the other four renderings (word, brush, border, IsOffline) all switch on it rather than re-reading the raw flags, so the CardBorderBrush desync fixed in the second commit can't recur elsewhere.
• Row gates (CpuIsElevated/CpuIsCritical/BlockingIsElevated/DeadlocksAreElevated) are shared between the row brushes and StatusReason, matching the PR's stated invariant that the tooltip can't name a green row or omit a colored one.
• Confirmed via MainWindow.xaml.cs:729-731 that HasCollectorErrors on ServerSummaryItem is only ever set when connStatus.IsOnline == true, so the Offline+CollectorErrors combination the new test guards against is indeed currently unreachable in practice — the fix is correctly framed as defensive, not a live-bug fix.
• Nullable handling (IsOnline as bool?, CpuPercentForAlert as double?) is sound; the pattern-match _ => Unknown and nullable comparisons (null >= 50 → false) behave as intended.

Lite/Darling parity — verified against Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Fleet.cs: the closing tooltip line ("Double-click the card to open this server's tab") matches verbatim as claimed, and the CardStatus discriminant pattern mirrors the viewer's ServerCardStatus/Headline approach from #2429. The reduced scope (3 metrics vs. the viewer's 6, no FleetRollup/BuildReason, no AwaitingFirstCollection) is real and correctly documented as an intentional asymmetry rather than a drift — Lite genuinely has no equivalent flag/type to port.

Security / perf — no user input, network, file, or process handling introduced; property getters are cheap and only reoccur on WPF binding refresh (unchanged pattern from before this PR).

Nice touch that the second commit is itself a self-caught fix (CardBorderBrush reading the raw flags instead of CardStatus), with a regression test (Assert.Equal(1, CountOccurrences(source, "IsOnline switch"))) pinning that the discriminant has exactly one reader.