From 592747d98dd9bf862fb43c30a95be7e3b743a235 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 12:14:18 +0000 Subject: [PATCH] fix: resolve review budget feedback --- pilot/review/budget.py | 22 +++++++++++----------- pilot/review/opencode_runtime.py | 24 +++++++++++++++++++++++- tests/pilot/review_tests/budget_test.py | 16 +++++++++++++++- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/pilot/review/budget.py b/pilot/review/budget.py index 59b1f82..f30780b 100644 --- a/pilot/review/budget.py +++ b/pilot/review/budget.py @@ -163,15 +163,15 @@ def equivalent_cost(usage: dict, model: str, price_target: str = "") -> float: """Estimate comparison cost for one completed iteration.""" try: from cost_model import PRICES, Usage, cost - target = price_target or os.environ.get("PRAGENT_PRICE_TARGET", "claude-sonnet-5") - price = PRICES.get(target) - if price is None: - return 0.0 - return cost(Usage( - uncached_input=max(0, int(usage.get("input") or 0) - int(usage.get("cache_read") or 0)), - cached_input=int(usage.get("cache_read") or 0), - cache_writes=int(usage.get("cache_write") or 0), - output=int(usage.get("output") or 0), - ), price) - except Exception: + except (ImportError, ModuleNotFoundError): return 0.0 + target = price_target or os.environ.get("PRAGENT_PRICE_TARGET", "claude-sonnet-5") + price = PRICES.get(target) + if price is None: + return 0.0 + return cost(Usage( + uncached_input=max(0, int(usage.get("input") or 0) - int(usage.get("cache_read") or 0)), + cached_input=int(usage.get("cache_read") or 0), + cache_writes=int(usage.get("cache_write") or 0), + output=int(usage.get("output") or 0), + ), price) diff --git a/pilot/review/opencode_runtime.py b/pilot/review/opencode_runtime.py index 534f2d8..f1de8b5 100644 --- a/pilot/review/opencode_runtime.py +++ b/pilot/review/opencode_runtime.py @@ -1,6 +1,7 @@ """Isolated opencode process runtime.""" import os +import selectors import subprocess import time @@ -130,6 +131,8 @@ def _run_process( cmd, cwd=cwd, env=env, capture_output=True, text=True, stdin=subprocess.DEVNULL, timeout=timeout, ) + if runner not in (None, subprocess.run): + raise ValueError("custom runners are unsupported for budgeted streaming") existing_reason = budget_state.reason() if existing_reason: budget_state.cap_reason = existing_reason @@ -142,9 +145,27 @@ def _run_process( previous = {"steps": 0, "total": 0, "output": 0, "cost": 0.0} cap_reason = "" started = time.monotonic() + selector = selectors.DefaultSelector() try: assert proc.stdout is not None - for line in proc.stdout: + selector.register(proc.stdout, selectors.EVENT_READ) + while selector.get_map(): + remaining = budget.max_duration_seconds - (time.monotonic() - started) + if remaining <= 0: + cap_reason = "max_duration_seconds" + budget_state.cap_reason = cap_reason + proc.terminate() + break + events = selector.select(timeout=remaining) + if not events: + cap_reason = "max_duration_seconds" + budget_state.cap_reason = cap_reason + proc.terminate() + break + line = proc.stdout.readline() + if not line: + selector.unregister(proc.stdout) + break output.append(line) _, usage = parse_events("".join(output)) if usage: @@ -180,6 +201,7 @@ def _run_process( proc.kill() proc.wait() finally: + selector.close() if proc.stdout: proc.stdout.close() stderr = proc.stderr.read() if proc.stderr else "" diff --git a/tests/pilot/review_tests/budget_test.py b/tests/pilot/review_tests/budget_test.py index bb5e6da..91369a6 100644 --- a/tests/pilot/review_tests/budget_test.py +++ b/tests/pilot/review_tests/budget_test.py @@ -3,6 +3,7 @@ import os import sys import json +import subprocess ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) sys.path.insert(0, os.path.join(ROOT, "pilot")) @@ -56,7 +57,20 @@ def test_process_terminates_after_step_budget(): proc = opencode_runtime._run_process( [sys.executable, "-u", "-c", code], cwd=".", env=os.environ.copy(), timeout=10, parse_events=parse_opencode_events, budget=budget, - budget_state=state, model="glm-5.2:cloud", runner=None, + budget_state=state, model="glm-5.2:cloud", runner=subprocess.run, ) assert state.snapshot()["cap_reason"] == "max_steps" assert proc.stdout.count("step_finish") == 1 + + +def test_process_terminates_silent_child_at_duration_budget(): + code = "import time; time.sleep(30)" + budget = Budget(max_steps=20, max_duration_seconds=1) + state = BudgetState(budget) + proc = opencode_runtime._run_process( + [sys.executable, "-u", "-c", code], cwd=".", env=os.environ.copy(), + timeout=10, parse_events=parse_opencode_events, budget=budget, + budget_state=state, model="glm-5.2:cloud", runner=subprocess.run, + ) + assert state.snapshot()["cap_reason"] == "max_duration_seconds" + assert proc.stdout == ""