fix(calculation): hold evaluation on Bad inputs and propagate the worst input status; status-keyed dedup (05/STAB-19)

2026-08-20 10:37:29 -04:00
parent 3f8d5815b3
commit 48efdac600
4 changed files with 397 additions and 21 deletions
@@ -82,7 +82,10 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
    // SHORT-HELD state lock — it is taken for dictionary work only and is NEVER held across a script run
    // (Driver.Calculation-005).
    private readonly object _lock = new();
-    private readonly Dictionary<string, object?> _depValues = new(StringComparer.Ordinal);
+    // Dependency SNAPSHOTS, not bare values (archreview 05/STAB-19): the status code is what tells a
+    // comms-lost input apart from a genuine Good null, and storing values only is what let this driver
+    // compute a Good output from a dead source.
+    private readonly Dictionary<string, DataValueSnapshot> _depValues = new(StringComparer.Ordinal);
    private readonly HashSet<string> _arrivedDeps = new(StringComparer.Ordinal);
    private readonly Dictionary<string, TagState> _stateByRawPath = new(StringComparer.Ordinal);

@@ -332,7 +335,7 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
        if (!_running) return;
        lock (_lock)
        {
-            _depValues[rawPath] = value;
+            _depValues[rawPath] = new DataValueSnapshot(value, statusCode, timestampUtc, timestampUtc);
            _arrivedDeps.Add(rawPath);
            if (!_changeDependents.TryGetValue(rawPath, out var dependents) || dependents.Count == 0) return;

@@ -393,7 +396,10 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
        {
            if (!_stateByRawPath.TryGetValue(reference, out var st) || !st.HasValue)
                return new DataValueSnapshot(null, StatusBadWaitingForInitialData, null, now);
-            var status = st.LastPublishedBad ? StatusBadInternalError : StatusGood;
+            // Exactly what the subscriber was last told — including a propagated input sub-code. Deriving
+            // it from a two-state flag here would make a read disagree with the subscription over the
+            // same node (05/STAB-19).
+            var status = st.LastPublishedStatus;
            return new DataValueSnapshot(st.LastValue, status, st.LastTimestampUtc, st.LastTimestampUtc);
        }
    }
@@ -404,12 +410,16 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
    /// trigger fired, and that trigger's timestamp. Built under <see cref="_lock"/> by
    /// <see cref="PrepareEvaluation"/> and executed outside it, so the script itself needs no shared state.</summary>
    /// <param name="Definition">The calc tag being evaluated.</param>
-    /// <param name="Dependencies">Its declared reads' values, as of the trigger.</param>
+    /// <param name="Dependencies">Its declared reads' snapshots — value, status and timestamps — as of the trigger.</param>
    /// <param name="TimestampUtc">The trigger's timestamp, stamped on whatever is published.</param>
+    /// <param name="WorstInputStatus">The worst status among <paramref name="Dependencies"/>
+    /// (<see cref="InputQuality.WorstOf"/>). Bad ⇒ the script is HELD and this status is published;
+    /// Good or Uncertain ⇒ the script runs and its result is stamped with it.</param>
    private sealed record PendingEvaluation(
        CalculationTagDefinition Definition,
-        IReadOnlyDictionary<string, object?> Dependencies,
-        DateTime TimestampUtc);
+        IReadOnlyDictionary<string, DataValueSnapshot> Dependencies,
+        DateTime TimestampUtc,
+        uint WorstInputStatus);

    /// <summary>
    ///     Prepare one calc tag's evaluation: resolve the definition, apply the change-gate, and snapshot
@@ -441,10 +451,14 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
        if (string.IsNullOrWhiteSpace(def.ScriptSource)) return null; // unresolved script — never computes
        if (!def.DependencyRefs.All(_arrivedDeps.Contains)) return null; // still warming up — do not evaluate

-        var deps = new Dictionary<string, object?>(StringComparer.Ordinal);
+        var deps = new Dictionary<string, DataValueSnapshot>(StringComparer.Ordinal);
        foreach (var dep in def.DependencyRefs)
            if (_depValues.TryGetValue(dep, out var v)) deps[dep] = v;
