From 196f694aa1391146a53923df5507462c345f63a9 Mon Sep 17 00:00:00 2001 From: Richard Eckart de Castilho Date: Sun, 9 Aug 2026 14:01:11 +0200 Subject: [PATCH] #1648 - AlignedString.resolve returns an empty interval for the first character after a deletion - Fix AlignedString.resolve() to correctly handle single-character intervals at anchor positions by preventing anchor-inclusive segment lookup for endSeg in non-empty intervals - Add regression tests for resolve() correctness: testResolveSingleCharacterAtStart, testResolveSingleCharacterAtInnerAnchor, testResolveRoundTripsForEveryCharacter, testResolveRoundTripsForEveryCharacterWithAnchors --- .../transform/alignment/AlignedString.java | 15 +++- .../alignment/AlignedStringTest.java | 82 +++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/dkpro-core-api-transform-asl/src/main/java/org/dkpro/core/api/transform/alignment/AlignedString.java b/dkpro-core-api-transform-asl/src/main/java/org/dkpro/core/api/transform/alignment/AlignedString.java index bd7f6109b..cc082d6c0 100644 --- a/dkpro-core-api-transform-asl/src/main/java/org/dkpro/core/api/transform/alignment/AlignedString.java +++ b/dkpro-core-api-transform-asl/src/main/java/org/dkpro/core/api/transform/alignment/AlignedString.java @@ -131,8 +131,19 @@ public Interval resolve(final Interval i) // situation that startSeg may point into segment A, then follows a // deleted segment B and endSeg will point into C. The resolved interval // will thus contain A+B instead of only A. - final DataSegment endSeg = getSegmentAt( - (i.getStart() != i.getEnd()) ? i.getEnd() - 1 : i.getEnd(), true); + // + // For a non-empty interval we must not accept an anchor here: anchors have zero width + // and therefore never contain the last character of the interval. Returning the anchor + // that sits at the position of the last character would make the end-seeking loop below + // believe that it had to move right onto the containing segment, so that the start of + // that segment - rather than a position inside it - would be used as the interval end. + final DataSegment endSeg; + if (i.getStart() != i.getEnd()) { + endSeg = getSegmentAt(i.getEnd() - 1, false); + } + else { + endSeg = getSegmentAt(i.getEnd(), true); + } // For start find oblique segment here or to left. // If none start is start of first segment. diff --git a/dkpro-core-api-transform-asl/src/test/java/org/dkpro/core/api/transform/alignment/AlignedStringTest.java b/dkpro-core-api-transform-asl/src/test/java/org/dkpro/core/api/transform/alignment/AlignedStringTest.java index 8ac616173..7776530d4 100644 --- a/dkpro-core-api-transform-asl/src/test/java/org/dkpro/core/api/transform/alignment/AlignedStringTest.java +++ b/dkpro-core-api-transform-asl/src/test/java/org/dkpro/core/api/transform/alignment/AlignedStringTest.java @@ -523,6 +523,88 @@ public void testResolveAfterInsertAtEnd() assertEquals(baseString, bottom.get(i.getStart(), i.getEnd())); } + /** + * Resolving a non-empty interval must never yield an empty interval - otherwise the + * corresponding text in the underlying data cannot be recovered. + *

+ * A single character starting exactly at an anchor position is the critical case: anchors have + * zero width, so the segment that carries the last character of the interval must be used to + * determine the interval end, not the anchor sitting at that position. + */ + @Test + public void testResolveSingleCharacterAtStart() + { + bottom = new AlignedString("abcdefghij"); + top = new AlignedString(bottom); + + // No modification at all - position 0 always coincides with the leading anchor + final Interval i = top.resolve(new ImmutableInterval(0, 1)); + + assertEquals(1, i.getLength()); + assertEquals("a", bottom.get(i.getStart(), i.getEnd())); + } + + /** + * Resolving a single character that starts at an anchor in the middle of the data must yield + * that character rather than an empty interval. + */ + @Test + public void testResolveSingleCharacterAtInnerAnchor() + { + bottom = new AlignedString("abcdefghij"); + top = new AlignedString(bottom); + + // Force an anchor - and therefore a segment boundary - in the middle of the data + top.getAnchor(5); + + final Interval i = top.resolve(new ImmutableInterval(5, 6)); + + assertEquals(1, i.getLength()); + assertEquals("f", bottom.get(i.getStart(), i.getEnd())); + } + + /** + * Every single character of a view must resolve to exactly the same character in the underlying + * data, whether or not the view was modified. + */ + @Test + public void testResolveRoundTripsForEveryCharacter() + { + bottom = new AlignedString("abcdefghij"); + top = new AlignedString(bottom); + top.delete(3, 7); + + final String topText = top.get(); + assertEquals("abchij", topText); + + for (int p = 0; p < topText.length(); p++) { + final Interval i = top.resolve(new ImmutableInterval(p, p + 1)); + assertEquals(String.valueOf(topText.charAt(p)), bottom.get(i.getStart(), i.getEnd()), + "Character at position " + p + " does not resolve to itself"); + } + } + + /** + * Same as {@link #testResolveRoundTripsForEveryCharacter()} but on an unmodified view with an + * additional anchor, showing that the round trip does not depend on a deletion being present. + */ + @Test + public void testResolveRoundTripsForEveryCharacterWithAnchors() + { + bottom = new AlignedString("abcdefghij"); + top = new AlignedString(bottom); + top.getAnchor(5); + + final String topText = top.get(); + assertEquals("abcdefghij", topText); + + for (int p = 0; p < topText.length(); p++) { + final Interval i = top.resolve(new ImmutableInterval(p, p + 1)); + assertEquals(String.valueOf(topText.charAt(p)), bottom.get(i.getStart(), i.getEnd()), + "Character at position " + p + " does not resolve to itself"); + } + } + /** * The cached segment start positions must stay consistent with the actual segment chain. *