fix: dt duration unit incorrectly mapped to ns (#301)

[ashmitjsg]

Summary

Fixes #301. The backend-dependent dt duration unit was being relabeled as ns
during unrolling of delay and box statements when no device_cycle_time is set.
Since dt can't be converted to SI units without a sample rate, it's now preserved.

Root cause

In _visit_delay_statement / _visit_box_statement, the output unit was chosen
solely from whether device_cycle_time was set, ignoring the source unit. SI units
were unaffected (the evaluator converts them to ns first); only dt was mislabeled.

Changes

• Add _resolve_duration_unit helper that preserves dt when the source literal
  was dt (or a device cycle time is set), else ns.
• Apply it at both the delay and box duration sites.
• Tests for dt preservation in test_delay.py and test_box.py.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Fixed duration unit preservation in delay and box statements. The dt (device time) unit is now correctly maintained when unrolling statements without a device cycle time.

• Tests

  • Added tests to validate duration unit handling in delay and box statements.

[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

Run ID: 96f6422d-0e3c-4716-95bf-7a8f736103b8

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

📝 Walkthrough

Walkthrough

This PR fixes a bug where the backend-dependent dt duration unit was incorrectly converted to ns during unrolling of delay and box statements. A new helper method centralizes duration-unit resolution logic and is applied to both statement types, with comprehensive tests validating the fix.

Changes

dt Duration Unit Preservation

Layer / File(s)	Summary
Duration unit resolution helper and integration src/pyqasm/visitor.py	New _resolve_duration_unit method preserves dt when the source literal is dt or the module has a device cycle time set, otherwise emits ns. The helper replaces inline conditional logic in _visit_delay_statement and _visit_box_statement.
Tests for dt unit preservation tests/qasm3/test_delay.py, tests/qasm3/test_box.py	New tests test_delay_dt_unit_preserved and test_box_dt_unit_preserved verify that delay[100dt] and box[200dt] preserve dt units after unrolling and serialization, while non-dt units like us are converted to ns.
Changelog entry CHANGELOG.md	Documents the fix: dt duration unit is now preserved instead of incorrectly relabeled as ns when unrolling delay and box statements without device_cycle_time.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: dt duration unit incorrectly mapped to ns (#301)' accurately describes the main change: fixing a bug where dt units were being converted to ns during unrolling.
Linked Issues check	✅ Passed	The pull request fully addresses issue #301 by preserving dt units during unrolling instead of incorrectly mapping them to ns, as required by the linked issue.
Out of Scope Changes check	✅ Passed	All changes directly address the dt unit preservation issue: helper method addition, duration literal construction updates, and comprehensive unit tests validating the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/pyqasm/visitor.py`:
- Line 2918: The overflow error message for box[...dt] is using a hardcoded 'ns'
instead of the resolved unit; update the code that constructs the box overflow
error to use the resolved unit returned by
self._resolve_duration_unit(_box_time_var) (the same value passed as
unit=self._resolve_duration_unit(_box_time_var)), so the error text reports the
preserved AST unit rather than always showing "ns".

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 986647fc-5e9b-4fbc-81ce-323067760282

📥 Commits

Reviewing files that changed from the base of the PR and between abefaf3 and 182083a.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/pyqasm/visitor.py
• tests/qasm3/test_box.py
• tests/qasm3/test_delay.py

[ryanhill1]

LGTM, will wait for approval from @TheGupta2012 before merging

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[TheGupta2012]

Thanks @ashmitjsg , changes look good!

changes resolved