fix(pdf): allow configuring rotated text direction

[Yang Tiankai (RRiiiccckkk)]

Issue for this PR

Closes #2560

Type of change

• Bug fix
• New feature
• Documentation update

What does this PR do?

While converting the issue's agstat.pdf, I found that rotated headings came out as noitalupoP and firahK. The pdf_char_dir_rotated option lets callers select btt or ttb for readable rotated text on plain and table pages. Invalid values raise a conversion error; omitting the option or passing None preserves existing behavior.

Real PDF fixtures cover both directions, mixed page order, table contents, empty pages, and invalid options. The README documents the option. This addresses the issue's configurable-direction request; adjacent numeric cells and multi-line header layout still follow the existing extraction rules.

How did you verify your code works?

On macOS arm64 with Python 3.12.9:

• From packages/markitdown, uvx hatch test tests/test_pdf.py -q — 54 passed.
• From packages/markitdown, uvx hatch test — 1113 passed, 11 skipped.
• From the repository root, uv run --no-project --python 3.12 --with ./packages/markitdown --with ./packages/markitdown-ocr --with pytest pytest packages/markitdown-ocr/tests -q — 108 passed, using the repository's CI dependency constraints.
• From packages/markitdown, uvx hatch run hatch-test.py3.12:python -m pytest ../markitdown-mcp/tests -q — 21 passed, with the local MCP package installed in the test environment.
• From the repository root, uvx pre-commit run --all-files — passed.
• Converted the public agstat.pdf through MarkItDown.convert_stream: pdf_char_dir_rotated="btt" produced Projected Population and Kharif without the reversed words. Adjacent numeric cells remain grouped.

Screenshots / recordings

Not applicable; this change affects PDF text conversion and has no user interface.

Checklist

• I ran the tests locally.
• I kept unrelated changes out of this PR.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Yang Tiankai (RRiiiccckkk)]

@microsoft-github-policy-service agree

[XU (kokokoXUY)]

Review: fix(pdf): allow configuring rotated text direction

Thanks for the patch - exposing pdfplumber's char_dir_rotated is the right lever
for bottom-to-top text, and on the document you added it to the fixture it does
work. I reproduced the fix locally and it is real. I did find three things that I
think should be addressed before this lands, plus a couple of smaller notes. All
of them are about when the option is reachable, not about the idea itself.

What the patch does

• packages/markitdown/src/markitdown/converters/_pdf_converter.py
  • _extract_form_content_from_words(page, *, char_dir_rotated=None) (:120) adds
    char_dir_rotated to the extract_words kwargs when it is not None (:131-141).
  • _extract_tables_from_words(page, *, char_dir_rotated=None) (:407) gets the
    same keyword.
  • convert reads kwargs.get("pdf_char_dir_rotated") (:557), threads it into
    extract_text_kwargs (:558-560) so plain pages call
    page.extract_text(**extract_text_kwargs) (:587), and passes it to
    _extract_form_content_from_words(page, char_dir_rotated=char_dir_rotated)
    (:577-579) - unconditionally, including when the value is None.
• README.md (:113-121) documents the new option with a "btt" example.
• A new one-page fixture packages/markitdown/tests/test_files/rotated_table.pdf
  (ReportLab, 63 chars, 43 upright / 20 rotated) plus one test in
  test_pdf_tables.py (test_rotated_text_direction_can_be_configured, :1200).

What I verified locally

1. Red side. On the base commit the keyword is ignored: the fixture output is
   byte-identical with and without pdf_char_dir_rotated="btt" (both keep the
   reversed run noitalupoP detcejorP, 160 chars, no pdfminer fallback).
2. Green side. On the head commit the same call returns
   Projected Population first, table intact, len=160, no pdfminer fallback.
   At the pdfplumber level extract_words(..., char_dir_rotated="btt") changes
   the last word from noitalupoP detcejorP to Projected Population, and
   extract_text(char_dir_rotated="btt") from noitalupoP/detcejorP to
   Projected/Population.
3. Tests. packages/markitdown/tests/test_pdf_tables.py on the head commit:
   23 passed. The same file against the base source: 1 failed, 22 passed, the
   failure being exactly the new test
   (AssertionError: assert 'Projected' in 'noitalupoP detcejorP\n| Column A ...').
4. No default-behaviour change. Across all fixtures of the merged tree
   (including the new pdf_cleanup_* ones) the output is identical with and
   without btt; only rotated_table.pdf differs.
5. Formatting. black 23.7.0 --check on both changed Python files: unchanged;
   git diff --check clean.
6. Integration. I merged the PR into current main locally (see below) and
   re-ran the probe there: same results as the head commit.

Blocking

1. The option is a silent no-op for PDFs that have no form/table page.
convert still ends with:

if form_page_count == 0:
    pdf_bytes.seek(0)
    markdown = pdfminer.high_level.extract_text(pdf_bytes)
else:
    markdown = "\n\n".join(markdown_chunks).strip()

