Skip to content

Commit

Permalink
.Net: Template Engine interface cleanup (#2108)
Browse files Browse the repository at this point in the history
### Description
Primary change: removing some methods from the ITemplateEngine
interface, to reduce the surface area and make it less taxing for other
TemplateEngine implementations. The methods that have been removed are
still there in our implementation -- they've just been marked internal.

Also making some property fields nullable: `Description` and
`DefaultValue`. These being `null` implies they're never set, whereas a
default value of "empty string" may be a valid implementation choice in
a rendered prompt.

There's more to be done here, but this is an easy first pass.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [X] The code builds clean without any errors or warnings
- [X] The PR follows the [SK Contribution
Guidelines](https:/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https:/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#dev-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄

Co-authored-by: Dmytro Struk <[email protected]>
  • Loading branch information
shawncal and dmytrostruk authored Jul 21, 2023
1 parent 798ebc5 commit 264e716
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ private void PopulateList(StringBuilder list, IDictionary<string, List<FunctionV
// Function parameters
foreach (var p in func.Parameters)
{
var description = string.IsNullOrEmpty(p.Description) ? p.Name : p.Description;
var description = string.IsNullOrEmpty(p.Description) ? p.Name : p.Description!;
var defaultValueString = string.IsNullOrEmpty(p.DefaultValue) ? string.Empty : $" (default value: {p.DefaultValue})";
list.AppendLine($"Parameter \"{p.Name}\": {AddPeriod(description)} {defaultValueString}");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.SemanticKernel.SkillDefinition;
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public sealed class ParameterView
{
private string _name = "";
private string _name = string.Empty;

/// <summary>
/// Parameter name. Alphanumeric chars + "_" only.
Expand All @@ -30,12 +30,12 @@ public string Name
/// <summary>
/// Parameter description.
/// </summary>
public string Description { get; set; } = string.Empty;
public string? Description { get; set; }

/// <summary>
/// Default value when the value is not provided.
/// </summary>
public string DefaultValue { get; set; } = string.Empty;
public string? DefaultValue { get; set; }

/// <summary>
/// Constructor
Expand All @@ -52,11 +52,9 @@ public ParameterView()
/// <param name="defaultValue">Default parameter value, if not provided</param>
public ParameterView(
string name,
string description,
string defaultValue)
string? description = null,
string? defaultValue = null)
{
Verify.ValidFunctionParamName(name);

this.Name = name;
this.Description = description;
this.DefaultValue = defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,4 @@ Task<string> RenderAsync(
string templateText,
SKContext context,
CancellationToken cancellationToken = default);

/// <summary>
/// Given a list of blocks render each block and compose the final result
/// </summary>
/// <param name="blocks">Template blocks generated by ExtractBlocks</param>
/// <param name="context">Access into the current kernel execution context</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
/// <returns>The prompt template ready to be used for an AI request</returns>
Task<string> RenderAsync(
IList<Block> blocks,
SKContext context,
CancellationToken cancellationToken = default);

/// <summary>
/// Given a list of blocks, render the Variable Blocks, replacing placeholders with the actual value in memory
/// </summary>
/// <param name="blocks">List of blocks, typically all the blocks found in a template</param>
/// <param name="variables">Container of all the temporary variables known to the kernel</param>
/// <returns>An updated list of blocks where Variable Blocks have rendered to Text Blocks</returns>
IList<Block> RenderVariables(
IList<Block> blocks,
ContextVariables? variables);
}
48 changes: 12 additions & 36 deletions dotnet/src/SemanticKernel/SemanticFunctions/PromptTemplate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.SemanticKernel.Orchestration;
using Microsoft.SemanticKernel.SkillDefinition;
using Microsoft.SemanticKernel.TemplateEngine;
Expand All @@ -22,9 +20,6 @@ public sealed class PromptTemplate : IPromptTemplate
private readonly string _template;
private readonly IPromptTemplateEngine _templateEngine;

// ReSharper disable once NotAccessedField.Local
private readonly ILogger _log = NullLogger.Instance;

// ReSharper disable once NotAccessedField.Local
private readonly PromptTemplateConfig _promptConfig;

Expand All @@ -35,7 +30,7 @@ public sealed class PromptTemplate : IPromptTemplate
/// <param name="promptTemplateConfig">Prompt template configuration.</param>
/// <param name="kernel">Kernel in which template is to take effect.</param>
public PromptTemplate(string template, PromptTemplateConfig promptTemplateConfig, IKernel kernel)
: this(template, promptTemplateConfig, kernel.PromptTemplateEngine, kernel.Log)
: this(template, promptTemplateConfig, kernel.PromptTemplateEngine)
{
}

Expand All @@ -45,17 +40,14 @@ public PromptTemplate(string template, PromptTemplateConfig promptTemplateConfig
/// <param name="template">Template.</param>
/// <param name="promptTemplateConfig">Prompt template configuration.</param>
/// <param name="promptTemplateEngine">Prompt template engine.</param>
/// <param name="log">Optional logger for prompt template.</param>
public PromptTemplate(
string template,
PromptTemplateConfig promptTemplateConfig,
IPromptTemplateEngine promptTemplateEngine,
ILogger? log = null)
IPromptTemplateEngine promptTemplateEngine)
{
this._template = template;
this._templateEngine = promptTemplateEngine;
this._promptConfig = promptTemplateConfig;
if (log != null) { this._log = log; }
}

/// <summary>
Expand All @@ -65,40 +57,24 @@ public PromptTemplate(
/// <returns>List of parameters</returns>
public IList<ParameterView> GetParameters()
{
var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

// Parameters from config.json
List<ParameterView> result = new();
foreach (PromptTemplateConfig.InputParameter? p in this._promptConfig.Input.Parameters)
Dictionary<string, ParameterView> result = new(StringComparer.OrdinalIgnoreCase);
foreach (var p in this._promptConfig.Input.Parameters)
{
if (p == null) { continue; }

result.Add(new ParameterView
{
Name = p.Name,
Description = p.Description,
DefaultValue = p.DefaultValue
});

seen.Add(p.Name);
result[p.Name] = new ParameterView(p.Name, p.Description, p.DefaultValue);
}

// Parameters from the template
List<VarBlock> listFromTemplate = this._templateEngine.ExtractBlocks(this._template)
.Where(x => x.Type == BlockTypes.Variable)
.Select(x => (VarBlock)x)
.Where(x => x != null)
.ToList();

foreach (VarBlock x in listFromTemplate)
foreach (var block in this._templateEngine.ExtractBlocks(this._template))
{
if (seen.Contains(x.Name)) { continue; }

result.Add(new ParameterView { Name = x.Name });
seen.Add(x.Name);
string? blockName = (block as VarBlock)?.Name;
if (!string.IsNullOrEmpty(blockName) && !result.ContainsKey(blockName!))
{
result.Add(blockName!, new ParameterView(blockName!));
}
}

return result;
return result.Values.ToList();
}

/// <summary>
Expand Down
19 changes: 15 additions & 4 deletions dotnet/src/SemanticKernel/TemplateEngine/PromptTemplateEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,14 @@ public async Task<string> RenderAsync(string templateText, SKContext context, Ca
return await this.RenderAsync(blocks, context, cancellationToken).ConfigureAwait(false);
}

/// <inheritdoc/>
public async Task<string> RenderAsync(IList<Block> blocks, SKContext context, CancellationToken cancellationToken = default)
/// <summary>
/// Given a list of blocks render each block and compose the final result
/// </summary>
/// <param name="blocks">Template blocks generated by ExtractBlocks</param>
/// <param name="context">Access into the current kernel execution context</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
/// <returns>The prompt template ready to be used for an AI request</returns>
internal async Task<string> RenderAsync(IList<Block> blocks, SKContext context, CancellationToken cancellationToken = default)
{
this._log.LogTrace("Rendering list of {0} blocks", blocks.Count);
var tasks = new List<Task<string>>(blocks.Count);
Expand Down Expand Up @@ -98,8 +104,13 @@ public async Task<string> RenderAsync(IList<Block> blocks, SKContext context, Ca
return result.ToString();
}

/// <inheritdoc/>
public IList<Block> RenderVariables(IList<Block> blocks, ContextVariables? variables)
/// <summary>
/// Given a list of blocks, render the Variable Blocks, replacing placeholders with the actual value in memory
/// </summary>
/// <param name="blocks">List of blocks, typically all the blocks found in a template</param>
/// <param name="variables">Container of all the temporary variables known to the kernel</param>
/// <returns>An updated list of blocks where Variable Blocks have rendered to Text Blocks</returns>
internal IList<Block> RenderVariables(IList<Block> blocks, ContextVariables? variables)
{
this._log.LogTrace("Rendering variables");
return blocks.Select(block => block.Type != BlockTypes.Variable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ async Task<SKContext> ExecuteAsync(SKContext context)
{
Name = p.AlternativeName ?? p.Name,
Description = $"{p.Description ?? p.Name}{(p.IsRequired ? " (required)" : string.Empty)}",
DefaultValue = p.DefaultValue ?? string.Empty
DefaultValue = p.DefaultValue
})
.ToList();

Expand Down

0 comments on commit 264e716

Please sign in to comment.