fix(extensions): return pooled buffers after enumeration failures

[snowyukitty]

What this changes

Return the rented ErrorCollection buffer when an ICollection<Error> enumerator throws, and use the number of items actually yielded when Count overstates the sequence length. A private pool seam makes the exception-path ownership test deterministic without changing the public API.

Related issue

Fixes #24.

Checklist

• A test fails without this change
• dotnet test Verdict.sln passes locally (one unrelated existing ProblemDetailsFactory test races with tests that mutate process-wide defaults when the assembly runs in parallel; it passes in isolation)
• CHANGELOG.md updated under ## [Unreleased]

Verification

• dotnet build Verdict.sln --configuration Release (0 errors)
• Verdict.Extensions.Tests (145 passed)
• Verdict.ApiApproval.Tests (8 passed; public API unchanged)
• Solution tests excluding the pre-existing isolation failure (600 passed), followed by that test alone (1 passed)
• Scoped dotnet format ... --verify-no-changes (0 files changed)
• git diff --check

Public API and performance

The public API is unchanged. The new helper is private, and the normal path still uses ArrayPool<Error>.Shared; no relevant benchmark exists in the repository.

Disclosure: OpenAI Codex assisted with implementation and tests. The exact revised patch was independently reviewed by Google Antigravity and validated locally; no AI model is credited as a legal author.

Summary by CodeRabbit

• Bug Fixes

  • Improved error collection handling when enumerating collections fails partway through.
  • Correctly tracks the number of items actually read, even when a collection reports an inaccurate count.
  • Ensured temporary data is properly cleared and returned after successful or failed operations.

• Documentation

  • Added an unreleased changelog entry describing these reliability improvements.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d5062fe2-3c8c-4adc-a34c-2b4a99a97c8f

📥 Commits

Reviewing files that changed from the base of the PR and between 363bca1 and 57b5161.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/Verdict.Extensions/ErrorCollection.cs
• tests/Verdict.Extensions.Tests/ErrorCollectionDisposalTests.cs

---

📝 Walkthrough

Walkthrough

ErrorCollection.Create(IEnumerable<Error>) now uses pool-aware enumeration cleanup, preserves actual yielded counts, and returns cleared buffers on failures. Tests add misreporting collections and tracking pools to validate these behaviors.

Changes

ErrorCollection pooling

Layer / File(s)	Summary
Pool-aware enumeration and disposal src/Verdict.Extensions/ErrorCollection.cs	Enumeration uses the selected pool, records actual item counts, cleans up buffers on empty or failed enumeration, and returns successful buffers through their originating pool.
Enumeration failure and count validation tests/Verdict.Extensions.Tests/ErrorCollectionDisposalTests.cs, CHANGELOG.md	Tests verify buffer identity, clearing, exception propagation, and inaccurate collection counts. The changelog documents the fixes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 57b51

This localized change returns pooled buffers correctly after enumeration failures and uses the number of items actually yielded; the public API is unchanged and no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: arnelirobles

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary fix: returning pooled buffers after enumeration failures.
Linked Issues check	✅ Passed	The changes return pooled buffers with clearing on failure and track actual yielded items, satisfying issue #24.
Out of Scope Changes check	✅ Passed	The changelog, tests, and private pool seam directly support issue #24 without unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[arnelirobles]

Merged. Thank you, and I owe you an explanation for three days of silence that had nothing to do with the work.

Why it sat. Two failures stacked, both mine:

1. I was not watching my own repositories. GitHub does not subscribe an owner to an org repo by default, so no notification was ever generated for this PR. I only found it this morning while sweeping the board manually. Fixed now across all 30 repos.
2. The CI run had been sitting in action_required since 15 August at 18:08, the first-time-contributor approval gate. Then I added build as a required status check, which turned a dormant approval prompt into a hard block. The nasty part is that it is invisible: gh pr checks shows nothing at all for a run in that state, so this PR looked like it passed everything while a required check silently could not run. One approve call and it went green in 54 seconds.

Neither was about your code, and you had no way to see either from your side.

The review

I verified it rather than reading it:

• Release build clean, 145 Extensions tests pass, 8 API approval tests pass with the public surface unchanged.
• Mutation-tested. I put the leak back by removing the pool.Return from the ICollection catch, and Create_WhenCollectionEnumerationThrows_ReturnsClearedBufferToPool fails. So the test genuinely pins the bug rather than passing alongside it.

Three things I want to name specifically.

The CreateWithPool seam is the right call. You cannot assert on ArrayPool<Error>.Shared deterministically, so the obvious version of this test would have been flaky or meaningless. A private overload taking the pool makes the exception path observable without moving anything into the public API, and the approval test confirms that.

You fixed something the issue did not ask for. Using actualCount instead of collection.Count means a collection whose Count overstates its enumeration no longer leaves uninitialised slots in the result. #24 did not mention that; it is a real second bug and the fix is correct.

You diagnosed #33 three days before I did. Your checklist left dotnet test Verdict.sln deliberately unticked with an accurate one-line cause: process-wide defaults mutated by parallel tests. Last night I hit the same failure while preparing a release, spent twenty minutes on it, briefly suspected my own version bump, and filed a duplicate issue for it. Yours was on the board the whole time.

Leaving a box unticked and explaining why is worth more than a green checklist, and it is rarer. It is now in DELIVERY.md as house style: if you cannot verify it, say what you actually measured.

Where to next, if you want more

Your instinct for buffer ownership is exactly what these need, in the order I would pick them:

• A MultiResult and everything derived from it share one pooled buffer, so disposing either breaks the other #28, a MultiResult and everything derived from it share one pooled buffer. The closest thing to what you just fixed and the most interesting: it is an ownership question rather than a missing try, so Select/Where handing out views over a buffer someone else will return is the real design problem. If any issue here is yours, it is this one.
• The allocation gate covers only Result<int> in the core package #25, the allocation gate covers only Result<int>. You clearly understand how to make allocation assertions deterministic, which is the hard half.
• The netstandard2.0 assemblies ship without ever being tested #30, the netstandard2.0 assemblies ship without ever being tested. We multi-target and only test net8.0, so the assembly half our consumers resolve has never had a test run against it.

No pressure on any of them, and comment /take on whichever you fancy so nobody duplicates you. Note that 2.8.0 is being tagged today and your fix is in it.

[arnelirobles]

@snowyukitty this shipped in Verdict 2.8.0, and you are now on https://baryo.dev/community and on the org profile at https://github.com/BaryoDev.

Written up as what it was: buffers were returned to the pool only on the success path, so an enumeration that threw leaked them. A quiet failure that would have been very hard to find from the symptom.

Sorry again about the three days of silence, which was a workflow gate on my side rather than anything about your PR.

#25, #28 and #30 are still open if you fancy another. Thank you.