Fix local_annotations() ignoring the stop= MRO boundary#74
Merged
Conversation
annotations(cls, stop=...) is documented to only consider fields declared between `stop` (exclusive) and `cls` (inclusive) in the MRO -- callers rely on this to exclude internal bookkeeping attributes declared on a base class above `stop` (e.g. faust.Record passes stop=Record to exclude its internal ModelT/Model bases). local_annotations(cls) resolved fields via typing.get_type_hints(cls, ...), which always merges annotations from the *entire* real MRO regardless of `stop` -- iter_mro_reversed() correctly bounds which *classes* get scanned, but get_type_hints() on any one of them independently re-walks the full ancestry, defeating the boundary. A non-ClassVar annotation on an excluded base (faust.types.models.ModelT.__evaluated_fields__, mirroring the ClassVar-marked __is_model__ right above it) leaked back in as if it were a real field of every subclass -- breaking every faust.Record's generated __init__ signature. Read cls.__annotations__ directly (the class's own annotations only, not inherited) instead. String/ForwardRef annotations are still resolved per-value by the existing _resolve_refs()/eval_type() pass right below. Adds a regression test mirroring the ModelT scenario. Introduced by e8d0a96 ("Support Python 3.13, drop Python 3.8"). Note: this does not resolve every fallout observed downstream in faust -- faust/tests/functional/test_models.py still has ~11 failures involving related/polymorphic model field coercion after this fix (down from total collection failure across faust's whole model/table test suite). That appears to be a narrower, separate issue and needs further investigation. Assisted-by: Claude Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HgnKFtXZbCjXNoVNWa5JLd
The stop= MRO-boundary fix in the previous commit read cls.__annotations__ directly, which broke CI on Python 3.14: PEP 649 (deferred evaluation of annotations) changes how the __annotations__ attribute is computed for a class, and CPython explicitly documents that direct __annotations__ access under PEP 649 can behave unreliably -- inspect.get_annotations() is the recommended replacement. On 3.14 this manifested as annotations silently going missing (test_annotations, test_annotations__skip_classvar) or a forward reference no longer raising the expected NameError (test_annotations__no_local_ns_raises). inspect.get_annotations() is also a strictly better fit for what local_annotations() needs than raw attribute access ever was: it reads only cls.__dict__['__annotations__'] -- a class's own annotations, with no MRO fallback -- whereas plain cls.__annotations__ attribute access on Python < 3.10 falls back to an inherited base's __annotations__ via normal MRO lookup when cls itself declares none of its own, which could reintroduce a leak of the same shape this fix exists to close. inspect.get_annotations was added in Python 3.10; add a same-behavior backport (_own_annotations) for the 3.9 floor. Assisted-by: Claude Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HgnKFtXZbCjXNoVNWa5JLd
wbarnha
force-pushed
the
claude/faust-mode-open-prs-ur3hqs
branch
from
July 19, 2026 04:02
328f77b to
31c7187
Compare
Not a real fix -- both raw cls.__annotations__ access and inspect.get_annotations(cls) produce the identical broken partial result on the CI 3.14 runner, and there's no 3.14 interpreter available in this environment to debug locally. This test dumps every angle (cls.__dict__, attribute access, inspect.get_annotations with and without eval_str, __annotate__, local_annotations()) to captured stdout so the actual 3.14 CI run can serve as the debugging oracle. Will be removed/replaced by the real fix in a follow-up commit. Assisted-by: Claude Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HgnKFtXZbCjXNoVNWa5JLd
…shim
The diagnostic test added in the previous two commits (now removed) traced
the actual 3.14 failure to eval_type(), not local_annotations(): every
call crashed with
AttributeError: 'ForwardRef' object has no attribute
'__forward_evaluated__'. Did you mean: '__forward_module__'?
eval_type() unconditionally routed every ForwardRef through
_ForwardRef_safe_eval(), a hand-rolled evaluator that read ForwardRef's
private __forward_evaluated__/__forward_code__/__forward_value__
attributes -- by its own comment, a workaround for "3.6/3.7". mode's floor
is Python 3.9, so this path has been dead weight for the versions it was
written for, and Python 3.14 no longer exposes those attributes on
ForwardRef at all, turning the dead weight into a hard crash on every
string-annotated field (e.g. `foo: "int"`), which test_annotations,
test_annotations__skip_classvar and test_annotations__no_local_ns_raises
all exercise.
typing._eval_type (imported as _eval_type, already used unconditionally
right below the removed call) resolves ForwardRef instances directly and
is the same stdlib function typing.get_type_hints() itself relies on, so
it always matches whichever ForwardRef implementation the running
interpreter actually has. Drop _ForwardRef_safe_eval and the _type_check
import it was the sole user of, and let _eval_type handle ForwardRef
resolution on its own, as it always could.
Verified: full suite passes on 3.11 and 3.13 (736), ruff clean, the
ModelT-leak regression from the stop= MRO-boundary fix stays fixed, and a
faust-streaming/faust checkout instantiating faust.Record subclasses still
works correctly with this mode installed locally. No 3.14 interpreter is
available in this environment (network policy blocks python-build-standalone
and deadsnakes, no Docker daemon) -- this fix was derived from real 3.14 CI
runtime output via a temporary diagnostic test, not guessed, and CI's actual
3.14 job is the verification oracle for this specific change.
Assisted-by: Claude
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgnKFtXZbCjXNoVNWa5JLd
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
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.
Description
Severity: breaks every
faust.Recordsubclass constructor, on every supported Python version — introduced bye8d0a96("Support Python 3.13, drop Python 3.8"), currently blocking essentially allfaust-streaming/faustCI.Root cause #1 —
local_annotations()ignoring thestop=MRO boundaryannotations(cls, stop=...)is documented to only consider fields declared betweenstop(exclusive) andcls(inclusive) in the MRO — callers rely on this to exclude internal bookkeeping attributes declared on a base class abovestop(e.g.faust.Recordpassesstop=Recordto exclude its internalModelT/Modelbases).local_annotations(cls)resolved fields viatyping.get_type_hints(cls, ...), which always merges annotations from the entire real MRO regardless ofstop.iter_mro_reversed()correctly bounds which classes get scanned, butget_type_hints()on any one of them independently re-walks the full ancestry, defeating the boundary. A non-ClassVarannotation on an excluded base —faust.types.models.ModelT.__evaluated_fields__, sitting right next to the correctly-filteredClassVar-marked__is_model__— leaked back in as if it were a real field of every subclass, becoming a required positional parameter of every generated__init__.Fix:
local_annotations()now reads a class's own annotations viainspect.get_annotations(cls)(with a same-behavior backport for the 3.9 floor, since it was added in 3.10) instead ofget_type_hints. This has no MRO fallback at all — strictly safer than the boundary get_type_hints() defeated.Root cause #2 — found via CI's actual Python 3.14 runner, not in this repo's control
Fixing #1 alone still crashed on Python 3.14 CI (a different, pre-existing latent bug this investigation surfaced): every field with a string-quoted annotation (e.g.
foo: "int") crashed witheval_type()unconditionally routed everyForwardRefthrough_ForwardRef_safe_eval(), a hand-rolled evaluator readingForwardRef's private__forward_evaluated__/__forward_code__/__forward_value__attributes — by its own comment, a workaround for "3.6/3.7". mode's floor is Python 3.9, so this path was already dead weight for the versions it targeted, and Python 3.14 no longer exposes those attributes onForwardRefat all, turning dead weight into a hard crash.Fix: removed
_ForwardRef_safe_eval(and the now-unused_type_checkimport).typing._eval_type— already called unconditionally right below the removed code — resolvesForwardRefinstances directly and is the same stdlib functiontyping.get_type_hints()itself relies on, so it always matches whicheverForwardRefimplementation the running interpreter actually has.No 3.14 interpreter was available in this development environment (network policy blocks python-build-standalone/deadsnakes, no Docker daemon) — root cause #2 was diagnosed from real 3.14 CI output via a temporary diagnostic test (since removed), not guessed.
Verification
test_annotations__stop_excludes_base_above_stop, a regression test mirroring theModelTscenario (root cause Pass loop attribute to asyncio.ensure_future #1). Confirmed it fails with the pre-fix code and passes with the fix.test_annotations,test_annotations__skip_classvar, andtest_annotations__no_local_ns_raisesalready exercise string-quoted forward refs, so they cover root cause Rename package and fix configuration files #2 without needing a new dedicated test.faust-streaming/faustcheckout with this fixedmodeinstalled in place of the PyPI release:Foo(x=1, name="hi")instantiates correctly andFoo._options.fieldshas no__evaluated_fields__leak.Known residual issue — not fully resolved by this PR
Independent of both fixes above, faust's
tests/functional/test_models.pystill has an estimated ~11 failures involving related/polymorphic model field coercion (test_custom_coercion,test_polymorphic_fields,test_Secret,test_default_no_blessed_key, and others), down from total collection failure for the whole file beforehand. This looks like a narrower, separate issue — possibly other fallout frome8d0a96— and needs further investigation before considering faust's model test suite fully green again. (Not independently re-verified in this environment due to an unrelated pytest-version/conftest mismatch when running faust's own suite here.)🤖 Generated with Claude Code
https://claude.ai/code/session_01HgnKFtXZbCjXNoVNWa5JLd