[Bugfix] Validation fixes

[sakari-malkki]

Fixes the issue in data validation with multi-byte px files where the start of DATA section was wrongly assigned.
Provides optional validation options to limit the amount of duplicate feedback items.
Bunch of missing xml documentation added.

[Copilot]

🟡 Changes recommended

There are confirmed documentation correctness issues (invalid nested XML <param> tags) and an API-breaking removal/rename in StreamUtilities that needs an explicit compatibility/versioning decision before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR fixes incorrect detection of the DATA section start offset for PX files containing UTF-8 BOM/multibyte metadata by switching to a byte-accurate stream scan for the first data value after the top-level DATA= entry. It also introduces configurable validation feedback retention (per filename + level + rule) via ValidationOptions and a concurrent ValidationFeedbackSink, and updates validators, benchmarks, docs, and tests accordingly.

Changes:

• Add StreamUtilities.FindDataStartPosition* to compute an absolute raw byte offset for the first data value after DATA= (BOM/multibyte-safe), and propagate long offsets through validation/data-reader APIs.
• Add feedback retention options (ValidationOptions) and a thread-safe sink (ValidationFeedbackSink) and thread them through Syntax/Content/Data/PxFile/Database validators.
• Expand unit tests and documentation to cover the new data-start logic and feedback limiting behavior.

File summaries

