Skip to content

Validate WorkerClient batch coordinates before image access - #165

Merged
arjunrajlab merged 1 commit into
masterfrom
codex/validate-worker-batch-ranges
Aug 7, 2026
Merged

Validate WorkerClient batch coordinates before image access#165
arjunrajlab merged 1 commit into
masterfrom
codex/validate-worker-batch-ranges

Conversation

@arjunrajlab

Copy link
Copy Markdown
Collaborator

Summary

  • validate Batch XY/Z/Time coordinates against dataset IndexRange before image access
  • report that batch positions start at 1 and include the dataset's valid range
  • report malformed batch syntax through sendError instead of leaking parser exceptions
  • validate stacked channel selections and preserve parsed batch ranges across repeated preflight checks
  • run the Cellpose-SAM preflight before loading the GPU model
  • document Cellpose-SAM's 1-indexed batch contract and record the scoped Compose test-runner gap discovered during verification

Root cause

Batch fields are entered as 1-indexed positions and converted to zero-based coordinates. The production value Batch Z: "0-34" therefore became [-1, ..., 33]. WorkerClient passed -1 directly to coordinatesToFrameIndex, where the dataset map raised KeyError: -1 without first sending a useful worker error.

The earlier Failed to get docker network hostname-resolution message was non-fatal: the container started, detected CUDA, and reached image loading before the coordinate failure.

Impact

Invalid batch coordinates now fail before frame lookup, with a message such as: Batch Z contains invalid position 0. Batch positions start at 1; ... valid range is 1-35. Cellpose-SAM performs this validation before constructing the large model, and all workers using WorkerClient.process() inherit the shared guard.

Validation

  • pytest -q worker_client/tests — 19 passed
  • pytest -q annotation_utilities/tests — 61 passed
  • pytest -q workers/annotations/cellposesam/tests — 13 passed
  • python -m py_compile for the changed Python modules
  • git diff --check

./build_workers.sh --build-and-run-tests cellposesam successfully rebuilt the shared base images but did not run a worker test because cellposesam/cellposesam_test are absent from Compose; the script nevertheless returned zero. That pre-existing build-runner issue is documented as TODO-005 in this PR.

@arjunrajlab
arjunrajlab marked this pull request as ready for review August 7, 2026 10:57
@arjunrajlab
arjunrajlab merged commit 7935299 into master Aug 7, 2026
1 check passed
arjunrajlab added a commit that referenced this pull request Aug 7, 2026
Resolves three conflicts between this branch's dataset-aware `all` batch
parsing and master's batch-coordinate validation (#165) / select-field
validation (#168).

worker_client.py: master added `_parse_batch_values`, which reported
malformed Batch input with sendError before the clients were built. This
branch needs the dataset's IndexRange to expand `all`, so parsing has to
happen after datasetClient exists. Folded master's per-field error message
into the shared `get_batch_ranges()` so it raises a ValueError naming the
offending field, and WorkerClient catches it, sends the banner, and
re-raises. `_parse_batch_values` is gone; the direct SAM batching loops now
inherit the clear message too, which they never had.

CELLPOSESAM.md / CROP.md: kept master's more precise wording (1-indexed,
empty-selection error, size-one dimension note) and added this branch's
`all` documentation alongside it rather than replacing either.

Also mirrored the branch's skill updates into .claude/skills/, which it had
only applied to .agents/, and fixed the duplicate "### 7" the textual
auto-merge produced in the hardening catalog. Corrected the stale sweep
counts there (claimed ~6 WorkerClient / ~26 direct; actually 43 / 10).

Added two regression tests for the merged behavior: get_batch_ranges names
the field that failed to parse, and returns re-iterable lists rather than
one-shot generators.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant