Skip to content

fix: keep literal N/A and NULL cells in CSVDocumentSplitter - #13310

Open
vidigoat wants to merge 1 commit into
deepset-ai:mainfrom
vidigoat:fix/csv-splitter-literal-na
Open

vidigoat wants to merge 1 commit into
deepset-ai:mainfrom
vidigoat:fix/csv-splitter-literal-na

Conversation

@vidigoat

Copy link
Copy Markdown
Contributor

Related Issues

Proposed Changes:

CSVDocumentSplitter parses each document with pandas.read_csv, which by default treats literal strings such as N/A, NA, NULL, and nan as missing values (dtype=object does not prevent this). As a result:

  • those cells come back empty in the split output (A1,N/A,keep becomes A1,,keep);
  • in threshold mode, a row made only of such values is treated as an empty row, used as a split boundary, and dropped from the output.

This PR adds keep_default_na=False and na_values=[""] to the splitter's default read_csv_kwargs, placed before the caller's kwargs so an explicit override still wins. na_values=[""] is what keeps genuinely empty fields parsed as NaN, so empty-row/column detection and splitting behave exactly as before. This is the combination @julian-risch pointed to when closing #12806 ("disabling pandas' default NA parsing without re-adding empty-string detection breaks the handling of empty rows and columns"), and the same one #12772 uses for the cleaner.

Also updated the read_csv_kwargs docstring (listing the new defaults) and added one sentence to docs-website/docs/pipeline-components/preprocessors/csvdocumentsplitter.mdx saying what counts as an empty row or column.

How did you test it?

  • Reproduced on main (c6c46c67b) with the issue's inputs: row-wise output was ['Code,Status,Note\n', 'A1,,keep\n', 'A2,,keep\n', 'A3,,keep\n']; threshold mode on Code,Status\nA1,ok\nN/A,N/A\nA3,ok\n returned ['Code,Status\nA1,ok\n', 'A3,ok\n'] (the N/A,N/A row was lost). With this change the cells are preserved, the input comes back as one table, and a genuinely empty , row still splits.
  • Added two regression tests to test/components/preprocessors/test_csv_document_splitter.py:
    • test_literal_na_strings_are_kept_as_cell_content (row-wise mode)
    • test_row_of_literal_na_strings_is_not_a_split_boundary (threshold mode; the same input also contains a real empty row, which must still split, and the test checks row_idx_start)
      Both fail with the fix reverted (2 failed, 29 passed) and pass with it.
  • hatch run test:unit test/components/preprocessors/test_csv_document_splitter.py: 31 passed
  • hatch run test:unit test/components/preprocessors/ test/components/converters/test_csv_to_document.py: 488 passed
  • hatch run fmt-check: passed; hatch run test:types: no issues in 562 source files; reno lint .: passed; pre-commit run --files <changed files>: all hooks passed
  • Checked that read_csv_kwargs={"keep_default_na": True} restores the previous behavior (mentioned in the release note).

Notes for the reviewer

  • @michaelkillgta offered to take CSVDocumentCleaner deletes a row that says N/A, and both CSV components empty those cells #12784 on Sep 25. I couldn't find a PR from them since then, so I'm opening this splitter-only fix. If they already have work in progress, I'm happy to close this in favour of theirs.
  • Behavior change: anyone who relied on N/A/NULL cells being blanked by the splitter will now see them as content. The release note says how to get the old behavior back.
  • If a caller sets na_values in read_csv_kwargs without also setting keep_default_na, only their values (no longer pandas' default list) are treated as missing. That follows from the documented "extend or override" semantics of read_csv_kwargs.
  • The versioned docs are not changed, since this changes behavior for the next release rather than correcting released docs.

This PR was developed with Claude Code (AI) on my behalf. The regression tests fail on main and pass with this change, and the checks listed above pass locally.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

pandas.read_csv treats strings such as N/A, NA, NULL and nan as missing
values by default. CSVDocumentSplitter therefore emptied those cells in
its output, and a row made only of such values was treated as an empty
row and used as a split boundary.

Read CSV content with keep_default_na=False and na_values=[""] so that
only genuinely empty fields count as missing. Callers can still override
both options through read_csv_kwargs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vidigoat
vidigoat requested a review from a team as a code owner October 18, 2026 07:44
@vidigoat
vidigoat requested review from julian-risch and removed request for a team October 18, 2026 07:44
@vercel

vercel Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

@vidigoat is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added topic:tests type:documentation Improvements on the docs labels Oct 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/preprocessors
  csv_document_splitter.py
  haystack/core/pipeline
  pipeline.py
Project Total  

This report was generated by python-coverage-comment-action

@0xamlab

0xamlab commented Oct 18, 2026

Copy link
Copy Markdown

Verified locally: the full test_csv_document_splitter.py suite passes (31/31) and the two new regression tests fail on main without this change. On base, pandas' default NA parser turns literal N/A, NULL, and NA cells into empty strings, so the row-wise split drops the values and a row of N/A values is mistaken for a blank boundary. Pinning keep_default_na=False and na_values=[""] at haystack/components/preprocessors/csv_document_splitter.py:109-116 fixes that while still treating only truly empty fields as missing. Clean, focused fix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants