🐛 Fix: Update PlanStepInput to match interpolated variables correctly…

…. Update appsettings.json to add missing function error options.

webapi/CopilotChat/Options/PlannerOptions.cs

@@ -1,4 +1,4 @@
-/// Copyright (c) Microsoft. All rights reserved.
+// Copyright (c) Microsoft. All rights reserved.
 using System.ComponentModel.DataAnnotations;
 using SemanticKernel.Service.CopilotChat.Models;
 
@@ -9,6 +9,26 @@ namespace SemanticKernel.Service.CopilotChat.Options;
 /// </summary>
 public class PlannerOptions
 {
+    /// <summary>
+    /// Whether to allow missing functions in plan on creation. If allowed, proposed plan will be sanitized of no-op functions.
+    /// Functions are considered missing if they're not available in the planner's kernel's context.
+    /// </summary>
+    public class MissingFunctionErrorOptions
+    {
+        /// <summary>
+        /// Flag to indicate if skips are allowed on MissingFunction error
+        /// If set to true, the plan will be created with missing functions as no-op steps that are filtered from the final proposed plan.
+        /// If this is set to false, the plan creation will fail if any functions are missing.
+        /// </summary>
+        public bool AllowRetries { get; set; } = true;
+
+        /// <summary>
+        /// Max retries allowed on MissingFunctionsError.
+        /// </summary>
+        /// [Range(1, 5)]
+        public int MaxRetriesAllowed { get; set; } = 3;
+    }
+
     public const string PropertyName = "Planner";
 
     /// <summary>
@@ -24,11 +44,9 @@ public class PlannerOptions
     public double? RelevancyThreshold { get; set; } = 0;
 
     /// <summary>
-    /// Whether to allow missing functions in the plan on creation then sanitize output. Functions are considered missing if they're not available in the planner's kernel's context.
-    /// If set to true, the plan will be created with missing functions as no-op steps that are filtered from the final proposed plan.
-    /// If this is set to false, the plan creation will fail if any functions are missing.
+    /// Options on how to handle missing functions in plan on creation.
     /// </summary>
-    public bool SkipOnMissingFunctionsError { get; set; } = true;
+    public MissingFunctionErrorOptions MissingFunctionError { get; set; } = new MissingFunctionErrorOptions();
 
     /// <summary>
     /// Whether to retry plan creation if LLM returned response that doesn't contain valid plan (e.g., invalid XML or JSON, contains missing function, etc.).

webapi/CopilotChat/Skills/ChatSkills/CopilotChatPlanner.cs

@@ -39,7 +39,14 @@ public class CopilotChatPlanner
     /// Flag to indicate that a variable is unknown and needs to be filled in by the user.
     /// This is used to flag any inputs that had dependencies from removed steps.
     /// </summary>
-    private static readonly string UNKNOWN_VARIABLE_FLAG = "$???";
+    private const string UNKNOWN_VARIABLE_FLAG = "$???";
+
+    /// <summary>
+    /// Regex to match variable names from plan parameters.
+    /// Matches: $variableName, $variable_name, $variable-name, $some_variable_Name, $variableName123, $variableName_123, $variableName-123
+    /// Does not match: $123variableName, $100 $200 
+    /// </summary>
+    private const string VARIABLE_REGEX = @"\$([A-Za-z]+[_-]*[\w]+)";
 
     /// <summary>
     /// Initializes a new instance of the <see cref="CopilotChatPlanner"/> class.
@@ -66,19 +73,19 @@ public async Task<Plan> CreatePlanAsync(string goal, ILogger logger)
             return new Plan(goal);
         }
 
-        Plan plan = this._plannerOptions!.Type == PlanType.Sequential
+        Plan plan = this._plannerOptions?.Type == PlanType.Sequential
                ? await new SequentialPlanner(
                     this.Kernel,
                     new SequentialPlannerConfig
                     {
                         RelevancyThreshold = this._plannerOptions?.RelevancyThreshold,
                         // Allow plan to be created with missing functions
-                        AllowMissingFunctions = this._plannerOptions!.SkipOnMissingFunctionsError
+                        AllowMissingFunctions = this._plannerOptions?.MissingFunctionError.AllowRetries ?? false
                     }
                 ).CreatePlanAsync(goal)
                : await new ActionPlanner(this.Kernel).CreatePlanAsync(goal);
 
