Guard channelCheckboxes parsing against malformed (non-dict) values

.gitignore

@@ -0,0 +1,7 @@
+# Python build/test artifacts
+__pycache__/
+*.py[cod]
+*$py.class
+.pytest_cache/
+.coverage
+*.egg-info/

annotation_utilities/annotation_utilities/annotation_tools.py

@@ -70,6 +70,35 @@ def get_annotations_with_tag(elements, tag, exclusive=False):
     return result
 
 
+def get_selected_channels(value, field_name='channel selection'):
+    """Parse a ``channelCheckboxes`` interface value into a sorted list of
+    selected channel indices (ints).
+
+    The expected (documented) shape is a dict mapping channel-index strings to
+    booleans, e.g. ``{"0": True, "1": False}`` -> ``[0]``. A missing value
+    (``None``) means nothing was selected and returns ``[]`` so callers can
+    apply their own required-field logic.
+
+    Any other shape -- notably a list such as ``[0]``, which has been observed
+    arriving from some clients -- is treated as a misconfiguration rather than
+    silently recovered: guessing a channel from an unexpected payload could run
+    a tool on data the user never intended to select. Such values raise
+    ``ValueError`` so the caller can surface a clear error to the user.
+
+    NOTE: the list form is undocumented; the front-end (NimbusImage repo) is the
+    source of truth for what ``channelCheckboxes`` actually serializes and
+    should be checked to fix the root cause.
+    """
+    if value is None:
+        return []
+    if isinstance(value, dict):
+        return sorted(int(k) for k, v in value.items() if v)
+    raise ValueError(
+        f"'{field_name}' has an unexpected format "
+        f"({type(value).__name__}: {value!r}); expected channel checkboxes "
+        f"(a mapping of channel index to on/off).")
+
+
 def find_matching_annotations_by_location(source, target_list, Time=True, XY=True, Z=True):
     """
     This function filters the target_list based on the 'location' of the source point.

annotation_utilities/tests/test_get_selected_channels.py

@@ -0,0 +1,40 @@
+import sys
+from pathlib import Path
+
+import pytest
+
+
+package_root = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(package_root))
+
+from annotation_utilities.annotation_tools import get_selected_channels
+
+
+def test_dict_returns_sorted_selected_channels():
+    assert get_selected_channels({'0': True, '1': False}) == [0]
+    assert get_selected_channels({'2': True, '0': True, '1': False}) == [0, 2]
+
+
+def test_empty_dict_returns_empty_list():
+    assert get_selected_channels({}) == []
+
+
+def test_none_returns_empty_list():
+    assert get_selected_channels(None) == []
+
+
+def test_all_false_dict_returns_empty_list():
+    assert get_selected_channels({'0': False, '1': False}) == []
+
+
+@pytest.mark.parametrize("bad_value", [[0], [], [0, 1], (0,), '0', 0, True])
+def test_non_dict_values_raise_value_error(bad_value):
+    # The list form ([0]) in particular has been seen in the wild; it must be
+    # rejected rather than silently recovered.
+    with pytest.raises(ValueError):
+        get_selected_channels(bad_value)
+
+
+def test_error_message_includes_field_name():
+    with pytest.raises(ValueError, match="Channels to correct"):
+        get_selected_channels([0], field_name='Channels to correct')

build_machine_learning_workers.sh

@@ -29,7 +29,7 @@ echo "Building Cellpose train worker"
 docker build ./workers/annotations/cellpose_train/ -t annotations/cellpose_train_worker:latest $NO_CACHE
 
 echo "Building Cellpose-SAM worker"
-docker build ./workers/annotations/cellposesam/ -t annotations/cellposesam_worker:latest $NO_CACHE
+docker build . -f ./workers/annotations/cellposesam/Dockerfile -t annotations/cellposesam_worker:latest $NO_CACHE
 
 echo "Building Stardist worker"
 docker build ./workers/annotations/stardist/ -t annotations/stardist_worker:latest $NO_CACHE

todo/TODO_REGISTRY.md

@@ -13,6 +13,7 @@ Master index of deferred work, technical debt, and future improvements for the I
 | ID | Title | Status | Priority | File | Related PR |
 |----|-------|--------|----------|------|------------|
 | TODO-001 | ML worker build optimization (mamba + shared base images) | Deferred | Medium | [ml-worker-build-optimization.md](ml-worker-build-optimization.md) | [#132](https://github.com/arjunrajlaboratory/ImageAnalysisProject/pull/132) |
+| TODO-002 | Malformed `channelCheckboxes` values from externally-authored tool configs | Open | Medium | [channelcheckboxes-serialization.md](channelcheckboxes-serialization.md) | _(this branch)_ |
 
 ## Completed TODOs
 

todo/channelcheckboxes-serialization.md

@@ -0,0 +1,77 @@
+# TODO-002: Malformed `channelCheckboxes` values from externally-authored tool configs
+
+**Status:** Open (worker guardrail done; root-cause cleanup outstanding)
+**Priority:** Medium
+**Related PR:** _(this branch)_
+
+## Summary
+
+The `channelCheckboxes` interface type is documented (in `CLAUDE.md` and the
+`nimbus-interface` skill reference) as returning a dict mapping channel-index
+strings to booleans, e.g. `{"0": True, "1": False}`. All worker code was written
+against that assumption (`[int(k) for k, v in value.items() if v]`).
+
+A production Cellpose-SAM error log showed the values arriving as **lists**
+instead:
+
+```
+"Channel for Slot 1": [0], "Channel for Slot 2": [], "Channel for Slot 3": []
+```
+
+Calling `.items()` on a list raised an opaque
+`AttributeError: 'list' object has no attribute 'items'` that crashed the worker
+**before** its existing required-field check (`if not slot1...: sendError(...)`)
+could run. So the safeguard wasn't missing — it was being skipped.
+
+## Root cause (confirmed)
+
+The list shape is **not** produced by the worker, the front-end UI, or the docs:
+
+- **Worker interface**: `Channel for Slot 1/2/3` declare no `default`, so
+  `getDefault("channelCheckboxes", undefined)` returns `{}` — an empty dict,
+  never `[0]`.
+- **NimbusImage front-end**: `channelCheckboxes` is typed `Record<number, boolean>`
+  from the first commit; the `ChannelCheckboxGroup` widget only ever emits dicts;
+  the empty value is `{}`, never `[]`.
+- **Backend** (`server/helpers/tasks.py`): JSON-encodes `workerInterfaceValues`
+  verbatim into `--parameters` — no transformation.
+- Nothing in either repo constructs these values as arrays.
+
+An empty `[]` for an unset slot is the tell: no code path generates `[]`, only
+`{}`. These values were **persisted on a saved tool whose `workerInterfaceValues`
+were written with the wrong shape** — channel slots encoded as arrays of selected
+indices (the intuitive-but-wrong representation) instead of `{index: bool}` dicts.
+The config is re-sent verbatim on every run, which is why it fails *consistently*
+for those specific tools rather than randomly.
+
+Fingerprints (tag `mg_cellposesam_v2`, name `cellpose-sam zero-shot on
+LCA5_P21_rep1`) point to programmatic/scripted batch tool creation via the
+API/Swagger that bypassed the UI widget, guessing `[0]` for "channel 0 selected."
+
+**Conclusion:** the front-end never emits lists. A list always means
+malformed/externally-authored input, never a real UI selection — so there is no
+"list-form selection" semantic to preserve.
+
+## What was done (worker guardrail — DONE)
+
+Added a shared helper `annotation_utilities.annotation_tools.get_selected_channels()`
+that parses the documented dict form, treats `None` as "nothing selected", and
+**rejects any other shape (including lists) with a `ValueError`**. Each affected
+worker catches it and calls `sendError(...)` with a clear "interface is out of
+date or misconfigured, please re-select the channels" message instead of crashing.
+
+Affected workers: `cellposesam`, `registration`, `deconwolf`,
+`histogram_matching`, `gaussian_blur`, `rolling_ball`.
+
+We chose to **reject** rather than normalize-and-run: since these values come
+from an external script that may have guessed the wrong channel, failing loudly
+is safer than silently running a tool on a channel the user never confirmed.
+
+## Outstanding (the real fix)
+
+1. Find and fix whatever created the `mg_cellposesam_v2` batch with array-shaped
+   channel values, so it writes `{index: bool}` dicts.
+2. Optionally run a one-time normalization over saved tool configs to repair the
+   existing broken tools (otherwise they keep erroring until recreated).
+
+Until then, affected tools get a clear error instead of a crash, but won't run.

workers/annotations/cellposesam/Dockerfile

@@ -32,7 +32,7 @@ RUN wget \
 
 FROM base as build
 
-COPY ./environment.yml /
+COPY ./workers/annotations/cellposesam/environment.yml /
 RUN conda env create --file /environment.yml
 SHELL ["conda", "run", "-n", "worker", "/bin/bash", "-c"]
 
@@ -45,16 +45,21 @@ RUN git clone https://github.com/Kitware/UPennContrast/
 RUN pip install -r /UPennContrast/devops/girder/annotation_client/requirements.txt
 RUN pip install -e /UPennContrast/devops/girder/annotation_client/
 
-COPY ./download_models.py /
+COPY ./workers/annotations/cellposesam/download_models.py /
 RUN python /download_models.py
 
-COPY ./utils.py /
-COPY ./girder_utils.py /
-COPY ./entrypoint.py /
+COPY ./workers/annotations/cellposesam/utils.py /
+COPY ./workers/annotations/cellposesam/girder_utils.py /
+COPY ./workers/annotations/cellposesam/entrypoint.py /
 
-RUN git clone https://github.com/arjunrajlaboratory/ImageAnalysisProject/
-RUN pip install /ImageAnalysisProject/annotation_utilities
-RUN pip install /ImageAnalysisProject/worker_client
+# Install annotation_utilities and worker_client from the local build context
+# (not a fresh clone of the default branch) so the entrypoint and these shared
+# packages always come from the same checkout. Cloning master here would ship a
+# stale package on branch/PR builds and break helpers added in the same change.
+COPY ./annotation_utilities /annotation_utilities
+RUN pip install /annotation_utilities
+COPY ./worker_client /worker_client
+RUN pip install /worker_client
 
 LABEL isUPennContrastWorker=True \
       isAnnotationWorker=True \

workers/annotations/cellposesam/entrypoint.py

@@ -8,6 +8,8 @@
 import annotation_client.workers as workers
 from annotation_client.utils import sendError, sendWarning, sendProgress
 
+import annotation_utilities.annotation_tools as annotation_tools
+
 import numpy as np  # library for array manipulation
 import deeptile
 from deeptile.extensions.segmentation import cellpose_segmentation
@@ -201,35 +203,48 @@ def compute(datasetId, apiUrl, token, params):
     padding = float(worker.workerInterface['Padding'])
     smoothing = float(worker.workerInterface['Smoothing'])
 
-    # Process new channel selections
-    slot1_channel_str_keys = [k for k, v in worker.workerInterface.get(
-        'Channel for Slot 1', {}).items() if v]
-    slot2_channel_str_keys = [k for k, v in worker.workerInterface.get(
-        'Channel for Slot 2', {}).items() if v]
-    slot3_channel_str_keys = [k for k, v in worker.workerInterface.get(
-        'Channel for Slot 3', {}).items() if v]
+    # Process channel selections. Each slot is a channelCheckboxes field, which
+    # should arrive as a dict ({"0": True}). get_selected_channels rejects any
+    # other shape (e.g. a list [0]) with a ValueError so we report a clear error
+    # to the user instead of crashing with an opaque AttributeError on .items().
+    try:
+        slot1_channels = annotation_tools.get_selected_channels(
+            worker.workerInterface.get('Channel for Slot 1'), 'Channel for Slot 1')
+        slot2_channels = annotation_tools.get_selected_channels(
+            worker.workerInterface.get('Channel for Slot 2'), 'Channel for Slot 2')
+        slot3_channels = annotation_tools.get_selected_channels(
+            worker.workerInterface.get('Channel for Slot 3'), 'Channel for Slot 3')
+    except ValueError as e:
+        sendError(
+            "Could not read the channel selections. This usually means the tool "
+            "interface is out of date or misconfigured. Please re-open the tool, "
+            "re-select the channels for each slot, and run it again.",
+            info=str(e))
+        raise
 
     stack_channels = []
 
-    if not slot1_channel_str_keys:
-        sendError("No channel selected for Slot 1. This is a required field.")
+    if not slot1_channels:
+        sendError(
+            "No channel selected for Slot 1. This is a required field. "
+            "Please select at least one channel for Slot 1 and run the tool again.")
         raise ValueError("No channel selected for Slot 1.")
-    if len(slot1_channel_str_keys) > 1:
+    if len(slot1_channels) > 1:
         sendWarning(
-            f"Multiple channels selected for Slot 1 ({slot1_channel_str_keys}). Using the first: {slot1_channel_str_keys[0]}.")
-    stack_channels.append(int(slot1_channel_str_keys[0]))
+            f"Multiple channels selected for Slot 1 ({slot1_channels}). Using the first: {slot1_channels[0]}.")
+    stack_channels.append(slot1_channels[0])
 
-    if slot2_channel_str_keys:
-        if len(slot2_channel_str_keys) > 1:
+    if slot2_channels:
+        if len(slot2_channels) > 1:
             sendWarning(
-                f"Multiple channels selected for Slot 2 ({slot2_channel_str_keys}). Using the first: {slot2_channel_str_keys[0]}.")
-        stack_channels.append(int(slot2_channel_str_keys[0]))
+                f"Multiple channels selected for Slot 2 ({slot2_channels}). Using the first: {slot2_channels[0]}.")
+        stack_channels.append(slot2_channels[0])
 
-    if slot3_channel_str_keys:
-        if len(slot3_channel_str_keys) > 1:
+    if slot3_channels:
+        if len(slot3_channels) > 1:
             sendWarning(
-                f"Multiple channels selected for Slot 3 ({slot3_channel_str_keys}). Using the first: {slot3_channel_str_keys[0]}.")
-        stack_channels.append(int(slot3_channel_str_keys[0]))
+                f"Multiple channels selected for Slot 3 ({slot3_channels}). Using the first: {slot3_channels[0]}.")
+        stack_channels.append(slot3_channels[0])
 
     if not stack_channels:  # Should technically be caught by slot 1 check, but as a safeguard.
         sendError("No channels were selected for processing.")

workers/annotations/deconwolf/entrypoint.py

@@ -10,6 +10,8 @@
 import annotation_client.workers as workers
 from annotation_client.utils import sendProgress, sendWarning, sendError
 
+import annotation_utilities.annotation_tools as annotation_tools
+
 import numpy as np
 import tifffile
 import large_image as li
@@ -354,8 +356,16 @@ def compute(datasetId, apiUrl, token, params):
     workerInterface = params['workerInterface']
 
     # Parse channel selection
-    allChannels = workerInterface.get('Channels to deconvolve', {})
-    channels = [int(k) for k, v in allChannels.items() if v]
+    try:
+        channels = annotation_tools.get_selected_channels(
+            workerInterface.get('Channels to deconvolve'), 'Channels to deconvolve')
+    except ValueError as e:
+        sendError(
+            "Could not read the 'Channels to deconvolve' selection. This usually "
+            "means the tool interface is out of date or misconfigured. Please "
+            "re-open the tool, re-select the channels, and run it again.",
+            info=str(e))
+        return
     print(f"Selected channels to deconvolve: {channels}")
 
     if len(channels) == 0:

workers/annotations/gaussian_blur/entrypoint.py

@@ -11,6 +11,8 @@
 
 from annotation_client.utils import sendProgress, sendError
 
+import annotation_utilities.annotation_tools as annotation_tools
+
 import imageio
 import numpy as np
 
@@ -118,12 +120,16 @@ def compute(datasetId, apiUrl, token, params):
     workerInterface = params['workerInterface']
     sigma = float(workerInterface['Sigma'])
     channel = int(workerInterface['Channel'])
-    allChannels = workerInterface['All channels']
-
-    print("allChannels", allChannels)
-    # Output is allChannels {'1': True, '2': True}
-    # This means that channels 1 and 2 are being blurred
-    channels = [int(k) for k, v in allChannels.items() if v]
+    try:
+        channels = annotation_tools.get_selected_channels(
+            workerInterface.get('All channels'), 'All channels')
+    except ValueError as e:
+        sendError(
+            "Could not read the 'All channels' selection. This usually means the "
+            "tool interface is out of date or misconfigured. Please re-open the "
+            "tool, re-select the channels, and run it again.",
+            info=str(e))
+        return
     print("channels", channels)
 
     tile = params['tile']

workers/annotations/histogram_matching/entrypoint.py

@@ -11,6 +11,8 @@
 
 from annotation_client.utils import sendProgress, sendError
 
+import annotation_utilities.annotation_tools as annotation_tools
+
 import imageio
 import numpy as np
 
@@ -105,12 +107,16 @@ def compute(datasetId, apiUrl, token, params):
         reference_Time = 0
     else:
         reference_Time = int(workerInterface['Reference Time Coordinate']) - 1
-    allChannels = workerInterface['Channels to correct']
-
-    print("allChannels", allChannels)
-    # Output is allChannels {'1': True, '2': True}
-    # This means that channels 1 and 2 are being blurred
-    channels = [int(k) for k, v in allChannels.items() if v]
+    try:
+        channels = annotation_tools.get_selected_channels(
+            workerInterface.get('Channels to correct'), 'Channels to correct')
+    except ValueError as e:
+        sendError(
+            "Could not read the 'Channels to correct' selection. This usually "
+            "means the tool interface is out of date or misconfigured. Please "
+            "re-open the tool, re-select the channels, and run it again.",
+            info=str(e))
+        return
     print("channels", channels)
     if len(channels) == 0:
         sendError("No channels to correct")