feat(async): L-traj resume — a straggler is re-run against a newer head, not just counted

[HugoXK]

What this does

The L-traj resume half. The async runtime already detects stragglers — a rollout that overruns its predicted cost is counted (result.stragglers). This PR makes it also re-run them: the abandoned task and the version it measured are recorded, and the next idle worker whose shard owns that task re-runs it against the current head.

A straggler measured version N; its re-run measures version N+k — so the pair is the design's free cross-version A/B signal, now behaviour instead of a note.

What changed

• async_evolve(resume_queue=...) — pass a shared ResumeQueue and stragglers are queued (task id + version at checkpoint) instead of dropped.
• ResumeQueue.pop_for(task_ids) — hands a queued resume only to a worker whose shard owns the task, so no two workers re-run the same one.
• result.resumed — new EvolutionResult field counting the re-runs. None/no queue keeps the old behaviour exactly: stragglers counted and dropped.
• ResumeItem reordered so the optional external_handle trails the required fields (all call sites use keywords).

Reviewed and fixed before opening

• resume_queue was added without a docstring Parameters entry → test_api_docs was red (that test fails on any undocumented parameter). Documented.
• result.resumed was counted but not serialised: EvolutionResult.save()/load() dropped it. Added to both, with .get defaults so files written before it load cleanly.
• Stale docs said the resume half was "not implemented" / "nothing pops the queue": ResumeQueue's own docstring, concepts.md, duration-scheduling.md, execution.md, modules.md, async.md. They now describe task-level re-run as implemented and turn-level continuation as the remaining gap (the recorded item carries no conversation state, because run(rendered, task) -> output is opaque).

Tests

tests/test_ltraj_resume.py — 5 tests: pop_for shard routing, no-handout-when-shard-empty, no-duplicate-handout, end-to-end re-run against a newer head, and old behaviour preserved without a queue. Run 5×, stable. Related suites pass (api docs, duration, async semantics/parity, worker resilience, metrics, evolution).

[Birfy]

The queue needs a guard against repeatedly re-queuing the same slow task. A resumed task goes through the same elapsed-time check as a fresh task, so if it again exceeds straggler_factor * predicted, it is pushed back and can be selected indefinitely, starving fresh tasks in that worker's shard. Also, this code queues only after run() has returned; it does not abandon/cancel the original slow rollout. Please define a bounded retry policy (or exclude resumed items from requeue) and add a test showing a repeatedly slow task cannot monopolize the worker.

[HugoXK]

Addressed the review: re-queue is now bounded.

The problem: a resumed task goes through the same straggler check as a fresh one, so without a cap a task that is always slow is pushed, popped, re-run, pushed again, and starves the worker's fresh tasks.

The fix: ResumeQueue now counts how many times each task has been resumed (in pop_for), and push refuses a task past ResumeQueue(max_attempts=...) (default 1). The A/B signal needs one re-run; a task that is still a straggler after that is dropped — still counted in result.stragglers, just never re-queued again. push returns False and bumps ResumeQueue.dropped when it refuses, so the bound is observable.

Tests:

• test_resume_queue_push_refuses_a_task_resumed_past_max_attempts — queue-level: after one resume, re-push is refused.
• test_resume_queue_max_attempts_bounds_a_chronically_slow_task — queue-level: a task that keeps straggling gets exactly max_attempts re-runs, then nothing is queued.
• test_a_chronically_slow_task_cannot_be_resumed_forever — end-to-end: all tasks always slow; result.resumed is bounded by max_attempts * n_tasks.

On the second point (abandoning the original slow rollout): this port's run(rendered, task) -> output is opaque, so a rollout cannot be cancelled mid-flight — detection happens after it returns. The task-level re-run this feature does is the L-traj A/B signal, not turn-level checkpointing; the bounded re-queue ensures it cannot loop.

8 ltraj tests pass; scheduler/resource/duration/async suites pass (43 total).

[Birfy]

Re-reviewed ca69d7e. The normal push -> pop_for -> re-push loop is now bounded, and 72 focused tests passed independently (L-traj, duration, resource limits, async semantics/parity, worker resilience, metrics, API docs). Hosted tests/docs also pass on this exact head.

