Skip to content

Commit 02b29df

Browse files
authored
Complete E23-T4 pr-review verification coverage (#109)
1 parent c7c42b3 commit 02b29df

File tree

4 files changed

+102
-8
lines changed

4 files changed

+102
-8
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ All notable changes to this project are documented in this file.
151151
- Moved `plan_execution` runtime persistence out of `opencode.json` into `~/.config/opencode/my_opencode/runtime/plan_execution.json` to prevent OpenCode startup failures caused by unrecognized top-level config keys.
152152
- Added selftest coverage for `/pr-review` analyzer missing-evidence and blocker-evidence decision paths, and marked Task 23.2 complete in the roadmap.
153153
- Integrated `pr-review` checks into unified `/doctor`, updated installer self-check/hints for PR review workflows, and marked Task 23.3 complete in the roadmap.
154+
- Expanded pr-review verification coverage for risk-detection false-positive control and missing-evidence behavior, and marked Epic 23 Task 23.4/exit criteria complete in the roadmap.
154155

155156
## v0.2.0 - 2026-02-12
156157

IMPLEMENTATION_ROADMAP.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ Use this map to avoid overlapping implementations.
5959
| E20 | Execution Budget Guardrails | done | High | E2, E11 | bd-63f | Bound time/tool/token usage for autonomous runs |
6060
| E21 | Bounded Loop Mode Presets | merged | Medium | E22, E28 | TBD | Merged into E22/E28 loop controls |
6161
| E22 | Autoflow Unified Orchestration Command | done | High | E14, E15, E17, E19, E20 | TBD | One command for plan-run-resume-report lifecycle |
62-
| E23 | PR Review Copilot | in_progress | High | E3 | bd-1hc | Pre-PR quality, output, and risk review automation |
62+
| E23 | PR Review Copilot | done | High | E3 | bd-u6t | Pre-PR quality, output, and risk review automation |
6363
| E24 | Release Train Assistant | planned | High | E14, E23 | TBD | Validate, draft, and gate releases reliably |
6464
| E25 | Incident Hotfix Mode | planned | Medium | E20, E22 | TBD | Constrained emergency workflow with strict safety |
6565
| E26 | Repo Health Score and Drift Monitor | planned | Medium | E9, E12, E20 | TBD | Operational visibility and continuous diagnostics |
@@ -809,7 +809,7 @@ Every command-oriented epic must ship all of the following:
809809

810810
## Epic 23 - PR Review Copilot
811811

812-
**Status:** `in_progress`
812+
**Status:** `done`
813813
**Priority:** High
814814
**Goal:** Add a command that reviews pending PR changes for risk, quality, and release readiness before merge.
815815
**Depends on:** Epic 3
@@ -829,12 +829,13 @@ Every command-oriented epic must ship all of the following:
829829
- [x] Subtask 23.3.2: Integrate with pre-merge checklist and doctor output
830830
- [x] Subtask 23.3.3: Document triage flow for warnings vs blockers
831831
- [x] Notes: Added `scripts/pr_review_command.py` command surface with concise/JSON output and `checklist`/`doctor` subcommands, wired aliases in `opencode.json`, integrated `pr-review` into unified doctor checks, and documented blocker-vs-warning triage guidance in README.
832-
- [ ] Task 23.4: Verification
833-
- [ ] Subtask 23.4.1: Add tests for risk detection and false positive control
834-
- [ ] Subtask 23.4.2: Add tests for missing-evidence behavior
835-
- [ ] Subtask 23.4.3: Add install-test smoke checks
836-
- [ ] Exit criteria: copilot catches high-risk omissions before merge
837-
- [ ] Exit criteria: outputs are actionable and low-noise in default mode
832+
- [x] Task 23.4: Verification
833+
- [x] Subtask 23.4.1: Add tests for risk detection and false positive control
834+
- [x] Subtask 23.4.2: Add tests for missing-evidence behavior
835+
- [x] Subtask 23.4.3: Add install-test smoke checks
836+
- [x] Notes: Expanded `scripts/selftest.py` with docs-only false-positive guard assertions and tested-source-change missing-evidence checks, and installer smoke now exercises `/pr-review`, `/pr-review checklist`, and `/pr-review doctor` workflows.
837+
- [x] Exit criteria: copilot catches high-risk omissions before merge
838+
- [x] Exit criteria: outputs are actionable and low-noise in default mode
838839

839840
---
840841

README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,12 @@ Examples:
255255
/pr-review doctor --json
256256
```
257257

258+
Task 23.4 verification notes:
259+
260+
- selftest validates blocker detection for hard-evidence security findings and missing-evidence recommendation behavior.
261+
- selftest validates false-positive control for docs-only diffs (`recommendation=approve`, no findings).
262+
- install smoke validates `/pr-review`, `/pr-review checklist`, and `/pr-review doctor` command paths.
263+
258264
## Installed plugin stack 🔌
259265

260266
- `@mohak34/opencode-notifier@latest` - desktop and sound alerts for completion, errors, and permission prompts.

scripts/selftest.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,92 @@ def run_bg(*args: str) -> subprocess.CompletedProcess[str]:
11601160
"pr-review doctor should confirm analyzer readiness",
11611161
)
11621162

1163+
analyzer_docs_only_diff = tmp / "pr_review_docs_only.diff"
1164+
analyzer_docs_only_diff.write_text(
1165+
"""diff --git a/README.md b/README.md
1166+
index 1111111..2222222 100644
1167+
--- a/README.md
1168+
+++ b/README.md
1169+
@@ -10,0 +11,2 @@
1170+
+## Notes
1171+
+Updated documentation only.
1172+
""",
1173+
encoding="utf-8",
1174+
)
1175+
analyzer_docs_only = subprocess.run(
1176+
[
1177+
sys.executable,
1178+
str(PR_REVIEW_ANALYZER_SCRIPT),
1179+
"analyze",
1180+
"--diff-file",
1181+
str(analyzer_docs_only_diff),
1182+
"--json",
1183+
],
1184+
capture_output=True,
1185+
text=True,
1186+
env=refactor_env,
1187+
check=False,
1188+
cwd=REPO_ROOT,
1189+
)
1190+
expect(
1191+
analyzer_docs_only.returncode == 0,
1192+
"pr-review analyzer should parse docs-only diff",
1193+
)
1194+
analyzer_docs_only_report = parse_json_output(analyzer_docs_only.stdout)
1195+
expect(
1196+
analyzer_docs_only_report.get("recommendation") == "approve",
1197+
"pr-review analyzer should avoid false positives for docs-only changes",
1198+
)
1199+
expect(
1200+
not analyzer_docs_only_report.get("findings"),
1201+
"pr-review analyzer should keep docs-only default output low-noise",
1202+
)
1203+
1204+
analyzer_tested_change_diff = tmp / "pr_review_tested_change.diff"
1205+
analyzer_tested_change_diff.write_text(
1206+
"""diff --git a/scripts/calc.py b/scripts/calc.py
1207+
index 1111111..2222222 100644
1208+
--- a/scripts/calc.py
1209+
+++ b/scripts/calc.py
1210+
@@ -1,0 +1,2 @@
1211+
+def calc_total(values):
1212+
+ return sum(values)
1213+
diff --git a/tests/test_calc.py b/tests/test_calc.py
1214+
index 3333333..4444444 100644
1215+
--- a/tests/test_calc.py
1216+
+++ b/tests/test_calc.py
1217+
@@ -1,0 +1,2 @@
1218+
+def test_calc_total():
1219+
+ assert True
1220+
""",
1221+
encoding="utf-8",
1222+
)
1223+
analyzer_tested_change = subprocess.run(
1224+
[
1225+
sys.executable,
1226+
str(PR_REVIEW_ANALYZER_SCRIPT),
1227+
"analyze",
1228+
"--diff-file",
1229+
str(analyzer_tested_change_diff),
1230+
"--json",
1231+
],
1232+
capture_output=True,
1233+
text=True,
1234+
env=refactor_env,
1235+
check=False,
1236+
cwd=REPO_ROOT,
1237+
)
1238+
expect(
1239+
analyzer_tested_change.returncode == 0,
1240+
"pr-review analyzer should parse tested source-change diff",
1241+
)
1242+
analyzer_tested_change_report = parse_json_output(analyzer_tested_change.stdout)
1243+
expect(
1244+
"tests"
1245+
not in set(analyzer_tested_change_report.get("missing_evidence", [])),
1246+
"pr-review analyzer should not report missing tests when test files changed",
1247+
)
1248+
11631249
result = subprocess.run(
11641250
[
11651251
sys.executable,

0 commit comments

Comments
 (0)