-        return this._plannerOptions!.SkipOnMissingFunctionsError ? this.SanitizePlan(plan, plannerFunctionsView, logger) : plan;
+        return this._plannerOptions!.MissingFunctionError.AllowRetries ? this.SanitizePlan(plan, plannerFunctionsView, logger) : plan;
     }
 
     #region Private
@@ -104,7 +111,7 @@ private Plan SanitizePlan(Plan plan, FunctionsView availableFunctions, ILogger l
                 availableOutputs.AddRange(step.Outputs);
 
                 // Regex to match variable names
-                Regex variableRegEx = new(@"\$((\w+[_-]*)+)", RegexOptions.Singleline);
+                Regex variableRegEx = new(VARIABLE_REGEX, RegexOptions.Singleline);
 
                 // Check for any inputs that may have dependencies from removed steps
                 foreach (var input in step.Parameters)

webapi/CopilotChat/Skills/ChatSkills/ExternalInformationSkill.cs

@@ -120,10 +120,14 @@ public async Task<string> AcquireExternalInformationAsync(
             // Create a plan and set it in context for approval.
             var contextString = string.Join("\n", context.Variables.Where(v => v.Key != "userIntent").Select(v => $"{v.Key}: {v.Value}"));
             Plan? plan = null;
-            int retriesAvail = 3;
+            // Use default planner options if planner options are null.
+            var plannerOptions = this._planner.PlannerOptions ?? new PlannerOptions();
+            int retriesAvail = plannerOptions.MissingFunctionError.AllowRetries
+                ? plannerOptions.MissingFunctionError.MaxRetriesAllowed // Will always be at least 1
+                : plannerOptions.AllowRetriesOnInvalidPlan ? 1 : 0;
 
             do
-            { // TODO: [Issue #2256] Remove retry logic once Core team stabilizes planner
+            { // TODO: [Issue #2256] Remove InvalidPlan retry logic once Core team stabilizes planner
                 try
                 {
                     plan = await this._planner.CreatePlanAsync($"Given the following context, accomplish the user intent.\nContext:\n{contextString}\nUser Intent:{userIntent}", context.Log);
@@ -132,7 +136,7 @@ public async Task<string> AcquireExternalInformationAsync(
                 {
                     if (retriesAvail > 0)
                     {
-                        // PlanningExceptions are limited to one (1) pass as built-in stabilization. MissingFunctionErrors are allowed 3 retries as they are user-allowed skips.
+                        // PlanningExceptions are limited to one (1) pass as built-in stabilization. Retry limit of MissingFunctionErrors is user-configured.
                         retriesAvail = e is PlanningException ? 0 : retriesAvail--;
 
                         // Retry plan creation if LLM returned response that doesn't contain valid plan (invalid XML or JSON).
@@ -147,7 +151,7 @@ public async Task<string> AcquireExternalInformationAsync(
             {
                 // Merge any variables from ask context into plan parameters as these will be used on plan execution.
                 // These context variables come from user input, so they are prioritized.
-                if (this._planner.PlannerOptions!.Type == PlanType.Action)
+                if (plannerOptions.Type == PlanType.Action)
                 {
                     // Parameters stored in plan's top level state
                     this.MergeContextIntoPlan(context.Variables, plan.Parameters);
@@ -160,7 +164,7 @@ public async Task<string> AcquireExternalInformationAsync(
                     }
                 }
 
-                this.ProposedPlan = new ProposedPlan(plan, this._planner.PlannerOptions!.Type, PlanState.NoOp);
+                this.ProposedPlan = new ProposedPlan(plan, plannerOptions.Type, PlanState.NoOp);
             }
         }
 
@@ -172,17 +176,18 @@ public async Task<string> AcquireExternalInformationAsync(
     /// <summary>
     /// Retry on plan creation error if:
     /// 1. PlannerOptions.AllowRetriesOnInvalidPlan is true and exception contains error code InvalidPlan.
-    /// 2. PlannerOptions.SkipOnMissingFunctionsError is true and exception contains error code FunctionNotAvailable.
+    /// 2. PlannerOptions.MissingFunctionError.AllowRetries is true and exception contains error code FunctionNotAvailable.
     /// </summary>
     private bool IsRetriableError(Exception e)
     {
         var retryOnInvalidPlanError = e is PlanningException
-            && (e as PlanningException)!.ErrorCode == PlanningException.ErrorCodes.InvalidPlan
+            && ((e as PlanningException)!.ErrorCode == PlanningException.ErrorCodes.InvalidPlan
+                || (e.InnerException as PlanningException)!.ErrorCode == PlanningException.ErrorCodes.InvalidPlan)
             && this._planner.PlannerOptions!.AllowRetriesOnInvalidPlan;
 
         var retryOnMissingFunctionError = e is KernelException
             && (e as KernelException)!.ErrorCode == KernelException.ErrorCodes.FunctionNotAvailable
-            && this._planner.PlannerOptions!.SkipOnMissingFunctionsError;
+            && this._planner.PlannerOptions!.MissingFunctionError.AllowRetries;
 
         return retryOnMissingFunctionError || retryOnInvalidPlanError;
     }

webapi/appsettings.json

@@ -61,7 +61,10 @@
     // Whether to allow missing functions in the plan on creation then sanitize output. Functions are considered missing if they're not available in the planner's kernel's context.
     // If set to true, the plan will be created with missing functions as no-op steps that are filtered from the final proposed plan.
     // If this is set to false, the plan creation will fail if any functions are missing.
-    "SkipOnMissingFunctionsError": "true",
+    "MissingFunctionsError": {
+      "AllowSkips": "true",
+      "MaxRetriesAllowed": "3" // Max retries allowed on MissingFunctionsError. If set to 0, no retries will be attempted.
+    },
     // Whether to retry plan creation if LLM returned response with invalid plan.
     "AllowRetriesOnInvalidPlan": "true"
   },

webapp/src/components/chat/plan-viewer/PlanStepInput.tsx

@@ -27,10 +27,12 @@ const useClasses = makeStyles({
 
 // Regex to match interpolated variables in the form of $VARIABLE_NAME or $VARIABLE2_NAME.
 // Variables that are not interpolated will fail to match.
-// \$(\w+[_-]*)+ matches the variable name
-// (?=[^\w]+) is a positive lookahead matching the end of static string
-// (?:.+\s*) is a noncapturing group that matches the start of static string
-const INTERPOLATED_VARIABLE_REGEX = /((\$(\w+[_-]*)+)(?=[^\w]+))|((?:.+\s*)(\$(\w+[_-]*)+))/g;
+// \$[A-Za-z]+[_-]*[\w]+ matches the variable name
+// (?=([^-_\d\w])+) is a positive lookahead matching the end of static string (matches any character that is not a letter, number, underscore, or dash)
+// (?:.+\s*) is a noncapturing group that matches the start of static string (matches any character followed by whitespace)
+// Matches: "Interpolated $variable_name", "$variable_name Interpolated", "Interpolated $variable_name Interpolated"
+// Doesn't match: standalone variables (e.g. "$variable_name") or dollar amounts (e.g. "$1.00", "$100")
+const INTERPOLATED_VARIABLE_REGEX = /((\$[A-Za-z]+[_-]*[\w]+)(?=([^-_\d\w])+))|((?:.+\s*)(\$[A-Za-z]+[_-]*[\w]+))/g;
 
 interface PlanStepInputProps {
     input: IPlanInput;
@@ -113,7 +115,7 @@ export const PlanStepInput: React.FC<PlanStepInputProps> = ({
 
     return (
         <Badge
-            color={editsRequired ? 'danger' : 'informative'}
+            color={enableEdits && editsRequired ? 'danger' : 'informative'}
             shape="rounded"
             appearance="tint"
             className={classes.root}