Skip to content

fix: record an external gate's own depth, not its skipped decomposition's - #359

Open
ryanhill1 wants to merge 2 commits into
mainfrom
fix-external-gate-depth
Open

fix: record an external gate's own depth, not its skipped decomposition's#359
ryanhill1 wants to merge 2 commits into
mainfrom
fix-external-gate-depth

Conversation

@ryanhill1

@ryanhill1 ryanhill1 commented Aug 7, 2026

Copy link
Copy Markdown
Member

Fixes #352

_visit_external_gate_operation calls _visit_basic_gate_operation for validation only, but that call still ran _update_qubit_depth_for_gate over the decomposition that was never emitted — so unroll(external_gates=["crz"]) on a single crz reported depth() == 12 while emitting one statement. Since #341 the same path handles every decomposable gate inside a #pragma braket verbatim box.

Depth recording is now suppressed around the validation-only call (the same _recording_ext_gate_depth mechanism the custom-gate path uses), and the external gate records its own depth — including the branch-qubit marking variant inside conditionals.

…on's

_visit_external_gate_operation ran _visit_basic_gate_operation for
validation only, but that call still updated qubit depths over the
decomposition that was never emitted. Depth recording is now suppressed
around the validation call and the external gate records its own depth,
mirroring the existing custom-gate path. Affects both explicit
external_gates and braket verbatim boxes.

Fixes #352
@ryanhill1
ryanhill1 requested a review from TheGupta2012 as a code owner August 7, 2026 13:11
@argus-eye

argus-eye Bot commented Aug 7, 2026

Copy link
Copy Markdown

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: 4
  • Diff lines (±): 82
  • Historical avg: ~318.9k tokens · ~$1.35 · across last 6 review(s)

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

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

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: 69084bf1-5dd7-4e24-bc60-bbd919a67c70

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

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

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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pyqasm/visitor.py 66.66% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@ryanhill1

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

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.

@ryanhill1

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

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.

@ryanhill1

Copy link
Copy Markdown
Member Author

@Argus-Eye review

@argus-eye

This comment has been minimized.

@argus-eye argus-eye Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔎 Argus · 10/10 — Argus reviewed 4 files and found no issues.

🔍 PR intent vs diff (LLM analysis)

Argus read the diff against the stated intent. This is not an execution log — reviewer still needs to test behavior.

Goal: Ensure external gates record their own depth instead of the depth of a decomposition skipped during validation.
Stated acceptance criteria (from PR/issue — not independently verified):

  • Suppress depth recording around the validation-only _visit_basic_gate_operation call.
  • Record the external gate's own depth for explicit external_gates and Braket verbatim boxes.
  • Record the external gate's own depth in the branch-qubit marking variant inside conditionals.

✅ Intent delivered

Argus reviewed 4 files and found no issues. Code looks good.


Simulated 2 scenarios — all pass.

🔢 124.9k tokens · $0.3314 total
Stage Tokens Cost
Intent 2.5k $0.0000
Triage 2.7k $0.0000
Lead agent 1.6k $0.0000
Review · bug_hunter 20.7k $0.0857
Review · security 20.4k $0.0746
Review · architecture 20.5k $0.0845
Review · regression 20.8k $0.0866
Review 24.2k $0.0000
Acceptance 1.2k $0.0000
Simulation 10.4k $0.0000

Contract: production/full · checked: bug_hunter, security, architecture, regression · review took 1m30s

Dashboard → · React 👎 to dismiss · Reply to any inline comment or use @argus-eye help to chat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

External/verbatim gates report the depth of the decomposition they skipped

2 participants