So when no page is classified as a form page, the option-aware pdfplumber text
built on :587 is computed and then thrown away. I proved this by replacing
pdfminer.high_level.extract_text with a sentinel and converting a generated
one-page PDF whose whole body is bottom-to-top text (no table structure): with
the option the pdfplumber text is Projected\nPopulation\nKharif\nRice... (in
the correct order, unlike the reversed pdfplumber text without the option), yet
the final output was the sentinel - i.e. the pdfminer result - both with and
without pdf_char_dir_rotated="btt". The pdfminer result for that document is a
letter-per-line column (e\nc\ni\nR...), so the user gets worse text than
pdfplumber already had available. Your README example reads as a general
rotation fix (MarkItDown().convert("rotated.pdf", pdf_char_dir_rotated="btt")),
but for rotated prose, rotated headers or rotated footers in an otherwise
plain PDF the parameter changes nothing. Suggestion: when the option is given,
prefer the pdfplumber text at the document level (or make the pdfminer fallback
conditional on the option not being set), so the documented behaviour actually
holds for all rotated PDFs.

2. Invalid direction values silently degrade the whole document.
"ltr", "rtl" and "foo" all make pdfplumber raise
ValueError: line_dir_rotated=... is incompatible with char_dir_rotated=... /
must be one of ['ttb', 'btt', 'ltr', 'rtl'], which the broad
except Exception: on :601-604 swallows. On the fixture the document then falls
from the table markdown (160 chars, pipes present) to plain pdfminer text
(102 chars, table structure gone) with no warning at all. A one-line check
against {"ttb", "btt", "ltr", "rtl"} (or letting the ValueError surface) would
turn a silent whole-document downgrade into a clear error.

3. Current main breaks: 4 tests fail with this converter, and the PR
conflicts with the PDF test consolidation.

• Merge: git merge pr2562 into current main (4cc9fa1) gives
  CONFLICT (content): Merge conflict in packages/markitdown/tests/test_pdf.py
  • the consolidation upstream (#2579) is detected as a rename of
    test_pdf_tables.py, so the PR's test edits collide with one 356-line region
    (conflict markers span lines 1046-1402). The PR needs a rebase; per
    packages/markitdown/tests/README.md the new test now belongs in
    test_pdf.py, which already has the same
    class TestPdfTableStructureConsistency (:912) with a markitdown fixture
    (:916), so the test body can move almost unchanged.
• Behaviour on main: with the PR converter applied, tests/test_pdf.py reports
  4 failed, 33 passed (baseline on main: 37 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 (:1264, :1284) and
  AssertionError: assert [] == [('plain', 1), ...] (:1273, :1292).
  Cause: the pdf_activity fixture (:1210) monkeypatches
  _extract_form_content_from_words with a single-argument spy
  (def extract_form(page), :1233, installed at :1244), while this PR now calls
  the helper with the keyword char_dir_rotated=... even when the value is None
  (:577-579). That raises TypeError: unexpected keyword argument, which the
  except Exception: on :601-604 hides, so the affected page quietly falls back
  to pdfminer text and the expected activity events never appear. Two changes
  would fix it: only pass the keyword when the option is actually set, and avoid
  swallowing programming errors in a broad except Exception.
  (For what it is worth, PR #2491 hit the same fixture in the same way, so this
  looks like a pattern worth settling once in the converter.)

Suggestions

• _extract_tables_from_words has no callers anywhere in the repository (head
  and main): the new keyword-only parameter on it can never be exercised.
  Either drop it or wire the helper up.
• README: the new paragraph sits at the end of the "Optional dependencies" list,
  right before ### Plugins (:113-121). It documents a conversion option, not a
  dependency, and it does not say which values are accepted
  (ttb/btt/ltr/rtl are pdfplumber's vocabulary) nor that the option only
  reaches documents whose pages go through the pdfplumber path. "" currently
  behaves like "not set" although it is passed through as a value.
• The new test only checks strings. It would be stronger (and cheaper to keep
  green) if it also asserted that the table rows are still present with the
  option set, and - following tests/README.md - it could be a vector in
  _test_vectors.py plus a case in test_pdf.py instead of a new test added to
  a file that main has since deleted.
• Nothing here changes behaviour when the option is absent, which is the
  important half of the contract, and I verified that on every fixture I could
  reach.

How this was produced

I inspected the PR head 5f88506fc218c973a961c05d177f03fd214fd105 and its base
b8f79c57ebc0044be41323d89b2a45d3fda8460e, checked out both trees plus a merge
rehearsal against main 4cc9fa17653d695d64fb9eee5b33d4de55ff84e8, and ran
pdfplumber 0.11.10 / pdfminer.six 20260107 (what the lockfile range
pdfplumber>=0.11.9, pdfminer.six>=20251230 resolves to here). The generated
rotated-prose PDF is a hand-written one-page file with 0 1 -1 0 x y Tm text
operators; the sentinel run replaces pdfminer.high_level.extract_text with a
constant string to show which extractor produced the final markdown. Commands
were run on Windows with the repository's venv; the checks used disposable
checkouts, and the temporary merge worktree was removed afterwards.

Verdict

The direction of the change is right and the fix does what the description says
for pages that go through the pdfplumber extraction path. I would not merge it as
is, because on current main the branch conflicts and the converter breaks four
existing tests, and because the option is currently unreachable for PDFs without
a form page and can silently downgrade a document when the value is invalid.
Fixing those (plus moving the test into test_pdf.py) would make this a clean
merge for me.