Skip to content

Extract SR and GSPS code from dicom.rs#44

Merged
timcogan merged 2 commits intomasterfrom
refactor/dicom-module-split
Mar 13, 2026
Merged

Extract SR and GSPS code from dicom.rs#44
timcogan merged 2 commits intomasterfrom
refactor/dicom-module-split

Conversation

@timcogan
Copy link
Copy Markdown
Owner

@timcogan timcogan commented Mar 13, 2026

Summary by CodeRabbit

  • New Features

    • GSPS overlay extraction from DICOM with support for multiple graphic types, unit handling, and frame-specific filtering
    • DICOM Structured Report parsing producing document metadata and hierarchical content nodes
  • Refactor

    • Reorganized DICOM handling into dedicated GSPS and Structured Report submodules
  • Tests

    • Comprehensive tests covering parsing, reference resolution, frame filtering, and edge cases

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 75c75b18-fc7e-414f-bc8c-d910e19d872a

📥 Commits

Reviewing files that changed from the base of the PR and between 8cfa347 and c433995.

📒 Files selected for processing (1)
  • src/dicom.rs

📝 Walkthrough

Walkthrough

Refactors GSPS and Structured Report parsing out of src/dicom.rs into new modules src/dicom/gsps.rs and src/dicom/sr.rs; src/dicom.rs now re-exports the new modules. DESIGN.md updated with expanded DICOM entry and module ownership additions for src/app.rs and tools/benchmark.

Changes

Cohort / File(s) Summary
Documentation
DESIGN.md
Expanded DICOM section to describe facade + format-specific parsers; added module ownership entries for src/app.rs and tools/benchmark.
GSPS Module
src/dicom/gsps.rs, src/dicom.rs
New gsps module added with data models (GspsUnits, GspsGraphic, GspsOverlayGraphic, GspsOverlay), parsing logic, load_gsps_overlays, and tests. src/dicom.rs re-exports GSPS items and removed inline GSPS definitions.
Structured Report Module
src/dicom/sr.rs, src/dicom.rs
New sr module with StructuredReportNode, StructuredReportDocument, load_structured_report, parsing helpers, and test stub. src/dicom.rs re-exports SR items and removed inline SR definitions.
Main DICOM Refactor
src/dicom.rs
Removed in-file GSPS/SR types and helpers (~large deletion); added mod gsps; pub use gsps::...; mod sr; pub use sr::...; and updated imports/tests to reference new modules.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 Hopping through code with a bright little nod,

I split GSPS and SR like a neat little pod,
Modules in rows, exports in line,
Parsers that hum and tests that shine,
A rabbit's small cheer for structure refined 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: extracting SR (Structured Report) and GSPS code from dicom.rs into dedicated modules (src/dicom/sr.rs and src/dicom/gsps.rs).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/dicom-module-split
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/dicom.rs`:
- Around line 17-20: The public type GspsOverlay exposes Vec<GspsOverlayGraphic>
but GspsOverlayGraphic is only re-exported under #[cfg(test)], causing a
private_interfaces lint; remove the #[cfg(test)] guard so that pub use
gsps::GspsOverlayGraphic is re-exported publicly (aligning with GspsOverlay),
ensuring external code can name the element type and the public API is
consistent with load_gsps_overlays/GspsOverlay/GspsUnits; then run the
linter/tests to confirm the warning is resolved.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 425dffa0-3791-4097-ad76-7b2a9ba74b07

📥 Commits

Reviewing files that changed from the base of the PR and between 943491c and 8cfa347.

📒 Files selected for processing (4)
  • DESIGN.md
  • src/dicom.rs
  • src/dicom/gsps.rs
  • src/dicom/sr.rs

Comment thread src/dicom.rs Outdated
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