Retry billing fixes, bad-link steering, opt-in parallel tools, current-gen Claude pricing (#802)
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user