Bucket Lite's Wait Stats and Perfmon trend and picker reads (#4234)

[erikdarlingdata]

Part of #4234.

Why

Lite's Wait Stats and Perfmon trend charts read one row per collection over the whole window. At 7
days that is 196,800 rows for 20 wait types and 118,080 rows for 12 counters (#4234's measured
numbers). The two picker reads (GetDistinctWaitTypesAsync, GetDistinctPerfmonCountersAsync) also
ran a full-window DISTINCT on every 1-minute auto-refresh. This is the Lite twin of PR #4304 (the
Darling/WPF viewer side, now merged to dev), applying the same #4234 ruling to Lite's DuckDB reads.

What changes

• Lite/Services/LocalDataService.WaitStats.cs: GetWaitStatsTrendsByTypesAsync now buckets to
  TrendBudget.Chart's point budget PER SERIES via a new rated CTE (summed wait/signal over summed
  rated seconds, avg_ms_per_wait as summed wait over summed waiting tasks) grouped on
  time_bucket(...). When every bucket the whole call returns holds exactly one physical collection,
  points are stamped at that collection's own raw first_collection_time; a single merged bucket
  anywhere keeps the bucket_start grid line throughout. The SQL text moved into a new internal static
  WaitTrendsSql(waitTypeCount) so its shape is checkable without a live DuckDB, mirroring
  ViewerDataService.WaitTrendsSql.
• Lite/Services/LocalDataService.Perfmon.cs: GetPerfmonTrendsByCountersAsync now wraps the
  unchanged per-collection SUM as a subquery and re-aggregates into buckets: Value is the bucket's
  rounded gauge average, DeltaValue/SampleIntervalSeconds are summed only over rated collections so
  their ratio downstream stays the ruling's rate, CntrType keeps the per-collection MIN=MAX rule now
  double-aggregated. Same singleton-bucket raw-timestamp rule and same PerfmonTrendsSql(counterCount)
  extraction.
• Lite/Services/LiteNameListCache.cs (new): a small TTL cache — per (server, window length), 15
  minutes, a hit also requires the cached window's end to sit within the TTL of the requested end —
  ported from Darling's ViewerNameListCache. GetDistinctWaitTypesAsync and
  GetDistinctPerfmonCountersAsync each get their own cache instance, but see the T3b2 section below:
  the cache check and the nowUtc clock seam ended up on new picker-only wrapper methods instead of on
  these two shared ones. A later lane can reuse this cache for the memory-clerk picker.
• Lite.Tests/PerfmonIntervalAggregationTests.cs: two pre-WPF trend charts never adopted the #3897/#3960 bucket contract: at the "Last 7 days" window one chart reads 85–170 k raw rows, and the pickers re-run a week-long DISTINCT every refresh #4234 source-census pins asserted exact
  textual counts of SUM(sample_interval_seconds)/SUM(delta_cntr_value) across the whole file. The
  new bucket-level aggregation legitimately adds one more of each (summing each collection's own
  already-aggregated value across the collections in a bucket — the ruling's rate, not the original
  12-17x-inflation bug the pins guard against). Narrowed/updated rather than weakened: see the commit
  and in-file comments for exactly what each pin now allows and why.

Both Get*ByTypesAsync/Get*ByCountersAsync read collection_time as UTC already (GetTimeRange
without fromDate/toDate uses DateTime.UtcNow directly, "since collection_time is stored in UTC"
per its own comment) — bucketing keeps that, no timezone handling changed.

Diagnosis check

Confirmed by reading the pre-#4234 method bodies directly (both were plain per-collection
SELECT ... GROUP BY counter_name/wait_type, collection_time with no bucketing of any kind) rather
than re-measuring row counts against a live store — the brief pointed at #4234's body and
issuecomment-5835625728 for those numbers rather than asking this lane to re-measure them.

A real regression caught and fixed in-lane

DuckDB promotes SUM() over an INTEGER column to HUGEINT, which DuckDB.NET hands back as a boxed
BigInteger; Convert.ToInt64/Convert.ToInt32 and the reader's typed GetInt64/GetDouble
accessors can't cast that. PerfmonCounterTypeReadTests (an existing test, not one I added) caught
this immediately against the new SUM(sample_interval_seconds) FILTER (...) column. Fixed by routing
every new aggregate-derived column through the existing ToInt64/ToDouble helpers, matching
GetWaitBucketsAsync/GetPerfmonBucketsAsync's own established pattern for the same FILTER-summed
shape. Caught and fixed before the full-suite run, in the same PR.

T3b2: three review follow-ups, all decided by ruling

