The rest of the trend family buckets to a point budget: wait, CPU, tempdb, memory, perfmon, PG query duration (#3960)

Darling/Darling.Tests/ConsumedTimestampFrameDisciplineTests.cs

@@ -968,10 +968,17 @@ are relabelled here rather than deleted. The one still unmarked is a Lite-only s
             "event_time", "default_trace_events", 1,
             "#3202: the read projects event_time_utc and the payload stamps THAT, so the field name is the "
             + "column's while the value is not"),
-        (SiteLabel.DeSkewedAtRead, "Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs",
-            "sample_time", "cpu_utilization_stats", 1,
-            "#1262: get_cpu_utilization de-skews sample_time in SQL by the per-batch quantised offset, then "
-            + "buckets the de-skewed value, so the emitted expression is the bucket key"),
+        /* #3960 REMOVED this row (get_cpu_utilization's sample_time, cpu_utilization_stats, DarlingMcpDataTools.cs):
+           the SQL still de-skews sample_time exactly as before (CpuUtilizationBucketedSql wraps CpuUtilizationSql
+           unchanged), but the JSON emission moved from an inline `sample_time = g.Key.ToString("o")` projection into
+           the shared TrendPayloads.CpuUtilization builder's generic Stamp(DateTime) helper — which every bucketed
+           trend's timestamp now goes through, PerformanceMonitor.Common/Mcp, outside McpSourceFiles' reach, and with
+           no field name textually beside its ToString("o") for McpPayloadEmission to key on. The census cannot see a
+           site it cannot reach; the site itself did not regress (TrendPayloadBudgetLiveTests and
+           DarlingMcpTrendToolsTests.MemoryTrend_Description_PromisesTheJoinedGrantSeries_AndNamesTheNullGap's
+           siblings still exercise get_cpu_utilization's values against a live store). A future column that reaches
+           TrendPayloads still de-skews at read in its own SQL is equally invisible here; broadening this census to
+           scan PerformanceMonitor.Common's shared builders is future work, not #3960's. */
 
         /* ── desktop renders: the column's frame against the renderer's (#3207) ──
 
@@ -994,10 +1001,14 @@ the render label from 22 to 0. Both labels moved by fixing their sites and delet
        #3419 took the de-skewed label from 34 to 26 without fixing anything: the eight plan_correction
        sites are not sites at all, because the column is UTC in the store and the four surfaces that reach
        it now emit it unconverted. A site whose column and whose consumer are in the same frame has nothing
-       for this census to say about it. */
+       for this census to say about it.
+
+       #3960 took it from 26 to 25: get_cpu_utilization's sample_time row is gone, not fixed nor broken — the
+       row above explains why the census can no longer reach that site now that it buckets through the shared
+       TrendPayloads.CpuUtilization builder. */
     private const int McpPayloadUnmarkedSites = 1;
     private const int DesktopRenderMismatchSites = 0;
-    private const int DeSkewedAtReadSites = 26;
+    private const int DeSkewedAtReadSites = 25;
 
     /* ═══════════════════════ 5. resolving which table a site's column came from ═══════════════════════ */
 
@@ -1258,18 +1269,22 @@ empty found here is a broken matcher rather than a clean tree. It goes when that
     /// from the file's own reach. Pinned at set equality so the decline cannot grow quietly, which is the
     /// only thing that makes "cannot key on the column name" a cost rather than an excuse.
     ///
-    /// <para>All nineteen are Query Store, system_health, long-query or PostgreSQL reads, and #3206's own
+    /// <para>All eighteen are Query Store, system_health, long-query or PostgreSQL reads, and #3206's own
     /// sweep checked each of them rather than assuming: <c>query_store_stats.last_execution_time</c> is
     /// genuinely naive UTC, Lite's Default Trace read already de-skews (#2967), and
     /// <c>memory_pressure_events.sample_time</c> is UTC per #2932. None is an offender — but this guard is
