A rule's own growth, tier and identity reach the registry - #1107
Merged
Conversation
The registry runs one thing and describes another. 27 of the 30 sets execute `MatchedRuleSet.ApplyHere`, and 29 take their `Rules` from `RuleRegistryGenerator` reading the `switch` those sets no longer run -- #825's open half, and what #746 tier 2 names as gating the rest. Repointing them at `AsAddressable()` is the change that fixes it, and this is the three things that had to be true of `AsAddressable()` first. **Growth was a proxy, and wrong 23 times in 322 in both directions.** It compared the lengths of the two rendered pattern strings, which is what the generator has to do -- it reads C# source text and has no tree to count nodes on. Here there is a tree. `Divf(var a, Sinf(var b))` to `Mulf(var a, Cosecantf(var b))` read as Expands because `Cosecantf` is a longer word than `Sinf`, and DivisionPreparing's two read as Collects for that reason reversed. Worse for the thirteen `Boolean` rules that *declare* their growth on a code-built replacement: with no string to measure, the proxy answered Unknown and threw the author's declaration away. `MatchedRule.Growth` is exact from `MatchPattern.NodeCount` and is now what is reported. **A tier per rule, and null where there is none.** A set's tier is the minimum over its rules, so one conditional rule makes a set of a hundred report as conditional -- which is why all thirty sets declare `SoundUnderAssumptions` and the field distinguishes nothing. Asked per rule, **181 of the 322 data rules are `Sound`** and 141 are conditional. `RewriteRule.Soundness` is `Soundness?` rather than `Soundness`: a `switch` arm declares no tier, so it has none of its own, and copying its set's down would assert a claim the source does not make. A caller wanting the effective tier writes `rule.Soundness ?? set.Soundness`, and the null is what tells it the second is a fallback. **And somewhere for the identity to live, which is what blocked the repoint.** The generator reads the comment above a `switch` arm into `RewriteRule.Description` -- `a / (b / c) = a * c / b` and its like, **95 of the registry's 407 arms**. A rule written as data has its identity in a comment too, and a comment is not readable at run time, so `AsAddressable` passed `description: null` and converting a set to data *lost* the description rather than keeping it. #746 tier 2 wants metadata "rich enough that v5.0 can render a step as a sentence", and of a rule's name, its rendered pattern and this, only this is the sentence. `MatchedRule` takes one now, a reversed rule prefixes rather than repeats it, and `RationalizeDenominator` -- the one set the registry already describes from its data form -- demonstrates it end to end: 0 of 2 rules reported an identity, now both do. `RewriteRuleSet` gains a constructor taking a `MatchedRuleSet` and reading both the runner and the rules off it, so a set cannot describe arms it does not execute. Also takes the factory call off the caller: the common-denominator family would otherwise be built twice, once for the delegate and once for the rules, and describe a different instance from the one that runs. **Not in this: the repoint itself.** It was written and measured and then taken back out. Pointing all 27 converted sets at `AsAddressable()` builds and leaves nine test failures, seven distinct, and they are informative rather than churn: the registry's arm count goes 407 -> 313 (the collapse the commutative patterns achieved -- `Common` 100 -> 62, `Boolean` 36 -> 20), rules start being named by their data names rather than by their rendered patterns, and the 95 descriptions all become null. So it changes public enumeration and needs a BREAKING-CHANGES entry, it invalidates `RuleConfluenceTest`'s index-named conflicts, and until the descriptions are ported set by set it is a net loss of metadata. That is the next change and it is one per set, with `MatchedRulesAgreeWithTheSwitchTest` as the net. Public surface: one additive property. Full suite: 8973 passed, 14 skipped, 0 failed. Part of #746 tier 2 and #825.
This was referenced Aug 30, 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.
The registry runs one thing and describes another. 27 of the 30 sets execute
MatchedRuleSet.ApplyHere; 29 take theirRulesfromRuleRegistryGeneratorreading theswitchthose sets no longer run. That is #825's open half, and #746 tier 2 names it as what gates the rest:
"
RuleRegistryGeneratorreadsPatternSourceandSourceLineoff aswitch's arms, so theswitchcannot be deleted even where it no longer runs."Pointing those sets at
AsAddressable()is the change that fixes it. This is the three things thathad to be true of
AsAddressable()first — each of them a defect on its own.Growth was a proxy, and wrong 23 times in 322 — in both directions
It compared the lengths of the two rendered pattern strings. That is what the generator has to
do: it reads C# source text and has no tree to count nodes on. Here there is a tree.
Boolean, 13 rulesCollectsUnknownCollapseTrigonometricFunctions, 2RearrangesExpandsCosecantfis a longer word thanSinf. Same number of nodesCommon, 4RearrangesUnknownDivisionPreparing, 2RearrangesCollectsInequalityEquality, 2RearrangesUnknownMatchedRule.Growthis exact fromMatchPattern.NodeCount, and is now what gets reported. Thethirteen
Booleanrules are the sharp case: those rules declare their growth, and a proxy overrendered text cannot see a declaration.
A tier per rule, and
nullwhere there is noneA set's tier is the minimum over its rules, so one conditional rule makes a set of a hundred
report as conditional. That is why all thirty sets declare
SoundUnderAssumptionsand the fielddistinguishes nothing. Asked per rule instead:
Sound— holds for every complex argument, nothing assumedSoundUnderAssumptionsRewriteRule.SoundnessisSoundness?, notSoundness. Aswitcharm declares no tier, so it hasnone of its own, and copying its set's down would assert a claim the source does not make. A caller
wanting the effective tier writes
rule.Soundness ?? set.Soundness— and thenullis what tells itthe second is a fallback rather than a measurement.
And somewhere for the identity to live, which is what blocked the repoint
The generator reads the comment above a
switcharm intoRewriteRule.Description—a / (b / c) = a * c / band its like, 95 of the registry's 407 arms. A rule written as data has its identityin a comment too, and a comment is not readable at run time, so
AsAddressablepasseddescription: null: converting a set to data lost the description rather than keeping it.#746 tier 2 wants metadata "rich enough that v5.0 can render a step as a sentence". Of a rule's
name (
dividing-by-a-quotient-multiplies-by-its-reciprocal), its rendered pattern(
Divf(var a, Divf(var b, var c))) and its identity (a / (b / c) = a * c / b), only the third isthe sentence.
MatchedRuletakes one now; a reversed rule prefixes rather than repeats it; andRationalizeDenominator— the one set the registry already describes from its data form —demonstrates it end to end: 0 of its 2 rules reported an identity, now both do, through the
public surface.
RewriteRuleSetalso gains a constructor taking aMatchedRuleSetand reading both the runner andthe rules off it, so a set cannot describe arms it does not execute. It takes the factory call off
the caller too: the common-denominator family — one definition read at three sort levels — would
otherwise be constructed twice, once for the delegate and once for the rules, and describe a
different instance from the one that runs.
What is deliberately not in this: the repoint itself
It was written, built and measured, and then taken back out. Pointing all 27 converted sets at
AsAddressable()compiles and leaves nine test failures, seven distinct — and they are informativerather than churn:
patterns achieved —
Common100 → 62,Trigonometric43 → 33,Boolean36 → 20,Factorization22 → 11. But it changes public enumeration and so needs a
BREAKING-CHANGES.mdentry.better and is still a change to a public string.
null, until they are ported. Until then the repoint is a net lossof metadata, which is the reason this PR stops here.
RuleConfluenceTest's recorded conflicts are index-named (Common[12,89]), and index 89 stopsexisting. Rule priorities, computed from the patterns rather than typed #1106's name-based recording is what replaces them.
So the repoint is the next change, and it is one set at a time with
MatchedRulesAgreeWithTheSwitchTestas the net. Deleting the ~27 deadswitchmethods is a separatedecision again, and not mine: those switches are the oracle that agreement test proves the data form
against, so deleting them deletes the evidence the exchange was safe.
State
Public surface: one additive property. Full suite: 8973 passed, 14 skipped, 0 failed.
Part of #746 tier 2 and #825.