Give a PostgreSQL target PostgreSQL tabs in the WPF viewer (#2530)

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
+- **The WPF viewer renders PostgreSQL tabs at a PostgreSQL target, and stops rendering nineteen empty SQL Server ones** ([#2530]) - `git grep IsPostgres` across `PerformanceMonitor.Darling.Viewer` returned **nothing**, so an Aurora target opened on the SQL Server correlated-lane Overview and got the whole nineteen-tab strip - tempdb, Trace Flags, Query Store, Plan Viewer, Always On - nearly all of it permanently empty and several tabs meaningless for the engine. It now gets **six**: Overview, Activity, Vacuum, Waits, I/O and Replication, the same six the web dashboard chose in [#2547], with the same ids and the same grouping so the two front ends do not teach one engine two shapes. All **nine** PostgreSQL collectors land on exactly one tab, and that placement is DERIVED from `CollectorCatalog` in both directions - a tenth collector turns the pin red naming itself, which is the check that did not exist while eight of them shipped MCP-only for three releases. The `ViewerCollectorCoverageTests` allow-list, which carried all nine as tracked debt, is now **empty**.
+- **The two Aurora-only PostgreSQL panels are SHOWN on stock PostgreSQL, not hidden** ([#2530]) - `pg_wait_stats` and `pg_statement_stats` both gate on `IsAurora`, so on a stock target they can never have content. Each panel prints `CollectorEngineCapability.NotCollectedMessage` instead - the same sentence the MCP surface and the web dashboard print, naming the server, the engine, the collector and the exact `aurora_stat_*` surface, ending "and never will". The defect being closed is UNEXPLAINED emptiness, not emptiness; hiding them would also make the tab strip a different shape on two PostgreSQL servers in one fleet. The Overview tab is built from the CATALOG rather than from `collection_log` for the same reason: a gated-off collector writes no log row at all, so a log-driven grid would have dropped precisely the row an operator most needs explained.
+- **One copy of the PostgreSQL read SQL, shared by the MCP surface and the desktop viewer** ([#2530]) - the nine `DarlingPg*Reader` classes moved from the service's MCP folder into `PerformanceMonitor.Darling.Storage`, which both front ends reference. Copying them into the viewer would have meant a second copy of, among others, a 200-line recursive blocking walk whose revisit guard, root attribution and truncation flag were each a separate review finding - and the copy that drifts is never the one being read. No query text changed; every existing reader test still pins the same constants.
+- **The viewer's server rows carry the engine discriminator, from BOTH server reads** ([#2530]) - `DarlingServer` gained `EngineKind`, `EngineEdition`, `IsPostgres` and `EngineDescription`, and both `ServersSql` and `ManagedServersSql` select `engine_kind` and `sql_engine_edition`. Both, because the sidebar uses `ManagedServersSql` on any seeded store - i.e. every real deployment - so a column added to only the other one would have left every PostgreSQL target on the SQL Server tab set while a unit test passed. Only a POSITIVE claim switches: a null or unrecognised token keeps the SQL Server tabs and shows NO engine badge, because the tabs such a server gets are a default rather than a finding.
 - **PostgreSQL temp-file spills, cache hit ratio, deadlocks and the rollback ratio - one collector, one read** ([#2539]) - nothing touched `pg_stat_database`, so four separate questions had no answer on a PostgreSQL target. The one that mattered: **temp files are work `work_mem` could not hold**, which is the most common reason a PostgreSQL query is slow for a cause its plan shape does not show - and a spilling query looks identical to an expensive one in the statement statistics. `pg_statement_stats` does carry per-query `temp_blks_written`, but it reads `aurora_stat_statements()` and is **Aurora only**, so on stock PostgreSQL there was no temp-file evidence anywhere; on Aurora it is a top-N over statements `pg_stat_statements` still retains, and this is the ground truth those are a share of. Schema **v83** adds `collect.pg_database_stats`, the new `pg_database_stats` collector reads the view per minute, and `get_pg_database_stats` differences it across a window. Every major and every target shape: the newest column selected arrived in PostgreSQL 9.2 (verified live on 13, 16 and 17), it is core rather than Aurora, and a standby is included deliberately because a sort on a read replica spills exactly the way it does on a writer.
 - **A PostgreSQL statistics reset is reported AS a reset** ([#2539]) - `pg_stat_database`'s counters are cumulative since the last `pg_stat_reset()`, so a reset silently zeroes them, and the two obvious ways to difference them are both wrong: last-minus-first goes **negative**, max-minus-min **spikes** to the pre-reset lifetime total. Both look like measurements. The read clamps per interval so neither can happen, and then says so: `stats_reset_count` from the server's own per-database `stats_reset` moving, `counter_rewind_count` from a counter falling below its predecessor, and a per-database note that the window totals are LOWER BOUNDS. Both signals are carried because neither sees every reset - a reset followed by enough activity to climb back past the old value inside one collection interval leaves every difference positive, and only the timestamp knows. The sharpest case is a database's FIRST-EVER reset, where `stats_reset` moves NULL -> timestamp: the first sample of a series is excluded by its ROW_NUMBER rather than by its `LAG` being NULL, because `LAG(stats_reset)` is NULL both when there is no previous row AND when the previous row had never been reset - the ordinary state - so a guard on it would miss the one reset nothing else can see. Rows are stored **per database** for the same reason: `stats_reset` is per database, so an aggregate would have to pick one timestamp for rows that legitimately disagree, and a single database's reset would corrupt the cluster's delta with nothing left in the data to say so.
 - **The web dashboard renders PostgreSQL tabs at a PostgreSQL target, and stops rendering twelve empty SQL Server ones** ([#2530]) - the per-server page's `SERVER_TABS` was a FLAT list applied to every server, so an Aurora instance got Wait Stats, CPU, Memory, Blocking, File I/O, Queries, Configuration, Config Changes, tempdb and the rest - nearly all permanently empty, several meaningless for the engine - while the eight `get_pg_*` reads that DO have data were reachable only through MCP. The registry is now two registries, and `serverTabsFor(card)` chooses from the fleet card's server-derived `is_postgres`. A PostgreSQL target gets **six** tabs: **Overview** (freeze headroom, xmin horizon, autovacuum backlog and replication-slot vitals over findings and collection health), **Activity** (`get_pg_blocking` with its capture denominator, top query shapes, and [#2539]'s per-database temp-file spills directly beneath them, because a spill is the explanation a statement's own stats cannot give), **Vacuum**, **Waits**, **I/O** and **Replication**. Six against twelve is the design and not a shortfall: parity was explicitly not the constraint, because bloat, wraparound, the xmin horizon and autovacuum have no SQL Server analogue and are what actually pages a PostgreSQL DBA. **Vacuum is one tab on purpose** - an old xmin horizon starves vacuum, starved vacuum falls behind on freezing, freezing falling behind ends in wraparound; read separately each looks survivable and together they are one escalating story, so the three reads sit in that causal order under a note that says why. **Waits is SHOWN on stock PostgreSQL** even though `aurora_stat_system_waits()` means it can never fill there: [#2532] taught that read to answer `not_collected` naming the server, the engine, the collector and the exact Aurora surface and to say the gap is permanent, so the panel explains itself - and hiding it would have made the tab set change shape between two PostgreSQL servers in one fleet and made the one Aurora-only capability invisible from a stock instance. A **NULL** `engine_kind` keeps the SQL Server set, unchanged: only a positive PostgreSQL claim switches, because absence is not evidence for either engine. The bar and the grid wait for the ONE `/api/fleet` read the page already made rather than flashing the wrong twelve tabs and correcting them. The card also gained `engine_description` (the engine's name in words, from `MonitoredEngineKind`), so the header can badge the engine without any surface owning a second copy of the vocabulary - and `vizStat` gained the `emptyText` guard `vizLine` already had, because several PostgreSQL reads answer their HEALTHY case with a data body carrying prose and none of the summary keys, which used to render as a row of em-dashes.

Darling/Darling.Tests/DarlingPgAutovacuumReaderTests.cs

@@ -10,6 +10,7 @@
 using System.Linq;
 using System.Text.RegularExpressions;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgBlockingReaderOrdinalTests.cs

@@ -13,6 +13,7 @@
 using System.Linq;
 using System.Text.RegularExpressions;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgBlockingReaderTests.cs

@@ -10,6 +10,7 @@
 using System.Linq;
 using System.Text.RegularExpressions;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgDatabaseReaderTests.cs

@@ -9,6 +9,7 @@
 using System;
 using System.Linq;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgIoReaderTests.cs

@@ -9,6 +9,7 @@
 using System;
 using System.Linq;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgReadSqlParsesLiveTests.cs

@@ -56,9 +56,15 @@ public sealed class DarlingPgReadSqlParsesLiveTests
     /// rather than the reader methods because a method would need a connection, a server and a window; the
     /// constant IS the shipped text, which is the thing under test.
     /// </summary>
+    /// <para>The namespace comes from a reader TYPE rather than a string literal (#2530), for the reason
+    /// the discovery is reflective in the first place: the readers moved to
+    /// <c>PerformanceMonitor.Darling.Storage</c> so the WPF viewer could run the same query text, and a
+    /// hardcoded namespace matched nothing afterwards. The anti-vacuity floor below caught it — a guard that
+    /// stopped guarding, reported as one — but a filter anchored to a type it must find anyway cannot break
+    /// that way twice.</para>
     private static IReadOnlyList<(string Name, string Sql)> ShippedReadSql() =>
         typeof(DarlingPgStatementReader).Assembly.GetTypes()
-            .Where(t => t.Namespace == "PerformanceMonitor.Darling.Service.Mcp"
+            .Where(t => t.Namespace == typeof(DarlingPgStatementReader).Namespace
                         && t.Name.StartsWith("DarlingPg", StringComparison.Ordinal)
                         && t.Name.EndsWith("Reader", StringComparison.Ordinal))
             .SelectMany(t => t

Darling/Darling.Tests/DarlingPgSlotReaderTests.cs

@@ -9,6 +9,7 @@
 using System;
 using System.Linq;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgStatementReaderTests.cs

@@ -8,6 +8,7 @@
 
 using System;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgWaitReaderTests.cs

@@ -8,6 +8,7 @@
 
 using System;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgWraparoundReaderTests.cs

@@ -8,6 +8,7 @@
 
 using System;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/DarlingPgXminReaderTests.cs

@@ -9,6 +9,7 @@
 using System;
 using System.Linq;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/PgInt64IdentityWireShapeTests.cs

@@ -12,6 +12,7 @@
 using System.Linq;
 using System.Text.Json;
 using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
 using Xunit;
 
 namespace Darling.Tests;

Darling/Darling.Tests/ViewerCollectorCoverageTests.cs

@@ -11,6 +11,7 @@
 using System.IO;
 using System.Linq;
 using System.Runtime.CompilerServices;
+using System.Text.RegularExpressions;
 using PerformanceMonitor.Collectors;
 using Xunit;
 
@@ -20,7 +21,8 @@ namespace Darling.Tests;
 /// THE COVERAGE PIN (parity board Tier 0), Darling side — the mirror of Lite's
 /// <c>CollectorViewerCoverageTests</c>. Every collector table in <see cref="CollectorCatalog"/> must have
 /// a consumer in the Darling viewer's reader layer
-/// (<c>PerformanceMonitor.Darling.Viewer/ViewerDataService*.cs</c>) — referenced directly by table name or
+/// (<c>PerformanceMonitor.Darling.Viewer/ViewerDataService*.cs</c>, plus the shared store readers those
+/// files CALL — see <see cref="ReaderLayerText"/>) — referenced directly by table name or
 /// via its <c>v_</c> view — OR be listed in <see cref="KnownStoreOnlyOrUnbuiltTables"/>.
 ///
 /// Darling currently reads EVERY catalog table, so the allow-list is empty and this pin is fully green.
@@ -48,31 +50,9 @@ public sealed class ViewerCollectorCoverageTests
         // database_states is read by ViewerDataService.DatabaseStates.cs (the override editor's backing
         // store), so it is covered by the reader-layer scan and needs no allow-list entry.
 
-        // UNBUILT UI (parity board Tier 1) -- remove each when the PostgreSQL tab ships.
-        // The nine PostgreSQL collector tables are read through MCP today, not the WPF viewer: each has a
-        // reader class + MCP tool (get_pg_wait_stats, get_pg_top_queries, get_pg_wraparound_risk,
-        // get_pg_xmin_horizon, get_pg_replication_slots, get_pg_autovacuum_health, get_pg_io_stats,
-        // get_pg_blocking, get_pg_database_stats) and is exposed over /api/read, so the data is not
-        // invisible — it is just not on a
-        // WPF tab, because the viewer's surfaces are SQL-Server-shaped and a PostgreSQL target's signals do
-        // not slot into them. Deliberately listed one per line so removing one at a time is a one-line diff.
-        "pg_wait_stats",
-        "pg_statement_stats",
-        "pg_wraparound_stats",
-        "pg_xmin_horizon",
-        "pg_replication_slot_stats",
-        "pg_autovacuum_stats",
-        "pg_io_stats",
-        // pg_blocking_edges is the one with a genuine SQL Server counterpart on the Blocking tab, and it
-        // still cannot share it: that tab is built on blocked_process_report XML (an engine-recorded event
-        // with a deadlock graph and a threshold), while this table holds periodic samples of an edge list.
-        // Rendering samples through a surface labelled as an event log would misrepresent coverage, which is
-        // a worse outcome than the tab not existing yet.
-        "pg_blocking_edges",
-        // pg_database_stats (#2539) is read by get_pg_database_stats over /api/read, not by a WPF tab, for
-        // the same reason as the seven above: the viewer's surfaces are SQL-Server-shaped and there is no
-        // tab whose meaning survives being pointed at a PostgreSQL target.
-        "pg_database_stats",
+        // EMPTY, and it stays empty. The nine PostgreSQL collector tables were the last entries here,
+        // carrying "remove each when the PostgreSQL tab ships" — and #2530 shipped exactly those tabs, so
+        // all nine came off in one commit. Every table in the catalog now has a Darling viewer reader.
     };
 
     [Fact]
@@ -143,10 +123,27 @@ public void AllowList_HasNoEntryThatIsActuallyRead()
     private static bool ReferencedIn(string readerText, string table) =>
         readerText.Contains(table, StringComparison.OrdinalIgnoreCase);
 
-    /// <summary>Concatenated text of the Darling viewer's reader layer
+    /// <summary>
+    /// Concatenated text of the Darling viewer's reader layer
     /// (<c>PerformanceMonitor.Darling.Viewer/ViewerDataService*.cs</c>), located from this test file's
     /// compile-time path (Darling.Tests sits at <c>Darling/Darling.Tests/</c>; the viewer is the sibling
-    /// <c>Darling/PerformanceMonitor.Darling.Viewer/</c>).</summary>
+    /// <c>Darling/PerformanceMonitor.Darling.Viewer/</c>) — PLUS the shared store readers those files name.
+    ///
+    /// <para><b>Why the second half exists (#2530).</b> The nine PostgreSQL reads run the SAME query text on
+    /// the MCP surface and on the WPF tabs, from <c>DarlingPg</c>-prefixed readers in
+    /// <c>PerformanceMonitor.Darling.Storage</c>. A scan that looked only at <c>ViewerDataService*.cs</c>
+    /// would have demanded a SECOND copy of that SQL — including a 200-line recursive blocking walk whose
+    /// revisit guard, root attribution and truncation flag were each a separate review finding — purely to
+    /// satisfy a text match. Duplicating load-bearing SQL so a coverage pin passes is the opposite of what
+    /// the pin is for.</para>
+    ///
+    /// <para><b>It is still DERIVED, and still viewer-shaped.</b> A shared reader is followed only when the
+    /// viewer's own source NAMES its class — one hop, outwards from the viewer, never the reverse, so a
+    /// shared reader naming another cannot drag in coverage the viewer never asked for. Stop calling a
+    /// reader from the viewer and its table loses coverage immediately, which is exactly the failure this
+    /// pin exists to catch; proved by deleting one call and watching that table appear in the uncovered
+    /// list.</para>
+    /// </summary>
     private static string ReaderLayerText([CallerFilePath] string thisFile = "")
     {
         var testDir = Path.GetDirectoryName(thisFile)!;
@@ -167,7 +164,40 @@ private static string ReaderLayerText([CallerFilePath] string thisFile = "")
             $"No ViewerDataService*.cs files found under {readerDir} — the coverage pin cannot read the " +
             "Darling viewer's reader layer (did the reader layer move?).");
 
-        return string.Join("\n", files.Select(f => StripSchemaProbeLines(File.ReadAllText(f))));
+        var viewerText = string.Join("\n", files.Select(f => StripSchemaProbeLines(File.ReadAllText(f))));
+
+        return viewerText + "\n" + SharedReadersNamedBy(viewerText, testDir);
+    }
+
+    /// <summary>
+    /// The source of every shared store reader the viewer's own reader layer names, concatenated.
+    /// <para>A name that resolves to no file under <c>PerformanceMonitor.Darling.Storage</c> is skipped
+    /// rather than asserted on: the viewer legitimately mentions reader classes it cannot reference (the
+    /// service's own, in prose), and a missing file grants no coverage — which is the safe direction to
+    /// fail in.</para>
+    /// </summary>
+    private static string SharedReadersNamedBy(string viewerText, string testDir)
+    {
+        var sharedDir = Path.GetFullPath(Path.Combine(testDir, "..", "PerformanceMonitor.Darling.Storage"));
+
+        var named = Regex
+            .Matches(viewerText, @"\bDarling[A-Za-z0-9]*Reader\b")
+            .Select(m => m.Value)
+            .Distinct(StringComparer.Ordinal)
+            .OrderBy(n => n, StringComparer.Ordinal)
+            .ToList();
+
+        var followed = new List<string>();
+        foreach (var name in named)
+        {
+            var path = Path.Combine(sharedDir, name + ".cs");
+            if (File.Exists(path) && !HasBuildOutputSegment(path))
+            {
+                followed.Add(StripSchemaProbeLines(File.ReadAllText(path)));
+            }
+        }
+
+        return string.Join("\n", followed);
     }
 
     /// <summary>