fix(msword): keep the sign of a negative drawing position by chrikrah · Pull Request #4292 · docling-project/docling

chrikrah · 2026-09-18T07:08:23Z

_get_paragraph_position reads a floating drawing's offset from y, top, positionY and two others. It then strips every character that is not a digit or a period, at docling/backend/msword_backend.py:1958:

clean_value = re.sub(r"[^0-9.]", "", value)

That removes a leading minus. A drawing anchored above or left of its reference point carries a legal negative offset, and it comes back positive:

y="-45.5"  ->  45.5
y="45.5"   ->  45.5
y="45.5pt" ->  45.5
y="-0.5"   ->  0.5

After:

y="-45.5"  ->  -45.5
y="45.5"   ->  45.5
y="45.5pt" ->  45.5
y="-0.5"   ->  -0.5

Why it matters

The value feeds the top-to-bottom sort of floating textbox paragraphs, at :2031, :2045 and :2058. A drawing 45.5 above the anchor sorts as though it sat 45.5 below it. Its text then lands in the wrong place.

Tests

$ python -m pytest tests/test_backend_msword.py -q
67 passed

$ python -m ruff check docling/backend/msword_backend.py tests/test_backend_msword.py
All checks passed!
$ python -m ruff format --check docling/backend/msword_backend.py tests/test_backend_msword.py
2 files already formatted

Five new cases. Four cover y, top, positionY and y-position. The fifth pins that a unit suffix such as 45.5pt still parses.

Revert msword_backend.py alone, keep them, and the file gives 4 failed, 63 passed.

No groundtruth file changed. No committed DOCX fixture carries a negative offset, so nothing in the suite reached this line.

Scope

The regex still strips units. It now keeps a minus anywhere in the string. For these attributes a minus only ever appears at the front, and -45.5pt parses to -45.5.

Duplicate search

Nine open pull requests touch docling/backend/msword_backend.py: #4282, #4188, #4129, #4124, #3891, #3791, #3707, #3641 and #3579. Not one mentions _get_paragraph_position or clean_value.

gh pr list --state all --search "msword position negative" returns nothing.

Checklist:

Documentation has been updated, if necessary.
No document covers how a floating drawing's position is parsed.
Examples have been added, if necessary.
The change corrects existing output rather than adding a capability.
Tests have been added, if necessary.

github-actions · 2026-09-18T07:08:36Z

❌ DCO Check Failed

Hi @chrikrah, your pull request has failed the Developer Certificate of Origin (DCO) check.

This repository supports remediation commits, so you can fix this without rewriting history — but you must follow the required message format.

🛠 Quick Fix: Add a remediation commit

Run this command:

git commit --allow-empty -s -m "DCO Remediation Commit for Christopher Krah <48338417+chrikrah@users.noreply.github.com>

I, Christopher Krah <48338417+chrikrah@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 885d5e3ce2348535c30524eeee6d00b293051484"
git push

🔧 Advanced: Sign off each commit directly

For the latest commit:

git commit --amend --signoff
git push --force-with-lease

For multiple commits:

git rebase --signoff origin/main
git push --force-with-lease

More info: DCO check report

mergify · 2026-09-18T07:09:20Z

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:$.+$)?(!)?:

codecov · 2026-09-18T07:35:49Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

dolfim-ibm

lgtm

dolfim-ibm · 2026-09-21T09:12:20Z

@chrikrah small observation: [^0-9.\-] keeps a minus anywhere in the string, so something like 1e-5 collapses to 1-5 and disappears into the except ValueError. re.search(r"-?\d+(?:\.\d+)?", value) would express the intent more precisely.

dolfim-ibm is right about `[^0-9.\-]` on docling-project#4292. The class keeps a minus wherever it sits, so an exponent collapses into something float() rejects and the except swallows it. Measured on the branch head: '1e-5' -> clean_value='1-5' -> ValueError -> offset dropped '1.5e-3in' -> clean_value='1.5-3' -> ValueError -> offset dropped The drawing then falls back to its XML source line. Before this branch it was not better, only wrong in the other direction: `[^0-9.]` read '1e-5' as 15.0. So the value is matched now instead of sieved, and the try/except goes with it. The pattern can only match a valid float literal, so float() cannot raise there, and "no number in the attribute" is the `if match` branch rather than a caught exception. One deviation from the suggested `-?\d+(?:\.\d+)?`: it needs a digit before the period, so on '.5' it matches the '5' and returns 5.0, ten times too large, and +5.0 for '-.5'. Both old versions read that form correctly, and a decimal without its leading zero is ordinary in a length attribute, so the pattern is `-?\d*\.?\d+`. Two test cases pin it. value [^0-9.] (pre-PR) [^0-9.\-] (head) -?\d*\.?\d+ '-45.5' 45.5 -45.5 -45.5 '45.5pt' 45.5 45.5 45.5 '.5' 0.5 0.5 0.5 '-.5' 0.5 -0.5 -0.5 '2e-5' 25.0 dropped 2.0 'auto' dropped dropped dropped The five position tests now go through one helper instead of building the element each time. Tests: 72 passed, 1 skipped in tests/test_backend_msword.py, the skip being the one that needs LibreOffice. Reverting msword_backend.py alone to the branch head gives 3 failed, 69 passed, 1 skipped; reverting it to cea4b45, the state before this branch, gives 8 failed, 64 passed, 1 skipped. ruff check and ruff format --check are clean on both files. Signed-off-by: Christopher Krah <48338417+chrikrah@users.noreply.github.com>

