Skip to content

Commit 14c5bbc

Browse files
committed
examples: address code-review findings (standards and spec)
Make the sandbox the accepted path. run_review defaults to the auto runtime, which uses the container sandbox when Docker is available and the local subprocess sandbox otherwise; in-process becomes an explicit dev fast-path, and both the selftest and the agent tool run through the sandbox. The standalone run_checks.py and pipeline/scanners.py are reconciled to emit identical findings (aligned severity, confidence and path normalization), enforced by a parity test, so the two paths cannot drift. Make failures visible. A missing scanner produces a needs-human-review finding instead of a silent zero-finding report; the monitoring tool-call count reflects the scanners that actually ran; and the persisted review status is derived from the run (blocked, failed, or completed) rather than always completed. Correctness and deliverables. File-level findings key on the rule as well as the category so distinct issues in one category are not collapsed. The container path's post-processing is extracted into build_container_result and unit-tested without Docker. The agent CLI gains a dry-run flag that forces the fake model, the stale run_review docstring is corrected, and the change adds the committed sample report and a RULES.md documenting all six categories. yapf and the trailing-newline fix leave the tree lint-clean. Updates #92 RELEASE NOTES: NONE
1 parent 3b14631 commit 14c5bbc

22 files changed

Lines changed: 433 additions & 65 deletions

File tree

examples/skills_code_review_agent/README.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,26 @@ renders `review_report.json` + `review_report.md`.
1111
```bash
1212
pip install -r requirements.txt
1313

14-
# Review a bundled fixture (dry-run, deterministic — no model needed):
15-
python run_review.py --fixture 0001_insecure.diff --out-dir /tmp/cr
14+
# Review a bundled fixture (no model needed). Default runtime is the sandbox
15+
# (auto -> container if Docker is up, else the local subprocess sandbox):
16+
python run_review.py --fixture security.diff --out-dir /tmp/cr
1617

17-
# Review your own diff or working tree:
18+
# Review your own diff, working tree, or an explicit file list:
1819
python run_review.py --diff-file my.diff
1920
python run_review.py --repo-path /path/to/repo --no-db
21+
python run_review.py --files pipeline/engine.py,pipeline/scanners.py
2022

2123
# Scored self-test over the labelled fixtures (detection-rate / false-positive-rate):
2224
python selftest.py
2325

24-
# Run the review through the LlmAgent (fake model, no API key needed):
25-
python run_agent.py --fixture 0001_insecure.diff
26+
# Run the review through the LlmAgent with the fake model (no API key needed):
27+
python run_agent.py --fixture security.diff --dry-run
2628
```
2729

30+
A sample report is committed under [`sample_output/`](./sample_output/); the rule catalog is in
31+
[`../../skills/code-review/docs/RULES.md`](../../skills/code-review/docs/RULES.md) and the design note
32+
in [DESIGN.md](./DESIGN.md).
33+
2834
## How it works
2935

3036
Findings come from **deterministic static scanners**, not the LLM, so results are reproducible and

examples/skills_code_review_agent/agent/agent.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""Construct the code-review LlmAgent (Skills + tool), used by run_agent.py."""
87
from __future__ import annotations
98

@@ -14,15 +13,16 @@
1413
from .tools import build_review_tool
1514

1615

17-
def create_agent() -> LlmAgent:
16+
def create_agent(dry_run: bool = False) -> LlmAgent:
1817
"""An LlmAgent that reviews a diff by calling the review_code tool, then summarizes.
1918
20-
The guard Filter (slice 3) attaches on the tool via ``filters_name`` — a TOOL-scoped filter,
21-
not on the agent (which resolves in the AGENT namespace and would raise).
19+
``dry_run`` forces the fake model even if an API key is set. The guard Filter attaches on the tool
20+
via ``filters_name`` — a TOOL-scoped filter, not on the agent (which resolves in the AGENT
21+
namespace and would raise).
2222
"""
2323
return LlmAgent(
2424
name="code_review_agent",
25-
model=get_model(),
25+
model=get_model(force_fake=dry_run),
2626
instruction=INSTRUCTION,
2727
tools=[build_review_tool()],
2828
)

examples/skills_code_review_agent/agent/config.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""Configuration for the agent path — model selection and defaults."""
87
from __future__ import annotations
98

@@ -14,10 +13,10 @@
1413
from .model import FakeReviewModel
1514

1615

17-
def get_model() -> LLMModel:
18-
"""Return the fake model by default (no API key); a real OpenAI model if one is configured."""
16+
def get_model(force_fake: bool = False) -> LLMModel:
17+
"""Fake model by default / when ``force_fake`` (dry-run); a real OpenAI model if a key is set."""
1918
api_key = os.getenv("TRPC_AGENT_API_KEY")
20-
if api_key:
19+
if api_key and not force_fake:
2120
from trpc_agent_sdk.models import OpenAIModel
2221

