Repository navigation
A resumed pass takes the implementer seat before the step loop - #106
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Live on muesli #768 this morning, after the plan passed and the implementation reached pull request #777: every wave re-reviewed the same head and recorded nothing, spending a full codex review each time.
The journal shows two refusals in a row:
A wave that resumes a run already at
implementingenters the step loop holding the runner's resume fence, which is an operator attempt. The step loop records the implementation output under it, and the kernel correctly refuses: only an implementer may record an output._kernel_record_outputstill echoes the hash it stored, so the review then binds a blob the kernel never accepted as the run's output, and that is refused too. No verdict lands,next_stepsays review again, the loop waits, and the next wave repeats it. The fix-wave guard that stops a refused verdict being reported as a silent reviewer did its job, which is why this showed up as a quiet wait rather than a misleading park.Two changes:
_resume_back_halfdispatches the implementer after the seats are chosen and before_step_loopruns, so a resumed pass holds the same kind of generation a first pass does. First passes are unchanged: they already hold the implementer generation at that point. An implementer dispatch costs no seat; the budget counts authors and reviewers.Effect on the merge gate
None. Nothing about what a merge requires changes. A verdict still has to bind the artifact the kernel holds, which is precisely the check that refused the bad binding; this change makes the runner present the right binding instead of a wrong one.
Testing
bash batch/run-queue.sh --self-testpasses on LinuxPython suite 1992 passed, 3 skipped;
--self-testrc 0 on macOS. CI ran the self-test on Linux for this branch and it passed (6m21s).Two new tests, each mutation-checked: removing the resume-point dispatch fails
test_a_resumed_pass_takes_the_implementer_seat_before_the_loop, and binding the helper's echo instead of the kernel's answer failstest_a_refused_output_leaves_the_review_bound_to_what_the_kernel_holds. The step-loop test's kernel stubs now model the kernel faithfully: an accepted output becomes the current artifact, a refused one does not. The end-to-endrun_itemharness gains the read-back call in its expected sequence and a stub for it; no generation numbers move, because first passes gain no dispatch.Anything a reviewer should look at twice
_step_looparrives atimplementingwithout an implementer generation. I found none: a repair round dispatches one, and a recorded review redispatches one.max_seats. That will surface on a long enough run.