Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
347 changes: 347 additions & 0 deletions docs/superpowers/plans/2026-07-01-react-matchable-unity-annotations.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
# Restore matchability of spot-crop (unity) annotations on the React Encounter page

**Date:** 2026-07-01
**Target:** `main` (GitHub Milestone 10.12)
**Type:** Bug fix (React regression introduced April 2026)

## Problem

On the React Encounter page, a spot-mapping crop annotation displays with no
annotation boundary and a grayed-out "New Match" button, even though the
annotation is genuinely matchable. The same annotation shows as matchable in
`obrowse.jsp` and satisfies the backend match-eligibility rule.

### Root cause (confirmed end-to-end)

Spot-mapping crops are created by `SubmitSpotsAndImage.java`:

```java
MediaAsset crMa = store.create(params); // :98 the crop = its own MediaAsset
Annotation ann = new Annotation(speciesString, crMa); // :122 whole-image "unity" annotation
ann.setMatchAgainst(true); // :123 deliberately matchable
```

`new Annotation(String, MediaAsset)` (`Annotation.java:117-120`) is explicitly the
"trivial" constructor — it builds a single **unity feature** (`FeatureType == null`)
covering the whole cropped image. So these annotations are, by design, both
"trivial" (whole-image) **and** matchable (`matchAgainst=true`, `acmId` set).

The React Encounter page determines matchability from a **geometry proxy** computed
live in `Encounter.opensearchDocumentSerializer()` — `isTrivial` and the bounding
box — and never consults the real fields (`matchAgainst`, `acmId`), which are not
even serialized in the encounter API response. `Annotation.isTrivial()`
(`Annotation.java:457-468`) returns `true` for any unity feature, so the
April-2026 filter `!isTrivial && bbox>0` (commits `eb795ae45d`, `33b08f860a`)
drops the annotation.

The DB row for the reported annotation confirms this exactly: unity feature
(`FEATURE.TYPE_ID_OID` NULL), `ANNOTATION.WIDTH/HEIGHT = 0`, `matchAgainst=true`,
`acmId` set, MediaAsset = the crop.

### Scope narrowing

- The **downstream match already works**: `NewMatchStore.annotationIds`
(`NewMatchStore.js:106-116`) filters only by `encounterId` — not `isTrivial`/bbox —
so once the button is clickable, the match payload already includes this annotation.
- Therefore the **only load-bearing bug** is the button gate
(`hasNonTrivialAnnotations`), which reads the filtered `encounterAnnotations`
getter that drops `isTrivial`.
- The **missing boundary rectangle is cosmetic and out of scope**: a whole-image
crop has no sub-region to outline.

## Approach

Stop using the geometry proxy as the matchability gate. Serialize the real
signal (`matchAgainst`, `acmId`) into the encounter API, and gate the New Match
button on it — mirroring the backend's exact eligibility rule
(`matchAgainst=true AND acmId IS NOT NULL`).

This is strictly more correct than the current logic: genuine placeholder
annotations have `matchAgainst=false` and stay hidden, while spot-crop
annotations (and any other legitimately matchable whole-image annotation)
reappear.

## Backend change

`Encounter.opensearchDocumentSerializer()` — annotation loop (~`Encounter.java:4464-4484`).
Add two fields inside the per-annotation object:

```java
jgen.writeBooleanField("matchAgainst", ann.getMatchAgainst());
jgen.writeStringField("acmId", ann.getAcmId());
```

Notes:
- Served **live** via `jsonForApiGet` → `opensearchDocumentAsJSONObject`
(`Base.java:302-315` builds the JSON fresh from the object each call).
**No reindex and no OpenSearch mapping change required** — the fields appear on
the next API call.
- Additive and backward-compatible. The same serializer also builds the indexed
encounter document, so the fields will additionally land under the nested
`annotations` array on the next reindex via dynamic mapping — additive, not
queried on. (Codex to sanity-check the nested `annotations` mapping.)

## Frontend changes

1. **New getter** `EncounterStore.hasMatchableAnnotations`
(`frontend/src/pages/Encounter/stores/EncounterStore.js`). Reads the **raw**
`encounterData.mediaAssets[selectedImageIndex].annotations` (NOT the filtered
`encounterAnnotations` getter), filters `encounterId === encounterData.id`, and
returns `.some(a => a.matchAgainst && !!a.acmId)`.

