A failing golden item can say which claim moved, and gate on the two that never false-positive - #238
Open
vishalkalbi27 wants to merge 12 commits into
Open
A failing golden item can say which claim moved, and gate on the two that never false-positive#238vishalkalbi27 wants to merge 12 commits into
vishalkalbi27 wants to merge 12 commits into
Conversation
…ot false-positive
…laim, not a crash A statement writing LIMIT 1.5, LIMIT 1e3 or EXTRACT(YEAR FROM d) = 2025.5 parses, and reading it raised ValueError straight out of read_claims — whose whole contract is that it never does, because the caller is an eval run that must survive whatever a generator emits. A non-integral literal is now a shape the module declines to compare, which is the None every other unmodelled shape already reads as. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bounds were compared as raw strings, so a generated d >= '2025-01-01 00:00:00' AND d < '2026-01-01 00:00:00' gated against a golden d >= '2025-01-01' AND d < '2026-01-01'. That is a correct statement failed on this module's own spelling preferences, which is the outcome the two gates were selected to make impossible. A bound is now canonicalized before it is compared — a zero time-of-day dropped, the T separator folded — and still REPORTED exactly as the statement wrote it. The fold is sound where the inclusive-upper-bound shift the module refuses is not, and the comment says which side of that line it sits on: dropping midnight names the same instant whatever the column's type, moving '2025-12-31' to '2026-01-01' is true only on a DATE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four values arrived in the diff unbounded, and the diff rides on output the calling model reads as server-authored: * a date bound, because the ISO pattern ended in a trailing .* — a 200,000-character literal carrying CR and ESC survived intact into the JSON. The time half is spelled out as a grammar now, which is also what makes the pattern the bound on the one claim value that never passes through the expression echo. * a table name and both endpoints of a join key, which were only case-folded. A quoted identifier is caller-written text that no fold sanitizes; each goes through the same per-name bound the rest of the package echoes a name with. * a gate's column, passed through from the dataset's own must_filter entry. The required-column gate also dedupes on the normalized name — a dataset listing a column twice was asking for one thing and got two identical verdicts beside a failing item — normalizes with a plain lowercase rather than the table-name helper, and its docstring now says what it is NOT: it matches a bare column name across every SELECT in the tree, so it is not usable as a tenancy or row-scope check even though its name reads like one. The echo test asserted that no three-word span of the statement survived, which held only because the fixture's literals were single tokens: a claim key embeds the caller's literals BY DESIGN, so two statements filtering on different values cannot compare equal. It now asserts what the module actually holds — neither statement whole, no value parseable as a statement, no clause keyword as a TOKEN rather than a substring (a column named from_date leaked nothing), and the expression bound — with a second test stating the carried-literal behavior as the intent it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mported on read_claims carried a second unreadable sentence for the case where _parse_reporting returns no reason, which it does only on its no-sqlglot path — and this module imports sqlglot's expressions unguarded at the top, so it raises ModuleNotFoundError on exactly that build and the branch cannot be reached. The constant and the comment asserting its reachability go with it. Also: the not-a-single-SELECT sentence claimed the statement's claims belong to its arms, which is true of a set operation and false of the DELETE, INSERT and UPDATE the module emits it verbatim for; it is printed beside a failing item, so it now says only what is generally true. And the leaf guard in the key renderer tested a condition its first half already implies, since iter_expressions yields every Expression-valued arg including this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dered The gate matches a required column against `filtered_columns` by its normalized spelling, so the set has to hold what the statement wrote. That left the one claim value a caller could read unbounded and unsanitized, which is the channel every other value is bounded to close. The bound belongs on the render instead.
vishalkalbi27
requested review from
ashwin-agami and
sandeep-agami
as code owners
August 25, 2026 11:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a golden-dataset item fails, the run can say the two result sets disagree but not why. The
person who edits the semantic model needs the sentence after that: the statement filtered a
different quarter, or dropped a required predicate, or joined through a different key.
This reads each statement into a fixed set of seven structural claims — tables, filter predicates,
the resolved date window, group keys, join keys, ordering, limit — and diffs them, so a failure
reads as a named difference instead of two tables that disagree.
Two of those claims also gate, and only two. A question scoped to a date range answered by a
statement with no date predicate is wrong even when the numbers happen to agree on today's data; so
is a statement that drops a required filter. Neither shows up in the rows until the data changes
underneath, which is exactly when nobody is looking. Everything else is reported and never
gates, because predicate placement is free and rewrites are unbounded: two statements that compute
the same value, one filtering in
WHEREand one inside the aggregate, differ structurally and agreeon every row. A comparator that gated on that would fail correct answers routinely. What is decidable
is what a claim says, not whether two statements are equivalent — and the second question has no
general answer.
unknownis a first-class outcome. A predicate the resolver cannot fold, or a statement it cannotparse, reports
unknownand gates nothing. A resolver that guessed would fail correct statements inexactly the cases nobody can debug.
Spec: AH-108
Changes
semantic_model/golden_claims.py—read_claims,diff_claims, andcompare_statements,which is the one a runner calls. Nothing else in the package changes; there is no new dependency,
no packaging change, and no caller yet.
CLAIM_NAMESis the whole vocabulary and atest asserts its length, because every additional claim is another false-positive surface.
>= '2025-01-01' AND < '2026-01-01'andEXTRACT(YEAR FROM d) = 2025fold to the same half-open interval and agree; three spellings of oneyear are common enough that a syntactic comparison would report a difference on most correct
answers. Each bound keeps its inclusivity and is not normalized to the next day — that rewrite
is sound only on a
DATEcolumn and nothing here carries a column type — which is what makesBETWEEN '2025-01-01' AND '2025-12-31'over a timestamp visible as the off-by-one it is, namingthe bound that moved. Midnight-equivalent spellings are folded, because that normalization is
type-independent.
WHERE, every joinON,HAVING,QUALIFY, and an aggregate'sFILTER (WHERE …). It deliberately errs toward"filtered": a gate chosen for never false-positiving has to. It is therefore not a tenancy or
row-scope check, and its docstring says so, because the field reads like one.
runtime._parse_reporting, and normalizationreuses
_fold_unquoted_identifiers,_own_alias_map,_table_references,_filtering_conjuncts,_mentioned_predicates,_predicate_columnsand_predicate_pairs. Alias binding uses_own_alias_maprather than_alias_map, which is subtree-wide and last-wins and would mis-bind acorrelated subquery.
re-serializing a parsed statement, so
.sql()was not available — and the structural key is thebetter answer anyway: it is what makes "the diff carries claims rather than a clause of SQL" a
property of the representation instead of a property of the fixtures.
per-name and per-expression echo bounds. Identifiers and literals are caller-written text, and a
claim rides on output a model reads as server-authored.
filtered_columnsis held as the statementspelled it — the gate matches against it — and bounded when it is rendered.
SELECT, or carries a literalthe resolver cannot read comes back as a claim set that says so, not as an exception out of a run.
Complexity tracking
filtered_columns, distinct fromfilter_predicatesunreadable: Optional[str]_rendered(), a structural key renderer_MAX_KEY_DEPTH/_DEEPERORleft-deep, so a wide predicate is a deep tree; unbounded recursion would raise out of a function whose contract is that it never raises. Affects only a report-only claim, never a gate._canonical_bound()Checklist
uv run dev.py checkgreen — ruff, the full suite, gitleaks.tests/test_golden_claims.py, every case asserted on twodialects. 100% patch coverage on the new module.
Review
Reviewed before opening: correctness/tests, security, and the house structural rubric. Six
must-fixes were found and fixed, the two most consequential being a
ValueErrorthat escaped thenever-raises contract on a non-integer literal (
LIMIT 1.5), and a date gate that fired on'2025-01-01 00:00:00'versus'2025-01-01'— a correct statement failed on a spelling, which isthe one outcome these two gates exist to avoid. Both are now pinned by tests.
🤖 Generated with Claude Code