chrikrah · 2026-09-26T09:37:07Z

Confirmed, and taken in 874501d. [^0-9.\-] keeps a minus anywhere, so the exponent collapses and the offset is dropped:

'1e-5'      -> clean_value='1-5'    -> ValueError, swallowed
'1.5e-3in'  -> clean_value='1.5-3'  -> ValueError, swallowed

Pre-PR [^0-9.] read '1e-5' as 15.0, wrong the other way.

One deviation from your literal pattern, with the measurement behind it. -?\d+(?:\.\d+)? requires a digit before the period, so it regresses a leading-dot decimal:

value    [^0-9.] (pre-PR)   [^0-9.\-] (head)   -?\d+(?:\.\d+)?   -?\d*\.?\d+
'.5'     0.5                0.5                5.0               0.5
'-.5'    0.5                -0.5               5.0               -0.5
'2e-5'   25.0               dropped            2.0               2.0
'auto'   dropped            dropped            dropped           dropped

5.0 for '-.5' is ten times too large and loses the sign, which is the defect this branch exists to fix. So I used re.search(r"-?\d*\.?\d+", value), which keeps every case the old pattern got right and fixes the exponent. Patching your exact literal in makes the two leading-dot cases fail with assert 5.0 == -0.5. Say the word if you prefer the wording and I will switch it.

re.search returns None for a value with no digits, such as y="auto", and a test pins that. The try/except (ValueError, TypeError) is gone, since the pattern can only match a valid float literal.

72 passed, 1 skipped

The skip needs LibreOffice. Across all four msword files, 81 passed, 1 skipped. Revert the production file and 3 fail; revert to pre-PR cea4b45 and 8 fail. ruff check and ruff format --check clean.

dolfim-ibm · 2026-10-07T06:06:18Z

@chrikrah could you please fix the merge conflicts?

_get_paragraph_position reads a floating drawing's offset from attributes like y, top and positionY, then strips everything that is not a digit or a period: clean_value = re.sub(r"[^0-9.]", "", value) That removes a leading minus. A drawing anchored above or left of its reference point carries a legal negative offset, and it comes back positive: y="-45.5" -> 45.5 The value feeds the top-to-bottom sort of floating textbox paragraphs at :2031, :2045 and :2058, so such a drawing sorts as though it sat that far below the anchor instead of above it. Tests: 67 passed in tests/test_backend_msword.py. Five new cases, four covering the position attributes and one pinning that a unit suffix like 45.5pt still parses. Reverting msword_backend.py alone gives 4 failed, 63 passed. ruff check and ruff format --check are clean on both files. Signed-off-by: Christopher Krah <48338417+chrikrah@users.noreply.github.com>

dolfim-ibm is right about `[^0-9.\-]` on docling-project#4292. The class keeps a minus wherever it sits, so an exponent collapses into something float() rejects and the except swallows it. Measured on the branch head: '1e-5' -> clean_value='1-5' -> ValueError -> offset dropped '1.5e-3in' -> clean_value='1.5-3' -> ValueError -> offset dropped The drawing then falls back to its XML source line. Before this branch it was not better, only wrong in the other direction: `[^0-9.]` read '1e-5' as 15.0. So the value is matched now instead of sieved, and the try/except goes with it. The pattern can only match a valid float literal, so float() cannot raise there, and "no number in the attribute" is the `if match` branch rather than a caught exception. One deviation from the suggested `-?\d+(?:\.\d+)?`: it needs a digit before the period, so on '.5' it matches the '5' and returns 5.0, ten times too large, and +5.0 for '-.5'. Both old versions read that form correctly, and a decimal without its leading zero is ordinary in a length attribute, so the pattern is `-?\d*\.?\d+`. Two test cases pin it. value [^0-9.] (pre-PR) [^0-9.\-] (head) -?\d*\.?\d+ '-45.5' 45.5 -45.5 -45.5 '45.5pt' 45.5 45.5 45.5 '.5' 0.5 0.5 0.5 '-.5' 0.5 -0.5 -0.5 '2e-5' 25.0 dropped 2.0 'auto' dropped dropped dropped The five position tests now go through one helper instead of building the element each time. Tests: 72 passed, 1 skipped in tests/test_backend_msword.py, the skip being the one that needs LibreOffice. Reverting msword_backend.py alone to the branch head gives 3 failed, 69 passed, 1 skipped; reverting it to cea4b45, the state before this branch, gives 8 failed, 64 passed, 1 skipped. ruff check and ruff format --check are clean on both files. Signed-off-by: Christopher Krah <48338417+chrikrah@users.noreply.github.com>

