Skip to content

fix(sqlite): register explicit datetime adapter, close deprecation warning - #266

Open
cdeust wants to merge 4 commits into
mainfrom
fix-sqlite-datetime-adapter-260
Open

fix(sqlite): register explicit datetime adapter, close deprecation warning#266
cdeust wants to merge 4 commits into
mainfrom
fix-sqlite-datetime-adapter-260

Conversation

@cdeust

@cdeust cdeust commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

sqlite_compat.py relied on sqlite3's implicit default datetime adapter, deprecated as of Python 3.12 and firing on 3 tests (test_consolidate.py::test_with_memories, ::test_protected_memories_skip_compression, test_memory_lifecycle.py::test_store_consolidate_recall). The 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: sqlite_compat.py registers an explicit sqlite3.register_adapter(datetime, _adapt_datetime_iso) callback (the sanctioned Python-docs recipe the deprecation warning itself points to). It writes the same "T"-separated .isoformat() spelling every other datetime write path in this codebase already uses (sqlite_store._now_iso(), etc.) — a single canonical wire format instead of two. Old rows on disk (written in 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 (coding-standards.md §14)

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); sqlite_compat.py is now 232 lines. Updated the two existing tests that monkeypatched _SUPPORTS_RETURNING to target the module that actually defines it (patching the old re-exported copy would have been a silent no-op).

Running scoped mutation testing (mutmut, coding-standards.md §12) 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 per the Python docs, so it is always None after executemany() regardless — no input can distinguish the mutant from the real code.

A separate, unrelated mutation gap in _translate_sql itself (20 survivors, verbatim regex-translation code that predates this change entirely — confirmed identical source + identical tests as before the split) is outside this fix's blast radius and filed as #265 rather than folded in here, per §14.3.

Completion Ledger

Path introduced Asserting test
_adapt_datetime_iso returns tz-aware .isoformat() TestAdaptDatetimeIso::test_returns_plain_isoformat
_adapt_datetime_iso on a naive datetime TestAdaptDatetimeIso::test_naive_datetime_round_trips_through_isoformat
No DeprecationWarning via PsycopgCompatConnection.execute test_connection_execute_with_raw_datetime_param
No DeprecationWarning via cursor().execute test_cursor_execute_with_raw_datetime_param
No DeprecationWarning via cursor().executemany test_executemany_with_raw_datetime_params
Old (space-sep) and new (T-sep) spellings parse identically test_old_spelling_and_new_spelling_parse_identically
Pre-fix row on disk still reads correctly post-fix test_pre_fix_row_on_disk_still_reads_correctly
_CompatCursor.had_returning default is False test_compat_cursor_had_returning_defaults_to_false
_CompatCursor.rowcount mirrors the wrapped cursor test_compat_cursor_rowcount_mirrors_the_wrapped_cursor
_CompatCursor.fetchone synthesises the exact "id" key test_compat_cursor_synthesises_the_exact_id_key_when_flagged
fetchone requires flag AND lastrowid (not OR) test_compat_cursor_needs_both_flag_and_lastrowid_to_synthesise
executemany clears _had_returning, reports real rowcount test_executemany_clears_had_returning_and_reports_real_rowcount
PsycopgCompatConnection.execute computes had_returning from the real SQL test_connection_execute_computes_had_returning_from_the_actual_sql
executescript runs every statement verbatim test_executescript_runs_every_statement_verbatim
enable_load_extension forwards the flag verbatim test_enable_load_extension_forwards_the_flag_verbatim
filterwarnings turns the deprecation into an error proven by -W error::DeprecationWarning full-suite run (green)

