feat(opencode_review): python fallback for summary fields
This commit is contained in:
@@ -55,6 +55,8 @@ import time
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
|
||||
from ai_review import is_test_path
|
||||
|
||||
# Where the factory lives (opencode.json + .opencode/). Default: the pragent
|
||||
# repo root (this file is at <root>/pilot/opencode_review.py).
|
||||
_DEFAULT_FACTORY = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
@@ -565,6 +567,14 @@ _LENS_ID_RE = re.compile(r"^[a-z0-9-]{1,32}$")
|
||||
SEVERITY_ORDER = ("low", "medium", "high", "critical")
|
||||
SEVERITY_RANK = {s: i for i, s in enumerate(SEVERITY_ORDER)}
|
||||
|
||||
# Local emoji map for the synthesized walkthrough. Kept separate from
|
||||
# `ai_review._SEVERITY_EMOJI` (which also has "trivial"/"info"/"nit") so
|
||||
# this stays a small surface for the fallback path.
|
||||
_SEVERITY_EMOJI_SUMMARY = {
|
||||
"critical": "🔴", "high": "🔴", "medium": "🟡",
|
||||
"low": "🔵", "trivial": "⚪", "info": "⚪",
|
||||
}
|
||||
|
||||
|
||||
@_dc.dataclass(frozen=True)
|
||||
class ReviewerSpec:
|
||||
@@ -917,12 +927,69 @@ def _synthesize_summary_fields(
|
||||
(`ai_review.parse_review_output` extracts them as the 5th, 6th, and
|
||||
7th tuple elements, defaulting to `[]` / `""` when missing).
|
||||
|
||||
STUB for Task 7. The real implementation arrives in Task 8; for now
|
||||
every return is empty so the synthesized JSON shape stays parseable
|
||||
and downstream tests that default the new fields to `[]` / `""`
|
||||
continue to pass.
|
||||
Real implementation (Task 8). Python fallback used when the lens
|
||||
fan-out path is engaged (the synthesized JSON fence in `run_lenses_review`
|
||||
has no model to call, so we build these fields deterministically from
|
||||
the merged findings + the diff):
|
||||
- walkthrough: one line per changed file. When findings exist, group
|
||||
by path and pick the peak-severity problem as the headline; when
|
||||
no findings exist, just announce "changed".
|
||||
- risk_verdict: a one-line verdict driven by the highest severity
|
||||
bucket that has any findings ("Critical risk" / "High risk" /
|
||||
"Medium risk" / "Low risk").
|
||||
- test_coverage: "Tests changed" if any changed path matches
|
||||
`is_test_path`, else "No tests for behavioral change in `<path>`."
|
||||
pointing at the first non-test path.
|
||||
"""
|
||||
return [], "", ""
|
||||
# walkthrough
|
||||
walkthrough: list[str] = []
|
||||
if findings:
|
||||
by_path: dict[str, list[dict]] = {}
|
||||
for f in findings:
|
||||
by_path.setdefault(f.get("path", "?"), []).append(f)
|
||||
for path, group in sorted(by_path.items()):
|
||||
peak = max(
|
||||
group,
|
||||
key=lambda x: SEVERITY_RANK.get(x.get("severity", "low"), 0),
|
||||
)
|
||||
problem_lines = (peak.get("problem") or "").splitlines()
|
||||
problem = problem_lines[0][:80].strip() if problem_lines else ""
|
||||
emoji = _SEVERITY_EMOJI_SUMMARY.get(
|
||||
peak.get("severity", "low"), "⚪")
|
||||
walkthrough.append(f"`{path}` — {emoji} {problem}")
|
||||
else:
|
||||
files = changed_paths if changed_paths is not None else changed_files(diff)
|
||||
for p in files:
|
||||
walkthrough.append(f"`{p}` — changed")
|
||||
|
||||
# risk_verdict
|
||||
sev_counts = {"critical": 0, "high": 0, "medium": 0, "low": 0}
|
||||
for f in findings:
|
||||
s = f.get("severity", "low")
|
||||
sev_counts[s] = sev_counts.get(s, 0) + 1
|
||||
if sev_counts["critical"]:
|
||||
rv = f"Critical risk: {sev_counts['critical']} critical finding(s)."
|
||||
elif sev_counts["high"]:
|
||||
rv = f"High risk: {sev_counts['high']} high finding(s)."
|
||||
elif sev_counts["medium"]:
|
||||
rv = f"Medium risk: {sev_counts['medium']} medium finding(s)."
|
||||
else:
|
||||
rv = "Low risk: clean or minor nits only."
|
||||
|
||||
# test_coverage
|
||||
paths = changed_paths if changed_paths is not None else changed_files(diff)
|
||||
test_changed = any(is_test_path(p) for p in paths)
|
||||
non_test = [p for p in paths if not is_test_path(p)]
|
||||
if test_changed and non_test:
|
||||
tc = "Tests changed"
|
||||
elif non_test:
|
||||
tc = f"No tests for behavioral change in `{non_test[0]}`."
|
||||
elif test_changed:
|
||||
tc = "Tests changed"
|
||||
else:
|
||||
tc = ""
|
||||
|
||||
return walkthrough, rv, tc
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1334,6 +1401,8 @@ def run_lenses_review(
|
||||
)
|
||||
synthesized_payload = {
|
||||
"summary": summary,
|
||||
"summary_changes": [],
|
||||
"risks": [],
|
||||
"walkthrough": walkthrough,
|
||||
"risk_verdict": risk_verdict,
|
||||
"test_coverage": test_coverage,
|
||||
|
||||
@@ -859,3 +859,49 @@ def test_no_surface_response_zero_lenses_wording():
|
||||
summary, findings, _c, _r, _w, _rv, _tc = ai_review.parse_review_output(text)
|
||||
assert findings == []
|
||||
assert "after path filtering" in summary
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _synthesize_summary_fields — Task 8: real Python fallback implementation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_synthesize_walkthrough_groups_findings_by_path():
|
||||
findings = [
|
||||
{"path": "a.py", "line": 1, "severity": "medium", "problem": "fix x"},
|
||||
{"path": "b.py", "line": 2, "severity": "high", "problem": "fix y"},
|
||||
]
|
||||
w, _, _ = oc._synthesize_summary_fields(findings, "")
|
||||
assert any("a.py" in line for line in w)
|
||||
assert any("b.py" in line for line in w)
|
||||
|
||||
|
||||
def test_synthesize_walkthrough_empty_when_no_findings_uses_changed_files():
|
||||
w, _, _ = oc._synthesize_summary_fields(
|
||||
[],
|
||||
"diff --git a/x.py b/x.py\n@@ -1 +1 @@\n-old\n+new\n+++ b/x.py\n",
|
||||
)
|
||||
assert any("x.py" in line for line in w)
|
||||
|
||||
|
||||
def test_synthesize_risk_verdict_critical():
|
||||
findings = [{"severity": "critical"}]
|
||||
_, rv, _ = oc._synthesize_summary_fields(findings, "")
|
||||
assert "Critical risk" in rv
|
||||
|
||||
|
||||
def test_synthesize_risk_verdict_clean():
|
||||
_, rv, _ = oc._synthesize_summary_fields([], "")
|
||||
assert "Low risk" in rv
|
||||
|
||||
|
||||
def test_synthesize_test_coverage_with_test_path():
|
||||
_, _, tc = oc._synthesize_summary_fields(
|
||||
[], "+diff\n", changed_paths=["pilot/foo.py", "tests/test_foo.py"])
|
||||
assert tc == "Tests changed"
|
||||
|
||||
|
||||
def test_synthesize_test_coverage_missing_tests():
|
||||
_, _, tc = oc._synthesize_summary_fields(
|
||||
[], "+diff\n", changed_paths=["pilot/foo.py"])
|
||||
assert "No tests for behavioral change" in tc
|
||||
|
||||
Reference in New Issue
Block a user