Skip to content

ci(review): route reviews to GPT-5.6 tiers #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
luckyPipewrench merged 1 commit into from
Aug 2, 2026
Merged

ci(review): route reviews to GPT-5.6 tiers #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .github/workflows/pr-review.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ permissions:
contents: read
pull-requests: write

concurrency:
group: pr-review-${{ github.repository }}-${{ github.event.issue.number }}
cancel-in-progress: true

jobs:
review:
# Restrict to the repository owner account. The review runner is checked
Expand Down Expand Up @@ -44,6 +48,9 @@ jobs:
- name: Install dependencies
run: pip install --require-hashes -r .github/requirements-pr-review.txt

- name: Test trusted review runner
run: python -m unittest tests/test_pr_review_routing.py

- name: Determine review mode
id: mode
env:
Expand All @@ -59,10 +66,15 @@ jobs:
- name: Run PR review
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }}
LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }}
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
REVIEW_MODE: ${{ steps.mode.outputs.mode }}
PR_NUMBER: ${{ github.event.issue.number }}
REPO: ${{ github.repository }}
PR_REVIEW_MODEL_FAST: gpt-5.4-mini
PR_REVIEW_MODEL_DEEP: gpt-5.5
# Empty/unset repository variables deliberately fall back to the
# Python defaults in scripts/pr-review.py. Keep defaults there so
# manual and workflow runs cannot drift.
PR_REVIEW_MODEL_FAST: ${{ vars.PR_REVIEW_MODEL_FAST }}
PR_REVIEW_MODEL_DEEP: ${{ vars.PR_REVIEW_MODEL_DEEP }}
run: python scripts/pr-review.py
61 changes: 34 additions & 27 deletions scripts/pr-review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"""AI-powered PR review for the pipelock-verify Python package.

Triggered by /review comments on PRs. Supports multiple review modes:
/review - Security and correctness review (smaller model, default)
/review deep - Deeper review (larger model)
/review - Security and correctness review (default model)
/review deep - Deeper review (higher-capacity model)
/review tests - Test coverage and boundary analysis
/review docs - Documentation accuracy check

Expand All @@ -21,12 +21,14 @@
OPENAI_API_KEY - Direct OpenAI API

Model selection:
PR_REVIEW_MODEL_FAST - Model for default/tests/docs (default: gpt-5.4-mini)
PR_REVIEW_MODEL_DEEP - Model for /review deep (default: gpt-5.5)
PR_REVIEW_MODEL_FAST - Optional model override for default/tests/docs
(default: gpt-5.6-luna)
PR_REVIEW_MODEL_DEEP - Optional model override for /review deep
(default: gpt-5.6-terra)

The PR_REVIEW_MODEL_FAST env var keeps its name for backwards compatibility
with any existing repo-secrets overrides; the user-facing /review fast
alias was dropped 2026-04-23 because the default mode is fast enough.
with existing repository-variable overrides; the user-facing /review fast
alias was dropped 2026-04-23 because the default mode is sufficient.
"""

import json
Expand All @@ -38,10 +40,13 @@
# --- Constants ---

