Code review improvements: bug fixes, test coverage, simplifications

splch · splch · commit f4aa5f5ed256 · 2026-04-09T14:11:25.000+09:00
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -7,8 +7,26 @@ on:
 permissions:
   contents: read
 
+env:
+  UV_FROZEN: true
+
 jobs:
+  test:
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
+        with:
+          persist-credentials: false
+      - uses: astral-sh/setup-uv@cec208311dfd045dd5311c1add060b2062131d57 # v8.0.0
+        with:
+          python-version: "3.12"
+          enable-cache: false
+      - run: uv sync
+      - run: uv run pytest
+
   build:
+    needs: test
     runs-on: ubuntu-latest
     timeout-minutes: 10
     steps:
diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,16 @@
 __pycache__/
 *.pyc
-.venv/
-dist/
 *.egg-info/
+dist/
+build/
+.venv/
 .ruff_cache/
+.pytest_cache/
+.mypy_cache/
+htmlcov/
+.coverage
+docs/
+.DS_Store
+.env
+.idea/
+*.orig
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -32,6 +32,7 @@ Most of the code in `ionq_core/` is **auto-generated** from the IonQ OpenAPI spe
 - `ionq_core/__init__.py` -- public API exports
 - `ionq_core/ionq_client.py` -- IonQClient convenience wrapper
 - `ionq_core/_exceptions.py` -- exception hierarchy
+- `ionq_core/_extensions.py` -- extension API for downstream SDKs
 - `ionq_core/_transport.py` -- retry transport
 - `ionq_core/_pagination.py` -- pagination helpers
 - `ionq_core/_polling.py` -- job polling helpers
@@ -48,7 +49,7 @@ else
     cp openapi.json /tmp/patched-spec.json
 fi
 
-uvx openapi-python-client generate \
+uvx openapi-python-client==0.28.3 generate \
     --path /tmp/patched-spec.json \
     --meta none \
     --config openapi-python-client-config.yaml \
diff --git a/README.md b/README.md
@@ -141,7 +141,7 @@ All exceptions inherit from `APIError`, which inherits from `IonQError`. Connect
 
 ## Retries
 
-The client automatically retries transient errors (429, 500, 502, 503) and connection/timeout failures with exponential backoff. Default: 2 retries.
+The client automatically retries transient errors (429, 500, 502, 503, 520-529) and connection/timeout failures with exponential backoff. Default: 2 retries.
 
 ```python
 client = IonQClient(max_retries=5)  # more retries
@@ -332,7 +332,7 @@ else
 fi
 
 # Regenerate (preserves ionq_client.py)
-uvx openapi-python-client generate \
+uvx openapi-python-client==0.28.3 generate \
     --path /tmp/patched-spec.json \
     --meta none \
     --config openapi-python-client-config.yaml \
@@ -347,11 +347,11 @@ If the upstream spec contains patterns that the code generator cannot handle, fi
 ## Development
 
 ```sh
