Skip to content

csvbase: report the correct line number in extraction errors - #215

Open
MugenLab wants to merge 1 commit into
beancount:masterfrom
MugenLab:fix-csv-error-lineno
Open

MugenLab wants to merge 1 commit into
beancount:masterfrom
MugenLab:fix-csv-error-lineno

Conversation

@MugenLab

Copy link
Copy Markdown

Importer.extract() computes lineno as the 1-based physical line number of
the data row:

offset = int(self.header) + bool(self.names) + 1
for lineno, row in enumerate(self.read(filepath), offset):

That is also the number metadata() records through
data.new_metadata(filepath, lineno), so it is what ends up in the metadata of
the extracted transactions. The error path, however, formatted lineno + 1:

raise RuntimeError(
    f"Error processing {filepath} line {lineno + 1} with values {row!r}"
) from ex

so the same row was reported at two different line numbers depending on whether
it parsed or not, and the error pointed one line past the offending row.

Reproduced with beancount 3.2.3 on a three-line file (header plus two data rows,
the last one invalid):

physical line 1: date,narration,amount
physical line 2: 2025-01-25,Good,-1.00
physical line 3: 2025-01-26,Bad,invalid

Error processing /tmp/t.csv line 4 with values ('2025-01-26', 'Bad', 'invalid')
metadata for the row on physical line 2 -> {'filename': '/tmp/t.csv', 'lineno': 2}

The existing test_report_exception encoded the old behaviour (its docfile has
two lines and the failing row is the second one, but the expected message said
line 3), so its expectation is updated to line 2.

Two regression tests are added:

  • test_report_exception_matches_metadata_lineno extracts a file successfully,
    remembers the lineno metadata of the last row, then makes that same row
    invalid and requires the reported line number to be the one from the
    metadata. This pins the two code paths together rather than hard-coding a
    constant, so they cannot drift apart again.
  • test_report_exception_line_numbering covers a file that has both skipped
    header lines (header = 1) and a names row, and asserts against the line
    index actually read back from the file.

Without the one-character fix all three tests fail; with it the full suite
(pytest beangulp examples, 174 tests) passes, and ruff check beangulp/ examples/ is clean.


Disclosure: this change was written by an AI agent. A human is accountable for
the submission and will respond to review comments.

Importer.extract() computes `lineno` as the 1-based physical line
number of the data row: the enumeration starts at
`int(header) + bool(names) + 1`.  That is the number `metadata()`
records via `data.new_metadata(filepath, lineno)`, and it is the
number that ends up in the metadata of the extracted transactions.

The error path, however, formatted `lineno + 1`, so the same row was
reported at two different line numbers depending on whether it parsed
or not, and the error pointed one line past the offending row.

For a file with a header line and two data rows, an invalid value on
the third line was reported as "line 4".

Add a regression test that ties the reported line number to the one
recorded in the metadata, so the two cannot drift apart again, and one
that exercises a file with both skipped header lines and a names row.

This change was authored by an AI agent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant