Skip to content

Commit

Permalink
Address code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
markwallace-microsoft committed Dec 2, 2024
1 parent 70f3ea0 commit ffdcf2e
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Text.Json;
using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.Connectors.MistralAI;
Expand Down Expand Up @@ -68,4 +69,44 @@ public void FromExecutionSettingsWhenSerializedHasPropertiesShouldPopulateSpecia
Assert.True(MistralExecutionSettings.SafePrompt);
Assert.Equal(123, MistralExecutionSettings.RandomSeed);
}

[Fact]
public void FreezeShouldPreventPropertyModification()
{
// Arrange
var settings = new MistralAIPromptExecutionSettings
{
Temperature = 0.7,
TopP = 1,
MaxTokens = 100,
SafePrompt = false,
Stop = ["foo", "bar"]
};

// Act
settings.Freeze();

// Assert
// Try to modify a property after freezing
Assert.Throws<InvalidOperationException>(() => settings.Temperature = 0.8);
Assert.Throws<InvalidOperationException>(() => settings.TopP = 0.9);
Assert.Throws<InvalidOperationException>(() => settings.MaxTokens = 50);
Assert.Throws<InvalidOperationException>(() => settings.SafePrompt = true);
Assert.Throws<NotSupportedException>(() => settings.Stop.Add("baz"));
}

[Fact]
public void FreezeShouldNotAllowMultipleFreezes()
{
// Arrange
var settings = new MistralAIPromptExecutionSettings();
settings.Freeze(); // First freeze

// Act
settings.Freeze(); // Second freeze (should not throw)

// Assert
// No exception should be thrown
Assert.True(settings.IsFrozen); // Assuming IsFrozen is a property indicating the freeze state
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public ContentChunkType(string type)
/// <param name="right"> the second <see cref="ContentChunkType"/> instance to compare </param>
/// <returns> false if left and right are both null or have equivalent labels; true otherwise </returns>
public static bool operator !=(ContentChunkType left, ContentChunkType right)
=> !(left == right);
=> !left.Equals(right);

/// <inheritdoc/>
public override bool Equals([NotNullWhen(true)] object? obj)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.

using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.SemanticKernel.ChatCompletion;
Expand Down Expand Up @@ -247,6 +248,11 @@ public override void Freeze()
return;
}

if (this._stop is not null)
{
this._stop = new ReadOnlyCollection<string>(this._stop);
}

base.Freeze();
}

Expand All @@ -267,7 +273,7 @@ public override PromptExecutionSettings Clone()
ResponseFormat = this.ResponseFormat,
FrequencyPenalty = this.FrequencyPenalty,
PresencePenalty = this.PresencePenalty,
Stop = this.Stop,
Stop = this.Stop is not null ? new List<string>(this.Stop) : null,
};
}

Expand Down

0 comments on commit ffdcf2e

Please sign in to comment.