fix(historian): #677 review — start-of-data Partial forward-only, bounded/skipped earliest-sample query, wider SDK parity

F1: the SDK applies its m_startOfData Partial rule to forward reads only; ours now does too (AggregateEvaluator.EvaluateAll + TimeWeightedAggregator). New reverse late-start parity tests through the SDK oracle (unit) and real SQL (conformance, both backends). F2: AggregateEvaluator.NeedsEarliestSample — the earliest-sample query runs only for forward reads of aggregates other than Average, and is bounded by the window end (SqlServer < @end, QuestDB ts < $2). F3: per-sample SDK parity extended to Range, Start, End, Delta, Min/Max ActualTime and PercentGood/Bad (oracle maps StatusAggregateCalculator); pre-existing definitional differences masked and marked #680. Docs: Historian.md Partial/SDK-parity sections, CLAUDE.md bullet.
2026-10-11 11:47:22 -04:00
parent 9120182314
commit 7349260231
12 changed files with 198 additions and 41 deletions
@@ -271,6 +271,34 @@ public abstract class ProcessedConformance : ConformanceBase
            .ShouldAllBe(x => (x.StatusCode & 0x4u) == 0);
    }

+    [Fact]
+    public async Task A_reverse_read_sets_no_start_of_data_Partial_and_matches_the_SDK()
+    {
+        // Review F1: the SDK applies the start-of-data rule to FORWARD reads only. Reverse 40 → 0 at 10 s over data that
+        // starts at +7: (0,10] holds the data's start but is not Partial — checked through the real SQL and the SDK oracle.
+        var rows = Enumerable.Range(0, 20)
+            .Select(i => (At(7 + i * 5), HistorianValue.FromDouble(i % 2 == 0 ? 20 : 40), Good)).ToArray();
+        await B.SeedAsync("RLS", rows);
+        var sdkRows = rows.Select(r => (B.Round(r.Item1), r.Item2, r.Item3)).ToArray();
+        var src = B.CreateDataSource();
+        var ten = TimeSpan.FromSeconds(10);
+        foreach (var agg in new[]
+                 {
+                     HistoryAggregateType.Count, HistoryAggregateType.Minimum, HistoryAggregateType.Range,
+                     HistoryAggregateType.Delta, HistoryAggregateType.TimeAverage, HistoryAggregateType.TimeAverage2,
+                     HistoryAggregateType.Total2,
+                 })
+        {
+            var ours = (await src.ReadProcessedAsync("RLS", At(40), At(0), ten, agg, Ct)).Samples;
+            ours.ShouldAllBe(x => (x.StatusCode & 0x4u) == 0, agg.ToString());
+            var tw = AggregateFamilies.FamilyOf(agg) == AggregateFamily.TimeWeighted;
+            SdkAggregateOracle.ShouldMatch(ours, SdkAggregateOracle.Run(agg, B.Round(At(40)), B.Round(At(0)), ten, sdkRows),
+                $"{agg} reverse late start", compareCalculatedInterpolatedBits: tw,
+                // pre-existing, #680: the SDK's Delta is Good with a Bad sample present; ours Uncertain (none here).
+                compareCodeBits: agg != HistoryAggregateType.Delta);
+        }
+    }
+
    /// <summary>Dense: every 2.5 s from −10 to +100, values cycling, one Bad sample at +22.5 (no empty interval).</summary>
    private Task SeedDenseAsync() => B.SeedAsync("P", DenseRows());

@@ -299,9 +327,13 @@ public abstract class ProcessedConformance : ConformanceBase
                     HistoryAggregateType.Average, HistoryAggregateType.Minimum, HistoryAggregateType.Maximum,
                     HistoryAggregateType.Count, HistoryAggregateType.TimeAverage, HistoryAggregateType.TimeAverage2,
                     HistoryAggregateType.Total, HistoryAggregateType.Total2, HistoryAggregateType.Interpolative,
