# AI integrations: rely or verify? Checking Semantic Kernel

Semantic Kernel is a Microsoft's SDK for integrating AI models into applications\. Can PVS\-Studio static analyzer find defects in the source code of a project like this? This article answers this question\. Enjoy reading\!

![1367_semantic-kernel/image1.png](https://import.viva64.com/docx/blog/1367_semantic-kernel/image1.png)

Projects that involve AI integration increasingly shape everyday development\. One such project is [Semantic Kernel](https://github.com/microsoft/semantic-kernel)\. It's an SDK for building AI agents and orchestrating LLM scenarios, and Microsoft actively develops it\.

But underneath even the most modern solutions, you'll find ordinary C\# code—with all the usual problems that come with it\. We'll try to confirm that today\.

To analyze the C\# part of the project, we used [PVS\-Studio static analyzer version 7\.41](https://pvs-studio.com/en/pvs-studio/)\. The source code we checked corresponds to this [commit](https://github.com/microsoft/semantic-kernel/tree/134e52e7540270ca4055bd5eab4d7606f9689ce1)\. 

Note that we'll only discuss the most interesting code snippets to keep the article concise\.

Let's start breaking down suspicious spots\!

## Misplaced priorities

**Code snippet 1**

```cpp
internal static async Task 
                CreateToolsAsync(this AgentDefinition agentDefinition,
                                 BedrockAgent agent, 
                                 CancellationToken cancellationToken)
{
  ....
  if (knowledgeBase.Options
                   ?.TryGetValue(KnowledgeBaseId, 
                                 out var value) ?? false && value is not null 
                                                         && value is string)
  {
    var knowledgeBaseId = value as string;
    var description = knowledgeBase.Description ?? string.Empty;
    await agent.AssociateAgentKnowledgeBaseAsync(knowledgeBaseId!, 
                                                 description,
                                                 cancellationToken)
               .ConfigureAwait(false)
  }
  ....
}
```

The PVS\-Studio warning: [V3177](https://pvs-studio.com/en/docs/warnings/v3177/) The 'false' literal belongs to the '&&' operator with a higher priority\. It is possible the literal was intended to belong to '??' operator instead\. [BedrockAgentDefinitionExtensions\.cs 44](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Agents/Bedrock/Extensions/BedrockAgentDefinitionExtensions.cs#L44)

The developer likely intended this condition to work as follows: if `knowledgeBase.Options?.TryGetValue(KnowledgeBaseId, out var value)` is not `null` then check that `value is not null && value is string`\. But the actual behavior differs\. The `&&` operator takes higher priority than `??`, so the checks on `value` never execute\.

Another clue that an error lurks here: the right operand of `??`\. It looks like `false && value is not null && value is string`\. The outcome of that expression is obvious\. The condition contains only `&&` operators, and one operand is `false`\. So, the entire expression is always `false`\.



To fix it, the author needs to add parentheses:

```cpp
(knowledgeBase.Options
                   ?.TryGetValue(KnowledgeBaseId, 
                                 out var value) ?? false) && value is not null 
                                                          && value is string
```

Note that 4 other places in the source code contain the same issue:

* [TavilyTextSearch\.cs 477](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Plugins/Plugins.Web/Tavily/TavilyTextSearch.cs#L477)
* [TavilyTextSearch\.cs 505](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Plugins/Plugins.Web/Tavily/TavilyTextSearch.cs#L505)
* [TavilyTextSearch\.cs 540](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Plugins/Plugins.Web/Tavily/TavilyTextSearch.cs#L540)
* [TavilyTextSearch\.cs 574](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Plugins/Plugins.Web/Tavily/TavilyTextSearch.cs#L574)

**Code snippet 2**

```cpp
private static readonly RestApiParameterFilter s_restApiParameterFilter = 
(RestApiParameterFilterContext context) =>
{
  if (   ("me_sendMail".Equals(context.Operation.Id, 
                               StringComparison.OrdinalIgnoreCase) 
      || ("me_calendar_CreateEvents".Equals(context.Operation.Id,
                                            StringComparison.OrdinalIgnoreCase))
      && "payload".Equals(context.Parameter.Name, 
                          StringComparison.OrdinalIgnoreCase)))
  {
    context.Parameter
           .Schema = TrimPropertiesFromRequestBody(context.Parameter.Schema);
    return context.Parameter;
  }
  return context.Parameter;
};
```

The PVS\-Studio warning: [V3088](https://pvs-studio.com/en/docs/warnings/v3088/) The expression was enclosed by parentheses twice: \(\(expression\)\)\. One pair of parentheses is unnecessary or misprint is present\. [CopilotAgentBasedPlugins\.cs 212](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/samples/Concepts/Plugins/CopilotAgentBasedPlugins.cs#L212)

The outermost parentheses wrap the entire `if` condition, making them pointless\. The developer probably intended this logic: apply the `payload` parameter name check to both operations \(`me_sendMail` and `me_calendar_CreateEvents`\)\. But in reality, the `payload` check applies only to the second operation because `&&` has a higher priority than `||`\. To fix it, the developer needs to parenthesize only the checks for `me_sendMail` and `me_calendar_CreateEvents`, not the entire condition\.

## Forgot something

**Code snippet 3**

```cpp
private static string? GetAudioOutputMimeType(ChatAudioOptions? audioOptions)
{
  if (audioOptions is null)
  {
    return null;
  }

  if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav) // <=
  {
    return "audio/wav";
  }

  if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Mp3)
  {
    return "audio/mp3";
  }

  if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Opus)
  {
    return "audio/opus";
  }

  if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav) // <=
  {
    return "audio/wav";
  }

  if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Flac)
  {
    return "audio/flac";
  }

  if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Pcm16)
  {
    return "audio/pcm16";
  }

  throw new NotSupportedException
  (
    $"Unsupported audio output format '{audioOptions.OutputAudioFormat}'. " +
    "Supported formats are 'wav', 'mp3', 'opus', 'flac' and 'pcm16'."
  );
}
```

The PVS\-Studio warning: [V3021](https://pvs-studio.com/en/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless [ClientCore\.ChatCompletion\.cs 985](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Connectors/Connectors.OpenAI/Core/ClientCore.ChatCompletion.cs#L985)

The `GetAudioOutputMimeType` method contains the same check twice: `audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav`\. The repeated check for `ChatOutputAudioFormat.Wav` is unreachable code—the method returns when the first match occurs\. This is a typical copy\-paste error\.

Interestingly, the `ChatOutputAudioFormat` struct declares a `ChatOutputAudioFormat Aac` format\. This is the only format the `GetAudioOutputMimeType` method doesn't handle\. Perhaps it should replace one of the `audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav` check\.

```cpp
public readonly partial struct ChatOutputAudioFormat 
                                 : IEquatable<ChatOutputAudioFormat>
{
  ....
  public static ChatOutputAudioFormat Wav { get; } = 
    new ChatOutputAudioFormat(WavValue);

  public static ChatOutputAudioFormat Aac { get; } =        // <=
    new ChatOutputAudioFormat(AacValue);

  public static ChatOutputAudioFormat Mp3 { get; } = 
    new ChatOutputAudioFormat(Mp3Value);

  public static ChatOutputAudioFormat Flac { get; } = 
    new ChatOutputAudioFormat(FlacValue);

  public static ChatOutputAudioFormat Opus { get; } = 
    new ChatOutputAudioFormat(OpusValue);

  public static ChatOutputAudioFormat Pcm16 { get; } = 
    new ChatOutputAudioFormat(Pcm16Value);
  ....
}
```

**Code snippet 4**

```cpp
public async Task<KernelSearchResults<TextSearchResult>>
      GetTextSearchResultsAsync(string query, 
                                TextSearchOptions? searchOptions = null,
                                CancellationToken cancellationToken = default)
{
  var searchResult = await this.SearchInternalAsync(query, 
                                                    searchOptions, 
                                                    cancellationToken)
                               .ConfigureAwait(false);

  var results = 
   searchResult.Select(x => 
      new TextSearchResult(x.Text ?? string.Empty) 
        { Name = x.SourceName, Link = x.SourceLink });
  return 
    new(searchResult.Select(x =>
      new TextSearchResult(x.Text ?? string.Empty)
        { Name = x.SourceName, Link = x.SourceLink }).ToAsyncEnumerable());
}
```

The PVS\-Studio warning: [V3220](https://pvs-studio.com/en/docs/warnings/v3220/) The result of the 'Select' LINQ method with deferred execution is never used\. The method will not be executed\. [TextSearchStore\.cs 204](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/SemanticKernel.Core/Data/TextSearchStore/TextSearchStore.cs#L204)

The `results` variable receives the result of a LINQ method call, but the variable is never used after that\. Many LINQ methods have [deferred execution](https://learn.microsoft.com/en-us/dotnet/csharp/linq/get-started/introduction-to-linq-queries#deferred)\. So, value obtained from the LINQ expression is not evaluated when forming that expression\. It will be evaluated only when we iterate over the resulting sequence \(for example, in a `foreach` loop\)\.

The value assignment to `results` is followed by a `return` statement with a similar LINQ expression\. Most likely, one of the LINQ expressions is redundant\. The author should either remove the `results` variable or use it when forming the return value: 

```cpp
return new(results.ToAsyncEnumerable());
```

**Code snippet 5**

```cpp
/// <summary>
/// Creates a new instance of the <see cref="KernelProcess"/> class.
/// </summary>
/// <param name="state">The process state.</param>
/// <param name="steps">The steps of the process.</param>
/// <param name="edges">The edges of the process.</param>
/// <param name="threads">The threads associated with the process.</param>
public KernelProcess(
           KernelProcessState state, 
           IList<KernelProcessStepInfo> steps,
           Dictionary<string, List<KernelProcessEdge>>? edges = null, 
           IReadOnlyDictionary<string, 
                               KernelProcessAgentThread>? threads = null)
    : base(typeof(KernelProcess), state, edges ?? [])
{
  Verify.NotNull(steps);
  Verify.NotNullOrWhiteSpace(state.Name);

  this.Steps = [.. steps];
}
```

The PVS\-Studio warning: [V3117](https://pvs-studio.com/en/docs/warnings/v3117/) Constructor parameter 'threads' is not used\. [KernelProcess\.cs 46](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Experimental/Process.Abstractions/KernelProcess.cs#L46)

The `threads` parameter wasn't used\. When calling the `KernelProcess` method, the caller may pass an argument matching `threads`\. That suggests the caller expects the parameter to be used\. 

Note that the `KernelProcess` class has a property named `Threads`\. Perhaps that property should initialize from the unused parameter\.

## Array index out of bounds

**Code snippet 6**

```cpp
private int ComputeTruncationIndex(
                     IReadOnlyList<ChatMessageContent> chatHistory,
                     ChatMessageContent? systemMessage)
{
  var truncationIndex = -1;

  var totalTokenCount = (int)(systemMessage?.Metadata?["TokenCount"] ?? 0);
  for (int i = chatHistory.Count - 1; i >= 0; i--)                        // <=
  {
    truncationIndex = i;
    var tokenCount = (int)(chatHistory[i].Metadata?["TokenCount"] ?? 0);
    if (tokenCount + totalTokenCount > this._maxTokenCount)
    {
      break;
    }
    totalTokenCount += tokenCount;
  }

  // Skip function related content
  while (truncationIndex < chatHistory.Count)                             // <=
  {
    if (chatHistory[truncationIndex].Items.Any(i => i is FunctionCallContent 
                                                      or FunctionResultContent))
    {
      truncationIndex++;
    }
    else
    {
      break;
    }
  }

  return truncationIndex;
}
```

The PVS\-Studio warning: [V3106](https://pvs-studio.com/en/docs/warnings/v3106/) Possible negative index value\. The value of 'truncationIndex' index could reach \-1\. [ChatHistoryMaxTokensReducer\.cs 75](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/samples/Concepts/ChatCompletion/ChatHistoryReducers/ChatHistoryMaxTokensReducer.cs#L75)

The `truncationIndex` variable initializes to `-1`\. If the `chatHistory` collection is empty, the execution flow won't enter the `for` loop as the condition `-1 >= 0` fails before the first iteration\. Thus, the value of `truncationIndex` doesn't change\. Immediately after the `for` loop comes a `while` loop whose condition will be true if `chatHistory` has zero elements \(`-1 < 0`\)\. Inside the `while` loop, the code uses `truncationIndex` as an index, and it may still be `-1`\. Accessing an element with a negative index throws an `IndexOutOfRangeException`\.

To fix this, the developer should check that the index is not negative before using it\.

## Issues with null

**Code snippet 7**

```cpp
public KernelProcessProxy ToKernelProcessProxy()
{
  KernelProcessStepInfo processStepInfo = this.ToKernelProcessStepInfo();
  if (this.State is not KernelProcessStepState state)
  {
    throw new KernelException($"Unable to read state from proxy with name " +
                              $"'{this.State.Name}', Id '{this.State.Id}' " +
                              $"and type {this.State.GetType()}.");
  }

  return new KernelProcessProxy(state, this.Edges)
  {
    ProxyMetadata = this.ProxyMetadata,
  };
}

public required KernelProcessStepState State { get; init; }
```

The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) \[SEC\-NULL\] Possible null dereference\. Consider inspecting 'this\.State'\. [DaprProxyInfo\.cs 30](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Experimental/Process.Runtime.Dapr/DaprProxyInfo.cs#L30)

The condition checks that `this.State` does not match the`KernelProcessStepState` type\. The `State` property declaration indicates that its type is `KernelProcessStepState`\. So the condition seems always false? Not exactly\. If `this.State` is `null`, then `this.State is not KernelProcessStepState` will be `true`\.

We expect a `KernelException` in the `then` block of the `if` statement\. But the actual behavior differes\. The exception message accesses properties of `this.State`\. The only way execution flow reaches the `then` block is when `this.State` is `null`, the result will be a `NullReferenceException`\.

**Code snippet 8**

```cpp
private static RestApiPayload? 
                  CreateRestApiOperationPayload(string operationId,
                                                OpenApiRequestBody requestBody)
{
  if (requestBody?.Content is null)
  {
    return null;
  }

  var mediaType = GetMediaType(requestBody.Content) 
            ?? throw new KernelException(
               $"Neither of the media types of {operationId} is supported.");

  var mediaTypeMetadata = requestBody.Content[mediaType];

  var payloadProperties = GetPayloadProperties(operationId, 
                                               mediaTypeMetadata.Schema); // <=

  return new RestApiPayload(mediaType,
                            payloadProperties,
                            requestBody.Description,
                            mediaTypeMetadata?.Schema?.ToJsonSchema());   // <=
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'mediaTypeMetadata' object was used before it was verified against null\. Check lines: 464, 466\. [OpenApiDocumentParser\.cs 464](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Functions/Functions.OpenApi/OpenApi/OpenApiDocumentParser.cs#L464)

The `mediaTypeMetadata` variable is fisrt accessed without a `null` check, and then on the very next line is checked using a null\-conditional operator `?.`\. Perhaps the check in the second case is redundant, and authors should remove it\. If `mediaTypeMetadata` can indeed be `null`, then the first access will result in a `NullReferenceException`\.

**Code snippet 9**

```cpp
internal async Task<ReActStep?> GetNextStepAsync(....)
{
  ....
  if (this._logger.IsEnabled(LogLevel.Information))                       // <=
  {
    this._logger?.LogInformation("question: {Question}", question);
    this._logger?.LogInformation("functionDescriptions: {FunctionDescriptions}",
                                 functionDesc);
    this._logger?.LogInformation("Scratchpad: {ScratchPad}", scratchPad);
  }

  var llmResponse = await this._reActFunction
                              .InvokeAsync(kernel, arguments)
                              .ConfigureAwait(false);

  string llmResponseText = llmResponse.GetValue<string>()!.Trim();

  if (this._logger?.IsEnabled(LogLevel.Debug) ?? false)
  {
    this._logger?.LogDebug("Response : {ActionText}", llmResponseText);
  }
  ....
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'this\.\_logger' object was used before it was verified against null\. Check lines: 144, 146\. [ReActEngine\.cs 144](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Experimental/Orchestration.Flow/Execution/ReActEngine.cs#L144)

This case resembles the previous example\. In the same context, the `this._logger`field is used with a `null` check and without it\. Besides the code above, the `GetNextStepAsync` method contains two more places where `this._logger` is not checked for `null`\. As before, this indicates inconsistent handling of a potential `null` value for`this._logger`\.

**Code snippet 10**

```cpp
public override async Task<MAAI.AgentRunResponse> 
                         RunAsync(IEnumerable<ChatMessage> messages, 
                                  MAAI.AgentThread? thread = null,
                                  MAAI.AgentRunOptions? options = null,
                                  CancellationToken cancellationToken = default)
{  
  ....
  AgentResponseItem<ChatMessageContent>? lastResponseItem = null; 
  ChatMessage? lastResponseMessage = null;                              // <=

  await foreach (var responseItem in ....)
  {
    lastResponseItem = responseItem;
  }

  return new MAAI.AgentRunResponse(responseMessages)
  {
    AgentId = this._innerAgent.Id,
    RawRepresentation = lastResponseItem,
    AdditionalProperties = lastResponseMessage?.AdditionalProperties,   // <=
    CreatedAt = lastResponseMessage?.CreatedAt,                         // <=
  };
}
```

The PVS\-Studio warnings:  

* [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'lastResponseMessage' is always null\. The operator '?\.' is meaningless\. [SemanticKernelAIAgent\.cs 111](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Agents/Abstractions/AIAgent/SemanticKernelAIAgent.cs#L111)
* [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'lastResponseMessage' is always null\. The operator '?\.' is meaningless\. [SemanticKernelAIAgent\.cs 112](https://github.com/microsoft/semantic-kernel/blob/134e52e7540270ca4055bd5eab4d7606f9689ce1/dotnet/src/Agents/Abstractions/AIAgent/SemanticKernelAIAgent.cs#L112)

The `lastResponseMessage` variable initializes to `null`\. The code then uses its properties to initialize `AdditionalProperties` and `CreatedAt` of a new object\. Turns out, those properties will always initialize to `null`\.

Perhaps by the time of `AdditionalProperties` and `CreatedAt` initialization, `lastResponseMessage` should contain a non\-null value\. If the intention is to assign `null` to these properties, then authors can remove `lastResponseMessage`, as it only appears in the shown snippets\.

## Conclusion

Our analysis of the C\# portion of Semantic Kernel shows the expected picture: despite the project high profile and Microsoft involvement, the code contains typical issues\. This is not unusual—complex systems under active development almost always contain questionable spots and various defects\.

If you'd like to analyze your own project with PVS\-Studio, you can try the analyzer via this [link](https://pvs-studio.com/en/pvs-studio/try-free/)\.