From 0a164062b01968c1499f27856c5f0bd9f25dc3e6 Mon Sep 17 00:00:00 2001 From: cdeust Date: Thu, 30 Jul 2026 01:57:24 +0200 Subject: [PATCH] docs(changelog): add issue #260 entry for the sqlite datetime adapter fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The datetime-adapter fix (82844f3) shipped without a CHANGELOG entry, which coding-standards.md §13.1 H4 requires for any consumer-observable behavior change (here: the deprecation warning disappears and the wire format is documented). Adds the entry in the same style/detail as its neighbors, cross-referencing #260 and #265. Refs #260 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 083aa1e5..99f3d773 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ adheres to [Semantic Versioning](https://semver.org/). ### 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. - **`mcp-toplist-badge.yml`'s monthly refresh can now actually open its PR** (#273). A real `workflow_dispatch` run (triggered while dispatch-verifying #246) reached `Open refresh PR` and failed there: `GitHub Actions is not permitted to create or approve pull requests`. The repo had **"Allow GitHub Actions to create and approve pull requests"** unchecked at Settings → Actions → General, which blocks PR *creation* itself — a stronger failure than the one the workflow's own comment anticipated ("GitHub deliberately does not trigger workflows on `GITHUB_TOKEN`-authored PRs", which only explains why such a PR's checks don't start, not why it would fail to be created at all). Fixed at the repo-policy layer (`gh api -X PUT repos/cdeust/Cortex/actions/permissions/workflow -F can_approve_pull_request_reviews=true`), which is where the root cause lives — not in the workflow, which already had the correct `secrets.BADGE_REFRESH_TOKEN || secrets.GITHUB_TOKEN` fallback and needed no logic change. The workflow's comment now documents both distinct failure modes and the fix, so a future repo transfer or org policy reset that reintroduces this is diagnosable from the file alone. A stray `chore/mcp-toplist-badge-refresh` branch pushed-then-abandoned by the earlier failing run had already been deleted (confirmed absent by this fix); the verification run for this fix leaves no stray branch either — its outcome is quoted in PR #273's description. +- **`sqlite_compat.py` relied on sqlite3's *implicit* default `datetime` adapter, deprecated as of Python 3.12** (#260), firing on 3 tests (`test_consolidate.py::test_with_memories`, `::test_protected_memories_skip_compression`, `test_memory_lifecycle.py::test_store_consolidate_recall`). Root cause: `cascade.py::_update_stage_entered` binds a raw `datetime.datetime` object as a SQL parameter instead of an ISO string — confirmed the **sole** such call site in this codebase by instrumenting all three `execute`/`executemany` paths in `sqlite_compat.py` and running the full suite against it. Fix: an explicit `sqlite3.register_adapter(datetime, _adapt_datetime_iso)` (the sanctioned Python-docs recipe the deprecation warning itself points to), writing the same "T"-separated `.isoformat()` spelling every other datetime write path here already produces (`sqlite_store._now_iso()`, etc.) — one canonical wire format instead of two. Old rows on disk (the deprecated adapter's space-separated spelling) keep reading correctly: `datetime.fromisoformat()` — the read path every consumer here uses — parses both spellings to an identical value (verified empirically and pinned by a test), so **no migration is required**. A `pyproject.toml` `filterwarnings` entry turns this specific DeprecationWarning into a hard failure going forward — a regression tripwire, not a silence. Boy-scout: `sqlite_compat.py` was already 335 lines — over this repo's 300-line file cap — before this change touched it; split the pure SQL-dialect translation logic (`_translate_sql`/`_returning_was_stripped`/`_SUPPORTS_RETURNING`) into a new `sqlite_sql_translate.py` module (behaviour-preserving, byte-identical logic), retargeting the two existing tests that monkeypatched `_SUPPORTS_RETURNING` to the module that actually defines it. Scoped mutation testing (mutmut) against the touched `sqlite_compat.py` surfaced 13 pre-existing gaps in `_CompatCursor`/`_CompatExecutingCursor`/`PsycopgCompatConnection` field wiring (not the datetime fix itself) — added targeted tests for all of them; one mutant (`executemany`'s `self.lastrowid = None`) is a documented equivalent (`sqlite3.Cursor.lastrowid` is only meaningful after a single-row `execute()` INSERT, so it is always `None` after `executemany()` regardless of which literal is written). A separate mutation gap in `_translate_sql` itself (20 survivors, verbatim regex-translation code moved unchanged from `sqlite_compat.py`, same tests before/after) predates this change and is filed as #265 rather than folded in, per §14.3. - **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.