fix: remove_idle_qubits KeyError on shared operand nodes

[SAY-5]

Summary of changes

Closes #331.

unroll() can emit gate statements that share the same IndexedIdentifier operand node (the crz decomposition does this), and _remap_qubits rewrites bit.indices[0][0].value in place per statement, so a node shared by k statements gets remapped k times. With a lower-indexed idle qubit the repeated lookups walk off the index map and raise KeyError, e.g. crz(0.5) q[1], q[2]; on a 3-qubit register.

This tracks visited node ids so each operand node is remapped exactly once, and adds a regression test with the minimal repro from the issue. The remap now also produces correct indices for shared nodes rather than silently double-remapping them when the shift happens to stay inside the map.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ed5f8066-a76e-439a-9e4f-bb14e5ea34eb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

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

[ryanhill1]

Thanks @SAY-5 — this is a clean, minimal fix. Approving.

Verification done during review:

• Reproduced the KeyError: 0 from #331 on main and confirmed this branch fixes it.
• Checked the remapped output is semantically correct: the resulting 2-qubit circuit is unitarily equivalent to crz(0.5) via qiskit's Operator.equiv.
• Adversarial probes all pass: calling remove_idle_qubits() twice, in_place=False (deepcopy memoization preserves the aliasing, so the guard still works on copies), multiple registers each with idle qubits and aliased operands, a custom gate spanning two registers, and measurements mixed in. Also confirmed unroll() never shares an index-literal node across different registers, so scoping visited_node_ids per _remap_qubits call is sound.
• Full test suite passes locally (the handful of failures in my env are a missing optional tabulate dep and reproduce on main).

One optional, non-blocking suggestion: the sibling tests in test_transformations.py compare the full unrolled program via check_unrolled_qasm. Doing the same here would also lock in the rotation-gate indices — i.e. the "silent double-remap when the shift stays inside the map" case from the PR description, which the current in/not in assertions don't pin down. Fine to land as-is or as a follow-up.

Heads-up: while reviewing I found the same root cause bites reverse_qubit_order() — same repro, KeyError: -1 from the negative-marker pass reading an already-marked shared node. Filed as #333 (with the same two fix options as #331; the deep-copy-at-unroll route would retire the whole bug class). If you're interested in picking that one up too, it's yours — the mechanics are exactly what you just fixed here. Also filed the unrelated pre-existing #334 spotted nearby.

[ryanhill1]

Pushed two small maintenance commits to your branch (via maintainer-edit) to unblock the merge: a merge of main resolving the CHANGELOG conflict with #330, and a one-line pylint fix (R0914 — dropped the redundant old_idx local in _remap_qubits, using index_node.value directly in the map lookup). No behavior changes; remember to git pull before any further edits on this branch.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ryanhill1]

Re-approving after the stale-review dismissal — the substantive change is unchanged from my original review; the two new commits are the main merge (CHANGELOG conflict with #330) and the one-line pylint R0914 fix.