fix: preserve numeric text in table HTML metadata#4373
fix: preserve numeric text in table HTML metadata#4373gyx09212214-prog wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
| 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)) |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
|
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 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 The HTML-backed table path is the one I'd most want covered. You noted you couldn't reproduce the lossy conversion through Null-as-empty-cell is correct, but there's a semantic trap worth a comment in the code. Rendering nulls as empty Thanks for picking this up — the |
There was a problem hiding this comment.
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
|
Thanks for the review and the additional context here. I’ve incorporated the feedback and tightened the PR around the original data-integrity issue:
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 --checkThe 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. |
Summary
Refs #3871.
This updates table HTML generation so delimited table partitioners preserve numeric-looking cell text in
metadata.text_as_htmlinstead of letting pandas serialize numeric columns throughDataFrame.to_html().Changes:
478923,000001,000123,007,0.000123, and very large integer-like valueshtmlify_dataframe()helper backed byhtmlify_matrix_of_cell_texts(), avoiding pandas float/scientific-notation formattingnan,None, or<NA>textmetadata.text_as_html, including TSV file-like input and explicit leading-zero identifierspartition_html(..., chunking_strategy="by_title")regression guard for numeric table cells, now including explicit leading-zero valuesNotes
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 anHTML -> DataFrame -> to_html()round-trip:TableBlock.iter_elements()builds table data directly from DOM cell text and renders it throughhtmlify_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