diff --git a/docs/superpowers/plans/2026-07-01-react-matchable-unity-annotations.md b/docs/superpowers/plans/2026-07-01-react-matchable-unity-annotations.md new file mode 100644 index 0000000000..9760d2007b --- /dev/null +++ b/docs/superpowers/plans/2026-07-01-react-matchable-unity-annotations.md @@ -0,0 +1,347 @@ +# Restore Matchability of Spot-Crop (Unity) Annotations — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make genuinely-matchable whole-image spot-crop annotations usable from the React Encounter page by gating the "New Match" button on real matchability (`matchAgainst && acmId`) instead of an `isTrivial`/bbox geometry proxy. + +**Architecture:** Backend serializes the real matchability signal (`matchAgainst`, `acmId`) into each annotation in the encounter API response (`/api/v3/encounters/{id}`). The React frontend adds a `hasMatchableAnnotations` store getter computed from the raw annotations and gates the New Match button on it, leaving the existing `encounterAnnotations` getter and all other flows untouched. + +**Tech Stack:** Java (Jackson JsonGenerator serialization, JUnit 5 + Mockito), React/MobX (Jest + React Testing Library). + +## Global Constraints + +- Base branch: `origin/main`; target GitHub Milestone 10.12. Working branch: `fix/react-matchable-unity-annotations` (already created). +- Matchability predicate is exactly `matchAgainst === true && acmId` (truthy) — mirrors backend eligibility (`matchAgainst=true AND acmId IS NOT NULL`). +- Line endings: this repo is LF. Before every `git add`, run `grep -c $'\r' ` and expect `0`; if not, `sed -i 's/\r$//' `. +- JUnit 5 assertion message goes LAST: `assertEquals(expected, actual)` / `assertTrue(cond, "msg")`. +- Do NOT modify: `EncounterStore.encounterAnnotations` getter, the `ImageCard` boundary-drawing `rects` filter, `NewMatchStore.annotationIds`, or `SubmitSpotsAndTransformImage.java`. +- CI hides jest failures (continue-on-error); always run jest locally and read the result. + +--- + +### Task 1: Backend — serialize `matchAgainst` and `acmId` per annotation + +**Files:** +- Modify: `src/main/java/org/ecocean/Encounter.java` (annotation loop in `opensearchDocumentSerializer`, ~lines 4467-4472) +- Test: `src/test/java/org/ecocean/api/EncounterApiTest.java` (extend `encounterApiGetTest`, ~lines 211-237) + +**Interfaces:** +- Consumes: `Annotation.getMatchAgainst()` → `boolean` (`Annotation.java:737`), `Annotation.getAcmId()` → `String` (`Annotation.java:328`). +- Produces: each object in the response `mediaAssets[].annotations[]` now has `matchAgainst` (boolean) and `acmId` (string, may be JSON null). Frontend Task 2 relies on these keys. + +- [ ] **Step 1: Add failing assertions + stubs to the existing serialization test** + +In `src/test/java/org/ecocean/api/EncounterApiTest.java`, inside `encounterApiGetTest`, add two stubs next to the existing annotation stubs (after `Annotation.java:214`'s `when(mockAnnot.getId())...`): + +```java + when(mockAnnot.getMatchAgainst()).thenReturn(true); + when(mockAnnot.getAcmId()).thenReturn("test-acm-id"); +``` + +Then, immediately after the existing `"id"` assertion (currently ending at line 237), add: + +```java + assertEquals(true, + res.getJSONArray("mediaAssets").getJSONObject(1).getJSONArray( + "annotations").getJSONObject(0).getBoolean("matchAgainst")); + assertEquals("test-acm-id", + res.getJSONArray("mediaAssets").getJSONObject(1).getJSONArray( + "annotations").getJSONObject(0).getString("acmId")); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `mvn test -Dtest=EncounterApiTest#encounterApiGetTest` +Expected: FAIL — `JSONObject["matchAgainst"] not found.` (fields not yet serialized). + +- [ ] **Step 3: Implement the serialization** + +In `src/main/java/org/ecocean/Encounter.java`, in the annotation loop of `opensearchDocumentSerializer`, immediately after the `isTrivial` line (`jgen.writeBooleanField("isTrivial", ann.isTrivial());`, ~line 4471) add: + +```java + jgen.writeBooleanField("matchAgainst", ann.getMatchAgainst()); + jgen.writeStringField("acmId", ann.getAcmId()); +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `mvn test -Dtest=EncounterApiTest#encounterApiGetTest` +Expected: PASS (`BUILD SUCCESS`, Tests run: 1, Failures: 0). + +- [ ] **Step 5: Commit** + +```bash +cd /mnt/c/Wildbook-clean2 +for f in src/main/java/org/ecocean/Encounter.java src/test/java/org/ecocean/api/EncounterApiTest.java; do grep -c $'\r' "$f"; done +git add src/main/java/org/ecocean/Encounter.java src/test/java/org/ecocean/api/EncounterApiTest.java +git commit -m "feat: serialize matchAgainst and acmId in encounter API annotations + +Encounter.opensearchDocumentSerializer now emits matchAgainst and acmId per +annotation so the React Encounter page can gate matching on real eligibility +instead of an isTrivial/bbox geometry proxy. Additive, served live via +jsonForApiGet (no reindex). + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 2: Frontend — `hasMatchableAnnotations` getter + ImageModalStore delegation + +**Files:** +- Modify: `frontend/src/pages/Encounter/stores/EncounterStore.js` (add getter after `encounterAnnotations`, ~line 613) +- Modify: `frontend/src/pages/Encounter/stores/ImageModalStore.js` (add delegating getter after `encounterAnnotations`, ~line 41) +- Test: `frontend/src/__tests__/pages/Encounter/EncounterStore.test.js` (new tests in the existing "Encounter Annotations / Match Result Clickable" describe block) + +**Interfaces:** +- Consumes: `encounterData.mediaAssets[selectedImageIndex].annotations[]` with `{ encounterId, matchAgainst, acmId }` (from Task 1). +- Produces: `EncounterStore.hasMatchableAnnotations` → `boolean`; `ImageModalStore.hasMatchableAnnotations` → `boolean` (delegates). Tasks 3 consume these. + +- [ ] **Step 1: Write the failing tests** + +In `frontend/src/__tests__/pages/Encounter/EncounterStore.test.js`, add these tests inside the `describe("Encounter Annotations / Match Result Clickable", ...)` block (after the existing `it("filters encounter annotations ...")`): + +```javascript + 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); + }); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cd frontend && npx jest src/__tests__/pages/Encounter/EncounterStore.test.js -t "hasMatchableAnnotations"` +Expected: FAIL (getter returns `undefined`, not `true`/`false`). + +- [ ] **Step 3: Add the getter to EncounterStore** + +In `frontend/src/pages/Encounter/stores/EncounterStore.js`, add immediately after the `encounterAnnotations` getter (after the closing `}` at ~line 613): + +```javascript + get hasMatchableAnnotations() { + const annotations = + this.encounterData?.mediaAssets?.[this._selectedImageIndex]?.annotations || + []; + return annotations.some( + (a) => + a?.encounterId === this.encounterData?.id && + a?.matchAgainst === true && + !!a?.acmId, + ); + } +``` + +- [ ] **Step 4: Add the delegating getter to ImageModalStore** + +In `frontend/src/pages/Encounter/stores/ImageModalStore.js`, add immediately after the `encounterAnnotations` getter (after its closing `}` at ~line 41): + +```javascript + get hasMatchableAnnotations() { + return this.encounterStore.hasMatchableAnnotations; + } +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cd frontend && npx jest src/__tests__/pages/Encounter/EncounterStore.test.js -t "hasMatchableAnnotations"` +Expected: PASS (3 passed). + +- [ ] **Step 6: Commit** + +```bash +cd /mnt/c/Wildbook-clean2 +for f in frontend/src/pages/Encounter/stores/EncounterStore.js frontend/src/pages/Encounter/stores/ImageModalStore.js frontend/src/__tests__/pages/Encounter/EncounterStore.test.js; do grep -c $'\r' "$f"; done +git add frontend/src/pages/Encounter/stores/EncounterStore.js frontend/src/pages/Encounter/stores/ImageModalStore.js frontend/src/__tests__/pages/Encounter/EncounterStore.test.js +git commit -m "feat: add hasMatchableAnnotations store getter + +Computes matchability from raw annotations (matchAgainst && acmId), filtered +by encounterId, without the isTrivial/bbox filter of encounterAnnotations. +ImageModalStore delegates to it. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 3: Frontend — wire the New Match gate in ImageCard and ImageModal + +**Files:** +- Modify: `frontend/src/pages/Encounter/ImageCard.jsx` (~lines 107-109 and usages ~793, 804, 806) +- Modify: `frontend/src/components/ImageModal.jsx` (~lines 216-218 and usages ~1109, 1111) +- Test: `frontend/src/__tests__/pages/Encounter/ImageCard.test.js` (`makeStore` default + new disabled test) +- Test: `frontend/src/__tests__/pages/Encounter/ImageModal.test.js` (`makeImageStore` default + new disabled test) + +**Interfaces:** +- Consumes: `store.hasMatchableAnnotations` (ImageCard) and `imageStore.hasMatchableAnnotations` (ImageModal) from Task 2. +- Produces: New Match button enabled iff `hasMatchableAnnotations` is truthy. + +- [ ] **Step 1: Write failing tests** + +In `frontend/src/__tests__/pages/Encounter/ImageCard.test.js`, add `hasMatchableAnnotations: true,` to the `makeStore` defaults object (after `matchResultClickable: false,`, ~line 116). Then add this test after `test("clicking NEW_MATCH opens match criteria modal", ...)` (~line 201): + +```javascript + 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(); + }); +``` + +In `frontend/src/__tests__/pages/Encounter/ImageModal.test.js`, add `hasMatchableAnnotations: true,` to the `makeImageStore` defaults object (after `matchResultClickable: true,`, ~line 103). Then add this test after `test("match results button is disabled when matchResultClickable is false", ...)` (~line 243): + +```javascript + 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(); + }); +``` + +Note: the ImageCard fail-first test above already works without this trick — `makeStore`'s default `encounterAnnotations` (`baseEncounterData` annotation with `boundingBox: [10,20,100,40]`) makes the OLD gate true, so the NEW_MATCH click fires before the impl and the `not.toHaveBeenCalled()` assertion fails first. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cd frontend && npx jest src/__tests__/pages/Encounter/ImageCard.test.js src/__tests__/pages/Encounter/ImageModal.test.js -t "NEW_MATCH"` +Expected: FAIL — ImageCard still reads the old computed `hasNonTrivialAnnotations` (from `encounterAnnotations`), and ImageModal's button still keys off the old variable. + +- [ ] **Step 3: Wire ImageCard** + +In `frontend/src/pages/Encounter/ImageCard.jsx`, replace lines 107-109: + +```javascript + const hasNonTrivialAnnotations = store.encounterAnnotations?.some( + (a) => !a.isTrivial && (a.boundingBox?.[2] || 0) > 0 && (a.boundingBox?.[3] || 0) > 0 + ); +``` + +with: + +```javascript + const hasMatchableAnnotations = store.hasMatchableAnnotations; +``` + +Then rename the three usages: at the `onClick` guard (`if (!hasNonTrivialAnnotations) return;`, ~line 793) and the two `style` references (`cursor:` ~804 and `opacity:` ~806), change `hasNonTrivialAnnotations` → `hasMatchableAnnotations`. + +- [ ] **Step 4: Wire ImageModal** + +In `frontend/src/components/ImageModal.jsx`, replace lines 216-218: + +```javascript + const hasNonTrivialAnnotations = imageStore.encounterAnnotations?.some( + (a) => !a.isTrivial && (a.boundingBox?.[2] || 0) > 0 && (a.boundingBox?.[3] || 0) > 0 + ); +``` + +with: + +```javascript + const hasMatchableAnnotations = imageStore.hasMatchableAnnotations; +``` + +Then rename the two usages: `disabled={!hasNonTrivialAnnotations}` (~line 1109) and `if (!hasNonTrivialAnnotations)` (~line 1111), change `hasNonTrivialAnnotations` → `hasMatchableAnnotations`. + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cd frontend && npx jest src/__tests__/pages/Encounter/ImageCard.test.js src/__tests__/pages/Encounter/ImageModal.test.js` +Expected: PASS for the whole two files (existing NEW_MATCH-open test still passes via the `hasMatchableAnnotations: true` default; new disabled tests pass). + +- [ ] **Step 6: Verify no stray references remain** + +Run: `grep -rn "hasNonTrivialAnnotations" frontend/src` +Expected: no output (all renamed). + +- [ ] **Step 7: Commit** + +```bash +cd /mnt/c/Wildbook-clean2 +for f in frontend/src/pages/Encounter/ImageCard.jsx frontend/src/components/ImageModal.jsx frontend/src/__tests__/pages/Encounter/ImageCard.test.js frontend/src/__tests__/pages/Encounter/ImageModal.test.js; do grep -c $'\r' "$f"; done +git add frontend/src/pages/Encounter/ImageCard.jsx frontend/src/components/ImageModal.jsx frontend/src/__tests__/pages/Encounter/ImageCard.test.js frontend/src/__tests__/pages/Encounter/ImageModal.test.js +git commit -m "fix: gate New Match button on hasMatchableAnnotations + +ImageCard and ImageModal now enable New Match when the image has an +annotation with matchAgainst && acmId, instead of the isTrivial/bbox +geometry proxy that hid matchable whole-image spot-crop annotations. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Verification (after all tasks) + +- [ ] Backend: `mvn test -Dtest=EncounterApiTest#encounterApiGetTest` → PASS. +- [ ] Frontend: `cd frontend && npx jest src/__tests__/pages/Encounter/EncounterStore.test.js src/__tests__/pages/Encounter/ImageCard.test.js src/__tests__/pages/Encounter/ImageModal.test.js` → new tests PASS; note any pre-existing failures unrelated to this change (e.g. the `encounterAnnotations` bbox-filter test) and do not "fix" them here. +- [ ] `grep -rn "hasNonTrivialAnnotations" frontend/src` → empty. +- [ ] Codex review of the full diff; converge to no Major findings. +- [ ] Open PR against `main`, assign Milestone 10.12; PR body documents the out-of-scope residual (unity annotations remain non-selectable for re-opening past match results) and the cosmetic boundary behavior. + +## Notes / Known limitations (carry into PR body) + +- Whole-image crops have no sub-rectangle, so no boundary overlay is drawn — cosmetic, expected. +- The `encounterAnnotations` getter stays filtered, so unity annotations remain non-selectable (can't re-open past match results from the Encounter page). New Match itself works end-to-end because it navigates straight to the fresh task's results and `NewMatchStore.annotationIds` already includes the annotation. diff --git a/docs/superpowers/specs/2026-07-01-react-matchable-unity-annotations-design.md b/docs/superpowers/specs/2026-07-01-react-matchable-unity-annotations-design.md new file mode 100644 index 0000000000..b2b6d42672 --- /dev/null +++ b/docs/superpowers/specs/2026-07-01-react-matchable-unity-annotations-design.md @@ -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). diff --git a/frontend/src/__tests__/pages/Encounter/EncounterStore.test.js b/frontend/src/__tests__/pages/Encounter/EncounterStore.test.js index d60ebe56b0..e124922498 100644 --- a/frontend/src/__tests__/pages/Encounter/EncounterStore.test.js +++ b/frontend/src/__tests__/pages/Encounter/EncounterStore.test.js @@ -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", diff --git a/frontend/src/__tests__/pages/Encounter/ImageCard.test.js b/frontend/src/__tests__/pages/Encounter/ImageCard.test.js index 65f3f42155..155c8e14a5 100644 --- a/frontend/src/__tests__/pages/Encounter/ImageCard.test.js +++ b/frontend/src/__tests__/pages/Encounter/ImageCard.test.js @@ -114,6 +114,7 @@ const makeStore = (overrides = {}) => ({ setSelectedAnnotationId: jest.fn(), setIntl: jest.fn(), matchResultClickable: false, + hasMatchableAnnotations: true, modals: { setOpenMatchCriteriaModal: jest.fn(), }, @@ -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(); diff --git a/frontend/src/__tests__/pages/Encounter/ImageModal.test.js b/frontend/src/__tests__/pages/Encounter/ImageModal.test.js index bc3cca488e..2fa82912d7 100644 --- a/frontend/src/__tests__/pages/Encounter/ImageModal.test.js +++ b/frontend/src/__tests__/pages/Encounter/ImageModal.test.js @@ -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, @@ -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 }); diff --git a/frontend/src/components/ImageModal.jsx b/frontend/src/components/ImageModal.jsx index 28d9092b60..61ccfe7695 100644 --- a/frontend/src/components/ImageModal.jsx +++ b/frontend/src/components/ImageModal.jsx @@ -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. @@ -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 ( diff --git a/frontend/src/pages/Encounter/ImageCard.jsx b/frontend/src/pages/Encounter/ImageCard.jsx index e58b1423ee..dbd7f2bc0d 100644 --- a/frontend/src/pages/Encounter/ImageCard.jsx +++ b/frontend/src/pages/Encounter/ImageCard.jsx @@ -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 ( @@ -790,7 +788,7 @@ const ImageCard = observer(({ store = {} }) => {
{ - if (!hasNonTrivialAnnotations) return; + if (!hasMatchableAnnotations) return; if ( !store.encounterData?.mediaAssets?.[store.selectedImageIndex] @@ -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, }} > diff --git a/frontend/src/pages/Encounter/stores/EncounterStore.js b/frontend/src/pages/Encounter/stores/EncounterStore.js index 58a35ea441..d4e58b67c6 100644 --- a/frontend/src/pages/Encounter/stores/EncounterStore.js +++ b/frontend/src/pages/Encounter/stores/EncounterStore.js @@ -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; } diff --git a/frontend/src/pages/Encounter/stores/ImageModalStore.js b/frontend/src/pages/Encounter/stores/ImageModalStore.js index 25c178a8e7..d718819dcb 100644 --- a/frontend/src/pages/Encounter/stores/ImageModalStore.js +++ b/frontend/src/pages/Encounter/stores/ImageModalStore.js @@ -40,6 +40,10 @@ class ImageModalStore { return this.encounterStore.encounterAnnotations; } + get hasMatchableAnnotations() { + return this.encounterStore.hasMatchableAnnotations; + } + get selectedAnnotationId() { return this.encounterStore.selectedAnnotationId; } diff --git a/src/main/java/org/ecocean/Encounter.java b/src/main/java/org/ecocean/Encounter.java index 47e820297f..4ba248cc82 100644 --- a/src/main/java/org/ecocean/Encounter.java +++ b/src/main/java/org/ecocean/Encounter.java @@ -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() diff --git a/src/test/java/org/ecocean/api/EncounterApiTest.java b/src/test/java/org/ecocean/api/EncounterApiTest.java index a1afc0a994..06fe8b277f 100644 --- a/src/test/java/org/ecocean/api/EncounterApiTest.java +++ b/src/test/java/org/ecocean/api/EncounterApiTest.java @@ -212,6 +212,8 @@ class EncounterApiTest { when(mockAnnot.getMediaAsset()).thenReturn(mockMA); when(mockAnnot.getIdentificationStatus()).thenReturn("test"); when(mockAnnot.getId()).thenReturn("test-annot-id"); + when(mockAnnot.getMatchAgainst()).thenReturn(true); + when(mockAnnot.getAcmId()).thenReturn("test-acm-id"); when(myShepherd.getAnnotation(any(String.class))).thenReturn(mockAnnot); when(mockMA.hasAnnotations()).thenReturn(true); ArrayList anns = new ArrayList(); @@ -235,6 +237,12 @@ class EncounterApiTest { "test"); assertEquals(res.getJSONArray("mediaAssets").getJSONObject(1).getJSONArray( "annotations").getJSONObject(0).getString("id"), "test-annot-id"); + assertEquals(true, + res.getJSONArray("mediaAssets").getJSONObject(1).getJSONArray( + "annotations").getJSONObject(0).getBoolean("matchAgainst")); + assertEquals("test-acm-id", + res.getJSONArray("mediaAssets").getJSONObject(1).getJSONArray( + "annotations").getJSONObject(0).getString("acmId")); } } }