feat(streaming): answer "am I actually getting the sample rate I asked for?"

[tylerkron]

What was wrong

If you streamed from a DAQiFi device and wanted to know whether you were actually getting the rate you asked for, Core could not tell you. You could ask for 1 kHz and get something else — because frames were lost, or because the device's clock was not keeping real time — and nothing in the library would say so. The pieces that existed each answered a narrower question: the gap detector flags individual stalls as they happen, the dropped-sample counter reports only what a slow consumer of your own threw away, and the device's SYSTem:STReam:STATS? counters describe what the firmware believes it sent rather than what your application received. Anyone who wanted the actual answer had to re-implement it, which is what the desktop app did.

How it was fixed

A new AcquisitionStatistics you attach for the duration of an acquisition and then read a snapshot from. Per channel it reports the sample count, the rate the host really received at, the rate the device's own timestamps claim, the smallest/largest/mean gap between samples, and the value range and mean; device-wide it reports the totals and how far behind the device's account of the time the host was.

The two rates are reported side by side deliberately, and that is the part worth pushing back on if you disagree: both dropping below the commanded rate means samples went missing, while the two disagreeing means the device clock and real time have parted company — a distinction no single number can carry. The bench run below is exactly that case.

It hooks nothing. It subscribes to the per-channel sample events the decode pipeline already raises — the same seam StreamSamplesAsync uses — so no decode-path code changed at all, and a consumer that never attaches one is unaffected. Recording a sample allocates nothing.

The device's timestamps are not assumed to advance. TimestampProcessor deliberately reconstructs an earlier time when a frame arrives out of order, so a backwards step is excluded from the jitter bounds, the device-clock span is measured between the earliest and latest timestamps seen, and a per-channel OutOfOrderSampleCount reports that it happened rather than smoothing it away.

Three things the desktop original got wrong are fixed on the way across, so a SummaryLogger replacement is not a like-for-like port: value extremes are now seeded from the first sample (desktop left them at zero, so a channel sitting at 4.5 V reported a minimum of 0 V), means divide by the samples actually seen rather than a configured window size, and a window runs until you Reset() it instead of being swapped out automatically every N samples.

Verification

Full suite green on net9.0 (3069 Core + 86 Mcp) and net10.0 (3069), 0 warnings; 26 new tests. Six mutations confirm the tests bite: zero-seeded extremes (6 failures), an off-by-one in the rate denominator (3), a missed unsubscribe on channel repopulation (1), a removed post-dispose guard (1), a negative interval folded into the jitter bounds (1), and a device-clock span taken from first-and-last instead of the extremes (1).

Bench — Nq1 fw 3.7.2 on /dev/cu.usbmodem1101, serial, non-destructive (connect, enable AI0-2, stream, stop, disconnect; no reboot/format/delete/SD:GET/firmware/LAN writes). Commanded 1000 Hz for 3 s:

channel  n      measured Hz  devclock Hz  min int ms  mean int ms  max int ms
Analog  0   2383      794.44      1000.03       0.000        1.000       2.000
Analog  1   2383      794.59      1000.03       0.000        1.000       2.000
Analog  2   2383      794.59      1000.03       0.000        1.000       2.000
latency: min -2.04 ms  mean 306.99 ms  max 617.71 ms

The device's clock says 1000.03 Hz; the host received 794.4 Hz — the 79.4 % ratio this bench unit has shown on every fire, and the firmware clock defect (daqifi-nyquist-firmware #716) that motivated reporting both. The numbers are self-consistent: 3 s × (1 − 794/1000) = 618 ms of accumulated drift, which is the 617.71 ms maximum latency. The alternating 0 ms / 2 ms intervals are the duplicate-timestamp quirk (firmware #717) showing up as jitter. A 500 Hz run and a mid-stream Reset() behaved the same way.

closes #502

Not merging — for review.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Add AcquisitionStatistics to report measured sample rate, jitter, and latency

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add opt-in AcquisitionStatistics to measure host-observed acquisition health during streaming
• Expose immutable per-channel and device-wide snapshot records (rates, gaps, value range, latency)
• Add comprehensive unit and allocation-cost tests plus README usage documentation

