Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ question ─▶ schema retrieval ─▶ LLM: plan + SQL ─▶ AST validation
| Validation | deterministic | `sqlglot` AST checks, row limit enforced (see below). The SQL that runs is regenerated from the validated tree. |
| Execution | deterministic | Read-only connection, statement timeout, client-side row cap. |
| Repair | LLM, bounded | A repairable failure (validator rejection, SQL error, timeout, bad result, malformed model reply) is sent back to the model at most `MAX_RETRIES` times; every repair is validated again. Unsafe intent is never retried. |
| Result checks | deterministic | Empty result, aggregate over nothing, ranking led by NULL, NULL-only column, row limit reached, duplicates. Missing-value probes catch filter values and `LIKE` patterns that match nothing (e.g. `'Ivory Coast'`, a time written `'0:01:54%'` where `'1:54.123'` is stored) and show how stored values look. |
| Result checks | deterministic | Empty result, aggregate over nothing, ranking led by NULL, columns concatenated into one, NULL-only column, row limit reached, duplicates. Missing-value probes catch filter values and `LIKE` patterns that match nothing (e.g. `'Ivory Coast'`, a time written `'0:01:54%'` where `'1:54.123'` is stored) and show how stored values look. |
| Chart | deterministic | Chosen from column kinds and values: stat tile, line, bar, scatter or none. The model's suggestion is only a tie-breaker. |
| Answer | LLM + check | 1–3 sentences. Every number must come from the rows; otherwise a template answer built from the rows is used. |

Expand Down
1 change: 1 addition & 0 deletions backend/app/agent/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ def done(outcome: AgentResult, **detail: Any) -> _Solved:
tables=validated.tables,
limit=validated.limit,
limit_action=validated.limit_action,
float_casts=validated.float_casts,
plan_warnings=(
check_plan(plan, validated.tables, validated.limit) if plan else ["no plan returned"]
),
Expand Down
31 changes: 31 additions & 0 deletions backend/app/agent/result_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,20 @@ def check_result(
)
)

if isinstance(tree, exp.Select) and len(tree.expressions) == len(result.columns):
for column, projection in zip(result.columns, tree.expressions, strict=True):
joined = _joined_columns(projection)
if joined:
checks.append(
ResultCheck(
code="joined_columns",
severity="warning",
message=f"{column} joins {_listing(joined)} into one text value.",
column=column,
repairable=True,
)
)

if result.row_count > 1:
for index, column in enumerate(result.columns):
if all(row[index] is None for row in result.rows):
Expand Down Expand Up @@ -139,6 +153,23 @@ def _hashable(value: Any) -> Any:
return repr(value) if isinstance(value, (list, dict)) else value


def _joined_columns(projection: exp.Expr) -> list[str]:
"""Names of the stored columns an output column concatenates (`a || ' ' || b`, CONCAT,
CONCAT_WS), if it joins two or more; otherwise empty. A single column with a label
('$' || price) is a format, and a concatenation inside an aggregate (COUNT(DISTINCT a || b))
or a subquery is a computation, not an output: neither is reported."""
for node in projection.walk(prune=lambda n: isinstance(n, (exp.AggFunc, exp.Subquery))):
if isinstance(node, (exp.DPipe, exp.Concat, exp.ConcatWs)):
names = dict.fromkeys((c.table, c.name) for c in node.find_all(exp.Column, bfs=False))
if len(names) > 1:
return [name for _, name in names]
return []


def _listing(names: list[str]) -> str:
return ", ".join(names[:-1]) + f" and {names[-1]}"


