From 24bee5e60f95cff44da80282a8ee455d7f0cb51b Mon Sep 17 00:00:00 2001
From: He Wang <wanghechn@qq.com>
Date: Tue, 21 Jul 2026 19:21:52 +0800
Subject: [PATCH] feat(review): enrich review attribution metadata

feat:
- Show agent identity before the skill fingerprint in review summaries
- Prefer configured model names for display without changing ACP selection
- Preserve the GitHub repository attribution and place model usage last

fix:
- Update JSON retry generics for Python 3.12 Ruff compliance

docs:
- Document display-only model fallback behavior

test:
- Cover model precedence, agent labels, footer layout, and ACP isolation
---
 .env.example                                  |  2 +
 AGENTS.md                                     |  4 ++
 README.md                                     |  4 ++
 src/code_review_bot/agent/json_extract.py     |  5 +-
 src/code_review_bot/config.py                 | 15 +++++-
 src/code_review_bot/review/orchestrator.py    |  6 ++-
 .../review/publish/formatter.py               | 21 ++++++--
 src/code_review_bot/review/runner.py          | 26 ++++++---
 src/code_review_bot/skill/protocol.py         |  1 +
 tests/test_agent_factory.py                   | 25 +++++++++
 tests/test_config.py                          | 54 +++++++++++++++++++
 tests/test_review_publisher.py                | 51 ++++++++++++++++--
 tests/test_review_runner.py                   | 22 ++++++++
 13 files changed, 217 insertions(+), 19 deletions(-)

diff --git a/.env.example b/.env.example
index 1acef26..6b8680c 100644
--- a/.env.example
+++ b/.env.example
@@ -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
diff --git a/AGENTS.md b/AGENTS.md
index c8883ea..b6244fd 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -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
diff --git a/README.md b/README.md
index 140a1b5..12d3c2a 100644
--- a/README.md
+++ b/README.md
@@ -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
diff --git a/src/code_review_bot/agent/json_extract.py b/src/code_review_bot/agent/json_extract.py
index a5dc95b..35ca6e0 100644
--- a/src/code_review_bot/agent/json_extract.py
+++ b/src/code_review_bot/agent/json_extract.py
@@ -4,14 +4,13 @@
 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]]
 
@@ -19,7 +18,7 @@
 _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,
diff --git a/src/code_review_bot/config.py b/src/code_review_bot/config.py
index 5256139..e3d3c91 100644
--- a/src/code_review_bot/config.py
+++ b/src/code_review_bot/config.py
@@ -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.",
@@ -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:
@@ -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:
diff --git a/src/code_review_bot/review/orchestrator.py b/src/code_review_bot/review/orchestrator.py
index 4852111..0ec305b 100644
--- a/src/code_review_bot/review/orchestrator.py
+++ b/src/code_review_bot/review/orchestrator.py
@@ -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)
diff --git a/src/code_review_bot/review/publish/formatter.py b/src/code_review_bot/review/publish/formatter.py
index 1527932..f36d9c3 100644
--- a/src/code_review_bot/review/publish/formatter.py
+++ b/src/code_review_bot/review/publish/formatter.py
@@ -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",
@@ -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,
@@ -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"
diff --git a/src/code_review_bot/review/runner.py b/src/code_review_bot/review/runner.py
index 91100e0..cc309b9 100644
--- a/src/code_review_bot/review/runner.py
+++ b/src/code_review_bot/review/runner.py
@@ -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)
@@ -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
@@ -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"),
@@ -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,
diff --git a/src/code_review_bot/skill/protocol.py b/src/code_review_bot/skill/protocol.py
index 3103a62..d4bf105 100644
--- a/src/code_review_bot/skill/protocol.py
+++ b/src/code_review_bot/skill/protocol.py
@@ -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
diff --git a/tests/test_agent_factory.py b/tests/test_agent_factory.py
index cf5dc74..54b7e2a 100644
--- a/tests/test_agent_factory.py
+++ b/tests/test_agent_factory.py
@@ -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
diff --git a/tests/test_config.py b/tests/test_config.py
index 9c56442..a30359f 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -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
diff --git a/tests/test_review_publisher.py b/tests/test_review_publisher.py
index 5429329..9b6ca08 100644
--- a/tests/test_review_publisher.py
+++ b/tests/test_review_publisher.py
@@ -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,
@@ -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(
@@ -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
diff --git a/tests/test_review_runner.py b/tests/test_review_runner.py
index 4b2a5e1..2aefa0a 100644
--- a/tests/test_review_runner.py
+++ b/tests/test_review_runner.py
@@ -172,3 +172,25 @@ async def run_once(self, prompt: str, **kwargs: object) -> AgentRunResult:
     assert result.runtime.input_tokens is None
     assert result.runtime.output_tokens == 3
     assert result.runtime.total_tokens == 3
+
+
+@pytest.mark.asyncio
+async def test_coding_agent_review_runner_prefers_configured_display_model() -> None:
+    class RuntimeAgent:
+        async def run_once(self, prompt: str, **kwargs: object) -> AgentRunResult:
+            return AgentRunResult(
+                text='{"summary":"ok","findings":[]}',
+                parts=[],
+                usage={"input_tokens": 10, "output_tokens": 2, "total_tokens": 12},
+                model="response-model",
+            )
+
+    result = await CodingAgentReviewRunner(
+        RuntimeAgent(),
+        agent_type="opencode",
+        configured_model="configured-model",
+    ).review(FakeSkill(), make_task_context())
+
+    assert result.runtime is not None
+    assert result.runtime.agent_type == "opencode"
+    assert result.runtime.model == "configured-model"