Skip to content

Commit bcad2f3

Browse files
committed
fix(eval_optimize_loop): address code review feedback
- run_pipeline.py: REJECT exits 0 by default; added --fail-on-reject for CI - pipeline.py: live mode validates evalset/optimizer config file existence - pipeline.py: note cost_usd=0 is intentional (not yet collected from optimizer) - pipeline.py: add asyncio.wait_for timeout enforcement for live mode - test_pipeline.py: fix mock to return different baseline/candidate data - test_models.py: remove dead code (unused path variable, discarded dump)
1 parent fdbd1e4 commit bcad2f3

5 files changed

Lines changed: 83 additions & 18 deletions

File tree

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
optimization_report.*
1+
*
2+
!.gitignore

examples/optimization/eval_optimize_loop/pipeline.py

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,24 @@ def from_config(
5151
config = PipelineConfig.model_validate_json(raw)
5252
cls._resolve_paths(config, config_dir)
5353

54-
if config.mode == "live" and (call_agent is None or target_prompt is None):
55-
raise ValueError(
56-
"live mode requires call_agent and target_prompt "
57-
"to be passed to from_config()"
58-
)
54+
if config.mode == "live":
55+
if call_agent is None or target_prompt is None:
56+
raise ValueError(
57+
"live mode requires call_agent and target_prompt "
58+
"to be passed to from_config()"
59+
)
60+
missing = []
61+
for attr in ("live_train_evalset", "live_val_evalset", "optimizer_config_path"):
62+
value = getattr(config, attr, None)
63+
if not value:
64+
missing.append(f"{attr} is empty")
65+
elif not os.path.isfile(value):
66+
missing.append(f"{attr}: file not found ({value})")
67+
if missing:
68+
raise ValueError(
69+
"live mode requires valid evalset paths and optimizer config: "
70+
+ "; ".join(missing)
71+
)
5972

6073
pipeline = cls(config)
6174
pipeline._live_call_agent = call_agent
@@ -93,10 +106,34 @@ async def run(self) -> PipelineResult:
93106
fa = attribute_failures(self._extract_case_results(baseline_train))
94107
candidate_train, candidate_val = await self._evaluate_trace_candidate()
95108
elif self._config.mode == "live":
96-
baseline_train, baseline_val = await self._evaluate_live_baseline()
97-
fa = attribute_failures(self._extract_case_results(baseline_train))
98-
await self._run_optimization()
99-
candidate_train, candidate_val = await self._evaluate_live_candidate()
109+
# Enforce pipeline-level timeout via asyncio.wait_for.
110+
# AgentOptimizer has its own internal timeout, but this gate-level
111+
# limit ensures the entire run() does not exceed budget.
112+
timeout = self._config.gate.max_duration_seconds
113+
async def _run_live() -> tuple:
114+
baseline_train, baseline_val = await self._evaluate_live_baseline()
115+
fa = attribute_failures(self._extract_case_results(baseline_train))
116+
await self._run_optimization()
117+
candidate_train, candidate_val = await self._evaluate_live_candidate()
118+
return baseline_train, baseline_val, fa, candidate_train, candidate_val
119+
120+
try:
121+
baseline_train, baseline_val, fa, candidate_train, candidate_val = (
122+
await asyncio.wait_for(_run_live(), timeout=timeout)
123+
)
124+
except asyncio.TimeoutError:
125+
duration = time.monotonic() - t0
126+
finished_at = datetime.now(timezone.utc).isoformat()
127+
return PipelineResult(
128+
mode=self._config.mode,
129+
gate_decision="REJECT",
130+
gate_reasons=[f"pipeline timed out after {timeout}s"],
131+
duration_seconds=duration,
132+
cost_usd=0.0,
133+
seed=self._config.seed,
134+
started_at=started_at,
135+
finished_at=finished_at,
136+
)
100137
else:
101138
raise ValueError(f"unknown mode: {self._config.mode}")
102139

@@ -112,6 +149,10 @@ async def run(self) -> PipelineResult:
112149
delta = compute_delta(baseline_split, candidate_split)
113150

114151
duration = time.monotonic() - t0
152+
# Note: cost_usd is always 0.0 here because the pipeline does not
153+
# yet collect per-round LLM costs from the optimizer. The gate's
154+
# max_cost_usd rule will therefore not reject on cost grounds.
155+
# Users should monitor costs via their LLM provider dashboard.
115156
gate = apply_gate(
116157
delta, self._config.gate, cost_usd=0.0, duration_seconds=duration
117158
)

examples/optimization/eval_optimize_loop/run_pipeline.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ async def main() -> None:
2727
default=str(_HERE / "pipeline.json"),
2828
help="Path to pipeline.json (default: pipeline.json in same directory)",
2929
)
30+
parser.add_argument(
31+
"--fail-on-reject",
32+
action="store_true",
33+
help="Exit with code 1 when the pipeline REJECTs the candidate (for CI gating). "
34+
"Without this flag, REJECT is considered a valid pipeline outcome and exits 0.",
35+
)
3036
args = parser.parse_args()
3137

