ronantakizawa/codereview-bench huggingface dataset:
Repository: microsoft/semantic-kernel
File: dotnet/src/Agents/OpenAI/OpenAIResponseAgentInvokeOptions.cs (C#)

Code before:
﻿// Copyright (c) Microsoft. All rights reserved.

namespace Microsoft.SemanticKernel.Agents.OpenAI;

/// <summary>
/// Optional parameters for <see cref="OpenAIAssistantAgent"/> invocation.
/// </summary>
public sealed class OpenAIResponseAgentInvokeOptions : AgentInvokeOptions
{
    /// <summary>
    /// Initializes a new instance of the <see cref="OpenAIResponseAgentInvokeOptions"/> class.
    /// </summary>
    public OpenAIResponseAgentInvokeOptions()
    {
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="OpenAIResponseAgentInvokeOptions"/> class by cloning the provided options.
    /// </summary>
    /// <param name="options">The options to clone.</param>
    public OpenAIResponseAgentInvokeOptions(AgentInvokeOptions options)
        : base(options)
    {
        Verify.NotNull(options);
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="OpenAIResponseAgentInvokeOptions"/> class by cloning the provided options.
    /// </summary>
    /// <param name="options">The options to clone.</param>
    public OpenAIResponseAgentInvokeOptions(OpenAIResponseAgentInvokeOptions options)

Review comment (suggestion):
```suggestion
/// Optional parameters for <see cref="OpenAIResponseAgent"/> invocation.
```

Code after:
﻿// Copyright (c) Microsoft. All rights reserved.

namespace Microsoft.SemanticKernel.Agents.OpenAI;

/// <summary>
/// Optional parameters for <see cref="OpenAIResponseAgent"/> invocation.
/// </summary>
public sealed class OpenAIResponseAgentInvokeOptions : AgentInvokeOptions
{
    /// <summary>
    /// Initializes a new instance of the <see cref="OpenAIResponseAgentInvokeOptions"/> class.
    /// </summary>
    public OpenAIResponseAgentInvokeOptions()
    {
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="OpenAIResponseAgentInvokeOptions"/> class by cloning the provided options.
    /// </summary>
    /// <param name="options">The options to clone.</param>
    public OpenAIResponseAgentInvokeOptions(AgentInvokeOptions options)
        : base(options)
    {
        Verify.NotNull(options);
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="OpenAIResponseAgentInvokeOptions"/> class by cloning the provided options.
    /// </summary>
    /// <param name="options">The options to clone.</param>
    public OpenAIResponseAgentInvokeOptions(OpenAIResponseAgentInvokeOptions options)
Repository: microsoft/semantic-kernel
File: dotnet/src/Connectors/Connectors.Google/Extensions/GeminiKernelFunctionMetadataExtensions.cs (C#)

Code before:
﻿// Copyright (c) Microsoft. All rights reserved.

using System.Collections.Generic;

namespace Microsoft.SemanticKernel.Connectors.Google.Extensions;

/// <summary>
/// Extensions for <see cref="KernelFunctionMetadata"/> specific to the Gemini connector.
/// </summary>
public static class GeminiKernelFunctionMetadataExtensions
{
    /// <summary>
    /// Convert a <see cref="KernelFunctionMetadata"/> to an <see cref="GeminiFunction"/>.
    /// </summary>
    /// <param name="metadata">The <see cref="KernelFunctionMetadata"/> object to convert.</param>
    /// <returns>An <see cref="GeminiFunction"/> object.</returns>
    public static GeminiFunction ToGeminiFunction(this KernelFunctionMetadata metadata)
    {
        IReadOnlyList<KernelParameterMetadata> metadataParams = metadata.Parameters;

        var openAIParams = new GeminiFunctionParameter[metadataParams.Count];
        for (int i = 0; i < openAIParams.Length; i++)
        {
            var param = metadataParams[i];

            openAIParams[i] = new GeminiFunctionParameter(
                param.Name,
                GetDescription(param),
                param.IsRequired,
                param.ParameterType,

Review comment (refactor):
We can move this to be visible in the same namespace where the `KernelFunctionMetadata` is.

```suggestion
namespace Microsoft.SemanticKernel;
```

Code after:
﻿// Copyright (c) Microsoft. All rights reserved.

using System.Collections.Generic;
using Microsoft.SemanticKernel.Connectors.Google;

namespace Microsoft.SemanticKernel;

/// <summary>
/// Extensions for <see cref="KernelFunctionMetadata"/> specific to the Gemini connector.
/// </summary>
public static class GeminiKernelFunctionMetadataExtensions
{
    /// <summary>
    /// Convert a <see cref="KernelFunctionMetadata"/> to an <see cref="GeminiFunction"/>.
    /// </summary>
    /// <param name="metadata">The <see cref="KernelFunctionMetadata"/> object to convert.</param>
    /// <returns>An <see cref="GeminiFunction"/> object.</returns>
    public static GeminiFunction ToGeminiFunction(this KernelFunctionMetadata metadata)
    {
        IReadOnlyList<KernelParameterMetadata> metadataParams = metadata.Parameters;

        var openAIParams = new GeminiFunctionParameter[metadataParams.Count];
        for (int i = 0; i < openAIParams.Length; i++)
        {
            var param = metadataParams[i];

            openAIParams[i] = new GeminiFunctionParameter(
                param.Name,
                GetDescription(param),
                param.IsRequired,
Repository: microsoft/semantic-kernel
File: dotnet/src/Plugins/Plugins.StructuredData.EntityFramework/StructuredDataService.cs (C#)

Code before:
﻿// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Data.Entity;
using System.Data.Entity.Infrastructure;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
#if NET462
using Community.OData.Linq;
#else
using OData2Linq;
#endif

#pragma warning disable CA1308 // Normalize strings to uppercase

namespace Microsoft.SemanticKernel;

/// <summary>
/// Provides a structured data service for a database context.
/// </summary>
/// <typeparam name="TContext">Database context type.</typeparam>
public class StructuredDataService<TContext> : IDisposable where TContext : DbContext
{
    /// <summary>
    /// Gets the database context.
    /// </summary>
    public TContext Context { get; }

    /// <summary>
    /// Initializes a new instance with a connection string.
    /// </summary>
    /// <param name="connectionString">The connection string.</param>
    public StructuredDataService(string connectionString)

Review comment (bug):
@ArnaudB88 Thanks for the contribution.

This implementation is too specific about the NET462 and will fail for netstandard2.0, and other framework versions, suggest making the change only for NET_8_OR_GREATER, apply for others as this will only work if the runtime is .Net 8 or greater.

```suggestion
#if !NET_8_OR_GREATER
```

Code after:
﻿// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Data.Entity;
using System.Data.Entity.Infrastructure;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
#if NET8_0_OR_GREATER
using OData2Linq;
#else
using Community.OData.Linq;
#endif

#pragma warning disable CA1308 // Normalize strings to uppercase

namespace Microsoft.SemanticKernel;

/// <summary>
/// Provides a structured data service for a database context.
/// </summary>
/// <typeparam name="TContext">Database context type.</typeparam>
public class StructuredDataService<TContext> : IDisposable where TContext : DbContext
{
    /// <summary>
    /// Gets the database context.
    /// </summary>
    public TContext Context { get; }

    /// <summary>
    /// Initializes a new instance with a connection string.
    /// </summary>
    /// <param name="connectionString">The connection string.</param>
    public StructuredDataService(string connectionString)
Repository: microsoft/semantic-kernel
File: dotnet/src/Connectors/Connectors.Ollama/Settings/OllamaPromptExecutionSettings.cs (C#)

Code before:

        set
        {
            this.ThrowIfFrozen();
            this._numPredict = value;
        }
    }

    /// <inheritdoc/>
    public override void Freeze()
    {
        if (this.IsFrozen)
        {
            return;
        }

        base.Freeze();

        if (this._stop is not null)
        {
            this._stop = new System.Collections.ObjectModel.ReadOnlyCollection<string>(this._stop);
        }
    }

    /// <inheritdoc/>
    public override PromptExecutionSettings Clone()
    {
        return new OllamaPromptExecutionSettings()
        {
            ModelId = this.ModelId,
            ExtensionData = this.ExtensionData is not null ? new Dictionary<string, object>(this.ExtensionData) : null,
            Temperature = this.Temperature,
            TopP = this.TopP,
            TopK = this.TopK,
            NumPredict = this.NumPredict,
            Stop = this.Stop is not null ? new List<string>(this.Stop) : null,
            FunctionChoiceBehavior = this.FunctionChoiceBehavior,
        };
    }

    #region private

    private IList<string>? _stop;
    private float? _temperature;
    private float? _topP;
    private int? _topK;
    private int? _numPredict;

    #endregion
}


Review comment (refactor):
Ideally, this should be solved by adding a copy constructor to the PromptExecutionSettings, similar to how it was done for ChatOptions in this [PR](https://github.com/dotnet/extensions/blob/5858a796217aab02fe4bcabe67447987c2cd6530/src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatCompletion/ChatOptions.cs#L20), so that it can be reused for cloning by the PromptExecutionSettings class itself and its derivatives to initialize the properties of their base classes.

Code after:

        set
        {
            this.ThrowIfFrozen();
            this._numPredict = value;
        }
    }

    /// <inheritdoc/>
    public override void Freeze()
    {
        if (this.IsFrozen)
        {
            return;
        }

        base.Freeze();

        if (this._stop is not null)
        {
            this._stop = new System.Collections.ObjectModel.ReadOnlyCollection<string>(this._stop);
        }
    }

    /// <inheritdoc/>
    public override PromptExecutionSettings Clone()
    {
        return new OllamaPromptExecutionSettings()
        {
            ModelId = this.ModelId,
            ServiceId = this.ServiceId,
            ExtensionData = this.ExtensionData is not null ? new Dictionary<string, object>(this.ExtensionData) : null,
            Temperature = this.Temperature,
            TopP = this.TopP,
            TopK = this.TopK,
            NumPredict = this.NumPredict,
            Stop = this.Stop is not null ? new List<string>(this.Stop) : null,
            FunctionChoiceBehavior = this.FunctionChoiceBehavior,
        };
    }

    #region private

    private IList<string>? _stop;
    private float? _temperature;
    private float? _topP;
    private int? _topK;
    private int? _numPredict;

    #endregion
}
Repository: microsoft/semantic-kernel
File: dotnet/src/InternalUtilities/src/Diagnostics/ActivityExtensions.cs (C#)

Code before:
        {
            activity?.SetTag("gen_ai.tool.call.result", SerializeResult(result, jsonSerializerOptions));
        }
#pragma warning restore SKEXP0001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

        return activity;

        [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "By design. See comment below.")]
        [RequiresUnreferencedCode("Calls System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)")]
        [RequiresDynamicCode("Calls System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)")]
        static string SerializeResult(FunctionResult result, JsonSerializerOptions? jsonSerializerOptions)
        {
            // Try to retrieve the result as a string first
            try
            {
                return result.GetValue<string>() ?? string.Empty;
            }
            catch { }

            // Fallback to JSON serialization
            try
            {
                if (jsonSerializerOptions is not null)
                {
                    JsonTypeInfo<object?> typeInfo = (JsonTypeInfo<object?>)jsonSerializerOptions.GetTypeInfo(typeof(object));
                    return JsonSerializer.Serialize(result.GetValue<object?>(), typeInfo);
                }
                return JsonSerializer.Serialize(result.GetValue<object?>());
            }
            catch (NotSupportedException)
            {
                return "Failed to serialize result to Json";
            }
        }
    }

    /// <summary>
    /// Starts an activity with the specified name and tags.
    /// </summary>
    public static Activity? StartActivityWithTags(this ActivitySource source, string name, IEnumerable<KeyValuePair<string, object?>> tags, ActivityKind kind = ActivityKind.Internal)
        => source.StartActivity(name, kind, default(ActivityContext), tags);

    /// <summary>
    /// Adds tags to the activity.
    /// </summary>
    public static Activity SetTags(this Activity activity, ReadOnlySpan<KeyValuePair<string, object?>> tags)
    {
        foreach (var tag in tags)
        {
            activity.SetTag(tag.Key, tag.Value);
        }

Review comment (suggestion):
```suggestion
                    return JsonSerializer.Serialize(result.Value typeInfo);
```

Code after:

        try
        {
            while (true)
            {
                try
                {
                    if (!await resultEnumerator.MoveNextAsync())
                    {
                        break;
                    }
                }
                catch (Exception ex) when (activity is not null)
                {
                    activity.SetError(ex);
                    throw;
                }

                yield return resultEnumerator.Current;
            }
        }
        finally
        {
            await resultEnumerator.DisposeAsync();
        }
    }
}
Repository: microsoft/semantic-kernel
File: dotnet/src/InternalUtilities/src/Diagnostics/ActivityExtensions.cs (C#)

Code before:
        }
#pragma warning restore SKEXP0001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

        return activity;

        [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "By design. See comment below.")]
        [RequiresUnreferencedCode("Calls System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)")]
        [RequiresDynamicCode("Calls System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)")]
        static string SerializeResult(FunctionResult result, JsonSerializerOptions? jsonSerializerOptions)
        {
            // Try to retrieve the result as a string first
            try
            {
                return result.GetValue<string>() ?? string.Empty;
            }
            catch { }

            // Fallback to JSON serialization
            try
            {
                if (jsonSerializerOptions is not null)
                {
                    JsonTypeInfo<object?> typeInfo = (JsonTypeInfo<object?>)jsonSerializerOptions.GetTypeInfo(typeof(object));
                    return JsonSerializer.Serialize(result.GetValue<object?>(), typeInfo);
                }
                return JsonSerializer.Serialize(result.GetValue<object?>());
            }
            catch (NotSupportedException)
            {
                return "Failed to serialize result to Json";
            }
        }
    }

    /// <summary>
    /// Starts an activity with the specified name and tags.
    /// </summary>
    public static Activity? StartActivityWithTags(this ActivitySource source, string name, IEnumerable<KeyValuePair<string, object?>> tags, ActivityKind kind = ActivityKind.Internal)
        => source.StartActivity(name, kind, default(ActivityContext), tags);

    /// <summary>
    /// Adds tags to the activity.
    /// </summary>
    public static Activity SetTags(this Activity activity, ReadOnlySpan<KeyValuePair<string, object?>> tags)
    {
        foreach (var tag in tags)
        {
            activity.SetTag(tag.Key, tag.Value);
        }

        return activity;

Review comment (suggestion):
```suggestion
                return JsonSerializer.Serialize(result.Value);
```

Code after:
        {
            while (true)
            {
                try
                {
                    if (!await resultEnumerator.MoveNextAsync())
                    {
                        break;
                    }
                }
                catch (Exception ex) when (activity is not null)
                {
                    activity.SetError(ex);
                    throw;
                }

                yield return resultEnumerator.Current;
            }
        }
        finally
        {
            await resultEnumerator.DisposeAsync();
        }
    }
}
Repository: microsoft/semantic-kernel
File: dotnet/src/Agents/OpenAI/Internal/ResponseCreationOptionsFactory.cs (C#)

Code before:
                PreviousResponseId = responseAgentInvokeOptions.ResponseCreationOptions.PreviousResponseId,
            };
            creationOptions.Tools.AddRange(responseAgentInvokeOptions.ResponseCreationOptions.Tools);
            responseAgentInvokeOptions.ResponseCreationOptions.Metadata.ToList().ForEach(kvp => creationOptions.Metadata[kvp.Key] = kvp.Value);
        }
        else
        {
            creationOptions = new ResponseCreationOptions
            {
                EndUserId = agent.GetDisplayName(),
                Instructions = instructions,
                StoredOutputEnabled = agent.StoreEnabled,
            };
        }

        if (agent.StoreEnabled && agentThread.Id is not null)
        {
            creationOptions.PreviousResponseId = agentThread.Id;
        }

        var responseTools = agent.GetKernel(invokeOptions).Plugins
            .SelectMany(kp => kp.Select(kf => kf.ToResponseTool(kp.Name)));
        if (responseTools is not null && responseTools.Any())
        {
            creationOptions.Tools.AddRange(responseTools);
            if (creationOptions.ToolChoice == null)
            {
                creationOptions.ToolChoice = ResponseToolChoice.CreateAutoChoice();
            }
        }

        return creationOptions;
    }
}


Review comment (suggestion):
```suggestion
            if (creationOptions.ToolChoice is null)
```

Code after:
                PreviousResponseId = responseAgentInvokeOptions.ResponseCreationOptions.PreviousResponseId,
            };
            creationOptions.Tools.AddRange(responseAgentInvokeOptions.ResponseCreationOptions.Tools);
            responseAgentInvokeOptions.ResponseCreationOptions.Metadata.ToList().ForEach(kvp => creationOptions.Metadata[kvp.Key] = kvp.Value);
        }
        else
        {
            creationOptions = new ResponseCreationOptions
            {
                EndUserId = agent.GetDisplayName(),
                Instructions = instructions,
                StoredOutputEnabled = agent.StoreEnabled,
            };
        }

        if (agent.StoreEnabled && agentThread.Id is not null)
        {
            creationOptions.PreviousResponseId = agentThread.Id;
        }

        var responseTools = agent.GetKernel(invokeOptions).Plugins
            .SelectMany(kp => kp.Select(kf => kf.ToResponseTool(kp.Name)));
        if (responseTools is not null && responseTools.Any())
        {
            creationOptions.Tools.AddRange(responseTools);
            if (creationOptions.ToolChoice is null)
            {
                creationOptions.ToolChoice = ResponseToolChoice.CreateAutoChoice();
            }
        }

        return creationOptions;
    }
}
Repository: microsoft/terminal
File: src/cascadia/TerminalApp/Profile.cpp (C++)

Code before:
            }
        }
    }

    return defaultValue;
}

std::string_view Profile::_SerializeSnapOnOutput(SnapOnOutputFlag snapOnOutput)
{
    if (snapOnOutput == SnapOnOutputFlag::Never)
    {
        return SnapOnOutputNever;
    }
    else if (snapOnOutput == SnapOnOutputFlag::Always)
    {
        return SnapOnOutputAlways;
    }
    else if (snapOnOutput == SnapOnOutputFlag::AtBottom)
    {
        return SnapOnOutputAtBottom;
    }
    else if (snapOnOutput == SnapOnOutputFlag::NoSelection)
    {
        return SnapOnOutputNoSelection;
    }
    return "";
}

// Method Description:
// - Helper function for converting a user-specified scrollbar state to its corresponding enum
// Arguments:
// - The value from the settings.json file
// Return Value:
// - The corresponding enum value which maps to the string provided by the user
ScrollbarState Profile::ParseScrollbarState(const std::wstring& scrollbarState)
{
    if (scrollbarState == AlwaysVisible)
    {
        return ScrollbarState::Visible;
    }
    else if (scrollbarState == AlwaysHide)
    {
        return ScrollbarState::Hidden;
    }
    else
    {
        return ScrollbarState::Visible;
    }
}

// Method Description:

Review comment (bug):
We don't actually use the "serialize" functions anywhere (I think). So, I think this is incorrect, but I also don't know what the "correct" behavior should be. We're also not using it so I guess I'll just worry about this when we start serializing again?

Code after:
        return json.asBool() ? CloseOnExitMode::Graceful : CloseOnExitMode::Never;
    }

    if (json.isString())
    {
        auto closeOnExit = json.asString();
        if (closeOnExit == CloseOnExitAlways)
        {
            return CloseOnExitMode::Always;
        }
        else if (closeOnExit == CloseOnExitGraceful)
        {
            return CloseOnExitMode::Graceful;
        }
        else if (closeOnExit == CloseOnExitNever)
        {
            return CloseOnExitMode::Never;
        }
    }

    return CloseOnExitMode::Graceful;
}

// Method Description:
// - Helper function for converting a user-specified scrollbar state to its corresponding enum
// Arguments:
// - The value from the settings.json file
// Return Value:
// - The corresponding enum value which maps to the string provided by the user
ScrollbarState Profile::ParseScrollbarState(const std::wstring& scrollbarState)
{
    if (scrollbarState == AlwaysVisible)
    {
        return ScrollbarState::Visible;
    }
    else if (scrollbarState == AlwaysHide)
    {
        return ScrollbarState::Hidden;
    }
    else
    {
        return ScrollbarState::Visible;
    }
}

// Method Description:
// - Helper function for converting a user-specified image stretch mode
//   to the appropriate enum value
// Arguments:
// - The value from the settings.json file
// Return Value: