Skip to content

[codex] Fix missing IndexRange fallbacks - #140

Merged
arjunrajlab merged 4 commits into
masterfrom
codex/indexrange-fallback-audit
May 29, 2026
Merged

[codex] Fix missing IndexRange fallbacks#140
arjunrajlab merged 4 commits into
masterfrom
codex/indexrange-fallback-audit

Conversation

@arjunrajlab

Copy link
Copy Markdown
Collaborator

Summary

  • Treat missing tiles['IndexRange'] as a single XY/Z/time/channel in shared image stack helpers.
  • Apply the same missing-IndexRange fallback to line_scan_worker all-channel scans.
  • Remove an unused direct IndexRange read from sam2_propagate.
  • Add regression tests for missing IndexRange, missing dimension keys, and multi-channel behavior.

Root Cause

Single-frame datasets can omit IndexRange entirely. Several workers and shared helpers either indexed tiles['IndexRange'] directly or defaulted missing dimensions to zero, causing KeyError or empty-stack failures.

Validation

  • PYTHONPATH=/Users/arjunraj/code/ImageAnalysisProject/workers/properties/lines/line_scan_worker /private/tmp/imageanalysis-indexrange-venv/bin/python -m pytest -q worker_client/tests annotation_utilities/tests workers/properties/lines/line_scan_worker/tests -> 11 passed
  • Attempted ./build_workers.sh --build-and-run-tests line_scan_worker; Docker build was able to start only with escalated socket access, then stalled while resolving base image metadata and was stopped before completion.

arjunrajlab and others added 4 commits May 26, 2026 10:15
These shared packages are installed into worker images rather than built
as workers, so they had no docker-compose *_test service and their tests
(including the new IndexRange fallback regression tests) never ran in CI.
Add a native pytest job so they run on every PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Building and testing every Docker worker on a single runner is too heavy to
run on every push/PR, so the `test` job is commented out (kept in-file for
easy re-enable). The lightweight native `package-tests` job for
annotation_utilities and worker_client still runs. Document in CLAUDE.md that
the full worker tests should be run locally via build_workers.sh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arjunrajlab
arjunrajlab marked this pull request as ready for review May 27, 2026 00:14

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5298a58d25

ℹ️ 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".

Comment on lines +5 to +7
branches: [ master ]
pull_request:
branches: [ main ]
branches: [ master ]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore workflow trigger to the active default branch

The workflow now only triggers for master, but this repository’s active branch is main (and there is no master branch), so pushes/PRs against normal development branches will skip this CI entirely. That means the new package tests in this same change won’t run automatically, allowing regressions to land unnoticed. Update the on.push.branches / on.pull_request.branches filters to include the actual branch used by the repo.

Useful? React with 👍 / 👎.

@arjunrajlab
arjunrajlab merged commit c441da6 into master May 29, 2026
1 check passed
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