2322
return OpenAIModel(

examples/skills_code_review_agent/agent/filter.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""ReviewGuardFilter — the framework enforcement site for the review policy (issue #92, req 7).
87
98
A TOOL-scoped filter (``register_tool_filter``): it inspects a tool call's args before the tool

examples/skills_code_review_agent/agent/model.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""FakeReviewModel — deterministic, no-API-key model for the dry-run agent path (criterion 6/8).
87
98
It does not call any LLM. On the first turn it emits a single tool call to ``review_code`` with the

examples/skills_code_review_agent/agent/prompts.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""System instruction for the code-review agent's LLM finding source."""
87

98
INSTRUCTION = ("You are an automated code reviewer. When given a diff, call the `review_code` tool with the "

examples/skills_code_review_agent/pipeline/dedup.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""Dedup + denoise (issue #92, requirement 6).
87
98
- Dedup: at most one finding per (file, line, category); keep the highest-confidence one and mark
@@ -21,6 +20,10 @@
2120

2221

2322
def dedup_key(f: Finding) -> str:
23+
# File-level findings (line is None) share file+category but are distinct issues — key on the
24+
# rule/title too so two different file-level findings in one category don't collapse into one.
25+
if f.line is None:
26+
return f"{f.file}::{f.category}:{f.rule_id or f.title}"
2427
return f"{f.file}:{f.line}:{f.category}"
2528

2629

examples/skills_code_review_agent/pipeline/engine.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def run_review(
7373
repo_path: Optional[str] = None,
7474
files: Optional[list[str]] = None,
7575
repo_root: str = ".",
76-
runtime: str = "inprocess",
76+
runtime: str = "auto",
7777
sandbox_timeout: float | None = None,
7878
max_output_bytes: int | None = None,
7979
policy: ReviewPolicy | None = None,
@@ -93,6 +93,13 @@ def run_review(
9393
started = time.monotonic()
9494
exception_dist: dict[str, int] = {}
9595

96+
# `auto` is the default: sandbox, not in-process. This sync entry can't drive the async container
97+
# runtime, so auto resolves to the local subprocess sandbox here; the CLI upgrades auto->container
98+
# when Docker is available (see run_review.py / run_review_container). `inprocess` is an opt-in dev
99+
# fast-path that must be requested explicitly.
100+
if runtime == "auto":
101+
runtime = "local"
102+
96103
if diff_text is not None:
97104
summary, scan_dir = _materialize(diff_text)
98105
source_type, source_ref = "diff_file", "<diff>"
@@ -157,7 +164,7 @@ def _assemble(task_id, summary, raw, sandbox_runs, source_type, source_ref, star
157164
monitoring = {
158165
"total_sec": round(time.monotonic() - started, 3),
159166
"sandbox_sec": round(sum(r.duration_sec for r in sandbox_runs), 3),
160-
"tool_calls": len(scanners.ADAPTERS),
167+
"tool_calls": scanners.tool_calls_available(),
161168
"block_count": len(filter_blocks),
162169
"finding_count": len(active),
163170
"severity_dist": severity_dist,

examples/skills_code_review_agent/pipeline/report.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
# Copyright (C) 2026 Tencent. All rights reserved.
44
#
55
# tRPC-Agent-Python is licensed under Apache-2.0.
6-
76
"""Build and render the review report (issue #92, requirement: report with 7 sections).
87
98
The 7 sections (findings summary, severity stats, human-review items, Filter-block summary,

examples/skills_code_review_agent/pipeline/sandbox.py

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -174,20 +174,41 @@ async def run_container(
174174
timeout=timeout,
175175
limits=WorkspaceResourceLimits(memory_mb=memory_mb)))
176176
collected = await fs.collect_outputs(ws, WorkspaceOutputSpec(globs=["findings.json"]))
177-
findings: list[Finding] = []
178-
for cf in getattr(collected, "files", collected) or []:
179-
try:
180-
findings = parse_findings_json(json.loads(cf.content.decode("utf-8")))
181-
except Exception: # noqa: BLE001
182-
continue
183-
_, out_bytes = _truncate(run.stdout, max_bytes)
184-
_, err_bytes = _truncate(run.stderr, max_bytes)
185-
result = SandboxRunResult(script="run_checks.py",
186-
exit_code=run.exit_code,
187-
duration_sec=round(time.monotonic() - started, 3),
188-
timed_out=run.timed_out,
189-
stdout_bytes=out_bytes,
190-
stderr_bytes=err_bytes)
191-
return findings, result
177+
return build_container_result(getattr(collected, "files", collected),
178+
stdout=run.stdout,
179+
stderr=run.stderr,
180+
exit_code=run.exit_code,
181+
timed_out=run.timed_out,
182+
duration_sec=time.monotonic() - started,
183+
max_bytes=max_bytes)
192184
finally:
193185
await manager.cleanup(exec_id)
186+
187+
188+
def build_container_result(collected_files,
189+
*,
190+
stdout,
191+
stderr,
192+
exit_code,
193+
timed_out,
194+
duration_sec,
195+
max_bytes=MAX_OUTPUT_BYTES) -> tuple[list[Finding], SandboxRunResult]:
196+
"""Pure post-processing of a container run (parse findings.json + build SandboxRunResult).
197+
198+
Extracted so the container path's logic is unit-testable without Docker (the Docker-only part is
199+
just staging + running the workspace). ``collected_files`` are objects with a ``.content`` bytes attr.
200+
"""
201+
findings: list[Finding] = []
202+
for cf in collected_files or []:
203+
try:
204+
findings = parse_findings_json(json.loads(cf.content.decode("utf-8")))
205+
except Exception: # noqa: BLE001 - a malformed collected file degrades the source, not the task
206+
continue
207+
_, out_bytes = _truncate(stdout, max_bytes)
208+
_, err_bytes = _truncate(stderr, max_bytes)
209+
return findings, SandboxRunResult(script="run_checks.py",
210+
exit_code=exit_code,
211+
duration_sec=round(duration_sec, 3),
212+
timed_out=timed_out,
213+
stdout_bytes=out_bytes,
214+
stderr_bytes=err_bytes)

0 commit comments

Comments
 (0)