fix(qudora): correct backend field mapping against the live API - #1306
Open
ryanhill1 wants to merge 2 commits into
Open
fix(qudora): correct backend field mapping against the live API#1306ryanhill1 wants to merge 2 commits into
ryanhill1 wants to merge 2 commits into
Conversation
Verified the provider against api.qudora.com with a real token. The mocked
fixtures were modeled on assumption rather than a captured response, and
diverged from what the API returns in ways that hid two bugs:
- `_build_profile` read `backend["id"]`, a key no backend record has (the
numeric identifier is `user_id`). `qudora_backend_id` was therefore `None`
for every device, so `QudoraDevice.status()` always took its "id
unavailable" branch and reported ONLINE without calling the status
endpoint, which was unreachable in practice. `/backends/status/108`
returns "Idle"; the `username` is rejected with a 422.
- `QudoraJob.result()` read `device_id` from the job record's `target`, but
QUDORA echoes the backend's display name there ("QVLS-Q1 Emulator"), not
the id jobs are submitted against. Results came back with a `device_id`
matching no device the provider can return; it now prefers the device's id.
The listing also publishes `simulator`, `basis_gates`, `status_name` and
`is_online`, so `simulator=True` is no longer hardcoded and `basis_gates`
is populated. Device ids are backend `username`s, which are email addresses.
Per CONTRIBUTING, required fields are now indexed directly rather than
fetched with a default: an unmapped `JobStatusName` raises instead of
degrading to `JobStatus.UNKNOWN`, which is not terminal and would have made
`result()` poll a finished job until `wait_for_final_state` timed out.
Annotations use PEP 604 syntax, and the mypy exclusion is dropped — the
remaining errors are the same structural ones rigetti, quantinuum and pasqal
already produce unexcluded.
Fixtures are rebuilt from captured payloads, and `test_qudora_remote.py`
adds credentialed tests behind the `remote` marker that assert the response
shape the unit tests hard-code, so upstream drift fails loudly. Adding
`tests/runtime/qudora/__init__.py` also puts the suite back under pylint,
which skipped the directory entirely for lack of a package marker.
Registers `qudora` in docs/api/qbraid.runtime.rst, the only provider missing.
Argus reviewAuto-review is off for this repo. Tick the box below to run a review on this PR.
Estimated cost
Tip: you can also comment |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
`_detect_language` scanned line-by-line for a prefix of "OPENQASM", which
reads a version named inside a block comment as the program's own:
/*
OPENQASM 2.0;
*/
OPENQASM 3.0;
...
returned OpenQASM2, sending the wrong `language` to QUDORA. It also accepted
`OPENQASM 3` with no semicolon as a valid declaration.
Delegates to `qbraid.programs.typer.get_qasm_type_alias`, which routes through
pyqasm's `extract_qasm_version` (already a dependency here via `_validate_qasm`):
comments are stripped before the header is located, and the version must match
`OPENQASM <major>[.<minor>];`. The alias is mapped to the QUDORA language, so a
dialect QUDORA does not accept — `qasm2_kirin` — is now named in the error
rather than silently matching the "OPENQASM 2" prefix.
Also documents why `submit()` defaults `name` to "qbraid" rather than omitting
it. QUDORA's schema requires a string: both a null value and a missing key are
rejected with 422 `{"type": "string_type", "loc": ["body", "name"]}`, verified
against the live API and pinned by a remote test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targets
feature/qudora-integration(#1292), notmain.I ran the provider against
api.qudora.comwith a real token while reviewing #1292. The mocked fixtures were modeled on assumption rather than a captured response, and diverge from what the API actually returns — which hid two bugs.What the live API returns
username"qamelion""xg1-emulator-gwdg@qudora.com"(an email)"id": 1idkey — it isuser_idsimulatorTruestatus_name/is_onlinemax_programs_per_jobbasis_gates["rxx", "r"]Bugs this surfaced
Every device reported ONLINE unconditionally.
_build_profilereadbackend.get("id"), a key no record has, soqudora_backend_idwas alwaysNoneandstatus()took its "id unavailable ⇒ assume online" branch every time. The status endpoint was unreachable in practice.GET /backends/status/108returns"Idle"; passing theusernamegives a 422.Result.device_idwas the display name. QUDORA echoes the backend'sfull_nameback in the job record'starget, so a real job produceddevice_id="QVLS-Q1 Emulator"— matching no device the provider can return.result()now prefers the device's own id.Changes
provider.py—simulatorandbasis_gatesfrom the real fields,qudora_backend_idfromuser_id, required fields indexed directly. Drops the two comments asserting fields don't exist, including theplans/qudora/qudora-sdk.mdreference (not in the repo).device.py— removes the ONLINE fallback;_DEVICE_STATUS_MAP[status_name]indexed directly.job.py—_JOB_STATUS_MAPhoisted to module level (matching_QUANTINUUM_STATUS_MAP) and indexed directly. An unmappedJobStatusNamenow raises rather than degrading toJobStatus.UNKNOWN, which is not a terminal state and would have maderesult()poll a finished job untilwait_for_final_statetimed out.str | None), per the CONTRIBUTING section merged in docs: tighten Unreleased changelog entries and add length guidance #1288.pyproject.toml— drops the mypy exclusion. Both qudora-specific errors are fixed; the remaining ones are the same structural errors rigetti, quantinuum and pasqal already produce unexcluded.docs/api/qbraid.runtime.rst— registersqudora, the only provider missing from the submodule list.Tests
Fixtures rebuilt from captured payloads (
BACKENDSverbatim from the live listing,_job_recordmodeled on real job 85974). New cases cover theuser_id-vs-idtrap, unmapped statuses raising, missing-field failures, and thedevice_iddivergence.test_qudora_remote.pyadds 7 tests behind theremotemarker, asserting the response shape the unit fixtures hard-code so upstream drift fails there instead of silently.Adding
tests/runtime/qudora/__init__.pymatters more than it looks: without a package marker,pylint qbraid bin testsskipped the entire test file in CI. Adding it surfaced 5 line-length violations, now fixed.Verified end-to-end on
simulator-prod@qudora.com(jobs 85974–85977): single circuit, batch, and cancel all complete. Bit ordering confirmed —x q[0]gives{"01": 199, "00": 1}and a batch ofx q[0]/x q[1]gives[{"01": 100}, {"10": 99, "11": 1}], so qubit 0 is rightmost. The existing orientation test's assumption was correct; it just had no ground truth behind it.tests/runtime/qudora: 64 passed (including 7 live-API tests)tests/runtime: 916 passed, 74 skippedtox -e format-checkequivalent: pylint 10.00/10, isort/black/ruff/headers clean🤖 Generated with Claude Code