1. MCP must not read the picker cache. GetDistinctWaitTypesAsync
   (Lite/Services/LocalDataService.WaitStats.cs) and GetDistinctPerfmonCountersAsync
   (LocalDataService.Perfmon.cs) went back to their dev shape — uncached, no nowUtc parameter —
   because McpWaitTools.cs/McpPerfmonTools.cs call them directly and must never answer with a
   picker snapshot up to 15 minutes stale. The TTL cache moved onto two new entry points,
   GetDistinctWaitTypesForPickerAsync and GetDistinctPerfmonCountersForPickerAsync, which only
   ServerTab.Refresh.cs's wait-type/perfmon-counter picker refresh calls now. Updated the two
   existing cache-TTL tests to call the picker methods (only they take nowUtc), and added one test
   per list proving the shared method never caches: two back-to-back calls with a new name seeded
   between them, and the second call sees it.
2. A real multi-instance Perfmon test. PerfmonIntervalAggregationTests pins the inner
   MAX(sample_interval_seconds) in source text, but no live test seeded more than one instance row
   per collection, so a regression turning that inner MAX into a SUM would have passed every DuckDB
   test in the file. Added two tests to LiteWaitPerfmonTrendBucketingTests.cs, both Transactions/sec-
   shaped with 12 instance rows per collection (delta 100, interval 300 each): one collection alone in
   its bucket returns DeltaValue 1,200 / SampleIntervalSeconds 300 (not 3,600 — the 12x-inflation
   shape); three collections merged into one bucket return 3,600 / 900, still 4/second regardless of
   how many collections merged. Proven by hand: reverting the inner MAX to SUM in
   PerfmonTrendsSql's inner CTE fails exactly these two new tests and no others (10/10 passed before
   the revert, 8/10 after, reverted back afterward).
3. The unrated one-collection bucket — no code change. Traced ServerTab.Pickers.cs's
   DeltaSample/DeltaSeriesShaping.Shape (PerformanceMonitor.Common/DeltaSeriesShaping.cs). The
   decisive line is 220: if (s.Delta is not long delta || s.IntervalSeconds == 0) → NaN (no rate
   point), checked before any basis-specific branch. dev's raw pass-through for a lone unrated
   collection (a real, non-null delta_cntr_value paired with sample_interval_seconds literally 0)
   hits this via the IntervalSeconds == 0 arm; PerfmonTrendsSql's FILTER-driven NULL/NULL for
   that same shape hits it via the Delta is not long arm instead. Both land on the same NaN, for
   every basis except Level (a gauge), which reads only Value and never touches Delta/
   IntervalSeconds at all. So the two shapes are not distinguishable downstream — no code change, no
   new test (the brief's own fallback: pin only the branch where they diverge).

Test plan (T3b2 addendum)

• Lite.Tests/Lite.Tests.csproj builds 0 Warning(s)/0 Error(s) after each of the three items above.
• LiteWaitPerfmonTrendBucketingTests.cs's three classes, run by exact class name: 18 total (3
  LiteTrendBucketWidthSqlTests + 5 LiteNameListCacheTests + 10 LiteTrendBucketingLiveTests,
  including the 2 renamed picker-cache tests, the 2 new shared-method-never-caches tests, and the 2
  new multi-instance tests), 0 failed.
