fix(sdcard): IsNonResultLine no longer false-positives on filenames starting with 'error' (#190)

[cptkoolbeenz]

Summary

Pre-fix, IsNonResultLine returned true for any line starting with the bare substring "ERROR" (case-insensitive). GetSdCardFilesAsync silently dropped legit SD filenames whose basename starts with "error" (e.g. error_log.csv, Errors_summary.bin) because the permissive classifier treated them as non-result error/status text.

Tightened to require ERROR followed by :, /\t, !, or end-of-line. The strict **ERROR prefix path and IsScpiErrorLine are unchanged.

Test plan

• New test GetSdCardFilesAsync_FilenamesStartingWithErrorAreNotMisclassified cans three filenames (error_log.csv, Errors_summary.bin, normal.bin) into the LIST? response and asserts all three round-trip into the parsed file list.
• 891 tests pass on net9.0 + net10.0 (was 890).

Refs

• Closes fix(sdcard): IsNonResultLine false-positives on filenames starting with 'error' (PR #182 follow-up) #190
• Parallel Python fix: daqifi/daqifi-python-core PR feat: Ensure StreamMessageConsumer fully encapsulates protobuf for external consumers #102

[qodo-code-review]

Review Summary by Qodo

Fix IsNonResultLine false-positives on error-prefixed filenames

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Fix IsNonResultLine false-positives on SD filenames starting with "error"
• Tighten ERROR prefix matching to require specific delimiters (:, space, !, tab)
• Add regression test for filenames like error_log.csv and Errors_summary.bin
• Prevent legitimate SD card files from being silently dropped during parsing

Diagram

flowchart LR
  A["IsNonResultLine<br/>bare ERROR check"] -->|"Tighten to require<br/>delimiter after ERROR"| B["ERROR followed by<br/>: space ! or tab"]
  B -->|"Prevents matching"| C["Filenames like<br/>error_log.csv"]
  C -->|"Now included in"| D["GetSdCardFilesAsync<br/>results"]

Loading

File Changes

1. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs 🐞 Bug fix +20/-3

Tighten ERROR prefix matching in IsNonResultLine

• Refactored IsNonResultLine to require ERROR followed by specific delimiters (:,  , !,
 \t)
• Added length check to ensure ERROR is at least 5 characters before checking next character
• Preserved **ERROR prefix path and IsScpiErrorLine behavior unchanged
• Updated comment to document the fix for issue #190 and explain the tightened matching logic

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs

---

2. src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs 🧪 Tests +27/-0

Add regression test for error-prefixed filenames

• Added new regression test GetSdCardFilesAsync_FilenamesStartingWithErrorAreNotMisclassified
• Test verifies three filenames (error_log.csv, Errors_summary.bin, normal.bin) are correctly
 parsed
• Asserts all three files appear in the returned file list (previously only normal.bin would pass)
• Includes detailed comment explaining the regression scenario from issue #190

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Parser still skips bare ERROR ✓ Resolved 🐞 Bug ≡ Correctness

Description

SdCardFileListParser.ParseFileList still skips any listing line that starts with the bare prefix
"ERROR" (case-insensitive), so filenames like "error_log.csv"/"Errors_summary.bin" can still be
silently omitted even after IsNonResultLine was tightened. GetSdCardFilesAsync always runs this
parser after ThrowIfSdCardListError, so the PR may not fully fix #190 for response shapes that
return bare filenames.

Code

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[R758-774]

        private static bool IsNonResultLine(string line)
        {
            var trimmed = line.TrimStart();
-            return trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)
-                || trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase);
+            if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase))
+                return true;
+            // Must be followed by a non-letter so plain filenames like
+            // "error_log.csv" (which TrimStart leaves intact) don't match.
+            if (trimmed.Length >= 5
+                && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase))
+            {
+                if (trimmed.Length == 5)
+                    return true;
+                var next = trimmed[5];
+                if (next == ':' || next == ' ' || next == '!' || next == '\t')
+                    return true;
+            }
+            return false;

Evidence

The SD list flow always calls the parser after the error-check; the parser still has a permissive
StartsWith("ERROR") skip that will treat legitimate filenames beginning with "error" as error
lines and omit them.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[287-363]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[34-48]

Agent prompt

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

## Issue description
`SdCardFileListParser.ParseFileList` currently skips any line starting with the bare prefix `ERROR` (case-insensitive). This will still drop legitimate filenames that start with `error`/`Errors` when the device response line begins with that filename.

## Issue Context
Even though `DaqifiStreamingDevice.ThrowIfSdCardListError` now uses a stricter `IsNonResultLine`, `GetSdCardFilesAsync` subsequently calls `SdCardFileListParser.ParseFileList(lines)` which independently filters lines.

## Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[41-48]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[358-362]

## Implementation notes
- Align the parser’s error skipping logic with `IsScpiErrorLine` / the tightened delimiter rules (e.g., skip `**ERROR...` and `ERROR:` but not bare `ERROR<letter>`).
- Add/adjust tests to cover responses containing bare filenames starting with `error` (no `Daqifi/` prefix).

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

---

2. Test missing GetFileName ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

GetSdCardFilesAsync_FilenamesWithSingleBangSurvive computes the expected value by only stripping the
"Daqifi/" prefix, but the production parser also applies Path.GetFileName before returning
SdCardFileInfo.FileName. This makes the assertion inconsistent with production behavior for listing
lines that contain additional directory segments (e.g., "Daqifi/subdir/file.bin").

Code

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[R186-189]

+            const string daqifiPrefix = "Daqifi/";
+            var expected = filename.StartsWith(daqifiPrefix, StringComparison.OrdinalIgnoreCase)
+                ? filename.Substring(daqifiPrefix.Length)
+                : filename;