File	Description
Px.Utils/Validation/ValidationOptions.cs	Adds public options object for feedback retention limits (default 100, unlimited supported).
Px.Utils/Validation/ValidationFeedbackSink.cs	Introduces concurrent sink enforcing per-signature retention with truncation annotation.
Px.Utils/Validation/SyntaxValidation/ValidationStructuredEntry.cs	Minor XML doc update; removes BOM from file header.
Px.Utils/Validation/SyntaxValidation/SyntaxValidator.cs	Computes data-start byte offset via StreamUtilities; adds overloads accepting ValidationOptions/sink.
Px.Utils/Validation/SyntaxValidation/SyntaxValidationUtilityMethods.cs	Adds missing XML param doc entry.
Px.Utils/Validation/SyntaxValidation/SyntaxValidationResult.cs	Changes DataStartStreamPosition to long and clarifies semantics (absolute raw byte offset).
Px.Utils/Validation/PxFileValidator.cs	Threads a shared sink through syntax/content/data/custom validators; adds overloads with ValidationOptions.
Px.Utils/Validation/DataValidation/DataValidatorFunctions.cs	Adds missing XML param docs for filename.
Px.Utils/Validation/DataValidation/DataValidator.cs	Uses StreamUtilities for origin-based data start; adds overloads with ValidationOptions/sink and long offsets.
Px.Utils/Validation/DatabaseValidation/LocalFileSystem.cs	Moves coverage-exclusion attribute below XML docs and adds <inheritdoc/> tags.
Px.Utils/Validation/DatabaseValidation/IFileSystem.cs	Adds full XML documentation for interface members.
Px.Utils/Validation/DatabaseValidation/DatabaseValidator.cs	Adds overloads with ValidationOptions and uses shared sink across concurrent processing.
Px.Utils/Validation/ContentValidation/ContentValidator.ValidationEntryFunctions.cs	Fixes XML param name (entry vs entries).
Px.Utils/Validation/ContentValidation/ContentValidator.UtilityMethods.cs	Adds missing XML param doc entry.
Px.Utils/Validation/ContentValidation/ContentValidator.cs	Adds overloads with ValidationOptions and sink-based reporting.
Px.Utils/PxFile/Metadata/IPxFileMetadataReader.cs	Adds missing cancellation token XML docs; removes BOM from file header.
Px.Utils/PxFile/Data/StreamUtilities.cs	Replaces keyword-position finder with DATA-value start finder returning absolute raw byte offsets.
Px.Utils/PxFile/Data/PxFileStreamDataReader.cs	Uses FindDataStartPosition* and updates docs/exception messages accordingly.
Px.Utils/Px.Utils.csproj	Bumps package version to 1.6.0.
Px.Utils/Operations/SumMatrixFunctionExtensions.cs	Adds XML docs for insert-index parameters (currently has invalid nested <param> tags).
Px.Utils/Operations/MultiplicationMatrixFunction.cs	Adds XML docs for insert-index parameters (one overload currently has invalid nested <param> tags).
Px.Utils/Operations/DivisionMatrixFunctionExtensions.cs	Fixes malformed generic XML cref tags.
Px.Utils/ModelBuilders/MatrixMetadataBuilder.cs	Fixes XML cref for IReadOnlyDictionary<TKey,TValue>.
Px.Utils.UnitTests/Validation/ValidationFeedbackSinkTests.cs	Adds unit tests for retention limits, truncation annotation, and invalid limits.
Px.Utils.UnitTests/Validation/SyntaxValidationTests/StreamSyntaxValidationTests.cs	Adds coverage for BOM/multibyte data-start offset and limited feedback retention.
Px.Utils.UnitTests/Validation/DataValidationTests/DataValidationTest.cs	Updates expectations + adds tests for custom DATA keyword and retention limit behavior.
Px.Utils.UnitTests/Validation/ContentValidationTests/ContentValidationTests.cs	Adds test for retention limiting in content validation.
Px.Utils.UnitTests/PxFileTests/DataTests/StreamUtilitiesTests.cs	Adds comprehensive tests for byte-accurate DATA start offsets (BOM, multibyte, whitespace, buffer splits, empty DATA).
Px.Utils.TestingApp/Commands/PxFileValidationBenchmark.cs	Adds CLI help and passes ValidationOptions into validation.
Px.Utils.TestingApp/Commands/MetadataSyntaxValidationBenchmark.cs	Adds CLI help and passes ValidationOptions into validation.
Px.Utils.TestingApp/Commands/MetadataContentValidationBenchmark.cs	Passes ValidationOptions into content validation.
Px.Utils.TestingApp/Commands/DataValidationBenchmark.cs	Uses FindDataStartPosition and passes ValidationOptions into data validation.
Px.Utils.TestingApp/Commands/DatabaseValidationBenchmark.cs	Adds CLI help and passes ValidationOptions into database validation.
Px.Utils.TestingApp/Commands/Benchmark.cs	Adds -l/-limit flag parsing and stores ValidationOptions for benchmark commands.
docs/README.md	Documents data-start behavior and new feedback retention feature + examples.
docs/architecture.validation.md	Documents sink-based retention policy and data-start offset semantics.
docs/architecture.testing.md	Registers new tests for sink and stream utilities.
docs/architecture.readers.md	Documents reader behavior at stream position 0 and new helpers.

Review details

Suppressed comments (2)

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:41

• The XML documentation is currently invalid because a tag (insetIndex) is nested inside the tag. Each parameter needs its own non-nested element.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:58

• The XML documentation is currently invalid because a tag (insetIndex) is nested inside the tag for this overload as well.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

• Files reviewed: 38/38 changed files
• Comments generated: 5
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

Several validation entry points infer encoding via IFileSystem.GetEncoding/GetEncodingAsync without restoring stream position, which can cause validation to start from the wrong offset, and there are also confirmed documentation/build issues (malformed XML docs, empty-data edge case, and benchmark arg handling).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (9)

Px.Utils/Validation/PxFileValidator.cs:189

• Same stream-position issue as the sync path: GetEncodingAsync advances the stream, and the subsequent SyntaxValidator.ValidateAsync reads from the wrong offset unless the position is restored.

            encoding ??= await new LocalFileSystem().GetEncodingAsync(stream, cancellationToken);
            conf ??= PxFileConfiguration.Default;

Px.Utils/Validation/SyntaxValidation/SyntaxValidator.cs:203

• Async path has the same issue as the sync path: GetEncodingAsync may advance the stream, causing BuildValidationEntriesAsync to miss earlier metadata unless the original position is restored.
  Px.Utils/Validation/DataValidation/DataValidator.cs:160
• Async validation has the same stream-position issue: GetEncodingAsync can advance the stream, which can cause DATA start detection to run from the wrong offset unless the position is restored.

            fileSystem ??= new LocalFileSystem();
            encoding ??= await fileSystem.GetEncodingAsync(stream, cancellationToken);
            SetValidationParameters(encoding, filename);

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:58

• This overload has the same malformed XML documentation issue as the previous one: the sumMap tag isn't closed before insetIndex is documented.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/Validation/PxFileValidator.cs:96

• When encoding is inferred via LocalFileSystem.GetEncoding(stream), the method advances the stream position; SyntaxValidator then reads from the advanced position, so metadata validation can start mid-file. Preserve the original stream position after encoding detection before continuing.

This issue also appears on line 188 of the same file.

            encoding ??= new LocalFileSystem().GetEncoding(stream);
            conf ??= PxFileConfiguration.Default;

Px.Utils/Validation/SyntaxValidation/SyntaxValidator.cs:98

• If encoding is detected via IFileSystem.GetEncoding(stream), that call can advance the stream; BuildValidationEntries then starts parsing from the wrong position. Capture and restore the original stream position after encoding detection.

This issue also appears on line 201 of the same file.
Px.Utils/Validation/DataValidation/DataValidator.cs:469

• When the stream is not at position 0, GetStreamIndexOfFirstDataValue treats any non-whitespace byte as a valid data start. This incorrectly accepts an explicitly empty entry (e.g., DATA= ;) by returning the entry separator position instead of -1.

                if (currentByte is not CharacterConstants.SPACE and not CharacterConstants.HORIZONTALTAB and not CharacterConstants.CARRIAGERETURN and not CharacterConstants.LINEFEED)
                {
                    return stream.Position - 1;
                }

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:41

• The XML documentation for this overload has a malformed block: the sumMap tag isn't closed before the insetIndex starts, which breaks doc generation.

This issue also appears on line 56 of the same file.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils.TestingApp/Commands/Benchmark.cs:160

• Invalid -l/-limit usage (e.g., missing value or multiple values) is currently silently ignored because only the Count==1 case is handled. This makes the benchmark run with defaults instead of failing fast on bad input.

                    else if (feedbackLimitFlags.Contains(key) && Parameters[key].Count == 1)
                    {
                        ValidationOptions = ParseValidationOptions(Parameters[key][0]);
                    }

• Files reviewed: 38/38 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

There are confirmed issues in the updated public API docs/signatures (malformed XML docs and inconsistent parameter naming) and in benchmark argument validation behavior that should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (4)

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:60

• Same issue in this overload: the <param name="sumMap"> tag contains another <param> tag and its closing </param> is at the wrong place, producing malformed XML docs. The parameter name insetIndex is also inconsistent with the synchronous overload’s insertIndex.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>
        /// <returns>A new <see cref="Matrix{TData}"/> object that contais the results of the additions.</returns>
        public async static Task<Matrix<TData>> SumToNewValueAsync<TData>(this Task<Matrix<TData>> input, DimensionValue newValue, IDimensionMap sumMap, int insetIndex = -1)

Px.Utils.TestingApp/Commands/Benchmark.cs:160

• In batch mode, invalid values for known flags (e.g. -i foo or -limit 1 2) are silently ignored because the parser only handles the happy-path cases; this makes benchmark runs succeed with unintended defaults instead of failing fast.

            if (Parameters.Keys.All(key => Array.Exists(ParameterFlags.ToArray(), flag => flag.Contains(key))))
            {
                foreach (string key in Parameters.Keys)
                {
                    if (iterFlags.Contains(key) && Parameters[key].Count == 1 &&
                       int.TryParse(Parameters[key][0], out int iterations))
                    {
                        Iterations = iterations;
                    }
                    else if (feedbackLimitFlags.Contains(key) && Parameters[key].Count == 1)
                    {
                        ValidationOptions = ParseValidationOptions(Parameters[key][0]);
                    }

Px.Utils/Validation/SyntaxValidation/SyntaxValidationUtilityMethods.cs:52

• The parameter name stringDelimeter is misspelled (should be stringDelimiter). Since this is part of the method signature, the typo leaks into call sites and XML docs, and makes the API harder to read/search.

        /// <param name="stringDelimeter">Symbol that encloses a string</param>
        /// <param name="endSymbol">Optional symbol that closes the enclosement. If none given, startSymbol is used for both starting and ending the enclosement</param>
        /// <return>Returns an <see cref="ExtractSectionResult"/> object that contains the extracted sections,
        /// the string that remains after the operation and starting indexes of extracted sections</return>
        internal static ExtractSectionResult ExtractSectionFromString(string input, char startSymbol, char stringDelimeter, char? endSymbol = null)
        {

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:43

• The XML docs for this overload are not well-formed: the <param name="sumMap"> tag never closes before starting the <param name="insetIndex"> tag. Also, the public parameter name insetIndex looks like a typo compared to the synchronous insertIndex overload, which impacts named-argument callers and generated docs.

This issue also appears on line 56 of the same file.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>
        /// <returns>A new <see cref="Matrix{TData}"/> object that contais the results of the additions.</returns>
        public async static Task<Matrix<TData>> SumToNewValueAsync<TData>(this Matrix<TData> input, DimensionValue newValue, IDimensionMap sumMap, int insetIndex = -1)

• Files reviewed: 38/38 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

There are confirmed correctness issues around BOM-at-stream-origin keyword/DATA detection and a few behavioral inconsistencies (e.g., PxFileValidator ignoring the provided fileSystem for encoding detection) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (8)

Px.Utils/PxFile/Data/StreamUtilities.cs:267

• TryFindDataStartPosition has the same BOM-at-origin issue as keyword lookup: if the file begins with a BOM and the first top-level entry is DATA, the BOM bytes prevent the DATA keyword from being recognized. Skipping common UTF BOMs on the first buffer avoids returning -1 incorrectly for valid inputs.

        private static bool TryFindDataStartPosition(
            ReadOnlySpan<byte> buffer, 
            long bufferStart, 
            ref DataStartSearchState state, 
            out long dataStartPosition)

Px.Utils/PxFile/Data/StreamUtilities.cs:327

• TryFindDataStartPositionUnchecked can also fail for BOM-prefixed files when DATA is the first entry, because the BOM bytes are processed as non-whitespace at entry start and prevent top-level keyword matching. Skipping BOM bytes in the first buffer keeps the unchecked locator consistent with the public FindDataStartPosition behavior.

        private static bool TryFindDataStartPositionUnchecked(ReadOnlySpan<byte> buffer, long bufferStart, ref UncheckedDataStartSearchState state, out long dataStartPosition)
        {
            for (int i = 0; i < buffer.Length; i++)
            {
                byte currentByte = buffer[i];

Px.Utils/Validation/PxFileValidator.cs:198

• PxFileValidator.ValidateAsync also ignores the provided fileSystem for encoding detection and always uses LocalFileSystem, which is inconsistent with the method parameter/summary and with other validators.

            if (encoding is null)
            {
                long originalPosition = stream.Position;
                encoding = await new LocalFileSystem().GetEncodingAsync(stream, cancellationToken);
                stream.Position = originalPosition;

Px.Utils/Validation/PxFileValidator.cs:213

• ValidateAsync() closes the provided stream on the success path, but returns early without closing it when the DATA start cannot be found, leading to inconsistent resource lifetime and potential handle leaks in async validation failures.

            if (syntaxValidationResult.DataStartStreamPosition == -1)
            {
                sink.Report(new KeyValuePair<ValidationFeedbackKey, ValidationFeedbackValue>(
                    new(ValidationFeedbackLevel.Error,
                        ValidationFeedbackRule.StartOfDataSectionNotFound),

Px.Utils/PxFile/Data/StreamUtilities.cs:199

• FindKeywordPosition can fail when the stream starts with a UTF BOM and the keyword is the first top-level entry (e.g., BOM + "DATA=") because the BOM bytes cause the state machine to leave "entry start" before it sees the keyword. Skipping common UTF BOMs at bufferStart==0 allows correct matching at stream origin.

This issue also appears in the following locations of the same file:

• line 263
• line 323

        private static long FindKeywordPosition(ReadOnlySpan<byte> buffer, long bufferStart, ref TopLevelKeywordSearchState state)
        {
            for (int i = 0; i < buffer.Length; i++)
            {
                if (TryProcessTopLevelKeywordByte(buffer[i], ref state))

Px.Utils/Validation/PxFileValidator.cs:100

• PxFileValidator ignores the provided fileSystem for encoding detection and always uses LocalFileSystem. This breaks the documented contract that fileSystem is used for encoding detection (and makes behavior differ from SyntaxValidator/DataValidator).

This issue also appears on line 194 of the same file.

            if (encoding is null)
            {
                long originalPosition = stream.Position;
                encoding = new LocalFileSystem().GetEncoding(stream);
                stream.Position = originalPosition;

Px.Utils/Validation/PxFileValidator.cs:114

• Validate() closes the provided stream on the success path, but returns early without closing it when the DATA start cannot be found. This makes resource ownership inconsistent and can leak file handles for failure cases.

This issue also appears on line 209 of the same file.

            if (syntaxValidationResult.DataStartStreamPosition == -1)
            {
                sink.Report(new KeyValuePair<ValidationFeedbackKey, ValidationFeedbackValue>(
                    new(ValidationFeedbackLevel.Error,
                        ValidationFeedbackRule.StartOfDataSectionNotFound),

Px.Utils.TestingApp/Commands/Benchmark.cs:156

• In batch mode, invalid -iter/-limit values (wrong arity or non-integer for -iter) are silently ignored because the foreach loop just falls through. This makes CI/automation runs succeed with unintended defaults instead of failing fast on invalid CLI input.

                    if (iterFlags.Contains(key) && Parameters[key].Count == 1 &&
                       int.TryParse(Parameters[key][0], out int iterations))
                    {
                        Iterations = iterations;
                    }

• Files reviewed: 39/39 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

It introduces a confirmed BOM-handling bug in FindDataStartPositionUnchecked* and includes malformed XML documentation in SumMatrixFunctionExtensions, both of which should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (7)

Px.Utils.TestingApp/Commands/Benchmark.cs:152

• SetRunParameters silently ignores invalid values/counts for supported flags when the key is recognized (e.g., "-i" with a non-integer or multiple values). In batch mode this makes argument errors hard to detect and can lead to benchmarks running with default parameters unexpectedly.

            if (Parameters.Keys.All(key => Array.Exists(ParameterFlags.ToArray(), flag => flag.Contains(key))))
            {
                foreach (string key in Parameters.Keys)
                {
                    if (iterFlags.Contains(key) && Parameters[key].Count == 1 &&

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:41

• XML documentation is malformed here: the tag is never closed before starting the next , which will break doc generation and IntelliSense (the insetIndex is nested inside sumMap).

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:58

• Same malformed XML documentation issue as above: the tag is not closed before starting , so generated docs/IntelliSense will be incorrect.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/PxFile/Data/StreamUtilities.cs:139

• FindDataStartPositionUncheckedImpl does not skip a UTF-8 BOM at stream origin. If a validated PX stream starts with BOM + "DATA=..." (DATA as the first entry), the initial BOM bytes cause the top-level keyword matcher to leave "entry start" mode and the DATA keyword will never be matched (no prior entry separator to reset), so the method incorrectly returns -1.

        private static long FindDataStartPositionUncheckedImpl(Stream stream, PxFileConfiguration conf, int bufferSize)
        {
            byte[] dataKeywordBytes = Encoding.ASCII.GetBytes(conf.Tokens.KeyWords.Data + conf.Symbols.KeywordSeparator);
            byte[] buffer = new byte[bufferSize];
            UncheckedDataStartSearchState state = new(dataKeywordBytes, (byte)conf.Symbols.EntrySeparator, (byte)conf.Symbols.Key.StringDelimeter);

Px.Utils/PxFile/Data/StreamUtilities.cs:158

• FindDataStartPositionUncheckedImplAsync has the same BOM-at-origin issue as the synchronous unchecked variant: for BOM + "DATA=..." at the start of the stream, the keyword matcher will never recognize DATA and the method returns -1.

        private static async Task<long> FindDataStartPositionUncheckedImplAsync(Stream stream, PxFileConfiguration conf, int bufferSize, CancellationToken cancellationToken)
        {
            byte[] dataKeywordBytes = Encoding.ASCII.GetBytes(conf.Tokens.KeyWords.Data + conf.Symbols.KeywordSeparator);
            byte[] buffer = new byte[bufferSize];
            UncheckedDataStartSearchState state = new(dataKeywordBytes, (byte)conf.Symbols.EntrySeparator, (byte)conf.Symbols.Key.StringDelimeter);

Px.Utils.TestingApp/Commands/DataValidationBenchmark.cs:20

• Spelling: "colums" should be "columns" in the help text.

        "\t-c, -cols: How many data colums the validator should expect." + Environment.NewLine +

Px.Utils/Validation/DatabaseValidation/DatabaseValidator.cs:62

• ProcessPxFile/ProcessPxFileAsync now pass a shared ValidationFeedbackSink into PxFileValidator, so the returned ValidationFeedback is typically empty for valid files; the caller then still calls sink.ReportRange(feedback). This adds avoidable allocations/overhead and makes the flow harder to follow.

                fileTasks.Add(Task.Run(() =>
                {
                    (DatabaseFileInfo? file, ValidationFeedback feedback) = ProcessPxFile(fileName, sink);
                    if (file != null) pxFiles.Add(file);
                    sink.ReportRange(feedback);

• Files reviewed: 39/39 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.

Suppressed comments (7)

Px.Utils/PxFile/Data/StreamUtilities.cs:158

• FindDataStartPositionUncheckedImplAsync has the same BOM handling issue as the sync variant: when called at stream position 0, a UTF-8 BOM prevents DATA from being matched if it is the first top-level entry (no entry separator occurs before it to reset state).

Skip the UTF-8 BOM at stream origin before scanning.

        private static async Task<long> FindDataStartPositionUncheckedImplAsync(Stream stream, PxFileConfiguration conf, int bufferSize, CancellationToken cancellationToken)
        {
            byte[] dataKeywordBytes = Encoding.ASCII.GetBytes(conf.Tokens.KeyWords.Data + conf.Symbols.KeywordSeparator);
            byte[] buffer = new byte[bufferSize];
            UncheckedDataStartSearchState state = new(dataKeywordBytes, (byte)conf.Symbols.EntrySeparator, (byte)conf.Symbols.Key.StringDelimeter);

Px.Utils.TestingApp/Commands/Benchmark.cs:156

• SetRunParameters only updates Iterations when parsing succeeds; if -i/-iter is provided with a non-integer value (or multiple values), it is silently ignored and the benchmark runs with the default iteration count. In batch mode this should be treated as an invalid invocation and fail fast (same applies to -l/-limit).

                    if (iterFlags.Contains(key) && Parameters[key].Count == 1 &&
                       int.TryParse(Parameters[key][0], out int iterations))
                    {
                        Iterations = iterations;
                    }

Px.Utils/Validation/DatabaseValidation/DatabaseValidator.cs:132

• ProcessPxFile allocates and returns a ValidationFeedback feedbacks = [], but for the success path (fileInfo != null) nothing is ever added to it because validation now reports directly into the provided ValidationFeedbackSink. This makes the returned feedback always empty and adds unnecessary allocation / confusion at the call site.

Return an empty feedback collection directly for the success path, and only allocate/populate feedback when GetPxFileInfo returns a standalone feedback item.

This issue also appears on line 150 of the same file.

        private (DatabaseFileInfo?, ValidationFeedback) ProcessPxFile(string fileName, ValidationFeedbackSink sink)
        {
            ValidationFeedback feedbacks = [];
            using Stream stream = _fileSystem.GetFileStream(fileName);
            (DatabaseFileInfo? fileInfo, KeyValuePair<ValidationFeedbackKey, ValidationFeedbackValue>? feedback) = GetPxFileInfo(fileName, stream);

Px.Utils/Validation/DatabaseValidation/DatabaseValidator.cs:154

• ProcessPxFileAsync has the same issue as the sync variant: ValidationFeedback feedbacks = [] is always empty on the success path because validation writes directly to ValidationFeedbackSink. This adds unnecessary allocation and makes the return value misleading.

        private async Task<(DatabaseFileInfo?, ValidationFeedback)> ProcessPxFileAsync(string fileName, ValidationFeedbackSink sink, CancellationToken cancellationToken)
        {
            ValidationFeedback feedbacks = [];
            using Stream stream = _fileSystem.GetFileStream(fileName);
            (DatabaseFileInfo? fileInfo, KeyValuePair<ValidationFeedbackKey, ValidationFeedbackValue>? feedback) = await GetPxFileInfoAsync(fileName, stream, cancellationToken);

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:60

• The second SumToNewValueAsync overload has the same malformed XML documentation issue: <param name="sumMap"> isn't closed before <param name="insetIndex">, which results in invalid/nested XML doc tags.

        /// <param name="newValue">This value will be added to the dimension defined by the <paramref name="sumMap"/></param>
        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>
        /// <returns>A new <see cref="Matrix{TData}"/> object that contais the results of the additions.</returns>
        public async static Task<Matrix<TData>> SumToNewValueAsync<TData>(this Task<Matrix<TData>> input, DimensionValue newValue, IDimensionMap sumMap, int insetIndex = -1)

Px.Utils/PxFile/Data/StreamUtilities.cs:138

• FindDataStartPositionUncheckedImpl does not skip a UTF-8 BOM when the stream is at origin. If a BOM is present and the first top-level entry is DATA=..., the initial BOM bytes will flip the search state away from IsAtEntryStart and the DATA keyword will never be detected (no entry separator occurs before it). This makes PxFileStreamDataReader's automatic positioning fail for BOM-prefixed files whose first entry is DATA.

Add BOM skipping at the start of the unchecked search implementations (sync + async) to make behavior consistent with the public helpers and with the docs claiming BOM support.

This issue also appears on line 153 of the same file.

        private static long FindDataStartPositionUncheckedImpl(Stream stream, PxFileConfiguration conf, int bufferSize)
        {
            byte[] dataKeywordBytes = Encoding.ASCII.GetBytes(conf.Tokens.KeyWords.Data + conf.Symbols.KeywordSeparator);
            byte[] buffer = new byte[bufferSize];
            UncheckedDataStartSearchState state = new(dataKeywordBytes, (byte)conf.Symbols.EntrySeparator, (byte)conf.Symbols.Key.StringDelimeter);

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:43

• The XML documentation for SumToNewValueAsync is malformed: the <param name="sumMap"> tag is never closed before starting <param name="insetIndex">, so the insetIndex param tag becomes nested inside the sumMap param tag. This breaks XML doc generation and IntelliSense rendering.

Close the sumMap param tag before declaring insetIndex (and keep the descriptive text under the correct param).

This issue also appears on line 55 of the same file.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>
        /// <returns>A new <see cref="Matrix{TData}"/> object that contais the results of the additions.</returns>
        public async static Task<Matrix<TData>> SumToNewValueAsync<TData>(this Matrix<TData> input, DimensionValue newValue, IDimensionMap sumMap, int insetIndex = -1)

[Copilot]

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:58

• This XML documentation block has the same issue as above: <param name="insetIndex"> is nested inside the <param name="sumMap"> tag and the tags are not properly closed, which can trigger XML doc warnings/errors.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:41

• The XML documentation is malformed here: a tag is nested inside the tag and never closed. This typically triggers XML doc warnings (and can fail builds when warnings are treated as errors).

This issue also appears on line 56 of the same file.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils.TestingApp/Commands/DataValidationBenchmark.cs:21

• Spelling: "colums" should be "columns" in the help text.

        "Validates the px file data." + Environment.NewLine +
        "\t-r, -rows: How many data rows the validator should expect." + Environment.NewLine +
        "\t-c, -cols: How many data colums the validator should expect." + Environment.NewLine +
        "\t-l, -limit: Feedback items retained per file, level, and rule; use a positive number. Defaults to 100.";

Px.Utils.TestingApp/Commands/Benchmark.cs:149

• Parameters.Keys.All(...) evaluates to true when Parameters is empty, so calling Run() with no inputs will skip the interactive prompt and silently keep the default Iterations/ValidationOptions. If interactive mode is intended when no parameters are supplied, guard the condition with Parameters.Count > 0.

        protected virtual void SetRunParameters()
        {
            ValidationOptions = new();
            if (Parameters.Keys.All(key => Array.Exists(ParameterFlags.ToArray(), flag => flag.Contains(key))))
            {

[Copilot]

Pull request overview

Copilot reviewed 42 out of 42 changed files in this pull request and generated no new comments.

Suppressed comments (6)

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:41

• The new insetIndex <param> tag is nested inside the still-open sumMap tag, producing malformed XML documentation and a compiler documentation warning. Close the sumMap description before starting the next parameter.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated.
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/Operations/SumMatrixFunctionExtensions.cs:58

• The added insetIndex tag is nested before the sumMap tag is closed, so this XML documentation is malformed. Finish the sumMap text first, then add the separate parameter tag.

        /// <param name="sumMap">Defines the <see cref="Dimension"/> relative to which the sums are calculated. 
        /// <param name="insetIndex">The index at which to insert the new value. Defaults to -1, which appends the value to the end.</param>
        /// Also defines which <see cref="DimensionValue"/>s are included in the sum.</param>

Px.Utils/PxFile/Data/StreamUtilities.cs:136

• The unchecked synchronous locator does not skip a UTF-8 BOM. For a valid stream beginning directly with BOM + DATA=..., the first BOM byte marks the state as no longer at an entry start, so PxFileStreamDataReader returns “Could not find…” despite the documented BOM support. Skip the BOM when this helper starts at position 0.

            byte[] dataKeywordBytes = Encoding.ASCII.GetBytes(conf.Tokens.KeyWords.Data + conf.Symbols.KeywordSeparator);

Px.Utils/Validation/DataValidation/DataValidator.cs:426

• Position 0 is also a valid position for a data-only stream whose first item starts at byte 0, which the public Validate/ValidateAsync contract says is supported. Treating zero as a sentinel for a full PX file makes such input (for example 1 2;) report StartOfDataSectionNotFound. Use an explicit mode/API for full-file discovery rather than overloading the byte position.

            if (stream.Position == 0)
            {
                return StreamUtilities.FindDataStartPosition(stream, _conf, _streamBufferSize);

Px.Utils/Validation/PxFileValidator.cs:236

• This closes the caller-owned stream only in the asynchronous missing-DATA path, while the synchronous path returns without closing it. It also changes the previous async behavior for this failure case. Remove the close so the two overloads remain consistent.

                stream.Close();
                return;

Px.Utils/PxFile/Data/StreamUtilities.cs:155

• The asynchronous unchecked locator has the same BOM-at-origin failure as the synchronous path: a leading UTF-8 BOM prevents DATA= from being recognized when it is the first entry. Skip the BOM before initializing the search state.

            byte[] dataKeywordBytes = Encoding.ASCII.GetBytes(conf.Tokens.KeyWords.Data + conf.Symbols.KeywordSeparator);