Fix QuickNAT indexed skip connections

[yinkev]

Closes #7475.

Summary

• make QuickNAT's indexed skip connection call its indexed submodule with the required (input, indices) signature
• preserve cat, add, and mul fusion modes and the original pooling indices
• reject unsupported fusion modes explicitly
• align the public se_block annotation with the already-supported None configuration
• add unskipped core coverage independent of the optional squeeze-and-excitation package

Root cause

SkipConnectionWithIdx delegated to the ordinary SkipConnection.forward(). That base implementation calls its submodule with only the input tensor, but QuickNAT's nested SequentialWithIdx requires both input and indices. The existing QuickNAT test class was skipped whenever the optional SE dependency was unavailable, so the core path was not exercised.

Validation

• the core regression fails on upstream/dev with SequentialWithIdx.forward() missing ... 'indices'
• focused core and fusion-mode tests: 5 passed
• QuickNAT unittest module: 14 tests, 9 optional-dependency skips, 5 passes
• Black, isort, Ruff, DCO sign-off, and git diff --check pass

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[coderabbitai]

📝 Walkthrough

Walkthrough

SkipConnectionWithIdx.forward now directly invokes its submodule, combines outputs for cat, add, and mul modes, raises NotImplementedError for unsupported modes, and preserves indices. A dependency-independent Quicknat test runs evaluation with se_block=None and verifies the output shape.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change fixes the reported missing-indices crash for se_block=None and adds a core regression test for #7475.
Out of Scope Changes check	✅ Passed	The code and test changes stay focused on the QuickNAT bug fix and regression coverage.
Title check	✅ Passed	The title clearly and concisely summarizes the main QuickNAT skip-connection fix.
Description check	✅ Passed	The description covers the change summary, root cause, validation, issue link, and type checkboxes, so it is mostly complete.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

monai/networks/nets/quicknat.py (1)

45-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add Google-style docstrings to the new Python definitions.

Both changed areas introduce definitions without documenting their contracts.

• monai/networks/nets/quicknat.py#L45-L55: document arguments, return tuple, and raised exception.
• tests/networks/nets/test_quicknat.py#L39-L40: document the test class and regression test.

As per path instructions, every Python definition should have an appropriate Google-style docstring.

🤖 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 `@monai/networks/nets/quicknat.py` around lines 45 - 55, Document the QuickNat
forward definition in monai/networks/nets/quicknat.py, covering input and
indices arguments, the returned output-and-indices tuple, and the
NotImplementedError for unsupported modes. Also add Google-style docstrings to
the test class and regression test at tests/networks/nets/test_quicknat.py lines
39-40, describing their purpose and behavior.

Source: Path instructions

tests/networks/nets/test_quicknat.py (1)

41-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align se_block’s type contract with this test.

The test passes None, but Quicknat.__init__ declares se_block: str. Update the public annotation and documentation to accept str | None, or consistently use the documented "None" sentinel; otherwise static type checking can reject the configuration this PR supports.

🤖 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 `@tests/networks/nets/test_quicknat.py` around lines 41 - 46, Update
Quicknat.__init__ so the se_block parameter annotation accepts str | None, and
revise its public documentation to describe None as a supported value. Keep the
test configuration passing None without requiring a string sentinel.

🤖 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 `@monai/networks/nets/quicknat.py`:
- Around line 47-54: Add focused unit tests for the Quicknat fusion logic
covering “cat”, “add”, and “mul” modes, asserting each produces the expected
output. Also test that an unsupported mode raises NotImplementedError and that
the relevant index is preserved, using the Quicknat-related test setup and
symbols.

---

Nitpick comments:
In `@monai/networks/nets/quicknat.py`:
- Around line 45-55: Document the QuickNat forward definition in
monai/networks/nets/quicknat.py, covering input and indices arguments, the
returned output-and-indices tuple, and the NotImplementedError for unsupported
modes. Also add Google-style docstrings to the test class and regression test at
tests/networks/nets/test_quicknat.py lines 39-40, describing their purpose and
behavior.

In `@tests/networks/nets/test_quicknat.py`:
- Around line 41-46: Update Quicknat.__init__ so the se_block parameter
annotation accepts str | None, and revise its public documentation to describe
None as a supported value. Keep the test configuration passing None without
requiring a string sentinel.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e3f9bca-66cc-4aae-92d6-dfdd4d271cce

📥 Commits

Reviewing files that changed from the base of the PR and between 3ee058b and ef1b752.

📒 Files selected for processing (2)

• monai/networks/nets/quicknat.py
• tests/networks/nets/test_quicknat.py

[yinkev]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[yinkev]

Replaced by #9021 to conform to this repository's documented contributor workflow and branch-naming convention. The replacement uses the same signed commit and identical code diff: #9021