Evidence

The test strips only the Daqifi prefix, while the production parser additionally extracts the
basename using Path.GetFileName, so their normalization steps are not equivalent.

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[182-190]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[64-72]

Agent prompt

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

### Issue description
The test computes `expected` by stripping only the `Daqifi/` prefix, but the production parser returns only the basename via `Path.GetFileName(...)`. This can make the test assertion diverge from real behavior for path-shaped entries.

### Issue Context
`SdCardFileListParser.ParseFileList` strips `Daqifi/` (OrdinalIgnoreCase) and then extracts the filename with `Path.GetFileName(path)`.

### Fix Focus Areas
- src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[186-190]

### Suggested change
After computing `expected`, apply `expected = Path.GetFileName(expected);` (and optionally mirror other parser normalization if you later add cases with whitespace/size tokens).

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

---

3. Brittle prefix stripping test ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

GetSdCardFilesAsync_FilenamesWithSingleBangSurvive computes the expected filename using a
case-sensitive StartsWith("Daqifi/"), but the production parser strips "Daqifi/" using
OrdinalIgnoreCase, so any casing variation in responses would fail the test despite correct
behavior.

Code

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[R182-185]

+            // filename may or may not have the Daqifi/ prefix stripped depending
+            // on the parser's path handling; just confirm it survived.
+            var expected = filename.StartsWith("Daqifi/") ? filename.Substring("Daqifi/".Length) : filename;
+            Assert.Contains(expected, names);

Evidence

The test uses a case-sensitive prefix check to decide whether to strip Daqifi/, but the parser
strips that prefix case-insensitively, so the test’s expectation can diverge from production for
different casing.

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[182-185]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[64-68]

Agent prompt

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

### Issue description
The test computes `expected` using a case-sensitive `StartsWith("Daqifi/")`, while production strips the prefix using `StringComparison.OrdinalIgnoreCase`. This makes the test brittle and can produce false failures if the response ever uses different casing (e.g., `daqifi/` or `DAQIFI/`).

### Issue Context
Production behavior in `SdCardFileListParser.ParseFileList` strips the `Daqifi/` prefix case-insensitively. The new test should mirror that behavior.

### Fix Focus Areas
- src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[182-185]

### Suggested change
Update the test to use `filename.StartsWith("Daqifi/", StringComparison.OrdinalIgnoreCase)` (and keep the same substring length), or assert presence of either stripped/unstripped variant if you truly want to allow both.

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

---

4. error! filename misclassified ✓ Resolved 🐞 Bug ≡ Correctness

Description

MatchesErrorPrefix treats a single '!' immediately after the ERROR prefix as an error/status
delimiter, so a legitimate filename like "error!log.bin" will be dropped from ParseFileList and may
also be treated as a non-result line by SD listing classification. This conflicts with
ValidateSdCardFileName which allows '!' in filenames, so callers can create/upload a file that later
cannot be listed reliably.

Code

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[R147-148]

+            var next = trimmed[prefix.Length];
+            return next == ':' || next == ' ' || next == '!' || next == '\t';

Evidence

The new delimiter logic explicitly treats ! as an error delimiter, and ParseFileList skips those
lines. Separately, SD filename validation only rejects quotes/newlines/CR/semicolons, implying !
is allowed and such filenames are expected to work end-to-end.

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[25-55]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[140-149]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[827-840]

Agent prompt

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

### Issue description
`SdCardFileListParser.MatchesErrorPrefix` currently treats `!` as a valid delimiter after `ERROR`, which will classify filenames beginning with `error!` as error/status lines and drop them from listings.

### Issue Context
- File listing parsing skips any line where `IsErrorResponseLine(path)` is true.
- Filename validation allows `!`, so such filenames are permitted by API contract.

### Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[140-149]

### Suggested fix
- Remove `next == '!'` from the delimiter set, since the known firmware status shape `"Error !! ..."` is still matched via the space delimiter.
- If you must support a no-space variant, match it explicitly (e.g., `ERROR!!`), not any single `!` character.

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

---

View more (3)

5. Duplicated error-line logic ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The same "is this line an error/status line?" predicate is implemented separately in
DaqifiStreamingDevice.IsNonResultLine and SdCardFileListParser.IsErrorResponseLine, so a future
change to delimiters/trim behavior could be applied to one but not the other. This increases the
chance of reintroducing #190-style filename false-positives in only one of the two SD-card parsing
paths.

Code

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[R126-134]

+            // Defensive Trim — current callers already pre-trim, but a future
+            // reuse with raw CRLF input must still classify "ERROR\r" as an
+            // error line rather than letting the trailing '\r' disqualify it
+            // (mirrors IsNonResultLine in DaqifiStreamingDevice).
+            var trimmed = line.Trim();
+            if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase))
+                return true;
+            if (trimmed.Length >= 5
+                && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase))

Evidence

The parser explicitly states it "Mirrors DaqifiStreamingDevice.IsNonResultLine" and re-implements
the same Trim + StartsWith("**ERROR") + StartsWith("ERROR") + delimiter check. The device code
likewise performs Trim() before applying the same classification logic, meaning there are now two
independent implementations that must stay in sync.

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[119-143]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[749-780]

Agent prompt

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

### Issue description
`IsNonResultLine` (in `DaqifiStreamingDevice`) and `IsErrorResponseLine` (in `SdCardFileListParser`) intentionally mirror the same delimiter-based `ERROR` classification logic. Keeping two copies risks future divergence (one updated, the other forgotten), potentially reintroducing the SD filename misclassification bug.

