refactor(decoders): share memoizing get-or-build cache across adapters#101
Merged
Conversation
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
…dable 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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_get_or_build(cache, model, build)into a newdecoders/_caching.py, replacing 4 hand-duplicated cache sites acrossPydanticDecoder/MsgspecDecoder(the object cache and the verdict cache, each duplicated once per decoder).decode()in both classes — itstry/except TypeErrorfallback becomes dead code once the shared cache handles the unhashable-model case internally (traced through Seam B's contract to confirm it's genuinely unreachable).decode()'s now-removed fallback) are corrected to mock the cache dict instead.Behavior change (intentional, reviewed, accepted):
can_decode()now returnsTrue(notFalse) for a genuinely unhashable model that the underlying library can actually build a schema/decoder for (e.g.Annotated[int, []]). This fixes a pre-existing inconsistency —decode()already handled such models correctly via its own fallback, whilecan_decode()incorrectly rejected them, meaningMissingDecoderErrorcould wrongly fire pre-flight for a model that would have decoded fine. This was caught by task review via an empirical repro, investigated, and explicitly approved as an accepted fix rather than reverted. Two new regression tests (one per decoder, using a genuinely unhashable model through the real non-mocked path) pin the corrected behavior.architecture/decoders.mdis updated to document it.Design:
planning/changes/2026-07-13.05-decoders-shared-memoizing-cache.md(updated in-flight to record the finding and the decision)Test plan
just test— 779 passed, 100% coveragejust lint-ci— ruff format/check,ty check, planning validator all cleanjust docs-build— mkdocs--strictclean