Skip to content

Commit c781da1

Browse files
octo-patchocto-patchrogerbarreto
authored
.Net: fix: address three static analysis issues (audio format, text search, KernelProcess) (#13925)
Fixes #13922 ## Problem Three static analysis issues identified by PVS-Studio in the .NET codebase: 1. **`ClientCore.ChatCompletion.cs`** — `GetAudioOutputMimeType` contained a duplicate `ChatOutputAudioFormat.Wav` check (lines 985 and 1000) making the second branch unreachable. The `Aac` audio format supported by the OpenAI SDK was silently unhandled, causing a `NotSupportedException` at runtime. 2. **`TextSearchStore.cs`** — `GetTextSearchResultsAsync` assigned a LINQ expression to a `results` variable (deferred execution) that was never iterated. The return statement duplicated the projection. The unused variable was dead code. 3. **`KernelProcess.cs`** — The constructor accepted a `threads` parameter but never assigned it to the `Threads` property, silently discarding all threads passed by callers. ## Solution 1. Replace the second `Wav` branch with an `Aac` branch and update the error message to list `'aac'` as a supported format. 2. Reuse the `results` variable in the `return` statement instead of duplicating the `Select` projection. 3. Assign the `threads` parameter to `this.Threads` when it is not null. ## Testing - Changes are logic-only with no new external dependencies. - Existing unit tests for `OpenAIChatCompletionService` and `TextSearchStore` continue to apply. - No new runtime behaviour is introduced for the `Wav` fix (it was already handled earlier in the method); `Aac` now maps to `"audio/aac"` as expected. --------- Co-authored-by: octo-patch <octo-patch@github.com> Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>
1 parent dd026f3 commit c781da1

6 files changed

Lines changed: 160 additions & 9 deletions

File tree

dotnet/src/Connectors/Connectors.OpenAI.UnitTests/Services/OpenAIChatCompletionServiceTests.cs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1981,6 +1981,16 @@ public async Task OnAutoFunctionInvocationAsync(AutoFunctionInvocationContext co
19811981
{ "{\"voice\":\"echo\",\"format\":\"opus\"}", "{\"voice\":\"echo\",\"format\":\"opus\"}" },
19821982
};
19831983

1984+
public static TheoryData<ChatOutputAudioFormat, string> AudioMimeTypeMappingData => new()
1985+
{
1986+
{ ChatOutputAudioFormat.Wav, "audio/wav" },
1987+
{ ChatOutputAudioFormat.Aac, "audio/aac" },
1988+
{ ChatOutputAudioFormat.Mp3, "audio/mp3" },
1989+
{ ChatOutputAudioFormat.Opus, "audio/opus" },
1990+
{ ChatOutputAudioFormat.Flac, "audio/flac" },
1991+
{ ChatOutputAudioFormat.Pcm16, "audio/pcm16" },
1992+
};
1993+
19841994
#pragma warning disable CS8618, CA1812
19851995
private sealed class MathReasoning
19861996
{
@@ -2191,6 +2201,56 @@ public async Task ItHandlesAudioContentWithMetadataInResponseAsync()
21912201
// The ExpiresAt value is converted to a DateTime object, so we can't directly compare it to the Unix timestamp
21922202
}
21932203

2204+
[Theory]
2205+
[MemberData(nameof(AudioMimeTypeMappingData))]
2206+
public async Task ItMapsAudioOutputFormatToCorrectMimeTypeAsync(ChatOutputAudioFormat format, string expectedMimeType)
2207+
{
2208+
// Arrange
2209+
var chatCompletion = new OpenAIChatCompletionService(modelId: "gpt-4o", apiKey: "NOKEY", httpClient: this._httpClient);
2210+
2211+
var responseJson = """
2212+
{
2213+
"model": "gpt-4o",
2214+
"choices": [
2215+
{
2216+
"message": {
2217+
"role": "assistant",
2218+
"content": "This is the text response.",
2219+
"audio": {
2220+
"data": "AQIDBA=="
2221+
}
2222+
},
2223+
"finish_reason": "stop"
2224+
}
2225+
],
2226+
"usage": {
2227+
"prompt_tokens": 10,
2228+
"completion_tokens": 20,
2229+
"total_tokens": 30
2230+
}
2231+
}
2232+
""";
2233+
2234+
this._messageHandlerStub.ResponseToReturn = new HttpResponseMessage(HttpStatusCode.OK)
2235+
{ Content = new StringContent(responseJson) };
2236+
2237+
var settings = new OpenAIPromptExecutionSettings
2238+
{
2239+
Modalities = ChatResponseModalities.Text | ChatResponseModalities.Audio,
2240+
Audio = new ChatAudioOptions(ChatOutputAudioVoice.Alloy, format)
2241+
};
2242+
2243+
// Act
2244+
var result = await chatCompletion.GetChatMessageContentAsync(this._chatHistoryForTest, settings);
2245+
2246+
// Assert
2247+
Assert.NotNull(result);
2248+
Assert.Equal(2, result.Items.Count);
2249+
var audioContent = result.Items[1] as AudioContent;
2250+
Assert.NotNull(audioContent);
2251+
Assert.Equal(expectedMimeType, audioContent.MimeType);
2252+
}
2253+
21942254
[Fact]
21952255
public async Task GetChatMessageContentsThrowsExceptionWithEmptyBinaryContentAsync()
21962256
{

dotnet/src/Connectors/Connectors.OpenAI/Core/ClientCore.ChatCompletion.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,9 +1006,9 @@ private OpenAIChatMessageContent CreateChatMessageContent(OAIChat.ChatCompletion
10061006
return "audio/opus";
10071007
}
10081008

1009-
if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Wav)
1009+
if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Aac)
10101010
{
1011-
return "audio/wav";
1011+
return "audio/aac";
10121012
}
10131013

