Skip to content

fix(wiring): honor the creator default for a ContextProvider passed via kwargs - #340

Merged
lesnik512 merged 1 commit into
mainfrom
fix/context-provider-kwargs-wiring
Jul 17, 2026
Merged

lesnik512 merged 1 commit into
mainfrom
fix/context-provider-kwargs-wiring

Conversation

@lesnik512

Copy link
Copy Markdown
Member

The bug

The same creator parameter behaved differently depending on how its ContextProvider reached it:

wired by TYPE            -> DEFAULT-APPLIED   warned=[]
wired via kwargs={...}   -> got None          warned=['ContextValueNoneWarning']

Wired by type, an unset context value consults absent_disposition and the parameter's default applies. Passed explicitly via kwargs={"request": ctx_provider}, WiringPlan.build's static overlay tested only isinstance(value, AbstractProvider), so the provider landed in provider_kwargs and resolved through ContextProvider.resolve — discarding the declared default, injecting None, and emitting a spurious ContextValueNoneWarning.

Silently dropping a declared default is wrong on its own. It also mis-signals the 3.0 upgrade: once the warning becomes ContextValueNotSetError, this path would raise for a parameter that has a perfectly good default.

The fix

Bucket a ContextProvider with a parsed SignatureItem into context_kwargs — the same pair the by-type path builds — so _resolve_context_value honors default-then-nullable-then-required identically. How the provider reaches the parameter is now a declaration detail, not a behavior switch.

The deliberate non-fix

The item is not None guard is intentional. With a **kwargs creator or skip_creator_parsing=True there is no parsed signature, so there is no default to honor and nothing to fix; those keep today's direct-resolve semantics. Both alternatives were considered and rejected:

  • required → raises where 2.x returns None, a break mid-2.x
  • nullable → silently swallows the unset-context signal 3.0 turns into ContextValueNotSetError

Pinned by test_kwargs_context_provider_without_parsed_signature_keeps_direct_resolve.

Notes

pure_provider and edges derive from the buckets, so a provider moving between them is accounted for automatically and the validation graph still sees the same edge.

The third branch pushed build to complexity 11 (C901). The plan is memoized and cold, so there's no frame-count constraint to justify a noqa the way resolver_compiler.py's hot closures have — the overlay loop is extracted to _apply_overlay instead, which is also where the guard is documented.

Verification

  • Failing test first: AssertionError: assert 'default-applied' == 'got None'
  • just test-ci427 passed, 100% coverage held
  • just lint — clean (ruff + ty)
  • modern-di-fastapi suite green against the patched build (8 passed) — it registers the Request/WebSocket context providers this touches
  • 3 of the 5 new tests are guards pinning behavior that must not change (present value, override, no-parsed-signature)

Promoted into architecture/providers.md, which documented ContextProvider wiring but never covered the kwargs={...} route.

Change: planning/changes/2026-07-17.04-context-provider-kwargs-wiring.md

🤖 Generated with Claude Code

…ia kwargs

The same creator parameter behaved differently depending on how its
ContextProvider reached it. Wired by type, an unset context value consults
absent_disposition and the parameter's default applies. Passed explicitly via
kwargs={"request": ctx_provider}, WiringPlan.build's static overlay tested only
isinstance(value, AbstractProvider), so it landed in provider_kwargs and
resolved through ContextProvider.resolve — discarding the default, injecting
None, and emitting ContextValueNoneWarning:

    wired by TYPE            -> DEFAULT-APPLIED   warned=[]
    wired via kwargs={...}   -> got None          warned=['ContextValueNoneWarning']

Silently dropping a declared default is wrong on its own, and it mis-signals the
3.0 upgrade: once the warning becomes ContextValueNotSetError, this path would
raise for a parameter that has a perfectly good default.

Bucket a ContextProvider with a parsed SignatureItem into context_kwargs, the
same pair the by-type path builds, so _resolve_context_value honors
default-then-nullable-then-required identically. The item is not None guard is
deliberate: with a **kwargs creator or skip_creator_parsing=True there is no
parsed signature and so no default to honor, and those keep today's
direct-resolve semantics. Routing them as required would raise where 2.x returns
None; as nullable would drop the unset-context signal 3.0 depends on.

pure_provider and edges derive from the buckets, so a provider moving between
them is accounted for and the validation graph sees the same edge. The third
branch pushed build to complexity 11, so the overlay loop is extracted to
_apply_overlay rather than suppressed — the plan is memoized and cold, with no
frame-count constraint to justify a noqa.

427 passed, 100% coverage held; modern-di-fastapi green against the patched
build.

Change: planning/changes/2026-07-17.04-context-provider-kwargs-wiring.md

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Details
Benchmark suite Current: c1c79c6 Previous: 068d199 Ratio
benchmarks/test_guard_lifecycle.py::test_g6_build_child_container 408695.5731001136 iter/sec (stddev: 0.00007362267104524295) 392912.2415103384 iter/sec (stddev: 0.00006730389570553561) 0.96
benchmarks/test_guard_lifecycle.py::test_g7_request_lifecycle 62961.31301525725 iter/sec (stddev: 0.000014094344573509744) 72770.75062631334 iter/sec (stddev: 0.00001329131413106073) 1.16
benchmarks/test_guard_resolve.py::test_g1_transient_resolve 1175348.1122435385 iter/sec (stddev: 5.95367480766129e-7) 1193437.9111739814 iter/sec (stddev: 5.42904861477518e-7) 1.02
benchmarks/test_guard_resolve.py::test_g2_cached_resolve 2227825.3424833533 iter/sec (stddev: 6.934588645809727e-8) 2291716.318481566 iter/sec (stddev: 7.102135901315073e-8) 1.03
benchmarks/test_guard_resolve.py::test_g3_deep_chain 501958.4192541486 iter/sec (stddev: 7.102264432709567e-7) 499024.3996069766 iter/sec (stddev: 6.913079935933826e-7) 0.99
benchmarks/test_guard_resolve.py::test_g4_wide_resolve 316082.49966120324 iter/sec (stddev: 0.0000010265210954720752) 306267.83789653325 iter/sec (stddev: 7.307822562581718e-7) 0.97
benchmarks/test_guard_resolve.py::test_g5_cross_scope 1020075.1881085082 iter/sec (stddev: 4.5962166339105566e-7) 1062612.5233875404 iter/sec (stddev: 5.142772617906347e-7) 1.04

This comment was automatically generated by workflow using github-action-benchmark.

@lesnik512

Copy link
Copy Markdown
Member Author

Confirmed by the reporter: their Factory does pass request via kwargs={...}, so this is the real cause of the ContextValueNoneWarning in their suite — not the by-type optional param it first looked like.

Worth linking for review: this is the same seam as #320. That fix established the principle in WiringPlan.edges:

Providers supplied via kwargs={...} are edges like any other: only the declaration differs, not the dependency.

#320 enforced it for validationedges derives from the buckets, so the validated graph cannot drift from the resolved one. The static overlay was still violating the same principle for context resolution: it bucketed a ContextProvider as a plain provider, so the declaration route silently changed runtime behavior (default discarded, None injected, spurious warning).

So this isn't a new rule, it's the resolve-side completion of the one #320 already articulated. Two bugs in this overlay in four days suggests the kwargs={...} route is worth watching as a class, not just patching case by case.

@lesnik512
lesnik512 merged commit b5c8208 into main Jul 17, 2026
8 checks passed
@lesnik512
lesnik512 deleted the fix/context-provider-kwargs-wiring branch July 17, 2026 14:59
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