Skip to content

Commit

Permalink
.Net: Minor cleanup of collection nullability. (#2107)
Browse files Browse the repository at this point in the history
Collections are being checked for null, even when they cannot or should
not ever be null.

Fixes #2064
Fixes #2065

### Description
Removing nullable operators/checks from Skills collections

### Contribution Checklist
- [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#dev-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
shawncal committed Jul 21, 2023
1 parent 4a0ed49 commit 798ebc5
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,12 @@ public static class SKContextSequentialPlannerExtensions
string? semanticQuery = null,
CancellationToken cancellationToken = default)
{
var excludedSkills = config.ExcludedSkills ?? new();
var excludedFunctions = config.ExcludedFunctions ?? new();
var includedFunctions = config.IncludedFunctions ?? new();

if (context.Skills == null)
{
throw new KernelException(
KernelException.ErrorCodes.SkillCollectionNotSet,
"Skill collection not found in the context");
}

var functionsView = context.Skills.GetFunctionsView();

var availableFunctions = functionsView.SemanticFunctions
.Concat(functionsView.NativeFunctions)
.SelectMany(x => x.Value)
.Where(s => !excludedSkills.Contains(s.SkillName) && !excludedFunctions.Contains(s.Name))
.Where(s => !config.ExcludedSkills.Contains(s.SkillName) && !config.ExcludedFunctions.Contains(s.Name))
.ToList();

List<FunctionView>? result = null;
Expand Down Expand Up @@ -104,7 +93,7 @@ public static class SKContextSequentialPlannerExtensions
result.AddRange(await GetRelevantFunctionsAsync(context, availableFunctions, memories, cancellationToken).ConfigureAwait(false));

// Add any missing functions that were included but not found in the search results.
var missingFunctions = includedFunctions
var missingFunctions = config.IncludedFunctions
.Except(result.Select(x => x.Name))
.Join(availableFunctions, f => f, af => af.Name, (_, af) => af);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public ISemanticTextMemory Memory
/// <summary>
/// Read only skills collection
/// </summary>
public IReadOnlySkillCollection? Skills { get; internal set; }
public IReadOnlySkillCollection Skills { get; }

/// <summary>
/// Access registered functions by skill + name. Not case sensitive.
Expand All @@ -119,13 +119,6 @@ public ISemanticTextMemory Memory
/// <returns>Delegate to execute the function</returns>
public ISKFunction Func(string skillName, string functionName)
{
if (this.Skills is null)
{
throw new KernelException(
KernelException.ErrorCodes.SkillCollectionNotSet,
"Skill collection not found in the context");
}

return this.Skills.GetFunction(skillName, functionName);
}

Expand Down
3 changes: 0 additions & 3 deletions dotnet/src/SemanticKernel/SkillDefinition/SKFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,6 @@ public FunctionView Describe()
/// <inheritdoc/>
public async Task<SKContext> InvokeAsync(SKContext context, CompleteRequestSettings? settings = null, CancellationToken cancellationToken = default)
{
// If the function is invoked manually, the user might have left out the skill collection
context.Skills ??= this._skillCollection;

if (this.IsSemantic)
{
this.AddDefaultValues(context.Variables);
Expand Down

0 comments on commit 798ebc5

Please sign in to comment.