Stop pretty-printing MCP tool results (#2350)

CHANGELOG.md

@@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - **Declared peer stores: a Darling MCP server that names its siblings instead of answering "unknown server"** ([#2339]) - tier 1 of the multi-store fix, disclosure only. With the fleet split across several boxes (one store each: SQL Server primaries on one, their readable replicas on another, PostgreSQL on a third) every box's MCP server answered over ITS store alone, so a server monitored by a sibling resolved as not-found - indistinguishable from a server nobody monitors, which with a deliberately-split fleet is now the normal case rather than an edge. An optional `peers` block in darling.json (`thisStoreCovers`, plus per-peer `name` / `covers` / optional `matches` name-substrings) is disclosed at the three places an agent forms its picture of the fleet: the MCP instructions gain a Fleet Coverage section above the tool census, `list_servers` gains `this_store_covers` + `peer_fleets` + a `peer_note` (its empty-registry answer is prose rather than JSON, and carries the peer list too - a store with nothing registered is a fresh or just-restarted box, the worst place to drop the disclosure), and the server-resolution miss appends "not monitored HERE - matches the declared coverage of <peer>" to the existing available-servers listing. There is NO credential, NO address and NO connectivity behind it: the service never contacts a peer and says so in every message, and the publish itself REFUSES a peers block whose text looks like a connection string or credential, since all of it is sent verbatim to every connected MCP client. That guard lives in the publish rather than only in config validation because the MCP host loads its own config and deliberately never validates it (its fail-closed checks are host-local), so validation alone would have left the one path that actually broadcasts uncovered; a refusal publishes nothing at all rather than the valid subset. An empty `peer_fleets` carries its own note - "this may be the only store, or nobody declared the siblings, and this server cannot tell those apart" - so absence never reads as "you are looking at the whole fleet". With nothing declared the PROSE surfaces are byte-for-byte unchanged (the instructions, the resolution miss, and the empty-registry sentence), with ONE deliberate exception: `list_servers`' JSON envelope carries `this_store_covers` / `peer_fleets` / `peer_note` on every response, declared or not, so a client comparing that tool's exact shape sees three new keys on upgrade. That is the point rather than an oversight - an empty `peer_fleets` means either "only store" or "nobody declared the siblings", and a conditional note would say nothing in exactly the case that produces the wrong conclusion. Lite has no peers concept and gets no twin. Federated cross-store reads stay unbuilt on purpose
 
+### Changed
+- **MCP tool results serialize compact instead of pretty-printed** ([#2350]) - the only consumer of a tool result is a language model, and indentation buys a model nothing. One property on the shared `McpHelpers.JsonOptions` in Common, so both SKUs move together, plus the two readers that carry their own options for the `/api/*` twins. Saving is payload-shaped - 23% of the bytes on a 15-field record array, 36% on a narrow one - and the TOKEN saving is smaller than the byte saving, because BPE tokenizers pack runs of spaces efficiently. The config files people hand-edit (`servers.json`, profiles, schedules) keep indenting, and a test pins that boundary in both directions.
+
 ### Fixed
 - **Extended-length paths no longer slip past the install-location guards** ([#2348]) - `\\?\UNC\server\share` is the long spelling of a REAL share and `\\?\C:\Users\bob` of a REAL profile, but the wholesale `\\?\` exclusion waved both through undiagnosed, because skipping a check is not the same as passing it. Both implementations now strip the prefix BEFORE classifying, so the long spelling gets the same verdict as the short one, and `\\?\C:\PerformanceMonitorDarling` - an ordinary local root written the long way - is still correctly left alone. The shared decision table and the cross-language parity test hold the service and `install-darling.ps1` to it together.
 - **The store's scale test no longer asserts that TimescaleDB compresses more rows in more time** ([#2266] item 1, measured on a rig) - `ScaleTest_JobDurationGrowsWithVolume_...` required `d10 > d1` between two sub-second job durations, and it has failed on PR after PR whose diffs cannot reach it (`d1=970/d10=863`, then `d1=689/d10=689`). Fifteen consecutive runs of the exact sequence against TimescaleDB 2.29/PG17 settle what no amount of reasoning from CI logs could: the chunks compress perfectly (counts go 1, 2, 3; per-day rows are exactly 2000/50000/500000 every single time), so the earlier suspicion that both runs were compressing nothing is **refuted** - but a 10x volume increase buys only ~3.2x the duration, about **85 ms** of absolute signal, because compression cost is largely fixed per run. CI's baseline for the same pair is 690-970 ms, roughly twenty times that fixed cost, so the volume-dependent component there is ~10% of the measurement's own magnitude and sits inside the run-to-run variance of launching a background worker on Windows. That is a benchmark of somebody else's compression engine on shared hardware, and no threshold, ratio or volume rescues it: at ~0.19 ms per thousand rows it would take millions of rows per chunk to clear a variance nobody has measured on the platform that actually fails. The byte-identical pair was never as improbable as it looked either, because that pair is only ever read when the test FAILS, which selects for differences already near zero. **It is replaced by something strictly stronger, not weaker**: each measured run must have compressed the chunk its own seed created, and that chunk must hold exactly the seeded row count - exact counts instead of two timings. A negative control proves the difference rather than assuming it. Seed the 10x rows into a chunk that is not yet compression-eligible and the old assertion fails and the new ones fail too, naming `compressed=2`; but seed them into the **1x chunk** and the old assertion **passes 3/3 with a 6-8x ratio** while the fixture has quietly stopped producing two chunks at two volumes, and only the new assertions catch it (`rows=[2000,550000]`, `total=2`). So the shipped assertion was not merely flaky, it was blind to the fixture defect it was supposed to be guarding. What the product owns is asserted and unchanged: a real duration is measured, the V56 series records both readings in order, and the real evaluator fires the [#2136] cadence alert from a real reading. One gap closed on the way past - `d10 > 0` was never asserted, and `ReadJobDurationMsAsync` maps a NULL duration to 0, so a 10x run whose duration was unmeasurable satisfied the telemetry check as `0 == 0` and passed. The test is renamed to stop claiming what it no longer measures.
@@ -2813,6 +2816,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#2340]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2340
 [#2344]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2344
 [#2348]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2348
+[#2350]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2350
 [#2331]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2331
 [#2181]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2181
 [#2317]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2317

Darling/Darling.Tests/DarlingAgReaderTests.cs

@@ -388,8 +388,8 @@ public void SerializedShape_CarriesTheFieldsBothConsumersRead()
 
         /* Severities serialize as NAMES (the JsonStringEnumConverter), not integers — the browser maps the name to
            a CSS class, so a numeric enum would silently break every color. */
-        Assert.Contains("\"severity\": \"Critical\"", json, StringComparison.Ordinal);
-        Assert.DoesNotContain("\"severity\": 3", json, StringComparison.Ordinal);
+        JsonAssert.Contains("\"severity\": \"Critical\"", json);
+        JsonAssert.DoesNotContain("\"severity\": 3", json);
     }
 
     /* ─────────────────────────── SQL dialect pins ─────────────────────────── */

Darling/Darling.Tests/DarlingFleetReaderTests.cs

@@ -236,13 +236,13 @@ public void FleetServerCard_SerializesSnakeCase_WithStringBands()
         }
 
         /* Bands / severities serialize as strings, not ordinals — the frontend maps a name to a color. */
-        Assert.Contains("\"band\": \"Critical\"", json, StringComparison.Ordinal);
-        Assert.Contains("\"cpu_severity\": \"Critical\"", json, StringComparison.Ordinal);
-        Assert.Contains("\"threads_severity\": \"Unknown\"", json, StringComparison.Ordinal);
+        JsonAssert.Contains("\"band\": \"Critical\"", json);
+        JsonAssert.Contains("\"cpu_severity\": \"Critical\"", json);
+        JsonAssert.Contains("\"threads_severity\": \"Unknown\"", json);
         /* naive-UTC instants carry no zone suffix (localized in the browser). */
-        Assert.Contains("\"last_collection\": \"2026-07-18T03:30:00\"", json, StringComparison.Ordinal);
-        Assert.Contains("\"deadlock_last_seen\": \"2026-07-18T03:15:00\"", json, StringComparison.Ordinal);
-        Assert.DoesNotContain("\"cpu_severity\": 3", json, StringComparison.Ordinal);
+        JsonAssert.Contains("\"last_collection\": \"2026-07-18T03:30:00\"", json);
+        JsonAssert.Contains("\"deadlock_last_seen\": \"2026-07-18T03:15:00\"", json);
+        JsonAssert.DoesNotContain("\"cpu_severity\": 3", json);
     }
 
     [Fact]
@@ -252,17 +252,17 @@ public void FleetServerCard_SerializesPerServerPlatform_ForComposerD4Greying()
            measure's appliesTo.azureSqlDb to auto-grey a measure that platform can't collect. */
         var azure = new FleetServerCard { ServerId = 1, DisplayName = "az-db", ServerName = "az-db", EngineEdition = 5, IsAzureSqlDb = true };
         var azureJson = JsonSerializer.Serialize(azure, DarlingFleetReader.JsonOptions);
-        Assert.Contains("\"engine_edition\": 5", azureJson, StringComparison.Ordinal);
-        Assert.Contains("\"is_azure_sql_db\": true", azureJson, StringComparison.Ordinal);
-        Assert.Contains("\"is_azure_mi\": false", azureJson, StringComparison.Ordinal);
+        JsonAssert.Contains("\"engine_edition\": 5", azureJson);
+        JsonAssert.Contains("\"is_azure_sql_db\": true", azureJson);
+        JsonAssert.Contains("\"is_azure_mi\": false", azureJson);
 
         /* A server that has not connected: null edition serializes as JSON null and both flags are false, so the
            frontend has no signal and keeps the measure badge rather than greying on a guess. */
         var unknown = new FleetServerCard { ServerId = 2, DisplayName = "new", ServerName = "new" };
         var unknownJson = JsonSerializer.Serialize(unknown, DarlingFleetReader.JsonOptions);
-        Assert.Contains("\"engine_edition\": null", unknownJson, StringComparison.Ordinal);
-        Assert.Contains("\"is_azure_sql_db\": false", unknownJson, StringComparison.Ordinal);
-        Assert.Contains("\"is_azure_mi\": false", unknownJson, StringComparison.Ordinal);
+        JsonAssert.Contains("\"engine_edition\": null", unknownJson);
+        JsonAssert.Contains("\"is_azure_sql_db\": false", unknownJson);
+        JsonAssert.Contains("\"is_azure_mi\": false", unknownJson);
     }
 
     [Fact]
@@ -306,7 +306,7 @@ public void FleetOverviewResult_SerializesRollupShape()
             Assert.Contains(field, json, StringComparison.Ordinal);
         }
 
-        Assert.Contains("\"band_label\": \"Critical\"", json, StringComparison.Ordinal);
+        JsonAssert.Contains("\"band_label\": \"Critical\"", json);
     }
 }
 

Darling/Darling.Tests/DarlingMcpConfigHistoryToolsTests.cs

@@ -395,9 +395,9 @@ await DarlingMcpTestData.ExecAsync(connection, ct,
 
             var qsh = await DarlingMcpConfigHistoryTools.GetQueryStoreHealth(postgres, ServerName);
             DarlingMcpTestData.AssertEnvelope(qsh, ServerName, "databases");
-            Assert.Contains("\"state_matches_desired\": false", qsh, StringComparison.Ordinal);
+            JsonAssert.Contains("\"state_matches_desired\": false", qsh);
             Assert.Contains("storage cap reached", qsh, StringComparison.Ordinal);
-            Assert.Contains("\"pct_of_cap\": 100", qsh, StringComparison.Ordinal);
+            JsonAssert.Contains("\"pct_of_cap\": 100", qsh);
 
             bodySucceeded = true;
         }

Darling/Darling.Tests/DarlingMcpDefaultTraceToolsTests.cs

@@ -210,7 +210,7 @@ await DarlingMcpTestData.ExecAsync(connection, ct,
             Assert.Contains("AutoGrowShrink", json, StringComparison.Ordinal);
             Assert.Contains("SEVERE_MARKER", json, StringComparison.Ordinal);
             Assert.DoesNotContain("ROUTINE_MARKER", json, StringComparison.Ordinal);
-            Assert.Contains("\"total_events\": 2", json, StringComparison.Ordinal);
+            JsonAssert.Contains("\"total_events\": 2", json);
 
             /* Unknown server → the listing error; empty store → the miss. */
             Assert.StartsWith("Could not resolve server.", await DarlingMcpDefaultTraceTools.GetDefaultTraceEvents(postgres, "darling-no-such-server"), StringComparison.Ordinal);

Darling/Darling.Tests/JsonAssert.cs

@@ -0,0 +1,100 @@
+/*
+ * 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 System.Text;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// Substring assertions over serialized JSON that ignore LAYOUT (#2350).
+///
+/// <para>A test written as <c>Assert.Contains("\"severity\": \"Critical\"", json)</c> reads as a claim about
+/// content — this field serialized with this value — but is actually a claim about formatting, because the space
+/// after the colon exists only under <c>WriteIndented</c>. When MCP tool results went compact, eighteen such
+/// assertions failed across four files without a single one of the things they were testing having changed.</para>
+///
+/// <para>These helpers normalize both sides by dropping whitespace that sits BETWEEN tokens while preserving
+/// whitespace INSIDE strings, so <c>"a": "b c"</c> and <c>"a":"b c"</c> compare equal and the two-space value in
+/// <c>"b c"</c> survives. The assertion then means what it always looked like it meant.</para>
+///
+/// <para>Deliberately not a full JSON parse: these are substring assertions on purpose — they check a field
+/// serialized a particular way (an enum as its string name rather than its ordinal, a null that stayed null)
+/// without pinning the shape of the whole envelope around it.</para>
+/// </summary>
+internal static class JsonAssert
+{
+    /// <summary>xUnit's argument order (expected first) so call sites read the same as the assertion they replace.</summary>
+    internal static void Contains(string expectedFragment, string json)
+    {
+        Assert.Contains(StripInsignificantWhitespace(expectedFragment), StripInsignificantWhitespace(json), StringComparison.Ordinal);
+    }
+
+    /// <inheritdoc cref="Contains"/>
+    internal static void DoesNotContain(string unexpectedFragment, string json)
+    {
+        Assert.DoesNotContain(StripInsignificantWhitespace(unexpectedFragment), StripInsignificantWhitespace(json), StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// Removes whitespace outside string literals. Tracks escaping so a <c>\"</c> inside a string does not end it
+    /// and a <c>\\</c> before a quote does not escape it — get that wrong and the parser falls out of the string,
+    /// starts stripping real spaces from values, and the assertion silently starts comparing something else.
+    /// </summary>
+    internal static string StripInsignificantWhitespace(string json)
+    {
+        if (string.IsNullOrEmpty(json))
+        {
+            return json ?? string.Empty;
+        }
+
+        var builder = new StringBuilder(json.Length);
+        var inString = false;
+        var escaped = false;
+
+        foreach (var c in json)
+        {
+            if (inString)
+            {
+                builder.Append(c);
+
+                if (escaped)
+                {
+                    escaped = false;
+                }
+                else if (c == '\\')
+                {
+                    escaped = true;
+                }
+                else if (c == '"')
+                {
+                    inString = false;
+                }
+
+                continue;
+            }
+
+            if (c == '"')
+            {
+                inString = true;
+                builder.Append(c);
+                continue;
+            }
+
+            if (c is ' ' or '\t' or '\r' or '\n')
+            {
+                continue;
+            }
+
+            builder.Append(c);
+        }
+
+        return builder.ToString();
+    }
+}

Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingAgReader.cs

@@ -57,12 +57,12 @@ namespace PerformanceMonitor.Darling.Service.Mcp;
 internal static class DarlingAgReader
 {
     /// <summary>Shared serializer options — snake_case field names come from the DTOs' <c>[JsonPropertyName]</c>
-    /// attributes, severities serialize as their string names, and the output is indented (the MCP tool
-    /// convention). ONE options object so <c>/api/ag</c> and <c>get_ag_health</c> serialize the identical
-    /// shape.</summary>
+    /// attributes, severities serialize as their string names, and the output is COMPACT (#2350 - the MCP tool
+    /// convention, since the reader on both ends is a parser rather than a person). ONE options object so
+    /// <c>/api/ag</c> and <c>get_ag_health</c> serialize the identical shape.</summary>
     public static readonly JsonSerializerOptions JsonOptions = new()
     {
-        WriteIndented = true,
+        WriteIndented = false,
         Converters = { new JsonStringEnumConverter() },
     };
 

Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingFleetReader.cs

@@ -43,11 +43,11 @@ internal static class DarlingFleetReader
 {
     /// <summary>Shared serializer options for the fleet DTOs — snake_case field names come from the DTOs'
     /// <c>[JsonPropertyName]</c> attributes, enum bands serialize as their string names, and the output is
-    /// indented (matching the MCP tool convention). ONE options object so the web endpoint and the MCP tool
-    /// serialize the identical shape.</summary>
+    /// COMPACT (#2350, matching the MCP tool convention). ONE options object so the web endpoint and the MCP
+    /// tool serialize the identical shape.</summary>
     public static readonly JsonSerializerOptions JsonOptions = new()
     {
-        WriteIndented = true,
+        WriteIndented = false,
         Converters = { new JsonStringEnumConverter() },
     };