3238
pipeline = EvalOptimizePipeline.from_config(args.pipeline_config)
@@ -39,7 +45,7 @@ async def main() -> None:
3945
print(f" optimization_report.json")
4046
print(f" optimization_report.md")
4147

42-
if result.gate_decision == "REJECT":
48+
if args.fail_on_reject and result.gate_decision == "REJECT":
4349
sys.exit(1)
4450

4551

examples/optimization/eval_optimize_loop/tests/test_models.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ def test_pipeline_result_roundtrip():
7575
started_at="2026-01-01T00:00:00",
7676
finished_at="2026-01-01T00:00:05",
7777
)
78-
path = "/tmp/test_pipeline_result.json"
79-
result.model_dump_json(indent=2)
8078
# Verify direct read-back works
8179
loaded = PipelineResult.model_validate_json(result.model_dump_json())
8280
assert loaded.mode == "trace"

examples/optimization/eval_optimize_loop/tests/test_pipeline.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ def test_pipeline_from_config_trace_mode():
5757

5858
def test_pipeline_from_config_live_mode():
5959
"""Pipeline loads live mode config correctly."""
60+
Path("/tmp/train.json").touch()
61+
Path("/tmp/val.json").touch()
62+
Path("/tmp/optimizer.json").touch()
63+
6064
config_data = {
6165
"mode": "live",
6266
"live_train_evalset": "/tmp/train.json",
@@ -318,11 +322,21 @@ async def test_run_optimization_calls_injected_hook():
318322
async def test_run_trace_mode_orchestration():
319323
pipeline = _make_pipeline()
320324

321-
fake_train = _make_fake_eval_result("train", ["a", "b"], [True, False])
322-
fake_val = _make_fake_eval_result("val", ["c", "d"], [True, True])
325+
fake_train_base = _make_fake_eval_result("train_base", ["a", "b"], [False, False])
326+
fake_train_cand = _make_fake_eval_result("train_cand", ["a", "b"], [True, False])
327+
fake_val_base = _make_fake_eval_result("val_base", ["c", "d"], [True, True])
328+
fake_val_cand = _make_fake_eval_result("val_cand", ["c", "d"], [True, False])
323329

324330
async def _fake_run_eval(_path: str):
325-
return fake_train if "train" in _path else fake_val
331+
if "train_base" in _path:
332+
return fake_train_base
333+
if "train_cand" in _path:
334+
return fake_train_cand
335+
if "val_base" in _path:
336+
return fake_val_base
337+
if "val_cand" in _path:
338+
return fake_val_cand
339+
raise RuntimeError(f"unexpected path: {_path}")
326340

327341
with patch.object(pipeline, "_run_eval", side_effect=_fake_run_eval):
328342
with patch("examples.optimization.eval_optimize_loop.pipeline.write_reports"):
@@ -332,8 +346,13 @@ async def _fake_run_eval(_path: str):
332346
assert result.seed == 42
333347
assert "train" in result.baseline
334348
assert "val" in result.baseline
335-
assert result.baseline["train"].pass_rate == 0.5
336-
assert result.baseline["val"].pass_rate == 1.0
349+
assert result.baseline["train"].pass_rate == 0.0 # both fail
350+
assert result.baseline["val"].pass_rate == 1.0 # both pass
351+
assert result.candidate["train"].pass_rate == 0.5 # one passes now
352+
assert result.candidate["val"].pass_rate == 0.5 # one regressed
353+
# Delta: train has newly_passing, val has newly_failing
354+
assert "a" in result.delta.train.newly_passing
355+
assert "d" in result.delta.val.newly_failing
337356

338357

339358
@pytest.mark.asyncio

0 commit comments

Comments
 (0)