Skip to content

Commit d31e15a

Browse files
authored
Merge pull request #166 from artiz/claude/orphan-picture-text-165
pdf: pictures no longer claim cells in orphan recovery (#165)
2 parents 7c950a0 + 35a52e6 commit d31e15a

8 files changed

Lines changed: 171 additions & 64 deletions

File tree

CLAUDE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ validated for byte-for-byte conformance against upstream Python docling.
88

99
- **Every commit must be signed off by the author.** End each commit message
1010
with `Signed-off-by: name <email>`.
11+
- **Releases are automatic on merge; only the `fix:` prefix matters.** CI
12+
(`scripts/ci/bump_version.sh`): a merge whose commits are all
13+
`fix:`/`perf:`/`revert:` cuts a patch (0.49.0 → 0.49.1); **any other
14+
commit** — whatever its prefix — bumps the 0.x "major" (0.49.0 → 0.50.0).
15+
So prefix bug fixes `fix(scope): …` and don't worry about the rest.
16+
Docs/CI-only merges still release nothing (release.sh only fires when a
17+
publishable crate's source changed). No automatic semver-major: v1.0.0 is
18+
cut by hand (`force_version` dispatch input) when the repo hits its 100-star
19+
milestone.
1120
- Claude Web: **Never open pull requests on `artiz/docling.rs`.** Push a `claude/<topic>`
1221
branch and hand back a compare link
1322
(`https://github.com/docling-project/docling.rs/compare/master...artiz:docling.rs:<branch>?expand=1`);

crates/docling-pdf/src/assemble.rs

Lines changed: 106 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,24 @@ pub fn resolve(regions: Vec<Region>) -> Vec<Region> {
121121
}
122122

123123
/// Drop a regular region that is >80% contained in a surviving special region we
124-
/// render **as a single unit** — a picture, or a table/table-of-contents index —
125-
/// ported from docling's "Remove regular clusters that are included in wrappers"
126-
/// step: the special absorbs it as a child (a table cell, an in-figure label), so
127-
/// it must not also be emitted as its own paragraph/list-item. This stops the
128-
/// survey list-items from appearing both inside the detected table and again as
129-
/// bullets (`table_mislabeled_as_picture`).
124+
/// render **as a single unit** — a table/table-of-contents index — ported from
125+
/// docling's "Remove regular clusters that are included in wrappers" step: the
126+
/// special absorbs it as a child (a table cell), so it must not also be emitted
127+
/// as its own paragraph/list-item. This stops the survey list-items from
128+
/// appearing both inside the detected table and again as bullets
129+
/// (`table_mislabeled_as_picture`).
130+
///
131+
/// `picture` regions stay in the swallow set even after #165: docling keeps a
132+
/// picture's contained clusters as the `PictureItem`'s *children* in the
133+
/// document JSON (`ReadingOrderModel._add_child_elements`), but its
134+
/// `MarkdownPictureSerializer` prints only the caption and the image — the
135+
/// children never reach the Markdown (verified against the corpus groundtruth:
136+
/// `amt_handbook`'s in-figure callout labels are absent). Dropping the
137+
/// fully-contained regulars here reproduces exactly that. What #165 *does*
138+
/// change is upstream, in [`add_orphan_regions`]: pictures no longer claim
139+
/// cells, so a line only partially under a figure box (straddling its border,
140+
/// ≤80 % contained) now forms an orphan region that survives this drop — those
141+
/// words were silently erased before, and docling emits them.
130142
///
131143
/// `form` / `key_value_region` wrappers are deliberately **excluded**: this
132144
/// pipeline does not render them as a structured block (they are skipped), so
@@ -315,10 +327,23 @@ pub fn add_orphan_regions(regions: &mut Vec<Region>, cells: &[TextCell]) {
315327
// intersection-over-self > 0.2; only cells below that for *every* region are
316328
// orphans. (Our text extraction uses a stricter 0.5, but matching docling's
317329
// 0.2 here avoids emitting cells it already placed in a neighbouring region.)
330+
//
331+
// Only *regular* clusters claim cells: docling's `_find_unassigned_cells`
332+
// walks `regular_clusters` alone, so a cell under a `picture` or a wrapper
333+
// (`table`/`document_index`/`form`/`key_value_region`) that no regular
334+
// cluster covers still becomes an orphan text cluster (#165). The orphans
335+
// that end up *fully* inside the special are re-dropped by
336+
// [`drop_contained_regulars`] (docling's Markdown drops them the same way
337+
// — a picture's children never reach its `MarkdownPictureSerializer`
338+
// output, a table's text renders through the reconstructed grid). The
339+
// observable fix is the border-straddlers: a line only partially under a
340+
// figure box used to lose its cells to the picture's 0.2 claim and vanish
341+
// — now it forms an orphan region and is emitted, as docling does.
318342
let assigned = |c: &TextCell| {
319343
let ca = area(c.l, c.t, c.r, c.b).max(1.0);
320344
regions
321345
.iter()
346+
.filter(|r| r.label != "picture" && !is_wrapper(r.label))
322347
.any(|r| inter(r, c.l, c.t, c.r, c.b) / ca > 0.2)
323348
};
324349
// Collect orphan cells (non-empty, unassigned), in page order.
@@ -400,6 +425,11 @@ pub fn recover_text_panels(regions: &mut Vec<Region>, cells: &[TextCell]) {
400425
})
401426
.collect();
402427
let mut out: Vec<Region> = Vec::with_capacity(regions.len());
428+
// Synthesized paragraphs and the demoted panels' boxes are kept separate
429+
// from `out` until the end: the dedup filter below must not confuse a
430+
// paragraph we just built with a pre-existing region inside the panel.
431+
let mut demoted_paras: Vec<Region> = Vec::new();
432+
let mut demoted_boxes: Vec<(f32, f32, f32, f32)> = Vec::new();
403433
for (i, r) in regions.drain(..).enumerate() {
404434
if r.label != "picture" || captioned[i] {
405435
out.push(r);
@@ -468,7 +498,7 @@ pub fn recover_text_panels(regions: &mut Vec<Region>, cells: &[TextCell]) {
468498
}
469499
_ => {
470500
if let Some((pl, pt, pr, pb)) = para.take() {
471-
out.push(Region {
501+
demoted_paras.push(Region {
472502
label: "text",
473503
score: r.score,
474504
l: pl,
@@ -482,7 +512,7 @@ pub fn recover_text_panels(regions: &mut Vec<Region>, cells: &[TextCell]) {
482512
}
483513
}
484514
if let Some((pl, pt, pr, pb)) = para {
485-
out.push(Region {
515+
demoted_paras.push(Region {
486516
label: "text",
487517
score: r.score,
488518
l: pl,
@@ -491,7 +521,23 @@ pub fn recover_text_panels(regions: &mut Vec<Region>, cells: &[TextCell]) {
491521
b: pb,
492522
});
493523
}
524+
demoted_boxes.push((r.l, r.t, r.r, r.b));
525+
}
526+
// The paragraphs are rebuilt from *all* of the panel's cells, so any
527+
// surviving text region inside a demoted panel (an orphan cluster or a
528+
// layout-detected fragment — pictures no longer swallow them, #165) would
529+
// say the same words twice. Consume those; wrappers and pictures stay.
530+
if !demoted_boxes.is_empty() {
531+
out.retain(|r| {
532+
r.label == "picture" || is_wrapper(r.label) || {
533+
let ra = area(r.l, r.t, r.r, r.b).max(1.0);
534+
!demoted_boxes
535+
.iter()
536+
.any(|&(l, t, rr, b)| inter(r, l, t, rr, b) / ra > 0.5)
537+
}
538+
});
494539
}
540+
out.extend(demoted_paras);
495541
*regions = out;
496542
}
497543

@@ -1880,6 +1926,58 @@ mod tests {
18801926
use crate::pdfium_backend::{LinkAnnot, PdfPage, TextCell};
18811927
use docling_core::Node;
18821928

1929+
/// #165: a picture no longer claims cells at 0.2 intersection-over-self.
1930+
/// A line straddling the figure border (≤80 % contained) becomes an orphan
1931+
/// region and survives the contained-regulars drop — before the fix its
1932+
/// cells were silently erased. A line fully inside the picture is still
1933+
/// re-dropped, matching docling's Markdown (a picture's children never
1934+
/// reach its serializer's output).
1935+
#[test]
1936+
fn border_straddling_lines_survive_picture_interior_is_still_dropped() {
1937+
let pic = Region {
1938+
label: "picture",
1939+
score: 0.9,
1940+
l: 0.0,
1941+
t: 0.0,
1942+
r: 100.0,
1943+
b: 100.0,
1944+
};
1945+
// ~35 % of this cell overlaps the picture (l=90..120 of 0..100): above
1946+
// the old 0.2 claim (was swallowed), below full containment (survives).
1947+
let straddler = TextCell {
1948+
text: "axis label".into(),
1949+
l: 90.0,
1950+
t: 40.0,
1951+
r: 120.0,
1952+
b: 48.0,
1953+
};
1954+
let interior = TextCell {
1955+
text: "in-figure callout".into(),
1956+
l: 10.0,
1957+
t: 10.0,
1958+
r: 60.0,
1959+
b: 18.0,
1960+
};
1961+
let mut regions = vec![pic];
1962+
super::add_orphan_regions(&mut regions, &[straddler, interior]);
1963+
assert_eq!(
1964+
regions.iter().filter(|r| r.label == "text").count(),
1965+
2,
1966+
"both unclaimed lines become orphans"
1967+
);
1968+
super::drop_contained_regulars(&mut regions);
1969+
let texts: Vec<(f32, f32)> = regions
1970+
.iter()
1971+
.filter(|r| r.label == "text")
1972+
.map(|r| (r.l, r.r))
1973+
.collect();
1974+
assert_eq!(
1975+
texts,
1976+
[(90.0, 120.0)],
1977+
"the straddler is emitted, the fully-contained callout is not"
1978+
);
1979+
}
1980+
18831981
/// A colored terms-and-conditions panel detected as `picture` demotes into
18841982
/// per-paragraph `text` regions (the blank line between C.7 and C.8 splits
18851983
/// them); a chart whose only text is a few narrow axis labels keeps its

crates/docling-pdf/src/lib.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -595,14 +595,9 @@ impl Worker {
595595
// (picture_classification stays byte-exact), so the recognized cells
596596
// there only ever serve the panel-demotion decision above.
597597
if ocred && !pic_cells.is_empty() {
598-
let mut probe: Vec<layout::Region> = regions
599-
.iter()
600-
.filter(|r| r.label != "picture")
601-
.cloned()
602-
.collect();
603-
let keep_from = probe.len();
604-
assemble::add_orphan_regions(&mut probe, &pic_cells);
605-
regions.extend(probe.drain(keep_from..));
598+
// Pictures (and wrappers) no longer count as claimers (#165), so
599+
// the plain orphan pass places the recognized lines directly.
600+
assemble::add_orphan_regions(&mut regions, &pic_cells);
606601
} else if !ocred && !pic_cells.is_empty() {
607602
// Digital page, picture kept: its speculative OCR cells must not
608603
// linger in the text-cell set (they were appended at the tail).

crates/docling-wasm/src/digital.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,9 @@ impl DigitalConverter {
245245
// Dense wide OCR text demotes the crop to paragraphs;
246246
// sparse text keeps the picture and reads out beside it.
247247
docling_pdf::assemble::recover_text_panels(&mut regions, &page.cells);
248-
let mut probe: Vec<Region> = regions
249-
.iter()
250-
.filter(|r| r.label != "picture")
251-
.cloned()
252-
.collect();
253-
let keep_from = probe.len();
254-
add_orphan_regions(&mut probe, &ocr_cells);
255-
regions.extend(probe.drain(keep_from..));
248+
// Pictures no longer count as claimers (#165), so the
249+
// plain orphan pass places the recognized lines directly.
250+
add_orphan_regions(&mut regions, &ocr_cells);
256251
}
257252
}
258253
}

crates/docling-wasm/src/scanned.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -250,14 +250,9 @@ impl ScannedConverter {
250250
// text (a chat screenshot's bubbles) keeps the picture and
251251
// reads out beside it, as before.
252252
docling_pdf::assemble::recover_text_panels(&mut regions, &cells);
253-
let mut probe: Vec<docling_pdf::layout::Region> = regions
254-
.iter()
255-
.filter(|r| r.label != "picture")
256-
.cloned()
257-
.collect();
258-
let keep_from = probe.len();
259-
docling_pdf::assemble::add_orphan_regions(&mut probe, &pcells);
260-
regions.extend(probe.drain(keep_from..));
253+
// Pictures no longer count as claimers (#165), so the plain
254+
// orphan pass places the recognized lines directly.
255+
docling_pdf::assemble::add_orphan_regions(&mut regions, &pcells);
261256
}
262257
}
263258

docs/PDF_CONFORMANCE.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ are no longer scored.)
2929
| 2305.03393v1 | 26 | title-page reading order + author-ID run spacing |
3030
| normal_4pages | 50 | reading order (heading numbering, footnote order) + recovered panel text |
3131
| right_to_left_03 | 60 | RTL bidi |
32-
| table_mislabeled_as_picture | 86 | layout over-detects tables (survey rendered as tables) |
33-
| 2206.01062 | 80 | TableFormer multi-row headers + title-page reading order |
32+
| table_mislabeled_as_picture | 80 | layout over-detects tables (survey rendered as tables) |
33+
| 2206.01062 | 76 | TableFormer multi-row headers + title-page reading order |
3434
| 2203.01017v2 | 132 | TableFormer structure + reading order |
3535
| redp5110_sampled | 196 | TOC OTSL structure (model-level); cover-page ordering |
3636

@@ -65,6 +65,16 @@ normal_4pages' cover publisher block), not regressions; captioned figures
6565
(the corpus' document screenshots) and sparse-label charts keep their crops
6666
and their byte-exact output — `picture_classification` stays **exact**.
6767

68+
The #165 orphan-claimer fix mirrors docling's regular/special cluster split:
69+
only regular clusters claim cells (`_find_unassigned_cells` walks
70+
`regular_clusters` alone), so a line that merely *straddles* a figure border
71+
no longer loses its cells to the picture's 0.2 claim — it becomes an orphan
72+
text region and is emitted, as docling does. Orphans that end up fully inside
73+
a picture or table are still re-dropped, matching docling's Markdown (a
74+
picture's children never reach `MarkdownPictureSerializer` output; a table's
75+
text renders through the grid). Took 2206 80→76 and table_mislabeled 86→80
76+
with every other fixture byte-identical.
77+
6878
## DocLang (`.dclx`) conformance
6979

7080
Separate from the Markdown metric above: how close `--to dclx` gets to docling's

scripts/ci/bump_version.sh

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
#!/usr/bin/env bash
22
#
3-
# Decide the next workspace version from the conventional-commit messages since
4-
# the last release tag, and print it to stdout. Prints NOTHING (exit 0) when no
5-
# release-worthy commit is found, so the caller can skip the release.
3+
# Decide the next workspace version from the commit messages since the last
4+
# release tag, and print it to stdout. Prints NOTHING (exit 0) when the range
5+
# is empty, so the caller can skip the release.
66
#
7-
# <type>!: … or "BREAKING CHANGE" in the body -> major
8-
# feat: … -> minor
9-
# fix:/perf:/revert: … -> patch
10-
# docs/chore/ci/refactor/test/style/build/… -> no release
7+
# only fix:/perf:/revert: commits in the range -> patch
8+
# anything else -> minor (the 0.x "major")
9+
#
10+
# Releases deliberately do NOT depend on conventional-commit prefixes: the
11+
# repo's de-facto style is bare area prefixes ("pdf: …", "wasm: …"), and
12+
# requiring feat:/fix: silently stopped every release after v0.49.0. Any
13+
# non-fix merge now bumps 0.49 -> 0.50; a docs/CI-only merge still releases
14+
# nothing because release.sh's source-change gate sees no publishable crate
15+
# source in the diff.
16+
#
17+
# There is intentionally NO automatic semver-major: v1.0.0 is a milestone the
18+
# maintainer cuts by hand (FORCE_VERSION=1.0.0 via the CI dispatch input) —
19+
# "когда 100 звёзд дадут". Until then everything stays 0.x.
1120
#
1221
# Pure: reads git history + the root Cargo.toml; writes nothing.
1322
# Usage: scripts/ci/bump_version.sh
@@ -23,27 +32,20 @@ else
2332
range="HEAD" # no release tag yet: consider the whole history
2433
fi
2534

26-
# Subject + body of every non-merge commit in range.
27-
log="$(git log "$range" --no-merges --format='%s%n%b')"
35+
# Subjects decide the bump.
36+
subjects="$(git log "$range" --no-merges --format='%s')"
2837

2938
bump=""
30-
if grep -qE '^[a-z]+(\([^)]*\))?!:' <<<"$log" || grep -q 'BREAKING CHANGE' <<<"$log"; then
31-
bump="major"
32-
elif grep -qE '^feat(\([^)]*\))?:' <<<"$log"; then
39+
if grep -vE '^(fix|perf|revert)(\([^)]*\))?:' <<<"$subjects" | grep -q '[^[:space:]]'; then
3340
bump="minor"
34-
elif grep -qE '^(fix|perf|revert)(\([^)]*\))?:' <<<"$log"; then
41+
elif grep -q '[^[:space:]]' <<<"$subjects"; then
3542
bump="patch"
3643
fi
3744

3845
[[ -z "$bump" ]] && exit 0
3946

4047
IFS=. read -r major minor patch <<<"$current"
4148
case "$bump" in
42-
major)
43-
major=$((major + 1))
44-
minor=0
45-
patch=0
46-
;;
4749
minor)
4850
minor=$((minor + 1))
4951
patch=0

scripts/ci/release.sh

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#!/usr/bin/env bash
22
#
33
# Master-only release step (run by .github/workflows/ci.yml after the lint/test
4-
# gates pass). Computes the next version from conventional commits; if there is
5-
# one, it bumps the workspace version, commits + tags it, pushes to master,
6-
# publishes the changed crates, and assembles the GitHub Release notes (a commit
7-
# list since the previous tag) for the workflow to publish. A clean no-op when no
8-
# release-worthy commit landed since the last tag.
4+
# gates pass). Computes the next version from the commits since the last tag
5+
# (scripts/ci/bump_version.sh: fix:/perf:/revert:-only range -> patch, anything
6+
# else -> the 0.x "major"; no automatic 1.0.0); if there is one, it bumps the
7+
# workspace version, commits + tags it, pushes to master, publishes the changed
8+
# crates, and assembles the GitHub Release notes (a commit list since the
9+
# previous tag) for the workflow to publish. A clean no-op when nothing landed
10+
# since the last tag.
911
#
1012
# The release commit is pushed with RELEASE_PAT (an admin token, so it satisfies
1113
# the master branch ruleset) and carries `[skip ci]`, so it does not re-trigger CI.
@@ -20,16 +22,17 @@ cd "$(dirname "$0")/../.."
2022

2123
new="${FORCE_VERSION:-$(scripts/ci/bump_version.sh)}"
2224
if [[ -z "$new" ]]; then
23-
echo "No release-worthy commits since the last tag — nothing to release."
25+
echo "No commits since the last tag — nothing to release."
2426
exit 0
2527
fi
2628
[[ -n "${FORCE_VERSION:-}" ]] && echo ">> forced release of v$new"
2729

28-
# A release-worthy *message* is not enough: only cut a new crates.io version when
29-
# a *publishable* crate's actual source changed since the last tag. This stops a
30-
# docs / CI / Python-binding-only change (e.g. `feat(docs): …`, `feat(docling-py): …`)
31-
# from minting and publishing a new version of every crate with no code change.
32-
# FORCE_VERSION bypasses the gate (a deliberate manual (re)publish).
30+
# Commits alone are not enough: only cut a new crates.io version when a
31+
# *publishable* crate's actual source changed since the last tag. This is what
32+
# stops a docs / CI / demo / Python-binding-only merge (now that every non-fix
33+
# commit is release-worthy) from minting and publishing a new version of every
34+
# crate with no code change. FORCE_VERSION bypasses the gate (a deliberate
35+
# manual (re)publish).
3336
if [[ -z "${FORCE_VERSION:-}" ]]; then
3437
gate_prev_tag="$(git tag --list 'v*' --sort=-version:refname | head -n1)"
3538
gate_range="${gate_prev_tag:+$gate_prev_tag..}HEAD"
@@ -42,7 +45,7 @@ if [[ -z "${FORCE_VERSION:-}" ]]; then
4245
gate_paths+=("crates/$c/src" "crates/$c/Cargo.toml" "crates/$c/build.rs")
4346
done
4447
if [[ -z "$(git diff --name-only "$gate_range" -- "${gate_paths[@]}")" ]]; then
45-
echo "Release-worthy commit found, but no publishable crate source changed" \
48+
echo "Commits since the last tag, but no publishable crate source changed" \
4649
"since ${gate_prev_tag:-the start of history} — skipping release."
4750
exit 0
4851
fi

0 commit comments

Comments
 (0)