Diagram

graph TD
  A([Caller app]) --> B["IStreamingDevice"] --> C["IChannel(s)"] --> D["SampleReceived"] --> E["AcquisitionStatistics"] --> F["Snapshot records"]
  A --> E
  G["StreamSamplesAsync"] --> E

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Compute stats inside decode/streaming pipeline

• ➕ No event subscription bookkeeping or resubscription on ChannelsPopulated
• ➕ Could potentially avoid per-sample event overhead if events are disabled
• ➖ Not truly opt-in; risks impacting all streaming consumers
• ➖ Increases coupling and complexity in the decode path (higher regression risk)
• ➖ Harder to keep allocation-free without affecting existing APIs

2. Rely on firmware counters (e.g., SYSTem:STReam:STATS?)

• ➕ Low host-side overhead; leverages device-maintained counters
• ➕ Can work even if host misses events due to consumer issues
• ➖ Does not answer 'what did my application actually receive'
• ➖ Cannot diagnose device-clock drift vs host-time without host measurements
• ➖ Firmware counters can diverge from host reality under transport loss

3. Expose a single 'effective sample rate' number only

• ➕ Simpler API surface and easier to explain at a glance
• ➕ Less room for consumer confusion about which rate to trust
• ➖ Loses critical diagnostic signal: clock drift vs dropped/missing samples
• ➖ Would force callers to re-infer drift via separate tooling/logging

Recommendation: Keep the PR’s approach: an opt-in observer over existing per-channel SampleReceived events with immutable snapshots, reporting both host-measured and device-clock-derived rates. This preserves zero-cost when unused, avoids decode-path changes, and the dual-rate design directly supports diagnosing both sample loss and device clock drift.

Files changed (5) +1291 / -0

Enhancement (3) +653 / -0

AcquisitionStatistics.csImplement opt-in AcquisitionStatistics aggregator with device attachment support +477/-0

Implement opt-in AcquisitionStatistics aggregator with device attachment support

• Adds a thread-safe, allocation-free-per-sample statistics aggregator that can either attach to an IStreamingDevice (subscribing to per-channel SampleReceived and resubscribing on ChannelsPopulated) or be fed manually. Tracks per-channel counts, timestamp gaps, value min/max/sum, plus device-wide sample totals and min/max/mean host-vs-device timestamp latency, and exposes Reset() and Snapshot().

src/Daqifi.Core/Device/AcquisitionStatistics.cs

AcquisitionStatisticsSnapshot.csAdd immutable device-wide snapshot record for acquisition stats +62/-0

Add immutable device-wide snapshot record for acquisition stats

• Defines an immutable snapshot record containing window start time, total samples, first/last receive times, latency min/max/mean, and a per-channel statistics list. Provides a Duration convenience property measured from first to last received samples.

src/Daqifi.Core/Device/AcquisitionStatisticsSnapshot.cs

ChannelAcquisitionStatistics.csAdd immutable per-channel acquisition statistics record and rate math +114/-0

Add immutable per-channel acquisition statistics record and rate math

• Defines per-channel snapshot data (timestamps, host receive times, min/max gap, value range, mean) and computes MeasuredSampleRateHz vs DeviceClockSampleRateHz side-by-side. Encapsulates correct rate calculation using (SampleCount - 1) over the observed span and handles single-sample/zero-span cases safely.

src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs

Tests (1) +605 / -0

AcquisitionStatisticsTests.csAdd comprehensive tests for AcquisitionStatistics correctness and cost +605/-0

Add comprehensive tests for AcquisitionStatistics correctness and cost

• Introduces extensive unit coverage for value extremes/means, rate and interval math (including drift and repeated timestamps), latency tracking, Reset() window semantics, multi-channel ordering, thread-safety, and device-attachment resubscription on channel repopulation. Includes allocation-budget tests for Record() and attached decode-path overhead.

src/Daqifi.Core.Tests/Device/AcquisitionStatisticsTests.cs

Documentation (1) +33 / -0

README.mdDocument acquisition statistics feature and usage example +33/-0

Document acquisition statistics feature and usage example

