Fix LLMSQLQueryOperator not stripping single-line markdown code fences - #70137
Merged
Conversation
Lee-W
approved these changes
Jul 20, 2026
_strip_llm_output only removed ```sql fences when the LLM's response spanned multiple lines. LLMs that wrap short queries in a fence on a single line (e.g. "```SELECT 1```") passed the fenced text straight into SQL validation/execution with the backticks still attached. Add a fallback branch for the single-line case, alongside the existing multi-line handling.
ColtenOuO
force-pushed
the
fix-llm-sql-single-line-fence
branch
from
July 20, 2026 15:50
d7f205b to
e6a9e6c
Compare
kaxil
reviewed
Jul 20, 2026
Address review feedback: a single-line fence like "```sql SELECT 1```" left "sql " stuck to the front of the query, since the earlier fix only stripped the outer backticks. Drop the leading word too, but only when it's "sql" or the resolved dialect, so a real leading SQL keyword is never mistaken for a tag.
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
3 participants
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
LLMSQLQueryOperator._strip_llm_outputstrips markdown code fences from LLM output before the generated SQL is validated/executed. The existing logic only handles fences where the LLM's response spans multiple lines(e.g.
```sql\nSELECT 1\n```) — it splits on"\n"and requires at least 2 lines before attempting to drop the opening/closing fence.When the whole fenced response is on a single line (e.g.
```SELECT 1```, no internal newline),text.split("\n")yields asingle element, the
len(lines) >= 2guard is never satisfied, and the fence-stripping is skipped entirely — the backticks stay attached and the value is passed straight into_validate_sql(and potentially execution) as-is.The operator's system prompt already asks the LLM not to use markdown at all (
"Return ONLY the SQL query, no explanation or markdown."), but the existing multi-line fence-handling logic (and its test coverage) already assumes LLMs commonly ignore that instruction and wrap output in a fence anyway.This change extends the same defensive handling to the single-line variant of that same behavior, which the multi-line path doesn't cover.
Changes
_strip_llm_outputgains anelifbranch for the case where the fencedtext has no internal newline: strip the leading/trailing
```directlyinstead of relying on line-splitting. The existing multi-line branch is
untouched, so behavior for every previously-tested case is unchanged.
Test plan
TestStripLLMOutput: a single-line fencewith no language tag, and one wrapping a longer query.
test_llm_sql.pysuite passes (44 passed), including allpre-existing multi-line fence cases unchanged.
ruff format/ruff check,prek --stage pre-commit, andbreeze run mypyall clean on the changed file.