Fix bit order of unrolled classical register conditions

[ryanhill1]

Summary of changes

When unroll() expands a whole-register condition into per-bit branches, it uses the wrong bit order. It reads c[0] as the most significant bit, but in OpenQASM c[0] is the least significant. So if (c == 2) runs its body when c == 1, and nothing reports an error.

bit[2] c;
if (c == 2) { x q[0]; }

	Unrolled output
main	if (c[0] == true) { if (c[1] == false) { ... } } (this tests for c == 1)
this PR	if (c[1] == true) { if (c[0] == false) { ... } }

I simulated both versions with Qiskit Aer, preparing c = 2. The original program flips the flag qubit, but main's unrolled output does not.

Same bug elsewhere in the conversion

• >=, <=, >, < skipped bits that looked irrelevant, which was only correct for some values. For example, c >= 1 reduced to a check on one bit.
• Comparison values outside the register's range produced indices past the end of the register (c == 4 on bit[2] raised ValidationError) or nonsense like bin(-1). Now a comparison that always holds unrolls to the plain body, and one that never holds unrolls to the else branch.
• Single-bit conditions ignored the operator and coerced the value to bool. So c[0] < 1 became c[0] == true, and c[0] == 2 became c[0] == true.

Fix

The bits-to-condition logic now lives in one helper, Qasm3Transformer.unroll_register_comparison. It treats c[0] as the LSB and tests bits from MSB to LSB. At the first bit that differs from the value, it jumps to if_block or else_block. Comparisons that are constant over the register's range are folded away. Single-bit conditions go through the same helper after they are reduced to the bit values that satisfy them.

Tests

• An exhaustive check: for each operator, register widths 1–3, comparison values from −1 to 2ʷ, and every register value, it walks the unrolled branches and compares the result with Python's integer comparison.
• Exact expected bits for c == N at widths 1, 2, 3 and 5, plus the minimal c == 2 case as text.
• Single-bit cases: c[1] < 1, c[1] == 2, c[1] >= 0, !c[1].
• Updated test_multi_bit_if and test_rebase_if, whose expected output encoded the bug.
• Also run locally against Qiskit Aer (not added to the suite): 648 condition/register-value cases, all matching.

Interaction with #338

#338 rebuilds QASM 2 if(c==N) from the per-bit chain and works around this bug by reversing the bit order. Once this PR lands, that reversal produces wrong values. Whichever of the two PRs merges second must include this one-line change in src/pyqasm/modules/qasm2.py, _flatten_branch:

-        # unrolling ravels the comparison value MSB-first, so invert that ordering here
-        reg_value = sum(1 << (size - 1 - index) for index, set_ in bit_values.items() if set_)
+        reg_value = sum(1 << index for index, set_ in bit_values.items() if set_)

I checked this by applying #338 on top of this branch. Without the flip, 3 of #338's own tests fail (test_multibit_branch_survives_unrolling[1|2] and test_branch_on_register_level_measurement_round_trips). With the flip, the full suite passes, and if(c==N) round-trips for N = 0–3.

Summary by CodeRabbit

• Bug Fixes
  • Fixed unrolling of classical-register conditions so equality and relational comparisons, including out-of-range values, follow the correct bit order.
  • Corrected single-bit comparisons and negation to select the appropriate branches.
  • Updated rebased conditional operations to reflect the corrected condition handling.

Also in this PR:

• Updates visitor handling for bit-level inequality conditions during conversion.

_{Auto-enriched by Argus}

[argus-eye]

Argus review

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

Running Argus review...

Estimated cost

• Files changed: 4
• Diff lines (±): 392
• Historical avg: ~317.3k tokens · ~$1.35 · across last 12 review(s)

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

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: Repository: qBraid/pyqasm/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 605942e2-1877-45f3-b482-11671cfbb9ec

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: qBraid/pyqasm/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ed4f9333-2d7c-446f-91af-af8163a83057

📥 Commits

Reviewing files that changed from the base of the PR and between 520a020 and d3b609a.

📒 Files selected for processing (5)