• Adds an 'Acquisition statistics' section showing how to attach AcquisitionStatistics, stream, and print per-channel measured rate vs device-clock rate plus jitter/value range. Updates the feature table to advertise acquisition health reporting.

README.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Out-of-order meaning mismatch ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New

Description

RecordCore now treats a sample as “out-of-order” when its timestamp is behind the channel’s
furthest-advanced timestamp (state.LatestTimestamp), which can count additional samples beyond those
that are earlier than the immediately previous sample. This conflicts with the public XML docs for
OutOfOrderSampleCount and can mislead callers interpreting the metric (and which intervals were
excluded from jitter bounds).

Code

src/Daqifi.Core/Device/AcquisitionStatistics.cs[R412-415]

+                    // frame after it jumps forward again to roughly where it was; measuring that
+                    // jump from the rewound value would report the whole rewind as a gap, which is
+                    // an artifact of the reordering rather than a stall in the data.
+                    RecordInterval(state, timestamp.Ticks - state.LatestTimestamp.Ticks);

Relevance

●●● Strong

Team often fixes XML docs when semantics drift; likely update OutOfOrderSampleCount docs or rename
for clarity.

PR-#321
PR-#348
PR-#513

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new code measures gaps against state.LatestTimestamp (the high-water mark) and increments
OutOfOrderCount for negative deltas, while the public record type documents OutOfOrderSampleCount
as “timestamp earlier than the sample before them,” which is a different definition.

src/Daqifi.Core/Device/AcquisitionStatistics.cs[325-352]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[410-428]
src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[65-69]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AcquisitionStatistics.RecordCore` computes the interval against the channel’s furthest-advanced timestamp (`state.LatestTimestamp`) and increments `OutOfOrderCount` when that delta is negative. This means `OutOfOrderSampleCount` effectively becomes “samples with timestamp behind the high-water mark,” not strictly “earlier than the sample before them” as currently documented.

## Issue Context
This behavior appears intentional (it prevents a stale frame followed by a recovery frame from producing a huge artificial ‘worst gap’), but the public API docs currently describe a different semantics.

## Fix Focus Areas
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[410-415]
- src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[65-69]

## Suggested fix
Choose one:
1) **Update the XML docs** (recommended if this high-water-mark definition is intended): change the OutOfOrderSampleCount description to match “timestamp earlier than the latest timestamp seen so far (high-water mark)” and clarify that intervals for those samples are excluded from jitter bounds.
2) **Preserve the old ‘previous sample’ meaning**: track a separate per-channel `LastRecordedTimestamp` for the *counting* definition while keeping `LatestTimestamp` as the high-water mark for interval measurement; update logic accordingly so docs and behavior match.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Out-of-order inflates jitter ✓ Resolved 🐞 Bug ≡ Correctness

Description

In AcquisitionStatistics.RecordCore, an out-of-order (backward) timestamp is counted and excluded
from jitter bounds, but the same out-of-order timestamp is still stored into PreviousTimestamp, so
the next interval can become artificially large and incorrectly update Min/MaxSampleInterval. This
can surface as a false "worst gap" spike that is purely an artifact of frame reordering.

Code

src/Daqifi.Core/Device/AcquisitionStatistics.cs[424]

+                state.PreviousTimestamp = timestamp;

Relevance

●●● Strong

Team often accepts subtle streaming/timestamp correctness fixes to avoid misleading stats/jitter
artifacts.

PR-#362
PR-#353
PR-#150

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

RecordInterval excludes negative deltas by incrementing out-of-order count and returning, but
RecordCore still unconditionally updates PreviousTimestamp to the (possibly backward) timestamp,
making the next delta span across the out-of-order sample and potentially polluting jitter bounds.
The new test asserts that a backward step must not land in interval bounds, but it doesn’t cover
this follow-on inflation scenario.

src/Daqifi.Core/Device/AcquisitionStatistics.cs[326-333]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[407-408]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[424-424]
src/Daqifi.Core.Tests/Device/AcquisitionStatisticsTests.cs[205-226]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AcquisitionStatistics.RecordInterval(...)` treats negative timestamp deltas as out-of-order and excludes them from jitter bounds, but `RecordCore(...)` still overwrites `state.PreviousTimestamp` with the out-of-order timestamp. This causes the *next* sample’s delta to be computed against the rewound timestamp, which can inflate the measured interval and incorrectly update `MinIntervalTicks` / `MaxIntervalTicks`.

