diff --git a/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs b/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs index e4e37380f..c051690f2 100644 --- a/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs +++ b/Darling/Darling.Tests/BuiltinAlertPersistenceRungTests.cs @@ -7,8 +7,11 @@ */ using System; +using System.Globalization; +using System.IO; using System.Linq; using System.Reflection; +using System.Text.RegularExpressions; using PerformanceMonitor.Alerting; using PerformanceMonitor.Common; using PerformanceMonitor.Darling.Storage; @@ -322,6 +325,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). ///