From 2c10dc3689bd8d600842e12781caaeff9b807ce4 Mon Sep 17 00:00:00 2001 From: Richard Eckart de Castilho Date: Tue, 20 Aug 2024 21:01:53 +0200 Subject: [PATCH] #4994 - Annotation browser sometimes does not load in sidebar curation mode - Make browser component more robust to even render if arc targets are missing (ignoring those arcs) - Improve curation sidebar renderer to handle cases where a link source is hidden due to an already merged annotation - Add hashCode and equals to Positions because it also implements compareTo --- .../annotation/rendering/LabelRenderer.java | 6 +- .../annotation/layer/span/SpanRenderer.java | 11 ++ .../webanno/curation/casdiff/CasDiff.java | 39 ++++--- .../casdiff/api/DiffAdapter_ImplBase.java | 5 +- .../curation/casdiff/api/Position.java | 2 + .../casdiff/api/Position_ImplBase.java | 95 +++++++++++++++- .../casdiff/relation/RelationPosition.java | 24 +++++ .../curation/casdiff/span/SpanPosition.java | 46 +++++++- .../main/ts/src/AnnotationsByLabelList.svelte | 11 +- .../main/ts/src/AnnotationsByLayerList.svelte | 11 +- .../model/compact_v2/CompactAnnotatedText.ts | 8 +- .../src/model/compact_v2/CompactArgument.ts | 5 +- .../src/model/compact_v2/CompactRelation.ts | 9 +- .../ukp/inception/rendering/vmodel/VArc.java | 10 +- .../ukp/inception/rendering/vmodel/VSpan.java | 6 ++ .../render/CurationSidebarRenderer.java | 102 +++++++++++++++--- 16 files changed, 326 insertions(+), 64 deletions(-) diff --git a/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/rendering/LabelRenderer.java b/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/rendering/LabelRenderer.java index 47f598f4333..e8ac2473d2d 100644 --- a/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/rendering/LabelRenderer.java +++ b/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/rendering/LabelRenderer.java @@ -22,11 +22,9 @@ import org.springframework.core.annotation.Order; import de.tudarmstadt.ukp.clarin.webanno.api.annotation.config.AnnotationAutoConfiguration; -import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationLayer; import de.tudarmstadt.ukp.inception.rendering.pipeline.RenderStep; import de.tudarmstadt.ukp.inception.rendering.request.RenderRequest; import de.tudarmstadt.ukp.inception.rendering.vmodel.VDocument; -import de.tudarmstadt.ukp.inception.rendering.vmodel.VObject; /** *

