feat(core): InputQuality.WorstOf — the one worst-of-input rule (archreview R3-02 contract)

archreview/plans/R3-02-stale-good-family-plan.md

@@ -580,3 +580,23 @@ T17 anytime; T18 after all; T19 last
 ```
 
 Overall effort: **~2 dev-days code+unit, plus the rig session**. The three HIGH-RISK data-plane slices (T3 MQTT stale, T8 value-node Bad, T13/T16 hold-on-Bad) each land behind their full suites; the two recovery contract tests (T10, T16's identical-value case) must each be seen to FAIL once against a reverted dedup before merge — that is the whole reason this family re-appeared.
+
+---
+
+## Execution notes
+
+Branch `fix/r3-02-stale-good-family` (worktree `.claude/worktrees/agent-a696a39981023fa0b`), off master
+`72329eab`. Executed in plan block order T1, B (T2–T5), C (T6–T10), D (T12–T14), E (T15–T16), T17, T18.
+**T11 and T19 are deliberately NOT in this branch** — T11 waits for the sibling R3-03 branch to merge
+(it is the only edit that would touch `DriverInstanceActor` lifecycle alongside R3-03's restart work),
+and T19 is the operator rig gate.
+
+Red-before observations are recorded per task below: what was run, and the failure text that proves the
+test could only pass once the production change landed.
+
+### T1 — `InputQuality.WorstOf`
+
+- RED: `dotnet test tests/Core/…Core.Abstractions.Tests --filter "FullyQualifiedName~InputQualityTests"`
+  ⇒ **build failure**, 8 × `error CS0103: The name 'InputQuality' does not exist in the current context`.
+- GREEN: 13 passed. `Core.ScriptedAlarms.Tests` 94/94 green with `WorstInputStatus` re-pointed (the
+  never-pushed/null skips stay in the alarm engine — they are alarm policy, not the shared rule).

archreview/plans/R3-02-stale-good-family-plan.md.tasks.json

@@ -5,7 +5,7 @@
     {
       "id": "T1",
       "subject": "InputQuality.WorstOf shared helper (Core.Abstractions) + truth-table tests; re-point ScriptedAlarmEngine.WorstInputStatus",
-      "status": "pending",
+      "status": "done",
       "blockedBy": []
     },
     {

src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs

@@ -0,0 +1,74 @@
+namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+/// <summary>
+///     The one worst-of-input rule every derived-value seam uses — scripted alarms, virtual tags and
+///     the Calculation driver.
+///
+///     <para><b>The contract it implements</b> (archreview R3-02, "the stale-Good family"): a value
+///     consumer never reports a quality better than its source's. A node derived from driver values
+///     carries, at worst, the worst status among its inputs — a Bad input makes the derived node Bad
+///     with that input's sub-code, an Uncertain input makes it Uncertain, and a source that has
+///     stopped communicating is a Bad input for everything downstream of it.</para>
+///
+///     <para><b>Why it lives here.</b> Core.Abstractions is the one assembly all three seams already
+///     reference (Commons, Runtime and the driver assemblies all sit above it), and
+///     <see cref="DataValueSnapshot"/> — the shape the rule reads — is declared beside it. Stating
+///     the rule three times is how the three seams drifted apart in the first place.</para>
+///
+///     <para><b>Severity, not equality.</b> Comparison is on the OPC UA Part 4 severity bits (the top
+///     two: <c>00</c> Good, <c>01</c> Uncertain, <c>10</c>/<c>11</c> Bad); the sub-code below them is
+///     carried through verbatim because the sub-code is the diagnostic
+///     (<c>BadNoCommunication</c> = go look at the device, <c>BadWaitingForInitialData</c> = it never
+///     arrived, <c>BadInternalError</c> = the script).</para>
+/// </summary>
+public static class InputQuality
+{
+    /// <summary>
+    ///     No value has ever been received for this input — the status a fresh node is materialised
+    ///     with, and what <see cref="WorstOf"/> reports for a dependency slot that holds no snapshot
+    ///     at all. Spelled here rather than taken from <c>Commons.OpcUa.OpcUaStatus</c> because
+    ///     Core.Abstractions sits <em>below</em> Commons and deliberately references nothing.
+    /// </summary>
+    public const uint BadWaitingForInitialData = 0x80320000u;
+
+    /// <summary>
+    ///     The worst status among <paramref name="inputs"/>: the highest Part 4 severity, carrying
+    ///     that input's own sub-code. A <see langword="null"/> entry is a dependency that has never
+    ///     been pushed and reports <see cref="BadWaitingForInitialData"/> — reporting Good for it is
+    ///     the laundering this helper exists to stop. An empty set is Good: there is nothing to be
+    ///     worse than.
+    /// </summary>
+    /// <param name="inputs">The dependency snapshots, in a stable order.</param>
+    /// <returns>The worst status code observed.</returns>
+    /// <remarks>
+    ///     Among <em>equal</em> severity the first entry seen wins, so a re-evaluation over the same
+    ///     inputs reports the same sub-code instead of oscillating between two equally-bad codes.
+    /// </remarks>
+    public static uint WorstOf(IEnumerable<DataValueSnapshot?> inputs)
+    {
+        ArgumentNullException.ThrowIfNull(inputs);
+
+        var worst = 0u;
+        var worstSeverity = 0u;
+        foreach (var input in inputs)
+        {
+            var status = input?.StatusCode ?? BadWaitingForInitialData;
+            var severity = status >> 30;
+            if (severity <= worstSeverity) continue;
+            worstSeverity = severity;
+            worst = status;
+        }
+
+        return worst;
+    }
+
+    /// <summary>
+    ///     True when a consumer may evaluate against a value carrying <paramref name="statusCode"/>.
+    ///     Good and Uncertain are usable — an Uncertain value is present and a script can make a
+    ///     qualified decision from it; Bad is not, and a consumer holds instead (the alarm engine's
+    ///     long-standing policy, generalised).
+    /// </summary>
+    /// <param name="statusCode">The status code to classify.</param>
+    /// <returns><see langword="true"/> for Good or Uncertain severity.</returns>
+    public static bool IsUsable(uint statusCode) => (statusCode >> 30) < 2;
+}

src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs

@@ -919,26 +919,18 @@ public sealed class ScriptedAlarmEngine : IDisposable
     /// the case the skip was hiding was the case that matters most. The upstream source now stamps a
     /// never-pushed path <see cref="StatusNeverPushed"/> and nothing else does, so the two are tellable
     /// apart. A null snapshot is still skipped: that is a cache the refill did not populate, not an
-    /// observation.</para></summary>
+    /// observation.</para>
+    /// <para><b>The comparison itself lives in <see cref="InputQuality.WorstOf"/></b> (archreview R3-02)
+    /// — the same rule the virtual-tag and Calculation seams now use, so "worst input wins, carrying its
+    /// sub-code, first-seen on a severity tie" is stated once. What stays here is the alarm-specific
+    /// <em>filter</em>: this engine's two skips are deliberately NOT part of the shared rule, because the
+    /// other two seams treat a never-pushed input as Bad rather than as absent.</para></summary>
     private static uint WorstInputStatus(IReadOnlyDictionary<string, DataValueSnapshot> cache)
     {
-        uint worst = 0u;
-        var worstSeverity = 0u;
-        foreach (var kv in cache)
-        {
-            // No entry at all — nothing was observed, so there is nothing to report on.
-            if (kv.Value is null) continue;
-            // Observed as "no data has ever arrived" — the cold-start placeholder, not a fault.
-            if (kv.Value.StatusCode == StatusNeverPushed) continue;
-            var status = kv.Value.StatusCode;
-            var severity = status >> 30;
-            if (severity > worstSeverity)
-            {
-                worstSeverity = severity;
-                worst = status;
-            }
-        }
-        return worst;
+        // No entry at all — nothing was observed, so there is nothing to report on.
+        // Observed as "no data has ever arrived" — the cold-start placeholder, not a fault.
+        var observed = cache.Values.Where(v => v is not null && v.StatusCode != StatusNeverPushed);
+        return InputQuality.WorstOf(observed);
     }
 
     /// <summary>Update the tracked worst-quality bucket for an alarm; return true iff the 3-state bucket

tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InputQualityTests.cs

@@ -0,0 +1,97 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests;
+
+/// <summary>
+///     Truth table for <see cref="InputQuality"/> — the one worst-of-input rule every derived-value
+///     seam uses (scripted alarms, virtual tags, the Calculation driver). The quality contract
+///     (archreview R3-02) is "a value consumer never reports a quality better than its source's",
+///     and this type is the single place that decides what "worse" means, so a divergence here is a
+///     divergence in every seam at once.
+/// </summary>
+public sealed class InputQualityTests
+{
+    private const uint Good = 0x00000000u;
+    private const uint Uncertain = 0x40000000u;
+    private const uint UncertainLastUsableValue = 0x40900000u;
+    private const uint BadNoCommunication = 0x80310000u;
+    private const uint BadInternalError = 0x80020000u;
+    private const uint BadWaitingForInitialData = 0x80320000u;
+
+    private static DataValueSnapshot Snap(object? value, uint status) =>
+        new(value, status, SourceTimestampUtc: DateTime.UtcNow, ServerTimestampUtc: DateTime.UtcNow);
+
+    /// <summary>An empty input set reports Good — there is nothing to be worse than.</summary>
+    [Fact]
+    public void WorstOf_no_inputs_is_Good()
+    {
+        InputQuality.WorstOf([]).ShouldBe(Good);
+    }
+
+    /// <summary>All-Good inputs report Good.</summary>
+    [Fact]
+    public void WorstOf_all_good_is_Good()
+    {
+        InputQuality.WorstOf([Snap(1, Good), Snap(2, Good)]).ShouldBe(Good);
+    }
+
+    /// <summary>A single Uncertain input carries its own sub-code out, not the generic Uncertain.</summary>
+    [Fact]
+    public void WorstOf_propagates_the_uncertain_sub_code()
+    {
+        InputQuality.WorstOf([Snap(1, Good), Snap(2, UncertainLastUsableValue)])
+            .ShouldBe(UncertainLastUsableValue);
+    }
+
+    /// <summary>Bad beats Uncertain regardless of the order the inputs are seen in.</summary>
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void WorstOf_bad_beats_uncertain(bool badFirst)
+    {
+        DataValueSnapshot?[] inputs = badFirst
+            ? [Snap(null, BadNoCommunication), Snap(2, Uncertain)]
+            : [Snap(2, Uncertain), Snap(null, BadNoCommunication)];
+
+        InputQuality.WorstOf(inputs).ShouldBe(BadNoCommunication);
+    }
+
+    /// <summary>
+    ///     A null entry means "this dependency has never been pushed", which is
+    ///     <c>BadWaitingForInitialData</c> — the same code the materialiser stamps on a fresh node.
+    ///     Reporting Good for it would be the exact laundering this helper exists to stop.
+    /// </summary>
+    [Fact]
+    public void WorstOf_a_null_entry_is_BadWaitingForInitialData()
+    {
+        InputQuality.WorstOf([Snap(1, Good), null]).ShouldBe(BadWaitingForInitialData);
+    }
+
+    /// <summary>
+    ///     Among equal severity the FIRST seen wins, so a re-evaluation over the same inputs reports
+    ///     the same sub-code rather than oscillating between two equally-bad codes.
+    /// </summary>
+    [Fact]
+    public void WorstOf_equal_severity_keeps_the_first_seen()
+    {
+        InputQuality.WorstOf([Snap(null, BadNoCommunication), Snap(null, BadInternalError)])
+            .ShouldBe(BadNoCommunication);
+        InputQuality.WorstOf([Snap(null, BadInternalError), Snap(null, BadNoCommunication)])
+            .ShouldBe(BadInternalError);
+    }
+
+    /// <summary>Good and Uncertain are usable (a script may still inspect the value); Bad is not.</summary>
+    [Theory]
+    [InlineData(Good, true)]
+    [InlineData(Uncertain, true)]
+    [InlineData(UncertainLastUsableValue, true)]
+    [InlineData(BadNoCommunication, false)]
+    [InlineData(BadWaitingForInitialData, false)]
+    [InlineData(0xC0000000u, false)]
+    public void IsUsable_follows_Part4_severity(uint statusCode, bool expected)
+    {
+        InputQuality.IsUsable(statusCode).ShouldBe(expected);
+    }
+}