diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 7507fe2a..e0d16e9b 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -40,8 +40,8 @@ effect. State the numbers, not "benchmarked". `modern_di/` → don't write it; enforceable → a test; a user needs it → `docs/`; otherwise it does not get written. - [ ] **Rejected an alternative** with reasoning that would otherwise be - re-litigated? File it in [`planning/decisions/`](../planning/decisions/) - with a revisit trigger — not here. + re-litigated? File it as an ADR in [`docs/adr/`](../docs/adr/), numbered + `NNNN-slug.md`, with a revisit trigger — not here. - [ ] **Found real work you are not doing now?** File it in [`planning/deferred/`](../planning/deferred/), self-contained, with a revisit trigger — not here. diff --git a/CLAUDE.md b/CLAUDE.md index e565afd2..e3916d59 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,13 +15,13 @@ or read it for every recipe and its intent. The non-obvious essentials: - `just test [args]` — pytest, **no coverage**; targeted runs won't trip the gate. Passes args through: `just test tests/providers/test_factory.py -k `. - `just test-ci` — the **gated** full run (100% line coverage); this is what CI runs. -- `just lint` (autofix) / `just lint-ci` (no autofix; also validates planning bundles). -- `just check-planning` validates `planning/deferred/` + `planning/decisions/` frontmatter; `just index` prints that listing. +- `just lint` (autofix) / `just lint-ci` (no autofix; also runs the repo-wide link check). +- `just check-links` validates every relative Markdown link and heading anchor in the repo, including the trees `mkdocs --strict` never sees. ## Architecture - **Scope** — `IntEnum`, `APP=1 → SESSION=2 → REQUEST=3 → ACTION=4 → STEP=5`. A provider resolves only from a container of the same or deeper (higher-int) scope; otherwise a clear error is raised. -- **Container** — the central object. Root: `Container(scope=Scope.APP, groups=[MyGroup])`; children via `container.build_child_container(scope=Scope.REQUEST, context={...})`. Children share the parent's providers/overrides registries; cache/context are per-container. Pass `validate=True` (or call `container.validate()`) for cycle + transitive-scope checks. +- **Container** — the central object. Root: `Container(scope=Scope.APP, groups=[MyGroup])`; children via `container.build_child_container(scope=Scope.REQUEST, context={...})`. Children share the parent's providers/overrides registries; cache/context are per-container. Call `container.validate()` for cycle + transitive-scope checks; it is the only trigger — `Container(validate=...)` is accepted, ignored, and warns. There is no separate capability-page home for behavior detail — it lives in the code and its `INVARIANT:`-marked tests. Before writing prose about a capability, run the admission check in @@ -67,15 +67,15 @@ verification); it is reviewed with the diff. There is no change file and no lane to choose. A trivial PR (typo, dep bump, formatter) deletes the template and ships a conventional-commit title. -Two things outlive the PR and are committed under `planning/`: an alternative -**rejected** with reasoning goes to `planning/decisions/`, and real work **not +Two things outlive the PR: an alternative **rejected** with reasoning becomes an +ADR in [`docs/adr/`](docs/adr/) (`NNNN-slug.md`, sequential — see +[`docs/agents/domain.md`](docs/agents/domain.md)), and real work **not scheduled** goes to `planning/deferred/` (self-contained, with a revisit trigger). There is no separate truth-home directory — the living truth about behaviour is the code and its `INVARIANT:`-marked tests, and a behaviour change is reviewed with the diff, not promoted to a page. See -[`planning/README.md`](planning/README.md) for the full convention, including -the admission check that decides where a given fact belongs; it is a documented -local deviation from `planning-convention` 2.2.0. +[`planning/README.md`](planning/README.md) for the admission check that decides +where a given fact belongs. - **Cutting a release (maintainers)** is tag-driven via [`.github/workflows/release.yml`](.github/workflows/release.yml): write the @@ -103,8 +103,8 @@ local deviation from `planning-convention` 2.2.0. - Docstrings: public API documents the contract; internal helpers get a one-line contract, plus at most 1–2 lines for a genuinely non-obvious constraint. Never narrate implementation or justify code to a reviewer — - cross-file rationale lives in an `INVARIANT:` test docstring or - `planning/decisions/`. + cross-file rationale lives in an `INVARIANT:` test docstring or an ADR under + `docs/adr/`. ## Vocabulary diff --git a/docs/adr/0001-sync-async-close-separate.md b/docs/adr/0001-sync-async-close-separate.md new file mode 100644 index 00000000..0b32db15 --- /dev/null +++ b/docs/adr/0001-sync-async-close-separate.md @@ -0,0 +1,24 @@ +# Keep sync and async `close` paths separate + +**Decision:** `close_sync` / `close_async` stay explicit pairs at all three layers (`Container`, +`CacheRegistry`, `CacheItem`); they are not unified into single parametrized methods. + +"Six near-identical `close_*` methods" is a surface reading. Only the `Container` pair is +near-identical (one line differs). The other two diverge intrinsically: + +- **`CacheItem`** — async awaits the finalizer's result; sync **cannot await**, so it detects an + async finalizer, `.close()`s the coroutine to suppress the never-awaited warning, and raises + `AsyncFinalizerInSyncCloseError`. +- **`CacheRegistry`** — async clears `_creation_order` entirely; sync **preserves** the items that + raised `AsyncFinalizerInSyncCloseError` so a later `close_async()` can finish them. + +The genuinely shared code is a ~4-line wrapper, a one-line guard, and an iterate-collect-raise +skeleton; unifying would re-introduce the sync-can't-await and preserve-for-later behaviours as +conditional branches, **adding** complexity rather than concentrating it. Those branches also carry +every historical finalizer fix — LIFO teardown (`3f9a64b`), await/reject sync finalizers +(`19e7c72`), async-finalizer rejection (`faf2108`), `clear_cache` finalizer-dedup (`8ce0ff4`), all +shipped in 2.15.0 — and the area has been stable since. + +**Revisit trigger:** finalizer/close bugs start recurring (the signal that the explicit pairs are +*causing* errors rather than encoding them), or a third distinct sync/async-spanning consumer +appears that would share real logic. diff --git a/docs/adr/0002-cache-arg-over-singleton-class.md b/docs/adr/0002-cache-arg-over-singleton-class.md new file mode 100644 index 00000000..d0bd64ab --- /dev/null +++ b/docs/adr/0002-cache-arg-over-singleton-class.md @@ -0,0 +1,22 @@ +# Ergonomic caching toggle: `cache=` argument, not a `Singleton` class + +**Decision:** `Factory`'s caching toggle is one `cache` argument accepting `bool | CacheSettings | +None` — absent/`None`/`False` off, `True` on with defaults, `CacheSettings(...)` on and tuned. One +argument, one mental model, one place caching is expressed. `CacheSettings` is unchanged as the +tuning object; the sugar normalizes into the existing `self.cache_settings` attribute. + +Rejected alternatives, all forms of expressing caching twice: + +- **A `Singleton` provider class.** Says "caching is on" in the class name *and* in a still-required + `cache_settings` for finalizers (the most common advanced case); the two can drift, and forbidding + `cache_settings` on a `Singleton` would strand finalizers. It also reverses the 2.x "no separate + `Singleton` class" call. +- **A `cached=True` flag alongside `cache_settings=`.** Two arguments meaning "cache" needs a + both-passed conflict rule, and `cached=True` has no path to a finalizer without switching forms. +- **Overloading `cache_settings=` to accept `True`.** Works and needs no new name, but the noun-y + argument reads wrong (`settings=True`); renaming keeps the single-axis model and reads naturally + in both forms. + +**Revisit trigger:** a caching mode a single `bool | CacheSettings` argument cannot express cleanly. +The other half of the original trigger has fired and resolved: 3.0 dropped the `cache_settings=` +alias, so `cache=` is the only spelling. diff --git a/docs/adr/0003-no-enter-scope-alias.md b/docs/adr/0003-no-enter-scope-alias.md new file mode 100644 index 00000000..76e61d05 --- /dev/null +++ b/docs/adr/0003-no-enter-scope-alias.md @@ -0,0 +1,17 @@ +# No `enter_scope` alias for `build_child_container` + +**Decision:** `build_child_container` remains the single scope-entry spelling; no `enter_scope` +alias and no rename. + +Every peer names this operation by intent — wireup `enter_scope({Type: obj})`, .NET +`CreateScope()`, dishka's callable container — and `build_child_container` names the mechanism and +is the longest scope-entry spelling in the studied field. Rejected anyway: in modern-di the +mechanism *is* the concept. Child containers are real, user-visible objects with their own cache and +context registries, and "enter scope" vocabulary would hide exactly the mental model the docs work +to teach. A second spelling of the most-written call after `resolve()` also conflicts with the +conservative-feature-set constraint, with wireup's serial renames as the cautionary precedent. + +**Revisit trigger:** recurring user feedback that scope entry is hard to discover — issues asking +"how do I enter a request scope". Closing the migrant-familiarity gap in the docs, with a +vocabulary-table row mapping `enter_scope` / `CreateScope` to `build_child_container`, is the +cheaper response and has not been written yet. diff --git a/docs/adr/0004-no-generator-creators.md b/docs/adr/0004-no-generator-creators.md new file mode 100644 index 00000000..1ebe6f72 --- /dev/null +++ b/docs/adr/0004-no-generator-creators.md @@ -0,0 +1,18 @@ +# No generator creators in core `Factory` + +**Decision:** `Factory` does not auto-detect generator creators and turn post-`yield` code into a +finalizer; `CacheSettings(finalizer=)` remains the only teardown spelling in core. + +Every Python peer — dishka, wireup, svcs, FastAPI yield-dependencies, that-depends `Resource` — +spells teardown as code after `yield`, making it the strongest muscle-memory delta for migrants. It +was rejected because the change is breaking (`Factory(creator=generator_fn)` is legal today and +resolves to the raw generator object) and carries open design complexity: per-instance finalizer +records for non-cached factories, declaration-time rejection of async generators, and `bound_type` +extraction from `Iterator[T]`. That is a large addition against a modest ergonomic win, and the +capability is reachable without core changes — a `Factory` subclass in userland or a sibling package +can wrap a generator creator and register the continuation through `CacheSettings(finalizer=)`. One +explicit teardown spelling also preserves the property that async finalizers work under sync +resolution, which the generator form cannot express. + +**Revisit trigger:** recurring user requests for yield-based teardown, or a community-built +generator-factory subclass demonstrating both the demand and a settled design. diff --git a/docs/adr/0005-no-multibinding.md b/docs/adr/0005-no-multibinding.md new file mode 100644 index 00000000..d54a0e55 --- /dev/null +++ b/docs/adr/0005-no-multibinding.md @@ -0,0 +1,26 @@ +# No multibinding or collection injection + +**Decision:** `modern-di` does not support registering several providers for one type and injecting +them together as a collection. There is no `multi=True`, no `list[T]` fan-in, no binder-set API. +Field precedent is broad (MEDI's `IEnumerable`, Spring's `List`, `injector`'s multibind, +Angular's `multi: true`), and the usual motivation is plugin-style extension. + +The registry is a **type → provider map**. Multibinding turns it into type → *collection of* +providers, which changes every operation built on it: + +- Registration stops being "this type is now wired" and becomes "this type has one more + contributor", so `DuplicateProviderTypeError` has to become conditional on an opt-in flag. +- Resolution by type stops having one answer: `resolve(T)` and `resolve(list[T])` would resolve + different things from one registration, and the wiring plan would need a third parameter category. +- Overrides get ambiguous — overriding `T` either replaces the collection or one contributor, and + both readings are defensible. +- Validation loses the property that a missing type is unambiguous: an empty collection is + indistinguishable from a wiring mistake. + +That is permanent, structural registry cost for demand inferred from other ecosystems rather than +observed in this one. The workaround costs a user one provider: a `Factory` that takes the +individual dependencies and returns the list. + +**Revisit trigger:** concrete user demand — a real plugin-style use case the one-Factory workaround +does not serve. Field precedent alone is not the trigger; it is what was already weighed here. If +reopened, the registry-semantics consequences above are the design problem, not the API spelling. diff --git a/docs/adr/0006-no-redirect-plugin.md b/docs/adr/0006-no-redirect-plugin.md new file mode 100644 index 00000000..cd20d82f --- /dev/null +++ b/docs/adr/0006-no-redirect-plugin.md @@ -0,0 +1,20 @@ +# No redirect plugin — merged-page URLs 404 + +**Decision:** no dependency on `mkdocs-redirects` or any redirect plugin. The two URLs orphaned by +the docs-dedupe merges (`testing/fixtures/`, `introduction/that-depends-or-modern-di/`) 404. + +`mkdocs-redirects` 1.2.3 (2026-03-28) is a hostile release: it adds a dependency on `properdocs` — a +MkDocs fork whose code hooks into every build to print scare-marketing urging users to switch to the +fork — and caps `mkdocs<=1.6.1`, fighting this repo's `mkdocs>=1.6,<2` pin. The prior release, +1.2.2 (2024-11-07), is clean, and there has been no clean release since. Two 404s is a small, +contained cost; carrying a supply-chain-compromised dependency to avoid it is not. + +- **Pinning `==1.2.2`** freezes the immediate problem but leaves an untrustworthy upstream in the + chain: any loosened resolution re-admits 1.2.3+, and the pin is a standing note-to-self that has + to survive every future audit. +- **A local mkdocs hook** avoids the dependency but adds build-time code to maintain for two URLs. +- **Committed meta-refresh stub pages** work without a plugin but add permanently maintained files + for a problem that exists only because of the merge. + +**Revisit trigger:** MkDocs gains native redirect support, or `mkdocs-redirects` changes hands / +publishes a clean release dropping the `properdocs` dependency and the `mkdocs<=1.6.1` cap. diff --git a/docs/adr/0007-unify-graph-traversal.md b/docs/adr/0007-unify-graph-traversal.md new file mode 100644 index 00000000..b5bc32f5 --- /dev/null +++ b/docs/adr/0007-unify-graph-traversal.md @@ -0,0 +1,29 @@ +# Extract the shared provider-graph traversal, keep the two cycle policies + +**Decision:** one `DependencyGraph` module owns the provider-graph traversal and cycle extraction; +`validate()`, the runtime `RecursionError` guard, and alias scope-resolution all call it. The two +*policies* stay distinct (collect-all vs first-cycle). + +Before it, both detectors re-implemented the same traversal — the `path[cycle_start:]` slice and +`CircularDependencyError` construction appeared verbatim in each — and `Alias.effective_scope` +hand-rolled a third chain-walk. With the traversal shared, deleting `DependencyGraph` makes +cycle-detection complexity reappear across all four callers: a real seam, not a hypothetical one. +dishka confirms the one-walk-many-concerns model; it can drop its runtime guard only because it +makes validation effectively mandatory. + +Rejected alternatives: + +- **A pure cycle-extraction helper.** Removes the verbatim copy but leaves the DFS structure written + twice; fails the deletion test. +- **Two traversal methods in the module** (recursive walk + iterative find). Relocates the + duplication rather than removing it. +- **Type-checking `Alias` inside `DependencyGraph`** to follow the chain. Reintroduces concrete-type + import coupling; a generic `redirect_target` node hook keeps the module Alias-agnostic. +- **A per-container validated stamp.** A child would not inherit the root's validation; the graph is + shared, so the stamp is registry-level. +- **Folding scope-inversion into reachability (dishka-style).** Simpler, but yields a less precise + error (`missing` vs `inverted`); the dedicated `InvalidScopeDependencyError` is worth more. + +**Revisit trigger:** benchmarks show the `walk()` event-stream indirection measurably slows +`validate()` or the guard. The original second limb — validation becoming mandatory, collapsing the +seam to one caller — is retired: 3.1 went the other way, so the runtime guard is permanent. diff --git a/docs/adr/0008-integration-kit-shape.md b/docs/adr/0008-integration-kit-shape.md new file mode 100644 index 00000000..9920e1d5 --- /dev/null +++ b/docs/adr/0008-integration-kit-shape.md @@ -0,0 +1,29 @@ +# Integration kit is low-level primitives in core, and outliers bypass it + +**Decision:** the shared adapter skeleton lives in a framework-agnostic module inside core, exposing +only low-level primitives; genuine outliers call `build_child_container` directly rather than the +primitives growing parameters to swallow them. + +- **In core, not a new package.** The skeleton imports only stdlib + `modern_di`, so it does not + threaten the zero-dependency stance, and it deepens the `add_providers`/`resolve_dependency` seam + core already blesses. A 14th repo would add coordinated-release cost for agnostic code every + adapter already reaches through its `modern-di` dependency. +- **Low-level primitives only.** A `make_inject` convenience fails the deletion test: the adapters' + wrapper shapes are not identical (each fetches the child differently — request, ASGI scope, `g`, + contextvar), so the convenience would take a `get_container` callable and wrap three primitives — + a shallow module, exactly what the extraction exists to remove. +- **Outliers bypass.** Each absorbing parameter (scope-resolver callable, post-build `set_context` + hook, no-context mode) is needed by exactly one adapter — a hypothetical seam. Adding them taxes + the ten common-case adapters; keeping weird logic in the weird adapter is better locality. + +Reading all 13 adapters concretely narrowed what "bypass" means: only **typer** is a true Layer-1 +bypass (it binds no connection at all). aiohttp and grpc both use `bind(provider, connection)` and +only skip `classify_connection` — aiohttp because both its providers share one type so `isinstance` +cannot dispatch, grpc because it has one provider and no dispatch to do; grpc's `_build_child` +collapses to one `bind()` call, dropping its post-hoc `set_context`. aiogram's context is a +multi-provider merge with a hardcoded scope, a third shape `bind()` does not fit, so it stays a +two-line literal. No primitive grew a parameter for any of them. + +**Revisit trigger:** a third adapter needs the same non-`isinstance` scope-dispatch (making the +absorb-it seam real under the two-adapter rule), or adapter authors request the convenience layer +because the residual `inject` glue proves non-trivial in practice. diff --git a/docs/adr/0009-error-text-is-not-a-contract.md b/docs/adr/0009-error-text-is-not-a-contract.md new file mode 100644 index 00000000..ea2a51f9 --- /dev/null +++ b/docs/adr/0009-error-text-is-not-a-contract.md @@ -0,0 +1,23 @@ +# Rendered error text is not a public contract + +**Decision:** the *rendered* text of a `ModernDIError` is diagnostic output and may change in any +release. The **structured attributes** each error carries (`.provider_type`, `.cycle_path`, +`.suggestions`, `.dependency_path`, …) and the **class hierarchy** callers catch on are the public +contract, and those change only with the usual care. + +The forcing case: unifying the two chain drawers means `CircularDependencyError` renders through the +same path as `DependencyPathMixin`, which prints an aligned scope column. Either the cycle message +gains that column, or the shared drawer carries a `show_scope` flag forever to keep output +byte-identical. Freezing the bytes buys nothing real and costs compounding — every renderer grows a +compatibility flag and the formatting can never improve — so the cycle message gains the column and +the drawer needs no flag. + +The attributes are the other half of the split: a caller who wants to *act* on an error should read +`.cycle_path`, not regex the message. Pre-rendering suggestions into `.suggestions` violated that by +forcing a programmatic consumer to parse glyphs back out; structured `Suggestion` records fix the +contract rather than break it. This licenses a message *improving* without a deprecation cycle, and +it means message-text assertions in tests pin an implementation detail, not a promise. + +**Revisit trigger:** a downstream consumer — an integration, or a user in an issue — is found +parsing `str(exc)` to recover structured facts. That means the attribute surface is missing +something: add the attribute, and keep this decision. diff --git a/docs/adr/0010-grpc-registry-introspection-declined.md b/docs/adr/0010-grpc-registry-introspection-declined.md new file mode 100644 index 00000000..30d812f1 --- /dev/null +++ b/docs/adr/0010-grpc-registry-introspection-declined.md @@ -0,0 +1,20 @@ +# No blessed provider-introspection seam for grpc's registry drill + +**Decision:** no `Container.is_registered(type)` and no idempotent-`add_providers` mode for adapters +to query registration state. `modern-di-grpc` keeps its local `_ensure_context_provider` guard, +which checks `find_provider(ServicerContext) is None` before registering — grpc has no `setup_di` +(constructing an interceptor *is* the setup) and both interceptors may be built on one container, so +the guard prevents a `DuplicateProviderTypeError`. + +The deciding evidence: **grpc is the only consumer.** A grep across every `modern-di-*` adapter and +its tests found no other reader of `providers_registry` / `find_provider`, and by the standing rule +(one adapter is a hypothetical seam, two is a real one — the same principle that kept the +[integration-kit](0008-integration-kit-shape.md) outliers local) a single consumer does not justify +new core API. Reinforcing it: `container.providers_registry` and `find_provider` are both public, so +grpc is using a lower-level public API rather than breaching encapsulation; and `add_providers`' +strictness is a deliberate feature catching accidental double-registration, which an +`ignore_existing` mode would loosen globally to serve one adapter. + +**Revisit trigger:** a **second** adapter needs to query registration state, or a decision to +privatize `container.providers_registry` — at which point a blessed `is_registered` becomes grpc's +migration path. diff --git a/docs/adr/0011-fold-context-registry-declined.md b/docs/adr/0011-fold-context-registry-declined.md new file mode 100644 index 00000000..4b39357c --- /dev/null +++ b/docs/adr/0011-fold-context-registry-declined.md @@ -0,0 +1,22 @@ +# Keep `ContextRegistry` as its own module + +**Decision:** `modern_di/registries/context_registry.py` is not folded into `Container`. +`ContextRegistry` stays a named registry and `ContextProvider` keeps reading it via +`container.context_registry.find_context(...)`. + +It is the shallowest of the four registries — ~18 lines, a `dict[type, Any]` behind `find_context` +and `set_context` — and the deletion test on the *code* passes: fold it, and `Container` gains a +`self._context` dict plus a `find_context` method for the two touch points. What the deletion test +misses is that the conceptual slot does not vanish. The four registries are organised by a real +axis, stated in `CLAUDE.md`'s registries entry: shared tree-wide (`providers_registry`, +`overrides_registry`) versus per-container (`cache_registry`, `context_registry`). `ContextRegistry` +sits symmetric with `CacheRegistry`; its shallowness in line count reflects having less mechanism, +not a broken abstraction. Folding trades a uniform 2×2 model for ~18 fewer lines and grows the +already-largest file, with zero actual friction: no bug hides in the one-line delegation, and +context is not a change hot-path. A predictable four-registry pattern navigates better than one with +an exception. + +**Revisit trigger:** the four-registry model is restructured — any registry folded, or the +shared-vs-per-container framing abandoned — since the symmetry is the load-bearing reason; or +concrete friction emerges, a bug in the `Container` → `ContextRegistry` delegation, or the +indirection repeatedly obstructing context-related work. diff --git a/docs/adr/0012-provider-facing-seam-declined.md b/docs/adr/0012-provider-facing-seam-declined.md new file mode 100644 index 00000000..592ce9dc --- /dev/null +++ b/docs/adr/0012-provider-facing-seam-declined.md @@ -0,0 +1,28 @@ +# No provider-facing seam on `Container` + +**Decision:** no `ResolutionContext` view handed to providers, and no promotion of `Container`'s +provider-facing members into a declared interface. Resolution runs against the `Container` directly +and the existing `# noqa: SLF001` reaches stay. + +The deciding evidence: **there is exactly one `Container` implementation.** Everything that resolves +does so against the same collaborator, so a `ResolutionContext` would be one interface with one +implementation — a hypothetical seam under the standing one-adapter/two-adapter rule, the same +reasoning that declined [the grpc introspection seam](0010-grpc-registry-introspection-declined.md). + +The crossings it would formalize have since gone to zero. At decision time there were three +(`Factory._lock`, plus `_warn_and_reopen_if_closed` on `Factory` and `ContextProvider`); the +single-path compiled resolver dissolved `Factory.resolve` and `Alias.resolve`, moving the lock and +closed-state reaches into `resolver_compiler.py` — the compiler's business, ruled on in +[the per-provider compile seam](0014-per-provider-compile-seam-declined.md). The last one, +`ContextProvider.fetch_context_value` calling `container._prepare()`, has had no production caller +since #425. Option (a) would also have changed `AbstractProvider.resolve(container)` — then the +documented public extension contract — to `resolve(ctx)`: a large blast radius to formalize a +boundary only core code crosses, where `docs/providers/advanced-api.md` already declares which +members are supported (`find_container`) and which are internal (`_lock`, `_scope_map`, +`parent_container`). + +**Revisit trigger:** a **second `Container` implementation** appears, making `ResolutionContext` a +real two-adapter seam. The original second limb — a custom-provider author blocked on "do not build +on" internals — is retired: [the provider set is closed](0013-custom-providers-retracted.md), so +that author does not exist. If the set reopens, the migration path is the polymorphic `compile()` +hook, not a provider-facing view of `Container`. diff --git a/docs/adr/0013-custom-providers-retracted.md b/docs/adr/0013-custom-providers-retracted.md new file mode 100644 index 00000000..9c2cc0d8 --- /dev/null +++ b/docs/adr/0013-custom-providers-retracted.md @@ -0,0 +1,33 @@ +# Custom providers are not an extension point; the provider set is closed + +**Decision:** `modern-di` supports exactly four provider types — `Factory`, `Alias`, +`ContextProvider`, and the pre-built `container_provider`. Subclassing `AbstractProvider` (or +`Factory`) to add a provider type is **not** supported; the `docs/providers/advanced-api.md` section +promising it was retracted rather than honoured. + +Custom provider support was never designed. Until 2.28.0, `resolve_provider` ended in +`provider.resolve(self)`, so any subclass implementing `resolve()` worked — an emergent property of +Python inheritance that the docs then wrote down. The single-path compiled resolver (#334) replaced +that with `compile_resolver`, which selects a closure by exact type identity and raises otherwise, +and deleted `AbstractProvider.resolve` and `Alias.resolve`, so no hook survives to fall through to. +The closure was deliberate — asserted by +`tests/test_container.py::test_resolve_provider_raises_for_unhandled_provider_type`. Only the docs +were left behind. + +The blast radius and failure mode: `type(x) is Factory` is identity, not `isinstance`, so a +`LoggingFactory(Factory)` that overrides nothing fails exactly like a from-scratch provider; +`validate()` cannot catch it, since compilation is lazy, so a container reports clean and then +raises `TypeError` at first resolve under traffic. Against that: **zero consumers** — all 13 sibling +`modern-di-*` repos, the two templates, and `lite-bootstrap` contain no `AbstractProvider` subclass. + +Rejected: a 2.x fallback behind a `DeprecationWarning`, matching the `ContainerClosedWarning` / +`ContextValueNoneWarning` / `UnvalidatedContainerWarning` ramps. Those guard capabilities users +demonstrably rely on; this one would guard an audience believed empty, at the cost of resurrecting +the exact indirection #334 removed and carrying it through the 2.x line. The residual risk is +accepted knowingly, and the 2.29.0 notes carry it under a breaking-change heading. Also rejected: +holding the whole post-2.28.0 backlog for an unscoped 3.0. + +**Revisit trigger:** a real user reports a broken custom provider or `Factory` subclass, falsifying +the zero-audience premise. The migration path is then the polymorphic `compile()` hook named in +[the per-provider compile seam](0014-per-provider-compile-seam-declined.md), not a restored +interpreted fallback. diff --git a/docs/adr/0014-per-provider-compile-seam-declined.md b/docs/adr/0014-per-provider-compile-seam-declined.md new file mode 100644 index 00000000..202c674f --- /dev/null +++ b/docs/adr/0014-per-provider-compile-seam-declined.md @@ -0,0 +1,30 @@ +# No per-provider `compile()` seam + +**Decision:** `resolver_compiler`'s per-type closure builders stay where they are; they do not move +into a `provider.compile(registry) -> resolver` method on each provider class. The +`compile_resolver` type-dispatch and its `# noqa: SLF001` reaches into `Factory`/`Alias` privates +stay. + +The deciding evidence mirrors [the provider-facing seam](0012-provider-facing-seam-declined.md): +**there is exactly one compiler.** `resolver_compiler` is the sole consumer of those provider +privates and nothing varies across the proposed seam, so `compile()` is cleanliness, not a swap +point. Reinforcing it: + +- **The ~16 `SLF001` reaches are intra-package intimacy, not a leaked abstraction.** The compiler + co-evolves with the classes it compiles; they ship and change together, and the markers are honest + labels on a deliberate friendship. +- **The provider-type set is closed and tiny.** Polymorphic dispatch buys extensibility for types + that are essentially never added; an `if type() is` chain over four types is not worse. +- **Concentration is a deliberate property.** Every perf-critical closure lives in one file, + reviewed together, sharing the positional/kwargs and two-phase-error patterns. The full seam + sacrifices that; the middle form (a `compile()` that extracts fields for shared flat builders) + preserves it only by adding an indirection that earns nothing while the seam stays hypothetical. + +The one genuine defect was doc-rot: three docstrings and two inline comments named interpreted +methods that no longer existed. Those were rewritten to describe the behaviour directly, which is +the whole of the fix. + +**Revisit trigger:** a **second consumer of provider compile-time privates** appears (a distinct +compiler, an alternate resolver backend), or provider types become an open, user-extended set where +adding one must not require editing a central dispatch — at which point the polymorphic `compile()` +hook becomes the migration path. diff --git a/docs/adr/0015-warm-singleton-memo-swap-dropped.md b/docs/adr/0015-warm-singleton-memo-swap-dropped.md new file mode 100644 index 00000000..17b3d8c9 --- /dev/null +++ b/docs/adr/0015-warm-singleton-memo-swap-dropped.md @@ -0,0 +1,37 @@ +# Drop the warm-singleton resolver memo-swap + +**Decision:** the resolver memo is never self-modified after a singleton is built; a warm +singleton hit keeps paying the normal compiled-resolver path. The mechanism was built in full, +measured, and reverted. + +The lever it was aimed at: after the single-path compiled resolver (#334), the warm singleton hit +was 292 ns, against dishka ~245 ns, wireup ~98 ns, that-depends ~85 ns, and dependency-injector +~61 ns — modern-di paid `resolver_for` dispatch, the override front-guard, and `fetch_cache_item` on +every warm hit. + +wireup's technique — swap a cached provider's stored resolver for a bare `return value` closure +once the value exists — is sound here only for **APP** scope, where one registry ↔ one APP +container ↔ one tree-wide value holds; deeper scopes cache per child container, so a +registry-level constant would be wrong. It was run as a measurement-gated spike with a +pre-committed gate: ship iff the warm hit reached ≤ ~146 ns (at least halved) *and* beat dishka, +with zero correctness regression and a green free-threaded stress test. + +**Measured** (best-of-3, stable medians, machine-relative): guard g2 warm hit 584 → 375 ns; +comparative C2 333 → 208 ns. A consistent **~1.6x**, enough to pass dishka, short of the +≤146 ns gate. The shortfall is structural: `resolve_provider`'s dispatch floor — the +closed-container shim frame, the `resolver_for` lookup, the version check — sits upstream of the +closure body the swap replaces, so "near-free" was never architecturally reachable this way. A +~1.6x win does not buy a permanent cross-cutting invalidation invariant: a bypass resolver +spanning `override()` / `close()` / `add_providers()`, plus a second source of truth in +`_warm_swapped`. + +Two things from the attempt were kept: the free-threaded stress test surfaced that +close-during-resolve was not tear-free, and the research it triggered is now stated in +[design decisions](../introduction/design-decisions.md#the-thread-safety-boundary); and it pointed +at the dispatch-floor simplification (invalidate-on-mutation instead of a per-resolve version +stamp), which *removes* per-resolve work and shipped in #347. + +**Revisit trigger:** a user-reported warm-singleton bottleneck **plus** a design that removes the +dispatch floor itself — the swap alone provably cannot clear the bar, so re-proposing it unchanged +is settled. This record governs the memo-swap technique only; a that-depends-style +per-APP-container slot array was not pre-authorized here and would need its own measurement. diff --git a/docs/adr/0016-child-lazy-alloc-declined.md b/docs/adr/0016-child-lazy-alloc-declined.md new file mode 100644 index 00000000..b266a2bf --- /dev/null +++ b/docs/adr/0016-child-lazy-alloc-declined.md @@ -0,0 +1,34 @@ +# Decline lazy-allocation of child-container registries + +**Decision:** `Container.__init__` keeps eagerly building the per-child `RLock`, `CacheRegistry`, +and `ContextRegistry`. They are not lazy-allocated. + +After the `_next_deeper` memo (#348) took ~40% off default child-build, the remaining per-child cost +was three eager allocations: `RLock` (~195-214 ns), `CacheRegistry` (~217 ns), `ContextRegistry` +(~189 ns). + +**Measured** ceiling (`use_lock=True` vs `False`, which already skips the RLock alloc; py3.10, +guidance), starting from the strongest candidate — the `RLock`, since REQUEST children rarely create +singletons: + +- Isolated child build: RLock alloc ≈ **195 ns/child**. +- **Realistic caching request cycle** (build child → resolve a REQUEST-cached resource → close, the + C4/G7 shape): saving ≈ **0 (0.4%)** — a caching child *uses* the lock, so lazy only defers the + allocation and adds a `None`-check. +- Narrow **no-cache child** (transient/APP deps only): saving ≈ **67 ns (3.5%)**, and that is the + ceiling before any cost. + +Real integration request children inject context and cache a request-scoped resource — the C4/G7/G9 +scenarios all do — so the trio is used and lazy-allocation saves nothing there while taxing the hot +path. Against a 0-to-3.5% narrow win it costs a `None`-check on the cached-resolve hot path plus a +`_use_lock` slot, and **re-introduces the singleton-creation race the lock exists to prevent**: +lazy lock creation must itself be atomic, so it needs a guard lock or a CAS-style publish, a new +concurrency-correctness surface against the documented Beta contract in +[design decisions](../introduction/design-decisions.md#the-thread-safety-boundary). The +`CacheRegistry` / `ContextRegistry` variants are weaker still — used more often in realistic +children, so they save even less. + +**Revisit trigger:** a profile of a *realistic* request cycle (context + caching) showing these +allocations — not `_next_deeper` — dominating, or a user-reported per-request construction +bottleneck in a build-heavy, cache-free workload. Re-measure the net against `G6b` + `G1-G3` + +`C4/G7/G9`, and solve the lazy-lock atomicity, before reopening. diff --git a/docs/adr/0017-exec-hot-path-declined.md b/docs/adr/0017-exec-hot-path-declined.md new file mode 100644 index 00000000..c4caff8f --- /dev/null +++ b/docs/adr/0017-exec-hot-path-declined.md @@ -0,0 +1,29 @@ +# Re-decline `exec` codegen on the resolve hot path + +**Decision:** the shipped closure-compiled resolver stays the single resolve path. No `exec`-based +source-generation codegen, additive or otherwise. + +The reframe that reopened this holds: `exec` is a stdlib builtin, so `dataclasses`/`attrs`-style +codegen would not touch the zero-*dependency* guarantee — "it adds a dependency" was never the real +objection. Unbundled, four claims remain: + +- **Debuggability** — mitigable, but only via the attrs `linecache` discipline (script-builder, + hygiene rules, unique-filename scheme). +- **Maintainability / audit trust** — real, no neutralizer; a fixed standing cost and a second + mental model, independent of how small the win is. +- **Free-threading / nogil** — real, open, and modern-di-specific: it swaps captured cells for + generated-module globals under a concurrency contract still at Beta, and cannot be retired without + out-of-scope parallel-resolution work. +- **Deployment / `exec` bans** — mitigable via an additive fallback resolver, but that doubles the + resolve surface and deepens the maintainability cost rather than escaping it. + +**Measured**, the prize is bounded before any of that: `exec` is 0-4% faster than a hand-unrolled +closure at fixed arity (inside the noise band), with its only exclusive win — **~1.3-1.9x** — +confined to high-arity nodes and deep singleton/scoped chains, where closures already capture +~80-90% of the ceiling. Every path that neutralizes an objection pays for it in the maintainability +row, and dissolving the dependency-purity framing manufactures no win the measurement denies. + +**Revisit trigger:** a user-reported, real-world resolve bottleneck on a high-arity node or a deep +singleton/scoped chain — the two forms where `exec` could pay — that the closure resolver provably +cannot close. A synthetic micro-benchmark or a hypothetical does not qualify. This is the +codegen-ceiling half of the open warm-singleton perf-headroom question. diff --git a/docs/adr/0018-no-static-wiring-checker.md b/docs/adr/0018-no-static-wiring-checker.md new file mode 100644 index 00000000..552acbf1 --- /dev/null +++ b/docs/adr/0018-no-static-wiring-checker.md @@ -0,0 +1,29 @@ +# No static / compile-time wiring checker + +**Decision:** modern-di ships no static or compile-time dependency-graph checker and no type-checker +plugin (mypy, pyright, or `ty`). Whole-graph verification stays the opt-in runtime `validate()`, +backed by declaration-time signature parsing that already fails early on an unwireable creator. + +Three verified findings decide it: + +1. **True compile-time wiring verification exists only in compiled-language toolchains** — Dagger's + annotation processor, Google Wire's build-time codegen, Koin's K2 compiler plugin (GA June 2026). + Angular's "no provider" (NG0201) is a runtime error, .NET's scope validation is runtime/startup, + Spring's autowiring correctness is an IDE inspection. Runtime/startup validation is the + mainstream field standard, not a second-class fallback. +2. **Where compile-time validation exists, it *replaces* runtime verification rather than extending + it** — Koin's own docs tell users to delete their `verify()`/`checkModules()` tests once the + plugin is on. A static layer here would duplicate `validate()`, not reach past it. +3. **A Python type-checker plugin is infeasible for a conservative zero-dep library.** pyright + refuses third-party plugins on principle; `ty` — the checker modern-di itself uses — has no + plugin system (astral-sh/ty#291 closed "not planned"); only mypy exposes a plugin API, documented + as experimental with backwards-incompatible changes shipped without a deprecation period. + +So a checker would duplicate `validate()`, serve mypy users only, carry a permanent liability +against an unstable API, and not even help modern-di's own `ty` toolchain. The one in-constraint win +the research pointed at — injection markers that type-check to the concrete `T` — already ships: +`resolve(type[T]) -> T` and `Annotated[T, from_di(dep)]` both preserve the concrete static type. + +**Revisit trigger:** `ty` (or pyright) ships a **stable, supported** third-party plugin API **and** a +concrete user-reported wiring-safety need that runtime `validate()` demonstrably cannot meet (e.g. +per-call-site checking without executing `validate()`). Both conditions, not either alone. diff --git a/docs/adr/0019-except-body-creator-error-helper.md b/docs/adr/0019-except-body-creator-error-helper.md new file mode 100644 index 00000000..f7f4150a --- /dev/null +++ b/docs/adr/0019-except-body-creator-error-helper.md @@ -0,0 +1,25 @@ +# Extract the creator-call error rule via an except-body-only helper + +**Decision:** the creator-call `TypeError` rule lives in one `CreatorCallError.from_type_error` +classmethod, called from inside each site's `except TypeError` block. This supersedes the earlier +drift-lock work's "do not extract a shared helper" non-goal, which locked four copies of the rule +with a cross-path equivalence test instead. + +That rejection weighed exactly one helper shape: a helper wrapping the whole `creator(...)` call, +which adds a Python frame on **every** resolve — the success path the single-path compiled resolver +(#334) exists to keep frame-free. It did not weigh extracting only the `except` body (the `tb_next` +discriminate, the `CreatorCallError` construction, the `prepend_step`) while leaving +`try: return creator(...)` at each site, which runs only on the already-failing raise path. Under +that form: + +- The hot path stays `return creator(*args)` byte-for-byte — no frame is restored, confirmed before + ship by a `--benchmark-compare-fail=mean:5%` resolve-bench gate. +- The rule gets one home; changing it is one edit, not four. +- The equivalence test that existed only to police the copies is retired — one source cannot drift + from itself. +- Traceback fidelity is preserved: the return-or-`None` contract keeps the bare `raise` at each + site, so a creator-body `TypeError` propagates with its traceback unchanged. + +**Revisit trigger:** the resolve hot path regresses after this lands (meaning the success path was +not as frame-free as argued), **or** a future change needs the creator-call rule to differ per site +again, making a single shared rule wrong. diff --git a/docs/adr/0020-d3-root-lifecycle-inherent.md b/docs/adr/0020-d3-root-lifecycle-inherent.md new file mode 100644 index 00000000..4dbf6d6d --- /dev/null +++ b/docs/adr/0020-d3-root-lifecycle-inherent.md @@ -0,0 +1,53 @@ +# D3 root-lifecycle gaps are inherent — no integration code changes + +**Decision:** the eight integrations whose `setup_di` does not own both the root open/close and the +per-unit-of-work child keep their current lifecycle handling. The gaps are inherent framework limits +plus the deliberate caller-owns-root contract, so the treatment is this rationale, not code. + +| Integration | Root open/close | Why inherent | +|---|---|---| +| fastapi | `setup_di` owns both | ASGI lifespan is optional — a mounted sub-app / `lifespan="off"` never fires it | +| starlette | `setup_di` owns both | Same ASGI-lifespan-optional caveat | +| faststream | `setup_di` owns both | `TestBroker`/`TestApp` deliberately skip `on_startup` | +| taskiq | `setup_di` owns both | `run_receiver_task(run_startup=False)` skips the startup hook by default | +| celery | root owned; per-task child owned by `@inject`/`DITask` | `task_always_eager` bypasses the worker signals that open the root | +| flask | child owned; root is the caller's | Flask has **no app-shutdown hook**, so the root *close* is unavoidably the caller's | +| grpc | per-RPC child owned; root is the caller's | `start()`/`stop()` is caller-owned; the integration's seam is the interceptor, not the server lifecycle | +| typer | neither owned by `setup_di` | A Click callback hook *does* exist — the one fixable case, see below | + +**The first five have nothing to fix.** `setup_di` already owns both sides; each falls short only +because of a documented execution-context caveat, and every caveat is real framework behaviour. No +code closes a caveat the framework itself imposes; they are captured in the +[lifecycle rules](../integrations/writing-integrations.md#lifecycle-rules) and in each integration +page's deployment caveats. + +**flask and grpc give the root to the caller by design.** Owning the root open in Flask's +`setup_di`, or adding a gRPC server-wrapper helper, would add machinery and revisit a contract the +lifecycle rules state deliberately — *if the framework offers no lifecycle hook at all, the root's +open/close is the caller's to own; document it.* Removing one `open()`/`with` line the caller writes +once does not justify new API surface. + +**typer is the one fixable case, deferred.** A Typer/Click callback could open the root and close it +via `ctx.call_on_close`, but the command child *is* already owned inside `@inject`, an explicit +`with container:` is the right idiom for a process that exits in milliseconds, and an +integration-injected callback adds hidden control flow that must compose with a user's own +`@app.callback()` — non-trivial in Click. + +**One-call-setup scores follow from this, not from anything separate.** Where a second wiring action +is required (flask, grpc, typer), that action *is* the manual root `open()` ruled inherent above — +there is no independent setup fix. Under these rulings the ceiling for integrations that own their +whole lifecycle is four: litestar, aiogram, aiohttp, arq. The other eight are each gated by a +framework-inherent root-lifecycle limit, with the revisit trigger below. + +**Amendment (2026-07-26).** The trigger fired: maintainer-reported root-lifecycle friction — the +hard `ContainerClosedError` failure mode every caveat here relies on — was addressed by making +`open()` optional in core (3.1: a root is open from construction, and reuse after an explicit close +warns and reopens). That landed in `modern_di.Container`, not in any integration's wiring, so the +conclusion stands: every caveat changes failure mode (a hard raise becomes "finalizers silently do +not run") rather than disappearing, the deployment notes were reworded, and no integration's +lifecycle code changed. + +**Revisit trigger:** a real user reporting friction with a specific integration's root-lifecycle +ergonomics — most plausibly typer, where the callback fix would then be worth its composition cost. +Also: a flask/grpc-shaped framework gaining a startup/shutdown hook it currently lacks, at which +point its `setup_di` should own the root and its row reopens. diff --git a/docs/adr/0021-inject-asymmetry-inherent.md b/docs/adr/0021-inject-asymmetry-inherent.md new file mode 100644 index 00000000..1ac643ac --- /dev/null +++ b/docs/adr/0021-inject-asymmetry-inherent.md @@ -0,0 +1,49 @@ +# The @inject asymmetry is inherent — do not unify + +**Decision:** the four integrations that resolve `FromDI` decorator-free (fastapi, litestar, +faststream, taskiq) and the eight that require `@inject` (flask, starlette, aiohttp, celery, arq, +aiogram, typer, grpc) keep their current shapes. An adapter can drop `@inject` only where the host +framework evaluates a parameter *default* as a provider, and the eight offer no such seam, so there +is nothing to unify. + +| Integration | Per-parameter provider seam | Verdict | +|---|---|---| +| fastapi | `fastapi.Depends` | decorator-free | +| litestar | `Provide` | decorator-free | +| faststream | `faststream.Depends` | decorator-free | +| taskiq | `TaskiqDepends` | decorator-free | +| flask | none — view is a plain callable | inherent | +| starlette | none — endpoint is a plain ASGI callable | inherent | +| aiohttp | none — handler is `async def handler(request)` | inherent | +| celery | none — task is a plain callable with its own args | inherent | +| arq | none — `coroutine(ctx, …)`, `ctx` a plain dict | inherent | +| aiogram | name-based `data` injection, **not** provider-evaluation (closest call) | inherent | +| typer | none — defaults are CLI parsing (`Option`/`Argument`) | inherent | +| grpc | none — fixed `(request, context)` servicer signature | inherent | + +**aiogram is the one close call.** Its middleware `data` dict is matched to handler kwargs by +parameter *name* and never evaluates a default as a provider, so it cannot consume a `FromDI` +marker; the adapter uses it only to pass the child container. + +**Positioning follows.** The defensible claim is *no `@provide` ever, and no `@inject` in the four +biggest integrations* (where dishka needs `@inject` even for FastAPI/Litestar), not "decorator-free" +unqualified, which a single `grep` refutes. The adapter-side `auto_inject` (Flask) and `DITask` +(Celery) helpers apply `@inject` under the hood for convenience; they are not framework seams. + +**Quickstart length follows from this, not from anything separate.** The decorator-free floor for a +minimal single-dependency example is 7 DI-specific lines (two imports, a `Group` with one provider +and its dependency, `Container(...)`, `setup_di`), and a minimal example needs both providers to +demonstrate DI at all, so the floor cannot drop. Against the merged examples: aiogram, aiohttp and +arq are at 8 — the floor plus the `@inject` line; flask and typer at 9 — plus the manual root +`open()`/`with` ruled inherent in [the root-lifecycle record](0020-d3-root-lifecycle-inherent.md); +grpc at 10, plus `close_sync()`. Nothing is trimmable without deleting an inherent element, so there +is no independent quickstart fix. + +Same call as [the D3 root-lifecycle gaps](0020-d3-root-lifecycle-inherent.md) and +[the exec hot-path re-decline](0017-exec-hot-path-declined.md): where a gap reflects a framework +limitation rather than a modern-di shortfall, document the stance instead of adding machinery. + +**Revisit trigger:** an `@inject`-requiring framework gains a per-parameter dependency hook (a +future Flask/Starlette DI feature) — its integration should then bind `FromDI` to that hook and drop +`@inject`, reopening its row. Or a user reports the `@inject` requirement as real adoption friction +in a specific integration. diff --git a/docs/adr/0022-explicit-only-validation.md b/docs/adr/0022-explicit-only-validation.md new file mode 100644 index 00000000..fc4d6137 --- /dev/null +++ b/docs/adr/0022-explicit-only-validation.md @@ -0,0 +1,40 @@ +# Validation is explicit-only; implicit validation was built and discarded + +**Decision:** `container.validate()` is the only thing that walks the graph. Neither `__init__` nor +`open()` nor `add_providers` nor `resolve()` ever validates, and `Container(validate=...)` is an +accepted-and-ignored no-op that raises `ValidateArgumentWarning` until 4.0 — 3.0 callers pass +`validate=False` widely, including this repo's own benchmark guards. Shipped as 3.1.0. + +3.0 made `open()` mandatory and the sole validation trigger. Both tightenings caused trouble: the +mandatory open produced six production defects across integrations, all one root cause (the root's +open hook does not fire in some execution contexts, so the first unit of work raises); and binding +validation to `open()` produced an authoring rule that existed only because of that binding — open +the root *after* `setup_di`, or a by-type dependency on a not-yet-registered connection fails. + +The alternative that kept validation implicit was implemented and worked: split the walk, checking +cycles and inverted scopes eagerly at construction (they are *monotone* — more providers can only +add such an error), and holding completeness on the shared registry to raise at first use (the only +class a later `add_providers` can legitimately fix). It was discarded for the machinery it dragged +in: a two-flag container lifecycle, validation state parked on `ProvidersRegistry`, a +monotone/completeness classification threaded through the walk, and an `add_providers` rollback +path. That is a large permanent surface for a startup-time property, and the cheap way to keep the +guarantee without it — a per-resolve check — taxes the hot path for a concern that matters once, at +boot. + +`add_providers` is now a plain register with no rollback; the mutation clears `_validated`, which +`ProvidersRegistry` keeps purely as a memo of a clean walk — it gates nothing, but still +short-circuits the `RecursionError`-to-`CircularDependencyError` guard. + +**Measured:** because 3.0 ran a default `validate=True` walk at `open()`, dropping it made +construction markedly cheaper — roughly **2.6 µs against 15.5 µs** for a depth-6 chain +(`Container(...)` + `open()`, default arguments), matching the `test_g10_validate_deep_chain` guard +cost 3.0's `open()` paid. The resolve tier was unchanged, as expected. + +**The accepted cost:** the default safety posture drops silently. A broken graph previously raised at +`open()`; now it surfaces from an explicit `validate()`, or at resolve time as +`ArgumentResolutionError`. + +**Revisit trigger:** reports of graphs reaching production broken in a way an implicit walk would +have caught at boot — evidence that opt-in `validate()` is under-adopted. Reopen with the adoption +evidence, not with a new mechanism: any replacement must avoid both the four-part machinery above +and a per-resolve check. diff --git a/docs/adr/0023-debug-resolution-tracing-declined.md b/docs/adr/0023-debug-resolution-tracing-declined.md new file mode 100644 index 00000000..b902037b --- /dev/null +++ b/docs/adr/0023-debug-resolution-tracing-declined.md @@ -0,0 +1,50 @@ +# Decline opt-in DEBUG resolution tracing + +**Decision:** no module-level `logging.getLogger("modern_di")` narrating resolution at DEBUG level. +No resolution tracing ships in any form — neither the runtime-guarded logger nor the +compile-time-gated variant that would avoid its cost. + +Field precedent was real (Uber Fx narrates lifecycle events, Koin exposes an opt-in +`logger(Level.DEBUG)`), and a pluggable structured event-logger subsystem had already been rejected +on the conservative-feature-set principle; the shape that survived was stdlib logging and nothing +else. It rested on one never-measured estimate: "one `isEnabledFor(DEBUG)` boolean per chokepoint." + +The guard is not a boolean. `logger.isEnabledFor(DEBUG)` is an attribute load plus a dict lookup +inside a `try`, measuring **~19 ns net** (21.2 against a 2.25 ns loop floor) — roughly **10x** a bare +module-global bool check (~1.8 ns net). Against a per-node budget of ~120-140 ns, one guard is ~15% +of a node, and a cached factory needs two. Measured by patching the shipped closures with exactly +the proposed design and re-running the guard tier **with tracing off** — the cost every user pays +for a feature they never enable: + +| Scenario | base | traced | delta | +|---|---|---|---| +| G2 cached resolve (warm hit) | 140 ns | 192 ns | **+37%** | +| G16 by-type resolve | 181 ns | 237 ns | **+31%** | +| G4 wide, 10 siblings | 1333 ns | 1709 ns | **+28%** | +| G17 by-type, 200-provider registry | 188 ns | 235 ns | +26% | +| G12 override active, depth 6 | 1017 ns | 1187 ns | +17% | +| G3 deep chain, depth 6 | 833 ns | 958 ns | +15% | +| G9 context resolve | 625 ns | 708 ns | +13% | +| G1 transient | 333 ns | 375 ns | +13% | +| G5 cross-scope | 375 ns | 417 ns | +11% | + +It lands where it hurts most: hardest on the **warm cached hit**, the cheapest operation and the one +the singleton pattern makes most common, and it multiplies by graph size, since every node runs its +own guards — G4's +376 ns is 11 nodes each paying. + +A compile-time gate would have been free (resolvers are memoized closures and `_invalidate()` +already exists to drop them), and was declined on the feature-set principle rather than on cost: +activation becomes an explicit modern-di call that invalidates the resolver memo, so the feature +stops being "stdlib logging" — the one property that justified this shape over the event subsystem +already rejected — and becomes a second public activation API plus a compile mode to keep correct +forever. Diagnostics remain the job of the error messages, which already carry the resolution +breadcrumb chain (see `docs/troubleshooting/`) at zero hot-path cost. + +**Revisit trigger:** a user-reported diagnostic dead end the existing breadcrumb chain provably +cannot answer — a real issue where reporter and maintainer both failed to determine *why* the +container resolved as it did from the error alone. A preference for narration over breadcrumbs does +not qualify. + +*Measured on Python 3.14.6, Apple M4 (`perf_counter` resolution 41.67 ns). Guard-tier medians are +quantized to one timer tick, so a single delta carries that granularity; direction and magnitude +held across all nine scenarios and a repeat run.* diff --git a/docs/adr/0024-scope-map-inline-declined.md b/docs/adr/0024-scope-map-inline-declined.md new file mode 100644 index 00000000..06967024 --- /dev/null +++ b/docs/adr/0024-scope-map-inline-declined.md @@ -0,0 +1,33 @@ +# Declined: inlining `_scope_map` at the resolver navigation sites + +**Decision:** the compiled resolvers keep calling `_navigate` → `Container.find_container` for a +cross-scope hop. The `_scope_map` lookup is not inlined into the closures. + +Four independent lenses proposed replacing the ternary at the four navigation sites with an inlined +`container._scope_map.get(scope)`, falling back to `_navigate` on a miss — the same +hand-inlined-memo-hit pattern already used for `resolver_for` and `fetch_cache_item`. **Measured** +cross-scope resolve 185.4 → 140.7 ns (**-24%**), with a flat same-scope control. Reproduced, and +declined on the invariant rather than the number. + +**The argument audits a function body while the code being changed is a dispatch.** `find_container` +is a public method on a subclassable class, and `Container.__init__` builds children via +`self.__class__`, so a subclass is carried down the whole tree. Inlining the hit path means a +subclass that overrides `find_container` is **silently bypassed** — its override runs on the miss +path only; `unittest.mock.patch.object` shows calls recorded on the override going from +`['APP', 'APP']` to `[]`. + +**The consequence is worse than a missed hook.** The container returned by navigation is the one +whose `cache_registry` receives the singleton, so bypassing an override that redirects navigation +relocates *cached-instance ownership* — a different container's `close_async()` then runs that +instance's finalizer. That is a lifecycle bug, and nothing in the suite would catch it. It also +contradicts a standing decision: `find_container` is a blessed extension point that +[the provider-facing seam decline](0012-provider-facing-seam-declined.md) rests on, and demoting it +should be argued on its own terms, not absorbed as a side effect of an optimisation. + +It also failed the gates as submitted: coverage 99% (two unreachable lines where the fallback never +fires) and `lint-ci` red, at +20 lines with none deleted, six new rules for a maintainer to hold, +and `ContextProvider` still calling `find_container` — two navigation conventions in one codebase. + +**Revisit trigger:** `find_container` stops being an extension point — an explicit decision that +`Container` subclasses may not redirect navigation, with the lifecycle consequence above stated and +accepted. Then this becomes a plain inlining and the measured 24% is available. diff --git a/docs/adr/0025-alias-binds-nothing.md b/docs/adr/0025-alias-binds-nothing.md new file mode 100644 index 00000000..ce7803bd --- /dev/null +++ b/docs/adr/0025-alias-binds-nothing.md @@ -0,0 +1,44 @@ +# The alias hop inlines, but binds nothing + +**Decision:** `_compile_alias`'s closure reads `container.providers_registry` per resolve and inlines +both the source lookup and the source's resolver-memo read. It holds no reference to the source, its +resolver, or the registry. Inline-only ships at **~322 → ~252 ns (-22%)** and 4 frames → 1, giving up +roughly a third of the available win: an eager bind (resolve the source at compile time, close over +its resolver) measured **305.5 → 192.0 ns (-36%)**, reproduced by two independent verifiers, and a +lazy bind behind a `bound is None` branch has the same steady-state cost. + +**A bind buys an invalidation invariant; the inline buys none.** Both bind variants are sound only +because `ProvidersRegistry._invalidate()` clears `_resolvers`, so a stale binding dies with the +closure holding it. True today, but a *second* place the invariant has to hold — stated, defended, +and re-checked by anyone who later touches memo publication. The inline re-reads the live registry +and cannot go stale by construction. Same reasoning that dropped the +[warm-singleton memo swap](0015-warm-singleton-memo-swap-dropped.md): a bounded win does not buy a +permanent cross-cutting invariant. + +**Eager bind additionally escapes the override front-guard**, compiling the alias's whole source +subtree even when the alias is overridden and the source is never touched — the `modern-di-pytest` +mock pattern. `len(_resolvers)` after resolving an overridden alias goes from 1 to 1+depth (11 at +depth 10), cold cost +404%; it also raises `TypeError` eagerly for a source type `compile_resolver` +does not know, and drops the maximum pure alias chain from 494 to 329 hops. Lazy bind avoids all of +this; only the invariant argument rules it out. + +**Capturing the registry was declined on the same grounds one level down.** The first shipped form +took the registry as a compile-time parameter, saving one attribute load per hop. Since the registry +memoizes the closure in `_resolvers`, that made this the only compiled resolver forming +`registry → _resolvers → closure → cell → registry` — freeable then only by cyclic GC, never by +refcounting. Not a leak, but the repo already took the opposite position for containers (`64b7cec`), +and registries are per-root-container, so a suite building a container per test builds one per test. +Reading the registry off the `container` argument removed the cycle and measured **free** (250 → 249 +ns, inside noise), which also puts the alias in the shape every other closure in the module uses. + +Pinned by `test_alias_hop_costs_exactly_one_resolver_frame`, +`test_no_compiled_resolver_closes_over_its_registry`, +`test_overridden_alias_compiles_nothing_of_its_source`, and +`test_alias_picks_up_a_source_registered_after_a_failed_resolve` — that last catches only a +*negative* cache; a success-path cache is undetectable by construction, since a registered type's +provider can never be replaced and any registration clears `_resolvers`. + +**Revisit trigger:** an alias hop shows up hot in a profile from a real integration, **and** the +`_invalidate()`-clears-`_resolvers` invariant has acquired an explicit owner and test of its own — at +which point lazy bind (never eager) is worth the remaining ~60 ns. A second compiled closure needing +the registry at resolve time would reopen the capture question separately. diff --git a/docs/adr/0026-resolve-provider-not-a-seam.md b/docs/adr/0026-resolve-provider-not-a-seam.md new file mode 100644 index 00000000..f5d6198f --- /dev/null +++ b/docs/adr/0026-resolve-provider-not-a-seam.md @@ -0,0 +1,38 @@ +# `resolve_provider` is not an interception seam + +**Decision:** `Container.resolve_provider` is an entry point, not a hook. Overriding it in a +`Container` subclass is not a supported way to observe or intercept resolution, and the resolve path +is free to bypass it — which licenses inlining its body into `Container.resolve`, worth **-19% +(~38 ns) on every by-type resolve**, the path every `@inject` marker and framework integration takes. +`find_container` is **not** affected and remains a blessed extension point. + +**It is already not a seam, and that is measurable rather than arguable.** Since the compiled +resolvers shipped in 2.29.0, a resolver calls its dependencies' resolvers directly; nothing routes a +nested node through `resolve_provider`. Its only callers are `resolve()`, `resolve_dependency()`, and +the cycle back-edge thunk in `ProvidersRegistry.resolver_for`. Demonstrated on `main` before the +change: a subclass overriding `resolve_provider` and resolving a **4-node chain** records exactly +**1** call — the top-level one. An override has never seen the graph. What a subclass can still do is +instrument the *entry points* by overriding `resolve` and `resolve_provider`, which keeps working. + +**Deliberately narrower than [the `_scope_map` ruling](0024-scope-map-inline-declined.md), which +stands.** `find_container` is consulted on every cross-scope hop and the container it returns owns +the cached instance and runs its finalizer, so bypassing an override there silently relocates +lifecycle ownership — a bug, not a missed hook. Bypassing a `resolve_provider` override loses +observation, not correctness. **Field check:** an audit of all 13 sibling integration wheels found +zero `Container` subclasses and zero `resolve_provider` overrides, and `Container` subclassing was +never documented as an extension point. + +**Accepted costs**, disclosed rather than discovered later: a genuinely duplicated ~8-line body +(closed check, memo hit, `resolver_for` fallback, resolver call, `RecursionError` conversion) now +lives in both `resolve` and `resolve_provider` and must be edited in lockstep — the real, permanent +price; an exception raised through `resolve()` loses one traceback frame (5 → 4); recursion headroom +moves by one frame in the benign direction. + +**Consequence worth naming.** Together with +[the tracing decline](0023-debug-resolution-tracing-declined.md), modern-di offers no built-in way to +observe *per-node* resolution. That was already true — the compiled resolvers removed the last +interior call — and this records it rather than creating it. + +**Revisit trigger:** a concrete request for per-resolve interception from a real integration or user. +The answer then is a designed seam with a stated contract, not a re-blessing of subclass overrides, +which the compiled resolve path stopped honouring in 2.29.0. diff --git a/docs/integrations/writing-integrations.md b/docs/integrations/writing-integrations.md index 2b3e637b..af14b4ff 100644 --- a/docs/integrations/writing-integrations.md +++ b/docs/integrations/writing-integrations.md @@ -402,7 +402,7 @@ Each official integration is its own repository and PyPI package, mirroring the directly under `Full guide:`. - **Mirror `modern-di`'s** `CLAUDE.md` and `Justfile`. Keep behavioural invariants in named tests rather than in a prose truth home, and record rejected - alternatives under `planning/decisions/`. Keep resolution sync-only and add no + alternatives as ADRs under `docs/adr/`. Keep resolution sync-only and add no runtime dependency beyond the framework and `modern-di`. `ruff` is unpinned and CI floats it forward, so keep `CPY001` (no per-file copyright header) in the lint `ignore` and reflow any pre-existing Markdown-embedded code fences the diff --git a/justfile b/justfile index 47d9f46a..b2bf2276 100644 --- a/justfile +++ b/justfile @@ -12,13 +12,12 @@ lint: uv run ruff check --fix uv run ty check -# CI lint (no autofix) — same checks as `lint` plus the planning validator. +# CI lint (no autofix) — same checks as `lint` plus the repo-wide link check. lint-ci: uv run eof-fixer . --check uv run ruff format --check uv run ruff check --no-fix uv run ty check - uv run python planning/index.py --check uv run python planning/links.py # Check every relative Markdown link and heading anchor. `mkdocs --strict` only sees @@ -64,11 +63,3 @@ publish: # Build the docs site, failing on broken links / nav warnings; CI runs this on every PR. docs-build: uvx --with-requirements docs/requirements.txt mkdocs build --strict - -# Print the planning index (deferred, then decisions) to stdout. -index: - uv run python planning/index.py - -# Validate planning/deferred/ + planning/decisions/ frontmatter and naming; CI runs this. -check-planning: - uv run python planning/index.py --check diff --git a/mkdocs.yml b/mkdocs.yml index e4d57798..72f57d76 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -124,6 +124,10 @@ theme: exclude_docs: | /agents/ +# ADRs are built (so their outgoing links are validated) but stay out of the site menu. +not_in_nav: | + /adr/ + validation: omitted_files: warn absolute_links: warn diff --git a/modern_di/container.py b/modern_di/container.py index 423d91c9..d8e87aed 100644 --- a/modern_di/container.py +++ b/modern_di/container.py @@ -195,7 +195,7 @@ def resolve(self, dependency_type: type[types.T]) -> types.T: Carries its own copy of `resolve_provider`'s body rather than calling it: the extra frame is ~19% of a by-type resolve. The duplication is deliberate and the two must be - edited together -- see planning/decisions/2026-08-03-resolve-provider-not-a-seam.md. + edited together -- see docs/adr/0026-resolve-provider-not-a-seam.md. """ registry = self.providers_registry provider = registry._providers.get(dependency_type) # noqa: SLF001 diff --git a/planning/.convention-version b/planning/.convention-version deleted file mode 100644 index ccbccc3d..00000000 --- a/planning/.convention-version +++ /dev/null @@ -1 +0,0 @@ -2.2.0 diff --git a/planning/README.md b/planning/README.md index 1860b511..eb39540f 100644 --- a/planning/README.md +++ b/planning/README.md @@ -3,19 +3,7 @@ The standing record for `modern-di`. The living truth about *what the system does now* lives in the code itself and in its tests — an enforceable claim is an `INVARIANT:`-marked test, not a prose page. This directory holds what code and -tests cannot: the decisions taken (especially the options rejected) and the -work deliberately not scheduled. - -> **Local deviation.** This repo tracks the portable convention from -> [`lesnik512/planning-convention`](https://github.com/lesnik512/planning-convention) -> (applied version in `.convention-version`, beside this file), but currently -> **deviates from it** on four counts: `changes/`, `audits/`, and `retros/` were -> removed; the per-change spec moved into the PR body; decision frontmatter lost -> its `status` and `supersedes` keys; and `index.py` — a vendored file — was -> edited locally to match that schema. If the deviation holds, it goes upstream -> as convention 3.0.0 and is re-applied via that repo's `APPLY.md` flow. See -> [`deferred/2026-07-29-upstream-lean-convention.md`](deferred/2026-07-29-upstream-lean-convention.md) -> for the open question and its revisit trigger. +tests cannot. ## Quick path (start here) @@ -27,10 +15,11 @@ the template and ship a conventional-commit title. **2. File what outlives the PR:** -- an alternative you **rejected** with reasoning → `decisions/` +- an alternative you **rejected** with reasoning → an ADR in + [`docs/adr/`](../docs/adr/), numbered `NNNN-slug.md` - work that is real but **not scheduled** → `deferred/` -**3. Run `just check-planning` and `just check-links` before pushing.** +**3. Run `just check-links` before pushing.** ## Where a fact goes @@ -40,7 +29,7 @@ Four homes, one owner each: |---|---| | `modern_di/` | anything readable from the module — the default | | a named test | an **invariant**: must stay true, and a change could silently break it | -| `decisions/` | a rejected alternative, with the reasoning that would otherwise be re-litigated | +| `docs/adr/` | a rejected alternative, with the reasoning that would otherwise be re-litigated | | `docs/` | anything a user needs | Before writing a line anywhere: @@ -61,9 +50,9 @@ times. Promotion discipline was not the problem: 72% of commits touching none removed one, so the pages ratcheted toward restating code, and restatement is what goes stale. The absence of the directory is the mechanism. -`decisions/` and `INVARIANT:` docstrings inherit the same risk from the other direction: nothing yet -prunes a record once its call is settled or a docstring once its claim stops mattering, so keeping -either lean is a habit this project owes them now, not a one-time fix earned by deleting a directory. +ADRs and `INVARIANT:` docstrings inherit the same risk from the other direction: +nothing prunes a record once its call is settled or a docstring once its claim stops +mattering, so keeping either lean is a standing habit, not a one-time fix. An invariant is written as a test whose name is the claim, with a docstring opening `INVARIANT:` and a second paragraph naming **what breaks it**. That second paragraph @@ -73,73 +62,27 @@ test alone would fail on; a sibling test may be the one that actually trips. The of truth is the invariant plus the whole suite, not the docstring plus its single test — the accepted cost is that a reader cannot tell, from one docstring alone, whether that test or a sibling one catches a given regression. -`tests/test_invariant_census.py` enforces the shape and checks that every test name or -path cited from a `modern_di/`, `tests/`, `CLAUDE.md`, or `planning/decisions/`/`planning/deferred/` -comment or docstring resolves to something real. - -## What lives where - -A shipped change leaves two traces, none of them a file in this directory: the diff -and the PR body. Between them they answer *what changed* and *why*. - -`planning/` holds only what those two cannot: - -- **`decisions/` — what was decided against.** A rejected alternative leaves no - trace in a diff (the code that was not written) and isn't an enforceable claim - (there's nothing to assert). Without a home it gets re-proposed. -- **`deferred/` — what is waiting.** Real work, not scheduled. Nothing else in - the repo records the absence of something. - -If a fact fits in code, a test, the diff, or the PR body, it goes there instead. -This directory is the residue, and it should stay small. +`tests/test_invariant_census.py` enforces that shape. ## Artifacts -- **[`decisions/-.md`](decisions/)** — one file per design - decision taken, especially options *rejected*, each with a revisit trigger, so - reviews don't re-litigate them. Frontmatter: `summary`, plus `superseded_by` - once something supersedes it. - **[`deferred/-.md`](deferred/)** — one file per open item, each **self-contained**: it inlines the evidence and reasoning needed to pick - it up cold, and cites no report. Frontmatter: `summary`. A required - `**Revisit trigger:**` section — an item with no trigger is abandoned, not - deferred. + it up cold. A required `**Revisit trigger:**` section — an item with no trigger + is abandoned, not deferred. This directory is being retired in favour of GitHub + Issues; do not add to it. - **[`releases/.md`](releases/)** — one file per curated release, from `_templates/release.md`. Used **verbatim** as the GitHub Release body by [`release.yml`](../.github/workflows/release.yml), which fails a stable tag that has no matching file. No frontmatter; the file name is the version. -- **[`_templates/`](_templates/)** — `decision.md`, `deferred.md`, `release.md`. +- **[`_templates/`](_templates/)** — `release.md`. - **[`scripts/`](scripts/)** — reusable multi-agent audit harnesses. A sweep's - durable output is a PR plus `deferred/` items; the report itself is transient + durable output is a PR plus an issue or an ADR; the report itself is transient and is not committed. +- **`links.py`** — repo-wide Markdown link and heading-anchor check, run by + `just check-links` and by `just lint-ci`. It covers the trees a site builder + never sees. -### Location is status - -Neither artifact carries a `status:` field. Where a file sits, and which keys it -has, is what its state means. - -A **deferred item's presence in `deferred/` is its status**. When it resolves: - -- **it ships** → delete the file. Its truth is now in the code (or its tests) - and the release notes. -- **it is declined** → move it to `decisions/`, so the refusal is on record. - -A **decision is accepted unless it says otherwise**. There is no exit from -`decisions/` — a superseded decision stays readable, or it gets re-litigated — -so the one state worth recording is marked by adding `superseded_by: `, -which `just index` renders. Absent means accepted. The inverse `supersedes` key -is gone: it is derivable, and two pointers per relationship is one too many to -keep honest. - -`date` and `slug` are derived from the file name and never repeated in -frontmatter. `summary` is one line; it is the only field the index renders for -an ordinary entry. - -## Index - -The listing is **generated**, not maintained — run `just index` to print it: -deferred first (the open queue), then decisions, newest-first. The frontmatter in -each file is the single source of truth; there is no committed copy to drift. -`just check-planning` validates it, and `just check-links` validates every -relative Markdown link and heading anchor in the repo — including the trees a -site builder never sees. +A **deferred item's presence in `deferred/` is its status**. When it resolves: if +it ships, delete the file (its truth is now in the code and the release notes); if +it is declined, write the refusal as an ADR under [`docs/adr/`](../docs/adr/). diff --git a/planning/_templates/decision.md b/planning/_templates/decision.md deleted file mode 100644 index 23f2a547..00000000 --- a/planning/_templates/decision.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -summary: One line — shown in `just index`. ---- - - - - -# One-line capitalized title - -**Decision:** What was decided, in a sentence. - -## Context - -Why this came up; the options that were on the table. - -## Decision & rationale - -The call and why — including why the alternatives were rejected. Enough that a -future explorer doesn't re-litigate it. - -## Revisit trigger - -The concrete signal that should reopen this decision. diff --git a/planning/_templates/deferred.md b/planning/_templates/deferred.md deleted file mode 100644 index 43146f48..00000000 --- a/planning/_templates/deferred.md +++ /dev/null @@ -1,18 +0,0 @@ ---- -summary: One line — shown in `just index`. ---- - -# One-line capitalized title - -What the item is, in a sentence or two. - -## Why it is open - -The substance: the evidence, measurements, and reasoning needed to pick this up -cold. Inline it — a deferred item cites no report and no change file, because -this file is the only place the reasoning lives. - -## Revisit trigger - -The concrete signal that should make someone act on this. An item with no -trigger is not deferred, it is abandoned. diff --git a/planning/decisions/2026-06-23-sync-async-close-separate.md b/planning/decisions/2026-06-23-sync-async-close-separate.md deleted file mode 100644 index 947aed2e..00000000 --- a/planning/decisions/2026-06-23-sync-async-close-separate.md +++ /dev/null @@ -1,46 +0,0 @@ ---- -summary: Keep sync/async close paths separate — the divergence is intrinsic, not duplication. ---- - -# Keep sync and async `close` paths separate - -**Decision:** Leave `close_sync` / `close_async` as explicit pairs at all three -layers (`Container`, `CacheRegistry`, `CacheItem`); do not unify them into single -parametrized methods. - -## Context - -Architecture-review candidate #4 flagged "six near-identical `close_*` methods" as -a shallow seam ripe for deduplication. Investigation found the surface similarity -misleading — only the `Container` pair is near-identical (one line differs: -`cache_registry.close_sync()` vs `await …close_async()`). The other two layers -diverge intrinsically: - -- **`CacheItem`** — async `await`s the finalizer's result; sync **cannot await**, - so it detects an async finalizer (the `is_async_finalizer` flag or an awaitable - result), `.close()`s the coroutine to suppress the "never awaited" warning, and - raises `AsyncFinalizerInSyncCloseError`. -- **`CacheRegistry`** — async clears `_creation_order` entirely; sync - **preserves** the items that raised `AsyncFinalizerInSyncCloseError` in - `_creation_order` so a later `close_async()` can finish them. - -The genuinely-shared code is tiny (a ~4-line wrapper, a one-line guard, an -iterate-collect-raise skeleton). The *different* code is exactly the -sync-can't-await and preserve-for-later behaviors — a unified method would -re-introduce them as conditional branches, **adding** complexity rather than -concentrating it (the deletion test fails). - -## Decision & rationale - -Keep the explicit pairs. The divergent branches carry every historical finalizer -fix: B-7 LIFO teardown (`3f9a64b`), B-8 await/reject sync finalizers (`19e7c72`), -async-finalizer rejection (`faf2108`), `clear_cache` finalizer-dedup (`8ce0ff4`) -— all shipped in 2.15.0, and the area has been stable since. Unifying would risk -regressing precisely those cases for no locality, leverage, or testability gain. -The layering is honest: each `close` level adds one real concern. - -## Revisit trigger - -If finalizer/close bugs start recurring (the signal that the explicit pairs are -*causing* errors rather than encoding them), or if a third distinct -sync/async-spanning consumer appears that would share real logic. diff --git a/planning/decisions/2026-07-04-cache-arg-over-singleton-class.md b/planning/decisions/2026-07-04-cache-arg-over-singleton-class.md deleted file mode 100644 index cbbebecd..00000000 --- a/planning/decisions/2026-07-04-cache-arg-over-singleton-class.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -summary: Chose `Factory(cache=bool|CacheSettings)` over a `Singleton` class, a `cached=` flag, or `cache_settings=True`. ---- - -# Ergonomic caching toggle: `cache=` argument, not a `Singleton` class - -**Decision:** Make `Factory`'s caching toggle a single `cache` argument accepting -`bool | CacheSettings | None`, deprecating the `cache_settings=` alias — rather -than reintroducing a `Singleton` provider class, adding a separate `cached=True` -flag, or overloading the old `cache_settings=` to accept `True`. - -## Context - -`cache_settings=providers.CacheSettings()` is verbose for the common "just cache -it" case. A cross-framework survey found modern-di in the least ergonomic bucket -(a construction-time settings object); the ergonomic patterns are verb-level -(Koin `single {}`, .NET `AddSingleton()`) or a distinct `Singleton` class -(dependency-injector, that-depends). Options considered: - -1. **`Singleton` provider class** — a thin `Factory` preset with caching on. -2. **`cached=True` flag** — new boolean arg alongside `cache_settings=`. -3. **`cache_settings=True`** — overload the existing arg to accept a bool. -4. **`cache=` argument** (chosen) — rename to `cache`, accept - `bool | CacheSettings | None`, deprecate `cache_settings=`. - -## Decision & rationale - -`cache=` collapses caching onto one axis: absent/`None`/`False` off, `True` on -with defaults, `CacheSettings(...)` on and tuned. One argument, one mental model, -one place caching is expressed. - -- **Rejected `Singleton` class.** It expresses "caching is on" in two places — - the class name *and* a still-required `cache_settings` for finalizers (the most - common advanced case). Those two can drift, and forbidding `cache_settings` on - a `Singleton` would strand finalizers. It also reverses the 2.x "no separate - `Singleton` class" decision. A single `cache` axis has neither problem. -- **Rejected `cached=True` flag.** Two arguments both meaning "cache" requires a - both-passed conflict rule and gives `cached=True` no path to a finalizer - without switching forms — the same two-places-can-drift smell. -- **Rejected `cache_settings=True`.** Works and needs no new name, but the noun-y - argument reads wrong (`settings=True`). Renaming to `cache` keeps the - single-axis model *and* reads naturally in both forms (`cache=True` / - `cache=CacheSettings(...)`); the deprecation is a soft warn-only alias. - -`CacheSettings` is retained unchanged as the tuning object; only the entry point -gets sugar. Internals are untouched — the sugar normalizes into the existing -`self.cache_settings` attribute in `__init__`. - -## Revisit trigger - -A caching mode arises that a single `bool | CacheSettings` argument cannot -express cleanly. - -The other half of this trigger — the `cache_settings=` deprecation reaching -removal — has since fired and resolved: 3.0 dropped the alias, so `cache=` is -the only spelling and `cache_settings` survives only as the normalized internal -attribute. The single-axis model the decision chose is now the whole surface. diff --git a/planning/decisions/2026-07-05-no-enter-scope-alias.md b/planning/decisions/2026-07-05-no-enter-scope-alias.md deleted file mode 100644 index f9ae9512..00000000 --- a/planning/decisions/2026-07-05-no-enter-scope-alias.md +++ /dev/null @@ -1,30 +0,0 @@ ---- -summary: Rejected a Container.enter_scope() alias; build_child_container stays the single scope-entry spelling. ---- - -# No enter_scope alias for build_child_container - -**Decision:** `build_child_container` remains the single scope-entry spelling; no `enter_scope` -alias or rename. - -## Context - -The 2026-07-05 3.0 UX research (candidate API-3) noted every peer names this operation by intent — -wireup `enter_scope({Type: obj})`, .NET `CreateScope()`, dishka's callable container — while -`build_child_container` names the mechanism and is the longest scope-entry spelling in the studied -field. Options surfaced: add as permanent alias, add and deprecate the old name in 3.0, or reject. - -## Decision & rationale - -Rejected: in modern-di the mechanism is the concept. Child containers are real, user-visible -objects with their own cache and context registries — "enter scope" vocabulary would hide exactly -the mental model the docs work to teach. A second spelling of the most-written call after -`resolve()` conflicts with the conservative-feature-set constraint, and wireup's serial renames are -the cautionary precedent for renaming churn. The migrant-familiarity gap is closed in docs instead: -the accepted DOC-4 (FastAPI users page) and DOC-5 (cross-framework vocabulary table) carry the -"looking for enter_scope / CreateScope? It's build_child_container" mapping. - -## Revisit trigger - -Recurring user feedback that scope entry is hard to discover (e.g. issues asking "how do I enter a -request scope" that the docs mapping fails to deflect). diff --git a/planning/decisions/2026-07-05-no-generator-creators.md b/planning/decisions/2026-07-05-no-generator-creators.md deleted file mode 100644 index dd68c71e..00000000 --- a/planning/decisions/2026-07-05-no-generator-creators.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -summary: Rejected yield-based generator creators for 3.0; CacheSettings(finalizer=) stays the single teardown channel. ---- - -# No generator creators in core Factory - -**Decision:** `Factory` will not auto-detect generator creators and turn post-`yield` code into a -finalizer; `CacheSettings(finalizer=)` remains the only teardown spelling in core. - -## Context - -The 2026-07-05 3.0 UX research (candidate API-2) found every Python peer — dishka, wireup, svcs, -FastAPI yield-dependencies, that-depends `Resource` — spells teardown as code after `yield` in the -factory itself, making it the strongest muscle-memory delta for migrants. The proposal ranked #4 in -the shortlist and would have been breaking: `Factory(creator=generator_fn)` is legal today and -resolves to the raw generator object, so auto-detection changes existing behavior. It also carried -open design complexity: per-instance finalizer records for non-cached factories (or a `cache=` -requirement), declaration-time rejection of async generators, and `bound_type` extraction from -`Iterator[T]`. - -## Decision & rationale - -Rejected for core: the additions are complex relative to the ergonomic win, and the capability is -achievable without core changes — a `Factory` subclass (in userland or a sibling package) can wrap a -generator creator and register the continuation via the existing `CacheSettings(finalizer=)` -channel. Keeping one explicit teardown spelling also preserves the property that async finalizers -work under sync resolution, which the generator form cannot express. - -## Revisit trigger - -Recurring user requests for yield-based teardown, or a community-built generator-factory subclass -demonstrating both the demand and a settled design. diff --git a/planning/decisions/2026-07-05-no-multibinding.md b/planning/decisions/2026-07-05-no-multibinding.md deleted file mode 100644 index 0455cf10..00000000 --- a/planning/decisions/2026-07-05-no-multibinding.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -summary: No multibinding / collection injection — resolving `list[T]` to every provider registered for `T` contradicts the type→provider map the registry is built on; revisit only on concrete user demand, not on field precedent. ---- - -# No multibinding or collection injection - -**Decision:** `modern-di` does not support registering several providers for one -type and injecting them together as a collection. There is no `multi=True`, no -`list[T]` fan-in, no binder-set API. - -## Context - -Multibinding is common in the wider field and users arriving from those -ecosystems expect it: - -- MEDI resolves `IEnumerable` to every registration for `T` -- Spring injects `List` / `Map` -- `injector` exposes an explicit multibind API -- Angular uses `{ provide: TOKEN, useValue: x, multi: true }` - -The usual motivation is plugin-style extension: several handlers, validators, or -middleware registered independently and consumed as a set. - -## Decision & rationale - -The registry is a **type → provider map**. Multibinding requires that map to -become type → *collection of* providers, which changes the meaning of every -operation built on it: - -- Registration stops being "this type is now wired" and becomes "this type has - one more contributor", so `DuplicateProviderTypeError` — a deliberate - declaration-time guard — has to become conditional on an opt-in flag. -- Resolution by type stops having one answer. `resolve(T)` and `resolve(list[T])` - would resolve different things from the same registration, and the wiring plan - would need a third parameter category alongside provider-resolved and context - kwargs. -- Overrides get ambiguous: overriding `T` in a test either replaces the whole - collection or one contributor, and both readings are defensible. -- Validation loses a property it currently relies on — that a missing type is - unambiguous. An empty collection is indistinguishable from a wiring mistake. - -That is new registry semantics for a feature nobody has asked for here. It sits -outside the conservative feature set for the same reason the other borrowed -subsystems do: the cost is permanent and structural, the demand is inferred from -other ecosystems rather than observed in this one. - -The workaround costs a user one provider: declare a `Factory` that takes the -individual dependencies and returns the list. That is explicit, traceable to a -declaration site, and needs nothing from the framework. - -## Revisit trigger - -Concrete user demand — an actual request describing a real plugin-style use case -that the one-Factory workaround does not serve. Field precedent alone is not the -trigger; that is what was already weighed here. If it is ever reopened, the -registry-semantics consequences above are the design problem to solve first, not -the API spelling. diff --git a/planning/decisions/2026-07-07-no-redirect-plugin.md b/planning/decisions/2026-07-07-no-redirect-plugin.md deleted file mode 100644 index 95c1b1c8..00000000 --- a/planning/decisions/2026-07-07-no-redirect-plugin.md +++ /dev/null @@ -1,55 +0,0 @@ ---- -summary: Drop the mkdocs-redirects plugin entirely — merged-page URLs 404 instead of being preserved. ---- - -# No redirect plugin — merged-page URLs 404 - -**Decision:** Do not depend on `mkdocs-redirects` (or any redirect plugin) to -preserve the two old URLs from the docs-dedupe page merges -(`testing/fixtures/`, `introduction/that-depends-or-modern-di/`). Both URLs -404 after the merge. - -## Context - -Task 1 of the docs-dedupe change added `mkdocs-redirects>=1.2,<2` to -`docs/requirements.txt` and a `redirects` plugin block to `mkdocs.yml` so the -two merged pages' old URLs would keep resolving. Investigating the pin before -shipping found that `mkdocs-redirects` 1.2.3 (2026-03-28) is a hostile -release: it adds a dependency on `properdocs` — a fork of MkDocs whose code -hooks into every build to print scare-marketing urging users to switch to the -fork — and it caps `mkdocs<=1.6.1`, fighting our own `mkdocs>=1.6,<2` pin. -The prior release, 1.2.2 (2024-11-07), is clean; there has been no clean -release since. - -## Decision & rationale - -Remove the `mkdocs-redirects` dependency and the `redirects` plugin block -entirely, rather than pin around the compromised release. Pinning `==1.2.2` -would keep an unmaintained-and-now-hostile project in the dependency chain: -any future `pip`/`uv` resolution loosened even slightly re-admits 1.2.3+, and -a hard pin still ships a package whose maintainers have shown willingness to -weaponize a point release. Two URLs 404ing is a small, contained cost; -carrying a supply-chain-compromised dependency to avoid it is not a good -trade. - -### Rejected alternatives - -- **Pin `mkdocs-redirects==1.2.2`.** Freezes the immediate problem but leaves - an untrustworthy upstream in the chain — the next dependency bump (manual - or automated) can silently reintroduce 1.2.3+, and the pin itself is a - standing note-to-self that has to survive every future audit. -- **Local mkdocs hook to implement redirects ourselves.** Avoids the - dependency but adds custom build-time code to maintain for two URLs — more - surface area than the problem warrants. -- **Committed static meta-refresh stub pages** (e.g. hand-written HTML/MD - files at the old paths that redirect via ``). - Works without a plugin but adds permanent maintenance-owned files for a - problem that only exists because of the merge; simpler to just let the - URLs 404. - -## Revisit trigger - -Revisit if MkDocs gains native redirect support (removing the need for a -third-party plugin), or if `mkdocs-redirects` changes hands / publishes a -clean release that drops the `properdocs` dependency and the `mkdocs<=1.6.1` -cap. diff --git a/planning/decisions/2026-07-12-unify-graph-traversal.md b/planning/decisions/2026-07-12-unify-graph-traversal.md deleted file mode 100644 index 2b8c42ae..00000000 --- a/planning/decisions/2026-07-12-unify-graph-traversal.md +++ /dev/null @@ -1,63 +0,0 @@ ---- -summary: Reverse validation.md's "deliberate duplication" stance on the extraction axis — extract the shared graph traversal into a DependencyGraph module while keeping validate() and the runtime guard as distinct policies. ---- - -# Extract the shared provider-graph traversal, keep the two cycle policies - -**Decision:** Extract the provider-graph traversal and cycle-extraction into one -`DependencyGraph` module that `validate()`, the runtime `RecursionError` guard, -and alias scope-resolution all call. Keep the two *policies* distinct -(collect-all vs first-cycle). This reverses, on the extraction axis only, the -duplication `validation.md` currently defends. - -## Context - -`validation.md` argues the two cycle detectors are a "deliberate duplication, -not a refactor of the one DFS into two callers," because `validate()` collects -all errors while the runtime guard only answers "is a cycle reachable on an -exhausted stack" — and unifying them "would couple the resolve hot path to the -all-errors walker for no user benefit." - -That objection targets merging the two **policies** into one walker. It does not -cover the fact that both re-implement the same **traversal + cycle-extraction** -(the `path[cycle_start:]` slice and `CircularDependencyError` construction appear -verbatim in both), and that `Alias.effective_scope` hand-rolls a third -chain-walk. The peer framework dishka confirms the one-walk-many-concerns model -(its `GraphValidator` folds cycle + missing + scope into a single DFS) — it can -drop its runtime guard only because it makes validation effectively mandatory. - -## Decision & rationale - -The deletion test decides it: with the traversal shared, deleting -`DependencyGraph` makes cycle-detection complexity reappear across all four -callers — a real seam. `validate=False` stays a supported choice after 3.0, so -the runtime guard is *permanent*; two permanent callers of one traversal is a -real seam, not a hypothetical one. - -Rejected alternatives: - -- **Pure cycle-extraction helper only** — removes the verbatim copy but leaves - the DFS structure written twice; fails the deletion test. -- **Two traversal methods in the module (recursive walk + iterative find)** — - still two DFS bodies; relocates the duplication rather than removing it. -- **`DependencyGraph` type-checks `Alias`** to follow the chain — reintroduces - the concrete-type import coupling the codebase otherwise avoids; instead a - generic `redirect_target` node hook keeps the module Alias-agnostic. -- **Per-container validated stamp** — a child would not inherit the root's - validation; the graph is shared, so the stamp is registry-level. -- **Fold scope-inversion into reachability (dishka-style)** — simpler mechanism - but a less precise error (`missing` vs `inverted`); keep the dedicated - `InvalidScopeDependencyError`. - -## Revisit trigger - -Benchmarks show the `walk()` event-stream indirection measurably slows -`validate()` or the guard. - -The original second limb — "`validate=False` is ever dropped, so the runtime -guard disappears and the seam collapses to a single caller" — is retired: 3.1 -went the other way. Validation is now explicit-only, `Container(validate=...)` -is an ignored no-op removed at 4.0, and the `RecursionError`-to- -`CircularDependencyError` guard is therefore permanent rather than contingent. -Two callers of one traversal is settled; only a single-caller collapse would -reopen the seam, and nothing on the roadmap produces one. diff --git a/planning/decisions/2026-07-13-integration-kit-shape.md b/planning/decisions/2026-07-13-integration-kit-shape.md deleted file mode 100644 index e14c1de7..00000000 --- a/planning/decisions/2026-07-13-integration-kit-shape.md +++ /dev/null @@ -1,63 +0,0 @@ ---- -summary: Integration kit lives in core as low-level primitives; outliers bypass rather than the primitive absorbing them. ---- - -# Integration kit is low-level primitives in core, and outliers bypass it - -**Decision:** Extract the shared adapter skeleton into a framework-agnostic -module inside `modern-di` core, exposing only low-level primitives; genuine -outliers call core's `build_child_container` directly rather than the primitives -growing parameters to swallow them. - -## Context - -The 13 integrations duplicate a framework-agnostic skeleton (annotation-scanning -injector + per-request child lifecycle). Options on the table for each axis: - -- **Home:** core module · new `modern-di-integrations` package · status quo. -- **Interface width:** low-level primitives only · two-tier (primitives + a - `make_inject`/child-context-manager convenience layer). -- **Outliers** (aiohttp websocket probe, grpc `set_context` split, typer - no-context): bypass the kit · primitive absorbs them via extra parameters. - -## Decision & rationale - -- **In core, not a new package.** The skeleton imports only stdlib + `modern_di`, - so it doesn't threaten core's zero-dependency stance, and it deepens the same - `add_providers`/`resolve_dependency` seam core already blesses. A 14th repo - would add coordinated-release cost for agnostic code every adapter already - reaches via its `modern-di` dependency. -- **Low-level primitives only.** A `make_inject` convenience fails the deletion - test: the adapters' wrapper shapes are *not* identical (each fetches the child - differently — request, ASGI scope, `g`, contextvar), so the convenience must - take a `get_container` callable and just wraps three primitives — a shallow - module, exactly what the extraction exists to remove. -- **Outliers bypass.** Each absorbing parameter (scope-resolver callable, - post-build `set_context` hook, no-context mode) is needed by exactly one - adapter — a hypothetical seam, not a real one. Adding them taxes the 10 - common-case adapters and lowers depth. The outliers already call - `build_child_container` (the real blessed primitive) directly; keeping the weird - logic in the weird adapter is better locality. - -## Addendum (2026-07-13): "bypass" is narrower than first assumed - -Reading all 13 adapters concretely (not just the 4 sampled at design time) -showed only **typer** is a true Layer-1 bypass — it binds no connection at all. -aiohttp and grpc both use `bind(provider, connection)` for scope+context -derivation; they only skip `classify_connection` (aiohttp because both its -providers share one type, so isinstance can't dispatch; grpc because it has one -provider and no dispatch to do). grpc's `_build_child` collapses to one `bind()` -call, dropping its post-hoc `set_context` entirely. aiogram's context is a -multi-provider merge with a hardcoded scope — a third shape `bind()` doesn't fit -either, so it stays a two-line literal, but for a different reason than typer's -(no connection to bind) or aiohttp's (dispatch, not derivation, is the -mismatch). The **decision stands** — no primitive grew parameters to absorb any -of these — the correction is only which adapters end up calling which -primitive. - -## Revisit trigger - -A third adapter needs the same non-isinstance scope-dispatch (making the -"absorb it" seam real, two-adapters rule), or the convenience layer is requested -by adapter authors because the residual `inject` glue proves non-trivial in -practice. diff --git a/planning/decisions/2026-07-14-error-text-is-not-a-contract.md b/planning/decisions/2026-07-14-error-text-is-not-a-contract.md deleted file mode 100644 index 5eb97237..00000000 --- a/planning/decisions/2026-07-14-error-text-is-not-a-contract.md +++ /dev/null @@ -1,50 +0,0 @@ ---- -summary: Rendered error text is diagnostic, not a public contract — the structured attributes are; a change may reformat a message without a deprecation cycle. ---- - -# Rendered error text is not a public contract - -**Decision:** The *rendered* text of a `ModernDIError` is diagnostic output and -may change in any release. The **structured attributes** each error carries -(`.provider_type`, `.cycle_path`, `.suggestions`, `.dependency_path`, …) and the -**class hierarchy** callers catch on are the public contract, and those change -only with the usual care. - -## Context - -Came up while unifying the two error renderers into one. -Unifying the two chain drawers means `CircularDependencyError` renders through the -same code path as `DependencyPathMixin`, which prints an aligned scope column. -Either the cycle message gains that column (a user-visible change), or the shared -drawer carries a `show_scope` flag forever to preserve byte-identical output. - -The same question sits under the suggestion work: `.suggestions` currently holds -pre-rendered bullet strings, and making it hold structured `Suggestion` records -changes what a caller reading that attribute sees. - -## Decision & rationale - -Error text is for a human reading a traceback. Freezing its bytes buys nothing -real and costs compounding: every renderer grows a compatibility flag, and the -formatting can never be improved. So the cycle message gains the scope column, -and the drawer needs no flag. - -The attributes are a different matter, and the distinction is the point of the -split: a caller who wants to *act* on an error should read `.cycle_path`, not -regex the message. Pre-rendering suggestions into `.suggestions` violated that — -it forced a programmatic consumer to parse glyphs back out. Structured records -fix the contract rather than break it. - -This does not license churn. It licenses a message *improving* without a -deprecation cycle, and it means message-text assertions in tests are pinning an -implementation detail, not a promise. - -Rejected: freezing the rendered text. It would have preserved output nobody -depends on, at the price of a permanent flag in the one drawer this change exists -to unify. - -## Revisit trigger - -A downstream consumer (an integration, or a user in an issue) is found parsing -`str(exc)` to recover structured facts. That means the attribute surface is -missing something — add the attribute, and keep this decision. diff --git a/planning/decisions/2026-07-14-grpc-registry-introspection-declined.md b/planning/decisions/2026-07-14-grpc-registry-introspection-declined.md deleted file mode 100644 index 069b7895..00000000 --- a/planning/decisions/2026-07-14-grpc-registry-introspection-declined.md +++ /dev/null @@ -1,52 +0,0 @@ ---- -summary: Decline a blessed provider-introspection seam (container.is_registered) for grpc's idempotent registration — one consumer is a hypothetical seam; grpc keeps its local guard. ---- - -# No blessed provider-introspection seam for grpc's registry drill - -**Decision:** Do not add `Container.is_registered(type)` (or an -idempotent-`add_providers` mode) for adapters to query registration state -(Candidate 5 from the 2026-07-13 architecture review). `modern-di-grpc` -keeps its local `_ensure_context_provider` guard. - -## Context - -`modern-di-grpc`'s `_ensure_context_provider` calls -`container.providers_registry.find_provider(ServicerContext) is None` before -`container.add_providers(grpc_context_provider)`, to register the context -provider idempotently. grpc has no `setup_di` — constructing an interceptor -*is* the setup, and both interceptors (sync `DIInterceptor` + async -`DIAioInterceptor`) may be built on one container, so the guard prevents a -`DuplicateProviderTypeError` from `add_providers`. The review read this as -grpc "reaching past the blessed seam" and proposed a blessed -`is_registered` query. The integration-kit design deferred it as a distinct -change ([2026-07-13-integration-kit-shape](2026-07-13-integration-kit-shape.md) -non-goals). - -Options on the table: (a) bless `container.is_registered(type)` and convert -grpc; (b) add an `ignore_existing` mode to `add_providers` so grpc registers -unconditionally; (c) close it — keep grpc's local guard. - -## Decision & rationale - -Chose (c). The deciding evidence: **grpc is the only consumer** — a grep -across every `modern-di-*` adapter and its tests found no other reader of -`providers_registry` / `find_provider`. By the project's standing rule (one -adapter = hypothetical seam, two = a real one — the same principle that kept -the integration-kit outliers' logic local rather than growing the primitives), -a single consumer does not justify new core API. - -Reinforcing it: `container.providers_registry` and `find_provider` are both -**public** — grpc is using a lower-level public API, not breaching -encapsulation. And `add_providers`' strictness (raising on a duplicate -`bound_type`) is a deliberate feature that catches accidental -double-registration; option (b) would loosen it globally to serve one -adapter, a large blast radius. grpc's guard is two legible, local lines that -belong with grpc's unusual "the interceptor is the setup" shape. - -## Revisit trigger - -A **second** adapter needs to query registration state (making the seam real -under the two-adapter rule), or a decision to privatize -`container.providers_registry` as an implementation detail — at which point a -blessed `is_registered` becomes the migration path for grpc. diff --git a/planning/decisions/2026-07-14-signatureitem-opacity-superseded.md b/planning/decisions/2026-07-14-signatureitem-opacity-superseded.md deleted file mode 100644 index 5add8bf3..00000000 --- a/planning/decisions/2026-07-14-signatureitem-opacity-superseded.md +++ /dev/null @@ -1,55 +0,0 @@ ---- -summary: Making SignatureItem an opaque resolved parameter (a 2026-07-13 review candidate) is superseded — the graph-traversal unification already extracted its two behaviours as wiring.py functions. ---- - -# SignatureItem opacity is superseded by the wiring extraction - -**Decision:** Do not pursue "make `SignatureItem` an opaque resolved -parameter" (Candidate 3 from the 2026-07-13 architecture review). Its -substance already shipped in the graph-traversal unification (the -`DependencyGraph` module, PR #308), which placed the behaviour better than the -candidate's sketch. - -## Context - -The review flagged `SignatureItem` (`types_parser.py`) as a shallow record -whose five raw fields (`arg_type`, `args`, `is_nullable`, `default`, -`raw_annotation`) were decoded independently at ~5 sites, and proposed -absorbing that behind two operations — `match_provider(reg)` ("which provider -backs this?") and `disposition()` ("what if absent?") — ideally as methods on -`SignatureItem`, so callers never touch the raw fields. - -## Decision & rationale - -The core leak — the old wiring loop reading raw fields to pick providers and -handle absence — is gone. `modern_di/wiring.py` now owns exactly those two -operations as free functions: - -- `find_dep_provider(registry, owner, item)` = `match_provider` — prefers - `arg_type`, falls back to union `args`. -- `absent_disposition(item)` = `disposition` — default → OMIT, nullable → - NULL, else UNWIRABLE. - -`WiringPlan.build` (the former leaky loop) calls these two and touches no raw -field. The candidate's proposal is realized. - -The free-function placement in `wiring.py` is **deliberately better** than the -candidate's "methods on `SignatureItem`" idea: `SignatureItem` stays a pure -data record in `types_parser.py` and does not import `ProvidersRegistry` / -`AbstractProvider`. Methods-on-`SignatureItem` would invert that layering -(the parse-tree type depending on the resolution machinery). - -The residual raw-field reads that remain (all in `providers/factory.py`) are a -thinner, different set — the parameterized-generic construction guard -(`raw_annotation` + `default`), the error builder (`arg_type` + `args`), and -deriving `bound_type` from the return sig's `arg_type`. None is duplication, -none collapses into `match_provider`/`disposition`, and each is a single -legitimate read. Forcing a further "opacity" refactor here would be churn -without a genuine deepening. - -## Revisit trigger - -A *new* duplicated raw-field decode idiom appears across two or more sites -(the repeated-idiom smell the review originally caught), or `types_parser.py` -gains logic that would genuinely benefit from behaviour living on the record -without inverting the `types_parser` → `wiring` layering. diff --git a/planning/decisions/2026-07-15-fold-context-registry-declined.md b/planning/decisions/2026-07-15-fold-context-registry-declined.md deleted file mode 100644 index 68bdd0bc..00000000 --- a/planning/decisions/2026-07-15-fold-context-registry-declined.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -summary: Decline folding ContextRegistry into Container — it is a documented, symmetric node in the shared-vs-per-container four-registry model, not incidental co-location, so the ~18-line deletion would break a uniform abstraction for no real friction. ---- - -# Keep ContextRegistry as its own module - -**Decision:** Do not fold `modern_di/registries/context_registry.py` into -`Container` (Candidate 5 from the 2026-07-15 architecture review). -`ContextRegistry` stays a named registry; `ContextProvider` keeps reading it via -`container.context_registry.find_context(...)`. - -## Context - -`ContextRegistry` is the shallowest of the four registries — ~18 lines, a -`dict[type, Any]` behind `find_context` (returns `UNSET` on miss) and -`set_context`, with no dedicated tests. Its interface is as large as its -implementation, and the deletion test on the *code* passes: fold it, and -`Container` gains a `self._context` dict, keeps `set_context`, and grows a -`find_context` method (a natural sibling to the already-blessed `find_container` -primitive) for the two touch points — `Container.set_context` (write) and -`ContextProvider.fetch_context_value` (read). The review flagged it Speculative -and noted it was originally only worth doing alongside Candidate 3 (the -provider-facing seam), which has since been declined -([2026-07-15-provider-facing-seam-declined](2026-07-15-provider-facing-seam-declined.md)). - -Options: (a) fold it and update the docs; (b) decline. - -## Decision & rationale - -Chose (b). The deciding evidence: **`ContextRegistry` is a documented, symmetric -node in a deliberate model, not incidental co-location.** -`architecture/containers.md` (since removed) had a "Registry sharing" section that organised the -four registries by a real axis — *shared across the tree* (`ProvidersRegistry`, -`OverridesRegistry`) vs *per-container* (`CacheRegistry`, `ContextRegistry`). -`ContextRegistry` sits symmetric with `CacheRegistry` as one of the two -per-container registries. Its shallowness in line count reflects having less -*mechanism*, not a broken abstraction — its *role* in the model is fully -symmetric. - -So the deletion test result is misleading here: the *code*-complexity vanishes, -but the *conceptual* slot does not — `Container` still has per-container context -state; folding merely turns a named registry (symmetric with `CacheRegistry`) -into an unnamed inline dict. That trades a clean, documented, uniform 2×2 model -for ~18 fewer lines, breaks the containers.md table into "three registries + an -inline dict," and grows the already-largest file — all with **zero actual -friction**: no bug hides in the one-line delegation, and context is not a -change hot-path. Uniformity is worth more than the deletion here, and it serves -the same AI-navigability goal the candidate invoked: a predictable four-registry -pattern navigates better than one with an exception. - -## Revisit trigger - -The **four-registry model is restructured** — any registry folded, or the -shared-vs-per-container framing in `containers.md` abandoned — since the symmetry -is the load-bearing reason to keep it; **or** concrete friction emerges: a bug in -the `Container` → `ContextRegistry` delegation, or the indirection repeatedly -obstructing context-related work. diff --git a/planning/decisions/2026-07-15-provider-facing-seam-declined.md b/planning/decisions/2026-07-15-provider-facing-seam-declined.md deleted file mode 100644 index b9c7ae9c..00000000 --- a/planning/decisions/2026-07-15-provider-facing-seam-declined.md +++ /dev/null @@ -1,84 +0,0 @@ ---- -summary: Decline a provider-facing seam (ResolutionContext view / promoting Container internals) — one Container implementation makes it a hypothetical seam, the crossings are core-internal (three then, one since the compiled resolver), and the supported/internal split is documented. ---- - -# No provider-facing seam on Container - -**Decision:** Do not extract a `ResolutionContext` view handed to providers, nor -promote `Container`'s provider-facing members into a new declared interface -(Candidate 3 from the 2026-07-15 architecture review). Resolution runs against -the `Container` directly; the existing `# noqa: SLF001` reaches stay. - -## Context - -`Container` presents two interfaces through one class: a user-facing one -(`resolve`, `validate`, `override`, `set_context`, `build_child_container`, -`close`) and an undeclared provider-facing one. The review read the built-in -providers reaching past the seam into privates as friction and proposed either a -narrow `ResolutionContext` object handed to `resolve`, or promoting the members -providers need into a documented internal interface. - -Exploration sharpened the picture, and the compiled resolver has since narrowed -it further: - -- **The crossings are few, and all core-internal.** At decision time there were - three: `Factory._lock` (only to pass to `CacheItem.get_or_create`), plus - `_warn_and_reopen_if_closed` on `Factory` and on `ContextProvider` — built-in - providers touching a sibling core class. (The review's "five" predated - candidate 1's `Factory.resolve` reorder.) Today there is **one**: - `ContextProvider` calling `container._prepare()`. The single-path compiled - resolver dissolved `Factory.resolve` and `Alias.resolve` outright, so the - lock and closed-state reaches moved out of the providers and into - `resolver_compiler.py` — the compiler's business, ruled on separately in - [2026-07-17-per-provider-compile-seam-declined](2026-07-17-per-provider-compile-seam-declined.md), - not a provider-facing boundary. -- **The split is documented.** `docs/providers/advanced-api.md` blesses - `find_container(scope)` — "the primitive the compiled resolvers use to locate - the container at a provider's scope" — and marks `_lock` / `_scope_map` / - `parent_container` as "internal — no stability guarantee, do not build on - them." -- **Closed-state handling moved twice and settled.** 3.0 replaced - warn-and-reopen with a hard raise; 3.1 made `open()` optional, so a root is - open from construction and reuse after an explicit close warns and reopens. - `_warn_and_reopen_if_closed` no longer exists in the tree; `_prepare()` is - what remains of that reach. - -Options on the table: (a) a `ResolutionContext` view narrowing what a provider -can touch; (b) minimal — bless a small provider-facing contract so custom -providers aren't forced into "do not build on" internals; (c) decline. - -## Decision & rationale - -Chose (c). The deciding evidence: **there is one `Container` implementation**. -Everything that resolves does so against the same single collaborator, so a -`ResolutionContext` abstraction would be one interface with one implementation. -By the project's standing rule (one adapter = hypothetical seam, two = a real -one), the seam is hypothetical on the axis that matters. This is the same -reasoning that declined the grpc introspection seam -([2026-07-14-grpc-registry-introspection-declined](2026-07-14-grpc-registry-introspection-declined.md)). - -Reinforcing it: option (a) would have changed -`AbstractProvider.resolve(container)` — then the documented public extension -contract — to `resolve(ctx)`, a large blast radius to formalize a boundary only -core code crosses. The crossings are core-touching-core, and the docs already -declare which members are supported (`find_container`) versus internal -(`_lock`); the `# noqa: SLF001` markers are honest labels, not a leak. - -The one genuine gap weighed at the time — that a custom-provider author had no -*supported* way to do Factory-grade singleton locking or closed-state handling — -has since dissolved from the other end rather than being closed. The provider -set is closed and subclassing `AbstractProvider` is not an extension point -([2026-07-17-custom-providers-retracted](2026-07-17-custom-providers-retracted.md)), -so there is no audience left for a provider-facing contract to serve. - -## Revisit trigger - -A **second `Container` implementation** appears, making `ResolutionContext` a -real two-adapter seam. - -The original second limb — a real custom-provider author blocked on the "do not -build on" internals — is retired: the provider set is closed, so that author -does not exist. If the set ever reopens, the migration path is the polymorphic -`compile()` hook named in -[2026-07-17-per-provider-compile-seam-declined](2026-07-17-per-provider-compile-seam-declined.md), -not a provider-facing view of `Container`. diff --git a/planning/decisions/2026-07-17-custom-providers-retracted.md b/planning/decisions/2026-07-17-custom-providers-retracted.md deleted file mode 100644 index dc11ade2..00000000 --- a/planning/decisions/2026-07-17-custom-providers-retracted.md +++ /dev/null @@ -1,86 +0,0 @@ ---- -summary: The provider-type set is closed (Factory, Alias, ContextProvider, container_provider). Retract "Subclassing AbstractProvider" from the documented supported extension points rather than restoring the open dispatch the compiled resolver removed. ---- - -# Custom providers are not an extension point; the provider set is closed - -**Decision:** `modern-di` supports exactly four provider types — `Factory`, -`Alias`, `ContextProvider`, and the pre-built `container_provider`. Subclassing -`AbstractProvider` (or `Factory`) to add a provider type is **not** supported. -The `docs/providers/advanced-api.md` section promising it is retracted rather -than honored. - -## Context - -Until 2.28.0, `Container.resolve_provider` ended in `provider.resolve(self)` — a -plain polymorphic call. Custom provider support was never designed; it was an -**emergent property** of dispatching through a method. Any subclass implementing -`resolve()` worked automatically, because that is how Python inheritance behaves. -`docs/providers/advanced-api.md` then documented that accident under -"## Supported extension points", with an implement-`resolve(container)` recipe. - -The single-path compiled resolver (`2026-07-16.02`, #334) replaced the -polymorphic call with `compile_resolver`, which selects a closure by exact type -identity and rejects everything else: - -```python -if type(provider) is Factory: - ... -if type(provider) is Alias: - ... -if type(provider) is ContextProvider: - ... -raise TypeError(f"no compiled resolver for provider type {...}") -``` - -It also deleted `AbstractProvider.resolve` and `Alias.resolve`, so no hook -survives to fall through to. The closure of the set was deliberate — asserted by -`tests/test_container.py::test_resolve_provider_raises_for_unhandled_provider_type` -and reasoned from in -[2026-07-17-per-provider-compile-seam-declined](2026-07-17-per-provider-compile-seam-declined.md) -("the provider-type set is closed and tiny"). Only the docs were left behind. - -Investigation (2026-07-17) established the blast radius and the failure mode: - -- **`type(x) is Factory` is identity, not `isinstance`.** A `LoggingFactory(Factory)` - that overrides nothing at all fails exactly like a from-scratch provider. The - break is wider than "exotic custom providers". -- **`validate()` cannot catch it.** Validation walks the dependency graph via - `get_dependencies()`; compilation happens lazily in `resolver_for` at first - resolve. A container built with `validate=True` reports clean and then raises - `TypeError` at first resolve, under traffic. -- **Zero consumers.** All 13 sibling `modern-di-*` repos, the two templates, and - `lite-bootstrap` contain no `AbstractProvider` subclass. (`that-depends` hits - are that library's own unrelated base class.) - -Options weighed: (a) retract the docs and ship 2.29.0; (b) restore a fallback in -2.x behind a `DeprecationWarning` naming the 3.0 removal, matching the -`ContainerClosedWarning` / `ContextValueNoneWarning` / `UnvalidatedContainerWarning` -ramps, and retract at 3.0; (c) hold the whole post-2.28.0 backlog for 3.0. - -## Decision & rationale - -Chose (a). The capability was never intended, is asserted against by a test, and -has no consumer anywhere in the project's own ecosystem. Restoring an open -dispatch to preserve an accident would re-import the exact indirection #334 -removed, and would contradict a decision taken the same day on the same seam. - -Rejected (b) — the deprecation ramp — despite it being the house pattern for -2.x→3.0 removals. Those three ramps each guard a capability users demonstrably -rely on; this one would guard an audience believed to be empty, at the cost of -carrying a resurrected fallback path plus a warning class through the 2.x line. -The residual risk is accepted knowingly: an unknown external user subclassing -`Factory` gets a runtime `TypeError` on a minor bump, with `validate()` giving a -false all-clear first. The 2.29.0 notes call this out explicitly under a -breaking-change heading so the release, not the docs, carries the warning. - -Rejected (c) — 3.0 is not scoped, and holding the compiled resolver and the perf -work behind it strands shipped, tested work for an unscoped release. - -## Revisit trigger - -A real user reports a broken custom provider or `Factory` subclass — which would -falsify the zero-audience premise this rests on. At that point the migration path -is the polymorphic `compile()` hook named in -[2026-07-17-per-provider-compile-seam-declined](2026-07-17-per-provider-compile-seam-declined.md)'s -own revisit trigger, not a restored interpreted fallback. diff --git a/planning/decisions/2026-07-17-per-provider-compile-seam-declined.md b/planning/decisions/2026-07-17-per-provider-compile-seam-declined.md deleted file mode 100644 index c33fc000..00000000 --- a/planning/decisions/2026-07-17-per-provider-compile-seam-declined.md +++ /dev/null @@ -1,67 +0,0 @@ ---- -summary: Decline a per-provider compile() seam that would dissolve resolver_compiler's type-dispatch and its reaches into provider privates — one compiler makes it a hypothetical seam, and the concentrated hot-path closures are a deliberate property. Only the phantom docstrings were a real defect; those were fixed. ---- - -# No per-provider compile() seam - -**Decision:** Do not move `resolver_compiler`'s per-type closure builders into a -`provider.compile(registry) -> resolver` method on each provider class (Card 1 -of the 2026-07-17 architecture review). The `compile_resolver` type-dispatch and -its `# noqa: SLF001` reaches into `Factory`/`Alias` privates stay. The three -phantom docstrings that named dissolved interpreted methods were the one real -defect and were corrected. - -## Context - -The single-path compiled resolver (`2026-07-16.02`) made resolve fast by -inlining per-type closures in `resolver_compiler.py`. The review read three -things as friction: (1) `compile_resolver` dispatches on provider type with a -hardcoded `if type(provider) is Factory / Alias / ...` chain; (2) the builders -reach ~16 times into `Factory` privates (`_creator`, `_parsed_kwargs`, -`_resolution_step`, `_resolve_context_value`, `_call_creator`, -`_argument_resolution_error`) and twice into `Alias` (`SLF001`); (3) three -docstrings claimed the closures "mirror" `Factory._resolve_kwargs`, -`Alias.resolve`, and `_ContainerProvider.resolve` — methods that no longer exist. - -The proposed deepening: give each provider a `compile()` method, collapsing the -dispatch to `provider.compile(registry)` and turning the private reaches into -`self.` access. Options weighed: (a) full seam — move each builder into its -provider class; (b) middle form — `compile()` extracts its own fields and hands -them to shared flat closure-builders that stay concentrated; (c) decline the -structural change and fix only the docstrings. - -## Decision & rationale - -Chose (c). The deciding evidence mirrors -[2026-07-15-provider-facing-seam-declined](2026-07-15-provider-facing-seam-declined.md): -**there is exactly one compiler.** `resolver_compiler` is the sole consumer of -those provider privates; nothing else varies across the proposed seam. By the -project's standing rule (one adapter = hypothetical seam, two = a real one), a -`compile()` seam is hypothetical — cleanliness, not a swap point. - -Reinforcing it: - -- **The `SLF001` reaches are intra-package intimacy, not a leaked abstraction.** - The compiler co-evolves with the classes it compiles; they ship and change - together. The markers are honest labels on a deliberate friendship, not a - boundary violation. -- **The provider-type set is closed and tiny.** Polymorphic dispatch buys - extensibility for types that are essentially never added; the `if type() is` - chain over four types is not worse. -- **Concentration is a deliberate, valuable property.** Every perf-critical - closure lives in one file, reviewed together, sharing the positional/kwargs and - two-phase-error patterns. The full seam (a) sacrifices that; the middle form - (b) preserves concentration only by adding a field-extraction indirection that - earns nothing while the seam stays hypothetical. - -The genuine defect was the doc-rot: three docstrings and two inline comments -named dissolved methods. Those were rewritten to describe the behavior directly -(no interpreted-path references remain), which is the whole of the fix. - -## Revisit trigger - -A **second consumer of provider compile-time privates** appears (a distinct -compiler, an alternate resolver backend), making `compile()` a real two-adapter -seam — **or** provider types become an open, user-extended set where adding one -must not require editing a central dispatch, at which point the polymorphic -`compile()` hook becomes the migration path. diff --git a/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md b/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md deleted file mode 100644 index 71a5b520..00000000 --- a/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md +++ /dev/null @@ -1,79 +0,0 @@ ---- -summary: Drop the warm-singleton resolver memo-swap — built, fully tested, and measured at ~1.6x (short of the pre-committed 2x gate, because `resolve_provider`'s dispatch floor is unremovable by the swap); a bounded win does not buy a permanent cross-cutting invalidation invariant. ---- - -# Drop the warm-singleton resolver memo-swap - -**Decision:** `modern-di` does not self-modify its resolver memo after a -singleton is built. The warm-singleton hit keeps paying the normal compiled-cached -resolver path. The mechanism was implemented in full, measured, and reverted. - -## Context - -The single-path compiled resolver (#334) left one perf lever open: the warm -singleton hit was 292 ns, against dishka ~245 ns, wireup ~98 ns, that-depends -~85 ns, and dependency-injector ~61 ns. modern-di paid `resolver_for` dispatch, -the override front-guard, and `fetch_cache_item` on *every* warm hit. - -wireup's technique is to swap the cached provider's stored resolver for a bare -`return value` closure once the value exists. Adapting it here is constrained by -modern-di's shared-registry / per-container-cache split: the resolver lives -tree-wide on `ProvidersRegistry` while the cached value is per-container, so a -bare swap is only sound for **APP-scoped** singletons, where a clean -1:1:1 holds — one registry ↔ one APP container ↔ one tree-wide value. Deeper -scopes cache per child container, so a registry-level constant would be wrong. - -This was run as a **measurement-gated spike** with a pre-committed kill-gate: -ship iff the warm hit reached ≤ ~146 ns (at least halved) *and* beat dishka, -with zero correctness regression and a green free-threaded stress test. - -## Decision & rationale - -The mechanism was built exactly as designed — APP-scope predicate, install on -the cold-miss branch after `mark_created`, `_warm_original` for O(1) restore, -and invalidation across registry mutation (version bump), `override()` -(un-swap the pid), and root close (invalidate all, which also restores the -closed-container warn/reopen shim a bare closure would bypass). It passed the -free-threaded stress test at `--count=100` (200/200) and `test-ci` at 100% -coverage. - -**Measured** (best-of-3, stable medians, machine-relative): guard g2 warm hit -584 → 375 ns; comparative C2 333 → 208 ns. A consistent **~1.6x** reduction — -enough to flip modern-di ahead of dishka (292 ns), but short of the ≤146 ns gate. - -**Root cause of the shortfall:** `resolve_provider`'s dispatch floor — the -`_warn_and_reopen_if_closed` frame, the `resolver_for` dict lookup, and the -version check — is not removable by the swap. Unlike wireup, modern-di keeps the -resolve → `resolver_for` dispatch, so "near-free" was never architecturally -reachable by this technique. The swap optimizes the closure body; the floor is -upstream of it. - -**The ruling:** a ~1.6x win does not justify a permanent cross-cutting -invariant — a bypass resolver spanning `override` / `close` / `add_providers`, -plus a second source of truth in `_warm_swapped` — against the -conservative-feature-set and legibility principles. Below the bar, nothing ships -but the measurement. - -Two things from the attempt were kept: - -1. The free-threaded stress test surfaced that close-during-resolve was not - torn-free. The research it triggered established that the - build → resolve → dispose lifecycle with single-threaded teardown is the - universal field standard, now stated explicitly in - [design decisions](../../docs/introduction/design-decisions.md#the-thread-safety-boundary). -2. It revealed a better direction — the **dispatch-floor simplification** - (invalidate-on-mutation instead of a version stamp per resolve), which - *removes* per-resolve work instead of adding a bypass and is licensed by that - now-explicit lifecycle contract. It shipped in #347. - -The revert restored `providers_registry.py`, `resolver_compiler.py`, -`container.py`, and their tests; `concurrency.md` was kept. - -## Revisit trigger - -A user-reported warm-singleton bottleneck **plus** a design that removes the -dispatch floor itself — the swap alone provably cannot clear the bar, so -re-proposing it unchanged is settled. The open perf question is tracked -separately as a deferred item; this decision governs only the memo-swap -technique. A that-depends-style per-APP-container slot array was explicitly not -pre-authorized here and would need its own measurement. diff --git a/planning/decisions/2026-07-19-child-lazy-alloc-declined.md b/planning/decisions/2026-07-19-child-lazy-alloc-declined.md deleted file mode 100644 index 9f52151e..00000000 --- a/planning/decisions/2026-07-19-child-lazy-alloc-declined.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -summary: Decline lazy-allocating the child-container RLock/CacheRegistry/ContextRegistry — measured saving is ~0 for realistic caching request cycles and ~2-3.5% only for a narrow no-cache child, not worth a resolve hot-path branch plus re-introducing the singleton-creation race. ---- - -# Decline lazy-allocation of child-container registries - -**Decision:** Keep `Container.__init__` eagerly building the per-child `RLock`, -`CacheRegistry`, and `ContextRegistry`. Do not lazy-allocate them. - -## Context - -After the `_next_deeper` memo (`2026-07-19.02`, #348) took ~40% off default -child-build, the remaining per-child cost was three eager allocations — -`RLock` (~195-214 ns), `CacheRegistry` (~217 ns), `ContextRegistry` (~189 ns). -Deferred work proposed lazy-allocating them (construct on first use) to cut -construction cost, with the net (construction saving vs a resolve hot-path -branch) left "untested." This settles it, starting with the strongest candidate, -the `RLock` (REQUEST children rarely create singletons, so it is the allocation -most often wasted). - -## Decision & rationale - -Measured ceiling (current `use_lock=True` vs `False`, which already skips the -RLock alloc; py3.10, guidance): - -- Isolated child build: RLock alloc ≈ **195 ns/child**. -- **Realistic *caching* request cycle** (build child → resolve a REQUEST-cached - resource → close, the C4/G7 shape): saving ≈ **0 (0.4%)** — a caching child - *uses* the lock, so lazy only defers the allocation, and adds a `None`-check. -- Narrow **no-cache child** (transient/APP deps only, no request caching, no - context): saving ≈ **67 ns (3.5%)** — and this is the ceiling, before any cost. - -So the best case is ~0 for the workloads that matter. Real integration request -children inject context (uses `ContextRegistry`) and cache a request-scoped -resource (uses `CacheRegistry` and the `RLock`) — the C4/G7/G9 scenarios all do — -so the trio is *used*, and lazy-allocation saves nothing there while taxing the -hot path. Against a ~0-to-3.5% narrow win, lazy-allocation costs: - -1. A `None`-check on the cached-resolve hot path plus a `_use_lock` slot. -2. **Re-introducing the singleton-creation race the lock exists to prevent** — - lazy lock creation must itself be atomic, so it needs a guard lock or a - CAS-style publish, a new concurrency-correctness surface against the - documented Beta contract - ([design decisions](../../docs/introduction/design-decisions.md#the-thread-safety-boundary)). - -The `CacheRegistry`/`ContextRegistry` variants are *weaker* still: they are used -more often in realistic children, so they save even less. Net negative for a -conservative, zero-dependency library. Decline all three. - -## Revisit trigger - -A profile of a *realistic* request cycle (with context + caching) showing -child-container construction — specifically these allocations, not `_next_deeper` -— dominating, or a user-reported per-request construction bottleneck in a -build-heavy, cache-free workload (deeply nested short-lived scopes). Re-measure -the net against `G6b` + `G1-G3` + `C4/G7/G9`, and solve the lazy-lock atomicity, -before reopening. diff --git a/planning/decisions/2026-07-19-exec-hot-path-declined.md b/planning/decisions/2026-07-19-exec-hot-path-declined.md deleted file mode 100644 index 74abc2b4..00000000 --- a/planning/decisions/2026-07-19-exec-hot-path-declined.md +++ /dev/null @@ -1,55 +0,0 @@ ---- -summary: Re-decline exec codegen on the resolve hot path — the reframe that "zero-dependency" was never the real objection holds, but the bounded 1.3-1.9x prize (high-arity/deep-chain only) does not clear the standing maintainability and free-threading costs. ---- - -# Re-decline exec codegen on the resolve hot path - -**Decision:** Keep the shipped closure-compiled resolver as the single resolve -path. Do not add `exec`-based source-generation codegen, additive or otherwise. - -## Context - -The codegen-ceiling item filed the remaining 1.3-1.9x gap behind -`dishka`/`wireup` on transient and deep-chain graphs as "rejected for a -zero-dependency library" — a stance, not a task. This reopened that stance to -check its ground: `exec` is a stdlib builtin, not a dependency, so -`dataclasses`/`attrs`/`cattrs`-style `exec` codegen would not touch the -zero-*dependency* guarantee at all. That reframe was accepted; what defeated the -proposal was the bounded size of the prize, below. - -## Decision & rationale - -The reframe holds — "it adds a dependency" was never the real objection, once -unbundled the objection separates into four claims: - -- Debuggability — mitigable, but only via the attrs `linecache` discipline - (script-builder, hygiene rules, unique-filename scheme). -- Maintainability / audit trust — real, no neutralizer; a fixed standing cost - and a second mental model, independent of how small the win is. -- Free-threading / nogil — real and open, modern-di-specific; swaps captured - cells for generated-module globals under a concurrency contract still at - Beta, and cannot be retired without out-of-scope parallel-resolution work. -- Deployment / exec bans — mitigable via an additive fallback resolver, but - that fix doubles the resolve surface and deepens the maintainability cost - rather than escaping it. - -The perf gate bounds the prize before any of that: `exec` is 0-4% faster than -a hand-unrolled closure at fixed arity (inside the noise band), with its only -exclusive win — ~1.3-1.9x — confined to high-arity nodes and deep -singleton/scoped chains, where closures already capture ~80-90% of the -ceiling. Every path that neutralizes an objection pays for it in the -maintainability row, and dissolving the dependency-purity framing manufactures -no win the measurement denies. - -**Holding: re-decline.** The shipped closure-compiled resolver stays the -single resolve path; `exec` codegen stays out, additive or otherwise. - -## Revisit trigger - -A user-reported, real-world resolve bottleneck on a high-arity node or a deep -singleton/scoped chain — the two forms where `exec` could pay — that the closure -resolver provably cannot close. A synthetic micro-benchmark or a hypothetical -does not qualify. - -*Linked from the codegen-ceiling half of -[`warm-singleton-perf-headroom`](../deferred/2026-07-17-warm-singleton-perf-headroom.md).* diff --git a/planning/decisions/2026-07-19-no-static-wiring-checker.md b/planning/decisions/2026-07-19-no-static-wiring-checker.md deleted file mode 100644 index 4c5fafb7..00000000 --- a/planning/decisions/2026-07-19-no-static-wiring-checker.md +++ /dev/null @@ -1,65 +0,0 @@ ---- -summary: No static/compile-time wiring checker and no mypy/pyright/ty plugin; opt-in runtime validate() plus declaration-time signature parsing is the deliberate model. ---- - -# No static / compile-time wiring checker - -**Decision:** modern-di ships no static or compile-time dependency-graph checker -and no type-checker plugin (mypy, pyright, or `ty`). Whole-graph verification -stays the opt-in runtime `validate()`, backed by declaration-time signature -parsing that already fails early on an unwireable creator. - -## Context - -The 2026-07-19 DI static-analysis research examined how -the wider field verifies wiring *before* runtime, to decide whether modern-di -should add a static safety net — an opt-in mypy/pyright/`ty` plugin or a -richer statically-checkable API — on top of runtime `validate()`. Three verified -findings framed the call: - -1. **True compile-time wiring verification exists only in compiled-language - toolchains** — Dagger's annotation processor, Google Wire's build-time - codegen, and Koin's K2 compiler plugin (GA June 2026). Each is a heavyweight - toolchain, not a library artifact. Angular's headline "no provider" (NG0201) - is a runtime error; .NET's built-in scope validation is runtime/startup; - Spring's autowiring correctness is an IDE inspection. Runtime/startup - validation — modern-di's `validate()` model — is the mainstream field - standard, not a second-class fallback. -2. **Where compile-time validation exists, it is positioned as a *replacement* - for runtime verification, not an extension.** Koin's own docs tell users they - can delete their `verify()`/`checkModules()` tests once the compiler plugin is - on. A static layer for modern-di would therefore duplicate `validate()`, not - add reach beyond it. -3. **A Python type-checker plugin is infeasible for a conservative zero-dep - library.** pyright refuses third-party plugins on principle (cross-checker - breakage, distribution/maintenance, security of downloaded code); `ty` — the - checker modern-di itself uses — has no plugin system (astral-sh/ty#291 closed - "not planned"), with Astral building framework support natively instead; only - mypy exposes a plugin API, and it is documented as experimental, with - backwards-incompatible changes shipped without a deprecation period. - -## Decision & rationale - -Rejected. A static checker would, at best, **duplicate `validate()`** (finding -2) while **only serving mypy users** and carrying a **permanent maintenance -liability against an unstable, experimental plugin API** (finding 3) — and it -would not even help modern-di's own `ty` toolchain, which has no plugin seam. It -buys no reach that the opt-in runtime `validate()` (missing providers, -scope-direction violations, cycles, all-errors aggregated) plus declaration-time -`UnsupportedCreatorParameterError` does not already provide, at real cost to the -zero-dependency and conservative-feature-set constraints. Every genuine static -net in the field is a compiled-language toolchain artifact (finding 1), which a -pure-Python library cannot cheaply emulate. - -The one in-constraint win the research pointed at — injection markers that -type-check to the concrete `T` — modern-di already ships: `resolve(type[T]) -> T` -and `integrations.py`'s `Annotated[T, from_di(dep)]` both preserve the concrete -static type, matching dishka's `FromDishka[T]`. That strength is documented -(comparison.md, design-decisions.md non-goals), not extended with a checker. - -## Revisit trigger - -`ty` (or pyright) ships a **stable, supported** third-party plugin API **and** a -concrete user-reported wiring-safety need that runtime `validate()` demonstrably -cannot meet (e.g. per-call-site checking without executing `validate()`). Both -conditions, not either alone. diff --git a/planning/decisions/2026-07-20-except-body-creator-error-helper.md b/planning/decisions/2026-07-20-except-body-creator-error-helper.md deleted file mode 100644 index b3150ba6..00000000 --- a/planning/decisions/2026-07-20-except-body-creator-error-helper.md +++ /dev/null @@ -1,55 +0,0 @@ ---- -summary: Accept an except-body-only helper for the creator-call TypeError rule — it dodges the frame cost that made the drift-lock bundle reject a shared helper, because the moved code runs only on the raise path. ---- - -# Extract the creator-call error rule via an except-body-only helper - -**Decision:** Centralize the creator-call `TypeError` rule in one -`CreatorCallError.from_type_error` classmethod, called from inside each site's -`except TypeError` block. This supersedes the "no shared helper" non-goal -recorded by the earlier creator-call-error drift-lock work. - -## Context - -The earlier creator-call-error drift-lock work (2026-07-17) faced the same -four-copy duplication of the rule "a binding `TypeError` becomes `CreatorCallError` with a -prepended step; a `TypeError` from inside the creator body propagates unchanged." -It explicitly rejected deduping: *"Do not extract a shared helper — that -reintroduces the frame the inlining removed,"* and locked the copies with a -cross-path equivalence test instead. - -That rejection weighed exactly one helper shape: a helper wrapping the whole -`creator(...)` call, which adds a Python frame on **every** resolve — the success -path the single-path compiled resolver (PR #334) exists to keep frame-free. - -The option it did not weigh: extract only the `except`-body — the `tb_next` -discriminate, the `CreatorCallError` construction, and the `prepend_step` — while -leaving `try: return creator(...)` at each site. That code runs only when the -creator call raises `TypeError`, i.e. on the already-failing raise path, never on -success. - -## Decision & rationale - -Chose the except-body helper. The drift-lock objection is real but scoped to -whole-call wrapping; it does not bind the except-body form. Under this form: - -- The success (hot) path stays `return creator(*args)` byte-for-byte — no frame - is restored. A `--benchmark-compare-fail=mean:5%` resolve-bench gate confirms - it empirically before ship. -- The rule gets one home (locality); changing it is one edit, not four. -- The equivalence test that existed only to police the copies is retired — one - source cannot drift from itself. -- Traceback fidelity is preserved: the return-or-`None` contract keeps the bare - `raise` at each site, so a creator-body `TypeError` propagates with its - traceback unchanged. - -The narrower frame concern the drift-lock work protected is honored, not -overridden — the reversal applies precisely because the except-body form -sidesteps it. - -## Revisit trigger - -The resolve hot path regresses after this lands (meaning the success path was not -as frame-free as argued), **or** a future change needs the creator-call rule to -differ per site again (making a single shared rule wrong) — either reopens the -single-home decision. diff --git a/planning/decisions/2026-07-25-d1-d4-derived-inherent.md b/planning/decisions/2026-07-25-d1-d4-derived-inherent.md deleted file mode 100644 index bffe6bea..00000000 --- a/planning/decisions/2026-07-25-d1-d4-derived-inherent.md +++ /dev/null @@ -1,65 +0,0 @@ ---- -summary: D1 (one-call setup) and D4 (quickstart length) sub-2 scores are arithmetic consequences of the already-inherent @inject (D5) and caller-owned root lifecycle (D3) — no independent quickstart or setup fix; the audit's item-5 one-line trim does not hold. Post-rollout blessed-ready count is 4 (the ceiling under the inherent rulings). ---- - -# D1/D4 sub-2 scores are derived — no independent fix - -**Decision:** The D1 and D4 scores below 2 in the 2026-07-22 blessed-ready -on-ramp audit are arithmetic consequences of two facts already ruled inherent — the `@inject` -requirement ([D5](2026-07-25-inject-asymmetry-inherent.md)) and the caller-owned -root lifecycle ([D3](2026-07-25-d3-root-lifecycle-inherent.md)) — not independent -gaps. No quickstart or `setup_di` changes; the audit's §4 item-5 "trim one line" -does not hold. - -## Context - -D1 scores one-call setup (distinct wiring actions); D4 scores steps-to-first- -dependency (`L`, the DI-specific lines in a minimal single-dependency quickstart, -2 = `L` ≤ 7). The audit's §4 backlog carried D4 items (5, and the D4 halves of 8, -9) and D1 items (7, 8, 9) as candidate "quickstart trims." This checks whether any -is an independent fix once D3 and D5 are ruled inherent. - -## Decision & rationale - -**D1 < 2 is D3, restated.** Only flask, grpc, typer score D1=1, and in each the -second action is the manual root `open()` — the exact caller-owned-root fact ruled -inherent in the D3 decision. The audit itself notes flask's "D1=1 and D3=1 share -one root cause." No independent D1 fix exists. - -**D4 < 2 is D5 + D3, by line count.** The decorator-free floor is `L`=7 (two -imports, a `Group` + one provider + its dependency, `Container(...)`, `setup_di`). -Verified against the merged examples: - -- **aiogram, aiohttp, arq** — `L`=8 = the floor **+ the `@inject` line** (D5, - inherent). The audit's item 5 ("trim one line, D5 not part of it") is wrong: the - only line over 7 *is* `@inject`. -- **flask, typer** — `L`=9 = floor + `@inject` + the manual root `open()`/`with` - (D3, inherent). -- **grpc** — `L`=10 = floor + `@inject` + manual `open()` + `close_sync()` (D3, - inherent); the lone D4=0. - -Nothing is trimmable without deleting an inherent element. A minimal -single-dependency example needs both providers (a service *and* the thing it -depends on) to demonstrate DI at all, so the floor itself cannot drop. - -**The scores are also mostly moot.** The verdict rule is D1=D2=D3=2 and no `0`; -**D4=1 and D5=1 never block.** So once the canonical-example rollout fixed D2/D6, -the D4=1 rows are not held back by D4 at all — only grpc's D4=0 is a dimension- -zero, and grpc is already blocked by its inherent D1/D3. - -**Corrected blessed-ready count: 4.** Post-rollout, litestar plus **aiogram, -aiohttp, arq** clear the verdict (D1=D2=D3=2, no zero; their D4=1/D5=1 do not -block) — up from 1 at audit time. That is the **ceiling** under the inherent -rulings: the other eight are each gated by a framework-inherent **D3** lifecycle -score (or, for flask/grpc/typer, D1 too), every one with a revisit trigger in the -D3/D5 records. No integration reaches blessed-ready by a quickstart edit. - -**Consistency.** Same conclusion as the D3, D5, and -[exec](2026-07-19-exec-hot-path-declined.md) records: a sub-2 score reflecting a -framework limitation, not a modern-di gap, is documented, not engineered away. - -## Revisit trigger - -Whichever underlying record reopens: if `@inject` (D5) or the caller-owned root -lifecycle (D3) becomes avoidable for an integration, its D1/D4 improve for free. -No independent D1/D4 trigger. diff --git a/planning/decisions/2026-07-25-d3-root-lifecycle-inherent.md b/planning/decisions/2026-07-25-d3-root-lifecycle-inherent.md deleted file mode 100644 index 9ba734b0..00000000 --- a/planning/decisions/2026-07-25-d3-root-lifecycle-inherent.md +++ /dev/null @@ -1,97 +0,0 @@ ---- -summary: The 8 integrations scoring D3<2 in the blessed-ready audit stay as-is — the root-lifecycle gaps are inherent framework-lifecycle limits plus the deliberate caller-owns-root contract, not fixable ergonomics; typer's callback fix considered and deferred. ---- - -# D3 root-lifecycle gaps are inherent — no integration code changes - -**Decision:** The 8 integrations scoring D3<2 (lifespan handled for the user) in -the 2026-07-22 blessed-ready on-ramp audit keep their current lifecycle handling. The gaps are inherent framework-lifecycle -limits plus the deliberately-chosen caller-owns-root contract, so the treatment -is this documented rationale, not integration code. - -## Context - -The audit's **D3** dimension scores whether `setup_di` owns *both* the root -open/close and the per-unit-of-work child, with no documented deployment context -where the root fails to open. Its bar gives **1** when one side is owned or a -documented root-open caveat exists, and **0** when the user wires both sides. Of -the 12 integrations, four clear it (aiogram, aiohttp, arq, litestar); the other -eight are below, in two shapes. The question this reopens: are any of the eight a -*fixable* ergonomics gap, or are they inherent framework behavior the score -merely reflects. - -Per-integration verdict (fixable vs inherent): - -| Integration | D3 | Root open/close | Why inherent | -|---|---|---|---| -| fastapi | 1 | `setup_di` owns both | ASGI lifespan is optional — a mounted sub-app / `lifespan="off"` never fires it; a real, documented deployment caveat, not a code gap | -| starlette | 1 | `setup_di` owns both | Same ASGI-lifespan-optional caveat | -| faststream | 1 | `setup_di` owns both | `TestBroker`/`TestApp` deliberately skip `on_startup`; a test-mode caveat, documented | -| taskiq | 1 | `setup_di` owns both | `run_receiver_task(run_startup=False)` skips the startup hook by default; documented | -| celery | 1 | root owned; per-task child owned by `@inject`/`DITask` | `task_always_eager` bypasses the worker signals that open the root; documented | -| flask | 1 | child owned; root is the caller's | Flask has **no app-shutdown hook**, so the root *close* is unavoidably the caller's | -| grpc | 1 | per-RPC child owned; root is the caller's | The server's `start()`/`stop()` is caller-owned; the integration's seam is the interceptor, not the server lifecycle | -| typer | 0 | neither owned by `setup_di` | Callback hook *does* exist (the one fixable case) — see below | - -## Decision & rationale - -**Category A (fastapi, starlette, faststream, taskiq, celery) — nothing to fix.** -`setup_di` already owns both sides; each scores 1 *only* because a documented -execution-context caveat exists, and every one of those caveats is real framework -behavior: ASGI's lifespan scope is optional, test brokers deliberately skip -startup hooks, `run_startup` defaults off, eager execution bypasses worker -signals. No code closes a caveat the framework itself imposes. These are already -captured in the [lifecycle rules](../../docs/integrations/writing-integrations.md#lifecycle-rules) -and in the per-integration deployment caveats on each integration page. - -**Category B (flask, grpc) — root ownership is the caller's, by design.** Flask -has no app-shutdown hook and gRPC's server lifecycle is caller-owned, so the root -*close* is inherently the caller's. Owning the root *open* in Flask's `setup_di`, -or adding a gRPC server-wrapper helper, would add machinery and revisit the -contract the lifecycle rules already state deliberately — *"if the framework -offers no lifecycle hook at all, the root's open/close is the caller's to own — -document it."* The gain (removing one `open()`/`with` line the caller writes -once) does not justify a new contract or new API surface, against the -conservative-feature-set principle. - -**typer — the one fixable case, deferred.** typer is the lone D3=0 and the only -one with an available hook (a Typer/Click callback could open the root and close -it via `ctx.call_on_close`). It is declined for now because: - -- The 0 is largely a **scoring artifact** — the command child *is* owned (inside - `@inject`'s `_build_command_container`); only the root is left to the caller, - and an explicit `with container:` is the right idiom for a short-lived CLI - process, not a footgun. -- Auto-owning the root via an integration-injected callback adds hidden control - flow and must compose with a user's own `@app.callback()` (non-trivial in - Click) — real machinery to replace a one-line `with container:` on a process - that exits in milliseconds. - -**Consistency.** This is the same call as `@inject` being ruled inherent (audit -§2 D5: no framework exposes an unused injection seam) and the -[exec hot-path re-decline](2026-07-19-exec-hot-path-declined.md): keep the -conservative feature set and the explicit lifecycle; document the deliberate -stance rather than add machinery to move a score the framework, not modern-di, -holds down. - -## Revisit trigger - -A real user reporting friction with a specific integration's root-lifecycle -ergonomics — most plausibly the typer CLI, where the callback fix would then be -worth its composition cost. Also: if a Category-B framework gains a -startup/shutdown lifecycle hook it currently lacks, its `setup_di` should own the -root and this row is reopened. - -## Amendment (2026-07-26) - -The revisit trigger above fired: a maintainer-reported root-lifecycle friction — -the hard `ContainerClosedError` failure mode every documented caveat in this -audit relies on — was addressed on 2026-07-26 by making `open()` optional in the -core (`modern-di` 3.1: a root container is open from construction, and reusing -one after an explicit close warns and reopens instead of raising). That fix -landed in `modern_di.Container` itself, not in any -integration's `setup_di`/lifecycle wiring, so this decision's conclusion — -**no integration code changes** — still stands: every caveat this audit -documented changes failure mode (from a hard raise to "finalizers silently -don't run") rather than disappearing, and the per-integration deployment notes -were reworded to say so, but no integration's lifecycle code changed. diff --git a/planning/decisions/2026-07-25-inject-asymmetry-inherent.md b/planning/decisions/2026-07-25-inject-asymmetry-inherent.md deleted file mode 100644 index 10986f73..00000000 --- a/planning/decisions/2026-07-25-inject-asymmetry-inherent.md +++ /dev/null @@ -1,78 +0,0 @@ ---- -summary: The @inject asymmetry (4 integrations decorator-free, 8 require @inject) is inherent, not an adapter gap — @inject is required exactly where the host framework exposes no per-parameter provider seam; do not unify, document the rationale. ---- - -# The @inject asymmetry is inherent — do not unify - -**Decision:** The four integrations that resolve `FromDI` decorator-free -(fastapi, litestar, faststream, taskiq) and the eight that require `@inject` -(flask, starlette, aiohttp, celery, arq, aiogram, typer, grpc) keep their current -shapes. The split is inherent — an adapter can drop `@inject` only where the host -framework offers a per-parameter provider seam, and the eight offer none — so -there is nothing to unify. Treatment is this documented rationale. - -## Context - -The 2026-07-22 blessed-ready on-ramp audit's -**D5** dimension scores whether a handler needs a DI decorator: **2** = none, **1** -= `@inject` required but inherent (no framework seam), **0** = `@inject` required -but a seam exists the adapter fails to use. The audit found the split is **4 vs 8** -(not the adoption research's stale "7 of 12"), and — the substantive question this -records — **every one of the eight is a 1, never a 0**: no framework exposes an -unused seam. - -The seam test: `FromDI` can live as a bare parameter *default* (no decorator) -only where the framework itself evaluates a parameter's default as a provider. - -| Integration | D5 | Per-parameter provider seam | Verdict | -|---|---|---|---| -| fastapi | 2 | `fastapi.Depends` | decorator-free | -| litestar | 2 | `Provide` | decorator-free | -| faststream | 2 | `faststream.Depends` | decorator-free | -| taskiq | 2 | `TaskiqDepends` | decorator-free | -| flask | 1 | none — view is a plain callable, no `Depends` | inherent | -| starlette | 1 | none — endpoint is a plain ASGI callable | inherent | -| aiohttp | 1 | none — handler is `async def handler(request)` | inherent | -| celery | 1 | none — task is a plain callable with its own args | inherent | -| arq | 1 | none — `coroutine(ctx, …)`, `ctx` a plain dict | inherent | -| aiogram | 1 | name-based `data` injection, **not** provider-evaluation (closest call) | inherent | -| typer | 1 | none — defaults are CLI parsing (`Option`/`Argument`) | inherent | -| grpc | 1 | none — fixed `(request, context)` servicer signature | inherent | - -## Decision & rationale - -`@inject` is required in the eight **precisely because** their frameworks call a -handler with a fixed signature and expose no per-parameter provider-evaluation -hook. Where such a hook exists (the four), the adapter binds `FromDI` to it and -the decorator disappears; where it does not, `@inject` is the only way to read the -markers off the signature and resolve them. An adapter cannot manufacture a seam -the framework does not have. So the asymmetry is a property of the host -frameworks, not a modern-di shortfall, and "unify the eight to decorator-free" is -not achievable in code. - -**aiogram is the one close call.** aiogram *does* have name-based contextual-data -injection (a middleware `data` dict matched to handler kwargs by parameter name), -which the adapter uses to pass the child container. But it matches by name and -never evaluates a parameter *default* as a provider the way `Depends`/`Provide` -do, so it cannot consume a `FromDI` marker — `@inject` is still required. Ruled 1, -not 0. - -**Positioning follows from this.** The defensible claim is *no `@provide` ever, and -no `@inject` in the four biggest integrations* (where dishka needs `@inject` even -for FastAPI/Litestar), not "decorator-free" unqualified — which a single `grep` -refutes. The adapter-side `auto_inject` (Flask) and `DITask` (Celery) helpers apply -`@inject` under the hood for convenience; they are not framework seams and do not -change the verdict. - -**Consistency.** Same call as the -[D3 root-lifecycle gaps ruled inherent](2026-07-25-d3-root-lifecycle-inherent.md) -and the [exec hot-path re-decline](2026-07-19-exec-hot-path-declined.md): where a -sub-2 score reflects a framework limitation rather than a modern-di gap, document -the deliberate stance instead of adding machinery to move it. - -## Revisit trigger - -A Category-8 framework gains a per-parameter dependency hook it currently lacks -(e.g. a future Flask/Starlette DI feature) — its integration should then bind -`FromDI` to that hook and drop `@inject`, reopening its row. Or a user reports the -`@inject` requirement as real adoption friction in a specific integration. diff --git a/planning/decisions/2026-07-26-explicit-only-validation.md b/planning/decisions/2026-07-26-explicit-only-validation.md deleted file mode 100644 index abfae2db..00000000 --- a/planning/decisions/2026-07-26-explicit-only-validation.md +++ /dev/null @@ -1,87 +0,0 @@ ---- -summary: Validation is explicit-only — `validate()` is the sole trigger. The alternative that kept it implicit (splitting the walk into monotone checks at construction and completeness at first use) was implemented, measured, and discarded: its machinery was out of proportion to the guarantee, and the cheap version taxes the resolve hot path for a startup-time concern. ---- - -# Validation is explicit-only; implicit validation was built and discarded - -**Decision:** `container.validate()` is the only thing that walks the graph. -Neither `__init__` nor `open()` nor `add_providers` nor `resolve()` ever -validates, and `Container(validate=...)` is a deprecated no-op. This records -the design that was tried instead and why it lost. - -## Context - -3.0 (tagged 2026-07-20) made `open()` mandatory and made it the sole validation -trigger. Both tightenings caused trouble: - -- Mandatory `open()` produced six production defects across integrations, all - one root cause — the root's open hook does not fire in some execution - contexts, so the first unit of work raises. -- Binding validation to `open()` produced an authoring rule that existed only - because of that binding: open the root *after* `setup_di`, or a by-type - dependency on a not-yet-registered connection fails validation. - -The question was whether to keep an implicit safety guarantee at all, and if so -how to pay for it. - -## Decision & rationale - -An earlier revision of this change kept validation implicit by **splitting the -walk in two**: - -- **Cycles and inverted scopes, eagerly at construction.** These are *monotone* — - registering more providers can only ever add such an error, never remove one — - so they are safe to check before the graph is complete. -- **Completeness (missing dependencies, dangling aliases), held on the shared - registry and raised at first use**, since it is the only class of error that a - later `add_providers` can legitimately fix. - -It was implemented and it worked. The cost was the machinery it dragged in: - -1. A two-flag container lifecycle, so a fresh container could be told from a - closed one. -2. Validation state parked on `ProvidersRegistry`. -3. A monotone/completeness classification threaded through the graph walk. -4. An `add_providers` rollback path, so a batch that broke the graph could be - undone. - -That is a large, permanent surface for a startup-time property. And the cheap -way to keep the implicit guarantee *without* it — a per-resolve check — taxes -the hot path for a concern that only matters once, at boot. - -**Maintainer ruling: make the trigger explicit and predictable instead.** -`validate()` is one call, it reports every wiring bug at once, and nothing about -when it runs has to be inferred. `add_providers` becomes a plain register with -no rollback; the mutation clears `_validated`, so the next explicit `validate()` -re-walks. `ProvidersRegistry` keeps `_validated` purely as a memo of a clean -walk — it gates nothing — and it still short-circuits `resolve_provider`'s -`RecursionError`-to-`CircularDependencyError` guard. - -The measurement that came with the ruling: because 3.0 ran a default -`validate=True` graph walk at `open()`, dropping it made construction markedly -cheaper — roughly **2.6 µs against 15.5 µs** for a depth-6 chain -(`Container(...)` + `open()`, default arguments), matching the -`test_g10_validate_deep_chain` guard cost that 3.0's `open()` paid. The resolve -tier was unchanged, as expected: compiled resolvers keep the identical single -`if target.closed:` test per code path, with only the body swapped. The ad hoc -benchmark scenario that produced the construction figure was deleted along with -the split-validation design it existed to measure. - -Shipped as 3.1.0. Every 3.0 pattern keeps working; no integration needed a code -change. `Container(validate=...)` stays accepted-and-ignored (with -`ValidateArgumentWarning`) until 4.0 specifically because 3.0 callers pass -`validate=False` widely, including this repo's own benchmark guards. - -**The accepted cost:** the default safety posture drops silently. A broken graph -previously raised at `open()`; now it surfaces from an explicit `validate()`, or -at resolve time as `ArgumentResolutionError`. Nothing that worked breaks, but a -user who believed the default protected them loses that. The deprecation warning -plus the release notes and integration docs carry the replacement idiom. - -## Revisit trigger - -Reports of graphs reaching production broken in a way an implicit walk would -have caught at boot — i.e. evidence that opt-in `validate()` is under-adopted in -practice. Re-open with the adoption evidence, not with a new mechanism: the -split-validation design is settled, and any replacement must avoid both the -four-part machinery above and a per-resolve check. diff --git a/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md b/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md deleted file mode 100644 index 287d360c..00000000 --- a/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md +++ /dev/null @@ -1,83 +0,0 @@ ---- -summary: Decline opt-in DEBUG resolution tracing — the `isEnabledFor` front-guard the shape depends on measures ~19ns against a ~120-140ns per-node budget, costing +11% to +37% on every resolve for every user, worst on the warm cached hit nobody opts out of. ---- - -# Decline opt-in DEBUG resolution tracing - -**Decision:** Do not add a module-level `logging.getLogger("modern_di")` that -narrates resolution at DEBUG level. No resolution tracing ships in any form — -neither the runtime-guarded logger nor the compile-time-gated variant that would -avoid its cost. - -## Context - -The deferred item proposed narrating resolution at DEBUG level: resolve start, -cache hit against creator call, override short-circuit, context reads, and -finalizer order at close. Field precedent was real — Uber Fx narrates lifecycle -events, Koin exposes an opt-in `logger(Level.DEBUG)` — and both treat "why did -the container do that" as a first-class diagnostic. A *pluggable structured -event-logger subsystem* (Fx's `fxevent.Logger`, Koin's Logger abstraction) had -already been rejected against the conservative-feature-set principle; the -narrowed shape that survived was stdlib logging and nothing else. - -That shape rested on one cost estimate: "one `isEnabledFor(DEBUG)` boolean per -chokepoint on the hot path, plus 5-8 log statements." The estimate was never -measured. This decision measured it. - -## Decision & rationale - -The guard is not a boolean. `logger.isEnabledFor(DEBUG)` is an attribute load -plus a dict lookup inside a `try`, and it measures **~19ns net** (21.2ns against -a 2.25ns loop floor) — roughly **10x** a bare module-global bool check (~1.8ns -net). Against a per-node budget of ~120-140ns, one guard is ~15% of a node, and -a cached factory needs two (resolve-start and cache-hit). - -Measured by patching the shipped closures in `resolver_compiler.py` with exactly -the proposed design and re-running the guard tier, **with tracing off** — logging -never configured, i.e. the cost every user pays for a feature they never enable: - -| Scenario | base | traced | delta | -|---|---|---|---| -| G2 cached resolve (warm hit) | 140 ns | 192 ns | **+37%** | -| G16 by-type resolve | 181 ns | 237 ns | **+31%** | -| G4 wide, 10 siblings | 1333 ns | 1709 ns | **+28%** | -| G17 by-type, 200-provider registry | 188 ns | 235 ns | +26% | -| G12 override active, depth 6 | 1017 ns | 1187 ns | +17% | -| G3 deep chain, depth 6 | 833 ns | 958 ns | +15% | -| G9 context resolve | 625 ns | 708 ns | +13% | -| G1 transient | 333 ns | 375 ns | +13% | -| G5 cross-scope | 375 ns | 417 ns | +11% | - -Two properties make this land where it hurts most. The cost falls hardest on the -**warm cached hit** — the cheapest operation and the one the singleton pattern -makes the most common. And it **multiplies by graph size**, since every node runs -its own guards: G4's +376ns is 11 nodes each paying. - -A compile-time gate was available and would have been free. Resolvers are -compiled closures memoized on the registry, and `ProvidersRegistry._invalidate()` -already exists to drop them, so `compile_resolver` could emit a traced or a plain -closure and the off-cost would be exactly zero. It was declined too, on the -feature-set principle rather than on cost: it buys back the nanoseconds by making -activation an explicit modern-di call that invalidates the resolver memo, so the -feature stops being "stdlib logging" — the one property that justified the -narrowed shape over the event subsystem already rejected — and becomes a second -public activation API plus a compile mode to keep correct forever. Paying that to -restore an estimate that measurement had already broken is not a trade worth -taking. - -**Holding: decline.** Resolution stays untraced. Diagnostics remain the job of -the error messages, which already carry the resolution breadcrumb chain -(`architecture/`, since removed, and `docs/troubleshooting/`) at zero hot-path cost. - -## Revisit trigger - -A user-reported diagnostic dead end that the existing breadcrumb chain provably -cannot answer — a real issue where the reporter and the maintainer both failed to -determine *why* the container resolved as it did from the error alone. A -hypothetical, or a preference for narration over breadcrumbs, does not qualify. - -*Measured on Python 3.14.6, Apple M4 (`perf_counter` resolution 41.67ns). Guard-tier -medians are quantized to one timer tick, so a single delta carries that granularity; -the direction and magnitude held across all nine scenarios and a repeat run.* - -*Declined from the deferred item `2026-07-05-debug-resolution-tracing`.* diff --git a/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md b/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md deleted file mode 100644 index e4d7c0f5..00000000 --- a/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md +++ /dev/null @@ -1,55 +0,0 @@ ---- -summary: Declined inlining the ContextProvider resolver into its compiled closure — it freezes cp.scope against a live mutation, buys 0 ns on the factory-dependency path that actually matters, and was dominated by a one-line find_context change that is faster on both paths. ---- - -# Declined: inlining the ContextProvider resolver into its closure - -**Decision:** `_compile_context_provider` keeps delegating to the bound -`ContextProvider.resolve`. Its body is not inlined into the compiled closure. - -## Context - -`_compile_context_provider` is the one Factory-style resolver that does not -inline its own body: it delegates to `cp.resolve`, which calls -`fetch_context_value`, which calls `find_container` (always — with no same-scope -int-compare fast path, unlike every Factory closure) and then `find_context`. -An automated inventory measured a direct `ContextProvider` resolve at ~233 ns -against a plain factory's ~196 ns, with 8 Python calls against 4, and proposed -capturing `cp.context_type` and `cp.scope` at compile time to remove four frames. - -## Decision & rationale - -Three independent reasons, any one sufficient. - -**It freezes `cp.scope` against a mutation that is live at the time.** Capturing -the scope in the closure is only sound if the scope cannot change after compile. -It could: `AbstractProvider._stamp_group_scope` mutated `provider.scope` and -touched no registry, so `_invalidate()` never fired and the memoized resolver was -never dropped. A `Group` subclass declared after first resolve could restamp a -shared `ContextProvider` from APP to REQUEST; today's delegating resolver raises -`ScopeNotInitializedError`, and the inlined one would return a silently stale -value. That hazard has since been closed at its source — documented at the time -in `architecture/providers.md` (since removed) and enforced by -`ProviderScopeFrozenError` — but it was closed by *freezing the scope at -registration*, not by making the capture safe in general, and it was found while -refuting this candidate rather than before proposing it. - -**It buys nothing where it matters.** The measured win is on a *direct* -`resolve_provider(context_provider)` call. On the factory-dependency path — a -`Factory` with a context kwarg, which is what every integration actually does — -the gain measured **0 ns**, because that path goes through -`Factory._resolve_context_value`, not through the compiled ContextProvider -resolver at all. - -**It was dominated.** A one-line change to `ContextRegistry.find_context` -(dropping a `typing.cast` and a dead `None` check) was faster on *both* paths for -a net **-1 line**, against this candidate's +27 lines and 0 deleted. That shipped -instead; see the `find_context` PR body for its numbers and for why the -`.get(key, UNSET)` variant was rejected in turn. - -## Revisit trigger - -A profile from a real integration shows direct `ContextProvider` resolution — not -context *kwargs* on a factory — as a measurable cost, **and** the scope capture -can be licensed by something stronger than the current freeze-at-registration -rule. Absent the first, this optimises a path nobody takes. diff --git a/planning/decisions/2026-08-01-scope-map-inline-declined.md b/planning/decisions/2026-08-01-scope-map-inline-declined.md deleted file mode 100644 index a6aecf00..00000000 --- a/planning/decisions/2026-08-01-scope-map-inline-declined.md +++ /dev/null @@ -1,58 +0,0 @@ ---- -summary: Declined inlining Container._scope_map at the four resolver navigation sites — the measured ~24% cross-scope win requires bypassing find_container, a blessed extension point, and silently relocates cached-singleton and finalizer ownership for any Container subclass that overrides it. ---- - -# Declined: inlining `_scope_map` at the resolver navigation sites - -**Decision:** The compiled resolvers keep calling `_navigate` → -`Container.find_container` for a cross-scope hop. The `_scope_map` lookup is not -inlined into the closures. - -## Context - -Same-scope dependencies skip navigation via an int compare, but every -*cross-scope* node pays a `_navigate` frame plus a `find_container` frame. Four -of the inventory's lenses independently proposed replacing the ternary at the -four navigation sites with an inlined `container._scope_map.get(scope)`, falling -back to `_navigate` only on a miss — the same hand-inlined-memo-hit pattern -already used for `resolver_for` and `fetch_cache_item`. Measured cross-scope -resolve 185.4 → 140.7 ns (**-24%**), with a flat same-scope control. - -## Decision & rationale - -The measurement is real and was reproduced. It is declined on the invariant, not -the number. - -**The invariant audits a function body while the code being changed is a -dispatch.** Every formulation justified itself by what `find_container`'s body -does. But `find_container` is a public method on a subclassable class, and -`Container.__init__` builds children via `self.__class__`, so a subclass is -carried down the whole container tree. Inlining the hit path means a subclass -that overrides `find_container` is **silently bypassed** — its override runs on -the miss path only. `unittest.mock.patch.object` makes this visible directly: -calls recorded on the override go from `['APP', 'APP']` to `[]`. - -**The consequence is worse than a missed hook.** The container returned by -navigation is the one whose `cache_registry` receives the singleton. Bypassing an -override that redirects navigation therefore relocates *cached-instance -ownership*, so a different container's `close_async()` runs that instance's -finalizer. That is a lifecycle bug, not a perf regression, and nothing in the -suite would catch it. - -**It contradicts a standing decision.** `find_container` is a blessed extension -point; -[`2026-07-15-provider-facing-seam-declined.md`](2026-07-15-provider-facing-seam-declined.md) -rests on that seam existing. Demoting it is a design change, and it should be -argued on its own terms rather than absorbed as a side effect of an optimisation. - -**It also failed the gates as submitted**: coverage 99% (two unreachable lines -where the fallback never fires) and `lint-ci` red, at +20 lines with none -deleted, and six new rules a maintainer must hold. `ContextProvider` would still -call `find_container`, leaving two navigation conventions in one codebase. - -## Revisit trigger - -`find_container` stops being an extension point — an explicit decision that -`Container` subclasses may not redirect navigation, with the lifecycle -consequence above stated and accepted. Then this becomes a plain inlining and the -measured 24% is available. diff --git a/planning/decisions/2026-08-03-alias-binds-nothing.md b/planning/decisions/2026-08-03-alias-binds-nothing.md deleted file mode 100644 index 6e66ff25..00000000 --- a/planning/decisions/2026-08-03-alias-binds-nothing.md +++ /dev/null @@ -1,79 +0,0 @@ ---- -summary: The alias hop inlines its source lookup but caches nothing and captures nothing — binding the source resolver into the closure measured faster still, but buys a new invalidation invariant, and capturing the registry made it the one resolver forming a cycle with the memo that holds it. ---- - -# The alias hop inlines, but binds nothing - -**Decision:** `_compile_alias`'s closure reads `container.providers_registry` per -resolve and inlines both the source lookup and the source's resolver-memo read. -It holds no reference to the source, its resolver, or the registry. - -## Context - -`Alias` was the one compiled closure that did not reach its dependency's resolver -by direct reference. It called `Alias._find_source` → `find_provider`, then -`Container.resolve_provider` — four Python frames per hop where a `Factory` -dependency costs one. Three shapes were on the table: - -- **Eager bind.** Resolve the source at compile time and close over its resolver. - Measured alias resolve 305.5 → 192.0 ns (**-36%**), reproduced by two - independent verifiers. -- **Lazy bind.** Bind on the first non-overridden resolve, behind a - `bound is None` branch. Same steady-state cost as eager, without eager's - compile-time reach. -- **Inline only.** Read `_providers` and `_resolvers` per resolve, call the - source's resolver directly, cache nothing. Two dict `get`s per hop where a bind - has none. - -## Decision & rationale - -Inline-only ships, at **~322 → ~252 ns (-22%)** and 4 frames → 1. It gives up -roughly a third of the available win. - -**A bind buys an invalidation invariant; the inline buys none.** Both bind -variants are only sound because `ProvidersRegistry._invalidate()` clears -`_resolvers`, so a stale binding dies with the closure holding it. That is true -today, and it is a *second* place the invariant has to hold — stated, defended, -and re-checked by anyone who later touches memo publication. The inline re-reads -the live registry and cannot go stale by construction. The design principle here -is the conservative one: a bounded win does not buy a permanent cross-cutting -invariant, the same reasoning that dropped the -[warm-singleton memo swap](2026-07-18-warm-singleton-memo-swap-dropped.md). - -**Eager bind additionally escapes the override front-guard.** It compiles the -alias's whole source subtree even when the alias is overridden and the source is -never touched — the `modern-di-pytest` mock pattern. Shown structurally: -`len(_resolvers)` after resolving an overridden alias goes from 1 to 1+depth (11 -at depth 10), cold cost +404%. It also raises `TypeError` eagerly for a source -whose provider type `compile_resolver` does not know, and drops the maximum pure -alias chain from 494 to 329 hops. Lazy bind avoids all of this; only the -invariant argument above rules it out. - -**Capturing the registry was declined on the same grounds one level down.** The -first shipped form took the registry as a compile-time parameter, saving one -attribute load per hop. Since the registry memoizes the closure in `_resolvers`, -that made `_compile_alias`'s `resolve` the only compiled resolver forming -`registry → _resolvers → closure → cell → registry` — a registry with an alias in -it could then be freed only by cyclic GC, never by refcounting. Not a leak, but -the repo already took the opposite position for containers -(`Container.__init__`'s `scope: self` note, and `64b7cec`), and registries are -per-root-container, not per-process — a suite building a container per test -builds one per test. Reading the registry off the `container` argument removed -the cycle and measured **free** (250 → 249 ns, inside noise), which also puts the -alias back in the shape every other closure in the module already uses. - -Pinned by `test_alias_hop_costs_exactly_one_resolver_frame`, -`test_no_compiled_resolver_closes_over_its_registry`, -`test_overridden_alias_compiles_nothing_of_its_source`, and -`test_alias_picks_up_a_source_registered_after_a_failed_resolve` — that last one -catches only a *negative* cache; a success-path cache is undetectable by -construction, because a registered type's provider can never be replaced and any -registration clears `_resolvers`. - -## Revisit trigger - -An alias hop shows up hot in a profile from a real integration, **and** the -`_invalidate()`-clears-`_resolvers` invariant has acquired an explicit owner and -test of its own — at which point lazy bind (never eager) is worth the remaining -~60 ns. A second compiled closure needing the registry at resolve time would -reopen the capture question separately. diff --git a/planning/decisions/2026-08-03-resolve-provider-not-a-seam.md b/planning/decisions/2026-08-03-resolve-provider-not-a-seam.md deleted file mode 100644 index 25793905..00000000 --- a/planning/decisions/2026-08-03-resolve-provider-not-a-seam.md +++ /dev/null @@ -1,76 +0,0 @@ ---- -summary: `resolve_provider` is not an interception seam and `Container` subclassing is not a supported way to intercept resolution — since the compiled resolvers landed it has only ever seen top-level calls, so inlining it into `resolve()` narrows nothing that worked. ---- - -# `resolve_provider` is not an interception seam - -**Decision:** `Container.resolve_provider` is an entry point, not a hook. Overriding -it in a `Container` subclass is not a supported way to observe or intercept -resolution, and the resolve path is free to bypass it. This licenses inlining its -body into `Container.resolve`. `find_container` is **not** affected and remains a -blessed extension point. - -## Context - -Inlining `find_provider` + `resolve_provider` into `Container.resolve` measures -**-19% (~38 ns) on every by-type resolve** — the path every `@inject` marker and -framework integration takes. It was deferred partly because the same structural -objection that killed -[`2026-08-01-scope-map-inline-declined.md`](2026-08-01-scope-map-inline-declined.md) -appears to apply: `resolve_provider` is a public method on a subclassable class, -and `Container.__init__` builds children via `self.__class__`, so a subclass rides -the whole tree. Bypassing it would mean a subclass's override no longer runs for -by-type calls. - -## Decision & rationale - -**It is already not a seam, and that is measurable rather than arguable.** Since -the compiled resolvers shipped in 2.29.0, a resolver calls its dependencies' -resolvers *directly*; nothing routes a nested node through `resolve_provider`. Its -only callers are `resolve()`, `resolve_dependency()`, and the cycle back-edge -thunk in `ProvidersRegistry.resolver_for`. Demonstrated on `main` before this -change: a `Container` subclass overriding `resolve_provider` and resolving a -**4-node chain** records exactly **1** call — the top-level one. An override has -never seen the graph. Inlining removes one of the three top-level call sites; the -by-reference and marker-dispatch entries still route through it. - -So the thing the objection protects does not exist. What a subclass can still do -after this change is instrument the *entry points* by overriding `resolve` and -`resolve_provider` — which is what someone wanting that would actually reach for, -and it keeps working. - -**This is deliberately narrower than the `_scope_map` ruling, which stands.** -`find_container` is consulted on every cross-scope hop, and the container it -returns owns the cached instance and runs its finalizer — bypassing an override -there silently relocates lifecycle ownership, which is a bug, not a missed hook. -`resolve_provider` has no such consequence: bypassing an override loses -observation, not correctness. The two are not the same call and are not being -ruled on together. - -**Field check.** An audit of all 13 sibling integration wheels found zero -`Container` subclasses and zero `resolve_provider` overrides. `Container` -subclassing was not documented as an extension point anywhere in `architecture/` -(since removed) or `docs/`. - -**Accepted costs**, disclosed rather than discovered later: - -- A genuinely duplicated ~8-line body (closed check, memo hit, `resolver_for` - fallback, resolver call, `RecursionError` conversion) now lives in both `resolve` - and `resolve_provider` and must be edited in lockstep. This is the real price and - it is permanent. -- An exception raised through `resolve()` loses one traceback frame (5 → 4; - `resolve_provider` no longer appears). Verified directly. -- Recursion headroom moves by one frame in the benign direction. - -**Consequence worth naming.** Together with -[`2026-07-30-debug-resolution-tracing-declined.md`](2026-07-30-debug-resolution-tracing-declined.md), -modern-di offers no built-in way to observe *per-node* resolution. That was already -true — the compiled resolvers removed the last interior call — and this decision -records it rather than creating it. Entry-point instrumentation remains available -by overriding both public entry methods. - -## Revisit trigger - -A concrete request for per-resolve interception from a real integration or user. -The answer then is a designed seam with a stated contract — not a re-blessing of -subclass overrides, which the compiled resolve path stopped honouring in 2.29.0. diff --git a/planning/deferred/2026-07-17-warm-singleton-perf-headroom.md b/planning/deferred/2026-07-17-warm-singleton-perf-headroom.md index 9c47e55b..de9dfe13 100644 --- a/planning/deferred/2026-07-17-warm-singleton-perf-headroom.md +++ b/planning/deferred/2026-07-17-warm-singleton-perf-headroom.md @@ -34,14 +34,14 @@ Two levers, both now settled: - **The memo-swap** (swap the cached provider's resolver for a bare `return value` closure) was built, fully tested, measured at ~1.6x, and dropped — see - [`warm-singleton-memo-swap-dropped`](../decisions/2026-07-18-warm-singleton-memo-swap-dropped.md). + [`0015-warm-singleton-memo-swap-dropped`](../../docs/adr/0015-warm-singleton-memo-swap-dropped.md). The decisive finding is that `resolve_provider`'s dispatch floor is upstream of the swap and unremovable by it, so the technique provably cannot reach "near-free". - **The codegen ceiling on transient and deep-chain** is the cost of staying `exec`-free: dishka and wireup inline dependency calls into generated source, removing the per-node closure-call frame modern-di keeps. Re-declined — see - [`exec-hot-path-declined`](../decisions/2026-07-19-exec-hot-path-declined.md). + [`0017-exec-hot-path-declined`](../../docs/adr/0017-exec-hot-path-declined.md). The supporting measurement: closures already capture ~80-90% of the available ceiling, and `exec` buys a further 0-4% at fixed arity. So being modestly behind the codegen leaders on construction-heavy graphs is an accepted floor, diff --git a/planning/index.py b/planning/index.py deleted file mode 100644 index cf1f278d..00000000 --- a/planning/index.py +++ /dev/null @@ -1,179 +0,0 @@ -# ruff: noqa: INP001 # planning/ is not a Python package (this file is vendored into consumers' planning/) -"""Generate the planning index from frontmatter. - -Run via ``just index``. Globs ``planning/deferred/*.md`` and -``planning/decisions/*.md``, reads their frontmatter, and prints a Markdown -listing to stdout — deferred (the open queue) then decisions, newest-first. -Never writes a file: the listing is a query over the files, not a committed -artifact. - -``date`` and ``slug`` are derived from the file name, not -frontmatter — the name is the single source of truth for both. - -Both artifact kinds carry ``summary`` and nothing else required. A decision has -no ``status`` field: absent ``superseded_by`` means accepted, and its presence -means superseded. -""" - -import pathlib -import re -import sys - - -ROOT = pathlib.Path(__file__).parent -DEFERRED_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") -DECISION_RE = re.compile(r"^(?P\d{4}-\d{2}-\d{2})-(?P.+)$") -DEFERRED_REQUIRED = ("summary",) -DECISION_REQUIRED = ("summary",) - - -def parse_frontmatter(text: str) -> dict[str, str]: - """Parse a single-line-scalar YAML frontmatter block into a dict.""" - lines = text.splitlines() - if not lines or lines[0].strip() != "---": - return {} - fields: dict[str, str] = {} - for line in lines[1:]: - if line.strip() == "---": - break - if line[:1] in (" ", "\t"): - continue - key, sep, value = line.partition(": ") - if not sep: - continue - cleaned = value.strip().strip('"').strip("'") - fields[key.strip()] = "" if cleaned == "null" else cleaned - return fields - - -def _named(fields: dict[str, str], name: str, pattern: re.Pattern[str]) -> dict[str, str]: - """Inject ``date``/``slug`` derived from a file name into ``fields``.""" - match = pattern.match(name) - if match: - fields["date"] = match.group("date") - fields["slug"] = match.group("slug") - return fields - - -def load_deferred(root: pathlib.Path) -> list[dict[str, str]]: - """Read each deferred item's summary; derive date/slug from the file name.""" - deferred_dir = root / "deferred" - deferred: list[dict[str, str]] = [] - if not deferred_dir.is_dir(): - return deferred - for path in sorted(deferred_dir.glob("*.md")): - if path.name == "README.md" or path.name.startswith(("_", ".")): - continue - fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, DEFERRED_RE) - fields["path"] = f"deferred/{path.name}" - fields["name"] = path.stem - deferred.append(fields) - return deferred - - -def load_decisions(root: pathlib.Path) -> list[dict[str, str]]: - """Read each decision's frontmatter; derive date/slug from the file name.""" - decisions_dir = root / "decisions" - decisions: list[dict[str, str]] = [] - if not decisions_dir.is_dir(): - return decisions - for path in sorted(decisions_dir.glob("*.md")): - if path.name == "README.md" or path.name.startswith("_"): - continue - fields = _named(parse_frontmatter(path.read_text(encoding="utf-8")), path.stem, DECISION_RE) - fields["path"] = f"decisions/{path.name}" - fields["name"] = path.stem - decisions.append(fields) - return decisions - - -def format_row(row: dict[str, str]) -> str: - """Render one deferred item or decision as a Markdown list item.""" - slug = row.get("slug", "?") - path = row.get("path", "") - date = row.get("date", "") - summary = row.get("summary") or "(no summary)" - line = f"- **[{slug}]({path})** ({date}) — {summary}" - if row.get("superseded_by"): - line += f" _(superseded by {row['superseded_by']})_" - return line - - -def render(deferred: list[dict[str, str]], decisions: list[dict[str, str]]) -> str: - """Render the full Markdown listing: deferred then decisions, newest-first.""" - out = ["# Planning index", "", "_Generated by `just index` — do not edit._", "", "## Deferred", ""] - deferred_rows = sorted(deferred, key=lambda b: b.get("name", ""), reverse=True) - out += [format_row(b) for b in deferred_rows] if deferred_rows else ["_None._"] - out += ["", "## Decisions", ""] - decision_rows = sorted(decisions, key=lambda d: d.get("name", ""), reverse=True) - out += [format_row(d) for d in decision_rows] if decision_rows else ["_None._"] - out.append("") - return "\n".join(out).rstrip() + "\n" - - -def _require(fields: dict[str, str], keys: tuple[str, ...], rel: str, violations: list[str]) -> None: - """Append a violation for each required key that is absent or empty.""" - violations.extend(f"{rel}: missing or empty frontmatter key '{key}'" for key in keys if not fields.get(key)) - - -def _check_deferred(path: pathlib.Path, violations: list[str]) -> None: - """Validate one deferred item (requires `summary` + a revisit trigger).""" - rel = f"deferred/{path.name}" - if DEFERRED_RE.match(path.stem) is None: - violations.append(f"{rel}: file name is not 'YYYY-MM-DD-slug.md'") - text = path.read_text(encoding="utf-8") - _require(parse_frontmatter(text), DEFERRED_REQUIRED, rel, violations) - if "Revisit trigger" not in text: - violations.append( - f"{rel}: no '**Revisit trigger:**' section — an item with no trigger is abandoned, not deferred" - ) - - -def _check_decision(path: pathlib.Path, violations: list[str]) -> None: - """Validate one decision file (requires `summary`).""" - rel = f"decisions/{path.name}" - if DECISION_RE.match(path.stem) is None: - violations.append(f"{rel}: file name is not 'YYYY-MM-DD-slug.md'") - _require(parse_frontmatter(path.read_text(encoding="utf-8")), DECISION_REQUIRED, rel, violations) - - -def check(root: pathlib.Path) -> list[str]: - """Validate every deferred item and decision; return the list of violation strings.""" - violations: list[str] = [] - deferred_dir = root / "deferred" - decisions_dir = root / "decisions" - if deferred_dir.is_dir(): - for path in sorted(deferred_dir.iterdir()): - if path.name == "README.md" or path.name.startswith(("_", ".")): - continue - if path.suffix != ".md": - violations.append(f"deferred/{path.name}: unexpected non-md file in deferred/") - else: - _check_deferred(path, violations) - if decisions_dir.is_dir(): - for path in sorted(decisions_dir.glob("*.md")): - if path.name == "README.md" or path.name.startswith("_"): - continue - _check_decision(path, violations) - return violations - - -def main(argv: list[str] | None = None, root: pathlib.Path | None = None) -> int: - """Print the listing to stdout, or validate deferred items and decisions with --check.""" - argv = sys.argv[1:] if argv is None else argv - root = ROOT if root is None else root - if "--check" in argv: - violations = check(root) - if violations: - sys.stderr.write(f"planning: {len(violations)} violation(s)\n") - for violation in violations: - sys.stderr.write(f" - {violation}\n") - return 1 - sys.stdout.write("planning: OK\n") - return 0 - sys.stdout.write(render(load_deferred(root), load_decisions(root))) - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/planning/releases/2.29.0.md b/planning/releases/2.29.0.md index 9974fb74..145836b4 100644 --- a/planning/releases/2.29.0.md +++ b/planning/releases/2.29.0.md @@ -66,7 +66,7 @@ to add a provider type is no longer supported. See below. behavior into a creator function, or use `Alias`, instead of introducing a provider type. Rationale and the rejected alternatives (a 2.x deprecation ramp, holding for 3.0) are recorded in - [`planning/decisions/2026-07-17-custom-providers-retracted.md`](https://github.com/modern-python/modern-di/blob/main/planning/decisions/2026-07-17-custom-providers-retracted.md). + [`docs/adr/0013-custom-providers-retracted.md`](https://github.com/modern-python/modern-di/blob/main/docs/adr/0013-custom-providers-retracted.md). ## Docs & internals diff --git a/planning/releases/3.2.0.md b/planning/releases/3.2.0.md index d039e318..952b22a5 100644 --- a/planning/releases/3.2.0.md +++ b/planning/releases/3.2.0.md @@ -73,7 +73,7 @@ rather than cumulatively. the source's compiled resolver directly. It caches nothing and captures nothing, so a source registered later is still picked up on the next resolve; the binding variants that measured faster are declined with reasoning in - `planning/decisions/2026-08-03-alias-binds-nothing.md`. + [`docs/adr/0025-alias-binds-nothing.md`](https://github.com/modern-python/modern-di/blob/main/docs/adr/0025-alias-binds-nothing.md). - **The cached resolver's warm path lost its `MAKE_CELL`.** The cold-miss thunk is built with `functools.partial(build_cold, target)` rather than a lambda diff --git a/planning/releases/3.3.0.md b/planning/releases/3.3.0.md index a4f18ab8..681060d0 100644 --- a/planning/releases/3.3.0.md +++ b/planning/releases/3.3.0.md @@ -52,7 +52,7 @@ performance change rather than a breaking one. resolving a 4-node chain — because resolvers call each other directly. An audit of all 13 sibling integration wheels found zero `Container` subclasses and zero overrides. Full reasoning in - [`planning/decisions/2026-08-03-resolve-provider-not-a-seam.md`](../decisions/2026-08-03-resolve-provider-not-a-seam.md). + [`docs/adr/0026-resolve-provider-not-a-seam.md`](https://github.com/modern-python/modern-di/blob/main/docs/adr/0026-resolve-provider-not-a-seam.md). `find_container` is unaffected and remains a blessed extension point. - **A `ContextProvider`'s identity is fixed once something resolves through it.** diff --git a/planning/releases/3.4.0.md b/planning/releases/3.4.0.md index 9b4c621b..aa5d8825 100644 --- a/planning/releases/3.4.0.md +++ b/planning/releases/3.4.0.md @@ -59,7 +59,7 @@ class as the provider-type closure shipped in 2.29.0. - **`ContextProvider.resolve(container)` is removed.** Its only caller was the compiled resolver branch above, and the polymorphic `provider.resolve(self)` dispatch it belonged to was retired in 2.29.0 - ([decision](https://github.com/modern-python/modern-di/blob/main/planning/decisions/2026-07-17-custom-providers-retracted.md)). + ([decision](https://github.com/modern-python/modern-di/blob/main/docs/adr/0013-custom-providers-retracted.md)). Nothing in the docs referenced it. Resolve through the container (`container.resolve(SomeType)` or `container.resolve_provider(provider)`), which is unchanged and raises the same `ContextValueNotSetError` with the same message. diff --git a/planning/scripts/perf-readability-audit.workflow.mjs b/planning/scripts/perf-readability-audit.workflow.mjs index 58299565..9461a43c 100644 --- a/planning/scripts/perf-readability-audit.workflow.mjs +++ b/planning/scripts/perf-readability-audit.workflow.mjs @@ -34,7 +34,7 @@ const CONTEXT_BLOB_SCHEMA = { }, decisions: { type: 'array', - description: 'Every planning/decisions/*.md ruling: slug + one-line holding (especially what was rejected).', + description: 'Every docs/adr/*.md ruling: slug + one-line holding (especially what was rejected).', items: { type: 'object', additionalProperties: false, @@ -152,7 +152,7 @@ Do exactly this: 2. file_map: every file under modern_di/ (source) — relative path, line count, one-line role. Skip __pycache__. -3. decisions: read every file under planning/decisions/. For each, record its slug (the filename without date/extension is fine) and a one-line holding — WHAT WAS DECIDED, especially what was rejected (e.g. "declined folding ContextRegistry into Container", "no exec codegen"). These are the settled-ground guardrail. +3. decisions: read every file under docs/adr/. For each, record its slug (the filename without number/extension is fine) and a one-line holding — WHAT WAS DECIDED, especially what was rejected (e.g. "declined folding ContextRegistry into Container", "no exec codegen"). These are the settled-ground guardrail. 4. deferred_items: read every file in planning/deferred/. For each item, record a short title, a one-line gist, and its revisit trigger. Capture the perf items faithfully (warm-singleton memo-swap dropped, codegen ceiling, free-threaded non-scaling) — these must not be re-proposed as open. diff --git a/tests/test_invariant_census.py b/tests/test_invariant_census.py index 257f6ca9..7497b41b 100644 --- a/tests/test_invariant_census.py +++ b/tests/test_invariant_census.py @@ -1,49 +1,24 @@ -"""Census of invariant tests and the citations that point to them. +"""Census of the invariant tests themselves. -Every ``test_*`` name cited from a comment or docstring under ``modern_di/`` or ``tests/`` -resolves to a real test, every ``tests/.py`` or ``planning/.md`` path cited the same -way resolves to a real file, and every ``INVARIANT:`` docstring states what breaks it. A rename -that orphans a citation fails here rather than in review -- the citations are all that replaced -the deleted prose documentation pages. +Every ``INVARIANT:`` docstring states what breaks it: the claim alone is a label, and the second +paragraph is the anti-refactor warning. """ import ast import pathlib -import re -import tokenize _REPO_ROOT = pathlib.Path(__file__).parent.parent -_SRC_DIR = _REPO_ROOT / "modern_di" _TESTS_DIR = _REPO_ROOT / "tests" -_BENCHMARKS_DIR = _REPO_ROOT / "benchmarks" -_DECISIONS_DIR = _REPO_ROOT / "planning" / "decisions" -_DEFERRED_DIR = _REPO_ROOT / "planning" / "deferred" -# Markdown scanned for test-name citations. Scoped deliberately -- docs/ uses illustrative -# user-facing test names (e.g. docs/recipes/testing-overrides.md) that are not real tests here. -_MD_CITATION_SOURCES = ( - _REPO_ROOT / "CLAUDE.md", - *sorted(_DECISIONS_DIR.glob("*.md")), - *sorted(_DEFERRED_DIR.glob("*.md")), -) -# `\b` before the lookahead forces the whole identifier, so `test_resolver_compiler.py` -# (a module name, not a citation) is rejected instead of matching a truncated prefix. -_CITATION = re.compile(r"\b(test_[a-z0-9_]+)\b(?!\.py)") -# A `tests/.py` or `planning/.md` path, or a bare dated `planning/` filename cited -# without its directory prefix (e.g. `2026-07-26-explicit-only-validation.md`). -_PATH_CITATION = re.compile(r"\b((?:tests|planning)/[\w./-]+\.(?:py|md)|\d{4}-\d{2}-\d{2}-[\w-]+\.md)\b") _INVARIANT = "INVARIANT:" # The claim paragraph, then the "what breaks it" paragraph -- fewer than two means the second is missing. _MIN_PARAGRAPHS = 2 -# Module included so a module-level docstring counts; ast.walk yields it before its descendants. -_DOCSTRING_NODE_TYPES = (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef) - -def _walk_test_functions(root: pathlib.Path) -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: +def _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: found = [] - for path in sorted(root.rglob("*.py")): + for path in sorted(_TESTS_DIR.rglob("*.py")): tree = ast.parse(path.read_text(encoding="utf-8")) found.extend( (path, node) @@ -53,65 +28,6 @@ def _walk_test_functions(root: pathlib.Path) -> list[tuple[pathlib.Path, ast.Fun return found -def _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: - return _walk_test_functions(_TESTS_DIR) - - -def _citation_paths() -> list[pathlib.Path]: - return sorted({*_SRC_DIR.rglob("*.py"), *_TESTS_DIR.rglob("*.py")}) - - -def _comment_matches(path: pathlib.Path, pattern: re.Pattern[str]) -> set[str]: - """Names matching `pattern` in real comments only -- tokenize, so a `#` inside a string is not a comment.""" - with path.open("rb") as handle: - return { - name - for token in tokenize.tokenize(handle.readline) - if token.type == tokenize.COMMENT - for name in pattern.findall(token.string) - } - - -def _docstring_matches(path: pathlib.Path, pattern: re.Pattern[str]) -> set[str]: - """Names matching `pattern` in any module/class/function docstring in the file.""" - tree = ast.parse(path.read_text(encoding="utf-8")) - names: set[str] = set() - for node in ast.walk(tree): - if isinstance(node, _DOCSTRING_NODE_TYPES): - docstring = ast.get_docstring(node) - if docstring: - names.update(pattern.findall(docstring)) - return names - - -def _comment_citations(path: pathlib.Path) -> set[str]: - return _comment_matches(path, _CITATION) - - -def _docstring_citations(path: pathlib.Path) -> set[str]: - return _docstring_matches(path, _CITATION) - - -def _path_citation_exists(citation: str) -> bool: - """Resolve `citation`: a full path directly, a bare dated filename under `decisions/`/`deferred/`.""" - if citation.startswith(("tests/", "planning/")): - return (_REPO_ROOT / citation).is_file() - return (_DECISIONS_DIR / citation).is_file() or (_DEFERRED_DIR / citation).is_file() - - -def test_every_cited_test_exists() -> None: - known = {node.name for _, node in _test_functions()} - assert known, "the walk over tests/ found no test functions" - - orphans = sorted( - f"{path.relative_to(_REPO_ROOT)}: {name}" - for path in _citation_paths() - for name in _comment_citations(path) | _docstring_citations(path) - if name not in known - ) - assert not orphans, f"comments or docstrings cite tests that do not exist: {orphans}" - - def test_every_invariant_states_what_breaks_it() -> None: marked = [ (path, node) for path, node in _test_functions() if (ast.get_docstring(node) or "").startswith(_INVARIANT) @@ -124,32 +40,3 @@ def test_every_invariant_states_what_breaks_it() -> None: if len([part for part in (ast.get_docstring(node) or "").split("\n\n") if part.strip()]) < _MIN_PARAGRAPHS ) assert not bare, f"INVARIANT tests with no 'what breaks it' paragraph: {bare}" - - -def test_every_cited_path_and_markdown_test_name_exists() -> None: - """Guard two citation forms `test_every_cited_test_exists` misses. - - A path cited from a `modern_di/`/`tests/` comment or docstring, and a test name cited from - `CLAUDE.md` or a `planning/decisions/`/`planning/deferred/` record -- neither trips the - name-only, Python-only check above, since one is spelled as a path and the other lives outside - `.py` files. - """ - path_orphans = sorted( - f"{path.relative_to(_REPO_ROOT)}: {citation}" - for path in _citation_paths() - for citation in _comment_matches(path, _PATH_CITATION) | _docstring_matches(path, _PATH_CITATION) - if not _path_citation_exists(citation) - ) - assert not path_orphans, f"comments or docstrings cite paths that do not exist: {path_orphans}" - - known = {node.name for _, node in _test_functions()} - known |= {node.name for _, node in _walk_test_functions(_BENCHMARKS_DIR)} - assert known, "the walk over tests/ and benchmarks/ found no test functions" - - md_orphans = sorted( - f"{md_path.relative_to(_REPO_ROOT)}: {name}" - for md_path in _MD_CITATION_SOURCES - for name in _CITATION.findall(md_path.read_text(encoding="utf-8")) - if name not in known - ) - assert not md_orphans, f"Markdown cites tests that do not exist: {md_orphans}"