Skip to content

Commit

Permalink
.Net: Processes - Sample Pattern Adjustments (#9181)
Browse files Browse the repository at this point in the history
### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

Noticed some tiny stuff I wanted to make available for review, separate
from the _Agent_ sample.

### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

- `LocalKernelProcessContext` is `IDisposable` - Update to incorporate
`using` statement to ensure disposal (best practice)
- `Console.Write` not resulting any output (`TextWriter.Write(char
value) is an empty virtual method that requires override...or in this
case interception -
https://github.com/microsoft/referencesource/blob/master/mscorlib/system/io/textwriter.cs#L193.)
- Simplify `ScriptedUserInputStep` abstraction. State is never null and
can be treated as such with a minor tweak. (Null check in the sample
could be misleading - it gave me momentary pause)
- Removed redundant `CreateKernelWithChatCompletion` method where
present (already in base class)
- Simplified `Step01_Processes.ChatBotState` initialization. The
conditional inner initialization might be slightly misleading (It gave
me momentary pause).

### 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://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [X] All unit tests pass, and I have added new tests where possible
- [X] I didn't break anyone 😄
  • Loading branch information
crickman authored Oct 9, 2024
1 parent 2e607e7 commit 4364ad1
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ public static class Functions
/// <summary>
/// The state object for the user input step. This object holds the user inputs and the current input index.
/// </summary>
protected UserInputState? _state;
private UserInputState? _state;

/// <summary>
/// Method to be overridden by the user to populate with custom user messages
/// </summary>
public virtual void PopulateUserInputs()
/// <param name="state">The initialized state object for the step.</param>
public virtual void PopulateUserInputs(UserInputState state)
{
return;
}
Expand All @@ -43,7 +44,7 @@ public override ValueTask ActivateAsync(KernelProcessStepState<UserInputState> s
state.State ??= new();
_state = state.State;

PopulateUserInputs();
PopulateUserInputs(_state);

return ValueTask.CompletedTask;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public async Task UseSimpleProcessAsync()
KernelProcess kernelProcess = process.Build();

// Start the process with an initial external event
var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = ChatBotEvents.StartProcess, Data = null });
using var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = ChatBotEvents.StartProcess, Data = null });
}

/// <summary>
Expand All @@ -88,16 +88,13 @@ public void PrintIntroMessage()
/// </summary>
private sealed class ChatUserInputStep : ScriptedUserInputStep
{
public override void PopulateUserInputs()
public override void PopulateUserInputs(UserInputState state)
{
if (_state != null)
{
_state.UserInputs.Add("Hello");
_state.UserInputs.Add("How tall is the tallest mountain?");
_state.UserInputs.Add("How low is the lowest valley?");
_state.UserInputs.Add("How wide is the widest river?");
_state.UserInputs.Add("exit");
}
state.UserInputs.Add("Hello");
state.UserInputs.Add("How tall is the tallest mountain?");
state.UserInputs.Add("How low is the lowest valley?");
state.UserInputs.Add("How wide is the widest river?");
state.UserInputs.Add("exit");
}
}

Expand All @@ -124,7 +121,6 @@ public static class Functions
public override ValueTask ActivateAsync(KernelProcessStepState<ChatBotState> state)
{
_state = state.State ?? new();
_state.ChatMessages ??= new();
return ValueTask.CompletedTask;
}

Expand All @@ -147,9 +143,8 @@ public async Task GetChatResponseAsync(KernelProcessStepContext context, string
}

System.Console.ForegroundColor = ConsoleColor.Yellow;
System.Console.Write("Assistant: ");
System.Console.WriteLine($"ASSISTANT: {response.Content}");
System.Console.ResetColor();
System.Console.WriteLine(response.Content);

// Update state with the response
_state.ChatMessages.Add(response);
Expand All @@ -164,7 +159,7 @@ public async Task GetChatResponseAsync(KernelProcessStepContext context, string
/// </summary>
private sealed class ChatBotState
{
internal ChatHistory ChatMessages { get; set; } = new();
internal ChatHistory ChatMessages { get; } = new();
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ namespace Step02;
/// </summary>
public class Step02_AccountOpening(ITestOutputHelper output) : BaseTest(output, redirectSystemConsoleOutput: true)
{
// Target Open AI Services
protected override bool ForceOpenAI => true;

private KernelProcess SetupAccountOpeningProcess<TUserInputStep>() where TUserInputStep : ScriptedUserInputStep
{
ProcessBuilder process = new("AccountOpeningProcess");
Expand Down Expand Up @@ -143,21 +146,18 @@ public async Task UseAccountOpeningProcessSuccessfulInteractionAsync()
{
Kernel kernel = CreateKernelWithChatCompletion();
KernelProcess kernelProcess = SetupAccountOpeningProcess<UserInputSuccessfulInteraction>();
var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = AccountOpeningEvents.StartProcess, Data = null });
using var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = AccountOpeningEvents.StartProcess, Data = null });
}

private sealed class UserInputSuccessfulInteraction : ScriptedUserInputStep
{
public override void PopulateUserInputs()
public override void PopulateUserInputs(UserInputState state)
{
if (_state != null)
{
_state.UserInputs.Add("I would like to open an account");
_state.UserInputs.Add("My name is John Contoso, dob 02/03/1990");
_state.UserInputs.Add("I live in Washington and my phone number es 222-222-1234");
_state.UserInputs.Add("My userId is 987-654-3210");
_state.UserInputs.Add("My email is john.contoso@contoso.com, what else do you need?");
}
state.UserInputs.Add("I would like to open an account");
state.UserInputs.Add("My name is John Contoso, dob 02/03/1990");
state.UserInputs.Add("I live in Washington and my phone number es 222-222-1234");
state.UserInputs.Add("My userId is 987-654-3210");
state.UserInputs.Add("My email is john.contoso@contoso.com, what else do you need?");
}
}

Expand All @@ -169,21 +169,18 @@ public async Task UseAccountOpeningProcessFailureDueToCreditScoreFailureAsync()
{
Kernel kernel = CreateKernelWithChatCompletion();
KernelProcess kernelProcess = SetupAccountOpeningProcess<UserInputCreditScoreFailureInteraction>();
var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = AccountOpeningEvents.StartProcess, Data = null });
using var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = AccountOpeningEvents.StartProcess, Data = null });
}

private sealed class UserInputCreditScoreFailureInteraction : ScriptedUserInputStep
{
public override void PopulateUserInputs()
public override void PopulateUserInputs(UserInputState state)
{
if (_state != null)
{
_state.UserInputs.Add("I would like to open an account");
_state.UserInputs.Add("My name is John Contoso, dob 01/01/1990");
_state.UserInputs.Add("I live in Washington and my phone number es 222-222-1234");
_state.UserInputs.Add("My userId is 987-654-3210");
_state.UserInputs.Add("My email is john.contoso@contoso.com, what else do you need?");
}
state.UserInputs.Add("I would like to open an account");
state.UserInputs.Add("My name is John Contoso, dob 01/01/1990");
state.UserInputs.Add("I live in Washington and my phone number es 222-222-1234");
state.UserInputs.Add("My userId is 987-654-3210");
state.UserInputs.Add("My email is john.contoso@contoso.com, what else do you need?");
}
}

Expand All @@ -195,31 +192,18 @@ public async Task UseAccountOpeningProcessFailureDueToFraudFailureAsync()
{
Kernel kernel = CreateKernelWithChatCompletion();
KernelProcess kernelProcess = SetupAccountOpeningProcess<UserInputFraudFailureInteraction>();
var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = AccountOpeningEvents.StartProcess, Data = null });
using var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent() { Id = AccountOpeningEvents.StartProcess, Data = null });
}

private sealed class UserInputFraudFailureInteraction : ScriptedUserInputStep
{
public override void PopulateUserInputs()
public override void PopulateUserInputs(UserInputState state)
{
if (_state != null)
{
_state.UserInputs.Add("I would like to open an account");
_state.UserInputs.Add("My name is John Contoso, dob 02/03/1990");
_state.UserInputs.Add("I live in Washington and my phone number es 222-222-1234");
_state.UserInputs.Add("My userId is 123-456-7890");
_state.UserInputs.Add("My email is john.contoso@contoso.com, what else do you need?");
}
state.UserInputs.Add("I would like to open an account");
state.UserInputs.Add("My name is John Contoso, dob 02/03/1990");
state.UserInputs.Add("I live in Washington and my phone number es 222-222-1234");
state.UserInputs.Add("My userId is 123-456-7890");
state.UserInputs.Add("My email is john.contoso@contoso.com, what else do you need?");
}
}

protected new Kernel CreateKernelWithChatCompletion()
{
var builder = Kernel.CreateBuilder();
builder.AddOpenAIChatCompletion(
TestConfiguration.OpenAI.ChatModelId,
TestConfiguration.OpenAI.ApiKey);

return builder.Build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ namespace Step03;
/// </summary>
public class Step03_FoodPreparation(ITestOutputHelper output) : BaseTest(output, redirectSystemConsoleOutput: true)
{
// Target Open AI Services
protected override bool ForceOpenAI => true;

[Fact]
public async Task UsePrepareFriedFishProcessAsync()
{
Expand Down Expand Up @@ -50,7 +53,7 @@ protected async Task UsePrepareSpecificProductAsync(ProcessBuilder processBuilde
KernelProcess kernelProcess = processBuilder.Build();

// Assert
var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent()
using var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent()
{
Id = externalTriggerEvent, Data = new List<string>()
});
Expand Down Expand Up @@ -85,20 +88,10 @@ protected async Task UsePrepareFoodOrderProcessSingleItemAsync(FoodItem foodItem
Kernel kernel = CreateKernelWithChatCompletion();
KernelProcess kernelProcess = SingleFoodItemProcess.CreateProcess().Build();

var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent()
using var runningProcess = await kernelProcess.StartAsync(kernel, new KernelProcessEvent()
{
Id = SingleFoodItemProcess.ProcessEvents.SingleOrderReceived,
Data = foodItem
});
}

protected new Kernel CreateKernelWithChatCompletion()
{
var builder = Kernel.CreateBuilder();
builder.AddOpenAIChatCompletion(
TestConfiguration.OpenAI.ChatModelId,
TestConfiguration.OpenAI.ApiKey);

return builder.Build();
}
}
12 changes: 12 additions & 0 deletions dotnet/src/InternalUtilities/samples/InternalUtilities/BaseTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,21 @@ public override void WriteLine(string? value)
=> this.Output.WriteLine(value ?? string.Empty);

/// <inheritdoc/>
/// <remarks>
/// <see cref="ITestOutputHelper"/> only supports output that ends with a newline.
/// User this method will resolve in a call to <see cref="WriteLine(string?)"/>.
/// </remarks>
public override void Write(object? value = null)
=> this.Output.WriteLine(value ?? string.Empty);

/// <inheritdoc/>
/// <remarks>
/// <see cref="ITestOutputHelper"/> only supports output that ends with a newline.
/// User this method will resolve in a call to <see cref="WriteLine(string?)"/>.
/// </remarks>
public override void Write(char[]? buffer)
=> this.Output.WriteLine(new string(buffer));

/// <inheritdoc/>
public override Encoding Encoding => Encoding.UTF8;

Expand Down

0 comments on commit 4364ad1

Please sign in to comment.