Skip to content

Commit

Permalink
.Net: Plans using FunctionRunner + RequestSettings (Revert) (#3279)
Browse files Browse the repository at this point in the history
This change actually added a regression on the hooks, duplicating the
triggering while using plans.

RequestSettings at the Kernel level needd more discussion.

Reverts #3264
  • Loading branch information
RogerBarreto authored Oct 24, 2023
1 parent 0c1b669 commit 5bfff5d
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public async Task ItCanCreatePlanAsync(string goal)
// Arrange
var kernel = new Mock<IKernel>();
kernel.Setup(x => x.LoggerFactory).Returns(new Mock<ILoggerFactory>().Object);
kernel.Setup(x => x.RunAsync(It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>(), It.IsAny<ISKFunction>()))
.Returns<ContextVariables, AIRequestSettings, CancellationToken, ISKFunction[]>(async (vars, requestSettings, cancellationToken, functions) =>
kernel.Setup(x => x.RunAsync(It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>(), It.IsAny<ISKFunction>()))
.Returns<ContextVariables, CancellationToken, ISKFunction[]>(async (vars, cancellationToken, functions) =>
{
var functionResult = await functions[0].InvokeAsync(kernel.Object, vars, cancellationToken: cancellationToken);
return KernelResult.FromFunctionResults(functionResult.GetValue<string>(), new List<FunctionResult> { functionResult });
Expand Down Expand Up @@ -173,8 +173,8 @@ public async Task InvalidXMLThrowsAsync()

// Mock Plugins
kernel.Setup(x => x.Functions).Returns(functions.Object);
kernel.Setup(x => x.RunAsync(It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>(), It.IsAny<ISKFunction>()))
.Returns<ContextVariables, AIRequestSettings, CancellationToken, ISKFunction[]>(async (vars, requestSettings, cancellationToken, functions) =>
kernel.Setup(x => x.RunAsync(It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>(), It.IsAny<ISKFunction>()))
.Returns<ContextVariables, CancellationToken, ISKFunction[]>(async (vars, cancellationToken, functions) =>
{
var functionResult = await functions[0].InvokeAsync(kernel.Object, vars, cancellationToken: cancellationToken);
return KernelResult.FromFunctionResults(functionResult.GetValue<string>(), new List<FunctionResult> { functionResult });
Expand Down
5 changes: 1 addition & 4 deletions dotnet/src/SemanticKernel.Abstractions/IKernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.SemanticKernel.AI;
using Microsoft.SemanticKernel.Events;
using Microsoft.SemanticKernel.Http;
using Microsoft.SemanticKernel.Memory;
Expand Down Expand Up @@ -53,14 +52,12 @@ public interface IKernel
/// Run a pipeline composed of synchronous and asynchronous functions.
/// </summary>
/// <param name="variables">Input to process</param>
/// <param name="requestSettings">Request settings to use in the functions call</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
/// <param name="pipeline">List of functions</param>
/// <returns>Result of the function composition</returns>
Task<KernelResult> RunAsync(
ContextVariables variables,
AIRequestSettings? requestSettings = null,
CancellationToken cancellationToken = default,
CancellationToken cancellationToken,
params ISKFunction[] pipeline);

/// <summary>
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

using System.Threading;
using System.Threading.Tasks;
using Microsoft.SemanticKernel.AI;

namespace Microsoft.SemanticKernel.Orchestration;

Expand All @@ -16,13 +15,11 @@ public interface IFunctionRunner
/// </summary>
/// <param name="skFunction">Target function to run</param>
/// <param name="variables">Input to process</param>
/// <param name="requestSettings">Request settings to override</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
/// <returns>Result of the function composition</returns>
Task<FunctionResult> RunAsync(
ISKFunction skFunction,
ContextVariables? variables = null,
AIRequestSettings? requestSettings = null,
CancellationToken cancellationToken = default);

/// <summary>
Expand Down
5 changes: 2 additions & 3 deletions dotnet/src/SemanticKernel.Core/Kernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.SemanticKernel.AI;
using Microsoft.SemanticKernel.Diagnostics;
using Microsoft.SemanticKernel.Events;
using Microsoft.SemanticKernel.Http;
Expand Down Expand Up @@ -98,7 +97,7 @@ public ISKFunction RegisterCustomFunction(ISKFunction customFunction)
}

/// <inheritdoc/>
public async Task<KernelResult> RunAsync(ContextVariables variables, AIRequestSettings? requestSettings, CancellationToken cancellationToken = default, params ISKFunction[] pipeline)
public async Task<KernelResult> RunAsync(ContextVariables variables, CancellationToken cancellationToken, params ISKFunction[] pipeline)
{
var context = this.CreateNewContext(variables);

Expand Down Expand Up @@ -129,7 +128,7 @@ public async Task<KernelResult> RunAsync(ContextVariables variables, AIRequestSe
continue;
}

functionResult = await skFunction.InvokeAsync(context, requestSettings, cancellationToken).ConfigureAwait(false);
functionResult = await skFunction.InvokeAsync(context, cancellationToken: cancellationToken).ConfigureAwait(false);

context = functionResult.Context;

Expand Down
27 changes: 4 additions & 23 deletions dotnet/src/SemanticKernel.Core/KernelExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,26 +85,7 @@ public static Task<KernelResult> RunAsync(
CancellationToken cancellationToken = default)
{
Verify.NotNull(kernel);
return kernel.RunAsync(variables ?? new(), null, cancellationToken, skFunction);
}

/// <summary>
/// Run a pipeline composed of synchronous and asynchronous functions.
/// </summary>
/// <param name="kernel">The kernel.</param>
/// <param name="variables">Variables to process</param>
/// <param name="cancellationToken">Cancellation token</param>
/// <param name="pipeline">List of functions</param>
/// <returns>Result of the function composition</returns>
public static Task<KernelResult> RunAsync(
this IKernel kernel,
ContextVariables variables,
CancellationToken cancellationToken = default,
params ISKFunction[] pipeline)
{
Verify.NotNull(kernel);

return kernel.RunAsync(variables, null, cancellationToken, pipeline);
return kernel.RunAsync(variables ?? new(), cancellationToken, skFunction);
}

/// <summary>
Expand Down Expand Up @@ -150,7 +131,7 @@ public static Task<KernelResult> RunAsync(
params ISKFunction[] pipeline)
{
Verify.NotNull(kernel);
return kernel.RunAsync(variables, null, CancellationToken.None, pipeline);
return kernel.RunAsync(variables, CancellationToken.None, pipeline);
}

/// <summary>
Expand All @@ -166,7 +147,7 @@ public static Task<KernelResult> RunAsync(
params ISKFunction[] pipeline)
{
Verify.NotNull(kernel);
return kernel.RunAsync(new ContextVariables(), null, cancellationToken, pipeline);
return kernel.RunAsync(new ContextVariables(), cancellationToken, pipeline);
}

/// <summary>
Expand All @@ -184,6 +165,6 @@ public static Task<KernelResult> RunAsync(
params ISKFunction[] pipeline)
{
Verify.NotNull(kernel);
return kernel.RunAsync(new ContextVariables(input), null, cancellationToken, pipeline);
return kernel.RunAsync(new ContextVariables(input), cancellationToken, pipeline);
}
}
11 changes: 3 additions & 8 deletions dotnet/src/SemanticKernel.Core/Orchestration/FunctionRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.SemanticKernel.AI;

namespace Microsoft.SemanticKernel.Orchestration;

Expand All @@ -24,20 +23,16 @@ public FunctionRunner(IKernel kernel)
}

/// <inheritdoc/>
public async Task<FunctionResult> RunAsync(
ISKFunction skFunction,
ContextVariables? variables = null,
AIRequestSettings? requestSettings = null,
CancellationToken cancellationToken = default)
public async Task<FunctionResult> RunAsync(ISKFunction skFunction, ContextVariables? variables = null, CancellationToken cancellationToken = default)
{
return (await this._kernel.RunAsync(variables ?? new(), requestSettings, cancellationToken, skFunction).ConfigureAwait(false))
return (await this._kernel.RunAsync(skFunction, variables, cancellationToken).ConfigureAwait(false))
.FunctionResults.First();
}

/// <inheritdoc/>
public Task<FunctionResult> RunAsync(string pluginName, string functionName, ContextVariables? variables = null, CancellationToken cancellationToken = default)
{
var function = this._kernel.Functions.GetFunction(pluginName, functionName);
return this.RunAsync(function, variables, null, cancellationToken);
return this.RunAsync(function, variables, cancellationToken);
}
}
12 changes: 4 additions & 8 deletions dotnet/src/SemanticKernel.Core/Planning/Plan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public async Task<Plan> InvokeNextStepAsync(SKContext context, CancellationToken
var functionVariables = this.GetNextStepVariables(context.Variables, step);

// Execute the step
var result = await context.Runner.RunAsync(step, functionVariables, null, cancellationToken).ConfigureAwait(false);
var result = await context.Runner.RunAsync(step, functionVariables, cancellationToken).ConfigureAwait(false);

var resultValue = result.Context.Result.Trim();

Expand Down Expand Up @@ -336,14 +336,10 @@ public async Task<FunctionResult> InvokeAsync(
var functionContext = context.Clone(functionVariables, context.Functions);

// Execute the step

result = await context.Runner.RunAsync(
this.Function.WithInstrumentation(context.LoggerFactory),
functionVariables,
requestSettings,
cancellationToken)
result = await this.Function
.WithInstrumentation(context.LoggerFactory)
.InvokeAsync(functionContext, requestSettings, cancellationToken)
.ConfigureAwait(false);

this.UpdateFunctionResultWithOutputs(result);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.AI;
using Microsoft.SemanticKernel.Diagnostics;
using Microsoft.SemanticKernel.Orchestration;
using Microsoft.SemanticKernel.Planning;
Expand Down Expand Up @@ -277,8 +276,8 @@ public async Task CanStepAndSerializePlanWithStepsAsync()
returnContext.Variables.Update(returnContext.Variables.Input + c.Variables.Input))
.Returns(() => Task.FromResult(new FunctionResult("functionName", "pluginName", returnContext)));

this._functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, AIRequestSettings, CancellationToken>((function, variables, settings, ct) =>
this._functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, CancellationToken>((function, variables, ct) =>
{
var c = new SKContext(new Mock<IFunctionRunner>().Object, this._serviceProvider.Object, variables);
returnContext.Variables.Update(returnContext.Variables.Input + c.Variables.Input);
Expand Down Expand Up @@ -356,8 +355,8 @@ public async Task CanStepAndSerializePlanWithStepsAndContextAsync()

mockFunction.Setup(x => x.Describe()).Returns(new FunctionView("functionName", "pluginName"));

this._functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, AIRequestSettings, CancellationToken>((function, variables, settings, ct) =>
this._functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, CancellationToken>((function, variables, ct) =>
{
var c = new SKContext(new Mock<IFunctionRunner>().Object, this._serviceProvider.Object, variables);
c.Variables.TryGetValue("variables", out string? v);
Expand Down Expand Up @@ -398,7 +397,7 @@ public async Task CanStepAndSerializePlanWithStepsAndContextAsync()
// Assert
Assert.NotNull(plan);
Assert.Equal($"{stepOutput}{planInput}foo{stepOutput}{planInput}foobar", plan.State.ToString());
this._functionRunner.Verify(x => x.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()), Times.Exactly(2));
this._functionRunner.Verify(x => x.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()), Times.Exactly(2));

// Act
var serializedPlan2 = plan.ToJson();
Expand Down Expand Up @@ -459,8 +458,8 @@ public async Task CanStepAndSerializeAndDeserializePlanWithStepsAndContextAsync(

plan.AddSteps(mockFunction.Object, mockFunction.Object);

this._functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, AIRequestSettings, CancellationToken>((function, variables, settings, ct) =>
this._functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, CancellationToken>((function, variables, ct) =>
{
var c = new SKContext(new Mock<IFunctionRunner>().Object, this._serviceProvider.Object, variables);
c.Variables.TryGetValue("variables", out string? v);
Expand Down Expand Up @@ -499,7 +498,7 @@ public async Task CanStepAndSerializeAndDeserializePlanWithStepsAndContextAsync(
// Assert
Assert.NotNull(plan);
Assert.Equal($"{stepOutput}{planInput}foo{stepOutput}{planInput}foobar", plan.State.ToString());
this._functionRunner.Verify(x => x.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()), Times.Exactly(2));
this._functionRunner.Verify(x => x.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()), Times.Exactly(2));

// Act
var serializedPlan2 = plan.ToJson();
Expand Down
12 changes: 6 additions & 6 deletions dotnet/src/SemanticKernel.UnitTests/Planning/PlanTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,8 @@ public async Task CanExecutePlanWithStateAsync()
// Arrange
var (kernel, functionRunner, serviceProvider) = this.SetupKernelMock();

functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, AIRequestSettings, CancellationToken>(async (function, variables, settings, ct) =>
functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, CancellationToken>(async (function, variables, ct) =>
{
var c = new SKContext(functionRunner.Object, serviceProvider.Object, variables);
var functionResult = await function.InvokeAsync(c, cancellationToken: ct);
Expand Down Expand Up @@ -668,8 +668,8 @@ public async Task CanExecutePlanWithJoinedResultAsync()
// Arrange
var (kernel, functionRunner, serviceProvider) = this.SetupKernelMock();

functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, AIRequestSettings, CancellationToken>(async (function, variables, settings, ct) =>
functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, CancellationToken>(async (function, variables, ct) =>
{
var c = new SKContext(functionRunner.Object, serviceProvider.Object, variables);
var functionResult = await function.InvokeAsync(c, cancellationToken: ct);
Expand Down Expand Up @@ -842,8 +842,8 @@ public async Task CanExecutePlanWithExpandedAsync()
return new SKContext(functionRunner.Object, serviceProvider.Object, contextVariables, functions);
});

functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<AIRequestSettings?>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, AIRequestSettings, CancellationToken>(async (function, variables, settings, ct) =>
functionRunner.Setup(k => k.RunAsync(It.IsAny<ISKFunction>(), It.IsAny<ContextVariables>(), It.IsAny<CancellationToken>()))
.Returns<ISKFunction, ContextVariables, CancellationToken>(async (function, variables, ct) =>
{
var c = new SKContext(functionRunner.Object, serviceProvider.Object, variables);
var functionResult = await function.InvokeAsync(c, cancellationToken: ct);
Expand Down

0 comments on commit 5bfff5d

Please sign in to comment.