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-
TimeoutErrormisclassification (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, abortedmutated 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, orNone(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() -> bool—not stopped and in_flight < window, with in-flight tracked by driver-reported deltas or passed in; decide at implementation which reads better — bias to passinglen(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 callsnext_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-awarebucket.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¶
- Streaming bookkeeping — extract only what is textually twinned
today: the window/reorder-buffer accounting and the
_yield_ends_streamfailure 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. - Retry twins —
_execute_outcome/_async_execute_outcomeshare 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. _plan.pydeleted — helpers move into_engine.py;result.pykeeps only public containers.- Naming and docstring pass — module docstrings state the
ledger/driver split;
core.pyandaio.pydocstrings point to_engine.pyas 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'score.py + aio.py + _plan.py. - Each driver must read top-to-bottom as one linear story.
- Zero
nonlocalin 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:
- (Codex high + fact-check moderate, same seam)
next_item()cannot own sync admission: the slot is allocated before the deadline-awarebucket.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. - (Codex high + fact-check moderate + design-review trace)
finish(pending)cannot own salvage. Salvage is driver-participated (wait(in_flight, timeout=0)vstask.done()scans) and deliberately asymmetric: the sync-timeout path never salvages (structurally cannot — its only route to expiry is an emptywait), the async-timeout path does. A uniformfinish()silently changes sync-timeout behavior — the exact class of regression the plan forbade. - (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. - (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.
- (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.
- (Codex medium + fact-check low)
_StreamLedgerbreaks 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. - (Design review + fact-check, independently)
_retry_stepwas hollow: the retry decision logic is already deduplicated as pure methods onRetry(_should_retry/_server_wait/_delayinpolicies.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-winsstop()), applied in place in all three engines. The fact-check enumerated every flag mutation site and proved first-writer-wins is exactly equivalent to theaborted and not timed_outread-time formula — including the async timeout-salvage ordering, the only both-flags path. Bonus: the closures no longer neednonlocalfor stop state.- Twin-file cross-pointers in
core.py/aio.pymodule 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.