Test plan

  • Reproduced the bug (instrumented execute/executemany, confirmed cascade.py::_update_stage_entered is the sole raw-datetime call site via a full-suite run)
  • tests_py/infrastructure/test_sqlite_datetime_adapter_260.py (7 tests) — fails on pre-fix code (verified: ImportError on _adapt_datetime_iso, and the DeprecationWarning reproduces standalone against the old code)
  • tests_py/handlers/test_wiki_pipeline_sqlite.py (+12 tests, 2 monkeypatch retargets)
  • Scoped mutation testing: sqlite_compat.py → 0 unaccounted survivors (1 documented equivalent); sqlite_sql_translate.py → 20 pre-existing survivors filed as sqlite_sql_translate.py: 20 surviving mutants in _translate_sql / _returning_was_stripped #265
  • ruff check + ruff format --check on all touched files
  • pyright on both touched production modules — 0 errors
  • Full suite (SQLite backend): 6373 passed, 81 skipped, 0 warnings (up from 6352 baseline + 21 new tests)
  • tests_py/handlers/test_consolidate.py + tests_py/integration/test_memory_lifecycle.py green under -W error::DeprecationWarning

Closes #260

🤖 Generated with Claude Code

https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u

cdeust and others added 4 commits July 30, 2026 03:01
…rning

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
…-count docs

Merges origin/main (3 commits ahead: #250 json_native mutants, #252
temporal timezone fix, #253 tree-sitter-language-pack pin) to pick up
its own doc-count bumps before recomputing the canonical figure, per
the standing rule that count-carrying doc hunks resolve on merits (main's
current figure + this branch's own prose), never a wholesale side pick.

Root-caused the Docker Smoke failure (run 30485406920): the container
answered `initialize` (id=1) then exited ~4s later without ever
answering `tools/list` (id=3), no protocol error either. Reproduced 0/5
locally (idle machine) but recurs on loaded CI runners, including on an
unrelated PR (#249, run 30495787124) with the identical signature —
confirming it is not a regression in this PR's sqlite_compat.py changes.
Read mcp/server/stdio.py: stdin_reader's `async for line in stdin` loop
closes its channel writer the instant stdin reaches EOF, and nothing in
the SDK's session lifecycle blocks that shutdown on an in-flight request
handler — `tools/list` does strictly more work than `initialize` (tool
registry enumeration, upstream-availability probes) so it is the one
that loses the race under load. Root-cause fix: drive the container's
stdin through a FIFO instead of a pipe from a process that exits
immediately, and hold the write end open — polling stdout for the
response (or a bounded ~55s wait) — before sending the real EOF. This
removes the race instead of retrying past it.

Recomputed the canonical test count on the merged tree (6564, measured
via `pytest --collect-only -q`) and refreshed every doc site
check_doc_claims.py enforces (README, CONTRIBUTING, CLAUDE.md,
ASSURANCE-CASE, .bestpractices.json) plus the self-hosted test-count
badge (regenerated via scripts/generate_repo_badges.py, not hand-edited).
Verified: `check_doc_claims.py --test-count 6564` and
`generate_repo_badges.py --check --test-count 6564` both pass; the
merged tree's own test subset (sqlite datetime adapter, wiki pipeline,
temporal, json_native, typecheck-parity, infrastructure + handlers —
1967 tests) is green; ruff check/format clean; shellcheck clean on the
modified script.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
… fix

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
Rebasing fix-sqlite-datetime-adapter-260 onto origin/main (56f2f4f) picked
up 4 more commits than the 6564-test figure this branch's own merge commit
(1d4cc3b) had recomputed. Re-measured on the fully rebased tree via
`pytest --collect-only -q`: 6565 tests, 0 collection errors. Verified with
`scripts/check_doc_claims.py --test-count 6565` (pass) and
`scripts/generate_repo_badges.py --test-count 6565` (regenerates
assets/badge-tests.svg to match; `--check` confirms no drift after).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
@cdeust
cdeust force-pushed the fix-sqlite-datetime-adapter-260 branch from 60cee39 to 3c18dab Compare July 30, 2026 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite_compat.py relies on the deprecated stdlib datetime adapter (3 DeprecationWarnings in the suite)

1 participant