Darling and Lite's MCP server instructions drop from ~86KB/52KB to under 8KB each (#3898)

Darling/Darling.Tests/AnalysisFactsReadRunsDetectorParityTests.cs

@@ -139,15 +139,16 @@ public void BothToolDescriptions_SayTheDetectorRunsOnThisRead()
             Assert.Contains("the cost is the detector's baseline reads on top of the collector's", attribute, StringComparison.Ordinal);
         }
 
-        /* And the instructions rows no longer describe the read as collect+score while reserving anomaly
-           detection for analyze_server. */
+        /* #3898 Phase 2 (D5): the instructions' per-tool row that used to restate this is gone on both SKUs —
+           the "Asking about a PAST window" section's as_of guardrail still says the anchor reaches the
+           detector, which is the fact this half of the pin protects; the stale collect+score phrasing it
+           guarded against cannot come back once there is no per-tool row left to carry it. */
         foreach (var (sku, instructions) in new[]
         {
             ("Darling", RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingMcpInstructions.cs")),
             ("Lite", RepoFile.ReadRepoFile("Lite", "Mcp", "McpInstructions.cs")),
         })
         {
-            Assert.Contains("`get_analysis_facts` | Exposes raw scored facts from the collect+detect+score pipeline", instructions, StringComparison.Ordinal);
             Assert.Contains("re-run fact collection, anomaly detection and scoring over the anchored window", instructions, StringComparison.Ordinal);
             Assert.DoesNotContain("`analyze_server`'s anomaly detection moves with it", instructions, StringComparison.Ordinal);
         }

Darling/Darling.Tests/CollectionCaveatLedgerTests.cs

@@ -477,32 +477,29 @@ touched one and not the other produced a surface where an agent learned a field
         Assert.Equal(descriptions[0].Text, descriptions[1].Text);
     }
 
+    /// <summary>
+    /// #3898 Phase 2 (D5): the instructions' per-tool row that used to carry this clause is gone on both
+    /// SKUs — <c>get_collection_health</c>'s own description is now the only surface, and it already states
+    /// the clause without the mid-sentence splice this pin was written against (the defect where it landed
+    /// inside "come in two sizes and … the mean describes neither").
+    /// </summary>
     [Fact]
-    public void BothInstructionTables_CarryTheClauseOnTheirCollectionHealthRow()
+    public void BothDescriptions_CarryTheAnalysisCaveatsClause_WithoutASplice()
     {
         foreach (var (sku, path) in new[]
         {
-            ("Darling", new[] { "Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingMcpInstructions.cs" }),
-            ("Lite", new[] { "Lite", "Mcp", "McpInstructions.cs" })
+            ("Darling", new[] { "Darling", "PerformanceMonitor.Darling.Service", "Mcp", "DarlingMcpDataTools.cs" }),
+            ("Lite", new[] { "Lite", "Mcp", "McpHealthTools.cs" })
         })
         {
-            /* The RAW reader, with each line trimmed: every anchor here lives inside ONE table row, so
-               nothing this pin matches spans a line break and the LF reader would state a property it does
-               not have (the RepoFileAdoptionTests distinction). */
             var source = RepoFile.ReadRepoFile(path);
-            var row = source
-                .Split('\n')
-                .Select(l => l.Trim())
-                .SingleOrDefault(l => l.StartsWith("| `get_collection_health` |", StringComparison.Ordinal));
-            Assert.False(row is null, $"{sku}: the get_collection_health instruction row has moved");
-
-            Assert.Contains("; carries analysis_caveats when a recent analysis pass could not read a fact family |", row!, StringComparison.Ordinal);
-            /* The clause is APPENDED to the row's prose, not spliced into the middle of a sentence — the
-               defect this pin was written against, where it landed inside "come in two sizes and … the mean
-               describes neither". */
-            Assert.DoesNotContain("and ; carries analysis_caveats", row!, StringComparison.Ordinal);
-            Assert.DoesNotContain("fact familyread", row!, StringComparison.Ordinal);
-            Assert.DoesNotContain("fact familythe", row!, StringComparison.Ordinal);
+            var at = source.IndexOf("carries analysis_caveats", StringComparison.Ordinal);
+            Assert.True(at > 0, $"{sku}: get_collection_health's analysis_caveats clause has moved");
+
+            var clause = source.Substring(at, 200);
+            Assert.DoesNotContain("mean describes neitheranalysis_caveats", clause, StringComparison.Ordinal);
+            Assert.DoesNotContain("familyread", clause, StringComparison.Ordinal);
+            Assert.DoesNotContain("familythe", clause, StringComparison.Ordinal);
         }
     }
 

Darling/Darling.Tests/DarlingMcpAgToolsTests.cs

