Redesign VeRO as a generic program optimization harness#41
Redesign VeRO as a generic program optimization harness#41varunursekar wants to merge 21 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| result = await self._finalize() | ||
| _atomic_write_json( | ||
| self.result_path, | ||
| result.model_dump(mode="json"), | ||
| ) | ||
| self._result = result | ||
| return result |
There was a problem hiding this comment.
_atomic_write_json inside an asyncio.Lock in finalize
_atomic_write_json (mkstemp → write → fsync → rename) is called synchronously while holding self._lock, blocking the event loop for the full duration of the disk write. Every coroutine waiting on self._lock — or any other coroutine in the same event loop — is stalled until fsync returns. The same pattern was addressed in engine.py and budget.py with asyncio.to_thread; the same fix applies here.
| result = await self._finalize() | |
| _atomic_write_json( | |
| self.result_path, | |
| result.model_dump(mode="json"), | |
| ) | |
| self._result = result | |
| return result | |
| result = await self._finalize() | |
| await asyncio.to_thread( | |
| _atomic_write_json, | |
| self.result_path, | |
| result.model_dump(mode="json"), | |
| ) | |
| self._result = result | |
| return result |
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/harbor/verifier.py
Line: 394-400
Comment:
Blocking `_atomic_write_json` inside an `asyncio.Lock` in `finalize`
`_atomic_write_json` (mkstemp → write → fsync → rename) is called synchronously while holding `self._lock`, blocking the event loop for the full duration of the disk write. Every coroutine waiting on `self._lock` — or any other coroutine in the same event loop — is stalled until fsync returns. The same pattern was addressed in `engine.py` and `budget.py` with `asyncio.to_thread`; the same fix applies here.
```suggestion
result = await self._finalize()
await asyncio.to_thread(
_atomic_write_json,
self.result_path,
result.model_dump(mode="json"),
)
self._result = result
return result
```
How can I resolve this? If you propose a fix, please make it concise.
| finally: | ||
| await candidate_workspace.destroy() |
There was a problem hiding this comment.
TaskGroup cancellation
await candidate_workspace.destroy() in the finally block is not wrapped with asyncio.shield. When max_concurrency > 1 and one produce(proposal) task raises, asyncio.TaskGroup cancels all remaining tasks by calling task.cancel(). Python raises CancelledError at the first await the cancelled task encounters — which is the await candidate_workspace.destroy() call here. The destroy() coroutine is then aborted at its own internal await (e.g., the self._sandbox.run call inside _remove_worktree), leaving the git worktree registered and on disk. The temp_copy context manager already uses asyncio.shield for the same reason, so the fix is consistent with existing practice.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vero/src/vero/optimization/optimizer.py
Line: 281-282
Comment:
**Unshielded worktree cleanup under `TaskGroup` cancellation**
`await candidate_workspace.destroy()` in the `finally` block is not wrapped with `asyncio.shield`. When `max_concurrency > 1` and one `produce(proposal)` task raises, `asyncio.TaskGroup` cancels all remaining tasks by calling `task.cancel()`. Python raises `CancelledError` at the first `await` the cancelled task encounters — which is the `await candidate_workspace.destroy()` call here. The `destroy()` coroutine is then aborted at its own internal `await` (e.g., the `self._sandbox.run` call inside `_remove_worktree`), leaving the git worktree registered and on disk. The `temp_copy` context manager already uses `asyncio.shield` for the same reason, so the fix is consistent with existing practice.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
vero.tomlworkflow, baseline-onlyvero evaluate, repeatablevero run, and a complete CLI for objectives, constraints, case selection, refs, environments, limits, and concurrencypaper/v1and thepaper-v1tagHarbor and Modal
harbor runjobsharbor[modal]==0.18.0and select an explicit inner Python versionHardening
database.jsonfrom canonical completed evaluation directories on resume, closing the write-order crash windowrefs/vero/...refs without leaking normal branchesValidation
uv lock --checkgpt-5) run completed the full nested workflow on Modal: status, candidate validation through inner Harbor, submission, trusted hidden final evaluation through inner Harbor, and outer reward 1.0 with no exceptionsThe final live topology is:
Harbor (Modal, Codex) -> VeRO sidecar -> Harbor validation (Modal) -> submit -> VeRO verifier -> Harbor hidden final (Modal)Greptile Summary
This PR replaces the paper-era VeRO implementation with a fully generic program optimization harness: backend-neutral candidate/evaluation/optimization/runtime interfaces, a declarative
vero.tomlworkflow, Harbor/Modal integration as both outer task and inner evaluation backend, and hardened worktree, budget, and session lifecycle management.EvaluationEnginewith authorization resolver, discriminated-union optimizer config,asyncio.to_threadfor all durable writes (engine, budget, sidecar, session), andasyncio.TaskGroup-based parallel candidate production replacing the previousasyncio.gatherapproach.GitCandidateTransportfetches via unique temporary refs to avoidFETCH_HEADraces, retains commits under hiddenrefs/vero/…refs, and validates all paths and symlinks before extraction in the build compiler.Optimizer._produce_candidate, thefinally: await candidate_workspace.destroy()is not wrapped withasyncio.shield, so whenTaskGroupcancels sibling tasks the git worktree removal can be interrupted mid-execution, leaving worktrees registered and on disk — unliketemp_copy, which already usesasyncio.shieldfor the same cleanup.Confidence Score: 4/5
Safe to merge with the worktree cleanup fix; all other hardening, atomicity, and security changes look correct.
The entire new evaluation/optimization/harbor stack is well-structured and the previously-flagged blocking-write issues are resolved with asyncio.to_thread throughout. One gap remains: Optimizer._produce_candidate calls await candidate_workspace.destroy() in a bare finally block that is not shielded, so a TaskGroup cancellation during parallel production can abort the git worktree teardown mid-way, leaving worktrees registered on disk. All other cleanup paths (temp_copy, budget reservation, session manifest) are already shielded consistently.
vero/src/vero/optimization/optimizer.py — the _produce_candidate finally block needs asyncio.shield on destroy().
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant CLI as vero CLI participant Session as OptimizationSession participant Optimizer as Optimizer participant TG as asyncio.TaskGroup participant Producer as CandidateProducer participant Workspace as GitWorkspace copy participant Engine as EvaluationEngine participant Backend as EvaluationBackend CLI->>Session: run() Session->>Optimizer: run(baseline) Optimizer->>Engine: evaluate_candidate(baseline) Engine->>Backend: evaluate(request) Backend-->>Engine: EvaluationReport Engine-->>Optimizer: EvaluationRecord baseline loop max_rounds Optimizer->>Optimizer: strategy.propose(context) Optimizer->>TG: create tasks parallel proposals TG->>Workspace: copy from_version parent.version TG->>Producer: produce workspace evaluation_gateway Producer->>Engine: evaluate_current optional checkpoints Engine-->>Producer: EvaluationRecord Producer-->>TG: CandidateChange Note over TG,Workspace: finally await destroy not shielded TG->>Engine: evaluate_candidate new_candidate Engine->>Backend: evaluate request Backend-->>Engine: EvaluationReport Engine-->>Optimizer: EvaluationRecord end Optimizer-->>Session: OptimizationResult Session-->>CLI: best candidate%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant CLI as vero CLI participant Session as OptimizationSession participant Optimizer as Optimizer participant TG as asyncio.TaskGroup participant Producer as CandidateProducer participant Workspace as GitWorkspace copy participant Engine as EvaluationEngine participant Backend as EvaluationBackend CLI->>Session: run() Session->>Optimizer: run(baseline) Optimizer->>Engine: evaluate_candidate(baseline) Engine->>Backend: evaluate(request) Backend-->>Engine: EvaluationReport Engine-->>Optimizer: EvaluationRecord baseline loop max_rounds Optimizer->>Optimizer: strategy.propose(context) Optimizer->>TG: create tasks parallel proposals TG->>Workspace: copy from_version parent.version TG->>Producer: produce workspace evaluation_gateway Producer->>Engine: evaluate_current optional checkpoints Engine-->>Producer: EvaluationRecord Producer-->>TG: CandidateChange Note over TG,Workspace: finally await destroy not shielded TG->>Engine: evaluate_candidate new_candidate Engine->>Backend: evaluate request Backend-->>Engine: EvaluationReport Engine-->>Optimizer: EvaluationRecord end Optimizer-->>Session: OptimizationResult Session-->>CLI: best candidatePrompt To Fix All With AI
Reviews (6): Last reviewed commit: "Document nested Modal Harbor workflow" | Re-trigger Greptile