Unify provider-graph traversal behind a DependencyGraph module - #308
Merged
Merged
Conversation
…tion Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Locks today's behavior of Container.validate() (cycle detection shape, collecting multiple error kinds in one pass) and the runtime resolve cycle guard, against the public API only, so a later DependencyGraph refactor can be checked for parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a `validated_version` marker (int | None, default None) reset to None on every mutation (register, add_providers, _remove_providers). Later work will let validate() set it on success and short-circuit re-validation when the registry hasn't changed since. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lets a later DependencyGraph module follow alias chains without knowing about the concrete Alias type. Alias overrides it to return its source, or None when the source is unregistered (dangling). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add modern_di/dependency_graph.py with the NodeEntered/Edge/Cycle/ DependenciesError event NamedTuples, the Event union, and DependencyGraph.walk: a single explicit-stack (non-recursive) pre-order DFS of the static provider graph. Event order mirrors Container.validate's recursive _visit exactly (Edge before a possible Cycle; one NodeEntered per node; visiting/visited shared across roots), so a later task can reproduce validate() output byte-for-byte and reuse the walk inside a RecursionError handler near the stack limit. Imports only AbstractProvider and exceptions at runtime; Container stays behind TYPE_CHECKING to avoid an import cycle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
find_cycle_from reuses the iterative walk to locate the first cycle reachable from a provider, staying safe near the stack limit for a later RecursionError handler. terminal_scope follows redirect_target hops to the terminal provider's scope, guarding against alias cycles via a seen-id set. Both will replace the hand-rolled chain walk in Alias.effective_scope in a later task. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite Container.validate() as a fold over the shared DependencyGraph.walk event stream instead of a bespoke recursive DFS, and short-circuit when the providers registry has already been validated at its current version. Remove AbstractProvider.effective_scope and Alias.effective_scope; the scope-ordering check now uses DependencyGraph.terminal_scope on both sides of each edge. Event emission order (NodeEntered -> DependenciesError -> Edge -> Cycle in registry order) reproduces the old _visit output byte-for-byte, so the characterization/parity suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…find_reachable_cycle Rewire resolve_provider's RecursionError handler to reuse the shared DependencyGraph.find_cycle_from walker instead of a second explicit-stack DFS, and short-circuit (re-raise untouched) when the registry is already validated. Add dependency_graph.build_cycle_error so validate() and the guard construct CircularDependencyError from one place, removing the last verbatim duplication. Delete the now-redundant _find_reachable_cycle and _convert_recursion_error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both `_explode` monkeypatch helpers intentionally never execute their body (they exist to fail the test loudly if the patched method IS called). Mark them `# pragma: no cover` per the convention already used in test_runtime_cycle_guard.py, so line coverage doesn't count dead-by-design guard bodies against the 100% gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CPython suspends the coverage tracer for a few frames while unwinding a RecursionError, so a re-raise written inline in the except block is never recorded as covered. Route the handling back through a separate module-level function so the tracer gets a fresh call boundary to re-arm on, restoring the 100% coverage gate without changing any resolve behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the recursive-DFS/effective_scope narration with the shipped DependencyGraph model: validate() as a fold over one iterative walk, the runtime guard sharing that walk via find_cycle_from, terminal_scope following the redirect_target node hook, and the registry-level validated_version short-circuit. effective_scope no longer exists in code and is now absent from all architecture and docs prose. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename test_effective_scope_handles_mutual_alias_cycle to test_terminal_scope_handles_mutual_alias_cycle so it's grep-discoverable under the current terminal_scope API, and add a one-line comment noting the Event match in Container.validate() is exhaustive by construction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lesnik512
added a commit
that referenced
this pull request
Jul 14, 2026
Candidate 3 (make SignatureItem opaque) is superseded by the graph-traversal unification (#308); Candidate 5 (bless a provider-introspection seam for grpc) is declined as a hypothetical seam (grpc is the sole consumer). Both record load-bearing reasons so a future architecture review does not re-suggest them. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lesnik512
added a commit
that referenced
this pull request
Jul 14, 2026
`WiringPlan` built a third dict, `dependencies`, alongside the buckets the
runtime resolves from. The declaration-time `kwargs={...}` overlay wrote to
`provider_kwargs` but not to `dependencies`, so a provider supplied that way
was a real runtime edge that validation could not see.
Consequences, both reproduced: a cycle routed through `kwargs=` passed
`validate()` and then surfaced as a bare `RecursionError` (the runtime guard's
`find_cycle_from` saw no edge either), and a scope inversion routed through
`kwargs=` passed `validate()` and failed at resolve time with
`ScopeNotInitializedError`.
Delete the `dependencies` field; derive `WiringPlan.edges` from
`provider_kwargs` + `context_kwargs` instead. validate() now traverses exactly
what resolve() follows, so the two graphs cannot drift apart again. What differs
between the two ways of declaring a dependency is how the edge is declared,
never whether it exists.
Newly reported configs were already broken at runtime, so there are no false
positives — but a dormant broken config can turn a green validate() red.
Also: rewrite dependency_graph's module docstring, which described the event
order against `Container.validate`'s `_visit` (deleted in #308), and rename
test_dependency_graph_parity.py -> _contract.py; it tests no parity, and there
is no second implementation left to be at parity with.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Aug 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unifies the four independent walks of the provider dependency graph behind a single deep module. Full rationale, design, and trade-offs live in the change file — this PR does not restate them:
planning/changes/2026-07-12.01-dependency-graph-module.mdplanning/decisions/2026-07-12-unify-graph-traversal.mdWhat changed
modern_di/dependency_graph.py— one iterative, event-streamDependencyGraph.walk(NodeEntered/Edge/Cycle/DependenciesError), plusfind_cycle_fromandterminal_scope.validate()is now a fold overwalk; the runtimeRecursionErrorguard routes throughfind_cycle_from;_find_reachable_cycle/_convert_recursion_errordeleted.Alias.effective_scopereplaced by a genericredirect_targetnode hook +DependencyGraph.terminal_scope, keeping the module Alias-agnostic.validated_versionstamp: a validated (acyclic) graph skips the runtime cycle walk, and a repeatvalidate()is free. Un-stamped on every registry mutation.Heads-up for review
This reverses the "deliberate duplication" stance documented in
architecture/validation.md— on the extraction axis only (the two cycle policies stay distinct). The reasoning and revisit triggers are in the decision record.architecture/validation.md/containers.md/providers.mdwere promoted in this PR to match.Verification
just test-ci— 358 passed, 100% line coverage.just lint-ciandjust check-planning— clean.validate()output is byte-identical to before (characterization tests pin cycle-path ordering, error kinds, and order).🤖 Generated with Claude Code