Skip to content

Commit 9743b79

Browse files
fix: address CodeRabbit pgvector review on #979
Use can_use_pgvector_similarity for Librarian backend readiness, degrade similarity query errors to no-match, and document deferred HNSW on Essential-1. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 2eb5902 commit 9743b79

5 files changed

Lines changed: 58 additions & 21 deletions

File tree

application/cmd/cre_main.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,15 +1115,14 @@ def run_librarian(
11151115

11161116
backend = RetrieverBackend(cfg.retriever_backend)
11171117
if backend is RetrieverBackend.pgvector:
1118-
dialect = database.session.connection().dialect.name
1119-
if dialect != "postgresql":
1118+
# Readiness must include the embedding_vec column (not dialect alone):
1119+
# Postgres pre-migration would otherwise fail on every retrieve().
1120+
if not database.can_use_pgvector_similarity():
11201121
logger.warning(
1121-
"CRE_LIBRARIAN_RETRIEVER_BACKEND=pgvector selected on a %r "
1122-
"database, but the pgvector backend needs Postgres with the "
1123-
"embedding_vec column (Alembic c7d8e9f0a1b2 / #977) and will "
1124-
"fail at retrieve() "
1125-
"time here. Set the backend to in_memory until then.",
1126-
dialect,
1122+
"CRE_LIBRARIAN_RETRIEVER_BACKEND=pgvector selected, but "
1123+
"Postgres is not ready for pgvector similarity (need Alembic "
1124+
"c7d8e9f0a1b2 / #977 embedding_vec). Set the backend to "
1125+
"in_memory until then."
11271126
)
11281127
# The CRE ids present in the hub are exactly the known ids the explicit
11291128
# resolver may auto-link to (W2 seeded this from the golden set; here it is

application/database/db.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2585,19 +2585,28 @@ def find_most_similar_embedding_id(
25852585
"q": to_pgvector_literal(query_embedding),
25862586
"doc_type": doc_type,
25872587
}
2588-
# Prefer the Session so we stay inside the app transaction; fall back
2589-
# to a Connection from the Engine for SQLAlchemy 2.
2590-
if hasattr(self.session, "execute"):
2591-
row = self.session.execute(sql_text(sql), params).fetchone()
2592-
else:
2593-
with bind.connect() as conn:
2594-
row = conn.execute(sql_text(sql), params).fetchone()
2595-
if not row:
2596-
return None, None
2597-
score = float(row.score)
2598-
if score < similarity_threshold:
2588+
try:
2589+
# Prefer the Session so we stay inside the app transaction; fall
2590+
# back to a Connection from the Engine for SQLAlchemy 2.
2591+
if hasattr(self.session, "execute"):
2592+
row = self.session.execute(sql_text(sql), params).fetchone()
2593+
else:
2594+
with bind.connect() as conn:
2595+
row = conn.execute(sql_text(sql), params).fetchone()
2596+
if not row:
2597+
return None, None
2598+
score = float(row.score)
2599+
if score < similarity_threshold:
2600+
return None, None
2601+
return str(row.object_id), score
2602+
except Exception as exc:
2603+
# Match prior sklearn-path resilience: chat/import should degrade
2604+
# to "no match" rather than 500 on a transient driver/query error.
2605+
logger.warning(
2606+
"pgvector similarity query failed (%s); returning no match",
2607+
exc,
2608+
)
25992609
return None, None
2600-
return str(row.object_id), score
26012610

26022611
def assert_embedding_contract(
26032612
self,

application/database/pgvector_utils.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
only where there is enough RAM (local or a high-resource worker), then
66
sync/backfill vectors. Alembic on prod only: enable extension, add column,
77
copy CSV→vector for matching dims.
8+
9+
HNSW/IVFFlat indexes are deferred (Essential-1 capacity); see migration
10+
``c7d8e9f0a1b2`` docstring for the follow-up plan.
811
"""
912

1013
from __future__ import annotations

application/tests/prompt_client_pgvector_similarity_test.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Chat/import similarity prefers pgvector when the DB reports it ready."""
22

33
import unittest
4-
from unittest.mock import MagicMock
4+
from unittest.mock import MagicMock, patch
55

66
from application.prompt_client import prompt_client as prompt_client_mod
77

@@ -44,5 +44,26 @@ def test_cre_paginated_uses_pgvector_when_available(self) -> None:
4444
self.assertEqual(kwargs["id_column"], "cre_id")
4545

4646

47+
class FindMostSimilarEmbeddingIdResilienceTest(unittest.TestCase):
48+
def test_query_error_returns_no_match(self) -> None:
49+
from application.database.db import Node_collection
50+
51+
database = Node_collection.__new__(Node_collection)
52+
session = MagicMock()
53+
session.execute.side_effect = RuntimeError("driver boom")
54+
session.get_bind.return_value = MagicMock()
55+
database.session = session
56+
57+
with patch("application.database.db.logger"):
58+
match_id, score = database.find_most_similar_embedding_id(
59+
[0.1, 0.2, 0.3],
60+
doc_type="CRE",
61+
id_column="cre_id",
62+
similarity_threshold=0.7,
63+
)
64+
self.assertIsNone(match_id)
65+
self.assertIsNone(score)
66+
67+
4768
if __name__ == "__main__":
4869
unittest.main()

migrations/versions/c7d8e9f0a1b2_add_embedding_vec_pgvector.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
OOM-killed. This migration only: CREATE EXTENSION vector, ADD COLUMN
1010
embedding_vec, and copy matching-dim CSV → vector. Fix missing / wrong-dim
1111
rows offline on a high-RAM machine, then re-run / backfill.
12+
13+
ANN index (HNSW/IVFFlat) is intentionally deferred: opencreorg runs on
14+
Heroku Postgres Essential-1; add a follow-up migration with
15+
``CREATE INDEX ... USING hnsw (embedding_vec vector_cosine_ops)`` once
16+
dims/backfill are settled and plan capacity allows.
1217
"""
1318

1419
from alembic import op

0 commit comments

Comments
 (0)