From 022c56d6848b9893e2cbb77d51485cb0e0015728 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 10 Aug 2026 12:44:28 +0300 Subject: [PATCH] fix: type the middleware factory to FastStream's construction contract ty 0.0.69 added `unbound-type-variable` and the weekly dependency check went red: `_DIMiddlewareFactory.__call__` declared `*args: P.args, **kwargs: P.kwargs` with the ParamSpec `P` bound to nothing. It was never solvable, which is why the code carried two `# ty: ignore[invalid-argument-type]` comments conceding it did not match `BrokerMiddleware`. Declare FastStream's real contract instead -- `__call__(msg, /, *, context: ContextRepo) -> _DiMiddleware`, mirrored on `_DiMiddleware.__init__` -- and drop `P` plus `typing.Generic[P]`. Both ignore comments come out and `add_middleware(...)` now type-checks on its own, so a future upstream contract change is a ty error at the call site rather than a first-message runtime failure. `msg: object` rather than `Any`: the protocol's message parameter is contravariant and the value is only forwarded to `BaseMiddleware.__init__`. This fired the revisit trigger on the two-class-split decision. Collapsing to `functools.partial` was built and measured -- it is ~10 LOC shorter and equally clean -- but rejected: `partial` types as `(*args: Any, **kwargs: Any)` and satisfies any protocol, so breaking the construction contract under it leaves ty silent. The decision file records the outcome and a new trigger. Closes #38 Co-authored-by: Claude Opus 5 (1M context) --- architecture/dependency-injection.md | 11 ++- modern_di_faststream/main.py | 15 ++-- ...2026-08-10.01-middleware-contract-typed.md | 79 +++++++++++++++++++ .../2026-06-25-keep-dimiddlewarefactory.md | 36 +++++++++ 4 files changed, 131 insertions(+), 10 deletions(-) create mode 100644 planning/changes/2026-08-10.01-middleware-contract-typed.md diff --git a/architecture/dependency-injection.md b/architecture/dependency-injection.md index f6e8704..9d5b50c 100644 --- a/architecture/dependency-injection.md +++ b/architecture/dependency-injection.md @@ -39,7 +39,16 @@ is a `NameError` at import, not a silent runtime miss. `_DiMiddleware` (constructed by `_DIMiddlewareFactory`, which binds the container ahead of FastStream's deferred middleware construction — see [the decision to keep the two-class split][d-factory]) runs `consume_scope` on -every message: +every message. + +The factory declares FastStream's construction contract literally — +`__call__(msg, /, *, context: ContextRepo) -> _DiMiddleware`, mirrored on +`_DiMiddleware.__init__` — so `broker.add_middleware(_DIMiddlewareFactory(...))` +is checked against the `BrokerMiddleware` protocol at type-check time. That +call site is the package's guard against FastStream changing the contract +underneath it: a mismatch is a `ty` error, not a first-message runtime failure. + +On each message the middleware: 1. `modern_di.integrations.bind(faststream_message_provider, msg)` derives the child's scope and context from the message — `bind(provider, connection)` diff --git a/modern_di_faststream/main.py b/modern_di_faststream/main.py index c39b333..93b85d2 100644 --- a/modern_di_faststream/main.py +++ b/modern_di_faststream/main.py @@ -9,7 +9,6 @@ T_co = typing.TypeVar("T_co", covariant=True) -P = typing.ParamSpec("P") faststream_message_provider = providers.ContextProvider(scope=Scope.REQUEST, context_type=faststream.StreamMessage) @@ -27,15 +26,14 @@ class _DIMiddlewareFactory: def __init__(self, di_container: Container) -> None: self.di_container = di_container - def __call__(self, *args: P.args, **kwargs: P.kwargs) -> "_DiMiddleware[P]": - return _DiMiddleware(self.di_container, *args, **kwargs) + def __call__(self, msg: object, /, *, context: faststream.ContextRepo) -> "_DiMiddleware": + return _DiMiddleware(self.di_container, msg, context=context) -class _DiMiddleware(faststream.BaseMiddleware, typing.Generic[P]): - def __init__(self, di_container: Container, *args: P.args, **kwargs: P.kwargs) -> None: +class _DiMiddleware(faststream.BaseMiddleware): + def __init__(self, di_container: Container, msg: object, /, *, context: faststream.ContextRepo) -> None: self.di_container = di_container - # BaseMiddleware.__init__ expects (msg, /, *, context: ContextRepo); ParamSpec forwarding can't prove that. - super().__init__(*args, **kwargs) # ty: ignore[invalid-argument-type] + super().__init__(msg, context=context) async def consume_scope( self, @@ -73,8 +71,7 @@ def setup_di( # raising ContainerClosedError. Reopening an already-open container is a no-op. app.on_startup(container.open) app.after_shutdown(container.close_async) - # _DIMiddlewareFactory.__call__ ParamSpec doesn't structurally match BrokerMiddleware[Any, Any]. - app.broker.add_middleware(_DIMiddlewareFactory(container)) # ty: ignore[invalid-argument-type] + app.broker.add_middleware(_DIMiddlewareFactory(container)) return container diff --git a/planning/changes/2026-08-10.01-middleware-contract-typed.md b/planning/changes/2026-08-10.01-middleware-contract-typed.md new file mode 100644 index 0000000..766c0f9 --- /dev/null +++ b/planning/changes/2026-08-10.01-middleware-contract-typed.md @@ -0,0 +1,79 @@ +--- +summary: Typed the middleware factory to FastStream's real `(msg, /, *, context)` construction contract — unblocked `ty` 0.0.69's `unbound-type-variable`, dropped both `# ty: ignore[invalid-argument-type]`, and made `add_middleware` a checked call site. +--- + +# Change: Type the middleware factory to FastStream's construction contract + +**Lane:** lightweight — 6 insertions / 9 deletions in one source file, no +public-API change, no new test. + +## Goal + +`ty` 0.0.69 added `unbound-type-variable` and the weekly dependency check +([#38]) went red: + +``` +error[unbound-type-variable]: ParamSpec `P` is not in scope + --> modern_di_faststream/main.py:30:31 +``` + +`_DIMiddlewareFactory.__call__` declared `*args: P.args, **kwargs: P.kwargs` +with `P` bound to nothing. The ParamSpec was never solvable — which is why the +code carried two `# ty: ignore[invalid-argument-type]` comments conceding it +did not match `BrokerMiddleware`. The fix is to state the real signature. + +## Approach + +FastStream's `BrokerMiddleware` protocol is +`__call__(msg, /, *, context: ContextRepo) -> BaseMiddleware[...]`. Declare +exactly that on `_DIMiddlewareFactory.__call__` and mirror it on +`_DiMiddleware.__init__`; drop `P` and `typing.Generic[P]`. + +```python +def __call__(self, msg: object, /, *, context: faststream.ContextRepo) -> "_DiMiddleware": + return _DiMiddleware(self.di_container, msg, context=context) +``` + +`msg: object` (not `Any`) keeps ANN401 quiet and is sound — the protocol's +message parameter is contravariant, and `_DiMiddleware` only forwards the value +to `BaseMiddleware.__init__`. + +Both `# ty: ignore[invalid-argument-type]` comments come out: the ParamSpec +forwarding they papered over is gone, and `add_middleware(...)` now type-checks +on its own. + +This closes the revisit trigger in +[the two-class-split decision][d-factory] — the forwarding is type-clean, so +collapsing to `functools.partial` was reconsidered and **rejected on new +grounds**. See that file's Revisit outcome; in short, `partial` types as +`(*args: Any, **kwargs: Any)` and satisfies any protocol, so it silently +accepts contract drift that the explicit factory catches. The factory is the +assertion site, not a pass-through. + +Promotes into [`architecture/dependency-injection.md`](../../architecture/dependency-injection.md) +(Per-message scope). + +## Files + +- `modern_di_faststream/main.py` — concrete construction signature; `P` and both + `ty: ignore`s deleted +- `architecture/dependency-injection.md` — record the checked construction seam +- `planning/decisions/2026-06-25-keep-dimiddlewarefactory.md` — Revisit outcome + +## Verification + +- [x] Failing check first — `uv run ty check` → + `error[unbound-type-variable]: ParamSpec 'P' is not in scope`, 1 diagnostic. +- [x] Apply the change. +- [x] `uv run ty check` — `All checks passed!` +- [x] Contract-drift probe: renaming `context` in the factory's `__call__` makes + `ty` fail at the `add_middleware` call — + *"`_DIMiddlewareFactory` is not assignable to protocol + `BrokerMiddleware[Any, Any]` ... parameter `context` is missing"*. The same + break under `functools.partial` passes silently. This is the evidence + behind rejecting the collapse. +- [x] `just test` — 6 passed, 100% coverage. +- [x] `just lint` — clean. + +[#38]: https://github.com/modern-python/modern-di-faststream/issues/38 +[d-factory]: ../decisions/2026-06-25-keep-dimiddlewarefactory.md diff --git a/planning/decisions/2026-06-25-keep-dimiddlewarefactory.md b/planning/decisions/2026-06-25-keep-dimiddlewarefactory.md index 3cf72b5..c1db513 100644 --- a/planning/decisions/2026-06-25-keep-dimiddlewarefactory.md +++ b/planning/decisions/2026-06-25-keep-dimiddlewarefactory.md @@ -52,3 +52,39 @@ accepts a pre-bound middleware instance (removing the need for a deferred factory), **or** the `ParamSpec` forwarding becomes type-clean so the `# ty: ignore` can be dropped. At that point collapsing into a single class or a `partial` becomes a genuine simplification and this decision should be reopened. + +## Revisit outcome — 2026-08-10: reopened, decision re-affirmed + +The second trigger fired. [Typing the factory to FastStream's real +`(msg, /, *, context: ContextRepo)` contract][c-typed] made the forwarding +type-clean and dropped both `# ty: ignore[invalid-argument-type]` comments, so +`functools.partial(_DiMiddleware, container)` was built and measured against the +named factory. Both pass `ty`, ruff, and the suite; `partial` is ~10 LOC +shorter. + +**Keep the split anyway — on a ground this decision did not originally have.** +The above predicted `partial` "almost certainly keeps the same `# ty: ignore`"; +that prediction is now false, so the original argument no longer decides. What +decides instead is *type-checkability at the registration seam*: + +`functools.partial` types as `(*args: Any, **kwargs: Any)`, which is assignable +to **any** protocol. Under `partial`, breaking the construction contract — e.g. +renaming `_DiMiddleware.__init__`'s `context` keyword — leaves `ty` reporting +`All checks passed!`, and the mismatch surfaces at runtime on the first message. +With the explicit factory, the same break is a compile-time error at +`add_middleware`: *"`_DIMiddlewareFactory` is not assignable to protocol +`BrokerMiddleware[Any, Any]` ... parameter `context` is missing"*. + +That reframes the deletion test. The factory is not a pass-through whose +complexity merely *moves* — it is the site where this package's adaptation to +FastStream's contract is **asserted and checked**. Deleting it deletes the +check. For a package whose whole job is that adaptation, and which just spent +two release cycles with the mismatch masked by `ty: ignore`s, the ~10 LOC buy a +real guard against silent upstream drift. + +**New revisit trigger:** `functools.partial` (or the call site) gains precise +signature typing such that a contract break is caught at `add_middleware`, **or** +FastStream starts accepting a pre-bound middleware instance. Either removes the +factory's remaining justification. + +[c-typed]: ../changes/2026-08-10.01-middleware-contract-typed.md