From 19042ea535af519d4d2add7cb794c12e6c1b1a9c Mon Sep 17 00:00:00 2001 From: crayfish-ai <270082434+crayfish-ai@users.noreply.github.com> Date: Thu, 30 Jul 2026 20:27:26 +0800 Subject: [PATCH 1/3] perf(session): route SQLite PRAGMAs through central apply_database_pragmas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review from @teknium1 on PR #71755: - Extended apply_database_pragmas() to handle cache_size, mmap_size, and temp_store from config.yaml (alongside existing wal_autocheckpoint and journal_size_limit). No hardcoded defaults — all values are opt-in via config.yaml, avoiding policy conflicts with other PRs. - Applied to ALL connection types: writer (_connect_and_init), read_only cross-profile attach, and WAL per-thread readers (_get_read_conn). Previously PRAGMAs only ran on the writer path. - Removed inline PRAGMAs from _connect_and_init — single source of truth in apply_database_pragmas(). - Documented config keys with examples in function docstring. --- hermes_state.py | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index 68c8ba36af0f4..ac59a6c5aaabf 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -855,16 +855,23 @@ def apply_database_pragmas( *, db_label: str = "state.db", ) -> None: - """Apply optional WAL-sizing PRAGMAs from ``config.yaml``. + """Apply optional performance and WAL-sizing PRAGMAs from ``config.yaml``. - Reads the ``database:`` section and applies ``wal_autocheckpoint`` - and ``journal_size_limit`` when set to integer values. The journal - mode itself is NOT handled here — ``database.journal_mode`` is owned - by :func:`resolve_journal_mode` inside :func:`apply_wal_with_fallback`, - which layers the operator setting under all the safety guards - (never live-downgrading an on-disk WAL DB, filesystem fallback, - WAL-reset-bug gating). Keeping a single owner prevents a second, - unguarded journal-mode switch path. + Reads the ``database:`` section and applies configurable PRAGMAs when set + to integer values. The journal mode itself is NOT handled here — + ``database.journal_mode`` is owned by :func:`resolve_journal_mode` inside + :func:`apply_wal_with_fallback`, which layers the operator setting under + all the safety guards (never live-downgrading an on-disk WAL DB, + filesystem fallback, WAL-reset-bug gating). + + Supported keys under ``database:`` in config.yaml: + + * ``cache_size`` — negative value = KiB, positive = pages + (e.g. ``-262144`` = 256 MB page cache) + * ``mmap_size`` — max bytes for memory-mapped I/O (0 = disabled) + * ``temp_store`` — 0=DEFAULT(file), 1=FILE, 2=MEMORY, 3=ALWAYS + * ``wal_autocheckpoint`` — WAL auto-checkpoint threshold in pages + * ``journal_size_limit`` — max journal/WAL size in bytes Best-effort: config load or pragma failures are ignored so DB init never breaks on a malformed ``database:`` section. @@ -877,7 +884,15 @@ def apply_database_pragmas( except Exception: return - for pragma_name in ("wal_autocheckpoint", "journal_size_limit"): + # Performance PRAGMAs (applied to ALL connection types: writer, read_only, + # and WAL per-thread readers). + for pragma_name in ( + "cache_size", + "mmap_size", + "temp_store", + "wal_autocheckpoint", + "journal_size_limit", + ): raw_value = cfg_get(cfg, "database", pragma_name, default=None) if raw_value is None: continue @@ -1919,6 +1934,7 @@ def __init__(self, db_path: Path = None, read_only: bool = False): isolation_level=None, ) self._conn.row_factory = sqlite3.Row + apply_database_pragmas(self._conn, db_label="state.db") # FTS capability flags normally come from writable schema # initialisation. Probe existing virtual tables with SELECTs # only so read-only search keeps its FTS and trigram paths. @@ -2006,6 +2022,9 @@ def _connect_and_init(): apply_wal_with_fallback(self._conn, db_label="state.db") == "wal" ) apply_database_pragmas(self._conn, db_label="state.db") + self._conn.execute("PRAGMA cache_size=-8000") + self._conn.execute("PRAGMA journal_mode=WAL") + self._conn.execute("PRAGMA synchronous=NORMAL") self._conn.execute("PRAGMA foreign_keys=ON") self._fts_cjk_loaded = load_fts5_cjk_extension(self._conn) self._init_schema() @@ -2128,6 +2147,7 @@ def _get_read_conn(self) -> Optional[sqlite3.Connection]: isolation_level=None, ) conn.row_factory = sqlite3.Row + apply_database_pragmas(conn, db_label="state.db") # Load the CJK tokenizer extension on this connection so # messages_fts_cjk queries work on the read path. The .so # registers the tokenizer in the connection's in-memory From 0e8e84cafc211ea49460bac554c4668e04fe9211 Mon Sep 17 00:00:00 2001 From: crayfish-ai <270082434+crayfish-ai@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:46:21 +0800 Subject: [PATCH 2/3] fix(pr): remove remnant local PRAGMAs from PR branch --- hermes_state.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index ac59a6c5aaabf..907ca56b3dce9 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1934,7 +1934,6 @@ def __init__(self, db_path: Path = None, read_only: bool = False): isolation_level=None, ) self._conn.row_factory = sqlite3.Row - apply_database_pragmas(self._conn, db_label="state.db") # FTS capability flags normally come from writable schema # initialisation. Probe existing virtual tables with SELECTs # only so read-only search keeps its FTS and trigram paths. @@ -1945,6 +1944,7 @@ def __init__(self, db_path: Path = None, read_only: bool = False): # raw-copy for the rest of the process — the writable heal # that follows would then repair WITHOUT its forensic backup. try: + apply_database_pragmas(self._conn, db_label="state.db") cursor = self._conn.cursor() self._fts_enabled = ( self._fts_table_probe(cursor, "messages_fts") is True @@ -2022,9 +2022,6 @@ def _connect_and_init(): apply_wal_with_fallback(self._conn, db_label="state.db") == "wal" ) apply_database_pragmas(self._conn, db_label="state.db") - self._conn.execute("PRAGMA cache_size=-8000") - self._conn.execute("PRAGMA journal_mode=WAL") - self._conn.execute("PRAGMA synchronous=NORMAL") self._conn.execute("PRAGMA foreign_keys=ON") self._fts_cjk_loaded = load_fts5_cjk_extension(self._conn) self._init_schema() From dff7eb0b6b6f77b49e24499a94653f6b8de9d785 Mon Sep 17 00:00:00 2001 From: kshitij Date: Mon, 3 Aug 2026 16:19:30 +0530 Subject: [PATCH 3/3] test(session): guard config-gated performance PRAGMAs across all connection types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E2E guard for the salvaged PR #71755: database.cache_size/mmap_size/ temp_store from config.yaml must reach the writer connection, the read-only cross-profile attach, and the WAL per-thread reader — and a default install (no database: keys) must keep byte-identical SQLite defaults on every connection type. Also covers integer-coercion rejection of garbage values for the three new keys. cache_size uses -16000 (not the doc example -2000) because -2000 is SQLite's compiled-in default and would not discriminate a regression. --- tests/test_hermes_state.py | 142 +++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index cf7c813338b82..74a028c754713 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -3835,6 +3835,38 @@ def test_ignores_non_integer_values(self, tmp_path, monkeypatch): assert conn.execute("PRAGMA wal_autocheckpoint").fetchone()[0] == before finally: conn.close() + + def test_ignores_non_integer_performance_values(self, tmp_path, monkeypatch): + """Garbage cache_size/mmap_size/temp_store values must be rejected.""" + import sqlite3 + from hermes_state import apply_database_pragmas + + conn = sqlite3.connect(str(tmp_path / "pragmas.db")) + try: + before = { + name: conn.execute(f"PRAGMA {name}").fetchone()[0] + for name in ("cache_size", "mmap_size", "temp_store") + } + self._patch_cfg( + monkeypatch, + { + "database": { + "cache_size": "big", + "mmap_size": [256], + "temp_store": "ram please", + } + }, + ) + apply_database_pragmas(conn, db_label="test.db") + after = { + name: conn.execute(f"PRAGMA {name}").fetchone()[0] + for name in ("cache_size", "mmap_size", "temp_store") + } + assert after == before + finally: + conn.close() + + class TestInsightsToolCallIndex: """The Insights assistant tool-call scan has a predicate-aligned index. @@ -4017,3 +4049,113 @@ def connect_without_trigram(*args, **kwargs): assert db.search_messages("zebra") finally: db.close() + + + +class TestPerformancePragmasEndToEnd: + """E2E guard for PR #71755: config-gated cache_size / mmap_size / + temp_store must reach EVERY connection type (writer, read-only + cross-profile attach, WAL per-thread reader) — and default installs + (no ``database:`` keys) must see byte-identical SQLite defaults. + + NOTE: SQLite's compiled-in default for ``cache_size`` is already + ``-2000``, so the configured value here is ``-16000`` — a value the + test can actually discriminate from the default (a reverted prod + change must FAIL this test, not accidentally pass it). + """ + + PRAGMAS = ("cache_size", "mmap_size", "temp_store") + CONFIGURED = {"cache_size": -16000, "mmap_size": 1048576, "temp_store": 2} + + @staticmethod + def _read(conn): + return { + name: conn.execute(f"PRAGMA {name}").fetchone()[0] + for name in ("cache_size", "mmap_size", "temp_store") + } + + @staticmethod + def _sqlite_defaults(tmp_path): + import sqlite3 + + conn = sqlite3.connect(str(tmp_path / "baseline.db")) + try: + return { + name: conn.execute(f"PRAGMA {name}").fetchone()[0] + for name in ("cache_size", "mmap_size", "temp_store") + } + finally: + conn.close() + + def _fresh_home(self, tmp_path, monkeypatch, config_text=None): + import hermes_state + + # Local venvs may bundle a WAL-reset-vulnerable SQLite (e.g. 3.46.0), + # which would silently disable WAL and skip the per-thread reader + # path. Force WAL eligibility so _get_read_conn is truly exercised + # (established pattern used by the WAL tests above). + monkeypatch.setattr( + hermes_state, + "is_sqlite_wal_reset_vulnerable", + lambda version_info=None: False, + ) + home = tmp_path / "hermes_home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + if config_text is not None: + (home / "config.yaml").write_text(config_text) + return home + + def test_configured_pragmas_reach_all_connection_types( + self, tmp_path, monkeypatch + ): + from hermes_state import SessionDB + + home = self._fresh_home( + tmp_path, + monkeypatch, + "database:\n" + " cache_size: -16000\n" + " temp_store: 2\n" + " mmap_size: 1048576\n", + ) + db_path = home / "state.db" + db = SessionDB(db_path=db_path) + try: + # Writer connection. + assert self._read(db._conn) == self.CONFIGURED + # WAL per-thread reader. + rconn = db._get_read_conn() + assert rconn is not None, "WAL reader expected on local filesystem" + assert self._read(rconn) == self.CONFIGURED + finally: + db.close() + + # Read-only cross-profile attach. + ro = SessionDB(db_path=db_path, read_only=True) + try: + assert self._read(ro._conn) == self.CONFIGURED + finally: + ro.close() + + def test_defaults_unchanged_without_config(self, tmp_path, monkeypatch): + """No database: keys in config.yaml → SQLite defaults untouched.""" + from hermes_state import SessionDB + + defaults = self._sqlite_defaults(tmp_path) + home = self._fresh_home(tmp_path, monkeypatch, config_text=None) + db_path = home / "state.db" + db = SessionDB(db_path=db_path) + try: + assert self._read(db._conn) == defaults + rconn = db._get_read_conn() + if rconn is not None: + assert self._read(rconn) == defaults + finally: + db.close() + + ro = SessionDB(db_path=db_path, read_only=True) + try: + assert self._read(ro._conn) == defaults + finally: + ro.close()