# ИИ что? Проверяем Semantic Kernel

Semantic Kernel — SDK от Microsoft для интеграции моделей искусственного интеллекта в приложения\. Сможет ли статический анализатор PVS\-Studio отыскать дефекты в исходниках такого проекта? Постараемся ответить на этот вопрос в статье\. Приятного чтения\!

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

Проекты, связанные с интеграцией искусственного интеллекта, всё чаще становятся частью повседневной разработки\. Один из таких проектов — [Semantic Kernel](https://github.com/microsoft/semantic-kernel)\. Он представляет собой SDK для построения AI\-агентов и оркестрации LLM\-сценариев и активно развивается компанией Microsoft\.

Однако под капотом даже самых современных решений скрывается вполне обычный C\# код — со всеми присущими ему проблемами\. В этом мы с вами сегодня постараемся убедиться\.

Для проверки С\# части проекта использовался статический анализатор [PVS\-Studio версии 7\.41](https://pvs-studio.ru/ru/pvs-studio/)\. Проверяемый исходный код соответствует [коммиту](https://github.com/microsoft/semantic-kernel/tree/134e52e7540270ca4055bd5eab4d7606f9689ce1)\. 

Стоит отметить, что мы расскажем только о самых интересных для рассмотрения фрагментах кода, чтобы не затягивать статью\.

Приступим к разбору подозрительных мест\!

## Приоритеты расставлены неправильно

**Фрагмент кода 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)
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3177](https://pvs-studio.ru/ru/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)

Скорее всего, предполагалась, что условие будет работать так: если `knowledgeBase.Options?.TryGetValue(KnowledgeBaseId, out var value)` не `null`, то проверяется, что `value is not null && value is string`, однако поведение будет иным\. Оператор `&&` имеет более высокий приоритет, чем `??`, поэтому проверки для `value` не будут выполнены\.

Ещё один маркер того, что здесь допущена ошибка — правый операнд `??`\. Он имеет вид: `false && value is not null && value is string`\. Нетрудно догадаться, каков будет результат его выполнения\. Условие содержит только операторы `&&`, а один из операндов — `false`\. Следовательно, всё выражение тоже `false`\.



Для исправления нужно добавить скобки:

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

Стоит отметить, что есть ещё 4 места в исходном коде, которые содержат аналогичную проблему:

* [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)

**Фрагмент кода 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;
};
```

Предупреждение PVS\-Studio: [V3088](https://pvs-studio.ru/ru/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)

Скобки верхнего уровня полностью обрамляют условие `if`, что делает их бесполезными\. Возможно, предполагалась следующая логика: применять проверку имени параметра `payload` для обеих операций \(`me_sendMail` и `me_calendar_CreateEvents`\)\. Однако фактически проверка `payload` выполняется только для второй операции, так как приоритет `&&` выше, чем у `||`\. Для исправления нужно взять в скобки только проверки для `me_sendMail` и `me_calendar_CreateEvents`, а не всё выражение условия целиком\.

## Забыли

**Фрагмент кода 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'."
  );
}
```

Предупреждение PVS\-Studio: [V3021](https://pvs-studio.ru/ru/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)

В методе `GetAudioOutputMimeType` дважды встречается одинаковая проверка: `audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav`\. Повторная проверка на соответствие `ChatOutputAudioFormat.Wav` является недостижимым кодом — при первом совпадении выполнение метода завершится\. Это типичная ошибка, которая может быть допущена при механическом копировании\.

Что примечательно, в структуре `ChatOutputAudioFormat` объявлен формат `ChatOutputAudioFormat Aac`\. Это единственный формат, который не обрабатывается в методе `GetAudioOutputMimeType`\. Возможно, именно он должен быть вместо одной из проверок `audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav`\.

```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);
  ....
}
```

**Фрагмент кода 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());
}
```

Предупреждение PVS\-Studio: [V3220](https://pvs-studio.ru/ru/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)

Переменной `results` присваивается результат вызова LINQ метода, после чего переменная не используется\. Многие LINQ методы имеют [отложенное выполнение](https://learn.microsoft.com/en-us/dotnet/csharp/linq/get-started/introduction-to-linq-queries#deferred)\. Соответственно, вычисление значения, получаемого из LINQ выражения, не осуществляется при формировании этого выражения\. Оно будет вычислено при обходе полученной последовательности \(например, в `foreach`\)\.

Сразу после присваивания значения переменной `results` идёт `return` с аналогичным LINQ выражением\. Скорее всего, одно из LINQ выражений лишнее\. Нужно либо вообще убрать переменную `results`, либо использовать её при формировании возвращаемого значения: 

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

**Фрагмент кода 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];
}
```

