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

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

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.SemanticKernel.Agents;

/// <summary>
/// Represents an input for an agent.
/// </summary>
[Experimental("SKEXP0110")]
public sealed class AgentInput
{
    /// <summary>
    /// Gets or sets the name of the input.
    /// </summary>
    /// <remarks>
    /// This can be either a string, number, array, object, or boolean.
    /// </remarks>
    public string? Type { get; set; }

    /// <summary>
    /// Gets or sets the name of the input.
    /// </summary>
    public string? Name { get; set; }

    /// <summary>
    /// Gets or sets a description of the input.
    /// </summary>
    public string? Description { get; set; }

    /// <summary>
    /// Gets or sets a default value for the input.
    /// </summary>
    public object? Default { get; set; }

    /// <summary>
    /// Gets or sets whether the input is considered required (rather than optional).
    /// </summary>
    /// <remarks>

Review comment (suggestion):
```suggestion
    /// Gets or sets the type of the input.
```

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

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.SemanticKernel.Agents;

/// <summary>
/// Represents an input for an agent.
/// </summary>
[Experimental("SKEXP0110")]
public sealed class AgentInput
{
    /// <summary>
    /// Gets or sets the type of the input.
    /// </summary>
    /// <remarks>
    /// This can be either a string, number, array, object, or boolean.
    /// </remarks>
    public string? Type { get; set; }

    /// <summary>
    /// Gets or sets the name of the input.
    /// </summary>
    public string? Name { get; set; }

    /// <summary>
    /// Gets or sets a description of the input.
    /// </summary>
    public string? Description { get; set; }

    /// <summary>
    /// Gets or sets a default value for the input.
    /// </summary>
    public object? Default { get; set; }

