Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down