[tylerkron]

Core shipped both halves of a recording and nothing that joined them. ILiveSampleSource handed you an IAsyncEnumerable<LiveSample> in one namespace; CsvExporter asked for an ISampleSource in another; there was no supported way to connect the two. So the most basic DAQ workflow there is — stream from the device and record it to a file — had no answer in the package. Anyone who wanted it had to write the adapter themselves, and the adapter is fiddly enough that DAQiFi's own MCP server, the example app, and the test project each ended up with their own copy.

This adds the join. LiveSampleSource is a public ISampleSource over a live stream that yields rows as frames decode rather than buffering the session, so a recording's memory does not grow with its length. IStreamingDevice.RecordLiveSamplesToCsvAsync(writer, options, duration, bufferCapacity, ct) drives it through CsvExporter and returns a LiveCsvRecordingResult counting what reached the file and what did not — dropped, unmapped, non-monotonic. A recording with silent gaps is worse than one that failed, so nothing is swallowed.

Purely additive. Two new types, one extension method, one README recipe. No existing public signature moved — the breaking-change question from #612/#616 is left to you. ADR 0002's source-compatibility promise only constrains how LiveCsvRecordingResult may grow later (positional appends), which is what it permits.

What a reviewer should push back on

The extension hangs off IStreamingDevice but needs ILiveSampleSource too, so it does a runtime is not ILiveSampleSource check and throws ArgumentException. The recorder needs channels and identity from one interface and samples from the other, and neither implies the other (DaqifiStreamingDevice implements both). The alternatives were passing the same object twice, or a generic constraint that fails inference for anyone holding an IStreamingDevice. The runtime check has repo precedent in DaqifiAgent.RequireLiveSamples. It is the least-bad of three imperfect options and worth a second opinion.

duration is the clean stop; cancellationToken is the abort. The window arrives as a cancellation (the read it interrupts is unbounded) and is absorbed inside the private iterator, not at the exporter — that is what makes a timed recording end with a complete file, because the exporter sees the enumeration finish normally and flushes its last accumulated line. Letting the OCE reach the exporter silently discards that row. There is deliberately no graceful "stop signal" for a user-driven Stop button yet; today that means duration, or cancel and accept an abort.

GetSampleCountAsync returns 0 — exactly the "count unavailable" case the interface documents, and what makes CsvExporter skip percentage progress instead of dividing by an unknown total. Consequently there is no IProgress<int> parameter: a percentage of an unbounded stream is meaningless, and RowCount/SampleCount are the progress.

RowCount is timestamps, not samples. The exporter collapses consecutive rows sharing a timestamp into one line, so counting samples would over-report by the channel count. That correctness note now lives in Core next to the exporter instead of being rediscovered per consumer.

Also: values are written as ScaledValue (a user who configured a transducer asked for PSI, not the volts underneath); samples match channels by (Type, ChannelNumber) rather than object identity, because a device rebuilds its channel objects on status repopulation; blank device name/serial normalize to "unknown", the same disposition as #627.

Verification

Solution builds with -warnaserror under the root Directory.Build.props from #647: 0 warnings. Full suite green — 3878 Core tests on net9.0 and net10.0, 217 MCP tests, 0 failures. 45 new tests across LiveSampleSourceTests (24) and LiveCsvRecordingTests (21), weighted to edge cases: null/blank/duplicate channels, type-vs-number collision, empty stream, unmapped samples, backwards timestamps, three flavours of cancellation, no-enabled-channels, writer flush/non-dispose, buffer-capacity forwarding.

Every test was proved able to fail via a 19-mutation matrix, each mutation reverted after its run. Which mutation covered which test:

Mutation to production code	Tests turned red
M1 RowCount++ made unconditional	4
M2 unmapped samples not counted	3
M3 ScaledValue → Value	1
M4 blank serial kept instead of normalized	2
M5 duplicate channels kept	1
M6 non-monotonic never counted	1
M7 token not forwarded to the wrapped stream	3
M8 null channel entry allowed	1
M9b duration OCE rethrown instead of absorbed	8
M10 dropped reported absolutely, not as a delta	1
M11 disabled channels included	3
M12 bufferCapacity validation dropped	2
M13 duration validation dropped	2
M14 writer not flushed	1
M15 bufferCapacity not forwarded	1
M16b caller-cancel clause dropped from the catch filter	1
M17 up-front cancellation check dropped	0 — see below

