mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(tests): stabilize image-edit VCR cassettes to stop live gpt-image-1 spend
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=<hex>`` 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).
This commit is contained in:
parent
a72414a061
commit
4254c4ae78
4 changed files with 232 additions and 26 deletions
131
scripts/flush_image_edit_vcr_cassettes.py
Executable file
131
scripts/flush_image_edit_vcr_cassettes.py
Executable file
|
|
@ -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())
|
||||
|
|
@ -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=<random hex>``
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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=<random hex>`` 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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
]
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue