Bucket the viewer's query/procedure/execution-count trend charts server-side (#4234)

[erikdarlingdata]

Part of #4234.

Why

The WPF Darling viewer's Performance Trends tab read the query-duration, procedure-duration and
execution-count charts (ViewerDataService.QueryTrends.cs) at the raw tier as one row per collection. A
7-day chart on a 1-minute cadence could ship thousands of rows per series, the same defect PR #4304 already
fixed for the Wait Stats and Perfmon trend charts. This lane (T2d) applies the same bucketing to the
remaining raw-tier reads named in #4234: QueryDurationTrendSql, ProcedureDurationTrendSql and
ExecutionCountTrendSql. The hourly tier and QueryStoreDurationTrendSql were out of scope (already
bounded).

What changes

• DurationTrendRouting.BuildBucketedRawTrendSql(rawTable, withDatabaseFilter) (Storage): added a
  withDatabaseFilter bool, mirroring BuildRawTrendSql. With the filter, $4 is the viewer's guarded
  text[] database filter and the bucket width moves to $5; every MCP caller passes false, so their SQL
  text and $4 width are byte-identical to before. Added a trailing COUNT(*) AS collection_count column
  (the builder already had first_collection_time); trailing so the MCP tools, which read this statement
  positionally, are unaffected.
• DarlingTrendReader.cs: the two MCP callers (QueryDurationTrendSql, ProcedureDurationTrendSql fields,
  ~750/~995) now pass withDatabaseFilter: false explicitly. Same SQL text as before, mechanical change to
  satisfy the new required parameter.
