From a7b36381c434c6a32d26e1be6137a17857a97597 Mon Sep 17 00:00:00 2001 From: mtinti Date: Thu, 25 Jun 2026 19:07:08 +0100 Subject: [PATCH 1/4] Add cohort health board breakdown commands (final-list + build tree) Two related commands that break an RDMP cohort down by Scottish health board, using a demography catalogue's region cipher column (e.g. SHARE_Demography.Region), plus a shared Region -> board/node lookup. - HealthBoardLookup: hardcoded Region cipher -> health board (name, HB code) and safe-haven node; unmapped/NULL regions resolve to an Unknown node so counts are never silently dropped. - ExecuteCommandExportCohortHealthBoardBreakdown (+ CIC overload): per-board COUNT(DISTINCT chi) of a cohort's final inclusion list, joined to the demography catalogue, with per-node subtotals, an Unknown bucket and a grand total. Works from a committed ExtractableCohort or a CohortIdentificationConfiguration. - ExecuteCommandExportCohortBuildHealthBoardBreakdown: reproduces the Cohort Builder's per-set/per-container FinalCount and cumulative running totals split by health board (long-format CSV). Operates purely on the query cache: builds once to populate the per-set cache tables, then recomposes each count point from those tables and splits it by region (one GROUP BY per node, all boards at once) - no per-board rebuild, no source-server hits after the single build. Requires a query caching server and the demography catalogue on the cache server. - Tests: no-DB unit tests for the lookups/reports/SQL plus database-backed end-to-end tests (committed cohort, CIC, and a deterministic build-tree fixture asserting the national + per-board cumulative tree reconciles at every node). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_0169JCnaL3fhhZjseDx2XXT2 --- .../CohortBuildHealthBoardBreakdownTests.cs | 249 ++++++++++++++ .../HealthBoardBreakdownReportTests.cs | 304 +++++++++++++++++ .../CohortBuildHealthBoardBreakdownReport.cs | 176 ++++++++++ .../HealthBoardBreakdownReport.cs | 169 +++++++++ Rdmp.Core/CohortCreation/HealthBoardLookup.cs | 61 ++++ ...ndExportCohortBuildHealthBoardBreakdown.cs | 320 ++++++++++++++++++ ...CommandExportCohortHealthBoardBreakdown.cs | 252 ++++++++++++++ 7 files changed, 1531 insertions(+) create mode 100644 Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs create mode 100644 Rdmp.Core.Tests/CohortCreation/HealthBoardBreakdownReportTests.cs create mode 100644 Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs create mode 100644 Rdmp.Core/CohortCreation/HealthBoardBreakdownReport.cs create mode 100644 Rdmp.Core/CohortCreation/HealthBoardLookup.cs create mode 100644 Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs create mode 100644 Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortHealthBoardBreakdown.cs diff --git a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs new file mode 100644 index 0000000000..1138c77e8d --- /dev/null +++ b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs @@ -0,0 +1,249 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System; +using System.Collections.Generic; +using System.Data; +using System.IO; +using System.Linq; +using FAnsi; +using FAnsi.Discovery; +using NUnit.Framework; +using Rdmp.Core.CohortCreation; +using Rdmp.Core.CommandExecution; +using Rdmp.Core.CommandExecution.AtomicCommands; +using Rdmp.Core.Curation.Data; +using Rdmp.Core.Curation.Data.Aggregation; +using Rdmp.Core.Curation.Data.Cohort; +using Rdmp.Core.Databases; +using Rdmp.Core.MapsDirectlyToDatabaseTable.Versioning; +using Rdmp.Core.ReusableLibraryCode.Checks; +using Tests.Common; +using Tests.Common.Scenarios; + +namespace Rdmp.Core.Tests.CohortCreation; + +/// +/// End-to-end validation of the per-health-board build breakdown on the deterministic fixture in +/// proposals/cohort-healthboard-breakdown/BUILD-BREAKDOWN-TEST-FIXTURE.md: a top EXCEPT over an +/// inclusion INTERSECT (Registry 120 ∩ Demography 100 = 100) minus four exclusion sets, with the +/// cohort partitioned across 3 boards (1 patient ↔ 1 board). Requires the test SQL Server + a query +/// cache (see mac-test-env). +/// +public class CohortBuildHealthBoardBreakdownTests : FromToDatabaseTests +{ + private static IEnumerable Ids(int from, int to) => + Enumerable.Range(from, to - from + 1).Select(i => $"P{i:000}"); + + [Test] + public void BuildBreakdown_Fixture_NationalAndThreeBoards() + { + var db = GetCleanedServer(DatabaseType.MicrosoftSQLServer); + + // ---- query cache ---- + var cacheDb = DiscoveredServerICanCreateRandomDatabasesAndTablesOn.ExpectDatabase( + $"{TestDatabaseNames.Prefix}QueryCacheHB"); + if (cacheDb.Exists()) + DeleteTables(cacheDb); + var patcher = new QueryCachingPatcher(); + new MasterDatabaseScriptExecutor(cacheDb).CreateAndPatchDatabase(patcher, new AcceptAllCheckNotifier()); + var cacheServer = new ExternalDatabaseServer(CatalogueRepository, "HBQueryCache", patcher); + cacheServer.SetProperties(cacheDb); + + // ---- synthetic data (see fixture doc) ---- + var demog = new DataTable(); + demog.Columns.Add("chi"); + demog.Columns.Add("Region"); + foreach (var chi in Ids(1, 50)) demog.Rows.Add(chi, "T"); // Tayside + foreach (var chi in Ids(51, 80)) demog.Rows.Add(chi, "G"); // Greater Glasgow & Clyde + foreach (var chi in Ids(81, 100)) demog.Rows.Add(chi, "F"); // Fife + + var registry = OneCol("chi", Ids(1, 120)); // 100 + 20 registry-only (no region) + var excl1 = OneCol("chi", Ids(1, 10).Concat(Ids(51, 56)).Concat(Ids(81, 84))); // 10/6/4 = 20 + var excl2 = OneCol("chi", Ids(11, 18).Concat(Ids(57, 60)).Concat(Ids(85, 87))); // 8/4/3 = 15 + var excl3 = OneCol("chi", Ids(19, 20).Concat(Ids(61, 62)).Concat(Ids(88, 88))); // 2/2/1 = 5 + var excl4 = OneCol("chi", Ids(21, 21).Concat(Ids(63, 63))); // 1/1/0 = 2 + + var aggDemography = MakeSet(db, "BB_Demography", demog, out var demogCata); + var aggRegistry = MakeSet(db, "BB_Registry", registry, out _); + var aggExcl1 = MakeSet(db, "BB_Excl1", excl1, out _); + var aggExcl2 = MakeSet(db, "BB_Excl2", excl2, out _); + var aggExcl3 = MakeSet(db, "BB_Excl3", excl3, out _); + var aggExcl4 = MakeSet(db, "BB_Excl4", excl4, out _); + + // ---- CIC tree: ROOT(EXCEPT)[ Inclusion(INTERSECT)[Registry, Demography], Excl1..4 ] ---- + var cic = new CohortIdentificationConfiguration(CatalogueRepository, "BB_BuildTest") + { + QueryCachingServer_ID = cacheServer.ID + }; + var root = new CohortAggregateContainer(CatalogueRepository, SetOperation.EXCEPT) { Name = "Root" }; + root.SaveToDatabase(); + cic.RootCohortAggregateContainer_ID = root.ID; + cic.SaveToDatabase(); + + var incl = new CohortAggregateContainer(CatalogueRepository, SetOperation.INTERSECT) { Name = "Inclusion", Order = 0 }; + incl.SaveToDatabase(); + root.AddChild(incl); + incl.AddChild(aggRegistry, 0); + incl.AddChild(aggDemography, 1); + root.AddChild(aggExcl1, 1); + root.AddChild(aggExcl2, 2); + root.AddChild(aggExcl3, 3); + root.AddChild(aggExcl4, 4); + + // AddChild inserts at the top, so set the intended order explicitly (persists via SetOrder) + incl.Order = 0; + aggExcl1.Order = 1; + aggExcl2.Order = 2; + aggExcl3.Order = 3; + aggExcl4.Order = 4; + aggRegistry.Order = 0; + aggDemography.Order = 1; + + foreach (var a in new[] { aggRegistry, aggDemography, aggExcl1, aggExcl2, aggExcl3, aggExcl4 }) + cic.EnsureNamingConvention(a); + + // ---- run the command ---- + var file = new FileInfo(Path.GetTempFileName()); + try + { + var cmd = new ExecuteCommandExportCohortBuildHealthBoardBreakdown( + new ThrowImmediatelyActivator(RepositoryLocator, null), cic, file, demogCata.Name, "Region"); + Assert.That(cmd.IsImpossible, Is.False, cmd.ReasonCommandImpossible); + cmd.Execute(); + + var rows = ParseLong(File.ReadAllText(file.FullName)); + + LongRow Row(string board, string token) => + rows.Single(r => r.Board == board && r.Name.Contains(token, StringComparison.Ordinal)); + + const string T = "Tayside", G = "Greater Glasgow & Clyde", F = "Fife"; + + Assert.Multiple(() => + { + // unfiltered tree (must equal the UI / RDMP's own counts) + Assert.That(Row("Unfiltered", "Root").Final, Is.EqualTo(58)); + Assert.That(Row("Unfiltered", "Inclusion").Final, Is.EqualTo(100)); + Assert.That(Row("Unfiltered", "Registry").Final, Is.EqualTo(120)); + Assert.That(Row("Unfiltered", "Demography").Final, Is.EqualTo(100)); + Assert.That(Row("Unfiltered", "Demography").Cum, Is.EqualTo(100)); // INTERSECT cumulative + Assert.That(Row("Unfiltered", "Excl1").Cum, Is.EqualTo(80)); + Assert.That(Row("Unfiltered", "Excl2").Cum, Is.EqualTo(65)); + Assert.That(Row("Unfiltered", "Excl3").Cum, Is.EqualTo(60)); + Assert.That(Row("Unfiltered", "Excl4").Cum, Is.EqualTo(58)); + + // Tayside cumulative through the tree + Assert.That(Row(T, "Inclusion").Final, Is.EqualTo(50)); + Assert.That(Row(T, "Excl1").Cum, Is.EqualTo(40)); + Assert.That(Row(T, "Excl2").Cum, Is.EqualTo(32)); + Assert.That(Row(T, "Excl3").Cum, Is.EqualTo(30)); + Assert.That(Row(T, "Excl4").Cum, Is.EqualTo(29)); + Assert.That(Row(T, "Root").Final, Is.EqualTo(29)); + + // Glasgow + Fife endpoints + Assert.That(Row(G, "Root").Final, Is.EqualTo(17)); + Assert.That(Row(G, "Excl4").Cum, Is.EqualTo(17)); + Assert.That(Row(F, "Root").Final, Is.EqualTo(12)); + Assert.That(Row(F, "Excl4").Cum, Is.EqualTo(12)); + + // the 20 registry-only people (no demography row) land in Unknown on the Registry set + Assert.That(Row("Unknown", "Registry").Final, Is.EqualTo(20)); + }); + + // ---- partition: at EVERY node, the boards (+Unknown) sum to the unfiltered total ---- + foreach (var node in rows.GroupBy(r => (r.Name, r.Order, r.Container))) + { + var unfiltered = node.Single(r => r.Board == "Unfiltered"); + var boardFinal = node.Where(r => r.Board != "Unfiltered").Sum(r => r.Final ?? 0); + Assert.That(boardFinal, Is.EqualTo(unfiltered.Final), + $"Final partition mismatch at node '{unfiltered.Name}'"); + + if (unfiltered.Cum.HasValue) + { + var boardCum = node.Where(r => r.Board != "Unfiltered").Sum(r => r.Cum ?? 0); + Assert.That(boardCum, Is.EqualTo(unfiltered.Cum), + $"Cumulative partition mismatch at node '{unfiltered.Name}'"); + } + } + } + finally + { + file.Delete(); + } + } + + [Test] + public void Report_BuildRows_UnfilteredFirst_UnknownDerived_BoardMajor() + { + // one node: unfiltered 100, T=50 G=30 (known), so Unknown = 100 - 80 = 20 + var nodes = new List + { + new() + { + Seq = 0, Type = "Cohort Set", Name = "Set1", Container = "Root", SetOperation = "", + FinalUnfiltered = 100, CumulativeUnfiltered = null, + FinalByRegion = new Dictionary { ["T"] = 50, ["G"] = 30 } + } + }; + + var rows = CohortBuildHealthBoardBreakdownReport.BuildRows(nodes); + + Assert.Multiple(() => + { + Assert.That(rows[0].Board, Is.EqualTo("Unfiltered")); + Assert.That(rows[0].FinalCount, Is.EqualTo(100)); + Assert.That(rows.Single(r => r.Board == "Tayside").FinalCount, Is.EqualTo(50)); + Assert.That(rows.Single(r => r.Board == "Greater Glasgow & Clyde").FinalCount, Is.EqualTo(30)); + Assert.That(rows.Last().Board, Is.EqualTo("Unknown")); + Assert.That(rows.Last().FinalCount, Is.EqualTo(20)); // 100 - (50+30) + // partition holds + Assert.That(rows.Where(r => r.Board != "Unfiltered").Sum(r => r.FinalCount ?? 0), Is.EqualTo(100)); + }); + + var csv = CohortBuildHealthBoardBreakdownReport.ToCsv(rows); + Assert.That(csv.Split('\n')[0].Trim(), + Is.EqualTo("Board,Node,Order,Type,Name,Container,SetOperation,FinalCount,CumulativeCount")); + } + + private static DataTable OneCol(string col, IEnumerable values) + { + var dt = new DataTable(); + dt.Columns.Add(col); + foreach (var v in values) dt.Rows.Add(v); + return dt; + } + + private AggregateConfiguration MakeSet(DiscoveredDatabase db, string name, DataTable data, out ICatalogue cata) + { + var tbl = db.CreateTable(name, data); + cata = Import(tbl); + var chi = cata.GetAllExtractionInformation(ExtractionCategory.Any) + .Single(e => e.GetRuntimeName().Equals("chi", StringComparison.OrdinalIgnoreCase)); + chi.IsExtractionIdentifier = true; + chi.SaveToDatabase(); + + var agg = new AggregateConfiguration(CatalogueRepository, cata, name) { CountSQL = null }; + agg.SaveToDatabase(); + _ = new AggregateDimension(CatalogueRepository, chi, agg); + return agg; + } + + private sealed record LongRow(string Board, string Node, int Order, string Type, string Name, + string Container, string SetOp, int? Final, int? Cum); + + private static List ParseLong(string csv) + { + var rows = new List(); + foreach (var line in csv.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries).Skip(1)) + { + var c = line.Split(','); + int? N(string s) => string.IsNullOrEmpty(s) ? null : int.Parse(s); + rows.Add(new LongRow(c[0], c[1], int.Parse(c[2]), c[3], c[4], c[5], c[6], N(c[7]), N(c[8]))); + } + + return rows; + } +} diff --git a/Rdmp.Core.Tests/CohortCreation/HealthBoardBreakdownReportTests.cs b/Rdmp.Core.Tests/CohortCreation/HealthBoardBreakdownReportTests.cs new file mode 100644 index 0000000000..1989f99347 --- /dev/null +++ b/Rdmp.Core.Tests/CohortCreation/HealthBoardBreakdownReportTests.cs @@ -0,0 +1,304 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System; +using System.Collections.Generic; +using System.Data; +using System.IO; +using System.Linq; +using NUnit.Framework; +using Rdmp.Core.CohortCreation; +using Rdmp.Core.CommandExecution; +using Rdmp.Core.CommandExecution.AtomicCommands; +using Rdmp.Core.Curation.Data; +using Rdmp.Core.Curation.Data.Aggregation; +using Rdmp.Core.Curation.Data.Cohort; +using Tests.Common; +using Tests.Common.Scenarios; + +namespace Rdmp.Core.Tests.CohortCreation; + +/// +/// No-database unit tests for the health board lookup, the report projection (ordering, subtotals, +/// unknown bucket, CSV escaping) and the breakdown SQL. +/// +public class HealthBoardBreakdownReportTests +{ + [Test] + public void Lookup_KnownAndUnknownRegions() + { + Assert.Multiple(() => + { + var tayside = HealthBoardLookup.Resolve("T"); + Assert.That(tayside.Name, Is.EqualTo("Tayside")); + Assert.That(tayside.HbCode, Is.EqualTo(3)); + Assert.That(tayside.Node, Is.EqualTo("East")); + + // case-insensitive + trimmed + Assert.That(HealthBoardLookup.Resolve(" g ").Name, Is.EqualTo("Greater Glasgow & Clyde")); + + // legacy board has no numeric code but is still a real board + var clyde = HealthBoardLookup.Resolve("C"); + Assert.That(clyde.Name, Is.EqualTo("Clyde")); + Assert.That(clyde.HbCode, Is.Null); + Assert.That(clyde.Node, Is.EqualTo("West")); + + // unmapped + null both land in the Unknown node, never null + Assert.That(HealthBoardLookup.Resolve("Q").Node, Is.EqualTo(HealthBoardLookup.UnknownNode)); + Assert.That(HealthBoardLookup.Resolve(null).Node, Is.EqualTo(HealthBoardLookup.UnknownNode)); + }); + } + + [Test] + public void BuildRecords_GroupsByNode_Subtotals_UnknownLast_AndGrandTotal() + { + var dt = new DataTable(); + dt.Columns.Add("Region"); + dt.Columns.Add("n", typeof(int)); + dt.Rows.Add("T", 3); // Tayside (East) + dt.Rows.Add("F", 2); // Fife (East) + dt.Rows.Add("G", 4); // Glasgow (West) + dt.Rows.Add("Q", 1); // unknown cipher + dt.Rows.Add(DBNull.Value, 2); // NULL region -> Unknown (folds with Q) + + var records = HealthBoardBreakdownReport.BuildRecords(dt); + + // Unknown node must come last + var nodeOrder = records + .Where(r => r.Kind == HealthBoardBreakdownReport.RowKind.NodeSubtotal) + .Select(r => r.Node) + .ToList(); + Assert.That(nodeOrder, Is.EqualTo(new[] { "East", "West", HealthBoardLookup.UnknownNode })); + + int Subtotal(string node) => records.Single(r => + r.Kind == HealthBoardBreakdownReport.RowKind.NodeSubtotal && r.Node == node).Count; + int Board(string name) => records.Single(r => + r.Kind == HealthBoardBreakdownReport.RowKind.Board && r.HbName == name).Count; + + Assert.Multiple(() => + { + Assert.That(Board("Tayside"), Is.EqualTo(3)); + Assert.That(Board("Fife"), Is.EqualTo(2)); + Assert.That(Board("Greater Glasgow & Clyde"), Is.EqualTo(4)); + + Assert.That(Subtotal("East"), Is.EqualTo(5)); + Assert.That(Subtotal("West"), Is.EqualTo(4)); + // unmapped 'Q' (1) + NULL (2) fold into a single Unknown bucket + Assert.That(Subtotal(HealthBoardLookup.UnknownNode), Is.EqualTo(3)); + + // grand total = sum of all + var total = records.Single(r => r.Kind == HealthBoardBreakdownReport.RowKind.GrandTotal); + Assert.That(total.Count, Is.EqualTo(12)); + + // a single Unknown board row, not one per cipher + Assert.That(records.Count(r => r.Kind == HealthBoardBreakdownReport.RowKind.Board + && r.Node == HealthBoardLookup.UnknownNode), Is.EqualTo(1)); + }); + } + + [Test] + public void ToCsv_HeaderAndEscaping() + { + var records = new List + { + new() { Kind = HealthBoardBreakdownReport.RowKind.Board, Region = "T", HbCode = 3, HbName = "Tayside", Node = "East", Count = 3 }, + new() { Kind = HealthBoardBreakdownReport.RowKind.Board, Region = "C", HbCode = null, HbName = "Clyde", Node = "West", Count = 1 } + }; + + var csv = HealthBoardBreakdownReport.ToCsv(records); + var lines = csv.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries); + + Assert.Multiple(() => + { + Assert.That(lines[0], Is.EqualTo("Region,HBCode,HBName,Node,Count")); + Assert.That(lines[1], Is.EqualTo("T,3,Tayside,East,3")); + // null HbCode renders as an empty field, not "0" + Assert.That(lines[2], Is.EqualTo("C,,Clyde,West,1")); + }); + } + + [Test] + public void BuildBreakdownSql_ContainsJoinAndGroupBy() + { + var sql = ExecuteCommandExportCohortHealthBoardBreakdown.BuildBreakdownSql( + regionSelect: "[d]..[Demography].[Region]", + chiSelect: "[d]..[Demography].[chi]", + demographyTable: "[d]..[Demography]", + cohortIdSubquery: "SELECT [c]..[Cohort].[PrivateID] FROM [c]..[Cohort] WHERE [c]..[Cohort].[cohortDefinition_id]=42"); + + Assert.Multiple(() => + { + Assert.That(sql, Does.Contain("COUNT(DISTINCT [d]..[Demography].[chi])")); + Assert.That(sql, Does.Contain("AS Region")); + Assert.That(sql, Does.Contain("AS n")); + Assert.That(sql, Does.Contain("[d]..[Demography].[chi] IN (")); + Assert.That(sql, Does.Contain("WHERE [c]..[Cohort].[cohortDefinition_id]=42")); + Assert.That(sql, Does.Contain("GROUP BY [d]..[Demography].[Region]")); + }); + } +} + +/// +/// End-to-end against the test SQL Server: a synthetic SHARE_Demography table is joined to a real +/// committed cohort and the exported breakdown is checked for distinct-patient counts, node +/// subtotals, the Unknown bucket, exclusion of non-cohort patients and total reconciliation. +/// Requires the test SQL Server (see mac-test-env). +/// +public class HealthBoardBreakdownDatabaseTests : TestsRequiringACohort +{ + [Test] + public void Breakdown_RealCohort_CountsPerBoardNodeAndReconciles() + { + // cohort private ids seeded by TestsRequiringACohort: + // Priv_12345, Priv_66666, Priv_54321, Priv_66999, Priv_14722, Priv_wtf11 (6 patients) + var demo = new DataTable(); + demo.Columns.Add("chi"); + demo.Columns.Add("Region"); + demo.Rows.Add("Priv_12345", "T"); // Tayside (East) + demo.Rows.Add("Priv_12345", "T"); // duplicate -> proves COUNT(DISTINCT) + demo.Rows.Add("Priv_66666", "T"); // Tayside + demo.Rows.Add("Priv_54321", "G"); // Glasgow (West) + demo.Rows.Add("Priv_66999", "G"); // Glasgow + demo.Rows.Add("Priv_14722", "Q"); // unmapped cipher -> Unknown + demo.Rows.Add("OUTSIDER_1", "T"); // not in the cohort -> must be excluded + // Priv_wtf11 deliberately absent from demography -> reconciliation shortfall of 1 + + var db = GetCleanedServer(FAnsi.DatabaseType.MicrosoftSQLServer); + var demoTbl = db.CreateTable("SHARE_Demography", demo); + + var cata = Import(demoTbl); + var chiEi = cata.GetAllExtractionInformation(ExtractionCategory.Any) + .Single(e => e.GetRuntimeName().Equals("chi", StringComparison.OrdinalIgnoreCase)); + chiEi.IsExtractionIdentifier = true; + chiEi.SaveToDatabase(); + + var file = new FileInfo(Path.GetTempFileName()); + try + { + var cmd = new ExecuteCommandExportCohortHealthBoardBreakdown( + new ThrowImmediatelyActivator(RepositoryLocator, null), + (ExtractableCohort_DataExport())!, file, cata.Name, "Region"); + + Assert.That(cmd.IsImpossible, Is.False, cmd.ReasonCommandImpossible); + cmd.Execute(); + + var counts = ParseCsv(File.ReadAllText(file.FullName)); + + Assert.Multiple(() => + { + Assert.That(counts["Tayside"], Is.EqualTo(2)); // distinct: dup folded + Assert.That(counts["Greater Glasgow & Clyde"], Is.EqualTo(2)); + Assert.That(counts["East - total"], Is.EqualTo(2)); + Assert.That(counts["West - total"], Is.EqualTo(2)); + Assert.That(counts[$"{HealthBoardLookup.UnknownNode} - total"], Is.EqualTo(1)); // 'Q' + Assert.That(counts["TOTAL"], Is.EqualTo(5)); // 5 of 6 matched; OUTSIDER excluded + }); + } + finally + { + file.Delete(); + } + } + + // small accessor to the protected IExtractableCohort as a concrete ExtractableCohort + private global::Rdmp.Core.DataExport.Data.ExtractableCohort ExtractableCohort_DataExport() => + _extractableCohort as global::Rdmp.Core.DataExport.Data.ExtractableCohort; + + /// Parses the breakdown CSV into HBName -> Count (board names carry no commas). + internal static Dictionary ParseCsv(string csv) + { + var result = new Dictionary(); + var lines = csv.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries); + foreach (var line in lines.Skip(1)) // skip header + { + var cells = line.Split(','); + // Region,HBCode,HBName,Node,Count + result[cells[2]] = int.Parse(cells[4]); + } + + return result; + } +} + +/// +/// End-to-end for the live input: a cohort is built +/// over a synthetic patients catalogue and broken down against a separate synthetic SHARE_Demography +/// table on the same server. Requires the test SQL Server (see mac-test-env). +/// +public class HealthBoardBreakdownCicDatabaseTests : DatabaseTests +{ + [Test] + public void Breakdown_FromCic_CountsPerBoardAndReconciles() + { + var db = GetCleanedServer(FAnsi.DatabaseType.MicrosoftSQLServer); + + // patients catalogue = the cohort source (everyone here is in the cohort) + var patients = new DataTable(); + patients.Columns.Add("chi"); + foreach (var chi in new[] { "c1", "c2", "c3", "c4", "c6" }) + patients.Rows.Add(chi); + var patientsTbl = db.CreateTable("HBPatients", patients); + var patientsCata = Import(patientsTbl); + var patientChi = patientsCata.GetAllExtractionInformation(ExtractionCategory.Any) + .Single(e => e.GetRuntimeName().Equals("chi", StringComparison.OrdinalIgnoreCase)); + patientChi.IsExtractionIdentifier = true; + patientChi.SaveToDatabase(); + + // demography: maps some patients to boards; c6 absent (-> shortfall), cX not in cohort + var demo = new DataTable(); + demo.Columns.Add("chi"); + demo.Columns.Add("Region"); + demo.Rows.Add("c1", "T"); // Tayside (East) + demo.Rows.Add("c2", "T"); // Tayside + demo.Rows.Add("c3", "G"); // Glasgow (West) + demo.Rows.Add("c4", "Q"); // unmapped -> Unknown + demo.Rows.Add("cX", "T"); // not in cohort -> excluded + var demoTbl = db.CreateTable("SHARE_Demography", demo); + var demoCata = Import(demoTbl); + demoCata.GetAllExtractionInformation(ExtractionCategory.Any) + .Single(e => e.GetRuntimeName().Equals("chi", StringComparison.OrdinalIgnoreCase)) + .IsExtractionIdentifier = true; + foreach (var e in demoCata.GetAllExtractionInformation(ExtractionCategory.Any) + .Where(e => e.GetRuntimeName().Equals("chi", StringComparison.OrdinalIgnoreCase))) + e.SaveToDatabase(); + + // build a single-set CIC selecting chi from the patients catalogue + var agg = new AggregateConfiguration(CatalogueRepository, patientsCata, "HBCohortSet") { CountSQL = null }; + agg.SaveToDatabase(); + _ = new AggregateDimension(CatalogueRepository, patientChi, agg); + + var cic = new CohortIdentificationConfiguration(CatalogueRepository, "HBCic"); + cic.CreateRootContainerIfNotExists(); + cic.RootCohortAggregateContainer.AddChild(agg, 0); + cic.EnsureNamingConvention(agg); + + var file = new FileInfo(Path.GetTempFileName()); + try + { + var cmd = new ExecuteCommandExportCicHealthBoardBreakdown( + new ThrowImmediatelyActivator(RepositoryLocator, null), + cic, file, demoCata.Name, "Region"); + + Assert.That(cmd.IsImpossible, Is.False, cmd.ReasonCommandImpossible); + cmd.Execute(); + + var counts = HealthBoardBreakdownDatabaseTests.ParseCsv(File.ReadAllText(file.FullName)); + Assert.Multiple(() => + { + Assert.That(counts["Tayside"], Is.EqualTo(2)); + Assert.That(counts["Greater Glasgow & Clyde"], Is.EqualTo(1)); + Assert.That(counts["East - total"], Is.EqualTo(2)); + Assert.That(counts["West - total"], Is.EqualTo(1)); + Assert.That(counts[$"{HealthBoardLookup.UnknownNode} - total"], Is.EqualTo(1)); // c4='Q' + Assert.That(counts["TOTAL"], Is.EqualTo(4)); // c6 absent from demography, cX excluded + }); + } + finally + { + file.Delete(); + } + } +} diff --git a/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs b/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs new file mode 100644 index 0000000000..b37113e835 --- /dev/null +++ b/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs @@ -0,0 +1,176 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; + +namespace Rdmp.Core.CohortCreation; + +/// +/// Projects a cohort build's count tree (the per-set / per-container FinalCount and cumulative +/// running totals shown in the Cohort Builder) split by health board into a long, file-friendly CSV: +/// one row per (board × count-point). The boards partition the cohort (1 patient ↔ 1 board), so an +/// Unfiltered pseudo-board (RDMP's own counts) and an Unknown board (patients with no / +/// unmapped region) bracket the real boards and reconcile to the unfiltered total at every node. +/// +public static class CohortBuildHealthBoardBreakdownReport +{ + public const string UnfilteredBoard = "Unfiltered"; + public const string UnknownBoard = "Unknown"; + + /// One count point of the build tree with its per-region counts (known boards only). + public sealed class NodeBreakdown + { + /// Tree walk order (stable, used to order rows within a board). + public int Seq { get; init; } + + /// "Cohort Set" or "Container". + public string Type { get; init; } = ""; + + public string Name { get; init; } = ""; + + /// Name of the parent container (empty for the root). + public string Container { get; init; } = ""; + + /// UNION / INTERSECT / EXCEPT for containers; empty for sets. + public string SetOperation { get; init; } = ""; + + public int DisplayOrder { get; init; } + + /// RDMP's own count for this node (the unfiltered total). + public int FinalUnfiltered { get; init; } + + /// RDMP's own cumulative within the parent container; null if not applicable. + public int? CumulativeUnfiltered { get; init; } + + /// Region cipher → final count (known boards only; GROUP BY Region result). + public IReadOnlyDictionary FinalByRegion { get; init; } = new Dictionary(); + + /// Region cipher → cumulative count; null when this node has no cumulative. + public IReadOnlyDictionary CumulativeByRegion { get; init; } + } + + public sealed class Row + { + public string Board { get; init; } = ""; + public string Node { get; init; } = ""; + public int Order { get; init; } + public string Type { get; init; } = ""; + public string Name { get; init; } = ""; + public string Container { get; init; } = ""; + public string SetOperation { get; init; } = ""; + public int? FinalCount { get; init; } + public int? CumulativeCount { get; init; } + } + + private static readonly string[] Header = + { + "Board", "Node", "Order", "Type", "Name", "Container", "SetOperation", "FinalCount", "CumulativeCount" + }; + + /// + /// Flattens the per-node breakdowns into long rows, board-major: the + /// tree first, then each real board's full tree (every board appears at every node, 0 where absent), + /// then the tree last. Within a board, rows follow the tree walk order. + /// + public static IReadOnlyList BuildRows(IReadOnlyList nodes) + { + var ordered = nodes.OrderBy(n => n.Seq).ToList(); + + // every region cipher seen anywhere → its board; keep a deterministic board ordering + var boards = ordered + .SelectMany(n => n.FinalByRegion.Keys) + .Select(r => HealthBoardLookup.Resolve(r)) + .Where(b => b.Node != HealthBoardLookup.UnknownNode) + .GroupBy(b => b.Region, System.StringComparer.OrdinalIgnoreCase) + .Select(g => g.First()) + .OrderBy(b => b.Node, System.StringComparer.OrdinalIgnoreCase) + .ThenBy(b => b.Name, System.StringComparer.OrdinalIgnoreCase) + .ToList(); + + var rows = new List(); + + // 1) Unfiltered (RDMP's own counts) + foreach (var n in ordered) + rows.Add(MakeRow(n, UnfilteredBoard, "", n.FinalUnfiltered, n.CumulativeUnfiltered)); + + // 2) each real board + foreach (var b in boards) + foreach (var n in ordered) + { + var fin = n.FinalByRegion.TryGetValue(b.Region, out var f) ? f : 0; + int? cum = n.CumulativeByRegion == null + ? null + : n.CumulativeByRegion.TryGetValue(b.Region, out var c) ? c : 0; + rows.Add(MakeRow(n, b.Name, b.Node, fin, cum)); + } + + // 3) Unknown = unfiltered − Σ known boards (patients not in demography / unmapped region) + foreach (var n in ordered) + { + var knownFinal = n.FinalByRegion + .Where(kv => HealthBoardLookup.Resolve(kv.Key).Node != HealthBoardLookup.UnknownNode) + .Sum(kv => kv.Value); + var finalUnknown = n.FinalUnfiltered - knownFinal; + + int? cumUnknown = null; + if (n.CumulativeUnfiltered.HasValue && n.CumulativeByRegion != null) + { + var knownCum = n.CumulativeByRegion + .Where(kv => HealthBoardLookup.Resolve(kv.Key).Node != HealthBoardLookup.UnknownNode) + .Sum(kv => kv.Value); + cumUnknown = n.CumulativeUnfiltered.Value - knownCum; + } + + rows.Add(MakeRow(n, UnknownBoard, UnknownBoard, finalUnknown, cumUnknown)); + } + + return rows; + } + + private static Row MakeRow(NodeBreakdown n, string board, string node, int? final, int? cumulative) => new() + { + Board = board, + Node = node, + Order = n.DisplayOrder, + Type = n.Type, + Name = n.Name, + Container = n.Container, + SetOperation = n.SetOperation, + FinalCount = final, + CumulativeCount = cumulative + }; + + public static string ToCsv(IEnumerable rows) + { + var sb = new StringBuilder(); + sb.AppendLine(string.Join(",", Header.Select(Escape))); + foreach (var r in rows) + sb.AppendLine(string.Join(",", new[] + { + r.Board, r.Node, + r.Order.ToString(CultureInfo.InvariantCulture), + r.Type, r.Name, r.Container, r.SetOperation, + r.FinalCount?.ToString(CultureInfo.InvariantCulture) ?? "", + r.CumulativeCount?.ToString(CultureInfo.InvariantCulture) ?? "" + }.Select(Escape))); + return sb.ToString(); + } + + public static void WriteCsv(string path, IReadOnlyList nodes) => + File.WriteAllText(path, ToCsv(BuildRows(nodes))); + + private static string Escape(string field) + { + field ??= ""; + if (field.Contains(',') || field.Contains('"') || field.Contains('\n') || field.Contains('\r')) + return $"\"{field.Replace("\"", "\"\"")}\""; + return field; + } +} diff --git a/Rdmp.Core/CohortCreation/HealthBoardBreakdownReport.cs b/Rdmp.Core/CohortCreation/HealthBoardBreakdownReport.cs new file mode 100644 index 0000000000..cca505e8ca --- /dev/null +++ b/Rdmp.Core/CohortCreation/HealthBoardBreakdownReport.cs @@ -0,0 +1,169 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System; +using System.Collections.Generic; +using System.Data; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; + +namespace Rdmp.Core.CohortCreation; + +/// +/// Turns a per-Region count (the SELECT Region, COUNT(DISTINCT chi) ... GROUP BY Region +/// result of a cohort's final inclusion list joined to SHARE_Demography) into a flat, file-friendly +/// breakdown: one row per health board, a subtotal per safe-haven node, and a grand total. Region +/// ciphers are mapped to boards/nodes via ; unmapped or NULL regions +/// are reported under the bucket so the rows reconcile to +/// the cohort size. +/// +public static class HealthBoardBreakdownReport +{ + /// The kind of row: a single board, a per-node subtotal, or the grand total. + public enum RowKind + { + Board, + NodeSubtotal, + GrandTotal + } + + /// One row of the breakdown. + public sealed class BreakdownRecord + { + public RowKind Kind { get; init; } + + /// Region cipher (board rows only; empty for subtotal/total rows). + public string Region { get; init; } = ""; + + /// Numeric health board code, where one exists (board rows only). + public int? HbCode { get; init; } + + /// Display name: the board name, "<Node> - total", or "TOTAL". + public string HbName { get; init; } = ""; + + /// Safe-haven node (board + subtotal rows; empty for the grand total). + public string Node { get; init; } = ""; + + /// Distinct patient count. + public int Count { get; init; } + } + + private static readonly string[] Header = { "Region", "HBCode", "HBName", "Node", "Count" }; + + /// + /// Builds the ordered breakdown records from a region/count table. Boards are grouped by node + /// (nodes alphabetical, last; boards within a node + /// alphabetical), each node followed by its subtotal, then a final grand total. The + /// is expected to have a region column and an integer count column. + /// + public static IReadOnlyList BuildRecords(DataTable dt, string regionColumn = "Region", + string countColumn = "n") + { + // Collapse to one count per resolved board (defensive: a region could appear twice if the + // source query did not group cleanly, and several ciphers/NULLs all fold into Unknown). + var boards = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (DataRow row in dt.Rows) + { + var regionVal = row[regionColumn] == DBNull.Value ? null : row[regionColumn]?.ToString(); + var hb = HealthBoardLookup.Resolve(regionVal); + var count = row[countColumn] == DBNull.Value + ? 0 + : Convert.ToInt32(row[countColumn], CultureInfo.InvariantCulture); + + // key on the resolved board (Region for known boards, "" for the single Unknown bucket) + var key = hb.Node == HealthBoardLookup.UnknownNode ? "\0unknown" : hb.Region; + if (boards.TryGetValue(key, out var existing)) + boards[key] = (existing.Hb, existing.Count + count); + else + boards[key] = (hb, count); + } + + var records = new List(); + + // node ordering: alphabetical, Unknown always last + var nodes = boards.Values + .Select(b => b.Hb.Node) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(n => n == HealthBoardLookup.UnknownNode ? 1 : 0) + .ThenBy(n => n, StringComparer.OrdinalIgnoreCase); + + var grandTotal = 0; + + foreach (var node in nodes) + { + var inNode = boards.Values + .Where(b => string.Equals(b.Hb.Node, node, StringComparison.OrdinalIgnoreCase)) + .OrderBy(b => b.Hb.Name, StringComparer.OrdinalIgnoreCase) + .ToList(); + + foreach (var (hb, count) in inNode) + records.Add(new BreakdownRecord + { + Kind = RowKind.Board, + Region = hb.Region, + HbCode = hb.HbCode, + HbName = hb.Name, + Node = hb.Node, + Count = count + }); + + var nodeTotal = inNode.Sum(b => b.Count); + grandTotal += nodeTotal; + + records.Add(new BreakdownRecord + { + Kind = RowKind.NodeSubtotal, + HbName = $"{node} - total", + Node = node, + Count = nodeTotal + }); + } + + records.Add(new BreakdownRecord + { + Kind = RowKind.GrandTotal, + HbName = "TOTAL", + Count = grandTotal + }); + + return records; + } + + /// Serialises records to CSV (with a header row). + public static string ToCsv(IEnumerable records) + { + var sb = new StringBuilder(); + sb.AppendLine(string.Join(",", Header.Select(Escape))); + + foreach (var r in records) + sb.AppendLine(string.Join(",", new[] + { + r.Region, + r.HbCode?.ToString(CultureInfo.InvariantCulture) ?? "", + r.HbName, + r.Node, + r.Count.ToString(CultureInfo.InvariantCulture) + }.Select(Escape))); + + return sb.ToString(); + } + + /// Convenience: build the records from a region/count table and write the CSV. + public static void WriteCsv(string path, DataTable dt, string regionColumn = "Region", + string countColumn = "n") => + File.WriteAllText(path, ToCsv(BuildRecords(dt, regionColumn, countColumn))); + + private static string Escape(string field) + { + field ??= ""; + if (field.Contains(',') || field.Contains('"') || field.Contains('\n') || field.Contains('\r')) + return $"\"{field.Replace("\"", "\"\"")}\""; + return field; + } +} diff --git a/Rdmp.Core/CohortCreation/HealthBoardLookup.cs b/Rdmp.Core/CohortCreation/HealthBoardLookup.cs new file mode 100644 index 0000000000..a0a2a67ff5 --- /dev/null +++ b/Rdmp.Core/CohortCreation/HealthBoardLookup.cs @@ -0,0 +1,61 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System.Collections.Generic; + +namespace Rdmp.Core.CohortCreation; + +/// +/// A Scottish health board: the single-letter cipher held in +/// SHARE_Demography, its numeric (null for legacy boards), its display +/// , and the safe-haven it rolls up to. +/// +public sealed record HealthBoard(string Region, int? HbCode, string Name, string Node); + +/// +/// Hardcoded mapping from a SHARE_Demography Region cipher to its health board and +/// safe-haven node. Single source of truth for the cohort health-board breakdown report; an +/// unrecognised or NULL region resolves to a non-null "(unknown)" board under the +/// so counts are never silently dropped. +/// +public static class HealthBoardLookup +{ + /// Node assigned to any region cipher not present in the lookup (or NULL/empty). + public const string UnknownNode = "Unknown"; + + // keyed by the single-letter Region cipher held in SHARE_Demography.Region + private static readonly Dictionary ByRegion = new(System.StringComparer.OrdinalIgnoreCase) + { + ["A"] = new("A", 11, "Ayrshire & Arran", "West"), + ["B"] = new("B", 6, "Borders", "South East"), + ["Y"] = new("Y", 12, "Dumfries & Galloway", "West"), + ["F"] = new("F", 4, "Fife", "East"), + ["V"] = new("V", 7, "Forth Valley", "East"), + ["N"] = new("N", 2, "Grampian", "North"), + ["G"] = new("G", 16, "Greater Glasgow & Clyde", "West"), + ["H"] = new("H", 17, "Highland", "North"), + ["L"] = new("L", 10, "Lanarkshire", "West"), + ["S"] = new("S", 5, "Lothian", "South East"), + ["R"] = new("R", 13, "Orkney", "North"), + ["Z"] = new("Z", 14, "Shetland", "North"), + ["T"] = new("T", 3, "Tayside", "East"), + ["W"] = new("W", 15, "Western Isles", "North"), + ["C"] = new("C", null, "Clyde", "West"), // legacy board: no numeric HB_Code (intentional) + }; + + /// + /// Resolves a Region cipher to its . Unknown, NULL or empty + /// ciphers map to a placeholder board (name "(unknown)", node ) + /// rather than null, so unmapped patients are reported and reconcile to the cohort total. + /// + public static HealthBoard Resolve(string region) + { + var key = region?.Trim(); + return !string.IsNullOrEmpty(key) && ByRegion.TryGetValue(key, out var hb) + ? hb + : new HealthBoard(key ?? "", null, "(unknown)", UnknownNode); + } +} diff --git a/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs b/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs new file mode 100644 index 0000000000..dba00d47fc --- /dev/null +++ b/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs @@ -0,0 +1,320 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System.Collections.Generic; +using System.Data; +using System.IO; +using System.Linq; +using System.Threading; +using FAnsi.Discovery; +using Rdmp.Core.CohortCreation; +using Rdmp.Core.CohortCreation.Execution; +using Rdmp.Core.Curation.Data; +using Rdmp.Core.Curation.Data.Aggregation; +using Rdmp.Core.Curation.Data.Cohort; +using Rdmp.Core.MapsDirectlyToDatabaseTable; +using Rdmp.Core.QueryCaching.Aggregation; +using Rdmp.Core.ReusableLibraryCode.DataAccess; + +namespace Rdmp.Core.CommandExecution.AtomicCommands; + +/// +/// Reproduces the Cohort Builder's per-set / per-container count tree (the FinalCount and cumulative +/// running totals shown as UNION/INTERSECT/EXCEPT are applied) split by Scottish health board, and +/// writes it to a long-format CSV. Operates purely on the query cache: it builds the cohort once to +/// populate the per-set cache tables, then recomposes every count point from those cache tables and +/// splits it by SHARE_Demography.Region with one GROUP BY per node (all boards at once). +/// +public class ExecuteCommandExportCohortBuildHealthBoardBreakdown : BasicCommandExecution +{ + private readonly CohortIdentificationConfiguration _cic; + private readonly string _demographyCatalogue; + private readonly string _regionColumn; + private readonly int _timeout; + private FileInfo _toFile; + + private ExtractionInformation _regionEi; + private ExtractionInformation _idEi; + + private DiscoveredDatabase _cacheDb; + private CachedAggregateConfigurationResultsManager _cacheManager; + private string _demogTable; + private string _demogId; + private string _regionName; + + private readonly Dictionary _setCacheTable = new(); + private readonly Dictionary<(bool isContainer, int id), (int final, int? cumulative)> _baseline = new(); + + public ExecuteCommandExportCohortBuildHealthBoardBreakdown(IBasicActivateItems activator, + [DemandsInitialization("The cohort identification configuration whose build tree to break down")] + CohortIdentificationConfiguration cic, + [DemandsInitialization("CSV file to write. Defaults to -build-healthboard.csv in the current directory")] + FileInfo toFile = null, + [DemandsInitialization("Demography catalogue holding the region column", DefaultValue = "SHARE_Demography")] + string demographyCatalogue = "SHARE_Demography", + [DemandsInitialization("Region (health board cipher) column on the demography catalogue", DefaultValue = "Region")] + string regionColumn = "Region", + [DemandsInitialization("Per-query command timeout in seconds", DefaultValue = 5000)] + int timeout = 5000) : base(activator) + { + _cic = cic; + _demographyCatalogue = demographyCatalogue; + _regionColumn = regionColumn; + _timeout = timeout; + _toFile = toFile; + + if (_cic == null) + { + SetImpossible("No CohortIdentificationConfiguration was supplied"); + return; + } + + if (_cic.RootCohortAggregateContainer_ID == null) + { + SetImpossible($"'{_cic}' has no root container to run"); + return; + } + + if (_cic.QueryCachingServer_ID == null) + { + SetImpossible($"'{_cic}' has no query caching server - this breakdown works only on cached results"); + return; + } + + ResolveDemography(activator); + } + + private void ResolveDemography(IBasicActivateItems activator) + { + var demography = activator.RepositoryLocator.CatalogueRepository + .GetAllObjects() + .FirstOrDefault(c => string.Equals(c.Name, _demographyCatalogue, System.StringComparison.OrdinalIgnoreCase)); + + if (demography == null) + { + SetImpossible($"Could not find a catalogue called '{_demographyCatalogue}'"); + return; + } + + var eis = demography.GetAllExtractionInformation(ExtractionCategory.Any); + _regionEi = eis.FirstOrDefault(e => + string.Equals(e.GetRuntimeName(), _regionColumn, System.StringComparison.OrdinalIgnoreCase)); + _idEi = eis.FirstOrDefault(e => e.IsExtractionIdentifier); + + if (_regionEi == null) + { + SetImpossible($"'{_demographyCatalogue}' has no column called '{_regionColumn}'"); + return; + } + + if (_idEi == null) + { + SetImpossible($"'{_demographyCatalogue}' has no IsExtractionIdentifier column to join the cohort on"); + return; + } + + // co-location: the recompose + GROUP BY join runs on the cache server, so demography must be there + var cacheServer = _cic.QueryCachingServer.Server; + var demogServer = _idEi.ColumnInfo.TableInfo.Server; + if (!string.IsNullOrWhiteSpace(cacheServer) && !string.IsNullOrWhiteSpace(demogServer) + && !string.Equals(cacheServer.Trim(), demogServer.Trim(), System.StringComparison.OrdinalIgnoreCase)) + SetImpossible( + $"Demography catalogue is on server '{demogServer}' but the query cache is on '{cacheServer}'. " + + "This breakdown joins on the cache server, so they must be the same server."); + } + + public override void Execute() + { + base.Execute(); + + _toFile ??= BasicActivator.IsInteractive + ? BasicActivator.SelectFile("Path to write build health board breakdown to", "Build health board breakdown", "*.csv") + : new FileInfo(Path.Combine(System.Environment.CurrentDirectory, $"{Sanitise(_cic.Name)}-build-healthboard.csv")); + + if (_toFile == null) + return; + + _cacheDb = _cic.QueryCachingServer.Discover(DataAccessContext.InternalDataProcessing); + _cacheManager = new CachedAggregateConfigurationResultsManager(_cic.QueryCachingServer); + _demogTable = _idEi.ColumnInfo.TableInfo.Name; + _demogId = _idEi.GetRuntimeName(); + _regionName = _regionEi.GetRuntimeName(); + + // 1. Build once: populates every per-set cache table and gives the baseline (unfiltered) counts. + var compiler = new CohortCompiler(BasicActivator, _cic) { IncludeCumulativeTotals = true }; + var runner = new CohortCompilerRunner(compiler, _timeout) { RunSubcontainers = true }; + runner.Run(new CancellationToken()); + + var crashed = compiler.Tasks.Keys.Where(t => t.State == CompilationState.Crashed).ToList(); + if (crashed.Any()) + { + SetImpossible($"{crashed.Count} task(s) crashed during the build - cannot produce a reliable breakdown"); + BasicActivator.Show($"Build failed: {crashed[0].CrashMessage?.Message}"); + return; + } + + foreach (var task in compiler.Tasks.Keys) + { + var isContainer = task switch + { + AggregationContainerTask => true, + AggregationTask => false, + _ => (bool?)null // skip joinables / plugin tasks + }; + if (isContainer == null || task.Child == null) + continue; + _baseline[(isContainer.Value, task.Child.ID)] = + (task.FinalRowCount, task.CumulativeRowCount); + } + + // 2. Walk the tree, recomposing each count point from the cache and splitting by region. + var nodes = new List(); + var seq = 0; + Walk(_cic.RootCohortAggregateContainer, null, 0, nodes, ref seq); + + CohortBuildHealthBoardBreakdownReport.WriteCsv(_toFile.FullName, nodes); + + // reconciliation note + var drift = nodes.Count(n => + n.FinalByRegion.Where(kv => HealthBoardLookup.Resolve(kv.Key).Node != HealthBoardLookup.UnknownNode) + .Sum(kv => kv.Value) > n.FinalUnfiltered); + var summary = $"Exported build health board breakdown to {_toFile.FullName} ({nodes.Count} count points)"; + if (drift > 0) + summary += $" - WARNING: {drift} node(s) have board counts exceeding the unfiltered total (check demography keys)"; + BasicActivator.Show(summary); + } + + private void Walk(CohortAggregateContainer container, CohortAggregateContainer parent, int indexInParent, + List nodes, ref int seq) + { + // container node row (cumulative is within its parent) + var (cFinal, cCum) = _baseline.TryGetValue((true, container.ID), out var cb) ? cb : (0, null); + IReadOnlyDictionary cCumByRegion = null; + if (parent != null && indexInParent > 0 && cCum.HasValue) + cCumByRegion = RunRegionCounts(CumulativeSql(parent, indexInParent)); + + nodes.Add(new CohortBuildHealthBoardBreakdownReport.NodeBreakdown + { + Seq = seq++, + Type = "Container", + Name = container.Name, + Container = parent?.Name ?? "", + SetOperation = container.Operation.ToString(), + DisplayOrder = container.Order, + FinalUnfiltered = cFinal, + CumulativeUnfiltered = parent != null && indexInParent > 0 ? cCum : null, + FinalByRegion = RunRegionCounts(IdSql(container)), + CumulativeByRegion = cCumByRegion + }); + + var kids = EnabledOrdered(container); + for (var i = 0; i < kids.Count; i++) + { + switch (kids[i]) + { + case AggregateConfiguration agg: + var (aFinal, aCum) = _baseline.TryGetValue((false, agg.ID), out var ab) ? ab : (0, null); + IReadOnlyDictionary aCumByRegion = null; + if (i > 0 && aCum.HasValue) + aCumByRegion = RunRegionCounts(CumulativeSql(container, i)); + + nodes.Add(new CohortBuildHealthBoardBreakdownReport.NodeBreakdown + { + Seq = seq++, + Type = "Cohort Set", + Name = agg.Name, + Container = container.Name, + SetOperation = "", + DisplayOrder = agg.Order, + FinalUnfiltered = aFinal, + CumulativeUnfiltered = i > 0 ? aCum : null, + FinalByRegion = RunRegionCounts(CachedSetSql(agg)), + CumulativeByRegion = aCumByRegion + }); + break; + + case CohortAggregateContainer sub: + Walk(sub, container, i, nodes, ref seq); + break; + } + } + } + + // --- identifier-list SQL composed purely from the per-set cache tables --- + + private string IdSql(IOrderable node) => node switch + { + AggregateConfiguration agg => CachedSetSql(agg), + CohortAggregateContainer c => Compose(c, EnabledOrdered(c)), + _ => throw new System.NotSupportedException(node.GetType().Name) + }; + + private string CumulativeSql(CohortAggregateContainer container, int upToInclusive) => + Compose(container, EnabledOrdered(container).Take(upToInclusive + 1).ToList()); + + private string Compose(CohortAggregateContainer container, IReadOnlyList children) + { + var op = $"\n{container.Operation}\n"; // UNION / INTERSECT / EXCEPT are valid SQL Server keywords + return string.Join(op, children.Select(ch => $"({IdSql(ch)})")); + } + + private string CachedSetSql(AggregateConfiguration agg) + { + if (!_setCacheTable.TryGetValue(agg.ID, out var t)) + { + var table = _cacheManager.GetLatestResultsTableUnsafe(agg, + AggregateOperation.IndexedExtractionIdentifierList) as DiscoveredTable; + if (table == null) + throw new System.Exception($"Cohort set '{agg.Name}' has no cached identifier list - the build did not cache it"); + var col = table.DiscoverColumns()[0].GetRuntimeName(); + t = (table.GetFullyQualifiedName(), col); + _setCacheTable[agg.ID] = t; + } + + return $"SELECT {t.col} AS id FROM {t.fqn}"; + } + + private List EnabledOrdered(CohortAggregateContainer container) => + container.GetOrderedContents().Where(o => o switch + { + AggregateConfiguration a => !a.IsDisabled, + CohortAggregateContainer c => !c.IsDisabled, + _ => true + }).ToList(); + + // --- run a GROUP BY Region join (on the cache server) for one count point --- + + private IReadOnlyDictionary RunRegionCounts(string idListSql) + { + var sql = + $"SELECT d.[{_regionName}] AS Region, COUNT(DISTINCT i.id) AS n\n" + + $"FROM (\n{idListSql}\n) i\n" + + $"INNER JOIN {_demogTable} d ON d.[{_demogId}] = i.id\n" + + $"GROUP BY d.[{_regionName}]"; + + var result = new Dictionary(System.StringComparer.OrdinalIgnoreCase); + using var con = _cacheDb.Server.GetConnection(); + con.Open(); + using var cmd = _cacheDb.Server.GetCommand(sql, con); + cmd.CommandTimeout = _timeout; + using var r = cmd.ExecuteReader(); + while (r.Read()) + { + if (r["Region"] == System.DBNull.Value) + continue; // NULL region folds into Unknown via baseline subtraction + result[r["Region"].ToString()] = System.Convert.ToInt32(r["n"]); + } + + return result; + } + + private static string Sanitise(string name) + { + foreach (var c in Path.GetInvalidFileNameChars()) + name = name.Replace(c, '_'); + return name; + } +} diff --git a/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortHealthBoardBreakdown.cs b/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortHealthBoardBreakdown.cs new file mode 100644 index 0000000000..ce16a28440 --- /dev/null +++ b/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortHealthBoardBreakdown.cs @@ -0,0 +1,252 @@ +// Copyright (c) The University of Dundee 2024-2024 +// This file is part of the Research Data Management Platform (RDMP). +// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. +// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// You should have received a copy of the GNU General Public License along with RDMP. If not, see . + +using System.Data; +using System.IO; +using System.Linq; +using Rdmp.Core.CohortCreation; +using Rdmp.Core.Curation.Data; +using Rdmp.Core.Curation.Data.Cohort; +using Rdmp.Core.DataExport.Data; +using Rdmp.Core.QueryBuilding; +using Rdmp.Core.ReusableLibraryCode.DataAccess; + +namespace Rdmp.Core.CommandExecution.AtomicCommands; + +/// +/// Shared logic for breaking a cohort's final inclusion list down by Scottish health board: joins +/// the cohort's patient identifiers to a demography catalogue's region column and writes a +/// per-board / per-node COUNT(DISTINCT chi) to CSV (see +/// and ). Concrete subclasses supply the cohort identifier list from +/// either a committed or a live +/// . +/// +public abstract class ExecuteCommandExportHealthBoardBreakdownBase : BasicCommandExecution +{ + protected readonly string DemographyCatalogue; + protected readonly string RegionColumn; + protected readonly int Timeout; + protected FileInfo ToFile; + + private ExtractionInformation _regionEi; + private ExtractionInformation _idEi; + + protected ExecuteCommandExportHealthBoardBreakdownBase(IBasicActivateItems activator, + FileInfo toFile, string demographyCatalogue, string regionColumn, int timeout) : base(activator) + { + DemographyCatalogue = demographyCatalogue; + RegionColumn = regionColumn; + Timeout = timeout; + ToFile = toFile; + } + + /// A short name for the cohort source, used for the default output file name. + protected abstract string SourceName { get; } + + /// + /// Produces the cohort identifier sub-query, any parameter DECLARE SQL that must be hoisted to + /// the front of the batch, and the cohort's own distinct patient count (for reconciliation). + /// + protected abstract (string subquery, string paramSql, int distinctCount) GetCohortIdentifierSql(); + + /// + /// Resolves the demography catalogue's region + identifier columns and marks the command + /// impossible if anything is missing. Call from each subclass constructor once its cohort input + /// has been validated. + /// + protected void ResolveDemography() + { + var demography = BasicActivator.RepositoryLocator.CatalogueRepository + .GetAllObjects() + .FirstOrDefault(c => string.Equals(c.Name, DemographyCatalogue, System.StringComparison.OrdinalIgnoreCase)); + + if (demography == null) + { + SetImpossible($"Could not find a catalogue called '{DemographyCatalogue}'"); + return; + } + + var eis = demography.GetAllExtractionInformation(ExtractionCategory.Any); + _regionEi = eis.FirstOrDefault(e => + string.Equals(e.GetRuntimeName(), RegionColumn, System.StringComparison.OrdinalIgnoreCase)); + _idEi = eis.FirstOrDefault(e => e.IsExtractionIdentifier); + + if (_regionEi == null) + SetImpossible($"'{DemographyCatalogue}' has no column called '{RegionColumn}'"); + else if (_idEi == null) + SetImpossible($"'{DemographyCatalogue}' has no IsExtractionIdentifier column to join the cohort on"); + } + + public override void Execute() + { + base.Execute(); + + ToFile ??= BasicActivator.IsInteractive + ? BasicActivator.SelectFile("Path to write health board breakdown to", "Health board breakdown", "*.csv") + : new FileInfo(Path.Combine(System.Environment.CurrentDirectory, $"{Sanitise(SourceName)}-healthboard.csv")); + + if (ToFile == null) + return; + + // The cohort identifier list differs by input type; everything downstream is identical. + var (cohortIdSubquery, paramSql, cohortDistinct) = GetCohortIdentifierSql(); + + var sql = paramSql + BuildBreakdownSql( + regionSelect: _regionEi.SelectSQL ?? _regionEi.ColumnInfo.Name, + chiSelect: _idEi.SelectSQL ?? _idEi.ColumnInfo.Name, + demographyTable: _idEi.ColumnInfo.TableInfo.Name, + cohortIdSubquery: cohortIdSubquery); + + // Run on the demography server; cohort tables are on the same server (3-part names resolve). + var db = DataAccessPortal.ExpectDatabase(_idEi.ColumnInfo.TableInfo, DataAccessContext.InternalDataProcessing); + + var dt = new DataTable(); + using (var con = db.Server.GetConnection()) + { + con.Open(); + using var cmd = db.Server.GetCommand(sql, con); + cmd.CommandTimeout = Timeout; + using var da = db.Server.GetDataAdapter(cmd); + da.Fill(dt); + } + + var records = HealthBoardBreakdownReport.BuildRecords(dt, "Region", "n"); + File.WriteAllText(ToFile.FullName, HealthBoardBreakdownReport.ToCsv(records)); + + var inBreakdown = records + .Where(r => r.Kind == HealthBoardBreakdownReport.RowKind.GrandTotal) + .Select(r => r.Count) + .FirstOrDefault(); + + // Reconcile against the cohort's own distinct count: any shortfall = patients absent from + // the demography catalogue (those never reach the GROUP BY join). + var summary = $"Exported health board breakdown to {ToFile.FullName} ({inBreakdown} of {cohortDistinct} patients matched in '{DemographyCatalogue}'"; + var missing = cohortDistinct - inBreakdown; + summary += missing > 0 + ? $"; {missing} not found in the demography catalogue)" + : ")"; + + BasicActivator.Show(summary); + } + + /// Runs a count of the cohort identifier list (with hoisted parameters). + protected int CountCohort(string paramSql, string body) + { + var db = DataAccessPortal.ExpectDatabase(_idEi.ColumnInfo.TableInfo, DataAccessContext.InternalDataProcessing); + using var con = db.Server.GetConnection(); + con.Open(); + using var cmd = db.Server.GetCommand($"{paramSql}SELECT count(*) FROM ({body}) _hb_recon", con); + cmd.CommandTimeout = Timeout; + return System.Convert.ToInt32(cmd.ExecuteScalar()); + } + + /// + /// Composes the per-region count query (pure string; unit-testable without a database). + /// + public static string BuildBreakdownSql(string regionSelect, string chiSelect, string demographyTable, + string cohortIdSubquery) => + $""" + SELECT {regionSelect} AS Region, + COUNT(DISTINCT {chiSelect}) AS n + FROM {demographyTable} + WHERE {chiSelect} IN ( + {cohortIdSubquery} + ) + GROUP BY {regionSelect} + """; + + protected static string Sanitise(string name) + { + foreach (var c in Path.GetInvalidFileNameChars()) + name = name.Replace(c, '_'); + return name; + } +} + +/// +/// Breaks a committed 's released inclusion list down by health board. +/// +public class ExecuteCommandExportCohortHealthBoardBreakdown : ExecuteCommandExportHealthBoardBreakdownBase +{ + private readonly ExtractableCohort _cohort; + + public ExecuteCommandExportCohortHealthBoardBreakdown(IBasicActivateItems activator, + [DemandsInitialization("The committed cohort whose final inclusion list to break down")] + ExtractableCohort cohort, + [DemandsInitialization("CSV file to write. Defaults to -healthboard.csv in the current directory")] + FileInfo toFile = null, + [DemandsInitialization("Name of the demography catalogue holding the region column", DefaultValue = "SHARE_Demography")] + string demographyCatalogue = "SHARE_Demography", + [DemandsInitialization("Name of the region (health board cipher) column on the demography catalogue", DefaultValue = "Region")] + string regionColumn = "Region", + [DemandsInitialization("Per-query command timeout in seconds", DefaultValue = 5000)] + int timeout = 5000) : base(activator, toFile, demographyCatalogue, regionColumn, timeout) + { + _cohort = cohort; + + if (_cohort == null) + SetImpossible("No cohort was supplied"); + else + ResolveDemography(); + } + + protected override string SourceName => _cohort.ToString(); + + protected override (string subquery, string paramSql, int distinctCount) GetCohortIdentifierSql() + { + // Committed cohort: a flat table. Fully qualify it (usually a different database on the same + // server); WhereSQL() is already database-qualified. + var cohortTable = _cohort.ExternalCohortTable.DiscoverCohortTable().GetFullyQualifiedName(); + var subquery = $"SELECT {_cohort.GetPrivateIdentifier(true)} FROM {cohortTable} WHERE {_cohort.WhereSQL()}"; + return (subquery, "", _cohort.GetCountDistinctFromDatabase(Timeout)); + } +} + +/// +/// Breaks a live 's build query down by health board. +/// +public class ExecuteCommandExportCicHealthBoardBreakdown : ExecuteCommandExportHealthBoardBreakdownBase +{ + private readonly CohortIdentificationConfiguration _cic; + + public ExecuteCommandExportCicHealthBoardBreakdown(IBasicActivateItems activator, + [DemandsInitialization("The cohort identification configuration whose final inclusion list to break down")] + CohortIdentificationConfiguration cic, + [DemandsInitialization("CSV file to write. Defaults to -healthboard.csv in the current directory")] + FileInfo toFile = null, + [DemandsInitialization("Name of the demography catalogue holding the region column", DefaultValue = "SHARE_Demography")] + string demographyCatalogue = "SHARE_Demography", + [DemandsInitialization("Name of the region (health board cipher) column on the demography catalogue", DefaultValue = "Region")] + string regionColumn = "Region", + [DemandsInitialization("Per-query command timeout in seconds", DefaultValue = 5000)] + int timeout = 5000) : base(activator, toFile, demographyCatalogue, regionColumn, timeout) + { + _cic = cic; + + if (_cic == null) + SetImpossible("No CohortIdentificationConfiguration was supplied"); + else if (_cic.RootCohortAggregateContainer_ID == null) + SetImpossible($"'{_cic}' has no root container to run"); + else + ResolveDemography(); + } + + protected override string SourceName => _cic.Name; + + protected override (string subquery, string paramSql, int distinctCount) GetCohortIdentifierSql() + { + // CIC: inline the live build query. DoNotWriteOutParameters keeps the DECLAREs out of the + // SELECT so we can hoist them to the front of the whole batch. + var builder = new CohortQueryBuilder(_cic, null) { DoNotWriteOutParameters = true }; + var body = builder.SQL; + var paramSql = string.Concat(builder.ParameterManager.GetFinalResolvedParametersList() + .Select(QueryBuilder.GetParameterDeclarationSQL)); + + // distinct count = number of identifiers the build query yields (its output is already the + // distinct identifier list); reuse the same hoisted parameters. + return (body, paramSql, CountCohort(paramSql, body)); + } +} From 72bce6f5f7c9f98099fc2f15e4c7ae2545d4d2d2 Mon Sep 17 00:00:00 2001 From: mtinti Date: Sat, 27 Jun 2026 09:03:13 +0100 Subject: [PATCH 2/4] Make cohort-build health board breakdown report wide (horizontal) Reshape the build-breakdown output from long to a wide matrix: one row per (count-point x metric), container/set name once, a Total column (RDMP's own national count), one column per Scottish health board, plus an Other column (present non-Scottish / unmapped region codes, previously hidden inside Unknown), a NotKnown residual (not-in-demography + NULL region), a Metric column (Final + Cumulative) and a bottom "% of final cohort" row. Boards + Other + NotKnown reconcile to Total on every row. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_0169JCnaL3fhhZjseDx2XXT2 --- .../CohortBuildHealthBoardBreakdownTests.cs | 174 +++++++++------- .../CohortBuildHealthBoardBreakdownReport.cs | 193 +++++++++--------- 2 files changed, 198 insertions(+), 169 deletions(-) diff --git a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs index 1138c77e8d..0a4cd65af3 100644 --- a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs +++ b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs @@ -115,59 +115,51 @@ public void BuildBreakdown_Fixture_NationalAndThreeBoards() Assert.That(cmd.IsImpossible, Is.False, cmd.ReasonCommandImpossible); cmd.Execute(); - var rows = ParseLong(File.ReadAllText(file.FullName)); - - LongRow Row(string board, string token) => - rows.Single(r => r.Board == board && r.Name.Contains(token, StringComparison.Ordinal)); + var w = ParseWide(File.ReadAllText(file.FullName)); const string T = "Tayside", G = "Greater Glasgow & Clyde", F = "Fife"; Assert.Multiple(() => { - // unfiltered tree (must equal the UI / RDMP's own counts) - Assert.That(Row("Unfiltered", "Root").Final, Is.EqualTo(58)); - Assert.That(Row("Unfiltered", "Inclusion").Final, Is.EqualTo(100)); - Assert.That(Row("Unfiltered", "Registry").Final, Is.EqualTo(120)); - Assert.That(Row("Unfiltered", "Demography").Final, Is.EqualTo(100)); - Assert.That(Row("Unfiltered", "Demography").Cum, Is.EqualTo(100)); // INTERSECT cumulative - Assert.That(Row("Unfiltered", "Excl1").Cum, Is.EqualTo(80)); - Assert.That(Row("Unfiltered", "Excl2").Cum, Is.EqualTo(65)); - Assert.That(Row("Unfiltered", "Excl3").Cum, Is.EqualTo(60)); - Assert.That(Row("Unfiltered", "Excl4").Cum, Is.EqualTo(58)); - - // Tayside cumulative through the tree - Assert.That(Row(T, "Inclusion").Final, Is.EqualTo(50)); - Assert.That(Row(T, "Excl1").Cum, Is.EqualTo(40)); - Assert.That(Row(T, "Excl2").Cum, Is.EqualTo(32)); - Assert.That(Row(T, "Excl3").Cum, Is.EqualTo(30)); - Assert.That(Row(T, "Excl4").Cum, Is.EqualTo(29)); - Assert.That(Row(T, "Root").Final, Is.EqualTo(29)); + // Total column == RDMP's own (national, non-breakdown) counts + Assert.That(w.Cell("Root", "Final", "Total"), Is.EqualTo(58)); + Assert.That(w.Cell("Inclusion", "Final", "Total"), Is.EqualTo(100)); + Assert.That(w.Cell("Registry", "Final", "Total"), Is.EqualTo(120)); + Assert.That(w.Cell("Demography", "Final", "Total"), Is.EqualTo(100)); + Assert.That(w.Cell("Demography", "Cumulative", "Total"), Is.EqualTo(100)); // INTERSECT cumulative + Assert.That(w.Cell("Excl1", "Cumulative", "Total"), Is.EqualTo(80)); + Assert.That(w.Cell("Excl2", "Cumulative", "Total"), Is.EqualTo(65)); + Assert.That(w.Cell("Excl3", "Cumulative", "Total"), Is.EqualTo(60)); + Assert.That(w.Cell("Excl4", "Cumulative", "Total"), Is.EqualTo(58)); + + // Tayside running total down the EXCEPT chain + Assert.That(w.Cell("Inclusion", "Final", T), Is.EqualTo(50)); + Assert.That(w.Cell("Excl1", "Cumulative", T), Is.EqualTo(40)); + Assert.That(w.Cell("Excl2", "Cumulative", T), Is.EqualTo(32)); + Assert.That(w.Cell("Excl3", "Cumulative", T), Is.EqualTo(30)); + Assert.That(w.Cell("Excl4", "Cumulative", T), Is.EqualTo(29)); + Assert.That(w.Cell("Root", "Final", T), Is.EqualTo(29)); // Glasgow + Fife endpoints - Assert.That(Row(G, "Root").Final, Is.EqualTo(17)); - Assert.That(Row(G, "Excl4").Cum, Is.EqualTo(17)); - Assert.That(Row(F, "Root").Final, Is.EqualTo(12)); - Assert.That(Row(F, "Excl4").Cum, Is.EqualTo(12)); - - // the 20 registry-only people (no demography row) land in Unknown on the Registry set - Assert.That(Row("Unknown", "Registry").Final, Is.EqualTo(20)); + Assert.That(w.Cell("Root", "Final", G), Is.EqualTo(17)); + Assert.That(w.Cell("Excl4", "Cumulative", G), Is.EqualTo(17)); + Assert.That(w.Cell("Root", "Final", F), Is.EqualTo(12)); + Assert.That(w.Cell("Excl4", "Cumulative", F), Is.EqualTo(12)); + + // 20 registry-only people (no demography row) -> NotKnown; Other stays 0 (no non-Scottish codes) + Assert.That(w.Cell("Registry", "Final", "NotKnown"), Is.EqualTo(20)); + Assert.That(w.Cell("Registry", "Final", "Other"), Is.EqualTo(0)); + + // % of final cohort row (root final = 58) + Assert.That(w.Percent(T), Is.EqualTo("50.0")); + Assert.That(w.Percent(G), Is.EqualTo("29.3")); + Assert.That(w.Percent(F), Is.EqualTo("20.7")); }); - // ---- partition: at EVERY node, the boards (+Unknown) sum to the unfiltered total ---- - foreach (var node in rows.GroupBy(r => (r.Name, r.Order, r.Container))) - { - var unfiltered = node.Single(r => r.Board == "Unfiltered"); - var boardFinal = node.Where(r => r.Board != "Unfiltered").Sum(r => r.Final ?? 0); - Assert.That(boardFinal, Is.EqualTo(unfiltered.Final), - $"Final partition mismatch at node '{unfiltered.Name}'"); - - if (unfiltered.Cum.HasValue) - { - var boardCum = node.Where(r => r.Board != "Unfiltered").Sum(r => r.Cum ?? 0); - Assert.That(boardCum, Is.EqualTo(unfiltered.Cum), - $"Cumulative partition mismatch at node '{unfiltered.Name}'"); - } - } + // ---- partition: every data row's columns after Total sum back to Total ---- + foreach (var (name, metric) in w.Keys) + Assert.That(w.SumAfterTotal(name, metric), Is.EqualTo(w.Cell(name, metric, "Total")), + $"partition mismatch at {name}/{metric}"); } finally { @@ -176,36 +168,48 @@ LongRow Row(string board, string token) => } [Test] - public void Report_BuildRows_UnfilteredFirst_UnknownDerived_BoardMajor() + public void Report_Split_SeparatesScottishOtherAndNotKnown() + { + // Total 100; T=50, G=30 (Scottish), X=15 (present but not a Scottish board) -> Other 15; + // NotKnown = 100 - 80 - 15 = 5 (not in demography / null region) + var b = CohortBuildHealthBoardBreakdownReport.Split(100, + new Dictionary { ["T"] = 50, ["G"] = 30, ["X"] = 15 }); + + Assert.Multiple(() => + { + Assert.That(b.Total, Is.EqualTo(100)); + Assert.That(b.Boards["T"], Is.EqualTo(50)); + Assert.That(b.Boards["G"], Is.EqualTo(30)); + Assert.That(b.Boards.ContainsKey("X"), Is.False); // non-Scottish code is NOT a board + Assert.That(b.Other, Is.EqualTo(15)); // it lands in Other + Assert.That(b.NotKnown, Is.EqualTo(5)); // residual + Assert.That(b.Boards.Values.Sum() + b.Other + b.NotKnown, Is.EqualTo(b.Total)); + }); + } + + [Test] + public void Report_ToCsv_WideHeaderAndMetricRows() { - // one node: unfiltered 100, T=50 G=30 (known), so Unknown = 100 - 80 = 20 var nodes = new List { new() { - Seq = 0, Type = "Cohort Set", Name = "Set1", Container = "Root", SetOperation = "", - FinalUnfiltered = 100, CumulativeUnfiltered = null, + Seq = 0, Type = "Container", Name = "Root", Container = "", SetOperation = "EXCEPT", + FinalUnfiltered = 80, CumulativeUnfiltered = null, FinalByRegion = new Dictionary { ["T"] = 50, ["G"] = 30 } } }; - var rows = CohortBuildHealthBoardBreakdownReport.BuildRows(nodes); + var csv = CohortBuildHealthBoardBreakdownReport.ToCsv(nodes); + var header = csv.Split('\n')[0].Trim(); Assert.Multiple(() => { - Assert.That(rows[0].Board, Is.EqualTo("Unfiltered")); - Assert.That(rows[0].FinalCount, Is.EqualTo(100)); - Assert.That(rows.Single(r => r.Board == "Tayside").FinalCount, Is.EqualTo(50)); - Assert.That(rows.Single(r => r.Board == "Greater Glasgow & Clyde").FinalCount, Is.EqualTo(30)); - Assert.That(rows.Last().Board, Is.EqualTo("Unknown")); - Assert.That(rows.Last().FinalCount, Is.EqualTo(20)); // 100 - (50+30) - // partition holds - Assert.That(rows.Where(r => r.Board != "Unfiltered").Sum(r => r.FinalCount ?? 0), Is.EqualTo(100)); + Assert.That(header, Does.StartWith("Order,Type,Name,Container,SetOperation,Metric,Total,")); + Assert.That(header, Does.Contain("Tayside")); + Assert.That(header, Does.EndWith("Other,NotKnown")); + Assert.That(csv, Does.Contain("% of final cohort")); }); - - var csv = CohortBuildHealthBoardBreakdownReport.ToCsv(rows); - Assert.That(csv.Split('\n')[0].Trim(), - Is.EqualTo("Board,Node,Order,Type,Name,Container,SetOperation,FinalCount,CumulativeCount")); } private static DataTable OneCol(string col, IEnumerable values) @@ -231,19 +235,47 @@ private AggregateConfiguration MakeSet(DiscoveredDatabase db, string name, DataT return agg; } - private sealed record LongRow(string Board, string Node, int Order, string Type, string Name, - string Container, string SetOp, int? Final, int? Cum); - - private static List ParseLong(string csv) + /// Parsed wide CSV: data rows keyed by (Name-token, Metric), plus the percent row. + private sealed class Wide { - var rows = new List(); - foreach (var line in csv.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries).Skip(1)) + private readonly Dictionary _col; + private readonly List _data = new(); + private string[] _percent; + + public Wide(string csv) + { + var lines = csv.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries); + var header = lines[0].Split(','); + _col = header.Select((h, i) => (h, i)).ToDictionary(x => x.h, x => x.i); + foreach (var line in lines.Skip(1)) + { + var c = line.Split(','); + if (c[_col["Metric"]] == CohortBuildHealthBoardBreakdownReport.PercentMetric) + _percent = c; + else + _data.Add(c); + } + } + + public IEnumerable<(string name, string metric)> Keys => + _data.Select(c => (c[_col["Name"]], c[_col["Metric"]])); + + private string[] Find(string nameToken, string metric) => + _data.Single(c => c[_col["Name"]].Contains(nameToken, StringComparison.Ordinal) + && c[_col["Metric"]] == metric); + + public int Cell(string nameToken, string metric, string column) => + int.Parse(Find(nameToken, metric)[_col[column]]); + + /// Sum of every column after Total (boards + Other + NotKnown) for a row. + public int SumAfterTotal(string name, string metric) { - var c = line.Split(','); - int? N(string s) => string.IsNullOrEmpty(s) ? null : int.Parse(s); - rows.Add(new LongRow(c[0], c[1], int.Parse(c[2]), c[3], c[4], c[5], c[6], N(c[7]), N(c[8]))); + var row = _data.Single(c => c[_col["Name"]] == name && c[_col["Metric"]] == metric); + return Enumerable.Range(_col["Total"] + 1, row.Length - _col["Total"] - 1).Sum(i => int.Parse(row[i])); } - return rows; + public string Percent(string column) => _percent[_col[column]]; } + + private static Wide ParseWide(string csv) => new(csv); } diff --git a/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs b/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs index b37113e835..0fe3ef6059 100644 --- a/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs +++ b/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs @@ -14,79 +14,88 @@ namespace Rdmp.Core.CohortCreation; /// /// Projects a cohort build's count tree (the per-set / per-container FinalCount and cumulative -/// running totals shown in the Cohort Builder) split by health board into a long, file-friendly CSV: -/// one row per (board × count-point). The boards partition the cohort (1 patient ↔ 1 board), so an -/// Unfiltered pseudo-board (RDMP's own counts) and an Unknown board (patients with no / -/// unmapped region) bracket the real boards and reconcile to the unfiltered total at every node. +/// running totals shown in the Cohort Builder) split by health board into a WIDE CSV: one row per +/// (count-point × metric), the container/set name written once, a Total column (RDMP's own +/// national count), one column per Scottish health board, an Other column (present non-Scottish / +/// unmapped region codes) and a NotKnown residual (patients not in demography / NULL region). +/// A bottom % of final cohort row gives each board's share of the final national cohort. +/// Boards + Other + NotKnown reconcile to Total on every row. /// public static class CohortBuildHealthBoardBreakdownReport { - public const string UnfilteredBoard = "Unfiltered"; - public const string UnknownBoard = "Unknown"; + public const string OtherColumn = "Other"; + public const string NotKnownColumn = "NotKnown"; + public const string PercentMetric = "% of final cohort"; /// One count point of the build tree with its per-region counts (known boards only). public sealed class NodeBreakdown { - /// Tree walk order (stable, used to order rows within a board). public int Seq { get; init; } - - /// "Cohort Set" or "Container". public string Type { get; init; } = ""; - public string Name { get; init; } = ""; - /// Name of the parent container (empty for the root). + /// Parent container name (empty for the root). public string Container { get; init; } = ""; - /// UNION / INTERSECT / EXCEPT for containers; empty for sets. public string SetOperation { get; init; } = ""; - public int DisplayOrder { get; init; } - /// RDMP's own count for this node (the unfiltered total). + /// RDMP's own count for this node (the unfiltered/national total). public int FinalUnfiltered { get; init; } /// RDMP's own cumulative within the parent container; null if not applicable. public int? CumulativeUnfiltered { get; init; } - /// Region cipher → final count (known boards only; GROUP BY Region result). + /// Region cipher → final count (every present code; GROUP BY Region result). public IReadOnlyDictionary FinalByRegion { get; init; } = new Dictionary(); /// Region cipher → cumulative count; null when this node has no cumulative. public IReadOnlyDictionary CumulativeByRegion { get; init; } } - public sealed class Row + /// The Total / per-board / Other / NotKnown counts for one node+metric. + public sealed class Buckets { - public string Board { get; init; } = ""; - public string Node { get; init; } = ""; - public int Order { get; init; } - public string Type { get; init; } = ""; - public string Name { get; init; } = ""; - public string Container { get; init; } = ""; - public string SetOperation { get; init; } = ""; - public int? FinalCount { get; init; } - public int? CumulativeCount { get; init; } - } + public int Total { get; init; } - private static readonly string[] Header = - { - "Board", "Node", "Order", "Type", "Name", "Container", "SetOperation", "FinalCount", "CumulativeCount" - }; + /// Region cipher → count (mapped Scottish boards only). + public IReadOnlyDictionary Boards { get; init; } = new Dictionary(); + + /// Sum of present region codes that are NOT one of the 15 Scottish boards. + public int Other { get; init; } + + /// Total − boards − Other = not-in-demography + NULL region. + public int NotKnown { get; init; } + } /// - /// Flattens the per-node breakdowns into long rows, board-major: the - /// tree first, then each real board's full tree (every board appears at every node, 0 where absent), - /// then the tree last. Within a board, rows follow the tree walk order. + /// Splits one node's region counts into Total / mapped-boards / Other / NotKnown. + /// is the GROUP BY Region result (every present code); is RDMP's own count. /// - public static IReadOnlyList BuildRows(IReadOnlyList nodes) + public static Buckets Split(int total, IReadOnlyDictionary byRegion) { - var ordered = nodes.OrderBy(n => n.Seq).ToList(); + var boards = new Dictionary(System.StringComparer.OrdinalIgnoreCase); + var other = 0; + foreach (var (code, n) in byRegion) + if (HealthBoardLookup.Resolve(code).Node == HealthBoardLookup.UnknownNode) + other += n; // present but not a Scottish board (non-Scottish / unmapped) + else + boards[code] = n; + + return new Buckets + { + Total = total, + Boards = boards, + Other = other, + NotKnown = total - boards.Values.Sum() - other + }; + } - // every region cipher seen anywhere → its board; keep a deterministic board ordering - var boards = ordered - .SelectMany(n => n.FinalByRegion.Keys) - .Select(r => HealthBoardLookup.Resolve(r)) + /// The ordered mapped boards that appear anywhere (column order: node then name). + private static List BoardColumns(IEnumerable nodes) => + nodes + .SelectMany(n => n.FinalByRegion.Keys.Concat(n.CumulativeByRegion?.Keys ?? Enumerable.Empty())) + .Select(HealthBoardLookup.Resolve) .Where(b => b.Node != HealthBoardLookup.UnknownNode) .GroupBy(b => b.Region, System.StringComparer.OrdinalIgnoreCase) .Select(g => g.First()) @@ -94,77 +103,65 @@ public static IReadOnlyList BuildRows(IReadOnlyList nodes) .ThenBy(b => b.Name, System.StringComparer.OrdinalIgnoreCase) .ToList(); - var rows = new List(); + /// Builds the wide CSV (data rows per node+metric, then a % of final cohort row). + public static string ToCsv(IReadOnlyList nodes) + { + var ordered = nodes.OrderBy(n => n.Seq).ToList(); + var boards = BoardColumns(ordered); + + var header = new List { "Order", "Type", "Name", "Container", "SetOperation", "Metric", "Total" }; + header.AddRange(boards.Select(b => b.Name)); + header.Add(OtherColumn); + header.Add(NotKnownColumn); + + var sb = new StringBuilder(); + sb.AppendLine(string.Join(",", header.Select(Escape))); - // 1) Unfiltered (RDMP's own counts) - foreach (var n in ordered) - rows.Add(MakeRow(n, UnfilteredBoard, "", n.FinalUnfiltered, n.CumulativeUnfiltered)); - - // 2) each real board - foreach (var b in boards) - foreach (var n in ordered) - { - var fin = n.FinalByRegion.TryGetValue(b.Region, out var f) ? f : 0; - int? cum = n.CumulativeByRegion == null - ? null - : n.CumulativeByRegion.TryGetValue(b.Region, out var c) ? c : 0; - rows.Add(MakeRow(n, b.Name, b.Node, fin, cum)); - } - - // 3) Unknown = unfiltered − Σ known boards (patients not in demography / unmapped region) foreach (var n in ordered) { - var knownFinal = n.FinalByRegion - .Where(kv => HealthBoardLookup.Resolve(kv.Key).Node != HealthBoardLookup.UnknownNode) - .Sum(kv => kv.Value); - var finalUnknown = n.FinalUnfiltered - knownFinal; - - int? cumUnknown = null; + AppendCountRow(sb, n, boards, "Final", Split(n.FinalUnfiltered, n.FinalByRegion)); if (n.CumulativeUnfiltered.HasValue && n.CumulativeByRegion != null) - { - var knownCum = n.CumulativeByRegion - .Where(kv => HealthBoardLookup.Resolve(kv.Key).Node != HealthBoardLookup.UnknownNode) - .Sum(kv => kv.Value); - cumUnknown = n.CumulativeUnfiltered.Value - knownCum; - } - - rows.Add(MakeRow(n, UnknownBoard, UnknownBoard, finalUnknown, cumUnknown)); + AppendCountRow(sb, n, boards, "Cumulative", + Split(n.CumulativeUnfiltered.Value, n.CumulativeByRegion)); } - return rows; + // bottom: % of final cohort (root node's Final), after a blank separator + var root = ordered.FirstOrDefault(n => string.IsNullOrEmpty(n.Container)) ?? ordered.FirstOrDefault(); + if (root != null && root.FinalUnfiltered > 0) + { + sb.AppendLine(); + var b = Split(root.FinalUnfiltered, root.FinalByRegion); + double Pct(int v) => v * 100.0 / b.Total; + var cells = new List { "", "", PercentMetric, "", "", PercentMetric, Fmt(100.0) }; + cells.AddRange(boards.Select(bd => Fmt(Pct(b.Boards.TryGetValue(bd.Region, out var v) ? v : 0)))); + cells.Add(Fmt(Pct(b.Other))); + cells.Add(Fmt(Pct(b.NotKnown))); + sb.AppendLine(string.Join(",", cells.Select(Escape))); + } + + return sb.ToString(); } - private static Row MakeRow(NodeBreakdown n, string board, string node, int? final, int? cumulative) => new() + private static void AppendCountRow(StringBuilder sb, NodeBreakdown n, List boards, + string metric, Buckets b) { - Board = board, - Node = node, - Order = n.DisplayOrder, - Type = n.Type, - Name = n.Name, - Container = n.Container, - SetOperation = n.SetOperation, - FinalCount = final, - CumulativeCount = cumulative - }; - - public static string ToCsv(IEnumerable rows) - { - var sb = new StringBuilder(); - sb.AppendLine(string.Join(",", Header.Select(Escape))); - foreach (var r in rows) - sb.AppendLine(string.Join(",", new[] - { - r.Board, r.Node, - r.Order.ToString(CultureInfo.InvariantCulture), - r.Type, r.Name, r.Container, r.SetOperation, - r.FinalCount?.ToString(CultureInfo.InvariantCulture) ?? "", - r.CumulativeCount?.ToString(CultureInfo.InvariantCulture) ?? "" - }.Select(Escape))); - return sb.ToString(); + var cells = new List + { + n.DisplayOrder.ToString(CultureInfo.InvariantCulture), + n.Type, n.Name, n.Container, n.SetOperation, metric, + b.Total.ToString(CultureInfo.InvariantCulture) + }; + cells.AddRange(boards.Select(bd => + (b.Boards.TryGetValue(bd.Region, out var v) ? v : 0).ToString(CultureInfo.InvariantCulture))); + cells.Add(b.Other.ToString(CultureInfo.InvariantCulture)); + cells.Add(b.NotKnown.ToString(CultureInfo.InvariantCulture)); + sb.AppendLine(string.Join(",", cells.Select(Escape))); } public static void WriteCsv(string path, IReadOnlyList nodes) => - File.WriteAllText(path, ToCsv(BuildRows(nodes))); + File.WriteAllText(path, ToCsv(nodes)); + + private static string Fmt(double d) => d.ToString("0.0", CultureInfo.InvariantCulture); private static string Escape(string field) { From 56503de74e148049b454e05c35e1d961d7e8bc09 Mon Sep 17 00:00:00 2001 From: mtinti Date: Sat, 27 Jun 2026 11:29:14 +0100 Subject: [PATCH 3/4] Show clean container/set names in build breakdown (strip cic_ prefixes) RDMP prefixes cohort set names with cic__ and cloning across CICs stacks them (e.g. cic_18286_cic_18284_cic_17950_People in SHARE...). The build breakdown now strips those prefixes for display, so the report shows the real name (e.g. "People in SHARE Current For Contact [Recruitment]", "Excl Grp 1 and 2"). Adds a unit test for the cleaner, moves the no-DB report tests into a plain (non-database) fixture, and adds a Retry + clear guard on the DB integration test to absorb transient local-SQL errors. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_0169JCnaL3fhhZjseDx2XXT2 --- .../CohortBuildHealthBoardBreakdownTests.cs | 117 +++++++++++------- ...ndExportCohortBuildHealthBoardBreakdown.cs | 15 ++- 2 files changed, 83 insertions(+), 49 deletions(-) diff --git a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs index 0a4cd65af3..60a79bfe77 100644 --- a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs +++ b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs @@ -39,6 +39,7 @@ private static IEnumerable Ids(int from, int to) => Enumerable.Range(from, to - from + 1).Select(i => $"P{i:000}"); [Test] + [Retry(3)] // DB integration test: absorb the occasional transient SQL error under local load public void BuildBreakdown_Fixture_NationalAndThreeBoards() { var db = GetCleanedServer(DatabaseType.MicrosoftSQLServer); @@ -115,6 +116,12 @@ public void BuildBreakdown_Fixture_NationalAndThreeBoards() Assert.That(cmd.IsImpossible, Is.False, cmd.ReasonCommandImpossible); cmd.Execute(); + // the build can be marked impossible mid-Execute if a task crashed (transient DB issue); + // surface that clearly instead of failing later on an empty file + Assert.That(cmd.IsImpossible, Is.False, $"build did not complete: {cmd.ReasonCommandImpossible}"); + Assert.That(file.Exists && new FileInfo(file.FullName).Length > 0, Is.True, + "no breakdown was written (the cohort build did not finish)"); + var w = ParseWide(File.ReadAllText(file.FullName)); const string T = "Tayside", G = "Greater Glasgow & Clyde", F = "Fife"; @@ -167,51 +174,6 @@ public void BuildBreakdown_Fixture_NationalAndThreeBoards() } } - [Test] - public void Report_Split_SeparatesScottishOtherAndNotKnown() - { - // Total 100; T=50, G=30 (Scottish), X=15 (present but not a Scottish board) -> Other 15; - // NotKnown = 100 - 80 - 15 = 5 (not in demography / null region) - var b = CohortBuildHealthBoardBreakdownReport.Split(100, - new Dictionary { ["T"] = 50, ["G"] = 30, ["X"] = 15 }); - - Assert.Multiple(() => - { - Assert.That(b.Total, Is.EqualTo(100)); - Assert.That(b.Boards["T"], Is.EqualTo(50)); - Assert.That(b.Boards["G"], Is.EqualTo(30)); - Assert.That(b.Boards.ContainsKey("X"), Is.False); // non-Scottish code is NOT a board - Assert.That(b.Other, Is.EqualTo(15)); // it lands in Other - Assert.That(b.NotKnown, Is.EqualTo(5)); // residual - Assert.That(b.Boards.Values.Sum() + b.Other + b.NotKnown, Is.EqualTo(b.Total)); - }); - } - - [Test] - public void Report_ToCsv_WideHeaderAndMetricRows() - { - var nodes = new List - { - new() - { - Seq = 0, Type = "Container", Name = "Root", Container = "", SetOperation = "EXCEPT", - FinalUnfiltered = 80, CumulativeUnfiltered = null, - FinalByRegion = new Dictionary { ["T"] = 50, ["G"] = 30 } - } - }; - - var csv = CohortBuildHealthBoardBreakdownReport.ToCsv(nodes); - var header = csv.Split('\n')[0].Trim(); - - Assert.Multiple(() => - { - Assert.That(header, Does.StartWith("Order,Type,Name,Container,SetOperation,Metric,Total,")); - Assert.That(header, Does.Contain("Tayside")); - Assert.That(header, Does.EndWith("Other,NotKnown")); - Assert.That(csv, Does.Contain("% of final cohort")); - }); - } - private static DataTable OneCol(string col, IEnumerable values) { var dt = new DataTable(); @@ -279,3 +241,68 @@ public int SumAfterTotal(string name, string metric) private static Wide ParseWide(string csv) => new(csv); } + +/// No-database tests for the wide report projection and the name cleaner. +public class CohortBuildHealthBoardBreakdownReportTests +{ + [Test] + public void Split_SeparatesScottishOtherAndNotKnown() + { + // Total 100; T=50, G=30 (Scottish), X=15 (present but not a Scottish board) -> Other 15; + // NotKnown = 100 - 80 - 15 = 5 (not in demography / null region) + var b = CohortBuildHealthBoardBreakdownReport.Split(100, + new Dictionary { ["T"] = 50, ["G"] = 30, ["X"] = 15 }); + + Assert.Multiple(() => + { + Assert.That(b.Total, Is.EqualTo(100)); + Assert.That(b.Boards["T"], Is.EqualTo(50)); + Assert.That(b.Boards["G"], Is.EqualTo(30)); + Assert.That(b.Boards.ContainsKey("X"), Is.False); // non-Scottish code is NOT a board + Assert.That(b.Other, Is.EqualTo(15)); // it lands in Other + Assert.That(b.NotKnown, Is.EqualTo(5)); // residual + Assert.That(b.Boards.Values.Sum() + b.Other + b.NotKnown, Is.EqualTo(b.Total)); + }); + } + + [Test] + public void ToCsv_WideHeaderAndMetricRows() + { + var nodes = new List + { + new() + { + Seq = 0, Type = "Container", Name = "Root", Container = "", SetOperation = "EXCEPT", + FinalUnfiltered = 80, CumulativeUnfiltered = null, + FinalByRegion = new Dictionary { ["T"] = 50, ["G"] = 30 } + } + }; + + var csv = CohortBuildHealthBoardBreakdownReport.ToCsv(nodes); + var header = csv.Split('\n')[0].Trim(); + + Assert.Multiple(() => + { + Assert.That(header, Does.StartWith("Order,Type,Name,Container,SetOperation,Metric,Total,")); + Assert.That(header, Does.Contain("Tayside")); + Assert.That(header, Does.EndWith("Other,NotKnown")); + Assert.That(csv, Does.Contain("% of final cohort")); + }); + } + + [Test] + public void CleanName_StripsStackedCicPrefixes() + { + Assert.Multiple(() => + { + Assert.That(ExecuteCommandExportCohortBuildHealthBoardBreakdown.CleanName( + "cic_18286_cic_18284_cic_17950_cic_16459_People in SHARE Current For Contact [Recruitment]"), + Is.EqualTo("People in SHARE Current For Contact [Recruitment]")); + Assert.That(ExecuteCommandExportCohortBuildHealthBoardBreakdown.CleanName( + "cic_18286_cic_18284_cic_17950_Excl Grp 1 and 2"), + Is.EqualTo("Excl Grp 1 and 2")); + Assert.That(ExecuteCommandExportCohortBuildHealthBoardBreakdown.CleanName("Root"), Is.EqualTo("Root")); + Assert.That(ExecuteCommandExportCohortBuildHealthBoardBreakdown.CleanName(null), Is.EqualTo("")); + }); + } +} diff --git a/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs b/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs index dba00d47fc..e725d4eaf4 100644 --- a/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs +++ b/Rdmp.Core/CommandExecution/AtomicCommands/ExecuteCommandExportCohortBuildHealthBoardBreakdown.cs @@ -8,6 +8,7 @@ using System.Data; using System.IO; using System.Linq; +using System.Text.RegularExpressions; using System.Threading; using FAnsi.Discovery; using Rdmp.Core.CohortCreation; @@ -200,8 +201,8 @@ private void Walk(CohortAggregateContainer container, CohortAggregateContainer p { Seq = seq++, Type = "Container", - Name = container.Name, - Container = parent?.Name ?? "", + Name = CleanName(container.Name), + Container = CleanName(parent?.Name), SetOperation = container.Operation.ToString(), DisplayOrder = container.Order, FinalUnfiltered = cFinal, @@ -225,8 +226,8 @@ private void Walk(CohortAggregateContainer container, CohortAggregateContainer p { Seq = seq++, Type = "Cohort Set", - Name = agg.Name, - Container = container.Name, + Name = CleanName(agg.Name), + Container = CleanName(container.Name), SetOperation = "", DisplayOrder = agg.Order, FinalUnfiltered = aFinal, @@ -311,6 +312,12 @@ private IReadOnlyDictionary RunRegionCounts(string idListSql) return result; } + // RDMP prefixes cohort set names with "cic__" (EnsureNamingConvention); cloning a cohort across + // CICs stacks them (e.g. cic_18286_cic_18284_cic_17950_People in SHARE...). Strip them for display. + private static readonly Regex CicPrefix = new(@"^(cic_\d+_)+", RegexOptions.Compiled); + + public static string CleanName(string name) => string.IsNullOrEmpty(name) ? "" : CicPrefix.Replace(name, ""); + private static string Sanitise(string name) { foreach (var c in Path.GetInvalidFileNameChars()) From 4626d080582dc0960ceaeb982114040913512f9b Mon Sep 17 00:00:00 2001 From: mtinti Date: Sat, 27 Jun 2026 11:40:27 +0100 Subject: [PATCH 4/4] Repeat header above the % row in the build breakdown CSV The wide build-breakdown output now repeats the column header just above the "% of final cohort" row, so each percentage lines up under its health board. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_0169JCnaL3fhhZjseDx2XXT2 --- .../CohortCreation/CohortBuildHealthBoardBreakdownTests.cs | 2 ++ .../CohortCreation/CohortBuildHealthBoardBreakdownReport.cs | 1 + 2 files changed, 3 insertions(+) diff --git a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs index 60a79bfe77..c2fd11f349 100644 --- a/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs +++ b/Rdmp.Core.Tests/CohortCreation/CohortBuildHealthBoardBreakdownTests.cs @@ -212,6 +212,8 @@ public Wide(string csv) foreach (var line in lines.Skip(1)) { var c = line.Split(','); + if (c[0] == "Order") + continue; // the header is repeated just above the percentage row if (c[_col["Metric"]] == CohortBuildHealthBoardBreakdownReport.PercentMetric) _percent = c; else diff --git a/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs b/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs index 0fe3ef6059..e3c8450aa3 100644 --- a/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs +++ b/Rdmp.Core/CohortCreation/CohortBuildHealthBoardBreakdownReport.cs @@ -130,6 +130,7 @@ public static string ToCsv(IReadOnlyList nodes) if (root != null && root.FinalUnfiltered > 0) { sb.AppendLine(); + sb.AppendLine(string.Join(",", header.Select(Escape))); // repeat header so % aligns to each board var b = Split(root.FinalUnfiltered, root.FinalByRegion); double Pct(int v) => v * 100.0 / b.Total; var cells = new List { "", "", PercentMetric, "", "", PercentMetric, Fmt(100.0) };