Band Lite's collection freshness, so a quiet server stops looking green (#2452)

[erikdarlingdata]

Fixes #2452 (part 1). Part 2 is split out as #2458 — see the last section for why, and why the issue's reason for pairing them did not survive the implementation.

The defect, and why it is reachable in shipped Lite

The Overview card carries five metric rows and a status word. None of them was collection freshness, so the state the issue describes — connection fine, nothing erroring, store taking no new rows for hours — rendered as a green "Online", a neutral border, and a timestamp in the plain foreground brush where four hours ago looked exactly like four seconds ago.

The issue asks for a measurement: whether Lite installs actually see collection stop without a collector erroring. Lite's own source answers it without needing one, and the path is short:

• RemoteCollectorService.RecordCollectorResult treats PERMISSIONS as not a failure — deliberately, per its comment, because a permission denial is not transient. It sets IsPermissionRestricted and leaves ConsecutiveErrors alone.
• IsCollectorPermissionRestricted then returns out of RunCollectorAsync before LogCollectionAsync, so a restricted collector writes no collection_log row at all — not even a failed one.
• GetHealthSummary counts ErroringCollectors from ConsecutiveErrors > 0.

So a login that loses rights across the board stops the store dead with ErroringCollectors at zero, the connection check still passing, and every card green. _collectorHealth is in-memory too, so an app restart zeroes every streak while the store stays exactly as stale as the previous session left it.

What changed, and the one thing that deliberately did not

Option 1 from the issue, sharing the thresholds rather than growing new ones.

ServerHealthClassifier.ClassifyFreshness out of PerformanceMonitor.Common — the same Fresh / Stale / Offline / NeverCollected ladder the Darling viewer bands, off the same ServerHealthThresholds numbers. Lite already referenced that project, so this cost one using. No threshold is added to Lite, and that is a claim about correctness rather than convenience: Lite's fastest configured collector in collection_schedule.json runs every minute, which is exactly the derivation CollectorCadence documents, so StaleThreshold is true here on its own terms rather than borrowed from a service with a different rhythm.

Option 2 — folding freshness into the status word — is turned down, and the reason is written on StatusDisplay. Lite's status word is a connection word (IsOnline is a live check, which succeeds whether or not anything is being collected) and its "Warning" already means one specific thing: collectors are erroring. #2429 found the viewer renders that same word for a stale collection and for a metric breach with nothing on the card telling them apart, which is the card @ehaar wrote in about in #2422. Reproducing that here would have traded one defect for the other.

So freshness bands its own row and names its own band there. What the card actually renders, printed from the shipped ServerSummaryItem spliced out of this branch:

collection	status word	border	Last Collect row	card tooltip's middle line
current	Online green	neutral #2A2D35	11:59:40 green	Every metric on this card is inside its threshold
stale (2m01s)	Online green	amber #FFD54F	11:57:59 (stale) amber	Needs attention: Last collect 11:57:59 (stale)
stopped (4h)	Online green	amber #FFD54F	08:00:00 (stopped) red	Needs attention: Last collect 08:00:00 (stopped)
never collected	Online green	amber #FFD54F	Never amber	Needs attention: Last collect Never

and the Last Collect row carries its own tooltip, which is the part that answers the "do not reproduce the conflation" requirement literally:

Collection is stale — nothing has landed for over 2 minutes.
This is about collection stopping, not about the server's metrics: the connection check can still be passing and no collector reporting an error.

The status word staying green across all four rows is not an oversight — it is the point. IsOnline answers "did the last connection check succeed", and it did. "Online" and "fresh" are different questions and the card now answers both, each in its own place, each in its own words. The thresholds inside the tooltip are read from ServerHealthThresholds rather than typed, so the sentence cannot come to disagree with the band it is explaining.

Three smaller decisions

The border escalates, on its own arm reading the same amber as the collector-error one. Collector errors and collector silence are one class of fault — the monitoring of this server is not working — and silence is the harder of the two to notice, which is the whole issue; a fix that left the card looking calm at a glance would have fixed a row nobody was asked to read. It stays amber even for a stopped collection: red belongs to the offline overlay, and a red border under a green "Online" would be the loudest contradiction on the card. The row underneath carries the severity in its own colour and its own word.

It is a separate arm rather than a clause on the collector-error one because #2451's TheCardBorder_RendersTheSameDiscriminantTheWordDoes pins that arm's source text — CardStatus == ServerCardStatus.CollectorErrors ? "#FFD54F" — and folding || CollectionIsNotFresh in put the new clause between the discriminant and the colour, so the literal stopped appearing. That pin is a landed guard from another lane asserting something true and worth asserting, so the branch moved rather than the pin. Two named causes reading one colour is the better shape anyway: each arm gets its own line and its own reason.