chrikrah

@dolfim-ibm rebased onto ed04377. The only conflict was the end of tests/test_backend_msword.py, where main had added tests, and the fix is unchanged.

$ pytest tests/test_backend_msword.py -q
72 passed, 1 skipped            # main ed04377
83 passed, 1 skipped            # rebased tip d14083f
8 failed, 75 passed, 1 skipped  # tip with msword_backend.py from ed04377

CI on the new head is waiting for a workflow approval, and the 874501d push dismissed your lgtm from 09-21, so the PR needs both before it can merge. Could you approve the run and re-review?

github-actions · 2026-10-13T07:26:42Z

AI triage (advisory)

Checked commit 885d5e3ce2. The analysis read the diff only. It did not run code from this pull request.

The PR changes _get_paragraph_position in the DOCX backend. It now reads the leading number of a position attribute with its sign, so negative drawing offsets stay negative. It adds tests for the sign, unit suffixes, exponent notation, a missing leading zero, and a value with no number.

CI safety: no concern found. A maintainer must still approve CI.

1 related pull request(s) in the same code

PR	State	Signal	Assessment
#4587	open	changes the same code	related: It also edits `msword_backend.py` and its test file, but it splits paragraphs at page and column breaks. It does not change `_get_paragraph_position`.

_{The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.}

PeterStaar-IBM · 2026-10-15T06:23:27Z

@chrikrah could you please resolve the merge conflict (There are unfortunately a lot of PR's coming in)

PeterStaar-IBM

lgtm!

Resolves the tests/test_backend_msword.py conflict: keeps this branch's _get_paragraph_position tests alongside upstream's new footnote-body cases. Signed-off-by: Christopher Krah <christopher.krah@plexito.de>

chrikrah

Conflict resolved: I merged main at 770d713 and pushed 885d5e3 on top, which runs ruff format over the conflicted file. Only tests/test_backend_msword.py conflicted. The resolution keeps both sides: this branch's _get_paragraph_position tests plus upstream's new footnote-body cases.

$ ~/.venvs/docling/bin/python -m pytest -q tests/test_backend_msword.py    # 885d5e3, venv ~/.venvs/docling
1 failed, 97 passed in 56.06s
$ ~/.venvs/docling/bin/python -m pytest -q tests/test_backend_msword.py    # 6fadbae, clean upstream/main
1 failed, 86 passed in 55.37s
# the 1 failure is test_e2e_docx_conversions both times: pre-existing, not from this merge
$ ~/.venvs/docling/bin/python -m pytest -q tests/test_backend_msword.py -k position    # 885d5e3
11 passed, 87 deselected in 2.92s
$ ~/.venvs/docling/bin/ruff format --check tests/test_backend_msword.py docling/backend/msword_backend.py
2 files already formatted
$ ~/.venvs/docling/bin/ruff check tests/test_backend_msword.py docling/backend/msword_backend.py
All checks passed!

On the regex: @dolfim-ibm's re.search suggestion from his 09-21 comment landed in d14083f. The [^0-9.\-] strip it replaced came from us in df84b40. The exponent measurement stands: '1e-5' becomes '1-5', Python raises ValueError, and the offset disappears. The three patterns, measured:

$ python3 - <<'PY'
import re
for v in ['.5', '-.5', '1e-5', 'auto']:
    print(v, re.sub(r'[^0-9.\-]', '', v),
          (m := re.search(r'-?\d+(?:\.\d+)?', v)) and m.group(0),
          (m := re.search(r'-?\d*\.?\d+', v)) and m.group(0))
PY
# cols: input | our old strip | dolfim's -?\d+(?:\.\d+)? | ours -?\d*\.?\d+
.5 .5 5 .5
-.5 -.5 5 -.5
1e-5 1-5 1 1
auto  None None
# dolfim's reads -.5 as 5, so ours stays

@PeterStaar-IBM, lint-and-type is green at 885d5e3, DCO still reads ACTION_REQUIRED, and the 885d5e3 push dismissed your lgtm! since this repo drops stale approvals on push. Could you clear the DCO check and re-approve at 885d5e3?

PeterStaar-IBM · 2026-10-16T06:53:06Z

Conflict resolved: I merged main at 770d713 and pushed 885d5e3 on top, which runs ruff format over the conflicted file. Only tests/test_backend_msword.py conflicted. The resolution keeps both sides: this branch's _get_paragraph_position tests plus upstream's new footnote-body cases.
$ ~/.venvs/docling/bin/python -m pytest -q tests/test_backend_msword.py    # 885d5e3, venv ~/.venvs/docling
1 failed, 97 passed in 56.06s
$ ~/.venvs/docling/bin/python -m pytest -q tests/test_backend_msword.py    # 6fadbae, clean upstream/main
1 failed, 86 passed in 55.37s
# the 1 failure is test_e2e_docx_conversions both times: pre-existing, not from this merge
$ ~/.venvs/docling/bin/python -m pytest -q tests/test_backend_msword.py -k position    # 885d5e3
11 passed, 87 deselected in 2.92s
$ ~/.venvs/docling/bin/ruff format --check tests/test_backend_msword.py docling/backend/msword_backend.py
2 files already formatted
$ ~/.venvs/docling/bin/ruff check tests/test_backend_msword.py docling/backend/msword_backend.py
All checks passed!
On the regex: @dolfim-ibm's re.search suggestion from his 09-21 comment landed in d14083f. The [^0-9.\-] strip it replaced came from us in df84b40. The exponent measurement stands: '1e-5' becomes '1-5', Python raises ValueError, and the offset disappears. The three patterns, measured:
$ python3 - <<'PY'
import re
for v in ['.5', '-.5', '1e-5', 'auto']:
    print(v, re.sub(r'[^0-9.\-]', '', v),
          (m := re.search(r'-?\d+(?:\.\d+)?', v)) and m.group(0),
          (m := re.search(r'-?\d*\.?\d+', v)) and m.group(0))
PY
# cols: input | our old strip | dolfim's -?\d+(?:\.\d+)? | ours -?\d*\.?\d+
.5 .5 5 .5
-.5 -.5 5 -.5
1e-5 1-5 1 1
auto  None None
# dolfim's reads -.5 as 5, so ours stays
@PeterStaar-IBM, lint-and-type is green at 885d5e3, DCO still reads ACTION_REQUIRED, and the 885d5e3 push dismissed your lgtm! since this repo drops stale approvals on push. Could you clear the DCO check and re-approve at 885d5e3?

wonderful, let's let the CI do its thing

dolfim-ibm previously approved these changes Sep 21, 2026

View reviewed changes

chrikrah dismissed dolfim-ibm’s stale review via 874501d September 26, 2026 09:36

chrikrah added 2 commits October 10, 2026 06:31

chrikrah force-pushed the fix/msword-negative-position branch from 874501d to d14083f Compare October 10, 2026 06:40

chrikrah commented Oct 10, 2026

View reviewed changes

github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) bug Something isn't working docx issue related to docx backend labels Oct 13, 2026

