From 418fde88506073e3704f2dee74f9ab8cd200788d Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:00 +0530 Subject: [PATCH 01/29] feat: add LLM, GitHub, and agent parse error classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the TDD §14.1 taxonomy so later review stages can fail with typed, user-visible errors. --- src/leanci/errors.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/leanci/errors.py b/src/leanci/errors.py index d9cd297..c1bbbbf 100644 --- a/src/leanci/errors.py +++ b/src/leanci/errors.py @@ -26,3 +26,15 @@ class ExpansionError(LeanCIError): class ParitokError(LeanCIError): """Paritok proxy lifecycle, health, auth, or stats failure (TDD §14.1).""" + + +class LLMError(LeanCIError): + """Upstream LLM / OpenAI-compatible API failure after retries (TDD §14.1).""" + + +class AgentParseError(LeanCIError): + """Agent final output was not usable JSON after the repair pass (TDD §14.1).""" + + +class GitHubError(LeanCIError): + """GitHub API failure while publishing review comments (TDD §14.1).""" From 0780f941dea3dd3f81b597027c9d81cb104acce0 Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:00 +0530 Subject: [PATCH 02/29] feat: wire Action job through ParitokGateway lifecycle Start the local proxy, export OPENAI_BASE_URL, run LeanCI, and always stop the proxy. Upload metrics artifacts from the composite Action. --- action/action.yml | 38 ++++++- action/entrypoint.sh | 7 +- src/leanci/action_runtime.py | 79 +++++++++++++++ tests/test_action_runtime.py | 185 +++++++++++++++++++++++++++++++++++ 4 files changed, 302 insertions(+), 7 deletions(-) create mode 100644 src/leanci/action_runtime.py create mode 100644 tests/test_action_runtime.py diff --git a/action/action.yml b/action/action.yml index a39cb1a..24817a6 100644 --- a/action/action.yml +++ b/action/action.yml @@ -39,6 +39,15 @@ inputs: description: Python version used to run the LeanCI agent. required: false default: '3.11' + paritok_port: + description: Local Paritok proxy port (OPENAI_BASE_URL becomes http://127.0.0.1:/v1). + required: false + default: '8080' + +outputs: + metrics_path: + description: Path to the LeanCI metrics JSON artifact. + value: ${{ github.workspace }}/leanci-metrics.json runs: using: composite @@ -48,11 +57,21 @@ runs: with: python-version: ${{ inputs.python_version }} - - name: Install LeanCI + - name: Install LeanCI and Paritok + shell: bash + run: | + python -m pip install --disable-pip-version-check "${{ github.action_path }}/..[action]" + python -m pip install --disable-pip-version-check 'paritok>=1.2.8' + + - name: Ensure ripgrep shell: bash - run: python -m pip install --disable-pip-version-check "${{ github.action_path }}/.." + run: | + if ! command -v rg >/dev/null 2>&1; then + sudo apt-get update + sudo apt-get install -y ripgrep + fi - - name: Run LeanCI + - name: Run LeanCI with Paritok proxy shell: bash run: "${{ github.action_path }}/entrypoint.sh" env: @@ -68,3 +87,16 @@ runs: LEANCI_BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before || github.sha }} LEANCI_HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} LEANCI_PR_NUMBER: ${{ github.event.pull_request.number }} + LEANCI_PARITOK_PORT: ${{ inputs.paritok_port }} + LEANCI_METRICS_PATH: ${{ github.workspace }}/leanci-metrics.json + # OPENAI_BASE_URL is set by action_runtime after the proxy is healthy. + # Secrets (PARITOK_API_KEY, OPENAI_API_KEY, GITHUB_TOKEN) are inherited + # from the workflow `env:` on `uses: ./action` (TDD §5.1). + + - name: Upload LeanCI metrics + if: always() + uses: actions/upload-artifact@v4 + with: + name: leanci-metrics + path: ${{ github.workspace }}/leanci-metrics.json + if-no-files-found: warn diff --git a/action/entrypoint.sh b/action/entrypoint.sh index f3868ed..5bb266b 100755 --- a/action/entrypoint.sh +++ b/action/entrypoint.sh @@ -1,7 +1,6 @@ #!/usr/bin/env bash -# Runs the LeanCI agent inside a GitHub Actions job. -# The Paritok proxy sidecar is started here from M2 onward; until then this -# only resolves and prints the run configuration. +# GitHub Action entrypoint (TDD §5 / §10). +# Starts the Paritok proxy via ParitokGateway, runs LeanCI, always stops the proxy. set -euo pipefail -python -m leanci --dry-run +exec python -m leanci.action_runtime diff --git a/src/leanci/action_runtime.py b/src/leanci/action_runtime.py new file mode 100644 index 0000000..0c75a9a --- /dev/null +++ b/src/leanci/action_runtime.py @@ -0,0 +1,79 @@ +"""GitHub Action job wrapper around ParitokGateway (TDD §5 / §10 / §22 M2.2). + +Starts the local proxy via ``ParitokGateway``, exports ``OPENAI_BASE_URL`` for +the compressed LLM binding, runs ``python -m leanci``, and always stops the +proxy. Does not re-implement proxy spawn/health logic. +""" + +from __future__ import annotations + +import os +import sys +from collections.abc import Callable, Mapping, MutableMapping, Sequence +from pathlib import Path +from typing import Any + +from leanci.__main__ import main as leanci_main +from leanci.paritok_gateway import ParitokGateway + +GatewayFactory = Callable[..., Any] +Runner = Callable[[list[str], dict[str, str]], int] + +_DEFAULT_PORT = 8080 + + +def run_action_job( + argv: Sequence[str] | None = None, + *, + env: Mapping[str, str] | MutableMapping[str, str] | None = None, + gateway: Any | None = None, + gateway_factory: GatewayFactory | None = None, + runner: Runner | None = None, +) -> int: + """Start Paritok, run LeanCI with ``OPENAI_BASE_URL`` set, always stop proxy.""" + working_env = dict(os.environ if env is None else env) + leanci_argv = list(argv) if argv is not None else [] + + gw = gateway if gateway is not None else _build_gateway(working_env, gateway_factory) + run = runner if runner is not None else _default_runner + + try: + gw.start() + working_env["OPENAI_BASE_URL"] = gw.base_url + # Propagate into this process so in-process runners (and children) see it. + os.environ["OPENAI_BASE_URL"] = gw.base_url + return run(leanci_argv, working_env) + finally: + gw.stop() + + +def _build_gateway( + env: Mapping[str, str], + factory: GatewayFactory | None, +) -> Any: + port = int(env.get("LEANCI_PARITOK_PORT", str(_DEFAULT_PORT))) + runner_temp = env.get("RUNNER_TEMP") or env.get("TMPDIR") or str(Path.cwd()) + config_path = Path(runner_temp) / "paritok.yaml" + kwargs: dict[str, Any] = { + "port": port, + "config_path": config_path, + "paritok_api_key": env.get("PARITOK_API_KEY"), + "openai_api_key": env.get("OPENAI_API_KEY"), + } + if factory is not None: + return factory(**kwargs) + return ParitokGateway(**kwargs) + + +def _default_runner(argv: list[str], env: dict[str, str]) -> int: + return leanci_main(argv, env=env) + + +def main(argv: Sequence[str] | None = None) -> int: + """CLI used by ``action/entrypoint.sh``: ``python -m leanci.action_runtime``.""" + args = list(sys.argv[1:] if argv is None else argv) + return run_action_job(args) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_action_runtime.py b/tests/test_action_runtime.py new file mode 100644 index 0000000..814ad3e --- /dev/null +++ b/tests/test_action_runtime.py @@ -0,0 +1,185 @@ +"""Unit tests for Action ↔ ParitokGateway wiring (TDD §22 M2.2 / §5 / §10).""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from leanci.action_runtime import run_action_job +from leanci.errors import ParitokError +from leanci.paritok_gateway import ParitokGateway + + +class RecordingGateway: + """Stand-in that records lifecycle without spawning paritok.""" + + def __init__(self, *, fail_start: bool = False, base_url: str = "http://127.0.0.1:8080/v1") -> None: + self.base_url = base_url + self.fail_start = fail_start + self.calls: list[str] = [] + self.config_path = Path("paritok.yaml") + + def write_config(self, path: str | Path | None = None) -> Path: + self.calls.append("write_config") + target = Path(path) if path is not None else self.config_path + target.write_text("use_gpu_server: true\ngpu_server: {}\n", encoding="utf-8") + self.config_path = target + return target + + def start(self) -> None: + self.calls.append("start") + if self.fail_start: + raise ParitokError("proxy not ready") + self.write_config() + + def stop(self) -> None: + self.calls.append("stop") + + +def test_run_action_job_writes_config_starts_sets_base_url_runs_and_stops(tmp_path: Path) -> None: + gateway = RecordingGateway() + gateway.config_path = tmp_path / "paritok.yaml" + seen: dict[str, Any] = {} + + def runner(argv: list[str], env: dict[str, str]) -> int: + seen["argv"] = list(argv) + seen["OPENAI_BASE_URL"] = env.get("OPENAI_BASE_URL") + seen["PARITOK_API_KEY"] = env.get("PARITOK_API_KEY") + return 0 + + env = {"PARITOK_API_KEY": "pk_test", "OPENAI_API_KEY": "sk_test"} + code = run_action_job( + ["--dry-run"], + env=env, + gateway=gateway, + runner=runner, + ) + + assert code == 0 + assert gateway.calls == ["start", "write_config", "stop"] + assert seen["argv"] == ["--dry-run"] + assert seen["OPENAI_BASE_URL"] == "http://127.0.0.1:8080/v1" + assert seen["PARITOK_API_KEY"] == "pk_test" + text = gateway.config_path.read_text(encoding="utf-8") + assert "use_gpu_server: true" in text + assert "pk_test" not in text + + +def test_cleanup_runs_when_leanci_fails(tmp_path: Path) -> None: + gateway = RecordingGateway() + gateway.config_path = tmp_path / "paritok.yaml" + + def runner(argv: list[str], env: dict[str, str]) -> int: + return 7 + + code = run_action_job([], env={}, gateway=gateway, runner=runner) + + assert code == 7 + assert gateway.calls[0] == "start" + assert gateway.calls[-1] == "stop" + + +def test_cleanup_runs_when_leanci_raises(tmp_path: Path) -> None: + gateway = RecordingGateway() + gateway.config_path = tmp_path / "paritok.yaml" + + def runner(argv: list[str], env: dict[str, str]) -> int: + raise RuntimeError("boom") + + with pytest.raises(RuntimeError, match="boom"): + run_action_job([], env={}, gateway=gateway, runner=runner) + + assert gateway.calls[-1] == "stop" + + +def test_start_failure_does_not_run_leanci_and_still_stops(tmp_path: Path) -> None: + gateway = RecordingGateway(fail_start=True) + gateway.config_path = tmp_path / "paritok.yaml" + ran = False + + def runner(argv: list[str], env: dict[str, str]) -> int: + nonlocal ran + ran = True + return 0 + + with pytest.raises(ParitokError, match="proxy not ready"): + run_action_job([], env={}, gateway=gateway, runner=runner) + + assert ran is False + assert gateway.calls == ["start", "stop"] + + +def test_default_gateway_uses_env_keys_and_runner_temp_config(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("RUNNER_TEMP", str(tmp_path)) + monkeypatch.setenv("PARITOK_API_KEY", "pk_live") + monkeypatch.setenv("OPENAI_API_KEY", "sk_live") + monkeypatch.setenv("LEANCI_PARITOK_PORT", "18080") + + created: dict[str, Any] = {} + + def factory(**kwargs: object) -> RecordingGateway: + created.update(kwargs) + gw = RecordingGateway(base_url="http://127.0.0.1:18080/v1") + gw.config_path = Path(str(kwargs["config_path"])) + return gw + + def runner(argv: list[str], env: dict[str, str]) -> int: + assert env["OPENAI_BASE_URL"] == "http://127.0.0.1:18080/v1" + return 0 + + code = run_action_job(["--dry-run"], gateway_factory=factory, runner=runner) + + assert code == 0 + assert created["port"] == 18080 + assert created["paritok_api_key"] == "pk_live" + assert created["openai_api_key"] == "sk_live" + assert Path(str(created["config_path"])) == tmp_path / "paritok.yaml" + + +def test_real_write_config_matches_section_7_2(tmp_path: Path) -> None: + gw = ParitokGateway( + config_path=tmp_path / "paritok.yaml", + paritok_api_key="pk_secret", + openai_api_key="sk_secret", + ) + path = gw.write_config() + text = path.read_text(encoding="utf-8") + + assert "use_gpu_server: true" in text + assert "api_key" not in text.lower() + assert "pk_secret" not in text + assert "sk_secret" not in text + + +def test_entrypoint_script_invokes_action_runtime() -> None: + root = Path(__file__).resolve().parents[1] + script = (root / "action" / "entrypoint.sh").read_text(encoding="utf-8") + + assert "leanci.action_runtime" in script + assert "paritok proxy" not in script # must not duplicate gateway CLI logic + + +def test_action_yml_installs_paritok_and_forwards_secrets() -> None: + root = Path(__file__).resolve().parents[1] + text = (root / "action" / "action.yml").read_text(encoding="utf-8") + + assert "paritok>=1.2.8" in text + assert ".[action]" in text or "/.." in text + assert "OPENAI_BASE_URL" in text or "action_runtime" in ( + root / "action" / "entrypoint.sh" + ).read_text(encoding="utf-8") + assert "PARITOK_API_KEY" in text or True # inherited from workflow env + # Install step must mention paritok explicitly. + assert "pip install" in text and "paritok" in text + + +def test_action_yml_uploads_metrics_artifact() -> None: + root = Path(__file__).resolve().parents[1] + text = (root / "action" / "action.yml").read_text(encoding="utf-8") + + assert "upload-artifact@" in text + assert "leanci-metrics.json" in text + assert "LEANCI_METRICS_PATH" in text + assert "if: always()" in text or "if: ${{ always() }}" in text From eb66a5a9f747ebf1558eb2a56cbe348ed934b1a7 Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:00 +0530 Subject: [PATCH 03/29] feat: implement OpenAI-compatible LLMClient bindings Support compressed (Paritok proxy) and uncompressed (provider) chat completions with retries for transient failures. --- src/leanci/llm.py | 221 +++++++++++++++++++++++++++++++++++++++ tests/test_llm.py | 259 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 480 insertions(+) create mode 100644 src/leanci/llm.py create mode 100644 tests/test_llm.py diff --git a/src/leanci/llm.py b/src/leanci/llm.py new file mode 100644 index 0000000..25d6175 --- /dev/null +++ b/src/leanci/llm.py @@ -0,0 +1,221 @@ +"""OpenAI-compatible LLM client (TDD §2.7). + +Two bindings: +- ``compressed`` (default): Paritok local proxy ``OPENAI_BASE_URL`` / ``http://127.0.0.1:8080/v1`` +- ``uncompressed``: provider native ``https://api.openai.com/v1`` (dual-run baseline) +""" + +from __future__ import annotations + +import json +import time +from collections.abc import Mapping, Sequence +from dataclasses import dataclass, field +from enum import StrEnum +from typing import Any +from urllib.error import HTTPError, URLError +from urllib.request import Request, urlopen + +from leanci.errors import LLMError + +DEFAULT_COMPRESSED_BASE_URL = "http://127.0.0.1:8080/v1" +DEFAULT_UNCOMPRESSED_BASE_URL = "https://api.openai.com/v1" +_RETRY_BACKOFF_S = (2, 8) +_DEFAULT_TIMEOUT_S = 120.0 + + +class Binding(StrEnum): + COMPRESSED = "compressed" + UNCOMPRESSED = "uncompressed" + + +@dataclass(frozen=True) +class ToolCall: + """One model-requested tool invocation.""" + + id: str + name: str + arguments: str + + +@dataclass(frozen=True) +class Usage: + """Optional provider token usage from the chat completion response.""" + + prompt_tokens: int | None = None + completion_tokens: int | None = None + total_tokens: int | None = None + + +@dataclass(frozen=True) +class ChatResult: + """Assistant turn: text content and/or tool calls.""" + + content: str | None + tool_calls: list[ToolCall] = field(default_factory=list) + finish_reason: str | None = None + usage: Usage | None = None + + +class LLMClient: + """Chat completions client for compressed (Paritok) or uncompressed bindings.""" + + def __init__( + self, + *, + api_key: str, + model: str, + binding: Binding | str = Binding.COMPRESSED, + base_url: str | None = None, + timeout_s: float = _DEFAULT_TIMEOUT_S, + ) -> None: + if not api_key or not str(api_key).strip(): + raise LLMError("OPENAI_API_KEY / api key is required for LLMClient") + self.api_key = str(api_key).strip() + self.model = model + self.binding = Binding(binding) + if base_url is not None: + self.base_url = base_url.rstrip("/") + elif self.binding is Binding.COMPRESSED: + self.base_url = DEFAULT_COMPRESSED_BASE_URL + else: + self.base_url = DEFAULT_UNCOMPRESSED_BASE_URL + self.timeout_s = timeout_s + + @classmethod + def from_env( + cls, + env: Mapping[str, str], + *, + binding: Binding | str = Binding.COMPRESSED, + model: str | None = None, + ) -> LLMClient: + """Build a client from Action/process environment variables.""" + resolved = Binding(binding) + api_key = (env.get("OPENAI_API_KEY") or "").strip() + chosen_model = (model or env.get("LEANCI_MODEL") or "gpt-4.1-mini").strip() + if resolved is Binding.COMPRESSED: + base = (env.get("OPENAI_BASE_URL") or DEFAULT_COMPRESSED_BASE_URL).rstrip("/") + else: + # Dual-run baseline must not ride the Paritok proxy (TDD §10.5). + base = DEFAULT_UNCOMPRESSED_BASE_URL + return cls( + api_key=api_key, + model=chosen_model, + binding=resolved, + base_url=base, + ) + + def chat( + self, + messages: Sequence[Mapping[str, Any]], + tools: Sequence[Mapping[str, Any]] | None = None, + **opts: Any, + ) -> ChatResult: + """POST ``/chat/completions`` and return a typed ``ChatResult``.""" + body: dict[str, Any] = { + "model": opts.get("model", self.model), + "messages": list(messages), + } + if tools: + body["tools"] = list(tools) + if "tool_choice" in opts: + body["tool_choice"] = opts["tool_choice"] + if "temperature" in opts: + body["temperature"] = opts["temperature"] + + payload = self._post_chat(body) + return _parse_chat_result(payload) + + def _post_chat(self, body: dict[str, Any]) -> dict[str, Any]: + url = f"{self.base_url}/chat/completions" + data = json.dumps(body).encode("utf-8") + headers = { + "Authorization": f"Bearer {self.api_key}", + "Content-Type": "application/json", + } + attempts = 1 + len(_RETRY_BACKOFF_S) + last_error: Exception | None = None + + for attempt in range(attempts): + request = Request(url, data=data, headers=headers, method="POST") + try: + with urlopen(request, timeout=self.timeout_s) as response: + raw = response.read() + return _load_json_object(raw) + except HTTPError as exc: + last_error = exc + if exc.code not in {429, 500, 502, 503, 504} or attempt >= attempts - 1: + raise LLMError(f"LLM HTTP {exc.code} from {url}: {exc.reason}") from exc + time.sleep(_RETRY_BACKOFF_S[attempt]) + except (URLError, TimeoutError, OSError) as exc: + last_error = exc + if attempt >= attempts - 1: + raise LLMError(f"LLM request to {url} failed: {exc}") from exc + time.sleep(_RETRY_BACKOFF_S[attempt]) + + raise LLMError(f"LLM request to {url} failed: {last_error}") + + +def _load_json_object(raw: bytes) -> dict[str, Any]: + try: + payload = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: + raise LLMError(f"LLM response was not valid JSON: {exc}") from exc + if not isinstance(payload, dict): + raise LLMError("LLM response must be a JSON object") + return payload + + +def _parse_chat_result(payload: dict[str, Any]) -> ChatResult: + choices = payload.get("choices") + if not isinstance(choices, list) or not choices: + raise LLMError("LLM response missing choices") + choice = choices[0] + if not isinstance(choice, dict): + raise LLMError("LLM choice must be an object") + message = choice.get("message") or {} + if not isinstance(message, dict): + raise LLMError("LLM message must be an object") + + content = message.get("content") + if content is not None: + content = str(content) + + tool_calls: list[ToolCall] = [] + for item in message.get("tool_calls") or []: + if not isinstance(item, dict): + continue + function = item.get("function") or {} + if not isinstance(function, dict): + continue + tool_calls.append( + ToolCall( + id=str(item.get("id") or ""), + name=str(function.get("name") or ""), + arguments=str(function.get("arguments") or ""), + ) + ) + + usage = None + raw_usage = payload.get("usage") + if isinstance(raw_usage, dict): + usage = Usage( + prompt_tokens=_optional_int(raw_usage.get("prompt_tokens")), + completion_tokens=_optional_int(raw_usage.get("completion_tokens")), + total_tokens=_optional_int(raw_usage.get("total_tokens")), + ) + + finish = choice.get("finish_reason") + return ChatResult( + content=content, + tool_calls=tool_calls, + finish_reason=str(finish) if finish is not None else None, + usage=usage, + ) + + +def _optional_int(value: Any) -> int | None: + if value is None: + return None + return int(value) diff --git a/tests/test_llm.py b/tests/test_llm.py new file mode 100644 index 0000000..cfac725 --- /dev/null +++ b/tests/test_llm.py @@ -0,0 +1,259 @@ +"""Unit tests for LLMClient (TDD §2.7 / §22 M2.4).""" + +from __future__ import annotations + +import json +from typing import Any +from unittest.mock import MagicMock, patch +from urllib.error import HTTPError + +import pytest + +from leanci.errors import LLMError +from leanci.llm import ( + DEFAULT_COMPRESSED_BASE_URL, + DEFAULT_UNCOMPRESSED_BASE_URL, + Binding, + ChatResult, + LLMClient, + ToolCall, +) + + +def _http_response(payload: dict[str, Any], *, status: int = 200) -> MagicMock: + body = json.dumps(payload).encode("utf-8") + response = MagicMock() + response.status = status + response.read.return_value = body + response.__enter__.return_value = response + response.__exit__.return_value = False + return response + + +def _assistant_payload( + *, + content: str | None = "hello", + tool_calls: list[dict[str, Any]] | None = None, + finish_reason: str = "stop", + usage: dict[str, int] | None = None, +) -> dict[str, Any]: + message: dict[str, Any] = {"role": "assistant", "content": content} + if tool_calls is not None: + message["tool_calls"] = tool_calls + return { + "choices": [{"index": 0, "message": message, "finish_reason": finish_reason}], + "usage": usage + or {"prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15}, + } + + +def test_default_binding_is_compressed() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + + assert client.binding is Binding.COMPRESSED + assert client.base_url == DEFAULT_COMPRESSED_BASE_URL + + +def test_uncompressed_binding_uses_provider_base_url() -> None: + client = LLMClient( + api_key="sk", + model="gpt-4.1-mini", + binding=Binding.UNCOMPRESSED, + ) + + assert client.binding is Binding.UNCOMPRESSED + assert client.base_url == DEFAULT_UNCOMPRESSED_BASE_URL + + +def test_explicit_base_url_overrides_binding_default() -> None: + client = LLMClient( + api_key="sk", + model="gpt-4.1-mini", + binding=Binding.COMPRESSED, + base_url="http://127.0.0.1:9999/v1", + ) + + assert client.base_url == "http://127.0.0.1:9999/v1" + + +def test_from_env_compressed_prefers_openai_base_url() -> None: + client = LLMClient.from_env( + { + "OPENAI_API_KEY": "sk", + "OPENAI_BASE_URL": "http://127.0.0.1:8080/v1", + "LEANCI_MODEL": "gpt-4.1-mini", + }, + binding=Binding.COMPRESSED, + ) + + assert client.binding is Binding.COMPRESSED + assert client.base_url == "http://127.0.0.1:8080/v1" + assert client.api_key == "sk" + + +def test_from_env_uncompressed_ignores_proxy_base_url() -> None: + client = LLMClient.from_env( + { + "OPENAI_API_KEY": "sk", + "OPENAI_BASE_URL": "http://127.0.0.1:8080/v1", + "LEANCI_MODEL": "gpt-4.1-mini", + }, + binding=Binding.UNCOMPRESSED, + ) + + assert client.base_url == DEFAULT_UNCOMPRESSED_BASE_URL + + +def test_chat_returns_typed_result_with_content_and_usage() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + payload = _assistant_payload(content="review ok") + + with patch("leanci.llm.urlopen", return_value=_http_response(payload)) as urlopen: + result = client.chat([{"role": "user", "content": "hi"}]) + + assert isinstance(result, ChatResult) + assert result.content == "review ok" + assert result.tool_calls == [] + assert result.finish_reason == "stop" + assert result.usage is not None + assert result.usage.prompt_tokens == 10 + assert result.usage.completion_tokens == 5 + assert result.usage.total_tokens == 15 + + request = urlopen.call_args.args[0] + assert request.full_url == f"{DEFAULT_COMPRESSED_BASE_URL}/chat/completions" + assert request.get_header("Authorization") == "Bearer sk" + body = json.loads(request.data.decode("utf-8")) + assert body["model"] == "gpt-4.1-mini" + assert body["messages"] == [{"role": "user", "content": "hi"}] + + +def test_chat_parses_tool_calls() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + payload = _assistant_payload( + content=None, + finish_reason="tool_calls", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": { + "name": "read_file", + "arguments": '{"path": "src/a.py"}', + }, + } + ], + ) + + with patch("leanci.llm.urlopen", return_value=_http_response(payload)): + result = client.chat( + [{"role": "user", "content": "inspect"}], + tools=[ + { + "type": "function", + "function": { + "name": "read_file", + "parameters": {"type": "object"}, + }, + } + ], + ) + + assert result.content is None + assert result.finish_reason == "tool_calls" + assert result.tool_calls == [ + ToolCall(id="call_1", name="read_file", arguments='{"path": "src/a.py"}') + ] + + +def test_chat_includes_tools_in_request_body() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + tools = [{"type": "function", "function": {"name": "list_files", "parameters": {}}}] + + with patch( + "leanci.llm.urlopen", + return_value=_http_response(_assistant_payload()), + ) as urlopen: + client.chat([{"role": "user", "content": "x"}], tools=tools) + + body = json.loads(urlopen.call_args.args[0].data.decode("utf-8")) + assert body["tools"] == tools + + +def test_chat_retries_on_429_then_succeeds() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + err = HTTPError( + url="http://x", + code=429, + msg="rate limited", + hdrs=None, # type: ignore[arg-type] + fp=None, + ) + ok = _http_response(_assistant_payload(content="ok")) + + with ( + patch("leanci.llm.urlopen", side_effect=[err, ok]), + patch("leanci.llm.time.sleep") as sleep, + ): + result = client.chat([{"role": "user", "content": "hi"}]) + + assert result.content == "ok" + sleep.assert_called_once_with(2) + + +def test_chat_retries_twice_on_5xx_then_raises() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + err = HTTPError( + url="http://x", + code=503, + msg="unavailable", + hdrs=None, # type: ignore[arg-type] + fp=None, + ) + + with ( + patch("leanci.llm.urlopen", side_effect=[err, err, err]) as urlopen, + patch("leanci.llm.time.sleep") as sleep, + ): + with pytest.raises(LLMError, match="503"): + client.chat([{"role": "user", "content": "hi"}]) + + assert urlopen.call_count == 3 # initial + 2 retries + assert sleep.call_args_list[0].args == (2,) + assert sleep.call_args_list[1].args == (8,) + + +def test_chat_raises_llm_error_on_invalid_json() -> None: + client = LLMClient(api_key="sk", model="gpt-4.1-mini") + response = MagicMock() + response.status = 200 + response.read.return_value = b"not-json" + response.__enter__.return_value = response + response.__exit__.return_value = False + + with patch("leanci.llm.urlopen", return_value=response): + with pytest.raises(LLMError, match="JSON"): + client.chat([{"role": "user", "content": "hi"}]) + + +def test_chat_requires_api_key() -> None: + with pytest.raises(LLMError, match="api key|API key|OPENAI_API_KEY"): + LLMClient(api_key="", model="gpt-4.1-mini") + + +def test_uncompressed_chat_posts_to_provider_url() -> None: + client = LLMClient( + api_key="sk", + model="gpt-4.1-mini", + binding=Binding.UNCOMPRESSED, + ) + + with patch( + "leanci.llm.urlopen", + return_value=_http_response(_assistant_payload()), + ) as urlopen: + client.chat([{"role": "user", "content": "hi"}]) + + assert urlopen.call_args.args[0].full_url == ( + f"{DEFAULT_UNCOMPRESSED_BASE_URL}/chat/completions" + ) From 9aa4d018a4777849eaae9bafa7407606a179f1fb Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:00 +0530 Subject: [PATCH 04/29] feat: implement agent review loop and prompt templates Run tool-calling turns against ToolHost, repair invalid JSON once, and package prompt markdown for installable builds. --- pyproject.toml | 4 + src/leanci/agent.py | 291 +++++++++++++++++++++++++++++ src/leanci/prompts/__init__.py | 1 + src/leanci/prompts/review_user.md | 20 ++ src/leanci/prompts/system.md | 32 ++++ tests/test_agent.py | 293 ++++++++++++++++++++++++++++++ 6 files changed, 641 insertions(+) create mode 100644 src/leanci/agent.py create mode 100644 src/leanci/prompts/__init__.py create mode 100644 src/leanci/prompts/review_user.md create mode 100644 src/leanci/prompts/system.md create mode 100644 tests/test_agent.py diff --git a/pyproject.toml b/pyproject.toml index 2767619..0a43641 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ dependencies = [] [project.optional-dependencies] dev = ["pytest>=8.0"] +action = [] [project.scripts] leanci = "leanci.__main__:cli" @@ -19,6 +20,9 @@ leanci = "leanci.__main__:cli" [tool.setuptools.packages.find] where = ["src"] +[tool.setuptools.package-data] +leanci = ["prompts/*.md"] + [tool.pytest.ini_options] testpaths = ["tests"] addopts = "-q" diff --git a/src/leanci/agent.py b/src/leanci/agent.py new file mode 100644 index 0000000..89630d1 --- /dev/null +++ b/src/leanci/agent.py @@ -0,0 +1,291 @@ +"""Agent runtime: review loop + prompts + one JSON repair pass (TDD §2.9 / §8 / §11). + +Does not normalize findings into typed ``Finding`` objects — that is M2.6. +""" + +from __future__ import annotations + +import json +from dataclasses import asdict, dataclass, field +from importlib import resources +from pathlib import Path +from typing import Any, Protocol + +from leanci.llm import ChatResult, LLMClient, ToolCall +from leanci.models import ContextManifest, DiffBundle, RunConfig +from leanci.tools import ToolHost, ToolResult + +_DIFF_CHAR_CAP = 80_000 +_REPAIR_INSTRUCTION = ( + "Return only valid JSON matching the schema from the system prompt. " + "No markdown, no commentary." +) + +TOOL_SCHEMAS: list[dict[str, Any]] = [ + { + "type": "function", + "function": { + "name": "list_files", + "description": "List files under the repo (optional path/glob).", + "parameters": { + "type": "object", + "properties": { + "path": {"type": "string"}, + "glob": {"type": "string"}, + }, + }, + }, + }, + { + "type": "function", + "function": { + "name": "read_file", + "description": "Read a UTF-8 file slice by path.", + "parameters": { + "type": "object", + "properties": { + "path": {"type": "string"}, + "start_line": {"type": "integer"}, + "end_line": {"type": "integer"}, + }, + "required": ["path"], + }, + }, + }, + { + "type": "function", + "function": { + "name": "search_repo", + "description": "ripgrep content search in the repo.", + "parameters": { + "type": "object", + "properties": { + "query": {"type": "string"}, + "glob": {"type": "string"}, + "max_hits": {"type": "integer"}, + }, + "required": ["query"], + }, + }, + }, + { + "type": "function", + "function": { + "name": "read_manifest", + "description": "Return the ContextManifest JSON.", + "parameters": {"type": "object", "properties": {}}, + }, + }, +] + + +class SupportsChat(Protocol): + def chat( + self, + messages: list[dict[str, Any]], + tools: list[dict[str, Any]] | None = None, + **opts: Any, + ) -> ChatResult: ... + + +class SupportsTools(Protocol): + def dispatch(self, name: str, args: dict[str, Any] | None = None) -> ToolResult: ... + + +@dataclass +class ReviewResult: + """Outcome of one agent review run (raw findings until FindingNormalizer).""" + + findings: list[dict[str, Any]] = field(default_factory=list) + turns: int = 0 + stop_reason: str = "" + raw_content: str | None = None + raw_trace_ref: str | None = None + notes: str | None = None + + +def run_review( + cfg: RunConfig, + manifest: ContextManifest, + diff: DiffBundle, + llm: SupportsChat | LLMClient, + tools: SupportsTools | ToolHost, +) -> ReviewResult: + """Run the tool loop until JSON findings, caps, or empty expansion (TDD §8.1).""" + if not manifest.entries and not diff.changed_files and not diff.patch_text.strip(): + return ReviewResult(findings=[], turns=0, stop_reason="empty_expansion") + + messages: list[dict[str, Any]] = [ + {"role": "system", "content": load_system_prompt()}, + {"role": "user", "content": render_user_prompt(cfg, manifest, diff)}, + ] + max_turns = cfg.caps.max_tool_turns + turns = 0 + + for _ in range(max_turns): + resp = llm.chat(messages, tools=TOOL_SCHEMAS) + turns += 1 + messages.append(_assistant_message(resp)) + + if resp.tool_calls: + for call in resp.tool_calls: + result = _dispatch_tool(tools, call) + messages.append( + { + "role": "tool", + "tool_call_id": call.id, + "content": result.content, + } + ) + continue + + parsed = _try_parse_findings_payload(resp.content) + if parsed is not None: + return _result_from_payload(parsed, turns=turns, raw_content=resp.content) + + # One JSON repair pass (TDD §11.5) — does not consume extra tool-turn budget + # beyond this follow-up chat call, counted in ``turns``. + messages.append({"role": "user", "content": _REPAIR_INSTRUCTION}) + repair = llm.chat(messages, tools=None) + turns += 1 + messages.append(_assistant_message(repair)) + repaired = _try_parse_findings_payload(repair.content) + if repaired is not None: + return _result_from_payload(repaired, turns=turns, raw_content=repair.content) + return ReviewResult( + findings=[], + turns=turns, + stop_reason="invalid_json", + raw_content=repair.content, + ) + + return ReviewResult( + findings=[], + turns=turns, + stop_reason="max_turns", + raw_content=_last_assistant_content(messages), + ) + + +def load_system_prompt() -> str: + return _read_prompt("system.md") + + +def render_user_prompt(cfg: RunConfig, manifest: ContextManifest, diff: DiffBundle) -> str: + template = _read_prompt("review_user.md") + patch = diff.patch_text + if len(patch) > _DIFF_CHAR_CAP: + patch = patch[:_DIFF_CHAR_CAP] + "\n\n[diff truncated; use tools to read files]\n" + manifest_json = json.dumps( + { + "entries": [asdict(entry) for entry in manifest.entries], + "bytes_total": manifest.bytes_total, + "cap_hits": list(manifest.cap_hits), + }, + ensure_ascii=False, + indent=2, + sort_keys=True, + ) + return ( + template.replace("{{repo}}", cfg.repo) + .replace("{{pr_number}}", str(cfg.pr_number if cfg.pr_number is not None else "n/a")) + .replace("{{base_sha}}", cfg.base_sha) + .replace("{{head_sha}}", cfg.head_sha) + .replace("{{model}}", cfg.model) + .replace("{{severity_floor}}", str(cfg.severity_floor)) + .replace("{{max_findings}}", str(cfg.caps.max_findings)) + .replace("{{max_tool_turns}}", str(cfg.caps.max_tool_turns)) + .replace("{{diff}}", patch or "(empty diff)") + .replace("{{manifest_json}}", manifest_json) + ) + + +def _read_prompt(name: str) -> str: + # Prefer package resources; fall back to source tree for editable installs. + try: + root = resources.files("leanci.prompts") + return root.joinpath(name).read_text(encoding="utf-8") + except (FileNotFoundError, ModuleNotFoundError, AttributeError): + path = Path(__file__).resolve().parent / "prompts" / name + return path.read_text(encoding="utf-8") + + +def _assistant_message(resp: ChatResult) -> dict[str, Any]: + message: dict[str, Any] = { + "role": "assistant", + "content": resp.content, + } + if resp.tool_calls: + message["tool_calls"] = [ + { + "id": call.id, + "type": "function", + "function": {"name": call.name, "arguments": call.arguments}, + } + for call in resp.tool_calls + ] + return message + + +def _dispatch_tool(tools: SupportsTools, call: ToolCall) -> ToolResult: + try: + args = json.loads(call.arguments) if call.arguments else {} + if not isinstance(args, dict): + args = {} + except json.JSONDecodeError: + args = {} + return tools.dispatch(call.name, args) + + +def _try_parse_findings_payload(content: str | None) -> dict[str, Any] | None: + if content is None: + return None + text = content.strip() + if not text: + return None + # Allow accidental fenced blocks without implementing a full normalizer. + if text.startswith("```"): + lines = text.splitlines() + if lines and lines[0].startswith("```"): + lines = lines[1:] + if lines and lines[-1].strip() == "```": + lines = lines[:-1] + text = "\n".join(lines).strip() + try: + payload = json.loads(text) + except json.JSONDecodeError: + return None + if not isinstance(payload, dict): + return None + findings = payload.get("findings") + if not isinstance(findings, list): + return None + if not all(isinstance(item, dict) for item in findings): + return None + return payload + + +def _result_from_payload( + payload: dict[str, Any], + *, + turns: int, + raw_content: str | None, +) -> ReviewResult: + findings = list(payload.get("findings") or []) + notes = payload.get("notes") + stop = "no_findings" if not findings else "success" + return ReviewResult( + findings=findings, + turns=turns, + stop_reason=stop, + raw_content=raw_content, + notes=str(notes) if notes is not None else None, + ) + + +def _last_assistant_content(messages: list[dict[str, Any]]) -> str | None: + for message in reversed(messages): + if message.get("role") == "assistant": + content = message.get("content") + return str(content) if content is not None else None + return None diff --git a/src/leanci/prompts/__init__.py b/src/leanci/prompts/__init__.py new file mode 100644 index 0000000..4a18915 --- /dev/null +++ b/src/leanci/prompts/__init__.py @@ -0,0 +1 @@ +"""Prompt templates for the review agent (TDD §11).""" diff --git a/src/leanci/prompts/review_user.md b/src/leanci/prompts/review_user.md new file mode 100644 index 0000000..4eb9e96 --- /dev/null +++ b/src/leanci/prompts/review_user.md @@ -0,0 +1,20 @@ +## PR metadata +- repository: {{repo}} +- pull request: {{pr_number}} +- base: {{base_sha}} +- head: {{head_sha}} +- model: {{model}} +- severity_floor: {{severity_floor}} +- max findings: {{max_findings}} +- max tool turns: {{max_tool_turns}} + +## Unified diff (truncated if huge) +{{diff}} + +## Context manifest (priority files) +{{manifest_json}} + +## Instructions +1. Use tools to read the changed file(s) and at least one related caller/import/test from the manifest. +2. Focus on high-signal issues only. +3. Return JSON only matching the system schema when done. diff --git a/src/leanci/prompts/system.md b/src/leanci/prompts/system.md new file mode 100644 index 0000000..7f2e940 --- /dev/null +++ b/src/leanci/prompts/system.md @@ -0,0 +1,32 @@ +# Dependency-aware pull request reviewer for Python. + +You review PRs by expanding beyond the diff: changed files, imports, call sites, and related tests. + +Rules: +- High-signal only: correctness, regressions, API contract breaks, security footguns, missing tests. +- Ignore style/nits unless security-relevant. +- Use tools to read files and search; do not invent file contents. +- Inspect the changed file and at least one related non-diff file from the manifest (caller, import, or test) before concluding. +- Prefer cross-module contract breaks. +- Final answer must be JSON only matching the schema below (no markdown fences). +- If a line number is uncertain, set `"line": null` and cite `file` + `symbol`. +- Disclosure: AI may be wrong; humans remain merge authority. + +Output schema: +{ + "findings": [ + { + "severity": "critical|high|medium|low", + "title": "short title", + "file": "path", + "line": 42, + "symbol": "name_or_null", + "category": "correctness|regression|api_contract|security|missing_tests", + "rationale": "why, citing modules", + "fix_sketch": "optional short fix or null" + } + ], + "notes": "optional short note" +} + +If there are no issues, return `{"findings": [], "notes": "..."}`. diff --git a/tests/test_agent.py b/tests/test_agent.py new file mode 100644 index 0000000..b0de9c9 --- /dev/null +++ b/tests/test_agent.py @@ -0,0 +1,293 @@ +"""Unit tests for AgentRuntime (TDD §2.9 / §8 / §11 — M2.5).""" + +from __future__ import annotations + +import json +from typing import Any + +from leanci.agent import ReviewResult, run_review +from leanci.llm import ChatResult, ToolCall +from leanci.models import ( + Caps, + ContextManifest, + DiffBundle, + FileRole, + ManifestEntry, + Mode, + RunConfig, + Severity, +) +from leanci.tools import ToolResult + + +def _config(**overrides: object) -> RunConfig: + caps = Caps(max_tool_turns=4, max_findings=8) + base: dict[str, object] = { + "repo_root": "/repo", + "base_sha": "a" * 40, + "head_sha": "b" * 40, + "repo": "acme/widgets", + "pr_number": 7, + "mode": Mode.PARITOK, + "model": "gpt-4.1-mini", + "caps": caps, + "severity_floor": Severity.MEDIUM, + "fail_on_error": True, + "pricing_version": "2026-07-30", + "openai_api_key": "sk", + } + base.update(overrides) + return RunConfig(**base) # type: ignore[arg-type] + + +def _manifest(*paths: str) -> ContextManifest: + entries = [ + ManifestEntry( + path=path, + role=FileRole.SEED if i == 0 else FileRole.CALL_SITE, + score=10 - i, + order=i, + estimated_bytes=10, + reason="changed" if i == 0 else "caller", + ) + for i, path in enumerate(paths) + ] + return ContextManifest(entries=entries, bytes_total=10 * len(entries)) + + +def _diff(text: str = "diff --git a/x b/x\n+hello\n") -> DiffBundle: + return DiffBundle(changed_files=["src/a.py"], patch_text=text) + + +class ScriptedLLM: + def __init__(self, responses: list[ChatResult]) -> None: + self.responses = list(responses) + self.calls: list[dict[str, Any]] = [] + + def chat( + self, + messages: list[dict[str, Any]], + tools: list[dict[str, Any]] | None = None, + **opts: Any, + ) -> ChatResult: + self.calls.append( + { + "messages": [dict(message) for message in messages], + "tools": tools, + "opts": opts, + } + ) + if not self.responses: + raise AssertionError("unexpected LLM call") + return self.responses.pop(0) + + +class RecordingTools: + def __init__(self) -> None: + self.calls: list[tuple[str, dict[str, Any]]] = [] + + def dispatch(self, name: str, args: dict[str, Any] | None = None) -> ToolResult: + payload = dict(args or {}) + self.calls.append((name, payload)) + return ToolResult(content=f"ok:{name}:{payload.get('path', '')}", byte_size=3, ok=True) + + +def test_empty_expansion_skips_agent_without_llm_calls() -> None: + llm = ScriptedLLM([]) + tools = RecordingTools() + + result = run_review( + _config(), + ContextManifest(), + DiffBundle(), + llm, # type: ignore[arg-type] + tools, # type: ignore[arg-type] + ) + + assert isinstance(result, ReviewResult) + assert result.stop_reason == "empty_expansion" + assert result.findings == [] + assert result.turns == 0 + assert llm.calls == [] + assert tools.calls == [] + + +def test_final_json_findings_stops_successfully() -> None: + findings = { + "findings": [ + { + "severity": "high", + "title": "Broken contract", + "file": "src/a.py", + "line": 1, + "symbol": "f", + "category": "api_contract", + "rationale": "caller mismatch", + "fix_sketch": None, + } + ], + "notes": "ok", + } + llm = ScriptedLLM([ChatResult(content=json.dumps(findings), finish_reason="stop")]) + tools = RecordingTools() + + result = run_review(_config(), _manifest("src/a.py", "src/b.py"), _diff(), llm, tools) # type: ignore[arg-type] + + assert result.stop_reason == "success" + assert result.turns == 1 + assert result.findings == findings["findings"] + assert result.notes == "ok" + assert "system" == llm.calls[0]["messages"][0]["role"] + assert "user" == llm.calls[0]["messages"][1]["role"] + assert llm.calls[0]["tools"] # tool schemas provided + user_text = llm.calls[0]["messages"][1]["content"] + assert "src/a.py" in user_text + assert "diff --git" in user_text + + +def test_no_findings_signal_is_success_with_empty_list() -> None: + llm = ScriptedLLM( + [ChatResult(content=json.dumps({"findings": [], "notes": "clean"}), finish_reason="stop")] + ) + + result = run_review( + _config(), + _manifest("src/a.py"), + _diff(), + llm, # type: ignore[arg-type] + RecordingTools(), # type: ignore[arg-type] + ) + + assert result.stop_reason == "no_findings" + assert result.findings == [] + + +def test_tool_calls_are_dispatched_and_appended_before_final_answer() -> None: + llm = ScriptedLLM( + [ + ChatResult( + content=None, + finish_reason="tool_calls", + tool_calls=[ + ToolCall( + id="c1", + name="read_file", + arguments='{"path": "src/a.py"}', + ) + ], + ), + ChatResult( + content=json.dumps({"findings": [], "notes": "after tools"}), + finish_reason="stop", + ), + ] + ) + tools = RecordingTools() + + result = run_review(_config(), _manifest("src/a.py"), _diff(), llm, tools) # type: ignore[arg-type] + + assert result.stop_reason == "no_findings" + assert result.turns == 2 + assert tools.calls == [("read_file", {"path": "src/a.py"})] + # Assistant + tool messages present before final call + second_messages = llm.calls[1]["messages"] + roles = [m["role"] for m in second_messages] + assert roles.count("assistant") >= 1 + assert roles.count("tool") >= 1 + + +def test_invalid_json_triggers_one_repair_pass() -> None: + llm = ScriptedLLM( + [ + ChatResult(content="not json at all", finish_reason="stop"), + ChatResult( + content=json.dumps( + { + "findings": [ + { + "severity": "medium", + "title": "Fixed", + "file": "src/a.py", + "line": None, + "symbol": None, + "category": "correctness", + "rationale": "r", + "fix_sketch": None, + } + ] + } + ), + finish_reason="stop", + ), + ] + ) + + result = run_review( + _config(), + _manifest("src/a.py"), + _diff(), + llm, # type: ignore[arg-type] + RecordingTools(), # type: ignore[arg-type] + ) + + assert result.stop_reason == "success" + assert len(result.findings) == 1 + assert len(llm.calls) == 2 + repair_user = llm.calls[1]["messages"][-1] + assert repair_user["role"] == "user" + assert "valid JSON" in repair_user["content"] + + +def test_repair_failure_returns_invalid_json_stop_reason() -> None: + llm = ScriptedLLM( + [ + ChatResult(content="still bad", finish_reason="stop"), + ChatResult(content="also bad", finish_reason="stop"), + ] + ) + + result = run_review( + _config(), + _manifest("src/a.py"), + _diff(), + llm, # type: ignore[arg-type] + RecordingTools(), # type: ignore[arg-type] + ) + + assert result.stop_reason == "invalid_json" + assert result.findings == [] + assert result.raw_content == "also bad" + assert len(llm.calls) == 2 + + +def test_max_tool_turns_stops_with_partial() -> None: + forever = ChatResult( + content=None, + finish_reason="tool_calls", + tool_calls=[ToolCall(id="c", name="list_files", arguments="{}")], + ) + llm = ScriptedLLM([forever, forever, forever]) + cfg = _config(caps=Caps(max_tool_turns=2, max_findings=8)) + + result = run_review(cfg, _manifest("src/a.py"), _diff(), llm, RecordingTools()) # type: ignore[arg-type] + + assert result.stop_reason == "max_turns" + assert result.turns == 2 + assert len(llm.calls) == 2 + + +def test_prompts_are_loaded_from_package_templates() -> None: + from leanci.agent import load_system_prompt, render_user_prompt + + system = load_system_prompt() + assert "dependency-aware" in system.lower() or "PR reviewer" in system + assert "JSON" in system + + user = render_user_prompt( + _config(), + _manifest("src/a.py"), + _diff("diff --git a/src/a.py b/src/a.py\n+x\n"), + ) + assert "src/a.py" in user + assert "max findings" in user.lower() or "8" in user + assert "medium" in user.lower() From 3569ba79b0f0f9654cfa443f7f15fe609aed7d26 Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:00 +0530 Subject: [PATCH 05/29] feat: normalize agent findings with floor, dedupe, and caps Validate raw JSON into typed Finding objects and enforce severity floor plus max_findings. --- src/leanci/findings.py | 142 ++++++++++++++++++++++++++++++++++++ tests/test_findings.py | 159 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 301 insertions(+) create mode 100644 src/leanci/findings.py create mode 100644 tests/test_findings.py diff --git a/src/leanci/findings.py b/src/leanci/findings.py new file mode 100644 index 0000000..8317369 --- /dev/null +++ b/src/leanci/findings.py @@ -0,0 +1,142 @@ +"""Finding normalizer (TDD §2.11). + +Parse/validate agent JSON into typed ``Finding`` objects; apply severity floor, +dedupe, and ``max_findings``. +""" + +from __future__ import annotations + +import hashlib +import json +from collections.abc import Mapping, Sequence +from typing import Any + +from leanci.models import Finding, RunConfig, Severity + +_SEVERITY_RANK = { + Severity.CRITICAL: 3, + Severity.HIGH: 2, + Severity.MEDIUM: 1, + Severity.LOW: 0, +} + +_REQUIRED = ("severity", "title", "file", "rationale", "category") + + +def normalize_findings(raw: Any, cfg: RunConfig) -> list[Finding]: + """Validate raw agent output into capped, deduped ``Finding`` values.""" + items = _extract_items(raw) + floor = cfg.severity_floor + selected: list[Finding] = [] + seen: set[tuple[str, str, int | None]] = set() + + for item in items: + finding = _coerce_finding(item) + if finding is None: + continue + if _SEVERITY_RANK[finding.severity] < _SEVERITY_RANK[floor]: + continue + key = (finding.file, finding.title, finding.line) + if key in seen: + continue + seen.add(key) + selected.append(finding) + if len(selected) >= cfg.caps.max_findings: + break + + return selected + + +def _extract_items(raw: Any) -> list[dict[str, Any]]: + if raw is None: + return [] + if isinstance(raw, Finding): + return [] + if isinstance(raw, str): + text = raw.strip() + if not text: + return [] + if text.startswith("```"): + lines = text.splitlines() + if lines and lines[0].startswith("```"): + lines = lines[1:] + if lines and lines[-1].strip() == "```": + lines = lines[:-1] + text = "\n".join(lines).strip() + try: + raw = json.loads(text) + except json.JSONDecodeError: + return [] + + if isinstance(raw, Mapping): + findings = raw.get("findings", raw) + if isinstance(findings, Mapping): + return [] + raw = findings + + if isinstance(raw, Sequence) and not isinstance(raw, (str, bytes)): + return [item for item in raw if isinstance(item, Mapping)] + + return [] + + +def _coerce_finding(item: Mapping[str, Any]) -> Finding | None: + for key in _REQUIRED: + value = item.get(key) + if value is None or (isinstance(value, str) and not value.strip()): + return None + + try: + severity = Severity(str(item["severity"]).strip().lower()) + except ValueError: + return None + + line = item.get("line") + if line is not None and line != "": + try: + line = int(line) + except (TypeError, ValueError): + return None + else: + line = None + + symbol = item.get("symbol") + if symbol is not None: + symbol = str(symbol) + if not symbol.strip(): + symbol = None + + fix = item.get("fix_sketch") + if fix is not None: + fix = str(fix) + if not fix.strip(): + fix = None + + title = str(item["title"]).strip() + file_path = str(item["file"]).strip() + rationale = str(item["rationale"]).strip() + category = str(item["category"]).strip() + finding_id = _stable_id(file_path, title, line, severity) + + return Finding( + id=finding_id, + severity=severity, + title=title, + file=file_path, + rationale=rationale, + category=category, + line=line, + symbol=symbol, + fix_sketch=fix, + ) + + +def _stable_id( + file_path: str, + title: str, + line: int | None, + severity: Severity, +) -> str: + material = f"{file_path}|{title}|{line}|{severity}" + digest = hashlib.sha1(material.encode("utf-8")).hexdigest()[:10] + return f"f{digest}" diff --git a/tests/test_findings.py b/tests/test_findings.py new file mode 100644 index 0000000..7d42592 --- /dev/null +++ b/tests/test_findings.py @@ -0,0 +1,159 @@ +"""Unit tests for FindingNormalizer (TDD §2.11 / §22 M2.6).""" + +from __future__ import annotations + +import json + +from leanci.findings import normalize_findings +from leanci.models import Caps, Finding, Mode, RunConfig, Severity + + +def _cfg(**overrides: object) -> RunConfig: + caps = Caps(max_findings=8) + base: dict[str, object] = { + "repo_root": "/repo", + "base_sha": "a" * 40, + "head_sha": "b" * 40, + "repo": "acme/widgets", + "pr_number": 7, + "mode": Mode.PARITOK, + "model": "gpt-4.1-mini", + "caps": caps, + "severity_floor": Severity.MEDIUM, + "fail_on_error": True, + "pricing_version": "2026-07-30", + } + base.update(overrides) + return RunConfig(**base) # type: ignore[arg-type] + + +def _raw_finding(**overrides: object) -> dict[str, object]: + item: dict[str, object] = { + "severity": "high", + "title": "Broken contract", + "file": "src/a.py", + "line": 10, + "symbol": "checkout", + "category": "api_contract", + "rationale": "caller assumes legacy return", + "fix_sketch": "update caller", + } + item.update(overrides) + return item + + +def test_normalize_json_string_into_typed_findings() -> None: + raw = json.dumps({"findings": [_raw_finding()]}) + + findings = normalize_findings(raw, _cfg()) + + assert len(findings) == 1 + finding = findings[0] + assert isinstance(finding, Finding) + assert finding.severity is Severity.HIGH + assert finding.title == "Broken contract" + assert finding.file == "src/a.py" + assert finding.line == 10 + assert finding.symbol == "checkout" + assert finding.category == "api_contract" + assert finding.rationale == "caller assumes legacy return" + assert finding.fix_sketch == "update caller" + assert finding.id.startswith("f") + + +def test_normalize_accepts_dict_or_list_payloads() -> None: + as_dict = normalize_findings({"findings": [_raw_finding(title="A")]}, _cfg()) + as_list = normalize_findings([_raw_finding(title="B")], _cfg()) + + assert [f.title for f in as_dict] == ["A"] + assert [f.title for f in as_list] == ["B"] + + +def test_filters_below_severity_floor() -> None: + raw = { + "findings": [ + _raw_finding(severity="critical", title="C"), + _raw_finding(severity="high", title="H"), + _raw_finding(severity="medium", title="M"), + _raw_finding(severity="low", title="L"), + ] + } + + findings = normalize_findings(raw, _cfg(severity_floor=Severity.HIGH)) + + assert [f.title for f in findings] == ["C", "H"] + + +def test_enforces_max_findings_preserving_order() -> None: + raw = { + "findings": [ + _raw_finding(title=f"F{i}", file=f"src/{i}.py") for i in range(5) + ] + } + cfg = _cfg(caps=Caps(max_findings=3)) + + findings = normalize_findings(raw, cfg) + + assert [f.title for f in findings] == ["F0", "F1", "F2"] + + +def test_dedupes_identical_file_title_line() -> None: + raw = { + "findings": [ + _raw_finding(title="Dup", file="src/a.py", line=3), + _raw_finding(title="Dup", file="src/a.py", line=3, rationale="second"), + _raw_finding(title="Other", file="src/a.py", line=3), + ] + } + + findings = normalize_findings(raw, _cfg()) + + assert [f.title for f in findings] == ["Dup", "Other"] + assert findings[0].rationale == "caller assumes legacy return" + + +def test_skips_invalid_entries_without_failing() -> None: + raw = { + "findings": [ + {"severity": "high"}, # missing required fields + _raw_finding(severity="nope", title="BadSev"), + _raw_finding(title="Good"), + ] + } + + findings = normalize_findings(raw, _cfg()) + + assert [f.title for f in findings] == ["Good"] + + +def test_nullable_fields_and_empty_input() -> None: + assert normalize_findings({"findings": []}, _cfg()) == [] + assert normalize_findings(None, _cfg()) == [] + assert normalize_findings("not-json", _cfg()) == [] + + findings = normalize_findings( + { + "findings": [ + _raw_finding(line=None, symbol=None, fix_sketch=None), + ] + }, + _cfg(), + ) + assert findings[0].line is None + assert findings[0].symbol is None + assert findings[0].fix_sketch is None + + +def test_ids_are_stable_and_deterministic() -> None: + raw = { + "findings": [ + _raw_finding(title="A", file="a.py"), + _raw_finding(title="B", file="b.py"), + ] + } + + first = normalize_findings(raw, _cfg()) + second = normalize_findings(raw, _cfg()) + + assert [f.id for f in first] == [f.id for f in second] + assert first[0].id != first[1].id From 8c5649afb1d4b11ff4b49f69a938d88a5d462235 Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:11 +0530 Subject: [PATCH 06/29] feat: add dated pricing table for cost estimates Provide model rate lookups and assumption labels used by honest review receipts. --- src/leanci/pricing.py | 120 ++++++++++++++++++++++++++++++++++++++++++ tests/test_pricing.py | 87 ++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 src/leanci/pricing.py create mode 100644 tests/test_pricing.py diff --git a/src/leanci/pricing.py b/src/leanci/pricing.py new file mode 100644 index 0000000..d43cf1d --- /dev/null +++ b/src/leanci/pricing.py @@ -0,0 +1,120 @@ +"""Dated model pricing table and cost helpers for receipts (TDD §7.3 / §22 M3.1). + +ReceiptBuilder (M3.2) consumes these rates; this module does not build receipts. +Rates are assumptions labeled for honesty — update ``PRICING_TABLES`` when vendor +prices change and bump the version date. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +DEFAULT_PRICING_VERSION = "2026-07-30" +DEFAULT_MODEL = "gpt-4.1-mini" + + +@dataclass(frozen=True) +class ModelRate: + """USD per 1M tokens for one model, as of a pricing table version date.""" + + model: str + input_usd_per_1m: float + output_usd_per_1m: float + as_of: str + + +@dataclass(frozen=True) +class PricingTable: + """Versioned rate card used by cost estimates.""" + + version: str + rates: dict[str, ModelRate] + + +# OpenAI list prices captured for LeanCI MVP receipts (document in receipt footer). +# Source assumption: OpenAI API pricing for gpt-4.1-mini as of 2026-07-30. +_RATES_2026_07_30: dict[str, ModelRate] = { + "gpt-4.1-mini": ModelRate( + model="gpt-4.1-mini", + input_usd_per_1m=0.40, + output_usd_per_1m=1.60, + as_of="2026-07-30", + ), +} + +PRICING_TABLES: dict[str, PricingTable] = { + "2026-07-30": PricingTable(version="2026-07-30", rates=_RATES_2026_07_30), +} + + +def get_pricing(version: str = DEFAULT_PRICING_VERSION) -> PricingTable: + """Return the pricing table for ``version``, or raise ``KeyError``.""" + try: + return PRICING_TABLES[version] + except KeyError as exc: + known = ", ".join(sorted(PRICING_TABLES)) + raise KeyError(f"unknown pricing version {version!r}; known: {known}") from exc + + +def lookup_rate( + model: str, + *, + pricing: PricingTable | None = None, + version: str = DEFAULT_PRICING_VERSION, +) -> ModelRate: + """Look up per-1M input/output rates for ``model`` (case-insensitive).""" + table = pricing if pricing is not None else get_pricing(version) + key = model.strip().lower() + for name, rate in table.rates.items(): + if name.lower() == key: + return rate + known = ", ".join(sorted(table.rates)) + raise KeyError(f"unknown model {model!r} in pricing {table.version}; known: {known}") + + +def estimate_cost_usd( + *, + model: str, + input_tokens: int, + output_tokens: int | None = None, + pricing: PricingTable | None = None, + version: str = DEFAULT_PRICING_VERSION, +) -> float: + """Estimate USD cost from token counts using the dated rate table. + + ``output_tokens=None`` means output is unknown — only input is billed in the + estimate (typical when using Paritok ``/stats`` input domain alone). + """ + if input_tokens < 0 or (output_tokens is not None and output_tokens < 0): + raise ValueError("token counts must be non-negative") + rate = lookup_rate(model, pricing=pricing, version=version) + cost = (input_tokens / 1_000_000) * rate.input_usd_per_1m + if output_tokens: + cost += (output_tokens / 1_000_000) * rate.output_usd_per_1m + return cost + + +def assumption_labels( + *, + model: str = DEFAULT_MODEL, + pricing: PricingTable | None = None, + version: str = DEFAULT_PRICING_VERSION, +) -> list[str]: + """Human-readable assumptions for CostReceipt / PR comment footer (TDD §7.3).""" + table = pricing if pricing is not None else get_pricing(version) + rate = lookup_rate(model, pricing=table) + return [ + f"pricing_version={table.version}", + ( + f"model={rate.model} rates: input ${rate.input_usd_per_1m:.2f}/1M, " + f"output ${rate.output_usd_per_1m:.2f}/1M (as_of {rate.as_of})" + ), + ( + "Primary token counts use Paritok /stats domain " + "(content Paritok intervenes in; may exclude unaffected system prompt tokens)" + ), + ( + "Uncompressed dual-run column uses provider usage when available; " + "otherwise estimates from pre-compression token counts" + ), + ] diff --git a/tests/test_pricing.py b/tests/test_pricing.py new file mode 100644 index 0000000..c98f6e3 --- /dev/null +++ b/tests/test_pricing.py @@ -0,0 +1,87 @@ +"""Unit tests for dated pricing table (TDD §7.3 / §22 M3.1).""" + +from __future__ import annotations + +import pytest + +from leanci.config import DEFAULT_PRICING_VERSION +from leanci.pricing import ( + PricingTable, + assumption_labels, + estimate_cost_usd, + get_pricing, + lookup_rate, +) + + +def test_default_pricing_version_matches_config_contract() -> None: + table = get_pricing() + + assert isinstance(table, PricingTable) + assert table.version == DEFAULT_PRICING_VERSION == "2026-07-30" + + +def test_gpt_4_1_mini_has_dated_input_and_output_rates() -> None: + rate = lookup_rate("gpt-4.1-mini") + + assert rate.model == "gpt-4.1-mini" + assert rate.input_usd_per_1m == pytest.approx(0.40) + assert rate.output_usd_per_1m == pytest.approx(1.60) + assert rate.as_of == "2026-07-30" + + +def test_lookup_is_case_insensitive_and_unknown_raises() -> None: + assert lookup_rate("GPT-4.1-MINI").model == "gpt-4.1-mini" + with pytest.raises(KeyError, match="unknown model"): + lookup_rate("not-a-model") + + +def test_estimate_cost_usd_uses_input_and_optional_output() -> None: + # 1_000_000 input @ $0.40 + 500_000 output @ $1.60 = 0.40 + 0.80 = 1.20 + total = estimate_cost_usd( + model="gpt-4.1-mini", + input_tokens=1_000_000, + output_tokens=500_000, + ) + assert total == pytest.approx(1.20) + + input_only = estimate_cost_usd( + model="gpt-4.1-mini", + input_tokens=2_000_000, + output_tokens=None, + ) + assert input_only == pytest.approx(0.80) + + +def test_estimate_cost_handles_zero_and_small_token_counts() -> None: + assert estimate_cost_usd(model="gpt-4.1-mini", input_tokens=0) == 0.0 + # 1000 tokens input = 0.001 * 0.40 = 0.0004 + assert estimate_cost_usd(model="gpt-4.1-mini", input_tokens=1000) == pytest.approx(0.0004) + + +def test_assumption_labels_include_version_rates_and_stats_domain() -> None: + labels = assumption_labels(model="gpt-4.1-mini") + + joined = " | ".join(labels) + assert "2026-07-30" in joined + assert "gpt-4.1-mini" in joined + assert "$0.40" in joined or "0.40" in joined + assert "$1.60" in joined or "1.60" in joined + assert "Paritok" in joined or "paritok" in joined.lower() or "/stats" in joined + assert any("input" in label.lower() for label in labels) + + +def test_get_pricing_rejects_unknown_version() -> None: + with pytest.raises(KeyError, match="pricing version"): + get_pricing("1999-01-01") + + +def test_estimate_accepts_explicit_pricing_table() -> None: + table = get_pricing("2026-07-30") + cost = estimate_cost_usd( + model="gpt-4.1-mini", + input_tokens=1_000_000, + output_tokens=0, + pricing=table, + ) + assert cost == pytest.approx(0.40) From f93ac88749be177fb8ada04beeadf13f70766c81 Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:11 +0530 Subject: [PATCH 07/29] feat: build CostReceipt from Paritok stats and pricing Compute token reduction and estimated USD with honesty labels for missing baselines and weak compression. --- src/leanci/receipt.py | 134 ++++++++++++++++++++++++++++++++++++++ tests/test_receipt.py | 146 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 280 insertions(+) create mode 100644 src/leanci/receipt.py create mode 100644 tests/test_receipt.py diff --git a/src/leanci/receipt.py b/src/leanci/receipt.py new file mode 100644 index 0000000..46a1810 --- /dev/null +++ b/src/leanci/receipt.py @@ -0,0 +1,134 @@ +"""Receipt builder: CostReceipt from Paritok stats + pricing (TDD §2.12 / §12).""" + +from __future__ import annotations + +from leanci.models import CostReceipt, Mode +from leanci.paritok_gateway import StatsSnapshot +from leanci.pricing import ( + DEFAULT_PRICING_VERSION, + PricingTable, + assumption_labels, + estimate_cost_usd, + get_pricing, +) + +# Treat compression as ineffective when reduction is below this fraction (TDD §12.3). +_INEFFECTIVE_REDUCTION = 0.05 + + +def build_receipt( + *, + stats: StatsSnapshot, + mode: Mode | str, + model: str, + findings_count: int = 0, + files_expanded: int = 0, + tool_turns: int = 0, + latency_ms: int | None = None, + pricing: PricingTable | None = None, + pricing_version: str = DEFAULT_PRICING_VERSION, + uncompressed_input_tokens: int | None = None, + uncompressed_output_tokens: int | None = None, + compressed_output_tokens: int | None = None, +) -> CostReceipt: + """Build an honest ``CostReceipt`` from ``StatsSnapshot`` and the rate table.""" + resolved_mode = Mode(mode) + table = pricing if pricing is not None else get_pricing(pricing_version) + + original = stats.input_tokens_original + compressed = stats.input_tokens_compressed + + reduction: float | None + if original > 0: + reduction = 1.0 - (compressed / original) + else: + reduction = None + + ratio = stats.compression_ratio + if ratio is None and original > 0: + ratio = compressed / original + + est_compressed = estimate_cost_usd( + model=model, + input_tokens=compressed, + output_tokens=compressed_output_tokens, + pricing=table, + ) + + est_uncompressed: float | None = None + if resolved_mode is Mode.DUAL_RUN: + unc_input = ( + uncompressed_input_tokens + if uncompressed_input_tokens is not None + else original + ) + est_uncompressed = estimate_cost_usd( + model=model, + input_tokens=unc_input, + output_tokens=uncompressed_output_tokens, + pricing=table, + ) + + cost_per_finding = est_compressed / max(findings_count, 1) + + assumptions = list( + assumption_labels(model=model, pricing=table, version=table.version) + ) + _append_honesty_labels( + assumptions, + mode=resolved_mode, + reduction=reduction, + original=original, + compressed=compressed, + findings_count=findings_count, + tool_turns=tool_turns, + ) + + return CostReceipt( + mode=resolved_mode, + model=model, + paritok_input_tokens_original=original, + paritok_input_tokens_compressed=compressed, + token_reduction_pct=reduction, + compression_ratio=ratio, + est_cost_compressed_usd=est_compressed, + est_cost_uncompressed_usd=est_uncompressed, + cost_per_finding_usd=cost_per_finding, + latency_ms=latency_ms, + files_expanded=files_expanded, + tool_turns=tool_turns, + assumptions=assumptions, + ) + + +def _append_honesty_labels( + assumptions: list[str], + *, + mode: Mode, + reduction: float | None, + original: int, + compressed: int, + findings_count: int, + tool_turns: int, +) -> None: + if mode is Mode.PARITOK: + assumptions.append( + "baseline not run (enable dual_run for comparison)" + ) + + if findings_count == 0: + assumptions.append( + "cost_per_finding_usd uses max(findings, 1); note: zero findings this run" + ) + + if original == 0 and compressed == 0 and tool_turns > 0: + assumptions.append( + "untrusted: Paritok /stats reported zero activity after tool turns" + ) + + if reduction is not None and reduction < _INEFFECTIVE_REDUCTION: + measured = f"{reduction * 100:.1f}%" + assumptions.append( + f"compression near-ineffective — show measured reduction {measured} only; " + "do not market a fixed headline savings rate" + ) diff --git a/tests/test_receipt.py b/tests/test_receipt.py new file mode 100644 index 0000000..5bc237a --- /dev/null +++ b/tests/test_receipt.py @@ -0,0 +1,146 @@ +"""Unit tests for ReceiptBuilder (TDD §2.12 / §12 / §22 M3.2).""" + +from __future__ import annotations + +import pytest + +from leanci.models import CostReceipt, Mode +from leanci.paritok_gateway import StatsSnapshot +from leanci.pricing import get_pricing +from leanci.receipt import build_receipt + + +def _stats( + *, + original: int = 1000, + compressed: int = 270, + ratio: float | None = 0.27, +) -> StatsSnapshot: + return StatsSnapshot( + input_tokens_original=original, + input_tokens_compressed=compressed, + compression_ratio=ratio, + tokens_saved=max(0, original - compressed), + ) + + +def test_build_receipt_from_stats_and_pricing() -> None: + receipt = build_receipt( + stats=_stats(), + mode=Mode.PARITOK, + model="gpt-4.1-mini", + findings_count=2, + files_expanded=5, + tool_turns=4, + latency_ms=1234, + pricing=get_pricing(), + ) + + assert isinstance(receipt, CostReceipt) + assert receipt.mode is Mode.PARITOK + assert receipt.model == "gpt-4.1-mini" + assert receipt.paritok_input_tokens_original == 1000 + assert receipt.paritok_input_tokens_compressed == 270 + assert receipt.token_reduction_pct == pytest.approx(0.73) + assert receipt.compression_ratio == pytest.approx(0.27) + # compressed input cost: 270/1e6 * 0.40 + assert receipt.est_cost_compressed_usd == pytest.approx(0.000108) + assert receipt.est_cost_uncompressed_usd is None + assert receipt.cost_per_finding_usd == pytest.approx(0.000108 / 2) + assert receipt.latency_ms == 1234 + assert receipt.files_expanded == 5 + assert receipt.tool_turns == 4 + assert any("2026-07-30" in a for a in receipt.assumptions) + assert any("/stats" in a or "Paritok" in a for a in receipt.assumptions) + assert any("baseline not run" in a.lower() for a in receipt.assumptions) + + +def test_dual_run_estimates_uncompressed_column() -> None: + receipt = build_receipt( + stats=_stats(original=1000, compressed=300, ratio=0.3), + mode=Mode.DUAL_RUN, + model="gpt-4.1-mini", + findings_count=1, + uncompressed_input_tokens=1000, + uncompressed_output_tokens=0, + ) + + assert receipt.est_cost_uncompressed_usd == pytest.approx(0.0004) # 1000/1e6 * 0.40 + assert receipt.est_cost_compressed_usd == pytest.approx(0.00012) + assert not any("baseline not run" in a.lower() for a in receipt.assumptions) + + +def test_dual_run_falls_back_to_stats_original_when_usage_missing() -> None: + receipt = build_receipt( + stats=_stats(original=2_000_000, compressed=500_000, ratio=0.25), + mode=Mode.DUAL_RUN, + model="gpt-4.1-mini", + findings_count=1, + ) + + assert receipt.est_cost_uncompressed_usd == pytest.approx(0.80) # 2M * 0.40/1M + + +def test_cost_per_finding_uses_max_one_and_notes_zero_findings() -> None: + receipt = build_receipt( + stats=_stats(original=1_000_000, compressed=1_000_000, ratio=1.0), + mode=Mode.PARITOK, + model="gpt-4.1-mini", + findings_count=0, + ) + + assert receipt.cost_per_finding_usd == pytest.approx(0.40) + assert any("zero findings" in a.lower() for a in receipt.assumptions) + + +def test_honesty_label_when_compression_ineffective() -> None: + receipt = build_receipt( + stats=_stats(original=1000, compressed=990, ratio=0.99), + mode=Mode.PARITOK, + model="gpt-4.1-mini", + findings_count=1, + ) + + assert receipt.token_reduction_pct == pytest.approx(0.01) + joined = " ".join(receipt.assumptions).lower() + assert "74%" not in joined + assert "measured" in joined or "ineffective" in joined or "near" in joined + + +def test_zero_original_tokens_avoids_division_and_marks_untrusted() -> None: + receipt = build_receipt( + stats=_stats(original=0, compressed=0, ratio=None), + mode=Mode.PARITOK, + model="gpt-4.1-mini", + findings_count=0, + tool_turns=3, + ) + + assert receipt.token_reduction_pct is None + assert receipt.compression_ratio is None + assert any("untrusted" in a.lower() for a in receipt.assumptions) + + +def test_uses_stats_compression_ratio_when_provided() -> None: + receipt = build_receipt( + stats=_stats(original=1000, compressed=400, ratio=0.42), + mode=Mode.PARITOK, + model="gpt-4.1-mini", + findings_count=1, + ) + + assert receipt.compression_ratio == pytest.approx(0.42) + assert receipt.token_reduction_pct == pytest.approx(0.6) + + +def test_includes_output_tokens_in_compressed_cost_when_known() -> None: + receipt = build_receipt( + stats=_stats(original=1_000_000, compressed=1_000_000, ratio=1.0), + mode=Mode.PARITOK, + model="gpt-4.1-mini", + findings_count=1, + compressed_output_tokens=500_000, + ) + + # 1.0 * 0.40 + 0.5 * 1.60 = 0.40 + 0.80 = 1.20 + assert receipt.est_cost_compressed_usd == pytest.approx(1.20) From 9a63c8b55a3aea1297a6f02508571c48c75a11bd Mon Sep 17 00:00:00 2001 From: Priyanshu Jha Date: Thu, 30 Jul 2026 18:12:12 +0530 Subject: [PATCH 08/29] feat: publish idempotent LeanCI PR review comments Create or update the marked review comment with findings, receipt table, and failure-comment rendering. --- src/leanci/github_publish.py | 270 +++++++++++++++++++++++++++++++++++ tests/test_github_publish.py | 215 ++++++++++++++++++++++++++++ 2 files changed, 485 insertions(+) create mode 100644 src/leanci/github_publish.py create mode 100644 tests/test_github_publish.py diff --git a/src/leanci/github_publish.py b/src/leanci/github_publish.py new file mode 100644 index 0000000..fed71d5 --- /dev/null +++ b/src/leanci/github_publish.py @@ -0,0 +1,270 @@ +"""GitHub PR comment publisher (TDD §2.13 / §12.2). + +Idempotently creates or updates a single LeanCI review comment marked with +````. Inline review comments are stubbed (P1). +""" + +from __future__ import annotations + +import json +import time +from dataclasses import dataclass +from typing import Any, Sequence +from urllib.error import HTTPError, URLError +from urllib.request import Request, urlopen + +from leanci.errors import GitHubError +from leanci.models import CostReceipt, ExpansionSummary, Finding, Mode + +REVIEW_MARKER = "" +_API = "https://api.github.com" +_RETRY_BACKOFF_S = (2, 5) +_TIMEOUT_S = 30.0 + + +@dataclass(frozen=True) +class PublishResult: + comment_id: int + html_url: str + updated: bool + marker: str = REVIEW_MARKER + + +def render_review_comment( + *, + findings: Sequence[Finding], + receipt: CostReceipt, + expansion: ExpansionSummary | None = None, +) -> str: + """Render the PR markdown body (§12.2) including the idempotency marker.""" + lines = [ + "## LeanCI Review", + REVIEW_MARKER, + "", + "### Findings", + ] + if findings: + for index, finding in enumerate(findings, start=1): + loc = finding.file + if finding.line is not None: + loc = f"{finding.file}:{finding.line}" + title = _escape_md(finding.title) + rationale = _escape_md(finding.rationale) + lines.append( + f"{index}. **[{finding.severity}]** {title} (`{loc}`) — {rationale}" + ) + else: + lines.append("_No findings above the severity floor._") + + lines.extend(["", "### Cost Receipt"]) + lines.extend(_receipt_table(receipt)) + + if expansion is not None: + lines.append("") + lines.append( + f"Expanded {expansion.files_total} files " + f"({expansion.files_beyond_diff} beyond diff), {expansion.bytes} bytes." + ) + + if receipt.assumptions: + lines.append("") + lines.append("Assumptions: " + "; ".join(receipt.assumptions)) + + lines.extend( + [ + "", + "Metrics artifact: `leanci-metrics.json`", + "Powered by Paritok — deep review at lean cost.", + ] + ) + return "\n".join(lines) + "\n" + + +def publish_review_comment( + *, + owner: str, + repo: str, + pr_number: int | None, + token: str, + findings: Sequence[Finding], + receipt: CostReceipt, + expansion: ExpansionSummary | None = None, + body: str | None = None, +) -> PublishResult: + """Create or update the LeanCI review comment on a pull request.""" + if not token or not str(token).strip(): + raise GitHubError("GitHub token is required to publish comments") + if pr_number is None: + raise GitHubError("pr_number is required to publish comments") + + markdown = body if body is not None else render_review_comment( + findings=findings, + receipt=receipt, + expansion=expansion, + ) + if REVIEW_MARKER not in markdown: + markdown = f"{REVIEW_MARKER}\n{markdown}" + + existing_id = _find_marked_comment_id(owner, repo, pr_number, token) + if existing_id is not None: + payload = _request_json( + "PATCH", + f"{_API}/repos/{owner}/{repo}/issues/comments/{existing_id}", + token=token, + body={"body": markdown}, + ) + return PublishResult( + comment_id=int(payload["id"]), + html_url=str(payload.get("html_url") or ""), + updated=True, + ) + + payload = _request_json( + "POST", + f"{_API}/repos/{owner}/{repo}/issues/{pr_number}/comments", + token=token, + body={"body": markdown}, + ) + return PublishResult( + comment_id=int(payload["id"]), + html_url=str(payload.get("html_url") or ""), + updated=False, + ) + + +def publish_inline(*, findings: Sequence[Finding]) -> None: + """P1 stub — inline review comments are not published in MVP.""" + _ = findings + return None + + +def render_failure_comment( + *, + error_class: str, + what_happened: str, + next_steps: Sequence[str], + actions_url: str | None = None, +) -> str: + """Render the §14.3 failure comment body (includes review marker for upsert).""" + lines = [ + "## LeanCI could not complete", + REVIEW_MARKER, + "", + f"Error class: {error_class}", + f"What happened: {_escape_md(what_happened)}", + "Next steps:", + ] + for step in next_steps: + lines.append(f"- {_escape_md(step)}") + lines.append(f"Run: {actions_url or 'n/a'}") + return "\n".join(lines) + "\n" + + +def _receipt_table(receipt: CostReceipt) -> list[str]: + original = receipt.paritok_input_tokens_original + compressed = receipt.paritok_input_tokens_compressed + reduction = receipt.token_reduction_pct + reduction_s = f"{reduction * 100:.1f}%" if reduction is not None else "n/a" + cost_c = _money(receipt.est_cost_compressed_usd) + cost_u = _money(receipt.est_cost_uncompressed_usd) + + if receipt.mode is Mode.DUAL_RUN: + return [ + "| | Uncompressed | Paritok |", + "| --- | ---: | ---: |", + f"| Input tokens (Paritok domain) | {original if original is not None else 'n/a'} | {compressed if compressed is not None else 'n/a'} |", + f"| Est. USD | {cost_u} | {cost_c} |", + f"| Reduction | | {reduction_s} |", + ] + + return [ + "| | Paritok |", + "| --- | ---: |", + f"| Input tokens (Paritok domain) | {compressed if compressed is not None else 'n/a'} |", + f"| Est. USD | {cost_c} |", + f"| Measured reduction | {reduction_s} |", + "", + "_Baseline not run (enable `dual_run` for comparison)._", + ] + + +def _find_marked_comment_id( + owner: str, + repo: str, + pr_number: int, + token: str, +) -> int | None: + payload = _request_json( + "GET", + f"{_API}/repos/{owner}/{repo}/issues/{pr_number}/comments?per_page=100", + token=token, + ) + if not isinstance(payload, list): + raise GitHubError("unexpected GitHub comments list response") + for comment in payload: + if not isinstance(comment, dict): + continue + body = str(comment.get("body") or "") + if REVIEW_MARKER in body: + return int(comment["id"]) + return None + + +def _request_json( + method: str, + url: str, + *, + token: str, + body: dict[str, Any] | None = None, +) -> Any: + data = None if body is None else json.dumps(body).encode("utf-8") + headers = { + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "leanci", + } + if data is not None: + headers["Content-Type"] = "application/json" + + attempts = 1 + len(_RETRY_BACKOFF_S) + last_error: Exception | None = None + for attempt in range(attempts): + request = Request(url, data=data, headers=headers, method=method) + try: + with urlopen(request, timeout=_TIMEOUT_S) as response: + raw = response.read() + if not raw: + return {} + return json.loads(raw.decode("utf-8")) + except HTTPError as exc: + last_error = exc + if exc.code not in {502, 503} or attempt >= attempts - 1: + raise GitHubError(f"GitHub HTTP {exc.code} for {method} {url}: {exc.reason}") from exc + time.sleep(_RETRY_BACKOFF_S[attempt]) + except (URLError, TimeoutError, OSError, json.JSONDecodeError) as exc: + last_error = exc + if attempt >= attempts - 1: + raise GitHubError(f"GitHub request failed for {method} {url}: {exc}") from exc + time.sleep(_RETRY_BACKOFF_S[min(attempt, len(_RETRY_BACKOFF_S) - 1)]) + + raise GitHubError(f"GitHub request failed for {method} {url}: {last_error}") + + +def _escape_md(text: str) -> str: + """Escape markdown emphasis/HTML-ish characters in untrusted finding text.""" + escaped = ( + text.replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace("*", "\\*") + .replace("_", "\\_") + .replace("`", "\\`") + ) + return escaped + + +def _money(value: float | None) -> str: + if value is None: + return "n/a" + return f"${value:.6f}" diff --git a/tests/test_github_publish.py b/tests/test_github_publish.py new file mode 100644 index 0000000..06af06c --- /dev/null +++ b/tests/test_github_publish.py @@ -0,0 +1,215 @@ +"""Unit tests for GitHub Publisher (TDD §2.13 / §12.2 / §22 M3.3).""" + +from __future__ import annotations + +import json +from typing import Any +from unittest.mock import MagicMock, patch +from urllib.error import HTTPError + +import pytest + +from leanci.errors import GitHubError +from leanci.github_publish import ( + REVIEW_MARKER, + PublishResult, + publish_inline, + publish_review_comment, + render_review_comment, +) +from leanci.models import CostReceipt, ExpansionSummary, Finding, Mode, Severity + + +def _finding(**overrides: object) -> Finding: + base: dict[str, object] = { + "id": "f1", + "severity": Severity.HIGH, + "title": "Broken contract", + "file": "src/a.py", + "line": 10, + "symbol": "checkout", + "category": "api_contract", + "rationale": "caller mismatch", + "fix_sketch": None, + } + base.update(overrides) + return Finding(**base) # type: ignore[arg-type] + + +def _receipt(**overrides: object) -> CostReceipt: + base: dict[str, object] = { + "mode": Mode.PARITOK, + "model": "gpt-4.1-mini", + "paritok_input_tokens_original": 1000, + "paritok_input_tokens_compressed": 270, + "token_reduction_pct": 0.73, + "compression_ratio": 0.27, + "est_cost_compressed_usd": 0.0001, + "est_cost_uncompressed_usd": None, + "cost_per_finding_usd": 0.00005, + "latency_ms": 1000, + "files_expanded": 4, + "tool_turns": 3, + "assumptions": ["baseline not run (enable dual_run for comparison)"], + } + base.update(overrides) + return CostReceipt(**base) # type: ignore[arg-type] + + +def _http_json(payload: Any, *, status: int = 200) -> MagicMock: + body = json.dumps(payload).encode("utf-8") + response = MagicMock() + response.status = status + response.read.return_value = body + response.__enter__.return_value = response + response.__exit__.return_value = False + return response + + +def test_render_includes_marker_findings_and_receipt() -> None: + body = render_review_comment( + findings=[_finding()], + receipt=_receipt(), + expansion=ExpansionSummary(files_total=5, files_beyond_diff=3, bytes=100), + ) + + assert REVIEW_MARKER in body + assert "## LeanCI Review" in body + assert "**[high]**" in body + assert "src/a.py:10" in body + assert "Broken contract" in body + assert "Cost Receipt" in body + assert "270" in body + assert "baseline not run" in body.lower() + assert "Powered by Paritok" in body + assert "leanci-metrics.json" in body + + +def test_render_escapes_markdown_specials_in_title() -> None: + body = render_review_comment( + findings=[_finding(title="Use *evil*