fix(pdf): preserve positioned word boundaries without discarding tables

[Çağdaş Yürekli (cagdasyurekli)]

Fixes #120.

The positioned-text PDF in the issue currently loses word boundaries and starts as a spurious Markdown table. Detect repeated collapsed ASCII words before form classification, and use pdfplumber's tighter text tolerance only on affected prose pages. Ordinary prose keeps its existing extraction path, and genuine form pages retain Markdown tables even when most other pages have collapsed text.

This is an alternative to #1902, building on its detector/fallback approach and the hardening previously shared in this review. Compared with that PR, the tests exercise the actual page detector, tight extraction is limited to affected pages, and there is no document-wide override that can discard genuine tables. The change is based on current main (eb31b5c).

Validation on Python 3.12:

• GITHUB_ACTIONS=true hatch test --python 3.12 -q --tb=short: 873 passed, 43 skipped (the repository's conditional skips).
• Focused PDF suites: 43 passed, 2 optional skips. Coverage includes ordinary prose, isolated long identifiers, non-ASCII text, empty fallback output, table misclassification, mixed-page ordering, and page cleanup.
• Original issue PDF: 30+ character ASCII runs 522 → 0; CharlesUniversity 1 → 0; Charles University 0 → 2; output no longer begins as a table.
• pre-commit run --all-files and git diff --check: passed; the newly added test file also passed the Black hook.

The detector is deliberately conservative and ASCII-based; this addresses the reported positioned-text case, not arbitrary PDF layout recovery.

Submission containing materials of a third party: Puneet Dixit (puneetdixit200), whose detector/fallback implementation in #1902 is adapted here under the repository's MIT license. Existing license notices are retained.

[XU (kokokoXUY)]

Summary

This PR targets issue #120: PDFs whose text is drawn glyph by glyph with no space
characters come out of the pdfminer path as one long run of letters
(Largelanguagemodels(LLMs)arebecomingacrucialbuildingblock...). The patch adds two
deliberately conservative "space starvation" detectors, computes the pdfplumber words
once per page and reuses them for the table/form extraction, and switches the plain
text source between pdfminer and pdfplumber when the document looks starved.

I reproduced the failure mode with hand-built PDFs and confirmed that the fix works as
advertised on pages that trip both thresholds. I also found three things that should be
handled before this lands: combined with current main it fails four tests (the new
call convention meets a one-argument test helper, and a broad except Exception
swallows the resulting TypeError and silently drops table extraction), the per-page
gate misses sparse pages, and the long-word ratio can reclassify a genuine borderless
table of long cells as starved and drop its table markup.

What the patch does

• _has_space_starved_text(text) (_pdf_converter.py:66-83): at least 500 ASCII
  letters, then whitespace_ratio < 0.03 and at least 5 runs of 30+ letters.
• _has_space_starved_words(words) (:86-99): at least 50 alphabetic words, then
  long_word_count >= 5 and long_word_count / alpha_word_count >= 0.025.
• _select_plain_text_extraction(pdfminer_text, pdfplumber_text) (:102-107): use the
  pdfplumber text when it is non-empty and the pdfminer text looks starved, otherwise
  keep pdfminer.
• convert (:608-631): one
  page.extract_words(keep_blank_chars=True, x_tolerance=3, y_tolerance=3) per page,
  reused for the starvation check and for _extract_form_content_from_words; starved
  pages request the pdfplumber plain text with PDFPLUMBER_TEXT_X_TOLERANCE = 1
  (:68); :638-646 selects the document level source.
• _extract_form_content_from_words(page, words=None) (:170-190) returns None
  early when the page looks starved, so a page that previously produced a bogus
  borderless table now falls through to the plain text path.

What I verified locally

1. Failure reproduction. I wrote a small generator that emits PDF 1.4 content streams
   placing each line as one run of 12 eleven-letter words separated by a configurable
   gap and no space glyphs. With 12pt Courier, a word gap in [1.05, 1.15] pt produces
   the exact symptom: pdfminer merges 50 lines into single 30+ letter runs
   (ws_ratio = 0.0078), extract_words(x_tolerance=3) also merges them (50 words,
   longest 134 chars), while extract_words(x_tolerance=1) splits them correctly (600
   words, longest 12) and extract_text(x_tolerance=1) inserts spaces.
2. Fix works on pages that trip both gates. 50 lines per page: before the patch
   len=6725, collapsed=69, spaced=0; after len=7273, collapsed=0, spaced=69, byte
   for byte identical to extract_text(x_tolerance=1).
3. Control fixtures do not regress. At a 1.3pt gap pdfminer already inserts spaces
   (no bug to fix): both revisions print spaced=69, collapsed=0. At a 1.0pt gap even
   x_tolerance=1 cannot split the runs: both revisions print collapsed=69.
4. Existing fixtures are unaffected. All 7 PDF fixtures present in this PR's tree
   produce identical length and sha256 before and after.
5. Real fixture benefit. On main's packages/markitdown/tests/test_files/pdf_cleanup_mixed.pdf
   the patch restores word boundaries in about 20 lines of prose
   (Largelanguagemodels(LLMs)arebecomingacrucialbuildingblockindevelopingpowerfulagents
   becomes Large language models (LLMs) are becoming a crucial building block in developing powerful agents; 10029 -> 10426 chars) while all 48 table rows stay
   identical, so the fix has a measurable effect on a real repository fixture without
   table loss there.
6. Tests. The four PDF suites touched by this PR (test_pdf_tables.py,
   test_pdf_memory.py, test_pdf_word_boundaries.py, test_pdf_masterformat.py)
   report 43 passed, 2 skipped on the PR head. Running the new tests against unpatched
   sources gives 9 failed, 7 passed, 2 skipped. One of the new tests,
   test_collapsed_prose_is_not_a_borderless_table, passes on unpatched sources (see
   point C below).
7. Formatting. black 23.7.0 (the pinned version in .pre-commit-config.yaml) reports
   "3 files would be left unchanged" for the converter and both test files;
   git diff --check is clean.
8. Merge rehearsal. _pdf_converter.py has not been touched on main since this PR's
   base commit, so the file itself applies cleanly. git merge-tree --write-tree
   against main reports exactly one conflict:
   CONFLICT (modify/delete): packages/markitdown/tests/test_pdf_memory.py deleted in upstream-main and modified in pr2491 (the file was removed when tests were
   consolidated into test_pdf.py by #2579).

Issues

A. Combined with current main, four existing tests fail. I built the merge tree
(4cc9fa1) with this PR's converter plus main's packages/markitdown/tests/test_pdf.py
and ran the suite: 4 failed, 33 passed. The failures are
TestPdfMemoryOptimization::test_page_close_called_on_every_page,
test_plain_text_pdf_falls_back_to_pdfminer,
test_plain_text_pdf_still_closes_all_pages and
test_mixed_pdf_uses_form_extraction_per_page, with
ValueError: ('form', 1) is not in list (test_pdf.py:1264, 1284) and
assert [] == [('form', 1), ..., ('form', 3)] (:1272, :1291). Restoring main's
converter makes the same file pass (37 passed), so the failures come from the
interaction, not from main.

Root cause: main's pdf_activity fixture (test_pdf.py:1209-1246) replaces
_extract_form_content_from_words with a one-argument spy def extract_form(page).
The patch calls it as _extract_form_content_from_words(page, words=words), the spy
raises TypeError: extract_form() got an unexpected keyword argument 'words', and
convert's broad except Exception fallback turns that programming error into a
silent degradation: the spy records no call for the page and the page loses its table
extraction entirely. I measured it with the real helper versus the one-argument spy on
the same page: 7858 characters versus 4148 characters of markdown.

Suggested: rebase onto main, update the helper to accept the new argument (for example
def extract_form(page, words=None)) and port the test_pdf_memory.py additions into
test_pdf.py, since the old file no longer exists. It is also worth narrowing the
except Exception fallback around the per-page extraction, or at least logging it, so
that a signature mismatch cannot quietly discard table content.

B. The per-page gate needs 50 alphabetic words, so sparse pages keep the bug. The
document level detector fires on my 40-line fixture (letters 5340, ws_ratio = 0.0079)
and _select_plain_text_extraction does switch to pdfplumber, but the page level gate
does not (alpha_word_count = 40 < 50), so the pdfplumber text is still requested with
the default tolerance and stays collapsed: before the patch collapsed=55, spaced=0,
after the patch collapsed=55, spaced=0. The same happens with a two-column fixture of
2 x 25 words per page (collapsed=70 before and after). Only the 50-word page is
repaired. Suggested: make the decision relative rather than absolute (on the 40-word
fixture, len(extract_words(x_tolerance=1)) / len(extract_words(x_tolerance=3)) is
480 / 40 = 12, which is a much stronger signal than a count of 40), or run the
existing _has_space_starved_text check on the pdfplumber page text, or simply
re-extract affected paragraphs with the tight tolerance once the document level check
fires.

C. Long-cell borderless tables can be reclassified as starved. A page with 3 columns x
20 rows of 30-character cells at 60pt column spacing (60 alphabetic words, 100% long)
previously produced a table with 21 rows and now returns None, so the table markup is
lost. With 48 words the same geometry still produces a table (17 rows), so this is a
hard switch at the 50-word threshold rather than a gradual one. The PR's own test for
this case uses 200pt column spacing, which makes line_width = 490 > 612 * 0.55 and
len(combined_text) = 101 > 60 true, so is_paragraph is already true and unpatched
sources return None as well: that test passes without the patch (I confirmed this in
the red run) and therefore does not exercise the new branch. Suggested: build the test
from the narrow-spacing geometry where unpatched sources do return a table, and
consider excluding pages whose long runs are separated by consistent wide gaps, that
is, pages that already look like a multi-column layout.

D. Minor notes. PDFPLUMBER_TEXT_X_TOLERANCE is only used for extract_text; the
extract_words call inside convert still uses x_tolerance=3, so words collected
for a starved page that keeps a table remain merged. The document level swap also
changes whitespace on pages where nothing needed fixing (the 40-line fixture goes from
5381 to 5379 characters with identical visible text), so the change is not strictly
additive. Finally, new PDF tests should go into test_pdf.py per
packages/markitdown/tests/README.md.

How this was produced

Automated review by an AI agent (Claude via DeepSeek Harness) with the project's own
dependencies in an isolated venv. Verification used synthetic hand-written PDFs plus
the repository fixtures, pytest on the PR head and on unpatched sources, black 23.7.0,
git merge-tree and a detached worktree to rehearse the merge against main, and a
monkeypatch spy to attribute the four main-side test failures. No files were pushed to
this branch; all checks ran on local checkouts.

Verdict

The diagnosis and the approach look right to me and the fix demonstrably repairs
collapsed prose on a real repository fixture without harming tables there, so I would
like to see this land. As it stands the branch cannot be merged: it conflicts with the
consolidated test file and fails four tests on top of main because of the new keyword
call convention. I would merge a rebased version that fixes the helper signature and
the test placement, and I would ask for a follow-up on the sparse-page gap and on the
long-cell table reclassification. Happy to re-review once the branch is rebased.

[Çağdaş Yürekli (cagdasyurekli)]

Rebased onto current main (4cc9fa17) and addressed the review in this branch:

1. Removed the obsolete PDF test files and moved all new regressions into the consolidated test_pdf.py. Updated the real-library activity spy for the keyword arguments; the four cleanup/fallback tests now pass.
2. Rebuilt the collapsed-prose regression using 60pt column spacing. Against unchanged main it returns a Markdown table and fails; with this patch it returns None and passes. A second real, handwritten PDF regression exercises forty positioned-text lines: unchanged main keeps merged words, while the patch recovers all 480 words.
3. Removed the fifty-word page cutoff, retaining the repeated-long-run and ratio checks so sparse affected pages also receive tight extraction.
4. Applied PDFPLUMBER_TEXT_X_TOLERANCE to both word and text extraction on affected pages. Preserve the original starvation decision after re-extraction, since the tightened words no longer contain collapsed runs.
5. Excluded layouts with repeated wide aligned gutters from starvation detection. Added two-column controls and long-cell table tests at 48, 51, and 60 cells; each keeps its Markdown table rather than switching at fifty words.
6. Added a warning on the existing parser fallback so an unexpected signature/parser failure does not silently discard table extraction.

Validation: Black 23.7.0, git diff --check, the full core pytest suite, and frozen failing-before/passing-after regressions. 1066 passed, 57 skipped using the repository's existing CI/optional/credential skip conditions (GITHUB_ACTIONS=true).

On the repository's real pdf_cleanup_mixed.pdf, all 48 table rows are byte-identical while the collapsed prose is repaired (10029 → 10426 characters).

The detector remains a conservative ASCII/layout heuristic; this does not claim general PDF layout recovery. XU (@kokokoXUY), the branch is rebased and these review points are addressed. Could you re-review?