Repository navigation
Decide whether a node type may be defined outside the kernel (#1026) - #1040
Merged
Merged
Conversation
Entity is a public abstract record with a protected constructor, which reads as an invitation to derive from it, and five of its eleven abstract members are internal or private protected, so a subclass in another assembly fails to compile with CS0534 naming exactly those five. That is the blocker under #746 items 53 and 56 and under Packaging.md's "blocked, not decided" for domain packages -- item 56 attributes the seam to #338, looking a type up so it can be parsed, which is the second half of it. The decision is to open the contract without publishing anything: four of the five are engine machinery with a default the kernel can supply and be right about -- Priority is read only by the printers, SortHashName is a deduplication table over kernel node kinds, ToSymPy already throws from a base class for a node SymPy has no name for, and an empty inversion is what a third of the node types already return. Only IntrinsicCondition has no safe default: Boolean.True is the positive claim that an operation is total, and only the node's author knows whether it holds, so it becomes protected abstract. Publishing the five instead would drag two internal enums and 30 frozen enum values into the public API for the same capability. The document costs the alternative -- a closed hierarchy extended by data -- against that, and records why it is not the status quo either: Number.Real, Rational, Complex and Variable are concrete and unsealed, and a subclass of one compiles against the nupkg today and goes through Simplify and Solve intact. The defaults were given to a node kind the kernel has never seen and run through the pipelines EveryNodeSurvivesEveryPipelineTest uses. Sixteen of the seventeen held, including like-term grouping and the provided machinery. The one that did not is a twelfth obligation written nowhere: every node type carries `ToString() => Stringize()`, and without it the record-synthesized ToString recurses through Entity.PrintMembers until the stack guard fires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd
This was referenced Aug 23, 2026
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.
Closes the design half of #1026 and unblocks #746 item 56.
What was wrong
Entityis apublic abstract partial recordwith aprotectedconstructor. Five of its eleven abstract members areinternalorprivate protected, so a node type cannot be defined outsideAngouriMath.dll. Measured, not remembered — a class library referencing the built kernel and implementing all six reachable members:Exactly five, and no other error.
InternalsVisibleTois not an escape — a published extension package cannot be on that list.What the document decides
Sources/AngouriMath/Docs/Contributing/NodeContract.md. Open the contract without publishing anything.Traced, per member, from what reads it rather than from what it looks like:
PriorityPriority.FuncFunctions/Output/; it is bracketing precedence, and 27 node types already inheritFuncfromEntity.FunctionSortHashNameFunctions/Simplification/Patterns/; the values are a deduplication table over kernel node kinds (Sumf/Minusf→summinus_), and an external node has no partner in itToSymPyNotSufficientlySupportedExceptionEntity.Set.SpecialSetalready throws exactly that from a base classInvertNodeEnumerable.Empty<Entity>()IntrinsicConditionprotected abstractpublicmember (DomainCondition).Boolean.True— 39 of 60 implementations — is the positive claim that an operation is total, and for an unknown node that is the unsafe directionNothing becomes
public. That is the whole argument against the obvious reading: publishing the five dragsinternal enum Priority(30 named values,|-composed with deliberate ties) andinternal enum SortLevelinto the public API, so every later change to bracketing or like-term grouping becomes aBREAKING-CHANGES.mdentry — for a capability the defaults deliver without it.The alternative — keep
Entityclosed, let a domain package contribute data — is costed rather than dismissed: it keeps exhaustive matching and the reflection sweep sound, butInnerSimplifyis a function rather than a table row, so the "data" acquires delegates immediately, the indirection lands onSimplify's default path, and a domain package's own rules stop matching on types.Two things measured that were not known
The hierarchy is already open.
Number.Complex,Number.Real,Number.RationalandVariableare concrete and unsealed by threeSealedOrAbstractexemptions, so a subclass inherits every implementation and compiles from outside today:Stringizegives3,3 + xsimplifies to3 + x,SolveEquation("x")gives{ -3 }. So "closed, therefore exhaustive matching is sound" is already only true by convention, andEveryNodeSurvivesEveryPipelineTest— which enumeratestypeof(Entity).Assembly.GetTypes()— cannot see such a type.There is a twelfth obligation nobody has written down. All 68 concrete node types carry
public override string ToString() => Stringize();(69 occurrences, one on an abstract base).AddingNode.csdoes not mention it and it is not optional: without it the record compiler synthesizesToString, which calls the synthesizedEntity.PrintMembers, which appendsEntity's ownEntity-valued public properties, each callingToStringagain:Entityshould declarepublic sealed override string ToString() => Stringize();once — 69 lines deleted, and the trap gone for every future node.What was measured
The five proposed defaults were given to a node kind the kernel has never seen (
GeoPointf(X, Y), printingpoint(x, y)) and put through the seventeen pipelinesEveryNodeSurvivesEveryPipelineTestuses, on five shapes. Compiled against a local build of this tree withInternalsVisibleToadded and signing off — the only way such a type compiles today; not a build ofmaster.Sixteen of seventeen held.
point(x, x) + point(x, x)→2 * point(x, x)(the pattern layer grouped an unknown node through the default sort hash);point(x, x) / point(x, x)→1 provided not point(x, x) = 0(theprovidedmachinery reached it throughDomainCondition);Solverefused with the library's ownUncompilableNodeException. The seventeenth is theToStringrecursion above.Trimming
Any design needing runtime assembly scanning is disqualified — the kernel declares
IsAotCompatibleandAotSmokeTestgates it. That bites #338 as written ("look up the types, inherited fromFunctionEntity"); the capability behind it needs explicit registration or a generated table. Option A itself is virtual dispatch and needs no reflection at all.Scope
Documentation only — no code changes. Where the document concludes code must change, that is its recommendation, not this diff.
Docs/Contributing/README.mdgains one index entry.🤖 Generated with Claude Code
https://claude.ai/code/session_01Bjumi5K7fg8yx6UK1mZTQd