+                     HistoryAggregateType.Range, HistoryAggregateType.Start, HistoryAggregateType.End,
+                     HistoryAggregateType.Delta, HistoryAggregateType.MinimumActualTime, HistoryAggregateType.MaximumActualTime,
+                     HistoryAggregateType.PercentGood, HistoryAggregateType.PercentBad,
                 })
        {
            var ours = (await src.ReadProcessedAsync("P", At(from), At(to), interval, agg, Ct)).Samples.ToList();
+            var startOrEnd = agg is HistoryAggregateType.Start or HistoryAggregateType.End;
            var tw = AggregateFamilies.FamilyOf(agg) == AggregateFamily.TimeWeighted;
            if (tw && agg != HistoryAggregateType.Interpolative)
            {
@@ -318,7 +350,12 @@ public abstract class ProcessedConformance : ConformanceBase
            SdkAggregateOracle.ShouldMatch(ours, SdkAggregateOracle.Run(agg, start, end, interval, rows),
                $"{agg} {from}→{to}/{intervalS}s",
                compareCalculatedInterpolatedBits: tw && !reverseInterpolative,
-                compareCodeBits: agg is not (HistoryAggregateType.TimeAverage or HistoryAggregateType.Total) && !reverseInterpolative);
+                // pre-existing, #680: TimeAverage/Total status rule; Start/End raw-sample timestamp + status; Delta status;
+                // PercentGood/Bad per-sample vs per-duration values (see SdkAggregateParityTests).
+                compareCodeBits: agg is not (HistoryAggregateType.TimeAverage or HistoryAggregateType.Total
+                                     or HistoryAggregateType.Delta) && !reverseInterpolative && !startOrEnd,
+                compareTimestamps: !startOrEnd,
+                compareValues: agg is not (HistoryAggregateType.PercentGood or HistoryAggregateType.PercentBad));
        }
    }

@@ -52,6 +52,8 @@ public static class SdkAggregateOracle
                or HistoryAggregateType.MinimumActualTime or HistoryAggregateType.MaximumActualTime =>
                new MinMaxAggregateCalculator(id, start, end, ms, false, cfg, null),
            HistoryAggregateType.Count => new CountAggregateCalculator(id, start, end, ms, false, cfg, null),
+            HistoryAggregateType.PercentGood or HistoryAggregateType.PercentBad =>
+                new StatusAggregateCalculator(id, start, end, ms, false, cfg, null),
            HistoryAggregateType.Start or HistoryAggregateType.End or HistoryAggregateType.Delta =>
                new StartEndAggregateCalculator(id, start, end, ms, false, cfg, null),
            HistoryAggregateType.Interpolative => new AggregateCalculator(id, start, end, ms, false, cfg, null),
@@ -84,8 +86,11 @@ public static class SdkAggregateOracle
    /// <param name="compareCalculatedInterpolatedBits">Also compare the Calculated / Interpolated aggregate bits.</param>
    /// <param name="compareCodeBits">Compare the severity + sub-code (false only for a documented status divergence).</param>
    /// <param name="compareValues">Compare the values (false only for a documented value divergence).</param>