### Issue Context
The API/docs/tests indicate that backward steps should be counted as out-of-order and kept out of the jitter bounds, but the current state update makes the following forward delta span across the out-of-order sample.

### Fix Focus Areas
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[326-349]
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[407-408]
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[424-426]

Suggested approach:
- When `intervalTicks < 0`, still increment `OutOfOrderCount` and still update `EarliestTimestamp`/`LatestTimestamp` extremes, but do **not** update the reference used for the next interval calculation (e.g., keep a separate `PreviousMonotonicTimestamp`, or only update `PreviousTimestamp` when the delta is non-negative).
- Add/extend a unit test demonstrating a large backward step followed by a normal timestamp, asserting that `MaxSampleInterval` does not get inflated by the compensating forward delta.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Backward timestamps skew stats ✓ Resolved 🐞 Bug ≡ Correctness

Description

AcquisitionStatistics assumes sample timestamps are monotonic; if TimestampProcessor yields a
backward hostTimestamp for an out-of-order frame, per-channel Min/MaxSampleInterval can become
negative and DeviceClockSampleRateHz/MeanSampleInterval can collapse to 0 or otherwise become
misleading when the overall timestamp span is non-positive.

Code

src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[R109-112]

+        internal static double Rate(long sampleCount, TimeSpan span) =>
+            sampleCount > 1 && span.Ticks > 0
+                ? (sampleCount - 1) / span.TotalSeconds
+                : 0.0;

Relevance

●●● Strong

Team often hardens stats/clock edge cases; backward timestamps could silently break reported
rates/intervals.

PR-#150
PR-#353
PR-#349

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

TimestampProcessor explicitly supports out-of-order frames by emitting a negative delta and
reconstructing a timestamp that moves backward; StreamFrameDecoder uses that reconstructed timestamp
as the DataSample timestamp. AcquisitionStatistics then computes interval ticks by subtracting
consecutive timestamps without guarding against non-positive intervals, and
ChannelAcquisitionStatistics.Rate returns 0 when the overall span is non-positive, which can happen
when timestamps move backward.

