Fix bounding box not appearing on /react/encounter after detection#1559
Merged
JasonWildMe merged 4 commits intomainfrom May 1, 2026
Merged
Fix bounding box not appearing on /react/encounter after detection#1559JasonWildMe merged 4 commits intomainfrom
JasonWildMe merged 4 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1559 +/- ##
=======================================
Coverage 51.54% 51.54%
=======================================
Files 307 308 +1
Lines 11942 11952 +10
Branches 3836 3842 +6
=======================================
+ Hits 6155 6161 +6
- Misses 5502 5503 +1
- Partials 285 288 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Feature.parametersAsString was mapped via <property> 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 <property> to <field name="parametersAsString"> - 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) <noreply@anthropic.com>
The catchall registerRoute(() => true, new NetworkFirst()) was caching every fetch including the /api/v3/encounters/<id> 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
fef5ffe to
adeb87e
Compare
naknomum
approved these changes
May 1, 2026
Member
naknomum
left a comment
There was a problem hiding this comment.
code looks good, problem fixed in testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three independent bugs combined to prevent bounding boxes from rendering on
/react/encounterafter detection completed. Each is fixed in its own commit on this branch.Feature.parametersreturned null on cross-PM reads — the JDO mapping used<property>with no backing String field, so DataNucleus' bytecode enhancement persisted the value without invoking the user-definedsetParametersAsStringon load. The in-memoryparametersJSONObject stayed null,getBbox()returned null, and the encounter API serializer emittedboundingBox: []even though the DB row had correctwidth/height/x/yJSON./api/polling responses — the catchallregisterRoute(() => true, new NetworkFirst())caches every fetch. NetworkFirst falls back to cache on slow/erroring network, so a brief empty-bbox response could be replayed to subsequent polls and even hard refreshes.null detectionStatusas terminal, but bulk-imported encounters arrive withdetectionStatus = nullplus a trivial placeholder annotation while detection is queued in WBIA. Polling stopped on the first poll, never noticed when detection actually completed, and the in-placesetMediaAssetsmutation preventeduseEffects from re-running anyway.After all three fixes: backend correctly persists and serves bbox data, SW does not cache it, polling stays alive through the WBIA wait, React UI updates without a refresh.
Verified end-to-end by uploading bulk-import encounters on flakebook.wildme.org during this session: bounding boxes now appear automatically once WBIA returns, no manual refresh needed.
Test plan
mvn clean install— build succeeds with new JDO mapping. (Verified locally.)npm run buildinfrontend/— bundle compiles. (Verified locally — eslint clean on changed files.)importTaskIdand trivial-only annotations don't poll forever (max-poll cap of ~5 minutes serves as a safety net regardless).Out of scope (separate tickets recommended)
MediaAsset.derivationMethoduses the same broken<property>pattern as oldFeature.parameters. Likely silently broken; nothing user-visible currently depends on it.IndexingManager.addIndexingQueueEntryhas a dedup race where late updates during a bg index runnable's lifetime are silently dropped.BulkImport.doDeleteviolatesMATCHRESULT_FK1when deleting an import whose annotations are referenced fromMATCHRESULT.QueueUtil.backgroundhalts the entire queue consumer on the first exception ingetNext()— recommend catch-and-continue instead.detectionStatus = "queued"/"sent"server-side instead of leaving itnull. Would let us drop theisAwaitingBulkImportDetectionworkaround.