diff --git a/src/typeagent/knowpro/add_messages.py b/src/typeagent/knowpro/add_messages.py index 295143a8..3362daff 100644 --- a/src/typeagent/knowpro/add_messages.py +++ b/src/typeagent/knowpro/add_messages.py @@ -13,6 +13,7 @@ from ..aitools.embeddings import IEmbeddingModel, NormalizedEmbedding from ..storage.memory import semrefindex +from .common import normalize_term from .interfaces import ( AddMessagesResult, IKnowledgeExtractor, @@ -244,13 +245,15 @@ def _collect_related_terms_for_fuzzy_index( """Collect canonical related-term texts for the fuzzy related-terms index. These terms are derived from the same knowledge that feeds semantic refs. - We lowercase and deduplicate while preserving order to match index behavior. + We normalize each term with `normalize_term` (strip, NFC, collapse + whitespace, lowercase), drop empty results, and deduplicate while + preserving order to match index behavior. """ seen: set[str] = set() related_terms: list[str] = [] def _add_term(term: str) -> None: - canonical = term.strip().lower() + canonical = normalize_term(term) if canonical and canonical not in seen: seen.add(canonical) related_terms.append(canonical) diff --git a/src/typeagent/knowpro/common.py b/src/typeagent/knowpro/common.py index 7634e413..0eebfc48 100644 --- a/src/typeagent/knowpro/common.py +++ b/src/typeagent/knowpro/common.py @@ -1,9 +1,23 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import re +import unicodedata + from .interfaces import SearchTerm +def normalize_term(term: str) -> str: + """Canonical form of an index term, shared by all storage backends. + + Strips surrounding whitespace, applies NFC normalization, collapses runs + of whitespace to a single space, and lowercases. Every term index must + apply this on both write and lookup so backends agree on term identity. + """ + term = unicodedata.normalize("NFC", term.strip()) + return re.sub(r"\s+", " ", term).lower() + + def is_search_term_wildcard(search_term: SearchTerm) -> bool: """Check if a search term is a wildcard.""" return search_term.term.text == "*" diff --git a/src/typeagent/knowpro/conversation_base.py b/src/typeagent/knowpro/conversation_base.py index 312cb9a2..b8b1adb9 100644 --- a/src/typeagent/knowpro/conversation_base.py +++ b/src/typeagent/knowpro/conversation_base.py @@ -22,6 +22,7 @@ from ..aitools import model_adapters, utils from ..aitools.embeddings import NormalizedEmbedding from ..storage.memory import semrefindex +from .common import normalize_term from .convsettings import ConversationSettings from .interfaces import ( AddMessagesResult, @@ -490,16 +491,17 @@ async def _update_related_terms_incremental( fuzzy_index = self.secondary_indexes.term_to_related_terms_index.fuzzy_index if fuzzy_index is not None and new_semrefs: - new_terms = set() + new_terms: set[str] = set() for semref in new_semrefs: knowledge = semref.knowledge if isinstance(knowledge, ConcreteEntity): - new_terms.add(knowledge.name.lower()) + new_terms.add(normalize_term(knowledge.name)) elif isinstance(knowledge, Topic): - new_terms.add(knowledge.text.lower()) + new_terms.add(normalize_term(knowledge.text)) elif isinstance(knowledge, Action): for verb in knowledge.verbs: - new_terms.add(verb.lower()) + new_terms.add(normalize_term(verb)) + new_terms.discard("") # Whitespace-only terms must not be embedded if new_terms: await fuzzy_index.add_terms(list(new_terms)) diff --git a/src/typeagent/storage/memory/propindex.py b/src/typeagent/storage/memory/propindex.py index ae928278..26dba174 100644 --- a/src/typeagent/storage/memory/propindex.py +++ b/src/typeagent/storage/memory/propindex.py @@ -6,6 +6,7 @@ from typing import assert_never from ...knowpro.collections import TextRangesInScope +from ...knowpro.common import normalize_term from ...knowpro.interfaces import ( IConversation, IPropertyToSemanticRefIndex, @@ -240,13 +241,12 @@ async def add_property( value: str, semantic_ref_ordinal: SemanticRefOrdinal | ScoredSemanticRefOrdinal, ) -> None: - term_text = make_property_term_text(property_name, value) + term_text = self._make_term_text(property_name, value) if isinstance(semantic_ref_ordinal, int): semantic_ref_ordinal = ScoredSemanticRefOrdinal( semantic_ref_ordinal, 1.0, ) - term_text = self._prepare_term_text(term_text) if term_text in self._map: self._map[term_text].append(semantic_ref_ordinal) else: @@ -269,12 +269,12 @@ async def lookup_property( property_name: str, value: str, ) -> list[ScoredSemanticRefOrdinal] | None: - term_text = make_property_term_text(property_name, value) - return self._map.get(self._prepare_term_text(term_text)) + return self._map.get(self._make_term_text(property_name, value)) async def remove_property(self, prop_name: str, semref_id: int) -> None: """Remove all properties for a specific property name and semantic ref.""" # Find and remove entries matching both property name and semref_id + prop_name = normalize_term(prop_name) keys_to_remove = [] for term_text, scored_refs in self._map.items(): prop_name_from_term, _ = split_property_term_text(term_text) @@ -315,7 +315,13 @@ async def remove_all_for_semref(self, semref_id: int) -> None: def _prepare_term_text(self, term_text: str) -> str: """Do any pre-processing of the term.""" - return term_text.lower() + return normalize_term(term_text) + + def _make_term_text(self, property_name: str, value: str) -> str: + """Build the normalized key; must match the SQLite property index.""" + return make_property_term_text( + normalize_term(property_name), normalize_term(value) + ) async def lookup_property_in_property_index( diff --git a/src/typeagent/storage/memory/semrefindex.py b/src/typeagent/storage/memory/semrefindex.py index edd1cba7..0c687cd1 100644 --- a/src/typeagent/storage/memory/semrefindex.py +++ b/src/typeagent/storage/memory/semrefindex.py @@ -8,6 +8,7 @@ from typechat import Failure from ...knowpro import convknowledge, secindex +from ...knowpro.common import normalize_term from ...knowpro.convsettings import ConversationSettings, SemanticRefIndexSettings from ...knowpro.interfaces import ( # Interfaces.; Other imports. IConversation, @@ -735,10 +736,10 @@ async def deserialize(self, data: TermToSemanticRefIndexData) -> None: scored_refs = [ ScoredSemanticRefOrdinal.deserialize(s) for s in scored_refs_data ] - self._map[term] = scored_refs + self._map.setdefault(term, []).extend(scored_refs) def _prepare_term(self, term: str) -> str: - return term.lower() + return normalize_term(term) # ... diff --git a/src/typeagent/storage/sqlite/propindex.py b/src/typeagent/storage/sqlite/propindex.py index c2fc274e..96c3d8bb 100644 --- a/src/typeagent/storage/sqlite/propindex.py +++ b/src/typeagent/storage/sqlite/propindex.py @@ -6,12 +6,12 @@ from collections.abc import Sequence import sqlite3 +from ...knowpro.common import normalize_term from ...knowpro.interfaces import ( IPropertyToSemanticRefIndex, ScoredSemanticRefOrdinal, SemanticRefOrdinal, ) -from ...storage.memory import propindex class SqlitePropertyIndex(IPropertyToSemanticRefIndex): @@ -49,12 +49,8 @@ async def add_property( score = 1.0 # Normalize property name and value (to match in-memory implementation) - term_text = propindex.make_property_term_text(property_name, value) - term_text = term_text.lower() # Matches PropertyIndex._prepare_term_text - property_name, value = propindex.split_property_term_text(term_text) - # Remove "prop." prefix that was added by make_property_term_text - if property_name.startswith("prop."): - property_name = property_name[5:] + property_name = normalize_term(property_name) + value = normalize_term(value) cursor = self.db.cursor() cursor.execute( @@ -85,12 +81,9 @@ async def add_properties_batch( else: semref_id = ordinal score = 1.0 - term_text = propindex.make_property_term_text(property_name, value) - term_text = term_text.lower() - property_name, value = propindex.split_property_term_text(term_text) - if property_name.startswith("prop."): - property_name = property_name[5:] - rows.append((property_name, value, score, semref_id)) + rows.append( + (normalize_term(property_name), normalize_term(value), score, semref_id) + ) cursor = self.db.cursor() cursor.executemany( "INSERT INTO PropertyIndex (prop_name, value_str, score, semref_id) VALUES (?, ?, ?, ?)", @@ -107,12 +100,8 @@ async def lookup_property( value: str, ) -> list[ScoredSemanticRefOrdinal] | None: # Normalize property name and value (to match in-memory implementation) - term_text = propindex.make_property_term_text(property_name, value) - term_text = term_text.lower() # Matches PropertyIndex._prepare_term_text - property_name, value = propindex.split_property_term_text(term_text) - # Remove "prop." prefix that was added by make_property_term_text - if property_name.startswith("prop."): - property_name = property_name[5:] + property_name = normalize_term(property_name) + value = normalize_term(value) cursor = self.db.cursor() cursor.execute( @@ -132,7 +121,7 @@ async def remove_property(self, prop_name: str, semref_id: int) -> None: cursor = self.db.cursor() cursor.execute( "DELETE FROM PropertyIndex WHERE prop_name = ? AND semref_id = ?", - (prop_name, semref_id), + (normalize_term(prop_name), semref_id), ) async def remove_all_for_semref(self, semref_id: int) -> None: diff --git a/src/typeagent/storage/sqlite/semrefindex.py b/src/typeagent/storage/sqlite/semrefindex.py index f462955e..2e07191e 100644 --- a/src/typeagent/storage/sqlite/semrefindex.py +++ b/src/typeagent/storage/sqlite/semrefindex.py @@ -4,10 +4,9 @@ """SQLite-based semantic reference index implementation.""" from collections.abc import Sequence -import re import sqlite3 -import unicodedata +from ...knowpro.common import normalize_term from ...knowpro.interfaces import ( ITermToSemanticRefIndex, ScoredSemanticRefOrdinal, @@ -150,7 +149,7 @@ async def deserialize(self, data: TermToSemanticRefIndexData) -> None: # Prepare all insertion data for bulk operation insertion_data = [] for item in data["items"]: - if item and item["term"]: + if item and item.get("term") is not None: term = self._prepare_term(item["term"]) for semref_ordinal_data in item["semanticRefOrdinals"]: if isinstance(semref_ordinal_data, dict): @@ -168,15 +167,4 @@ async def deserialize(self, data: TermToSemanticRefIndexData) -> None: ) def _prepare_term(self, term: str) -> str: - """Normalize term by converting to lowercase, stripping whitespace, and normalizing Unicode.""" - # Strip leading/trailing whitespace - term = term.strip() - - # Normalize Unicode to NFC form (canonical composition) - term = unicodedata.normalize("NFC", term) - - # Collapse multiple whitespace characters to single space - term = re.sub(r"\s+", " ", term) - - # Convert to lowercase - return term.lower() + return normalize_term(term) diff --git a/tests/test_term_normalization.py b/tests/test_term_normalization.py new file mode 100644 index 00000000..e414070f --- /dev/null +++ b/tests/test_term_normalization.py @@ -0,0 +1,146 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Memory and SQLite term indexes must normalize terms identically (#322).""" + +from collections.abc import Generator +import sqlite3 + +import pytest + +from typeagent.knowpro.common import normalize_term +from typeagent.knowpro.interfaces import ( + IPropertyToSemanticRefIndex, + ITermToSemanticRefIndex, +) +from typeagent.storage.memory.propindex import PropertyIndex +from typeagent.storage.memory.semrefindex import TermToSemanticRefIndex +from typeagent.storage.sqlite.propindex import SqlitePropertyIndex +from typeagent.storage.sqlite.schema import init_db_schema +from typeagent.storage.sqlite.semrefindex import SqliteTermToSemanticRefIndex + + +@pytest.mark.parametrize( + "raw, expected", + [ + ("Space Needle ", "space needle"), + ("A ", "a"), + ("\tfoo\n bar", "foo bar"), + ("Café", "café"), # NFD -> NFC + (" ", ""), + ("", ""), + ], +) +def test_normalize_term(raw: str, expected: str) -> None: + assert normalize_term(raw) == expected + + +@pytest.fixture(params=["memory", "sqlite"]) +def term_index( + request: pytest.FixtureRequest, +) -> Generator[ITermToSemanticRefIndex, None, None]: + if request.param == "memory": + yield TermToSemanticRefIndex() + else: + db = sqlite3.connect(":memory:") + init_db_schema(db) + yield SqliteTermToSemanticRefIndex(db) + db.close() + + +@pytest.fixture(params=["memory", "sqlite"]) +def prop_index( + request: pytest.FixtureRequest, +) -> Generator[IPropertyToSemanticRefIndex, None, None]: + if request.param == "memory": + yield PropertyIndex() + else: + db = sqlite3.connect(":memory:") + init_db_schema(db) + yield SqlitePropertyIndex(db) + db.close() + + +@pytest.mark.asyncio +async def test_term_index_normalizes_whitespace_and_case( + term_index: ITermToSemanticRefIndex, +) -> None: + assert await term_index.add_term("Space Needle ", 1) == "space needle" + assert await term_index.get_terms() == ["space needle"] + found = await term_index.lookup_term("space needle") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + found = await term_index.lookup_term(" SPACE\tNEEDLE") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + + +@pytest.mark.asyncio +async def test_term_index_lookup_ignores_trailing_space( + term_index: ITermToSemanticRefIndex, +) -> None: + await term_index.add_term("A", 0) + found = await term_index.lookup_term("A ") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [0] + + +@pytest.mark.asyncio +async def test_term_index_whitespace_only_terms_agree( + term_index: ITermToSemanticRefIndex, +) -> None: + await term_index.add_term(" ", 0) + await term_index.add_term("\t", 0) + assert await term_index.size() == 1 + assert await term_index.get_terms() == [""] + + +@pytest.mark.asyncio +async def test_term_index_deserialize_merges_colliding_terms( + term_index: ITermToSemanticRefIndex, +) -> None: + data = { + "items": [ + { + "term": "Foo", + "semanticRefOrdinals": [{"semanticRefOrdinal": 1, "score": 1.0}], + }, + { + "term": "foo ", + "semanticRefOrdinals": [{"semanticRefOrdinal": 2, "score": 1.0}], + }, + ] + } + await term_index.deserialize(data) # type: ignore[arg-type] + found = await term_index.lookup_term("foo") + assert found is not None + assert sorted(r.semantic_ref_ordinal for r in found) == [1, 2] + + +@pytest.mark.asyncio +async def test_property_index_normalizes_values( + prop_index: IPropertyToSemanticRefIndex, +) -> None: + await prop_index.add_property("name", "Space Needle ", 1) + found = await prop_index.lookup_property("name", "space needle") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + found = await prop_index.lookup_property("NAME", " Space\tNeedle") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + + +@pytest.mark.asyncio +async def test_property_index_remove_property_normalizes_name( + prop_index: IPropertyToSemanticRefIndex, +) -> None: + await prop_index.add_property(" Name ", "x", 1) + await prop_index.remove_property(" NAME", 1) + assert not await prop_index.lookup_property("name", "x") + + +@pytest.mark.asyncio +async def test_term_index_roundtrips_empty_normalized_term( + term_index: ITermToSemanticRefIndex, +) -> None: + await term_index.add_term(" ", 3) + data = await term_index.serialize() + await term_index.clear() + await term_index.deserialize(data) + found = await term_index.lookup_term("") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [3]