diff --git a/CHANGELOG.md b/CHANGELOG.md index d29f2d0a..2ec89284 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,14 @@ letter followed by a combining mark, such as `T̈`). Indexes stored by earlier versions keep such terms in the old form, so they may no longer match; re-ingest to rebuild the indexes. +- Search terms, query term matching (e.g. entity boosting) and the + related-terms alias index in memory and SQLite now use the same + normalization as the term indexes (#346). Previously search terms and query + matching only lowercased, and the alias index stored and matched keys + verbatim, so a search for `"space needle"` missed the boost for an entity + named `"Space Needle"`, and an alias added for `"Space Needle"` was never + found by a search. SQLite aliases stored by earlier versions keep their old + keys and may no longer match; re-ingest such databases to rebuild them. #### Core typeagent package - Replace `CamelCaseField` (and its module `typeagent.knowpro.field_helpers`) diff --git a/src/typeagent/knowpro/query.py b/src/typeagent/knowpro/query.py index 005ef7a3..10a552db 100644 --- a/src/typeagent/knowpro/query.py +++ b/src/typeagent/knowpro/query.py @@ -20,7 +20,7 @@ TextRangeCollection, TextRangesInScope, ) -from .common import is_search_term_wildcard +from .common import is_search_term_wildcard, normalize_term from .dataclasses import dataclass as pydantic_dataclass from .interfaces import ( DateRange, @@ -128,12 +128,14 @@ async def get_text_range_for_date_range( def get_matching_term_for_text(search_term: SearchTerm, text: str) -> Term | None: - # Do case-INSENSITIVE comparisons, since stored entities may have different case. - if text.lower() == search_term.term.text.lower(): + # Compare normalized text, since stored entities may differ in case, + # whitespace or Unicode form. + text = normalize_term(text) + if text == normalize_term(search_term.term.text): return search_term.term if search_term.related_terms: for related_term in search_term.related_terms: - if text.lower() == related_term.text.lower(): + if text == normalize_term(related_term.text): return related_term return None diff --git a/src/typeagent/knowpro/search.py b/src/typeagent/knowpro/search.py index 6c3d738c..fcc74c3a 100644 --- a/src/typeagent/knowpro/search.py +++ b/src/typeagent/knowpro/search.py @@ -9,6 +9,7 @@ from ..storage.memory import related_terms_index from ..storage.memory.message_index import IMessageTextEmbeddingIndex from .collections import MessageAccumulator, SemanticRefAccumulator +from .common import normalize_term from .dataclasses import dataclass from .interfaces import ( IConversation, @@ -542,7 +543,7 @@ def validate_and_prepare_search_term(self, search_term: CompiledSearchTerm) -> b related_term.weight = self.default_term_match_weight return True - # Currently, just changes the case of a term + # Currently, just normalizes a term the same way the term indexes do # But here, we may do other things like: # - Check for noise terms # - Do additional rewriting @@ -550,7 +551,7 @@ def validate_and_prepare_search_term(self, search_term: CompiledSearchTerm) -> b # Return false if the term should be rejected def validate_and_prepare_term(self, term: Term | None) -> bool: if term: - term.text = term.text.lower() + term.text = normalize_term(term.text) return True def boost_entities( diff --git a/src/typeagent/knowpro/searchlib.py b/src/typeagent/knowpro/searchlib.py index fe6d7532..6486994a 100644 --- a/src/typeagent/knowpro/searchlib.py +++ b/src/typeagent/knowpro/searchlib.py @@ -9,6 +9,7 @@ from typing import cast from ..storage.memory.property_index import PropertyNames +from .common import normalize_term from .interfaces import ( ISemanticRefCollection, KnowledgePropertyName, @@ -149,7 +150,7 @@ def _parse_search_term(text: str) -> SearchTerm | None: """Parse a search term string with optional related terms separated by ';'.""" term_strings = _split_term_values(text, ";") if len(term_strings) > 0: - term_strings = [t.lower() for t in term_strings] + term_strings = [normalize_term(t) for t in term_strings] search_term = SearchTerm(term=Term(text=term_strings[0])) if len(term_strings) > 1: search_term.related_terms = [] diff --git a/src/typeagent/storage/memory/related_terms_index.py b/src/typeagent/storage/memory/related_terms_index.py index b7d325d6..54c2cace 100644 --- a/src/typeagent/storage/memory/related_terms_index.py +++ b/src/typeagent/storage/memory/related_terms_index.py @@ -14,7 +14,7 @@ VectorBase, ) from typeagent.knowpro.collections import TermSet -from typeagent.knowpro.common import is_search_term_wildcard +from typeagent.knowpro.common import is_search_term_wildcard, normalize_term from typeagent.knowpro.convsettings import RelatedTermIndexSettings from typeagent.knowpro.interfaces import ( IConversation, @@ -45,13 +45,16 @@ async def add_related_term( ) -> None: if not isinstance(related_terms, list): related_terms = [related_terms] - terms: dict[str, Term] = self.map.setdefault(text, {}) + terms: dict[str, Term] = self.map.setdefault(normalize_term(text), {}) for related in related_terms: # Store a copy so the caller's Term can't alias the index. - terms.setdefault(related.text, dataclasses.replace(related)) + related_text = normalize_term(related.text) + terms.setdefault( + related_text, dataclasses.replace(related, text=related_text) + ) async def lookup_term(self, text: str) -> list[Term] | None: - result = self.map.get(text) + result = self.map.get(normalize_term(text)) if result: # Return copies: search preparation lowercases and reweights # related terms in place, which must not change the index. @@ -60,7 +63,7 @@ async def lookup_term(self, text: str) -> list[Term] | None: return None async def remove_term(self, text: str) -> None: - self.map.pop(text, None) + self.map.pop(normalize_term(text), None) async def clear(self) -> None: self.map.clear() diff --git a/src/typeagent/storage/sqlite/related_terms_index.py b/src/typeagent/storage/sqlite/related_terms_index.py index 9097002d..c6769bc9 100644 --- a/src/typeagent/storage/sqlite/related_terms_index.py +++ b/src/typeagent/storage/sqlite/related_terms_index.py @@ -9,6 +9,7 @@ from ...aitools.embeddings import NormalizedEmbedding from ...aitools.vectorbase import TextEmbeddingIndexSettings, VectorBase +from ...knowpro.common import normalize_term from ...knowpro.interfaces import ( ITermToRelatedTerms, ITermToRelatedTermsFuzzy, @@ -31,7 +32,10 @@ def __init__(self, db: sqlite3.Connection): async def lookup_term(self, text: str) -> list[Term] | None: cursor = self.db.cursor() - cursor.execute("SELECT alias FROM RelatedTermsAliases WHERE term = ?", (text,)) + cursor.execute( + "SELECT alias FROM RelatedTermsAliases WHERE term = ?", + (normalize_term(text),), + ) results = [Term(row[0]) for row in cursor.fetchall()] return results if results else None @@ -42,23 +46,27 @@ async def add_related_term( if isinstance(related_terms, Term): related_terms = [related_terms] + text = normalize_term(text) cursor = self.db.cursor() # Add new aliases (use INSERT OR IGNORE to avoid duplicates) for related_term in related_terms: cursor.execute( "INSERT OR IGNORE INTO RelatedTermsAliases (term, alias) VALUES (?, ?)", - (text, related_term.text), + (text, normalize_term(related_term.text)), ) async def remove_term(self, text: str) -> None: cursor = self.db.cursor() - cursor.execute("DELETE FROM RelatedTermsAliases WHERE term = ?", (text,)) + cursor.execute( + "DELETE FROM RelatedTermsAliases WHERE term = ?", (normalize_term(text),) + ) async def clear(self) -> None: cursor = self.db.cursor() cursor.execute("DELETE FROM RelatedTermsAliases") async def set_related_terms(self, term: str, related_terms: list[str]) -> None: + term = normalize_term(term) cursor = self.db.cursor() # Clear existing aliases for this term cursor.execute("DELETE FROM RelatedTermsAliases WHERE term = ?", (term,)) @@ -66,7 +74,7 @@ async def set_related_terms(self, term: str, related_terms: list[str]) -> None: for alias in related_terms: cursor.execute( "INSERT OR IGNORE INTO RelatedTermsAliases (term, alias) VALUES (?, ?)", - (term, alias), + (term, normalize_term(alias)), ) async def size(self) -> int: @@ -116,9 +124,9 @@ async def deserialize(self, data: TermToRelatedTermsData | None) -> None: if data is not None: for item in data.get("relatedTerms") or []: if item and item.get("termText") and item.get("relatedTerms"): - term = item["termText"] + term = normalize_term(item["termText"]) for term_data in item["relatedTerms"]: - insertion_data.append((term, term_data["text"])) + insertion_data.append((term, normalize_term(term_data["text"]))) cursor = self.db.cursor() cursor.execute("DELETE FROM RelatedTermsAliases") diff --git a/tests/test_sqlite_indexes.py b/tests/test_sqlite_indexes.py index 4961f3db..61a34eed 100644 --- a/tests/test_sqlite_indexes.py +++ b/tests/test_sqlite_indexes.py @@ -263,7 +263,7 @@ async def test_aliases_operations(self, sqlite_db: sqlite3.Connection): term_texts = [term.text for term in results] assert "artificial intelligence" in term_texts assert "machine learning" in term_texts - assert "ML" in term_texts + assert "ml" in term_texts # Aliases are stored normalized @pytest.mark.asyncio async def test_serialize_deserialize(self, sqlite_db: sqlite3.Connection): diff --git a/tests/test_term_normalization.py b/tests/test_term_normalization.py index cfb2c6d7..70e1ea59 100644 --- a/tests/test_term_normalization.py +++ b/tests/test_term_normalization.py @@ -5,17 +5,31 @@ from collections.abc import Generator import sqlite3 +from typing import cast import pytest from typeagent.knowpro.common import normalize_term from typeagent.knowpro.interfaces import ( + IConversation, IPropertyToSemanticRefIndex, + ITermToRelatedTerms, ITermToSemanticRefIndex, + SearchTerm, + Term, ) +from typeagent.knowpro.knowledge_schema import ConcreteEntity +from typeagent.knowpro.query import ( + get_matching_term_for_text, + match_entity_name_or_type, +) +from typeagent.knowpro.search import QueryCompiler +from typeagent.knowpro.searchlib import create_search_terms from typeagent.storage.memory.property_index import PropertyIndex +from typeagent.storage.memory.related_terms_index import TermToRelatedTermsMap from typeagent.storage.memory.semantic_ref_index import TermToSemanticRefIndex from typeagent.storage.sqlite.property_index import SqlitePropertyIndex +from typeagent.storage.sqlite.related_terms_index import SqliteRelatedTermsAliases from typeagent.storage.sqlite.schema import init_db_schema from typeagent.storage.sqlite.semantic_ref_index import SqliteTermToSemanticRefIndex @@ -77,6 +91,19 @@ def prop_index( db.close() +@pytest.fixture(params=["memory", "sqlite"]) +def alias_index( + request: pytest.FixtureRequest, +) -> Generator[ITermToRelatedTerms, None, None]: + if request.param == "memory": + yield TermToRelatedTermsMap() + else: + db = sqlite3.connect(":memory:") + init_db_schema(db) + yield SqliteRelatedTermsAliases(db) + db.close() + + @pytest.mark.asyncio async def test_term_index_normalizes_whitespace_and_case( term_index: ITermToSemanticRefIndex, @@ -218,3 +245,101 @@ async def test_term_index_normalization_round_trips( assert await term_index.get_terms() == [stored] found = await term_index.lookup_term(term) assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + + +# Search side and alias index (#346) + + +def test_create_search_terms_normalizes() -> None: + [search_term] = create_search_terms([" Space\tNeedle ;Seattle Tower"]) + assert search_term.term.text == "space needle" + assert search_term.related_terms is not None + assert [t.text for t in search_term.related_terms] == ["seattle tower"] + + +def test_validate_and_prepare_term_normalizes() -> None: + # validate_and_prepare_term doesn't touch the conversation. + compiler = QueryCompiler(cast(IConversation, None), None) + term = Term(" Space\tNeedle ") + assert compiler.validate_and_prepare_term(term) + assert term.text == "space needle" + + +def test_get_matching_term_for_text_normalizes() -> None: + related_term = Term("seattle tower") + search_term = SearchTerm(Term("space needle"), related_terms=[related_term]) + assert get_matching_term_for_text(search_term, "Space Needle") is ( + search_term.term + ) + assert get_matching_term_for_text(search_term, " Seattle\tTower") is related_term + assert get_matching_term_for_text(search_term, "space") is None + + +def test_match_entity_name_or_type_normalizes() -> None: + entity = ConcreteEntity(name="Space Needle", type=["Tourist Attraction"]) + assert match_entity_name_or_type(SearchTerm(Term("space needle")), entity) + assert match_entity_name_or_type(SearchTerm(Term("tourist attraction")), entity) + entity = ConcreteEntity(name="Cafe\u0301", type=[]) # NFD + assert match_entity_name_or_type(SearchTerm(Term("café")), entity) + + +@pytest.mark.asyncio +async def test_alias_index_normalizes_key_and_aliases( + alias_index: ITermToRelatedTerms, +) -> None: + await alias_index.add_related_term("Space Needle", Term(" Seattle\tTower ")) + found = await alias_index.lookup_term(" SPACE\tNEEDLE") + assert found is not None and [t.text for t in found] == ["seattle tower"] + found = await alias_index.lookup_term("space needle") + assert found is not None and [t.text for t in found] == ["seattle tower"] + + await alias_index.remove_term("Space Needle ") + assert await alias_index.lookup_term("space needle") is None + + +@pytest.mark.asyncio +async def test_alias_index_normalizes_unicode_form( + alias_index: ITermToRelatedTerms, +) -> None: + await alias_index.add_related_term("Cafe\u0301", Term("coffee shop")) # NFD + found = await alias_index.lookup_term("café") # NFC + assert found is not None and [t.text for t in found] == ["coffee shop"] + + +@pytest.mark.asyncio +async def test_alias_index_merges_aliases_of_colliding_keys( + alias_index: ITermToRelatedTerms, +) -> None: + await alias_index.add_related_term("Space Needle", Term("tower")) + await alias_index.add_related_term("space needle", [Term("Tower"), Term("spire")]) + found = await alias_index.lookup_term("space needle") + assert found is not None and sorted(t.text for t in found) == ["spire", "tower"] + assert await alias_index.size() == 1 + + +@pytest.mark.asyncio +async def test_alias_index_deserialize_normalizes( + alias_index: ITermToRelatedTerms, +) -> None: + await alias_index.deserialize( + { + "relatedTerms": [ + { + "termText": "Space Needle", + "relatedTerms": [{"text": "Seattle Tower"}], + }, + {"termText": "space needle", "relatedTerms": [{"text": "spire"}]}, + ] + } + ) + found = await alias_index.lookup_term("space needle") + assert found is not None + assert sorted(t.text for t in found) == ["seattle tower", "spire"] + + # A round trip keeps the normalized form. + data = await alias_index.serialize() + await alias_index.clear() + await alias_index.deserialize(data) + found = await alias_index.lookup_term(" Space\tNeedle") + assert found is not None + assert sorted(t.text for t in found) == ["seattle tower", "spire"]