(opennlp) 08/15: relation: Reject whitespace-bearing triggers and judge blankness by the project predicate

[kr...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

krickert pushed a commit to branch OPENNLP-1920-relation-extraction
in repository https://gitbox.apache.org/repos/asf/opennlp.git

commit ee2f5bbe4c8c74cb5e691f12a6b20f953ab58c37
Author: Kristian Rickert <kr...@gmail.com>
AuthorDate: Fri Jul 17 00:22:33 2026 -0400

    relation: Reject whitespace-bearing triggers and judge blankness by the project predicate
    
    A trigger is compared against a single pivot token, so a trigger containing any
    whitespace is a rule that can never match; construction now rejects it, closing
    the half of the never-matching defect the case-mapping fix left open. Type,
    path, and trigger blankness are judged under the project whitespace definition,
    which covers the no-break spaces the JDK blank check misses, so a path spelled
    from them can no longer construct a zero-step dead rule, and the steps of every
    constructible pattern are guaranteed non-empty. New tests pin the
    supplementary-plane cased trigger pair and the negative side of the annotator's
    mapping seam: the JDK-lowercased spelling of the dotted capital I matches
    nothing.
---
 .../opennlp/tools/relation/RelationPattern.java    | 49 ++++++++++++++++------
 .../tools/relation/RelationAnnotatorTest.java      | 18 ++++++++
 .../tools/relation/RelationPatternTest.java        | 47 +++++++++++++++++----
 3 files changed, 93 insertions(+), 21 deletions(-)

diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationPattern.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationPattern.java
index 6e202c011..3b2f58f1a 100644
--- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationPattern.java
+++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationPattern.java
@@ -48,31 +48,43 @@ import opennlp.tools.util.StringUtil;
  * @param path The path shape as described above. Must not be {@code null} or blank, and
  *             every up step must come before the first down step.
  * @param trigger The required pivot form, or {@code null} for any pivot. Must not be
- *                blank, and must be unchanged by {@link StringUtil#toLowerCase(CharSequence)}.
+ *                blank or contain whitespace, since it is matched against a single
+ *                token, and must be unchanged by
+ *                {@link StringUtil#toLowerCase(CharSequence)}.
  *
  * @since 3.0.0
  */
 public record RelationPattern(String type, String path, String trigger) {
 
   /**
-   * Validates the rule.
+   * Validates the rule. Blankness is judged under the project whitespace definition,
+   * which unlike the JDK's includes no-break spaces, so a value spelled entirely from
+   * them cannot construct a rule that could never match.
    *
    * @throws IllegalArgumentException Thrown if {@code type} or {@code path} is
-   *         {@code null} or blank, {@code path} is malformed, or {@code trigger} is blank
-   *         or not already lowercased under
+   *         {@code null} or blank, {@code path} is malformed, or {@code trigger} is
+   *         blank, contains whitespace, or is not already lowercased under
    *         {@link StringUtil#toLowerCase(CharSequence)}.
    */
   public RelationPattern {
-    if (type == null || type.isBlank()) {
+    if (type == null || blank(type)) {
       throw new IllegalArgumentException("type must not be null or blank");
     }
-    if (path == null || path.isBlank()) {
+    if (path == null || blank(path)) {
       throw new IllegalArgumentException("path must not be null or blank");
     }
     if (trigger != null) {
-      if (trigger.isBlank()) {
+      if (trigger.isEmpty()) {
         throw new IllegalArgumentException("trigger must not be blank");
       }
+      for (int i = 0; i < trigger.length(); ) {
+        final int cp = trigger.codePointAt(i);
+        if (StringUtil.isWhitespace(cp)) {
+          throw new IllegalArgumentException("trigger must not contain whitespace,"
+              + " since it is matched against a single token: " + trigger);
+        }
+        i += Character.charCount(cp);
+      }
       if (!StringUtil.toLowerCase(trigger).equals(trigger)) {
         throw new IllegalArgumentException("trigger must be lowercased, but was: " + trigger);
       }
@@ -94,16 +106,29 @@ public record RelationPattern(String type, String path, String trigger) {
   /**
    * Splits the path into its steps.
    *
-   * @return The steps in order. Never {@code null}. Empty only when the path consists
-   *         solely of characters that are whitespace to
-   *         {@link StringUtil#isWhitespace(char)} but not to {@link String#isBlank()},
-   *         for example the no-break space U+00A0; such a pattern never matches, since
-   *         every path between two distinct entity heads has at least one step.
+   * @return The steps in order. Never {@code null} and never empty: the constructor
+   *         judges blankness by the same whitespace definition this split separates
+   *         on, so every constructible path holds at least one step.
    */
   public List<String> steps() {
     return splitSteps(path);
   }
 
+  /**
+   * Reports whether a value is blank under the project whitespace definition, which
+   * unlike the JDK's includes no-break spaces.
+   */
+  private static boolean blank(String value) {
+    for (int i = 0; i < value.length(); ) {
+      final int cp = value.codePointAt(i);
+      if (!StringUtil.isWhitespace(cp)) {
+        return false;
+      }
+      i += Character.charCount(cp);
+    }
+    return true;
+  }
+
   /**
    * Splits a path into whitespace-free steps with a single left-to-right character
    * scan. Every character that {@link StringUtil#isWhitespace(char)} accepts acts as a
diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java
index 374201638..23d997a47 100644
--- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java
+++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java
@@ -366,6 +366,24 @@ public class RelationAnnotatorTest {
         relations.get(0).span().getStart(), relations.get(0).span().getEnd()).toString());
   }
 
+  /**
+   * The negative side of the mapping seam: the JDK's locale-independent lowercasing of
+   * the dotted capital I expands to an {@code i} followed by the combining dot above
+   * (U+0307), a spelling the project mapping never produces for the pivot, so a
+   * trigger written that way matches nothing. This pins that the annotator compares
+   * through the project mapping alone, and that a trigger prepared with
+   * {@link String#toLowerCase()} does not silently work by accident.
+   */
+  @Test
+  void testJdkLowercasedTriggerSpellingDoesNotMatchThePivot() {
+    final RelationAnnotator annotator = new RelationAnnotator(List.of(
+        new RelationPattern("located_in", ">appos >nmod", "i\u0307stanbul")));
+
+    final Document document = annotator.annotate(dottedCapitalPivotDocument());
+
+    Assertions.assertTrue(document.get(RelationAnnotator.RELATIONS).isEmpty());
+  }
+
   /**
    * Verifies that annotating a document that already carries a relations layer fails:
    * documents reject duplicate layers, so the second annotation pass throws.
diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java
index 07291edec..c65bb0d25 100644
--- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java
+++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java
@@ -195,9 +195,11 @@ public class RelationPatternTest {
    */
   @Test
   void testTriggerCheckFollowsTheProjectCaseMapping() {
+    // the Deseret pair is supplementary-plane: capital long I, U+10400, lowercases to
+    // U+10428, so acceptance must follow the mapping across surrogate pairs as well
     for (final String candidate : new String[] {"founded", "Founded",
         DOTTED_CAPITAL_ISTANBUL, "istanbul", JDK_LOWERCASED_ISTANBUL, "STRASSE",
-        SHARP_S_STRASSE}) {
+        SHARP_S_STRASSE, "\uD801\uDC00", "\uD801\uDC28"}) {
       final String message = "trigger acceptance for " + candidate
           + " must follow StringUtil.toLowerCase";
       if (StringUtil.toLowerCase(candidate).equals(candidate)) {
@@ -211,15 +213,42 @@ public class RelationPatternTest {
   }
 
   /**
-   * Documents the boundary between the blank guard and the whitespace scan: the blank
-   * check only recognizes the whitespace characters the JDK does, while the step scan
-   * also treats space separators such as {@code U+00A0} as whitespace. A path holding
-   * only a no-break space therefore passes construction but splits into zero steps, and
-   * such a pattern can never match because every computed path has at least one step.
+   * Verifies that blankness is judged by the same whitespace definition the step scan
+   * separates on: a path holding only a no-break space would split into zero steps and
+   * could never match, so it is rejected at construction instead of building a dead
+   * rule, and the same holds for a type spelled from nothing but no-break spaces.
    */
   @Test
-  void testNoBreakSpaceOnlyPathConstructsWithZeroSteps() {
-    final RelationPattern pattern = new RelationPattern("t", "\u00A0", null);
-    Assertions.assertEquals(List.of(), pattern.steps());
+  void testNoBreakSpaceOnlyValuesAreRejectedAsBlank() {
+    final IllegalArgumentException path = Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> new RelationPattern("t", "\u00A0", null));
+    Assertions.assertEquals("path must not be null or blank", path.getMessage());
+
+    final IllegalArgumentException type = Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> new RelationPattern("\u00A0", "<nsubj", null));
+    Assertions.assertEquals("type must not be null or blank", type.getMessage());
+  }
+
+  /**
+   * Verifies that a trigger containing whitespace is rejected at construction: the
+   * trigger is compared against a single pivot token, which the tokenizer guarantees
+   * carries no whitespace, so a multi-word or padded trigger is a rule that could
+   * never match. The whitespace scan follows the project predicate, so a no-break
+   * space inside or around the trigger is caught the same way as a plain space.
+   */
+  @Test
+  void testTriggerContainingWhitespaceIsRejected() {
+    final IllegalArgumentException spaced = Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> new RelationPattern("t", "<nsubj", "new york"));
+    Assertions.assertEquals("trigger must not contain whitespace, since it is matched"
+        + " against a single token: new york", spaced.getMessage());
+
+    Assertions.assertThrows(IllegalArgumentException.class,
+        () -> new RelationPattern("t", "<nsubj", "founded\u00A0"));
+    Assertions.assertThrows(IllegalArgumentException.class,
+        () -> new RelationPattern("t", "<nsubj", "\u00A0"));
   }
 }