From 33a57cc9e8b42bf6ccb42773ca8f690718ea81e8 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Wed, 20 May 2026 13:52:54 -0700 Subject: [PATCH] fix(assets): address bot review comments - Soften offset param prose: it's not deprecated, just not preferred for sequential walks. Random-access UIs (jump-to-page, item count displays) legitimately still want offset, so dropping the 'deprecated' framing rather than promoting it to a machine-readable deprecated:true flag. - Add explicit HTTP status assertions before every json() / next_cursor read in test_list_cursor.py so a failing request surfaces as an HTTP error instead of a confusing KeyError on a 4xx/5xx body. --- openapi.yaml | 7 +++++-- tests-unit/assets_test/test_list_cursor.py | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index 464751c4f..4d806e8f5 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1518,8 +1518,11 @@ paths: type: integer default: 0 description: | - Deprecated in favor of `after` cursor pagination. When both are - supplied, `after` wins and `offset` is ignored. + Offset-based pagination. Cursor pagination via `after` is preferred + for sequential walks (stable across concurrent inserts/deletes) but + `offset` remains fully supported for random access (jump-to-page + UIs, "showing items X–Y of N" displays). When both are supplied, + `after` wins and `offset` is ignored. - name: after in: query schema: diff --git a/tests-unit/assets_test/test_list_cursor.py b/tests-unit/assets_test/test_list_cursor.py index 9378edad6..44ab21037 100644 --- a/tests-unit/assets_test/test_list_cursor.py +++ b/tests-unit/assets_test/test_list_cursor.py @@ -109,6 +109,7 @@ def test_cursor_wins_over_offset(http: requests.Session, api_base: str, asset_fa }, timeout=120, ) + assert r.status_code == 200, r.text cursor = r.json()["next_cursor"] assert cursor is not None @@ -144,6 +145,7 @@ def test_next_cursor_absent_when_no_more_results(http: requests.Session, api_bas }, timeout=120, ) + assert r.status_code == 200, r.text body = r.json() assert body["has_more"] is False assert "next_cursor" not in body @@ -159,6 +161,7 @@ def test_cursor_pagination_first_page_mints_cursor(http: requests.Session, api_b params={"include_tags": "unit-tests,cursor-first-page", "sort": "name", "order": "asc", "limit": "2"}, timeout=120, ) + assert r.status_code == 200, r.text body = r.json() assert body["has_more"] is True assert body.get("next_cursor"), "first page must mint a cursor when more rows exist" @@ -175,6 +178,7 @@ def test_cursor_no_spurious_cursor_when_page_size_equals_remainder(http: request params={"include_tags": "unit-tests,cursor-exact-multiple", "sort": "name", "order": "asc", "limit": "2"}, timeout=120, ) + assert r.status_code == 200, r.text cursor = r.json()["next_cursor"] assert cursor is not None # Page 2 — should exhaust the set with no cursor for a phantom page 3 @@ -183,6 +187,7 @@ def test_cursor_no_spurious_cursor_when_page_size_equals_remainder(http: request params={"include_tags": "unit-tests,cursor-exact-multiple", "sort": "name", "order": "asc", "limit": "2", "after": cursor}, timeout=120, ) + assert r2.status_code == 200, r2.text body = r2.json() assert len(body["assets"]) == 2 assert body["has_more"] is False @@ -245,6 +250,7 @@ def test_cursor_order_mismatch_returns_400(http: requests.Session, api_base: str }, timeout=120, ) + assert r.status_code == 200, r.text cursor = r.json()["next_cursor"] assert cursor is not None @@ -321,5 +327,6 @@ def test_cursor_pagination_stable_after_delete(http: requests.Session, api_base: }, timeout=120, ) + assert r2.status_code == 200, r2.text body2 = r2.json() assert [a["name"] for a in body2["assets"]] == names[2:]