fix(csv): a zone on offset-less CSV text is attached to the wall-clock time (#231)

[paddymul]

Stacked on #189: the base is feat/adr-007-009-cache-redesign, not main. Fixes #231.

Terms: a zoned column is one whose schema type is a timestamp with a time zone, such as timestamp('America/New_York'). Offset-less text is a timestamp with no UTC offset at its end (2024-01-02 09:30:00); text with an offset ends in +00:00, -0500, Z and so on. A source's snapshot is the parquet file the import writes (ADR-011 D1), so since ADR-011 the CSV reader's output is baked into the source version.

Problem

_polars_dtype (io.py:128-129) passes a zoned type to polars' CSV reader as pl.Datetime(unit, time_zone=tz). The reader parses offset-less text as UTC and converts it into the zone, so 09:30 under New York was stored as 04:30-05:00 in January and 05:30-04:00 in July. ADR-005 D9(a) says the zone is attached to the wall-clock time. Nothing failed: every row was off by four or five hours.

Fix

A zoned column is read as text and parsed after the scan with str.to_datetime(time_unit, time_zone=tz, ambiguous="raise") (io._parse_zoned). That call does what D9(a) says: text without an offset gets the zone attached, and text with an offset is an instant and is converted into the zone. The declared unit is kept, so timestamp('America/New_York', 9) keeps every ns digit (#145). A naive timestamp and every other type still go through polars' reader as before.

Decisions the issue left open:

• A wall-clock time the zone skips or repeats at a DST change raises. 2024-03-10 02:30 never happens in New York, and 2024-11-03 01:30 happens twice. polars raises on both (ambiguous="raise", and a skipped time raises by default), so no value is shifted to get past it.
• A column that mixes offset and offset-less text raises. A column is read one way. I considered reading each value by its own text (what Postgres and DuckDB do for timestamptz input), but raising is the conservative choice and costs no extra code: polars' streaming engine infers one format per column from its first non-null value in file order and parses every later batch with it (the strptime-infer node, perf: Streaming strptime with format=None pola-rs/polars#27056), so the other kind fails wherever it first appears. test_a_zoned_column_mixing_offset_and_offsetless_text_raises[offset-late] puts the offset 50,000 rows in, past the first batch, and I checked 36 layouts (4 splits × 3 batch sizes × 1, 2 and 8 threads) by hand: all raise.
• The error names the column, the row and the value. polars' own messages name an internal column (_POLARS_TMP_0) and suggest options the importer cannot pass (strict=False, non_existent='null'). After a failed read, _explain_zoned_failure reads the zoned columns again as text and reports the first mixed row, the first skipped or repeated time, or the first value that is not a timestamp. For example: column 'ts' is a timestamp with the zone 'America/New_York', where text with no UTC offset is a wall-clock time, and '2024-03-10 02:30:00' (row 3) does not exist in America/New_York: the clocks skip it at a daylight-saving change, so no instant matches it. tallyman does not pick an instant for it. Correct the value, or write it with its UTC offset. Anything it cannot explain falls through to the existing "the schema does not parse" error with its suggested schema.

A polars 1.40.1 bug this has to work around

The streaming strptime-infer node in polars 1.40.1 (installed) writes a batch as nulls, with no error, when the first non-null value it sees matches no datetime format. A zoned column whose first value was hello imported as [null, null], dropping the good value after it too; polars' reader, which the code used before, raised. Upstream fixed it in pola-rs/polars#28986 (a polars_ensure! when strict). The value the node infers from is the column's first non-null value in file order, so _unreadable_first_zoned_value parses that one value eagerly before the read and raises when it is not a timestamp. test_text_that_is_not_a_timestamp_raises_in_a_zoned_column[first-value] fails without that check.

Deviation from the suggested fix

The issue suggests reading the column naive and then calling .dt.replace_time_zone(tz). polars' reader under a naive Datetime override accepts offset text, converts it to UTC and drops the offset, so after a naive read nothing says which rows had one: 2024-07-02T09:30:00-04:00 comes back as naive 13:30, and attaching New York would store 13:30-04:00. Reading the text and parsing it with time_zone=tz keeps "text with an offset still converts" true.

Found while here

• ADR-005 D9(b) does not hold at 1f8cb02. polars 1.40.1's reader keeps ns digits, and the snapshot writer (parquet format 2.6) keeps them too. I checked timestamp(9), timestamp('UTC', 9) and (with this fix) timestamp('America/New_York', 9) end to end through update_and_depend: all keep .123456789. The red run shows the same for the zoned column before the fix: the digits survive and only the instant is shifted. ADR-005 D9 gets an implementation note saying (a) and (b) were both wrong about the installed polars.
• A naive timestamp over offset text converts it to UTC and drops the offset, silently. This is the same class of bug for naive columns, and this PR does not change it.
• Existing sources imported with a zoned schema hold shifted snapshots under an unchanged entry hash, since the hash covers the bytes and the reader options, not the reader's code. Re-import them (single-user project, no migration).

Execution sites: none added in #118's sense (.execute() / to_pyarrow_batches()). New polars .collect() calls: one head(1) per zoned column before the read, and the queries in _explain_zoned_failure, which run only after a read has failed.

Tests

All in tests/test_tallyman_read_csv_contract.py. Each imports a CSV through update_and_depend and reads the snapshot back with pyarrow.

Failing before the fix:

• test_a_zone_attaches_to_offsetless_text: 09:30 is 09:30-05:00 in January and 09:30-04:00 in July, and an empty field stays null.
• test_a_zoned_nanosecond_column_keeps_its_digits: timestamp('America/New_York', 9) keeps the type and every digit of 09:30:00.123456789.
• test_a_wall_clock_time_the_zone_skips_raises and test_a_wall_clock_time_the_zone_repeats_raises.
• test_a_zoned_column_mixing_offset_and_offsetless_text_raises, three layouts: offset second, offset first, and offset at row 50,001.

Guards, which pass before and after and are in the fix commit:

• test_offset_text_converts_into_the_zone: +00:00 in January becomes 04:30-05:00, Z in July becomes 05:30-04:00.
• test_utc_reads_offsetless_text_as_utc, with a leading null.
• test_a_naive_timestamp_stays_naive.
• test_a_zoned_column_renamed_by_position_beside_inferred_columns: the parse is keyed on the header name and runs before a positional rename, in inference mode.
• test_text_that_is_not_a_timestamp_raises_in_a_zoned_column, as the first value and as a later one. With the eager first-value check removed the first case fails (the import succeeds with nulls); with InvalidOperationError dropped from the ladder's except, the later case and the three mixed cases fail with a raw polars error.

How it was built

1. 9243c9b adds the seven failing tests. CI run 36017271086: ruff passed, and the fast suite had 7 failed and 954 passed. The seven failures were exactly the new tests: the two value tests on the shifted instant, the two DST tests on DID NOT RAISE, and the three mixed cases on the message (the old error blamed the schema, named the clone's hashed file name and suggested reading the column as a string).
2. 71a93c2 adds the fix, the guards and the ADR note. CI run 36019495448: ruff, the fast suite (967 passed, and vitest 15 passed) and the integration suite (7 passed) all pass.

Checked locally before each push: the seven tests fail on the unfixed io.py for the reasons above and the guards pass on it; with the fix the fast suite had 961 passed (test_fouc::test_unknown_api_path_404s answers 503 until the SPA is built in the worktree, and passed after pnpm build), the integration suite 7 passed, and uvx ruff check passed. The repo has no .pre-commit-config.yaml; the new code is ruff format clean, and I did not reformat the pre-existing lines in io.py and the test file that ruff format would change. A 2,000,000-row import took about the same time with a naive, a UTC and a New York column (1.0 s, 0.6 s, 0.6 s, one run each).

Not in this PR

• A naive timestamp over offset text (above).
• The suggested schema in the general parse error maps any Datetime to timestamp, so for a zoned column it suggests dropping the zone.
• The general parse error names the file by its clone's hashed name (e7e909b3….csv), not the path the user imported.

🤖 Generated with Claude Code

[paddymul]

Review summary

/code-review of 71a93c2 found nine items. Two were regressions: CSVs that imported before this PR and failed after it. Both are fixed here, test first. The other seven don't block the merge and are filed as #257.

A zoned column is one whose schema type carries a time zone, such as timestamp('America/New_York'). This PR reads it as text and parses it after the scan.

Fixed in this PR

Finding	Fix
A CSV with a column named row and any zoned column failed with a raw polars DuplicateError: _zoned_text added a row index named row before selecting its column.	e061241: the index is added after the select, so no header can collide with it.
With missing_utf8_is_empty_string=True, an empty cell in a zoned column became "" and the import failed with "'' is not a timestamp". polars' reader used to store null.	e061241: _zoned_text_value turns an empty zoned cell into null before the parse and the checks.

Tests in 4d38c3e: test_a_zoned_column_beside_a_column_named_row, test_a_zoned_failure_beside_a_column_named_row_is_explained, and test_an_empty_zoned_cell_is_null_when_empty_strings_are_kept[empty-first, empty-later]. Red run 36042688678: exactly those 4 failed and 980 passed. Also checked by hand after the fix: a zoned column that is itself named row, a zoned column named text beside a row column, and a quoted "" with default options (stored as null).

Deferred to #257

• A naive timestamp over offset text still converts to UTC and drops the offset. This PR already lists it as out of scope.
• A zoned failure repeats the inference ladder three times when any column is inferred, and the error explanation scans the file up to four times per zoned column.
• _UTC_OFFSET accepts a space before the offset and polars does not, so the mixed-column error can recommend a format that then fails to parse.
• "row N" counts data records, not file lines, so with skip_rows or multi-line quoted fields it points at the wrong line.
• test_a_zoned_column_renamed_by_position_beside_inferred_columns uses UTC, where the a tz-aware timestamp in an import schema reads offset-less text as UTC and converts it, contrary to ADR-005 D9(a) #231 bug and the fix store the same instant.
• polars has no version bound, so nothing ties the 1.40.1 workaround or the one-format-per-column behaviour to a version.

Not a bug: the review also checked that text which is not a timestamp still raises past the first value with 1 and 8 threads, that mixed text formats in one column (date only, no seconds, T or a space, with or without fractional seconds) parse, and that time zones given as fixed offsets and ms/ns units behave as before.

🤖 Generated with Claude Code