@@ -155,13 +155,13 @@ public void Description_SurfacesTheReadersTwoDocumentedTraps()
         Assert.Contains("is_suspended", description, StringComparison.Ordinal);
         Assert.Contains("0 lag while data movement is suspended", description, StringComparison.Ordinal);
 
-        /* The instructions' AG paragraph is the other text an agent reads first; it carries the same two traps
-           in its own words, and the perspective trap (LOCAL-replica-only columns) it always had. */
-        var instructions = DarlingMcpInstructions.Text;
-        Assert.Contains("lag and queue depth are NOT banded", instructions, StringComparison.Ordinal);
-        Assert.Contains("the DMV reports lag as 0 (not null) while data movement is suspended", instructions, StringComparison.Ordinal);
-        Assert.Contains("keeps returning its last non-empty snapshot", instructions, StringComparison.Ordinal);
-        Assert.Contains("populated only for the LOCAL replica", instructions, StringComparison.Ordinal);
+        /* #3898 Phase 2 (D5): the instructions' per-tool AG paragraph is gone — the tool's own description is
+           now the ONLY surface, so the perspective trap (LOCAL-replica-only columns) moved onto it here rather
+           than staying duplicated. All four traps checked in one place. */
+        Assert.Contains("lag and queue depth are NOT banded", description, StringComparison.Ordinal);
+        Assert.Contains("the DMV reports 0 lag while data movement is suspended", description, StringComparison.Ordinal);
+        Assert.Contains("keeps returning its last non-empty snapshot", description, StringComparison.Ordinal);
+        Assert.Contains("populated only for the LOCAL replica", description, StringComparison.Ordinal);
     }
 }
 

Darling/Darling.Tests/DarlingMcpBlockingToolsTests.cs

@@ -151,23 +151,25 @@ public void ParamContract_DedupKeyIsAnAppendedOptionalOnTheIncidentReaders()
     }
 
     /// <summary>
-    /// The instructions have to ADVERTISE <c>dedup_key</c>, or the feature is unreachable in practice: an agent
-    /// picks tools and arguments from this text, and a parameter it never reads is one it never passes. Same
-    /// reason the store-metrics and AG tools pin their own mentions.
-    ///
-    /// <para>Also pins the two caveats that turn an empty result into a diagnosable one — the display-name
-    /// scoping and that <c>hours_back</c> still bounds the search — because those are the failure modes an agent
-    /// would otherwise report as "no such incident".</para>
+    /// The parameter's OWN description has to ADVERTISE <c>dedup_key</c>'s scoping, or the feature is
+    /// unreachable in practice: an agent picks tools and arguments from a tool's own description, and a
+    /// caveat it never reads is one it never accounts for. #3898 Phase 2 (D5) moved this pin off the
+    /// instructions (which used to carry a second, shorter mention) onto the parameter description that was
+    /// always the primary surface — the display-name scoping is the failure mode an agent would otherwise
+    /// report as "no such incident".
     /// </summary>
-    [Fact]
-    public void Instructions_AdvertiseDedupKeyAndItsScoping()
+    [Theory]
+    [InlineData("get_blocking")]
+    [InlineData("get_deadlocks")]
+    [InlineData("get_deadlock_detail")]
+    public void ParamContract_DedupKeyDescription_AdvertisesItsScoping(string tool)
     {
-        var text = DarlingMcpInstructions.Text;
+        var method = ToolMethods().Single(m => m.GetCustomAttribute<McpServerToolAttribute>()!.Name == tool);
+        var description = method.GetParameters().Single(p => p.Name == "dedup_key")
+            .GetCustomAttribute<DescriptionAttribute>()!.Description;
 
-        Assert.Contains("dedup_key", text, StringComparison.Ordinal);
-        Assert.Contains("Dedup Key", text, StringComparison.Ordinal);
-        Assert.Contains("DISPLAY name", text, StringComparison.Ordinal);
-        Assert.Contains("hours_back` still bounds the search", text, StringComparison.Ordinal);
+        Assert.Contains("Dedup Key", description, StringComparison.Ordinal);
+        Assert.Contains("display name", description, StringComparison.Ordinal);
     }
 
     [Fact]

Darling/Darling.Tests/DarlingMcpDataToolsTests.cs

@@ -240,15 +240,16 @@ of the same length would not be visible. Both filters must be findable through i
     /// <summary>
     /// <c>get_collection_log</c>'s parameter list, held identical across every surface that writes it down.
     ///
