From 41c9e66bd1fa90d467b972a35c40b9e10d57d50e Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:59:00 -0400 Subject: [PATCH 1/2] Point every rung citation for alert_persistence_state at V118, and pin that they do #3282's rung renumbered from V117 to V118 when #3315 merged first, and five comments across both SKUs still cite V117 - which is now the mute-rule reload-beacon rung. Review found three of them on #3328; the other two are in LiteAlertStateStore and PgAlertStateStore. The repo uses exactly these citations to check Lite/Darling parity, so a wrong one sends the next person to the wrong rung. This was the last commit on #3328 and the merge landed one commit short of it, so it comes back on its own. The pin derives the number from StorageVersion.SchemaVersion rather than comparing against a literal, so a future renumber reds it instead of leaving silent copies. Each phrase carries its own SUBJECT, which is what makes a citation attributable without parsing comments at all - and both earlier spellings are why. Scoped to the FILE it flagged nine correct citations for other rungs, which is a check someone turns off. Scoped to comment BLOCKS it needed a hand-rolled line-prefix comment filter, which CommentFilterAdoptionTests refuses without a stated bound - correctly, and the bound I measured for it came back unreliable on its own terms. A phrase naming both the table and the rung needs neither. Reddened three ways before its pass was trusted: one citation reverted to V117, a SchemaVersion bump that leaves all five behind, and a reworded phrase - which has to fail loudly rather than silently matching nothing, so the pattern is asserted to match at all before its captures are compared. Assert.NotEmpty is load-bearing there: Regex.Escape escapes '{' and not '}', which was verified by running it rather than assumed. --- .../BuiltinAlertPersistenceRungTests.cs | 84 +++++++++++++++++++ .../PgAlertStateStore.cs | 2 +- Lite/Database/DuckDbInitializer.cs | 2 +- Lite/Database/Schema.cs | 2 +- Lite/Services/DuckDbAlertHistoryStore.cs | 2 +- Lite/Services/LiteAlertStateStore.cs | 2 +- 6 files changed, 89 insertions(+), 5 deletions(-) diff --git a/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs b/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs index e4e37380f..df63c1e2a 100644 --- a/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs +++ b/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs @@ -7,7 +7,11 @@ */ using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; using System.Linq; +using System.Text.RegularExpressions; using System.Reflection; using PerformanceMonitor.Alerting; using PerformanceMonitor.Common; @@ -322,6 +326,86 @@ private static string EvaluatePgCpuBody() return body; } + /// + /// Every comment that cites this rung's NUMBER cites the right one. + /// + /// A rung number in prose is a frozen claim, and this branch froze the wrong one: #3315 landed + /// V117 first, this rung renumbered to V118, and five comments across both SKUs kept saying V117 — + /// three of which review found and two of which it did not. The repo leans on exactly these citations + /// to check Lite/Darling parity, so a wrong one sends the next person to the wrong rung. + /// + /// The number is derived from rather than + /// compared against a literal, so a future renumber reds this instead of leaving silent copies. + /// + /// And each phrase carries its own SUBJECT, which is what makes a citation attributable + /// without parsing comments at all. Two earlier spellings of this pin are the reason it is shaped this + /// way. Scoped to the file, it flagged nine correct citations — those files cite V32, V40, V44, V50, + /// V60, V61, V80 and V81 for their own tables, all right. Scoped to comment BLOCKS instead, it needed a + /// hand-rolled line-prefix comment filter, which CommentFilterAdoptionTests correctly refuses + /// without a stated bound — and the bound I measured for it came back unreliable on its own terms. A + /// phrase that names both the table and the rung needs neither: it cannot match another rung's citation + /// because it does not describe another rung's subject. + /// + [Fact] + public void EveryCommentCitingThisRungsNumber_CitesTheRealOne() + { + var rung = StorageVersion.SchemaVersion; + + /* Each entry names a file and the citation in it, with {0} where the rung goes. The subject words + are part of the phrase deliberately — see the remarks. A reword reds this, which is correct: the + prose and the pin are one claim, so the pin has to be edited with it. */ + var citations = new (string Path, string Phrase)[] + { + (Path.Combine("Lite", "Database", "Schema.cs"), + "config.alert_persistence_state (PgMigrations V{0})"), + (Path.Combine("Lite", "Database", "DuckDbInitializer.cs"), + "persistence-gate state, porting Darling's V{0}"), + (Path.Combine("Lite", "Services", "DuckDbAlertHistoryStore.cs"), + "the Lite twin of Darling's V{0} table"), + (Path.Combine("Lite", "Services", "LiteAlertStateStore.cs"), + "Darling's V{0} table, so the shared engine's CPU gate"), + (Path.Combine("Darling", "PerformanceMonitor.Darling.Service", "PgAlertStateStore.cs"), + "persistence-gate record from the V{0}"), + }; + + foreach (var (relative, phrase) in citations) + { + var source = RepoFile.ReadRepoFileLf(relative); + + /* EXACTLY once. Absent means the prose was reworded and this pin went stale with it; twice + means a copy was made that the next renumber would miss. */ + var expected = string.Format(CultureInfo.InvariantCulture, phrase, rung); + Assert.Equal(1, CountOf(source, expected)); + + /* And the same phrase carrying ANY other rung number must not appear — the renumber case. The + regex is built from the phrase itself, so it cannot drift away from the string above. */ + var pattern = Regex.Escape(phrase).Replace(@"V\{0}", @"V(\d+)", StringComparison.Ordinal); + var found = Regex.Matches(source, pattern, RegexOptions.CultureInvariant) + .Select(m => m.Groups[1].Value) + .ToArray(); + + /* The regex has to MATCH, or the assertion below is over an empty set and passes for the wrong + reason — the failure mode of building a pattern out of an escaped literal. */ + Assert.NotEmpty(found); + Assert.All(found, n => Assert.Equal(rung.ToString(CultureInfo.InvariantCulture), n)); + } + } + + /// Non-overlapping occurrences of — IndexOf in a loop, because + /// there is no overload that counts and a Split would allocate the whole file per call. + private static int CountOf(string haystack, string needle) + { + var count = 0; + var at = haystack.IndexOf(needle, StringComparison.Ordinal); + while (at >= 0) + { + count++; + at = haystack.IndexOf(needle, at + needle.Length, StringComparison.Ordinal); + } + + return count; + } + /// /// The README's alert catalog states the sample count, so the number is pinned to the constant rather /// than left as a prose copy of it. A doc that says "3 samples" while the code says something else is diff --git a/Darling/PerformanceMonitor.Darling.Service/PgAlertStateStore.cs b/Darling/PerformanceMonitor.Darling.Service/PgAlertStateStore.cs index 27db57824..9f6bb33cc 100644 --- a/Darling/PerformanceMonitor.Darling.Service/PgAlertStateStore.cs +++ b/Darling/PerformanceMonitor.Darling.Service/PgAlertStateStore.cs @@ -381,7 +381,7 @@ a missed or duplicated alert. The alert itself has already been decided by the g } /// - /// #3282: loads one subject's built-in persistence-gate record from the V117 + /// #3282: loads one subject's built-in persistence-gate record from the V118 /// config.alert_persistence_state table. /// /// Returns null on failure, which the engine reads as "no memory" and arms the gate from zero — diff --git a/Lite/Database/DuckDbInitializer.cs b/Lite/Database/DuckDbInitializer.cs index d4222db0d..953805fdc 100644 --- a/Lite/Database/DuckDbInitializer.cs +++ b/Lite/Database/DuckDbInitializer.cs @@ -1551,7 +1551,7 @@ positional appender and old parquet are unaffected. */ if (fromVersion < 58) { - /* v58 (#3282): the built-in alert catalog's persistence-gate state, porting Darling's V117. + /* v58 (#3282): the built-in alert catalog's persistence-gate state, porting Darling's V118. Before this no built-in alert required its condition to PERSIST — one sample over the bar fired and the next sample under it resolved — so a momentary CPU spike was indistinguishable from sustained saturation. New table only; fresh installs get it from diff --git a/Lite/Database/Schema.cs b/Lite/Database/Schema.cs index de0d1d0b6..a2df7438c 100644 --- a/Lite/Database/Schema.cs +++ b/Lite/Database/Schema.cs @@ -119,7 +119,7 @@ last_observed_at is not display data — it is what makes a row's staleness deci fingerprint's NEXT incident would decay its already-counted mark to the new window count and report the recurrence as nothing new. */ /* The BUILT-IN alert catalog's persistence-gate state (#3282), the twin of Darling's - config.alert_persistence_state (PgMigrations V117) — same columns, same key. "How long must this + config.alert_persistence_state (PgMigrations V118) — same columns, same key. "How long must this condition hold before it counts" for the gauge alerts: consecutive breaching samples so far, consecutive clearing samples so far, and whether an incident is currently open. diff --git a/Lite/Services/DuckDbAlertHistoryStore.cs b/Lite/Services/DuckDbAlertHistoryStore.cs index 8224dba65..b244a3ab3 100644 --- a/Lite/Services/DuckDbAlertHistoryStore.cs +++ b/Lite/Services/DuckDbAlertHistoryStore.cs @@ -530,7 +530,7 @@ gate has already decided by this point. */ /// /// #3282: loads one subject's built-in persistence-gate record from config_alert_persistence_state - /// — the Lite twin of Darling's V117 table. Returns null when there is no row, which the engine reads as + /// — the Lite twin of Darling's V118 table. Returns null when there is no row, which the engine reads as /// "no memory" and arms the gate from zero. /// /// Null on failure too, deliberately. A load failure cannot resurrect a firing bit it did diff --git a/Lite/Services/LiteAlertStateStore.cs b/Lite/Services/LiteAlertStateStore.cs index c06a746ab..6fbae59d6 100644 --- a/Lite/Services/LiteAlertStateStore.cs +++ b/Lite/Services/LiteAlertStateStore.cs @@ -181,7 +181,7 @@ public Task SaveIncidentOccurrencesAsync( /// /// #3282: the built-in persistence-gate record, over config_alert_persistence_state — Lite's twin - /// of Darling's V117 table, so the shared engine's CPU gate behaves the same on both SKUs. Wrapped in + /// of Darling's V118 table, so the shared engine's CPU gate behaves the same on both SKUs. Wrapped in /// Task.Run like every other method here: DuckDB.NET's I/O is synchronous under its async facade /// and the engine runs on the WPF dispatcher, so an unwrapped call is a UI hitch (#1202). /// From 9f4f80ec167c50bdf09a55c316bb58d2022ecb03 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 11 Sep 2026 20:24:34 -0400 Subject: [PATCH 2/2] Drop the unused using the pin added, and sort the rest Review nit. System.Collections.Generic came in with the first spelling of the pin, which used a List, and stayed behind when the phrase-based version replaced it. The added lines also put System.Text.RegularExpressions ahead of System.Reflection. --- Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs b/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs index df63c1e2a..c051690f2 100644 --- a/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs +++ b/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs @@ -7,12 +7,11 @@ */ using System; -using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; -using System.Text.RegularExpressions; using System.Reflection; +using System.Text.RegularExpressions; using PerformanceMonitor.Alerting; using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage;