From 6435c010681772587dd7cf77439c3b0961afdb99 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 19 Sep 2026 03:47:27 -0400 Subject: [PATCH 1/2] One payload said NoData and No Data, eight spellings of four severity words were two bands and six other vocabularies, and the page cut had five names (partial #3653 A15/A16) get_daily_summary / get_daily_summary_range on both SKUs publish overall_health and health_band from the same enum token (NoData, not "No Data"); the four PostgreSQL tools that inferred limit_reached from a full page, the autovacuum run history's history_capped and get_analysis_findings' truncation_note (both SKUs) fetch one past their cap through McpHelpers.BoundPage and publish truncated (+ *_returned); the census classifies the fourteen cut keys and the eight severity spellings by what each actually is and fails a retired or unclassified spelling by name. McpHelpers.ValidateDaysBack adopts the five inline days_back refusals and list_servers goes through FormatError; FormatError's shape is untouched (Q11). --- .../DarlingPgAutovacuumVerdictLiveTests.cs | 14 +- .../McpFilterSemanticsLivePostgresTests.cs | 5 +- Darling/Darling.Tests/McpPageContractTests.cs | 20 +- .../McpPayloadContractCensusTests.cs | 508 ++++++++++++++---- .../Darling.Tests/PgLogEventMetricsTests.cs | 18 +- .../Mcp/DarlingMcpCollectorCostTools.cs | 7 +- .../Mcp/DarlingMcpDataTools.cs | 5 +- .../Mcp/DarlingMcpHealthTools.cs | 24 +- .../Mcp/DarlingMcpPgAutovacuumTools.cs | 43 +- .../Mcp/DarlingMcpPgIndexUsageTools.cs | 22 +- .../Mcp/DarlingMcpPgSessionStatesTools.cs | 27 +- .../Mcp/DarlingMcpPgTableBloatTools.cs | 22 +- .../Mcp/DarlingMcpStallProbeTools.cs | 7 +- .../Mcp/DarlingMcpStoreMetricsTools.cs | 7 +- .../Mcp/DarlingMcpTools.cs | 23 +- .../wwwroot/js/pages/server-tabs.js | 14 +- .../wwwroot/js/util.js | 16 +- Lite.Tests/McpPageContractTests.cs | 3 +- Lite/Mcp/McpAnalysisTools.cs | 23 +- Lite/Mcp/McpHealthTools.cs | 23 +- PerformanceMonitor.Common/Mcp/McpHelpers.cs | 21 + 21 files changed, 649 insertions(+), 203 deletions(-) diff --git a/Darling/Darling.Tests/DarlingPgAutovacuumVerdictLiveTests.cs b/Darling/Darling.Tests/DarlingPgAutovacuumVerdictLiveTests.cs index cf53901322..aa301fad33 100644 --- a/Darling/Darling.Tests/DarlingPgAutovacuumVerdictLiveTests.cs +++ b/Darling/Darling.Tests/DarlingPgAutovacuumVerdictLiveTests.cs @@ -123,12 +123,18 @@ so the figure the verdict came from is visible. */ undersized call discloses its page scope (the get_pg_database_stats pattern). */ Assert.Equal(3, payload.GetProperty("past_threshold_count").GetInt32()); Assert.Equal(1, payload.GetProperty("growing_count").GetInt32()); - Assert.False(payload.GetProperty("limit_reached").GetBoolean()); - + Assert.False(payload.GetProperty("truncated").GetBoolean()); + + /* #3653: the cut is OBSERVED off a limit + 1 fetch (#3594), so a limit of exactly the population + is complete and one below it is truncated with the page under its *_returned name. */ + var whole = JsonDocument.Parse( + await DarlingMcpPgAutovacuumTools.GetPgAutovacuumHealth(dataSource, ServerName, 4, tables.Length)).RootElement; + Assert.False(whole.GetProperty("truncated").GetBoolean()); + Assert.Equal(tables.Length, whole.GetProperty("tables_returned").GetInt32()); var truncated = JsonDocument.Parse( await DarlingMcpPgAutovacuumTools.GetPgAutovacuumHealth(dataSource, ServerName, 4, 2)).RootElement; - Assert.Equal(2, truncated.GetProperty("table_count").GetInt32()); - Assert.True(truncated.GetProperty("limit_reached").GetBoolean()); + Assert.Equal(2, truncated.GetProperty("tables_returned").GetInt32()); + Assert.True(truncated.GetProperty("truncated").GetBoolean()); bodySucceeded = true; } diff --git a/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs b/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs index 6a1f17614f..bbc85ac591 100644 --- a/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs +++ b/Darling/Darling.Tests/McpFilterSemanticsLivePostgresTests.cs @@ -253,7 +253,8 @@ NOT Healthy. */ Assert.Equal(1, ghost.GetProperty("collection_runs").GetInt64()); Assert.Equal("purged", ghost.GetProperty("data_state").GetString()); Assert.Equal("NoData", ghost.GetProperty("health_band").GetString()); - Assert.Equal("No Data", ghost.GetProperty("overall_health").GetString()); + /* #3653: one band, one spelling \u2014 overall_health carries the same token health_band does. */ + Assert.Equal("NoData", ghost.GetProperty("overall_health").GetString()); Assert.Contains("PURGED", ghost.GetProperty("data_note").GetString(), StringComparison.Ordinal); Assert.Contains(expectedHorizon.ToString("yyyy-MM-dd"), ghost.GetProperty("data_note").GetString(), StringComparison.Ordinal); @@ -296,7 +297,7 @@ await DarlingMcpTestData.ExecAsync(connection, ct, /* The single-day read of a past-horizon day is a DATA payload (the rows are there), not unavailable. */ var singlePast = JsonDocument.Parse(await DarlingMcpHealthTools.GetDailySummary(postgres, ServerName, ghostDay.ToString("yyyy-MM-dd"))).RootElement; Assert.Equal("past_horizon", singlePast.GetProperty("data_state").GetString()); - Assert.Equal("No Data", singlePast.GetProperty("overall_health").GetString()); + Assert.Equal("NoData", singlePast.GetProperty("overall_health").GetString()); await DarlingMcpTestData.ExecAsync(connection, ct, $"DELETE FROM deadlocks WHERE server_id = {ServerId}"); /* The horizon is the store's EFFECTIVE retention: a fleet-wide override shortening one signal diff --git a/Darling/Darling.Tests/McpPageContractTests.cs b/Darling/Darling.Tests/McpPageContractTests.cs index cfcf4d5baa..a1143ce294 100644 --- a/Darling/Darling.Tests/McpPageContractTests.cs +++ b/Darling/Darling.Tests/McpPageContractTests.cs @@ -799,10 +799,12 @@ public void PgServerConfig_NamesItsDenominators_InItsDescription() /// the way sweeps Math.Abs, with /// the same population floor so a marker that stops matching cannot pass for free. /// - /// What this does NOT sweep: limit_reached = x.Count >= limit. That key says the page is as - /// long as the cap — true by construction — and its prose says "more MAY exist"; it is a weaker dialect, - /// not an inference, and whether it collapses into an observed truncated is the vocabulary lane's - /// call. McpPayloadContractCensusTests holds it in the truncation-key inventory. + /// What this does NOT sweep: limit_reached = x.Count >= limit. That key said the page was as + /// long as the cap — true by construction — and its prose said "more MAY exist"; a weaker dialect, not an + /// inference, and the discriminator witness below keeps the non-match on record. The vocabulary lane + /// (#3653) then collapsed it: the four tools that spelled it moved onto BoundPage (they are in + /// ) and McpPayloadContractCensusTests.RetiredCutSpellings + /// fails its return by name. /// [Fact] public void NoTool_InfersTruncationFromItsCap_OnEitherSku() @@ -848,6 +850,12 @@ public void NoTool_InfersTruncationFromItsCap_OnEitherSku() /// receive the limit + 1 fetch and trim it; DarlingMcpPgPlanToolsTests executes both /// boundaries against the builders, and /// executes the deadlock and index reads against live Postgres. + /// + /// #3653 (the vocabulary lane) added the four tools that had spelled the cut limit_reached = + /// x.Count >= limit — the inference this file's sweep deliberately did not flag because the key said + /// "as long as the cap", not "more exist" — now on the same helper with truncated and a + /// *_returned page count. McpPayloadContractCensusTests holds the vocabulary half (the + /// retired spelling cannot return); this roster holds the page contract. /// public static readonly (Type Tools, string ToolName, string? Builder, string ReadName, string Sql)[] PgPagedToolsThroughTheHelper = [ @@ -855,6 +863,10 @@ public static readonly (Type Tools, string ToolName, string? Builder, string Rea (typeof(DarlingMcpPgPlanTools), "get_pg_plans", "BuildPlansJson", nameof(DarlingPgPlanCaptureReader.PgPlanCaptureSql), DarlingPgPlanCaptureReader.PgPlanCaptureSql), (typeof(DarlingMcpPgPlanTools), "get_pg_plan_capture_readiness", "BuildReadinessJson", nameof(DarlingPgPlanCaptureReadinessReader.PgPlanCaptureReadinessSql), DarlingPgPlanCaptureReadinessReader.PgPlanCaptureReadinessSql), (typeof(DarlingMcpPgIndexTools), "get_pg_index_bloat", null, nameof(DarlingPgIndexBloatReader.PgIndexBloatSql), DarlingPgIndexBloatReader.PgIndexBloatSql), + (typeof(DarlingMcpPgAutovacuumTools), "get_pg_autovacuum_health", null, nameof(DarlingPgAutovacuumReader.PgAutovacuumSql), DarlingPgAutovacuumReader.PgAutovacuumSql), + (typeof(DarlingMcpPgSessionStatesTools), "get_pg_session_states", null, nameof(DarlingPgSessionStatesReader.PgSessionStatesSql), DarlingPgSessionStatesReader.PgSessionStatesSql), + (typeof(DarlingMcpPgIndexUsageTools), "get_pg_index_usage", null, nameof(DarlingPgIndexUsageReader.PgIndexUsageSql), DarlingPgIndexUsageReader.PgIndexUsageSql), + (typeof(DarlingMcpPgTableBloatTools), "get_pg_table_bloat", null, nameof(DarlingPgTableBloatReader.PgTableBloatSql), DarlingPgTableBloatReader.PgTableBloatSql), ]; [Fact] diff --git a/Darling/Darling.Tests/McpPayloadContractCensusTests.cs b/Darling/Darling.Tests/McpPayloadContractCensusTests.cs index 858bfbe4b6..cd70bb670c 100644 --- a/Darling/Darling.Tests/McpPayloadContractCensusTests.cs +++ b/Darling/Darling.Tests/McpPayloadContractCensusTests.cs @@ -14,6 +14,7 @@ using System.Text.Json; using System.Text.RegularExpressions; using PerformanceMonitor.Common; +using PerformanceMonitor.Darling.Service.Mcp; using Xunit; namespace Darling.Tests; @@ -31,22 +32,33 @@ namespace Darling.Tests; /// /// /// errors one shape — census-able as "every catch returns through a SHARED shape, never an -/// ad-hoc one", and pinned that way; but the shared shapes are TWO (McpHelpers.FormatError is a bare -/// string, McpHelpers.Status("error", …) is a JSON envelope), so the rule itself is NOT met today and -/// the split is pinned as an inventory for the vocabulary lane to collapse. +/// ad-hoc one", and pinned that way (the one ad-hoc site, list_servers, went through FormatError +/// in #3653's vocabulary lane, so the ad-hoc roster is empty); but the shared shapes are TWO +/// (McpHelpers.FormatError is a bare string, McpHelpers.Status("error", …) is a JSON envelope), +/// so the rule itself is NOT met today. Collapsing them is one helper change with a fleet-wide wire effect +/// (~212 tools' failures become an envelope) and is held for the maintainer's ruling (#3653 Q11); the split +/// stays pinned as a file-grain inventory until then. /// refuse what you cannot honor — census-able: every bounded parameter reaches a shared validator -/// (or one of four rostered inline refusals), and no parameter is clamped against a ceiling. The -/// Math.Abs arm is McpPageContractTests.NoTool_ReadsAParameterAsItsAbsoluteValue_OnEitherSku -/// and is not repeated here. +/// (McpHelpers.ValidateDaysBack now covers the day-grained span, so the inline-refusal roster is +/// empty), and no parameter is clamped against a ceiling. The Math.Abs arm is +/// McpPageContractTests.NoTool_ReadsAParameterAsItsAbsoluteValue_OnEitherSku and is not repeated here. /// name = truth — NOT mechanically pinnable in general; two greps approximate it and are pinned /// as inventories: every total_* assigned from a .Count names a WHOLE set (rostered, each with /// the set it counts), and every reader anchored on MAX(collection_time) projects the anchor so a /// stamp is possible (two rostered exceptions). The rest of the rule — does the key say what the number /// is — is a review-checklist item, and this file says so rather than pretending a regex reads meaning. -/// one vocabulary — NOT collapsed here (that is the vocabulary lane's, after the other MCP lanes -/// land); the INVENTORY is pinned instead: every truncation-key dialect, every severity-word spelling, and -/// every statement-terminal literal LIMIT, each as an exact roster that fails when a dialect appears -/// or disappears without the roster moving. The lane that collapses them shrinks the rosters. +/// one vocabulary — collapsed and CLASSIFIED (#3653 A15/A16, the vocabulary lane): the page cut +/// has one spelling (truncated, observed) and the keys that are not page cuts are rostered by class +/// with the reason each keeps its name (a second bound, a source-side cut, the withheld summary, a homonym); +/// the band canon is one spelling (PascalCase) and every other severity-word literal is rostered with the +/// vocabulary it actually belongs to; and every statement-terminal literal LIMIT is inventoried. Each +/// roster is an equality that fails when a spelling appears or disappears without the roster moving. +/// Residue, stated rather than hidden: truncated carries two facts — the page cut, and on the #2364 +/// trend family and get_query_store_top the WINDOW floor (the store's retention did not reach the whole +/// window) — and separating them is a rename across a file another lane holds; the PostgreSQL severity-token +/// ladder (ok / info_* / warning[_*] / critical_*) is a different vocabulary from the band, and folding +/// it in is a reshape (band + reason) for the maintainer; shown survives beside an honest total because +/// four PgTarget* tests read it and that fence stands. /// /// /// Every fact states the bound it rests on, in the idiom the sibling censuses use: the population it @@ -112,14 +124,12 @@ public sealed class McpPayloadContractCensusTests /* ───────────────────────── rule: errors one shape ───────────────────────── */ /// - /// The one catch in a tool method whose return is neither shared shape. list_servers returns an - /// interpolated sentence of its own. Shrink-only: the file is another lane's tonight, and the fix is one - /// line through either shared helper. + /// The catches in a tool method whose return is neither shared shape. Empty: the one there was + /// (list_servers returned an interpolated sentence of its own) went through McpHelpers.FormatError + /// in #3653's vocabulary lane. Kept as a roster rather than a bare "zero" so a new ad-hoc return fails + /// by name and a deliberate exception has somewhere to state its reason. /// - public static readonly (string File, string Tool)[] AdHocErrorReturns = - [ - ("DarlingMcpDataTools.cs", "list_servers"), - ]; + public static readonly (string File, string Tool)[] AdHocErrorReturns = []; /// /// The files whose tool catches return the JSON envelope (McpHelpers.Status("error", …)) rather @@ -297,23 +307,15 @@ private static IEnumerable CatchReturns(string span) /// /// The tools that refuse a bad days_back INLINE (if (days_back <= 0 || days_back > Max…) - /// return "Invalid days_back …") rather than through a McpHelpers validator — because none - /// exists for a day-grained span. They refuse; they do not clamp. Rostered so the rule "every bounded - /// parameter reaches the shared vocabulary" is honest about its five exceptions, and so a - /// ValidateDaysBack that lands later shrinks this to nothing. Three of the five refuse with a bare - /// sentence rather than an envelope — the errors-one-shape split, one layer down. + /// return "Invalid days_back …") rather than through a McpHelpers validator. Empty since #3653's + /// vocabulary lane added McpHelpers.ValidateDaysBack(daysBack, maxDaysBack) and moved the five sites + /// onto it — each keeping its own ceiling (60 / 90 / 366 / 400 days: the retention of the series it reads). + /// Kept as a roster so a new inline refusal fails by name. /// - public static readonly (string File, string Tool)[] InlineDaysBackRefusals = - [ - ("DarlingMcpCollectorCostTools.cs", "get_collector_cost"), - ("DarlingMcpHealthTools.cs", "get_daily_summary_range"), - ("DarlingMcpStallProbeTools.cs", "get_collector_stall_probes"), - ("DarlingMcpStoreMetricsTools.cs", "get_store_metrics"), - ("McpHealthTools.cs", "get_daily_summary_range"), - ]; + public static readonly (string File, string Tool)[] InlineDaysBackRefusals = []; private static readonly Regex SharedValidatorCall = new( - @"\bMcpHelpers\.(ValidateWindow|ValidateUncappedWindow|ValidateHoursBack|ValidateTop|ResolveAsOf|ParseSummaryDate)\(", + @"\bMcpHelpers\.(ValidateWindow|ValidateUncappedWindow|ValidateHoursBack|ValidateDaysBack|ValidateTop|ResolveAsOf|ParseSummaryDate)\(", RegexOptions.Compiled); private static readonly Regex InlineDaysBackRefusal = new( @@ -322,7 +324,7 @@ public static readonly (string File, string Tool)[] InlineDaysBackRefusals = /// /// Every tool method that declares a bounded parameter validates it through the shared validators — /// in its own span, or in a same-file private helper the span calls (the nine health-parser tools share - /// one CollectAsync) — or is one of the four inline days_back refusals, exactly. + /// one CollectAsync) — or is a rostered inline days_back refusal, exactly (none today). /// /// Bound: parameters are read off the method signature (the text before the first member-indent /// brace), matched by declared name; a helper counts when the span names it and its own body carries a @@ -374,6 +376,23 @@ resolves as_of through the shared parser — an early continue would hide the in + "], rostered [" + string.Join(", ", InlineDaysBackRefusals.Select(t => $"{t.File} {t.Tool}")) + "]"); } + /// The day-grained validator, executed at its boundaries: it refuses zero, negatives and one past + /// the ceiling it is handed, accepts the ceiling itself, and speaks the first sentence of + /// ValidateHoursBack's refusal — the sentence the two range-tool tests + /// (DarlingDailySummaryRangeTests, Lite's DailySummaryRangeToolTests) pin on the wire. + [Fact] + public void ValidateDaysBack_RefusesOutsideTheCeilingItIsHanded_InTheSharedSentence() + { + Assert.Null(McpHelpers.ValidateDaysBack(1, 60)); + Assert.Null(McpHelpers.ValidateDaysBack(60, 60)); + Assert.Equal("Invalid days_back value '0'. Must be a positive integer (1-60).", McpHelpers.ValidateDaysBack(0, 60)); + Assert.Equal("Invalid days_back value '-7'. Must be a positive integer (1-60).", McpHelpers.ValidateDaysBack(-7, 60)); + Assert.Equal("Invalid days_back value '61'. Must be a positive integer (1-60).", McpHelpers.ValidateDaysBack(61, 60)); + Assert.Equal("Invalid days_back value '367'. Must be a positive integer (1-366).", McpHelpers.ValidateDaysBack(367, McpHelpers.MaxDailySummaryDaysBack)); + /* The sentence shape is ValidateHoursBack's first sentence, word for word but for the parameter. */ + Assert.StartsWith("Invalid hours_back value '0'. Must be a positive integer (1-", McpHelpers.ValidateHoursBack(0), StringComparison.Ordinal); + } + /// A same-file private static member the span calls whose own body carries a validator call. private static bool CallsAValidatingHelper(string toolCode, string source) { @@ -677,52 +696,126 @@ public void TheSharedTimestampParsers_RefuseWhatTheGeneralParserAccepts() "the control: the general parser under invariant UTC styles accepts the shape the allowlist refuses — that acceptance is what the roster above inventories"); } - /* ───────────────────────── rule: one vocabulary — the inventories ───────────────────────── */ + /* ───────────────────────── rule: one vocabulary — the classification ───────────────────────── */ /// - /// Every payload key on either SKU that speaks about a cut, by name. Fourteen spellings. Some are - /// different FACTS and not dialects of one — query_text_may_be_truncated is about text cut at - /// collection, capture_was_truncated about the collector's per-capture cap, scan_truncated - /// about a fingerprint scan ceiling, *_withheld about a reach verdict — and the vocabulary lane - /// decides which are which. The page-cut dialects proper are truncated (the #3594 spelling), - /// limit_reached (five PostgreSQL tools; true by construction under >= limit, so weaker - /// than an observation), is_partial, partial_count, history_capped, and the - /// shown-beside-a-total pair that carries no flag at all. Pinned at KEY grain: another tool joining - /// an existing dialect changes nothing here; a fifteenth spelling, or the collapse of one, moves the roster. - /// Swept over whole tool sources rather than tool-method spans, because five of the fourteen are emitted - /// from wire-shape builders below the tools (get_pg_blocking's chain keys, the autovacuum tool's - /// history_capped — itself a runs.Count >= RunsReadPerTable against a collector constant, - /// the limit_reached shape one layer down — and the logging audit's partial_count). + /// The cut vocabulary, classified (#3653 A15/A16). #3699 inventoried fourteen payload keys that speak + /// about a cut and left the question of which were dialects of one fact and which were different facts to + /// this lane. Read from the source, they are five things, and the rosters below say which is which so the + /// census can assert the CLASSIFICATION rather than a flat list: + /// + /// + /// The page cut — the tool returned fewer rows than exist because of the caller's cap (or a + /// read cap of its own). ONE spelling: truncated, OBSERVED off a cap + 1 fetch + /// (McpHelpers.BoundPage), beside the page's count under a *_returned name (#3594). Four + /// PostgreSQL tools spelled it limit_reached = x.Count >= limit (an inference, not an + /// observation — a population of exactly limit read as cut) and the autovacuum tool's run history + /// spelled it history_capped = runs.Count >= RunsReadPerTable, the same inference against a + /// collector constant; get_analysis_findings on both SKUs inferred its truncation_note from + /// >= WindowCoveringLimit. All six now fetch one past the cap and publish truncated; the + /// two retired spellings are named in so their return is a specific + /// failure, not a generic one. + /// A second bound in the same payload — : get_blocking / + /// get_deadlocks under a dedup_key scan the window up to a stated ceiling BEFORE the page is + /// cut, and the two cuts are two facts a caller acts on differently (raise limit, or narrow the + /// window). The second is spelled <bound>_truncated, observed the same way. + /// The prose beside the flag — : truncation_note is the + /// *_note idiom every disclosure on the surface uses, null when nothing was cut. Three tools carry + /// it; on get_query_store_top the truncated it explains is the #2364 WINDOW floor (the store's + /// raw retention did not reach the whole window) — the same key, a different fact, and the residue this + /// classification leaves (see the class summary). + /// Source-side cuts — : the capture, the chain or the text was + /// cut BEFORE the store, by the collector. They keep their names because they are true and different: a + /// caller can do nothing about them by re-paging, and folding them into truncated would tell that + /// caller to raise a limit that changes nothing. + /// The withheld summary — : #3594's own vocabulary for a + /// reach verdict that withholds a figure rather than publishing a page's count under a whole's name. + /// + /// + /// Two more keys the inventory's regex caught are NOT about a cut at all and are excluded by name + /// with the reason (): is_partial is a PARTIAL INDEX (CREATE INDEX … + /// WHERE), partial_count counts logging-audit facets whose verdict is partial. And one + /// page count survives under a neutral noun (): shown + /// beside an honest whole total_* on the health-parser, default-trace and analysis-facts tools — + /// the cut is exact (total − shown) and disclosed by the pair, and its rename to *_returned + + /// truncated is fenced tonight: four PgTarget* test files read shown off + /// get_analysis_facts, and that fence is a standing decision, not this lane's to cross. + /// + /// Every roster is an EQUALITY on both the key and the files that emit it. A fifteenth spelling + /// fails with the class it should have joined; a rostered key that leaves a file fails until the roster + /// follows it. Swept over whole tool sources, because several are emitted from wire-shape builders below + /// the last tool. /// - public static readonly string[] TruncationKeyDialects = + public static readonly (string Key, string[] Files, string WhatWasCut)[] SecondBoundCutKeys = + [ + ("scan_truncated", ["DarlingMcpBlockingTools.cs"], + "the dedup_key fingerprint scan's ceiling (FingerprintScanCeiling), observed off a ceiling + 1 fetch, beside the page's own truncated — two bounds in one payload, the second spelled _truncated"), + ]; + + public static readonly (string Key, string[] Files, string WhatItExplains)[] CutNoteKeys = + [ + ("truncation_note", ["DarlingMcpDataTools.cs", "DarlingMcpTools.cs", "McpAnalysisTools.cs"], + "the prose beside truncated: on get_analysis_findings (both SKUs) the WindowCoveringLimit read cap, observed off a cap + 1 fetch; on get_query_store_top the #2364 window floor — the store's raw retention did not reach the whole requested window"), + ]; + + public static readonly (string Key, string[] Files, string WhatWasCut)[] SourceSideCutKeys = + [ + ("capture_was_truncated", ["DarlingMcpPgSessionStatesTools.cs"], + "the pg_session_states collector's per-capture row cap bit at COLLECTION: the stored rows for that capture are a worst-first sample of the instance's sessions"), + ("chain_may_be_truncated", ["DarlingMcpPgBlockingTools.cs"], + "the pg_blocking collector's chain-walk depth cap at COLLECTION: the blocking chain may continue past the last edge stored"), + ("chain_truncation_note", ["DarlingMcpPgBlockingTools.cs"], + "the prose beside chain_may_be_truncated — what the depth cap is and why the root shown may not be the root"), + ("query_text_may_be_truncated", ["DarlingMcpPgBlockingTools.cs"], + "the statement text cut at COLLECTION to the collector's per-row text cap (track_activity_query_size on the target is the other cutter) — the store never held the rest"), + ]; + + public static readonly (string Key, string[] Files, string WhatIsWithheld)[] WithheldSummaryKeys = + [ + ("answered_rows_withheld", ["DarlingMcpPgIndexTools.cs"], "#3594's reach verdict: the answered-index summary is withheld rather than published as a count of the page"), + ("created_rows_withheld", ["DarlingMcpPgServerStateTools.cs"], "#3594's reach verdict: the created-slot summary is withheld rather than published as a count of the page"), + ]; + + public static readonly (string Key, string[] Files, string WhyItSurvives)[] PageCountsUnderANeutralNoun = [ - "answered_rows_withheld", - "capture_was_truncated", - "chain_may_be_truncated", - "chain_truncation_note", - "created_rows_withheld", - "history_capped", - "is_partial", - "limit_reached", - "partial_count", - "query_text_may_be_truncated", - "scan_truncated", - "shown", - "truncated", - "truncation_note", + ("shown", ["DarlingMcpDefaultTraceTools.cs", "DarlingMcpHealthParserTools.cs", "DarlingMcpTools.cs", "McpAnalysisTools.cs", "McpDefaultTraceTools.cs", "McpHealthParserTools.cs"], + "the page's count beside an honest WHOLE total (total_entries / total_events / total_facts, or a _count over the whole in-memory set) — the cut is exact and disclosed by the pair; the #3594 spelling is *_returned + truncated, and the rename is fenced tonight because four PgTarget* test files read shown off get_analysis_facts"), ]; + public static readonly (string Key, string[] Files, string WhatItActuallyIs)[] CutHomonyms = + [ + ("is_partial", ["DarlingMcpPgIndexUsageTools.cs"], "a PARTIAL INDEX (CREATE INDEX … WHERE) — an index property the collector reads off pg_index, not a cut"), + ("partial_count", ["DarlingMcpPgLoggingAuditTools.cs"], "the number of logging-audit facets whose verdict is `partial` (DarlingPgLoggingAudit.Partial) — a verdict tally, not a cut"), + ]; + + /// The spellings this lane retired, named so their return fails as THE regression rather than as + /// an unclassified key: both were x.Count >= cap inferences, and the fix is cap + 1 + + /// McpHelpers.BoundPage + truncated. + public static readonly string[] RetiredCutSpellings = ["limit_reached", "history_capped"]; + /// A payload key at initializer indent: name = …, or the shorthand name, alone on /// its line. Keys only — the value side is not read — so a local named truncated assigned in the /// body is counted once, where it is emitted. private static readonly Regex PayloadKey = new(@"^\s+([a-z][a-z0-9_]*)\s*(?:=(?!=)|,\s*$)", RegexOptions.Compiled | RegexOptions.Multiline); + /// Every spelling a cut has been given or could plausibly be given next: the fourteen the + /// inventory found, plus the shapes a new page cut would most likely take (*_reached, *_capped, + /// cap_hit / limit_hit, has_more, more_available, omitted). Widened on purpose beyond what + /// the tree contains: a negative census is only as good as the shapes it looks for. private static readonly Regex AboutACut = new( - @"trunc|^limit_reached$|^is_partial$|^partial_count$|^partial$|_capped$|^shown$|^more_available$|^has_more$|_withheld$|^omitted$", + @"trunc|_reached$|^capped$|_capped$|^(?:cap|limit|ceiling)_hit$|^is_partial$|^partial_count$|^partial$|^shown$|^more_available$|^has_more$|_withheld$|^omitted$", RegexOptions.Compiled); + private static IEnumerable<(string Key, string[] Files)> ClassifiedCutKeys() => + SecondBoundCutKeys.Select(k => (k.Key, k.Files)) + .Concat(CutNoteKeys.Select(k => (k.Key, k.Files))) + .Concat(SourceSideCutKeys.Select(k => (k.Key, k.Files))) + .Concat(WithheldSummaryKeys.Select(k => (k.Key, k.Files))) + .Concat(PageCountsUnderANeutralNoun.Select(k => (k.Key, k.Files))) + .Concat(CutHomonyms.Select(k => (k.Key, k.Files))); + [Fact] - public void TheTruncationKeyInventory_IsExact_OnBothSkus() + public void EveryCutKey_IsThePageDialect_OrClassified_OnBothSkus() { var files = 0; var found = new Dictionary>(StringComparer.Ordinal); @@ -747,48 +840,166 @@ public void TheTruncationKeyInventory_IsExact_OnBothSkus() } Assert.True(files >= 60, $"only {files} tool sources were read across both SKUs"); - /* The spelling every other census keys on must be in the inventory on most tool files, or the key - matcher is dead. */ + /* The page dialect must be on most tool files, or the key matcher is dead. */ Assert.True(found.TryGetValue("truncated", out var truncated) && truncated.Count >= 25, "the key matcher no longer finds `truncated` across twenty-odd tool files"); - Assert.True(TruncationKeyDialects.ToHashSet(StringComparer.Ordinal).SetEquals(found.Keys), - "the truncation-key inventory no longer matches the tree — new dialect(s): [" - + string.Join("; ", found.Keys.Except(TruncationKeyDialects).Order().Select(k => $"{k} on {string.Join(", ", found[k])}")) - + "]; rostered but gone (the collapse landed — shrink the roster): [" - + string.Join(", ", TruncationKeyDialects.Except(found.Keys).Order()) + "]"); + + var classified = ClassifiedCutKeys().ToDictionary(k => k.Key, k => k.Files.Order(StringComparer.Ordinal).ToArray(), StringComparer.Ordinal); + Assert.True(classified.Keys.Distinct(StringComparer.Ordinal).Count() == classified.Count, "a key is classified twice; every cut key belongs to exactly one class"); + + var retired = RetiredCutSpellings.Where(found.ContainsKey).ToList(); + Assert.True(retired.Count == 0, + "a retired cut spelling is back — it was an `x.Count >= cap` inference; fetch cap + 1, bind through McpHelpers.BoundPage and publish `truncated`: " + + string.Join("; ", retired.Select(k => $"{k} on {string.Join(", ", found[k])}"))); + + var unclassified = found.Keys.Where(k => k != "truncated" && !classified.ContainsKey(k)).Order(StringComparer.Ordinal).ToList(); + Assert.True(unclassified.Count == 0, + "a cut spelling with no class — a page cut is spelled `truncated` (observed, beside *_returned); a second bound joins SecondBoundCutKeys as _truncated; a cut made before the store joins SourceSideCutKeys with what was cut; a homonym joins CutHomonyms with what it actually is: " + + string.Join("; ", unclassified.Select(k => $"{k} on {string.Join(", ", found[k])}"))); + + var gone = classified.Keys.Where(k => !found.ContainsKey(k)).Order(StringComparer.Ordinal).ToList(); + Assert.True(gone.Count == 0, "rostered cut keys no longer emitted anywhere (the collapse reached them — shrink the roster): " + string.Join(", ", gone)); + + var moved = classified + .Where(c => !c.Value.SequenceEqual(found[c.Key], StringComparer.Ordinal)) + .Select(c => $"{c.Key}: rostered on [{string.Join(", ", c.Value)}], emitted on [{string.Join(", ", found[c.Key])}]") + .ToList(); + Assert.True(moved.Count == 0, "a classified cut key is emitted from a different set of files than its roster says: " + string.Join("; ", moved)); } + /// The cut matcher fires on every spelling the tree has used and the shapes a new one would take, + /// and not on the neighbours that merely share a token. + [Theory] + [InlineData("truncated", true)] + [InlineData("scan_truncated", true)] + [InlineData("limit_reached", true)] + [InlineData("history_capped", true)] + [InlineData("cap_hit", true)] + [InlineData("has_more", true)] + [InlineData("shown", true)] + [InlineData("answered_rows_withheld", true)] + [InlineData("is_partial", true)] + [InlineData("events_returned", false)] + [InlineData("total_events", false)] + [InlineData("limit", false)] + [InlineData("partial_index_count", false)] + /* Buffer-cache hits share the token a cap hit would use; the arm is anchored to the cap words. */ + [InlineData("shared_blks_hit", false)] + [InlineData("blocks_hit", false)] + public void TheCutMatcher_FiresOnEverySpellingAndTheLikelyNextOnes_AndNotOnNeighbours(string key, bool expected) => + Assert.Equal(expected, AboutACut.IsMatch(key)); + /// - /// Every spelling of a severity word that appears as a whole string literal in a source under either - /// SKU's Mcp directory (tools and the Darling service's own readers alike), with the files that spell - /// it. Three casings: Pascal (Healthy/Warning/Critical/Unknown — the - /// HealthSeverity vocabulary written out by the three PostgreSQL object tools and the AG reader's - /// label switch), lower (warning is audit_config's recommendation status on both SKUs and - /// the wraparound tool's band; unknown is a wait-instrument token, the logging audit's verdict - /// constant and Lite's version fallback), and UPPER (CRITICAL is the plan-cache reader's band; - /// HEALTHY is SQL Server's own synchronization_health_desc token, parsed on the way IN by - /// the AG and fleet readers rather than emitted). Not all are severities and not all are outputs, but - /// they are the same words on the same surface, which is what a vocabulary lane has to know before it - /// collapses anything. Enum vocabularies that reach the wire through row types (DailyHealthBand, - /// HealthSeverity) are not literals here and are pinned by their named instance below rather than - /// swept — a sweep over .ToString() would have to resolve types. + /// The six sites this lane moved onto the page dialect, read back: each fetches one past its cap and binds + /// through McpHelpers.BoundPage, none infers, and the four caller-capped ones publish their page count + /// under a *_returned name. The page census () + /// carries the four tools' full contract; this is the vocabulary half: the spelling is truncated. /// - public static readonly (string Spelling, string[] Files)[] SeverityWordSpellings = + [Theory] + [InlineData("DarlingMcpPgAutovacuumTools.cs", "get_pg_autovacuum_health", "limit + 1", "BoundPage(fetched, limit)", "tables_returned")] + [InlineData("DarlingMcpPgSessionStatesTools.cs", "get_pg_session_states", "limit + 1", "BoundPage(fetched, limit)", "sessions_returned")] + [InlineData("DarlingMcpPgIndexUsageTools.cs", "get_pg_index_usage", "limit + 1", "BoundPage(fetched, limit)", "indexes_returned")] + [InlineData("DarlingMcpPgTableBloatTools.cs", "get_pg_table_bloat", "limit + 1", "BoundPage(fetched, limit)", "tables_returned")] + [InlineData("DarlingMcpTools.cs", "get_analysis_findings", "WindowCoveringLimit + 1", "BoundPage(fetched, FindingOccurrences.WindowCoveringLimit)", null)] + [InlineData("McpAnalysisTools.cs", "get_analysis_findings", "WindowCoveringLimit + 1", "BoundPage(fetched, FindingOccurrences.WindowCoveringLimit)", null)] + public void TheSixMovedSites_FetchOnePastTheCap_AndObserveThroughBoundPage(string file, string tool, string fetch, string bind, string? pageCount) + { + var span = StripComments(ToolMethods().Single(t => t.File == file && t.Tool == tool).Span); + Assert.Contains(fetch, span, StringComparison.Ordinal); + Assert.Contains("McpHelpers." + bind, span, StringComparison.Ordinal); + Assert.DoesNotMatch(new Regex(@"\.Count\s*>=\s*(limit|\w*Limit\w*|\w*PerTable)\b"), span); + Assert.Matches(new Regex(@"\n\s+truncated,"), span); + if (pageCount is not null) + { + Assert.Contains(pageCount + " = ", span, StringComparison.Ordinal); + } + } + + /// The run-history block one layer down: RecentRuns binds to RunsReadPerTable and the + /// tool asks the log for one more per relation, so the flag it publishes is the same observed + /// truncated. + [Fact] + public void TheAutovacuumRunHistory_BindsToItsCapAndObserves() + { + var source = StripComments(ToolSources().Single(s => s.File == "DarlingMcpPgAutovacuumTools.cs").Source); + Assert.Contains("RunsReadPerTable + 1);", source, StringComparison.Ordinal); + Assert.Contains("McpHelpers.BoundPage(runs, RunsReadPerTable)", source, StringComparison.Ordinal); + Assert.DoesNotContain("history_capped", source, StringComparison.Ordinal); + Assert.DoesNotContain(">= RunsReadPerTable", source, StringComparison.Ordinal); + } + + /* ───────────────────────── rule: one vocabulary — severity words ───────────────────────── */ + + /// + /// The band canon. One spelling for a health band on the wire: the PascalCase enum token — + /// Healthy / Warning / Critical / Unknown (HealthSeverity, + /// AgTopology.SeverityLabel, ServerHealthClassifier.BandLabel) and NoData for the daily + /// band's no-verdict (DailyHealthBand). It is the spelling the web client keys its CSS classes on + /// (band-Warning, sev-Critical). These are the files that write a band out as a string literal; + /// every other band reaches the wire through an enum's ToString() and cannot drift by spelling. + /// + public static readonly (string Spelling, string[] Files)[] BandCanonSpellings = [ - ("CRITICAL", ["DarlingPlanCacheSchedulerReader.cs"]), ("Critical", ["DarlingAgReader.cs", "DarlingMcpPgIndexUsageTools.cs", "DarlingMcpPgSessionStatesTools.cs", "DarlingMcpPgTableBloatTools.cs"]), - ("HEALTHY", ["DarlingAgReader.cs", "DarlingFleetReader.cs"]), ("Healthy", ["DarlingAgReader.cs", "DarlingMcpPgIndexUsageTools.cs", "DarlingMcpPgSessionStatesTools.cs", "DarlingMcpPgTableBloatTools.cs"]), - ("Unknown", ["DarlingAgReader.cs", "DarlingMcpPgSessionStatesTools.cs", "DarlingMcpPgTableBloatTools.cs", "DarlingMcpTools.cs", "McpAnalysisTools.cs"]), + ("Unknown", ["DarlingAgReader.cs", "DarlingMcpPgSessionStatesTools.cs", "DarlingMcpPgTableBloatTools.cs"]), ("Warning", ["DarlingAgReader.cs", "DarlingMcpPgIndexUsageTools.cs", "DarlingMcpPgSessionStatesTools.cs", "DarlingMcpPgTableBloatTools.cs"]), - ("unknown", ["DarlingMcpPgWaitSamplingTools.cs", "DarlingPgLoggingAudit.cs", "McpHealthTools.cs"]), - ("warning", ["DarlingMcpPgWraparoundTools.cs", "DarlingMcpTools.cs", "McpAnalysisTools.cs"]), + ]; + + /// + /// Every other literal that spells a severity word under either SKU's Mcp directory, with what it + /// actually is. #3699 counted eight spellings of four words; read from the source, six of the eight are + /// not bands at all — they are other vocabularies that happen to use the same word, or the word on the way + /// IN — and moving them to the canon would either break a scale whose other rungs are not severity words or + /// rewrite what a DMV says. Each is rostered by (spelling, file) with the vocabulary it belongs to, so a + /// NEW lower- or upper-case severity literal in any other file fails as a band spelled wrong, and a rostered + /// one that disappears fails until the roster follows it. + /// + /// The one of these that is a severity by any reading — the PostgreSQL tools' severity-TOKEN ladder, + /// ok / info_* / warning[_*] / critical_* (autovacuum, replication slots, wraparound; the web client + /// renders the token untranslated by design, server-tabs.js) — is a different vocabulary from the + /// band, not a case variant of it: the tier is a prefix and the REASON is the rest of the token. Collapsing + /// it into the canon means splitting severity into a band plus a reason on three tools, which + /// is a payload reshape and a maintainer's call, not a spelling. The bare warning on the wraparound + /// ladder is that family's tier with no qualifier, and + /// executes the ladder around it. + /// + public static readonly (string Spelling, string File, string WhatItIs)[] SeverityHomonyms = + [ + ("CRITICAL", "DarlingPlanCacheSchedulerReader.cs", + "bloat_level's top rung of the Dashboard's NORMAL / MEDIUM / HIGH / CRITICAL level scale, reproduced verbatim on both SKUs (Lite: LocalDataService.ClassifyPlanCacheBloat) and coloured by both viewers' BloatLevelBrush — a level scale whose other three rungs are not severity words"), + ("HEALTHY", "DarlingAgReader.cs", + "SQL Server's own synchronization_health_desc token, PARSED on the way in and banded to HealthSeverity — input vocabulary, never emitted"), + ("HEALTHY", "DarlingFleetReader.cs", + "the collector-health row status (the shared NEVER_RUN / NO_PERMISSIONS / FAILING / STALE / WARNING / HEALTHY vocabulary), compared on the way in to count a server's healthy collectors — input vocabulary"), + ("Unknown", "DarlingMcpTools.cs", + "audit_config's edition-NAME fallback (Enterprise / Standard / … / Unknown) — a name, spelled as the canon by coincidence"), + ("Unknown", "McpAnalysisTools.cs", + "audit_config's edition-NAME fallback — the Lite twin of the above"), + ("unknown", "DarlingMcpPgWaitSamplingTools.cs", + "the wait-instrument token for an arm this build does not know (PgWaitInstrument's service_sampled / pg_wait_sampling / … vocabulary) — an instrument, not a band"), + ("unknown", "DarlingPgLoggingAudit.cs", + "the logging audit's verdict vocabulary (instrumented / partial / off / unknown) — a verdict, not a band"), + ("unknown", "McpHealthTools.cs", + "get_collection_health's version-string fallback when the assembly carries no version — a version, not a band"), + ("warning", "DarlingMcpPgWraparoundTools.cs", + "the bare tier of the PostgreSQL severity-TOKEN ladder (ok / info_anti_wraparound_vacuum_expected / warning / critical_failsafe_range / critical_wraparound_imminent) — a tier-prefixed token family, not a case variant of the band"), + ("warning", "DarlingMcpTools.cs", + "audit_config's per-recommendation status (ok / warning / review) — the lower-case status vocabulary every `status` key on the surface speaks, not a band"), + ("warning", "McpAnalysisTools.cs", + "audit_config's per-recommendation status — the Lite twin of the above"), ]; private static readonly Regex SeverityWord = new( @"^(?:healthy|warning|critical|unknown|nodata|no data|info|elevated)$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + /// + /// Every severity-word literal under either SKU's Mcp directory is the band canon in a rostered file, or a + /// rostered homonym — exactly. Bound: whole string-literal bodies (a description that contains the word + /// in a sentence is not a literal that IS the word), on tool and reader sources alike; the literal + /// walker's floor is 5,000 literals across both SKUs. + /// [Fact] - public void TheSeverityWordInventory_IsExact_OnBothSkus() + public void EverySeverityWordLiteral_IsTheBandCanon_OrARosteredHomonym_OnBothSkus() { var found = new Dictionary>(StringComparer.Ordinal); var literals = 0; @@ -812,36 +1023,119 @@ public void TheSeverityWordInventory_IsExact_OnBothSkus() } Assert.True(literals >= 5000, $"only {literals} string literals were read across both SKUs' tool sources; the literal walker has stopped matching"); - var expected = SeverityWordSpellings.ToDictionary(s => s.Spelling, s => s.Files.Order(StringComparer.Ordinal).ToArray(), StringComparer.Ordinal); - var actual = found.ToDictionary(f => f.Key, f => f.Value.ToArray(), StringComparer.Ordinal); + + /* The canon is PascalCase by construction: a homonym may share the canon's spelling (the edition + fallback does) but a canon spelling is never lower- or upper-case. */ + Assert.All(BandCanonSpellings, c => Assert.Matches(new Regex("^[A-Z][a-z]+$"), c.Spelling)); + + var expected = new Dictionary>(StringComparer.Ordinal); + foreach (var (spelling, files) in BandCanonSpellings) + { + expected[spelling] = new SortedSet(files, StringComparer.Ordinal); + } + + foreach (var (spelling, file, _) in SeverityHomonyms) + { + if (!expected.TryGetValue(spelling, out var files)) + { + expected[spelling] = files = new SortedSet(StringComparer.Ordinal); + } + + Assert.True(files.Add(file), $"{spelling} in {file} is rostered twice"); + } + + var report = string.Join("; ", found.OrderBy(a => a.Key, StringComparer.Ordinal).Select(a => $"{a.Key} in {string.Join(", ", a.Value)}")); Assert.True( - expected.Keys.ToHashSet(StringComparer.Ordinal).SetEquals(actual.Keys) - && expected.All(e => e.Value.SequenceEqual(actual[e.Key], StringComparer.Ordinal)), - "the severity-word inventory no longer matches the tree: found [" - + string.Join("; ", actual.OrderBy(a => a.Key, StringComparer.Ordinal).Select(a => $"{a.Key} in {string.Join(", ", a.Value)}")) - + "]"); + expected.Keys.ToHashSet(StringComparer.Ordinal).SetEquals(found.Keys) + && expected.All(e => e.Value.SetEquals(found[e.Key])), + "the severity-word classification no longer matches the tree — a new lower- or upper-case literal is a band spelled wrong (the canon is Healthy / Warning / Critical / Unknown / NoData), unless it is another vocabulary, in which case it joins SeverityHomonyms with what it is. Found: [" + report + "]"); + } + + /// The canon, executed off its sources of truth: the enum tokens and the two label helpers agree + /// on the spelling, and the daily band's no-verdict token has no space in it. + [Fact] + public void TheBandCanon_IsWhatTheSharedClassifiersSpell() + { + Assert.Equal("Warning", AgTopology.SeverityLabel(HealthSeverity.Warning)); + Assert.Equal("Critical", ServerHealthClassifier.BandLabel(FleetHealthBand.Critical)); + Assert.Equal("Healthy", HealthSeverity.Healthy.ToString()); + Assert.Equal("Unknown", HealthSeverity.Unknown.ToString()); + Assert.Equal("NoData", DailyHealthBand.NoData.ToString()); + Assert.Equal("Offline", FleetHealthBand.Offline.ToString()); + } + + /// + /// CRITICAL in the plan-cache reader is a rung of a LEVEL scale, not a band: the same helper on Lite + /// spells all four rungs identically (read from source, because this project does not reference the + /// desktop app), and the Darling one is executed at each boundary. Moving one rung to Critical + /// would leave Critical / HIGH / MEDIUM / NORMAL — a scale in two cases, which is the defect this + /// file exists to prevent. + /// + [Fact] + public void ThePlanCacheBloatLevel_IsOneScaleOnBothSkus_AndNotABand() + { + Assert.Equal("CRITICAL", DarlingPlanCacheSchedulerReader.ClassifyPlanCacheBloat(100, 51).Level); + Assert.Equal("HIGH", DarlingPlanCacheSchedulerReader.ClassifyPlanCacheBloat(100, 31).Level); + Assert.Equal("MEDIUM", DarlingPlanCacheSchedulerReader.ClassifyPlanCacheBloat(100, 21).Level); + Assert.Equal("NORMAL", DarlingPlanCacheSchedulerReader.ClassifyPlanCacheBloat(100, 20).Level); + Assert.Equal("NORMAL", DarlingPlanCacheSchedulerReader.ClassifyPlanCacheBloat(0, 0).Level); + + var lite = File.ReadAllText(RepoFile.PathTo("Lite/Services/LocalDataService.PlanCache.cs")); + var liteHelper = lite[lite.IndexOf("ClassifyPlanCacheBloat(long totalPlans", StringComparison.Ordinal)..]; + liteHelper = liteHelper[..liteHelper.IndexOf("\n }", StringComparison.Ordinal)]; + foreach (var rung in new[] { "\"CRITICAL\"", "\"HIGH\"", "\"MEDIUM\"", "\"NORMAL\"" }) + { + Assert.Contains(rung, liteHelper, StringComparison.Ordinal); + } } /// - /// The named instance behind "NoData and No Data in one payload": get_daily_summary on both SKUs - /// publishes health_band = HealthBand.ToString() (NoData) beside overall_health = - /// DailyHealthBandCalculator.Label(…) (No Data) — the same band under two spellings in one - /// object. Pinned as the mechanism, executed against the shared calculator, so the lane that collapses - /// the pair has the two sites and the two spellings by name and this fact to delete. + /// warning in the wraparound tool is a TIER of a token ladder whose neighbours carry the tier as a + /// prefix and the reason as the rest — executed at each boundary — not a lower-case band. The same family + /// on the autovacuum and slot tools spells every non-ok rung with a reason; the wraparound ladder's + /// warning rung is the one bare tier in the family, and that is a rename for the maintainer's ruling, not + /// a case fix. /// [Fact] - public void TheDailySummary_CarriesTheSameBandUnderTwoSpellings_OnBothSkus() + public void TheLadderTier_IsAPrefixOfATokenFamily_NotABand() + { + Assert.Equal("ok", DarlingMcpPgWraparoundTools.Classify(10.0, 20.0)); + Assert.StartsWith("info_", DarlingMcpPgWraparoundTools.Classify(10.0, 100.0), StringComparison.Ordinal); + Assert.Equal("warning", DarlingMcpPgWraparoundTools.Classify(60.0, 100.0)); + Assert.StartsWith("critical_", DarlingMcpPgWraparoundTools.Classify(80.0, 100.0), StringComparison.Ordinal); + Assert.StartsWith("critical_", DarlingMcpPgWraparoundTools.Classify(99.0, 100.0), StringComparison.Ordinal); + + Assert.StartsWith("warning_", DarlingMcpPgAutovacuumTools.Classify(false, 2.5, false), StringComparison.Ordinal); + Assert.StartsWith("critical_", DarlingMcpPgAutovacuumTools.Classify(false, 10.0, null), StringComparison.Ordinal); + Assert.StartsWith("warning_", DarlingMcpPgSlotTools.Classify("extended", true, null), StringComparison.Ordinal); + Assert.StartsWith("critical_", DarlingMcpPgSlotTools.Classify("lost", false, null), StringComparison.Ordinal); + } + + /// + /// The named instance behind "NoData and No Data in one payload", now closed: get_daily_summary on + /// both SKUs publishes overall_health and health_band from the SAME expression — the enum + /// token — so the two keys cannot disagree. The calculator's human label ("No Data") still exists, for the + /// desktop viewers' tooltips, and the executed pair below is the proof that it differs from the token on + /// exactly one value and that the token is what the wire now carries. row.OverallHealth (the label) + /// no longer appears in either tool. + /// + [Fact] + public void TheDailySummary_PublishesOneBandSpelling_OnBothSkus() { Assert.Equal("NoData", DailyHealthBand.NoData.ToString()); Assert.Equal("No Data", DailyHealthBandCalculator.Label(DailyHealthBand.NoData)); Assert.Equal("Healthy", DailyHealthBandCalculator.Label(DailyHealthBand.Healthy)); - foreach (var (file, tool) in new[] { ("DarlingMcpHealthTools.cs", "get_daily_summary"), ("McpHealthTools.cs", "get_daily_summary") }) + foreach (var (file, tool) in new[] + { + ("DarlingMcpHealthTools.cs", "get_daily_summary"), ("DarlingMcpHealthTools.cs", "get_daily_summary_range"), + ("McpHealthTools.cs", "get_daily_summary"), ("McpHealthTools.cs", "get_daily_summary_range"), + }) { - var span = ToolMethods().Single(t => t.File == file && t.Tool == tool).Span; - var code = StripComments(span); + var code = StripComments(ToolMethods().Single(t => t.File == file && t.Tool == tool).Span); Assert.Contains("health_band = row.HealthBand.ToString()", code, StringComparison.Ordinal); - Assert.Contains("overall_health = row.OverallHealth", code, StringComparison.Ordinal); + Assert.Contains("overall_health = row.HealthBand.ToString()", code, StringComparison.Ordinal); + Assert.DoesNotContain("OverallHealth", code, StringComparison.Ordinal); } } diff --git a/Darling/Darling.Tests/PgLogEventMetricsTests.cs b/Darling/Darling.Tests/PgLogEventMetricsTests.cs index 6c528261ce..2eca907eda 100644 --- a/Darling/Darling.Tests/PgLogEventMetricsTests.cs +++ b/Darling/Darling.Tests/PgLogEventMetricsTests.cs @@ -381,7 +381,7 @@ public void TheRunShape_IsOneSpelling_OnBothSurfaces_AndKeepsNullsInside() BufferHits: 10, BufferMisses: 128, BufferDirtied: 64, WalRecords: 5, WalBytes: 1024)).ToList(); var block = JsonSerializer.Serialize(DarlingMcpPgAutovacuumTools.RecentRuns(runs), McpHelpers.JsonOptions); JsonAssert.Contains("\"runs_counted\": 7", block); - JsonAssert.Contains("\"history_capped\": false", block); + JsonAssert.Contains("\"truncated\": false", block); JsonAssert.Contains("\"vacuum_runs\": 5", block); JsonAssert.Contains("\"analyze_runs\": 2", block); JsonAssert.Contains("\"total_duration_ms\": 28000", block); @@ -395,9 +395,19 @@ public void TheRunShape_IsOneSpelling_OnBothSurfaces_AndKeepsNullsInside() Assert.Null(DarlingMcpPgAutovacuumTools.RecentRuns(null)); Assert.Null(DarlingMcpPgAutovacuumTools.RecentRuns(Array.Empty())); - /* Twenty-five read means "capped": the aggregates are a sample and say so. */ - var many = Enumerable.Range(0, DarlingMcpPgAutovacuumTools.RunsReadPerTable).Select(i => runs[0] with { OccurredAtUtc = at.AddMinutes(-i) }).ToList(); - JsonAssert.Contains("\"history_capped\": true", JsonSerializer.Serialize(DarlingMcpPgAutovacuumTools.RecentRuns(many), McpHelpers.JsonOptions)); + /* #3653: the cut is OBSERVED, not inferred. Exactly RunsReadPerTable runs is a COMPLETE history (the old + `>= RunsReadPerTable` read it as capped); one more is the signal, and the aggregates then cover the + bound page of RunsReadPerTable, never the 26th row. */ + var exactlyTheCap = Enumerable.Range(0, DarlingMcpPgAutovacuumTools.RunsReadPerTable).Select(i => runs[0] with { OccurredAtUtc = at.AddMinutes(-i) }).ToList(); + var complete = JsonSerializer.Serialize(DarlingMcpPgAutovacuumTools.RecentRuns(exactlyTheCap), McpHelpers.JsonOptions); + JsonAssert.Contains("\"truncated\": false", complete); + JsonAssert.Contains($"\"runs_counted\": {DarlingMcpPgAutovacuumTools.RunsReadPerTable}", complete); + var onePast = Enumerable.Range(0, DarlingMcpPgAutovacuumTools.RunsReadPerTable + 1).Select(i => runs[0] with { OccurredAtUtc = at.AddMinutes(-i) }).ToList(); + var capped = JsonSerializer.Serialize(DarlingMcpPgAutovacuumTools.RecentRuns(onePast), McpHelpers.JsonOptions); + JsonAssert.Contains("\"truncated\": true", capped); + JsonAssert.Contains($"\"runs_counted\": {DarlingMcpPgAutovacuumTools.RunsReadPerTable}", capped); + /* The oldest counted run is the 25th newest, not the 26th: the page is the newest RunsReadPerTable. */ + JsonAssert.Contains("\"oldest_counted_run_at\": \"" + at.AddMinutes(-(DarlingMcpPgAutovacuumTools.RunsReadPerTable - 1)).ToString("yyyy-MM-dd'T'HH:mm:ss"), capped); } [Fact] diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpCollectorCostTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpCollectorCostTools.cs index ee3edde7ca..c4cb797ac1 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpCollectorCostTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpCollectorCostTools.cs @@ -56,9 +56,12 @@ public static async Task GetCollectorCost( [Description("Days of history to summarize. Default 7; max 90 (the series' own retention).")] int days_back = 7, [Description("Optional: a collector name (e.g. query_store) to return its daily trend instead of the ranked fleet list.")] string? collector_name = null) { - if (days_back <= 0 || days_back > MaxDaysBack) + /* #3653: the shared day-grained refusal, in ValidateHoursBack's sentence; the ceiling stays this + tool's (the series' own retention). */ + var daysError = McpHelpers.ValidateDaysBack(days_back, MaxDaysBack); + if (daysError != null) { - return $"Invalid days_back value '{days_back}'. Must be a positive integer (1-{MaxDaysBack})."; + return daysError; } var since = DateTime.UtcNow.AddDays(-days_back); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs index 107e753883..560b26dd6c 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs @@ -866,7 +866,10 @@ public static async Task ListServers( } catch (Exception ex) { - return $"Could not read the servers registry from the Postgres store: {ex.Message}"; + /* #3653 (errors one shape): through the shared sentence helper like every other SQL Server-family + tool on this SKU, rather than an interpolated sentence of this tool's own. The operation names + what was being read so the message loses nothing the ad-hoc one said. */ + return McpHelpers.FormatError("list_servers (reading the servers registry from the store)", ex); } /* #2339: the empty-registry answer is prose, not the JSON envelope, so it carries the peer diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs index b77c18670b..f44275f4da 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpHealthTools.cs @@ -88,7 +88,7 @@ collection of ANY collector — a live collection log beside a dead CPU collecto } } - [McpServerTool(Name = "get_daily_summary"), Description("Gets a daily health summary: overall composite health band (Healthy/Warning/Critical), total wait time, top wait type, unique query count, deadlocks, blocking events, memory pressure (and severe memory pressure), high-CPU samples, collection errors, and actionable alert count for one day. Use this for a quick overview to decide which areas need investigation. A day before the store's retention_horizon (the oldest day the shortest-lived signal table still holds) returns status=unavailable with data_state=purged rather than a health band: its per-signal counts would be COALESCEd zeros, not measurements, and a zero is only a measurement inside retention. A returned day carries data_state=collected (a verdict), past_horizon (before the horizon but some signal table still holds rows — the purge has not reached it; No Data, non-zero counts real) or no_run_record (inside retention, no collector run recorded — banded on the counts as read, which are measurements there; the collection-error share has no denominator).")] + [McpServerTool(Name = "get_daily_summary"), Description("Gets a daily health summary: overall composite health band (Healthy/Warning/Critical), total wait time, top wait type, unique query count, deadlocks, blocking events, memory pressure (and severe memory pressure), high-CPU samples, collection errors, and actionable alert count for one day. Use this for a quick overview to decide which areas need investigation. A day before the store's retention_horizon (the oldest day the shortest-lived signal table still holds) returns status=unavailable with data_state=purged rather than a health band: its per-signal counts would be COALESCEd zeros, not measurements, and a zero is only a measurement inside retention. A returned day carries data_state=collected (a verdict), past_horizon (before the horizon but some signal table still holds rows — the purge has not reached it; health_band=NoData, non-zero counts real) or no_run_record (inside retention, no collector run recorded — banded on the counts as read, which are measurements there; the collection-error share has no denominator).")] public static async Task GetDailySummary( NpgsqlDataSource postgres, [Description("Server name or display name.")] string? server_name = null, @@ -117,7 +117,7 @@ day because the two send a caller to different places (nowhere useful, versus co new { summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), - overall_health = row.OverallHealth, + overall_health = row.HealthBand.ToString(), data_state = DailySummaryRetention.Label(row.DataState), retention_horizon = row.RetentionHorizon?.ToString("yyyy-MM-dd"), collection_runs = row.CollectionRuns, @@ -128,13 +128,22 @@ day because the two send a caller to different places (nowhere useful, versus co return McpHelpers.Status( "empty", $"No data collected for {resolved.ServerName} on {row.SummaryDate:yyyy-MM-dd}.", - new { summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), overall_health = row.OverallHealth }); + new { summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), overall_health = row.HealthBand.ToString() }); return JsonSerializer.Serialize(new { server = resolved.ServerName, summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), - overall_health = row.OverallHealth, + /* #3653 (A15/A16, one vocabulary): overall_health and health_band are the SAME band and now + carry the SAME spelling — the enum token (Healthy / Warning / Critical / NoData), which is + what every other band on the wire is spelled as (HealthSeverity, FleetHealthBand) and what + the web client builds its CSS classes from. overall_health used to publish the calculator's + human LABEL, and the two differed on exactly one value: "No Data" beside "NoData" in one + object. The label ("No Data") stays on the row type for the desktop viewers' tooltips; the + wire gets the token. overall_health is kept as a key because it is the older of the two + (the Dashboard's daily-summary column name) — removing it is a wire-shape change, not a + spelling. */ + overall_health = row.HealthBand.ToString(), health_band = row.HealthBand.ToString(), /* #3541 A9: collected, past_horizon or no_run_record here (purged returned above); the note says what the zeros are on a non-collected day, null on a collected one. */ @@ -180,8 +189,8 @@ public static async Task GetDailySummaryRange( bounded, because the aggregate underneath scans the RAW per-collection series for every signal except the query count (which routes to the rollup). The ceiling is SHARED with Lite so the two SKUs cannot accept different spans. */ - if (days_back <= 0 || days_back > McpHelpers.MaxDailySummaryDaysBack) - return $"Invalid days_back value '{days_back}'. Must be a positive integer (1-{McpHelpers.MaxDailySummaryDaysBack})."; + var daysError = McpHelpers.ValidateDaysBack(days_back, McpHelpers.MaxDailySummaryDaysBack); + if (daysError != null) return daysError; /* The anchor is the ONLY source of "now" in this body — see AsOfWindowAnchorTests, which fails a tool that advertises as_of and then reads the process clock anyway. (That check is a source scan and @@ -253,7 +262,8 @@ many returned days fall before it. */ days = rows.Select(row => new { summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), - overall_health = row.OverallHealth, + /* #3653: one spelling for one band — see get_daily_summary. */ + overall_health = row.HealthBand.ToString(), health_band = row.HealthBand.ToString(), data_state = DailySummaryRetention.Label(row.DataState), data_note = DailySummaryRetention.Note(row.DataState, range.RetentionHorizon, row.SignalSourcesPresent), diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs index 98cbc0b320..5f04b3407e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgAutovacuumTools.cs @@ -84,7 +84,7 @@ public static async Task GetPgAutovacuumHealth( NpgsqlDataSource postgres, [Description("Server name or display name.")] string? server_name = null, [Description("Hours of history to analyze, used for the dead-tuple growth comparison. Default 24.")] int hours_back = 24, - [Description("Maximum tables to return, worst first. Default 20.")] int limit = 20, + [Description("Maximum tables to return, worst first. Default 20. This is what bounds the page - read truncated to know whether the server held more tables with pending work than were returned; it is observed by fetching one row past this cap, never inferred from a full page.")] int limit = 20, [Description(McpHelpers.AsOfDescription)] string? as_of = null) { var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); @@ -98,8 +98,14 @@ public static async Task GetPgAutovacuumHealth( try { var now = windowEnd; - var rows = await DarlingPgAutovacuumReader.GetPgAutovacuumAsync( - postgres, resolved.ServerId, now.AddHours(-hours_back), now, limit); + /* #3653 (one vocabulary): the page cut is OBSERVED, not inferred. The reader is asked for one row + past the cap and McpHelpers.BoundPage turns that into (page, truncated) — the #3594 dialect every + honest page in the repository speaks. This replaced `limit_reached = tables.Count >= limit`, which + read a server with exactly `limit` tables behind on vacuum as a cut page and told the caller to + raise a limit that had nothing more to give. */ + var fetched = await DarlingPgAutovacuumReader.GetPgAutovacuumAsync( + postgres, resolved.ServerId, now.AddHours(-hours_back), now, limit + 1); + var (rows, truncated) = McpHelpers.BoundPage(fetched, limit); if (rows.Count == 0) { @@ -127,10 +133,13 @@ engine before making the claim (#2532). */ /* #3603: the cost half, read once for every table on the page and attached per table below. The relation key is the parser's `schema.table`; the database rides along because one relation name can exist in several databases and the log names the database on every run. */ + /* One past RunsReadPerTable per relation, for the same reason as the page above: RecentRuns binds + each table's runs to the cap and OBSERVES whether the log held more, instead of inferring it from + a history that happened to be exactly RunsReadPerTable long. */ var runs = await DarlingPgLogEventReader.GetAutovacuumRunsAsync( postgres, resolved.ServerId, now.AddHours(-hours_back), now, rows.Select(r => $"{r.SchemaName}.{r.TableName}").Distinct(StringComparer.Ordinal).ToList(), - RunsReadPerTable); + RunsReadPerTable + 1); var runsByTable = runs .GroupBy(r => (Database: r.DatabaseName, Relation: r.RelationName)) .ToDictionary(g => g.Key, g => g.OrderByDescending(r => r.OccurredAtUtc).ToList()); @@ -224,15 +233,17 @@ when the dead-tuple ratio is not the one that put the table here. */ server = resolved.ServerName, hours_back, status = "tables_with_pending_maintenance", - table_count = tables.Count, + /* The page's count under the page's name (#3594: a page count is never a total, and it is + spelled *_returned so a reader can tell it from one). */ + tables_returned = tables.Count, /* Page-scoped counts: each is computed over the rows the read's LIMIT let through, not - the server. limit_reached is the discriminator that makes that caveat actionable — when - it bit, the caller knows these are top-N figures and can raise the limit (the + the server. truncated is the discriminator that makes that caveat actionable — when it is + true, the caller knows these are top-N figures and can raise the limit (the get_pg_database_stats pattern). */ autovacuum_disabled_count = tables.Count(t => t.autovacuum_disabled), past_threshold_count = tables.Count(t => t.threshold_ratio >= 1 || t.insert_threshold_ratio >= 1), growing_count = tables.Count(t => t.dead_tuples_growing == true), - limit_reached = tables.Count >= limit, + truncated, worst_table = tables[0].table_name, worst_severity = tables[0].severity, /* #3603: how much of the page the cost half could speak for, and — when it is none — the @@ -289,16 +300,26 @@ reasons this read can and cannot separate. Said once here rather than per table. return any ? total : null; } + /* #3653 (one vocabulary): the caller hands in up to RunsReadPerTable + 1 runs (the tool reads one past + the cap per relation); the cap is applied HERE and the cut observed — `truncated` is true exactly when + the log held a run this block did not count. This replaced `history_capped = runs.Count >= + RunsReadPerTable`, the same `>= cap` inference #3594 named on the page tools, one layer down and + against a collector constant rather than the caller's limit: a table with exactly 25 logged runs read + as capped. Every aggregate below is over the bound page, so the denominator is still RunsReadPerTable + at most. */ + var (page, truncated) = McpHelpers.BoundPage(runs, RunsReadPerTable); + runs = page; + var durations = runs.Where(r => r.DurationMs is not null).Select(r => r.DurationMs!.Value).ToList(); var totalMisses = Sum(runs.Select(r => r.BufferMisses)); var totalDirtied = Sum(runs.Select(r => r.BufferDirtied)); return new { - /* The denominator of every aggregate here — capped at RunsReadPerTable, and history_capped says - when the cap bit, so "25 runs" on a table that ran 200 times reads as a sample, not a count. */ + /* The denominator of every aggregate here — capped at RunsReadPerTable, and truncated says when + the cap bit, so "25 runs" on a table that ran 200 times reads as a sample, not a count. */ runs_counted = runs.Count, - history_capped = runs.Count >= RunsReadPerTable, + truncated, vacuum_runs = runs.Count(r => !r.IsAnalyze), analyze_runs = runs.Count(r => r.IsAnalyze), newest_run_at = runs[0].OccurredAtUtc, diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIndexUsageTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIndexUsageTools.cs index 0b2e88309c..caa06e8c8f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIndexUsageTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgIndexUsageTools.cs @@ -219,7 +219,7 @@ public static async Task GetPgIndexUsage( NpgsqlDataSource postgres, [Description("Server name or display name.")] string? server_name = null, [Description("Hours of history to analyze. Default 168 (seven days). Widen this to the longest interval any scheduled job runs on before calling an index unused.")] int hours_back = 168, - [Description("Maximum indexes to return, biggest unscanned first. Default 25.")] int limit = 25, + [Description("Maximum indexes to return, biggest unscanned first. Default 25. This is what bounds the page - read truncated to know whether the server held more indexes than were returned; it is observed by fetching one row past this cap, never inferred from a full page.")] int limit = 25, [Description(McpHelpers.AsOfDescription)] string? as_of = null) { var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); @@ -234,8 +234,13 @@ public static async Task GetPgIndexUsage( { var end = windowEnd; var start = end.AddHours(-hours_back); - var rows = await DarlingPgIndexUsageReader.GetPgIndexUsageAsync( - postgres, resolved.ServerId, start, end, limit); + /* #3653 (one vocabulary): the page cut is OBSERVED off a limit + 1 fetch through McpHelpers.BoundPage + (the #3594 dialect), replacing `limit_reached = indexes.Count >= limit` — which read a server with + exactly `limit` reportable indexes as a cut page. Bound BEFORE the aggregates below, so every count + is a count of the page. */ + var fetched = await DarlingPgIndexUsageReader.GetPgIndexUsageAsync( + postgres, resolved.ServerId, start, end, limit + 1); + var (rows, truncated) = McpHelpers.BoundPage(fetched, limit); if (rows.Count == 0) { @@ -315,7 +320,8 @@ against production. */ server = resolved.ServerName, hours_back, status = "index_usage", - index_count = indexes.Count, + /* The page's count under the page's name (#3594). */ + indexes_returned = indexes.Count, /* Deliberately NOT called "droppable". These are the indexes with no scans in the window and no structural blocker - which is a shortlist to investigate, not a work queue. The field name has to survive being read by something that will act on it. */ @@ -326,7 +332,7 @@ field name has to survive being read by something that will act on it. */ statistics_were_reset_in_window = resetSeen, /* Same discipline as get_pg_database_stats: every total above covers the rows the LIMIT let through, and the caller has to be able to tell when the cut bit. */ - limit_reached = indexes.Count >= limit, + truncated, note = "An index with no scans is a CANDIDATE, never a conclusion. Three things this data " + "cannot see, in the order they bite: an index backing a constraint enforces it " + "without ever registering a scan; a query that runs less often than " @@ -345,9 +351,9 @@ field name has to survive being read by something that will act on it. */ + "failed CREATE INDEX CONCURRENTLY leftovers, maintained by writes and used by " + "nobody." : string.Empty) - + (indexes.Count >= limit - ? $" The row limit of {limit} was REACHED, so the totals cover only the indexes " - + "returned. Raise limit for the full picture." + + (truncated + ? $" TRUNCATED: the server held more indexes than the {limit} returned, so the totals " + + "cover only the indexes returned. Raise limit for the full picture." : string.Empty), indexes, }, McpHelpers.JsonOptions); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSessionStatesTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSessionStatesTools.cs index 45d670a492..33a2284d05 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSessionStatesTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgSessionStatesTools.cs @@ -234,7 +234,7 @@ public static async Task GetPgSessionStates( NpgsqlDataSource postgres, [Description("Server name or display name.")] string? server_name = null, [Description("Hours of history to analyze. Default 24.")] int hours_back = 24, - [Description("Maximum sessions to return, horizon holders first then longest transaction. Default 25.")] int limit = 25, + [Description("Maximum sessions to return, horizon holders first then longest transaction. Default 25. This is what bounds the page - read truncated to know whether the window held more sessions than were returned; it is observed by fetching one row past this cap, never inferred from a full page.")] int limit = 25, [Description(McpHelpers.AsOfDescription)] string? as_of = null) { var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); @@ -249,8 +249,12 @@ public static async Task GetPgSessionStates( { var end = windowEnd; var start = end.AddHours(-hours_back); - var rows = await DarlingPgSessionStatesReader.GetPgSessionStatesAsync( - postgres, resolved.ServerId, start, end, limit); + /* #3653 (one vocabulary): the page cut is OBSERVED off a limit + 1 fetch through McpHelpers.BoundPage + (the #3594 dialect), replacing `limit_reached = sessions.Count >= limit` — which read a window of + exactly `limit` sessions as a cut page. */ + var fetched = await DarlingPgSessionStatesReader.GetPgSessionStatesAsync( + postgres, resolved.ServerId, start, end, limit + 1); + var (rows, truncated) = McpHelpers.BoundPage(fetched, limit); /* Fetched whether or not there are rows. On a surface where zero rows is the healthy answer, the denominator is not an error path - it is what makes a healthy answer believable. */ @@ -268,7 +272,9 @@ denominator is not an error path - it is what makes a healthy answer believable. var idleWithoutHorizon = rows.Count(r => r.IdleInTransactionSamples > 0 && r.PeakHorizonAge < 0 && !r.StateWasRedacted); var redacted = rows.Count(r => r.StateWasRedacted); - var truncated = rows.Any(r => r.CaptureWasTruncated); + /* The collector's per-capture cap, a SOURCE-side fact carried per row as capture_was_truncated — a + different cut from the page's `truncated` above, and named apart from it on purpose. */ + var captureWasTruncated = rows.Any(r => r.CaptureWasTruncated); var sessions = rows.Select(r => new { @@ -336,7 +342,8 @@ out of six connections is a different instance from two out of four thousand. */ server = resolved.ServerName, hours_back, status = "session_states", - session_count = sessions.Count, + /* The page's count under the page's name (#3594). */ + sessions_returned = sessions.Count, horizon_holder_count = holders, idle_in_transaction_holder_count = idleHolders, /* Reported as its own number because it is the correction this tool exists to make: these @@ -348,7 +355,7 @@ sessions look exactly like the harmful ones and are not. */ first_capture_at = captures.FirstCaptureAt?.ToString("o"), last_capture_at = captures.LastCaptureAt?.ToString("o"), redacted_row_count = redacted, - limit_reached = sessions.Count >= limit, + truncated, note = "This is a SAMPLE taken every collection cycle, not an event log. PostgreSQL records " + "nothing about session state unless something asks it, so a transaction that opened " + "and closed between two samples left no trace - " @@ -367,14 +374,14 @@ sessions look exactly like the harmful ones and are not. */ + "than refusing the read. Those rows say nothing about session state and their " + "severity is reported as unknown." : string.Empty) - + (truncated + + (captureWasTruncated ? " At least one capture hit the collector's per-capture row cap, so the stored " + "rows for it are a worst-first sample of a larger set - compare " + "reportable_sessions_on_instance against the rows returned." : string.Empty) - + (sessions.Count >= limit - ? $" The row limit of {limit} was REACHED, so the counts above cover only the " - + "sessions returned. Raise limit for the full picture." + + (truncated + ? $" TRUNCATED: the window held more sessions than the {limit} returned, so the counts " + + "above cover only the sessions returned. Raise limit for the full picture." : string.Empty), sessions, }, McpHelpers.JsonOptions); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgTableBloatTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgTableBloatTools.cs index c1a3e2f9c3..c64d88b98f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgTableBloatTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPgTableBloatTools.cs @@ -223,7 +223,7 @@ public static async Task GetPgTableBloat( NpgsqlDataSource postgres, [Description("Server name or display name.")] string? server_name = null, [Description("Hours of history to analyze. Default 168 (seven days), which shows whether the waste is growing or being reclaimed.")] int hours_back = 168, - [Description("Maximum tables to return, biggest estimated waste first. Default 25.")] int limit = 25, + [Description("Maximum tables to return, biggest estimated waste first. Default 25. This is what bounds the page - read truncated to know whether the server held more measured tables than were returned; it is observed by fetching one row past this cap, never inferred from a full page.")] int limit = 25, [Description(McpHelpers.AsOfDescription)] string? as_of = null) { var (resolved, error) = await DarlingServerResolver.ResolveOrErrorAsync(postgres, server_name); @@ -238,8 +238,13 @@ public static async Task GetPgTableBloat( { var end = windowEnd; var start = end.AddHours(-hours_back); - var rows = await DarlingPgTableBloatReader.GetPgTableBloatAsync( - postgres, resolved.ServerId, start, end, limit); + /* #3653 (one vocabulary): the page cut is OBSERVED off a limit + 1 fetch through McpHelpers.BoundPage + (the #3594 dialect), replacing `limit_reached = tables.Count >= limit` — which read a server with + exactly `limit` measured tables as a cut page. Bound BEFORE the aggregates below, so every count and + every estimate sum is over the page. */ + var fetched = await DarlingPgTableBloatReader.GetPgTableBloatAsync( + postgres, resolved.ServerId, start, end, limit + 1); + var (rows, truncated) = McpHelpers.BoundPage(fetched, limit); if (rows.Count == 0) { @@ -357,7 +362,8 @@ estimate was or was not published rather than having to take it on faith. */ server = resolved.ServerName, hours_back, status = "table_bloat", - table_count = tables.Count, + /* The page's count under the page's name (#3594). */ + tables_returned = tables.Count, /* Named for what it is: a sum of ESTIMATES over the tables whose estimates were fit to publish. Not "reclaimable bytes", which is a promise the arithmetic cannot make. */ estimated_bloat_bytes_over_trusted_rows = trustedBloatBytes, @@ -365,7 +371,7 @@ estimate was or was not published rather than having to take it on faith. */ trusted_estimate_count = trusted.Count, suppressed_estimate_count = suppressedCount, pgstattuple_available_anywhere = anyPgstattuple, - limit_reached = tables.Count >= limit, + truncated, /* Named at the top so it is read before any number below it. */ figures_are_estimates_not_measurements = true, note = "Every bloat figure here is an ESTIMATE computed from PostgreSQL's column-width " @@ -383,9 +389,9 @@ estimate was or was not published rather than having to take it on faith. */ + "whole instance; on PostgreSQL 13 that role does not exist and explicit GRANT " + "SELECT is the only route." : string.Empty) - + (tables.Count >= limit - ? $" The row limit of {limit} was REACHED, so the totals cover only the tables " - + "returned. Raise limit for the full picture." + + (truncated + ? $" TRUNCATED: the server held more measured tables than the {limit} returned, so the " + + "totals cover only the tables returned. Raise limit for the full picture." : string.Empty), tables, }, McpHelpers.JsonOptions); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStallProbeTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStallProbeTools.cs index d834ef6562..6596fa9636 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStallProbeTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStallProbeTools.cs @@ -52,9 +52,12 @@ public static async Task GetCollectorStallProbes( [Description("Days of history. Default 7; max 60 (the samples' own retention).")] int days_back = 7, [Description("Maximum sample rows to return. Default 50.")] int limit = DefaultLimit) { - if (days_back <= 0 || days_back > MaxDaysBack) + /* #3653: the shared day-grained refusal, in ValidateHoursBack's sentence; the ceiling stays this + tool's (the samples' own retention). */ + var daysError = McpHelpers.ValidateDaysBack(days_back, MaxDaysBack); + if (daysError != null) { - return $"Invalid days_back value '{days_back}'. Must be a positive integer (1-{MaxDaysBack})."; + return daysError; } var validation = McpHelpers.ValidateTop(limit); diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStoreMetricsTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStoreMetricsTools.cs index 5e6bbfc1c4..49b668622e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStoreMetricsTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpStoreMetricsTools.cs @@ -46,9 +46,12 @@ public static async Task GetStoreMetrics( NpgsqlDataSource postgres, [Description("Days of daily-series history. Default 30; max 400 (the series' own retention).")] int days_back = 30) { - if (days_back <= 0 || days_back > MaxDaysBack) + /* #3653: the shared day-grained refusal, in ValidateHoursBack's sentence; the ceiling stays this + tool's (the daily series' own retention). */ + var daysError = McpHelpers.ValidateDaysBack(days_back, MaxDaysBack); + if (daysError != null) { - return $"Invalid days_back value '{days_back}'. Must be a positive integer (1-{MaxDaysBack})."; + return daysError; } try diff --git a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpTools.cs b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpTools.cs index 5df23a71f0..e52a2b87d7 100644 --- a/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpTools.cs +++ b/Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpTools.cs @@ -802,8 +802,15 @@ computed over a silently-truncated read would lie about first_seen/occurrences. from "analyze that window now" (that is analyze_server with the same anchor). Both are worth having: this one is the historical record and cannot change, the other recomputes from whatever rows the store still holds. */ - var findings = await analysisService.GetRecentFindingsAsync( - resolved.ServerId, hours_back, FindingOccurrences.WindowCoveringLimit, asOfUtc: anchor); + /* #3653 (one vocabulary): the read-cap cut is OBSERVED, not inferred. The store is asked for one row + past WindowCoveringLimit and McpHelpers.BoundPage binds the page to the cap — `truncated` is true + exactly when the window held an occurrence the newest-first LIMIT dropped. This replaced + `findings.Count >= WindowCoveringLimit`, the same `>= cap` inference #3594 named on the page tools, + against the store-read constant rather than a caller's limit: a window of exactly 10,000 + occurrences read as cut. The page is the same newest 10,000 the store returned before. */ + var fetched = await analysisService.GetRecentFindingsAsync( + resolved.ServerId, hours_back, FindingOccurrences.WindowCoveringLimit + 1, asOfUtc: anchor); + var (findings, truncated) = McpHelpers.BoundPage(fetched, FindingOccurrences.WindowCoveringLimit); if (findings.Count == 0) { @@ -844,11 +851,13 @@ stats. The store keeps every row — this shapes the read only. */ server = resolved.ServerName, finding_count = groups.Count, total_occurrences = findings.Count, - // No silent caps: a read that fills the window-covering limit has had its OLDEST - // rows dropped by the store's newest-first LIMIT, so occurrence stats may - // under-report — say so instead of letting first_seen quietly lie. - truncation_note = findings.Count >= FindingOccurrences.WindowCoveringLimit - ? $"Read hit the {FindingOccurrences.WindowCoveringLimit}-row cap; the oldest occurrences in the window were dropped, so occurrences/first_seen may under-report. Use a smaller hours_back for exact stats." + // No silent caps: a read the window-covering limit CUT has had its OLDEST rows dropped by + // the store's newest-first LIMIT, so occurrence stats may under-report — say so instead of + // letting first_seen quietly lie. truncated is the #3594 flag (observed above); the note + // says what was cut and what to do about it. + truncated, + truncation_note = truncated + ? $"TRUNCATED: the window held more than the {FindingOccurrences.WindowCoveringLimit}-row read cap; the oldest occurrences in the window were dropped, so total_occurrences, occurrences and first_seen may under-report. Use a smaller hours_back for exact stats." : null, findings = groups.Select(g => { diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js index 0d7585222f..8170bb3736 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/server-tabs.js @@ -3185,7 +3185,7 @@ const PG_SLOT_STATS = [ ]; const PG_AUTOVACUUM_STATS = [ - { key: "table_count", label: "Tables returned", format: "int" }, + { key: "tables_returned", label: "Tables returned", format: "int" }, { key: "past_threshold_count", label: "Past threshold (of those)", format: "int" }, { key: "growing_count", label: "Still growing (of those)", format: "int" }, { key: "autovacuum_disabled_count", label: "Autovacuum off (of those)", format: "int" }, @@ -3230,12 +3230,12 @@ const PG_DATABASE_STATS = [ `estimated_bloat_*_over_trusted_rows` keeps the read's own name: it is a sum of ESTIMATES over the rows whose estimates were fit to publish, and a shorter label would promise reclaimable bytes. */ const PG_BLOAT_STATS = [ - { key: "table_count", label: "Tables returned", format: "int" }, + { key: "tables_returned", label: "Tables returned", format: "int" }, { key: "estimated_bloat_mb_over_trusted_rows", label: "Est. bloat (trusted rows)", format: "mb" }, { key: "trusted_estimate_count", label: "Estimates published", format: "int" }, { key: "suppressed_estimate_count", label: "Estimates suppressed", format: "int" }, { key: "pgstattuple_available_anywhere", label: "pgstattuple installed", format: "bool" }, - { key: "limit_reached", label: "Limit reached", format: "bool" }, + { key: "truncated", label: "Truncated", format: "bool" }, ]; /* The capture tiles lead, and they are not decoration on this read: it is an EXCEPTION surface sampled once @@ -3250,23 +3250,23 @@ const PG_BLOAT_STATS = [ const PG_SESSION_STATE_STATS = [ { key: "captures_in_window", label: "Captures", format: "int" }, { key: "captures_with_sessions", label: "With reportable sessions", format: "int" }, - { key: "session_count", label: "Sessions returned", format: "int" }, + { key: "sessions_returned", label: "Sessions returned", format: "int" }, { key: "horizon_holder_count", label: "Pinned the xmin horizon", format: "int" }, { key: "idle_in_transaction_pinning_nothing", label: "Idle in xact, pinning NOTHING", format: "int" }, { key: "redacted_row_count", label: "Redacted rows", format: "int" }, - { key: "limit_reached", label: "Limit reached", format: "bool" }, + { key: "truncated", label: "Truncated", format: "bool" }, ]; /* "unscanned_without_a_structural_blocker" is deliberately not shortened to "droppable" on the tile either: the field name survived review for saying what it is, and a label that renamed it would undo that where a reader actually looks. */ const PG_INDEX_USAGE_STATS = [ - { key: "index_count", label: "Indexes returned", format: "int" }, + { key: "indexes_returned", label: "Indexes returned", format: "int" }, { key: "unscanned_without_a_structural_blocker", label: "Unscanned, nothing blocking a drop", format: "int" }, { key: "mb_held_by_those_indexes", label: "Held by those indexes", format: "mb" }, { key: "invalid_index_count", label: "Invalid indexes", format: "int" }, { key: "statistics_were_reset_in_window", label: "Stats reset in window", format: "bool" }, - { key: "limit_reached", label: "Limit reached", format: "bool" }, + { key: "truncated", label: "Truncated", format: "bool" }, ]; /* `total_reads` / `total_read_time_ms` are the WINDOW's (#3613); the combination count is the PAGE's and is diff --git a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/util.js b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/util.js index 3b4e3c6254..7330c401d8 100644 --- a/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/util.js +++ b/Darling/PerformanceMonitor.Darling.Service/wwwroot/js/util.js @@ -287,13 +287,25 @@ export function applyFormat(name, value) { /* ─────────────────────────── band / severity classes ─────────────────────────── */ +/* The band vocabulary on the wire is ONE spelling — the PascalCase enum token (HealthSeverity / FleetHealthBand / + DailyHealthBand: Healthy, Warning, Critical, Unknown, Offline, NoData) — and the CSS classes below are keyed on it. + #3653 (one vocabulary) made every MCP band spell it that way; this map is the belt to that brace: a class + builder that compares strings case-insensitively, so a band that arrives as "warning" or "WARNING" from a + surface the census does not sweep still colours, instead of silently producing a class no rule matches. + Anything outside the vocabulary passes through unchanged (a caller that maps "No Data" itself keeps working). */ +const CANONICAL_BANDS = ["Healthy", "Warning", "Critical", "Unknown", "Offline", "NoData"]; +export function canonicalBand(band) { + if (band == null || band === "") return "Unknown"; + const wanted = String(band).toLowerCase(); + return CANONICAL_BANDS.find((b) => b.toLowerCase() === wanted) || String(band); +} /** CSS class for a fleet band ("Healthy"/"Warning"/"Critical"/"Offline") — colors live in CSS. */ export function bandClass(band) { - return "band-" + (band || "Unknown"); + return "band-" + canonicalBand(band); } /** CSS class for a per-metric severity ("Unknown"/"Healthy"/"Warning"/"Critical"). */ export function sevClass(sev) { - return "sev-" + (sev || "Unknown"); + return "sev-" + canonicalBand(sev); } /* ─────────────────────────── object access ─────────────────────────── */ diff --git a/Lite.Tests/McpPageContractTests.cs b/Lite.Tests/McpPageContractTests.cs index 593a7f52d6..f58049555b 100644 --- a/Lite.Tests/McpPageContractTests.cs +++ b/Lite.Tests/McpPageContractTests.cs @@ -512,7 +512,8 @@ public async Task DailySummary_StopsPaintingPurgedDaysGreen_AndPublishesTheHoriz Assert.Equal(1, ghost.GetProperty("collection_runs").GetInt64()); Assert.Equal("purged", ghost.GetProperty("data_state").GetString()); Assert.Equal("NoData", ghost.GetProperty("health_band").GetString()); - Assert.Equal("No Data", ghost.GetProperty("overall_health").GetString()); + /* #3653: one band, one spelling \u2014 overall_health carries the same token health_band does. */ + Assert.Equal("NoData", ghost.GetProperty("overall_health").GetString()); Assert.StartsWith("PURGED", ghost.GetProperty("data_note").GetString(), StringComparison.Ordinal); /* Inside retention with signal rows and no run record: a disclosure, not a withheld verdict — the day diff --git a/Lite/Mcp/McpAnalysisTools.cs b/Lite/Mcp/McpAnalysisTools.cs index 06f3e9ea59..6656993fd6 100644 --- a/Lite/Mcp/McpAnalysisTools.cs +++ b/Lite/Mcp/McpAnalysisTools.cs @@ -745,8 +745,15 @@ computed over a silently-truncated read would lie about first_seen/occurrences. from "analyze that window now" (that is analyze_server with the same anchor). Both are worth having: this one is the historical record and cannot change, the other recomputes from whatever rows the store still holds. */ - var findings = await analysisService.GetRecentFindingsAsync( - resolved.ServerId, hours_back, FindingOccurrences.WindowCoveringLimit, asOfUtc: anchor); + /* #3653 (one vocabulary): the read-cap cut is OBSERVED, not inferred. The store is asked for one row + past WindowCoveringLimit and McpHelpers.BoundPage binds the page to the cap — `truncated` is true + exactly when the window held an occurrence the newest-first LIMIT dropped. This replaced + `findings.Count >= WindowCoveringLimit`, the same `>= cap` inference #3594 named on the page tools, + against the store-read constant rather than a caller's limit: a window of exactly 10,000 + occurrences read as cut. The page is the same newest 10,000 the store returned before. */ + var fetched = await analysisService.GetRecentFindingsAsync( + resolved.ServerId, hours_back, FindingOccurrences.WindowCoveringLimit + 1, asOfUtc: anchor); + var (findings, truncated) = McpHelpers.BoundPage(fetched, FindingOccurrences.WindowCoveringLimit); if (findings.Count == 0) { @@ -787,11 +794,13 @@ stats. The store keeps every row — this shapes the read only. */ server = resolved.ServerName, finding_count = groups.Count, total_occurrences = findings.Count, - // No silent caps: a read that fills the window-covering limit has had its OLDEST - // rows dropped by the store's newest-first LIMIT, so occurrence stats may - // under-report — say so instead of letting first_seen quietly lie. - truncation_note = findings.Count >= FindingOccurrences.WindowCoveringLimit - ? $"Read hit the {FindingOccurrences.WindowCoveringLimit}-row cap; the oldest occurrences in the window were dropped, so occurrences/first_seen may under-report. Use a smaller hours_back for exact stats." + // No silent caps: a read the window-covering limit CUT has had its OLDEST rows dropped by + // the store's newest-first LIMIT, so occurrence stats may under-report — say so instead of + // letting first_seen quietly lie. truncated is the #3594 flag (observed above); the note + // says what was cut and what to do about it. + truncated, + truncation_note = truncated + ? $"TRUNCATED: the window held more than the {FindingOccurrences.WindowCoveringLimit}-row read cap; the oldest occurrences in the window were dropped, so total_occurrences, occurrences and first_seen may under-report. Use a smaller hours_back for exact stats." : null, findings = groups.Select(g => { diff --git a/Lite/Mcp/McpHealthTools.cs b/Lite/Mcp/McpHealthTools.cs index ad8aa679cc..8cda191410 100644 --- a/Lite/Mcp/McpHealthTools.cs +++ b/Lite/Mcp/McpHealthTools.cs @@ -63,7 +63,7 @@ collection of ANY collector — a live collection log beside a dead CPU collecto } } - [McpServerTool(Name = "get_daily_summary"), Description("Gets a daily health summary: overall composite health band (Healthy/Warning/Critical), total wait time, top wait type, unique query count, deadlocks, blocking events, memory pressure (and severe memory pressure), high-CPU samples, collection errors, and actionable alert count for one day. Use this for a quick overview to decide which areas need investigation. A day before the store's retention_horizon (the oldest day the shortest-lived signal table still holds) returns status=unavailable with data_state=purged rather than a health band: its per-signal counts would be COALESCEd zeros, not measurements, and a zero is only a measurement inside retention. A returned day carries data_state=collected (a verdict), past_horizon (before the horizon but some signal table still holds rows — the purge has not reached it; No Data, non-zero counts real) or no_run_record (inside retention, no collector run recorded — banded on the counts as read, which are measurements there; the collection-error share has no denominator).")] + [McpServerTool(Name = "get_daily_summary"), Description("Gets a daily health summary: overall composite health band (Healthy/Warning/Critical), total wait time, top wait type, unique query count, deadlocks, blocking events, memory pressure (and severe memory pressure), high-CPU samples, collection errors, and actionable alert count for one day. Use this for a quick overview to decide which areas need investigation. A day before the store's retention_horizon (the oldest day the shortest-lived signal table still holds) returns status=unavailable with data_state=purged rather than a health band: its per-signal counts would be COALESCEd zeros, not measurements, and a zero is only a measurement inside retention. A returned day carries data_state=collected (a verdict), past_horizon (before the horizon but some signal table still holds rows — the purge has not reached it; health_band=NoData, non-zero counts real) or no_run_record (inside retention, no collector run recorded — banded on the counts as read, which are measurements there; the collection-error share has no denominator).")] public static async Task GetDailySummary( LocalDataService dataService, ServerManager serverManager, @@ -92,7 +92,7 @@ the two send a caller to different places. Darling's twin says the same words. * new { summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), - overall_health = row.OverallHealth, + overall_health = row.HealthBand.ToString(), data_state = DailySummaryRetention.Label(row.DataState), retention_horizon = row.RetentionHorizon?.ToString("yyyy-MM-dd"), collection_runs = row.CollectionRuns, @@ -105,14 +105,22 @@ the two send a caller to different places. Darling's twin says the same words. * return McpHelpers.Status( "empty", $"No data collected for {resolved.ServerName} on {missDate:yyyy-MM-dd}.", - new { summary_date = missDate.ToString("yyyy-MM-dd"), overall_health = row?.OverallHealth }); + new { summary_date = missDate.ToString("yyyy-MM-dd"), overall_health = row?.HealthBand.ToString() }); } return JsonSerializer.Serialize(new { server = resolved.ServerName, summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), - overall_health = row.OverallHealth, + /* #3653 (A15/A16, one vocabulary): overall_health and health_band are the SAME band and now + carry the SAME spelling — the enum token (Healthy / Warning / Critical / NoData), which is + what every other band on the wire is spelled as (HealthSeverity, FleetHealthBand) and what + the Darling web client builds its CSS classes from. overall_health used to publish the + calculator's human LABEL, and the two differed on exactly one value: "No Data" beside + "NoData" in one object. The label stays on the row type for the viewer's tooltip; the wire + gets the token. overall_health is kept as a key because it is the older of the two (the + Dashboard's daily-summary column name) — removing it is a wire-shape change, not a spelling. */ + overall_health = row.HealthBand.ToString(), health_band = row.HealthBand.ToString(), /* #3541 A9: collected, past_horizon or no_run_record here (purged returned above); the note says what the zeros are on a non-collected day, null on a collected one. */ @@ -166,8 +174,8 @@ public static async Task GetDailySummaryRange( if (error != null) return error; /* The ceiling is SHARED with Darling so the two SKUs cannot accept different spans. */ - if (days_back <= 0 || days_back > McpHelpers.MaxDailySummaryDaysBack) - return $"Invalid days_back value '{days_back}'. Must be a positive integer (1-{McpHelpers.MaxDailySummaryDaysBack})."; + var daysError = McpHelpers.ValidateDaysBack(days_back, McpHelpers.MaxDailySummaryDaysBack); + if (daysError != null) return daysError; /* The anchor is the ONLY source of "now" in this body — see AsOfWindowAnchorTests, which fails a tool that advertises as_of and then reads the process clock anyway. (That check is a source scan @@ -234,7 +242,8 @@ first row's is the range's. */ days = rows.Select(row => new { summary_date = row.SummaryDate.ToString("yyyy-MM-dd"), - overall_health = row.OverallHealth, + /* #3653: one spelling for one band — see get_daily_summary. */ + overall_health = row.HealthBand.ToString(), health_band = row.HealthBand.ToString(), data_state = DailySummaryRetention.Label(row.DataState), data_note = row.RetentionHorizon is { } rowHorizon ? DailySummaryRetention.Note(row.DataState, rowHorizon, row.SignalSourcesPresent) : null, diff --git a/PerformanceMonitor.Common/Mcp/McpHelpers.cs b/PerformanceMonitor.Common/Mcp/McpHelpers.cs index 9c527cd6d0..6cbcac4683 100644 --- a/PerformanceMonitor.Common/Mcp/McpHelpers.cs +++ b/PerformanceMonitor.Common/Mcp/McpHelpers.cs @@ -83,6 +83,27 @@ internal static class McpHelpers return null; } + /// + /// Validates a days_back parameter against the ceiling its tool declares. Returns null if valid, an + /// error message if invalid — the day-grained twin of , in the same sentence + /// shape, so a caller who has seen one refusal recognises the other. + /// + /// Why the ceiling is a parameter (#3653, one vocabulary). Five tools took days_back and + /// each refused it INLINE with its own copy of this sentence, because no shared validator existed for a + /// day-grained span and their ceilings differ for real reasons: the daily-summary range reads a year + /// (, shared across both SKUs), the stall-probe samples keep 60 days, + /// the collector-cost series 90, the store-metrics daily series 400 — each the retention of the series it + /// reads, which is the only honest bound on a history read. One validator with the ceiling handed in keeps + /// the refusal one sentence on the wire while leaving each tool's bound where its lineage is documented. + /// Refuses, never clamps: a span the tool cannot honour is an error, not a quietly different question. + /// + public static string? ValidateDaysBack(int daysBack, int maxDaysBack) + { + if (daysBack <= 0 || daysBack > maxDaysBack) + return $"Invalid days_back value '{daysBack}'. Must be a positive integer (1-{maxDaysBack})."; + return null; + } + /// /// Validates a windowed read's two time knobs together and hands back the window's END — the single call /// every windowed read makes in place of a bare . From f5a5000e08a7ef72dea9909d22f1467a0d0e515f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sat, 19 Sep 2026 03:59:05 -0400 Subject: [PATCH 2/2] The one live pin that quoted the viewer label on the wire reads the band token (overall_health = NoData beside health_band), and asserts the two keys agree --- Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs b/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs index 5831227034..a427c4d7ba 100644 --- a/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs +++ b/Darling/Darling.Tests/DarlingMcpHealthToolsTests.cs @@ -450,7 +450,10 @@ deadlock row surviving there means the purge has not reached the day (data_state Assert.Contains("\"deadlock_count\":1", onItsOwnDay, StringComparison.Ordinal); var judged = JsonDocument.Parse(onItsOwnDay).RootElement; Assert.Equal("past_horizon", judged.GetProperty("data_state").GetString()); - Assert.Equal("No Data", judged.GetProperty("overall_health").GetString()); + /* #3653: one band, one spelling — overall_health carries the token health_band does ("NoData", not the + viewer label "No Data"). */ + Assert.Equal("NoData", judged.GetProperty("overall_health").GetString()); + Assert.Equal("NoData", judged.GetProperty("health_band").GetString()); Assert.Contains("1 of 7 signal sources", judged.GetProperty("data_note").GetString(), StringComparison.Ordinal); Assert.DoesNotContain("Healthy", onItsOwnDay, StringComparison.Ordinal); Assert.Contains("2026-07-20", onItsOwnDay, StringComparison.Ordinal);