diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 2747ce9..88c471f 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -114,7 +114,7 @@ falsifyai/ ← repo root └── examples/ ``` -All subpackages have empty `__init__.py` files only — no implementation yet. +All subpackages shown above are implemented — the pipeline runs end-to-end (spec → materialize → execute → judge → resolve → save → CLI). See [CHANGELOG.md](../CHANGELOG.md) for what landed when. ## Design anchors (when implementing, do not reinvent) diff --git a/AGENTS.md b/AGENTS.md index 28b1796..d995fd3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,7 +114,7 @@ falsifyai/ ← repo root └── examples/ ``` -All subpackages have empty `__init__.py` files only — no implementation yet. +All subpackages shown above are implemented — the pipeline runs end-to-end (spec → materialize → execute → judge → resolve → save → CLI). See [CHANGELOG.md](CHANGELOG.md) for what landed when. ## Design anchors (when implementing, do not reinvent) diff --git a/CHANGELOG.md b/CHANGELOG.md index 379186e..53bdc50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,22 @@ follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Documentation + +- **Architecture-doc freshness + preservation guardrails.** + [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) now reflects the full + 11-command CLI surface (the read-only consumers `inspect`, `history`, + `timeline`, `matrix`, `verify`, `export` alongside `run` / `replay` / `diff` + and the `doctor` diagnostic) and the current perturbation (`unicode_chars`, + `paraphrase`) and invariant (`schema_match`) families. Two anti-entropy guards + now back the doc's claims with executable invariants: a single parametrized + harness asserts every read-only consumer closes its `ReplayStore` (on normal + return *and* on post-construction read failure), and the CLI import-hygiene + guard now also forbids read-only commands from importing + `falsifyai.verdict.resolver` — mechanizing the long-standing "consumers never + re-resolve" guarantee and closing a gap where `replay` had no such check. No + runtime behavior changes. + ## [0.6.4] — 2026-06-05 Patch release. Closes a self-falsification in the *evidence-generation* layer diff --git a/README.md b/README.md index 3af1e0f..524e4a6 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,7 @@ See [`docs/case-studies/`](docs/case-studies/) for the index and the framing con ## CLI reference -Ten subcommands, one workflow: +Eleven subcommands, one workflow: ```bash falsifyai run [--store-path PATH] [--nli] @@ -221,6 +221,7 @@ falsifyai minimize [--case CASE_ID] [--family typo_noise|unicode] [- falsifyai verify [--store-path PATH] falsifyai verify --all [--store-path PATH] falsifyai export --bundle .fai.zip [--spec-path PATH] [--allow-corrupted] [--overwrite] [--exported-at ISO8601] [--store-path PATH] +falsifyai doctor [--store-path PATH] # read-only environment diagnostics; run this first if an install looks broken ``` `history` shows raw newest-first rows and refuses to aggregate; `timeline` is its inference counterpart (chronological trend + regression detection). `matrix` generalizes the pairwise `diff` to N model runs. `minimize` searches for the *smallest* perturbation that breaks a case — the minimal falsifier. @@ -264,7 +265,7 @@ Ship the *evidence* with your PR, not just the pass/fail signal: - **Not a prompt optimization suite.** No prompt tuning, no automated A/B over wordings. The spec is authored deliberately. - **Not a telemetry platform.** No streaming, no production dashboards, no time-series. The artifact is per-run preserved evidence. - **Not a generalized observability product.** The CLI compresses; the artifact preserves. The headline tells you whether to look; the artifact tells you what to look at. -- **Not a workflow orchestrator.** Ten subcommands are the entire surface. +- **Not a workflow orchestrator.** Eleven subcommands are the entire surface. - **Not an AI governance suite.** Governance platforms consume reliability evidence; FalsifyAI produces it. These exclusions keep the surface compressible. Adding any of them corrupts the discipline. @@ -383,6 +384,13 @@ uv sync --extra dev uv run pytest ``` +If anything looks off — a confusing install, a missing optional extra, an +unwritable store path — run the read-only diagnostic before filing an issue: + +```bash +uv run falsifyai doctor +``` + --- ## License diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 99b02e7..5b86d16 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -109,14 +109,20 @@ piece of behavior lives in exactly one of them. | **Evidence interpretation** | Judges observations and compresses them into a verdict (which is *a claim about the evidence*). | `falsifyai.invariants`, `falsifyai.verdict`, `falsifyai.falsifiability`, `falsifyai.cli.render` | | **Evidence preservation** — *the durable product* | Persists the full evidence trail so it outlives the run. The replay artifact is the system's central object; the other two layers exist to produce and feed it. | `falsifyai.replay` (artifact, store, serializer) | -The CLI subcommands (`falsifyai run` / `replay` / `diff`) are **consumers -of these layers**, not a fourth layer: +The CLI subcommands are **consumers of these layers**, not a fourth layer. The +surface is 11 commands: the producer `run` (and `minimize`), the read-only +consumers `replay`, `diff`, `inspect`, `history`, `timeline`, `matrix`, +`verify`, and `export`, plus the `doctor` environment diagnostic. - `run` orchestrates generation → interpretation → preservation. - `replay` reads preservation and feeds it back through interpretation for rendering (never re-resolves). - `diff` reads two preserved artifacts and compares their interpretation outputs. +- the other read-only consumers (`inspect`, `history`, `timeline`, `matrix`, + `verify`, `export`) follow `replay`'s shape: they read preserved verdicts and + never re-enter the resolver. `doctor` touches neither artifacts nor the + resolver — it probes the environment and store backend. ### The line to hold @@ -437,8 +443,8 @@ know which question is being asked. These are not the same epistemic problem. The MVP `falsifyai run` answers *"Is this stable?"* (falsification-first). `falsifyai diff` answers *"Did -migration regress?"* (comparative). Future commands (`history`, -`inspect`) will address the others. +migration regress?"* (comparative). The shipped consumers (`history`, +`inspect`, `timeline`, `matrix`) address the others. **Architectural implication:** don't prematurely encode this as a CLI surface split (e.g., `run` vs `measure-fragility`). The strategy may be @@ -456,13 +462,13 @@ A one-line orientation for each subpackage: | Subpackage | Role | |---|---| | `falsifyai.spec` | Pydantic models + YAML loader + `materialize()` | -| `falsifyai.perturbation` | `Perturbation` Protocol + `typo_noise` + `casing_variant` + registry | +| `falsifyai.perturbation` | `Perturbation` Protocol + `typo_noise` + `casing_variant` + `unicode_chars` + `paraphrase` (bidirectional-NLI validity) + registry | | `falsifyai.execution` | `ModelAdapter` Protocol + `LiteLLMAdapter` + `ExecutionEngine` + `InMemoryCache` | -| `falsifyai.invariants` | `Invariant` Protocol + `contains` + `semantic_equivalence` + `EmbeddingBackend` | +| `falsifyai.invariants` | `Invariant` Protocol + `contains` + `semantic_equivalence` + `schema_match` + `EmbeddingBackend` + plugin entry-point group | | `falsifyai.verdict` | `Verdict` enum + `resolver` (priority chain) + `stratify` + `consistency` | | `falsifyai.falsifiability` | Per-case + suite-level falsifiability scoring | -| `falsifyai.replay` | `ReplayStore` Protocol + `SQLiteStore` + `InMemoryStore` + artifact + serializer | -| `falsifyai.cli` | `main` (argparse) + `run` + `replay` + `diff` + `render` + `errors` | +| `falsifyai.replay` | `ReplayStore` Protocol + `SQLiteStore` + `InMemoryStore` + pluggable store backends + artifact + serializer | +| `falsifyai.cli` | `main` (argparse) + `run` / `minimize` + read-only consumers (`replay`, `diff`, `inspect`, `history`, `timeline`, `matrix`, `verify`, `export`) + `doctor` + `render` + `errors` | A new contributor should be able to find any feature in <30 seconds using this table. diff --git a/falsifyai/cli/doctor.py b/falsifyai/cli/doctor.py index 4410701..da56c26 100644 --- a/falsifyai/cli/doctor.py +++ b/falsifyai/cli/doctor.py @@ -2,8 +2,9 @@ Reports the runtime facts a user needs to explain a confusing install: the Python and package versions, whether the core runtime dependencies import, -which optional extras (``[semantic]``, ``[nli]``) are available, and whether a -replay store can actually be written. +which optional extras (``[semantic]``, ``[nli]``) are available, which store +backend the configured ``--store-path`` selects (and whether that backend is +registered), and whether a replay store can actually be written. It *diagnoses only*. It never installs anything, writes config, or mutates user data -- the store-writability probe lives in a tempfile and is deleted, and @@ -95,8 +96,45 @@ def _extra_check(label: str, modules: list[str], extra: str) -> Check: return Check(label, "not installed", _INFO, f'pip install "falsifyai[{extra}]"') -def _store_check(store_path: str) -> Check: - """Prove a replay store can be written -- via a throwaway tempfile DB, never +def _store_checks(store_path: str) -> list[Check]: + """Diagnose the store the configured ``--store-path`` actually selects. + + Two facts. (1) *Backend resolution*: is a store registered for the selected + URI scheme? This is the same resolution ``build_store`` performs, so an + unknown scheme (a missing store plugin) fails here in diagnostics instead of + surfacing as a crash at ``run`` time. (2) *Writability*: for the built-in + SQLite store, prove a store can actually be written. A plugin store + (``postgres://`` ...) is reported as registered but *not* write-probed -- + constructing one may open a network connection or allocate resources, which + would break doctor's diagnose-only contract. + """ + from falsifyai.replay.registry import discover_stores, store_scheme + + scheme = store_scheme(store_path) + backends = sorted(discover_stores()) # reads entry points only; no construction + + if scheme not in backends: + return [ + Check( + "store backend", + f"{scheme} (no backend registered)", + _FAIL, + f"available: {', '.join(backends) or 'none'} -- install a store plugin " + f"for {scheme!r}", + ) + ] + + backend = Check("store backend", f"{scheme} (available: {', '.join(backends)})", _OK) + if scheme == "sqlite": + return [backend, _sqlite_write_check(store_path)] + if scheme == "memory": + return [backend, Check("store write", ":memory: (ephemeral, no disk)", _OK)] + # A registered plugin backend: don't construct it (possible side effects). + return [backend, Check("store write", f"{store_path} (plugin store; not probed)", _INFO)] + + +def _sqlite_write_check(store_path: str) -> Check: + """Prove a SQLite store can be written -- via a throwaway tempfile DB, never the user's real store -- and that the configured store dir is writable.""" from falsifyai.replay.sqlite_store import SQLiteStore @@ -129,7 +167,7 @@ def collect_checks(store_path: str) -> list[Check]: _core_deps_check(), _extra_check("[semantic] extra", ["sentence_transformers"], "semantic"), _extra_check("[nli] extra", ["transformers", "torch"], "nli"), - _store_check(store_path), + *_store_checks(store_path), ] diff --git a/falsifyai/cli/main.py b/falsifyai/cli/main.py index 64a5183..c8e6ed5 100644 --- a/falsifyai/cli/main.py +++ b/falsifyai/cli/main.py @@ -28,17 +28,6 @@ import sys from collections.abc import Sequence -from falsifyai.cli import diff as diff_cmd -from falsifyai.cli import doctor as doctor_cmd -from falsifyai.cli import export as export_cmd -from falsifyai.cli import history as history_cmd -from falsifyai.cli import inspect as inspect_cmd -from falsifyai.cli import matrix as matrix_cmd -from falsifyai.cli import minimize as minimize_cmd -from falsifyai.cli import replay as replay_cmd -from falsifyai.cli import run as run_cmd -from falsifyai.cli import timeline as timeline_cmd -from falsifyai.cli import verify as verify_cmd from falsifyai.cli.errors import CLIError @@ -322,28 +311,56 @@ def main(argv: Sequence[str] | None = None) -> int: parser.print_help() return 0 + # Command modules are imported lazily inside each branch, not at module top + # level: ``run`` / ``minimize`` pull in the model-execution stack (litellm), + # which is dead weight — and emits import-time warnings — for the read-only + # commands (``doctor``, ``verify``, ``replay``, ...). Dispatching one must not + # load the model stack. The guard in tests/meta/test_cli_import_hygiene.py + # fails if a command import leaks back to module top level. try: if args.command == "run": + from falsifyai.cli import run as run_cmd + return run_cmd.cmd_run(args) if args.command == "replay": + from falsifyai.cli import replay as replay_cmd + return replay_cmd.cmd_replay(args) if args.command == "inspect": + from falsifyai.cli import inspect as inspect_cmd + return inspect_cmd.cmd_inspect(args) if args.command == "diff": + from falsifyai.cli import diff as diff_cmd + return diff_cmd.cmd_diff(args) if args.command == "history": + from falsifyai.cli import history as history_cmd + return history_cmd.cmd_history(args) if args.command == "timeline": + from falsifyai.cli import timeline as timeline_cmd + return timeline_cmd.cmd_timeline(args) if args.command == "matrix": + from falsifyai.cli import matrix as matrix_cmd + return matrix_cmd.cmd_matrix(args) if args.command == "minimize": + from falsifyai.cli import minimize as minimize_cmd + return minimize_cmd.cmd_minimize(args) if args.command == "verify": + from falsifyai.cli import verify as verify_cmd + return verify_cmd.cmd_verify(args) if args.command == "export": + from falsifyai.cli import export as export_cmd + return export_cmd.cmd_export(args) if args.command == "doctor": + from falsifyai.cli import doctor as doctor_cmd + return doctor_cmd.cmd_doctor(args) except CLIError as exc: print(f"falsifyai: error: {exc}", file=sys.stderr) diff --git a/falsifyai/execution/__init__.py b/falsifyai/execution/__init__.py index a202b8b..85b8cdb 100644 --- a/falsifyai/execution/__init__.py +++ b/falsifyai/execution/__init__.py @@ -1,12 +1,16 @@ """Execution layer — adapter, cache, and engine.""" +from typing import TYPE_CHECKING + from falsifyai.execution.adapter import ModelAdapter from falsifyai.execution.cache import ExecutionCache, InMemoryCache from falsifyai.execution.engine import ExecutionEngine from falsifyai.execution.errors import ExecutionError -from falsifyai.execution.litellm_adapter import LiteLLMAdapter from falsifyai.execution.models import Execution, ModelRequest +if TYPE_CHECKING: + from falsifyai.execution.litellm_adapter import LiteLLMAdapter + __all__ = [ "Execution", "ExecutionCache", @@ -17,3 +21,19 @@ "ModelAdapter", "ModelRequest", ] + + +def __getattr__(name: str) -> object: + # ``LiteLLMAdapter`` is resolved lazily (PEP 562): importing it pulls in + # ``litellm`` — heavy, and noisy with import-time warnings. Touching any + # execution *submodule* (e.g. ``execution.models`` via a replay artifact) + # runs this package ``__init__``; eagerly importing the adapter here would + # drag litellm into every read-only CLI command. Deferring it keeps + # ``from falsifyai.execution import LiteLLMAdapter`` working while only + # paying the cost when the adapter is actually used. Guarded by + # tests/meta/test_cli_import_hygiene.py. + if name == "LiteLLMAdapter": + from falsifyai.execution.litellm_adapter import LiteLLMAdapter + + return LiteLLMAdapter + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/tests/integration/test_run_end_to_end.py b/tests/integration/test_run_end_to_end.py index b7d3c0f..d0ec75b 100644 --- a/tests/integration/test_run_end_to_end.py +++ b/tests/integration/test_run_end_to_end.py @@ -99,3 +99,50 @@ def test_artifact_roundtrip_via_load_session(tmp_path, monkeypatch) -> None: # Semantic equality across save -> load. assert loaded == sessions + + +# --------------------------------------------------------------------------- +# Store lifecycle: run is a *producer*; it owns the store it writes to and must +# close it whether the save succeeds or raises. (Read-only consumers are covered +# by tests/unit/test_cli_store_lifecycle.py; run is kept here, with the model +# stack already mocked, so the producer/consumer boundary stays clean.) +# --------------------------------------------------------------------------- + + +class _TrackingStore: + """Minimal producer-side store double: records save_session + close.""" + + def __init__(self, *, fail_save: bool = False) -> None: + self.saved: list = [] + self.closed = False + self._fail_save = fail_save + + def save_session(self, artifact) -> None: + if self._fail_save: + raise RuntimeError("forced save failure") + self.saved.append(artifact) + + def close(self) -> None: + self.closed = True + + +def test_run_closes_store_after_save(monkeypatch, capsys) -> None: + _patch_adapter(monkeypatch, "Paris is the capital of France.") + store = _TrackingStore() + monkeypatch.setattr(cli_run, "build_store", lambda _p: store) + + cli_run.cmd_run(_args(_SMOKE_SPEC, ":memory:")) + + assert len(store.saved) == 1 + assert store.closed is True + + +def test_run_closes_store_when_save_fails(monkeypatch, capsys) -> None: + _patch_adapter(monkeypatch, "Paris is the capital of France.") + store = _TrackingStore(fail_save=True) + monkeypatch.setattr(cli_run, "build_store", lambda _p: store) + + with pytest.raises(RuntimeError): + cli_run.cmd_run(_args(_SMOKE_SPEC, ":memory:")) + + assert store.closed is True diff --git a/tests/meta/test_cli_import_hygiene.py b/tests/meta/test_cli_import_hygiene.py new file mode 100644 index 0000000..ef33e72 --- /dev/null +++ b/tests/meta/test_cli_import_hygiene.py @@ -0,0 +1,97 @@ +"""Structural guardrail: read-only CLI commands stay off the model stack and the +verdict resolver. + +A diagnostic or artifact-reading command (``doctor``, ``verify``, ``replay``, +``inspect``, ``diff``, ``history``, ``timeline``, ``matrix``, ``export``) reads +preserved evidence. It must not load either of: + +* **the model-execution stack** (``litellm`` / the execution adapter). Loading it + for a read-only command is dead weight, and ``litellm`` emits import-time + warnings — exactly the noise that first prompted this guard. +* **the verdict resolver** (``falsifyai.verdict.resolver``). The verdict is + assigned once, at ``run`` time, and preserved in the artifact; a consumer reads + ``case.verdict`` from what was stored and *never re-resolves* (ARCHITECTURE.md: + "Replay does NOT re-invoke the resolver"). Re-importing the resolver in a + consumer is the first step toward silently re-deriving a claim the artifact + already preserves. Consumers may import the ``Verdict`` *enum* + (``falsifyai.verdict.models``) to read and compare stored verdicts — only the + resolver *module* is forbidden. + +Only ``run`` and ``minimize`` legitimately execute a model and resolve verdicts; +both are intentionally absent from the list below. + +This centralizes a guarantee that previously lived as per-command assertions +scattered across the command test files using two weaker techniques (in-process +``sys.modules`` deletion, and AST direct-import scans that miss transitive +imports). The dispatcher (``falsifyai/cli/main.py``) enforces the +no-model-stack half by importing each command module lazily inside its dispatch +branch; importing ``falsifyai.cli.main`` is the key case — if a command import +leaks back to module top level, importing the dispatcher pulls the forbidden +module and this test fails. + +A subprocess per module is mandatory: ``sys.modules`` is process-global, so once +any in-process test imports ``run``, both litellm and the resolver are resident +and an in-process check would be meaningless. +""" + +import subprocess +import sys + +import pytest + +# Read-only / consumer commands plus the dispatcher. ``run`` and ``minimize`` are +# intentionally absent — they execute the model and resolve verdicts, so they +# legitimately import both the model stack and the resolver. +READ_ONLY_MODULES = [ + "falsifyai.cli.main", + "falsifyai.cli.replay", + "falsifyai.cli.inspect", + "falsifyai.cli.diff", + "falsifyai.cli.history", + "falsifyai.cli.timeline", + "falsifyai.cli.matrix", + "falsifyai.cli.verify", + "falsifyai.cli.export", + "falsifyai.cli.doctor", +] + +# The model-execution stack and the verdict resolver: a read-only consumer must +# import neither. The enum module (``falsifyai.verdict.models``) is deliberately +# NOT here — consumers read stored verdicts from it. +FORBIDDEN = ( + "litellm", + "falsifyai.execution.litellm_adapter", + "falsifyai.verdict.resolver", +) + +# Imported in a clean interpreter; prints leaked module names and exits non-zero. +_PROBE = ( + "import importlib, sys\n" + "importlib.import_module(sys.argv[1])\n" + "forbidden = (\n" + " 'litellm',\n" + " 'falsifyai.execution.litellm_adapter',\n" + " 'falsifyai.verdict.resolver',\n" + ")\n" + "leaked = [m for m in forbidden if m in sys.modules]\n" + "if leaked:\n" + " sys.stderr.write(', '.join(leaked))\n" + " sys.exit(1)\n" +) + + +@pytest.mark.parametrize("module", READ_ONLY_MODULES) +def test_read_only_command_does_not_import_model_stack_or_resolver(module: str) -> None: + proc = subprocess.run( + [sys.executable, "-c", _PROBE, module], + capture_output=True, + text=True, + ) + assert proc.returncode == 0, ( + f"{module} transitively imported a forbidden module ({proc.stderr.strip()}). " + f"Read-only commands must import none of {FORBIDDEN}: the model stack is dead " + f"weight, and the resolver must not be re-entered (consumers read the stored " + f"verdict, they never re-resolve). If this is falsifyai.cli.main, a command " + f"import probably leaked back to module top level instead of staying in its " + f"lazy dispatch branch." + ) diff --git a/tests/unit/test_cli_diff_sharpening.py b/tests/unit/test_cli_diff_sharpening.py index c429ec7..736fd46 100644 --- a/tests/unit/test_cli_diff_sharpening.py +++ b/tests/unit/test_cli_diff_sharpening.py @@ -15,7 +15,6 @@ - Exit-code priority: 5 beats 6 - --show-timeline: all rows visible, markers, numeric delta, exit-code parity - Composition: --strict --show-timeline - - Architectural assertion: diff.py must NOT import verdict.resolver """ import argparse @@ -615,32 +614,3 @@ def test_strict_and_show_timeline_exit_6_with_full_row_list(monkeypatch, capsys) assert rc == 6 assert "c1" in out assert "c2" in out - - -# --------------------------------------------------------------------------- -# Architectural assertion: diff.py must NOT import verdict.resolver -# --------------------------------------------------------------------------- - - -def test_diff_does_not_import_resolver() -> None: - """falsifyai.cli.diff must not transitively import falsifyai.verdict.resolver. - - Enforces the consumer-surface separation in §11 of PR-28 plan: diff is a - pure reader of preserved artifacts. Re-resolving on read violates the - no-re-resolution invariant and the architectural rule from CLAUDE.md. - """ - import sys - - for mod_name in list(sys.modules): - if mod_name.startswith("falsifyai.cli.diff"): - del sys.modules[mod_name] - if mod_name == "falsifyai.verdict.resolver": - del sys.modules[mod_name] - - import falsifyai.cli.diff # noqa: F401 - - assert "falsifyai.verdict.resolver" not in sys.modules, ( - "falsifyai.cli.diff must not import falsifyai.verdict.resolver " - "(re-resolving violates the preservation guarantee). " - "Read case.verdict from the loaded artifact instead." - ) diff --git a/tests/unit/test_cli_doctor.py b/tests/unit/test_cli_doctor.py index 614c966..8a1a46e 100644 --- a/tests/unit/test_cli_doctor.py +++ b/tests/unit/test_cli_doctor.py @@ -82,7 +82,7 @@ def test_render_flags_problems(self) -> None: assert "FAIL" in text assert "-> do something" in text - def test_collect_checks_has_six_rows(self, tmp_path) -> None: + def test_collect_checks_rows(self, tmp_path) -> None: checks = collect_checks(str(tmp_path / "replays.db")) labels = [c.label for c in checks] assert labels == [ @@ -91,5 +91,41 @@ def test_collect_checks_has_six_rows(self, tmp_path) -> None: "core deps", "[semantic] extra", "[nli] extra", + "store backend", "store write", ] + + +class TestStoreBackend: + def test_default_path_reports_sqlite_backend(self, tmp_path, capsys) -> None: + rc = cmd_doctor(_args(str(tmp_path / "replays.db"))) + out = capsys.readouterr().out + assert rc == 0 + assert "store backend" in out + assert "sqlite" in out + + def test_unknown_scheme_is_unhealthy_exit_3(self, capsys) -> None: + # No store plugin handles 'postgres' in this install, so backend + # resolution fails in diagnostics instead of crashing at run time. + rc = cmd_doctor(_args("postgres://host/db")) + out = capsys.readouterr().out + assert rc == 3 + assert "no backend registered" in out + + def test_memory_scheme_is_ephemeral_ok(self, capsys) -> None: + rc = cmd_doctor(_args(":memory:")) + out = capsys.readouterr().out + assert rc == 0 + assert "ephemeral" in out + + def test_registered_plugin_scheme_is_not_probed(self, capsys, monkeypatch) -> None: + # Simulate an installed plugin store without shipping one: the scheme is + # registered, but doctor must not construct it (possible side effects). + monkeypatch.setattr( + "falsifyai.replay.registry.discover_stores", + lambda: {"sqlite": object(), "memory": object(), "postgres": object()}, + ) + rc = cmd_doctor(_args("postgres://host/db")) + out = capsys.readouterr().out + assert rc == 0 + assert "plugin store; not probed" in out diff --git a/tests/unit/test_cli_export.py b/tests/unit/test_cli_export.py index 48e5f78..f4a3aa6 100644 --- a/tests/unit/test_cli_export.py +++ b/tests/unit/test_cli_export.py @@ -5,9 +5,6 @@ A ``_FakeStore`` mirrors the pattern from ``test_cli_verify.py`` so we can serve hand-constructed artifacts including intentional corruption. - -The architectural assertion ``test_export_does_not_import_resolver`` is -co-located here (matches PR-31 convention). """ import argparse @@ -347,34 +344,3 @@ def test_default_exported_at_uses_current_utc_time(tmp_path, monkeypatch) -> Non m = _read_manifest(bundle_path) exported_at = datetime.fromisoformat(m["exported_at"]) assert before <= exported_at <= after - - -# --------------------------------------------------------------------------- -# Architectural assertion (co-located, mirrors PR-31 convention) -# --------------------------------------------------------------------------- - - -def test_export_does_not_import_resolver() -> None: - """falsifyai.cli.export must not transitively import falsifyai.verdict.resolver. - - Mirrors test_diff_does_not_import_resolver and test_verify_does_not_import_resolver. - Preservation discipline: export reads case.verdict from the loaded artifact, - never re-resolves. - """ - import sys - - for mod_name in list(sys.modules): - if mod_name.startswith("falsifyai.cli.export"): - del sys.modules[mod_name] - if mod_name.startswith("falsifyai.bundle"): - del sys.modules[mod_name] - if mod_name == "falsifyai.verdict.resolver": - del sys.modules[mod_name] - - import falsifyai.cli.export # noqa: F401 - - assert "falsifyai.verdict.resolver" not in sys.modules, ( - "falsifyai.cli.export must not import falsifyai.verdict.resolver " - "(re-resolving violates the preservation guarantee). " - "Read case.verdict from the loaded artifact instead." - ) diff --git a/tests/unit/test_cli_history.py b/tests/unit/test_cli_history.py index 61c976d..667c069 100644 --- a/tests/unit/test_cli_history.py +++ b/tests/unit/test_cli_history.py @@ -257,35 +257,6 @@ def test_history_unknown_case_id_raises(monkeypatch) -> None: history_module.cmd_history(_args(case_id="nonexistent_case")) -# --------------------------------------------------------------------------- -# Architectural assertion (§12.2) -# --------------------------------------------------------------------------- - - -def test_history_does_not_import_resolver() -> None: - """history.py must not transitively import the resolver module. - - Same architectural rule as inspect (PR-19): consumer surfaces over - preserved evidence never re-resolve. The verdict shown is the one - stored at run time; re-resolving would violate EVIDENCE.md §5.1. - """ - import sys - - for mod_name in list(sys.modules): - if mod_name.startswith("falsifyai.cli.history"): - del sys.modules[mod_name] - if mod_name == "falsifyai.verdict.resolver": - del sys.modules[mod_name] - - import falsifyai.cli.history # noqa: F401 - - assert "falsifyai.verdict.resolver" not in sys.modules, ( - "falsifyai.cli.history must not import falsifyai.verdict.resolver " - "(re-resolving violates the preservation guarantee). Read case.verdict " - "from the loaded artifact instead." - ) - - # --------------------------------------------------------------------------- # Exit code (E1) # --------------------------------------------------------------------------- diff --git a/tests/unit/test_cli_inspect.py b/tests/unit/test_cli_inspect.py index 13cc8ab..02004fa 100644 --- a/tests/unit/test_cli_inspect.py +++ b/tests/unit/test_cli_inspect.py @@ -350,36 +350,6 @@ def test_session_not_found_raises_infrastructure_error(monkeypatch) -> None: inspect_module.cmd_inspect(_args(session_id="never-saved")) -# --------------------------------------------------------------------------- -# Architectural assertion (§12.1 + §12.3) — load-bearing -# --------------------------------------------------------------------------- - - -def test_inspect_does_not_import_resolver() -> None: - """inspect.py must not transitively import the resolver module. - - Enforces the architectural rule from §5 of the plan: inspect is pure - consumer surface. The verdict shown is the one stored at run time; - re-resolving on read would violate EVIDENCE.md §5.1 (immutability). - """ - import sys - - # Clear any prior cli.inspect import so we observe a fresh import graph. - for mod_name in list(sys.modules): - if mod_name.startswith("falsifyai.cli.inspect"): - del sys.modules[mod_name] - if mod_name == "falsifyai.verdict.resolver": - del sys.modules[mod_name] - - import falsifyai.cli.inspect # noqa: F401 - - assert "falsifyai.verdict.resolver" not in sys.modules, ( - "falsifyai.cli.inspect must not import falsifyai.verdict.resolver " - "(re-resolving violates the preservation guarantee). If you need " - "verdict information, read it from the stored artifact." - ) - - def test_inspect_does_not_crash_on_unicode_model_outputs(monkeypatch) -> None: """LLM outputs routinely contain Unicode (e.g. U+202F narrow no-break space) that non-UTF-8 terminals (Windows cp1252) cannot encode. inspect must diff --git a/tests/unit/test_cli_main.py b/tests/unit/test_cli_main.py index bfc1416..3e0bbd7 100644 --- a/tests/unit/test_cli_main.py +++ b/tests/unit/test_cli_main.py @@ -42,7 +42,9 @@ def test_cli_error_is_caught_and_returns_exit_code(monkeypatch, capsys) -> None: def _raise(args): # noqa: ANN001 raise SpecError("bad spec", exit_code=3) - monkeypatch.setattr(cli_main.run_cmd, "cmd_run", _raise) + # main() imports the run module lazily inside its dispatch branch, so patch + # the source attribute rather than a (no-longer-existing) main-level alias. + monkeypatch.setattr("falsifyai.cli.run.cmd_run", _raise) rc = cli_main.main(["run", "missing.yaml"]) assert rc == 3 captured = capsys.readouterr() diff --git a/tests/unit/test_cli_matrix.py b/tests/unit/test_cli_matrix.py index 366984e..df79613 100644 --- a/tests/unit/test_cli_matrix.py +++ b/tests/unit/test_cli_matrix.py @@ -1,9 +1,7 @@ """Tests for falsifyai.cli.matrix (reliability matrix).""" import argparse -import ast import io -from pathlib import Path from types import SimpleNamespace import falsifyai.cli.matrix as matrix_mod @@ -64,18 +62,6 @@ def test_render_contains_rows_and_legend() -> None: assert "anthropic:claude" in out -def test_matrix_module_does_not_import_resolver() -> None: - """matrix is a consumer surface; it must never re-resolve verdicts.""" - source = ast.parse(Path(matrix_mod.__file__).read_text(encoding="utf-8")) - imported = set() - for node in ast.walk(source): - if isinstance(node, ast.ImportFrom) and node.module: - imported.add(node.module) - elif isinstance(node, ast.Import): - imported.update(alias.name for alias in node.names) - assert "falsifyai.verdict.resolver" not in imported - - def test_cmd_matrix_unknown_session_raises(tmp_path) -> None: from falsifyai.cli.errors import InfrastructureError diff --git a/tests/unit/test_cli_store_lifecycle.py b/tests/unit/test_cli_store_lifecycle.py new file mode 100644 index 0000000..7c1eb59 --- /dev/null +++ b/tests/unit/test_cli_store_lifecycle.py @@ -0,0 +1,180 @@ +"""Centralized guardrail: every read-only CLI consumer closes its ReplayStore. + +Each consumer command builds a store via ``build_store`` and owns its lifecycle: +the store must be closed in a ``finally`` block whether the command returns +normally or a post-construction store read raises. ``SQLiteStore`` holds a file +handle; leaking it across a long-lived process — or, on Windows, blocking a +later unlink of the database file — is the failure this locks out. + +This is a single parametrized harness rather than a close assertion duplicated +across eight command test files. ``run`` (and ``minimize``) are intentionally +absent: they are *producers* that orchestrate generation -> interpretation -> +preservation, not read-only consumers, and their store lifecycle is asserted in +their own test file where the execution stack is already mocked. Folding them in +here would blur the producer/consumer boundary the harness exists to protect. + +Two situations matter, for every consumer: + +* **normal return** — the ``try`` block completes (any exit code); ``finally`` + must close. This harness asserts the close, not the exit code — exit semantics + are each command's own tests' job. +* **read failure** — a store read raises after construction; ``finally`` must + still close, and the exception must propagate. + +The store stand-in only ever returns/raises; it never reaches a real database, +so the harness stays decoupled from store backends and from each command's +rendering details. +""" + +import argparse +from collections.abc import Iterator + +import pytest + +import falsifyai.cli.diff as cli_diff +import falsifyai.cli.export as cli_export +import falsifyai.cli.history as cli_history +import falsifyai.cli.inspect as cli_inspect +import falsifyai.cli.matrix as cli_matrix +import falsifyai.cli.replay as cli_replay +import falsifyai.cli.timeline as cli_timeline +import falsifyai.cli.verify as cli_verify +from falsifyai.cli.errors import InfrastructureError +from falsifyai.replay.models import ReplayArtifact +from falsifyai.replay.protocol import SessionNotFoundError +from tests.fixtures.build_artifact import make_artifact + +_SESSION_ID = "11111111-1111-1111-1111-111111111111" + +# Exceptions a consumer may raise once a post-construction read fails: most wrap +# the miss into InfrastructureError; the query-based commands (history, timeline) +# let SessionNotFoundError propagate. Either way the store must already be closed. +_READ_FAILURE_EXC = (InfrastructureError, SessionNotFoundError) + + +class _TrackingStore: + """ReplayStore stand-in that records ``close()`` and can fail reads on demand. + + ``raise_on_read=True`` makes both read methods raise ``SessionNotFoundError`` + *after* construction, simulating a store that built fine but whose read + fails. ``artifacts`` stays directly readable so a test can derive ids / case + ids for argument construction without tripping the forced failure. + """ + + def __init__(self, artifacts: list[ReplayArtifact], *, raise_on_read: bool = False) -> None: + self.artifacts = list(artifacts) + self._by_id = {a.session_id: a for a in artifacts} + self.raise_on_read = raise_on_read + self.closed = False + + def load_session(self, session_id: str) -> ReplayArtifact: + if self.raise_on_read or session_id not in self._by_id: + raise SessionNotFoundError(session_id) + return self._by_id[session_id] + + def query_sessions(self, **_kwargs) -> Iterator[ReplayArtifact]: + if self.raise_on_read: + raise SessionNotFoundError("forced read failure") + yield from sorted(self.artifacts, key=lambda a: a.created_at, reverse=True) + + def close(self) -> None: + self.closed = True + + +# --------------------------------------------------------------------------- +# Per-command argument builders. Each takes the served artifact (for ids / +# case ids) and a tmp_path (only export needs a real output path), and returns +# a Namespace shaped like the command's argparse output. +# --------------------------------------------------------------------------- + + +def _diff_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace( + store_path=":memory:", + baseline_session_id=a.session_id, + candidate_session_id=a.session_id, + strict=False, + show_timeline=False, + ) + + +def _export_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace( + session_id=a.session_id, + bundle=str(tmp_path / "bundle.fai.zip"), + spec_path=None, + allow_corrupted=False, + overwrite=False, + exported_at=None, + store_path=":memory:", + ) + + +def _history_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace(case_id=a.case_results[0].case_id, limit=0, store_path=":memory:") + + +def _inspect_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace(session_id=a.session_id, store_path=":memory:", case=None, full=False) + + +def _matrix_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace(session_ids=[a.session_id], store_path=":memory:") + + +def _replay_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace(store_path=":memory:", session_id=a.session_id, latest=False) + + +def _timeline_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace(case_id=a.case_results[0].case_id, limit=0, store_path=":memory:") + + +def _verify_args(a: ReplayArtifact, tmp_path) -> argparse.Namespace: + return argparse.Namespace(session_id=a.session_id, all=False, store_path=":memory:") + + +# (id, cli_module, cmd_callable, build_args). The module is needed to patch its +# ``build_store`` binding; consumers import it by name into their own namespace. +_CONSUMERS = [ + ("diff", cli_diff, cli_diff.cmd_diff, _diff_args), + ("export", cli_export, cli_export.cmd_export, _export_args), + ("history", cli_history, cli_history.cmd_history, _history_args), + ("inspect", cli_inspect, cli_inspect.cmd_inspect, _inspect_args), + ("matrix", cli_matrix, cli_matrix.cmd_matrix, _matrix_args), + ("replay", cli_replay, cli_replay.cmd_replay, _replay_args), + ("timeline", cli_timeline, cli_timeline.cmd_timeline, _timeline_args), + ("verify", cli_verify, cli_verify.cmd_verify, _verify_args), +] + +_PARAMS = [(module, cmd, build_args) for _id, module, cmd, build_args in _CONSUMERS] +_IDS = [entry[0] for entry in _CONSUMERS] + + +@pytest.mark.parametrize("module, cmd, build_args", _PARAMS, ids=_IDS) +def test_consumer_closes_store_on_completion( + module, cmd, build_args, monkeypatch, tmp_path, capsys +): + """A normal return runs the ``finally`` close. Exit code is irrelevant here.""" + artifact = make_artifact(session_id=_SESSION_ID) + store = _TrackingStore([artifact]) + monkeypatch.setattr(module, "build_store", lambda _p: store) + + cmd(build_args(artifact, tmp_path)) + + assert store.closed is True + + +@pytest.mark.parametrize("module, cmd, build_args", _PARAMS, ids=_IDS) +def test_consumer_closes_store_on_read_failure( + module, cmd, build_args, monkeypatch, tmp_path, capsys +): + """A post-construction read failure still runs the ``finally`` close.""" + artifact = make_artifact(session_id=_SESSION_ID) + store = _TrackingStore([artifact], raise_on_read=True) + monkeypatch.setattr(module, "build_store", lambda _p: store) + + with pytest.raises(_READ_FAILURE_EXC): + cmd(build_args(artifact, tmp_path)) + + assert store.closed is True diff --git a/tests/unit/test_cli_timeline.py b/tests/unit/test_cli_timeline.py index 3f15750..8373c53 100644 --- a/tests/unit/test_cli_timeline.py +++ b/tests/unit/test_cli_timeline.py @@ -1,8 +1,6 @@ """Tests for falsifyai.cli.timeline.""" import argparse -import ast -from pathlib import Path from types import SimpleNamespace import falsifyai.cli.timeline as timeline_mod @@ -50,17 +48,6 @@ def test_sparkline_maps_low_to_high() -> None: assert line[-1] == "@" # highest level -def test_timeline_module_does_not_import_resolver() -> None: - source = ast.parse(Path(timeline_mod.__file__).read_text(encoding="utf-8")) - imported = set() - for node in ast.walk(source): - if isinstance(node, ast.ImportFrom) and node.module: - imported.add(node.module) - elif isinstance(node, ast.Import): - imported.update(alias.name for alias in node.names) - assert "falsifyai.verdict.resolver" not in imported - - def test_cmd_timeline_unknown_case_raises(tmp_path) -> None: from falsifyai.cli.errors import InfrastructureError diff --git a/tests/unit/test_cli_verify.py b/tests/unit/test_cli_verify.py index 7ea5ff8..c9557fa 100644 --- a/tests/unit/test_cli_verify.py +++ b/tests/unit/test_cli_verify.py @@ -230,34 +230,3 @@ def test_all_renders_aggregate_footer(monkeypatch, capsys) -> None: assert "2 sessions" in captured.out assert "15 passed" in captured.out assert "1 failed" in captured.out - - -# --------------------------------------------------------------------------- -# Architectural: cmd_verify must not transitively import verdict.resolver -# --------------------------------------------------------------------------- - - -def test_verify_does_not_import_resolver() -> None: - """falsifyai.cli.verify must not transitively import falsifyai.verdict.resolver. - - Mirrors test_diff_does_not_import_resolver — preservation discipline: - verify is a pure reader of stored artifacts; it reads case.verdict from - the artifact, never re-resolves. - """ - import sys - - for mod_name in list(sys.modules): - if mod_name.startswith("falsifyai.cli.verify"): - del sys.modules[mod_name] - if mod_name.startswith("falsifyai.integrity"): - del sys.modules[mod_name] - if mod_name == "falsifyai.verdict.resolver": - del sys.modules[mod_name] - - import falsifyai.cli.verify # noqa: F401 - - assert "falsifyai.verdict.resolver" not in sys.modules, ( - "falsifyai.cli.verify must not import falsifyai.verdict.resolver " - "(re-resolving violates the preservation guarantee). " - "Read case.verdict from the loaded artifact instead." - )