diff --git a/tests/test_storage_sqlite.py b/tests/test_storage_sqlite.py index cb80c616..b19c10e3 100644 --- a/tests/test_storage_sqlite.py +++ b/tests/test_storage_sqlite.py @@ -4,6 +4,7 @@ from __future__ import annotations from typing import Any +import pytest import sqlalchemy as sa from turnstone.core.storage._schema import workstreams @@ -576,6 +577,48 @@ class TestSearch: results = backend.search_history_recent(limit=1) assert len(results) == 1 + def test_search_history_survives_oversized_row(self, backend): + # A multi-MB row of mostly-unique words: on PostgreSQL its full + # tsvector exceeds the 1MB hard limit, which used to abort every + # search_history scan ("string is too long for tsvector") — one + # giant tool dump silently killed history recall entirely. + backend.register_workstream("s1") + giant = "gargantuan beacon " + " ".join(f"w{i}" for i in range(300_000)) + assert len(giant) > 2_000_000 + backend.save_message("s1", "tool", giant) + backend.save_message("s1", "user", "hello world") + + results = backend.search_history("hello") + assert any("hello" in str(r[3]) for r in results) + + # The oversized row itself stays findable by its head. + results = backend.search_history("gargantuan beacon") + assert any("gargantuan" in str(r[3]) for r in results) + + def test_search_history_fts_error_falls_back_to_ilike(self, request, backend, monkeypatch): + # PostgreSQL only: a failed FTS statement aborts the connection's + # autobegun transaction, and the ILIKE fallback runs on that same + # connection — without a rollback first it dies with + # InFailedSqlTransaction instead of returning results. + if request.config.getoption("--storage-backend") != "postgresql": + pytest.skip("exercises PostgreSQL aborted-transaction fallback") + + backend.register_workstream("s1") + backend.save_message("s1", "user", "hello fallback world") + + real_execute = sa.engine.Connection.execute + + def failing_fts_execute(self, statement, *args, **kwargs): + if "to_tsvector" in str(statement): + # A genuine server-side error, so the transaction is aborted + # exactly as when to_tsvector rejects a row. + return real_execute(self, sa.text("SELECT 1/0")) + return real_execute(self, statement, *args, **kwargs) + + monkeypatch.setattr(sa.engine.Connection, "execute", failing_fts_execute) + results = backend.search_history("fallback") + assert any("fallback" in str(r[3]) for r in results) + # -- Workstream operations ----------------------------------------------------- diff --git a/turnstone/core/storage/_postgresql.py b/turnstone/core/storage/_postgresql.py index fef88c7b..92231d6e 100644 --- a/turnstone/core/storage/_postgresql.py +++ b/turnstone/core/storage/_postgresql.py @@ -178,6 +178,13 @@ from turnstone.core.workstream import BULK_CLOSE_STATE_VALUES, WorkstreamKind log = get_logger(__name__) +# PostgreSQL rejects any tsvector larger than 1MB ("string is too long for +# tsvector"), and search_history computes tsvectors inline per row — so one +# oversized row would abort the whole scan and every search with it. Worst +# case a tsvector runs ~4x its input (unique short lexemes + position data), +# so 250K chars keeps even pathological rows safely under the limit. +_FTS_INPUT_CAP_CHARS = 250_000 + def _resolve_pg_listen_url(override: str, sqlalchemy_url: str) -> str: """Resolve the URL used by the dedicated LISTEN connection. @@ -1289,26 +1296,31 @@ class PostgreSQLBackend: scope_params["excl_ws"] = exclude_ws_id scope_params["excl_after"] = -1 if exclude_after is None else exclude_after with self._conn() as conn: - # Use PostgreSQL full-text search if search_vector column exists + # Full-text search over an inline tsvector (there is no indexed + # search_vector column). The input is capped — see + # _FTS_INPUT_CAP_CHARS — so a single giant row (multi-MB tool + # dumps exist) cannot trip PostgreSQL's 1MB tsvector limit and + # abort every search; oversized rows stay findable by their head. try: return list( conn.execute( sa.text( "SELECT c.timestamp, c.ws_id, c.role, c.content, c.tool_name " "FROM conversations c " - "WHERE to_tsvector('english', COALESCE(c.content, '')) " + "WHERE to_tsvector('english', left(COALESCE(c.content, ''), :fts_cap)) " " @@ plainto_tsquery('english', :query) " # Exclude compaction-checkpoint markers (resume-only # summary artifacts); IS DISTINCT FROM is NULL-safe so # normal rows (_source NULL) are not dropped. "AND c._source IS DISTINCT FROM :compaction_source " + scope_sql - + "ORDER BY ts_rank(to_tsvector('english', COALESCE(c.content, '')), " + + "ORDER BY ts_rank(to_tsvector('english', left(COALESCE(c.content, ''), :fts_cap)), " " plainto_tsquery('english', :query)) DESC " "LIMIT :limit OFFSET :offset" ), { "query": query, + "fts_cap": _FTS_INPUT_CAP_CHARS, "compaction_source": _COMPACTION_SOURCE, "limit": capped, "offset": capped_offset, @@ -1317,6 +1329,11 @@ class PostgreSQLBackend: ).fetchall() ) except Exception: + # The failed statement aborted the connection's autobegun + # transaction; PostgreSQL then refuses every command until a + # rollback, so without this the fallback can never run + # (InFailedSqlTransaction). + conn.rollback() # Fallback to ILIKE return list( conn.execute(