    /// <summary>
    /// Gets or sets whether the input is considered required (rather than optional).
    /// </summary>
    /// <remarks>
Repository: microsoft/semantic-kernel
File: dotnet/src/Agents/Abstractions/Definition/AgentOutput.cs (C#)

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

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.SemanticKernel.Agents;

/// <summary>
/// Represents an output for an agent.
/// </summary>
[Experimental("SKEXP0110")]
public sealed class AgentOutput
{
    /// <summary>
    /// Gets or sets the name of the output.
    /// </summary>
    /// <remarks>
    /// This can be either a string, number, array, object, or boolean.
    /// </remarks>
    public string? Type { get; set; }

    /// <summary>
    /// Gets or sets the name of the output.
    /// </summary>
    public string? Name { get; set; }

    /// <summary>
    /// Gets or sets a description of the output.
    /// </summary>
    public string? Description { get; set; }

    /// <summary>
    /// Gets or sets JSON Schema describing this output.
    /// </summary>
    public string? JsonSchema { get; set; }
}


Review comment (suggestion):
```suggestion
    /// Gets or sets the type of the output.
```

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

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.SemanticKernel.Agents;

/// <summary>
/// Represents an output for an agent.
/// </summary>
[Experimental("SKEXP0110")]
public sealed class AgentOutput
{
    /// <summary>
    /// Gets or sets the type of the output.
    /// </summary>
    /// <remarks>
    /// This can be either a string, number, array, object, or boolean.
    /// </remarks>
    public string? Type { get; set; }

    /// <summary>
    /// Gets or sets the name of the output.
    /// </summary>
    public string? Name { get; set; }

    /// <summary>
    /// Gets or sets a description of the output.
    /// </summary>
    public string? Description { get; set; }

    /// <summary>
    /// Gets or sets JSON Schema describing this output.
    /// </summary>
    public string? JsonSchema { get; set; }
}
Repository: microsoft/semantic-kernel
File: dotnet/src/Agents/Abstractions/Definition/AgentInput.cs (C#)

Code before:

    /// <summary>
    /// Gets or sets a description of the input.
    /// </summary>
    public string? Description { get; set; }

    /// <summary>
    /// Gets or sets a default value for the input.
    /// </summary>
    public object? Default { get; set; }

    /// <summary>
    /// Gets or sets whether the input is considered required (rather than optional).
    /// </summary>
    /// <remarks>
    /// The default is true.
    /// </remarks>
    public bool Required { get; set; } = true;

    /// <summary>
    /// Gets or sets JSON Schema describing this input.
    /// </summary>
    public string? JsonSchema { get; set; }

    /// <summary>
    /// Gets or sets a value indicating whether the input can emit structural text.
    /// </summary>
    /// <remarks>
    /// The default is true.
    /// When set to false the value of the input is treated as safe content i.e. the input can emit structural text.
    /// </remarks>
    public bool Strict { get; set; } = true;

    /// <summary>
    /// Gets or sets a sample value for the input.
    /// </summary>
    /// <remarks>
    /// This is used to provide examples to the user of the agent.
    /// They can also be used to provide examples to the tooling that will be used to load and execute the agent.
    /// If this section is not included, the runtime will use the default value for the input.
    /// </remarks>
    public object? Sample { get; set; }
}


Review comment (suggestion):
```suggestion
    /// Gets or sets a value indicating whether the input can contain structural text.
```

Code after:

    /// <summary>
    /// Gets or sets a description of the input.
    /// </summary>
    public string? Description { get; set; }

    /// <summary>
    /// Gets or sets a default value for the input.
    /// </summary>
    public object? Default { get; set; }

    /// <summary>
    /// Gets or sets whether the input is considered required (rather than optional).
    /// </summary>
    /// <remarks>
    /// The default is true.
    /// </remarks>
    public bool Required { get; set; } = true;

    /// <summary>
    /// Gets or sets JSON Schema describing this input.
    /// </summary>
    public string? JsonSchema { get; set; }

    /// <summary>
    /// Gets or sets a value indicating whether the input can contain structural text.
    /// </summary>
    /// <remarks>
    /// The default is true.
    /// When set to false the value of the input is treated as safe content i.e. the input can emit structural text.
    /// </remarks>
    public bool Strict { get; set; } = true;

    /// <summary>
    /// Gets or sets a sample value for the input.
    /// </summary>
    /// <remarks>
    /// This is used to provide examples to the user of the agent.
    /// This can also be used by developer tooling as the value to use without needing to prompt the developer to enter a value.
    /// </remarks>
    public object? Sample { get; set; }
}
Repository: microsoft/semantic-kernel
File: dotnet/src/SemanticKernel.Core/PromptTemplate/KernelPromptTemplate.cs (C#)

Code before:

        void AddIfMissing(string variableName)
        {
            if (!string.IsNullOrEmpty(variableName) && seen.Add(variableName))
            {
                config.InputVariables.Add(new InputVariable { Name = variableName });
            }
        }
    }

    private static bool ShouldEncodeTags(bool disableTagEncoding, HashSet<string> safeBlocks, Block block)
    {
        if (block is VarBlock varBlock)
        {
            return !safeBlocks.Contains(varBlock.Name);
        }

        return !disableTagEncoding && block is not TextBlock;
    }

    private static string? ConvertToString(object? value, CultureInfo? culture = null)
    {
        if (value is null) { return null; }

        return value is IEnumerable<string> enumerableString
            ? string.Join(" ", enumerableString)
            : InternalTypeConverter.ConvertToString(value, culture);
    }
    #endregion
}


Review comment (suggestion):
Didn't we say that we should only convert if it's a known concrete type, because others may have implemented their own tostring logic.
So:
```suggestion
        return value.GetType == typeof(List<string>)
            ? string.Join(" ", (List<string>)value)
```
We also cannot use is since it will also match derived types.

Code after:

        void AddIfMissing(string variableName)
        {
            if (!string.IsNullOrEmpty(variableName) && seen.Add(variableName))
            {
                config.InputVariables.Add(new InputVariable { Name = variableName });
            }
        }
    }

    private static bool ShouldEncodeTags(bool disableTagEncoding, HashSet<string> safeBlocks, Block block)
    {
        if (block is VarBlock varBlock)
        {
            return !safeBlocks.Contains(varBlock.Name);
        }

        return !disableTagEncoding && block is not TextBlock;
    }

    private static string? ConvertToString(object? value, CultureInfo? culture = null)
    {
        if (value is null) { return null; }

        return value is List<string> stringList
            ? string.Join("\n", stringList)
            : InternalTypeConverter.ConvertToString(value, culture);
    }
    #endregion
}
Repository: microsoft/semantic-kernel
File: dotnet/src/IntegrationTests/Connectors/Memory/SqliteVec/SqliteVectorStoreFixture.cs (C#)

Code before:
{
    private readonly string _databasePath = Path.GetTempFileName();

    public string ConnectionString => $"Data Source={this._databasePath}";

    public SqliteCollection<TKey, TRecord> GetCollection<TKey, TRecord>(
        string collectionName,
        SqliteCollectionOptions? options = default)
        where TKey : notnull
        where TRecord : class
    {
        return new SqliteCollection<TKey, TRecord>(
            this.ConnectionString,
            collectionName,
            options);
    }

    public SqliteDynamicCollection GetDynamicCollection(
        string collectionName,
        SqliteCollectionOptions options)
    {
        return new SqliteDynamicCollection(
            this.ConnectionString,
            collectionName,
            options);
    }

    public void Dispose()
    {
        this.Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            File.Delete(this._databasePath);
        }
    }
}


Review comment (suggestion):
nit: expression-bodied:

```suggestion
    => new SqliteDynamicCollection(
        this.ConnectionString,
        collectionName,
        options);
```

