fix(db): make insert_documentation an upsert (closes #48)

CHANGELOG.md

@@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- `db.insert_documentation` (called by `seed_data.seed_one` and the scraper's
+  auto-seed step) is now an upsert. Re-seeding the same corpus name no longer
+  raises `sqlite3.IntegrityError` against `documentations.name UNIQUE`. The
+  documentations row is upserted via `INSERT ... ON CONFLICT(name) DO UPDATE
+  ... RETURNING id`, which keeps the original `created_at` and the existing
+  `doc_id` stable across refreshes. Sections, their `query_cache` entries
+  (via `ON DELETE CASCADE`), and their `sections_fts` index entries (via the
+  FTS5 'delete' protocol) are then cleared and the fresh sections inserted
+  in the same SQLite transaction. Embedding writes to `data/embeddings.npy`
+  and `data/_internal/section_mapping.json` are deferred until after the
+  commit, so a rollback never leaves the numpy sidecar ahead of the
+  committed DB state. Closes #48.
+- `db._get_connection` now executes `PRAGMA foreign_keys = ON` on every
+  connection. Without this, `ON DELETE CASCADE` on `sections.doc_id` and
+  `query_cache.section_id` was a silent no-op for everything outside
+  `init_db`, leaving orphan rows after deletes. The upsert above relies on
+  the cascade, but every other delete path benefits too.
+
 ### Added
 
 - Content-hash provenance through every layer of the scraper pipeline

src/king_context/db.py

@@ -458,26 +458,66 @@ def insert_documentation(doc_data: Dict[str, Any]) -> int:
     """
     conn = _get_connection()
     cursor = conn.cursor()
+    # Embeddings are written to disk by _generate_and_save_embedding and live
+    # outside the SQLite transaction. Stage them here and flush only AFTER the
+    # commit so a rolled back transaction never leaves orphan numpy rows on
+    # disk that disagree with the committed DB state.
+    embeddings_pending: list[tuple[int, str]] = []
 
     try:
-        # Get current timestamp in ISO format
         now = datetime.now().isoformat()
 
-        # Insert documentation record
-        cursor.execute("""
+        # Atomic upsert of the documentations row. ON CONFLICT preserves
+        # `created_at` (we only overwrite display_name, version, base_url,
+        # updated_at) and removes the read-modify-write race two concurrent
+        # scrapers would have hit with a SELECT-then-DELETE-then-INSERT.
+        cursor.execute(
+            """
             INSERT INTO documentations (name, display_name, version, base_url, created_at, updated_at)
             VALUES (?, ?, ?, ?, ?, ?)
-        """, (
-            doc_data["name"],
-            doc_data["display_name"],
-            doc_data.get("version"),
-            doc_data["base_url"],
-            now,
-            now
-        ))
-        doc_id = cursor.lastrowid
-
-        # Insert sections
+            ON CONFLICT(name) DO UPDATE SET
+                display_name = excluded.display_name,
+                version = excluded.version,
+                base_url = excluded.base_url,
+                updated_at = excluded.updated_at
+            RETURNING id
+            """,
+            (
+                doc_data["name"],
+                doc_data["display_name"],
+                doc_data.get("version"),
+                doc_data["base_url"],
+                now,
+                now,
+            ),
+        )
+        doc_id = cursor.fetchone()[0]
+
+        # Drop any prior sections for this doc. sections_fts is an external
+        # content table and does not auto-prune on cascade or DELETE, so each
+        # row's index entry must be removed via the FTS5 'delete' protocol
+        # before the sections rows themselves go away. Use a separate cursor
+        # so the streaming SELECT and the per-row INSERT do not contend on
+        # the same statement handle.
+        read_cursor = conn.cursor()
+        read_cursor.execute(
+            "SELECT id, title, content FROM sections WHERE doc_id = ?",
+            (doc_id,),
+        )
+        for sid, stitle, scontent in read_cursor:
+            # The `or ""` mirrors the insert path below: NULL content stored in
+            # `sections` was originally fed to FTS5 as the empty string. The
+            # 'delete' values must match what FTS5 saw on insert, byte for
+            # byte, or term frequencies drift silently.
+            cursor.execute(
+                "INSERT INTO sections_fts(sections_fts, rowid, title, content) "
+                "VALUES('delete', ?, ?, ?)",
+                (sid, stitle, scontent if scontent is not None else ""),
+            )
+        read_cursor.close()
+        cursor.execute("DELETE FROM sections WHERE doc_id = ?", (doc_id,))
+
+        # Insert new sections
         sections = doc_data.get("sections", [])
         for section in sections:
             cursor.execute("""
@@ -506,18 +546,25 @@ def insert_documentation(doc_data: Dict[str, Any]) -> int:
                 section.get("content", "")
             ))
 
-            # Generate and save embedding for section
-            _generate_and_save_embedding(section_id, section.get("content", ""))
+            embeddings_pending.append((section_id, section.get("content", "")))
 
         conn.commit()
-        return doc_id
 
     except Exception:
         conn.rollback()
         raise
     finally:
         conn.close()
 
+    # Embeddings flushed AFTER commit, so a SQLite rollback above leaves the
+    # numpy file untouched. Failures here do not corrupt the DB; at worst the
+    # corpus has fresh sections without their embedding rows, which the
+    # hybrid reranker handles gracefully (FTS-only fallback).
+    for section_id, content in embeddings_pending:
+        _generate_and_save_embedding(section_id, content)
+
+    return doc_id
+
 
 def list_documentations() -> List[Dict[str, Any]]:
     """List all documentations with their section counts.
@@ -700,5 +747,13 @@ def _escape_fts5_query(query: str) -> str:
 
 
 def _get_connection() -> sqlite3.Connection:
-    """Retorna conexão com o banco."""
-    return sqlite3.connect(DB_PATH)
+    """Open a database connection with foreign keys enforced.
+
+    SQLite requires ``PRAGMA foreign_keys = ON`` per connection; without it,
+    ``ON DELETE CASCADE`` on ``sections`` and ``query_cache`` is a silent
+    no-op. Enable it on every connection so cascade behaviour matches the
+    schema's stated contract.
+    """
+    conn = sqlite3.connect(DB_PATH)
+    conn.execute("PRAGMA foreign_keys = ON")
+    return conn

tests/conftest.py

@@ -61,9 +61,21 @@ def fake_stage_clients(primary, schema_fallback=None):
 
 @pytest.fixture
 def temp_db(tmp_path, monkeypatch):
-    """Create a temporary database path for testing."""
+    """Create a temporary database path for testing.
+
+    Also redirects the embedding sidecar files (``embeddings.npy``,
+    ``section_mapping.json``) and resets the in-process embedding state so a
+    test that triggers ``_generate_and_save_embedding`` cannot write to the
+    real repo's ``data/`` directory.
+    """
     temp_db_path = tmp_path / "test_docs.db"
     monkeypatch.setattr(db, "DB_PATH", temp_db_path)
+    monkeypatch.setattr(db, "EMBEDDINGS_PATH", tmp_path / "embeddings.npy")
+    monkeypatch.setattr(
+        db, "SECTION_MAPPING_PATH", tmp_path / "section_mapping.json"
+    )
+    monkeypatch.setattr(db, "_embeddings", None)
+    monkeypatch.setattr(db, "_section_id_to_idx", {})
     yield temp_db_path
     if temp_db_path.exists():
         temp_db_path.unlink()