Repository navigation
Modernize inference and use pinned Hugging Face checkpoints - #10
Merged
Merged
Conversation
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
Reviewer's GuideModernizes DeepCoro into an installable modular inference package, changes checkpoint loading to pinned and offline-capable Hugging Face resolution, fixes numerous DICOM/geometry/preprocessing/runtime edge cases, and updates deployment, documentation, compatibility entry points, and CI. Sequence diagram for pinned and offline checkpoint resolutionsequenceDiagram
participant CLI
participant Pipeline
participant Hub as deepcoro.hub
participant HF as HuggingFaceCache
participant Models as ModelLoader
CLI->>Pipeline: deepcoro(input_file_path, save_dir, ...)
Pipeline->>Hub: resolve_models(models_dir, repo_id, revision, cache_dir, local_files_only)
alt local models supplied
Hub->>Hub: validate_models(models_dir)
else Hub models requested
Hub->>HF: snapshot_download(repo_id, pinned revision, allow_patterns)
HF-->>Hub: checkpoint snapshot
Hub->>Hub: validate_models(snapshot)
end
Hub-->>Pipeline: validated model root
Pipeline->>Models: load_model(segmentation checkpoint)
Pipeline->>Models: load_regression_model(algorithm6.pt)
Sequence diagram for DICOM stenosis inference pipelinesequenceDiagram
participant Pipeline
participant CSV as AnnotationReader
participant DICOM as DICOMReader
participant Geometry
participant Segmentation
participant Segments
participant Regressor
participant Output
Pipeline->>CSV: read_annotations(input_file_path)
Pipeline->>DICOM: read_dicom(relative or absolute path)
DICOM-->>Pipeline: pixels, pixel_spacing, fps, patient_age
Pipeline->>Geometry: resize_coordinates(box, pixel_spacing, pixels.shape, target_size)
Pipeline->>Geometry: register(pixels, frame, resized_box)
Pipeline->>Segmentation: perform_segmentation_inference(models, image, device)
Segmentation-->>Pipeline: frame labels
Pipeline->>Segments: identify_segments(stenoses, labels, artery_view)
Segments-->>Pipeline: selected artery segments
Pipeline->>Geometry: create_cropped_registered_video(pixels, frame, shifts, box)
Pipeline->>Regressor: forward(video, age, artery_segment)
Regressor-->>Pipeline: stenosis probability
Pipeline->>Output: write df_stenosis.csv and optional MP4
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: High
Review is required. This assessment did not approve the pull request.
The diff rewrites DeepCoro inference. main.py and utils.py are compatibility shims; DICOM validation, crop geometry, tracking, segmentation, regression, and CSV/video outputs now live in the new deepcoro package (23 files, +1269/−743). That is the product’s clinical output path, not a packaging-only change.
Why this is high risk:
- Segment selection votes across frames and keeps one annotation per segment. Severe stenosis is a fixed probability cutoff (
0.23). Percent stenosis, the results CSV, and annotated videos can all differ from the previous pipeline. - Checkpoint loading changed (
weights_only, no ImageNet encoder initialization, regressor in float64). Default weights are downloaded at runtime from a pinned Hugging Face snapshot instead of being cloned into the image at build time. - The container contract changed:
python:3.11-slim, models are not baked in, the entrypoint is thedeepcoroCLI, and a Hugging Face cache volume is required. - No CODEOWNERS file. The pull request was merged with no review on it.
Requested reviewer: @elodielabrecque (primary author of the previous algorithm code in utils.py). @cacoool has the Docker history but is not a repository collaborator, so that review request was rejected.
Sent by Cursor Automation: Assign PR reviewers
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Fixed security issues:
- Pillow (link)
- certifi (link)
- fsspec (link)
- opencv-contrib-python (link)
- pydicom (link)
- torch (link)
- urllib3 (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="README.md" line_range="114" />
<code_context>
+ -v /path/to/dcm_input:/dcm_input:ro \
+ -v /path/to/results:/results \
+ -v /path/to/hf-cache:/cache/huggingface \
+ deepcoro_inference
+```
+
</code_context>
<issue_to_address>
**Docker cannot use host credentials**
When docker downloads from the gated repository using a host-only `HF_TOKEN` or a login token outside the mounted cache, `docker run` does not forward a host `HF_TOKEN`, and the container cannot use a host `hf auth login` token unless that token is in the mounted cache; the gated-repository download is rejected and inference stops before running.
Document forwarding `HF_TOKEN` securely with `docker run -e HF_TOKEN`, or mount the host Hugging Face token cache into the container.
Also at `Dockerfile:12`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and changes to checkpoint loading, preprocessing, tracking, and inference can produce an incorrect artery segment or stenosis estimate, which is written to the results CSV and annotated videos. Those outputs can be regenerated from the original DICOMs and annotations after reverting or fixing the code.
Blocking findings: README.md:114
| -v /path/to/dcm_input:/dcm_input:ro \ | ||
| -v /path/to/results:/results \ | ||
| -v /path/to/hf-cache:/cache/huggingface \ | ||
| deepcoro_inference |
There was a problem hiding this comment.
🟡 Medium · Docker cannot use host credentials
When docker downloads from the gated repository using a host-only HF_TOKEN or a login token outside the mounted cache, docker run does not forward a host HF_TOKEN, and the container cannot use a host hf auth login token unless that token is in the mounted cache; the gated-repository download is rejected and inference stops before running.
Document forwarding HF_TOKEN securely with docker run -e HF_TOKEN, or mount the host Hugging Face token cache into the container.
Also at Dockerfile:12.
Prompt for AI agents
In `README.md` at line 114:
**Docker cannot use host credentials**
When docker downloads from the gated repository using a host-only `HF_TOKEN` or a login token outside the mounted cache, `docker run` does not forward a host `HF_TOKEN`, and the container cannot use a host `hf auth login` token unless that token is in the mounted cache; the gated-repository download is rejected and inference stops before running.
Document forwarding `HF_TOKEN` securely with `docker run -e HF_TOKEN`, or mount the host Hugging Face token cache into the container.
Also at `Dockerfile:12`.
This was referenced Oct 28, 2026
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
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.
Modernizes the legacy DICOM stenosis pipeline into an installable
deepcoropackage with separate input validation, geometry, preprocessing, model, Hub, and CLI modules. Inference resolves eight existing Hugging Face checkpoints from a pinned revision, supports cached/offline runs, and no longer embeds weights or downloads unrelated initialization weights.Fixes DICOM-local frame indexing, CSV-relative paths, crop dimensions, blank-image normalization, small-image segmentation, empty outputs, tracking failures, and modern PyTorch checkpoint compatibility. Updates Docker packaging, documentation, and CI while preserving source entry points.
Validation: 35 tests pass; all eight published checkpoints downloaded and loaded; actual segmentation and GPU regression forwards, synthetic DICOM inference, MP4 encoding/decoding, offline reuse, wheel build, and installed CLI checked. Docker build was not run because Docker is unavailable in this environment.
Summary by Sourcery
Modernize DeepCoro inference into a validated installable package with reproducible external checkpoints, robust execution paths, and updated delivery tooling.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: