diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f6f0392..03010cec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,66 @@ All notable changes to AgentFlow are documented in this file. failing at import. The stale-shim hazard is now written into the test's own design rules, since the next rename will ripple the same way. +* **The gate is measurable on this machine now, not only on Sundays.** + `python scripts/mutation_local.py --module ` runs one module of the + same gate locally in about two minutes. It exists because `mutmut run` calls + `sys.exit(1)` at import time on native Windows, so between weekly runs nobody + here could see a score at all — which is part of why nine consecutive red + Sundays went unnoticed. The driver never invokes the `mutmut` CLI: it reads + its targets from `scripts/mutation_report.MODULE_TARGETS`, builds the + workspace with `prepare_workspace`, generates mutants with mutmut's own + `mutate_file_contents`, and runs each one as a plain `pytest` subprocess + selected through `MUTANT_UNDER_TEST`. The gate's definition of a target is + still declared in exactly one place. +* **It reproduces CI rather than approximating it.** Measured against run + 34266462154 on `serving/semantic_layer/query/sql_builder.py`: 141 mutants, + the same population, down to the surviving mutant names. At `10e20a5` the + module scores 90.8% (128 killed, 13 survived) — the 88.7% CI last reported + plus the three mutants `2cda8da` and `10e20a5` killed since. Only pytest exit + 0 (survived) and 1 (killed) count as verdicts; anything else is reported as a + harness failure and fails the run, where `mutation_report.py` counts exit 3 + as a kill. That is the one deliberate divergence, and `CONTRIBUTING.md` says + so rather than claiming exact parity. +* **A score you can trust to be about your own tree.** Three failure modes are + closed by construction: the workspace is stamped with the root, the module, + its source, the materialized package tree, the target's tests and + `pyproject.toml`, and is rebuilt whenever any of those move, so a second run + never reports the first one's sources; a mutant that comes back without a + verdict is retried once serially before it is called a harness failure, so + the number does not drift with machine load; and a `--workspace` that is a + checkout — this repository, anything inside it, or any directory holding a + `.git` — is refused instead of emptied. The mutated module never leaves the + temp workspace: the working tree is clean after a run. + +* **`sql_builder.py` is off the threshold line, and its residue is honest.** + It cleared 90% by a single mutant (90.8%, 128 killed of 141), which is not a + margin worth keeping: the next covered line added to the module would have + put the gate back in the red for reasons unrelated to the change. Nine of the + thirteen survivors could never have been killed. Eight mutated a + `typing.cast` type argument — a cast returns its second argument untouched + and never evaluates the first — so both casts are plain annotations now and + the mutants stop existing; the ninth turned `rows = []` into `rows = None` in + a branch whose next statement is `bool(rows)`, and carries a + `# pragma: no mutate` with the reason above it. The remaining four were the + `dialect="duckdb"` argument, and two of them are now dead: DuckDB list + indexing is 1-based where sqlglot's default dialect is not, so + `list_value(1, 2)[1]` read without the dialect comes back out of the scoper + as `[2]` — the tenant scoper would have changed which element the query asked + for while it added a WHERE clause. The module measures 98.4% (124 killed of + 126) with `scripts/mutation_local.py` on py3.13. +* **The two mutants still alive are named in the test file, not suppressed.** + `_scope_sql__mutmut_41` and `_43` drop the dialect from the parse of the + relation `_qualify_table` generated itself, and that string has one fixed + shape which — parsed with the dialect or without it — renders identically + under the `sql(dialect="duckdb")` `_scope_sql` applies on the way out, so no + input reaches them with a difference to observe. Not the same as neutral: the + *default-dialect render* of that shape rewrites `EXCLUDE` to `EXCEPT`, which + is why the mutants on the render itself stay killable and dead. + They are not equivalent — a `_qualify_table` that ever emitted + DuckDB-specific syntax would make them killable — so they get a written + record of what was tried and came out identical rather than a pragma that + would outlive its reason. + ### Terraform — an exact core pin took the provider update channel down with it * **`required_version = "= 1.15.4"` broke Dependabot's terraform ecosystem the diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 674d4c24..bc9ce7a4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -148,6 +148,43 @@ replaceable runtime artifacts, not reviewed evidence or production acceptance. Promote a reviewed snapshot only under a new date-stamped identity with provenance. +`python scripts/mutation_local.py --module ` measures one module of +that same gate on this machine, so a mutation score is available before you +push instead of only after the weekly workflow. `--list-modules` prints the +targets; the score, the mutant population and the surviving mutant names match +the CI run for the same commit — up to pytest's internal-error exits, which +`scripts/mutation_report.py` counts as kills (exit 3) while this driver refuses +to score them at all — because the driver reads its targets from +`scripts/mutation_report.MODULE_TARGETS`, builds its workspace with +`prepare_workspace`, and generates mutants with mutmut's own engine +(`mutmut.mutation.file_mutation`). It needs `mutmut` installed (it is in the +`dev` extra) but never invokes the `mutmut` CLI: mutants are executed as plain +`pytest` subprocesses selected through `MUTANT_UNDER_TEST`, which is what makes +this work on native Windows, where `mutmut run` exits at import time. Expect +minutes, not seconds — one pytest process per mutant, `--jobs` in parallel. +The driver exits 1 when the module is below its threshold or when any mutant +got no verdict (only pytest exit 0 = survived and 1 = killed are verdicts; +anything else is a harness failure, never a kill). A mutant that comes back +without a verdict — usually a timeout from running `--jobs` of them at once — +is retried once serially with a longer timeout before it is reported that way, +so the score does not move with the machine's load. Its workspace and JSON +report live under the OS temp directory, outside the repository; the workspace +is reused across runs only when it is stamped with the same root, module, +module source, materialized top-level package tree, target tests and +`pyproject.toml` (which `prepare_workspace` always renders into the workspace as +a real file, carrying pytest addopts, filterwarnings and `[tool.mutmut]`), and +is rebuilt otherwise, so a second run never reports the first one's copy of +those sources. The stamp does not cover the trees `prepare_workspace` normally +symlinks — `src/`, `sdk/`, `config/`, `scripts/` and the rest of `tests/` +beyond the target's own test files — which it copies instead where the OS +refuses symlinks; on such a machine, pass a fresh `--workspace` after editing +them. A `--workspace` is emptied on rebuild, +so one that is a checkout — this repository, anything inside the tree the +sources come from, or any directory holding a `.git` — is refused instead, and +only a directory carrying the driver's own marker is ever cleared. `--root` +points it at another checkout; `--only` re-runs named mutants, written either +bare or exactly as the report prints them (`.`). + `python scripts/evaluate_trivy_policy.py` writes ignored Trivy policy summaries under `.artifacts/trivy/`. Relative `--report`, `--waivers`, and `--output` paths resolve from the project root, not the caller CWD, and every diff --git a/scripts/mutation_local.py b/scripts/mutation_local.py new file mode 100644 index 00000000..42704f34 --- /dev/null +++ b/scripts/mutation_local.py @@ -0,0 +1,857 @@ +"""Measure one module of the CI mutation gate locally, without `mutmut run`. + +`.github/workflows/mutation.yml` is the only place the gate has been +measurable: `mutmut run` refuses to start on native Windows (mutmut's +`__main__` calls `sys.exit(1)` at import time), so between weekly runs nobody +here could see a mutation score. This driver produces the same number on this +machine -- minutes, not seconds: one pytest process per mutant, `--jobs` of +them at a time -- by reusing the gate's own pieces instead of reimplementing +them: + +* `scripts.mutation_report.MODULE_TARGETS` for the module -> (threshold, tests) + mapping and `scripts.mutation_report.prepare_workspace` for the workspace, + so the definition of a target lives in exactly one place; +* `mutmut.mutation.file_mutation.mutate_file_contents` for the mutants, so the + mutant population is byte-identical to what `mutmut run` would generate; +* plain `pytest` subprocesses to execute them, selected through the + `MUTANT_UNDER_TEST` environment variable that mutmut's trampoline reads -- + which is what sidesteps the Windows guard. + +`mutmut` must be installed (it is in the `dev` extra), but this script never +invokes the `mutmut` CLI. + +The number is only worth having if it is about the code in front of you and +comparable to CI's, so two things are load-bearing beyond the mechanism: +a reused workspace is proven to be the one that was asked for (a stamp carrying +the root, the module and digests of the module's source, the whole top-level +package the workspace copies, the target's tests and the `pyproject.toml` +`prepare_workspace` renders the workspace's config from; anything else is +rebuilt), and a mutant that comes back without a verdict is retried once +serially before it is reported as a harness failure -- otherwise a timeout +under `--jobs` makes the score a function of how loaded the machine is. + +A workspace is emptied on rebuild, so `--workspace` is resolved to an absolute +path and accepted only when it is not a checkout and carries this driver's own +marker: a `pyproject.toml` marks Python projects in general, this repository +included, and is no evidence that the directory is safe to delete. + +Usage: + python scripts/mutation_local.py --list-modules + python scripts/mutation_local.py --module serving/semantic_layer/query/sql_builder.py + python scripts/mutation_local.py --module agentflow/retry.py --only , + +Exits 1 when the module scores below its declared threshold or when any mutant +could not be given a verdict; 0 otherwise. +""" + +from __future__ import annotations + +import argparse +import concurrent.futures +import hashlib +import json +import os +import shutil +import subprocess +import sys +import tempfile +import time +import uuid +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from scripts import mutation_report # noqa: E402 + +# Written into a throwaway directory that is put on the child processes' +# PYTHONPATH. It must never be shipped as a file inside the repository: a +# sitecustomize.py reachable from the project root is imported by *every* +# interpreter started there, silently changing unrelated runs. +SITECUSTOMIZE_SOURCE = '''\ +"""Let a mutmut-generated module import its trampoline on native Windows. + +The generated module does `from mutmut.mutation.trampoline import ...`, and +that module imports three names from `mutmut.__main__`. `mutmut/__main__.py` +refuses to run on Windows with `sys.exit(1)` at import time, which kills the +pytest process before a single test runs -- every mutant would then look +"killed" and the score would be a meaningless 100%. Pre-registering a stub for +that one submodule keeps the three names available and never executes the +guard. + +Generated by scripts/mutation_local.py; not part of the repository. +""" + +import sys +import types + +try: + import mutmut +except Exception: # pragma: no cover - mutmut simply is not installed + pass +else: + if "mutmut.__main__" not in sys.modules: + + class MutmutProgrammaticFailException(Exception): + pass + + def mangled_name_from_mutant_name(mutant_name: str) -> str: + assert "__mutmut_" in mutant_name, mutant_name + return mutant_name.partition("__mutmut_")[0] + + def record_trampoline_hit(name, caller=None): + """Only mutmut's `stats` pass records hits; this driver never runs it.""" + + stub = types.ModuleType("mutmut.__main__") + stub.MutmutProgrammaticFailException = MutmutProgrammaticFailException + stub.mangled_name_from_mutant_name = mangled_name_from_mutant_name + stub.record_trampoline_hit = record_trampoline_hit + sys.modules["mutmut.__main__"] = stub + mutmut.__main__ = stub +''' + +# Only these two are verdicts. pytest's remaining exit codes (2 = internal +# error / interrupted, 3 = internal error, 4 = usage, 5 = no tests collected) +# mean the harness failed, not that the mutant lived or died, and a timeout +# gets no exit code at all. +SURVIVED_EXIT_CODE = 0 +KILLED_EXIT_CODE = 1 + +BACKUP_SUFFIX = ".mutation-local-orig" +REPORT_FILENAME = "mutation-local-report.json" +STAMP_FILENAME = "mutation-local-stamp.json" +SCRATCH_DIRNAME = ".mutation-local-tmp" + +# Dropped into a workspace the moment this driver starts building one, and the +# only thing that makes a directory eligible to be emptied later. It has to be +# written before `prepare_workspace` rather than with the stamp afterwards, so +# that a prepare killed halfway still leaves a directory the next run may +# rebuild instead of one it has to refuse. +MARKER_FILENAME = ".mutation-local-workspace" +MARKER_TEXT = ( + "Built by scripts/mutation_local.py. Everything in this directory is\n" + "deleted and rebuilt whenever the driver's stamp stops matching. Do not\n" + "keep anything here.\n" +) + + +@dataclass +class ModuleRun: + """Outcome of measuring one module.""" + + module_path: Path + threshold: float + generated: int + killed: list[str] = field(default_factory=list) + survived: list[str] = field(default_factory=list) + errored: list[tuple[str, int | None]] = field(default_factory=list) + retried: list[str] = field(default_factory=list) + + def record(self, mutant_name: str, exit_code: int | None) -> None: + """File one mutant under the verdict its exit code carries.""" + verdict = classify_exit_code(exit_code) + if verdict == "survived": + self.survived.append(mutant_name) + elif verdict == "killed": + self.killed.append(mutant_name) + else: + self.errored.append((mutant_name, exit_code)) + + @property + def scored(self) -> int: + return len(self.killed) + len(self.survived) + + @property + def score(self) -> float: + return len(self.killed) / self.scored if self.scored else 0.0 + + @property + def passed(self) -> bool: + return bool(self.scored) and not self.errored and self.score >= self.threshold + + +def classify_exit_code(exit_code: int | None) -> str: + """Map a pytest exit code onto a mutant verdict. + + 0 means the tests passed with the mutant active, i.e. nothing noticed the + change: the mutant survived. 1 means at least one test failed: killed. + Anything else -- including a timeout, which arrives as None -- is a harness + failure and must never be counted as a kill. + """ + if exit_code == SURVIVED_EXIT_CODE: + return "survived" + if exit_code == KILLED_EXIT_CODE: + return "killed" + return "errored" + + +def dotted_module(module_path: Path) -> str: + """`serving/api/rate_limiter.py` -> `serving.api.rate_limiter`. + + Mutant names are `.`; the workspace mutates each + module under a top-level package name, so the repo-relative key from + MODULE_TARGETS is already the import path. + """ + return ".".join(module_path.with_suffix("").parts) + + +def write_source(path: Path, text: str) -> None: + """Write a source file without newline translation. + + The repository is LF; the default text mode on Windows would hand the + module back as CRLF, and on a symlinked workspace that lands in the working + tree as a diff nobody asked for. + """ + path.write_text(text, encoding="utf-8", newline="") + + +def write_sitecustomize(directory: Path) -> Path: + """Generate the trampoline shim into `directory` and return that directory.""" + directory.mkdir(parents=True, exist_ok=True) + write_source(directory / "sitecustomize.py", SITECUSTOMIZE_SOURCE) + return directory + + +def child_env(shim_dir: Path, mutant: str | None = None) -> dict[str, str]: + env = dict(os.environ) + existing = env.get("PYTHONPATH") + env["PYTHONPATH"] = str(shim_dir) + (os.pathsep + existing if existing else "") + env["PYTHONDONTWRITEBYTECODE"] = "1" + if mutant is not None: + env["MUTANT_UNDER_TEST"] = mutant + else: + env.pop("MUTANT_UNDER_TEST", None) + return env + + +def materialize_package(workspace: Path, module_path: Path) -> bool: + """Replace the workspace's symlinked top-level package with a real copy. + + `prepare_workspace` symlinks `serving` / `agentflow` at the real sources. + This driver overwrites the module file with each mutated version, so + through a symlink every write would land in the working tree. Returns True + when a copy was made. + """ + top = workspace / module_path.parts[0] + if not top.is_symlink(): + return False + real = top.resolve() + top.unlink() + shutil.copytree(real, top) + return True + + +def source_package_dir(module_path: Path) -> Path: + """The checkout directory `prepare_workspace` mounts as the top-level package. + + The MODULE_TARGETS keys are workspace-relative (`serving/...`, + `agentflow/...`) because those two packages are mounted at the top level; + this is the same mapping read backwards, so the driver can look at the real + sources before it decides whether an existing workspace is still about them. + """ + root = mutation_report.ROOT + top = module_path.parts[0] + if top == "agentflow": + return root / "sdk" / "agentflow" + if top == "serving": + return root / "src" / "agentflow_runtime" / "serving" + return root / top + + +def source_module_file(module_path: Path) -> Path: + """Where a MODULE_TARGETS key lives in the checkout it is measured from.""" + return source_package_dir(module_path).joinpath(*module_path.parts[1:]) + + +def tree_sha256(directory: Path) -> str: + """Digest of every source file under `directory`, path names included. + + Byte-compiled leftovers are skipped: they follow the sources they came from + and would otherwise make a workspace look stale on a machine that imported + the package once. + """ + digest = hashlib.sha256() + for path in sorted(directory.rglob("*")): + if "__pycache__" in path.parts or path.suffix == ".pyc" or not path.is_file(): + continue + digest.update(path.relative_to(directory).as_posix().encode("utf-8")) + digest.update(b"\0") + digest.update(hashlib.sha256(path.read_bytes()).digest()) + digest.update(b"\0") + return digest.hexdigest() + + +def path_sha256(path: Path) -> str: + if path.is_dir(): + return tree_sha256(path) + if path.is_file(): + return hashlib.sha256(path.read_bytes()).hexdigest() + return "missing" + + +def workspace_stamp(module_path: Path, target: mutation_report.ModuleTarget) -> dict[str, str]: + """What a workspace must have been built from for reusing it to be honest. + + Root and module because a workspace keyed only on the module's stem is + reused across `--root` checkouts and across same-named modules. The digests + because the tool's whole point is "run it before you commit": the workspace + holds a *real copy* of the whole top-level package (see + `materialize_package`), so keying on the target module's own file alone + would let an edit to any sibling -- or to the tests that do the killing -- + be measured against the previous run's copy of it. `pyproject.toml` for the + same reason: `prepare_workspace` renders the workspace's copy from the + checkout's, always as a real file, so pytest addopts, filterwarnings, plugin + toggles and `[tool.mutmut]` all reach the run through a copy that would + otherwise go stale silently. + + What `prepare_workspace` symlinks is live by construction and needs no + digest. Where the OS refuses symlinks it copies the linked trees instead + (`src`/`sdk`, `tests`, `config`, `scripts`); those copies are not digested + -- on a machine with symlinks they are links and hashing them every run + would be pure cost -- so on such a machine an edit to `src`/`sdk`, + `config`, `scripts` or to the rest of `tests` beyond the target's own test + files (which `tests_sha256` covers everywhere) needs a fresh `--workspace`. + """ + source = source_module_file(module_path) + if not source.is_file(): + raise SystemExit(f"module source not found: {source}") + tests_digest = hashlib.sha256() + for test_path in target.tests: + tests_digest.update(test_path.encode("utf-8")) + tests_digest.update(b"\0") + tests_digest.update(path_sha256(mutation_report.ROOT / test_path).encode("utf-8")) + tests_digest.update(b"\0") + return { + "root": mutation_report.ROOT.as_posix(), + "module": module_path.as_posix(), + "source_sha256": hashlib.sha256(source.read_bytes()).hexdigest(), + "package_sha256": tree_sha256(source_package_dir(module_path)), + "tests_sha256": tests_digest.hexdigest(), + "pyproject_sha256": path_sha256(mutation_report.ROOT / "pyproject.toml"), + } + + +def read_stamp(path: Path) -> dict[str, str] | None: + """The stamp `path` carries, or None if there is none to trust.""" + try: + loaded = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return None + return loaded if isinstance(loaded, dict) else None + + +def describe_stamp(stamp: dict[str, str]) -> str: + return ( + f" {stamp['module']} from {stamp['root']} " + f"(source sha256 {stamp['source_sha256'][:12]}, " + f"package sha256 {stamp['package_sha256'][:12]})" + ) + + +def remove_workspace_entry(entry: Path) -> None: + """Delete one workspace entry without ever following a link out of it. + + `prepare_workspace` symlinks `tests`, `config`, `scripts`, ... at the real + checkout -- `os.symlink` only, never a junction. `shutil.rmtree` refuses to + descend a directory symlink on Windows and following one would delete the + checkout, so links are unlinked as links, which on Windows means `rmdir` + when they point at a directory. + """ + if entry.is_symlink(): + try: + entry.unlink() + except OSError: + entry.rmdir() + return + if entry.is_dir(): + shutil.rmtree(entry) + return + entry.unlink(missing_ok=True) + + +def check_workspace_location(workspace: Path) -> None: + """Refuse a `--workspace` that is a checkout rather than a scratch directory. + + A rebuild empties the directory, so the two shapes that would cost real + work are refused before anything is deleted: a path inside the checkout the + sources come from (`--workspace .` typed in a repository root is the whole + reason this exists), and any directory carrying a `.git`. + """ + resolved = workspace.resolve() + for checkout in {ROOT, Path(mutation_report.ROOT).resolve()}: + if resolved == checkout or checkout in resolved.parents: + raise SystemExit( + f"refusing to use {resolved} as a workspace: it is the checkout {checkout} " + "or lives inside it -- pass a --workspace outside the repository" + ) + if (resolved / ".git").exists(): + raise SystemExit( + f"refusing to use {resolved} as a workspace: it holds a .git, so it is a " + "checkout -- pass a --workspace outside the repository" + ) + + +def clear_workspace(workspace: Path) -> None: + """Empty `workspace` so it can be rebuilt from the sources actually asked for. + + A rebuild deletes everything in there, and `--workspace` is a path the + caller types, so only a directory carrying this driver's own marker or + stamp is emptied. A `pyproject.toml` is not a marker of a workspace built + here: it is a marker of Python projects generally, this repository + included, which is exactly the wrong value to accept. + """ + check_workspace_location(workspace) + if not workspace.is_dir(): + return + entries = sorted(workspace.iterdir()) + ours = (workspace / MARKER_FILENAME).exists() or (workspace / STAMP_FILENAME).exists() + if entries and not ours: + raise SystemExit( + f"refusing to rebuild {workspace}: it is not empty and holds no mutation-local " + "workspace -- pass a --workspace of its own" + ) + for entry in entries: + remove_workspace_entry(entry) + + +def measure_covered_lines( + workspace: Path, + module_file: Path, + tests: tuple[str, ...], + *, + python: str, + shim_dir: Path, +) -> set[int]: + """Line numbers of `module_file` executed by `tests`. + + `mutate_only_covered_lines = true` is set in `[tool.mutmut]`, so the mutant + population -- and therefore the mutant numbering -- depends on coverage. + Skipping this step produces mutants CI never generated. + """ + data_file = workspace / ".coverage-mutation-local" + data_file.unlink(missing_ok=True) + command = [ + python, + "-m", + "coverage", + "run", + f"--data-file={data_file}", + f"--include={module_file.as_posix()}", + "-m", + "pytest", + "-q", + "-p", + "no:cacheprovider", + *tests, + ] + result = subprocess.run( + command, + cwd=workspace, + env=child_env(shim_dir), + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + print(result.stdout[-4000:]) + print(result.stderr[-2000:], file=sys.stderr) + raise SystemExit(f"baseline test run failed (exit {result.returncode})") + + from coverage import CoverageData + + coverage_data = CoverageData(basename=str(data_file)) + coverage_data.read() + for measured in coverage_data.measured_files(): + if Path(measured).resolve() == module_file.resolve(): + return set(coverage_data.lines(measured) or []) + raise SystemExit(f"coverage recorded no lines for {module_file}") + + +def generate_mutants(module_path: Path, source: str, covered_lines: set[int]) -> Any: + """Mutants for `source`, straight from the engine `mutmut run` uses. + + Imported lazily so the driver's own unit tests can stub this out without + mutmut installed -- and so `--list-modules` works without it too. + """ + from mutmut.mutation.file_mutation import mutate_file_contents + + return mutate_file_contents(module_path.as_posix(), source, covered_lines) + + +def run_mutant( + workspace: Path, + tests: tuple[str, ...], + mutant_name: str, + *, + python: str, + shim_dir: Path, + basetemp: Path, + timeout: float, +) -> tuple[str, int | None]: + """Run `tests` with one mutant active; return its pytest exit code. + + Each mutant gets its own `--basetemp`: pytest wipes and recreates that + directory at startup, so concurrent runs sharing one abort each other and + exit 2. + """ + command = [ + python, + "-m", + "pytest", + "-x", + "-q", + "--no-header", + "-p", + "no:cacheprovider", + f"--basetemp={basetemp}", + *tests, + ] + try: + result = subprocess.run( + command, + cwd=workspace, + env=child_env(shim_dir, mutant_name), + capture_output=True, + text=True, + timeout=timeout, + check=False, + ) + except subprocess.TimeoutExpired: + return mutant_name, None + return mutant_name, result.returncode + + +def retry_missing_verdicts( + run: ModuleRun, + workspace: Path, + tests: tuple[str, ...], + *, + python: str, + shim_dir: Path, + scratch: Path, + timeout: float, +) -> None: + """Give every mutant that came back without a verdict one serial retry. + + A timeout is not a verdict and must never be counted as a kill -- but under + `--jobs` it usually says more about the machine than about the mutant: at + six concurrent pytest processes four sql_builder mutants CI kills hit the + parallel timeout here and cost the run 0.4 points. One uncontended second + chance, with a longer timeout, removes contention as the explanation. + Whatever still has no verdict afterwards is the harness failure it looks + like, and is reported as one. + """ + pending = list(run.errored) + run.errored = [] + print( + f"no verdict for {len(pending)} mutant(s) in the parallel pass -- " + f"retrying serially (timeout {timeout:.0f}s)", + flush=True, + ) + for index, (mutant_name, _) in enumerate(pending): + _, exit_code = run_mutant( + workspace, + tests, + mutant_name, + python=python, + shim_dir=shim_dir, + basetemp=scratch / f"retry{index}", + timeout=timeout, + ) + run.retried.append(mutant_name) + run.record(mutant_name, exit_code) + + +def measure_module( + module_path: Path, + target: mutation_report.ModuleTarget, + workspace: Path, + shim_dir: Path, + *, + python: str, + jobs: int, + timeout: float, + retry_timeout: float | None = None, + only: set[str] | None = None, +) -> ModuleRun: + """Prepare or reuse a workspace, generate the mutants, and run them all.""" + if retry_timeout is None: + retry_timeout = timeout * 3 + + check_workspace_location(workspace) + stamp = workspace_stamp(module_path, target) + stamp_path = workspace / STAMP_FILENAME + # A previous run killed mid-flight would leave the module mutated; the + # pristine copy taken when the workspace was built is what it is restored + # from, so a reused workspace never mutates a mutant. + backup = workspace / f"{module_path.stem}{BACKUP_SUFFIX}" + reusable = ( + read_stamp(stamp_path) == stamp + and (workspace / "pyproject.toml").exists() + and backup.exists() + ) + if reusable: + print(f"workspace reused: {workspace}") + else: + # Anything else -- a workspace left by a different --root, by another + # module, by the edit made since, or by a prepare that never finished + # -- gets rebuilt. Reusing it would report a number about sources + # nobody asked for, and a green that predates your change is worse than + # no number at all. + clear_workspace(workspace) + workspace.mkdir(parents=True, exist_ok=True) + write_source(workspace / MARKER_FILENAME, MARKER_TEXT) + mutation_report.prepare_workspace(workspace, module_path, target) + print(f"workspace prepared: {workspace}") + print(describe_stamp(stamp)) + materialize_package(workspace, module_path) + + module_file = workspace / module_path + if not backup.exists(): + write_source(backup, module_file.read_text(encoding="utf-8")) + source = backup.read_text(encoding="utf-8") + write_source(module_file, source) + if not reusable: + # Stamped only now the workspace is complete, pristine copy included: + # an interrupted prepare must not leave a stamp the next run believes. + write_source(stamp_path, json.dumps(stamp, indent=2) + "\n") + + started = time.monotonic() + covered = measure_covered_lines( + workspace, module_file, target.tests, python=python, shim_dir=shim_dir + ) + print(f"covered lines: {len(covered)} ({time.monotonic() - started:.1f}s)") + + mutated = generate_mutants(module_path, source, covered) + names = list(mutated.mutant_names) + print(f"generated mutants: {len(names)}") + + dotted = dotted_module(module_path) + selected = names + if only is not None: + # The engine names mutants `x__mutmut_2`; everything this driver prints + # -- the survivor list and the JSON report -- carries the dotted module + # in front. A name copied out of that report has to select the mutant it + # names, so the prefix is stripped when it is there and both spellings + # are accepted. + prefix = f"{dotted}." + wanted = {name.removeprefix(prefix) for name in only} + selected = [name for name in names if name in wanted] + missing = wanted - set(selected) + if missing: + print(f"not generated (ignored): {sorted(prefix + name for name in missing)}") + + run = ModuleRun(module_path=module_path, threshold=target.threshold, generated=len(names)) + # pytest wipes and recreates its `--basetemp` at startup, so the scratch is + # namespaced per invocation on top of per mutant: a workspace whose stamp + # matches is deliberately shared, and two runs of the same module -- the + # command typed in a second terminal -- would otherwise hand pytest the same + # `mutant` directories and abort each other out of a verdict. + scratch = workspace / SCRATCH_DIRNAME / f"run-{os.getpid()}-{uuid.uuid4().hex[:8]}" + write_source(module_file, mutated.code) + started = time.monotonic() + try: + with concurrent.futures.ThreadPoolExecutor(max_workers=jobs) as pool: + futures = [ + pool.submit( + run_mutant, + workspace, + target.tests, + f"{dotted}.{mutant}", + python=python, + shim_dir=shim_dir, + basetemp=scratch / f"mutant{index}", + timeout=timeout, + ) + for index, mutant in enumerate(selected) + ] + for done, future in enumerate(concurrent.futures.as_completed(futures), start=1): + run.record(*future.result()) + if done % 20 == 0 or done == len(selected): + elapsed = time.monotonic() - started + print(f" {done}/{len(selected)} ({elapsed:.0f}s)", flush=True) + if run.errored: + retry_missing_verdicts( + run, + workspace, + target.tests, + python=python, + shim_dir=shim_dir, + scratch=scratch, + timeout=retry_timeout, + ) + finally: + # Always hand the workspace back unmutated, including on Ctrl-C. + write_source(module_file, source) + # This invocation's scratch is nobody else's to read, and a reused + # workspace would otherwise collect one tree per run. + shutil.rmtree(scratch, ignore_errors=True) + return run + + +def report_payload(run: ModuleRun) -> dict: + return { + "module": run.module_path.as_posix(), + "threshold": run.threshold, + "score": run.score, + "generated": run.generated, + "killed": len(run.killed), + "survived": sorted(run.survived), + "errored": [[name, code] for name, code in sorted(run.errored)], + "retried": sorted(run.retried), + "passed": run.passed, + } + + +def print_report(run: ModuleRun) -> None: + print() + print( + f"{run.module_path.name}: score={run.score:.1%} threshold={run.threshold:.0%} " + f"(killed={len(run.killed)}, survived={len(run.survived)})" + ) + for mutant_name in sorted(run.survived): + print(f" - {mutant_name}") + if run.retried: + print(f" {len(run.retried)} mutant(s) had no verdict in parallel; retried serially") + if run.errored: + print(f" no verdict for {len(run.errored)} mutant(s) -- harness failure, not a kill:") + for mutant_name, exit_code in sorted(run.errored): + print(f" ! {mutant_name} exit={'timeout' if exit_code is None else exit_code}") + + +def default_workspace(module_path: Path) -> Path: + # Outside the repository: the workspace holds a full copy of the mutated + # package plus pytest scratch directories. Keyed on the module's stem, so a + # different --root or a different module lands on the same path -- which is + # exactly what the stamp check in `measure_module` is there to catch. + return Path(tempfile.gettempdir()) / "agentflow-mutation-local" / module_path.stem + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser( + description=( + "Measure the CI mutation gate for one module locally, using mutmut's " + "mutation engine but plain pytest subprocesses (no `mutmut run`)." + ), + ) + parser.add_argument( + "--module", + help="MODULE_TARGETS key, e.g. serving/semantic_layer/query/sql_builder.py", + ) + parser.add_argument( + "--list-modules", + action="store_true", + help="print the gate's modules with their thresholds and exit", + ) + parser.add_argument( + "--root", + type=Path, + default=None, + help="checkout to build the workspace from (default: this script's repository)", + ) + parser.add_argument( + "--workspace", + type=Path, + default=None, + help=( + "where to build the workspace (default: a temp directory). Resolved to an " + "absolute path, rebuilt (emptied) unless its stamp matches this root, " + "module, package tree, tests and pyproject.toml, and refused outright when " + "it is a checkout" + ), + ) + parser.add_argument( + "--only", + default=None, + help="comma-separated mutant names to run, bare or as printed (`.`)", + ) + parser.add_argument("--jobs", type=int, default=4, help="concurrent pytest processes") + parser.add_argument( + "--timeout", + type=float, + default=300.0, + help="seconds a single mutant may run before it is reported as errored", + ) + parser.add_argument( + "--retry-timeout", + type=float, + default=None, + help=( + "seconds for the serial second chance given to a mutant that came back " + "without a verdict (default: 3x --timeout)" + ), + ) + parser.add_argument( + "--python", + default=sys.executable, + help="interpreter for the child pytest runs (default: the current one)", + ) + parser.add_argument( + "--json", + type=Path, + default=None, + help=f"write the JSON report here (default: /{REPORT_FILENAME})", + ) + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv) + + if args.list_modules: + for declared_path, declared in mutation_report.MODULE_TARGETS.items(): + print(f"{declared_path.as_posix()} threshold={declared.threshold:.0%}") + return 0 + if not args.module: + print("--module is required (see --list-modules)", file=sys.stderr) + return 2 + + module_path = Path(args.module) + target: mutation_report.ModuleTarget | None = mutation_report.MODULE_TARGETS.get(module_path) + if target is None: + print(f"unknown module: {args.module} (see --list-modules)", file=sys.stderr) + return 2 + + if args.root is not None: + # MODULE_TARGETS still comes from this checkout; only the sources the + # workspace is built from move. + mutation_report.ROOT = Path(args.root).resolve() + + # Resolved before anything reads it: `module_file` inherits this path, and a + # relative one turns into a `--include=` pattern coverage never matches + # against the absolute paths it records -- the module then measures as + # uncovered, with an error message about the wrong thing. + workspace = Path(args.workspace or default_workspace(module_path)).resolve() + only = None + if args.only: + only = {name.strip() for name in args.only.split(",") if name.strip()} + + with tempfile.TemporaryDirectory(prefix="agentflow-mutation-shim-") as shim_root: + shim_dir = write_sitecustomize(Path(shim_root)) + run = measure_module( + module_path, + target, + workspace, + shim_dir, + python=args.python, + jobs=max(1, args.jobs), + timeout=args.timeout, + retry_timeout=args.retry_timeout, + only=only, + ) + + print_report(run) + report_path = args.json or workspace / REPORT_FILENAME + report_path.parent.mkdir(parents=True, exist_ok=True) + report_path.write_text( + json.dumps(report_payload(run), indent=2) + "\n", encoding="utf-8", newline="\n" + ) + print(f"report: {report_path}") + return 0 if run.passed else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/agentflow_runtime/serving/semantic_layer/query/sql_builder.py b/src/agentflow_runtime/serving/semantic_layer/query/sql_builder.py index 074a9b3b..1db02878 100644 --- a/src/agentflow_runtime/serving/semantic_layer/query/sql_builder.py +++ b/src/agentflow_runtime/serving/semantic_layer/query/sql_builder.py @@ -1,7 +1,6 @@ from __future__ import annotations import re -from typing import cast import sqlglot from sqlglot import exp @@ -97,7 +96,14 @@ def _holds_foreign_tenant_rows(self: SQLBuilderHost, physical: str) -> bool: One probe per table per process (cached), like the ``_table_columns`` probe the old guard used. """ - cache = cast("dict[str, bool] | None", getattr(self, "_foreign_tenant_cache", None)) + # Declared rather than `cast(...)`. `typing.cast` returns its second + # argument untouched and never evaluates the first, so every mutant of a + # cast's type argument is equivalent by construction: no test can tell + # `cast("dict[str, bool] | None", x)` from `cast(None, x)`. Eight such + # mutants — four here, four in `_qualify_table` — sat in the mutation + # gate's denominator (CI run 34266462154) pretending to be gaps. An + # annotation says the same thing to mypy and leaves nothing to mutate. + cache: dict[str, bool] | None = getattr(self, "_foreign_tenant_cache", None) if cache is not None and physical in cache: return cache[physical] @@ -112,7 +118,13 @@ def _holds_foreign_tenant_rows(self: SQLBuilderHost, physical: str) -> bool: ) except BackendExecutionError: # Not materialized yet, or no tenant column: nothing to leak. - rows = [] + # + # `rows = None` is the only mutant of this line and it is equivalent: + # `rows` is read exactly once, by the `bool(rows)` below, and + # `bool([]) == bool(None) == False`. Marked so it stops being + # generated — the gate's denominator should hold only mutants a test + # could kill. + rows = [] # pragma: no mutate found = bool(rows) if cache is not None: cache[physical] = found @@ -141,9 +153,11 @@ def _qualify_table(self: SQLBuilderHost, table_name: str, tenant_id: str | None) promises and the two stores stay column-identical. """ predicate = self._tenant_predicate(tenant_id) - cache = cast( - "dict[tuple[str, str | None], str] | None", - getattr(self, "_qualified_table_cache", None), + # Declared, not `cast(...)`, for the reason spelled out in + # `_holds_foreign_tenant_rows`: a cast's type argument is erased at + # runtime, so its mutants are unkillable by construction. + cache: dict[tuple[str, str | None], str] | None = getattr( + self, "_qualified_table_cache", None ) cache_key = (table_name, predicate) if cache is not None and cache_key in cache: diff --git a/tests/sdk/test_retry.py b/tests/sdk/test_retry.py index ce6f83aa..a612378c 100644 --- a/tests/sdk/test_retry.py +++ b/tests/sdk/test_retry.py @@ -40,9 +40,68 @@ def test_is_retryable_method_only_idempotent(): assert is_retryable_method("HEAD") is True assert is_retryable_method("PUT") is True assert is_retryable_method("DELETE") is True + assert is_retryable_method("OPTIONS") is True assert is_retryable_method("POST") is False +def test_is_retryable_method_normalizes_the_verb(): + # Both SDK clients hand this whatever the caller wrote. + assert is_retryable_method("get") is True + assert is_retryable_method("post") is False + + +# --------------------------------------------------------------------------- # +# POST with an Idempotency-Key. This branch decides whether a write is replayed +# after a 429/502/503/504, so getting it wrong duplicates the write — and it had +# no tests at all, which is why retry.py sat at exactly its 75% mutation +# threshold (run 34265359911: 5 survivors, all in is_retryable_method). +# --------------------------------------------------------------------------- # + + +def test_post_is_retryable_when_a_mapping_carries_an_idempotency_key(): + assert is_retryable_method("POST", headers={"Idempotency-Key": "abc"}) is True + + +def test_post_idempotency_key_is_matched_case_insensitively(): + # HTTP header names are case-insensitive and every client spells this one + # differently; a case-sensitive match would silently stop retrying. + assert is_retryable_method("POST", headers={"IDEMPOTENCY-KEY": "abc"}) is True + assert is_retryable_method("POST", headers={"idempotency-key": "abc"}) is True + + +def test_post_idempotency_key_is_found_among_other_headers(): + # One matching header is enough — the check is `any`, not `all`. + headers = {"Content-Type": "application/json", "Idempotency-Key": "abc"} + assert is_retryable_method("POST", headers=headers) is True + + +def test_post_is_not_retryable_on_a_merely_similar_header(): + assert is_retryable_method("POST", headers={"Idempotency": "abc"}) is False + assert is_retryable_method("POST", headers={"X-Request-Id": "abc"}) is False + + +def test_post_is_not_retryable_without_usable_headers(): + assert is_retryable_method("POST", headers=None) is False + assert is_retryable_method("POST", headers={}) is False + + +def test_post_accepts_the_idempotency_key_from_a_header_sequence(): + # httpx hands headers over as pairs, not a mapping. + headers = [("Content-Type", "application/json"), ("Idempotency-Key", "abc")] + assert is_retryable_method("POST", headers=headers) is True + + +def test_header_sequence_matches_on_the_name_not_the_value(): + # A pair whose *value* is the key name must not count. + assert is_retryable_method("POST", headers=[("X-Header", "Idempotency-Key")]) is False + assert is_retryable_method("POST", headers=[("Content-Type", "application/json")]) is False + + +def test_a_non_idempotent_verb_other_than_post_ignores_the_key(): + # The header rescues POST only; PATCH is not made safe by announcing one. + assert is_retryable_method("PATCH", headers={"Idempotency-Key": "abc"}) is False + + def test_retryable_statuses(): assert 429 in RETRYABLE_STATUS assert 503 in RETRYABLE_STATUS diff --git a/tests/unit/test_ci_soak_foundation.py b/tests/unit/test_ci_soak_foundation.py index 2591b50a..f0c2e8b5 100644 --- a/tests/unit/test_ci_soak_foundation.py +++ b/tests/unit/test_ci_soak_foundation.py @@ -2,9 +2,11 @@ import hashlib import json +import shutil import subprocess from pathlib import Path +import pytest import yaml PROJECT_ROOT = Path(__file__).resolve().parents[2] @@ -166,6 +168,11 @@ def test_soak_overlay_wires_consumer_groups_and_ready_api() -> None: assert "/health/ready" in " ".join(str(value) for value in api["healthcheck"]["test"]) +@pytest.mark.requires_docker +@pytest.mark.skipif( + shutil.which("docker") is None, + reason="merging the soak compose files needs the docker CLI; CI has it, a dev box need not", +) def test_merged_soak_compose_overrides_api_healthcheck_for_background_consumers() -> None: services = _merged_compose()["services"] diff --git a/tests/unit/test_mutation_local.py b/tests/unit/test_mutation_local.py new file mode 100644 index 00000000..bfdf3fe8 --- /dev/null +++ b/tests/unit/test_mutation_local.py @@ -0,0 +1,927 @@ +"""Unit tests for the local mutation driver's own logic. + +Everything that costs minutes -- the mutmut engine and the pytest subprocesses +-- is stubbed here. A real mutation run belongs on the command line +(`python scripts/mutation_local.py --module ...`), not in the unit suite. +""" + +from __future__ import annotations + +import hashlib +import json +import os +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +import scripts.mutation_local as mutation_local +import scripts.mutation_report as mutation_report + +TARGET = mutation_report.ModuleTarget(threshold=0.90, tests=("tests/unit/test_thing.py",)) +MODULE_PATH = Path("pkg/thing.py") +PRISTINE = "VALUE = 1\nOTHER = 2\n" +MUTATED = "VALUE = 2\nOTHER = 3\n" + + +@pytest.mark.parametrize( + ("exit_code", "expected"), + [ + (0, "survived"), + (1, "killed"), + (2, "errored"), + (3, "errored"), + (5, "errored"), + (-9, "errored"), + (None, "errored"), + ], +) +def test_classify_exit_code_treats_only_zero_and_one_as_verdicts(exit_code, expected): + assert mutation_local.classify_exit_code(exit_code) == expected + + +def test_dotted_module_name_matches_the_top_level_import_path(): + assert mutation_local.dotted_module(Path("agentflow/retry.py")) == "agentflow.retry" + assert ( + mutation_local.dotted_module(Path("serving/semantic_layer/query/sql_builder.py")) + == "serving.semantic_layer.query.sql_builder" + ) + + +def test_write_source_does_not_translate_newlines(tmp_path: Path): + path = tmp_path / "module.py" + + mutation_local.write_source(path, "first\nsecond\n") + + assert path.read_bytes() == b"first\nsecond\n" + + +SHIM_CHECK = """ +import sys + +import mutmut +from mutmut.mutation.trampoline import wrap_in_trampoline + +stub = sys.modules["mutmut.__main__"] +assert type(stub).__name__ == "module", type(stub) +# The real submodule would have been read off disk -- and on Windows would have +# called sys.exit(1) on the way. +assert getattr(stub, "__file__", None) is None, stub.__file__ +assert issubclass(stub.MutmutProgrammaticFailException, Exception) +assert stub.mangled_name_from_mutant_name("x__mutmut_3") == "x" +assert stub.record_trampoline_hit("x__mutmut_3") is None +assert callable(wrap_in_trampoline) +""" + + +def test_write_sitecustomize_lets_a_child_import_the_trampoline(tmp_path: Path): + """The shim is only worth anything if a child interpreter can actually use it. + + Asserting on its source text would pass while every mutant died during + collection -- and a mutant that dies in collection exits 1 and scores + "killed", so the driver would report a silent, meaningless 100%. This runs + the thing: `mutmut.mutation.trampoline` imports three names from + `mutmut.__main__`, whose import is exactly what `sys.exit(1)`s on native + Windows, so a child that gets through this import and finds the stub in + `sys.modules` is the whole mechanism working end to end. + """ + shim_dir = mutation_local.write_sitecustomize(tmp_path / "shim") + + assert b"\r\n" not in (shim_dir / "sitecustomize.py").read_bytes() + result = subprocess.run( + [sys.executable, "-c", SHIM_CHECK], + env=mutation_local.child_env(shim_dir), + cwd=tmp_path, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr[-2000:] + + +def test_child_env_prepends_the_shim_and_carries_the_mutant(monkeypatch, tmp_path: Path): + monkeypatch.setenv("PYTHONPATH", "existing-entry") + + env = mutation_local.child_env(tmp_path, "pkg.thing.x__mutmut_1") + + assert env["PYTHONPATH"].split(os.pathsep)[0] == str(tmp_path) + assert env["PYTHONPATH"].split(os.pathsep)[1] == "existing-entry" + assert env["MUTANT_UNDER_TEST"] == "pkg.thing.x__mutmut_1" + + +def test_child_env_clears_an_inherited_mutant_selection(monkeypatch, tmp_path: Path): + monkeypatch.setenv("MUTANT_UNDER_TEST", "leftover") + + assert "MUTANT_UNDER_TEST" not in mutation_local.child_env(tmp_path) + + +def _checkout(tmp_path: Path, source: str = PRISTINE) -> Path: + """A checkout holding the module under test; also `mutation_report.ROOT`.""" + real_package = tmp_path / "repo" / "pkg" + real_package.mkdir(parents=True, exist_ok=True) + (real_package / "__init__.py").write_bytes(b"") + (real_package / "thing.py").write_bytes(source.encode("utf-8")) + return real_package + + +def _symlink_package(workspace: Path, real_package: Path) -> None: + try: + os.symlink(real_package, workspace / "pkg", target_is_directory=True) + except OSError as exc: # pragma: no cover - unprivileged Windows shells + pytest.skip(f"symlinks not available here: {exc}") + + +class _Workspace: + """An unbuilt workspace plus the checkout and the `prepare_workspace` stub. + + `measure_module` builds it itself: the real `prepare_workspace` copies the + whole repository, so it is replaced by a stub that lays down only what the + driver looks at -- a `pyproject.toml` and the top-level package symlinked + at the real sources, exactly the shape CI's workspace has. + """ + + def __init__(self, monkeypatch, tmp_path: Path, source: str = PRISTINE): + self.root = tmp_path / "repo" + self.real_package = _checkout(tmp_path, source) + self.path = tmp_path / "workspace" + self.prepared: list[Path] = [] + monkeypatch.setattr(mutation_report, "ROOT", self.root) + monkeypatch.setattr(mutation_report, "prepare_workspace", self._prepare) + + def _prepare(self, workspace: Path, module_path: Path, target) -> None: + self.prepared.append(Path(workspace)) + (workspace / "pyproject.toml").write_text("[tool.mutmut]\n", encoding="utf-8") + _symlink_package(Path(workspace), self.real_package) + + def module_source(self) -> bytes: + return (self.real_package / "thing.py").read_bytes() + + def stamp(self) -> dict: + return json.loads((self.path / mutation_local.STAMP_FILENAME).read_text(encoding="utf-8")) + + +def test_materialize_package_replaces_the_symlink_with_a_real_copy(tmp_path: Path): + real_package = _checkout(tmp_path) + workspace = tmp_path / "workspace" + workspace.mkdir() + _symlink_package(workspace, real_package) + + assert mutation_local.materialize_package(workspace, MODULE_PATH) is True + + assert not (workspace / "pkg").is_symlink() + assert (workspace / "pkg" / "thing.py").read_bytes() == PRISTINE.encode("utf-8") + # A second call on the materialized copy is a no-op. + assert mutation_local.materialize_package(workspace, MODULE_PATH) is False + + +TIMED_OUT = "timeout" + + +def _stub_engine( + monkeypatch, + exit_codes: list[int | None | str], + *, + mutants: int | None = None, +) -> list[dict]: + """Stub coverage measurement, the mutmut engine and the pytest subprocess. + + `exit_codes` is consumed in call order, so a retry of the Nth mutant reads + the entry after the parallel pass's last one. The sentinel TIMED_OUT raises + `subprocess.TimeoutExpired` the way a real hung mutant does. + """ + calls: list[dict] = [] + monkeypatch.setattr( + mutation_local, + "measure_covered_lines", + lambda *args, **kwargs: {1, 2}, + ) + generated = len(exit_codes) if mutants is None else mutants + monkeypatch.setattr( + mutation_local, + "generate_mutants", + lambda module_path, source, covered: SimpleNamespace( + code=MUTATED, + mutant_names=[f"x__mutmut_{index + 1}" for index in range(generated)], + ), + ) + + def fake_run(command, **kwargs): + index = len(calls) + calls.append( + { + "command": command, + "env": kwargs["env"], + "cwd": kwargs["cwd"], + "timeout": kwargs.get("timeout"), + "mutant": kwargs["env"].get("MUTANT_UNDER_TEST"), + "module_on_disk": (Path(kwargs["cwd"]) / MODULE_PATH).read_bytes(), + } + ) + outcome = exit_codes[index] + if outcome == TIMED_OUT: + raise subprocess.TimeoutExpired(command, kwargs.get("timeout")) + return SimpleNamespace(returncode=outcome, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + return calls + + +def test_measure_module_materializes_the_package_before_it_mutates_the_module( + monkeypatch, + tmp_path: Path, +): + workspace = _Workspace(monkeypatch, tmp_path) + real_package = workspace.real_package + calls = _stub_engine(monkeypatch, [1, 0]) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + # The mutated source reached the workspace copy... + assert [call["module_on_disk"] for call in calls] == [MUTATED.encode("utf-8")] * 2 + # ...and never the real sources behind the symlink. + assert (real_package / "thing.py").read_bytes() == PRISTINE.encode("utf-8") + # The workspace is handed back unmutated, byte for byte. + assert (workspace.path / "pkg" / "thing.py").read_bytes() == PRISTINE.encode("utf-8") + assert run.generated == 2 + + +def _basetemps(calls: list[dict]) -> list[str]: + return [ + argument + for call in calls + for argument in call["command"] + if argument.startswith("--basetemp=") + ] + + +def test_measure_module_gives_every_mutant_its_own_basetemp(monkeypatch, tmp_path: Path): + workspace = _Workspace(monkeypatch, tmp_path) + calls = _stub_engine(monkeypatch, [1, 1, 1]) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=3, + timeout=5.0, + ) + + basetemps = _basetemps(calls) + assert len(basetemps) == 3 + assert len(set(basetemps)) == 3 + + +def test_two_invocations_sharing_a_workspace_get_separate_basetemps( + monkeypatch, + tmp_path: Path, +): + """The same command run twice lands on the same workspace by design. + + A matching stamp is what makes sharing it safe, and nothing serialises the + two -- the second terminal reuses the workspace rather than rebuilding it. + pytest wipes and recreates its `--basetemp` at startup, so a scratch path + keyed only on the mutant's index would let two pools abort each other. That + is never a wrong score (only exits 0 and 1 are verdicts), but it is a run + thrown away, so the scratch is namespaced per invocation. + """ + workspace = _Workspace(monkeypatch, tmp_path) + + first = _stub_engine(monkeypatch, [1, 1]) + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=2, + timeout=5.0, + ) + second = _stub_engine(monkeypatch, [1, 1]) + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=2, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path] # the second run reused it + assert len(_basetemps(first)) == len(_basetemps(second)) == 2 + assert not set(_basetemps(first)) & set(_basetemps(second)) + + +def test_measure_module_scores_verdicts_and_never_counts_an_error_as_a_kill( + monkeypatch, + tmp_path: Path, +): + workspace = _Workspace(monkeypatch, tmp_path) + # The fourth mutant exits 2 twice: once in parallel, once on its retry. + _stub_engine(monkeypatch, [1, 1, 0, 2, 2], mutants=4) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert sorted(run.killed) == ["pkg.thing.x__mutmut_1", "pkg.thing.x__mutmut_2"] + assert run.survived == ["pkg.thing.x__mutmut_3"] + assert run.errored == [("pkg.thing.x__mutmut_4", 2)] + assert run.scored == 3 + assert run.score == pytest.approx(2 / 3) + # Below threshold anyway, but an unexplained exit code alone fails the gate. + assert run.passed is False + + +def test_measure_module_selects_only_the_requested_mutants(monkeypatch, tmp_path: Path): + workspace = _Workspace(monkeypatch, tmp_path) + calls = _stub_engine(monkeypatch, [1, 1, 1]) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + only={"x__mutmut_2", "x__mutmut_404"}, + ) + + assert len(calls) == 1 + assert calls[0]["env"]["MUTANT_UNDER_TEST"] == "pkg.thing.x__mutmut_2" + assert run.generated == 3 + assert run.killed == ["pkg.thing.x__mutmut_2"] + + +def test_measure_module_accepts_a_survivor_name_the_way_it_prints_it( + monkeypatch, + tmp_path: Path, + capsys, +): + """`--only` has to take the names the tool's own report hands back. + + Everything printed carries the dotted module in front + (`pkg.thing.x__mutmut_2`), while the engine names mutants bare, so an + unstripped prefix would select nothing and score zero. + """ + workspace = _Workspace(monkeypatch, tmp_path) + calls = _stub_engine(monkeypatch, [1, 1, 1]) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + only={"pkg.thing.x__mutmut_2", "pkg.thing.x__mutmut_404"}, + ) + + assert len(calls) == 1 + assert calls[0]["env"]["MUTANT_UNDER_TEST"] == "pkg.thing.x__mutmut_2" + assert run.killed == ["pkg.thing.x__mutmut_2"] + assert run.scored == 1 + # A name that is not in the population is still reported, dotted like the rest. + assert "pkg.thing.x__mutmut_404" in capsys.readouterr().out + + +def test_source_module_file_maps_a_gate_target_back_to_the_checkout(monkeypatch, tmp_path: Path): + monkeypatch.setattr(mutation_report, "ROOT", tmp_path) + + assert ( + mutation_local.source_module_file(Path("agentflow/retry.py")) + == tmp_path / "sdk" / "agentflow" / "retry.py" + ) + assert ( + mutation_local.source_module_file(Path("serving/api/rate_limiter.py")) + == tmp_path / "src" / "agentflow_runtime" / "serving" / "api" / "rate_limiter.py" + ) + + +def test_every_gate_target_resolves_to_a_file_in_this_checkout(): + """The mapping is only useful while it still matches `prepare_workspace`.""" + for module_path in mutation_report.MODULE_TARGETS: + assert mutation_local.source_module_file(module_path).is_file(), module_path + + +def test_clear_workspace_removes_links_without_following_them(tmp_path: Path): + real_package = _checkout(tmp_path) + workspace = tmp_path / "workspace" + workspace.mkdir() + _symlink_package(workspace, real_package) + (workspace / "pyproject.toml").write_text("[tool.mutmut]\n", encoding="utf-8") + (workspace / mutation_local.MARKER_FILENAME).write_text("", encoding="utf-8") + (workspace / ".mutation-local-tmp" / "mutant0").mkdir(parents=True) + + mutation_local.clear_workspace(workspace) + + assert list(workspace.iterdir()) == [] + # The checkout the link pointed at is untouched. + assert (real_package / "thing.py").read_bytes() == PRISTINE.encode("utf-8") + + +def test_clear_workspace_refuses_a_directory_it_did_not_build(tmp_path: Path): + """`--workspace` is typed by hand, and a rebuild deletes everything in it.""" + somewhere_else = tmp_path / "notes" + somewhere_else.mkdir() + (somewhere_else / "important.txt").write_text("keep me", encoding="utf-8") + + with pytest.raises(SystemExit, match="refusing to rebuild"): + mutation_local.clear_workspace(somewhere_else) + + assert (somewhere_else / "important.txt").read_text(encoding="utf-8") == "keep me" + + +def test_clear_workspace_refuses_a_python_project_it_did_not_build(tmp_path: Path): + """A `pyproject.toml` marks Python projects generally, not a workspace. + + The likeliest wrong `--workspace` is a project root, and accepting that + marker would empty it: sources, virtualenv and all. + """ + project = tmp_path / "some-project" + (project / "src" / "pkg").mkdir(parents=True) + (project / "pyproject.toml").write_text("[project]\nname = 'x'\n", encoding="utf-8") + (project / "src" / "pkg" / "code.py").write_text("VALUE = 1\n", encoding="utf-8") + + with pytest.raises(SystemExit, match="refusing to rebuild"): + mutation_local.clear_workspace(project) + + assert (project / "src" / "pkg" / "code.py").read_text(encoding="utf-8") == "VALUE = 1\n" + assert sorted(path.name for path in project.iterdir()) == ["pyproject.toml", "src"] + + +def test_clear_workspace_refuses_a_checkout_carrying_a_git_directory(tmp_path: Path): + checkout = tmp_path / "checkout" + (checkout / ".git").mkdir(parents=True) + # Even a marked directory: a `.git` means someone typed the wrong path. + (checkout / mutation_local.MARKER_FILENAME).write_text("", encoding="utf-8") + + with pytest.raises(SystemExit, match="holds a [.]git"): + mutation_local.clear_workspace(checkout) + + assert (checkout / ".git").is_dir() + + +def test_clear_workspace_refuses_a_path_inside_the_checkout(monkeypatch, tmp_path: Path): + monkeypatch.setattr(mutation_report, "ROOT", tmp_path / "repo") + inside = tmp_path / "repo" / "workspace" + inside.mkdir(parents=True) + (inside / mutation_local.MARKER_FILENAME).write_text("", encoding="utf-8") + + with pytest.raises(SystemExit, match="lives inside it"): + mutation_local.clear_workspace(inside) + + assert inside.is_dir() + + +def test_clear_workspace_refuses_this_repository_root(): + """`--workspace .` typed here is the mistake the guard exists for.""" + with pytest.raises(SystemExit, match="refusing to use"): + mutation_local.clear_workspace(mutation_local.ROOT) + + assert (mutation_local.ROOT / ".git").exists() + + +def test_measure_module_stamps_the_workspace_with_what_it_was_built_from( + monkeypatch, + tmp_path: Path, +): + workspace = _Workspace(monkeypatch, tmp_path) + _stub_engine(monkeypatch, [1]) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path] + stamp = workspace.stamp() + assert stamp["root"] == workspace.root.as_posix() + assert stamp["module"] == "pkg/thing.py" + assert stamp["source_sha256"] == hashlib.sha256(workspace.module_source()).hexdigest() + # The workspace copies the whole package and (without symlinks) the tests, + # so both are digested too. + assert stamp["package_sha256"] == mutation_local.tree_sha256(workspace.real_package) + assert set(stamp) == { + "root", + "module", + "source_sha256", + "package_sha256", + "tests_sha256", + "pyproject_sha256", + } + + +def test_measure_module_reuses_a_workspace_whose_stamp_matches(monkeypatch, tmp_path: Path): + workspace = _Workspace(monkeypatch, tmp_path) + _stub_engine(monkeypatch, [1, 1], mutants=1) + + for _ in range(2): + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + # Same root, same module, same source: built once, measured twice. + assert workspace.prepared == [workspace.path] + + +def test_measure_module_rebuilds_when_the_module_source_changed(monkeypatch, tmp_path: Path): + workspace = _Workspace(monkeypatch, tmp_path) + _stub_engine(monkeypatch, [1, 1], mutants=1) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + # The edit this tool exists to be run after. + (workspace.real_package / "thing.py").write_bytes(b"VALUE = 41\nOTHER = 2\n") + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path, workspace.path] + assert ( + workspace.stamp()["source_sha256"] == hashlib.sha256(workspace.module_source()).hexdigest() + ) + # The pristine copy was re-taken with the workspace, so the mutants are + # generated from the edited source rather than the previous run's. + backup = workspace.path / f"{MODULE_PATH.stem}{mutation_local.BACKUP_SUFFIX}" + assert backup.read_bytes() == b"VALUE = 41\nOTHER = 2\n" + + +def test_measure_module_rebuilds_when_a_sibling_in_the_package_changed( + monkeypatch, + tmp_path: Path, +): + """The workspace holds a real copy of the whole package, not just the module. + + `materialize_package` is a no-op once that copy exists, so a stamp keyed on + the target module alone would measure the first run's copy of every sibling + -- a confident score about a tree that is half stale. + """ + workspace = _Workspace(monkeypatch, tmp_path) + sibling = workspace.real_package / "sibling.py" + sibling.write_bytes(b"SIBLING = 'first-run'\n") + _stub_engine(monkeypatch, [1, 1], mutants=1) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + sibling.write_bytes(b"SIBLING = 'second-run'\n") + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path, workspace.path] + assert (workspace.path / "pkg" / "sibling.py").read_bytes() == b"SIBLING = 'second-run'\n" + + +def test_measure_module_rebuilds_when_the_targets_tests_changed(monkeypatch, tmp_path: Path): + """Where symlinks are unavailable the tests are copied too, and go stale.""" + workspace = _Workspace(monkeypatch, tmp_path) + test_file = workspace.root / TARGET.tests[0] + test_file.parent.mkdir(parents=True, exist_ok=True) + test_file.write_bytes(b"def test_value():\n assert VALUE == 1\n") + _stub_engine(monkeypatch, [1, 1], mutants=1) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + test_file.write_bytes(b"def test_value():\n assert VALUE == 1\n assert OTHER == 2\n") + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path, workspace.path] + + +def test_measure_module_rebuilds_when_the_checkouts_pyproject_changed(monkeypatch, tmp_path: Path): + """`prepare_workspace` renders the workspace's pyproject from the checkout's. + + It is written as a real file, never a symlink, and it carries pytest's + addopts, filterwarnings and plugin toggles plus `[tool.mutmut]` -- i.e. it + changes what the mutant runs do. Left out of the stamp, an edit to it would + be measured under the previous run's pytest configuration. + """ + workspace = _Workspace(monkeypatch, tmp_path) + pyproject = workspace.root / "pyproject.toml" + pyproject.write_bytes(b'[tool.pytest.ini_options]\naddopts = "-q"\n') + _stub_engine(monkeypatch, [1, 1], mutants=1) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + assert ( + workspace.stamp()["pyproject_sha256"] == hashlib.sha256(pyproject.read_bytes()).hexdigest() + ) + pyproject.write_bytes(b'[tool.pytest.ini_options]\naddopts = "-q -p no:randomly"\n') + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path, workspace.path] + assert ( + workspace.stamp()["pyproject_sha256"] == hashlib.sha256(pyproject.read_bytes()).hexdigest() + ) + + +def test_measure_module_never_reuses_another_checkouts_workspace(monkeypatch, tmp_path: Path): + """`--root` must not be silently ignored, even when the sources are identical.""" + workspace = _Workspace(monkeypatch, tmp_path) + _stub_engine(monkeypatch, [1, 1], mutants=1) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + other = _checkout(tmp_path / "elsewhere") + monkeypatch.setattr(mutation_report, "ROOT", other.parent) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path, workspace.path] + assert workspace.stamp()["root"] == other.parent.as_posix() + + +def test_measure_module_rebuilds_when_the_pristine_copy_is_missing(monkeypatch, tmp_path: Path): + """An interrupted first run must not leave a workspace the next one believes.""" + workspace = _Workspace(monkeypatch, tmp_path) + _stub_engine(monkeypatch, [1, 1], mutants=1) + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + (workspace.path / f"{MODULE_PATH.stem}{mutation_local.BACKUP_SUFFIX}").unlink() + + mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert workspace.prepared == [workspace.path, workspace.path] + + +def test_measure_module_retries_a_mutant_without_a_verdict_serially(monkeypatch, tmp_path: Path): + workspace = _Workspace(monkeypatch, tmp_path) + calls = _stub_engine(monkeypatch, [1, TIMED_OUT, 1], mutants=2) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + retry_timeout=30.0, + ) + + # Retried exactly once, uncontended, with a longer timeout than the pass + # that timed it out -- and killed on the strength of the retry's verdict. + assert [call["mutant"] for call in calls] == [ + "pkg.thing.x__mutmut_1", + "pkg.thing.x__mutmut_2", + "pkg.thing.x__mutmut_2", + ] + assert [call["timeout"] for call in calls] == [5.0, 5.0, 30.0] + assert run.retried == ["pkg.thing.x__mutmut_2"] + assert sorted(run.killed) == ["pkg.thing.x__mutmut_1", "pkg.thing.x__mutmut_2"] + assert run.errored == [] + + +def test_measure_module_keeps_a_mutant_errored_when_the_retry_has_no_verdict_either( + monkeypatch, + tmp_path: Path, +): + workspace = _Workspace(monkeypatch, tmp_path) + calls = _stub_engine(monkeypatch, [TIMED_OUT, TIMED_OUT], mutants=1) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + # Never promoted to a kill, and the default retry gets three times as long. + assert [call["timeout"] for call in calls] == [5.0, 15.0] + assert run.retried == ["pkg.thing.x__mutmut_1"] + assert run.killed == [] + assert run.errored == [("pkg.thing.x__mutmut_1", None)] + assert run.passed is False + + +def test_measure_module_does_not_retry_when_every_mutant_has_a_verdict(monkeypatch, tmp_path: Path): + workspace = _Workspace(monkeypatch, tmp_path) + calls = _stub_engine(monkeypatch, [1, 0]) + + run = mutation_local.measure_module( + MODULE_PATH, + TARGET, + workspace.path, + tmp_path / "shim", + python="python", + jobs=1, + timeout=5.0, + ) + + assert len(calls) == 2 + assert run.retried == [] + + +def test_module_run_passes_only_at_or_above_its_threshold(): + at_threshold = mutation_local.ModuleRun( + module_path=MODULE_PATH, + threshold=0.90, + generated=10, + killed=[f"m{index}" for index in range(9)], + survived=["m9"], + ) + below = mutation_local.ModuleRun( + module_path=MODULE_PATH, + threshold=0.90, + generated=10, + killed=[f"m{index}" for index in range(8)], + survived=["m8", "m9"], + ) + nothing_scored = mutation_local.ModuleRun(module_path=MODULE_PATH, threshold=0.90, generated=0) + + assert at_threshold.passed is True + assert below.passed is False + assert nothing_scored.passed is False + + +def test_module_run_fails_on_a_mutant_without_a_verdict_even_at_a_passing_score(): + """A harness failure is never a green, however good the scored mutants look. + + Scored on its own the run is at threshold; one mutant that came back with an + unexplained exit code means the population was not fully measured, so the + number is not the gate's number and must not be reported as a pass. + """ + errored_at_threshold = mutation_local.ModuleRun( + module_path=MODULE_PATH, + threshold=0.90, + generated=11, + killed=[f"m{index}" for index in range(9)], + survived=["m9"], + errored=[("m10", 2)], + ) + + assert errored_at_threshold.score == pytest.approx(0.9) + assert errored_at_threshold.score >= errored_at_threshold.threshold + assert errored_at_threshold.passed is False + + +def test_main_rejects_a_module_outside_the_gate(capsys): + assert mutation_local.main(["--module", "serving/not_a_target.py"]) == 2 + assert "unknown module" in capsys.readouterr().err + + +def test_main_resolves_a_relative_workspace_before_anything_uses_it(monkeypatch, tmp_path: Path): + """A relative `--workspace` would reach coverage as a relative `--include=`. + + coverage matches that pattern against the absolute paths it records, so it + measures nothing and the module dies with "coverage recorded no lines", + naming the wrong problem; the stamp, backup and report inherit the same + relativeness. + """ + seen: list[Path] = [] + + def fake_measure(module_path, target, workspace, shim_dir, **kwargs): + seen.append(workspace) + return mutation_local.ModuleRun( + module_path=module_path, + threshold=target.threshold, + generated=1, + killed=["m0"], + ) + + monkeypatch.setattr(mutation_local, "measure_module", fake_measure) + monkeypatch.chdir(tmp_path) + module = next(iter(mutation_report.MODULE_TARGETS)).as_posix() + + exit_code = mutation_local.main( + ["--module", module, "--workspace", "ws", "--json", str(tmp_path / "report.json")] + ) + + assert exit_code == 0 + assert seen == [(tmp_path / "ws").resolve()] + assert seen[0].is_absolute() + + +def test_main_list_modules_prints_the_gate_definition(capsys): + assert mutation_local.main(["--list-modules"]) == 0 + + printed = capsys.readouterr().out + for module_path in mutation_report.MODULE_TARGETS: + assert module_path.as_posix() in printed diff --git a/tests/unit/test_sql_builder_mutation.py b/tests/unit/test_sql_builder_mutation.py index c9646672..e16af242 100644 --- a/tests/unit/test_sql_builder_mutation.py +++ b/tests/unit/test_sql_builder_mutation.py @@ -49,13 +49,48 @@ scored it ``n/a``, and the weekly mutation gate went red on 2026-07-12 and stayed red. Anything added to sql_builder's imports belongs here too. -Reproduced at 96.0% (killed 167, survived 7) via the WSL/mutmut harness (py3.10); -the CI gate (mutation.yml on py3.11) is the source of truth. The 7 survivors are -genuine equivalent mutants, not gaps: four mutate the *string* inside -``cast("dict[...]", value)`` -- the runtime ``typing.cast`` ignores its first -argument, so any text change there is a no-op -- and three flip -``parse_one(..., dialect="duckdb")`` / ``parsed.sql(dialect=...)`` to -``dialect=None``, which renders the plain SELECTs this builder handles identically. +A note on the score, because the number here was wrong for two months. This +docstring used to record "96.0% (killed 167, survived 7), the 7 are equivalent +mutants, not gaps", measured on a WSL/py3.10 harness. ``1096e2e`` then broke the +import shim above, the module scored ``n/a`` for nine weeks, and nobody could +have noticed the figure going stale. The first run after the shim was repaired +(``3820a2f``) measured 80.3% -- killed 106, survived 26, against a 90% threshold +-- and 14 of those 26 were in ``_holds_foreign_tenant_rows``, a method this file +did not test at all. So the old paragraph was not merely out of date: it was +describing a mutant population that no longer existed, and it read as +reassurance while a tenant-isolation guard sat unpinned. + +The CI gate (mutation.yml on py3.11) is the source of truth for the score CI +enforces, but since ``25769d7`` the same population is measurable here too: +``python scripts/mutation_local.py --module +serving/semantic_layer/query/sql_builder.py``, about two minutes. Mutant +*counts* still differ per interpreter, because ``mutate_only_covered_lines`` +makes the population depend on coverage attribution, so a number written down +here has to carry where it was measured. + +Two measurements, both py3.13 in ``.venv`` on 2026-09-09. At ``25769d7``: 141 +mutants, 128 killed, 13 survived, 90.8% -- the same population CI run +34266462154 generated, down to the surviving mutant names. After this file and +``sql_builder.py`` were changed to take the module off the threshold line: 126 +mutants, 124 killed, 2 survived, 98.4%. + +Fifteen mutants left the denominator, and which ones matters more than the +count. Eight were mutants of a ``typing.cast`` type argument -- a cast returns +its second argument untouched and never evaluates the first, so no test can +ever tell them from the original. Six more were mutants of the ``cast(...)`` +*call itself* (its argument count, its second argument); they were killable, +and they stopped existing along with the two casts, which are now plain +annotations. The last one is the equivalent ``rows = []`` -> ``rows = None`` in +``_holds_foreign_tenant_rows``, marked ``# pragma: no mutate`` in place. The +casts were not pragma'd instead, because mutmut's pragma is recorded against a +whole *statement*, at its first line. On the one-line cast it would have taken +the eleven killable mutants on that line (the ``getattr``'s own seven, the +assignment, and the cast call's argument mutants) out of the denominator as +well -- the opposite of the point -- and on the four-line one in +``_qualify_table`` it would not have reached the type string at all, only the +``cache = ...`` on the opening line. Two further mutants were killed rather +than removed, by +``test_scope_sql_does_not_change_which_list_element_the_query_asks_for``. """ from __future__ import annotations @@ -201,6 +236,27 @@ def load(self) -> _TenantsConfig: return _TenantsConfig(self._tenants) +class _Backend: + """Answers the one question `_holds_foreign_tenant_rows` asks a store. + + It records the SQL rather than only replaying a verdict: the probe text *is* + the check. A mutant that widens `<>` to `=`, drops the `LIMIT 1`, or asks + about some tenant other than the default still returns a truthy row and + would pass a test that only looked at the boolean. + """ + + def __init__(self, rows: object = (), error: BaseException | None = None) -> None: + self._rows = rows + self._error = error + self.queries: list[str] = [] + + def execute(self, sql: str) -> object: + self.queries.append(sql) + if self._error is not None: + raise self._error + return self._rows + + class _Host(SQLBuilderMixin): def __init__( self, @@ -209,12 +265,22 @@ def __init__( tenant_router: _TenantRouter, table_columns: dict[str, set[str]] | None = None, cache: dict | None = None, + backend: _Backend | None = None, + foreign_tenant_cache: dict[str, bool] | None = None, ) -> None: self.catalog = catalog self._tenant_router = tenant_router self._table_columns_map = dict(table_columns or {}) if cache is not None: self._qualified_table_cache = cache + # Absent, not None, when no store is supplied: the production host always + # has `_backend`, and `_holds_foreign_tenant_rows` reads both attributes + # through `getattr(..., None)`, so a double that never sets them exercises + # the same defaulted reads the mixin performs. + if backend is not None: + self._backend = backend + if foreign_tenant_cache is not None: + self._foreign_tenant_cache = foreign_tenant_cache def _table_columns(self, table_name: str) -> set[str]: return self._table_columns_map.get(table_name, set()) @@ -379,6 +445,102 @@ def test_quote_literal_string_is_quoted_and_escaped(): assert _host()._quote_literal("O'Brien") == "'O''Brien'" +# --------------------------------------------------------------------------- # +# _holds_foreign_tenant_rows: the fail-closed probe behind an unscoped read. +# A request that carries no tenant context is answered only when the table has +# nothing to leak — every row in it belongs to DEFAULT_TENANT. Both directions +# have teeth: a false negative hands an anonymous caller every tenant's rows, a +# false positive 503s the single-tenant demo that never sets a tenant at all. +# (audit p2_1 #5) +# +# The method had no tests. Its only exercised path was the `_backend is None` +# early return the host doubles fell into, so the probe, the cache and the +# fail-closed branch it feeds were all unpinned — 14 of the 26 mutants that +# survived the 2026-09-08 gate run (score 80.3%, threshold 90%) live here. +# --------------------------------------------------------------------------- # + +FOREIGN_TENANT_PROBE = "SELECT 1 FROM orders WHERE tenant_id <> 'default' LIMIT 1" + + +def test_holds_foreign_tenant_rows_is_true_when_the_store_returns_a_row(): + host = _host(backend=_Backend(rows=[(1,)])) + assert host._holds_foreign_tenant_rows("orders") is True + + +def test_holds_foreign_tenant_rows_is_false_when_the_store_returns_nothing(): + host = _host(backend=_Backend(rows=[])) + assert host._holds_foreign_tenant_rows("orders") is False + + +def test_holds_foreign_tenant_rows_asks_only_about_non_default_tenants(): + # The probe text *is* the check, so it is pinned whole. A mutant that widens + # `<>` to `=`, drops the `LIMIT 1`, or names a tenant other than the default + # still returns a truthy row, and a test that only read the boolean would + # call every one of those correct. + backend = _Backend(rows=[]) + host = _host(backend=backend) + host._holds_foreign_tenant_rows("orders") + assert backend.queries == [FOREIGN_TENANT_PROBE] + + +def test_holds_foreign_tenant_rows_probes_the_table_it_was_given(): + backend = _Backend(rows=[]) + host = _host(backend=backend) + host._holds_foreign_tenant_rows("customers") + assert backend.queries == ["SELECT 1 FROM customers WHERE tenant_id <> 'default' LIMIT 1"] + + +def test_holds_foreign_tenant_rows_treats_an_unreadable_table_as_empty(): + # Not materialized yet, or no tenant column: there are no foreign rows in it + # to leak, so the unscoped read stays allowed. + error = sql_builder_module.BackendExecutionError("no such table: orders") + host = _host(backend=_Backend(error=error)) + assert host._holds_foreign_tenant_rows("orders") is False + + +def test_holds_foreign_tenant_rows_lets_an_unexpected_failure_through(): + # Only the store's own "cannot read that" is benign. A connection fault is + # not evidence of an empty table, and must not be laundered into permission. + host = _host(backend=_Backend(error=RuntimeError("connection reset"))) + with pytest.raises(RuntimeError): + host._holds_foreign_tenant_rows("orders") + + +def test_holds_foreign_tenant_rows_serves_a_cached_verdict_without_probing(): + backend = _Backend(rows=[(1,)]) + host = _host(backend=backend, foreign_tenant_cache={"orders": False}) + assert host._holds_foreign_tenant_rows("orders") is False + assert backend.queries == [] + + +def test_holds_foreign_tenant_rows_caches_what_it_learned(): + # One probe per table per process, not one per read. + backend = _Backend(rows=[(1,)]) + cache: dict[str, bool] = {} + host = _host(backend=backend, foreign_tenant_cache=cache) + assert host._holds_foreign_tenant_rows("orders") is True + assert cache == {"orders": True} + assert host._holds_foreign_tenant_rows("orders") is True + assert len(backend.queries) == 1 + + +def test_holds_foreign_tenant_rows_caches_per_table(): + # Keyed by table: one table's emptiness must never vouch for another's. + backend = _Backend(rows=[(1,)]) + host = _host(backend=backend, foreign_tenant_cache={"orders": False}) + assert host._holds_foreign_tenant_rows("customers") is True + assert backend.queries == ["SELECT 1 FROM customers WHERE tenant_id <> 'default' LIMIT 1"] + + +def test_holds_foreign_tenant_rows_still_answers_without_a_cache(): + # The cache is an optimisation the host may not offer; the verdict is not. + backend = _Backend(rows=[(1,)]) + host = _host(backend=backend) + assert host._holds_foreign_tenant_rows("orders") is True + assert host._holds_foreign_tenant_rows("orders") is True + assert len(backend.queries) == 2 + + # --------------------------------------------------------------------------- # # _qualify_table: the scoped relation every entity read goes through, plus its # cache. This is the chokepoint — a surviving mutant here is a cross-tenant read. @@ -455,6 +617,42 @@ def test_qualify_table_propagates_an_invalid_tenant_id(): host._qualify_table("orders", "acme'; DROP TABLE orders--") +def test_qualify_table_refuses_an_unscoped_read_of_a_multi_tenant_table(monkeypatch): + # No tenant context *and* the table holds somebody else's rows: the caller + # gets a refusal, not everyone's data. This is the branch the probe exists + # to feed, and until now nothing reached it — the host doubles had no store, + # so `_holds_foreign_tenant_rows` always short-circuited to False and the + # guard was never taken in a test. + monkeypatch.setattr(sql_builder_module, "get_current_tenant_id", lambda default=None: None) + backend = _Backend(rows=[(1,)]) + host = _host(tenant_router=_TenantRouter(has_config=True), backend=backend) + with pytest.raises(ValueError, match="Tenant context is required"): + host._qualify_table("orders", None) + # And it refused because of *this* table. A mutant that probes something + # else still finds a row and still raises, so the exception alone does not + # prove the guard asked the right question. + assert backend.queries == [FOREIGN_TENANT_PROBE] + + +def test_qualify_table_allows_an_unscoped_read_of_a_single_tenant_table(monkeypatch): + # The other half of the same branch: a store whose rows all belong to the + # default tenant has nothing to leak, so the deployment that never sets a + # tenant keeps reading. + monkeypatch.setattr(sql_builder_module, "get_current_tenant_id", lambda default=None: None) + host = _host(tenant_router=_TenantRouter(has_config=True), backend=_Backend(rows=[])) + assert host._qualify_table("orders", None) == SCOPED_ORDERS_UNSCOPED + + +def test_qualify_table_does_not_probe_when_a_tenant_is_in_context(): + # The probe only means anything for an unscoped read. Running it on the + # scoped path would add a query per table per request, and a mutant that + # loosens the `predicate is None` guard into `or` does exactly that. + backend = _Backend(rows=[(1,)]) + host = _host(tenant_router=_TenantRouter(has_config=True), backend=backend) + assert host._qualify_table("orders", "acme") == SCOPED_ORDERS_ACME + assert backend.queries == [] + + # --------------------------------------------------------------------------- # # _scope_sql: the same boundary, applied to SQL the engine did not build itself # (metric templates, NL-generated SQL). @@ -506,7 +704,12 @@ def test_scope_sql_fails_closed_on_a_recursive_cte_shadowing_a_table(): # genuinely ambiguous with the recursion), and no legitimate query names one # after a physical table. Fail closed rather than leak. host = _host(catalog=_Catalog("orders"), tenant_router=_TenantRouter(has_config=True)) - with pytest.raises(ValueError, match="Recursive CTE shadows tenant-scoped table"): + # The message names the table it refused over: an operator reading the 503 + # needs to know which one, and pinning the rendered name is also what stops a + # mutant from reporting `['ORDERS']` while the check itself still works. + with pytest.raises( + ValueError, match=r"Recursive CTE shadows tenant-scoped table\(s\): \['orders'\]" + ): host._scope_sql( "WITH RECURSIVE orders AS (SELECT 1 AS id UNION ALL SELECT id FROM orders) " "SELECT id FROM orders", @@ -514,6 +717,20 @@ def test_scope_sql_fails_closed_on_a_recursive_cte_shadowing_a_table(): ) +def test_scope_sql_allows_a_recursive_cte_that_shadows_nothing(): + # The rule above is about *shadowing*, not about recursion. A recursive CTE + # whose name collides with no serving table is an ordinary query and has to + # keep working — without this, a guard that refused every `WITH RECURSIVE` + # would look identical to one that refused only the dangerous ones. + host = _host(catalog=_Catalog("orders"), tenant_router=_TenantRouter(has_config=True)) + scoped = host._scope_sql( + "WITH RECURSIVE counter AS (SELECT 1 AS n UNION ALL SELECT n + 1 FROM counter) " + "SELECT n FROM counter", + "acme", + ) + assert "counter" in scoped + + def test_scope_sql_unscoped_still_hides_the_tenant_column(monkeypatch): # No tenant (auth disabled) -> no predicate, but the read still goes through # the scoped relation, so tenant_id never surfaces in a caller's `SELECT *`. @@ -580,3 +797,62 @@ def test_scope_sql_forwards_the_tenant_id_to_qualify_table(): ) host._scope_sql("SELECT * FROM widgets JOIN orders ON widgets.id = orders.id", "acme") assert calls == [("orders", "acme")] + + +# --------------------------------------------------------------------------- # +# The dialect the incoming SQL is read in. Scoping a query must not change what +# the query means. +# --------------------------------------------------------------------------- # + + +def test_scope_sql_does_not_change_which_list_element_the_query_asks_for(): + # `dialect="duckdb"` on the parse of the *incoming* SQL is not decoration. + # DuckDB's list indexing is 1-based; sqlglot's default dialect reads the + # same `[1]` as 0-based and re-renders it as `[2]` on the way out. So a + # caller that asked for the first element of `list_value(1, 2)` would get a + # scoped query asking for the second one -- the tenant scoper would have + # silently changed the answer while adding a WHERE clause. Kills the + # `dialect=None` and dropped-`dialect` mutants on the parse of the incoming + # SQL (`_scope_sql__mutmut_8` and `_10`). + host = _host(catalog=_Catalog("orders"), tenant_router=_TenantRouter(has_config=True)) + out = host._scope_sql("SELECT list_value(1, 2)[1] AS x FROM orders", "acme") + assert out == f"SELECT [1, 2][1] AS x FROM {SCOPED_ORDERS_ACME}" + + +# --------------------------------------------------------------------------- # +# Two mutants of this module are left alive on purpose. This is the record of +# why, so the next reader does not mistake them for a gap (measured 2026-09-09 +# with `python scripts/mutation_local.py --module +# serving/semantic_layer/query/sql_builder.py`, sqlglot 30.12.0): +# +# _scope_sql__mutmut_41 parse_one(scoped, dialect=None) +# _scope_sql__mutmut_43 parse_one(scoped) +# +# Both mutate the *second* parse in `_scope_sql` -- the one that reads `scoped`, +# the relation `_qualify_table` built a line earlier, not anything a caller +# supplied. That string has one fixed shape, +# +# (SELECT * EXCLUDE (tenant_id) FROM WHERE tenant_id = '') +# AS "
" +# +# and whichever dialect parses it -- `duckdb` or sqlglot's default -- the AST +# that comes back renders identically under the `sql(dialect="duckdb")` +# `_scope_sql` always applies on the way out. That is the invariant that makes +# the two mutants unreachable, and it is narrower than "dialect-neutral": the +# *default-dialect render* of that same AST is not identical, it rewrites +# `EXCLUDE (tenant_id)` to `EXCEPT (tenant_id)`. Which is exactly why the +# mutants on the render (`parsed.sql(dialect="duckdb")`, line 240) are dead and +# pinned -- do not weaken that argument on the strength of this note. Tried on +# the parse side, and identical under the duckdb render either way: that exact +# sub-select, a bare `SELECT * EXCLUDE (col)`, a struct literal, and FROM-first +# syntax. The one construct that does differ is the list indexing the test +# above uses, and it cannot appear here -- this module writes the string +# itself, and never writes that. +# +# They are deliberately NOT marked `# pragma: no mutate`, unlike the `rows = []` +# mutant in sql_builder.py. That one is unkillable by construction; these two are +# merely unreachable through the generator as it stands today, and a +# `_qualify_table` that ever emitted DuckDB-specific syntax would make them +# killable again. A suppression would outlive the reason for it; this note does +# not. +# --------------------------------------------------------------------------- #