def _string_filters(tree: exp.Expr) -> Iterator[tuple[exp.Table, str, str, exp.Literal]]:
"""(table, column, operator, literal) for each `column = 'text'`, `column IN ('text', ...)`
and `column [I]LIKE 'pattern'` filter on a real table column, in WHERE and JOIN conditions of
Expand Down
5 changes: 3 additions & 2 deletions backend/app/agent/sql_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from app.agent.schema_retriever import SchemaContext
from app.llm.client import LLMCall, LLMClient

PROMPT_VERSION = "sql-generator/7"
PROMPT_VERSION = "sql-generator/9"
MAX_HISTORY_TURNS = 3

SYSTEM_PROMPT = """\
Expand Down Expand Up @@ -168,7 +168,7 @@ def generate_sql(
return llm.complete_json(system, prompt, GeneratedSQL, temperature)


REPAIR_PROMPT_VERSION = "sql-repair/3"
REPAIR_PROMPT_VERSION = "sql-repair/4"

_REPAIR_HINTS = {
"timeout": (
Expand All @@ -186,6 +186,7 @@ def generate_sql(
"(see the listed values and column comments), or match it case-insensitively."
),
"null_first_in_ranking": "The ranking starts with NULL values: exclude NULLs or use NULLS LAST.",
"joined_columns": "Return each stored column as its own result column instead of concatenating them.",
}


Expand Down
31 changes: 30 additions & 1 deletion backend/app/agent/sql_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class ValidatedSQL:
tables: list[str] # qualified names of the database tables read
limit: int
limit_action: Literal["kept", "added", "clamped"]
float_casts: int = 0 # NUMERIC casts in a division computed as DOUBLE PRECISION instead


def _reject(code: RejectionCode, message: str) -> SQLRejectedError:
Expand Down Expand Up @@ -259,6 +260,31 @@ def _enforce_limit(tree: exp.Query, max_rows: int) -> tuple[int, Literal["kept",
return max_rows, "clamped"


# Arithmetic between a cast and the division it feeds: CAST(x AS NUMERIC) * 100 / total.
_ARITHMETIC = (exp.Paren, exp.Mul, exp.Add, exp.Sub, exp.Neg)


def _float_divisions(tree: exp.Expr) -> int:
"""Compute ratios in floating point: CAST(x AS NUMERIC) without precision, where it only serves
a division, becomes CAST(x AS DOUBLE PRECISION). Models cast to NUMERIC to avoid integer
division; the result then comes back as an arbitrary-precision decimal where a ratio or a
percentage is expected to be a float (BIRD's answers are, and an exact comparison of
0.0657... as decimal and as float fails). A cast with precision (NUMERIC(10, 2)) is a
deliberate choice and stays, and so does any cast that is not part of a division."""
rewritten = 0
for cast in list(tree.find_all(exp.Cast)):
to = cast.args.get("to")
if not isinstance(to, exp.DataType) or to.this != exp.DataType.Type.DECIMAL or to.expressions:
continue
node: exp.Expr = cast
while isinstance(node.parent, _ARITHMETIC):
node = node.parent
if isinstance(node.parent, exp.Div):
cast.set("to", exp.DataType.build("DOUBLE PRECISION"))
rewritten += 1
return rewritten


def validate_sql(sql: str, profile: DatabaseProfile, dialect: str, max_rows: int) -> ValidatedSQL:
"""Accept or reject generated SQL. Raises SQLRejectedError with a machine-readable code."""
tree = _parse(sql, dialect)
Expand All @@ -267,4 +293,7 @@ def validate_sql(sql: str, profile: DatabaseProfile, dialect: str, max_rows: int
tables = _check_tables(tree, profile)
_check_columns(tree, profile, dialect)
limit, action = _enforce_limit(tree, max_rows)
return ValidatedSQL(sql=tree.sql(dialect=dialect), tables=tables, limit=limit, limit_action=action)
casts = _float_divisions(tree)
return ValidatedSQL(
sql=tree.sql(dialect=dialect), tables=tables, limit=limit, limit_action=action, float_casts=casts
)
11 changes: 11 additions & 0 deletions backend/tests/test_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,17 @@ def test_ranking_led_by_nulls_is_repaired(shop) -> None:
assert "null_first_in_ranking" in llm.calls[1][1] and "NULLS LAST" in llm.calls[1][1]


def test_concatenated_columns_are_repaired_into_separate_columns(shop) -> None:
llm = sequence(
reply("SELECT id, status || ' / ' || ordered_at AS summary FROM shop.orders WHERE id = 1"),
reply("SELECT id, status, ordered_at FROM shop.orders WHERE id = 1"),
)
result = AgentController(llm, max_rows=100).run("Status and date of order 1?", shop)
assert result.status == "success" and result.columns == ["id", "status", "ordered_at"]
assert "summary joins status and ordered_at into one text value" in llm.calls[1][1]
assert result.checks == [] and result.metadata.retry_count == 1


def test_earlier_result_is_kept_when_a_repair_ends_worse(shop) -> None:
llm = sequence(
reply("SELECT count(*) AS n FROM shop.orders WHERE status = 'Shipped'"),
Expand Down
36 changes: 36 additions & 0 deletions backend/tests/test_result_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,42 @@ def test_duplicate_rows() -> None:
assert codes(result(["a"], [[1], [1], [2]])) == ["duplicate_rows"]


@pytest.mark.parametrize(
"projection",
[
"m.first_name || ' ' || m.last_name AS full_name",
"CONCAT(first_name, ' ', last_name) AS full_name",
"CONCAT_WS(' ', first_name, last_name) AS full_name",
"TRIM(first_name || ' ' || COALESCE(last_name, '')) AS full_name",
],
)
def test_concatenated_columns_are_repairable(projection: str) -> None:
res = result(["id", "full_name"], [[1, "Ada Lovelace"]])
checks = check_result(res, f"SELECT id, {projection} FROM member AS m", "postgres", 100, "kept")
assert [(c.code, c.column, c.repairable) for c in checks] == [("joined_columns", "full_name", True)]
assert checks[0].message == "full_name joins first_name and last_name into one text value."


def test_concatenated_columns_in_the_final_select_of_a_cte_query() -> None:
sql = "WITH m AS (SELECT * FROM member) SELECT city || ', ' || country AS place FROM m"
checks = check_result(result(["place"], [["Oslo, Norway"]]), sql, "postgres", 100, "kept")
assert [c.message for c in checks] == ["place joins city and country into one text value."]


@pytest.mark.parametrize(
"sql",
[
"SELECT '$' || price AS price FROM t", # one column with a label: a format
"SELECT COUNT(DISTINCT a || b) AS pairs FROM t", # computed over, not returned
"SELECT STRING_AGG(first_name || ' ' || last_name, ', ') AS names FROM t",
"SELECT (SELECT a || b FROM u LIMIT 1) AS x FROM t", # inside a subquery
"WITH k AS (SELECT a || '-' || b AS key FROM t) SELECT key FROM k", # a key built in a CTE
],
)
def test_other_concatenation_is_not_reported(sql: str) -> None:
assert codes(result(["x"], [["v"]]), sql) == []


# --- missing-value probes (integration) --------------------------------------------------


Expand Down
11 changes: 11 additions & 0 deletions backend/tests/test_sql_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ def test_repair_prompt_contains_the_failure_and_a_hint() -> None:
assert "Do not repeat the same SQL" in prompt


def test_joined_columns_repair_asks_for_separate_columns() -> None:
from app.agent.schema_retriever import SchemaContext
from app.agent.sql_generator import FailedAttempt, build_repair_prompt

message = "full_name joins first_name and last_name into one text value."
failed = FailedAttempt("SELECT a || b FROM t", "result", "joined_columns", message)
prompt = build_repair_prompt("q", SchemaContext(tables=[], text=""), failed)
assert f"result failed a check (joined_columns): {message}" in prompt
assert "its own result column" in prompt


def test_validation_failure_is_described_as_a_rejection() -> None:
from app.agent.schema_retriever import SchemaContext
from app.agent.sql_generator import FailedAttempt, build_repair_prompt
Expand Down
32 changes: 32 additions & 0 deletions backend/tests/test_sql_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,38 @@ def test_limit_applies_to_the_whole_union() -> None:
assert result.limit_action == "added"


# --- ratios in floating point -----------------------------------------------------------


@pytest.mark.parametrize(
"expression",
[
"CAST(COUNT(*) AS NUMERIC) / COUNT(*)",
"CAST(COUNT(*) AS DECIMAL) * 100 / NULLIF(COUNT(*), 0)",
"(CAST(COUNT(*) AS DECIMAL)) / COUNT(*)",
"COUNT(*) / COUNT(*)::numeric",
],
)
def test_numeric_casts_in_a_division_become_double_precision(expression: str) -> None:
result = validate(f"SELECT {expression} AS share FROM countries")
assert result.float_casts == 1
assert "DOUBLE PRECISION" in result.sql and "DECIMAL" not in result.sql
validate(result.sql) # what runs is still a valid query


@pytest.mark.parametrize(
"expression",
[
"CAST(COUNT(*) AS NUMERIC(10, 2)) / COUNT(*)", # a chosen precision stays
"CAST(COUNT(*) AS NUMERIC)", # no division
"SUM(CAST(LENGTH(name) AS NUMERIC)) / COUNT(*)", # inside an aggregate: not the ratio itself
],
)
def test_other_numeric_casts_are_left_alone(expression: str) -> None:
result = validate(f"SELECT {expression} AS value FROM countries")
assert result.float_casts == 0 and "DOUBLE PRECISION" not in result.sql


def test_inner_limits_do_not_count_as_the_outer_limit() -> None:
result = validate("SELECT name FROM countries WHERE id IN (SELECT country_id FROM co2_emissions LIMIT 5)")
assert result.limit_action == "added"
Expand Down
Loading