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
15 changes: 12 additions & 3 deletions backend/app/agent/sql_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,17 @@ async def _log_query_history(
Persist query to audit log with full result cache.
result_json stores rows + columns so history replays are free (no Claude call).
"""
# asyncpg has no built-in dict→jsonb encoder. Serialise to JSON text and
# cast in SQL (`$4::jsonb`); otherwise the INSERT raises silently and
# history rows are lost (the rate-limit counter still increments).
try:
result_json = {"rows": rows, "columns": columns}
result_json = json.dumps({"rows": rows, "columns": columns}, default=str)
await database_pool.execute(
"""
INSERT INTO query_history
(question, generated_sql, explanation, result_json,
row_count, execution_ms, retries, user_id)
VALUES ($1,$2,$3,$4,$5,$6,$7,$8)
VALUES ($1, $2, $3, $4::jsonb, $5, $6, $7, $8)
""",
question,
sql,
Expand All @@ -243,4 +246,10 @@ async def _log_query_history(
user_id,
)
except Exception:
logger.exception("Failed to write query_history — non-fatal")
# Best-effort: the user already saw their result; do not 500 a successful
# query because the audit-log write failed. Log loudly so we notice if
# the silent-loss regression returns.
logger.exception(
"Failed to write query_history",
extra={"user_id": user_id, "row_count": row_count},
)
2 changes: 2 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ def _load_dotenv_repo_root() -> None:
os.environ.setdefault(
"DATABASE_URL", "postgresql://test:test@127.0.0.1:5432/insightiq_test"
)
os.environ.setdefault("JWT_SECRET_KEY", "test-jwt-secret-not-used-in-unit-tests")
os.environ.setdefault("GOOGLE_CLIENT_ID", "test-google-client-id.apps.googleusercontent.com")
69 changes: 69 additions & 0 deletions tests/test_sql_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
from datetime import date, datetime
from pydantic import ValidationError

from unittest.mock import AsyncMock, patch

from app.agent.sql_agent import (
_assert_safe_sql,
_log_query_history,
_serialise_row,
)
from app.api.schemas import QueryRequest
Expand Down Expand Up @@ -194,3 +197,69 @@ def test_too_long_raises(self):
"""Questions over 500 chars must fail validation."""
with pytest.raises(ValidationError):
QueryRequest(question="x" * 501)


# ── _log_query_history (regression: silent JSONB encode failure) ──────────


class TestLogQueryHistoryJsonbEncoding:
"""
Regression tests for the bug where `query_history` rows were never
persisted because asyncpg cannot encode a Python dict to JSONB.

Symptom in production: the per-user rate-limit counter incremented
(1 of 2 remaining) but History panel stayed empty.
"""

@pytest.mark.asyncio
async def test_passes_json_string_and_jsonb_cast(self):
"""result_json must be sent as a JSON string with explicit ::jsonb cast."""
with patch("app.agent.sql_agent.database_pool.execute", new=AsyncMock()) as mock_exec:
await _log_query_history(
question="What is MRR?",
sql="SELECT 1",
explanation="demo",
rows=[{"mrr": 1.0}],
columns=["mrr"],
row_count=1,
execution_ms=12,
retries=0,
user_id=42,
)

assert mock_exec.await_count == 1
sql_arg, *params = mock_exec.await_args.args

assert "$4::jsonb" in sql_arg, "SQL must cast result_json parameter to jsonb"

# result_json is the 4th positional bind value
result_json_param = params[3]
assert isinstance(result_json_param, str), (
"result_json must be a JSON string — asyncpg has no dict→jsonb encoder"
)
import json as _json

decoded = _json.loads(result_json_param)
assert decoded == {"rows": [{"mrr": 1.0}], "columns": ["mrr"]}

@pytest.mark.asyncio
async def test_db_failure_is_swallowed_not_propagated(self):
"""
Audit-log write is best-effort: a DB failure must not 500 a query
the user already received a successful response for.
"""
with patch(
"app.agent.sql_agent.database_pool.execute",
new=AsyncMock(side_effect=RuntimeError("transient")),
):
await _log_query_history(
question="q",
sql="SELECT 1",
explanation="",
rows=[],
columns=[],
row_count=0,
execution_ms=0,
retries=0,
user_id=1,
)
Loading