Argument suggestions, context falsy/None fix, docs lead with with#184
Merged
lesnik512 merged 2 commits intoMay 30, 2026
Merged
Conversation
…one bug, switch docs to context managers
- ArgumentResolutionError now appends the same "Did you mean:" hints
(subclass/baseclass/typo) that ProviderNotRegisteredError emits, so
factory-argument resolution failures get the same UX.
- ContextRegistry.find_context now returns a sentinel (types.UNSET) when
a key is absent instead of conflating absence with falsy values. The
factory disambiguates via ContextProvider._find_context_value, so
context={K: None}, False, 0, "", [], {} are all valid context values.
ContextProvider.resolve keeps its T | None public contract.
- Docs lead with `with` / `async with`. The 2.x migration guide's claim
that context managers were removed is corrected — they are the
recommended form; close_sync/close_async remain for manual control.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lesnik512
changed the title
with
5 tasks
lesnik512
added a commit
that referenced
this pull request
May 30, 2026
… value CacheItem.cache used None both as the 'not cached' sentinel and as a potentially-valid cached value. Two consequences: 1. A creator that returns None was treated as never-cached: it would be re-created on every resolve and its finalizer would never run. 2. The `if self.cache and ...` guard in close_sync/close_async skipped finalizers for any cached-but-falsy resource (empty dict, 0, False, empty string). Same anti-pattern PR #184 fixed in ContextRegistry. Switched to types.UNSET as the 'not cached' sentinel. Now: - A None-returning creator caches once, returns the cached None on subsequent resolves, and runs its finalizer at close time. - Falsy-but-set cached values (empty dict/list/0/False) get their finalizers called too. Touches the four sites that distinguished cached from not-cached: cache_registry.py (_clear, close_async, close_sync, cached_count) and factory.py (the two cached-value short-circuit reads in resolve). Three new regression tests cover: falsy-sync, falsy-async, and the None-returning creator path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Three Tier-1 fixes from
plan.md(items #2, #3, #5) — localized, no public API breakage.#2 —
ArgumentResolutionErrorruns the suggestion engineThe rich "Did you mean:" hints (subclass / baseclass / typo) previously only fired on
ProviderNotRegisteredError. The most common debugging case — "why won't argumentdbresolve inside this factory?" — got a bare message. NowArgumentResolutionErroraccepts an optionalsuggestions=and appends them the same way. The factory passesproviders_registry.build_suggestions(arg_type)at the no-provider-found raise site. The ContextProvider-missing-context raise site is unchanged (suggestions about unrelated types would mislead there).#3 —
ContextProviderno longer conflates "missing" with "falsy" or "None"ContextRegistry.find_contextusedif context_type and (context := self.context.get(...))which silently dropped legitimateFalse,0,"",[],{}values. Fixed viainmembership check +types.UNSETsentinel return. The factory's missing-context detection now usesContextProvider._find_context_value(container) is types.UNSETinstead ofprovider.resolve(container) is None, socontext={Key: None}is also a valid value.ContextProvider.resolve()keeps itsT | Nonepublic contract (translatesUNSET → None).#5 — Docs lead with
with/async withContainer.__enter__/__exit__/__aenter__/__aexit__already exist (container.py:156–166), but the quickstart and integration examples led withtry/finally+ manualclose_sync()/close_async(). Worse, the 2.x migration guide falsely claimed context managers had been removed. Switched all examples to context managers and corrected the migration guide.Test plan
just lint— clean (ruff format + check, ty)just test— 122 passed, 100% coveragetests/test_suggestions.py: subclass / baseclass / typo / no-match suggestions surface throughArgumentResolutionErrortests/providers/test_context_provider.py: parametrized falsy values round-trip, factory acceptsbool=Falsevia context, factory acceptsOptional[X]resolving toNonevia context🤖 Generated with Claude Code