-    /// <para>Five places name these parameters: Darling's signature, Lite's signature, the two quick-reference
-    /// instruction tables (byte-identical rows on both SKUs), and the <c>/api/catalog</c> descriptor. #3287
-    /// added two filters to the first two and review found the other three stale — and nothing caught it,
-    /// because the new parameters are OPTIONAL and no shipped caller sends them, so every existing pin stayed
-    /// green. That is the same shape as the defect being fixed: a surface advertising a parameter list that no
-    /// longer matches what the read accepts, with no way for a consumer to discover the difference.</para>
+    /// <para>Three places name these parameters: Darling's signature, Lite's signature, and the
+    /// <c>/api/catalog</c> descriptor — down from five once #3898 Phase 2 (D5) retired the two quick-reference
+    /// instruction tables that used to carry byte-identical rows on both SKUs. #3287 added two filters to the
+    /// first two and review found the other three stale — and nothing caught it, because the new parameters
+    /// are OPTIONAL and no shipped caller sends them, so every existing pin stayed green. That is the same
+    /// shape as the defect being fixed: a surface advertising a parameter list that no longer matches what the
+    /// read accepts, with no way for a consumer to discover the difference.</para>
     ///
     /// <para>So the list is derived from the signature and compared, rather than each surface being spot-checked
-    /// for the two names this change happened to add. A third filter added to the tool and not to the tables
+    /// for the two names this change happened to add. A third filter added to the tool and not to the catalog
     /// reds this.</para>
     ///
     /// <para>The catalog is asserted by CONTAINMENT rather than equality, because it names parameters as they
@@ -262,19 +263,6 @@ public void CollectionLogParams_AreTheSameListOnEverySurfaceThatWritesThemDown()
 
         Assert.Equal(reflected, LiteMcpParamNames("get_collection_log", "GetCollectionLog"));
 
-        /* The tables render the list as backticked names joined by ", " — the shape every other row uses. */
-        var expectedCell = string.Join(", ", reflected.Select(n => $"`{n}`"));
-
-        var liteInstructions = File.ReadAllText(Path.Combine(RepoRoot(), "Lite", "Mcp", "McpInstructions.cs"));
-        foreach (var (surface, text) in new[]
-                 {
-                     ("Darling's instruction table", DarlingMcpInstructions.Text),
-                     ("Lite's instruction table", liteInstructions),
-                 })
-        {
-            Assert.Equal(expectedCell, CollectionLogInstructionCell(surface, text));
-        }
-
         var catalog = DarlingWebEndpoints.CatalogDescriptors["get_collection_log"].Params;
 
         foreach (var filter in new[] { "collector_name", "min_duration_ms" })
@@ -430,23 +418,6 @@ private static string ToolDescriptionOf(string source, string toolName)
         return source[at..end];
     }
 
-    /// <summary>The "Key Parameters" cell of the <c>get_collection_log</c> row in a quick-reference table.</summary>
-    private static string CollectionLogInstructionCell(string surface, string text)
-    {
-        var row = text
-            .Split('\n')
-            .SingleOrDefault(l => l.TrimStart().StartsWith("| `get_collection_log` |", StringComparison.Ordinal));
-
-        Assert.True(row is not null, $"{surface} has no `get_collection_log` row — this pin needs re-anchoring.");
-
-        /* Four cells between five pipes, so the parameter list is the last populated one. The row is a single
-           line by convention and its prose carries no pipe, which is what makes this safe. */
-        var cells = row!.TrimEnd('\r').Split('|');
-        Assert.True(cells.Length >= 4, $"{surface}'s `get_collection_log` row is not a four-cell row.");
-
-        return cells[^2].Trim();
-    }
-
     /// <summary>The advertised MCP parameter names of one Lite tool, in declaration order, read out of Lite's
     /// own source — Darling.Tests holds no ProjectReference to Lite, and reading Lite .cs across the seam is
     /// the pattern #2839 established for exactly this.</summary>

Darling/Darling.Tests/DarlingMcpFleetSweepToolsTests.cs

@@ -109,6 +109,7 @@ public void Description_CarriesTheMuteSemantics_AndTheQuietIsNotCleanBlock()
         var description = ToolMethods().Single()
             .GetCustomAttribute<DescriptionAttribute>()!.Description;
 
+        Assert.Contains("MUTE SEMANTICS", description, StringComparison.Ordinal);
         Assert.Contains("DELIVERY WAS OFF", description, StringComparison.Ordinal);
         Assert.Contains("would_have_paged", description, StringComparison.Ordinal);
         Assert.Contains("muted, and nothing would have paged", description, StringComparison.Ordinal);
@@ -118,6 +119,9 @@ public void Description_CarriesTheMuteSemantics_AndTheQuietIsNotCleanBlock()
         Assert.Contains("instrument_liveness", description, StringComparison.Ordinal);
         /* The knob's home is named, so an agent that wants the cadence goes to the settings read. */
         Assert.Contains("fleet_sweep", description, StringComparison.Ordinal);
