Address open review feedback from PR #8263 (TRX streaming store)

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Follow-up to #8263 addressing the open review threads on the merged PR.

Changes

• TrxTestResultExtractor doc comment — the serializer per-record cap is 64 MiB (TrxTestResultSerializer.MaxRecordLengthBytes), not 16 MiB as the comment stated. Updated so the rationale for MaxCapturedFieldChars cites the correct number. (Copilot review threads x2)
• TrxResultStreamingStore.WriteRecordWithRetryAsync nullable consistency — the two bare ExceptionDispatchInfo.Capture(lastError).Throw() rethrows now use the null-forgiving operator, matching the final Capture(lastError!).Throw() at the end of the method. (Copilot review thread)
• TRX streaming-store writer no longer blocks a threadpool thread — switched from ITask.Run to ITask.RunLongRunning so the BlockingCollection<T>.TryTake poll loop (which blocks for up to _flushIntervalMs while idle) runs on its own dedicated background thread instead of starving the threadpool. (Youssef Fahmy (@Youssef1313) review thread)

cc Youssef Fahmy (@Youssef1313)

[Copilot]

Pull request overview

Follow-up to PR #8263 that tightens documentation/nullable consistency in the TRX streaming store, and attempts to move the streaming-writer loop off the shared threadpool to reduce contention during idle waits.

Changes:

• Corrected TrxTestResultExtractor comment to reflect the serializer’s 64 MiB per-record cap.
• Made ExceptionDispatchInfo.Capture(lastError) rethrows nullable-consistent via lastError!.
• Switched the streaming-store writer startup from ITask.Run to ITask.RunLongRunning.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxTestResultExtractor.cs	Updates rationale comment to reference the correct serializer record-size cap (64 MiB).
src/Platform/Microsoft.Testing.Extensions.TrxReport/Streaming/TrxResultStreamingStore.cs	Starts the writer via RunLongRunning and aligns nullable usage in retry rethrow paths.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

✅ 21/21 dimensions clean — no findings.

All three changes are correct:

• RunLongRunning: The API exists on ITask/SystemTask and correctly spawns a dedicated background thread, keeping the blocking BlockingCollection.TryTake off the shared threadpool. Rationale in the comment is accurate.
• lastError!: Both null-forgiving operators are safe. The catch sites are only reachable after at least one loop iteration that unconditionally assigns lastError; null is impossible there.
• Comment 16 MiB → 64 MiB: Matches the actual MaxRecordLengthBytes = 64 * 1024 * 1024 constant in TrxTestResultSerializer.cs; the MaxCapturedFieldChars = 1 MiB truncation cap is still well below 64 MiB.

Generated by Expert Code Review (on open) for issue #8285 · ● 8.6M

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 6fbd731. The TRX streaming writer loop is now await-free/synchronous while running on the long-running background thread, so the BlockingCollection.TryTake polling and retry/flush/log waits stay off the shared threadpool. Targeted build and TRX report tests pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Amaury Levé (Evangelink)]

Copilot address review comments

[Copilot]

Copilot address review comments

Addressed in 0222b80. The TRX streaming store now uses best-effort synchronous error logging instead of blocking on LogErrorAsync, and the write-failure message now says "after up to" the retry count. Targeted build/tests and final validation pass.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1