Pass a Config to the target unresolved when the parameter is annotated cfn.Config - #39
Conversation
…d Config (#38) instantiate() resolves every Config in args/kwargs before calling the target. That is the right default, but it makes one shape impossible: a target that needs the config itself, because it builds it later, more than once, or with overrides it only learns at runtime — a server applying per-connection overrides, for instance. The workaround was a module-level dict of configs with a string key threaded through configuronic, which gives up dotted overrides, --help and any checking of the selector. A parameter annotated `cfn.Config` now receives the stored config instead of its instantiation. The declaration lives in the target's signature rather than at the call site, because wanting a config rather than an object is a property of the target, not of each call: callers keep passing ordinary values, so `.override()` (including dotted keys reaching into the config), `--help` and `get_required_args` all keep working on it. `Config | None`, `Optional[Config]`, `Annotated[Config, ...]` and string annotations are recognised too. String annotations are resolved one parameter at a time in the target's own namespace, so an unresolvable forward reference elsewhere in the signature cannot hide a good annotation here, and an unresolvable one is treated as "not a Config annotation" rather than as an error. A non-config value on such a parameter is passed through untouched; configs nested in containers (`list[Config]`) still resolve as before. The per-target declaration is cached, since reading it costs an order of magnitude more than instantiating a small config and it depends on the target alone. Closes #38. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90397a2836
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two gaps in how a `Config` annotation is recognised, both found in review: Quoting an annotation in a module that also postpones evaluation stores the *source text* of the quoted expression, so `pipeline: 'cfn.Config'` arrives as "'cfn.Config'" and evaluates to a string rather than the class. Resolve that string too, as get_type_hints does; each round strips one layer of quoting, and an equality check stops a value that would resolve to itself. `inspect.signature` reports the wrapped function's parameters for a functools.partial or a functools.wraps wrapper, but the annotations were written in that function's module, not the wrapper's. Follow the same hops when picking the namespace to resolve them in — partial.func, __wrapped__, and __init__ for a class — guarding against a circular __wrapped__ chain, which inspect.signature accepts when the target also carries a __signature__. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a612e1531a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more false negatives, both found in review: A string annotation was only evaluated when its source text mentioned `Config`, so an alias — `from configuronic import Config as C`, annotation stored as `'C'` — was never resolved and the parameter stayed eager. The spelling filter was there to keep instantiate() from evaluating unrelated annotations, which the per-target cache now handles: evaluate the annotation and decide on the object it resolves to. For a class, `inspect.signature` reports the parameters of a metaclass `__call__`, of `__new__`, or of `__init__`, by rules that vary across Python versions, so pinning the namespace to `__init__` resolved nothing at all for a class built by `__new__` (whose `__init__` is `object.__init__`) or by a metaclass. Offer all three namespaces, most specific first, and take the first that resolves the annotation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d3aedec29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more, both found in review: `inspect.signature` stops unwrapping at a callable that declares its own `__signature__` — the parameters, and the names their annotations use, then belong to the wrapper rather than to what it wraps. The namespace walk followed `__wrapped__` past it and resolved in the wrong module. It now stops there too, and mirrors inspect's order exactly, so a class carrying `__wrapped__` unwraps as inspect would rather than being treated as a class. Module-level names can be defined in terms of each other (`A = 'B'`, `B = 'A'`), and a parameter annotated with one resolved to the other and back until instantiate() hit a RecursionError. Resolution now carries the strings already visited and gives up when one repeats, as it does for any other annotation it cannot resolve. That subsumes the previous "same string twice in a row" check and covers cycles of any length, including ones formed through a union. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
https://github.com/Positronic-Robotics/configuronic/blob/0e0aff39c6d888165e851c887013dc0b1c93165c/config.py#L347
Unwrap decorated class signature sources
When a class target's __init__, __new__, or metaclass __call__ is decorated with functools.wraps from another module, inspect.signature(class_target) reports the wrapped method's annotations, but this class-specific source list retains the wrapper and resolves strings in the decorator module. Consequently an annotation such as 'cfn.Config' cannot resolve and the nested config is instantiated eagerly. Fresh evidence beyond the earlier wrapper case is that only the top-level target is unwrapped; class signature-defining methods are not.
https://github.com/Positronic-Robotics/configuronic/blob/0e0aff39c6d888165e851c887013dc0b1c93165c/config.py#L459
Key the declaration cache by target identity
lru_cache keys callable targets using their __hash__ and __eq__, so two distinct callable objects that compare equal but expose different signatures—for example, value-equal instances with per-instance __signature__ values—share the first object's lazy declaration. The second target can therefore receive an unresolved Config when its own parameter is eager, or vice versa; use identity-based caching or bypass this cache for callable instances whose equality does not imply an identical signature.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more, both found in review: `inspect.signature` follows a class's `__init__` / `__new__` / metaclass `__call__` through its decorators, so a method decorated with `functools.wraps` from another module reported the wrapped method's annotations while the namespace still came from the decorator's module. The walk to the signature-defining callable is now a helper, applied to each class candidate as well as to the target itself. `lru_cache` keys by `__hash__`/`__eq__`, and a callable object may define either: two targets that compare equal can still report different signatures (a per-instance `__signature__`, say), and they shared one declaration — so a parameter could be handed over unresolved because a *different*, equal-comparing target wanted it that way. The cache is now keyed by identity, which also retires the unhashable-target special case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
|
Both findings from the latest review came in the review body rather than as threads, so answering here. Confirmed and fixed in 42623b1. Decorated class signature sources. Cache key. Keying by Both tests fail against the previous commit. 221 tests green on 3.10/3.11/3.12, ruff clean, instantiate cost ~4 µs on the two-node benchmark. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42623b11d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ut Annotated Two more, both found in review: The parameters of a callable object come from its class' `__call__`, which may be inherited from a base in another module — but the namespace came from the module the instance's own class lives in, so an annotation written with a name only the base's module has did not resolve. That class' `__call__` is now the first namespace offered, followed the same way as any other signature source. An annotation is any object, and one carrying a `__metadata__` attribute of its own is not necessarily `typing.Annotated`: reading `__origin__` off it raised AttributeError out of instantiate(). Ask `typing.get_origin` instead of duck-typing the attribute. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9890de46c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Offering every candidate namespace and taking the first that resolves was too forgiving: for a class, only one of the metaclass `__call__`, `__new__` and `__init__` wrote a given parameter, and when that one cannot resolve a name a sibling namespace that happens to define it must not answer instead. The candidates now carry what each declares, and a parameter is resolved in the namespaces of those declaring it under the same name and annotation — usually exactly one. Only when none of them declares it (a callable with an explicit `__signature__`, say) is there nothing to go on, and all of them are offered as before. That keeps this independent of how CPython picks the signature-defining callable, which depends on which of the three the class defines itself and is not ours to re-derive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd586ca57a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A name bound in a class body is in scope for an annotation written there — plainly so when the module does not postpone evaluation, where `pipeline: Alias` is simply the object. Under `from __future__ import annotations` the same source became the text 'Alias', which only the module globals were searched for, so the same code meant different things depending on a module-level import. Each candidate now carries the body of the class it was defined in, used as the locals of the evaluation, which is also how `typing.get_type_hints` resolves a class' own annotations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd7ffad7c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more, both introduced by the previous two commits: The class body paired with `__init__` / `__new__` / a metaclass `__call__` was the class we started from, but an inherited method was written in a base, and its class-body names live there. Each candidate is now paired with the class in the MRO that actually defines it. Matching a declaration against the reported parameter compared annotations with `==`, which runs whatever `__eq__` the annotation object defines — arbitrary code, executed while merely reading a signature, and `instantiate()` failed outright when it raised. Annotation objects are carried through to the reported parameter, so identity answers this; strings still get a real comparison, being the one case where equal-but-distinct objects are plausible and the comparison is safe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
https://github.com/Positronic-Robotics/configuronic/blob/9b6fdd8f2eeca13bcfe4c121f4edb2bdb3332d9c/config.py#L393
Resolve bound-method aliases in their declaring class
When the target is a bound instance or class method whose postponed annotation uses a class-body alias, such as Factory.Alias = cfn.Config and Factory().build(pipeline: Alias), this pairs the bound method with no local namespace. Alias is therefore searched only in module globals, remains unresolved, and the stored config is eagerly instantiated. Locate the class that defines the bound method via its __self__/MRO and include that class's namespace, as is already done for constructors and callable objects.
https://github.com/Positronic-Robotics/configuronic/blob/9b6fdd8f2eeca13bcfe4c121f4edb2bdb3332d9c/config.py#L491
Unwrap PEP 695 type aliases before testing the annotation
On supported Python 3.12+ versions, parameters annotated through type Deferred = cfn.Config or type MaybeDeferred = cfn.Config | None expose a typing.TypeAliasType. Its typing origin is None and it is not identical to Config, so this logic marks the parameter eager and builds the stored config before calling the target. Resolve the alias's underlying value, with cycle protection, before checking Annotated, unions, and Config identity.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… value A bound method is written in a class body like any other, but it was paired with no class namespace, so an annotation using a class-body alias went unresolved. It is now paired with the class that defines it, found from the instance or class it is bound to. A `type X = ...` alias (PEP 695, 3.12+) is neither a union nor `Config` itself, so a parameter annotated `type Deferred = cfn.Config` was built eagerly despite asking for the config. Its `__value__` is now followed before the `Annotated`, union and identity checks. That value is evaluated on access and can be the alias itself (`type X = X`), so aliases already followed are tracked alongside the annotation strings, and one that comes round again is treated as unresolvable rather than recursing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
|
Latest two arrived in the review body rather than as threads, so answering here. Both confirmed and fixed in 087b6d6. Bound-method target. A bound method is written in a class body like any other, but it was paired with no class namespace, so an annotation using a class-body alias went unresolved. It is now paired with the class that defines it, found from the instance or class it is bound to. Covered by
Both new tests fail against 9b6fdd8 on 3.12. 230 tests pass there, 228 + 2 skipped on 3.10 and 3.11, ruff clean. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 087b6d6ecf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A bound method names what it is bound to, so its class body was already found; a static method — or any function reached through the class that defines it — is a plain function, and only its qualified name says where it was written. Walk that name from the function's own globals, giving up on a `<locals>` component, since a scope inside a function is gone by then. Also records why the declaration cache does not notice a target whose annotations are rewritten after first use: checking would mean re-reading the signature every time, which is the cost the cache exists to avoid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50fef56311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Accessing a `functools.partialmethod` on the class hands back a plain function generated inside functools, so the namespace offered for its annotations was that module's rather than the one the method was written in. `inspect.signature` follows the reference back to the real method; now so does the walk. The attribute holding it is `_partialmethod` up to 3.12 and `__partialmethod__` from 3.13, so both names are read. Bound, a partialmethod is an ordinary `functools.partial` and already worked; the test pins both forms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4670b68e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…never fail on one
Resolving an annotation evaluates whatever expression was written there. Doing that for
every parameter of a target meant a config could import modules and run code for
parameters it never set — a module using PEP 562 lazy loading had its submodules pulled
in by an unrelated instantiate(). Declarations are now worked out per parameter, on the
first config that supplies it, and remembered from then on.
Reading a signature must never be what breaks instantiate(). A target is free to define
`__signature__`, `__getattr__` or `__class__` however it likes, so introspection can
raise anything; only TypeError and ValueError were caught, and a `__signature__`
property raising RuntimeError propagated out of a call that worked before this branch.
A parameter that cannot be classified is simply not a Config parameter.
Also, from the same review pass:
- When two candidates declare the same parameter identically, only the first can be the
one the signature came from, so the rest no longer get to answer for a name it cannot
resolve. This closes the sibling-namespace leak the docstring already claimed to
prevent, for the case where the parameter names match as well.
- The de-duplication of candidate namespaces compared `vars(cls)` by identity, which is
a fresh mappingproxy every call and so never matched; it now compares what the
namespaces are taken from. A class defining both `__new__` and `__init__` no longer
evaluates each annotation twice.
- The unwrap walk recorded ids without holding the objects, so a recycled id could stop
it at a callable it had never seen. It holds them now, as `_TargetKey` already does.
- `override()`, `override_data()` and `__call__()` take `self` positionally. An
untrusted caller supplying `{'self': ...}` — the documented `override_data` use — hit
a TypeError that escaped the ConfigError handling a server has in place.
- A `ForwardRef` that names the module it was written in is resolved there, as
`typing.get_type_hints` does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d73143c04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An alias used with arguments (`Deferred[Config]`) is a generic alias over the alias rather than the alias itself, so following it means going through `get_origin` — and what its value is written in terms of are its type parameters, which the arguments bind. Both are followed now, so an alias standing for a `Config` is recognised specialized as well as bare. A type parameter can be bound to itself (`SelfBound[T]`), so substitution carries the same seen-set that already stops a cyclic alias. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4b4a8c86c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Deciding which parameters ask for a config had grown to a third of config.py, in front of the Config API rather than beside it. It is a self-contained job — one entry point in, and only the marker class out — so it moves to a module of its own, which is also where the two rules it answers to can be stated once: resolve an annotation only in the namespace that wrote it, and never fail or run user code to find out what it means. They were previously implicit across six docstrings. `Config` is passed to `declarations_for()` rather than imported there, so reading signatures has nothing to say about what a config means, and the cache stays with the lookup it belongs to. Three cleanups carried along: - `_lazy_slots()` built two sets that were consumed one line later in the same call; `_instantiate_internal` asks per argument instead. - "Lazy" meant both "receives the config unresolved" and "classified on demand". The names now say which: `_config_parameters(target)`, and `wants_config.positional(index)` at the call site. - The state threaded through the annotation walk (`_seen`, `_bound`) was two underscore-prefixed parameters that recursion had to remember to pass on. It is a `_Resolution` carrying its own `wants_marker()`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
Unwrapping ran to the end of the chain before looking for `__self__`, and a decorated method leads through `__wrapped__` to a function that knows nothing about the instance. The binding was therefore lost, and with it the class body — so an annotation naming an alias bound there was left unresolved and the config was built. It only shows when the class is defined inside a function, since otherwise the class is recoverable from the qualified name. The binding can sit anywhere along the chain, not just at either end (`functools.partial` of a decorated bound method puts it in the middle), so the walk now returns every link and the binding is looked for from the target inwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35f950b60f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A class can supply its parameters directly by binding `__signature__` in its body, and `inspect.signature` reports those in preference to anything its constructor says. Only the metaclass `__call__`, `__new__` and `__init__` were offered as namespaces, so when such a class inherited its constructor, an annotation naming a name bound in its own body resolved nowhere and the config was built. The class itself is now offered first when it declares a signature, paired with the body that binds `__signature__` — a base's, if that is where it was written. Only when it declares one: offering the class unconditionally would let it answer for parameters its constructor wrote, with the wrong body. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a06b9c32ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A class that declares no signature of its own still reports one when its metaclass binds `__signature__`, on every version this package supports. The class' own MRO was searched for the attribute, which never finds it there, so the annotations were resolved with no class body at all and against the wrong module — the class', not the metaclass'. The declaring body is now looked for in the metaclass when the class does not define one itself. A candidate with no globals of its own also takes them from the module of the body it was written in rather than its own, which is the same rule and matters equally for a signature written in a base class living elsewhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5164a559b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`inspect.signature` reads that value as "no explicit signature" and falls through to the constructor, but the attribute was there, so the class was offered as the namespace for parameters its base had written — and, being the most specific candidate, it won them. An inherited constructor whose annotation names something bound where it was written then resolved nowhere, and the config was built. A regression from the two commits that introduced the candidate; a subclass without the attribute was unaffected. The candidate is now offered only when `__signature__` holds a real `Signature`, which is the test `inspect.signature` itself applies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 853f3a499d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A private name is mangled where it is written — `__Alias` in the body of `Factory` is stored as `_Factory__Alias` — while a postponed annotation keeps the source text. Handing the class dict straight to `eval` therefore left `__Alias` unresolved, and the same class without the `__future__` import was lazy while this one was not. The class body is now offered with private names under the spelling they were written as, alongside the mangled ones. `typing.get_type_hints` raises `NameError` on this today; matching the non-postponed spelling is the same reason the class body is consulted at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91d66c726b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Which of a class' metaclass `__call__`, `__new__`, `__init__` or declared `__signature__` supplied the signature is CPython's business, and restating those rules here would risk resolving an ordinary annotation in the wrong module. So when more than one of them declares the same parameter with the same annotation, each answers in its own namespace and the parameter is only handed the config if they agree. Previously the first was taken, which meant the answer rested on the order the candidates happen to be offered in. Declining leaves the config built, which is what happens for every parameter that does not ask for one, and is the safer way to be wrong: a target handed an object it did not expect fails where it is called, one handed a config it did not expect fails somewhere inside itself. Only rivals can disagree in that sense. A callable object and its class' `__call__` are one declaration reached two ways, so they stay a chain from most specific outwards rather than two answers that have to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
`type Lazy = 'C'` hands back the string `'C'`, which names something in the module the alias was written in — a module that imports `Lazy` need not have heard of `C`. That string was resolved against the importing target's namespaces, so the config was built. Following a step that names a namespace now carries it along, so what is reached through it resolves there first. A `ForwardRef` naming its own module already did this for one round; it now does so for the rest of the walk too, since a string that resolves to another string was written in the same place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a9d34b7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The declaration was cached per target, which made it stale for a target that changes what it declares — an answer kept from before, wrong in a way nothing reveals. Correctness wins over the saving: a target is read on every instantiate, so what it declares now is what is used. Answers are still remembered for the length of one lookup, so a target with several such parameters reads its signature once rather than once per parameter, and a parameter with no annotation at all no longer sends anything looking for the namespaces it would have been resolved in. Instantiating a two-node config goes from ~3.5 us to ~53 us, and a node whose annotations are postponed to ~255 us, the resolution of a stringified annotation being most of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
A declared `__signature__` is not one candidate among several: it is what `inspect.signature` reports, and nothing a constructor declares can change that. It was still being treated as a rival, so an inherited `__init__` declaring the same parameter with the same text — and resolving it to nothing where that text was written — disagreed with it, and the disagreement declined a signature that was never in doubt. The other candidates stay as namespaces to fall back on when the declared signature mentions a parameter none of them wrote. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
Text already followed was tracked by the text alone, so a spelling that appears twice in different scopes was mistaken for a name defined in terms of itself. Importing an alias under the same name its value quotes is enough to hit it: `C` is the alias in one module and `Config` in the other, and the second `C` was refused before it could be resolved where it was written. What has been followed is now the pair of the text and the namespaces it would be resolved in, so a cycle is still a cycle — the same text, in the same scope, twice — while the same text in a scope the walk has just moved into is what it has always been, a different name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cb810ccbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two fixes to where a declaration's annotations are resolved, both about a namespace that shares its globals with another. A callable object whose class declares the signature was paired with no class body at all: an instance has no qualified name and no binding to follow, so there was nothing to find it by. But the declaration is a statement in the class body, which is where its annotations were written — the same rule already applied to a class that declares one. And a scope was identified by its globals alone, so a class body and the module around it counted as one. A name resolved in each of them looked like the same name resolved twice, and the second was refused as a cycle: enough to break `C = Lazy` in a class body whose module also binds `C`. Both halves now count, with one shared empty mapping standing for "nothing local", so that two sources with no body of their own stay one scope rather than two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
Reading the signature on every call is not only the annotated parameters' concern — every config pays it, so it belongs where a user upgrading will see it rather than only in the pull request that made the trade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NP2p59PnRALnY4QMmRFWJz
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e4da50237
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # body is where its annotations were written; a function carries its own. | ||
| declared_in = _defining_class(type(func), '__signature__') | ||
| written_in = declared_in if declared_in is not None else written_in | ||
| candidates = [(type(func).__call__, _defining_class(type(func), '__call__')), (func, written_in)] |
There was a problem hiding this comment.
Honor callable-instance signatures over
__call__
When a callable instance inherits an explicit __signature__ from one base and __call__ from another, both sources remain candidates here. If both declare the same parameter and annotation text (for example, 'Alias') but bind that alias differently, _scopes_for() selects the earlier __call__ declaration even though inspect.signature() unambiguously selected __signature__. A signature whose base defines Alias = Config can therefore be resolved using a __call__ base where Alias names the built type, causing the supplied config to be instantiated; an explicit callable-instance signature should end source selection just as an explicit class signature does.
Useful? React with 👍 / 👎.
A parameter annotated
cfn.Confignow receives the stored config instead of its instantiation, so a target can build it later, more than once, or with overrides it only learns at runtime.The declaration lives in the target's signature rather than at the call site: wanting a config rather than an object is a property of the target, not of each call. Callers keep passing ordinary values and writing ordinary overrides, so the dict-of-configs-keyed-by-string workaround — and everything it gave up — goes away:
Closes #38.
Implementation
_instantiate_internalconsults the target's signature and passes a slot through as stored where the parameter is annotatedConfig.Deciding that is a self-contained job with one entry point, so it lives in
configuronic/_annotations.py(585 lines) rather than in front of theConfigAPI.Configis handed todeclarations_for()rather than imported there, so reading signatures has nothing to say about what a config means._Resolution.wants_markeraccepts a bareConfig, a union containing it (Config | None,Optional[Config]),Annotated[Config, ...], atype X = ...alias standing for any of those — bare or specialized,Deferred[Config](PEP 695, 3.12+) — and string annotations of all these forms._Declarationmaps the annotations onto positional indices and names (including*args/**kwargs), so positional arguments and class targets work; a target with no introspectable signature (a C callable) declares nothing.instantiate()that used to cost single-digit microseconds now costs tens of microseconds per config node, and a couple of hundred for a node whose annotations are postponed and have to be resolved as text. A parameter with no annotation costs nothing beyond reading the signature.Decisions on the open points from the issue:
Config | NoneNoneConfigvaluelist[Config]still resolve.override()/.override_data(), which copyAnnotation resolution
Under
from __future__ import annotationsan annotation arrives as source text, and deciding what it means is most of this diff's complexity. Three rules govern it:Resolve it where it was written. The namespace is found by following the target the way
inspect.signaturefinds the parameters — throughfunctools.partial,functools.wrapsandpartialmethodwrappers, stopping at a callable that declares its own__signature__, into a class' declared signature, metaclass__call__,__new__or__init__(each followed through its own decorators), and to the class'__call__for a callable object, which may be inherited from a base elsewhere. Names bound in the class body that wrote the method count too — including private ones, under the spelling they were written as — which is how the same annotation resolves when the module does not postpone evaluation. A declared__signature__is a statement in a class body, so it resolves in the body that binds it. Following an alias or a forward reference brings the module it was written in along, since what is reached through it was written there.When it cannot be told where it was written, decline. A class' declared
__signature__settles the question outright. Failing that, several callables may declare the same parameter identically, and which one CPython picked is not knowable here — so each answers in its own namespace and the config is only handed over if they agree. Declining leaves the config built, which is what happens for every parameter that does not ask for one.Never fail, and never run user code to find out. An annotation that cannot be resolved is simply not a
Configannotation —instantiate()must not start raising over a forward reference, a name defined in terms of itself, a self-referentialtypealias, or an annotation object with an opinionated__eq__. The decision is made on the resolved object's identity againstConfig, so every spelling works, including a local alias.Tests
tests/test_lazy_config_params.py(80 tests) with support modules undertests/support_package/for the postponed-annotation cases. Every acceptance criterion from the issue is covered, plus the resolution surface above: partials,partialmethods,functools.wrapswrappers across modules, declared__signature__s on functions, classes, metaclasses and callable objects, classes built by__new__or a metaclass, decorated and inherited__init__s, decorated bound methods of classes defined inside functions, class-body aliases (private and inherited), module-level aliases, bound and static methods, callable objects,typealiases (bare, specialized, quoted and shadowed), rival declarations that agree and that disagree, unhashable targets, a user class that happens to be namedConfig, and every "must not raise" case named above.251 tests pass on Python 3.12 and 3.13, 243 + 8 skipped (the PEP 695 ones) on 3.10 and 3.11;
ruff check .andruff format --checkpass. The motivating example was also run end to end throughcfn.cli, checking the three shell invocations above.Docs
README section "Receiving a config instead of an object" next to the instantiation semantics, a pointer from the API reference,
instantiate()docstring, CHANGELOG entry, and a 0.7.0 version bump (matching the convention from #37).Generated by Claude Code