• CHANGELOG.md
• src/pyqasm/transformer.py
• src/pyqasm/visitor.py
• tests/qasm3/test_if.py
• tests/qasm3/test_rebase.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Classical-register condition unrolling now tests bits from most significant to least significant. Single-bit comparisons and negation receive explicit handling. Tests cover equality, relational comparisons, out-of-range values, and rebasing output.

Changes

Classical Register Conditions

Layer / File(s)	Summary
Condition dispatch and branch construction src/pyqasm/visitor.py, src/pyqasm/transformer.py	The visitor evaluates single-bit conditions and passes multi-bit comparison operators and integer values to the transformer. The transformer builds nested branches with bits tested from most significant to least significant, and returns a branch block directly when the comparison is uniformly true or false across the register range.
Comparison and rebase validation tests/qasm3/test_if.py, tests/qasm3/test_rebase.py, CHANGELOG.md	Tests check bit ordering, equality and relational results, out-of-range values, single-bit conditions, and rebasing output. The changelog describes the corrected condition handling.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant QasmVisitor
  participant Qasm3Transformer
  participant BranchingStatement
  QasmVisitor->>Qasm3Transformer: Pass register, operator, value, and branch blocks
  Qasm3Transformer->>BranchingStatement: Build nested MSB-first bit branches
  Qasm3Transformer-->>QasmVisitor: Return generated branch statements

Loading

Merge Risk: ⚪ Minimal · up to d3b60

This change corrects the bit order used when unroll() converts classical register conditions. It is backed by broad comparison tests, and no outstanding merge-blocking risk was identified. The author notes that PR #338 must drop its QASM 2 bit-order reversal if it merges after this one.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 and concisely describes the main change: correcting bit order when unrolling classical register conditions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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!

[argus-eye]

Argus · 9/10 — Bit inequality raises KeyError

Check 1 warning before you merge. Argus also found 1 suggestion in 5 files.

• 🟡 Warning · Bit inequality raises KeyError · src/pyqasm/visitor.py:L97
• 💡 Suggestion · Constant comparisons lack else-branch coverage · tests/qasm3/test_if.py:L300

What this PR does · 5 author checks

• Fix unrolled classical-register conditions so comparisons use the correct bit order and select the correct branch.

Author's checks

• Treat c[0] as the least significant bit and test register bits from most significant to least significant.
• Make equality and relational comparisons choose the same branch as integer comparison, including when the comparison value is outside the register's range.
• Fold comparisons that always hold to the plain body and comparisons that never hold to the else branch.
• Route single-bit conditions through the shared comparison logic so operators and values select the correct branches.
• Pass the exhaustive checks across operators, register widths 1–3, comparison values from −1 through 2ʷ, and every register value.

⚠️ Argus could not finish 5 file checks, so this review is partial. More findings can exist.

4 earlier findings may still apply to files this PR changes

• src/pyqasm/visitor.py · Branch-condition normalization is not written back into the AST · from qBraid/pyqasm#404
• src/pyqasm/visitor.py · Whole-register assignment discards the bit register's width metadata · from qBraid/pyqasm#404
• src/pyqasm/visitor.py · Mixed-program detection checks total declared capacity instead of declaration presence · from qBraid/pyqasm#362
• src/pyqasm/visitor.py · Custom gates double-count branch operations after expansion · from qBraid/pyqasm#345

_{usage: 744.1k tokens · 16 stages}

_{Contract: production/full · checked: bug_hunter, security, architecture, regression · review took 2m30s}

Stage	Model	Tokens	Cost
Intent	openai/gpt-5.6-terra	4.7k	$0.0056
Triage	openai/gpt-6-luna	9.4k	$0.0007
Lead agent	openai/gpt-5.6-terra	1.8k	$0.0105
Review specialists (11)	openai/gpt-6-sol	404.6k	$0.9225
Simulation	openai/gpt-5.6-terra	20.7k	$0.0813
Scoring	openai/gpt-5.6-terra	3.0k	$0.0079

---

<sub>Dashboard → · React 👎 on an inline comment to dismiss it · Reply to an inline comment or use @argus-eye help

🧠 Learned: 2 patterns · 2 finding memories · 2 file memories · 1 PR summary · …</sub>

[TheGupta2012]

@ryanhill1 I have merged #337 and updated the tests as well, lgtm overall