Reject duplicate explicit row keys in CSV and JSON inputs

[codeofwxz]

With an explicit --key, repeated values currently overwrite earlier rows silently. In the sample from #31, the command exits successfully and reports a removed column even though the input contains duplicate row keys.

This change rejects duplicate explicit keys in CSV/TSV and JSON inputs. The CLI reports the input filename, key column and value, and the first and repeated data row numbers, then exits with status 1 without printing a diff. Python callers receive DuplicateKeyError, a subclass of ValueError.

Both loaders share the check because both previously indexed rows by key using a dictionary comprehension. Identical rows with an explicit key are rejected too, since that key must uniquely identify a row. Empty field names are supported: --key= or key="" selects the empty-named field. Omitting the key or passing key=None retains content-based deduplication. Callers previously using an empty string to mean no key should omit it or use None.

Row numbers count data records from 1, excluding the CSV/TSV header; they are not physical line numbers for multiline CSV fields. The README documents these behaviors.

This targets duplicate row-key values in #31. The separate change in #45 targets duplicate column headings; this patch does not add heading validation.

Validation on Windows / Python 3.9.13:

• Current full suite: 56 passed (python -m pytest -q).
• The original BUG: Duplicate key causes undefined behaviour, user not warned #31 CLI sample exits 1 with a duplicate-key error and empty stdout, instead of reporting a removed column.
• Coverage includes identical duplicate rows, short rows, multiline CSV fields, empty CSV key values, JSON key values 0 and null, either input in CSV/TSV/JSON CLI comparisons, and unchanged omitted/None-key deduplication.
• Empty-named fields have both unique-key success and duplicate-key failure coverage. Loader tests require DuplicateKeyError and verify its ValueError compatibility.
• git diff --check and incremental patch application checks passed.

Hosted checks for the updated head are tracked separately in this PR; the local results above do not claim a completed multi-version CI matrix.

Fixes #31.

[Copilot]

🔵 Needs a closer look

Explicit empty key names bypass duplicate-key validation, and loader tests do not assert the promised concrete exception type.

Pull request overview

This PR rejects duplicate explicit row keys in CSV/TSV and JSON inputs, adds actionable CLI errors, raises DuplicateKeyError, and documents the behavior.

Changes:

• Adds shared duplicate-key validation for loaders.
• Updates CLI error reporting.
• Adds loader and CLI coverage.
• Documents row numbering and no-key deduplication.

File summaries

File	Reviewed changes and final comments
tests/test_csv_diff.py	Tests loader validation and deduplication. Nit (1 vote): assert DuplicateKeyError specifically for CSV and JSON cases.
tests/test_cli.py	Tests CSV, TSV, and JSON CLI error reporting.
README.md	Documents duplicate-key behavior and row numbering.
csv_diff/cli.py	Formats duplicate-key errors with filenames and row details.
csv_diff/__init__.py	Implements duplicate-key validation. Moderate (1 vote): explicit empty keys bypass validation in both CSV and JSON branches; use an explicit None check.

Review details

Suppressed comments (4)

csv_diff/init.py:40

• This branch is still guarded by if key:, so an explicitly supplied empty key name (for example, a CSV header ,name loaded with key="") skips _validate_unique_keys and falls back to content hashing. That violates the documented behavior for any explicit --key and allows repeated values in the selected empty-name column; test for key is not None instead.

        _validate_unique_keys(rows, key)

csv_diff/init.py:56

• The JSON loader has the same falsy-key gap: key="" is treated as no key, so duplicate values in a valid empty-named JSON property are not rejected. Since the change promises validation whenever an explicit key is provided, use an explicit None check for this branch.

        _validate_unique_keys(raw_list, key)

tests/test_csv_diff.py:132

• The public contract says duplicate explicit keys raise DuplicateKeyError, but this assertion accepts any ValueError; a regression to a generic ValueError would still pass. Assert the concrete exception type here so the CSV loader's new API is covered.

    with pytest.raises(ValueError, match="Duplicate key") as error:

tests/test_csv_diff.py:146

• This JSON test has the same coverage gap: it only verifies the ValueError base class, so it would not catch a regression that stops exposing the promised DuplicateKeyError type. Assert the concrete exception class here as well.

    with pytest.raises(ValueError, match="Duplicate key") as error:

• Files reviewed: 5/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.