From 3e66c42ed2902ec903ed005b25d79c4d93109680 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 11 Aug 2026 09:32:36 +0300 Subject: [PATCH 01/18] test: add the invariant census and mark the frame-budget invariants --- tests/test_invariant_census.py | 73 +++++++++++++++++++++++++++++++++ tests/test_resolver_compiler.py | 40 ++++++++++-------- 2 files changed, 95 insertions(+), 18 deletions(-) create mode 100644 tests/test_invariant_census.py diff --git a/tests/test_invariant_census.py b/tests/test_invariant_census.py new file mode 100644 index 0000000..d6b6240 --- /dev/null +++ b/tests/test_invariant_census.py @@ -0,0 +1,73 @@ +"""Census of invariant tests and the comments that cite them. + +Every ``test_*`` name cited from a comment in ``modern_di/`` resolves to a real test, 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 ``architecture/`` pages. +""" + +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" + +# `\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)") +_INVARIANT = "INVARIANT:" +# The claim paragraph, then the "what breaks it" paragraph -- fewer than two means the second is missing. +_MIN_PARAGRAPHS = 2 + + +def _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: + found = [] + for path in sorted(_TESTS_DIR.rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8")) + found.extend( + (path, node) + for node in ast.walk(tree) + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name.startswith("test_") + ) + return found + + +def _cited_names(path: pathlib.Path) -> set[str]: + """Names cited from 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 _CITATION.findall(token.string) + } + + +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 sorted(_SRC_DIR.rglob("*.py")) + for name in _cited_names(path) + if name not in known + ) + assert not orphans, f"comments 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) + ] + assert marked, "no test carries an INVARIANT: docstring; the convention is not in use" + + bare = sorted( + f"{path.relative_to(_REPO_ROOT)}::{node.name}" + for path, node in marked + 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}" diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index 3e964d0..dfc1a25 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -459,13 +459,12 @@ def _spy( def test_resolve_costs_exactly_one_resolver_frame_per_node() -> None: - # Every compiled resolver front-guards its own override, navigates its own scope and - # inlines its own kwargs build + creator call. That duplication is deliberate: any of it - # extracted into a shared helper would cost one Python frame *per resolved node*, which - # is the whole reason the compiled path exists (architecture/performance.md). - # - # Measured as a difference between two chain depths, so the fixed cost of the harness - # and of `resolve_provider` itself cancels and only the per-node slope is asserted. + """INVARIANT: resolving one node costs exactly one Python frame -- its own compiled resolver. + + Extracting the override guard, the scope hop, the kwargs build or the creator call into a shared + helper costs one frame *per resolved node* and moves the slope from 2 to 3. Measured as a + difference between two chain depths, so the harness's fixed cost cancels. + """ shallow_container, shallow_root = _warm_chain(2) deep_container, deep_root = _warm_chain(6) @@ -475,14 +474,17 @@ def test_resolve_costs_exactly_one_resolver_frame_per_node() -> None: assert (deep - shallow) == (6 - 2) * _CALLS_PER_NODE, ( f"per-node cost is {(deep - shallow) / (6 - 2)} Python calls, expected {_CALLS_PER_NODE} " f"on Python {sys.version_info.major}.{sys.version_info.minor}. A helper extracted from " - f"the compiled resolvers costs one frame per resolved node -- see architecture/performance.md." + f"the compiled resolvers costs one frame per resolved node." ) def test_alias_hop_costs_exactly_one_resolver_frame() -> None: - # An alias forwards to its source's compiled resolver by direct reference, like every - # Factory dependency. Routing through `_find_source` + `find_provider` + - # `resolve_provider` instead costs four frames per hop -- see architecture/performance.md. + """INVARIANT: an alias hop costs one Python frame, like any Factory dependency. + + The alias resolver inlines the source lookup and the source's resolver-memo read. Routing + through `_find_source` + `find_provider` + `resolve_provider` instead costs four frames per hop. + """ + class _Source: ... class _Iface: ... @@ -504,7 +506,7 @@ class _Aliased(Group): assert (with_alias - without_alias) == 1, ( f"an alias hop costs {with_alias - without_alias} Python calls, expected 1 (its own " - f"resolver). Looking the source up per resolve costs four -- see architecture/performance.md." + f"resolver). Looking the source up per resolve costs four." ) @@ -569,10 +571,13 @@ def test_positional_path_selects_the_arity_specialised_closure(arity: int, expec def test_cached_resolver_has_no_cell_on_the_warm_path() -> None: - # The cold-miss thunk must not close over `target`: a closure promotes it to a cell, so - # MAKE_CELL runs in the resolver's prologue on every call -- including the warm hit that - # returns early, and the override hit that never reaches `target` at all. Measured at ~18 ns - # of a ~162 ns warm resolve. Nothing else in the suite would catch a revert to a lambda. + """INVARIANT: the cached-factory resolver has no cell variables. + + The cold-miss thunk must stay a `functools.partial`, never a lambda closing over `target`: a + closure promotes `target` to a cell, so MAKE_CELL runs in the prologue on every call -- + including the warm hit that returns two lines later. Nothing else in the suite catches a revert. + """ + class G(Group): cached = providers.Factory(creator=_A, scope=Scope.APP, cache=True) @@ -581,6 +586,5 @@ class G(Group): code = typing.cast("_pytypes.FunctionType", resolver).__code__ assert code.co_cellvars == (), ( - f"the cached-factory resolver grew cell variables {code.co_cellvars}; " - f"a MAKE_CELL now runs on every warm hit -- see architecture/performance.md" + f"the cached-factory resolver grew cell variables {code.co_cellvars}; a MAKE_CELL now runs on every warm hit" ) From 7974cdefce6bd83cb0ab1695024d3ece373c8d39 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 11 Aug 2026 09:47:41 +0300 Subject: [PATCH 02/18] test: mark the invariants architecture/ used to state Adds INVARIANT: docstrings to the tests that already pin the override, navigation, positional-path, live-context, validation, scope-algebra and container-lifecycle claims formerly stated in architecture/. Converts leading # comments that carried the same rationale into the docstring; comments explaining a test's own scope or limitations are left in place. --- tests/providers/test_context_provider.py | 32 ++++++--- tests/registries/test_providers_registry.py | 6 ++ tests/test_container.py | 6 ++ tests/test_custom_scope.py | 19 ++++-- tests/test_dependency_graph_contract.py | 7 ++ tests/test_resolver_compiler.py | 74 ++++++++++++++++++--- tests/test_runtime_cycle_guard.py | 8 ++- 7 files changed, 124 insertions(+), 28 deletions(-) diff --git a/tests/providers/test_context_provider.py b/tests/providers/test_context_provider.py index 985afb9..7f92dfe 100644 --- a/tests/providers/test_context_provider.py +++ b/tests/providers/test_context_provider.py @@ -160,6 +160,11 @@ class _LateCtxGroup(Group): def test_set_context_after_first_resolve_is_seen_by_later_resolves() -> None: + """INVARIANT: a ContextProvider dependency is read live on every resolve. + + Only the *binding* is frozen at compile time, never the value. Caching the value would make a + later `set_context` invisible to non-cached factories across scopes. + """ container = Container(scope=Scope.APP, groups=[_LateCtxGroup]) container.open() first = container.resolve(_NeedsLateCtx) @@ -318,8 +323,11 @@ class _CachedCtxGroup(Group): def test_late_context_does_not_rebuild_cached_singleton() -> None: - # Documented limitation: a cached factory's instance is fixed at first build; - # a later set_context does not retroactively rebuild it. + """INVARIANT: a cached factory is built once and does not re-read a later context value. + + This is the deliberate boundary on live context: caching wins. Making the cached path re-read + would turn `cache=True` into a per-resolve check. + """ app = Container(scope=Scope.APP, groups=[_CachedCtxGroup]) app.open() first = app.resolve(_CachedCtxSvc) @@ -467,11 +475,13 @@ def test_kwargs_context_provider_without_parsed_signature_injects_present_value( @pytest.mark.parametrize("cache", [False, True]) def test_scope_error_through_a_context_kwarg_carries_one_breadcrumb_step(cache: bool) -> None: - # The context hop raises a bare scope error and the Factory closure prepends its own step - # exactly once. Routing the hop through the compiler's `_navigate`, which prepends a step - # itself, would render the factory twice while every other assertion stayed green. - # Parametrized because the lookup is folded into the transient and cached closures - # separately, and each copy can regress on its own. + """INVARIANT: a scope error through a context kwarg carries exactly one breadcrumb step. + + The folded loops call `find_container`, never the compiler's `_navigate` -- that helper prepends + a step and the enclosing closure prepends the factory's own, so the caller would appear twice. + The cached and transient loops are separate copies, which is why this is parametrized. + """ + class Cfg: ... @dataclasses.dataclass(kw_only=True, slots=True) @@ -492,8 +502,12 @@ class G(Group): def test_same_scope_context_hop_does_not_call_find_container(monkeypatch: pytest.MonkeyPatch) -> None: - # The hop is an int compare when the container is already at the provider's scope; calling - # find_container to be handed back the same container costs a frame per context kwarg. + """INVARIANT: a same-scope context kwarg costs no navigation. + + The compiled closure folds the scope compare inline. Replacing it with an unconditional + `find_container` call adds a frame per context kwarg to the hottest path. + """ + class Cfg: ... @dataclasses.dataclass(kw_only=True, slots=True) diff --git a/tests/registries/test_providers_registry.py b/tests/registries/test_providers_registry.py index cff768f..dfa8e37 100644 --- a/tests/registries/test_providers_registry.py +++ b/tests/registries/test_providers_registry.py @@ -30,6 +30,12 @@ def test_mark_validated_sets_validated() -> None: def test_mutation_clears_validated() -> None: + """INVARIANT: every registry mutation clears `_validated`. + + `register` and `add_providers` are the only mutators. One that forgets to clear leaves a stale + clean result, so a later `validate()` returns free without walking the now-larger graph -- and + the runtime cycle guard stays armed to re-raise instead of converting. + """ registry = ProvidersRegistry() registry.mark_validated() assert registry.is_validated() is True diff --git a/tests/test_container.py b/tests/test_container.py index f38f051..f7caffb 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -520,6 +520,12 @@ def test_explicit_open_after_close_does_not_warn() -> None: def test_child_built_off_closed_parent_warns_only_when_the_parent_resolves() -> None: + """INVARIANT: building a child container does not require the parent to be open. + + `build_child_container` reads the parent's scope map and its two shared registries; it resolves + nothing and touches no cache, so there is deliberately no closed-check on the parent. Adding one + would break every integration that builds a request child after a shutdown/restart cycle. + """ app = Container(scope=Scope.APP, groups=[_AppBrokerGroup]) app.open() app.close_sync() diff --git a/tests/test_custom_scope.py b/tests/test_custom_scope.py index 71f5323..15dfb04 100644 --- a/tests/test_custom_scope.py +++ b/tests/test_custom_scope.py @@ -97,21 +97,26 @@ def test_invalid_child_scope_with_conflicting_value() -> None: def test_scope_algebra_answers_deeper_members_for_any_int_enum() -> None: - # The rule has one home and takes ANY IntEnum: Python forbids extending an enum that - # has members (`class MyScope(Scope)` -> TypeError), so a custom scope is a standalone - # IntEnum and the algebra cannot be methods on Scope without silently skipping it. + """INVARIANT: the scope algebra takes any IntEnum, not only `Scope`. + + A custom scope cannot subclass `Scope` (Python forbids extending an enum with members), so an + algebra expressed as methods on `Scope` would apply to the five built-in members and nothing + else. Free functions are what make custom scopes work at all. + """ assert _deeper_members(MyScope.TENANT) == [MyScope.BACKGROUND_JOB] assert _deeper_members(MyScope.BACKGROUND_JOB) == [] assert _deeper_members(Scope.ACTION) == [Scope.STEP] def test_scope_algebra_next_deeper_is_the_shallowest_deeper_member() -> None: - # Non-contiguous values: the next scope is the smallest member greater than the current - # one, never current.value + 1 (which need not be a member at all). + """INVARIANT: `_next_deeper` returns the shallowest deeper member of the provider's own enum. + + Not `value + 1` -- a non-contiguous custom enum (`TENANT=6, JOB=10`) must derive `JOB` from + `TENANT`. Returning `None` at the deepest member (rather than raising) is what keeps `scope.py` + from importing `exceptions.py`. + """ assert _next_deeper(GappedScope.TENANT) is GappedScope.BACKGROUND_JOB assert _next_deeper(Scope.APP) is Scope.SESSION - # None at the deepest member: `scope.py` stays dependency-free, so raising - # MaxScopeReachedError here would cycle (exceptions imports scope for allowed_scopes). assert _next_deeper(GappedScope.BACKGROUND_JOB) is None assert _next_deeper(Scope.STEP) is None diff --git a/tests/test_dependency_graph_contract.py b/tests/test_dependency_graph_contract.py index 058bacd..835cfce 100644 --- a/tests/test_dependency_graph_contract.py +++ b/tests/test_dependency_graph_contract.py @@ -60,6 +60,13 @@ class G(Group): def test_validate_is_free_when_already_validated(monkeypatch: pytest.MonkeyPatch) -> None: + """INVARIANT: `_validated` memoizes a clean walk; it never gates whether validation may run. + + Nothing validates automatically, so the flag only records that the last walk of the current + registry contents was clean. Treating it as a gate would make `validate()` silently skip a graph + the caller asked it to check. + """ + class X: ... class G(Group): diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index dfc1a25..397dc57 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -237,6 +237,12 @@ def _arity_group( @pytest.mark.parametrize("arity", [0, 1, 2]) def test_arity_rung_front_guards_the_override(arity: int) -> None: + """INVARIANT: every compiled resolver checks the override registry before anything else. + + The guard runs before scope navigation, before the cache and before the creator, so overriding + an otherwise-unwireable factory still short-circuits. Each arity rung is a full copy of the + closure, so a rung added without the guard regresses only that rung. + """ group = _arity_group(arity) container = Container(scope=Scope.APP, groups=[group]) container.open() @@ -247,6 +253,11 @@ def test_arity_rung_front_guards_the_override(arity: int) -> None: @pytest.mark.parametrize("arity", [0, 1, 2]) def test_arity_rung_navigates_to_its_own_scope(arity: int) -> None: + """INVARIANT: a resolver walks to its declared scope exactly once per resolve. + + The same-scope case is an int compare, not a `find_container` call. Each arity rung carries its + own copy of the hop, so a rung added without it resolves from the wrong container. + """ # Line coverage of the cross-scope hop's success path. The wrong target is not observable # here -- deps navigate themselves -- so the mutant that skips navigation is killed by the # closed-target and dependency-error tests below, not by this one. @@ -354,6 +365,13 @@ def test_can_call_positionally_accepts_ordered_provider_signature() -> None: def test_can_call_positionally_rejects_static_or_context_kwarg() -> None: + """INVARIANT: the positional-path predicate excludes a static-or-context kwarg. + + A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow + path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of + them trades correctness for speed. + """ + # rule 1: a context param makes the plan non-pure, so kwargs folding must run. def creator(dep: _A, req: _Req) -> _Ordered: raise NotImplementedError # pragma: no cover - parsed for wiring, never resolved @@ -370,6 +388,13 @@ def creator(dep: _A, req: _Req) -> _Ordered: def test_can_call_positionally_rejects_defaulted_omitted_param() -> None: + """INVARIANT: the positional-path predicate excludes a defaulted, omitted param. + + A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow + path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of + them trades correctness for speed. + """ + # rule 2a: `opt` has a default and no provider, so it is omitted -> provider_kwargs is a # strict prefix of the signature, not the whole of it. def creator(dep: _A, opt: int = 5) -> _Ordered: @@ -384,6 +409,13 @@ def creator(dep: _A, opt: int = 5) -> _Ordered: def test_can_call_positionally_rejects_kwargs_overlay_reorder() -> None: + """INVARIANT: the positional-path predicate excludes a kwargs-overlay reorder. + + A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow + path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of + them trades correctness for speed. + """ + # rule 2b: supplying `a` via the kwargs overlay defers it to the end of provider_kwargs, # so the binding order (b, a) no longer matches the signature (a, b). def creator(a: _A, b: _B) -> _Ordered: @@ -401,6 +433,13 @@ def creator(a: _A, b: _B) -> _Ordered: def test_can_call_positionally_rejects_keyword_only_param() -> None: + """INVARIANT: the positional-path predicate excludes a keyword-only param. + + A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow + path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of + them trades correctness for speed. + """ + # rule 3: a keyword-only dep can never be passed positionally. def creator(*, dep: _A) -> _Ordered: raise NotImplementedError # pragma: no cover - parsed for wiring, never resolved @@ -414,6 +453,13 @@ def creator(*, dep: _A) -> _Ordered: def test_can_call_positionally_rejects_positional_only_param() -> None: + """INVARIANT: the positional-path predicate excludes a positional-only param. + + A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow + path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of + them trades correctness for speed. + """ + # rule 4: `prefix` is positional-only WITH a default, dropped from parsed_kwargs so the # remaining names look like a clean prefix ("dep",) -- but a positional call would bind # `dep` to the `prefix` slot. The parser's has_positional_only_gap flag must reject it. @@ -511,8 +557,12 @@ class _Aliased(Group): def test_overridden_alias_compiles_nothing_of_its_source() -> None: - # The override front-guard runs before the source is ever looked up, so the mock pattern - # never pays to compile a subtree it will not touch. + """INVARIANT: an override short-circuits before its provider's subtree is compiled. + + The front-guard runs before the alias source is looked up, so the mock pattern never pays to + compile a subtree it will not touch. Moving the guard below the source lookup breaks that. + """ + class _Source: ... class _Iface: ... @@ -530,9 +580,12 @@ class _G(Group): def test_no_compiled_resolver_closes_over_its_registry() -> None: - # A resolver that captures its registry forms a cycle with the memo holding it, so the - # registry is reclaimable only by cyclic GC. Every closure reads its registries off the - # container argument instead. + """INVARIANT: no compiled resolver captures its registry in a closure cell. + + A resolver that captures the registry forms a cycle with the memo holding it, so the registry + becomes reclaimable only by cyclic GC. Every closure reads its registries off the container arg. + """ + class _Src: ... class _Iface: ... @@ -559,11 +612,12 @@ class _G(Group): ("arity", "expected"), [(0, "resolve_arity0"), (1, "resolve_arity1"), (2, "resolve_positional")] ) def test_positional_path_selects_the_arity_specialised_closure(arity: int, expected: str) -> None: - # Asserted on the code object because nothing else can see it: the rungs are semantically - # identical to the generic star-call, and from 3.12 PEP 709 inlines the comprehension the - # frame-budget test would otherwise notice. Delete a rung and this fails on every - # interpreter; without it, only the 3.10 and 3.11 jobs would catch the regression, and they - # would report it as an extracted helper. + """INVARIANT: arity 0 and 1 compile to their own specialised closures. + + Below 3.12 a comprehension is a separate code object, so a generic star-call costs a third frame + per node. Deleting a rung fails here on every interpreter; without this test only the 3.10 and + 3.11 jobs would notice, and they would misreport it as an extracted helper. + """ group = _arity_group(arity) container = Container(scope=Scope.APP, groups=[group]) resolver = container.providers_registry.resolver_for(group.target) diff --git a/tests/test_runtime_cycle_guard.py b/tests/test_runtime_cycle_guard.py index 654cfa4..ca911c7 100644 --- a/tests/test_runtime_cycle_guard.py +++ b/tests/test_runtime_cycle_guard.py @@ -158,8 +158,12 @@ class RecursiveGroup(Group): def test_validated_graph_reraises_recursionerror_without_walk(monkeypatch: pytest.MonkeyPatch) -> None: - # A self-recursive creator on a validated (acyclic-static) graph must re-raise the - # RecursionError untouched, short-circuiting before find_cycle_from is ever consulted. + """INVARIANT: on a validated graph an escaped RecursionError re-raises untouched. + + A validated graph is known acyclic, so the overflow is genuine self-recursion in a creator. + Walking anyway would misreport it as a circular dependency and burn stack near the limit. + """ + class SelfRec: def __init__(self) -> None: raise RecursionError From 98a2a5c458e47fbfefbd0d3d15ddf57cb985dbd0 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 11 Aug 2026 09:59:02 +0300 Subject: [PATCH 03/18] test: attribute the unwireable-override and skip-navigation claims to their real tests Two INVARIANT docstrings overclaimed what their test proves: the override guard test implied it exercises an otherwise-unwireable factory (it doesn't - test_unwireable_factory_override_short_circuits does), and the scope-hop test implied it catches a skipped-navigation regression (its own retained comment already said it doesn't - two sibling tests do). Reword both to state only what each test proves and cite the test that proves the rest. --- tests/test_resolver_compiler.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index 397dc57..54e4aed 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -239,9 +239,11 @@ def _arity_group( def test_arity_rung_front_guards_the_override(arity: int) -> None: """INVARIANT: every compiled resolver checks the override registry before anything else. - The guard runs before scope navigation, before the cache and before the creator, so overriding - an otherwise-unwireable factory still short-circuits. Each arity rung is a full copy of the - closure, so a rung added without the guard regresses only that rung. + The guard runs before scope navigation, before the cache and before the creator. Each arity rung + is a full copy of the closure, so a rung added without the guard regresses only that rung. That + the same guard still short-circuits an otherwise-unwireable factory is proven separately by + `test_unwireable_factory_override_short_circuits` in `tests/providers/test_factory.py` -- this + test's dependencies are all ordinarily wireable. """ group = _arity_group(arity) container = Container(scope=Scope.APP, groups=[group]) @@ -255,12 +257,11 @@ def test_arity_rung_front_guards_the_override(arity: int) -> None: def test_arity_rung_navigates_to_its_own_scope(arity: int) -> None: """INVARIANT: a resolver walks to its declared scope exactly once per resolve. - The same-scope case is an int compare, not a `find_container` call. Each arity rung carries its - own copy of the hop, so a rung added without it resolves from the wrong container. + The same-scope case is an int compare, not a `find_container` call. This test pins that the rung + navigates at all; the wrong target is not observable here (dependencies navigate themselves), so + the skip-navigation mutant is killed by `test_arity_rung_reopens_a_closed_target` and + `test_arity_rung_prepends_its_step_to_a_dependency_error`. """ - # Line coverage of the cross-scope hop's success path. The wrong target is not observable - # here -- deps navigate themselves -- so the mutant that skips navigation is killed by the - # closed-target and dependency-error tests below, not by this one. group = _arity_group(arity) app = Container(scope=Scope.APP, groups=[group]) app.open() From 8d1c1de9ccd240e5d13e2a4c670cb532cf9db89c Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 11 Aug 2026 10:10:41 +0300 Subject: [PATCH 04/18] test: narrow the validated-flag invariant to what its test actually proves The "never gates whether validation may run" clause wasn't exercised by this test (it only shows a repeat validate() skips the re-walk) and read as contradicting is_validated(): return, which does gate the re-walk. Keep only the memoization claim here and attribute the never-auto- triggers half to the four test_container.py tests that actually prove it. --- tests/test_dependency_graph_contract.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/test_dependency_graph_contract.py b/tests/test_dependency_graph_contract.py index 835cfce..fed7fbd 100644 --- a/tests/test_dependency_graph_contract.py +++ b/tests/test_dependency_graph_contract.py @@ -60,11 +60,14 @@ class G(Group): def test_validate_is_free_when_already_validated(monkeypatch: pytest.MonkeyPatch) -> None: - """INVARIANT: `_validated` memoizes a clean walk; it never gates whether validation may run. - - Nothing validates automatically, so the flag only records that the last walk of the current - registry contents was clean. Treating it as a gate would make `validate()` silently skip a graph - the caller asked it to check. + """INVARIANT: `_validated` memoizes a clean walk, so a repeat `validate()` skips the walk. + + A mutator that forgets to clear the flag leaves a stale clean result: `validate()` returns free + without re-walking a graph that actually changed. That nothing validates automatically -- + construction, `add_providers`, `open()` and `resolve()` all leave the flag alone -- is proven + separately by `test_construction_never_validates`, + `test_add_providers_never_validates_and_does_not_roll_back`, `test_open_never_validates` and + `test_resolve_never_validates` in `tests/test_container.py`. """ class X: ... From e19a607fc74a52f0fb468c4117dbd68f5e5faa26 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Tue, 11 Aug 2026 11:09:33 +0300 Subject: [PATCH 05/18] test: mark the never-auto-validates invariant on its four tests architecture/validation.md states that validate() is the only thing that walks the graph - construction, open(), add_providers and resolve() never do. That invariant was missing from the original map and would otherwise survive nowhere once the page is deleted. The four tests already enforce it; mark each with its own INVARIANT docstring naming only the entry point that test exercises. --- tests/test_container.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/test_container.py b/tests/test_container.py index f7caffb..e6fb5e7 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -993,12 +993,26 @@ class _DeferFactoryNeedingRequestGroup(Group): def test_construction_never_validates() -> None: + """INVARIANT: `validate()` is the only thing that walks the graph. + + Constructing a container from a cyclic group raises nothing; only the later `validate()` call + surfaces `ValidationFailedError`. An `__init__` that walked eagerly is the split-validation + machinery `2026-07-26-explicit-only-validation.md` built and discarded. + """ container = Container(scope=Scope.APP, groups=[CycleGroup]) # a cycle: no raise here any more with pytest.raises(ValidationFailedError): container.validate() def test_add_providers_never_validates_and_does_not_roll_back() -> None: + """INVARIANT: `validate()` is the only thing that walks the graph. + + `add_providers` registers `Broken` quietly -- no raise, no rollback -- even onto a registry + already marked validated; only the next explicit `validate()` call surfaces + `ValidationFailedError`. An `add_providers` that walked eagerly is the rollback path + `2026-07-26-explicit-only-validation.md` built and discarded. + """ + @dataclasses.dataclass(kw_only=True, slots=True) class Missing: ... @@ -1018,6 +1032,13 @@ class Broken: def test_open_never_validates() -> None: + """INVARIANT: `validate()` is the only thing that walks the graph. + + `open()` on a cyclic graph raises nothing, neither called directly nor entered via the context + manager. Binding validation to `open()` was 3.0's design, discarded per + `2026-07-26-explicit-only-validation.md` after the root's open hook not firing in some execution + contexts caused six production defects. + """ container = Container(scope=Scope.APP, groups=[CycleGroup]) container.open() # no raise with container: # nor via the context manager @@ -1025,7 +1046,13 @@ def test_open_never_validates() -> None: def test_resolve_never_validates() -> None: - # A broken graph surfaces at the resolve that hits it, not as an aggregate. + """INVARIANT: `validate()` is the only thing that walks the graph. + + Resolving `_DeferBrokenService` on an unvalidated broken graph raises `ArgumentResolutionError` + for the one missing dependency, not `ValidationFailedError` for the whole graph -- proving + `resolve()` never walks looking for other errors. Making it validate first is the per-resolve tax + `2026-07-26-explicit-only-validation.md` rejected. + """ container = Container(scope=Scope.APP, groups=[_DeferBrokenGroup]) with pytest.raises(ArgumentResolutionError): container.resolve(_DeferBrokenService) From 6c998c5eebffd6b6135a610986a41a1ce9fa187d Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 18:46:58 +0300 Subject: [PATCH 06/18] docs(comments): cite invariant tests instead of architecture/ pages Replaces every architecture/*.md pointer in modern_di/ and tests/ with the name of the test that actually enforces the claim, or a self-contained sentence where no test covers it. Widens test_invariant_census.py to scan docstrings as well as comments, and both modern_di/ and tests/, so the new citations (and the ~7 hand-verified ones from Task 2) are guarded against rename rot. --- modern_di/container.py | 8 ++--- modern_di/providers/factory.py | 6 ++-- modern_di/registries/cache_registry.py | 8 ++--- modern_di/registries/providers_registry.py | 3 +- modern_di/resolver_compiler.py | 13 ++++---- tests/providers/test_factory.py | 5 +-- tests/test_free_threading.py | 2 +- tests/test_invariant_census.py | 36 +++++++++++++++++----- tests/test_resolver_compiler.py | 4 +-- 9 files changed, 54 insertions(+), 31 deletions(-) diff --git a/modern_di/container.py b/modern_di/container.py index 0c5d23e..423d91c 100644 --- a/modern_di/container.py +++ b/modern_di/container.py @@ -130,8 +130,8 @@ def __init__( # noqa: PLR0913, PLR0917 self.providers_registry: ProvidersRegistry self.overrides_registry: OverridesRegistry # Inlined, not a helper: __init__ is on the per-request child-build path - # (architecture/performance.md). A root seeds container_provider so `Container` - # resolves to the resolving container. + # (see test_resolve_costs_exactly_one_resolver_frame_per_node). A root seeds + # container_provider so `Container` resolves to the resolving container. if parent_container: self.providers_registry = parent_container.providers_registry self.overrides_registry = parent_container.overrides_registry @@ -234,8 +234,8 @@ def resolve_provider(self, provider: "AbstractProvider[types.T]") -> types.T: self._prepare() try: # Inlined memo hit; `resolver_for` is called only on a miss, where it owns the cycle - # guard and the memo write (architecture/performance.md). Inside the try so a - # RecursionError while compiling still becomes CircularDependencyError. + # guard and the memo write (see test_resolve_costs_exactly_one_resolver_frame_per_node). + # Inside the try so a RecursionError while compiling still becomes CircularDependencyError. registry = self.providers_registry resolver = registry._resolvers.get(provider.provider_id) # noqa: SLF001 if resolver is None: diff --git a/modern_di/providers/factory.py b/modern_di/providers/factory.py index 2a7a340..ffbc72e 100644 --- a/modern_di/providers/factory.py +++ b/modern_di/providers/factory.py @@ -165,9 +165,9 @@ def _argument_resolution_error( def _plan(self, container: "Container") -> WiringPlan: # Memoized on the shared providers registry, so a deeper-scope factory builds its plan once - # tree-wide (architecture/performance.md). Building runs outside the container lock — a - # deterministic function of the registry's contents, so a race at worst repeats the build - # (architecture/concurrency.md). + # tree-wide (see test_resolve_costs_exactly_one_resolver_frame_per_node). Building runs + # outside the container lock — a deterministic function of the registry's contents, so a + # race at worst repeats the build (see tests/test_free_threading.py). return container.providers_registry.plan_for(self, self._parsed_kwargs, self._kwargs) def get_dependencies(self, container: "Container") -> dict[str, "AbstractProvider[typing.Any]"]: diff --git a/modern_di/registries/cache_registry.py b/modern_di/registries/cache_registry.py index 3cc984b..241e9b6 100644 --- a/modern_di/registries/cache_registry.py +++ b/modern_di/registries/cache_registry.py @@ -87,10 +87,10 @@ def cached_count(self) -> int: def fetch_cache_item(self, provider: Factory[types.T_co]) -> CacheItem: # Get before setdefault: a plain setdefault eagerly builds a throwaway CacheItem on every - # hit (architecture/performance.md). The creation path keeps setdefault, whose atomicity is - # what makes concurrent first-resolvers share one CacheItem — and it runs outside the - # container lock (see architecture/concurrency.md), because the singleton cache - # and its double-checked lock live on that object. + # hit (see test_cached_resolver_has_no_cell_on_the_warm_path). The creation path keeps + # setdefault, whose atomicity is what makes concurrent first-resolvers share one CacheItem + # — and it runs outside the container lock, because the singleton cache and its + # double-checked lock live on that object. item = self._items.get(provider.provider_id) if item is not None: return item diff --git a/modern_di/registries/providers_registry.py b/modern_di/registries/providers_registry.py index 15c035e..070b412 100644 --- a/modern_di/registries/providers_registry.py +++ b/modern_di/registries/providers_registry.py @@ -142,7 +142,8 @@ def _invalidate(self) -> None: Called under `self._lock` by every mutation. Clearing has the same breadth the old version bump did (a bump invalidated every memo anyway) and frees stale entries eagerly. Sound - because mutation is a single-threaded configure-phase operation (architecture/concurrency.md). + because mutation is a single-threaded configure-phase operation (configure-phase mutation; + see tests/test_free_threading.py). """ self._plans.clear() self._resolvers.clear() diff --git a/modern_di/resolver_compiler.py b/modern_di/resolver_compiler.py index 40b0eac..5490c75 100644 --- a/modern_di/resolver_compiler.py +++ b/modern_di/resolver_compiler.py @@ -4,8 +4,8 @@ the navigation via an int compare), inlines the kwargs build and creator call, and calls its dependencies' resolvers by reference. Behavior-sensitive helpers (`_resolution_step`, `prepend_step`) are reused, not reimplemented. Context kwargs are folded at compile time -- -`ContextProvider.scope` and `.context_type` are fixed once registered, see -architecture/providers.md -- so the whole context lookup is inline here and owns its behaviour. +`ContextProvider.scope` and `.context_type` are fixed once registered, so the whole context +lookup is inline here and owns its behaviour (see test_same_scope_context_hop_does_not_call_find_container). """ import functools @@ -94,7 +94,7 @@ def _compile_transient_factory( # noqa: C901, PLR0915 (two hot-path closures: p if _can_call_positionally(f, plan): # Positional fast path; `pure` is True here, so no static/context folding runs. - # See architecture/performance.md. + # See test_resolve_costs_exactly_one_resolver_frame_per_node. pos = tuple(r for _name, r in prov) # Arity ladder. `len(pos)` is fixed at compile time, so 0 and 1 deps get a closure that @@ -158,7 +158,8 @@ def resolve_arity1(container: "Container") -> typing.Any: return resolve_arity1 def resolve_positional(container: "Container") -> typing.Any: - # Inlined per closure, not extracted: frame budget — see architecture/performance.md. + # Inlined per closure, not extracted: frame budget -- see + # test_resolve_costs_exactly_one_resolver_frame_per_node. overrides = container.overrides_registry if overrides.has_overrides: override = overrides.fetch_override(pid) @@ -320,7 +321,7 @@ def resolve(container: "Container") -> typing.Any: if target.closed: target._prepare() # Inlined memo hit; the method is called only on a miss, where its `setdefault` makes - # concurrent first-resolvers share one CacheItem. See architecture/performance.md. + # concurrent first-resolvers share one CacheItem. See test_cached_resolver_has_no_cell_on_the_warm_path. cache_registry = target.cache_registry cache_item = cache_registry._items.get(pid) if cache_item is None: @@ -332,7 +333,7 @@ def resolve(container: "Container") -> typing.Any: target._lock, # `partial`, never a lambda closing over `target`: a closure promotes `target` to a cell, # so MAKE_CELL runs in this resolver's prologue on every warm hit too. See - # architecture/performance.md. + # test_cached_resolver_has_no_cell_on_the_warm_path. resolve=functools.partial(build_cold, target), # positional/kwargs builders have distinct arg types; get_or_create feeds each its own. create=typing.cast("typing.Callable[[typing.Any], typing.Any]", create_cold), diff --git a/tests/providers/test_factory.py b/tests/providers/test_factory.py index 69e9f68..55803b9 100644 --- a/tests/providers/test_factory.py +++ b/tests/providers/test_factory.py @@ -183,8 +183,9 @@ def test_factory_overridden_request_scope() -> None: def test_override_bypasses_scope_check_from_shallower_container() -> None: # Documented intentional: an override is returned before the scope check, so a - # deeper-scoped provider can be resolved from a shallower container — see - # architecture/testing-and-overrides.md "Scope behaviour under overrides". + # deeper-scoped provider can be resolved from a shallower container. An overridden + # provider resolves from whichever container was asked -- the short-circuit fires + # before find_container. app_container = Container(groups=[MyGroup]) app_container.open() with pytest.raises(ScopeNotInitializedError): diff --git a/tests/test_free_threading.py b/tests/test_free_threading.py index 4e49ccb..93608d7 100644 --- a/tests/test_free_threading.py +++ b/tests/test_free_threading.py @@ -5,7 +5,7 @@ and the setdefault-shared CacheItem; on a 3.14t build it runs those paths GIL-free. The free-threaded *interpreter* assertion lives in CI (_checks.yml), not here, to keep this suite version-agnostic and 100%-line-covered on every build. -See architecture/concurrency.md. +See planning/decisions/2026-08-11-free-threaded-beta-not-stable.md. """ import threading diff --git a/tests/test_invariant_census.py b/tests/test_invariant_census.py index d6b6240..67b7eb2 100644 --- a/tests/test_invariant_census.py +++ b/tests/test_invariant_census.py @@ -1,8 +1,9 @@ -"""Census of invariant tests and the comments that cite them. +"""Census of invariant tests and the citations that point to them. -Every ``test_*`` name cited from a comment in ``modern_di/`` resolves to a real test, 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 ``architecture/`` pages. +Every ``test_*`` name cited from a comment or docstring under ``modern_di/`` or ``tests/`` +resolves to a real test, 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. """ import ast @@ -22,6 +23,9 @@ # 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 _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: found = [] @@ -35,7 +39,11 @@ def _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFun return found -def _cited_names(path: pathlib.Path) -> set[str]: +def _citation_paths() -> list[pathlib.Path]: + return sorted({*_SRC_DIR.rglob("*.py"), *_TESTS_DIR.rglob("*.py")}) + + +def _comment_citations(path: pathlib.Path) -> set[str]: """Names cited from real comments only -- tokenize, so a `#` inside a string is not a comment.""" with path.open("rb") as handle: return { @@ -46,17 +54,29 @@ def _cited_names(path: pathlib.Path) -> set[str]: } +def _docstring_citations(path: pathlib.Path) -> set[str]: + """Names cited from 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(_CITATION.findall(docstring)) + return names + + 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 sorted(_SRC_DIR.rglob("*.py")) - for name in _cited_names(path) + for path in _citation_paths() + for name in _comment_citations(path) | _docstring_citations(path) if name not in known ) - assert not orphans, f"comments cite tests that do not exist: {orphans}" + assert not orphans, f"comments or docstrings cite tests that do not exist: {orphans}" def test_every_invariant_states_what_breaks_it() -> None: diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index 54e4aed..59b960b 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -114,8 +114,8 @@ class _L5: #: Python calls one extra chain node costs: its resolver closure, plus its creator. #: The creator is the user's own object construction and is irreducible; the **1** -#: resolver frame is the budget this module exists to hold. See -#: ``architecture/performance.md``. +#: resolver frame is the budget this module exists to hold. Pinned below by +#: ``test_resolve_costs_exactly_one_resolver_frame_per_node``. #: #: Version-independent because these chain nodes have arity 1, which the positional #: path compiles to a closure that names its argument and calls the creator directly From 031c31a5d62c392b6fac08cb455d85a9bdba751a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 18:57:21 +0300 Subject: [PATCH 07/18] test: pin the one-graph, fails-loudly and import-purity invariants --- tests/test_custom_scope.py | 21 +++++++++++++++++++ tests/test_dependency_graph_contract.py | 28 +++++++++++++++++++++++++ tests/test_resolver_compiler.py | 22 ++++++++++++++++++- 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/tests/test_custom_scope.py b/tests/test_custom_scope.py index 15dfb04..4e67841 100644 --- a/tests/test_custom_scope.py +++ b/tests/test_custom_scope.py @@ -1,8 +1,11 @@ +import ast import dataclasses import enum +import pathlib import pytest +import modern_di.scope from modern_di import Container, Group, Scope, providers from modern_di.exceptions import ( InvalidChildScopeError, @@ -189,3 +192,21 @@ class ZeroEnum(enum.IntEnum): parent.open() with pytest.raises(InvalidChildScopeError): parent.build_child_container(scope=ZeroEnum.ZERO) + + +def test_scope_module_imports_only_enum() -> None: + """INVARIANT: `modern_di/scope.py` imports nothing but `enum`. + + `exceptions.py` imports `_deeper_members` to derive `InvalidChildScopeError.allowed_scopes`, so + a `scope.py` that imported `exceptions` would cycle. That is why `_next_deeper` returns `None` + at the deepest member instead of raising `MaxScopeReachedError` itself. + """ + tree = ast.parse(pathlib.Path(modern_di.scope.__file__).read_text(encoding="utf-8")) + imported: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + imported.add(alias.name.split(".")[0]) + elif isinstance(node, ast.ImportFrom) and node.module: # pragma: no cover - scope.py has no `from` import today + imported.add(node.module.split(".")[0]) + assert imported == {"enum"}, f"scope.py grew imports: {sorted(imported)}" diff --git a/tests/test_dependency_graph_contract.py b/tests/test_dependency_graph_contract.py index fed7fbd..e637e45 100644 --- a/tests/test_dependency_graph_contract.py +++ b/tests/test_dependency_graph_contract.py @@ -95,3 +95,31 @@ class G(Group): container.open() with pytest.raises(exceptions.CircularDependencyError): container.resolve(_A) + + +def test_validate_walks_the_same_edges_resolve_follows() -> None: + """INVARIANT: the graph validate() walks is the graph resolve() follows. + + Edges come from `WiringPlan.edges`, a view derived from the same buckets resolve() reads, so a + provider named in a declaration-time `kwargs={...}` is an edge like any type-matched one. + Assembling the validation edge set separately would let the two drift, and a cycle routed + through a `kwargs=` provider would surface as a bare RecursionError instead. + """ + + class _Leaf: ... + + class _Root: + def __init__(self, leaf: _Leaf) -> None: + self.leaf = leaf # pragma: no cover - validate() never instantiates providers + + class G(Group): + leaf = Factory(scope=Scope.REQUEST, creator=_Leaf) + # Named via kwargs, not type-matched: the by-type pass skips a name present in kwargs, + # so this edge exists only if the overlay pass feeds it into WiringPlan.edges. + root = Factory(scope=Scope.APP, creator=_Root, kwargs={"leaf": leaf}) + + container = Container(scope=Scope.APP, groups=[G]) + with pytest.raises(exceptions.ValidationFailedError) as caught: + container.validate() + + assert any(isinstance(error, exceptions.InvalidScopeDependencyError) for error in caught.value.errors) diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index 59b960b..b58444f 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -17,8 +17,9 @@ from modern_di import Container, Group, Scope, exceptions, providers from modern_di.providers import ContextProvider +from modern_di.providers.abstract import AbstractProvider from modern_di.registries.providers_registry import ProvidersRegistry -from modern_di.resolver_compiler import _can_call_positionally +from modern_di.resolver_compiler import _can_call_positionally, compile_resolver from modern_di.wiring import WiringPlan @@ -643,3 +644,22 @@ class G(Group): assert code.co_cellvars == (), ( f"the cached-factory resolver grew cell variables {code.co_cellvars}; a MAKE_CELL now runs on every warm hit" ) + + +# --------------------------------------------------------------------------- +# compile_resolver dispatch — an unhandled provider type fails at compile time +# --------------------------------------------------------------------------- + + +def test_compile_resolver_rejects_an_unknown_provider_type() -> None: + """INVARIANT: a provider type with no compiler branch raises TypeError. + + There is no interpreted fallback to inherit shared behaviour from, so a new provider type that + forgets its branch must fail at compile time rather than resolve to something plausible. + """ + + class _Unsupported(AbstractProvider[int]): + __slots__ = () + + with pytest.raises(TypeError, match="no compiled resolver for provider type _Unsupported"): + compile_resolver(_Unsupported(scope=Scope.APP, bound_type=None), ProvidersRegistry()) From 69ad3d674197496656df411a4772e7e3b9673b97 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 19:04:34 +0300 Subject: [PATCH 08/18] fix(test): catch relative imports in the scope.py import-purity check --- tests/test_custom_scope.py | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/test_custom_scope.py b/tests/test_custom_scope.py index 4e67841..b79450f 100644 --- a/tests/test_custom_scope.py +++ b/tests/test_custom_scope.py @@ -194,19 +194,41 @@ class ZeroEnum(enum.IntEnum): parent.build_child_container(scope=ZeroEnum.ZERO) -def test_scope_module_imports_only_enum() -> None: - """INVARIANT: `modern_di/scope.py` imports nothing but `enum`. +def _module_level_imports(source: str) -> set[str]: + """Top-level module names `source` imports, from both `import x` and `from x import y`. - `exceptions.py` imports `_deeper_members` to derive `InvalidChildScopeError.allowed_scopes`, so - a `scope.py` that imported `exceptions` would cycle. That is why `_next_deeper` returns `None` - at the deepest member instead of raising `MaxScopeReachedError` itself. + A relative `from . import y` parses to `ImportFrom(module=None, level=1, ...)` -- `node.module` + is `None`, so that case falls back to the names in `node.names` themselves rather than + silently dropping the import (which would let a `from . import exceptions` pass unnoticed). """ - tree = ast.parse(pathlib.Path(modern_di.scope.__file__).read_text(encoding="utf-8")) + tree = ast.parse(source) imported: set[str] = set() for node in ast.walk(tree): if isinstance(node, ast.Import): for alias in node.names: imported.add(alias.name.split(".")[0]) - elif isinstance(node, ast.ImportFrom) and node.module: # pragma: no cover - scope.py has no `from` import today - imported.add(node.module.split(".")[0]) + elif isinstance(node, ast.ImportFrom): + if node.module: + imported.add(node.module.split(".")[0]) + else: + for alias in node.names: + imported.add(alias.name.split(".")[0]) + return imported + + +def test_scope_module_imports_only_enum() -> None: + """INVARIANT: `modern_di/scope.py` imports nothing but `enum`. + + `exceptions.py` imports `_deeper_members` to derive `InvalidChildScopeError.allowed_scopes`, so + a `scope.py` that imported `exceptions` would cycle. That is why `_next_deeper` returns `None` + at the deepest member instead of raising `MaxScopeReachedError` itself. + """ + source = pathlib.Path(modern_di.scope.__file__).read_text(encoding="utf-8") + imported = _module_level_imports(source) assert imported == {"enum"}, f"scope.py grew imports: {sorted(imported)}" + + # Prove the extractor itself would catch a relative import of the forbidden dependency -- the + # assertion above is only trustworthy if this branch is real, not a no-op. + assert _module_level_imports("from . import exceptions\n") == {"exceptions"} + # And the absolute `from x import y` form, so both `ImportFrom` branches are genuinely exercised. + assert _module_level_imports("from enum import IntEnum\n") == {"enum"} From b8451c5e36a6be33649a0889b150bb9d57f8bc0a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 19:11:21 +0300 Subject: [PATCH 09/18] docs(decisions): record the contracts architecture/ carried --- .../2026-08-11-drop-architecture-directory.md | 87 +++++++++++++++++++ ...026-08-11-free-threaded-beta-not-stable.md | 66 ++++++++++++++ ...11-integration-kit-per-adapter-boundary.md | 62 +++++++++++++ ...8-11-override-value-is-not-type-checked.md | 66 ++++++++++++++ ...ebinding-an-in-use-provider-unsupported.md | 60 +++++++++++++ ...11-transient-teardown-order-unspecified.md | 53 +++++++++++ 6 files changed, 394 insertions(+) create mode 100644 planning/decisions/2026-08-11-drop-architecture-directory.md create mode 100644 planning/decisions/2026-08-11-free-threaded-beta-not-stable.md create mode 100644 planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md create mode 100644 planning/decisions/2026-08-11-override-value-is-not-type-checked.md create mode 100644 planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md create mode 100644 planning/decisions/2026-08-11-transient-teardown-order-unspecified.md diff --git a/planning/decisions/2026-08-11-drop-architecture-directory.md b/planning/decisions/2026-08-11-drop-architecture-directory.md new file mode 100644 index 0000000..a700f81 --- /dev/null +++ b/planning/decisions/2026-08-11-drop-architecture-directory.md @@ -0,0 +1,87 @@ +--- +summary: The `architecture/` directory is deleted; its facts route to code, a named test, `planning/decisions/`, or `docs/` instead of a prose page that restates the code and drifts. +--- + +# Drop the `architecture/` directory + +**Decision:** `architecture/` is deleted. What it used to carry now routes to one of four homes: an +enforceable claim becomes an `INVARIANT:`-marked test guarded by `tests/test_invariant_census.py`; a +negative contract (something deliberately *not* guaranteed) becomes a `planning/decisions/` record; a +framework-facing contract moves to `docs/`; a term worth pinning down that isn't fully derivable from +code stays in whatever residual glossary form `planning/` ends up using. Nothing is promoted to a +standalone prose "truth home" page again. + +## Context + +`architecture/` was 11 prose pages, one per capability, meant to be the living, code-current account of +the library's behaviour — the `CLAUDE.md` at the repo root called it out as "quick orientation only... +the authoritative, code-current account of each capability." The convention was: a behaviour change +hand-edits the matching capability page in the same PR, and a checklist item asked reviewers to confirm +that happened. + +That promotion discipline **worked** — PRs did update the pages. It just didn't work in the direction +that keeps a doc small. Two prior attempts tried to correct the resulting bloat by cutting the pages +back down rather than questioning whether the pages should exist at all: + +- **`b2404c4`** (#282, 2026-07-07, "docs: trim architecture/ to charter — invariants only") — a + 433-line net cut. +- **`047b6ea`** (#395, 2026-07-29, "docs(architecture): thin resolution.md to invariants; one owner per + concept") — a 292-line net cut from `resolution.md` alone. + +Both regrew. `resolution.md` — a 175-line file at the point this decision was made — accumulated ++617/−442 lines across 23 commits since it existed; a page trimmed twice still ended up net-larger than +either trim removed. Contrast `glossary.md`, which took +93/−4 across only 2 commits: a page that +mostly just gets written once and referenced, not continuously re-edited to track behaviour, doesn't +have this problem at all. The asymmetry is the tell — it isn't that contributors write bloated prose in +general, it's specifically the pages the promotion checklist forces continuous re-editing of. + +## Decision & rationale + +**The promotion discipline was not the failure — it was working exactly as designed, and that's the +problem.** Measured at the branch point for this change: of the 67 commits touching `modern_di/` since +mid-June, 48 also touched `architecture/` — 72%. That's a *high* compliance rate with "did you update +the page?" The checklist item that drove it only ever asked that one question. It never asked "should +this paragraph exist?" So every PR that touched behaviour had a structural incentive to *add* a +sentence explaining the new behaviour, and no PR had any correspondent incentive to *remove* a sentence +whose enforcement value had already been captured elsewhere — in a test, in a type signature, in the +code itself being self-evident on read. Addition-without-subtraction, run for weeks across dozens of +compliant PRs, is exactly the churn profile `resolution.md` shows. + +Tasks 1–4 of this branch already did the harder part of the fix: they read the 11 pages, found the +claims that were actually enforceable, and turned 30 of them into `INVARIANT:`-marked tests. A test +regresses automatically when it stops being true; a prose paragraph doesn't. That converts "the page +still says X" from a discipline problem back into a mechanical one. + +**What's left — five records in this file's cohort — could not go into a test**, because their entire +content is the *absence* of a guarantee: no assertion follows from "the library does not promise this +order" or "this value is not type-checked." Deleting the page without capturing them would silently +turn each into either an unfixed bug report (someone "fixes" the unspecified transient teardown order) +or an unsupported reliance (someone starts depending on override values being type-checked because +nothing said otherwise). `planning/decisions/` is the right home because these are exactly what that +directory is for: options considered and a call made, with reasoning a future explorer would otherwise +re-litigate. + +**Rejected alternative: keep a smaller `architecture/`.** This is the option the two prior thinnings +already tried, twice, and it regrew both times. A third attempt with a stricter charter has no reason +to fare differently unless the underlying incentive changes — and the underlying incentive is the +promotion checklist itself, which this decision removes rather than tightens. + +**Rejected alternative: turn the glossary into a decision record.** A glossary is consulted *while +writing* — a contributor mid-PR needs to know what `bound_type` means right now, inline with the code +they're editing. A decision record is consulted *while deciding* — before writing, to check whether an +option was already rejected. Those are different reading moments; collapsing them into one file type +would serve neither well. + +**Rejected alternative: drop the glossary entirely, relying on code and docstrings.** Rejected because +the glossary's `Avoid:` entries — the deliberately-not-used synonyms for a term, and why — are not +derivable from reading the code. Code shows what a name *is*; it doesn't show what a contributor +*almost* called it instead and why that was wrong. That negative information has nowhere else to live. + +## Revisit trigger + +An invariant is found that has no test form (nothing to assert), no decision form (it isn't a call +between rejected alternatives — it's simply a fact worth stating), and is needed by more than one +reader. A single such fact is better placed in the nearest docstring or `docs/` page; a *pattern* of +such facts, recurring enough that they'd naturally cluster into one file again, is the signal that a +`architecture/`-shaped truth home is needed after all — and if that happens, the fix this time is a +charter that states what does *not* belong on the page, not just what does. diff --git a/planning/decisions/2026-08-11-free-threaded-beta-not-stable.md b/planning/decisions/2026-08-11-free-threaded-beta-not-stable.md new file mode 100644 index 0000000..e5d98c3 --- /dev/null +++ b/planning/decisions/2026-08-11-free-threaded-beta-not-stable.md @@ -0,0 +1,66 @@ +--- +summary: Free-threaded (PEP 703) support is Beta, not Stable, because it relies on object-publication ordering that CPython's implementation provides but does not formally guarantee. +--- + +# Free-threaded support is Beta, not Stable + +**Decision:** modern-di's free-threaded (PEP 703) support is labeled Beta. It stays Beta rather than +graduating to Stable until the one guarantee it currently borrows from CPython's implementation +behaviour, rather than from CPython's spec, is either formalized upstream or removed from modern-di's +own reliance. + +## Context + +Free-threaded CPython makes single built-in-container operations (`dict.setdefault`, `dict[k] = v`, +`dict.get`, `list.append`) internally atomic — no single such operation can corrupt the structure. +modern-di's concurrency design leans on that: every compound check-then-act sequence over shared state +in the resolve path is either idempotent (a rebuild-if-stale race just produces a duplicate, discarded +build) or already runs under the container's own lock. Registry *mutation* (`register`, +`add_providers`, removal) is guarded by the registry's own lock; the cycle-guard `_building` set is +thread-local, so a same-thread cycle is still caught while a concurrent first-resolve of the same +provider on another thread just compiles it independently. + +That much is sound on the guarantees CPython actually documents. But one more thing is needed for +correctness: when one thread publishes a newly-built object (a compiled resolver, a cached instance) +by storing a reference where another thread will read it, the reading thread must see that object's +fully-initialized fields, not a partially-constructed one. That's object-publication ordering, and +CPython's language spec does not formally guarantee it — CPython publishes no memory model. + +## Decision & rationale + +**The gap is between implementation behaviour and spec guarantee, and that gap is exactly what Beta +means here.** In the current CPython implementation, publication that happens through a container's +internal critical section does provide the necessary ordering — so today's behaviour is correct on +every free-threaded build tested. But "correct because of how the interpreter happens to be built" +and "correct because the language promises it" are different claims, and only the second one is safe +to call Stable. A future CPython release is free to change unspecified implementation behaviour +without that being a compatibility break by CPython's own rules, even though it would be one for +modern-di's free-threaded users. + +Two adjacent things are explicitly *not* what keeps this at Beta, and are worth separating out because +they get conflated with the ordering question: + +- **Thread-safety itself is not in question.** Concurrent resolve is thread-safe under the compound-op + analysis above; that part doesn't move. +- **Throughput is a separate, already-tracked concern.** Concurrent resolve is thread-safe but its + throughput does not scale with thread count on a free-threaded build — diagnosed as atomic + refcount contention on shared hot-path objects (the returned singleton value, then the shared + provider objects, then the compiled-resolver closures' captured cells), not the per-container lock. + That's a performance ceiling CPython itself would have to lift (deferred reference counting + expanding to ordinary instances and cells), not a correctness gap, and it's tracked separately in + [`2026-07-19-free-threaded-throughput.md`](../deferred/2026-07-19-free-threaded-throughput.md). + +**Caveats that hold regardless of Beta/Stable status:** configure and close are single-threaded edges. +`override`/`reset_override` and `set_context` mutate shared state without a lock — racing them against +a live `resolve()` is inherently unordered, GIL or not. `close`/`open` are the same: tear a container +down only after concurrent resolution has stopped. These are usage contracts, not bugs, and staying at +Beta doesn't change them. + +## Revisit trigger + +CPython documents a formal memory model for free-threaded builds that covers the publication ordering +this relies on — at that point the reliance becomes a spec guarantee rather than an implementation +behaviour, and Beta can graduate to Stable. Absent that, a CPython release that changes unspecified +publication-ordering behaviour and breaks modern-di's free-threaded tests would confirm the gap is real +rather than theoretical, and is itself grounds to keep the label at Beta indefinitely rather than +guessing at a graduation date. diff --git a/planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md b/planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md new file mode 100644 index 0000000..eda0a87 --- /dev/null +++ b/planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md @@ -0,0 +1,62 @@ +--- +summary: Root-container lifecycle, per-connection child stash/read-back, sync-vs-async close, and handler-signature rewriting stay in each adapter — the integration kit does not absorb them. +--- + +# What stays per-adapter is not part of the integration kit + +**Decision:** Four responsibilities stay outside `modern_di.integrations` and are re-implemented by +each of the 13 framework adapters individually: root-container lifecycle (open/close, where it's +attached to framework state), where the per-connection child container is stashed and read back, +choosing `close_sync` vs. `close_async`, and any handler-signature rewriting (stripping a parameter, +inserting a context object). None of these move into the shared kit. + +## Context + +`modern_di.integrations` extracts what the 13 adapters duplicated near-verbatim: Layer 1 +(`bind`, `classify_connection`) derives a child container's scope/context from `ContextProvider`s, and +Layer 2 (`Marker`, `from_di`, `parse_markers`, `resolve_markers`) is the `Annotated`-marker injector, +replacing the `_parse_inject_params`/resolve pair every non-native-DI integration had reimplemented. +The related decision [`2026-07-13-integration-kit-shape.md`](2026-07-13-integration-kit-shape.md) +settled the kit's overall shape (low-level primitives in core, outliers bypass rather than the +primitives absorbing them). This record is narrower: it's about the four things that were considered +for extraction *into* the kit and rejected, not about the outlier adapters that bypass it. + +## Decision & rationale + +Each of the four is irreducibly framework-specific in a way the shared primitives are not: + +- **Root-container lifecycle** — *where* a framework hangs the root container (an ASGI app's + `state`, a Celery worker's global, a Typer command's context object) and *when* it opens and closes + it are governed entirely by that framework's own lifecycle hooks. There's no shared shape to + extract; a common "lifecycle manager" would need a callback per framework anyway, which is just the + per-adapter code with extra indirection. +- **Where the per-connection child is stashed and read back** — an HTTP framework has a request + object, an ASGI framework has a scope dict, a message-queue framework has neither and uses a + contextvar or a task-local. The storage medium varies by what the framework hands the adapter, not + by anything modern-di controls. +- **`close_sync` vs. `close_async`** — which one an adapter calls depends on whether the framework's + own request/task teardown hook is sync or async, which is a property of the framework's execution + model, not of the container. +- **Handler-signature rewriting** — stripping an injected parameter or inserting a context object + before calling the user's handler requires knowing that framework's handler-calling convention + (decorator-wrapped function, class-based view method, positional vs. keyword dispatch). There is no + framework-agnostic way to rewrite "a handler" in general. + +**Rejected alternative: grow the primitives to absorb these.** This was the same shape of argument +`2026-07-13-integration-kit-shape.md` already settled for the three concrete outliers (aiohttp +websocket probe, grpc `set_context` split, typer no-context) — an absorbing parameter needed by one +adapter taxes the other adapters and lowers the kit's depth. These four are the general case of that +same argument: each is needed by *every* adapter, but in a *different shape* per adapter, so there is +no single parameter or callback that would fit all 13 without becoming a second, parallel dispatch +mechanism duplicating what the framework already provides. + +The full contract an integration implements around the shared primitives — including these four +per-adapter responsibilities — is documented in +[docs/integrations/writing-integrations.md](../../docs/integrations/writing-integrations.md). + +## Revisit trigger + +A second and third adapter converge on the *same* concrete shape for stashing the per-connection +child, or for signature rewriting — not just the same responsibility, but the same mechanism — making +a shared helper a two-adapters-rule extraction rather than a hypothetical one, the same bar +`2026-07-13-integration-kit-shape.md` used for its outliers. diff --git a/planning/decisions/2026-08-11-override-value-is-not-type-checked.md b/planning/decisions/2026-08-11-override-value-is-not-type-checked.md new file mode 100644 index 0000000..4f601b4 --- /dev/null +++ b/planning/decisions/2026-08-11-override-value-is-not-type-checked.md @@ -0,0 +1,66 @@ +--- +summary: An override value bypasses the scope check, cache lookup and creator, so it need not be an instance of the declared type and a REQUEST-scoped provider can be overridden and resolved from an APP container. +--- + +# An override value is not type-checked or scope-checked + +**Decision:** `container.override(provider, obj)` does not require `obj` to be an instance of the +provider's declared type, and does not require the resolving container to satisfy the provider's +declared scope. Both checks are structurally bypassed by where the override short-circuit sits in +`resolve_provider`, and neither is going to be added. + +## Context + +`resolve_provider` checks the override registry *before* delegating to the provider at all. If an +override is present, its value is returned directly — bypassing the scope check, the cache lookup, and +the creator invocation entirely. This is what makes overrides fast and simple to reason about +mechanically, but it also means none of the machinery that would normally validate a resolved value +ever runs on an override. + +Two concrete consequences fall out of that positioning: + +- **No type check.** The override object does not need to be an instance of the provider's declared + type at runtime — Python does not enforce it, and modern-di adds no check of its own. A caller who + passes an incompatible object gets no error at override time and no error at resolve time; they get + exactly the wrong object back, silently. +- **No scope check.** An overridden provider is resolved from whichever container `resolve_provider` + is called on — the original provider's declared scope is irrelevant, because the short-circuit fires + before `find_container` ever runs. In practice, a REQUEST-scoped provider can be overridden and + resolved from an APP-scoped container without raising `ScopeNotInitializedError`. + +Overrides also don't interact with the cache: if a singleton was already resolved before +`container.override(...)` was called, subsequent `resolve_provider` calls return the override value, +not the cached instance, and after `reset_override` the original cache entry (if any) is still present +and returned again. That's a related but separate behaviour from the two checks above — it follows +from the same "override fires first" positioning but isn't itself a missing check. + +## Decision & rationale + +**The scope bypass is a feature, not an oversight — it's often exactly what tests want.** A test that +overrides a REQUEST-scoped database provider with a stub, then resolves it from the APP-scoped root +container without building a child container down to REQUEST first, is a common and legitimate +pattern. Requiring the caller to build the full scope chain just to install a stub defeats much of the +point of having overrides at all. This is also why the mechanism lives ahead of the scope check +structurally, not behind it: putting it behind would mean paying the scope-chain-walk cost that +overrides exist partly to let callers skip. + +**The type bypass is an accepted cost of the same positioning, not a separately chosen feature.** +Adding a runtime `isinstance` check would mean either checking it against `bound_type` (which doesn't +handle generics, protocols, or duck-typed test doubles — the exact things overrides are commonly used +for) or against nothing meaningful. A check narrow enough to be correct would reject legitimate test +doubles; a check loose enough to accept them would catch almost nothing. So the responsibility is left +with the caller: "callers should pass a compatible object for type safety" is a documented expectation, +not an enforced one. + +**Consequence worth naming together:** these two bypasses compound. An override can supply an object +of the wrong type *and* be resolved from a container that could never have satisfied the original +provider's scope, and nothing in the resolve path will catch either. That's the accepted shape of the +mechanism, not a partially-fixed bug. + +## Revisit trigger + +A real report of a production bug caused by an override silently returning a type-incompatible object +— as opposed to a test-time convenience use, which is the mechanism working as designed. At that point +the design question is narrower than "add type checking to overrides" — it's whether an *opt-in* +strict-override mode is worth the API surface, given that the common case (test doubles, protocols) +is exactly what a blanket check would break. diff --git a/planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md b/planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md new file mode 100644 index 0000000..8300360 --- /dev/null +++ b/planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md @@ -0,0 +1,60 @@ +--- +summary: Changing a ContextProvider's `scope` or `context_type` after a consumer's resolver has compiled against it is unsupported and only partially enforced — construct a second provider instead. +--- + +# Rebinding an in-use `ContextProvider` is unsupported + +**Decision:** Mutating a `ContextProvider`'s `scope` or `context_type` after something has already +resolved through it — compiling a consumer's resolver closure against it — is not a supported +operation. This is a contract, not a mechanism: enforcement is inconsistent by design, not a gap to +close. Construct a second provider instead of rebinding an existing one. + +## Context + +A `ContextProvider`'s `scope` and `context_type` are read once, when a consumer's resolver is +compiled, and folded directly into that closure. Nothing about either attribute touches a registry, +so nothing invalidates the memo when they change afterward. Changing either attribute post hoc applies +only to resolvers compiled *later* — silently, with no error and no signal that older, already-compiled +consumers are now working from a stale value. + +Whether that silent staleness is caught at all depends on which attribute and which route: + +- `scope` on a **registered** provider is enforced against group stamping by + `ProviderScopeFrozenError` — attempting to re-stamp a registered provider's scope raises. +- `scope` on a provider that was never registered — one passed only inline, e.g. + `Factory(creator, kwargs={"x": cp})` — is **not** enforced. `_registered` stays `False` for such a + provider, so a later `Group` can stamp its scope without error, silently. +- `context_type` is **not enforced on either route**. There is no equivalent guard for it at all. + +So three of the four (attribute, route) combinations either enforce nothing or enforce it +inconsistently with the fourth. That asymmetry was deliberate at the time each piece was built — +`ProviderScopeFrozenError` exists to protect group stamping, not attribute mutation in general — but +it means a caller cannot rely on an exception to catch a rebind. + +## Decision & rationale + +**Declare the whole surface unsupported rather than patch the three unguarded corners.** Closing all +three gaps would mean tracking "has anything resolved through this provider yet" as new mutable state +on every `ContextProvider`, checked on every attribute write, to protect an operation (rebinding scope +or context type on a live provider) that has no legitimate use case distinguishable from a bug: nobody +needs a provider that changes identity mid-flight, and existing resolved values are not migrated with +it in any case. + +The one exception path — a `ContextProvider` passed via `kwargs={...}` for a parameter with no parsed +`SignatureItem` (a `**kwargs` creator, or `skip_creator_parsing=True`) — still goes through +direct-resolve semantics and raises `ContextValueNotSetError` when unset, which is unrelated to this +decision; it's about absence, not about rebinding an already-resolved provider. + +**Accepted cost:** a caller who does rebind a never-registered provider's `scope`, or either route's +`context_type`, gets no error — just resolvers that silently disagree about what type or scope the +provider has, split along whichever side of the compile boundary they landed on. This is disclosed +rather than fixed because fixing it costs a mutable per-provider tracking flag for a mistake that has +no other repro path in the test suite or issue history. + +## Revisit trigger + +A real bug report traces back to one of the three unguarded corners — most plausibly the +never-registered-provider `scope` gap, since that one silently *succeeds* where the registered case +raises. At that point the fix is a symmetric guard (extend `ProviderScopeFrozenError`-style enforcement +to the unregistered route, and add an equivalent for `context_type`) rather than continuing to disclaim +it. diff --git a/planning/decisions/2026-08-11-transient-teardown-order-unspecified.md b/planning/decisions/2026-08-11-transient-teardown-order-unspecified.md new file mode 100644 index 0000000..847815f --- /dev/null +++ b/planning/decisions/2026-08-11-transient-teardown-order-unspecified.md @@ -0,0 +1,53 @@ +--- +summary: The order in which a resolver collects transient (uncached) dependencies is not part of the contract, even though the arity ladder happens to preserve it today. +--- + +# Transient teardown order is unspecified + +**Decision:** The order in which transient dependencies are collected during resolution is not +part of modern-di's contract. A future change to the resolver's shape may alter it without that +being a breaking change. + +## Context + +An uncached (transient) dependency that its consumer's creator uses and drops — never retains — is +freed by CPython's ordinary refcounting the moment the resolver's local reference to it goes out of +scope. modern-di manages no finalizer for such an object: `CacheSettings(finalizer=)` only applies to +cached providers, and `close_sync`/`close_async` only tear down what a container owns. So the *only* +place this order is observable at all is the drop order of objects a creator never kept a reference +to — and that order falls out of however the resolver's compiled closure happens to hold its locals, +not from any stated rule. + +The question came up concretely when the positional fast-path's arity ladder landed: arity 0 and 1 +compile to a closure that names its argument and calls the creator directly; arity 2+ still builds a +list and star-calls it. Naming vs. list-building are different mechanisms for holding intermediate +values, so it was worth checking whether they drop objects in a different order. + +## Decision & rationale + +**Nothing observable changed when the arity ladder landed, and the reason generalizes.** The ladder +caps at arity 1, so it never holds more than one named local at a time — there is no order to alter +between one item and itself. Measured directly, main vs. ladder, on CPython 3.10 and 3.14, across +arities 1 through 3: the collection order is identical in every case tested. + +The rule is stated in advance of the case that would actually test it. A rung added at arity 2+ would +release named locals with the frame teardown rather than through the star-call's intermediate list — +and on CPython below 3.12, frame-local release order and list-teardown order are not the same thing. +Such a rung would be a legitimate performance change, not a breaking one, because the order was never +promised. Declaring the contract now means that future work doesn't have to treat "does this change +teardown order" as a correctness question — only a "did anyone tell users this order was reliable" +question, and the answer is no. + +**Accepted cost:** a user who has silently relied on today's incidental order (for example, using +transient side effects on drop as a poor man's ordering signal) gets no deprecation warning if a +future resolver shape changes it. This is deliberate — retaining that order would pin the resolver's +internal representation of arity 2+ closures indefinitely, for a guarantee nobody asked for and the +library never advertised. + +## Revisit trigger + +A rung is added to the arity ladder at arity 2 or higher (or the star-call path is otherwise +restructured) and CPython's frame-local teardown order diverges from list-teardown order on a +supported version. At that point, re-measure whether the divergence is observable by any real +finalizer-adjacent use case before deciding whether it's worth stabilizing rather than continuing to +disclaim it. From d31625450e686645126a2dabb3859a56ddeb2cf7 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 19:19:50 +0300 Subject: [PATCH 10/18] docs(decisions): correct thinning-commit churn figures in drop-architecture record --- .../2026-08-11-drop-architecture-directory.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/planning/decisions/2026-08-11-drop-architecture-directory.md b/planning/decisions/2026-08-11-drop-architecture-directory.md index a700f81..d38d012 100644 --- a/planning/decisions/2026-08-11-drop-architecture-directory.md +++ b/planning/decisions/2026-08-11-drop-architecture-directory.md @@ -23,10 +23,12 @@ That promotion discipline **worked** — PRs did update the pages. It just didn' that keeps a doc small. Two prior attempts tried to correct the resulting bloat by cutting the pages back down rather than questioning whether the pages should exist at all: -- **`b2404c4`** (#282, 2026-07-07, "docs: trim architecture/ to charter — invariants only") — a - 433-line net cut. +- **`b2404c4`** (#282, 2026-07-07, "docs: trim architecture/ to charter — invariants only") — 433 + deletions against 110 insertions across six pages, a net −323. - **`047b6ea`** (#395, 2026-07-29, "docs(architecture): thin resolution.md to invariants; one owner per - concept") — a 292-line net cut from `resolution.md` alone. + concept") — 292 deletions against 147 insertions across four pages (`README.md`, `containers.md`, + `providers.md`, `resolution.md`), a net −145; `resolution.md` alone was 206 deletions against 121 + insertions, a net −85. Both regrew. `resolution.md` — a 175-line file at the point this decision was made — accumulated +617/−442 lines across 23 commits since it existed; a page trimmed twice still ended up net-larger than @@ -38,9 +40,11 @@ general, it's specifically the pages the promotion checklist forces continuous r ## Decision & rationale **The promotion discipline was not the failure — it was working exactly as designed, and that's the -problem.** Measured at the branch point for this change: of the 67 commits touching `modern_di/` since -mid-June, 48 also touched `architecture/` — 72%. That's a *high* compliance rate with "did you update -the page?" The checklist item that drove it only ever asked that one question. It never asked "should +problem.** Measured at `ed9b00d`, the commit this branch forked from (deliberately excluding this +branch's own commits, so the figure describes the pre-existing problem rather than this branch's own +churn): of the 67 commits touching `modern_di/` since mid-June, 48 also touched `architecture/` — 72%. +That's a *high* compliance rate with "did you update the page?" The checklist item that drove it only +ever asked that one question. It never asked "should this paragraph exist?" So every PR that touched behaviour had a structural incentive to *add* a sentence explaining the new behaviour, and no PR had any correspondent incentive to *remove* a sentence whose enforcement value had already been captured elsewhere — in a test, in a type signature, in the From 36774922ef12b6a1a627edaf1fc7cc85cf5b754a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 19:27:21 +0300 Subject: [PATCH 11/18] docs: fold the free-threaded support level into design decisions --- docs/integrations/writing-integrations.md | 19 +++++++++++-------- docs/introduction/design-decisions.md | 8 ++++++++ docs/introduction/performance.md | 2 +- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/integrations/writing-integrations.md b/docs/integrations/writing-integrations.md index aec26ee..2b3e637 100644 --- a/docs/integrations/writing-integrations.md +++ b/docs/integrations/writing-integrations.md @@ -119,8 +119,9 @@ depends on how the framework runs handlers: scope + context for one provider without the isinstance scan. An adapter whose unit of work carries **no** connection object (a Typer command) skips the kit entirely and calls `build_child_container(scope=...)` directly. See - [architecture/integration-kit.md](https://github.com/modern-python/modern-di/blob/main/architecture/integration-kit.md) - for which shape fits which adapter. + [How the existing integrations realize the + contract](#how-the-existing-integrations-realize-the-contract) for which + shape fits which adapter. - **Middleware** (FastStream) — a `BaseMiddleware` whose `consume_scope` builds the child, stashes it in the framework context for the duration of the call, @@ -399,11 +400,13 @@ Each official integration is its own repository and PyPI package, mirroring the genuinely unreachable boot line (`if __name__ == "__main__"` / server-run). Link it from the README with a `Usage example: [examples/](./examples)` line directly under `Full guide:`. -- **Mirror `modern-di`'s** `CLAUDE.md`, `Justfile`, and `architecture/` truth - home. 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 current `ruff` reformats. +- **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 + 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 + current `ruff` reformats. - **Docs.** Add a `docs/integrations/.md` usage page **in the `modern-di` repo** and a nav entry for it in `mkdocs.yml` (under the matching family group: Web / Tasks & events / Bots / RPC / CLI / Testing). Follow the @@ -467,6 +470,6 @@ Each official integration is its own repository and PyPI package, mirroring the - [ ] `examples/app.py` (+ smoke test asserting real injected output, 100% coverage, no `omit`) and a README `Usage example: [examples/](./examples)` line. -- [ ] `CLAUDE.md`, `Justfile`, `architecture/` mirrored; +- [ ] `CLAUDE.md` and `Justfile` mirrored; invariants pinned by named tests; [planning-convention](https://github.com/lesnik512/planning-convention) followed. diff --git a/docs/introduction/design-decisions.md b/docs/introduction/design-decisions.md index ac95aaa..d8bc1e1 100644 --- a/docs/introduction/design-decisions.md +++ b/docs/introduction/design-decisions.md @@ -25,6 +25,14 @@ Cached `Factory` providers use a per-container reentrant lock (`threading.RLock` per-container dict with no ordering, queueing, or merge; concurrent writes to the same key keep whichever landed last. Do them during setup, or per-request on a request-local child container — never from competing threads. +- **Free-threaded CPython (PEP 703) is supported at `2 - Beta`.** Production-ready + and tested under real multithreading on the `3.14t` build. It is Beta rather than + Stable for one specific reason: modern-di relies on object-publication ordering — + that a reader observing a stored reference sees fully-initialized fields — and + CPython publishes no memory model, so that is implementation behaviour rather than + a spec guarantee. Throughput also does not scale across cores; per-op latency is + competitive, but atomic reference counting of the objects every resolve shares + tracks the GIL. ## 3. No global state diff --git a/docs/introduction/performance.md b/docs/introduction/performance.md index 4963aad..cb30c88 100644 --- a/docs/introduction/performance.md +++ b/docs/introduction/performance.md @@ -215,7 +215,7 @@ would have cost it. **Thread-safety configuration differs, at each framework's default.** dishka's `make_container` defaults to `lock_factory=`, so every `get()` behind its C1–C3 cells acquires a lock; modern-di's cached read is lock-free by design (see -[`architecture/concurrency.md`](https://github.com/modern-python/modern-di/blob/main/architecture/concurrency.md)). +[Design decisions](design-decisions.md#the-thread-safety-boundary)). Both run at their defaults, which is the comparison a user gets out of the box — a dishka user targeting single-threaded work can pass `lock_factory=None`, and that would move dishka's C1–C3 cells. The axis is disclosed rather than normalized away. From d3a6ba35215564ca20edc7c9aa4b762d9d9e3f28 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 19:38:16 +0300 Subject: [PATCH 12/18] docs(planning): four homes and the admission check replace the truth home --- .github/PULL_REQUEST_TEMPLATE.md | 11 +++-- CLAUDE.md | 56 +++++++++++++---------- Justfile | 2 +- planning/README.md | 78 +++++++++++++++++++++++--------- 4 files changed, 96 insertions(+), 51 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index cb3a381..f4a1d1a 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -32,14 +32,17 @@ effect. State the numbers, not "benchmarked". ### Before merging -- [ ] **Behaviour changed?** Hand-edit the matching `architecture/.md` - **in this PR**, so the promotion is reviewed with the code. That edit is - what keeps `architecture/` true. +- [ ] **Behaviour changed?** If a wrong change here could pass silently, pin it with + a test whose name is the claim and whose docstring opens `INVARIANT:` and says + what breaks it. Do **not** write prose about mechanism — there is no page for + it. See [`planning/README.md`](../planning/README.md#where-a-fact-goes). +- [ ] **Adding a fact anywhere?** Run the admission check: derivable from + `modern_di/` → don't write it; enforceable → a test; deliberately not + guaranteed → `planning/decisions/`; a user needs it → `docs/`. - [ ] **Rejected an alternative** with reasoning that would otherwise be re-litigated? File it in [`planning/decisions/`](../planning/decisions/) 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. -- [ ] **New or sharpened domain term?** Update `architecture/glossary.md`. - [ ] `just lint-ci` and `just test-ci` pass. diff --git a/CLAUDE.md b/CLAUDE.md index 76d50b8..a7a0089 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,24 +20,12 @@ or read it for every recipe and its intent. The non-obvious essentials: ## Architecture -> Quick orientation only. The authoritative, code-current account of each capability lives in [`architecture/`](architecture/) — one file per capability. **When a change alters a capability's behavior, update the matching `architecture/.md` in the same PR** — that promotion is what keeps `architecture/` true; code that changes without it silently rots the truth home. - - **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. -Where the detail lives — read the matching capability file before changing behavior: - -| File | Covers | -|---|---| -| [architecture/scopes.md](architecture/scopes.md) | `Scope` hierarchy + the resolution rule | -| [architecture/containers.md](architecture/containers.md) | `Container`, registries, child containers, lifecycle/finalizers | -| [architecture/providers.md](architecture/providers.md) | `Group`, `Factory`/caching, `ContextProvider`, `Alias` | -| [architecture/resolution.md](architecture/resolution.md) | how `resolve()` wires deps from type hints | -| [architecture/validation.md](architecture/validation.md) | `validate()` cycle + scope checks | -| [architecture/testing-and-overrides.md](architecture/testing-and-overrides.md) | overrides + the `modern-di-pytest` integration | -| [architecture/integration-kit.md](architecture/integration-kit.md) | framework-agnostic primitives for building an integration adapter | -| [architecture/concurrency.md](architecture/concurrency.md) | thread-safety + free-threaded (PEP 703) support, at Beta | -| [architecture/performance.md](architecture/performance.md) | why the warm resolve path is shaped as it is: the per-node frame budget, inlined memo hits, how to measure | +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 +[`planning/README.md`](planning/README.md#where-a-fact-goes). ### Key files @@ -45,7 +33,7 @@ Every module under `modern_di/` except the package `__init__.py` re-exports. If you add a module, add it here. - `modern_di/container.py` — Container class, the main entry point -- `modern_di/resolver_compiler.py` — the **single resolve path**: one flat closure compiled per provider, memoized on the registry. Each resolver front-guards its own override, navigates its scope once, and inlines the kwargs build and creator call to hold the per-node frame budget at 1 — the rationale lives in [architecture/performance.md](architecture/performance.md), and the budget is enforced by a test, so **do not extract a helper from these closures**. A new provider type must add a branch here or `compile_resolver` raises +- `modern_di/resolver_compiler.py` — the **single resolve path**: one flat closure compiled per provider, memoized on the registry. Each resolver front-guards its own override, navigates its scope once, and inlines the kwargs build and creator call to hold the per-node frame budget at 1 — the rationale lives in `test_resolve_costs_exactly_one_resolver_frame_per_node`, so **do not extract a helper from these closures**. A new provider type must add a branch here or `compile_resolver` raises - `modern_di/wiring.py` — `WiringPlan`: partitions a creator's parsed parameters into provider / static / context buckets plus `unwireable`. A pure function of its inputs (no cache, scope, or live context), so it runs outside the container lock and is exercisable without a Container - `modern_di/providers/factory.py` — Factory and CacheSettings (singleton pattern via caching + optional finalizer) - `modern_di/providers/context_provider.py` — ContextProvider for runtime-injected values @@ -55,7 +43,7 @@ you add a module, add it here. - `modern_di/types.py` — the `UNSET` sentinel (`UnsetType`) that separates "not passed" from "explicitly `None`", plus the shared TypeVars. Load-bearing on the resolve path: it is the miss marker for both the override lookup and the cache slot - `modern_di/types_parser.py` — Signature introspection engine (parses type hints for DI wiring) - `modern_di/dependency_graph.py` — the one static graph walk (`DependencyGraph.walk`), consumed by `validate()` and the runtime cycle guard. Explicit-stack, never recursive: a caller runs it inside a `RecursionError` handler near CPython's stack limit. It walks `WiringPlan.edges`, so what `validate()` traverses is exactly what `resolve()` follows -- `modern_di/registries/` — the four registries: `providers_registry` (type → provider, plus the shared plan/resolver memos) and `overrides_registry` are shared tree-wide; `cache_registry` and `context_registry` are per-container. See [architecture/containers.md](architecture/containers.md) +- `modern_di/registries/` — the four registries: `providers_registry` (type → provider, plus the shared plan/resolver memos) and `overrides_registry` are shared tree-wide; `cache_registry` and `context_registry` are per-container - `modern_di/integrations.py` — the integration kit: Layer 1 (`bind`, `classify_connection`) derives a child container's scope/context from `ContextProvider`s; Layer 2 (`Marker`, `from_di`, `parse_markers`, `resolve_markers`) is the `Annotated`-marker injector. Neither layer wraps `build_child_container` - `modern_di/suggester.py` — what a suggestion *is* (the `Suggestion` record) and how to *find* one: `suggest(requested_type, providers)` owns the policy (hierarchy hints, typo matching, cap, ordering); `close_matches` is the shared difflib primitive (also used by `UnknownFactoryKwargError`). Carries no formatting - `modern_di/scope.py` — Scope enum @@ -69,7 +57,7 @@ you add a module, add it here. - Overrides: `container.override(provider, mock_obj)` / `container.reset_override(provider)` - Scope chains: `app_container.build_child_container(scope=Scope.REQUEST)` - `asyncio_mode = "auto"` — async test functions work without extra markers -- The **`modern-di-pytest`** integration (a sibling repo/package, not a dependency here) → [architecture/testing-and-overrides.md](architecture/testing-and-overrides.md) +- The **`modern-di-pytest`** integration (a sibling repo/package, not a dependency here) ## Workflow @@ -82,11 +70,12 @@ 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 scheduled** goes to `planning/deferred/` (self-contained, with a revisit -trigger). `architecture/` (repo root) stays the living **truth home** — a -behaviour change hand-edits the matching capability page in the same PR. -See [`planning/README.md`](planning/README.md) for the full convention; it is a -documented local deviation from `planning-convention` 2.2.0. The `## Architecture` -section above is quick orientation; `architecture/` holds the authoritative account. +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. - **Cutting a release (maintainers)** is tag-driven via [`.github/workflows/release.yml`](.github/workflows/release.yml): write the @@ -114,4 +103,23 @@ section above is quick orientation; `architecture/` holds the authoritative acco - 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 `architecture/`. + cross-file rationale lives in an `INVARIANT:` test docstring or + `planning/decisions/`. + +## Vocabulary + +A term is listed only when there is a synonym to reject. + +- **Container** — owns the registries and resolves within a scope. *Avoid:* injector. +- **Provider** — a declaration of *how to produce* a dependency; the recipe, not the value. *Avoid:* service, + dependency. +- **Scope** — one band in the container hierarchy. *Avoid:* lifetime, layer. +- **Group** — a non-instantiable namespace class declaring providers. *Avoid:* module. +- **Resolution** — producing a value from its provider. *Avoid:* injection (reserve that for passing a resolved + value into a handler). +- **Override** — a test-time replacement of a resolved value. *Avoid:* mock, patch (an override supplies a + concrete value; it does not wrap or spy). +- **Bound type** — the type a provider is registered under. *Avoid:* registered type, return type. +- **Wiring plan** — the partition of a creator's parameters by how each is satisfied. *Avoid:* compiled kwargs. +- **Finalizer** — a cleanup callback on a cached provider, run LIFO at close. *Avoid:* teardown, destructor. +- **Connection** — the framework object a unit of work carries. *Avoid:* request (too HTTP-specific). diff --git a/Justfile b/Justfile index f126a77..47d9f46 100644 --- a/Justfile +++ b/Justfile @@ -22,7 +22,7 @@ lint-ci: uv run python planning/links.py # Check every relative Markdown link and heading anchor. `mkdocs --strict` only sees -# docs/; architecture/ and planning/ live outside docs_dir and are read on GitHub. +# docs/; planning/ lives outside docs_dir and is read on GitHub. check-links: uv run python planning/links.py diff --git a/planning/README.md b/planning/README.md index 7c65d54..83d70ca 100644 --- a/planning/README.md +++ b/planning/README.md @@ -1,9 +1,10 @@ # Planning The standing record for `modern-di`. The living truth about *what the system -does now* lives in [`architecture/`](../architecture/) at the repo root; this -directory holds what `architecture/` cannot: the decisions taken (especially the -options rejected) and the work deliberately not scheduled. +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) @@ -24,36 +25,69 @@ to write and nothing to commit: the PR body *is* the spec, reviewed inline with the diff. A trivial PR (typo, dep bump, formatter, mechanical rename) may delete the template and ship a conventional-commit title. -**2. Promote in the same PR.** If the change alters a capability's behavior, -hand-edit the matching `architecture/.md` in the same diff, so the -edit is reviewed with the code. That promotion is what keeps `architecture/` -true. - -**3. File what outlives the PR:** +**2. File what outlives the PR:** - an alternative you **rejected** with reasoning → `decisions/` - work that is real but **not scheduled** → `deferred/` -- a term worth pinning down → `architecture/glossary.md` -**4. Run `just check-planning` and `just check-links` before pushing.** +**3. Run `just check-planning` and `just check-links` before pushing.** + +## Where a fact goes + +Four homes, one owner each: + +| Home | Holds | +|---|---| +| `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, or a **negative contract** — something deliberately unspecified | +| `docs/` | anything a user needs | + +Before writing a line anywhere: + +> Can an agent get this by reading `modern_di/`? → **don't write it.** +> Would a wrong change here fail a test? → it belongs **in the test**, not in prose. +> Is it something we deliberately do *not* guarantee? → **`decisions/`**. +> Does a user need it? → **`docs/`**. +> Otherwise it does not get written. + +**Prose about mechanism has no home. There is no file to add a paragraph to.** + +This is deliberate, and it is the second lesson rather than the first. A capability +directory (`architecture/`) was kept for four months and cut to invariants twice — +`b2404c4` (#282, 2026-07-07, −433 lines) and `047b6ea` (#395, 2026-07-29, −292 +lines) — and regrew both times. Promotion discipline was not the problem: 72% of +commits touching `modern_di/` also touched it. Every PR added a paragraph that felt +load-bearing and none removed one, so the pages ratcheted toward restating code, and +restatement is what goes stale. The absence of the directory is the mechanism. See +[`decisions/2026-08-11-drop-architecture-directory.md`](decisions/2026-08-11-drop-architecture-directory.md). + +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 +is where an anti-refactor warning lives — design rationale, not a report of what this +one test happens to catch. It does not have to describe a regression that *this* +test alone would fail on; a sibling test may be the one that actually trips. The unit +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 cited +from a `modern_di/` comment exists. ## What lives where -A shipped change leaves three traces, none of them a file in this directory: the -diff, the updated capability page in [`architecture/`](../architecture/), and the -PR body. Between them they answer *what changed*, *what is true now*, and *why*. +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 three cannot: +`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 does not belong in - `architecture/` (it is not current behaviour). Without a home it gets - re-proposed. + 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 `architecture/`, the diff, or the PR body, it goes there -instead. This directory is the residue, and it should stay small. +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. ## Artifacts @@ -82,8 +116,8 @@ 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 `architecture/` and the - release notes. +- **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 From e0718e94db62a8284fb56ee91a1ad5e481ff4cc8 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 19:49:16 +0300 Subject: [PATCH 13/18] docs: drop architecture/; invariants are tests, contracts are decisions --- architecture/README.md | 51 ---- architecture/concurrency.md | 108 --------- architecture/containers.md | 198 ---------------- architecture/glossary.md | 89 ------- architecture/integration-kit.md | 76 ------ architecture/performance.md | 217 ------------------ architecture/providers.md | 199 ---------------- architecture/resolution.md | 175 -------------- architecture/scopes.md | 92 -------- architecture/testing-and-overrides.md | 81 ------- architecture/validation.md | 184 --------------- benchmarks/README.md | 2 +- .../2026-07-13-integration-kit-shape.md | 5 +- ...26-07-15-fold-context-registry-declined.md | 2 +- ...-07-18-warm-singleton-memo-swap-dropped.md | 2 +- .../2026-07-19-child-lazy-alloc-declined.md | 3 +- .../2026-07-26-explicit-only-validation.md | 5 +- ...07-30-debug-resolution-tracing-declined.md | 2 +- ...ontextprovider-resolver-inline-declined.md | 4 +- .../2026-08-03-resolve-provider-not-a-seam.md | 4 +- .../2026-07-19-free-threaded-throughput.md | 4 +- .../2026-07-29-upstream-lean-convention.md | 32 ++- planning/releases/2.28.0.md | 2 +- planning/releases/2.30.0.md | 4 +- 24 files changed, 49 insertions(+), 1492 deletions(-) delete mode 100644 architecture/README.md delete mode 100644 architecture/concurrency.md delete mode 100644 architecture/containers.md delete mode 100644 architecture/glossary.md delete mode 100644 architecture/integration-kit.md delete mode 100644 architecture/performance.md delete mode 100644 architecture/providers.md delete mode 100644 architecture/resolution.md delete mode 100644 architecture/scopes.md delete mode 100644 architecture/testing-and-overrides.md delete mode 100644 architecture/validation.md diff --git a/architecture/README.md b/architecture/README.md deleted file mode 100644 index b9de489..0000000 --- a/architecture/README.md +++ /dev/null @@ -1,51 +0,0 @@ -# Architecture - -The living truth about what `modern-di` does **now** — one file per capability, -updated by hand whenever a change ships. This directory is the present; the *why* -of a specific change is its PR body, and decisions deliberately taken — including -options rejected — live in [`../planning/decisions/`](../planning/decisions/). - -These files carry **no frontmatter** — they are prose, dated by git. - -**The boundary against `docs/`.** `architecture/` answers *how is it built, and -why that way*. [`docs/`](../docs/) answers *how do I use it*. A fact a user needs -belongs in `docs/`; a fact only a maintainer needs belongs here. Where both must -state the same rule, each states it at its own altitude — that restatement is the -point, not duplication to be removed. A runnable block a user would copy is usage, -not mechanism: it belongs in `docs/`, and a page here keeps only the invariant it -was demonstrating. - -**One owner per concept.** That altitude argument works between `architecture/` -and `docs/` because they have different audiences. Two capability pages have the -*same* audience, so restatement between them is just two places to update. Each -concept is owned by exactly one page; every other page gets a one-line -cross-link — see [containers.md](containers.md#validate) pointing at -[validation.md](validation.md) for the shape. - -## Capabilities - -- [scopes.md](scopes.md) — the `Scope` hierarchy and the resolution rule. -- [containers.md](containers.md) — the `Container`, its registries, child - containers, and lifecycle. -- [providers.md](providers.md) — `Group`, `Factory`/caching, `ContextProvider`, - `Alias`. -- [resolution.md](resolution.md) — how `resolve()` wires dependencies from type - hints. -- [validation.md](validation.md) — `validate()` cycle and scope checks. -- [testing-and-overrides.md](testing-and-overrides.md) — overrides and the - `modern-di-pytest` integration. -- [concurrency.md](concurrency.md) — thread-safety and free-threaded (PEP 703) - support, at Beta. -- [performance.md](performance.md) — why the warm resolve path is shaped as it - is: the per-node frame budget, the inlined memo hits, and how to measure a - change without misreading the guard tier. -- [glossary.md](glossary.md) — the project's ubiquitous language. -- [integration-kit.md](integration-kit.md) — framework-agnostic primitives for - building a framework integration adapter. - -## Promotion rule - -Shipping a change hand-edits the affected capability file(s) here to match the -new reality, **in the same PR as the code** — reviewed with the diff, never -applied as a separate post-merge step. That hand-edit is what keeps this -directory true. diff --git a/architecture/concurrency.md b/architecture/concurrency.md deleted file mode 100644 index d2395c4..0000000 --- a/architecture/concurrency.md +++ /dev/null @@ -1,108 +0,0 @@ -# Concurrency and free-threaded (PEP 703) safety - -modern-di is safe to resolve from multiple threads, and supported on free-threaded -CPython (PEP 703, the `3.14t` build) at level **`2 - Beta`**: production-ready -and tested under real multithreading, with the caveats below documented. This -page is the standing contract. - -## The lifecycle - -A container has three phases, and thread-safety is defined per phase — build → resolve → dispose shape: - -1. **Configure — single-threaded (startup).** Registering providers - (`add_providers`, group construction) mutates the registry under its own lock, - but the resolve path reads the registry without taking that lock; `override` / - `reset_override` and `set_context` mutate shared state with no lock at all. - Either way, racing these against live `resolve()` is unsafe — do them on one - thread before concurrent resolution begins. -2. **Resolve — concurrent (the hot phase).** `resolve` / `resolve_provider` / - `resolve_dependency` and `build_child_container` are safe to call from many - threads at once. Singleton creation is locked and double-checked (see below), - so a cached value is built exactly once and shared. -3. **Close — single-threaded (shutdown).** `close_sync` / `close_async` run - finalizers and reset caches/overrides; `open` reopens a closed container so it - can resolve and build children again. Close (or reopen) a container at a - single-threaded edge, after all concurrent resolution has finished — - **closing or reopening a container while other threads still resolve from it - is not supported.** - -Reuse-after-close is a race the concurrent resolve phase does handle, distinct -from the unsupported race above: once a container has settled into the closed -state at a single-threaded edge, nothing prevents several threads from then -independently calling `resolve` on that (now-closed) container at once — each -unaware the others are doing the same. A container is open from construction -(see [containers.md](containers.md#optional-open-lifecycle)), so this is the -only path back to `closed = True` in the first place. `resolve` and -`resolve_provider` each call `_prepare()` whenever `self.closed` is `True`; `_prepare()` warns and sets -`closed = False`, unlocked. The reopen needs no lock because it is idempotent — -N threads racing a closed container all write the same `False`, and they go on -to share one singleton via the cache lock below. What is *not* serialized is the -warning: each racing thread may emit its own `ContainerClosedWarning`. The -contract is **at least one** warning per reuse-after-close, not exactly one. -Under the default warning filters Python's own per-location registry collapses -the duplicates anyway; `simplefilter("always")` reveals them. `open()` is the -same unlocked idempotent write, minus the warning. - -## The model - -- **Singleton creation is the only locked path.** A cached `Factory` builds its - value under the resolving container's `threading.RLock`, double-checked: the - dependency graph resolves *outside* the lock, then creation and the cache store - run *inside* it behind a second cache-populated check, so at most one caller ever - runs the creator (`CacheItem.get_or_create`). Concurrent first-resolvers of the - same singleton share **one** `CacheItem` because `CacheRegistry.fetch_cache_item` - publishes it with `dict.setdefault` — a single atomic operation. Containers built - with `use_lock=False` opt out of the lock and are single-thread-only. -- **Registry memoization is lock-free and idempotent.** The compiled resolver, the - wiring plan, and their registry caches (`_resolvers`, `_plans`) are pure functions - of `(provider, registry contents)`, cleared on mutation. Two threads racing to build - the same entry produce identical objects; the worst - case is one duplicated build, never a wrong result. Clearing on mutation is sound because - mutation is a single-threaded configure-phase operation (see [The lifecycle](#the-lifecycle) - above). **Publication is generation-checked**, which restores the rebuild-stale safety - net a plain version stamp used to provide: both `resolver_for` and `plan_for` read - `_generation` before building, build outside the lock, and then publish under it *only if* - the generation is unchanged. Without that check a build begun before an `_invalidate()` - would store its result after the clear, stranding an entry compiled against a registry - that no longer exists — permanently, since the invalidation meant to drop it has already - happened. A build that loses the race is returned to its caller and simply not memoized. The cycle-guard `_building` - set is **thread-local**: it tracks which providers are being compiled on *this* - call stack, so a genuine same-thread `A -> B -> A` cycle is still caught by the - back-edge thunk, while a concurrent first-resolve of the same provider on another - thread simply compiles it independently (an idempotent duplicate) rather than - being misread as a cycle. (A shared `_building` set was a real bug fixed in this - change — it recursed to `RecursionError` on acyclic graphs under concurrent - first-resolution.) Registry *mutation* (`register` / `add_providers` / removal) - is guarded by the registry's own lock. - -## Why this is sound without the GIL - -Free-threaded CPython makes single built-in-container operations (`dict.setdefault`, -`dict[k] = v`, `dict.get`, `list.append`) internally atomic — one such operation -cannot corrupt the structure. modern-di never relies on a *compound* check-then-act -over shared state being atomic: every such sequence above is either idempotent -(rebuild-if-stale) or already under the container lock. The one reliance CPython -does not *formally* guarantee is object-publication ordering — that a reader -observing a stored reference sees the object's fully-initialized fields — because -CPython publishes no memory model. In the current implementation, publication -through a container's internal critical section provides that ordering; that gap -between "implementation behavior" and "spec guarantee" is why the claim is **Beta**, -not **Stable**. - -## Caveats - -- **Configure and close at single-threaded edges** (see [The lifecycle](#the-lifecycle)). - `override` / `reset_override` and `set_context` mutate shared state without a - lock; racing them against live `resolve()` is inherently unordered (it always - was, GIL or not). `close` / `open` are the same: tear a container down only - after concurrent resolution has stopped. -- **Thread-safe, but resolve throughput does not scale across cores.** Measured - (guard benchmarks G14/G15): concurrent resolution is correct and per-op latency - is competitive, but adding threads does not raise throughput on a free-threaded - build — it tracks the GIL. The cause is CPython's atomic reference counting of - the objects every resolve shares (the returned singleton value, the provider - objects, the compiled-resolver closures and their captured cells), not the - per-container lock and not anything modern-di can remove without immortalizing - those objects (no public CPython API). It is a CPython-level limitation that its - own expanding deferred reference counting (PEP 703) will lift for free as it - reaches ordinary instances. See the [free-threaded scaling diagnosis](../planning/deferred/2026-07-19-free-threaded-throughput.md). diff --git a/architecture/containers.md b/architecture/containers.md deleted file mode 100644 index d409f7d..0000000 --- a/architecture/containers.md +++ /dev/null @@ -1,198 +0,0 @@ -# Containers - -`Container` is the central entry point for the dependency injection system. Every interaction with -providers — resolution, scoping, overriding — flows through a `Container`. - -## Creating a root container - -Constructor parameters: - -| Parameter | Default | Effect | -|---|---|---| -| `scope` | `Scope.APP` | The scope level this container occupies. Must be an `IntEnum`. | -| `groups` | `None` | One or more `Group` subclasses whose providers are registered into `providers_registry`. | -| `context` | `None` | Mapping of `type → object` pre-populated into `context_registry`. | -| `use_lock` | `True` | Wraps resolution in a `threading.RLock`; set `False` for single-threaded use. | -| `validate` | `None` | Deprecated and ignored; emits `ValidateArgumentWarning`, removed in 4.0. See [docs](../docs/providers/lifecycle.md#the-deprecated-validate-constructor-argument). | - -A root container (no `parent_container`) creates fresh `ProvidersRegistry` and `OverridesRegistry` -instances. It also auto-registers `container_provider` (see [below](#container_provider)) under the -`Container` type. - -A freshly-constructed container starts **open** (`closed = False`) — see -[Optional-open lifecycle](#optional-open-lifecycle). - -## Optional-open lifecycle - -A container is **open** the moment it is constructed (`closed = False`) — there is no required -`open()` step and no first-use preparation. `close_sync()` / `close_async()` run finalizers and set -`closed = True`; that is the only way a container becomes closed. `open()` and `with` / `async with` -stay available — call them to get finalizers on the way out, and to reopen a closed container -deliberately. - -Two states, tracked by the public `closed: bool`: - -| State | `closed` | Implicit use | `open()` / `with` | -|---|---|---|---| -| Open | `False` | proceeds | no-op | -| Closed | `True` | reopens, warns | reopens, silent | - -Reusing a closed container **implicitly** — a `resolve` / `resolve_provider` / `resolve_dependency` -that reaches it without going through `open()` / `with` first, directly or by building a child and -resolving through it — reopens it and emits `ContainerClosedWarning` (a `RuntimeWarning`, so it is -visible outside `__main__`, unlike `DeprecationWarning`); calling `open()` explicitly reopens silently, -since a deliberate reopen is not a diagnostic-worthy event. The warning is per container, not per -resolve: a closed REQUEST child resolving an APP-scoped provider through a closed APP parent reopens -and warns twice, once for each closed container the resolve passes through. See [Lifecycle: close and -reopen](#lifecycle-close-and-reopen) for what close and reopen do to the cache, and [Open and -reopen](#open-and-reopen-context-manager-protocol) for `_prepare()` / `open()` mechanics. - -## Child containers - -`build_child_container` creates a new `Container` whose `parent_container` is the current one. -Rules: - -- The child's scope must be strictly greater (deeper) than the parent's scope; `Container.__init__` is the - guard, so passing a too-shallow scope to `build_child_container` raises `InvalidChildScopeError` from there. - Passing `scope=None` derives the next scope via `scope._next_deeper` — the shallowest *member* deeper than - the parent, **not** `value + 1`, so non-contiguous custom enums (`TENANT=6, JOB=10`) work; if the parent is - already at the deepest member, `MaxScopeReachedError` is raised. See - [scopes.md](scopes.md#the-scope-algebra). -- Building a child does not require the parent be open. `build_child_container` reads the parent's - `_scope_map` and its two shared registries (`providers_registry`, `overrides_registry`); it resolves - nothing and touches no cache, so a closed parent is irrelevant to it — there is no closed-check on - the parent. The returned child itself starts open, same as any freshly-constructed container; see - [Optional-open lifecycle](#optional-open-lifecycle). - - This is safe because validation state lives on the shared `ProvidersRegistry`, not on any one - container, and nothing validates automatically in the first place — `validate()` is the only trigger - (see [validation.md](validation.md)). Building a child off a closed parent therefore skips nothing - that would otherwise have run. - -The child gets its own, independent `_scope_map` dict holding all of its ancestors, enabling -`find_container(scope)` to reach any ancestor scope in O(1). The map never contains the container -itself: a `scope: self` entry would make every container a reference cycle, so no container could -be freed by reference counting and each would wait for a generational GC pass. `find_container` -short-circuits on its own scope before consulting the map, so the self-entry was never read. - -## Registry sharing - -The four registries split into two categories: - -| Registry | Shared across container tree? | Purpose | -|---|---|---| -| `ProvidersRegistry` | Yes — all containers share one instance | Maps `type → AbstractProvider`; populated at root construction time from `groups`, and later via `Container.add_providers`. Also holds the shared `_plans` wiring-plan memo (keyed by `provider_id`, cleared on registry mutation), so a plan is built once tree-wide. | -| `OverridesRegistry` | Yes — all containers share one instance | Maps `provider_id → override object`; used by tests to substitute real instances. | -| `CacheRegistry` | No — each container has its own | Maps `provider_id → CacheItem`; stores resolved singleton instances and their finalizers for this scope level. | -| `ContextRegistry` | No — each container has its own | Maps `type → runtime object`; populated via `context=` at construction or `container.set_context()` after the fact. | - -Because `ProvidersRegistry` and `OverridesRegistry` are shared, registering a group or setting an -override on any container in the tree is immediately visible to all other containers in the same -tree. - -### Integration seam - -`add_providers` (registration) and `resolve_dependency` (provider-or-type -dispatch) are the blessed integration seam — see -[writing-integrations.md](../docs/integrations/writing-integrations.md). -`add_providers` is **root-only**: called on a child, it raises -`ChildContainerRegistrationError` (`modern_di/exceptions.py`), since the -registry it mutates is shared tree-wide. All validation state lives on the -shared `ProvidersRegistry`, not on `Container` — there is no per-container -validated flag (see [validation.md](validation.md#what-validate-checks)). -`add_providers` registers and nothing more: it does not validate, and there -is no rollback on a bad batch — a cycle or inverted scope introduced by the -new providers is only reported the next time something calls -[`validate()`](#validate). The one effect on validation state is indirect: -mutating the registry clears `ProvidersRegistry._validated`, so a later -`validate()` re-walks the now-larger graph rather than trusting a stale clean -result. Because the registry is shared tree-wide, a batch registered through -the root is immediately visible to every container in the tree, so a child's -`resolve` sees a root's `add_providers` call without the child doing anything. -`resolve_dependency` carries no restriction of its own; it is a resolve verb, -callable on any container regardless of validation state. - -## `container_provider` - -A singleton instance of `_ContainerProvider` is registered under the `Container` type in the -`ProvidersRegistry` of every root container. Its `resolve` method returns the container passed to -it, so resolving `Container` from any child yields that child container — not the root; see -[docs/providers/container.md](../docs/providers/container.md) for the user-facing behavior and -examples. `_ContainerProvider` has `scope=Scope.APP` and `bound_type=None` (it is registered -explicitly under `Container` rather than inferred from a type annotation). - -## Lifecycle: close and reopen - -`close_sync()` / `close_async()` are the only transitions to `closed = True`; `_prepare()` and -`open()` are the only ones back, and they differ in whether the reopen warns. - -### Closing - -`close_sync()` and `close_async()` both do two things in order: - -1. **Finalizers** — iterate over the container's `CacheRegistry._creation_order` list in **reverse - (LIFO)** order and call each `CacheItem`'s finalizer if one is configured and the item has not - already been finalized. On `close_sync()`, any item whose finalizer is async raises - `AsyncFinalizerInSyncCloseError`; those items are left in `_creation_order` so a subsequent - `close_async()` can clean them up. - -2. **`closed = True`** — set in a `finally` block, even if finalizers raised. A subsequent - `resolve` / `resolve_provider` (or a nested provider resolving at a closed ancestor scope) self-heals: it - reopens the container via `_prepare()` and emits `ContainerClosedWarning`, rather than raising. - Re-enter the container via `with`/`async with`, or call `container.open()`, for a silent reopen - instead — see [Optional-open lifecycle](#optional-open-lifecycle). - -Additionally, when `close_sync()` or `close_async()` is called on a **root** container (one with -no `parent_container`), all overrides are cleared from the shared `OverridesRegistry` before the -cache is finalized. - -Child containers only finalize their own `CacheRegistry`; the shared `OverridesRegistry` is left -alone. - -### `clear_cache` per `CacheItem` - -After running a finalizer, `CacheItem._clear()` evicts the cached instance (and resets `finalized`) -only if `CacheSettings.clear_cache` is `True` (the default); otherwise the cached value survives -close, ready to be returned again without re-running the creator. - -### Open and reopen (context-manager protocol) - -`_prepare()` — not `open()` — is the primitive the resolve path calls: `resolve_provider` and `resolve` -(and the compiled-resolver dispatch they wrap) call it whenever `self.closed` is `True`, before doing -anything else — `resolve` holds its own copy of that check rather than delegating -([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)). -That caller-side `if closed` check is the only guard: `_prepare()` itself takes no lock and makes -no re-check, warning with `ContainerClosedWarning` and clearing `closed` unconditionally. Concurrent -reuse of one closed container therefore warns **at least once**, not exactly once — see -[concurrency.md](concurrency.md#the-lifecycle). `open()` is a separate, public entry point that clears -`closed` unconditionally, with no closed-check and no warning — a deliberate reopen is not a -diagnostic-worthy event. Neither method runs validation; `open()` -is a plain lifecycle op, symmetric with `close_sync()` / `close_async()`. `Container` implements both -sync (`__enter__` / `__exit__`) and async (`__aenter__` / `__aexit__`) context managers; both call -`open()` on entry and `close_sync()` / `close_async()` on exit. - -Concretely: using the same container object as a context manager a second time reopens it (clears -`closed`), resolves providers fresh if `clear_cache=True` was set on their `CacheSettings` (since -close removed those cached values), and then closes it again on exit. Providers whose -`CacheSettings.clear_cache` is `False` retain their cached instances across reopen cycles. - -`open()` is public — rather than `_prepare()` alone — for callback-style lifecycles that cannot wrap -the container in a `with` block, such as a framework startup hook reopening a long-lived root -container after a shutdown. The FastStream integration is the reference case: -`app.on_startup(container.open)` paired with `app.after_shutdown(container.close_async)`. - -## `validate()` - -See [validation.md](validation.md) for what `container.validate()` checks and how it reports -aggregated errors. It is the only thing that validates — construction, `open()`, and `add_providers` -never do. - -## `set_context()` - -Registers a runtime value directly into the container's `ContextRegistry`. Context values are -resolved **live** on every resolve (see [resolution](resolution.md)), so a value set here is -picked up by subsequent resolves of **non-cached** providers — including factories in deeper-scoped -child containers that read this container's context — with no cache invalidation needed. - -A **cached** provider (`Factory(cache=...)`) is built once and its instance is *not* -rebuilt by a later `set_context`; set the context before its first resolve. diff --git a/architecture/glossary.md b/architecture/glossary.md deleted file mode 100644 index fe0b49a..0000000 --- a/architecture/glossary.md +++ /dev/null @@ -1,89 +0,0 @@ -# Glossary - -The project's ubiquitous language — the domain terms worth pinning down: those -with a synonym to reject, or a meaning subtle enough that code, specs, and -capability pages must agree on it. Not an exhaustive dictionary of every class -name; a term earns a place when there is something to disambiguate, and entries -are authored lazily as that need arises. Living prose, no frontmatter, dated by -git. Each entry says what a term *is* (not what it does) and links to the -capability page that owns the behaviour; an _Avoid_ line names the synonyms to -reject. No implementation detail; this is a glossary, not a spec. - -**Container**: -The central object: it owns the provider registries and resolves dependencies -within a scope. Containers form a parent→child hierarchy, one per scope band. See -[containers.md](containers.md). -_Avoid_: injector - -**Provider**: -A declaration of *how to produce* a dependency — the recipe, not the value. -`Factory`, `Alias`, and `ContextProvider` are the concrete kinds. See -[providers.md](providers.md). -_Avoid_: service, dependency (those name the produced value, not its recipe) - -**Scope**: -One band in the container hierarchy (`APP → SESSION → REQUEST → ACTION → STEP`), -ordered shallow to deep; a provider resolves only from a container at the same or -a deeper band. See [scopes.md](scopes.md). -_Avoid_: lifetime, layer - -**Group**: -A non-instantiable namespace class whose class attributes declare providers. See -[providers.md](providers.md). -_Avoid_: module - -**Resolution**: -The act of producing a dependency's value from its provider, wiring the -provider's own dependencies from their type hints. Sync-only (async resolution -was removed in 2.x). See [resolution.md](resolution.md). -_Avoid_: injection (reserve that for the integration act of passing a resolved -value into a handler) - -**Validation**: -A static check of the provider graph — cycles plus scope ordering — run before -resolution, without calling any creator. See [validation.md](validation.md). - -**Override**: -A test-time replacement of a provider's resolved value with a supplied object, -short-circuiting its creator. See -[testing-and-overrides.md](testing-and-overrides.md). -_Avoid_: mock, patch (an override supplies a concrete value; it does not wrap or spy) - -**Child container**: -A container built at a deeper scope from a parent. It shares the parent's -providers and overrides registries but owns its own cache and context. See -[containers.md](containers.md). - -**Bound type**: -The type a provider is registered under and resolvable by — taken from the -creator's return annotation unless set explicitly. See -[resolution.md](resolution.md). -_Avoid_: registered type, return type - -**Wiring plan**: -The partition of a creator's parameters by how each is satisfied — a provider, a -static value, a context lookup, or unwireable. A pure function of the provider -and the registry's contents. See [resolution.md](resolution.md). -_Avoid_: compiled kwargs - -**Finalizer**: -A cleanup callback bound to a cached provider, run when the container closes -(LIFO); may be sync or async. See [containers.md](containers.md). -_Avoid_: teardown, destructor - -**Connection**: -The framework-specific object a unit of work carries — an HTTP request, a -broker message, a CLI invocation's context. Not every unit of work has one (a -Typer command's underlying callable does not). -_Avoid_: request (too HTTP-specific — a broker message is also a connection) - -**Connection match**: -A child container's derived `scope` and `context`, produced by binding a -connection to one `ContextProvider`. See -[integration-kit.md](integration-kit.md). - -**Integration kit**: -The framework-agnostic primitives (`modern_di.integrations`) an integration -adapter composes to derive connection scope/context and to run the -`Annotated`-marker injector, instead of hand-rolling either. See -[integration-kit.md](integration-kit.md). diff --git a/architecture/integration-kit.md b/architecture/integration-kit.md deleted file mode 100644 index c7d0482..0000000 --- a/architecture/integration-kit.md +++ /dev/null @@ -1,76 +0,0 @@ -# Integration Kit - -Framework-agnostic primitives for building a **modern-di integration** — the -shared skeleton every framework adapter (FastAPI, Starlette, gRPC, ...) needs, -extracted so it has one home instead of thirteen. Lives in -`modern_di/integrations.py`. See -[docs/integrations/writing-integrations.md](../docs/integrations/writing-integrations.md) -for the user-facing spec an integration author follows; this page is the -capability's truth home. - -## Layer 1 — connection derivation - -`ConnectionMatch(scope, context)` pairs a child container's intended scope with -the context dict `build_child_container(context=...)` expects. - -`bind(provider, connection) -> ConnectionMatch` derives both from one -`ContextProvider` and one connection object: `scope=provider.scope`, -`context={provider.context_type: connection}`. Neither `bind` nor -`classify_connection` calls `build_child_container` itself — the caller's own -call stays the single, un-wrapped way to open a child; these functions only -decide what to pass it. - -`classify_connection(connection, providers) -> ConnectionMatch | None` is -isinstance-over-tuple dispatch built on `bind`: the first provider whose -`context_type` `connection` is an instance of wins. Returns `None` — never -raises — on no match, matching every dispatch adapter's existing fallback of -opening an auto-scoped, context-less child. - -Not every adapter's shape fits either primitive. A single-connection-kind -adapter with no context to inject (a CLI command) has nothing to derive and -calls `build_child_container(scope=...)` directly. A multi-provider fan-in that -merges several providers' context into one child at a hardcoded scope (rather -than deriving scope from any one of them) also bypasses both — see -[decisions/2026-07-13-integration-kit-shape.md](../planning/decisions/2026-07-13-integration-kit-shape.md) -for which adapters land where. - -## Layer 2 — the `Annotated` marker injector - -`Marker(dependency)` is what `resolve_dependency` should resolve for one -`Annotated[T, marker]` parameter; `dependency` is an `AbstractProvider` or a -bare type, exactly `resolve_dependency`'s own accepted shape. -`Marker.resolve(container)` resolves it — the shape every native-DI -integration's own per-parameter resolver (`Dependency.__call__`) already had, -now shared instead of duplicated per adapter. - -`from_di(dependency) -> T` is the default `Marker` factory — -`Annotated[T, from_di(dep)]` type-checks as `T` via `typing.cast`. Integrations -with their own per-handler injection seam (FastAPI's `Depends`, Litestar's -`Provide`) define their own factory that wraps a `Marker` instead; the rest -re-export `from_di` verbatim as their public `FromDI`. - -`parse_markers(func) -> dict[str, Marker]` scans `func`'s `Annotated` -parameter hints once, at decoration time, and returns every parameter whose -metadata holds a `Marker` — the first one found per parameter, `return` never -scanned. `resolve_markers(container, markers) -> dict[str, Any]` resolves each -by name. Together these are the `_parse_inject_params`/resolve pair that was -duplicated near-verbatim across every integration without native DI. - -## Double-wrap guard - -`is_injected(func)` / `mark_injected(wrapper)` read and set one shared -attribute flag (`__modern_di_injected__`). An adapter whose auto-inject sweep -can visit the same handler twice (a shared view function registered under two -routes, a handler re-wrapped on plugin re-init) marks it on first wrap and -skips on the next. Adapters with no auto-inject sweep — a single `@inject` per -handler is never applied twice — have no need for either. - -## What stays per-adapter - -Root-container lifecycle (open/close, where it's attached to framework state), -where the per-connection child is stashed and read back, `close_sync` vs -`close_async`, and any handler-signature rewriting (stripping a parameter, -inserting a context object) are irreducibly framework-specific and are not -part of this module. See -[docs/integrations/writing-integrations.md](../docs/integrations/writing-integrations.md) -for the full contract an integration implements around these primitives. diff --git a/architecture/performance.md b/architecture/performance.md deleted file mode 100644 index acc03a8..0000000 --- a/architecture/performance.md +++ /dev/null @@ -1,217 +0,0 @@ -# Hot-path performance design - -Why the resolve path is shaped the way it is. Several places in `modern_di/` look -like duplication a reviewer would want to collapse, or like a method call that -was needlessly hand-inlined. They are deliberate, and this page is why. The rules -below are the contract; the numbers that motivated them are quarantined in -[Measurements](#measurements), because a measurement ages and a rule does not. - -The scope is the **warm resolve path** only — what runs on every `resolve_provider` -once the graph is compiled. Compile-time work (`compile_resolver`, `WiringPlan`) -runs once per provider per registry and is not optimized for; `validate()` and -error rendering are cold by construction. - -## The per-node frame budget - -**Resolving one node in a dependency graph costs exactly one Python frame — the -node's own compiled resolver.** Nothing else. A chain of depth 6 costs 6 resolver -frames plus the 6 creator calls the user asked for. - -This is the rule the rest of the page serves, and it is why -[`resolver_compiler.py`](../modern_di/resolver_compiler.py) contains what reads as -copy-paste. Each compiled closure independently: - -- front-guards its own override (`overrides.has_overrides`, then `fetch_override`), -- navigates to its own scope target, -- reopens a closed target, -- inlines the kwargs (or positional) argument build, -- inlines the creator call and its `TypeError` handling. - -Extracting any of that into a shared helper is correct, tidy, and costs **one -Python frame per resolved node** — a cost that scales with graph depth on the -hottest path in the library. Python has no inlining to give it back. - -**This is enforced, not merely documented.** -`tests/test_resolver_compiler.py::test_resolve_costs_exactly_one_resolver_frame_per_node` -counts Python-level calls (via `sys.setprofile`, so a frame that is pushed and -popped mid-resolve still counts) across two chain depths and asserts the per-node -slope is exactly 2 — one resolver frame, one creator. Extracting the override -guard into a helper moves it to 3 and fails the test by name. - -**The slope is 2 on every supported interpreter, and that took work.** A -resolver's argument build used to be a comprehension — `` on the -positional path, `` on the kwargs path — and before -[PEP 709](https://peps.python.org/pep-0709/) a comprehension is a separate code -object, so below 3.12 it cost a third frame per resolved node. The **arity -ladder** removed that on the path the budget measures: `len(pos)` is fixed at -compile time, so a factory with 0 or 1 provider dependencies compiles to a -closure that names its argument and calls the creator directly — no list, no -`CALL_FUNCTION_EX`, and no comprehension for PEP 709 to inline or not inline. A -comprehension frame survives only on the arity-2+ generic star-call and on the -kwargs path, neither of which the chain test measures. - -**The ladder stops at arity 1 deliberately.** Every rung is a full copy of the -closure — override guard, scope hop, closed-target reopen, both error handlers — -so each rung multiplies the branch set that has to be reached by tests, not just -the binding. Arity 0 and 1 are where the measured win lives (leaves, and chains, -which are arity 1 per node): ~-33% on a single transient resolve and ~-28% on a -depth-6 chain. Rungs beyond 1 duplicate that whole branch set for a gain no -scenario in `benchmarks/` demonstrates. - -The corollary for anyone adding a provider type: put the whole resolver in the -closure. `compile_resolver` raising `TypeError` for an unknown provider type is -deliberate — there is no interpreted fallback to inherit shared behaviour from. - -## Inlined memo hits - -Six lookups are hand-inlined across four call sites, with the method called -only on a miss: - -| Call site | Inlines | Method still owns | -|---|---|---| -| `Container.resolve_provider` | `providers_registry._resolvers.get(pid)` | the cycle guard and memo write, on a miss | -| `Container.resolve` | `providers_registry._providers.get(dependency_type)` and `._resolvers.get(pid)` | `find_provider`'s absence result, and `resolver_for`'s cycle guard and memo write, on a miss | -| `_compile_cached_factory`'s `resolve` | `cache_registry._items.get(pid)` | `setdefault`, which is what makes concurrent first-resolvers share one `CacheItem` | -| `_compile_alias`'s `resolve` | `providers_registry._providers.get(source_type)` and `._resolvers.get(source.provider_id)` | `_find_source`'s error, and `resolver_for`'s cycle guard and memo write, on a miss | - -In each case the method being inlined *opens with exactly that lookup and -returns*, so the inline is not a reimplementation that can drift — it is the -method's own fast path, hoisted past its frame. All four keep calling the real -method on a miss, so the miss-path invariants (cycle detection, single shared -`CacheItem`, the dangling-source error) are untouched. - -`Container.resolve` goes further than a hoisted lookup: it carries a **copy of -`resolve_provider`'s whole body** — the closed check, the memo hit, the -`resolver_for` fallback, the resolver call and the `RecursionError` conversion. -That is the one place in the library where a block of logic is deliberately -duplicated rather than shared, and both copies must be edited together. It is -worth ~-19% on a by-type resolve, and it is licensed by `resolve_provider` not -being an interception seam — an override has seen only top-level calls since the -compiled resolvers landed -([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)). - -The alias case inlines two lookups rather than one, because the hop is two indirections deep: without them an -alias costs four Python frames (`_find_source`, `find_provider`, `resolve_provider`, then the source's -resolver) where every `Factory` dependency costs one. -`tests/test_resolver_compiler.py::test_alias_hop_costs_exactly_one_resolver_frame` holds it at one. - -The warm cached resolve also returns before `CacheItem.get_or_create`, having -already made the same `is UNSET` sentinel check that method opens with. - -### No cell on the warm path - -The cached-factory resolver's cold-miss thunk is built with -`functools.partial(build_cold, target)` and **must never be a lambda closing over -`target`**. A closure promotes `target` to a cell variable for the *whole* -resolver, so `MAKE_CELL` runs in the prologue on **every** call — including the -warm hit that returns two lines later, and the override hit that never reaches -`target` at all. It also turns four `LOAD_FAST` into `LOAD_DEREF`. - -The mechanism is worth stating precisely, because the obvious rationale is wrong: -`functools.partial` is **not** cheaper than a lambda. On CPython 3.14 it costs -~69 ns to construct against the lambda's ~36 ns, and ~105 ns against ~75 ns for -construct-plus-call. It is chosen *despite* that, because it is constructed once -per cold miss while the cell it avoids was charged on every call. The thunk must -therefore stay inside the cell-bearing resolver; hoisting its construction -elsewhere would flip the trade. - -Enforced by -`tests/test_resolver_compiler.py::test_cached_resolver_has_no_cell_on_the_warm_path`, -which asserts `co_cellvars == ()` on the compiled resolver — nothing else in the -suite would catch a revert to a lambda. - -## The positional fast path - -When a creator's entire signature is provider dependencies in declaration order, -the compiled resolver calls `creator(*args)` instead of `creator(**kwargs)`, -skipping the dict build and the keyword-binding cost. - -Eligibility is deliberately narrow — `_can_call_positionally` returns `False` on -any static kwarg, context kwarg, omitted defaulted param, keyword-only param, -kwargs-overlay reordering, or positional-only gap. **When in doubt, exclude**: a -wrong `True` here silently binds arguments to the wrong parameters, which is a -correctness bug, not a slow path. `tests/test_resolver_compiler.py` pins all four -exclusion rules plus the positive case directly. - -## Scope navigation - -A resolver navigates to its target container **once**, and same-scope -dependencies — the common case — skip navigation entirely via an int compare -(`container if container.scope == scope else _navigate(...)`) rather than calling -`find_container`. `Container._scope_map` then makes any genuine cross-scope hop -O(1), and holds ancestors only: a `scope: self` entry would make every container -a reference cycle, so none could be freed by refcounting. - -A `Factory`'s **context kwargs** carry the same guard, folded. Each binding's -`provider_id`, scope, `context_type` and absent-disposition are captured at compile -time, and the compiled closure does the override guard, the scope compare, the -registry read and the disposition inline — so a request value read from the request -container costs no navigation frame and no helper frame. Measured at ~-6% (~42 ns -per context kwarg) on `g9_context`. The value itself is still read live on every -resolve; only the binding is frozen, which is licensed by a `ContextProvider`'s -identity being fixed once in use -([providers.md](providers.md#contextprovider--runtime-injected-values)). - -Both folded loops use `find_container` and **never** the compiler's `_navigate`: -that helper prepends a resolution step, and the enclosing closure prepends the -factory's own, so the caller would appear twice in the breadcrumb. The loops are -separate copies and each can regress alone, so -`test_scope_error_through_a_context_kwarg_carries_one_breadcrumb_step` is -parametrized over both. `ContextProvider.fetch_context_value` keeps the same -int-compare guard for the direct-resolve path, which is now its only caller. - -`Scope._next_deeper` is memoized because it is a constant function of an immutable -enum member, consulted per child on the default `build_child_container()` path. - -## Allocation and lock avoidance - -- **`CacheRegistry.fetch_cache_item` has a get-before-setdefault fast path**, because - a plain `setdefault` eagerly constructs a throwaway `CacheItem` on every hit. - The creation path still goes through `setdefault`, whose atomicity is what makes - concurrent first-resolvers share one item. -- **`Container.__init__` inlines its registry wiring** rather than calling a helper: - it is on the per-request child-build path. -- **The cached-read path takes no lock at all**, and reopening a closed container - takes none either. See [concurrency.md](concurrency.md) — that page owns the - thread-safety contract, this one only notes that the absence of a lock is - intentional rather than an oversight. -- **`WiringPlan`s are memoized on the shared providers registry**, so a - deeper-scope factory builds its plan once tree-wide, not once per child - container. - -## How to measure a change here - -**Do not read a `just bench` median to judge a small change.** Guard-tier -scenarios are auto-calibrated and land on `iterations=1`, so their medians are -quantized to one `time.perf_counter` tick — 41 ns on an Apple M4, which is 23% of -G2's whole value. Two runs of unchanged code report numbers a tick apart, and that -has already caused one false reading of a real change. -[`benchmarks/README.md`](../benchmarks/README.md) documents this in full. - -Measure the specific call directly, with enough iterations to escape the grid. -The A/B/A harness under `.superpowers/spike/` (git-ignored, so it survives -`git checkout -- modern_di/`) does this: it measures base, then candidate, -then base again, and reports the delta against the baseline's own drift. - -For a change that claims to be *free*, there is a stronger check than any -benchmark: fingerprint the compiled resolvers' code objects (`co_code`, -`co_consts`, `co_names`, `co_varnames`, `co_freevars`, recursively) before and -after. Byte-identical output means there is nothing to measure. - -## Measurements - -> Measured 2026-08 on an Apple M4, CPython 3.14. **Absolutes age and are -> machine-specific — the rules above are the contract, these are only the -> evidence that motivated them.** Re-measure before citing. - -| What | Cost | Context | -|---|---|---| -| `creator(**kwargs)` vs `creator(*args)` | **4-6x** | the positional fast path's whole justification | -| `resolver_for` method frame | ~34 ns | of a ~170 ns warm resolve (~20%) | -| `fetch_cache_item` method frame | ~23 ns | of the same ~170 ns warm resolve (~14%) | -| `MAKE_CELL` + 4 `LOAD_DEREF` in the cached resolver prologue | ~18 ns | of a ~162 ns warm cached hit (~11%); no path regressed, including a one-shot cold miss | -| `functools.partial` vs a `lambda`, construct + call | +30 ns | why the partial is a deliberate trade, not a free swap | -| `typing.cast` on the context resolve path | ~19 ns | removed in #404 | -| `isinstance` vs an `is` identity check | ~10 ns | why the `UNSET` check is `is`, with a scoped `ty: ignore` | -| A redundant `open()` lock acquire | ~81 ns | found in the comparative C6 body | -| One `time.perf_counter` tick | 41 ns | the guard tier's resolution floor | diff --git a/architecture/providers.md b/architecture/providers.md deleted file mode 100644 index a24581c..0000000 --- a/architecture/providers.md +++ /dev/null @@ -1,199 +0,0 @@ -# Provider Catalog - -Every provider type in `modern-di`: what each one is, and the mechanism behind it. This page is the -truth home for the catalog, kept true by the promotion rule — a change to a provider's behaviour -edits this page in the same PR. `docs/providers/` covers how to use them. - ---- - -## `Group` — provider namespace - -`Group` is a non-instantiable base class. Attempting to instantiate it (or any subclass) raises -`GroupInstantiationError`. Its sole purpose is to act as a namespace for declaring providers as class-level -attributes: - -```python -from modern_di import providers, Group, Scope - - -class AppProviders(Group): - db_pool = providers.Factory(create_pool, scope=Scope.APP) - user_repo = providers.Factory(UserRepository, scope=Scope.REQUEST) -``` - -`Group.get_named_providers()` walks the MRO and returns a `dict[str, AbstractProvider]` mapping each declared -attribute name to its provider — respecting inheritance order, de-duplicating by first-seen name, and letting a -non-provider override mask the parent provider of the same name. `Group.get_providers()` is derived from it as -`list(cls.get_named_providers().values())`, so the traversal and de-duplication rules live in one place. - -### Group-level default scope - -A `Group` subclass may declare a default scope as a class kwarg — `class RequestGroup(Group, scope=Scope.REQUEST)`. -At class creation, `Group.__init_subclass__` stamps that scope onto every scope-defaulted `Factory`/`ContextProvider` -declared in that class body. Priority: an explicit `scope=` on the provider always wins; otherwise the nearest -group `scope=` kwarg in the MRO applies (a subclass without its own kwarg inherits its ancestor's; a subclass with -its own kwarg overrides it for its own body); otherwise the provider falls back to `Scope.APP`. `Alias` never -participates in stamping — its scope is always derived from its source, never chosen (see below). A -scope-defaulted provider instance shared between two group bodies with different defaults raises -`GroupScopeConflictError` (a `RegistrationError` subclass) at the second group's class-creation time, rather than -letting import order decide; sharing the same instance with the same default scope across groups is a no-op. - -A group body **without** a `scope=` kwarg stamps nothing, so a provider listed only there stays unclaimed and a -later group may still stamp it. That is sound only until the provider is registered: `add_providers`/`register` -set `AbstractProvider._registered`, and a compiled resolver captures `scope` in its closure, so a change after -that point would apply to later-compiled resolvers only. `_stamp_group_scope` therefore raises -`ProviderScopeFrozenError` when a stamp would *change* the scope of a registered provider. A same-scope stamp -still returns early and stays legal, so the shared-instance pattern above is unaffected. See -[docs/providers/scopes.md#group-level-default-scope](../docs/providers/scopes.md#group-level-default-scope) for -the user-facing walkthrough. - ---- - -## `Factory` — the universal provider - -`Factory` is the main building block. Every provider that calls a creator callable (a constructor or factory -function passed as its `creator` argument) is a `Factory`. Each registerable provider's subject argument — -`Factory.creator`, `ContextProvider.context_type`, `Alias.source_type` — is positional-or-keyword and leads its -`__init__`; every other parameter stays keyword-only. - -### Signature - -```python -Factory( - creator: Callable[..., T], - *, - scope: IntEnum = UNSET, # defaults to the group's scope, else Scope.APP - bound_type: type | None = UNSET, - kwargs: dict[str, Any] | None = None, - cache: bool | CacheSettings[T] | None = None, - skip_creator_parsing: bool = False, -) -``` - -### Declaration-time signature parsing - -When `skip_creator_parsing=False` (the default), `Factory.__init__` calls `types_parser.parse_creator(creator)` -immediately. This extracts the return type (used as the provider's `bound_type` unless overridden) and a mapping -of parameter names to `SignatureItem` descriptors. Dependency resolution is therefore type-driven: at resolution -time each parameter is matched against the container's `providers_registry` by its annotated type. - -If `bound_type` is supplied explicitly it overrides the inferred return type (useful when the creator returns a -protocol or base class narrower than the concrete type). See -[docs/providers/factories.md](../docs/providers/factories.md#creator-signature-support-matrix) for the full -per-parameter-shape behavior table (`UnsupportedCreatorParameterError` conditions, escape hatches, union -resolution order). - -### Recursive resolution - -A `Factory`'s parsed parameter map is partitioned into a wiring plan, and its compiled resolver calls -each dependency's resolver by reference — see [resolution.md](resolution.md#compiled-resolvers), which -owns the plan, the memoization, and the breadcrumb. - -### Static kwargs and `skip_creator_parsing` - -See [docs/providers/factories.md](../docs/providers/factories.md#kwargs) for `kwargs=` (static values, -overriding provider-resolved ones for the same key; an unknown key raises `UnknownFactoryKwargError` at -declaration time, unless the creator accepts `**kwargs` or its signature cannot be reflected) and -`skip_creator_parsing=True` (disables signature introspection; a `UserWarning` is -emitted if `bound_type` isn't given explicitly, since the provider then can't be resolved by type). - ---- - -## `CacheSettings` — singleton behavior - -There is no separate `Singleton` class — see [docs/providers/factories.md](../docs/providers/factories.md) -for the user-facing singleton idiom. - -`CacheSettings` is a `dataclass` with the following fields: - -| Field | Type | Default | Purpose | -|---|---|---|---| -| `clear_cache` | `bool` | `True` | Whether the cached instance is evicted when the container closes. | -| `finalizer` | `Callable[[T], None \| Awaitable[None]] \| None` | `None` | Optional teardown called on container close, before cache eviction. | -| `is_async_finalizer` | `bool` | *(computed)* | Not an init parameter — derived by `inspect.iscoroutinefunction(finalizer)` in `__post_init__`. The container uses it to decide whether to `await` the finalizer. | - -Without `cache`, a `Factory`'s resolver calls the creator on every resolution and returns a fresh instance -each time. - ---- - -## `ContextProvider` — runtime-injected values - -`ContextProvider` holds a value that is supplied at container-creation time via the `context` mapping rather than -being constructed by a factory: - -```python -providers.ContextProvider(HttpRequest, scope=Scope.REQUEST) -``` - -At resolution time it looks the value up in the container's `context_registry` for the matching scope. What -happens next depends on **how** the value is fetched — direct resolve vs. as another provider's dependency — -and the two paths are independent: - -- **Direct resolve** (`container.resolve(HttpRequest)` / `container.resolve_provider(the_provider)` → - `ContextProvider.resolve`): if no value was supplied (the key is absent), it raises - `ContextValueNotSetError`, naming the context type and the resolving container's scope — see - [Migration: To 3.x](../docs/migration/to-3.x.md#5-direct-resolve-of-an-unset-contextprovider-raises). - `ContextValueNoneWarning` still exists in `exceptions.py` (retained so existing - `filterwarnings` configs don't break) but nothing raises it any more. -- **As a dependent parameter** of another provider (e.g. a `Factory` constructor argument typed as the context - type): unaffected by the above — no exception is raised on this path. The compiled `Factory` closure does the - whole lookup inline, applying `absent_disposition`'s ruling for an absent value: if the dependent parameter has - a default or is nullable it is silently satisfied; otherwise an `ArgumentResolutionError` is raised. - -**A `ContextProvider`'s identity is fixed once something has resolved through it.** Its `scope` and its -`context_type` are read when a consumer's resolver is compiled and folded into that closure, so changing either -afterwards applies only to resolvers compiled later — silently, since neither attribute touches a registry and -so nothing invalidates the memo. How much of that is *enforced* differs by attribute and by route: - -- `scope` on a **registered** provider is enforced against group stamping by `ProviderScopeFrozenError`. -- `scope` on a provider that is never registered — one passed only as `Factory(creator, kwargs={"x": cp})` — - is **not** enforced: `_registered` stays `False`, so a later `Group` may stamp it without error. -- `context_type` is not enforced on either route. - -So this is a contract, not a mechanism: rebinding `provider.scope` or `provider.context_type` on a provider that -is already in use is unsupported. Construct a second provider instead. - -Either **declaration route** reaches that dependent-parameter path: matched by type from the registry, or -passed explicitly as `Factory(creator, kwargs={"request": the_provider})`. `WiringPlan.build` buckets both -into `context_kwargs` with the parameter's `SignatureItem`, so the two agree — how the provider reaches the -parameter is a declaration detail, not a behavior switch. - -The one exception is a `ContextProvider` passed via `kwargs={...}` for a parameter with **no parsed -`SignatureItem`** — a `**kwargs` creator, or `skip_creator_parsing=True`. There is no default or nullability -to consult, so it stays on the direct-resolve path above and raises `ContextValueNotSetError` when unset. -Treating it as required would raise where the parameter itself has no such constraint declared; treating it -as nullable would silently swallow the unset-context signal — so it keeps direct-resolve semantics instead. - -`ContextProvider` also accepts an optional `bound_type` that overrides the inferred bound type. - ---- - -## `Alias` — re-exporting a type under a different name - -`Alias` delegates resolution to another registered provider, located by the source type: - -```python -providers.Alias(ConcreteDatabase, bound_type=DatabaseProtocol) -``` - -The compiled `Alias` resolver calls its source's compiled resolver directly, after its own override guard — it -holds no cache of its own — wrapping a scope/resolution error with the alias's own step. The source lookup and -the source's resolver-memo read are inlined into the closure (see -[performance.md](performance.md#inlined-memo-hits)); nothing is cached there, so a source registered after the -alias first resolves is picked up on the next one. `Alias` also accepts an optional `bound_type` override. See -[docs/providers/alias.md](../docs/providers/alias.md) for the user-facing rationale and caching implications. - -`Alias` overrides the `redirect_target(container)` node hook to return its source provider (`None` when the -source type is unregistered), marking the alias as a transparent redirect. `DependencyGraph.terminal_scope` -follows that hook down an alias chain to the terminal non-alias provider and returns that provider's scope — -which is what `Container.validate()` and scope-error reporting compare against (see -[validation.md](validation.md#terminal-scope-and-alias-transparency)). An alias's own scope is always `Scope.APP` -internally; its effective scope at resolution time is derived from its source provider's scope. - ---- - -## `container_provider` — the container itself - -A pre-built singleton, auto-registered in every container, that resolves to the `Container` asking for -it. See [containers.md](containers.md#container_provider) for its registration mechanics. diff --git a/architecture/resolution.md b/architecture/resolution.md deleted file mode 100644 index 9c3a82a..0000000 --- a/architecture/resolution.md +++ /dev/null @@ -1,175 +0,0 @@ -# Resolution - -How `modern-di` wires an object graph from type hints. This page states what must stay true of the -resolve path; `modern_di/resolver_compiler.py` and `modern_di/wiring.py` are where it is currently -made true. - -## Entry points - -- `container.resolve(SomeType)` — looks up `SomeType` in `providers_registry` (raising - `ProviderNotRegisteredError`, with closest-match suggestions, if none is registered), then dispatches to the - compiled resolver itself. It holds its own copy of `resolve_provider`'s body rather than delegating, so the - by-type path pays no extra frame; the two copies must be edited together - ([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)). -- `container.resolve_provider(provider)` — resolves by provider reference, skipping the registry lookup. - It reopens the entry container if it was closed (see [containers.md](containers.md#closing)), then calls - `providers_registry.resolver_for(provider)(self)` and wraps any escaped `RecursionError` (the runtime cycle - guard — see [validation.md](validation.md)). - -## Compiled resolvers - -Resolution runs through **one path**: a per-provider **compiled resolver**, a flat -`Callable[[Container], T]` built once by `compile_resolver` and memoized on the registry by -`provider_id`. No interpreted fallback ships. - -The memo is cleared whenever the registry mutates, so the next call rebuilds. A container and every -child share one registry, so a resolver is compiled once for the whole tree. - -### Cycle-safe compilation - -Compilation captures each dependency's resolver **by reference**, so a resolver holds direct callables -to its dependencies rather than recursing through `resolve_provider`. To close back-edges safely, -`resolver_for` marks a provider as *building* before it recurses: a back-edge into a provider whose -resolver is still under construction captures a **thunk** that routes through the runtime -`resolve_provider` instead of a half-built closure. A genuine cycle therefore still overflows the -stack at resolve time and is converted to `CircularDependencyError` by the runtime guard — the same -salvage `validate()` surfaces up front (see [validation.md](validation.md)). - -## Invariants - -These hold regardless of how the compiler is structured. A change that breaks one is a behaviour -change, not a refactor. - -- **One frame per node.** A resolved node costs one Python frame. This is why the override guard is - inlined into every closure rather than checked centrally, why the kwargs build and creator call are - inlined, and why a warm cache hit returns without entering `get_or_create`. Any extraction that adds - a per-node frame trades against this budget. -- **Override wins first.** Every resolver front-guards the override registry before scope navigation, - before the cache, and before the creator. Overriding an otherwise-unwireable factory still - short-circuits to the supplied value. The guard is skipped entirely when no overrides are registered. -- **Navigate once.** A resolver walks to the container at its declared scope exactly once per resolve; - the same-scope case is an int compare, not a lookup. Cross-scope navigation raises - `ScopeNotInitializedError` / `ScopeSkippedError` with this provider's step prepended (see - [scopes.md](scopes.md)). A cross-scope target that is independently closed is reopened there. -- **One graph.** The wiring plan that `resolve()` reads is the same plan `validate()` traverses, so the - validated graph cannot drift from the resolved one. A provider named in a declaration-time - `kwargs={...}` is an edge exactly like a type-matched one. -- **Context is live.** A `ContextProvider` dependency is read on every resolve, so a later `set_context` - is picked up by non-cached factories across scopes. A cached factory is built once and does not - re-read it. -- **Errors are built fresh, never memoized.** `prepend_step` *mutates* the exception as it propagates, - so a stored instance would accumulate breadcrumbs across repeated or nested resolves. -- **Behaviour-sensitive helpers are reused, not reimplemented.** `_resolution_step`, `prepend_step`, - `ContextProvider.resolve`, and `CreatorCallError.from_type_error` have one home each; the compiler - calls them rather than inlining their semantics. The **context-kwarg lookup is the deliberate - exception**: it is folded into each compiled closure, and the helper it replaced was deleted rather - than left alongside, so the semantics still have exactly one home per closure instead of two homes - to keep in step. Its licence is that a registered `ContextProvider`'s scope and `context_type` are - fixed — see [providers.md](providers.md#contextprovider--runtime-injected-values). -- **A new provider type fails loudly.** `compile_resolver` raises `TypeError` for any type it has no - branch for — the single place an unsupported provider is rejected. - -## Wiring plan - -The **wiring plan** partitions a creator's parameters by how each is satisfied. It is consulted when a -`Factory`'s resolver is compiled, not on each resolve, and is memoized per provider on the registry -alongside the resolver. - -`WiringPlan.build` is a **pure function** of `(parsed_kwargs, kwargs, providers_registry, owner)`: it -reads no cache, scope, or live context, so it runs outside the container lock. It is **type matching -only** — it decides *which* provider backs each parameter, never what value that provider holds. Each -parameter lands in one of four places: - -- **`static_kwargs`** — supplied via the provider's declaration-time `kwargs`, or filled with `None` - for an absent nullable dependency. -- **`provider_kwargs`** — matched to a provider by resolved type, or named outright in `kwargs=`. - Self-references are excluded. -- **`context_kwargs`** — matched to a `ContextProvider`; resolved live. -- **`unwireable`** — no provider, no default, not nullable. Recorded as a `(name, SignatureItem)` fact, - not a pre-built exception. Such a factory compiles to an always-raising resolver, since the graph is - broken before any resolve happens. - -Absence is decided once, by the shared `absent_disposition` helper: default → omit, nullable → `None`, -otherwise unwireable. The same helper applies live when a context value turns out to be unset, so the -static and live paths cannot disagree. - -Two matching rules are subtle enough to state outright: - -- A bare parameterized generic (`list[str]`) is **rejected at declaration** — it cannot be resolved by - type. Inside a union, each member degrades to its origin, so `int | list[str]` matches a provider - registered for plain `list`. The element type is not enforced; this asymmetry is intentional and is - not a wiring guarantee. -- A bare `None` annotation is the **degenerate nullable** — a union with zero non-`None` members — and - takes the same branches as `X | None`. In the *return* position (`-> None`) nullability is unread; - only `arg_type` is consulted, to derive `bound_type`. - -## Positional fast path - -When the whole parsed signature is provider dependencies in declaration order — nothing static, -context, default-omitted, keyword-only, or positional-only, and no `kwargs=` overlay extra — the -creator is called positionally, skipping the measured 4–6× `**kwargs` cost. - -The eligibility predicate is deliberately conservative: **when in doubt it excludes**, and the resolver -keeps `creator(**kwargs)`. This is an optimization that must never change binding semantics, so the -negative cases matter more than the positive one — a keyword-only parameter, or a positional-only -parameter dropped from `_parsed_kwargs` by the parser, would shift or reject positional binding, and -both keep the kwargs call. - -Arity 0 and 1 compile to a closure that names its argument and calls the creator directly, instead of -building a list and star-calling it; arity 2+ keeps the star-call. See -[performance.md](performance.md#the-per-node-frame-budget) for why the ladder stops there. - -### Transient teardown order is unspecified - -**The order in which transient dependencies are collected is not part of the contract.** It is only -observable at all for a dependency the creator does *not* retain — one it uses and drops — where the -resolver briefly holds the only reference. modern-di manages no finalizer for such an object: -`CacheSettings(finalizer=)` applies to cached providers, and `close_sync` / `close_async` to what a -container owns. An unretained transient is freed by CPython's ordinary refcounting, and the resolver's -shape decides the order. - -**Nothing observable changed when the arity ladder landed**, and the reason is worth recording: the -ladder caps at arity 1, so it never holds more than one named local and there is no order to alter. -Measured main-vs-ladder on 3.10 and 3.14, arities 1 through 3, the collection order is identical. The -rule is stated in advance of the case that would test it — a rung at arity 2+ would release named -locals with the frame rather than through the star-call's intermediate list, and on CPython below -3.12 that is a different order. Such a rung is a performance change, not a breaking one. - -## Breadcrumb definition sites - -Each step a `Factory` prepends onto a breadcrumb chain may carry an optional definition site — the -creator's declaration point, rendered as a trailing `module:line` anchor — alongside the provider name. -The site is captured lazily, only when a step is actually being built on an error path, and memoized -per provider so a repeated failure never re-inspects the creator. Capture is best-effort: a plain -function or method resolves for free from its code object, a class falls back to source inspection, and -anything without an inspectable source (C callables, `functools.partial`) yields no site rather than -raising. `Alias` steps never carry a definition site, since an alias has no creator of its own. - -## One renderer - -Every glyph in every message lives in `exceptions.py`, and nowhere else. Two private drawers own the -shared formatting: - -- `_render_chain(steps)` draws a `list[ResolutionStep]` as the indented arrow tree with an aligned - scope column — used by both `DependencyPathMixin` (a resolution breadcrumb) and - `CircularDependencyError` (a cycle), which is why the two cannot drift. -- `_render_suggestions(items)` draws the `Did you mean:` block from `list[suggester.Suggestion]` — used - by `ProviderNotRegisteredError`, `ArgumentResolutionError`, and `UnknownFactoryKwargError`. - -What crosses into an error is **facts, never formatting**. `suggester.suggest` returns `Suggestion` -records — `(name, reason, scope)` — so `.suggestions` on a caught exception is data a caller can act on -rather than glyphs it would have to parse back apart. An error derives what it can from what it was -handed: `InvalidChildScopeError` computes `.allowed_scopes` from `parent_scope`, and -`UnknownFactoryKwargError` runs its own `close_matches`. Neither is computed at a raise site. Messages -stay inline f-strings in the class that raises them; only the shared glyph logic is factored out. - -Rendered error text is diagnostic, not a public contract; the structured attributes and the class -hierarchy are. See -[2026-07-14-error-text-is-not-a-contract](../planning/decisions/2026-07-14-error-text-is-not-a-contract.md). - -## Thread safety - -When `use_lock=True` (the default), the container holds a `threading.RLock`, acquired **only** around -the cache-write critical section. Argument building and recursive resolution happen outside it. The -double-checked locking pattern ensures that if two threads race to resolve the same uncached provider, -only one calls the creator and the other uses the freshly stored result. diff --git a/architecture/scopes.md b/architecture/scopes.md deleted file mode 100644 index 286f953..0000000 --- a/architecture/scopes.md +++ /dev/null @@ -1,92 +0,0 @@ -# Scopes - -`Scope` is an `IntEnum` defined in `modern_di/scope.py`. It has five named levels: - -``` -APP = 1 → SESSION = 2 → REQUEST = 3 → ACTION = 4 → STEP = 5 -``` - -Higher integer values represent deeper (more short-lived) scopes. The ordering is significant: the integer value -determines both the scope hierarchy and the validity rules for provider resolution. - -## Resolution rule - -Every provider is bound to a scope at declaration time. The rule is: - -> A provider bound to scope **S** may only be resolved from a container whose scope is **S or deeper** -> (i.e., `container.scope >= provider.scope`). - -Attempting to resolve a provider from a container whose scope is shallower than the provider's scope raises one of -two exceptions, depending on what went wrong (see `exceptions.py` for the exact messages): - -- **`ScopeNotInitializedError`** — raised when the required scope is deeper than the resolving container's scope - (the child container for that scope has not been built yet). -- **`ScopeSkippedError`** — raised when the required scope is shallower than the resolving container's scope but - is not present anywhere in the ancestor chain (i.e., the chain was started at a scope that skipped it). - -Both exceptions inherit from `ContainerError → ModernDIError → RuntimeError`. - -Both also carry a breadcrumb `dependency_path` (via the shared `DependencyPathMixin` — see the scope -walk in [resolution.md](resolution.md)), so a **captive dependency** (a shallower-scoped provider that, -directly or transitively, depends on a deeper-scoped one) reports both the capturing provider's name -and the one that actually failed to resolve, in addition to the two scope names. Raised with an empty -path (e.g. a bare `find_container` call with no provider frame involved), the message falls back to -the base one-liner with no breadcrumb prepended. - -## How the container locates the right-scope container - -Each `Container` maintains a `scope_map: dict[IntEnum, Container]` of its **ancestors**. A root container's -map is empty; each child extends its parent's: `{**parent.scope_map, parent_scope: parent}`. A container is -never in its own map — that self-reference would make every container a reference cycle, leaving each one to -be reclaimed by the garbage collector rather than by reference counting. - -`Container.find_container(scope)` performs the lookup: - -1. If `scope` is this container's own scope, return `self` — checked first, before the map, which is why the - map does not need a self-entry. -2. If `scope` is in `scope_map`, return the corresponding container immediately — no tree walk needed. -3. If `scope` is not in `scope_map` and `scope > self.scope`, raise `ScopeNotInitializedError` (the required - child container has not been built yet). -4. If `scope` is not in `scope_map` and `scope <= self.scope`, raise `ScopeSkippedError` (the scope was - never present in this chain). - -The `scope_map` is built incrementally at construction time, so lookups are O(1). There is no runtime -parent-chain traversal during resolution. - -## Custom scopes - -`Scope` is a convenience enum, but `Container` accepts any `enum.IntEnum` member as its scope. Teams that need -more levels (or different names) can define their own `IntEnum` and use it throughout. The same integer-ordering -rules apply. Passing a non-`IntEnum` value raises `InvalidScopeTypeError`. - -A custom scope is a **standalone** `IntEnum`, never a subclass of `Scope` — Python forbids extending an enum -that has members (`class MyScope(Scope)` raises `TypeError: cannot extend enumeration 'Scope'`). - -## The scope algebra - -The one rule that is not just an integer comparison — *"which members of my enum are deeper than me"* — lives in -`scope.py`, next to the concept it belongs to: - -- `_deeper_members(scope)` — the members of `scope`'s own enum deeper than it, shallowest first. -- `_next_deeper(scope)` — the shallowest of those, or `None` at the deepest member. - -Both take **any** `IntEnum`, which the custom-scope contract forces: since a custom scope cannot subclass -`Scope`, an algebra expressed as methods on `Scope` would silently apply to the five built-in members and to -nothing else. Free functions apply uniformly, so custom scopes get the rule for free. - -`scope.py` imports only `enum` and stays that way by necessity: `exceptions.py` imports `_deeper_members` (to -derive `InvalidChildScopeError.allowed_scopes`), so `_next_deeper` returns `None` at the deepest member rather -than raising `MaxScopeReachedError` itself — raising it here would make `scope` import `exceptions` and the two -would cycle. `Container.build_child_container` owns that raise. - -Both consumers read the rule from this one home: `build_child_container` derives an omitted child scope with -`_next_deeper`, and `InvalidChildScopeError` reports the valid choices with `_deeper_members`. The plain -ordering checks (`scope <= parent.scope`, `scope > self.scope`) stay as they are — `IntEnum` already gives -comparison for free, and wrapping it would add an interface without adding a rule. - -## See also - -- [docs/providers/scopes.md](../docs/providers/scopes.md) for a worked, user-facing walk-through of - resolving across scopes and building child containers. -- [containers.md](containers.md#child-containers) for `build_child_container`'s scope rules - (auto-increment, `MaxScopeReachedError`). diff --git a/architecture/testing-and-overrides.md b/architecture/testing-and-overrides.md deleted file mode 100644 index e7401f4..0000000 --- a/architecture/testing-and-overrides.md +++ /dev/null @@ -1,81 +0,0 @@ -# Testing and Overrides - -This document describes how `modern-di` supports test isolation via overrides and how tests wire up containers. For -the `modern-di-pytest` integration (a sibling package), see the dedicated section below. - -## Overrides - -### The OverridesRegistry - -`OverridesRegistry` is a thin dataclass holding a single `dict[int, Any]` keyed by `provider_id` (the integer -identity of the provider object). It is created once on the root container and **shared** across the entire container -tree — all child containers hold a reference to the same registry instance. - -### container.override and container.reset_override - -```python -container.override(provider: AbstractProvider[T], override_object: T) -> OverrideHandle[T] -container.reset_override(provider: AbstractProvider[T] | None = None) -> None -``` - -`container.override(provider, obj)` writes `obj` into the shared `OverridesRegistry` under the provider's id and -returns an `OverrideHandle[T]`, generic over the override object's type. The override is active from the `override()` -call itself, not from `__enter__` — imperative callers that discard the handle see identical behavior to before. -Used as a context manager, the handle's `__exit__` restores the snapshot taken at the `override()` call — the -provider's prior override if one existed, otherwise no override — unconditionally, even on exception and even if -`reset_override()` ran inside the block. Nested overrides of the same provider unwind in order, each handle -restoring what was active before it. The `OverridesRegistry` itself stays a flat dict; the stack lives in the -handles, not the registry. `container.reset_override(provider)` removes that entry directly. Calling -`reset_override()` with no argument (or `None`) clears **all** overrides from the registry. - -Because the registry is shared, calling either method on a child container has the same effect as calling it on the -root — the override is visible tree-wide. `close_async` and `close_sync` on the root container also call -`reset_override()` automatically, clearing all overrides when the root is torn down. - -### How overrides short-circuit resolution - -`resolve_provider` checks the override registry before delegating to the provider — see -[resolution.md](resolution.md)'s Step 1 for the mechanism. The override value is -returned directly, bypassing the scope check, cache lookup, and creator invocation. This means the override -object does not need to be an instance of the provider's declared type at runtime (Python does not enforce it), -but callers should pass a compatible object for type safety. - -### Scope behaviour under overrides - -An overridden provider is resolved from whichever container `resolve_provider` is called on — the scope of the -original provider is irrelevant because the short-circuit fires before `find_container`. In practice this means a -REQUEST-scoped provider can be overridden and resolved from an APP container without raising -`ScopeNotInitializedError`, which is often what tests want. - -Overrides do not interact with the cache. If a singleton (cached factory) was already resolved before -`container.override(...)` is called, subsequent calls to `resolve_provider` return the override value, not the -cached instance. After `reset_override`, the original cache entry (if any) is still present and is returned again. - -## Testing patterns - -Declare providers as `Group` class attributes and pass the group to `Container` (see -[containers.md](containers.md#creating-a-root-container)). Resolve by provider reference -(`resolve_provider`, most precise) or by type (`resolve`) — both go through the override check. -Test deeper scopes via `build_child_container`; each child gets its own cold `cache_registry` (see -[Registry sharing](containers.md#registry-sharing)), so a request-scoped provider resolved in one -child never leaks a cached instance into another. Inject overrides with -`container.override(provider, obj)` — visible tree-wide immediately, since `OverridesRegistry` is -shared (see above) — and reset with `reset_override()`, or rely on the root container's -`close_sync`/`close_async` to clear all overrides automatically at teardown. - -## modern-di-pytest integration - -`modern-di-pytest` is a **separate package** in a sibling repository. `modern-di` does not depend on it. - -The package exposes two callables for turning DI providers into pytest fixtures: - -**`modern_di_fixture(type_or_provider)`** — creates a single pytest fixture that resolves the given type or -provider from a container fixture already present in the test session. - -**`expose(*groups)`** — bulk-generates one pytest fixture per provider across one or more `Group` subclasses. -Duplicate attribute names across the supplied groups raise `ValueError`. The generated fixtures are named after the -attribute and resolve the corresponding provider automatically. - -Both callables are meant to be used at module level (or in a `conftest.py`) to declare fixtures. At test time, -requesting a fixture by name resolves the provider through the normal `resolve_provider` path, which means overrides -applied to the container before the fixture is invoked are honoured. diff --git a/architecture/validation.md b/architecture/validation.md deleted file mode 100644 index e5e6c5b..0000000 --- a/architecture/validation.md +++ /dev/null @@ -1,184 +0,0 @@ -# Container Validation - -`Container.validate()` audits the static provider graph for wiring errors before any dependency is resolved. It -is the authoritative catch-all for three classes of bug: **circular dependencies**, **inverted scope -dependencies**, and **missing required dependencies**. - -## When validation runs - -`container.validate()` is the only thing that walks the graph. Nothing validates at construction, at -`open()`, at [`add_providers`](containers.md#integration-seam), or at `resolve()` — a container is fully -usable, and stays usable, without ever calling `validate()`. That the trigger is explicit is a -deliberate choice, not an omission: an implicit-validation design was built and discarded — see -[`explicit-only-validation`](../planning/decisions/2026-07-26-explicit-only-validation.md). Call it explicitly, whenever you want the -whole graph checked at once: - -```python -container = Container(scope=Scope.APP, groups=[MyGroup]) -container.validate() # walks now; raises ValidationFailedError if any issue is found -``` - -`Container(validate=...)` exists only for backward compatibility: passing `True` or `False` is ignored -and emits `exceptions.ValidateArgumentWarning` (a `DeprecationWarning`) — see the [constructor -table](containers.md#creating-a-root-container). The argument is removed in 4.0; there is no way to make -construction validate. - -**The `_validated` flag memoizes, it does not gate.** `ProvidersRegistry` carries a `_validated: bool` -(`is_validated()` / `mark_validated()`) that records only whether the *last* walk of the current -registry contents found no errors — it plays no role in deciding whether to validate, since nothing does -that automatically. `validate()` checks it first and returns immediately if still `True`, so a repeat -`validate()` after a clean walk is free. `ProvidersRegistry` has only two mutators — `register` and -`add_providers` — and both clear it back to `False`, so the next `validate()` re-walks. The same flag arms the runtime -`RecursionError`-to-`CircularDependencyError` guard (see the blockquote below): once a walk has -confirmed the graph acyclic, an escaped `RecursionError` is known to be genuine self-recursion and -re-raises untouched, with no re-walk. Before any `validate()` call, `_validated` is `False` from -construction — the guard only short-circuits after someone has actually validated. - -## What validate() checks - -`validate()` is a **fold** over one depth-first walk of the provider graph: -`DependencyGraph().walk(providers_registry, self)` (in `modern_di/dependency_graph.py`) runs a single -iterative, explicit-stack traversal and yields an event stream — `NodeEntered`, `Edge`, `Cycle`, -`DependenciesError` — while `validate()` applies one policy per event kind (`NodeEntered` → -`iter_validation_issues`; `Edge` → the scope-ordering check; `Cycle` → `CircularDependencyError`; -`DependenciesError` → collect the raised exception). The walk emits *structure*; `validate()` supplies -*policy*. That same walker also backs the runtime cycle guard (see the blockquote below), so the two -share one traversal — a stance reversed, on the extraction axis only, from the "deliberate duplication" -defense this file once carried; the reasoning is recorded in -[decisions/2026-07-12-unify-graph-traversal.md](../planning/decisions/2026-07-12-unify-graph-traversal.md). - -It collects **all errors** across the entire walk before raising, so a single call surfaces all wiring -bugs at once rather than stopping at the first one. `validate()` raises `exceptions.ValidationFailedError` -if any are found; its `.errors` attribute lists every collected exception, and its `__str__` groups them -by class name so a report mixing several error kinds reads as one section per kind — see -`ValidationFailedError.__str__` in `modern_di/exceptions.py`. Validation runs entirely against -`providers_registry`, so a root APP-scope container validates deeper-scoped providers without building -child containers. - -**The graph it walks is the graph that resolves.** Edges come from `WiringPlan.edges`, a view *derived* -from the same buckets `resolve()` reads (`provider_kwargs` + `context_kwargs`) rather than assembled -separately — so the validated graph cannot drift from the resolved one. In particular a provider supplied -via a declaration-time `kwargs={...}` is an edge like any type-matched one, and a cycle or scope inversion -routed through it is caught here rather than surfacing at resolve time as a bare `RecursionError` or a -`ScopeNotInitializedError`. See [resolution.md](resolution.md) for how the buckets are filled. - -**Validated-flag short-circuit.** `ProvidersRegistry` carries a `_validated: bool`, set by `mark_validated()` -on a successful walk; a later `validate()` while `_validated` is still `True` returns immediately without -re-walking, so a repeat `validate()` is free. Its only two mutators, `register` and `add_providers`, both -clear `_validated` back to `False`, so any change to the graph re-arms both -`validate()` and the runtime guard. The flag lives on the registry, which is shared tree-wide, so validating -any one container marks the graph clean for every container in the tree. - -### Circular dependencies - -When the walk follows an edge to a provider still on the active path (tracked in the walk's internal -`visiting` set), it emits a `Cycle` event whose `providers` list closes the loop by repeating the first -node last (e.g., `[A, B, A]`). `validate()` maps that event to a `CircularDependencyError` (built via -`dependency_graph.build_cycle_error`), which carries the loop as `.steps` — one `ResolutionStep` -(scope, name, optional definition site) per node. Before rendering, `build_cycle_error` rotates the loop -to start at its minimum-`provider_id` node, so the same cycle renders identically no matter which -provider the walk happened to seed from. The walk does **not** descend into the cycle, but the -rest of the graph continues to be checked. `CircularDependencyError.__str__` renders those steps as a -multi-line arrow chain, not an inline `A -> B -> A` string. - -`.cycle_path` (the bare list of type names, e.g. `["A", "B", "A"]`) and `.cycle_locations` (the parallel -`module:line` anchors) are **views derived from `.steps`**, so they cannot fall out of step with each -other or with what is rendered — the equal-length invariant is structural rather than enforced at render -time. Definition sites use the same lazy, memoized, best-effort capture described for breadcrumb steps in -[resolution.md](resolution.md#breadcrumb-definition-sites). - -Because a `ResolutionStep` carries its provider's scope, the cycle renders through the *same* chain drawer -as a resolution breadcrumb — including the aligned scope column — so a cycle and a failed resolution path -read identically. See [resolution.md](resolution.md#one-renderer) for that drawer. - -> **Runtime resolution has a cycle guard too — but `validate()` remains the way to see all errors up front.** -> `Container.resolve_provider` **and `Container.resolve`** each wrap the compiled-resolver dispatch -> (`resolver_for(provider)(self)`) in `try/except RecursionError` — `resolve` carries its own copy of that -> body rather than delegating, so the by-type entry point pays no extra frame -> ([decision](../planning/decisions/2026-08-03-resolve-provider-not-a-seam.md)). The -> handler first short-circuits: if the registry is already validated (`_validated` is `True`), the static -> graph is known acyclic, so the overflow is genuine self-recursion and the `RecursionError` re-raises untouched -> without any walk. Otherwise, when an unvalidated circular graph's first resolve overflows the stack, the handler -> re-walks the static graph from the failing provider via `DependencyGraph().find_cycle_from` — the same -> iterative, explicit-stack `walk` that `validate()` uses (it must stay flat, since it runs close to the recursion -> limit) — and, if a static cycle is reachable, raises `CircularDependencyError` (built by -> `dependency_graph.build_cycle_error`) with the cycle path, `from` the original `RecursionError`. -> `resolve_provider` is re-entrant: a cycle's back-edge is compiled as a thunk that routes back through it (a -> provider whose resolver was still under construction — see [resolution.md](resolution.md#cycle-safe-compilation)), -> so a loop stacks one `resolve_provider` frame per back-edge and the innermost one converts. -> `CircularDependencyError.prepend_step` overrides the breadcrumb machinery every other -> `ResolutionError` uses (see [resolution.md](resolution.md)) as a no-op, so an outer frame unwinding past the -> conversion adds nothing to it: the error is already self-contained the moment `build_cycle_error` constructs it, -> naming every provider in the loop canonically rooted at its minimum-`provider_id` node — not a partial breadcrumb -> an outer frame still needs to complete. A `RecursionError` from a creator that recurses on its own (no static -> cycle in the graph) is re-raised untouched, not misreported as a circular dependency. Both the guard and -> `validate()` consume -> the one `DependencyGraph` walker: `validate()` collects *all* errors of *all* kinds up front, while the guard -> only answers "is a cycle reachable from here" on an already-exhausted stack. Call `container.validate()` in -> development to surface *every* cycle (and other wiring bugs) before the first resolve, rather than only the -> one a particular resolve happens to hit. - -### Inverted scope dependencies - -For every dependency edge `provider → dep` (the walk's `Edge` event), `validate()` compares their -**terminal scopes** (see below). If `dep`'s terminal scope is strictly deeper than `provider`'s terminal -scope, the dependency is inverted: a shallower-lived provider cannot hold a reference to a deeper-lived -one. The error is recorded as `InvalidScopeDependencyError` (see `exceptions.py` for the exact message), -which names the provider, the parameter, the dependent provider, and the offending scopes. The walk -continues into the dependency so further issues in that subtree are also surfaced. - -### Missing required dependencies - -When the walk enters a provider (the `NodeEntered` event, emitted before that provider's dependencies are -read), `validate()` calls `provider.iter_validation_issues(container)` and appends any returned exceptions -to the error list. `Factory` implements this hook to yield `ArgumentResolutionError` for each constructor -parameter that has no matching provider in `providers_registry`, no default value, and no static `kwargs` -entry. - -## Terminal scope and alias transparency - -`validate()`'s scope-ordering check uses `DependencyGraph.terminal_scope(provider, container)` on both -sides of every dependency edge — not `provider.scope` directly. - -`terminal_scope` follows the `AbstractProvider.redirect_target(container)` node hook from provider to -provider until it reaches one whose resolution terminates there (`redirect_target` returns `None`), then -returns that terminal provider's `.scope`. The hook defaults to `None` on `AbstractProvider`, so for most -providers `terminal_scope` returns `self.scope` in a single step. `Alias` overrides `redirect_target` to -return its source provider (and `None` when the source type is unregistered), so `terminal_scope` follows -an alias chain to its terminal non-alias target and reports **that provider's scope**. This makes -validation transitive through aliases. Consider: - -``` -Factory(scope=APP, creator=Caller) # depends on IFace -Alias(source_type=Impl, bound_type=IFace) # no scope parameter -Factory(scope=REQUEST, creator=Impl) -``` - -The alias's terminal scope is `REQUEST` (the scope of `Impl`). When `validate()` checks the `Caller → IFace` -edge, it compares `APP` against `REQUEST` and raises `InvalidScopeDependencyError`. Without `terminal_scope`, -the alias's own `scope` attribute (defaulting to `APP`) would mask the true depth of the dependency. - -Two edge cases in `terminal_scope` are handled safely: - -- **Redirect cycle**: if the chain revisits a provider (tracked in `terminal_scope`'s `seen` set), it - breaks out and falls back to the starting provider's own `.scope` instead of looping forever. The cycle - itself is separately detected and reported as a `Cycle` event by the walk, which traverses the same - `source` edge. -- **Dangling source**: if an alias's source type is not registered, `redirect_target` returns `None`, so - the chain stops at the alias and falls back to its `.scope`. The dangling source is separately - reported by the alias's dependency lookup raising `AliasSourceNotRegisteredError` during the walk - (a `ResolutionError`, surfaced as a `DependenciesError` event). - -## Exception types - -| Exception | Base | Raised by | -|---|---|---| -| `ValidationFailedError` | `ContainerError` | `Container.validate()` — aggregate wrapper | -| `CircularDependencyError` | `ResolutionError` | recorded inside `validate()` on cycle detection | -| `InvalidScopeDependencyError` | `RegistrationError` | recorded inside `validate()` on inverted scope edge | -| `ArgumentResolutionError` | `ResolutionError` | yielded by `Factory.iter_validation_issues()` | - -Every concrete `ModernDIError` subclass — these three included — carries a class-level `docs_slug` -naming its page under `docs/troubleshooting/`; the census test (`tests/test_docs_slug_census.py`) -pins both that every slug is set and unique and that its page actually exists, so a new exception -cannot ship without a matching troubleshooting page. diff --git a/benchmarks/README.md b/benchmarks/README.md index ba8abec..7b11778 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -176,7 +176,7 @@ not of measured work. **Thread-safety configuration differs, at each framework's default.** dishka's `make_container` defaults to `lock_factory=`, so every `get()` in C1-C3 acquires a lock; `make_async_container` defaults to `asyncio.Lock`. modern-di's cached-read path is lock-free by -design (see `architecture/concurrency.md`), and its creation lock is double-checked. Every +design (see `docs/introduction/design-decisions.md`), and its creation lock is double-checked. Every framework here runs at its default, which is the comparison a user gets out of the box -- but a dishka user targeting single-threaded work can pass `lock_factory=None`, and that would move dishka's C1-C3 cells. The axis is disclosed rather than normalized away. diff --git a/planning/decisions/2026-07-13-integration-kit-shape.md b/planning/decisions/2026-07-13-integration-kit-shape.md index 8f2641b..9dffde6 100644 --- a/planning/decisions/2026-07-13-integration-kit-shape.md +++ b/planning/decisions/2026-07-13-integration-kit-shape.md @@ -7,8 +7,9 @@ summary: Integration kit lives in core as low-level primitives; outliers bypass **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. The shipped shape is documented in -[`integration-kit.md`](../../architecture/integration-kit.md). +growing parameters to swallow them. The boundary this shipped with is recorded in +[`2026-08-11-integration-kit-per-adapter-boundary.md`](2026-08-11-integration-kit-per-adapter-boundary.md), +which lists what stays per-adapter rather than moving into the shared kit. ## Context diff --git a/planning/decisions/2026-07-15-fold-context-registry-declined.md b/planning/decisions/2026-07-15-fold-context-registry-declined.md index e684aed..68bdd0b 100644 --- a/planning/decisions/2026-07-15-fold-context-registry-declined.md +++ b/planning/decisions/2026-07-15-fold-context-registry-declined.md @@ -29,7 +29,7 @@ Options: (a) fold it and update the docs; (b) decline. Chose (b). The deciding evidence: **`ContextRegistry` is a documented, symmetric node in a deliberate model, not incidental co-location.** -`architecture/containers.md` has a "Registry sharing" section that organises the +`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 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 index 54eeead..93a9eea 100644 --- a/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md +++ b/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md @@ -60,7 +60,7 @@ Two things from the attempt were kept: 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 - [`concurrency.md`](../../architecture/concurrency.md). + [`2026-08-11-free-threaded-beta-not-stable.md`](2026-08-11-free-threaded-beta-not-stable.md). 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 diff --git a/planning/decisions/2026-07-19-child-lazy-alloc-declined.md b/planning/decisions/2026-07-19-child-lazy-alloc-declined.md index 2dbab0a..f041688 100644 --- a/planning/decisions/2026-07-19-child-lazy-alloc-declined.md +++ b/planning/decisions/2026-07-19-child-lazy-alloc-declined.md @@ -40,7 +40,8 @@ hot path. Against a ~0-to-3.5% narrow win, lazy-allocation costs: 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 freshly - documented Beta contract ([`concurrency.md`](../../architecture/concurrency.md)). + documented Beta contract + ([`2026-08-11-free-threaded-beta-not-stable.md`](2026-08-11-free-threaded-beta-not-stable.md)). The `CacheRegistry`/`ContextRegistry` variants are *weaker* still: they are used more often in realistic children, so they save even less. Net negative for a diff --git a/planning/decisions/2026-07-26-explicit-only-validation.md b/planning/decisions/2026-07-26-explicit-only-validation.md index e00404f..abfae2d 100644 --- a/planning/decisions/2026-07-26-explicit-only-validation.md +++ b/planning/decisions/2026-07-26-explicit-only-validation.md @@ -6,9 +6,8 @@ summary: Validation is explicit-only — `validate()` is the sole trigger. The a **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. The rule is in -[`validation.md`](../../architecture/validation.md); this records the design -that was tried instead and why it lost. +validates, and `Container(validate=...)` is a deprecated no-op. This records +the design that was tried instead and why it lost. ## Context diff --git a/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md b/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md index ae66513..f8a45ce 100644 --- a/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md +++ b/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md @@ -67,7 +67,7 @@ taking. **Holding: decline.** Resolution stays untraced. Diagnostics remain the job of the error messages, which already carry the resolution breadcrumb chain -(`architecture/` and `docs/troubleshooting/`) at zero hot-path cost. +(`architecture/`, at the time, and `docs/troubleshooting/`) at zero hot-path cost. ## Revisit trigger diff --git a/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md b/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md index 33e723f..e4d7c0f 100644 --- a/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md +++ b/planning/decisions/2026-08-01-contextprovider-resolver-inline-declined.md @@ -28,8 +28,8 @@ touched no registry, so `_invalidate()` never fired and the memoized resolver wa 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 — see -[`architecture/providers.md`](../../architecture/providers.md) and +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. 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 index 677fc0c..2579390 100644 --- a/planning/decisions/2026-08-03-resolve-provider-not-a-seam.md +++ b/planning/decisions/2026-08-03-resolve-provider-not-a-seam.md @@ -49,8 +49,8 @@ ruled on together. **Field check.** An audit of all 13 sibling integration wheels found zero `Container` subclasses and zero `resolve_provider` overrides. `Container` -subclassing is not documented as an extension point anywhere in `architecture/` or -`docs/`. +subclassing was not documented as an extension point anywhere in `architecture/` +(since removed) or `docs/`. **Accepted costs**, disclosed rather than discovered later: diff --git a/planning/deferred/2026-07-19-free-threaded-throughput.md b/planning/deferred/2026-07-19-free-threaded-throughput.md index 274134f..6356a0b 100644 --- a/planning/deferred/2026-07-19-free-threaded-throughput.md +++ b/planning/deferred/2026-07-19-free-threaded-throughput.md @@ -31,8 +31,8 @@ shared object — inherent), then the shared **provider** objects (distinct-cont closures and their captured cells (every `LOAD_DEREF` of a shared capture increfs it). The per-container lock is **not** the bottleneck. First-resolve does additionally serialize on the double-checked creation lock — see -[`concurrency.md`](../../architecture/concurrency.md), which states the supported -lifecycle contract and the Beta status of free-threaded support. +[`2026-08-11-free-threaded-beta-not-stable.md`](../decisions/2026-08-11-free-threaded-beta-not-stable.md), +which states the supported lifecycle contract and the Beta status of free-threaded support. A throwaway **immortalization experiment** (ctypes set of `ob_ref_local` on the free-threaded build, offset verified against a known-immortal object) confirmed diff --git a/planning/deferred/2026-07-29-upstream-lean-convention.md b/planning/deferred/2026-07-29-upstream-lean-convention.md index ed70113..f19de65 100644 --- a/planning/deferred/2026-07-29-upstream-lean-convention.md +++ b/planning/deferred/2026-07-29-upstream-lean-convention.md @@ -17,8 +17,9 @@ re-apply via that repo's `APPLY.md` flow. files hold nothing durable. That held — 113 change files plus 18 audit reports yielded only 3 promotions to `decisions/`. But the *reason* was not the predicted one. The why-nots were not missing from `changes/`; they had **already been -promoted** into `architecture/`, `ROADMAP.md`, `benchmarks/README.md`, and -`docs/introduction/design-decisions.md` at ship time. Of the 33 files examined, +promoted** into `architecture/` (since removed — see the note below), +`ROADMAP.md`, `benchmarks/README.md`, and `docs/introduction/design-decisions.md` +at ship time. Of the 33 files examined, 16 were redundant because their content had been promoted or already had a `decisions/` file. @@ -32,7 +33,8 @@ to be true of itself before this is safe" — and 3.0.0 would ship it to consume who may not meet that bar. Three routes: 1. **Soak longer here**, then upstream as 3.0.0 with a stated precondition: - adopt only if your `architecture/` promotions are reliable. + adopt only if your capability-documentation promotions are reliable + (`architecture/` was this repo's version of that target; see the note below). 2. **Upstream as an optional profile** alongside the current one, so repos choose. This was considered when the plan was made and passed over; the harvest is an argument to reconsider it. @@ -73,6 +75,30 @@ Three things should inform whichever route is taken, and none has evidence yet: supersession, the honest conclusion is that `decisions/` needs no state marker at all and the key should go too. +## Update: the promotion target this item relied on is gone + +`architecture/` was deleted 2026-08-11 +([`2026-08-11-drop-architecture-directory.md`](../decisions/2026-08-11-drop-architecture-directory.md)): +11 capability pages routed instead to code, a named `INVARIANT:`-marked test, +`decisions/`, or `docs/`, per the four-homes admission check now in +`planning/README.md`. The premise this item's central finding rested on — +deleting `changes/` was safe *because* `architecture/` promotion discipline +worked — is no longer demonstrable the same way, because the thing being +promoted *into* no longer exists as a single directory to point at. + +This does not resolve the upstream question; if anything it sharpens it. The +harvest's finding still holds as a historical fact (16 of 33 examined files +were redundant because their content had already reached a durable home by ship +time). But route 1's phrasing above — "adopt only if your `architecture/` +promotions are reliable" — no longer names a target this repo still has. A +2.30.0-vintage answer would need to generalize it to "adopt only if your +capability facts reliably reach *some* durable home (code, tests, +`decisions/`, `docs/`)," which is the four-homes model this repo now runs, not +the two-destination one (`architecture/` and everything else) the harvest was +measured against. Whether that generalized claim holds needs its own evidence; +none exists yet. The revisit trigger below is unchanged — this update narrows +what "check" means but does not move the date. + ## Revisit trigger After roughly a month of real changes under the new convention (so, from diff --git a/planning/releases/2.28.0.md b/planning/releases/2.28.0.md index a40a452..e21bc11 100644 --- a/planning/releases/2.28.0.md +++ b/planning/releases/2.28.0.md @@ -44,7 +44,7 @@ changes. `integrations.is_injected`/`mark_injected` guard against double-wrapping a handler an auto-inject sweep visits more than once. - See [architecture/integration-kit.md](https://github.com/modern-python/modern-di/blob/main/architecture/integration-kit.md) + See [architecture/integration-kit.md](https://github.com/modern-python/modern-di/blob/2.28.0/architecture/integration-kit.md) for the full design, and the updated [writing-integrations guide](https://modern-di.modern-python.org/integrations/writing-integrations/) for how an adapter composes these. diff --git a/planning/releases/2.30.0.md b/planning/releases/2.30.0.md index 3aa8217..ca54674 100644 --- a/planning/releases/2.30.0.md +++ b/planning/releases/2.30.0.md @@ -42,11 +42,11 @@ provider concurrently could raise a spurious `RecursionError`. The fix leads. `set_context` during single-threaded setup, *before* resolving concurrently — mutating a shared registry mid-resolution is inherently unordered (it always was, GIL or not). The full contract lives in - [`architecture/concurrency.md`](https://github.com/modern-python/modern-di/blob/main/architecture/concurrency.md). + [`architecture/concurrency.md`](https://github.com/modern-python/modern-di/blob/2.30.0/architecture/concurrency.md). ## Docs & internals -- New [`architecture/concurrency.md`](https://github.com/modern-python/modern-di/blob/main/architecture/concurrency.md) +- New [`architecture/concurrency.md`](https://github.com/modern-python/modern-di/blob/2.30.0/architecture/concurrency.md) capability page — the standing thread-safety contract (the locked singleton path, the idempotent lock-free memoization, the thread-local cycle guard, the Beta stance and its publication-ordering caveat). From 10021ea3b5097f0daaece49359d9d3db85f1af5c Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 20:19:06 +0300 Subject: [PATCH 14/18] fix(docs): close out final review of drop-architecture-dir Applies the fix wave from the branch's final whole-branch review: gives the union-member-origin-degradation contract a home (INVARIANT docstring + docs correction), fixes two planning/decisions cross-references that misroute readers, restores the glossary charter's subtle-meaning clause in CLAUDE.md, closes three citation-path gaps in the invariant census (module paths, planning-record paths, and Markdown-cited test names), names decisions/ and INVARIANT: docstrings as accumulation surfaces that need their own subtraction habit, and applies four minor cleanups (deduplicated docstrings, a redundant clause, "since removed" phrasing, and the positional-fast-path measurement rationale). --- CLAUDE.md | 5 +- docs/providers/factories.md | 10 ++- modern_di/registries/providers_registry.py | 3 +- modern_di/resolver_compiler.py | 1 + modern_di/types_parser.py | 3 + planning/README.md | 4 + ...07-30-debug-resolution-tracing-declined.md | 2 +- .../2026-08-11-drop-architecture-directory.md | 9 +- tests/test_invariant_census.py | 84 ++++++++++++++++--- tests/test_resolver_compiler.py | 23 +++-- tests/test_types_parser.py | 9 ++ 11 files changed, 120 insertions(+), 33 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a7a0089..4d00b1a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,7 +13,7 @@ This project uses `just` (task runner) and `uv` (package manager). The 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 test_name`. + 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. @@ -108,7 +108,8 @@ local deviation from `planning-convention` 2.2.0. ## Vocabulary -A term is listed only when there is a synonym to reject. +A term is listed only when there is a synonym to reject, or a meaning subtle enough that code and +docs must agree on it. - **Container** — owns the registries and resolves within a scope. *Avoid:* injector. - **Provider** — a declaration of *how to produce* a dependency; the recipe, not the value. *Avoid:* service, diff --git a/docs/providers/factories.md b/docs/providers/factories.md index 7771541..dcef0a5 100644 --- a/docs/providers/factories.md +++ b/docs/providers/factories.md @@ -210,13 +210,19 @@ The table below summarises how Modern-DI handles each parameter shape during **d |---|---|---| | `param: SomeClass` — plain type annotation with a registered provider | Resolved and injected automatically. | `ArgumentResolutionError` at resolve if no provider is registered and there is no default. | | `param: X | None` / `Optional[X]` | Provider injected if one is registered; otherwise `None`. | Never fails — see [Optional parameters](#optional-parameters). | -| `param: A | B` — union without `None` | First registered type from the union is injected. | `ArgumentResolutionError` at resolve if neither `A` nor `B` has a registered provider. | -| `param: list[X]` / any parameterized generic | **`UnsupportedCreatorParameterError` at declaration** unless the parameter has a default value or is covered by `kwargs`. | Raised at `Factory(...)` call time. | +| `param: A | B` — union without `None` | First registered type from the union is injected. A member that is itself a parameterized generic (e.g. `int | list[X]`) degrades to its bare origin (`list`) for matching purposes — see the note below. | `ArgumentResolutionError` at resolve if neither `A` nor `B` has a registered provider. | +| `param: list[X]` / any parameterized generic, **outside a union** | **`UnsupportedCreatorParameterError` at declaration** unless the parameter has a default value or is covered by `kwargs`. | Raised at `Factory(...)` call time. | | Positional-only param (`def f(x: T, /)`) | **`UnsupportedCreatorParameterError` at declaration** unless the parameter has a default (in which case it is silently skipped). | Raised at `Factory(...)` call time. | | Unannotated param (`def f(x)`) | Parsed but unresolvable by type. | `ArgumentResolutionError` at resolve unless covered by `kwargs`. | | Signature whose hints `get_type_hints` cannot resolve (e.g. a forward reference to an undefined name, or — on Python < 3.14 — `functools.partial`) | `UserWarning` is emitted and type-based wiring is skipped; parameters are still parsed (as unannotated). Silence by passing `skip_creator_parsing=True` and an explicit `bound_type`. | A required unannotated param with no provider/default raises `ArgumentResolutionError` at resolve unless covered by `kwargs` (a parameterized-generic or positional-only param still raises `UnsupportedCreatorParameterError` at declaration). | | `skip_creator_parsing=True` | No wiring at all — every required argument must be supplied via `kwargs`. | `CreatorCallError` at resolve for any missing required argument. | +A parameterized generic used *inside* a union (`param: int | list[X]`) is the one exception to +the "parameterized generic raises at declaration" row above: the member degrades to its bare +origin type like any other union member, so it can match a provider registered for `list`. The +element type `X` is not checked in that case — this is intentional, not a wiring guarantee, so +don't rely on it to route only correctly-typed collections. + **Escaping problem shapes** — if a parameter shape would raise at declaration, there are three escape routes, in order of preference: 1. Give the parameter a default value (`def f(items: list[X] | None = None)`). diff --git a/modern_di/registries/providers_registry.py b/modern_di/registries/providers_registry.py index 070b412..9074833 100644 --- a/modern_di/registries/providers_registry.py +++ b/modern_di/registries/providers_registry.py @@ -142,8 +142,7 @@ def _invalidate(self) -> None: Called under `self._lock` by every mutation. Clearing has the same breadth the old version bump did (a bump invalidated every memo anyway) and frees stale entries eagerly. Sound - because mutation is a single-threaded configure-phase operation (configure-phase mutation; - see tests/test_free_threading.py). + because mutation is a single-threaded configure-phase operation (see tests/test_free_threading.py). """ self._plans.clear() self._resolvers.clear() diff --git a/modern_di/resolver_compiler.py b/modern_di/resolver_compiler.py index 5490c75..b302c8b 100644 --- a/modern_di/resolver_compiler.py +++ b/modern_di/resolver_compiler.py @@ -94,6 +94,7 @@ def _compile_transient_factory( # noqa: C901, PLR0915 (two hot-path closures: p if _can_call_positionally(f, plan): # Positional fast path; `pure` is True here, so no static/context folding runs. + # Measured: creator(**kwargs) costs 4-6x this path -- do not simplify it away. # See test_resolve_costs_exactly_one_resolver_frame_per_node. pos = tuple(r for _name, r in prov) diff --git a/modern_di/types_parser.py b/modern_di/types_parser.py index 6ba9758..82903b6 100644 --- a/modern_di/types_parser.py +++ b/modern_di/types_parser.py @@ -33,6 +33,9 @@ def from_type(cls, type_: type, default: object = UNSET) -> "SignatureItem": # union type if isinstance(type_, types.UnionType) or typing.get_origin(type_) is typing.Union: + # A parameterized generic member degrades to its origin (list[str] -> list); the + # element type is not enforced. Intentional asymmetry, not a wiring guarantee -- + # see test_signature_item_parser. union_members = [typing.get_origin(x) or x for x in typing.get_args(type_)] non_none_members = [member for member in union_members if member is not types.NoneType] if len(non_none_members) != len(union_members): diff --git a/planning/README.md b/planning/README.md index 83d70ca..729903f 100644 --- a/planning/README.md +++ b/planning/README.md @@ -62,6 +62,10 @@ load-bearing and none removed one, so the pages ratcheted toward restating code, restatement is what goes stale. The absence of the directory is the mechanism. See [`decisions/2026-08-11-drop-architecture-directory.md`](decisions/2026-08-11-drop-architecture-directory.md). +`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. + 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 is where an anti-refactor warning lives — design rationale, not a report of what this diff --git a/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md b/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md index f8a45ce..287d360 100644 --- a/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md +++ b/planning/decisions/2026-07-30-debug-resolution-tracing-declined.md @@ -67,7 +67,7 @@ taking. **Holding: decline.** Resolution stays untraced. Diagnostics remain the job of the error messages, which already carry the resolution breadcrumb chain -(`architecture/`, at the time, and `docs/troubleshooting/`) at zero hot-path cost. +(`architecture/`, since removed, and `docs/troubleshooting/`) at zero hot-path cost. ## Revisit trigger diff --git a/planning/decisions/2026-08-11-drop-architecture-directory.md b/planning/decisions/2026-08-11-drop-architecture-directory.md index d38d012..a808cae 100644 --- a/planning/decisions/2026-08-11-drop-architecture-directory.md +++ b/planning/decisions/2026-08-11-drop-architecture-directory.md @@ -8,8 +8,8 @@ summary: The `architecture/` directory is deleted; its facts route to code, a na enforceable claim becomes an `INVARIANT:`-marked test guarded by `tests/test_invariant_census.py`; a negative contract (something deliberately *not* guaranteed) becomes a `planning/decisions/` record; a framework-facing contract moves to `docs/`; a term worth pinning down that isn't fully derivable from -code stays in whatever residual glossary form `planning/` ends up using. Nothing is promoted to a -standalone prose "truth home" page again. +code stays in the Vocabulary block in `CLAUDE.md`. Nothing is promoted to a standalone prose "truth +home" page again. ## Context @@ -81,6 +81,11 @@ the glossary's `Avoid:` entries — the deliberately-not-used synonyms for a ter derivable from reading the code. Code shows what a name *is*; it doesn't show what a contributor *almost* called it instead and why that was wrong. That negative information has nowhere else to live. +**Where it landed:** a short Vocabulary block in `CLAUDE.md`, not a standalone glossary file. It is +small enough — ten entries, name plus one-line `Avoid:` — that the continuous-re-editing failure mode +the rest of this decision describes doesn't apply to it, and it is read as part of the file every +contributor already opens rather than a page someone has to remember exists. + ## Revisit trigger An invariant is found that has no test form (nothing to assert), no decision form (it isn't a call diff --git a/tests/test_invariant_census.py b/tests/test_invariant_census.py index 67b7eb2..257f6ca 100644 --- a/tests/test_invariant_census.py +++ b/tests/test_invariant_census.py @@ -1,9 +1,10 @@ """Census of invariant tests and the citations that point to them. Every ``test_*`` name cited from a comment or docstring under ``modern_di/`` or ``tests/`` -resolves to a real test, 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. +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. """ import ast @@ -15,10 +16,23 @@ _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 @@ -27,9 +41,9 @@ _DOCSTRING_NODE_TYPES = (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef) -def _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: +def _walk_test_functions(root: pathlib.Path) -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFunctionDef]]: found = [] - for path in sorted(_TESTS_DIR.rglob("*.py")): + for path in sorted(root.rglob("*.py")): tree = ast.parse(path.read_text(encoding="utf-8")) found.extend( (path, node) @@ -39,33 +53,52 @@ def _test_functions() -> list[tuple[pathlib.Path, ast.FunctionDef | ast.AsyncFun 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_citations(path: pathlib.Path) -> set[str]: - """Names cited from real comments only -- tokenize, so a `#` inside a string is not a comment.""" +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 _CITATION.findall(token.string) + for name in pattern.findall(token.string) } -def _docstring_citations(path: pathlib.Path) -> set[str]: - """Names cited from any module/class/function docstring in the file.""" +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(_CITATION.findall(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" @@ -91,3 +124,32 @@ 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}" diff --git a/tests/test_resolver_compiler.py b/tests/test_resolver_compiler.py index b58444f..d7706c2 100644 --- a/tests/test_resolver_compiler.py +++ b/tests/test_resolver_compiler.py @@ -371,7 +371,8 @@ def test_can_call_positionally_rejects_static_or_context_kwarg() -> None: A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of - them trades correctness for speed. + them trades correctness for speed. The other reject-case tests for `_can_call_positionally` + below share this rationale rather than repeating it. """ # rule 1: a context param makes the plan non-pure, so kwargs folding must run. @@ -392,9 +393,8 @@ def creator(dep: _A, req: _Req) -> _Ordered: def test_can_call_positionally_rejects_defaulted_omitted_param() -> None: """INVARIANT: the positional-path predicate excludes a defaulted, omitted param. - A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow - path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of - them trades correctness for speed. + See `test_can_call_positionally_rejects_static_or_context_kwarg` for why a wrong `True` here is + a correctness bug, not a slow path. """ # rule 2a: `opt` has a default and no provider, so it is omitted -> provider_kwargs is a @@ -413,9 +413,8 @@ def creator(dep: _A, opt: int = 5) -> _Ordered: def test_can_call_positionally_rejects_kwargs_overlay_reorder() -> None: """INVARIANT: the positional-path predicate excludes a kwargs-overlay reorder. - A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow - path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of - them trades correctness for speed. + See `test_can_call_positionally_rejects_static_or_context_kwarg` for why a wrong `True` here is + a correctness bug, not a slow path. """ # rule 2b: supplying `a` via the kwargs overlay defers it to the end of provider_kwargs, @@ -437,9 +436,8 @@ def creator(a: _A, b: _B) -> _Ordered: def test_can_call_positionally_rejects_keyword_only_param() -> None: """INVARIANT: the positional-path predicate excludes a keyword-only param. - A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow - path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of - them trades correctness for speed. + See `test_can_call_positionally_rejects_static_or_context_kwarg` for why a wrong `True` here is + a correctness bug, not a slow path. """ # rule 3: a keyword-only dep can never be passed positionally. @@ -457,9 +455,8 @@ def creator(*, dep: _A) -> _Ordered: def test_can_call_positionally_rejects_positional_only_param() -> None: """INVARIANT: the positional-path predicate excludes a positional-only param. - A wrong `True` silently binds arguments to the wrong parameters -- a correctness bug, not a slow - path. Every negative case must keep `creator(**kwargs)`; widening the predicate to admit one of - them trades correctness for speed. + See `test_can_call_positionally_rejects_static_or_context_kwarg` for why a wrong `True` here is + a correctness bug, not a slow path. """ # rule 4: `prefix` is positional-only WITH a default, dropped from parsed_kwargs so the diff --git a/tests/test_types_parser.py b/tests/test_types_parser.py index 9d277e3..f14ca57 100644 --- a/tests/test_types_parser.py +++ b/tests/test_types_parser.py @@ -35,6 +35,15 @@ class GenericClass(typing.Generic[types.T]): ... ], ) def test_signature_item_parser(type_: type, result: SignatureItem) -> None: + """INVARIANT: inside a union, a parameterized generic member degrades to its bare origin. + + `list[str] | None` and `GenericClass[str] | None` (parametrize table above) both resolve to + `arg_type=list` / `arg_type=GenericClass` — the element type is dropped, not enforced. `int | list[str]` can + match a provider registered for plain `list` regardless of what it holds. This is intentional + and is not a wiring guarantee: the asymmetry with the bare-generic case (`list[str]` alone + raises `UnsupportedCreatorParameterError` at declaration, see `_generic_param_creator` below) + is deliberate, not a bug to reconcile by making one side match the other. + """ assert SignatureItem.from_type(type_) == result From 79e761d886d66261927fad5544f1cf89026ece9d Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 20:44:06 +0300 Subject: [PATCH 15/18] fix(planning): widen census sentence, extract the union-degradation invariant planning/README.md understated test_invariant_census.py after the final fix wave widened its citation scope beyond modern_di/ comments; the sentence now names all four sources it checks. The union-degradation contract lived as an INVARIANT docstring on test_signature_item_parser, a 12-row parametrized table whose name is not the claim -- editing rows :30/:32 could silently orphan it. Extracted a dedicated test_union_member_degrades_to_bare_origin asserting the claim directly and moved the docstring there; the two table rows stay, since the table's job is general parser regression coverage, not hosting this one invariant. Updated the mechanism comment in types_parser.py to cite the new test. --- modern_di/types_parser.py | 2 +- planning/README.md | 5 +++-- tests/test_types_parser.py | 28 +++++++++++++++++++--------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/modern_di/types_parser.py b/modern_di/types_parser.py index 82903b6..cf140b4 100644 --- a/modern_di/types_parser.py +++ b/modern_di/types_parser.py @@ -35,7 +35,7 @@ def from_type(cls, type_: type, default: object = UNSET) -> "SignatureItem": if isinstance(type_, types.UnionType) or typing.get_origin(type_) is typing.Union: # A parameterized generic member degrades to its origin (list[str] -> list); the # element type is not enforced. Intentional asymmetry, not a wiring guarantee -- - # see test_signature_item_parser. + # see test_union_member_degrades_to_bare_origin. union_members = [typing.get_origin(x) or x for x in typing.get_args(type_)] non_none_members = [member for member in union_members if member is not types.NoneType] if len(non_none_members) != len(union_members): diff --git a/planning/README.md b/planning/README.md index 729903f..c16b950 100644 --- a/planning/README.md +++ b/planning/README.md @@ -74,8 +74,9 @@ 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 cited -from a `modern_di/` comment exists. +`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 diff --git a/tests/test_types_parser.py b/tests/test_types_parser.py index f14ca57..b171390 100644 --- a/tests/test_types_parser.py +++ b/tests/test_types_parser.py @@ -35,18 +35,28 @@ class GenericClass(typing.Generic[types.T]): ... ], ) def test_signature_item_parser(type_: type, result: SignatureItem) -> None: - """INVARIANT: inside a union, a parameterized generic member degrades to its bare origin. - - `list[str] | None` and `GenericClass[str] | None` (parametrize table above) both resolve to - `arg_type=list` / `arg_type=GenericClass` — the element type is dropped, not enforced. `int | list[str]` can - match a provider registered for plain `list` regardless of what it holds. This is intentional - and is not a wiring guarantee: the asymmetry with the bare-generic case (`list[str]` alone - raises `UnsupportedCreatorParameterError` at declaration, see `_generic_param_creator` below) - is deliberate, not a bug to reconcile by making one side match the other. - """ assert SignatureItem.from_type(type_) == result +def test_union_member_degrades_to_bare_origin() -> None: + """INVARIANT: inside a union, each member degrades to its bare origin type; the element type is not enforced. + + `list[str] | None` and `GenericClass[str] | None` both resolve to `arg_type=list` and + `arg_type=GenericClass` — the element type is dropped, not enforced. `int | list[str]` can match a + provider registered for plain `list` regardless of what it holds. This is intentional and not a + wiring guarantee: the asymmetry with the bare-generic case (`list[str]` alone raises + `UnsupportedCreatorParameterError` at declaration — see + `test_parameterized_generic_param_without_default_raises_at_declaration`) is deliberate, not a bug + to reconcile by making one side match the other. + """ + assert SignatureItem.from_type(list[str] | None) == SignatureItem( # ty: ignore[invalid-argument-type] + arg_type=list, is_nullable=True + ) + assert SignatureItem.from_type(GenericClass[str] | None) == SignatureItem( # ty: ignore[invalid-argument-type] + arg_type=GenericClass, is_nullable=True + ) + + @pytest.mark.parametrize("default", [None, 3, "x"]) def test_nonetype_threads_its_default(default: object) -> None: """A `None`-annotated parameter keeps its default, like every other annotation. From 8e4d158d19065127ec52373f4a69c20a19808b97 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 21:32:20 +0300 Subject: [PATCH 16/18] docs(decisions): drop the five 2026-08-11 records Keeps only the free-threaded Beta record, which is cited from tests/test_free_threading.py and guarded by the census. Fixes the three links that would have dangled, and corrects the two thinning-commit figures in planning/README.md, which still labelled gross deletions as net cuts (433 deletions against 110 insertions across six pages; 292 against 147 across four). --- planning/README.md | 12 +-- .../2026-07-13-integration-kit-shape.md | 4 +- .../2026-08-11-drop-architecture-directory.md | 96 ------------------- ...11-integration-kit-per-adapter-boundary.md | 62 ------------ ...8-11-override-value-is-not-type-checked.md | 66 ------------- ...ebinding-an-in-use-provider-unsupported.md | 60 ------------ ...11-transient-teardown-order-unspecified.md | 53 ---------- .../2026-07-29-upstream-lean-convention.md | 3 +- 8 files changed, 8 insertions(+), 348 deletions(-) delete mode 100644 planning/decisions/2026-08-11-drop-architecture-directory.md delete mode 100644 planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md delete mode 100644 planning/decisions/2026-08-11-override-value-is-not-type-checked.md delete mode 100644 planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md delete mode 100644 planning/decisions/2026-08-11-transient-teardown-order-unspecified.md diff --git a/planning/README.md b/planning/README.md index c16b950..b01b72f 100644 --- a/planning/README.md +++ b/planning/README.md @@ -55,12 +55,12 @@ Before writing a line anywhere: This is deliberate, and it is the second lesson rather than the first. A capability directory (`architecture/`) was kept for four months and cut to invariants twice — -`b2404c4` (#282, 2026-07-07, −433 lines) and `047b6ea` (#395, 2026-07-29, −292 -lines) — and regrew both times. Promotion discipline was not the problem: 72% of -commits touching `modern_di/` also touched it. Every PR added a paragraph that felt -load-bearing and none removed one, so the pages ratcheted toward restating code, and -restatement is what goes stale. The absence of the directory is the mechanism. See -[`decisions/2026-08-11-drop-architecture-directory.md`](decisions/2026-08-11-drop-architecture-directory.md). +`b2404c4` (#282, 2026-07-07: 433 deletions against 110 insertions across six pages) +and `047b6ea` (#395, 2026-07-29: 292 against 147 across four) — and regrew both +times. Promotion discipline was not the problem: 72% of commits touching +`modern_di/` also touched it. Every PR added a paragraph that felt load-bearing and +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 diff --git a/planning/decisions/2026-07-13-integration-kit-shape.md b/planning/decisions/2026-07-13-integration-kit-shape.md index 9dffde6..e14c1de 100644 --- a/planning/decisions/2026-07-13-integration-kit-shape.md +++ b/planning/decisions/2026-07-13-integration-kit-shape.md @@ -7,9 +7,7 @@ summary: Integration kit lives in core as low-level primitives; outliers bypass **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. The boundary this shipped with is recorded in -[`2026-08-11-integration-kit-per-adapter-boundary.md`](2026-08-11-integration-kit-per-adapter-boundary.md), -which lists what stays per-adapter rather than moving into the shared kit. +growing parameters to swallow them. ## Context diff --git a/planning/decisions/2026-08-11-drop-architecture-directory.md b/planning/decisions/2026-08-11-drop-architecture-directory.md deleted file mode 100644 index a808cae..0000000 --- a/planning/decisions/2026-08-11-drop-architecture-directory.md +++ /dev/null @@ -1,96 +0,0 @@ ---- -summary: The `architecture/` directory is deleted; its facts route to code, a named test, `planning/decisions/`, or `docs/` instead of a prose page that restates the code and drifts. ---- - -# Drop the `architecture/` directory - -**Decision:** `architecture/` is deleted. What it used to carry now routes to one of four homes: an -enforceable claim becomes an `INVARIANT:`-marked test guarded by `tests/test_invariant_census.py`; a -negative contract (something deliberately *not* guaranteed) becomes a `planning/decisions/` record; a -framework-facing contract moves to `docs/`; a term worth pinning down that isn't fully derivable from -code stays in the Vocabulary block in `CLAUDE.md`. Nothing is promoted to a standalone prose "truth -home" page again. - -## Context - -`architecture/` was 11 prose pages, one per capability, meant to be the living, code-current account of -the library's behaviour — the `CLAUDE.md` at the repo root called it out as "quick orientation only... -the authoritative, code-current account of each capability." The convention was: a behaviour change -hand-edits the matching capability page in the same PR, and a checklist item asked reviewers to confirm -that happened. - -That promotion discipline **worked** — PRs did update the pages. It just didn't work in the direction -that keeps a doc small. Two prior attempts tried to correct the resulting bloat by cutting the pages -back down rather than questioning whether the pages should exist at all: - -- **`b2404c4`** (#282, 2026-07-07, "docs: trim architecture/ to charter — invariants only") — 433 - deletions against 110 insertions across six pages, a net −323. -- **`047b6ea`** (#395, 2026-07-29, "docs(architecture): thin resolution.md to invariants; one owner per - concept") — 292 deletions against 147 insertions across four pages (`README.md`, `containers.md`, - `providers.md`, `resolution.md`), a net −145; `resolution.md` alone was 206 deletions against 121 - insertions, a net −85. - -Both regrew. `resolution.md` — a 175-line file at the point this decision was made — accumulated -+617/−442 lines across 23 commits since it existed; a page trimmed twice still ended up net-larger than -either trim removed. Contrast `glossary.md`, which took +93/−4 across only 2 commits: a page that -mostly just gets written once and referenced, not continuously re-edited to track behaviour, doesn't -have this problem at all. The asymmetry is the tell — it isn't that contributors write bloated prose in -general, it's specifically the pages the promotion checklist forces continuous re-editing of. - -## Decision & rationale - -**The promotion discipline was not the failure — it was working exactly as designed, and that's the -problem.** Measured at `ed9b00d`, the commit this branch forked from (deliberately excluding this -branch's own commits, so the figure describes the pre-existing problem rather than this branch's own -churn): of the 67 commits touching `modern_di/` since mid-June, 48 also touched `architecture/` — 72%. -That's a *high* compliance rate with "did you update the page?" The checklist item that drove it only -ever asked that one question. It never asked "should -this paragraph exist?" So every PR that touched behaviour had a structural incentive to *add* a -sentence explaining the new behaviour, and no PR had any correspondent incentive to *remove* a sentence -whose enforcement value had already been captured elsewhere — in a test, in a type signature, in the -code itself being self-evident on read. Addition-without-subtraction, run for weeks across dozens of -compliant PRs, is exactly the churn profile `resolution.md` shows. - -Tasks 1–4 of this branch already did the harder part of the fix: they read the 11 pages, found the -claims that were actually enforceable, and turned 30 of them into `INVARIANT:`-marked tests. A test -regresses automatically when it stops being true; a prose paragraph doesn't. That converts "the page -still says X" from a discipline problem back into a mechanical one. - -**What's left — five records in this file's cohort — could not go into a test**, because their entire -content is the *absence* of a guarantee: no assertion follows from "the library does not promise this -order" or "this value is not type-checked." Deleting the page without capturing them would silently -turn each into either an unfixed bug report (someone "fixes" the unspecified transient teardown order) -or an unsupported reliance (someone starts depending on override values being type-checked because -nothing said otherwise). `planning/decisions/` is the right home because these are exactly what that -directory is for: options considered and a call made, with reasoning a future explorer would otherwise -re-litigate. - -**Rejected alternative: keep a smaller `architecture/`.** This is the option the two prior thinnings -already tried, twice, and it regrew both times. A third attempt with a stricter charter has no reason -to fare differently unless the underlying incentive changes — and the underlying incentive is the -promotion checklist itself, which this decision removes rather than tightens. - -**Rejected alternative: turn the glossary into a decision record.** A glossary is consulted *while -writing* — a contributor mid-PR needs to know what `bound_type` means right now, inline with the code -they're editing. A decision record is consulted *while deciding* — before writing, to check whether an -option was already rejected. Those are different reading moments; collapsing them into one file type -would serve neither well. - -**Rejected alternative: drop the glossary entirely, relying on code and docstrings.** Rejected because -the glossary's `Avoid:` entries — the deliberately-not-used synonyms for a term, and why — are not -derivable from reading the code. Code shows what a name *is*; it doesn't show what a contributor -*almost* called it instead and why that was wrong. That negative information has nowhere else to live. - -**Where it landed:** a short Vocabulary block in `CLAUDE.md`, not a standalone glossary file. It is -small enough — ten entries, name plus one-line `Avoid:` — that the continuous-re-editing failure mode -the rest of this decision describes doesn't apply to it, and it is read as part of the file every -contributor already opens rather than a page someone has to remember exists. - -## Revisit trigger - -An invariant is found that has no test form (nothing to assert), no decision form (it isn't a call -between rejected alternatives — it's simply a fact worth stating), and is needed by more than one -reader. A single such fact is better placed in the nearest docstring or `docs/` page; a *pattern* of -such facts, recurring enough that they'd naturally cluster into one file again, is the signal that a -`architecture/`-shaped truth home is needed after all — and if that happens, the fix this time is a -charter that states what does *not* belong on the page, not just what does. diff --git a/planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md b/planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md deleted file mode 100644 index eda0a87..0000000 --- a/planning/decisions/2026-08-11-integration-kit-per-adapter-boundary.md +++ /dev/null @@ -1,62 +0,0 @@ ---- -summary: Root-container lifecycle, per-connection child stash/read-back, sync-vs-async close, and handler-signature rewriting stay in each adapter — the integration kit does not absorb them. ---- - -# What stays per-adapter is not part of the integration kit - -**Decision:** Four responsibilities stay outside `modern_di.integrations` and are re-implemented by -each of the 13 framework adapters individually: root-container lifecycle (open/close, where it's -attached to framework state), where the per-connection child container is stashed and read back, -choosing `close_sync` vs. `close_async`, and any handler-signature rewriting (stripping a parameter, -inserting a context object). None of these move into the shared kit. - -## Context - -`modern_di.integrations` extracts what the 13 adapters duplicated near-verbatim: Layer 1 -(`bind`, `classify_connection`) derives a child container's scope/context from `ContextProvider`s, and -Layer 2 (`Marker`, `from_di`, `parse_markers`, `resolve_markers`) is the `Annotated`-marker injector, -replacing the `_parse_inject_params`/resolve pair every non-native-DI integration had reimplemented. -The related decision [`2026-07-13-integration-kit-shape.md`](2026-07-13-integration-kit-shape.md) -settled the kit's overall shape (low-level primitives in core, outliers bypass rather than the -primitives absorbing them). This record is narrower: it's about the four things that were considered -for extraction *into* the kit and rejected, not about the outlier adapters that bypass it. - -## Decision & rationale - -Each of the four is irreducibly framework-specific in a way the shared primitives are not: - -- **Root-container lifecycle** — *where* a framework hangs the root container (an ASGI app's - `state`, a Celery worker's global, a Typer command's context object) and *when* it opens and closes - it are governed entirely by that framework's own lifecycle hooks. There's no shared shape to - extract; a common "lifecycle manager" would need a callback per framework anyway, which is just the - per-adapter code with extra indirection. -- **Where the per-connection child is stashed and read back** — an HTTP framework has a request - object, an ASGI framework has a scope dict, a message-queue framework has neither and uses a - contextvar or a task-local. The storage medium varies by what the framework hands the adapter, not - by anything modern-di controls. -- **`close_sync` vs. `close_async`** — which one an adapter calls depends on whether the framework's - own request/task teardown hook is sync or async, which is a property of the framework's execution - model, not of the container. -- **Handler-signature rewriting** — stripping an injected parameter or inserting a context object - before calling the user's handler requires knowing that framework's handler-calling convention - (decorator-wrapped function, class-based view method, positional vs. keyword dispatch). There is no - framework-agnostic way to rewrite "a handler" in general. - -**Rejected alternative: grow the primitives to absorb these.** This was the same shape of argument -`2026-07-13-integration-kit-shape.md` already settled for the three concrete outliers (aiohttp -websocket probe, grpc `set_context` split, typer no-context) — an absorbing parameter needed by one -adapter taxes the other adapters and lowers the kit's depth. These four are the general case of that -same argument: each is needed by *every* adapter, but in a *different shape* per adapter, so there is -no single parameter or callback that would fit all 13 without becoming a second, parallel dispatch -mechanism duplicating what the framework already provides. - -The full contract an integration implements around the shared primitives — including these four -per-adapter responsibilities — is documented in -[docs/integrations/writing-integrations.md](../../docs/integrations/writing-integrations.md). - -## Revisit trigger - -A second and third adapter converge on the *same* concrete shape for stashing the per-connection -child, or for signature rewriting — not just the same responsibility, but the same mechanism — making -a shared helper a two-adapters-rule extraction rather than a hypothetical one, the same bar -`2026-07-13-integration-kit-shape.md` used for its outliers. diff --git a/planning/decisions/2026-08-11-override-value-is-not-type-checked.md b/planning/decisions/2026-08-11-override-value-is-not-type-checked.md deleted file mode 100644 index 4f601b4..0000000 --- a/planning/decisions/2026-08-11-override-value-is-not-type-checked.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -summary: An override value bypasses the scope check, cache lookup and creator, so it need not be an instance of the declared type and a REQUEST-scoped provider can be overridden and resolved from an APP container. ---- - -# An override value is not type-checked or scope-checked - -**Decision:** `container.override(provider, obj)` does not require `obj` to be an instance of the -provider's declared type, and does not require the resolving container to satisfy the provider's -declared scope. Both checks are structurally bypassed by where the override short-circuit sits in -`resolve_provider`, and neither is going to be added. - -## Context - -`resolve_provider` checks the override registry *before* delegating to the provider at all. If an -override is present, its value is returned directly — bypassing the scope check, the cache lookup, and -the creator invocation entirely. This is what makes overrides fast and simple to reason about -mechanically, but it also means none of the machinery that would normally validate a resolved value -ever runs on an override. - -Two concrete consequences fall out of that positioning: - -- **No type check.** The override object does not need to be an instance of the provider's declared - type at runtime — Python does not enforce it, and modern-di adds no check of its own. A caller who - passes an incompatible object gets no error at override time and no error at resolve time; they get - exactly the wrong object back, silently. -- **No scope check.** An overridden provider is resolved from whichever container `resolve_provider` - is called on — the original provider's declared scope is irrelevant, because the short-circuit fires - before `find_container` ever runs. In practice, a REQUEST-scoped provider can be overridden and - resolved from an APP-scoped container without raising `ScopeNotInitializedError`. - -Overrides also don't interact with the cache: if a singleton was already resolved before -`container.override(...)` was called, subsequent `resolve_provider` calls return the override value, -not the cached instance, and after `reset_override` the original cache entry (if any) is still present -and returned again. That's a related but separate behaviour from the two checks above — it follows -from the same "override fires first" positioning but isn't itself a missing check. - -## Decision & rationale - -**The scope bypass is a feature, not an oversight — it's often exactly what tests want.** A test that -overrides a REQUEST-scoped database provider with a stub, then resolves it from the APP-scoped root -container without building a child container down to REQUEST first, is a common and legitimate -pattern. Requiring the caller to build the full scope chain just to install a stub defeats much of the -point of having overrides at all. This is also why the mechanism lives ahead of the scope check -structurally, not behind it: putting it behind would mean paying the scope-chain-walk cost that -overrides exist partly to let callers skip. - -**The type bypass is an accepted cost of the same positioning, not a separately chosen feature.** -Adding a runtime `isinstance` check would mean either checking it against `bound_type` (which doesn't -handle generics, protocols, or duck-typed test doubles — the exact things overrides are commonly used -for) or against nothing meaningful. A check narrow enough to be correct would reject legitimate test -doubles; a check loose enough to accept them would catch almost nothing. So the responsibility is left -with the caller: "callers should pass a compatible object for type safety" is a documented expectation, -not an enforced one. - -**Consequence worth naming together:** these two bypasses compound. An override can supply an object -of the wrong type *and* be resolved from a container that could never have satisfied the original -provider's scope, and nothing in the resolve path will catch either. That's the accepted shape of the -mechanism, not a partially-fixed bug. - -## Revisit trigger - -A real report of a production bug caused by an override silently returning a type-incompatible object -— as opposed to a test-time convenience use, which is the mechanism working as designed. At that point -the design question is narrower than "add type checking to overrides" — it's whether an *opt-in* -strict-override mode is worth the API surface, given that the common case (test doubles, protocols) -is exactly what a blanket check would break. diff --git a/planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md b/planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md deleted file mode 100644 index 8300360..0000000 --- a/planning/decisions/2026-08-11-rebinding-an-in-use-provider-unsupported.md +++ /dev/null @@ -1,60 +0,0 @@ ---- -summary: Changing a ContextProvider's `scope` or `context_type` after a consumer's resolver has compiled against it is unsupported and only partially enforced — construct a second provider instead. ---- - -# Rebinding an in-use `ContextProvider` is unsupported - -**Decision:** Mutating a `ContextProvider`'s `scope` or `context_type` after something has already -resolved through it — compiling a consumer's resolver closure against it — is not a supported -operation. This is a contract, not a mechanism: enforcement is inconsistent by design, not a gap to -close. Construct a second provider instead of rebinding an existing one. - -## Context - -A `ContextProvider`'s `scope` and `context_type` are read once, when a consumer's resolver is -compiled, and folded directly into that closure. Nothing about either attribute touches a registry, -so nothing invalidates the memo when they change afterward. Changing either attribute post hoc applies -only to resolvers compiled *later* — silently, with no error and no signal that older, already-compiled -consumers are now working from a stale value. - -Whether that silent staleness is caught at all depends on which attribute and which route: - -- `scope` on a **registered** provider is enforced against group stamping by - `ProviderScopeFrozenError` — attempting to re-stamp a registered provider's scope raises. -- `scope` on a provider that was never registered — one passed only inline, e.g. - `Factory(creator, kwargs={"x": cp})` — is **not** enforced. `_registered` stays `False` for such a - provider, so a later `Group` can stamp its scope without error, silently. -- `context_type` is **not enforced on either route**. There is no equivalent guard for it at all. - -So three of the four (attribute, route) combinations either enforce nothing or enforce it -inconsistently with the fourth. That asymmetry was deliberate at the time each piece was built — -`ProviderScopeFrozenError` exists to protect group stamping, not attribute mutation in general — but -it means a caller cannot rely on an exception to catch a rebind. - -## Decision & rationale - -**Declare the whole surface unsupported rather than patch the three unguarded corners.** Closing all -three gaps would mean tracking "has anything resolved through this provider yet" as new mutable state -on every `ContextProvider`, checked on every attribute write, to protect an operation (rebinding scope -or context type on a live provider) that has no legitimate use case distinguishable from a bug: nobody -needs a provider that changes identity mid-flight, and existing resolved values are not migrated with -it in any case. - -The one exception path — a `ContextProvider` passed via `kwargs={...}` for a parameter with no parsed -`SignatureItem` (a `**kwargs` creator, or `skip_creator_parsing=True`) — still goes through -direct-resolve semantics and raises `ContextValueNotSetError` when unset, which is unrelated to this -decision; it's about absence, not about rebinding an already-resolved provider. - -**Accepted cost:** a caller who does rebind a never-registered provider's `scope`, or either route's -`context_type`, gets no error — just resolvers that silently disagree about what type or scope the -provider has, split along whichever side of the compile boundary they landed on. This is disclosed -rather than fixed because fixing it costs a mutable per-provider tracking flag for a mistake that has -no other repro path in the test suite or issue history. - -## Revisit trigger - -A real bug report traces back to one of the three unguarded corners — most plausibly the -never-registered-provider `scope` gap, since that one silently *succeeds* where the registered case -raises. At that point the fix is a symmetric guard (extend `ProviderScopeFrozenError`-style enforcement -to the unregistered route, and add an equivalent for `context_type`) rather than continuing to disclaim -it. diff --git a/planning/decisions/2026-08-11-transient-teardown-order-unspecified.md b/planning/decisions/2026-08-11-transient-teardown-order-unspecified.md deleted file mode 100644 index 847815f..0000000 --- a/planning/decisions/2026-08-11-transient-teardown-order-unspecified.md +++ /dev/null @@ -1,53 +0,0 @@ ---- -summary: The order in which a resolver collects transient (uncached) dependencies is not part of the contract, even though the arity ladder happens to preserve it today. ---- - -# Transient teardown order is unspecified - -**Decision:** The order in which transient dependencies are collected during resolution is not -part of modern-di's contract. A future change to the resolver's shape may alter it without that -being a breaking change. - -## Context - -An uncached (transient) dependency that its consumer's creator uses and drops — never retains — is -freed by CPython's ordinary refcounting the moment the resolver's local reference to it goes out of -scope. modern-di manages no finalizer for such an object: `CacheSettings(finalizer=)` only applies to -cached providers, and `close_sync`/`close_async` only tear down what a container owns. So the *only* -place this order is observable at all is the drop order of objects a creator never kept a reference -to — and that order falls out of however the resolver's compiled closure happens to hold its locals, -not from any stated rule. - -The question came up concretely when the positional fast-path's arity ladder landed: arity 0 and 1 -compile to a closure that names its argument and calls the creator directly; arity 2+ still builds a -list and star-calls it. Naming vs. list-building are different mechanisms for holding intermediate -values, so it was worth checking whether they drop objects in a different order. - -## Decision & rationale - -**Nothing observable changed when the arity ladder landed, and the reason generalizes.** The ladder -caps at arity 1, so it never holds more than one named local at a time — there is no order to alter -between one item and itself. Measured directly, main vs. ladder, on CPython 3.10 and 3.14, across -arities 1 through 3: the collection order is identical in every case tested. - -The rule is stated in advance of the case that would actually test it. A rung added at arity 2+ would -release named locals with the frame teardown rather than through the star-call's intermediate list — -and on CPython below 3.12, frame-local release order and list-teardown order are not the same thing. -Such a rung would be a legitimate performance change, not a breaking one, because the order was never -promised. Declaring the contract now means that future work doesn't have to treat "does this change -teardown order" as a correctness question — only a "did anyone tell users this order was reliable" -question, and the answer is no. - -**Accepted cost:** a user who has silently relied on today's incidental order (for example, using -transient side effects on drop as a poor man's ordering signal) gets no deprecation warning if a -future resolver shape changes it. This is deliberate — retaining that order would pin the resolver's -internal representation of arity 2+ closures indefinitely, for a guarantee nobody asked for and the -library never advertised. - -## Revisit trigger - -A rung is added to the arity ladder at arity 2 or higher (or the star-call path is otherwise -restructured) and CPython's frame-local teardown order diverges from list-teardown order on a -supported version. At that point, re-measure whether the divergence is observable by any real -finalizer-adjacent use case before deciding whether it's worth stabilizing rather than continuing to -disclaim it. diff --git a/planning/deferred/2026-07-29-upstream-lean-convention.md b/planning/deferred/2026-07-29-upstream-lean-convention.md index f19de65..3e0a2a3 100644 --- a/planning/deferred/2026-07-29-upstream-lean-convention.md +++ b/planning/deferred/2026-07-29-upstream-lean-convention.md @@ -77,8 +77,7 @@ Three things should inform whichever route is taken, and none has evidence yet: ## Update: the promotion target this item relied on is gone -`architecture/` was deleted 2026-08-11 -([`2026-08-11-drop-architecture-directory.md`](../decisions/2026-08-11-drop-architecture-directory.md)): +`architecture/` was deleted 2026-08-11: 11 capability pages routed instead to code, a named `INVARIANT:`-marked test, `decisions/`, or `docs/`, per the four-homes admission check now in `planning/README.md`. The premise this item's central finding rested on — From 5f52566c1c3c5206ead1282b891586010551673e Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 21:36:45 +0300 Subject: [PATCH 17/18] docs(decisions): drop the free-threaded Beta record The Beta support level and the thread-safety boundary it stated now live at user altitude in docs/introduction/design-decisions.md; the four references repoint there. planning/decisions/ gains nothing from this branch. --- ...-07-18-warm-singleton-memo-swap-dropped.md | 2 +- .../2026-07-19-child-lazy-alloc-declined.md | 4 +- ...026-08-11-free-threaded-beta-not-stable.md | 66 ------------------- .../2026-07-19-free-threaded-throughput.md | 4 +- tests/test_free_threading.py | 2 +- 5 files changed, 6 insertions(+), 72 deletions(-) delete mode 100644 planning/decisions/2026-08-11-free-threaded-beta-not-stable.md 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 index 93a9eea..71a5b52 100644 --- a/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md +++ b/planning/decisions/2026-07-18-warm-singleton-memo-swap-dropped.md @@ -60,7 +60,7 @@ Two things from the attempt were kept: 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 - [`2026-08-11-free-threaded-beta-not-stable.md`](2026-08-11-free-threaded-beta-not-stable.md). + [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 diff --git a/planning/decisions/2026-07-19-child-lazy-alloc-declined.md b/planning/decisions/2026-07-19-child-lazy-alloc-declined.md index f041688..9f52151 100644 --- a/planning/decisions/2026-07-19-child-lazy-alloc-declined.md +++ b/planning/decisions/2026-07-19-child-lazy-alloc-declined.md @@ -39,9 +39,9 @@ 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 freshly + CAS-style publish, a new concurrency-correctness surface against the documented Beta contract - ([`2026-08-11-free-threaded-beta-not-stable.md`](2026-08-11-free-threaded-beta-not-stable.md)). + ([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 diff --git a/planning/decisions/2026-08-11-free-threaded-beta-not-stable.md b/planning/decisions/2026-08-11-free-threaded-beta-not-stable.md deleted file mode 100644 index e5d98c3..0000000 --- a/planning/decisions/2026-08-11-free-threaded-beta-not-stable.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -summary: Free-threaded (PEP 703) support is Beta, not Stable, because it relies on object-publication ordering that CPython's implementation provides but does not formally guarantee. ---- - -# Free-threaded support is Beta, not Stable - -**Decision:** modern-di's free-threaded (PEP 703) support is labeled Beta. It stays Beta rather than -graduating to Stable until the one guarantee it currently borrows from CPython's implementation -behaviour, rather than from CPython's spec, is either formalized upstream or removed from modern-di's -own reliance. - -## Context - -Free-threaded CPython makes single built-in-container operations (`dict.setdefault`, `dict[k] = v`, -`dict.get`, `list.append`) internally atomic — no single such operation can corrupt the structure. -modern-di's concurrency design leans on that: every compound check-then-act sequence over shared state -in the resolve path is either idempotent (a rebuild-if-stale race just produces a duplicate, discarded -build) or already runs under the container's own lock. Registry *mutation* (`register`, -`add_providers`, removal) is guarded by the registry's own lock; the cycle-guard `_building` set is -thread-local, so a same-thread cycle is still caught while a concurrent first-resolve of the same -provider on another thread just compiles it independently. - -That much is sound on the guarantees CPython actually documents. But one more thing is needed for -correctness: when one thread publishes a newly-built object (a compiled resolver, a cached instance) -by storing a reference where another thread will read it, the reading thread must see that object's -fully-initialized fields, not a partially-constructed one. That's object-publication ordering, and -CPython's language spec does not formally guarantee it — CPython publishes no memory model. - -## Decision & rationale - -**The gap is between implementation behaviour and spec guarantee, and that gap is exactly what Beta -means here.** In the current CPython implementation, publication that happens through a container's -internal critical section does provide the necessary ordering — so today's behaviour is correct on -every free-threaded build tested. But "correct because of how the interpreter happens to be built" -and "correct because the language promises it" are different claims, and only the second one is safe -to call Stable. A future CPython release is free to change unspecified implementation behaviour -without that being a compatibility break by CPython's own rules, even though it would be one for -modern-di's free-threaded users. - -Two adjacent things are explicitly *not* what keeps this at Beta, and are worth separating out because -they get conflated with the ordering question: - -- **Thread-safety itself is not in question.** Concurrent resolve is thread-safe under the compound-op - analysis above; that part doesn't move. -- **Throughput is a separate, already-tracked concern.** Concurrent resolve is thread-safe but its - throughput does not scale with thread count on a free-threaded build — diagnosed as atomic - refcount contention on shared hot-path objects (the returned singleton value, then the shared - provider objects, then the compiled-resolver closures' captured cells), not the per-container lock. - That's a performance ceiling CPython itself would have to lift (deferred reference counting - expanding to ordinary instances and cells), not a correctness gap, and it's tracked separately in - [`2026-07-19-free-threaded-throughput.md`](../deferred/2026-07-19-free-threaded-throughput.md). - -**Caveats that hold regardless of Beta/Stable status:** configure and close are single-threaded edges. -`override`/`reset_override` and `set_context` mutate shared state without a lock — racing them against -a live `resolve()` is inherently unordered, GIL or not. `close`/`open` are the same: tear a container -down only after concurrent resolution has stopped. These are usage contracts, not bugs, and staying at -Beta doesn't change them. - -## Revisit trigger - -CPython documents a formal memory model for free-threaded builds that covers the publication ordering -this relies on — at that point the reliance becomes a spec guarantee rather than an implementation -behaviour, and Beta can graduate to Stable. Absent that, a CPython release that changes unspecified -publication-ordering behaviour and breaks modern-di's free-threaded tests would confirm the gap is real -rather than theoretical, and is itself grounds to keep the label at Beta indefinitely rather than -guessing at a graduation date. diff --git a/planning/deferred/2026-07-19-free-threaded-throughput.md b/planning/deferred/2026-07-19-free-threaded-throughput.md index 6356a0b..385f7c0 100644 --- a/planning/deferred/2026-07-19-free-threaded-throughput.md +++ b/planning/deferred/2026-07-19-free-threaded-throughput.md @@ -31,8 +31,8 @@ shared object — inherent), then the shared **provider** objects (distinct-cont closures and their captured cells (every `LOAD_DEREF` of a shared capture increfs it). The per-container lock is **not** the bottleneck. First-resolve does additionally serialize on the double-checked creation lock — see -[`2026-08-11-free-threaded-beta-not-stable.md`](../decisions/2026-08-11-free-threaded-beta-not-stable.md), -which states the supported lifecycle contract and the Beta status of free-threaded support. +[design decisions](../../docs/introduction/design-decisions.md#the-thread-safety-boundary), +which states the supported thread-safety boundary and the Beta status of free-threaded support. A throwaway **immortalization experiment** (ctypes set of `ob_ref_local` on the free-threaded build, offset verified against a known-immortal object) confirmed diff --git a/tests/test_free_threading.py b/tests/test_free_threading.py index 93608d7..3b6b2ae 100644 --- a/tests/test_free_threading.py +++ b/tests/test_free_threading.py @@ -5,7 +5,7 @@ and the setdefault-shared CacheItem; on a 3.14t build it runs those paths GIL-free. The free-threaded *interpreter* assertion lives in CI (_checks.yml), not here, to keep this suite version-agnostic and 100%-line-covered on every build. -See planning/decisions/2026-08-11-free-threaded-beta-not-stable.md. +See docs/introduction/design-decisions.md for the supported thread-safety boundary. """ import threading From 05a825caad7fc2cca57a344fe7bd201e043e8527 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 13 Aug 2026 21:39:31 +0300 Subject: [PATCH 18/18] docs(planning): drop the negative-contract route from the admission check Nothing on this branch exercised it, and decisions/ took no records. decisions/ now holds rejected alternatives only, stated the same way in the four-homes table, the admission check, and the PR template. --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++-- planning/README.md | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index f4a1d1a..7507fe2 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -37,8 +37,8 @@ effect. State the numbers, not "benchmarked". what breaks it. Do **not** write prose about mechanism — there is no page for it. See [`planning/README.md`](../planning/README.md#where-a-fact-goes). - [ ] **Adding a fact anywhere?** Run the admission check: derivable from - `modern_di/` → don't write it; enforceable → a test; deliberately not - guaranteed → `planning/decisions/`; a user needs it → `docs/`. + `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. diff --git a/planning/README.md b/planning/README.md index b01b72f..1860b51 100644 --- a/planning/README.md +++ b/planning/README.md @@ -40,14 +40,13 @@ 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, or a **negative contract** — something deliberately unspecified | +| `decisions/` | a rejected alternative, with the reasoning that would otherwise be re-litigated | | `docs/` | anything a user needs | Before writing a line anywhere: > Can an agent get this by reading `modern_di/`? → **don't write it.** > Would a wrong change here fail a test? → it belongs **in the test**, not in prose. -> Is it something we deliberately do *not* guarantee? → **`decisions/`**. > Does a user need it? → **`docs/`**. > Otherwise it does not get written.