diff --git a/README.md b/README.md index 58819fb..d9c21da 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ Choose between [OpenAI](https://developers.openai.com/) or [Anthropic](https://w | Provider | Default Model | API Key | | --------- | ------------------- | ------------------- | -| OpenAI | `gpt-5.5` | `openai_api_key` | +| OpenAI | `gpt-5.6-luna` | `openai_api_key` | | Anthropic | `claude-sonnet-4-6` | `anthropic_api_key` | The model is auto-detected based on which API key you provide. Override with the `model` input, or use `review_model` to override PR review only. @@ -111,7 +111,7 @@ jobs: # AI API keys - provide OpenAI OR Anthropic (model auto-detected from key) openai_api_key: ${{ secrets.OPENAI_API_KEY }} # anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - # model: gpt-5.5 # Optional: set model explicitly + # model: gpt-5.6-luna # Optional: set model explicitly # review_model: claude-opus-4-7 # Optional: override PR review model brave_api_key: ${{ secrets.BRAVE_API_KEY }} # Used for broken link resolution ``` diff --git a/README.zh-CN.md b/README.zh-CN.md index ec85c2b..6ae307c 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -49,7 +49,7 @@ | 提供商 | 默认模型 | API Key | | --------- | ------------------- | ------------------- | -| OpenAI | `gpt-5.5` | `openai_api_key` | +| OpenAI | `gpt-5.6-luna` | `openai_api_key` | | Anthropic | `claude-sonnet-4-6` | `anthropic_api_key` | 模型会根据提供的 API key 自动检测。可通过 `model` 输入覆盖默认模型，也可使用 `review_model` 仅覆盖 PR review 模型。 @@ -111,7 +111,7 @@ jobs: # AI API keys - provide OpenAI OR Anthropic (model auto-detected from key) openai_api_key: ${{ secrets.OPENAI_API_KEY }} # anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - # model: gpt-5.5 # Optional: set model explicitly + # model: gpt-5.6-luna # Optional: set model explicitly # review_model: claude-opus-4-7 # Optional: override PR review model brave_api_key: ${{ secrets.BRAVE_API_KEY }} # Used for broken link resolution ``` diff --git a/action.yml b/action.yml index 2a8a08f..e40cea2 100644 --- a/action.yml +++ b/action.yml @@ -76,10 +76,10 @@ inputs: description: "Anthropic API Key" required: false model: - description: "AI Model - defaults to GPT-5.5 or Sonnet 4.6 based on API key" + description: "AI Model - defaults to GPT-5.6 Luna or Sonnet 4.6 based on API key" required: false review_model: - description: "AI Model for PR reviews (optional, defaults to gpt-5.5)" + description: "AI Model for PR reviews (optional, defaults to gpt-5.6-terra with medium reasoning)" required: false first_issue_response: description: "Example response to a new issue" diff --git a/actions/__init__.py b/actions/__init__.py index 9a1e471..2345c53 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -28,4 +28,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.2.24" +__version__ = "0.2.25" diff --git a/actions/first_interaction.py b/actions/first_interaction.py index 63551fb..4c87104 100644 --- a/actions/first_interaction.py +++ b/actions/first_interaction.py @@ -216,9 +216,13 @@ def main(*args, **kwargs): # Automatic PR review after first interaction if AUTO_PR_REVIEW: print("Starting automatic PR review...") - review_number = review_pr.dismiss_previous_reviews(event) - review_data = review_pr.generate_pr_review(event.repository, diff, title, summary, event) - review_pr.post_review_summary(event, review_data, review_number) + try: + review_diff, head_sha = event.get_pr_diff_snapshot() + except RuntimeError as e: + print(f"Skipping stale PR review: {e}") + return + review_data = review_pr.generate_pr_review(event.repository, review_diff, title, summary, event, head_sha) + review_pr.post_review_summary(event, review_data) print("PR review completed") return diff --git a/actions/review_pr.py b/actions/review_pr.py index 48b77d0..4fe4975 100644 --- a/actions/review_pr.py +++ b/actions/review_pr.py @@ -32,7 +32,7 @@ MAX_REVIEW_COMMENTS = 8 MAX_TOOL_OUTPUT_CHARS = 20000 MAX_TOOL_FILE_LINES = 240 MAX_AGENT_TURNS = 16 -MAX_REVIEW_COST = 2.00 # USD ceiling per review across all agent turns +REVIEW_COST_SOFT_LIMIT = 2.00 # stop requesting tools after cumulative spend reaches this amount SEVERITY_RANK = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3, "SUGGESTION": 4, None: 5} @@ -83,21 +83,6 @@ def search_repo(query: str, path_glob=None) -> str: return _clip_tool_output("\n".join(matches)) if matches else "No matches found." -def _pr_head_sha(event: Action | None) -> str | None: - """Return the live PR head SHA (payload fallback) so reviews read the same code the diff describes.""" - if event is None or not event.pr: - return None - if (number := event.pr.get("number")) and event.repository: - try: - response = event.get(f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{number}") - if response.status_code == 200 and (sha := (response.json().get("head") or {}).get("sha")): - return sha - except Exception as e: - print(f"Live PR head lookup failed: {e}") - sha = (event.pr.get("head") or {}).get("sha") - return sha if isinstance(sha, str) else None - - def _fetch_head_file(event: Action, sha: str, path: str) -> str | None: """Fetch one file's text from the PR head via the GitHub contents API; None only if it does not exist (404).""" url = f"{GITHUB_API_URL}/repos/{event.repository}/contents/{quote(path)}?ref={sha}" @@ -182,7 +167,7 @@ def build_review_agent_tools( if not head_tree: response = event.get(f"{GITHUB_API_URL}/repos/{event.repository}/git/trees/{head_sha}?recursive=1") if response.status_code != 200: - return f"list_files failed: HTTP {response.status_code}." # don't cache failures + raise RuntimeError(f"list_files failed: HTTP {response.status_code}") head_tree.append([t["path"] for t in response.json().get("tree", []) if t.get("type") == "blob"]) files = sorted(p for p in head_tree[0] if (not path_glob or fnmatch(p, path_glob)) and not should_skip_file(p)) return _clip_tool_output("\n".join(files[:300])) if files else "No matching files found." @@ -321,11 +306,7 @@ def get_repo_guidelines( # Prefer CLAUDE.md for Anthropic models, AGENTS.md for others; load only one, never both agent_prefs = ("CLAUDE.md", "AGENTS.md") if "claude" in model.lower() else ("AGENTS.md", "CLAUDE.md") for filename in ("CONTRIBUTING.md", *agent_prefs): - try: - content = (_read_head_file(event, head_sha, local_checkout, filename) or "")[:MAX_CONTEXT_FILE_CHARS] - except Exception as e: - print(f"Failed to read {filename}: {e}") - continue + content = (_read_head_file(event, head_sha, local_checkout, filename) or "")[:MAX_CONTEXT_FILE_CHARS] if content: guidelines.append(f"### {filename}\n~~~\n{content}\n~~~") print(f"Loaded {filename} ({len(content)} chars) for review context") @@ -376,19 +357,28 @@ def parse_diff_files(diff_text: str) -> tuple[dict, str]: else: augmented_lines.append(line) + files = {path: sides for path, sides in files.items() if sides["RIGHT"] or sides["LEFT"]} return files, "\n".join(augmented_lines) def generate_pr_review( - repository: str, diff_text: str, pr_title: str, pr_description: str, event: Action = None + repository: str, + diff_text: str, + pr_title: str, + pr_description: str, + event: Action = None, + head_sha: str | None = None, ) -> dict: """Generate comprehensive PR review with line-specific comments and overall assessment.""" + head_sha = head_sha or (event.get_pr_head_sha() if event else None) + if diff_text.startswith("ERROR:"): + return {"comments": [], "summary": f"{ERROR_MARKER}: {diff_text}", "head_sha": head_sha} if not diff_text: - return {"comments": [], "summary": "No changes detected in diff"} + return {"comments": [], "summary": "No changes detected in diff", "head_sha": head_sha} diff_files, augmented_diff = parse_diff_files(diff_text) if not diff_files: - return {"comments": [], "summary": "No files with changes detected in diff"} + return {"comments": [], "summary": "No reviewable text changes detected in diff", "head_sha": head_sha} # Filter out generated/vendored files filtered_files = {p: s for p, s in diff_files.items() if not should_skip_file(p)} @@ -400,6 +390,7 @@ def generate_pr_review( "comments": [], "summary": f"All {len(skipped_files)} changed files are generated/vendored (skipped review)", "skipped_files": skipped_files, + "head_sha": head_sha, } file_list = list(diff_files.keys()) @@ -408,7 +399,6 @@ def generate_pr_review( # Read model-appropriate guidelines from the PR head for project-specific review context review_model = get_review_model() is_agent_review_model = not _is_anthropic_model(review_model) - head_sha = _pr_head_sha(event) local_checkout = _verified_local_checkout(head_sha) if head_sha: print(f"Reviewing PR head {head_sha[:7]} ({'local checkout' if local_checkout else 'via GitHub API'})") @@ -419,10 +409,7 @@ def generate_pr_review( if event and head_sha and not is_agent_review_model and len(file_list) <= 10: # Reasonable file count limit file_contents, total_chars = [], len(augmented_diff) + len(guidelines_section) for file_path in file_list: # already filtered by should_skip_file above - try: - text = _read_head_file(event, head_sha, local_checkout, file_path) or "" - except Exception: - continue + text = _read_head_file(event, head_sha, local_checkout, file_path) or "" if not text or len(text) > 100_000: # skip missing and >100KB files entirely continue snippet = text[:MAX_CONTEXT_FILE_CHARS] @@ -575,10 +562,11 @@ def generate_pr_review( messages, text_format={"format": {"type": "json_schema", "name": "pr_review", "strict": True, "schema": schema}}, model=review_model, + reasoning_effort="medium", tools=tools, tool_handlers=tool_handlers, max_turns=MAX_AGENT_TURNS, - max_cost=MAX_REVIEW_COST, + max_cost=REVIEW_COST_SOFT_LIMIT, 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 @@ -687,36 +675,7 @@ def generate_pr_review( f"{ERROR_MARKER}: `{type(e).__name__}`\n\n" f"
Debug Info\n\n```\n{error_details}\n```\n
" ) - return {"comments": [], "summary": summary} - - -def dismiss_previous_reviews(event: Action) -> int: - """Dismiss previous bot reviews and delete inline comments, returns count for numbering.""" - if not (pr_number := event.pr.get("number")) or not (bot_username := event.get_username()): - return 1 - - review_count = 0 - reviews_base = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/reviews" - reviews_url = f"{reviews_base}?per_page=100" - if (response := event.get(reviews_url)).status_code == 200: - for review in response.json(): - if review.get("user", {}).get("login") == bot_username and REVIEW_MARKER in (review.get("body") or ""): - review_count += 1 - if review.get("state") in ["APPROVED", "CHANGES_REQUESTED"] and (review_id := review.get("id")): - event.put(f"{reviews_base}/{review_id}/dismissals", json={"message": "Superseded by new review"}) - - # Delete previous inline comments - comments_url = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/comments?per_page=100" - delete_base = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/comments" - if (response := event.get(comments_url)).status_code == 200: - for comment in response.json(): - if comment.get("user", {}).get("login") == bot_username and (comment_id := comment.get("id")): - event.delete( - f"{delete_base}/{comment_id}", - expected_status=[200, 204, 404], - ) - - return review_count + 1 + return {"comments": [], "summary": summary, "head_sha": head_sha} def get_local_head_sha() -> str | None: @@ -744,27 +703,55 @@ def _verified_local_checkout(head_sha: str | None) -> bool: return False -def post_review_summary(event: Action, review_data: dict, review_number: int) -> None: +def clear_previous_review(event: Action) -> None: + """Dismiss the bot's active review decisions and delete its superseded inline comments.""" + pr_number, bot_username = event.pr.get("number"), event.get_username() + reviews_base = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/reviews" + reviews = event.get(reviews_base, params={"per_page": 100}, hard=True).json() + owned_reviews = { + review["id"] + for review in reviews + if review.get("user", {}).get("login") == bot_username and REVIEW_MARKER in (review.get("body") or "") + } + for review in reviews: + if review.get("id") in owned_reviews and review.get("state") in ("APPROVED", "CHANGES_REQUESTED"): + event.put( + f"{reviews_base}/{review['id']}/dismissals", + json={"message": "Superseded by new review"}, + hard=True, + ) + + comments_base = f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/comments" + comments = event.get(comments_base, params={"per_page": 100}, hard=True).json() + for comment in comments: + if comment.get("pull_request_review_id") in owned_reviews: + event.delete(f"{GITHUB_API_URL}/repos/{event.repository}/pulls/comments/{comment['id']}", hard=True) + + +def post_review_summary(event: Action, review_data: dict) -> None: """Post overall review summary and inline comments as a single PR review.""" if not (pr_number := event.pr.get("number")): return - # Anchor to the exact head the review was generated from; fall back to the local checkout, then live head - commit_sha = review_data.get("head_sha") or get_local_head_sha() or _pr_head_sha(event) - if not commit_sha: - return + commit_sha = review_data["head_sha"] + if event.get_pr_head_sha() != commit_sha: + raise RuntimeError("PR head changed during review generation") - review_title = f"{REVIEW_MARKER} {review_number}" if review_number > 1 else REVIEW_MARKER comments = review_data.get("comments", []) summary = review_data.get("summary") or "" # Don't approve if error occurred, inline comments exist, or medium-or-higher severity issues has_error = not summary or ERROR_MARKER in summary + has_evidence = bool(review_data.get("diff_files")) has_inline_comments = review_data.get("comments_before_filtering", 0) > 0 has_issues = any(c.get("severity") not in ["LOW", "SUGGESTION", None] for c in comments) - event_type = "COMMENT" if (has_error or has_inline_comments or has_issues) else "APPROVE" + event_type = ( + "COMMENT" + if (has_error or not has_evidence or has_inline_comments or has_issues or review_data.get("diff_truncated")) + else "APPROVE" + ) - body = f"{review_title}\n\n{ACTIONS_CREDIT}\n\n{summary[:3000]}\n\n" + body = f"{REVIEW_MARKER}\n\n{ACTIONS_CREDIT}\n\n{summary[:3000]}\n\n" if comments: body += f"💬 Posted {len(comments)} inline comment{'s' if len(comments) != 1 else ''}\n" @@ -806,7 +793,7 @@ def post_review_summary(event: Action, review_data: dict, review_number: int) -> if review_comments: payload["comments"] = review_comments - event.post(f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/reviews", json=payload) + event.post(f"{GITHUB_API_URL}/repos/{event.repository}/pulls/{pr_number}/reviews", json=payload, hard=True) def main(*args, **kwargs): @@ -828,12 +815,16 @@ def main(*args, **kwargs): return print(f"Starting PR review for #{event.pr['number']}") - review_number = dismiss_previous_reviews(event) - - diff = event.get_pr_diff() - review = generate_pr_review(event.repository, diff, event.pr.get("title") or "", event.pr.get("body") or "", event) - - post_review_summary(event, review, review_number) + try: + diff, head_sha = event.get_pr_diff_snapshot() + except RuntimeError as e: + print(f"Skipping stale PR review: {e}") + return + clear_previous_review(event) + review = generate_pr_review( + event.repository, diff, event.pr.get("title") or "", event.pr.get("body") or "", event, head_sha + ) + post_review_summary(event, review) print("PR review completed") diff --git a/actions/utils/github_utils.py b/actions/utils/github_utils.py index 63437e4..c72b261 100644 --- a/actions/utils/github_utils.py +++ b/actions/utils/github_utils.py @@ -236,9 +236,9 @@ class Action: return True return False - def get_pr_diff(self) -> str: + def get_pr_diff(self, refresh: bool = False) -> str: """Retrieves the diff content for a specified pull request with caching.""" - if self._pr_diff_cache: + if self._pr_diff_cache and not refresh: return self._pr_diff_cache url = f"{GITHUB_API_URL}/repos/{self.repository}/pulls/{self.pr.get('number')}" @@ -251,6 +251,26 @@ class Action: self._pr_diff_cache = "ERROR: UNABLE TO RETRIEVE DIFF." return self._pr_diff_cache + def get_pr_head_sha(self) -> str | None: + """Return the live PR head SHA.""" + if not self.pr: + return None + response = self.get(f"{GITHUB_API_URL}/repos/{self.repository}/pulls/{self.pr.get('number')}") + if response.status_code == 200 and (sha := (response.json().get("head") or {}).get("sha")): + return sha + return None + + def get_pr_diff_snapshot(self) -> tuple[str, str]: + """Fetch a diff whose PR head stayed unchanged for the full request.""" + for _ in range(2): + head_before = self.get_pr_head_sha() + diff = self.get_pr_diff(refresh=True) + head_after = self.get_pr_head_sha() + if head_before and head_before == head_after: + return diff, head_after + print(f"PR head moved while fetching diff ({head_before} -> {head_after}); retrying") + raise RuntimeError("PR head changed repeatedly while fetching the review diff") + def get_repo_data(self, endpoint: str) -> dict: """Fetches repository data from a specified endpoint.""" return self.get(f"{GITHUB_API_URL}/repos/{self.repository}/{endpoint}").json() diff --git a/actions/utils/openai_utils.py b/actions/utils/openai_utils.py index ac5f260..3c65fb1 100644 --- a/actions/utils/openai_utils.py +++ b/actions/utils/openai_utils.py @@ -18,11 +18,12 @@ ANTHROPIC_API_KEY = os.getenv("ANTHROPIC_API_KEY") MODEL = os.getenv("MODEL") # Auto-detected from API keys if not set REVIEW_MODEL = os.getenv("REVIEW_MODEL") # Optional override for PR reviews MAX_PROMPT_CHARS = round(128000 * 3.3 * 0.5) # deliberate COST ceiling, not a context limit; agent tools read the rest +WEB_SEARCH_CALL_COST = 0.01 # $10 per 1K calls # Default models (single source of truth) -OPENAI_MODEL_DEFAULT = "gpt-5.5" +OPENAI_MODEL_DEFAULT = "gpt-5.6-luna" ANTHROPIC_MODEL_DEFAULT = "claude-sonnet-4-6" -PR_REVIEW_MODEL_DEFAULT = "gpt-5.5" +PR_REVIEW_MODEL_DEFAULT = "gpt-5.6-terra" MODEL_COSTS = { # (input, output) per 1M tokens # OpenAI models @@ -34,6 +35,9 @@ MODEL_COSTS = { # (input, output) per 1M tokens "gpt-5.3-codex": (1.75, 14.00), "gpt-5.5": (5.00, 30.00), "gpt-5.4": (2.50, 15.00), + "gpt-5.6-sol": (5.00, 30.00), + "gpt-5.6-terra": (2.50, 15.00), + "gpt-5.6-luna": (1.00, 6.00), "gpt-5-nano-2025-08-07": (0.05, 0.40), "gpt-5-mini-2025-08-07": (0.25, 2.00), # Anthropic Claude models @@ -222,7 +226,7 @@ def _add_openai_usage(total_usage: dict | None, response_json: dict) -> dict | N total_usage = total_usage or { "input_tokens": 0, "output_tokens": 0, - "input_tokens_details": {"cached_tokens": 0}, + "input_tokens_details": {"cache_write_tokens": 0, "cached_tokens": 0}, "output_tokens_details": {"reasoning_tokens": 0}, } total_usage["input_tokens"] += usage.get("input_tokens", 0) @@ -230,29 +234,40 @@ def _add_openai_usage(total_usage: dict | None, response_json: dict) -> dict | N total_usage["input_tokens_details"]["cached_tokens"] += (usage.get("input_tokens_details") or {}).get( "cached_tokens", 0 ) + total_usage["input_tokens_details"]["cache_write_tokens"] += (usage.get("input_tokens_details") or {}).get( + "cache_write_tokens", 0 + ) total_usage["output_tokens_details"]["reasoning_tokens"] += (usage.get("output_tokens_details") or {}).get( "reasoning_tokens", 0 ) return total_usage -def _normalize_usage_tokens(usage: dict) -> tuple[int, int]: - """Return (input_tokens, cached_tokens) for OpenAI Responses or Anthropic Messages usage shapes. +def _normalize_usage_tokens(usage: dict) -> tuple[int, int, int]: + """Return input, cache-read, and cache-write tokens for OpenAI Responses or Anthropic Messages usage shapes. Anthropic reports cache reads/writes outside input_tokens, so both fold back into the input total and reads count as cached — the same normalization ultralytics/assistant applies, keeping cross-repo telemetry identical. """ cache_read = usage.get("cache_read_input_tokens", 0) input_tokens = usage.get("input_tokens", 0) + cache_read + usage.get("cache_creation_input_tokens", 0) - cached_tokens = (usage.get("input_tokens_details") or {}).get("cached_tokens", 0) or cache_read - return input_tokens, cached_tokens + details = usage.get("input_tokens_details") or {} + cached_tokens = details.get("cached_tokens", 0) or cache_read + cache_write_tokens = details.get("cache_write_tokens", 0) + return input_tokens, cached_tokens, cache_write_tokens def _openai_usage_cost(usage: dict, model: str) -> float: - """Compute billed USD cost for one usage block (cached input billed at 10% of the input rate).""" + """Compute billed USD cost including GPT-5.6 cache-write and long-context rates.""" costs = MODEL_COSTS.get(model, (0.0, 0.0)) - input_tokens, cached_tokens = _normalize_usage_tokens(usage) - return ((input_tokens - cached_tokens * 0.9) * costs[0] + usage.get("output_tokens", 0) * costs[1]) / 1e6 + input_tokens, cached_tokens, cache_write_tokens = _normalize_usage_tokens(usage) + cache_write_premium = cache_write_tokens * 0.25 if model.startswith("gpt-5.6-") else 0 + billed_input = input_tokens - cached_tokens * 0.9 + cache_write_premium + long_context = model.startswith("gpt-5.6-") and input_tokens > 272000 + return ( + billed_input * costs[0] * (2 if long_context else 1) + + usage.get("output_tokens", 0) * costs[1] * (1.5 if long_context else 1) + ) / 1e6 def _format_tool_calls(calls: list[str]) -> str: @@ -264,13 +279,15 @@ def _format_tool_calls(calls: list[str]) -> str: return f"{len(calls)} tools" + (f" ({types})" if calls else "") -def _print_openai_usage(response_json: dict, model: str, elapsed: float, metadata: str = "") -> None: +def _print_openai_usage( + response_json: dict, model: str, elapsed: float, metadata: str = "", billed_cost: float | None = None +) -> None: """Print token/cost telemetry: 'model: 136036→289 tokens (72% cached, 31 thinking), $0.69, 8.9s'.""" if usage := response_json.get("usage"): - input_tokens, cached_tokens = _normalize_usage_tokens(usage) + input_tokens, cached_tokens, _ = _normalize_usage_tokens(usage) output_tokens = usage.get("output_tokens", 0) # includes thinking, noted in the parenthetical thinking_tokens = (usage.get("output_tokens_details") or {}).get("reasoning_tokens", 0) - cost = _openai_usage_cost(usage, model) + cost = _openai_usage_cost(usage, model) if billed_cost is None else billed_cost notes = [] if cached_tokens: notes.append(f"{round(100 * cached_tokens / input_tokens)}% cached") @@ -306,7 +323,10 @@ def _post_openai_response( r.reason = f"{r.reason}\n{error_body}" r.raise_for_status() - return r.json(), elapsed + response_json = r.json() + return ( + _poll_openai_response(response_json, headers) if response_json.get("status") else response_json + ), elapsed 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. @@ -340,14 +360,11 @@ def _handle_function_call(call: dict, tool_handlers: dict[str, Callable]) -> dic """Execute one model-requested function call and return a Responses API output item.""" name = call.get("name") call_id = call.get("call_id") - try: - if name not in tool_handlers: - raise KeyError(f"Unknown tool: {name}") - output = tool_handlers[name](**_parse_tool_arguments(call)) - if not isinstance(output, str): - output = json.dumps(output) - except Exception as e: - output = f"{name or 'tool'} failed: {type(e).__name__}: {e}" + if name not in tool_handlers: + raise KeyError(f"Unknown tool: {name}") + output = tool_handlers[name](**_parse_tool_arguments(call)) + if not isinstance(output, str): + output = json.dumps(output) return {"type": "function_call_output", "call_id": call_id, "output": output} @@ -367,9 +384,9 @@ def get_agent_response( ) -> str | dict: """Run an iterative OpenAI Responses API agent with application-managed function tools. - 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. + max_cost is a USD ceiling across all turns (0 disables); a tool request after reaching it aborts the incomplete + agent run. 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: @@ -396,6 +413,7 @@ def get_agent_response( base_data = { "model": model, + "service_tier": "default", "store": True, "temperature": temperature, "tools": tools, @@ -409,6 +427,7 @@ def get_agent_response( tool_calls = [] total_elapsed = 0.0 + total_cost = 0.0 total_usage = None previous_response_id = None next_input = conversation @@ -423,9 +442,18 @@ def get_agent_response( previous_response_id = response_json.get("id") output_items = response_json.get("output", []) turn_calls = _response_tool_calls(output_items) + turn_cost = ( + _openai_usage_cost(response_json.get("usage") or {}, model) + + turn_calls.count("web_search") * WEB_SEARCH_CALL_COST + ) + total_cost += turn_cost tool_calls += turn_calls _print_openai_usage( - response_json, model, elapsed, f"turn {turn + 1}/{max_turns}, {_format_tool_calls(turn_calls)}" + response_json, + model, + elapsed, + f"turn {turn + 1}/{max_turns}, {_format_tool_calls(turn_calls)}", + turn_cost, ) function_calls = [item for item in output_items if item.get("type") == "function_call"] @@ -435,18 +463,14 @@ def get_agent_response( model, total_elapsed, f"agent total, {turn + 1} turns, {_format_tool_calls(tool_calls)}", + total_cost, ) return _finalize_response_content(response_json, text_format) if not previous_response_id: raise RuntimeError("OpenAI response did not include an id for server-managed continuation") - if max_cost and _openai_usage_cost(total_usage, model) >= max_cost: - print(f"Agent cost budget ${max_cost:.2f} reached; skipping remaining tool turns") - next_input = [ # pending calls still need outputs for the chained synthesis request to be valid - {"type": "function_call_output", "call_id": call.get("call_id"), "output": "Tool budget exhausted."} - for call in function_calls - ] - break + if max_cost and total_cost >= max_cost: + raise RuntimeError(f"Agent cost budget ${max_cost:.2f} reached before requested tools could run") 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)) @@ -467,9 +491,14 @@ def get_agent_response( response_json, elapsed = _post_openai_response(data, headers, max(retries, 2), request_timeout) total_elapsed += elapsed total_usage = _add_openai_usage(total_usage, response_json) + total_cost += _openai_usage_cost(response_json.get("usage") or {}, model) _print_openai_usage(response_json, model, elapsed, f"turn final/{max_turns}, 0 tools") _print_openai_usage( - {"usage": total_usage}, model, total_elapsed, f"agent total, {turn + 2} turns, {_format_tool_calls(tool_calls)}" + {"usage": total_usage}, + model, + total_elapsed, + f"agent total, {turn + 2} turns, {_format_tool_calls(tool_calls)}", + total_cost, ) return _finalize_response_content(response_json, text_format) @@ -566,7 +595,7 @@ def get_response( # Parse response response_json = r.json() - if background: + if background or (not is_anthropic and response_json.get("status")): response_json = _poll_openai_response(response_json, headers) elapsed = time.time() - started if is_anthropic: diff --git a/cla/action.yml b/cla/action.yml index fba88f1..dd44dfe 100644 --- a/cla/action.yml +++ b/cla/action.yml @@ -16,11 +16,17 @@ inputs: runs: using: "composite" steps: + - uses: astral-sh/setup-uv@v7 + with: + ignore-empty-workdir: true + enable-cache: false + version: "0.9.4" + - name: Install requests run: | cd "$GITHUB_ACTION_PATH/.." - python -m venv "$RUNNER_TEMP/ultralytics-cla" - "$RUNNER_TEMP/ultralytics-cla/bin/python" -m pip install --disable-pip-version-check \ + uv venv "$RUNNER_TEMP/ultralytics-cla" + uv pip install --python "$RUNNER_TEMP/ultralytics-cla/bin/python" \ "requests==2.32.4; python_version < '3.10'" \ "requests==2.33.0; python_version >= '3.10'" shell: bash diff --git a/tests/test_first_interaction.py b/tests/test_first_interaction.py index 8a46390..4830637 100644 --- a/tests/test_first_interaction.py +++ b/tests/test_first_interaction.py @@ -1,6 +1,6 @@ # Ultralytics 🚀 AGPL-3.0 License - https://ultralytics.com/license -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, call, patch import pytest @@ -137,7 +137,8 @@ def test_generate_pr_review_uses_synchronous_response(mock_get_agent_response, m "search_repo", } assert kwargs["max_turns"] == review_pr.MAX_AGENT_TURNS - assert kwargs["max_cost"] == review_pr.MAX_REVIEW_COST + assert kwargs["max_cost"] == review_pr.REVIEW_COST_SOFT_LIMIT + assert kwargs["reasoning_effort"] == "medium" assert kwargs["request_timeout"] == (30, 120) assert "FULL FILE CONTENTS" not in mock_get_agent_response.call_args.args[0][1]["content"] @@ -177,17 +178,103 @@ def test_review_agent_search_scans_local_checkout_only(tmp_path, monkeypatch): def test_pr_head_sha_prefers_live_value(): - """Test head SHA resolution prefers the live PR value and falls back to the event payload.""" + """Test head SHA resolution requires the live PR value.""" event = MagicMock() event.repository = "org/repo" event.pr = {"number": 5, "head": {"sha": "old"}} live = MagicMock(status_code=200) live.json.return_value = {"head": {"sha": "new"}} event.get.return_value = live - assert review_pr._pr_head_sha(event) == "new" + assert review_pr.Action.get_pr_head_sha(event) == "new" event.get.return_value = MagicMock(status_code=500) - assert review_pr._pr_head_sha(event) == "old" - assert review_pr._pr_head_sha(None) is None + assert review_pr.Action.get_pr_head_sha(event) is None + + +def test_review_snapshot_retries_until_diff_and_head_match(): + """Test review snapshots refetch the diff when a push races the first request.""" + event = MagicMock() + event.get_pr_head_sha.side_effect = ["old", "new", "new", "new"] + event.get_pr_diff.side_effect = ["old diff", "new diff"] + + assert review_pr.Action.get_pr_diff_snapshot(event) == ("new diff", "new") + assert event.get_pr_diff.call_args_list == [call(refresh=True), call(refresh=True)] + assert event.get_pr_head_sha.call_count == 4 + + +def test_post_review_summary_fails_when_github_rejects_review(): + """Test review publication is a required operation rather than a silent best effort.""" + event = MagicMock() + event.repository = "org/repo" + event.pr = {"number": 7} + event.get_pr_head_sha.return_value = "abc" + + review_pr.post_review_summary(event, {"head_sha": "abc", "summary": "LGTM", "comments": []}) + + assert event.post.call_args.kwargs["hard"] is True + + +def test_clear_previous_review_preserves_summaries_and_deletes_inline_comments(): + """Test replacement reviews invalidate bot decisions and remove only bot inline comments.""" + event = MagicMock() + event.repository = "org/repo" + event.pr = {"number": 7} + event.get_username.return_value = "review-bot" + event.get.side_effect = [ + MagicMock( + json=lambda: [ + {"id": 1, "state": "APPROVED", "body": review_pr.REVIEW_MARKER, "user": {"login": "review-bot"}}, + {"id": 2, "state": "COMMENTED", "body": review_pr.REVIEW_MARKER, "user": {"login": "review-bot"}}, + {"id": 3, "state": "APPROVED", "body": "Human review", "user": {"login": "human"}}, + {"id": 6, "state": "COMMENTED", "body": "Other automation", "user": {"login": "review-bot"}}, + ] + ), + MagicMock( + json=lambda: [ + {"id": 4, "pull_request_review_id": 1, "user": {"login": "review-bot"}}, + {"id": 5, "pull_request_review_id": 6, "user": {"login": "review-bot"}}, + ] + ), + ] + + review_pr.clear_previous_review(event) + + event.put.assert_called_once_with( + "https://api.github.com/repos/org/repo/pulls/7/reviews/1/dismissals", + json={"message": "Superseded by new review"}, + hard=True, + ) + event.delete.assert_called_once_with( + "https://api.github.com/repos/org/repo/pulls/comments/4", + hard=True, + ) + + +def test_incomplete_review_evidence_cannot_approve(): + """Test unavailable or truncated diffs produce comments rather than approvals.""" + event = MagicMock() + event.repository = "org/repo" + event.pr = {"number": 7} + event.get_pr_head_sha.return_value = "abc" + + error = review_pr.generate_pr_review("org/repo", "ERROR: UNABLE TO RETRIEVE DIFF.", "PR", "", event, "abc") + review_pr.post_review_summary(event, error) + assert event.post.call_args.kwargs["json"]["event"] == "COMMENT" + + review_pr.post_review_summary(event, {"head_sha": "abc", "summary": "LGTM", "comments": [], "diff_truncated": True}) + assert event.post.call_args.kwargs["json"]["event"] == "COMMENT" + + binary = "diff --git a/image.png b/image.png\nBinary files a/image.png and b/image.png differ" + binary_review = review_pr.generate_pr_review("org/repo", binary, "PR", "", event, "abc") + review_pr.post_review_summary(event, binary_review) + assert event.post.call_args.kwargs["json"]["event"] == "COMMENT" + + with pytest.raises(KeyError): + review_pr.post_review_summary(event, {"summary": "stale result", "comments": []}) + + review_pr.post_review_summary( + event, {"head_sha": "abc", "summary": "All changed files were skipped", "comments": []} + ) + assert event.post.call_args.kwargs["json"]["event"] == "COMMENT" def test_review_agent_tools_read_pr_head_via_api(tmp_path, monkeypatch): @@ -223,7 +310,8 @@ def test_review_agent_tools_read_pr_head_via_api(tmp_path, monkeypatch): handlers["read_file"](path="flaky.py", start_line=None, end_line=None) tree_response.status_code = 500 - assert handlers["list_files"](path_glob=None) == "list_files failed: HTTP 500." # failures are not cached + with pytest.raises(RuntimeError, match="list_files failed: HTTP 500"): + handlers["list_files"](path_glob=None) tree_response.status_code = 200 assert handlers["list_files"](path_glob=None).splitlines() == ["tests/test_python.py"] @@ -232,10 +320,10 @@ def test_review_agent_tools_read_pr_head_via_api(tmp_path, monkeypatch): def test_get_repo_guidelines_fetches_pr_head(mock_fetch): """Test guidelines are fetched from the PR head via the GitHub API.""" mock_fetch.side_effect = lambda event, sha, path: "Be nice" if path == "AGENTS.md" else None - section = review_pr.get_repo_guidelines("gpt-5.5", event=MagicMock(), head_sha="abc") + section = review_pr.get_repo_guidelines("gpt-5.6-sol", event=MagicMock(), head_sha="abc") assert "AGENTS.md" in section and "Be nice" in section assert "CONTRIBUTING.md" not in section - assert review_pr.get_repo_guidelines("gpt-5.5", event=None, head_sha="abc") == "" + assert review_pr.get_repo_guidelines("gpt-5.6-sol", event=None, head_sha="abc") == "" def test_review_agent_tools_can_list_and_read_changed_file_diffs(): diff --git a/tests/test_openai_utils.py b/tests/test_openai_utils.py index 691b266..ec481a7 100644 --- a/tests/test_openai_utils.py +++ b/tests/test_openai_utils.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock, patch +import pytest import requests from actions.utils.openai_utils import ( @@ -9,6 +10,7 @@ from actions.utils.openai_utils import ( OPENAI_MODEL_DEFAULT, PR_REVIEW_MODEL_DEFAULT, _is_anthropic_model, + _openai_usage_cost, _response_tool_calls, get_agent_response, get_response, @@ -19,10 +21,32 @@ from actions.utils.openai_utils import ( def test_default_models(): """Test canonical default models are priced so max_cost budgets stay enforceable.""" - assert OPENAI_MODEL_DEFAULT == "gpt-5.5" - assert PR_REVIEW_MODEL_DEFAULT == "gpt-5.5" + assert OPENAI_MODEL_DEFAULT == "gpt-5.6-luna" + assert PR_REVIEW_MODEL_DEFAULT == "gpt-5.6-terra" assert OPENAI_MODEL_DEFAULT in MODEL_COSTS # unpriced models disable max_cost budgets assert PR_REVIEW_MODEL_DEFAULT in MODEL_COSTS + assert MODEL_COSTS["gpt-5.6-sol"] == (5.00, 30.00) + assert MODEL_COSTS["gpt-5.6-terra"] == (2.50, 15.00) + assert MODEL_COSTS["gpt-5.6-luna"] == (1.00, 6.00) + + +def test_gpt_56_cost_includes_cache_write_and_long_context_rates(): + """GPT-5.6 cache writes bill at 125%, with long requests at 2x input and 1.5x output.""" + usage = { + "input_tokens": 1000, + "input_tokens_details": {"cached_tokens": 200, "cache_write_tokens": 300}, + "output_tokens": 100, + } + expected = ((1000 - 200 * 0.9 + 300 * 0.25) * 5.00 + 100 * 30.00) / 1e6 + assert _openai_usage_cost(usage, "gpt-5.6-sol") == expected + usage["input_tokens"] = 272001 + expected = ((272001 - 200 * 0.9 + 300 * 0.25) * 5.00 * 2 + 100 * 30.00 * 1.5) / 1e6 + assert _openai_usage_cost(usage, "gpt-5.6-sol") == expected + turns = [{"input_tokens": 150000, "output_tokens": 0}] * 2 + assert sum(_openai_usage_cost(turn, "gpt-5.6-luna") for turn in turns) == 0.3 + assert _openai_usage_cost({"input_tokens": 300000, "output_tokens": 0}, "gpt-5.6-luna") == 0.6 + old_model_expected = ((1000 - 200 * 0.9) * 5.00 + 100 * 30.00) / 1e6 + assert _openai_usage_cost({**usage, "input_tokens": 1000}, "gpt-5.5") == old_model_expected def test_is_anthropic_model(): @@ -30,7 +54,7 @@ def test_is_anthropic_model(): assert _is_anthropic_model("claude-sonnet-4-6") is True assert _is_anthropic_model("claude-haiku-4-5-20251001") is True assert _is_anthropic_model("claude-opus-4-7") is True - assert _is_anthropic_model("gpt-5.5") is False + assert _is_anthropic_model("gpt-5.6-terra") is False assert _is_anthropic_model("gpt-5-mini-2025-08-07") is False @@ -65,7 +89,7 @@ def test_remove_outer_codeblocks(): def test_get_review_model_override(): """Test review model override logic.""" with patch("actions.utils.openai_utils.REVIEW_MODEL", "claude-opus-4-7"): - with patch("actions.utils.openai_utils.MODEL", "gpt-5.5"): + with patch("actions.utils.openai_utils.MODEL", "gpt-5.6-terra"): assert get_review_model() == "claude-opus-4-7" @@ -184,9 +208,14 @@ def test_get_agent_response_calls_function_tools(mock_post): "call_id": "call_123", "name": "lookup_value", "arguments": '{"value": "abc"}', - } + }, + {"type": "web_search_call"}, ], - "usage": {"input_tokens": 10, "input_tokens_details": {"cached_tokens": 4}, "output_tokens": 5}, + "usage": { + "input_tokens": 10, + "input_tokens_details": {"cached_tokens": 4, "cache_write_tokens": 3}, + "output_tokens": 5, + }, } second_response = MagicMock() second_response.status_code = 200 @@ -199,7 +228,11 @@ def test_get_agent_response_calls_function_tools(mock_post): "content": [{"type": "output_text", "text": '{"comments": [], "summary": "done"}'}], } ], - "usage": {"input_tokens": 20, "input_tokens_details": {"cached_tokens": 8}, "output_tokens": 7}, + "usage": { + "input_tokens": 20, + "input_tokens_details": {"cached_tokens": 8, "cache_write_tokens": 6}, + "output_tokens": 7, + }, } mock_post.side_effect = [first_response, second_response] @@ -238,6 +271,8 @@ def test_get_agent_response_calls_function_tools(mock_post): assert mock_post.call_count == 2 first_payload = mock_post.call_args_list[0].kwargs["json"] assert first_payload["store"] is True + assert first_payload["service_tier"] == "default" + assert first_payload["reasoning"] == {"effort": "low"} assert "include" not in first_payload assert "previous_response_id" not in first_payload assert first_payload["input"] == [{"role": "user", "content": "review"}] @@ -251,10 +286,10 @@ def test_get_agent_response_calls_function_tools(mock_post): } ] printed = "\n".join(str(c.args[0]) for c in mock_print.call_args_list if c.args) - assert "turn 1/6, 1 tools (lookup_value)" in printed + assert "turn 1/6, 2 tools (lookup_value, web_search)" in printed assert "turn 2/6, 0 tools" in printed - assert "30→12 tokens (40% cached), $0.00046" in printed - assert "agent total, 2 turns, 1 tools (lookup_value)" in printed + assert "30→12 tokens (40% cached), $0.01" in printed + assert "agent total, 2 turns, 2 tools (lookup_value, web_search)" in printed assert "Agent tool turn" not in printed # tool names live in the per-turn usage line now @@ -375,7 +410,7 @@ def test_get_response_anthropic(mock_post): @patch("requests.post") def test_get_agent_response_stops_at_cost_budget(mock_post): - """Test the cost budget skips remaining tool turns and forces final synthesis with stub tool outputs.""" + """Test the cost budget aborts rather than synthesizing from incomplete tool evidence.""" tool_response = MagicMock() tool_response.status_code = 200 tool_response.elapsed.total_seconds.return_value = 1.0 @@ -389,22 +424,9 @@ def test_get_agent_response_stops_at_cost_budget(mock_post): "arguments": '{"value": "abc"}', } ], - "usage": {"input_tokens": 1_000_000, "output_tokens": 0}, # $5.00 for gpt-5.5, over any small budget + "usage": {"input_tokens": 1_000_000, "output_tokens": 0}, # $5.00 for gpt-5.6-sol, over any small budget } - final_response = MagicMock() - final_response.status_code = 200 - final_response.elapsed.total_seconds.return_value = 1.0 - final_response.json.return_value = { - "id": "resp_final", - "output": [ - { - "type": "message", - "content": [{"type": "output_text", "text": '{"comments": [], "summary": "budget"}'}], - } - ], - "usage": {"input_tokens": 20, "output_tokens": 7}, - } - mock_post.side_effect = [tool_response, final_response] + mock_post.return_value = tool_response schema = { "type": "object", @@ -430,26 +452,73 @@ def test_get_agent_response_stops_at_cost_budget(mock_post): def forbidden_handler(value): raise AssertionError("tool handlers must not run once the cost budget is reached") - with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"): - result = get_agent_response( + with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"), pytest.raises( + RuntimeError, match=r"cost budget.*reached" + ): + get_agent_response( [{"role": "user", "content": "review"}], tools=tools, tool_handlers={"lookup_value": forbidden_handler}, text_format={"format": {"type": "json_schema", "name": "review", "strict": True, "schema": schema}}, - model="gpt-5.5", + model="gpt-5.6-sol", max_turns=8, max_cost=1.00, retries=0, ) - assert result == {"comments": [], "summary": "budget"} - assert mock_post.call_count == 2 # budget reached on turn 1, remaining turns skipped - final_payload = mock_post.call_args_list[1].kwargs["json"] - assert final_payload["tool_choice"] == "none" - assert final_payload["previous_response_id"] == "resp_tool" - assert final_payload["input"][0] == { - "type": "function_call_output", - "call_id": "call_123", - "output": "Tool budget exhausted.", + mock_post.assert_called_once() + + +@patch("requests.post") +def test_get_agent_response_rejects_incomplete_response(mock_post): + """Test terminal incomplete responses cannot become review evidence.""" + response = MagicMock(status_code=200) + response.elapsed.total_seconds.return_value = 1.0 + response.json.return_value = {"id": "resp_incomplete", "status": "incomplete", "incomplete_details": "limit"} + mock_post.return_value = response + + with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"), pytest.raises( + RuntimeError, match="ended with limit" + ): + get_agent_response([{"role": "user", "content": "review"}], tools=[], tool_handlers={}, retries=0) + + +@patch("requests.post") +def test_get_response_rejects_incomplete_response(mock_post): + """Test synchronous Responses completions reject terminal incomplete output.""" + response = MagicMock(status_code=200) + response.elapsed.total_seconds.return_value = 1.0 + response.json.return_value = {"id": "resp_incomplete", "status": "incomplete", "incomplete_details": "limit"} + mock_post.return_value = response + + with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"), pytest.raises( + RuntimeError, match="ended with limit" + ): + get_response([{"role": "user", "content": "summary"}], check_links=False, retries=0) + + +@patch("requests.post") +def test_get_agent_response_rejects_failed_tool(mock_post): + """Test failed evidence tools abort the agent run.""" + response = MagicMock(status_code=200) + response.elapsed.total_seconds.return_value = 1.0 + response.json.return_value = { + "id": "resp_tool", + "status": "completed", + "output": [{"type": "function_call", "call_id": "call_123", "name": "read_file", "arguments": "{}"}], + "usage": {"input_tokens": 10, "output_tokens": 5}, } - assert "Synthesize the gathered tool results" in final_payload["input"][-1]["content"] + mock_post.return_value = response + + def failed_read(): + raise RuntimeError("read failed") + + with patch("actions.utils.openai_utils.OPENAI_API_KEY", "test-key"), pytest.raises( + RuntimeError, match="read failed" + ): + get_agent_response( + [{"role": "user", "content": "review"}], + tools=[], + tool_handlers={"read_file": failed_read}, + retries=0, + )