### Issue Context
Both call sites are part of the same LIST? pipeline: one classifies response lines (pre-parse) and the other skips error lines during per-line parsing. They should remain identical.

### Fix Focus Areas
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[762-765]
- src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[126-134]

### Suggested fix approach
1. Introduce a single internal helper (e.g., `internal static class SdCardErrorLineClassifier` in `src/Daqifi.Core/Device/SdCard/`) with `IsErrorLikeLine(string line)` implementing the delimiter logic (including the defensive `Trim()` behavior).
2. Replace both `IsNonResultLine` and `IsErrorResponseLine` bodies with calls to this helper (or remove one wrapper if appropriate).
3. Keep existing tests; they should continue to pass and will now guard a single implementation.

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

---

6. Misleading error-line comment ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

SdCardFileListParser comments say the "ERROR" prefix must be followed by a non-letter, but
IsErrorResponseLine only treats ':', space, '!', tab, or end-of-line as error delimiters. This
documentation mismatch can cause future edits to accidentally broaden the check (e.g., to any
non-letter) and reintroduce the #190 filename false-positive.

Code

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[R43-50]

+                // Skip SCPI error responses: "**ERROR: -200, ..." or
+                // "ERROR: -200, ...". Bare "ERROR" prefix can't be used
+                // here because filenames like "error_log.csv" /
+                // "Errors_summary.bin" emitted without the Daqifi/
+                // directory prefix would also match (closes #190 second
+                // location — IsNonResultLine in DaqifiStreamingDevice
+                // had the same bug). Require a non-letter follower so
+                // ordinary filenames pass through.

Evidence

The parser comment claims a generic “non-letter follower” rule, but the new helper only recognizes a
small explicit delimiter set, so the comment is inaccurate relative to the code that will actually
run.

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[43-51]
src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[118-135]

Agent prompt

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

### Issue description
`SdCardFileListParser`’s comments describe the error-line rule as “ERROR followed by a non-letter”, but the implementation actually only matches specific delimiters (`:`, space, `!`, tab, or end-of-line). This is misleading and could prompt a future refactor that reintroduces the original regression.

### Issue Context
The code intentionally **does not** treat `_` (underscore) and other non-letters as delimiters, so filenames like `ERROR_archive.bin` are not misclassified.

### Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[43-51]
- src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[118-122]

### Suggested fix
Update both comment blocks to explicitly list the accepted delimiters (e.g., “followed by `:`, space, `!`, tab, or end-of-line”), rather than saying “non-letter”.

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

---

7. Missing edge-case sdcard test ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

The new regression test does not truly cover the edge case where an SD card listing consists solely
of filenames starting with error, because it uses prefixed paths like Daqifi/normal.bin that
don’t start with error and thus wouldn’t have triggered the historical StartsWith("ERROR")
misclassification. As a result, the checklist-required all-error*-only scenario remains untested
and could regress without being caught.

Code

src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[R71-96]

+        [Fact]
+        public async Task GetSdCardFilesAsync_FilenamesStartingWithErrorAreNotMisclassified()
+        {
+            // Regression for #190: IsNonResultLine's prior bare "ERROR"
+            // prefix check false-positived on legit SD filenames whose
+            // basename starts with "error" (e.g. "error_log.csv"). The
+            // permissive classifier dropped them from the parsed file
+            // list. Tightened to require ERROR followed by `:`/` `/`!`/
+            // tab so ordinary filenames pass through.
+            var device = new TestableSdCardStreamingDevice("TestDevice");
+            device.CannedTextResponse = new List<string>
+            {
+                "Daqifi/error_log.csv",
+                "Daqifi/Errors_summary.bin",
+                "Daqifi/normal.bin",
+            };
+            device.Connect();
+
+            var files = await device.GetSdCardFilesAsync();
+
+            var names = files.Select(f => f.FileName).ToList();
+            Assert.Contains("error_log.csv", names);
+            Assert.Contains("Errors_summary.bin", names);
+            Assert.Contains("normal.bin", names);
+            Assert.Equal(3, files.Count);
+        }

Evidence

