fix(historian): #684 review — Uncertain non-numeric samples do not refuse the Good-value aggregates; real-SQL Uncertain-above-max test

- Average/Minimum/Maximum/Range/*ActualTime refuse (BadAggregateInvalidInputs) only a GOOD String/DateTime sample (new GoodNonNumericCount / good_non_numeric_count column); an Uncertain one is skipped and makes the min/max family Uncertain, matching the SDK (Average skips it, ComputeMinMax flags the uncastable value). Delta keeps its usable-sample refusal. - ProcessedConformance: an Uncertain value above the Good maximum (pins UncMax/unc_max through the real SQL) and an Uncertain String through the real SQL. - The_uncertain_dataset_is_not_vacuous also proves the min/max STATUS discrimination. Co-Authored-By: Claude <noreply@anthropic.com>
2026-10-12 09:54:51 -04:00
parent a11ab0dc08
commit c0d6bacd9c
13 changed files with 240 additions and 27 deletions
@@ -305,6 +305,46 @@ public sealed class AggregateEvaluatorTests
        v.StatusCode.ShouldBe(good ? GoodCalc : UncertainCalc);
    }

+    [Theory]
+    [InlineData(HistoryAggregateType.Average)]
+    [InlineData(HistoryAggregateType.Minimum)]
+    [InlineData(HistoryAggregateType.Maximum)]
+    [InlineData(HistoryAggregateType.Range)]
+    [InlineData(HistoryAggregateType.MinimumActualTime)]
+    [InlineData(HistoryAggregateType.MaximumActualTime)]
+    public void Only_a_good_non_numeric_sample_refuses_a_good_value_aggregate(HistoryAggregateType agg)
+    {
+        // #684 review: an UNCERTAIN String/DateTime sample is outside the Good-value set — served (Uncertain), not refused.
+        var uncertainString = GoodTenThirtyPlusUncertain();
+        uncertainString.Total = uncertainString.Usable = 3;
+        uncertainString.NonNumeric = 1;
+        uncertainString.GoodNonNumeric = 0;
+        var v = Eval(agg, uncertainString);
+        v.Value.ShouldNotBeNull();
+        v.StatusCode.ShouldBe(UncertainCalc);
+        // A GOOD one still refuses the interval (the value set would hold a value with no numeric meaning).
+        var goodString = GoodTenThirtyPlusUncertain();
+        goodString.Total = goodString.Usable = goodString.Good = 3;
+        goodString.NonNumeric = 1;
+        goodString.GoodNonNumeric = 1;
+        Eval(agg, goodString).StatusCode.ShouldBe(InvalidInputsCalc);
+        // A row that does not carry the Good count (null) keeps the stricter pre-#684 check: any usable one refuses.
+        uncertainString.GoodNonNumeric = null;
+        Eval(agg, uncertainString).StatusCode.ShouldBe(InvalidInputsCalc);
+    }
+
+    [Fact]
+    public void Delta_still_refuses_any_usable_non_numeric_sample()
+    {
+        // Delta reads the first/last USABLE samples (#680), so an Uncertain String can be its input: unchanged by #684.
+        var row = new BucketRow
+        {
+            Total = 2, Usable = 2, Good = 1, NonNumeric = 1, GoodNonNumeric = 0,
+            First = HistorianValue.FromString("x"), Last = HistorianValue.FromDouble(1), FirstAny = A[0], LastAny = A[1],
+        };
+        AggregateEvaluator.Evaluate(HistoryAggregateType.Delta, row, At(0), Now).StatusCode.ShouldBe(InvalidInputsCalc);
+    }
+
    [Theory]
    [InlineData(HistoryAggregateType.Minimum)]
    [InlineData(HistoryAggregateType.Maximum)]
@@ -341,10 +341,23 @@ public sealed class SdkAggregateParityTests
    [MemberData(nameof(Windows))]
    public void The_uncertain_dataset_is_not_vacuous(double from, double to, double intervalS)
    {
-        // #684: DenseUncertain must actually move the per-sample values the pre-#684 (Good + Uncertain) columns computed —
-        // otherwise its parity proves nothing. Average over the usable samples differs from the SDK's in some interval of
-        // every window.
+        // #684: DenseUncertain must discriminate the pre-#684 behaviour in every window, or its parity proves nothing.
+        // (1) VALUES: Average over the usable (Good + Uncertain) samples differs from the SDK's Good-only Average in some
+        // interval. (2) STATUS: Minimum / Maximum hold an Uncertain sample in some interval the SDK still calls Good (16 sits
+        // inside the Good range) — the pre-#684 "any non-Good sample ⇒ Uncertain" rule would answer Uncertain there.
+        // (Minimum / Maximum VALUES do not discriminate here: an in-range 16 never is the extreme — by design, so the
+        // min/max family needs no mask; the value half is pinned by UncertainBeyond and AggregateEvaluatorTests.)
        var plan = BucketPlan.Create(At(from), At(to), TimeSpan.FromSeconds(intervalS));
+        // The one-interval windows hold the Bad +22.5 sample, so the SDK is Uncertain there for a reason the old rule shares:
+        // the status half is checked on the multi-interval windows only (the value half below covers every window).
+        foreach (var agg in plan.BucketCount > 1 ? new[] { HistoryAggregateType.Minimum, HistoryAggregateType.Maximum } : [])
+        {
+            var sdkMinMax = Sdk(agg, from, to, intervalS, DenseUncertain);
+            var rows = InMemoryRows(plan, DenseUncertain).ToList();
+            var holdsUncertain = (plan.Reversed ? Enumerable.Reverse(rows) : rows).Select(r => r.Usable > r.Good).ToList();
+            holdsUncertain.Zip(sdkMinMax).Any(p => p.First && (p.Second.StatusCode & 0xFFFF0000u) == 0)
+                .ShouldBeTrue($"{agg}: some interval holds an Uncertain sample yet is Good in the SDK");
+        }
        var usableAverage = InMemoryRows(plan, DenseUncertain.Select(x => x.IsUsable && !x.IsGood
                ? x with { Status = Good } : x))
            .Select(r => r.Avg).ToList();
@@ -395,6 +408,49 @@ public sealed class SdkAggregateParityTests
        });
    }

+    /// <summary>#684 review: Good numeric samples with an Uncertain STRING among them (+4).</summary>
+    private static readonly StoredSample[] UncertainString =
+    [
+        S(-5, HistorianValue.FromDouble(20), Good),
+        S(2, HistorianValue.FromDouble(10), Good),
+        S(4, HistorianValue.FromString("x"), Uncertain),
+        S(8, HistorianValue.FromDouble(30), Good),
+        S(25, HistorianValue.FromDouble(20), Good),
+    ];
+
+    [Theory]
+    [InlineData(0, 20)]
+    [InlineData(20, 0)]
+    public void An_uncertain_non_numeric_sample_is_skipped_by_the_good_value_aggregates_like_the_SDK(double from, double to)
+    {
+        // #684 review: an Uncertain String is not part of the Good-value set, so it no longer refuses the interval
+        // (BadAggregateInvalidInputs); the SDK's Average skips it (value-based status: Uncertain) and its ComputeMinMax flags
+        // the value it cannot cast (Uncertain). Exact match, every bit but Calculated.
+        var failures = new List<string>();
+        foreach (var agg in new[]
+                 {
+                     HistoryAggregateType.Average, HistoryAggregateType.Minimum, HistoryAggregateType.Maximum,
+                     HistoryAggregateType.Range, HistoryAggregateType.MinimumActualTime, HistoryAggregateType.MaximumActualTime,
+                 })
+        {
+            try
+            {
+                var ours = Ours(agg, from, to, 0, UncertainString);
+                SdkAggregateOracle.ShouldMatch(ours, Sdk(agg, from, to, 0, UncertainString), $"{agg} UncertainString {from}→{to}",
+                    compareCalculatedInterpolatedBits: false);
+                (ours.Single().StatusCode & StatusBits.CodeBitsMask).ShouldBe(HistorianStatusCodes.UncertainDataSubNormal, agg.ToString());
+            }
+            catch (Exception ex)
+            {
+                failures.Add(ex.Message);
+            }
+        }
+        failures.ShouldBeEmpty(string.Join("\n", failures));
+        // Delta keeps its usable picks and refusal (#680, unchanged): no Good-only reading applies to it.
+        Ours(HistoryAggregateType.Delta, from, to, 0, UncertainString).Single().StatusCode
+            .ShouldBe(HistorianInfoBits.WithCalculated(HistorianStatusCodes.BadAggregateInvalidInputs));
+    }
+
    [Theory]
    [InlineData(0, 20)]
    [InlineData(20, 0)]
@@ -647,6 +703,7 @@ public sealed class SdkAggregateParityTests
                Good = all.Count(s => s.IsGood),
                Bad = all.Count - usable.Count,
                NonNumeric = usable.Count - numeric.Count,
+                GoodNonNumeric = usable.Count(s => s.IsGood) - numeric.Count(s => s.IsGood),
                Avg = good.Count == 0 ? null : good.Average(s => s.Value.Number),
                Min = good.Count == 0 ? null : good.Min(s => s.Value.Number),
                Max = good.Count == 0 ? null : good.Max(s => s.Value.Number),