fix: harden embeddings sync/rewrite scripts after #979 review

northdpole · cursoragent · northdpole · commit a2a02722eb58 · 2026-07-17T13:30:21.000+01:00
Skip empty vectors instead of inserting [], make SQLite rewrite
idempotent/rerun-safe, fail hard if local Postgres never becomes ready.

Co-authored-by: Cursor &lt;cursoragent@cursor.com&gt;
diff --git a/Makefile b/Makefile
@@ -57,10 +57,15 @@ docker-postgres:
 			-p 5432:5432 \
 			"$$wanted"; \
 	fi; \
-	for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do \
-		docker exec cre-postgres pg_isready -U cre -d cre >/dev/null 2>&1 && break; \
+	ready=0; \
+	for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30; do \
+		if docker exec cre-postgres pg_isready -U cre -d cre >/dev/null 2>&1; then ready=1; break; fi; \
 		sleep 1; \
 	done; \
+	if [ "$$ready" != "1" ]; then \
+		echo "error: cre-postgres did not become ready within 30s" >&2; \
+		exit 1; \
+	fi; \
 	docker exec cre-postgres psql -U cre -d cre -c "CREATE EXTENSION IF NOT EXISTS vector;" >/dev/null
 
 start-containers: docker-neo4j docker-redis
diff --git a/application/tests/embeddings_sync_scripts_test.py b/application/tests/embeddings_sync_scripts_test.py
@@ -0,0 +1,136 @@
+"""Unit tests for embeddings sync / SQLite rewrite helpers."""
+
+from __future__ import annotations
+
+import sqlite3
+import tempfile
+import unittest
+from pathlib import Path
+
+from scripts.rewrite_sqlite_embeddings_to_vec import rewrite
+from scripts.sync_embeddings_table import _as_vec_literal, _fetch_sqlite_embeddings
+
+
+class AsVecLiteralTest(unittest.TestCase):
+    def test_none_and_empty_return_none(self) -> None:
+        self.assertIsNone(_as_vec_literal(None))
+        self.assertIsNone(_as_vec_literal(""))
+        self.assertIsNone(_as_vec_literal("[]"))
+        self.assertIsNone(_as_vec_literal("  [ ]  "))
+
+    def test_literal_and_csv(self) -> None:
+        self.assertEqual(_as_vec_literal("[1.0,2.0]"), "[1.0,2.0]")
+        self.assertEqual(_as_vec_literal("1.0,2.0"), "[1.0,2.0]")
+
+
+class SyncSqliteFetchTest(unittest.TestCase):
+    def test_skips_empty_vectors(self) -> None:
+        with tempfile.TemporaryDirectory() as td:
+            path = Path(td) / "t.sqlite"
+            conn = sqlite3.connect(path)
+            conn.execute(
+                """
+                CREATE TABLE embeddings (
+                    embedding_vec TEXT,
+                    doc_type TEXT,
+                    cre_id TEXT,
+                    node_id TEXT,
+                    embeddings_content TEXT,
+                    embeddings_url TEXT
+                )
+                """
+            )
+            conn.executemany(
+                "INSERT INTO embeddings VALUES (?,?,?,?,?,?)",
+                [
+                    ("[0.1,0.2]", "CRE", "a", None, "x", None),
+                    (None, "CRE", "b", None, "y", None),
+                    ("[]", "CRE", "c", None, "z", None),
+                ],
+            )
+            conn.commit()
+            conn.close()
+            _cols, rows, skipped = _fetch_sqlite_embeddings(str(path))
+            self.assertEqual(len(rows), 1)
+            self.assertEqual(skipped, 2)
+            self.assertEqual(rows[0][0], "[0.1,0.2]")
+
+
+class RewriteSqliteTest(unittest.TestCase):
+    def test_idempotent_and_no_double_brackets(self) -> None:
+        with tempfile.TemporaryDirectory() as td:
+            path = Path(td) / "t.sqlite"
+            conn = sqlite3.connect(path)
+            conn.execute(
+                """
+                CREATE TABLE embeddings (
+                    id TEXT,
+                    doc_type TEXT NOT NULL,
+                    cre_id TEXT,
+                    node_id TEXT,
+                    embeddings TEXT NOT NULL,
+                    embeddings_content TEXT,
+                    embeddings_url TEXT
+                )
+                """
+            )
+            conn.execute(
+                "INSERT INTO embeddings VALUES (?,?,?,?,?,?,?)",
+                ("1", "CRE", "a", None, "0.1,0.2", "t", None),
+            )
+            conn.execute(
+                "INSERT INTO embeddings VALUES (?,?,?,?,?,?,?)",
+                ("2", "CRE", "b", None, "[0.3,0.4]", "t2", None),
+            )
+            conn.commit()
+            conn.close()
+
+            self.assertEqual(rewrite(str(path)), 0)
+            self.assertEqual(rewrite(str(path)), 0)  # already vec-only
+
+            conn = sqlite3.connect(path)
+            cols = {r[1] for r in conn.execute("PRAGMA table_info(embeddings)")}
+            self.assertIn("embedding_vec", cols)
+            self.assertNotIn("embeddings", cols)
+            vecs = [
+                r[0]
+                for r in conn.execute(
+                    "SELECT embedding_vec FROM embeddings ORDER BY id"
+                )
+            ]
+            self.assertEqual(vecs[0], "[0.1,0.2]")
+            self.assertEqual(vecs[1], "[0.3,0.4]")  # not [[0.3,0.4]]
+            conn.close()
+
+    def test_rerun_safe_after_leftover_embeddings_new(self) -> None:
+        with tempfile.TemporaryDirectory() as td:
+            path = Path(td) / "t.sqlite"
+            conn = sqlite3.connect(path)
+            conn.execute(
+                """
+                CREATE TABLE embeddings (
+                    id TEXT,
+                    doc_type TEXT NOT NULL,
+                    cre_id TEXT,
+                    node_id TEXT,
+                    embeddings TEXT NOT NULL,
+                    embeddings_content TEXT,
+                    embeddings_url TEXT
+                )
+                """
+            )
+            conn.execute(
+                "INSERT INTO embeddings VALUES (?,?,?,?,?,?,?)",
+                ("1", "CRE", "a", None, "1,2", None, None),
+            )
+            # Simulate interrupted prior run.
+            conn.execute(
+                "CREATE TABLE embeddings_new (id TEXT, doc_type TEXT, embedding_vec TEXT)"
+            )
+            conn.commit()
+            conn.close()
+            self.assertEqual(rewrite(str(path)), 0)
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/scripts/link_pci_dss_cre.py b/scripts/link_pci_dss_cre.py
@@ -38,6 +38,7 @@ def main() -> int:
 
     from application.cmd import cre_main
     from application.database import db
