Skip to content

fix: preserve numeric text in table HTML metadata#4373

Open
gyx09212214-prog wants to merge 5 commits into
Unstructured-IO:mainfrom
gyx09212214-prog:gyx09212214-prog/preserve-html-table-number-text
Open

fix: preserve numeric text in table HTML metadata#4373
gyx09212214-prog wants to merge 5 commits into
Unstructured-IO:mainfrom
gyx09212214-prog:gyx09212214-prog/preserve-html-table-number-text

Conversation

@gyx09212214-prog

@gyx09212214-prog gyx09212214-prog commented Jun 12, 2026

Copy link
Copy Markdown

Summary

Refs #3871.

This updates table HTML generation so delimited table partitioners preserve numeric-looking cell text in metadata.text_as_html instead of letting pandas serialize numeric columns through DataFrame.to_html().

Changes:

  • read CSV and TSV data as strings with default NA coercion disabled, preserving source lexemes such as 478923, 000001, 000123, 007, 0.000123, and very large integer-like values
  • render CSV/TSV table HTML through a shared htmlify_dataframe() helper backed by htmlify_matrix_of_cell_texts(), avoiding pandas float/scientific-notation formatting
  • preserve dataframe null values as empty table cells instead of serializing them as nan, None, or <NA> text
  • add regression coverage for CSV and TSV metadata.text_as_html, including TSV file-like input and explicit leading-zero identifiers
  • keep the existing partition_html(..., chunking_strategy="by_title") regression guard for numeric table cells, now including explicit leading-zero values

Notes

I still could not reproduce the scientific-notation conversion through current partition_html() alone with a minimal HTML table fixture. I confirmed the current HTML table partition path does not do an HTML -> DataFrame -> to_html() round-trip: TableBlock.iter_elements() builds table data directly from DOM cell text and renders it through htmlify_matrix_of_cell_texts().

The reproducible lossy path is the DataFrame-backed CSV/TSV table serialization path, which matches the pandas to_html() failure mode discussed in #3871.

Testing

python -m pytest -q test_unstructured\common\test_html_table.py test_unstructured\partition\test_csv.py test_unstructured\partition\test_tsv.py
python -m ruff check unstructured\common\html_table.py unstructured\partition\csv.py unstructured\partition\tsv.py test_unstructured\common\test_html_table.py test_unstructured\partition\test_csv.py test_unstructured\partition\test_tsv.py test_unstructured\partition\html\test_partition.py
python -m ruff format --check unstructured\common\html_table.py unstructured\partition\csv.py unstructured\partition\tsv.py test_unstructured\common\test_html_table.py test_unstructured\partition\test_csv.py test_unstructured\partition\test_tsv.py test_unstructured\partition\html\test_partition.py
git diff --check

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Shadow auto-approve: would auto-approve. Adds test coverage for numeric table cell text preservation in HTML partitioning. No source code or business logic changes—only a new test fixture and assertions.

Re-trigger cubic

@gyx09212214-prog gyx09212214-prog changed the title test: guard HTML table numeric cell text fix: preserve numeric text in table HTML metadata Jun 19, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="unstructured/partition/tsv.py">

<violation number="1" location="unstructured/partition/tsv.py:59">
P2: Custom agent: **Enforce Pragmatic Test Coverage**

Change to TSV partition core logic lacks test coverage</violation>
</file>

Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic

Comment thread unstructured/partition/tsv.py Outdated
html_table = HtmlTable.from_html_text(
dataframe.to_html(index=False, header=include_header, na_rep="")
)
html_table = HtmlTable.from_html_text(_dataframe_to_html_text(dataframe, include_header))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Enforce Pragmatic Test Coverage

Change to TSV partition core logic lacks test coverage

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At unstructured/partition/tsv.py, line 59:

<comment>Change to TSV partition core logic lacks test coverage</comment>