@@ -49,8 +47,8 @@ public String getId() @Override public void render(VDocument aVDoc, RenderRequest aRequest) { - for (AnnotationLayer layer : aVDoc.getAnnotationLayers()) { - for (VObject vobj : aVDoc.objects(layer.getId())) { + for (var layer : aVDoc.getAnnotationLayers()) { + for (var vobj : aVDoc.objects(layer.getId())) { if (vobj.getLabelHint() != null) { // Label hint was already set earlier - do not overwrite it! continue; diff --git a/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/span/SpanRenderer.java b/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/span/SpanRenderer.java index b9087c31ff3..7b3ff861ef1 100644 --- a/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/span/SpanRenderer.java +++ b/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/span/SpanRenderer.java @@ -19,6 +19,7 @@ import static de.tudarmstadt.ukp.clarin.webanno.model.LinkMode.WITH_ROLE; import static de.tudarmstadt.ukp.clarin.webanno.model.MultiValueMode.ARRAY; +import static de.tudarmstadt.ukp.inception.support.uima.ICasUtil.selectAnnotationByAddr; import static de.tudarmstadt.ukp.inception.support.uima.ICasUtil.selectByAddr; import static java.util.Collections.emptyList; import static org.apache.commons.lang3.StringUtils.abbreviate; @@ -123,6 +124,16 @@ public List lookupLazyDetails(CAS aCas, VID aVid) group.addDetail( new VLazyDetail("Text", abbreviate(fs.getCoveredText(), MAX_HOVER_TEXT_LENGTH))); + if (aVid.getAttribute() != VID.NONE) { + var adapter = getTypeAdapter(); + var feature = adapter.listFeatures().stream().sequential().skip(aVid.getAttribute()) + .findFirst().get(); + List sourceLinks = adapter.getFeatureValue(feature, fs); + var target = selectAnnotationByAddr(aCas, sourceLinks.get(aVid.getSlot()).targetAddr); + group.addDetail(new VLazyDetail("Target", + abbreviate(target.getCoveredText(), MAX_HOVER_TEXT_LENGTH))); + } + var details = super.lookupLazyDetails(aCas, aVid); if (!group.getDetails().isEmpty()) { details.add(0, group); diff --git a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/CasDiff.java b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/CasDiff.java index 4e38965d7ed..9906042776b 100644 --- a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/CasDiff.java +++ b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/CasDiff.java @@ -21,11 +21,11 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; import static java.util.stream.Collectors.groupingBy; -import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toCollection; +import static java.util.stream.Stream.concat; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -33,7 +33,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; -import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; import org.apache.uima.cas.ArrayFS; @@ -42,7 +41,6 @@ import org.apache.uima.cas.FeatureStructure; import org.apache.uima.cas.SofaFS; import org.apache.uima.cas.Type; -import org.apache.uima.cas.TypeSystem; import org.apache.uima.cas.text.AnnotationFS; import org.apache.uima.fit.util.FSUtil; import org.apache.uima.jcas.cas.AnnotationBase; @@ -445,8 +443,8 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2) return true; } - Type type1 = aFS1.getType(); - Type type2 = aFS2.getType(); + var type1 = aFS1.getType(); + var type2 = aFS2.getType(); // Types must be the same if (!type1.getName().equals(type2.getName())) { @@ -464,14 +462,13 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2) // such as begin, end, etc. Mind that the types may come from different CASes at different // levels of upgrading, so it could be that the types actually have slightly different // features. - Set labelFeatures = adapter.getLabelFeatures(); - List sortedFeatures = Stream - .concat(type1.getFeatures().stream().map(Feature::getShortName), - type2.getFeatures().stream().map(Feature::getShortName)) // - .filter(labelFeatures::contains) // - .sorted() // - .distinct() // - .collect(toList()); + var labelFeatures = adapter.getLabelFeatures(); + var sortedFeatures = concat(type1.getFeatures().stream().map(Feature::getShortName), + type2.getFeatures().stream().map(Feature::getShortName)) // + .filter(labelFeatures::contains) // + .sorted() // + .distinct() // + .collect(toCollection(ArrayList::new)); if (!recurseIntoLinkFeatures) { // #1795 Chili REC: We can/should change CasDiff2 such that it does not recurse into @@ -502,13 +499,13 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2) switch (range.getName()) { case CAS.TYPE_NAME_STRING_ARRAY: { - var value1 = FSUtil.getFeature(aFS1, f1, Set.class); + var value1 = f1 != null ? FSUtil.getFeature(aFS1, f1, Set.class) : null; if (value1 == null) { - value1 = Collections.emptySet(); + value1 = emptySet(); } - var value2 = FSUtil.getFeature(aFS2, f2, Set.class); + var value2 = f2 != null ? FSUtil.getFeature(aFS2, f2, Set.class) : null; if (value2 == null) { - value2 = Collections.emptySet(); + value2 = emptySet(); } if (!value1.equals(value2)) { return false; @@ -589,8 +586,8 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2) } default: { // Must be some kind of feature structure then - FeatureStructure valueFS1 = f1 != null ? aFS1.getFeatureValue(f1) : null; - FeatureStructure valueFS2 = f2 != null ? aFS2.getFeatureValue(f2) : null; + var valueFS1 = f1 != null ? aFS1.getFeatureValue(f1) : null; + var valueFS2 = f2 != null ? aFS2.getFeatureValue(f2) : null; // Ignore the SofaFS - we already checked that the CAS is the same. if (valueFS1 instanceof SofaFS) { @@ -607,7 +604,7 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2) // Q: Why do we not check recursively? // A: Because e.g. for chains, this would mean we consider the whole chain as a // single annotation, but we want to consider each link as an annotation - TypeSystem ts1 = aFS1.getCAS().getTypeSystem(); + var ts1 = aFS1.getCAS().getTypeSystem(); if (ts1.subsumes(ts1.getType(CAS.TYPE_NAME_ANNOTATION), type1)) { if (!equalsAnnotationFS((AnnotationFS) aFS1, (AnnotationFS) aFS2)) { return false; diff --git a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/DiffAdapter_ImplBase.java b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/DiffAdapter_ImplBase.java index e1658f786a9..26e62d6c418 100644 --- a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/DiffAdapter_ImplBase.java +++ b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/DiffAdapter_ImplBase.java @@ -17,8 +17,9 @@ */ package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api; +import static java.util.Collections.unmodifiableSet; + import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -44,7 +45,7 @@ public abstract class DiffAdapter_ImplBase public DiffAdapter_ImplBase(String aType, Set aLabelFeatures) { type = aType; - labelFeatures = Collections.unmodifiableSet(new HashSet<>(aLabelFeatures)); + labelFeatures = unmodifiableSet(new HashSet<>(aLabelFeatures)); } public void addLinkFeature(String aName, String aRoleFeature, String aTargetFeature) diff --git a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position.java b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position.java index 7f24f3376d3..8554e2d1278 100644 --- a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position.java +++ b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position.java @@ -55,4 +55,6 @@ public interface Position String getDocumentId(); String toMinimalString(); + + boolean isLinkFeaturePosition(); } diff --git a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position_ImplBase.java b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position_ImplBase.java index e52e8a959dd..235f07b5689 100644 --- a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position_ImplBase.java +++ b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position_ImplBase.java @@ -17,8 +17,11 @@ */ package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api; +import java.util.Objects; + import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.builder.HashCodeBuilder; import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.LinkCompareBehavior; @@ -30,16 +33,18 @@ public abstract class Position_ImplBase private final String type; private final String feature; + private final LinkCompareBehavior linkCompareBehavior; + private final String role; private final int linkTargetBegin; private final int linkTargetEnd; - private final String linkTargetText; - - private final LinkCompareBehavior linkCompareBehavior; + // BEGIN: For debugging only - not included in compareTo/hashCode/equals! private final String collectionId; private final String documentId; + private final String linkTargetText; + // END: For debugging only - not included in compareTo/hashCode/equals! public Position_ImplBase(String aCollectionId, String aDocumentId, String aType, String aFeature, String aRole, int aLinkTargetBegin, int aLinkTargetEnd, @@ -53,10 +58,10 @@ public Position_ImplBase(String aCollectionId, String aDocumentId, String aType, role = aRole; linkTargetBegin = aLinkTargetBegin; linkTargetEnd = aLinkTargetEnd; - linkTargetText = aLinkTargetText; collectionId = aCollectionId; documentId = aDocumentId; + linkTargetText = aLinkTargetText; } @Override @@ -77,6 +82,12 @@ public String getRole() return role; } + @Override + public boolean isLinkFeaturePosition() + { + return getFeature() != null; + } + @Override public int getLinkTargetBegin() { @@ -115,6 +126,16 @@ public LinkCompareBehavior getLinkCompareBehavior() @Override public int compareTo(Position aOther) { + // int collectionIdCmp = collectionId.compareTo(aOther.getCollectionId()); + // if (collectionIdCmp != 0) { + // return collectionIdCmp; + // } + // + // int documentIdCmp = documentId.compareTo(aOther.getDocumentId()); + // if (documentIdCmp != 0) { + // return documentIdCmp; + // } + int typeCmp = type.compareTo(aOther.getType()); if (typeCmp != 0) { return typeCmp; @@ -159,6 +180,72 @@ else if (linkCompareBehavior != null) { } } + @Override + public boolean equals(final Object other) + { + if (!(other instanceof Position_ImplBase)) { + return false; + } + + Position_ImplBase castOther = (Position_ImplBase) other; + var result = // Objects.equals(collectionId, castOther.collectionId) // + // && Objects.equals(documentId, castOther.documentId) // + Objects.equals(type, castOther.type) // + && Objects.equals(feature, castOther.feature) // + && Objects.equals(linkCompareBehavior, castOther.linkCompareBehavior); + + // If the base properties are equal, then we have to continue only linkCompareBehavior if it + // is non-null. + if (!result && linkCompareBehavior == null) { + return false; + } + + switch (linkCompareBehavior) { + case LINK_TARGET_AS_LABEL: + // Include role into position + return Objects.equals(role, castOther.role); + case LINK_ROLE_AS_LABEL: + // Include target into position + return Objects.equals(linkTargetBegin, castOther.linkTargetBegin) // + && Objects.equals(linkTargetEnd, castOther.linkTargetEnd); + default: + throw new IllegalStateException( + "Unknown link target comparison mode [" + linkCompareBehavior + "]"); + } + } + + @Override + public int hashCode() + { + var builder = new HashCodeBuilder() // + // .append(collectionId) // + // .append(documentId) // + .append(type) // + .append(feature) // + .append(linkCompareBehavior); + + if (linkCompareBehavior == null) { + return builder.toHashCode(); + } + + switch (linkCompareBehavior) { + case LINK_TARGET_AS_LABEL: + // Include role into position + builder.append(role); + break; + case LINK_ROLE_AS_LABEL: + // Include target into position + builder.append(linkTargetBegin); + builder.append(linkTargetEnd); + break; + default: + throw new IllegalStateException( + "Unknown link target comparison mode [" + linkCompareBehavior + "]"); + } + + return builder.toHashCode(); + } + protected void toStringFragment(StringBuilder builder) { if (getCollectionId() != null) { diff --git a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/relation/RelationPosition.java b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/relation/RelationPosition.java index 38d85b276e5..339d4f5a721 100644 --- a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/relation/RelationPosition.java +++ b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/relation/RelationPosition.java @@ -17,6 +17,8 @@ */ package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.relation; +import java.util.Objects; + import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.LinkCompareBehavior; import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api.Position; import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api.Position_ImplBase; @@ -110,6 +112,28 @@ else if (targetBegin != otherSpan.targetBegin) { } } + @Override + public boolean equals(final Object other) + { + if (!(other instanceof RelationPosition)) { + return false; + } + if (!super.equals(other)) { + return false; + } + RelationPosition castOther = (RelationPosition) other; + return Objects.equals(sourceBegin, castOther.sourceBegin) + && Objects.equals(sourceEnd, castOther.sourceEnd) + && Objects.equals(targetBegin, castOther.targetBegin) + && Objects.equals(targetEnd, castOther.targetEnd); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), sourceBegin, sourceEnd, targetBegin, targetEnd); + } + @Override public String toString() { diff --git a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/span/SpanPosition.java b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/span/SpanPosition.java index 4b0b6c0929a..ff6b8cbb8a8 100644 --- a/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/span/SpanPosition.java +++ b/inception/inception-curation-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/span/SpanPosition.java @@ -17,6 +17,8 @@ */ package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.span; +import java.util.Objects; + import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.LinkCompareBehavior; import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api.Position; import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api.Position_ImplBase; @@ -33,6 +35,15 @@ public class SpanPosition private final int end; private final String text; + public SpanPosition(String aCollectionId, String aDocumentId, String aType, int aBegin, + int aEnd, String aText) + { + super(aCollectionId, aDocumentId, aType, null, null, 0, 0, null, null); + begin = aBegin; + end = aEnd; + text = aText; + } + public SpanPosition(String aCollectionId, String aDocumentId, String aType, int aBegin, int aEnd, String aText, String aFeature, String aRole, int aLinkTargetBegin, int aLinkTargetEnd, String aLinkTargetText, LinkCompareBehavior aLinkCompareBehavior) @@ -44,6 +55,14 @@ public SpanPosition(String aCollectionId, String aDocumentId, String aType, int text = aText; } + /** + * @return the span position that owns the link feature (if this is a link feature position). + */ + public SpanPosition getBasePosition() + { + return new SpanPosition(getCollectionId(), getDocumentId(), getType(), begin, end, text); + } + /** * @return the begin offset. */ @@ -81,10 +100,29 @@ public int compareTo(Position aOther) } } + @Override + public boolean equals(final Object other) + { + if (!(other instanceof SpanPosition)) { + return false; + } + if (!super.equals(other)) { + return false; + } + SpanPosition castOther = (SpanPosition) other; + return Objects.equals(begin, castOther.begin) && Objects.equals(end, castOther.end); + } + + @Override + public int hashCode() + { + return Objects.hash(super.hashCode(), begin, end); + } + @Override public String toString() { - StringBuilder builder = new StringBuilder(); + var builder = new StringBuilder(); builder.append("Span ["); toStringFragment(builder); builder.append(", span=(").append(begin).append('-').append(end).append(')'); @@ -96,9 +134,10 @@ public String toString() @Override public String toMinimalString() { - StringBuilder builder = new StringBuilder(); + var builder = new StringBuilder(); builder.append(begin).append('-').append(end).append(" [").append(text).append(']'); - LinkCompareBehavior linkCompareBehavior = getLinkCompareBehavior(); + var linkCompareBehavior = getLinkCompareBehavior(); + if (linkCompareBehavior != null) { switch (linkCompareBehavior) { case LINK_TARGET_AS_LABEL: @@ -114,6 +153,7 @@ public String toMinimalString() "Unknown link target comparison mode [" + linkCompareBehavior + "]"); } } + return builder.toString(); } } diff --git a/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLabelList.svelte b/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLabelList.svelte index 7d6991a804c..fffece6ffd6 100644 --- a/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLabelList.svelte +++ b/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLabelList.svelte @@ -76,20 +76,21 @@ if ($sortByScore && aIsRec && bIsRec) { return b.score - a.score; } + return ( compareSpanText(data, a, b) || compareOffsets(a.offsets[0], b.offsets[0]) - ); + ) } if (a instanceof Relation && b instanceof Relation) { if ($sortByScore && aIsRec && bIsRec) { return b.score - a.score; } - return compareOffsets( - (a.arguments[0].target as Span).offsets[0], - (b.arguments[0].target as Span).offsets[0] - ); + + const targetA = a.arguments[0].target as Span + const targetB = b.arguments[0].target as Span + return compareOffsets(targetA.offsets[0], targetB.offsets[0]); } console.error("Unexpected annotation type combination", a, b); diff --git a/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLayerList.svelte b/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLayerList.svelte index 82cbb979583..cffb09dcca0 100644 --- a/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLayerList.svelte +++ b/inception/inception-diam-editor/src/main/ts/src/AnnotationsByLayerList.svelte @@ -76,20 +76,21 @@ if ($sortByScore && aIsRec && bIsRec) { return b.score - a.score; } + return ( compareSpanText(data, a, b) || compareOffsets(a.offsets[0], b.offsets[0]) - ); + ) } if (a instanceof Relation && b instanceof Relation) { if ($sortByScore && aIsRec && bIsRec) { return b.score - a.score; } - return compareOffsets( - (a.arguments[0].target as Span).offsets[0], - (b.arguments[0].target as Span).offsets[0] - ); + + const targetA = a.arguments[0].target as Span + const targetB = b.arguments[0].target as Span + return compareOffsets(targetA.offsets[0], targetB.offsets[0]); } console.error("Unexpected annotation type combination", a, b); diff --git a/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactAnnotatedText.ts b/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactAnnotatedText.ts index a93d0e12159..410c71b5abf 100644 --- a/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactAnnotatedText.ts +++ b/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactAnnotatedText.ts @@ -41,12 +41,16 @@ export function unpackCompactAnnotatedText (raw: CompactAnnotatedText): Annotate for (const span of raw.spans || []) { const cookedSpan = unpackCompactSpan(cooked, span) - cooked.spans.set(cookedSpan.vid, cookedSpan) + if (cookedSpan) { + cooked.spans.set(cookedSpan.vid, cookedSpan) + } } for (const rel of raw.relations || []) { const cookedRel = unpackCompactRelation(cooked, rel) - cooked.relations.set(cookedRel.vid, cookedRel) + if (cookedRel) { + cooked.relations.set(cookedRel.vid, cookedRel) + } } const annotationMarkers = raw.annotationMarkers?.map(unpackCompactAnnotationMarker) diff --git a/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactArgument.ts b/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactArgument.ts index 3ad6bfe4e8b..4602c6370cb 100644 --- a/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactArgument.ts +++ b/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactArgument.ts @@ -25,13 +25,14 @@ export type CompactArgument = [ label?: string ] -export function unpackCompactArgument (doc: AnnotatedText, raw: CompactArgument): Argument { +export function unpackCompactArgument (doc: AnnotatedText, raw: CompactArgument): Argument | undefined { const cooked = new Argument() cooked.targetId = raw[0] + cooked.label = raw[1] cooked.target = doc.spans.get(cooked.targetId) if (!cooked.target) { console.warn(`Target ${cooked.targetId} not found`) + return undefined } - cooked.label = raw[1] return cooked } diff --git a/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactRelation.ts b/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactRelation.ts index ada22e9236b..8edca6b3b0d 100644 --- a/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactRelation.ts +++ b/inception/inception-js-api/src/main/ts/src/model/compact_v2/CompactRelation.ts @@ -27,7 +27,7 @@ export type CompactRelation = [ attributes?: CompactRelationAttributes ] -export function unpackCompactRelation (doc: AnnotatedText, raw: CompactRelation): Relation { +export function unpackCompactRelation (doc: AnnotatedText, raw: CompactRelation): Relation | undefined { const cooked = new Relation() cooked.document = doc cooked.layer = doc.__getOrCreateLayer(raw[0]) @@ -38,5 +38,12 @@ export function unpackCompactRelation (doc: AnnotatedText, raw: CompactRelation) cooked.score = raw[3]?.s cooked.hideScore = raw[3]?.hs ? true : false cooked.comments = unpackCompactComments(doc, cooked, raw[3]?.cm) + + // Check if all arguments are defined + if (cooked.arguments.some(arg => arg === undefined)) { + console.warn(`Not decoding invalid relation relation ${cooked.vid}: undefined arguments`, cooked) + return undefined + } + return cooked } diff --git a/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VArc.java b/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VArc.java index 830c1d64e27..3cd81b1f41d 100644 --- a/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VArc.java +++ b/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VArc.java @@ -40,8 +40,8 @@ private VArc(Builder builder) super(builder.layer, builder.vid, builder.equivalenceSet, builder.features); setPlaceholder(builder.placeholder); setLabelHint(builder.label); - this.source = builder.source; - this.target = builder.target; + source = builder.source; + target = builder.target; } /** @@ -144,6 +144,12 @@ public VID getTarget() return target; } + @Override + public String toString() + { + return "VArc [" + getVid() + "]"; + } + public static Builder builder() { return new Builder(); diff --git a/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VSpan.java b/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VSpan.java index bcff88c0e76..08c3a4b0d50 100644 --- a/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VSpan.java +++ b/inception/inception-model-vdoc/src/main/java/de/tudarmstadt/ukp/inception/rendering/vmodel/VSpan.java @@ -128,6 +128,12 @@ public static Builder builder() return new Builder(); } + @Override + public String toString() + { + return "VSpan [" + getVid() + "]"; + } + public static final class Builder { private AnnotationLayer layer; diff --git a/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/inception/ui/curation/sidebar/render/CurationSidebarRenderer.java b/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/inception/ui/curation/sidebar/render/CurationSidebarRenderer.java index 75874f84727..9213bbb987f 100644 --- a/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/inception/ui/curation/sidebar/render/CurationSidebarRenderer.java +++ b/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/inception/ui/curation/sidebar/render/CurationSidebarRenderer.java @@ -57,6 +57,7 @@ import de.tudarmstadt.ukp.inception.rendering.vmodel.VCommentType; import de.tudarmstadt.ukp.inception.rendering.vmodel.VDocument; import de.tudarmstadt.ukp.inception.rendering.vmodel.VID; +import de.tudarmstadt.ukp.inception.rendering.vmodel.VObject; import de.tudarmstadt.ukp.inception.rendering.vmodel.VSpan; import de.tudarmstadt.ukp.inception.schema.api.AnnotationSchemaService; import de.tudarmstadt.ukp.inception.schema.api.layer.LayerSupportRegistry; @@ -178,6 +179,12 @@ public void render(VDocument aVdoc, RenderRequest aRequest) continue; } + // Do not render configurations that belong only the the target user. Those are + // already rendered by the normal annotation rendering mechanism + if (cfg.getCasGroupIds().size() == 1 && cfg.getCasGroupIds().contains(targetUser)) { + continue; + } + var user = cfg.getRepresentativeCasGroupId(); // We need to pass in *all* the annotation features here because we also to that in @@ -191,27 +198,32 @@ public void render(VDocument aVdoc, RenderRequest aRequest) for (var object : objects) { if (cfg.getPosition() instanceof SpanPosition spanPosition) { - if (spanPosition.getFeature() != null && object instanceof VSpan) { + if (spanPosition.isLinkFeaturePosition() && object instanceof VSpan) { // When processing a slot position, do not render the origin span - // because that should be already rendered by a separate position + // because that should be already when we encounter the base span + // position + continue; + } + + if (!spanPosition.isLinkFeaturePosition() && object instanceof VArc) { + // When processing a span position, do not render slot links + // because these should be rendered when we encounter the slot position continue; } } - if (!showAll && object instanceof VSpan) { + if (!showAll) { // Check if the target already contains an annotation from this // configuration - var sourceConfiguration = diff.findConfiguration( - cfg.getRepresentativeCasGroupId(), new AID(object.getVid())); - if (sourceConfiguration.map($ -> $.getAID(targetUser) != null) - .orElse(false)) { + if (isCurationSuggestionHiddenByMergedAnnotation(targetUser, diff, cfg, + object)) { continue; } // Check if the position could be merged at all or if the merge is blocked // by an overlapping annotation (which may not be contained in the // configuration) - if (noOverlap && fs instanceof Annotation ann) { + if (object instanceof VSpan && noOverlap && fs instanceof Annotation ann) { var cas = aRequest.getCas(); if (cas. select(ann.getType().getName()) .anyMatch(f -> ann.overlapping(f))) { @@ -220,6 +232,7 @@ public void render(VDocument aVdoc, RenderRequest aRequest) } } + // Check if the object has already been rendered var curationVid = new CurationVID(user, object.getVid()); if (generatedCurationVids.contains(curationVid)) { continue; @@ -232,12 +245,11 @@ public void render(VDocument aVdoc, RenderRequest aRequest) aVdoc.add(object); aVdoc.add(new VComment(object.getVid(), VCommentType.INFO, - "Annotators: " + cfg.getCasGroupIds().stream().collect(joining(", ")))); + "Annotators: " + cfg.getCasGroupIds().stream() + .filter(a -> !targetUser.equals(a)).collect(joining(", ")))); if (object instanceof VArc arc) { - // Currently works for relations but not for slots - arc.setSource(getCurationVid(targetUser, diff, cfg, arc.getSource())); - arc.setTarget(getCurationVid(targetUser, diff, cfg, arc.getTarget())); + resolveArcEndpoints(targetUser, diff, showAll, cfg, arc); LOG.trace("Rendering curation vid: {} source: {} target: {}", arc.getVid(), arc.getSource(), arc.getTarget()); } @@ -249,6 +261,14 @@ public void render(VDocument aVdoc, RenderRequest aRequest) } } + private boolean isCurationSuggestionHiddenByMergedAnnotation(String targetUser, DiffResult diff, + Configuration cfg, VObject object) + { + var sourceConfiguration = diff.findConfiguration(cfg.getRepresentativeCasGroupId(), + new AID(object.getVid())); + return sourceConfiguration.map($ -> $.getAID(targetUser) != null).orElse(false); + } + private CasDiff createDiff(RenderRequest aRequest, List selectedUsers) { var casses = new LinkedHashMap(); @@ -273,17 +293,73 @@ private CasDiff createDiff(RenderRequest aRequest, List selectedUsers) aRequest.getWindowEndOffset()); } + private void resolveArcEndpoints(String targetUser, DiffResult diff, boolean showAll, + Configuration cfg, VArc arc) + { + if (showAll) { + var representativeCasGroupId = cfg.getRepresentativeCasGroupId(); + arc.setSource(new CurationVID(representativeCasGroupId, arc.getSource())); + arc.setTarget(resolveVisibleEndpoint(targetUser, diff, cfg, arc.getTarget())); + return; + } + + if (cfg.getPosition() instanceof SpanPosition spanPosition + && spanPosition.isLinkFeaturePosition()) { + arc.setSource(resolveVisibleLinkHost(targetUser, diff, cfg, arc.getSource())); + } + else { + arc.setSource(resolveVisibleEndpoint(targetUser, diff, cfg, arc.getSource())); + } + arc.setTarget(resolveVisibleEndpoint(targetUser, diff, cfg, arc.getTarget())); + } + + private VID resolveVisibleLinkHost(String aTargetUser, DiffResult aDiff, Configuration aCfg, + VID aVid) + { + var representativeCasGroupId = aCfg.getRepresentativeCasGroupId(); + + // If this is a link feature position, derive the base span position (i.e. the span + // which owns the link feature) and check if that has already been merged. If yes, we + // need to return the merged position instead of the curator's position. + if (aCfg.getPosition() instanceof SpanPosition spanPosition + && spanPosition.isLinkFeaturePosition()) { + var originalSpanPosition = spanPosition.getBasePosition(); + var cfgSet = aDiff.getConfigurationSet(originalSpanPosition); + if (cfgSet != null) { + var targetConfigurations = cfgSet.getConfigurations(aTargetUser); + if (!targetConfigurations.isEmpty()) { + // FIXME: This is probably sub-optimal. What if the target user has multiple + // configurations at this position? Currently, we simply attach to the first + // one - which may not be the best matching one. + // In particular, it may not be the one onto which the link will eventually + // be merged... + var curatedAID = targetConfigurations.get(0).getAID(aTargetUser); + if (curatedAID != null) { + return new VID(curatedAID.addr); + } + } + } + } + + return new CurationVID(representativeCasGroupId, aVid); + } + /** * Find and return the rendered VID which is equivalent to the given VID. E.g. if the given VID * belongs to an already curated annotation, then locate the VID for the rendered annotation of * the curation user by searching the diff for configuration set that contains the annotators * annotation and then switching over to the configuration containing the curators annotation. */ - private VID getCurationVid(String aTargetUser, DiffResult aDiff, Configuration aCfg, VID aVid) + private VID resolveVisibleEndpoint(String aTargetUser, DiffResult aDiff, Configuration aCfg, + VID aVid) { var representativeCasGroupId = aCfg.getRepresentativeCasGroupId(); + var sourceConfiguration = aDiff.findConfiguration(representativeCasGroupId, new AID(aVid)); + // This is for relation annotation endpoints and link targets. Here we can look for the + // configuration which contains the annotators annotation and find the corresponding + // curated version for it in the same configuration if (sourceConfiguration.isPresent()) { var curatedAID = sourceConfiguration.get().getAID(aTargetUser); if (curatedAID != null) {