[tylerkron]

Summary

SdCardCsvFileParser.FindDataStartIndex accessed line[2] whenever a line started with "ch", but StartsWith("ch") only guarantees length >= 2. A bare "ch" line (exactly 2 chars) threw IndexOutOfRangeException, which — because FindDataStartIndex is called synchronously inside ParseAsync — propagated out of ParseAsync/ParseFileAsync and aborted the whole parse.

This violated the parser's own robustness contract: every other line-parsing method (TryParseCsvDataRow, TryParseJsonLine) wraps parsing so malformed input is skipped gracefully. FindDataStartIndex was the single place that could crash on malformed/truncated SD log content.

Fix

Add a line.Length > 2 guard before the line[2] access. Behavior is unchanged for every line of length >= 3 (the only case where line[2] was previously valid). A too-short "ch" line now falls through and is treated as a data row, which TryParseCsvDataRow then skips as malformed — consistent with how all other bad lines are handled.

Tests

• ParseAsync_BareChLine_DoesNotThrowAndParsesRemainingRows — a bare "ch" line between valid rows: valid rows still parse, no throw.
• ParseAsync_OnlyBareChLine_ReturnsNoSamplesWithoutThrowing — a file whose only data line is "ch": zero samples, no throw.

Both fail with IndexOutOfRangeException before the fix (verified by reverting the source change) and pass after.

Validation

• Core builds clean net9 + net10 (0 warnings).
• Full suite green: 1750 passed net9 + net10 (2 skipped each, +2 new tests), 23 MCP, 0 failed.
• No bench: pure client-side CSV-file parser change with no wire/hardware behavior; the pathological line can't be produced non-destructively on the device, and unit tests fully cover it.

Closes #365.

Not merging — opened for your review.

🤖 Generated with Claude Code

[tylerkron]

/agentic_review

[qodo-code-review]

PR Summary by Qodo

Guard SdCard CSV data-start detection against bare "ch" line

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Prevent IndexOutOfRangeException on truncated/malformed "ch" lines during CSV parsing.
• Preserve existing header detection for valid header lines.
• Add regression tests to ensure parsing skips bad lines and continues.

Diagram

graph TD
  A["SdCardCsvFileParser.ParseAsync"] --> B["FindDataStartIndex"] --> C["Header detection (Length guard)"]
  A --> D["TryParseCsvDataRow"]
  E["SdCardCsvFileParserTests"] --> A

Loading

High-Level Assessment

The chosen fix (length guard before line[2]) is the minimal, behavior-preserving hardening for malformed input and aligns with the parser’s existing “skip bad lines” contract. Broader alternatives (e.g., refactoring header detection into a dedicated tokenizer/regex or making FindDataStartIndex fully exception-safe) add complexity without clear additional value for the reported crash.

Files changed (2) +52 / -2

Bug fix (1) +7 / -2

SdCardCsvFileParser.csPrevent out-of-range access when detecting "ch" headers +7/-2

Prevent out-of-range access when detecting "ch" headers

• Adds a 'line.Length > 2' guard before reading 'line[2]' in 'FindDataStartIndex' when a line starts with "ch". This prevents 'IndexOutOfRangeException' on a bare "ch" line and allows the parser to continue, letting malformed lines be skipped by the existing row-parsing logic.

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs

Tests (1) +45 / -0

SdCardCsvFileParserTests.csAdd regression tests for bare "ch" line handling (#365) +45/-0

Add regression tests for bare "ch" line handling (#365)

• Adds two tests asserting that a bare "ch" line does not throw and that parsing continues for subsequent valid rows, and that a file containing only "ch" yields zero samples without throwing. These tests verify the parser’s robustness behavior for truncated/malformed SD log content.

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

[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. Bare 'ch' breaks header scan ✓ Resolved 🐞 Bug ≡ Correctness

Description

ParseHeader stops on any line that starts with "ch" (including a bare "ch"), but FindDataStartIndex
now treats a bare "ch" as the first data row; if a malformed bare "ch" appears before the real
column header, parsing will begin too early and the real header can be skipped as a malformed data
row. This yields an empty/incorrect CsvColumnLayout/DeviceConfig (e.g., HasDigitalPair=false,
channel counts 0) and can cause digital columns to be dropped or misclassified as analog values for
the remainder of the parse.

Code

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[R244-246]

            if (line.Contains("_ts,", StringComparison.OrdinalIgnoreCase) ||
-                (line.StartsWith("ch", StringComparison.OrdinalIgnoreCase) && !char.IsDigit(line[2])) ||
+                (line.StartsWith("ch", StringComparison.OrdinalIgnoreCase) && line.Length > 2 && !char.IsDigit(line[2])) ||
                line.StartsWith("ain", StringComparison.OrdinalIgnoreCase))

Relevance

⭐⭐⭐ High

Team accepts SD-card CSV robustness/correctness fixes; multiple parser bug fixes merged (e.g., PR
#152).

PR-#152
PR-#132
PR-#225

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code paths disagree on whether a bare "ch" is a header or data, and both ParseHeader and
FindDataStartIndex stop scanning once they hit it (ParseHeader via break, FindDataStartIndex via
return i). With an incorrect layout (HasDigitalPair=false) produced from the malformed header,
TryParseCsvDataRow will never parse digital data and will treat all pairs as analog instead.

src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[176-223]
src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[225-255]
src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[349-403]

Agent prompt

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

## Issue description
`FindDataStartIndex` was fixed to avoid `line[2]` on a bare "ch" by requiring `line.Length > 2`, which makes a bare "ch" fall through and be treated as the first data row.

However, `ParseHeader` still treats any `StartsWith("ch")` line as the column header and `break`s immediately. If a malformed bare "ch" appears before the real column header, `ParseHeader` will produce an empty/incorrect layout/config, and `ParseCsvLines` will start at the bare "ch" line; the later real header line will be fed to `TryParseCsvDataRow` and skipped, leaving the parser to interpret subsequent rows with the incorrect layout.

## Issue Context
This is an edge-case malformed-input scenario, but after the crash fix it becomes a silent mis-parse (wrong layout/config) rather than a hard failure.

## Fix Focus Areas
- src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[135-223]
- src/Daqifi.Core/Device/SdCard/SdCardCsvFileParser.cs[225-255]

## Suggested fix
1. Make `ParseHeader` use the same header-detection predicate as `FindDataStartIndex` (e.g., require `_ts,` OR `StartsWith("ch") && line.Length > 2 && !char.IsDigit(line[2])` OR `StartsWith("ain")`).
2. Optionally, teach `FindDataStartIndex` to *skip* a bare "ch" line (case-insensitive) instead of returning it as `dataStartIndex`, so it can still find the actual header/data start later.
3. Add a regression test where the first non-comment line is "ch", followed by a valid header and valid data, asserting that:
  - samples parse correctly
  - DeviceConfig channel counts/layout reflect the real header
  - digital columns (if present) are not dropped.

ⓘ 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 4d92522

[tylerkron]

Ready for review: Qodo re-review is clean (0 bugs / 0 rule violations, updated to the latest commit 4d92522, the one review thread resolved) and CI build is green. Not merging — leaving for your code-owner review.