src/Daqifi.Core/Device/TimestampProcessor.cs[175-186]
src/Daqifi.Core/Device/Internal/StreamFrameDecoder.cs[417-422]
src/Daqifi.Core/Device/TimestampGapDetector.cs[99-105]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[358-375]
src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[90-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TimestampProcessor` can intentionally generate a negative `SecondsBetweenMessages` for out-of-order frames, which makes reconstructed `DataSample.Timestamp` go backwards. `AcquisitionStatistics` currently uses device-derived timestamps as if they are strictly increasing.

This leads to:
- Negative per-sample intervals being recorded (and potentially reported as `MinSampleInterval`).
- `DeviceClockSampleRateHz` and `MeanSampleInterval` becoming 0 or otherwise misleading when `LastSampleTimestamp <= FirstSampleTimestamp`.

## Issue Context
- Decode path timestamps: `StreamFrameDecoder` uses `TimestampProcessor.ProcessTimestamp(...).Timestamp` as the `hostTimestamp` stored in `DataSample.Timestamp`.
- Out-of-order handling: `TimestampProcessor` may set `secondsBetweenMessages` negative when rollover detection is deemed a false positive (out-of-order message).
- Existing precedent: `TimestampGapDetector.IsGap(...)` treats non-positive deltas as “no usable delta”. AcquisitionStatistics should similarly define a policy for backward/non-increasing timestamps.

## Fix Focus Areas
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[358-375]
- src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[90-112]

## Suggested fix approach
1. In `AcquisitionStatistics.RecordCore`, when computing `intervalTicks` from consecutive sample timestamps, treat `intervalTicks <= 0` as an unusable interval:
  - Option A (simple): do not update `MinIntervalTicks`/`MaxIntervalTicks` on non-positive intervals.
  - Option B (more explicit): track a per-channel counter/flag for out-of-order timestamps and expose it in snapshot (if desired).
2. For device-clock rate/mean interval computations, avoid relying on `FirstSampleTimestamp`/`LastSampleTimestamp` being ordered:
  - Track `MinSampleTimestamp`/`MaxSampleTimestamp` in `ChannelState` and compute device-clock span from `(max - min)`.
  - Or, if you want “arrival-order first/last”, then explicitly treat non-positive span as “unknown” rather than returning 0 (e.g., expose a nullable rate, or add an `IsDeviceClockRateValid` flag).
3. Add/extend a unit test that records timestamps out of order (e.g., t0, t0+2ms, t0+1ms) and asserts:
  - intervals never go negative
  - device-clock rate/mean interval remains meaningful (or explicitly marked invalid), per chosen policy.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit 0f5afd5

Results up to commit 6918327 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Backward timestamps skew stats ✓ Resolved 🐞 Bug ≡ Correctness

Description

AcquisitionStatistics assumes sample timestamps are monotonic; if TimestampProcessor yields a
backward hostTimestamp for an out-of-order frame, per-channel Min/MaxSampleInterval can become
negative and DeviceClockSampleRateHz/MeanSampleInterval can collapse to 0 or otherwise become
misleading when the overall timestamp span is non-positive.

Code

src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[R109-112]

+        internal static double Rate(long sampleCount, TimeSpan span) =>
+            sampleCount > 1 && span.Ticks > 0
+                ? (sampleCount - 1) / span.TotalSeconds
+                : 0.0;

Relevance

●●● Strong

Team often hardens stats/clock edge cases; backward timestamps could silently break reported
rates/intervals.

PR-#150
PR-#353
PR-#349

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

TimestampProcessor explicitly supports out-of-order frames by emitting a negative delta and
reconstructing a timestamp that moves backward; StreamFrameDecoder uses that reconstructed timestamp
as the DataSample timestamp. AcquisitionStatistics then computes interval ticks by subtracting
consecutive timestamps without guarding against non-positive intervals, and
ChannelAcquisitionStatistics.Rate returns 0 when the overall span is non-positive, which can happen
when timestamps move backward.

src/Daqifi.Core/Device/TimestampProcessor.cs[175-186]
src/Daqifi.Core/Device/Internal/StreamFrameDecoder.cs[417-422]
src/Daqifi.Core/Device/TimestampGapDetector.cs[99-105]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[358-375]
src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[90-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TimestampProcessor` can intentionally generate a negative `SecondsBetweenMessages` for out-of-order frames, which makes reconstructed `DataSample.Timestamp` go backwards. `AcquisitionStatistics` currently uses device-derived timestamps as if they are strictly increasing.

This leads to:
- Negative per-sample intervals being recorded (and potentially reported as `MinSampleInterval`).
- `DeviceClockSampleRateHz` and `MeanSampleInterval` becoming 0 or otherwise misleading when `LastSampleTimestamp <= FirstSampleTimestamp`.

## Issue Context
- Decode path timestamps: `StreamFrameDecoder` uses `TimestampProcessor.ProcessTimestamp(...).Timestamp` as the `hostTimestamp` stored in `DataSample.Timestamp`.
- Out-of-order handling: `TimestampProcessor` may set `secondsBetweenMessages` negative when rollover detection is deemed a false positive (out-of-order message).
- Existing precedent: `TimestampGapDetector.IsGap(...)` treats non-positive deltas as “no usable delta”. AcquisitionStatistics should similarly define a policy for backward/non-increasing timestamps.

## Fix Focus Areas
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[358-375]
- src/Daqifi.Core/Device/ChannelAcquisitionStatistics.cs[90-112]

## Suggested fix approach
1. In `AcquisitionStatistics.RecordCore`, when computing `intervalTicks` from consecutive sample timestamps, treat `intervalTicks <= 0` as an unusable interval:
  - Option A (simple): do not update `MinIntervalTicks`/`MaxIntervalTicks` on non-positive intervals.
  - Option B (more explicit): track a per-channel counter/flag for out-of-order timestamps and expose it in snapshot (if desired).
2. For device-clock rate/mean interval computations, avoid relying on `FirstSampleTimestamp`/`LastSampleTimestamp` being ordered:
  - Track `MinSampleTimestamp`/`MaxSampleTimestamp` in `ChannelState` and compute device-clock span from `(max - min)`.
  - Or, if you want “arrival-order first/last”, then explicitly treat non-positive span as “unknown” rather than returning 0 (e.g., expose a nullable rate, or add an `IsDeviceClockRateValid` flag).
3. Add/extend a unit test that records timestamps out of order (e.g., t0, t0+2ms, t0+1ms) and asserts:
  - intervals never go negative
  - device-clock rate/mean interval remains meaningful (or explicitly marked invalid), per chosen policy.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 02664af ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Out-of-order inflates jitter ✓ Resolved 🐞 Bug ≡ Correctness

Description

In AcquisitionStatistics.RecordCore, an out-of-order (backward) timestamp is counted and excluded
from jitter bounds, but the same out-of-order timestamp is still stored into PreviousTimestamp, so
the next interval can become artificially large and incorrectly update Min/MaxSampleInterval. This
can surface as a false "worst gap" spike that is purely an artifact of frame reordering.

Code

src/Daqifi.Core/Device/AcquisitionStatistics.cs[424]

+                state.PreviousTimestamp = timestamp;

Relevance

●●● Strong

Team often accepts subtle streaming/timestamp correctness fixes to avoid misleading stats/jitter
artifacts.

PR-#362
PR-#353
PR-#150

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

RecordInterval excludes negative deltas by incrementing out-of-order count and returning, but
RecordCore still unconditionally updates PreviousTimestamp to the (possibly backward) timestamp,
making the next delta span across the out-of-order sample and potentially polluting jitter bounds.
The new test asserts that a backward step must not land in interval bounds, but it doesn’t cover
this follow-on inflation scenario.

src/Daqifi.Core/Device/AcquisitionStatistics.cs[326-333]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[407-408]
src/Daqifi.Core/Device/AcquisitionStatistics.cs[424-424]
src/Daqifi.Core.Tests/Device/AcquisitionStatisticsTests.cs[205-226]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AcquisitionStatistics.RecordInterval(...)` treats negative timestamp deltas as out-of-order and excludes them from jitter bounds, but `RecordCore(...)` still overwrites `state.PreviousTimestamp` with the out-of-order timestamp. This causes the *next* sample’s delta to be computed against the rewound timestamp, which can inflate the measured interval and incorrectly update `MinIntervalTicks` / `MaxIntervalTicks`.

### Issue Context
The API/docs/tests indicate that backward steps should be counted as out-of-order and kept out of the jitter bounds, but the current state update makes the following forward delta span across the out-of-order sample.

### Fix Focus Areas
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[326-349]
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[407-408]
- src/Daqifi.Core/Device/AcquisitionStatistics.cs[424-426]

Suggested approach:
- When `intervalTicks < 0`, still increment `OutOfOrderCount` and still update `EarliestTimestamp`/`LatestTimestamp` extremes, but do **not** update the reference used for the next interval calculation (e.g., keep a separate `PreviousMonotonicTimestamp`, or only update `PreviousTimestamp` when the delta is non-negative).
- Add/extend a unit test demonstrating a large backward step followed by a normal timestamp, asserting that `MaxSampleInterval` does not get inflated by the compensating forward delta.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 02664af

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 747cb6f

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0f5afd5

[tylerkron]

Qodo-clean, CI green — ready for review. (4 rounds on head 0f5afd5; the last round is Bugs (0) / Rule violations (0) with all 3 inline threads resolved, byte-identical on a settle re-check 4 min later, and its SHA reference matches the head. Rounds 1-3 each found one real defect in how the statistics handle a device timestamp that moves backwards — fixed in 02664af, 747cb6f, 0f5afd5 — see the threads. Full suite green net9.0 (3070 Core + 86 Mcp) and net10.0 (3070); bench re-validated on the Nq1 after every code change, 500 Hz and 1 kHz, non-destructive.) Not merging — for review.