bug: BackendOrchestrator uses DatabaseManager "conn" attribute which doesn't exist, silently breaking schema v3.4.5 migration on every boot

[barrygfox]

Summary

BackendOrchestrator accesses self._db.conn in two methods, but DatabaseManager has no .conn attribute — it uses per-call connections. So:

1. The v3.4.5 schema migration never runs → the access_count_30d column is never added to atomic_facts resulting in silently disabling 30-day recency scoring (F-14).
2. The _update_status() method then silently fails on every call and the backend_status table is never updated.

Both failures are swallowed by except Exception: pass / logger.warning(...) blocks, so the daemon starts successfully but operates in a degraded state with no visible indication beyond a single non-fatal warning in the log.

Reproduction

# Minimal repro — no daemon needed
import sqlite3
from superlocalmemory.storage.database import DatabaseManager
from superlocalmemory.core.backend_orchestrator import BackendOrchestrator

db = DatabaseManager(":memory:")
# DatabaseManager has no .conn attribute:
assert not hasattr(db, "conn")  # True — AttributeError follows if accessed

# In production the orchestrator catches and logs this on every boot:
# WARNING - Schema v3.4.5 apply failed (non-fatal): 'DatabaseManager' object
#           has no attribute 'conn'

Observed in daemon log on every startup since at least 3.4.5:

Schema v3.4.5 apply failed (non-fatal): 'DatabaseManager' object has no attribute 'conn'

Proposed root cause

DatabaseManager is documented as using per-call connections ("Per-call connections: no shared state between processes") and intentionally exposes no persistent connection. The private _connect() method opens a fresh connection, returns it, and callers are expected to close it.

backend_orchestrator.py:359–371 — _apply_schema_v345():

if not schema_version_applied(self._db.conn):   # AttributeError here
    result = apply_migration(self._db.conn)      # and here

backend_orchestrator.py:341–353 — _update_status():

self._db.conn.execute("INSERT OR REPLACE INTO backend_status ...")  # AttributeError here
self._db.conn.commit()                                               # and here

Why tests don't catch it

tests/core/test_orchestrator.py uses a MockDB that provides a .conn attribute:

class MockDB:
    def __init__(self):
        self.conn = sqlite3.connect(":memory:")  # masks the production bug

The tests pass because MockDB.conn exists whereasDatabaseManager.conn does not.

Potential fix

_apply_schema_v345() — open and close a dedicated connection:

def _apply_schema_v345(self) -> None:
    try:
        from superlocalmemory.storage.schema_v345 import (
            apply_migration, schema_version_applied,
        )
        conn = self._db._connect()
        try:
            if not schema_version_applied(conn):
                result = apply_migration(conn)
                conn.commit()
                if result.get("errors"):
                    logger.warning("Schema v3.4.5 had errors: %s", result["errors"])
        finally:
            conn.close()
    except ImportError:
        logger.debug("schema_v345 not found — skipping")
    except Exception as exc:
        logger.warning("Schema v3.4.5 apply failed (non-fatal): %s", exc)

_update_status() — use the public execute() method (handles connection lifecycle internally):

def _update_status(self, name: str, status: str,
                    count: int = 0, error: str = "") -> None:
    self._backend_cache[name] = status
    try:
        self._db.execute(
            "INSERT OR REPLACE INTO backend_status "
            "(backend_name, status, record_count, error_message, last_sync_at) "
            "VALUES (?, ?, ?, ?, datetime('now'))",
            (name, status, count, error),
        )
    except Exception:
        pass

Suggested test additions

def test_apply_schema_v345_with_real_database_manager(tmp_path):
    """Schema migration must succeed when using a real DatabaseManager."""
    from superlocalmemory.storage.database import DatabaseManager
    import sqlite3

    db_path = tmp_path / "test.db"
    db = DatabaseManager(db_path)
    # Ensure backend_status table exists (normally created by engine init)
    conn = db._connect()
    conn.execute("""
        CREATE TABLE IF NOT EXISTS backend_status (
            backend_name TEXT PRIMARY KEY,
            status TEXT DEFAULT 'not_initialized',
            record_count INTEGER DEFAULT 0,
            last_sync_at TEXT,
            error_message TEXT DEFAULT ''
        )
    """)
    conn.execute("""
        CREATE TABLE IF NOT EXISTS atomic_facts (
            fact_id TEXT PRIMARY KEY,
            profile_id TEXT,
            content TEXT,
            created_at TEXT
        )
    """)
    conn.commit()
    conn.close()

    orch = BackendOrchestrator(db=db, config=MockConfig())
    # Must not raise and must not log a warning — the migration should apply cleanly
    orch._apply_schema_v345()

    # Verify the column was actually added
    check_conn = db._connect()
    row = check_conn.execute(
        "SELECT 1 FROM pragma_table_info('atomic_facts') WHERE name='access_count_30d'"
    ).fetchone()
    check_conn.close()
    assert row is not None, "access_count_30d column was not created"


