Skip to content

Commit 49fe3f9

Browse files
JuliaEdomclaude
andcommitted
test(mutation): take sql_builder off the threshold line, and name what stays alive
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018GqSmXR1bp1xPTZdsdjgTE
1 parent 25769d7 commit 49fe3f9

3 files changed

Lines changed: 139 additions & 10 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,35 @@ All notable changes to AgentFlow are documented in this file.
8585
`.git` — is refused instead of emptied. The mutated module never leaves the
8686
temp workspace: the working tree is clean after a run.
8787

88+
* **`sql_builder.py` is off the threshold line, and its residue is honest.**
89+
It cleared 90% by a single mutant (90.8%, 128 killed of 141), which is not a
90+
margin worth keeping: the next covered line added to the module would have
91+
put the gate back in the red for reasons unrelated to the change. Nine of the
92+
thirteen survivors could never have been killed. Eight mutated a
93+
`typing.cast` type argument — a cast returns its second argument untouched
94+
and never evaluates the first — so both casts are plain annotations now and
95+
the mutants stop existing; the ninth turned `rows = []` into `rows = None` in
96+
a branch whose next statement is `bool(rows)`, and carries a
97+
`# pragma: no mutate` with the reason above it. The remaining four were the
98+
`dialect="duckdb"` argument, and two of them are now dead: DuckDB list
99+
indexing is 1-based where sqlglot's default dialect is not, so
100+
`list_value(1, 2)[1]` read without the dialect comes back out of the scoper
101+
as `[2]` — the tenant scoper would have changed which element the query asked
102+
for while it added a WHERE clause. The module measures 98.4% (124 killed of
103+
126) with `scripts/mutation_local.py` on py3.13.
104+
* **The two mutants still alive are named in the test file, not suppressed.**
105+
`_scope_sql__mutmut_41` and `_43` drop the dialect from the parse of the
106+
relation `_qualify_table` generated itself, and that string has one fixed
107+
shape which — parsed with the dialect or without it — renders identically
108+
under the `sql(dialect="duckdb")` `_scope_sql` applies on the way out, so no
109+
input reaches them with a difference to observe. Not the same as neutral: the
110+
*default-dialect render* of that shape rewrites `EXCLUDE` to `EXCEPT`, which
111+
is why the mutants on the render itself stay killable and dead.
112+
They are not equivalent — a `_qualify_table` that ever emitted
113+
DuckDB-specific syntax would make them killable — so they get a written
114+
record of what was tried and came out identical rather than a pragma that
115+
would outlive its reason.
116+
88117
### Terraform — an exact core pin took the provider update channel down with it
89118

90119
* **`required_version = "= 1.15.4"` broke Dependabot's terraform ecosystem the

‎src/agentflow_runtime/serving/semantic_layer/query/sql_builder.py‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from __future__ import annotations
22

33
import re
4-
from typing import cast
54

65
import sqlglot
76
from sqlglot import exp
@@ -97,7 +96,14 @@ def _holds_foreign_tenant_rows(self: SQLBuilderHost, physical: str) -> bool:
9796
One probe per table per process (cached), like the ``_table_columns``
9897
probe the old guard used.
9998
"""
100-
cache = cast("dict[str, bool] | None", getattr(self, "_foreign_tenant_cache", None))
99+
# Declared rather than `cast(...)`. `typing.cast` returns its second
100+
# argument untouched and never evaluates the first, so every mutant of a
101+
# cast's type argument is equivalent by construction: no test can tell
102+
# `cast("dict[str, bool] | None", x)` from `cast(None, x)`. Eight such
103+
# mutants — four here, four in `_qualify_table` — sat in the mutation
104+
# gate's denominator (CI run 34266462154) pretending to be gaps. An
105+
# annotation says the same thing to mypy and leaves nothing to mutate.
106+
cache: dict[str, bool] | None = getattr(self, "_foreign_tenant_cache", None)
101107
if cache is not None and physical in cache:
102108
return cache[physical]
103109

@@ -112,7 +118,13 @@ def _holds_foreign_tenant_rows(self: SQLBuilderHost, physical: str) -> bool:
112118
)
113119
except BackendExecutionError:
114120
# Not materialized yet, or no tenant column: nothing to leak.
115-
rows = []
121+
#
122+
# `rows = None` is the only mutant of this line and it is equivalent:
123+
# `rows` is read exactly once, by the `bool(rows)` below, and
124+
# `bool([]) == bool(None) == False`. Marked so it stops being
125+
# generated — the gate's denominator should hold only mutants a test
126+
# could kill.
127+
rows = [] # pragma: no mutate
116128
found = bool(rows)
117129
if cache is not None:
118130
cache[physical] = found
@@ -141,9 +153,11 @@ def _qualify_table(self: SQLBuilderHost, table_name: str, tenant_id: str | None)
141153
promises and the two stores stay column-identical.
142154
"""
143155
predicate = self._tenant_predicate(tenant_id)
144-
cache = cast(
145-
"dict[tuple[str, str | None], str] | None",
146-
getattr(self, "_qualified_table_cache", None),
156+
# Declared, not `cast(...)`, for the reason spelled out in
157+
# `_holds_foreign_tenant_rows`: a cast's type argument is erased at
158+
# runtime, so its mutants are unkillable by construction.
159+
cache: dict[tuple[str, str | None], str] | None = getattr(
160+
self, "_qualified_table_cache", None
147161
)
148162
cache_key = (table_name, predicate)
149163
if cache is not None and cache_key in cache:

‎tests/unit/test_sql_builder_mutation.py‎

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,37 @@
6060
describing a mutant population that no longer existed, and it read as
6161
reassurance while a tenant-isolation guard sat unpinned.
6262
63-
The CI gate (mutation.yml on py3.11) is the only source of truth for this score;
64-
mutant *counts* differ per interpreter, because ``mutate_only_covered_lines``
65-
makes the population depend on coverage attribution. Do not restate a number
66-
here that was not read off a mutation.yml run, and record the run id with it.
63+
The CI gate (mutation.yml on py3.11) is the source of truth for the score CI
64+
enforces, but since ``25769d7`` the same population is measurable here too:
65+
``python scripts/mutation_local.py --module
66+
serving/semantic_layer/query/sql_builder.py``, about two minutes. Mutant
67+
*counts* still differ per interpreter, because ``mutate_only_covered_lines``
68+
makes the population depend on coverage attribution, so a number written down
69+
here has to carry where it was measured.
70+
71+
Two measurements, both py3.13 in ``.venv`` on 2026-09-09. At ``25769d7``: 141
72+
mutants, 128 killed, 13 survived, 90.8% -- the same population CI run
73+
34266462154 generated, down to the surviving mutant names. After this file and
74+
``sql_builder.py`` were changed to take the module off the threshold line: 126
75+
mutants, 124 killed, 2 survived, 98.4%.
76+
77+
Fifteen mutants left the denominator, and which ones matters more than the
78+
count. Eight were mutants of a ``typing.cast`` type argument -- a cast returns
79+
its second argument untouched and never evaluates the first, so no test can
80+
ever tell them from the original. Six more were mutants of the ``cast(...)``
81+
*call itself* (its argument count, its second argument); they were killable,
82+
and they stopped existing along with the two casts, which are now plain
83+
annotations. The last one is the equivalent ``rows = []`` -> ``rows = None`` in
84+
``_holds_foreign_tenant_rows``, marked ``# pragma: no mutate`` in place. The
85+
casts were not pragma'd instead, because mutmut's pragma is recorded against a
86+
whole *statement*, at its first line. On the one-line cast it would have taken
87+
the eleven killable mutants on that line (the ``getattr``'s own seven, the
88+
assignment, and the cast call's argument mutants) out of the denominator as
89+
well -- the opposite of the point -- and on the four-line one in
90+
``_qualify_table`` it would not have reached the type string at all, only the
91+
``cache = ...`` on the opening line. Two further mutants were killed rather
92+
than removed, by
93+
``test_scope_sql_does_not_change_which_list_element_the_query_asks_for``.
6794
"""
6895

6996
from __future__ import annotations
@@ -770,3 +797,62 @@ def test_scope_sql_forwards_the_tenant_id_to_qualify_table():
770797
)
771798
host._scope_sql("SELECT * FROM widgets JOIN orders ON widgets.id = orders.id", "acme")
772799
assert calls == [("orders", "acme")]
800+
801+
802+
# --------------------------------------------------------------------------- #
803+
# The dialect the incoming SQL is read in. Scoping a query must not change what
804+
# the query means.
805+
# --------------------------------------------------------------------------- #
806+
807+
808+
def test_scope_sql_does_not_change_which_list_element_the_query_asks_for():
809+
# `dialect="duckdb"` on the parse of the *incoming* SQL is not decoration.
810+
# DuckDB's list indexing is 1-based; sqlglot's default dialect reads the
811+
# same `[1]` as 0-based and re-renders it as `[2]` on the way out. So a
812+
# caller that asked for the first element of `list_value(1, 2)` would get a
813+
# scoped query asking for the second one -- the tenant scoper would have
814+
# silently changed the answer while adding a WHERE clause. Kills the
815+
# `dialect=None` and dropped-`dialect` mutants on the parse of the incoming
816+
# SQL (`_scope_sql__mutmut_8` and `_10`).
817+
host = _host(catalog=_Catalog("orders"), tenant_router=_TenantRouter(has_config=True))
818+
out = host._scope_sql("SELECT list_value(1, 2)[1] AS x FROM orders", "acme")
819+
assert out == f"SELECT [1, 2][1] AS x FROM {SCOPED_ORDERS_ACME}"
820+
821+
822+
# --------------------------------------------------------------------------- #
823+
# Two mutants of this module are left alive on purpose. This is the record of
824+
# why, so the next reader does not mistake them for a gap (measured 2026-09-09
825+
# with `python scripts/mutation_local.py --module
826+
# serving/semantic_layer/query/sql_builder.py`, sqlglot 30.12.0):
827+
#
828+
# _scope_sql__mutmut_41 parse_one(scoped, dialect=None)
829+
# _scope_sql__mutmut_43 parse_one(scoped)
830+
#
831+
# Both mutate the *second* parse in `_scope_sql` -- the one that reads `scoped`,
832+
# the relation `_qualify_table` built a line earlier, not anything a caller
833+
# supplied. That string has one fixed shape,
834+
#
835+
# (SELECT * EXCLUDE (tenant_id) FROM <table> WHERE tenant_id = '<id>')
836+
# AS "<table>"
837+
#
838+
# and whichever dialect parses it -- `duckdb` or sqlglot's default -- the AST
839+
# that comes back renders identically under the `sql(dialect="duckdb")`
840+
# `_scope_sql` always applies on the way out. That is the invariant that makes
841+
# the two mutants unreachable, and it is narrower than "dialect-neutral": the
842+
# *default-dialect render* of that same AST is not identical, it rewrites
843+
# `EXCLUDE (tenant_id)` to `EXCEPT (tenant_id)`. Which is exactly why the
844+
# mutants on the render (`parsed.sql(dialect="duckdb")`, line 240) are dead and
845+
# pinned -- do not weaken that argument on the strength of this note. Tried on
846+
# the parse side, and identical under the duckdb render either way: that exact
847+
# sub-select, a bare `SELECT * EXCLUDE (col)`, a struct literal, and FROM-first
848+
# syntax. The one construct that does differ is the list indexing the test
849+
# above uses, and it cannot appear here -- this module writes the string
850+
# itself, and never writes that.
851+
#
852+
# They are deliberately NOT marked `# pragma: no mutate`, unlike the `rows = []`
853+
# mutant in sql_builder.py. That one is unkillable by construction; these two are
854+
# merely unreachable through the generator as it stands today, and a
855+
# `_qualify_table` that ever emitted DuckDB-specific syntax would make them
856+
# killable again. A suppression would outlive the reason for it; this note does
857+
# not.
858+
# --------------------------------------------------------------------------- #

0 commit comments

Comments
 (0)