Fix/interface bugs - #36
Merged
Merged
Conversation
persist_settings() rewrote the whole settings.json from the caller's list, so call sites that only pass a subset of keys (class list upload, MATLAB ZIP export) silently erased every other saved setting. Settings then reverted to defaults on the next app start. Add update_settings_file(), a read-merge-write helper (NULL removes a key), and route persist_settings() through it so partial writes are safe regardless of which keys a call site knows about.
Startup restore and auto-save of the class list were gated on
grepl("sqlite", save_format), which is FALSE for the value "both".
With 'Both SQLite and MAT' selected, class list edits were never written
to the database and never restored on the next start, even though
annotations were being saved to that same database. Use
save_format %in% c("sqlite", "both") like every other call site.
Add a testServer regression test covering both the auto-save and the
startup restore paths.
Predict rebaselines original_classifications to the prediction result, so the current-vs-original diff used to protect manually reclassified images forgot every relabel made before an earlier prediction. A second Predict click therefore re-classified those images, silently overwriting the user's corrections. Also protect every file recorded in the changes log, which tracks all manual relabels since the sample was loaded and survives sample caching, and count skipped images against the files actually present.
External class review sets current_sample to the synthetic name "__external_review__", but leaving class review mode only cleared the class_review_* fields. Switching back to Sample Mode and loading a sample (or closing the app after relabeling) then ran the auto-save path, which wrote a bogus __external_review__ sample into the annotations database, polluted the Class Review filter dropdowns, and copied the external PNGs into the export folder. Clear the loaded review data when leaving class review mode, and guard save_to_cache() and the session-end save loop against class review state as defense in depth.
The Rematch Unmatched handler assigned build_worms_match_rows() results
(9 columns) into an explicit list of 7 column names, so [<-.data.frame
always errored ('provided 9 variables to replace 7 variables') and the
button never updated the match table. Assign by the result's own column
names instead, which also keeps the positional alignment correct.
save_sample_annotations() swallowed every error into the same FALSE it returns for an empty changes log, so a failed save (e.g. MAT backend with Python unavailable) was indistinguishable from 'nothing to save' and surfaced in the UI as a benign warning. Let errors propagate — all callers (Save button, navigation autosave, session cleanup) already wrap the call in tryCatch with proper error reporting. The temp folder is now cleaned up via on.exit so it is removed on failure too.
The Save button derived adc_folder solely from the ROI file map and aborted with 'Cannot find ROI data folder' whenever the sample had no .roi file — even for SQLite saves, which never use adc_folder. PNG-only samples could therefore only be saved implicitly via navigation autosave, which already had a PNG-folder fallback. Expose find_sample_png_dir() from the sample loading module, use it as the same fallback in the Save button handler, and only require an ADC folder when a MAT export is actually requested. The 'no changes' notification now reflects that failures are reported separately (save errors propagate and show as errors since the previous commit).
The drag-select rubber band (#selection-box) is position:fixed, i.e.
viewport-anchored, but was positioned with pageX/pageY (document
coordinates). Once the gallery was scrolled, the box appeared offset
below the cursor by the scroll distance and selected the wrong images.
Use clientX/clientY to match the fixed positioning.
updateCardStyle() compared card.data('relabeled') to the string 'true',
but jQuery's .data() auto-converts 'true' to a boolean, so the check
always failed and deselecting a relabeled image dropped its yellow
border. Read the attribute directly instead.
Three gallery fixes: - Empty image lists: images[1:0, ] on a 0-row data frame returns a phantom 1-row all-NA frame (1:0 is c(1, 0)), so the 'No images to display' branch was unreachable and page_info printed 'Page 1/1 (1-0 of 0)'. Return an explicit empty pagination result instead. - Page navigation desync: the pagination clamp was display-only, so after the image list shrank (e.g. relabeling away the last page of a filtered class) rv$current_page stayed out of range and the first prev-page click did nothing. Navigate from the clamped page actually displayed. - Missing score column: a classification CSV without a score column made the caption code evaluate if (logical(0)), killing the whole gallery render. Backfill score in load_from_csv() like the other optional columns, and guard the call site.
With zero images loaded, the annotation-mode indicator computed
round((0/0)*100) = NaN, and sprintf('%d%%', NaN) is an R error, so the
whole title bar failed to render (taking the mode-switch link with it).
Show 0% when there are no images.
load_from_cache() had two related defects: - It never cleared rv$cached_validation_classifications (unlike finalize_sample_load), so switching a cached sample to validation mode could restore the previous sample's classifications into it — every tile showed 'Not found' and a save would have written sample A's rows under sample B's name. - It mutated classifications/current_sample before checking that the ROI file still exists, so a failed cache load (moved/unmounted data) left the gallery pointing at the previous sample's image folder. Validate the image source first, clear the stale validation snapshot, and reset the page to 1 like every other sample-entry path.
A failed or empty model fetch (endpoint down, sleeping HF space) cleared the cfg_prediction_model widget, and Apply then persisted the empty value - opening Settings while offline and saving anything wiped the model and left the Predict button permanently disabled. Failure paths now keep the previously configured model as the only choice, and apply_settings falls back to the stored model when the select sends NULL. Deliberately clearing the Gradio URL still clears the model.
The app modules use base R's %||% (added in 4.4.0). On older R this surfaced as a raw 'could not find function' error at runtime instead of an installation-time message.
- An emptied instrument filter exported the entire database: NULL from a cleared multi-select was treated as 'no filter', the opposite of the widget's 'remove to exclude' semantics. When more than one instrument exists (i.e. the filter widget was shown), an empty selection now aborts with a warning. With a single instrument the widget is never rendered and NULL still means no filter. - Classes added during PNG import were appended to rv$class2use but never pushed to the 'Relabel to:' dropdown, so they were unusable until the class editor was opened. Refresh the dropdown like every other class2use mutation site. - Dashboard export loops did a bare 'next' when the skip-class filter emptied a sample, so the exported/failed/skipped counts did not sum to the samples processed. Count those samples as skipped.
With 'Rename classes' checked, the AphiaID map was keyed by the new accepted names while the taxonomy lookups passed to save_worms_map() stayed keyed by the old class names, so save_class_taxonomy_db() resolved NA for every renamed class - their scientific_name, accepted_name and accepted_aphia_id columns were left empty and the EcoTaxa hierarchy export was blank for exactly those classes. Re-key the match rows by the same effective names as the map.
The summary table gated on any score being present, but computed mean/min/max per class with na.rm = TRUE, so a class whose images all lack scores (e.g. manually relabeled before prediction) rendered as NaN%, Inf% and -Inf%. Such classes now show NA.
- Random/prev/next updated the server-side sample dropdown with a bare updateSelectizeInput(selected=), which can only select values already on the client (capped at ~1000 options). With larger filtered lists the dropdown went blank and (for random) the following Load click was swallowed. Route all of them through update_sample_list(), which does a full server-side update and already consumes pending_sample_select. - populate_from_index() forced the year filter back to the earliest year and the instrument filter to All on every rescan, so saving an unrelated setting yanked the user's filters. Preserve current selections when still valid (isolated reads, so the scan observer gains no new reactive dependencies). - The annotated-samples cache observer persisted the reactive value even when the startup scan had bailed early and it still held its empty initial value, stripping the on-disk index of all annotation markers. Skip persisting until samples have been discovered. - Local sample loads emitted two 'N images' toasts with different counts (pre- and post-extraction filter). Only the post-filter count is shown now, matching dashboard loads. - Session-end autosave passed the current sample's PNG folder for every cached sample, so PNG/MAT export silently failed for all but the last-viewed one. Cache entries now carry their own PNG folder. The cleanup also no longer deletes the persistent dashboard PNG cache (same startsWith guard as every other cleanup site), and an empty annotator is normalized to 'Unknown' in both autosave paths instead of writing ''.
anderstorstensson
added a commit
that referenced
this pull request
Aug 26, 2026
anderstorstensson
added a commit
that referenced
this pull request
Aug 27, 2026
* docs: add NEWS entries for interface bug review (#36) * perf: make gallery image selection instant Selecting images (click, drag-select, or the Select Page/All and Deselect All buttons) re-rendered the entire page of image cards on the server for every selection change, greying out the gallery while Shiny recomputed the HTML and the browser rebuilt the DOM - purely to apply a border color the client-side JS had already drawn. - gallery renderUI now isolates rv$selected_images; click/drag styling is handled client-side as before, so selection changes no longer invalidate the render - Select Page / Select All / Deselect All sync card styling via a new setSelectedCards custom message instead of a full re-render - relabeling and all sample/mode switches also reassign rv$classifications in the same observer, so those re-renders pick up the cleared selection with no extra sync - remaining renders are faster: per-card which() scan over original_classifications replaced with a named-vector lookup built once per render, and per-card selection membership vectorized * fix: keep gallery card size constant across selection states The selected (3px blue) and relabeled (3px yellow) borders made cards 4px wider/taller than the default 1px border, so selecting an image could push the last card of a flex row onto the next line, shifting image positions mid-selection. Move all card visual state from inline styles (server render + updateCardStyle JS) into .image-card CSS rules where border + padding always sum to 8px, so state toggles change color only, never geometry. This also lets the JS drop updateCardStyle and the data-relabeled attribute entirely: deselecting simply reveals the class-based yellow border again.
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
1 participant
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.
No description provided.