M16 initially killed nothing: the && !cancellationToken.IsCancellationRequested half of the filter only matters when both tokens are cancelled, which no test reached. Rather than delete a defensible guard or leave it unpinned, a test was added that gates the scripted stream on a TaskCompletionSource so it decides exactly which tokens are cancelled when the enumeration throws.

M17 is killed by nothing, and is kept deliberately: CsvExporter.ExportAsync happens to make the same check, so no test can distinguish it — but the recorder's contract should not depend on a collaborator's internal ordering. Flagging it rather than quietly leaving it.

Bench-validated on real hardware (fw 3.7.2, sn 9090539562006014104), via a scratch console app project-referencing this branch, since the example CLI has no live-CSV command:

• 100 Hz / 5 s: rows=395 samples=790 dropped=0 unmapped=0 nonMonotonic=0. samples == 2 * rows exactly — the per-frame collapse holds on hardware. CSV timestamps exactly 10.000 ms apart across all 395 rows.
• 1000 Hz / 4 s with bufferCapacity: 1 (deliberately starved): rows=3006 samples=3025 dropped=3323. This is the "dropped-sample count surfaced to the caller" criterion demonstrated on hardware — the losses are reported, not silent.

All runs non-destructive: connect, enable channels, stream, record, stop, disconnect.

Deferred

This is step 2 of #639. Step 1 — promoting Daqifi.Mcp's internal SdCardSampleSource into Core — is filed as #655 and deliberately not here: it is a cross-project move (delete, re-point, keep MCP's tests green), and mixing that with new public API makes one PR a reviewer has to evaluate in two modes. #655 also records new evidence the issue did not have: the example app carries a third hand-rolled copy of the same adapter.

closes #639

---

Not merging — for review.

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Record live device streams directly to CSV (no session buffering)

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add an adapter to export live samples to CSV as frames decode (no memory growth).
• Provide IStreamingDevice extension to record for a duration or until stream ends, returning loss
 counters.
• Add README recipe and comprehensive tests covering stopping semantics and export correctness.

Diagram

