Misc bug fixes (#208)

Fixed a bunch of things found by asking Claude for a review. It found several nits, a logic bug in the code to determine whether a set of text ranges all lie inside another, and a real doozie: a mutable default in `SemanticRefAccumulator.__init__`. Quite a few new tests were added, too.

Author: bmerkle

src/typeagent/aitools/utils.py

@@ -191,7 +191,7 @@ def parse_azure_endpoint(
     if not azure_endpoint:
         raise RuntimeError(f"Environment variable {endpoint_envvar} not found")
 
-    m = re.search(r"[?,]api-version=([\d-]+(?:preview)?)", azure_endpoint)
+    m = re.search(r"[?&]api-version=([\d-]+(?:preview)?)", azure_endpoint)
     if not m:
         raise RuntimeError(
             f"{endpoint_envvar}={azure_endpoint} doesn't contain valid api-version field"

src/typeagent/aitools/vectorbase.py

@@ -150,8 +150,9 @@ def fuzzy_lookup_embedding_in_subset(
         max_hits: int | None = None,
         min_score: float | None = None,
     ) -> list[ScoredInt]:
+        ordinals_set = set(ordinals_of_subset)
         return self.fuzzy_lookup_embedding(
-            embedding, max_hits, min_score, lambda i: i in ordinals_of_subset
+            embedding, max_hits, min_score, lambda i: i in ordinals_set
         )
 
     async def fuzzy_lookup(

src/typeagent/emails/email_import.py

@@ -263,7 +263,8 @@ def _merge_chunks(
                 yield cur_chunk
             cur_chunk = new_chunk
         else:
-            cur_chunk += separator
+            if cur_chunk:
+                cur_chunk += separator
             cur_chunk += new_chunk
 
     if (len(cur_chunk)) > 0:

src/typeagent/knowpro/answers.py

@@ -404,11 +404,12 @@ async def get_enclosing_date_range_for_text_range(
     start_timestamp = (await messages.get_item(range.start.message_ordinal)).timestamp
     if not start_timestamp:
         return None
-    end_timestamp = (
-        (await messages.get_item(range.end.message_ordinal)).timestamp
-        if range.end
-        else None
-    )
+    end_timestamp: str | None = None
+    if range.end:
+        end_ordinal = range.end.message_ordinal
+        if end_ordinal < await messages.size():
+            end_timestamp = (await messages.get_item(end_ordinal)).timestamp
+        # else: range extends to the end of the conversation; leave as None.
     return DateRange(
         start=Datetime.fromisoformat(start_timestamp),
         end=Datetime.fromisoformat(end_timestamp) if end_timestamp else None,
@@ -535,7 +536,7 @@ def facets_to_merged_facets(facets: list[Facet]) -> MergedFacets:
     merged_facets: MergedFacets = {}
     for facet in facets:
         name = facet.name.lower()
-        value = str(facet).lower()
+        value = str(facet.value).lower()
         merged_facets.setdefault(name, []).append(value)
     return merged_facets
 

src/typeagent/knowpro/collections.py

@@ -91,10 +91,14 @@ def add(self, value: T, score: float, is_exact_match: bool = True) -> None:
                     )
                 )
             else:
+                # New related-only match: hit_count stays 0 because
+                # only exact matches count as direct hits.  This matters
+                # for select_with_hit_count / _matches_with_min_hit_count
+                # which filter on hit_count to weed out noise.
                 self.set_match(
                     Match(
                         value,
-                        hit_count=1,
+                        hit_count=0,
                         score=0.0,
                         related_hit_count=1,
                         related_score=score,
@@ -250,9 +254,11 @@ def smooth_match_score[T](match: Match[T]) -> None:
 
 
 class SemanticRefAccumulator(MatchAccumulator[SemanticRefOrdinal]):
-    def __init__(self, search_term_matches: set[str] = set()):
+    def __init__(self, search_term_matches: set[str] | None = None):
         super().__init__()
-        self.search_term_matches = search_term_matches
+        self.search_term_matches = (
+            search_term_matches if search_term_matches is not None else set()
+        )
 
     def add_term_matches(
         self,
@@ -330,8 +336,7 @@ async def group_matches_by_type(
             semantic_ref = await semantic_refs.get_item(match.value)
             group = groups.get(semantic_ref.knowledge.knowledge_type)
             if group is None:
-                group = SemanticRefAccumulator()
-                group.search_term_matches = self.search_term_matches
+                group = SemanticRefAccumulator(self.search_term_matches)
                 groups[semantic_ref.knowledge.knowledge_type] = group
             group.set_match(match)
         return groups
@@ -513,11 +518,10 @@ def add_ranges(self, text_ranges: "list[TextRange] | TextRangeCollection") -> No
             for text_range in text_ranges._ranges:
                 self.add_range(text_range)
 
-    def is_in_range(self, inner_range: TextRange) -> bool:
-        if len(self._ranges) == 0:
-            return False
-        i = bisect.bisect_left(self._ranges, inner_range)
-        for outer_range in self._ranges[i:]:
+    def contains_range(self, inner_range: TextRange) -> bool:
+        # Since ranges are sorted by start, once we pass inner_range's start
+        # no further range can contain it.
+        for outer_range in self._ranges:
             if outer_range.start > inner_range.start:
                 break
             if inner_range in outer_range:
@@ -544,7 +548,7 @@ def is_range_in_scope(self, inner_range: TextRange) -> bool:
             # We have a very simple impl: we don't intersect/union ranges yet.
             # Instead, we ensure that the inner range is not rejected by any outer ranges.
             for outer_ranges in self.text_ranges:
-                if not outer_ranges.is_in_range(inner_range):
+                if not outer_ranges.contains_range(inner_range):
                     return False
         return True
 

src/typeagent/knowpro/interfaces_search.py

@@ -18,13 +18,14 @@
 )
 
 __all__ = [
-    "SearchTerm",
     "KnowledgePropertyName",
     "PropertySearchTerm",
+    "SearchSelectExpr",
+    "SearchTerm",
     "SearchTermGroup",
     "SearchTermGroupTypes",
+    "SemanticRefSearchResult",
     "WhenFilter",
-    "SearchSelectExpr",
 ]
 
 
@@ -142,15 +143,3 @@ class SemanticRefSearchResult:
 
     term_matches: set[str]
     semantic_ref_matches: list[ScoredSemanticRefOrdinal]
-
-
-__all__ = [
-    "KnowledgePropertyName",
-    "PropertySearchTerm",
-    "SearchSelectExpr",
-    "SearchTerm",
-    "SearchTermGroup",
-    "SearchTermGroupTypes",
-    "SemanticRefSearchResult",
-    "WhenFilter",
-]

src/typeagent/knowpro/query.py

@@ -45,6 +45,7 @@
     Thread,
 )
 from .kplib import ConcreteEntity
+from .utils import aenumerate
 
 # TODO: Move to compilelib.py
 type BooleanOp = Literal["and", "or", "or_max"]
@@ -101,11 +102,14 @@ async def get_text_range_for_date_range(
     messages = conversation.messages
     range_start_ordinal: MessageOrdinal = -1
     range_end_ordinal = range_start_ordinal
-    async for message in messages:
-        if Datetime.fromisoformat(message.timestamp) in date_range:
+    async for ordinal, message in aenumerate(messages):
+        if (
+            message.timestamp
+            and Datetime.fromisoformat(message.timestamp) in date_range
+        ):
             if range_start_ordinal < 0:
-                range_start_ordinal = message.ordinal
-            range_end_ordinal = message.ordinal
+                range_start_ordinal = ordinal
+            range_end_ordinal = ordinal
         else:
             if range_start_ordinal >= 0:
                 # We have a range, so break.
@@ -696,7 +700,7 @@ class WhereSemanticRefExpr(QueryOpExpr[SemanticRefAccumulator]):
 
     async def eval(self, context: QueryEvalContext) -> SemanticRefAccumulator:
         accumulator = await self.source_expr.eval(context)
-        filtered = SemanticRefAccumulator(accumulator.search_term_matches)
+        filtered = SemanticRefAccumulator(set(accumulator.search_term_matches))
 
         # Filter matches asynchronously
         filtered_matches = []

src/typeagent/knowpro/search.py

@@ -90,11 +90,9 @@ class SearchOptions:
 
     def __repr__(self):
         parts = []
-        for key in dir(self):
-            if not key.startswith("_"):
-                value = getattr(self, key)
-                if value is not None:
-                    parts.append(f"{key}={value!r}")
+        for key, value in vars(self).items():
+            if not key.startswith("_") and value is not None:
+                parts.append(f"{key}={value!r}")
         return f"{self.__class__.__name__}({', '.join(parts)})"
 
 

src/typeagent/knowpro/searchlang.py

@@ -83,11 +83,9 @@ class LanguageSearchOptions(SearchOptions):
 
     def __repr__(self):
         parts = []
-        for key in dir(self):
-            if not key.startswith("_"):
-                value = getattr(self, key)
-                if value is not None:
-                    parts.append(f"{key}={value!r}")
+        for key, value in vars(self).items():
+            if not key.startswith("_") and value is not None:
+                parts.append(f"{key}={value!r}")
         return f"{self.__class__.__name__}({', '.join(parts)})"
 
 
@@ -371,6 +369,9 @@ def compile_action_term_as_search_terms(
             self.compile_entity_terms_as_search_terms(
                 action_term.additional_entities, action_group
             )
+        # only append the nested or_max wrapper when created one (use_or_max) and it's non-empty.
+        if use_or_max and action_group.terms:
+            term_group.terms.append(action_group)
         return term_group
 
     def compile_search_terms(
@@ -609,21 +610,6 @@ def add_entity_name_to_group(
                 exact_match_value,
             )
 
-    def add_search_term_to_groupadd_entity_name_to_group(
-        self,
-        entity_term: EntityTerm,
-        property_name: PropertyNames,
-        term_group: SearchTermGroup,
-        exact_match_value: bool = False,
-    ) -> None:
-        if not entity_term.is_name_pronoun:
-            self.add_property_term_to_group(
-                property_name.value,
-                entity_term.name,
-                term_group,
-                exact_match_value,
-            )
-
     def add_property_term_to_group(
         self,
         property_name: str,

src/typeagent/knowpro/utils.py

@@ -3,9 +3,18 @@
 
 """Utility functions for the knowpro package."""
 
+from collections.abc import AsyncIterable
+
 from .interfaces import MessageOrdinal, TextLocation, TextRange
 
 
+async def aenumerate[T](aiterable: AsyncIterable[T], start: int = 0):
+    i = start
+    async for item in aiterable:
+        yield i, item
+        i += 1
+
+
 def text_range_from_message_chunk(
     message_ordinal: MessageOrdinal,
     chunk_ordinal: int = 0,