test: add code review fixtures and pytest coverage

Author: Small-fish-QAQ

examples/skills_code_review_agent/README.md

@@ -57,6 +57,27 @@ Using the `sqlite3` CLI:
 sqlite3 examples/skills_code_review_agent/output/reviews.sqlite3 "select severity, category, file, line, title from findings order by id;"
 ```
 
+## Run Tests
+
+```bash
+python -m pytest examples/skills_code_review_agent/tests
+```
+
+The tests run only local parsing, rules, redaction, report generation, and
+dedupe checks. They do not call an LLM, Docker, Cube, remote network, or any
+external service.
+
+## Fixtures
+
+- `clean.diff`: safe changes; should produce no high-severity findings.
+- `security.diff`: mixed security sample covering secret, missing timeout, broad exception, SQL risk, and resource lifecycle.
+- `sql_injection.diff`: SQL string concatenation.
+- `missing_timeout.diff`: `httpx` request without `timeout=`.
+- `broad_except.diff`: broad `except Exception` with swallowed failure.
+- `resource_leak.diff`: `open(...)` without a context manager.
+- `duplicate.diff`: duplicate hunk producing the same finding, used to verify dedupe.
+- `secret_redaction.diff`: multiple secret-like values, used to verify reports omit plaintext secrets.
+
 ## Current Scope
 
 - Parses unified diff hunks and added line numbers.
@@ -69,6 +90,7 @@ sqlite3 examples/skills_code_review_agent/output/reviews.sqlite3 "select severit
 - Redacts likely API keys, tokens, secrets, and passwords before writing reports.
 - Produces `review_report.json` and `review_report.md`.
 - Optionally persists review tasks, findings, and report metadata to SQLite.
+- Includes local pytest coverage for the deterministic rule fixtures.
 
 ## Not Implemented Yet
 

examples/skills_code_review_agent/agent/rules.py

@@ -110,6 +110,27 @@ def _next_added_line(file_lines: list[ChangedLine], index: int) -> ChangedLine |
     return file_lines[index + 1]
 
 
+def _dedupe_findings(findings: list[Finding]) -> list[Finding]:
+    seen: set[tuple[object, ...]] = set()
+    deduped: list[Finding] = []
+    for finding in findings:
+        key = (
+            finding.severity,
+            finding.category,
+            finding.file,
+            finding.line,
+            finding.title,
+            finding.evidence,
+            finding.recommendation,
+            finding.source,
+        )
+        if key in seen:
+            continue
+        seen.add(key)
+        deduped.append(finding)
+    return deduped
+
+
 def run_static_rules(changed_lines: Iterable[ChangedLine]) -> list[Finding]:
     """Run all deterministic phase-1 rules over added diff lines."""
 
@@ -192,4 +213,4 @@ def run_static_rules(changed_lines: Iterable[ChangedLine]) -> list[Finding]:
                 )
             )
 
-    return sorted(findings, key=lambda item: (item.file, item.line, item.category, item.source))
+    return sorted(_dedupe_findings(findings), key=lambda item: (item.file, item.line, item.category, item.source))

examples/skills_code_review_agent/fixtures/broad_except.diff

@@ -0,0 +1,11 @@
+diff --git a/app/worker.py b/app/worker.py
+index 3000001..3000002 100644
+--- a/app/worker.py
++++ b/app/worker.py
+@@ -1,5 +1,10 @@
+ def run_job(job):
+-    return job.run()
++    try:
++        return job.run()
++    except Exception:
++        return None

examples/skills_code_review_agent/fixtures/duplicate.diff

@@ -0,0 +1,12 @@
+diff --git a/app/duplicate.py b/app/duplicate.py
+index 5000001..5000002 100644
+--- a/app/duplicate.py
++++ b/app/duplicate.py
+@@ -1,3 +1,4 @@
+ import requests
+ def fetch():
++    return requests.get("https://duplicate.example/api")
+@@ -1,3 +1,4 @@
+ import requests
+ def fetch():
++    return requests.get("https://duplicate.example/api")

examples/skills_code_review_agent/fixtures/missing_timeout.diff

@@ -0,0 +1,11 @@
+diff --git a/app/client.py b/app/client.py
+index 2000001..2000002 100644
+--- a/app/client.py
++++ b/app/client.py
+@@ -1,5 +1,7 @@
+ import httpx
+ 
+ 
+ def load_status():
+-    return {"ok": True}
++    return httpx.get("https://status.example/api")

examples/skills_code_review_agent/fixtures/resource_leak.diff

@@ -0,0 +1,9 @@
+diff --git a/app/files.py b/app/files.py
+index 4000001..4000002 100644
+--- a/app/files.py
++++ b/app/files.py
+@@ -1,4 +1,6 @@
+ def read_token(path):
+-    return ""
++    handle = open(path, encoding="utf-8")
++    return handle.read()

examples/skills_code_review_agent/fixtures/secret_redaction.diff

@@ -0,0 +1,11 @@
+diff --git a/app/settings.py b/app/settings.py
+index 6000001..6000002 100644
+--- a/app/settings.py
++++ b/app/settings.py
+@@ -1,4 +1,8 @@
+ def settings():
+-    return {}
++    api_key = "sk-test-abcdefghijklmnop"
++    auth_token = "token-value-1234567890"
++    password = "correct-horse-prod-password"
++    return {"enabled": True}

examples/skills_code_review_agent/fixtures/sql_injection.diff

@@ -0,0 +1,9 @@
+diff --git a/app/search.py b/app/search.py
+index 1000001..1000002 100644
+--- a/app/search.py
++++ b/app/search.py
+@@ -1,4 +1,6 @@
+ def find_user(db, user_id):
+-    return db.execute("SELECT * FROM users WHERE id = :id", {"id": user_id})
++    sql = "SELECT * FROM users WHERE id = " + user_id
++    return db.execute(sql)

examples/skills_code_review_agent/tests/conftest.py

@@ -0,0 +1,11 @@
+"""Pytest setup for the self-contained code review example."""
+
+from __future__ import annotations
+
+import sys
+from pathlib import Path
+
+
+EXAMPLE_ROOT = Path(__file__).resolve().parents[1]
+if str(EXAMPLE_ROOT) not in sys.path:
+    sys.path.insert(0, str(EXAMPLE_ROOT))

examples/skills_code_review_agent/tests/test_static_review.py

@@ -0,0 +1,86 @@
+"""Deterministic tests for the phase-1/2 code review example."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from agent.diff_parser import ParsedDiff
+from agent.diff_parser import parse_unified_diff
+from agent.findings import Finding
+from agent.report import build_report
+from agent.report import write_reports
+from agent.rules import run_static_rules
+
+
+EXAMPLE_ROOT = Path(__file__).resolve().parents[1]
+FIXTURES = EXAMPLE_ROOT / "fixtures"
+
+
+def _review_fixture(name: str) -> tuple[ParsedDiff, list[Finding]]:
+    diff_text = (FIXTURES / name).read_text(encoding="utf-8")
+    parsed = parse_unified_diff(diff_text)
+    findings = run_static_rules(parsed.changed_lines)
+    return parsed, findings
+
+
+def _categories(findings: list[Finding]) -> set[str]:
+    return {finding.category for finding in findings}
+
+
+def _count_category(findings: list[Finding], category: str) -> int:
+    return sum(1 for finding in findings if finding.category == category)
+
+
+def test_clean_diff_has_no_high_findings() -> None:
+    _, findings = _review_fixture("clean.diff")
+    assert [finding for finding in findings if finding.severity == "high"] == []
+
+
+def test_security_diff_contains_expected_categories() -> None:
+    _, findings = _review_fixture("security.diff")
+    categories = _categories(findings)
+    assert "secret" in categories
+    assert "network-timeout" in categories
+    assert "error-handling" in categories
+
+
+def test_sql_injection_fixture() -> None:
+    _, findings = _review_fixture("sql_injection.diff")
+    assert "sql-injection" in _categories(findings)
+
+
+def test_missing_timeout_fixture() -> None:
+    _, findings = _review_fixture("missing_timeout.diff")
+    assert "network-timeout" in _categories(findings)
+
+
+def test_broad_except_fixture() -> None:
+    _, findings = _review_fixture("broad_except.diff")
+    assert "error-handling" in _categories(findings)
+
+
+def test_resource_leak_fixture() -> None:
+    _, findings = _review_fixture("resource_leak.diff")
+    assert "resource-lifecycle" in _categories(findings)
+
+
+def test_secret_redaction_report_omits_plaintext_values(tmp_path: Path) -> None:
+    parsed, findings = _review_fixture("secret_redaction.diff")
+    report = build_report(
+        diff_file="fixtures/secret_redaction.diff",
+        files=parsed.files,
+        findings=findings,
+        dry_run=True,
+    )
+    json_path, md_path = write_reports(report, tmp_path)
+    combined = json_path.read_text(encoding="utf-8") + md_path.read_text(encoding="utf-8")
+
+    assert "sk-test-abcdefghijklmnop" not in combined
+    assert "token-value-1234567890" not in combined
+    assert "correct-horse-prod-password" not in combined
+    assert "<redacted:" in combined
+
+
+def test_duplicate_fixture_dedupes_repeated_finding() -> None:
+    _, findings = _review_fixture("duplicate.diff")
+    assert _count_category(findings, "network-timeout") == 1