Skip to content

fix: write an empty XLSX cell as empty in the markdown table format - #12776

Open
L4XB wants to merge 1 commit into
deepset-ai:mainfrom
L4XB:fix/xlsx-markdown-missing-value
Open

L4XB wants to merge 1 commit into
deepset-ai:mainfrom
L4XB:fix/xlsx-markdown-missing-value

Conversation

@L4XB

@L4XB L4XB commented Sep 16, 2026

Copy link
Copy Markdown

Why:

XLSXToDocument renders the same empty cell two different ways depending on table_format:

XLSXToDocument(table_format="csv")       # ,A,B\n1,col_c,col_d\n2,True,
XLSXToDocument(table_format="markdown")  # |  2 | True  | nan   |

The CSV format writes an empty field. The markdown format writes the string nan, which reads as content — a retriever indexes it, and an LLM asked what is in that cell answers nan.

This is not hypothetical in this repo: the fixture sheet is literally called "Table Missing Value", and test_run_markdown pinned the nan:

assert (
    documents[1].content
    == "|    | A     | B     |\n|---:|:------|:------|\n|  1 | col_c | col_d |\n|  2 | True  | nan   |"
)

The documented escape hatch does not work either. table_format_kwargs is passed to to_markdown, and tabulate does have a missingval option, but it only applies to None: a NaN reaches tabulate's formatter as a number and is written out as nan, so table_format_kwargs={"missingval": "N/A"} changes nothing today.

What:

  • Replace the empty cells with None before rendering (value.astype(object).where(value.notna(), None)), so tabulate sees a missing value rather than a float.
  • Default missingval to "" in the resolved kwargs, so an empty cell reads as empty — the same as the CSV format. table_format_kwargs still wins, so {"missingval": "N/A"} now does what the docstring says.

Only the markdown branch is touched; the CSV branch, the hyperlink handling and the metadata are unchanged.

|    | A     | B     |          |    | A     | B     |
|---:|:------|:------|   -->    |---:|:------|:------|
|  1 | col_c | col_d |          |  1 | col_c | col_d |
|  2 | True  | nan   |          |  2 | True  |       |

Tests:

  • test_run_markdown — the pinned expectation moves from nan to an empty cell. That assertion is the regression: on main the new expectation fails with
    - |  2 | True  | nan   |
    + |  2 | True  |       |
    
  • test_run_markdown_missing_value (new) — table_format_kwargs={"missingval": "N/A"} reaches tabulate. It fails on main too, because the option had no effect there.
test/components/converters/test_xlsx_to_document.py
  main         2 failed, 15 passed
  this branch  17 passed

The other 15 pass unchanged on both sides, including every CSV-format test.

ruff check and ruff format --check are clean on both files. A release note is included.

`XLSXToDocument` wrote the string `nan` into an empty cell when
`table_format="markdown"`, while `table_format="csv"` wrote the same cell as
an empty field. The repo's own fixture sheet named "Table Missing Value"
carried both renderings, and the test pinned the inconsistency.

tabulate's `missingval` only covers `None`: a NaN reaches its formatter as a
number and is written out as `nan`. Replace the empty cells with `None` before
rendering, and default `missingval` to the empty string, so an empty cell reads
as empty and `table_format_kwargs={"missingval": "N/A"}` works as documented.
@L4XB
L4XB requested a review from a team as a code owner September 16, 2026 12:30
@L4XB
L4XB requested review from davidsbatista and removed request for a team September 16, 2026 12:30
@vercel

vercel Bot commented Sep 16, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @L4XB, thanks for your interest in contributing to Haystack! 🙏

⚠️ You currently have 3 open pull requests in this repository (#12769, #12768 and this one). Our review capacity is limited, so please hold off opening more PRs until we've had a chance to review your first 2 open PRs. This helps us give each contribution the attention it deserves. Thank you!

This is an automated message to help us keep the review queue healthy.

@L4XB

L4XB commented Sep 16, 2026

Copy link
Copy Markdown
Author

recheck

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant