-
Notifications
You must be signed in to change notification settings - Fork 0
β‘ Bolt: Cache numpy arrays in embedding search #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -502,6 +502,38 @@ def search_similar( | |
| model_used=self.config.model_name.value, | ||
| ) | ||
|
|
||
| def _compute_similarity( | ||
| self, item: Any, query_embedding: List[float], query_np: Any | ||
| ) -> Optional[float]: | ||
| # β‘ Bolt: Cache normalized numpy arrays safely to avoid regenerating on query | ||
| item_np = getattr(item, "_cached_np_embedding", None) | ||
|
|
||
| if item_np is None: | ||
| item_embedding = getattr(item, "embedding", None) | ||
| if item_embedding is None: | ||
| return None | ||
| if NUMPY_AVAILABLE and query_np is not None: | ||
| item_np = np.array(item_embedding) | ||
| if self.config.normalize_embeddings: | ||
| norm = np.linalg.norm(item_np) | ||
| if norm > 0: | ||
| item_np = item_np / norm | ||
| try: | ||
| setattr(item, "_cached_np_embedding", item_np) | ||
| except AttributeError: | ||
| pass # Skip caching if item is a strict class (e.g., uses slots) | ||
|
Comment on lines
+521
to
+524
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion | π Major Replace The β»οΈ Proposed fix try:
setattr(item, "_cached_np_embedding", item_np)
except AttributeError:
- pass # Skip caching if item is a strict class (e.g., uses slots)
+ logger.debug(
+ "Cannot cache numpy embedding on item %s: attribute assignment restricted",
+ type(item).__name__,
+ )As per coding guidelines: "Absolutely prohibit stubs, placeholders, pass statements, TODO comments, NotImplementedError, or FIXME comments; every implementation must be production-ready." π§° Toolsπͺ Ruff (0.15.7)[warning] 522-522: Do not call Replace (B010) π€ Prompt for AI Agents |
||
|
|
||
| if NUMPY_AVAILABLE and query_np is not None and item_np is not None: | ||
| return float(np.dot(query_np, item_np)) | ||
|
|
||
| item_embedding = getattr(item, "embedding", None) | ||
| if item_embedding is None: | ||
| return None | ||
| dot_product = sum(a * b for a, b in zip(query_embedding, item_embedding)) | ||
| norm_q = sum(x**2 for x in query_embedding) ** 0.5 | ||
| norm_i = sum(x**2 for x in item_embedding) ** 0.5 | ||
| return dot_product / (norm_q * norm_i) if norm_q * norm_i > 0 else 0.0 | ||
|
Comment on lines
+505
to
+535
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale cache risk when The cached Consider one of:
π‘οΈ Proposed fix: track normalization state with the cache- item_np = getattr(item, "_cached_np_embedding", None)
+ cached = getattr(item, "_cached_np_embedding", None)
+ cached_normalized = getattr(item, "_cached_np_normalized", None)
+
+ # Invalidate cache if normalization setting changed
+ if cached is not None and cached_normalized != self.config.normalize_embeddings:
+ cached = None
+ item_np = cached
if item_np is None:
item_embedding = getattr(item, "embedding", None)
if item_embedding is None:
return None
if NUMPY_AVAILABLE and query_np is not None:
item_np = np.array(item_embedding)
if self.config.normalize_embeddings:
norm = np.linalg.norm(item_np)
if norm > 0:
item_np = item_np / norm
try:
setattr(item, "_cached_np_embedding", item_np)
+ setattr(item, "_cached_np_normalized", self.config.normalize_embeddings)
except AttributeError:
- pass # Skip caching if item is a strict class (e.g., uses slots)
+ # Item class restricts attribute assignment (e.g., __slots__)
+ logger.debug("Cannot cache embedding on item: attribute assignment not allowed")π§° Toolsπͺ Ruff (0.15.7)[warning] 522-522: Do not call Replace (B010) [warning] 532-532: Add explicit value for parameter (B905) π€ Prompt for AI Agents |
||
|
|
||
| def _search_knowledge_items( | ||
| self, | ||
| query_embedding: List[float], | ||
|
|
@@ -526,36 +558,15 @@ def _search_knowledge_items( | |
| query_np = query_np / norm | ||
|
|
||
| for item in self._knowledge_items: | ||
| # Filter by knowledge type if specified | ||
| if knowledge_types: | ||
| item_type = getattr(item, "knowledge_type", "general") | ||
| if item_type not in [kt.value for kt in knowledge_types]: | ||
| continue | ||
|
|
||
| # Get item embedding | ||
| item_embedding = getattr(item, "embedding", None) | ||
| if item_embedding is None: | ||
| similarity = self._compute_similarity(item, query_embedding, query_np) | ||
| if similarity is None: | ||
| continue | ||
|
|
||
| # Calculate similarity | ||
| if NUMPY_AVAILABLE and query_np is not None: | ||
| item_np = np.array(item_embedding) | ||
| if self.config.normalize_embeddings: | ||
| norm = np.linalg.norm(item_np) | ||
| if norm > 0: | ||
| item_np = item_np / norm | ||
| similarity = float(np.dot(query_np, item_np)) | ||
| else: | ||
| # Simple cosine similarity | ||
| dot_product = sum( | ||
| a * b for a, b in zip(query_embedding, item_embedding) | ||
| ) | ||
| norm_q = sum(x**2 for x in query_embedding) ** 0.5 | ||
| norm_i = sum(x**2 for x in item_embedding) ** 0.5 | ||
| similarity = ( | ||
| dot_product / (norm_q * norm_i) if norm_q * norm_i > 0 else 0.0 | ||
| ) | ||
|
|
||
| # Apply threshold | ||
| if similarity >= min_similarity: | ||
| match = SimilarityMatch( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In
_compute_similarity, the new cache path writesitem_npback onto each knowledge item withsetattr, but it only catchesAttributeError. Strict model objects (for example, classes that reject unknown attributes via__setattr__) commonly raiseValueErrororTypeErrorinstead, which will now bubble up and fail the entire similarity search request for that item set. This optimization path should degrade gracefully (skip caching) rather than turning unsupported item types into request-time exceptions.Useful? React with πΒ / π.