test(gsheet-sync): #204 pin CSV<->grid round-trip contract (11 edge-case tests) - #507
Merged
Merged
Conversation
…case tests Secondaire of ai-01 dispatch bwpx5q (gate-safe #204 deep-queue). Pins the home-grown CsvToGrid/GridToCsv converters - the bridge between a Sheets download and local CSV, and between CSV and a Sheets upload - with edge-case contract tests. The critical invariant is round-trip symmetry on the multiline/quoted/ragged data that real Argumentum CSVs contain. Two non-obvious behaviours are now pinned: 1. CsvToGrid has HasHeaderRecord=true but the manual while(Read()) loop returns the header as the first grid row (no ReadHeader() skip) - deliberate, since Sheets expects the header in the uploaded grid. A GetRecords<T>() "cleanup" would silently drop row 0 and break every upload. 2. The read loop is bounded by csv.Parser.Count (dynamic per-record), so ragged rows survive without MissingFieldException and round-trip unaltered. CsvToGrid/GridToCsv are already pure (no extraction needed); this is additive test coverage on the fragile CSV round-trip contract. Test-only, output-neutral. - 11 new tests, 0 prod change - Full suite: 329 passed / 0 failed / 5 skipped (was 318 + 11) Contributes to #204. Refs #193 (GSheetSync), #216. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
|
[NanoClaw] LGTM — test-only, 1 file (+197) 12 contract tests pinning CSV↔grid round-trip symmetry for GSheetService. Covers the right edge cases: RFC 4180 embedded newlines, escaped quotes, ragged rows, null cells, trailing newlines. Class docstring documents two non-obvious fragile behaviours (header row preserved deliberately, dynamic field count bound). Clean test-only extraction. No concerns. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Secondaire of ai-01 dispatch
bwpx5q(gate-safe #204 deep-queue). Pins the home-grownCsvToGrid/GridToCsvconverters — the bridge between a Google Sheet download and a local CSV, and between a local CSV and a Sheets upload — with 11 edge-case contract tests. The invariant that matters most is round-trip symmetry (grid → csv → grid identity) on the multiline / quoted / ragged data that real Argumentum CSVs actually contain.This is the 2nd pure target the dispatch offered ("
CsvToGridedge"). The converters are already pure (staticCsvToGrid, instanceGridToCsv— both plain string→grid / grid→string transforms), so there is no inline logic to extract: this PR is additive test coverage on the fragile CSV round-trip contract, matching the spirit of #204 (expand coverage on fragile contracts).Two non-obvious behaviours now pinned
Reading the code, two things are easy to "clean up" into a regression. Both are documented in the test file's XML doc and pinned by tests:
CsvToGridis configured withHasHeaderRecord = true, but the manualwhile (csv.Read())loop returns the header as the first grid row (there is no separateReadHeader()skip). This is deliberate: Sheets expects the header row inside the uploaded grid. A "cleanup" that switched toGetRecords<T>()or added an explicit header skip would silently drop row 0 and break every upload. (Pinned byRoundTrip_*identity + the existingCsvToGrid_PreservesHeaderRow.)The read loop is bounded by
csv.Parser.Count, which is the current record's field count (dynamic, per-row), not the header's. So ragged rows (rows with fewer/more fields than the header) are preserved without throwingMissingFieldException, and round-trip without padding or truncation. (Pinned byRoundTrip_PreservesRaggedRows.)Tests —
CsvGridConversionContractTests(11)Parse direction (RFC 4180):
CsvToGrid_ParsesQuotedFieldWithEmbeddedNewline— a newline inside a quoted field is part of that field, not a record separator (highest-risk CSV edge case for Argumentum descriptions/examples).CsvToGrid_ParsesEscapedDoubleQuotes—""→".Round-trip symmetry (the sync invariant):
RoundTrip_PreservesQuotedFieldWithEmbeddedNewline— multiline cell survives grid→csv→grid.RoundTrip_PreservesEscapedDoubleQuotesRoundTrip_PreservesEmptyAndCommaFields— empty cells + comma-forcing cells.RoundTrip_PreservesRaggedRows— rows of differing lengths.RoundTrip_SingleColumnCsv_NoCommas— degenerate single-column CSV.Degenerate inputs (no NRE, no phantom rows):
CsvToGrid_EmptyString_ReturnsEmptyGridGridToCsv_EmptyOrNullGrid_ReturnsEmptyStringGridToCsv_NullCells_BecomeEmptyStrings— pins thecell?.ToString() ?? ""guard (null → empty, not literal"null").CsvToGrid_TrailingNewline_DoesNotCreatePhantomRow— the terminator newline must not add an empty row.Verification
RowsetNb/rscount, no workflow/rules.Scope notes
SyncSafetyChecker(the other dispatch candidate) already has 11 tests covering its thresholds/boundaries/zero-rows/custom-config — well-covered, lower value, so not touched.Contributes to #204. Refs #193 (GSheetSync), #216.
🤖 Worker po-2024 — secondaire of dispatch
bwpx5q.