From 58f75b015e7cfe1804cefea846d752125eece957 Mon Sep 17 00:00:00 2001 From: Luca Foppiano Date: Wed, 22 Jul 2026 08:33:49 +0200 Subject: [PATCH 1/5] Revert "Fix TEI paragraph split when a paragraph starts after a trailing reference marker (#1482)" This reverts commit 672983e35f2399300c019289fb90da3cb9766a5f. #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- 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,

+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. --- .../grobid/core/document/TEIFormatter.java | 32 +++---------------- .../core/document/TEIFormatterTest.java | 27 ---------------- 2 files changed, 4 insertions(+), 55 deletions(-) diff --git a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java index 90a79f1f03..c493a1bd12 100755 --- a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java +++ b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java @@ -57,7 +57,6 @@ import org.grobid.core.lang.Language; import org.grobid.core.layout.*; import org.grobid.core.lexicon.Lexicon; -import org.grobid.core.tokenization.LabeledTokensContainer; import org.grobid.core.tokenization.TaggingTokenCluster; import org.grobid.core.tokenization.TaggingTokenClusteror; import org.grobid.core.utilities.*; @@ -1801,7 +1800,7 @@ public StringBuilder toTEITextPiece( if (CollectionUtils.isEmpty(matchedLabelPositions)) { String clusterContent = LayoutTokensUtil.normalizeDehyphenizeText(clusterTokens); - if (isNewParagraph(lastClusterLabel, curParagraph, cluster)) { + if (isNewParagraph(lastClusterLabel, curParagraph)) { if (curParagraph != null && config.isWithSentenceSegmentation()) { segmentIntoSentences(curParagraph, curParagraphTokens, config, doc.getLanguage()); } @@ -1832,7 +1831,7 @@ public StringBuilder toTEITextPiece( curParagraph.appendChild(clusterContent); curParagraphTokens.addAll(clusterTokens); } else { - if (isNewParagraph(lastClusterLabel, curParagraph, cluster)) { + if (isNewParagraph(lastClusterLabel, curParagraph)) { if (curParagraph != null && config.isWithSentenceSegmentation()) { segmentIntoSentences( curParagraph, @@ -2154,32 +2153,9 @@ private static Element generateNoteRef( return ref; } - public static boolean isNewParagraph( - TaggingLabel lastClusterLabel, - Element curParagraph, - TaggingTokenCluster currentCluster) { - if (curParagraph == null) { - return true; - } - - if (!MARKER_LABELS.contains(lastClusterLabel) - && lastClusterLabel != TaggingLabels.FIGURE - && lastClusterLabel != TaggingLabels.TABLE) { - return true; - } - - if (MARKER_LABELS.contains(lastClusterLabel) - && currentCluster != null - && CollectionUtils.isNotEmpty(currentCluster.getLabeledTokensContainers())) { - LabeledTokensContainer firstContainer = currentCluster.getLabeledTokensContainers().get(0); - return firstContainer.isBeginning(); - } - - return false; - } - public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { - return isNewParagraph(lastClusterLabel, curParagraph, null); + return (!MARKER_LABELS.contains(lastClusterLabel) && lastClusterLabel != TaggingLabels.FIGURE + && lastClusterLabel != TaggingLabels.TABLE) || curParagraph == null; } public void segmentIntoSentences( diff --git a/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java b/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java index 153af4017c..c3b4ec4cc4 100644 --- a/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java +++ b/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java @@ -32,10 +32,7 @@ import org.grobid.core.data.Figure; import org.grobid.core.data.Note; import org.grobid.core.data.Table; -import org.grobid.core.engines.label.TaggingLabels; import org.grobid.core.layout.LayoutToken; -import org.grobid.core.tokenization.LabeledTokensContainer; -import org.grobid.core.tokenization.TaggingTokenCluster; import org.grobid.core.utilities.GrobidProperties; import org.grobid.core.utilities.LayoutTokensUtil; @@ -426,28 +423,4 @@ public void testMarkReferencesTableTEI_truncatedRef2_referenceAtBeginning() thro assertThat(nodes.get(5).toXML(), is(" ")); } - @Test - public void testIsNewParagraph_afterMarkerWithBeginningParagraphLabel() { - TaggingTokenCluster paragraphCluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); - paragraphCluster.addLabeledTokensContainer( - new LabeledTokensContainer(List.of(), "Because", TaggingLabels.PARAGRAPH, true)); - - boolean isNewParagraph = TEIFormatter - .isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), paragraphCluster); - - assertThat(isNewParagraph, is(true)); - } - - @Test - public void testIsNewParagraph_afterMarkerWithoutBeginningParagraphLabel() { - TaggingTokenCluster paragraphCluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); - paragraphCluster.addLabeledTokensContainer( - new LabeledTokensContainer(List.of(), "continuation", TaggingLabels.PARAGRAPH, false)); - - boolean isNewParagraph = TEIFormatter - .isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), paragraphCluster); - - assertThat(isNewParagraph, is(false)); - } - } From 350d3b8354b04d214b7f82c4044928930fa69ce6 Mon Sep 17 00:00:00 2001 From: Luca Foppiano Date: Wed, 22 Jul 2026 08:39:56 +0200 Subject: [PATCH 2/5] fix: split paragraphs after a callout on layout, not 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:

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. --- .../grobid/core/document/TEIFormatter.java | 152 +++++++++++++++- .../core/document/TEIFormatterTest.java | 163 ++++++++++++++++++ 2 files changed, 311 insertions(+), 4 deletions(-) diff --git a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java index c493a1bd12..b744850e9a 100755 --- a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java +++ b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java @@ -1800,7 +1800,7 @@ public StringBuilder toTEITextPiece( if (CollectionUtils.isEmpty(matchedLabelPositions)) { String clusterContent = LayoutTokensUtil.normalizeDehyphenizeText(clusterTokens); - if (isNewParagraph(lastClusterLabel, curParagraph)) { + if (isNewParagraph(lastClusterLabel, curParagraph, cluster, curParagraphTokens)) { if (curParagraph != null && config.isWithSentenceSegmentation()) { segmentIntoSentences(curParagraph, curParagraphTokens, config, doc.getLanguage()); } @@ -1831,7 +1831,7 @@ public StringBuilder toTEITextPiece( curParagraph.appendChild(clusterContent); curParagraphTokens.addAll(clusterTokens); } else { - if (isNewParagraph(lastClusterLabel, curParagraph)) { + if (isNewParagraph(lastClusterLabel, curParagraph, cluster, curParagraphTokens)) { if (curParagraph != null && config.isWithSentenceSegmentation()) { segmentIntoSentences( curParagraph, @@ -2153,9 +2153,153 @@ private static Element generateNoteRef( return ref; } + /** + * Decide whether the current cluster opens a new paragraph, or continues the one + * interrupted by the preceding cluster. + *

+ * Only the marker/figure/table case is interesting: any other preceding label always + * starts a new paragraph. A citation, figure or table callout sits inside a + * paragraph far more often than it ends one, so the default after a callout is to + * continue. + *

+ * The sequence label cannot make this decision. A callout interrupts the labelled + * sequence, so the resumed text is always a fresh {@code I-} - it carries + * "beginning of sequence" whether it continues the interrupted paragraph or starts a new + * one, and keying off it splits at nearly every callout (issue #1482). The signal has to + * come from the layout instead: text resuming on the same line as the callout is bound + * to it, and a genuine paragraph break additionally shows up as a new block. + * + * @param precedingTokens the tokens accumulated in the current paragraph, whose last + * element is the callout the current cluster follows. When null + * or empty there is no layout evidence, and the paragraph is + * continued. + */ + public static boolean isNewParagraph( + TaggingLabel lastClusterLabel, + Element curParagraph, + TaggingTokenCluster currentCluster, + List precedingTokens) { + if (curParagraph == null) { + return true; + } + + if (!MARKER_LABELS.contains(lastClusterLabel) + && lastClusterLabel != TaggingLabels.FIGURE + && lastClusterLabel != TaggingLabels.TABLE) { + return true; + } + + if (MARKER_LABELS.contains(lastClusterLabel)) { + return startsNewParagraphAfterCallout(precedingTokens, currentCluster); + } + + return false; + } + + /** + * Backward-compatible overload with no layout information: reproduces the 0.9.0 behaviour + * of never splitting after a marker, figure or table. Used where no callout context is + * available (e.g. {@link org.grobid.core.data.Table}). + */ public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { - return (!MARKER_LABELS.contains(lastClusterLabel) && lastClusterLabel != TaggingLabels.FIGURE - && lastClusterLabel != TaggingLabels.TABLE) || curParagraph == null; + return isNewParagraph(lastClusterLabel, curParagraph, null, null); + } + + /** + * Layout test for text following a callout. Returns true only on positive evidence of a + * paragraph break, so that an unknown layout continues the paragraph as before #1482. + */ + private static boolean startsNewParagraphAfterCallout( + List precedingTokens, + TaggingTokenCluster currentCluster) { + if (CollectionUtils.isEmpty(precedingTokens) || currentCluster == null) { + return false; + } + + LayoutToken callout = lastPositionedToken(precedingTokens); + LayoutToken resumed = firstPositionedToken(currentCluster.concatTokens()); + if (callout == null || resumed == null) { + return false; + } + + // Across a page break the coordinates are not comparable. A paragraph does run over + // a page boundary, so this is not evidence of a break either way - continue. + if (callout.getPage() != resumed.getPage()) { + return false; + } + + // Text resuming on the same line as the callout is bound to it. This is both the + // most common case and the one that needs no further evidence. Note that + // LayoutToken.isNewLineAfter() cannot be used here: it is only populated by + // enrichWithNewLineInfo(), which the fulltext path never calls, so it is always + // false on these tokens. + double sameLineTolerance = Math.max(1.0, callout.getHeight() / 2.0); + if (Math.abs(resumed.getY() - callout.getY()) <= sameLineTolerance) { + return false; + } + + // Text that resumes higher up the page has wrapped to the top of the next column. + // That is a column break, not a paragraph break. + if (resumed.getY() < callout.getY()) { + return false; + } + + // A line break alone proves nothing: a paragraph interrupted by a callout near the + // right margin also wraps. A new paragraph is a new block in the layout, so require + // that. Blocks are the unit pdfalto groups paragraphs into, which makes a block + // change the closest thing to a paragraph boundary the layout offers. + if (callout.getBlockPtr() == resumed.getBlockPtr()) { + return false; + } + + // The block signal on its own splits about as often wrongly as rightly: a block + // boundary also falls inside a paragraph. A paragraph that genuinely starts here + // also reads like one, so require the resumed text to open the way a sentence does. + // This is what rejects the continuations that resume on lower case and the fragments + // that begin with the punctuation closing the previous sentence. + return startsLikeASentence(resumed.getText()); + } + + private static boolean startsLikeASentence(String text) { + if (StringUtils.isBlank(text)) { + return false; + } + char first = text.charAt(0); + return Character.isUpperCase(first) || Character.isDigit(first); + } + + /** + * The last token carrying layout coordinates. Whitespace tokens are synthesised without + * position (x = y = -1), and the token immediately before a cluster boundary is very + * often one of those, so taking the last token blindly compares against a token that has + * no place on the page. + */ + private static LayoutToken lastPositionedToken(List tokens) { + for (int i = tokens.size() - 1; i >= 0; i--) { + if (isPositioned(tokens.get(i))) { + return tokens.get(i); + } + } + return null; + } + + private static LayoutToken firstPositionedToken(List tokens) { + if (CollectionUtils.isEmpty(tokens)) { + return null; + } + for (LayoutToken token : tokens) { + if (isPositioned(token)) { + return token; + } + } + return null; + } + + private static boolean isPositioned(LayoutToken token) { + return token != null + && StringUtils.isNotBlank(token.getText()) + && token.getX() >= 0 + && token.getY() >= 0; } public void segmentIntoSentences( diff --git a/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java b/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java index c3b4ec4cc4..72523162d3 100644 --- a/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java +++ b/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java @@ -32,7 +32,10 @@ import org.grobid.core.data.Figure; import org.grobid.core.data.Note; import org.grobid.core.data.Table; +import org.grobid.core.engines.label.TaggingLabels; import org.grobid.core.layout.LayoutToken; +import org.grobid.core.tokenization.LabeledTokensContainer; +import org.grobid.core.tokenization.TaggingTokenCluster; import org.grobid.core.utilities.GrobidProperties; import org.grobid.core.utilities.LayoutTokensUtil; @@ -423,4 +426,164 @@ public void testMarkReferencesTableTEI_truncatedRef2_referenceAtBeginning() thro assertThat(nodes.get(5).toXML(), is(" ")); } + // --- isNewParagraph: layout-based split after a callout (replaces #1482) --- + // The two #1482 scenarios are kept here as the first two cases, now asserting the + // corrected (layout-driven) contract instead of the isBeginning() one. + + @Test + public void testIsNewParagraph_afterMarkerBeginningLabel_noLayoutEvidenceContinues() { + // #1482 asserted this split because the resumed cluster carried isBeginning(). That + // flag is uninformative after a callout, so with no layout to go on the paragraph + // continues. + TaggingTokenCluster paragraphCluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); + paragraphCluster.addLabeledTokensContainer( + new LabeledTokensContainer(List.of(), "Because", TaggingLabels.PARAGRAPH, true)); + + boolean isNewParagraph = TEIFormatter + .isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), paragraphCluster, null); + + assertThat(isNewParagraph, is(false)); + } + + @Test + public void testIsNewParagraph_afterMarkerContinuationLabel_continues() { + TaggingTokenCluster paragraphCluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); + paragraphCluster.addLabeledTokensContainer( + new LabeledTokensContainer(List.of(), "continuation", TaggingLabels.PARAGRAPH, false)); + + boolean isNewParagraph = TEIFormatter + .isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), paragraphCluster, null); + + assertThat(isNewParagraph, is(false)); + } + + @Test + public void testIsNewParagraph_nonMarkerLabelAlwaysStartsANewParagraph() { + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.SECTION, new Element("p"), null, null), + is(true)); + } + + @Test + public void testIsNewParagraph_nullParagraphAlwaysStartsANewParagraph() { + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, null, null, null), + is(true)); + } + + @Test + public void testIsNewParagraph_twoArgOverloadKeeps090Behaviour() { + // The Table path and any external caller use the 2-arg overload: never split after a + // marker/figure/table, split after anything else. + assertThat(TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p")), is(false)); + assertThat(TEIFormatter.isNewParagraph(TaggingLabels.FIGURE, new Element("p")), is(false)); + assertThat(TEIFormatter.isNewParagraph(TaggingLabels.SECTION, new Element("p")), is(true)); + } + + @Test + public void testIsNewParagraph_textResumingOnTheSameLineContinuesTheParagraph() { + List preceding = List.of(token("]", 300.0, 400.0, 0)); + TaggingTokenCluster cluster = clusterOf(token("Because", 310.0, 400.0, 0)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + @Test + public void testIsNewParagraph_newLineInTheSameBlockContinuesTheParagraph() { + List preceding = List.of(token("]", 500.0, 400.0, 7)); + TaggingTokenCluster cluster = clusterOf(token("Because", 72.0, 412.0, 7)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + @Test + public void testIsNewParagraph_newBlockOnANewLineStartsANewParagraph() { + List preceding = List.of(token("]", 500.0, 400.0, 7)); + TaggingTokenCluster cluster = clusterOf(token("Because", 72.0, 412.0, 8)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(true)); + } + + @Test + public void testIsNewParagraph_newBlockResumingOnLowerCaseContinuesTheParagraph() { + List preceding = List.of(token("]", 500.0, 400.0, 7)); + TaggingTokenCluster cluster = clusterOf(token("because", 72.0, 412.0, 8)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + @Test + public void testIsNewParagraph_newBlockResumingOnPunctuationContinuesTheParagraph() { + List preceding = List.of(token("]", 500.0, 400.0, 7)); + TaggingTokenCluster cluster = clusterOf(token(".", 72.0, 412.0, 8)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + @Test + public void testIsNewParagraph_textWrappingToTheTopOfTheNextColumnContinuesTheParagraph() { + List preceding = List.of(token("]", 280.0, 700.0, 7)); + TaggingTokenCluster cluster = clusterOf(token("Because", 320.0, 90.0, 8)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + @Test + public void testIsNewParagraph_acrossAPageBreakContinuesTheParagraph() { + List preceding = List.of(pageToken("]", 280.0, 700.0, 7, 4)); + TaggingTokenCluster cluster = clusterOf(pageToken("Because", 72.0, 90.0, 8, 5)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + @Test + public void testIsNewParagraph_unpositionedTrailingSpaceIsSkipped() { + LayoutToken blank = new LayoutToken(); + blank.setText(" "); + blank.setX(-1.0); + blank.setY(-1.0); + List preceding = List.of(token("]", 300.0, 400.0, 7), blank); + TaggingTokenCluster cluster = clusterOf(token("Because", 310.0, 400.0, 7)); + + assertThat( + TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), + is(false)); + } + + private static LayoutToken token(String text, double x, double y, int blockPtr) { + return pageToken(text, x, y, blockPtr, 1); + } + + private static LayoutToken pageToken(String text, double x, double y, int blockPtr, int page) { + LayoutToken t = new LayoutToken(); + t.setText(text); + t.setX(x); + t.setY(y); + t.setHeight(10.0); + t.setBlockPtr(blockPtr); + t.setPage(page); + return t; + } + + private static TaggingTokenCluster clusterOf(LayoutToken... tokens) { + TaggingTokenCluster cluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); + cluster.addLabeledTokensContainer( + new LabeledTokensContainer( + List.of(tokens), tokens[0].getText(), TaggingLabels.PARAGRAPH, true)); + return cluster; + } } From ddaf2228b188d0e952cd633775524e9ea152c1df Mon Sep 17 00:00:00 2001 From: Luca Foppiano Date: Wed, 22 Jul 2026 23:27:55 +0200 Subject: [PATCH 3/5] feat: attempt to fix this bug without introducing more regressions Signed-off-by: Luca Foppiano --- .../grobid/core/document/TEIFormatter.java | 132 +++++++++--------- 1 file changed, 69 insertions(+), 63 deletions(-) diff --git a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java index b744850e9a..976ef1e44f 100755 --- a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java +++ b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java @@ -2157,20 +2157,25 @@ private static Element generateNoteRef( * Decide whether the current cluster opens a new paragraph, or continues the one * interrupted by the preceding cluster. *

- * Only the marker/figure/table case is interesting: any other preceding label always - * starts a new paragraph. A citation, figure or table callout sits inside a - * paragraph far more often than it ends one, so the default after a callout is to - * continue. + * Any preceding label other than a marker, figure or table always starts a new + * paragraph. Those three can sit inside a paragraph (a citation callout, an + * inline figure/table reference), so after one the decision is made from the layout of + * the text that resumes, relative to the text before the interruption. *

- * The sequence label cannot make this decision. A callout interrupts the labelled - * sequence, so the resumed text is always a fresh {@code I-} - it carries - * "beginning of sequence" whether it continues the interrupted paragraph or starts a new - * one, and keying off it splits at nearly every callout (issue #1482). The signal has to - * come from the layout instead: text resuming on the same line as the callout is bound - * to it, and a genuine paragraph break additionally shows up as a new block. + * The sequence label cannot make this decision: an interruption starts a fresh labelled + * sequence, so the resumed text always looks like a "beginning" whether it continues the + * paragraph or not, and keying off that splits at nearly every callout (issue #1482). + * The layout is read as a typesetter would: the resumed text opens a new paragraph only + * when it starts a new line, that line sits below with more vertical space than + * the paragraph's own line spacing, it is aligned to the column's left edge (an offset + * or centred line is a display equation, not a paragraph), and it reads like the start + * of a sentence. Anything short of that - same line, wrapped to a new column, a plain + * wrapped line, an offset line - continues the paragraph, so an unclear layout degrades + * to the 0.9.0 behaviour of never splitting after a marker rather than to a new failure + * mode. * * @param precedingTokens the tokens accumulated in the current paragraph, whose last - * element is the callout the current cluster follows. When null + * positioned element is where the interruption happened. When null * or empty there is no layout evidence, and the paragraph is * continued. */ @@ -2183,83 +2188,73 @@ public static boolean isNewParagraph( return true; } - if (!MARKER_LABELS.contains(lastClusterLabel) - && lastClusterLabel != TaggingLabels.FIGURE - && lastClusterLabel != TaggingLabels.TABLE) { + boolean interrupting = MARKER_LABELS.contains(lastClusterLabel) + || lastClusterLabel == TaggingLabels.FIGURE + || lastClusterLabel == TaggingLabels.TABLE; + if (!interrupting) { return true; } - if (MARKER_LABELS.contains(lastClusterLabel)) { - return startsNewParagraphAfterCallout(precedingTokens, currentCluster); - } - - return false; - } - - /** - * Backward-compatible overload with no layout information: reproduces the 0.9.0 behaviour - * of never splitting after a marker, figure or table. Used where no callout context is - * available (e.g. {@link org.grobid.core.data.Table}). - */ - public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { - return isNewParagraph(lastClusterLabel, curParagraph, null, null); - } - - /** - * Layout test for text following a callout. Returns true only on positive evidence of a - * paragraph break, so that an unknown layout continues the paragraph as before #1482. - */ - private static boolean startsNewParagraphAfterCallout( - List precedingTokens, - TaggingTokenCluster currentCluster) { + // From here the preceding cluster interrupted a paragraph; decide from the layout. if (CollectionUtils.isEmpty(precedingTokens) || currentCluster == null) { return false; } - - LayoutToken callout = lastPositionedToken(precedingTokens); + LayoutToken interrupted = lastPositionedToken(precedingTokens); LayoutToken resumed = firstPositionedToken(currentCluster.concatTokens()); - if (callout == null || resumed == null) { + if (interrupted == null || resumed == null) { return false; } - // Across a page break the coordinates are not comparable. A paragraph does run over - // a page boundary, so this is not evidence of a break either way - continue. - if (callout.getPage() != resumed.getPage()) { + // Across a page break the coordinates are not comparable, and a paragraph does run + // over a page boundary, so this is not evidence of a break either way. + if (interrupted.getPage() != resumed.getPage()) { return false; } - // Text resuming on the same line as the callout is bound to it. This is both the - // most common case and the one that needs no further evidence. Note that - // LayoutToken.isNewLineAfter() cannot be used here: it is only populated by - // enrichWithNewLineInfo(), which the fulltext path never calls, so it is always - // false on these tokens. - double sameLineTolerance = Math.max(1.0, callout.getHeight() / 2.0); - if (Math.abs(resumed.getY() - callout.getY()) <= sameLineTolerance) { + // Same line as the interruption: the resumed text is bound to it. The dominant case + // (a callout inside a line of running text). Note LayoutToken.isNewLineAfter() cannot + // be used to detect the line change: it is only populated by enrichWithNewLineInfo(), + // which the fulltext path never calls, so it is always false here - the new line is + // read from the Y coordinate instead. + double verticalGap = resumed.getY() - interrupted.getY(); + double sameLineTolerance = Math.max(1.0, interrupted.getHeight() / 2.0); + if (Math.abs(verticalGap) <= sameLineTolerance) { return false; } - // Text that resumes higher up the page has wrapped to the top of the next column. - // That is a column break, not a paragraph break. - if (resumed.getY() < callout.getY()) { + // Resumed higher up the page: it wrapped to the top of the next column. A column + // break, not a paragraph break. + if (verticalGap < 0) { return false; } - // A line break alone proves nothing: a paragraph interrupted by a callout near the - // right margin also wraps. A new paragraph is a new block in the layout, so require - // that. Blocks are the unit pdfalto groups paragraphs into, which makes a block - // change the closest thing to a paragraph boundary the layout offers. - if (callout.getBlockPtr() == resumed.getBlockPtr()) { + // The resumed line must be aligned to the column's left edge. Text of a new paragraph + // starts at the left margin (or indents by about one em); a display equation is set in + // much further, and is otherwise indistinguishable - it starts a new line below with + // the same extra vertical space. The horizontal position is what tells them apart. The + // tolerance is one em (the resumed text's own font size), so it scales with the + // document rather than assuming a fixed point size. + double leftEdge = leftEdge(precedingTokens); + double oneEm = resumed.getFontSize() > 0 ? resumed.getFontSize() : resumed.getHeight(); + if (leftEdge >= 0 && resumed.getX() - leftEdge > oneEm) { return false; } - // The block signal on its own splits about as often wrongly as rightly: a block - // boundary also falls inside a paragraph. A paragraph that genuinely starts here - // also reads like one, so require the resumed text to open the way a sentence does. - // This is what rejects the continuations that resume on lower case and the fragments - // that begin with the punctuation closing the previous sentence. + // Finally, a paragraph reads like one: it opens with a capital or a digit. This + // rejects continuations resuming on lower case, and the punctuation closing the + // interrupted sentence. return startsLikeASentence(resumed.getText()); } + /** + * Backward-compatible overload with no layout information: reproduces the 0.9.0 behaviour + * of never splitting after a marker, figure or table. Used where no callout context is + * available (e.g. {@link org.grobid.core.data.Table}). + */ + public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { + return isNewParagraph(lastClusterLabel, curParagraph, null, null); + } + private static boolean startsLikeASentence(String text) { if (StringUtils.isBlank(text)) { return false; @@ -2268,6 +2263,17 @@ private static boolean startsLikeASentence(String text) { return Character.isUpperCase(first) || Character.isDigit(first); } + /** Left edge (minimum X) of the paragraph's positioned tokens, or -1 if unknown. */ + private static double leftEdge(List tokens) { + double min = Double.MAX_VALUE; + for (LayoutToken t : tokens) { + if (isPositioned(t) && t.getX() < min) { + min = t.getX(); + } + } + return min == Double.MAX_VALUE ? -1 : min; + } + /** * The last token carrying layout coordinates. Whitespace tokens are synthesised without * position (x = y = -1), and the token immediately before a cluster boundary is very From 4bcc3f6e24548e06edacf101c45ed39d0ee2ad74 Mon Sep 17 00:00:00 2001 From: Luca Foppiano Date: Wed, 22 Jul 2026 22:30:09 +0100 Subject: [PATCH 4/5] Revert "feat: attempt to fix this bug without introducing more regressions" This reverts commit ddaf2228b188d0e952cd633775524e9ea152c1df. --- .../grobid/core/document/TEIFormatter.java | 132 +++++++++--------- 1 file changed, 63 insertions(+), 69 deletions(-) diff --git a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java index 976ef1e44f..b744850e9a 100755 --- a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java +++ b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java @@ -2157,25 +2157,20 @@ private static Element generateNoteRef( * Decide whether the current cluster opens a new paragraph, or continues the one * interrupted by the preceding cluster. *

- * Any preceding label other than a marker, figure or table always starts a new - * paragraph. Those three can sit inside a paragraph (a citation callout, an - * inline figure/table reference), so after one the decision is made from the layout of - * the text that resumes, relative to the text before the interruption. + * Only the marker/figure/table case is interesting: any other preceding label always + * starts a new paragraph. A citation, figure or table callout sits inside a + * paragraph far more often than it ends one, so the default after a callout is to + * continue. *

- * The sequence label cannot make this decision: an interruption starts a fresh labelled - * sequence, so the resumed text always looks like a "beginning" whether it continues the - * paragraph or not, and keying off that splits at nearly every callout (issue #1482). - * The layout is read as a typesetter would: the resumed text opens a new paragraph only - * when it starts a new line, that line sits below with more vertical space than - * the paragraph's own line spacing, it is aligned to the column's left edge (an offset - * or centred line is a display equation, not a paragraph), and it reads like the start - * of a sentence. Anything short of that - same line, wrapped to a new column, a plain - * wrapped line, an offset line - continues the paragraph, so an unclear layout degrades - * to the 0.9.0 behaviour of never splitting after a marker rather than to a new failure - * mode. + * The sequence label cannot make this decision. A callout interrupts the labelled + * sequence, so the resumed text is always a fresh {@code I-} - it carries + * "beginning of sequence" whether it continues the interrupted paragraph or starts a new + * one, and keying off it splits at nearly every callout (issue #1482). The signal has to + * come from the layout instead: text resuming on the same line as the callout is bound + * to it, and a genuine paragraph break additionally shows up as a new block. * * @param precedingTokens the tokens accumulated in the current paragraph, whose last - * positioned element is where the interruption happened. When null + * element is the callout the current cluster follows. When null * or empty there is no layout evidence, and the paragraph is * continued. */ @@ -2188,73 +2183,83 @@ public static boolean isNewParagraph( return true; } - boolean interrupting = MARKER_LABELS.contains(lastClusterLabel) - || lastClusterLabel == TaggingLabels.FIGURE - || lastClusterLabel == TaggingLabels.TABLE; - if (!interrupting) { + if (!MARKER_LABELS.contains(lastClusterLabel) + && lastClusterLabel != TaggingLabels.FIGURE + && lastClusterLabel != TaggingLabels.TABLE) { return true; } - // From here the preceding cluster interrupted a paragraph; decide from the layout. + if (MARKER_LABELS.contains(lastClusterLabel)) { + return startsNewParagraphAfterCallout(precedingTokens, currentCluster); + } + + return false; + } + + /** + * Backward-compatible overload with no layout information: reproduces the 0.9.0 behaviour + * of never splitting after a marker, figure or table. Used where no callout context is + * available (e.g. {@link org.grobid.core.data.Table}). + */ + public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { + return isNewParagraph(lastClusterLabel, curParagraph, null, null); + } + + /** + * Layout test for text following a callout. Returns true only on positive evidence of a + * paragraph break, so that an unknown layout continues the paragraph as before #1482. + */ + private static boolean startsNewParagraphAfterCallout( + List precedingTokens, + TaggingTokenCluster currentCluster) { if (CollectionUtils.isEmpty(precedingTokens) || currentCluster == null) { return false; } - LayoutToken interrupted = lastPositionedToken(precedingTokens); + + LayoutToken callout = lastPositionedToken(precedingTokens); LayoutToken resumed = firstPositionedToken(currentCluster.concatTokens()); - if (interrupted == null || resumed == null) { + if (callout == null || resumed == null) { return false; } - // Across a page break the coordinates are not comparable, and a paragraph does run - // over a page boundary, so this is not evidence of a break either way. - if (interrupted.getPage() != resumed.getPage()) { + // Across a page break the coordinates are not comparable. A paragraph does run over + // a page boundary, so this is not evidence of a break either way - continue. + if (callout.getPage() != resumed.getPage()) { return false; } - // Same line as the interruption: the resumed text is bound to it. The dominant case - // (a callout inside a line of running text). Note LayoutToken.isNewLineAfter() cannot - // be used to detect the line change: it is only populated by enrichWithNewLineInfo(), - // which the fulltext path never calls, so it is always false here - the new line is - // read from the Y coordinate instead. - double verticalGap = resumed.getY() - interrupted.getY(); - double sameLineTolerance = Math.max(1.0, interrupted.getHeight() / 2.0); - if (Math.abs(verticalGap) <= sameLineTolerance) { + // Text resuming on the same line as the callout is bound to it. This is both the + // most common case and the one that needs no further evidence. Note that + // LayoutToken.isNewLineAfter() cannot be used here: it is only populated by + // enrichWithNewLineInfo(), which the fulltext path never calls, so it is always + // false on these tokens. + double sameLineTolerance = Math.max(1.0, callout.getHeight() / 2.0); + if (Math.abs(resumed.getY() - callout.getY()) <= sameLineTolerance) { return false; } - // Resumed higher up the page: it wrapped to the top of the next column. A column - // break, not a paragraph break. - if (verticalGap < 0) { + // Text that resumes higher up the page has wrapped to the top of the next column. + // That is a column break, not a paragraph break. + if (resumed.getY() < callout.getY()) { return false; } - // The resumed line must be aligned to the column's left edge. Text of a new paragraph - // starts at the left margin (or indents by about one em); a display equation is set in - // much further, and is otherwise indistinguishable - it starts a new line below with - // the same extra vertical space. The horizontal position is what tells them apart. The - // tolerance is one em (the resumed text's own font size), so it scales with the - // document rather than assuming a fixed point size. - double leftEdge = leftEdge(precedingTokens); - double oneEm = resumed.getFontSize() > 0 ? resumed.getFontSize() : resumed.getHeight(); - if (leftEdge >= 0 && resumed.getX() - leftEdge > oneEm) { + // A line break alone proves nothing: a paragraph interrupted by a callout near the + // right margin also wraps. A new paragraph is a new block in the layout, so require + // that. Blocks are the unit pdfalto groups paragraphs into, which makes a block + // change the closest thing to a paragraph boundary the layout offers. + if (callout.getBlockPtr() == resumed.getBlockPtr()) { return false; } - // Finally, a paragraph reads like one: it opens with a capital or a digit. This - // rejects continuations resuming on lower case, and the punctuation closing the - // interrupted sentence. + // The block signal on its own splits about as often wrongly as rightly: a block + // boundary also falls inside a paragraph. A paragraph that genuinely starts here + // also reads like one, so require the resumed text to open the way a sentence does. + // This is what rejects the continuations that resume on lower case and the fragments + // that begin with the punctuation closing the previous sentence. return startsLikeASentence(resumed.getText()); } - /** - * Backward-compatible overload with no layout information: reproduces the 0.9.0 behaviour - * of never splitting after a marker, figure or table. Used where no callout context is - * available (e.g. {@link org.grobid.core.data.Table}). - */ - public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { - return isNewParagraph(lastClusterLabel, curParagraph, null, null); - } - private static boolean startsLikeASentence(String text) { if (StringUtils.isBlank(text)) { return false; @@ -2263,17 +2268,6 @@ private static boolean startsLikeASentence(String text) { return Character.isUpperCase(first) || Character.isDigit(first); } - /** Left edge (minimum X) of the paragraph's positioned tokens, or -1 if unknown. */ - private static double leftEdge(List tokens) { - double min = Double.MAX_VALUE; - for (LayoutToken t : tokens) { - if (isPositioned(t) && t.getX() < min) { - min = t.getX(); - } - } - return min == Double.MAX_VALUE ? -1 : min; - } - /** * The last token carrying layout coordinates. Whitespace tokens are synthesised without * position (x = y = -1), and the token immediately before a cluster boundary is very From e4ac23723eb4d057c5ec6c607f4285bc6a3f3599 Mon Sep 17 00:00:00 2001 From: Luca Foppiano Date: Wed, 22 Jul 2026 22:30:27 +0100 Subject: [PATCH 5/5] Revert "fix: split paragraphs after a callout on layout, not the sequence label" This reverts commit 350d3b8354b04d214b7f82c4044928930fa69ce6. --- .../grobid/core/document/TEIFormatter.java | 152 +--------------- .../core/document/TEIFormatterTest.java | 163 ------------------ 2 files changed, 4 insertions(+), 311 deletions(-) diff --git a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java index b744850e9a..c493a1bd12 100755 --- a/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java +++ b/grobid-core/src/main/java/org/grobid/core/document/TEIFormatter.java @@ -1800,7 +1800,7 @@ public StringBuilder toTEITextPiece( if (CollectionUtils.isEmpty(matchedLabelPositions)) { String clusterContent = LayoutTokensUtil.normalizeDehyphenizeText(clusterTokens); - if (isNewParagraph(lastClusterLabel, curParagraph, cluster, curParagraphTokens)) { + if (isNewParagraph(lastClusterLabel, curParagraph)) { if (curParagraph != null && config.isWithSentenceSegmentation()) { segmentIntoSentences(curParagraph, curParagraphTokens, config, doc.getLanguage()); } @@ -1831,7 +1831,7 @@ public StringBuilder toTEITextPiece( curParagraph.appendChild(clusterContent); curParagraphTokens.addAll(clusterTokens); } else { - if (isNewParagraph(lastClusterLabel, curParagraph, cluster, curParagraphTokens)) { + if (isNewParagraph(lastClusterLabel, curParagraph)) { if (curParagraph != null && config.isWithSentenceSegmentation()) { segmentIntoSentences( curParagraph, @@ -2153,153 +2153,9 @@ private static Element generateNoteRef( return ref; } - /** - * Decide whether the current cluster opens a new paragraph, or continues the one - * interrupted by the preceding cluster. - *

- * Only the marker/figure/table case is interesting: any other preceding label always - * starts a new paragraph. A citation, figure or table callout sits inside a - * paragraph far more often than it ends one, so the default after a callout is to - * continue. - *

- * The sequence label cannot make this decision. A callout interrupts the labelled - * sequence, so the resumed text is always a fresh {@code I-} - it carries - * "beginning of sequence" whether it continues the interrupted paragraph or starts a new - * one, and keying off it splits at nearly every callout (issue #1482). The signal has to - * come from the layout instead: text resuming on the same line as the callout is bound - * to it, and a genuine paragraph break additionally shows up as a new block. - * - * @param precedingTokens the tokens accumulated in the current paragraph, whose last - * element is the callout the current cluster follows. When null - * or empty there is no layout evidence, and the paragraph is - * continued. - */ - public static boolean isNewParagraph( - TaggingLabel lastClusterLabel, - Element curParagraph, - TaggingTokenCluster currentCluster, - List precedingTokens) { - if (curParagraph == null) { - return true; - } - - if (!MARKER_LABELS.contains(lastClusterLabel) - && lastClusterLabel != TaggingLabels.FIGURE - && lastClusterLabel != TaggingLabels.TABLE) { - return true; - } - - if (MARKER_LABELS.contains(lastClusterLabel)) { - return startsNewParagraphAfterCallout(precedingTokens, currentCluster); - } - - return false; - } - - /** - * Backward-compatible overload with no layout information: reproduces the 0.9.0 behaviour - * of never splitting after a marker, figure or table. Used where no callout context is - * available (e.g. {@link org.grobid.core.data.Table}). - */ public static boolean isNewParagraph(TaggingLabel lastClusterLabel, Element curParagraph) { - return isNewParagraph(lastClusterLabel, curParagraph, null, null); - } - - /** - * Layout test for text following a callout. Returns true only on positive evidence of a - * paragraph break, so that an unknown layout continues the paragraph as before #1482. - */ - private static boolean startsNewParagraphAfterCallout( - List precedingTokens, - TaggingTokenCluster currentCluster) { - if (CollectionUtils.isEmpty(precedingTokens) || currentCluster == null) { - return false; - } - - LayoutToken callout = lastPositionedToken(precedingTokens); - LayoutToken resumed = firstPositionedToken(currentCluster.concatTokens()); - if (callout == null || resumed == null) { - return false; - } - - // Across a page break the coordinates are not comparable. A paragraph does run over - // a page boundary, so this is not evidence of a break either way - continue. - if (callout.getPage() != resumed.getPage()) { - return false; - } - - // Text resuming on the same line as the callout is bound to it. This is both the - // most common case and the one that needs no further evidence. Note that - // LayoutToken.isNewLineAfter() cannot be used here: it is only populated by - // enrichWithNewLineInfo(), which the fulltext path never calls, so it is always - // false on these tokens. - double sameLineTolerance = Math.max(1.0, callout.getHeight() / 2.0); - if (Math.abs(resumed.getY() - callout.getY()) <= sameLineTolerance) { - return false; - } - - // Text that resumes higher up the page has wrapped to the top of the next column. - // That is a column break, not a paragraph break. - if (resumed.getY() < callout.getY()) { - return false; - } - - // A line break alone proves nothing: a paragraph interrupted by a callout near the - // right margin also wraps. A new paragraph is a new block in the layout, so require - // that. Blocks are the unit pdfalto groups paragraphs into, which makes a block - // change the closest thing to a paragraph boundary the layout offers. - if (callout.getBlockPtr() == resumed.getBlockPtr()) { - return false; - } - - // The block signal on its own splits about as often wrongly as rightly: a block - // boundary also falls inside a paragraph. A paragraph that genuinely starts here - // also reads like one, so require the resumed text to open the way a sentence does. - // This is what rejects the continuations that resume on lower case and the fragments - // that begin with the punctuation closing the previous sentence. - return startsLikeASentence(resumed.getText()); - } - - private static boolean startsLikeASentence(String text) { - if (StringUtils.isBlank(text)) { - return false; - } - char first = text.charAt(0); - return Character.isUpperCase(first) || Character.isDigit(first); - } - - /** - * The last token carrying layout coordinates. Whitespace tokens are synthesised without - * position (x = y = -1), and the token immediately before a cluster boundary is very - * often one of those, so taking the last token blindly compares against a token that has - * no place on the page. - */ - private static LayoutToken lastPositionedToken(List tokens) { - for (int i = tokens.size() - 1; i >= 0; i--) { - if (isPositioned(tokens.get(i))) { - return tokens.get(i); - } - } - return null; - } - - private static LayoutToken firstPositionedToken(List tokens) { - if (CollectionUtils.isEmpty(tokens)) { - return null; - } - for (LayoutToken token : tokens) { - if (isPositioned(token)) { - return token; - } - } - return null; - } - - private static boolean isPositioned(LayoutToken token) { - return token != null - && StringUtils.isNotBlank(token.getText()) - && token.getX() >= 0 - && token.getY() >= 0; + return (!MARKER_LABELS.contains(lastClusterLabel) && lastClusterLabel != TaggingLabels.FIGURE + && lastClusterLabel != TaggingLabels.TABLE) || curParagraph == null; } public void segmentIntoSentences( diff --git a/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java b/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java index 72523162d3..c3b4ec4cc4 100644 --- a/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java +++ b/grobid-core/src/test/java/org/grobid/core/document/TEIFormatterTest.java @@ -32,10 +32,7 @@ import org.grobid.core.data.Figure; import org.grobid.core.data.Note; import org.grobid.core.data.Table; -import org.grobid.core.engines.label.TaggingLabels; import org.grobid.core.layout.LayoutToken; -import org.grobid.core.tokenization.LabeledTokensContainer; -import org.grobid.core.tokenization.TaggingTokenCluster; import org.grobid.core.utilities.GrobidProperties; import org.grobid.core.utilities.LayoutTokensUtil; @@ -426,164 +423,4 @@ public void testMarkReferencesTableTEI_truncatedRef2_referenceAtBeginning() thro assertThat(nodes.get(5).toXML(), is(" ")); } - // --- isNewParagraph: layout-based split after a callout (replaces #1482) --- - // The two #1482 scenarios are kept here as the first two cases, now asserting the - // corrected (layout-driven) contract instead of the isBeginning() one. - - @Test - public void testIsNewParagraph_afterMarkerBeginningLabel_noLayoutEvidenceContinues() { - // #1482 asserted this split because the resumed cluster carried isBeginning(). That - // flag is uninformative after a callout, so with no layout to go on the paragraph - // continues. - TaggingTokenCluster paragraphCluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); - paragraphCluster.addLabeledTokensContainer( - new LabeledTokensContainer(List.of(), "Because", TaggingLabels.PARAGRAPH, true)); - - boolean isNewParagraph = TEIFormatter - .isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), paragraphCluster, null); - - assertThat(isNewParagraph, is(false)); - } - - @Test - public void testIsNewParagraph_afterMarkerContinuationLabel_continues() { - TaggingTokenCluster paragraphCluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); - paragraphCluster.addLabeledTokensContainer( - new LabeledTokensContainer(List.of(), "continuation", TaggingLabels.PARAGRAPH, false)); - - boolean isNewParagraph = TEIFormatter - .isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), paragraphCluster, null); - - assertThat(isNewParagraph, is(false)); - } - - @Test - public void testIsNewParagraph_nonMarkerLabelAlwaysStartsANewParagraph() { - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.SECTION, new Element("p"), null, null), - is(true)); - } - - @Test - public void testIsNewParagraph_nullParagraphAlwaysStartsANewParagraph() { - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, null, null, null), - is(true)); - } - - @Test - public void testIsNewParagraph_twoArgOverloadKeeps090Behaviour() { - // The Table path and any external caller use the 2-arg overload: never split after a - // marker/figure/table, split after anything else. - assertThat(TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p")), is(false)); - assertThat(TEIFormatter.isNewParagraph(TaggingLabels.FIGURE, new Element("p")), is(false)); - assertThat(TEIFormatter.isNewParagraph(TaggingLabels.SECTION, new Element("p")), is(true)); - } - - @Test - public void testIsNewParagraph_textResumingOnTheSameLineContinuesTheParagraph() { - List preceding = List.of(token("]", 300.0, 400.0, 0)); - TaggingTokenCluster cluster = clusterOf(token("Because", 310.0, 400.0, 0)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - @Test - public void testIsNewParagraph_newLineInTheSameBlockContinuesTheParagraph() { - List preceding = List.of(token("]", 500.0, 400.0, 7)); - TaggingTokenCluster cluster = clusterOf(token("Because", 72.0, 412.0, 7)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - @Test - public void testIsNewParagraph_newBlockOnANewLineStartsANewParagraph() { - List preceding = List.of(token("]", 500.0, 400.0, 7)); - TaggingTokenCluster cluster = clusterOf(token("Because", 72.0, 412.0, 8)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(true)); - } - - @Test - public void testIsNewParagraph_newBlockResumingOnLowerCaseContinuesTheParagraph() { - List preceding = List.of(token("]", 500.0, 400.0, 7)); - TaggingTokenCluster cluster = clusterOf(token("because", 72.0, 412.0, 8)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - @Test - public void testIsNewParagraph_newBlockResumingOnPunctuationContinuesTheParagraph() { - List preceding = List.of(token("]", 500.0, 400.0, 7)); - TaggingTokenCluster cluster = clusterOf(token(".", 72.0, 412.0, 8)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - @Test - public void testIsNewParagraph_textWrappingToTheTopOfTheNextColumnContinuesTheParagraph() { - List preceding = List.of(token("]", 280.0, 700.0, 7)); - TaggingTokenCluster cluster = clusterOf(token("Because", 320.0, 90.0, 8)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - @Test - public void testIsNewParagraph_acrossAPageBreakContinuesTheParagraph() { - List preceding = List.of(pageToken("]", 280.0, 700.0, 7, 4)); - TaggingTokenCluster cluster = clusterOf(pageToken("Because", 72.0, 90.0, 8, 5)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - @Test - public void testIsNewParagraph_unpositionedTrailingSpaceIsSkipped() { - LayoutToken blank = new LayoutToken(); - blank.setText(" "); - blank.setX(-1.0); - blank.setY(-1.0); - List preceding = List.of(token("]", 300.0, 400.0, 7), blank); - TaggingTokenCluster cluster = clusterOf(token("Because", 310.0, 400.0, 7)); - - assertThat( - TEIFormatter.isNewParagraph(TaggingLabels.CITATION_MARKER, new Element("p"), cluster, preceding), - is(false)); - } - - private static LayoutToken token(String text, double x, double y, int blockPtr) { - return pageToken(text, x, y, blockPtr, 1); - } - - private static LayoutToken pageToken(String text, double x, double y, int blockPtr, int page) { - LayoutToken t = new LayoutToken(); - t.setText(text); - t.setX(x); - t.setY(y); - t.setHeight(10.0); - t.setBlockPtr(blockPtr); - t.setPage(page); - return t; - } - - private static TaggingTokenCluster clusterOf(LayoutToken... tokens) { - TaggingTokenCluster cluster = new TaggingTokenCluster(TaggingLabels.PARAGRAPH); - cluster.addLabeledTokensContainer( - new LabeledTokensContainer( - List.of(tokens), tokens[0].getText(), TaggingLabels.PARAGRAPH, true)); - return cluster; - } }