Remove the dead vcrpy/aiohttp compat shim (closes #2140)

[JSv4]

Closes #2140.

Finding: the shim is dead, and confirmed so

aiohttp 3.14 removed aiohttp.streams.AsyncStreamReaderMixin, which vcrpy 8.1.1's vcr/stubs/aiohttp_stubs.py subclassed at import time. vcrpy loads that stub lazily when a cassette is entered, so every VCR cassette entry raised AttributeError (issue #1920). The workaround was ensure_aiohttp_vcr_compat(), which re-injected the removed name as an empty class.

vcrpy 8.2.0 fixed the stub upstream (kevin1024/vcrpy#996). In 8.2.1 and 8.3.0, MockStream inherits only from asyncio.StreamReader and handles the aiohttp 3.14 stream_writer argument itself. The pin has been at 8.2.1 or later since then, so the issue's hypothesis is correct: the shim went dead before the 8.2.1 → 8.3.0 dependabot bump in #2132, not because of it.

One correction to the issue's framing: the shim was not merely inert. Under aiohttp 3.14 its hasattr guard fell through, so conftest import was still writing a fake AsyncStreamReaderMixin into the real aiohttp.streams namespace on every test run. Removing it also removes that namespace pollution.

Verification

Entered a cassette with no shim applied and checked MockStream.__bases__ in two environments:

Environment	vcrpy	aiohttp	Result
local django image	8.2.1	3.14.1	cassette entry OK, __bases__ == (asyncio.StreamReader,)
clean venv	8.3.0	3.14.1	cassette entry OK, __bases__ == (asyncio.StreamReader,)

The first row is the exact pre-bump pairing the issue asks about, which is what makes it decisive: the shim was already redundant at the old pin.

The three non-pytest maybe_vcr_cassette() call sites — opencontractserver/tasks/data_extract_tasks.py:637 (celery worker) and config/websocket/consumers/unified_agent_conversation.py:1163/:1431 (Daphne) — are exercised by frontend-e2e-extract.yml and frontend-e2e-websocket.yml in OC_LLM_VCR_MODE=replay. Both build via local.yml, and compose/local/django/Dockerfile:59 installs requirements/local.txt, so they are protected by the version pin rather than by the deleted call.

Changes

• Removed ensure_aiohttp_vcr_compat() from opencontractserver/utils/vcr_replay.py and its call in maybe_vcr_cassette()
• Removed the import and module-level call from conftest.py
• Replaced EnsureAiohttpVcrCompatTests with VcrCassetteEntryTests in opencontractserver/tests/test_vcr_replay.py
• Rewrote the requirements/local.txt comment, which still described vcrpy 8.1.1 as the pinned version and Add Redis integration tests and CI workflow #996 as unreleased

EnsureAiohttpVcrCompatTests tested the shim itself, so it goes with the shim. VcrCassetteEntryTests keeps the part of the guard that survives: entering a cassette forces the lazy import of vcr/stubs/aiohttp_stubs.py, so it fails if a future vcrpy/aiohttp pairing reintroduces an import-time incompatibility. The review agent confirmed this by reproducing vcrpy 8.1.1's class line against aiohttp 3.14.1 — both new tests fail with the original AttributeError.

The aiohttp>=3.13,<3.14 cap this was paired with (issue #1914) was already lifted on main; no requirements file pins aiohttp today.

Testing

72 passing locally across the targeted VCR-consuming modules: test_vcr_replay.py, test_structured_response_simple.py, test_structured_response_api.py, test_individual_extract_tasks.py. The remaining VCR modules are slow integration suites and are left to CI — the mechanism is proven at conftest-import level, so they would re-confirm the same thing. pre-commit clean (black, isort, flake8, mypy, changelog fragment validation).

Note for the release editor

changelog.d/1920-aiohttp-cap.changed.md:15 cites opencontractserver/tests/test_vcr_replay.py::EnsureAiohttpVcrCompatTests as its regression test — the class this PR renames. That fragment is another PR's record and the one-file-per-PR convention says not to edit it here, so it is left alone; worth repointing at VcrCassetteEntryTests during collation. The three aiohttp/vcrpy fragments (#1914 cap added, #1920 cap lifted + shim added, #2140 shim removed) all land in the same [Unreleased] batch and net to zero, so they may be worth folding into one line at that point.

[claude]

Review

Clean, well-substantiated deletion PR. The investigation in the description (verifying MockStream.__bases__ against both the pre-bump pairing vcrpy==8.2.1/aiohttp==3.14.1 and the current vcrpy==8.3.0/aiohttp==3.14.1) is exactly the kind of evidence a "this is now dead code" claim should carry, and I independently confirmed the parts that matter:

• No leftover references to ensure_aiohttp_vcr_compat or the shim class remain outside the historical docstring in test_vcr_replay.py:333 (which is describing past behavior, not calling it).
• conftest.py edit is clean — no stray blank line left behind.
• The three non-pytest maybe_vcr_cassette() call sites (opencontractserver/tasks/data_extract_tasks.py:637, config/websocket/consumers/unified_agent_conversation.py:1163/:1431) do run outside conftest, and compose/local/django/Dockerfile does install requirements/local.txt in the wheel-build step, so the version-pin protection claim checks out.
• VcrCassetteEntryTests imports still resolve (TestCase, _LLM_HOSTS, _match_llm_body, _normalize_body, maybe_vcr_cassette all still used — no orphaned imports after ensure_aiohttp_vcr_compat was dropped from the import list).

Code quality

• Good call keeping test_vcr_cassette_entry_works (behavioral guard: cassette entry must not raise) and adding test_aiohttp_stub_is_importable (pins the exact failure mode: the lazily-imported stub module must evaluate). Slightly overlapping in what they exercise, but each documents a different layer of the contract, which is worth the redundancy here given this is a "don't let a future dependency bump silently break CI again" guard.
• The rewritten requirements/local.txt comment is accurate and no longer describes a stale 8.1.1/unreleased-PR state.

Minor nit (non-blocking)

• requirements/local.txt: the comment says "Keep at >=8.2.0" but the pin itself is exact (vcrpy==8.3.0). Not a bug — just could be slightly more precise (e.g., "currently pinned to 8.3.0; do not drop below 8.2.0") so a future reader isn't confused about why an exact pin has a ">=" instruction next to it.

Process note

• Appreciated the explicit callout that changelog.d/1920-aiohttp-cap.changed.md references the renamed test class and is intentionally left untouched per the one-fragment-per-PR convention — correct call per this repo's changelog policy, and the flag for the release editor to repoint it during collation is exactly the right way to handle that.

No functional issues found. This is a safe, well-tested removal.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!