2. **Delegation** — `ImageModalStore` (Encounter,
`frontend/src/pages/Encounter/stores/ImageModalStore.js`) delegates
`hasMatchableAnnotations` to `encounterStore` (mirrors the existing
`encounterAnnotations` delegation). The shared `components/ImageModal.jsx` is
imported **only** by `Encounter/ImageCard.jsx` (verified), so it always receives
the Encounter `ImageModalStore`. The SearchPages `ImageModalStore` is **not**
in scope — SearchPages uses `ImageGalleryModal`, which has no New Match button.

3. **`ImageCard.jsx:107`** — replace the local `hasNonTrivialAnnotations`
(filtered getter + redundant re-filter) with `store.hasMatchableAnnotations`.
Update its three usages (~793, 804, 806).

4. **`ImageModal.jsx:216`** — replace with `imageStore.hasMatchableAnnotations`.
Update its two usages (~1109, 1111).

### Left untouched (deliberately)

- The `encounterAnnotations` getter (`EncounterStore.js:601-613`) — feeds
edit/select/task-id/`currentAnnotation` flows; changing its filter would ripple
broadly.
- The boundary-drawing `rects` filter (`ImageCard.jsx:124-128`) — cosmetic.
- `NewMatchStore.annotationIds` — already includes the annotation.

### Naming

Rename `hasNonTrivialAnnotations` → `hasMatchableAnnotations` in both components;
the old name is now misleading.

## Data flow (after fix)

encounter API (`matchAgainst`+`acmId` per annotation) →
`EncounterStore.hasMatchableAnnotations` → New Match button enabled →
`MatchCriteriaModal` → `NewMatchStore.annotationIds` (already includes the
annotation) → match runs.

## Testing

- **Backend:** assert `opensearchDocumentSerializer` emits `matchAgainst` and
`acmId` for each serialized annotation (extend the existing Encounter
serialization test if present; otherwise add a focused one).
- **Frontend (jest):**
- `EncounterStore.test.js`: `hasMatchableAnnotations` is `true` for a
`matchAgainst + acmId` annotation **even when `isTrivial` is true and the
bounding box is the full-image box** (unity features serialize a positive
full-image bbox, not an empty one — the annotation is dropped by `isTrivial`,
not by bbox); `false` when `matchAgainst` is false or `acmId` is missing;
`false` for annotations belonging to a different `encounterId`.
- `ImageCard.test.js`: New Match enabled/disabled tracks
`hasMatchableAnnotations`.
- `ImageModal.test.js`: same for the modal button.
- **CI caveat:** jest failures are invisible in CI (continue-on-error). Run jest
locally to verify.

## Risks

- Additive API/index fields — backward compatible. Encounter mapping does not set
`dynamic:false`, so the new nested fields map dynamically on later indexing; add
explicit mapping only if they later become queried/sorted.
- Rename touches a handful of lines across two components.

## Generality

The reported case is spot crops (`SubmitSpotsAndImage.java`), but the fix is not
specific to that path. The UI gate becomes "any annotation satisfying the backend
predicate `matchAgainst && acmId`" — which also covers annotations promoted to
matchable by other paths (e.g. `AnnotationSetMatchAgainst`, IA/detection flows).

## Out of scope

- Drawing a boundary for whole-image crops (cosmetic).
- **Residual geometry-proxy consumers.** The `encounterAnnotations` getter stays
filtered, so flows keyed off annotation *selection* still exclude unity
annotations: `matchResultClickable`/"Match Results" (`EncounterStore.js:627`,
`ImageModal.jsx:1056`), the edit-annotation params (`ImageCard.jsx:49`), and
`currentAnnotation`. This does **not** block New Match (the initiate flow
navigates straight to `/react/match-results?taskId=…` with the fresh task), but
a user cannot later *re-select* a unity annotation on the Encounter page to
revisit its past match results. Documented as a known limitation; making unity
annotations selectable would require touching the shared getter and is deferred.
- `SubmitSpotsAndTransformImage.java` (per user instruction — it builds a real
bounding-box feature and does not set `matchAgainst`; not the path that created
the reported annotation).
58 changes: 58 additions & 0 deletions frontend/src/__tests__/pages/Encounter/EncounterStore.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,64 @@ describe("EncounterStore", () => {
]);
});

