From 3a30f1cde3ae77e936a789ab29256f3d54a196b5 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 17:11:50 +0300 Subject: [PATCH 1/4] docs(planning): add decoders shared memoizing-cache design --- ...7-13.05-decoders-shared-memoizing-cache.md | 249 ++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md diff --git a/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md b/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md new file mode 100644 index 0000000..6a4b2c8 --- /dev/null +++ b/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md @@ -0,0 +1,249 @@ +--- +summary: Share the decoders' memoizing get-or-build cache pattern (4 duplicated sites across pydantic.py/msgspec.py) via one generic _get_or_build function, and drop decode()'s now-dead TypeError fallback. +--- + +# Design: Share the decoders' memoizing cache pattern + +## Summary + +`src/httpware/decoders/pydantic.py` and `decoders/msgspec.py` each +implement the identical "check a `dict[type, V]` cache, build-and-memoize +on miss, unhashable models bypass the cache entirely" pattern at two sites +per file: the object cache (`_get_adapter`/`_get_msgspec_decoder`) and the +verdict cache (`can_decode`). This change extracts one generic +`_get_or_build` function into a new `decoders/_caching.py`, used at all +four sites, and simplifies `decode()` in both classes by dropping a +try/except that becomes dead code once the cache handles the unhashable +case internally. Two existing tests that mocked the wrong layer are +corrected to test the real mechanism. No observable behavior change. + +## Motivation + +- **Four duplicated cache sites, not two.** `PydanticDecoder._get_adapter` + (`:45-50`) and `MsgspecDecoder._get_msgspec_decoder` (`:67-72`) are + structurally identical (differing only in what gets built and which + dict holds it). `PydanticDecoder.can_decode` (`:52-67`) and + `MsgspecDecoder.can_decode` (`:74-89`) are byte-identical (modulo + docstring wording) — same `try: cache.get(model) / except TypeError: + probe fresh`, same `if cached is not None: return cached`, same + probe-store-return. +- **The sentinel trick is shared too.** Both cache flavors — `dict[type, + bool]` (verdicts) and `dict[type, TypeAdapter | Decoder]` (built + objects) — rely on the same fact: a real stored value (`True`/`False`, + or a built object) is never `None`, so `dict.get(model)` returning + `None` unambiguously means "not cached." One generic function serves + both flavors. +- **Corroborating signal:** `tests/test_decoders_pydantic.py` and + `tests/test_decoders_msgspec.py` (241 lines each) independently + re-verify the same caching contract via `# noqa: SLF001` pokes at + `_adapters`/`_can_decode_results` and `_msgspec_decoders`/ + `_can_decode_results` — the memoization concern isn't exercisable + through the public `ResponseDecoder` protocol alone, so tests reach + through the seam rather than past it. +- **Deletion test:** delete `_get_or_build` and the identical + get-or-build logic reappears at all four call sites — real, load-bearing + logic, not a pass-through. +- **A second, independent finding surfaced while re-reading both files + closely:** `decode()` in both classes has its own try/except `TypeError` + fallback (falls back to a fresh, uncached build) that exists solely to + handle the same unhashable-model case `_get_or_build` will now handle + internally. Traced through Seam B's contract + (`decoders/_resolver.py`'s `_DecoderResolver.resolve`): `decode()` only + ever runs on a model that already passed `can_decode`'s probe in the + same request, so `TypeAdapter(model)`/`msgspec.json.Decoder(model)` + construction cannot newly fail at `decode()` time — the only thing the + existing fallback ever guarded against was the cache-lookup + `TypeError`. Once that's handled inside `_get_or_build`, the fallback in + `decode()` becomes genuinely unreachable and is removed. + +## Non-goals + +- No behavior change from the caller's perspective: `can_decode` and + `decode` return the same values for the same inputs, including for + unhashable models (fresh, uncached build every call, same as today). +- Not extracting the tiny `if not is_X_installed: raise + ImportError(MESSAGE)` check duplicated in both `__init__`s (2 lines, + different flag and message per call — a shared helper would need both + passed as parameters, more ceremony than it replaces). Same judgment + already applied to `_internal/status.py`'s 3-line predicate functions + in the original architecture review. +- Not touching `_probe_can_decode` (genuinely different per decoder — the + actual varying logic this refactor is careful to leave alone) or + `_contains_custom_type` (msgspec-specific, unrelated concern). +- Not touching the `ResponseDecoder` protocol, `_DecoderResolver`, or + `_BoundDecoder` (Seam B's orchestration, already deep, unaffected). + +## Design + +### 1. `decoders/_caching.py` — new module + +```python +"""Shared memoizing get-or-build cache for the decoder adapters (Seam C). + +Imports neither pydantic nor msgspec — safe for both decoder modules to +import without crossing Seam C's per-extra import isolation. +""" + +import typing + + +V = typing.TypeVar("V") + + +def _get_or_build(cache: dict[type, V], model: type, build: typing.Callable[[], V]) -> V: + """Return cache[model], building and memoizing it via build() on miss. + + Unhashable models bypass the cache entirely: dict.get(model) raises + TypeError, in which case this returns a fresh, uncached build() every + call rather than raising. A real cached value (bool verdict or a + built adapter/decoder) is never None, so a None result unambiguously + means "not cached yet" — callers must not store None as a value. + """ + try: + cached = cache.get(model) + except TypeError: + return build() + if cached is not None: + return cached + result = build() + cache[model] = result + return result +``` + +Placed alongside `decoders/_resolver.py` — both are private, cross-file +modules inside `decoders/` shared by the seam's adapters, not moved to +`_internal/` (which is for logic shared across otherwise-unrelated parts +of the whole package, not decoders-specific plumbing). + +### 2. `PydanticDecoder` — four call sites become two, `decode()` simplifies + +```python +from httpware.decoders._caching import _get_or_build + +# ... + +def _get_adapter(self, model: type[T]) -> "TypeAdapter[T]": + return _get_or_build(self._adapters, model, lambda: TypeAdapter(model)) + +def can_decode(self, model: type) -> bool: + """Return True iff pydantic can build a schema for `model`. + + The verdict is memoized per `model` so a rejection (which costs a + `PydanticSchemaGenerationError` round-trip) is not re-probed on every + dispatch. Unhashable models skip the cache and probe fresh. + """ + return _get_or_build(self._can_decode_results, model, lambda: self._probe_can_decode(model)) + +def decode(self, content: bytes, model: type[T]) -> T: + """Validate `content` as JSON against `model` in a single parse pass.""" + adapter = self._get_adapter(model) + return adapter.validate_json(content) +``` + +`_probe_can_decode` is untouched. + +### 3. `MsgspecDecoder` — the same shape + +```python +from httpware.decoders._caching import _get_or_build + +# ... + +def _get_msgspec_decoder(self, model: type[T]) -> "msgspec.json.Decoder[T]": + return _get_or_build(self._msgspec_decoders, model, lambda: msgspec.json.Decoder(model)) + +def can_decode(self, model: type) -> bool: + """Return True iff msgspec natively understands `model` end-to-end. + + The verdict is memoized per `model`: the probe below (an uncached + `type_info` call plus a recursive tree walk) runs once per type, not + on every dispatch. Unhashable models skip the cache and probe fresh. + """ + return _get_or_build(self._can_decode_results, model, lambda: self._probe_can_decode(model)) + +def decode(self, content: bytes, model: type[T]) -> T: + """Validate `content` as JSON against `model` in a single parse pass.""" + decoder = self._get_msgspec_decoder(model) + return decoder.decode(content) +``` + +`_probe_can_decode` and `_contains_custom_type` are untouched. + +### 4. Test corrections (necessary, not optional) + +Two existing tests mock the wrong layer and must change, or they break +once `decode()`'s fallback is removed: + +- `tests/test_decoders_pydantic.py::test_unhashable_model_falls_back_to_uncached_adapter` + (currently `:134-150`) mocks `PydanticDecoder._get_adapter` itself with + `side_effect=TypeError(...)` to exercise `decode()`'s fallback. Once + `_get_adapter` delegates to `_get_or_build`, it can no longer raise + `TypeError` for this reason, so the mock describes a scenario the real + method can't produce. Rewrite to mock the cache dict instead + (`decoder._adapters = MagicMock(); decoder._adapters.get.side_effect = + TypeError(...)`), mirroring the pattern the existing + `test_pydantic_can_decode_unhashable_model_does_not_raise` (`:236-241`) + already uses for the verdict cache. Same assertions + (`decoder.decode(b"42", int) == 42`, then a genuinely malformed payload + still raises `pydantic.ValidationError`). +- `tests/test_decoders_msgspec.py::test_unhashable_model_falls_back_to_uncached_decoder` + (currently `:140-154`) — identical fix, mocking `decoder._msgspec_decoders` + instead of `MsgspecDecoder._get_msgspec_decoder`. + +Once fixed this way, both tests directly exercise `_get_or_build`'s +`TypeError`-bypass path for the object-cache flavor, complementing the +existing `can_decode`-unhashable tests (verdict-cache flavor) — decent +direct coverage of the new shared function without a dedicated test +file. + +**Everything else in both test files is unaffected**, verified by reading +every test in both files: +- `test_can_decode_returns_false_when_decoder_build_raises` (msgspec + `:130-137`) mocks `_get_msgspec_decoder` to test `_probe_can_decode`'s + own broad `except Exception: return False` — untouched, since + `_probe_can_decode` still calls `self._get_msgspec_decoder(model)` as a + method regardless of its internal implementation. +- The `patch.object(decoder, "_get_adapter", wraps=decoder._get_adapter)` + spy tests (pydantic `:219`, `:229`) wrap the real method rather than + replacing it — call-count assertions hold unchanged since the number of + calls to `_get_adapter`/`_probe_can_decode` doesn't change. +- `test_cache_invariance_*` (pydantic `:87-131`) patch the module-level + `TypeAdapter` name directly — independent of internal call structure. +- `test_pydantic_can_decode_uses_cache`/`test_msgspec_can_decode_uses_cache` + check dict contents (`len(...) == 1`, membership) — unaffected, the + dicts are populated the same way. + +### 5. Minor comment accuracy fix + +`test_cache_invariance_concurrent_first_calls_threadpool`'s comment +(pydantic `:127-130`) says "the get→set sequence in `_get_adapter` is +not [atomic]" — the get→set sequence now lives in `_get_or_build`. Update +the comment to name the new location. + +## Testing + +- **Parity net:** every other existing test in both files stays green + unchanged (see the itemized check above). +- **Corrected tests:** the two rewritten unhashable-fallback tests, as + specified in Design §4. +- No new dedicated test file for `_get_or_build` — the corrected tests + plus the existing `can_decode`-unhashable tests already exercise both + value-type flavors (bool, built object) of its `TypeError`-bypass path, + and every cache-hit/cache-miss branch is already exercised by the + existing cache-content and call-count assertions. +- `just lint && just test` both clean; 100% coverage maintained. + +## Risk + +- **Silently breaking the two rewritten tests' original intent** + (unlikely × medium): the fix must preserve the same observable + contract (unhashable model still decodes correctly via a fresh, + uncached build) even though the mock target moves. *Mitigation:* keep + the same assertions, only change what's mocked and how. +- **Generic typing friction** (`_get_or_build`'s `V` vs. `T` narrowing at + each call site) (unlikely × low): `self._adapters`/`self._msgspec_decoders` + are already typed `dict[type, X[typing.Any]]` today, so the existing + code already widens through `Any` in `_get_adapter`'s own return + statement before this change — no new widening is introduced. + *Mitigation:* `ty check` in the verification gate catches any real + regression. From c44c80b43e3a7d6749c933c87e5608676547a829 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 17:16:15 +0300 Subject: [PATCH 2/4] refactor(decoders): share memoizing get-or-build cache across adapters Extracts the duplicated memoizing-cache pattern (4 sites: _get_adapter, _get_msgspec_decoder, and can_decode in both PydanticDecoder and MsgspecDecoder) into a shared _get_or_build helper in the new decoders/_caching.py. As a consequence, decode()'s try/except TypeError fallback in both classes becomes dead code -- the cache now handles the unhashable-model case internally -- so it's dropped from both. Corrects two tests that mocked the wrong layer (the whole _get_adapter/_get_msgspec_decoder method) to instead mock the cache dict, matching how the sibling can_decode-unhashable tests already do it. Design: planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md --- src/httpware/decoders/_caching.py | 30 ++++++++++++++++++++++++++++++ src/httpware/decoders/msgspec.py | 22 ++++------------------ src/httpware/decoders/pydantic.py | 22 ++++------------------ tests/test_decoders_msgspec.py | 21 ++++++++++----------- tests/test_decoders_pydantic.py | 27 +++++++++++++-------------- 5 files changed, 61 insertions(+), 61 deletions(-) create mode 100644 src/httpware/decoders/_caching.py diff --git a/src/httpware/decoders/_caching.py b/src/httpware/decoders/_caching.py new file mode 100644 index 0000000..c3989f3 --- /dev/null +++ b/src/httpware/decoders/_caching.py @@ -0,0 +1,30 @@ +"""Shared memoizing get-or-build cache for the decoder adapters (Seam C). + +Imports neither pydantic nor msgspec — safe for both decoder modules to +import without crossing Seam C's per-extra import isolation. +""" + +import typing + + +V = typing.TypeVar("V") + + +def _get_or_build(cache: dict[type, V], model: type, build: typing.Callable[[], V]) -> V: + """Return cache[model], building and memoizing it via build() on miss. + + Unhashable models bypass the cache entirely: dict.get(model) raises + TypeError, in which case this returns a fresh, uncached build() every + call rather than raising. A real cached value (bool verdict or a + built adapter/decoder) is never None, so a None result unambiguously + means "not cached yet" — callers must not store None as a value. + """ + try: + cached = cache.get(model) + except TypeError: + return build() + if cached is not None: + return cached + result = build() + cache[model] = result + return result diff --git a/src/httpware/decoders/msgspec.py b/src/httpware/decoders/msgspec.py index 5043309..5b51ce3 100644 --- a/src/httpware/decoders/msgspec.py +++ b/src/httpware/decoders/msgspec.py @@ -4,6 +4,7 @@ from typing import TypeVar from httpware._internal import import_checker +from httpware.decoders._caching import _get_or_build if import_checker.is_msgspec_installed: @@ -65,11 +66,7 @@ def __init__(self) -> None: self._can_decode_results = {} def _get_msgspec_decoder(self, model: type[T]) -> "msgspec.json.Decoder[T]": - decoder = self._msgspec_decoders.get(model) - if decoder is None: - decoder = msgspec.json.Decoder(model) - self._msgspec_decoders[model] = decoder - return decoder + return _get_or_build(self._msgspec_decoders, model, lambda: msgspec.json.Decoder(model)) def can_decode(self, model: type) -> bool: """Return True iff msgspec natively understands `model` end-to-end. @@ -78,15 +75,7 @@ def can_decode(self, model: type) -> bool: `type_info` call plus a recursive tree walk) runs once per type, not on every dispatch. Unhashable models skip the cache and probe fresh. """ - try: - cached = self._can_decode_results.get(model) - except TypeError: # unhashable model — can't memoize, probe fresh - return self._probe_can_decode(model) - if cached is not None: - return cached - result = self._probe_can_decode(model) - self._can_decode_results[model] = result - return result + return _get_or_build(self._can_decode_results, model, lambda: self._probe_can_decode(model)) def _probe_can_decode(self, model: type) -> bool: """Decide whether msgspec natively decodes `model` (the uncached path). @@ -111,8 +100,5 @@ def _probe_can_decode(self, model: type) -> bool: def decode(self, content: bytes, model: type[T]) -> T: """Validate `content` as JSON against `model` in a single parse pass.""" - try: - decoder = self._get_msgspec_decoder(model) - except TypeError: - decoder = msgspec.json.Decoder(model) + decoder = self._get_msgspec_decoder(model) return decoder.decode(content) diff --git a/src/httpware/decoders/pydantic.py b/src/httpware/decoders/pydantic.py index 1dc0c99..3eb02e7 100644 --- a/src/httpware/decoders/pydantic.py +++ b/src/httpware/decoders/pydantic.py @@ -11,6 +11,7 @@ class entirely when `is_pydantic_installed` is False, so `AsyncClient()` does from typing import TypeVar from httpware._internal import import_checker +from httpware.decoders._caching import _get_or_build if import_checker.is_pydantic_installed: @@ -43,11 +44,7 @@ def __init__(self) -> None: self._can_decode_results = {} def _get_adapter(self, model: type[T]) -> "TypeAdapter[T]": - adapter = self._adapters.get(model) - if adapter is None: - adapter = TypeAdapter(model) - self._adapters[model] = adapter - return adapter + return _get_or_build(self._adapters, model, lambda: TypeAdapter(model)) def can_decode(self, model: type) -> bool: """Return True iff pydantic can build a schema for `model`. @@ -56,15 +53,7 @@ def can_decode(self, model: type) -> bool: `PydanticSchemaGenerationError` round-trip) is not re-probed on every dispatch. Unhashable models skip the cache and probe fresh. """ - try: - cached = self._can_decode_results.get(model) - except TypeError: # unhashable model — can't memoize, probe fresh - return self._probe_can_decode(model) - if cached is not None: - return cached - result = self._probe_can_decode(model) - self._can_decode_results[model] = result - return result + return _get_or_build(self._can_decode_results, model, lambda: self._probe_can_decode(model)) def _probe_can_decode(self, model: type) -> bool: """Decide whether pydantic can build a schema for `model` (uncached). @@ -82,8 +71,5 @@ def _probe_can_decode(self, model: type) -> bool: def decode(self, content: bytes, model: type[T]) -> T: """Validate `content` as JSON against `model` in a single parse pass.""" - try: - adapter = self._get_adapter(model) - except TypeError: - adapter = TypeAdapter(model) + adapter = self._get_adapter(model) return adapter.validate_json(content) diff --git a/tests/test_decoders_msgspec.py b/tests/test_decoders_msgspec.py index 899cf26..480c7e1 100644 --- a/tests/test_decoders_msgspec.py +++ b/tests/test_decoders_msgspec.py @@ -140,18 +140,17 @@ def test_can_decode_returns_false_when_decoder_build_raises() -> None: def test_unhashable_model_falls_back_to_uncached_decoder() -> None: """Unhashable `model` falls back to a direct uncached `msgspec.json.Decoder`. - Mirrors `PydanticDecoder`'s unhashable-fallback test: when `_get_msgspec_decoder` - raises `TypeError` (e.g., an unhashable parameterized type), `decode` bypasses - the cache so the user-visible error is `msgspec`'s own decode error, not a - `TypeError` from the cache lookup. + Mirrors `PydanticDecoder`'s unhashable-fallback test: when the decoder + cache lookup raises `TypeError` (e.g., an unhashable parameterized + type), `decode` bypasses the cache so the user-visible error is + `msgspec`'s own decode error, not a `TypeError` from the cache lookup. """ - with patch.object( - MsgspecDecoder, - "_get_msgspec_decoder", - side_effect=TypeError("unhashable type"), - ): - result = MsgspecDecoder().decode(b"42", int) - assert result == 42 # noqa: PLR2004 + decoder = MsgspecDecoder() + decoder._msgspec_decoders = MagicMock() # noqa: SLF001 + decoder._msgspec_decoders.get.side_effect = TypeError("unhashable type") # noqa: SLF001 + + result = decoder.decode(b"42", int) + assert result == 42 # noqa: PLR2004 @pytest.mark.parametrize( diff --git a/tests/test_decoders_pydantic.py b/tests/test_decoders_pydantic.py index 60b6bf5..d33fc1d 100644 --- a/tests/test_decoders_pydantic.py +++ b/tests/test_decoders_pydantic.py @@ -125,7 +125,7 @@ def one_decode(_: int) -> User: assert all(type(r) is User and r.id == 1 for r in results) # `dict` reads/writes are atomic in CPython but the get→set sequence in - # `_get_adapter` is not — concurrent first-callers may both build a + # `_get_or_build` is not — concurrent first-callers may both build a # TypeAdapter before one wins (idempotent; loser is GC'd). Bounded by # worker count. assert 1 <= spy.call_count <= n_workers @@ -134,20 +134,19 @@ def one_decode(_: int) -> User: def test_unhashable_model_falls_back_to_uncached_adapter() -> None: """Unhashable `model` falls back to a direct uncached `TypeAdapter`. - When `_get_adapter` raises `TypeError` (e.g., `Annotated[int, unhashable_metadata]`), - `decode` bypasses the cache so `pydantic.ValidationError` surfaces cleanly instead - of leaking a `TypeError` to the caller. + When the adapter cache lookup raises `TypeError` (e.g., an unhashable + model), `decode` bypasses the cache so `pydantic.ValidationError` + surfaces cleanly instead of leaking a `TypeError` to the caller. """ - with patch.object( - PydanticDecoder, - "_get_adapter", - side_effect=TypeError("unhashable type"), - ): - result = PydanticDecoder().decode(b"42", int) - assert result == 42 # noqa: PLR2004 - - with pytest.raises(pydantic.ValidationError): - PydanticDecoder().decode(b'"not-an-int"', int) + decoder = PydanticDecoder() + decoder._adapters = MagicMock() # noqa: SLF001 + decoder._adapters.get.side_effect = TypeError("unhashable type") # noqa: SLF001 + + result = decoder.decode(b"42", int) + assert result == 42 # noqa: PLR2004 + + with pytest.raises(pydantic.ValidationError): + decoder.decode(b'"not-an-int"', int) @pytest.mark.parametrize( From 95239a5208c23a05252843cbd6481cf560071977 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 17:29:35 +0300 Subject: [PATCH 3/4] test(decoders): pin can_decode/decode consistency for unhashable buildable models Task review of c44c80b found the shared memoizing cache changes can_decode's verdict (False -> True) for a genuinely unhashable, schema-buildable model (typing.Annotated[int, []]). Verified this fixes a pre-existing inconsistency rather than introducing a regression: decode() already handled such models correctly via its own fallback, while can_decode() incorrectly rejected them. Maintainer approved accepting the fix; the design doc's Summary, Motivation, Non-goals, and Testing sections were updated accordingly. Adds one regression test per decoder pinning the now-consistent behavior. --- ...7-13.05-decoders-shared-memoizing-cache.md | 48 +++++++++++++++++-- tests/test_decoders_msgspec.py | 17 +++++++ tests/test_decoders_pydantic.py | 17 +++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md b/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md index 6a4b2c8..f8cb099 100644 --- a/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md +++ b/planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md @@ -15,7 +15,10 @@ verdict cache (`can_decode`). This change extracts one generic four sites, and simplifies `decode()` in both classes by dropping a try/except that becomes dead code once the cache handles the unhashable case internally. Two existing tests that mocked the wrong layer are -corrected to test the real mechanism. No observable behavior change. +corrected to test the real mechanism. **One intentional, incidental +behavior fix**: `can_decode` now agrees with `decode` for genuinely +unhashable, schema-buildable models (see Motivation) — everything else is +behavior-preserving. ## Motivation @@ -55,12 +58,39 @@ corrected to test the real mechanism. No observable behavior change. existing fallback ever guarded against was the cache-lookup `TypeError`. Once that's handled inside `_get_or_build`, the fallback in `decode()` becomes genuinely unreachable and is removed. +- **Discovered during task review, verified empirically: a pre-existing + inconsistency between `can_decode` and `decode` for genuinely + unhashable models, which this refactor incidentally fixes.** Today, + `_probe_can_decode` calls `self._get_adapter(model)` inside a broad + `except Exception: return False`. For a genuinely unhashable model + (e.g. `typing.Annotated[int, []]` — confirmed unhashable, and confirmed + buildable by both pydantic and msgspec), the *old* `_get_adapter`/ + `_get_msgspec_decoder` propagate the cache-lookup `TypeError` uncaught, + which that `except Exception` catches — so `can_decode` returns `False` + for *any* unhashable model today, regardless of whether the underlying + library could actually build a schema for it. Meanwhile `decode()` + already works fine for the same model (it has its own separate + fallback) — meaning today, `MissingDecoderError` can incorrectly fire + for a model `decode()` would have handled correctly. Once + `_get_or_build` swallows that same `TypeError` internally (which it + must, to keep `decode()` working for unhashable models — see the point + above), `_probe_can_decode`'s `try` no longer sees an exception for + this case, so `can_decode` now returns `True` — consistent with + `decode`'s actual capability. This is accepted as an intentional + incidental fix (see Non-goals and Testing). ## Non-goals -- No behavior change from the caller's perspective: `can_decode` and - `decode` return the same values for the same inputs, including for - unhashable models (fresh, uncached build every call, same as today). +- No *unintentional* behavior change from the caller's perspective. One + intentional exception, accepted as a bonus fix rather than reverted: + `can_decode` now returns `True` (not `False`) for a genuinely unhashable + model that the underlying library can actually build a schema for — + matching what `decode` already does for the same model, closing a + pre-existing inconsistency. Every other input/output pair for + `can_decode`/`decode` is unchanged, including unhashable models handled + via a fresh, uncached build (still true — that mechanism is unchanged, + only the *verdict* changes for the specific unhashable-but-buildable + case). - Not extracting the tiny `if not is_X_installed: raise ImportError(MESSAGE)` check duplicated in both `__init__`s (2 lines, different flag and message per call — a shared helper would need both @@ -231,6 +261,16 @@ the comment to name the new location. value-type flavors (bool, built object) of its `TypeError`-bypass path, and every cache-hit/cache-miss branch is already exercised by the existing cache-content and call-count assertions. +- **New regression tests for the intentional behavior fix** (added after + task review surfaced the gap empirically): one test per decoder, using + a genuinely unhashable model (`typing.Annotated[int, []]` — confirmed + unhashable via `hash()`, confirmed buildable by both pydantic and + msgspec), asserting `can_decode(model) is True` and that `decode(...)` + on the same model succeeds and agrees — proving the two are consistent + post-fix. No existing test used a genuinely unhashable model through + the real (non-mocked) code path; the two corrected tests from Design §4 + mock the cache dict with an otherwise-hashable model, which does not + exercise this interaction. - `just lint && just test` both clean; 100% coverage maintained. ## Risk diff --git a/tests/test_decoders_msgspec.py b/tests/test_decoders_msgspec.py index 480c7e1..4e54f5f 100644 --- a/tests/test_decoders_msgspec.py +++ b/tests/test_decoders_msgspec.py @@ -1,6 +1,7 @@ """Unit tests for httpware.decoders.msgspec.MsgspecDecoder.""" import dataclasses +import typing from http import HTTPStatus from unittest.mock import MagicMock, patch @@ -231,6 +232,22 @@ def test_msgspec_can_decode_unhashable_model_does_not_raise() -> None: assert decoder.can_decode(_Item) is True +def test_can_decode_agrees_with_decode_for_unhashable_buildable_model() -> None: + """can_decode and decode must agree for a genuinely unhashable, schema-buildable model. + + `typing.Annotated[int, []]` is unhashable (its metadata is a list) but + msgspec can still build a decoder for it. Before this refactor, + can_decode incorrectly returned False for any unhashable model + regardless of buildability; decode() already worked correctly for + the same model via its own fallback. This test pins the fixed, + consistent behavior. + """ + model = typing.Annotated[int, []] + decoder = MsgspecDecoder() + assert decoder.can_decode(model) is True # ty: ignore[invalid-argument-type] + assert decoder.decode(b"42", model) == 42 # noqa: PLR2004 # ty: ignore[invalid-argument-type] + + def test_contains_custom_type_raises_import_error_when_msgspec_absent( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/test_decoders_pydantic.py b/tests/test_decoders_pydantic.py index d33fc1d..1f87c6a 100644 --- a/tests/test_decoders_pydantic.py +++ b/tests/test_decoders_pydantic.py @@ -3,6 +3,7 @@ import asyncio import concurrent.futures import dataclasses +import typing from unittest.mock import MagicMock, patch import msgspec @@ -238,3 +239,19 @@ def test_pydantic_can_decode_unhashable_model_does_not_raise() -> None: decoder._can_decode_results = MagicMock() # noqa: SLF001 decoder._can_decode_results.get.side_effect = TypeError("unhashable type") # noqa: SLF001 assert decoder.can_decode(User) is True + + +def test_can_decode_agrees_with_decode_for_unhashable_buildable_model() -> None: + """can_decode and decode must agree for a genuinely unhashable, schema-buildable model. + + `typing.Annotated[int, []]` is unhashable (its metadata is a list) but + pydantic can still build a schema for it. Before this refactor, + can_decode incorrectly returned False for any unhashable model + regardless of buildability; decode() already worked correctly for + the same model via its own fallback. This test pins the fixed, + consistent behavior. + """ + model = typing.Annotated[int, []] + decoder = PydanticDecoder() + assert decoder.can_decode(model) is True # ty: ignore[invalid-argument-type] + assert decoder.decode(b"42", model) == 42 # noqa: PLR2004 # ty: ignore[invalid-argument-type] From ebde64d357bb9ad5b48f5836f0c2020b0e5333c4 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 13 Jul 2026 17:38:23 +0300 Subject: [PATCH 4/4] docs(architecture): document unhashable-model handling in decoders.md Final review flagged that decoders.md never documented unhashable-model behavior, and this branch changes it (can_decode now reflects true buildability regardless of hashability). Add the missing note per CLAUDE.md's convention of updating the matching architecture/ capability file in the same PR that changes its behavior. --- architecture/decoders.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/architecture/decoders.md b/architecture/decoders.md index be4f97d..0097abd 100644 --- a/architecture/decoders.md +++ b/architecture/decoders.md @@ -12,4 +12,5 @@ Both clients take `decoders: Sequence[ResponseDecoder] | None = None` (a *list*, - `decode(content: bytes, model: type[T]) -> T` — the decode itself. Any exception is wrapped by `_BoundDecoder.decode` — the bound decoder `resolve` returns, sealing decoder + model together — into `httpware.DecodeError` (a `ClientError` subclass carrying `response`, `model`, `original`); this runs for `send` / `send_with_response` on both clients when `response_model=` is set. Decoder implementers do not need to raise `DecodeError` directly. - **Pre-flight check:** when `response_model=` is set and no decoder claims it, `_DecoderResolver.resolve` (called by `send` / `send_with_response` before `_dispatch`) raises `MissingDecoderError(model=..., registered_names=...)` BEFORE the HTTP call. `MissingDecoderError` is a sibling of `DecodeError` under `ClientError`, and is distinct from it: `DecodeError` means the decoder ran and the payload was malformed; the two have distinct corrective actions (install an extra or pass `decoders=[...]`). - **Default list:** `decoders=None` resolves via `client.py:_build_default_decoders()` against installed extras — pydantic-first when both are present, either-only when only one is installed, empty tuple when neither. `AsyncClient()` / `Client()` never raise on missing extras; failure surfaces only at the first `response_model=` use site. -- **Rule:** the decoder must operate on raw bytes in a single parse pass. Two-pass decoding (`json.loads` then `validate_python`) is rejected: a single bytes-in / typed-object-out pass avoids the redundant intermediate `dict` allocation and parses faster. The Pydantic adapter implements this as `TypeAdapter(model).validate_json(content)`, with the `TypeAdapter` cached per-instance on `PydanticDecoder._adapters: dict[type, TypeAdapter]` (populated lazily on first `_get_adapter()` call); the msgspec adapter mirrors the pattern with `MsgspecDecoder._msgspec_decoders: dict[type, msgspec.json.Decoder]`. Cache lifetime matches the decoder/client, not the process — no module-level state, no autouse cache-clear fixtures in tests. +- **Rule:** the decoder must operate on raw bytes in a single parse pass. Two-pass decoding (`json.loads` then `validate_python`) is rejected: a single bytes-in / typed-object-out pass avoids the redundant intermediate `dict` allocation and parses faster. The Pydantic adapter implements this as `TypeAdapter(model).validate_json(content)`, with the `TypeAdapter` cached per-instance on `PydanticDecoder._adapters: dict[type, TypeAdapter]` (populated lazily on first `_get_adapter()` call); the msgspec adapter mirrors the pattern with `MsgspecDecoder._msgspec_decoders: dict[type, msgspec.json.Decoder]`. Cache lifetime matches the decoder/client, not the process — no module-level state, no autouse cache-clear fixtures in tests. Both built-ins' `_get_adapter`/`_get_msgspec_decoder` and `can_decode` share one memoizing helper (`decoders/_caching.py:_get_or_build`). +- **Unhashable models:** a `model` that isn't hashable (e.g. `Annotated[X, unhashable_metadata]`) bypasses the cache entirely — every call builds fresh, uncached. `can_decode`'s verdict reflects true buildability regardless of hashability: an unhashable model that the underlying library can actually build a schema/decoder for returns `True`, not `False`. Hashability is orthogonal to whether a model can be decoded.