PeterStaar-IBM previously approved these changes Oct 15, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 15, 2026

fix(docx): split paragraphs at page and column breaks #4587

Open

3 tasks done

Merge upstream/main into fix(msword) number matching

770d713

Resolves the tests/test_backend_msword.py conflict: keeps this branch's _get_paragraph_position tests alongside upstream's new footnote-body cases. Signed-off-by: Christopher Krah <christopher.krah@plexito.de>

chrikrah dismissed PeterStaar-IBM’s stale review via 770d713 October 16, 2026 03:58

github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) and removed ai:ci-safe AI triage found no CI-safety concern (advisory) labels Oct 16, 2026

style(tests): format test_backend_msword.py

885d5e3

github-actions Bot added ai:ci-safe AI triage found no CI-safety concern (advisory) and removed ai:ci-safe AI triage found no CI-safety concern (advisory) labels Oct 16, 2026

chrikrah commented Oct 16, 2026

View reviewed changes

Conversation

chrikrah commented Sep 18, 2026

Why it matters

Tests

Scope

Duplicate search

Uh oh!

github-actions Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠 Quick Fix: Add a remediation commit

Uh oh!

mergify Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Protections

🟢 Enforce conventional commit

Uh oh!

codecov Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dolfim-ibm left a comment

Choose a reason for hiding this comment

Uh oh!

dolfim-ibm commented Sep 21, 2026

Uh oh!

chrikrah commented Sep 26, 2026

Uh oh!

dolfim-ibm commented Oct 7, 2026

Uh oh!

chrikrah left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI triage (advisory)

Uh oh!

PeterStaar-IBM commented Oct 15, 2026

Uh oh!

PeterStaar-IBM left a comment

Choose a reason for hiding this comment

Uh oh!

chrikrah left a comment

Choose a reason for hiding this comment

Uh oh!

PeterStaar-IBM commented Oct 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 18, 2026 •

edited

Loading

mergify Bot commented Sep 18, 2026 •

edited

Loading

codecov Bot commented Sep 18, 2026 •

edited

Loading

github-actions Bot commented Oct 13, 2026 •

edited

Loading