it("hasMatchableAnnotations is true for a matchAgainst+acmId unity annotation (isTrivial, full-image bbox)", () => {
store.setEncounterData({
id: "enc-123",
mediaAssets: [
{
annotations: [
{
id: "ann-1",
encounterId: "enc-123",
matchAgainst: true,
acmId: "acm-1",
isTrivial: true,
boundingBox: [0, 0, 1000, 500],
},
],
},
],
});
store.setSelectedImageIndex(0);

expect(store.hasMatchableAnnotations).toBe(true);
// delegation: ImageModalStore.hasMatchableAnnotations forwards to EncounterStore
expect(store.imageModal.hasMatchableAnnotations).toBe(true);
});

it("hasMatchableAnnotations is false when matchAgainst is false or acmId is missing", () => {
store.setEncounterData({
id: "enc-123",
mediaAssets: [
{
annotations: [
{ id: "a", encounterId: "enc-123", matchAgainst: false, acmId: "acm-1" },
{ id: "b", encounterId: "enc-123", matchAgainst: true },
],
},
],
});
store.setSelectedImageIndex(0);

expect(store.hasMatchableAnnotations).toBe(false);
});

it("hasMatchableAnnotations ignores annotations from a different encounterId", () => {
store.setEncounterData({
id: "enc-123",
mediaAssets: [
{
annotations: [
{ id: "a", encounterId: "enc-999", matchAgainst: true, acmId: "acm-1" },
],
},
],
});
store.setSelectedImageIndex(0);

expect(store.hasMatchableAnnotations).toBe(false);
});

