Consolidate the six repetition combinators into one repeat-loop helper

[carpentry-agent]

What

The six repetition combinators — count, at-most, at-least, many, many1, and range — each inlined the same "accumulate results, dispatch on OkConsumed/OkEmpty/ErrEmpty/ErrConsumed, then fold the consumed bit into the final Reply" loop. They differed only in their lower and upper bounds. This extracts a single two-phase engine and reduces all six to one-line wrappers:

(defn repeat-loop [p src len start-cur lo hi] ...)   ; mandatory first `lo`, then greedy up to `hi` (hi < 0 = unbounded)

(defn count    [n p]    (Parser.init (fn [src len cur] (repeat-loop &p src len @cur n n))))
(defn at-most  [n p]    (Parser.init (fn [src len cur] (repeat-loop &p src len @cur 0 n))))
(defn at-least [n p]    (Parser.init (fn [src len cur] (repeat-loop &p src len @cur n -1))))
(defn many     [p]      (Parser.init (fn [src len cur] (repeat-loop &p src len @cur 0 -1))))
(defn many1    [p]      (Parser.init (fn [src len cur] (repeat-loop &p src len @cur 1 -1))))
(defn range    [lo hi p](Parser.init (fn [src len cur] (repeat-loop &p src len @cur lo hi))))

range was already the fully general form, so it becomes the canonical shape; the others are just points in the (lo, hi) space. This mirrors the existing sep-by1-loop/end-by-loop helpers. Net −64 lines, and the public signatures, doc strings, and behavior of all six are unchanged. skip-many, skip-many1, and many-till keep their own loops (they discard results / take a terminator, so they don't fit this shape).

Bug fix

While unifying, I found a latent inconsistency in the consumed-bit accounting. count, at-least, and range set consumed-any only on OkConsumed, so if the underlying parser consumed input and then failed on the first (mandatory) iteration, they folded that into ErrEmpty instead of ErrConsumed. That lets alt (<|>) wrongly backtrack past a partial match, which violates the Parsec consumed-input discipline the rest of the library follows — sep-by1-loop and many1 already set the bit on ErrConsumed. The shared repeat-loop does the same, so all six are now uniformly correct.

The existing tests never caught this because they only exercise these combinators with byte, which fails empty (never consume-then-fail).

Tests

Added coverage using (then (byte \a) (byte \b)) — which consumes on a match and then fails — wrapped in alt so the consumed bit is observable (a consumed failure must not backtrack, an empty failure must). Covers count, many1, at-least, and range, plus a guard that a genuine empty failure still backtracks. I confirmed each new consumed-failure test fails against the pre-fix behavior.

Full suite: 275/275 (was 270). carp-fmt -c and angler clean; gendocs.carp regenerates without error.

[carpentry-reviewer]

Build & Tests

Checked out and built locally (carp v0.6.0). Full suite 275/275, carp-fmt -c and angler clean, no CHANGELOG in the repo so nothing to update. CI green (ubuntu + macos). parsec.carp nets −115 lines (the −64 in the description is after the added test block).

I verified the refactor two ways rather than trusting the diff:

The bug fix is real and precisely scoped. I ran the branch's new tests against main's old parsec.carp: count, at-least, and range fail the consumed-failure assertion, while many1 passes (it already set the bit) and the empty-failure control passes on both. That matches the PR's claim exactly — old count/at-least/range folded a first-iteration consume-then-fail into ErrEmpty, so alt wrongly backtracked past a partial match. repeat-loop sets consumed-any on every ErrConsumed, so all six are now uniform with sep-by1-loop/many1.

Behavior is otherwise preserved. All 270 pre-existing tests pass unchanged on the new engine, which exercises every combinator across many inputs. I also traced each combinator against repeat-loop:

• count (lo=n,hi=n): phase-2 guard i<hi can't fire → exactly n runs.
• at-most (lo=0,hi=n): error only reachable via the ErrConsumed arm, which now also sets consumed-any, so the result matches old's unconditional ErrConsumed.
• many (lo=0,hi=-1) and many1 (lo=1,hi=-1): equivalent. The one divergence I found — old many1 returned OkEmpty [v1] immediately on an empty-success first hit, whereas the new one runs p once more in phase 2 — is benign: parsers are pure, so the extra call re-hits OkEmpty and stops without pushing, yielding the same OkEmpty [v1] cur. (And many1 on an empty-success parser is already a documented anti-pattern.)

Findings

No blocking issues. One non-blocking note for completeness:

• Negative upper bound is now "unbounded" instead of "no extra". repeat-loop treats hi < 0 as unbounded (that's how at-least/many are built on it), so a hypothetical (range lo negative) or (at-most negative p) now loops greedily where the old code stopped after the mandatory phase. This is invalid input with no caller in the library (I grepped — none), so it's GIGO either way; flagging only because the refactor did change that corner.

Verdict: merge

A clean −115-line deduplication that also fixes a genuine, verified consumed-bit bug, with regression tests that provably bite the old behavior and full preservation of the existing 270 tests.