This is an automated email from the ASF dual-hosted git repository. krickert pushed a commit to branch OPENNLP-205 in repository https://gitbox.apache.org/repos/asf/opennlp.git
commit f5b2189beeea58a90cf62136531a9e00f999fefc Author: Kristian Rickert <[email protected]> AuthorDate: Thu Jul 2 07:06:19 2026 -0400 OPENNLP-205: Map end-of-sentence positions to spans through the CharClass whitespace engine Refactors the position-to-span mapping of SentenceDetectorME, open since 2011: the mapping now lives in a package-visible mapPositionsToSpans seam that is directly testable without a trained model, builds spans and probabilities in lockstep (the old pre-sized array dropped a whitespace-only candidate's probability by index, which could misalign the pairing and leave null slots), and trims with the Unicode White_Space set of CharClass.whitespace() from the normalization engine instead of StringUtil.isWhitespace. Behavior deltas, both pinned by tests: the next line control (U+0085) now counts as whitespace, so it is trimmed from span edges instead of riding along as sentence content; the U+001C..U+001F information separators, which are not Unicode White_Space, are no longer trimmed. Model feature generation (SDContextGenerator) and the break-acceptance heuristics are deliberately untouched, so existing models see exactly the features they were trained on. The characterization suite from the previous commit passes unchanged except the U+0085 expectation, updated to the corrected behavior; four new tests cover the mapping seam directly, including the whitespace-only-candidate alignment guarantee that is unreachable through the public API. --- .../tools/sentdetect/SentenceDetectorME.java | 123 +++++++++++---------- .../SentenceDetectorMESpanMappingTest.java | 66 ++++++++++- 2 files changed, 124 insertions(+), 65 deletions(-) diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/sentdetect/SentenceDetectorME.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/sentdetect/SentenceDetectorME.java index 074db9625..b4a963345 100644 --- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/sentdetect/SentenceDetectorME.java +++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/sentdetect/SentenceDetectorME.java @@ -42,6 +42,7 @@ import opennlp.tools.util.Span; import opennlp.tools.util.StringList; import opennlp.tools.util.StringUtil; import opennlp.tools.util.TrainingParameters; +import opennlp.tools.util.normalizer.CharClass; /** * A sentence detector for splitting up raw text into sentences. @@ -185,14 +186,20 @@ public class SentenceDetectorME implements SentenceDetector, Probabilistic { return sentences; } + // The whitespace definition of the end-of-sentence position mapping: the Unicode White_Space + // set (OPENNLP-205). Unlike the previously used StringUtil.isWhitespace, this covers the next + // line control (U+0085) and does not treat the U+001C..U+001F information separators as + // whitespace. + private static final CharClass WHITESPACE = CharClass.whitespace(); + private int getFirstWS(CharSequence s, int pos) { - while (pos < s.length() && !StringUtil.isWhitespace(s.charAt(pos))) + while (pos < s.length() && !WHITESPACE.contains(s.charAt(pos))) pos++; return pos; } private int getFirstNonWS(CharSequence s, int pos) { - while (pos < s.length() && StringUtil.isWhitespace(s.charAt(pos))) + while (pos < s.length() && WHITESPACE.contains(s.charAt(pos))) pos++; return pos; } @@ -244,77 +251,75 @@ public class SentenceDetectorME implements SentenceDetector, Probabilistic { } int[] starts = ArrayMath.toIntArray(positions); + Span[] spans = mapPositionsToSpans(s, starts, localProbs); - // string does not contain sentence end positions - if (starts.length == 0) { - - // remove leading and trailing whitespace - int start = 0; - int end = s.length(); - - while (start < s.length() && StringUtil.isWhitespace(s.charAt(start))) - start++; + // Publish for backward-compatible probs() access (last-writer-wins under concurrency) + state.sentProbs = localProbs; - while (end > 0 && StringUtil.isWhitespace(s.charAt(end - 1))) - end--; + return spans; + } - if (end - start > 0) { - localProbs.add(1d); - state.sentProbs = localProbs; - return new Span[] {new Span(start, end)}; - } - else { - state.sentProbs = localProbs; + /** + * Maps accepted sentence-start positions to trimmed sentence {@link Span}s, the core of the + * end-of-sentence position to span mapping (OPENNLP-205). Package-visible so the mapping is + * directly testable without a trained model. + * + * <p>Each span runs from the previous position (or the text start) to the next position, with + * Unicode {@code White_Space} trimmed from both edges. A candidate that is whitespace-only is + * dropped together with its probability, so {@code probs} and the returned spans always stay + * aligned; text after the last position becomes a final span with probability {@code 1.0}. + * With no positions at all, the whole text is one trimmed span, or no span when it is + * blank.</p> + * + * @param s The text the positions refer to. + * @param starts The accepted sentence-start positions, ascending. + * @param probs The probability per position; mutated so it mirrors the returned spans. + * @return The trimmed spans, with each probability attached, in order. + */ + static Span[] mapPositionsToSpans(CharSequence s, int[] starts, List<Double> probs) { + // The string does not contain sentence end positions. + if (starts.length == 0) { + Span whole = trimmedSpan(s, 0, s.length()); + if (whole == null) { return new Span[0]; } + probs.add(1d); + return new Span[] {whole}; } - // Convert the sentence end indexes to spans - - boolean leftover = starts[starts.length - 1] != s.length(); - Span[] spans = new Span[leftover ? starts.length + 1 : starts.length]; - + final List<Span> spans = new ArrayList<>(starts.length + 1); + final List<Double> keptProbs = new ArrayList<>(starts.length + 1); for (int si = 0; si < starts.length; si++) { - int start; - - if (si == 0) { - start = 0; - } - else { - start = starts[si - 1]; - } - - // A span might contain only white spaces, in this case the length of - // the span will be zero after trimming and should be ignored. - Span span = new Span(start, starts[si]).trim(s); - if (span.length() > 0) { - spans[si] = span; - } - else { - localProbs.remove(si); + // A candidate might contain only whitespace; it is dropped together with its probability, + // which keeps the spans and the probabilities aligned by construction. + Span span = trimmedSpan(s, si == 0 ? 0 : starts[si - 1], starts[si]); + if (span != null) { + spans.add(new Span(span, probs.get(si))); + keptProbs.add(probs.get(si)); } } - - if (leftover) { - Span span = new Span(starts[starts.length - 1], s.length()).trim(s); - if (span.length() > 0) { - spans[spans.length - 1] = span; - localProbs.add(1d); + if (starts[starts.length - 1] != s.length()) { + Span span = trimmedSpan(s, starts[starts.length - 1], s.length()); + if (span != null) { + spans.add(new Span(span, 1d)); + keptProbs.add(1d); } } - /* - * set the prob for each span - */ - for (int i = 0; i < spans.length; i++) { - double prob = localProbs.get(i); - spans[i] = new Span(spans[i], prob); + probs.clear(); + probs.addAll(keptProbs); + return spans.toArray(new Span[0]); + } + // Returns [start, end) with Unicode White_Space trimmed from both edges, or null when nothing + // remains. + private static Span trimmedSpan(CharSequence s, int start, int end) { + while (start < end && WHITESPACE.contains(s.charAt(start))) { + start++; } - - // Publish for backward-compatible probs() access (last-writer-wins under concurrency) - state.sentProbs = localProbs; - - return spans; + while (end > start && WHITESPACE.contains(s.charAt(end - 1))) { + end--; + } + return end > start ? new Span(start, end) : null; } /** diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/sentdetect/SentenceDetectorMESpanMappingTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/sentdetect/SentenceDetectorMESpanMappingTest.java index 8db313b23..73d925404 100644 --- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/sentdetect/SentenceDetectorMESpanMappingTest.java +++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/sentdetect/SentenceDetectorMESpanMappingTest.java @@ -121,13 +121,13 @@ public class SentenceDetectorMESpanMappingTest extends AbstractSentenceDetectorT } @Test - void nextLineControlBetweenSpacedSentences() { - // U+0085 NEL is Unicode White_Space. The pre-refactoring mapping did not treat it as - // whitespace (StringUtil.isWhitespace misses it), so the second span starts at the NEL - // and carries it as leading content instead of starting at the word. The OPENNLP-205 - // refactoring moves this span start onto the word. + void nextLineControlBetweenSpacedSentencesIsTrimmed() { + // U+0085 NEL is Unicode White_Space. The pre-refactoring mapping missed it + // (StringUtil.isWhitespace does not cover it), so the second span used to start at the NEL + // and carry it as leading content; with the mapping on the CharClass Unicode White_Space set + // (OPENNLP-205), the span starts at the word. String input = "This is a test. \u0085 " + SECOND; - assertSpans(tokenEnd, input, new Span(0, 15), new Span(16, 59)); + assertSpans(tokenEnd, input, new Span(0, 15), new Span(18, 59)); } @Test @@ -138,4 +138,58 @@ public class SentenceDetectorMESpanMappingTest extends AbstractSentenceDetectorT String input = "This is a test.\u0085" + SECOND; assertSpans(tokenEnd, input, new Span(0, 57)); } + + @Test + void mapPositionsToSpansHandlesNoPositions() { + java.util.List<Double> probs = new java.util.ArrayList<>(); + Assertions.assertEquals(0, + SentenceDetectorME.mapPositionsToSpans(" \t ", new int[0], probs).length); + Assertions.assertTrue(probs.isEmpty()); + Span[] whole = SentenceDetectorME.mapPositionsToSpans(" some text ", new int[0], probs); + Assertions.assertEquals(1, whole.length); + Assertions.assertEquals(new Span(2, 11).getStart(), whole[0].getStart()); + Assertions.assertEquals(new Span(2, 11).getEnd(), whole[0].getEnd()); + Assertions.assertEquals(java.util.List.of(1d), probs); + } + + @Test + void mapPositionsToSpansDropsAWhitespaceOnlyCandidateWithItsProbability() { + // The whitespace-only-candidate branch is not reachable through sentPosDetect's position + // invariants, but the mapping guarantees alignment structurally: the span and its + // probability are dropped together. The old implementation removed the probability by index + // from a pre-sized array, which could misalign the pairing. + java.util.List<Double> probs = new java.util.ArrayList<>(java.util.List.of(0.9d, 0.8d)); + Span[] spans = SentenceDetectorME.mapPositionsToSpans("ab cd", new int[] {4, 4}, probs); + Assertions.assertEquals(2, spans.length); + Assertions.assertEquals(0, spans[0].getStart()); + Assertions.assertEquals(2, spans[0].getEnd()); + Assertions.assertEquals(0.9d, spans[0].getProb()); + Assertions.assertEquals(4, spans[1].getStart()); + Assertions.assertEquals(6, spans[1].getEnd()); + Assertions.assertEquals(1d, spans[1].getProb()); + Assertions.assertEquals(java.util.List.of(0.9d, 1d), probs); + } + + @Test + void mapPositionsToSpansTrimsTheFullUnicodeWhitespaceSet() { + java.util.List<Double> probs = new java.util.ArrayList<>(java.util.List.of(0.7d)); + Span[] spans = SentenceDetectorME.mapPositionsToSpans( + "One.\u0085\u00A0\u2028Two", new int[] {7}, probs); + Assertions.assertEquals(2, spans.length); + Assertions.assertEquals(0, spans[0].getStart()); + Assertions.assertEquals(4, spans[0].getEnd()); + Assertions.assertEquals(7, spans[1].getStart()); + Assertions.assertEquals(10, spans[1].getEnd()); + } + + @Test + void informationSeparatorsAreContentNotWhitespace() { + // Deliberate delta from the old StringUtil-based mapping: the U+001C..U+001F information + // separators are not Unicode White_Space, so they are no longer trimmed from span edges. + java.util.List<Double> probs = new java.util.ArrayList<>(java.util.List.of(0.7d)); + Span[] spans = SentenceDetectorME.mapPositionsToSpans("A.\u001C B", new int[] {4}, probs); + Assertions.assertEquals(2, spans.length); + Assertions.assertEquals(0, spans[0].getStart()); + Assertions.assertEquals(3, spans[0].getEnd()); // includes the separator control + } }