MAX_DIFF_CHARS = 100_000
DEFAULT_MODEL_FAST = "gpt-5.4-mini"
DEFAULT_MODEL_DEEP = "gpt-5.5"
# These are the sole default model definitions. GitHub Actions may supply
# optional repository-variable overrides, but an unset or empty override must
# fall back here so workflow configuration cannot drift from local behavior.
DEFAULT_MODEL_FAST = "gpt-5.6-luna"
DEFAULT_MODEL_DEEP = "gpt-5.6-terra"
DEFAULT_TEMPERATURE = 0.2
DEFAULT_MAX_COMPLETION_TOKENS = 4096
DEFAULT_MAX_COMPLETION_TOKENS = 8192
DEEP_MAX_COMPLETION_TOKENS = 25000
DEFAULT_LLM_TIMEOUT_SECONDS = 120
DEEP_LLM_TIMEOUT_SECONDS = 300
Expand Down Expand Up @@ -181,6 +186,13 @@ def build_llm_payload(
return payload


def model_for_mode(mode: str) -> str:
"""Return the configured model for a review mode, with Python defaults."""
if mode == "deep":
return os.environ.get("PR_REVIEW_MODEL_DEEP") or DEFAULT_MODEL_DEEP
return os.environ.get("PR_REVIEW_MODEL_FAST") or DEFAULT_MODEL_FAST


def summarize_usage(data: dict) -> str:
"""Return compact token usage details for operator-visible errors."""
usage = data.get("usage")
Expand All @@ -200,11 +212,11 @@ def summarize_usage(data: dict) -> str:
def extract_chat_content(data: dict) -> str:
"""Extract visible text from a chat-completions response."""
choices = data.get("choices", [])
if not choices:
raise LLMReviewError("LLM returned no choices. Raw response: " + json.dumps(data)[:500])
if not isinstance(choices, list) or not choices:
raise LLMReviewError("LLM returned no choices.")

choice = choices[0]
message = choice.get("message", {})
choice = choices[0] if isinstance(choices[0], dict) else {}
message = choice.get("message") if isinstance(choice.get("message"), dict) else {}
content = message.get("content", "")
if isinstance(content, list):
content = "".join(part.get("text", "") for part in content if isinstance(part, dict))
Expand All @@ -229,10 +241,7 @@ def call_llm(diff: str, mode: str, system_prompt: str) -> str:
litellm_key = os.environ.get("LITELLM_API_KEY", "")
openai_key = os.environ.get("OPENAI_API_KEY", "")

if mode == "deep":
model = os.environ.get("PR_REVIEW_MODEL_DEEP") or DEFAULT_MODEL_DEEP
else:
model = os.environ.get("PR_REVIEW_MODEL_FAST") or DEFAULT_MODEL_FAST
model = model_for_mode(mode)

if litellm_url and litellm_key:
api_url = litellm_url.rstrip("/") + "/chat/completions"
Expand Down Expand Up @@ -264,13 +273,13 @@ def call_llm(diff: str, mode: str, system_prompt: str) -> str:
timeout = DEEP_LLM_TIMEOUT_SECONDS if is_deep else DEFAULT_LLM_TIMEOUT_SECONDS
resp = requests.post(api_url, headers=headers, json=payload, timeout=timeout)
if resp.status_code != 200:
body = resp.text[:500]
raise LLMReviewError(
f"LLM API returned {resp.status_code}.\n\n"
f"**Model:** `{model}`\n\n"
f"**Response:**\n```\n{body}\n```"
)
data = resp.json()
raise LLMReviewError(f"LLM API returned {resp.status_code} for model `{model}`.")
try:
data = resp.json()
except (json.JSONDecodeError, ValueError) as error:
raise LLMReviewError("LLM returned invalid JSON.") from error
if not isinstance(data, dict):
raise LLMReviewError("LLM returned a non-object JSON response.")
return extract_chat_content(data)


Expand Down Expand Up @@ -328,9 +337,7 @@ def main() -> None:
post_comment(repo, pr_number, gh_token, f"**AI Review Error:** {e}")
sys.exit(1)

model_name = os.environ.get(
"PR_REVIEW_MODEL_DEEP" if mode == "deep" else "PR_REVIEW_MODEL_FAST"
) or (DEFAULT_MODEL_DEEP if mode == "deep" else DEFAULT_MODEL_FAST)
model_name = model_for_mode(mode)

mode_labels = {
"default": "security",
Expand Down
147 changes: 147 additions & 0 deletions tests/test_pr_review_routing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
"""Regression tests for the trusted GitHub PR-review runner's model routing."""

from __future__ import annotations

import importlib.util
import re
import unittest
from pathlib import Path
from types import ModuleType
from unittest.mock import Mock, patch

ROOT = Path(__file__).resolve().parents[1]
SCRIPT_PATH = ROOT / "scripts" / "pr-review.py"
WORKFLOW_PATH = ROOT / ".github" / "workflows" / "pr-review.yaml"


def load_pr_review_module() -> ModuleType:
"""Load the workflow script without executing its CLI entry point."""
spec = importlib.util.spec_from_file_location("pr_review_routing", SCRIPT_PATH)
assert spec is not None
assert spec.loader is not None
module = importlib.util.module_from_spec(spec)
with patch.dict("sys.modules", {"requests": Mock()}):
spec.loader.exec_module(module)
return module


class PRReviewRoutingTests(unittest.TestCase):
def test_model_defaults_and_mode_routing(self):
module = load_pr_review_module()
with patch.dict(
module.os.environ,
{"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""},
clear=False,
):
self.assertEqual(module.DEFAULT_MODEL_FAST, "gpt-5.6-luna")
self.assertEqual(module.DEFAULT_MODEL_DEEP, "gpt-5.6-terra")
self.assertEqual(module.model_for_mode("default"), "gpt-5.6-luna")
self.assertEqual(module.model_for_mode("tests"), "gpt-5.6-luna")
self.assertEqual(module.model_for_mode("docs"), "gpt-5.6-luna")
self.assertEqual(module.model_for_mode("deep"), "gpt-5.6-terra")
self.assertEqual(module.FAST_REASONING_EFFORT, "low")
self.assertEqual(module.DEEP_REASONING_EFFORT, "medium")

def test_gpt_5_6_payloads_pin_the_requested_reasoning_effort(self):
module = load_pr_review_module()

ordinary_payload = module.build_llm_payload(
module.DEFAULT_MODEL_FAST,
"system prompt",
"diff",
reasoning_effort=module.FAST_REASONING_EFFORT,
)
deep_payload = module.build_llm_payload(
module.DEFAULT_MODEL_DEEP,
"system prompt",
"diff",
reasoning_effort=module.DEEP_REASONING_EFFORT,
)

self.assertEqual(ordinary_payload["model"], "gpt-5.6-luna")
self.assertEqual(ordinary_payload["reasoning_effort"], "low")
self.assertEqual(ordinary_payload["max_completion_tokens"], 8192)
self.assertEqual(deep_payload["model"], "gpt-5.6-terra")
self.assertEqual(deep_payload["reasoning_effort"], "medium")

def test_model_repository_variable_overrides(self):
module = load_pr_review_module()
with patch.dict(
module.os.environ,
{
"PR_REVIEW_MODEL_FAST": "provider/ordinary-override",
"PR_REVIEW_MODEL_DEEP": "provider/deep-override",
},
clear=False,
):
self.assertEqual(module.model_for_mode("default"), "provider/ordinary-override")
self.assertEqual(module.model_for_mode("tests"), "provider/ordinary-override")
self.assertEqual(module.model_for_mode("docs"), "provider/ordinary-override")
self.assertEqual(module.model_for_mode("deep"), "provider/deep-override")

def test_empty_repository_variable_overrides_use_python_defaults(self):
module = load_pr_review_module()
with patch.dict(
module.os.environ,
{"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""},
clear=False,
):
self.assertEqual(module.model_for_mode("default"), module.DEFAULT_MODEL_FAST)
self.assertEqual(module.model_for_mode("deep"), module.DEFAULT_MODEL_DEEP)

def test_workflow_delegates_model_defaults_to_python(self):
workflow = WORKFLOW_PATH.read_text(encoding="utf-8")

self.assertIn("PR_REVIEW_MODEL_FAST: ${{ vars.PR_REVIEW_MODEL_FAST }}", workflow)
self.assertIn("PR_REVIEW_MODEL_DEEP: ${{ vars.PR_REVIEW_MODEL_DEEP }}", workflow)
self.assertIsNone(re.search(r"PR_REVIEW_MODEL_(?:FAST|DEEP): gpt-", workflow))

def test_workflow_keeps_trusted_runner_and_owner_gate(self):
workflow = WORKFLOW_PATH.read_text(encoding="utf-8")

self.assertIn("github.event.comment.user.login == 'luckyPipewrench'", workflow)
self.assertIn("github.event.comment.author_association == 'OWNER'", workflow)
self.assertIn("ref: ${{ github.event.repository.default_branch }}", workflow)
self.assertIn("persist-credentials: false", workflow)
self.assertIn("LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }}", workflow)
self.assertIn("LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }}", workflow)
self.assertIn("OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}", workflow)
self.assertIn(
"group: pr-review-${{ github.repository }}-${{ github.event.issue.number }}", workflow
)
self.assertIn("cancel-in-progress: true", workflow)
self.assertIn("python -m unittest tests/test_pr_review_routing.py", workflow)

runner = SCRIPT_PATH.read_text(encoding="utf-8")
self.assertNotIn("resp.text[:500]", runner)

def test_response_shape_errors_are_generic_and_fail_closed(self):
module = load_pr_review_module()

with self.assertRaises(module.LLMReviewError) as ctx:
module.extract_chat_content({"choices": [], "private": "provider detail"})
self.assertIn("no choices", str(ctx.exception))
self.assertNotIn("provider detail", str(ctx.exception))

with self.assertRaisesRegex(module.LLMReviewError, "empty content"):
module.extract_chat_content({"choices": [None]})

def test_call_rejects_invalid_or_non_object_json(self):
module = load_pr_review_module()
invalid = Mock(status_code=200)
invalid.json.side_effect = ValueError("invalid")
with (
patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True),
patch.object(module.requests, "post", return_value=invalid),
self.assertRaisesRegex(module.LLMReviewError, "invalid JSON"),
):
module.call_llm("diff", "default", "system")

non_object = Mock(status_code=200)
non_object.json.return_value = []
with (
patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True),
patch.object(module.requests, "post", return_value=non_object),
self.assertRaisesRegex(module.LLMReviewError, "non-object JSON"),
):
module.call_llm("diff", "default", "system")
Loading