OPENNLP-1861: Offset/alignment layer — Alignment, AlignedText, buildAligned (1b/7) by krickert · Pull Request #1109 · apache/opennlp

krickert · 2026-06-23T14:03:57Z

Part 1b of the OPENNLP-1850 stack: the offset/alignment layer, split out of the former foundation PR (#1103) on review request so the conceptually hard part gets a focused read on its own. Builds on the engine in #1108.

Adds:

Alignment (bidirectional edit-sequence, andThen-composable) and AlignedText
the OffsetAwareNormalizer capability interface and TextNormalizer.buildAligned()
the *Aligned CharClass variants, the offset-aware rungs, and the line-break-preserving rung
the dense span-mapping tests: binary-search span mapping, expansion/deletion edge cases, and andThen composition including the insertion-inside-an-expansion case

Base: OPENNLP-1850-1a-engine (#1108). Stack: 1a → 1b (this) → 2a → 2b → 2c → DL → docs.

rzo1 · 2026-06-25T12:08:09Z

OffsetAwareNormalizer javadoc @links opennlp.tools.namefind.OffsetMappingNameFinder, which isn't introduced until OPENNLP-1865: Offset-safe input normalization in the DL components (3-dl/7) #1105. Harmless under doclint=none, but it's a dangling link if this PR is read or merged on its own.
LineBreakPreservingWhitespaceCharSequenceNormalizer javadoc still opens "A CharSequenceNormalizer" though the class now implements OffsetAwareNormalizer.

krickert · 2026-06-25T18:13:04Z

@rzo1 Both fixed (current tip 702acc52).

Dangling forward-link. The OffsetAwareNormalizer javadoc no longer hard-links the DL-layer type — it now reads {@code OffsetMappingNameFinder} (in the DL layer) instead of {@link …}, so the reference is accurate without creating a cross-PR dangling link when 1b is read or merged on its own.

LineBreakPreserving… opener. Reworded from "A CharSequenceNormalizer" to "An OffsetAwareNormalizer", matching what the class now implements.

mawiesne · 2026-06-30T06:11:50Z

@krickert Can you quickly rebase - we have changes in asf.yml in main now, missing on this branch.

krickert · 2026-06-30T12:56:58Z

@mawiesne Rebased onto current main as part of the full-stack rebase (see #1108). New tip c73ebeb7.

mawiesne

Thx @krickert and team. There some changes required with this PR. We need to change the signature of various public API methods to handle exceptional cases differently. Moreover, we need to be better in testing new classes. Please check my comments. If those are in, we can consider this PR for merging.

mawiesne · 2026-07-02T05:51:23Z

+ * alignments compose with {@link #andThen(Alignment)}, which is what lets a multi-stage
+ * normalization pipeline still map a result all the way back to the original.</p>
+ *
+ * <p>Instances are immutable and thread-safe; build one with {@link Builder}.</p>


Please add the annotation @ThreadSafe to the class here.

mawiesne · 2026-07-02T05:53:16Z

+     */
+    public Builder equal(int charCount) {
+      if (charCount < 0) {
+        throw new IllegalArgumentException("charCount must not be negative: " + charCount);


Please add IllegalArgumentException to Javadoc here.

mawiesne · 2026-07-02T05:53:27Z

+     */
+    public Builder replace(int originalCount, int normalizedCount) {
+      if (originalCount < 0 || normalizedCount < 0) {
+        throw new IllegalArgumentException("counts must not be negative: " + originalCount


Please add IllegalArgumentException to Javadoc here.

mawiesne · 2026-07-02T05:55:31Z

+   * @return The normalized text and its alignment.
+   */
+  public AlignedText normalizeAligned(CharSequence text) {
+    Objects.requireNonNull(text, "text");


Please IllegalArgumentException instead of requireNonNull's NullPointerException.
Add proper doc for IllegalArgumentException to Javadoc here.

mawiesne · 2026-07-02T05:55:45Z

+   * @return The collapsed text and its alignment.
+   */
+  public AlignedText collapseAligned(CharSequence text) {
+    Objects.requireNonNull(text, "text");


Please IllegalArgumentException instead of requireNonNull's NullPointerException.
Add proper doc for IllegalArgumentException to Javadoc here.

mawiesne · 2026-07-02T05:57:30Z

+   * @return The trimmed text and its alignment.
+   */
+  public AlignedText trimAligned(CharSequence text) {
+    Objects.requireNonNull(text, "text");


Please IllegalArgumentException instead of requireNonNull's NullPointerException.
Adjust doc for IllegalArgumentException to Javadoc here.

mawiesne · 2026-07-02T05:57:40Z

+   * @return The filtered text and its alignment.
+   */
+  public AlignedText removeAllAligned(CharSequence text) {
+    Objects.requireNonNull(text, "text");


Please IllegalArgumentException instead of requireNonNull's NullPointerException.
Adjust doc for IllegalArgumentException to Javadoc here.

mawiesne · 2026-07-02T05:58:32Z

+   * @return The transformed text and its alignment.
+   */
+  public static AlignedText substituteAligned(CharSequence text, IntFunction<String> substitution) {
+    Objects.requireNonNull(text, "text");


Please IllegalArgumentException instead of requireNonNull's NullPointerException.
Same for second parameter substitution.
Add doc for IllegalArgumentException to Javadoc here. It should be generic for any parameter violation.

mawiesne · 2026-07-02T06:01:09Z

-public class DashCharSequenceNormalizer implements CharSequenceNormalizer {
+public class DashCharSequenceNormalizer implements OffsetAwareNormalizer {

  private static final long serialVersionUID = 6620885194730155303L;


The serialVersionUID can't be correct as the class now implements a different interface (OffsetAwareNormalizer). Hence, it should be regenerated.

Note: All other, similar classes which now implement OffsetAwareNormalizer need this recomputation of the UID value. I won't leave a comment for every single occurrence. Please adjust accordingly on your own.

mawiesne · 2026-07-02T06:03:32Z

+ * {@link Alignment#andThen(Alignment)} so a span found in the fully normalized text maps back to the
+ * original input, and a non-alignable rung is rejected loudly.
+ */
+public class AlignedNormalizerPipelineTest {


Please also add new test class which tests the LineBreakPreservingWhitespaceCharSequenceNormalizer class. This is currently untested in this PR!

Be hard and always provide tests for new classes.... !

krickert · 2026-07-02T09:29:00Z

All points addressed in ba7e0cd:

CharClass reports IllegalArgumentException for null parameters, with @throws documented on every affected method. The sweep covers the whole class, not only the aligned methods, so normalize and normalizeAligned share one contract; a null-contract test pins every entry point.
Alignment carries @threadsafe; Builder.equal/replace document their IllegalArgumentException for negative counts.
serialVersionUID regenerated with serialver for all ten classes that now implement OffsetAwareNormalizer.
New dedicated LineBreakPreservingWhitespaceCharSequenceNormalizerTest: horizontal collapse, break preservation including CRLF and U+2028/U+2029, edge trimming, whitespace-only input, aligned span mapping through the collapse-then-trim composition, and null rejection.

Stack cascade-rebased bottom-up (range-diff clean), full verify green.

mawiesne · 2026-07-02T09:53:38Z

@krickert Can you rebase this branch: it has conflicts against main.

…gned, *Aligned (1b) The conceptually hard half of the former foundation PR, split out on review request: the bidirectional Alignment edit-sequence and AlignedText, the OffsetAwareNormalizer capability interface, TextNormalizer.buildAligned(), the *Aligned CharClass variants and the offset-aware rungs, the line-break-preserving rung, and the dense span-mapping tests (binary-search span mapping, expansion/deletion edge cases, andThen composition including the insertion-in-expansion case). Builds on the engine in 1a.

…ving opener (alignment) OffsetAwareNormalizer no longer {@link}s OffsetMappingNameFinder (introduced in the DL layer) so the javadoc resolves when this PR is read on its own. LineBreakPreservingWhitespaceCharSequenceNormalizer's opener now reads 'An OffsetAwareNormalizer' to match the interface it implements.

…normalizedString() Both sides of AlignedText are now CharSequence so the record is symmetric with the caller's original input. Callers that need the materialized normalized form use normalizedString(); engine paths still build via StringBuilder.toString(), so there is no extra allocation on the hot path when normalized is already a String. Add AlignedTextTest and adjust two runtime tests to use normalizedString() where a String is required.

krickert · 2026-07-02T09:56:57Z

Rebased the whole stack onto main after #1108 merged, so this PR's diff shows only the alignment layer again (plus the review commit ba7e0cd's content, now at 01a8360). Range-diff clean, full test suites green. Thanks for the fast merge.

@throws

…, line-break rung test Addresses the 1b review round: - CharClass reports IllegalArgumentException for null parameters across the whole class (not requireNonNull's NullPointerException), so an invalid parameter and an invalid code point surface through the same exception type; @throws documented on every affected method, and the sweep covers the non-aligned twins too so normalize and normalizeAligned share one contract. A null-contract test pins every entry point. - Alignment carries @threadsafe; Builder.equal/replace document their IllegalArgumentException for negative counts. - serialVersionUID regenerated via serialver for the ten classes that now implement OffsetAwareNormalizer. - New dedicated LineBreakPreservingWhitespaceCharSequenceNormalizerTest: horizontal collapse, break preservation incl. CRLF and U+2028/U+2029, edge trimming, whitespace-only input, aligned mapping through the collapse-then-trim composition, and null rejection.

krickert force-pushed the OPENNLP-1850-1b-alignment branch 2 times, most recently from 9af6d92 to 9dc7d51 Compare June 24, 2026 11:54

krickert force-pushed the OPENNLP-1850-1a-engine branch from 69f5136 to 20226fb Compare June 25, 2026 08:26

krickert force-pushed the OPENNLP-1850-1b-alignment branch from 9dc7d51 to b24c9ee Compare June 25, 2026 08:26

krickert changed the title ~~OPENNLP-1850: Offset/alignment layer — Alignment, AlignedText, buildAligned (1b/5)~~ OPENNLP-1850: Offset/alignment layer — Alignment, AlignedText, buildAligned (1b/7) Jun 25, 2026

krickert marked this pull request as ready for review June 25, 2026 11:28

krickert force-pushed the OPENNLP-1850-1b-alignment branch from b24c9ee to 702acc5 Compare June 25, 2026 17:24

rzo1 requested review from atarora, jzonthemtn and mawiesne June 26, 2026 13:08

krickert force-pushed the OPENNLP-1850-1b-alignment branch from 702acc5 to 2bed555 Compare June 27, 2026 12:29

rzo1 approved these changes Jun 28, 2026

View reviewed changes

Comment thread opennlp-api/src/main/java/opennlp/tools/util/normalizer/AlignedText.java Outdated

mawiesne approved these changes Jun 30, 2026

View reviewed changes

mawiesne changed the title ~~OPENNLP-1850: Offset/alignment layer — Alignment, AlignedText, buildAligned (1b/7)~~ OPENNLP-1861: Offset/alignment layer — Alignment, AlignedText, buildAligned (1b/7) Jun 30, 2026

krickert force-pushed the OPENNLP-1850-1a-engine branch from b2a17a1 to e83e0ea Compare June 30, 2026 12:53

krickert force-pushed the OPENNLP-1850-1b-alignment branch from 45ad38b to c73ebeb Compare June 30, 2026 12:53

mawiesne requested changes Jul 2, 2026

View reviewed changes

mawiesne assigned krickert Jul 2, 2026

mawiesne added java Pull requests that update Java code tests Pull requests that add or update test code labels Jul 2, 2026

mawiesne mentioned this pull request Jul 2, 2026

OPENNLP-1863: Layered Term model — Term, TermAnalyzer (2b/7) #1111

Merged

krickert force-pushed the OPENNLP-1850-1b-alignment branch from c73ebeb to ba7e0cd Compare July 2, 2026 09:28

Base automatically changed from OPENNLP-1850-1a-engine to main July 2, 2026 09:28

krickert added 3 commits July 2, 2026 05:54

krickert force-pushed the OPENNLP-1850-1b-alignment branch from ba7e0cd to 01a8360 Compare July 2, 2026 09:56

krickert force-pushed the OPENNLP-1850-1b-alignment branch from 01a8360 to f089424 Compare July 2, 2026 10:00

mawiesne requested a review from rzo1 July 2, 2026 10:31

mawiesne approved these changes Jul 2, 2026

View reviewed changes

rzo1 approved these changes Jul 2, 2026

View reviewed changes

rzo1 merged commit a485a8d into main Jul 2, 2026
10 checks passed

rzo1 deleted the OPENNLP-1850-1b-alignment branch July 2, 2026 11:19

Uh oh!

Conversation

krickert commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rzo1 commented Jun 25, 2026

Uh oh!

krickert commented Jun 25, 2026

Uh oh!

Uh oh!

mawiesne commented Jun 30, 2026

Uh oh!

krickert commented Jun 30, 2026

Uh oh!

mawiesne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krickert commented Jul 2, 2026

Uh oh!

mawiesne commented Jul 2, 2026

Uh oh!

krickert commented Jul 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krickert commented Jun 23, 2026 •

edited

Loading