Fix NFA compilation failure for grammars with shared optional prefixes

ts/docs/architecture/core/actionGrammar.md

@@ -602,10 +602,13 @@ tail contract, so the non-tail wrapper builder is only reached when
 `tailFactoring` is off.
 
 **NFA compatibility.** Tail RulesParts are understood only by the
-NFA-interpreter matcher (`grammarMatcher.ts`). The NFA compiler /
-DFA path (`nfaCompiler.ts`) explicitly throws on encountering one,
-so consumers that route through the NFA/DFA path must leave
-`tailFactoring` off.
+AST-walking matcher (`grammarMatcher.ts`). The NFA compiler / DFA path
+(`nfaCompiler.ts`) explicitly throws on encountering one - this is a known gap and is not yet implemented. NFAs are naturally well-suited to sharing prefix states, but the equivalent frame-sharing
+/ variable-forwarding wiring for that backend hasn't been built yet.
+Until it is, consumers that route through the NFA/DFA path must leave
+`tailFactoring` off (and `promoteTailRulesParts`, which produces the
+same tail-call shape) when calling `optimizeGrammar` /
+`loadGrammarRules` directly.
 
 **No fixed-point loop.** Factoring is applied once per group of
 alternatives — the trie's grouping converges in a single pass and
@@ -1518,6 +1521,17 @@ matched text as a string. For example, `"hello"` produces `"hello"`.
 explicit value expression produce no value — the compiler warns about
 this because the output is ambiguous.
 
+The NFA compiler (`nfaCompiler.ts`) mirrors this same "no value unless
+actually needed" principle via the shared `deriveEffectiveValue`
+(`grammarValueDeriver.ts`), which forwards a rule's
+single variable-bearing part (whatever its shape - a single-part rule,
+or a multi-part rule reshaped by `factorCommonPrefixes`), and only
+treats an ambiguous or missing implicit value as a hard error when the
+value is actually required — i.e. for top-level action rules, or nested
+rules a parent captures via `$(name:<Rule>)`. A nested, uncaptured rule
+with an ambiguous shape is harmless (nothing reads its value) and
+silently resolves to "no value," exactly like the `none` kind above.
+
 ### Design Principles
 
 1. **Strict Conformance** — the purpose of type checking is to ensure

ts/packages/actionGrammar/src/grammarOptimizer.ts

