|
| 1 | +# Spec: Small-fixes mop-up (0.8.5) |
| 2 | + |
| 3 | +**Date:** 2026-06-08 |
| 4 | +**Topic slug:** `small-fixes-mop-up` |
| 5 | +**Branch:** `fix/small-mop-up` |
| 6 | +**Target release:** `0.8.5` — patch (4 unrelated small fixes, no API change) |
| 7 | +**Status:** drafted, awaiting user review |
| 8 | + |
| 9 | +## Purpose |
| 10 | + |
| 11 | +Close 4 small audit findings in one PR — two Lows in production code (chain.py TYPE_CHECKING violation, pydantic.py NameError window), one user-facing docs Nit (LoggingMiddleware print() example), and one public-API test Nit (asymmetric `__all__` assertion). Each is independently small; bundling avoids release churn. |
| 12 | + |
| 13 | +| # | Severity | File | Headline | |
| 14 | +|---|---|---|---| |
| 15 | +| 1 | Low | `src/httpware/middleware/chain.py:9-10` | `if typing.TYPE_CHECKING:` block prevents `typing.get_type_hints()`; violates project memory | |
| 16 | +| 2 | Low | `src/httpware/decoders/pydantic.py:15-16, 27, 43` | `TypeAdapter` is conditionally bound — `NameError` window if module is reloaded with `is_pydantic_installed=False` | |
| 17 | +| 3 | Nit | `docs/middleware.md:156-161` | `LoggingMiddleware` example uses `print()` — contradicts CLAUDE.md "no print()" invariant | |
| 18 | +| 4 | Nit | `tests/test_public_api.py:69-71` | `expected - __all__` is one-directional; bogus entries in `__all__` are not caught | |
| 19 | + |
| 20 | +## Non-goals |
| 21 | + |
| 22 | +- No new public API. No new exception types. |
| 23 | +- No changes to `Middleware` / `AsyncMiddleware` Protocol definitions. |
| 24 | +- No restructuring of the optional-extras seam beyond the targeted pydantic.py change. |
| 25 | +- No rewrite of the `RequestIdMiddleware` example next door (only `LoggingMiddleware` is in scope). |
| 26 | +- No changes to msgspec.py — the audit flagged pydantic specifically. |
| 27 | + |
| 28 | +## Architecture |
| 29 | + |
| 30 | +### Four commits, one PR |
| 31 | + |
| 32 | +Order is mechanical (small standalone fixes — no dependencies between them): |
| 33 | + |
| 34 | +1. `fix(chain): hoist Middleware/AsyncMiddleware imports out of TYPE_CHECKING guard` — finding #1 |
| 35 | +2. `fix(pydantic): unconditional TypeAdapter import — eliminate NameError window` — finding #2 |
| 36 | +3. `docs(middleware): rewrite LoggingMiddleware example with logging, not print()` — finding #3 |
| 37 | +4. `test(public-api): symmetric assertion against __all__` — finding #4 |
| 38 | +5. `docs(release): draft 0.8.5 notes` |
| 39 | + |
| 40 | +(4 fix commits + 1 release-notes commit = 5 total.) |
| 41 | + |
| 42 | +## Per-finding change list |
| 43 | + |
| 44 | +### Finding #1 — chain.py TYPE_CHECKING block |
| 45 | + |
| 46 | +The current code: |
| 47 | + |
| 48 | +```python |
| 49 | +import typing |
| 50 | +from collections.abc import Awaitable, Callable, Sequence |
| 51 | + |
| 52 | +import httpx2 |
| 53 | + |
| 54 | + |
| 55 | +if typing.TYPE_CHECKING: |
| 56 | + from httpware.middleware import AsyncMiddleware, Middleware |
| 57 | + |
| 58 | + |
| 59 | +_AsyncNext: typing.TypeAlias = Callable[[httpx2.Request], Awaitable[httpx2.Response]] |
| 60 | +_Next: typing.TypeAlias = Callable[[httpx2.Request], httpx2.Response] |
| 61 | + |
| 62 | + |
| 63 | +def compose_async(middleware: "Sequence[AsyncMiddleware]", terminal: _AsyncNext) -> _AsyncNext: |
| 64 | + ... |
| 65 | + |
| 66 | + |
| 67 | +def compose(middleware: "Sequence[Middleware]", terminal: _Next) -> _Next: |
| 68 | + ... |
| 69 | +``` |
| 70 | + |
| 71 | +The two function signatures use string annotations referencing `AsyncMiddleware` / `Middleware`, which are only imported when `typing.TYPE_CHECKING`. `typing.get_type_hints(compose_async)` at runtime raises `NameError: name 'AsyncMiddleware' is not defined`. |
| 72 | + |
| 73 | +Audit verified: `httpware.middleware.__init__` does not import `chain.py` back. So hoisting the imports to module top is safe. |
| 74 | + |
| 75 | +**Fix:** drop the `if typing.TYPE_CHECKING:` guard; import the protocols unconditionally; drop the string-annotation quotes from the parameter types. |
| 76 | + |
| 77 | +```python |
| 78 | +import typing |
| 79 | +from collections.abc import Awaitable, Callable, Sequence |
| 80 | + |
| 81 | +import httpx2 |
| 82 | + |
| 83 | +from httpware.middleware import AsyncMiddleware, Middleware |
| 84 | + |
| 85 | + |
| 86 | +_AsyncNext: typing.TypeAlias = Callable[[httpx2.Request], Awaitable[httpx2.Response]] |
| 87 | +_Next: typing.TypeAlias = Callable[[httpx2.Request], httpx2.Response] |
| 88 | + |
| 89 | + |
| 90 | +def compose_async(middleware: Sequence[AsyncMiddleware], terminal: _AsyncNext) -> _AsyncNext: |
| 91 | + ... |
| 92 | + |
| 93 | + |
| 94 | +def _wrap(layer: AsyncMiddleware, inner: _AsyncNext) -> _AsyncNext: |
| 95 | + ... |
| 96 | + |
| 97 | + |
| 98 | +def compose(middleware: Sequence[Middleware], terminal: _Next) -> _Next: |
| 99 | + ... |
| 100 | + |
| 101 | + |
| 102 | +def _wrap_sync(layer: Middleware, inner: _Next) -> _Next: |
| 103 | + ... |
| 104 | +``` |
| 105 | + |
| 106 | +(String annotations on `_wrap` and `_wrap_sync` also unquoted.) |
| 107 | + |
| 108 | +### Finding #2 — pydantic.py NameError window |
| 109 | + |
| 110 | +The current code conditionally imports `TypeAdapter`: |
| 111 | + |
| 112 | +```python |
| 113 | +if import_checker.is_pydantic_installed: |
| 114 | + from pydantic import TypeAdapter |
| 115 | +``` |
| 116 | + |
| 117 | +Then references `TypeAdapter` at runtime in `_get_adapter` and `PydanticDecoder.decode`'s `TypeError` fallback. When the flag is False (only reachable by test-reload), `TypeAdapter` is undefined, and subsequent calls raise `NameError` instead of the documented `ImportError`. |
| 118 | + |
| 119 | +**Fix (audit's Option 1):** hoist the `from pydantic import TypeAdapter` unconditionally. The module is gated upstream by `client.py:_default_pydantic_decoder()`'s fail-fast check (`PydanticDecoder.__init__` raises `ImportError` when the flag is False) — there is no real-world path that loads this module without pydantic installed. |
| 120 | + |
| 121 | +```python |
| 122 | +"""PydanticDecoder — module-level cached TypeAdapter adapter for ResponseDecoder. |
| 123 | +
|
| 124 | +Requires the `pydantic` extra: `pip install httpware[pydantic]`. The optional-extras |
| 125 | +gate is enforced upstream — `client.py:_default_pydantic_decoder()` raises ImportError |
| 126 | +before this module is imported when pydantic is absent. Tests simulating "pydantic |
| 127 | +not installed" patch `import_checker.is_pydantic_installed=False` at runtime, which |
| 128 | +makes `PydanticDecoder.__init__` raise ImportError; the module itself is still loaded. |
| 129 | +""" |
| 130 | + |
| 131 | +import functools |
| 132 | +from typing import TypeVar |
| 133 | + |
| 134 | +from pydantic import TypeAdapter |
| 135 | + |
| 136 | +from httpware._internal import import_checker |
| 137 | +``` |
| 138 | + |
| 139 | +The module docstring updates to describe the new contract; the `if import_checker.is_pydantic_installed:` guard around the import is removed. |
| 140 | + |
| 141 | +This change does NOT affect the existing pydantic-missing tests (which patch the flag at runtime — the module is already loaded by then). The isolation test (`test_importing_httpware_does_not_import_pydantic`) is also unaffected: `import httpware` does not transitively import `httpware.decoders.pydantic` (verified by the test passing today and the import graph staying the same after this change). |
| 142 | + |
| 143 | +### Finding #3 — LoggingMiddleware print() |
| 144 | + |
| 145 | +The current `docs/middleware.md` snippet: |
| 146 | + |
| 147 | +```python |
| 148 | +import httpx2 |
| 149 | + |
| 150 | +from httpware import Client |
| 151 | +from httpware.middleware import Next |
| 152 | + |
| 153 | + |
| 154 | +class LoggingMiddleware: |
| 155 | + def __call__(self, request: httpx2.Request, next: Next) -> httpx2.Response: # noqa: A002 |
| 156 | + print(f"-> {request.method} {request.url}") |
| 157 | + response = next(request) |
| 158 | + print(f"<- {response.status_code}") |
| 159 | + return response |
| 160 | + |
| 161 | + |
| 162 | +with Client(base_url="https://api.example.com", middleware=[LoggingMiddleware()]) as client: |
| 163 | + client.get("/users/1") |
| 164 | +``` |
| 165 | + |
| 166 | +**Fix:** replace the two `print()` calls with a module-level logger and `info()` calls. Mirror the style of the existing `RequestIdMiddleware` example in the same file (which already uses `logging.getLogger(...)` pattern). |
| 167 | + |
| 168 | +```python |
| 169 | +import logging |
| 170 | + |
| 171 | +import httpx2 |
| 172 | + |
| 173 | +from httpware import Client |
| 174 | +from httpware.middleware import Next |
| 175 | + |
| 176 | + |
| 177 | +_LOGGER = logging.getLogger("myapp.logging_middleware") |
| 178 | + |
| 179 | + |
| 180 | +class LoggingMiddleware: |
| 181 | + def __call__(self, request: httpx2.Request, next: Next) -> httpx2.Response: # noqa: A002 |
| 182 | + _LOGGER.info("-> %s %s", request.method, request.url) |
| 183 | + response = next(request) |
| 184 | + _LOGGER.info("<- %s", response.status_code) |
| 185 | + return response |
| 186 | + |
| 187 | + |
| 188 | +with Client(base_url="https://api.example.com", middleware=[LoggingMiddleware()]) as client: |
| 189 | + client.get("/users/1") |
| 190 | +``` |
| 191 | + |
| 192 | +### Finding #4 — test_expected_exports symmetric assertion |
| 193 | + |
| 194 | +The current test (`tests/test_public_api.py:69-71`): |
| 195 | + |
| 196 | +```python |
| 197 | +missing = expected - set(httpware.__all__) |
| 198 | +assert not missing, f"expected exports missing from __all__: {missing}" |
| 199 | +``` |
| 200 | + |
| 201 | +Catches symbols in `expected` not in `__all__`, but not the reverse — a symbol added to `__all__` that ISN'T in `expected` slips through. (The companion `test_all_exports_resolve` catches symbols in `__all__` that don't actually exist; the gap is real symbols accidentally added to `__all__`.) |
| 202 | + |
| 203 | +**Fix:** symmetric assertion. The cleanest is to drop the directional subtraction and assert set equality: |
| 204 | + |
| 205 | +```python |
| 206 | +actual = set(httpware.__all__) |
| 207 | +assert expected == actual, ( |
| 208 | + f"__all__ mismatch:\n" |
| 209 | + f" missing from __all__: {expected - actual}\n" |
| 210 | + f" unexpected in __all__: {actual - expected}" |
| 211 | +) |
| 212 | +``` |
| 213 | + |
| 214 | +The error message names both directions so future test failures pinpoint which side drifted. |
| 215 | + |
| 216 | +## Tests |
| 217 | + |
| 218 | +For findings #1 + #4, add focused tests: |
| 219 | + |
| 220 | +### Test for finding #1 (chain.py TYPE_CHECKING fix) |
| 221 | + |
| 222 | +Add to `tests/test_middleware.py` (or `tests/test_middleware_sync.py`, whichever fits — the test exercises `compose_async`/`compose`): |
| 223 | + |
| 224 | +```python |
| 225 | +def test_compose_async_get_type_hints_resolves_without_nameerror() -> None: |
| 226 | + """typing.get_type_hints(compose_async) must resolve to real classes, not raise NameError. |
| 227 | +
|
| 228 | + Pre-0.8.5: AsyncMiddleware was imported only under `if typing.TYPE_CHECKING`, |
| 229 | + so get_type_hints raised NameError at runtime. |
| 230 | + """ |
| 231 | + import typing |
| 232 | + |
| 233 | + from httpware.middleware.chain import compose_async |
| 234 | + |
| 235 | + hints = typing.get_type_hints(compose_async) |
| 236 | + # The 'middleware' parameter's hint should mention AsyncMiddleware (Sequence[AsyncMiddleware]). |
| 237 | + assert "middleware" in hints |
| 238 | + |
| 239 | + |
| 240 | +def test_compose_get_type_hints_resolves_without_nameerror() -> None: |
| 241 | + """Sync mirror of the above for the sync `compose`.""" |
| 242 | + import typing |
| 243 | + |
| 244 | + from httpware.middleware.chain import compose |
| 245 | + |
| 246 | + hints = typing.get_type_hints(compose) |
| 247 | + assert "middleware" in hints |
| 248 | +``` |
| 249 | + |
| 250 | +### Test for finding #4 (symmetric assertion) |
| 251 | + |
| 252 | +The rewritten `test_expected_exports` IS the test — no additional test needed. To verify the symmetric assertion catches the new failure mode, the implementer should manually inject a fake symbol into `__all__` locally, run the test, confirm it fails with the "unexpected in __all__" message, then revert. |
| 253 | + |
| 254 | +### No new test for finding #2 (pydantic NameError window) |
| 255 | + |
| 256 | +The NameError window was only reachable by module-reload — a scenario that doesn't occur in production and the existing tests don't exercise. After Fix #2, `TypeAdapter` is always bound at module-load (since pydantic is required for the module to load). A test that "reloads the module with the flag patched" would only verify the new module-import contract, which is already implicitly tested by every test that imports `pydantic.py`. |
| 257 | + |
| 258 | +### No new test for finding #3 (LoggingMiddleware print) |
| 259 | + |
| 260 | +It's a docs example. The change is verified by reading the docs file and confirming no `print()` calls remain in the snippet. |
| 261 | + |
| 262 | +## Verification |
| 263 | + |
| 264 | +After each commit: |
| 265 | + |
| 266 | +```bash |
| 267 | +just lint-ci |
| 268 | +uv run pytest -x --no-cov -q |
| 269 | +``` |
| 270 | + |
| 271 | +Full suite + lint green after every commit. |
| 272 | + |
| 273 | +## Release notes |
| 274 | + |
| 275 | +`planning/releases/0.8.5.md` — short patch, no behavioral change visible to users on the happy path. The pydantic.py change is a contract narrowing (the docstring "importing without the extra works" was untrue in the test-reload edge case; now the contract matches reality). The chain.py change is observable only via `typing.get_type_hints` — useful for users introspecting middleware signatures. |
| 276 | + |
| 277 | +## Acceptance criteria |
| 278 | + |
| 279 | +1. Four fix commits + one release-notes commit on branch `fix/small-mop-up`. |
| 280 | +2. `just lint-ci` and `uv run pytest` green after every commit. |
| 281 | +3. PR opened against `main` with title `fix(small-fixes): close 4 audit findings (0.8.5)`. |
| 282 | +4. After merge, tag `0.8.5` from the merge SHA; GitHub Release published from `planning/releases/0.8.5.md`. |
| 283 | +5. Memory `release_0_8_5_shipped` added. |
| 284 | + |
| 285 | +## Open questions |
| 286 | + |
| 287 | +None. All four fixes are precisely specified and the audit's recommended directions hold. |
0 commit comments