PR Compliance ID 2 explicitly requires a unit test for SD card listings containing filenames
starting with error, including the specific edge case where the entire listing is comprised of
such filenames. However, the added test’s CannedTextResponse includes lines like
Daqifi/normal.bin and other Daqifi/-prefixed entries, and IsNonResultLine classifies
non-result lines by checking whether the trimmed line itself starts with ERROR/**ERROR; a
Daqifi/ prefix prevents that predicate from ever matching, meaning the test can pass even if the
original regression (bare filename lines like error_log.csv) still exists and the
all-error*-only listing behavior is not exercised.

Add regression test for SD card listings containing files starting with 'error'
src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[71-96]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[758-774]

Agent prompt

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

## Issue description
The regression test meant to validate handling of SD card filenames starting with `error` does not reproduce the historical failure mode and fails to cover the checklist-required edge case where *all* returned LIST? lines are `error*` filenames (with no “normal” filename present). Because the canned response lines are prefixed with `Daqifi/`, they do not start with `error` and therefore would not have matched the old `StartsWith("ERROR")`-based classification in `IsNonResultLine`, allowing the test to pass even if the real regression remains.

## Issue Context
- Compliance requires a unit test that covers SD card listings containing `error*` filenames, including the scenario where the entire listing consists solely of such filenames.
- Current test `GetSdCardFilesAsync_FilenamesStartingWithErrorAreNotMisclassified` includes `Daqifi/normal.bin`, so it only validates mixed listings and not the “only error-prefixed filenames” case.
- `IsNonResultLine` checks `TrimStart()` and then `StartsWith("ERROR"...)` / `StartsWith("**ERROR"...)`; if test inputs are prefixed (e.g., `Daqifi/...`), the predicate won’t trigger, so the historical misclassification risk isn’t exercised.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Device/SdCard/SdCardOperationsTests.cs[71-96]
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[758-775]

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

---

8. SCPI-only doc is misleading ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

IsErrorResponseLine’s XML summary claims it detects only “SCPI error response” lines, but it is also
used as the permissive SD non-result classifier (including firmware status text such as "Error !!
..."). This documentation mismatch can cause future maintainers to “tighten” the method to SCPI-only
and accidentally re-break SD listing error/status handling.

Code

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[R120-126]

+        /// Returns true if the line is a SCPI error response — either the
+        /// canonical <c>**ERROR</c> marker or a bare <c>ERROR</c> token, in
+        /// each case followed by a SCPI delimiter (<c>:</c>, space, <c>!</c>,
+        /// tab) or end of line. Trims both ends so a bare <c>"ERROR\r"</c>
+        /// from CRLF line endings still classifies. Plain filenames whose
+        /// basename starts with <c>error</c> / <c>Errors</c> pass through
+        /// unmatched (closes #190).

Evidence

The parser helper’s documentation says SCPI-only, while the device code comments and call site show
it is intentionally used as a permissive classifier for firmware status text too.

src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[119-126]
src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[749-758]

Agent prompt

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

### Issue description
`SdCardFileListParser.IsErrorResponseLine` is documented as SCPI-only, but it is used for broader SD response classification (including firmware status lines).

### Issue Context
`DaqifiStreamingDevice.IsNonResultLine` delegates to this method specifically to catch firmware status text like `"Error !! ..."`.

### Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs[120-131]

### Suggested fix
Update the XML summary to state it matches device error/status lines used for SD listing classification (SCPI `**ERROR...` and firmware `Error !! ...`-style text), not just SCPI errors.

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

---

9. IsNonResultLine comment mismatched ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The updated comment claims ERROR is tightened to be followed by ":", space, or "*", but the
implementation only accepts ":", space, "!", tab, or end-of-line (plus a separate **ERROR prefix
check). This mismatch can mislead future maintenance/debugging around the error-shape rules.

Code

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[R749-772]

        // Permissive: any line that looks like a device error or status message,
        // including firmware text such as "Error !! ...". Used to recognize that
        // the parser would yield no result, without polluting LastScpiError with
-        // non-SCPI text.
+        // non-SCPI text. Closes #190 — bare "ERROR" prefix false-positives on
+        // legit SD filenames that happen to start with "error" (e.g.
+        // "error_log.csv"), which would mask the file from GetSdCardFilesAsync's
+        // returned list. Tightened to require ERROR followed by ":", " " (firmware
+        // pattern "Error !!"), or "*" (the "**ERROR" SCPI marker, also matched by
+        // IsScpiErrorLine).
        private static bool IsNonResultLine(string line)
        {
            var trimmed = line.TrimStart();
-            return trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)
-                || trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase);
+            if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase))
+                return true;
+            // Must be followed by a non-letter so plain filenames like
+            // "error_log.csv" (which TrimStart leaves intact) don't match.
+            if (trimmed.Length >= 5
+                && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase))
+            {
+                if (trimmed.Length == 5)
+                    return true;
+                var next = trimmed[5];
+                if (next == ':' || next == ' ' || next == '!' || next == '\t')
+                    return true;

Evidence

The comment enumerates * but the conditional delimiter check does not include it; **ERROR is
separately matched by StartsWith("**ERROR").

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[749-774]

Agent prompt

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

## Issue description
The comment above `IsNonResultLine` mentions `*` as a post-`ERROR` delimiter, but the code does not treat `'*'` as a valid delimiter; `**ERROR` is handled separately.

## Issue Context
The implementation checks `next == ':' || next == ' ' || next == '!' || next == '\t'` and also handles `**ERROR` with a separate `StartsWith("**ERROR")`.

## Fix Focus Areas
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[749-772]

## Implementation notes
- Either remove `"*"` from the comment, or reword to explicitly say `**ERROR` is matched as a separate prefix case.

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

---

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit c7d18d5

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 96dd1cb

Warning

/improve is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Category	Suggestion	Impact
Possible issue	Prevent null input crashes Guard against null or whitespace-only inputs before calling Trim() in IsErrorResponseLine to prevent potential NullReferenceException. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [40-45] internal static bool IsErrorResponseLine(string line) { + if (string.IsNullOrWhiteSpace(line)) + return false; + var trimmed = line.Trim(); return MatchesErrorPrefix(trimmed, "**ERROR") || MatchesErrorPrefix(trimmed, "ERROR"); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: While adding a defensive null check provides extra safety, it offers marginal value here since the known upstream callers typically filter out or guarantee non-null strings.	Low
	Add defensive input validation Add a null or whitespace guard in IsNonResultLine to prevent crashes from potential null upstream inputs. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs [8-11] private static bool IsNonResultLine(string line) { + if (string.IsNullOrWhiteSpace(line)) + return false; + return SdCardFileListParser.IsErrorResponseLine(line); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 2 __ Why: Adding defensive checks in the caller is unnecessary boilerplate, especially since upstream callers typically ensure valid inputs and standard practice is to validate within the utility method itself.	Low
Update

---

Previous suggestions

✅ Suggestions up to commit 52a53ac

Category	Suggestion	Impact
Possible issue	✅ Recognize all canonical error markers Suggestion Impact: The PR removed the local IsErrorResponseLine/MatchesErrorPrefix implementation from SdCardFileListParser and switched SD-card line filtering to use ScpiResponseClassifier.IsErrorResponseLine instead, centralizing error-marker recognition logic rather than adjusting the local delimiter checks as suggested. code diff: - if (IsErrorResponseLine(path)) + // Skip SCPI error responses ("**ERROR: -200, ...", "ERROR: -200, ...") + // and firmware status text ("Error !! ..."). Classification rule lives + // in ScpiResponseClassifier so it stays consistent with + // DaqifiStreamingDevice.IsNonResultLine (closes #190). + if (ScpiResponseClassifier.IsErrorResponseLine(path)) { continue; } @@ -115,50 +110,6 @@ return null; } - - /// <summary> - /// Returns true if the line is a non-result error/status line that - /// SD listing parsers should drop. Matches both SCPI error responses - /// (canonical <c>**ERROR</c> marker, bare <c>ERROR</c> token followed - /// by a SCPI delimiter <c>:</c> / space / tab / end-of-line) and - /// firmware status text (<c>Error !! ...</c> with space, or the - /// no-space <c>Error!!</c> form). A double-<c>!</c> is required when - /// no other delimiter is present so legitimate filenames like - /// <c>error!log.bin</c> aren't dropped — single <c>!</c> alone is - /// ambiguous between error-status and filename. Trims both ends so - /// a bare <c>"ERROR\r"</c> from CRLF line endings still classifies. - /// Plain filenames whose basename starts with <c>error</c> / - /// <c>Errors</c> pass through unmatched (closes #190). - /// </summary> - /// <remarks> - /// Shared with <c>DaqifiStreamingDevice.IsNonResultLine</c> so any - /// future delimiter / trim refinement stays consistent across both - /// SD-response classification paths. - /// </remarks> - internal static bool IsErrorResponseLine(string line) - { - var trimmed = line.Trim(); - return MatchesErrorPrefix(trimmed, "**ERROR") - || MatchesErrorPrefix(trimmed, "ERROR"); - } - - private static bool MatchesErrorPrefix(string trimmed, string prefix) - { - if (trimmed.Length < prefix.Length - || !trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) - return false; - if (trimmed.Length == prefix.Length) - return true; - var next = trimmed[prefix.Length]; - if (next == ':' || next == ' ' || next == '\t') - return true; - // Single '!' is ambiguous (could be a filename like "error!log.bin"). - // Require '!!' so we still catch firmware "Error!!" status text but - // let plain filenames pass through. - return next == '!' - && trimmed.Length > prefix.Length + 1 - && trimmed[prefix.Length + 1] == '!'; - } **Bypass the strict delimiter checks for the ERROR prefix and immediately return true. This avoids false negatives on valid error lines while maintaining the strict checks for bare ERROR strings. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [40-63] internal static bool IsErrorResponseLine(string line) { + if (string.IsNullOrWhiteSpace(line)) + return false; + var trimmed = line.Trim(); - return MatchesErrorPrefix(trimmed, "**ERROR") - || MatchesErrorPrefix(trimmed, "ERROR"); + + if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + return true; + + return MatchesErrorPrefix(trimmed, "ERROR"); } private static bool MatchesErrorPrefix(string trimmed, string prefix) { if (trimmed.Length < prefix.Length || !trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) return false; if (trimmed.Length == prefix.Length) return true; + var next = trimmed[prefix.Length]; if (next == ':' || next == ' ' || next == '\t') return true; + // Single '!' is ambiguous (could be a filename like "error!log.bin"). // Require '!!' so we still catch firmware "Error!!" status text but // let plain filenames pass through. return next == '!' && trimmed.Length > prefix.Length + 1 && trimmed[prefix.Length + 1] == '!'; } Suggestion importance[1-10]: 8 __ Why: This insightfully avoids under-matching actual SCPI errors starting with **ERROR that might use different delimiters, as the strict delimiter rule is only needed for the bare ERROR prefix.	Medium
	Prevent null-input classification crashes Add a guard against null or whitespace-only inputs in IsErrorResponseLine to prevent exceptions when trimming. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [40-45] internal static bool IsErrorResponseLine(string line) { + if (string.IsNullOrWhiteSpace(line)) + return false; + var trimmed = line.Trim(); return MatchesErrorPrefix(trimmed, "**ERROR") || MatchesErrorPrefix(trimmed, "ERROR"); } Suggestion importance[1-10]: 5 __ Why: Adding a null check prevents potential NullReferenceException crashes if line is null, though its necessity depends on upstream caller guarantees.	Low

Suggestions up to commit dfb3e22

Category	Suggestion	Impact
Possible issue	Guard against null input lines Guard against null or empty input lines in the IsErrorResponseLine method to prevent potential NullReferenceExceptions. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [40-45] internal static bool IsErrorResponseLine(string line) { + if (string.IsNullOrEmpty(line)) + return false; + var trimmed = line.Trim(); + if (trimmed.Length == 0) + return false; + return MatchesErrorPrefix(trimmed, "**ERROR") || MatchesErrorPrefix(trimmed, "ERROR"); } Suggestion importance[1-10]: 4 __ Why: Adding a null check provides defensive programming against unexpected null inputs, although it addresses an edge case that is unlikely to occur in practice.	Low
	Skip null entries during parsing Add a null check before calling Trim() on entries in the lines collection to prevent parsing crashes. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [2-16] foreach (var line in lines) { + if (line is null) + continue; + var path = line.Trim(); // Skip SCPI error responses: "**ERROR: -200, ..." or // "ERROR: -200, ...". Bare "ERROR" prefix can't be used // here because filenames like "error_log.csv" / // "Errors_summary.bin" emitted without the Daqifi/ // directory prefix would also match (closes #190 second // location — IsNonResultLine in DaqifiStreamingDevice // had the same bug). Match `ERROR` only when followed // by an SCPI delimiter (':', ' ', '!', '\t') or end of // line so ordinary filenames pass through. if (IsErrorResponseLine(path)) { continue; } Suggestion importance[1-10]: 3 __ Why: Skipping null entries adds safety for potential edge cases, though it is highly unlikely that the device's I/O lines would contain null entries.	Low

✅ Suggestions up to commit 89ed903

Category	Suggestion	Impact
Possible issue	✅ Avoid misclassifying **ERROR-prefixed filenames Suggestion Impact: Updated IsErrorResponseLine to require delimiters (or end of line) for both "**ERROR" and "ERROR" by refactoring into a shared MatchesErrorPrefix helper, preventing misclassification of "**ERROR"-prefixed filenames. code diff: + /// Returns true if the line is a SCPI error response — either the + /// canonical <c>**ERROR</c> marker or a bare <c>ERROR</c> token, in + /// each case followed by a SCPI delimiter (<c>:</c>, space, <c>!</c>, + /// tab) or end of line. Trims both ends so a bare <c>"ERROR\r"</c> + /// from CRLF line endings still classifies. Plain filenames whose + /// basename starts with <c>error</c> / <c>Errors</c> pass through + /// unmatched (closes #190). /// </summary> /// <remarks> /// Shared with <c>DaqifiStreamingDevice.IsNonResultLine</c> so any @@ -132,18 +133,19 @@ internal static bool IsErrorResponseLine(string line) { var trimmed = line.Trim(); - if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + return MatchesErrorPrefix(trimmed, "**ERROR") + || MatchesErrorPrefix(trimmed, "ERROR"); + } + + private static bool MatchesErrorPrefix(string trimmed, string prefix) + { + if (trimmed.Length < prefix.Length + || !trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + return false; + if (trimmed.Length == prefix.Length) return true; - if (trimmed.Length >= 5 - && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) - { - if (trimmed.Length == 5) - return true; - var next = trimmed[5]; - if (next == ':' || next == ' ' || next == '!' || next == '\t') - return true; - } - return false; + var next = trimmed[prefix.Length]; + return next == ':' || next == ' ' || next == '!' || next == '\t'; } **Apply the same delimiter requirement to the ERROR check as is used for ERROR to prevent theoretical false positives. This ensures consistent logic for both error prefixes. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [34-49] internal static bool IsErrorResponseLine(string line) { var trimmed = line.Trim(); - if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) - return true; + + if (trimmed.Length >= 7 + && trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + { + if (trimmed.Length == 7) + return true; + var next = trimmed[7]; + if (next == ':' || next == ' ' || next == '!' || next == '\t') + return true; + } + if (trimmed.Length >= 5 && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) { if (trimmed.Length == 5) return true; var next = trimmed[5]; if (next == ':' || next == ' ' || next == '!' || next == '\t') return true; } + return false; } Suggestion importance[1-10]: 4 __ Why: While filenames containing * are generally invalid on FAT/exFAT filesystems used by SD cards (making the issue practically impossible), applying the same delimiter logic improves code consistency.	Low

✅ Suggestions up to commit 73cb29e

Category	Suggestion	Impact
Possible issue	Centralize error-line classification logic Extract the duplicated error-line classification logic into a shared helper method. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs [12-33] private static bool IsNonResultLine(string line) { - // Trim both ends — bare "ERROR\r" from a CRLF line ending would - // otherwise leave the '\r' as trimmed[5] and fall through the - // delimiter switch below. - var trimmed = line.Trim(); - if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) - return true; - // Must be followed by an SCPI delimiter (':', ' ', '!', '\t') - // or end of line, so plain filenames like "error_log.csv" - // (which TrimStart leaves intact) don't match. - if (trimmed.Length >= 5 - && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) - { - if (trimmed.Length == 5) - return true; - var next = trimmed[5]; - if (next == ':' || next == ' ' || next == '!' || next == '\t') - return true; - } - return false; + return SdCardFileListParser.IsErrorResponseLine(line); } Suggestion importance[1-10]: 7 __ Why: Centralizing duplicated logic into a single helper method is a good practice that improves maintainability and reduces the risk of future inconsistencies.	Medium
General	Recognize comma-delimited error format Add , to the list of acceptable delimiters after ERROR to handle SCPI variants without spaces. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [26-45] private static bool IsErrorResponseLine(string line) { - // Defensive Trim — current callers already pre-trim, but a future - // reuse with raw CRLF input must still classify "ERROR\r" as an - // error line rather than letting the trailing '\r' disqualify it - // (mirrors IsNonResultLine in DaqifiStreamingDevice). var trimmed = line.Trim(); if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) return true; if (trimmed.Length >= 5 && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) { if (trimmed.Length == 5) return true; var next = trimmed[5]; - if (next == ':' || next == ' ' || next == '!' || next == '\t') + if (next == ':' || next == ' ' || next == '!' || next == '\t' || next == ',') return true; } return false; } Suggestion importance[1-10]: 4 __ Why: Adding a comma as a delimiter is a reasonable speculative enhancement that improves robustness for edge cases in SCPI error formatting without risking false positives.	Low

Suggestions

Category	Suggestion	Impact
Possible issue	Handle CRLF in error detection Use Trim() instead of TrimStart() in IsNonResultLine to properly handle trailing carriage returns (\r) in device responses. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs [12-30] private static bool IsNonResultLine(string line) { - var trimmed = line.TrimStart(); + var trimmed = line.Trim(); // trim both ends to remove CR/LF artifacts if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) return true; + // Must be followed by an SCPI delimiter (':', ' ', '!', '\t') - // or end of line, so plain filenames like "error_log.csv" - // (which TrimStart leaves intact) don't match. + // or end of line, so plain filenames like "error_log.csv" don't match. if (trimmed.Length >= 5 && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) { if (trimmed.Length == 5) return true; + var next = trimmed[5]; if (next == ':' || next == ' ' || next == '!' || next == '\t') return true; } + return false; } Suggestion importance[1-10]: 8 __ Why: The suggestion identifies a valid edge case where carriage returns (\r) could cause real error lines to be misclassified, and provides a simple, effective fix using Trim().	Medium
	✅ Normalize whitespace before matching Suggestion Impact: IsErrorResponseLine now trims the input line before checking for "**ERROR" / "ERROR", ensuring "ERROR\r" from CRLF line endings is classified correctly; logic was also refactored into a shared prefix-matching helper. code diff: + /// <summary> + /// Returns true if the line is a SCPI error response — either the + /// canonical <c>**ERROR</c> marker or a bare <c>ERROR</c> token, in + /// each case followed by a SCPI delimiter (<c>:</c>, space, <c>!</c>, + /// tab) or end of line. Trims both ends so a bare <c>"ERROR\r"</c> + /// from CRLF line endings still classifies. Plain filenames whose + /// basename starts with <c>error</c> / <c>Errors</c> pass through + /// unmatched (closes #190). + /// </summary> + /// <remarks> + /// Shared with <c>DaqifiStreamingDevice.IsNonResultLine</c> so any + /// future delimiter / trim refinement stays consistent across both + /// SD-response classification paths. + /// </remarks> + internal static bool IsErrorResponseLine(string line) { - // Defensive Trim — current callers already pre-trim, but a future - // reuse with raw CRLF input must still classify "ERROR\r" as an - // error line rather than letting the trailing '\r' disqualify it - // (mirrors IsNonResultLine in DaqifiStreamingDevice). var trimmed = line.Trim(); - if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + return MatchesErrorPrefix(trimmed, "**ERROR") + || MatchesErrorPrefix(trimmed, "ERROR"); + } + + private static bool MatchesErrorPrefix(string trimmed, string prefix) + { + if (trimmed.Length < prefix.Length + || !trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + return false; + if (trimmed.Length == prefix.Length) return true; - if (trimmed.Length >= 5 - && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) - { - if (trimmed.Length == 5) - return true; - var next = trimmed[5]; - if (next == ':' || next == ' ' || next == '!' || next == '\t') - return true; - } - return false; + var next = trimmed[prefix.Length]; + return next == ':' || next == ' ' || next == '!' || next == '\t'; } Add a Trim() call inside IsErrorResponseLine to defensively handle potential CRLF line endings, ensuring robustness for future usages. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [26-40] private static bool IsErrorResponseLine(string line) { - if (line.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + var trimmed = line.Trim(); // ensures "ERROR\r" is treated as "ERROR" + if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) return true; - if (line.Length >= 5 - && line.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) + + if (trimmed.Length >= 5 + && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) { - if (line.Length == 5) + if (trimmed.Length == 5) return true; - var next = line[5]; + + var next = trimmed[5]; if (next == ':' || next == ' ' || next == '!' || next == '\t') return true; } + return false; } Suggestion importance[1-10]: 4 __ Why: While the input is already trimmed by the current caller, adding Trim() inside the IsErrorResponseLine helper improves its robustness for potential future reuse.	Low

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit b9ccac2

[qodo-code-review]

PR Code Suggestions ✨

Warning

/improve is deprecated. Use /agentic_review instead (removed after 2026-05-31).

Category	Suggestion	Impact
Possible issue	Handle CRLF in error detection Use Trim() instead of TrimStart() in IsNonResultLine to properly handle trailing carriage returns (\r) in device responses. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs [12-30] private static bool IsNonResultLine(string line) { - var trimmed = line.TrimStart(); + var trimmed = line.Trim(); // trim both ends to remove CR/LF artifacts if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) return true; + // Must be followed by an SCPI delimiter (':', ' ', '!', '\t') - // or end of line, so plain filenames like "error_log.csv" - // (which TrimStart leaves intact) don't match. + // or end of line, so plain filenames like "error_log.csv" don't match. if (trimmed.Length >= 5 && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) { if (trimmed.Length == 5) return true; + var next = trimmed[5]; if (next == ':' || next == ' ' || next == '!' || next == '\t') return true; } + return false; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion identifies a valid edge case where carriage returns (\r) could cause real error lines to be misclassified, and provides a simple, effective fix using Trim().	Medium
	✅ Normalize whitespace before matching Suggestion Impact: The commit added `line.Trim()` inside `IsErrorResponseLine` and updated the matching logic to operate on the trimmed string (refactored via `MatchesErrorPrefix`), explicitly handling cases like "ERROR\r". code diff: - private static bool IsErrorResponseLine(string line) + /// <summary> + /// Returns true if the line is a SCPI error response — either the + /// canonical <c>**ERROR</c> marker or a bare <c>ERROR</c> token, in + /// each case followed by a SCPI delimiter (<c>:</c>, space, <c>!</c>, + /// tab) or end of line. Trims both ends so a bare <c>"ERROR\r"</c> + /// from CRLF line endings still classifies. Plain filenames whose + /// basename starts with <c>error</c> / <c>Errors</c> pass through + /// unmatched (closes #190). + /// </summary> + /// <remarks> + /// Shared with <c>DaqifiStreamingDevice.IsNonResultLine</c> so any + /// future delimiter / trim refinement stays consistent across both + /// SD-response classification paths. + /// </remarks> + internal static bool IsErrorResponseLine(string line) { - if (line.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + var trimmed = line.Trim(); + return MatchesErrorPrefix(trimmed, "**ERROR") + || MatchesErrorPrefix(trimmed, "ERROR"); + } + + private static bool MatchesErrorPrefix(string trimmed, string prefix) + { + if (trimmed.Length < prefix.Length + || !trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + return false; + if (trimmed.Length == prefix.Length) return true; - if (line.Length >= 5 - && line.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) - { - if (line.Length == 5) - return true; - var next = line[5]; - if (next == ':' || next == ' ' || next == '!' || next == '\t') - return true; - } - return false; + var next = trimmed[prefix.Length]; + return next == ':' || next == ' ' || next == '!' || next == '\t'; } Add a Trim() call inside IsErrorResponseLine to defensively handle potential CRLF line endings, ensuring robustness for future usages. src/Daqifi.Core/Device/SdCard/SdCardFileListParser.cs [26-40] private static bool IsErrorResponseLine(string line) { - if (line.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) + var trimmed = line.Trim(); // ensures "ERROR\r" is treated as "ERROR" + if (trimmed.StartsWith("**ERROR", StringComparison.OrdinalIgnoreCase)) return true; - if (line.Length >= 5 - && line.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) + + if (trimmed.Length >= 5 + && trimmed.StartsWith("ERROR", StringComparison.OrdinalIgnoreCase)) { - if (line.Length == 5) + if (trimmed.Length == 5) return true; - var next = line[5]; + + var next = trimmed[5]; if (next == ':' || next == ' ' || next == '!' || next == '\t') return true; } + return false; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: While the input is already trimmed by the current caller, adding Trim() inside the IsErrorResponseLine helper improves its robustness for potential future reuse.	Low
More

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 73cb29e

[qodo-code-review]

Persistent suggestions updated to latest commit 73cb29e

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 89ed903

[qodo-code-review]

Persistent suggestions updated to latest commit 89ed903

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 5af7218

[qodo-code-review]

PR Code Suggestions ✨

Warning

/improve is deprecated. Use /agentic_review instead (removed after 2026-05-31).

No code suggestions found for the PR.

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit dfb3e22

[qodo-code-review]

Persistent suggestions updated to latest commit dfb3e22

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 52a53ac

[qodo-code-review]

Persistent suggestions updated to latest commit 52a53ac

[cptkoolbeenz]

/improve

[cptkoolbeenz]

/agentic_review

[qodo-code-review]

Persistent review updated to latest commit 96dd1cb

[qodo-code-review]

Persistent suggestions updated to latest commit 96dd1cb

[tylerkron]

Live device verification

Tested against a real DAQiFi Nq1 (FW 3.4.4) over serial at /dev/cu.usbmodem101, built against this PR's local Daqifi.Core via the daqifi-core-example-app CLI (-p:DaqifiCoreProjectPath=...):

Discovery:

$ --discover-serial
Found: Nq1 (/dev/cu.usbmodem101) SN:9090539562006014104 FW:3.4.4

SD list (the path the fix touches):

$ --serial /dev/cu.usbmodem101 --sd-list
Connected to /dev/cu.usbmodem101 @ 9600 baud
Listing SD card files...
  log_20260502_125152.bin             2026-05-02 12:51:52  [Protobuf]
Total: 1 file(s)

What this proves

• ✅ No regression on real hardware. Full round-trip through InitializeAsync, SYST:STOR:SD:LIST?, the refactored ScpiResponseClassifier → SdCardFileListParser path, and LAN-interface restoration all succeed against firmware 3.4.4. The actual CRLF-framed serial response parses to the expected log_<timestamp>.bin row.

Caveat

The bug-fix half (filenames starting with error* no longer being misclassified) can't be directly observed on the live device: the SD card only contained log_*.bin files, and the CLI has no way to write an arbitrary filename to the SD (only --sd-log-start which always produces log_<timestamp>.bin). That half stays covered by the new unit tests (GetSdCardFilesAsync_FilenamesStartingWithErrorAreNotMisclassified, OnlyErrorPrefixedFilenames_AllSurvive, RealErrorLinesStillSkipped, FilenamesWithSingleBangSurvive).

[tylerkron]

Additional live-device sanity checks

Ran a broader set of checks against the same DAQiFi Nq1 (FW 3.4.4, serial at /dev/cu.usbmodem101) to confirm the refactor commit (4b018de) didn't break adjacent code paths.

Check	Command	Result
Device enumeration	--discover-serial	Nq1 (/dev/cu.usbmodem101) SN:9090539562006014104 FW:3.4.4
SD listing (the fix path)	--serial /dev/cu.usbmodem101 --sd-list	log_20260502_125152.bin listed cleanly
LAN chip info (another SCPI text-response path)	--serial /dev/cu.usbmodem101 --lan-chip-info	ChipId: 1377184, FwVersion: 19.7.7, BuildDate: Mar 30 2022
Live streaming (binary protobuf path)	--serial /dev/cu.usbmodem101 --rate 100 --duration 3 --channels 7 --limit 10 --min-samples 5	10 samples delivered, clean StopStreaming, exit 0

What this covers

Both SCPI text-response paths that route through the new ScpiResponseClassifier — GetSdCardFilesAsync and GetLanChipInfoAsync — round-trip correctly on real hardware. The protobuf streaming path is unrelated to the classifier change but included as a general health check, and is also healthy.

The bug-fix scenario itself (filenames starting with error* no longer being misclassified) remains covered by unit tests rather than live hardware, since there's no CLI-accessible way to write an arbitrarily-named file to the SD card without physically removing it.