Предупреждение PVS\-Studio: [V3117](https://pvs-studio.ru/ru/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)

Параметр `threads` не был использован\. При некоторых вызовах метода `KernelProcess` передаётся аргумент, соответствующий параметру `threads`\. Получается, что вызывающий код рассчитывает на использование этого параметра\. 

Стоит отметить, что в классе `KernelProcess` есть свойство `Threads`, возможно, именно оно должно инициализироваться неиспользуемым параметром\.

## Выход за границу

**Фрагмент кода 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;
}
```

Предупреждение PVS\-Studio: [V3106](https://pvs-studio.ru/ru/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)

Переменная `truncationIndex` инициализируется значением `-1`\. Если коллекция `chatHistory` будет пустой, то поток выполнения не зайдёт в цикл `for`, так как перед первой итерацией выполнится проверка `-1 >= 0`\. Следовательно, значение для `truncationIndex` не поменяется\. Сразу после `for` идёт цикл `while`, условие которого будет истинным, если в `chatHistory` ноль элементов \(`-1 < 0`\)\. В теле `while` в качестве индекса используется переменная `truncationIndex`, которая может иметь значение `-1`\. При обращении по отрицательному индексу будет выброшено исключение `IndexOutOfRangeException`\.

Для исправления нужно проверять, что индекс не отрицательный, перед обращением по нему\.

## Проблемы с null

**Фрагмент кода 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; }
```

Предупреждение PVS\-Studio: [V3080](https://pvs-studio.ru/ru/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)

В условии проверяется, что `this.State` не соответствует типу `KernelProcessStepState`\. Если посмотреть на объявление свойства `State`, можно увидеть, что оно имеет тип `KernelProcessStepState`\. Получается, что условие всегда ложное? На самом деле это не совсем так\. Если `this.State` будет иметь значение `null`, то `this.State is not KernelProcessStepState` — `true`\. 

Ожидается, что в блоке `then` оператора `if` будет выброшено исключение типа `KernelException`, однако поведение будет иным\. Так как в сообщении исключения обращаются к свойствам `this.State`, а единственное условие, при котором поток выполнения зайдёт в `then` блок `this.State` — `null`, будет выброшено исключение `NullReferenceException`\.

**Фрагмент кода 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());   // <=
}
```

Предупреждение PVS\-Studio: [V3095](https://pvs-studio.ru/ru/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)

К переменной `mediaTypeMetadata` сначала обращаются без проверки на `null`, а буквально на соседней строчке с проверкой `?.`\. Возможно, проверка во втором случае избыточна, тогда её стоит убрать\. Если же `mediaTypeMetadata` может иметь значение `null`, то при первом обращении будет выброшено исключение `NullReferenceException`\.

**Фрагмент кода 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);
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3095](https://pvs-studio.ru/ru/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._logger` в одном и том же контексте используется то с проверкой, то без неё\. Стоит отметить, что помимо представленного кода, в методе `GetNextStepAsync` есть ещё два места, где `this._logger` не проверяется на `null`\. Как и в предыдущем примере, здесь присутствует проблема согласованности возможного значения `null` для `this._logger`\.

**Фрагмент кода 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,                         // <=
  };
}
```

Предупреждения PVS\-Studio:  

* [V3022](https://pvs-studio.ru/ru/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.ru/ru/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)

Переменной `lastResponseMessage` присваивается значение `null`\. С помощью свойств этой переменной инициализируются `AdditionalProperties` и `CreatedAt` нового объекта\. Получается, что они всегда будут инициализироваться значением `null`\.

Возможно, на момент инициализации `AdditionalProperties` и `CreatedAt` переменная `lastResponseMessage` должна иметь значение, отличное от `null`\. Если же свойства должны инициализироваться значением `null`, то `lastResponseMessage` нужно убрать, так как она фигурирует только в рассматриваемых местах\.

## Заключение

Проверка C\# части Semantic Kernel показала ожидаемую картину: несмотря на высокий уровень проекта и участие крупной компании, код не лишён типичных проблем\. Это не является чем\-то необычным — сложные системы с активной разработкой почти всегда содержат спорные места и различные дефекты\.

Если вы хотите самостоятельно проверить проект с помощью PVS\-Studio, то попробовать анализатор можете по [ссылке](https://pvs-studio.ru/ru/pvs-studio/try-free/)\!