Port mono_t2 using skills - #7
Merged
Merged
Conversation
added 3 commits
July 22, 2026 21:58
… overlay) Output maps now always go through write_map_nifti (2D when z=1, 3D otherwise), so single-slice derivatives match qMRLab's make_nii dim[0]=2 instead of a 3D z=1 volume that misregistered against 2D references on oblique acquisitions. Claude-Session: https://claude.ai/code/session_014DCAAax3rn83w46iL9QNEf
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a monoexponential T2 model with exponential and linear fitting, registers it as MESE, extends ChangesMono T2 model
CLI and output pipeline
Recipes and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant OSF
participant qmrust_bidsify
participant BIDS_Dataset
participant qmrust_fit
participant compare_maps
OSF->>qmrust_bidsify: provide SEdata.nii.gz and Mask.nii.gz
qmrust_bidsify->>BIDS_Dataset: write MESE data and EchoTime sidecars
BIDS_Dataset->>qmrust_fit: provide BIDS recipe and protocol
qmrust_fit->>BIDS_Dataset: write sub-01_T2map.nii.gz
compare_maps->>BIDS_Dataset: read generated T2 map
compare_maps->>OSF: compare against FitResults reference
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/qmrust-cli/src/bidsify.rs (1)
195-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo unit test covers the
Some(source_header)branch.Every
write_bids_treecall in this test module passesNone; header preservation for NIfTI sources is only exercised by the OSF integration script, not a fast unit test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/qmrust-cli/src/bidsify.rs` around lines 195 - 222, Add a focused unit test for the write_bids_tree path with Some(source_header), using a distinctive NIfTI header and verifying the generated output preserves its relevant header fields. Keep the existing None-based tests unchanged and exercise the header selection behavior around source_header.ci/compare_maps.py (1)
20-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
read_niiignoresscl_slope/scl_interNIfTI intensity-rescale fields.Values are compared directly from the raw voxel buffer with no rescale applied. This is safe as long as both files are written unscaled (float64, slope=1/inter=0, as
qmrust's writer guarantees), but if the qMRLab referenceFitResultsmap ever carries a non-identity scale/intercept, this script would silently compare unscaled values.♻️ Proposed fix
dim = struct.unpack("<8h", b[40:56]) datatype = struct.unpack("<h", b[70:72])[0] + scl_slope = struct.unpack("<f", b[112:116])[0] + scl_inter = struct.unpack("<f", b[116:120])[0] vox_offset = int(struct.unpack("<f", b[108:112])[0]) n = 1 for d in dim[1 : dim[0] + 1]: n *= d fmt = {16: "f", 64: "d"}.get(datatype) if fmt is None: sys.exit(f"unsupported NIfTI datatype {datatype} in {path}") size = struct.calcsize(fmt) raw = b[vox_offset : vox_offset + n * size] - return list(struct.unpack("<" + fmt * n, raw)) + values = struct.unpack("<" + fmt * n, raw) + if scl_slope not in (0.0, 1.0) or scl_inter != 0.0: + slope = scl_slope if scl_slope != 0.0 else 1.0 + return [v * slope + scl_inter for v in values] + return list(values)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/compare_maps.py` around lines 20 - 35, Update read_nii to parse the NIfTI scl_slope and scl_inter fields and apply the intensity transform slope * voxel + intercept to every decoded value before returning the flat list. Preserve identity behavior for files with slope 1 and intercept 0, and handle the NIfTI convention for an unset or zero slope appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/qmrust-cli/src/bidsify.rs`:
- Around line 49-59: Validate mask-source compatibility in the source-selection
logic around read_nifti_source and read_mat_source: reject --mask when
--nii-data is selected, and reject --nii-mask when --mat-data or --mat-dir is
selected. Preserve the existing mutually exclusive source validation and only
proceed when each mask flag matches its corresponding input source.
---
Nitpick comments:
In `@ci/compare_maps.py`:
- Around line 20-35: Update read_nii to parse the NIfTI scl_slope and scl_inter
fields and apply the intensity transform slope * voxel + intercept to every
decoded value before returning the flat list. Preserve identity behavior for
files with slope 1 and intercept 0, and handle the NIfTI convention for an unset
or zero slope appropriately.
In `@crates/qmrust-cli/src/bidsify.rs`:
- Around line 195-222: Add a focused unit test for the write_bids_tree path with
Some(source_header), using a distinctive NIfTI header and verifying the
generated output preserves its relevant header fields. Keep the existing
None-based tests unchanged and exercise the header selection behavior around
source_header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63ababc9-60c5-41fb-888d-b817ad685777
📒 Files selected for processing (19)
.claude/skills/porting-qmrlab-models/SKILL.mdci/compare_maps.pyci/integration_osf.shcrates/qmrust-cli/src/bidsify.rscrates/qmrust-cli/src/commands.rscrates/qmrust-cli/src/io/nifti.rscrates/qmrust-cli/src/main.rscrates/qmrust-core/src/models/mod.rscrates/qmrust-core/src/models/mono_t2/config.rscrates/qmrust-core/src/models/mono_t2/fit.rscrates/qmrust-core/src/models/mono_t2/mod.rscrates/qmrust-core/src/models/mono_t2/model.rscrates/qmrust-core/src/registry.rscrates/rust-bids/src/default_grouping.yamldocs/agents/ADDING-A-MODEL.mddocs/agents/ARCHITECTURE.mddocs/agents/DATA-PIPELINE.mdrecipes/bids/mono_t2_config.yamlrecipes/non-bids/mono_t2_config.yaml
…ion; ci: honor NIfTI scl Claude-Session: https://claude.ai/code/session_014DCAAax3rn83w46iL9QNEf
This was referenced Jul 29, 2026
Merged
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
1 participant
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.
Summary by CodeRabbit
qmrust bidsifynow supports 4D NIfTI inputs and NIfTI masks, preserving spatial header information and producing compatible BIDS output.