From 2003b343bfbd07ac0c41dc18280aef93a8b8a442 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 22 May 2026 10:54:08 -0400
Subject: [PATCH] fix(server): resolve Medium code-review finding (Server-003)

Fix ReadRawAsync: correct XML doc from newest-first to oldest-first
(ascending source timestamp per OPC UA Part 11); move maxValuesPerNode
cap inside the time-window filter loop so paging limits apply to
in-window results only, not the whole buffer snapshot.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 code-reviews/Server/findings.md                |  4 ++--
 .../Phase7/RingBufferHistoryWriter.cs          | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/code-reviews/Server/findings.md b/code-reviews/Server/findings.md
index 1d12bbf..e34e115 100644
--- a/code-reviews/Server/findings.md
+++ b/code-reviews/Server/findings.md
@@ -60,13 +60,13 @@
 | Severity | Medium |
 | Category | Correctness & logic bugs |
 | Location | `src/Server/ZB.MOM.WW.OtOpcUa.Server/Phase7/RingBufferHistoryWriter.cs:96-119` |
-| Status | Open |
+| Status | Resolved |
 
 **Description:** `ReadRawAsync`'s XML doc claims "newest-first," but `TagRingBuffer.Snapshot()` returns oldest-to-newest and the loop preserves that order — so results are oldest-first. Also `maxValuesPerNode` is capped against total buffer size *before* the `[startUtc, endUtc)` filter, so a paged read returns the oldest in-window samples, contradicting the doc and usual HistoryRead expectations.
 
 **Recommendation:** Make code and doc agree on ordering (raw HistoryRead is normally ascending source-timestamp). Apply `maxValuesPerNode` to the in-window count, not the whole buffer.
 
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-22 — corrected XML doc from "newest-first" to "oldest-first (ascending source timestamp, matching OPC UA Part 11 §6.4 raw-values default)"; moved `maxValuesPerNode` cap inside the time-window loop so the limit applies only to in-window results, not the whole buffer snapshot.
 
 ### Server-004
 | Field | Value |
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Server/Phase7/RingBufferHistoryWriter.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Server/Phase7/RingBufferHistoryWriter.cs
index dab8f1a..52bdfcc 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Server/Phase7/RingBufferHistoryWriter.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Server/Phase7/RingBufferHistoryWriter.cs
@@ -90,8 +90,12 @@ public sealed class RingBufferHistoryWriter : IHistoryWriter, IHistorianDataSour
 
     /// <summary>
     ///     Returns samples in the ring buffer whose source timestamp falls within
-    ///     [<paramref name="startUtc"/>, <paramref name="endUtc"/>), newest-first.
-    ///     <paramref name="maxValuesPerNode"/> caps the result count.
+    ///     [<paramref name="startUtc"/>, <paramref name="endUtc"/>), oldest-first
+    ///     (ascending source timestamp, matching OPC UA Part 11 §6.4 raw-values default).
+    ///     <paramref name="maxValuesPerNode"/> caps the count of <em>in-window</em> results,
+    ///     not the whole buffer snapshot — a paged read therefore returns the oldest
+    ///     <paramref name="maxValuesPerNode"/> samples within the time window, not the oldest
+    ///     samples in the buffer irrespective of the window (Server-003).
     /// </summary>
     public Task<HistoryReadResult> ReadRawAsync(
         string fullReference,
@@ -104,15 +108,19 @@ public sealed class RingBufferHistoryWriter : IHistoryWriter, IHistorianDataSour
             return Task.FromResult(new HistoryReadResult([], null));
 
         var all = buffer.Snapshot();
-        var limit = (int)Math.Min(maxValuesPerNode, (uint)all.Length);
-        var result = new List<DataValueSnapshot>(limit);
 
+        // Iterate the full snapshot (oldest → newest) and apply the time-window filter first;
+        // then cap with maxValuesPerNode so the limit applies to in-window results only.
+        // Snapshot() returns an array bounded by _capacity so this is O(capacity) at most.
+        var result = new List<DataValueSnapshot>();
         foreach (var snap in all)
         {
-            if (result.Count >= limit) break;
             var ts = snap.SourceTimestampUtc ?? snap.ServerTimestampUtc;
             if (ts >= startUtc && ts < endUtc)
+            {
                 result.Add(snap);
+                if (maxValuesPerNode > 0 && result.Count >= (int)maxValuesPerNode) break;
+            }
         }
 
         return Task.FromResult(new HistoryReadResult(result, null));