-        return new PendingEvaluation(def, deps, timestampUtc);
+
+        // Worst-of-input, computed from the SAME snapshot the evaluation will use, so the status
+        // published can never disagree with the values it was derived from (05/STAB-19).
+        var worst = InputQuality.WorstOf(def.DependencyRefs.Select(r => deps.GetValueOrDefault(r)));
+        return new PendingEvaluation(def, deps, timestampUtc, worst);
    }

    /// <summary>Start the evaluation pump if it is not already running. MUST be called under
@@ -513,7 +527,31 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
    private void RunEvaluation(PendingEvaluation pending)
    {
        var def = pending.Definition;
-        var result = _evaluator.Evaluate(def.RawPath, def.ScriptSource, pending.Dependencies);
+
+        // HOLD ON BAD (05/STAB-19, matching ScriptedAlarmEngine's AreInputsReady): a Bad input is not
+        // usable, so the script does NOT run — which removes the "(double)ctx.GetTag(x).Value threw"
+        // storm at its source rather than catching it downstream — and the tag reports the input's own
+        // sub-code. Uncertain IS usable: the script runs and the output inherits the Uncertain code.
+        if (!InputQuality.IsUsable(pending.WorstInputStatus))
+        {
+            (object? Value, uint StatusCode)? badPublish;
+            lock (_lock)
+            {
+                if (!_stateByRawPath.TryGetValue(def.RawPath, out var badState)) return;
+                badPublish = MarkInputsBad(badState, pending.WorstInputStatus, pending.TimestampUtc);
+            }
+
+            // No script log: an input dying is not a script fault, and the script log is the per-failure
+            // SCRIPT diagnostic. Here the status code is the diagnostic.
+            if (badPublish is { } bad) Publish(def.RawPath, bad.Value, bad.StatusCode, pending.TimestampUtc);
+            return;
+        }
+
+        // Projected to bare values for now — the evaluator's own read cache still fabricates Good, which
+        // the next task fixes by handing it the snapshots. Only reachable with USABLE inputs (the Bad
+        // hold above returned), so nothing Bad is being flattened here.
+        var depValues = pending.Dependencies.ToDictionary(kv => kv.Key, kv => kv.Value.Value, StringComparer.Ordinal);
+        var result = _evaluator.Evaluate(def.RawPath, def.ScriptSource, depValues);

        string? failureReason = null;
        (object? Value, uint StatusCode)? toPublish;
@@ -530,7 +568,7 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
            }
            else
            {
-                toPublish = MarkEvaluationSucceeded(st, result.Value, pending.TimestampUtc);
+                toPublish = MarkEvaluationSucceeded(st, result.Value, pending.WorstInputStatus, pending.TimestampUtc);
            }
        }

@@ -539,23 +577,51 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
    }

    /// <summary>Record a successful evaluation. MUST be called under <see cref="_lock"/>; returns what to
-    /// publish, or null when the value is deduped. The dedup is bypassed while recovering from Bad so a
-    /// recovery whose value equals the pre-failure value still republishes Good (otherwise the node would
-    /// stay Bad forever).</summary>
+    /// publish, or null when the result is deduped.
+    /// <para><b>The dedup key is (value, status), not value alone</b> — 05/STAB-19 generalising the
+    /// Bad-recovery bypass 02/S13 introduced. Recovery is a publish, never an assumption: the node is
+    /// currently carrying the previous status, so a result whose VALUE is unchanged but whose STATUS
+    /// differs — the shape of every recovery, since a device that comes back usually comes back holding
+    /// the same reading — must still land, or the node stays Bad forever.</para></summary>
    /// <param name="st">The tag's published state.</param>
    /// <param name="value">The computed value.</param>
+    /// <param name="statusCode">The status to stamp: the worst input status (Good, or an Uncertain sub-code).</param>
    /// <param name="timestampUtc">The trigger's timestamp.</param>
    /// <returns>The snapshot to publish, or null when deduped.</returns>
