.Net: fix: address three static analysis issues (audio format, text search, KernelProcess)

[octo-patch]

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.

[github-actions]

Automated Code Review

Reviewers: 4 | Confidence: 93%

✓ Correctness

Three clean bug fixes: (1) A duplicate dead-code Wav format check is replaced with a new Aac format handler, correctly adding AAC support while the original Wav handler at line 985 remains untouched. The error message is updated accordingly. (2) The KernelProcess constructor was silently ignoring the threads parameter; the fix assigns it to this.Threads when non-null, which is correct since the property (line 23) has an init accessor with a default empty dictionary. (3) TextSearchStore.GetTextSearchResultsAsync had a computed results variable that was never used, with the return statement redundantly re-projecting from searchResult; the fix correctly uses the already-computed results variable.

✓ Security Reliability

Three straightforward bug fixes with no security or reliability concerns. (1) Audio format: replaces a duplicate/dead WAV check with new AAC format support — WAV is still handled at line 985. (2) KernelProcess: fixes a data-loss bug where the threads constructor parameter was silently ignored. (3) TextSearchStore: removes a redundant duplicate LINQ projection, using the already-computed results variable instead.

✓ Test Coverage

All three changes are correct bug fixes, but none of them include tests. The AAC audio format MIME type mapping is new behavior with no test coverage. The KernelProcess threads constructor fix resolves a silent data-loss bug (threads parameter was accepted but discarded) with no regression test. The TextSearchStore fix is a safe cleanup of dead code with functionally equivalent behavior. While the code changes themselves are sound, the first two fixes especially warrant corresponding tests to prevent regressions.

✗ Design Approach

The audio and text-search changes look directionally fine, but the KernelProcess update is too narrow for the persistence problem it appears to target. It now honors the optional threads constructor argument for direct callers, yet the main round-trip paths that serialize and reconstruct KernelProcess still do not carry Threads, so thread-backed processes will continue to lose that state when they flow through Dapr or persisted metadata.

Flagged Issues

• KernelProcess now copies the optional threads argument, but the Dapr and state-metadata round-trip paths still drop thread definitions entirely (Experimental/Process.Runtime.Dapr/DaprProcessInfo.cs:21,57 and InternalUtilities/process/Abstractions/KernelProcessStateMetadataFactory.cs:16-27). This is a symptom-level fix for one caller, not a complete fix for process thread persistence.

Suggestions

• Add a unit test for the KernelProcess constructor verifying that when threads are passed, they are stored in the Threads property. Currently no test exercises this code path — LocalProcess.ToKernelProcessAsync() (LocalProcess.cs:611) passes threads via the constructor and would have silently lost them before this fix.

---

Automated review by octo-patch's agents

[github-actions]

Incomplete fix / test gap: This only addresses the direct-constructor path. LocalProcess.ToKernelProcessAsync already passes this._process.Threads (LocalProcess.cs:611), but Dapr round-trips still serialize only Steps (DaprProcessInfo.cs:21) and rebuild KernelProcess without threads (DaprProcessInfo.cs:57); ProcessStateMetadataFactory likewise persists only StepsState. If the goal is to preserve threads across snapshots/resume, Threads should be added to the persisted/transport models. At minimum, add a regression test that constructs a KernelProcess with a non-null threads argument and asserts process.Threads matches.