feat(#912): migrate normalization rules to the premise-conclusion schema#915
feat(#912): migrate normalization rules to the premise-conclusion schema#915yegor256 wants to merge 7 commits into
Conversation
Normalization rules now accept the same premise-conclusion schema used by
𝕄/𝒞/𝔻 ('match' + 'premises') alongside the legacy 'pattern' + 'where'
keys, and 'explain --normalize' renders them through the shared inference-
rule LaTeX path instead of the forked \phinoNormalizationRule macro.
- src/Yaml.hs: custom FromJSON Rule accepting 'match'/'premises' as well as
the legacy 'pattern'/'where'; 'premises' desugar into the build-term
'where' extras the engine already runs, so the rewriting engine is
untouched. 'verb'/'verbArgs' moved here from Dataize for reuse.
- src/LaTeX.hs: explainRule now folds into 'inference' (when/having become
the side condition, 'where' extensions become premise judgments, the
conclusion is 𝒩(match) ⟿ result via \phinoNormalize); trrule and
extraArgumentsToLatex removed.
- resources/normalize/*.yaml: migrated to 'match'; dot.yaml's 'where'
contextualize call is now a genuine premise.
- Legacy '--rule' files and all test packs keep working via the retained
legacy keys.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
The migrated 'explain --normalize' example tripped markdown-lint MD013; show a short sample of rules (an axiom, a premise rule, a conditioned rule) with elision, mirroring the other explain examples, so every line stays within 80 columns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
'explain --normalize' now emits the shared inference-rule layout
(\begin{phinoInference} with \phinoName/\phinoCondition/\phinoPremise/
\phinoConclusion, plus \phinoNormalize and \phinoContextualize) instead
of \phinoNormalizationRule. The latex.yml workflow compiles that output
through an ad-hoc preamble, so add the matching \newcommand/\newenvironment
definitions; otherwise pdflatex aborts with "Environment phinoInference
undefined".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
Benchmark comparisonRegression threshold: +20%. Same-runner A/B against the PR merge-base; the bench fixture is frozen across both runs.
OK: no case regressed above the +20% threshold. |
|
@maxonfjvipon good? |
maxonfjvipon
left a comment
There was a problem hiding this comment.
@yegor256 As I understand we still have some sort of a bridge? Are we planning to get rid of it? If so let's add pdd todo
| extraToLatex :: Y.Extra -> String | ||
| extraToLatex extra = fromMaybe assignment judgment | ||
| where | ||
| assignment = renderToLatex (extraToCST extra) defaultLatexContext |
There was a problem hiding this comment.
@yegor256 Let's add function types to all of the functions
There was a problem hiding this comment.
Added type signatures to the extraToLatex helpers. Also went further per @yegor256's note: dropped the legacy pattern/where schema entirely — the parser now accepts only match/premises, and all rule YAML (built-in normalization rules plus the test/--rule packs) is migrated. A premise is either a reduction judgment (verb-keyed) or a general build-term call (meta/function/args), so sed/concat/etc. still work and the rewriting engine is unchanged.
Generated by Claude Code
There was a problem hiding this comment.
Good questions — two clarifications:
ExtraArgument is still parsed and load-bearing, so we can't just drop it. The general build-term premise form — { meta, function, args }, used by sed / concat / size / random-tau / dataize / etc. — is parsed straight into Extra (FromJSON Extra/FromJSON ExtraArgument), and the rewriting engine consumes [Extra] directly in extraSubstitutions. So it isn't only a bridge; it's the engine's representation for build-term premises.
The real bridge is premiseToExtra: the verb-keyed premises (contextualize:/morph:/…) are parsed as the typed Operation/Premise (shared with 𝕄/𝒞/𝔻) and then desugared into Extra. So there are two premise representations and one converter between them. Fully removing it means having the engine consume Premise directly and retiring Extra/ExtraArgument — a larger change than this PR, so I've added a pdd todo at premiseToExtra (@todo #912:60min …) to track it. Happy to do it here instead if you'd prefer.
having is untouched — still parsed (o .:? "having") and applied as the post-premises side condition in Rule.hs (meetMaybeCondition rule.having after the premises run). The new schema only renamed pattern→match and where→premises; when/having are unchanged, so hone-maven-plugin keeps working. It's covered after the migration by part-of-after.yaml, which uses match + a concat premise + a having: { not: { part-of: … } } guard.
Generated by Claude Code
…912-fp1u0i # Conflicts: # src/Dataize.hs # src/LaTeX.hs
Per review, remove the old machinery entirely instead of keeping it
alongside the new schema. The Y.Rule parser now accepts only the
premise-conclusion schema ('match' + 'premises'); the legacy 'pattern' +
'where' keys are gone.
- src/Yaml.hs: Rule fields renamed pattern->match, where_->premises;
parser requires 'match' and parses 'premises'. A premise is either a
reduction judgment (verb-keyed, desugared via premiseToExtra) or a
general build-term call ('meta'/'function'/'args'); both reduce to the
same Extra the engine already consumes, so 'sed'/'concat'/etc. keep
working and the rewriting engine is untouched.
- src/Rule.hs, src/Rewriter.hs, src/LaTeX.hs, src/CLI/Helpers.hs: updated
to rule.match / rule.premises. Added type signatures to extraToLatex
helpers (review comment).
- Migrated every Y.Rule YAML to the new schema: resources/normalize/*,
test-resources/{yaml-packs,yaml-typos,cli}/*, and the inline custom
rules in test-resources/rewriter-packs/custom/*. Condition-packs use a
separate schema and are untouched.
- README: rule example and schema docs use match/premises.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
Renaming the 'pattern' rule field let hlint fully parse the module (the field name previously tripped a suppressed parse error), surfacing latent 'Eta reduce' warnings on the phino* printf helpers. Make them point-free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
|
@maxonfjvipon here's a summary of what this PR now does, after folding in your review feedback. Goal (#912)Migrate normalization rules to the same premise-conclusion schema used by 𝕄/𝒞/𝔻, and collapse the forked LaTeX backend so all five reduction systems render through one inference-rule layout. What changed
One open call worth your eyesThe normalization conclusion reuses the existing CIGreen locally on GHC 9.8.4: Generated by Claude Code |
maxonfjvipon
left a comment
There was a problem hiding this comment.
@yegor256 What about ExtraArgument type? It seems it's not parsed anymore and used only for bridge from new premises front end to old when+where front end? If so, it would be better to get rid of it. Also what about having? How this new premises approach resolves it? We need it for hone-maven-plugin
Mark the premiseToExtra bridge (verb-form Premise -> Extra) and the Extra/ExtraArgument representation for a follow-up that lets the engine consume Premise directly, per review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
Closes #912.
Normalization rules now use the same premise-conclusion schema as 𝕄/𝒞/𝔻, and
explainemits one uniform inference-rule LaTeX layout across all five reduction systems —explainRule/trrulecollapse into the sharedinferencehelper.What changed
src/Yaml.hs—FromJSON Ruleis now a custom parser that accepts the premise-conclusion keys (match+premises) as well as the legacy keys (pattern+where). Apremiseslist desugars (premiseToExtra) into the build-termwhereextras the engine already runs, so the rewriting engine itself is untouched.verb/verbArgsmoved here fromDataizeso both the desugaring and the dataizer share them.src/LaTeX.hs—explainRulefolds intoinference:when/havingbecome the side condition above the line,whereextensions become premise judgments (sodot'scontextualizeshows as 𝒞(n, formation) ⟿ e), and the conclusion is the one-step rewrite 𝒩(match) ⟿ result.trruleandextraArgumentsToLatexare removed.resources/normalize/*.yaml— migrated tomatch;dot.yaml'swherecontextualize call is now a genuinepremisesentry.README.md— theexplain --normalizeexample now shows the inference-rule output (consistent with the already-updated--contextualizeexample).test/CLISpec.hs—explainexpectations updated to the\begin{phinoInference}format.Design decisions (the issue left two open)
These were the two forks the issue raised; I picked defensible defaults and call them out here so they can be revisited in review:
Yaml.Ruleschema is also the general rewriting-rule schema used by the user-facing--rulefeature and ~50+ test packs (condition-packs/,rewriter-packs/,cli/). A hard rename would break all of those. Instead the parser accepts both schemas, the built-inresources/normalize/*.yamlfiles are migrated to the new form, and nothing else breaks. Becausepremisesdesugar to the existingwhereextras,src/Rule.hsandsrc/Rewriter.hsneeded no changes — the engine keeps consumingwhereextras unchanged. (Note:where/Extrais strictly more general thanpremises— it supports arbitrary build-term functions likesed/concat— which is why it stays the engine mechanism rather than being replaced.)\phinoNormalize{lhs}{rhs}, the same macro the 𝒩 premises of the other systems already use, so the inference layout is shared without inventing a separate one-step relation symbol. This keeps the relation in the 𝒩 family and needs no new.stymacro. Happy to switch to a distinct ↦ macro or unify with ⟿ if preferred.Verification
cabal test --ghc-options=-Werror→ 1073 examples, 0 failuresfourmolu --mode checkandhlintclean on all changed filesphino explain --normalize(inference output incl. thedotpremise) and arewrite --normalize(thedotrule fires correctly end-to-end)🤖 Generated with Claude Code
https://claude.ai/code/session_01NZ7UYvDx9A7XhFyc4NSPy9
Generated by Claude Code