it("returns true when match result is clickable", () => {
store.setEncounterData({
id: "enc-123",
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/__tests__/pages/Encounter/ImageCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ const makeStore = (overrides = {}) => ({
setSelectedAnnotationId: jest.fn(),
setIntl: jest.fn(),
matchResultClickable: false,
hasMatchableAnnotations: true,
modals: {
setOpenMatchCriteriaModal: jest.fn(),
},
Expand Down Expand Up @@ -199,6 +200,15 @@ describe("ImageCard", () => {
expect(store.modals.setOpenMatchCriteriaModal).toHaveBeenCalledWith(true);
});

test("NEW_MATCH does nothing when hasMatchableAnnotations is false", async () => {
const user = userEvent.setup();
const store = makeStore({ hasMatchableAnnotations: false });
renderCard(store);

await user.click(screen.getByText("NEW_MATCH"));
expect(store.modals.setOpenMatchCriteriaModal).not.toHaveBeenCalled();
});

test("clicking VISUAL_MATCHER opens visual matcher page", async () => {
const user = userEvent.setup();
const store = makeStore();
Expand Down
17 changes: 17 additions & 0 deletions frontend/src/__tests__/pages/Encounter/ImageModal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ const makeImageStore = (overrides = {}) => ({
setSelectedAnnotationId: jest.fn(),
selectedAnnotationId: null,
matchResultClickable: true,
hasMatchableAnnotations: true,
encounterAnnotations: [{ id: "ann-1", iaTaskId: "task-123" }],
tags: [],
addTagsFieldOpen: false,
Expand Down Expand Up @@ -242,6 +243,22 @@ describe("ImageModal", () => {
expect(btn).toBeDisabled();
});

test("NEW_MATCH button is disabled when hasMatchableAnnotations is false", () => {
// encounterAnnotations has a positive-bbox annotation so the OLD isTrivial/bbox
// gate would ENABLE the button — the button is disabled ONLY once the gate reads
// hasMatchableAnnotations. This guarantees the test fails before the impl.
const store = makeImageStore({
hasMatchableAnnotations: false,
encounterAnnotations: [
{ id: "ann-1", isTrivial: false, boundingBox: [0, 0, 10, 10] },
],
});
renderModal({ imageStore: store });

const btn = screen.getByText("NEW_MATCH").closest("button");
expect(btn).toBeDisabled();
});

test("delete image button confirms and calls imageStore.deleteImage", async () => {
const store = makeImageStore();
renderModal({ imageStore: store });
Expand Down
8 changes: 3 additions & 5 deletions frontend/src/components/ImageModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,7 @@ export const ImageModal = observer(
);
}, [rects]);

const hasNonTrivialAnnotations = imageStore.encounterAnnotations?.some(
(a) => !a.isTrivial && (a.boundingBox?.[2] || 0) > 0 && (a.boundingBox?.[3] || 0) > 0
);
const hasMatchableAnnotations = imageStore.hasMatchableAnnotations;

// Guard placed after all hooks so hook order stays stable across renders
// (Rules of Hooks). All hooks above are null-safe when assets is empty.
Expand Down Expand Up @@ -1106,9 +1104,9 @@ export const ImageModal = observer(
backgroundColor={themeColor?.wildMeColors?.cyan700}
borderColor={themeColor?.wildMeColors?.cyan700}
target={true}
disabled={!hasNonTrivialAnnotations}
disabled={!hasMatchableAnnotations}
onClick={() => {
if (!hasNonTrivialAnnotations) {
if (!hasMatchableAnnotations) {
return;
}
if (
Expand Down
10 changes: 4 additions & 6 deletions frontend/src/pages/Encounter/ImageCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,7 @@ const ImageCard = observer(({ store = {} }) => {
};
const handleLeave = () => setTip({ show: false, x: 0, y: 0, text: "" });

const hasNonTrivialAnnotations = store.encounterAnnotations?.some(
(a) => !a.isTrivial && (a.boundingBox?.[2] || 0) > 0 && (a.boundingBox?.[3] || 0) > 0
);
const hasMatchableAnnotations = store.hasMatchableAnnotations;

useEffect(() => {
if (
Expand Down Expand Up @@ -790,7 +788,7 @@ const ImageCard = observer(({ store = {} }) => {
<div
className="d-flex align-items-center justify-content-center flex-column"
onClick={() => {
if (!hasNonTrivialAnnotations) return;
if (!hasMatchableAnnotations) return;

if (
!store.encounterData?.mediaAssets?.[store.selectedImageIndex]
Expand All @@ -801,9 +799,9 @@ const ImageCard = observer(({ store = {} }) => {
store.modals.setOpenMatchCriteriaModal(true);
}}
style={{
cursor: hasNonTrivialAnnotations ? "pointer" : "not-allowed",
cursor: hasMatchableAnnotations ? "pointer" : "not-allowed",
paddingTop: "20px",
opacity: hasNonTrivialAnnotations ? 1 : 0.5,
opacity: hasMatchableAnnotations ? 1 : 0.5,
}}
>
<RefreshIcon />
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/pages/Encounter/stores/EncounterStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,18 @@ class EncounterStore {
);
}

get hasMatchableAnnotations() {
const annotations =
this.encounterData?.mediaAssets?.[this._selectedImageIndex]?.annotations ||
[];
return annotations.some(
(a) =>
a?.encounterId === this.encounterData?.id &&
a?.matchAgainst === true &&
a?.acmId != null,
);
}

get selectedAnnotationId() {
return this._selectedAnnotationId;
}
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/pages/Encounter/stores/ImageModalStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ class ImageModalStore {
return this.encounterStore.encounterAnnotations;
}

get hasMatchableAnnotations() {
return this.encounterStore.hasMatchableAnnotations;
}

get selectedAnnotationId() {
return this.encounterStore.selectedAnnotationId;
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/ecocean/Encounter.java
Original file line number Diff line number Diff line change
Expand Up @@ -4481,6 +4481,8 @@ public void opensearchDocumentSerializer(JsonGenerator jgen, Shepherd myShepherd
jgen.writeStringField("iaClass", ann.getIAClass());
jgen.writeStringField("viewpoint", ann.getViewpoint());
jgen.writeBooleanField("isTrivial", ann.isTrivial());
jgen.writeBooleanField("matchAgainst", ann.getMatchAgainst());
jgen.writeStringField("acmId", ann.getAcmId());
jgen.writeNumberField("theta", ann.getTheta());
jgen.writeArrayFieldStart("boundingBox");
Feature ft = ann.getFeature(); // attempt force loading features for getBbox()
Expand Down
Loading
Loading