From 6bed38f000df34ed0bf1b7c7242ecc7dd2039fa4 Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Tue, 8 Sep 2026 15:00:46 -0400 Subject: [PATCH 1/6] test(mutation): pin the tenant guard the gate found unpinned The first mutation run after 3820a2f repaired the import shim (run 34265359911, 2026-09-08) scored sql_builder.py at 80.3% against a 90% threshold -- killed 106, survived 26. That is not a regression from the repair; it is the first honest measurement in nine weeks. The shim had been broken since 1096e2e, so the module scored n/a and the gate's complaint was about the harness, not the tests. 14 of the 26 survivors were in `_holds_foreign_tenant_rows`, which had no tests. It is the fail-closed probe that decides whether a request carrying no tenant context may read a table at all (audit p2_1 #5): if the table holds a row belonging to anyone but DEFAULT_TENANT, the read is refused. The only path any test reached was the `_backend is None` early return the host doubles fell into, so the probe SQL, the per-table cache and the refusal branch in `_qualify_table` were all unexercised -- a guard against cross-tenant reads with nothing holding it in place. Thirteen tests, through a `_Backend` double that records the SQL rather than only replaying a verdict. The probe text is the check: a mutant that widens `<>` to `=`, drops `LIMIT 1`, or asks about a tenant other than the default still returns a truthy row, so a test reading only the boolean would call all three correct. Also pinned: the store's own "cannot read that" means an unmaterialized table and stays permissive, while an unexpected failure is not laundered into permission; the cache answers without probing, is written once, and is keyed per table; and `_qualify_table` refuses the unscoped read of a multi-tenant table, allows it on a single-tenant one, and does not probe at all when a tenant is in context. The docstring's "96.0%, the 7 survivors are equivalent mutants" paragraph is replaced. It was measured on a py3.10 harness before the shim broke, and it read as reassurance about a mutant population that no longer existed. 54 passed (was 41), and 54 passed again inside a rebuilt mutmut workspace -- top-level `serving`, no `src` -- which is the only shape the shims run in. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015HsJh4f7Lqv3XmkCPSA1uF --- tests/unit/test_sql_builder_mutation.py | 180 +++++++++++++++++++++++- 1 file changed, 173 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_sql_builder_mutation.py b/tests/unit/test_sql_builder_mutation.py index c9646672..d6a0f331 100644 --- a/tests/unit/test_sql_builder_mutation.py +++ b/tests/unit/test_sql_builder_mutation.py @@ -49,13 +49,21 @@ 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 only source of truth for this score; +mutant *counts* differ per interpreter, because ``mutate_only_covered_lines`` +makes the population depend on coverage attribution. Do not restate a number +here that was not read off a mutation.yml run, and record the run id with it. """ from __future__ import annotations @@ -201,6 +209,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 +238,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 +418,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 +590,37 @@ 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) + host = _host(tenant_router=_TenantRouter(has_config=True), backend=_Backend(rows=[(1,)])) + with pytest.raises(ValueError, match="Tenant context is required"): + host._qualify_table("orders", None) + + +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). From 2cda8daa47a2e7f8391bd277fd681dd6d16a0a7c Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Tue, 8 Sep 2026 23:14:04 -0400 Subject: [PATCH 2/6] test(mutation): pin the retry rule that sat exactly on its threshold Run 34266462154 scored `sdk/agentflow/retry.py` at 75.0% against a 75% threshold: 15 killed, 5 survived, and all five survivors were in `is_retryable_method`. A module sitting exactly on its floor is not passing -- it fails the gate the moment anything shifts, and it says nothing about whether the rule underneath is held in place. `is_retryable_method` decides whether a request is replayed after a 429 or a 5xx. The only branch any test reached was the verb allowlist, and even that was partial: `OPTIONS` was in the set and in nobody's assertion. The whole POST-with-an-Idempotency-Key branch -- the one that decides whether a *write* is retried -- had no test at all. Mutants that dropped the case-folding, that matched the header name against a pair's value instead of its key, or that extended the rescue to other non-idempotent verbs therefore all survived. Eleven tests, one per decision the function makes: the missing `OPTIONS` verb, case normalisation of the verb itself, the key found in a mapping and in a header sequence, matched case-insensitively, found among unrelated headers, rejected on a merely similar header name (`Idempotency`), on no headers and on an empty mapping, matched on the name and not on the value, and ignored entirely for PATCH. The tests widen coverage into a branch that was previously invisible to the gate, so mutmut now generates 30 mutants for this module where it generated 20. All 30 are killed: 75.0% -> 100%. Measured by driving mutmut's own mutation engine (`mutate_file_contents` plus the `MUTANT_UNDER_TEST` trampoline) under pytest, which is the only way to run it on this machine; the same driver reproduces run 34266462154's sql_builder figures exactly, mutant name for mutant name. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CvU8wMbmkbJ6JhapXomgd2 --- tests/sdk/test_retry.py | 59 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) 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 From 10e20a58f2b0e5ff0c086804ac153a3ae10af0e6 Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Tue, 8 Sep 2026 23:14:04 -0400 Subject: [PATCH 3/6] test(mutation): make the tenant guard's tests name what they refused over Three of the sixteen mutants that survived run 34266462154 lived in assertions that were satisfied by the wrong thing: - `_qualify_table`'s refusal of an unscoped read was proved by the exception alone. A mutant that probes a different table still finds a row and still raises, so `pytest.raises` passed while the guard asked the wrong question. The test now pins the SQL the backend actually saw. - The recursive-CTE refusal matched only the first half of its message. The table it refused over is the half an operator needs when they read the 503, and it is also what stops a mutant that reports `['ORDERS']` from looking correct. The `match=` now covers the rendered name. - Nothing exercised a recursive CTE that shadows *nothing*. A guard that refused every `WITH RECURSIVE` and one that refused only the dangerous ones were indistinguishable; the new test separates them. sql_builder.py: 88.7% -> 90.8% (128 of 141 killed), measured locally against the same mutant population CI generates. That clears the 90% threshold by one mutant, which is not where this module should stay -- the thirteen remaining survivors are a separate piece of work. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CvU8wMbmkbJ6JhapXomgd2 --- tests/unit/test_sql_builder_mutation.py | 28 +++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_sql_builder_mutation.py b/tests/unit/test_sql_builder_mutation.py index d6a0f331..e6874b8d 100644 --- a/tests/unit/test_sql_builder_mutation.py +++ b/tests/unit/test_sql_builder_mutation.py @@ -597,9 +597,14 @@ def test_qualify_table_refuses_an_unscoped_read_of_a_multi_tenant_table(monkeypa # 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) - host = _host(tenant_router=_TenantRouter(has_config=True), backend=_Backend(rows=[(1,)])) + 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): @@ -672,7 +677,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", @@ -680,6 +690,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 *`. From 02b750698b91fa7a1c037d3e827fb71e833e26e5 Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Wed, 9 Sep 2026 08:06:58 -0400 Subject: [PATCH 4/6] test(soak): stop a docker-only assertion from failing the whole local suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test_merged_soak_compose_overrides_api_healthcheck_for_background_consumers` merges the three compose files by shelling out to `docker compose config`. There was no availability guard, so on a machine without the Docker CLI the call raises `FileNotFoundError: [WinError 2]` and the test fails rather than skipping — one red in 3953 tests, enough to fail every local full-suite run and hide anything that goes red after it. The repository already declares `requires_docker` ("marks tests that require local Docker") for exactly this, but the unit lane selects tests by path and never deselects by marker, so the marker alone skips nothing. The test now carries both: the marker for the vocabulary, and a `skipif` on `shutil.which("docker")` that actually takes effect. CI installs Docker, so the assertion still runs where the compose contract is worth checking. Verified on this machine: the file goes from `1 failed, 7 passed` to `7 passed, 1 skipped`, with the skip reason naming the missing CLI. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FHfk1kcS9om9nHXW32tyfo --- tests/unit/test_ci_soak_foundation.py | 7 +++++++ 1 file changed, 7 insertions(+) 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"] From 25769d7376f6f7ae1ff2f6be467a58550c110aec Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Wed, 9 Sep 2026 08:14:55 -0400 Subject: [PATCH 5/6] feat(mutation): make the mutation gate measurable locally, not once a week MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.github/workflows/mutation.yml` runs on Sundays and on dispatch, and it was the only way anyone here could see a mutation score: `mutmut run` calls `sys.exit(1)` at import time on native Windows, and this machine has no WSL. The gate was red for nine consecutive Sundays partly because nobody could see a number between pushes. `scripts/mutation_local.py --module ` measures one module of that same gate in about two minutes, and never invokes the `mutmut` CLI. It reads its targets from `scripts/mutation_report.MODULE_TARGETS` and builds its workspace with `prepare_workspace`, so the gate's definition of a target still lives in one place; it generates mutants with mutmut's own `mutate_file_contents`, so the population is the engine's, not an imitation; and it runs each mutant as a plain `pytest` subprocess selected through `MUTANT_UNDER_TEST`, which is what sidesteps the Windows guard. Four mechanics carry it. Coverage is measured first, because `mutate_only_covered_lines` makes the population coverage-dependent and getting it wrong drifts the mutant numbering away from CI's. A generated `sitecustomize.py` pre-registers a stub `mutmut.__main__` for the child processes, without which every mutant dies during collection and scores a silent, meaningless 100%. The symlinked package is materialized before the module is mutated, so a mutated source never lands in the working tree. And each mutant gets a private `--basetemp`, namespaced per invocation, because pytest wipes and recreates that directory at startup. The number is only worth having if it is about the tree in front of you: the workspace is stamped with the root, the module, its source, the materialized package, the target's tests and `pyproject.toml`, and rebuilt whenever any of those move; a mutant that returns without a verdict is retried once serially before it is called a harness failure, so the score does not track machine load; and a `--workspace` that is a checkout — this repository, anything inside it, or any directory holding a `.git` — is refused rather than emptied. Verified against CI, not just under pytest: at 10e20a5, `serving/semantic_layer/query/sql_builder.py` generates 141 mutants, the same population as CI run 34266462154 down to the surviving names, and scores 90.8% (128 killed, 13 survived, none without a verdict) in 124s. The 88.7% CI last reported plus the three mutants killed since by 2cda8da and 10e20a5. The working tree was clean afterwards. Only pytest exit 0 (survived) and 1 (killed) count as verdicts; anything else is a harness failure and fails the run, where `mutation_report.py` counts exit 3 as a kill. That is the one deliberate divergence from CI, and CONTRIBUTING says so rather than claiming exact parity. 43 unit tests cover the driver's own logic with subprocess and mutmut stubbed — no real mutation run in the suite, which would be minutes long. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FHfk1kcS9om9nHXW32tyfo --- CHANGELOG.md | 31 + CONTRIBUTING.md | 37 ++ scripts/mutation_local.py | 857 +++++++++++++++++++++++++++ tests/unit/test_mutation_local.py | 927 ++++++++++++++++++++++++++++++ 4 files changed, 1852 insertions(+) create mode 100644 scripts/mutation_local.py create mode 100644 tests/unit/test_mutation_local.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f6f0392..6c773e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,37 @@ 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. + ### 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/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 From 49fe3f9dbdd390745b2f2832bb2f6c9da0ba5432 Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Wed, 9 Sep 2026 10:16:57 -0400 Subject: [PATCH 6/6] test(mutation): take sql_builder off the threshold line, and name what stays alive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module cleared the 90% mutation threshold by a single mutant (128 killed of 141 = 90.8%), which is not a margin worth keeping: the next covered line added to it would have put the gate back in the red for reasons unrelated to the change. Thirteen mutants survived; nine of them could never have been killed. Eight mutated the type argument of a `typing.cast` — a cast returns its second argument untouched and never evaluates the first, so no test can observe the difference. Both casts are plain annotations now, which says the same thing to mypy and leaves nothing to mutate; the cast calls' own six killable mutants went with them, so the denominator shrank to 126. The ninth turned `rows = []` into `rows = None` in a branch whose next statement is `bool(rows)` — equivalent, and marked `# pragma: no mutate` in place, with the reason above it. Nothing was silenced that was not first shown to be equivalent, and the threshold in scripts/mutation_report.py is untouched. The remaining four were the `dialect="duckdb"` argument, and two of them are now dead: DuckDB indexes lists from 1 where sqlglot's default dialect does not, so `list_value(1, 2)[1]` parsed without the dialect leaves the scoper as `[2]` — the tenant scoper would have changed which element the query asked for while it added a WHERE clause. The new test states that as a property of the query, not of sqlglot. `_scope_sql__mutmut_41` and `_43` stay alive and are named in the test file with what was tried against them, rather than pragma'd: they drop the dialect from the parse of the relation `_qualify_table` generates itself, whose one fixed shape parses to an AST that renders identically under the `sql(dialect="duckdb")` applied on the way out. That is not the same as dialect-neutral — the default-dialect *render* of that shape rewrites `EXCLUDE` to `EXCEPT`, which is exactly why the render-side mutants are dead and must stay pinned. Measured with scripts/mutation_local.py (py3.13, sqlglot 30.12.0) against the bytes that ship: 126 generated, 124 killed, 2 survived, 98.4%. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018GqSmXR1bp1xPTZdsdjgTE --- CHANGELOG.md | 29 ++++++ .../semantic_layer/query/sql_builder.py | 26 +++-- tests/unit/test_sql_builder_mutation.py | 94 ++++++++++++++++++- 3 files changed, 139 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c773e77..03010cec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,35 @@ All notable changes to AgentFlow are documented in this file. `.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/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/unit/test_sql_builder_mutation.py b/tests/unit/test_sql_builder_mutation.py index e6874b8d..e16af242 100644 --- a/tests/unit/test_sql_builder_mutation.py +++ b/tests/unit/test_sql_builder_mutation.py @@ -60,10 +60,37 @@ 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 only source of truth for this score; -mutant *counts* differ per interpreter, because ``mutate_only_covered_lines`` -makes the population depend on coverage attribution. Do not restate a number -here that was not read off a mutation.yml run, and record the run id with it. +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 @@ -770,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. +# --------------------------------------------------------------------------- #