def test_update_status_with_real_database_manager(tmp_path):
    """_update_status must persist to DB when using a real DatabaseManager."""
    from superlocalmemory.storage.database import DatabaseManager

    db_path = tmp_path / "test.db"
    db = DatabaseManager(db_path)
    conn = db._connect()
    conn.execute("""
        CREATE TABLE IF NOT EXISTS backend_status (
            backend_name TEXT PRIMARY KEY,
            status TEXT DEFAULT 'not_initialized',
            record_count INTEGER DEFAULT 0,
            last_sync_at TEXT,
            error_message TEXT DEFAULT ''
        )
    """)
    conn.commit()
    conn.close()

    orch = BackendOrchestrator(db=db, config=MockConfig())
    orch._update_status("cozo", "active", count=42)

    rows = db.execute("SELECT status, record_count FROM backend_status WHERE backend_name = 'cozo'")
    assert len(rows) == 1
    assert dict(rows[0])["status"] == "active"
    assert dict(rows[0])["record_count"] == 42

Note: Additional affected call site

tier_manager.py line 447 has the same pattern (db.conn passed to bulk_update_tiers_from_sqlite). Not included in this issue, but maybe worth fixing in the same pass.

Environment

• SLM version: 3.6.13 (also present in current main)
• Python: 3.14
• Platform: macOS Darwin 25.5.0

[barrygfox]

Fixes for the existing tests

The issue suggests that MockDB in tests/core/test_orchestrator.py masks the reported bug by providing a .conn attribute. The suggested test additions are new standalone tests, but the existing tests (test_daemon_start_no_backends, test_status_tracking) should also catch this — they just need their fixture updated.

Replacing MockDB with a real DatabaseManager backed by tmp_path would make those tests fail before the fix and pass after it, turning them into regression tests at no extra cost:

# tests/core/test_orchestrator.py

import pytest
from pathlib import Path
from superlocalmemory.storage.database import DatabaseManager
import superlocalmemory.storage.schema as schema_module  # adjust import to actual schema module

@pytest.fixture()
def db(tmp_path: Path) -> DatabaseManager:
    mgr = DatabaseManager(tmp_path / "orchestrator_test.db")
    mgr.initialize(schema_module)  # creates backend_status, atomic_facts, etc.
    return mgr

# Then update each test to accept db as a fixture argument instead of MockDB()

This is the same pattern already used in tests/test_storage/test_database.py, so it follows existing project conventions. The MockDB class can be removed entirely once no tests depend on it.

The key benefit: test_daemon_start_no_backends(db) would have raised AttributeError: 'DatabaseManager' object has no attribute 'conn' as soon as _apply_schema_v345() was introduced, surfacing this before it reached a release.

[barrygfox]

Missing impact: graph edge indexes also never created

The original report focused on access_count_30d, but schema_v345.apply_migration() also creates two indexes that were silently missing:

CREATE INDEX IF NOT EXISTS idx_graph_edges_source_id ON graph_edges(profile_id, source_id);
CREATE INDEX IF NOT EXISTS idx_graph_edges_target_id ON graph_edges(profile_id, target_id);

These indexes back bulk import performance (F-20 per the migration docstring). Without them, any operation that scans graph_edges by source_id or target_id does a full table scan. On larger graphs this compounds with the SpreadingActivation query load.

Confirmed via post-fix verification — both indexes are now present after applying the _connect() fix:

idx_graph_edges_source_id  ← was missing before fix
idx_graph_edges_target_id  ← was missing before fix

Any test cases written for the v345 migration should assert both effects — the column and the indexes. The index assertion would look like:

for idx in ("idx_graph_edges_source_id", "idx_graph_edges_target_id"):
    row = check_conn.execute(
        "SELECT 1 FROM sqlite_master WHERE type='index' AND name=?", (idx,)
    ).fetchone()
    assert row is not None, f"{idx} index was not created"