docs(retro): 0.9.0 multi-decoder routing retro

Author: lesnik512

planning/retros/2026-06-10-v0.9-multi-decoder-routing.md

@@ -0,0 +1,99 @@
+# Retro: v0.9 multi-decoder routing
+
+**Released:** 2026-06-10 as `0.9.0`
+**Landing PRs:** [#41](https://github.com/modern-python/httpware/pull/41) (multi-decoder routing), [#42](https://github.com/modern-python/httpware/pull/42) (per-instance decoder cache follow-up)
+**Tag:** `0.9.0` at commit `b0b2bca`
+
+## 1. What shipped
+
+`AsyncClient` and `Client` now take `decoders: Sequence[ResponseDecoder] | None = None` (plural list) instead of `decoder: ResponseDecoder | None = None` (singular instance). The client dispatches `response_model=Foo` by walking the list and picking the first decoder whose new `can_decode(model) -> bool` predicate returns True. A new `MissingDecoderError` (sibling of `DecodeError` under `ClientError`) fires *before* the HTTP call when no decoder claims the model. The 0.3.0 eager `ImportError` on missing pydantic at `AsyncClient.__init__` is reversed — failure now surfaces only when `response_model=` is used and no decoder can handle it.
+
+PR #42 was an internal refactor: the module-level `@functools.lru_cache` factories (`_get_adapter`, `_get_msgspec_decoder`) became per-instance dicts on the decoder classes. No user-visible change; removes two autouse cache-clear test fixtures (one of which had been added one PR earlier as a review-loop fix in PR #41).
+
+## 2. The brainstorm pivot that made the release
+
+The session started with a narrow concern: "AsyncClient() raises ImportError if pydantic isn't installed, which is strange for an optional extra." I proposed a focused fix — lazy default. The user reframed mid-conversation: *"why is there a single decoder at all?"*
+
+That reframing changed the scope from a 1-task patch to a 9-task minor release. The "one decoder per client" invariant wasn't justified by anything about HTTP — it was an accident of the original Seam B shape. Once questioned, the right answer was a type-dispatched list. The lazy-default question dissolved as a side effect (a list can be empty without raising).
+
+**Lesson:** when a user asks "why is X this way?" and the answer is "historical accident," that's a signal the design is up for grabs. The narrow fix would have shipped without the user being able to mix pydantic + msgspec models in one client — a real capability gap that the broader pivot closed.
+
+## 3. The "narrow native-only" claim policy was wrong
+
+My first sketch had `PydanticDecoder.can_decode` claim only `BaseModel` subclasses and `MsgspecDecoder.can_decode` claim only `Struct` subclasses. The user pushed back with three words: *"Don't like this part. Let's think more."*
+
+The flaw was scope. With strict native-only claims, `response_model=dict`, `response_model=list[Foo]`, `response_model=MyDataclass`, `response_model=int` — all real use cases — get claimed by no decoder. The user would either have to write a third "everything else" decoder or accept that `response_model=` silently doesn't work for 80% of model shapes.
+
+The correct shape was broad-claim-ordering-wins: each decoder claims everything its library can handle (including dataclasses and primitives, which both libraries support); the user's `decoders=[...]` list order resolves ambiguity for shared shapes; native types route correctly because each library rejects the other's natives.
+
+**Lesson:** "narrow is safer" intuition fails when the shared-shape gap is large. The user's three-word pushback saved me from a design that would have looked clean and been useless.
+
+## 4. The msgspec CustomType surprise
+
+The single most surprising technical detail in the project. The spec assumed `msgspec.json.Decoder(pydantic.BaseModel)` would raise `TypeError` at construction, mirroring how pydantic's `TypeAdapter(msgspec.Struct)` raises. Empirically false: msgspec builds a Decoder via a generic `CustomType` fallback at construction time and only fails at decode time with `msgspec.ValidationError`.
+
+The Task 1 implementer caught this during TDD ("flagging the principled deviation: the plan's natural try/except probe doesn't work; switched to `msgspec.inspect.type_info(model)` + `CustomType` filter"). The fix is now load-bearing in `MsgspecDecoder.can_decode` and pinned by `test_msgspec_rejects_pydantic_basemodel`.
+
+A future agent who refactors `can_decode` to a "simpler" `try/except` around the Decoder constructor will silently break dispatch — `MsgspecDecoder` will claim `BaseModel`, route to it, and the user sees `DecodeError("Expected 'User', got 'dict'")` instead of `MissingDecoderError`. Memory entry `msgspec_basemodel_customtype_quirk.md` exists specifically to short-circuit that re-discovery.
+
+**Lesson:** spec assumptions about external library behavior must be empirically verified, not inferred from "this is how the library *would* work." The spec was wrong; the implementer was right to deviate; the deviation is now the canonical behavior.
+
+## 5. The per-instance cache wasn't in the original scope
+
+After 0.9.0's headline feature was design-locked and most of the implementation was done, the user noticed: *"we don't need lru cache for decoders because they are built at client creation."*
+
+The framing was slightly off (the lru_cache is on per-model adapters, not on decoder instances), but the underlying intuition was right: the cache's lifetime should match the decoder's, not the process's. That observation produced a 50-LOC cleanup PR (#42) that:
+
+- Removed module-level globals from both decoder modules.
+- Removed two autouse cache-clear test fixtures (one of which had been added one PR earlier in PR #41 as a review-loop fix — the symmetry mismatch the reviewer caught was a hint that the module-level cache shape wasn't load-bearing).
+- Made the test suite's cache invariants visible per-instance instead of through `functools.lru_cache.cache_info()`.
+
+The cleanup is technically not a 0.9.0 requirement, but folding it in before the tag kept the release coherent: future agents reading `decoders/pydantic.py` won't have to triangulate between "the new decoders=[...] API" and "the leftover module-level lru_cache from the old shape."
+
+**Lesson:** a user noticing "this part feels wrong" near the end of a feature is worth listening to even when the framing is slightly off. The underlying intuition is usually the load-bearing thing.
+
+## 6. The 100%-coverage gate shaped task granularity
+
+The plan's Tasks 4 and 5 (`AsyncClient` and `Client` migration) had to be one commit each — every existing test that reads `client._decoder` or passes `decoder=` breaks the moment the client init signature changes, and the 100% coverage gate rejects any half-state. Splitting them into smaller "rename attribute" + "rename kwarg" + "add dispatcher" commits would have left the suite broken in between.
+
+The writing-plans skill's "bite-sized tasks (2–5 minutes)" guidance is harder to apply under a strict coverage gate. The compromise was "atomic refactor = one task, even if it's bigger than 5 minutes, with all impacted test files migrated in the same commit." The plan documented this rationale upfront in a "Sequencing rationale" section so the implementer knew the bigger task wasn't accidental.
+
+**Lesson:** coverage gates are real constraints on task decomposition. Plans for projects with strict coverage should explicitly flag atomic refactors that can't be split.
+
+## 7. Per-task review pipeline cost vs. benefit
+
+Subagent-driven execution ran spec→quality reviews per task for Tasks 1 and 2; skipped them for Tasks 3 (one-line helper), 5 (sync mirror of already-reviewed Task 4), 6, and 7 (verbatim test files from the plan). The judgment was: when the task is mechanical or matches the plan verbatim, the formal review pipeline is ceremony.
+
+The final whole-feature reviewer caught a real issue per-task reviews missed: Section 7 of `engineering.md` ("Caller-facing pattern: consumers select the implementation by passing it explicitly, e.g., `AsyncClient(decoder=PydanticDecoder())`") was stale and contradicted the updated Seam B. Per-task reviews were looking at the local task scope; the whole-feature review caught the cross-task incoherence.
+
+**Lesson:** the final whole-branch review earns its keep specifically by catching things that are correct-in-isolation-per-task but wrong-when-the-feature-is-read-as-a-whole. Don't skip it.
+
+## 8. Network flakiness during subagent dispatch
+
+Two subagent invocations during the 9-task sequence hit "socket connection closed unexpectedly" mid-flight. Task 3 (`_build_default_decoders` helper) lost the agent before it could implement anything — recovered by doing the task inline. Task 4's quality reviewer hit the same error post-investigation but the report had landed before the disconnect.
+
+For long-running agents that touch many files, the failure mode is more likely than for short ones. Mitigation: design plan tasks so any individual subagent invocation can fail and be retried without losing significant work. The TDD discipline (tests first, then impl, then commit) naturally bounds the blast radius — if the network drops between "write tests" and "implement," the working tree has the failing tests but nothing else; restart is cheap.
+
+**Lesson:** the bigger the subagent task, the higher the network-fail probability. Prefer many smaller dispatches over one giant one. The subagent-driven workflow's "fresh agent per task" rule helps here.
+
+## 9. Decisions worth keeping
+
+- **Type-dispatched decoder list with broad-claim + ordering-wins semantics.** The right primitive for "I want to use both pydantic and msgspec."
+- **`can_decode(model) -> bool` as the Protocol predicate.** Explicit, cheap, no exceptions-as-control-flow.
+- **Pre-flight `MissingDecoderError`.** Failing before the HTTP call saves a wasted round-trip and gives a deterministic error message tied to client configuration, not server behavior.
+- **`registered_names: tuple[str, ...]` on the exception** (class names snapshot, not decoder instances). Pickle-safe, gives users a useful message hint, doesn't keep decoder instances alive in tracebacks.
+- **Per-instance cache.** Lifecycle matches the decoder/client; no module-level state to clear between tests; explicit ownership.
+
+## 10. Decisions worth revisiting
+
+- **`_dispatch_decoder` naming.** The PR #41 quality reviewer noted: it's a *finder*, not a *dispatcher*. The dispatch (calling `decoder.decode(...)`) happens at the call site. Kept the current name for consistency with the spec's "dispatch" terminology, but `_find_decoder` or `_match_decoder` would read better. Open for a future rename.
+- **`_registered_names()` helper extraction.** Currently `tuple(type(d).__name__ for d in self._decoders)` appears at 4 raise sites (2 in each client class). The Task 5 implementer made a defensible judgment call to leave it inline (4 short lines vs. one indirection). If a 5th call site is ever added, extract.
+- **`pyproject.toml` version field is a stale placeholder.** Has been `"0.3.0"` since the 0.3.0 release; the publish CI sets the real version via `uv version $GITHUB_REF_NAME` from the git tag. Works, but reads as broken to a casual contributor. Optional cleanup: dynamic-versioning, or a CI gate that rejects PRs where pyproject.toml version doesn't match the highest tag.
+
+## 11. Lessons (short form)
+
+- **A user's three-word pushback can re-shape a feature.** "Don't like this part. Let's think more" saved a bad design.
+- **Spec assumptions about external library behavior must be empirically verified.** The msgspec `CustomType` surprise would have shipped wrong if the implementer hadn't tested.
+- **The final whole-branch review catches things per-task reviews miss.** Cross-task stale references, narrative drift, contradictions between sections.
+- **The brainstorm is harder than the plan when the design is genuinely open.** ~6 question rounds in the brainstorm; the plan flowed once the design was settled.
+- **Memory entries pay off when they preserve non-obvious technical detail.** The `msgspec_basemodel_customtype_quirk` entry is the kind of thing a fresh agent would otherwise spend 30 minutes re-discovering.