From 867db309af3e181ea5000a473cd13c8317d90bff Mon Sep 17 00:00:00 2001 From: JasonWildMe Date: Sun, 26 Apr 2026 13:22:40 -0700 Subject: [PATCH 1/3] Fix Feature.parameters loading after cross-PM reads Feature.parametersAsString was mapped via with no backing String field, so DataNucleus' bytecode enhancement persisted the value without invoking the user-defined setParametersAsString on load. The in-memory JSONObject parameters field stayed null, and getBbox() returned null on any PM that wasn't the one that created the Feature in memory. The encounter API serializer then emitted boundingBox: [] even though the DB row had correct width/height/x/y JSON. Fix mirrors the working MediaAsset.parameters pattern: - Add String parametersAsString field to Feature.java - Change package.jdo from to - getParameters() lazy-parses from parametersAsString when null - setParameters() keeps both fields in sync - get/setParametersAsString operate on the new field DB column unchanged. Existing rows load correctly with no migration. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/main/java/org/ecocean/media/Feature.java | 31 +++++++++++-------- .../resources/org/ecocean/media/package.jdo | 4 +-- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/ecocean/media/Feature.java b/src/main/java/org/ecocean/media/Feature.java index 62ca63f7d4..6da114baf2 100644 --- a/src/main/java/org/ecocean/media/Feature.java +++ b/src/main/java/org/ecocean/media/Feature.java @@ -23,6 +23,7 @@ public class Feature implements java.io.Serializable { protected FeatureType type; protected JSONObject parameters; + protected String parametersAsString; // this link back to the objs with .features that include us protected Annotation annotation; @@ -52,6 +53,7 @@ public Feature(final String id, final FeatureType type, final JSONObject params) this.id = id; this.type = type; this.parameters = params; + if (params != null) this.parametersAsString = params.toString(); this.setRevision(); } @@ -90,31 +92,34 @@ public MediaAsset getMediaAsset() { } public JSONObject getParameters() { -// System.out.println("getParameters() called -> " + parameters); + if (parameters != null) return parameters; + if (parametersAsString == null) return null; + try { + parameters = new JSONObject(parametersAsString); + } catch (JSONException je) { + System.out.println(this + " -- error parsing parameters json string (" + + parametersAsString + "): " + je.toString()); + return null; + } return parameters; } public void setParameters(JSONObject p) { -// System.out.println("setParameters(" + p + ") called"); parameters = p; + parametersAsString = (p == null) ? null : p.toString(); } + // only DataNucleus should be calling get/setParametersAsString. always use get/setParameters() instead. public String getParametersAsString() { -// System.out.println("getParametersAsString() called -> " + parameters); + if (parametersAsString != null) return parametersAsString; if (parameters == null) return null; - return parameters.toString(); + parametersAsString = parameters.toString(); + return parametersAsString; } public void setParametersAsString(String p) { -// System.out.println("setParametersAsString(" + p + ") called"); - if (p == null) return; - try { - parameters = new JSONObject(p); - } catch (JSONException je) { - System.out.println(this + " -- error parsing parameters json string (" + p + "): " + - je.toString()); - parameters = null; - } + parametersAsString = p; + parameters = null; // force lazy re-parse on next getParameters() } public long getRevision() { diff --git a/src/main/resources/org/ecocean/media/package.jdo b/src/main/resources/org/ecocean/media/package.jdo index 6738f60044..5a98162ac5 100755 --- a/src/main/resources/org/ecocean/media/package.jdo +++ b/src/main/resources/org/ecocean/media/package.jdo @@ -116,9 +116,9 @@ - + - + From 909bdd817dc809dcf51f846236022a44d61ccbac Mon Sep 17 00:00:00 2001 From: JasonWildMe Date: Sun, 26 Apr 2026 13:22:54 -0700 Subject: [PATCH 2/3] Exclude /api/ from service worker NetworkFirst caching The catchall registerRoute(() => true, new NetworkFirst()) was caching every fetch including the /api/v3/encounters/ polling responses. NetworkFirst falls back to cache on slow network or transient errors, so a poll that briefly returned an empty annotations array got cached and replayed to subsequent polls and even hard refreshes, masking correct backend state. API responses are not idempotent (encounter state changes during detection) and should never go through SW caching. Static assets and navigations still benefit from the strategy. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/src/service-worker.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/service-worker.js b/frontend/src/service-worker.js index f677fe0cef..58d1c2dff6 100644 --- a/frontend/src/service-worker.js +++ b/frontend/src/service-worker.js @@ -68,9 +68,10 @@ registerRoute( }), ); +// Never cache API endpoints — responses are not idempotent and stale data +// (e.g. boundingBox, detectionStatus) breaks polling-driven UI updates. registerRoute( - // ({url}) => true, - () => true, + ({ url }) => !url.pathname.startsWith("/api/"), new NetworkFirst(), ); From adeb87e52d3e011741956502f5a9d06319cbf586 Mon Sep 17 00:00:00 2001 From: JasonWildMe Date: Sun, 26 Apr 2026 13:23:10 -0700 Subject: [PATCH 3/3] Fix encounter polling lifecycle for bulk-import detection Three issues prevented the bounding box from rendering after detection completes for bulk-imported encounters: 1. setMediaAssets mutated _encounterData.mediaAssets in place. React useEffects keyed on store.encounterData saw the same reference and never re-ran, so rects/imageReady never updated when polling returned new annotations. Switched to immutable update: replace _encounterData with a new object. 2. The polling predicate treated null detectionStatus as terminal. Bulk imports create encounters with a trivial placeholder annotation and detectionStatus=null while detection is queued to WBIA. The first poll saw null, stopped polling, and never noticed when detection actually completed. Added isAwaitingBulkImportDetection helper that keeps polling alive when detectionStatus is null, importTaskId is set, and only trivial annotations exist. 3. ImageCard's "Detection in progress" indicator used the same broken predicate. Now uses the shared helper so the spinner shows during the WBIA wait. Centralized the predicates in pollingHelpers.js so Encounter.jsx and ImageCard.jsx can't drift. Added MAX_POLL_CYCLES=100 (~5 min at 3s interval) safety cap so encounters that never get detection don't poll forever. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/src/pages/Encounter/Encounter.jsx | 34 ++++++--------- frontend/src/pages/Encounter/ImageCard.jsx | 11 ++--- .../src/pages/Encounter/pollingHelpers.js | 43 +++++++++++++++++++ .../pages/Encounter/stores/EncounterStore.js | 2 +- 4 files changed, 59 insertions(+), 31 deletions(-) create mode 100644 frontend/src/pages/Encounter/pollingHelpers.js diff --git a/frontend/src/pages/Encounter/Encounter.jsx b/frontend/src/pages/Encounter/Encounter.jsx index 232285b7ef..0ab979e088 100644 --- a/frontend/src/pages/Encounter/Encounter.jsx +++ b/frontend/src/pages/Encounter/Encounter.jsx @@ -38,6 +38,11 @@ import { Divider } from "antd"; import { get } from "lodash-es"; import CollabModal from "./CollabModal"; import Alert from "react-bootstrap/Alert"; +import { + shouldContinuePollingEncounter, + POLL_INTERVAL_MS, + MAX_POLL_CYCLES, +} from "./pollingHelpers"; const Encounter = observer(() => { const [store] = useState(() => new EncounterStore()); @@ -72,26 +77,7 @@ const Encounter = observer(() => { useEffect(() => { let cancelled = false; let timeoutId = null; - - const isTerminalDetectionStatus = (status) => - !status || - status === "complete" || - status === "error" || - status === "pending"; - - const shouldContinuePolling = (encounterData) => { - const mediaAssets = Array.isArray(encounterData?.mediaAssets) - ? encounterData.mediaAssets - : []; - - if (mediaAssets.length === 0) { - return false; - } - - return mediaAssets.some( - (asset) => !isTerminalDetectionStatus(asset?.detectionStatus), - ); - }; + let pollCount = 0; const fetchEncounter = async () => { try { @@ -108,8 +94,12 @@ const Encounter = observer(() => { store.setMediaAssets(res.data.mediaAssets); } - if (shouldContinuePolling(res.data)) { - timeoutId = window.setTimeout(fetchEncounter, 3000); + pollCount++; + if ( + pollCount < MAX_POLL_CYCLES && + shouldContinuePollingEncounter(res.data) + ) { + timeoutId = window.setTimeout(fetchEncounter, POLL_INTERVAL_MS); } } catch (_err) { if (!cancelled && isInitialLoad.current) { diff --git a/frontend/src/pages/Encounter/ImageCard.jsx b/frontend/src/pages/Encounter/ImageCard.jsx index 7a85be934a..c4f0c3fc95 100644 --- a/frontend/src/pages/Encounter/ImageCard.jsx +++ b/frontend/src/pages/Encounter/ImageCard.jsx @@ -14,6 +14,7 @@ import Tooltip from "../../components/ToolTip"; import axios from "axios"; import { useIntl } from "react-intl"; import SpotMappingIcon2 from "../../components/icons/SpotMappingIcon2"; +import { isAssetActivelyAwaitingDetection } from "./pollingHelpers"; const ImageCard = observer(({ store = {} }) => { const imgRef = useRef(null); @@ -67,17 +68,11 @@ const ImageCard = observer(({ store = {} }) => { JSON.stringify(editAnnotationParams), ); - const isTerminalDetectionStatus = (status) => - !status || - status === "complete" || - status === "error" || - status === "pending"; - const selectedAsset = store.encounterData?.mediaAssets?.[store.selectedImageIndex]; - const selectedAssetDetectionStatus = selectedAsset?.detectionStatus; const isDetectionInProgress = - !!selectedAsset && !isTerminalDetectionStatus(selectedAssetDetectionStatus); + !!selectedAsset && + isAssetActivelyAwaitingDetection(selectedAsset, store.encounterData); const handleEnter = (text) => setTip((s) => ({ ...s, show: true, text })); const handleMove = (e) => { diff --git a/frontend/src/pages/Encounter/pollingHelpers.js b/frontend/src/pages/Encounter/pollingHelpers.js new file mode 100644 index 0000000000..af3b941c61 --- /dev/null +++ b/frontend/src/pages/Encounter/pollingHelpers.js @@ -0,0 +1,43 @@ +// Shared detection-status predicates used by Encounter.jsx polling loop and +// ImageCard.jsx "Detection in progress" indicator. Keeping them here avoids +// the two files drifting out of sync. + +const POLL_INTERVAL_MS = 3000; +const MAX_POLL_CYCLES = 100; // 100 cycles * 3s = ~5 minutes + +const isAnnotationTrivial = (a) => a?.isTrivial === true || a?.trivial === true; + +export const isTerminalDetectionStatus = (status) => + !status || + status === "complete" || + status === "error" || + status === "pending"; + +// True when an asset is from a bulk import and detection has been queued +// to WBIA but the callback hasn't returned yet. In this state the API +// returns detectionStatus=null and only a trivial placeholder annotation. +// Scoped to encounters that have an importTaskId so we don't poll forever +// on legacy/manual encounters that intentionally never run detection. +export const isAwaitingBulkImportDetection = (asset, encounterData) => { + if (!encounterData?.importTaskId) return false; + if (asset?.detectionStatus !== null && asset?.detectionStatus !== undefined) + return false; + const anns = Array.isArray(asset?.annotations) ? asset.annotations : []; + return anns.length > 0 && anns.every(isAnnotationTrivial); +}; + +export const isAssetActivelyAwaitingDetection = (asset, encounterData) => + !isTerminalDetectionStatus(asset?.detectionStatus) || + isAwaitingBulkImportDetection(asset, encounterData); + +export const shouldContinuePollingEncounter = (encounterData) => { + const mediaAssets = Array.isArray(encounterData?.mediaAssets) + ? encounterData.mediaAssets + : []; + if (mediaAssets.length === 0) return false; + return mediaAssets.some((asset) => + isAssetActivelyAwaitingDetection(asset, encounterData), + ); +}; + +export { POLL_INTERVAL_MS, MAX_POLL_CYCLES }; diff --git a/frontend/src/pages/Encounter/stores/EncounterStore.js b/frontend/src/pages/Encounter/stores/EncounterStore.js index bdc491626b..7796dbdb8e 100644 --- a/frontend/src/pages/Encounter/stores/EncounterStore.js +++ b/frontend/src/pages/Encounter/stores/EncounterStore.js @@ -162,7 +162,7 @@ class EncounterStore { setMediaAssets(mediaAssets) { if (this._encounterData) { - this._encounterData.mediaAssets = mediaAssets; + this._encounterData = { ...this._encounterData, mediaAssets }; } }