-uv sync                              # Install dependencies
-uv run pytest                        # Run tests
-uv run ruff check ionq_core/ tests/  # Lint
-uv run ruff format ionq_core/ tests/ # Format
-uv run ty check ionq_core/           # Type check
+uv sync                    # Install dependencies
+uv run pytest              # Run tests
+uv run ruff check          # Lint
+uv run ruff format --check # Check formatting
+uv run ty check ionq_core/ # Type check
 ```
 
 ## License
diff --git a/ionq_core/_exceptions.py b/ionq_core/_exceptions.py
@@ -1,5 +1,7 @@
 """Structured exceptions for the IonQ API client."""
 
+from __future__ import annotations
+
 
 class IonQError(Exception):
     """Base exception for all IonQ API errors."""
diff --git a/ionq_core/_transport.py b/ionq_core/_transport.py
@@ -1,11 +1,12 @@
 """Retry transport for httpx with exponential backoff."""
 
 import asyncio
+import calendar
 import email.utils
 import logging
 import random
 import time
-from collections.abc import Generator
+from collections.abc import Iterator
 from typing import NoReturn
 
 import httpx
@@ -34,11 +35,11 @@ def _parse_retry_after(response: httpx.Response) -> float | None:
         pass
     parsed = email.utils.parsedate(header)
     if parsed is not None:
-        return max(0.0, time.mktime(parsed) - time.time())
+        return max(0.0, calendar.timegm(parsed) - time.time())
     return None
 
 
-def _backoff_delays(max_retries: int) -> Generator[float]:
+def _backoff_delays(max_retries: int) -> Iterator[float]:
     yield 0.0
     for attempt in range(max_retries):
         base = 0.5 * (2**attempt)
diff --git a/ionq_core/ionq_client.py b/ionq_core/ionq_client.py
@@ -32,10 +32,8 @@ def _build_user_agent(*tokens: str | None) -> str:
         f"python/{platform.python_version()}",
         f"httpx/{httpx.__version__}",
         f"os/{platform.system().lower()}",
+        *filter(None, tokens),
     ]
-    for token in tokens:
-        if token:
-            parts.append(token)
     return " ".join(parts)
 
 
@@ -107,9 +105,7 @@ def IonQClient(
     effective_timeout = (
         extension.timeout if (extension and extension.timeout is not None) else (timeout or _DEFAULT_TIMEOUT)
     )
-    effective_retries = (
-        extension.max_retries if (extension and extension.max_retries is not None) else max_retries
-    )
+    effective_retries = extension.max_retries if (extension and extension.max_retries is not None) else max_retries
     effective_retry_codes = (
         extension.retryable_status_codes
         if (extension and extension.retryable_status_codes is not None)
diff --git a/pyproject.toml b/pyproject.toml
@@ -10,7 +10,9 @@ authors = [{name = "IonQ", email = "support@ionq.co"}]
 keywords = ["ionq", "quantum", "quantum-computing", "sdk", "api-client"]
 classifiers = [
     "Development Status :: 3 - Alpha",
+    "Intended Audience :: Developers",
     "License :: OSI Approved :: Apache Software License",
+    "Operating System :: OS Independent",
     "Programming Language :: Python :: 3",
     "Programming Language :: Python :: 3.12",
     "Programming Language :: Python :: 3.13",
@@ -49,9 +51,8 @@ build-backend = "hatchling.build"
 packages = ["ionq_core"]
 
 [tool.ruff]
-line-length = 120
 target-version = "py312"
-preview = true
+line-length = 120
 extend-exclude = [
     "ionq_core/api",
     "ionq_core/models",
@@ -64,11 +65,14 @@ extend-exclude = [
 docstring-code-format = true
 
 [tool.ruff.lint]
-select = ["E", "F", "I", "UP", "B", "SIM"]
+select = ["E", "F", "I", "UP", "B", "SIM", "RUF"]
 
 [tool.ruff.lint.isort]
 known-first-party = ["ionq_core"]
 
+[tool.ruff.lint.per-file-ignores]
+"tests/**" = ["RUF012"]
+
 [tool.ty.environment]
 python-version = "3.12"
 
@@ -80,7 +84,9 @@ invalid-argument-type = "ignore"
 [tool.pytest.ini_options]
 testpaths = ["tests"]
 asyncio_mode = "auto"
+xfail_strict = true
+filterwarnings = ["error"]
 markers = [
     "integration: marks tests that hit the real IonQ API (deselect with '-m \"not integration\"')",
 ]
-addopts = "-m 'not integration'"
+addopts = "-m 'not integration' --tb=short"
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -5,11 +5,8 @@
 
 import pytest
 
-from ionq_core import IonQClient
+from ionq_core import AuthenticatedClient, IonQClient
 from ionq_core.api.default import delete_job
-from ionq_core.client import AuthenticatedClient
-
-pytestmark = pytest.mark.integration
 
 _job_ids: list[str] = []
 
diff --git a/tests/test_api.py b/tests/test_api.py
@@ -110,15 +110,17 @@ def test_sync(self, httpx_mock, auth_client):
             status_code=201,
             json={"id": "new-job-id", "status": "submitted", "session_id": None},
         )
-        body = CircuitJobCreationPayload.from_dict({
-            "type": "ionq.circuit.v1",
-            "backend": "simulator",
-            "shots": 100,
-            "input": {
-                "gateset": "qis",
-                "circuit": [{"gate": "h", "targets": [0]}],
-            },
-        })
+        body = CircuitJobCreationPayload.from_dict(
+            {
+                "type": "ionq.circuit.v1",
+                "backend": "simulator",
+                "shots": 100,
+                "input": {
+                    "gateset": "qis",
+                    "circuit": [{"gate": "h", "targets": [0]}],
+                },
+            }
+        )
         result = create_job.sync(client=auth_client, body=body)
         assert isinstance(result, JobCreationResponse)
         assert result.id == "new-job-id"
diff --git a/tests/test_pagination.py b/tests/test_pagination.py
@@ -1,4 +1,4 @@
-from ionq_core import aiter_jobs, iter_jobs
+from ionq_core import aiter_jobs, aiter_session_jobs, iter_jobs, iter_session_jobs
 
 
 def _jobs_page(job_ids, next_cursor=None):
@@ -47,3 +47,29 @@ async def test_multiple_pages(self, httpx_mock, auth_client):
         httpx_mock.add_response(json=_jobs_page(["j2"]))
         jobs = [j async for j in aiter_jobs(auth_client)]
         assert [j.id for j in jobs] == ["j1", "j2"]
+
+
+class TestIterSessionJobs:
+    def test_single_page(self, httpx_mock, auth_client):
+        httpx_mock.add_response(json=_jobs_page(["j1", "j2"]))
+        jobs = list(iter_session_jobs(auth_client, "sess-1"))
+        assert [j.id for j in jobs] == ["j1", "j2"]
+
+    def test_multiple_pages(self, httpx_mock, auth_client):
+        httpx_mock.add_response(json=_jobs_page(["j1"], next_cursor="cursor1"))
+        httpx_mock.add_response(json=_jobs_page(["j2"]))
+        jobs = list(iter_session_jobs(auth_client, "sess-1"))
+        assert [j.id for j in jobs] == ["j1", "j2"]
+
+
+class TestAiterSessionJobs:
+    async def test_single_page(self, httpx_mock, auth_client):
+        httpx_mock.add_response(json=_jobs_page(["j1"]))
+        jobs = [j async for j in aiter_session_jobs(auth_client, "sess-1")]
+        assert [j.id for j in jobs] == ["j1"]
+
+    async def test_multiple_pages(self, httpx_mock, auth_client):
+        httpx_mock.add_response(json=_jobs_page(["j1"], next_cursor="cursor1"))
+        httpx_mock.add_response(json=_jobs_page(["j2"]))
+        jobs = [j async for j in aiter_session_jobs(auth_client, "sess-1")]
+        assert [j.id for j in jobs] == ["j1", "j2"]
diff --git a/tests/test_polling.py b/tests/test_polling.py
@@ -1,6 +1,6 @@
 import pytest
 
-from ionq_core._polling import JobFailedError, JobTimeoutError, wait_for_job
+from ionq_core._polling import JobFailedError, JobTimeoutError, async_wait_for_job, wait_for_job
 
 
 def _job_response(job_id, status, failure=None):
@@ -57,3 +57,22 @@ def test_timeout_raises(self, httpx_mock, auth_client, monkeypatch):
             httpx_mock.add_response(json=_job_response("j1", "submitted"))
         with pytest.raises(JobTimeoutError, match="j1"):
             wait_for_job(auth_client, "j1", poll_interval=0.01, timeout=0.0)
+
+
+class TestAsyncWaitForJob:
+    async def test_already_completed(self, httpx_mock, auth_client):
+        httpx_mock.add_response(json=_job_response("j1", "completed"))
+        job = await async_wait_for_job(auth_client, "j1", timeout=5)
+        assert job.status == "completed"
+
+    async def test_raises_on_failure(self, httpx_mock, auth_client):
+        httpx_mock.add_response(
+            json=_job_response("j1", "failed", failure={"code": "SimulationError", "message": "boom"})
+        )
+        with pytest.raises(JobFailedError, match="j1"):
+            await async_wait_for_job(auth_client, "j1", timeout=5)
+
+    async def test_canceled_returns(self, httpx_mock, auth_client):
+        httpx_mock.add_response(json=_job_response("j1", "canceled"))
+        job = await async_wait_for_job(auth_client, "j1", timeout=5)
+        assert job.status == "canceled"
diff --git a/tests/test_transport.py b/tests/test_transport.py
@@ -32,7 +32,7 @@ def close(self):
 
 
 def _response(status_code, headers=None, json_body=None):
-    return httpx.Response(status_code, headers=dict(headers or {}), json=json_body)
+    return httpx.Response(status_code, headers=headers or {}, json=json_body)
 
 
 def _request(method="GET"):
@@ -57,10 +57,12 @@ def test_retries_on_503_then_succeeds(self):
         assert fake._call_count == 2
 
     def test_retries_on_429_then_succeeds(self):
-        transport, fake = self._make_transport([
-            _response(429, headers={"retry-after": "0"}),
-            _response(200),
-        ])
+        transport, fake = self._make_transport(
+            [
+                _response(429, headers={"retry-after": "0"}),
+                _response(200),
+            ]
+        )
         resp = transport.handle_request(_request())
         assert resp.status_code == 200
         assert fake._call_count == 2
@@ -131,27 +133,33 @@ def test_max_retries_zero(self):
         assert fake._call_count == 1
 
     def test_error_body_parsed(self):
-        transport, _ = self._make_transport([
-            _response(400, json_body={"error": "Bad Request", "message": "Invalid input"}),
-        ])
+        transport, _ = self._make_transport(
+            [
+                _response(400, json_body={"error": "Bad Request", "message": "Invalid input"}),
+            ]
+        )
         with pytest.raises(BadRequestError) as exc_info:
             transport.handle_request(_request())
         assert exc_info.value.body == {"error": "Bad Request", "message": "Invalid input"}
 
     def test_error_message_surfaced(self):
-        transport, _ = self._make_transport([
-            _response(404, json_body={"message": "Job not found"}),
-        ])
+        transport, _ = self._make_transport(
+            [
+                _response(404, json_body={"message": "Job not found"}),
+            ]
+        )
         with pytest.raises(NotFoundError, match="Job not found"):
             transport.handle_request(_request())
 
     def test_retry_after_header_respected(self, monkeypatch):
         sleeps = []
         monkeypatch.setattr("ionq_core._transport.time.sleep", sleeps.append)
-        transport, _ = self._make_transport([
-            _response(429, headers={"retry-after": "10"}),
-            _response(200),
-        ])
+        transport, _ = self._make_transport(
+            [
+                _response(429, headers={"retry-after": "10"}),
+                _response(200),
+            ]
+        )
         transport.handle_request(_request())
         assert sleeps[0] >= 10.0
 
@@ -199,7 +207,7 @@ async def test_retries_exhausted_raises(self):
         assert fake._call_count == 3
 
     async def test_timeout_retried(self):
-        transport, fake = self._make_transport([httpx.ReadTimeout("timed out"), _response(200)])
+        transport, _fake = self._make_transport([httpx.ReadTimeout("timed out"), _response(200)])
         resp = await transport.handle_async_request(_request())
         assert resp.status_code == 200
 
