Resolve PEP 695 type aliases in _isinstance and typehint_issubclass - #6986
Conversation
Greptile SummaryThe PR resolves PEP 695 and typing_extensions type aliases during runtime instance and type-hint subclass checks.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/utils/types.py | Adds lazy TypeAliasType resolution to _isinstance and typehint_issubclass while preserving existing non-alias paths. |
| tests/units/reflex_base/utils/test_types.py | Adds direct coverage for bare, literal, generic, optional, and union-valued aliases across both runtime type helpers. |
| tests/units/test_event.py | Adds event-handler regression coverage for alias-annotated arguments and genuine mismatches. |
| tests/units/test_state.py | Adds state-assignment regression coverage for alias-annotated variables and mismatch logging. |
| packages/reflex-base/news/+pep695-alias-runtime.bugfix.md | Describes the resolved state-assignment and event-compilation failures. |
Reviews (5): Last reviewed commit: "Merge branch 'main' into claude/rel-fix-..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
FarhanAliRaza
left a comment
There was a problem hiding this comment.
Validated end to end in a real app (Python 3.14 native type statements: plain, Literal, Items[T], Pair[K, V], Key | None): alias-annotated var assignment works, uncalled alias-annotated handlers on on_change/on_submit compile and fire, mismatches log instead of raise, plain vars unaffected. Regression tests fail without the source change. One perf concern inline.
|
@benedikt-bartscher awesome thanks for that. will merge yours in and rebase this one. 0.9.9 is getting close 🤞 |
|
I guess we can improve performance for the new benchmarks a bit |
yeah, that unconditional type alias unpacking is more expensive than i realized. probably need to cache it and see if that makes a difference |
PR #6944 added resolve_type_alias() but wired it only into Var.guess_type, so alias-annotated state vars compiled while every runtime path that re-checks the raw annotation still choked on the TypeAliasType: - State.__setattr__ validates assignments through _isinstance(), which fell through to the bare isinstance() call and raised "TypeError: isinstance() arg 2 must be a type" on every assignment to an alias-annotated var, aborting the event handler instead of at most logging a mismatch (0.9.9a1 pre-release FINDING-002). - Passing an event handler with an alias-annotated argument uncalled to an event trigger crashed page compile: typehint_issubclass() reached the bare issubclass() for a plain alias and failed the origin comparison for a subscripted one, escalating to a fatal error (FINDING-006). Resolve the alias at the entry of both functions, reusing resolve_type_alias(), which already handles parameterized aliases (Items[str]) and aliases nested in unions (Key | None). Alias-annotated vars now behave exactly like their resolved annotations at runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
The unconditional resolve_type_alias() at the entry of _isinstance and
typehint_issubclass ran on every recursive per-item and per-member call,
regressing event processing by ~28% (local pytest-benchmark mean;
CodSpeed -13.6%) even though no alias was present. Move resolution to
the exact points where an alias becomes opaque:
- _isinstance: catch the TypeError from isinstance() on a bare alias
(zero-cost try on the hot path, mirroring safe_issubclass) and check
the already-computed origin for subscripted aliases; union and Literal
branches resolve alias members through their existing per-member
recursion.
- typehint_issubclass: same try/except in the non-generic issubclass
branch, a one-sided check before each union decomposition (so an
alias of a union keeps union semantics), and a two-sided check before
the final origin comparison.
- resolve_type_alias: rebuild a union only when a member actually
resolved, instead of allocating a resolved tuple on every call (also
helps Var.guess_type, which calls it unconditionally).
Local microbench (20k-iter timeit, best of runs) vs origin/main:
_isinstance('x', str|None) 1.61us -> 1.62us (was 4.78us),
typehint_issubclass(str, str|None) 1.81us -> 1.89us (was 5.98us),
_isinstance(list(range(20)), list[int], nested=1) 16.4us -> 16.7us
(was 26.9us). test_process_event mean back inside main's noise band
(2.78-2.96ms main, 2.87-3.09ms fixed, 3.72-3.79ms before).
Also add the review-requested regression test for uncalled
alias-annotated handlers through call_event_handler (previously the
opaque "Could not compare types" TypeError at page compile), alias-of-
union coverage locking the guard placement, and the missing docstring
on _type_alias_types() in the types tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
7e845d2 to
e688d7d
Compare
|
Pushed e688d7d taking a slightly different route than caching: the unconditional Generated by Claude Code |
CodSpeed still flagged test_isinstance_container[list_dict] (-7.58%): the isinstance(origin, TypeAliasTypes) probe ran on every generic check before the args dispatch. Move it into the final fallback's TypeError handler — a subscripted alias matches none of the container branches and get_base_class hands the opaque alias back, so the existing isinstance there raises deterministically. Alias-free generic checks now pay nothing; interleaved local A/B puts this at parity with main (8.4ms vs 8.45ms; previous head 9.07ms) on the flagged benchmark. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
|
i wouldn't be surprised if that last remaining codspeed issue is really just a fluke. 4% regression on different hardware than the baseline, and we have our threshold set at 3% |
|
CodSpeed status on this PR, for whoever merges: the genuine regression is fixed and the remaining flag looks like environment noise.
Each successive flag has been a different benchmark under a cross-environment comparison, so I'm not pushing further speculative changes. If you agree it's noise, acknowledging on CodSpeed (or a re-run against a same-environment baseline) should clear the check. Generated by Claude Code |
maybe #6993 helps a tiny bit |
All Submissions:
Type of change
Changes To Core Features:
Defect
Found in 0.9.9a1 pre-release testing as FINDING-002 (HIGH) and FINDING-006 (MEDIUM). PR #6944 added
resolve_type_alias()toreflex_base/utils/types.pybut wired it only intoVar.guess_type, so state vars annotated with a PEP 695 alias (type Key = Literal["a", "b"], or thetyping_extensions.TypeAliasTypebackport that 3.10/3.11 users hit) compiled while every runtime path that re-checks the raw annotation still choked on theTypeAliasType:State.__setattr__validates assignments through_isinstance(), which fell through to the bareisinstance()call and raisedTypeError: isinstance() arg 2 must be a type, a tuple of types, or a unionon every assignment to an alias-annotated var. The guard was designed to onlylogger.errora mismatch, but the TypeError escaped and aborted event processing — alias vars compiled but every mutating event handler died.rx.input(on_change=S.choose)), crashed page compile:typehint_issubclass()reached the bareissubclass()for a plain alias (TypeError: Could not compare types <class 'str'> and Key) and failed the origin comparison for a subscripted alias (Pair[str, str]), escalating what should at most be a warning into a fatalEventHandlerArgTypeMismatchError.Fix
Resolve
TypeAliasTypeat the entry of_isinstance(the annotation operand) andtypehint_issubclass(both operands), reusing the existingresolve_type_alias()helper, which already handles parameterized aliases (Items[str]) and aliases nested in unions (Key | None). Nested annotations (e.g. an alias insidelist[...]) are covered by both functions' per-argument recursion resolving at each entry. Alias-annotated vars and handler arguments now behave exactly like their resolved annotations at runtime; the resolution is a no-op fast path for non-alias types. News fragment added inpackages/reflex-base/news/.Test plan
Regression tests written first and shown to fail on unfixed
mainwith exactly the reported errors:tests/units/reflex_base/utils/test_types.py::test_isinstance_resolves_type_aliasand::test_typehint_issubclass_resolves_type_alias— plain alias,Literalalias, parameterized generic alias, andalias | None, parameterized over thetyping_extensionsbackport and (on 3.12+) the nativetyping.TypeAliasTypethat thetypestatement produces.tests/units/test_state.py::test_setattr_alias_annotated_var— a state with all four alias-annotated var shapes assigns them via an event handler; also pins that a mismatched value is logged, not raised.Verified locally:
uv run pytest tests/units/utils/test_types.py tests/units/reflex_base/utils/test_types.py tests/units/reflex_base/vars/test_base.py tests/units/test_state.py tests/units/test_event.py tests/units/components tests/units/test_app.py(4600+ passed),uv run ruff check .,uv run ruff format .,uv run pyright reflex tests(0 errors),uv run pyright packages/reflex-base(0 errors, 1 pre-existing warning in an untouched file). Both pre-release repro scripts (repro_alias_setattr.py,repro_alias_event_arg.py, Python 3.14 nativetypestatements) now pass, and a parameterized-aliason_submithandler behaves identically to a plaindict[str, str]annotation.🤖 Generated with Claude Code
https://claude.ai/code/session_01EMjBXPozsNeQNSBZecNH8x
Generated by Claude Code