Skip to content

Commit f81b1d5

Browse files
paddymulclaude
andcommitted
test(csv-import): red — reserved-column exclusion threaded through header readers
The totality check excludes the reserved 'original_row_order' from the header a schema must cover (9182e0f #3), but three other surfaces read the header and do not apply that exclusion consistently, breaking exactly in the path #3 enables: - The #143 suggested-schema hint whole-file-infers every column, so it emits an un-pasteable cell for original_row_order; pasting it back raises 'positional schema has 3 columns but the CSV header has 2'. - Schema-error diagnostics print the reserved-stripped header, so a 3-column file is reported as 'has 2 [a, b]' — an off-by-one the user cannot reconcile. - A canonical original_row_order that is not the trailing column silently rebinds positional cells onto the wrong data column (no error, wrong output). Also covers the reserved-output-name collision: a spec that renames a DATA column ONTO original_row_order must raise, not leak a polars DuplicateError. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9182e0f commit f81b1d5

1 file changed

Lines changed: 110 additions & 0 deletions

File tree

tests/test_tallyman_read_csv.py

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,3 +405,113 @@ def test_existing_row_order_column_noninteger_raises(project, monkeypatch):
405405
err = res["error"]
406406
assert "original_row_order" in err
407407
assert "reserved" in err.lower() or "integer" in err.lower() or "0.." in err
408+
409+
410+
@pytest.mark.parametrize(
411+
"schema",
412+
[
413+
(("original_row_order", "int64"), ("&rest", "infer")), # positional rename target
414+
(("a", "int64"), ("original_row_order", "string")), # positional, second column
415+
],
416+
)
417+
def test_schema_output_name_original_row_order_raises(project, monkeypatch, schema):
418+
"""A schema that maps a DATA column onto the reserved 'original_row_order' output
419+
name collides with tallyman's auto-added row index. Pre-fix this leaked a raw
420+
polars DuplicateError ('column original_row_order is duplicate'); ingest must
421+
instead reject the reserved output name with a clear ValueError."""
422+
monkeypatch.setenv("TALLYMAN_PROJECT", project)
423+
from tallyman_xorq.io import tallyman_read_csv
424+
425+
p = data_dir(project) / "renameoro.csv"
426+
p.write_text("a,b\nx,10\ny,20\n")
427+
with pytest.raises(ValueError, match="reserved"):
428+
tallyman_read_csv(str(p), schema=schema)
429+
430+
431+
# --------------------------------------------------------------------------- #
432+
# Reserved-column exclusion, threaded consistently (review follow-ups).
433+
#
434+
# The totality check excludes tallyman's reserved 'original_row_order' column so
435+
# a complete DATA-column schema is not spuriously "not total". Three surfaces
436+
# read the header and must apply that same exclusion consistently, or the
437+
# recovery / diagnostic / positional-binding contracts break in exactly the path
438+
# the exclusion newly enables:
439+
# 1. the #143 suggested-schema recovery hint must stay paste-ready,
440+
# 2. schema-error diagnostics must not hide the reserved column, and
441+
# 3. positional binding must not silently rebind onto the wrong data column.
442+
# --------------------------------------------------------------------------- #
443+
def test_suggested_schema_recovery_is_pasteable_with_reserved_column(project, monkeypatch):
444+
"""#143 recovery contract, reserved-column path: when an explicit schema fails
445+
to parse a CSV that carries a canonical 'original_row_order' column, the
446+
suggested schema in the error must be paste-ready. The whole-file suggestion
447+
must NOT emit a cell for the reserved column (which the caller cannot spec) —
448+
pre-fix it did, so pasting the suggestion back raised 'positional schema has 3
449+
columns but the CSV header has 2'."""
450+
import ast
451+
452+
monkeypatch.setenv("TALLYMAN_PROJECT", project)
453+
from tallyman_xorq.io import tallyman_read_csv
454+
455+
p = data_dir(project) / "suggest_oro.csv"
456+
# Canonical 0..N-1 original_row_order (trailing, as tallyman exports it); the
457+
# amount column holds a float that an int64 pin cannot parse, so the
458+
# explicit-mode suggestion path fires.
459+
p.write_text("id,amount,original_row_order\n1,12.5,0\n2,3.0,1\n")
460+
with pytest.raises(ValueError) as exc:
461+
tallyman_read_csv(str(p), schema=(("id", "int64"), ("amount", "int64")))
462+
msg = str(exc.value)
463+
assert "Suggested schema" in msg, msg
464+
assert "original_row_order" not in msg, f"suggestion leaked the reserved column: {msg}"
465+
466+
# The suggestion must paste back and parse (pre-fix it raised on the column count).
467+
suggested = ast.literal_eval(msg.rsplit("schema=", 1)[-1].strip())
468+
expr = tallyman_read_csv(str(p), schema=suggested)
469+
out = {k: str(v) for k, v in expr.schema().items()}
470+
assert out.get("amount") == "float64", out
471+
assert "original_row_order" in out # the reserved column is still reused
472+
473+
474+
@pytest.mark.parametrize(
475+
"schema",
476+
[
477+
(("a", "int64"), ("b", "int64"), ("c", "int64")), # over-long positional spec
478+
{"zzz": "int64", "&rest": "infer"}, # by-name miss
479+
],
480+
)
481+
def test_schema_error_diagnostic_names_reserved_column(project, monkeypatch, schema):
482+
"""A schema error against a CSV that carries 'original_row_order' must not hide
483+
that column. Pre-fix the diagnostic printed the reserved-stripped header, so a
484+
3-column file (a,b,original_row_order) was reported as 'has 2 [a, b]' — an
485+
off-by-one that the user, staring at a 3-column file, cannot reconcile. The
486+
reserved column must appear in the message."""
487+
monkeypatch.setenv("TALLYMAN_PROJECT", project)
488+
from tallyman_xorq.io import tallyman_read_csv
489+
490+
p = data_dir(project) / "diag_oro.csv"
491+
p.write_text("a,b,original_row_order\n1,2,0\n3,4,1\n")
492+
with pytest.raises(ValueError) as exc:
493+
tallyman_read_csv(str(p), schema=schema)
494+
assert "original_row_order" in str(exc.value), (
495+
f"diagnostic hides the reserved column: {exc.value}"
496+
)
497+
498+
499+
def test_positional_schema_rejects_nontrailing_reserved_column(project, monkeypatch):
500+
"""Positional cells bind by physical column position. A valid canonical
501+
'original_row_order' column that is NOT the last column shifts that mapping —
502+
excluding it from the middle silently rebinds later cells onto the wrong data
503+
column (renaming/dropping a column with no error). Ingest must reject a
504+
positional schema in this layout rather than silently corrupt the output."""
505+
monkeypatch.setenv("TALLYMAN_PROJECT", project)
506+
from tallyman_xorq.io import tallyman_read_csv
507+
508+
p = data_dir(project) / "oro_middle.csv"
509+
# original_row_order is canonical 0..N-1 (so _validate_existing_row_order
510+
# passes) but sits in the MIDDLE, not trailing.
511+
p.write_text("sku,original_row_order,qty\nA,0,10\nB,1,20\n")
512+
with pytest.raises(ValueError) as exc:
513+
tallyman_read_csv(str(p), schema=(("sku", "string"), ("row", "int64")))
514+
msg = str(exc.value).lower()
515+
assert "original_row_order" in msg and ("position" in msg or "last" in msg or "by-name" in msg), (
516+
f"expected a positional/last-column guard message; got: {exc.value}"
517+
)

0 commit comments

Comments
 (0)