-    /// not what establishes that, and it does not pretend to.</para>
+    /// not what establishes that, and it does not pretend to.
+    ///
+    /// <para>#3960 REMOVED Lite <c>McpCpuTools.cs</c>'s row (get_cpu_utilization's sample_time, declined here
+    /// because a Lite tool file carries no FROM/JOIN for the census to resolve a table from): the emission moved
+    /// into the shared <c>TrendPayloads.CpuUtilization</c> builder, the same move and the same reasoning as the
+    /// DeSkewedAtRead row this change removes from Darling's side, below.</para>
     /// </summary>
     private static readonly (string File, string Column, int Sites)[] DeclinedAmbiguousMcpSites =
     [
         ("Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs", "last_execution_time", 1),
         ("Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgCpuUtilizationTools.cs", "sample_time", 1),
         ("Lite/Mcp/McpBlockingTools.cs", "event_time", 2),
-        ("Lite/Mcp/McpCpuTools.cs", "sample_time", 1),
         ("Lite/Mcp/McpDefaultTraceTools.cs", "event_time", 1),
         ("Lite/Mcp/McpHealthParserTools.cs", "event_time", 9),
         ("Lite/Mcp/McpLongQueryTools.cs", "event_time", 1),
@@ -1746,10 +1761,14 @@ public void EveryDeSkewedAtReadSite_CarriesItsConversionInTheReaderItDependsOn()
     {
         var evidence = new (string Column, string ReaderFile, string Conversion)[]
         {
+            /* #3960 REMOVED the ("sample_time", DarlingDataReader.cs, "MAX(sample_time) OVER (...)") row that
+               stood here: the conversion still runs, unchanged, inside CpuUtilizationSql (CpuUtilizationBucketedSql
+               wraps it verbatim) — only the once-inline JSON emission this evidence backed moved to the shared
+               TrendPayloads.CpuUtilization builder, which the DeSkewedAtRead inventory above can no longer see
+               (see that array's own #3960 comment). Keeping this row would assert "sample_time" is still a
+               DeSkewedAtRead column, which is no longer true of the CENSUS even though it stays true of the SQL. */
             ("event_time", "Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingDefaultTraceReader.cs",
                 "dte.event_time - make_interval(mins => svr.offset_minutes) AS event_time_utc"),
-            ("sample_time", "Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingDataReader.cs",
-                "MAX(sample_time) OVER (PARTITION BY server_id, collection_time) - collection_time"),
             ("blocked_last_tran_started", "Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingBlockingReader.cs",
                 "blocked_last_tran_started - make_interval(mins => svr.offset_minutes) AS blocked_last_tran_started"),
             ("blocking_last_tran_started", "Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingBlockingReader.cs",

Darling/Darling.Tests/DarlingCustomViewsTests.cs

@@ -190,7 +190,7 @@ public void Catalog_ParamsNameTheActualWireQueryKeys_NotCSharpParamNames()
            read — the whole reason the metadata is hand-authored (reflection would emit the C# param names). */
         AssertParamKeys("get_query_store_top", "server", "hours", "top", "database_name", "as_of");
         AssertParamKeys("get_wait_stats", "server", "hours", "limit", "as_of");
-        AssertParamKeys("get_wait_trend", "wait_type", "server", "hours", "as_of");
+        AssertParamKeys("get_wait_trend", "wait_type", "server", "hours", "as_of", "bucket_minutes");
         AssertParamKeys("get_plan_xml", "query_hash", "server", "database_name");
         AssertParamKeys("get_top_queries_by_cpu", "server", "hours", "top", "database_name", "parallel_only", "min_dop", "as_of");
         AssertParamKeys("compare_analysis", "server", "hours", "baseline_hours_back", "as_of");

Darling/Darling.Tests/DarlingMcpTrendToolsTests.cs

@@ -77,10 +77,10 @@ private static (string Name, bool Optional)[] McpParams(string toolName)
     }
 
     [Theory]
-    [InlineData("get_memory_trend", "server_name,hours_back,as_of")]
-    [InlineData("get_perfmon_trend", "counter_name,server_name,hours_back,as_of")]
-    /* #3897: bucket_minutes (and on the file I/O trend database_name) appended BEHIND as_of, so a positional
-       caller of the pre-#3897 contract still binds; Lite's twins carry the same lists. */
+    /* #3897 / #3960: bucket_minutes (and on the file I/O trend database_name) appended BEHIND as_of, so a
+       positional caller of the earlier contract still binds; Lite's twins carry the same lists. */
+    [InlineData("get_memory_trend", "server_name,hours_back,as_of,bucket_minutes")]
+    [InlineData("get_perfmon_trend", "counter_name,server_name,hours_back,as_of,bucket_minutes")]
     [InlineData("get_file_io_trend", "server_name,hours_back,as_of,bucket_minutes,database_name")]
     [InlineData("get_query_trend", "query_hash,database_name,server_name,hours_back,as_of")]
     [InlineData("get_query_duration_trend", "server_name,hours_back,as_of,bucket_minutes")]
@@ -106,16 +106,16 @@ public void ParamContract_ServerNameOptional_RequiredKeysAreNot()
     }
 
     /// <summary>#3529's description half, superseded by the #3548 join: the tool now DELIVERS granted
-    /// memory (joined per point from the grants series), so the description may promise it again — but it
-    /// must name the null gap rather than promising an always-filled field, and still point at
+    /// memory (joined per bucket from the grants series since #3960), so the description may promise it again —
+    /// but it must name the null gap rather than promising an always-filled field, and still point at
     /// get_memory_grants as the series' own tool.</summary>
     [Fact]
     public void MemoryTrend_Description_PromisesTheJoinedGrantSeries_AndNamesTheNullGap()
     {
         var method = ToolMethods().Single(m => m.GetCustomAttribute<McpServerToolAttribute>()!.Name == "get_memory_trend");
         var description = method.GetCustomAttribute<DescriptionAttribute>()!.Description;
 
-        Assert.Contains("granted memory joined per point", description, StringComparison.Ordinal);
+        Assert.Contains("granted memory from the memory-grant series joined per bucket", description, StringComparison.Ordinal);
         Assert.Contains("total_granted_mb is null", description, StringComparison.Ordinal);
         Assert.Contains("get_memory_grants", description, StringComparison.Ordinal);
     }

Darling/Darling.Tests/DarlingMemoryTrendGrantJoinTests.cs

@@ -25,9 +25,11 @@ namespace Darling.Tests;
 /// matched point carries the pool-summed measurement, a matched point measuring NOTHING granted is a
 /// genuine 0.0 (a snapshot existed — zero is a measurement, not a fabrication), and an unmatched point is
 /// null with the envelope's granted_note explaining the gap — a note that vanishes entirely when every
-/// point matched. The join is nearest-match within 30 seconds because each collector stamps its own
-/// DateTime.UtcNow per run: same-cycle rows sit seconds apart, so equality returns nothing, while a wider
-/// match would smear a slower grants cadence across points it never measured.
+/// point matched. Since #3960 the join is by BUCKET: both series are bucketed at the same width and a memory
+/// point takes the grant snapshots inside its bucket — their average as total_granted_mb and the largest as
+/// peak_granted_mb — where it used to take the nearest snapshot within 30 seconds. Each collector stamps its own
+/// DateTime.UtcNow per run, so same-cycle rows sit seconds apart and share a bucket, while a slower grants
+/// cadence still leaves the buckets it never measured null rather than smeared.
 ///
 /// <para>Gated on DARLING_TEST_PG like every other live class.</para>
 /// </summary>
@@ -59,12 +61,14 @@ public async Task JoinedPoints_CarryThePoolSum_AGenuineZero_ANullForTheUncovered
         {
             await DarlingMcpTestData.RegisterServerAsync(connection, ServerId, ServerName, ct);
 
-            /* ── fully covered: one memory point, one grants snapshot 3s later — value, and NO note ── */
-            var t0 = MinutesAgo(30);
+            /* ── fully covered: one memory point, one grants snapshot 3s later — value, and NO note ── Every
+               sample sits 10 seconds into a minute that starts a five-minute bucket, so each lands in its own
+               minute bucket and all four share one five-minute bucket, whatever the clock says now. */
+            var t0 = FiveMinuteFloor(MinutesAgo(30)).AddSeconds(10);
             await SeedMemoryAsync(connection, ct, t0);
             await SeedGrantAsync(connection, ct, t0.AddSeconds(3), poolId: 2, grantedMb: 50m);
 
-            var covered = JsonDocument.Parse(await DarlingMcpTrendTools.GetMemoryTrend(postgres, ServerName, 4)).RootElement;
+            var covered = JsonDocument.Parse(await DarlingMcpTrendTools.GetMemoryTrend(postgres, ServerName, 4, bucket_minutes: 1)).RootElement;
             Assert.Equal(50.0, covered.GetProperty("trend")[0].GetProperty("total_granted_mb").GetDouble(), precision: 6);
             Assert.False(covered.TryGetProperty("granted_note", out _),
                 "a window the grants series fully covers must not be captioned with a gap note");
@@ -83,17 +87,29 @@ public async Task JoinedPoints_CarryThePoolSum_AGenuineZero_ANullForTheUncovered
             await SeedGrantAsync(connection, ct, t2.AddSeconds(6), poolId: 2, grantedMb: 0m);
             /* nothing anywhere near t3 */
 
-            var root = JsonDocument.Parse(await DarlingMcpTrendTools.GetMemoryTrend(postgres, ServerName, 4)).RootElement;
+            var root = JsonDocument.Parse(await DarlingMcpTrendTools.GetMemoryTrend(postgres, ServerName, 4, bucket_minutes: 1)).RootElement;
             var trend = root.GetProperty("trend");
             Assert.Equal(4, trend.GetArrayLength());
             Assert.Equal(125.0, trend[1].GetProperty("total_granted_mb").GetDouble(), precision: 6);
+            Assert.Equal(125.0, trend[1].GetProperty("peak_granted_mb").GetDouble(), precision: 6);
             Assert.Equal(JsonValueKind.Number, trend[2].GetProperty("total_granted_mb").ValueKind);
             Assert.Equal(0.0, trend[2].GetProperty("total_granted_mb").GetDouble(), precision: 6);
             Assert.Equal(JsonValueKind.Null, trend[3].GetProperty("total_granted_mb").ValueKind);
+            Assert.Equal(JsonValueKind.Null, trend[3].GetProperty("peak_granted_mb").ValueKind);
 
             var note = root.GetProperty("granted_note").GetString()!;
             Assert.Contains("get_memory_grants", note, StringComparison.Ordinal);
-            Assert.Contains("30 seconds", note, StringComparison.Ordinal);
+            Assert.Contains("inside the bucket", note, StringComparison.Ordinal);
+
+            /* ── #3960: a wider bucket joins EVERY grant snapshot inside it — the per-snapshot pool sums 50, 125
+               and 0 average to 58.33 with 125 as the peak; the old nearest-snapshot join had no answer for a
+               point that summarizes several snapshots. The one bucket is covered, so no note. ── */
+            var wide = JsonDocument.Parse(await DarlingMcpTrendTools.GetMemoryTrend(postgres, ServerName, 4, bucket_minutes: 5)).RootElement;
+            var bucket = Assert.Single(wide.GetProperty("trend").EnumerateArray());
+            Assert.Equal(58.33, bucket.GetProperty("total_granted_mb").GetDouble(), precision: 6);
+            Assert.Equal(125.0, bucket.GetProperty("peak_granted_mb").GetDouble(), precision: 6);
+            Assert.Equal(40000.0, bucket.GetProperty("total_server_memory_mb").GetDouble(), precision: 6);
+            Assert.False(wide.TryGetProperty("granted_note", out _));
 
             bodySucceeded = true;
         }
@@ -107,6 +123,9 @@ await LiveStoreCleanup.RunAsync(cs!, bodySucceeded, async (cleanup, cleanupCt) =
     private static DateTime MinutesAgo(int minutes) =>
         DarlingMcpTestData.TruncateToSeconds(DateTime.UtcNow.AddMinutes(-minutes));
 
+    private static DateTime FiveMinuteFloor(DateTime t) =>
+        new(t.Ticks - (t.Ticks % TimeSpan.FromMinutes(5).Ticks), t.Kind);
+
     private static async Task SeedMemoryAsync(NpgsqlConnection connection, CancellationToken ct, DateTime t) =>
         await DarlingMcpTestData.ExecAsync(connection, ct, @"
 INSERT INTO memory_stats

Darling/Darling.Tests/PerfmonCounterTypeRungTests.cs

@@ -431,7 +431,11 @@ await WriteThroughTheCollectorAsync(connection, deltas, t2, new[]
             Assert.All(mcpGauge, p => Assert.Null(p.DeltaValue));
             Assert.All(mcpGauge, p => Assert.Null(p.SampleIntervalSeconds));
 
-            using (var trend = JsonDocument.Parse(await DarlingMcpTrendTools.GetPerfmonTrend(postgres, "Total Server Memory (KB)", ServerName)))
+            /* #3960: bucket_minutes: 1 pins per-collection granularity, since this test is about TYPE
+               classification, not about bucketing — t1/t3 are 10 minutes apart and the default 24h window's
+               auto-sized bucket (10 minutes) could otherwise fold them into one point depending on wall-clock
+               alignment against TrendBuckets.OriginSql. */
+            using (var trend = JsonDocument.Parse(await DarlingMcpTrendTools.GetPerfmonTrend(postgres, "Total Server Memory (KB)", ServerName, bucket_minutes: 1)))
             {
                 Assert.Equal("gauge", trend.RootElement.GetProperty("counter_kind").GetString());
                 Assert.Equal(PerfmonCounterTypes.PerfCounterLargeRawCount, trend.RootElement.GetProperty("cntr_type").GetInt32());
@@ -442,7 +446,7 @@ await WriteThroughTheCollectorAsync(connection, deltas, t2, new[]
                 Assert.Equal(8_000_000, points[1].GetProperty("value").GetInt64());
             }
 
-            using (var trend = JsonDocument.Parse(await DarlingMcpTrendTools.GetPerfmonTrend(postgres, "Batch Requests/sec", ServerName)))
+            using (var trend = JsonDocument.Parse(await DarlingMcpTrendTools.GetPerfmonTrend(postgres, "Batch Requests/sec", ServerName, bucket_minutes: 1)))
             {
                 Assert.Equal("rate", trend.RootElement.GetProperty("counter_kind").GetString());
                 var points = trend.RootElement.GetProperty("trend").EnumerateArray().ToList();
@@ -451,7 +455,7 @@ await WriteThroughTheCollectorAsync(connection, deltas, t2, new[]
                 Assert.Equal(300, points[1].GetProperty("sample_interval_seconds").GetInt64());
             }
 
-            using (var trend = JsonDocument.Parse(await DarlingMcpTrendTools.GetPerfmonTrend(postgres, "Lock waits", ServerName)))
+            using (var trend = JsonDocument.Parse(await DarlingMcpTrendTools.GetPerfmonTrend(postgres, "Lock waits", ServerName, bucket_minutes: 1)))
             {
                 Assert.Equal(JsonValueKind.Null, trend.RootElement.GetProperty("counter_kind").ValueKind);
             }

Darling/Darling.Tests/TrendBucketsTests.cs

@@ -132,6 +132,9 @@ public void AWidthOverTheCap_IsRefused_NamingTheNarrowestWidthThatFits()
         {
             TrendBuckets.FileIoMaxPoints, TrendBuckets.LockWaitMaxPoints, TrendBuckets.DurationMaxPoints,
             TrendBuckets.PgIoMaxPoints, TrendBuckets.PgDatabaseMaxPoints,
+            /* #3960 */
+            TrendBuckets.WaitMaxPoints, TrendBuckets.CpuMaxPoints, TrendBuckets.TempDbMaxPoints,
+            TrendBuckets.MemoryMaxPoints, TrendBuckets.PerfmonMaxPoints, TrendBuckets.PgQueryDurationMaxPoints,
         };
 
         foreach (var cap in caps)