Skip to content

Commit 945d826

Browse files
committed
fix(eval_optimize_loop): address AI review round 15 — 1 critical + 5 warnings + 1 suggestion
1 parent 697caad commit 945d826

6 files changed

Lines changed: 24 additions & 10 deletions

File tree

examples/optimization/eval_optimize_loop/run_pipeline.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,16 @@ def _pid_alive(pid):
128128
tf.flush()
129129
_os.fsync(tf.fileno())
130130
_os.replace(tmp, LOCK_FILE)
131-
# Verify ownership: if another process raced us, the file contains
132-
# their PID, not ours. In that case, back off.
131+
# Lock is ours after atomic replace. Set acquired=True immediately
132+
# so that finally-block cleanup works even if we crash during the
133+
# verification read below (prevents permanent lock residue).
134+
acquired = True
135+
# Verify ownership: if another process somehow raced us, the file
136+
# contains their PID, not ours. In that case, back off.
133137
with open(LOCK_FILE, "r", encoding="utf-8") as vf:
134138
lock_owner = int(vf.read().strip().split()[0])
135-
if lock_owner == my_pid:
136-
acquired = True
137-
# else: someone else replaced the lock between our replace and read;
138-
# fall through to acquired=False -> exit(75)
139+
if lock_owner != my_pid:
140+
acquired = False # not our lock; don't clean up in finally
139141
except (FileNotFoundError, ValueError, FileExistsError):
140142
pass
141143

examples/optimization/eval_optimize_loop/src/attribution.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ class AttributionReport:
8686
def primary_failure_category(self) -> Optional[AttributionCluster]:
8787
if not self.clusters:
8888
return None
89-
return max(self.clusters, key=lambda c: c.count)
89+
# Tie-break: (-c.count, c.category) for consistency with
90+
# optimization_priority ordering (same sort key used below).
91+
return sorted(self.clusters, key=lambda c: (-c.count, c.category))[0]
9092

9193
@property
9294
def cluster_map(self) -> dict[str, AttributionCluster]:
@@ -131,7 +133,7 @@ def run(
131133
for case in val_result.failed_cases:
132134
all_attrs.append(self._attribute_case(case, "val"))
133135
clusters = self._build_clusters(all_attrs)
134-
opt_priority = [c.category for c in sorted(clusters, key=lambda x: -x.count)]
136+
opt_priority = [c.category for c in sorted(clusters, key=lambda x: (-x.count, x.category))]
135137
attributed = [a for a in all_attrs if a.category != "unattributed"]
136138
return AttributionReport(
137139
total_failures=len(all_attrs),

examples/optimization/eval_optimize_loop/src/auditor.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ def build_trail(self, pipeline_name, mode, random_seed, optimization, baseline_v
5959
now = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
6060
run_id = datetime.now().strftime("%Y%m%d_%H%M%S_%f") + f"_{random_seed}"
6161
entries = []
62+
# NOTE: per-candidate AuditEntry records all carry the same aggregate
63+
# baseline_scores / candidate_scores / cost / latency from the single
64+
# validation run. This is intentional for fake mode where there is no
65+
# per-iteration re-evaluation; in real mode each iteration would have
66+
# its own scores.
6267
for cand in optimization.candidates:
6368
entry = AuditEntry(
6469
timestamp=now,

examples/optimization/eval_optimize_loop/src/baseline.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ async def _run_real_split(
362362
memory_service=memory_service,
363363
)
364364
# ???? ground_truth ??
365-
evaluator.ground_truth = gt_items
365+
evaluator.ground_truth = gt_items # NOTE: relies on PlateEvaluator internal attr; fragile if field renamed
366366

367367
report = await evaluator.run(verbose=False)
368368

examples/optimization/eval_optimize_loop/src/call_agent.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ def create_plate_call_agent(
3232
) -> "callable":
3333
"""Create a PlateAgent call_agent for AgentOptimizer.
3434
35+
Args:
36+
plate_agent_root: Absolute path to PlateAgent project root.
37+
prompt_dir: Reserved for future prompt-directory override (currently unused).
38+
3539
Returns async (query: str) -> str.
3640
Each call re-reads prompt from disk and creates a fresh session
3741
to ensure evaluation isolation.

examples/optimization/eval_optimize_loop/tests/test_validator.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ async def test_four_phase_to_gate(self):
255255
baseline_scores=results["val"].score_map,
256256
candidate_scores=val_result.score_map,
257257
baseline_train_scores=results["train"].score_map,
258-
candidate_train_scores=results["train"].score_map,
258+
candidate_train_scores=val_result.score_map,
259259
baseline_cost=results["val"].summary.avg_cost * results["val"].summary.total,
260260
candidate_cost=val_result.summary.total_cost_candidate,
261261
)
@@ -276,6 +276,7 @@ async def test_four_phase_to_gate(self):
276276
}
277277
j = json.dumps(full_output, ensure_ascii=False, indent=2)
278278
assert len(j) > 2000
279+
assert decision.accepted, f"Gate should accept: {decision.reason}"
279280

280281
print(f"\n Gate decision: accepted={decision.accepted} reason={decision.reason[:80]}")
281282
print(f" Val delta: {val_result.summary.avg_score_delta:+.3f}")

0 commit comments

Comments
 (0)