-    private (object? Value, uint StatusCode)? MarkEvaluationSucceeded(TagState st, object? value, DateTime timestampUtc)
+    private (object? Value, uint StatusCode)? MarkEvaluationSucceeded(
+        TagState st, object? value, uint statusCode, DateTime timestampUtc)
    {
-        if (!st.LastPublishedBad && st.HasValue && Equals(st.LastValue, value)) return null;
+        if (st.HasPublished && st.LastPublishedStatus == statusCode && st.HasValue && Equals(st.LastValue, value))
+            return null;

        st.HasValue = true;
        st.LastValue = value;
-        st.LastPublishedBad = false;
+        st.HasPublished = true;
+        st.LastPublishedStatus = statusCode;
        st.LastTimestampUtc = timestampUtc;
        _lastComputeUtc = DateTime.UtcNow;
-        return (value, StatusGood);
+        return (value, statusCode);
+    }
+
+    /// <summary>Record that this tag's INPUTS are Bad, so no evaluation ran (05/STAB-19). Publishes the
+    /// worst input's status — carrying the last known value — <b>once per change of that status</b>: a
+    /// repeat of the same Bad is silent, while <c>BadNoCommunication</c> arriving over an earlier
+    /// <c>BadInternalError</c> (or the reverse) is not masked, because the sub-code is what names which
+    /// failure the operator is looking at. MUST be called under <see cref="_lock"/>.</summary>
+    /// <param name="st">The tag's published state.</param>
+    /// <param name="worstInputStatus">The worst status among the tag's declared inputs.</param>
+    /// <param name="timestampUtc">The trigger's timestamp.</param>
+    /// <returns>The snapshot to publish, or null when that status was already published.</returns>
+    private static (object? Value, uint StatusCode)? MarkInputsBad(
+        TagState st, uint worstInputStatus, DateTime timestampUtc)
+    {
+        if (st.HasPublished && st.LastPublishedStatus == worstInputStatus) return null;
+
+        st.HasPublished = true;
+        st.LastPublishedStatus = worstInputStatus;
+        st.LastTimestampUtc = timestampUtc;
+        // Carry the last-known value (null if none) with the input's Bad status.
+        return (st.HasValue ? st.LastValue : null, worstInputStatus);
    }

    /// <summary>Failure tail (mirrors <c>VirtualTagActor</c> 02/S13): the caller always emits the
@@ -568,9 +634,10 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
    /// <returns>The Bad snapshot to publish on a fresh Good→Bad transition, else null.</returns>
    private static (object? Value, uint StatusCode)? MarkEvaluationFailed(TagState st, DateTime timestampUtc)
    {
-        if (st.LastPublishedBad) return null;
+        if (st.HasPublished && st.LastPublishedStatus == StatusBadInternalError) return null;

-        st.LastPublishedBad = true;
+        st.HasPublished = true;
+        st.LastPublishedStatus = StatusBadInternalError;
        st.LastTimestampUtc = timestampUtc;
        // Carry the last-known value (null if none) with Bad quality.
        return (st.HasValue ? st.LastValue : null, StatusBadInternalError);
@@ -643,7 +710,15 @@ public sealed class CalculationDriver : IDriver, IDependencyConsumer, ISubscriba
    {
        public bool HasValue;
        public object? LastValue;
-        public bool LastPublishedBad;
+
+        /// <summary>Whether anything has been published for this tag yet. Distinct from
+        /// <see cref="HasValue"/>: an inputs-Bad publish carries no value of its own.</summary>
+        public bool HasPublished;
+
+        /// <summary>The status code last published for this tag. Replaces the old two-state
+        /// <c>LastPublishedBad</c> flag (05/STAB-19): the dedup key is (value, status), and a Bad status
+        /// now carries the failing INPUT's sub-code rather than a fixed <c>BadInternalError</c>.</summary>
+        public uint LastPublishedStatus;
        public DateTime LastTimestampUtc;
    }