feat: add should_complete to CompletionConfig - #605
Conversation
862e017 to
c72785b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
c72785b to
306c81c
Compare
306c81c to
5483a62
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5483a62 to
fffee6d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fffee6d to
7ad4242
Compare
7ad4242 to
302330d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| if self.policy.should_complete is not None | ||
| else () | ||
| ) | ||
| decision = self.policy.evaluate(succeeded, failed, items_snapshot) |
There was a problem hiding this comment.
Codex AI review
[P1] Restore prior terminal state before evaluating the predicate
On a resumed incomplete batch, prior child checkpoints replay asynchronously while these counters start at zero. Evaluating after whichever replay event arrives first can make a deterministic status predicate choose a different outcome than before suspension, potentially checkpointing a different result and skipping work. Determine the terminal branches present at invocation start and defer predicate evaluation until all of that state has been restored, including equivalent handling for FLAT nesting.
| submit(pending.popleft()) | ||
| in_flight += 1 | ||
| running += 1 | ||
| needs_snapshot_rebuild = True |
There was a problem hiding this comment.
Codex AI review
[P2] Evaluate the new STARTED snapshot before blocking
This flag is set after submission, but execution proceeds directly to events.get() rather than returning to the loop. Consequently, the predicate is not called for the PENDING to STARTED transition; with max_concurrency=1, it may never observe STARTED before the branch becomes terminal. Predicates based on scheduling state can therefore fail to complete early or block indefinitely. Continue the outer loop after any submissions, or evaluate the rebuilt snapshot immediately before waiting for events.
Codex AI reviewTwo correctness issues affect replay determinism and Reviewed commit |
Issue #, if available: #519
Description of changes:
Adds a
should_completepredicate to CompletionConfig for map and parallel operations, giving full control over when a batch completes early.The predicate receives a CompletionStatus snapshot (counts + per-item statuses) and returns a CompletionDecision - either continue_batch() or complete_batch(outcome). The outcome determines whether
CUSTOM_COMPLETION_SUCCEEDEDorCUSTOM_COMPLETION_FAILEDis reported.Key design decisions -
Testing -
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.