Code after:
{
    private readonly string _databasePath = Path.GetTempFileName();

    public string ConnectionString => $"Data Source={this._databasePath}";

    public SqliteCollection<TKey, TRecord> GetCollection<TKey, TRecord>(
        string collectionName,
        SqliteCollectionOptions? options = default)
        where TKey : notnull
        where TRecord : class
        => new(
            this.ConnectionString,
            collectionName,
            options);

    public SqliteDynamicCollection GetDynamicCollection(
        string collectionName,
        SqliteCollectionOptions options)
        => new(
            this.ConnectionString,
            collectionName,
            options);

    public void Dispose()
    {
        this.Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            File.Delete(this._databasePath);
        }
    }
}
Repository: microsoft/semantic-kernel
File: dotnet/src/Connectors/VectorData.Abstractions/RecordDefinition/VectorStoreKeyProperty.cs (C#)

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

using System;

namespace Microsoft.Extensions.VectorData;

/// <summary>
/// Defines a key property on a vector store record.
/// </summary>
/// <remarks>
/// The characteristics defined here influence how the property is treated by the vector store.
/// </remarks>
public sealed class VectorStoreKeyProperty : VectorStoreProperty
{
    /// <summary>
    /// Initializes a new instance of the <see cref="VectorStoreKeyProperty"/> class.
    /// </summary>
    /// <param name="name">The name of the property on the data model. If the record is mapped to a .NET type, this corresponds to the .NET property name on that type.</param>
    /// <param name="type">The type of the property. Required for dynamic mapping, but can omitted when mapping a .NET type.</param>
    public VectorStoreKeyProperty(string name, Type? type = null)
        : base(name, type)
    {
    }
}


Review comment (suggestion):
```suggestion
    /// <param name="type">The type of the property. Required when using a record type of `Dictionary<string, object?>`, but can omitted when mapping any other .NET type.</param>
```

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

using System;

namespace Microsoft.Extensions.VectorData;

/// <summary>
/// Defines a key property on a vector store record.
/// </summary>
/// <remarks>
/// The characteristics defined here influence how the property is treated by the vector store.
/// </remarks>
public sealed class VectorStoreKeyProperty : VectorStoreProperty
{
    /// <summary>
    /// Initializes a new instance of the <see cref="VectorStoreKeyProperty"/> class.
    /// </summary>
    /// <param name="name">The name of the property on the data model. If the record is mapped to a .NET type, this corresponds to the .NET property name on that type.</param>
    /// <param name="type">The type of the property. Required when using a record type of <c>Dictionary&lt;string, object?&gt;</c> (dynamic mapping), but can omitted when mapping any other .NET type.</param>
    public VectorStoreKeyProperty(string name, Type? type = null)
        : base(name, type)
    {
    }
}
Repository: microsoft/semantic-kernel
File: python/semantic_kernel/contents/streaming_annotation_content.py (Python)

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

import logging
from typing import Any, ClassVar, Literal, TypeVar
from xml.etree.ElementTree import Element  # nosec

from pydantic import Field
from pydantic_settings import SettingsConfigDict

from semantic_kernel.contents.annotation_content import CitationType
from semantic_kernel.contents.const import STREAMING_ANNOTATION_CONTENT_TAG, ContentTypes
from semantic_kernel.contents.kernel_content import KernelContent
from semantic_kernel.utils.feature_stage_decorator import experimental

logger = logging.getLogger(__name__)

_T = TypeVar("_T", bound="StreamingAnnotationContent")


@experimental
class StreamingAnnotationContent(KernelContent):
    """Streaming Annotation content."""

    content_type: Literal[ContentTypes.STREAMING_ANNOTATION_CONTENT] = Field(
        STREAMING_ANNOTATION_CONTENT_TAG,  # type: ignore
        init=False,
    )
    tag: ClassVar[str] = STREAMING_ANNOTATION_CONTENT_TAG
    file_id: str | None = None
    quote: str | None = None
    start_index: int | None = None
    end_index: int | None = None
    url: str | None = None

Review comment (suggestion):
this is for Pydantic Settings, should be:
```suggestion
from pydantic import ConfigDict
```

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

import logging
from typing import Any, ClassVar, Literal, TypeVar
from xml.etree.ElementTree import Element  # nosec

from pydantic import ConfigDict, Field

from semantic_kernel.contents.annotation_content import CitationType
from semantic_kernel.contents.const import STREAMING_ANNOTATION_CONTENT_TAG, ContentTypes
from semantic_kernel.contents.kernel_content import KernelContent
from semantic_kernel.utils.feature_stage_decorator import experimental

logger = logging.getLogger(__name__)

_T = TypeVar("_T", bound="StreamingAnnotationContent")


@experimental
class StreamingAnnotationContent(KernelContent):
    """Streaming Annotation content."""

    content_type: Literal[ContentTypes.STREAMING_ANNOTATION_CONTENT] = Field(
        STREAMING_ANNOTATION_CONTENT_TAG,  # type: ignore
        init=False,
    )
    tag: ClassVar[str] = STREAMING_ANNOTATION_CONTENT_TAG
    file_id: str | None = None
    quote: str | None = None
    start_index: int | None = None
    end_index: int | None = None
    url: str | None = None
    title: str | None = None
Repository: microsoft/semantic-kernel
File: dotnet/src/VectorDataIntegrationTests/WeaviateIntegrationTests/Filter/WeaviateBasicQueryTests.cs (C#)

Code before:
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Not_over_And());

    public override Task Not_over_Or()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Not_over_Or());

    #endregion

    #region Unsupported Contains scenarios

    public override Task Contains_over_captured_string_array()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_captured_string_array());

    public override Task Contains_over_inline_int_array()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_inline_int_array());

    public override Task Contains_over_inline_string_array()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_inline_int_array());

    public override Task Contains_over_inline_string_array_with_weird_chars()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_inline_string_array_with_weird_chars());

    #endregion

    public new class Fixture : BasicQueryTests<Guid>.QueryFixture
    {
        public override TestStore TestStore => WeaviateTestStore.NamedVectorsInstance;
    }
}


