From a0bbb9e17f9cef69c1ab280e5a9e0468248c15c0 Mon Sep 17 00:00:00 2001 From: cdeust Date: Wed, 29 Jul 2026 21:39:18 +0200 Subject: [PATCH 1/2] fix(sqlite): register explicit datetime adapter, close deprecation warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sqlite_compat.py relied on sqlite3's implicit default datetime adapter, deprecated as of Python 3.12. The one call site binding a raw `datetime.datetime` parameter is cascade.py::_update_stage_entered (confirmed the sole such site in this codebase via an instrumented full-suite run). Registers an explicit `sqlite3.register_adapter` callback that writes the same "T"-separated ISO-8601 spelling every other datetime write path in this codebase already uses; old rows (written in the deprecated adapter's space-separated spelling) keep reading correctly via `datetime.fromisoformat`, which parses both spellings identically, so no migration is needed. Boy-scout: sqlite_compat.py was already 335 lines, over the repo's 300-line cap, before this change touched it. Split the pure SQL-dialect translation logic (_translate_sql/_returning_was_stripped) into a new sqlite_sql_translate.py module, bringing sqlite_compat.py to 232 lines. Also added missing test coverage for _CompatCursor/PsycopgCompatConnection field wiring surfaced by a scoped mutation run against the touched file (0 unaccounted survivors; one documented equivalent mutant, see test_executemany_clears_had_returning_and_reports_real_rowcount). A pre-existing, unrelated mutation gap in _translate_sql itself (20 survivors, verbatim code predating this change) is filed as #265 per coding-standards.md §14.3 rather than folded in here. Closes #260 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u --- mcp_server/infrastructure/sqlite_compat.py | 211 +++++------------- .../infrastructure/sqlite_schema_wiki.py | 6 +- .../infrastructure/sqlite_sql_translate.py | 170 ++++++++++++++ pyproject.toml | 10 + .../handlers/test_wiki_pipeline_sqlite.py | 148 +++++++++++- .../test_sqlite_datetime_adapter_260.py | 172 ++++++++++++++ 6 files changed, 552 insertions(+), 165 deletions(-) create mode 100644 mcp_server/infrastructure/sqlite_sql_translate.py create mode 100644 tests_py/infrastructure/test_sqlite_datetime_adapter_260.py diff --git a/mcp_server/infrastructure/sqlite_compat.py b/mcp_server/infrastructure/sqlite_compat.py index 3392f45a..9c7fb935 100644 --- a/mcp_server/infrastructure/sqlite_compat.py +++ b/mcp_server/infrastructure/sqlite_compat.py @@ -1,171 +1,68 @@ """SQLite <-> psycopg compatibility wrapper. -Translates PostgreSQL SQL conventions to SQLite equivalents so that -handler code using store._conn.execute() with psycopg-style SQL works -unchanged on the SQLite fallback backend. - -Translations: - - %s -> ? (parameter placeholders) - - ::jsonb, ::TEXT, ::REAL, ::INT -> stripped (type casts) - - SERIAL PRIMARY KEY -> INTEGER PRIMARY KEY AUTOINCREMENT - - TIMESTAMPTZ -> TEXT - - DEFAULT NOW() -> DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) - - ON CONFLICT ... DO UPDATE SET -> preserved (SQLite 3.24+) - - RETURNING id -> stripped (use lastrowid instead) - - wiki. -> wiki_
(SQLite has no schema namespaces) - - array_length(col, 1) -> json_array_length(col) (arrays are JSON TEXT) +Wraps a sqlite3 connection/cursor so handler code written against psycopg's +API (``conn.execute("... %s ...", params)``, ``with conn.cursor() as cur``) +runs unmodified on the SQLite fallback backend. SQL-dialect rewriting +(``%s`` -> ``?``, ``TIMESTAMPTZ`` -> ``TEXT``, etc.) lives in +``sqlite_sql_translate.py`` (split out in issue #260 to keep this file +under the project's 300-line cap); this module owns parameter adaptation +and the cursor/connection shape. + +Datetime wire format (issue #260): a bound `datetime.datetime` parameter is +serialized via `_adapt_datetime_iso` below — plain `.isoformat()`, the same +"T"-separated spelling `sqlite_store._now_iso()` and every other datetime +write path in this codebase already produce. This replaces reliance on the +stdlib's *implicit* default adapter (deprecated as of Python 3.12; see +`_adapt_datetime_iso`'s docstring for the exact spelling it used to produce +and why old rows keep reading correctly with no migration). """ from __future__ import annotations -import re import sqlite3 +from datetime import datetime from typing import Any +from mcp_server.infrastructure.sqlite_sql_translate import ( + _returning_was_stripped, + _translate_sql, +) + + +def _adapt_datetime_iso(value: datetime) -> str: + """Explicit `sqlite3.register_adapter` callback for `datetime.datetime`. + + Precondition: `value` is a `datetime.datetime` instance — sqlite3 only + invokes a registered adapter with instances of the exact type it was + registered for. + Postcondition: returns `value.isoformat()` — "T"-separated ISO-8601, + identical to `sqlite_store._now_iso()` and every other datetime write + path in this codebase, so a datetime-bearing column has one wire format + regardless of whether the caller passed an ISO string directly or (as + `cascade.py::_update_stage_entered` does) a raw `datetime` object as a + bound parameter — confirmed the sole such call site in this codebase by + an instrumented full-suite run (issue #260). + + This supersedes the stdlib's *implicit* default adapter, deprecated as + of Python 3.12: that fallback produced `value.isoformat(" ")` (a space + separator) instead of "T". `datetime.fromisoformat()` — the read path + used by every reader of a datetime-bearing column here (`decay_cycle.py`, + `consolidation_engine.py`, `cascade.py`) — parses both spellings to an + identical `datetime` object (verified empirically, CPython 3.12.12: + `fromisoformat("2026-07-29 12:34:56.789012+00:00") == + fromisoformat("2026-07-29T12:34:56.789012+00:00")`), so rows already on + disk in the old spelling keep reading correctly — no migration needed. + """ + return value.isoformat() -# SQLite grew RETURNING in 3.35.0 (2021-03-12). -# source: https://sqlite.org/lang_returning.html ("added in 3.35.0") -# When present we keep the clause, because upsert callers need the id of the -# row the statement actually touched: on the ON CONFLICT DO UPDATE path -# lastrowid does NOT identify the updated row, so the strip-and-synthesise -# fallback below would hand back the wrong page id. Older runtimes keep the -# historical strip behaviour. -_SUPPORTS_RETURNING = sqlite3.sqlite_version_info >= (3, 35, 0) - - -def _returning_was_stripped(sql: str) -> bool: - """True when this SQL had a RETURNING clause that translation removed. - The caller uses it to decide whether a `None` from fetchone() means "no - row" or "the clause was stripped, synthesise the id". It must be False - when RETURNING survives translation, otherwise a genuinely filtered-out - upsert (body_hash unchanged) would be reported as a write. - """ - return bool(re.search(r"\bRETURNING\b", sql, re.IGNORECASE)) and ( - not _SUPPORTS_RETURNING - ) - - -def _translate_sql(sql: str) -> str: - """Translate psycopg-style SQL to SQLite-compatible SQL.""" - # Named (pyformat) placeholders: %(name)s -> :name. Must run before the - # positional pass. The bulk claim/draft/page inserts bind by name, so - # without this the wiki pipeline extracts zero claims and reports the - # per-memory failure as `near "%": syntax error` (issue #206). - out = re.sub(r"%\((\w+)\)s", r":\1", sql) - - # Parameter placeholders: %s -> ? - out = out.replace("%s", "?") - - # Strip PostgreSQL type casts: ::jsonb, ::TEXT, ::REAL, and the ARRAY - # forms ::int[] / ::bigint[]. The trailing [] must be consumed with the - # cast — matching only `::int` left a stray `[]` behind, turning - # `%s::int[]` into `?[]` and failing with a syntax error (issue #206). - out = re.sub(r"::\w+(\s*\[\s*\])?", "", out) - - # SERIAL PRIMARY KEY -> INTEGER PRIMARY KEY AUTOINCREMENT - out = re.sub( - r"\bSERIAL\s+PRIMARY\s+KEY\b", - "INTEGER PRIMARY KEY AUTOINCREMENT", - out, - flags=re.IGNORECASE, - ) - - # TIMESTAMPTZ -> TEXT - out = re.sub(r"\bTIMESTAMPTZ\b", "TEXT", out, flags=re.IGNORECASE) - - # DEFAULT NOW() -> DEFAULT (strftime(...)) - out = re.sub( - r"\bDEFAULT\s+NOW\(\)", - "DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))", - out, - flags=re.IGNORECASE, - ) - - # Standalone NOW() in VALUES -> strftime(...) - out = re.sub( - r"\bNOW\(\)", - "strftime('%Y-%m-%dT%H:%M:%fZ', 'now')", - out, - flags=re.IGNORECASE, - ) - - # `UPDATE tbl l SET ...` -> `UPDATE tbl AS l SET ...`. PostgreSQL accepts - # a bare table alias; SQLite's UPDATE grammar requires the AS keyword. - # (SQLite has supported UPDATE ... FROM since 3.33, so only the alias - # spelling needs fixing.) Used by the backlink resolution pass. - # The table may still be schema-qualified (`wiki.links`) at this point — - # the wiki.
flattening runs further down — so the name pattern - # has to admit a dot. - out = re.sub( - r"\bUPDATE\s+([\w.]+)\s+(?!AS\b|SET\b)(\w+)\s+SET\b", - r"UPDATE \1 AS \2 SET", - out, - flags=re.IGNORECASE, - ) - - # `col = ANY(?)` -> `col IN (SELECT value FROM json_each(?))`. The bound - # parameter is a Python list, which the store's adapter serialises to a - # JSON array, so json_each expands it back into rows. Handles both the - # positional and the named placeholder forms. - out = re.sub( - r"=\s*ANY\s*\(\s*(\?|:\w+)\s*\)", - r"IN (SELECT value FROM json_each(\1))", - out, - flags=re.IGNORECASE, - ) - - # `col && ?` (PostgreSQL array overlap) -> a JSON intersection test. - # Both sides are JSON arrays under SQLite (see sqlite_schema_wiki), so - # "do these share any element" becomes a join of their json_each rows. - # Used by get_concepts_by_entity_overlap, which drives concept emergence. - out = re.sub( - r"\b([a-z_][a-z0-9_.]*)\s*&&\s*\?", - ( - r"EXISTS (SELECT 1 FROM json_each(\1) AS _ovl_l " - r"JOIN json_each(?) AS _ovl_r ON _ovl_l.value = _ovl_r.value)" - ), - out, - flags=re.IGNORECASE, - ) - - # `(xmax = 0) AS inserted` -> dropped. xmax is a PostgreSQL system column - # used by upsert_page to tell INSERT from UPDATE; SQLite has no analogue. - # Dropping it is safe because no caller reads the alias — upsert_page - # branches on whether a row came back at all, which RETURNING id alone - # answers. Leaving it in produced `near ",": syntax error` (issue #206). - out = re.sub( - r",\s*\(\s*xmax\s*=\s*0\s*\)\s+AS\s+\w+", - "", - out, - flags=re.IGNORECASE, - ) - - # `a IS DISTINCT FROM b` -> `a IS NOT b`: SQLite's IS/IS NOT are already - # NULL-safe, which is exactly what IS DISTINCT FROM means. - out = re.sub(r"\bIS\s+DISTINCT\s+FROM\b", "IS NOT", out, flags=re.IGNORECASE) - - # RETURNING ... -> stripped only on runtimes without native support. - if not _SUPPORTS_RETURNING: - out = re.sub(r"\bRETURNING\s+\w+\b", "", out, flags=re.IGNORECASE) - - # wiki.
-> wiki_
. SQLite has no schema namespaces, so the - # isolated `wiki` PostgreSQL schema is flattened to a table-name prefix - # (mcp_server/infrastructure/sqlite_schema_wiki.py declares them). - out = re.sub(r"\bwiki\.([a-z_]+)", r"wiki_\1", out, flags=re.IGNORECASE) - - # array_length(col, 1) -> json_array_length(col). PostgreSQL INTEGER[]/ - # BIGINT[] columns are stored as JSON TEXT under SQLite; PostgreSQL - # returns NULL for an empty array here, json_array_length returns 0 — - # both are falsy against the `> 0` predicate every call site uses. - out = re.sub( - r"\barray_length\s*\(\s*([a-z_.]+)\s*,\s*1\s*\)", - r"json_array_length(\1)", - out, - flags=re.IGNORECASE, - ) - - return out +# Registered once at import time (idempotent — sqlite3 stores adapters in a +# type-keyed dict, so re-import / re-registration just overwrites the same +# entry). Must happen before any `datetime` is bound as a parameter through +# this module's execute()/executemany() paths; every construction site of +# `PsycopgCompatConnection` imports this module first (there is no other way +# to obtain the class), so that ordering is guaranteed. +sqlite3.register_adapter(datetime, _adapt_datetime_iso) class _CompatCursor: diff --git a/mcp_server/infrastructure/sqlite_schema_wiki.py b/mcp_server/infrastructure/sqlite_schema_wiki.py index 25649de7..aaabf02c 100644 --- a/mcp_server/infrastructure/sqlite_schema_wiki.py +++ b/mcp_server/infrastructure/sqlite_schema_wiki.py @@ -1,9 +1,9 @@ """SQLite mirror of the PostgreSQL `wiki` schema (pg_schema.py::WIKI_SCHEMA_DDL). SQLite has no schema namespaces, so `wiki.
` is flattened to -`wiki_
`; sqlite_compat._translate_sql rewrites the SQL on the way in, -which is what lets the 52 modules that query these tables run unmodified on -both backends. +`wiki_
`; sqlite_sql_translate._translate_sql rewrites the SQL on the +way in (imported by sqlite_compat.py, issue #260), which is what lets the +52 modules that query these tables run unmodified on both backends. Type mapping applied here (issue #206): diff --git a/mcp_server/infrastructure/sqlite_sql_translate.py b/mcp_server/infrastructure/sqlite_sql_translate.py new file mode 100644 index 00000000..91e45e90 --- /dev/null +++ b/mcp_server/infrastructure/sqlite_sql_translate.py @@ -0,0 +1,170 @@ +"""PostgreSQL -> SQLite SQL dialect translation. + +Pure string/regex rewriting, zero I/O: extracted from `sqlite_compat.py` +(issue #260) to keep that file under the project's 300-line cap +(CLAUDE.md, coding-standards §4.1) after adding the datetime-adapter fix. +`sqlite_compat.py` imports `_translate_sql`/`_returning_was_stripped` from +here for its own use; the split is behaviour-preserving — same functions, +same module-global `_SUPPORTS_RETURNING` contract, full suite green +unchanged (see the issue #260 PR). + +Translations: + - %s -> ? (parameter placeholders) + - ::jsonb, ::TEXT, ::REAL, ::INT -> stripped (type casts) + - SERIAL PRIMARY KEY -> INTEGER PRIMARY KEY AUTOINCREMENT + - TIMESTAMPTZ -> TEXT + - DEFAULT NOW() -> DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) + - ON CONFLICT ... DO UPDATE SET -> preserved (SQLite 3.24+) + - RETURNING id -> stripped (use lastrowid instead) + - wiki.
-> wiki_
(SQLite has no schema namespaces) + - array_length(col, 1) -> json_array_length(col) (arrays are JSON TEXT) +""" + +from __future__ import annotations + +import re +import sqlite3 + +# SQLite grew RETURNING in 3.35.0 (2021-03-12). +# source: https://sqlite.org/lang_returning.html ("added in 3.35.0") +# When present we keep the clause, because upsert callers need the id of the +# row the statement actually touched: on the ON CONFLICT DO UPDATE path +# lastrowid does NOT identify the updated row, so the strip-and-synthesise +# fallback below would hand back the wrong page id. Older runtimes keep the +# historical strip behaviour. +_SUPPORTS_RETURNING = sqlite3.sqlite_version_info >= (3, 35, 0) + + +def _returning_was_stripped(sql: str) -> bool: + """True when this SQL had a RETURNING clause that translation removed. + + The caller uses it to decide whether a `None` from fetchone() means "no + row" or "the clause was stripped, synthesise the id". It must be False + when RETURNING survives translation, otherwise a genuinely filtered-out + upsert (body_hash unchanged) would be reported as a write. + """ + return bool(re.search(r"\bRETURNING\b", sql, re.IGNORECASE)) and ( + not _SUPPORTS_RETURNING + ) + + +def _translate_sql(sql: str) -> str: + """Translate psycopg-style SQL to SQLite-compatible SQL.""" + # Named (pyformat) placeholders: %(name)s -> :name. Must run before the + # positional pass. The bulk claim/draft/page inserts bind by name, so + # without this the wiki pipeline extracts zero claims and reports the + # per-memory failure as `near "%": syntax error` (issue #206). + out = re.sub(r"%\((\w+)\)s", r":\1", sql) + + # Parameter placeholders: %s -> ? + out = out.replace("%s", "?") + + # Strip PostgreSQL type casts: ::jsonb, ::TEXT, ::REAL, and the ARRAY + # forms ::int[] / ::bigint[]. The trailing [] must be consumed with the + # cast — matching only `::int` left a stray `[]` behind, turning + # `%s::int[]` into `?[]` and failing with a syntax error (issue #206). + out = re.sub(r"::\w+(\s*\[\s*\])?", "", out) + + # SERIAL PRIMARY KEY -> INTEGER PRIMARY KEY AUTOINCREMENT + out = re.sub( + r"\bSERIAL\s+PRIMARY\s+KEY\b", + "INTEGER PRIMARY KEY AUTOINCREMENT", + out, + flags=re.IGNORECASE, + ) + + # TIMESTAMPTZ -> TEXT + out = re.sub(r"\bTIMESTAMPTZ\b", "TEXT", out, flags=re.IGNORECASE) + + # DEFAULT NOW() -> DEFAULT (strftime(...)) + out = re.sub( + r"\bDEFAULT\s+NOW\(\)", + "DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))", + out, + flags=re.IGNORECASE, + ) + + # Standalone NOW() in VALUES -> strftime(...) + out = re.sub( + r"\bNOW\(\)", + "strftime('%Y-%m-%dT%H:%M:%fZ', 'now')", + out, + flags=re.IGNORECASE, + ) + + # `UPDATE tbl l SET ...` -> `UPDATE tbl AS l SET ...`. PostgreSQL accepts + # a bare table alias; SQLite's UPDATE grammar requires the AS keyword. + # (SQLite has supported UPDATE ... FROM since 3.33, so only the alias + # spelling needs fixing.) Used by the backlink resolution pass. + # The table may still be schema-qualified (`wiki.links`) at this point — + # the wiki.
flattening runs further down — so the name pattern + # has to admit a dot. + out = re.sub( + r"\bUPDATE\s+([\w.]+)\s+(?!AS\b|SET\b)(\w+)\s+SET\b", + r"UPDATE \1 AS \2 SET", + out, + flags=re.IGNORECASE, + ) + + # `col = ANY(?)` -> `col IN (SELECT value FROM json_each(?))`. The bound + # parameter is a Python list, which the store's adapter serialises to a + # JSON array, so json_each expands it back into rows. Handles both the + # positional and the named placeholder forms. + out = re.sub( + r"=\s*ANY\s*\(\s*(\?|:\w+)\s*\)", + r"IN (SELECT value FROM json_each(\1))", + out, + flags=re.IGNORECASE, + ) + + # `col && ?` (PostgreSQL array overlap) -> a JSON intersection test. + # Both sides are JSON arrays under SQLite (see sqlite_schema_wiki), so + # "do these share any element" becomes a join of their json_each rows. + # Used by get_concepts_by_entity_overlap, which drives concept emergence. + out = re.sub( + r"\b([a-z_][a-z0-9_.]*)\s*&&\s*\?", + ( + r"EXISTS (SELECT 1 FROM json_each(\1) AS _ovl_l " + r"JOIN json_each(?) AS _ovl_r ON _ovl_l.value = _ovl_r.value)" + ), + out, + flags=re.IGNORECASE, + ) + + # `(xmax = 0) AS inserted` -> dropped. xmax is a PostgreSQL system column + # used by upsert_page to tell INSERT from UPDATE; SQLite has no analogue. + # Dropping it is safe because no caller reads the alias — upsert_page + # branches on whether a row came back at all, which RETURNING id alone + # answers. Leaving it in produced `near ",": syntax error` (issue #206). + out = re.sub( + r",\s*\(\s*xmax\s*=\s*0\s*\)\s+AS\s+\w+", + "", + out, + flags=re.IGNORECASE, + ) + + # `a IS DISTINCT FROM b` -> `a IS NOT b`: SQLite's IS/IS NOT are already + # NULL-safe, which is exactly what IS DISTINCT FROM means. + out = re.sub(r"\bIS\s+DISTINCT\s+FROM\b", "IS NOT", out, flags=re.IGNORECASE) + + # RETURNING ... -> stripped only on runtimes without native support. + if not _SUPPORTS_RETURNING: + out = re.sub(r"\bRETURNING\s+\w+\b", "", out, flags=re.IGNORECASE) + + # wiki.
-> wiki_
. SQLite has no schema namespaces, so the + # isolated `wiki` PostgreSQL schema is flattened to a table-name prefix + # (mcp_server/infrastructure/sqlite_schema_wiki.py declares them). + out = re.sub(r"\bwiki\.([a-z_]+)", r"wiki_\1", out, flags=re.IGNORECASE) + + # array_length(col, 1) -> json_array_length(col). PostgreSQL INTEGER[]/ + # BIGINT[] columns are stored as JSON TEXT under SQLite; PostgreSQL + # returns NULL for an empty array here, json_array_length returns 0 — + # both are falsy against the `> 0` predicate every call site uses. + out = re.sub( + r"\barray_length\s*\(\s*([a-z_.]+)\s*,\s*1\s*\)", + r"json_array_length(\1)", + out, + flags=re.IGNORECASE, + ) + + return out diff --git a/pyproject.toml b/pyproject.toml index fb1a6027..fd91a578 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -401,6 +401,16 @@ required_plugins = ["pytest-timeout", "pytest-asyncio"] # reporting 0 warnings after deleting this entry. filterwarnings = [ "ignore:builtin type (SwigPyObject|SwigPyPacked|swigvarlink) has no __module__ attribute:DeprecationWarning", + # Regression tripwire for issue #260: `sqlite_compat.py` now registers an + # explicit `sqlite3.register_adapter(datetime, ...)` (see its module + # docstring), so nothing in this codebase should ever again hit the + # stdlib's *implicit* default adapter for `datetime.datetime`. If this + # fires, a new bound-raw-datetime call site was added outside that + # adapter's coverage — turn it into a hard failure so it cannot silently + # return the way it did before (3 warnings across + # tests_py/handlers/test_consolidate.py and + # tests_py/integration/test_memory_lifecycle.py). + "error:The default datetime adapter is deprecated as of Python 3.12.*:DeprecationWarning", ] markers = [ "invariants: Phase 0.3+ testable invariant predicates (I1/I2/I3/I4 and parity tests)", diff --git a/tests_py/handlers/test_wiki_pipeline_sqlite.py b/tests_py/handlers/test_wiki_pipeline_sqlite.py index 03db6897..fc823ae0 100644 --- a/tests_py/handlers/test_wiki_pipeline_sqlite.py +++ b/tests_py/handlers/test_wiki_pipeline_sqlite.py @@ -269,10 +269,17 @@ def test_returning_is_stripped_when_the_runtime_lacks_native_support(monkeypatch Without this the whole `if not _SUPPORTS_RETURNING:` arm is unreachable here (SQLite 3.35 added RETURNING in 2021), so every mutant of it survived a scoped mutation run — the branch was shipping untested. + + `_SUPPORTS_RETURNING` is patched on `sqlite_sql_translate` — the module + that DEFINES `_translate_sql`/`_returning_was_stripped` (issue #260 + split) — not on `sqlite_compat`, which only re-exports the names via + import. Patching the re-exported copy would be a silent no-op: the + functions read the flag from their own module's globals. """ import mcp_server.infrastructure.sqlite_compat as sc + import mcp_server.infrastructure.sqlite_sql_translate as sqltr - monkeypatch.setattr(sc, "_SUPPORTS_RETURNING", False) + monkeypatch.setattr(sqltr, "_SUPPORTS_RETURNING", False) assert sc._translate_sql( "INSERT INTO wiki.pages (slug) VALUES (%s) RETURNING id" @@ -286,10 +293,13 @@ def test_stripped_returning_synthesises_the_insert_id(monkeypatch): pg_store_wiki_common._returning_id RAISES on None, so every bulk wiki insert depends on this synthesis when running below SQLite 3.35. + + See `test_returning_is_stripped_when_the_runtime_lacks_native_support` + for why the patch target is `sqlite_sql_translate`, not `sqlite_compat`. """ - import mcp_server.infrastructure.sqlite_compat as sc + import mcp_server.infrastructure.sqlite_sql_translate as sqltr - monkeypatch.setattr(sc, "_SUPPORTS_RETURNING", False) + monkeypatch.setattr(sqltr, "_SUPPORTS_RETURNING", False) conn = _compat_conn() conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT)") @@ -316,7 +326,7 @@ def test_returning_was_stripped_tracks_native_support(): synthesised `{"id": lastrowid}` — reporting a write that never happened, with an id that is not even the right row. """ - from mcp_server.infrastructure.sqlite_compat import ( + from mcp_server.infrastructure.sqlite_sql_translate import ( _SUPPORTS_RETURNING, _returning_was_stripped, ) @@ -375,7 +385,7 @@ def test_fresh_cursor_has_psycopg_shaped_defaults(): def test_returning_detection_is_case_insensitive(): - from mcp_server.infrastructure.sqlite_compat import ( + from mcp_server.infrastructure.sqlite_sql_translate import ( _SUPPORTS_RETURNING, _returning_was_stripped, ) @@ -393,6 +403,134 @@ def test_connection_execute_returns_rows_as_dicts(): assert conn.execute("SELECT a FROM t").fetchone() == {"a": "x"} +# ── _CompatCursor / PsycopgCompatConnection field wiring (issue #260 ───── +# boy-scout: surfaced by a scoped mutation run against sqlite_compat.py +# while fixing the datetime-adapter bug — these pins were missing entirely, +# not merely weak, so mutants of `_CompatCursor.__init__`/`fetchone` and +# `PsycopgCompatConnection.execute` survived). + + +def test_compat_cursor_had_returning_defaults_to_false(): + """`_CompatCursor` is instantiated directly here, not through + `PsycopgCompatConnection.execute` (which always passes every keyword + explicitly) — pinning the class's own declared default independent of + its one current caller.""" + import sqlite3 + + from mcp_server.infrastructure.sqlite_compat import _CompatCursor + + raw = sqlite3.connect(":memory:") + raw.execute("CREATE TABLE t (id INTEGER PRIMARY KEY)") + cur = raw.execute("SELECT id FROM t") # no rows + wrapped = _CompatCursor(cur, lastrowid=5) # had_returning NOT passed + assert wrapped.fetchone() is None + + +def test_compat_cursor_rowcount_mirrors_the_wrapped_cursor(): + import sqlite3 + + from mcp_server.infrastructure.sqlite_compat import _CompatCursor + + raw = sqlite3.connect(":memory:") + raw.execute("CREATE TABLE t (id INTEGER PRIMARY KEY)") + raw.execute("INSERT INTO t (id) VALUES (1)") + cur = raw.execute("UPDATE t SET id = 2") + wrapped = _CompatCursor(cur, lastrowid=None) + assert wrapped.rowcount == cur.rowcount == 1 + + +def test_compat_cursor_synthesises_the_exact_id_key_when_flagged(): + """Exact dict-equality (not `"id" in result`) pins the literal key + spelling and the stored `had_returning`/`lastrowid` values verbatim.""" + import sqlite3 + + from mcp_server.infrastructure.sqlite_compat import _CompatCursor + + raw = sqlite3.connect(":memory:") + raw.execute("CREATE TABLE t (id INTEGER PRIMARY KEY)") + cur = raw.execute("SELECT id FROM t") # no rows + wrapped = _CompatCursor(cur, lastrowid=7, had_returning=True) + assert wrapped.fetchone() == {"id": 7} + + +def test_compat_cursor_needs_both_flag_and_lastrowid_to_synthesise(): + """`had_returning AND lastrowid`, not `OR`: a falsy lastrowid (0 here) + must not synthesise a row even when RETURNING was stripped.""" + import sqlite3 + + from mcp_server.infrastructure.sqlite_compat import _CompatCursor + + raw = sqlite3.connect(":memory:") + raw.execute("CREATE TABLE t (id INTEGER PRIMARY KEY)") + cur = raw.execute("SELECT id FROM t") # no rows + wrapped = _CompatCursor(cur, lastrowid=0, had_returning=True) + assert wrapped.fetchone() is None + + +def test_executemany_clears_had_returning_and_reports_real_rowcount(): + """Pins the post-executemany field wiring, not just the row output. + + `lastrowid` is asserted `None` too, matching the documented sqlite3 + contract (a cursor's `lastrowid` is only meaningful after a single-row + `execute()` INSERT, never after `executemany()` + https://docs.python.org/3/library/sqlite3.html#sqlite3.Cursor.lastrowid) + — which makes a mutant that hardcodes `self.lastrowid = None` in + `executemany()` a documented EQUIVALENT mutant here, not a gap: no + input can make the real `self._cursor.lastrowid` differ from `None` + after an `executemany()` call, so no test can distinguish them. + """ + conn = _compat_conn() + conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT)") + with conn.cursor() as cur: + cur.executemany("INSERT INTO t (a) VALUES (%s)", [("x",), ("y",)]) + assert cur.lastrowid is None + assert cur.rowcount == 2 + assert cur._had_returning is False + + +def test_connection_execute_computes_had_returning_from_the_actual_sql( + monkeypatch, +): + """`PsycopgCompatConnection.execute` must compute `had_returning` from + the SQL it was actually given, not silently default it away. Forces the + pre-3.35 strip path (this runtime natively supports RETURNING) and + asserts the returned cursor synthesises the id — the same contract + `test_stripped_returning_synthesises_the_insert_id` pins for the + `cursor()` path, exercised here through `execute()` directly.""" + import mcp_server.infrastructure.sqlite_sql_translate as sqltr + + monkeypatch.setattr(sqltr, "_SUPPORTS_RETURNING", False) + + conn = _compat_conn() + conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT)") + cur = conn.execute("INSERT INTO t (a) VALUES (%s) RETURNING id", ("x",)) + assert cur.fetchone() == {"id": 1} + + +def test_executescript_runs_every_statement_verbatim(): + conn = _compat_conn() + conn.executescript( + "CREATE TABLE t (id INTEGER PRIMARY KEY); INSERT INTO t (id) VALUES (1);" + ) + assert conn.execute("SELECT id FROM t").fetchone() == {"id": 1} + + +def test_enable_load_extension_forwards_the_flag_verbatim(): + """Spies on the wrapped connection: sqlite3 accepts `enable_load_extension + (None)` silently (it does not raise), so only a spy — not a raised + exception — can tell a forwarded `False` apart from a hardcoded one.""" + calls = [] + + class _FakeRealConn: + def enable_load_extension(self, enabled): + calls.append(enabled) + + conn = PsycopgCompatConnection(_FakeRealConn()) + conn.enable_load_extension(True) + conn.enable_load_extension(False) + assert calls == [True, False] + + # ── The seven handler entry paths (issue #206 acceptance criterion 3) ──── diff --git a/tests_py/infrastructure/test_sqlite_datetime_adapter_260.py b/tests_py/infrastructure/test_sqlite_datetime_adapter_260.py new file mode 100644 index 00000000..5b93921f --- /dev/null +++ b/tests_py/infrastructure/test_sqlite_datetime_adapter_260.py @@ -0,0 +1,172 @@ +"""Explicit datetime adapter for the SQLite compat layer (issue #260). + +`cascade.py::_update_stage_entered` binds a raw `datetime.datetime` object +as a SQL parameter through `PsycopgCompatConnection.execute` — the only +such call site in this codebase (confirmed by an instrumented full-suite +run, see the issue #260 PR description). Before this fix, sqlite3 fell +back to its *implicit* default adapter for that type, which is deprecated +as of Python 3.12 and fired on every affected test +(`tests_py/handlers/test_consolidate.py::test_with_memories`, +`::test_protected_memories_skip_compression`, +`tests_py/integration/test_memory_lifecycle.py::test_store_consolidate_recall`). + +Every test in this module reproduces that exact bound-parameter shape and +fails on the pre-fix code (either by the DeprecationWarning firing, or — +for the backward-compat test — by proving the old and new spellings do not +collapse to the same parsed value, which would be the case if the fix had +silently changed the wire format instead of matching the existing one). +""" + +from __future__ import annotations + +import sqlite3 +import warnings +from datetime import datetime, timezone + +from mcp_server.infrastructure.sqlite_compat import ( + PsycopgCompatConnection, + _adapt_datetime_iso, +) + + +def _compat_conn() -> PsycopgCompatConnection: + raw = sqlite3.connect(":memory:") + raw.row_factory = sqlite3.Row + return PsycopgCompatConnection(raw) + + +class TestAdaptDatetimeIso: + """Unit contract for the adapter function itself.""" + + def test_returns_plain_isoformat(self): + value = datetime(2026, 7, 29, 12, 34, 56, 789012, tzinfo=timezone.utc) + assert _adapt_datetime_iso(value) == value.isoformat() + assert _adapt_datetime_iso(value) == "2026-07-29T12:34:56.789012+00:00" + + def test_naive_datetime_round_trips_through_isoformat(self): + """No tzinfo is still a valid `datetime`; the adapter must not raise.""" + value = datetime(2026, 7, 29, 12, 34, 56) + assert _adapt_datetime_iso(value) == "2026-07-29T12:34:56" + + +class TestNoDeprecationWarningOnBoundDatetime: + """Reproduces the exact bug: fails on pre-fix code (adapter not + registered), passes after (explicit adapter takes over before sqlite3's + default-adapter fallback ever triggers).""" + + def test_connection_execute_with_raw_datetime_param(self): + conn = _compat_conn() + conn.execute("CREATE TABLE memories (id INTEGER PRIMARY KEY, ts TEXT)") + conn.execute("INSERT INTO memories (id) VALUES (1)") + + now = datetime.now(timezone.utc) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + # Mirrors cascade.py::_update_stage_entered exactly: UPDATE ... + # SET = %s with a raw datetime bound, not `.isoformat()`. + conn.execute("UPDATE memories SET ts = %s WHERE id = %s", (now, 1)) + + deprecation_msgs = [ + str(w.message) for w in caught if issubclass(w.category, DeprecationWarning) + ] + assert deprecation_msgs == [], ( + f"expected no DeprecationWarning binding a raw datetime, " + f"got: {deprecation_msgs}" + ) + + row = conn.execute("SELECT ts FROM memories WHERE id = 1").fetchone() + assert row["ts"] == now.isoformat() + + def test_cursor_execute_with_raw_datetime_param(self): + """Same reproduction through the `with conn.cursor() as cur:` path.""" + conn = _compat_conn() + conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, ts TEXT)") + + now = datetime.now(timezone.utc) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + with conn.cursor() as cur: + cur.execute("INSERT INTO t (id, ts) VALUES (%s, %s)", (1, now)) + + deprecation_msgs = [ + str(w.message) for w in caught if issubclass(w.category, DeprecationWarning) + ] + assert deprecation_msgs == [] + + row = conn.execute("SELECT ts FROM t WHERE id = 1").fetchone() + assert row["ts"] == now.isoformat() + + def test_executemany_with_raw_datetime_params(self): + """The batch-insert compat path (issue #206) must adapt too.""" + conn = _compat_conn() + conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, ts TEXT)") + + t1 = datetime(2026, 1, 1, tzinfo=timezone.utc) + t2 = datetime(2026, 1, 2, tzinfo=timezone.utc) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + with conn.cursor() as cur: + cur.executemany( + "INSERT INTO t (id, ts) VALUES (%s, %s)", [(1, t1), (2, t2)] + ) + + deprecation_msgs = [ + str(w.message) for w in caught if issubclass(w.category, DeprecationWarning) + ] + assert deprecation_msgs == [] + + rows = conn.execute("SELECT ts FROM t ORDER BY id").fetchall() + assert [r["ts"] for r in rows] == [t1.isoformat(), t2.isoformat()] + + +class TestBackwardCompatibleRead: + """Acceptance criterion 3: existing SQLite stores keep reading. + + Writes a row in the PRE-CHANGE spelling (the stdlib's old *implicit* + adapter used `value.isoformat(" ")` — a space separator, confirmed + empirically against CPython 3.12.12 before this fix was written) via + raw SQL, alongside a row written through the POST-CHANGE explicit + adapter, and asserts `datetime.fromisoformat` — the read path every + consumer in this codebase uses (`decay_cycle.py`, `cascade.py`, + `consolidation_engine.py`) — parses both to the identical value. + """ + + def test_old_spelling_and_new_spelling_parse_identically(self): + value = datetime(2026, 7, 29, 12, 34, 56, 789012, tzinfo=timezone.utc) + old_spelling = value.isoformat(" ") # what the deprecated adapter wrote + new_spelling = _adapt_datetime_iso(value) # what this fix writes + + assert old_spelling != new_spelling # genuinely different spellings + assert datetime.fromisoformat(old_spelling) == datetime.fromisoformat( + new_spelling + ) + assert datetime.fromisoformat(old_spelling) == value + + def test_pre_fix_row_on_disk_still_reads_correctly(self): + """A row inserted with the OLD spelling (raw sqlite3, no compat + layer, simulating data written before this fix shipped) reads back + identically to one inserted through the NEW explicit-adapter path. + """ + conn = _compat_conn() + conn.execute( + "CREATE TABLE memories (id INTEGER PRIMARY KEY, stage_entered_at TEXT)" + ) + + value = datetime(2026, 6, 1, 8, 0, 0, tzinfo=timezone.utc) + old_spelling = value.isoformat(" ") + conn.execute( + "INSERT INTO memories (id, stage_entered_at) VALUES (%s, %s)", + (1, old_spelling), + ) + conn.execute( + "INSERT INTO memories (id, stage_entered_at) VALUES (%s, %s)", (2, value) + ) + + rows = { + r["id"]: r["stage_entered_at"] + for r in conn.execute( + "SELECT id, stage_entered_at FROM memories ORDER BY id" + ).fetchall() + } + assert datetime.fromisoformat(rows[1]) == datetime.fromisoformat(rows[2]) + assert datetime.fromisoformat(rows[1]) == value From 56589330594dc034ffcdad8a6c500dec261b839d Mon Sep 17 00:00:00 2001 From: cdeust Date: Thu, 30 Jul 2026 09:16:16 +0200 Subject: [PATCH 2/2] test(mutation): kill sqlite_sql_translate.py's 20 surviving mutants (#265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scoped mutmut run against `_translate_sql`/`_returning_was_stripped` surfaced 20 surviving mutants, all the same shape: dropping (or re-spelling the case of) `flags=re.IGNORECASE` on one of the module's regex calls. Every existing case-insensitivity fixture happens to supply an input whose case already matches the pattern's own literal spelling, so the flag's presence was never observable. - 6 real gaps closed with new opposite-case tests (mutmut_8, 62, 117, 134, 161, 186): lowercase `DEFAULT NOW()`, an uppercase `&&`-overlap column, `XMAX`/`as` case variants on the xmax-drop rule, a lowercase `RETURNING` strip inside `_translate_sql` (distinct from the one `_returning_was_stripped` already covered), an uppercase `ARRAY_LENGTH`, and lowercase `_returning_was_stripped` input under a monkeypatched `_SUPPORTS_RETURNING`. - 14 documented equivalent mutants (mutmut_10, 38, 51, 64, 78, 92, 105, 119, 136, 137, 149, 163, 175, 188): the mutation only re-spells the pattern's own literal case while `re.IGNORECASE` stays in place, which Python's `re` semantics make provably irrelevant to the match — confirmed empirically with a differential harness, not asserted by inspection. Re-running the reproduction: 6 killed, 14 equivalent, 0 unaccounted-for survivors. Suite grows 6560 -> 6591; doc-claim/badge counts resynced to the measured `pytest --collect-only -q` absolute. Closes #265 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u --- .bestpractices.json | 8 +- CHANGELOG.md | 1 + CLAUDE.md | 2 +- CONTRIBUTING.md | 4 +- README.md | 4 +- assets/badge-tests.svg | 8 +- docs/ASSURANCE-CASE.md | 2 +- .../test_sqlite_sql_translate_265.py | 110 ++++++++++++++++++ 8 files changed, 125 insertions(+), 14 deletions(-) create mode 100644 tests_py/infrastructure/test_sqlite_sql_translate_265.py diff --git a/.bestpractices.json b/.bestpractices.json index 30877995..3ff713fe 100644 --- a/.bestpractices.json +++ b/.bestpractices.json @@ -99,7 +99,7 @@ "build_floss_tools_justification": "pip, uv, hatchling, pytest, ruff and pyright are all FLOSS, and the build runs on ubuntu-latest and windows-latest GitHub runners.", "test_status": "Met", - "test_justification": "An automated test suite of 6588 tests lives under tests_py/ (measured 2026-07-30 with 'pytest --collect-only -q'), covering core, handlers, infrastructure, integration, invariants and architecture: https://github.com/cdeust/Cortex/tree/main/tests_py", + "test_justification": "An automated test suite of 6609 tests lives under tests_py/ (measured 2026-07-30 with 'pytest --collect-only -q'), covering core, handlers, infrastructure, integration, invariants and architecture: https://github.com/cdeust/Cortex/tree/main/tests_py", "test_invocation_status": "Met", "test_invocation_justification": "The whole suite runs with a single 'pytest' command, documented in CONTRIBUTING.md under Testing along with per-layer subsets: https://github.com/cdeust/Cortex/blob/main/CONTRIBUTING.md", @@ -114,7 +114,7 @@ "test_policy_justification": "CONTRIBUTING.md carries an explicit, mandatory testing policy: new functionality ships with tests in the automated suite, a bug fix carries a regression test that fails on the pre-fix code, and each failure path asserts its observable effect including the signal it emits \u2014 https://github.com/cdeust/Cortex/blob/main/CONTRIBUTING.md#the-testing-policy-mandatory. The per-tool checklist ('Add a unit test', 'Add an integration test if the tool touches the database') and the five-element mechanism checklist restate it per change type.", "tests_are_added_status": "Met", - "tests_are_added_justification": "New functionality ships with its tests; the suite has grown to 6588 tests alongside the v4.x feature series, and CI runs it on every pull request.", + "tests_are_added_justification": "New functionality ships with its tests; the suite has grown to 6609 tests alongside the v4.x feature series, and CI runs it on every pull request.", "tests_documented_added_status": "Met", "tests_documented_added_justification": "The requirement is written into the documented instructions for change proposals: https://github.com/cdeust/Cortex/blob/main/CONTRIBUTING.md#the-testing-policy-mandatory states that every change adding or altering observable behaviour must arrive with tests in the same PR, the per-tool and per-mechanism checklists repeat it as a concrete step, and .github/PULL_REQUEST_TEMPLATE.md requires a Test plan section in every pull request.", @@ -189,7 +189,7 @@ "static_analysis_often_justification": "CodeQL default setup analyses each push and pull request and additionally runs on a weekly schedule, so analysis happens per change rather than per release.", "dynamic_analysis_status": "Unmet", - "dynamic_analysis_justification": "No dynamic analysis tool in the badge's sense (fuzzer, sanitizer, or scanner) is applied. The project runs a 6588-test suite with coverage on every change, but a test suite is not a dynamic analysis tool and is not claimed as one here.", + "dynamic_analysis_justification": "No dynamic analysis tool in the badge's sense (fuzzer, sanitizer, or scanner) is applied. The project runs a 6609-test suite with coverage on every change, but a test suite is not a dynamic analysis tool and is not claimed as one here.", "dynamic_analysis_unsafe_status": "N/A", "dynamic_analysis_unsafe_justification": "Cortex is written in Python, a memory-safe language, so the memory-safety tooling this criterion asks about (ASan, Valgrind) does not apply.", @@ -297,7 +297,7 @@ "interfaces_current_justification": "The stack targets currently supported runtimes and APIs: Python 3.10 through 3.13, all four in the CI matrix, with pgvector 0.3+/PostgreSQL 17 and current major versions of FastMCP, Pydantic v2, numpy and sentence-transformers. Deprecated interfaces are removed rather than wrapped \u2014 the standing rule is one-shot migrations with no back-compat shims (CLAUDE.md), and CI runs on the newest released Python so a deprecation surfaces as a warning in the build rather than as a surprise at end of life.", "automated_integration_testing_status": "Met", - "automated_integration_testing_justification": "The full suite runs on every push and pull request to main via .github/workflows/ci.yml and reports success or failure per job: 6588 tests on Python 3.10-3.13 against PostgreSQL + pgvector, the same suite against the SQLite backend, the suite on Windows, and a Docker smoke job that boots the bare container and exercises the DB-less contract. tests_py/integration/ holds the database-backed integration tests specifically.", + "automated_integration_testing_justification": "The full suite runs on every push and pull request to main via .github/workflows/ci.yml and reports success or failure per job: 6609 tests on Python 3.10-3.13 against PostgreSQL + pgvector, the same suite against the SQLite backend, the suite on Windows, and a Docker smoke job that boots the bare container and exercises the DB-less contract. tests_py/integration/ holds the database-backed integration tests specifically.", "regression_tests_added50_status": "Met", "regression_tests_added50_justification": "Measured on 2026-07-27 over the merged pull requests titled as fixes in the preceding six months (2026-01-27 onward): 23 of 30 \u2014 76.7% \u2014 changed the test tree in the same PR, against the 50% this criterion asks for. (The measure counts a PR as carrying tests when it touches tests_py/ or tests_js/, which is a proxy for 'added a regression test'; the policy behind it is written down at https://github.com/cdeust/Cortex/blob/main/CONTRIBUTING.md#the-testing-policy-mandatory \u2014 a bug fix carries a regression test that fails on the pre-fix code.)", diff --git a/CHANGELOG.md b/CHANGELOG.md index 05047e54..3d1ef42f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ adheres to [Semantic Versioning](https://semver.org/). - **Coverage-guided fuzzing** (closes the Scorecard Fuzzing alert). Two harnesses in `fuzz/` over pure parsers that read untrusted text (§13.1 D2 — LLM-generated content is untrusted): the hand-rolled YAML frontmatter parser and the wiki source-path canonicaliser. Wired to **ClusterFuzzLite** (`.clusterfuzzlite/`, `.github/workflows/fuzz.yml`) — a 120s batch on PRs that blocks, and a longer scheduled run that does not, because a fuzzer left running will eventually find something and holding the merge queue hostage to an unrelated input makes the check ignored within a week. Writing the path harness **found a live bug**: `normalize_source_path` stripped `./` in a loop and then `/` exactly once, so removing the slashes could expose a `./` the loop had already walked past — `.//./x` came out as `./x`, still carrying the prefix the function exists to remove, and not idempotent. `extract_document_paths` dedupes on that result, so one document reachable by two spellings counted as two. Fixed by iterating to a fixed point; the four reproducers are committed as corpus inputs and **fail on the pre-fix code**. `fuzz/replay_corpus.py` runs every corpus input through its harness with no atheris, so the properties execute in the ordinary `pytest` suite on every platform — atheris publishes manylinux x86_64 wheels for cpython 3.12–3.14 and nothing else, and a property only one CI job can run is one that rots. ### Fixed +- **`sqlite_sql_translate.py`'s 20 surviving mutants, closed** (#265). This module (`_translate_sql`/`_returning_was_stripped`, split out of `sqlite_compat.py` by #260) left 20 mutants surviving a scoped mutmut run — every one the same shape: a mutant dropping (or re-spelling the case of) the `flags=re.IGNORECASE` argument on one of the module's `re.sub`/`re.search` calls. Every existing case-insensitivity fixture supplies an input whose case already matches the pattern's own literal spelling, so the flag's presence was never observable. A rescoped run on this tree measured 19 of the 20 named ids still surviving (`mutmut_136` had flipped to killed between runs — non-deterministic mutant/worker ordering, not a real fix, folded back into the equivalent set below by direct regex comparison). Six are real gaps, closed with `tests_py/infrastructure/test_sqlite_sql_translate_265.py` supplying the opposite-case input for each: lowercase `DEFAULT now()`, an uppercase `&&`-overlap column, `XMAX`/`as` case variants on the xmax-drop rule, a lowercase `RETURNING` strip inside `_translate_sql` distinct from the one `_returning_was_stripped` already covered, an uppercase `ARRAY_LENGTH`, and lowercase `_returning_was_stripped` input under a monkeypatched `_SUPPORTS_RETURNING` — 6 new tests, all failing on pre-fix code. The remaining 14 are **documented equivalent mutants**: the mutation only re-spells the pattern's own literal case (`SERIAL`→`serial`, char classes `[a-z_]`↔`[A-Z_]`, etc.) while `re.IGNORECASE` stays in place, which Python's `re` semantics make provably irrelevant to the match — confirmed empirically with a differential harness (uppercase/lowercase/mixed-case probes against the original and mutated pattern, identical match results in every case) rather than asserted by inspection alone. Re-running the reproduction after the fix: 6 killed, 14 equivalent, 0 unaccounted-for survivors. - **A `created_at` that states a timezone was stored as the wrong instant** (#252). `normalize_date_to_iso` had no timezone policy on any of its paths, so three defects stacked: (1) the "already ISO" guard was the substring test `"T" in raw`, and every US zone abbreviation contains a T — `8 May 2023 13:56 EST` was returned unparsed; (2) the built-in fast path matches the date at the START of the string and discards the rest, so `8 May 2023 13:56 +02:00` became midnight, dropping both the time and the offset; (3) on the dateutil path an abbreviation it cannot resolve is dropped with a warning nobody sees, leaving a naive datetime that PostgreSQL's `timestamptz` cast and `compute_recency_boost` both read as UTC. The instant was up to a day off and nothing was emitted. A stated zone is now honoured or the value is refused: `mcp_server/core/temporal_timezones.py` supplies dateutil a `tzinfos` resolver over the **RFC 5322 §4.3 obs-zone table** (the normative answer to "which EST?" — cross-checked against CPython's `email._parseaddr._timezones`), and any abbreviation outside it is refused with a warning naming the input, the abbreviation and the fix, rather than defaulted. Parsing no longer depends on the host's local zone name, and no `warnings.catch_warnings()` — process-global and not thread-safe — is taken on a store write path. `normalize_date_to_iso` moves out of `core/temporal.py` into `core/temporal_normalize.py`: storage normalization must not lose precision, retrieval scoring may, and they are now separate modules (both stores import from the new path). The refusal also covers the degraded path — a string that states a zone is never salvaged to a naive date, whatever made the parse fail. - **`python-dateutil` is now a declared dependency** (#252). `normalize_date_to_iso` has always parsed free-form dates with a time of day through it — the LoCoMo shape `1:56 pm on 8 May, 2023` its own docstring cites — but it was never declared and arrived only by transitive luck. It was absent from every `requirements/ci-*.txt`, so **every CI test job ran with that parser missing**: the fallback was dead code in CI, and any install resolving without it kept dates it could not read. Declared, locked and hash-pinned into the 9 exported requirement sets, so the write path behaves identically on every install and both backends. - **Both stores skipped the normalization entirely for the dates that needed it most** (#252, the same substring defect one layer up). `PgMemoryStore._build_insert_params` and `SqliteMemoryStore._insert_memory_rows` each guarded the call with `"T" not in raw_created` as a cheap "is it already ISO?" test — so `8 May 2023 13:56 EST` (and every `PST`/`CST`/`MST` string) went to the database untouched, whichever way `normalize_date_to_iso` behaved. The guard is gone from both: deciding what is already ISO belongs to the function that owns it, which returns a real ISO datetime unchanged. Asserted on the stored row, not just on the parser, and asserted equal across the two backends. diff --git a/CLAUDE.md b/CLAUDE.md index 885d751b..832e3e12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -23,7 +23,7 @@ coding write gates, causal graphs, and intent-aware retrieval. resolving from the `pyproject.toml` ranges instead gives you versions CI never had (issue #253). - Environment preflight: `python -m mcp_server.doctor` (backend-aware check list, fix message per check) -- Tests: `pytest` (full suite, 6588 tests) · `pytest tests_py/core/` (one layer) · `pytest --cov=mcp_server --cov-report=term-missing` +- Tests: `pytest` (full suite, 6609 tests) · `pytest tests_py/core/` (one layer) · `pytest --cov=mcp_server --cov-report=term-missing` - Lint BEFORE every commit: `ruff check && ruff format --check` — the CI enforces **both**; passing only `ruff check` is not enough. - Type gate (pyright, zero-diagnostic): resolve its environment from `uv.lock` (`uv sync --no-default-groups --extra … --group typecheck`), never from the diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6c0a45f8..709cd4fb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,7 +51,7 @@ bash scripts/setup.sh # macOS / Linux # Verify everything is wired uvx --python 3.13 --from "hypermnesia-mcp[postgresql]" cortex-doctor -# Run tests (6588 tests under tests_py/) +# Run tests (6609 tests under tests_py/) pytest # Run a benchmark @@ -163,7 +163,7 @@ The full standard lives in ## Testing ```bash -pytest # full suite (6588 tests) +pytest # full suite (6609 tests) pytest tests_py/core # core (pure business logic) only pytest tests_py/integration # PostgreSQL-backed integration pytest tests_py/benchmarks -k locomo # subset diff --git a/README.md b/README.md index 8d14c79b..ecae4197 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ CI License: MIT Python 3.10+ - 6588 tests passing + 6609 tests passing 97 referenced papers Version 4.16.0 OpenSSF Best Practices @@ -526,7 +526,7 @@ Cortex is **local-first**: your memories, conversations, and profiles stay on yo ## Development ```bash -pytest # 6588 tests +pytest # 6609 tests ruff check . # Lint ruff format --check . # Format python scripts/check_doc_claims.py # advertised counts must match the repo diff --git a/assets/badge-tests.svg b/assets/badge-tests.svg index 279dbc99..e17f44b0 100644 --- a/assets/badge-tests.svg +++ b/assets/badge-tests.svg @@ -1,5 +1,5 @@ - - 6588 tests passing + + 6609 tests passing