Skip to content

v0.6 Plan — The Ledger Refactor (internal quality)

Status: HELD — the kill-switch fired at plan stage. Three adversarial reviews (see the amendments log) independently concluded the ledger boundary does not exist cleanly in this code: salvage is driver-participated and deliberately asymmetric, the sync pacing-reject mutates two pieces of ledger state mid-driver, the honest net-deletion estimate (−55 to −65 lines, ~3.5%) is consumable by docstrings, and no near-term feature needs the room. The minimal cut that survived review was implemented on this branch: _StopReason + first-writer-wins _RunStop (in _plan.py, applied in place in all three engines) and twin-file cross-pointers in the core.py/aio.py module docstrings. Revisit only when a concrete engine feature (a third caller) needs the room, or a third drift bug defeats the cross-pointer guardrail.

The original draft follows, unedited, as the record of what was proposed and why it was held.


Pure internal refactor: zero public API change, zero behavior change. The 341-test suite is the contract — it must pass unmodified (except the timing-margin hardening in Phase 5, each change individually justified).

Read together with the v0.6 Engine Unification Plan, whose Round-2 review produced the debt list this plan retires, and DevX Principles.

Theme

Code is read far more often than written. The unification left the library with one conceptual engine but two textual ones: the sync and async collected drivers are ~150-line near-twins, and the streaming drivers twin the same way. The duplication is not hypothetical debt — it has a casualty list:

  • The cached-admission deadline fix (Round 2 #1) was one bug fixed twice, once per twin.
  • The source-TimeoutError misclassification (Round 2 #3) existed in only one twin — proof they drift.
  • The flag-exclusivity rule (aborted and not timed_out) is written at two call sites as a convention, not enforced by construction.
  • Run state lives in closure soup: nonlocal completed, failures, timed_out, aborted mutated from three places per engine — exactly where both Round-2 races lived.

The design idea, in one paragraph

Do not abstract over sync/async execution — that is the trap (colored functions make a unified driver awkward, inversion-of-control makes it unreadable). Instead extract the thing that is already identical and purely synchronous: the bookkeeping. A run is a ledger — slots, cache hits, failure counts, the stop reason, the aftermath fills, the progress reports. The ledger has no I/O and no awaits; it never blocks. Each driver stays a short, linear loop in its native idiom (concurrent.futures.wait / asyncio.wait) that asks the ledger what to admit and tells it what completed. The drivers own how to run things; the ledger owns what is true.

        sync driver                       async driver
   (pool, wait, shutdown)          (tasks, asyncio.wait, cancel)
            │                                 │
            └────────────► _Ledger ◄──────────┘
              admit-next / absorb / stop / aftermath / result
                    (pure state, one source of truth)

New internal module: pyarallel/_engine.py

Absorbs _plan.py (its five helpers are exactly this bookkeeping) and adds:

_StopReason — the state machine, made explicit

class _StopReason(enum.Enum):
    TIMED_OUT = "timed_out"
    ABORTED = "aborted"

A run is running (stop is None) or stopped for exactly one reason. timed_out/aborted become derived views. The exclusivity that is today a two-site convention becomes unrepresentable-by-construction: stop(reason) is first-writer-wins —

def stop(self, reason: _StopReason) -> None:
    if self._stop is None:
        self._stop = reason

_CollectedLedger — the shared run state

Owns: results, completed, failures, total, window, store, deadline, max_errors, on_progress, _stop. Methods (names final at implementation, shapes fixed here):

  • next_item() -> tuple[int, Any] | None — the cached-admission loop, written once: deadline check (stop(TIMED_OUT)), next(source) / StopIteration, slot allocation, checkpoint lookup with cache-hit absorption and progress. Returns the next slot that actually needs running, or None (exhausted or stopped — caller checks .stopped). The driver submits it in its own idiom. No callbacks, no inversion.
  • absorb(idx, outcome, error) — classify, checkpoint-put, count failures, stop(ABORTED) at the threshold, progress. (The driver extracts outcome/error from its Future/Task — the only per-runtime line — and hands over plain values.)
  • want_more() -> boolnot stopped and in_flight < window, with in-flight tracked by driver-reported deltas or passed in; decide at implementation which reads better — bias to passing len(in_flight) in, keeping the ledger free of driver objects.
  • finish(pending: Iterable[int]) — the aftermath, written once: timeout marking / abort salvage-fills, sized fill-by-count vs unsized shorter-result, no-drain guaranteed structurally (the ledger never holds the source after stop… it holds the source, so: the aftermath provably never calls next_item).
  • result() -> ParallelResult — flags derived from _stop, exclusive forever, in one place.

The ledger is plain synchronous Python — both drivers call it directly; no async variant exists or is needed.

What the drivers keep (deliberately)

  • Sync: pool construction, _submit_task (contextvars), deadline-aware bucket.wait, the gated mid-fill sweep, wait(FIRST_COMPLETED), shutdown semantics.
  • Async: task creation, semaphore/limiter-inside-task, asyncio.wait, asyncio.timeout + scope.expired(), cancel-and-await cleanup.

Each driver should land at ~60–80 lines of linear code with zero nonlocal — every mutation goes through a named ledger method.

Also in scope

  1. Streaming bookkeeping — extract only what is textually twinned today: the window/reorder-buffer accounting and the _yield_ends_stream failure counting into a small _StreamLedger (same module). The yield-driven loops stay in place, per-runtime. Same net-deletion rule applies; if streaming extraction doesn't delete lines, skip it and record why.
  2. Retry twins_execute_outcome / _async_execute_outcome share their decision logic via one pure function: _retry_step(retry, attempt, exc) -> _RetryStep | None (should-retry, delay, server-wait). The twins keep only their native sleep/wait lines. The timing/attempt-counting contract is pinned by the existing metadata tests.
  3. _plan.py deleted — helpers move into _engine.py; result.py keeps only public containers.
  4. Naming and docstring pass — module docstrings state the ledger/driver split; core.py and aio.py docstrings point to _engine.py as the single source of bookkeeping truth.

Explicitly out of scope

  • Any public API or behavior change (including ItemResult.__init__ — shipped surface, works, boring).
  • Any sync/async execution abstraction (sans-I/O drivers — rejected as astronautics; the ledger is the sans-I/O core and that is enough).
  • Sequential engines: they are already linear and readable; port them to the ledger only if it deletes lines there too.
  • Performance work (Phase-1 benchmark re-run is a gate, not a goal).

The kill-switch (Carmack clause)

The refactor must pay in the only currency that counts for readers:

  • Net line count across core.py + aio.py + _engine.py (+result.py) must go down versus today's core.py + aio.py + _plan.py.
  • Each driver must read top-to-bottom as one linear story.
  • Zero nonlocal in the collected drivers.
  • If mid-implementation the ledger needs driver-type awareness, or callbacks-into-the-driver, or an async variant — stop, revert the phase, record why in the amendments log. Duplication beats the wrong abstraction; we already know the duplication's price and it is payable.

Phases

Phase Work Gate
1 _engine.py: _StopReason + _CollectedLedger; sync collected driver ported full suite green, zero test edits; sync driver has no nonlocal
2 Async collected driver ported onto the same ledger full suite green; Round-2 regression tests (cached-deadline, scope.expired, exclusivity) still pin behavior
3 Streaming _StreamLedger (or recorded skip); retry _retry_step full suite green; net-deletion rule holds
4 Delete _plan.py; docstring/naming pass; fold benchmark re-run (same 4 cases as unification Phase 1 — must stay within noise of current numbers) benchmarks recorded below
5 Test hardening: audit wall-clock assertions, widen margins where the regression is preserved (e.g. bigger sleeps, larger gaps), each change justified inline suite green 3× consecutively
6 Review round (four-review) + merge gates all findings triaged below

Branch: v0.6-ledger-refactor, off the merged main (after PR #15). Gates per phase: uv run pytest -q, ruff check + format, mypy --strict, mkdocs --strict.

What success reads like

A new contributor opens core.py, reads a 70-line driver that says submit, wait, absorb, finish, follows one import to _engine.py, and finds every rule of the library — window, cache, deadline, abort, no-drain, flag exclusivity — written exactly once, with the docstring saying why. Nothing clever. Boring on purpose. That is the artwork.


Review amendments log

Round 1 — pre-implementation plan review (2026-07-06): HOLD

Reviewers: Codex adversarial (no-ship as written), independent fact-check review, Opus (implementable with amendments), adversarial design review, Sonnet (hold). The three converged from independent angles; the decision is hold, per the plan's own kill-switch.

Why held — the boundary does not exist cleanly:

  1. (Codex high + fact-check moderate, same seam) next_item() cannot own sync admission: the slot is allocated before the deadline-aware bucket.wait, and the pacing-reject branch fills that exact slot and sets the stop reason — the driver would need a companion stop-before-submit ledger call, or the bucket moves into the ledger. Either way the stated boundary breaks.
  2. (Codex high + fact-check moderate + design-review trace) finish(pending) cannot own salvage. Salvage is driver-participated (wait(in_flight, timeout=0) vs task.done() scans) and deliberately asymmetric: the sync-timeout path never salvages (structurally cannot — its only route to expiry is an empty wait), the async-timeout path does. A uniform finish() silently changes sync-timeout behavior — the exact class of regression the plan forbade.
  3. (Codex medium) "Pure state, no I/O" was false as written: the ledger would own next(source), checkpoint lookup/put, and progress callbacks — fallible side effects that live inside carefully sequenced timeout/cleanup envelopes today.
  4. (Fact-check moderate) Honest line math: −55 to −65 net lines (~3.5%), entirely consumable by this codebase's docstring style. The kill-switch's own currency says the refactor does not pay.
  5. (Design review high) No near-term beneficiary (the remaining v0.6 items don't touch the engine); the cut runs exactly through the seam where both Round-2 races lived, relocating half of each invariant to a file that cannot see the other half; and the move contradicts the four-day-old Round-2 precedent ("folding saves a file, not complexity") plus DevX principle 8.
  6. (Codex medium + fact-check low) _StreamLedger breaks even or adds lines: the sync _failures_pending() future-peek is deliberately absent in async — sharing the bookkeeping imports the wrong policy or requires Future awareness.
  7. (Design review + fact-check, independently) _retry_step was hollow: the retry decision logic is already deduplicated as pure methods on Retry (_should_retry/_server_wait/_delay in policies.py); the twins duplicate only loop shape and runtime-colored calls, which the extraction would not touch.

What survived review and was implemented (this branch):

  • _StopReason + _RunStop (first-writer-wins stop()), applied in place in all three engines. The fact-check enumerated every flag mutation site and proved first-writer-wins is exactly equivalent to the aborted and not timed_out read-time formula — including the async timeout-salvage ordering, the only both-flags path. Bonus: the closures no longer need nonlocal for stop state.
  • Twin-file cross-pointers in core.py/aio.py module docstrings — the cheap drift guardrail; both historical drift bugs would have been prompted by it.

Explicitly deferred with the hold: timing-test margin hardening (Phase 5) — a real hygiene item, but it belongs in its own change, not bundled with a refactor (design review finding).

Revisit criteria: a concrete engine feature needing a third bookkeeping caller, or a third twin-drift bug that the docstring guardrail failed to prevent.