From 1088fd9458b63ecb10af27225524584b09af971a Mon Sep 17 00:00:00 2001 From: aurascoper Date: Mon, 27 Jul 2026 23:33:11 -0500 Subject: [PATCH] docs/ci: repair ADR numbering and enforce reference integrity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CONTRIBUTING.md` makes `docs/architecture/decision-log/` the canonical location for architectural decisions, so a reference resolving to no file — or to two — is a provenance defect rather than a typo. ADR-004 has been double-assigned for a year. `ROADMAP.md` recorded it ("worth a rename/renumber pass"), the 2026-07-19 review re-found it as finding M6, and neither caused the fix. Twenty-two references across thirteen files cited a number owned by two accepted documents. It survived because the codebase had already routed around it: the embedding ADR states that reviewers should cite it by section ("can be cited as `ADR-004 §3.5`"), so twelve citations carried a §-marker that disambiguated what the number could not. Dispositions, all twenty-two resolved individually — none ambiguous: 14 → ADR-010 8 §-marked code/test citations, the ADR's own heading and 3 self-citation examples, embedding_contract.md, README.md 3 stay the privacy ADR's heading, and ADR-005 / ADR-008, which already cite it by full filename 3 rewritten ROADMAP.md, which documented the defect and is now obsolete 2 preserved docs/reviews/, historical findings left as written The review's "renumber to ADR-009+" recommendation is deliberately not honoured: 009 is claimed by twenty-two live citations in the generation-runtime layer, so the embedding contract takes 010. A numeric gap is legal — ADR numbers are identifiers, not a contiguity guarantee — and the checker asserts that. The gate ships with the repair, not after it, because it would otherwise red-light the branch introducing it. Two modes: `hard` fails on normative roots (Sources/, Tests/, Scripts/, docs/architecture/); `advisory` reports historical prose without failing. Without that split, excluding `docs/reviews/` and requiring failure because of its ADR-009+ recommendation is a contradiction. hard / unmodified FAIL 1 error (duplicate), and only that hard / repaired PASS 0 warnings advisory / unmodified PASS 2 warnings all / unmodified FAIL 1 error + 1 warning all / repaired PASS 1 warning (historical, preserved) python3 Tests/eval/test_adr_references.py 16 tests, OK python3 Scripts/check_adr_references.py PASS swift build OK deterministic suites 287 executed, 3 skipped, 0 failures git diff --check clean Two traps found by executing the repair rather than reasoning about it, both now pinned as tests: a §-marked sed pass leaves a markdown link naming the old *filename*; and a resolution note announcing "ADR-009 is reserved…" fails the very gate it announces, since that ADR does not exist yet. The ROADMAP wording names the reserved number without emitting a canonical token. Stdlib-only, run as a direct `python3` step in the existing macOS job: `main` has no Python job, so a `Tests/eval/*.py` file would otherwise execute nowhere. Tests run before the checker — a gate whose only evidence is that it never complained is not a gate. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 15 + README.md | 2 +- Scripts/check_adr_references.py | 633 ++++++++++++++++++ Sources/BCILLM/MLXSentenceEmbedder.swift | 4 +- Sources/EmbeddingBench/BenchmarkRunner.swift | 2 +- Sources/EmbeddingBench/main.swift | 2 +- .../SemanticBGEReplayRegressionTests.swift | 2 +- .../MLXSentenceEmbedderTests.swift | 6 +- Tests/eval/test_adr_references.py | 234 +++++++ docs/architecture/ROADMAP.md | 13 +- ...010-sentence-embedder-backend-contract.md} | 8 +- docs/architecture/embedding_contract.md | 2 +- 12 files changed, 903 insertions(+), 20 deletions(-) create mode 100755 Scripts/check_adr_references.py create mode 100644 Tests/eval/test_adr_references.py rename docs/architecture/decision-log/{ADR-004-sentence-embedder-backend-contract.md => ADR-010-sentence-embedder-backend-contract.md} (97%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9cfd75..6fbda04 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,21 @@ jobs: test -f Tests/BCIEEGTests/Fixtures/reference_pipeline.json \ || { echo "::error::Golden fixture missing — GoldenRecordingRegressionTests would skip vacuously"; exit 1; } + # CONTRIBUTING.md makes docs/architecture/decision-log/ the canonical + # location for architectural decisions, so a reference that resolves to no + # file — or to two — is a provenance defect, not a typo. ADR-004 was + # double-assigned for a year: the roadmap recorded it, a code review + # re-found it, and neither caused the fix. + # + # A direct python3 step rather than a job: the checker is stdlib-only and + # macos-26 ships python3, so this needs no setup-python and no new runner. + # Tests run first — a gate whose only evidence is that it never complained + # is not a gate. + - name: ADR reference integrity + run: | + python3 Tests/eval/test_adr_references.py + python3 Scripts/check_adr_references.py --mode all + # Compiles every target (incl. BCILLM's MLX Metal kernels via the runner's # full Xcode) — the primary gate that the whole graph builds. - name: Build (all targets) diff --git a/README.md b/README.md index 7a2bdf4..ed78372 100644 --- a/README.md +++ b/README.md @@ -578,7 +578,7 @@ toolkit, architectural spec, and codebase are useful on their own. - [`HARDWARE_SETUP.md`](HARDWARE_SETUP.md) · [`MODEL_SETUP.md`](MODEL_SETUP.md) · [`CALIBRATION.md`](CALIBRATION.md) · [`TROUBLESHOOTING.md`](TROUBLESHOOTING.md) - [`SLEEP_CYCLE_DESIGN.md`](SLEEP_CYCLE_DESIGN.md) — full D1–D8 sleep architecture spec. - [`docs/Architecture.md`](docs/Architecture.md) · [`docs/Math.md`](docs/Math.md) · [`docs/Validation.md`](docs/Validation.md) · [`docs/Research.md`](docs/Research.md) -- [`docs/architecture/embedding_contract.md`](docs/architecture/embedding_contract.md) — the `SentenceEmbedder` backend contract every conformer (stub, Core ML, future MLX) must satisfy; ratified by [ADR-004](docs/architecture/decision-log/ADR-004-sentence-embedder-backend-contract.md). +- [`docs/architecture/embedding_contract.md`](docs/architecture/embedding_contract.md) — the `SentenceEmbedder` backend contract every conformer (stub, Core ML, future MLX) must satisfy; ratified by [ADR-010](docs/architecture/decision-log/ADR-010-sentence-embedder-backend-contract.md). ## Citation diff --git a/Scripts/check_adr_references.py b/Scripts/check_adr_references.py new file mode 100755 index 0000000..1113dff --- /dev/null +++ b/Scripts/check_adr_references.py @@ -0,0 +1,633 @@ +#!/usr/bin/env python3 +"""Validate NeuralCompose's ADR registry and ADR references. + +Hard mode validates: + +1. Every ADR file has a canonical filename: + ADR-NNN-descriptive-slug.md +2. Every ADR file has one matching top-level heading: + # ADR-NNN: Title +3. No ADR number is assigned to more than one file. +4. Every well-formed ADR reference in normative repository roots resolves + to exactly one ADR file. +5. Full-filename references name an ADR file that actually exists. +6. Malformed numeric ADR references fail. + +Numeric gaps are allowed. For example, ADR-010 may exist while ADR-009 is +reserved for work that has not landed yet. + +Historical review documents are not normative. Advisory mode scans them and +reports findings without failing. + +Usage: + python3 Scripts/check_adr_references.py + python3 Scripts/check_adr_references.py --mode hard + python3 Scripts/check_adr_references.py --mode advisory + python3 Scripts/check_adr_references.py --mode all + python3 Scripts/check_adr_references.py --root /path/to/worktree +""" + +from __future__ import annotations + +import argparse +import dataclasses +import os +import re +import sys +from collections import defaultdict +from pathlib import Path +from typing import DefaultDict, Dict, Iterable, List, Optional, Sequence, Set, Tuple + + +ADR_DIRECTORY = Path("docs/architecture/decision-log") + +HARD_ROOTS: Tuple[Path, ...] = ( + Path("Sources"), + Path("Tests"), + Path("Scripts"), + Path("docs/architecture"), +) + +ADVISORY_ROOTS: Tuple[Path, ...] = ( + Path("README.md"), + Path("docs/reviews"), + Path("Evaluation/reports"), +) + +# These files necessarily contain synthetic malformed references and checker +# terminology. They test the policy; they are not normative ADR consumers. +HARD_SCAN_EXCLUSIONS: Set[Path] = { + Path("Scripts/check_adr_references.py"), + Path("Tests/eval/test_adr_references.py"), +} + +ADR_FILENAME_RE = re.compile( + r"^ADR-(?P\d{3})-[A-Za-z0-9][A-Za-z0-9._-]*\.md$" +) + +ADR_HEADING_RE = re.compile( + r"^#\s+ADR-(?P\d{3}):(?:\s|$)" +) + +# Used to produce a better diagnostic when a heading resembles an ADR heading +# but does not use exactly three digits and the required hyphen. +ADR_HEADING_LIKE_RE = re.compile( + r"^#\s+ADR(?P[-_ ]?)(?P\d{1,4})\s*:" +) + +# A strict number reference. The next character must not continue a number, +# identifier, or the historical "ADR-009+" recommendation syntax. +ADR_REFERENCE_RE = re.compile( + r"\bADR-(?P\d{3})(?![0-9A-Za-z_+])" +) + +FULL_ADR_FILENAME_REFERENCE_RE = re.compile( + r"\b(?P" + r"ADR-(?P\d{3})-[A-Za-z0-9][A-Za-z0-9._-]*\.md" + r")\b" +) + +# Detect numeric ADR-like strings that are not canonical references: +# ADR-9, ADR-09, ADR-0009, ADR009, ADR_009, ADR 009, ADR-009+. +# +# Generic placeholders such as ADR-NNN are deliberately not matched. +ADR_LIKE_RE = re.compile( + r"\bADR" + r"(?P[-_ ]?)" + r"(?P\d{1,4})" + r"(?P\+?)" +) + + +@dataclasses.dataclass(frozen=True) +class Finding: + severity: str # "error" or "warning" + code: str + path: Path + line: Optional[int] + message: str + + def sort_key(self) -> Tuple[str, int, str, str]: + return ( + self.path.as_posix(), + self.line or 0, + self.code, + self.message, + ) + + +@dataclasses.dataclass(frozen=True) +class ADRDefinition: + number: str + path: Path + heading_line: int + + +@dataclasses.dataclass(frozen=True) +class Registry: + by_number: Dict[str, Tuple[ADRDefinition, ...]] + by_filename: Dict[str, ADRDefinition] + duplicate_numbers: Set[str] + + +@dataclasses.dataclass(frozen=True) +class AuditResult: + findings: Tuple[Finding, ...] + + @property + def errors(self) -> Tuple[Finding, ...]: + return tuple( + finding for finding in self.findings + if finding.severity == "error" + ) + + @property + def warnings(self) -> Tuple[Finding, ...]: + return tuple( + finding for finding in self.findings + if finding.severity == "warning" + ) + + +def _relative(path: Path, root: Path) -> Path: + try: + return path.resolve().relative_to(root.resolve()) + except ValueError: + return path + + +def _read_text(path: Path) -> Optional[str]: + """Read a probable text file, returning None for binary/non-UTF-8 files.""" + try: + raw = path.read_bytes() + except OSError: + return None + + if b"\x00" in raw: + return None + + try: + return raw.decode("utf-8") + except UnicodeDecodeError: + return None + + +def _iter_files(root: Path, locations: Sequence[Path]) -> Iterable[Path]: + seen: Set[Path] = set() + + for location in locations: + target = root / location + + if target.is_file(): + candidates = (target,) + elif target.is_dir(): + candidates = ( + path for path in target.rglob("*") + if path.is_file() and not path.is_symlink() + ) + else: + continue + + for path in candidates: + resolved = path.resolve() + if resolved in seen: + continue + seen.add(resolved) + yield path + + +def _definition_registry( + root: Path, + severity: str, +) -> Tuple[Registry, List[Finding]]: + adr_directory = root / ADR_DIRECTORY + findings: List[Finding] = [] + definitions: DefaultDict[str, List[ADRDefinition]] = defaultdict(list) + by_filename: Dict[str, ADRDefinition] = {} + + if not adr_directory.is_dir(): + findings.append(Finding( + severity=severity, + code="ADR_DIRECTORY_MISSING", + path=ADR_DIRECTORY, + line=None, + message="canonical ADR directory does not exist", + )) + return ( + Registry( + by_number={}, + by_filename={}, + duplicate_numbers=set(), + ), + findings, + ) + + for path in sorted(adr_directory.glob("ADR-*")): + if not path.is_file(): + continue + + relative_path = _relative(path, root) + filename_match = ADR_FILENAME_RE.fullmatch(path.name) + + if filename_match is None: + findings.append(Finding( + severity=severity, + code="ADR_FILENAME_MALFORMED", + path=relative_path, + line=None, + message=( + "ADR filename must match " + "ADR-NNN-descriptive-slug.md" + ), + )) + continue + + filename_number = filename_match.group("number") + text = _read_text(path) + + if text is None: + findings.append(Finding( + severity=severity, + code="ADR_READ_ERROR", + path=relative_path, + line=None, + message="ADR file is not readable UTF-8 text", + )) + continue + + exact_headings: List[Tuple[int, str]] = [] + heading_like: Optional[Tuple[int, str]] = None + + for line_number, line in enumerate(text.splitlines(), start=1): + exact = ADR_HEADING_RE.match(line) + if exact is not None: + exact_headings.append( + (line_number, exact.group("number")) + ) + continue + + approximate = ADR_HEADING_LIKE_RE.match(line) + if approximate is not None and heading_like is None: + heading_like = (line_number, line.strip()) + + if not exact_headings: + if heading_like is not None: + findings.append(Finding( + severity=severity, + code="ADR_HEADING_MALFORMED", + path=relative_path, + line=heading_like[0], + message=( + "top-level ADR heading must match " + "'# ADR-NNN: Title'; found " + f"{heading_like[1]!r}" + ), + )) + else: + findings.append(Finding( + severity=severity, + code="ADR_HEADING_MISSING", + path=relative_path, + line=None, + message=( + "ADR file requires one top-level heading matching " + "'# ADR-NNN: Title'" + ), + )) + continue + + if len(exact_headings) > 1: + findings.append(Finding( + severity=severity, + code="ADR_HEADING_MULTIPLE", + path=relative_path, + line=exact_headings[1][0], + message=( + "ADR file contains more than one canonical " + "top-level ADR heading" + ), + )) + continue + + heading_line, heading_number = exact_headings[0] + + if heading_number != filename_number: + findings.append(Finding( + severity=severity, + code="ADR_HEADING_MISMATCH", + path=relative_path, + line=heading_line, + message=( + f"filename claims ADR-{filename_number}, " + f"heading claims ADR-{heading_number}" + ), + )) + + definition = ADRDefinition( + number=filename_number, + path=relative_path, + heading_line=heading_line, + ) + definitions[filename_number].append(definition) + by_filename[path.name] = definition + + duplicate_numbers: Set[str] = { + number + for number, values in definitions.items() + if len(values) > 1 + } + + # One finding per duplicate number. Reference scanning deliberately avoids + # emitting one ambiguity error per citation; the registry defect is the + # root cause and should be reported once. + for number in sorted(duplicate_numbers): + paths = ", ".join( + definition.path.as_posix() + for definition in sorted( + definitions[number], + key=lambda item: item.path.as_posix(), + ) + ) + findings.append(Finding( + severity=severity, + code="ADR_DUPLICATE", + path=ADR_DIRECTORY, + line=None, + message=f"ADR-{number} is assigned to multiple files: {paths}", + )) + + registry = Registry( + by_number={ + number: tuple(values) + for number, values in definitions.items() + }, + by_filename=by_filename, + duplicate_numbers=duplicate_numbers, + ) + return registry, findings + + +def _scan_references( + root: Path, + locations: Sequence[Path], + registry: Registry, + severity: str, + exclusions: Set[Path], +) -> List[Finding]: + findings: List[Finding] = [] + + for path in sorted( + _iter_files(root, locations), + key=lambda value: value.as_posix(), + ): + relative_path = _relative(path, root) + + if relative_path in exclusions: + continue + + text = _read_text(path) + if text is None: + continue + + for line_number, line in enumerate(text.splitlines(), start=1): + malformed_spans: List[Tuple[int, int]] = [] + + for match in ADR_LIKE_RE.finditer(line): + separator = match.group("separator") + number = match.group("number") + plus = match.group("plus") + + is_canonical = ( + separator == "-" + and len(number) == 3 + and not plus + ) + + if is_canonical: + continue + + malformed_spans.append(match.span()) + findings.append(Finding( + severity=severity, + code="ADR_REFERENCE_MALFORMED", + path=relative_path, + line=line_number, + message=( + f"malformed ADR reference {match.group(0)!r}; " + "use ADR-NNN" + ), + )) + + for match in FULL_ADR_FILENAME_REFERENCE_RE.finditer(line): + filename = match.group("filename") + + if filename not in registry.by_filename: + findings.append(Finding( + severity=severity, + code="ADR_FILENAME_REFERENCE_MISSING", + path=relative_path, + line=line_number, + message=( + f"reference names ADR file {filename!r}, " + "but that exact file does not exist" + ), + )) + + for match in ADR_REFERENCE_RE.finditer(line): + # Do not reinterpret part of a malformed token as a second, + # otherwise ADR-009+ would produce both malformed and missing. + if any( + start <= match.start() < end + for start, end in malformed_spans + ): + continue + + number = match.group("number") + + # Duplicate numbers already produce one registry finding. + if number in registry.duplicate_numbers: + continue + + definitions = registry.by_number.get(number, ()) + + if not definitions: + findings.append(Finding( + severity=severity, + code="ADR_REFERENCE_MISSING", + path=relative_path, + line=line_number, + message=( + f"ADR-{number} is referenced but has no " + "canonical decision-log file" + ), + )) + elif len(definitions) > 1: + # Defensive: normally covered by duplicate_numbers. + findings.append(Finding( + severity=severity, + code="ADR_REFERENCE_AMBIGUOUS", + path=relative_path, + line=line_number, + message=( + f"ADR-{number} resolves to more than one file" + ), + )) + + return findings + + +def audit_repository(root: Path, mode: str = "hard") -> AuditResult: + """Audit one repository tree. + + Modes: + hard: + Registry and normative references are errors. Exit should fail. + + advisory: + Registry, normative references, and historical prose are warnings. + Exit should not fail. + + all: + Registry/normative findings are errors; historical prose findings are + warnings. + """ + root = root.resolve() + + if mode not in {"hard", "advisory", "all"}: + raise ValueError(f"unsupported mode: {mode}") + + hard_severity = "warning" if mode == "advisory" else "error" + registry, findings = _definition_registry( + root=root, + severity=hard_severity, + ) + + findings.extend(_scan_references( + root=root, + locations=HARD_ROOTS, + registry=registry, + severity=hard_severity, + exclusions=HARD_SCAN_EXCLUSIONS, + )) + + if mode in {"advisory", "all"}: + findings.extend(_scan_references( + root=root, + locations=ADVISORY_ROOTS, + registry=registry, + severity="warning", + exclusions=set(), + )) + + unique: Dict[ + Tuple[str, str, str, Optional[int], str], + Finding, + ] = {} + + for finding in findings: + key = ( + finding.severity, + finding.code, + finding.path.as_posix(), + finding.line, + finding.message, + ) + unique[key] = finding + + return AuditResult( + findings=tuple( + sorted(unique.values(), key=lambda value: value.sort_key()) + ) + ) + + +def _escape_github_command(value: str) -> str: + return ( + value.replace("%", "%25") + .replace("\r", "%0D") + .replace("\n", "%0A") + ) + + +def _render_finding(finding: Finding, github_annotations: bool) -> None: + location = finding.path.as_posix() + if finding.line is not None: + location = f"{location}:{finding.line}" + + if github_annotations: + command = "error" if finding.severity == "error" else "warning" + attributes = f"file={finding.path.as_posix()}" + if finding.line is not None: + attributes += f",line={finding.line}" + + message = _escape_github_command( + f"[{finding.code}] {finding.message}" + ) + print(f"::{command} {attributes}::{message}") + return + + print( + f"{finding.severity.upper():7} " + f"{location} [{finding.code}] {finding.message}" + ) + + +def main(argv: Optional[Sequence[str]] = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--root", + type=Path, + default=Path.cwd(), + help="repository root to audit; defaults to the current directory", + ) + parser.add_argument( + "--mode", + choices=("hard", "advisory", "all"), + default="hard", + help=( + "hard fails on normative defects; advisory reports everything " + "without failing; all combines hard errors and advisory warnings" + ), + ) + parser.add_argument( + "--github-annotations", + choices=("auto", "always", "never"), + default="auto", + help="emit GitHub Actions workflow annotations", + ) + args = parser.parse_args(argv) + + root = args.root.resolve() + if not root.is_dir(): + parser.error(f"repository root does not exist: {root}") + + annotations = ( + args.github_annotations == "always" + or ( + args.github_annotations == "auto" + and os.environ.get("GITHUB_ACTIONS") == "true" + ) + ) + + result = audit_repository(root=root, mode=args.mode) + + for finding in result.findings: + _render_finding(finding, github_annotations=annotations) + + if result.errors: + print( + f"ADR integrity: FAIL " + f"({len(result.errors)} error(s), " + f"{len(result.warnings)} warning(s))" + ) + return 1 + + print( + f"ADR integrity: PASS " + f"({len(result.warnings)} warning(s))" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/Sources/BCILLM/MLXSentenceEmbedder.swift b/Sources/BCILLM/MLXSentenceEmbedder.swift index d29cab9..f6a1f58 100644 --- a/Sources/BCILLM/MLXSentenceEmbedder.swift +++ b/Sources/BCILLM/MLXSentenceEmbedder.swift @@ -99,7 +99,7 @@ public final class MLXSentenceEmbedder: SentenceEmbedder, Sendable { let tokens = tokenizer.encode(text: text) guard !tokens.isEmpty else { // Deterministic finite fallback for input the tokenizer - // reduces to nothing (ADR-004 §3.1) — same shape of answer + // reduces to nothing (ADR-010 §3.1) — same shape of answer // as CoreMLSentenceEmbedder's degenerate-norm path. return Self.axisUnitVector(dimension: expectedDimension) } @@ -142,7 +142,7 @@ public final class MLXSentenceEmbedder: SentenceEmbedder, Sendable { } // The pooler already normalized; re-normalize on the Swift side so - // the ADR-004 §3.1 unit-norm invariant never depends on upstream + // the ADR-010 §3.1 unit-norm invariant never depends on upstream // library behavior, with the axis fallback for degenerate vectors. let normalized = Self.l2Normalized(values) ?? Self.axisUnitVector(dimension: expectedDimension) diff --git a/Sources/EmbeddingBench/BenchmarkRunner.swift b/Sources/EmbeddingBench/BenchmarkRunner.swift index de43a8e..cbabc21 100644 --- a/Sources/EmbeddingBench/BenchmarkRunner.swift +++ b/Sources/EmbeddingBench/BenchmarkRunner.swift @@ -3,7 +3,7 @@ import Foundation /// Measures any `SentenceEmbedder` conformer. Deliberately generic — this /// file must never import CoreML, MLX, or any backend-specific type -/// (ADR-004 §3.5 Gate 1). A future `CoreMLSentenceEmbedder` or +/// (ADR-010 §3.5 Gate 1). A future `CoreMLSentenceEmbedder` or /// `MLXSentenceEmbedder` is measured through this exact same code path. enum BenchmarkRunner { static let batchSizes = [1, 8, 32, 128] diff --git a/Sources/EmbeddingBench/main.swift b/Sources/EmbeddingBench/main.swift index 21c94cb..4b6ffd4 100644 --- a/Sources/EmbeddingBench/main.swift +++ b/Sources/EmbeddingBench/main.swift @@ -8,7 +8,7 @@ import Foundation // `MLXSentenceEmbedder`. Backend selection is argument parsing over concrete // conformers — the same "not a runtime-selection registry" convention as // SemanticEval (CLAUDE.md). `BenchmarkRunner` is unchanged either way -// (ADR-004 §3.5 Gate 1: it never sees a backend-specific type). +// (ADR-010 §3.5 Gate 1: it never sees a backend-specific type). // // Flags (all optional — bare invocation behaves exactly as it did in // Stage 3.2: try Core ML at Models/BGE-small-en-v1.5, else stub): diff --git a/Tests/BCICoreTests/SemanticBGEReplayRegressionTests.swift b/Tests/BCICoreTests/SemanticBGEReplayRegressionTests.swift index 95e5fd0..05649e7 100644 --- a/Tests/BCICoreTests/SemanticBGEReplayRegressionTests.swift +++ b/Tests/BCICoreTests/SemanticBGEReplayRegressionTests.swift @@ -8,7 +8,7 @@ import CryptoKit /// text -> CoreMLSentenceEmbedder -> Embedding -> RandomProjectionProjector -> coords /// /// A **separate file** from `SemanticReplayRegressionTests.swift` by design -/// (ADR-004 §3.5 Gate 1 forbids editing that file for a backend +/// (ADR-010 §3.5 Gate 1 forbids editing that file for a backend /// substitution) — structurally a duplicate, retargeted at /// `Tests/Fixtures/semantic_bge_small_v1.json`. The stub's replay stays /// frozen forever; this is independent evidence for a second backend. diff --git a/Tests/BCILLMTests/MLXSentenceEmbedderTests.swift b/Tests/BCILLMTests/MLXSentenceEmbedderTests.swift index cf1e61a..6b1de85 100644 --- a/Tests/BCILLMTests/MLXSentenceEmbedderTests.swift +++ b/Tests/BCILLMTests/MLXSentenceEmbedderTests.swift @@ -10,7 +10,7 @@ import XCTest /// under plain `swift test` (no Xcode-built metallib required; MLX kernels /// only load when arrays are evaluated). /// -/// The real-model path (load + encode + ADR-004 §3.1 invariants) is gated on +/// The real-model path (load + encode + ADR-010 §3.1 invariants) is gated on /// `NEURALCOMPOSE_MLX_EMBEDDER_MODEL_DIR` and needs an Xcode-built test run — /// same gating convention as the MLX generation regression tests. final class MLXSentenceEmbedderTests: XCTestCase { @@ -98,7 +98,7 @@ final class MLXSentenceEmbedderTests: XCTestCase { } } - // MARK: - Numeric invariants (ADR-004 §3.1) + // MARK: - Numeric invariants (ADR-010 §3.1) func testL2NormalizedProducesUnitNorm() { let normalized = MLXSentenceEmbedder.l2Normalized([3, 4]) @@ -143,7 +143,7 @@ final class MLXSentenceEmbedderTests: XCTestCase { for embedding in embeddings { XCTAssertEqual(embedding.values.count, embedder.dimension) XCTAssertEqual(embedding.modelID, embedder.modelID) - // Unit norm within 1e-4 (ADR-004 §2.1), finite everywhere — + // Unit norm within 1e-4 (ADR-010 §2.1), finite everywhere — // including for the empty string. XCTAssertTrue(embedding.values.allSatisfy(\.isFinite)) let norm = embedding.values.reduce(Float(0)) { $0 + $1 * $1 }.squareRoot() diff --git a/Tests/eval/test_adr_references.py b/Tests/eval/test_adr_references.py new file mode 100644 index 0000000..c1fe527 --- /dev/null +++ b/Tests/eval/test_adr_references.py @@ -0,0 +1,234 @@ +#!/usr/bin/env python3 +"""Contract tests for Scripts/check_adr_references.py. + +Standard library only — `unittest`, no pytest, no NumPy, no MLX. The checker +runs as a direct `python3` step in the Swift CI job before any Python job +exists, so its own tests must not acquire dependencies the gate does not have. + +Two kinds of test here, deliberately separated: + + * synthetic fixtures build throwaway trees and prove the checker *can* fail. + A gate whose only evidence is that it never complained is not a gate. + * one live test runs against the real repository and asserts hard mode + passes. That is the gate itself. +""" + +from __future__ import annotations + +import shutil +import sys +import tempfile +import unittest +from pathlib import Path +from typing import Iterable, Optional + + +REPOSITORY_ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(REPOSITORY_ROOT / "Scripts")) + +import check_adr_references as checker # noqa: E402 + + +class _FixtureTree: + """A throwaway repository tree with a controllable ADR registry.""" + + def __init__(self) -> None: + self.root = Path(tempfile.mkdtemp(prefix="adr-fixture-")) + (self.root / checker.ADR_DIRECTORY).mkdir(parents=True) + for directory in ("Sources", "Tests", "Scripts", "docs/reviews"): + (self.root / directory).mkdir(parents=True, exist_ok=True) + + def cleanup(self) -> None: + shutil.rmtree(self.root, ignore_errors=True) + + def adr(self, filename: str, heading: Optional[str] = None) -> None: + """Write an ADR. `heading` defaults to one matching the filename.""" + if heading is None: + number = filename.split("-")[1] + heading = f"# ADR-{number}: Fixture decision" + path = self.root / checker.ADR_DIRECTORY / filename + path.write_text(f"{heading}\n\n**Status**: Accepted\n", encoding="utf-8") + + def write(self, relative: str, text: str) -> None: + path = self.root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + def codes(self, mode: str = "hard", severity: str = "error") -> list[str]: + result = checker.audit_repository(root=self.root, mode=mode) + findings = result.errors if severity == "error" else result.warnings + return sorted(finding.code for finding in findings) + + +class ADRRegistryTests(unittest.TestCase): + """Registry-level defects: duplicates, headings, filenames.""" + + def setUp(self) -> None: + self.tree = _FixtureTree() + self.addCleanup(self.tree.cleanup) + + def test_clean_registry_passes(self) -> None: + self.tree.adr("ADR-001-first.md") + self.tree.adr("ADR-002-second.md") + self.assertEqual(self.tree.codes(), []) + + def test_duplicate_number_fails_once_not_per_citation(self) -> None: + """The registry defect is the root cause and is reported one time. + + Reproduces main's actual state: two accepted documents claiming + ADR-004, with several citations that would each otherwise be reported + ambiguous. One finding, not N. + """ + self.tree.adr("ADR-004-privacy.md") + self.tree.adr("ADR-004-embedding.md") + self.tree.write("Sources/A.swift", "// see ADR-004 §3.1\n") + self.tree.write("Sources/B.swift", "// see ADR-004 §3.5\n") + self.tree.write("Tests/C.swift", "// see ADR-004\n") + + self.assertEqual(self.tree.codes(), ["ADR_DUPLICATE"]) + + def test_filename_heading_mismatch_fails(self) -> None: + self.tree.adr("ADR-005-mismatch.md", heading="# ADR-006: Wrong number") + self.assertIn("ADR_HEADING_MISMATCH", self.tree.codes()) + + def test_missing_heading_fails(self) -> None: + path = self.tree.root / checker.ADR_DIRECTORY / "ADR-007-noheading.md" + path.write_text("Some prose with no ADR heading.\n", encoding="utf-8") + self.assertIn("ADR_HEADING_MISSING", self.tree.codes()) + + def test_malformed_filename_fails(self) -> None: + path = self.tree.root / checker.ADR_DIRECTORY / "ADR-8-short.md" + path.write_text("# ADR-008: Short number\n", encoding="utf-8") + self.assertIn("ADR_FILENAME_MALFORMED", self.tree.codes()) + + +class ADRReferenceTests(unittest.TestCase): + """Reference-level defects in normative roots.""" + + def setUp(self) -> None: + self.tree = _FixtureTree() + self.addCleanup(self.tree.cleanup) + self.tree.adr("ADR-001-first.md") + + def test_dangling_reference_fails(self) -> None: + self.tree.write("Sources/A.swift", "// governed by ADR-042\n") + self.assertEqual(self.tree.codes(), ["ADR_REFERENCE_MISSING"]) + + def test_resolvable_reference_passes(self) -> None: + self.tree.write("Sources/A.swift", "// governed by ADR-001 §2\n") + self.assertEqual(self.tree.codes(), []) + + def test_malformed_reference_fails(self) -> None: + for token in ("ADR-1", "ADR-01", "ADR-0001", "ADR001", "ADR_001"): + with self.subTest(token=token): + self.tree.write("Sources/A.swift", f"// see {token}\n") + self.assertIn("ADR_REFERENCE_MALFORMED", self.tree.codes()) + + def test_renamed_file_leaves_dangling_full_filename_reference(self) -> None: + """The trap that a section-marked sed pass leaves behind. + + Rewriting `ADR-004 §3.5` everywhere still misses a markdown link whose + target is the old *filename*. On the real repository this was + `README.md` and `ROADMAP.md`. + """ + self.tree.write( + "docs/architecture/ROADMAP.md", + "see [contract](decision-log/ADR-001-renamed-away.md)\n", + ) + self.assertEqual(self.tree.codes(), ["ADR_FILENAME_REFERENCE_MISSING"]) + + def test_forward_reference_to_unlanded_adr_fails(self) -> None: + """A normative document may not cite a number that does not exist yet. + + Found by running the repair: a ROADMAP note announcing that "ADR-009 is + reserved for generation-runtime semantics" fails the very gate it + announces, because ADR-009 lands later with lane A. Reserved numbers + must be described without emitting a canonical token. + """ + self.tree.write( + "docs/architecture/ROADMAP.md", + "ADR-009 is reserved for the runtime that lands later.\n", + ) + self.assertEqual(self.tree.codes(), ["ADR_REFERENCE_MISSING"]) + + self.tree.write( + "docs/architecture/ROADMAP.md", + "The intervening number is reserved for the runtime.\n", + ) + self.assertEqual(self.tree.codes(), []) + + +class ADRPolicyBoundaryTests(unittest.TestCase): + """Numeric gaps are legal; history is not normative.""" + + def setUp(self) -> None: + self.tree = _FixtureTree() + self.addCleanup(self.tree.cleanup) + + def test_numeric_gap_passes(self) -> None: + """ADR-010 may exist while 009 is reserved for unlanded work.""" + self.tree.adr("ADR-008-eighth.md") + self.tree.adr("ADR-010-tenth.md") + self.tree.write("Sources/A.swift", "// see ADR-010 §1\n") + self.assertEqual(self.tree.codes(), []) + + def test_historical_review_is_advisory_not_normative(self) -> None: + """The exact contradiction this two-mode design exists to avoid. + + `docs/reviews/` legitimately names numbers that did not exist when the + review was written. Requiring the gate both to exclude reviews and to + fail because of them cannot be satisfied. + """ + self.tree.adr("ADR-001-first.md") + self.tree.write( + "docs/reviews/code-review.md", + "Renumber the later one to ADR-009+.\n", + ) + + self.assertEqual(self.tree.codes(mode="hard"), []) + self.assertEqual( + self.tree.codes(mode="all", severity="warning"), + ["ADR_REFERENCE_MALFORMED"], + ) + + def test_advisory_mode_never_reports_errors(self) -> None: + self.tree.adr("ADR-004-one.md") + self.tree.adr("ADR-004-two.md") + result = checker.audit_repository(root=self.tree.root, mode="advisory") + self.assertEqual(result.errors, ()) + self.assertTrue(result.warnings) + + def test_checker_and_its_tests_are_excluded_from_hard_scan(self) -> None: + """Otherwise this file's own synthetic tokens would fail the gate.""" + self.tree.adr("ADR-001-first.md") + self.tree.write("Scripts/check_adr_references.py", "# ADR-999 ADR_001\n") + self.tree.write("Tests/eval/test_adr_references.py", "# ADR-999\n") + self.assertEqual(self.tree.codes(), []) + + +class LiveRepositoryTests(unittest.TestCase): + """The gate itself, against the tree this test is running in.""" + + def test_repository_passes_hard_mode(self) -> None: + result = checker.audit_repository(root=REPOSITORY_ROOT, mode="hard") + self.assertEqual( + result.errors, + (), + "\n".join( + f"{f.path}:{f.line or 0} [{f.code}] {f.message}" + for f in result.errors + ), + ) + + def test_decision_log_exists_and_is_non_empty(self) -> None: + """Guards against a vacuous pass if the directory is ever moved.""" + adr_directory = REPOSITORY_ROOT / checker.ADR_DIRECTORY + self.assertTrue(adr_directory.is_dir(), f"missing {checker.ADR_DIRECTORY}") + self.assertTrue( + list(adr_directory.glob("ADR-*.md")), + "decision log contains no ADRs — the gate would pass trivially", + ) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/docs/architecture/ROADMAP.md b/docs/architecture/ROADMAP.md index c506790..a54c5d2 100644 --- a/docs/architecture/ROADMAP.md +++ b/docs/architecture/ROADMAP.md @@ -109,12 +109,13 @@ specific tasks (files, PRs, tests) live in GitHub Issues. pipeline) ### Cross-cutting -- `✓` Initial ADR set in `docs/architecture/decision-log/` (ADR-001 - through ADR-005; note ADR-004 is currently double-assigned — - `ADR-004-privacy-first-acquisition.md` and - `ADR-004-sentence-embedder-backend-contract.md` both claim that - number, worth a rename/renumber pass since other docs may already - reference them by filename) +- `✓` Initial ADR set in `docs/architecture/decision-log/`. ADR-004 was + double-assigned; resolved by renumbering the embedding contract to + ADR-010 and rewriting its citations. The intervening number is + reserved for generation-runtime semantics and lands with the runtime + it governs, so a numeric gap here is expected and is not an error. + `Scripts/check_adr_references.py` now enforces one file per number and + that every normative reference resolves. - `✓` `docs/architecture/PRINCIPLES.md` — engineering values that govern how new work is integrated - `□` Layered architecture diagram in the main README diff --git a/docs/architecture/decision-log/ADR-004-sentence-embedder-backend-contract.md b/docs/architecture/decision-log/ADR-010-sentence-embedder-backend-contract.md similarity index 97% rename from docs/architecture/decision-log/ADR-004-sentence-embedder-backend-contract.md rename to docs/architecture/decision-log/ADR-010-sentence-embedder-backend-contract.md index 99705c1..ba197b9 100644 --- a/docs/architecture/decision-log/ADR-004-sentence-embedder-backend-contract.md +++ b/docs/architecture/decision-log/ADR-010-sentence-embedder-backend-contract.md @@ -1,4 +1,4 @@ -# ADR-004: SentenceEmbedder backend contract +# ADR-010: SentenceEmbedder backend contract **Status**: Accepted **Date**: 2026-07-12 @@ -21,7 +21,7 @@ for a reviewer of a pull request, because it is 315 lines long and organized by scope (producer/artifact/system) rather than by invariant. Reviewers and future contributors need a short, normative reference -that can be cited as `ADR-004 §3.5` from a PR comment, an audit, or a +that can be cited as `ADR-010 §3.5` from a PR comment, an audit, or a code-review note. The contract document is the specification the ADR points at; the ADR is the document you cite. @@ -133,7 +133,7 @@ implementation details into the rest of the app. Both must pass. document is 315 lines, organized by scope, and is the right artifact for someone *implementing* a backend. It is the wrong artifact for *citing* in a PR review ("violates §7.1" is more useful as "violates -ADR-004 §3.5 Gate 1"). Splitting them keeps the ADR short and stable +ADR-010 §3.5 Gate 1"). Splitting them keeps the ADR short and stable (the ADR's §3 list changes only when an invariant changes; the contract's prose can be edited to clarify without re-ratifying the invariants). @@ -167,7 +167,7 @@ A future contributor adding a `SentenceEmbedder` conformer that: without going through a per-backend reference set. Each of those is a smell. The ADR's §3 list is the *checklist* the PR -is judged against. "Violates ADR-004 §3.5 Gate 2" is a complete code- +is judged against. "Violates ADR-010 §3.5 Gate 2" is a complete code- review note. ## When this rule does not apply diff --git a/docs/architecture/embedding_contract.md b/docs/architecture/embedding_contract.md index c63ce5f..653a46e 100644 --- a/docs/architecture/embedding_contract.md +++ b/docs/architecture/embedding_contract.md @@ -29,7 +29,7 @@ This document is a *companion* to the ADRs, not a replacement: - [PRINCIPLES §6](../PRINCIPLES.md) — components communicate across protocol boundaries - [ADR-002 — Deterministic replay as the validation backbone](decision-log/ADR-002-deterministic-replay.md) — the pattern this contract follows - [ADR-003 — Runtime separation](decision-log/ADR-003-runtime-separation.md) — why MLX/ANNE stay out of `BCICore` -- [ADR-004 — SentenceEmbedder backend contract](decision-log/ADR-004-sentence-embedder-backend-contract.md) — the *short normative reference* for this document. The ADR is the right artifact to cite from a PR review (`ADR-004 §3.5 Gate 1`); this document is the long-form spec the ADR ratifies. +- [ADR-010 — SentenceEmbedder backend contract](decision-log/ADR-010-sentence-embedder-backend-contract.md) — the *short normative reference* for this document. The ADR is the right artifact to cite from a PR review (`ADR-010 §3.5 Gate 1`); this document is the long-form spec the ADR ratifies. **Document vs. ADR**: When the ADR and this document disagree, the ADR's §3 list is normative. The prose here explains the list, cites