diff --git a/Makefile b/Makefile index 8be7b9a..6fd7e47 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,16 @@ PYTHON ?= python3 -.PHONY: review-build review-check +.PHONY: review-build review-data-check review-test review-check review-build: - $(PYTHON) review/build_review.py + $(PYTHON) -B review/build_review.py -review-check: - $(PYTHON) review/build_review.py --check +review-data-check: + $(PYTHON) -B review/build_review.py --check-data + +review-test: + $(PYTHON) -B -m unittest discover -s review/tests -p 'test_*.py' + +review-check: review-data-check review-test + $(PYTHON) -B review/build_review.py --check node review/check_review.mjs diff --git a/review/build_review.py b/review/build_review.py index 6994b0f..bdebfca 100644 --- a/review/build_review.py +++ b/review/build_review.py @@ -13,6 +13,8 @@ from markdown_it import MarkdownIt +from review_data import load_review_payload, normalize_reviews, validate_review_payload + ROOT = Path(__file__).resolve().parents[1] REVIEW_DIR = Path(__file__).resolve().parent @@ -21,9 +23,6 @@ TEMPLATE_PATH = REVIEW_DIR / "review-template.html" STYLESHEET_PATH = REVIEW_DIR / "standalone.css" OUTPUT_PATH = REVIEW_DIR / "agent-surface-rfc-review.html" -VALID_PRIORITIES = {"P0", "P1", "P2", "P3"} -VALID_SIDES = {"left", "right"} -VALID_STATUSES = {"present", "partial", "missing"} def slugify(value: str) -> str: @@ -59,56 +58,9 @@ def render_rfc() -> tuple[str, dict[str, str]]: def load_reviews(heading_ids: dict[str, str]) -> list[dict[str, object]]: - payload = json.loads(DATA_PATH.read_text(encoding="utf-8")) - reviews = payload.get("reviews") - if not isinstance(reviews, list) or not reviews: - raise ValueError("review-data.json must contain a non-empty reviews array") - - ids = [review.get("id") for review in reviews] - if not all(isinstance(review_id, int) for review_id in ids): - raise ValueError("Every review id must be an integer") - if sorted(ids) != list(range(1, len(reviews) + 1)): - raise ValueError("Review ids must be unique and sequential, starting at 1") - - missing_headings: list[str] = [] - normalized_reviews: list[dict[str, object]] = [] - required_fields = {"id", "title", "description", "category", "side", "status", "rationale", "priority", "anchors"} - for review in sorted(reviews, key=lambda item: item["id"]): - absent = required_fields - set(review) - if absent: - raise ValueError(f"Review #{review.get('id', '?')} is missing fields: {', '.join(sorted(absent))}") - if review["side"] not in VALID_SIDES: - raise ValueError(f"Review #{review['id']} has invalid side: {review['side']}") - if review["status"] not in VALID_STATUSES: - raise ValueError(f"Review #{review['id']} has invalid status: {review['status']}") - if review["priority"] not in VALID_PRIORITIES: - raise ValueError(f"Review #{review['id']} has invalid priority: {review['priority']}") - if not all(isinstance(review[field], str) and review[field].strip() for field in required_fields - {"id", "anchors"}): - raise ValueError(f"Review #{review['id']} has an empty text field") - anchors = review["anchors"] - if not isinstance(anchors, list) or not anchors: - raise ValueError(f"Review #{review['id']} must have anchors") - anchor_headings = [] - for anchor in anchors: - heading = anchor if isinstance(anchor, str) else anchor.get("heading") if isinstance(anchor, dict) else None - if not isinstance(heading, str) or not heading.strip(): - raise ValueError(f"Review #{review['id']} has an invalid anchor") - anchor_headings.append(heading) - if len(anchor_headings) != len(set(anchor_headings)): - raise ValueError(f"Review #{review['id']} must have unique anchors") - - resolved_anchors = [] - for heading in anchor_headings: - anchor_id = heading_ids.get(heading) - if anchor_id is None: - missing_headings.append(f"#{review['id']}: {heading}") - continue - resolved_anchors.append({"heading": heading, "anchorId": anchor_id}) - normalized_reviews.append({**review, "anchors": resolved_anchors}) - - if missing_headings: - raise ValueError("Unresolved RFC headings:\n" + "\n".join(missing_headings)) - return normalized_reviews + payload = load_review_payload(DATA_PATH) + validate_review_payload(payload, heading_ids) + return normalize_reviews(payload, heading_ids) def build_document() -> str: @@ -142,9 +94,24 @@ def build_document() -> str: def main() -> int: parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--check", action="store_true", help="fail when the committed dashboard is stale") + mode = parser.add_mutually_exclusive_group() + mode.add_argument("--check", action="store_true", help="fail when the committed dashboard is stale") + mode.add_argument( + "--check-data", + action="store_true", + help="validate review metadata without writing the dashboard", + ) args = parser.parse_args() - document = build_document() + try: + if args.check_data: + _, heading_ids = render_rfc() + load_reviews(heading_ids) + print(f"Review data is valid: {DATA_PATH.relative_to(ROOT)}") + return 0 + document = build_document() + except (json.JSONDecodeError, ValueError) as error: + print(error, file=sys.stderr) + return 1 if args.check: if not OUTPUT_PATH.exists() or OUTPUT_PATH.read_text(encoding="utf-8") != document: print("Dashboard is stale; run: make review-build", file=sys.stderr) diff --git a/review/requirements.txt b/review/requirements.txt index b2301e9..6011a1c 100644 --- a/review/requirements.txt +++ b/review/requirements.txt @@ -1 +1,2 @@ markdown-it-py>=3.0,<5 +jsonschema>=4.23,<5 diff --git a/review/review-data.json b/review/review-data.json index 61da548..410d49b 100644 --- a/review/review-data.json +++ b/review/review-data.json @@ -1,4 +1,102 @@ { + "schema_version": 2, + "planning_metadata_mode": "transitional", + "profiles": [ + { + "id": "core", + "title": "Core Protocol", + "depends_on": [] + }, + { + "id": "manifest", + "title": "Agent Surface Manifest", + "depends_on": [ + "core" + ] + }, + { + "id": "oauth-grants", + "title": "OAuth Grants", + "depends_on": [ + "core", + "manifest" + ] + }, + { + "id": "receipts-provenance", + "title": "Receipts and Provenance", + "depends_on": [ + "core", + "manifest", + "oauth-grants" + ] + }, + { + "id": "action-execution", + "title": "Action Execution", + "depends_on": [ + "core", + "manifest", + "oauth-grants", + "receipts-provenance" + ] + }, + { + "id": "events-sessions", + "title": "Events and Sessions", + "depends_on": [ + "core", + "oauth-grants" + ] + }, + { + "id": "privacy-consent", + "title": "Privacy and Consent", + "depends_on": [ + "core", + "oauth-grants", + "receipts-provenance" + ] + }, + { + "id": "identity-passport", + "title": "Identity and Passport", + "depends_on": [ + "core", + "oauth-grants" + ] + }, + { + "id": "operations-safety", + "title": "Operations Safety", + "depends_on": [ + "core", + "action-execution", + "events-sessions" + ] + }, + { + "id": "conformance-tooling", + "title": "Conformance and Tooling", + "depends_on": [ + "core", + "manifest", + "oauth-grants", + "receipts-provenance", + "action-execution", + "events-sessions", + "privacy-consent", + "identity-passport", + "operations-safety" + ] + } + ], + "releases": [ + { + "id": "0.2", + "status": "planning" + } + ], "reviews": [ { "id": 1, diff --git a/review/review-data.schema.json b/review/review-data.schema.json new file mode 100644 index 0000000..bab8f30 --- /dev/null +++ b/review/review-data.schema.json @@ -0,0 +1,312 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://0al-spec.github.io/agent-surface/review/review-data.schema.json", + "title": "Agent Surface RFC review planning data", + "type": "object", + "additionalProperties": false, + "required": [ + "schema_version", + "planning_metadata_mode", + "profiles", + "releases", + "reviews" + ], + "properties": { + "schema_version": { + "const": 2 + }, + "planning_metadata_mode": { + "enum": [ + "transitional", + "required" + ] + }, + "profiles": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/profile" + } + }, + "releases": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/release" + } + }, + "reviews": { + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/review" + } + } + }, + "allOf": [ + { + "if": { + "properties": { + "planning_metadata_mode": { + "const": "required" + } + } + }, + "then": { + "properties": { + "reviews": { + "items": { + "required": [ + "profile", + "depends_on", + "target_release", + "maturity", + "evidence" + ] + } + } + } + } + } + ], + "$defs": { + "identifier": { + "type": "string", + "pattern": "^[a-z][a-z0-9]*(?:-[a-z0-9]+)*$" + }, + "anchorSlug": { + "type": "string", + "pattern": "^[a-z0-9]+(?:-[a-z0-9]+)*$" + }, + "nonEmptyString": { + "type": "string", + "minLength": 1, + "pattern": "\\S" + }, + "profile": { + "type": "object", + "additionalProperties": false, + "required": [ + "id", + "title", + "depends_on" + ], + "properties": { + "id": { + "$ref": "#/$defs/identifier" + }, + "title": { + "$ref": "#/$defs/nonEmptyString" + }, + "description": { + "$ref": "#/$defs/nonEmptyString" + }, + "depends_on": { + "type": "array", + "uniqueItems": true, + "items": { + "$ref": "#/$defs/identifier" + } + } + } + }, + "release": { + "type": "object", + "additionalProperties": false, + "required": [ + "id", + "status" + ], + "properties": { + "id": { + "type": "string", + "minLength": 1, + "pattern": "\\S" + }, + "title": { + "$ref": "#/$defs/nonEmptyString" + }, + "status": { + "enum": [ + "planning", + "active", + "released", + "deferred" + ] + } + } + }, + "anchor": { + "type": "object", + "additionalProperties": false, + "required": [ + "heading", + "anchorId" + ], + "properties": { + "heading": { + "$ref": "#/$defs/nonEmptyString" + }, + "anchorId": { + "$ref": "#/$defs/anchorSlug" + } + } + }, + "evidence": { + "type": "object", + "additionalProperties": false, + "required": [ + "kind", + "ref" + ], + "properties": { + "kind": { + "enum": [ + "rfc_anchor", + "schema", + "test", + "implementation", + "interop_test", + "registry" + ] + }, + "ref": { + "$ref": "#/$defs/nonEmptyString" + }, + "description": { + "$ref": "#/$defs/nonEmptyString" + } + } + }, + "review": { + "type": "object", + "additionalProperties": false, + "dependentRequired": { + "profile": [ + "depends_on", + "target_release", + "maturity", + "evidence" + ], + "depends_on": [ + "profile", + "target_release", + "maturity", + "evidence" + ], + "target_release": [ + "profile", + "depends_on", + "maturity", + "evidence" + ], + "maturity": [ + "profile", + "depends_on", + "target_release", + "evidence" + ], + "evidence": [ + "profile", + "depends_on", + "target_release", + "maturity" + ] + }, + "required": [ + "id", + "title", + "description", + "category", + "side", + "anchors", + "status", + "rationale", + "priority" + ], + "properties": { + "id": { + "type": "integer", + "minimum": 1 + }, + "title": { + "$ref": "#/$defs/nonEmptyString" + }, + "description": { + "$ref": "#/$defs/nonEmptyString" + }, + "category": { + "$ref": "#/$defs/nonEmptyString" + }, + "side": { + "enum": [ + "left", + "right" + ] + }, + "anchors": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "$ref": "#/$defs/anchor" + } + }, + "status": { + "enum": [ + "present", + "partial", + "missing" + ] + }, + "rationale": { + "$ref": "#/$defs/nonEmptyString" + }, + "priority": { + "enum": [ + "P0", + "P1", + "P2", + "P3" + ] + }, + "profile": { + "$ref": "#/$defs/identifier" + }, + "depends_on": { + "type": "array", + "uniqueItems": true, + "items": { + "type": "integer", + "minimum": 1 + } + }, + "target_release": { + "type": [ + "string", + "null" + ], + "minLength": 1, + "pattern": "\\S" + }, + "maturity": { + "enum": [ + "proposal", + "specified", + "machine_validated", + "implementation_tested", + "interop_tested", + "stable" + ] + }, + "evidence": { + "type": "array", + "uniqueItems": true, + "items": { + "$ref": "#/$defs/evidence" + } + } + } + } + } +} diff --git a/review/review_data.py b/review/review_data.py new file mode 100644 index 0000000..27c061f --- /dev/null +++ b/review/review_data.py @@ -0,0 +1,217 @@ +"""Validation and derived planning state for RFC review metadata.""" + +from __future__ import annotations + +import json +from collections import Counter, deque +from collections.abc import Mapping, Sequence +from pathlib import Path +from typing import Any + +from jsonschema import Draft202012Validator + + +REVIEW_DIR = Path(__file__).resolve().parent +DATA_PATH = REVIEW_DIR / "review-data.json" +SCHEMA_PATH = REVIEW_DIR / "review-data.schema.json" +MATURITY_ORDER = ( + "proposal", + "specified", + "machine_validated", + "implementation_tested", + "interop_tested", + "stable", +) + + +def load_review_payload(path: Path = DATA_PATH) -> dict[str, Any]: + payload = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(payload, dict): + raise ValueError("review-data.json must contain a JSON object") + return payload + + +def validate_review_payload( + payload: Mapping[str, Any], + heading_ids: Mapping[str, str], +) -> None: + """Validate the JSON shape and cross-object planning invariants.""" + + _validate_schema(payload) + profiles = payload["profiles"] + releases = payload["releases"] + reviews = payload["reviews"] + require_planning_metadata = payload["planning_metadata_mode"] == "required" + + profile_ids = _unique_ids(profiles, "profile") + release_ids = _unique_ids(releases, "release") + review_ids = _unique_ids(reviews, "review") + if sorted(review_ids) != list(range(1, len(reviews) + 1)): + raise ValueError("Review ids must be unique and sequential, starting at 1") + + _validate_dependency_graph(profiles, profile_ids, "profile") + _validate_dependency_graph(reviews, review_ids, "review") + + actual_anchor_ids = set(heading_ids.values()) + for review in reviews: + review_id = review["id"] + if require_planning_metadata: + missing = {"profile", "depends_on", "target_release", "maturity", "evidence"} - set(review) + if missing: + raise ValueError( + f"Review #{review_id} is missing planning metadata: {', '.join(sorted(missing))}" + ) + + profile = review.get("profile") + if profile is not None and profile not in profile_ids: + raise ValueError(f"Review #{review_id} references unknown profile: {profile}") + + target_release = review.get("target_release") + if target_release is not None and target_release not in release_ids: + raise ValueError(f"Review #{review_id} references unknown release: {target_release}") + + review_anchor_ids: set[str] = set() + anchor_headings: set[str] = set() + for anchor in review["anchors"]: + heading = anchor["heading"] + anchor_id = anchor["anchorId"] + if heading in anchor_headings: + raise ValueError(f"Review #{review_id} must have unique anchor headings") + anchor_headings.add(heading) + expected_anchor_id = heading_ids.get(heading) + if expected_anchor_id is None: + raise ValueError(f"Review #{review_id} references unresolved RFC heading: {heading}") + if anchor_id != expected_anchor_id: + raise ValueError( + f"Review #{review_id} has stale anchorId for {heading!r}: " + f"expected {expected_anchor_id!r}, got {anchor_id!r}" + ) + review_anchor_ids.add(anchor_id) + + evidence = review.get("evidence", []) + evidence_keys = [(item["kind"], item["ref"]) for item in evidence] + duplicate_evidence = sorted( + {key for key in evidence_keys if evidence_keys.count(key) > 1} + ) + if duplicate_evidence: + formatted = ", ".join(f"{kind}:{ref}" for kind, ref in duplicate_evidence) + raise ValueError(f"Review #{review_id} has duplicate evidence references: {formatted}") + evidence_kinds = {item["kind"] for item in evidence} + for item in evidence: + if item["kind"] != "rfc_anchor": + raise ValueError( + f"Review #{review_id} evidence kind {item['kind']!r} is not supported " + "until an authoritative resolver is configured" + ) + ref = item["ref"] + if ref not in actual_anchor_ids: + raise ValueError(f"Review #{review_id} evidence references unknown RFC anchor: {ref}") + if ref not in review_anchor_ids: + raise ValueError( + f"Review #{review_id} RFC evidence {ref!r} must also be declared in anchors" + ) + + maturity = review.get("maturity") + if maturity is not None: + if review["status"] in {"partial", "missing"} and maturity != "proposal": + raise ValueError( + f"Review #{review_id} status {review['status']!r} cannot exceed proposal maturity" + ) + _validate_maturity_evidence(review_id, maturity, evidence_kinds) + + +def normalize_reviews( + payload: Mapping[str, Any], heading_ids: Mapping[str, str] +) -> list[dict[str, Any]]: + """Return reviews in stable id order with RFC anchors normalized from headings.""" + + normalized: list[dict[str, Any]] = [] + for review in sorted(payload["reviews"], key=lambda item: item["id"]): + anchors = [ + {"heading": anchor["heading"], "anchorId": heading_ids[anchor["heading"]]} + for anchor in review["anchors"] + ] + normalized.append({**review, "anchors": anchors}) + return normalized + + +def _validate_schema(payload: Mapping[str, Any]) -> None: + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + Draft202012Validator.check_schema(schema) + errors = sorted( + Draft202012Validator(schema).iter_errors(payload), + key=lambda error: tuple(str(part) for part in error.absolute_path), + ) + if not errors: + return + details = [] + for error in errors: + path = "$" + "".join( + f"[{part}]" if isinstance(part, int) else f".{part}" for part in error.absolute_path + ) + details.append(f"{path}: {error.message}") + raise ValueError("review-data.json does not match review-data.schema.json:\n" + "\n".join(details)) + + +def _unique_ids(items: Sequence[Mapping[str, Any]], label: str) -> set[Any]: + identifiers = [item["id"] for item in items] + duplicates = sorted(item_id for item_id, count in Counter(identifiers).items() if count > 1) + if duplicates: + raise ValueError(f"Duplicate {label} ids: {', '.join(map(str, duplicates))}") + return set(identifiers) + + +def _validate_dependency_graph( + items: Sequence[Mapping[str, Any]], valid_ids: set[Any], label: str +) -> None: + graph = {item["id"]: tuple(item.get("depends_on", ())) for item in items} + for item_id, dependencies in graph.items(): + if item_id in dependencies: + raise ValueError(f"{label.title()} {item_id!r} cannot depend on itself") + unknown = sorted(set(dependencies) - valid_ids) + if unknown: + raise ValueError( + f"{label.title()} {item_id!r} references unknown dependencies: " + f"{', '.join(map(str, unknown))}" + ) + + dependent_ids: dict[Any, list[Any]] = {item_id: [] for item_id in graph} + remaining_dependencies = { + item_id: len(dependencies) for item_id, dependencies in graph.items() + } + for item_id, dependencies in graph.items(): + for dependency in dependencies: + dependent_ids[dependency].append(item_id) + + ready = deque( + sorted(item_id for item_id, count in remaining_dependencies.items() if count == 0) + ) + visited_count = 0 + while ready: + item_id = ready.popleft() + visited_count += 1 + for dependent_id in sorted(dependent_ids[item_id]): + remaining_dependencies[dependent_id] -= 1 + if remaining_dependencies[dependent_id] == 0: + ready.append(dependent_id) + + if visited_count != len(graph): + cyclic_ids = sorted( + item_id for item_id, count in remaining_dependencies.items() if count > 0 + ) + raise ValueError( + f"{label.title()} dependency graph is cyclic; unresolved nodes: " + + ", ".join(map(str, cyclic_ids)) + ) + + +def _validate_maturity_evidence( + review_id: int, maturity: str, evidence_kinds: set[str] +) -> None: + if maturity not in {"proposal", "specified"}: + raise ValueError( + f"Review #{review_id} maturity {maturity!r} is not supported until " + "authoritative evidence resolvers are configured" + ) + if maturity == "specified" and "rfc_anchor" not in evidence_kinds: + raise ValueError(f"Review #{review_id} maturity {maturity!r} requires rfc_anchor evidence") diff --git a/review/tests/fixtures/invalid-partial-metadata.json b/review/tests/fixtures/invalid-partial-metadata.json new file mode 100644 index 0000000..aba2bfe --- /dev/null +++ b/review/tests/fixtures/invalid-partial-metadata.json @@ -0,0 +1,36 @@ +{ + "schema_version": 2, + "planning_metadata_mode": "required", + "profiles": [ + { + "id": "core", + "title": "Core Protocol", + "depends_on": [] + } + ], + "releases": [ + { + "id": "0.2", + "status": "planning" + } + ], + "reviews": [ + { + "id": 1, + "title": "Half-migrated review", + "description": "Only one planning field is present.", + "category": "Fixture", + "side": "left", + "anchors": [ + { + "heading": "Abstract", + "anchorId": "abstract" + } + ], + "status": "partial", + "rationale": "Invalid by design.", + "priority": "P1", + "profile": "core" + } + ] +} diff --git a/review/tests/fixtures/invalid-partial-specified.json b/review/tests/fixtures/invalid-partial-specified.json new file mode 100644 index 0000000..a2a3b36 --- /dev/null +++ b/review/tests/fixtures/invalid-partial-specified.json @@ -0,0 +1,45 @@ +{ + "schema_version": 2, + "planning_metadata_mode": "required", + "profiles": [ + { + "id": "core", + "title": "Core Protocol", + "depends_on": [] + } + ], + "releases": [ + { + "id": "0.2", + "status": "planning" + } + ], + "reviews": [ + { + "id": 1, + "title": "Overstated review", + "description": "Partial coverage cannot claim specified maturity.", + "category": "Fixture", + "side": "left", + "anchors": [ + { + "heading": "Abstract", + "anchorId": "abstract" + } + ], + "status": "partial", + "rationale": "Invalid by design.", + "priority": "P1", + "profile": "core", + "depends_on": [], + "target_release": null, + "maturity": "specified", + "evidence": [ + { + "kind": "rfc_anchor", + "ref": "abstract" + } + ] + } + ] +} diff --git a/review/tests/fixtures/invalid-review-cycle.json b/review/tests/fixtures/invalid-review-cycle.json new file mode 100644 index 0000000..f4e1665 --- /dev/null +++ b/review/tests/fixtures/invalid-review-cycle.json @@ -0,0 +1,65 @@ +{ + "schema_version": 2, + "planning_metadata_mode": "required", + "profiles": [ + { + "id": "core", + "title": "Core Protocol", + "depends_on": [] + } + ], + "releases": [ + { + "id": "0.2", + "status": "planning" + } + ], + "reviews": [ + { + "id": 1, + "title": "First review", + "description": "First half of a dependency cycle.", + "category": "Fixture", + "side": "left", + "anchors": [ + { + "heading": "Abstract", + "anchorId": "abstract" + } + ], + "status": "missing", + "rationale": "Invalid by design.", + "priority": "P1", + "profile": "core", + "depends_on": [ + 2 + ], + "target_release": null, + "maturity": "proposal", + "evidence": [] + }, + { + "id": 2, + "title": "Second review", + "description": "Second half of a dependency cycle.", + "category": "Fixture", + "side": "right", + "anchors": [ + { + "heading": "Abstract", + "anchorId": "abstract" + } + ], + "status": "missing", + "rationale": "Invalid by design.", + "priority": "P1", + "profile": "core", + "depends_on": [ + 1 + ], + "target_release": null, + "maturity": "proposal", + "evidence": [] + } + ] +} diff --git a/review/tests/fixtures/invalid-stale-anchor.json b/review/tests/fixtures/invalid-stale-anchor.json new file mode 100644 index 0000000..1c2ea6f --- /dev/null +++ b/review/tests/fixtures/invalid-stale-anchor.json @@ -0,0 +1,40 @@ +{ + "schema_version": 2, + "planning_metadata_mode": "required", + "profiles": [ + { + "id": "core", + "title": "Core Protocol", + "depends_on": [] + } + ], + "releases": [ + { + "id": "0.2", + "status": "planning" + } + ], + "reviews": [ + { + "id": 1, + "title": "Stale anchor review", + "description": "The persisted anchor id does not match the RFC heading.", + "category": "Fixture", + "side": "left", + "anchors": [ + { + "heading": "Abstract", + "anchorId": "not-abstract" + } + ], + "status": "partial", + "rationale": "Invalid by design.", + "priority": "P1", + "profile": "core", + "depends_on": [], + "target_release": null, + "maturity": "proposal", + "evidence": [] + } + ] +} diff --git a/review/tests/fixtures/valid-minimal-v2.json b/review/tests/fixtures/valid-minimal-v2.json new file mode 100644 index 0000000..71afcc5 --- /dev/null +++ b/review/tests/fixtures/valid-minimal-v2.json @@ -0,0 +1,45 @@ +{ + "schema_version": 2, + "planning_metadata_mode": "required", + "profiles": [ + { + "id": "core", + "title": "Core Protocol", + "depends_on": [] + } + ], + "releases": [ + { + "id": "0.2", + "status": "planning" + } + ], + "reviews": [ + { + "id": 1, + "title": "Minimal review", + "description": "Minimal valid review fixture.", + "category": "Fixture", + "side": "left", + "anchors": [ + { + "heading": "Abstract", + "anchorId": "abstract" + } + ], + "status": "partial", + "rationale": "The fixture exercises the planning metadata bundle.", + "priority": "P1", + "profile": "core", + "depends_on": [], + "target_release": null, + "maturity": "proposal", + "evidence": [ + { + "kind": "rfc_anchor", + "ref": "abstract" + } + ] + } + ] +} diff --git a/review/tests/test_review_data.py b/review/tests/test_review_data.py new file mode 100644 index 0000000..a8e24dc --- /dev/null +++ b/review/tests/test_review_data.py @@ -0,0 +1,188 @@ +from __future__ import annotations + +import copy +import json +import sys +import unittest +from pathlib import Path + + +REVIEW_DIR = Path(__file__).resolve().parents[1] +FIXTURE_DIR = Path(__file__).resolve().parent / "fixtures" +sys.path.insert(0, str(REVIEW_DIR)) + +from build_review import render_rfc # noqa: E402 +from review_data import load_review_payload, validate_review_payload # noqa: E402 + + +class ReviewDataValidationTests(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + _, cls.heading_ids = render_rfc() + + def fixture(self, name: str) -> dict[str, object]: + return json.loads((FIXTURE_DIR / name).read_text(encoding="utf-8")) + + def valid_payload(self) -> dict[str, object]: + return self.fixture("valid-minimal-v2.json") + + def assert_invalid(self, payload: dict[str, object], message: str) -> None: + with self.assertRaisesRegex(ValueError, message): + validate_review_payload(payload, self.heading_ids) + + def test_canonical_review_data_is_valid_during_transition(self) -> None: + validate_review_payload(load_review_payload(), self.heading_ids) + + def test_valid_v2_planning_bundle_is_accepted(self) -> None: + validate_review_payload(self.valid_payload(), self.heading_ids) + + def test_invalid_schema_version_is_rejected(self) -> None: + payload = self.valid_payload() + payload["schema_version"] = 1 + self.assert_invalid(payload, "does not match review-data.schema.json") + + def test_missing_required_field_is_rejected(self) -> None: + payload = self.valid_payload() + del payload["reviews"][0]["title"] + self.assert_invalid(payload, "does not match review-data.schema.json") + + def test_persisted_derived_blocks_are_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["blocks"] = [] + self.assert_invalid(payload, "does not match review-data.schema.json") + + def test_half_migrated_planning_bundle_fixture_is_rejected(self) -> None: + self.assert_invalid( + self.fixture("invalid-partial-metadata.json"), + "does not match review-data.schema.json", + ) + + def test_duplicate_profile_id_is_rejected(self) -> None: + payload = self.valid_payload() + payload["profiles"].append(copy.deepcopy(payload["profiles"][0])) + self.assert_invalid(payload, "Duplicate profile ids") + + def test_duplicate_release_id_is_rejected(self) -> None: + payload = self.valid_payload() + payload["releases"].append(copy.deepcopy(payload["releases"][0])) + self.assert_invalid(payload, "Duplicate release ids") + + def test_duplicate_review_id_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"].append(copy.deepcopy(payload["reviews"][0])) + self.assert_invalid(payload, "Duplicate review ids") + + def test_unknown_profile_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["profile"] = "unknown" + self.assert_invalid(payload, "references unknown profile") + + def test_unknown_release_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["target_release"] = "9.9" + self.assert_invalid(payload, "references unknown release") + + def test_unknown_review_dependency_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["depends_on"] = [99] + self.assert_invalid(payload, "references unknown dependencies") + + def test_self_dependency_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["depends_on"] = [1] + self.assert_invalid(payload, "cannot depend on itself") + + def test_profile_cycle_is_rejected(self) -> None: + payload = self.valid_payload() + payload["profiles"].extend( + [ + {"id": "one", "title": "One", "depends_on": ["two"]}, + {"id": "two", "title": "Two", "depends_on": ["one"]}, + ] + ) + self.assert_invalid(payload, "Profile dependency graph is cyclic") + + def test_review_cycle_fixture_is_rejected(self) -> None: + self.assert_invalid( + self.fixture("invalid-review-cycle.json"), "Review dependency graph is cyclic" + ) + + def test_stale_anchor_fixture_is_rejected(self) -> None: + self.assert_invalid(self.fixture("invalid-stale-anchor.json"), "stale anchorId") + + def test_unknown_rfc_evidence_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["evidence"] = [ + {"kind": "rfc_anchor", "ref": "unknown-anchor"} + ] + self.assert_invalid(payload, "evidence references unknown RFC anchor") + + def test_duplicate_evidence_identity_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["evidence"] = [ + {"kind": "rfc_anchor", "ref": "abstract", "description": "one"}, + {"kind": "rfc_anchor", "ref": "abstract", "description": "two"}, + ] + self.assert_invalid(payload, "duplicate evidence references") + + def test_specified_maturity_requires_rfc_evidence(self) -> None: + payload = self.valid_payload() + review = payload["reviews"][0] + review["status"] = "present" + review["maturity"] = "specified" + review["evidence"] = [] + self.assert_invalid(payload, "requires rfc_anchor evidence") + + def test_unresolved_evidence_kind_is_rejected(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["evidence"].append( + {"kind": "schema", "ref": "does-not-exist.schema.json"} + ) + self.assert_invalid(payload, "authoritative resolver") + + def test_unverifiable_maturity_levels_are_rejected(self) -> None: + payload = self.valid_payload() + review = payload["reviews"][0] + review["status"] = "present" + for maturity in ( + "machine_validated", + "implementation_tested", + "interop_tested", + "stable", + ): + with self.subTest(maturity=maturity): + review["maturity"] = maturity + self.assert_invalid(payload, "authoritative evidence resolvers") + + def test_partial_specified_fixture_is_rejected(self) -> None: + self.assert_invalid( + self.fixture("invalid-partial-specified.json"), "cannot exceed proposal maturity" + ) + + def test_present_coverage_may_remain_a_proposal(self) -> None: + payload = self.valid_payload() + payload["reviews"][0]["status"] = "present" + validate_review_payload(payload, self.heading_ids) + + def test_required_planning_metadata_rejects_legacy_card(self) -> None: + payload = load_review_payload() + payload["planning_metadata_mode"] = "required" + with self.assertRaisesRegex(ValueError, "does not match review-data.schema.json"): + validate_review_payload(payload, self.heading_ids) + + def test_long_reverse_order_dependency_chain_is_valid(self) -> None: + payload = self.valid_payload() + template = payload["reviews"][0] + reviews = [] + for review_id in range(1100, 0, -1): + review = copy.deepcopy(template) + review["id"] = review_id + review["title"] = f"Review {review_id}" + review["depends_on"] = [] if review_id == 1 else [review_id - 1] + reviews.append(review) + payload["reviews"] = reviews + validate_review_payload(payload, self.heading_ids) + + +if __name__ == "__main__": + unittest.main()