+    /// <param name="compareTimestamps">Compare the timestamps (false only for a documented timestamp divergence).</param>
+    /// <param name="comparePartialBit">Compare the Partial bit (false only for a documented divergence).</param>
    public static void ShouldMatch(IReadOnlyList<DataValueSnapshot> ours, IReadOnlyList<Result> sdk, string label,
-        bool compareCalculatedInterpolatedBits = true, bool compareCodeBits = true, bool compareValues = true)
+        bool compareCalculatedInterpolatedBits = true, bool compareCodeBits = true, bool compareValues = true,
+        bool compareTimestamps = true, bool comparePartialBit = true)
    {
        var problems = new List<string>();
        if (ours.Count != sdk.Count) problems.Add($"interval count: ours {ours.Count}, sdk {sdk.Count}");
@@ -93,10 +98,13 @@ public static class SdkAggregateOracle
        {
            var o = ours[i];
            var s = sdk[i];
-            var bitMask = 0x4u | (compareCodeBits ? 0xFFFF0000u : 0u) | (compareCalculatedInterpolatedBits ? 0x3u : 0u);
-            var same = o.SourceTimestampUtc == s.Timestamp && (o.StatusCode & bitMask) == (s.StatusCode & bitMask);
-            if (!compareValues)
+            var bitMask = (comparePartialBit ? 0x4u : 0u) | (compareCodeBits ? 0xFFFF0000u : 0u)
+                          | (compareCalculatedInterpolatedBits ? 0x3u : 0u);
+            var same = (!compareTimestamps || o.SourceTimestampUtc == s.Timestamp)
+                       && (o.StatusCode & bitMask) == (s.StatusCode & bitMask);
+            if (!compareValues || (!compareCodeBits && StatusCode.IsBad(s.StatusCode)))
            {
+                // Values not compared (or the SDK answered Bad under a status this comparison does not check).
            }
            else if (same && !StatusCode.IsBad(s.StatusCode))
            {
@@ -139,6 +147,8 @@ public static class SdkAggregateOracle
        HistoryAggregateType.MinimumActualTime => ObjectIds.AggregateFunction_MinimumActualTime,
        HistoryAggregateType.MaximumActualTime => ObjectIds.AggregateFunction_MaximumActualTime,
        HistoryAggregateType.Count => ObjectIds.AggregateFunction_Count,
+        HistoryAggregateType.PercentGood => ObjectIds.AggregateFunction_PercentGood,
+        HistoryAggregateType.PercentBad => ObjectIds.AggregateFunction_PercentBad,
        HistoryAggregateType.Start => ObjectIds.AggregateFunction_Start,
        HistoryAggregateType.End => ObjectIds.AggregateFunction_End,
        HistoryAggregateType.Delta => ObjectIds.AggregateFunction_Delta,
@@ -132,6 +132,19 @@ public sealed class BucketPlanTests
        p.DataStartsInside(0, At(-5)).ShouldBeFalse();
    }

+    [Fact]
+    public void The_earliest_sample_is_needed_only_for_forward_reads_of_aggregates_other_than_Average()
+    {
+        // Review F2: backends skip the earliest-sample query when its answer cannot change a result.
+        var fwd = BucketPlan.Create(At(0), At(40), TimeSpan.FromSeconds(10));
+        var rev = BucketPlan.Create(At(40), At(0), TimeSpan.FromSeconds(10));
+        AggregateEvaluator.NeedsEarliestSample(HistoryAggregateType.Count, fwd).ShouldBeTrue();
+        AggregateEvaluator.NeedsEarliestSample(HistoryAggregateType.Average, fwd).ShouldBeFalse();
+        AggregateEvaluator.NeedsEarliestSample(HistoryAggregateType.Count, rev).ShouldBeFalse();
+        AggregateEvaluator.NeedsEarliestSample(HistoryAggregateType.Count, BucketPlan.Create(At(0), At(0), TimeSpan.Zero))
+            .ShouldBeFalse();
+    }
+
    [Fact]
    public void Assemble_fills_missing_buckets_with_the_empty_factory()
    {
@@ -84,7 +84,8 @@ public sealed class SdkAggregateParityTests
        var plan = BucketPlan.Create(At(from), At(to), TimeSpan.FromSeconds(intervalS));
        if (AggregateFamilies.FamilyOf(agg) == AggregateFamily.TimeWeighted)
            return TimeWeightedAggregator.Evaluate(agg, plan, data, Now);
-        return AggregateEvaluator.EvaluateAll(agg, plan, InMemoryRows(plan, data), data.Min(s => s.TimeUtc), Now);
+        return AggregateEvaluator.EvaluateAll(agg, plan, InMemoryRows(plan, data, agg == HistoryAggregateType.MaximumActualTime),
+            data.Min(s => s.TimeUtc), Now);
    }

    private static IReadOnlyList<SdkAggregateOracle.Result> Sdk(HistoryAggregateType agg, double from, double to,
@@ -130,13 +131,75 @@ public sealed class SdkAggregateParityTests
    [MemberData(nameof(Windows))]
    public void Per_sample_aggregates_match_the_SDK(double from, double to, double intervalS)
    {
-        foreach (var agg in new[]
-                 {
-                     HistoryAggregateType.Average, HistoryAggregateType.Minimum, HistoryAggregateType.Maximum,
-                     HistoryAggregateType.Count,
-                 })
-            SdkAggregateOracle.ShouldMatch(Ours(agg, from, to, intervalS, Dense), Sdk(agg, from, to, intervalS, Dense),
-                $"{agg} Dense {from}→{to}/{intervalS}s", compareCalculatedInterpolatedBits: false);
+        var failures = new List<string>();
+        foreach (var agg in PerSample)
+        {
+            try
+            {
+                SdkAggregateOracle.ShouldMatch(Ours(agg, from, to, intervalS, Dense), Sdk(agg, from, to, intervalS, Dense),
+                    $"{agg} Dense {from}→{to}/{intervalS}s", compareCalculatedInterpolatedBits: false,
+                    compareValues: !PercentAggregate(agg),
+                    // pre-existing, #680: the SDK's Start/End return the raw first/last sample (any quality) with its own
+                    // timestamp and stored status; ours return the first/last USABLE value stamped with the interval, under the
+                    // per-sample status rule. The SDK's Delta is Good even when a Bad sample sits in the interval; ours is
+                    // Uncertain. Values are compared wherever the SDK's answer is not Bad.
+                    compareTimestamps: !StartOrEnd(agg),
+                    compareCodeBits: !StartOrEnd(agg) && agg != HistoryAggregateType.Delta);
+            }
+            catch (Exception ex)
+            {
+                failures.Add(ex.Message);
+            }
+        }
+        failures.ShouldBeEmpty(string.Join("\n", failures));
+    }
+
+    private static readonly HistoryAggregateType[] PerSample =
+    [
+        HistoryAggregateType.Average, HistoryAggregateType.Minimum, HistoryAggregateType.Maximum,
+        HistoryAggregateType.Count, HistoryAggregateType.Range, HistoryAggregateType.Start, HistoryAggregateType.End,
+        HistoryAggregateType.Delta, HistoryAggregateType.MinimumActualTime, HistoryAggregateType.MaximumActualTime,
+        HistoryAggregateType.PercentGood, HistoryAggregateType.PercentBad,
+    ];
+
+    /// <summary>
+    ///     pre-existing, #680: ours is the per-SAMPLE percentage (100 × Good samples / all samples); the SDK's
+    ///     <c>StatusAggregateCalculator</c> is the per-DURATION one (Good time / interval time). Different definitions, so the
+    ///     values are not compared; timestamp, status and Partial still are.
+    /// </summary>
+    private static bool StartOrEnd(HistoryAggregateType agg) =>
+        agg is HistoryAggregateType.Start or HistoryAggregateType.End;
+
+    private static bool PercentAggregate(HistoryAggregateType agg) =>
+        agg is HistoryAggregateType.PercentGood or HistoryAggregateType.PercentBad;
+
+    [Theory]
+    [InlineData(HistoryAggregateType.TimeAverage)]
+    [InlineData(HistoryAggregateType.TimeAverage2)]
+    [InlineData(HistoryAggregateType.Total)]
+    [InlineData(HistoryAggregateType.Total2)]
+    [InlineData(HistoryAggregateType.Minimum)]
+    [InlineData(HistoryAggregateType.Maximum)]
+    [InlineData(HistoryAggregateType.Count)]
+    [InlineData(HistoryAggregateType.Range)]
+    [InlineData(HistoryAggregateType.Delta)]
+    [InlineData(HistoryAggregateType.Start)]
+    [InlineData(HistoryAggregateType.Average)]
+    public void A_reverse_read_over_data_starting_inside_the_window_sets_no_start_of_data_Partial(HistoryAggregateType agg)
+    {
+        // Review F1: the SDK applies its start-of-data rule only on a FORWARD read (a reverse read keys m_startOfData on the
+        // first value it is fed — the latest — which is past the window here). Reverse 40 → 0 at 10 s, first sample +7:
+        // (0,10] holds the start of the data but is NOT Partial.
+        var tw = AggregateFamilies.FamilyOf(agg) == AggregateFamily.TimeWeighted;
+        var ours = Ours(agg, 40, 0, 10, LateStart);
+        SdkAggregateOracle.ShouldMatch(ours, Sdk(agg, 40, 0, 10, LateStart), $"{agg} reverse LateStart",
+            compareCalculatedInterpolatedBits: tw,
+            // pre-existing, #680: Total over the interval the data starts inside (TimeAverage × interval vs the SDK's
+            // integral of the covered part); Start's raw-sample timestamp/status (see Per_sample_aggregates_match_the_SDK).
+            compareValues: agg != HistoryAggregateType.Total,
+            compareTimestamps: !StartOrEnd(agg),
+            compareCodeBits: !StartOrEnd(agg) && agg != HistoryAggregateType.Delta);
+        ours.ShouldAllBe(x => (x.StatusCode & HistorianInfoBits.HistorianPartial) == 0);
    }

    [Theory]
@@ -162,7 +225,7 @@ public sealed class SdkAggregateParityTests
    }

    /// <summary>The per-bucket statistics the backends' SQL computes, built in memory with <see cref="BucketPlan.IndexOf"/>.</summary>
-    internal static IEnumerable<BucketRow> InMemoryRows(BucketPlan plan, IEnumerable<StoredSample> samples)
+    internal static IEnumerable<BucketRow> InMemoryRows(BucketPlan plan, IEnumerable<StoredSample> samples, bool maximum = false)
    {
        foreach (var g in samples.Select(s => (s, b: plan.IndexOf(s.TimeUtc))).Where(x => x.b is not null)
                     .GroupBy(x => x.b!.Value).OrderBy(g => g.Key))
@@ -171,6 +234,9 @@ public sealed class SdkAggregateParityTests
            var usable = all.Where(s => s.IsUsable).ToList();
            var numeric = usable.Where(s => s.Value.Kind is HistorianValueKind.Double or HistorianValueKind.Int64
                or HistorianValueKind.Boolean).ToList();
+            // ActualTime: the extreme usable value, ties ⇒ earliest (the SQL's ROW_NUMBER order).
+            var extreme = numeric.OrderBy(s => maximum ? -s.Value.Number : s.Value.Number).ThenBy(s => s.TimeUtc)
+                .Cast<StoredSample?>().FirstOrDefault();
            yield return new BucketRow
            {
                Bucket = g.Key,
@@ -185,6 +251,8 @@ public sealed class SdkAggregateParityTests
                Sum = numeric.Count == 0 ? null : numeric.Sum(s => s.Value.Number),
                First = usable.Count == 0 ? HistorianValue.Empty : usable[0].Value,
                Last = usable.Count == 0 ? HistorianValue.Empty : usable[^1].Value,
+                Extreme = extreme?.Value.Number,
+                ExtremeTime = extreme?.TimeUtc,
            };
        }
    }