Added consistent timeout to eval#118
Merged
Merged
Conversation
Introduce a top-level eval_timeout (default 30s), enforced as a hard per-instance subprocess timeout on held-out evaluation and sourced by the bilevel planner and genplan validation. Move the generalize/specialize goal into the system prompt, where it also tells the agent the per-instance wall-clock budget its program gets at evaluation.
The fork + join + SIGINT/kill sequence was duplicated between the held-out eval timeout and the genplan validation loop. Extract it into one helper in episode.py; each caller supplies its own worker and interprets the outcome (genplan classifies into LLM feedback, eval returns metrics or re-raises).
There was a problem hiding this comment.
Pull request overview
This PR centralizes per-instance evaluation wall-clock timeouts under a single eval_timeout configuration value, enforces it during episode execution (including killing hung policies), and threads the timeout into system prompts so generated code is aware of the evaluation budget.
Changes:
- Add a forked-worker execution path (
run_in_forked_worker,run_episode_with_timeout) to enforce hard per-episode wall-clock limits. - Move goal + eval-time budget guidance into the system prompt and update prompt/test expectations accordingly.
- Rename/standardize timeout plumbing across approaches/configs to use
eval_timeoutconsistently.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/test_episode.py | Adds focused tests for forked-worker timeout behavior and run_episode_with_timeout outcomes (success/timeout/crash). |
| tests/test_prompts.py | Updates all system-prompt tests to pass timeout and adds coverage for the new goal/budget text placement and formatting. |
| tests/approaches/test_bilevel_planning_approach.py | Updates tests to use eval_timeout instead of planning_timeout. |
| tests/approaches/test_agentic_per_instance_approach.py | Adjusts assertions to reflect target-instance directives moving from task prompt to system prompt. |
| src/robocode/utils/genplan_validate.py | Replaces ad-hoc process/kill logic with the shared run_in_forked_worker helper for episode validation. |
| src/robocode/utils/episode.py | Introduces run_in_forked_worker + run_episode_with_timeout and a subprocess worker to propagate crashes/timeouts cleanly. |
| src/robocode/prompts.py | Adds generalization_goal(timeout, ...), moves goal/budget into system prompt, and simplifies task-prompt scaffolding. |
| src/robocode/approaches/llm_genplan_approach.py | Renames episode_timeout_s to eval_timeout and propagates into driver configuration and validation. |
| src/robocode/approaches/genplan_driver.py | Switches driver config key from episode_timeout_s to eval_timeout. |
| src/robocode/approaches/bilevel_planning_approach.py | Renames constructor param to eval_timeout and wires it into planner planning_timeout. |
| src/robocode/approaches/best_of_k_approach.py | Uses _eval_timeout for validation calls (standardizing timeout naming). |
| src/robocode/approaches/agentic_per_instance_approach.py | Switches scoring rollout to run_episode_with_timeout(timeout=self._eval_timeout). |
| src/robocode/approaches/agentic_cdl_approach.py | Adds eval_timeout and passes it into system-prompt construction. |
| src/robocode/approaches/agentic_base.py | Adds eval_timeout to base agentic approaches and threads it into system prompt + per-instance goal fields. |
| experiments/run_experiment.py | Passes top-level cfg.eval_timeout into approach instantiation and uses it for episode rollouts. |
| experiments/conf/config.yaml | Introduces top-level eval_timeout config. |
| experiments/conf/approach/llm_genplan.yaml | Removes per-approach timeout knob in favor of shared top-level eval_timeout. |
| experiments/conf/approach/bilevel_planning.yaml | Removes planning_timeout setting; documents that top-level eval_timeout is used. |
| experiments/conf/approach/best_of_k.yaml | Removes per-approach timeout knob in favor of shared top-level eval_timeout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
null does not cleanly disable the timeout: generalization_goal formats it with
{timeout:g} and the bilevel planner compares against it, so a null value crashes
those paths. eval_timeout is a positive number; set a large value for an
effectively unlimited budget.
Remove the dead timeout=None branch from run_episode_with_timeout (no caller passes None; render/history use run_episode directly) and fail fast in run_experiment when eval_timeout is null, so a null config errors clearly instead of crashing the prompt build or hanging the genplan/eval rollout.
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.
Ran into this while cleaning the prompts and decided to make it its own separate PR. This way, we have a single config value that sets timeouts for all our evals, both planner and agent code, and the timeout is sent also into the prompt so the agent knows not to write code that uses up too much time. Also a step towards cleaning up our eval