diff --git a/README.md b/README.md index ad691f6..1b85b79 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Latest tagged release: [v1.2 — architecture cohesion release](https://github.c - [`docs/v0.6-to-v1-artifact-diff.md`](docs/v0.6-to-v1-artifact-diff.md): fourth-to-fifth-demo artifact contract and compatibility diff - [`docs/evidence-pipeline-contract.md`](docs/evidence-pipeline-contract.md): JSON/JSONL schema contracts for reviewer-facing evidence artifacts - [`docs/schema-compatibility-matrix.md`](docs/schema-compatibility-matrix.md): schema versions, artifact paths, and v1.1-to-v1.2 compatibility notes -- [`docs/reviewer-artifact-diff.md`](docs/reviewer-artifact-diff.md): release artifact diff contract for reviewer-facing outputs +- [`docs/reviewer-artifact-diff.md`](docs/reviewer-artifact-diff.md): release artifact diff contract and executable structured triage report for reviewer-facing outputs - [`docs/vocabulary.md`](docs/vocabulary.md): cross-demo vocabulary for events, hits, signals, bounded correlation, findings, summaries, reports, and audit traces - [`docs/README.md`](docs/README.md): current route, supporting docs, and historical release evidence @@ -106,6 +106,7 @@ Other demo entrypoints: Useful inspection commands: - `telemetry-lab summarize --input data/raw/sample_events.jsonl` +- `python scripts/artifact_contract_diff.py --expected path/to/expected-artifacts --actual path/to/actual-artifacts --json-out artifact-diff.json` For CSV inputs, pass a `.csv` file to `--input`; use `--timestamp-col` when the timestamp column is not named `timestamp`. diff --git a/docs/README.md b/docs/README.md index abde80b..58a222c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -16,7 +16,7 @@ This directory separates the current reviewer route from supporting design notes - [`v0.6-to-v1-artifact-diff.md`](v0.6-to-v1-artifact-diff.md): additive artifact contract and compatibility diff from the fourth demo to the fifth - [`evidence-pipeline-contract.md`](evidence-pipeline-contract.md): JSON/JSONL schema contracts for reviewer-facing evidence artifacts - [`schema-compatibility-matrix.md`](schema-compatibility-matrix.md): schema versions, artifact paths, and compatibility labels -- [`reviewer-artifact-diff.md`](reviewer-artifact-diff.md): release diff contract for reviewer-facing artifact changes +- [`reviewer-artifact-diff.md`](reviewer-artifact-diff.md): release diff contract and executable structured triage report for reviewer-facing artifact changes - [`vocabulary.md`](vocabulary.md): cross-demo vocabulary for evidence workflow terms and bounded correlation - [`architecture.md`](architecture.md): local file-based workflow diagram - [`roadmap.md`](roadmap.md): v1 reviewer contract stabilization phase diff --git a/docs/reviewer-artifact-diff.md b/docs/reviewer-artifact-diff.md index 74bc80e..9011323 100644 --- a/docs/reviewer-artifact-diff.md +++ b/docs/reviewer-artifact-diff.md @@ -10,6 +10,55 @@ the local, file-based artifacts listed in [`docs/reviewer-pack.md`](reviewer-pac and the schema-covered evidence artifacts in [`docs/evidence-pipeline-contract.md`](evidence-pipeline-contract.md). +## Executable Triage Report + +Use the standalone comparator when a regeneration mismatch needs more context: + +```bash +python scripts/artifact_contract_diff.py \ + --expected path/to/committed-artifacts \ + --actual path/to/regenerated-artifacts \ + --json-out artifact-diff.json +``` + +The human summary and strict +[`artifact-contract-diff/v1`](../schemas/artifact_contract_diff.schema.json) +report expose missing, extra, and changed relative paths. JSON and JSONL entries +add record counts, top-level keys, exact schema/version markers, and run-manifest +digest fields when present. CSV, Markdown, text, JSON, and JSONL normalize CRLF +and CR to LF before comparison. Existing binary files are presence-only, so the +tool does not turn renderer-dependent PNG bytes into a reproducibility claim. + +The report is deterministic: it contains no timestamp, artifact body, or +absolute checkout path. Exit status is `0` for no differences, `1` for contract +differences, and `2` for invalid or unreadable input. This tool explains a +mismatch; it does not replace `python scripts/regenerate_artifacts.py --check`, +accept regenerated output, or infer compatibility labels automatically. + +Write `--json-out` outside both compared roots. The CLI rejects an output path +inside either tree and atomically replaces an existing external report only +after the new JSON has been written successfully. + +### Report Semantics + +| Field | Contract | +| --- | --- | +| `status` | `unchanged` requires an empty `differences` array; `changed` requires at least one difference. | +| `summary.*_files` | `expected_files = unchanged_files + missing_files + changed_files + presence_only_files`; the corresponding actual count substitutes `extra_files` for `missing_files`. | +| `unchanged_files` | Counts comparable text/JSON/JSONL files only. It does not include binaries checked for presence. | +| `differences[].status` | `missing` has only an expected snapshot, `extra` has only an actual snapshot, and `changed` has both. | +| `change_reasons` | Missing/extra use their single path reason. Changed comparable artifacts start with `content-changed` and may add structure, schema-version, or run-manifest-digest reasons. | +| `comparison_digest` | SHA-256 of the comparison bytes. Text-like artifacts use strict UTF-8 with CRLF and lone CR normalized to LF. | +| `comparison_size_bytes` | Length of those normalized comparison bytes, not necessarily the on-disk byte size. | +| `structure` | For changed, missing, or extra JSON/JSONL, records the container, record count, union of top-level keys, and validated schema/digest markers when present. | +| `presence_only_paths` | Sorted binary paths present in both trees. Their bytes are intentionally not compared or summarized. | + +The local triage contract is bounded: each root may contain at most 10,000 +files; structured summaries accept at most 64 MiB per changed JSON/JSONL file, +4,096 structural keys or schema markers, and 10,000 entries per run-manifest +digest map. Invalid digest shapes, unsafe embedded paths, symlinks, special +files, or exceeded limits fail closed with exit `2` and no JSON report. + ## Required Release Diff Sections Each release artifact diff must include: diff --git a/schemas/artifact_contract_diff.schema.json b/schemas/artifact_contract_diff.schema.json new file mode 100644 index 0000000..cbe505f --- /dev/null +++ b/schemas/artifact_contract_diff.schema.json @@ -0,0 +1,391 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/stacknil/telemetry-lab/schemas/artifact_contract_diff.schema.json", + "title": "Telemetry lab artifact contract diff v1", + "description": "Deterministic shallow comparison of two local reviewer artifact trees.", + "type": "object", + "additionalProperties": false, + "required": [ + "report_schema_version", + "status", + "summary", + "differences", + "presence_only_paths" + ], + "properties": { + "report_schema_version": { + "const": "artifact-contract-diff/v1" + }, + "status": { + "type": "string", + "enum": [ + "unchanged", + "changed" + ] + }, + "summary": { + "$ref": "#/$defs/summary" + }, + "differences": { + "type": "array", + "maxItems": 20000, + "items": { + "$ref": "#/$defs/difference" + } + }, + "presence_only_paths": { + "type": "array", + "maxItems": 10000, + "uniqueItems": true, + "items": { + "$ref": "#/$defs/relativePath" + } + } + }, + "allOf": [ + { + "if": { + "properties": { + "status": { + "const": "unchanged" + } + } + }, + "then": { + "properties": { + "differences": { + "maxItems": 0 + } + } + } + }, + { + "if": { + "properties": { + "status": { + "const": "changed" + } + } + }, + "then": { + "properties": { + "differences": { + "minItems": 1 + } + } + } + } + ], + "$defs": { + "relativePath": { + "type": "string", + "maxLength": 1024, + "pattern": "^(?![A-Za-z]:)(?!.*\\\\)(?!.*(?:^|/)\\.\\.?(?:/|$))[^/]+(?:/[^/]+)*$" + }, + "digest": { + "type": "string", + "pattern": "^sha256:[0-9a-f]{64}$" + }, + "summary": { + "type": "object", + "additionalProperties": false, + "required": [ + "expected_files", + "actual_files", + "unchanged_files", + "missing_files", + "extra_files", + "changed_files", + "presence_only_files" + ], + "properties": { + "expected_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + }, + "actual_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + }, + "unchanged_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + }, + "missing_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + }, + "extra_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + }, + "changed_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + }, + "presence_only_files": { + "type": "integer", + "minimum": 0, + "maximum": 10000 + } + } + }, + "difference": { + "type": "object", + "additionalProperties": false, + "required": [ + "path", + "status", + "artifact_kind", + "change_reasons" + ], + "properties": { + "path": { + "$ref": "#/$defs/relativePath" + }, + "status": { + "type": "string", + "enum": [ + "missing", + "extra", + "changed" + ] + }, + "artifact_kind": { + "type": "string", + "enum": [ + "json", + "jsonl", + "text", + "binary" + ] + }, + "change_reasons": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "missing-from-actual", + "extra-in-actual", + "content-changed", + "structure-changed", + "schema-version-changed", + "run-manifest-digest-changed" + ] + } + }, + "expected": { + "$ref": "#/$defs/snapshot" + }, + "actual": { + "$ref": "#/$defs/snapshot" + } + }, + "allOf": [ + { + "if": { + "properties": { + "status": { + "const": "missing" + } + } + }, + "then": { + "required": [ + "expected" + ], + "properties": { + "change_reasons": { + "const": [ + "missing-from-actual" + ] + } + }, + "not": { + "required": [ + "actual" + ] + } + } + }, + { + "if": { + "properties": { + "status": { + "const": "extra" + } + } + }, + "then": { + "required": [ + "actual" + ], + "properties": { + "change_reasons": { + "const": [ + "extra-in-actual" + ] + } + }, + "not": { + "required": [ + "expected" + ] + } + } + }, + { + "if": { + "properties": { + "status": { + "const": "changed" + } + } + }, + "then": { + "required": [ + "expected", + "actual" + ], + "properties": { + "artifact_kind": { + "enum": [ + "json", + "jsonl", + "text" + ] + }, + "change_reasons": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "prefixItems": [ + { + "const": "content-changed" + } + ], + "items": { + "type": "string", + "enum": [ + "structure-changed", + "schema-version-changed", + "run-manifest-digest-changed" + ] + } + } + } + } + } + ] + }, + "snapshot": { + "type": "object", + "additionalProperties": false, + "required": [ + "comparison_digest", + "comparison_size_bytes" + ], + "properties": { + "comparison_digest": { + "$ref": "#/$defs/digest" + }, + "comparison_size_bytes": { + "type": "integer", + "minimum": 0 + }, + "structure": { + "$ref": "#/$defs/structure" + } + } + }, + "structure": { + "type": "object", + "additionalProperties": false, + "required": [ + "container", + "record_count", + "top_level_keys" + ], + "properties": { + "container": { + "type": "string", + "enum": [ + "object", + "array", + "jsonl", + "string", + "number", + "boolean", + "null" + ] + }, + "record_count": { + "type": "integer", + "minimum": 0 + }, + "top_level_keys": { + "type": "array", + "maxItems": 4096, + "uniqueItems": true, + "items": { + "type": "string", + "maxLength": 1024 + } + }, + "schema_versions": { + "type": "object", + "maxProperties": 4096, + "additionalProperties": { + "type": "array", + "minItems": 1, + "maxItems": 4096, + "uniqueItems": true, + "items": { + "type": "string", + "maxLength": 1024 + } + } + }, + "run_manifest_digests": { + "$ref": "#/$defs/runManifestDigests" + } + } + }, + "runManifestDigests": { + "type": "object", + "additionalProperties": false, + "properties": { + "input_digest": { + "$ref": "#/$defs/digest" + }, + "config_digest": { + "$ref": "#/$defs/digest" + }, + "input_file_digests": { + "$ref": "#/$defs/fileDigestMap" + }, + "config_file_digests": { + "$ref": "#/$defs/fileDigestMap" + } + } + }, + "fileDigestMap": { + "type": "object", + "maxProperties": 10000, + "propertyNames": { + "$ref": "#/$defs/relativePath" + }, + "additionalProperties": { + "$ref": "#/$defs/digest" + } + } + } +} diff --git a/scripts/artifact_contract_diff.py b/scripts/artifact_contract_diff.py new file mode 100644 index 0000000..638ac9c --- /dev/null +++ b/scripts/artifact_contract_diff.py @@ -0,0 +1,141 @@ +from __future__ import annotations + +import argparse +import json +import os +import sys +import tempfile +from pathlib import Path +from typing import Sequence + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SRC_ROOT = REPO_ROOT / "src" +if str(SRC_ROOT) not in sys.path: + sys.path.insert(0, str(SRC_ROOT)) + +from telemetry_lab.artifact_contract_diff import ( # noqa: E402 + ArtifactContractDiffError, + ArtifactContractDiffReport, + compare_artifact_trees, +) + + +def main(argv: Sequence[str] | None = None) -> int: + args = _build_parser().parse_args(argv) + try: + expected_root = _resolve_path(Path(args.expected), "expected artifact root") + actual_root = _resolve_path(Path(args.actual), "actual artifact root") + json_out = ( + _resolve_path(Path(args.json_out), "JSON report") + if args.json_out + else None + ) + if json_out is not None: + _validate_report_destination(json_out, expected_root, actual_root) + report = compare_artifact_trees( + expected_root, + actual_root, + ) + if json_out is not None: + _write_json_report(report, json_out) + except ArtifactContractDiffError as exc: + print(f"[ERROR] {exc}", file=sys.stderr) + return 2 + + _print_human_summary(report) + return 1 if report.has_differences else 0 + + +def _write_json_report(report: ArtifactContractDiffReport, path: Path) -> None: + temporary_path: Path | None = None + try: + path.parent.mkdir(parents=True, exist_ok=True) + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + newline="\n", + prefix=f".{path.name}.", + suffix=".tmp", + dir=path.parent, + delete=False, + ) as handle: + temporary_path = Path(handle.name) + json.dump(report.to_dict(), handle, indent=2, sort_keys=True) + handle.write("\n") + handle.flush() + os.fsync(handle.fileno()) + os.replace(temporary_path, path) + temporary_path = None + except (OSError, TypeError, ValueError) as exc: + raise ArtifactContractDiffError("cannot write JSON report") from exc + finally: + if temporary_path is not None: + try: + temporary_path.unlink(missing_ok=True) + except OSError: + pass + + +def _resolve_path(path: Path, label: str) -> Path: + try: + return path.resolve(strict=False) + except OSError as exc: + raise ArtifactContractDiffError(f"cannot resolve {label}") from exc + + +def _validate_report_destination( + report_path: Path, + expected_root: Path, + actual_root: Path, +) -> None: + if _is_within(report_path, expected_root) or _is_within(report_path, actual_root): + raise ArtifactContractDiffError( + "JSON report must be outside both artifact roots" + ) + + +def _is_within(path: Path, root: Path) -> bool: + try: + path.relative_to(root) + except ValueError: + return False + return True + + +def _print_human_summary(report: ArtifactContractDiffReport) -> None: + if report.has_differences: + print(f"[DIFF] {len(report.differences)} artifact contract difference(s)") + for difference in report.differences: + reasons = ", ".join(difference.change_reasons) + print( + f"- {difference.path}: {difference.status} " + f"({difference.artifact_kind}; {reasons})" + ) + else: + print( + "[OK] No artifact contract differences " + f"({report.unchanged_files} comparable file(s))" + ) + if report.presence_only_paths: + print( + f"[INFO] {len(report.presence_only_paths)} binary artifact(s) " + "checked for presence only" + ) + + +def _build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + description="Explain shallow contract differences between two artifact trees." + ) + parser.add_argument("--expected", required=True, help="Expected artifact directory") + parser.add_argument("--actual", required=True, help="Actual artifact directory") + parser.add_argument( + "--json-out", + help="Optional path for a deterministic artifact-contract-diff/v1 report", + ) + return parser + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/regenerate_artifacts.py b/scripts/regenerate_artifacts.py index 91a3502..6d30719 100644 --- a/scripts/regenerate_artifacts.py +++ b/scripts/regenerate_artifacts.py @@ -19,6 +19,11 @@ if str(SRC_ROOT) not in sys.path: sys.path.insert(0, str(SRC_ROOT)) +from telemetry_lab.artifact_contract_diff import ( # noqa: E402 + TEXT_ARTIFACT_SUFFIXES, + normalize_artifact_text_bytes, +) + @dataclass(frozen=True) class ArtifactSet: @@ -172,8 +177,10 @@ def compare_artifact_set( def artifacts_match(committed_path: Path, generated_path: Path) -> bool: - if committed_path.suffix.lower() in {".csv", ".json", ".jsonl", ".md", ".txt"}: - return _normalized_text(committed_path) == _normalized_text(generated_path) + if committed_path.suffix.lower() in TEXT_ARTIFACT_SUFFIXES: + return _normalized_text_bytes(committed_path) == _normalized_text_bytes( + generated_path + ) return committed_path.read_bytes() == generated_path.read_bytes() @@ -419,8 +426,8 @@ def _slug(value: str) -> str: return "".join(char if char.isalnum() else "-" for char in value.lower()).strip("-") -def _normalized_text(path: Path) -> str: - return path.read_bytes().decode("utf-8").replace("\r\n", "\n") +def _normalized_text_bytes(path: Path) -> bytes: + return normalize_artifact_text_bytes(path.read_bytes()) @contextmanager diff --git a/src/telemetry_lab/artifact_contract_diff.py b/src/telemetry_lab/artifact_contract_diff.py new file mode 100644 index 0000000..612522c --- /dev/null +++ b/src/telemetry_lab/artifact_contract_diff.py @@ -0,0 +1,748 @@ +from __future__ import annotations + +import codecs +import io +import json +import os +import re +import stat +from collections import Counter +from collections.abc import Iterable, Iterator, Mapping +from contextlib import contextmanager +from copy import deepcopy +from dataclasses import dataclass, replace +from hashlib import sha256 +from pathlib import Path +from typing import Any, BinaryIO, Final, Literal, TypeAlias + + +REPORT_SCHEMA_VERSION: Final = "artifact-contract-diff/v1" +TEXT_ARTIFACT_SUFFIXES: Final = frozenset( + {".csv", ".json", ".jsonl", ".md", ".txt"} +) +RUN_MANIFEST_DIGEST_FIELDS: Final = ( + "input_digest", + "config_digest", + "input_file_digests", + "config_file_digests", +) +READ_CHUNK_SIZE: Final = 64 * 1024 +MAX_ARTIFACT_FILES: Final = 10_000 +MAX_RELATIVE_PATH_LENGTH: Final = 1_024 +MAX_STRUCTURED_ARTIFACT_BYTES: Final = 64 * 1024 * 1024 +MAX_STRUCTURAL_ITEMS: Final = 4_096 +MAX_METADATA_STRING_LENGTH: Final = 1_024 +MAX_DIGEST_ENTRIES: Final = 10_000 + +ArtifactKind: TypeAlias = Literal["json", "jsonl", "text", "binary"] +DifferenceStatus: TypeAlias = Literal["missing", "extra", "changed"] +ChangeReason: TypeAlias = Literal[ + "missing-from-actual", + "extra-in-actual", + "content-changed", + "structure-changed", + "schema-version-changed", + "run-manifest-digest-changed", +] + +_DIGEST_PATTERN: Final = re.compile(r"sha256:[0-9a-f]{64}") +_RELATIVE_PATH_PATTERN: Final = re.compile( + r"^(?![A-Za-z]:)(?!.*\\)(?!.*(?:^|/)\.\.?(?:/|$))[^/]+(?:/[^/]+)*$" +) +_LOCAL_ABSOLUTE_PATH_PATTERN: Final = re.compile(r"^(?:[A-Za-z]:[\\/]|\\\\|/)") +_ALLOWED_CHANGE_REASONS: Final = frozenset( + { + "missing-from-actual", + "extra-in-actual", + "content-changed", + "structure-changed", + "schema-version-changed", + "run-manifest-digest-changed", + } +) + + +class ArtifactContractDiffError(ValueError): + """Raised when artifact trees cannot be compared safely.""" + + +@dataclass(frozen=True) +class ArtifactSnapshot: + comparison_digest: str + comparison_size_bytes: int + structure: Mapping[str, Any] | None = None + + def __post_init__(self) -> None: + if _DIGEST_PATTERN.fullmatch(self.comparison_digest) is None: + raise ArtifactContractDiffError("snapshot comparison digest is invalid") + if self.comparison_size_bytes < 0: + raise ArtifactContractDiffError("snapshot comparison size must be non-negative") + + def to_dict(self) -> dict[str, Any]: + result: dict[str, Any] = { + "comparison_digest": self.comparison_digest, + "comparison_size_bytes": self.comparison_size_bytes, + } + if self.structure is not None: + result["structure"] = deepcopy(dict(self.structure)) + return result + + +@dataclass(frozen=True) +class ArtifactDifference: + path: str + status: DifferenceStatus + artifact_kind: ArtifactKind + change_reasons: tuple[ChangeReason, ...] + expected: ArtifactSnapshot | None = None + actual: ArtifactSnapshot | None = None + + def __post_init__(self) -> None: + if not _is_valid_relative_path(self.path): + raise ArtifactContractDiffError("difference path must be a safe relative path") + if self.artifact_kind not in {"json", "jsonl", "text", "binary"}: + raise ArtifactContractDiffError("difference artifact kind is invalid") + if not self.change_reasons or len(set(self.change_reasons)) != len( + self.change_reasons + ): + raise ArtifactContractDiffError("difference reasons must be non-empty and unique") + if not set(self.change_reasons) <= _ALLOWED_CHANGE_REASONS: + raise ArtifactContractDiffError("difference reason is invalid") + + if self.status == "missing": + valid = ( + self.change_reasons == ("missing-from-actual",) + and self.expected is not None + and self.actual is None + ) + elif self.status == "extra": + valid = ( + self.change_reasons == ("extra-in-actual",) + and self.expected is None + and self.actual is not None + ) + elif self.status == "changed": + valid = ( + self.change_reasons[0] == "content-changed" + and "missing-from-actual" not in self.change_reasons + and "extra-in-actual" not in self.change_reasons + and self.expected is not None + and self.actual is not None + and self.artifact_kind != "binary" + ) + else: + raise ArtifactContractDiffError("difference status is invalid") + if not valid: + raise ArtifactContractDiffError(f"{self.status} difference is inconsistent") + + def to_dict(self) -> dict[str, Any]: + result: dict[str, Any] = { + "path": self.path, + "status": self.status, + "artifact_kind": self.artifact_kind, + "change_reasons": list(self.change_reasons), + } + if self.expected is not None: + result["expected"] = self.expected.to_dict() + if self.actual is not None: + result["actual"] = self.actual.to_dict() + return result + + +@dataclass(frozen=True) +class ArtifactContractDiffReport: + expected_files: int + actual_files: int + unchanged_files: int + missing_files: int + extra_files: int + changed_files: int + differences: tuple[ArtifactDifference, ...] + presence_only_paths: tuple[str, ...] + + def __post_init__(self) -> None: + counts = ( + self.expected_files, + self.actual_files, + self.unchanged_files, + self.missing_files, + self.extra_files, + self.changed_files, + ) + if any(count < 0 for count in counts): + raise ArtifactContractDiffError("report counts must be non-negative") + if self.expected_files > MAX_ARTIFACT_FILES or self.actual_files > MAX_ARTIFACT_FILES: + raise ArtifactContractDiffError("artifact file count exceeds the report limit") + + status_counts = Counter(difference.status for difference in self.differences) + if status_counts != Counter( + { + "missing": self.missing_files, + "extra": self.extra_files, + "changed": self.changed_files, + } + ): + raise ArtifactContractDiffError("report difference counts are inconsistent") + if self.expected_files != ( + self.unchanged_files + + self.missing_files + + self.changed_files + + len(self.presence_only_paths) + ): + raise ArtifactContractDiffError("expected file count is inconsistent") + if self.actual_files != ( + self.unchanged_files + + self.extra_files + + self.changed_files + + len(self.presence_only_paths) + ): + raise ArtifactContractDiffError("actual file count is inconsistent") + + difference_paths = tuple(difference.path for difference in self.differences) + if difference_paths != tuple(sorted(set(difference_paths))): + raise ArtifactContractDiffError( + "difference paths must be unique and sorted" + ) + if self.presence_only_paths != tuple(sorted(set(self.presence_only_paths))): + raise ArtifactContractDiffError( + "presence-only paths must be unique and sorted" + ) + if set(difference_paths) & set(self.presence_only_paths): + raise ArtifactContractDiffError( + "difference and presence-only paths must be disjoint" + ) + + @property + def has_differences(self) -> bool: + return bool(self.differences) + + def to_dict(self) -> dict[str, Any]: + return { + "report_schema_version": REPORT_SCHEMA_VERSION, + "status": "changed" if self.has_differences else "unchanged", + "summary": { + "expected_files": self.expected_files, + "actual_files": self.actual_files, + "unchanged_files": self.unchanged_files, + "missing_files": self.missing_files, + "extra_files": self.extra_files, + "changed_files": self.changed_files, + "presence_only_files": len(self.presence_only_paths), + }, + "differences": [difference.to_dict() for difference in self.differences], + "presence_only_paths": list(self.presence_only_paths), + } + + +def normalize_artifact_text_bytes(content: bytes) -> bytes: + """Normalize artifact newlines after validating strict UTF-8.""" + return b"".join(_iter_normalized_text_chunks((content,))) + + +def compare_artifact_trees( + expected_root: Path, + actual_root: Path, +) -> ArtifactContractDiffReport: + """Compare two artifact roots without leaking their absolute locations.""" + expected_root = _validated_root(expected_root, "expected") + actual_root = _validated_root(actual_root, "actual") + expected = _inventory(expected_root) + actual = _inventory(actual_root) + differences: list[ArtifactDifference] = [] + presence_only: list[str] = [] + unchanged = missing = extra = changed = 0 + + for relative_path in sorted(expected.keys() | actual.keys()): + expected_path = expected.get(relative_path) + actual_path = actual.get(relative_path) + kind = _artifact_kind(relative_path) + if expected_path is None: + extra += 1 + differences.append( + ArtifactDifference( + path=relative_path, + status="extra", + artifact_kind=kind, + change_reasons=("extra-in-actual",), + actual=_snapshot(actual_path, relative_path, kind), + ) + ) + elif actual_path is None: + missing += 1 + differences.append( + ArtifactDifference( + path=relative_path, + status="missing", + artifact_kind=kind, + change_reasons=("missing-from-actual",), + expected=_snapshot(expected_path, relative_path, kind), + ) + ) + elif kind == "binary": + presence_only.append(relative_path) + else: + expected_snapshot = _snapshot_identity(expected_path, relative_path, kind) + actual_snapshot = _snapshot_identity(actual_path, relative_path, kind) + if ( + expected_snapshot.comparison_digest + == actual_snapshot.comparison_digest + and expected_snapshot.comparison_size_bytes + == actual_snapshot.comparison_size_bytes + ): + unchanged += 1 + continue + expected_snapshot = _add_structure( + expected_snapshot, expected_path, relative_path, kind + ) + actual_snapshot = _add_structure( + actual_snapshot, actual_path, relative_path, kind + ) + changed += 1 + differences.append( + ArtifactDifference( + path=relative_path, + status="changed", + artifact_kind=kind, + change_reasons=_change_reasons(expected_snapshot, actual_snapshot), + expected=expected_snapshot, + actual=actual_snapshot, + ) + ) + + return ArtifactContractDiffReport( + expected_files=len(expected), + actual_files=len(actual), + unchanged_files=unchanged, + missing_files=missing, + extra_files=extra, + changed_files=changed, + differences=tuple(differences), + presence_only_paths=tuple(presence_only), + ) + + +def _validated_root(path: Path, label: str) -> Path: + try: + if _is_link_like(path): + raise ArtifactContractDiffError(f"{label} root must not be a symlink") + root_mode = path.lstat().st_mode + if not stat.S_ISDIR(root_mode): + raise ArtifactContractDiffError( + f"{label} root must be an existing directory" + ) + return path.resolve(strict=True) + except ArtifactContractDiffError: + raise + except OSError as exc: + raise ArtifactContractDiffError( + f"{label} root must be an existing directory" + ) from exc + + +def _inventory(root: Path) -> dict[str, Path]: + inventory: dict[str, Path] = {} + for current, dir_names, file_names in os.walk( + root, + followlinks=False, + onerror=_raise_walk_error, + ): + current_path = Path(current) + dir_names.sort() + file_names.sort() + for name in dir_names: + path = current_path / name + if _is_link_like(path): + relative = path.relative_to(root).as_posix() + raise ArtifactContractDiffError( + f"artifact tree contains a directory symlink: {relative}" + ) + for name in file_names: + path = current_path / name + relative = path.relative_to(root).as_posix() + try: + mode = path.lstat().st_mode + except OSError as exc: + raise ArtifactContractDiffError( + f"cannot inspect artifact: {relative}" + ) from exc + if stat.S_ISLNK(mode) or _is_link_like(path): + raise ArtifactContractDiffError( + f"artifact tree contains a file symlink: {relative}" + ) + if not stat.S_ISREG(mode): + raise ArtifactContractDiffError( + f"artifact tree contains a non-regular artifact: {relative}" + ) + if not _is_valid_relative_path(relative): + raise ArtifactContractDiffError( + "artifact tree contains an unsafe relative path" + ) + inventory[relative] = path + if len(inventory) > MAX_ARTIFACT_FILES: + raise ArtifactContractDiffError( + "artifact tree exceeds the report file-count limit" + ) + return inventory + + +def _raise_walk_error(error: OSError) -> None: + raise ArtifactContractDiffError("cannot traverse artifact tree") from error + + +def _is_link_like(path: Path) -> bool: + if path.is_symlink(): + return True + junction_check = getattr(path, "is_junction", None) + return bool(junction_check is not None and junction_check()) + + +def _is_valid_relative_path(value: object) -> bool: + return ( + isinstance(value, str) + and 0 < len(value) <= MAX_RELATIVE_PATH_LENGTH + and not any(ord(character) < 32 for character in value) + and _RELATIVE_PATH_PATTERN.fullmatch(value) is not None + ) + + +def _artifact_kind(relative_path: str) -> ArtifactKind: + suffix = Path(relative_path).suffix.lower() + if suffix == ".json": + return "json" + if suffix == ".jsonl": + return "jsonl" + if suffix in TEXT_ARTIFACT_SUFFIXES: + return "text" + return "binary" + + +def _snapshot( + path: Path | None, relative_path: str, kind: ArtifactKind +) -> ArtifactSnapshot: + snapshot = _snapshot_identity(path, relative_path, kind) + if path is None: + raise ArtifactContractDiffError(f"artifact path is unavailable: {relative_path}") + return _add_structure(snapshot, path, relative_path, kind) + + +def _snapshot_identity( + path: Path | None, relative_path: str, kind: ArtifactKind +) -> ArtifactSnapshot: + if path is None: + raise ArtifactContractDiffError(f"artifact path is unavailable: {relative_path}") + digest = sha256() + size = 0 + try: + with _open_regular_binary(path, relative_path) as handle: + chunks: Iterable[bytes] = iter( + lambda: handle.read(READ_CHUNK_SIZE), + b"", + ) + if kind != "binary": + chunks = _iter_normalized_text_chunks(chunks) + for chunk in chunks: + digest.update(chunk) + size += len(chunk) + except UnicodeDecodeError as exc: + raise ArtifactContractDiffError( + f"text artifact is not valid UTF-8: {relative_path}" + ) from exc + except OSError as exc: + raise ArtifactContractDiffError(f"cannot read artifact: {relative_path}") from exc + return ArtifactSnapshot( + comparison_digest="sha256:" + digest.hexdigest(), + comparison_size_bytes=size, + ) + + +@contextmanager +def _open_regular_binary(path: Path, relative_path: str) -> Iterator[BinaryIO]: + flags = ( + os.O_RDONLY + | getattr(os, "O_BINARY", 0) + | getattr(os, "O_NOFOLLOW", 0) + | getattr(os, "O_NONBLOCK", 0) + ) + descriptor: int | None = None + try: + descriptor = os.open(path, flags) + if not stat.S_ISREG(os.fstat(descriptor).st_mode): + raise ArtifactContractDiffError( + f"artifact is not a regular file: {relative_path}" + ) + with os.fdopen(descriptor, "rb") as handle: + descriptor = None + yield handle + except ArtifactContractDiffError: + raise + except OSError as exc: + raise ArtifactContractDiffError( + f"cannot open artifact safely: {relative_path}" + ) from exc + finally: + if descriptor is not None: + os.close(descriptor) + + +def _iter_normalized_text_chunks(chunks: Iterable[bytes]) -> Iterator[bytes]: + decoder = codecs.getincrementaldecoder("utf-8")("strict") + pending_carriage_return = False + for chunk in chunks: + decoder.decode(chunk, final=False) + if pending_carriage_return: + chunk = b"\r" + chunk + pending_carriage_return = False + if chunk.endswith(b"\r"): + chunk = chunk[:-1] + pending_carriage_return = True + normalized = chunk.replace(b"\r\n", b"\n").replace(b"\r", b"\n") + if normalized: + yield normalized + decoder.decode(b"", final=True) + if pending_carriage_return: + yield b"\n" + + +def _add_structure( + snapshot: ArtifactSnapshot, + path: Path, + relative_path: str, + kind: ArtifactKind, +) -> ArtifactSnapshot: + if kind not in {"json", "jsonl"}: + return snapshot + if snapshot.comparison_size_bytes > MAX_STRUCTURED_ARTIFACT_BYTES: + raise ArtifactContractDiffError( + f"structured artifact exceeds the {MAX_STRUCTURED_ARTIFACT_BYTES}-byte " + f"summary limit: {relative_path}" + ) + structure = _structured_summary(path, relative_path, kind) + return replace(snapshot, structure=structure) + + +def _structured_summary( + path: Path, relative_path: str, kind: ArtifactKind +) -> dict[str, Any]: + if kind == "jsonl": + return _jsonl_summary(path, relative_path) + try: + with _open_regular_binary(path, relative_path) as handle: + content = normalize_artifact_text_bytes(handle.read()) + except UnicodeDecodeError as exc: + raise ArtifactContractDiffError( + f"text artifact is not valid UTF-8: {relative_path}" + ) from exc + except OSError as exc: + raise ArtifactContractDiffError(f"cannot read artifact: {relative_path}") from exc + try: + value = json.loads(content.decode("utf-8")) + except json.JSONDecodeError as exc: + raise ArtifactContractDiffError( + f"invalid json artifact at {relative_path}:{exc.lineno}:{exc.colno}" + ) from exc + + records = value if isinstance(value, list) else [value] + return _summarize_records( + records, + container=_json_container(value), + relative_path=relative_path, + run_manifest=value if isinstance(value, Mapping) else None, + ) + + +def _jsonl_summary(path: Path, relative_path: str) -> dict[str, Any]: + def records() -> Iterator[object]: + try: + with _open_regular_binary(path, relative_path) as raw_handle: + with io.TextIOWrapper( + raw_handle, + encoding="utf-8", + errors="strict", + newline=None, + ) as text_handle: + for line_number, line in enumerate(text_handle, start=1): + if not line.strip(): + continue + try: + yield json.loads(line) + except json.JSONDecodeError as exc: + raise ArtifactContractDiffError( + f"invalid jsonl artifact at " + f"{relative_path}:{line_number}:{exc.colno}" + ) from exc + except UnicodeDecodeError as exc: + raise ArtifactContractDiffError( + f"text artifact is not valid UTF-8: {relative_path}" + ) from exc + except OSError as exc: + raise ArtifactContractDiffError( + f"cannot read artifact: {relative_path}" + ) from exc + + return _summarize_records( + records(), + container="jsonl", + relative_path=relative_path, + run_manifest=None, + ) + + +def _summarize_records( + records: Iterable[object], + *, + container: str, + relative_path: str, + run_manifest: Mapping[str, Any] | None, +) -> dict[str, Any]: + record_count = 0 + top_level_keys: set[str] = set() + schema_markers: dict[str, set[str]] = {} + for record in records: + record_count += 1 + if not isinstance(record, Mapping): + continue + for key in record: + if not _is_safe_metadata_string(key): + raise ArtifactContractDiffError( + f"structured artifact has an unsafe top-level key: {relative_path}" + ) + top_level_keys.add(key) + if len(top_level_keys) > MAX_STRUCTURAL_ITEMS: + raise ArtifactContractDiffError( + f"structured artifact has too many top-level keys: {relative_path}" + ) + _collect_schema_versions(record, schema_markers, relative_path) + + summary: dict[str, Any] = { + "container": container, + "record_count": record_count, + "top_level_keys": sorted(top_level_keys), + } + if schema_markers: + summary["schema_versions"] = { + field: sorted(values) for field, values in sorted(schema_markers.items()) + } + if run_manifest is not None: + digests = _validated_run_manifest_digests(run_manifest, relative_path) + if digests: + summary["run_manifest_digests"] = digests + return summary + + +def _collect_schema_versions( + record: Mapping[str, Any], + markers: dict[str, set[str]], + relative_path: str, +) -> None: + for field in ("$id", "$schema", "schema_id", "schema_version"): + value = record.get(field) + if isinstance(value, str): + _add_schema_marker(markers, field, value, relative_path) + artifact_versions = record.get("artifact_schema_versions") + if isinstance(artifact_versions, Mapping): + for field, value in artifact_versions.items(): + if isinstance(field, str) and isinstance(value, str): + _add_schema_marker( + markers, + f"artifact_schema_versions.{field}", + value, + relative_path, + ) + + +def _add_schema_marker( + markers: dict[str, set[str]], + field: str, + value: str, + relative_path: str, +) -> None: + if not _is_safe_metadata_string(field) or not _is_safe_metadata_string(value): + raise ArtifactContractDiffError( + f"structured artifact schema marker is unsafe: {relative_path}" + ) + markers.setdefault(field, set()).add(value) + if sum(len(values) for values in markers.values()) > MAX_STRUCTURAL_ITEMS: + raise ArtifactContractDiffError( + f"structured artifact has too many schema markers: {relative_path}" + ) + + +def _is_safe_metadata_string(value: object) -> bool: + return ( + isinstance(value, str) + and len(value) <= MAX_METADATA_STRING_LENGTH + and not any(ord(character) < 32 for character in value) + and _LOCAL_ABSOLUTE_PATH_PATTERN.match(value) is None + and not value.lower().startswith("file:") + ) + + +def _validated_run_manifest_digests( + value: Mapping[str, Any], relative_path: str +) -> dict[str, Any]: + digests: dict[str, Any] = {} + for field in RUN_MANIFEST_DIGEST_FIELDS: + if field not in value: + continue + candidate = value[field] + if field in {"input_digest", "config_digest"}: + if not isinstance(candidate, str) or _DIGEST_PATTERN.fullmatch(candidate) is None: + raise ArtifactContractDiffError( + f"invalid run-manifest digest field in {relative_path}" + ) + digests[field] = candidate + continue + if not isinstance(candidate, Mapping) or len(candidate) > MAX_DIGEST_ENTRIES: + raise ArtifactContractDiffError( + f"invalid run-manifest digest field in {relative_path}" + ) + validated_map: dict[str, str] = {} + for digest_path, digest_value in candidate.items(): + if ( + not _is_valid_relative_path(digest_path) + or not isinstance(digest_value, str) + or _DIGEST_PATTERN.fullmatch(digest_value) is None + ): + raise ArtifactContractDiffError( + f"invalid run-manifest digest field in {relative_path}" + ) + validated_map[digest_path] = digest_value + digests[field] = dict(sorted(validated_map.items())) + return digests + + +def _json_container(value: object) -> str: + if isinstance(value, Mapping): + return "object" + if isinstance(value, list): + return "array" + if value is None: + return "null" + if isinstance(value, bool): + return "boolean" + if isinstance(value, (int, float)): + return "number" + return "string" + + +def _change_reasons( + expected: ArtifactSnapshot, actual: ArtifactSnapshot +) -> tuple[ChangeReason, ...]: + reasons: list[ChangeReason] = ["content-changed"] + expected_structure = expected.structure or {} + actual_structure = actual.structure or {} + structural_fields = ("container", "record_count", "top_level_keys") + if any( + expected_structure.get(field) != actual_structure.get(field) + for field in structural_fields + ): + reasons.append("structure-changed") + if expected_structure.get("schema_versions") != actual_structure.get( + "schema_versions" + ): + reasons.append("schema-version-changed") + if expected_structure.get("run_manifest_digests") != actual_structure.get( + "run_manifest_digests" + ): + reasons.append("run-manifest-digest-changed") + return tuple(reasons) diff --git a/tests/test_artifact_contract_diff.py b/tests/test_artifact_contract_diff.py new file mode 100644 index 0000000..4372401 --- /dev/null +++ b/tests/test_artifact_contract_diff.py @@ -0,0 +1,467 @@ +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest +from jsonschema import Draft202012Validator + +from telemetry_lab import artifact_contract_diff as artifact_diff +from telemetry_lab.artifact_contract_diff import ( + ArtifactContractDiffError, + ArtifactDifference, + ArtifactSnapshot, + compare_artifact_trees, +) + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SCRIPT_PATH = REPO_ROOT / "scripts" / "artifact_contract_diff.py" +SCHEMA_PATH = REPO_ROOT / "schemas" / "artifact_contract_diff.schema.json" + + +@pytest.fixture +def artifact_roots(tmp_path: Path) -> tuple[Path, Path]: + expected = tmp_path / "expected" + actual = tmp_path / "actual" + expected.mkdir() + actual.mkdir() + return expected, actual + + +def _write_json(path: Path, value: object, *, newline: str = "\n") -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes((json.dumps(value, sort_keys=True) + newline).encode("utf-8")) + + +def _load_cli_script(): + spec = importlib.util.spec_from_file_location( + "artifact_contract_diff_cli", + SCRIPT_PATH, + ) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def test_compare_artifact_trees_normalizes_text_and_keeps_binary_presence_only( + artifact_roots: tuple[Path, Path], +) -> None: + expected, actual = artifact_roots + (expected / "report.md").write_bytes(b"# Report\r\n\r\nunchanged\r") + (actual / "report.md").write_bytes(b"# Report\n\nunchanged\n") + _write_json(expected / "summary.json", {"count": 1}, newline="\r\n") + _write_json(actual / "summary.json", {"count": 1}) + (expected / "plot.png").write_bytes(b"expected-renderer-bytes") + (actual / "plot.png").write_bytes(b"actual-renderer-bytes") + + report = compare_artifact_trees(expected, actual).to_dict() + + assert report == { + "report_schema_version": "artifact-contract-diff/v1", + "status": "unchanged", + "summary": { + "expected_files": 3, + "actual_files": 3, + "unchanged_files": 2, + "missing_files": 0, + "extra_files": 0, + "changed_files": 0, + "presence_only_files": 1, + }, + "differences": [], + "presence_only_paths": ["plot.png"], + } + + +def test_compare_artifact_trees_does_not_parse_identical_json( + artifact_roots: tuple[Path, Path], monkeypatch: pytest.MonkeyPatch +) -> None: + expected, actual = artifact_roots + _write_json(expected / "summary.json", {"count": 1}) + _write_json(actual / "summary.json", {"count": 1}) + + def fail_if_parsed(_text: str) -> object: + raise AssertionError("identical JSON should be accepted from its normalized digest") + + monkeypatch.setattr(artifact_diff.json, "loads", fail_if_parsed) + + assert compare_artifact_trees(expected, actual).has_differences is False + + +def test_compare_artifact_trees_reports_missing_and_extra_paths_in_stable_order( + artifact_roots: tuple[Path, Path], +) -> None: + expected, actual = artifact_roots + _write_json(expected / "z-common.json", {"status": "same"}) + _write_json(actual / "z-common.json", {"status": "same"}) + _write_json(expected / "nested" / "a-missing.json", {"status": "old"}) + _write_json(actual / "b-extra.json", {"status": "new"}) + + report = compare_artifact_trees(expected, actual).to_dict() + + assert [item["path"] for item in report["differences"]] == [ + "b-extra.json", + "nested/a-missing.json", + ] + assert [item["status"] for item in report["differences"]] == [ + "extra", + "missing", + ] + assert report["differences"][0]["change_reasons"] == ["extra-in-actual"] + assert report["differences"][1]["change_reasons"] == [ + "missing-from-actual" + ] + assert report["summary"] == { + "expected_files": 2, + "actual_files": 2, + "unchanged_files": 1, + "missing_files": 1, + "extra_files": 1, + "changed_files": 0, + "presence_only_files": 0, + } + + +@pytest.mark.parametrize( + ("name", "expected_bytes", "actual_bytes", "expected_count", "actual_count"), + [ + ( + "records.json", + b'[{"id": 1, "status": "old"}]\n', + b'[{"id": 1, "status": "old"}, {"id": 2, "score": 7}]\n', + 1, + 2, + ), + ( + "records.jsonl", + b'{"id": 1, "status": "old"}\n', + b'{"id": 1, "status": "old"}\n{"id": 2, "score": 7}\n', + 1, + 2, + ), + ], +) +def test_compare_artifact_trees_summarizes_changed_json_records( + artifact_roots: tuple[Path, Path], + name: str, + expected_bytes: bytes, + actual_bytes: bytes, + expected_count: int, + actual_count: int, +) -> None: + expected, actual = artifact_roots + (expected / name).write_bytes(expected_bytes) + (actual / name).write_bytes(actual_bytes) + + difference = compare_artifact_trees(expected, actual).to_dict()["differences"][0] + + assert difference["change_reasons"] == ["content-changed", "structure-changed"] + assert difference["expected"]["structure"]["record_count"] == expected_count + assert difference["actual"]["structure"]["record_count"] == actual_count + assert difference["expected"]["structure"]["top_level_keys"] == [ + "id", + "status", + ] + assert difference["actual"]["structure"]["top_level_keys"] == [ + "id", + "score", + "status", + ] + + +def test_compare_artifact_trees_exposes_exact_schema_version_change( + artifact_roots: tuple[Path, Path], +) -> None: + expected, actual = artifact_roots + _write_json( + expected / "summary.json", + { + "schema_version": "summary/v1", + "artifact_schema_versions": {"run_manifest": "run-manifest/v1"}, + }, + ) + _write_json( + actual / "summary.json", + { + "schema_version": "summary/v2", + "artifact_schema_versions": {"run_manifest": "run-manifest/v2"}, + }, + ) + + difference = compare_artifact_trees(expected, actual).to_dict()["differences"][0] + + assert difference["change_reasons"] == [ + "content-changed", + "schema-version-changed", + ] + assert difference["expected"]["structure"]["schema_versions"] == { + "artifact_schema_versions.run_manifest": ["run-manifest/v1"], + "schema_version": ["summary/v1"], + } + assert difference["actual"]["structure"]["schema_versions"] == { + "artifact_schema_versions.run_manifest": ["run-manifest/v2"], + "schema_version": ["summary/v2"], + } + + +def test_compare_artifact_trees_exposes_run_manifest_digest_change( + artifact_roots: tuple[Path, Path], +) -> None: + expected, actual = artifact_roots + old_digest = "sha256:" + "0" * 64 + new_digest = "sha256:" + "1" * 64 + base = { + "input_digest": old_digest, + "config_digest": old_digest, + "input_file_digests": {"data/input.jsonl": old_digest}, + "config_file_digests": {"configs/default.yaml": old_digest}, + } + changed = dict(base, config_digest=new_digest) + _write_json(expected / "run_manifest.json", base) + _write_json(actual / "run_manifest.json", changed) + + difference = compare_artifact_trees(expected, actual).to_dict()["differences"][0] + + assert difference["change_reasons"] == [ + "content-changed", + "run-manifest-digest-changed", + ] + assert difference["expected"]["structure"]["run_manifest_digests"] == base + assert difference["actual"]["structure"]["run_manifest_digests"] == changed + + +@pytest.mark.parametrize( + "invalid_digests", + [ + {"input_digest": "not-a-sha256"}, + {"input_file_digests": ["not", "a", "mapping"]}, + {"config_file_digests": {"../outside.yaml": "sha256:" + "0" * 64}}, + {"input_file_digests": {"C:/Users/example/input.jsonl": "sha256:" + "0" * 64}}, + ], +) +def test_artifact_contract_diff_cli_rejects_invalid_run_manifest_digest_fields( + artifact_roots: tuple[Path, Path], + tmp_path: Path, + invalid_digests: object, +) -> None: + expected, actual = artifact_roots + _write_json(expected / "run_manifest.json", {"input_digest": "sha256:" + "0" * 64}) + _write_json(actual / "run_manifest.json", invalid_digests) + report_path = tmp_path / "artifact-diff.json" + + result = subprocess.run( + [ + sys.executable, + str(SCRIPT_PATH), + "--expected", + str(expected), + "--actual", + str(actual), + "--json-out", + str(report_path), + ], + cwd=REPO_ROOT, + text=True, + capture_output=True, + ) + + assert result.returncode == 2 + assert "[ERROR] invalid run-manifest digest field" in result.stderr + assert not report_path.exists() + + +@pytest.mark.parametrize("root_name", ["expected", "actual"]) +def test_artifact_contract_diff_cli_refuses_output_inside_an_input_tree( + artifact_roots: tuple[Path, Path], root_name: str +) -> None: + expected, actual = artifact_roots + _write_json(expected / "summary.json", {"count": 1}) + _write_json(actual / "summary.json", {"count": 1}) + root = expected if root_name == "expected" else actual + report_path = root / "reports" / "artifact-diff.json" + report_path.parent.mkdir() + original = b'{"keep":"artifact"}\n' + report_path.write_bytes(original) + + result = subprocess.run( + [ + sys.executable, + str(SCRIPT_PATH), + "--expected", + str(expected), + "--actual", + str(actual), + "--json-out", + str(report_path), + ], + cwd=REPO_ROOT, + text=True, + capture_output=True, + ) + + assert result.returncode == 2 + assert "JSON report must be outside both artifact roots" in result.stderr + assert report_path.read_bytes() == original + + +def test_json_report_write_is_atomic_on_replace_failure( + artifact_roots: tuple[Path, Path], + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + expected, actual = artifact_roots + _write_json(expected / "summary.json", {"count": 1}) + _write_json(actual / "summary.json", {"count": 2}) + report = compare_artifact_trees(expected, actual) + report_path = tmp_path / "artifact-diff.json" + original = b'{"keep":"previous-report"}\n' + report_path.write_bytes(original) + cli = _load_cli_script() + + def fail_replace(_source: object, _destination: object) -> None: + raise OSError("simulated replace failure") + + monkeypatch.setattr(cli.os, "replace", fail_replace) + + with pytest.raises(ArtifactContractDiffError, match="cannot write JSON report"): + cli._write_json_report(report, report_path) + + assert report_path.read_bytes() == original + assert list(tmp_path.glob(".artifact-diff.json.*.tmp")) == [] + + +def test_report_schema_rejects_cross_field_status_contradictions( + artifact_roots: tuple[Path, Path] +) -> None: + expected, actual = artifact_roots + _write_json(expected / "missing.json", {"status": "old"}) + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + validator = Draft202012Validator(schema) + report = compare_artifact_trees(expected, actual).to_dict() + + invalid_reports = [dict(report, status="unchanged")] + wrong_missing_reason = json.loads(json.dumps(report)) + wrong_missing_reason["differences"][0]["change_reasons"] = ["content-changed"] + invalid_reports.append(wrong_missing_reason) + + _write_json(actual / "missing.json", {"status": "new"}) + changed_report = compare_artifact_trees(expected, actual).to_dict() + invalid_reports.append(dict(changed_report, differences=[])) + wrong_changed_reason = json.loads(json.dumps(changed_report)) + wrong_changed_reason["differences"][0]["change_reasons"] = [ + "missing-from-actual" + ] + invalid_reports.append(wrong_changed_reason) + changed_binary = json.loads(json.dumps(changed_report)) + changed_binary["differences"][0]["artifact_kind"] = "binary" + invalid_reports.append(changed_binary) + + for invalid_report in invalid_reports: + assert list(validator.iter_errors(invalid_report)) + + +def test_artifact_difference_rejects_invalid_status_reason_pair() -> None: + snapshot = ArtifactSnapshot( + comparison_digest="sha256:" + "0" * 64, + comparison_size_bytes=1, + ) + + with pytest.raises(ArtifactContractDiffError, match="missing difference"): + ArtifactDifference( + path="artifact.json", + status="missing", + artifact_kind="json", + change_reasons=("content-changed",), + expected=snapshot, + ) + + +@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires POSIX mkfifo") +def test_compare_artifact_trees_rejects_special_files( + artifact_roots: tuple[Path, Path] +) -> None: + expected, actual = artifact_roots + os.mkfifo(expected / "blocked.json") + + with pytest.raises(ArtifactContractDiffError, match="non-regular artifact"): + compare_artifact_trees(expected, actual) + + +def test_compare_artifact_trees_bounds_changed_structured_artifacts( + artifact_roots: tuple[Path, Path], monkeypatch: pytest.MonkeyPatch +) -> None: + expected, actual = artifact_roots + _write_json(expected / "summary.json", {"value": "old"}) + _write_json(actual / "summary.json", {"value": "new"}) + monkeypatch.setattr(artifact_diff, "MAX_STRUCTURED_ARTIFACT_BYTES", 8) + + with pytest.raises(ArtifactContractDiffError, match="summary limit"): + compare_artifact_trees(expected, actual) + + +def test_artifact_contract_diff_cli_writes_deterministic_schema_valid_report( + artifact_roots: tuple[Path, Path], tmp_path: Path +) -> None: + expected, actual = artifact_roots + _write_json(expected / "summary.json", [{"id": 1}]) + _write_json(actual / "summary.json", [{"id": 1}, {"id": 2}]) + report_path = tmp_path / "artifact-diff.json" + command = [ + sys.executable, + str(SCRIPT_PATH), + "--expected", + str(expected), + "--actual", + str(actual), + "--json-out", + str(report_path), + ] + + first = subprocess.run(command, cwd=REPO_ROOT, text=True, capture_output=True) + first_bytes = report_path.read_bytes() + second = subprocess.run(command, cwd=REPO_ROOT, text=True, capture_output=True) + report = json.loads(report_path.read_text(encoding="utf-8")) + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + + assert first.returncode == second.returncode == 1 + assert first.stdout == second.stdout + assert "[DIFF] 1 artifact contract difference(s)" in first.stdout + assert "summary.json" in first.stdout + assert str(expected) not in first.stdout + assert str(actual) not in first.stdout + assert first_bytes == report_path.read_bytes() + Draft202012Validator.check_schema(schema) + assert list(Draft202012Validator(schema).iter_errors(report)) == [] + + +def test_artifact_contract_diff_cli_returns_zero_for_unchanged_trees( + artifact_roots: tuple[Path, Path] +) -> None: + expected, actual = artifact_roots + _write_json(expected / "summary.json", {"count": 1}) + _write_json(actual / "summary.json", {"count": 1}) + + result = subprocess.run( + [ + sys.executable, + str(SCRIPT_PATH), + "--expected", + str(expected), + "--actual", + str(actual), + ], + cwd=REPO_ROOT, + text=True, + capture_output=True, + ) + + assert result.returncode == 0 + assert "[OK] No artifact contract differences" in result.stdout diff --git a/tests/test_artifact_regeneration_check.py b/tests/test_artifact_regeneration_check.py index 8e29676..a7723d5 100644 --- a/tests/test_artifact_regeneration_check.py +++ b/tests/test_artifact_regeneration_check.py @@ -6,6 +6,9 @@ import sys from pathlib import Path +import pytest + +from telemetry_lab.artifact_contract_diff import compare_artifact_trees from telemetry_lab.manifest import digest_file_bytes, digest_files @@ -117,3 +120,21 @@ def test_regenerate_artifacts_reports_mismatched_strict_artifact(tmp_path) -> No assert len(differences) == 1 assert differences[0].reason == "content differs" + + +@pytest.mark.parametrize("committed_newline", [b"\n", b"\r\n", b"\r"]) +def test_regeneration_and_triage_share_text_normalization( + tmp_path: Path, committed_newline: bytes +) -> None: + script = _load_regeneration_script() + committed_root = tmp_path / "committed" + generated_root = tmp_path / "generated" + committed_root.mkdir() + generated_root.mkdir() + committed_path = committed_root / "artifact.json" + generated_path = generated_root / "artifact.json" + committed_path.write_bytes(b'{"status":"same"}' + committed_newline) + generated_path.write_bytes(b'{"status":"same"}\n') + + assert script.artifacts_match(committed_path, generated_path) + assert not compare_artifact_trees(committed_root, generated_root).has_differences