Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ ACP_AGENT_TYPE=claude

# [optional] Model override. OpenCode expects the full provider/model ID.
# ACP_MODEL=
# Review comments display ACP_MODEL when set, otherwise ANTHROPIC_MODEL for Claude or
# OPENCODE_MODEL for OpenCode. These display fallbacks do not change ACP model selection.

# [optional] Max bytes per ACP JSON frame (default: 10485760 = 10 MB).
# ACP_STREAM_LIMIT=10485760
Expand Down
4 changes: 4 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ src/code_review_bot/
5. `ReviewPublisher.publish()` posts inline diff comments and formats the summary, storing the new
fingerprint set in hidden metadata for the next run. GitLab and GitHub without automatic
approval post it as a note; GitHub with automatic approval defers it to the final review body.
Summary notes end with a GitHub repository attribution line containing the formatted agent type
and skill fingerprint, followed by the model and token-usage line. The displayed model uses
`ACP_MODEL` when set, otherwise `ANTHROPIC_MODEL` for Claude or `OPENCODE_MODEL` for OpenCode;
these fallbacks do not change ACP model selection.
6. When `AUTO_APPROVE_ON_CLEAN_REVIEW=true` (and not in `--debug` mode), the orchestrator
approves the change request if no new findings were published, or revokes approval when new
findings exist (GitLab: approve/unapprove API; GitHub: `APPROVE` / `REQUEST_CHANGES` review
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ See `.env.example` for the full list with descriptions.
| `ACP_VERBOSE` | no | `true` | Log ACP input prompts, tool calls, and streamed agent messages |
| `ACP_STREAM_LIMIT` | no | `10485760` (10 MB) | Max bytes for one ACP newline-delimited JSON frame |

Review comments display `ACP_MODEL` when set. Otherwise, they fall back to `ANTHROPIC_MODEL` for
Claude or `OPENCODE_MODEL` for OpenCode. These fallbacks are display-only and do not change the
existing ACP model-selection behavior.

The review workspace checks out the target branch. Each ACP agent therefore discovers project
instructions and configuration through its own native rules, without the bot maintaining a list
of recognized files. The source branch is retained only as `refs/code-review/source` and is
Expand Down
5 changes: 2 additions & 3 deletions src/code_review_bot/agent/json_extract.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,21 @@
import logging
import re
from collections.abc import Awaitable, Callable
from typing import Any, TypeVar
from typing import Any

import json_repair
from pydantic import BaseModel, ValidationError

logger = logging.getLogger(__name__)

SchemaT = TypeVar("SchemaT", bound=BaseModel)
JSON_BLOCK_RE = re.compile(r"```(?:json)?\s*(?P<json>\{.*?\})\s*```", re.DOTALL)
TextRunner = Callable[[str], Awaitable[str]]

_DIAGNOSTIC_TRUNCATE = 4000
_CTRL_ESCAPE = {"\n": "\\n", "\r": "\\r", "\t": "\\t", "\b": "\\b", "\f": "\\f"}


async def complete_json_with_retries(
async def complete_json_with_retries[SchemaT: BaseModel](
prompt: str,
schema: type[SchemaT],
runner: TextRunner,
Expand Down
15 changes: 14 additions & 1 deletion src/code_review_bot/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class Settings(BaseSettings):
description="Required when ACP_AGENT_TYPE is not built-in. Optional override for built-in.",
)
acp_model: str | None = None
anthropic_model: str | None = None
opencode_model: str | None = None
acp_stream_limit: int = Field(
default=10 * 1024 * 1024,
description="Max bytes for one ACP newline-delimited JSON frame from the coding agent.",
Expand Down Expand Up @@ -141,6 +143,17 @@ def resolved_acp_args(self) -> list[str]:
args=self.acp_args,
)[1]

@property
def review_model_name(self) -> str | None:
"""Resolve the model label shown in review output without changing agent selection."""
if self.acp_model:
return self.acp_model
if self.acp_agent_type == "claude":
return self.anthropic_model
if self.acp_agent_type == "opencode":
return self.opencode_model
return None

@field_validator("acp_agent_type", mode="before")
@classmethod
def normalize_acp_agent_type(cls, value: object) -> str:
Expand All @@ -158,7 +171,7 @@ def blank_command_is_none(cls, value: object) -> str | None:
return None
return str(value).strip() if isinstance(value, str) else value

@field_validator("acp_model", mode="before")
@field_validator("acp_model", "anthropic_model", "opencode_model", mode="before")
@classmethod
def blank_model_is_none(cls, value: object) -> str | None:
if value is None:
Expand Down
6 changes: 5 additions & 1 deletion src/code_review_bot/review/orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@ async def review_change_request(self, cr_id: str) -> ReviewOutcome:

logger.info("Running skill name=%s version=%s", skill.name, skill.version)
agent = build_coding_agent(self.settings, review_workspace / "source")
result = await CodingAgentReviewRunner(agent).review(skill, task_context)
result = await CodingAgentReviewRunner(
agent,
agent_type=self.settings.acp_agent_type,
configured_model=self.settings.review_model_name,
).review(skill, task_context)

file_filter = FileFilter(task_context.excluded_patterns, task_context.included_patterns)
result.findings, file_excluded = file_filter.filter_findings(result.findings)
Expand Down
21 changes: 17 additions & 4 deletions src/code_review_bot/review/publish/formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,15 @@
from code_review_bot.skill.protocol import Finding, RuntimeMetadata, count_findings_by_severity

BOT_METADATA_PREFIX = "<!-- code-review-bot:"
CODE_REVIEW_BOT_LABEL = "whhe/code-review-bot"
CODE_REVIEW_BOT_URL = "https://github.com/whhe/code-review-bot"

AGENT_DISPLAY_NAMES: dict[str, str] = {
"claude": "Claude Code",
"codex": "Codex",
"opencode": "OpenCode",
}

SEVERITY_LABELS: dict[str, str] = {
"critical": "Critical",
"high": "High",
Expand Down Expand Up @@ -70,11 +77,8 @@ def format_review_note(
lines.append("")
if lines and lines[-1] != "":
lines.append("")
lines.append(_format_attribution_line(runtime, skill_version))
lines.append(_format_runtime_line(runtime))
lines.append(
f"_Generated by [whhe/code-review-bot]({CODE_REVIEW_BOT_URL}) · "
f"Skill fingerprint: `{skill_version}`_"
)
metadata = {
"head_sha": cr.head_sha,
"skill": skill_name,
Expand All @@ -96,6 +100,15 @@ def _format_runtime_line(runtime: RuntimeMetadata | None) -> str:
)


def _format_attribution_line(runtime: RuntimeMetadata | None, skill_version: str) -> str:
raw_agent = runtime.agent_type if runtime and runtime.agent_type else "unavailable"
agent = AGENT_DISPLAY_NAMES.get(raw_agent, raw_agent)
return (
f"*Generated by* [*{CODE_REVIEW_BOT_LABEL}*]({CODE_REVIEW_BOT_URL}) "
f"*· Agent:* *{agent}* *· Skill fingerprint:* *{skill_version}*"
)


def _format_token_value(value: int | None) -> str:
if value is None:
return "unavailable"
Expand Down
26 changes: 19 additions & 7 deletions src/code_review_bot/review/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,18 @@ def build_prompt(self, context: ReviewTaskContext) -> str: ...
class CodingAgentReviewRunner:
"""Executes a review skill by dispatching to a CodingAgent and parsing its JSON output."""

def __init__(self, agent: CodingAgent, max_json_retries: int = 1) -> None:
def __init__(
self,
agent: CodingAgent,
max_json_retries: int = 1,
*,
agent_type: str | None = None,
configured_model: str | None = None,
) -> None:
self.agent = agent
self.max_json_retries = max_json_retries
self.agent_type = agent_type
self.configured_model = configured_model

async def review(self, skill: PromptBuildingSkill, context: ReviewTaskContext) -> SkillResult:
prompt = skill.build_prompt(context)
Expand Down Expand Up @@ -57,12 +66,13 @@ async def run(current_prompt: str) -> str:
run,
max_retries=self.max_json_retries,
)
model_value: str | None = None
unique_models = list(dict.fromkeys(model_candidates))
if len(unique_models) == 1:
model_value = unique_models[0]
elif len(unique_models) > 1:
model_value = f"multiple models used: {', '.join(unique_models)}"
model_value = self.configured_model
if model_value is None:
unique_models = list(dict.fromkeys(model_candidates))
if len(unique_models) == 1:
model_value = unique_models[0]
elif len(unique_models) > 1:
model_value = f"multiple models used: {', '.join(unique_models)}"

def aggregate_tokens(key: str) -> int | None:
# Show totals only when every call reported the metric so the final
Expand All @@ -73,6 +83,7 @@ def aggregate_tokens(key: str) -> int | None:
return token_totals[key]

runtime = RuntimeMetadata(
agent_type=self.agent_type,
model=model_value,
input_tokens=aggregate_tokens("input_tokens"),
output_tokens=aggregate_tokens("output_tokens"),
Expand All @@ -82,6 +93,7 @@ def aggregate_tokens(key: str) -> int | None:
value is not None
for value in (
runtime.model,
runtime.agent_type,
runtime.input_tokens,
runtime.output_tokens,
runtime.total_tokens,
Expand Down
1 change: 1 addition & 0 deletions src/code_review_bot/skill/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def with_runtime(self, runtime: "RuntimeMetadata | None") -> "SkillResult":


class RuntimeMetadata(BaseModel):
agent_type: str | None = None
model: str | None = None
input_tokens: int | None = None
output_tokens: int | None = None
Expand Down
25 changes: 25 additions & 0 deletions tests/test_agent_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,28 @@ def test_codex_agent_uses_acp_model_config_option(tmp_path: Path) -> None:

assert isinstance(agent, AcpCodingAgent)
assert agent.config.model_config_option == "model"


@pytest.mark.parametrize(
("agent_type", "env_name", "env_value"),
[
("claude", "ANTHROPIC_MODEL", "claude-opus-4-6"),
("opencode", "OPENCODE_MODEL", "qwen3.7-max"),
],
)
def test_review_model_fallback_does_not_change_acp_model_selection(
monkeypatch: pytest.MonkeyPatch,
tmp_path: Path,
agent_type: str,
env_name: str,
env_value: str,
) -> None:
monkeypatch.delenv("ACP_MODEL", raising=False)
monkeypatch.setenv(env_name, env_value)
settings = _settings(agent_type)

agent = build_coding_agent(settings, tmp_path)

assert settings.review_model_name == env_value
assert isinstance(agent, AcpCodingAgent)
assert agent.config.model is None
54 changes: 54 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,60 @@ def test_settings_acp_model_blank_string_becomes_none(monkeypatch: pytest.Monkey
assert settings.acp_model is None


def test_settings_review_model_name_prefers_acp_model(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("ACP_AGENT_TYPE", "opencode")
monkeypatch.setenv("ACP_MODEL", "provider/explicit-model")
monkeypatch.setenv("OPENCODE_MODEL", "configured-model")
from code_review_bot.config import Settings

settings = Settings(_env_file=None)

assert settings.review_model_name == "provider/explicit-model"


@pytest.mark.parametrize(
("agent_type", "env_name", "env_value"),
[
("claude", "ANTHROPIC_MODEL", "claude-opus-4-6"),
("opencode", "OPENCODE_MODEL", "qwen3.7-max"),
],
)
def test_settings_review_model_name_falls_back_to_agent_config(
monkeypatch: pytest.MonkeyPatch,
agent_type: str,
env_name: str,
env_value: str,
) -> None:
monkeypatch.delenv("ACP_MODEL", raising=False)
monkeypatch.setenv("ACP_AGENT_TYPE", agent_type)
monkeypatch.setenv(env_name, env_value)
from code_review_bot.config import Settings

settings = Settings(_env_file=None)

assert settings.review_model_name == env_value


def test_settings_review_model_name_is_none_without_matching_config(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.delenv("ACP_MODEL", raising=False)
monkeypatch.delenv("ANTHROPIC_MODEL", raising=False)
monkeypatch.setenv("OPENCODE_MODEL", "qwen3.7-max")
from code_review_bot.config import Settings

settings = Settings(
git_repo_url="https://gitlab.test/group/project.git",
git_repo_token="tok",
acp_agent_type="claude",
_env_file=None,
)

assert settings.review_model_name is None


def test_settings_log_dir_derives_fixed_subdirs(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("LOG_DIR", "var/logs")
from code_review_bot.config import Settings
Expand Down
51 changes: 48 additions & 3 deletions tests/test_review_publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def make_finding(**overrides: object) -> Finding:

def make_runtime(**overrides: object) -> RuntimeMetadata:
data: dict[str, object] = {
"agent_type": "opencode",
"model": "provider/model",
"input_tokens": 12345,
"output_tokens": 678,
Expand Down Expand Up @@ -210,6 +211,52 @@ async def test_formatter_includes_runtime_line() -> None:
assert "Model: provider/model · Tokens: input 12,345 / output 678 / total 13,023" in body


@pytest.mark.asyncio
async def test_formatter_renders_attribution_before_final_model_line() -> None:
body = format_review_note(
cr=make_change_request(),
summary="Reviewed",
severity_counts={"critical": 0, "high": 1, "medium": 0, "low": 0},
located_count=1,
unlocated_findings=[],
skill_name="default",
skill_version="43b5df0c",
fingerprints=["fp1"],
runtime=make_runtime(model="qwen3.7-max"),
)

visible_lines = [line for line in body.splitlines() if not line.startswith(BOT_METADATA_PREFIX)]
assert visible_lines[-2] == (
"*Generated by* "
"[*whhe/code-review-bot*](https://github.com/whhe/code-review-bot) "
"*· Agent:* *OpenCode* *· Skill fingerprint:* *43b5df0c*"
)
assert visible_lines[-1] == (
"Model: qwen3.7-max · Tokens: input 12,345 / output 678 / total 13,023"
)


@pytest.mark.parametrize(
("agent_type", "expected"),
[
("claude", "Claude Code"),
("codex", "Codex"),
("opencode", "OpenCode"),
("custom-agent", "custom-agent"),
],
)
def test_formatter_uses_agent_display_name(agent_type: str, expected: str) -> None:
body = format_review_note(
cr=make_change_request(),
skill_name="default",
skill_version="43b5df0c",
fingerprints=[],
runtime=make_runtime(agent_type=agent_type),
)

assert f"*· Agent:* *{expected}*" in body


@pytest.mark.asyncio
async def test_formatter_marks_unavailable_runtime_values() -> None:
body = format_review_note(
Expand All @@ -224,9 +271,7 @@ async def test_formatter_marks_unavailable_runtime_values() -> None:
runtime=make_runtime(model=None, output_tokens=None),
)

assert (
"Model: unavailable · Tokens: input 12,345 / output unavailable / total 13,023" in body
)
assert "Model: unavailable · Tokens: input 12,345 / output unavailable / total 13,023" in body


@pytest.mark.asyncio
Expand Down
Loading