[thomhurst]

Carries the discovery List directly into execution, directly builds one metadata item, and skips empty dependency HashSets. 31 paired runs: TUnit session median 114.994 ms to 113.208 ms (-1.786 ms, -1.55%); paired delta -1.469 ms. Validation: deferred expansion 11 passed; dependency path 2; filtered discovery 1; reflection 1.

[coderabbitai]

Warning

Review limit reached

@thomhurst, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Limit details: You’ve used all 3 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 02ac7a48-9c79-45db-8aea-bd4119597873

📥 Commits

Reviewing files that changed from the base of the PR and between bc20d78 and d574f6e.

📒 Files selected for processing (6)

• src/TUnit.Engine/Building/TestBuilder.cs
• src/TUnit.Engine/Building/TestBuilderPipeline.cs
• src/TUnit.Engine/Framework/TestRequestHandler.cs
• src/TUnit.Engine/Interfaces/ITestExecutor.cs
• src/TUnit.Engine/TestDiscoveryService.cs
• src/TUnit.Engine/TestSessionCoordinator.cs

---

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR removes redundant collection copies along the discovery-to-execution path and adds fast paths for single-test discovery and tests without dependencies.

• Passes the concrete discovered-test list directly into execution.
• Builds zero or one metadata item without invoking the parallel mapping and flattening machinery.
• Avoids dependency traversal allocations when a test has no resolved dependencies.

Confidence Score: 5/5

The PR appears safe to merge with no concrete correctness or security regressions identified.

The optimized collection paths preserve current caller contracts, dependency population ordering, deferred expansion behavior, lifecycle registration, and reporting visibility.

Important Files Changed

Filename	Overview
src/TUnit.Engine/Building/TestBuilderPipeline.cs	Adds zero- and single-metadata fast paths while preserving per-metadata failed-test synthesis and caller-compatible result semantics.
src/TUnit.Engine/TestDiscoveryService.cs	Materializes the final filtered collection once as a List and exposes that concrete ownership through TestDiscoveryResult.
src/TUnit.Engine/Framework/TestRequestHandler.cs	Removes the execution-time array copy and passes the discovery result list directly to the coordinator.
src/TUnit.Engine/TestSessionCoordinator.cs	Executes against the supplied list directly; deferred expansion mutates it before all lifecycle and scheduling consumers, with no retained external observer.
src/TUnit.Engine/Building/TestBuilder.cs	Skips allocating traversal sets when the fully resolved test has no dependencies.
src/TUnit.Engine/Interfaces/ITestExecutor.cs	Narrows the internal execution contract from IEnumerable to List to express the mutable collection required by deferred expansion.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Test metadata] --> B[TestBuilderPipeline]
  B -->|zero items| C[Empty result]
  B -->|one item| D[Build directly]
  B -->|multiple items| E[Parallel build and flatten]
  C --> F[TestDiscoveryService]
  D --> F
  E --> F
  F --> G[Filter and register]
  G --> H[Pass concrete List directly]
  H --> I[Expand deferred placeholders]
  I --> J[Prepare lifecycle and schedule]

Loading

_{Reviews (1): Last reviewed commit: "perf(engine): remove single-test copies" | Re-trigger Greptile}

[claude]

Code review

Reviewed the diff (6 files, +28/-10) plus the surrounding call sites in TestDiscoveryService, TestRequestHandler, TestSessionCoordinator, and TestBuilderPipeline. No issues found — this is a clean, well-scoped set of allocation-reduction changes:

• TestBuilder.PopulateDependencies: early-return when test.Dependencies.Length == 0 is behaviorally equivalent to the prior path (the HashSet traversal would have produced an empty result anyway), just skips two allocations.
• TestBuilderPipeline.BuildTestsFromMetadataAsync: the 0/1-item fast paths bypass ParallelMap.SelectParallelAsync correctly — BuildTestsForMetadataAsync returns IReadOnlyList<AbstractExecutableTest>, which satisfies the method's IEnumerable<AbstractExecutableTest> return type.
• TestDiscoveryResult.Tests / ITestExecutor.ExecuteTests narrowing from IEnumerable to List: confirmed TestSessionCoordinator is the only implementer of the internal ITestExecutor interface, and TestRequestHandler is the only external caller — discoveryResult is a local, single-use variable in both call sites, so passing the same list reference into ExecuteTests (which later mutates it via deferred-placeholder expansion) is safe; nothing else retains or re-reads discoveryResult.Tests afterward.
• TestFilterService.FilterTests/FilterOutExplicitTests already return concrete List<AbstractExecutableTest> under their IReadOnlyCollection signature, so the as List<...> ?? [.. filteredTests] cast in TestDiscoveryService.DiscoverTests hits the fast path on every current call site; the ?? fallback is harmless defensive coding for future callers rather than dead weight.

All changes are internal-only (no public API surface, no source-generator output), so the CLAUDE.md dual-mode/snapshot-testing requirements don't apply here.

Nit (non-blocking): the filteredTests as List<AbstractExecutableTest> ?? [.. filteredTests] cast is currently unreachable in the ?? [...] branch given today's callers — worth a one-line comment noting it's defensive for future IEnumerable-returning implementations of FilterTests, so a future reader doesn't wonder if it's dead code.