10141014
if (audioOptions.OutputAudioFormat == ChatOutputAudioFormat.Flac)
@@ -1021,7 +1021,7 @@ private OpenAIChatMessageContent CreateChatMessageContent(OAIChat.ChatCompletion
10211021
return "audio/pcm16";
10221022
}
10231023

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

10271027
private OpenAIChatMessageContent CreateChatMessageContent(ChatMessageRole chatRole, string content, ChatToolCall[] toolCalls, FunctionCallContent[]? functionCalls, IReadOnlyDictionary<string, object?>? metadata, string? authorName)

dotnet/src/Experimental/Process.Abstractions/KernelProcess.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
using System;
44
using System.Collections.Generic;
5+
using System.Linq;
56
using Microsoft.SemanticKernel.Process.Internal;
67
using Microsoft.SemanticKernel.Process.Models;
78

@@ -50,5 +51,9 @@ public KernelProcess(KernelProcessState state, IList<KernelProcessStepInfo> step
5051
Verify.NotNullOrWhiteSpace(state.Name);
5152

5253
this.Steps = [.. steps];
54+
if (threads is not null)
55+
{
56+
this.Threads = threads.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
57+
}
5358
}
5459
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
3+
using System.Collections.Generic;
4+
using Xunit;
5+
6+
namespace Microsoft.SemanticKernel.Process.UnitTests;
7+
8+
/// <summary>
9+
/// Unit tests for <see cref="KernelProcess"/>.
10+
/// </summary>
11+
public class KernelProcessTests
12+
{
13+
/// <summary>
14+
/// Verifies that the <see cref="KernelProcess"/> constructor assigns the supplied
15+
/// <c>threads</c> dictionary to the <see cref="KernelProcess.Threads"/> property
16+
/// instead of silently discarding it.
17+
/// </summary>
18+
[Fact]
19+
public void Constructor_AssignsThreadsParameter_WhenProvided()
20+
{
21+
// Arrange
22+
var state = new KernelProcessState(name: "TestProcess", version: "v1", id: "p1");
23+
var steps = new List<KernelProcessStepInfo>();
24+
var threads = new Dictionary<string, KernelProcessAgentThread>
25+
{
26+
["main"] = new KernelProcessAgentThread { ThreadName = "main", ThreadId = "t-1" },
27+
["aux"] = new KernelProcessAgentThread { ThreadName = "aux", ThreadId = "t-2" },
28+
};
29+
30+
// Act
31+
var process = new KernelProcess(state, steps, edges: null, threads: threads);
32+
33+
// Assert
34+
Assert.Equal(2, process.Threads.Count);
35+
Assert.True(process.Threads.ContainsKey("main"));
36+
Assert.True(process.Threads.ContainsKey("aux"));
37+
Assert.Equal("t-1", process.Threads["main"].ThreadId);
38+
Assert.Equal("t-2", process.Threads["aux"].ThreadId);
39+
}
40+
41+
/// <summary>
42+
/// Verifies that the <see cref="KernelProcess"/> constructor leaves
43+
/// <see cref="KernelProcess.Threads"/> as an empty dictionary when the
44+
/// <c>threads</c> argument is null.
45+
/// </summary>
46+
[Fact]
47+
public void Constructor_DefaultsThreadsToEmptyDictionary_WhenNull()
48+
{
49+
// Arrange
50+
var state = new KernelProcessState(name: "TestProcess", version: "v1", id: "p2");
51+
var steps = new List<KernelProcessStepInfo>();
52+
53+
// Act
54+
var process = new KernelProcess(state, steps, edges: null, threads: null);
55+
56+
// Assert
57+
Assert.NotNull(process.Threads);
58+
Assert.Empty(process.Threads);
59+
}
60+
}

