fix(sqlite): backend parity — full-suite SQLite gate, and the silent wiki-pipeline failure it caught (Refs #220) - #231
Merged
Merged
Conversation
cdeust
force-pushed
the
fix/220-sqlite-full-parity
branch
from
July 28, 2026 10:20
ead5f47 to
0d9206b
Compare
cdeust
force-pushed
the
fix/220-sqlite-full-parity
branch
from
July 28, 2026 15:15
ac2f67b to
2cd592f
Compare
The SQLite suite failed 35 on main. It now passes 5639/5639, and PostgreSQL still passes 5723/5723. Three were real product bugs on the plugin's DEFAULT backend, not test noise: 1. `SqliteMemoryStore` had no `acquire_interactive`/`acquire_batch`. `anchor.py:141` and `get_rules.py:97` call them unconditionally, so a SQLite install raised AttributeError — an LSP break, since a handler cannot know which store it was handed. SQLite owns one WAL connection and must not open a competing one, so both yield `self._conn`: exactly the shape PgMemoryStore yields under POOL_DISABLED. (7 tests) 2. `handlers/lesson_promotion.py` called the PG-only `list_lesson_promotion_candidates` unconditionally -> `unrecognized token: "@"` on SQLite. Added the SQLite dialect twin (`substr` for `LEFT`, same eligibility/ordering/preview) and dispatched on store type, mirroring the convention already in `get_grooming_health.py`. 3. That failure was INVISIBLE: a bare `except Exception: candidates = []` turned it into an empty backlog. The handler reported "no candidates" on every SQLite call. The fallback stays (callers rely on the empty-list contract) but now logs with exc_info — §13.1 F1, and the same silent-degradation class as the FlashRank incident. The rest were harness defects, fixed rather than skipped: - `test_get_grooming_health` asserted backend-agnostic contracts (count == unbounded list length) while importing the PG dialect directly and resolving the store via `get_shared_store()`. It now goes through the handlers' dispatch, so it also covers the composition root. - 4 modules seed PostgreSQL directly (raw DSN / PG-only migrations) and assert against the resolved store; under a sqlite run they seeded one store and read another. Re-gated on `_USE_PG_STORE` (effective backend) instead of `_USE_PG` (reachability) — their subject IS the PG path. Backend-agnostic fixtures for these remain open under #220. - `test_I2_canonical_writer` pins heat writers by LINE NUMBER; the import added above shifted three by +2. Same writers, verified by reading the three lines; pins updated as the assertion message instructs. Verification (full suites, both backends, exit 0): SQLITE_EXIT=0 5639 passed, 96 skipped PG_EXIT=0 5723 passed, 12 skipped Real ~/.claude store checksummed before/after every run: untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013SQwMscnDJfTN7sz3Jwzg4
…#220) Three things #220 asked for that were still open after the parity fixes. 1. CI actually gates SQLite now. The job ran ONE file (test_sqlite_backend.py) behind a comment deferring the rest to a "full-parity effort" that named no issue. That is precisely how the three defects fixed in 17be6b5 reached the plugin's DEFAULT backend without CI noticing. It now runs the full suite, with CORTEX_MEMORY_STORE_BACKEND=sqlite set explicitly rather than inferred from the absence of a PG service container, so a future runner that happens to have PostgreSQL reachable cannot silently turn this into a second PostgreSQL run. 2. conftest.py was 606 lines against a 500-line cap (§4.1) — a defect in material this work touched, so it is fixed here rather than noted. Between-test cleanup and singleton reset moved verbatim to tests_py/_store_cleanup.py (conftest 437, new module 196). The one behavioural-adjacent change is the SQLite path binding: the moved code reads CORTEX_MEMORY_DB_PATH instead of conftest's _ISOLATED_SQLITE_PATH global, which removes an import cycle. It is the same value, and the import sits BELOW _redirect_real_data_roots() because that call is what sets the variable — the #219 ordering constraint, restated at the site. 3. test_I2_canonical_writer pins heat writers by LINE NUMBER and broke twice in this session from edits that only shifted lines (an import, then a docstring). Pins realigned to 530/560/624, each verified by reading the line. The fragility is real and worth replacing with content-based anchors; that is a separate change, not a silent deferral. Verification (full suites, both backends): SQLITE_EXIT=0 5639 passed, 96 skipped PG_EXIT=0 5723 passed, 12 skipped ruff check + format clean (931 files); doc claims OK at 5735 real ~/.claude store checksummed before/after: untouched Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013SQwMscnDJfTN7sz3Jwzg4
Broadening the SQLite CI job to the full suite (db1dcaa) immediately exposed that the job would fail. Simulating the REAL CI condition — SQLite backend with PostgreSQL UNREACHABLE, which is not what the local runs exercised because PG is up on this machine — gave 2 failed and 27 errors. Cause: four modules construct `PgMemoryStore()` directly with no skip gate at all. tests_py/infrastructure/test_pg_pool.py tests_py/infrastructure/test_pg_recall_scoring_debias.py tests_py/infrastructure/test_pg_user_mood.py tests_py/invariants/test_I10_pool_capacity.py They error on any PG-less run. That was invisible while the SQLite job ran a single file and became a hard failure the moment it ran the suite. Each is now `skipif(not _USE_PG)` — reachability, which is the right question for a test whose subject IS the PostgreSQL backend (contrast with the four suites re-gated on `_USE_PG_STORE` in 17be6b5, whose subject is the resolved store). Found the way it should have been found the first time: by measuring the target environment instead of assuming the local one resembles it. Verification — all three configurations, full suite: SQLite, PG unreachable (the CI job) exit 0 5980 passed, 219 skipped SQLite, PG reachable exit 0 6103 passed, 96 skipped PostgreSQL exit 0 6187 passed, 12 skipped ruff check + format clean; doc claims OK at 6199 collected real ~/.claude store checksummed before/after every run: untouched Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013SQwMscnDJfTN7sz3Jwzg4
…rom PG (#220) Broadening the SQLite CI job to the full suite exposed a harness defect that would have failed the job outright: 18 test modules import `psycopg` at module scope, directly or via `mcp_server.infrastructure.pg_store`. `psycopg` ships in the optional `[postgresql]` extra, which that job deliberately does NOT install — proving the SQLite-default install works is the job's whole point. A missing import at collection time is an ERROR, not a skip, so the run aborted. Measured in a psycopg-less venv before the fix: 18 errors during collection — pytest exits non-zero, 0 tests run. Each module now calls `pytest.importorskip("psycopg", ...)` BEFORE the import that needs it, turning the abort into a visible, reasoned SKIP. Two modules (test_staging_resolve_sink, test_wiki_citation_seed_pass) import psycopg directly ABOVE `import pytest`, so their guard sits above those imports rather than merely above the mcp_server ones — placing it by the mcp_server import alone left them failing. Verified stepwise: 18 -> 2 -> 0 collection errors, 6081 tests collected. test_recall_include_related is the one module of the 18 that did NOT get a guard, because it should never have needed one. The relation-walk it covers is a backend-agnostic FEATURE, but the file constructed `PgMemoryStore()` directly and skipped whenever the effective backend was not PostgreSQL — so recall's one-hop enrichment was never exercised on the plugin's DEFAULT backend. It now seeds through `get_shared_store()` (the same instance the handler reads) and drops its hand-rolled `%s` teardown, which conftest's autouse cleanup already performs on both backends. Verified paired: SQLite 3 passed PostgreSQL 3 passed Production was checked, not assumed: only `pg_store.py` imports psycopg at module scope and it is reached lazily through backend selection. A psycopg-less SQLite install opens the store, inserts and reads back (smoke-tested). Boy-scout (§14) — three blocking craftsmanship violations pre-existing in files this diff touches, fixed here rather than deferred: - test_pg_effective_stage_parity: 5-deep loop nest (§4.5 caps at 3) -> itertools.product over the same four axes, identical cases. - test_recall_e2e: a 62-line test (§4.2 caps at 50) -> the three inserts extracted to `_seed_ranked_trio`, values unchanged. Both re-verified on PostgreSQL: 12 passed. CONTRIBUTING now tells contributors to install every extra CI installs, with the measured skip counts (12 tests skip locally without them), so a local green run is the stricter one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RahDoFS4HWmqPzYghhYFar
The wiki pipeline produced NOTHING on every SQLite install — the plugin, `.mcpb`, and Cowork default — and said so only in a stage log. Root cause: shared query modules take a `conn` that may be a psycopg connection or a PsycopgCompatConnection, but imported psycopg's `dict_row` at module top level. psycopg ships in the optional [postgresql] extra, so on a SQLite-default install that import raises ModuleNotFoundError. `wiki_pipeline` wraps each stage in try/except, so the error surfaced as a logged stage failure with zero output rather than a crash — the same silent-degradation class as the FlashRank incident. Fixed at the root, not the throw site: `infrastructure/row_factory.py` resolves DICT_ROW once at import and the 17 shared query modules use it. On PostgreSQL it IS `dict_row`. On SQLite it is None, which is equivalent rather than a downgrade — PsycopgCompatConnection.cursor() accepts row_factory for signature parity and ignores it, because that cursor already returns dict rows unconditionally (#206). Real `psycopg.connect(row_factory=...)` call sites are deliberately untouched: psycopg is present by construction there. This was reachable only because the SQLite CI job now runs the full suite (bc5797d). While it ran one file, the defect was invisible. Verification: - both arms of the fallback proven by import test: with psycopg absent (blocked via meta_path) DICT_ROW is None; with it present DICT_ROW is psycopg's dict_row - prior session, genuinely psycopg-less venv, full suite: 5987 passed, 111 skipped, exit 0 - ruff check + format clean (951 files) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013SQwMscnDJfTN7sz3Jwzg4
…E501 debt The install block in CONTRIBUTING.md reports how many tests SKIP when the optional extras are missing — a true, dated measurement (#220). TEST_CLAIM matches any "N tests" phrase, so the doc-claim gate read it as an advertisement of the suite size and failed the 3.12 leg with: CONTRIBUTING.md:36: advertises 12 tests, canonical is 6268 Reproduced on this branch before the fix, exit 1. The line now declares [not-a-count-claim: tests], the exemption mechanism landed in #236. The prose keeps its number, its date and its breakdown: rewording a real measurement to keep a gate quiet hides the measurement instead of fixing the gate. The marker fails closed, and proved it — a first attempt split it across two source lines and exempted nothing, because NOT_A_CLAIM is matched per line. It has to sit on the same line as the number it disclaims. `test_the_declared_exemptions_are_the_reviewed_set` pinned the registry to the empty set, so this exemption could not be added silently; it now names CONTRIBUTING.md's entry and says why that number is not the suite size. Rebased onto main (04ae8be). Two conflicts, both resolved on the merits: - handlers/lesson_promotion.py — main's BLE001 sweep and this branch's SQLite dispatch both rewrote the same except arm. Kept the backend dispatch AND main's `silent_failure.note`, since dropping either loses a signal; the log additionally names the backend, which is the one fact distinguishing the dialect bug from a genuinely empty store. - invariants/test_I2_canonical_writer.py — a line-number allowlist neither side could be right about post-rebase. Re-pinned with the test as its own oracle (the documented convention), twice: once for the rebase, once after the docstring rewrap below moved the sites another line down. Boy-scout (§14): the 3 E501 violations this branch carries against main's now-blocking gate are fixed here, not deferred — sqlite_store.py's module docstring, and two `@pytest.mark.skipif( # ...` decorator comments whose text is now a coherent block above the decorator rather than split across it. Verified: ruff 0.15.20 format+check clean; doc gate green with the registry printing its first real member; 6268 collected (unchanged); the touched suites pass. Two pre-existing failures in tests_py/handlers/test_ingest_codebase.py are environmental, not from this change — a paired control with the edit stashed reproduces them identically, and the cause is a local PG lacking the migrated schema (`UndefinedTable: relation "entities" does not exist`). CI provisions PG and those legs passed on this branch's prior run. Refs #220 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RahDoFS4HWmqPzYghhYFar
…endent
`tests_py/handlers/test_ingest_codebase.py` passed in the full suite and
failed when run on its own:
psycopg.errors.UndefinedTable: relation "entities" does not exist
`_INGEST_MIGRATIONS[0]` is `CREATE INDEX ... ON entities (LOWER(name))` — an
INCREMENTAL migration that presupposes the base tables. The suite gate is
`_USE_PG_STORE`, which says PostgreSQL is the SELECTED backend, not that this
database has been migrated. conftest creates a FRESH database per session, so
whether the base schema exists when this file runs depends on whether some
earlier test happened to provision it. Running the file alone, nothing had —
order dependence (§13.1 G3), not an environment quirk.
`_ensure_base_schema()` now provisions it through the product's own path:
constructing `PgMemoryStore` IS the migration (`__init__` -> `_init_schema()`
-> `pg_schema.get_all_ddl()` under an advisory lock), which `mcp_server/
migrate.py` calls the single source of truth. The harness therefore cannot
drift from the shipped schema, and no DDL is hand-rolled here.
It is GUARDED on the missing precondition rather than run unconditionally,
because applying the DDL mid-suite is not free. Measured on this commit with
a paired control:
unconditional 6258 passed, 9 skipped, 1 failed
(test_near_dup_calibration_pass::TestRunNearDupApplyPass::
test_pairs_above_threshold_collapse_onto_hottest_member)
control 6259 passed, 9 skipped, 0 failed
guarded 6259 passed, 9 skipped, 0 failed
So the guarded form repairs the unprovisioned case and leaves the provisioned
one — CI, and any migrated developer DB — untouched.
Repair path verified directly rather than assumed: with `entities` dropped to
recreate the unprovisioned state, the file passes 18/18 on its own, and the
two tests PASS rather than skip, so this restores coverage instead of hiding
the gap by widening `skipif`.
Refs #220
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RahDoFS4HWmqPzYghhYFar
cdeust
force-pushed
the
fix/220-sqlite-full-parity
branch
from
July 28, 2026 15:24
2cd592f to
f243061
Compare
Broadening the SQLite CI job to the full suite (this PR's thesis) surfaced a
latent import-time break on the plugin's DEFAULT backend. Reproduced in a
genuinely psycopg-less venv matching the SQLite job's
`pip install -e ".[dev,sqlite,codebase]"`:
21 errors, INTERNALERROR> ModuleNotFoundError: No module named 'psycopg'
mcp_server/hooks/session_lifecycle.py:44
-> handlers/injection_receipts.py:16
-> infrastructure/pg_store_receipts.py:20
-> infrastructure/pg_store_host.py:27 import psycopg
`pg_store_host` is reachable by import from the SQLite-default install — the
backend every plugin / `.mcpb` / Cowork launch uses (PRIVACY.md lines 26-38) —
but imported psycopg at module scope. Only `ProgrammingError` is needed at
RUNTIME there; every other psycopg reference is an annotation, stringified by
`from __future__ import annotations`. The import now degrades like
`row_factory.DICT_ROW` does: psycopg's class when the extra is installed, a
private stand-in otherwise. The PG-only paths that raise it are unreachable
without psycopg, so the stand-in is never matched.
Fixed at the source rather than upstream: an earlier attempt deferred the
import in `injection_receipts` instead, which only moved the failure to call
time — `test_injection_receipts_emitter` drives that path with a fake
connection and went red. Breaking the chain treats the symptom; the module
should simply be importable.
Also guards `tests_py/infrastructure/test_sqlite_parity_197.py`, which imports
`pg_store_host` at module scope. A missing import at collection is an ERROR,
not a skip, so it aborted the entire run; it now carries the same
`pytest.importorskip("psycopg", ...)` guard as the other PG-touching suites.
Verified in the psycopg-less venv: 21 errors -> 1 -> 2 -> **6067 passed, 112
skipped, 0 failed**. pyright 0 errors on a fully-resolved environment; ruff
0.15.20 clean.
Refs #220
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RahDoFS4HWmqPzYghhYFar
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #220. Not
Closes— 207 skips remain on the SQLite backend, andthose are unfinished work, not a passing suite. See What is still open.
Why this exists
CI's SQLite job ran exactly one file behind a comment deferring the rest to a
"full-parity effort" that named no issue. SQLite is the DEFAULT backend for
plugin installs,
.mcpb, and Cowork. Turning that job into a full-suite runis what surfaced everything below.
The user-facing bug this caught
On every SQLite install the wiki pipeline produced nothing at all, and
said so only in a stage log line.
Shared query modules take a
connthat may be psycopg orPsycopgCompatConnection, but imported psycopg'sdict_rowat module toplevel. psycopg ships in the optional
[postgresql]extra, so on aSQLite-default install that import raised
ModuleNotFoundError;wiki_pipelinewraps each stage in try/except, so it degraded to zero outputinstead of crashing. Same silent-degradation class as the FlashRank incident.
Fixed at the root:
infrastructure/row_factory.pyresolvesDICT_ROWonce atimport; 17 shared query modules use it. On PostgreSQL it IS
dict_row; onSQLite it is
None, which is equivalent rather than a downgrade, becausePsycopgCompatConnection.cursor()acceptsrow_factoryfor signature parityand ignores it — that cursor already returns dict rows (#206). Real
psycopg.connect(row_factory=...)sites are deliberately untouched: psycopgis present by construction there.
Other real defects on the default backend
SqliteMemoryStorehad noacquire_interactive/acquire_batch, whichanchor.pyandget_rules.pycall unconditionally — an LSP break, since ahandler cannot know which store it was handed.
handlers/lesson_promotion.pycalled the PG-only candidate queryunconditionally →
unrecognized token: "@"on SQLite.except Exception: candidates = []reported it as an empty backlog. The fallback stays; it now logs with
exc_info.Test and infrastructure fixes
PgMemoryStore()with no gate at all and errored onany PG-less run — harmless while the job ran one file, a hard failure once
it ran the suite.
test_get_grooming_healthasserted backend-agnostic contracts whileimporting the PG dialect directly; it now goes through the handler dispatch.
conftest.pywas 606 lines against the 500-line cap; cleanup and singletonreset moved verbatim to
tests_py/_store_cleanup.py(437 / 196).CONTRIBUTING.mdnow names every extra CI installs — missing extras SKIPrather than fail, so a local run can look green while covering less.
Verification
DICT_ROW(import blocked viameta_path)None— fallback engagesdict_rowruff check+ruff formatclean (951 files). The real~/.claudestore waschecksummed before and after every run and never mutated.
What is still open (Refs, not Closes)
207 SQLite skips, in two populations needing opposite fixes:
SQLite — supersession read-path, entity merge, spread-activation scoping,
recall e2e. Fix: move fixtures to
get_shared_store()so they run on both.backfill passes, near-dup calibration. Skipping these hides that a SQLite
install cannot do them at all. This is the substantive parity gap.
Also open:
test_I2_canonical_writerpins heat writers by LINE NUMBER andshifted three times during this work from edits that only moved lines; it
should be content-anchored. And #223 (two
sentence_transformersSWIGDeprecationWarnings, external to this repo).
🤖 Generated with Claude Code
https://claude.ai/code/session_013SQwMscnDJfTN7sz3Jwzg4