@@ -22,6 +22,7 @@ import {
 import { leadingWordBoundaryScriptPrefix } from "./spacingScripts.js";
 import { leadingNonSeparatorRun } from "./grammarMatcher.js";
 import { getDispatchEffectiveMembers } from "./dispatchHelpers.js";
+import { deriveEffectiveValue } from "./grammarValueDeriver.js";
 import {
     globalPhraseSetRegistry,
     PhraseSetMatcher,
@@ -3238,14 +3239,11 @@ function trySubstituteMembers(
     let bailed = false;
     const rewriteOne = (m: GrammarRule): GrammarRule => {
         const renamed = renameAllChildBindings(m.parts, m.value, renameState);
-        const effective =
-            renamed.value !== undefined
-                ? renamed.value
-                : getImplicitDefaultValue({
-                      ...m,
-                      parts: renamed.parts,
-                      value: renamed.value,
-                  });
+        const effective = deriveEffectiveValue({
+            ...m,
+            parts: renamed.parts,
+            value: renamed.value,
+        });
         if (effective === undefined) {
             // Member has no explicit value and we can't synthesize
             // one to substitute (e.g. unbound single-part
@@ -3299,48 +3297,6 @@ function trySubstituteMembers(
     return { dispatch: newDispatch, alternatives: fallback };
 }
 
-/**
- * Compute a value expression equivalent to what the matcher's
- * implicit-default rule would produce for `rule` if it were matched
- * standalone.  Returns `undefined` if the rule doesn't have a
- * structurally-expressible default (e.g. an unbound single-part
- * `phraseSet` / `string` whose value depends on the matched text).
- *
- * Used by the value-substitution branch of `tryPromoteTrailing` to
- * fold each member's effective value into the parent's value
- * expression.
- */
-function getImplicitDefaultValue(
-    rule: GrammarRule,
-): CompiledValueNode | undefined {
-    if (rule.value !== undefined) return rule.value;
-    const parts = rule.parts;
-    if (parts.length === 0) return undefined;
-    if (parts.length === 1) {
-        // Single-part rule: matcher forwards the part's value.  For
-        // var-bearing parts we can express that as a variable
-        // reference; for unbound `string` / `phraseSet` (whose
-        // matcher value derives from the matched text) and unbound
-        // `rules` (whose value derives from the inner match) we
-        // can't reify the result without changing the AST, so bail.
-        const name = parts[0].variable;
-        return name !== undefined ? { type: "variable", name } : undefined;
-    }
-    // Multi-part: implicit default requires exactly one
-    // var-bearing part.  Same predicate as the inliner's
-    // binding-friendly check for multi-part children.
-    let theVar: string | undefined;
-    for (const p of parts) {
-        const name = p.variable;
-        if (name === undefined) continue;
-        if (theVar !== undefined) return undefined;
-        theVar = name;
-    }
-    return theVar !== undefined
-        ? { type: "variable", name: theVar }
-        : undefined;
-}
-
 // ─────────────────────────────────────────────────────────────────────────────
 // Structural validation: RulesPart.tailCall contract
 // ─────────────────────────────────────────────────────────────────────────────

ts/packages/actionGrammar/src/grammarValueDeriver.ts

@@ -0,0 +1,45 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+/**
+ * Value-derivation logic shared by grammarOptimizer.ts and
+ * nfaCompiler.ts: computing a rule's effective value expression.
+ */
+
+import type { CompiledValueNode, GrammarRule } from "./grammarTypes.js";
+
+/**
+ * A rule's explicit `->` value when present, otherwise the implicit
+ * value from a single variable-bearing part across the rule's `parts`.
+ * Throws if a value is required and missing/ambiguous. Not every rule
+ * needs a value - only when `requireValue` is set (top-level action
+ * rules, or nested rules captured by a parent variable) do
+ * ambiguous/missing values throw; otherwise both resolve to
+ * `undefined`. `describeRule` is only invoked when an error is thrown.
+ */
+export function deriveEffectiveValue(
+    rule: GrammarRule,
+    requireValue = false,
+    describeRule?: () => string,
+): CompiledValueNode | undefined {
+    if (rule.value !== undefined) {
+        return rule.value;
+    }
+    const variableParts = rule.parts.filter((p) => p.variable !== undefined);
+    if (variableParts.length === 1) {
+        return { type: "variable", name: variableParts[0].variable! };
+    }
+    if (!requireValue) {
+        return undefined;
+    }
+    const description = describeRule?.() ?? "";
+    const term = rule.parts.length === 1 ? "term" : "terms";
+    const reason =
+        variableParts.length > 1
+            ? "more than one part carries a variable - the implicit value is ambiguous"
+            : "no part carries a variable";
+    throw new Error(
+        `Internal error: ${description} has ${rule.parts.length} ${term} but no value expression, and ${reason}. ` +
+            "Rules must have an explicit value expression (using ->) unless exactly one part carries a variable.",
+    );
+}

ts/packages/actionGrammar/src/grammarValueTypeValidator.ts

@@ -500,11 +500,8 @@ export function classifyRuleValue(rule: GrammarRule): RuleValueKind {
     }
     const variableParts = rule.parts.filter((p) => p.variable !== undefined);
     if (variableParts.length === 1) {
-        return {
-            kind: "variable",
-            variableName: variableParts[0].variable!,
-            part: variableParts[0],
-        };
+        const part = variableParts[0];
+        return { kind: "variable", variableName: part.variable!, part };
     }
     if (variableParts.length === 0 && rule.parts.length === 1) {
         const part = rule.parts[0];

ts/packages/actionGrammar/src/nfaCompiler.ts

@@ -21,6 +21,7 @@ import {
     ValueExpression,
 } from "./environment.js";
 import { normalizeToken } from "./nfaMatcher.js";
+import { deriveEffectiveValue } from "./grammarValueDeriver.js";
 
 // Scripts that require a word-boundary separator between adjacent tokens.
 // CJK and other logographic/syllabic scripts are NOT included — no separator needed.
@@ -353,6 +354,21 @@ function normalizeRule(
     rule: GrammarRule,
     cache: Map<GrammarRule[], GrammarRule[]>,
 ): GrammarRule {
+    // tailCall is an NYI NFA/DFA backend gap, unrelated to value
+    // derivation. Checked here since normalizeRule visits every rule
+    // exactly once (top-level and nested) before compilation begins -
+    // the single earliest point to reject it.
+    const tailCallPart = rule.parts.find(
+        (p): p is RulesPart => p.type === "rules" && !!p.tailCall,
+    );
+    if (tailCallPart !== undefined) {
+        throw new Error(
+            `normalizeGrammar: tail RulesPart (name='${tailCallPart.name ?? "<unnamed>"}') is not yet supported by the ` +
+                "NFA/DFA backend - disable `tailFactoring` / `promoteTailRulesParts` in the " +
+                "grammar optimizer for NFA/DFA paths.",
+        );
+    }
+
     // First, normalize all nested RulesParts
     const normalizedParts = rule.parts.map((part) =>
         normalizePart(part, cache),
@@ -450,29 +466,6 @@ function stripDispatch(part: RulesPart): RulesPart {
     });
 }
 
-/**
- * Check if a rule is a single-variable rule (e.g., <ArtistName> = $(x:wildcard);)
- * Such rules should implicitly produce their variable's value: -> $(x)
- */
-function isSingleVariableRule(rule: GrammarRule): { variable: string } | false {
-    // A single-variable rule has:
-    // 1. No explicit value expression
-    // 2. Single part that is a wildcard or number with a variable
-    if (rule.value) {
-        return false; // Has explicit value
-    }
-    if (rule.parts.length !== 1) {
-        return false; // Multiple parts
-    }
-    const part = rule.parts[0];
-    if (part.type === "wildcard" || part.type === "number") {
-        if (part.variable) {
-            return { variable: part.variable };
-        }
-    }
-    return false;
-}
-
 /**
  * Collect all variable names from a rule's parts
  * Returns them in order of appearance
@@ -614,24 +607,12 @@ export function compileGrammarToNFA(grammar: Grammar, name?: string): NFA {
     ) {
         const rule = normalizedGrammar.alternatives[ruleIndex];
 
-        // VALIDATION: Multi-term rules MUST have value expressions
-        // Single-term rules can omit value expressions (they inherit from the term)
-        if (rule.parts.length > 1 && !rule.value) {
-            throw new Error(
-                `Grammar rule at index ${ruleIndex} has ${rule.parts.length} terms but no value expression. ` +
-                    `Multi-term rules must have an explicit value expression (using ->).`,
-            );
-        }
-
-        // Check for single-variable rules like <ArtistName> = $(x:wildcard);
-        // These should implicitly produce their variable's value: -> x
-        let effectiveValue = rule.value;
-        if (!effectiveValue) {
-            const singleVar = isSingleVariableRule(rule);
-            if (singleVar) {
-                effectiveValue = { type: "variable", name: singleVar.variable };
-            }
-        }
+        // Check for single-variable / factored rules with no explicit -> value.
+        const effectiveValue = deriveEffectiveValue(
+            rule,
+            true,
+            () => `Grammar rule at index ${ruleIndex}`,
+        );
 
         const ruleEntry = builder.createState(false);
 
@@ -1391,13 +1372,6 @@ function compileRulesPart(
     checkedVariables?: Set<string>,
     overrideVariableName?: string,
 ): number {
-    if (part.tailCall) {
-        throw new Error(
-            `compileRulesPart: tail RulesParts are not supported by the NFA compiler ` +
-                `(part.name='${part.name ?? "<unnamed>"}'). ` +
-                "Disable `tailFactoring` in the grammar optimizer for NFA/DFA paths.",
-        );
-    }
     if (part.alternatives.length === 0) {
         // Empty rules - epsilon transition
         builder.addEpsilonTransition(fromState, toState);
@@ -1472,13 +1446,6 @@ function compileRulesPartWithSlots(
     toState: number,
     context: RuleCompilationContext,
 ): number {
-    if (part.tailCall) {
-        throw new Error(
-            `compileRulesPartWithSlots: tail RulesParts are not supported by the NFA compiler ` +
-                `(part.name='${part.name ?? "<unnamed>"}'). ` +
-                "Disable `tailFactoring` in the grammar optimizer for NFA/DFA paths.",
-        );
-    }
     if (part.alternatives.length === 0) {
         // Empty rules - epsilon transition
         builder.addEpsilonTransition(fromState, toState);
@@ -1528,15 +1495,7 @@ function compileRulesPartWithSlots(
 
         // Compile and store the action value on the entry state FIRST
         // We need to know if there's a value before deciding about environments
-        // Passthrough normalization is done upfront, so rules already have explicit values
-        // Just check for single-variable rules like <ArtistName> = $(x:wildcard);
-        let effectiveValue = rule.value;
-        if (!effectiveValue) {
-            const singleVar = isSingleVariableRule(rule);
-            if (singleVar) {
-                effectiveValue = { type: "variable", name: singleVar.variable };
-            }
-        }
+        const effectiveValue = deriveEffectiveValue(rule);
 
         let compiledValue: ValueExpression | undefined;
         let nestedCompletionActionName: string | undefined;

ts/packages/actionGrammar/test/grammarOptimizerTailFactoring.spec.ts

@@ -368,7 +368,33 @@ describe("Grammar Optimizer - tailFactoring + NFA compatibility", () => {
             },
         });
         expect(() => compileGrammarToNFA(optimized)).toThrow(
-            /tail RulesParts are not supported by the NFA compiler/,
+            /tail RulesPart .* is not yet supported by the NFA\/DFA backend/,
+        );
+    });
+
+    // Regression test: a rule whose entire `parts` array is just the
+    // tailCall RulesPart (no prefix) is `isPassthroughRule`-eligible, so
+    // it must be caught before that normalization transform runs.
+    it("NFA compile of a single-part tailCall rule throws the same descriptive error", async () => {
+        const { compileGrammarToNFA } = await import("../src/nfaCompiler.js");
+        const memberA: GrammarRule = {
+            parts: [createStringPart(["alpha"])],
+            value: { type: "literal", value: "a" },
+        };
+        const memberB: GrammarRule = {
+            parts: [createStringPart(["beta"])],
+            value: { type: "literal", value: "b" },
+        };
+        const tailPart: RulesPart = {
+            type: "rules",
+            optional: undefined,
+            variable: undefined,
+            alternatives: [memberA, memberB],
+            tailCall: true,
+        };
+        const grammar: Grammar = { alternatives: [{ parts: [tailPart] }] };
+        expect(() => compileGrammarToNFA(grammar)).toThrow(
+            /tail RulesPart .* is not yet supported by the NFA\/DFA backend/,
         );
     });
 });