This was caught by running #2451's committed test class against this branch rather than waiting for CI to run it — and CI then failed on that one test and nothing else, which is the whole argument for doing it that way. All 12 of #2451's tests pass on this branch now.

NeverCollected is amber, never red. A server that has not been collected yet is queued, not dead — the reasoning ServerFreshness.NeverCollected already carries from the viewer, where a red "Offline" on a merely-queued server sent a 24-server field report chasing a phantom scheduler bug.

The band is stamped in GetServerSummaryAsync, the one place a ServerSummaryItem is built, rather than at the Overview loader — the two MCP reads call that method too, and a band only the Overview applied would be a fact that depended on which caller asked. The clock is handed in rather than read inside the band, so the classification stays pure over (last collection, now). null means nobody classified it: unreachable in the app, reachable from a fixture, and it renders exactly what dev rendered — the bare stamp, the card's unknown grey, no tooltip.

Background="Transparent" on both row cells is load-bearing rather than decoration, 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 and nowhere in the space around them.

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 ServerSummaryItem straight out of the shipped file (a generator reads the source and emits it — never retyped) behind a System.Windows.Media shim, with a real project reference to PerformanceMonitor.Common so the bands are the shipped classifier's. The table above is that harness's output.

	dev	this branch
the full committed test file	does not compile — 29 errors	16 passed / 0 failed
the identically-expressible subset	3 passed / 5 failed	8 passed / 0 failed

The absence is the finding rather than a measurement, so the subset runs identical bodies against both contracts. ApplyCollectionFreshness is invoked reflectively there — on dev there is genuinely nothing to stamp, which is the thing being measured, not a harness limit. The five that fail on dev are the quiet server's border, the row naming its band, the card tooltip naming the row, the XAML wiring, and the stamp itself. The three that pass on both sides are the controls, and they are the ones a wrong fix breaks first: a calm currently-collecting card keeps the neutral border (so this is not "paint every card amber"), the status word says the same thing for a stale card as for a fresh one (so this is not option 2 sneaking in), and a calm current card's tooltip still says nothing about collection.

EveryCardIsStampedWithItsBandWhereItIsBuilt is a source scan on purpose: deleting that one line compiles perfectly clean and silently returns every card to dev's behaviour, and a property nothing sets is this issue's own failure mode one level up. The XAML half has to text-scan for the same reason #2429 gave — a ToolTip attribute lives where no assertion about a C# object can reach it, and removing it compiles clean.

