From 991d9bf6b2e202ba3df26e9fbb045382c75b758c Mon Sep 17 00:00:00 2001 From: Glenn Jocher Date: Tue, 7 Jul 2026 13:20:13 +0200 Subject: [PATCH] Retry billing fixes, bad-link steering, opt-in parallel tools, current-gen Claude pricing (#802) --- actions/review_pr.py | 1 + actions/utils/openai_utils.py | 41 ++++++++++++++++++++++++++++------- tests/test_openai_utils.py | 19 ++++++++++++++-- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/actions/review_pr.py b/actions/review_pr.py index 2f33736..48b77d0 100644 --- a/actions/review_pr.py +++ b/actions/review_pr.py @@ -579,6 +579,7 @@ def generate_pr_review( tool_handlers=tool_handlers, max_turns=MAX_AGENT_TURNS, max_cost=MAX_REVIEW_COST, + parallel_tools=True, # review tools are read-only GitHub/diff reads, safe to batch concurrently request_timeout=(30, 120), retries=1, # one transient failure on any of the sequential turns would otherwise abort the whole review # Do not pass background=True; queued background reviews can consume the full 900s poll timeout. diff --git a/actions/utils/openai_utils.py b/actions/utils/openai_utils.py index f559d45..ac5f260 100644 --- a/actions/utils/openai_utils.py +++ b/actions/utils/openai_utils.py @@ -7,6 +7,7 @@ import os import re import time from collections.abc import Callable +from concurrent.futures import ThreadPoolExecutor import requests @@ -42,7 +43,9 @@ MODEL_COSTS = { # (input, output) per 1M tokens "claude-opus-4-5-20251101": (5.00, 25.00), "claude-opus-4-6": (5.00, 25.00), "claude-opus-4-7": (5.00, 25.00), - "claude-sonnet-5": (2.00, 10.00), # introductory pricing, matches ultralytics/assistant MODELS registry + "claude-sonnet-5": (2.00, 10.00), # introductory pricing through 2026-08-31, then (3.00, 15.00) + "claude-opus-4-8": (5.00, 25.00), + "claude-fable-5": (10.00, 50.00), } SYSTEM_PROMPT_ADDITION = """Guidance: - Ultralytics Branding: Use YOLO11, YOLO26, etc., not YOLOv11, YOLOv26 (only older versions like YOLOv10 have a v). @@ -304,7 +307,9 @@ def _post_openai_response( r.raise_for_status() return r.json(), elapsed - except (requests.exceptions.ConnectionError, requests.exceptions.Timeout, json.JSONDecodeError): + except (requests.exceptions.ConnectionError, json.JSONDecodeError): + # ConnectTimeout subclasses ConnectionError so it stays retryable; a ReadTimeout propagates instead, + # because the request may have completed server-side and re-POSTing it would double-bill. if attempt < retries: print(f"Retrying API request in {2**attempt}s (attempt {attempt + 1}/{retries + 1})...") time.sleep(2**attempt) @@ -356,6 +361,7 @@ def get_agent_response( reasoning_effort: str | None = None, max_turns: int = 6, max_cost: float = 0.0, + parallel_tools: bool = False, retries: int = 2, request_timeout: tuple[int, int] = (30, 120), ) -> str | dict: @@ -363,6 +369,7 @@ def get_agent_response( max_cost is a USD ceiling across all turns (0 disables); once reached, remaining tool turns are skipped and the agent synthesizes a final answer. Models missing from MODEL_COSTS disable max_cost loudly; max_turns still bounds. + parallel_tools runs a turn's batched tool calls concurrently: opt in ONLY when every handler is thread-safe. """ model = model or _get_default_model() if max_cost and model not in MODEL_COSTS: @@ -440,7 +447,11 @@ def get_agent_response( for call in function_calls ] break - next_input = [_handle_function_call(call, tool_handlers) for call in function_calls] + if parallel_tools and len(function_calls) > 1: # opt-in contract: handlers must be thread-safe + with ThreadPoolExecutor(max_workers=min(8, len(function_calls))) as pool: + next_input = list(pool.map(lambda call: _handle_function_call(call, tool_handlers), function_calls)) + else: + next_input = [_handle_function_call(call, tool_handlers) for call in function_calls] final_instruction = { "role": "user", @@ -581,14 +592,28 @@ def get_response( for x in remove: content = content.replace(x, "") - # Retry on bad links - if attempt < retries and check_links and not check_links_in_string(content): - print("Bad URLs detected, retrying") - continue + # Retry on bad links, feeding the broken URLs back so the model fixes them instead of rolling the dice + if check_links and (bad_urls := check_links_in_string(content, return_bad=True)[1]): + if attempt < retries: + print(f"Bad URLs detected, retrying with feedback: {bad_urls}") + feedback = [ + {"role": "assistant", "content": content}, + { + "role": "user", + "content": "Broken links in your reply: " + ", ".join(bad_urls) + ". " + "Rewrite the full reply, replacing each broken link with a working one or removing it.", + }, + ] + messages = [*messages, *feedback] + user_messages = [*user_messages, *feedback] + continue + content = check_links_in_string(content, replace=True) # final attempt: salvage via redirects/search return content - except (requests.exceptions.ConnectionError, requests.exceptions.Timeout, json.JSONDecodeError) as e: + except (requests.exceptions.ConnectionError, json.JSONDecodeError) as e: + # ConnectTimeout subclasses ConnectionError so it stays retryable; a ReadTimeout propagates instead, + # because the request may have completed server-side and re-POSTing it would double-bill. if attempt < retries: print(f"Retrying {e.__class__.__name__} in {2**attempt}s (attempt {attempt + 1}/{retries + 1})...") time.sleep(2**attempt) diff --git a/tests/test_openai_utils.py b/tests/test_openai_utils.py index d3e633a..691b266 100644 --- a/tests/test_openai_utils.py +++ b/tests/test_openai_utils.py @@ -104,6 +104,21 @@ def test_get_response(mock_post): mock_post.assert_called_once() +@patch("time.sleep") +@patch("requests.post") +def test_get_response_read_timeout_propagates(mock_post, mock_sleep): + """Test a read timeout is NOT retried: the request may have completed server-side and re-POSTing double-bills.""" + mock_post.side_effect = requests.exceptions.ReadTimeout() + + with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"): + try: + get_response([{"role": "user", "content": "Hello"}], check_links=False, retries=2) + raise AssertionError("ReadTimeout should propagate") + except requests.exceptions.ReadTimeout: + pass + assert mock_post.call_count == 1 # no re-POST of a possibly-billed request + + @patch("time.sleep") @patch("requests.post") def test_get_response_retries_rate_limits(mock_post, mock_sleep): @@ -142,7 +157,7 @@ def test_get_response_with_link_check(mock_check_links, mock_post): ] } mock_post.return_value = mock_response - mock_check_links.return_value = True + mock_check_links.return_value = (True, []) messages = [{"role": "user", "content": "Hello"}] @@ -274,7 +289,7 @@ def test_get_agent_response_summarizes_after_max_turns(mock_post): ], "usage": {"input_tokens": 20, "output_tokens": 7}, } - mock_post.side_effect = [tool_response, requests.exceptions.Timeout(), final_response] + mock_post.side_effect = [tool_response, requests.exceptions.ConnectTimeout(), final_response] schema = { "type": "object",