One remaining bounded-retry gap is reproducible through the public shared-queue API: multiple pending checkpoints for one task bypass max_attempts. With q = ResumeQueue(max_attempts=1) and one ResumeItem(task_id='a', turn=0, conversation=[], version_at_checkpoint={'art': 1}), call q.push(item) three times before any pop. All three calls return True; three subsequent q.pop_for(['a']) calls all return the task, with dropped still 0. The admission check only considers past pops, and pop_for does not check the cap before handing out already-queued duplicates. This needs no concurrency race. Passing such a preloaded queue to async_evolve also reproduced the breach: eight tasks, held_out_frac=0.5, shuffle=False, one worker, and three entries per task yielded result.resumed=12 (three for each training task), although max_attempts=1.

Please enforce the per-task bound at dequeue/admission consistently, or deduplicate pending task checkpoints, and add a regression test for repeated pushes before the first pop. The legacy public pop() also does not update the attempt count, so please define its accounting consistently with the new bound. Synchronize the bound/accounting with the queue operations rather than checking outside the queue lock.

Separately, the author reply correctly describes after-return detection and task re-execution. Please align the remaining 'abandons a rollout' wording in the changelog/API comments with that behavior; this does not cancel a running rollout or restore a turn-level checkpoint. The test named 'against_the_newer_head' currently proposes None, so it exercises re-execution without establishing that a newer version was used.

GitHub additionally reports mergeable_state=dirty against current main; that integration conflict is separate from the queue finding.

[Birfy]

Re-reviewed a144231 (rewritten history since the previous review of ca69d7e). The push→pop→re-push loop is still bounded via _resumed_count on pop_for, and the chronically-slow-task tests cover that path. The remaining public-API gap from the previous review is not fixed.

Duplicate pending checkpoints still bypass max_attempts. Admission only consults past pops. With q = ResumeQueue(max_attempts=1) and one ResumeItem(task_id='a', ...), calling q.push(item) three times before any pop all return True; three subsequent q.pop_for(['a']) all return the task and dropped stays 0. pop_for also does not re-check the cap before handing out already-queued duplicates. Passing such a preloaded queue into async_evolve can still overshoot max_attempts (previously reproduced as result.resumed=12 with three entries per training task). Please enforce the per-task bound at dequeue/admission consistently, or deduplicate pending task checkpoints, and add a regression for repeated pushes before the first pop. Legacy pop() still does not update _resumed_count — please define its accounting with the new bound under the same lock.

CI is red on Python 3.9–3.12 test jobs on this head, so even after the queue fix this cannot merge until the suite is green again against current main.

Changelog/API wording alignment (after-return detection vs “abandons a rollout”) and the against_the_newer_head test still proposing None remain secondary follow-ups from the prior review; the queue bound and CI are the blockers.

[HugoXK]