+        /* #3898 Phase 2: the instructions' cross-server paragraph carried this cadence fact; once that
+           paragraph was cut, the description became the only place an agent could learn it. */
+        Assert.Contains("at most one daily rollup", description, StringComparison.Ordinal);
     }
 
     /// <summary>#3487's output side, stated where an agent reads: every sweep id the payload carries
@@ -240,18 +244,9 @@ which is the whole reason the logger rides by closure. */
             source, StringComparison.Ordinal);
     }
 
-    /// <summary>The house rule every tool suite applies: the instructions an MCP client plans against
-    /// must carry the tool — and for this one, the same two field contracts the description carries,
-    /// because the instructions' cross-server section is where an agent first meets the sweep.</summary>
-    [Fact]
-    public void TheInstructions_CarryTheTool_AndItsFieldContracts()
-    {
-        var text = DarlingMcpInstructions.Text;
-        Assert.Contains("get_sweep_reports", text, StringComparison.Ordinal);
-        Assert.Contains("MUTE SEMANTICS", text, StringComparison.Ordinal);
-        Assert.Contains("QUIET IS NOT CLEAN", text, StringComparison.Ordinal);
-        Assert.Contains("bounded at ONE daily rollup", text, StringComparison.Ordinal);
-    }
+    /* #3898 Phase 2 (D5): the instructions' duplicate cross-server paragraph naming this tool and its field
+       contracts is gone — Description_CarriesTheMuteSemantics_AndTheQuietIsNotCleanBlock above is now the
+       only pin on them, the tool's own description being the sole surface left to carry them. */
 
     /* ---------------- advertised MCP schema ---------------- */
 

Darling/Darling.Tests/DarlingMcpInstructionsBudgetTests.cs

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System;
+using PerformanceMonitor.Darling.Service.Mcp;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #3898 Phase 2 (D2, D5): the server instructions dropped their per-tool "Tool Reference" tables and prose —
+/// duplicated content nothing enforced stayed in sync with the tool descriptions that are the real interface
+/// (#3696, #3821 both drifted). D2's target is 8,000 characters or less; this pins the shape rather than a
+/// single measured number, so a future PR can grow the instructions back toward the old size one paragraph
+/// at a time without ever noticing it crossed the line back into a "Tool Reference" table. A red-watch
+/// confirmed this fails against the pre-#3898-Phase-2 instructions (88,532 / 52,942 characters, and the old
+/// "### Diagnostic-analysis tools" family headers present).
+/// </summary>
+public sealed class DarlingMcpInstructionsBudgetTests
+{
+    private const int InstructionsBudget = 8_000;
+
+    [Fact]
+    public void Instructions_AreWithinTheD2Budget()
+    {
+        var length = DarlingMcpInstructions.Text.Length;
+        Assert.True(length <= InstructionsBudget,
+            $"DarlingMcpInstructions.Text is {length} characters; D2's budget is {InstructionsBudget}");
+    }
+
+    /// <summary>No fleet-coverage declaration reproduces <see cref="DarlingMcpInstructions.Text"/> exactly
+    /// (<c>DarlingPeerDisclosureTests</c> pins that identity); this just confirms the budget holds through the
+    /// public entry point a real host calls, not only the internal constant.</summary>
+    [Fact]
+    public void Build_WithNoDeclaredPeers_StaysWithinTheD2Budget()
+    {
+        var length = DarlingMcpInstructions.Build(DarlingPeerDirectory.Snapshot.Empty).Length;
+        Assert.True(length <= InstructionsBudget,
+            $"DarlingMcpInstructions.Build with no peers is {length} characters; D2's budget is {InstructionsBudget}");
+    }
+
+    /// <summary>The defect this whole phase fixes: a "Tool Reference" table restates what a tool's own
+    /// description already says, and nothing enforces that the copies agree (#3696, #3821). This fails if
+    /// either the old family headings or the old three-column table shape comes back wholesale — the one
+    /// deliberately-kept single-row "Notes" table (<c>get_query_store_health</c>'s capture-mode footnote,
+    /// <c>DarlingMcpConfigHistoryToolsTests</c>' pin) is two columns and carries no "Key Parameters" header, so
+    /// it does not trip this.</summary>
+    [Fact]
+    public void Instructions_CarryNoPerFamilyToolReferenceTable()
+    {
+        var text = DarlingMcpInstructions.Text;
+        Assert.DoesNotContain("### Diagnostic-analysis tools", text, StringComparison.Ordinal);
+        Assert.DoesNotContain("### Core data-read tools", text, StringComparison.Ordinal);
+        Assert.DoesNotContain("### Trend data-read tools", text, StringComparison.Ordinal);
+        Assert.DoesNotContain("| Tool | Purpose | Key Parameters |", text, StringComparison.Ordinal);
+    }
+}