From 2d4abaad7a2c42ca04bbcce21b8ecb8490b27d48 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 17 May 2024 15:20:47 -0400 Subject: [PATCH 1/2] Update LiquidPromptTemplate to use Fluid instead of Scriban --- dotnet/Directory.Packages.props | 2 +- .../LiquidPromptTemplate.cs | 100 +++++++++++------- .../PromptTemplates.Liquid.csproj | 2 +- 3 files changed, 66 insertions(+), 38 deletions(-) diff --git a/dotnet/Directory.Packages.props b/dotnet/Directory.Packages.props index 0f45264e4068..d8a16fe3d0af 100644 --- a/dotnet/Directory.Packages.props +++ b/dotnet/Directory.Packages.props @@ -85,7 +85,7 @@ - + diff --git a/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs b/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs index abb2b47aef4b..48ecf1ad8585 100644 --- a/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs +++ b/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs @@ -8,8 +8,8 @@ using System.Threading; using System.Threading.Tasks; using System.Web; -using Scriban; -using Scriban.Syntax; +using Fluid; +using Fluid.Ast; namespace Microsoft.SemanticKernel.PromptTemplates.Liquid; @@ -18,12 +18,14 @@ namespace Microsoft.SemanticKernel.PromptTemplates.Liquid; /// internal sealed partial class LiquidPromptTemplate : IPromptTemplate { + private static readonly FluidParser s_parser = new(); + private const string ReservedString = ":"; private const string ColonString = ":"; private const char LineEnding = '\n'; private readonly PromptTemplateConfig _config; private readonly bool _allowDangerouslySetContent; - private readonly Template _liquidTemplate; + private readonly IFluidTemplate _liquidTemplate; private readonly Dictionary _inputVariables; #if NET @@ -55,12 +57,8 @@ public LiquidPromptTemplate(PromptTemplateConfig config, bool allowDangerouslySe // Parse the template now so we can check for errors, understand variable usage, and // avoid having to parse on each render. - this._liquidTemplate = Template.ParseLiquid(config.Template); - if (this._liquidTemplate.HasErrors) - { - throw new ArgumentException($"The template could not be parsed:{Environment.NewLine}{string.Join(Environment.NewLine, this._liquidTemplate.Messages)}"); - } - Debug.Assert(this._liquidTemplate.Page is not null); + this._liquidTemplate = s_parser.Parse(config.Template) ?? + throw new ArgumentException("The template could not be parsed."); // Ideally the prompty author would have explicitly specified input variables. If they specified any, // assume they specified them all. If they didn't, heuristically try to find the variables, looking for @@ -92,7 +90,7 @@ public async Task RenderAsync(Kernel kernel, KernelArguments? arguments { Verify.NotNull(kernel); cancellationToken.ThrowIfCancellationRequested(); - var variables = this.GetVariables(arguments); + var variables = this.GetTemplateContext(arguments); var renderedResult = this._liquidTemplate.Render(variables); // parse chat history @@ -154,9 +152,12 @@ private string ReplaceReservedStringBackToColonIfNeeded(string text) /// /// Gets the variables for the prompt template, including setting any default values from the prompt config. /// - private Dictionary GetVariables(KernelArguments? arguments) + private TemplateContext GetTemplateContext(KernelArguments? arguments) { - var result = new Dictionary(); + var ctx = new TemplateContext(new TemplateOptions() + { + MemberAccessStrategy = new UnsafeMemberAccessStrategy() { MemberNameStrategy = MemberNameStrategies.SnakeCase }, + }); foreach (var p in this._config.InputVariables) { @@ -165,7 +166,7 @@ private string ReplaceReservedStringBackToColonIfNeeded(string text) continue; } - result[p.Name] = p.Default; + ctx.SetValue(p.Name, p.Default); } if (arguments is not null) @@ -177,17 +178,17 @@ private string ReplaceReservedStringBackToColonIfNeeded(string text) var value = (object)kvp.Value; if (this.ShouldReplaceColonToReservedString(this._config, kvp.Key, kvp.Value)) { - result[kvp.Key] = value.ToString()?.Replace(ColonString, ReservedString); + ctx.SetValue(kvp.Key, value.ToString()?.Replace(ColonString, ReservedString)); } else { - result[kvp.Key] = value; + ctx.SetValue(kvp.Key, value); } } } } - return result; + return ctx; } private bool ShouldReplaceColonToReservedString(PromptTemplateConfig promptTemplateConfig, string propertyName, object? propertyValue) @@ -209,20 +210,23 @@ private bool ShouldReplaceColonToReservedString(PromptTemplateConfig promptTempl } /// - /// Visitor for looking for variables that are only + /// Visitor for looking for variables that are only /// ever read and appear to represent very simple strings. If any variables - /// other than that are found, none are returned. + /// other than that are found, none are returned. This only handles very basic + /// cases where the template doesn't contain any more complicated constructs; + /// the heuristic can be improved over time. /// - private sealed class SimpleVariablesVisitor : ScriptVisitor + private sealed class SimpleVariablesVisitor : AstVisitor { private readonly HashSet _variables = new(StringComparer.OrdinalIgnoreCase); + private readonly Stack _statementStack = new(); private bool _valid = true; - public static HashSet InferInputs(Template template) + public static HashSet InferInputs(IFluidTemplate template) { var visitor = new SimpleVariablesVisitor(); - template.Page.Accept(visitor); + visitor.VisitTemplate(template); if (!visitor._valid) { visitor._variables.Clear(); @@ -231,27 +235,51 @@ public static HashSet InferInputs(Template template) return visitor._variables; } - public override void Visit(ScriptVariableGlobal node) + public override Statement Visit(Statement statement) + { + if (!this._valid) + { + return statement; + } + + this._statementStack.Push(statement); + try + { + return base.Visit(statement); + } + finally + { + this._statementStack.Pop(); + } + } + + protected override Expression VisitMemberExpression(MemberExpression memberExpression) { - if (this._valid) + if (memberExpression.Segments.Count == 1 && memberExpression.Segments[0] is IdentifierSegment id) { - switch (node.Parent) + bool isValid = true; + + if (this._statementStack.Count > 0) { - case ScriptAssignExpression assign when ReferenceEquals(assign.Target, node): - case ScriptForStatement forLoop: - case ScriptMemberExpression member: - // Unsupported use found; bail. - this._valid = false; - return; - - default: - // Reading from a simple variable. - this._variables.Add(node.Name); - break; + switch (this._statementStack.Peek()) + { + case ForStatement: + case AssignStatement assign when string.Equals(id.Identifier, assign.Identifier, StringComparison.OrdinalIgnoreCase): + isValid = false; + break; + } } - base.DefaultVisit(node); + if (isValid) + { + this._variables.Add(id.Identifier); + return base.VisitMemberExpression(memberExpression); + } } + + // Found something unsupported. Bail. + this._valid = false; + return memberExpression; } } } diff --git a/dotnet/src/Extensions/PromptTemplates.Liquid/PromptTemplates.Liquid.csproj b/dotnet/src/Extensions/PromptTemplates.Liquid/PromptTemplates.Liquid.csproj index 632202ce2e4e..1a8827cbbb09 100644 --- a/dotnet/src/Extensions/PromptTemplates.Liquid/PromptTemplates.Liquid.csproj +++ b/dotnet/src/Extensions/PromptTemplates.Liquid/PromptTemplates.Liquid.csproj @@ -23,6 +23,6 @@ - + \ No newline at end of file From b1ab1d995883499a9128d89edb6e6d469cbcc1b1 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 17 May 2024 23:37:49 -0400 Subject: [PATCH 2/2] Address PR feedback --- .../LiquidPromptTemplate.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs b/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs index 48ecf1ad8585..0e9193f290d7 100644 --- a/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs +++ b/dotnet/src/Extensions/PromptTemplates.Liquid/LiquidPromptTemplate.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Text; using System.Text.RegularExpressions; using System.Threading; @@ -19,6 +18,10 @@ namespace Microsoft.SemanticKernel.PromptTemplates.Liquid; internal sealed partial class LiquidPromptTemplate : IPromptTemplate { private static readonly FluidParser s_parser = new(); + private static readonly TemplateOptions s_templateOptions = new() + { + MemberAccessStrategy = new UnsafeMemberAccessStrategy() { MemberNameStrategy = MemberNameStrategies.SnakeCase }, + }; private const string ReservedString = ":"; private const string ColonString = ":"; @@ -57,8 +60,12 @@ public LiquidPromptTemplate(PromptTemplateConfig config, bool allowDangerouslySe // Parse the template now so we can check for errors, understand variable usage, and // avoid having to parse on each render. - this._liquidTemplate = s_parser.Parse(config.Template) ?? - throw new ArgumentException("The template could not be parsed."); + if (!s_parser.TryParse(config.Template, out this._liquidTemplate, out string error)) + { + throw new ArgumentException(error is not null ? + $"The template could not be parsed:{Environment.NewLine}{error}" : + "The template could not be parsed."); + } // Ideally the prompty author would have explicitly specified input variables. If they specified any, // assume they specified them all. If they didn't, heuristically try to find the variables, looking for @@ -154,10 +161,7 @@ private string ReplaceReservedStringBackToColonIfNeeded(string text) /// private TemplateContext GetTemplateContext(KernelArguments? arguments) { - var ctx = new TemplateContext(new TemplateOptions() - { - MemberAccessStrategy = new UnsafeMemberAccessStrategy() { MemberNameStrategy = MemberNameStrategies.SnakeCase }, - }); + var ctx = new TemplateContext(s_templateOptions); foreach (var p in this._config.InputVariables) {