diff --git a/backend/app/agent/sql_agent.py b/backend/app/agent/sql_agent.py index b215dd1..223a402 100644 --- a/backend/app/agent/sql_agent.py +++ b/backend/app/agent/sql_agent.py @@ -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, @@ -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}, + ) diff --git a/tests/conftest.py b/tests/conftest.py index e64fbc1..15c5453 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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") diff --git a/tests/test_sql_agent.py b/tests/test_sql_agent.py index 73535a6..ec4f7b5 100644 --- a/tests/test_sql_agent.py +++ b/tests/test_sql_agent.py @@ -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 @@ -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, + )