From ad84ba86321a2ac282652817c44b2d82e12e8afd Mon Sep 17 00:00:00 2001 From: OpenCode Date: Thu, 9 Jul 2026 18:05:07 +0700 Subject: [PATCH] feat: Phase J J9 external advisory review process -- credibility (#809) --- Makefile | 7 +- docs/evidence/METRICS_CURRENT.md | 6 +- .../EXTERNAL_ADVISORY_REVIEW_PROCESS.md | 96 +++++++++ docs/research/ROADMAP.md | 29 +++ src/openamp_foundry/cli/commands/reports.py | 34 +++ src/openamp_foundry/cli/main.py | 21 ++ .../governance/advisory_review.py | 175 +++++++++++++++ tests/governance/test_advisory_review.py | 201 ++++++++++++++++++ tests/test_test_count_regression.py | 4 +- 9 files changed, 568 insertions(+), 5 deletions(-) create mode 100644 docs/governance/EXTERNAL_ADVISORY_REVIEW_PROCESS.md create mode 100644 src/openamp_foundry/governance/advisory_review.py create mode 100644 tests/governance/test_advisory_review.py diff --git a/Makefile b/Makefile index 05e3ad04..2493ce2b 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help demo test lint ci clean bench-leakage bench-multi-negatives bench-baseline bench-hidden-active bench-cluster-split bench-expert-ablation bench-expert-ablation-500 bench-selectivity bench-feature-decomp bench-gate bench-easy-baseline bench-charge-matched bench-order-dependent bench-precision-at-k bench-per-family bench-simulation-ablation bench-simulation-ablation-within-amp simulation-gate bench-strategy-compare regenerate-all generate phase3 pilot validate-scoring validate-scoring-phase3 validate-scoring-strict external-pilot pilot-confident presynth-qc gold-standard diversity synthesis-order novelty-broad external-consensus questionnaire gate-check ip-report benchmark-card wave0-5-gate-check wave0-5-novelty-audit wave0-5-novelty-audit-v2 wave0-5-panel wave0-5-evidence wave0-5-fill-external wave0-5b-generate wave0-5b-filter recalibration-engine recalibration-engine-dry-run validate-policy-version generate-synthetic-lab-results bump-policy-version generate-review-packet failed-candidate-report safe-publication-filter classify-negative-informativeness validate-rejection-events negative-result-dashboard calibration-audit calibration-audit-example calibration-overfit-check result-quality-filter synthetic-result-policy-check calibration-decision-checklist calibration-rollback-plan simulation-registry validate-simulation-result-schema simulation-baseline-check adapter-gate-check simulation-provenance simulation-ensemble-check simulation-ci-report simulation-deprecation-check simulation-scope-check simulation-evidence-packet artifact-version candidate-manifest artifact-changelog integration-check adapter-author-check license-check artifact-compat-check contribution-check adoption-scorecard release-gate-check decision-log release-request-check coi-check rotation-plan-check security-report-check citation-check roadmap-sync-check +.PHONY: help demo test lint ci clean bench-leakage bench-multi-negatives bench-baseline bench-hidden-active bench-cluster-split bench-expert-ablation bench-expert-ablation-500 bench-selectivity bench-feature-decomp bench-gate bench-easy-baseline bench-charge-matched bench-order-dependent bench-precision-at-k bench-per-family bench-simulation-ablation bench-simulation-ablation-within-amp simulation-gate bench-strategy-compare regenerate-all generate phase3 pilot validate-scoring validate-scoring-phase3 validate-scoring-strict external-pilot pilot-confident presynth-qc gold-standard diversity synthesis-order novelty-broad external-consensus questionnaire gate-check ip-report benchmark-card wave0-5-gate-check wave0-5-novelty-audit wave0-5-novelty-audit-v2 wave0-5-panel wave0-5-evidence wave0-5-fill-external wave0-5b-generate wave0-5b-filter recalibration-engine recalibration-engine-dry-run validate-policy-version generate-synthetic-lab-results bump-policy-version generate-review-packet failed-candidate-report safe-publication-filter classify-negative-informativeness validate-rejection-events negative-result-dashboard calibration-audit calibration-audit-example calibration-overfit-check result-quality-filter synthetic-result-policy-check calibration-decision-checklist calibration-rollback-plan simulation-registry validate-simulation-result-schema simulation-baseline-check adapter-gate-check simulation-provenance simulation-ensemble-check simulation-ci-report simulation-deprecation-check simulation-scope-check simulation-evidence-packet artifact-version candidate-manifest artifact-changelog integration-check adapter-author-check license-check artifact-compat-check contribution-check adoption-scorecard release-gate-check decision-log release-request-check coi-check rotation-plan-check security-report-check citation-check roadmap-sync-check advisory-review-check PYTHON := $(shell [ -f .venv/bin/python ] && echo .venv/bin/python || echo python3) PYTEST := $(shell [ -f .venv/bin/pytest ] && echo .venv/bin/pytest || echo pytest) @@ -875,3 +875,8 @@ roadmap-sync-check: .venv/bin/openamp-foundry roadmap-sync-check \ --entry-json '{"item_id":"J8","phase":"J","description":"Add roadmap-to-issue sync checklist","priority":"B","sync_status":"synced","issue_number":807}' @echo "Roadmap sync check complete." + +advisory-review-check: + .venv/bin/openamp-foundry advisory-review-check \ + --review-json '{"review_id":"ADV-2026-001","review_type":"candidate_review","artifact_id":"OAMP-v0.7.6-candidates","reviewer_handle":"external-advisor-1","assigned_date":"2026-07-09","deadline_date":"2026-07-23","status":"pending"}' + @echo "Advisory review check complete." diff --git a/docs/evidence/METRICS_CURRENT.md b/docs/evidence/METRICS_CURRENT.md index e5fe6f06..39415c45 100644 --- a/docs/evidence/METRICS_CURRENT.md +++ b/docs/evidence/METRICS_CURRENT.md @@ -5,7 +5,9 @@ Machine-readable snapshot: `outputs/metrics_snapshot.json` regenerated with `mak > **Purpose:** One authoritative table of current pipeline metrics. If any doc disagrees > with this file, this file wins. Updated whenever benchmark/benchmark config changes. > -> **Last updated:** 2026-07-09 (Phase J J8 — roadmap-to-issue sync checklist — v0.7.6) +> **Last updated:** 2026-07-09 (Phase J J9 — external advisory review process — v0.7.7) +> **New in v0.7.7:** External advisory review process (J9 — Loop 117) — docs/governance/EXTERNAL_ADVISORY_REVIEW_PROCESS.md with reviewer eligibility criteria, review scope table (5 review types with minimum reviewer counts), 5-step review process (prepare packet, assign+disclose, receive+log, respond to findings, close), finding severity classification (critical/major/minor/informational), limitations. src/openamp_foundry/governance/advisory_review.py with AdvisoryReview dataclass (11 fields: review_id, review_type, artifact_id, reviewer_handle, assigned_date, deadline_date, status, finding_severity, finding_summary, resolved, dry_lab_only), AdvisoryReviewResult dataclass (5 fields: review_id, review_type, passed, errors, warnings, dry_lab_only=True), VALID_REVIEW_TYPES (5: benchmark_audit, candidate_review, evidence_review, governance_review, safety_policy_review), VALID_REVIEW_STATUSES (5: assigned, completed, deferred, in_progress, pending), VALID_FINDING_SEVERITIES (4: critical, informational, major, minor), MINIMUM_REVIEWER_COUNTS (5 entries: candidate_review and safety_policy_review require 2, others 1), validate_advisory_review() (9 checks: ADV- prefix, valid review_type, non-empty artifact_id, non-empty reviewer_handle, YYYY-MM-DD dates, valid status, valid finding_severity, dry_lab_only must be True; critical+unresolved warns, completed+no-summary warns, deferred warns), validate_advisory_review_dict() (7 required fields guard). CLI (openamp-foundry advisory-review-check) with --review-json, --format text|json. make advisory-review-check target. 29 tests. Credibility: external advisory reviews now have a validated structure and documented process. +> > **New in v0.7.6:** Roadmap-to-issue sync checklist (J8 — Loop 116) — docs/governance/ROADMAP_ISSUE_SYNC_CHECKLIST.md with 5-section sync checklist (roadmap items → issues, issues → roadmap, completed items, priority alignment, version consistency). src/openamp_foundry/governance/roadmap_sync.py with RoadmapSyncEntry dataclass (10 fields: item_id, phase, description, priority, sync_status, issue_number, pr_number, completed, completion_date, dry_lab_only), RoadmapSyncResult dataclass (5 fields: item_id, phase, passed, errors, warnings, dry_lab_only=True), VALID_SYNC_STATUSES (5: synced, missing_issue, orphaned_issue, stale, completed), VALID_PRIORITY_LEVELS (4: A, B, C, D), VALID_PHASES (7: E, F, G, H, I, J, K), validate_roadmap_sync_entry() (8 checks: non-empty item_id, valid phase, non-empty description, valid priority, valid sync_status, dry_lab_only must be True, completed must have completion_date, completion_date must be YYYY-MM-DD; priority A + missing_issue warns, stale warns, orphaned_issue warns, no issue_number warns), validate_roadmap_sync_dict() (5 required fields guard). CLI (openamp-foundry roadmap-sync-check) with --entry-json, --format text|json. make roadmap-sync-check target. 24 tests. Keeps strategy actionable: roadmap sync entries are now machine-validated. > > **New in v0.7.5:** Citation and reuse guide (J7 — Loop 115) — docs/governance/CITATION_AND_REUSE_GUIDE.md with citation formats (inline, BibTeX), reuse table (4 artifact types, open/attribution_required/contact_required/restricted classes), attribution requirements, honest-use boundary. src/openamp_foundry/governance/citation_policy.py with CitationEntry (11 fields: artifact_id, citation_type, title, version, authors, year, license_identifier, reuse_class, url, bibtex_key, dry_lab_only), CitationValidationResult (6 fields, dry_lab_only=True), VALID_CITATION_TYPES (4: dataset, method, schema, software), VALID_REUSE_CLASSES (4: attribution_required, contact_required, open, restricted), VALID_LICENSE_IDENTIFIERS (5: Apache-2.0, CC-BY-4.0, CC-BY-NC-4.0, MIT, Proprietary), validate_citation_entry() (9 checks: non-empty artifact_id, valid citation_type, non-empty title, non-empty version, non-empty authors, 4-digit year, valid license_identifier, valid reuse_class, dry_lab_only must be True; restricted warns, contact_required+no-url warns, empty bibtex_key warns), validate_citation_dict() (8 required fields guard). CLI (openamp-foundry citation-check) with --citation-json, --format text|json. make citation-check target. 24 tests. Ecosystem clarity: citation entries are now machine-validated. @@ -106,7 +108,7 @@ Machine-readable snapshot: `outputs/metrics_snapshot.json` regenerated with `mak > **New in v0.5.31:** Added dipeptide-order features for sequence-order awareness. `dipeptide_order_score` achieves AUROC 0.7861 on AMP-vs-scrambled discrimination — the strongest order-dependent feature in the pipeline. Only 7/31 features survive scrambling (amphipathicity/helix-wheel + dipeptide). All composition features are purely position-independent (exactly 0.5000 AUROC on scrambled test). > **New in v0.5.30:** Easy baseline benchmark added — charge density alone (AUROC 0.8166) outperforms the full pipeline ensemble (0.7792) on AMP-vs-Swiss-Prot-decoy discrimination. Honest finding documented: expected because pipeline optimizes for safety, not raw discrimination. > **New in v0.5.29:** Expanded benchmark to 500 AMPs + 500 composition-matched decoys (n=1000). AUROC 0.7792 (CI₉₅: 0.7505–0.8065) confirms signal generalizes. Cluster-aware CI: 0.746–0.8102. Representative AUROC: 0.778. Standard benchmark (n=191) retained for backward comparison. -> **Pipeline version:** v0.7.6 (J8 — roadmap-to-issue sync checklist) +> **Pipeline version:** v0.7.7 (J9 — external advisory review process) > **Branch:** main --- diff --git a/docs/governance/EXTERNAL_ADVISORY_REVIEW_PROCESS.md b/docs/governance/EXTERNAL_ADVISORY_REVIEW_PROCESS.md new file mode 100644 index 00000000..e7d709b6 --- /dev/null +++ b/docs/governance/EXTERNAL_ADVISORY_REVIEW_PROCESS.md @@ -0,0 +1,96 @@ +# External Advisory Review Process + +This document defines the process for external advisory review of OpenAMP Foundry +research outputs, governance decisions, and release candidates. + +## Purpose + +External review provides independent credibility for: +- Candidate selection rationale (why these sequences, not others) +- Benchmark honesty (do the benchmarks test what they claim?) +- Safety policy adequacy (are the safeguards sufficient?) +- Evidence package completeness (is the evidence ready for wet-lab partners?) + +External review is **advisory only** — it does not replace the internal +governance process defined in GOVERNANCE.md. + +## Reviewer Eligibility + +External advisors must: +1. Hold a relevant domain expertise (antimicrobial peptide biology, ML benchmarking, + biosafety policy, or open science governance) +2. Have no undisclosed conflict of interest (see COI_DISCLOSURE_TEMPLATE.md) +3. Agree in writing to the responsible use policy (RESPONSIBLE_USE.md) +4. Not be a current contributor to the OpenAMP Foundry codebase + +## Review Scope + +| Review type | Trigger | Reviewer count | +|----------------------|-------------------------------------------|----------------| +| Candidate review | Before releasing a candidate list | ≥ 2 | +| Benchmark audit | Before updating a benchmark threshold | ≥ 1 | +| Safety policy review | Before changing dual-use or safety policy | ≥ 2 | +| Evidence review | Before releasing an evidence package | ≥ 1 | +| Governance review | Before changing the release policy | ≥ 1 | + +## Review Process + +### Step 1: Prepare the review packet + +The internal team prepares a review packet containing: +- The artifact or decision under review +- Relevant evidence and benchmarks +- Known limitations and open questions +- The specific questions the advisor is asked to answer + +### Step 2: Assign and disclose + +- Assign the review to an eligible advisor +- Request a COI disclosure (see COI_DISCLOSURE_TEMPLATE.md) +- Provide the advisor with the review packet and a deadline (default: 14 days) + +### Step 3: Receive and log the review + +- Receive the advisor's written review +- Log the review outcome in the decision log (DECISION_LOG.md) +- Note any findings that change the internal assessment + +### Step 4: Respond to findings + +For each advisory finding: +- **Critical finding**: halt release until resolved +- **Major finding**: resolve before release or document why not resolved +- **Minor finding**: document and address in next release cycle +- **Informational**: note in the evidence package + +### Step 5: Close the review + +- Record the review outcome in the governance decision log +- Update the artifact or decision record with the advisory review reference +- Release or defer based on the findings + +## Machine-Validated Review Entries + +Advisory review entries can be validated with: + +```bash +openamp-foundry advisory-review-check --review-json '{"review_id": "ADV-2026-001", ...}' +``` + +See `src/openamp_foundry/governance/advisory_review.py` for the AdvisoryReview schema. + +## Limitations + +- This process validates review structure, not review quality +- External review does not guarantee correctness of the underlying research +- Reviewer availability may affect timelines +- The dry-lab-only constraint applies to all reviews: external advisors cannot + certify wet-lab validity from computational evidence alone + +## Linked Policies + +- [GOVERNANCE.md](../../GOVERNANCE.md) — project governance structure +- [docs/governance/COI_DISCLOSURE_TEMPLATE.md](COI_DISCLOSURE_TEMPLATE.md) — conflict of interest +- [RESPONSIBLE_USE.md](../../RESPONSIBLE_USE.md) — responsible use +- [docs/governance/DECISION_LOG.md](DECISION_LOG.md) — governance decision log +- [docs/evidence/PROOF_LADDER.md](../evidence/PROOF_LADDER.md) — evidence ladder diff --git a/docs/research/ROADMAP.md b/docs/research/ROADMAP.md index cfbafa41..657b02ad 100644 --- a/docs/research/ROADMAP.md +++ b/docs/research/ROADMAP.md @@ -1,5 +1,34 @@ # Roadmap +## v0.7.7 — Loop 117: Phase J J9 — External Advisory Review Process ✓ (2026-07-09) + +`docs/governance/EXTERNAL_ADVISORY_REVIEW_PROCESS.md` with reviewer eligibility +criteria (4 requirements), review scope table (5 review types with minimum +reviewer counts: candidate_review and safety_policy_review require ≥2 reviewers, +others ≥1), 5-step process (prepare packet, assign+disclose COI, receive+log, +respond to findings by severity, close and record), finding severity handling +(critical halts release, major resolves before release, minor defers, informational +notes), limitations section. + +`src/openamp_foundry/governance/advisory_review.py` with `AdvisoryReview` dataclass +(11 fields: review_id, review_type, artifact_id, reviewer_handle, assigned_date, +deadline_date, status, finding_severity, finding_summary, resolved, dry_lab_only), +`AdvisoryReviewResult` dataclass (5 fields, dry_lab_only=True), +`VALID_REVIEW_TYPES` (5: benchmark_audit, candidate_review, evidence_review, +governance_review, safety_policy_review), `VALID_REVIEW_STATUSES` (5: assigned, +completed, deferred, in_progress, pending), `VALID_FINDING_SEVERITIES` (4: +critical, informational, major, minor), `MINIMUM_REVIEWER_COUNTS` (5 entries), +`validate_advisory_review()` (9 checks + 3 warning conditions), +`validate_advisory_review_dict()` (7 required fields guard). + +CLI (`openamp-foundry advisory-review-check`) with `--review-json` (required), +`--format text|json`. Handler `_run_advisory_review_check` in reports.py. + +`make advisory-review-check` target. 29 tests. **3653 total.** + +Credibility: external advisory reviews now have a validated structure, documented +eligibility criteria, and a clear process from assignment to closure. + ## v0.7.6 — Loop 116: Phase J J8 — Roadmap-to-Issue Sync Checklist ✓ (2026-07-09) `docs/governance/ROADMAP_ISSUE_SYNC_CHECKLIST.md` with 5-section checklist: diff --git a/src/openamp_foundry/cli/commands/reports.py b/src/openamp_foundry/cli/commands/reports.py index 7097a39c..87ac83c2 100644 --- a/src/openamp_foundry/cli/commands/reports.py +++ b/src/openamp_foundry/cli/commands/reports.py @@ -2717,6 +2717,40 @@ def _run_citation_check(args: argparse.Namespace) -> int: return 0 if result.passed else 3 +def _run_advisory_review_check(args: argparse.Namespace) -> int: + """Validate an advisory review entry.""" + import json as _json + from openamp_foundry.governance.advisory_review import validate_advisory_review_dict + + try: + d = _json.loads(args.review_json) + except _json.JSONDecodeError as exc: + print(_json.dumps({"status": "error", "error": f"Invalid JSON: {exc}"})) + return 2 + + if not isinstance(d, dict): + print(_json.dumps({"status": "error", "error": "--review-json must be a JSON object"})) + return 2 + + result = validate_advisory_review_dict(d) + output_format = getattr(args, "format", "text") + + if output_format == "json": + import dataclasses + print(_json.dumps(dataclasses.asdict(result), indent=2)) + else: + status = "PASS" if result.passed else "FAIL" + print(f"Advisory review {result.review_id} ({result.review_type}): {status}") + for e in result.errors: + print(f" ERROR: {e}") + for w in result.warnings: + print(f" WARN: {w}") + if result.passed: + print(" Advisory review entry validated. Ready for logging.") + + return 0 if result.passed else 3 + + def _run_roadmap_sync_check(args: argparse.Namespace) -> int: """Validate a roadmap sync entry.""" import json as _json diff --git a/src/openamp_foundry/cli/main.py b/src/openamp_foundry/cli/main.py index a8e97a61..f15baa98 100644 --- a/src/openamp_foundry/cli/main.py +++ b/src/openamp_foundry/cli/main.py @@ -15,6 +15,7 @@ from openamp_foundry.cli.commands.reports import _run_security_report_check from openamp_foundry.cli.commands.reports import _run_citation_check from openamp_foundry.cli.commands.reports import _run_roadmap_sync_check +from openamp_foundry.cli.commands.reports import _run_advisory_review_check import argparse import json @@ -1981,6 +1982,23 @@ def build_parser() -> argparse.ArgumentParser: help="Output format (default: text).", ) + # ── Advisory review check (Phase J J9) ────────────────────────── + arc = sub.add_parser( + "advisory-review-check", + help=( + "Validate an external advisory review entry. Dry-lab only." + ), + ) + arc.add_argument( + "--review-json", type=str, required=True, + help="JSON string of an AdvisoryReview dict (required).", + ) + arc.add_argument( + "--format", type=str, default="text", + choices=["text", "json"], + help="Output format (default: text).", + ) + # ── Simulation-result confidence interval report (H7) ──────────── sicr = sub.add_parser( "simulation-ci-report", @@ -2243,6 +2261,9 @@ def main(argv: list[str] | None = None) -> int: if args.command == "roadmap-sync-check": return _run_roadmap_sync_check(args) + if args.command == "advisory-review-check": + return _run_advisory_review_check(args) + parser.error("unknown command") return 2 diff --git a/src/openamp_foundry/governance/advisory_review.py b/src/openamp_foundry/governance/advisory_review.py new file mode 100644 index 00000000..ffe759f0 --- /dev/null +++ b/src/openamp_foundry/governance/advisory_review.py @@ -0,0 +1,175 @@ +"""External advisory review process validation for OpenAMP Foundry.""" +from __future__ import annotations + +import re +from dataclasses import dataclass, field +from typing import List, Optional + +VALID_REVIEW_TYPES: set[str] = { + "benchmark_audit", + "candidate_review", + "evidence_review", + "governance_review", + "safety_policy_review", +} +VALID_REVIEW_STATUSES: set[str] = { + "assigned", + "completed", + "deferred", + "in_progress", + "pending", +} +VALID_FINDING_SEVERITIES: set[str] = { + "critical", + "informational", + "major", + "minor", +} +MINIMUM_REVIEWER_COUNTS: dict[str, int] = { + "benchmark_audit": 1, + "candidate_review": 2, + "evidence_review": 1, + "governance_review": 1, + "safety_policy_review": 2, +} + + +@dataclass +class AdvisoryReview: + """A single external advisory review entry.""" + + review_id: str + review_type: str + artifact_id: str + reviewer_handle: str + assigned_date: str + deadline_date: str + status: str + finding_severity: Optional[str] = None + finding_summary: str = "" + resolved: bool = False + dry_lab_only: bool = True + + +@dataclass +class AdvisoryReviewResult: + """Result of validating an AdvisoryReview.""" + + review_id: str + review_type: str + passed: bool + errors: List[str] = field(default_factory=list) + warnings: List[str] = field(default_factory=list) + dry_lab_only: bool = True + + +def validate_advisory_review(review: AdvisoryReview) -> AdvisoryReviewResult: + """Validate an AdvisoryReview against policy rules.""" + errors: list[str] = [] + warnings: list[str] = [] + + if not review.review_id or not str(review.review_id).strip(): + errors.append("review_id must be non-empty") + elif not review.review_id.startswith("ADV-"): + errors.append( + f"review_id must start with 'ADV-', got: {review.review_id!r}" + ) + + if review.review_type not in VALID_REVIEW_TYPES: + errors.append( + f"review_type must be one of {sorted(VALID_REVIEW_TYPES)}, " + f"got: {review.review_type!r}" + ) + + if not review.artifact_id or not review.artifact_id.strip(): + errors.append("artifact_id must be non-empty") + + if not review.reviewer_handle or not review.reviewer_handle.strip(): + errors.append("reviewer_handle must be non-empty") + + if not re.match(r"^\d{4}-\d{2}-\d{2}$", review.assigned_date): + errors.append( + f"assigned_date must be YYYY-MM-DD, got: {review.assigned_date!r}" + ) + + if not re.match(r"^\d{4}-\d{2}-\d{2}$", review.deadline_date): + errors.append( + f"deadline_date must be YYYY-MM-DD, got: {review.deadline_date!r}" + ) + + if review.status not in VALID_REVIEW_STATUSES: + errors.append( + f"status must be one of {sorted(VALID_REVIEW_STATUSES)}, " + f"got: {review.status!r}" + ) + + if review.finding_severity is not None and review.finding_severity not in VALID_FINDING_SEVERITIES: + errors.append( + f"finding_severity must be one of {sorted(VALID_FINDING_SEVERITIES)}, " + f"got: {review.finding_severity!r}" + ) + + if not review.dry_lab_only: + errors.append("dry_lab_only must be True for advisory review entries") + + # Warnings + if review.status == "completed" and review.finding_severity == "critical" and not review.resolved: + warnings.append( + "critical finding is unresolved: halt release until this is addressed" + ) + + if review.status == "completed" and not review.finding_summary: + warnings.append( + "status is 'completed' but finding_summary is empty: " + "document the review outcome" + ) + + if review.status == "deferred": + warnings.append( + "review is 'deferred': document the reason and reschedule" + ) + + return AdvisoryReviewResult( + review_id=review.review_id, + review_type=review.review_type, + passed=len(errors) == 0, + errors=errors, + warnings=warnings, + dry_lab_only=True, + ) + + +def validate_advisory_review_dict(d: dict) -> AdvisoryReviewResult: + """Validate a dict representation of an AdvisoryReview.""" + required = [ + "artifact_id", + "assigned_date", + "deadline_date", + "review_id", + "review_type", + "reviewer_handle", + "status", + ] + missing = [f for f in required if f not in d] + if missing: + return AdvisoryReviewResult( + review_id=d.get("review_id", ""), + review_type=d.get("review_type", ""), + passed=False, + errors=[f"Missing required fields: {missing}"], + dry_lab_only=True, + ) + review = AdvisoryReview( + review_id=d["review_id"], + review_type=d["review_type"], + artifact_id=d["artifact_id"], + reviewer_handle=d["reviewer_handle"], + assigned_date=d["assigned_date"], + deadline_date=d["deadline_date"], + status=d["status"], + finding_severity=d.get("finding_severity"), + finding_summary=d.get("finding_summary", ""), + resolved=d.get("resolved", False), + dry_lab_only=d.get("dry_lab_only", True), + ) + return validate_advisory_review(review) diff --git a/tests/governance/test_advisory_review.py b/tests/governance/test_advisory_review.py new file mode 100644 index 00000000..9f0dbdd2 --- /dev/null +++ b/tests/governance/test_advisory_review.py @@ -0,0 +1,201 @@ +"""Tests for external advisory review process validation.""" +import pytest +from openamp_foundry.governance.advisory_review import ( + MINIMUM_REVIEWER_COUNTS, + VALID_FINDING_SEVERITIES, + VALID_REVIEW_STATUSES, + VALID_REVIEW_TYPES, + AdvisoryReview, + AdvisoryReviewResult, + validate_advisory_review, + validate_advisory_review_dict, +) + + +def _valid_review(**kwargs) -> AdvisoryReview: + defaults = dict( + review_id="ADV-2026-001", + review_type="candidate_review", + artifact_id="OAMP-v0.7.6-candidates", + reviewer_handle="external-advisor-1", + assigned_date="2026-07-09", + deadline_date="2026-07-23", + status="pending", + finding_severity=None, + finding_summary="", + resolved=False, + dry_lab_only=True, + ) + defaults.update(kwargs) + return AdvisoryReview(**defaults) + + +class TestValidAdvisoryReview: + def test_valid_review_passes(self): + result = validate_advisory_review(_valid_review()) + assert result.passed is True + assert result.errors == [] + + def test_result_has_correct_review_id(self): + result = validate_advisory_review(_valid_review()) + assert result.review_id == "ADV-2026-001" + + def test_result_has_correct_review_type(self): + result = validate_advisory_review(_valid_review()) + assert result.review_type == "candidate_review" + + def test_all_review_types_accepted(self): + for rt in VALID_REVIEW_TYPES: + result = validate_advisory_review(_valid_review(review_type=rt)) + assert result.passed is True, f"Expected {rt} to pass" + + def test_all_statuses_accepted(self): + for status in VALID_REVIEW_STATUSES: + result = validate_advisory_review(_valid_review(status=status)) + assert result.passed is True, f"Expected status {status} to pass" + + def test_all_finding_severities_accepted(self): + for sev in VALID_FINDING_SEVERITIES: + result = validate_advisory_review( + _valid_review( + status="completed", + finding_severity=sev, + finding_summary="Some finding.", + ) + ) + assert result.passed is True, f"Expected severity {sev} to pass" + + def test_completed_with_summary_passes(self): + result = validate_advisory_review( + _valid_review( + status="completed", + finding_severity="minor", + finding_summary="All looks reasonable.", + resolved=True, + ) + ) + assert result.passed is True + + +class TestInvalidAdvisoryReview: + def test_empty_review_id_fails(self): + result = validate_advisory_review(_valid_review(review_id="")) + assert result.passed is False + assert any("review_id" in e for e in result.errors) + + def test_review_id_without_adv_prefix_fails(self): + result = validate_advisory_review(_valid_review(review_id="REV-2026-001")) + assert result.passed is False + assert any("ADV-" in e for e in result.errors) + + def test_invalid_review_type_fails(self): + result = validate_advisory_review(_valid_review(review_type="informal_chat")) + assert result.passed is False + assert any("review_type" in e for e in result.errors) + + def test_empty_artifact_id_fails(self): + result = validate_advisory_review(_valid_review(artifact_id="")) + assert result.passed is False + assert any("artifact_id" in e for e in result.errors) + + def test_empty_reviewer_handle_fails(self): + result = validate_advisory_review(_valid_review(reviewer_handle="")) + assert result.passed is False + assert any("reviewer_handle" in e for e in result.errors) + + def test_invalid_assigned_date_fails(self): + result = validate_advisory_review(_valid_review(assigned_date="09/07/2026")) + assert result.passed is False + assert any("assigned_date" in e for e in result.errors) + + def test_invalid_deadline_date_fails(self): + result = validate_advisory_review(_valid_review(deadline_date="2026-7-23")) + assert result.passed is False + assert any("deadline_date" in e for e in result.errors) + + def test_invalid_status_fails(self): + result = validate_advisory_review(_valid_review(status="cancelled")) + assert result.passed is False + assert any("status" in e for e in result.errors) + + def test_invalid_finding_severity_fails(self): + result = validate_advisory_review( + _valid_review(finding_severity="severe") + ) + assert result.passed is False + assert any("finding_severity" in e for e in result.errors) + + def test_dry_lab_only_false_fails(self): + result = validate_advisory_review(_valid_review(dry_lab_only=False)) + assert result.passed is False + assert any("dry_lab_only" in e for e in result.errors) + + +class TestAdvisoryReviewWarnings: + def test_critical_unresolved_warns(self): + result = validate_advisory_review( + _valid_review( + status="completed", + finding_severity="critical", + finding_summary="Critical issue found.", + resolved=False, + ) + ) + assert result.passed is True + assert any("critical" in w for w in result.warnings) + + def test_completed_without_summary_warns(self): + result = validate_advisory_review( + _valid_review(status="completed", finding_summary="") + ) + assert result.passed is True + assert any("finding_summary" in w for w in result.warnings) + + def test_deferred_status_warns(self): + result = validate_advisory_review(_valid_review(status="deferred")) + assert result.passed is True + assert any("deferred" in w for w in result.warnings) + + +class TestValidateAdvisoryReviewDict: + def test_valid_dict_passes(self): + d = { + "review_id": "ADV-2026-001", + "review_type": "candidate_review", + "artifact_id": "OAMP-v0.7.6-candidates", + "reviewer_handle": "external-advisor-1", + "assigned_date": "2026-07-09", + "deadline_date": "2026-07-23", + "status": "pending", + } + result = validate_advisory_review_dict(d) + assert result.passed is True + + def test_missing_fields_fails(self): + result = validate_advisory_review_dict({"review_id": "ADV-2026-001"}) + assert result.passed is False + assert any("Missing" in e for e in result.errors) + + +class TestConstants: + def test_valid_review_types_has_5(self): + assert len(VALID_REVIEW_TYPES) == 5 + + def test_valid_review_statuses_has_5(self): + assert len(VALID_REVIEW_STATUSES) == 5 + + def test_valid_finding_severities_has_4(self): + assert len(VALID_FINDING_SEVERITIES) == 4 + + def test_minimum_reviewer_counts_has_5(self): + assert len(MINIMUM_REVIEWER_COUNTS) == 5 + + def test_safety_policy_review_requires_2(self): + assert MINIMUM_REVIEWER_COUNTS["safety_policy_review"] == 2 + + def test_candidate_review_requires_2(self): + assert MINIMUM_REVIEWER_COUNTS["candidate_review"] == 2 + + def test_all_results_dry_lab_only_true(self): + result = validate_advisory_review(_valid_review()) + assert result.dry_lab_only is True diff --git a/tests/test_test_count_regression.py b/tests/test_test_count_regression.py index b41cee02..a823a12b 100644 --- a/tests/test_test_count_regression.py +++ b/tests/test_test_count_regression.py @@ -2,11 +2,11 @@ import subprocess import sys -# Baseline: 3624 tests as of Loop 116 (2026-07-09) -- Phase J J8 (roadmap-to-issue sync checklist) +# Baseline: 3653 tests as of Loop 117 (2026-07-09) -- Phase J J9 (external advisory review process) # Note: 3 pre-existing collection errors in test_benchmark_calibration.py, # test_benchmark_charge_distribution.py, test_benchmark_cheap_enemies.py. # Raise threshold if legitimate tests are removed; lower threshold is okay. -BASELINE = 3624 +BASELINE = 3653 TOLERANCE = 0.05 # allow 5% fluctuation