fix: coderabbit comments

This commit is contained in:
Rajat Ahuja 2025-12-05 16:29:20 -05:00
parent 60fde82218
commit 1988f18fbe
5 changed files with 23 additions and 13 deletions

View File

@ -442,15 +442,18 @@ class AppSettings(HonchoSettings):
"""Propagate top-level NAMESPACE to nested settings if not explicitly set.
After this validator runs, CACHE.NAMESPACE, METRICS.NAMESPACE, and
VECTOR_STORE.NAMESPACE are guaranteed to exist.
VECTOR_STORE.NAMESPACE are guaranteed to exist. Explicitly provided
nested namespaces are preserved.
"""
if self.CACHE.NAMESPACE is None:
self.CACHE.NAMESPACE = self.NAMESPACE
if self.METRICS.NAMESPACE is None:
self.METRICS.NAMESPACE = self.NAMESPACE
# Note: VECTOR_STORE.NAMESPACE has its own default of "honcho",
# but we propagate the top-level NAMESPACE if the user explicitly set it
# and wants consistency across all namespaced services
vector_namespace_explicit = "NAMESPACE" in self.VECTOR_STORE.model_fields_set
if not vector_namespace_explicit:
self.VECTOR_STORE.NAMESPACE = self.NAMESPACE
return self

View File

@ -655,11 +655,17 @@ async def is_rejected_duplicate(
namespace = vector_store.get_document_namespace(
workspace_name, observer, observed
)
await vector_store.delete_many(namespace, [existing_doc.id])
vector_deleted = False
try:
await vector_store.delete_many(namespace, [existing_doc.id])
vector_deleted = True
except Exception:
existing_doc.deleted_at = datetime.datetime.now(datetime.timezone.utc)
await db.flush()
# Delete from database after vector store succeeds
await db.delete(existing_doc)
await db.flush() # Flush to make deletion visible in this transaction
if vector_deleted:
await db.delete(existing_doc)
await db.flush() # Flush to make deletion visible in this transaction
return False # Don't reject the new document
@ -700,7 +706,7 @@ async def cleanup_soft_deleted_documents(
Returns:
Count of documents cleaned up (only those where vector deletion succeeded)
"""
cutoff = datetime.datetime.now(datetime.UTC) - datetime.timedelta(
cutoff = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(
minutes=older_than_minutes
)

View File

@ -190,10 +190,10 @@ async def create_messages(
):
with attempt:
await vector_store.upsert_many(namespace, vector_records)
except Exception as e:
except Exception:
# Final attempt failed - log but don't raise
# MessageEmbedding records exist in DB, vectors can be added later
logger.error(f"Failed to upsert message vectors after retries: {e}")
logger.exception("Failed to upsert message vectors after retries")
except Exception:
logger.exception(

View File

@ -432,6 +432,7 @@ async def delete_session(
)
)
embeddings = embedding_result.all()
vector_store = get_vector_store()
if embeddings:
# Build vector IDs: {message_id}_{chunk_index}
@ -439,7 +440,6 @@ async def delete_session(
# Try to delete from vector store (best effort)
try:
vector_store = get_vector_store()
namespace = vector_store.get_message_namespace(workspace_name)
await vector_store.delete_many(namespace, vector_ids)
logger.debug(
@ -479,7 +479,6 @@ async def delete_session(
if documents:
# Group document IDs by namespace (observer/observed)
docs_by_namespace: dict[str, list[str]] = {}
vector_store = get_vector_store()
for doc in documents:
namespace = vector_store.get_document_namespace(
workspace_name, doc.observer, doc.observed

View File

@ -11,6 +11,7 @@ def prepare_add_chunk_index_to_message_embeddings(
verifier: MigrationVerifier,
) -> None:
"""Seed state and assertions before upgrading to f1a2b3c4d5e6."""
verifier.assert_column_exists("message_embeddings", "embedding", nullable=False)
# Verify chunk_index column doesn't exist before migration
verifier.assert_column_exists("message_embeddings", "chunk_index", exists=False)
@ -22,3 +23,4 @@ def verify_add_chunk_index_to_message_embeddings(
"""Add assertions validating the effects of f1a2b3c4d5e6."""
# Verify chunk_index column was added with correct properties
verifier.assert_column_exists("message_embeddings", "chunk_index", nullable=False)
verifier.assert_column_exists("message_embeddings", "embedding", nullable=True)