Repository navigation
Conversation
20033b0 to
3cc5ad2
Compare
Birfy
left a comment
There was a problem hiding this comment.
Request changes before merge. The new multi-head tests pass, but the existing suite fails across Python 3.9/3.11/3.12; I reproduced 5 failures in the focused async/pipelined tests.
- Both drivers now unconditionally call
aggregator.step(n_workers)andaggregator.head_for_worker(...). Existing custom aggregators implement the published zero-argumentstep()and need not implementhead_for_worker; CI shows TypeError for PromptBreederPopulation and other implementations, and an async legacy aggregator's worker threads die with AttributeError. Keep these hooks optional for existing aggregators, with compatibility tests. async_evolve._workerchoosesworker_branchonce at startup, beforePopulationAggregator.step()creates forks. It never updates the branch after selection, so async workers continue to snapshot and submit againstdev; the advertised parallel heads are never explored. Choose the branch when refreshing each worker snapshot and add an end-to-end async multi-head test.- The single-head
EvidenceBuffer.ready()and_in_flightvalues changed from artifact ids to tuples, breaking existing async semantics and pipelined-gate tests. Preserve the existing single-head contract or update its consumers/tests deliberately. The currently failing CI must be green before merge.
2e9e617 to
71437b3
Compare
Birfy
left a comment
There was a problem hiding this comment.
The older custom-aggregator compatibility and async refresh concerns appear addressed. A remaining blocker is that _Candidate.branch is recorded during preparation, but the commit call in _decide() still omits it. _commit_with_retry therefore uses its default dev branch for candidates prepared against head/<slot>, so fork work can overwrite dev and leave the fork unchanged. Pass the candidate branch through and add a test proving a fork candidate updates only its own branch.
…re commit Ledger.fork(name, from_branch=DEV): creates a head/<slot> branch with its own version vector. A commit against it does not touch dev. live_heads() lists dev plus all head/ forks. discard_head cleans up. Fork names must start with head/ (git cannot have both dev as a ref file and dev/0 as a ref directory). Candidate.branch and EvidenceCard.branch carry the fork name through the selection and merge pipeline so the aggregator knows where to commit back. Aggregator.head_for_worker(worker_id) returns dev by default. _Candidate carries branch, _branch_of(cards) reads it from evidence, and _commit_with_retry commits to the candidate's branch. step(n_workers=1) accepts a worker count. _in_flight stays keyed by artifact id, not (aid, branch) — the pipelined gate's contract (at most one candidate per artifact in measurement) predates multi-head and its tests assert on ids. PopulationAggregator.step(n_workers) forks a head/<slot> per selected candidate and distributes workers across them via head_for_worker. finalize discards all forks and commits the best candidate to dev. Worker loops (sync evolution.py + async async_evolve.py): _snapshot_for returns (artifact, base_v, branch); each worker snapshots from head_for_worker(wid) via _head_for_worker (compat helper for custom aggregators without the method); async re-selects the branch on every refresh so forks created mid-run are explored. _step(aggregator, n_workers) passes the worker count only to aggregators whose step() accepts it, so a pre-multi-head zero-arg step() keeps working. 16 tests in tests/test_multi_head.py. async/pipelined/genesis regression exit 0.
_Candidate.branch was recorded during preparation but the commit call in _decide() defaulted to dev, so fork work overwrote the confirmed branch and left the fork unchanged. _decide now passes the candidate branch through to _commit_with_retry, and the CAS-rebase path rebases against that same branch (the version vectors differ per fork, so rebasing onto dev would be a different world). Test: test_decide_commits_a_fork_candidate_to_its_own_branch proves a head/0 candidate lands on head/0 and leaves dev at v1.
71437b3 to
b766875
Compare
|
Addressed the review: a fork candidate now commits to its own branch. The problem: The fix: Test: Rebased onto current upstream |
Birfy
left a comment
There was a problem hiding this comment.
Scoped re-review of b766875: the outstanding _decide branch-routing finding is addressed. The candidate now forwards its own branch to _commit_with_retry, and CAS retry snapshots the same branch. Independently ran 161 focused multi-head/ledger/async semantics/parity/policy-contract/pipelined/candidate/aggregator tests: all passed.
Also exercised the retry beyond the new direct-commit test: start candidate on head/0 v1, concurrently advance head/0 with a fork-only update and dev with a different dev-only update, then submit the stale fork candidate. Retry commits head/0 v3 preserving its winning update plus our diff; dev's state/version stay unchanged. This verifies retry does not rebase against dev.
This is confirmation of that fix, not whole-PR approval yet. Exact-head tests run 38023411855 is still red on all Python 3.9/3.10/3.11/3.12 jobs. I inspected each job's failure summary: the failures are test_every_internal_anchor_exists and test_install_guide_names_the_current_number_of_dry_run_ports (broken install-page anchors and stale 'All 20 algorithm ports print' text). Docs build run 38023411845 is green. Please integrate the shared documentation corrections and obtain green CI on the resulting exact head before final approval.
|
Requested CI rerun completed on unchanged current head b766875: attempt 2 is red. All four Python jobs repeat the two already identified install-page docs failures. There is one additional result worth separating from those: Python 3.9 job failed Bounded diagnosis:
Suggested next step: make this regression assert worker-originated reads under a controlled snapshot/refresh schedule, with a negative control that injects a per-rollout read and fails. Account separately for known merger/promotion work. Merely increasing the slack constant or rerunning until green would not establish the intended property. No code/test bounds changed and no further hosted retries started. |
feat(ledger): multiple live heads — fork, head_for_worker, branch-aware commit
The problem
The ledger held one
devbranch.PopulationAggregatorserialised candidates by rewriting it — select one, commit it todev, all workers start fromdev. No two workers could expand different candidates at once.selection.py's docstring said "Making the ledger hold concurrent branches, and redefiningetawhenheadis plural, is a separate change."What this PR adds
Ledger.fork(name, from_branch=DEV)— creates ahead/<slot>branch with its own version vector. A commit against it does not touchdev.live_heads()listsdevplus all forks.discard_headcleans up. Fork names must start withhead/(git cannot have bothdevas a ref file anddev/0as a ref directory, so the prefix avoids the collision).Candidate.branch+EvidenceCard.branch— carry the fork name through the selection and merge pipeline so the aggregator knows where to commit back.Aggregator.head_for_worker(worker_id)— returnsdevby default (every single-head caller).PopulationAggregatoroverrides it to cycle throughhead/<slot>forks, distributing workers across candidates.PopulationAggregator.step(n_workers)— forks ahead/<slot>per selected candidate.finalizediscards all forks and commits the best candidate todev._Candidate.branch+_branch_of(cards)— the merge pipeline carries the branch from evidence cards to the candidate, and_commit_with_retrycommits to it.Worker loops (sync
evolution.py+ asyncasync_evolve.py) —_snapshot_forreturns(artifact, base_v, branch). Each worker snapshots fromhead_for_worker(wid)instead of hardcodedLedger.DEV.EvidenceCardcarriesbranch=worker_branch.Backward compatibility
head_for_workerdevCandidate.branchNoneEvidenceCard.branchNonePopulationAggregator._forks[]head_for_workerreturnsdevstep(n_workers=1)n_workers=1_commit_with_retry(branch=DEV)Ledger.DEVAggregatorProtocol.step(n_workers=1)Verification
Files
10 files changed, +380, -23
Modified:
agentdescent/ledger.py—fork,live_heads,discard_head,HEAD_PREFIXagentdescent/aggregator.py—head_for_worker,_branch_of,_Candidate.branch,_commit_with_retry(branch=),AggregatorProtocol.step(n_workers=1),_drain_audit_queue+_verifier_can_spend+audit_drain_per_stepagentdescent/evolution.py—_snapshot_forreturns 3-tuple, worker loop uses branch,EvolutionResult.audit_drainedagentdescent/async_evolve.py—worker_branch, snapshot from it,EvidenceCard.branchagentdescent/population.py—step(n_workers)forks,head_for_workercycles,finalizediscards,_commit_state(branch=)agentdescent/selection.py—Candidate.branchagentdescent/evolvable.py—EvidenceCard.branchdocs/api.md— regeneratedCHANGELOG.md— entrytests/test_multi_head.py— 12 tests