From 2bd9953955aa6cf687ef003d21ea6bcbf28000fe Mon Sep 17 00:00:00 2001 From: samueltlg Date: Tue, 31 Mar 2026 23:21:16 +0100 Subject: [PATCH 1/5] fix: do not return 'null' in 'applyRule' if replacement has applied recursively/to operands --- src/compute-engine/boxed-expression/rules.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/compute-engine/boxed-expression/rules.ts b/src/compute-engine/boxed-expression/rules.ts index 434a7cdf..e3bee53b 100755 --- a/src/compute-engine/boxed-expression/rules.ts +++ b/src/compute-engine/boxed-expression/rules.ts @@ -902,7 +902,10 @@ export function applyRule( ? replace(expr, sub) : replace.subs(sub, { canonical }); - if (!result) return null; + if (!result) + return operandsMatched + ? { value: canonical ? expr.canonical : expr, because } + : null; // To aid in debugging, invoke onMatch when the rule matches onMatch?.(rule, expr, result); From 14af96927f2f169e2442d05e6c68dd3a3a8b3038 Mon Sep 17 00:00:00 2001 From: samueltlg Date: Fri, 3 Apr 2026 01:12:09 +0100 Subject: [PATCH 2/5] refactor: optimize rule application by exiting earlier on match-pattern canonical-variant wildcard loss --- src/compute-engine/boxed-expression/rules.ts | 33 ++++++++++---------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/compute-engine/boxed-expression/rules.ts b/src/compute-engine/boxed-expression/rules.ts index e3bee53b..5d5318ae 100755 --- a/src/compute-engine/boxed-expression/rules.ts +++ b/src/compute-engine/boxed-expression/rules.ts @@ -798,6 +798,21 @@ export function applyRule( if (!rule) return null; let canonical = options?.canonical ?? (expr.isCanonical || expr.isStructural); + // eslint-disable-next-line prefer-const + let { match, replace, condition, id, onMatch, onBeforeMatch } = rule; + const because = id ?? ''; + + const ce = expr.engine; + + if (canonical && match) { + const awc = getWildcards(match); + const canonicalMatch = match.canonical; + const bwc = getWildcards(canonicalMatch); + // If the canonical form of the match loses wildcards, this rule cannot match + // canonical expressions (they would already be simplified). Skip this rule. + if (!awc.every((x) => bwc.includes(x))) return null; + } + let operandsMatched = false; if (isFunction(expr) && options?.recursive) { @@ -821,26 +836,12 @@ export function applyRule( ) canonical = true; - expr = expr.engine.function(expr.operator, newOps, { + expr = ce.function(expr.operator, newOps, { form: canonical ? 'canonical' : 'raw', }); } } - // eslint-disable-next-line prefer-const - let { match, replace, condition, id, onMatch, onBeforeMatch } = rule; - const because = id ?? ''; - - if (canonical && match) { - const awc = getWildcards(match); - const canonicalMatch = match.canonical; - const bwc = getWildcards(canonicalMatch); - // If the canonical form of the match loses wildcards, this rule cannot match - // canonical expressions (they would already be simplified). Skip this rule. - if (!awc.every((x) => bwc.includes(x))) - return operandsMatched ? { value: expr, because } : null; - } - const useVariations = rule.useVariations ?? options?.useVariations ?? false; const matchPermutations = options?.matchPermutations ?? true; @@ -877,7 +878,7 @@ export function applyRule( }; try { - if (!condition(conditionSub, expr.engine)) + if (!condition(conditionSub, ce)) return operandsMatched ? { value: expr, because } : null; } catch (e) { console.error( From 591f870b64e8245e9c15b5f541684dd5bc9801a4 Mon Sep 17 00:00:00 2001 From: samueltlg Date: Wed, 8 Apr 2026 00:22:20 +0100 Subject: [PATCH 3/5] fix: parse object-Rule condition strings without '$' delimiters and canonicalize - Drop the '$...$' wrapping requirement so bare LaTeX condition strings parse correctly via ce.parse(). - Always canonicalize the condition so it can evaluate against substitutions. Co-Authored-By: samueltlg --- src/compute-engine/boxed-expression/rules.ts | 27 ++++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/compute-engine/boxed-expression/rules.ts b/src/compute-engine/boxed-expression/rules.ts index 5d5318ae..d68d8a7b 100755 --- a/src/compute-engine/boxed-expression/rules.ts +++ b/src/compute-engine/boxed-expression/rules.ts @@ -18,7 +18,6 @@ import type { } from '../global-types'; import { - asLatexString, isInequalityOperator, isRelationalOperator, } from '../latex-syntax/utils'; @@ -630,22 +629,16 @@ function boxRule( // Normalize the condition to a function let condFn: undefined | RuleConditionFunction; if (typeof condition === 'string') { - const latex = asLatexString(condition); - if (latex) { - // If the condition is a LaTeX string, it should be a predicate - // (an expression with a Boolean value). - const condPattern = - ce.parse(latex, { - form: options?.canonical ? 'canonical' : 'raw', - }) ?? ce.expr('Nothing'); - - // Substitute any unbound vars in the condition to a wildcard, - // then evaluate the condition - condFn = (x: BoxedSubstitution, _ce: ComputeEngine): boolean => { - const evaluated = condPattern.subs(x).evaluate(); - return isSymbol(evaluated, 'True'); - }; - } + // If the condition is a LaTeX string, it should be a predicate + // (an expression with a Boolean value). + const condPattern = ce.parse(condition) ?? ce.expr('Nothing'); + + // Substitute any unbound vars in the condition to a wildcard, + // then evaluate the condition + condFn = (x: BoxedSubstitution, _ce: ComputeEngine): boolean => { + const evaluated = condPattern.subs(x).evaluate(); + return isSymbol(evaluated, 'True'); + }; } else { if (condition !== undefined && typeof condition !== 'function') throw new Error( From 2a44d4d71c2564f0f24c51ac979cf34b9d1df29d Mon Sep 17 00:00:00 2001 From: samueltlg Date: Thu, 9 Apr 2026 23:52:29 +0100 Subject: [PATCH 4/5] refactor: singular documentative type-cast --- src/compute-engine/boxed-expression/boxed-function.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compute-engine/boxed-expression/boxed-function.ts b/src/compute-engine/boxed-expression/boxed-function.ts index 0aa658c2..4d544f6b 100644 --- a/src/compute-engine/boxed-expression/boxed-function.ts +++ b/src/compute-engine/boxed-expression/boxed-function.ts @@ -1251,7 +1251,7 @@ export class BoxedFunction let evaluateFn: Expression | Promise | undefined; try { - const opts = { + const opts: Partial & { engine?: ComputeEngine } = { numericApproximation, engine, signal: options?.signal, From 59c7cd9dd5c5d9d5e33a725133eedf94b5f05d3b Mon Sep 17 00:00:00 2001 From: Arno Gourdol Date: Sat, 9 May 2026 18:12:24 -0700 Subject: [PATCH 5/5] test: add regression tests for cherry-picked rule-application fixes Three tests covering the fixes from PR #301 commits 2bd99539, 591f870b: - applyRule preserves operand rewrites when function-replace returns undefined at top (tests the !result early-return path). - Object-rule condition string without '$' delimiters is honored (tests parsing of bare LaTeX condition strings). - Object-rule condition string is canonicalized so it can evaluate (tests that non-trivial conditions like 'a^2 > 0' work). Co-Authored-By: Claude Opus 4.7 (1M context) --- test/compute-engine/rules.test.ts | 61 +++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/compute-engine/rules.test.ts b/test/compute-engine/rules.test.ts index 115d6eb2..44b4d65e 100644 --- a/test/compute-engine/rules.test.ts +++ b/test/compute-engine/rules.test.ts @@ -179,3 +179,64 @@ describe('matchPermutations option', () => { ).toMatchInlineSnapshot(`["Multiply", 2, "x"]`); }); }); + +// Regression tests for fixes cherry-picked from PR #301. +describe('PR #301 cherry-picked fixes', () => { + it('applyRule preserves operand rewrites when function-replace returns undefined at top', () => { + // Pattern matches at both inner and outer Add. Function-replace rewrites the + // inner Add(y, z) but returns undefined for the outer (where _a is symbol x). + // Before the fix: result is null (operand rewrites silently discarded). + // After the fix: result is the operand-rewritten expression. + const expr = ce.expr(['Add', 'x', ['Add', 'y', 'z']], { form: 'raw' }); + + const rule = { + match: ['Add', '_a', '_b'], + replace: (_e, { _a, _b }) => { + if (_a.symbol === 'x') return undefined; + return ce.box(['Multiply', _a, _b]); + }, + }; + + const result = expr.replace(rule, { + recursive: true, + matchPermutations: false, + }); + expect(result).not.toBeNull(); + expect(result?.toString()).toBe('x + y * z'); + }); + + it('object-rule condition string without $ delimiters is honored', () => { + // Before the '$'-delimiter fix: the condition string was silently dropped, + // so the rule applied unconditionally. + const expr = ce.box(['Add', 3, 'x']); + + const ruleAccept = { + match: ['Add', '_a', 'x'], + replace: ['Multiply', '_a', 2], + condition: 'a > 0', + }; + expect(expr.replace(ruleAccept)?.toString()).toBe('6'); + + const ruleReject = { + match: ['Add', '_a', 'x'], + replace: ['Multiply', '_a', 2], + condition: 'a < 0', + }; + // 3 < 0 is false, so the rule should not apply. + expect(expr.replace(ruleReject)).toBeNull(); + }); + + it('object-rule condition string is canonicalized so it can evaluate', () => { + // Before the canonicalization fix: a non-trivial condition (e.g. 'a^2 > 0') + // was parsed as 'raw' and could not evaluate, so the rule never applied. + const expr = ce.box(['Add', 4, 'x']); + + const rule = { + match: ['Add', '_a', 'x'], + replace: ['Multiply', '_a', 2], + condition: 'a^2 > 0', + }; + // 4^2 = 16 > 0 → condition evaluates to True → rule applies. + expect(expr.replace(rule)?.toString()).toBe('8'); + }); +});