• ViewerDataService.QueryTrends.cs:
  • QueryDurationTrendSql / ProcedureDurationTrendSql now call BuildBucketedRawTrendSql(table, withDatabaseFilter: true) instead of the old per-collection QueryDurationTrendRawSql /
    ProcedureDurationTrendRawSql wrappers (those wrappers are unchanged and still used directly by
    DarlingMcpTrendToolsTests, so nothing was deleted).
  • ExecutionCountTrendSql has no shared builder (it is the only raw trend that projects one rate), so it
    is bucketed in place, following the same raw → rated → bucketed-SELECT shape, changed from const string to static readonly string because the interpolated TrendBucketSql.OriginSql reference is not
    a compile-time constant.
  • New ReadBucketedDurationTrendAsync: the PR WPF Wait Stats and Perfmon trend charts bucket server-side (#4234) #4304 pattern (GetWaitStatsTrendsByTypesAsync) ported to
    "one series per call": computes the auto bucket width against TrendBudget.Chart.AutoPoints, buffers
    every row, and stamps each point at its bucket's own first_collection_time (not bucket_start) only
    when EVERY bucket the call returned held exactly one physical collection; a single merged bucket anywhere
    keeps bucket_start for the whole series. A NULL rate (nothing rated in a bucket) is skipped, matching
    the existing ReadTrendPointsAsync convention.
  • ReadRoutedDurationTrendAsync (shared body of the two duration trends) and GetExecutionCountTrendAsync
    now branch: the RAW tier goes through ReadBucketedDurationTrendAsync; the HOURLY tier is untouched
    (already bounded to one row per hour, needs no further bucketing).
  • Doc comments updated where they claimed the raw tier was per-collection (QueryTrendSeries.Tier, the
    three SQL fields' summaries, ReadRoutedDurationTrendAsync's summary).
• Tests:
  • DarlingMcpTrendToolsTests.RawDurationTrendSql_ReadsTheStoredInterval_ThreeState_AndTheRawConstsAreItsAliases:
    updated the equality/identity assertions that compared the viewer's query/procedure fields against the
    old per-collection builder output to compare against the bucketed builder instead (both with the
    argument's reason stated in the doc comment), and updated the two source-text pins on
    DarlingTrendReader.cs's declarations for the new withDatabaseFilter: false argument.
  • ViewerTrendRoutingPortTests.ExecutionCountTrend_RoutesWithItsSibling_AndReadsTheSharedRollupColumn:
    updated the raw-arm source pin from ReadTrendPointsAsync(...) to ReadBucketedDurationTrendAsync(...).
  • New ViewerQueryTrendBucketingReviewTests.cs (ruling item 6's three required tests, per read):
    ViewerQueryTrendBucketWidthSqlTests (no live store) pins that each of the three SQL texts carries a
    bucket width (date_bin(CAST($5 AS integer) ...) and the first_collection_time/collection_count
    columns; ViewerQueryTrendBucketingLiveTests (gated on DARLING_TEST_PG) pins a 7-day window at
    ≤ TrendBudget.Chart.AutoPoints rows per series, and point equality (timestamps and values, seeded off
    the minute grid at :37 seconds) against the pre-bucketing shape when every bucket holds exactly one
    collection.

I confirmed the new source-check tests fail on the pre-fix code by hand: before this change none of the
three SQL texts contained date_bin anywhere (the old per-collection reads had no bucketing at all), so
Assert.Contains("date_bin(CAST($5 AS integer) ...", ...) fails on origin/dev.

Left DurationTrendRouting.QueryDurationTrendRawSql / ProcedureDurationTrendRawSql (the per-collection
builder wrappers) in place, unused by the viewer now but still exercised directly by
DarlingMcpTrendToolsTests's cross-checks — removing them was outside this lane's scope.

Follow-up (lane T2e)

Closed the gap the paragraph above used to flag: DescribeTrendCoverage's raw arm now names the bucket
width when points were merged (Source: raw (one point per 15 minutes), and so on) instead of always
claiming one point per collection, and the "data begins" instant ReadRoutedDurationTrendAsync /
GetExecutionCountTrendAsync disclose is now the first served collection's own timestamp
(first_collection_time) rather than the date_bin bucket start it landed in.

CHANGELOG entry

SECTION: Changed
ENTRY:

• Darling viewer's Performance Trends charts bucket server-side over long windows ([Bucket the viewer's query/procedure/execution-count trend charts server-side (#4234) #4333]) - The query duration, procedure duration and execution count charts read one point per collection on the raw tier, about 10,080 points each for a 7-day window at one collection a minute. The store now gathers collections into buckets sized to the window, rating each bucket as its summed work over its summed seconds, and the chart title names the bucket width. A window narrow enough that every bucket holds one collection plots each point at its own collection time, as before.
  REF:
  [Bucket the viewer's query/procedure/execution-count trend charts server-side (#4234) #4333]: Bucket the viewer's query/procedure/execution-count trend charts server-side (#4234) #4333

Test plan

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug — 0 Warning(s), 0 Error(s).
• Targeted classes against a local PostgreSQL 18 + TimescaleDB rig (port 55984):
  ViewerQueryTrendBucketWidthSqlTests, ViewerQueryTrendBucketingLiveTests,
  DarlingMcpTrendToolsTests, ViewerTrendRoutingPortTests, *DurationTrend*, *TrendRouting*,
  *QueryTrend*, ViewerTrendBucketingReviewTests, McpPayloadContractCensusTests,
  DocCommentHygiene*, DatabaseFilterTests, ViewerQueriesRestTests — 242 tests, 0 failures, 0 errors
  (re-run after merging origin/dev).
• Lane T2e, own rig (PostgreSQL 18 + TimescaleDB, port 55988): dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug — 0 Warning(s), 0 Error(s), after this lane's
  changes and again after merging origin/dev.
• Lane T2e targeted run (ViewerTrendRoutingPortTests, both classes in
  ViewerQueryTrendBucketingReviewTests.cs, DarlingMcpTrendToolsTests, *QueryTrend*,
  *TrendRouting*, DocCommentHygiene*): 140 tests, 0 failures, 0 errors — run once before merging
  origin/dev and once after, both clean.
• Lane T2e proved the new EffectiveStartUtc live assertion actually exercises the fix: reverted
  ReadRoutedDurationTrendAsync's firstServedUtc back to points[0].CollectionTime (the pre-fix
  shape), rebuilt, and ran
  QueryDurationTrend_SevenDayWindow_SeedStartsThreeDaysIn_EffectiveStartUtcIsFirstCollection_NotABucketStart
  alone — 1 test, 1 failure (Assert.Equal() Failure: Values differ), confirming it catches the bug.
  The revert was never committed; restored and rebuilt clean before the runs above.
• Full suite: not run here — CI runs it.
• Installer.Tests: intentionally not run (out of scope per lane orders).

🤖 Generated with Claude Code

https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