fix: nested external gate depth, and gate definition cycles of any length

[TheGupta2012]

Fixes #367 and #369.

Scope note: #376 (issue #369, cyclic gate definitions) was merged into this branch, so this PR now carries both fixes plus a small follow-up that inlines #376's single-caller error helper. Both touch _visit_custom_gate_operation, which is why they travel together.

The bug

An external custom gate whose body calls another custom gate reported the depth of the decomposition it never emitted. This is the shape the #352 fix did not reach — one level of nesting was already correct, which is why the existing tests passed.

qasm = '''OPENQASM 3.0;
include "stdgates.inc";
gate inner a, b { crz(0.5) a, b; }
gate outer a, b { inner a, b; h a; }
qubit[2] q;
outer q[0], q[1];
'''
module = loads(qasm)
module.unroll(external_gates=["outer"])
module.depth()   # 13, from one emitted statement

external_gates	statements	depth() before	after
["outer"]	1	13	1
["inner", "outer"]	1	2	1
single level, for contrast	1	1	1
None (nothing external)	13	13	13

Cause

_visit_custom_gate_operation assigned _recording_ext_gate_depth unconditionally and cleared it to False on exit, with no save-restore. The inner gate clobbered the outer gate's state in both directions:

• Descending into inner overwrote the flag with False, so the crz decomposition inside the body outer was skipping recorded its depth → 13.
• When inner returned it left the flag False, so outer never recorded its own single depth → the ["inner", "outer"] row reported 2 instead of 1.

That is why save-restore alone is not sufficient: it fixes ["inner", "outer"] and leaves ["outer"] at 13.

The fix

Two changes in _visit_custom_gate_operation:

1. Save the flag and restore it in a finally block, matching the pattern fix: record an external gate's own depth, not its skipped decomposition's #359 introduced in _visit_external_gate_operation.
2. Compute prev_recording or is_external rather than assigning, so an inner gate stays suppressed inside an enclosing external gate, and record the depth under is_external and not prev_recording — from the outermost external gate only.

Verification

• Full suite: 789 passed, 4 skipped. No existing expectation changed.
• tox -e format-check: pylint 10.00/10, isort, black, mypy and headers all clean.
• Mutation-tested. With the source fix reverted, both new parametrizations fail, and they fail differently — ["outer"] at 13, ["inner", "outer"] at 2 — which is the point: they pin two distinct halves of the bug.

Tests added

In tests/qasm3/test_depth.py, beside test_external_basic_gate_counts_own_depth:

• test_nested_external_custom_gate_counts_own_depth, parametrized over ["outer"] and ["inner", "outer"], asserting one emitted statement and depth() == 1.
• test_nested_custom_gate_depth_unchanged_without_external_gates — the same program still decomposes to 13 when nothing is external, so the flag handling cannot silently suppress ordinary counting.
• test_inner_external_gate_records_depth_inside_plain_custom_gate — external_gates=["inner"] still records the inner gate's own depth when the enclosing gate is not external (2, not 1).

The last two pass on main; they are regression guards, not bug pins.

Summary by CodeRabbit

• Bug Fixes

  • Improved recursive custom-gate validation to detect indirect cycles of any length and report the complete cycle path.
  • Corrected nested external-gate depth tracking, ensuring operation counts reflect the appropriate outermost and inner gates.
  • Preserved depth calculations when gates are fully decomposed or nested in different configurations.

• Tests

  • Added regression coverage for recursive gate definitions, repeated gate calls, and nested external-gate depth scenarios.

[argus-eye]

Argus review

Auto-review is off for this repo. Tick the box below to run a review on this PR.

• Trigger Argus review

Estimated cost

• Files changed: 3
• Diff lines (±): 112
• Historical avg: ~243.6k tokens · ~$0.95 · across last 10 review(s)

Tip: you can also comment @argus-eye review at any time.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 676d812b-bfbf-4e1d-96c9-96063cca2133

📥 Commits

Reviewing files that changed from the base of the PR and between d27f2be and 1c6bd07.

📒 Files selected for processing (5)

• CHANGELOG.md
• src/pyqasm/visitor.py
• tests/qasm3/resources/gates.py
• tests/qasm3/test_depth.py
• tests/qasm3/test_gates.py

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change updates custom-gate expansion to detect indirect recursive definitions and preserve nested external-gate depth state. Regression tests cover recursive cycles, shared gate expansion, repeated calls, and nested external-gate depth.

Changes

Gate expansion behavior

Layer / File(s)	Summary
Expansion cycle and depth control src/pyqasm/visitor.py, CHANGELOG.md	QasmVisitor tracks active custom gates, reports complete recursive cycle paths, restores nested external-gate state, and records depth only for outermost external gates.
Recursive expansion regression coverage tests/qasm3/resources/gates.py, tests/qasm3/test_gates.py	Tests cover shared descendants, repeated gate calls, and indirect two-gate and three-gate recursive definitions.
Nested external-gate depth coverage tests/qasm3/test_depth.py	Tests verify outer external gates count as one operation, full decomposition retains depth 13, and inner external gates contribute depth 2 in a plain custom gate.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 1c6bd

This PR corrects depth reporting for nested external custom gates and rejects cyclic gate definitions, with focused regression coverage; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant QASMInput
  participant QasmVisitor
  participant ExpandedCircuit
  QASMInput->>QasmVisitor: visit custom-gate operation
  QasmVisitor->>QasmVisitor: track active gate chain
  QasmVisitor->>ExpandedCircuit: emit external gate or decomposed operations
  QasmVisitor-->>QASMInput: report recursive cycle path when detected

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The cyclic gate-definition detection changes are not related to the requirements in directly linked issue #367.	Link the cycle-detection work to a relevant issue or separate it into another pull request.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both primary changes: nested external-gate depth tracking and cyclic gate-definition detection.
Linked Issues check	✅ Passed	The implementation satisfies issue #367 by correcting nested external-gate depth tracking and adding regression coverage for the required configurations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-nested-external-gate-depth

---

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.}

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[TheGupta2012]

coderabbit generated no comments, merging