dotnet/src/SemanticKernel.Core/Data/TextSearchStore/TextSearchStore.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,7 @@ public async Task<KernelSearchResults<TextSearchResult>> GetTextSearchResultsAsy
202202
var searchResult = await this.SearchInternalAsync(query, searchOptions, cancellationToken).ConfigureAwait(false);
203203

204204
var results = searchResult.Select(x => new TextSearchResult(x.Text ?? string.Empty) { Name = x.SourceName, Link = x.SourceLink });
205-
return new(searchResult.Select(x =>
206-
new TextSearchResult(x.Text ?? string.Empty)
207-
{
208-
Name = x.SourceName,
209-
Link = x.SourceLink
210-
}).ToAsyncEnumerable());
205+
return new(results.ToAsyncEnumerable());
211206
}
212207

213208
/// <inheritdoc/>

dotnet/src/SemanticKernel.UnitTests/Data/TextSearchStoreTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,37 @@ public async Task SearchAsyncReturnsSearchResults()
245245
Assert.Equal("Sample text", actualResultsList[0]);
246246
}
247247

248+
[Fact]
249+
public async Task GetTextSearchResultsAsyncReturnsTextSearchResults()
250+
{
251+
// Arrange
252+
var mockResults = new List<VectorSearchResult<TextSearchStore<string>.TextRagStorageDocument<string>>>
253+
{
254+
new(new TextSearchStore<string>.TextRagStorageDocument<string>
255+
{
256+
Text = "Sample text",
257+
SourceName = "src-name",
258+
SourceLink = "src-link",
259+
}, 0.9f)
260+
};
261+
262+
this._recordCollectionMock
263+
.Setup(r => r.SearchAsync("query", 3, It.IsAny<VectorSearchOptions<TextSearchStore<string>.TextRagStorageDocument<string>>>(), It.IsAny<CancellationToken>()))
264+
.Returns(mockResults.ToAsyncEnumerable());
265+
266+
using var store = new TextSearchStore<string>(this._vectorStoreMock.Object, "testCollection", 128);
267+
268+
// Act
269+
var actualResults = await store.GetTextSearchResultsAsync("query");
270+
271+
// Assert
272+
var actualResultsList = await actualResults.Results.ToListAsync();
273+
Assert.Single(actualResultsList);
274+
Assert.Equal("Sample text", actualResultsList[0].Value);
275+
Assert.Equal("src-name", actualResultsList[0].Name);
276+
Assert.Equal("src-link", actualResultsList[0].Link);
277+
}
278+
248279
[Fact]
249280
public async Task SearchAsyncWithHybridReturnsSearchResults()
250281
{

0 commit comments

Comments
 (0)