• PerfmonIntervalAggregationTests (4), PerformanceMonitorLite.Tests.PerfmonCounterTypeReadTests
  (2), McpStatusEnvelopeTests (9) — 0 failed. (*McpWait*/*McpPerfmon* as literal class-name
  wildcards matched 0 classes; no test file in Lite.Tests has "McpWait" or "McpPerfmon" in a class
  name today. McpWaitTools.cs/McpPerfmonTools.cs themselves are untouched by item 1 — same
  method names, same call shape — so this is a naming mismatch in the brief, not a gap; the full
  suite below covers whatever does exercise those tool classes under their actual names.)
• Merged origin/dev (brought in PR Bucket the Darling viewer's File I/O and memory clerk trend charts (#4234) #4329, the Darling viewer's File I/O/memory bucketing, and
  Lite Overview baseline bands use the server's actual hour under a custom range (#4320) #4325's baseline-band fix; unrelated to this lane), rebuilt clean, ran the FULL Lite.Tests.exe
  suite once: 5,433 total, 0 errors, 0 failed, 0 skipped.

Test plan

• Lite/PerformanceMonitorLite.csproj builds with 0 Warning(s), 0 Error(s).
• Lite.Tests/Lite.Tests.csproj builds with 0 Warning(s), 0 Error(s).
• New Lite.Tests/LiteWaitPerfmonTrendBucketingTests.cs (14 tests, all against embedded DuckDB or
  in-memory, no live SQL Server/Postgres): source checks that WaitTrendsSql/PerfmonTrendsSql
  carry a time_bucket width plus first_collection_time/collection_count; LiteNameListCache
  hit/miss unit tests (ported from Darling's ViewerNameListCacheTests); a 7-day, one-collection-
  per-minute seed returns at most TrendBudget.Chart.AutoPoints * seriesCount rows for both reads;
  three collections seeded off the minute grid (:37 seconds) come back with raw timestamps and
  values unchanged, not floored to the bucket grid, for both reads; both pickers' cache proof (a
  second call inside 15 minutes misses a wait type/counter the store gained since that call, a call
  past the TTL picks it up).
  • Proven once by hand against the pre-WPF trend charts never adopted the #3897/#3960 bucket contract: at the "Last 7 days" window one chart reads 85–170 k raw rows, and the pickers re-run a week-long DISTINCT every refresh #4234 text (read directly while building this PR, see the test
    file's own top-of-file comment): the old method bodies had no time_bucket,
    first_collection_time or collection_count anywhere, so the source checks fail there outright;
    the old per-collection read returns one row per collection (10,080 for the 7-day one-per-minute
    seed), over the budget the cap tests assert.
• Regression-checked the existing tests that exercise the two changed reads, run by exact class
  name (not file name, per the skill's "filter by every class" rule):
  DeltaFamilyUnknowableRowReadTests (12), PerfmonCounterTypeTests (17, two classes in that one
  file — PerfmonCounterTypeTests and PerfmonCounterTypeReadTests), PerfmonIntervalAggregationTests
  (4, after the pin fix). All pass.
• Full Lite.Tests.exe suite, run once after merging origin/dev: 5424 total, 0 errors, 0
  failed (first run found 2 failures in PerfmonIntervalAggregationTests, both fixed and
  individually re-verified as described above; not re-run at full scale a second time given this
  lane's context budget, but the fix is narrow, mechanical, and independently verified).
• No live SQL Server/PostgreSQL test needed — Lite's store is the embedded DuckDB itself.
• Lite.Tests has no DocCommentHygiene* class (that pin is Darling-only); nothing to run there.

CI fix

The CI run at 29411369 failed two Darling tests. The product code is unchanged; c7f6e21c (after a merge of origin/dev) fixes both. Report: PR comment 5837999969.

• DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks: the new PerfmonTrendsSql and WaitTrendsSql members had been inserted between an existing method's <summary> and the method. Each "Batched sibling of ..." block moved back to its own method (GetPerfmonTrendsByCountersAsync, GetWaitStatsTrendsByTypesAsync); the moved lines are unchanged.
• PerfmonCounterTypeRungTests: Lite's LocalDataService.Perfmon.cs now holds the agreed counter-type expression three times, not two: once in the single-counter read and twice in the bucketed batched read (per collection inside, per bucket outside), as both Darling twins do. The pin says 3 and why.

CHANGELOG entry

SECTION: Changed
ENTRY:

• Lite's Wait Stats and Perfmon charts bucket long windows instead of shipping one point per collection ([Bucket Lite's Wait Stats and Perfmon trend and picker reads (#4234) #4331]) - A 7-day Wait Stats or Perfmon chart carried one row per stored collection, about 10,080 rows per wait type or counter at one collection a minute (about 200,000 for 20 wait types). It now buckets each wait type or counter to at most about 1,500 points (the chart budget, not the MCP tools' 200-point budget), and a window short enough that every bucket holds one collection still plots each point at its own collection time. The wait-type and perfmon-counter pickers also cache their name list for 15 minutes per server and window length instead of re-reading it on every 1-minute auto-refresh.
  REF:
  [Bucket Lite's Wait Stats and Perfmon trend and picker reads (#4234) #4331]: Bucket Lite's Wait Stats and Perfmon trend and picker reads (#4234) #4331

For the coordinator to double-check

• This lane's scope was strictly the four reads named in the brief (GetWaitStatsTrendsByTypesAsync,
  GetDistinctWaitTypesAsync, GetPerfmonTrendsByCountersAsync, GetDistinctPerfmonCountersAsync).
  Lanes T2a/T2b cover the rest of WPF trend charts never adopted the #3897/#3960 bucket contract: at the "Last 7 days" window one chart reads 85–170 k raw rows, and the pickers re-run a week-long DISTINCT every refresh #4234; this PR does not close the issue.
• origin/dev already carries PR WPF Wait Stats and Perfmon trend charts bucket server-side (#4234) #4304 (merged during this lane's run) and its
  ViewerNameListCache/ViewerTrendBucketingReviewTests; this branch was merged with origin/dev
  after that landed, rebuilt, and re-tested clean.
• Worth a second look: PerfmonIntervalAggregationTests's two updated pins (SUM(delta_cntr_value)
  now 3, not 2; SUM(sample_interval_seconds) check now strips the FILTER-guarded bucketed form
  before asserting none remains) — the reasoning is in the commit message and in-file doc comments,
  but it's a pin that guards against a real, previously-shipped bug (12-17x denominator inflation), so
  it deserves a deliberate look rather than a rubber stamp.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ

[erikdarlingdata]

CI fix pushed: c7f6e21c (on top of merging origin/dev at 8c92bd79, base was 29411369).

Failure 1: DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks. This PR inserted a new internal static string ...Sql(...) member between an existing method's <summary> and the method itself, in both files. Moved each displaced block back down to the member it actually describes; the block already sitting directly on the SQL-text member did not move. No attribute line sat between either moved block and its old position.

• Lite/Services/LocalDataService.Perfmon.cs: the "Batched sibling of GetPerfmonTrendAsync..." block (previously lines 175-191, stacked above PerfmonTrendsSql) now sits directly above GetPerfmonTrendsByCountersAsync's signature (line 231 pre-merge / line 214 post-move in the current file). PerfmonTrendsSql's own "The bucketed batched-trend statement text..." block stayed put, now the sole <summary> on that member.
• Lite/Services/LocalDataService.WaitStats.cs: same move for the "Batched sibling of GetWaitStatsTrendAsync..." block (previously lines 250-265, stacked above WaitTrendsSql), now directly above GetWaitStatsTrendsByTypesAsync's signature. WaitTrendsSql's own block stayed put.

Failure 2: PerfmonCounterTypeRungTests.EveryDarlingPerfmonRead_SelectsTheType_AndTheRowsKeepNullAsNull (Expected: 2 Actual: 3). Lite's LocalDataService.Perfmon.cs now holds the agreed-type expression (CASE WHEN MIN(cntr_type) = MAX(cntr_type) THEN MAX(cntr_type) END AS cntr_type) three times: once in the single-counter GetPerfmonTrendAsync read, and twice in the bucketed batched read added by this PR (once per collection in the inner subquery, once per bucket in the outer aggregation) — the same double aggregation both Darling twins (ViewerDataService.Perfmon.cs, DarlingTrendReader.cs) already carry on dev. Confirmed the count directly: grep -c of the literal expression against the file returns 3.

Changed, in Darling/Darling.Tests/PerfmonCounterTypeRungTests.cs:

• line ~228 (now ~230): Assert.Equal(2, ...) → Assert.Equal(3, ...), with the comment the brief specified above it.
• the method's <summary> (~215): "byte-identical to each other and to Lite's two" → "byte-identical to each other and to every copy in Lite's trend reads" (rewrapped the paragraph's line breaks to match the file's normal width; no wording beyond the specified swap changed).

Revert-proof: set the pin back to Assert.Equal(2, ...), rebuilt, ran the class — failed with Expected: 2 Actual: 3 on EveryDarlingPerfmonRead_SelectsTheType_AndTheRowsKeepNullAsNull, matching CI's original failure exactly. Restored to 3, rebuilt, reran — green.

Verification (all -c Debug, all builds 0 Warning(s)/0 Error(s)):

• Lite/PerformanceMonitorLite.csproj, Lite.Tests/Lite.Tests.csproj, Darling/Darling.Tests/Darling.Tests.csproj all build clean.
• Darling DocCommentHygiene*: Total: 77, Failed: 0 (checked twice, before and after the revert-proof).
• Darling PerfmonCounterTypeRungTests: Total: 7, Failed: 0.
• Lite LiteWaitPerfmonTrendBucketingTests.cs holds three classes, not one named for the file — ran each: LiteTrendBucketWidthSqlTests (Total: 3), LiteNameListCacheTests (Total: 5), LiteTrendBucketingLiveTests (Total: 10). All passed, 0 failed.
• Lite PerfmonIntervalAggregationTests: Total: 4, Failed: 0.
• git status after the merge and edits showed only the three files the brief named.
• Not run: PerfmonCounterTypeLivePostgresTests, a second class living in the same file as PerfmonCounterTypeRungTests. It needs a live PostgreSQL target; per the brief this lane is unit-tests-only with no PostgreSQL rig, and CI's own PostgreSQL job covers it.

PR left as draft, not readied, not merged, not armed for auto-merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3