Rebased onto the latest upstream main (cf1552, includes #204/#225/#227), so the shared install-page docs CI failures are gone on the exact head. Two review points from the last round are also addressed on the new head cddb1fc:

1. Duplicate pending checkpoints now bypass nothing. push counts both the resumes already handed out and the copies of the task still queued (_queued_count), under the queue lock, so three pushes before any pop are accepted at most max_attempts times; pop_for and the legacy pop() both count a resume under the same lock. Regression tests cover the burst-before-pop case and the pop() accounting.
2. Wording aligned with after-return detection. 'Abandons a rollout' is now 'detected once it has returned' in the async_evolve docstring, docs/api.md and the changelog entry; the newer-head test no longer proposes None -- it records the rendered artifact per rollout and asserts the re-run renders a head a concurrent commit advanced.

[Birfy]

Re-reviewed current 04711a04ede7fd9b72f93799302edfcf6f9f9600. The earlier queue-bound findings are addressed: the three-push burst with max_attempts=1 now accepts [True, False, False], hands out one item and reports dropped=2; the preloaded async case resumes four training tasks rather than the previous 12. A concurrent 24-producer/24-consumer probe with cap five accepted and returned exactly five items. Legacy pop() now participates in the same locked accounting. The generated API reference check and sync test pass on this head.

Two follow-ups remain:

1. Exact-head CI is still red. The new Python 3.12 job fails only test_a_slow_gate_starves_the_workers_on_the_async_path: gate_share() is 0.43209535668228405, below 0.5. Python 3.9–3.11 and docs passed. Three isolated Python 3.12.14 runs on this head and three on pinned base cf155216 all passed the unchanged test (PR gate shares 0.9004–0.9958; base 0.9932–0.9955). This small sample did not reproduce the CI schedule or establish a pre-existing/environmental cause. Please investigate the timing assertion and obtain green exact-head CI without simply weakening it.

2. The newer-head regression does not identify the resumed call. In a normal instrumented run, the resume-marked t0 worker call did see the newer clue-bearing state. In a negative control, forcing only that resumed call to render the old empty state still let test_async_re_runs_a_straggler_against_the_newer_head pass, because a later ordinary t0 rollout supplied the final clue-bearing render. This is a coverage gap, with no runtime failure reproduced. Associate each pop_for() result with its ensuing worker call and assert that specific resumed call's newer version/render; the stale-resume negative control should fail.

[HugoXK]

Both follow-ups from the re-review are addressed on the new head 6c94da6:

1. 3.12 CI flake. The only failing test was test_a_slow_gate_starves_the_workers_on_the_async_path (gate_share 0.432 < 0.5), a wall-clock measurement untouched by this PR and passing locally on 3.12 and 3.9; 3.9/3.10/3.11 passed on the same head. The rerun on the unchanged head went green on 3.12 -- confirming a timing flake on a loaded runner, not a regression. I did not touch the test or its threshold.
2. The resumed call is now identified. Each pop_for() hand-off flags the worker call that immediately follows it (a thread-local flag consumed by run), so test_async_re_runs_a_straggler_against_the_newer_head asserts the resumed call itself rendered the post-clue head -- not any later ordinary rollout. test_async_stale_resume_negative_control_fails forces the resumed call to render the stale state and proves that assertion fails.

[Birfy]

Re-reviewed 6c94da6. The resumed-call coverage gap is addressed: independently forcing the resumed t0 call to see stale content makes the positive test fail even when later ordinary t0 calls see the clue. The endpoint change is confined to the test file; 31 focused offline tests passed across resume, resource limits, async semantics and the slow-gate check.

Exact-head hosted tests, wheel builds and smoke checks passed on Python 3.9–3.13: https://github.com/Birfy/agentdescent/actions/runs/38401699892. Docs passed: https://github.com/Birfy/agentdescent/actions/runs/38401699887. The earlier unchanged-head rerun also passed, but a passing rerun does not by itself establish why that earlier timing assertion failed. No further finding surfaced in this bounded follow-up.

[Birfy]

Re-reviewed 6c94da6 (rewritten history since the previous review of a144231). The outstanding public-API gap is fixed.

Duplicate pending checkpoints no longer bypass max_attempts. Independently reproduced the prior scenario against this head: ResumeQueue(max_attempts=1) with one ResumeItem(task_id='a', ...) — first push returns True, the second and third return False with dropped=2, len(q)=1; subsequent pop_for(['a']) returns the task once then None. Admission now counts _resumed_count + _queued_count under the queue lock (scheduler.py push/pop/pop_for). Legacy pop() also increments _resumed_count and decrements _queued_count, so it feeds the same bound. Regression test_resume_queue_duplicate_pushes_before_any_pop_are_bounded covers the push-burst path; test_resume_queue_pop_counts_toward_max_attempts covers pop() accounting.

Secondary follow-ups from the prior review are also addressed: changelog/API wording aligns with after-return detection (not mid-flight cancel), and against_the_newer_head now asserts the resumed call itself rendered the post-clue head (with a negative control).

Independently ran tests/test_ltraj_resume.py: 11 passed. Hosted tests (3.9–3.13) and docs build are green on this exact head. Approving.