dev moved twice while this was open (#2451, then #2450/#2453). Rebased onto 88fe948f and re-verified: whole solution builds 0 errors, and 28 cases green — this PR's 16 plus all 12 of #2451's, run against the rebased tree. Not force-pushed, because the PR is MERGEABLE/CLEAN against that same dev and the rebase was for verification rather than for the merge.

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

Part 2 is split out as #2458, not dropped

#2452's second half — ServerConnection.DotStatus being a fourth independent copy of the four-word ladder — is filed as #2458 rather than done here, and the issue's own reason for pairing them does not hold as this landed. It argued that "a freshness band would otherwise need writing twice". It is written once, on ServerSummaryItem, and the sidebar dot is untouched, so nothing is duplicated by leaving it.

The thing that does argue for splitting: the type DotStatus should collapse onto is ServerCardStatus, and in Lite that type arrives in #2451, which is still open. Doing it here would mean either duplicating that enum or stacking on an unmerged branch — and a stacked PR reports a meaningless green.

#2451 landed while this was open, and it is what completes option 1

The issue's option 1 has two halves: band the row, and "let the tooltip name it, the way it names CPU and Blocking today". The second half needed a tooltip built from the card's own rows, which did not exist in Lite when #2452 was written. #2451 merged into dev mid-flight and brought one, so this branch was rebuilt on top of it rather than textually merged, and takes the whole option instead of half.

CollectionIsNotFresh joins CpuIsElevated / BlockingIsElevated / DeadlocksAreElevated in #2451's per-row concern gates, and StatusReason quotes LastCollectionDisplay verbatim exactly as it quotes CpuDisplay — which is the property that makes that whole design work, per its own comment: the clause and the row cannot render different things. Because the display already carries the band word, the clause names its axis without borrowing a metric's vocabulary.

It goes first in the reason. Collection is the row that says whether the other four can be believed at all: a card whose collection stopped four hours ago is showing four-hour-old CPU, and meeting "CPU 4%" before learning that is the wrong order to be told the two facts in.

TheTooltipNamesTheCollectionRowExactlyWhenItIsNotGreen extends #2451's own invariant to this row and asserts it against the shipped brush rather than a copy of the thresholds:

Assert.Equal(
    Hex(card.LastCollectionBrush) != Green,
    card.StatusTooltip.Contains("Last collect ", StringComparison.Ordinal));

🤖 Generated with Claude Code

[claude]

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

Correctness

• ApplyCollectionFreshness is stamped once inside GetServerSummaryAsync, which is confirmed to be the single construction site for ServerSummaryItem — both Lite/Mcp/McpHealthTools.cs and Lite/Mcp/McpDiscoveryTools.cs call the same method, so the "one place, two MCP reads" claim in the PR description checks out.
• The freshness classification is independent of IsOnline/HasCollectorErrors, which are set later by the caller in MainWindow.xaml.cs:723 — no ordering hazard between the two.
• Boundary/null handling (unclassified null band → grey/no tooltip, NeverCollected → amber not red, strict > on thresholds) matches the shared ServerHealthClassifier in PerformanceMonitor.Common, which this PR doesn't modify.
• CollectionFreshnessTooltip/LastCollectionDisplay read thresholds from ServerHealthThresholds rather than restating literals, so the tooltip text can't drift from the band boundaries.

Lite/Darling parity

• No Darling changes in this PR. Checked whether that's a gap: Darling's ServerSummaryItem.CardStatus (ViewerDataService.Overview.cs) already derives its status word from freshness via ApplyFreshness, since the viewer has no live connection check the way Lite does — so Darling isn't carrying the defect this PR fixes. The PR body explains the follow-up (ServerConnection.DotStatus, filed as ServerConnection.DotStatus is a fourth copy of the four-word status ladder #2458) is intentionally split out because it depends on ServerCardStatus landing in Lite via Say why Lite's card is amber, and which axis the word is about (#2437) #2451 first. Reasoning holds up.

Security/Performance

• No new SQL, no new I/O, no new external input handling. One extra DateTime.UtcNow + enum classification per card per refresh — negligible.

Nice level of test coverage pinning the anti-conflation invariant (status word never reflects freshness) and the XAML wiring via source-scan tests, given nothing here is otherwise reachable by a C# assertion.

[claude]

Reviewed. This is a well-scoped, well-tested change and I don't see any correctness, security, or parity issues to block on.

What it does: Stamps ServerSummaryItem.CollectionFreshness (via the shared ServerHealthClassifier.ClassifyFreshness in PerformanceMonitor.Common) at the single construction site in LocalDataService.GetServerSummaryAsync, so the Overview card's "Last Collect" row gets its own band/brush/tooltip instead of the plain foreground brush it had before. All three callers (MainWindow.RefreshOverviewAsync, McpHealthTools.GetServerSummary, McpDiscoveryTools) funnel through that one method, and a test pins the single construction + stamp-after-construction ordering in source.

Correctness checks I ran through:

• ClassifyFreshness's > (strict) comparisons make the boundary values (StaleThreshold, OfflineThreshold) land in the lower band, matching the new test's TheBoundariesAreTheSharedClassifiersAndAreInclusiveOfTheLowerBand and matching the classifier's existing (untouched) behavior used by Darling.
• CollectorCadence = 1 minute is verified against Lite/config/collection_schedule.json — several collectors (wait_stats, cpu_utilization, memory_stats, etc.) really do run at frequency_minutes: 1, so the PR's claim that no new threshold is needed for Lite holds up.
• Both instants passed into the classifier are UTC (DateTime.UtcNow and the DuckDB-stored naive-UTC timestamp), so the subtraction is a true elapsed time regardless of Kind, as documented.
• The status word (StatusDisplay/StatusBrush) deliberately stays untouched by freshness — confirmed CardStatus only reads (IsOnline, HasCollectorErrors), so the "Warning" word still means exactly one thing. This is the right call given Lite's status word is a live-connection check, unlike Darling's headless service which has no live ping and therefore derives status from freshness by necessity (DarlingMcpDataTools.FreshnessStatus) — so this isn't Lite/Darling drift, it's the two apps correctly doing different things for different reasons, and it's the same conflation Say why the card is amber, and give "+52 more" somewhere to go (#2424) #2429/[QUESTION] Overview Warnings #2422 already fixed once.
• No other code path reads LastCollectionDisplay/LastCollectionBrush/CollectionFreshnessTooltip, so appending " (stale)"/" (stopped)" to the display string can't break another consumer.
• XAML: both new TextBlocks get Background="Transparent" so the tooltip hit-tests over the whole cell rather than just the glyphs — matches the #2429 note and is tested (TheCardsLastCollectRowIsBoundToTheBand).

No T-SQL in this diff, so the collector-query style conventions don't apply here. I wasn't able to actually run dotnet build/dotnet test in this sandbox (commands require interactive approval I don't have), so this is a static review only — worth having CI confirm the new Lite.Tests/ServerCollectionFreshnessTests.cs suite passes before merge.