graph TD
  A(["IStreamingDevice"]) --> B["RecordLiveSamplesToCsvAsync"] --> C(["ILiveSampleSource"])
  C --> D["ReadUntilStoppedAsync"] --> E["LiveSampleSource"] --> F["CsvExporter"] --> G["TextWriter / CSV"]

  subgraph Legend
    direction LR
    _api(["API / Interface"]) ~~~ _adapt["Adapter / Source"] ~~~ _out["Output"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Extension overload that takes both interfaces

• ➕ Avoids runtime type check and ArgumentException
• ➕ Makes requirements explicit in the signature
• ➖ Awkward call sites: same object often passed twice
• ➖ More API surface (multiple overloads) for one workflow

2. New combined interface (e.g., IStreamingLiveSampleDevice)

• ➕ Compile-time guarantee of both channel/identity and live stream availability
• ➕ Eliminates runtime check across all future APIs needing both
• ➖ Introduces a new public abstraction and adoption/migration burden
• ➖ Requires implementations/wrappers to implement yet another interface

3. Factory/helper that returns an ISampleSource (no device extension)

• ➕ Keeps recording orchestration out of IStreamingDevice extensions
• ➕ Gives building blocks without prescribing stop/abort policy
• ➖ Doesn’t directly solve the ‘one-liner record to CSV’ workflow
• ➖ Consumers still need to wire exporter + counters consistently

Recommendation: Keep the current approach: the IStreamingDevice extension provides the ergonomic ‘basic DAQ workflow’ entry point, while LiveSampleSource remains reusable for non-CSV sinks. The runtime ILiveSampleSource check is a reasonable compromise given ergonomics and existing repo precedent; the key is that the exception message/param are clear (as implemented) and tests lock in behavior. If API purity becomes a priority later, a dedicated combined interface can be added without breaking this additive entry point.

Files changed (5) +1398 / -0

Enhancement (2) +450 / -0

LiveCsvRecording.csAdd LiveCsvRecordingResult and RecordLiveSamplesToCsvAsync extension +231/-0

Add LiveCsvRecordingResult and RecordLiveSamplesToCsvAsync extension

• Introduces LiveCsvRecordingResult for reporting written rows/samples and losses (dropped/unmapped/non-monotonic). Adds an IStreamingDevice extension that snapshots enabled channels, streams via LiveSampleSource into CsvExporter, flushes the writer, and implements the ‘duration is clean stop, cancellation is abort’ policy via a private iterator that absorbs only the duration-triggered cancellation.

src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs

LiveSampleSource.csAdd LiveSampleSource: live stream → ISampleSource adapter +219/-0

Add LiveSampleSource: live stream → ISampleSource adapter

• Implements an ISampleSource over an IAsyncEnumerable<LiveSample> with no buffering, producing SampleRow items keyed by device identity + channel name/type. Maintains scalar counters for sample/row counts, unmapped samples, and non-monotonic timestamps; returns 0 for GetSampleCountAsync to disable percentage progress on live streams.

src/Daqifi.Core/Logging/Export/LiveSampleSource.cs

Tests (2) +917 / -0

LiveCsvRecordingTests.csAdd end-to-end tests for live CSV recording extension +514/-0

Add end-to-end tests for live CSV recording extension

• Introduces comprehensive tests for RecordLiveSamplesToCsvAsync covering argument validation, enabled-channel column selection, duration clean-stop vs cancellation abort, drop/unmapped accounting, writer flushing (without disposal), and a real decode-path regression test ensuring one row per frame and ascending timestamps.

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs

LiveSampleSourceTests.csAdd unit tests for LiveSampleSource adapter and counters +403/-0

Add unit tests for LiveSampleSource adapter and counters

• Adds deterministic tests validating channel key construction/order, duplicate handling, progress behavior when sample count is unknown, correct sample-to-row translation (scaled values), unmapped handling, row-count vs sample-count semantics, non-monotonic detection, and cancellation propagation into the wrapped stream.

src/Daqifi.Core.Tests/Logging/Export/LiveSampleSourceTests.cs

Documentation (1) +31 / -0

README.mdDocument live-to-CSV recording workflow +31/-0

Document live-to-CSV recording workflow

• Adds a README feature bullet and a dedicated section showing how to stream and record directly to CSV using RecordLiveSamplesToCsvAsync. Clarifies column selection (enabled-at-start), duration vs cancellation semantics, and how to use LiveSampleSource directly for non-TextWriter sinks.

README.md

[tylerkron]

/agentic_review

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Drop count misattributed ✓ Resolved 🐞 Bug ≡ Correctness

Description

RecordLiveSamplesToCsvAsync computes DroppedSampleCount from
ILiveSampleSource.DroppedLiveSampleCount deltas even though that counter is cumulative across all
live enumerations, so concurrent live consumers can inflate the recording result. This can also
violate the method’s own contract that “if no channel is enabled… the result is all zeros” by
reporting drops that happened during the call but were unrelated to this recording.

Code

src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[R170-174]

+        return new LiveCsvRecordingResult(
+            source.SampleCount,
+            source.RowCount,
+            live.DroppedLiveSampleCount - droppedBefore,
+            source.UnmappedSampleCount,

Relevance

●● Moderate

The counter-scope concern is plausible, but no close accepted or rejected precedent establishes team
treatment of concurrent drop attribution.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The recording result’s DroppedSampleCount is computed from a device-wide drop counter, but the live
stream explicitly allows multiple enumerations and documents that the drop count is cumulative
across all of them. Additionally, CsvExporter will early-return without starting enumeration when
there are no channels, so any drop delta observed during the call can be unrelated to this
recording, contradicting the method’s “all zeros” contract for no enabled channels.

src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[61-65]
src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[147-175]
src/Daqifi.Core/Device/Internal/LiveSampleStream.cs[52-69]
src/Daqifi.Core/Logging/Export/CsvExporter.cs[79-86]

Agent prompt

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

## Issue description
`RecordLiveSamplesToCsvAsync` reports `DroppedSampleCount` as a delta of `ILiveSampleSource.DroppedLiveSampleCount`, but that counter is device-wide across *all* enumerations. This makes the result incorrect when another live consumer is running, and it can produce non-zero drops even when the export is a no-op (no enabled channels), contradicting the XML docs.

## Issue Context
- `CsvExporter` returns early without enumerating when there are no channels.
- The live drop counter is explicitly cumulative across every enumeration and multiple enumerations are allowed.

## Fix Focus Areas
- src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[61-65]
- src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[147-175]
- src/Daqifi.Core/Device/Internal/LiveSampleStream.cs[52-69]
- src/Daqifi.Core/Logging/Export/CsvExporter.cs[79-86]

### Concrete changes to implement
1. **Guarantee the documented no-op behavior**: if `channels.Count == 0`, return `default(LiveCsvRecordingResult)` *before* sampling `DroppedLiveSampleCount` / computing its delta.
2. **Fix or clarify drop semantics** (choose one):
  - Preferred (correct): introduce a recording-scoped drop counter (e.g., new API that returns per-enumeration drops, or a new adapter/stream implementation used by recording that counts bounded-buffer drops for *this* recording only).
  - Minimal (if per-recording drops aren’t possible without API changes): update XML docs for `DroppedSampleCount` (and README recipe if needed) to explicitly state it is a **device-wide delta during the recording window**, and may include drops from other concurrent live enumerations.

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

---

2. Non-monotonic test deadline ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

The subscription-wait loop uses DateTime.UtcNow to enforce a timeout, but wall-clock time can jump
forward/backward, making the loop terminate early or run longer than intended and introducing
intermittent CI flakiness.

Code

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[R388-391]

+        var deadline = DateTime.UtcNow + Timeout;
+        while (device.LiveSubscriptionCount == 0 && DateTime.UtcNow < deadline)
+        {
+            await Task.Delay(5);

Relevance

●●● Strong

Recent accepted precedent explicitly requires replacing DateTime.UtcNow deadlines with a monotonic
clock in test wait loops.

PR-#602

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new wait loop computes a deadline from DateTime.UtcNow and polls until either
LiveSubscriptionCount changes or UtcNow crosses the deadline; this is inherently sensitive to
wall-clock adjustments because the exit condition depends on UtcNow.

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[385-395]

Agent prompt

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

### Issue description
`RecordLiveSamplesToCsvAsync_AgainstARealDeviceStream_CollapsesEachFrameIntoOneAscendingRow` gates progress on `DateTime.UtcNow` when waiting for `LiveSubscriptionCount` to become non-zero. Since `DateTime.UtcNow` is wall-clock time (not monotonic), NTP/time adjustments can cause the test’s timeout logic to behave incorrectly, creating intermittent test failures/hangs.

### Issue Context
This loop is the replacement for a fixed `Task.Delay(...)` and is intended to make the test deterministic on loaded machines; using a monotonic clock preserves that intent.

### Fix Focus Areas
- src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[388-393]

### Suggested fix
Replace the `DateTime.UtcNow` deadline logic with a monotonic timer (e.g., `Stopwatch`). Example:

```csharp
var sw = Stopwatch.StartNew();
while (device.LiveSubscriptionCount == 0 && sw.Elapsed < Timeout)
{
   await Task.Delay(5);
}
```

(or reuse an existing `WaitForAsync`-style helper that measures elapsed time via `Stopwatch`).

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

---

3. Fixed-delay test flakiness ✓ Resolved 🐞 Bug ☼ Reliability

Description

The “real decode path” test relies on Task.Delay(400) to wait for the recording to subscribe,
which can race on slow/loaded CI and intermittently drop the initial injected frames, failing
assertions. This makes the PR’s new test suite non-deterministic.

Code

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[R369-372]

+        // The recording subscribes from inside the exporter, so there is no synchronous point for a
+        // test to hook. Give it time to get there before injecting, or the leading frames decode
+        // while nothing is listening.
+        await Task.Delay(400);

Relevance

●●● Strong

Recent accepted precedents prioritize deterministic synchronization and eliminating timing-sensitive
test races.

PR-#587
PR-#602

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test uses a hard-coded 400ms delay to avoid injecting frames before subscription; this is
explicitly a race workaround and will be sensitive to environment timing, causing intermittent
failures when the assumption doesn’t hold.

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[369-372]

Agent prompt

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

## Issue description
`RecordLiveSamplesToCsvAsync_AgainstARealDeviceStream...` uses a fixed `Task.Delay(400)` as a proxy for “the exporter subscribed”. On slower machines, subscription may take longer and the test can lose leading injected frames, making it flaky.

## Issue Context
The comment in the test already notes there is no synchronous hook for subscription; the current solution is a hard-coded delay.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[369-377]

### Concrete changes to implement
Replace the fixed delay with a deterministic synchronization strategy, e.g. one of:
1. **Add an internal test seam** (preferred): expose an internal event/task from the live-stream subscription path (e.g., when the live enumeration registers/subscribes) and await it in the test.
2. **Handshake via observable output**: poll until the CSV header exists (or until a first row appears) before injecting the bulk frames, with a bounded timeout.
3. **Inject frames until observed**: send one “sentinel” frame in a bounded retry loop until `result.SampleCount`/writer content indicates at least one frame was processed, then proceed with the rest.

Keep the test time-bounded to avoid hangs.

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

---

Context sources

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit ec52604

Results up to commit 70b949a ⚖️ Balanced

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

1. Drop count misattributed ✓ Resolved 🐞 Bug ≡ Correctness

Description

RecordLiveSamplesToCsvAsync computes DroppedSampleCount from
ILiveSampleSource.DroppedLiveSampleCount deltas even though that counter is cumulative across all
live enumerations, so concurrent live consumers can inflate the recording result. This can also
violate the method’s own contract that “if no channel is enabled… the result is all zeros” by
reporting drops that happened during the call but were unrelated to this recording.

Code

src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[R170-174]

+        return new LiveCsvRecordingResult(
+            source.SampleCount,
+            source.RowCount,
+            live.DroppedLiveSampleCount - droppedBefore,
+            source.UnmappedSampleCount,

Relevance

●● Moderate

The counter-scope concern is plausible, but no close accepted or rejected precedent establishes team
treatment of concurrent drop attribution.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The recording result’s DroppedSampleCount is computed from a device-wide drop counter, but the live
stream explicitly allows multiple enumerations and documents that the drop count is cumulative
across all of them. Additionally, CsvExporter will early-return without starting enumeration when
there are no channels, so any drop delta observed during the call can be unrelated to this
recording, contradicting the method’s “all zeros” contract for no enabled channels.

src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[61-65]
src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[147-175]
src/Daqifi.Core/Device/Internal/LiveSampleStream.cs[52-69]
src/Daqifi.Core/Logging/Export/CsvExporter.cs[79-86]

Agent prompt

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

## Issue description
`RecordLiveSamplesToCsvAsync` reports `DroppedSampleCount` as a delta of `ILiveSampleSource.DroppedLiveSampleCount`, but that counter is device-wide across *all* enumerations. This makes the result incorrect when another live consumer is running, and it can produce non-zero drops even when the export is a no-op (no enabled channels), contradicting the XML docs.

## Issue Context
- `CsvExporter` returns early without enumerating when there are no channels.
- The live drop counter is explicitly cumulative across every enumeration and multiple enumerations are allowed.

## Fix Focus Areas
- src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[61-65]
- src/Daqifi.Core/Logging/Export/LiveCsvRecording.cs[147-175]
- src/Daqifi.Core/Device/Internal/LiveSampleStream.cs[52-69]
- src/Daqifi.Core/Logging/Export/CsvExporter.cs[79-86]

### Concrete changes to implement
1. **Guarantee the documented no-op behavior**: if `channels.Count == 0`, return `default(LiveCsvRecordingResult)` *before* sampling `DroppedLiveSampleCount` / computing its delta.
2. **Fix or clarify drop semantics** (choose one):
  - Preferred (correct): introduce a recording-scoped drop counter (e.g., new API that returns per-enumeration drops, or a new adapter/stream implementation used by recording that counts bounded-buffer drops for *this* recording only).
  - Minimal (if per-recording drops aren’t possible without API changes): update XML docs for `DroppedSampleCount` (and README recipe if needed) to explicitly state it is a **device-wide delta during the recording window**, and may include drops from other concurrent live enumerations.

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

---

2. Fixed-delay test flakiness ✓ Resolved 🐞 Bug ☼ Reliability

Description

The “real decode path” test relies on Task.Delay(400) to wait for the recording to subscribe,
which can race on slow/loaded CI and intermittently drop the initial injected frames, failing
assertions. This makes the PR’s new test suite non-deterministic.

Code

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[R369-372]

+        // The recording subscribes from inside the exporter, so there is no synchronous point for a
+        // test to hook. Give it time to get there before injecting, or the leading frames decode
+        // while nothing is listening.
+        await Task.Delay(400);

Relevance

●●● Strong

Recent accepted precedents prioritize deterministic synchronization and eliminating timing-sensitive
test races.

PR-#587
PR-#602

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test uses a hard-coded 400ms delay to avoid injecting frames before subscription; this is
explicitly a race workaround and will be sensitive to environment timing, causing intermittent
failures when the assumption doesn’t hold.

src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[369-372]

Agent prompt

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

## Issue description
`RecordLiveSamplesToCsvAsync_AgainstARealDeviceStream...` uses a fixed `Task.Delay(400)` as a proxy for “the exporter subscribed”. On slower machines, subscription may take longer and the test can lose leading injected frames, making it flaky.

## Issue Context
The comment in the test already notes there is no synchronous hook for subscription; the current solution is a hard-coded delay.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Logging/Export/LiveCsvRecordingTests.cs[369-377]

### Concrete changes to implement
Replace the fixed delay with a deterministic synchronization strategy, e.g. one of:
1. **Add an internal test seam** (preferred): expose an internal event/task from the live-stream subscription path (e.g., when the live enumeration registers/subscribes) and await it in the test.
2. **Handshake via observable output**: poll until the CSV header exists (or until a first row appears) before injecting the bulk frames, with a bounded timeout.
3. **Inject frames until observed**: send one “sentinel” frame in a bounded retry loop until `result.SampleCount`/writer content indicates at least one frame was processed, then proceed with the rest.

Keep the test time-bounded to avoid hangs.

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

---

[tylerkron]

Qodo round 2 was right on both counts, so both are fixed in 3958ddb rather than rebutted.

Drop count misattributed (High). Real, and it broke this method's own contract. DroppedLiveSampleCount is device-wide and cumulative across every enumeration; with no enabled channels CsvExporter returns before enumerating anything, yet the recording still took a delta over that shared counter — so a concurrent live consumer's drops came back as this recording's, contradicting the documented all-zeros result. The delta is now taken only when there was something to record, and the DroppedSampleCount XML doc states plainly that with a second consumer running the number is a delta over a shared counter rather than a per-recording one. Qodo's preferred fix — a recording-scoped counter — would mean new device API for a case a single-consumer caller never hits, so I took the minimal one and documented the residual honestly. New test ..._NoEnabledChannels_DoesNotChargeAnotherConsumersDrops pins it (the fake now grows the counter between the two reads, so it fails without the fix).

Fixed-delay test flakiness (Medium). Also real, and the same shape #633 removed elsewhere today. LiveSampleStream now publishes an internal LiveSubscriptionCount, incremented once every sample handler is attached, so the test waits for the subscription itself instead of a guessed 400 ms. Internal-only, behind the existing InternalsVisibleTo.

Bench validation re-run fresh on hardware this round (fw 3.7.2, sn 9090539562006014104), not inherited:

• 100 Hz / 5 s: rows=395 samples=790 dropped=0 unmapped=0 nonMonotonic=0, samples == 2 * rows, CSV timestamps 10.000 ms apart across all 395 rows.
• 1000 Hz / 4 s at bufferCapacity: 1 (deliberately starved): rows=3162 samples=3170 dropped=3175 — losses surfaced to the caller, and visible in the file as empty cells rather than silent gaps.

Also re-verified independently this round: the change is still purely additive against current main (f015149, unmoved — 0 deletions in the diff, no existing public signature touched), and two more of the mutation matrix reproduce — M11 (disabled channels included) → 3 red, exactly as claimed; M7 (token not forwarded) → 4 red, the 3 claimed plus LiveSampleSourceTests.StreamSamples_Cancellation_EndsTheEnumeration, which my slightly broader form of the mutation also kills.

Full suite green on the new head: 3879 Core on net9.0 and net10.0, 217 MCP, 0 failures; solution builds -warnaserror clean.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 3958ddb

[tylerkron]

Qodo round 3 found one new bug and it is valid, so it is fixed in ec52604 rather than rebutted.

Non-monotonic test deadline (Reliability). The subscription-wait loop added in 3958ddb — itself the fix for round 2's fixed-delay race — gated its timeout on DateTime.UtcNow. That is a wall clock, so an NTP step mid-wait either cuts the wait short (losing exactly the leading frames the loop exists to protect, reintroducing the race under a different cause) or stretches it past the intended bound. Now a Stopwatch. Qodo's cited precedent holds: #602 replaced a UtcNow deadline for the same reason, and Stopwatch is what every other wait loop in this suite already uses — the remaining DateTime.UtcNow hits in the test project are sample timestamps, not deadlines.

Test-only change; no production code touched, so the hardware bench result on 3958ddb still stands unchanged.

Full suite on the new head: 3879 Core passed on net9.0 and net10.0, 217 MCP, 0 failures, solution -warnaserror clean. Also test-merged current origin/main (3e77696, including #648 and #649) into a scratch branch: merges cleanly and the suite is green there too (3884 per TFM).

Round 2's two findings remain fixed and are shown as resolved.

[tylerkron]

/agentic_review

[qodo-code-review]

Qodo round 3 found one new bug and it is valid, so it is fixed in ec52604 rather than rebutted.

Non-monotonic test deadline (Reliability). The subscription-wait loop added in 3958ddb — itself the fix for round 2's fixed-delay race — gated its timeout on DateTime.UtcNow. That is a wall clock, so an NTP step mid-wait either cuts the wait short (losing exactly the leading frames the loop exists to protect, reintroducing the race under a different cause) or stretches it past the intended bound. Now a Stopwatch. Qodo's cited precedent holds: #602 replaced a UtcNow deadline for the same reason, and Stopwatch is what every other wait loop in this suite already uses — the remaining DateTime.UtcNow hits in the test project are sample timestamps, not deadlines.

Test-only change; no production code touched, so the hardware bench result on 3958ddb still stands unchanged.

Full suite on the new head: 3879 Core passed on net9.0 and net10.0, 217 MCP, 0 failures, solution -warnaserror clean. Also test-merged current origin/main (3e77696, including #648 and #649) into a scratch branch: merges cleanly and the suite is green there too (3884 per TFM).

Round 2's two findings remain fixed and are shown as resolved.

Confirmed. finding 2 is validly fixed in ec52604: the subscription wait now uses Stopwatch.StartNew() and elapsed.Elapsed < Timeout (LiveCsvRecordingTests.cs:391-394), eliminating the wall-clock deadline race. The reported full-suite results and clean merge validation provide solid coverage; no further action is needed from Qodo.

[qodo-code-review]

Code review by qodo was updated up to the latest commit ec52604

[tylerkron]

Qodo-clean, CI green — ready for review.

Round 4 on ec52604 reports 🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0), with all three findings raised across rounds 2-3 shown resolved and every evidence link anchored to ec52604. All 3 inline review threads resolved, 0 unresolved. Re-checked both surfaces after a settle window — unchanged. CI build green on this head.

Test-merged current origin/main (3e77696, i.e. #648 and #649 merged) into a scratch branch: merges cleanly, builds -warnaserror clean, suite green at 3884 Core per TFM on net9.0 and net10.0 plus 217 MCP. Still purely additive against current main — 1484 insertions, 0 deletions.

Bench validation was fresh against the real board, not inherited: 100 Hz / 5 s gave rows=395, samples=790, dropped=0, CSV timestamps exactly 10.000 ms apart; a deliberately starved 1000 Hz / 4 s at bufferCapacity: 1 gave dropped=3175, surfacing the loss to the caller. The only change since that run is a 5-line test-only fix (Stopwatch for DateTime.UtcNow in one wait loop), so those numbers still stand.