<file context>
@@ -41,18 +41,22 @@ def partition_tsv(
-    html_table = HtmlTable.from_html_text(
-        dataframe.to_html(index=False, header=include_header, na_rep="")
-    )
+    html_table = HtmlTable.from_html_text(_dataframe_to_html_text(dataframe, include_header))
 
     metadata = ElementMetadata(
</file context>

Comment thread unstructured/partition/csv.py Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="unstructured/partition/tsv.py">

<violation number="1" location="unstructured/partition/tsv.py:59">
P2: Custom agent: **Enforce Pragmatic Test Coverage**

Change to TSV partition core logic lacks test coverage</violation>
</file>

Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread unstructured/common/html_table.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic

@zeweihan

Copy link
Copy Markdown

Great direction — this is exactly the "carry the source lexeme as a string through the round-trip" fix that #3871 was missing, and reading CSV/TSV with NA coercion disabled so 478923 / 000001 / 0.000123 survive is the right call.

A couple of notes from ingesting a lot of structured-but-regulated documents, in case they're useful:

Leading zeros are a silent reference-breakage case, not just cosmetic. The 000001 example is more dangerous than it looks: account numbers, bond/CUSIP-like identifiers, statute section numbers, and clause IDs routinely carry leading zeros. Once a column is inferred as int64, 000001 becomes 1 with no metadata flag, and every downstream "find Section 0001" / "match account 00012345" retrieval query silently misses. It's worth adding an explicit leading-zero fixture to the regression suite ("000123", "007") so a future dtype-inference refactor can't silently regress it — the current string-everywhere approach fixes it, but it's the exact class of bug that creeps back when someone later re-enables pd.read_csv(..., dtype=...) inference for performance.

The HTML-backed table path is the one I'd most want covered. You noted you couldn't reproduce the lossy conversion through partition_html() alone with a minimal fixture. The dangerous real-world case for legal/financial PDFs isn't a hand-authored HTML table though — it's an HTML table that an OCR/layout model synthesized from a PDF page and handed to the partitioner, which then gets materialized through a DataFrame before text_as_html is rendered. If that path still goes HTML → DataFrame (dtype inference) → to_html(), the coercion can reappear even though the CSV/TSV path is now fixed. Worth confirming whether htmlify_dataframe() / htmlify_matrix_of_cell_texts() is the single funnel for all table-HTML emission (CSV, TSV, and the HTML partition path), or whether the HTML partitioner still has its own DataFrame round-trip. If it's the latter, the same to_html() failure mode is still reachable.

Null-as-empty-cell is correct, but there's a semantic trap worth a comment in the code. Rendering nulls as empty <td></td> is unambiguously better than literal nan / None / <NA> text leaking into the corpus. The one caveat: in regulatory/fee-schedule tables, an intentionally-blank cell usually means "not applicable" or "no penalty" rather than "missing/unknown". An empty string can't distinguish those, but that's acceptable — the important thing is the consumer sees empty (a parseable signal) instead of garbage tokens that look like content. If there's ever appetite, a separate cell_metadata with {was_null: true} would let a downstream consumer treat blanks as data-absent vs. value-absent, but empty-cell-first is the right default.

Thanks for picking this up — the Refs #3871 + string-everywhere approach is the clean fix.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 5 files (changes from recent commits).

Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic

Copy link
Copy Markdown
Author

Thanks for the review and the additional context here.

I’ve incorporated the feedback and tightened the PR around the original data-integrity issue:

  • CSV/TSV inputs are now read with source lexemes preserved as strings, with default NA coercion disabled, so values like 478923, 000001, 000123, 007, 0.000123, and very large integer-like identifiers survive into metadata.text_as_html unchanged.
  • CSV/TSV table HTML generation now goes through a shared htmlify_dataframe() helper backed by htmlify_matrix_of_cell_texts(), avoiding pandas DataFrame.to_html() numeric formatting and scientific notation.
  • Pandas null values are rendered as empty table cells instead of leaking nan, None, or <NA> tokens into the table text, with regression coverage for that behavior.
  • I added explicit leading-zero fixtures across the shared renderer, CSV, TSV filename/file-like paths, and the existing partition_html(..., chunking_strategy="by_title") regression guard.
  • I also confirmed the current partition_html table path does not do an HTML -> DataFrame -> to_html() round-trip; it builds table data directly from DOM cell text and renders through htmlify_matrix_of_cell_texts().

Validation run locally:

python -m pytest -q test_unstructured\common\test_html_table.py test_unstructured\partition\test_csv.py test_unstructured\partition\test_tsv.py
python -m ruff check unstructured\common\html_table.py unstructured\partition\csv.py unstructured\partition\tsv.py test_unstructured\common\test_html_table.py test_unstructured\partition\test_csv.py test_unstructured\partition\test_tsv.py test_unstructured\partition\html\test_partition.py
python -m ruff format --check unstructured\common\html_table.py unstructured\partition\csv.py unstructured\partition\tsv.py test_unstructured\common\test_html_table.py test_unstructured\partition\test_csv.py test_unstructured\partition\test_tsv.py test_unstructured\partition\html\test_partition.py
git diff --check

The remaining Cubic blocker appears to be an outdated unresolved TSV coverage thread from an earlier review; the TSV regression coverage has since been added and the latest review reported no new issues. If this scope looks good to maintainers, I’d appreciate another review and merge when convenient.

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.

2 participants