Skip to content

fix: validate() traverses providers supplied via kwargs= - #320

Merged
lesnik512 merged 1 commit into
mainfrom
fix/unify-wiring-edge-set
Jul 14, 2026
Merged

lesnik512 merged 1 commit into
mainfrom
fix/unify-wiring-edge-set

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Closes a hole where validate() reported a clean graph that then failed at resolve time.

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.

Both consequences reproduced against fe090f4:

G.b = Factory(scope=Scope.APP, creator=B, kwargs={"a": G.a})
G.a = Factory(scope=Scope.APP, creator=A, kwargs={"b": G.b})

c.validate()   # PASSES -- reports no cycle
c.resolve(A)   # RecursionError: maximum recursion depth exceeded

A bare RecursionError is the exact failure CircularDependencyError exists to prevent, and the runtime cycle guard could not save it: find_cycle_from walks the same blind edge set. A scope inversion routed through kwargs= behaved the same way, passing validate() and dying at resolve with ScopeNotInitializedError.

What changed

Delete the dependencies field. WiringPlan.edges is now derived from provider_kwargs + context_kwargs — the same buckets resolve() reads. validate() traverses exactly what resolve() follows, so the two graphs cannot drift apart again; the fix falls out of the derivation rather than being a second write someone has to remember to make.

What differs between the two ways of declaring a dependency is how the edge is declared, never whether it exists.

The runtime keeps its partition — provider_kwargs and context_kwargs take different resolution paths, and context resolution needs the SignatureItem carried alongside. The partition was load-bearing; the third dict was not.

Design and rejected alternatives: planning/changes/2026-07-14.06-unify-wiring-edge-set.md.

Upgrade note

Ships as a bug fix in a 2.x minor. Any config this newly reports was already broken at runtime (verified: RecursionError / ScopeNotInitializedError), so there are no false positives — but a user with a dormant broken config they never resolve will see a green validate() go red. Worth calling out in the release notes.

Not in scope

validate() builds each Factory's plan twice (measured: 120 WiringPlan.build calls for a 60-provider chain). Every way of fixing it either makes validate() touch the cache (violating get_dependencies' documented "no cache touch" contract), adds validate-only lifecycle state to Container, breaks dependency_graph.py's no-concrete-providers layering rule, or changes AbstractProvider's interface — the last of which is a provider-seam change deserving its own design. Left in place deliberately; the derivation adds ~4.9% to a cold boot-time validate() and nothing to resolve().

Also

Testing

TDD, both failing tests written first from the reproductions above.

  • test_walk_emits_cycle_closed_through_kwargs_overlay — cycle through the overlay is now a Cycle event.
  • test_validate_raises_on_inverted_scope_dependency_supplied_via_kwargs — scope inversion through the overlay now raises InvalidScopeDependencyError from validate().
  • test_wiring_plan_dependencies_excludes_static_supplied_providers pinned the bug as intended behavior; inverted to ..._edges_include_....
  • test_factory_self_reference* stay green — a type-matched self-reference must keep falling through to the creator default. (find_dep_provider excludes the owner; the kwargs overlay needs no such exclusion, since a provider cannot be passed to its own constructor.)

just test-ci: 381 passed, 100% coverage. just lint-ci: clean.

🤖 Generated with Claude Code

`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>
@lesnik512
lesnik512 merged commit f2204a2 into main Jul 14, 2026
7 checks passed
@lesnik512
lesnik512 deleted the fix/unify-wiring-edge-set branch July 14, 2026 19:29
lesnik512 added a commit that referenced this pull request Jul 14, 2026
…ion (#322)

PR #320 deleted `WiringPlan.dependencies` but left resolution.md describing it,
and landed the "One edge set" note un-indented mid-list, terminating the numbered
list so the absent-parameter item restarted at 1.

Name `edges` where the deleted field was named, and re-place the note after the
list ends.
lesnik512 added a commit that referenced this pull request Jul 17, 2026
Cover the 2.28.0->2.29.0 backlog: the single-path compiled resolver (#334)
and its measured perf, the wiring-plan memoization (#326), the three
correctness fixes (#340 ContextProvider-via-kwargs default, #321 NoneType
default/nullability, #320 validate traverses kwargs=), and the closed
provider set under an explicit breaking-change heading (per
planning/decisions/2026-07-17-custom-providers-retracted.md, the release
notes carry that warning, not the docs).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant