Recover a CREATE VIEW/TABLE swallowed by a preceding broken routine - #2724
Recover a CREATE VIEW/TABLE swallowed by a preceding broken routine#2724ayushcodes10 wants to merge 15 commits into
Conversation
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
This PR adds T-SQL bracket-quoted identifier support to the SQL extractor and broadens name-part handling across its regex fallback paths. Specifically, it introduces a _debracket_tsql pre-parse pass that rewrites [bracket]-quoted identifiers to backtick-quoted ones (with guards for strings/comments and array-type syntax), plus helpers (_strip_backtick_parts, _clean_name, _ident) to strip that synthetic quoting from display labels. It also factors identifier-part matching into a shared _SQL_NAME_PART regex used by the ERROR-node fallbacks for CREATE FUNCTION/PROCEDURE and swallowed CREATE VIEW/TABLE statements. The change touches the SQL extractor module and a large set of multilang extractor tests (Go, Rust, TypeScript, SQL) that appear to have been updated or added alongside it.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 279 functions depend on the 125 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract_sql()— 17 callers, 10 callees - worse:
walk()— 1 callers, 10 callees
Verification — 279 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 136 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 1 more finding(s) on lines outside this diff (see the check run).
ayushcodes10
changed the title
|
@safishamsi this PR's CI run is waiting on approval (first-time contributor gate) — could you approve the workflow run when you get a chance? Happy to address any review feedback in the meantime. |
tree_sitter_sql has no grammar rule for a bracket delimited identifier, so [dbo].[Orders] lands in ERROR nodes and comes out with a mangled label like dbo].[Orders instead of dbo.Orders. Backtick quoting already parses cleanly, so the source is rewritten before parsing and the synthetic backticks are stripped again for display. Upstream has since landed its own ERROR recovery for routine names (CREATE FUNCTION/PROCEDURE), matched by a shared regex plus a masked whole file scan. Since this rewrite runs before that scan, a bracketed routine name never reaches it as a bracket anymore, only as a backtick, so the shared regex and its masking scan are extended to also recognize backtick delimited names. Without that a bracketed routine would go from a recovered (if ugly) node to no node at all, a regression this change must not introduce. Guards against misfiring on array type and array literal syntax that also uses square brackets (int[], numeric(10)[3], ARRAY[1,2,3]), and against a bracket span that swallows a comment marker. Fixes Graphify-Labs#2712.
b369295 to
f8b6def
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Adds T-SQL bracket-quoted identifier support to the SQL extractor by rewriting [dbo].[Orders] spans to backtick-quoted form via _debracket_tsql before parsing, so statements the grammar would otherwise dump into ERROR nodes take the normal AST path; recovered names and display labels are un-rewritten by _strip_backtick_parts, applied only when the file was actually debracketed. Extends the error-recovery scan and _scan_sql masking to recognize backtick delimiters, and adds _VIEW_TABLE_RECOVERY_RX to recover CREATE TABLE/VIEW statements swallowed into an unrelated ERROR node's span, deduped against already-parsed tables. Array markers (int[], ARRAY[1,2,3]) and comment-bearing or numeric bracket content are left alone so they aren't misread as identifiers.
Worth a look
- CREATE TABLE IF NOT EXISTS recovery captures IF as the table name —
graphify/extractors/sql.py:52· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _debracket_tsql treats closing ) as subscript-like, breaking bracketed identifier after a parenthesized expression —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _strip_backtick_parts strips backticks whenever any bracket was debracketed, corrupting genuine MySQL backtick identifiers in same file —
graphify/extractors/sql.py:410· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 349 functions depend on the 169 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 29 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 349 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 181 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| parts.append(p) | ||
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 29 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
@safishamsi rebased onto the latest v8 and all checks are green now — ready for review whenever you get a chance. |
_clean_name gated on a single file wide debracketed flag, so once ANY bracket span in a file got rewritten to backtick form, every backtick-quoted name in that same file had its backticks stripped for display, including a genuinely backtick-quoted MySQL name completely unrelated to the T-SQL bracket span that triggered the rewrite. _debracket_tsql now returns the byte range spans it actually rewrote, in the rewritten source's own coordinates, and _clean_name only un-rewrites a name whose own byte range overlaps one of those spans. A name read off a tree-sitter node carries its own start/end byte naturally; the whole file regex fallback has no node, so its character offsets into the decoded source are converted to byte offsets first, since the decoded string and the byte-level spans can diverge once the file contains any multi byte character. Fixes Graphify-Labs#2721.
f8b6def to
56d4805
Compare
|
Same span-tracking fix as #2723 (see that PR for full details), plus a real bug the bot correctly flagged as high severity: _VIEW_TABLE_RECOVERY_RX was missing the IF NOT EXISTS gap _ROUTINE_RECOVERY_RX already had, so CREATE TABLE/VIEW IF NOT EXISTS in an error-bearing file fabricated a phantom node literally named "IF" (the capture group greedily grabbed the next bare word after CREATE TABLE/VIEW). Fixed and added a regression test. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Rewrites T-SQL bracket-quoted identifiers ([dbo].[Orders]) to backtick-quoted form before parsing via _debracket_tsql, so statements the grammar has no rule for take the normal AST path instead of falling into error recovery that mangled labels and dropped tables behind broken foreign keys; array-type markers like int[] and bracket spans containing comment openers are left alone. Un-rewrites names for display through _clean_name/_strip_backtick_parts, but only where a node's bytes overlap a span _debracket_tsql actually inserted, so a genuine backtick-quoted MySQL name elsewhere in the same file keeps its backticks. Extends the error-recovery scan to accept backtick names and adds _VIEW_TABLE_RECOVERY_RX to recover CREATE TABLE/VIEW statements swallowed into an unrelated ERROR node's span, deduping against already-parsed tables.
Worth a look
- Debracketing can change byte offsets used by extract_sql locations —
graphify/extractors/sql.py:459· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 355 functions depend on the 175 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 31 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 355 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 187 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| parts.append(p) | ||
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 31 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Exclude two cases the grammar or the scanner cannot handle safely: a bracket span containing a literal dot, which tree_sitter_sql still splits on even inside a backtick pair, and single/double quoted string content past its first line, which the scanner used to treat as ordinary source once it hit a newline before finding the real closing quote.
56d4805 to
fca75f8
Compare
A dollar quoted span ($$ ... $$ or $tag$ ... $tag$) is opaque body text, not SQL, so bracket like content inside it was being mistaken for a real bracket identifier and rewritten. Skip the whole span instead, the same way a string or comment already is.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Graphify review — findings
Rewrites T-SQL bracket-quoted identifiers ([dbo].[Orders]) into backtick-quoted ones via _debracket_tsql before parsing, so the grammar handles them on the normal AST path instead of dropping them into ERROR nodes that mangled labels and lost tables behind broken foreign keys; the inserted backtick spans are tracked so _strip_backtick_parts un-rewrites only bracket-derived names and leaves genuine MySQL backtick identifiers alone. Excludes array-marker brackets (int[], ARRAY[1,2,3]), comment-bearing, and dotted contents from rewriting, falling back to the pre-existing plain-bracket behavior for those. Adds _VIEW_TABLE_RECOVERY_RX to recover CREATE TABLE/VIEW statements swallowed into an unrelated ERROR span, deduped against already-parsed tables, and teaches _scan_sql plus the routine-recovery regex to treat backtick-delimited identifiers as recoverable names.
Worth a look
- _VIEW_TABLE_RECOVERY dedup only checks table_nids, not view/other node ids —
graphify/extractors/sql.py:928· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 359 functions depend on the 179 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 32 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 359 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 191 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| parts.append(p) | ||
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 32 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
fca75f8 to
9dfff03
Compare
|
Investigated the latest bot findings on this PR. Confirmed real, fixed (propagated from the base branch, now at 9dfff03): the dot-inside-brackets grammar limitation, the multi-line quoted-string corruption bug, and the missing dollar-quoted-body handling in _debracket_tsql — same three fixes described on #2723/#2722/#2721, all with regression tests. Also included in this branch's own history: the IF NOT EXISTS phantom node fix already noted in the earlier comment on this PR. Investigated and found to be a false positive: "Debracketing can change byte offsets used by extract_sql locations" (sql.py:459). Same conclusion as posted on #2721 — source_location here is line-only, never byte-based, and _debracket_tsql never inserts or removes a newline, so a within-line byte-length shift from a rewrite cannot change which line any later content is reported on. Verified empirically. Already covered on the base PR (#2723): the reproduced |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Adds T-SQL bracket-identifier recovery to the SQL extractor so [dbo].[Orders]-style names parse cleanly. _debracket_tsql rewrites bracket-quoted identifiers to backtick-quoted ones before parsing — skipping strings, dollar-quoted bodies, comments, and array-subscript brackets, and leaving dotted names like [My.Table] as plain brackets since the grammar can't round-trip them — while _strip_backtick_parts restores the original labels only for spans it inserted. Also adds _VIEW_TABLE_RECOVERY_RX to recover a CREATE TABLE/VIEW statement swallowed into another statement's ERROR span, deduped against already-parsed tables, and teaches _scan_sql to preserve backtick-delimited identifiers so the routine-recovery scan matches them too.
Worth a look
- Debracketing rewrites brackets inside existing backtick identifiers —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Table/view recovery can match DDL inside dollar-quoted function bodies —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 361 functions depend on the 181 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 32 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 361 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 193 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| parts.append(p) | ||
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 32 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
A MySQL backtick quoted identifier had no dedicated scan branch, so its content was walked like ordinary source. A bracket like substring inside one read as a real TSQL bracket identifier and got rewritten in place, producing doubled and orphaned backticks. Skip a backtick span whole, honoring a doubled backtick as an escaped literal one the same way this function writes it.
9dfff03 to
df3773f
Compare
|
Follow up on this round's bot findings (matching df3773f): Confirmed real, fixed (propagated from the base branch): the same backtick-corruption bug described on #2723/#2722/#2721 — a genuine backtick-quoted identifier containing bracket-like text got corrupted because _debracket_tsql had no dedicated scan branch for a backtick span. Fixed the same way, with a regression test. Investigated and found to be pre-existing, out of scope for this PR: "Table/view recovery can match DDL inside dollar-quoted function bodies". _VIEW_TABLE_RECOVERY_RX runs against _scan_sql's masked output, which does not model PostgreSQL dollar-quoting at all (documented in _scan_sql's own comment block) -- gated by root.has_error, so it only ever runs on a file that already failed to parse. Confirmed this is systemic, not something this PR's new regex introduced: building an equivalent repro against the pre-existing _ROUTINE_RECOVERY_RX (which predates this PR by several releases) reproduces the identical fabricated-node behavior for a CREATE PROCEDURE-shaped string sitting in the same spot. _VIEW_TABLE_RECOVERY_RX inherits this by design -- its own comment says it shares _ROUTINE_RECOVERY_RX's masked scan and gating -- so it is not a new regression, just the same known, accepted error-recovery-only trade-off applying to one more regex built on the same masked_src. No code change made for this one. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Adds T-SQL bracket-identifier handling to the SQL extractor: _debracket_tsql rewrites [dbo].[Orders]-style bracket-quoted names into backtick-quoted ones before parsing so the grammar's normal AST path handles them instead of falling into error recovery, distinguishing genuine identifiers from array markers, comments, dotted names, and quoted/dollar-quoted spans (with plain brackets left un-rewritten as the fallback). Extends _scan_sql and _ROUTINE_RECOVERY_RX to accept backtick-delimited identifiers so bracket-named routines still recover after a rewrite, and adds _VIEW_TABLE_RECOVERY_RX to recover CREATE TABLE/VIEW statements swallowed into an ERROR node's span, deduped against already-parsed tables.
Worth a look
- _debracket_tsql return type changed from bytes to tuple —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Backtick-escape doubling not applied when rewriting bracket identifier containing a backtick —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 363 functions depend on the 183 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 32 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 363 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 195 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| parts.append(p) | ||
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 32 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
A bracket span containing a literal backtick cannot be represented by rewriting to backtick form: the grammar treats the first backtick of the doubled escape this function would write as the closing delimiter rather than an escape, truncating the identifier and leaving the rest as a stray ERROR node. Also stop rewriting a PostgreSQL ARRAY constructor when whitespace separates the keyword from its bracket (ARRAY [1, 2, 3]). The preceding character check only looked one character back, which is a space in that shape rather than the keyword, so the array literal read as an identifier and got rewritten.
_VIEW_TABLE_RECOVERY_RX lacked the IF NOT EXISTS gap _ROUTINE_RECOVERY_RX already carries. CREATE TABLE/VIEW IF NOT EXISTS in an error bearing file matched, but the capture group greedily grabbed the next bare word after CREATE TABLE/VIEW, which was IF, not the real name one word later, fabricating a phantom node literally named IF. Fixes Graphify-Labs#2724.
df3773f to
6058080
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Rewrites T-SQL bracket-quoted identifiers ([dbo].[Orders]) into backtick-quoted ones before parsing via _debracket_tsql, so statements the grammar previously dropped into ERROR nodes now take the normal AST path, while carefully distinguishing quoted names from array markers (int[], ARRAY[...]) and skipping string/comment/dollar-quoted/backtick spans; the inserted-span coordinates are returned so a downstream reader can un-rewrite only the brackets it introduced and leave genuine MySQL backtick names alone. Extends the routine-recovery scan to accept backtick names and adds _VIEW_TABLE_RECOVERY_RX so a CREATE TABLE/VIEW swallowed into an ERROR span is recovered and deduped against already-parsed tables. Falls back to leaving a bracket un-rewritten for shapes the grammar still can't round-trip (embedded ., literal backtick, comment openers).
Worth a look
- Block comment scan can loop forever when unterminated —
graphify/extractors/sql.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _debracket_tsql return type changed from bytes to tuple —
graphify/extractors/sql.py:543· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 367 functions depend on the 187 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 32 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 367 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 199 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 32 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
src_text decoded with errors=replace, which collapses every invalid byte to one U+FFFD that re encodes to 3 bytes, permanently inflating every offset _clean_regex_name computes past an invalid byte anywhere earlier in the file. A routine recovered only through the regex fallback then had its overlap check miss every real debracket span, so a name debracketing did touch kept its backticks in the final label. Decode with surrogateescape instead, which round trips losslessly, and sanitize a name on the way out in case its own content carried one of those escaped bytes, since a label is not meant to hold a lone surrogate.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Rewrites T-SQL bracket-quoted identifiers ([dbo].[Orders]) to backtick-quoted form before parsing via _debracket_tsql, so statements the grammar has no rule for take the normal AST path instead of collapsing into ERROR nodes that mangle labels or drop whole tables behind broken foreign keys. Only true identifiers are rewritten — array markers (int[], ARRAY[...]), empty/numeric content, comment-openers, and names containing . or a literal backtick are left as plain brackets since the grammar can't round-trip them, and quoted/dollar-quoted/backtick spans are skipped so their contents aren't corrupted. Extends the ERROR-recovery scans to accept backtick-quoted names and adds _VIEW_TABLE_RECOVERY_RX to recover a CREATE TABLE/VIEW swallowed into an unrelated ERROR span, deduped against already-parsed tables.
Worth a look
- Block comment scan can loop forever when unterminated in _debracket_tsql —
graphify/extractors/sql.py· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- ARRAY constructor after newline is rewritten as an identifier —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- View/table recovery scans string literals and can fabricate declarations —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Table/view recovery misses T-SQL CREATE OR ALTER VIEW —
graphify/extractors/sql.py:60· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 369 functions depend on the 189 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 33 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 369 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 201 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 33 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
_VIEW_TABLE_RECOVERY_RX lacked the IF NOT EXISTS gap _ROUTINE_RECOVERY_RX already carries. CREATE TABLE/VIEW IF NOT EXISTS in an error bearing file matched, but the capture group greedily grabbed the next bare word after CREATE TABLE/VIEW, which was IF, not the real name one word later, fabricating a phantom node literally named IF. Fixes Graphify-Labs#2724.
a561ff8 to
c18bf47
Compare
|
Investigated the latest bot findings on this PR. Confirmed real, fixed (now at c18bf47, on the base branch and propagated through the whole stack -- _clean_regex_name and src_text originate on the base, not here): "_debracket_tsql return type changed from bytes to tuple" describes an accurate but non-actionable observation -- confirmed the one production call site (extract_sql itself) and every test call site already correctly unpack the new 2-tuple signature. Not a bug; the function is private with a single, already-updated caller. Also confirmed real and fixed: src_text (used by the whole-file regex recovery fallback) decoded with errors="replace", which collapses each invalid UTF-8 byte to one U+FFFD that re-encodes to 3 bytes -- permanently inflating every byte offset _clean_regex_name computes past an invalid byte anywhere earlier in the file. Reproduced concretely: a procedure recovered only through the regex fallback (its AS BEGIN...END body has no grammar rule at all, so it can never reach the tree-sitter-node path) had its overlap check miss every real debracket span once 200 invalid bytes preceded it in the file, so a name debracketing did touch kept its backticks in the final label ( Investigated and found to be a false positive: "Block comment scan can loop forever when unterminated" (high severity). Checked all three block-comment loops in this file (the two in _scan_sql plus the one in _debracket_tsql) -- each advances its scan index by 1 or 2 every iteration with no other way out, so each is mathematically bounded by the source length and cannot loop forever. Verified empirically under a timeout with several adversarial unterminated/nested-comment inputs, including a 100k-byte unterminated comment: all completed instantly. No code change made for this one. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Adds a T-SQL debracketing pass (_debracket_tsql) that rewrites [dbo].[Orders]-style bracket-quoted identifiers into backtick-quoted ones before parsing, so statements the grammar can't handle in brackets take the normal AST path instead of falling into error recovery; it skips string/dollar-quoted/backtick spans and leaves spans containing dots, embedded backticks, or comment openers un-rewritten as a fallback, and returns the inserted backtick spans so a downstream reader can un-rewrite only bracket-derived names. Extends the routine-recovery regex and _scan_sql to recognize backtick-quoted names (what debracketing produces), and adds _VIEW_TABLE_RECOVERY_RX to recover a CREATE TABLE/VIEW swallowed into an unrelated ERROR node's span, deduped against already-parsed table nodes. Adds multilang tests covering the new SQL recovery/bracket behavior alongside Go, Rust, and TS extraction cases.
Worth a look
- _debracket_tsql return type changed from bytes to tuple —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- src_text decoded with surrogateescape then re.finditer over CREATE TABLE uses group(1) directly without sanitizing surrogates —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- CREATE OR ALTER VIEW is not recovered —
graphify/extractors/sql.py:59· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- ARRAY constructor after newline is rewritten as a bracketed identifier —
graphify/extractors/sql.py:278· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 369 functions depend on the 189 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 33 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 369 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 201 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).
The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_resolve\_rescued\_specifier.
The verifier did not have enough to check \_resolve\_rescued\_specifier, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_dart.
The verifier did not have enough to check extract\_dart, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_resolve\_js\_module\_path.
The verifier did not have enough to check \_resolve\_js\_module\_path, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `start_dir` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_rust.
The verifier did not have enough to check extract\_rust, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_atomic\_replace.
The verifier did not have enough to check \_atomic\_replace, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `'str | Path'` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).
The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 33 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
The ARRAY keyword lookback only skipped spaces and tabs, so ARRAY followed by a newline then its bracket (SQL treats all whitespace between tokens the same way) still read as a bracket quoted identifier and got rewritten. Skip any whitespace kind, not just spaces and tabs.
_VIEW_TABLE_RECOVERY_RX lacked the IF NOT EXISTS gap _ROUTINE_RECOVERY_RX already carries. CREATE TABLE/VIEW IF NOT EXISTS in an error bearing file matched, but the capture group greedily grabbed the next bare word after CREATE TABLE/VIEW, which was IF, not the real name one word later, fabricating a phantom node literally named IF. Fixes Graphify-Labs#2724.
c18bf47 to
f722b21
Compare
|
Investigated this round's bot findings on this PR. Confirmed real, fixed: "CREATE OR ALTER VIEW is not recovered". _VIEW_TABLE_RECOVERY_RX only accepted OR REPLACE, matching Postgres, but not OR ALTER -- T-SQL's own re-creation syntax, since T-SQL has no OR REPLACE at all (_ROUTINE_RECOVERY_RX already accepts both, for the same reason, and its own comment documents why). Confirmed directly against the regex: CREATE OR ALTER VIEW ... never matched before this fix. Now accepts both forms, additive only (OR REPLACE still matches). Regression test added at f722b21. Also fixed here (propagated from the base branch, now at f722b21): the ARRAY-with-newline gap described on #2723. Investigated and found to be non-issues, no code change made:
|
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Rewrites T-SQL bracket-quoted identifiers to backtick-quoted ones before parsing via _debracket_tsql, so [dbo].[Orders]-style names take the normal AST path instead of landing in ERROR nodes and mangling labels or dropping tables behind broken foreign keys; the rewrite is skipped inside string, dollar-quoted, comment, and existing backtick spans, and bails to a plain bracket for spans that can't round-trip (dotted names, embedded backticks or comment openers, empty/numeric content). Extends the error-recovery scans to accept backtick names and adds _VIEW_TABLE_RECOVERY_RX to recover a CREATE TABLE/VIEW (including OR ALTER) swallowed into an unrelated ERROR span, deduped against already-registered tables. Returns the inserted backtick spans so a downstream reader un-rewrites only the bracket-derived identifiers and leaves genuine MySQL names alone.
No blocking issues surfaced. 12 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 373 functions depend on the 193 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 33 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 373 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 205 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).
The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_resolve\_rescued\_specifier.
The verifier did not have enough to check \_resolve\_rescued\_specifier, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_dart.
The verifier did not have enough to check extract\_dart, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_resolve\_js\_module\_path.
The verifier did not have enough to check \_resolve\_js\_module\_path, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `start_dir` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_rust.
The verifier did not have enough to check extract\_rust, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_atomic\_replace.
The verifier did not have enough to check \_atomic\_replace, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `'str | Path'` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).
The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 33 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Unlike a string or dollar quoted span, a backtick span must not cross a newline: a name does not span lines in practice, so an unclosed backtick is far more likely a stray character than a genuine multi line identifier. Scanning past a newline for one let one accidental, unmatched backtick anywhere in the file silently swallow everything after it, disabling debracketing for the rest of the file. A backtick that does not close on its own line is now left as an ordinary character, matching how _scan_sql treats an unterminated delimited identifier.
A regex recovered name whose own content carried one of src_text's surrogate escaped invalid bytes was sanitized via encode(errors=replace).decode(), but that handler turns a lone surrogate into a bare question mark, not the U+FFFD every other invalid byte path in this module produces. Replace it explicitly instead so the placeholder character stays consistent.
A bracket quoted FOREIGN KEY ... REFERENCES clause could confuse the parser badly enough that the whole child table (with its FK constraint) landed as bogus nested content inside the parent table's own subtree, dropping the child table from the graph and fabricating a self referencing EXTRACTED edge on the parent. Already fixed as a side effect of the Graphify-Labs#2712 debracketing change; this adds coverage for the specific Graphify-Labs#2713 repro. Fixes Graphify-Labs#2713.
This grammar has no rule at all for TSQL's AS BEGIN...END routine body, bracketed name or not, so every CREATE PROCEDURE/FUNCTION only ever gets extracted through the ERROR node regex fallback. That fallback matched a bare or double quoted name but not a bracketed one, so a bracketed routine name produced no node at all rather than a mangled one (0 of 845 procedures on a real SSMS scripted dump). Already fixed as a required part of Graphify-Labs#2712's rewrite: rewriting a bracketed name to a backtick quoted one before parsing means the fallback has to recognize that form too, or it would regress a bracketed routine from a recovered (if ugly) node to no node at all. This adds dedicated coverage for the Graphify-Labs#2718 repro, schema qualified and bare. Fixes Graphify-Labs#2718.
This grammar has no rule for TSQL's AS BEGIN...END routine body, so a CREATE FUNCTION/PROCEDURE with such a body lands in an ERROR node, and that ERROR span can absorb the NEXT statement too (e.g. a CREATE VIEW right after it), with no recovery path for anything but a routine. Adds a second recovery regex sharing the same delimited name alternatives and the same whole file masked scan already used for routines, gated the same way and deduped against table_nids so a statement that DID parse cleanly is never registered twice. Reproduces with bare identifiers, independent of the Graphify-Labs#2712/Graphify-Labs#2713/ Graphify-Labs#2718 bracket defects. Fixes Graphify-Labs#2719.
_VIEW_TABLE_RECOVERY_RX lacked the IF NOT EXISTS gap _ROUTINE_RECOVERY_RX already carries. CREATE TABLE/VIEW IF NOT EXISTS in an error bearing file matched, but the capture group greedily grabbed the next bare word after CREATE TABLE/VIEW, which was IF, not the real name one word later, fabricating a phantom node literally named IF. Fixes Graphify-Labs#2724.
_VIEW_TABLE_RECOVERY_RX only accepted OR REPLACE, matching Postgres, but not OR ALTER, TSQL's own re creation syntax since it has no OR REPLACE at all. _ROUTINE_RECOVERY_RX already accepts both for the same reason. A swallowed CREATE OR ALTER VIEW or TABLE could never be recovered by this regex before.
f722b21 to
d51c559
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Graphify review — findings
Rewrites T-SQL bracket-quoted identifiers ([dbo].[Orders]) to backtick-quoted form before parsing via _debracket_tsql, so statements the SQL grammar previously dropped into ERROR nodes now take the normal AST path; array markers (int[], ARRAY[…]), comment-bearing or dotted content, and spans inside strings, dollar-quotes, or genuine backtick identifiers are left untouched, and the inserted spans are tracked so only bracket-derived names get un-rewritten downstream. Adds _VIEW_TABLE_RECOVERY_RX to recover a CREATE TABLE/VIEW swallowed into an unrelated ERROR node's span, deduped against already-parsed tables, and accepts OR ALTER alongside OR REPLACE. Extends _scan_sql and _ROUTINE_RECOVERY_RX to treat backtick-delimited identifiers as recoverable names and normalizes lone surrogates to U+FFFD for placeholder consistency.
Worth a look
- _debracket_tsql return type changed from bytes to tuple —
graphify/extractors/sql.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- ARRAY constructor after a SQL comment is debracketed as an identifier —
graphify/extractors/sql.py:535· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _clean_regex_name reconstructs byte offsets from src_text but src_text decode error mode change may break span mapping —
graphify/extractors/sql.py:947· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Table/view ERROR recovery scans string literals as DDL —
graphify/extractors/sql.py:1045· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 377 functions depend on the 197 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_sql()— 34 callers, 11 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
walk()— 1 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 377 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 209 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_scan\_sql changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_scan\_sql behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"text":"''"\}, the old code produced \('', \[\]\) but the new code produces \(' ', \[\]\). Paste that input straight into a regression test.
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).
The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_resolve\_rescued\_specifier.
The verifier did not have enough to check \_resolve\_rescued\_specifier, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_dart.
The verifier did not have enough to check extract\_dart, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_resolve\_js\_module\_path.
The verifier did not have enough to check \_resolve\_js\_module\_path, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `start_dir` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_rust.
The verifier did not have enough to check extract\_rust, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_sql.
The verifier did not have enough to check extract\_sql, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_atomic\_replace.
The verifier did not have enough to check \_atomic\_replace, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `'str | Path'` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_query\_graph\_text (not a proof).
The verifier ran both versions of \_query\_graph\_text on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 3 more finding(s) on lines outside this diff (see the check run).
| return ".".join(parts) | ||
|
|
||
|
|
||
| def extract_sql(path: Path, content: str | bytes | None = None) -> dict: |
There was a problem hiding this comment.
extract_sql()
fans out to 11 callees (efferent coupling); 34 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
This grammar has no rule for T-SQL's AS BEGIN...END routine body, so a
CREATE FUNCTION/PROCEDURE with such a body lands in an ERROR node —
and that ERROR span can absorb the NEXT statement too (e.g. a CREATE
VIEW right after it), with no recovery path for anything but a
routine. Extend the ERROR-node regex recovery to also catch a
swallowed CREATE VIEW/TABLE name, registering it in table_nids so a
later reference in the same file resolves onto it. Reproduces with
bare identifiers, independent of the T-SQL bracket-quoting defects.
Fixes #2719.