diff --git a/.github/workflows/opencode-review-quality-ci.yml b/.github/workflows/opencode-review-quality-ci.yml new file mode 100644 index 000000000..1aa3882e8 --- /dev/null +++ b/.github/workflows/opencode-review-quality-ci.yml @@ -0,0 +1,91 @@ +name: OpenCode Review Quality CI + +on: + pull_request: + branches: [main] + paths: + - ".github/workflows/opencode-review-quality-ci.yml" + - "benchmarks/opencode_review/**" + - "scripts/ci/opencode_review_quality_score.py" + - "tests/test_opencode_review_quality_score.py" + - "tests/test_opencode_review_quality_evidence_contract.py" + - "docs/doctoring/opencode-review-quality-evaluation.md" + - "docs/superpowers/specs/2026-08-08-opencode-review-quality.md" + - "docs/superpowers/plans/2026-08-08-opencode-review-quality.md" + - "CHANGELOG.md" + +permissions: + contents: read + +concurrency: + group: opencode-review-quality-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + empirical-review-quality: + name: empirical-review-quality + if: github.event_name != 'pull_request' || github.event.action != 'closed' + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - name: Checkout exact source revision + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + + - name: Install exact hash-verified test runner dependencies + env: + PIP_DISABLE_PIP_VERSION_CHECK: "1" + PIP_NO_INPUT: "1" + shell: bash --noprofile --norc -e -o pipefail {0} + run: | + cat >"${RUNNER_TEMP}/opencode-review-quality-requirements.txt" <<'EOF' + coverage==7.15.2 --hash=sha256:b9a6367e4aff723e8ee8190836836124284e8fcd4265e307c844010cfa074f3f + iniconfig==2.1.0 --hash=sha256:9deba5723312380e77435581c6bf4935c94cbfab9b1ed33ef8d238ea168eb760 + packaging==26.2 --hash=sha256:5fc45236b9446107ff2415ce77c807cee2862cb6fac22b8a73826d0693b0980e + pluggy==1.6.0 --hash=sha256:e920276dd6813095e9377c0bc5566d94c932c33b27a3e3945d8389c374dd4746 + pygments==2.20.0 --hash=sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176 + pytest==9.1.1 --hash=sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c + EOF + python -m pip install \ + --only-binary=:all: \ + --require-hashes \ + -r "${RUNNER_TEMP}/opencode-review-quality-requirements.txt" + + - name: Verify empirical review quality contracts + shell: bash --noprofile --norc -e -o pipefail {0} + run: | + test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha || github.sha }}" + python -m coverage run \ + --branch \ + --source=scripts/ci \ + -m pytest \ + tests/test_opencode_review_quality_score.py \ + tests/test_opencode_review_quality_evidence_contract.py \ + -q + python -m coverage report \ + --include='scripts/ci/opencode_review_quality_score.py' \ + --fail-under=100 \ + --show-missing + python scripts/ci/opencode_review_quality_score.py \ + --input-root "${GITHUB_WORKSPACE}" \ + --output-root "${RUNNER_TEMP}" \ + --input benchmarks/opencode_review/pilot_baseline_v1.json \ + --json-output "${RUNNER_TEMP}/opencode-review-quality.json" \ + --markdown-output "${RUNNER_TEMP}/opencode-review-quality.md" + python -m json.tool "${RUNNER_TEMP}/opencode-review-quality.json" >/dev/null + python -m json.tool benchmarks/opencode_review/pilot_baseline_v1.json >/dev/null + grep -Fq 'Parity gate: **INSUFFICIENT_EVIDENCE**' \ + "${RUNNER_TEMP}/opencode-review-quality.md" + cat "${RUNNER_TEMP}/opencode-review-quality.md" >>"$GITHUB_STEP_SUMMARY" + python -m compileall -q \ + scripts/ci/opencode_review_quality_score.py \ + tests/test_opencode_review_quality_score.py \ + tests/test_opencode_review_quality_evidence_contract.py + git diff --exit-code diff --git a/CHANGELOG.md b/CHANGELOG.md index bf30091dd..fbfdc13c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Semantic Versioning where the repository publishes a release. ### Added +- Added an empirical OpenCode review-quality benchmark, fail-closed scorer, exact-head quality workflow, and APA 7th doctoring that keep lifecycle-yield evidence separate from head-matched expert-gold precision and recall, require Wilson-bound non-inferiority before any CodeRabbit-parity claim, and preserve 100% production statement/branch/docstring evidence. - Added a trusted pull-request comment router for `@cwl-noema-review` and review-only `@opencode-agent` dispatches, with an organization sweep, exact-head receipts, repository allowlisting, fixed runners, immutable checkout pins, and a permanent 100% statement/branch/docstring quality gate. - Added exact-base `uv.lock` materialization that reconstructs standalone nested projects with a checksum-pinned official `uv` exporter, isolated frozen/offline execution, strict exact-pin and SHA-256 output validation, and complete Python 3.10/3.14 quality evidence. diff --git a/benchmarks/opencode_review/pilot_baseline_v1.json b/benchmarks/opencode_review/pilot_baseline_v1.json new file mode 100644 index 000000000..41cb23cf3 --- /dev/null +++ b/benchmarks/opencode_review/pilot_baseline_v1.json @@ -0,0 +1,186 @@ +{ + "schema_version": "1.0", + "benchmark_id": "cwl-opencode-lifecycle-pilot-2026-08-08", + "evaluation_mode": "historical_lifecycle", + "limitations": [ + "The three pull requests were deliberately selected because both OpenCode and CodeRabbit evidence was observable; this is not a random organization-wide sample.", + "Reviewer outputs were produced on different lifecycle heads, so actionable yield is an operational contrast rather than a head-matched precision or recall comparison.", + "Actionable means that the reviewer emitted a concrete source-oriented finding; this pilot does not label each finding as an expert-verified true positive.", + "CodeRabbit rate-limited evidence is counted only when a GitHub record was directly observed, so availability may be overestimated.", + "No parity claim is permitted until a stratified head-matched expert-gold benchmark satisfies the configured sample-size floor." + ], + "parity_policy": { + "candidate_reviewer": "opencode", + "reference_reviewer": "coderabbit", + "minimum_head_matched_cases": 50, + "minimum_gold_findings": 50, + "non_inferiority_margin": 0.05, + "required_critical_high_recall": 1.0 + }, + "cases": [ + { + "case_id": "disksage-pr-140", + "repository": "ContextualWisdomLab/disksage", + "pull_request_number": 140, + "head_match": false, + "diff_size_bucket": "small", + "primary_language": "rust", + "gold_findings": [], + "reviewers": { + "opencode": { + "triggered_attempts": 4, + "completed_attempts": 4, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 4, + "duplicate_reviews": 3, + "findings": [] + }, + "coderabbit": { + "triggered_attempts": 2, + "completed_attempts": 1, + "rate_limited_attempts": 1, + "infrastructure_only_reviews": 0, + "duplicate_reviews": 0, + "findings": [ + { + "finding_id": "coderabbit-disk-140-publish-boundary", + "gold_finding_id": null, + "severity": "medium", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + } + ] + } + } + }, + { + "case_id": "egressweave-pr-62", + "repository": "ContextualWisdomLab/EgressWeave", + "pull_request_number": 62, + "head_match": false, + "diff_size_bucket": "medium", + "primary_language": "python", + "gold_findings": [], + "reviewers": { + "opencode": { + "triggered_attempts": 2, + "completed_attempts": 2, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 2, + "duplicate_reviews": 1, + "findings": [] + }, + "coderabbit": { + "triggered_attempts": 1, + "completed_attempts": 1, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 0, + "duplicate_reviews": 0, + "findings": [ + { + "finding_id": "coderabbit-egress-62-intermediate-symlink", + "gold_finding_id": null, + "severity": "high", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + }, + { + "finding_id": "coderabbit-egress-62-positive-regression", + "gold_finding_id": null, + "severity": "medium", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + } + ] + } + } + }, + { + "case_id": "inkspan-pr-65", + "repository": "ContextualWisdomLab/inkspan", + "pull_request_number": 65, + "head_match": false, + "diff_size_bucket": "large", + "primary_language": "typescript", + "gold_findings": [], + "reviewers": { + "opencode": { + "triggered_attempts": 2, + "completed_attempts": 2, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 2, + "duplicate_reviews": 1, + "findings": [] + }, + "coderabbit": { + "triggered_attempts": 1, + "completed_attempts": 1, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 0, + "duplicate_reviews": 0, + "findings": [ + { + "finding_id": "coderabbit-inkspan-65-doctoring-dates", + "gold_finding_id": null, + "severity": "low", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + }, + { + "finding_id": "coderabbit-inkspan-65-security-doc-sync", + "gold_finding_id": null, + "severity": "medium", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + }, + { + "finding_id": "coderabbit-inkspan-65-memory-claim", + "gold_finding_id": null, + "severity": "medium", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + }, + { + "finding_id": "coderabbit-inkspan-65-plan-count", + "gold_finding_id": null, + "severity": "low", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + }, + { + "finding_id": "coderabbit-inkspan-65-word-fixture-budget", + "gold_finding_id": null, + "severity": "high", + "actionable": true, + "source_backed": true, + "line_anchored": true, + "has_fix_direction": true, + "has_regression_test_direction": true + } + ] + } + } + } + ] +} diff --git a/docs/doctoring/opencode-review-quality-evaluation.md b/docs/doctoring/opencode-review-quality-evaluation.md new file mode 100644 index 000000000..3d2879da4 --- /dev/null +++ b/docs/doctoring/opencode-review-quality-evaluation.md @@ -0,0 +1,221 @@ +# OpenCode review quality evaluation and commercial parity program + +Status: Proposed empirical baseline +Date: 2026-08-08 +Owner: ContextualWisdomLab central review infrastructure + +## Decision summary + +The organization must not claim that OpenCode Review has reached CodeRabbit quality from prompt length, model reputation, test count, or a few agreeable reviews. Commercial parity is an empirical non-inferiority claim and requires current-head, expert-gold evidence. + +The immediate decision is therefore to: + +1. measure operational review yield from directly observed organization pull requests; +2. keep historical lifecycle evidence separate from head-matched defect precision and recall; +3. require a minimum of 50 head-matched pull requests and 50 expert-gold findings before parity can be evaluated; +4. compare the OpenCode candidate against CodeRabbit with Wilson 95% confidence intervals and a five-percentage-point non-inferiority margin; +5. require 100% recall for expert-labeled critical and high findings; +6. keep semantic review verdicts separate from CI and merge-readiness evidence; +7. operate the upgraded reviewer in shadow mode before it becomes a required merge gate. + +The pilot result is intentionally **`INSUFFICIENT_EVIDENCE`**, not PASS or FAIL. + +## Why a real-pull-request benchmark is mandatory + +Synthetic mutation suites remain useful for deterministic regression tests, but they do not establish production review quality. Kumar, Bararia, and Raj (2026) reported that the best tested model achieved F1 = 0.847 on synthetic samples but only F1 = 0.066 on real pull requests, with performance deteriorating sharply as diff size increased. AACR-Bench likewise argues that raw pull-request comments are incomplete ground truth and uses AI-assisted, expert-verified repository-level annotation, increasing defect coverage by 285% over the original review records (Zhang et al., 2026). + +ContextCRBench further shows that textual issue context and multi-stage filtering materially affect review performance; its industrial deployment reported a 61.98% improvement after adopting context-enriched, filtered evaluation data (Hu et al., 2025). The organization must therefore stratify by language, diff size, defect class, and repository context rather than evaluate only on tiny injected defects. + +## Directly observed organization pilot + +The pilot deliberately selected three pull requests for which both OpenCode and CodeRabbit left observable GitHub records. It is a lifecycle-yield study, not a randomized or head-matched experiment. + +| Case | OpenCode observed output | CodeRabbit observed output | +|---|---|---| +| `ContextualWisdomLab/disksage#140` | Four completed reviews; all were coverage-evidence-only and repeated the same infrastructure blocker; no source defect finding | One completed review after one directly observed rate-limit event; one actionable source finding concerning the `publish = false` package-boundary check | +| `ContextualWisdomLab/EgressWeave#62` | Two completed reviews; both coverage-evidence-only; no source defect finding | One completed review; two actionable findings concerning intermediate-symlink handling and the positive regression contract | +| `ContextualWisdomLab/inkspan#65` | Two completed reviews; both coverage-evidence-only despite PR-supplied direct coverage evidence; no source defect finding | One completed review; five actionable findings covering documentation provenance, security-document synchronization, a memory claim, plan-count consistency, and a word-fixture budget | + +Aggregate pilot result encoded in `benchmarks/opencode_review/pilot_baseline_v1.json`: + +| Metric | OpenCode | CodeRabbit | +|---|---:|---:| +| Triggered attempts with observable records | 8 | 4 | +| Completed attempts | 8 | 3 | +| Directly observed rate-limited attempts | 0 | 1 | +| Actionable source findings | 0 | 8 | +| Infrastructure-only review rate | 100% | 0% | +| Duplicate review rate | 62.5% | 0% | +| Evidenced availability rate | 100% | 75% | + +These values do **not** prove CodeRabbit precision, OpenCode recall, or head-to-head superiority because the reviews occurred on different lifecycle heads and the eight CodeRabbit comments have not yet been adjudicated against an independent expert-gold set. They do establish a serious operational failure mode: OpenCode can be continuously available while producing no semantic source-review value. + +## Root-cause finding in the current central dispatch + +The current `opencode-review-dispatch.yml` contains a deterministic path that constructs and posts a synthetic source-level `REQUEST_CHANGES` review when coverage evidence acquisition fails. The review points at a workflow line and describes missing coverage evidence, then exits before normal semantic review publication. This couples two distinct decisions: + +- **semantic review verdict** — whether the changed source contains an actionable defect; +- **merge readiness** — whether required checks, coverage, approvals, and policy evidence permit merge. + +Coverage evidence may legitimately block approval and merge readiness. It must not be transformed into a source-code defect. An unavailable evidence service is an infrastructure state, not proof that the changed implementation is wrong. + +The production repair is blocked from this pull request because active central branches are already changing the same large dispatch file. Concurrently replacing the file through the contents API would race another writer and risk discarding their exact-head work. The safe follow-up must land after those writers integrate or relinquish the file. + +## Target commercial architecture + +```mermaid +flowchart LR + A["Exact-head PR evidence"] --> B["Context and risk classifier"] + B --> C["Deterministic rule and static-analysis detectors"] + B --> D["NIM semantic detector candidates"] + C --> E["Independent finding verifier"] + D --> E + E --> F["Source-anchor and fix-contract validator"] + F --> G["Deduplication and severity calibration"] + G --> H["Semantic review verdict"] + A --> I["Checks, coverage, approvals, and policy evidence"] + I --> J["Merge-readiness verdict"] + H --> K["Versioned decision envelope"] + J --> K + K --> L["Shadow evaluation or GitHub publication"] +``` + +The versioned decision envelope must carry at least: + +```json +{ + "schema_version": "1.0", + "review_verdict": "APPROVE | REQUEST_CHANGES | COMMENT", + "merge_readiness": "READY | BLOCKED | UNKNOWN", + "findings": [], + "evidence_manifest": {}, + "model_attempts": [], + "quality_policy_version": "..." +} +``` + +### Detector and verifier separation + +BitsAI-CR uses a RuleChecker followed by a ReviewFilter and reports 75.0% precision in production, supporting an explicit detector–verifier split rather than trusting raw model comments (Sun et al., 2025). RovoDev similarly describes a context-aware, quality-checked review pipeline and reports that 38.7% of comments led to subsequent code changes, alongside a 30.8% reduction in pull-request cycle time (Tantithamthavorn et al., 2026). + +The verifier must reject a candidate finding unless it contains: + +- a changed or connected source path and positive line; +- a concrete trigger condition; +- an observable impact; +- a source-backed root cause; +- a minimal fix direction; +- an exact regression target or verification command when the repository exposes one; +- no duplication of an already published current-head finding. + +For executable defects, fix-guided verification can compare the original and proposed repair against trusted tests. This must remain a filter, not a general proof mechanism, because documentation, architecture, and untested behavior may have no executable oracle. Overcorrection research warns that more detailed prompts can increase false rejection and recommends verification of proposed fixes before accepting model judgments (Jin & Chen, 2026). + +### Adaptive test-time compute + +The upgraded reviewer should not run a fixed multi-agent topology on every pull request. Fugu, Conductor, and TRINITY support task-adaptive orchestration rather than uniform model use: + +- Fugu learns query-adaptive agentic scaffolds and exposes a quality-prioritized deeper mode for hard tasks (Tang et al., 2026). +- Conductor learns targeted instructions and communication topologies, including recursive test-time scaling, over heterogeneous worker pools (Nielsen et al., 2025). +- TRINITY dynamically assigns Thinker, Worker, and Verifier roles to selected models over multiple turns (Xu et al., 2025). + +For CWL review, a bounded risk classifier should allocate compute as follows: + +| Pull-request class | Default topology | +|---|---| +| Small documentation or metadata change with no behavioral claim | one detector, deterministic cross-file checks, one verifier only if a finding exists | +| Ordinary source change | one semantic detector plus independent verifier | +| Security, auth, data-loss, migration, numerical, or workflow-trust change | diverse parallel detectors, role-specific verifier, evidence synthesis | +| Large or cross-service change | decomposition by execution path, independent repository-context retrieval, multiple specialist detectors, final verifier | +| Ambiguous finding or detector disagreement | bounded recursive verification; no automatic publication when the budget is exhausted | + +Speed is not the primary objective. The optimization target is expert-gold defect utility under a bounded, reproducible compute budget. + +## Benchmark schema and metrics + +Two evaluation modes are deliberately separated. + +### `historical_lifecycle` + +Used for operational telemetry when reviews are not head-matched or expert-adjudicated. It reports: + +- triggered, completed, and rate-limited attempts; +- availability and Wilson interval; +- infrastructure-only review rate; +- duplicate review rate; +- actionable findings per completed review; +- source-backed, line-anchored, fix-direction, and regression-direction rates. + +It must never output precision, recall, F1, or a parity PASS. + +### `head_matched_gold` + +Used only when both reviewers inspect the same immutable head and independent experts label all relevant defects. It additionally reports: + +- true positives, false positives, and false negatives; +- precision, recall, and F1; +- Wilson 95% intervals for precision and recall; +- critical/high recall; +- reference-relative non-inferiority. + +A repeated model finding mapped to the same gold finding counts as a false positive after the first match. An unmatched actionable finding is a false positive. An unlabeled or unresolved expert disagreement must be excluded from the finalized gold set rather than silently assigned to either reviewer. + +## Commercial release gates + +OpenCode Review may be described as CodeRabbit-level only when all conditions hold on a frozen benchmark version: + +1. at least 50 head-matched pull requests and 50 expert-gold findings; +2. at least four primary languages, all three diff-size buckets, and security, correctness, performance, workflow, documentation, and data-model defect classes; +3. candidate precision lower 95% Wilson bound at least CodeRabbit point precision minus 0.05; +4. candidate recall lower 95% Wilson bound at least CodeRabbit point recall minus 0.05; +5. critical/high recall = 1.00; +6. source-backed and positive-line anchored rate = 1.00 for published blockers; +7. infrastructure-only source findings = 0; +8. duplicate current-head publication rate < 0.05; +9. schema-valid output rate = 1.00; +10. no stale-head evidence accepted; +11. resolution rate, reviewer rejection rate, and time-to-resolution tracked in shadow mode; +12. expert audit finds no credential, tenant, prompt-injection, or evidence-provenance regression. + +The reference is a moving commercial product. Benchmark snapshots must record CodeRabbit configuration, profile, path instructions, timestamp, and exact output records. CodeRabbit supports configurable review profiles, path-specific instructions, incremental and full-review modes, and Autofix; comparisons must therefore state which features and settings were enabled rather than treating the product as a single immutable model. + +## Rollout + +1. **Baseline** — land the scorer, pilot, CI, and doctoring with parity unavailable. +2. **Collection** — sample pull requests by language, size, and risk; capture both reviewers on identical heads. +3. **Expert gold** — use two independent reviewers plus adjudication; retain evidence and disagreement reason. +4. **Shadow orchestration** — run detector/verifier topologies without publishing blockers. +5. **Calibration** — tune routing, verifier thresholds, duplicate suppression, and severity mapping against the development split only. +6. **Frozen test** — evaluate once on a held-out benchmark; publish confidence intervals and failures. +7. **Limited publication** — publish comments but keep OpenCode non-required; measure developer resolution and rejection. +8. **Required gate** — enable only after the commercial release gates remain satisfied for two consecutive frozen benchmark versions. + +## Security and governance + +- Pull-request content and reviewer comments are untrusted input. +- The evaluator must never execute model-proposed commands or fixes. +- Trusted execution receipts must be produced outside the model process. +- Secrets, cookies, tokens, and raw credential material must not enter model context or benchmark artifacts. +- `NVIDIA_NIM_API_KEY` remains the credential for scheduled OpenCode model calls; this program does not introduce `COPILOT_GITHUB_TOKEN`. +- Benchmark artifacts require exact repository, pull request, head SHA, base SHA, reviewer configuration, timestamp, and source receipt provenance. +- Human reviewer identities may be pseudonymized in exported benchmark data while preserving adjudication independence. +- Benchmark and model-routing changes require versioned doctoring and a new held-out evaluation. + +## References + +Hu, R., Wang, X., Wen, X.-C., Zhang, Z., Jiang, B., Gao, P., Peng, C., & Gao, C. (2025). *Benchmarking LLMs for fine-grained code review with enriched context in practice* (arXiv:2511.07017). arXiv. https://doi.org/10.48550/arXiv.2511.07017 + +Jin, H., & Chen, H. (2026). *Are LLMs reliable code reviewers? Systematic overcorrection in requirement conformance judgement* (arXiv:2603.00539). arXiv. https://arxiv.org/abs/2603.00539 + +Kumar, S. P., Bararia, S., & Raj, K. (2026). *Bigger isn't always better: A comparative evaluation of LLMs for automated code review* (arXiv:2606.15689). arXiv. https://arxiv.org/abs/2606.15689 + +Nielsen, S., Cetin, E., Schwendeman, P., Sun, Q., Xu, J., & Tang, Y. (2025). *Learning to orchestrate agents in natural language with the Conductor* (arXiv:2512.04388). arXiv. https://arxiv.org/abs/2512.04388 + +Sun, T., Xu, J., Li, Y., Yan, Z., Zhang, G., Xie, L., Geng, L., Wang, Z., Chen, Y., Lin, Q., Duan, W., & Sui, K. (2025). BitsAI-CR: Automated code review via LLM in practice. In *Proceedings of the 33rd ACM International Conference on the Foundations of Software Engineering*. https://doi.org/10.1145/3696630.3728552 + +Tang, Y., Cetin, E., Xu, J., Sun, Q., Nielsen, S., Richard, V., Goda, H., Tymchenko, I., Nguyen, N., Lee, H., Ashiga, M., Kotyan, S., Kuroki, S., & Clanuwat, T. (2026). *Sakana Fugu technical report* (arXiv:2606.21228). arXiv. https://arxiv.org/abs/2606.21228 + +Tantithamthavorn, K., Zou, Y., Wong, A., Gupta, M., Wang, Z., Buller, M., Jiang, R., Watson, M., Jeong, M., Chen, K., & Wu, M. (2026). *RovoDev Code Reviewer: A large-scale online evaluation of LLM-based code review automation at Atlassian* (arXiv:2601.01129). arXiv. https://arxiv.org/abs/2601.01129 + +Xu, J., Sun, Q., Schwendeman, P., Nielsen, S., Cetin, E., & Tang, Y. (2025). *TRINITY: An evolved LLM coordinator* (arXiv:2512.04695). arXiv. https://arxiv.org/abs/2512.04695 + +Zhang, L., Yu, Y., Yu, M., Guo, X., Zhuang, Z., Rong, G., Shao, D., Shen, H., Kuang, H., Li, Z., Wang, B., Zhang, G., Xiang, B., & Xu, X. (2026). *AACR-Bench: Evaluating automatic code review with holistic repository-level context* (arXiv:2601.19494). arXiv. https://arxiv.org/abs/2601.19494 diff --git a/docs/superpowers/plans/2026-08-08-opencode-review-quality.md b/docs/superpowers/plans/2026-08-08-opencode-review-quality.md new file mode 100644 index 000000000..6e07483c4 --- /dev/null +++ b/docs/superpowers/plans/2026-08-08-opencode-review-quality.md @@ -0,0 +1,147 @@ +# OpenCode Review quality implementation plan + +> Execute in protected, exact-head slices. Do not edit the central dispatch concurrently with another active writer. + +**Goal:** Establish a defensible empirical quality gate, then use it to evolve OpenCode Review toward commercial non-inferiority with CodeRabbit. + +**Architecture:** A deterministic benchmark and scorer measure reviewer operations and expert-gold defect quality. The production reviewer is subsequently split into semantic review and merge-readiness channels, with detector, verifier, source-contract, deduplication, and publication stages. Model compute is allocated by PR risk and disagreement rather than a fixed topology. + +**Technology:** Python 3.14 standard library, pytest, coverage.py, GitHub Actions, OpenCode, NVIDIA NIM, GitHub review APIs. + +--- + +## Task 1: Record a fail-first empirical contract + +**Files:** +- Create: `benchmarks/opencode_review/pilot_baseline_v1.json` +- Create: `tests/test_opencode_review_quality_score.py` +- Create: `.github/workflows/opencode-review-quality-ci.yml` + +**Steps:** +1. Encode only directly observed review lifecycle records. +2. Mark every case as not head-matched and include explicit limitations. +3. Write a test that imports the not-yet-created scorer and asserts `INSUFFICIENT_EVIDENCE` plus the pilot operational metrics. +4. Open a Draft PR at the exact head. +5. Run the pinned workflow and capture the expected missing-scorer failure. +6. Do not count a predecessor-head failure as the RED receipt. + +**Exit evidence:** exact-head workflow failure at the missing production scorer. + +## Task 2: Implement the scorer test-first + +**Files:** +- Create: `scripts/ci/opencode_review_quality_score.py` +- Expand: `tests/test_opencode_review_quality_score.py` + +**Steps:** +1. Validate schema version, evaluation mode, case identifiers, repository coordinates, counts, rates, severities, and evidence links. +2. Reject stale-head gold mappings and duplicate case/finding identifiers. +3. Compute lifecycle availability, infrastructure-only, duplicate, actionable-yield, source-anchor, fix-direction, and regression-direction metrics. +4. Compute head-matched true positives, false positives, false negatives, precision, recall, F1, and critical/high recall. +5. Add Wilson 95% intervals and the five-point non-inferiority decision. +6. Return `INSUFFICIENT_EVIDENCE` for lifecycle, underpowered, missing-reviewer, and zero-denominator cases. +7. Add deterministic Markdown/JSON output and atomic file replacement. +8. Exercise every production branch and enforce all production callable docstrings. + +**Verification:** +```bash +python -m coverage run --branch --source=scripts/ci \ + -m pytest tests/test_opencode_review_quality_score.py -q +python -m coverage report \ + --include='scripts/ci/opencode_review_quality_score.py' \ + --fail-under=100 --show-missing +python scripts/ci/opencode_review_quality_score.py \ + --input benchmarks/opencode_review/pilot_baseline_v1.json \ + --json-output /tmp/opencode-review-quality.json \ + --markdown-output /tmp/opencode-review-quality.md +``` + +## Task 3: Doctor the decision and benchmark limitations + +**Files:** +- Create: `docs/doctoring/opencode-review-quality-evaluation.md` +- Create: `docs/superpowers/specs/2026-08-08-opencode-review-quality.md` +- Modify: `CHANGELOG.md` + +**Steps:** +1. Document the purposive sampling method and exact observed metrics. +2. State that different lifecycle heads prevent precision, recall, and parity inference. +3. Document the central coverage-to-source-review coupling defect. +4. Specify detector/verifier separation and semantic/merge-readiness separation. +5. Reference real-PR code-review evaluation, industrial filtering, contextual benchmarks, overcorrection, Fugu, Conductor, and TRINITY in APA 7th form. +6. Record the CI, scorer, and baseline in the changelog. + +## Task 4: Build a head-matched expert-gold corpus + +**Files:** +- Create: `benchmarks/opencode_review/head_matched_v1/*.json` +- Create: `docs/doctoring/opencode-review-annotation-guide.md` +- Create: `scripts/ci/opencode_review_sample.py` +- Create: `scripts/ci/opencode_review_adjudicate.py` +- Create tests for each production script. + +**Steps:** +1. Stratify at least 50 PR heads by language, diff size, risk, and defect class. +2. Pin exact base and head SHAs before either reviewer runs. +3. Run OpenCode and CodeRabbit with recorded configurations against identical heads. +4. Have two independent experts label defects from full repository context. +5. Adjudicate disagreements without showing reviewer identity. +6. Freeze the gold set and hash every record. +7. Keep the final held-out set unavailable to routing and prompt calibration. + +## Task 5: Separate semantic verdict from merge readiness + +**Files:** +- Modify only after lease clearance: `.github/workflows/opencode-review-dispatch.yml` +- Create: `scripts/ci/opencode_review_decision.py` +- Create: `tests/test_opencode_review_decision.py` + +**Steps:** +1. Write failing tests proving coverage failure cannot create a source finding. +2. Define a versioned decision envelope with `review_verdict` and `merge_readiness` as independent fields. +3. Move coverage/check/approval logic into merge readiness. +4. Keep semantic review active whenever exact-head bounded source evidence is available. +5. Publish infrastructure blockers as check summaries, never line-level source defects. +6. Preserve fail-closed approval and branch protection. +7. Verify the exact current dispatch head before every write. + +## Task 6: Add detector–verifier orchestration in shadow mode + +**Files:** +- Create: `scripts/ci/run_opencode_semantic_review_pool.sh` +- Create: `scripts/ci/opencode_review_verify.py` +- Modify after lease clearance: `.github/workflows/opencode-review-dispatch.yml` +- Add complete contract, fuzz, and adversarial tests. + +**Steps:** +1. Classify PR risk, languages, paths, and diff-size bucket deterministically. +2. Route low-risk PRs to a single detector and high-risk PRs to diverse specialist detectors. +3. Use independent verifier calls for every publication candidate. +4. Require path, positive line, trigger, impact, root cause, fix direction, and regression direction. +5. Suppress stale, duplicate, unsupported, and infrastructure-only findings. +6. Record model, provider, role, prompt hash, reasoning effort, and evidence hash. +7. Use `NVIDIA_NIM_API_KEY`; do not add or repurpose `COPILOT_GITHUB_TOKEN`. +8. Publish metrics only; do not publish GitHub blockers during shadow mode. + +## Task 7: Calibrate and run the held-out comparison + +**Steps:** +1. Tune routing and verifier thresholds only on the development split. +2. Run ablations for one detector, detector+verifier, parallel detectors, role-specific effort, and bounded recursion. +3. Evaluate once on the frozen held-out set. +4. Require the scorer's eligibility and commercial gates. +5. Publish per-language, per-size, and per-defect-class failures, not only aggregate F1. +6. Preserve failed evidence and negative results in doctoring. + +## Task 8: Limited production rollout + +**Steps:** +1. Publish non-required OpenCode comments to a bounded repository cohort. +2. Track developer resolution, rejection, duplicate, and time-to-resolution metrics. +3. Compare reviewer utility and cycle time with the reference cohort. +4. Roll back automatically on critical/high miss, source-contract failure, or false-blocker spike. +5. Make OpenCode required only after two consecutive frozen benchmark versions pass and independent approval is obtained. + +## Completion definition + +The program is complete only when the exact-head held-out gate passes, critical/high recall is 100%, infrastructure-only source findings are zero, duplicate publication is below 5%, protected-branch integration is green, and independent human reviewers approve the transition. Until then, the correct status is **improving, not CodeRabbit-equivalent**. diff --git a/docs/superpowers/specs/2026-08-08-opencode-review-quality.md b/docs/superpowers/specs/2026-08-08-opencode-review-quality.md new file mode 100644 index 000000000..1a1e6a6a8 --- /dev/null +++ b/docs/superpowers/specs/2026-08-08-opencode-review-quality.md @@ -0,0 +1,135 @@ +# OpenCode Review commercial-quality design + +Date: 2026-08-08 +Status: Proposed +Scope: ContextualWisdomLab central `.github` review infrastructure + +## Problem + +OpenCode Review currently conflates missing CI or coverage evidence with a semantic source defect. In the observed organization pilot, this produced eight completed reviews that were all infrastructure-only and yielded no actionable source findings, while CodeRabbit produced eight actionable comments across three completed reviews. Because the reviewers did not inspect identical immutable heads and no independent expert gold was available, the observation establishes an operational-yield gap but not precision, recall, or product parity. + +## Goals + +- Make review-quality claims falsifiable and reproducible. +- Preserve exact-head provenance for every benchmark case. +- Separate semantic source review from merge readiness. +- Add a detector–verifier pipeline with bounded adaptive compute. +- Suppress stale, duplicate, unsupported, and infrastructure-only source comments. +- Require statistical non-inferiority before claiming CodeRabbit-level quality. +- Preserve existing NVIDIA NIM credentials and repository protections. + +## Non-goals + +- Replacing CodeRabbit before empirical parity is proven. +- Treating synthetic mutations as sufficient evidence. +- Weakening coverage, security, or branch-protection requirements. +- Editing the central dispatch while another active branch owns overlapping lines. +- Introducing `COPILOT_GITHUB_TOKEN`. + +## Options considered + +### Prompt-only expansion + +Rejected as the primary solution. Longer prompts can increase review verbosity without improving correctness, and overcorrection research indicates that detailed explanations and proposed fixes can amplify false rejection unless findings are independently verified. + +### Larger single model + +Rejected as the primary solution. Recent real-PR evaluation found that model size did not reliably predict review quality and that all evaluated systems degraded sharply on realistic diffs. + +### Empirical detector–verifier system + +Selected. Candidate findings are generated by deterministic detectors and one or more semantic models, then independently verified, source-anchored, deduplicated, severity-calibrated, and measured against expert gold. Semantic verdict and merge readiness remain separate outputs. + +## Architecture + +```text +exact-head evidence + -> risk/context classifier + -> deterministic detectors + semantic detector pool + -> independent verifier + -> source-anchor/fix-contract validation + -> duplicate and severity calibration + -> semantic review verdict + +exact-head checks + coverage + reviews + policy + -> merge-readiness verdict + +semantic verdict + merge readiness + -> versioned decision envelope + -> shadow metrics or GitHub publication +``` + +## Adaptive compute policy + +- Low-risk metadata changes: one detector; verifier only when a finding exists. +- Ordinary source changes: one detector plus an independent verifier. +- Security, authorization, destructive migration, numerical, and workflow-trust changes: diverse parallel detectors plus specialist verification. +- Large cross-service changes: execution-path decomposition, repository-context retrieval, specialist workers, and final synthesis. +- Detector disagreement: bounded recursive verification; abstain when the budget or evidence contract is exhausted. + +This applies the role separation and task-adaptive routing patterns described by Fugu, Conductor, and TRINITY without assuming their reported benchmark results transfer directly to code review. + +## Benchmark design + +The benchmark has two modes: + +- `historical_lifecycle`: operational telemetry only; never reports precision, recall, or parity. +- `head_matched_gold`: both reviewers inspect the same immutable head and independent experts provide complete defect labels. + +Required case dimensions: + +- exact head and base SHA; +- repository, PR, language, diff-size bucket, and defect class; +- reviewer configuration and timestamp; +- triggered, completed, limited, and infrastructure-only attempts; +- finding path, line, severity, evidence, fix direction, and regression direction; +- expert-gold mapping and adjudication receipt. + +## Statistical gate + +Eligibility requires at least 50 head-matched PRs and 50 expert-gold findings. The candidate passes only when: + +- precision lower 95% Wilson bound >= reference point precision - 0.05; +- recall lower 95% Wilson bound >= reference point recall - 0.05; +- critical/high recall = 1.00; +- infrastructure-only source findings = 0; +- duplicate review rate < 0.05; +- all blocker findings are source-backed and positively line-anchored. + +Underpowered or lifecycle-only evidence returns `INSUFFICIENT_EVIDENCE`. + +## Failure handling + +- Missing coverage: `merge_readiness=BLOCKED`; semantic review still runs when bounded source evidence exists. +- Missing semantic evidence: `review_verdict=COMMENT` or abstain; never fabricate source defects. +- Model failure: record the attempt and fall back according to policy; no deterministic approval. +- Stale head: discard all findings and rerun on the current head. +- Duplicate finding: suppress publication and retain a metric record. +- Verifier disagreement: abstain or escalate; do not average unsupported assertions. +- Rate limit: record availability impact; do not count the attempt as a completed semantic review. + +## Security + +PR content is untrusted. Models cannot execute commands, publish reviews, read secrets, or alter evidence. Trusted source and execution receipts are produced by runner-owned deterministic steps. All artifacts are versioned and bounded. The production model path continues to use `NVIDIA_NIM_API_KEY` and existing credential boundaries. + +## Rollout + +1. Baseline scorer and purposive pilot. +2. Head-matched collection and expert annotation. +3. Decision-envelope seam in the central dispatch. +4. Detector/verifier shadow orchestration. +5. Calibration on a development split. +6. One-time held-out evaluation. +7. Non-required publication with developer-outcome metrics. +8. Required gate only after two consecutive frozen benchmark passes. + +## Acceptance criteria for this design slice + +- Fail-first exact-head CI evidence exists. +- The scorer rejects malformed evidence fail-closed. +- Historical evidence cannot produce precision, recall, or parity. +- Head-matched mode computes precision, recall, F1, critical/high recall, and Wilson intervals. +- Production scorer statement and branch coverage are 100%. +- All locally defined production callables have docstrings. +- The pilot result is `INSUFFICIENT_EVIDENCE` and reports the observed yield gap. +- No production dispatch, credentials, or branch protections are changed in this slice. diff --git a/scripts/ci/opencode_review_quality_score.py b/scripts/ci/opencode_review_quality_score.py new file mode 100644 index 000000000..4bd2c56d0 --- /dev/null +++ b/scripts/ci/opencode_review_quality_score.py @@ -0,0 +1,733 @@ +#!/usr/bin/env python3 +"""Validate and score empirical automated-review quality evidence.""" + +from __future__ import annotations + +import argparse +import json +import math +import re +import sys +from collections.abc import Mapping, Sequence +from pathlib import Path +from typing import Any + +VALID_MODES = {"historical_lifecycle", "head_matched_gold"} +VALID_BUCKETS = {"small", "medium", "large"} +VALID_SEVERITIES = {"critical", "high", "medium", "low"} +REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") +COMMIT_SHA_RE = re.compile(r"^[0-9a-f]{40}$") + + +class BenchmarkValidationError(ValueError): + """Signal malformed or internally inconsistent benchmark evidence.""" + + +def reject(message: str) -> None: + """Raise a stable benchmark validation error.""" + raise BenchmarkValidationError(message) + + +def object_value(value: Any, path: str) -> Mapping[str, Any]: + """Return a mapping or reject a schema-shape mismatch.""" + if not isinstance(value, Mapping): + reject(f"{path} must be an object") + return value + + +def require_exact_fields( + value: Mapping[str, Any], path: str, allowed_fields: set[str] +) -> None: + """Reject unreviewed object fields so every schema layer stays fail closed.""" + unknown_fields = sorted(set(value) - allowed_fields) + if unknown_fields: + reject(f"{path} has unknown fields: {', '.join(unknown_fields)}") + + +def array_value(value: Any, path: str) -> list[Any]: + """Return a list or reject a schema-shape mismatch.""" + if not isinstance(value, list): + reject(f"{path} must be an array") + return value + + +def text_value(value: Any, path: str) -> str: + """Return stripped non-empty text or reject it.""" + if not isinstance(value, str) or not value.strip(): + reject(f"{path} must be non-empty text") + return value.strip() + + +def bool_value(value: Any, path: str) -> bool: + """Return an actual Boolean, excluding integer lookalikes.""" + if not isinstance(value, bool): + reject(f"{path} must be boolean") + return value + + +def count_value(value: Any, path: str, *, positive: bool = False) -> int: + """Return a non-negative or positive integer count.""" + if isinstance(value, bool) or not isinstance(value, int) or value < 0: + reject(f"{path} must be a non-negative integer") + if positive and value == 0: + reject(f"{path} must be positive") + return value + + +def rate_value(value: Any, path: str) -> float: + """Return a finite rate in the closed unit interval.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + reject(f"{path} must be a number between 0 and 1") + result = float(value) + if not math.isfinite(result) or not 0.0 <= result <= 1.0: + reject(f"{path} must be a finite number between 0 and 1") + return result + + +def unique_text(value: Any, path: str, seen: set[str]) -> str: + """Return a unique non-empty identifier within a caller-owned set.""" + result = text_value(value, path) + if result in seen: + reject(f"{path} duplicates {result!r}") + seen.add(result) + return result + + +def commit_sha_value(value: Any, path: str, *, required: bool) -> str | None: + """Return one lowercase full commit SHA or reject missing/malformed evidence.""" + if value is None and not required: + return None + result = text_value(value, path) + if not COMMIT_SHA_RE.fullmatch(result): + reject(f"{path} must be a 40-character lowercase commit SHA") + return result + + +def validate_finding( + raw_value: Any, + path: str, + seen: set[str], + gold_ids: set[str], + head_match: bool, +) -> dict[str, Any]: + """Validate one reviewer-emitted finding and its evidence attributes.""" + value = object_value(raw_value, path) + require_exact_fields( + value, + path, + { + "finding_id", + "gold_finding_id", + "severity", + "actionable", + "source_backed", + "line_anchored", + "has_fix_direction", + "has_regression_test_direction", + }, + ) + finding_id = unique_text(value.get("finding_id"), f"{path}.finding_id", seen) + gold_id = value.get("gold_finding_id") + if gold_id is not None: + gold_id = text_value(gold_id, f"{path}.gold_finding_id") + if not head_match or gold_id not in gold_ids: + reject(f"{path}.gold_finding_id must reference current-head gold evidence") + severity = text_value(value.get("severity"), f"{path}.severity").casefold() + if severity not in VALID_SEVERITIES: + reject(f"{path}.severity is invalid") + return { + "finding_id": finding_id, + "gold_finding_id": gold_id, + "severity": severity, + "actionable": bool_value(value.get("actionable"), f"{path}.actionable"), + "source_backed": bool_value( + value.get("source_backed"), f"{path}.source_backed" + ), + "line_anchored": bool_value( + value.get("line_anchored"), f"{path}.line_anchored" + ), + "has_fix_direction": bool_value( + value.get("has_fix_direction"), f"{path}.has_fix_direction" + ), + "has_regression_test_direction": bool_value( + value.get("has_regression_test_direction"), + f"{path}.has_regression_test_direction", + ), + } + + +def validate_reviewer( + raw_value: Any, + path: str, + gold_ids: set[str], + head_match: bool, + expected_head_sha: str | None, +) -> dict[str, Any]: + """Validate one reviewer's attempts, blockers, duplicates, and findings.""" + value = object_value(raw_value, path) + require_exact_fields( + value, + path, + { + "triggered_attempts", + "completed_attempts", + "rate_limited_attempts", + "infrastructure_only_reviews", + "duplicate_reviews", + "reviewed_head_sha", + "findings", + }, + ) + fields = { + name: count_value(value.get(name), f"{path}.{name}") + for name in ( + "triggered_attempts", + "completed_attempts", + "rate_limited_attempts", + "infrastructure_only_reviews", + "duplicate_reviews", + ) + } + triggered = fields["triggered_attempts"] + completed = fields["completed_attempts"] + limited = fields["rate_limited_attempts"] + if completed + limited > triggered: + reject(f"{path} completed plus rate-limited attempts exceed triggered attempts") + if fields["infrastructure_only_reviews"] > completed: + reject(f"{path}.infrastructure_only_reviews exceeds completed_attempts") + if fields["duplicate_reviews"] > completed: + reject(f"{path}.duplicate_reviews exceeds completed_attempts") + reviewed_head_sha = commit_sha_value( + value.get("reviewed_head_sha"), + f"{path}.reviewed_head_sha", + required=expected_head_sha is not None, + ) + if expected_head_sha is not None and reviewed_head_sha != expected_head_sha: + reject(f"{path}.reviewed_head_sha must equal the case head_sha") + seen: set[str] = set() + fields["findings"] = [ + validate_finding(item, f"{path}.findings[{index}]", seen, gold_ids, head_match) + for index, item in enumerate(array_value(value.get("findings"), f"{path}.findings")) + ] + if fields["findings"] and completed == 0: + reject(f"{path}.findings require at least one completed review attempt") + fields["reviewed_head_sha"] = reviewed_head_sha + return fields + + +def validate_benchmark(raw_value: Any) -> dict[str, Any]: + """Validate and normalize the benchmark schema without external packages.""" + value = object_value(raw_value, "benchmark") + require_exact_fields( + value, + "benchmark", + { + "schema_version", + "benchmark_id", + "evaluation_mode", + "limitations", + "parity_policy", + "cases", + }, + ) + if value.get("schema_version") != "1.0": + reject("schema_version must equal '1.0'") + benchmark_id = text_value(value.get("benchmark_id"), "benchmark_id") + mode = text_value(value.get("evaluation_mode"), "evaluation_mode").casefold() + if mode not in VALID_MODES: + reject("evaluation_mode is invalid") + limitations = [ + text_value(item, f"limitations[{index}]") + for index, item in enumerate(array_value(value.get("limitations"), "limitations")) + ] + if not limitations: + reject("limitations must not be empty") + policy_value = object_value(value.get("parity_policy"), "parity_policy") + require_exact_fields( + policy_value, + "parity_policy", + { + "candidate_reviewer", + "reference_reviewer", + "minimum_head_matched_cases", + "minimum_gold_findings", + "non_inferiority_margin", + "required_critical_high_recall", + }, + ) + policy = { + "candidate_reviewer": text_value( + policy_value.get("candidate_reviewer"), "parity_policy.candidate_reviewer" + ).casefold(), + "reference_reviewer": text_value( + policy_value.get("reference_reviewer"), "parity_policy.reference_reviewer" + ).casefold(), + "minimum_head_matched_cases": count_value( + policy_value.get("minimum_head_matched_cases"), + "parity_policy.minimum_head_matched_cases", + positive=True, + ), + "minimum_gold_findings": count_value( + policy_value.get("minimum_gold_findings"), + "parity_policy.minimum_gold_findings", + positive=True, + ), + "non_inferiority_margin": rate_value( + policy_value.get("non_inferiority_margin"), + "parity_policy.non_inferiority_margin", + ), + "required_critical_high_recall": rate_value( + policy_value.get("required_critical_high_recall"), + "parity_policy.required_critical_high_recall", + ), + } + if policy["candidate_reviewer"] == policy["reference_reviewer"]: + reject("candidate and reference reviewers must differ") + + cases: list[dict[str, Any]] = [] + seen_cases: set[str] = set() + for case_index, raw_case in enumerate(array_value(value.get("cases"), "cases")): + path = f"cases[{case_index}]" + case = object_value(raw_case, path) + require_exact_fields( + case, + path, + { + "case_id", + "repository", + "pull_request_number", + "head_match", + "base_sha", + "head_sha", + "diff_size_bucket", + "primary_language", + "gold_findings", + "reviewers", + }, + ) + case_id = unique_text(case.get("case_id"), f"{path}.case_id", seen_cases) + repository = text_value(case.get("repository"), f"{path}.repository") + if not REPOSITORY_RE.fullmatch(repository): + reject(f"{path}.repository must use owner/name") + head_match = bool_value(case.get("head_match"), f"{path}.head_match") + if mode == "head_matched_gold" and not head_match: + reject(f"{path}.head_match must be true in head_matched_gold mode") + base_sha = commit_sha_value( + case.get("base_sha"), f"{path}.base_sha", required=head_match + ) + head_sha = commit_sha_value( + case.get("head_sha"), f"{path}.head_sha", required=head_match + ) + bucket = text_value( + case.get("diff_size_bucket"), f"{path}.diff_size_bucket" + ).casefold() + if bucket not in VALID_BUCKETS: + reject(f"{path}.diff_size_bucket is invalid") + gold_seen: set[str] = set() + gold_findings: list[dict[str, str]] = [] + for gold_index, raw_gold in enumerate( + array_value(case.get("gold_findings"), f"{path}.gold_findings") + ): + gold_path = f"{path}.gold_findings[{gold_index}]" + gold = object_value(raw_gold, gold_path) + require_exact_fields(gold, gold_path, {"finding_id", "severity"}) + severity = text_value( + gold.get("severity"), f"{gold_path}.severity" + ).casefold() + if severity not in VALID_SEVERITIES: + reject(f"{gold_path}.severity is invalid") + gold_findings.append( + { + "finding_id": unique_text( + gold.get("finding_id"), f"{gold_path}.finding_id", gold_seen + ), + "severity": severity, + } + ) + if gold_findings and not head_match: + reject(f"{path}.gold_findings require head_match=true") + reviewers_value = object_value(case.get("reviewers"), f"{path}.reviewers") + if not reviewers_value: + reject(f"{path}.reviewers must not be empty") + reviewers: dict[str, dict[str, Any]] = {} + reviewer_names: set[str] = set() + for name, reviewer in reviewers_value.items(): + normalized_name = text_value(name, f"{path}.reviewer").casefold() + if normalized_name in reviewer_names: + reject(f"{path}.reviewer name duplicates {normalized_name!r}") + reviewer_names.add(normalized_name) + reviewers[normalized_name] = validate_reviewer( + reviewer, + f"{path}.reviewers.{name}", + gold_seen, + head_match, + head_sha if head_match else None, + ) + if head_match and ( + policy["candidate_reviewer"] not in reviewers + or policy["reference_reviewer"] not in reviewers + ): + reject(f"{path} candidate or reference reviewer is absent") + cases.append( + { + "case_id": case_id, + "repository": repository, + "pull_request_number": count_value( + case.get("pull_request_number"), + f"{path}.pull_request_number", + positive=True, + ), + "head_match": head_match, + "base_sha": base_sha, + "head_sha": head_sha, + "diff_size_bucket": bucket, + "primary_language": text_value( + case.get("primary_language"), f"{path}.primary_language" + ).casefold(), + "gold_findings": gold_findings, + "reviewers": reviewers, + } + ) + if not cases: + reject("cases must not be empty") + return { + "schema_version": "1.0", + "benchmark_id": benchmark_id, + "evaluation_mode": mode, + "limitations": limitations, + "parity_policy": policy, + "cases": cases, + } + + +def ratio(numerator: int, denominator: int) -> float | None: + """Return a six-decimal ratio or ``None`` for an empty denominator.""" + return None if denominator == 0 else round(numerator / denominator, 6) + + +def wilson(successes: int, trials: int) -> list[float] | None: + """Return a 95% Wilson binomial interval or ``None`` for no trials.""" + if trials == 0: + return None + if successes < 0 or successes > trials: + raise ValueError("successes must be between zero and trials") + z = 1.959963984540054 + proportion = successes / trials + denominator = 1.0 + z * z / trials + center = (proportion + z * z / (2.0 * trials)) / denominator + margin = z * math.sqrt( + proportion * (1.0 - proportion) / trials + z * z / (4.0 * trials * trials) + ) / denominator + return [round(max(0.0, center - margin), 6), round(min(1.0, center + margin), 6)] + + +def score_reviewer(name: str, benchmark: Mapping[str, Any]) -> dict[str, Any]: + """Aggregate operational metrics and justified head-matched defect metrics.""" + totals = { + "triggered_attempts": 0, + "completed_attempts": 0, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 0, + "duplicate_reviews": 0, + } + findings: list[dict[str, Any]] = [] + gold: dict[tuple[str, str], str] = {} + case_count = 0 + actionable_cases = 0 + for case in benchmark["cases"]: + for item in case["gold_findings"]: + gold[(case["case_id"], item["finding_id"])] = item["severity"] + reviewer = case["reviewers"].get(name) + if reviewer is None: + continue + case_count += 1 + for field in totals: + totals[field] += reviewer[field] + case_findings = [item for item in reviewer["findings"] if item["actionable"]] + actionable_cases += int(bool(case_findings)) + for item in case_findings: + findings.append({**item, "case_id": case["case_id"]}) + actionable = len(findings) + completed = totals["completed_attempts"] + result = { + "reviewer": name, + "case_count": case_count, + **totals, + "availability_rate": ratio(totals["completed_attempts"], totals["triggered_attempts"]), + "availability_interval_95": wilson( + totals["completed_attempts"], totals["triggered_attempts"] + ), + "rate_limited_rate": ratio( + totals["rate_limited_attempts"], totals["triggered_attempts"] + ), + "infrastructure_only_review_rate": ratio( + totals["infrastructure_only_reviews"], completed + ), + "duplicate_review_rate": ratio(totals["duplicate_reviews"], completed), + "actionable_findings": actionable, + "actionable_findings_per_completed_review": ratio(actionable, completed), + "actionable_case_rate": ratio(actionable_cases, case_count), + "source_backed_rate": ratio( + sum(item["source_backed"] for item in findings), actionable + ), + "line_anchored_rate": ratio( + sum(item["line_anchored"] for item in findings), actionable + ), + "fix_direction_rate": ratio( + sum(item["has_fix_direction"] for item in findings), actionable + ), + "regression_test_direction_rate": ratio( + sum(item["has_regression_test_direction"] for item in findings), actionable + ), + "defect_metrics": None, + } + if benchmark["evaluation_mode"] != "head_matched_gold": + return result + matched: set[tuple[str, str]] = set() + true_positive = 0 + false_positive = 0 + for finding in findings: + gold_id = finding["gold_finding_id"] + key = (finding["case_id"], gold_id) if gold_id is not None else None + if key is None or key in matched or not finding["source_backed"]: + false_positive += 1 + else: + matched.add(key) + true_positive += 1 + false_negative = len(gold) - true_positive + precision = ratio(true_positive, true_positive + false_positive) + recall = ratio(true_positive, true_positive + false_negative) + f1 = ( + None + if precision is None or recall is None or precision + recall == 0 + else round(2 * precision * recall / (precision + recall), 6) + ) + high_gold = {key for key, severity in gold.items() if severity in {"critical", "high"}} + result["defect_metrics"] = { + "gold_findings": len(gold), + "true_positives": true_positive, + "false_positives": false_positive, + "false_negatives": false_negative, + "precision": precision, + "precision_interval_95": wilson(true_positive, true_positive + false_positive), + "recall": recall, + "recall_interval_95": wilson(true_positive, true_positive + false_negative), + "f1_score": f1, + "critical_high_recall": ratio(len(high_gold & matched), len(high_gold)), + } + return result + + +def parity_gate( + benchmark: Mapping[str, Any], scores: Mapping[str, Mapping[str, Any]] +) -> dict[str, Any]: + """Evaluate reference-relative parity only with sufficient expert gold data.""" + policy = benchmark["parity_policy"] + candidate_name = policy["candidate_reviewer"] + reference_name = policy["reference_reviewer"] + case_count = sum(case["head_match"] for case in benchmark["cases"]) + gold_count = sum(len(case["gold_findings"]) for case in benchmark["cases"]) + reasons: list[str] = [] + if benchmark["evaluation_mode"] != "head_matched_gold": + reasons.append("benchmark is not head_matched_gold") + if case_count < policy["minimum_head_matched_cases"]: + reasons.append("head-matched case count is below minimum") + if gold_count < policy["minimum_gold_findings"]: + reasons.append("gold finding count is below minimum") + candidate = scores.get(candidate_name) + reference = scores.get(reference_name) + if candidate is None or reference is None: + reasons.append("candidate or reference reviewer is absent") + elif candidate["defect_metrics"] is None or reference["defect_metrics"] is None: + reasons.append("candidate or reference lacks defect metrics") + if reasons: + return {"status": "INSUFFICIENT_EVIDENCE", "reasons": reasons} + candidate_metrics = candidate["defect_metrics"] + reference_metrics = reference["defect_metrics"] + assert candidate_metrics is not None and reference_metrics is not None + candidate_precision_interval = candidate_metrics["precision_interval_95"] + candidate_recall_interval = candidate_metrics["recall_interval_95"] + reference_precision = reference_metrics["precision"] + reference_recall = reference_metrics["recall"] + if ( + candidate_precision_interval is None + or candidate_recall_interval is None + or reference_precision is None + or reference_recall is None + ): + return { + "status": "INSUFFICIENT_EVIDENCE", + "reasons": [ + "candidate or reference precision or recall denominator is zero" + ], + } + margin = policy["non_inferiority_margin"] + precision_pass = candidate_precision_interval[0] >= reference_precision - margin + recall_pass = candidate_recall_interval[0] >= reference_recall - margin + high_recall = candidate_metrics["critical_high_recall"] + high_pass = high_recall is not None and high_recall >= policy["required_critical_high_recall"] + return { + "status": "PASS" if precision_pass and recall_pass and high_pass else "FAIL", + "method": "Wilson-lower-bound non-inferiority", + "precision_pass": precision_pass, + "recall_pass": recall_pass, + "critical_high_pass": high_pass, + "reasons": [], + } + + +def score_benchmark(raw_value: Any) -> dict[str, Any]: + """Return deterministic operational, defect, and parity metrics.""" + benchmark = validate_benchmark(raw_value) + names = sorted( + {name for case in benchmark["cases"] for name in case["reviewers"]} + ) + scores = {name: score_reviewer(name, benchmark) for name in names} + return { + "schema_version": "1.0", + "benchmark_id": benchmark["benchmark_id"], + "evaluation_mode": benchmark["evaluation_mode"], + "case_count": len(benchmark["cases"]), + "head_matched_case_count": sum(case["head_match"] for case in benchmark["cases"]), + "gold_finding_count": sum( + len(case["gold_findings"]) for case in benchmark["cases"] + ), + "reviewers": scores, + "parity_gate": parity_gate(benchmark, scores), + "limitations": benchmark["limitations"], + } + + +def render_markdown(report: Mapping[str, Any]) -> str: + """Render a concise Markdown report for CI and doctoring.""" + lines = [ + f"# OpenCode review quality — `{report['benchmark_id']}`", + "", + f"- Mode: `{report['evaluation_mode']}`", + f"- Cases: {report['case_count']}", + f"- Parity gate: **{report['parity_gate']['status']}**", + "", + "| Reviewer | Completed / triggered | Availability | Actionable / completed | Infrastructure-only | Duplicate |", + "|---|---:|---:|---:|---:|---:|", + ] + for name, score in sorted(report["reviewers"].items()): + values = [ + score["availability_rate"], + score["actionable_findings_per_completed_review"], + score["infrastructure_only_review_rate"], + score["duplicate_review_rate"], + ] + formatted = ["n/a" if value is None else f"{value:.3f}" for value in values] + lines.append( + f"| {name} | {score['completed_attempts']} / {score['triggered_attempts']} | " + f"{formatted[0]} | {formatted[1]} | {formatted[2]} | {formatted[3]} |" + ) + if report["parity_gate"]["status"] == "INSUFFICIENT_EVIDENCE": + lines.extend( + [ + "", + "Precision, recall, and CodeRabbit parity are **not inferable** from this evidence.", + ] + ) + lines.extend(["", "## Limitations", ""]) + lines.extend(f"- {item}" for item in report["limitations"]) + return "\n".join(lines) + "\n" + + +def confined_path( + path: Path, + root: Path, + label: str, + *, + must_exist: bool, +) -> Path: + """Resolve one path and reject escapes from an explicit trusted root.""" + try: + resolved_root = root.resolve(strict=True) + if not resolved_root.is_dir(): + reject(f"{label} root must be a directory") + resolved_path = path.resolve(strict=must_exist) + except OSError as error: + reject(f"cannot resolve {label} path: {error}") + try: + resolved_path.relative_to(resolved_root) + except ValueError: + reject(f"{label} path must remain within its trusted root") + if must_exist and not resolved_path.is_file(): + reject(f"{label} path must be a regular file") + return resolved_path + + +def load_json(path: Path) -> Any: + """Load one UTF-8 JSON document with stable errors.""" + try: + return json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as error: + reject(f"cannot load benchmark: {error}") + + +def write_text(path: Path, content: str) -> None: + """Atomically replace an output file after creating its parent.""" + path.parent.mkdir(parents=True, exist_ok=True) + temporary = path.with_name(f".{path.name}.tmp") + temporary.write_text(content, encoding="utf-8") + temporary.replace(path) + + +def main(argv: Sequence[str] | None = None) -> int: + """Run the scorer CLI and return stable gate-oriented statuses.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--input", type=Path, required=True) + parser.add_argument("--input-root", type=Path, default=Path.cwd()) + parser.add_argument("--json-output", type=Path) + parser.add_argument("--markdown-output", type=Path) + parser.add_argument("--output-root", type=Path, default=Path.cwd()) + parser.add_argument("--fail-on-parity-regression", action="store_true") + parser.add_argument("--require-parity-evidence", action="store_true") + arguments = parser.parse_args(argv) + try: + input_path = confined_path( + arguments.input, + arguments.input_root, + "input", + must_exist=True, + ) + json_output = ( + confined_path( + arguments.json_output, + arguments.output_root, + "JSON output", + must_exist=False, + ) + if arguments.json_output + else None + ) + markdown_output = ( + confined_path( + arguments.markdown_output, + arguments.output_root, + "Markdown output", + must_exist=False, + ) + if arguments.markdown_output + else None + ) + report = score_benchmark(load_json(input_path)) + except BenchmarkValidationError as error: + print(f"review-quality benchmark rejected: {error}", file=sys.stderr) + return 2 + json_text = json.dumps(report, ensure_ascii=False, indent=2, sort_keys=True) + "\n" + if json_output: + write_text(json_output, json_text) + else: + sys.stdout.write(json_text) + if markdown_output: + write_text(markdown_output, render_markdown(report)) + status = report["parity_gate"]["status"] + if arguments.require_parity_evidence and status == "INSUFFICIENT_EVIDENCE": + return 3 + return int(arguments.fail_on_parity_regression and status == "FAIL") + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_opencode_review_quality_evidence_contract.py b/tests/test_opencode_review_quality_evidence_contract.py new file mode 100644 index 000000000..e17743f2f --- /dev/null +++ b/tests/test_opencode_review_quality_evidence_contract.py @@ -0,0 +1,193 @@ +"""Regression tests for empirical review-evidence integrity.""" + +from __future__ import annotations + +import importlib.util +from pathlib import Path +from types import ModuleType +from typing import Any + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +MODULE_PATH = ROOT / "scripts/ci/opencode_review_quality_score.py" + + +def load_module() -> ModuleType: + """Load the exact scorer module without package side effects.""" + spec = importlib.util.spec_from_file_location( + "opencode_review_quality_evidence_contract", MODULE_PATH + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +quality = load_module() + + +def finding(identifier: str, gold_identifier: str) -> dict[str, Any]: + """Build one source-contract-complete reviewer finding.""" + return { + "finding_id": identifier, + "gold_finding_id": gold_identifier, + "severity": "high", + "actionable": True, + "source_backed": True, + "line_anchored": True, + "has_fix_direction": True, + "has_regression_test_direction": True, + } + + +def reviewer(identifier: str, gold_identifier: str, head_sha: str) -> dict[str, Any]: + """Build one completed semantic review fixture bound to the exact case head.""" + return { + "triggered_attempts": 1, + "completed_attempts": 1, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 0, + "duplicate_reviews": 0, + "reviewed_head_sha": head_sha, + "findings": [finding(identifier, gold_identifier)], + } + + +def benchmark(count: int = 50) -> dict[str, Any]: + """Build a minimally eligible exact-head expert-gold benchmark.""" + cases: list[dict[str, Any]] = [] + for index in range(count): + gold_identifier = f"gold-{index}" + head_sha = f"{index:040x}" + cases.append( + { + "case_id": f"case-{index}", + "repository": "ContextualWisdomLab/example", + "pull_request_number": index + 1, + "head_match": True, + "base_sha": "a" * 40, + "head_sha": head_sha, + "diff_size_bucket": ("small", "medium", "large")[index % 3], + "primary_language": ("python", "rust", "typescript")[index % 3], + "gold_findings": [ + {"finding_id": gold_identifier, "severity": "high"} + ], + "reviewers": { + "opencode": reviewer( + f"opencode-{index}", gold_identifier, head_sha + ), + "coderabbit": reviewer( + f"coderabbit-{index}", gold_identifier, head_sha + ), + }, + } + ) + return { + "schema_version": "1.0", + "benchmark_id": "exact-head-fixture", + "evaluation_mode": "head_matched_gold", + "limitations": ["Synthetic evidence-integrity fixture."], + "parity_policy": { + "candidate_reviewer": "opencode", + "reference_reviewer": "coderabbit", + "minimum_head_matched_cases": 50, + "minimum_gold_findings": 50, + "non_inferiority_margin": 0.05, + "required_critical_high_recall": 1.0, + }, + "cases": cases, + } + + +@pytest.mark.parametrize( + "mutate", + [ + lambda value: value.update({"unexpected": True}), + lambda value: value["parity_policy"].update({"unexpected": True}), + lambda value: value["cases"][0].update({"unexpected": True}), + lambda value: value["cases"][0]["gold_findings"][0].update( + {"unexpected": True} + ), + lambda value: value["cases"][0]["reviewers"]["opencode"].update( + {"unexpected": True} + ), + lambda value: value["cases"][0]["reviewers"]["opencode"][ + "findings" + ][0].update({"unexpected": True}), + ], +) +def test_strict_schema_rejects_unknown_fields(mutate: Any) -> None: + """Every schema layer must fail closed on unreviewed input fields.""" + value = benchmark() + mutate(value) + with pytest.raises(quality.BenchmarkValidationError, match="unknown fields"): + quality.validate_benchmark(value) + + +def test_head_matched_cases_require_immutable_base_and_head_shas() -> None: + """A Boolean assertion alone must not establish exact-head comparability.""" + missing = benchmark() + del missing["cases"][0]["base_sha"] + with pytest.raises(quality.BenchmarkValidationError, match="base_sha"): + quality.validate_benchmark(missing) + + malformed = benchmark() + malformed["cases"][0]["head_sha"] = "main" + with pytest.raises(quality.BenchmarkValidationError, match="head_sha"): + quality.validate_benchmark(malformed) + + +def test_reviewer_names_must_be_unique_after_normalization() -> None: + """Case-fold-equivalent reviewer names must not silently overwrite evidence.""" + value = benchmark() + value["cases"][0]["reviewers"]["OpenCode"] = value["cases"][0][ + "reviewers" + ]["opencode"] + with pytest.raises(quality.BenchmarkValidationError, match="reviewer.*duplicates"): + quality.validate_benchmark(value) + + +def test_every_exact_head_case_requires_both_compared_reviewers() -> None: + """Parity must not compare reviewers over different case subsets.""" + value = benchmark() + del value["cases"][0]["reviewers"]["opencode"] + with pytest.raises( + quality.BenchmarkValidationError, match="candidate or reference reviewer" + ): + quality.validate_benchmark(value) + + +def test_lifecycle_missing_candidate_remains_insufficient_evidence() -> None: + """Lifecycle evidence may be sparse but must never imply reviewer parity.""" + value = benchmark(count=1) + case = value["cases"][0] + value["evaluation_mode"] = "historical_lifecycle" + case["head_match"] = False + del case["base_sha"] + del case["head_sha"] + case["gold_findings"] = [] + del case["reviewers"]["opencode"] + case["reviewers"]["coderabbit"]["findings"] = [] + report = quality.score_benchmark(value) + assert report["parity_gate"]["status"] == "INSUFFICIENT_EVIDENCE" + assert "candidate or reference reviewer is absent" in report["parity_gate"][ + "reasons" + ] + + +def test_gold_match_without_source_contract_is_not_a_true_positive() -> None: + """Mapped but unsupported comments must count as noise and leave a miss.""" + value = benchmark() + value["cases"][0]["reviewers"]["opencode"]["findings"][0][ + "source_backed" + ] = False + metrics = quality.score_benchmark(value)["reviewers"]["opencode"][ + "defect_metrics" + ] + assert metrics is not None + assert ( + metrics["true_positives"], + metrics["false_positives"], + metrics["false_negatives"], + ) == (49, 1, 1) diff --git a/tests/test_opencode_review_quality_score.py b/tests/test_opencode_review_quality_score.py new file mode 100644 index 000000000..de88a3b59 --- /dev/null +++ b/tests/test_opencode_review_quality_score.py @@ -0,0 +1,462 @@ +"""Tests for the empirical automated-review quality scorer.""" + +from __future__ import annotations + +import copy +import importlib.util +import json +import runpy +import sys +from pathlib import Path +from types import ModuleType +from typing import Any + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +MODULE_PATH = ROOT / "scripts/ci/opencode_review_quality_score.py" +PILOT_PATH = ROOT / "benchmarks/opencode_review/pilot_baseline_v1.json" +BASE_SHA = "1" * 40 +HEAD_SHA = "2" * 40 + + +def load_module() -> ModuleType: + """Load the exact repository module without package side effects.""" + spec = importlib.util.spec_from_file_location("opencode_review_quality_score", MODULE_PATH) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +quality = load_module() + + +def finding(identifier: str, gold_id: str | None = None, actionable: bool = True) -> dict[str, Any]: + """Build one fully evidenced finding fixture.""" + return { + "finding_id": identifier, + "gold_finding_id": gold_id, + "severity": "high", + "actionable": actionable, + "source_backed": True, + "line_anchored": True, + "has_fix_direction": True, + "has_regression_test_direction": True, + } + + +def reviewer( + findings: list[dict[str, Any]] | None = None, + reviewed_head_sha: str | None = None, +) -> dict[str, Any]: + """Build one successful reviewer-run fixture.""" + value = { + "triggered_attempts": 1, + "completed_attempts": 1, + "rate_limited_attempts": 0, + "infrastructure_only_reviews": 0, + "duplicate_reviews": 0, + "findings": findings or [], + } + if reviewed_head_sha is not None: + value["reviewed_head_sha"] = reviewed_head_sha + return value + + +def benchmark(mode: str = "historical_lifecycle", count: int = 1) -> dict[str, Any]: + """Build a valid benchmark in lifecycle or head-matched mode.""" + value: dict[str, Any] = { + "schema_version": "1.0", + "benchmark_id": "fixture", + "evaluation_mode": mode, + "limitations": ["Synthetic scorer contract fixture."], + "parity_policy": { + "candidate_reviewer": "opencode", + "reference_reviewer": "coderabbit", + "minimum_head_matched_cases": 50, + "minimum_gold_findings": 50, + "non_inferiority_margin": 0.05, + "required_critical_high_recall": 1.0, + }, + "cases": [], + } + for index in range(count): + gold_id = f"gold-{index}" + head_match = mode == "head_matched_gold" + value["cases"].append( + { + "case_id": f"case-{index}", + "repository": "ContextualWisdomLab/example", + "pull_request_number": index + 1, + "head_match": head_match, + **({"base_sha": BASE_SHA, "head_sha": HEAD_SHA} if head_match else {}), + "diff_size_bucket": ("small", "medium", "large")[index % 3], + "primary_language": ("python", "rust", "typescript", "go")[index % 4], + "gold_findings": ([{"finding_id": gold_id, "severity": "high"}] if head_match else []), + "reviewers": { + "opencode": reviewer( + [finding(f"opencode-{index}", gold_id)] + if head_match + else [finding(f"opencode-{index}")], + HEAD_SHA if head_match else None, + ), + "coderabbit": reviewer( + [finding(f"coderabbit-{index}", gold_id)] if head_match else [], + HEAD_SHA if head_match else None, + ), + }, + } + ) + return value + + +def test_empirical_pilot_exposes_yield_gap_without_claiming_parity() -> None: + """The purposive pilot must report operational evidence only.""" + report = quality.score_benchmark(json.loads(PILOT_PATH.read_text(encoding="utf-8"))) + assert report["parity_gate"]["status"] == "INSUFFICIENT_EVIDENCE" + assert report["head_matched_case_count"] == report["gold_finding_count"] == 0 + opencode = report["reviewers"]["opencode"] + coderabbit = report["reviewers"]["coderabbit"] + assert (opencode["completed_attempts"], opencode["actionable_findings"]) == (8, 0) + assert opencode["infrastructure_only_review_rate"] == 1.0 + assert opencode["duplicate_review_rate"] == 0.625 + assert opencode["source_backed_rate"] is None + assert (coderabbit["triggered_attempts"], coderabbit["completed_attempts"]) == (4, 3) + assert coderabbit["availability_rate"] == 0.75 + assert coderabbit["actionable_findings"] == 8 + assert coderabbit["actionable_findings_per_completed_review"] == 2.666667 + assert coderabbit["source_backed_rate"] == coderabbit["line_anchored_rate"] == 1.0 + + +def test_large_perfect_gold_sample_passes_reference_relative_gate() -> None: + """A 100-case perfect candidate must pass the Wilson lower-bound gate.""" + report = quality.score_benchmark(benchmark("head_matched_gold", 100)) + metrics = report["reviewers"]["opencode"]["defect_metrics"] + assert metrics["precision"] == metrics["recall"] == metrics["f1_score"] == 1.0 + assert metrics["precision_interval_95"] == [0.963007, 1.0] + assert report["parity_gate"] == { + "status": "PASS", + "method": "Wilson-lower-bound non-inferiority", + "precision_pass": True, + "recall_pass": True, + "critical_high_pass": True, + "reasons": [], + } + + +def test_false_positive_duplicate_and_missed_high_findings_fail_gate() -> None: + """Duplicate mappings, unmatched comments, and missed highs must be penalized.""" + value = benchmark("head_matched_gold", 100) + value["cases"][0]["reviewers"]["opencode"]["findings"] += [ + finding("duplicate", "gold-0"), + finding("unmatched"), + finding("nit", actionable=False), + ] + for case in value["cases"][:10]: + case["reviewers"]["opencode"]["findings"] = [] + report = quality.score_benchmark(value) + metrics = report["reviewers"]["opencode"]["defect_metrics"] + assert metrics["true_positives"] == 90 + assert metrics["false_positives"] == 0 + assert metrics["false_negatives"] == 10 + assert report["parity_gate"]["status"] == "FAIL" + assert report["parity_gate"]["recall_pass"] is False + assert report["parity_gate"]["critical_high_pass"] is False + + +def test_duplicate_and_unmatched_findings_count_as_false_positives() -> None: + """Only the first actionable match to a gold identifier is a true positive.""" + value = benchmark("head_matched_gold", 100) + value["cases"][0]["reviewers"]["opencode"]["findings"] += [ + finding("duplicate", "gold-0"), + finding("unmatched"), + ] + metrics = quality.score_benchmark(value)["reviewers"]["opencode"]["defect_metrics"] + assert (metrics["true_positives"], metrics["false_positives"]) == (100, 2) + assert metrics["precision"] == 0.980392 + + +def test_head_matched_evidence_requires_exact_matching_shas() -> None: + """Gold metrics must bind every reviewer to one immutable PR head.""" + value = benchmark("head_matched_gold", 100) + del value["cases"][0]["base_sha"] + with pytest.raises(quality.BenchmarkValidationError, match="base_sha"): + quality.validate_benchmark(value) + + value = benchmark("head_matched_gold", 100) + value["cases"][0]["head_sha"] = "not-a-sha" + with pytest.raises(quality.BenchmarkValidationError, match="head_sha"): + quality.validate_benchmark(value) + + value = benchmark("head_matched_gold", 100) + value["cases"][0]["reviewers"]["opencode"]["reviewed_head_sha"] = "3" * 40 + with pytest.raises(quality.BenchmarkValidationError, match="reviewed_head_sha"): + quality.validate_benchmark(value) + + +def test_reference_zero_denominator_returns_insufficient_evidence() -> None: + """A reference with no actionable findings must not cause arithmetic failure.""" + value = benchmark("head_matched_gold", 100) + for case in value["cases"]: + case["reviewers"]["coderabbit"]["findings"] = [] + assert quality.score_benchmark(value)["parity_gate"] == { + "status": "INSUFFICIENT_EVIDENCE", + "reasons": ["candidate or reference precision or recall denominator is zero"], + } + + +def test_reviewer_names_and_completed_review_evidence_fail_closed() -> None: + """Case-fold collisions and findings without a completed review are invalid.""" + value = benchmark() + value["cases"][0]["reviewers"]["OpenCode"] = copy.deepcopy( + value["cases"][0]["reviewers"]["opencode"] + ) + with pytest.raises(quality.BenchmarkValidationError, match="reviewer name duplicates"): + quality.validate_benchmark(value) + + value = benchmark() + value["cases"][0]["reviewers"]["opencode"]["completed_attempts"] = 0 + with pytest.raises(quality.BenchmarkValidationError, match="findings require"): + quality.validate_benchmark(value) + + +def test_parity_refuses_small_missing_or_zero_denominator_evidence() -> None: + """Parity must remain unavailable for underpowered or invalid reviewer evidence.""" + report = quality.score_benchmark(benchmark("head_matched_gold", 2)) + assert report["parity_gate"]["status"] == "INSUFFICIENT_EVIDENCE" + assert len(report["parity_gate"]["reasons"]) == 2 + + value = benchmark("head_matched_gold", 100) + for case in value["cases"]: + del case["reviewers"]["coderabbit"] + with pytest.raises( + quality.BenchmarkValidationError, match="candidate or reference reviewer" + ): + quality.validate_benchmark(value) + + value = benchmark("head_matched_gold", 100) + for case in value["cases"]: + case["reviewers"]["opencode"]["findings"] = [] + case["reviewers"]["coderabbit"]["findings"] = [] + assert quality.score_benchmark(value)["parity_gate"]["reasons"] == [ + "candidate or reference precision or recall denominator is zero" + ] + + +def test_sparse_reviewer_cases_and_empty_ratios_are_supported() -> None: + """A reviewer may be absent from one lifecycle case without corrupting totals.""" + value = benchmark(count=2) + del value["cases"][1]["reviewers"]["opencode"] + value["cases"][0]["reviewers"]["coderabbit"] = reviewer() + report = quality.score_benchmark(value) + assert report["reviewers"]["opencode"]["case_count"] == 1 + assert report["reviewers"]["coderabbit"]["actionable_findings_per_completed_review"] == 0.0 + assert quality.ratio(1, 0) is None + assert quality.wilson(0, 0) is None + with pytest.raises(ValueError, match="between zero and trials"): + quality.wilson(2, 1) + + +@pytest.mark.parametrize( + ("mutate", "message"), + [ + (lambda value: [], "benchmark must be an object"), + (lambda value: {**value, "schema_version": "2"}, "schema_version"), + (lambda value: {**value, "benchmark_id": ""}, "benchmark_id"), + (lambda value: {**value, "evaluation_mode": "other"}, "evaluation_mode"), + (lambda value: {**value, "limitations": {}}, "limitations must be an array"), + (lambda value: {**value, "limitations": []}, "limitations must not be empty"), + (lambda value: {**value, "parity_policy": []}, "parity_policy must be an object"), + (lambda value: _set(value, ["parity_policy", "reference_reviewer"], "opencode"), "must differ"), + (lambda value: _set(value, ["parity_policy", "minimum_head_matched_cases"], 0), "must be positive"), + (lambda value: _set(value, ["parity_policy", "minimum_gold_findings"], True), "non-negative integer"), + (lambda value: _set(value, ["parity_policy", "non_inferiority_margin"], "x"), "must be a number"), + (lambda value: _set(value, ["parity_policy", "required_critical_high_recall"], 2), "finite number"), + (lambda value: {**value, "cases": []}, "cases must not be empty"), + (lambda value: _append_duplicate_case(value), "duplicates"), + (lambda value: _set(value, ["cases", 0, "repository"], "invalid"), "owner/name"), + (lambda value: _set(value, ["cases", 0, "pull_request_number"], 0), "must be positive"), + (lambda value: _set(value, ["cases", 0, "head_match"], "false"), "must be boolean"), + (lambda value: _set(value, ["cases", 0, "diff_size_bucket"], "huge"), "diff_size_bucket"), + (lambda value: _set(value, ["cases", 0, "primary_language"], ""), "primary_language"), + (lambda value: _set(value, ["cases", 0, "gold_findings"], {}), "gold_findings must be an array"), + (lambda value: _set(value, ["cases", 0, "reviewers"], {}), "reviewers must not be empty"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode"], []), "must be an object"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "triggered_attempts"], -1), "non-negative integer"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "rate_limited_attempts"], 1), "exceed triggered"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "infrastructure_only_reviews"], 2), "exceeds completed"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "duplicate_reviews"], 2), "exceeds completed"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "findings"], {}), "findings must be an array"), + (lambda value: _duplicate_finding(value), "duplicates"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "findings", 0, "severity"], "urgent"), "severity"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "findings", 0, "actionable"], 1), "must be boolean"), + (lambda value: _set(value, ["cases", 0, "reviewers", "opencode", "findings", 0, "gold_finding_id"], "unknown"), "current-head gold"), + ], +) +def test_validation_fails_closed(mutate: Any, message: str) -> None: + """Malformed counts, shapes, identifiers, and evidence links must be rejected.""" + value = benchmark() + with pytest.raises(quality.BenchmarkValidationError, match=message): + quality.validate_benchmark(mutate(value)) + + +def _set(value: dict[str, Any], path: list[Any], replacement: Any) -> dict[str, Any]: + """Mutate and return a nested fixture value.""" + cursor: Any = value + for key in path[:-1]: + cursor = cursor[key] + cursor[path[-1]] = replacement + return value + + +def _append_duplicate_case(value: dict[str, Any]) -> dict[str, Any]: + """Append a duplicate case identifier for validation coverage.""" + value["cases"].append(copy.deepcopy(value["cases"][0])) + return value + + +def _duplicate_finding(value: dict[str, Any]) -> dict[str, Any]: + """Append a duplicate finding identifier for validation coverage.""" + items = value["cases"][0]["reviewers"]["opencode"]["findings"] + items.append(copy.deepcopy(items[0])) + return value + + +def test_gold_validation_and_head_match_contracts() -> None: + """Gold mode must reject stale heads, invalid severities, and duplicate gold IDs.""" + value = benchmark() + value["cases"][0]["gold_findings"] = [ + {"finding_id": "unmatched-gold", "severity": "high"} + ] + with pytest.raises(quality.BenchmarkValidationError, match="head_match=true"): + quality.validate_benchmark(value) + value = benchmark("head_matched_gold") + value["cases"][0]["head_match"] = False + with pytest.raises(quality.BenchmarkValidationError, match="head_match"): + quality.validate_benchmark(value) + value = benchmark("head_matched_gold") + value["cases"][0]["gold_findings"][0]["severity"] = "urgent" + with pytest.raises(quality.BenchmarkValidationError, match="severity"): + quality.validate_benchmark(value) + value = benchmark("head_matched_gold") + value["cases"][0]["gold_findings"].append(copy.deepcopy(value["cases"][0]["gold_findings"][0])) + with pytest.raises(quality.BenchmarkValidationError, match="duplicates"): + quality.validate_benchmark(value) + + +def test_markdown_and_cli_outputs_are_deterministic(tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch) -> None: + """Reports, atomic writes, statuses, input errors, and module entrypoint are stable.""" + lifecycle = benchmark() + report = quality.score_benchmark(lifecycle) + markdown = quality.render_markdown(report) + assert "Precision, recall, and CodeRabbit parity are **not inferable**" in markdown + assert "| opencode | 1 / 1 | 1.000 | 1.000 | 0.000 | 0.000 |" in markdown + + assert "not inferable" not in quality.render_markdown(quality.score_benchmark(benchmark("head_matched_gold", 100))) + + input_path = tmp_path / "input.json" + input_path.write_text(json.dumps(lifecycle), encoding="utf-8") + json_path = tmp_path / "out/report.json" + md_path = tmp_path / "out/report.md" + root_args = ["--input-root", str(tmp_path), "--output-root", str(tmp_path)] + assert quality.main([*root_args, "--input", str(input_path), "--json-output", str(json_path), "--markdown-output", str(md_path)]) == 0 + assert json.loads(json_path.read_text(encoding="utf-8"))["parity_gate"]["status"] == "INSUFFICIENT_EVIDENCE" + assert not (json_path.parent / ".report.json.tmp").exists() + assert quality.main([*root_args, "--input", str(input_path), "--require-parity-evidence"]) == 3 + assert '"INSUFFICIENT_EVIDENCE"' in capsys.readouterr().out + + passing = tmp_path / "pass.json" + passing.write_text(json.dumps(benchmark("head_matched_gold", 100)), encoding="utf-8") + assert quality.main([*root_args, "--input", str(passing), "--fail-on-parity-regression"]) == 0 + failing_value = benchmark("head_matched_gold", 100) + for case in failing_value["cases"][:10]: + case["reviewers"]["opencode"]["findings"] = [] + failing = tmp_path / "fail.json" + failing.write_text(json.dumps(failing_value), encoding="utf-8") + assert quality.main([*root_args, "--input", str(failing), "--fail-on-parity-regression"]) == 1 + + invalid = tmp_path / "invalid.json" + invalid.write_text("{", encoding="utf-8") + assert quality.main([*root_args, "--input", str(invalid)]) == 2 + assert "benchmark rejected" in capsys.readouterr().err + with pytest.raises(quality.BenchmarkValidationError, match="cannot load"): + quality.load_json(tmp_path / "missing.json") + + monkeypatch.setattr( + sys, + "argv", + [str(MODULE_PATH), *root_args, "--input", str(input_path)], + ) + with pytest.raises(SystemExit, match="0"): + runpy.run_path(str(MODULE_PATH), run_name="__main__") + + +def test_cli_confines_paths_to_explicit_roots(tmp_path: Path) -> None: + """CLI input and output paths must not escape their trusted roots.""" + trusted = tmp_path / "trusted" + trusted.mkdir() + inside_input = trusted / "input.json" + inside_input.write_text(json.dumps(benchmark()), encoding="utf-8") + outside_input = tmp_path / "outside.json" + outside_input.write_text(json.dumps(benchmark()), encoding="utf-8") + outside_output = tmp_path / "outside-report.json" + root_args = [ + "--input-root", + str(trusted), + "--output-root", + str(trusted), + ] + + assert quality.main([*root_args, "--input", str(outside_input)]) == 2 + assert ( + quality.main( + [ + *root_args, + "--input", + str(inside_input), + "--json-output", + str(outside_output), + ] + ) + == 2 + ) + assert not outside_output.exists() + + escaped_input = trusted / "escaped.json" + escaped_input.symlink_to(outside_input) + assert quality.main([*root_args, "--input", str(escaped_input)]) == 2 + + with pytest.raises(quality.BenchmarkValidationError, match="root must be a directory"): + quality.confined_path( + inside_input, + inside_input, + "input", + must_exist=True, + ) + with pytest.raises(quality.BenchmarkValidationError, match="cannot resolve"): + quality.confined_path( + inside_input, + trusted / "missing-root", + "input", + must_exist=True, + ) + with pytest.raises(quality.BenchmarkValidationError, match="regular file"): + quality.confined_path( + trusted, + trusted, + "input", + must_exist=True, + ) + + +def test_all_production_callables_are_documented() -> None: + """Every locally defined production callable must retain a docstring.""" + missing = [ + name + for name, value in vars(quality).items() + if getattr(value, "__module__", None) == quality.__name__ + and callable(value) + and not getattr(value, "__doc__", None) + ] + assert missing == []