Guard channelCheckboxes parsing against malformed (non-dict) values#142
Guard channelCheckboxes parsing against malformed (non-dict) values#142arjunrajlab wants to merge 5 commits into
Conversation
The channelCheckboxes slot values were assumed to be dicts and read via .items(), but the value can arrive as a plain list (e.g. [0]). This caused an opaque 'list object has no attribute items' AttributeError to crash the worker before the friendly validation checks could run. Add a get_selected_channels() helper that normalizes dict, list, and empty/None inputs into a sorted list of channel indices, and emit a clear sendError when Slot 1 is unset or the selections can't be parsed.
…orkers
channelCheckboxes is documented to return a dict ({"0": True}), but a
production log showed it arriving as a list ([0]), which crashed workers with
an opaque AttributeError on .items() before any validation ran.
Add a shared, tested annotation_tools.get_selected_channels() helper that
parses the documented dict form, treats a missing value as 'nothing selected',
and rejects any other shape (notably lists) with a ValueError. We reject rather
than silently recover, since a [0] could be an un-chosen default and running a
tool on the wrong channel is worse than failing loudly.
Wire cellposesam, registration, deconwolf, histogram_matching, gaussian_blur,
and rolling_ball to the helper so they sendError() with a clear 'interface out
of date or misconfigured' message instead of crashing.
Add TODO-002 to verify against the NimbusImage front-end what channelCheckboxes
actually serializes and fix the root cause there.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e17e24ce1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| channels = annotation_tools.get_selected_channels( | ||
| workerInterface.get('All channels'), 'All channels') |
There was a problem hiding this comment.
When Gaussian Blur is built from its worker Dockerfile, only this changed entrypoint.py is copied on top of nimbusimage/image-processing-base:latest; the annotation_utilities package comes from whatever was installed in that base image. If a worker image is rebuilt or deployed without rebuilding/publishing the base image from the same checkout, this new call resolves against the old package and raises AttributeError: module ... has no attribute get_selected_channels before the intended sendError path runs. The same pattern applies to the other image-processing-base workers changed here, so either package the updated annotation_utilities in those worker images or avoid depending on a helper that may not exist in the base.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged, but leaving these (gaussian_blur, registration, histogram_matching, rolling_ball) as-is. They inherit annotation_utilities from nimbusimage/image-processing-base:latest by design — the shared package isn't vendored per-worker, so any change to it requires rebuilding the base image from the same checkout before rebuilding the workers. This is the existing architecture (e.g. registration already imported annotation_utilities from the base before this PR), and the normal ./build_workers.sh flow rebuilds the base. Vendoring the package into each worker image would diverge from that pattern. The cellposesam case was different because it explicitly cloned the default branch, which is now fixed.
Generated by Claude Code
| # 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( |
There was a problem hiding this comment.
The Cellpose-SAM Dockerfile copies this changed entrypoint from the build context, but still installs annotation_utilities from a fresh git clone https://github.com/arjunrajlaboratory/ImageAnalysisProject/ rather than the same checkout. In branch/PR builds where the remote default branch does not yet contain this helper, the image ships the new entrypoint with an older package and every compute reaches this call as AttributeError instead of the clear malformed-channel error. Install annotation_utilities from the local build context, as the deconwolf image does, to keep the entrypoint and package version aligned.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fc97d26. cellposesam now builds from the repo-root context (matching deconwolf/sam2) and installs annotation_utilities and worker_client via COPY ./annotation_utilities + pip install from the local checkout instead of cloning the default branch, so the entrypoint and the shared packages always come from the same checkout. Updated build_machine_learning_workers.sh accordingly.
Generated by Claude Code
…m local checkout cellposesam installed annotation_utilities/worker_client via a fresh clone of the default branch rather than the build checkout, so branch/PR builds shipped the new entrypoint against a stale package and would raise AttributeError on get_selected_channels before the intended sendError path. Switch the worker to the repo-root build context (matching deconwolf/sam2) and COPY+install both shared packages from the local checkout so the entrypoint and packages stay in lockstep.
2 participants
Problem
A production Cellpose-SAM run crashed with an opaque
AttributeError: 'list' object has no attribute 'items'. The channel-slotvalues arrived as lists instead of the documented dict shape:
Every worker reads
channelCheckboxesas a dict ([int(k) for k, v in value.items() if v]),so
.items()blew up before cellposesam's existing required-field check(
if not slot1...: sendError(...)) could run. The safeguard wasn't missing — itwas being skipped because the crash happened first.
Root cause (confirmed against the front-end)
The list shape is not something the system is supposed to produce:
default, so the front-endreturns
{}for an unset slot — never[0].channelCheckboxesis typedRecord<number, boolean>;the
ChannelCheckboxGroupwidget only ever emits dicts, and its empty value is{}, not[].workerInterfaceValuesverbatim — no transformation.The empty
[]for unset slots is the giveaway: nothing in either repo generates[], only{}. These values came from a saved tool whose config was authoredprogrammatically (fingerprints: tag
mg_cellposesam_v2) via the API, bypassingthe UI widget and guessing the intuitive-but-wrong array shape (
[0]= "channel 0selected"). That malformed config is re-sent verbatim on every run, which is why
it fails consistently for those specific tools.
Takeaway: the front-end never emits lists, so there is no legitimate
"list-form selection" to preserve — a non-dict value always means
malformed/externally-authored input.
Fix
annotation_utilities.annotation_tools.get_selected_channels(value, field_name):{"0": True}→[0](sorted ints)None→[](lets each worker keep its own required-field logic)ValueErrorchannelCheckboxesworkers now catch thatValueErrorand callsendError(...)with a clear "interface is out of date or misconfigured —re-select the channels" message instead of crashing:
cellposesam,registration,deconwolf,histogram_matching,gaussian_blur,rolling_ball.package-testsjob; fullannotation_utilitiessuite is green)..gitignorefor Python build/test artifacts (the repo had none).We deliberately reject rather than normalize-and-run: since these values
originate from an external script that may have guessed the wrong channel,
failing loudly is safer than silently running a tool on an unconfirmed channel.
Failure modes worth knowing
workerInterfaceValueswere writtenwith the wrong shape (array instead of
{index: bool}) — i.e. programmatic/scripted tool creation that bypassed the UI. Tools created through the
NimbusImage UI are unaffected.
instead of a crash — but they still won't run until their saved config is
fixed. This change addresses the symptom, not the source.
channelCheckboxesworkers, which is why the guard is applied uniformly.
Outstanding (tracked in
todo/channelcheckboxes-serialization.md, TODO-002)mg_cellposesam_v2batch so it writesdict-shaped channel values.
already-broken tools (otherwise they keep erroring until recreated).
https://claude.ai/code/session_01BdyfqRGZGvZsn4Y1CzcGcM
Generated by Claude Code