From 4254c4ae782f9893ab6ae40268138559545aa491 Mon Sep 17 00:00:00 2001 From: Mateo Wang <277851410+mateo-berri@users.noreply.github.com> Date: Sun, 17 May 2026 05:56:27 +0000 Subject: [PATCH] fix(tests): stabilize image-edit VCR cassettes to stop live gpt-image-1 spend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The image-edit cassettes for ``gpt-image-1`` were accumulating >50 episodes and being refused by the persister (``tests/_vcr_redis_persister.py``), so every CI run was hitting the real OpenAI endpoint. The async parametrize was the clearest tell: ``test_openai_image_edit_litellm_sdk[True]`` cached to 1 entry, but the ``[False]`` (async) sibling grew to 51 entries and never replayed. Two non-deterministic sources were fueling the growth, both fixed here. After this patch, the cassettes settle at one episode per unique call and replay for the 24-hour TTL like every other suite. 1. Pin httpx's multipart boundary at the source. The existing ``_normalize_multipart_boundary`` rewrites the boundary in the ``Content-Type`` header reliably, but on the async transport path the body is not always a contiguous ``bytes`` object when ``before_record_request`` runs, so the body-side replacement silently no-ops and the recorded cassette retains the random ``boundary=`` string. The next CI run gets a fresh random boundary, the ``safe_body`` matcher misses, and ``record_mode="new_episodes"`` appends another episode. Wrapping ``httpx._multipart.MultipartStream.__init__`` so it always uses ``vcr-static-boundary`` when no boundary is supplied eliminates the variance for both sync and async paths and leaves the normalizer in place as a backstop. Exposed as ``pin_httpx_multipart_boundary`` so other multipart-heavy suites (audio, ocr, batches) can adopt the same fixture later. 2. Pass raw ``bytes`` (not ``BytesIO`` streams) through the image-edit fixtures. A ``BytesIO`` whose file pointer is at EOF after the first multipart upload silently encodes an empty image on the next SDK / Router retry — yet another divergent body that VCR records as a new episode. ``bytes`` are immutable and position-less, so retries re-encode an identical payload every time. This is also a small production-correctness improvement: a customer passing ``BytesIO`` today would hit the same empty-body retry bug. The BytesIO-specific smoke test (``test_openai_image_edit_with_bytesio``) is preserved by giving ``get_test_images_as_bytesio`` its own factory instead of aliasing the bytes one. 3. Add ``scripts/flush_image_edit_vcr_cassettes.py`` — a one-shot Redis SCAN/DEL helper that clears the bloated pre-fix cassettes under ``litellm:vcr:cassette:tests/image_gen_tests/test_image_edits/*``. Without this, the next CI run still loads the existing 51-entry cassette, the new fixed-boundary body still doesn't match any of the stale entries, the persister still refuses to save, and the bleed continues. Run once with the production ``CASSETTE_REDIS_URL`` after merge (dry-run by default). --- scripts/flush_image_edit_vcr_cassettes.py | 131 ++++++++++++++++++++++ tests/_vcr_conftest_common.py | 49 ++++++++ tests/image_gen_tests/conftest.py | 18 +++ tests/image_gen_tests/test_image_edits.py | 60 +++++----- 4 files changed, 232 insertions(+), 26 deletions(-) create mode 100755 scripts/flush_image_edit_vcr_cassettes.py diff --git a/scripts/flush_image_edit_vcr_cassettes.py b/scripts/flush_image_edit_vcr_cassettes.py new file mode 100755 index 00000000000..7b87f36b284 --- /dev/null +++ b/scripts/flush_image_edit_vcr_cassettes.py @@ -0,0 +1,131 @@ +#!/usr/bin/env python3 +"""Flush the bloated image-edit VCR cassettes from the cassette Redis. + +Run this **once** after merging the multipart-boundary stabilization +PR. The pre-fix cassettes for the async image-edit tests have +accumulated >50 episodes (random multipart boundary on every run + +``record_mode="new_episodes"`` = monotonic growth), so the persister +refuses to save updates -- meaning every CI run after the fix would +still try to re-record against the stale 51-entry cassette, hit +``MAX_EPISODES_PER_CASSETTE`` again, get refused, and re-bill the live +provider. + +Deleting these keys forces the next CI run to record a clean cassette +under the new fixed-boundary + raw-bytes fixtures (1 episode per +unique call), after which the 24-hour TTL replay loop kicks in +normally. + +Scope is intentionally narrow: + * Only ``tests/image_gen_tests/test_image_edits/*`` cassette keys + are touched. Image-*generation* cassettes (TestOpenAIGPTImage1 + etc.) are unaffected -- they were already in the VCR HIT state. + * Lists every match in dry-run mode before deleting anything so the + operator can confirm the impact. + +Usage: + CASSETTE_REDIS_URL=redis://... \ + uv run python scripts/flush_image_edit_vcr_cassettes.py --dry-run + + CASSETTE_REDIS_URL=redis://... \ + uv run python scripts/flush_image_edit_vcr_cassettes.py --yes + +``CASSETTE_REDIS_URL`` is the same env var the persister reads at CI +start (see ``tests/_vcr_redis_persister.py``). +""" + +from __future__ import annotations + +import argparse +import os +import sys + +import redis + + +CASSETTE_REDIS_URL_ENV = "CASSETTE_REDIS_URL" +REDIS_KEY_PREFIX = "litellm:vcr:cassette:" +TARGET_KEY_PATTERN = f"{REDIS_KEY_PREFIX}tests/image_gen_tests/test_image_edits/*" + + +def _build_client(url: str) -> redis.Redis: + return redis.Redis.from_url( + url, + socket_timeout=10, + socket_connect_timeout=10, + decode_responses=False, + ) + + +def _scan_matching_keys(client: redis.Redis, pattern: str) -> list[bytes]: + return sorted(client.scan_iter(match=pattern, count=500)) + + +def _delete_keys(client: redis.Redis, keys: list[bytes]) -> int: + if not keys: + return 0 + # Batch into chunks so a single DEL call does not exceed the + # server's argument-count or buffer limits on large key sets. + deleted = 0 + chunk_size = 200 + for start in range(0, len(keys), chunk_size): + batch = keys[start : start + chunk_size] + deleted += int(client.delete(*batch)) + return deleted + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--yes", + action="store_true", + help="Actually delete the matched keys. Without this flag the script " + "runs in dry-run mode and only lists what would be deleted.", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="List matched keys without deleting (default behaviour when " + "--yes is omitted; kept as an explicit flag for clarity).", + ) + parser.add_argument( + "--pattern", + default=TARGET_KEY_PATTERN, + help=f"Override the SCAN match pattern. Default: {TARGET_KEY_PATTERN}", + ) + args = parser.parse_args(argv) + + url = os.environ.get(CASSETTE_REDIS_URL_ENV) + if not url: + print( + f"error: {CASSETTE_REDIS_URL_ENV} is not set. Set it to the " + "cassette Redis URL (same URL the persister reads in CI).", + file=sys.stderr, + ) + return 2 + + client = _build_client(url) + try: + client.ping() + except redis.RedisError as exc: + print(f"error: cannot reach cassette Redis: {exc}", file=sys.stderr) + return 2 + + matches = _scan_matching_keys(client, args.pattern) + print(f"matched {len(matches)} key(s) under pattern: {args.pattern}") + for key in matches: + print(f" {key.decode('utf-8', errors='replace')}") + + if not matches: + return 0 + + if not args.yes: + print("\ndry run -- pass --yes to actually delete these keys.") + return 0 + + deleted = _delete_keys(client, matches) + print(f"\ndeleted {deleted} key(s).") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/_vcr_conftest_common.py b/tests/_vcr_conftest_common.py index a179a21ba69..9b706e3e5cb 100644 --- a/tests/_vcr_conftest_common.py +++ b/tests/_vcr_conftest_common.py @@ -91,6 +91,55 @@ VCR_IMAGE_B64_PLACEHOLDER = "dGVzdA==" VCR_FIXED_MULTIPART_BOUNDARY = "vcr-static-boundary" +def pin_httpx_multipart_boundary(monkeypatch) -> None: + """Force every httpx multipart request to use a constant boundary. + + httpx's ``MultipartStream`` generates a fresh ``boundary=`` + via ``os.urandom(16)`` whenever the caller does not supply one + (see ``httpx._multipart.MultipartStream.__init__``). That random + boundary appears both in the ``Content-Type`` header and verbatim in + the request body between each part. + + ``_normalize_multipart_boundary`` rewrites the header reliably, but + on the async transport path the body is not always handed to + ``before_record_request`` as a contiguous ``bytes`` object — so the + body replacement silently no-ops and the recorded cassette retains + the random boundary string. Subsequent runs generate a *different* + random boundary, the ``safe_body`` matcher misses, and + ``record_mode="new_episodes"`` appends a fresh episode until the + cassette crosses ``MAX_EPISODES_PER_CASSETTE`` and the persister + refuses to save — re-billing live providers on every CI run. + + Pinning the boundary at the source removes the variance entirely: + every run emits byte-identical multipart bodies, the existing + ``safe_body`` matcher succeeds on the first request, and one + recorded episode per unique call satisfies replays for the cassette + TTL. + + This wraps ``MultipartStream.__init__`` instead of patching the + boundary-generation helper directly because httpx inlines + ``os.urandom(16).hex().encode("ascii")`` in the constructor body + rather than calling a named function. We preserve the caller's + boundary when one is explicitly supplied so production-style code + that pins its own boundary keeps working. + """ + try: + import httpx._multipart as _httpx_multipart + except ImportError: # pragma: no cover - httpx is a hard test dep + return + + _original_init = _httpx_multipart.MultipartStream.__init__ + + def _init_with_fixed_boundary(self, data, files, boundary=None): + if boundary is None: + boundary = VCR_FIXED_MULTIPART_BOUNDARY.encode("ascii") + return _original_init(self, data=data, files=files, boundary=boundary) + + monkeypatch.setattr( + _httpx_multipart.MultipartStream, "__init__", _init_with_fixed_boundary + ) + + def _scrub_response(response): if not isinstance(response, dict): return response diff --git a/tests/image_gen_tests/conftest.py b/tests/image_gen_tests/conftest.py index 93dec98e708..23e34c86dd4 100644 --- a/tests/image_gen_tests/conftest.py +++ b/tests/image_gen_tests/conftest.py @@ -15,6 +15,7 @@ from tests._vcr_conftest_common import ( # noqa: E402 emit_cassette_cache_session_banner, emit_vcr_classification_summary, install_live_call_probe, + pin_httpx_multipart_boundary, record_vcr_outcome, register_persister_if_enabled, vcr_config_dict, @@ -33,6 +34,23 @@ def event_loop(): loop.close() +@pytest.fixture(scope="session", autouse=True) +def _pin_multipart_boundary(): + """Pin httpx's random multipart boundary to a constant for the + entire image-gen test session. Without this, async multipart bodies + contain a fresh ``boundary=`` on every run; the + ``safe_body`` matcher misses, and ``record_mode="new_episodes"`` + grows each cassette by one entry per run until it crosses the + 50-episode persister cap and stops being saved — leaving the test + to hit the real provider on every CI run. See + ``pin_httpx_multipart_boundary`` for the full rationale. + """ + monkeypatch = pytest.MonkeyPatch() + pin_httpx_multipart_boundary(monkeypatch) + yield + monkeypatch.undo() + + @pytest.fixture(scope="module") def vcr_config(): return vcr_config_dict() diff --git a/tests/image_gen_tests/test_image_edits.py b/tests/image_gen_tests/test_image_edits.py index 656b8a69117..7c3632b82fc 100644 --- a/tests/image_gen_tests/test_image_edits.py +++ b/tests/image_gen_tests/test_image_edits.py @@ -103,12 +103,16 @@ class BaseLLMImageEditTest(ABC): pwd = os.path.dirname(os.path.realpath(__file__)) -# Image fixtures must be regenerated per access — module-level -# ``open(...)`` handles get consumed after a single multipart upload, leaving -# subsequent tests in the same process to send empty bodies. That non-determinism -# (a) blows the recorded cassette past ``MAX_EPISODES_PER_CASSETTE`` so the -# persister refuses to save (see ``tests/_vcr_redis_persister.py``), and -# (b) re-bills the live image edit endpoint on every CI run. +# Image fixtures are returned as raw ``bytes`` (not file handles or +# ``BytesIO`` streams) so that every SDK / Router retry sees the same +# payload. A ``BytesIO`` whose file pointer is left at EOF by the first +# multipart upload silently encodes an empty image on the second +# attempt, producing a different request body — VCR records that +# divergent body as a fresh episode, the cassette eventually crosses +# ``MAX_EPISODES_PER_CASSETTE`` in ``tests/_vcr_redis_persister.py``, +# the persister refuses to save, and every subsequent CI run re-bills +# the live image-edit endpoint. Raw bytes are immutable, position-less, +# and re-encoded identically on every retry attempt. def _read_image_bytes(filename: str) -> bytes: with open(os.path.join(pwd, filename), "rb") as f: return f.read() @@ -119,16 +123,29 @@ _LITELLM_SITE_BYTES = _read_image_bytes("litellm_site.png") def _make_test_images() -> list: - """Return a fresh pair of image streams seeded with the fixture bytes. + """Return the pair of fixture images as raw ``bytes`` payloads. - Use this everywhere you'd previously have used the module-level - ``TEST_IMAGES``. Each call returns brand new ``BytesIO`` objects whose - file pointers start at 0, so multipart uploads encode the full image - bytes on every test invocation. Parametrized and ``flaky``-retried - test methods call ``get_base_image_edit_call_args`` once per - invocation, so a fresh stream per call is sufficient — the factory - must not auto-rewind on EOF or the SDK's multipart writer will read - the same bytes forever (worker OOM). + ``httpx`` accepts a ``bytes`` value anywhere a file-like upload is + expected and re-encodes it identically on every multipart attempt + — so SDK-level retries can never produce a divergent empty-body + episode (the root cause of the cassette-overflow leak that bills + ``gpt-image-1`` on every CI run). + """ + return [_ISHAAN_GITHUB_BYTES, _LITELLM_SITE_BYTES] + + +def _make_single_test_image() -> bytes: + return _ISHAAN_GITHUB_BYTES + + +def get_test_images_as_bytesio(): + """Return the fixture images as fresh ``BytesIO`` streams. + + Kept distinct from ``_make_test_images`` so the BytesIO-specific + smoke tests (``test_openai_image_edit_with_bytesio``, + ``test_multiple_image_edit_with_different_formats``) still exercise + the file-like upload path. Each call yields brand new streams so + the file pointer always starts at 0 for that test invocation. """ return [ BytesIO(_ISHAAN_GITHUB_BYTES), @@ -136,15 +153,6 @@ def _make_test_images() -> list: ] -def _make_single_test_image() -> BytesIO: - return BytesIO(_ISHAAN_GITHUB_BYTES) - - -def get_test_images_as_bytesio(): - """Helper function to get test images as BytesIO objects""" - return _make_test_images() - - class TestOpenAIImageEditGPTImage1(BaseLLMImageEditTest): """ Concrete implementation of BaseLLMImageEditTest for OpenAI image edits. @@ -710,9 +718,9 @@ async def test_multiple_image_edit_with_different_formats(): try: prompt = "Create a cohesive artistic style across all images" - # Test with mixed BytesIO and file objects + # Test with mixed raw-bytes and BytesIO inputs mixed_images = [ - _make_single_test_image(), # File object + _make_single_test_image(), # raw ``bytes`` payload get_test_images_as_bytesio()[1], # BytesIO object ]