Revert fix on #400#1518
Merged
Merged
Conversation
…ing reference marker (#1482)" This reverts commit 672983e. #1482 keyed the split decision on the resumed cluster's isBeginning() flag. A callout interrupts the labelled sequence, so the resumed text is always a fresh I-<paragraph> and the flag is set whether or not the paragraph actually ends there. The predicate therefore fired at nearly every callout: over 11,945 documents vs 0.9.0, <p> +85.5%, 98.7% of documents affected, punctuation-only paragraphs +5,640%, text relocated in 23.3% of documents. Revert to the 0.9.0 behaviour (never split after a marker) as a clean base; the follow-up commit re-addresses #1482's real case with a layout signal instead of the sequence label.
Re-addresses the real #1482 case (a paragraph that genuinely ends at a trailing callout) on top of the clean 0.9.0 base restored by the revert, without the isBeginning() predicate that made the previous attempt split at nearly every callout. The decision comes from the layout of the callout and the resumed text: - Same line as the callout -> continue. The common case (a callout inside a paragraph), and it needs no further evidence. - A line break alone proves nothing, since a paragraph interrupted near the right margin also wraps, so a break must also cross a block boundary - blocks being the unit pdfalto groups paragraphs into. - A block boundary also falls inside a paragraph, so the resumed text must additionally read like the start of one (upper case or digit). This stops continuations, and the punctuation closing the interrupted sentence, from being split off. - Text resuming higher up the page has wrapped to the top of the next column, and a page change makes the coordinates incomparable. Neither is evidence of a paragraph break. Splitting requires positive evidence; anything unknown continues the paragraph, so the worst case degrades to 0.9.0 rather than a new failure mode. Only two overloads: the layout-aware one, and the 0.9.0-compatible 2-arg used by Table and external callers. Two details, both found only by instrumenting. LayoutToken .isNewLineAfter() cannot detect the line break: it is only populated by enrichWithNewLineInfo(), which the fulltext path never calls, so it is always false here - the break is read from the Y coordinates. And the token before a cluster boundary is very often a synthesised space with no coordinates (x = y = -1), so the comparison skips back to the last positioned token or every callout looks like a line break. Full corpus, 11,945 documents vs 0.9.0: <p> 1,170,384 -> 1,177,912 (+0.64%, master +85.5%); paragraphs opening lower case unchanged at 137,715 (master +162%); punctuation-only 3,357 -> 3,413 (master +3,002%). Body-text content is unaffected: of 448 documents differing from 0.9.0, 427 already differed master-vs-0.9.0 and the 21 remaining are pdfalto batch variance (equation and table tokens), which paragraph boundaries cannot move. The #1482 unit tests are kept as the first two cases in the suite, now asserting the corrected layout-driven contract.
Signed-off-by: Luca Foppiano <luca@foppiano.org>
…sions" This reverts commit ddaf222.
…ence label" This reverts commit 350d3b8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We revert the fix of #400 which introduced a regression, and postpone this fix.