Cherry-pick rule-application fixes from #301

src/compute-engine/boxed-expression/boxed-function.ts

@@ -1251,7 +1251,7 @@ export class BoxedFunction
 
       let evaluateFn: Expression | Promise<Expression> | undefined;
       try {
-        const opts = {
+        const opts: Partial<EvaluateOptions> & { engine?: ComputeEngine } = {
           numericApproximation,
           engine,
           signal: options?.signal,

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(
@@ -798,6 +791,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 +829,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 +871,7 @@ export function applyRule(
     };
 
     try {
-      if (!condition(conditionSub, expr.engine))
+      if (!condition(conditionSub, ce))
         return operandsMatched ? { value: expr, because } : null;
     } catch (e) {
       console.error(
@@ -902,7 +896,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);

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');
+  });
+});