Fully support type hierarchy in domain types - #1944
Merged
Merged
Conversation
…dy does `DynamicType.parents` has existed since dynamic types were introduced, but assignability never consulted it: `isAssignableFrom`/`isAssignableTo` compared names, so with `Employee -> Person`, `Person.isAssignableFrom(Employee)` was false. `JvmType` has always walked the real class hierarchy, so the two halves of the sealed `DomainType` disagreed about what inheritance means — a type system that lets you declare a parent and then denies it. Walk the declared chain instead. `isAssignableTo` matches this type or any transitive ancestor; `isAssignableFrom` delegates to it, so `a.isAssignableFrom(b) == b.isAssignableTo(a)` still holds. An ancestor may be a `JvmType` — how a non-JVM type declares it is a `Signal` — in which case that ancestor answers for its own class hierarchy, so `JvmType.isAssignableFrom` now delegates the `DynamicType` branch rather than returning a flat false, which would otherwise break the symmetry the moment the child walks its parents. The walk is iterative and de-duplicated by name, so a hand-built cyclic parent chain terminates instead of overflowing the stack; nothing stops a caller constructing one. Only dynamic parents are expanded — a `JvmType` ancestor answers for its own hierarchy, and expanding it here would reflectively load every superclass and interface to answer a question they answer themselves. Strictly additive: every pair that was assignable still is, and only parent-related pairs newly become so. The 32 existing assignability assertions pass unmodified. The `Class<*>` overloads are deliberately untouched — a dynamic type has no JVM class, and whether declaring a JVM parent should make it assignable to that `Class` is a separate question with its own blast radius (`AgentInvocation.findAgentByResultType` matches on exactly that overload). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KUhyouczgUVdjt2urwVy4X
|
igordayen
self-requested a review
August 17, 2026 04:33
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.



The problem
DynamicType.parentshas existed since dynamic types were introduced, butassignability never consulted it — it compared names:
JvmTypehas always walked the real class hierarchy, so the two halves of thesealed DomainType disagreed about what inheritance means.
The change
isAssignableTomatches this type or any transitive ancestor;isAssignableFromdelegates to it, so a.isAssignableFrom(b) == b.isAssignableTo(a) still holds.
An ancestor may be a
JvmType— how a non-JVM type declares that it is aSignal — in which case that ancestor answers for its own class hierarchy, and
a type declaring Signal is therefore also assignable to Signal's supertypes.
That is why JvmType.isAssignableFrom now delegates its DynamicType branch
instead of returning a flat false: leaving it would break the symmetry
invariant the moment the child walks its parents, and a silently asymmetric type
system is worse than a widened one.
The walk is iterative and de-duplicated by name, so a hand-built cyclic parent
chain terminates rather than overflowing the stack — nothing stops a caller
constructing one. Only dynamic parents are expanded: a JvmType ancestor answers
for its own hierarchy above, and expanding it here would reflectively load every
superclass and interface to answer a question they answer themselves.
Backward compatibility
signatures, no visibility changes, no new members; the walk helper is private.
blast radius is the two in-module implementations.
walk always includes this, so every pair that was assignable still is. New
trues arise only from a non-empty parents chain; nothing becomes false.
DynamicType with parents, so the widened branch is unreachable from the
framework's own code paths. It activates only for a consumer that populates
parents — which is the point of the field.
JvmType.isAssignableFrom(dynamicTypeDeclaringThatJvmParent), false → true,
for the symmetry reason above.
Deliberately out of scope
The Class<*> overloads still return false for DynamicType. A dynamic type
has no JVM class, and whether declaring a JVM parent should make it assignable to
that Class is a separate question with a wider blast radius —
AgentInvocation.findAgentByResultType matches on exactly that overload, so
widening it would change which agent an invocation resolves to. Worth deciding
on its own merits, not as a side effect of this.
Testing
DomainTypeAssignabilityTest: 43 green. All 32 pre-existing assertions passunmodified — that is the compatibility evidence. 11 new cases cover parent
and grandparent chains, non-symmetry between parent and child, unrelated types,
multiple parents, a diamond, a cyclic chain, a declared JVM parent and its
supertypes, cross-kind symmetry, and the untouched Class<*> overloads.