Conversation
…sonb
asyncpg cannot encode a Python dict into a JSONB column, so the
INSERT into query_history raised DataError on every successful query.
The blanket "non-fatal" except block hid the failure, but the
rate-limit counter on users.total_queries had already committed —
producing the user-facing bug where the budget badge decremented
("1 of 2 remaining") yet the History panel stayed empty.
Serialise the result cache with json.dumps(..., default=str) and add
an explicit ::jsonb cast on the bind parameter. Keep the audit-log
write best-effort (a successful query should not 500 because the
audit log failed) but log structured context so a regression is
visible in Azure logs.
Adds two regression tests:
- asserts result_json is bound as a string and SQL contains $4::jsonb
- asserts a transient DB failure does not propagate to the user
tests/conftest.py: setdefault JWT_SECRET_KEY and GOOGLE_CLIENT_ID so
unit tests load without a fully populated .env.
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
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
Fixes a silent data-loss bug where every query's row in
query_historywas dropped, while the per-user rate-limit counter still incremented. Users saw their budget badge decrement to1 of 2 remainingbut the History panel stayed empty and cached replays were impossible.Root cause
_log_query_historyinbackend/app/agent/sql_agent.pypassed a Pythondictas the bind value for theresult_json JSONBcolumn. asyncpg has no built-indict → jsonbencoder, so the INSERT raisedDataError. The error was caught by a blanketexcept Exception: logger.exception("... non-fatal")block and never surfaced, so the regression was invisible in logs that weren't tailed carefully.The earlier
UPDATE users SET total_queries = ...(no JSON) committed normally — hence the asymmetry between budget and history.Changes
backend/app/agent/sql_agent.pyjson.dumps({"rows": ..., "columns": ...}, default=str).$4::jsonbcast in the INSERT.user_id,row_count) so future regressions are visible in Azure Container Apps logs.tests/test_sql_agent.py— two new regression tests underTestLogQueryHistoryJsonbEncoding:test_passes_json_string_and_jsonb_cast— asserts the bind value is astrand the SQL contains$4::jsonb.test_db_failure_is_swallowed_not_propagated— verifies that a transient DB error during the audit-log write does not propagate to the user.tests/conftest.py—setdefaultJWT_SECRET_KEYandGOOGLE_CLIENT_IDso unit tests load without a fully populated.env.Test plan
pytest tests/test_sql_agent.py -v— 27 passed (25 existing + 2 new)query_historyand the History panel shows itGET /api/history/:idwithout a Claude call (from_cache: true)users.total_queriesmatchesCOUNT(*) FROM query_history WHERE user_id = meafter the fixRisk
Low — change is scoped to one function and adds an explicit cast that PostgreSQL accepts unconditionally. Existing history rows are unaffected; new rows will be inserted correctly.
Rollback
Revert this commit. The previous behaviour was data-loss but not a crash, so rollback is safe at any time.
Closes #45