Review comment (bug):
It just works as expected on my machine and does not throw `FailException`

Code after:

    public override Task Not_over_Or()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Not_over_Or());

    #endregion

    #region Unsupported Contains scenarios

    public override Task Contains_over_captured_string_array()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_captured_string_array());

    public override Task Contains_over_inline_int_array()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_inline_int_array());

    public override Task Contains_over_inline_string_array()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_inline_int_array());

    public override Task Contains_over_inline_string_array_with_weird_chars()
        => Assert.ThrowsAsync<NotSupportedException>(() => base.Contains_over_inline_string_array_with_weird_chars());

    #endregion

    public new class Fixture : BasicQueryTests<Guid>.QueryFixture
    {
        public override TestStore TestStore => WeaviateTestStore.NamedVectorsInstance;
    }
}
Repository: microsoft/semantic-kernel
File: dotnet/src/Connectors/VectorData.Abstractions/RecordDefinition/DistanceFunction.cs (C#)

Code before:
    /// Measures both the length and angle between two vectors.
    /// </summary>
    /// <remarks>
    /// The value of NegativeDotProduct = -1 * DotProductSimilarity.
    /// The higher the value, the greater the distance between the vectors and the less similar the vectors.
    /// </remarks>
    public const string NegativeDotProductSimilarity = nameof(NegativeDotProductSimilarity);

    /// <summary>
    /// Measures the Euclidean distance between two vectors.
    /// </summary>
    /// <remarks>
    /// Also known as l2-norm.
    /// </remarks>
    public const string EuclideanDistance = nameof(EuclideanDistance);

    /// <summary>
    /// Measures the Euclidean squared distance between two vectors.
    /// </summary>
    /// <remarks>
    /// Also known as l2-squared.
    /// </remarks>
    public const string EuclideanSquaredDistance = nameof(EuclideanSquaredDistance);

    /// <summary>
    /// Specifies the number of differences between vectors at each dimensions.
    /// </summary>
    public const string HammingDistance = nameof(HammingDistance);

    /// <summary>
    /// Measures the Manhattan distance between two vectors.
    /// </summary>
    public const string ManhattanDistance = nameof(ManhattanDistance);
}


Review comment (suggestion):
Measures is probably more accurate here, since this is describing an algorithm for measuring similarity (or dissimilarity).
```suggestion
    /// Measures the number of differences between vectors at each dimension.
```

Code after:
    /// Specifies the negative dot product similarity function, which measures both the length and angle between two vectors.
    /// </summary>
    /// <remarks>
    /// The value of NegativeDotProduct = -1 * DotProductSimilarity.
    /// The higher the value, the greater the distance between the vectors and the less similar the vectors.
    /// </remarks>
    public const string NegativeDotProductSimilarity = nameof(NegativeDotProductSimilarity);

    /// <summary>
    /// Specifies the function that measures the Euclidean distance between two vectors.
    /// </summary>
    /// <remarks>
    /// Also known as l2-norm.
    /// </remarks>
    public const string EuclideanDistance = nameof(EuclideanDistance);

    /// <summary>
    /// Specifies the function that measures the Euclidean squared distance between two vectors.
    /// </summary>
    /// <remarks>
    /// Also known as l2-squared.
    /// </remarks>
    public const string EuclideanSquaredDistance = nameof(EuclideanSquaredDistance);

    /// <summary>
    /// Specifies the function that measures the number of differences between vectors at each dimension.
    /// </summary>
    public const string HammingDistance = nameof(HammingDistance);

    /// <summary>
    /// Specifies the function that measures the Manhattan distance between two vectors.
    /// </summary>
    public const string ManhattanDistance = nameof(ManhattanDistance);
}