diff --git a/Darling/Darling.Tests/DarlingMcpAgToolsTests.cs b/Darling/Darling.Tests/DarlingMcpAgToolsTests.cs index 683ad61692..71629b769a 100644 --- a/Darling/Darling.Tests/DarlingMcpAgToolsTests.cs +++ b/Darling/Darling.Tests/DarlingMcpAgToolsTests.cs @@ -113,6 +113,56 @@ public void Instructions_DocumentTheAgTool() Assert.Contains("get_ag_health", DarlingMcpInstructions.Text, StringComparison.Ordinal); Assert.Contains("Availability Group", DarlingMcpInstructions.Text, StringComparison.Ordinal); } + + /// + /// #3653 A15/A16: the description says the two things the reader's own doc comment warns about, because a + /// caller reads the description and never the reader. 's summary carries two + /// bolded traps — Latest collection per server (the collectors write NO row for a server with no AGs, + /// so a dropped AG lingers as its last non-empty snapshot and the group's collection_time is the only + /// tell) and What the badge deliberately does NOT band (every severity restates a DMV state or health + /// string; lag and queue depth are raw magnitudes, so a badly lagging asynchronous secondary carries a healthy + /// severity) — and the description used to say neither. #3696 put both sentences on it, plus the trap inside + /// the second (the DMV reports 0 lag while data movement is suspended, so lag is read with + /// is_suspended). Nothing pinned them, and a description that loses a sentence fails nothing: the + /// reader's comment goes on being right in a file no agent opens. + /// + /// Anchored on the reader's own paragraph headers first, so the pin fails with "re-anchor" if the + /// reader stops documenting a trap rather than passing against prose that no longer exists; then on the + /// description and the instructions paragraph — the two texts an agent reads before calling — for each + /// trap's mechanism, consequence and the field that tells it apart. Phrase matching rather than a + /// paraphrase test, because these are the exact words a reader would search for. + /// + [Fact] + public void Description_SurfacesTheReadersTwoDocumentedTraps() + { + var reader = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingAgReader.cs"); + Assert.Contains("Latest collection per server.", reader, StringComparison.Ordinal); + Assert.Contains("What the badge deliberately does NOT band: lag and queue depth.", reader, StringComparison.Ordinal); + + var description = ToolMethods() + .Single(m => m.GetCustomAttribute()!.Name == "get_ag_health") + .GetCustomAttribute()!.Description; + + /* Trap one, the stale snapshot: mechanism, consequence, tell. */ + Assert.Contains("NO row for a server with no AGs", description, StringComparison.Ordinal); + Assert.Contains("last non-empty snapshot", description, StringComparison.Ordinal); + Assert.Contains("collection_time", description, StringComparison.Ordinal); + + /* Trap two, the un-banded magnitudes: the negative on the word a caller would look for, the fields to + read instead, and the DMV's 0-while-suspended inside it. */ + Assert.Contains("lag and queue depth are NOT banded", description, StringComparison.Ordinal); + Assert.Contains("secondary_lag_seconds", description, StringComparison.Ordinal); + Assert.Contains("is_suspended", description, StringComparison.Ordinal); + Assert.Contains("0 lag while data movement is suspended", description, StringComparison.Ordinal); + + /* The instructions' AG paragraph is the other text an agent reads first; it carries the same two traps + in its own words, and the perspective trap (LOCAL-replica-only columns) it always had. */ + var instructions = DarlingMcpInstructions.Text; + Assert.Contains("lag and queue depth are NOT banded", instructions, StringComparison.Ordinal); + Assert.Contains("the DMV reports lag as 0 (not null) while data movement is suspended", instructions, StringComparison.Ordinal); + Assert.Contains("keeps returning its last non-empty snapshot", instructions, StringComparison.Ordinal); + Assert.Contains("populated only for the LOCAL replica", instructions, StringComparison.Ordinal); + } } /// diff --git a/Darling/Darling.Tests/DarlingMcpTrendToolsTests.cs b/Darling/Darling.Tests/DarlingMcpTrendToolsTests.cs index 3ea6fec2f2..4758c3e5aa 100644 --- a/Darling/Darling.Tests/DarlingMcpTrendToolsTests.cs +++ b/Darling/Darling.Tests/DarlingMcpTrendToolsTests.cs @@ -18,6 +18,7 @@ using Npgsql; using PerformanceMonitor.Collectors; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Service; using PerformanceMonitor.Darling.Service.Mcp; using PerformanceMonitor.Darling.Storage; using PerformanceMonitor.Darling.Viewer; @@ -117,6 +118,47 @@ public void MemoryTrend_Description_PromisesTheJoinedGrantSeries_AndNamesTheNull Assert.Contains("get_memory_grants", description, StringComparison.Ordinal); } + /// + /// #3653 A15/A16: the web catalogue's line for get_query_duration_trend said "percentiles". The payload has + /// never carried one — SerializeTrend, the helper the duration trio shares, publishes + /// elapsed_ms_per_second and executions_per_second per point, a rate over the collection's + /// stored interval on the raw route and over the bucket width on the hourly one — and the tool's own + /// description has said so since #3541 A12. The catalogue () + /// is what the web client's view composer and the reads listing show a human choosing a read, so it was + /// the one place a wrong word about this read was still standing. #3696 corrected the line; this pins the + /// correction against BOTH ends so they cannot drift apart again: the line names the two rates the + /// serializer publishes, the serializer still publishes exactly those, and across the whole catalogue the + /// word "percentile" appears only negated — no read here computes one. A read that genuinely returns a + /// percentile (none does today) would add itself beside this sentence rather than fall through it. + /// + [Fact] + public void QueryDurationTrend_CatalogueLine_NamesTheRatesThePayloadCarries_NotPercentiles() + { + var line = DarlingWebEndpoints.CatalogDescriptors["get_query_duration_trend"].Description; + Assert.Contains("ms/sec", line, StringComparison.Ordinal); + Assert.Contains("executions/sec", line, StringComparison.Ordinal); + Assert.Contains("not percentiles", line, StringComparison.Ordinal); + + /* The payload side: the shared serializer's point projection, and no percentile of anything in it. */ + var source = Lf(RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingMcpTrendTools.cs")); + var at = source.IndexOf("private static string SerializeTrend(", StringComparison.Ordinal); + Assert.True(at > 0, "SerializeTrend moved — re-anchor this pin"); + var end = source.IndexOf("\n }\n", at, StringComparison.Ordinal); + var body = source[at..end]; + Assert.Contains("elapsed_ms_per_second = p.Value", body, StringComparison.Ordinal); + Assert.Contains("executions_per_second = p.ExecutionsPerSecond", body, StringComparison.Ordinal); + Assert.DoesNotContain("percentile", body, StringComparison.OrdinalIgnoreCase); + + /* And the whole catalogue: "percentile" only ever as a negation. */ + var claims = DarlingWebEndpoints.CatalogDescriptors + .Where(kv => System.Text.RegularExpressions.Regex.IsMatch(kv.Value.Description, @"(? kv.Key) + .OrderBy(k => k, StringComparer.Ordinal) + .ToArray(); + Assert.True(claims.Length == 0, + "catalogue line(s) claim a percentile no read computes: " + string.Join(", ", claims)); + } + [Fact] public void MemoryTrendSql_WindowedBothSides_CastsNumericToDouble() { diff --git a/Darling/Darling.Tests/ServerPageTabsTests.cs b/Darling/Darling.Tests/ServerPageTabsTests.cs index cab1466fa7..2c2f8395fd 100644 --- a/Darling/Darling.Tests/ServerPageTabsTests.cs +++ b/Darling/Darling.Tests/ServerPageTabsTests.cs @@ -440,6 +440,73 @@ public void TheQueryTrendDrillDown_ReadsTheWindowFloorOffItsOwnKey() Assert.DoesNotContain("trend.data.truncated", body, StringComparison.Ordinal); } + /// + /// The latest-snapshot reads are read by their stamp's one spelling. #3637 made captured_at the + /// census's word for "when this snapshot was taken" and holds + /// every latest read to it; four of them (get_database_sizes, get_running_jobs, + /// get_server_properties, get_session_stats) had stamped themselves as collection_time + /// before that vocabulary existed, and this page's Sessions tile read the old key — the one consumer outside + /// that census's boundary, and the reason the four sat in a named allowance instead of the roster (#3653 + /// A15/A16). #3697 renamed the payloads on both SKUs and moved the tile. Nothing pinned the tile, and a key + /// drifting back fails silently: an absent key renders as the empty dash, the tile still draws, and the + /// snapshot's age — the one number that says whether a Stamped read is stale — quietly stops appearing. + /// + /// Derived, not listed. Every read the stamp census rosters as Stamped or SearchBound publishes its + /// stamp ONCE at the top as captured_at and nothing under collection_time anywhere in its + /// payload (the census's negative sweep refuses the top-level key, and none of those tools projects the word + /// into a row either), so ANY descriptor this page binds to one of those reads that names + /// collection_time is a dead read — whether a stat tile over the top-level object or a table column + /// over its rows. Windowed reads are excluded on purpose: their grants[] rows carry a per-row + /// collection_time that IS a series column. The positive half is the one tile the item named: it must + /// show the stamp under the new word, so the pin cannot pass by the tile being deleted. + /// + [Fact] + public void TheStampedReads_AreReadByTheirStamp_NeverTheRetiredKey() + { + var js = ServerTabsJs; + + var stamped = McpLatestSnapshotStampTests.LatestTools + .Where(t => t.Shape is McpLatestSnapshotStampTests.Shape.Stamped or McpLatestSnapshotStampTests.Shape.SearchBound) + .Select(t => t.ToolName) + .ToHashSet(StringComparer.Ordinal); + Assert.True(stamped.IsSupersetOf(["get_database_sizes", "get_running_jobs", "get_server_properties", "get_session_stats"]), + "the four reads this pin was written for have left the stamp roster — re-anchor before editing"); + + var arrays = DescriptorArraysIn(js); + var problems = new List(); + var examined = new List(); + + foreach (var (read, arrayNames) in DescriptorArraysBoundIn(js)) + { + if (!stamped.Contains(read)) continue; + + foreach (var name in arrayNames) + { + Assert.True(arrays.TryGetValue(name, out var descriptors), + $"{name}, bound to {read}, is not a top-level const array in server-tabs.js — the parser below needs re-anchoring"); + examined.Add($"{read}/{name}"); + + if (Regex.IsMatch(descriptors, @"key:\s*""collection_time""")) + { + problems.Add($"{read} is read through {name}, which names collection_time — a key no Stamped read publishes; its stamp is captured_at"); + } + } + } + + /* The four named reads must all have been reached, or the parser has silently stopped seeing the panels + and every assertion above passed on nothing. */ + foreach (var read in new[] { "get_database_sizes", "get_running_jobs", "get_server_properties", "get_session_stats" }) + { + Assert.Contains(examined, e => e.StartsWith(read + "/", StringComparison.Ordinal)); + } + + Assert.True(problems.Count == 0, string.Join("; ", problems)); + + /* The positive half: the Sessions tile shows the stamp, under the word the census uses, as an age. */ + Assert.True(arrays.TryGetValue("SESSION_STATS", out var sessionStats), "SESSION_STATS moved — re-anchor this pin"); + Assert.Matches(@"\{ key: ""captured_at"", label: ""Collected"", format: ""reltime""", sessionStats); + } + /// /// Every viz a descriptor names is in the shipped vocabulary. The four kinds are the whole registry; a fifth /// would have to be added to panels.js's VIZ, to KnownVizList (or the composer could not offer it), to @@ -1096,4 +1163,50 @@ private static HashSet ReadNamesIn(string js) => yield return (m.Groups[1].Value, keys); } } + + /// Every top-level const NAME = [ ... ]; descriptor array in the module, by name. These are + /// the column and stat vocabularies the panels bind to (*_COLUMNS, *_STATS, *_SERIES); + /// the body is the text between the brackets, comments included. + private static Dictionary DescriptorArraysIn(string js) => + Regex.Matches(js, @"^const ([A-Z][A-Z0-9_]+) = \[(.*?)^\];", RegexOptions.Multiline | RegexOptions.Singleline) + .ToDictionary(m => m.Groups[1].Value, m => m.Groups[2].Value, StringComparer.Ordinal); + + /// + /// The (read, descriptor-array names) pairs the module's panel helpers bind: stat("T", "read", {...}, + /// NAME, ...), table("T", "read", {...}, "rows", NAME, ...) and fanout("read", {...}, [{ stats: + /// NAME }, { columns: NAME }]). Each call is sliced by bracket balance rather than by regex because a + /// fanout's spec array nests objects inside an array inside the call; the read is the first read-shaped + /// literal in the call, and the arrays are every *_COLUMNS / *_STATS token in it. The + /// helpers' own definitions (function stat(title, read, ...)) carry no literal and are skipped. + /// String literals are NOT skipped: a bracket inside a title or an empty-state sentence is balanced in + /// practice, and a lone one would end the slice early — a silent miss on the arrays after it, never a false + /// alarm, which the caller's reach check on its named reads bounds. + /// + private static IEnumerable<(string Read, string[] Arrays)> DescriptorArraysBoundIn(string js) + { + foreach (Match open in Regex.Matches(js, @"(? 0 && i < js.Length) + { + switch (js[i]) + { + case '(' or '[' or '{': depth++; break; + case ')' or ']' or '}': depth--; break; + } + i++; + } + + var call = js[open.Index..i]; + var read = Regex.Match(call, "\"(get_[a-z0-9_]+|audit_config)\""); + if (!read.Success) continue; + + var arrays = Regex.Matches(call, @"\b([A-Z][A-Z0-9_]*_(?:COLUMNS|STATS))\b") + .Select(m => m.Groups[1].Value) + .Distinct(StringComparer.Ordinal) + .ToArray(); + yield return (read.Groups[1].Value, arrays); + } + } }