+    from application.database.pgvector_utils import parse_stored_embedding_vec
     from application.defs import cre_defs as defs
     from application.prompt_client import prompt_client
 
@@ -90,8 +91,6 @@ def main() -> int:
             skipped_no_match += 1
             continue
 
-        from application.database.pgvector_utils import parse_stored_embedding_vec
-
         control_embeddings = parse_stored_embedding_vec(emb_row.embedding_vec)
         if not control_embeddings:
             logger.warning(
diff --git a/scripts/rewrite_sqlite_embeddings_to_vec.py b/scripts/rewrite_sqlite_embeddings_to_vec.py
@@ -6,9 +6,9 @@
 Old ``standards_cache.sqlite`` files still have a CSV ``embeddings`` column —
 the ORM will refuse them. This script converts in place:
 
-  * ADD ``embedding_vec`` TEXT
-  * copy ``'[' || embeddings || ']'``
-  * DROP ``embeddings``
+  * ADD ``embedding_vec`` TEXT (if missing)
+  * copy CSV → ``[…]`` literal without double-bracketing
+  * DROP ``embeddings`` via table recreate (rerun-safe)
 
 Usage::
 
@@ -22,6 +22,13 @@
 import sys
 
 
+def _csv_to_literal(csv: str) -> str:
+    s = (csv or "").strip()
+    if s.startswith("[") and s.endswith("]"):
+        return s
+    return f"[{s}]"
+
+
 def rewrite(path: str) -> int:
     conn = sqlite3.connect(path)
     try:
@@ -32,19 +39,36 @@ def rewrite(path: str) -> int:
         if "embedding_vec" in cols and "embeddings" not in cols:
             print(f"{path}: already embedding_vec-only; nothing to do")
             return 0
+        if "embeddings" not in cols:
+            print(
+                f"error: {path} has embedding_vec but no CSV embeddings column "
+                "and is incomplete; refusing to rewrite",
+                file=sys.stderr,
+            )
+            return 2
         if "embedding_vec" not in cols:
             conn.execute("ALTER TABLE embeddings ADD COLUMN embedding_vec TEXT")
-        conn.execute(
+
+        # Copy only rows still missing a vector; never wrap an already-bracketed
+        # CSV twice (some caches may already store ``[1,2]`` in the CSV column).
+        rows = conn.execute(
             """
-            UPDATE embeddings
-            SET embedding_vec = '[' || embeddings || ']'
+            SELECT rowid, embeddings, embedding_vec
+            FROM embeddings
             WHERE embeddings IS NOT NULL
               AND trim(embeddings) <> ''
               AND (embedding_vec IS NULL OR trim(embedding_vec) = '')
             """
-        )
+        ).fetchall()
+        for rowid, csv, _existing in rows:
+            conn.execute(
+                "UPDATE embeddings SET embedding_vec = ? WHERE rowid = ?",
+                (_csv_to_literal(str(csv)), rowid),
+            )
+
         nulls = conn.execute(
-            "SELECT COUNT(*) FROM embeddings WHERE embedding_vec IS NULL OR trim(embedding_vec) = ''"
+            "SELECT COUNT(*) FROM embeddings "
+            "WHERE embedding_vec IS NULL OR trim(embedding_vec) = ''"
         ).fetchone()[0]
         if nulls:
             print(
@@ -55,7 +79,10 @@ def rewrite(path: str) -> int:
             conn.rollback()
             return 3
         conn.commit()
+
         # SQLite lacks reliable DROP COLUMN on older builds; recreate table.
+        # Drop any leftover embeddings_new from a prior interrupted run.
+        conn.execute("DROP TABLE IF EXISTS embeddings_new")
         src_cols = {
             r[1] for r in conn.execute("PRAGMA table_info(embeddings)").fetchall()
         }
diff --git a/scripts/sync_embeddings_table.py b/scripts/sync_embeddings_table.py
