[Repo Assist] fix: CircularBuffer.GetEnumerator infinite-recursion and destructive-enumeration

[github-actions]

🤖 This PR was created by Repo Assist, an automated AI assistant.

Root Cause

Two bugs existed in CircularBuffer<'T>.GetEnumerator:

Bug 1 — Infinite recursion in interface implementations

interface IEnumerable<'T> with
    member this.GetEnumerator() =
        (this :> IEnumerable<'T>).GetEnumerator()  // ← calls itself forever

interface IEnumerable with
    member this.GetEnumerator() =
        (this :> IEnumerable).GetEnumerator()  // ← calls itself forever

Casting this to the interface and calling GetEnumerator() dispatches to the explicit interface implementation — the same method — causing a stack overflow on any for x in buffer, Seq.toList, or Seq.iter call.

Bug 2 — Destructive enumeration

The member this.GetEnumerator() loop consumed elements via this.Dequeue(1), so iterating the buffer would empty it. For an empty buffer the loop would never terminate (it kept calling loop() after yielding nothing, producing an infinite empty sequence). This violates the IReadOnlyCollection<'T> contract.

Fix

Replace both bugs with a single, non-destructive, index-based enumerator:

member this.GetEnumerator() : IEnumerator<'T> =
    (seq { for i in 0 .. length - 1 -> buffer.[(tail + i) % bufferSize] })
        .GetEnumerator()

interface IEnumerable<'T> with
    member this.GetEnumerator() : IEnumerator<'T> = this.GetEnumerator()

interface IEnumerable with
    member this.GetEnumerator() : IEnumerator = (this.GetEnumerator() :> IEnumerator)

The interface implementations now delegate to the concrete member (not through the interface), eliminating the recursion. The concrete member reads directly from the internal buffer array using (tail + i) % bufferSize — no elements are removed.

Trade-offs

Aspect	Before	After
Stack overflow on for x in buf	✅ (crash)	Fixed
Destructive enumeration	✅ (empties buffer)	Fixed — non-destructive
IReadOnlyCollection contract	❌ violated	✅ respected
Allocation	Recursive seq nodes	Single seq expression

New Tests (6)

• Enqueue(array, offset) 2-arg overload (was untested)
• Seq.toList is non-destructive — count unchanged after enumeration
• for-in loop returns elements in FIFO order
• Wrapped-around buffer preserves FIFO order via Seq.toList
• Empty buffer returns empty list
• IReadOnlyCollection.Count via interface after wrap

Test Status

✅ 901 tests passed, 0 failed (2 pre-existing pending skips)
✅ Fantomas format check passed

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@96b9d4c39aa22359c0b38265927eadb31dcf4e2a

[Copilot]

Pull request overview

Fixes CircularBuffer<'T> enumeration so it no longer stack-overflows (due to explicit-interface infinite recursion) and no longer mutates/empties the buffer during iteration, bringing behavior in line with IReadOnlyCollection<'T> expectations.

Changes:

• Replaced destructive, recursive GetEnumerator implementation with an index-based enumerator over the internal ring buffer.
• Updated explicit IEnumerable<'T> / IEnumerable implementations to delegate to the concrete GetEnumerator (avoiding self-recursion).
• Added tests covering non-destructive enumeration, FIFO ordering (including wrap-around), empty enumeration, and the Enqueue(array, offset) overload.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/FSharpx.Collections/CircularBuffer.fs	Reworks enumerator implementation to be non-destructive and fixes explicit-interface recursion.
tests/FSharpx.Collections.Tests/CircularBufferTests.fs	Adds regression tests for enumeration behavior and Enqueue(array, offset).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

GetEnumerator builds a seq that reads the mutable tail field on each iteration (buffer.[(tail + i) % bufferSize]). If the buffer is modified during enumeration, this can shift the effective start index mid-enumeration and produce duplicated/skipped elements. Consider snapshotting tail (and length) into local let bindings before constructing the sequence so the enumerator is stable for the duration of enumeration.

Suggested change

	member this.GetEnumerator() : IEnumerator<'T> =
	member this.GetEnumerator() : IEnumerator<'T> =
	let tail = tail
	let length = length

[Copilot]

Building result via result <- result @ [ x ] inside the loop repeatedly appends to the end of a list (O(n^2) behavior). Prefer accumulating with x :: result and reversing once at the end, or using a ResizeArray/System.Collections.Generic.List and converting to a list at the end.

Suggested change

	result <- result @ [ x ]
	result <- x :: result
	let result = List.rev result